Release v2.0.2: scope guard hardening + stop hook throttling#48
Open
AnExiledDev wants to merge 1 commit intomainfrom
Open
Release v2.0.2: scope guard hardening + stop hook throttling#48AnExiledDev wants to merge 1 commit intomainfrom
AnExiledDev wants to merge 1 commit intomainfrom
Conversation
Scope guard: resolve CWD with realpath to prevent symlink mismatches, detect .claude/worktrees/ and expand scope to project root so sibling worktrees aren't blocked, and improve error messages with resolved paths. Stop hooks: add 5-minute per-session cooldown to commit-reminder and spec-reminder to prevent repeated firing in team/agent scenarios.
📝 WalkthroughWalkthroughThe changes introduce a 5-minute per-session cooldown mechanism for commit and spec reminders to reduce repeated messages, and enhance workspace scope handling with automatic worktree detection that expands scope to the project root while using Changes
Sequence Diagram(s)sequenceDiagram
participant Session as Session
participant Script as Reminder Script
participant Cooldown as Cooldown State<br/>(/tmp)
participant Reminder as Reminder Logic
rect rgba(100, 150, 200, 0.5)
Note over Session,Reminder: Invocation Check
Script->>Session: Get session_id
Script->>Cooldown: Check if on_cooldown(session_id)
Cooldown-->>Script: cooldown active?
alt Cooldown Active
Script->>Script: Exit early
else Cooldown Inactive
rect rgba(150, 200, 100, 0.5)
Note over Reminder: Emit Reminder
Script->>Reminder: Evaluate conditions & prepare message
Reminder-->>Script: Reminder ready
Script->>Cooldown: Touch cooldown file<br/>(set timestamp)
Script->>Session: Emit reminder to user
end
end
end
sequenceDiagram
participant Agent as Agent/Script
participant CWD as Current Working<br/>Directory
participant Scope as Scope Resolution
participant Guard as Scope Guard
rect rgba(150, 100, 200, 0.5)
Note over Agent,Guard: Path Resolution & Scope Expansion
Agent->>CWD: Read CWD
Agent->>Scope: realpath(CWD)
Scope-->>Agent: Resolved CWD path
Agent->>Scope: detect .claude/worktrees/<br/>in resolved path?
alt Inside Worktree
Scope-->>Agent: scope_root = project_root
else Outside Worktree
Scope-->>Agent: scope_root = CWD
end
Agent->>Guard: Check if target within<br/>scope_root
Guard-->>Agent: Allow/Block decision
alt Path Mismatch
Guard->>Agent: Include resolved path<br/>in error message
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py (1)
246-248:⚠️ Potential issue | 🟡 MinorBash block reasons now mislabel
scope_rootas the “working directory.”After Line 348,
check_bash_scope()receivesscope_root, but rejection text still says “outside the working directory,” which is misleading in worktree sessions.Proposed fix
-def check_bash_scope(command: str, cwd: str) -> None: +def check_bash_scope(command: str, scope_root: str) -> None: @@ - if cwd == "/workspaces": + if scope_root == "/workspaces": return @@ - if not is_in_scope(resolved, cwd) and not is_allowlisted(resolved): + if not is_in_scope(resolved, scope_root) and not is_allowlisted(resolved): @@ - f"outside the working directory ({cwd}).", + f"outside the scope root ({scope_root}).", @@ - if not is_in_scope(resolved, cwd) and not is_allowlisted(resolved): + if not is_in_scope(resolved, scope_root) and not is_allowlisted(resolved): @@ - f"outside the working directory ({cwd}).", + f"outside the scope root ({scope_root}).",Also applies to: 278-325, 348-348
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py around lines 246 - 248, The rejection message in check_bash_scope incorrectly calls scope_root the “working directory”; update the error/ rejection text inside the check_bash_scope function (and the related Bash-block messages in the same file ranges) to refer to the workspace root/scope root (e.g., "outside the workspace root" or "outside the scope root") instead of "outside the working directory" so the message matches the actual value passed in; search for string occurrences in check_bash_scope and the Bash-block handling (around the previously noted ranges) and replace the misleading wording while preserving existing context and exit behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py:
- Around line 96-113: The cooldown filename currently embeds the raw session_id
into cooldown_path in both _is_on_cooldown and _touch_cooldown, which is unsafe;
instead derive a safe filename (e.g., hash the session_id with hashlib.sha256
and use the hex digest) and join it with a safe temp dir (e.g.,
tempfile.gettempdir() or pathlib.Path) so no path separators or traversal are
possible; update both functions to compute the sanitized filename once (e.g.,
safe_name = sha256(session_id.encode()).hexdigest()) and use that to build
cooldown_path before calling os.path.getmtime or opening the file.
In
@.devcontainer/plugins/devs-marketplace/plugins/spec-workflow/scripts/spec-reminder.py:
- Around line 46-63: The cooldown file name currently interpolates session_id
directly in _is_on_cooldown and _touch_cooldown, allowing path-injection;
instead derive a safe filename (e.g. hash the session_id with hashlib.sha256 and
use the hex digest) or otherwise sanitize/escape it, then build the
cooldown_path from that safe token (e.g.
f"/tmp/claude-spec-reminder-cooldown-{safe_token}"); keep the same logic for
reading/writing but use the safe token in both _is_on_cooldown and
_touch_cooldown so reads and writes target only intended files.
In
@.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/inject-workspace-cwd.py:
- Around line 42-44: The two adjacent f-strings use different variables (cwd vs
scope_root) producing conflicting scope text; update the first f-string to
consistently reference scope_root (or explicitly show both) so the message
aligns—e.g., replace "Working Directory: {cwd}" with "Working Directory:
{scope_root}" or "Working Directory: {scope_root} (cwd: {cwd})" so the
instruction consistently enforces the same boundary; change the string in the
code that builds the message where cwd and scope_root are used.
---
Outside diff comments:
In
@.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py:
- Around line 246-248: The rejection message in check_bash_scope incorrectly
calls scope_root the “working directory”; update the error/ rejection text
inside the check_bash_scope function (and the related Bash-block messages in the
same file ranges) to refer to the workspace root/scope root (e.g., "outside the
workspace root" or "outside the scope root") instead of "outside the working
directory" so the message matches the actual value passed in; search for string
occurrences in check_bash_scope and the Bash-block handling (around the
previously noted ranges) and replace the misleading wording while preserving
existing context and exit behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.devcontainer/CHANGELOG.md.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py.devcontainer/plugins/devs-marketplace/plugins/spec-workflow/scripts/spec-reminder.py.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/README.md.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/inject-workspace-cwd.py
Comment on lines
+96
to
+113
| def _is_on_cooldown(session_id: str) -> bool: | ||
| """Check if the reminder fired recently. Returns True to suppress.""" | ||
| cooldown_path = f"/tmp/claude-commit-reminder-cooldown-{session_id}" | ||
| try: | ||
| mtime = os.path.getmtime(cooldown_path) | ||
| return (time.time() - mtime) < COOLDOWN_SECS | ||
| except OSError: | ||
| return False | ||
|
|
||
|
|
||
| def _touch_cooldown(session_id: str) -> None: | ||
| """Mark the cooldown as active.""" | ||
| cooldown_path = f"/tmp/claude-commit-reminder-cooldown-{session_id}" | ||
| try: | ||
| with open(cooldown_path, "w") as f: | ||
| f.write("") | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
Apply the same cooldown-path hardening here.
The cooldown filename is built from unsanitized session_id in /tmp, creating the same path-manipulation risk in this hook.
Proposed fix
import json
import os
+import re
import subprocess
import sys
import time
+import tempfile
GIT_CMD_TIMEOUT = 5
COOLDOWN_SECS = 300 # 5 minutes between reminders per session
+_SAFE_SESSION_ID_RE = re.compile(r"[^A-Za-z0-9_.-]")
+
+
+def _cooldown_path(session_id: str) -> str:
+ safe_session_id = _SAFE_SESSION_ID_RE.sub("_", session_id)
+ return os.path.join(
+ tempfile.gettempdir(),
+ f"claude-commit-reminder-cooldown-{safe_session_id}",
+ )
@@
def _is_on_cooldown(session_id: str) -> bool:
"""Check if the reminder fired recently. Returns True to suppress."""
- cooldown_path = f"/tmp/claude-commit-reminder-cooldown-{session_id}"
+ cooldown_path = _cooldown_path(session_id)
@@
def _touch_cooldown(session_id: str) -> None:
"""Mark the cooldown as active."""
- cooldown_path = f"/tmp/claude-commit-reminder-cooldown-{session_id}"
+ cooldown_path = _cooldown_path(session_id)
try:
- with open(cooldown_path, "w") as f:
- f.write("")
+ fd = os.open(cooldown_path, os.O_WRONLY | os.O_CREAT, 0o600)
+ os.close(fd)
+ os.utime(cooldown_path, None)
except OSError:
pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _is_on_cooldown(session_id: str) -> bool: | |
| """Check if the reminder fired recently. Returns True to suppress.""" | |
| cooldown_path = f"/tmp/claude-commit-reminder-cooldown-{session_id}" | |
| try: | |
| mtime = os.path.getmtime(cooldown_path) | |
| return (time.time() - mtime) < COOLDOWN_SECS | |
| except OSError: | |
| return False | |
| def _touch_cooldown(session_id: str) -> None: | |
| """Mark the cooldown as active.""" | |
| cooldown_path = f"/tmp/claude-commit-reminder-cooldown-{session_id}" | |
| try: | |
| with open(cooldown_path, "w") as f: | |
| f.write("") | |
| except OSError: | |
| pass | |
| import json | |
| import os | |
| import re | |
| import subprocess | |
| import sys | |
| import time | |
| import tempfile | |
| GIT_CMD_TIMEOUT = 5 | |
| COOLDOWN_SECS = 300 # 5 minutes between reminders per session | |
| _SAFE_SESSION_ID_RE = re.compile(r"[^A-Za-z0-9_.-]") | |
| def _cooldown_path(session_id: str) -> str: | |
| safe_session_id = _SAFE_SESSION_ID_RE.sub("_", session_id) | |
| return os.path.join( | |
| tempfile.gettempdir(), | |
| f"claude-commit-reminder-cooldown-{safe_session_id}", | |
| ) | |
| def _is_on_cooldown(session_id: str) -> bool: | |
| """Check if the reminder fired recently. Returns True to suppress.""" | |
| cooldown_path = _cooldown_path(session_id) | |
| try: | |
| mtime = os.path.getmtime(cooldown_path) | |
| return (time.time() - mtime) < COOLDOWN_SECS | |
| except OSError: | |
| return False | |
| def _touch_cooldown(session_id: str) -> None: | |
| """Mark the cooldown as active.""" | |
| cooldown_path = _cooldown_path(session_id) | |
| try: | |
| fd = os.open(cooldown_path, os.O_WRONLY | os.O_CREAT, 0o600) | |
| os.close(fd) | |
| os.utime(cooldown_path, None) | |
| except OSError: | |
| pass |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 98-98: Probable insecure usage of temporary file or directory: "/tmp/claude-commit-reminder-cooldown-"
(S108)
[error] 108-108: Probable insecure usage of temporary file or directory: "/tmp/claude-commit-reminder-cooldown-"
(S108)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.devcontainer/plugins/devs-marketplace/plugins/session-context/scripts/commit-reminder.py
around lines 96 - 113, The cooldown filename currently embeds the raw session_id
into cooldown_path in both _is_on_cooldown and _touch_cooldown, which is unsafe;
instead derive a safe filename (e.g., hash the session_id with hashlib.sha256
and use the hex digest) and join it with a safe temp dir (e.g.,
tempfile.gettempdir() or pathlib.Path) so no path separators or traversal are
possible; update both functions to compute the sanitized filename once (e.g.,
safe_name = sha256(session_id.encode()).hexdigest()) and use that to build
cooldown_path before calling os.path.getmtime or opening the file.
Comment on lines
+46
to
+63
| def _is_on_cooldown(session_id: str) -> bool: | ||
| """Check if the reminder fired recently. Returns True to suppress.""" | ||
| cooldown_path = f"/tmp/claude-spec-reminder-cooldown-{session_id}" | ||
| try: | ||
| mtime = os.path.getmtime(cooldown_path) | ||
| return (time.time() - mtime) < COOLDOWN_SECS | ||
| except OSError: | ||
| return False | ||
|
|
||
|
|
||
| def _touch_cooldown(session_id: str) -> None: | ||
| """Mark the cooldown as active.""" | ||
| cooldown_path = f"/tmp/claude-spec-reminder-cooldown-{session_id}" | ||
| try: | ||
| with open(cooldown_path, "w") as f: | ||
| f.write("") | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
Harden cooldown file path construction against path injection.
session_id is interpolated directly into a /tmp/... path. A malformed session_id (path separators, traversal tokens) can redirect reads/writes outside the intended cooldown file location.
Proposed fix
import json
import os
+import re
import subprocess
import sys
import time
+import tempfile
GIT_CMD_TIMEOUT = 5
COOLDOWN_SECS = 300 # 5 minutes between reminders per session
+_SAFE_SESSION_ID_RE = re.compile(r"[^A-Za-z0-9_.-]")
+
+
+def _cooldown_path(session_id: str) -> str:
+ safe_session_id = _SAFE_SESSION_ID_RE.sub("_", session_id)
+ return os.path.join(
+ tempfile.gettempdir(),
+ f"claude-spec-reminder-cooldown-{safe_session_id}",
+ )
def _is_on_cooldown(session_id: str) -> bool:
"""Check if the reminder fired recently. Returns True to suppress."""
- cooldown_path = f"/tmp/claude-spec-reminder-cooldown-{session_id}"
+ cooldown_path = _cooldown_path(session_id)
try:
mtime = os.path.getmtime(cooldown_path)
return (time.time() - mtime) < COOLDOWN_SECS
@@
def _touch_cooldown(session_id: str) -> None:
"""Mark the cooldown as active."""
- cooldown_path = f"/tmp/claude-spec-reminder-cooldown-{session_id}"
+ cooldown_path = _cooldown_path(session_id)
try:
- with open(cooldown_path, "w") as f:
- f.write("")
+ fd = os.open(cooldown_path, os.O_WRONLY | os.O_CREAT, 0o600)
+ os.close(fd)
+ os.utime(cooldown_path, None)
except OSError:
pass🧰 Tools
🪛 Ruff (0.15.2)
[error] 48-48: Probable insecure usage of temporary file or directory: "/tmp/claude-spec-reminder-cooldown-"
(S108)
[error] 58-58: Probable insecure usage of temporary file or directory: "/tmp/claude-spec-reminder-cooldown-"
(S108)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.devcontainer/plugins/devs-marketplace/plugins/spec-workflow/scripts/spec-reminder.py
around lines 46 - 63, The cooldown file name currently interpolates session_id
directly in _is_on_cooldown and _touch_cooldown, allowing path-injection;
instead derive a safe filename (e.g. hash the session_id with hashlib.sha256 and
use the hex digest) or otherwise sanitize/escape it, then build the
cooldown_path from that safe token (e.g.
f"/tmp/claude-spec-reminder-cooldown-{safe_token}"); keep the same logic for
reading/writing but use the safe token in both _is_on_cooldown and
_touch_cooldown so reads and writes target only intended files.
Comment on lines
+42
to
44
| f"Working Directory: {cwd} — restrict all file operations to this directory unless explicitly instructed otherwise.\n" | ||
| f"All file operations and commands MUST target paths within {scope_root}. " | ||
| f"Do not read, write, or execute commands against paths outside this directory." |
There was a problem hiding this comment.
Scope instruction text is inconsistent in worktree mode.
Line 42 frames the boundary as cwd, while Line 43 enforces scope_root; when they differ, the instruction conflicts.
Proposed fix
- context = (
- f"Working Directory: {cwd} — restrict all file operations to this directory unless explicitly instructed otherwise.\n"
- f"All file operations and commands MUST target paths within {scope_root}. "
- f"Do not read, write, or execute commands against paths outside this directory."
- )
+ context = (
+ f"Working Directory: {cwd}\n"
+ f"Scope Root: {scope_root} — restrict all file operations and commands to this boundary.\n"
+ "Do not read, write, or execute commands against paths outside this scope root."
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/inject-workspace-cwd.py
around lines 42 - 44, The two adjacent f-strings use different variables (cwd vs
scope_root) producing conflicting scope text; update the first f-string to
consistently reference scope_root (or explicitly show both) so the message
aligns—e.g., replace "Working Directory: {cwd}" with "Working Directory:
{scope_root}" or "Working Directory: {scope_root} (cwd: {cwd})" so the
instruction consistently enforces the same boundary; change the string in the
code that builds the message where cwd and scope_root are used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/worktrees/in CWD and expands scope to project root, fixing false positives where sibling worktrees were blockedos.path.realpath()to match how target paths are resolved, preventing symlink/bind-mount mismatchesTest plan
Summary by CodeRabbit
New Features
Bug Fixes
Documentation