Repository navigation
Codex app-server: launch-input layer (MCP isolation, posture, hook-trust) - #578
Merged
Merged
Conversation
Renders the resolved (sandbox, approval) pair into codex app-server protocol JSON: sandboxPolicy objects (readOnly / workspaceWrite+writableRoots / dangerFullAccess), an approvalPolicy string validated against the three known kebab-case tokens, and the pinned approvalsReviewer="user". String-only approval policies (the granular object form crashes the app-server on turn/start) and the approvals-to-client pin (Guardian can never auto-approve a sandbox escalation) are enforced here, mirroring how CodexLauncher renders the same pair into PTY argv. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR Summary by QodoAdd Codex app-server launch-input safety layer
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Adds the launch-input layer the app-server runtime consumes, reusing the PTY
path's proven logic:
- BuildAppServerLaunchArgs assembles the `codex app-server` argv: `--disable apps`
(the codex_apps ChatGPT-connector runtime bypasses the mcp_servers disable
table) plus the shared MCP-isolation overrides, factored into AppendMcpIsolationArgs
used by both transports. On app-server it stamps default_tools_approval_mode=
"approve" on each whitelisted server (the first tool call otherwise wedges an
unattended turn even under approvalPolicy: never; trust_level does not suppress
it). Sandbox/approval/model/effort are per-turn protocol params, not argv.
- CodexHookTrust classifies a hooks/list snapshot: absence of a required critical
event fails closed, an untrusted kcap-owned hook is repaired by one seeded
restart via a full-table `-c hooks.state={…}` override (the dotted-key form
silently fails on path-shaped keys), a hook with no currentHash is unseedable.
- CodexToml (TOML basic-string encoder) and CodexHooksParser.IsCapacitorCodexHookCommand
are extracted so the two transports and the hooks.json/hooks-list surfaces share
one implementation.
The PTY BuildArgs path is byte-identical (appServer:false) — the existing
CodexLauncherTests isolation suite stays green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
force-pushed
the
tonyyoung/ai-1761-codex-appserver
branch
from
August 18, 2026 11:47
e59160a to
cde4f56
Compare
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo review: - Hook ownership marker (Security): IsCapacitorCodexHookCommand matched a kcap marker as a substring, so a project hook like `evil # kcap codex-hook` — overlaid from an untrusted branch into a reviewer worktree — would be treated as kcap-owned and trust-seeded by the app-server classifier, then run (hooks fire outside the turn sandbox). Now matches the command's real executable (basename, so a path-qualified /usr/local/bin/kcap still matches) + exact dispatcher arguments; anything else fails closed (not kcap-owned → never seeded). Behavior-preserving for every installed form (kcap hook --codex / kcap codex-hook / kapacitor codex-hook); the hooks.json preflight suite stays green. Added spoof/path tests. - Trimmed test comments that merely restated assertions (house rule), keeping only the non-obvious rationale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
First PR of the daemon-side Codex
app-servermigration (Closes #577; part 1 of 2 — the runtime that consumes these is the follow-up). Adds the pure, fully unit-tested launch-input pieces theCodexAppServerHostedAgentRuntimewill use, reusing the PTY path's proven logic so nothing drifts between the two transports.What's here
CodexAppServerPosture— renders the resolved(sandbox, approval)pair intocodex app-serverprotocol JSON:sandboxPolicyobjects (readOnly/workspaceWrite+writableRoots/dangerFullAccess), anapprovalPolicystring validated against the three known kebab-case tokens, and the pinnedapprovalsReviewer="user". String-only policies (the granular object form crashes the app-server onturn/start) and the approvals-to-client pin (Guardian can never auto-approve a sandbox escalation) are enforced here.CodexHookTrust— classifies ahooks/listsnapshot: absence of a required critical event (SessionStart/Stop/PermissionRequest) fails closed (seeding can't conjure a hook), an untrusted kcap-owned hook is repaired by one seeded restart via a full-table-c hooks.state={…}override (the dotted-key form silently fails on path-shaped keys), a hook with nocurrentHashis unseedable. Seeds every untrusted kcap hook so the transcript hooks fire too.CodexLauncher.BuildAppServerLaunchArgs— thecodex app-serverargv:--disable apps(thecodex_appsChatGPT-connector runtime is not anmcp_serversentry and bypasses the disable table) plus the shared MCP-isolation overrides. The isolation is factored intoAppendMcpIsolationArgsused by both transports; the app-server variant additionally stampsdefault_tools_approval_mode="approve"on each whitelisted server (the first tool call otherwise raises an approval that wedges an unattended turn even underapprovalPolicy: never;trust_leveldoes not suppress it). Sandbox/approval/model/effort are per-turn protocol params on this transport, so none of the PTY-only flags appear.CodexTomlandCodexHooksParser.IsCapacitorCodexHookCommandextracted so the encoder and the kcap-hook marker have one shared implementation.Safety
The PTY
BuildArgspath is byte-identical (appServer: false) — the existingCodexLauncherTestsisolation/argv suite stays green. 33 new unit tests cover the three additions.Grounded by a completed protocol spike on pinned codex 0.146.0 (the four launcher requirements above were all surfaced/verified there).
Linear: AI-1761 (epic AI-1759).