Skip to content

Add opt-in protected static inference through OpenShell - #2443

Closed
freeqaz-openai wants to merge 12 commits into
impl/egress-local-images-20261014from
impl/egress-mvp-20261014
Closed

freeqaz-openai wants to merge 12 commits into
impl/egress-local-images-20261014from
impl/egress-mvp-20261014

Conversation

@freeqaz-openai

@freeqaz-openai freeqaz-openai commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Superseded by native Egress Proxy stack #2488. Do not land this PR. The RFC is the base (#2484); replacement draft #2485 records this layer’s disposition. Closing without merging; branch, commits, review and evidence stay available.

DO NOT MERGE YET — draft implementation; interface names and integration may change. Human merge required.

This draft adds an opt-in path for protected static inference through OpenShell. For an admitted request, OCC checks current authorization, resolves the current Kubernetes Secret version, and protects the provider response before the supervisor releases it to the Agent. It is the first connected slice of RFC 0020 / #1530, not the complete Egress Proxy MVP. It requires a matching OpenShell gateway and supervisor candidate; the upstream counterpart is not yet published.

Area Change or remaining work
Changes Admit one static OpenAI source for a dedicated Codex execution and its configured model. Check current authority before material access and again before the sole send; fence rejected material versions and inspect complete supported responses before release. The supervisor is outside the Agent; its registration is gateway-validated, not binary attestation.
Startup Record and recheck installation evidence before authenticated enrollment releases the first model probe. Protected web search and request/stream retries are disabled; local tools and ordinary startup components remain. The default Kubernetes composition does not schedule automatic re-enrollment after a Harness-only restart; deploy a new revision for supported recovery.
Remaining Qualify a fresh revision with matching runtimes, including response custody and workload no-bypass networking. Mixed-version hot upgrades are unsupported. GitHub dynamic credential lifecycle, full old Driver retirement, additional bindings and general rollout are follow-ups.
Evidence Result
Source and fixture checks Scoped correction runs passed 245 startup/transport cases with local gRPC proxy exclusions, 73 selected Compute cases and three focused authority/response cases; an earlier full static-egress fixture suite passed 12. These checks do not establish installed enforcement.
Built artifacts The corrected OpenShell release binaries and images built successfully. Matching final OCE controller/runtime images remain pending; builds alone do not qualify this head.
Installed end-to-end Not run. The connected scenario is authored and independently reviewed; the authenticated local deployment profile is being implemented separately.
Real provider Not run; follows the installed enforcement test.

Stack base: draft #2426; prerequisite: RFC #1530; selected main: e46d3da. No merge is requested.

The repository autoreview helper failed before producing a verdict. Independent source reviews found and corrected five runtime issues. The final review found no additional P0/P1/P2 issues; its one API-description correction is included. Installed enforcement remains unqualified.

@clawsweeper

clawsweeper Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Oct 14, 2026
@clawsweeper

clawsweeper Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

What this changes

This PR adds optional protected static inference with current authorization, versioned Secret reads, startup enrollment, and response filtering through OpenShell.

Example: An operator replaces an OpenAI source's Secret value from version 1 to version 2.

  • Before: The legacy gateway retains its approved credential copy until an explicit source update.
  • After: A newly admitted protected source reads version 2 for its next authorized inference operation without exposing the key to the Agent.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The source and fixture coverage are substantial, but installed authority enforcement and existing-state upgrade compatibility remain unqualified.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Product

Kind: Feature · Worth it: Yes · Fix scope: Partial
User problem: Agents need inference access without receiving provider keys, while operators need current authorization and controlled credential replacement.
Reason: The recorded owner direction supports static live adoption, and the PR explicitly limits itself to one connected slice of the broader RFC.

Merge readiness

⛔ Blocked before merge - 4 items remain

Keep this PR open as useful, explicitly paused implementation work; current main does not provide its request-bound protected inference path, and no discrete introduced bug was established.

Priority: P2
Reviewed head: e3b251c69a92269139660fe53313adfd8dff7e29

Before merge

  • Add real behavior proof - Authority-chain proof required: exercise the actual generated Harness and matching OpenShell supervisor through the shipped deployment path, recording an allowed provider send and rejection before provider I/O for another Sandbox and a principal revoked after Resolve; also prove instance replacement invalidates outstanding authority and unsafe responses never reach the Agent. The complete PR body explicitly says installed enforcement and real-provider execution have not run; supplied fixtures and wire vectors do not establish those outcomes. Provide redacted terminal output or logs tied to the tested revision, excluding IPs, keys, and non-public endpoints. Migration 0054 also lacks an executed upgrade demonstrating that existing legacy source copies and revisions remain intact. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
  • Resolve security review attention item - No concrete bypass was established, but the new supervisor credential-custody boundary requires qualification against its matching supplier.
  • Resolve merge risk - Publish and pin the matching OpenShell gateway/supervisor source before landing this consumer so its registration, receipt, and credential-access contract can be reviewed together.
  • Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.

Findings

None.

