bug: reserve `_provider_*` policy key prefix and enforce at gateway boundaries

[johntmyers]

Agent Diagnostic

Investigation reviewing PR #1914 and the discussion in #1914 (comment).

Skills/tools used: code search across crates/openshell-policy, crates/openshell-server/src/grpc, and crates/openshell-sandbox; git history on the original composition PR (#1037).

Findings:

• crates/openshell-policy/src/compose.rs:15-34 defines provider_rule_name() which prefixes provider-derived rules with _provider_. The doc comment at compose.rs:41-42 declares this key namespace reserved for derived data.
• crates/openshell-policy/src/compose.rs:62-75 (unique_provider_rule_key) collision-renames provider rules to _provider_<name>_2, _3, ... when a key already exists in the source policy. This is the mechanism by which a stale baked-in _provider_* rule shadows the live one.
• crates/openshell-policy/src/merge.rs:400-416 and merge.rs:642-656 already filter _provider_* keys out of the endpoint-overlap fallback, confirming the prefix is treated as reserved internally.
• crates/openshell-server/src/grpc/validation.rs:619-628 (validate_policy_safety) does not reject user-authored policy with _provider_* network policy keys. There is no enforcement at the gateway boundary for either SetSandboxPolicy, CreateSandbox, or the supervisor-originated enrichment write-back path.

Net: the prefix is reserved by convention and by the composition layer, but not by any input validation. That asymmetry produces the bug.

Description

Actual behavior: A user-authored or sandbox-enriched policy can contain network_policies keys with the _provider_* prefix. When provider profile composition runs, the composer detects the collision and re-keys the provider rule as _provider_<name>_2. The original (now stale) _provider_<name> from the user policy remains in the effective policy. Non-additive provider profile updates (rename / remove endpoint) silently no-op because the live _2 rule sits behind the stale baseline rule.

This breaks the core update flow that PR #1914 introduces: openshell provider profile update succeeds, but for any sandbox whose persisted policy already contains the prior _provider_* key, the change does not take effect.

Expected behavior: The _provider_* key namespace is reserved for derived data. Any user-supplied policy or sandbox-originated write that attempts to set keys under that namespace is rejected (or filtered) at the gateway boundary, so provider composition always produces a single authoritative _provider_<name> rule per profile.

Reproduction Steps

Path 1 — policy get --full round-trip (user-driven)

openshell provider create --name test --type github --credential FOO=bar
openshell sandbox create --provider test -- true
openshell policy get <sandbox> --full > current-policy.yaml
openshell policy set <sandbox> -f current-policy.yaml
openshell provider profile update -f updated-profile.yaml  # remove or rename an endpoint
openshell policy get <sandbox> --full
# Observed: _provider_test (stale, from user policy) AND _provider_test_2 (current)

Path 2 — supervisor enrichment write-back (no user action)

Reproducer from pimlock — happens automatically as part of normal sandbox startup:

cat > custom-policy.yaml <<END
version: 1
filesystem_policy:
  include_workdir: true
  read_only: []
  read_write: []
landlock:
  compatibility: best_effort
process:
  run_as_user: sandbox
  run_as_group: sandbox
network_policies:
  sandbox_only:
    endpoints:
      - host: sandbox.example.com
        port: 443
        protocol: rest
        access: read-only
        enforcement: enforce
    binaries: []
END

openshell provider create --name test --type github --credential FOO=bar
openshell sandbox create --policy custom-policy.yaml --provider test -- true
openshell policy get --full
# Observed: both _provider_test and _provider_test_2 present

Full script with pauses for live-state exploration: https://gist.github.com/pimlock/d10de2a4ca5611a9a9ae78fce7e0a489?permalink_comment_id=6212304#gistcomment-6212304

Root cause of path 2: the supervisor enriches the effective policy (including provider-injected rules) and syncs it back to the gateway. The gateway treats the synced policy as authoritative user policy, baking the _provider_* keys into the source. On the next provider poll, composition collision-renames the new provider rule to _provider_test_2.

Proposed Fix

1. Validation: extend validate_policy_safety (or add a sibling validator) to reject network_policies keys matching ^_provider_ on every user-facing write path — at minimum SetSandboxPolicy and CreateSandbox. Error message should explain that the prefix is reserved for provider composition and direct users to policy get (without --full) for round-trippable output.
2. Supervisor enrichment write-back: strip _provider_* keys before the supervisor pushes enriched policy back to the gateway. Provider-derived rules are gateway-authoritative; the sandbox should never be the source of truth for them. Belt-and-braces: the gateway's enrichment ingestion path should also drop any _provider_* keys it receives from a sandbox.
3. No change to policy get --full: --full continues to render the effective policy including provider contributions — that's the purpose of the flag. policy get without --full already excludes _provider_* (merge.rs:416), so the documented round-trip workflow stays intact.
4. Docs: brief note in docs/sandboxes/manage-providers.mdx about the reservation.

Beta-stage, hard reject is fine.

Environment

• Branch: main / PR feat(providers): support profile updates #1914 branch
• Affects: gateway server (openshell-server), sandbox supervisor (openshell-sandbox), policy engine (openshell-policy).

Related

• PR feat(providers): support profile updates #1914 — surfaces the bug because non-additive profile updates now have a user-facing entry point.
• PR feat(providers): add profile-backed policy composition #1037 — original _provider_* composition design.
• Comment thread: feat(providers): support profile updates #1914 (comment)

[johntmyers]

🏗️ build-plan

Implementation Plan

Issue type: fix
Complexity: Medium
Confidence: High — clear path

Summary

Reserve _provider_* as a gateway-owned network policy key namespace at policy ingress. User-authored full policy writes should reject reserved keys with a targeted error, while sandbox-originated sync should strip provider-derived rules before backfilling or persisting source policy. policy get --full should continue showing effective provider rules.

Scope

• crates/openshell-policy/src/compose.rs: add/export a shared reserved-provider-rule predicate or constant; keep composition semantics unchanged.
• crates/openshell-policy/src/merge.rs: replace raw _provider_ prefix checks with the shared predicate without changing merge behavior.
• crates/openshell-server/src/grpc/validation.rs: add reserved network-policy key validation and unit coverage with an error that points users to round-trippable policy output.
• crates/openshell-server/src/grpc/sandbox.rs: enforce reserved-key rejection for CreateSandbox policy payloads.
• crates/openshell-server/src/grpc/policy.rs: reject reserved keys for user-authored full sandbox policy updates; strip reserved keys from sandbox-principal policy sync before CAS backfill and revision persistence; keep JIT effective-policy composition unchanged.
• crates/openshell-sandbox/src/lib.rs: strip provider-derived network policy entries before supervisor discovery/enrichment write-back.
• docs/sandboxes/providers-v2.mdx: document the reserved namespace and rewrite the stale collision-suffix wording.
• docs/sandboxes/policies.mdx: clarify that replaying policy get --full requires removing generated _provider_* entries before policy set.

Implementation Steps

1. Add the shared reserved-prefix helper in openshell-policy, then refactor existing provider-prefix checks in compose/merge and server policy code to use it.
2. Add gateway validation for user-authored full policy payloads on CreateSandbox and sandbox-scoped UpdateConfig / policy set, returning INVALID_ARGUMENT for _provider_* keys.
3. Add gateway sanitization for sandbox-principal UpdateConfig so incoming _provider_* entries are removed before spec.policy backfill and policy revision persistence.
4. Add sandbox-side sanitization before discover_and_sync_policy / sync_policy write-back so current supervisors stop sending provider-derived rules.
5. Update docs and adjust existing provider-composition tests that currently assert suffixing for user-authored _provider_* collisions.

Test Plan

• Unit tests:
  • crates/openshell-policy/src/compose.rs: reserved-prefix helper accepts _provider_foo and rejects non-reserved keys.
  • crates/openshell-server/src/grpc/validation.rs: reserved-key validator rejects _provider_* keys and accepts ordinary network policy keys.
  • crates/openshell-sandbox/src/lib.rs: strip helper removes only reserved provider entries and preserves user-authored entries.
• Integration-style handler tests:
  • crates/openshell-server/src/grpc/sandbox.rs: CreateSandbox rejects a policy containing _provider_* network policy keys.
  • crates/openshell-server/src/grpc/policy.rs: user UpdateConfig rejects reserved keys; sandbox-principal sync strips reserved keys before backfilling spec.policy and persisting the revision; effective provider composition still returns provider rules while stored source policy does not.
• E2E tests: N/A — this is validation/persistence hygiene in existing handler paths; no e2e files are expected to change.

Risks & Open Questions

• Global policy writes use validate_policy_safety today. The implementation should avoid accidentally changing global policy semantics unless maintainers explicitly want the reserved namespace rejected there as well.
• Gateway-side sanitization is required even with sandbox-side stripping, because older supervisors and alternate sandbox clients may still send provider-derived rules.
• Existing docs/tests that describe collision suffixing need to be updated so they do not preserve the old behavior as expected behavior.

Documentation Impact

• Update docs/sandboxes/providers-v2.mdx and docs/sandboxes/policies.mdx.
• docs/sandboxes/manage-providers.mdx can get a brief cross-reference if the final implementation touches provider command docs.
• docs/reference/gateway-config.mdx is not affected because this does not add, remove, rename, or change defaults for gateway TOML or driver config fields.

LSM Compatibility

No SELinux/AppArmor-specific impact expected. The change is policy validation and persistence hygiene only; it does not alter process identity, /proc access, binary execution, inter-process visibility, or LSM enforcement behavior.

---

Revision 1 — initial plan

[johntmyers]

🏗️ build-from-issue-agent

Implementation Complete

PR: #1991

What was built

Reserved _provider_* as a provider-derived network policy namespace. User-authored sandbox policy writes now reject reserved keys, and sandbox-originated sync strips provider-derived rules before persistence.

Tests

• Unit/focused: provider namespace helpers, gateway validation, CreateSandbox/UpdateConfig handlers, sandbox sync stripping
• Pre-commit: RUSTC_WRAPPER= mise run pre-commit passed
• E2E: N/A — no e2e/ files changed

Docs updated

• docs/sandboxes/providers-v2.mdx
• docs/sandboxes/policies.mdx

The issue will auto-close when the PR is merged.