Skip to content

Restore pinned Codex deny-floor adapter - #1538

Open
Chris0Jeky wants to merge 6 commits into
mainfrom
issue-1456/codex-deny-floor-adapter-v2
Open

Restore pinned Codex deny-floor adapter#1538
Chris0Jeky wants to merge 6 commits into
mainfrom
issue-1456/codex-deny-floor-adapter-v2

Conversation

@Chris0Jeky

@Chris0Jeky Chris0Jeky commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a fail-closed Codex PreToolUse bridge that adapts Codex Bash payloads to Taskdeck's canonical shared deny-floor dispatcher without weakening the producer contract.
  • Pin the currently deployed producer Chris0Jeky/agent-harness@c056a77d22ecdcdfb2389c4c83487eb54061f5aa, floor 1.6.19 (2026-07-27), normalized dispatcher SHA-256 524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b.
  • Add Windows/POSIX launchers, hostile-environment and deployment-race tests, docs-governance coverage, and the operator ritual for exact-producer and fresh-runtime proof.
  • Replace the stale unsigned implementation in Add pinned Codex deny-floor adapter #1457 with a signed branch cut from current origin/main.

Closes #1456
Supersedes #1457

Verification

  • py -3 -I -B -m unittest discover -s scripts/agent_hooks -p "test_*.py" — 24/24 passed, zero skips.
  • Current producer smoke from a clean standalone agent-harness@c056a77d... checkout — 2171/2171 passed, exit 0.
  • Taskdeck hook smoke, docs governance, golden-principles, and GitHub project-operations checks passed.
  • JSON/TOML parsing, PowerShell parsing, POSIX bash -n, git diff --check, and DCO across all six commits passed.
  • Backend tests not applicable: no backend product code or project files changed.
  • Frontend checks not applicable: no frontend code changed.
  • Playwright evidence not applicable: no UI behavior changed.
  • Hosted Required CI / CI Extended / CodeQL / DCO checks at exact head 6a09f435ace8b44ec6b18ca89e9c66a1b8ac72b6 (pending GitHub execution).

Preserved red evidence

The first reviewed-producer smoke run against floor 1.6.18 exited 1 after about 304 seconds at smoke_test.py:5774: the long-lived Codex process had inherited the retired GIT_CONFIG_GLOBAL setting. Running the identical producer bytes with only that child environment variable removed passed 2166/2166. The fresh-session Taskdeck configuration removes that variable; the documented gate fails if it remains inherited. After the producer advanced to current floor 1.6.19, an isolated clean run passed 2171/2171.

The producer audit's general-hygiene command exits 1 for three unrelated pre-existing findings (the global CLAUDE.md and AGENTS.md line budgets, plus the existing Taskdeck profile path). Its deny-floor proof fields are current: zero unproven roots and declared-versus-real identity matched.

Review

One independent adversarial round found six HIGH defects: whitespace-only stderr acceptance, Python import-path influence, a dispatcher replacement race, malformed event/cwd acceptance, dirty producer execution risk, and a Windows installed-pin skip. All six were fixed. A final exact-head review at 6a09f435... reported 0 CRITICAL, 0 HIGH, 0 MEDIUM, and 0 LOW findings.

Documentation

  • Updated docs/STATUS.md, docs/IMPLEMENTATION_MASTERPLAN.md, and docs/TESTING_GUIDE.md with the current producer identity and fail-fast runtime ritual.
  • Updated .codex/README.md for the reviewed adapter contract.
  • Left the maintainer-owned OUTSTANDING_TASKS.md unchanged.

Tracking / merge gates

  • Issue linked with an active closing reference.
  • Project item verified at status Review with derived priority Priority I.
  • Fresh exact-root Codex runtime proof: start a fresh Codex session in the Taskdeck root, inspect /hooks, run the allowed control git status --short, then run a non-writing force-push dry-run canary and confirm the denial is attributed to Taskdeck Codex deny-floor adapter.

CI Workflow Validation

This PR changes .github/workflows/reusable-docs-governance.yml, so CI Extended/actionlint is a mandatory hosted gate. It must be green at this exact head before merge.

Risk Notes

Static checks, Claude hook evidence, worktree-local execution, and old runtime sessions do not satisfy the final Codex hook activation gate. The PR must remain unmerged until hosted checks, all comments/threads, project fields, and the fresh exact-CWD human runtime canary are complete.

Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

Please review exact head 6a09f435ace8b44ec6b18ca89e9c66a1b8ac72b6 against main, focusing on fail-closed identity verification, hostile-environment isolation, deployment-race handling, and whether any path can accidentally allow a command.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent adversarial review — exact head 6a09f435ace8b44ec6b18ca89e9c66a1b8ac72b6 against base 2daeee9449cbdc454436df2a7acc702d3ecf5034.

Verdict:

  • CRITICAL: 0
  • HIGH: 0
  • MEDIUM: 0
  • LOW: 0

The bounded review/fix round closed all six HIGH findings found during review: whitespace-only stderr acceptance, inherited Python import-path influence, dispatcher replacement between identity check and execution, malformed event/CWD acceptance, unreviewed producer-checkout execution, and Windows installed-pin drift being silently skipped.

Exact-head evidence I ran or directly reconciled:

  • py -3 -B -m unittest discover -s scripts/agent_hooks -p "test_*.py" -v: 24/24 passed, zero skips; this includes the real Windows allow and attributed force-push-deny handler path under hostile HOME, GH_HOST, PYTHONPATH, and stripped PATH.
  • py -3 -B scripts/agent_hooks/smoke_test.py: passed.
  • Docs governance, golden-principles, and GitHub-operations governance: passed.
  • JSON/TOML parsing, PowerShell parsing, POSIX bash -n, and git diff --check: passed.
  • All six commits carry Signed-off-by trailers; reviewed worktree is clean.
  • Live producer main is c056a77d22ecdcdfb2389c4c83487eb54061f5aa; installed normalized dispatcher SHA-256 is 524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b, floor 1.6.19. Clean isolated producer smoke passed 2171/2171, exit 0.

No code, security, fail-closed, test, documentation, workflow, or merge-risk finding remains from this independent lens.

This review deliberately does not satisfy the remaining external gates: exact-head hosted CI/CI Extended/CodeQL/comment triage and the maintainer-run fresh exact-root Codex /hooks inspection plus allowed git status and Taskdeck-attributed non-writing deny canary. Do not merge until those gates are directly proven.

@Chris0Jeky Chris0Jeky moved this from Pending to Review in Taskdeck Execution Jul 27, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a09f435ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

executable,
"-I",
"-B",
str(dispatcher),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Execute the verified dispatcher bytes

During a dispatcher deployment/rollback or hostile A→B→A replacement, the pre-check can hash reviewed file A, while this subprocess reopens the pathname and executes unreviewed file B, after which the post-check sees restored file A. If B exits successfully with empty stdout, attribute_dispatcher_output() returns None and the Bash command is allowed; a deterministic swap-run-restore reproduction produced exactly that result while the adapter tests remained green. Execute an immutable snapshot or descriptor containing the bytes that were actually verified rather than re-resolving the path.

AGENTS.md reference: AGENTS.md:L112-L115

Useful? React with 👍 / 👎.

Comment thread .codex/hooks.json
Comment on lines +10 to +11
"command": "expected=524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b; dispatcher=$HOME/.claude/hooks/dispatch.py; wrapper=.codex/invoke_deny_floor.sh; /bin/sh \"$wrapper\" --dispatcher-suffix .claude/hooks/dispatch.py --event pre --runtime codex",
"commandWindows": "$expected='524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b'; $dispatcher=$env:USERPROFILE+'/.claude/hooks/dispatch.py'; $wrapper='.codex/invoke_deny_floor.ps1'; & $wrapper --dispatcher-suffix .claude/hooks/dispatch.py --event pre --runtime codex",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pin the repo-owned adapter in the trusted hook