Tests

  • Missing end-to-end proof: Supply a regular Agent deployment with matching runtimes proving allowed and forbidden final provider sends, revocation and instance replacement, enrollment and response custody, plus an executed existing-database upgrade preserving legacy sources.
Agent review details

How this fits together

OCC admits credential sources and revision plans; Compute and Sandbox install their enforcement configuration, while an authenticated supervisor requests credential material and send permission before returning a filtered provider response.

flowchart TB
  A[Agent request] --> S[OpenShell supervisor]
  S --> E[OCC egress authority]
  E --> I[Current IAM and revision state]
  E --> K[Kubernetes Secret]
  S -. required counterpart send .-> P[Inference provider]
  P -. response custody .-> S
  S --> E
Loading

Technical review

Best possible solution:

Complete the bounded opt-in slice on qualified fresh revisions while preserving legacy source approval semantics and keeping broader RFC work separate.

Do we have a high-confidence way to reproduce the issue?

This PR proposes a new capability rather than a reproducible bug; source inspection establishes its admission and authority paths without executing target code.

Is this the best way to solve the issue?

The implementation extends existing Driver composition and Agent lifecycle owners, uses independent IAM checks, and preserves explicit opt-in for legacy sources.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 3a65e931a305.

Merge-risk options

Maintainer options:

  1. Keep the consumer draft until the counterpart is reviewable (recommended)
    Publish an immutable OpenShell candidate revision and review its producer and sender contract alongside this consumer before landing.

Provenance checked

  • Credential source copies and live adoption changes intended behavior with a stated reason (d4f544c: The original Credential Gateway kept real keys outside Harness workloads through gateway-owned copies and placeholder substitution.)
  • Existing source update and withdrawal behavior keeps the original intent (c91b9c6: Source updates and withdrawal extend the gateway-copy lifecycle; this PR preserves that path for legacy sources and separately handles protected bindings.)
  • Dedicated Codex endpoint routing keeps the original intent (669cf7a: Support OpenAI-compatible Codex model endpoints #913 carries normalized OpenAI-compatible endpoints through native configuration and startup probing.)
  • Protected startup retry behavior changes intended behavior with a stated reason (a97cb35: Earlier startup behavior retried bounded model-probe timeouts; the protected path explicitly disables retries because a timeout can follow provider effects.)

Testing

Proof path: in-process harness.

Security

Needs attention: No concrete bypass was established, but the new supervisor credential-custody boundary requires qualification against its matching supplier.

Evidence

What I checked:

  • Pinned scope and clean checkout: Reviewed the introduced merge-base-to-head changes at e3b251c; final git status remained clean. No target code, tests, review helper, or second reviewer was run. (e3b251c69a92)
  • Current-main boundary: The authoritative implementation status still describes general credential-free model inference as planned; the existing Credential Gateway supplies approved copies rather than this per-operation authority path. (docs/design.md:17, 3a65e931a305)
  • Explicit draft hold: The complete current body says not to merge, identifies the unpublished OpenShell counterpart, and explicitly reports that installed end-to-end and real-provider verification have not run. (e3b251c69a92)
  • Recorded adoption direction: Free's decision in docs(rfc): propose egress proxy and credential lifecycle #1530 approves live adoption for new static sources while preserving existing sources until authorized opt-in; this PR retains that distinction. (packages/occ/src/index.ts:5089, e3b251c69a92)
  • Authority checks and remaining dependency: Send authorization consumes the handle before a fresh policy check and bounds send initiation; enforcement verification relies on the supplier revalidating registered instance and effective configuration. (packages/occ/src/egress.ts:342, e3b251c69a92)
  • Fixture proof boundary: The tests use production authority and transport components, but gateway enforcement observations are fixtures, the sender is test-written, and startup substitutes the native CLI; supplied wire vectors are explicitly synthetic component evidence. (docs/testing/static-egress.md:73, e3b251c69a92)

Labels

Label changes:

  • add P2: This is a substantial optional platform capability without evidence of an urgent current-user regression.
  • add merge-risk: 🚨 compatibility: The change introduces persisted source and execution formats plus optional Driver contracts; existing-state migration evidence remains insufficient.
  • add merge-risk: 🚨 security-boundary: Credential custody and final provider I/O cross the Agent/supervisor boundary, and the matching supplier implementation remains unpublished and unqualified.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

  • P2: This is a substantial optional platform capability without evidence of an urgent current-user regression.
  • merge-risk: 🚨 security-boundary: Credential custody and final provider I/O cross the Agent/supervisor boundary, and the matching supplier implementation remains unpublished and unqualified.
  • merge-risk: 🚨 compatibility: The change introduces persisted source and execution formats plus optional Driver contracts; existing-state migration evidence remains insufficient.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Prove allowed and nearest-forbidden provider I/O through the actual supervisor, including revocation after resolution and replacement of the registered instance.
  • Demonstrate migration from a pre-change database with legacy credential sources and revisions preserved.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 14, 2026, 12:38 PM ET / 16:38 UTC.

This branch had an error being deployed

1 failed deployment
integration-qa-pr — e3b251c6 Deployed Oct 14, 2026 by freeqaz-openai via QA advisory (compose) #905
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant