fix(policy): validate the applicable advisor candidate

[johntmyers]

Summary

Fixes policy-advisor approval so the prover evaluates the exact effective-policy candidate that can actually be applied. Reviews are bound to live policy, provider, and non-secret credential inputs; changed inputs refresh the candidate and require fresh review, while unchanged candidates reuse the stored prover result.

Related Issue

Closes#2821

Changes

• Canonicalize mechanistic endpoint proposals against the live effective policy, preserving existing L7 inspection contracts and treating provider rules as immutable inputs.
• Validate the complete merge, provider composition, credential preflight, and L7 policy shape before approval; persist actionable application errors separately from prover results.
• Persist current/candidate effective-policy hashes and an opaque review token, and require token-bound single and bulk approvals.
• Recompute from one live provider snapshot at approval time; reuse the prover verdict when the candidate token is unchanged and require fresh review when it changes.
• Reconcile pending chunks after successful policy writes so proposals already covered by the active policy leave the inbox.
• Surface candidate state in the CLI, TUI, policy.local API, and Go SDK.
• Document the flow and add a Docker E2E regression for expanding an existing inspected endpoint without downgrading it to L4.

Testing

• mise run pre-commit passes
• mise run ci passes
• Unit tests added/updated
• cargo test -p openshell-policy
• cargo test -p openshell-server grpc::policy::tests
• mise run go:ci
• mise run sdk:ts:ci
• E2E tests added/updated
• mise run e2e:mechanistic-existing-endpoint
• mise run e2e:docker

Checklist

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

[github-actions]

🌿 Preview your docs:https://nvidia-preview-pr-2850.docs.buildwithfern.com/openshell

[johntmyers]

gator-agent

Blocked

Head SHA: 2d72d0662cbe0ae70e80e7df3347d2e77cd44c51
Gator payload: 4

Gator is blocked because PR #2850 has merge conflicts with main.

Next action: @johntmyers, update the branch from current main, resolve the conflicts, and push the resolved head. Gator will run the independent code review after GitHub reports the PR mergeable.

[johntmyers]

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: 6b82f885654867d1b2fe391fff2610dfc35283b5
Base SHA: ef296806f52c03956a4bb7ce9a384105160f9c6a
Merge base SHA: ef296806f52c03956a4bb7ce9a384105160f9c6a
Patch ID: f04e290221220295af9e228daa89d7891c2992fe
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no

Blocking findings:

• GATOR-6b82f885-01: Review tokens and effective-policy hashes use non-canonical protobuf encodings for nested map fields, so unchanged supported policies can appear stale.
• GATOR-6b82f885-02: Canonicalizing a one-port denial against a multi-port inspected endpoint can grant the new binary every port in that endpoint.

Carried findings:

• None

Non-blocking suggestions:

• None

Docs: Fern documentation and the related CLI skill/reference were updated for the user-visible review-token and application-error behavior.

Next state: gator:in-review

[johntmyers]

🏗️ build-from-issue-agent

Addressed both blocking Gator findings in d9d38a561:

• Canonical review/effective-policy hashing now recursively sorts protobuf maps while preserving repeated-field order.
• A one-port advisor observation can reuse inspection metadata but cannot inherit sibling ports from a multi-port endpoint; it remains a narrow sandbox overlay.
• Added focused policy and gateway regressions for nested map ordering and multi-port binary isolation.

Verification:

• cargo test -p openshell-policy — 233 passed
• cargo test -p openshell-server grpc::policy::tests — 195 passed
• cargo clippy -p openshell-policy -p openshell-server --all-targets -- -D warnings — passed
• mise run pre-commit — passed after rebasing onto current main
• mise run e2e:mechanistic-existing-endpoint — attempted twice; both runs stopped during sandbox provisioning with MainProcessExited before reaching policy evaluation. The gateway started healthy, and the equivalent in-process gateway regression passes.

[johntmyers]

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: d9d38a561944dd041342c004dbfe6ff5b3c8d8de
Base SHA: 6c34a3c6458f1c1852fb9a89bb89b778a99fa19b
Merge base SHA: 6c34a3c6458f1c1852fb9a89bb89b778a99fa19b
Patch ID: e623f3b05064d830132c4a6d3fe099bba96f4397
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: 6b82f885654867d1b2fe391fff2610dfc35283b5
Review budget exhausted: no
Maintainer decision required: no

Thanks @johntmyers. I checked your two remediation updates in the author-only delta: canonical serialization now covers the relevant nested protobuf maps while preserving repeated-field order, and advisor canonicalization narrows a copied inspection contract to the observed port before owner selection.

Blocking findings:

• No blocking findings remain. GATOR-6b82f885-01 and GATOR-6b82f885-02 are resolved by this head.

Carried findings:

• None.

Docs: Fern documentation and the related CLI skill/reference cover the user-visible review-token and application-error behavior.

Next state: gator:in-review pending required E2E dispatch; pipeline watch begins only after a current-head E2E workflow is queued, running, or complete.

[github-actions]

Label test:e2e applied for d9d38a5. 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.

[zredlined]

@johntmyers Fix looks good to me. Ran a 30-min session with agent auto-approver, checking for the failure condition. 8 ambiguous agent policy proposal attempts happened in the session, each rejected before review (vs failing after approval)

