fix(server): gate only newly authored uninspected credentialed endpoints

[feloy]

Summary

Attaching a credentialed provider to a sandbox made every later openshell policy set and openshell policy update fail, even for completely unrelated edits, naming a rule the operator never authored. The credential gate now rejects only the uninspected credentialed endpoints an edit introduces, while endpoints the sandbox's current policy already carries are logged and admitted.

Related Issue

Refs #2998 (second acceptance criterion). Reported downstream as openkaiden/kaiden#2759.

Note for maintainers: #2998 is state:validated but is not state:accepted and is not on the roadmap, so no human acceptance signal has been recorded yet. This PR was opened on a direct contributor request; no lifecycle or workflow labels were changed.

Changes

The bug. Operators saw this on edits that had nothing to do with the endpoint named:

Error: × code: 'The system is not in a state required for the operation's execution',
  message: "credentialed endpoint 'github.com:443' in rule 'pypi' uses L4-only;
  configure L7 inspection or explicitly set allow_uninspected_credentials: true"

Reported variants named pypi, copilot, and claude_code — all rules that come from the sandbox image's baked default policy, not from the operator.

Root cause. Three things combine:

1. A sandbox's default policy is baked into its image (see docs/reference/default-policy.mdx). The tutorial reproduces it at docs/get-started/tutorials/github-sandbox.mdx: the pypi rule allows github.com:443 and api.github.com:443 with no protocol, i.e. L4-only.
2. The supervisor syncs that on-disk policy up to the gateway, and that path deliberately skips the credential gate — rejecting the delivery would crash-loop the sandbox (grpc/policy.rs, sandbox-caller branch of handle_update_config_inner). sandbox create --provider … passes too, because it only validates the request's policy plus the provider layers, both of which are clean.
3. stamp_provider_credentialed_endpoints conservatively marks any endpoint whose host and port overlap a credentialed provider's scope. The image's L4-only github.com:443 in rule pypi therefore becomes credentialed.

Every operator-authored edit then re-validated the whole effective policy and rejected on that inherited endpoint. The gateway admitted a policy through one door and refused to let the operator touch it through any other — including the edit that would have fixed the offending endpoint.

The fix. The gate is now differential on edit paths and strict on authoring paths:

• collect_uninspected_credentialed_endpoints replaces the first-match find_…, and UninspectedCredentialedEndpointKey (rule name, host, port, uninspected mode) gives each violation an identity. mode participates deliberately: moving an inherited endpoint from L4-only to tls: skip is a fresh authoring act, not the same finding carried forward.
• validate_uninspected_credentialed_endpoints_excluding(policy, inherited) rejects only violations absent from the baseline; inherited ones emit a warn! and pass. The reported endpoint is chosen deterministically, so the message no longer depends on map iteration order.
• Stamping was split out into stamp_operator_merged_credential_policy and remains a full recomputation. Only the accept/reject decision is differential — the invariant in architecture/security-policy.md ("a delta-based derivation would let a series of individually valid edits reach a state no single edit would have admitted") still holds.
• Baselines reuse what each call site already had: current_policy in stage_validated_merge_operation and apply_merge_operations_with_retry, base_policy in proposal evaluation, and a new validate_updated_sandbox_credential_policy for policy set (built on the existing current_base_policy_for_sandbox + effective_policy_for_source, so baseline and candidate are composed and stamped by the identical routine).
• sandbox create and provider attach keep the strict whole-policy gate: attaching a credentialed provider is a genuinely new credential exposure and must fail loudly.

Why this is safe. The admission gate is not the security boundary. architecture/security-policy.md already states the network supervisor independently enforces the same boundary, and the delivery path already declines to fail closed for exactly this reason ("the runtime backstop denies the traffic instead"). Admitting an inherited endpoint grants nothing — the proxy still denies that traffic until the endpoint gets an inspected protocol or the explicit allow_uninspected_credentials exception.

Alternatives considered and rejected: narrowing the stamp to the provider's own rule (the conservative host+port scope is deliberate per architecture/security-policy.md, and narrowing it weakens the control); gating the supervisor sync path (crash-loops sandboxes running stock images); a blanket --allow-uninspected-credentials CLI flag (silently rewrites the operator's policy to disable a security control).

Docs: architecture/security-policy.md gains the differential-gate rule and its rationale; docs/sandboxes/policies.mdx notes that an inherited uninspected credentialed endpoint does not block unrelated edits and that its traffic is still denied at runtime.

Testing

Verified against this implementation on this branch:

• cargo test -p openshell-server — 1459 tests pass, 0 failures.
• mise run pre-commit — passes (lint, format, license headers).
• mise run test — full suite.

Six new tests in crates/openshell-server/src/grpc/policy.rs, covering both the bug and the boundary that must not move:

Case	Expected
Inherited violation + unrelated policy update --add-endpoint	Admitted, endpoint persisted
Inherited violation + policy set replaying the current base policy (the policy get --base round trip)	Admitted
Removing the offending rule via policy set	Admitted — the operator can now fix it
A newly authored L4-only endpoint on a credentialed host in a new rule	Still FailedPrecondition, message names the new rule
Inherited endpoint switched to tls: skip	Still rejected — mode is part of the identity
Empty baseline	Gate stays strict

The integration fixture mirrors the real setup: a stored profile shaped like providers/github.yaml, a policy carrying the tutorial's L4-only pypi rule, delivered through the ungated sandbox-sync path exactly as an image policy arrives in production.

One note for reviewers: merge_policy folds an added rule into whichever existing rule already owns that host and port (crates/openshell-policy/src/merge.rs). The first draft of the rejection test passed for the wrong reason because of this — the new rule was absorbed into pypi rather than added. The test now targets a host no existing rule covers, with the reason recorded inline.

• mise run pre-commit passes
• Unit tests added/updated
• E2E tests added/updated (if applicable) — e2e/rust/tests/credential_gating.rs already covers the strict gate end to end; the new behavior is fully exercised by unit tests against the real gRPC handlers, including the store-backed merge-retry loop.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

Out of scope

Deliberately left for separate work, called out so they are not lost:

• Tutorial Rot #2998's first acceptance criterion (a sandbox enters Error phase on exit and cannot be reconnected). Depends on open feature issue feat(sandbox): add main restart policy #2798 (feat(sandbox): add main restart policy).
• openshell policy get --base prints a metadata header above the --- line that must be hand-stripped before policy set can consume the file. docs/sandboxes/policies.mdx documents the manual step. A -o yaml output emitting only the policy body would close the round trip.
• providers/pypi.yaml ships L4-only github.com:443 and api.github.com:443. Attaching pypi and a credentialed GitHub provider to the same sandbox still fails at create time, on the strict path this PR intentionally leaves intact. That is a real bug in its own right and deserves its own issue.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[johntmyers]

gator-agent

PR Review Status

This is valid, focused work for the second acceptance criterion in #2998, and the docs cover the changed policy behavior. The initial review found two reachable correctness gaps in the new differential gate.

Action required: @feloy, please preserve the complete effective port set in inherited endpoint identity and make full-policy replacement validation use the policy revision it commits against.

Blocking findings:

• GATOR-b4f81f7e-01: multi-port expansion can inherit a lossy first-port identity
• GATOR-b4f81f7e-02: concurrent full replacement can use a stale inherited baseline

Carried findings:

• None

Non-blocking suggestions:

• None

Gator metadata

• Validation: Project-valid because it directly implements the policy round-trip acceptance criterion in validated issue #2998.
• Docs: Fern policy docs and the security architecture document are updated.
• Checks: DCO passes; contributor workflows have not been dispatched because copy-PR validation is still pending.
• E2E: test:e2e is required for policy-enforcement behavior, but dispatch waits until blocking review feedback is resolved.
• Head SHA: b4f81f7e5cfaf3272a0a7c84bda5e4b513e1118a
• Base SHA: 8bc79552638a625971f8c6d30c26efa917a19cf9
• Merge base SHA: 8bc79552638a625971f8c6d30c26efa917a19cf9
• Patch ID: 3053d1de810baad7df2a0fb1b31f477678cb5651
• Gator payload: 8
• Review mode: initial
• Previous reviewed SHA: none
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:in-review

[github-actions]

Label test:e2e applied, but pull-request/3129 does not exist yet. A maintainer needs to comment /ok to test 198906ab9c0cdd76ca3b4b2e97f2fac952c56066 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

[johntmyers]

/ok to test 198906a

[johntmyers]

gator-agent

PR Review Status

Thanks @feloy. I checked the per-port identity change and the relocation of replacement validation into the versioned persistence attempt on head 198906ab9c0cdd76ca3b4b2e97f2fac952c56066; both prior findings are resolved, their Gator threads are closed, and the bounded follow-up review found no new blockers. The required E2E dispatch has been authorized and is waiting for the copy-PR mirror before Branch E2E can be rerun.

Blocking findings:

• No blocking findings remain

Carried findings:

• None

Gator metadata

• Validation: Project-valid because it directly implements the policy round-trip acceptance criterion in validated issue #2998.
• Docs: Fern policy docs and the security architecture document are updated.
• Checks: DCO passes; contributor workflows for this head are not yet dispatched because the copy-PR mirror is still being created.
• E2E: test:e2e is applied and /ok to test 198906ab9c0cdd76ca3b4b2e97f2fac952c56066 was posted; Branch E2E is not yet queued.
• Head SHA: 198906ab9c0cdd76ca3b4b2e97f2fac952c56066
• Base SHA: 8bc79552638a625971f8c6d30c26efa917a19cf9
• Merge base SHA: 8bc79552638a625971f8c6d30c26efa917a19cf9
• Patch ID: d963565afa6ca996167ba81fd15c675e248310b8
• Gator payload: 8
• Review mode: follow_up
• Previous reviewed SHA: b4f81f7e5cfaf3272a0a7c84bda5e4b513e1118a
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:in-review

[github-actions]

Label test:e2e applied for 198906a. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

[johntmyers]

gator-agent

PR Review Status

The bounded follow-up review checked the clippy-only assertion rewrite on head 4d3c37c476d3223af65864b3e9584387f1558b21. It preserves the test's two outcomes, introduces no new blocker, and does not reopen either resolved finding.

Blocking findings:

• No blocking findings remain

Carried findings:

• None

Gator metadata

• Validation: Project-valid because it directly implements the policy round-trip acceptance criterion in validated issue #2998.
• Docs: Existing Fern policy docs and the security architecture update remain sufficient; this delta has no UX change.
• Checks: DCO passes; Branch Checks, Helm Lint, and E2E are waiting for the current-head copy-PR mirror.
• E2E: test:e2e remains required and applied; current-head dispatch is pending /ok to test mirror refresh.
• Head SHA: 4d3c37c476d3223af65864b3e9584387f1558b21
• Base SHA: 8bc79552638a625971f8c6d30c26efa917a19cf9
• Merge base SHA: 8bc79552638a625971f8c6d30c26efa917a19cf9
• Patch ID: 891cab22d4b514b493be7a213b8cef92ed25864f
• Gator payload: 8
• Review mode: follow_up
• Previous reviewed SHA: 198906ab9c0cdd76ca3b4b2e97f2fac952c56066
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:in-review

[johntmyers]

/ok to test 4d3c37c

[github-actions]

Label test:e2e applied for 4d3c37c. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

[johntmyers]

I want to hold on this PR. I think there's a deeper rooted set of issues to address which I am putting together an outline for.