Skip to content

fix: fail closed on Codex hook errors#110

Open
majiayu000 wants to merge 2 commits intomainfrom
feat/codex-hook-fail-closed
Open

fix: fail closed on Codex hook errors#110
majiayu000 wants to merge 2 commits intomainfrom
feat/codex-hook-fail-closed

Conversation

@majiayu000
Copy link
Copy Markdown
Owner

What

  • treat nonzero pre-bash-guard.sh exits without a structured decision as hook errors in the Codex app-server wrapper
  • decline approval requests on those hook errors instead of falling back to pass
  • add a regression test that reproduces a failing hook process and verifies the wrapper intercepts and declines the approval

Why

The wrapper previously ignored hook exit status and defaulted missing decisions to pass, which let approval-gate enforcement fail open in Codex app-server mode when the hook process errored.

How to test

  • bash tests/test_codex_runtime.sh
  • python3 -m py_compile scripts/codex/app_server_wrapper.py

Checklist

  • Scope-limited fix
  • Regression coverage added
  • Local verification run
  • Human review of security-sensitive approval flow

Decline approval requests when the pre-bash hook exits nonzero without
emitting a structured decision so Codex app-server mode cannot bypass
guard enforcement on hook execution failures. Add a regression test that
reproduces the failing hook path and proves the wrapper fails closed.

Constraint: Approval hooks must fail closed when runtime wrapper execution breaks
Rejected: Keep default pass on missing decisions | nonzero hook exits silently disabled enforcement
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Runtime adapters should treat hook transport failures as declined approvals unless a structured decision overrides them
Tested: bash tests/test_codex_runtime.sh; python3 -m py_compile scripts/codex/app_server_wrapper.py
Not-tested: End-to-end codex app-server session against a live Codex backend
Signed-off-by: majiayu000 <1835304752@qq.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Hook Latency (P95)

Details
Benchmark suite Current: 29ca7a4 Previous: 0bb421d Ratio
pre-edit-guard (P95) 185 ms 195 ms 0.95
pre-write-guard (P95) 218 ms 219 ms 1.00
pre-bash-guard (P95) 250 ms 274 ms 0.91
post-edit-guard (100) (P95) 294 ms 299 ms 0.98
post-write-guard (100) (P95) 207 ms 208 ms 1.00
post-edit-guard (5000) (P95) 308 ms 316 ms 0.97
post-write-guard (5000) (P95) 207 ms 217 ms 0.95
stop-guard (5000) (P95) 129 ms 130 ms 0.99
learn-evaluator (5000) (P95) 131 ms 136 ms 0.96

This comment was automatically generated by workflow using github-action-benchmark.

Treat any non-zero pre-bash hook exit as a hook error before parsing decision text so malformed stderr or partial payloads cannot reopen command approvals.

Signed-off-by: majiayu000 <1835304752@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant