Skip to content

osep: human-in-the-loop approval gates - #1327

Open
GodBlf wants to merge 2 commits into
opensandbox-group:mainfrom
GodBlf:feat/1288-approval-gate
Open

osep: human-in-the-loop approval gates#1327
GodBlf wants to merge 2 commits into
opensandbox-group:mainfrom
GodBlf:feat/1288-approval-gate

Conversation

@GodBlf

@GodBlf GodBlf commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • Add draft OSEP-0015 for optional, fail-closed human approval gates covering selected execd commands and new-domain egress in the first phase.
  • Define data-plane-local approval ownership, lifecycle policy, runtime APIs, state transitions, SDK/CLI surfaces, security constraints, compatibility, rollout, and test requirements.
  • Add OSEP-0015 to the proposal index.

Relates to #1288. This PR covers the required design phase; runtime implementation remains gated on the OSEP reaching implementable status.

Testing

  • Not run (documentation-only OSEP; no executable code changed)
  • Unit tests
  • Integration tests
  • e2e / manual verification

Additional validation:

  • git diff --cached --check
  • Parsed and validated the OSEP YAML frontmatter
  • Verified the index entry, issue link, template placeholders, and ASCII content

Breaking Changes

  • None
  • Yes (describe impact and migration path)

Checklist

  • Linked Issue or clearly described motivation
  • Added/updated docs (if needed)
  • Added/updated tests (if needed)
  • Security impact considered
  • Backward compatibility considered

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jul 17, 2026
@GodBlf
GodBlf marked this pull request as ready for review July 17, 2026 07:11
Copilot AI review requested due to automatic review settings July 17, 2026 07:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread oseps/0015-human-in-the-loop-approval-gates.md Outdated
Comment thread oseps/0015-human-in-the-loop-approval-gates.md Outdated
@Pangjiping

Copy link
Copy Markdown
Collaborator

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:

  • No reviewer identity. §Runtime Approval APIs explicitly gives up on this: "They do not identify the reviewer in the first phase because runtime endpoint authentication does not currently provide a stable reviewer identity." A human-in-the-loop system whose records can't answer who approved this fails the first principle of the pattern.

  • No reviewer entry point. Non-Goal 5 defers browser review pages, scoped share links, push notifications, and a cross-sandbox inbox. What's left is SDK calls and osb CLI — i.e., a human must be actively polling a terminal to discover pending items. There is no channel that reaches the reviewer; the reviewer has to reach the system.

  • No separation between reviewer and callee. §Notes 6: "approval access is equivalent to existing authenticated access to the corresponding sandbox runtime endpoint." Anyone who can drive the sandbox can also approve its actions. In practice, the reviewer credential and the agent credential are the same credential.

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:

  • They will assume decisions are attributable → they aren't.
  • They will assume reviewers are notified → they must poll.
  • They will assume reviewer authority is scoped → it's the same scope as the agent's.

Request: either

  1. Add a real reviewer model to phase one (identity, entry point, scoped credential — even a minimal one), or
  2. Rename and reframe the feature to what it actually is — a runtime pause/resume primitive that can be used to build human review on top, but isn't itself human review.

The current framing sits in the gap between the two and inherits the weaknesses of both.

@GodBlf

GodBlf commented Jul 21, 2026

Copy link
Copy Markdown
Author

@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 352f2a22.

The first phase now includes all three missing reviewer properties:

  • Identity: each reviewer is one accountable human principal pre-registered by the operator. Every terminal reviewer decision records the verified decidedBy identity.
  • Entry point and notification: a broker-authenticated pending event causes the lifecycle server to send the reviewer an authenticated webhook containing a short-lived review link. The server hosts a minimal review page, with grant-based SDK/CLI alternatives.
  • Authority separation: normal execd/egress, sandbox, lifecycle, and agent credentials cannot read or decide approvals. The public reviewer API accepts only a server-signed grant scoped to one reviewer, sandbox, source, approval, permission set, and expiry. The server reaches component-local state with a separate non-empty per-sandbox broker token that is never returned to the agent.

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 ttlSeconds: 0. The revised proposal is ready for another look.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Comment on lines +443 to +444
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation OSEP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants