fix(policy): reserve provider rule namespace

[johntmyers]

🏗️ build-from-issue-agent

Summary

Reserve _provider_* as a provider-derived network policy namespace. User-authored sandbox create, full policy replacement, and incremental add-rule writes now reject reserved keys, while sandbox-originated policy sync strips provider-derived rules before persisting source policy.

Related Issue

Closes #1982

Changes

• crates/openshell-policy: added shared provider rule namespace helpers and reused them in merge fallback guards.
• crates/openshell-server: added reserved-key validation for user policy writes, sandbox-principal sync stripping, and regression tests for create/update/merge paths.
• crates/openshell-sandbox: strips provider-derived entries before supervisor policy sync/write-back.
• docs/sandboxes/providers-v2.mdx and docs/sandboxes/policies.mdx: documented the reserved namespace and round-trip behavior.

Deviations from Plan

Added rejection for incremental add_rule merge operations using _provider_* rule names. This closes the same user-facing policy-key ingress class while still allowing remove-style operations for cleanup.

Testing

• RUSTC_WRAPPER= cargo test -p openshell-policy provider_rule
• RUSTC_WRAPPER= cargo test -p openshell-server reserved_provider --lib
• RUSTC_WRAPPER= cargo test -p openshell-sandbox provider_policy_entries --lib
• RUSTC_WRAPPER= mise run pre-commit passes
• E2E skipped: no files under e2e/ changed

Tests added:

• Unit: provider namespace helper and strip behavior in openshell-policy; reserved-key validation in openshell-server; supervisor strip helper in openshell-sandbox.
• Integration-style handler: CreateSandbox reserved-key rejection; user UpdateConfig reserved-key rejection; sandbox-principal sync stripping before spec.policy and policy revision persistence.
• E2E: N/A.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)

Documentation updated:

• docs/sandboxes/providers-v2.mdx: reserved _provider_* namespace behavior.
• docs/sandboxes/policies.mdx: full-policy round-trip guidance for provider-derived entries.

[github-actions]

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

[johntmyers]

gator-agent

PR Review Status

Validation: This PR is project-valid because it implements the linked policy/gateway bug fix in #1982, keeps the scope concentrated to provider-derived policy namespace handling, and includes docs plus targeted regression coverage.
Head SHA: 071b7e3bde0586d30dc7529041e92d128b1cb637

Review findings:

• Blocking: crates/openshell-sandbox/src/lib.rs:1357 strips _provider_* entries from the fetched effective policy before OpaEngine::from_proto(&proto_policy) at crates/openshell-sandbox/src/lib.rs:1378. GetSandboxConfig composes provider layers into that response, so sandboxes with Providers v2 would drop provider-derived network rules from runtime enforcement. Keep the effective policy for runtime and strip only a clone used for sync/persistence. Add a regression test that provider-derived rules remain in the returned runtime policy / OPA input while the synced payload is stripped.
• Warning: crates/openshell-server/src/grpc/policy.rs:1507 still lets global full policy updates accept _provider_* keys. Global policy suppresses provider composition, so this is not the original collision bug, but it conflicts with the new docs and leaves one user-authored full-policy ingress path unvalidated. Either reject reserved keys there too or narrow the docs.

Docs: Updated for the direct policy UX change, but the warning above should be reconciled with the documented behavior.

Next state: gator:in-review

[johntmyers]

Addressed the Gator feedback in b4f6ced8.

• Runtime provider policy: sandbox fetch now keeps the composed effective policy intact for OPA input and strips reserved provider entries only from a cloned sync payload. Added regression test proto_sync_payload_strips_provider_entries_without_mutating_runtime_policy.
• Global updates: global full policy updates now call validate_no_reserved_provider_policy_keys, with update_config_global_policy_rejects_reserved_provider_key covering the rejected ingress path.

Validation:

• cargo fmt --all
• RUSTC_WRAPPER= cargo test -p openshell-sandbox sync_payload --lib
• RUSTC_WRAPPER= cargo test -p openshell-server reserved_provider --lib
• RUSTC_WRAPPER= cargo test -p openshell-server update_config_global_policy_rejects_reserved_provider_key --lib
• RUSTC_WRAPPER= mise run pre-commit

[johntmyers]

gator-agent

Re-check After Author Update

I re-evaluated latest head b4f6ced8a85dc6a613fa970ff069f08488fdd098 after the author's 2026-06-24 update saying the prior gator feedback was addressed.

Disposition: partially resolved. The prior runtime-policy mutation and global full-policy ingress findings are addressed, but one blocking legacy-data path remains.

Remaining items:

