Merge pull request #42 from AnExiledDev/worktree-codebase-review-fixes

Fix critical security findings from codebase review

Author: AnExiledDev

.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)

.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:

.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:

.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, ""
 
 

.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__":

.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__":

.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,26 +86,55 @@
 # ---------------------------------------------------------------------------
 # 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/",
 )
 
 
 # ---------------------------------------------------------------------------
 # 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)

.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