Repository navigation
fix(codex): decline permission escalation with a valid empty grant - #121537
Open
Wenfengcheng wants to merge 1 commit into
Open
Wenfengcheng wants to merge 1 commit into
Wenfengcheng wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #121297.
Codex app-server permission escalation requests receive
{"decision":"decline"}, butPermissionsRequestApprovalResponserequirespermissions. The server cannot deserialize the intended denial.Root cause
The permissions handler reused the command/file-change decision shape. Permission grants have a different wire contract.
Change
Return
{"permissions": {}}: grant nothing and preserve Hermes' existing unconditional denial policy. No permission escalation, approval callback, config reads, or new abstraction is added. Command and file-change approval behavior is unchanged.Add session-level regression coverage and a portable local subprocess round trip through the real JSON-RPC client. Remove the strict xfail for the already-existing POSIX CLI regression so a corrected reply does not become an XPASS failure.
Verification
Base:
3da4a42359c8b50a6ea6563a6c1f989e4dd1fbc4.decisionresponse.scripts/run_tests.sh tests/agent/transports/test_codex_app_server_session.py tests/agent/transports/test_codex_permission_wire.py tests/agent/transports/test_codex_app_server.py tests/e2e/core/providers/test_native_codex_app_server_faults.py -q: 57 passed, 8 skipped on Windows.git diff --checkpassed.hermes chat -qE2E suite is skipped on Windows; full repository suite and live Codex service were not exercised.27361d09c3c3a698b0536bc58303bcd6b4d5e278:scripts/run_tests.sh tests/agent/transports/ -q— 387 passed, 1 skipped across 20 files.Scope / overlap
#27746 is partial overlap, not this contract: its diff changes which permission escalations are accepted, but still serializes a
decisionfield for both acceptance and denial. This PR only fixes the wire representation of the existing deny policy and deliberately does not implement its permission-granting feature.#121499 changes the known-bug test gating infrastructure, not the production permission response. The removal here is limited to this fixed issue's strict xfail.
Acceptance: schema-valid denial is delivered here; no remaining item in #121297 is intentionally deferred. Excluded: permission grants/writable-root policy, single-query approval waiting (#121296), process teardown, and failed-turn output.