From 9f0501fae463b7706b7daf512825d424f0d1bc43 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 28 Apr 2026 11:10:08 +0200 Subject: [PATCH 1/2] Block shell redirect writes in hook guards The managed PreToolUse hooks already block direct Write/Edit tools on protected branches, but Bash allowlisting could classify command names like cat or printf as read-only before accounting for shell output redirection. The guard now detects file-writing redirection first and keeps stderr-only diagnostics allowed. Constraint: Protected main/dev edits must pivot into agent worktrees before file mutation Rejected: Remove cat/echo/printf from the allowlist | harmless read-only diagnostics still need those commands Confidence: high Scope-risk: narrow Directive: Check redirection before treating a Bash segment as read-only by command name Tested: python3 -m py_compile .codex/hooks/skill_guard.py .claude/hooks/skill_guard.py Tested: 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 Tested: openspec validate agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01 --type change --strict Tested: openspec validate --specs Not-tested: Full test/setup.test.js currently fails unrelated OpenSpec branch-start reuse assertion --- .claude/hooks/skill_guard.py | 34 +++++++++++++++++++ .codex/hooks/skill_guard.py | 34 +++++++++++++++++++ .../.openspec.yaml | 2 ++ .../proposal.md | 14 ++++++++ .../specs/hook-guardrails/spec.md | 13 +++++++ .../tasks.md | 34 +++++++++++++++++++ test/setup.test.js | 34 +++++++++++++++++++ 7 files changed, 165 insertions(+) create mode 100644 openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/.openspec.yaml create mode 100644 openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/proposal.md create mode 100644 openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/specs/hook-guardrails/spec.md create mode 100644 openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/tasks.md 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..375893a --- /dev/null +++ b/openspec/changes/agent-codex-block-shell-output-redirect-hook-bypass-2026-04-28-11-01/tasks.md @@ -0,0 +1,34 @@ +## 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. +- [ ] 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'); From b30164f317d12584d210de4bdd8adf3eda03dc84 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 28 Apr 2026 11:10:52 +0200 Subject: [PATCH 2/2] Record finish approval blocker The hook guard fix is implemented and locally verified, but remote finish requires network approval. The cleanup checklist now records the exact blocked finish command and retry condition so the lane can resume without re-discovering state. Constraint: Approval reviewer rejected network finish because usage limit is reached until 3:40 PM Confidence: high Scope-risk: narrow Tested: git status shows only the blocker note before commit Not-tested: Remote PR/merge/cleanup, blocked by approval quota --- .../tasks.md | 1 + 1 file changed, 1 insertion(+) 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 index 375893a..fb60188 100644 --- 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 @@ -30,5 +30,6 @@ This change is complete only when **all** of the following are true: ## 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).