• Blocking: existing persisted _provider_* source rules can still reach runtime policy before cleanup. current_effective_policy_for_sandbox and GetSandboxConfig compose stored/spec policy before stripping legacy reserved keys, so an already-affected sandbox can still receive stale _provider_* rules plus the newly composed provider rule. The sandbox then builds OPA from that unstripped fetched policy and strips only the sync clone. Incremental merges can also carry a legacy reserved key forward from the current revision. Please strip reserved provider keys from source policy before effective-policy composition, before GetSandboxConfig returns runtime policy, before lazy backfill persistence from spec.policy, and before merge persistence. Replace the regression test that currently asserts both _provider_work_github and _provider_work_github_2 remain present.
• Add regression coverage for legacy data cleanup: latest policy revision with _provider_* should not appear in GetSandboxConfig; runtime fetch should not include stale provider keys; incremental merge from a legacy latest revision should not preserve _provider_*; detaching a provider should not leave stale _provider_* grants active.

Docs: updated in the relevant Fern docs pages; no navigation change appears necessary.

Next state: gator:in-review

[johntmyers]

Follow-up in 5d9c464d on the remaining Gator item.

Project assumption for this PR: persisted user/source policies with _provider_* network policy keys are absent and unsupported. Given that, the remaining legacy-data cleanup paths and the requested legacy cleanup regression tests are out of scope for this fix; adding those tests would make a migration guarantee we are not taking on here.

I did remove the unsupported overlap regression that asserted a user/source _provider_work_github plus generated _provider_work_github_2 should both survive. The server test now covers the supported case: normal user-authored rule names compose alongside provider-generated _provider_* rules. The compose unit test now covers suffixing only for provider/provider rule-name collisions.

Validation:

• cargo fmt --all
• RUSTC_WRAPPER= cargo test -p openshell-policy compose_concatenates_provider_rules_without_overwriting_user_rules --lib
• RUSTC_WRAPPER= cargo test -p openshell-server sandbox_config_composes_user_and_provider_rules --lib
• RUSTC_WRAPPER= mise run pre-commit

[johntmyers]

gator-agent

Re-check After Author Update

I re-evaluated latest head 5d9c464dac45bc606930851899aa9490e175dd39 after the author's 2026-06-24 update saying the remaining legacy-data cleanup request was out of scope for this fix.

Disposition: not resolved. The earlier legacy-data migration scope is no longer the blocking item, but the latest head introduces a new runtime sync regression that needs author changes before gator can move this PR to pipeline watch.

Remaining items:

• Blocking: crates/openshell-sandbox/src/lib.rs:1392 computes stripped_provider_entries from the fetched effective policy, which will contain _provider_* entries for provider-attached sandboxes. The condition at crates/openshell-sandbox/src/lib.rs:1394 then syncs policy on every provider-backed sandbox startup even when baseline enrichment did not change anything. That can create redundant policy revisions/resource-version churn and increases the race window for sandbox sync to overwrite concurrent user policy edits. Strip provider entries only from the payload when a sync is already required for enrichment, and add regression coverage proving provider-derived entries alone do not trigger sync.

Docs: updated in the relevant Fern docs pages; no navigation change appears necessary.

Next state: gator:in-review

[johntmyers]

Addressed the latest Gator feedback and rebased onto current origin/main in ceaf8dc2.

Change made:

• Sandbox policy sync is now gated only by baseline enrichment. Provider-derived _provider_* entries are stripped from the sync payload only after enrichment has already required a sync, so provider-backed sandboxes do not churn policy revisions on startup just because the fetched runtime policy contains provider rules.
• Added regression coverage that provider-derived entries alone do not create a sync payload, while enrichment-triggered sync still strips provider entries without mutating the runtime policy used for OPA.

Validation:

• cargo fmt --all
• RUSTC_WRAPPER= cargo test -p openshell-sandbox proto_sync_payload --lib
• RUSTC_WRAPPER= cargo test -p openshell-server reserved_provider --lib
• RUSTC_WRAPPER= mise run pre-commit

[johntmyers]

gator-agent

PR Review Status

Validation: This PR is project-valid because it implements the linked provider-policy namespace hardening bug in #1982, keeps the scope concentrated to policy/gateway/sandbox sync boundaries, and updates the relevant Fern docs plus regression coverage.
Head SHA: ceaf8dc253f8864ff467765920cff42be2d5fd29

Review findings:

• Blocking: crates/openshell-server/src/grpc/policy.rs:3599 still lets already-stored draft chunks merge a reserved _provider_* rule name. New SubmitPolicyAnalysis requests reject those names, but manual approval, bulk approval, and auto-approval all call merge_chunk_into_policy, which constructs PolicyMergeOp::AddRule directly and bypasses validate_merge_operations_for_server. Because draft chunks are agent/proposal-originated data that can become active policy, please validate the constructed AddRule operation inside merge_chunk_into_policy before calling apply_merge_operations_with_retry, and add regression coverage for approval rejecting a stored _provider_* chunk.