[johntmyers]

🏗️ build-from-issue-agent

CI Diagnosis and E2E Test Attestation

The three failures had one lifecycle/test mismatch:

• OIDC sandbox creation expected attached process output.
• Managed-workspace sandbox creation expected attached process output.
• Operator-workspace sandbox creation expected attached process output.

Current main fixes those exact assertions in #2856 by using detached durable sandboxes, and #2857 fixes stale main-process exit handling. PR #2850 is now rebased onto both.

The issue-specific regression had the same one-shot-main-process assumption, so 8e77aef74 now creates its sandbox detached with sleep infinity.

Field	Value
Commit	8e77aef74
Command	mise run e2e:mechanistic-existing-endpoint
Gateway mode	Docker
Result	✅ Passed

Test summary

• Initial curl request denied with HTTP 403.
• Advisor chunk auto-approved with prover: no new findings.
• REST/read-only endpoint contract preserved.
• Policy hot-reloaded and the subsequent curl request succeeded.

Additional verification:

• cargo test -p openshell-policy — 233 passed
• cargo test -p openshell-server grpc::policy::tests — 195 passed
• mise run pre-commit — passed

[johntmyers]

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: 8e77aef742289e49d008571081f0814efe7968e1
Base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Merge base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Patch ID: a706ae97aee1ad413ab12fbd3ceb531ac450a34c
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: d9d38a561944dd041342c004dbfe6ff5b3c8d8de
Review budget exhausted: no
Maintainer decision required: no

Thanks @johntmyers. I checked the latest author-only delta and confirmed that it changes the issue regression to create a detached sandbox with a durable canonical process before exercising denial, approval, and hot reload. The two earlier reviewed commits are patch-equivalent across the rebase, so the prior resolutions remain valid. I also considered @zredlined's maintainer report that repeated ambiguous proposals were rejected before review; it is consistent with the intended fail-closed behavior and does not introduce a new obligation.

Blocking findings:

• No blocking findings remain. GATOR-6b82f885-01 and GATOR-6b82f885-02 remain resolved.

Carried findings:

• None.

Docs: Fern documentation and the related CLI skill/reference cover the user-visible review-token and application-error behavior; the latest delta changes only E2E lifecycle setup.

Next state: gator:watch-pipeline; current-head Branch Checks and E2E workflows are running.

[johntmyers]

🏗️ build-from-issue-agent

Implemented the approve-all follow-up in 22e595cb1.

What changed

• Bulk approval now validates reviewed chunks against one live input snapshot, stages compatible operations in request order, and writes the resulting policy once.
• A conflict inside the batch skips only that chunk, leaves it pending with an application error, and allows compatible chunks to be applied together.
• Live inputs are recomputed immediately before persistence. If the reviewed snapshot is stale, the full write is rejected, so no partial policy update is applied.
• Unchanged proposal hashes reuse the cached prover evaluation instead of rerunning the prover.
• Provider-derived rules remain validation inputs; they are never copied into or mutated as part of the sandbox policy write.
• The TUI no longer labels every skipped chunk as security-flagged. It directs the user to review the remaining pending chunks.
• Proto comments now document the one-snapshot/one-write contract and the broader meaning of chunks_skipped.

Regression coverage

• Multiple independent chunks are approved in one policy revision.
• Cached proposal evaluation is retained for unchanged inputs.
• A later advisor_proposed / inspected-endpoint conflict is skipped without discarding the compatible prefix.
• A stale reviewed snapshot applies none of the batch.
• TUI skip feedback does not assume a security finding.

Verification

• cargo test -p openshell-server grpc::policy::tests — 198 passed
• cargo test -p openshell-tui --lib — 47 passed
• cargo clippy -p openshell-server -p openshell-tui --all-targets -- -D warnings
• mise run pre-commit
• mise run test
• mise run ci

[johntmyers]

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: 22e595cb1749f2494d4cac8c3f63fbe42f94db84
Base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Merge base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Patch ID: 748f0cdcd8a263791af8d06c47b699cfcba91eb1
Gator payload: 4
Review mode: critical_only
Previous reviewed SHA: 8e77aef742289e49d008571081f0814efe7968e1
Review budget exhausted: yes
Maintainer decision required: no

Thanks @johntmyers. I checked your bulk-approval update in the author-only delta, including the one-snapshot staging path, one-write persistence guard, stale-input rejection, provider-layer validation, per-chunk conflict handling, and TUI result wording. The critical-only convergence pass found no newly introduced Critical defect.

Blocking findings:

• No blocking findings remain. GATOR-6b82f885-01 and GATOR-6b82f885-02 remain resolved.

Carried findings:

• None.

Docs: Existing Fern documentation covers the user-visible review-token and application-error behavior; this delta updates the bulk RPC contract and TUI wording without introducing a new setup flow.

Next state: gator:watch-pipeline; current-head Branch Checks and Helm Lint are green, and the required E2E workflow is queued/running.

[johntmyers]

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Head SHA: 22e595cb1749f2494d4cac8c3f63fbe42f94db84
Gator payload: 4

Final status: the PR merged while gator was monitoring the pipeline.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.