diff --git a/.claude/hooks/skill_guard.py b/.claude/hooks/skill_guard.py index c80cad1..2750a17 100755 --- a/.claude/hooks/skill_guard.py +++ b/.claude/hooks/skill_guard.py @@ -32,6 +32,10 @@ def emit_event(*_a: object, **_k: object) -> None: ) SHELL_ENV_PREFIX_RE = re.compile(r"^(?:[A-Za-z_][A-Za-z0-9_]*=\S+\s+)+") +SHELL_OUTPUT_REDIRECT_TOKENS = {">", ">>", ">|", "&>", "&>>", ">&", ">>&"} +SHELL_OUTPUT_REDIRECT_FALLBACK_RE = re.compile( + r"(^|\s)(?:&>>|&>|[0-9]*(?:>\||>>|>|>&|>>&))" +) SHELL_ALLOWED_SEGMENTS = ( re.compile(r"^(?:cd|pwd|true|false|echo|printf|export|unset|set(?:\s+-[A-Za-z-]+)?)\b"), re.compile(r"^git\s+(?:status|rev-parse|symbolic-ref|branch|log|show|diff|fetch|remote|config\s+--get|worktree\s+list|ls-files|submodule\s+status|stash\s+(?:list|show))\b"), @@ -362,6 +366,34 @@ def split_shell_segments(command: str) -> list[str]: return segments +def shell_segment_tokens(segment: str) -> list[str]: + try: + lexer = shlex.shlex(segment, posix=True, punctuation_chars="|&;<>") + lexer.whitespace_split = True + lexer.commenters = "" + return list(lexer) + except ValueError: + return [] + + +def shell_segment_has_output_redirection(segment: str) -> bool: + tokens = shell_segment_tokens(segment) + if not tokens: + return bool(SHELL_OUTPUT_REDIRECT_FALLBACK_RE.search(segment)) + for index, token in enumerate(tokens): + if token not in SHELL_OUTPUT_REDIRECT_TOKENS: + continue + target = tokens[index + 1] if index + 1 < len(tokens) else "" + if target == "&" and index + 2 < len(tokens) and tokens[index + 2].isdigit(): + continue + if token in {">&", ">>&"} and target.isdigit(): + continue + if target == "/dev/null": + continue + return True + return False + + def is_allowed_non_agent_shell_command(command: str) -> bool: normalized = command.strip() if not normalized: @@ -370,6 +402,8 @@ def is_allowed_non_agent_shell_command(command: str) -> bool: if not segments: return True for raw_segment in segments: + if shell_segment_has_output_redirection(raw_segment): + return False segment = normalize_shell_segment(raw_segment) if not segment: continue diff --git a/.codex/hooks/skill_guard.py b/.codex/hooks/skill_guard.py index c80cad1..2750a17 100755 --- a/.codex/hooks/skill_guard.py +++ b/.codex/hooks/skill_guard.py @@ -32,6 +32,10 @@ def emit_event(*_a: object, **_k: object) -> None: ) SHELL_ENV_PREFIX_RE = re.compile(r"^(?:[A-Za-z_][A-Za-z0-9_]*=\S+\s+)+") +SHELL_OUTPUT_REDIRECT_TOKENS = {">", ">>", ">|", "&>", "&>>", ">&", ">>&"} +SHELL_OUTPUT_REDIRECT_FALLBACK_RE = re.compile( + r"(^|\s)(?:&>>|&>|[0-9]*(?:>\||>>|>|>&|>>&))" +) SHELL_ALLOWED_SEGMENTS = ( re.compile(r"^(?:cd|pwd|true|false|echo|printf|export|unset|set(?:\s+-[A-Za-z-]+)?)\b"), re.compile(r"^git\s+(?:status|rev-parse|symbolic-ref|branch|log|show|diff|fetch|remote|config\s+--get|worktree\s+list|ls-files|submodule\s+status|stash\s+(?:list|show))\b"), @@ -362,6 +366,34 @@ def split_shell_segments(command: str) -> list[str]: return segments +def shell_segment_tokens(segment: str) -> list[str]: + try: + lexer = shlex.shlex(segment, posix=True, punctuation_chars="|&;<>") + lexer.whitespace_split = True + lexer.commenters = "" + return list(lexer) + except ValueError: + return [] + + +def shell_segment_has_output_redirection(segment: str) -> bool: + tokens = shell_segment_tokens(segment) + if not tokens: + return bool(SHELL_OUTPUT_REDIRECT_FALLBACK_RE.search(segment)) + for index, token in enumerate(tokens): + if token not in SHELL_OUTPUT_REDIRECT_TOKENS: + continue + target = tokens[index + 1] if index + 1 < len(tokens) else "" + if target == "&" and index + 2 < len(tokens) and tokens[index + 2].isdigit(): + continue + if token in {">&", ">>&"} and target.isdigit(): + continue + if target == "/dev/null": + continue + return True + return False + + def is_allowed_non_agent_shell_command(command: str) -> bool: normalized = command.strip() if not normalized: @@ -370,6 +402,8 @@ def is_allowed_non_agent_shell_command(command: str) -> bool: if not segments: return True for raw_segment in segments: + if shell_segment_has_output_redirection(raw_segment): + return False segment = normalize_shell_segment(raw_segment) if not segment: continue diff --git a/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/.openspec.yaml b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/.openspec.yaml new file mode 100644 index 0000000..0a064c1 --- /dev/null +++ b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-28 diff --git a/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/proposal.md b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/proposal.md new file mode 100644 index 0000000..9490718 --- /dev/null +++ b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/proposal.md @@ -0,0 +1,14 @@ +## Why + +- Recodee exposed a Guardex hook gap: after `Write` was blocked on protected `dev`, Claude could attempt the same file write through an allowlisted Bash command such as `cat > file`. Guardex should classify shell redirection before treating commands like `cat`, `printf`, or `echo` as read-only. + +## What Changes + +- Add shell output-redirection detection to the managed `.codex`, `.claude`, and `.agents` `skill_guard.py` hooks. +- Preserve safe diagnostic redirection such as `2>&1` and `2>/dev/null`. +- Add setup-level regression coverage that imports each managed hook copy and proves the bypass is blocked. + +## Impact + +- Affects only PreToolUse Bash classification in managed hook copies. +- Legitimate file-writing Bash commands on protected branches now block and must use `gx pivot` / an agent worktree. diff --git a/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/specs/hook-guardrails/spec.md b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/specs/hook-guardrails/spec.md new file mode 100644 index 0000000..88d6a17 --- /dev/null +++ b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/specs/hook-guardrails/spec.md @@ -0,0 +1,13 @@ +## ADDED Requirements + +### Requirement: Hook guards block protected-branch shell output writes +The managed Codex, Claude, and Agents `skill_guard.py` hooks SHALL treat shell output redirection to files as mutating behavior before applying the read-only Bash allowlist. + +#### Scenario: Allowlisted command redirects output to a file +- **WHEN** a non-agent shell on protected `main` runs a command such as `cat > target-file` +- **THEN** the guard blocks the command +- **AND** the block message identifies the protected branch. + +#### Scenario: Read-only diagnostics redirect stderr +- **WHEN** a read-only shell command uses `2>&1` or sends stderr to `/dev/null` +- **THEN** the guard continues to allow the command. diff --git a/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/tasks.md b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/tasks.md new file mode 100644 index 0000000..fb60188 --- /dev/null +++ b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/tasks.md @@ -0,0 +1,35 @@ +## Definition of Done + +This change is complete only when **all** of the following are true: + +- Every checkbox below is checked. +- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff. +- If any step blocks (test failure, conflict, ambiguous result), append a `BLOCKED:` line under section 4 explaining the blocker and **STOP**. Do not tick remaining cleanup boxes; do not silently skip the cleanup pipeline. + +## Handoff + +- Handoff: change=`agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01`; branch=`agent/codex/block-shell-output-redirect-hook-bypass-2026-04-28-11-01`; scope=`TODO`; action=`continue this sandbox or finish cleanup after a usage-limit/manual takeover`. +- Copy prompt: Continue `agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01` on branch `agent/codex/block-shell-output-redirect-hook-bypass-2026-04-28-11-01`. Work inside the existing sandbox, review `openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/tasks.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent/codex/block-shell-output-redirect-hook-bypass-2026-04-28-11-01 --base main --via-pr --wait-for-merge --cleanup`. + +## 1. Specification + +- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01`. +- [x] 1.2 Define normative requirements in `specs/hook-guardrails/spec.md`. + +## 2. Implementation + +- [x] 2.1 Implement scoped behavior changes. +- [x] 2.2 Add/update focused regression coverage. + +## 3. Verification + +- [x] 3.1 Run targeted project verification commands. Evidence: `python3 -m py_compile .codex/hooks/skill_guard.py .claude/hooks/skill_guard.py`; `node --test --test-name-pattern "repo hook settings reference real local hook directories|repo skill guard blocks shell output redirect bypasses" test/setup.test.js` -> 2 passed. +- [x] 3.2 Run `openspec validate agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01 --type change --strict`. Evidence: valid. +- [x] 3.3 Run `openspec validate --specs`. Evidence: no spec items found. + +## 4. Cleanup (mandatory; run before claiming completion) + +- [ ] 4.1 Run the cleanup pipeline: `gx branch finish --branch agent/codex/block-shell-output-redirect-hook-bypass-2026-04-28-11-01 --base main --via-pr --wait-for-merge --cleanup`. This handles commit -> push -> PR create -> merge wait -> worktree prune in one invocation. + - BLOCKED: `gx branch finish --branch agent/codex/block-shell-output-redirect-hook-bypass-2026-04-28-11-01 --base main --via-pr --wait-for-merge --cleanup` needs network approval, but escalation was rejected because the account hit the usage limit until 3:40 PM. Next step: rerun the same finish command after approval quota resets. +- [ ] 4.2 Record the PR URL and final merge state (`MERGED`) in the completion handoff. +- [ ] 4.3 Confirm the sandbox worktree is gone (`git worktree list` no longer shows the agent path; `git branch -a` shows no surviving local/remote refs for the branch). diff --git a/test/setup.test.js b/test/setup.test.js index 92c5d84..0e6d278 100644 --- a/test/setup.test.js +++ b/test/setup.test.js @@ -445,6 +445,40 @@ test('repo hook settings reference real local hook directories', () => { }); +test('repo skill guard blocks shell output redirect bypasses', () => { + const repoRoot = path.resolve(__dirname, '..'); + const script = String.raw` +import importlib.util +import pathlib +import sys + +repo = pathlib.Path(sys.argv[1]) +hook_paths = [ + ".codex/hooks/skill_guard.py", + ".claude/hooks/skill_guard.py", + ".agents/hooks/skill_guard.py", +] +mutating_command = "cat > apps/frontend/src/features/scraper/components/scraping-agents-page.tsx <<'EOF'\nexport {}\nEOF" + +for index, hook_path in enumerate(hook_paths): + spec = importlib.util.spec_from_file_location(f"skill_guard_{index}", repo / hook_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + assert module.is_allowed_non_agent_shell_command("git status 2>&1 | cat") is True + assert module.is_allowed_non_agent_shell_command("git rev-parse HEAD 2>/dev/null") is True + assert module.is_allowed_non_agent_shell_command(mutating_command) is False + module.current_branch = lambda _repo_root: "main" + module.resolve_protected_branches = lambda _repo_root: {"main", "dev"} + error = module.ensure_non_agent_shell_command_allowed(repo, mutating_command) + assert error is not None + assert "protected branch 'main'" in error +`; + + const result = runCmd('python3', ['-c', script, repoRoot], repoRoot); + assert.equal(result.status, 0, result.stderr || result.stdout); +}); + + test('setup and doctor preserve existing agent scripts in package.json by default', () => { const repoDir = initRepo(); const packagePath = path.join(repoDir, 'package.json');