Docs: updated in the relevant Fern docs pages; no navigation change appears necessary.

Next state: gator:in-review

[johntmyers]

Addressed the latest Gator feedback in 1b887bb4.

The approval merge path now validates the constructed PolicyMergeOp::AddRule inside merge_chunk_into_policy before persisting a policy revision. That covers manual approval, approve-all, and auto-approval because they all flow through this helper.

I also added regression coverage for a stored pending draft chunk named _provider_work_github: approval now returns InvalidArgument, leaves the chunk pending, and does not write a policy revision.

Verification run:

• cargo fmt --all
• RUSTC_WRAPPER= cargo test -p openshell-server approve_draft_chunk_rejects_stored_reserved_provider_rule_name --lib
• RUSTC_WRAPPER= cargo test -p openshell-server reserved_provider --lib
• RUSTC_WRAPPER= mise run pre-commit

[johntmyers]

gator-agent

PR Review Status

Validation: This PR is project-valid because it implements the linked provider-policy namespace hardening bug in #1982, keeps the scope concentrated to policy/gateway/sandbox sync boundaries, and updates the relevant Fern docs plus regression coverage.
Head SHA: 1b887bb41d8ce547d44687d76bd732600283e798

Review findings:

• No blocking findings remain. The independent reviewer found only a non-blocking wording suggestion for the reserved-key error message used by global policy updates.

Docs: updated in the relevant Fern docs pages; no navigation change appears necessary.

E2E: test:e2e is required because this touches policy enforcement and sandbox/gateway policy sync behavior.

Next state: gator:watch-pipeline

[github-actions]

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

[pimlock]

Looks good, just one comment around the error message/options for getting the policy document with our CLI.

[pimlock]

Without --full, the policy document is not returned at all. The way to get the policy without provider entries is to do openshell policy get --rev LAST_REV --full.

I think we need a flag to output policy without provider entries.

Thinking through options of how these 2 could be presented

# Option 1. what we have
policy get --user/base
policy get --full 
# or
policy get --include-providers # full may be better, because there may be more things contributing to the overall policy

# Option 2. explicit --show with param that controls what is shown (alternatively '--print')
policy get --show full
policy get --show user/base # or --print

# Option 3. use get for metadata and show for policy document?
policy show # prints the base one
policy show --full
policy show --include-providers

Looking at these, I think the clearest setup could be:

• flag/subcommand that controls if the policy document is printed
• flag that controls what should be included in the policy document

So I think the option 2 may be the sweet spot?

[johntmyers]

Implemented this in 9430cef2.

openshell policy get now has an explicit --base payload mode for the round-trippable user-authored policy, and --full remains the effective/composed view including provider entries. The flags are mutually exclusive. The --base implementation strips reserved _provider_* entries from the policy payload before YAML/JSON serialization, so users can do:

openshell policy get <sandbox> --base > current-policy.yaml
openshell policy set <sandbox> --policy current-policy.yaml --wait

I also updated the reserved-prefix validation message to point at --base and refreshed the Providers v2 / policy docs to use base policy + provider policy => effective policy terminology.

Verification:

• cargo fmt --all
• RUSTC_WRAPPER= cargo test -p openshell-cli policy_get --bins
• RUSTC_WRAPPER= cargo test -p openshell-cli --test sandbox_name_fallback_integration policy_get
• RUSTC_WRAPPER= cargo test -p openshell-server validate_no_reserved_provider_policy_keys --lib
• RUSTC_WRAPPER= mise run pre-commit

[johntmyers]

gator-agent

PR Review Status

Validation: This PR is project-valid because it implements the linked provider-policy namespace hardening bug in #1982, keeps the scope concentrated to policy/gateway/sandbox/CLI boundaries, and updates the relevant Fern docs plus regression coverage.
Head SHA: 9430cef2a914a13ef27ac1c602f0729dd3069bab

Review findings:

• No blocking findings remain. The independent reviewer raised two non-blocking cautions: already-persisted contaminated _provider_* source policies remain outside this PR's accepted scope from the earlier gator disposition, and the new --base UX still emits the existing metadata header in table output while the docs tell users to strip the header before reusing the YAML.

Docs: updated in the relevant Fern docs pages; no navigation change appears necessary.

E2E: test:e2e is required because this touches policy enforcement and sandbox/gateway policy sync behavior. The required OpenShell / E2E gate is currently queued for this head.

Next state: gator:watch-pipeline