feat(core): add structured network approval plumbing and policy decision model#11672
feat(core): add structured network approval plumbing and policy decision model#11672viyatb-oai merged 49 commits intomainfrom
Conversation
…elation hardening
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
I'm pretty concerned about the number of changes going into process_manager that are not gated behind a feature flag. will cc @jif-oai for visibility but I would love if we could figure out a way to test this without refactoring our load-bearing command executable path
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7532de8bd7
ℹ️ 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".
…/pr-b-network-core-plumbing
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f4152143e
ℹ️ 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".
Align PR-B callsites and structs with the branch's current network approval plumbing surface so workspace clippy passes cleanly. - Remove partially wired C-layer runtime/orchestrator fields from B-level code paths - Fill required protocol/error fields with compatibility defaults - Update remaining start_proxy callsites to current signature Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Update ExecApprovalRequestEvent test fixtures to include network_approval_context now that the protocol field is required on this branch. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
…nd retry flow (#11673) ### Description #### Summary Integrates structured network approvals into the core orchestration path. #### What changed - Wired structured network approval handling into tool orchestration/retry paths. - Integrated approval outcomes into command execution lifecycle. - Added orchestration-level handling for allow-once/session approval behavior. - Connected core session/delegate surfaces required for end-to-end approval flow. - Updated related core test coverage for orchestration behavior. #### Why With plumbing in place from parent, this PR makes network approvals operational in core command execution flow. #### Notes - Gated behavior remains aligned with managed network requirements and sandbox policy. --------- Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
- only construct inline network policy decider when network proxy config is enabled - map proxy protocol tag "http-connect" to HTTPS approval context Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Reuse decision/source constants from codex-network-proxy in network policy parsing to avoid duplicated string literals and drift. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Restore the single combined guard clause in is_likely_sandbox_denied for minimal diff churn with no behavior change. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Move shell runtime network-approval bookkeeping into a dedicated helper and add targeted comments for inline network decider wiring and attempt-scoped proxy env handling. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Extract policy decider builder setup into a helper and remove temporary debug logging in orchestrator to reduce noise and keep flow readable. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
…ry behavior Preserve the original orchestrator flow/comments and restore required network retry approval semantics (network context extraction, OnRequest network prompt path, and retry approval bypass guard). Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Restore process_manager control-flow closer to original shape while preserving required network approval behavior (attempt tracking, policy-denial handling, and delayed denial termination). Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Reduce shape churn in refresh_process_state while preserving async-safe network attempt cleanup for exited processes. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Keep refresh_process_state close to original structure while preserving exited-process network attempt cleanup. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Move attempt registration/cleanup and approval outcome handling into a shared tools/network_approval module, then wire shell and unified-exec runtimes through ToolOrchestrator-managed context. This removes duplicate network-attempt plumbing from process_manager and shell runtime, while preserving deferred handling for long-lived unified-exec sessions. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Drop low-value tests that only mirror the mode match logic in should_ask_on_allowlist_miss. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Parse network policy payload decision/source into typed proxy enums with custom deserializers, and remove string comparisons from approval gating logic. Also switch internal exec attempt IDs to UUID in ExecParams, converting to string only at proxy env boundaries. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
…nfig When unified-exec sees delayed user denial, terminate the process and explicitly release process state + unregister the network attempt in the watcher path. Also inline policy-decider builder configuration in NetworkProxySpec::start_proxy and remove the separate builder-returning helper. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Inline the restricted-sandbox match at callsite and remove a single-use helper to keep start_proxy flow linear. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Use Self::stdout_stream(ctx) instead of ShellRuntime::stdout_stream(ctx) for consistency with impl-local helper calls. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Move network approval attempt/session state into a dedicated service, wire structured blocked-request callbacks from the managed proxy, and route unified-exec cleanup through service APIs so approval flow no longer depends on proxy telemetry lookups. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Rename and centralize managed network decider/observer construction in network_approval to keep codex session startup wiring minimal, and remove the extra unified-exec delayed-denial debug log. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
…val types Regenerate app-server protocol JSON/TypeScript schema fixtures so they include the network approval context/protocol types referenced by approval events. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
This is a lot cleaner - some additional comments to address
| if let Some(pruned_entry) = pruned_entry { | ||
| Self::unregister_network_attempt_for_entry(&pruned_entry).await; | ||
| pruned_entry.process.terminate(); | ||
| } |
There was a problem hiding this comment.
Is there some lifecycle-related reason that we need to move the terminate call out of prune_processes_if_needed? Could we just call unregister_network_attempt_for_entry at the existing terminate callsite?
- add Config::managed_network_requirements_enabled() and use it in core, app-server, and debug_sandbox - extract managed proxy startup flow in Session helper - move unified-exec delayed network denial watcher into async_watcher and gate it by managed requirements - simplify network policy protocol parsing by using serde enum aliases instead of a custom deserializer Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
so close! Also, codex had this to say:
The new denial watcher drops non-user network outcomes, which can suppress deferred policy-denial errors for long-lived unified-exec processes. That behavior
is a functional regression in network-approval handling.
Review comment:
- [P2] Preserve non-user network outcomes in denial watcher — /Users/dylan.hurd/code/codex-3/codex-rs/core/src/unified_exec/async_watcher.rs:163-166
The watcher consumes approval outcomes with take_outcome() on every poll, but only acts when the value is DeniedByUser. If a long-lived unified-exec
process records DeniedByPolicy, this code silently drops that outcome and continues, so later calls cannot surface the policy rejection message (the
outcome was already removed). This is reproducible when a running process hits a deny-only host after startup; the proxy blocks it, but the deferred
rejection signal is lost
- avoid consuming non-user network outcomes in the unified-exec denial watcher - add take_user_denial_outcome so policy denials remain available for deferred reporting - gate managed proxy startup outside the awaited helper call in Session setup Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Use a chained if-let to make the early-return guard in unregister_network_attempt_for_entry more compact, matching reviewer suggestion. Co-Authored-By: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Description
Summary
Introduces the core plumbing required for structured network approvals
What changed
Why
establishes the minimal backend foundation for network approvals without yet changing high-level orchestration or TUI behavior.
Notes