feat(net): credential injection primitives — inject an auth secret in the proxy (RFC #66)#127
feat(net): credential injection primitives — inject an auth secret in the proxy (RFC #66)#127dzerik wants to merge 2 commits into
Conversation
… the proxy (RFC multikernel#66) Phase 1, Layer 2 (transparent mode): a named secret lives only in the supervisor and is rendered into a matching request's auth header (or query parameter) inside the MITM proxy — after the ACL check, so a denied request never touches the secret — while the sandboxed child never sees the real value. New `credential` module: - `SecretString`: zeroed on drop (volatile writes), redacted `Debug`, and deliberately no `Display`/`ToString`/`Serialize` and unreachable through any `Error` chain, so a stray `format!("{}", ..)` can't re-open the leak. - Sources `env:`/`file:`/`fd:` (a trailing newline is stripped); `literal:` is rejected — it leaks via ps / shell history. `file:`/`fd:` are capped at 64 KiB and `fd:` refuses the std streams (0/1/2) and reads through a dup. - `AuthShape` renderings: `bearer`, `basic:<user>` (base64), `header:<name>` / `apikey:<name>`, `query:<param>`. Injected headers are marked sensitive; a secret that can't render (e.g. CRLF) fails the request rather than forwarding it uncredentialed. - `OnExistingHeader` defaults to `Replace`: the proxy owns the credential, and SDKs (openai, anthropic, …) always send a placeholder `Authorization`, so keeping it would forward the placeholder and never inject. Pass the trailing `add-only` token to keep a value the child set instead. Wiring: `--credential NAME=SOURCE` and `--http-inject "METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]"` flow through the builder to a resolved `Arc<Vec<InjectRule>>` on the sandbox (never serialized — `#[serde(skip)]`; secrets are re-loaded from source per build, not persisted). `AclService` applies the first matching rule to the outbound request after `http_acl_check` and records the credential *name* — never its value. `--http-inject` requires an HTTP ACL proxy (rejected otherwise); with no CA it warns that only plaintext HTTP is intercepted. A checkpoint is rejected while injection is configured (the non-serialized secrets can't be restored into the image). Env-sourced secrets are stripped from the child's environment post-fork, so the agent can't read the real value straight out of its own env instead of relying on proxy-side injection. Deferred to later slices: `--service openai` (Layer 1), phantom-token env swap, explicit proxy mode, body-level injection, and a structured audit log (the current audit is a by-name stderr line). Tests: unit coverage of `SecretString` redaction, source loading (incl. `literal:`/std-fd rejection and the 64 KiB cap), every `AuthShape`, the `Replace` default vs `add-only`, query encoding, a CRLF secret failing the request, and rule parsing incl. Arc dedup + env-strip collection; integration tests that a credential is injected into the upstream while the child carries none, that a denied request never renders the secret, and that an `env:` credential is scrubbed from the child. HTTPS/MITM injection is exercised by the same scheme-agnostic path but a TLS end-to-end test is a follow-up.
dzerik
force-pushed
the
feat/credential-injection
branch
from
c8db97b to
a0eeac8
Compare
congwang-mk
left a comment
congwang-mk
left a comment
There was a problem hiding this comment.
Design looks solid overall: ACL-before-inject is structural, SecretString containment is well closed, and reusing HttpRule keeps one matching semantics. Four inline findings below. Two smaller notes not worth inline threads: (1) with a CA configured, a rule matching a port-80 host still injects into cleartext with no warning (the build-time warning only fires when no CA is set); (2) refusing fd:0 blocks the common stdin secret pattern (docker/systemd pipes), consider allowing it or documenting the <(...) workaround.
| // Remove env vars whose value was loaded into the supervisor as a credential | ||
| // source, so the agent can't read the real secret straight from its own | ||
| // environment (this is the child; the mutation is process-local and post-fork). | ||
| for name in &sandbox.inject_env_strip { |
There was a problem hiding this comment.
This strip runs after the sandbox.env set loop, so it also deletes an explicitly passed placeholder (--env OPENAI_API_KEY=dummy). SDKs refuse to start without the var set, which is exactly the placeholder flow the Replace default is designed for, so the canonical env: workflow breaks before any request is made. Strip the inherited env first, then apply sandbox.env: the inherited real secret is still removed and deliberate user config is honored.
| let uri = &parts.uri; | ||
| let path = uri.path(); | ||
| let existing = uri.query(); | ||
| // Honor AddOnly: don't append a param the request already carries. |
There was a problem hiding this comment.
Replace does not replace for the query shape: the existing param is only checked under AddOnly, so the default path appends a duplicate (key=child&key=sk-real, as the test pins). Most frameworks read the first occurrence, so the upstream authenticates against the child's placeholder and returns 401; it is also parameter-pollution ambiguity. On Replace, filter out existing param= pairs before appending, mirroring headers.insert.
| // `apikey:<header>` and `header:<name>` are the same rendering. | ||
| ("header" | "apikey", Some(name)) if !name.is_empty() => AuthShape::Header { name: name.to_string() }, | ||
| ("query", Some(param)) if !param.is_empty() => AuthShape::Query { param: param.to_string() }, | ||
| _ => { |
There was a problem hiding this comment.
A username containing : is accepted here (basic:a:b yields username a:b), which shifts the credential boundary in the encoded user:pass: the upstream sees user a, password b:<secret>. RFC 7617 forbids : in the user-id; reject it at parse time.
| pub fn parse_auth(spec: &str, credential: &str) -> Result<AuthShape, SandboxError> { | ||
| let (kind, arg) = match spec.split_once(':') { | ||
| Some((k, a)) => (k, Some(a)), | ||
| None => (spec, None), |
There was a problem hiding this comment.
Stale doc: says it returns (shape, credential_name) but the function returns only the AuthShape.
- context.rs: strip inherited credential env vars *before* applying `sandbox.env`, so a deliberate placeholder (`--env OPENAI_API_KEY=dummy`, which SDKs need set to start) survives while the inherited real secret is still removed. Previously the strip ran after and deleted the placeholder, breaking the canonical `env:` flow before any request. - credential.rs: `Replace` now actually replaces for the query shape — existing occurrences of the target param are dropped before appending, instead of producing a duplicate (`key=child&key=sk-real`) where a framework reading the first occurrence would authenticate against the child's value. The test is corrected to assert replacement, not the duplicate it pinned. - credential.rs: reject ':' in a `basic:<user>` user-id (RFC 7617) — it would shift the `user:pass` boundary and leak part of the secret into the password. - credential.rs: drop the stale `parse_auth` doc line claiming a tuple return.
2 participants
Implements RFC #66, Phase 1, Layer 2 (transparent mode). This is the bottom-up primitive slice — the generic mechanism, not the
--service openaipresets (those are a later slice).What it does
A named secret lives only in the supervisor. When the sandboxed child makes a request to a matching upstream, the secret is rendered into an auth header (or query parameter) inside the MITM proxy — after
http_acl_check, so a denied request never touches the secret — and the child never sees the real value.Design
SecretStringis zeroed on drop (volatile writes), has a redactedDebug, and deliberately implements noDisplay/ToString/serde::Serializeand is unreachable through anyErrorchain — so a strayformat!("{}", ..)or a serialized policy can't re-open the leak the type exists to close. It flows to the proxy asArc<Vec<InjectRule>>and is#[serde(skip)]on the sandbox (re-loaded from source per build, never persisted).AclService::handleapplies the first matching rule only after the ACL passes; the deny path returns before any rendering. Audit records the credential name, never the value.OnExistingHeaderdefaults toReplace. The proxy owns the credential, and every major SDK requires an API key to be set and always sends its own placeholderAuthorization. Keeping it would forward the placeholder and never inject — so the rule's target header is replaced by default; pass a trailingadd-onlyto keep a child-set value instead.env:/file:/fd:(a trailing newline is stripped);literal:is rejected (leaks via ps / shell history).file:/fd:are capped at 64 KiB;fd:refuses the std streams and reads through adupso the caller's fd stays open. Env-sourced secrets are stripped from the child's environment post-fork, so the agent can't read the value straight out of its own env.Safety rails
--http-injectwithout an ACL proxy is rejected; without a CA it warns that only plaintext HTTP is intercepted (a rule for an HTTPS host would silently no-op otherwise).Auth shapes
bearer·basic:<user>(base64) ·header:<name>/apikey:<name>·query:<param>(raw bytes percent-encoded, so a binary key isn't corrupted).Tests
SecretStringredaction, source loading (incl.literal:/std-fd rejection and the 64 KiB cap), everyAuthShape, theReplacedefault vsadd-only, queryAddOnlyde-dup, a CRLF secret failing the request, andresolve_inject_rulesArc de-dup + env-strip collection.env:credential is scrubbed from the child.applypath; a TLS end-to-end test needs a CA-trust harness and is tracked as a follow-up (noted in the test file).Deferred to later slices
--service openai(Layer 1) presets, phantom-token env swap, explicit proxy mode, body-level injection, and a structured audit log (the current audit is a by-name stderr line).