bug(gateway): provider environment revision is not deterministic when a provider profile has several annotations #3929

Activity

1. github-actions

   added

   state:triage-neededOpened without agent diagnostics and needs triageOpened without agent diagnostics and needs triage

   on Sep 30, 2026
2. letv1nnn commented on Sep 30, 2026

   

   letv1nnn

   on Sep 30, 2026

   Contributor

   More actions

   📋 triage-agent

   Triage Assessment

   Classification: validated-bug

   Summary

   Confirmed on main and in v0.1.0, the reporter's version, with high confidence. openshell-core generates protobuf map fields as std::collections::HashMap because crates/openshell-core/build.rs configures no btree_map. prost encodes map entries in iteration order. Every request re-decodes provider profiles from the store, so each decode builds a new HashMap with a fresh RandomState. The profile's encode_to_vec() output that feeds the provider environment revision therefore varies between calls whenever a map field has two or more entries.

   Investigation

   • Empirical reproduction on main. A ProviderProfile with two annotations, encoded once, then decoded and re-encoded 200 times, produced exactly 2 distinct byte encodings. This matches the reported alternation between two provider_env_revision values. With n entries, up to n! orderings are possible.
   • Hash sites that include profile encodings:
     • crates/openshell-server/src/provider_profile_sources.rs:105 and :126 (user profile source revision)
     • provider_profile_sources.rs:498, hash_scoped_profile_revision. Reached from grpc/policy.rs through compute_provider_env_revision_with_catalog_and_policy_bindings and compute_provider_env_revision_from_records_and_policy_bindings.
     • provider_profile_sources.rs:712 is test-only.
   • Already deterministic, not affected:
     • The provider's own credentials and credential_expiration_times, which policy.rs key-sorts before hashing.
     • The policy part of the revision, which uses deterministic_policy_hash in openshell-core/src/policy_identity.rs.
     • Only the profile encoding path lacks canonicalization.
   • Consistency within a call. Order is stable inside a single decoded instance, because HashMap::clone preserves hasher keys and layout. Each RPC builds its own catalog through snapshot_catalog, though, so GetSandboxConfig and GetSandboxProviderEnvironment independently land on one of the orderings and disagree about half the time. Separate gateway replicas would also disagree.
   • Supervisor reaction, which explains the logs:
     • openshell-supervisor/src/lib.rs sets provider_env_changed, requires the fetched environment identity to equal the desired identity, and otherwise emits the FAIL_CLOSED "static credentials were revoked" event and revokes the static environment.
     • Startup gives up after 5 unstable attempts.
     • On the server, configuration_generation_matches in grpc/policy.rs produces the ReportEndpointStatus FailedPrecondition.
   • Map fields affected beyond annotations. ProviderProfile.endpoints (sandbox.v1.NetworkEndpoint) carries nested maps: graphql_persisted_queries, L7Allow.query/params and L7DenyRule.query/params. Whether profile validation lets endpoints carry them was not checked; it affects scope, not validity. Profiles vended by interceptor sources go through the same :498 path.
   • Existing tests. No test uses a profile with more than one map entry, so the gap is untested.
   • Duplicates. None found. bug(supervisor): accept distinct provider environment revisions in process sidecar #2847 (closed) also involved provider_env_revision, but had a different cause: the process watcher treated the digest as a monotonic counter.

   Impact Signals

   • Affected users/scope: Any sandbox bound to a provider whose profile has two or more entries in any map field (annotations today, possibly nested endpoint maps). Static provider credentials are revoked on roughly half of settings polls, endpoint status reports fail, and startup can fail to stabilize. It fails closed: credentials are withheld, not exposed.
   • Regression: Unknown. The hashing is present in v0.1.0 and on main; no earlier known-good version was identified.
   • Workaround: Available but limiting: keep at most one entry in every map field of provider profiles.
   • Evidence quality: High. The code path was traced and the nondeterministic encoding was reproduced with a unit-level test.

   Candidate fix directions for whoever plans the work, not a plan: configure btree_map for the affected map paths in openshell-core/build.rs, which changes generated Rust types across crates and the Rust SDK, or canonicalize profile maps before hashing, reusing the sorted-map approach in policy_identity.rs. With either, a regression test with several annotations should assert that revisions are equal across decodes. Revision values will change once on upgrade, which triggers one benign provider-env refresh.

   Human Decision Required

   Decide whether OpenShell should address this issue. If yes, apply
   state:accepted, associate it with a roadmap item, or do both, and decide
   whether the work remains human-owned. Either action records acceptance;
   roadmap placement additionally records sequencing.
   To queue investigation or planning for an unattended agent, also apply
   agent:plan-requested. You can instead directly ask an agent to use
   create-spike or build-from-issue on this issue; the agent will warn about
   missing expected workflow labels and continue without changing them. If no,
   close it as not planned and record the rationale.

   Suggested labels (not applied, because the triaging account lacks triage permission): area:gateway, area:providers; replace state:triage-needed with state:validated.
3. letv1nnn commented on Sep 30, 2026

   

   letv1nnn

   on Sep 30, 2026

   Contributor

   More actions

   hey @rain-sicoreai, I'd like to take this one
4. sauagarwa

   added a commit that references this issue on Sep 30, 2026

   Address review of the APF engine switch; adapt to OpenShell 0.1.2

   ...

   59a9492
5. ericcurtin commented on Oct 1, 2026

   

   ericcurtin

   on Oct 1, 2026

   Contributor

   More actions

   PR up: #4077. Happy to defer if you are already on it, @letv1nnn.
6. drew

   closed this as completedin #4122on Oct 2, 2026