Skip to content

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

Closed
#1991
Closed
bug: reserve _provider_* policy key prefix and enforce at gateway boundaries#1982
#1991
Labels
area:gatewayGateway server and control-plane workarea:policyPolicy engine and policy lifecycle workarea:providersstate:agent-readyApproved for agent implementationstate:pr-openedPR has been opened for this issue

Description

@johntmyers
johntmyers
opened on Jun 23, 2026

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

Related

Activity

  1. johntmyers
    added
    area:policyPolicy engine and policy lifecycle work
    area:gatewayGateway server and control-plane work
    area:providers
    on Jun 23, 2026

  2. johntmyers commented on Jun 24, 2026

    @johntmyers
    johntmyers
    on Jun 24, 2026
    CollaboratorAuthor

    🏗️ 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

  3. johntmyers
    added
    state:review-readyReady for human review
    state:agent-readyApproved for agent implementation
    state:in-progressWork is currently in progress
    on Jun 24, 2026

  4. johntmyers commented on Jun 24, 2026

    @johntmyers
    johntmyers
    on Jun 24, 2026
    CollaboratorAuthor

    🏗️ 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.

  5. johntmyers
    added
    state:pr-openedPR has been opened for this issue
    and removed
    state:in-progressWork is currently in progress
    state:review-readyReady for human review
    on Jun 24, 2026
  6. johntmyers
    added 5 commits that reference this issue on Jun 24, 2026

    fix(policy): reserve provider rule namespace

    b4f6ced

    fix(policy): reserve provider rule namespace

    5d9c464

    fix(policy): reserve provider rule namespace

    ceaf8dc

    fix(policy): reserve provider rule namespace

    1b887bb

    fix(policy): reserve provider rule namespace

    9430cef
  7. johntmyers
    closed this as completedin #1991on Jun 26, 2026
  8. pull
    added a commit that references this issue on Jun 26, 2026

    fix(policy): reserve provider rule namespace (NVIDIA#1991)

    b855d8d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:gatewayGateway server and control-plane workarea:policyPolicy engine and policy lifecycle workarea:providersstate:agent-readyApproved for agent implementationstate:pr-openedPR has been opened for this issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions