osep: human-in-the-loop approval gates - #1327
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d87ad47cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Who is this feature actually for? The "reviewer" role is hollowed out. The proposal repeatedly invokes "authorized reviewer," "human-in-the-loop," and "supervised boundary," but never answers the most basic product question: who is the reviewer, where do they live, and how do they find out something needs a decision? Concretely:
The underlying contradiction: human-in-the-loop derives its value from the human being distinct from the automated caller — distinct identity, distinct judgment, distinct authority. This design collapses all three. The reviewer is indistinguishable, in the permission model, from any other authenticated caller of the sandbox. What's actually being shipped is a remotely unblockable runtime pause — a legitimate primitive, but a fundamentally different product from human review. Naming it "Human-in-the-Loop Approval Gates" leads users to build safety expectations the mechanism cannot support:
Request: either
The current framing sits in the gap between the two and inherits the weaknesses of both. |
|
@Pangjiping Thanks for calling out the gap between a remotely unblockable pause and a real human review boundary. I took option 1 and revised OSEP-0015 in commit The first phase now includes all three missing reviewer properties:
The proposal keeps the held action in execd/egress, but moves reviewer authentication, grant signing, notification, UI, and decision proxying into the lifecycle server. It also defines first-decision-wins behavior for multiple reviewers, grant leakage mitigations, webhook failure as fail-closed, and tests for agent self-approval attempts. I also addressed the two inline findings: separate reviewer authorization and explicit rejection of egress |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 352f2a2267
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| 1. operator `deny.always`: deny immediately; | ||
| 2. operator `allow.always`: allow immediately; | ||
| 3. first matching user rule: apply its allow or deny immediately; |
There was a problem hiding this comment.
Gate runtime egress policy mutations under approvals
With new_domains enabled, this precedence lets a normal user rule bypass review before the approval gate runs. I checked the current egress API and SDK: /policy is explicitly a runtime mutation endpoint reached with sandbox endpoint headers (specs/egress-api.yaml Access Model), patch_egress_rules calls it directly, and the sidecar authorizes it with OPENSANDBOX-EGRESS-AUTH in components/egress/policy_server.go. An autonomous agent that holds the normal egress credentials can patch {action: allow, target: <fqdn>} and retry, so no reviewer ever sees that new domain; the OSEP should freeze, deny, or reviewer-gate policy updates that add or raise allows while approval policy is active.
Useful? React with 👍 / 👎.
| metadata pattern as other internal endpoint tokens and injects it only into | ||
| execd and egress. It also injects an internal server callback URL that works for |
There was a problem hiding this comment.
Keep broker tokens out of execd child environments
This still leaves a self-approval path if the broker token is delivered to execd using the usual startup environment/config pattern. Fresh evidence: current execd command paths pass os.Environ() into user processes (components/execd/pkg/runtime/command.go:139 and :301, with bash/PTY paths doing the same), so any ungated command can print the broker token and then call broker-only approval routes. Require a non-inherited delivery mechanism or mandatory scrubbing for every execd child/session environment before treating the broker token as separate from the workload.
Useful? React with 👍 / 👎.
Summary
Relates to #1288. This PR covers the required design phase; runtime implementation remains gated on the OSEP reaching
implementablestatus.Testing
Additional validation:
git diff --cached --checkBreaking Changes
Checklist