Repository navigation
fix(sandbox): reserve gateway-internal Secrets from session secret_refs - #675
Merged
Merged
Conversation
xiaods
commented
Oct 27, 2026
xiaods
commented
Owner
CreateSession's validateSecretRefs only validated ref shape (count, required fields, env-var name, duplicates), never the target Secret's name. Any authenticated caller could pass the gateway's own key ring as an ordinary secret_ref (--secret KEY=sandbox-apikeys:keys.json); resolveSessionEnv then read it with the gateway SA's RBAC and injected the whole keys.json map into the sandbox env, where a trivial printenv returned the entire API key ring via Exec stdout — an authenticated caller escalating from "use a sandbox" to "read the gateway's credential store." Add a name-based gate at both layers: - env.go: reservedSessionSecretNames + isSessionSecretRefAllowed() fails closed on empty names, on the reserved sandbox-apikeys Secret (case-insensitive), and on the reserved ring key "keys.json" under any Secret name. validateSecretRefs rejects at CreateSession with InvalidArgument. - server.go: readSecretKey re-checks the same gate before the K8s Get, a fail-closed backstop for sessions that persisted a forbidden ref before the gate existed (upgrades). The gate works by exclusion, so caller-managed Secrets (e.g. --secret OPENAI_API_KEY=my-llm-secret:api_key) stay referenceable — no allowlist of operator secrets to maintain or drift on. Regression tests (verified to fail without the fix): CreateSession-level rejection incl. the ring key under a caller-managed Secret name; exec-time refusal with the ring seeded so only the gate can refuse (plus a caller-managed Secret still resolving); and a legacy pre-gate session CRD written directly, asserting resolution errors, yields nil env, and never echoes ring material in the error. Also documents the reservation in the SecretRef proto contract (sandbox.pb.go regenerated with the repo's documented protoc invocation — diff is the 4 comment lines only), the KIP-12 boundary table and sensitive-value-hygiene section, and the SKILL.md security red lines.
|
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.