feat(proxy): redact upstream responses inside the proxy - #9
Merged
Merged
Conversation
The proxy's response path holds the bytes itself — hyper hands it owned frames — so neither single-shot `redact_bytes` nor the blocking `redact_stream` fits: the first would have to buffer a whole body, and the second would cost a thread and a pair of channels per response. `StreamRedactor` keeps the bytes a pattern could still be starting in, which is never more than the longest pattern, and hands back everything that is settled. Because the automaton reports a match at the position its last byte lands on, nothing found so far can be undone by later input, and the split is placed so it never falls inside a match — so the concatenated output is what `redact_bytes` makes of the whole input, for any chunking. The tests assert exactly that over every split point and every fixed chunk size.
Until now the response was forwarded to the tool untouched, and the only redaction on that path was the daemon's stdout pipeline. `curl -o file`, `--dump-header` and `--trace` walk around it: they write the bytes into the sandbox root, where the agent reads them back. An upstream that reflects the injected credential — an echo endpoint, a debug page, a `Location` built from the token — therefore leaked it past the project's "redaction is mandatory on the output path" invariant. Every header value and every body byte now goes through the same automaton stdout goes through, streaming, with nothing buffered beyond the partial match at the end of a frame. The redactor is taken per response from the daemon's live handle rather than snapshotted when the session starts: a tool runs for minutes, the proxy injects whatever the store holds *now*, and a session snapshot would not know a token minted after the exec began. Three things are made to hold rather than handled. The request is rewritten to demand `Accept-Encoding: identity` and stripped of `Range`/`If-Range`, because a compressed body is opaque to a byte scanner and a range may begin in the middle of a secret. An upstream that answers with a content coding, an unknown transfer coding or a partial representation anyway is refused with 502 and its body dropped unread. And the upstream `Content-Length` is dropped whenever there is a body, since a placeholder is not the length of what it replaced — hyper frames the response as chunked instead. A bodiless response (HEAD, 1xx, 204, 304) keeps its length, which describes the representation rather than bytes on the wire.
SECURITY.md gains a Response redaction section and loses the `-o` residual risk, which is now closed for response bytes; what remains in its place is the limit that has always applied on the stdout path — an upstream that reflects the secret *transformed* is not caught, and the agent's own request data was never covered. The design record moves response redaction from open to landed and keeps the reasoning for the three fail-closed cases next to the decision. SKILL.md tells an agent what it will actually observe: redacted bytes in a downloaded file, no compressed transfer, no resumable downloads.
Two `Content-Encoding` header lines are one list, so an upstream that answers `identity` followed by `gzip` would have passed a check that only looked at the first value and handed the tool bytes the redactor cannot read. The transfer-coding check next to it already iterated all of them.
hyper's client keeps a non-canonical reason phrase in the response extensions, and hyper's server writes that extension back out. The response was rebuilt from the upstream's parts, extensions included, so an upstream answering `HTTP/1.1 200 <token>` put the token on the tool's side of the proxy without it ever passing the redactor, which only walks header values and body frames. `curl -v` and `--trace` would have written it to a file. The extensions are cleared before the response is rebuilt; nothing in them is needed downstream. [tests.rs](src/proxy/server/tests.rs) has the regression test, which failed before this change.
paveq
added this pull request to stack #10
September 17, 2026 12:02
paveq
removed this pull request from stack #10
September 23, 2026 06:54
paveq
marked this pull request as ready for review
September 23, 2026 07:03
paveq
added a commit
that referenced
this pull request
Sep 23, 2026
* feat(redact): add an incremental redactor for chunk-wise streams The proxy's response path holds the bytes itself — hyper hands it owned frames — so neither single-shot `redact_bytes` nor the blocking `redact_stream` fits: the first would have to buffer a whole body, and the second would cost a thread and a pair of channels per response. `StreamRedactor` keeps the bytes a pattern could still be starting in, which is never more than the longest pattern, and hands back everything that is settled. Because the automaton reports a match at the position its last byte lands on, nothing found so far can be undone by later input, and the split is placed so it never falls inside a match — so the concatenated output is what `redact_bytes` makes of the whole input, for any chunking. The tests assert exactly that over every split point and every fixed chunk size. * feat(proxy): redact upstream responses before they reach the tool Until now the response was forwarded to the tool untouched, and the only redaction on that path was the daemon's stdout pipeline. `curl -o file`, `--dump-header` and `--trace` walk around it: they write the bytes into the sandbox root, where the agent reads them back. An upstream that reflects the injected credential — an echo endpoint, a debug page, a `Location` built from the token — therefore leaked it past the project's "redaction is mandatory on the output path" invariant. Every header value and every body byte now goes through the same automaton stdout goes through, streaming, with nothing buffered beyond the partial match at the end of a frame. The redactor is taken per response from the daemon's live handle rather than snapshotted when the session starts: a tool runs for minutes, the proxy injects whatever the store holds *now*, and a session snapshot would not know a token minted after the exec began. Three things are made to hold rather than handled. The request is rewritten to demand `Accept-Encoding: identity` and stripped of `Range`/`If-Range`, because a compressed body is opaque to a byte scanner and a range may begin in the middle of a secret. An upstream that answers with a content coding, an unknown transfer coding or a partial representation anyway is refused with 502 and its body dropped unread. And the upstream `Content-Length` is dropped whenever there is a body, since a placeholder is not the length of what it replaced — hyper frames the response as chunked instead. A bodiless response (HEAD, 1xx, 204, 304) keeps its length, which describes the representation rather than bytes on the wire. * docs: record in-proxy response redaction SECURITY.md gains a Response redaction section and loses the `-o` residual risk, which is now closed for response bytes; what remains in its place is the limit that has always applied on the stdout path — an upstream that reflects the secret *transformed* is not caught, and the agent's own request data was never covered. The design record moves response redaction from open to landed and keeps the reasoning for the three fail-closed cases next to the decision. SKILL.md tells an agent what it will actually observe: redacted bytes in a downloaded file, no compressed transfer, no resumable downloads. * fix(proxy): check every Content-Encoding line, not just the first Two `Content-Encoding` header lines are one list, so an upstream that answers `identity` followed by `gzip` would have passed a check that only looked at the first value and handed the tool bytes the redactor cannot read. The transfer-coding check next to it already iterated all of them. * fix(proxy): do not forward the upstream's reason phrase hyper's client keeps a non-canonical reason phrase in the response extensions, and hyper's server writes that extension back out. The response was rebuilt from the upstream's parts, extensions included, so an upstream answering `HTTP/1.1 200 <token>` put the token on the tool's side of the proxy without it ever passing the redactor, which only walks header values and body frames. `curl -v` and `--trace` would have written it to a file. The extensions are cleared before the response is rebuilt; nothing in them is needed downstream. [tests.rs](src/proxy/server/tests.rs) has the regression test, which failed before this change.
paveq
added a commit
that referenced
this pull request
Sep 23, 2026
…gress proxy for general HTTP clients (#8) * feat(config): add proxy-tool schema and design for credential-injecting egress proxy Not every API has a purpose-built CLI (most of the GCP REST surface is out of gcloud's reach), but SECURITY.md rightly forbids handing curl a secret: the agent controls its arguments, and curl can ship its own environment anywhere. Restricting where curl may connect does not fix that, because allowed API hosts are multi-tenant and curl >= 8.3 can interpolate env vars by itself (--variable / --expand-url), macOS included. Proxy tools invert the model: the tool never holds a secret, and a daemon-side intercepting proxy attaches the credential after the request has left the tool, for operator-approved hosts only. The full design, including what is taken from and changed relative to claw-wrap, is in [proxy-tools-design.md](docs/proxy-tools-design.md). This commit lands the declarative half only: - [proxy.rs](src/proxy.rs): route table, host and METHOD /path matching. Deny by default; paths an upstream might normalize differently (.., //, %2F) are refused rather than guessed at. - [config.rs](src/config.rs): `proxy = true` + `[[tools.X.routes]]`. A proxy tool may not reference secrets in env nor set the proxy / CA variables the daemon will own. - [daemon.rs](src/daemon.rs): refuses to exec a proxy tool. There is no runtime yet, and spawning one the ordinary way would give it open network with no enforcement, which is the exact tool SECURITY.md bans. SECURITY.md also loses the claim that curl has no env interpolation. * refactor(sandbox): replace requires_network with a three-way NetworkAccess A proxy tool needs a third network state that a bool cannot express: reach the daemon's per-exec proxy listener and nothing else. `NetworkAccess::{None, Full, ProxyOnly(port)}` carries the bound port down to the backend, which is the only thing that makes the egress rule per-exec rather than global. On macOS `ProxyOnly` emits `(allow network-outbound (remote tcp "localhost:P"))` alone — no blanket `network-outbound`, no mDNSResponder socket, no unix-socket bind. Verified with `sandbox-exec` that a tool under that profile reaches port P, cannot reach a public IP, cannot resolve a name, and cannot reach a different loopback port even when something is listening there. Seatbelt rejects an IP literal in `remote tcp`; `localhost` is the only form that compiles and it covers the loopback interface the listener binds to. On Linux `ProxyOnly` adds the ABI v4 network rights under the existing `HardRequirement` compat level and grants `ConnectTcp` on that one port, so a kernel older than 6.7 fails the exec instead of running the tool unpinned. `None` and `Full` leave network rights unhandled, keeping every existing tool byte-identical to before. AgentPolicy keeps its `requires_network: bool` — an agent has exactly two states and always takes the second one. * feat(proxy): implement the interception proxy runtime The route model landed in phase 0 with nothing behind it. This is the runtime: a per-exec listener on an ephemeral loopback port that terminates TLS, vets each request against the tool's routes, attaches the credential, and forwards upstream over a verified connection. Why the credential is attached here and not in the tool's environment: a tool whose arguments the agent controls can read its own environment (`curl --variable %NAME`), write files the agent reads back, and upload anything it can read to a multi-tenant host that is on the allowlist. None of that matters if the tool never holds the secret, so the whole design rests on attaching it after the request has left the tool. Egress pinning is the second layer, not the first. Shape of the request path, kept small enough to audit: - `vet_request` is pure — request head, CONNECT authority, route in; a status and a reason out. Host must equal the authority (no domain fronting), the target must be origin-form, `Transfer-Encoding` with `Content-Length` and a duplicated `Content-Length` are refused, and the route decides on method and path with the query string excluded from matching. - The CONNECT authority is the single source of truth: it selects the route, names the leaf the tool is shown, is the name resolved and dialled, and is the name the upstream certificate is verified against. The client's SNI is never read, so `--resolve`, `--connect-to` and a forged `Host` cannot make any two of those disagree. - Every client-supplied copy of the injected header is removed before the credential goes on, so the upstream can never see two. - The secret is looked up per request — a refresh applies to the next one, a `Stale` slot fails this one with 502 — and assembled into a buffer that is zeroized, never through `format!`. - Addresses are vetted on the concrete `SocketAddr` that is then dialled, with no second lookup for a rebinding answer to slip into. Proxy auth is mandatory rather than optional: the daemon's trust boundary is a 0700 Unix socket, but a loopback TCP port has no file mode, so without a per-exec token another local user could race an exec and have the daemon sign their requests. The CA is generated once per daemon inside the async runtime, so the synchronous-startup invariant is untouched. The key never reaches disk, the certificate is `CA:TRUE, pathlen:0` and carries Name Constraints limited to the union of routed DNS names, and the leaf cache is capped because a wildcard route makes the host key space agent-controlled. Bounds on everything the peer drives: header read timeout and read-buffer cap via hyper, handshake and connect timeouts, and a cap on concurrent tunnels per exec. One task per connection, so a panic in the request path cannot reach the daemon. `Upstream::fixed` is `cfg(test)` only, which is what lets the tests point a route at a local rustls server without a production code path that can be told to skip the SSRF filter. Those tests include Apple's system curl (8.7.1, SecureTransport/LibreSSL) driven only by the environment the daemon sets: it honours `CURL_CA_BUNDLE` for an intercepted connection and accepts a leaf issued by the name-constrained CA, which settles the open question in docs/proxy-tools-design.md. * feat(daemon): run proxy tools through the per-exec proxy Replaces the fail-closed guard with the real flow. The exec handler now binds a proxy listener before building the sandbox profile — the port has to exist before the profile can name it — overlays the daemon-owned proxy and CA-bundle variables onto the child environment after the tool's own `env`, and sets `NetworkAccess::ProxyOnly` so the sandbox pins egress to that port. The session is held in a local for the rest of the handler. Dropping it aborts the serve task and closes the listener, so the proxy comes down with the child on every path out — normal exit, timeout, kill, client disconnect, and each early `return` in the validation sequence — without any of them having to remember to tear it down. Everything else about an exec is unchanged: stdout and stderr still go through the redactor, the timeout and child registry still apply, and a tool with no `proxy` key takes exactly the path it took before. The CA is created once per daemon in `async_main` (and its embedded twin) and shared as an `Arc`. It is built there rather than in `synchronous_startup` because key generation must not precede the fork. `Config` grows a `ca_path` derived beside `socket_path` and `pid_path`; the certificate is published there when at least one proxy tool exists, removed at graceful shutdown, and swept with the socket and PID file when a dead daemon's state is cleaned up. * test(proxy): cover the daemon-owned half of a proxy exec end to end Through a real daemon and the system curl: the CA certificate's lifecycle (published when a proxy tool exists, absent when none does, swept as stale state, removed at shutdown), the proxy's refusal of an unrouted host, and — the one that matters — that `curl --noproxy '*'` fails at the sandbox rather than at the environment. The environment is guidance; the profile is enforcement, and this asserts the difference. A completed request is deliberately not tested here. The proxy resolves the upstream itself and refuses any address that is not globally routable, so a local test server could only stand in as the upstream via a switch that turns the SSRF filter off — the switch that must not exist in a shipped binary. That path is covered in src/proxy/server.rs instead, where a cfg(test) connector can point a route at a local rustls server. * docs: describe proxy tools as a shipped feature Phase 0 left every document saying the runtime did not exist. Bringing them back in line with the code, each for its own audience: - SECURITY.md gains the real "Proxy tools" section: the invariant, what the daemon does per execution, the full request-handling table, the CA, and the residual risks the design doc listed as things to document once the runtime landed. The curl ban narrows from "never" to "never with secrets in its environment; only as a proxy tool", and the per-platform sandbox sections describe the three network states rather than a boolean. - README.md documents `proxy` / `routes` without the "schema only" caveat and shows the GCP impersonated-token config end to end. - SKILL.md tells the agent the four things it actually needs: use ordinary https URLs, do not pass auth headers, only the listed hosts exist, and a 403 is policy — retrying with --noproxy or -k makes it worse, not better. - ARCHITECTURE.md adds the two new modules, the proxy steps in the exec flow, a CONNECT-path diagram, and the new crates. - The design doc becomes a design record rather than a proposal, and its Apple curl question is answered: system curl 8.7.1 (SecureTransport/LibreSSL) honours CURL_CA_BUNDLE for an intercepted connection and accepts a leaf from the name-constrained CA, so no Homebrew curl requirement and no softening of the constraint. Phase 2 is marked written but unverified on a Linux host. CLAUDE.md's lib test count was also three releases stale, and now warns that `cargo test` wants `< /dev/null` — some client tests read the real stdin and hang on an inherited pipe that never closes. * fix(proxy): end connections and tunnels with the session, not just the listener ProxySession's drop aborted the accept task only. Connections and CONNECT tunnels run as tasks of their own, so an established tunnel went on attaching credentials after the exec that owned it had ended. The child and its process group are killed on every exit path, but a descendant that left the group (setsid) would have kept a working, credential-signing tunnel past the exec timeout. A cancellation token owned by the session now reaches every task under it; [tests.rs](src/proxy/server/tests.rs) has the regression test, which failed before this change. Two smaller hardenings in the same path: the CONNECT authority is reduced to one canonical spelling before it names a leaf certificate and keys the leaf cache, and the deprecated IPv4-compatible range (::/96) joins the addresses the upstream dialer refuses, so `::10.0.0.1` is treated like `10.0.0.1`. * docs(proxy): drop the stale schema-only decision from the design record The interview decision read 'this PR: design + config schema', which stopped being true once the runtime landed on the same branch. Record why the schema went first instead. * test(proxy): prove the sandbox denies a direct TCP connect without involving DNS The existing bypass test targets a DNS name. On Linux that cannot distinguish a working Landlock TCP rule from a failed name lookup, because Landlock does not cover UDP. An unroutable IP literal can: a sandbox denial fails connect() at once (curl exit 7), while an unsandboxed connect hangs until --max-time (exit 28). * docs(proxy): record that the Landlock network rule is exercised in Linux CI Phase 2 was marked unverified because no Linux toolchain was available where it was written. CI has since run both sides of the rule on a real kernel; what remains unexercised is only the refusal on pre-6.7 kernels. * docs: update lib test count Two proxy tests were added after the count was last refreshed. * feat(proxy): redact upstream responses inside the proxy (#9) * feat(redact): add an incremental redactor for chunk-wise streams The proxy's response path holds the bytes itself — hyper hands it owned frames — so neither single-shot `redact_bytes` nor the blocking `redact_stream` fits: the first would have to buffer a whole body, and the second would cost a thread and a pair of channels per response. `StreamRedactor` keeps the bytes a pattern could still be starting in, which is never more than the longest pattern, and hands back everything that is settled. Because the automaton reports a match at the position its last byte lands on, nothing found so far can be undone by later input, and the split is placed so it never falls inside a match — so the concatenated output is what `redact_bytes` makes of the whole input, for any chunking. The tests assert exactly that over every split point and every fixed chunk size. * feat(proxy): redact upstream responses before they reach the tool Until now the response was forwarded to the tool untouched, and the only redaction on that path was the daemon's stdout pipeline. `curl -o file`, `--dump-header` and `--trace` walk around it: they write the bytes into the sandbox root, where the agent reads them back. An upstream that reflects the injected credential — an echo endpoint, a debug page, a `Location` built from the token — therefore leaked it past the project's "redaction is mandatory on the output path" invariant. Every header value and every body byte now goes through the same automaton stdout goes through, streaming, with nothing buffered beyond the partial match at the end of a frame. The redactor is taken per response from the daemon's live handle rather than snapshotted when the session starts: a tool runs for minutes, the proxy injects whatever the store holds *now*, and a session snapshot would not know a token minted after the exec began. Three things are made to hold rather than handled. The request is rewritten to demand `Accept-Encoding: identity` and stripped of `Range`/`If-Range`, because a compressed body is opaque to a byte scanner and a range may begin in the middle of a secret. An upstream that answers with a content coding, an unknown transfer coding or a partial representation anyway is refused with 502 and its body dropped unread. And the upstream `Content-Length` is dropped whenever there is a body, since a placeholder is not the length of what it replaced — hyper frames the response as chunked instead. A bodiless response (HEAD, 1xx, 204, 304) keeps its length, which describes the representation rather than bytes on the wire. * docs: record in-proxy response redaction SECURITY.md gains a Response redaction section and loses the `-o` residual risk, which is now closed for response bytes; what remains in its place is the limit that has always applied on the stdout path — an upstream that reflects the secret *transformed* is not caught, and the agent's own request data was never covered. The design record moves response redaction from open to landed and keeps the reasoning for the three fail-closed cases next to the decision. SKILL.md tells an agent what it will actually observe: redacted bytes in a downloaded file, no compressed transfer, no resumable downloads. * fix(proxy): check every Content-Encoding line, not just the first Two `Content-Encoding` header lines are one list, so an upstream that answers `identity` followed by `gzip` would have passed a check that only looked at the first value and handed the tool bytes the redactor cannot read. The transfer-coding check next to it already iterated all of them. * fix(proxy): do not forward the upstream's reason phrase hyper's client keeps a non-canonical reason phrase in the response extensions, and hyper's server writes that extension back out. The response was rebuilt from the upstream's parts, extensions included, so an upstream answering `HTTP/1.1 200 <token>` put the token on the tool's side of the proxy without it ever passing the redactor, which only walks header values and body frames. `curl -v` and `--trace` would have written it to a file. The extensions are cleared before the response is rebuilt; nothing in them is needed downstream. [tests.rs](src/proxy/server/tests.rs) has the regression test, which failed before this change. * fix(refresh): swap the redactor in before publishing a refreshed secret `refresh_once` wrote the new value into the store slot and only then rebuilt and swapped the redactor. The proxy reads the slot per request, so a tunneled request landing in that window injected the new secret upstream while the per-response redactor snapshot still knew only the previous generation. An upstream that echoes the Authorization header (a 401 body, a token-info endpoint) would then have returned the fresh credential to the tool in plaintext, straight into `curl -o` files. Rebuilding first costs nothing: the rebuild only needs the new value as an argument, not in the slot. If the rebuild fails the slot is left untouched, so a secret the redactor cannot see is never published at all. * fix(proxy): match route rules against the percent-decoded path Rules were compared byte-for-byte against the request path as sent, with only `%2f`, `%2e`, `%5c` and `%00` refused. Any other escape in a literal segment therefore skipped every rule it should have hit: `DELETE /%72epos/o/n` did not match `deny = ["DELETE /repos/**"]`, was forwarded with the credential attached, and GitHub decoded it to `/repos/o/n`. Allow-lists failed closed, but every deny rule and every allow rule reached through a wildcard was bypassable this way. Segments are now percent-decoded exactly once before matching, which is what the upstream routes on. What still depends on the upstream's own normalization is refused as before, now judged on the decoded bytes: a `.`/`..` or empty segment, or an escape yielding `/`, `\`, NUL, or a second-round `%` (double encoding). Malformed escapes are refused too. Rule literals may no longer contain `%`, so there is one canonical form. * fix(daemon): report startup failures through the readiness pipe Generating and publishing the proxy CA added fallible steps between the socket bind and the readiness byte, on top of the PID-file write that was already there. The daemonize parent ignored the result of its pipe read and always exited 0, so any of those failing made `airlock daemon start` succeed silently: no daemon, a stale socket, and `airlock exec` later failing with connection refused. The grandchild's stdio is /dev/null, so nothing else could have carried the error. The pipe now carries a verdict: a READY byte, or a FAILED byte followed by the error text. The parent prints that text and exits 1, and treats EOF without a verdict as failure too. The write end is owned by a `ReadinessPipe` consumed exactly once, so no code path can write into the descriptor after it has been closed and its number reused. * refactor(daemon): share the proxy CA publish step `run_embedded` and `async_main` carried a byte-identical block for generating the CA and writing its certificate. One helper keeps the two startup paths from drifting. * docs: update lib test count * docs: rewrite proxy tool docs in simple technical English Shorter sentences and plain words make the proxy tool docs easier to read for non-native readers. The rewrite also uses one set of terms (proxy tool, route, allow/deny rules, credential) in all four files. It fixes claims in SKILL.md that did not match the code: Landlock does not block DNS, the proxy only strips the header a route injects, and curl's range options are handled by removing the Range header. README now lists all CA variables the daemon sets, not only CURL_CA_BUNDLE. * docs(proxy): explain how allow and deny rules combine The evaluation order (deny wins, an empty allow permits anything not denied, otherwise a request must match allow) was only written in a code comment and the design doc. README and SECURITY.md now state it. The old example paired a narrow allow list with `deny = ["DELETE /**"]`, which had no effect because DELETE was already outside the allow list. The new example uses a broad allow with a deny exception, so each list does something. * fix(proxy): apply deny rules to trailing-slash and ;param forms A request for `DELETE /v1/secrets/x/` got past `deny = ["DELETE /v1/secrets/*"]`: the trailing empty segment made the rule fail to match, and a broad allow then forwarded it. Express, Django and many gateways serve `/x/` as `/x`. `/v1/admin;x` got past literal deny segments in the same way, because Tomcat and Spring drop `;params`. Deny rules are now checked against every form the upstream may route the path as. Allow rules stay strict and match only the path as sent. A segment that is `.`, `..` or empty after removing `;params` (Tomcat's `..;`) is refused like its plain form. * fix(refresh): serialize redactor rebuilds across refresh tasks Each refresh rebuilt the redactor from every slot, swapped it in, then published its new value. Refresh tasks run in separate blocking threads with nothing ordering them, so B could build its redactor while A's slot still held the old value, and swap it in after A had published. The live redactor then missed the value the proxy was injecting, and an upstream that echoes the credential could return it to the tool in plaintext. Rebuild, swap and publish now run under one mutex that all refresh tasks share. The mutex also guards the previous value of each refreshed secret. Before, a rebuild kept two generations only for the secret being refreshed, so refreshing B dropped A's previous value while requests still in flight could be using it. * fix(proxy): refuse CONNECT with 503 when the tunnel limit is reached The tunnel slot was taken in `run_tunnel`, after the proxy had already answered `200` to the CONNECT. With all 32 slots in use, the tool saw a successful CONNECT followed by a closed connection, which surfaces as a TLS handshake EOF instead of a busy proxy. The slot is now taken in `handle_proxy_request` before the reply. When none is free, the tool gets a `503` it can retry, and the refusal is written to the audit log. * docs: update lib test count * fix(proxy): fail CONNECT with 500 when the leaf certificate cannot be made The leaf certificate was minted in `run_tunnel`, after the proxy had already answered `200`. A minting failure then showed up to the tool as a closed connection during the TLS handshake, and only a ring buffer line explained it. The proxy now mints the certificate before it answers the CONNECT. On failure the tool gets a `500` that says why, and the error is written to the audit log. * docs(proxy): state that the proxy runs in the daemon "Per-exec" and "for the lifetime of one `airlock exec`" read as if the client command ran the proxy. The daemon runs it, one listener for each exec request it handles. * refactor(proxy): store the injected header as a HeaderName `Inject` kept the header as a string, and the proxy parsed it again on every request. If that parse had failed, `strip_forbidden_headers` would have silently kept the tool's own copy of the credential header. It could not fail only because `Inject::parse` had checked the name earlier, in another module. The name is now parsed once at config load. The strip and the insert use it directly and have no error path. `airlock list` prints the name in lowercase, as `HeaderName` stores it. * refactor(proxy): derive reserved env vars from the vars the daemon sets Config kept its own list of env vars a proxy tool may not set, separate from the lists `apply_env` uses. Nothing kept the two in sync, and they had already drifted (`SSL_CERT_DIR` was only in config). A CA variable added to `apply_env` alone would have been settable from config. `is_reserved_env_var` now lives next to the lists in proxy::server and is built from them, plus `SSL_CERT_DIR`, which stays reserved but unset. * refactor(policy): decide a proxy tool's network access in build_tool_policy `build_tool_policy` always returned `NetworkAccess::Full`, and the daemon changed it to `ProxyOnly(port)` afterwards. If a later change dropped that override, a proxy tool would have run with full network access and no error. The caller now passes the proxy port in. A proxy tool without a port, or an ordinary tool with one, is a `ProxyPortMismatch` error instead of full network. * refactor(proxy): share one crypto provider and TLS version list The CA and the upstream client each built their own ring provider and spelled out TLS 1.3 + 1.2 separately. The comment on the CA's copy called it "the single crypto provider this daemon uses", which was not true. Both sides now use `CRYPTO_PROVIDER` and `TLS_VERSIONS` from proxy.rs, so one edit changes the TLS settings for both. * refactor(proxy): vet CONNECT in a pure function and refuse in one place `handle_proxy_request` mixed the CONNECT checks with I/O and wrote out the audit-then-refuse block five times. A malformed CONNECT target was the one refusal that never reached the audit log. `vet_connect` now makes the method, target, port and route decision as a pure function, like `vet_request`, so it is unit-tested without a listener. Every refusal on both the CONNECT and tunnel paths goes through `ProxyContext::refuse`, which audits it and sends the tool the same reason the log records. `Denial.reason` is a `Cow` so dynamic reasons such as the refused port keep their detail. A `ProxyContext::log` helper replaces the repeated `proxy [tool]` prefix formatting. * refactor(proxy): build the daemon-wide proxy state once as ProxyShared `ProxySession::start` and `start_with_upstream` took 7-8 arguments each, and the daemon threaded the CA, CA path, secrets and live redactor through `handle_connection` and `handle_exec_request` separately for every exec. `Upstream::public()` also cloned the webpki roots and built a new rustls client config on every exec, although neither ever changes. `ProxyShared` holds these and is built once in `publish_proxy_ca`. A session now starts from a tool, its policy and `&ProxyShared`. Tests build a `ProxyShared` with a fixed upstream, so `start_with_upstream` is gone. `handle_connection` no longer needs the live redactor or its `too_many_arguments` allow. * refactor(redact): name the live redactor handle RedactorSwap `Arc<RwLock<Arc<Redactor>>>` was written out 12 times across the daemon, refresh and proxy code. The alias sits next to `Redactor` and documents the swap-and-snapshot contract once, as `SecretStore` does for the secret slots. * refactor(refresh): bundle shared refresh state into RefreshShared `refresh_once` took eight arguments and needed a `too_many_arguments` allow once the previous-generation map was added. Every refresh task also cloned the store, redactor handle, generation map and ring buffer separately on each cycle. `RefreshShared` holds those four and is shared by all tasks through one `Arc`. `refresh_once` takes the command and `&RefreshShared`, and the allow is gone. The previous-generation map is now a private field, so only the refresh path can lock it. * refactor(daemon): remove runtime files through one helper The pid, socket and CA files were removed in four places: both branches of `check_and_cleanup_stale_state`, `graceful_shutdown`, and `cleanup_stale_files` in main.rs. Adding the CA file meant touching all of them, and the embedded daemon logged a spurious "failed to remove PID file" at every shutdown because it never writes one. `Config::runtime_files` and `DiscoveredPaths::runtime_files` list the files, and `remove_runtime_files` removes them and reports only real failures, not files that are already gone. `graceful_shutdown` takes the config instead of three copied paths. * docs: update lib test count * refactor(daemon): run both daemon modes through one Daemon start/serve `async_main_inner` and `run_embedded` each converted the listener, published the proxy CA, spawned refresh tasks, ran an accept loop, drained the refresh tasks and shut down, in two copies that had to be kept in step by hand ("identical to async_main"). They now share `Daemon::start` and `Daemon::serve`, and differ only in what is theirs: the PID file and readiness pipe, and whether SIGTERM or the `run` session's cancel signal ends the loop. The per-connection state is one `DaemonShared` behind an `Arc`, so `handle_connection` and `handle_exec_request` take it instead of six separately cloned handles, and `handle_exec_request` no longer needs its `too_many_arguments` allow. * fix(daemon): snapshot the exec redactor after reading the tool's secrets The redactor for a child's stdout and stderr was snapshotted when the connection was accepted, but the tool's secrets are read only once the client sends its exec request. The client chooses when that is. An agent could open a connection, wait until a refresh had replaced the token twice, and then run a tool: the child got the new value, and a redactor that had never seen it passed the value to the client in plaintext. The snapshot is now taken right after the env is built. A refresh swaps the redactor before it publishes the new value, so a snapshot taken after the read knows every value it returned. [redact_e2e_integration.rs](tests/redact_e2e_integration.rs) holds a connection open across two refreshes and failed before this change. * fix(daemon): install the SIGTERM handler before signalling readiness The handler was installed after `daemon start` had been told the daemon was ready, with an `expect`. A failure there panicked in a process with no stderr after the parent had already exited 0, which breaks the rule that every startup failure goes through the readiness pipe. A SIGTERM that arrived in that window (`daemon start && daemon stop`) took the default action and left the socket and PID file behind. * refactor(proxy): let each session hold the daemon's ProxyShared `ProxySession::start` copied the CA, secret store, redactor, ring buffer and upstream out of `ProxyShared` into every session's `ProxyContext`, which was the same set of handles in two structs. The context now holds an `Arc<ProxyShared>`, so the CA needs no `Arc` of its own and `Upstream` no longer has to be `Clone`. * refactor(daemon): split env resolution and output forwarding out of exec `handle_exec_request` built the tool env in an immediately-invoked closure, forwarded stdout and stderr through two copies of the same select arm plus a third copy in `drain_channel_to_client`, and removed the child from the registry in each of the five exit paths. `resolve_tool_env` is now a plain function with unit tests. Until now no test covered the stale-secret refusal. `send_output` forwards one chunk for all three sites, and the registry removal happens once after the match. * chore: clear the last clippy warning and make pattern_count test-only `try_canonicalize` hand-rolled `Option::filter`, and its doc omitted that an already-canonical path also yields `None`. `Redactor::pattern_count` was public API that only tests call. It now sits with the other test-only introspection helpers in the `#[cfg(test)]` impl. * refactor(proxy): keep the reserved env var lists with the proxy policy Config validation reached into `proxy::server`, the hyper/tokio runtime module, for `is_reserved_env_var`. The lists and the check now live in `proxy`, next to the other rules config enforces for a proxy tool (`FORBIDDEN_INJECT_HEADERS`), and the server imports the lists it applies at spawn. Config no longer depends on the server module. * docs: fix rustdoc link warnings Four intra-doc links pointed at private or out-of-scope items; one of them came from the RefreshShared refactor. `cargo doc` is now clean. * refactor(daemon): validate and spawn an exec in one fallible start_tool Steps 1-8 of `handle_exec_request` are synchronous checks, each ending in the same `log_and_send_error(...).await; return;` block, seven times over. They are now `start_tool`, which returns `Result<StartedTool, String>` and uses `?`, so the error is logged and sent in one place and the async handler starts at child registration. The proxy session moves into `StartedTool` and still lives until the handler returns. * docs: point the proxy design at start_tool and update lib test count * fix(proxy): name the proxy in every refusal the tool sees Since the CONNECT checks moved into `vet_connect`, a refused CONNECT sent the tool only the audit reason ("denied: no route for host"). That text does not say it came from Airlock, so a tool or agent could take it for an upstream error. `refuse` now adds the "airlock proxy: " prefix itself, and the three callers that spelled it out no longer do. * fix(daemon): refuse an exec whose secret reference is not in the store `resolve_tool_env` skipped a `{ secret = "..." }` reference whose label was missing from the store, so the tool ran without a variable it was configured with. Config load rejects such references, so this only happens if an invariant breaks. When it does, the exec now fails closed with an error naming the label, like a stale secret, instead of running with a partial environment. * refactor(daemon): let DaemonShared own the child registry outright `ChildRegistry` wrapped its set in an `Arc` and derived `Clone` so each connection could hold its own handle. Connections now reach it through the one `Arc<DaemonShared>`, so the inner `Arc` was a second layer of sharing that only the concurrency test used. That test now wraps the registry in an `Arc` itself. Also update the lib test count in CLAUDE.md.
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
1 participant
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.
Stacked on #8 (
feat/proxy-tools). Review and merge that first.Why
#8 left one gap in "redaction is mandatory on the output path": a proxy tool's response bytes were only redacted on the tool's stdout.
curl -o file,--dump-header,--tracewrite them into the sandbox root, where the agent reads them unredacted. An upstream that reflects the injected credential (echo/debug endpoints, token-info APIs, aLocationcarrying it) would leak it.This redacts inside the proxy, so no plaintext secret byte ever reaches the tool at all.
What
StreamRedactor(src/redact.rs): chunk-wise redaction whose output for any chunking equalsredact_bytes(whole input). It holds back only what could still become a match:max(end of last match, len − (max_pattern_len − 1)). Sound because the automaton uses standard match semantics — a match is reported when its last byte arrives and cannot be changed by later bytes. Tested over every two-way split point and every chunk size.RedactedBody(src/proxy/server.rs): wraps the upstream body and redacts frame by frame frompoll_frame. No thread, no channel, no task per response: the tool's read rate drives the polls, so backpressure is preserved and memory is one frame plus the held-back tail. Dropping the response drops the upstream read.Accept-Encoding: identityand stripsRange/If-Range; a response with a non-identityContent-Encoding, a transfer coding other thanchunked, or206/Content-Range→502, body dropped unread, audited. No decompressor added.Content-Lengthis dropped for responses with a body (a placeholder is not the secret's length) and hyper chunks instead; HEAD / 1xx / 204 / 304 keep theirs.Verification
cargo test593 passed / 0 failed,clippy --all-targets -D warningsclean,fmtclean. Includes a 33 MB streamed body, a secret split across upstream frames, base64 / URL / hex variants, mid-session secret refresh, mid-stream session drop, and the real/usr/bin/curl -o … --dump-header … --compressedwriting files that contain no secret.httpbin.org:curl -o out.json …/headers→ file holds[REDACTED:smoke_token], 0 occurrences of the secret;curl -Iworks and keepscontent-length;…/gzip(which compresses regardless ofidentity) → the intended 502.HTTP/1.1 200 <token>bypassed the redactor. Fixed (extensions cleared), with a regression test that failed beforehand.Behavior changes an agent will notice (documented in SKILL.md)
--compressedyields plain bytes, or a 502 from an upstream that compresses anyway.curl -C -) restart from zero.Known limits
Content-Encodingon a HEAD / 304 response is refused too, which is stricter than necessary.🤖 Generated with Claude Code