After /hooks trusts this definition, a checkout, deployment, or permitted non-Bash file edit can replace either relative launcher or deny_floor_adapter.py without changing the trusted hook command, because the embedded expected value is unused and neither launcher verifies the repo-owned files. Replacing only the adapter with a successful empty-output program made this exact handler return exit code 0 with empty stdout for a force-push payload, which is the canonical allow result. Bind the trusted definition to reviewed launcher and adapter digests, or invoke an immutable/content-addressed adapter.

AGENTS.md reference: AGENTS.md:L112-L115

Useful? React with 👍 / 👎.

python=$(command -v python3 2>/dev/null) || deny
[ -n "$python" ] || deny

exec "$python" -I -B "$core" "$@"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Convert POSIX interpreter failures into an explicit denial

If Python disappears or becomes non-executable after command -v, or the adapter fails during startup/import in a partial deployment, this exec replaces the shell and exits with only a nonzero status and stderr, never invoking deny() or emitting deny JSON. Codex reserves exit code 2 with a nonempty reason or valid deny JSON for a blocking PreToolUse result, so this ordinary hook-process failure can let the pending command continue; an invalid adapter fixture reproduced exit code 1 with zero stdout bytes. Keep the shell alive, capture the adapter status, and convert every nonzero result to an explicit denial as the Windows launcher already does.

AGENTS.md reference: AGENTS.md:L112-L115

Useful? React with 👍 / 👎.

Comment thread .codex/hooks.json
"hooks": [
{
"type": "command",
"command": "expected=524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b; dispatcher=$HOME/.claude/hooks/dispatch.py; wrapper=.codex/invoke_deny_floor.sh; /bin/sh \"$wrapper\" --dispatcher-suffix .claude/hooks/dispatch.py --event pre --runtime codex",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Strip loader injection variables before creating hook processes

With shell_environment_policy.inherit = "all", an inherited LD_PRELOAD, LD_AUDIT, or equivalent loader variable takes effect when this /bin/sh process is created, before the launcher can execute its unset statements. A preload constructor that exited the shell successfully made this exact force-push handler return code 0 with empty stdout and stderr—the canonical allow result—without modifying the adapter or dispatcher. Exclude dynamic-loader injection variables from the environment used to create hook processes and cover the exact handler with a hostile-environment regression.

AGENTS.md reference: AGENTS.md:L112-L115

Useful? React with 👍 / 👎.

Comment thread .codex/hooks.json
{
"type": "command",
"command": "expected=524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b; dispatcher=$HOME/.claude/hooks/dispatch.py; wrapper=.codex/invoke_deny_floor.sh; /bin/sh \"$wrapper\" --dispatcher-suffix .claude/hooks/dispatch.py --event pre --runtime codex",
"commandWindows": "$expected='524fed5ae6630313f4a000a6e9a8c7deb7b1a7a6d424913d8b12c4536b79a97b'; $dispatcher=$env:USERPROFILE+'/.claude/hooks/dispatch.py'; $wrapper='.codex/invoke_deny_floor.ps1'; & $wrapper --dispatcher-suffix .claude/hooks/dispatch.py --event pre --runtime codex",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Resolve the Windows wrapper from the hook source

When Codex is started below the repository root, it still discovers this root hook but launches it with the session subdirectory as its process cwd; this was reproduced with installed codex-cli 0.144.0-alpha.4 using -C <repo>/sub. The relative $wrapper therefore resolves to <repo>/sub/.codex/invoke_deny_floor.ps1, and PowerShell's & failure occurs before the wrapper's try/catch, producing no deny JSON and an ordinary exit code 1 rather than Codex's blocking exit-2 path. Resolve the wrapper from the hook source or project root and fail closed inline if it cannot be invoked.

AGENTS.md reference: AGENTS.md:L106-L107

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

Restore Taskdeck Codex deny-floor adapter and prove live hook trust

1 participant