Add mode:"mask" for env vars — sentinel in sandbox, real value injected at the proxy#318
Draft
elhajjj wants to merge 11 commits into
Draft
Add mode:"mask" for env vars — sentinel in sandbox, real value injected at the proxy#318elhajjj wants to merge 11 commits into
elhajjj wants to merge 11 commits into
Conversation
…an optional narrowing When neither per-entry nor block-level injectHosts is set, a masked credential now defaults to network.allowedDomains (injectable at every host the sandbox can reach) instead of failing validation. injectHosts becomes an optional narrowing on top of the network allowlist rather than a required allowlist of its own. Trade-off: a credential is injectable to every reachable host unless explicitly narrowed. Configs that want a credential confined to a subset of allowedDomains must say so. An explicit empty injectHosts (per-entry or block-level inherited by a masked entry) is still rejected: mask-but-never-inject is contradictory and almost certainly a config mistake. The subset-of-allowedDomains check on any explicitly-set injectHosts and the tlsTerminate requirement are unchanged.
…ns default injectHosts now lives only on each masked env-var entry. With no per-entry list, the credential is injected at every host in network.allowedDomains (injectHosts is an optional narrowing). The credentials block is .strict() so a stale block-level injectHosts is rejected rather than silently stripped — that would otherwise widen the credential to every reachable host without the operator noticing.
…ns check The literal Set.has() check rejected injectHosts: ['api.github.com'] when allowedDomains: ['*.github.com'], even though api.github.com is reachable. The check now asks whether every host an injectHosts entry could match is reachable via allowedDomains: exact entries match against allowed patterns; a wildcard entry *.X requires an allowed wildcard *.Y with Y == X or Y an ancestor of X (an exact allowedDomain can never cover a wildcard). matchesDomainPattern moves from sandbox-manager to a new domain-pattern module so the schema can use it without a circular import.
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.
Problem
With
mode: "deny"(#289), the sandboxed process can't read a credential at all — which means tools that need to authenticate (e.g.gh,aws) simply fail inside the sandbox. The useful primitive is: the sandbox sees a fake, and the trusted proxy substitutes the real value on egress to declared destinations — so tools work end-to-end without the sandbox ever holding the real secret.What this adds
Env vars only; files come in a follow-up.
mode: "mask"forcredentials.envVars. The sandbox seesfake_value_<uuid>; a per-session in-memory sentinel registry inSandboxManagerholds the fake↔real map (never on disk, never logged).injectHosts(optional narrowing): the proxy only swaps a credential's sentinel for its real value when the destination matches that credential'sinjectHosts. If unset, defaults tonetwork.allowedDomains— every reachable host. Each entry is validated as semantically reachable (wildcard-aware).MutateForwardedHeaders) threaded through the proxy; header injection runs on the TLS-terminated path only by default. The plaintext path passes the sentinel through unchanged (fails closed) unlesscredentials.allowPlaintextInject: true.network.tlsTerminateis required when masking (orallowPlaintextInjectas an explicit escape hatch) — substitution only happens where the proxy can see and re-encrypt the request.Config example
{ "network": { "allowedDomains": ["api.github.com", "registry.npmjs.org"], "deniedDomains": [], "tlsTerminate": {} }, "filesystem": { "denyRead": [], "allowWrite": ["."], "denyWrite": [] }, "credentials": { "envVars": [ { "name": "GH_TOKEN", "mode": "mask", "injectHosts": ["api.github.com"] }, { "name": "NPM_TOKEN", "mode": "mask" } ] } }GH_TOKEN's sentinel is swapped only atapi.github.com;NPM_TOKEN(noinjectHosts) defaults to everyallowedDomain. Sending one credential's sentinel to the other's host passes the fake through unchanged.Transport safety
Substitution runs only after the allow decision and only on the TLS-terminated path (
forwardUpstream);httpsRequestuses the defaultrejectUnauthorized: true, so a cert-verification failure means the substituted headers never leave the host. The SOCKS path and non-TLS CONNECT are opaque tunnels — never substituted. No proxy log line emits header values. The real value never appears in the wrapped command (visible tops); only the sentinel does.Builds on #289
Extends the
credentialsconfig block introduced in #289 (now merged). This branch was stacked on188f9a1(the head of #289), so the diff here is exactly the masking layer on top ofmode: "deny".Evidence it works
test/sandbox/credential-mask.test.ts(new file) + 14 intest/config-validation.test.ts.test/sandbox/credential-deny.test.tsis unchanged but kept green as a regression check.npx tsc --noEmitand eslint on all changed files: clean.injectHosts, and the sentinel when it doesn't (see "How to test" below for the exact run).Notes for reviewers
src/sandbox/domain-pattern.ts—matchesDomainPatternmoved out ofsandbox-manager.tsso the schema validator can use the same wildcard semantics for theinjectHosts ⊆ allowedDomainscheck.superRefinewrapper onSandboxRuntimeConfigSchemare-indented ~70 unchanged lines insandbox-config.ts— view the diff with whitespace hidden.updateConfigcan't enable masking mid-session ifinitialize()ran without acredentialsblock (the proxy'smutateHeadersseam is bound at init, like other proxy options). Adding masking at runtime requiresreset()+initialize().SandboxManager-level e2e tests useallowPlaintextInjectbecause there's no test-CA seam at the manager level; the TLS path is fully tested at thecreateHttpProxyServerlevel (real curl → CONNECT → terminate → real HTTPS upstream).injectHosts: ['*.github.com']) are accepted — the credential goes to any matching subdomain. Per-credential, so it's a conscious per-token choice; flag if exact-only is preferred.mode: "mask"(follow-up); body-carried credentials aren't substituted (header values only).How to test (Linux)
Drives the
srtCLI directly: a real bwrap sandbox, the srt-managed HTTP proxy, and a tiny local upstream that records what it received. UsesallowPlaintextInjectso a plain-HTTP upstream works without a test CA.Setup
1. Sandbox sees sentinel, not the real value
Expected:
The real value (
$GH_REAL) never appears in$OUT.2. No
injectHosts→ defaults toallowedDomainsExpected:
The upstream received the real value; no
fake_value_in the log line.3. Per-credential
injectHostsnarrowsExpected:
Host A (in
injectHosts) got the real value; host B (reachable but not ininjectHosts) got the sentinel — fails closed.4. Anti-laundering: sentinel A sent to credential B's host stays fake
Expected:
GH_TOKEN's sentinel sent to NPM_TOKEN's host is not swapped; NPM_TOKEN's own sentinel at its own host is.
5. Block-level
credentials.injectHostsis rejectedExpected:
rc≠0,Could not load settingswithUnrecognized key;SHOULD-NOT-RUNnever prints. Thecredentialsblock is.strict()so a stale block-levelinjectHostsis refused rather than silently widening every credential.6. Explicit per-entry
injectHosts: []is rejectedExpected:
rc≠0, error containsmasked but never injected;SHOULD-NOT-RUNnever prints.7. Masking requires
tlsTerminateorallowPlaintextInjectExpected:
rc≠0, error containsrequires network.tlsTerminate;SHOULD-NOT-RUNnever prints.How to test (macOS)
Same as the Linux section — only
SRT_DIRdiffers (point it at your checkout).sandbox-execreplaces bwrap automatically; the upstream,incurl, and all seven scenarios are unchanged.On macOS you can additionally exercise the real TLS path (without
allowPlaintextInject) by adding"tlsTerminate": {}to thenetworkblock and trusting srt's ephemeral MITM CA in your keychain for the duration of the test. The substitution then runs inforwardUpstreamafter CONNECT termination instead of on the plaintext path.