diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml index 2533e7b..86fbf3c 100644 --- a/.cargo/mutants.toml +++ b/.cargo/mutants.toml @@ -203,6 +203,21 @@ exclude_re = [ # commands/mod.rs — purges every instance + image (backend destroy calls). "\\bpurge_all_data\\b", + # commands/proxy.rs (#411) — `coop proxy setup` / `status`. run_setup reads a + # pasted token, prompts for a backend, and writes to the real secret store; + # store_vm_override resolves an instance and writes its proxy.json; run_status + # lists instances and writes stdout; the guidance/report printers only write + # stderr — none `--lib`-observable. The config writer `upsert_proxy_entry`, + # its `read_or_init_doc` helper, and the pure `auth_str` / `auth_for` / + # `service_for` / `redact_credential` / `resolve` / `describe` / + # `write_vm_status` (buffer-tested) stay IN scope — a survivor there is a real + # gap. + "src/commands/proxy\\.rs:.*\\brun_setup\\b", + "src/commands/proxy\\.rs:.*\\brun_status\\b", + "\\bstore_vm_override\\b", + "\\bprint_setup_guidance\\b", + "\\breport_setup\\b", + # admin.rs — uninstall/init IO: terminal summary, recursive data-dir wipe, # self-binary removal. The decide_remove_data / config_path_is_under_data_dir # / is_dev_target_path predicates are pure and stay in scope (unit-tested). @@ -301,6 +316,40 @@ exclude_re = [ "\\bcapture_version\\b", "\\bagent_binary\\b", + # ---- src/proxy.rs (#411): host-side credential-proxy lifecycle ---- + # Process spawn/teardown, binary discovery, PID-file IO, and the token-file + # writer — a `--lib` test cannot observe their effects (coop-proxy's own tests + # + tests/integration.sh cover the behavior). The pure `Provider` helpers + # (`port`/`base_port`/`upstream_host`/`name`, pinned by the port/range/host + # tests), `wire_config_json`, `mint_capability_token` (whose randomness/length + # a unit test asserts), and `read_capability_token` (trim/empty logic covered + # by `read_capability_token_ignores_empty_and_whitespace_files`) stay IN scope + # — a survivor on one of + # those is a real gap. File-anchored so the generic `start`/`stop` names don't + # scope out same-named functions elsewhere. + "src/proxy\\.rs:.*\\bstart_provider\\b", + "src/proxy\\.rs:.*\\bstop\\b", + "src/proxy\\.rs:.*\\bstop_provider\\b", + "src/proxy\\.rs:.*\\bkill_pid_file\\b", + "src/proxy\\.rs:.*\\bspawn_proxy\\b", + "src/proxy\\.rs:.*\\bspawn_reverse_forward\\b", + # slice-3 jail: liveness poll of a spawned process + TCP probe (fail-closed) + # and the platform spawn-command builder (macOS `sandbox-exec` wrap / Linux + # bare binary) — IO/spawn wrappers, not `--lib`-observable. `await_proxy_ready` + # has a unit test for its fail-closed early-exit branch; the poll/timeout loop + # itself stays excluded (IO-bound, slow timeout path). `wire_config_json` + # stays IN scope, pinned by the wire_json_* shape tests. coop-proxy's own + # `jail.rs` (the Landlock ruleset) is not in this sweep — coop-proxy is a + # separate, non-default workspace member — so it is asserted by + # `coop-proxy --jail-selftest` in tests/integration.sh instead. + "src/proxy\\.rs:.*\\bawait_proxy_ready\\b", + "src/proxy\\.rs:.*\\bconfined_command\\b", + "src/proxy\\.rs:.*\\bwrite_token_file\\b", + "src/proxy\\.rs:.*\\blocate_proxy_binary\\b", + "src/proxy\\.rs:.*\\bpid_path\\b", + "src/proxy\\.rs:.*\\bfwd_pid_path\\b", + "src/proxy\\.rs:.*\\btoken_path\\b", + # NOTE on `delete field … from struct …` mutants: cargo-mutants (27.x) does # not honour exclude_re for these — the regex never matches their name (not # by function, struct path, or even file:line). They are emitted only where diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6723b06..d2076ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,14 +30,22 @@ jobs: key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock') }} restore-keys: ${{ runner.os }}-cargo- + # coop-proxy's aws-lc-sys crypto backend (issue #411) needs cmake; the + # `--workspace` clippy/test below build it. Preinstalled on ubuntu-latest, + # installed explicitly so the job doesn't depend on the image contents. + - name: Install cmake + run: sudo apt-get update && sudo apt-get install -y cmake + - name: Format check run: cargo fmt -- --check + # `--workspace` so coop-proxy is linted/tested too (it is not a default + # member, to keep plain `cargo build` and the integration scripts light). - name: Clippy - run: cargo clippy --all-targets --all-features -- -D warnings + run: cargo clippy --workspace --all-targets --all-features -- -D warnings - name: Unit tests - run: cargo test + run: cargo test --workspace - name: Integration test — coop update run: ./tests/integration-update.sh @@ -60,8 +68,11 @@ jobs: - name: Install cargo-deny run: cargo binstall --no-confirm cargo-deny@0.19.9 + # `--workspace` roots the crate graph at every workspace member. With a + # root package present, cargo-deny otherwise checks only `coop`, missing + # `coop-proxy`'s async/HTTP/TLS supply chain (issue #411). - name: Check advisories, licenses, bans, sources - run: cargo deny check + run: cargo deny --workspace check taplo: runs-on: ubuntu-latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b38b30f..d0becba 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,8 +28,10 @@ jobs: os: macos-latest - target: x86_64-unknown-linux-musl os: ubuntu-latest + # Native ARM64 Linux runner — no cross-compilation, so coop-proxy's + # aws-lc-sys builds with the native musl toolchain (issue #411). - target: aarch64-unknown-linux-musl - os: ubuntu-latest + os: ubuntu-24.04-arm runs-on: ${{ matrix.os }} @@ -38,28 +40,29 @@ jobs: with: persist-credentials: false - - name: Install cross-compilation tools (Linux ARM) - if: matrix.target == 'aarch64-unknown-linux-musl' - run: | - sudo apt-get update - sudo apt-get install -y gcc-aarch64-linux-gnu musl-tools - mkdir -p .cargo - cat >> .cargo/config.toml <<'EOF' - [target.aarch64-unknown-linux-musl] - linker = "aarch64-linux-gnu-gcc" - EOF - - - name: Install musl tools (Linux x86_64) - if: matrix.target == 'x86_64-unknown-linux-musl' + # Each target builds on a matching-arch runner (no cross-compilation). + # The Linux musl legs need musl-gcc + cmake to build coop-proxy's + # aws-lc-sys crypto backend natively; macOS ships cmake preinstalled. + - name: Install musl tools and cmake (Linux) + if: runner.os == 'Linux' run: | sudo apt-get update - sudo apt-get install -y musl-tools + sudo apt-get install -y musl-tools cmake - name: Add target run: rustup target add ${{ matrix.target }} - name: Build - run: cargo build --release --target ${{ matrix.target }} + # `--workspace` builds both `coop` and `coop-proxy` from one source + # tree, so they ship in lockstep (issue #411). Native builds: musl-gcc + # is the C compiler (for aws-lc-sys) and the linker on each Linux + # runner. The macOS leg ignores these vars. + env: + CC_x86_64_unknown_linux_musl: musl-gcc + CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER: musl-gcc + CC_aarch64_unknown_linux_musl: musl-gcc + CARGO_TARGET_AARCH64_UNKNOWN_LINUX_MUSL_LINKER: musl-gcc + run: cargo build --release --workspace --target ${{ matrix.target }} - name: Package shell: bash @@ -68,6 +71,9 @@ jobs: name="${BINARY}-${tag}-${{ matrix.target }}" mkdir -p "staging/${name}" cp "target/${{ matrix.target }}/release/${BINARY}" "staging/${name}/" + # coop-proxy rides in the same tarball, inheriting the identical + # SLSA build-provenance attestation with no new machinery. + cp "target/${{ matrix.target }}/release/${BINARY}-proxy" "staging/${name}/" cp LICENSE* staging/${name}/ 2>/dev/null || true tar -czf "${name}.tar.gz" -C staging "${name}" echo "TARBALL=${name}.tar.gz" >> "$GITHUB_ENV" diff --git a/CHANGELOG.md b/CHANGELOG.md index 7402d28..ee911b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,62 @@ # Changelog +## Unreleased + +### New features + +- **Credential-injecting proxy — keep the model API keys out of the guest** + (#411) — New opt-in `[proxy]` config. When set, coop runs a small host-side + reverse proxy (`coop-proxy`, a new binary shipped in the same tarball) — one + process per (VM, provider) — for the lifetime of a remote-mode VM: the guest + is pointed at it and holds only a per-instance capability token, while the + real credential stays on the host and is injected onto requests upstream. + - **Claude Code** (`[proxy.anthropic]`) is pointed at the proxy via + `ANTHROPIC_BASE_URL`; supports an API key (`x-api-key`) or a Claude + `setup-token` (`Authorization: Bearer`). + - **Codex** (`[proxy.openai]`) is pointed at the proxy via a + `[model_providers.coop_local]` block (Responses API) with the capability + token as the provider bearer; OpenAI API keys inject as + `Authorization: Bearer`. In proxy mode Codex's `~/.codex/auth.json` is no + longer staged onto the guest disk; Codex subscription is out of scope — use + an API key. + + The raw `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` is no longer forwarded into the + guest, so a prompt-injected or rogue agent cannot read a usable key. + Resolution fails closed — a bad credential aborts the boot rather than booting + without injection. `coop model local` takes precedence and tears the + proxy down. The proxy binds host loopback and is reverse-tunnelled (`ssh -R`) + into the guest, so it works on both backends (Firecracker and Lima). GitHub + is a tracked follow-up. `coop update` keeps `coop` and `coop-proxy` in + lockstep. + +- **The credential proxy is jailed** (#411) — the host-side `coop-proxy` + process runs confined so a proxy exploit cannot write files, execute + programs, or reach any host beyond the upstream `:443` and DNS `:53`. On + Linux it self-applies Landlock (ABI v4; host kernel ≥6.7) before serving; on + macOS the launcher wraps it in `sandbox-exec` with a Seatbelt profile. The + confinement is fail-closed — if it cannot be established the VM start aborts + rather than running the credential-holding proxy unconfined. The jail is + port-scoped, not host-scoped (upstream identity is enforced by the proxy's + TLS verification), and does not restrict UDP on Linux; see + [`docs/trust-model.md`](docs/trust-model.md). + +- **Per-VM credential overrides + `coop proxy status`** (#411) — A single VM can + use a different credential than the `[proxy.]` default — for + per-project billing, scope, or revocation — via `coop proxy setup [--openai] + --vm `. The override is stored in that instance's state + (`/proxy.json`), not a growing config table, and its secret is + namespaced separately. Resolution is override → default → off. `coop proxy + status [--vm ]` shows what each VM resolves to, with credentials + redacted. + +- **`coop proxy setup` — store a provider credential like a GitHub PAT** (#411) + — Mirrors the `coop github` PAT wizard: paste a credential, pick a secret + backend (macOS Keychain / Linux secret-service / 1Password / 0600 file), and + coop stores it and writes the `cmd:` reference into `[proxy.]` — the + credential is never plaintext in the config. Anthropic is the default; + `--openai` configures Codex. The secret store is namespaced per service, so + proxy secrets live under their own directory rather than among the GitHub PATs. + ## v0.5.4 ### New features diff --git a/Cargo.lock b/Cargo.lock index dbc802e..359dd78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,12 +67,41 @@ version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" +[[package]] +name = "atomic-waker" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" + [[package]] name = "autocfg" version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" +[[package]] +name = "aws-lc-rs" +version = "1.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4342d8937fc7e5dd9b1c60292261c0670c882a2cd1719cfc11b1af41731e32ad" +dependencies = [ + "aws-lc-sys", + "zeroize", +] + +[[package]] +name = "aws-lc-sys" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d9ceb1da931507a12f4fccea479dccd00da1943e1b4ae72d8e502d707361444" +dependencies = [ + "cc", + "cmake", + "dunce", + "fs_extra", + "pkg-config", +] + [[package]] name = "bit-set" version = "0.8.0" @@ -103,6 +132,24 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "bytes" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" + +[[package]] +name = "cc" +version = "1.2.67" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e17dd265a7d0f31ef544e1b20e03add05d3b45b491b633b10d67145d2acc1a38" +dependencies = [ + "find-msvc-tools", + "jobserver", + "libc", + "shlex", +] + [[package]] name = "cfg-if" version = "1.0.4" @@ -161,6 +208,15 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" +[[package]] +name = "cmake" +version = "0.1.58" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0f78a02292a74a88ac736019ab962ece0bc380e3f977bf72e376c5d78ff0678" +dependencies = [ + "cc", +] + [[package]] name = "colorchoice" version = "1.0.5" @@ -198,6 +254,25 @@ dependencies = [ "url", ] +[[package]] +name = "coop-proxy" +version = "0.5.4" +dependencies = [ + "anyhow", + "http-body-util", + "hyper", + "hyper-util", + "landlock", + "rustls", + "serde", + "serde_json", + "tokio", + "tokio-rustls", + "tracing", + "tracing-subscriber", + "webpki-roots", +] + [[package]] name = "cpufeatures" version = "0.3.0" @@ -259,6 +334,32 @@ dependencies = [ "syn", ] +[[package]] +name = "dunce" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" + +[[package]] +name = "enumflags2" +version = "0.7.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1027f7680c853e056ebcec683615fb6fbbc07dbaa13b4d5d9442b146ded4ecef" +dependencies = [ + "enumflags2_derive", +] + +[[package]] +name = "enumflags2_derive" +version = "0.7.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67c78a4d8fdf9953a5c9d458f9efe940fd97a0cab0941c075a813ac594733827" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -281,6 +382,12 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "find-msvc-tools" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" + [[package]] name = "fnv" version = "1.0.7" @@ -302,6 +409,33 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs_extra" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42703706b716c37f96a77aea830392ad231f44c9e9a67872fa5548707e11b11c" + +[[package]] +name = "futures-channel" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d" +dependencies = [ + "futures-core", +] + +[[package]] +name = "futures-core" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" + +[[package]] +name = "futures-sink" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c39754e157331b013978ec91992bde1ac089843443c49cbc7f46150b0fad0893" + [[package]] name = "getrandom" version = "0.2.17" @@ -338,6 +472,25 @@ dependencies = [ "wasip3", ] +[[package]] +name = "h2" +version = "0.4.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6cb093c84e8bd9b188d4c4a8cb6579fc016968d14c99882163cd3ff402a4f155" +dependencies = [ + "atomic-waker", + "bytes", + "fnv", + "futures-core", + "futures-sink", + "http", + "indexmap", + "slab", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "hashbrown" version = "0.15.5" @@ -365,6 +518,51 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "http" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6970f50e31d6fc17d3fa27329444bfa74e196cf62e95052a3f6fee181dba6425" +dependencies = [ + "bytes", + "itoa", +] + +[[package]] +name = "http-body" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca2a8f2913ee65f60facd6a5905613afaa448497a0230cc41ce022d93290bc2c" +dependencies = [ + "bytes", + "http", +] + +[[package]] +name = "http-body-util" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9f41fd6a08e4d4ec69df65976da761afd5ad5e58a9d4acb46bd1c953a9e3ff2" +dependencies = [ + "bytes", + "futures-core", + "http", + "http-body", + "pin-project-lite", +] + +[[package]] +name = "httparse" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" + +[[package]] +name = "httpdate" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" + [[package]] name = "hybrid-array" version = "0.4.10" @@ -374,6 +572,42 @@ dependencies = [ "typenum", ] +[[package]] +name = "hyper" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55281c53a1894c864990125767da440a4e630446785086f52523b20033b74498" +dependencies = [ + "atomic-waker", + "bytes", + "futures-channel", + "futures-core", + "h2", + "http", + "http-body", + "httparse", + "httpdate", + "itoa", + "pin-project-lite", + "smallvec", + "tokio", + "want", +] + +[[package]] +name = "hyper-util" +version = "0.1.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" +dependencies = [ + "bytes", + "http", + "http-body", + "hyper", + "pin-project-lite", + "tokio", +] + [[package]] name = "icu_collections" version = "2.2.0" @@ -516,6 +750,27 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "jobserver" +version = "0.1.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c00acbd29eabad4a2392fa0e921c874934dbbf4194312ad20f04a0ed67a3cb3" +dependencies = [ + "getrandom 0.4.2", + "libc", +] + +[[package]] +name = "landlock" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "635839550ae8b90d9fd2571460a6645dc0aec070225956ca7a2831ed31d2795d" +dependencies = [ + "enumflags2", + "libc", + "thiserror", +] + [[package]] name = "lazy_static" version = "1.5.0" @@ -576,6 +831,17 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "mio" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30d65c71f1ce40ab09135ce117d742b9f8a19ff91a41a8b57ed50bc2de59c427" +dependencies = [ + "libc", + "wasi", + "windows-sys 0.61.2", +] + [[package]] name = "mutants" version = "0.0.4" @@ -630,6 +896,12 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "pkg-config" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e" + [[package]] name = "potential_utf" version = "0.1.5" @@ -779,6 +1051,20 @@ version = "0.8.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" +[[package]] +name = "ring" +version = "0.17.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" +dependencies = [ + "cc", + "cfg-if", + "getrandom 0.2.17", + "libc", + "untrusted", + "windows-sys 0.52.0", +] + [[package]] name = "rustix" version = "1.1.4" @@ -792,6 +1078,41 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustls" +version = "0.23.42" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c54fcab019b409d04215d3a17cb438fd7fbf192ee61461f20f4fe18704bc138" +dependencies = [ + "aws-lc-rs", + "once_cell", + "rustls-pki-types", + "rustls-webpki", + "subtle", + "zeroize", +] + +[[package]] +name = "rustls-pki-types" +version = "1.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "764899a24af3980067ee14bc143654f297b22eaebfe3c7b6b211920a5a59b046" +dependencies = [ + "zeroize", +] + +[[package]] +name = "rustls-webpki" +version = "0.103.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" +dependencies = [ + "aws-lc-rs", + "ring", + "rustls-pki-types", + "untrusted", +] + [[package]] name = "rusty-fork" version = "0.3.1" @@ -889,12 +1210,38 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" +[[package]] +name = "signal-hook-registry" +version = "1.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" +dependencies = [ + "errno", + "libc", +] + +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + [[package]] name = "smallvec" version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" +[[package]] +name = "socket2" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4" +dependencies = [ + "libc", + "windows-sys 0.61.2", +] + [[package]] name = "stable_deref_trait" version = "1.2.1" @@ -907,6 +1254,12 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "subtle" +version = "2.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" + [[package]] name = "syn" version = "2.0.117" @@ -981,6 +1334,56 @@ dependencies = [ "zerovec", ] +[[package]] +name = "tokio" +version = "1.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d988bcd52dbe076d3d46903332f58c912b87a2c49b1428419a5845154762ffee" +dependencies = [ + "bytes", + "libc", + "mio", + "pin-project-lite", + "signal-hook-registry", + "socket2", + "tokio-macros", + "windows-sys 0.61.2", +] + +[[package]] +name = "tokio-macros" +version = "2.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6328af13490e73a9b4694030fafd93f8c8c6a9dede33e821c3fc63eddf8042ba" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "tokio-rustls" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" +dependencies = [ + "rustls", + "tokio", +] + +[[package]] +name = "tokio-util" +version = "0.7.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ae9cec805b01e8fc3fd2fe289f89149a9b66dd16786abd8b19cfa7b48cb0098" +dependencies = [ + "bytes", + "futures-core", + "futures-sink", + "pin-project-lite", + "tokio", +] + [[package]] name = "toml" version = "1.1.2+spec-1.1.0" @@ -1081,6 +1484,12 @@ dependencies = [ "tracing-log", ] +[[package]] +name = "try-lock" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" + [[package]] name = "typenum" version = "1.19.0" @@ -1105,6 +1514,12 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "untrusted" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" + [[package]] name = "url" version = "2.5.8" @@ -1145,6 +1560,15 @@ dependencies = [ "libc", ] +[[package]] +name = "want" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa7760aed19e106de2c7c0b581b509f2f25d3dacaf737cb82ac61bc6d760b0e" +dependencies = [ + "try-lock", +] + [[package]] name = "wasi" version = "0.11.1+wasi-snapshot-preview1" @@ -1203,19 +1627,37 @@ dependencies = [ "semver", ] +[[package]] +name = "webpki-roots" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf85cb06032201fa7c6f829d7db5a7e5aa45bcc0655327713065f6f0576731bf" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-sys" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" +dependencies = [ + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.60.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2f500e4d28234f72040990ec9d39e3a6b950f9f22d3dba18416c35882612bcb" dependencies = [ - "windows-targets", + "windows-targets 0.53.5", ] [[package]] @@ -1227,6 +1669,22 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-targets" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", + "windows_i686_gnullvm 0.52.6", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", +] + [[package]] name = "windows-targets" version = "0.53.5" @@ -1234,58 +1692,106 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4945f9f551b88e0d65f3db0bc25c33b8acea4d9e41163edf90dcd0b19f9069f3" dependencies = [ "windows-link", - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_gnullvm", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", + "windows_aarch64_gnullvm 0.53.1", + "windows_aarch64_msvc 0.53.1", + "windows_i686_gnu 0.53.1", + "windows_i686_gnullvm 0.53.1", + "windows_i686_msvc 0.53.1", + "windows_x86_64_gnu 0.53.1", + "windows_x86_64_gnullvm 0.53.1", + "windows_x86_64_msvc 0.53.1", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + [[package]] name = "windows_aarch64_gnullvm" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a9d8416fa8b42f5c947f8482c43e7d89e73a173cead56d044f6a56104a6d1b53" +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + [[package]] name = "windows_aarch64_msvc" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9d782e804c2f632e395708e99a94275910eb9100b2114651e04744e9b125006" +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + [[package]] name = "windows_i686_gnu" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "960e6da069d81e09becb0ca57a65220ddff016ff2d6af6a223cf372a506593a3" +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + [[package]] name = "windows_i686_gnullvm" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fa7359d10048f68ab8b09fa71c3daccfb0e9b559aed648a8f95469c27057180c" +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + [[package]] name = "windows_i686_msvc" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e7ac75179f18232fe9c285163565a57ef8d3c89254a30685b57d83a38d326c2" +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + [[package]] name = "windows_x86_64_gnu" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c3842cdd74a865a8066ab39c8a7a473c0778a3f29370b5fd6b4b9aa7df4a499" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + [[package]] name = "windows_x86_64_gnullvm" version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ffa179e2d07eee8ad8f57493436566c7cc30ac536a3379fdf008f47f6bb7ae1" +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + [[package]] name = "windows_x86_64_msvc" version = "0.53.1" @@ -1456,6 +1962,12 @@ dependencies = [ "synstructure", ] +[[package]] +name = "zeroize" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e" + [[package]] name = "zerotrie" version = "0.2.4" diff --git a/Cargo.toml b/Cargo.toml index c7a5bc7..5a17dd6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,11 +1,27 @@ -[package] -name = "coop" +[workspace] +members = ["coop-proxy"] +# No `default-members`: with a root package present, plain `cargo build` / +# `cargo test` build only `coop`, so the integration scripts and everyday dev +# builds don't pull in coop-proxy's aws-lc-sys (and its cmake requirement). +# coop-proxy is reached explicitly via `--workspace` — CI clippy/test, the +# release build, and `cargo deny --workspace` all pass it. +# `fuzz/` is its own detached workspace (cargo-fuzz), kept out of this one. +exclude = ["fuzz"] + +[workspace.package] version = "0.5.4" edition = "2024" -description = "Isolated VM environment for running Claude Code" license = "Apache-2.0" repository = "https://github.com/trailofbits/coop" +[package] +name = "coop" +version.workspace = true +edition.workspace = true +description = "Isolated VM environment for running Claude Code" +license.workspace = true +repository.workspace = true + [dependencies] anyhow = "1" clap = { version = "4", features = ["derive"] } @@ -29,7 +45,12 @@ url = { version = "2", features = ["serde"] } [dev-dependencies] proptest = "1.11.0" -[lints.clippy] +[lints] +workspace = true + +# Shared across every workspace member (coop + coop-proxy) so the strict +# clippy/rustc policy is declared once. +[workspace.lints.clippy] pedantic = { level = "warn", priority = -1 } unwrap_used = "deny" expect_used = "warn" @@ -55,7 +76,7 @@ must_use_candidate = "allow" missing_errors_doc = "allow" missing_panics_doc = "allow" -[lints.rust] +[workspace.lints.rust] # `cfg(kani)` is set by `cargo kani` (the bounded-proof toolchain), never by a # normal build. Register it so the `proofs` module doesn't trip unexpected-cfgs. unexpected_cfgs = { level = "warn", check-cfg = ['cfg(kani)'] } diff --git a/config.example.toml b/config.example.toml index dd7b9cc..0e3b21e 100644 --- a/config.example.toml +++ b/config.example.toml @@ -102,6 +102,32 @@ # model = "gpt-oss:120b" # auth_token = "sk-..." # Optional; permissive servers ignore it. +# [proxy] # Host-side credential-injecting proxy +# # (issue #411). Opt-in: when set, coop runs +# # a `coop-proxy` process on the host and the +# # raw credential never enters the guest — +# # the guest is pointed at the proxy and gets +# # only a per-instance capability token, and +# # the raw API key is no longer forwarded. +# # Applies only in remote model mode +# # (`coop model remote`); local mode +# # takes precedence. Works on both backends. +# `coop proxy setup [--openai] [--vm ]` writes this for you: it takes a +# pasted credential, stores it in Keychain / 1Password / a 0600 file, and fills +# in the `cmd:` reference. Anthropic covers Claude Code; openai covers Codex. +# A `--vm ` override is stored in that VM's instance state (not here); +# `coop proxy status` shows what each VM resolves to (override → default → off). +# [proxy.anthropic] +# credential = "cmd:op read op://Private/Anthropic/credential" # same "cmd:" support +# auth = "api_key" # api_key → x-api-key (default); bearer → +# # Authorization: Bearer, for a Claude +# # `setup-token` (run `claude setup-token`). +# [proxy.openai] # Codex. OpenAI keys are always Bearer. +# credential = "cmd:op read op://Private/OpenAI/credential" +# auth = "bearer" # OpenAI keys inject as Authorization: Bearer. +# # Codex subscription (auth.json) is not +# # supported in proxy mode — use an API key. + # [profiles.my-tools] # apt_packages = ["ripgrep", "fd-find", "jq"] # pre_install = "curl -fsSL https://example.com/setup.sh | bash" diff --git a/coop-proxy/Cargo.toml b/coop-proxy/Cargo.toml new file mode 100644 index 0000000..6ef7008 --- /dev/null +++ b/coop-proxy/Cargo.toml @@ -0,0 +1,79 @@ +[package] +name = "coop-proxy" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +description = "Host-side credential-injecting reverse proxy for coop" + +[[bin]] +name = "coop-proxy" +path = "src/main.rs" + +[dependencies] +anyhow = "1" +serde = { version = "1", features = ["derive"] } +serde_json = "1" +tracing = "0.1" +tracing-subscriber = { version = "0.3", features = ["env-filter"] } +tokio = { version = "1", default-features = false, features = [ + "rt-multi-thread", + "net", + "io-util", + "macros", + "signal", + "sync", + "time", +] } +hyper = { version = "1", default-features = false, features = [ + "http1", + "http2", + "server", + "client", +] } +hyper-util = { version = "0.1", default-features = false, features = [ + "tokio", + "server", + "http1", + "http2", +] } +http-body-util = { version = "0.1", default-features = false } +tokio-rustls = { version = "0.26", default-features = false, features = [ + "tls12", +] } +# Crypto provider: aws-lc-rs (rustls's default, FIPS-capable — the design's +# recommendation, §13). Its `aws-lc-sys` C backend builds with cmake + a +# native musl compiler; the release workflow builds every target on a +# matching-arch runner (no cross-compilation), so `musl-gcc` compiles it +# cleanly for both musl targets. +rustls = { version = "0.23", default-features = false, features = [ + "std", + "tls12", + "aws_lc_rs", +] } +# Pinned Mozilla root set compiled into the binary — the two fixed upstreams +# need no OS trust-store variance, and a pinned root set is smaller surface and +# deterministic for a security tool. +webpki-roots = "1" + +# Linux jail (issue #411, slice 3): the proxy self-restricts with Landlock +# (ABI v4 — TCP bind/connect port rules, kernel ≥6.7; fs/exec rules, kernel +# ≥5.13) after its libraries are loaded and before it binds, so a proxy exploit +# cannot write files, exec programs, or reach any host beyond :443/:53. macOS +# uses Seatbelt via `sandbox-exec`, applied by the launcher (see coop's +# `proxy.rs`), so no crate is needed there. +[target.'cfg(target_os = "linux")'.dependencies] +landlock = "0.4.5" + +[dev-dependencies] +tokio = { version = "1", default-features = false, features = [ + "rt-multi-thread", + "net", + "io-util", + "macros", + "time", + "process", +] } + +[lints] +workspace = true diff --git a/coop-proxy/src/config.rs b/coop-proxy/src/config.rs new file mode 100644 index 0000000..ae8a77c --- /dev/null +++ b/coop-proxy/src/config.rs @@ -0,0 +1,165 @@ +//! Startup configuration for `coop-proxy`, read once from stdin. +//! +//! `coop` resolves the real upstream credential on the host (via its `cmd:` +//! secret-resolution machinery) and hands the whole blob to the proxy over a +//! stdin pipe — never argv (world-readable through `/proc//cmdline`) and +//! never a file on disk. The proxy deserializes it once at startup, closes +//! stdin, and holds the secret in process memory only. + +use std::fmt; +use std::net::SocketAddr; + +use serde::Deserialize; + +/// A secret string that never appears in `Debug` output or logs. +#[derive(Clone, Deserialize)] +#[serde(transparent)] +pub struct Secret(String); + +impl Secret { + /// Borrow the underlying value. Named to flag every read at review time. + pub fn expose(&self) -> &str { + &self.0 + } +} + +impl fmt::Debug for Secret { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("Secret()") + } +} + +/// How the proxy injects the real upstream credential onto forwarded +/// requests. The guest's credential slot is always stripped first (see +/// [`crate::proxy`]); this decides the header that replaces it. +#[derive(Debug, Clone, Deserialize)] +#[serde(tag = "scheme", rename_all = "snake_case")] +pub enum Injection { + /// Anthropic API key → `x-api-key: `. + XApiKey { credential: Secret }, + /// Bearer token (e.g. a Claude `setup-token`) → + /// `authorization: Bearer `. + Bearer { credential: Secret }, +} + +/// The full startup blob. One `coop-proxy` process serves exactly one +/// upstream (per-integration proxy) — Codex spawns a second process with its +/// own upstream and token rather than a route table inside one process. +#[derive(Debug, Clone, Deserialize)] +pub struct ProxyConfig { + /// Guest-visible listen address. Must be a concrete host interface bound + /// to the private host-guest link, never an unspecified address + /// (`0.0.0.0` / `[::]`) — enforced at bind time in [`crate::proxy::serve`]. + pub listen: SocketAddr, + + /// Per-instance capability token the guest must present on every request. + /// Worthless off the host (it only authorizes the local proxy, which + /// injects the real credential itself), so exfiltrating it gains a + /// compromised guest nothing. + pub capability_token: Secret, + + /// The fixed upstream host, e.g. `api.anthropic.com`. The guest controls + /// only the request path — never this host or the `https` scheme — which + /// is what closes SSRF: a rogue guest cannot retarget the injected key at + /// an attacker-controlled host. + pub upstream_host: String, + + /// The real credential and the header it is injected as. + pub injection: Injection, +} + +impl ProxyConfig { + /// Parse the startup blob from a JSON string. + pub fn from_json(s: &str) -> anyhow::Result { + let cfg: Self = serde_json::from_str(s) + .map_err(|e| anyhow::anyhow!("Failed to parse proxy config: {e}"))?; + // Defense in depth: an empty token would fail open, since + // `constant_time_eq(b"", b"")` is true and a bare `Authorization: + // Bearer ` presents an empty token. coop always mints a 64-hex token, + // so this is unreachable in production, but reject it at parse anyway. + if cfg.capability_token.expose().is_empty() { + anyhow::bail!("capability_token must not be empty"); + } + Ok(cfg) + } +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "tests")] +#[expect(clippy::panic, reason = "tests use panic! for unreachable arms")] +mod tests { + use super::*; + + fn sample_json() -> &'static str { + r#"{ + "listen": "172.16.0.1:8788", + "capability_token": "cap-token-abc", + "upstream_host": "api.anthropic.com", + "injection": { "scheme": "x_api_key", "credential": "sk-secret" } + }"# + } + + #[test] + fn parses_api_key_injection() { + let cfg = ProxyConfig::from_json(sample_json()).unwrap(); + assert_eq!(cfg.listen.port(), 8788); + assert_eq!(cfg.upstream_host, "api.anthropic.com"); + assert_eq!(cfg.capability_token.expose(), "cap-token-abc"); + match &cfg.injection { + Injection::XApiKey { credential } => assert_eq!(credential.expose(), "sk-secret"), + Injection::Bearer { .. } => panic!("expected x_api_key"), + } + } + + #[test] + fn parses_bearer_injection() { + let json = r#"{ + "listen": "127.0.0.1:1", + "capability_token": "t", + "upstream_host": "api.anthropic.com", + "injection": { "scheme": "bearer", "credential": "setup-tok" } + }"#; + let cfg = ProxyConfig::from_json(json).unwrap(); + match &cfg.injection { + Injection::Bearer { credential } => assert_eq!(credential.expose(), "setup-tok"), + Injection::XApiKey { .. } => panic!("expected bearer"), + } + } + + #[test] + fn rejects_unknown_scheme() { + let json = r#"{ + "listen": "127.0.0.1:1", + "capability_token": "t", + "upstream_host": "h", + "injection": { "scheme": "basic", "credential": "x" } + }"#; + assert!(ProxyConfig::from_json(json).is_err()); + } + + #[test] + fn rejects_empty_capability_token() { + let json = r#"{ + "listen": "127.0.0.1:1", + "capability_token": "", + "upstream_host": "api.anthropic.com", + "injection": { "scheme": "bearer", "credential": "x" } + }"#; + assert!(ProxyConfig::from_json(json).is_err()); + } + + #[test] + fn secret_debug_is_redacted() { + let cfg = ProxyConfig::from_json(sample_json()).unwrap(); + let rendered = format!("{cfg:?}"); + assert!( + !rendered.contains("sk-secret"), + "credential leaked: {rendered}" + ); + assert!( + !rendered.contains("cap-token-abc"), + "token leaked: {rendered}" + ); + assert!(rendered.contains("")); + } +} diff --git a/coop-proxy/src/jail.rs b/coop-proxy/src/jail.rs new file mode 100644 index 0000000..2c0910c --- /dev/null +++ b/coop-proxy/src/jail.rs @@ -0,0 +1,193 @@ +//! Self-confinement for `coop-proxy` (issue #411, slice 3). +//! +//! `coop-proxy` holds the real upstream credential and terminates connections +//! originated by the untrusted guest, so it is the feature's new attack +//! surface. This module bounds the blast radius of a proxy exploit: once +//! confined, the process cannot write any file, execute any program, or open a +//! TCP connection to anything but the two upstream ports (`443`) and DNS +//! (`53`). Reads stay open (the resolver config, the dynamic linker) and writes +//! to already-open descriptors (the inherited stderr log) are unaffected. +//! +//! **Linux** uses Landlock (ABI v4), applied by the process to *itself* from +//! [`crate::main`] — after its libraries are loaded and before the tokio +//! runtime spawns worker threads, so every thread inherits the domain. This +//! placement is deliberate: a launcher-side `pre_exec` grant of the filesystem +//! *execute* right cannot cover a dynamically linked binary (the kernel also +//! checks the right on the `ld.so` interpreter at `execve`), whereas a +//! post-startup self-restriction needs no execute grant at all — the proxy +//! never execs again. +//! +//! **macOS** is confined externally with a Seatbelt profile via `sandbox-exec` +//! (see coop's `proxy.rs`); there is nothing to do in-process there. +//! +//! **Limitations, stated honestly** (see `docs/trust-model.md`): the network +//! rules are port-scoped, not host-scoped — a fully compromised proxy could +//! still reach *some other* host on `:443` (the two upstreams' identity is +//! enforced at the TLS layer in [`crate::proxy`], and the guest still cannot +//! retarget them). Landlock's network rules cover TCP only, so UDP egress +//! (including DNS) is not restricted. Both match the DNS/CDN egress caveats +//! coop already documents. + +use anyhow::Result; + +/// A port outside the jail's allowlist, used by the self-test to prove a +/// connection to a non-upstream port is refused by the jail. +const PROBE_BLOCKED_PORT: u16 = 47821; + +/// Run the production confinement ruleset against this process, then probe that +/// it actually restricts on this host: filesystem write denied, program exec +/// denied, a non-upstream TCP port refused, and the upstream port (`443`) still +/// reachable. Invoked via `coop-proxy --jail-selftest`; the VM integration +/// suite runs it on both backends (directly on Linux, under `sandbox-exec` on +/// macOS) so the jail is asserted, not merely designed. +/// +/// Returns an error (non-zero exit) if any property does not hold, so the +/// integration test can assert on the exit status. +pub fn selftest() -> Result<()> { + #[cfg(target_os = "linux")] + apply(8788)?; + probe() +} + +/// Confine the current process. On Linux this establishes the Landlock domain +/// and **fails closed** if the kernel cannot fully enforce it. On other +/// platforms confinement is external and this is never called (the launcher +/// only requests it on Linux). +#[cfg(target_os = "linux")] +pub fn apply(listen_port: u16) -> Result<()> { + linux::apply(listen_port) +} + +#[cfg(target_os = "linux")] +mod linux { + use anyhow::{Context, Result, bail}; + use landlock::{ + ABI, Access, AccessFs, AccessNet, CompatLevel, Compatible, NetPort, Ruleset, RulesetAttr, + RulesetCreatedAttr, RulesetStatus, + }; + + /// Upstream HTTPS. The guest never influences the host or scheme (see + /// [`crate::proxy`]); this only widens the jail enough to reach it. + const UPSTREAM_PORT: u16 = 443; + /// DNS over TCP (the fallback path for truncated UDP responses). UDP DNS is + /// not gated by Landlock's TCP-only network rules and needs no allowance. + const DNS_TCP_PORT: u16 = 53; + + pub fn apply(listen_port: u16) -> Result<()> { + let abi = ABI::V4; + // Handle every filesystem-write and the execute right, plus TCP + // bind/connect. Deliberately do not handle the read rights, so all + // reads stay open (dynamic linker, `/etc/resolv.conf`). Nothing is + // granted for write or execute → they are denied everywhere. + let write_access = AccessFs::from_all(abi) & !AccessFs::from_read(abi); + let status = Ruleset::default() + // Fail closed on a kernel that cannot honour ABI v4 rather than + // silently enforcing a weaker subset. + .set_compatibility(CompatLevel::HardRequirement) + .handle_access(write_access) + .context("handle filesystem-write access")? + // `Execute` is a *read-family* right in Landlock (`from_read` + // returns `Execute | ReadFile | ReadDir`), so `& !from_read` above + // excluded it — it MUST be handled explicitly here or execve would + // stay ungated and the jail would not block program execution. Do + // not fold this into `write_access` or drop it as redundant. + .handle_access(AccessFs::Execute) + .context("handle filesystem-execute access")? + .handle_access(AccessNet::ConnectTcp | AccessNet::BindTcp) + .context("handle network access")? + .create() + .context("create Landlock ruleset")? + // Egress allowlist: connect to the two upstream ports only. + .add_rule(NetPort::new(UPSTREAM_PORT, AccessNet::ConnectTcp)) + .context("allow upstream connect")? + .add_rule(NetPort::new(DNS_TCP_PORT, AccessNet::ConnectTcp)) + .context("allow DNS connect")? + // Bind allowlist: only the proxy's own loopback listener. + .add_rule(NetPort::new(listen_port, AccessNet::BindTcp)) + .context("allow listener bind")? + .restrict_self() + .context("apply Landlock restriction")?; + + if !matches!(status.ruleset, RulesetStatus::FullyEnforced) { + bail!( + "Landlock jail is {:?}, not fully enforced — the host kernel is too old \ + (need ≥6.7 for network rules, ≥5.13 for filesystem rules). Refusing to run \ + the credential proxy unconfined (fail-closed).", + status.ruleset + ); + } + Ok(()) + } +} + +/// Probe the four confinement properties on the current (already-confined) +/// process and report. Distinguishes a jail refusal (`PermissionDenied`) from a +/// mere connection refusal, so no live peers are needed. +fn probe() -> Result<()> { + use std::io::ErrorKind::PermissionDenied; + use std::net::TcpStream; + + let denied = |e: &std::io::Error| e.kind() == PermissionDenied; + + // (c) filesystem write denied + let probe_file = std::env::temp_dir().join("coop-proxy-jail-selftest.probe"); + let write_blocked = match std::fs::File::create(&probe_file) { + Ok(_) => { + let _ = std::fs::remove_file(&probe_file); + false + } + Err(e) => denied(&e), + }; + + // (d) program execution denied. `/bin/sh` exists on both supported hosts + // (Linux, macOS); on a host without it execve would fail `NotFound` rather + // than `PermissionDenied`, which reports FAIL (over-strict) — a false + // negative, never a false pass. + let exec_blocked = std::process::Command::new("/bin/sh") + .args(["-c", "exit 0"]) + .status() + .is_err_and(|e| denied(&e)); + + // (b) a non-upstream TCP port is refused by the jail + let other_blocked = + TcpStream::connect(("127.0.0.1", PROBE_BLOCKED_PORT)).is_err_and(|e| denied(&e)); + + // (a) the upstream port (443) is still reachable — the jail must not block + // it. A connection refusal (nothing listening on loopback:443) is fine; + // only a jail refusal (PermissionDenied) would be a failure. + let upstream_ok = !TcpStream::connect(("127.0.0.1", 443)).is_err_and(|e| denied(&e)); + + let yn = |b: bool, yes: &'static str, no: &'static str| if b { yes } else { no }; + let pass = write_blocked && exec_blocked && other_blocked && upstream_ok; + tracing::info!( + "coop-proxy jail self-test: write={} exec={} connect-other={} connect-443={} => {}", + yn(write_blocked, "BLOCKED", "OPEN"), + yn(exec_blocked, "BLOCKED", "OPEN"), + yn(other_blocked, "BLOCKED", "OPEN"), + yn(upstream_ok, "ALLOWED", "BLOCKED"), + yn(pass, "PASS", "FAIL"), + ); + + if pass { + Ok(()) + } else { + anyhow::bail!("coop-proxy jail self-test FAILED — the process is not properly confined") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The probe must report failure when the process is *not* confined — + /// otherwise a `=> PASS` from the integration self-test would be hollow. + /// The test harness runs unconfined, so the write and exec probes succeed + /// (nothing is blocked) and `probe` must return an error. + #[test] + fn probe_fails_when_unconfined() { + assert!( + probe().is_err(), + "probe reported success in an unconfined process — the self-test would be hollow" + ); + } +} diff --git a/coop-proxy/src/main.rs b/coop-proxy/src/main.rs new file mode 100644 index 0000000..d34b7da --- /dev/null +++ b/coop-proxy/src/main.rs @@ -0,0 +1,107 @@ +//! `coop-proxy` — the host-side credential-injecting reverse proxy. +//! +//! `coop` spawns one process per proxied upstream (per-instance, per-agent), +//! writes the startup config to its stdin (see [`config`]), and supervises it +//! for the lifetime of the VM. The guest is pointed at this process via a +//! base-URL override and holds only a per-instance capability token; the real +//! upstream credential lives here, in host memory, and is attached to +//! outbound requests the guest never sees. + +mod config; +mod jail; +mod proxy; +mod tls; + +use std::io::Read; + +use anyhow::{Context, Result}; + +use crate::config::ProxyConfig; + +fn main() -> Result<()> { + tracing_subscriber::fmt() + .with_env_filter( + tracing_subscriber::EnvFilter::try_from_default_env() + .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new("info")), + ) + .with_writer(std::io::stderr) + .init(); + + let args: Vec = std::env::args().skip(1).collect(); + + // Diagnostic used by the integration suite to assert the jail actually + // restricts on the host (see `jail::selftest`). Handles no config or + // credential, so it runs before reading stdin. + if args.iter().any(|a| a == "--jail-selftest") { + return jail::selftest(); + } + + let cfg = read_config().context("failed to read startup config from stdin")?; + + // Self-confine before building the runtime: Landlock's domain is inherited + // by threads created afterwards, and the multi-thread runtime spawns its + // workers below — so applying it here covers every worker. + // + // Fail-closed by default: on Linux the credential proxy always confines + // unless the explicit `--no-jail` opt-out is passed. Only the in-repo + // `coop-proxy` tests pass it (they run on hosts that may lack Landlock); + // `coop` never does, so there is no path where the launcher serves the + // credential proxy unconfined. macOS confines externally via `sandbox-exec` + // (coop's `proxy.rs`), so this is Linux-only. + #[cfg(target_os = "linux")] + if !args.iter().any(|a| a == "--no-jail") { + jail::apply(cfg.listen.port()).context("failed to establish the Landlock jail")?; + tracing::info!( + "Landlock jail established: filesystem writes and program exec denied; \ + TCP egress limited to :443/:53" + ); + } + + let runtime = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .context("failed to build tokio runtime")?; + + runtime.block_on(async move { proxy::serve(cfg, shutdown_signal()).await }) +} + +/// Read the JSON startup blob from stdin to EOF, then let stdin close. The +/// secret lands in process memory only — never argv, never a file. +fn read_config() -> Result { + let mut buf = String::new(); + std::io::stdin() + .read_to_string(&mut buf) + .context("reading stdin")?; + ProxyConfig::from_json(&buf) +} + +/// Resolve on SIGTERM (coop's teardown signal) or Ctrl-C, so the proxy stops +/// accepting new connections and exits cleanly. +async fn shutdown_signal() { + #[cfg(unix)] + { + use tokio::signal::unix::{SignalKind, signal}; + let mut term = match signal(SignalKind::terminate()) { + Ok(s) => s, + Err(e) => { + tracing::warn!("failed to install SIGTERM handler: {e}"); + std::future::pending::<()>().await; + return; + } + }; + tokio::select! { + _ = term.recv() => tracing::info!("received SIGTERM"), + r = tokio::signal::ctrl_c() => { + if let Err(e) = r { + tracing::warn!("ctrl_c handler error: {e}"); + } + } + } + } + #[cfg(not(unix))] + { + if let Err(e) = tokio::signal::ctrl_c().await { + tracing::warn!("ctrl_c handler error: {e}"); + } + } +} diff --git a/coop-proxy/src/proxy.rs b/coop-proxy/src/proxy.rs new file mode 100644 index 0000000..b4d45f6 --- /dev/null +++ b/coop-proxy/src/proxy.rs @@ -0,0 +1,571 @@ +//! The reverse-proxy request path: verify the guest's capability token, +//! rewrite headers (strip the guest credential, inject the real one, pin the +//! upstream `Host`), and stream the request/response bodies to the fixed +//! upstream over TLS. +//! +//! Everything here is deliberately small and explicit — it is the one +//! component that terminates a connection originated by the untrusted guest +//! and attaches the real credential. The security-critical invariants: +//! +//! - The guest is never authorized unless it presents the exact per-instance +//! capability token (constant-time compared). +//! - The upstream host and scheme are fixed by the host-side config; only the +//! request path is taken from the guest (closes SSRF). +//! - The guest's own `authorization` / `x-api-key` (the capability token) is +//! stripped and replaced with the real credential — it never reaches the +//! upstream, and the real credential never reaches the guest. + +use std::convert::Infallible; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context as TaskContext, Poll}; + +use anyhow::{Context, Result}; +use http_body_util::{BodyExt, Full, combinators::BoxBody}; +use hyper::body::{Body, Bytes, Frame, Incoming, SizeHint}; +use hyper::header::{AUTHORIZATION, HOST, HeaderMap, HeaderName, HeaderValue}; +use hyper::service::service_fn; +use hyper::{Request, Response, StatusCode, Uri}; +use hyper_util::rt::{TokioExecutor, TokioIo}; +use hyper_util::server::conn::auto::Builder as ServerBuilder; +use rustls::pki_types::ServerName; +use tokio::net::{TcpListener, TcpStream}; +use tokio::sync::{OwnedSemaphorePermit, Semaphore}; +use tokio::time::{Duration, timeout}; +use tokio_rustls::TlsConnector; + +use crate::config::{Injection, ProxyConfig}; +use crate::tls; + +/// Fixed upstream port. The guest cannot influence host or port — only the +/// request path is forwarded. +const UPSTREAM_PORT: u16 = 443; + +/// How long to wait for the upstream TCP connect + TLS handshake before +/// giving up, so a rogue guest cannot pin host resources on a stuck dial. +const UPSTREAM_TIMEOUT: Duration = Duration::from_secs(30); + +/// Cap on concurrently in-flight proxied requests, so a rogue guest cannot +/// exhaust host CPU/memory by opening unbounded upstream connections. +const MAX_CONCURRENT_REQUESTS: usize = 256; + +/// The unified response body: either an upstream stream or a small local +/// error page, both boxed to one type. +type ProxyBody = BoxBody; + +/// A response body that holds its concurrency permit until the body is fully +/// streamed. Without this the permit would drop when the response *headers* +/// arrive, leaving the (possibly minutes-long, streaming) body uncounted — so +/// `MAX_CONCURRENT_REQUESTS` would not actually bound concurrent upstream +/// connections. Attaching the permit here makes the cap effective for a +/// request's whole lifetime. +struct GuardedBody { + inner: ProxyBody, + _permit: OwnedSemaphorePermit, +} + +impl Body for GuardedBody { + type Data = Bytes; + type Error = hyper::Error; + + fn poll_frame( + mut self: Pin<&mut Self>, + cx: &mut TaskContext<'_>, + ) -> Poll, Self::Error>>> { + Pin::new(&mut self.inner).poll_frame(cx) + } + + fn is_end_stream(&self) -> bool { + self.inner.is_end_stream() + } + + fn size_hint(&self) -> SizeHint { + self.inner.size_hint() + } +} + +/// The `x-api-key` header, constructed once. +fn x_api_key() -> HeaderName { + HeaderName::from_static("x-api-key") +} + +/// Shared, cheaply-cloneable per-connection state. +#[derive(Clone)] +struct Ctx { + cfg: Arc, + connector: TlsConnector, + permits: Arc, +} + +impl Ctx { + fn new(cfg: ProxyConfig) -> Result { + let connector = TlsConnector::from(tls::client_config()?); + Ok(Self { + cfg: Arc::new(cfg), + connector, + permits: Arc::new(Semaphore::new(MAX_CONCURRENT_REQUESTS)), + }) + } +} + +/// Bind the listener and serve until `shutdown` resolves. +/// +/// Refuses to bind an unspecified address (`0.0.0.0` / `[::]`): the proxy +/// must be reachable only over the private host-guest link, never every host +/// interface. +pub async fn serve( + cfg: ProxyConfig, + shutdown: impl std::future::Future, +) -> Result<()> { + if cfg.listen.ip().is_unspecified() { + anyhow::bail!( + "refusing to bind unspecified address {} — the proxy must bind only the \ + private host-guest interface", + cfg.listen + ); + } + let listener = TcpListener::bind(cfg.listen) + .await + .with_context(|| format!("failed to bind proxy listener on {}", cfg.listen))?; + tracing::info!( + "coop-proxy listening on {} → https://{}", + cfg.listen, + cfg.upstream_host + ); + let ctx = Ctx::new(cfg)?; + accept_loop(ctx, listener, shutdown).await; + Ok(()) +} + +async fn accept_loop( + ctx: Ctx, + listener: TcpListener, + shutdown: impl std::future::Future, +) { + tokio::pin!(shutdown); + loop { + tokio::select! { + () = &mut shutdown => { + tracing::info!("shutdown requested — no longer accepting connections"); + break; + } + accepted = listener.accept() => { + let stream = match accepted { + Ok((stream, _peer)) => stream, + Err(e) => { + tracing::warn!("accept failed: {e}"); + continue; + } + }; + let ctx = ctx.clone(); + tokio::spawn(async move { + let io = TokioIo::new(stream); + let service = service_fn(move |req| { + let ctx = ctx.clone(); + async move { handle(req, ctx).await } + }); + if let Err(e) = ServerBuilder::new(TokioExecutor::new()) + .serve_connection(io, service) + .await + { + tracing::debug!("guest connection error: {e}"); + } + }); + } + } + } +} + +/// A refusal carrying only a fixed, secret-free message. +struct Refusal { + status: StatusCode, + msg: &'static str, +} + +async fn handle(req: Request, ctx: Ctx) -> Result, Infallible> { + match proxy(req, &ctx).await { + Ok(resp) => Ok(resp), + Err(Refusal { status, msg }) => Ok(error_response(status, msg)), + } +} + +async fn proxy(req: Request, ctx: &Ctx) -> Result, Refusal> { + if !authorized(req.headers(), ctx.cfg.capability_token.expose()) { + return Err(Refusal { + status: StatusCode::UNAUTHORIZED, + msg: "missing or invalid coop-proxy capability token", + }); + } + + // Held until the response body finishes streaming (see `GuardedBody`), so + // the concurrency cap bounds a request for its whole lifetime. + let permit = ctx + .permits + .clone() + .try_acquire_owned() + .map_err(|_| Refusal { + status: StatusCode::SERVICE_UNAVAILABLE, + msg: "proxy at capacity", + })?; + + let (mut parts, body) = req.into_parts(); + parts.headers = + build_upstream_headers(&parts.headers, &ctx.cfg.upstream_host, &ctx.cfg.injection) + .map_err(|_| Refusal { + status: StatusCode::BAD_REQUEST, + msg: "request headers could not be rewritten", + })?; + parts.uri = origin_form(&parts.uri).map_err(|_| Refusal { + status: StatusCode::BAD_REQUEST, + msg: "invalid request target", + })?; + let upstream_req = Request::from_parts(parts, body); + + forward(upstream_req, ctx, permit).await +} + +async fn forward( + req: Request, + ctx: &Ctx, + permit: OwnedSemaphorePermit, +) -> Result, Refusal> { + let host = ctx.cfg.upstream_host.as_str(); + let server_name = ServerName::try_from(host.to_owned()).map_err(|_| Refusal { + status: StatusCode::INTERNAL_SERVER_ERROR, + msg: "configured upstream host is not a valid TLS server name", + })?; + + let connect = async { + let tcp = TcpStream::connect((host, UPSTREAM_PORT)).await?; + let _ = tcp.set_nodelay(true); + ctx.connector.connect(server_name, tcp).await + }; + let tls_stream = match timeout(UPSTREAM_TIMEOUT, connect).await { + Ok(Ok(s)) => s, + Ok(Err(e)) => { + tracing::warn!("upstream connect/TLS to {host} failed: {e}"); + return Err(bad_gateway()); + } + Err(_) => { + tracing::warn!("upstream connect/TLS to {host} timed out"); + return Err(bad_gateway()); + } + }; + + let (mut sender, conn) = hyper::client::conn::http1::handshake(TokioIo::new(tls_stream)) + .await + .map_err(|e| { + tracing::warn!("upstream handshake failed: {e}"); + bad_gateway() + })?; + tokio::spawn(async move { + if let Err(e) = conn.await { + tracing::debug!("upstream connection closed: {e}"); + } + }); + + let resp = sender.send_request(req).await.map_err(|e| { + tracing::warn!("upstream request failed: {e}"); + bad_gateway() + })?; + // Wrap the streaming body so the permit is released only when the body is + // fully drained, not when these headers arrive. + Ok(resp.map(|body| { + GuardedBody { + inner: body.boxed(), + _permit: permit, + } + .boxed() + })) +} + +fn bad_gateway() -> Refusal { + Refusal { + status: StatusCode::BAD_GATEWAY, + msg: "upstream request failed", + } +} + +/// Whether the request presents the exact capability token. Constant-time on +/// the token bytes so a timing side-channel cannot recover it. +fn authorized(headers: &HeaderMap, expected: &str) -> bool { + match presented_token(headers) { + Some(token) => constant_time_eq(&token, expected.as_bytes()), + None => false, + } +} + +/// Extract the capability token the guest presented, from either +/// `Authorization: Bearer ` (Claude Code with `ANTHROPIC_AUTH_TOKEN`) or +/// `x-api-key: `. Both slots are stripped before forwarding. +fn presented_token(headers: &HeaderMap) -> Option> { + if let Some(value) = headers.get(AUTHORIZATION) + && let Ok(s) = value.to_str() + && let Some(rest) = s.strip_prefix("Bearer ") + { + return Some(rest.as_bytes().to_vec()); + } + headers.get(x_api_key()).map(|v| v.as_bytes().to_vec()) +} + +/// Constant-time byte equality. The early length check leaks only length, +/// which for a fixed-width random token reveals nothing useful. +fn constant_time_eq(a: &[u8], b: &[u8]) -> bool { + if a.len() != b.len() { + return false; + } + let mut diff = 0u8; + for (x, y) in a.iter().zip(b.iter()) { + diff |= x ^ y; + } + diff == 0 +} + +/// Build the header set sent upstream: drop hop-by-hop headers, drop the +/// guest's `Host` and credential slots, pin the upstream `Host`, and inject +/// the real credential (marked sensitive so hyper never logs it). +fn build_upstream_headers( + incoming: &HeaderMap, + upstream_host: &str, + injection: &Injection, +) -> Result { + let mut out = HeaderMap::with_capacity(incoming.len() + 1); + for (name, value) in incoming { + if is_hop_by_hop(name) || name == HOST || is_credential_header(name) { + continue; + } + out.append(name.clone(), value.clone()); + } + + out.insert( + HOST, + HeaderValue::from_str(upstream_host) + .context("upstream host is not a valid header value")?, + ); + + match injection { + Injection::XApiKey { credential } => { + let mut value = HeaderValue::from_str(credential.expose()) + .context("credential is not a valid header value")?; + value.set_sensitive(true); + out.insert(x_api_key(), value); + } + Injection::Bearer { credential } => { + let mut value = HeaderValue::from_str(&format!("Bearer {}", credential.expose())) + .context("credential is not a valid header value")?; + value.set_sensitive(true); + out.insert(AUTHORIZATION, value); + } + } + + Ok(out) +} + +/// Whether a header carries a client credential we must strip before +/// forwarding (the guest's capability token arrives in one of these). +fn is_credential_header(name: &HeaderName) -> bool { + name == AUTHORIZATION || name.as_str() == "x-api-key" +} + +/// Connection-scoped headers that must not be forwarded across the proxy hop. +fn is_hop_by_hop(name: &HeaderName) -> bool { + const HOP_BY_HOP: [&str; 9] = [ + "connection", + "proxy-connection", + "keep-alive", + "transfer-encoding", + "te", + "trailer", + "upgrade", + "proxy-authenticate", + "proxy-authorization", + ]; + HOP_BY_HOP.contains(&name.as_str()) +} + +/// Reduce a request URI to origin form (path + query only) for the upstream +/// HTTP/1.1 request line; the upstream host is carried by the `Host` header. +fn origin_form(uri: &Uri) -> Result { + let pq = uri.path_and_query().map_or("/", |p| p.as_str()); + pq.parse::().context("invalid path-and-query") +} + +fn error_response(status: StatusCode, msg: &'static str) -> Response { + let body = Full::new(Bytes::from_static(msg.as_bytes())) + .map_err(|never: Infallible| match never {}) + .boxed(); + #[expect( + clippy::expect_used, + reason = "status/body are constants; builder cannot fail" + )] + Response::builder() + .status(status) + .body(body) + .expect("static error response is always valid") +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "tests")] +mod tests { + use super::*; + + fn hv(s: &str) -> HeaderValue { + HeaderValue::from_str(s).unwrap() + } + + fn api_key_injection(secret: &str) -> Injection { + let json = format!(r#"{{ "scheme": "x_api_key", "credential": "{secret}" }}"#); + serde_json::from_str(&json).unwrap() + } + + fn bearer_injection(secret: &str) -> Injection { + let json = format!(r#"{{ "scheme": "bearer", "credential": "{secret}" }}"#); + serde_json::from_str(&json).unwrap() + } + + // ── capability token ───────────────────────────────────── + + #[test] + fn constant_time_eq_matches_and_differs() { + assert!(constant_time_eq(b"abc", b"abc")); + assert!(!constant_time_eq(b"abc", b"abd")); + assert!(!constant_time_eq(b"abc", b"ab")); + assert!(!constant_time_eq(b"", b"x")); + assert!(constant_time_eq(b"", b"")); + } + + #[test] + fn presented_token_reads_bearer() { + let mut h = HeaderMap::new(); + h.insert(AUTHORIZATION, hv("Bearer tok-123")); + assert_eq!(presented_token(&h), Some(b"tok-123".to_vec())); + } + + #[test] + fn presented_token_reads_x_api_key() { + let mut h = HeaderMap::new(); + h.insert(x_api_key(), hv("tok-456")); + assert_eq!(presented_token(&h), Some(b"tok-456".to_vec())); + } + + #[test] + fn presented_token_bearer_wins_over_x_api_key() { + let mut h = HeaderMap::new(); + h.insert(AUTHORIZATION, hv("Bearer from-auth")); + h.insert(x_api_key(), hv("from-xapi")); + assert_eq!(presented_token(&h), Some(b"from-auth".to_vec())); + } + + #[test] + fn presented_token_none_when_absent_or_wrong_scheme() { + let mut h = HeaderMap::new(); + assert_eq!(presented_token(&h), None); + h.insert(AUTHORIZATION, hv("Basic abc")); + assert_eq!(presented_token(&h), None); + } + + #[test] + fn authorized_gate() { + let mut h = HeaderMap::new(); + h.insert(AUTHORIZATION, hv("Bearer right")); + assert!(authorized(&h, "right")); + assert!(!authorized(&h, "wrong")); + assert!(!authorized(&HeaderMap::new(), "right")); + } + + // ── header rewrite ─────────────────────────────────────── + + #[test] + fn rewrite_strips_guest_credentials_and_injects_api_key() { + let mut incoming = HeaderMap::new(); + incoming.insert(AUTHORIZATION, hv("Bearer capability-token")); + incoming.insert("x-api-key", hv("capability-token")); + incoming.insert("anthropic-version", hv("2023-06-01")); + incoming.insert("content-type", hv("application/json")); + incoming.insert(HOST, hv("172.16.0.1:8788")); + + let out = build_upstream_headers( + &incoming, + "api.anthropic.com", + &api_key_injection("sk-real"), + ) + .unwrap(); + + assert_eq!(out.get("x-api-key").unwrap(), "sk-real"); + assert!( + out.get(AUTHORIZATION).is_none(), + "guest bearer must be stripped" + ); + assert_eq!(out.get(HOST).unwrap(), "api.anthropic.com"); + assert_eq!(out.get("anthropic-version").unwrap(), "2023-06-01"); + assert_eq!(out.get("content-type").unwrap(), "application/json"); + } + + #[test] + fn rewrite_injects_bearer_and_replaces_guest_authorization() { + let mut incoming = HeaderMap::new(); + incoming.insert(AUTHORIZATION, hv("Bearer capability-token")); + let out = build_upstream_headers( + &incoming, + "api.anthropic.com", + &bearer_injection("setup-tok"), + ) + .unwrap(); + assert_eq!(out.get(AUTHORIZATION).unwrap(), "Bearer setup-tok"); + assert!(out.get("x-api-key").is_none()); + } + + #[test] + fn rewrite_marks_injected_credential_sensitive() { + let mut incoming = HeaderMap::new(); + incoming.insert(AUTHORIZATION, hv("Bearer t")); + let out = build_upstream_headers( + &incoming, + "api.anthropic.com", + &api_key_injection("sk-real"), + ) + .unwrap(); + assert!(out.get("x-api-key").unwrap().is_sensitive()); + } + + #[test] + fn rewrite_drops_hop_by_hop_headers() { + let mut incoming = HeaderMap::new(); + incoming.insert(AUTHORIZATION, hv("Bearer t")); + incoming.insert("connection", hv("keep-alive")); + incoming.insert("keep-alive", hv("timeout=5")); + incoming.insert("proxy-authorization", hv("Basic xyz")); + let out = build_upstream_headers(&incoming, "api.anthropic.com", &api_key_injection("k")) + .unwrap(); + assert!(out.get("connection").is_none()); + assert!(out.get("keep-alive").is_none()); + assert!(out.get("proxy-authorization").is_none()); + } + + #[test] + fn rewrite_overrides_guest_host_even_without_incoming_host() { + let incoming = HeaderMap::new(); + let out = build_upstream_headers(&incoming, "api.anthropic.com", &api_key_injection("k")) + .unwrap(); + assert_eq!(out.get(HOST).unwrap(), "api.anthropic.com"); + } + + // ── uri origin form ────────────────────────────────────── + + #[test] + fn origin_form_keeps_path_and_query() { + let uri: Uri = "http://172.16.0.1:8788/v1/messages?beta=true" + .parse() + .unwrap(); + assert_eq!( + origin_form(&uri).unwrap().to_string(), + "/v1/messages?beta=true" + ); + } + + #[test] + fn origin_form_defaults_empty_path_to_root() { + let uri: Uri = "http://172.16.0.1:8788".parse().unwrap(); + assert_eq!(origin_form(&uri).unwrap().to_string(), "/"); + } +} diff --git a/coop-proxy/src/testdata/untrusted_upstream_cert.der b/coop-proxy/src/testdata/untrusted_upstream_cert.der new file mode 100644 index 0000000..a479cfe Binary files /dev/null and b/coop-proxy/src/testdata/untrusted_upstream_cert.der differ diff --git a/coop-proxy/src/testdata/untrusted_upstream_key.pkcs8.der b/coop-proxy/src/testdata/untrusted_upstream_key.pkcs8.der new file mode 100644 index 0000000..f0fdda6 Binary files /dev/null and b/coop-proxy/src/testdata/untrusted_upstream_key.pkcs8.der differ diff --git a/coop-proxy/src/tls.rs b/coop-proxy/src/tls.rs new file mode 100644 index 0000000..e5ee254 --- /dev/null +++ b/coop-proxy/src/tls.rs @@ -0,0 +1,96 @@ +//! TLS for the proxy→upstream hop. +//! +//! The guest→proxy hop is plain HTTP on the private host-guest link (no TLS, +//! no CA — the guest is explicitly pointed at the proxy). Only this hop is +//! TLS: a normal outbound HTTPS connection to the pinned upstream, its +//! certificate verified against the compiled-in Mozilla root set +//! ([`webpki_roots`]). A bug that disabled this verification would MITM the +//! real credential, so the configuration is deliberately minimal and has no +//! path that skips verification. + +use std::sync::Arc; + +use anyhow::Result; +use rustls::{ClientConfig, RootCertStore}; + +/// Build the shared client TLS configuration: standard certificate +/// verification against the pinned `webpki-roots`, no client auth. +pub fn client_config() -> Result> { + // Install the aws-lc-rs provider as the process default so the builder + // below (and any downstream construction) resolves it unambiguously even + // though it is the only provider compiled in. + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + let mut roots = RootCertStore::empty(); + roots.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + if roots.is_empty() { + anyhow::bail!("no trust anchors compiled in — refusing to run without a root store"); + } + + let config = ClientConfig::builder() + .with_root_certificates(roots) + .with_no_client_auth(); + + Ok(Arc::new(config)) +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "tests")] +mod tests { + use super::*; + + #[test] + fn client_config_builds_with_pinned_roots() { + // Construction succeeds (the unwrap): the non-empty root store is + // enforced by the bail in `client_config`. That the resulting verifier + // actually rejects an untrusted cert is covered by + // `rejects_untrusted_upstream_cert` below. + client_config().unwrap(); + } + + // A self-signed `CN=localhost` cert + its PKCS#8 key, minted once with + // openssl (valid to 2126) so the test needs no cert-generation crate. It + // chains to no pinned root, so a correct verifier must reject it. + const UNTRUSTED_CERT_DER: &[u8] = include_bytes!("testdata/untrusted_upstream_cert.der"); + const UNTRUSTED_KEY_DER: &[u8] = include_bytes!("testdata/untrusted_upstream_key.pkcs8.der"); + + // The Tier-1 property: the proxy→upstream hop must reject a certificate + // that does not chain to the pinned roots. Stand up a TLS server with the + // untrusted self-signed cert and assert the config built by + // `client_config()` refuses the handshake. A regression that disabled + // verification (e.g. a `dangerous()` verifier) would let the bad cert + // through and fail this test. + #[tokio::test] + async fn rejects_untrusted_upstream_cert() { + use rustls::pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs8KeyDer, ServerName}; + use tokio::net::{TcpListener, TcpStream}; + use tokio_rustls::{TlsAcceptor, TlsConnector}; + + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + let server_cfg = rustls::ServerConfig::builder() + .with_no_client_auth() + .with_single_cert( + vec![CertificateDer::from(UNTRUSTED_CERT_DER)], + PrivateKeyDer::Pkcs8(PrivatePkcs8KeyDer::from(UNTRUSTED_KEY_DER)), + ) + .unwrap(); + let acceptor = TlsAcceptor::from(Arc::new(server_cfg)); + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + if let Ok((stream, _)) = listener.accept().await { + let _ = acceptor.accept(stream).await; + } + }); + + let connector = TlsConnector::from(client_config().unwrap()); + let tcp = TcpStream::connect(addr).await.unwrap(); + let name = ServerName::try_from("localhost").unwrap(); + assert!( + connector.connect(name, tcp).await.is_err(), + "self-signed upstream cert must be rejected by the pinned-root verifier" + ); + } +} diff --git a/coop-proxy/tests/gate.rs b/coop-proxy/tests/gate.rs new file mode 100644 index 0000000..50bd9fe --- /dev/null +++ b/coop-proxy/tests/gate.rs @@ -0,0 +1,148 @@ +//! End-to-end tests of the capability-token gate and the bind guard, driven +//! against the real `coop-proxy` binary — config fed over stdin, requests over +//! a loopback socket, exactly as `coop` runs it. +//! +//! These assert the security-critical refusal path without a live upstream: a +//! request that fails the capability check is rejected with 401 and the +//! upstream is never contacted; a request that passes fails closed at the +//! (unresolvable) upstream with 502, proving the gate opened without any real +//! credential reaching a real service. The authorized header-rewrite/injection +//! logic is covered by the unit tests in `src/proxy.rs`, and end-to-end against +//! a mock upstream by coop's VM integration suite. + +#![expect(clippy::unwrap_used, reason = "tests")] +#![expect(clippy::expect_used, reason = "tests")] +#![expect(clippy::panic, reason = "tests")] + +use std::net::SocketAddr; +use std::process::Stdio; +use std::time::Duration; + +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpStream; +use tokio::process::{Child, Command}; + +const BIN: &str = env!("CARGO_BIN_EXE_coop-proxy"); + +/// A config whose upstream can never resolve, so any request that passes the +/// gate fails closed rather than reaching a real service. +fn config_json(listen: &str) -> String { + format!( + r#"{{ + "listen": "{listen}", + "capability_token": "the-right-token", + "upstream_host": "proxy-test.invalid", + "injection": {{ "scheme": "x_api_key", "credential": "sk-should-never-leave" }} + }}"# + ) +} + +async fn free_loopback_addr() -> SocketAddr { + let probe = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = probe.local_addr().unwrap(); + drop(probe); + addr +} + +/// Spawn the proxy binary, feed it `listen` config on stdin, and wait until it +/// accepts connections on `addr`. +async fn spawn_serving(addr: SocketAddr) -> Child { + // `--no-jail`: this test drives the gate logic, not the jail, and runs on + // CI hosts that may lack Landlock (where the fail-closed jail would abort + // startup). coop never passes this flag; the jail is asserted separately by + // `--jail-selftest` in the VM integration suite. + let mut child = Command::new(BIN) + .arg("--no-jail") + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .kill_on_drop(true) + .spawn() + .unwrap(); + let mut stdin = child.stdin.take().unwrap(); + stdin + .write_all(config_json(&addr.to_string()).as_bytes()) + .await + .unwrap(); + drop(stdin); + + for _ in 0..200 { + if TcpStream::connect(addr).await.is_ok() { + return child; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + panic!("proxy did not start listening on {addr}"); +} + +/// Send a minimal HTTP/1.1 request and return the status line. +async fn request_status(addr: SocketAddr, auth_header: Option<&str>) -> String { + let mut stream = TcpStream::connect(addr).await.unwrap(); + let mut req = String::from("GET /v1/messages HTTP/1.1\r\nHost: proxy\r\n"); + if let Some(h) = auth_header { + req.push_str(h); + req.push_str("\r\n"); + } + req.push_str("Connection: close\r\n\r\n"); + stream.write_all(req.as_bytes()).await.unwrap(); + + let mut buf = Vec::new(); + let _ = tokio::time::timeout(Duration::from_secs(20), stream.read_to_end(&mut buf)).await; + let text = String::from_utf8_lossy(&buf); + text.lines().next().unwrap_or_default().to_string() +} + +#[tokio::test] +async fn rejects_missing_token_with_401() { + let addr = free_loopback_addr().await; + let _child = spawn_serving(addr).await; + let status = request_status(addr, None).await; + assert!(status.contains("401"), "expected 401, got: {status:?}"); +} + +#[tokio::test] +async fn rejects_wrong_token_with_401() { + let addr = free_loopback_addr().await; + let _child = spawn_serving(addr).await; + let status = request_status(addr, Some("Authorization: Bearer wrong-token")).await; + assert!(status.contains("401"), "expected 401, got: {status:?}"); +} + +#[tokio::test] +async fn valid_token_passes_gate_and_fails_closed_at_upstream() { + let addr = free_loopback_addr().await; + let _child = spawn_serving(addr).await; + let status = request_status(addr, Some("Authorization: Bearer the-right-token")).await; + assert!(status.contains("502"), "expected 502, got: {status:?}"); +} + +#[tokio::test] +async fn refuses_to_bind_unspecified_address() { + // `--no-jail` so the bind guard is asserted regardless of host Landlock + // support (see spawn_serving). + let mut child = Command::new(BIN) + .arg("--no-jail") + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .unwrap(); + let mut stdin = child.stdin.take().unwrap(); + stdin + .write_all(config_json("0.0.0.0:0").as_bytes()) + .await + .unwrap(); + drop(stdin); + + let output = tokio::time::timeout(Duration::from_secs(20), child.wait_with_output()) + .await + .expect("proxy should exit promptly on a bad bind address") + .unwrap(); + assert!(!output.status.success(), "proxy should exit non-zero"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("unspecified"), + "expected unspecified-bind refusal, got: {stderr}" + ); +} diff --git a/deny.toml b/deny.toml index fd469c3..0fc6a2e 100644 --- a/deny.toml +++ b/deny.toml @@ -2,9 +2,27 @@ ignore = [] [licenses] -allow = ["MIT", "Apache-2.0", "MPL-2.0", "Unicode-3.0"] +allow = ["MIT", "Apache-2.0", "MPL-2.0", "Unicode-3.0", "ISC"] confidence-threshold = 0.8 +# Crate-scoped license exceptions keep the global allowlist tight: these +# licenses appear only in the credential-proxy's TLS stack (issue #411), so +# they are permitted for exactly the crates that carry them. +[[licenses.exceptions]] +# FIPS-capable crypto backend for rustls (the recommended provider). +crate = "aws-lc-sys" +allow = ["BSD-3-Clause"] + +[[licenses.exceptions]] +# Pinned Mozilla root set compiled into coop-proxy (a data license). +crate = "webpki-roots" +allow = ["CDLA-Permissive-2.0"] + +[[licenses.exceptions]] +# Constant-time primitives used by the rustls/aws-lc-rs crypto stack. +crate = "subtle" +allow = ["BSD-3-Clause"] + [licenses.private] ignore = true @@ -12,6 +30,13 @@ ignore = true multiple-versions = "warn" wildcards = "deny" highlight = "all" +# Keep the OpenSSL / system-TLS path out of the graph so nothing silently +# pulls it in behind the rustls + aws-lc-rs stack. +deny = [ + { crate = "openssl" }, + { crate = "openssl-sys" }, + { crate = "native-tls" }, +] [sources] unknown-registry = "deny" diff --git a/docs/commands.md b/docs/commands.md index f994a3c..e047a62 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -922,6 +922,32 @@ coop github forget-pat --repo trailofbits/coop `file`, or `null` when unparseable) and `probe` (`ok`/`unexpected_format`/ `resolve_failed`, or `null` unless `--probe`). The token value is never emitted. +### `proxy` + +Manage the host-side credential-injecting proxy. When a `[proxy.]` +upstream is configured, coop runs a `coop-proxy` process on the host for the +lifetime of each remote-mode VM: the guest is pointed at the proxy and holds +only a per-instance capability token, while the real API key stays on the host +and is injected onto outbound requests the guest never sees. Applies only in +remote model mode (`coop model remote`); local mode takes precedence. See +the [credential proxy guide](credential-proxy.md) and the +[`proxy` configuration reference](configuration.md#proxy-section) for the data +model. + +| Subcommand | Effect | +|------------|--------| +| `setup [--anthropic] [--openai] [--vm ] [--api-key]` | Store a provider credential in a secret backend and wire it into `[proxy.]` (the default) or a per-VM override. Anthropic (Claude) is the default provider; pass `--openai` for Codex. `--vm ` stores the credential as a per-VM override in that instance's state instead of the global default. Anthropic only: `--api-key` stores an API key (`x-api-key`) instead of a Claude `setup-token`; ignored for `--openai`, whose keys are always injected as `Authorization: Bearer`. | +| `status [--vm ]` | Show what each VM's agents resolve to (per-VM override → default → off), with credentials redacted. Pass `--vm ` for the effective resolution of a single VM instead of all. | + +``` +coop proxy setup +coop proxy setup --openai +coop proxy setup --vm my-project +coop proxy setup --api-key +coop proxy status +coop proxy status --vm my-project +``` + ### `validate` Check the configuration file and prerequisites. Prints warnings and confirms the config loads correctly. With `--probe`, also exercises each `[github.pat]` entry against `api.github.com` to confirm the token is still live. diff --git a/docs/configuration.md b/docs/configuration.md index 74a9cfb..642865e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -286,6 +286,46 @@ sections of [docs/claude-integration.md](claude-integration.md) and [docs/codex-integration.md](codex-integration.md) for how each endpoint is materialized into guest config. +## `proxy` section + +`[proxy.anthropic]` and `[proxy.openai]` declare host-side +credential-injecting upstreams for Claude Code and Codex. When an upstream is +configured, coop runs a `coop-proxy` process on the host for the lifetime of +each remote-mode VM: the guest is pointed at the proxy (a base-URL override) and +holds only a per-instance capability token, while the real credential stays on +the host and is injected onto outbound requests the guest never sees. Absent +config means no proxy — credentials are forwarded into the guest exactly as +before. + +Proxy mode applies only in remote model mode +([`coop model remote`](commands.md#model)); local mode takes precedence. +Each provider is an optional default, and a VM can override its own credential +per provider with [`coop proxy setup --vm `](commands.md#proxy) (stored in +the instance's `proxy.json`, not in the config file). + +Both blocks take the same fields: + +| Field | Type | Default | Description | +|-------|------|---------|-------------| +| `credential` | string | required | The real credential — an API key or a Claude `setup-token`. A plain value or a `cmd:` invocation resolved at proxy start. Never written to disk and never forwarded into the guest. | +| `auth` | `api_key` \| `bearer` | `api_key` | How the proxy injects the credential upstream. `api_key` sends `x-api-key: ` (the Anthropic API-key form); `bearer` sends `Authorization: Bearer `, used for a Claude `setup-token`. OpenAI keys are always Bearer. | + +```toml +[proxy.anthropic] +credential = "cmd:op read op://Private/Anthropic/credential" +auth = "api_key" + +[proxy.openai] +credential = "cmd:op read op://Private/OpenAI/credential" +auth = "bearer" +``` + +`coop proxy setup` writes these entries for you: it takes a pasted credential, +stores it in a secret manager (Keychain / Secret Service / 1Password / a `0600` +file) and fills in the `cmd:` reference. See the +[credential proxy guide](credential-proxy.md) and +[`coop proxy`](commands.md#proxy) for the workflow. + ## `profiles` section Custom installation profiles for `coop setup --profile `. Each profile declares packages and scripts that run during rootfs template creation. diff --git a/docs/credential-proxy.md b/docs/credential-proxy.md new file mode 100644 index 0000000..fb69da3 --- /dev/null +++ b/docs/credential-proxy.md @@ -0,0 +1,131 @@ +# Credential-injecting proxy (`[proxy]`) + +Status: **v1 — Anthropic (Claude Code) and OpenAI (Codex); Firecracker and +Lima.** Opt-in; off by default. Design: +[`design/issue-411-injecting-proxy.md`](design/issue-411-injecting-proxy.md). + +## What it does + +Without proxy mode, coop forwards the raw `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` +into the guest as SSH `SendEnv` variables — a prompt-injected or rogue agent can +read them from its own environment and, with egress open, exfiltrate them. + +With proxy mode, the raw credential **never enters the guest**. coop runs a +small host-side reverse proxy (`coop-proxy`) — one process per (VM, provider) — +for the lifetime of the VM: + +- The proxy binds host loopback and is exposed into the guest by a per-instance + `ssh -R` reverse tunnel; the guest is pointed at `http://127.0.0.1:` and + holds only a **per-instance capability token**. + - **Claude Code:** `ANTHROPIC_BASE_URL` + `ANTHROPIC_AUTH_TOKEN` in the + managed `~/.claude/settings.json`. + - **Codex:** a `[model_providers.coop_local]` block in `~/.codex/config.toml` + (`base_url` at the proxy, `wire_api = "responses"`) with the capability + token supplied as the provider's bearer `env_key`. Proxy mode pins no model + — Codex keeps its own, only its egress is redirected. +- The proxy verifies that token (constant-time), strips it, injects the real + credential (`x-api-key`, or `Authorization: Bearer`), and streams the request + to the pinned upstream (`api.anthropic.com` / `api.openai.com`) over TLS. +- The raw `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` is no longer forwarded, and in + proxy mode Codex's `~/.codex/auth.json` is **not** staged onto the guest disk + (it holds a refreshable subscription token — the exact at-rest exposure #411 + closes). Codex subscription is therefore out of scope in proxy mode; use an + OpenAI API key. + +The capability token is worthless off the host — it only authorizes the local +proxy, which holds the real key itself — so exfiltrating it gains a compromised +guest nothing. + +## Enabling it + +The easiest path is `coop proxy setup`: it takes a pasted credential, stores it +in a secret backend of your choice (macOS Keychain / Linux secret-service / +1Password / a 0600 file), and writes the `[proxy.]` block for you with +a `cmd:` reference — so the credential is never plaintext in the config. + +- **Anthropic (default):** `coop proxy setup` takes a Claude `setup-token` + (subscription) or an API key with `--api-key`. To generate a token first, run + `claude setup-token` on the host (needs a Claude subscription; the token is + inference-scoped, ~1 year). +- **OpenAI (Codex):** `coop proxy setup --openai` takes an OpenAI API key + (always injected as `Authorization: Bearer`). + +Or configure it by hand: + +```toml +[proxy.anthropic] +credential = "cmd:op read op://Private/Anthropic/credential" # or a plain key +auth = "api_key" # api_key → x-api-key (default); bearer → a Claude setup-token + +[proxy.openai] +credential = "cmd:op read op://Private/OpenAI/credential" +auth = "bearer" # OpenAI keys inject as Authorization: Bearer +``` + +The `credential` uses the same `cmd:` resolution as every other coop secret and +is resolved on the **host** at VM start. For Claude subscription billing without +exposure, run `claude setup-token` on the host, stash the printed one-year +token, reference it via `cmd:`, and set `auth = "bearer"`. + +## Per-VM credential overrides + +The `[proxy.]` blocks are the **defaults** for every VM. A single VM +can use a different credential — for per-project billing, scope, or revocation — +with `coop proxy setup --openai --vm ` (or `--anthropic --vm `). The +override is stored in that instance's state (`/proxy.json`), not in a +growing config table, and its secret is namespaced separately (`coop-openai-`). + +Resolution per provider is **override → default → off**: the per-VM override +wins, else the config default, else the proxy is off for that provider. This is +purely host-side credential selection — the proxy binary and the per-VM +capability token are unchanged, so there is no new attack surface. + +`coop proxy status` shows the defaults and every VM's overrides; `coop proxy +status --vm ` shows one VM's effective resolution. Credentials are shown +as their `cmd:` reference (a command, not the secret); a hand-written literal is +redacted. + +Proxy mode applies only in **remote** model mode. `coop model local` takes +precedence (the VM routes at your local model server and the proxy is torn +down). If credential resolution fails at start, the VM **fails closed** — it +does not come up on a path where the agent silently has no or the wrong key. + +## What it does and does not guarantee + +It stops a rogue agent from **reading a usable key** from its environment or +disk. It does **not**: + +- Stop **use** of the credential while the agent runs — the agent still makes + model calls through the proxy (that is the point). Non-exposure limits + exfiltration of the raw key, not use during the session. +- Provide **egress** control — a token-less agent with open egress can still + reach arbitrary hosts (issue #2). The proxy composes with, but does not + replace, "no route out except the proxy." +- Provide **scope** enforcement — the injected key keeps its full account scope + (issue #73). Non-exposure and scope are orthogonal. + +The proxy itself is new attack surface, mitigated by a fixed per-route upstream +(the guest controls only the path, never the host — closing SSRF), TLS +verification against a pinned root set, a required capability token, and +resource limits. The listener binds only host loopback and is reverse-tunnelled +to exactly one guest — never a non-loopback interface, never the LAN. + +It is also **jailed** to bound the blast radius of a proxy exploit: `coop-proxy` +runs confined so it cannot write files, execute programs, or reach any host +beyond the upstream `:443` and DNS `:53`. On Linux this is Landlock (self-applied +before serving); on macOS it is a Seatbelt profile applied via `sandbox-exec`. +The confinement is **fail-closed** — if it cannot be established the VM start +aborts rather than running the credential-holding proxy unconfined. The jail is +port-scoped, not host-scoped (the two upstreams' identity is enforced by the +proxy's TLS verification, not the jail), and does not restrict UDP on Linux; see +[`trust-model.md`](trust-model.md) for the full threat model and the accepted +limitations. + +## Platform support + +**Firecracker (Linux) and Lima (macOS).** The proxy binds `127.0.0.1` on the +host and is exposed into the guest with a per-instance `ssh -R` reverse tunnel, +so it works identically on both backends (each already keeps an SSH channel to +its guest). The host-side proxy process is jailed on both — Landlock on Linux +(host kernel ≥6.7), Seatbelt via `sandbox-exec` on macOS. Not yet built: +GitHub. diff --git a/docs/design/issue-411-injecting-proxy.md b/docs/design/issue-411-injecting-proxy.md new file mode 100644 index 0000000..49ebd99 --- /dev/null +++ b/docs/design/issue-411-injecting-proxy.md @@ -0,0 +1,788 @@ +# Design: Issue #411 — credential non-exposure via a host-side injecting proxy + +**Status:** proposal · **Scope:** the credential-injection mechanism and its Claude↔Codex parity; egress filtering (#2) and subscription-token injection are adjacent, treated only where they touch this seam +**Author:** design pass for #411 · **Date:** 2026-07-16 + +--- + +## 0. TL;DR + +- **Build it host-side, per-integration, not as a helper VM and not (yet) as a + general MITM proxy.** The secret already lives on the host today (resolved via + `cmd:` and forwarded in); a host-side injecting proxy keeps it exactly where it + is and simply stops forwarding it inward. A helper VM is the *correct* home only + for a general arbitrary-upstream proxy, and only on Firecracker — see §3. +- **The per-integration proxy needs no MITM CA.** Because the guest is + *explicitly* pointed at the proxy via `ANTHROPIC_BASE_URL` / + `[model_providers.*].base_url`, the guest→proxy hop is a configured endpoint on + the private host-guest link (plain HTTP), and only the proxy→upstream hop is + TLS. The "guest-trusted MITM CA" the issue flags is a *general-proxy* cost, not + a per-integration one (§6). This is the single biggest reason to start + per-integration. +- **Claude↔Codex parity is clean for the API-key path and reuses config surfaces + coop already writes** (`claude_env_block`, `codex_local_config`). Both agents + support a base-URL override plus a "guest holds no real credential" disposition; + the proxy injects the real key upstream. See §5. +- **Subscription auth splits by vendor, and it collapses to "inject a static + token" — no OAuth-refresh code (§7).** Claude ships an official one-year + `setup-token`, so the proxy injects it like any bearer (this is also #62's + non-exposure answer). Codex ships *no* long-lived token — only a client-refreshed + `auth.json` that OpenAI itself steers away from for automation — so Codex + subscription is out of scope for the proxy and routes to API-key mode. Both + agents get full non-exposure via API keys; only Codex-subscription would have + required refresh logic, and it is deliberately not built. v1 also **stops staging + `~/.codex/auth.json` onto the guest disk** (`backend.rs:2326`) when proxy mode is + on, closing an existing at-rest exposure. +- **Ship order:** Anthropic first (the `ANTHROPIC_BASE_URL` rewrite seam already + exists), Codex second (the `[model_providers.coop_local]` seam already exists), + GitHub later and separately (it carries the #73 GraphQL-bypass caveat and has no + base-URL override — §8). + +--- + +## 1. What "credential non-exposure" must guarantee + +The property #411 asks for: **a prompt-injected or rogue agent inside the guest +cannot read a usable upstream credential.** Today it can — every path lands a raw +secret in the guest: + +- `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GITHUB_TOKEN` arrive as SSH `SendEnv` + environment variables, present in the agent's own process environment + (`backend.rs:1356-1410`). +- The local-model Claude token rides inside the guest's `~/.claude/settings.json` + (`backend.rs:1841-1858`). +- **Codex's subscription credential `~/.codex/auth.json` is copied verbatim onto + the guest disk** (`backend.rs:2326`, `CODEX_ALLOWED_FILES`). + +The VM boundary does not help here: egress is open today (#2), so a token the +agent can read is a token it can exfiltrate. Non-exposure means the credential +never enters the guest in usable form — the host (or a host-controlled boundary) +holds it and attaches it to upstream requests the guest cannot see. + +What non-exposure explicitly does **not** claim (see §9): it is not *scope* +enforcement (#73) and not *egress* control (#2). It reduces what a compromised +agent can steal; it composes with, but does not replace, those two. + +--- + +## 2. What exists today (the seams this reuses) + +The exploration behind this doc established the following (file:line verified): + +1. **There is no host-side proxy process today.** Local-model mode rewrites the + base URL to point the guest *directly* at a user-run server on the host + (`network::rewrite_host_url`, `network.rs:22-43`). coop is not in the data + path. So #411 is net-new plumbing, not "flip on the existing relay." + +2. **The bind point is free.** The guest already reaches the host at a + backend-specific gateway: `network.host_ip` (default `172.16.0.1`) on + Firecracker (`backend.rs:1104-1107`), `host.lima.internal` on Lima + (`backend.rs:1287-1290`, `lima.rs:27`), both surfaced through + `VmBackend::guest_host_address` (`backend.rs:869`). Any host listener on that + address is reachable from the guest with zero new networking. + +3. **Both agents already accept a base-URL override that coop writes.** + - Claude: `claude_env_block` (`model_state.rs:175-197`) writes + `ANTHROPIC_BASE_URL` + `ANTHROPIC_AUTH_TOKEN` into the managed + `settings.json`. + - Codex: `codex_local_config` (`model_state.rs:205-238`) writes a + `[model_providers.coop_local]` block with `base_url`, `wire_api = + "responses"`, `env_key = "COOP_LOCAL_API_KEY"`, merged into + `~/.codex/config.toml` by a read-merge-write that preserves user/guest config + (`stage_codex_files`, `backend.rs:2351-2453`). + +4. **A "dummy token" convention already exists.** `LOCAL_MODEL_AUTH_FALLBACK = + "coop-local"` (`config.rs:1598`) is the placeholder token used when a local + endpoint needs no real credential. The proxy design generalizes exactly this: + the guest gets a dummy (or nothing), the proxy holds the real key. + +5. **Port forwarding is the wrong direction but the right pattern.** + `port_forward.rs` uses `ssh -L` (host→guest); a proxy needs the guest to reach + a host listener (already available via the gateway, item 2). The module's + control-socket lifecycle pattern (`forwards.sock`, `-O exit`) is reusable for + managing a per-instance proxy process. + +The upshot: the guest-config half of this feature is **already built** for the +two model APIs. The net-new work is the host-side proxy process and its lifecycle. + +--- + +## 3. Architecture: host-side process vs. helper VM + +This is the decision the user raised. It is *not* primarily about where the secret +rests (the host already holds every secret). It is about **where the proxy's +attack surface lands when the untrusted guest attacks it** — the proxy is the one +new component that terminates a connection and parses HTTP originated by the guest. + +| | Host-side process | Helper VM | +|---|---|---| +| Blast radius of a proxy exploit | **The whole host** — arbitrary code-exec on the user's machine: every other secret, SSH keys, ability to tamper with coop | **A disposable VM** — the guest gains the injected credentials + egress (which it was spending anyway) but still has no path to the host | +| Secret location vs. today | unchanged (host) | moved into a new VM | +| New machinery | one host process + lifecycle | a second VM image, boot, inter-VM networking, resource cost | +| Firecracker fit | good (can additionally jail: uid/netns, jailer already present) | natural and cheap (microVMs are the point) | +| Lima/macOS fit | good (host process on `host.lima.internal`) | **awkward** — no first-class inter-VM networking, second Virtualization.framework VM is heavy against the #404 memory floor | +| Composes with #2 "no route out" | via a single host-local pinhole | as the sole egress terminus | + +The helper VM genuinely downgrades the worst case from **host compromise** to +**credential + egress compromise** — a real, principled win, and coop's founding +move ("the VM is the boundary") applied a second time. But it is justified only +when the proxy's attack surface is large, and it splits the backends: clean on +Firecracker, fighting-the-platform on Lima, which violates the `backend.rs` +shared-abstraction contract coop leans on. + +**Recommendation: host-side.** For the per-integration proxy the attack surface is +small — a reverse proxy to *known* HTTPS upstreams on a mature stack +(hyper/rustls), fed a request the guest composed. On Firecracker, jail it +(separate uid + network namespace) to claw back most of the host-exposure con +without a whole VM; the jailer machinery already exists. Reach for the helper VM +**only** if coop commits to the general MITM proxy (§4), where terminating TLS for +arbitrary upstreams behind a guest-trusted CA is a large enough surface to deserve +its own boundary — and accept at that point that it is a Firecracker-first feature. + +--- + +## 4. General proxy vs. per-integration (the issue's open question) + +The two axes correlate, so decide them together: + +| | Per-integration (recommended v1) | General MITM proxy | +|---|---|---| +| Coverage | the integrations we wire (model APIs, then GitHub) | every upstream uniformly | +| TLS | **none to terminate** — guest is explicitly pointed at the proxy via `base_url`; guest→proxy is plain HTTP on the private link, proxy→upstream is normal HTTPS | must terminate TLS for arbitrary hosts → **guest-trusted MITM CA to mint, install, and manage** | +| Per-service rules | one small injection rule per integration | a rule engine + CA + cert cache | +| Home (§3) | host-side | helper VM | +| Gap | any integration without a bespoke path still hands the agent a raw key | none, but far bigger build | + +The decisive asymmetry is the **MITM CA**. A general transparent proxy has to +impersonate `api.anthropic.com`, `github.com`, `registry.npmjs.org`, … to the +guest, which means a coop CA the guest trusts — a new long-lived root credential +to manage and a new thing that, if mis-scoped, breaks TLS trust in the guest. The +per-integration proxy sidesteps this entirely: the guest connects to a *declared* +proxy endpoint, not to an intercepted hostname, so there is nothing to +impersonate. + +**Recommendation: per-integration, host-side.** It is incremental, each slice is +independently shippable, it reuses seams that already exist (§2), and it avoids the +CA. Its one real cost — gaps for unwired integrations — is acceptable because the +highest-value credentials (the two model API keys) are exactly the ones with a +first-class base-URL override, and GitHub is separately addressable (§8). Revisit +the general proxy only if the gap list grows to where a uniform interceptor earns +its CA-management cost. + +--- + +## 5. Feature parity: Claude ↔ Codex + +Parity is a first-class requirement: Codex is a co-equal agent in coop. The +API-key path maps symmetrically onto both, reusing config surfaces coop already +generates. + +| | Claude Code | Codex | +|---|---|---| +| Point at proxy | `ANTHROPIC_BASE_URL` in managed `settings.json` (`claude_env_block`) | `[model_providers.coop].base_url` in `~/.codex/config.toml` (`codex_local_config`) | +| Guest-held credential | per-instance capability token in `ANTHROPIC_AUTH_TOKEN` | per-instance capability token via `env_key = "COOP_LOCAL_API_KEY"`, sent as `Authorization: Bearer` | +| Wire protocol | Messages API + SSE + tool-use round-trips | Responses API (`wire_api = "responses"`) + SSE | +| Proxy injects upstream | real key → `Authorization`/`x-api-key` → `api.anthropic.com` | real key → `Authorization: Bearer` → `api.openai.com` | + +Two facts from the Codex docs make this work and are worth pinning: + +- **A custom `[model_providers.*]` provider chooses what credential to send.** Codex + supports three dispositions: `requires_openai_auth = true` (use the ChatGPT/API + OpenAI token, `env_key` ignored), `env_key = "VAR"` (send that var as Bearer), + or **neither → no auth sent**. Proxy mode as shipped uses the `env_key` + disposition: `env_key = "COOP_LOCAL_API_KEY"` carries the per-instance + capability token as `Authorization: Bearer`, which the proxy verifies and + strips before injecting the real upstream credential. (Codex also supports + `http_headers` / `env_http_headers` and a command-backed + `[model_providers..auth]` refresh hook — not needed for v1 but relevant + to §7.) +- **The proxy stays protocol-agnostic.** It is a streaming reverse proxy that + injects an auth header per route and passes bytes through; it never parses + Messages-API vs Responses-API bodies. `wire_api` and endpoint paths are the + guest config's concern. This is *why one proxy covers both* — parity costs one + injection rule per upstream, not a second protocol implementation. + +**Parity verdict:** for the API-key path, full parity at every step, and most of +it is already coded (both base-URL writers exist; the dummy-token convention +exists). The proxy is the only shared new component. + +--- + +## 6. The proxy: shape and binding + +- **Type:** an explicit forward/reverse proxy, not a transparent interceptor. One + host process per running instance (or one shared process keyed by instance — + decide in the spike; per-instance is simpler to reason about and matches the + `forwards.sock` lifecycle pattern). +- **Listener:** binds the guest-visible gateway (`guest_host_address`) on a + loopback-scoped port. On Firecracker that is the bridge IP `172.16.0.1`; on Lima + the host side of `host.lima.internal`. Reachable from exactly one guest, not the + LAN. +- **Guest→proxy hop:** plain HTTP over the private host-guest link. No TLS, no CA. + The link is already private; #2's "no route out" can make it the *only* link. +- **Proxy→upstream hop:** normal outbound HTTPS to the pinned upstream + (`api.anthropic.com` / `api.openai.com`), with the real credential injected as + the upstream's expected header. The upstream host is fixed per route, not taken + from the guest — so the guest cannot retarget the injected key at an + attacker-controlled host. +- **Streaming:** must stream request and response bodies (SSE, tool-use + round-trips) rather than buffer — a hard requirement for both agents. A mature + async HTTP stack (hyper) handles this; the proxy adds a header and copies byte + streams. +- **Secret resolution:** the real key is resolved on the host at proxy start via + the existing `resolve_cmd_value` / secret-store machinery (`config.rs:34+`, + `secret_store.rs`) — no new secret-handling code, and the `cmd:` indirection + keeps plaintext off disk. +- **Guest config in proxy mode:** reuse `claude_env_block` / `codex_local_config` + with `base_url` = the proxy endpoint and the credential slot set to a per-instance + capability token (below). This is the same read-merge-write that already lands + local-model config (`stage_codex_files`), so it preserves user/guest config and + cleans up on mode switch. +- **The "dummy" token is a real per-instance capability.** Do not hand the guest a + fixed placeholder. Mint a random token at proxy start, give it to the guest as + `ANTHROPIC_AUTH_TOKEN` / the Codex provider bearer, and have the proxy require it + before injecting the real credential. This closes a real hole: a host-bound + listener is reachable by *any* local process, so without a check any process + could spend the user's account through the proxy. The capability is worthless off + the host (it only authorizes the local proxy, which injects the real key itself), + so exfiltrating it gains the agent nothing. Bind the listener to the guest-only + interface (bridge IP / the host side of `host.lima.internal`), never `0.0.0.0`; + on Firecracker, vsock is a stronger point-to-point option worth evaluating. + +--- + +## 6a. Implementation: reuse vs. build + +**Grounding fact:** coop today has *no* HTTP client, *no* async runtime, and *no* +TLS stack — `Cargo.toml` pulls only `url` among the relevant crates; the codebase +is synchronous and subprocess-oriented (`Cmd`, ssh, scp). Any proxy therefore +introduces a new async/HTTP/TLS dependency footprint into a deliberately lean, +security-critical tool. That, plus coop's single-binary distribution (it cross- +compiles and ships one artifact; it does not ask users to install and manage +daemons), drives the decision. + +**Recommendation: build a minimal, purpose-built reverse proxy in Rust as its own +workspace binary** (e.g. `coop-proxy`), which coop spawns and supervises like it +already spawns ssh. Build it on the *audited primitives* — `hyper` (HTTP/1.1 + +HTTP/2 + streaming bodies) and `rustls` (pure-Rust TLS, no C/boringssl build) on +`tokio` — and own only the thin coop-specific policy glue: route → fixed upstream, +inject one header, verify the capability token, stream bytes. Isolating it in a +separate binary keeps the main CLI's dependency surface unchanged, makes the +security-critical component independently auditable, and gives the Firecracker jail +(§3, slice 3) a natural unit to confine (separate uid/netns). + +Rejected alternatives, with reasons weighted for a security-critical host process +reachable by the untrusted guest and holding the raw credential: + +| Option | Why not | +|---|---| +| **LiteLLM proxy** (Python) | Ships a Python runtime + a multi-tenant gateway (DB, virtual keys, admin API) — enormous attack + supply-chain surface for a two-route header injector; breaks single-binary distribution | +| **oauth2-proxy** (Go) | Wrong direction — it authenticates *inbound* users to a protected app and injects *identity* headers, not *outbound* upstream API credentials; recent header-smuggling CVEs in exactly the injection path we'd rely on | +| **Envoy / nginx** | External daemon to install, configure, and keep patched on every user's host; C/C++ + a config language as attack surface; breaks single-binary distribution | +| **Pingora** (Rust framework) | Closest reasonable option, but a full load-balancer framework (routing, health checks, LB) with a large tree incl. boringssl (C build) — more surface than a fixed 2-route injector needs. Revisit *only* if coop builds the general MITM proxy (§4), where its TLS-termination/routing machinery would earn its keep | + +Non-negotiable either way: **do not hand-roll TLS, HTTP, or SSE framing.** Those +are the dangerous primitives; reuse the audited crates for them. What we write is +only the small, fixed policy layer — which is precisely why a purpose-built proxy +is *smaller* attack surface here than adopting a general framework, not larger. + +--- + +## 7. Auth modes: login flow, renewal, and official support + +The spike in §11.1 is resolved. The key result: **for every path except Codex +subscription, the proxy holds a *static, long-lived* credential and injects it — +there is no OAuth-refresh logic to write.** That is the security win, and it comes +from vendor-official mechanisms, not reverse engineering. + +| Mode | User setup (login flow) | Re-auth cadence | Auto-renews? | Officially supported? | +|---|---|---|---|---| +| **API key** (both agents) | none — supply the key via `cmd:` (Keychain / 1Password / 0600 file), exactly as coop resolves secrets today | never (until the user rotates the key) | n/a — no expiry | **Yes, fully.** Anthropic documents proxy routing via `ANTHROPIC_BASE_URL` + `ANTHROPIC_AUTH_TOKEN` ("routing through an LLM gateway or proxy"); Codex documents custom `[model_providers.*]` with a Bearer `env_key` | +| **Claude subscription** | run `claude setup-token` **once** on the host → stash the printed 1-year token → reference via `cmd:` | once a year (re-run `setup-token`) | no, but the token lives one year | **Yes** — `setup-token` is the documented headless path; the token is "scoped to inference only" | +| **Codex subscription** | `codex login` on the host, then coop must hold the refreshable `~/.codex/auth.json` | session goes stale after **~8 days** without a refresh | yes, but **client-side only** (Codex refreshes on use / on 401 and writes back to `auth.json`) | **Discouraged** — OpenAI: "API keys are still the recommended option for most CI/CD jobs"; there is **no long-lived headless token**, and the refresh endpoints are internal | + +**Consequences for the proxy:** + +- **API-key mode is the default and needs no login flow at all** — it is the + existing `cmd:` secret resolution, now terminating at the proxy instead of the + guest. Full Claude↔Codex parity. +- **Claude subscription is cleanly supportable** *because Anthropic ships a + one-year token.* The proxy holds a static bearer and injects it; renewal is a + once-a-year manual `setup-token`. No refresh loop, no undocumented endpoints. + This also finally gives #62 a non-exposure answer (the token never enters the + guest). +- **Codex subscription is the one path the proxy should *not* try to serve**, and + the blocker is OpenAI's design, not coop's: there is no long-lived token, and the + only credential is a refreshable `auth.json` whose refresh is a *client-side* + loop against internal endpoints that writes state back to the file. To keep the + guest from holding it, the proxy would have to reimplement that undocumented + refresh and own the rotating token — exactly the fragile, security-critical + machinery a security product should not build against an unstable contract. + OpenAI itself steers automation to API keys. **So: Codex subscription → use + API-key mode.** This is not a parity regression coop introduces; both agents get + full non-exposure via API keys, and Claude can *additionally* offer subscription + non-exposure only because Anthropic ships the token for it. + +**The `auth.json` exposure, closed in v1.** coop copies `~/.codex/auth.json` onto +the guest disk today (`backend.rs:2326`) — a refreshable token at rest, the worst +case #411 names. When proxy mode is on, **stop staging `auth.json`** (drop it from +the effective `CODEX_ALLOWED_FILES` for that instance). A user who wants Codex +subscription billing opts out of proxy mode explicitly and accepts the +disk-resident token — a documented, deliberate trade, not a silent one. + +**On `apiKeyHelper` (Claude) as an alternative — and why it is not one.** Claude +Code documents `apiKeyHelper`, a guest-side script it calls to fetch a token +(every 5 min or on 401). It is tempting but it is *not* non-exposure: the helper +returns the token to the in-guest CLI, so the raw credential still lands in the +guest. It reduces persistence, not readability. The proxy (guest holds only the +capability token, §6) is the actual non-exposure mechanism. + +### 7a. The "configure once, every VM, for a long time" property — exploit it, with guardrails + +A consequence of injection is that **one host-held credential serves every VM, +present and future, with no per-VM login.** For Claude this is especially stark: a +single `claude setup-token` yields a one-year credential, so the user logs in once +and every VM gets Claude for a year with zero in-guest auth. (It is not unique to +Claude — any host-held static credential the proxy injects, including both agents' +API keys, has the same shape; the Claude subscription token just extends it to +subscription billing.) This directly resolves #62's pain (re-auth in every guest). + +**Recommendation: exploit it — it is the headline UX win — and it is safe because +the credential never enters a guest.** A compromised guest cannot steal the token; +the worst it can do is *use* Claude inference while its VM is running and routed +through the proxy. Three guardrails keep the convenience from becoming a +concentration risk: + +1. **Stay on the *explicit* token path.** "One login for everything" must mean the + user deliberately runs `setup-token` (or supplies an API key) via `cmd:` — never + coop silently harvesting the host's live interactive `/login` session + (`~/.claude/.credentials.json` / Keychain). #62 already rejected implicit + credential reads as supply-chain magic; that judgment holds here. +2. **Keep per-VM control at the capability-token layer, not the upstream + credential.** The shared upstream token is the convenience; each VM's *access to + the proxy* is a distinct, per-instance, revocable capability token (§6). So + revocation and isolation granularity survive without per-VM logins: kill or + rotate one VM's capability and that VM loses Claude access immediately, while the + upstream token — and every other VM — is untouched. The single upstream login + must **not** collapse into a single shared capability token. +3. **Treat the proxy/host as the concentrated trust root it now is.** One + long-lived, subscription-wide credential reachable by every VM means a + proxy-or-host compromise yields up to a year of inference abuse. That is bounded + — Anthropic scopes `setup-token` to *inference only* (it cannot establish Remote + Control), so the ceiling is cost/quota abuse, not account takeover — but it is + real, and it is why the Tier 1 hardening (§14) and the Firecracker jail are not + optional polish. The proxy is also the natural place to observe/meter concurrent + multi-VM use. + +**Do not "harden" it in the wrong place.** Artificially shortening the *upstream* +token (forcing frequent re-logins) fights the convenience for little gain, since +that token never enters a guest. Put the short-lived, ephemeral, per-session +property where it actually reduces blast radius — the per-VM capability token — and +let the upstream credential be as long-lived as the vendor allows. + +--- + +## 8. Implementation slices + +Ordered, independently shippable: + +1. **Proxy core + Anthropic.** The host-side streaming reverse proxy as its own + workspace binary (`coop-proxy`, hyper + rustls + tokio — §6a), its per-instance + lifecycle (model the control-socket pattern from `port_forward.rs`), binding on + `guest_host_address`, secret resolution via `resolve_cmd_value`, and + capability-token verification (§6). Guest config: `claude_env_block` with + `base_url` = proxy and `ANTHROPIC_AUTH_TOKEN` = the per-instance capability + token. Covers both Claude modes at once: API key *and* the `setup-token` + subscription token are static bearers the proxy injects (§7) — no extra slice. + Smallest end-to-end slice because the base-URL seam already exists. +2. **Codex (API key).** Add the injection route for `api.openai.com`. Guest config: + `codex_proxy_config` block with `base_url` = proxy and `env_key = + "COOP_LOCAL_API_KEY"` carrying the capability token. Stop staging `auth.json` + when proxy mode is on (§7). + Delivers Claude↔Codex parity at the API-key tier. Codex subscription is out of + scope by vendor design (§7). +3. **Jailing (shipped).** Bound a proxy-exploit blast radius by confining + `coop-proxy`. **Implemented with Landlock (ABI v4), not the uid+netns jailer + sketched in §3/§11.3.** The settled architecture binds the listener on host + `127.0.0.1` reached via `ssh -R`, and an isolated network namespace gets its + own loopback the host-side tunnel could not reach — so instead of moving the + bind, the proxy self-confines with Landlock (filesystem-write + `exec` + denied; TCP egress limited to `:443`/`:53`) applied before it binds. macOS, + which has no in-process sandbox, is confined externally with a Seatbelt + profile via `sandbox-exec` — so this slice covers **both** backends, not just + Firecracker. Fail-closed on both. See [`../trust-model.md`](../trust-model.md) + and [`../credential-proxy.md`](../credential-proxy.md) for the shipped + mechanism and its accepted limitations (port-scoped not host-scoped; UDP + unrestricted on Linux; host kernel ≥6.7). +4. **GitHub (separate, later).** No base-URL override exists for `gh`/`git`, and + #73 already documented that URL/path filtering cannot constrain `gh api + graphql`. Injection (token never crosses to the guest) is still worthwhile as + an *exposure* reduction, but it is a different mechanism (credential helper or + `url.insteadOf` rewriting) and must not be labeled "scoped." Treat it as its own + design under #73's constraints. + +Each slice is gated behind explicit config (proxy mode is opt-in), per coop's +"no speculative features / additive and opt-in" stance. + +--- + +## 9. Threat model and residual risks + +- **What it stops:** a prompt-injected/rogue agent reading a usable API key from + its environment or from disk. After v1, in proxy mode, the two model API keys + are never in the guest. +- **What it does not stop (by construction):** + - *Use* of the injected credential while the agent runs — the agent can still + make model calls through the proxy (that is the point). Non-exposure limits + *exfiltration of the raw key*, not use during the session. + - *Egress* — a token-less agent with open egress can still reach arbitrary hosts + (#2). The proxy composes with #2 but does not replace it; the strong pairing is + proxy + "no route out except the proxy." + - *Scope* — the injected key retains its full account scope (#73). Non-exposure + and scope are orthogonal. +- **New attack surface:** the proxy itself. Mitigated by (a) a mature HTTP stack, + (b) fixed per-route upstreams (guest cannot retarget the injected key), (c) + host-side jailing on Firecracker (§3, slice 3). This surface is the whole reason + the helper VM stays on the table for the general proxy (§3/§4). +- **DNS/CDN caveats** from #2 are unaffected — this feature does not touch name + resolution. + +--- + +## 10. Rejected / deferred alternatives + +- **Helper VM for the per-integration proxy.** Rejected for v1: the per-integration + proxy's attack surface is small and CA-free, so a whole second VM (awkward on + Lima, heavy against #404, backend-diverging) is disproportionate. Retained as the + correct home for a *general* proxy (§3/§4). +- **General MITM proxy now.** Deferred: the guest-trusted CA and arbitrary-upstream + TLS termination are a large build for coverage the per-integration path already + gives on the high-value credentials (§4). +- **Keep forwarding raw keys, rely on the VM boundary alone.** Rejected: that is + the status quo #411 exists to fix; with open egress (#2) a readable key is an + exfiltratable key. +- **Reimplementing OAuth refresh in the proxy (esp. Codex subscription).** + Rejected: the refresh loops run against internal, undocumented endpoints and + rotate state a security product should not own against an unstable contract. Use + the static-token paths (API key everywhere; Claude `setup-token` for Claude + subscription) and route Codex subscription to API-key mode per OpenAI's own + guidance (§7). + +--- + +## 11. Open questions / spikes + +1. ~~**Subscription endpoint semantics.**~~ **Resolved (§7).** Claude ships an + official one-year `setup-token` (a static inference-scoped bearer) → proxy + injects it, no refresh. Codex ships *no* long-lived token; its only subscription + credential is a client-refreshed `auth.json`, and OpenAI recommends API keys for + automation → Codex subscription is out of scope for the proxy, API-key mode is + the answer. Net: only Codex-subscription would have needed refresh logic, and it + is deliberately not built. +2. **One proxy process vs. per-instance.** Per-instance is simpler to reason about + and matches the existing socket-lifecycle pattern; a shared process is fewer + moving parts but needs per-instance routing. Decide in slice 1. +3. ~~**Firecracker jail specifics.**~~ **Resolved (shipped, slice 3).** Not the + jailer/netns/uid options considered here — those fight the loopback + `ssh -R` + architecture (an isolated netns cannot reach the host-side tunnel). The proxy + self-confines with **Landlock** (ABI v4) on Linux and **Seatbelt** on macOS, + with the lightest lifecycle (no sudo, no per-instance netns/route teardown). + See [`../trust-model.md`](../trust-model.md). +4. **Config surface.** What the opt-in looks like (`[network]`/`[proxy]` block), + and how it interacts with `coop model local` (proxy mode and local mode both + rewrite `base_url` — they must not collide). + +--- + +## 12. Packaging, install, and attestation + +**The attestation is over the tarball, so bundle the proxy inside it.** The release +workflow attests each per-target artifact — `actions/attest-build-provenance` with +`subject-path: "coop-*.tar.gz"` (`release.yml`) — and `install.sh` verifies it with +`gh attestation verify --repo trailofbits/coop`, falling back to the +published `SHA256SUMS`. Anything shipped *inside* that already-attested tarball +inherits the identical SLSA build-provenance guarantee with **no new attestation +machinery**. So `coop-proxy` ships in the same tarball as `coop`. + +`coop-proxy` runs on the **host** (it holds the secret and forwards upstream), so +it builds for the exact three host triples already in the matrix +(`aarch64-apple-darwin`, `x86_64-unknown-linux-musl`, `aarch64-unknown-linux-musl`) +— no new build targets. + +**Crate layout.** The root `Cargo.toml` today is a single `coop` package (the +`fuzz/` dir is its own separate workspace). Promote the root to a workspace that +keeps `coop` as the root package and adds a `coop-proxy/` member with its *own* +`Cargo.toml` carrying the async/HTTP/TLS deps (`hyper`, `rustls`, `tokio`). Because +`coop` does **not** depend on `coop-proxy`, the `coop` binary's dependency closure +is unchanged — the heavy, security-critical deps are isolated to and independently +auditable in the proxy crate (§6a). `cargo build --release --workspace` produces +both binaries under `target//release/`. + +**Release diff (small):** +- Build: `cargo build --release --workspace --target ` (was: default + package only). +- Package: `cp` **both** `coop` and `coop-proxy` into `staging//` before + `tar`. Everything downstream — `SHA256SUMS`, the provenance attestation, the + `gh release create` — is untouched, because it all operates on the tarball. + +**Install diff (small):** after extraction, move **both** binaries into +`INSTALL_DIR`. Checksum + attestation verification are unchanged (still one tarball, +one subject). `coop` locates its proxy via `std::env::current_exe()`'s parent +directory, so they must land side by side — which the single-tarball install +guarantees. + +**Why not a separate artifact per binary.** A standalone `coop-proxy-*.tar.gz` with +its own attestation doubles the release outputs, adds a second `gh attestation +verify` to install, and introduces version-skew risk (proxy and CLI from different +builds). Bundling gives **lockstep versions by construction** (both from one build) +and one thing to verify — strictly simpler at equal cryptographic strength. + +**Why not a single binary with a hidden `coop proxy` subcommand (re-exec).** It is +the simplest packaging (nothing changes, one binary, trivially the same +attestation), and it keeps runtime isolation via re-exec + jail. But it links +`hyper`/`rustls`/`tokio` into the one `coop` binary, enlarging the main tool's +attack and supply-chain surface — the opposite of §6a's goal for a security- +critical tool. Rejected for that reason; the two-binaries-one-tarball approach +keeps the dependency surfaces separate at near-zero packaging cost. + +--- + +## 13. Dependency and feature hygiene + +Adding an async/HTTP/TLS stack to a tool that currently depends only on `url` is +the largest new supply-chain surface in this design. The rule is **every new dep +enters with `default-features = false` and an explicit, minimal feature list**, and +that minimality is *enforced in CI* by the `cargo deny check` that already runs +(`ci.yml`, `cargo-deny@0.19.9`) — no new CI step, just new rules in `deny.toml`. +Because `coop-proxy` is a workspace member, its graph is already in scope of that +check. + +**Enforcement: pin the exact feature set per direct dep.** cargo-deny's +`[[bans.features]]` with `exact = true` requires the *enabled* feature set to equal +the allowlist, so CI fails if anyone (or a transitive edge) turns on a feature we +did not sanction. Example (names/versions pinned against docs.rs at implementation +time): + +```toml +[[bans.features]] +crate = "tokio" +exact = true +allow = ["rt-multi-thread", "net", "io-util", "macros", "signal"] + +[[bans.features]] +crate = "hyper" +exact = true +allow = ["http1", "http2", "server", "client"] + +[[bans.features]] +crate = "hyper-util" +exact = true +allow = ["tokio", "server", "http1", "http2"] +``` + +with the matching `coop-proxy/Cargo.toml`: + +```toml +tokio = { version = "1", default-features = false, features = ["rt-multi-thread", "net", "io-util", "macros", "signal"] } +hyper = { version = "1", default-features = false, features = ["http1", "http2", "server", "client"] } +hyper-util = { version = "0.1", default-features = false, features = ["tokio", "server", "http1", "http2"] } +http-body-util = { version = "0.1", default-features = false } +tokio-rustls = { version = "0.26", default-features = false, features = ["tls12"] } +rustls = { version = "0.23", default-features = false, features = ["std", "tls12", "aws_lc_rs"] } +webpki-roots = "1" # pinned Mozilla roots (CDLA-Permissive-2.0, scoped exception) — no OS trust-store dependency +``` + +`webpki-roots` (a compiled-in, pinned CA set) is chosen over `rustls-native-certs` +deliberately: two fixed upstreams need no OS trust-store variance, and a pinned +root set is both smaller surface and more deterministic for a security tool. + +> **As shipped (v1).** Feature minimality is carried by each crate's +> `default-features = false` + explicit feature list in `coop-proxy/Cargo.toml`; +> the license exceptions and the `openssl`/`native-tls` bans below are enforced +> by the `cargo deny check` already in CI. The `[[bans.features]] exact = true` +> pins in this section were **not** shipped in `deny.toml` — they remain a +> recommended follow-up, not an active gate. + +**Ban the escape hatches outright.** Keep the OpenSSL/system-TLS path out of the +graph so nothing silently pulls it in: + +```toml +[bans] +deny = [{ crate = "openssl" }, { crate = "openssl-sys" }, { crate = "native-tls" }] +``` + +**The TLS crypto-provider license, scoped — not globally widened.** rustls itself +is `Apache-2.0`/`MIT`/`ISC`; its crypto backend is the snag. The pinned +`aws-lc-rs` (rustls default) stack carries `BSD-3-Clause` through its `aws-lc-sys` +C/`-sys` layer and through `subtle`, the pinned `webpki-roots` set is +`CDLA-Permissive-2.0`, plus `ISC` recurs across the stack — none of which was in +coop's `[licenses] allow` at design time. Handle the crate-specific terms with +**crate-scoped exceptions** and add the recurring `ISC` globally, so the +allowlist stays tight: + +```toml +[licenses] +allow = ["MIT", "Apache-2.0", "MPL-2.0", "Unicode-3.0", "ISC"] + +[[licenses.exceptions]] +crate = "aws-lc-sys" # crypto backend's C layer +allow = ["BSD-3-Clause"] + +[[licenses.exceptions]] +crate = "webpki-roots" # pinned Mozilla root set (a data license) +allow = ["CDLA-Permissive-2.0"] + +[[licenses.exceptions]] +crate = "subtle" # constant-time primitives in the crypto stack +allow = ["BSD-3-Clause"] +``` + +**Provider choice — a real tradeoff, flagged not hidden.** `aws-lc-rs` is the +recommended default (rustls default, AWS-maintained, FIPS-capable — the better +security posture) but builds C via `aws-lc-sys`, which needs cmake + a C +cross-toolchain for the two musl targets (the release runners already install +`musl-tools`; the aarch64 leg additionally needs the C cross-compiler already +present for the main build). `ring` cross-compiles more simply but the same +OpenSSL-license exception applies and its provider is less aligned with a +FIPS/security posture. Recommend `aws-lc-rs`; fall back to `ring` only if the +`aws-lc-sys` musl cross-build proves disproportionately costly. A pure-Rust +provider (e.g. `rustls-graviola`) is *not* recommended — an unproven crypto +implementation is a worse trade than a C build for a security-critical proxy. + +**Complementary checks.** Run `cargo machete` (or `cargo-udeps`) to catch deps/ +features that stop being used, and consider tightening `multiple-versions` from +`warn` to `deny` for the proxy graph so a duplicate transitive version is a +conscious decision. Neither is required for v1, but both keep the surface honest as +the crate evolves. + +--- + +## 14. What remains for a highly solid feature (definition of done) + +The design above is complete on paper; the following turns it into a feature you +would trust in a security product. Ordered by what gates the next thing. + +### Tier 0 — prove the premise (do this first; it can invalidate the design) + +Everything rests on one unproven assumption: **Claude Code and Codex both work, +end-to-end, pointed at a plain-HTTP header-injecting proxy — including SSE +streaming and tool-use round-trips — with no real credential in the guest.** Build +a throwaway hyper proxy in front of the *real* APIs and confirm: + +- Claude Code with `ANTHROPIC_BASE_URL` = proxy and `ANTHROPIC_AUTH_TOKEN` = a + placeholder: the proxy strips the guest header and injects the real credential as + `x-api-key` (API key) **or** `Authorization: Bearer` (`setup-token`), plus + `anthropic-version`. Confirm the API accepts it and Claude streams correctly. +- Codex with a custom provider `base_url` = proxy and `env_key` carrying the capability token: the proxy + injects `Authorization: Bearer` to the Responses endpoint; streaming + tool use + work. +- Neither agent pins certs, requires the token client-side for a feature, or sends + an unstrippable identifying header that breaks injection. + +If any of these fails, the mechanism changes before anything else is built. + +### Tier 1 — security-critical correctness (the reason this feature exists) + +- **Header-rewrite policy, audited and explicit.** Strip the guest capability + token; inject exactly the required upstream auth header(s); pin the upstream + `Host`; define an allowlist/strip rule for all other headers; reject + smuggling/oversized/duplicate-auth attempts. This is the crown-jewel path — it + gets a written spec and a security review. +- **Upstream is fixed, never guest-influenced.** The guest controls neither target + host nor scheme; only the path is forwarded to the pinned host. Closes SSRF. +- **TLS verification is correct and cannot regress.** Verify the upstream cert + against the pinned `webpki-roots`; a bug that disables verification MITMs the real + key. Consider pinning the two upstreams. Add a test that a bad cert is rejected. +- **Fail closed.** If `cmd:` secret resolution fails at proxy start, the VM does + not come up in a state where the agent silently has no or wrong credentials — + clear error, no fallback to a raw-key path. +- **Capability token done right.** CSPRNG-minted, per-instance, constant-time + compare, never logged; listener bound to the guest-only interface (never + `0.0.0.0`); a test proves another instance's token and other host interfaces are + rejected. +- **Resource limits.** Timeouts and concurrency/body-size caps so a rogue guest + cannot exhaust host CPU/memory through the proxy. +- **Secret in memory only.** Resolved lazily, never written to disk or logs; + redacted in all `Debug`; consider `zeroize` on drop. + +### Tier 2 — robustness and operability + +- **Lifecycle tied to the VM** (start/stop/destroy) via the `forwards.sock` + supervision pattern; orphan/zombie cleanup if coop itself dies; port-collision + pre-check. +- **Crash handling.** Proxy death mid-session surfaces a clear error (and/or + restarts), not a silent hang. +- **Multi-instance isolation.** Per-instance proxy + per-instance token so + instance A's guest can never reach instance B's credentials. (Resolves §11.2.) +- **`coop model local` interaction.** Proxy mode and local mode both rewrite + `base_url`; define precedence/mutual-exclusion. (Resolves §11.4.) + +### Tier 3 — validation gates (per CLAUDE.md) + +- **The security property is asserted by a test, not just designed:** an + integration phase, on **both** backends, that brings a VM up in proxy mode + against a mock upstream and asserts (a) the request reaches the upstream carrying + the injected header, (b) the raw credential appears **nowhere** in the guest env + or disk, (c) `~/.codex/auth.json` is not staged, (d) streaming works, (e) a + request without the capability token is refused. +- **Unit tests** for the pure logic (header rewrite, token check, route selection, + proxy-mode guest-config generation, config precedence) — behavior, edges, error + paths. +- **`.cargo/mutants.toml` scoping updated in the same PR** for the new pure + helpers vs. the IO/backend/TTY functions (the doc is emphatic this is not a + follow-up). +- **Security review** of the new surface (the `security-review` skill): SSRF, + header/request smuggling, TLS-verify correctness, resource exhaustion, the + host-reachable-port exposure on macOS. +- **Firecracker jailing implemented and validated** (slice 3), not just asserted. + +### Tier 4 — UX, docs, and surface + +- **Opt-in config shape** decided (default off, per coop's conservatism) and + fuzzed like the other config parsers if it adds fields. (Resolves §11.4 config + half.) +- **`coop status` shows proxy state** (on/off, which creds injected — redacted, + health). +- **Docs page** stating the threat model honestly (§9): what non-exposure does and + does not guarantee, and the weaker macOS story (host-local processes reach the + port, guarded only by the capability token). +- **Renewal ergonomics** for the Claude `setup-token` (warn before the yearly + expiry). + +### Cross-platform hardening: Lima is weaker by default but closable + +The blast-radius story is strongest on Firecracker (netns/uid jail, optional vsock, +guest-only bridge). On macOS/Lima the proxy defaults to a host-local TCP port +reachable by any local process, guarded only by the capability token. That gap is +**closable to near-Firecracker posture**, via macOS-native primitives — each with a +real caveat, and all VZ-driver-specific and needing a spike shared with #2's macOS +networking work: + +- **Jail the proxy with Seatbelt (`sandbox-exec`).** A profile confining + `coop-proxy` to: no filesystem writes, `connect` only to the two upstream + hosts:443, `bind` only its one listener, no `exec`. The sandbox inherits to + children and cannot be removed from inside — the macOS analog of the Linux + netns/uid jail, and close to it in strength. **Caveat:** `sandbox-exec` is + officially deprecated (still functional and widely used — Chromium, agent + harnesses — and Apple has shipped no clean CLI successor), so it is a pragmatic + but aging primitive. +- **Restrict egress, host-side.** Same two options as #2, and it must be host-side + because the Lima guest also has passwordless sudo: (a) **no route out** — + configure the guest with no default route, only a host-local link to the proxy + (structural, airtight, also defeats DNS tunneling); or (b) a host **`pf`** anchor + limiting the guest subnet to the proxy's listener (needs sudo, Lima-mode- + dependent, carries the usual filter gaps). Under `vzNAT` the guest already has no + VM-to-VM path, so this only has to constrain guest→internet. +- **Close the local-port exposure with vsock.** The VZ driver already uses vsock + (Lima runs its own guest agent over it), so the host↔guest hop can move to vsock + with a tiny guest-side vsock→`127.0.0.1` shim — leaving no host TCP port for other + local processes to reach. A unix-domain socket (0600) forwarded in is the same + idea. **Cost:** a small guest-side forwarder (the agents speak HTTP/TCP, so they + cannot target vsock directly). If that cost is not paid, the per-instance + capability token remains the guard. + +Net: Seatbelt ≈ the Linux jail, no-route-out is identical strength on both, and +vsock closes the port gap — so Lima can get close. The honest residual differences: +Seatbelt is deprecated where Linux namespaces are first-class, the vsock transport +needs a guest shim (more native on Firecracker), and all of it is VZ-specific and +unproven until the spike. State this plainly rather than implying uniform strength. diff --git a/docs/index.md b/docs/index.md index a41b562..1fb9a72 100644 --- a/docs/index.md +++ b/docs/index.md @@ -29,6 +29,9 @@ short navigational entrypoint; durable detail lives here. [`shell-completion.md`](shell-completion.md). - [`claude-integration.md`](claude-integration.md), [`codex-integration.md`](codex-integration.md) — agent integration. +- [`credential-proxy.md`](credential-proxy.md) — the opt-in `[proxy]` + credential-injecting proxy (issue #411): keeps the raw API key out of the + guest. - [`json-output-design.md`](json-output-design.md) — the `--json` design (a good precedent for design-doc style). diff --git a/docs/trust-model.md b/docs/trust-model.md index e8c1da6..8d6b1f7 100644 --- a/docs/trust-model.md +++ b/docs/trust-model.md @@ -92,6 +92,21 @@ user `env_forward` entries, and the VM SSH key. The invariants: - **The stored token is indirected, never inlined.** `config.toml` holds a `cmd:...` retrieval command (`secret_store.rs:CmdToken`), not the plaintext token; coop runs it at VM-start to fetch the value. +- **Proxy mode keeps the model API keys out of the guest entirely** (issue #411, + opt-in `[proxy]`). When enabled in remote model mode, `ANTHROPIC_API_KEY` + (Claude) and/or `OPENAI_API_KEY` (Codex) are **not** forwarded + (`prepare_env_forwarding`'s `suppress_anthropic_key`/`suppress_openai_key`, one + per provider); the host-side `coop-proxy` holds the real credential and the + guest gets only a per-instance capability token (Claude via `settings.json`, + Codex via the `coop_local` provider's bearer `env_key`). In proxy mode Codex's + `~/.codex/auth.json` is also **not** staged onto the guest disk (it holds a + refreshable subscription token). Each credential is resolved on the host and + handed to `coop-proxy` over **stdin**, never argv or disk; a resolution failure + fails the boot closed. A per-VM override + (`proxy_state.rs`, `/proxy.json`) selects a different host-side + credential for one VM — resolution is override → default → off — without + changing the proxy binary or the capability token. See + [`credential-proxy.md`](credential-proxy.md). ## SSH boundary @@ -122,6 +137,57 @@ user `env_forward` entries, and the VM SSH key. The invariants: - `rewrite_host_url` rewrites a loopback local-model endpoint to the guest-visible gateway address so the guest can reach a model server running on the host; non-loopback URLs pass through unchanged. +- **The credential proxy (issue #411) binds host loopback** (`127.0.0.1`, it + refuses an unspecified address at bind) and is exposed into the guest with a + per-instance `ssh -R` reverse tunnel (`proxy.rs:spawn_reverse_forward`), so — + like the port-forwards above — it never binds a non-loopback interface and is + reachable by exactly one guest, identically on both backends. It is guarded + by a per-instance capability token and forwards only to a fixed per-provider + upstream (`api.anthropic.com` / `api.openai.com`), never a guest-supplied host + — one proxy process and one tunnel per (VM, provider). Its own TLS-verifying + outbound HTTPS is the intended egress; a change that lets the guest influence + the upstream host, or that binds anything wider than loopback, is a finding. + +- **The credential proxy is jailed (issue #411, slice 3).** `coop-proxy` holds + the real credential and terminates connections the untrusted guest + originates, so it is the feature's new attack surface; the jail bounds the + blast radius of a proxy exploit. On **Linux** the proxy self-confines with + **Landlock** (ABI v4), applied in `coop-proxy`'s `main` after its libraries + load and before it binds, so every tokio worker inherits the domain: all + filesystem writes and all program `exec` are denied, and TCP `connect` is + limited to `:443` (upstream) and `:53` (DNS), `bind` to the listener port. + On **macOS** the launcher wraps the spawn in `sandbox-exec` with the Seatbelt + profile in [`src/seatbelt-proxy.sb`](../src/seatbelt-proxy.sb) (deny by + default; allow file reads, name resolution, the loopback listener, and egress + only to `:443`/`:53`). Either way the launch is **fail-closed**: if the jail + cannot be established the proxy exits before serving, `proxy.rs`'s post-spawn + readiness probe never connects, and the VM start aborts — the + credential-holding proxy never runs unconfined. Reads stay open (the resolver + config, the dynamic linker) and writes to the already-open stderr log fd are + unaffected, because both jails gate path opens / new connections, not + existing descriptors. + + **Accepted limitations, by construction** (do not file these as findings; do + flag a change that *widens* them): + - **Port-scoped, not host-scoped.** Both Landlock and Seatbelt filter by + port, not hostname/IP. A *fully compromised* proxy could still open a TCP + connection to some other host on `:443`; the two upstreams' identity is + enforced one layer up, at the proxy's TLS verification + pinned `Host`, and + the guest still cannot retarget them. Host-scoped egress would need IP + pinning (fragile against CDN rotation) or an L7 egress proxy — out of scope + and consistent with the DNS/CDN caveat #2 already accepts. + - **UDP egress is not restricted on Linux.** Landlock's network rules are + TCP-only, and DNS needs UDP `:53`, so arbitrary UDP egress remains possible + for a compromised proxy. Closing it would need `nftables` owner-matching + (a dedicated uid + `sudo` + per-instance teardown) — deliberately deferred. + - **Host-kernel floor / deprecated primitive.** The Linux jail needs kernel + ≥6.7 for the network rules (≥5.13 for filesystem/exec); an older host fails + closed. `sandbox-exec` is officially deprecated but still functional and + has no CLI successor — a pragmatic, aging primitive. + + These match the design's "Cross-platform hardening" note: the Firecracker + (Linux) story is the stronger one; Lima (macOS) is closable to near-parity + with Seatbelt, with the stated caveats. ## `coop update` trust chain diff --git a/install.sh b/install.sh index 56046db..a1d6120 100755 --- a/install.sh +++ b/install.sh @@ -156,11 +156,21 @@ tar -xzf "${TMPDIR}/${TARBALL}" -C "${TMPDIR}" info "Installing to ${INSTALL_DIR}..." mkdir -p "$INSTALL_DIR" -EXTRACTED="${TMPDIR}/${BINARY}-${VERSION}-${TRIPLE}/${BINARY}" +EXTRACTED_DIR="${TMPDIR}/${BINARY}-${VERSION}-${TRIPLE}" +EXTRACTED="${EXTRACTED_DIR}/${BINARY}" [ -f "$EXTRACTED" ] || die "Binary not found in tarball" mv "$EXTRACTED" "${INSTALL_DIR}/${BINARY}" chmod +x "${INSTALL_DIR}/${BINARY}" +# coop-proxy (issue #411) ships in the same tarball and must land next to +# coop — the CLI locates it via its own directory. Present from the version +# that introduced it; tolerate its absence when installing older releases. +PROXY_EXTRACTED="${EXTRACTED_DIR}/${BINARY}-proxy" +if [ -f "$PROXY_EXTRACTED" ]; then + mv "$PROXY_EXTRACTED" "${INSTALL_DIR}/${BINARY}-proxy" + chmod +x "${INSTALL_DIR}/${BINARY}-proxy" +fi + printf '\n %s %s installed to %s/%s\n' "$BINARY" "$VERSION" "$INSTALL_DIR" "$BINARY" # Check if INSTALL_DIR is on PATH diff --git a/src/backend.rs b/src/backend.rs index 8289e43..d494446 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -1356,13 +1356,20 @@ pub fn detect_instance_repo( pub fn prepare_env_forwarding( cfg: &CoopConfig, repo: Option<&crate::github_repo::RepoSlug>, + suppress_anthropic_key: bool, + suppress_openai_key: bool, ) -> Result { let claude = &cfg.claude; let codex = &cfg.codex; let mut env = EnvForward::default(); - // ANTHROPIC_API_KEY: prefer config, fall back to process env - if let Some(key) = &claude.api_key { + // ANTHROPIC_API_KEY: prefer config, fall back to process env. + // In proxy mode (issue #411) the raw key must never enter the guest — + // the host-side proxy holds it and injects it upstream, so we forward + // only the per-instance capability token via settings.json instead. + if suppress_anthropic_key { + tracing::debug!("proxy mode: not forwarding ANTHROPIC_API_KEY into the guest"); + } else if let Some(key) = &claude.api_key { let resolved = crate::config::resolve_cmd_value(key.expose()) .context("Failed to resolve claude.api_key")?; env.set("ANTHROPIC_API_KEY", resolved); @@ -1370,8 +1377,12 @@ pub fn prepare_env_forwarding( env.set("ANTHROPIC_API_KEY", key); } - // OPENAI_API_KEY: prefer config, fall back to process env - if let Some(key) = &codex.api_key { + // OPENAI_API_KEY: prefer config, fall back to process env. In proxy mode + // the raw key stays on the host (injected upstream); Codex authenticates + // to the proxy with the capability token via the provider `env_key`. + if suppress_openai_key { + tracing::debug!("proxy mode: not forwarding OPENAI_API_KEY into the guest"); + } else if let Some(key) = &codex.api_key { let resolved = crate::config::resolve_cmd_value(key.expose()) .context("Failed to resolve codex.api_key")?; env.set("OPENAI_API_KEY", resolved); @@ -1386,8 +1397,26 @@ pub fn prepare_env_forwarding( tracing::debug!("no GITHUB_TOKEN forwarded to guest"); } + // In proxy mode the raw provider key must never reach the guest by ANY + // path — not just the config/process-env fallback above, but also an + // explicit `env_forward` or `guest_env` entry. Collect the suppressed + // names so the loops below skip and loudly warn on such an entry. + let mut suppressed: Vec<&str> = Vec::new(); + if suppress_anthropic_key { + suppressed.push("ANTHROPIC_API_KEY"); + } + if suppress_openai_key { + suppressed.push("OPENAI_API_KEY"); + } + // User-specified env_forward vars from process environment for name in claude.env_forward.iter().chain(codex.env_forward.iter()) { + if suppressed.contains(&name.as_str()) { + tracing::warn!( + "proxy mode: ignoring env_forward entry '{name}' — the raw key stays on the host" + ); + continue; + } if !env.contains(name.as_str()) && let Ok(val) = std::env::var(name.as_str()) { @@ -1400,6 +1429,12 @@ pub fn prepare_env_forwarding( // an inherited one. Warn on collision so the override is visible — // values are not logged (they may be secrets). for (name, value) in &cfg.guest_env { + if suppressed.contains(&name.as_str()) { + tracing::warn!( + "proxy mode: ignoring guest_env entry '{name}' — the raw key stays on the host" + ); + continue; + } if env.contains(name.as_str()) { tracing::warn!("guest_env entry '{name}' overrides a previously resolved value"); } @@ -1500,37 +1535,52 @@ fn bootstrap_claude( // in local mode, so the routing is refreshed on every boot and // survives stop/start. let model_state = ModelState::load_or_default(inst)?; - let local_env = claude_local_env(&model_state, cfg, guest_host)?; - write_managed_claude_settings(&session.target, &local_env)?; - - // Work around the onboarding wizard ignoring CLAUDE_CODE_OAUTH_TOKEN - // (anthropics/claude-code#8938). Runs on every boot so a token added - // before a restart still takes effect; a no-op once the flag is present - // or when no OAuth token is forwarded. - seed_claude_onboarding(session, &claude_bin)?; - - // Marketplaces, plugins, MCP servers — persisted on guest disk, - // only install on first boot - if let BootMode::FirstBoot = mode { - // Compute delta: only install marketplaces/plugins not already - // baked into the golden image - let (missing_marketplaces, missing_plugins) = compute_plugin_delta(cfg, &inst.image); + // Proxy mode (issue #411): in remote mode with `[proxy]` configured, + // start the host-side injecting proxy and point the guest at it. The + // guest holds only the capability token; the real key stays on the host. + // Fails closed — a resolution or spawn failure aborts the boot. + let proxy = start_claude_proxy(inst, cfg, &model_state, &session.target)?; + // Run the rest under a guard: if any step fails after the proxy is live, + // tear it (and its reverse tunnel) down rather than orphaning a + // credential-holding process after a failed boot. A clean bootstrap leaves + // it running for the VM's lifetime. Mirrors `bootstrap_codex`. + let result = (|| -> Result<()> { + let local_env = claude_local_env(&model_state, cfg, guest_host, proxy.as_ref())?; + write_managed_claude_settings(&session.target, &local_env)?; + + // Work around the onboarding wizard ignoring CLAUDE_CODE_OAUTH_TOKEN + // (anthropics/claude-code#8938). Runs on every boot so a token added + // before a restart still takes effect; a no-op once the flag is present + // or when no OAuth token is forwarded. + seed_claude_onboarding(session, &claude_bin)?; + + // Marketplaces, plugins, MCP servers — persisted on guest disk, + // only install on first boot + if let BootMode::FirstBoot = mode { + // Compute delta: only install marketplaces/plugins not already + // baked into the golden image + let (missing_marketplaces, missing_plugins) = compute_plugin_delta(cfg, &inst.image); + + if !missing_marketplaces.is_empty() { + install_marketplaces(session, &claude_bin, &missing_marketplaces)?; + } - if !missing_marketplaces.is_empty() { - install_marketplaces(session, &claude_bin, &missing_marketplaces)?; - } + if !missing_plugins.is_empty() { + install_plugins(session, &claude_bin, &missing_plugins)?; + } - if !missing_plugins.is_empty() { - install_plugins(session, &claude_bin, &missing_plugins)?; + if !claude.mcp_servers.is_empty() { + register_mcp_servers(session, &claude_bin, &claude.mcp_servers)?; + } } - if !claude.mcp_servers.is_empty() { - register_mcp_servers(session, &claude_bin, &claude.mcp_servers)?; - } + tracing::info!("Claude Code bootstrap complete"); + Ok(()) + })(); + if result.is_err() && proxy.is_some() { + crate::proxy::stop_provider(inst, crate::proxy::Provider::Anthropic); } - - tracing::info!("Claude Code bootstrap complete"); - Ok(()) + result } /// Bootstrap Codex in the guest declaratively. @@ -1545,67 +1595,89 @@ fn bootstrap_codex( mode: BootMode, guest_host: &str, ) -> Result<()> { - let codex = &cfg.codex; - let source_dir = resolve_config_source_dir(&codex.config_dir, ".codex", "codex.config_dir"); - let model_state = ModelState::load_or_default(inst)?; - let local = codex_local_table(&model_state, cfg, guest_host)?; - // Once coop has materialized a Codex local block (endpoint resolves, - // or it was materialized on a prior `local` switch), coop owns the - // model/provider keys in config.toml regardless of mode: it writes the - // provider block in local mode and rewrites a clean file in remote - // mode, so switching back to cloud reliably drops the block — even if - // the configured endpoint has since been removed. - let manages_local = - model_state.resolved_codex(codex).is_some() || model_state.codex_materialized; - let has_plugins = !codex.marketplaces.is_empty() || !codex.plugins.is_empty(); - let needs_codex = - codex_bootstrap_needed(source_dir.as_deref(), &codex.mcp_servers, has_plugins) - || manages_local; - - if !needs_codex { - return Ok(()); - } - - if !session.target.exec_ok( - RemoteCommand::new() - .literal("test -x ") - .arg(crate::guest::codex_bin()), - ) { - bail!("{}", codex_missing_guest_cli_message()); - } + let mut model_state = ModelState::load_or_default(inst)?; + // Proxy mode (issue #411): in remote mode with `[proxy.openai]` (or a + // per-VM override) configured, start the host-side injecting proxy and + // point Codex at it. The guest holds only the capability token; the real + // key stays on the host. Fails closed — a resolution/spawn failure aborts. + let proxy = start_codex_proxy(inst, cfg, &model_state, &session.target)?; + // Run the rest under a guard: if any step fails after the proxy is live, + // tear it (and its reverse tunnel) down rather than orphaning a + // credential-holding process after a failed boot. A clean bootstrap leaves + // it running for the VM's lifetime. + let result = (|| -> Result<()> { + let codex = &cfg.codex; + let source_dir = resolve_config_source_dir(&codex.config_dir, ".codex", "codex.config_dir"); + let local = codex_provider_table(&model_state, cfg, guest_host, proxy.as_ref())?; + // coop_local is the provider id for both the local-model block and the + // proxy block. Remember once coop has materialized it so a later + // switch-off (to cloud, or after removing the config) reliably rewrites a + // clean file that drops the block — even when no local endpoint resolves + // (the proxy case). Persist so the memory survives the switch. + if local.is_some() && !model_state.codex_materialized { + model_state.codex_materialized = true; + model_state.save(inst)?; + } + let manages_local = local.is_some() + || model_state.resolved_codex(codex).is_some() + || model_state.codex_materialized; + let has_plugins = !codex.marketplaces.is_empty() || !codex.plugins.is_empty(); + let needs_codex = + codex_bootstrap_needed(source_dir.as_deref(), &codex.mcp_servers, has_plugins) + || manages_local; + + if !needs_codex { + return Ok(()); + } - copy_codex_config( - &session.target, - source_dir.as_deref(), - codex, - local.as_ref(), - manages_local, - )?; + if !session.target.exec_ok( + RemoteCommand::new() + .literal("test -x ") + .arg(crate::guest::codex_bin()), + ) { + // The caller's guard tears down the proxy started above on this error. + bail!("{}", codex_missing_guest_cli_message()); + } - // Marketplaces and plugins are persisted in the guest's config.toml and - // plugin cache (and preserved across restarts by `copy_codex_config`), so - // install only the delta not already baked into the golden image, and only - // on first boot — marketplaces first, since `plugin add` resolves against a - // configured marketplace. - if let BootMode::FirstBoot = mode { - let codex_bin = crate::guest::codex_bin(); - let (missing_marketplaces, missing_plugins) = compute_codex_plugin_delta(cfg, &inst.image); + copy_codex_config( + &session.target, + source_dir.as_deref(), + codex, + local.as_ref(), + manages_local, + proxy.is_some(), + )?; + + // Marketplaces and plugins are persisted in the guest's config.toml and + // plugin cache (and preserved across restarts by `copy_codex_config`), so + // install only the delta not already baked into the golden image, and only + // on first boot — marketplaces first, since `plugin add` resolves against a + // configured marketplace. + if let BootMode::FirstBoot = mode { + let codex_bin = crate::guest::codex_bin(); + let (missing_marketplaces, missing_plugins) = + compute_codex_plugin_delta(cfg, &inst.image); + + if !missing_marketplaces.is_empty() { + install_codex_marketplaces(session, &codex_bin, &missing_marketplaces)?; + } - if !missing_marketplaces.is_empty() { - install_codex_marketplaces(session, &codex_bin, &missing_marketplaces)?; + if !missing_plugins.is_empty() { + install_codex_plugins(session, &codex_bin, &missing_plugins)?; + } } - if !missing_plugins.is_empty() { - install_codex_plugins(session, &codex_bin, &missing_plugins)?; + match mode { + BootMode::Restart => tracing::info!("Codex bootstrap refreshed"), + BootMode::FirstBoot => tracing::info!("Codex bootstrap complete"), } - } - match mode { - BootMode::Restart => tracing::info!("Codex bootstrap refreshed"), - BootMode::FirstBoot => tracing::info!("Codex bootstrap complete"), + Ok(()) + })(); + if result.is_err() && proxy.is_some() { + crate::proxy::stop_provider(inst, crate::proxy::Provider::Openai); } - - Ok(()) + result } fn codex_missing_guest_cli_message() -> &'static str { @@ -2109,34 +2181,111 @@ fn claude_local_env( state: &ModelState, cfg: &CoopConfig, guest_host: &str, + proxy: Option<&crate::proxy::ProxyHandle>, ) -> Result> { - let Some(ep) = local_endpoint(state, state.resolved_claude(&cfg.claude)) else { - return Ok(BTreeMap::new()); + // Local mode takes precedence over proxy mode: both rewrite the base URL, + // and a VM switched to local should route at the user's model server. + if let Some(ep) = local_endpoint(state, state.resolved_claude(&cfg.claude)) { + let base_url = crate::network::rewrite_host_url(ep.host_url(), guest_host)?; + return Ok(crate::model_state::claude_env_block( + base_url.as_str(), + ep.model(), + &ep.auth_token_or_default(), + )); + } + // Remote mode + proxy active → point Claude Code at the host-side proxy. + if let Some(p) = proxy { + return Ok(crate::model_state::claude_proxy_env_block( + &p.base_url, + p.capability_token.expose(), + )); + } + Ok(BTreeMap::new()) +} + +/// Start the Anthropic credential proxy for this VM when proxy mode applies. +fn start_claude_proxy( + inst: &crate::config::Instance, + cfg: &CoopConfig, + model_state: &ModelState, + target: &SshTarget, +) -> Result> { + start_agent_proxy( + inst, + cfg, + model_state, + target, + crate::proxy::Provider::Anthropic, + ) +} + +/// Start the `OpenAI` credential proxy for this VM when proxy mode applies. +fn start_codex_proxy( + inst: &crate::config::Instance, + cfg: &CoopConfig, + model_state: &ModelState, + target: &SshTarget, +) -> Result> { + start_agent_proxy( + inst, + cfg, + model_state, + target, + crate::proxy::Provider::Openai, + ) +} + +/// Start one provider's credential proxy when proxy mode applies (remote model +/// mode + an effective upstream — per-VM override or `[proxy.]` +/// default), tearing down any stale proxy otherwise. Fails closed: a +/// resolution/spawn failure aborts the boot rather than falling back to +/// forwarding a raw key. Works on both backends — the proxy binds host +/// loopback and is reverse-tunnelled into the guest. +fn start_agent_proxy( + inst: &crate::config::Instance, + cfg: &CoopConfig, + model_state: &ModelState, + target: &SshTarget, + provider: crate::proxy::Provider, +) -> Result> { + let effective = if model_state.mode == crate::model_state::ModelMode::Remote { + crate::proxy_state::effective_upstream(inst, provider, &cfg.proxy)? + } else { + None }; - let base_url = crate::network::rewrite_host_url(ep.host_url(), guest_host)?; - Ok(crate::model_state::claude_env_block( - base_url.as_str(), - ep.model(), - &ep.auth_token_or_default(), - )) -} - -/// The Codex `config.toml` local-provider keys, with the endpoint's host -/// URL rewritten for the guest. `None` when not in local mode or no Codex -/// endpoint resolves. -fn codex_local_table( + let Some(upstream) = effective else { + // Not in proxy mode for this provider: ensure no proxy from a previous + // boot (e.g. before a switch to local mode, or after the config was + // removed) keeps running against this instance. + crate::proxy::stop_provider(inst, provider); + return Ok(None); + }; + Ok(Some(crate::proxy::start_provider( + inst, provider, &upstream, target, + )?)) +} + +/// The Codex `config.toml` provider keys for this VM: the local-model block in +/// local mode, or the proxy block in remote+proxy mode, with the endpoint's +/// host URL rewritten for the guest. Local mode takes precedence over proxy +/// mode (mirrors [`claude_local_env`]). `None` when neither applies. +fn codex_provider_table( state: &ModelState, cfg: &CoopConfig, guest_host: &str, + proxy: Option<&crate::proxy::ProxyHandle>, ) -> Result> { - let Some(ep) = local_endpoint(state, state.resolved_codex(&cfg.codex)) else { - return Ok(None); - }; - let base_url = crate::network::rewrite_host_url(ep.host_url(), guest_host)?; - Ok(Some(crate::model_state::codex_local_config( - base_url.as_str(), - ep.model(), - ))) + if let Some(ep) = local_endpoint(state, state.resolved_codex(&cfg.codex)) { + let base_url = crate::network::rewrite_host_url(ep.host_url(), guest_host)?; + return Ok(Some(crate::model_state::codex_local_config( + base_url.as_str(), + ep.model(), + ))); + } + if let Some(p) = proxy { + return Ok(Some(crate::model_state::codex_proxy_config(&p.base_url))); + } + Ok(None) } /// Gate an already-resolved endpoint on the VM being in local mode. In @@ -2158,6 +2307,7 @@ fn copy_codex_config( codex: &crate::config::CodexConfig, local: Option<&toml::Table>, manages_local: bool, + proxy_active: bool, ) -> Result<()> { // The guest's config.toml holds the Codex CLI's installed `[marketplaces.*]` // / `[plugins.*]` tables. We only need to carry them across a *rewrite* of @@ -2177,6 +2327,7 @@ fn copy_codex_config( local, manages_local, preserved.as_ref(), + proxy_active, ) .context("Failed to stage Codex config files")?; copy_staged_to_guest(target, &staged, ".codex", "Codex") @@ -2322,9 +2473,26 @@ fn stage_allowed_files(source_dir: &Path) -> Result { stage_selected_files(source_dir, &["CLAUDE.md"], &["rules", "commands"]) } -/// Allowlisted files copied verbatim from the host Codex config dir. +/// Allowlisted files copied verbatim from the host Codex config dir. In proxy +/// mode `auth.json` is dropped (see [`codex_allowed_files`]) — the refreshable +/// subscription token must not land on the guest disk (issue #411 §7). const CODEX_ALLOWED_FILES: &[&str] = &["AGENTS.md", "auth.json"]; +/// The Codex allowlist for this boot. In proxy mode `~/.codex/auth.json` is +/// dropped so the at-rest `OpenAI` subscription token never reaches the guest; +/// Codex authenticates to the proxy with the capability token instead. +fn codex_allowed_files(proxy_active: bool) -> Vec<&'static str> { + if proxy_active { + CODEX_ALLOWED_FILES + .iter() + .copied() + .filter(|f| *f != "auth.json") + .collect() + } else { + CODEX_ALLOWED_FILES.to_vec() + } +} + /// Allowlisted directories copied recursively from the host Codex config dir. const CODEX_ALLOWED_DIRS: &[&str] = &["prompts"]; @@ -2354,18 +2522,15 @@ fn stage_codex_files( local: Option<&toml::Table>, manages_local: bool, preserved: Option<&toml::Table>, + proxy_active: bool, ) -> Result { let staging = tempfile::TempDir::new().context("Failed to create staging directory")?; + let allowed_files = codex_allowed_files(proxy_active); let mut config = match source_dir { Some(path) => { - stage_selected_files_into( - path, - staging.path(), - CODEX_ALLOWED_FILES, - CODEX_ALLOWED_DIRS, - ) - .context("Failed to stage Codex allowlisted files")?; + stage_selected_files_into(path, staging.path(), &allowed_files, CODEX_ALLOWED_DIRS) + .context("Failed to stage Codex allowlisted files")?; let config_path = path.join(CODEX_CONFIG_FILE); if config_path.is_file() { @@ -2898,6 +3063,135 @@ Filesystem 1M-blocks Used Available Use% Mounted on boot_preflight(&CoopConfig::default()).unwrap(); } + #[test] + fn claude_local_env_uses_proxy_block_in_remote_mode() { + let state = ModelState { + mode: crate::model_state::ModelMode::Remote, + ..Default::default() + }; + let cfg = CoopConfig::default(); + let proxy = crate::proxy::ProxyHandle { + base_url: "http://172.16.0.1:8788".to_string(), + capability_token: crate::config::Secret::new("cap-token".to_string()), + }; + let env = claude_local_env(&state, &cfg, "172.16.0.1", Some(&proxy)).unwrap(); + assert_eq!(env["ANTHROPIC_BASE_URL"], "http://172.16.0.1:8788"); + assert_eq!(env["ANTHROPIC_AUTH_TOKEN"], "cap-token"); + // Proxy mode is transparent — no model pinning (unlike local mode). + assert!(!env.contains_key("ANTHROPIC_MODEL")); + } + + #[test] + fn claude_local_env_prefers_local_over_proxy() { + let ep = crate::config::LocalModel::new( + url::Url::parse("http://localhost:11434").unwrap(), + "qwen".to_string(), + None, + ) + .unwrap(); + let state = ModelState { + mode: crate::model_state::ModelMode::Local, + claude_endpoint: Some(ep), + ..Default::default() + }; + let cfg = CoopConfig::default(); + let proxy = crate::proxy::ProxyHandle { + base_url: "http://172.16.0.1:8788".to_string(), + capability_token: crate::config::Secret::new("cap-token".to_string()), + }; + // Even with a proxy handle present, local mode wins. + let env = claude_local_env(&state, &cfg, "172.16.0.1", Some(&proxy)).unwrap(); + assert_eq!(env["ANTHROPIC_MODEL"], "qwen"); + assert!(env["ANTHROPIC_BASE_URL"].starts_with("http://172.16.0.1:11434")); + } + + #[test] + fn claude_local_env_empty_without_proxy_or_local() { + let env = claude_local_env( + &ModelState::default(), + &CoopConfig::default(), + "172.16.0.1", + None, + ) + .unwrap(); + assert!(env.is_empty()); + } + + #[test] + fn codex_provider_table_uses_proxy_block_in_remote_mode() { + let state = ModelState { + mode: crate::model_state::ModelMode::Remote, + ..Default::default() + }; + let cfg = CoopConfig::default(); + let proxy = crate::proxy::ProxyHandle { + base_url: "http://127.0.0.1:9788".to_string(), + capability_token: crate::config::Secret::new("cap-token".to_string()), + }; + let table = codex_provider_table(&state, &cfg, "172.16.0.1", Some(&proxy)) + .unwrap() + .unwrap(); + assert_eq!( + table["model_provider"].as_str().unwrap(), + crate::model_state::CODEX_LOCAL_PROVIDER + ); + let provider = table["model_providers"][crate::model_state::CODEX_LOCAL_PROVIDER] + .as_table() + .unwrap(); + assert_eq!( + provider["base_url"].as_str().unwrap(), + "http://127.0.0.1:9788/v1" + ); + // Proxy mode is transparent — no model pin (unlike local mode). + assert!(!table.contains_key("model")); + } + + #[test] + fn codex_provider_table_prefers_local_over_proxy() { + let ep = crate::config::LocalModel::new( + url::Url::parse("http://localhost:11434/v1/").unwrap(), + "qwen".to_string(), + None, + ) + .unwrap(); + let state = ModelState { + mode: crate::model_state::ModelMode::Local, + codex_endpoint: Some(ep), + ..Default::default() + }; + let cfg = CoopConfig::default(); + let proxy = crate::proxy::ProxyHandle { + base_url: "http://127.0.0.1:9788".to_string(), + capability_token: crate::config::Secret::new("cap-token".to_string()), + }; + // Even with a proxy handle present, local mode wins (mirrors Claude). + let table = codex_provider_table(&state, &cfg, "172.16.0.1", Some(&proxy)) + .unwrap() + .unwrap(); + assert_eq!(table["model"].as_str().unwrap(), "qwen"); + let provider = table["model_providers"][crate::model_state::CODEX_LOCAL_PROVIDER] + .as_table() + .unwrap(); + assert!( + provider["base_url"] + .as_str() + .unwrap() + .starts_with("http://172.16.0.1:11434") + ); + } + + #[test] + fn codex_provider_table_none_without_proxy_or_local() { + let table = codex_provider_table( + &ModelState::default(), + &CoopConfig::default(), + "172.16.0.1", + None, + ) + .unwrap(); + assert!(table.is_none()); + } + #[test] fn onboarding_marked_complete_detects_true_flag() { assert!(onboarding_marked_complete( @@ -3388,7 +3682,8 @@ Filesystem 1M-blocks Used Available Use% Mounted on }, ); - let staging = stage_codex_files(Some(src.path()), &servers, None, false, None).unwrap(); + let staging = + stage_codex_files(Some(src.path()), &servers, None, false, None, false).unwrap(); assert!(staging.path().join("AGENTS.md").is_file()); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); @@ -3418,7 +3713,8 @@ Filesystem 1M-blocks Used Available Use% Mounted on }, ); - let staging = stage_codex_files(Some(src.path()), &servers, None, false, None).unwrap(); + let staging = + stage_codex_files(Some(src.path()), &servers, None, false, None, false).unwrap(); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); assert!(config.contains("model = \"gpt-5\"")); @@ -3440,6 +3736,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on None, false, None, + false, ) .unwrap(); assert_eq!( @@ -3448,6 +3745,31 @@ Filesystem 1M-blocks Used Available Use% Mounted on ); } + #[test] + fn stage_codex_files_drops_auth_json_in_proxy_mode() { + // Issue #411 §7: in proxy mode the refreshable OpenAI subscription + // token must not land on the guest disk. + let src = tempfile::TempDir::new().unwrap(); + std::fs::write(src.path().join("auth.json"), "{\"access_token\":\"test\"}").unwrap(); + std::fs::write(src.path().join("AGENTS.md"), "hi").unwrap(); + + let staging = stage_codex_files( + Some(src.path()), + &std::collections::HashMap::new(), + None, + false, + None, + true, + ) + .unwrap(); + assert!( + !staging.path().join("auth.json").exists(), + "auth.json must not be staged in proxy mode" + ); + // Other allowlisted files are unaffected. + assert!(staging.path().join("AGENTS.md").is_file()); + } + #[test] fn stage_codex_files_writes_local_provider_block() { // No source dir, no MCP servers: the local provider block alone @@ -3462,6 +3784,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on Some(&local), true, None, + false, ) .unwrap(); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); @@ -3482,6 +3805,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on Some(&local), true, None, + false, ) .unwrap(); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); @@ -3496,8 +3820,15 @@ Filesystem 1M-blocks Used Available Use% Mounted on fn stage_codex_files_remote_revert_drops_provider() { // Switching back to remote: no local table, but `manages_local` // forces a clean rewrite that omits the provider block. - let staging = - stage_codex_files(None, &std::collections::HashMap::new(), None, true, None).unwrap(); + let staging = stage_codex_files( + None, + &std::collections::HashMap::new(), + None, + true, + None, + false, + ) + .unwrap(); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); assert!( !config.contains("coop_local"), @@ -3592,6 +3923,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on None, false, Some(&preserved), + false, ) .unwrap(); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); @@ -3625,6 +3957,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on None, false, None, + false, ) .unwrap(); let config = std::fs::read_to_string(staging.path().join("config.toml")).unwrap(); @@ -3933,6 +4266,49 @@ Filesystem 1M-blocks Used Available Use% Mounted on assert!(debug.contains("EnvForward")); } + #[test] + fn proxy_mode_suppresses_anthropic_key_forwarding() { + // The crown-jewel invariant (issue #411): in proxy mode the raw + // Anthropic key must never be forwarded into the guest, regardless of + // config or the process environment. + let mut cfg = CoopConfig::default(); + cfg.claude.api_key = Some(crate::config::Secret::new("sk-ant-realkey".to_string())); + cfg.github = None; + + let suppressed = prepare_env_forwarding(&cfg, None, true, false).unwrap(); + assert!( + !suppressed.contains("ANTHROPIC_API_KEY"), + "raw Anthropic key leaked into guest env in proxy mode" + ); + + let forwarded = prepare_env_forwarding(&cfg, None, false, false).unwrap(); + assert!( + forwarded.contains("ANTHROPIC_API_KEY"), + "non-proxy mode should still forward the configured key" + ); + } + + #[test] + fn proxy_mode_suppresses_openai_key_forwarding() { + // Same crown-jewel invariant for Codex/OpenAI (issue #411 slice 2): in + // proxy mode the raw OpenAI key must never reach the guest. + let mut cfg = CoopConfig::default(); + cfg.codex.api_key = Some(crate::config::Secret::new("sk-openai-realkey".to_string())); + cfg.github = None; + + let suppressed = prepare_env_forwarding(&cfg, None, false, true).unwrap(); + assert!( + !suppressed.contains("OPENAI_API_KEY"), + "raw OpenAI key leaked into guest env in proxy mode" + ); + + let forwarded = prepare_env_forwarding(&cfg, None, false, false).unwrap(); + assert!( + forwarded.contains("OPENAI_API_KEY"), + "non-proxy mode should still forward the configured key" + ); + } + // ── guest_env merge precedence ────────────────────────── /// Build a `CoopConfig` whose env-resolving inputs are all empty @@ -3959,7 +4335,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on #[test] fn guest_env_entries_are_forwarded() { let cfg = cfg_with_guest_env(&[("RUST_LOG", "info"), ("MY_FLAG", "1")]); - let env = prepare_env_forwarding(&cfg, None).unwrap(); + let env = prepare_env_forwarding(&cfg, None, false, false).unwrap(); assert_eq!( env.as_envs().get("RUST_LOG").map(String::as_str), Some("info") @@ -3974,7 +4350,7 @@ Filesystem 1M-blocks Used Available Use% Mounted on let mut cfg = cfg_with_guest_env(&[("ANTHROPIC_API_KEY", "guest-env-wins")]); cfg.claude.api_key = Some(crate::config::Secret::new("from-claude-config".to_string())); - let env = prepare_env_forwarding(&cfg, None).unwrap(); + let env = prepare_env_forwarding(&cfg, None, false, false).unwrap(); assert_eq!( env.as_envs().get("ANTHROPIC_API_KEY").map(String::as_str), Some("guest-env-wins"), @@ -3986,7 +4362,97 @@ Filesystem 1M-blocks Used Available Use% Mounted on // Empty string is a legitimate value — distinguish "unset" from // "set to empty" so users can intentionally clear inherited vars. let cfg = cfg_with_guest_env(&[("EMPTY", "")]); - let env = prepare_env_forwarding(&cfg, None).unwrap(); + let env = prepare_env_forwarding(&cfg, None, false, false).unwrap(); assert_eq!(env.as_envs().get("EMPTY").map(String::as_str), Some("")); } + + #[test] + fn proxy_mode_suppresses_anthropic_key_from_guest_env() { + // The crown-jewel invariant must hold even when the raw key is listed + // explicitly in `guest_env` — the suppression left the name absent, so + // the collision warning never fires and it would otherwise forward + // silently. + let cfg = cfg_with_guest_env(&[("ANTHROPIC_API_KEY", "sk-ant-realkey")]); + let suppressed = prepare_env_forwarding(&cfg, None, true, false).unwrap(); + assert!( + !suppressed.contains("ANTHROPIC_API_KEY"), + "guest_env re-injected the raw Anthropic key in proxy mode" + ); + // Non-proxy mode still forwards the explicit guest_env value. + let forwarded = prepare_env_forwarding(&cfg, None, false, false).unwrap(); + assert_eq!( + forwarded + .as_envs() + .get("ANTHROPIC_API_KEY") + .map(String::as_str), + Some("sk-ant-realkey") + ); + } + + #[test] + fn proxy_mode_suppresses_openai_key_from_guest_env() { + let cfg = cfg_with_guest_env(&[("OPENAI_API_KEY", "sk-openai-realkey")]); + let suppressed = prepare_env_forwarding(&cfg, None, false, true).unwrap(); + assert!( + !suppressed.contains("OPENAI_API_KEY"), + "guest_env re-injected the raw OpenAI key in proxy mode" + ); + let forwarded = prepare_env_forwarding(&cfg, None, false, false).unwrap(); + assert_eq!( + forwarded + .as_envs() + .get("OPENAI_API_KEY") + .map(String::as_str), + Some("sk-openai-realkey") + ); + } + + #[test] + fn proxy_mode_suppresses_anthropic_key_from_env_forward() { + // The third raw-key entry path: an explicit env_forward entry resolved + // from the host process env. Serialize the env mutation and restore the + // prior value so parallel tests are unaffected. + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _guard = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + + let prior = std::env::var("ANTHROPIC_API_KEY").ok(); + // SAFETY: this is the only test that mutates ANTHROPIC_API_KEY, it holds + // ENV_LOCK while doing so, and it restores the prior value before + // returning. No coop test asserts on a process-inherited + // ANTHROPIC_API_KEY, so a transient read by another thread is benign. + unsafe { std::env::set_var("ANTHROPIC_API_KEY", "sk-ant-from-host-env") }; + + let mut cfg = CoopConfig::default(); + cfg.claude.api_key = None; + cfg.codex.api_key = None; + cfg.github = None; + cfg.claude.env_forward = + vec![crate::guest_env_state::EnvVarName::new("ANTHROPIC_API_KEY").unwrap()]; + + let suppressed = prepare_env_forwarding(&cfg, None, true, false).unwrap(); + let forwarded = prepare_env_forwarding(&cfg, None, false, false).unwrap(); + + // SAFETY: same lock still held; restore the environment to its prior state. + unsafe { + match &prior { + Some(v) => std::env::set_var("ANTHROPIC_API_KEY", v), + None => std::env::remove_var("ANTHROPIC_API_KEY"), + } + } + + assert!( + !suppressed.contains("ANTHROPIC_API_KEY"), + "env_forward re-injected the raw Anthropic key in proxy mode" + ); + assert_eq!( + forwarded + .as_envs() + .get("ANTHROPIC_API_KEY") + .map(String::as_str), + Some("sk-ant-from-host-env"), + "non-proxy mode should still forward an env_forward entry" + ); + } } diff --git a/src/commands/lifecycle.rs b/src/commands/lifecycle.rs index 095f8ee..5a6cacb 100644 --- a/src/commands/lifecycle.rs +++ b/src/commands/lifecycle.rs @@ -13,7 +13,7 @@ use super::{merge_runtime_guest_env, purge_all_data}; use crate::backend::VmBackend as _; use crate::{ backend, config, devcontainer, github_repo, guest, guest_env_state, model_state, pat_prompt, - port_forward, setup, signal, ssh, workspace, + port_forward, proxy, proxy_state, setup, signal, ssh, workspace, }; pub(crate) struct UpOpts<'a> { @@ -946,6 +946,7 @@ pub(super) fn allocate_and_start( if let Ok(target) = be.ssh_target(cfg, &inst) { port_forward::teardown_ssh_forwards(&inst, &target); } + crate::proxy::stop(&inst); if let Err(cleanup_err) = be.destroy_instance(cfg, &inst) { tracing::debug!("Cleanup failed (non-fatal): {cleanup_err}"); } @@ -1507,11 +1508,29 @@ fn bootstrap_and_post_start( mode: backend::BootMode, ) -> Result<()> { let post_start = opts.post_start_override.or(cfg.post_start.as_deref()); + let proxy_configured = + proxy_state::effective_upstream(inst, proxy::Provider::Anthropic, &cfg.proxy)?.is_some() + || proxy_state::effective_upstream(inst, proxy::Provider::Openai, &cfg.proxy)? + .is_some(); + if opts.no_agents && proxy_configured { + // Proxy mode suppresses the raw API keys on every session, but the + // proxy + guest base-URL override are only set up during agent + // bootstrap — which --no-agents skips. Warn so the loud auth failure + // isn't a mystery (contradictory config: proxy is about agent creds). + tracing::warn!( + "proxy mode is configured but --no-agents skips agent bootstrap; \ + agents will not be able to authenticate in this VM" + ); + } if opts.no_agents && post_start.is_none() { tracing::info!("Skipping guest agent bootstrap (--no-agents)"); return Ok(()); } - let session = prepare_session_from_target(cfg, None, target.clone(), repo)?; + // Pass `Some(inst)` so proxy-mode key suppression (and the guest-env / + // Codex-local overlays) apply to the bootstrap session too — otherwise the + // raw ANTHROPIC_API_KEY would be forwarded via SendEnv during bootstrap, + // defeating proxy-mode non-exposure (issue #411). + let session = prepare_session_from_target(cfg, Some(inst), target.clone(), repo)?; if opts.no_agents { tracing::info!("Skipping guest agent bootstrap (--no-agents)"); } else { @@ -1519,6 +1538,18 @@ fn bootstrap_and_post_start( backend::bootstrap_agents(&session, cfg, inst, mode, &guest_host)?; } if let Some(cmd) = post_start { + // Agent bootstrap may have just minted the per-instance capability + // token (proxy mode), which is forwarded to sessions via `SendEnv` + // (Codex's `COOP_LOCAL_API_KEY`). The session above was built before + // the token existed, so re-prepare it here — otherwise a `post_start` + // that runs Codex in proxy mode would lack the token and fail to + // authenticate. Under --no-agents no proxy started, so nothing new to + // pick up; keep the original session. + let session = if opts.no_agents { + session + } else { + prepare_session_from_target(cfg, Some(inst), target.clone(), repo)? + }; backend::run_post_start(&session, cmd); } Ok(()) @@ -1530,21 +1561,64 @@ pub(crate) fn prepare_session_from_target( target: backend::SshTarget, repo: Option<&github_repo::RepoSlug>, ) -> Result { - let mut env = backend::prepare_env_forwarding(cfg, repo)?; + // Load the per-instance model selection once: it decides both the + // proxy-mode key suppression and the Codex local-key forwarding below. + let model = match inst { + Some(inst) => Some(model_state::ModelState::load_or_default(inst)?), + None => None, + }; + + // In proxy mode (issue #411: an effective upstream — `[proxy.]` + // default or a per-VM override — and the VM in remote model mode) the raw + // key must never be forwarded into the guest; the host-side proxy holds it + // and the guest gets only the capability token. Overrides live in the + // instance state, so this is resolved per provider and needs `inst`. + let remote = model + .as_ref() + .is_some_and(|m| m.mode == model_state::ModelMode::Remote); + let (proxy_anthropic, proxy_openai) = match inst { + Some(inst) if remote => ( + proxy_state::effective_upstream(inst, proxy::Provider::Anthropic, &cfg.proxy)? + .is_some(), + proxy_state::effective_upstream(inst, proxy::Provider::Openai, &cfg.proxy)?.is_some(), + ), + _ => (false, false), + }; + + let mut env = backend::prepare_env_forwarding(cfg, repo, proxy_anthropic, proxy_openai)?; if let Some(inst) = inst { if let Some(state) = guest_env_state::GuestEnvState::try_load(inst)? { for (name, value) in &state.entries { + // In proxy mode a raw provider key snapshotted via `coop start + // --env` must not re-enter the guest — the host-side proxy + // holds it. Skip and warn, matching `prepare_env_forwarding`. + if (proxy_anthropic && name.as_str() == "ANTHROPIC_API_KEY") + || (proxy_openai && name.as_str() == "OPENAI_API_KEY") + { + tracing::warn!( + "proxy mode: ignoring runtime --env entry '{}' — the raw key stays on the host", + name.as_str() + ); + continue; + } env.set(name.as_str(), value.as_str()); } } - // In local-model mode, Codex reads its API key from the env var - // named by the provider's `env_key`. Claude's token rides in - // settings.json instead, so it needs no forwarding here. - let model = model_state::ModelState::load_or_default(inst)?; - if model.mode == model_state::ModelMode::Local - && let Some(ep) = model.resolved_codex(&cfg.codex) - { - env.set(model_state::CODEX_LOCAL_ENV_KEY, ep.auth_token_or_default()); + // Codex reads its provider key from the env var named by `env_key`. In + // local-model mode that is the local endpoint's token; in remote proxy + // mode it is the per-instance capability token (the raw key stays on + // the host). Claude's token rides in settings.json instead. The two + // modes are mutually exclusive (mode is Local or Remote). + if let Some(model) = &model { + if model.mode == model_state::ModelMode::Local + && let Some(ep) = model.resolved_codex(&cfg.codex) + { + env.set(model_state::CODEX_LOCAL_ENV_KEY, ep.auth_token_or_default()); + } else if proxy_openai + && let Some(token) = proxy::read_capability_token(inst, proxy::Provider::Openai) + { + env.set(model_state::CODEX_LOCAL_ENV_KEY, token); + } } } Ok(backend::SshSession { target, env }) @@ -1572,6 +1646,9 @@ pub(crate) fn cmd_stop( port_forward::teardown_ssh_forwards(inst, &target); } } + // Tear down the credential proxy (issue #411) — best-effort, no-op when + // proxy mode was never on. + crate::proxy::stop(inst); // The `coop-` SSH alias is left in place across stop: a stale // entry has no effect while the VM is down, and `coop start` refreshes // it (the Lima port changes per boot). `destroy`/`ssh-config --clean` @@ -1595,6 +1672,7 @@ pub(crate) fn cmd_destroy( if let Ok(target) = be.ssh_target(cfg, &inst) { port_forward::teardown_ssh_forwards(&inst, &target); } + crate::proxy::stop(&inst); be.destroy_instance(cfg, &inst)?; workspace::remove_ssh_config(&inst)?; tracing::info!("Instance '{}' destroyed", inst.name); @@ -2062,6 +2140,61 @@ mod tests { ); } + /// In proxy mode a raw `ANTHROPIC_API_KEY` persisted via `coop start + /// --env` must be dropped from the overlay, not re-injected into the guest + /// (issue #411). The VM defaults to remote model mode, so a configured + /// `[proxy.anthropic]` upstream makes proxy mode active. + #[test] + fn prepare_session_suppresses_persisted_proxy_key() { + use std::num::NonZeroU16; + + let tmp = tempfile::tempdir().expect("tempdir"); + let inst = super::config::Instance { + name: super::config::InstanceName::new("test").expect("valid name"), + index: super::config::InstanceIndex::new(0).expect("0 is in range"), + dir: tmp.path().to_path_buf(), + image: super::config::ImageName::new(super::config::DEFAULT_IMAGE) + .expect("DEFAULT_IMAGE is valid"), + }; + let mut state = super::guest_env_state::GuestEnvState::default(); + state.entries.insert( + super::guest_env_state::EnvVarName::new("ANTHROPIC_API_KEY").expect("valid env var"), + "sk-ant-realkey".to_string(), + ); + state.entries.insert( + super::guest_env_state::EnvVarName::new("FROM_CLI").expect("valid env var"), + "saved-value".to_string(), + ); + state.save(&inst).expect("save snapshot"); + + let mut cfg = super::config::CoopConfig::default(); + cfg.proxy.anthropic = Some(super::config::ProxyUpstream { + credential: super::config::Secret::new("cmd:true".to_string()), + auth: super::config::ProxyAuthScheme::ApiKey, + }); + + let target = super::backend::SshTarget { + host: super::backend::Hostname::new("127.0.0.1").expect("valid host"), + port: NonZeroU16::new(22).expect("non-zero"), + user: super::backend::SshUser::new("ubuntu").expect("valid user"), + key_path: tmp.path().join("id_test"), + }; + + let session = + super::prepare_session_from_target(&cfg, Some(&inst), target, None).expect("session"); + + let envs = session.env.as_envs(); + assert!( + !envs.contains_key("ANTHROPIC_API_KEY"), + "proxy mode re-injected the raw key from the persisted --env overlay", + ); + assert_eq!( + envs.get("FROM_CLI").map(String::as_str), + Some("saved-value"), + "non-suppressed --env entries must still flow through", + ); + } + #[test] fn prepare_session_skips_overlay_without_instance() { use std::num::NonZeroU16; diff --git a/src/commands/mod.rs b/src/commands/mod.rs index ee90067..adb8624 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -12,6 +12,7 @@ pub(crate) mod json; mod lifecycle; mod model; mod profiles; +mod proxy; mod quickstart; pub(crate) use admin::{UninstallOpts, cmd_init, cmd_uninstall, cmd_validate}; @@ -30,6 +31,7 @@ pub(crate) use lifecycle::{ }; pub(crate) use model::cmd_model; pub(crate) use profiles::{cmd_images, cmd_profiles}; +pub(crate) use proxy::cmd_proxy; pub(crate) use quickstart::{QuickstartOpts, cmd_quickstart}; use anyhow::Result; @@ -72,6 +74,7 @@ fn purge_all_data(be: &backend::PlatformBackend, cfg: &config::CoopConfig) -> Re if let Ok(target) = be.ssh_target(cfg, inst) { port_forward::teardown_ssh_forwards(inst, &target); } + crate::proxy::stop(inst); be.destroy_instance(cfg, inst)?; workspace::remove_ssh_config(inst)?; } diff --git a/src/commands/proxy.rs b/src/commands/proxy.rs new file mode 100644 index 0000000..f824a92 --- /dev/null +++ b/src/commands/proxy.rs @@ -0,0 +1,568 @@ +//! `coop proxy` subcommands (issue #411). +//! +//! `coop proxy setup` mirrors the GitHub PAT wizard: take a provider +//! credential pasted at the prompt, store it in a secret backend of the user's +//! choice, and write the resulting `cmd:` reference into `[proxy.]` +//! (the global default) or into a per-VM override — so the credential is never +//! a plaintext value in the config, and (in proxy mode) never enters the +//! guest. `coop proxy status` shows what each VM's agents resolve to, with +//! credentials redacted. + +#![expect( + clippy::print_stderr, + reason = "setup wizard is interactive CLI — stderr is user communication" +)] + +use std::io::Write as _; +use std::path::Path; + +use anyhow::{Context, Result}; + +use crate::ProxyAction; +use crate::config::{ + CoopConfig, InstanceName, ProxyAuthScheme, ProxyConfig, ProxyUpstream, Secret, +}; +use crate::github_pat::{pick_backend, read_token_no_echo}; +use crate::proxy::Provider; +use crate::proxy_state::ProxyState; +use crate::secret_store::{self, ANTHROPIC_SERVICE, AccountName, OPENAI_SERVICE}; + +pub(crate) fn cmd_proxy(cfg: &CoopConfig, config_path: &Path, action: &ProxyAction) -> Result<()> { + match action { + ProxyAction::Setup { + openai, + anthropic: _, + vm, + api_key, + } => { + let provider = if *openai { + Provider::Openai + } else { + Provider::Anthropic + }; + run_setup(cfg, config_path, provider, vm.as_deref(), *api_key) + } + ProxyAction::Status { vm } => run_status(cfg, vm.as_deref()), + } +} + +// ── setup ──────────────────────────────────────────────────── + +fn run_setup( + cfg: &CoopConfig, + config_path: &Path, + provider: Provider, + vm: Option<&str>, + api_key: bool, +) -> Result<()> { + // Validate the target VM (name + existence) up front, before the raw `--vm` + // string reaches the secret-store filesystem path or a secret is written: + // an invalid name must not become a path segment, and an unknown VM must + // not leave an orphaned secret behind (parse-don't-validate at the boundary). + let inst = match vm { + Some(vm) => { + let name = InstanceName::new(vm) + .with_context(|| format!("'{vm}' is not a valid instance name"))?; + Some(cfg.resolve_instance(Some(&name))?) + } + None => None, + }; + + let auth = auth_for(provider, api_key); + print_setup_guidance(provider, auth); + + let token = read_token_no_echo()?; + let backend = pick_backend()?; + + let service = service_for(provider, inst.as_ref().map(|i| i.name.as_str())); + let account = + AccountName::new(provider.name()).context("internal error: invalid proxy account name")?; + let state_dir = cfg.data_dir.join("state"); + let cmd_token = secret_store::store_secret(backend, &service, &account, &token, &state_dir) + .with_context(|| { + format!( + "Failed to store the {} credential in the chosen backend", + provider.name() + ) + })?; + + match &inst { + Some(inst) => store_vm_override(inst, provider, &cmd_token.to_string(), auth)?, + None => upsert_proxy_entry(config_path, provider, &cmd_token.to_string(), auth)?, + } + + report_setup(config_path, provider, vm, &cmd_token.to_string(), auth); + Ok(()) +} + +/// The injection scheme for a provider. `OpenAI` keys are always sent as +/// `Authorization: Bearer`; Anthropic uses `x-api-key` for an API key and +/// `Bearer` for a `setup-token`. +fn auth_for(provider: Provider, api_key: bool) -> ProxyAuthScheme { + match provider { + Provider::Anthropic if api_key => ProxyAuthScheme::ApiKey, + Provider::Anthropic | Provider::Openai => ProxyAuthScheme::Bearer, + } +} + +fn print_setup_guidance(provider: Provider, auth: ProxyAuthScheme) { + match provider { + Provider::Openai => eprintln!( + "Paste your OpenAI API key (sk-...). It is stored in your chosen secret backend, \ + never as plaintext in the config, and injected as Authorization: Bearer upstream. \ + Codex subscription (auth.json) is not supported in proxy mode — use an API key." + ), + Provider::Anthropic if auth == ProxyAuthScheme::ApiKey => eprintln!( + "Paste your Anthropic API key (sk-ant-...). It is stored in your chosen secret \ + backend, never as plaintext in the config." + ), + Provider::Anthropic => { + eprintln!( + "Run `claude setup-token` (needs a Claude subscription) to generate a long-lived, \ + inference-scoped token (~1 year), then paste it below." + ); + eprintln!( + "It is stored in your chosen secret backend, never as plaintext in the config or \ + in the guest." + ); + } + } +} + +fn report_setup( + config_path: &Path, + provider: Provider, + vm: Option<&str>, + credential_cmd: &str, + auth: ProxyAuthScheme, +) { + match vm { + Some(vm) => eprintln!( + "\nStored a per-VM {} credential override for '{vm}' (auth = \"{}\").", + provider.name(), + auth_str(auth), + ), + None => eprintln!( + "\nWrote {}:\n [proxy.{}]\n credential = \"{credential_cmd}\"\n auth = \"{}\"", + config_path.display(), + provider.name(), + auth_str(auth), + ), + } + eprintln!( + "\nProxy mode is now configured. It applies to remote-mode VMs on both backends; the raw \ + credential stays on the host and is injected upstream, never forwarded into the guest." + ); +} + +/// Persist a per-VM credential override in the (already-resolved) instance's +/// `proxy.json`. +fn store_vm_override( + inst: &crate::config::Instance, + provider: Provider, + credential_cmd: &str, + auth: ProxyAuthScheme, +) -> Result<()> { + let mut state = ProxyState::load_or_default(inst)?; + *state.slot_mut(provider) = Some(ProxyUpstream { + credential: Secret::new(credential_cmd.to_string()), + auth, + }); + state.save(inst) +} + +/// The secret-store service name for a provider, optionally per-VM. A per-VM +/// override appends `-` so it never collides with the global default. +fn service_for(provider: Provider, vm: Option<&str>) -> String { + let base = match provider { + Provider::Anthropic => ANTHROPIC_SERVICE, + Provider::Openai => OPENAI_SERVICE, + }; + match vm { + Some(vm) => format!("{base}-{vm}"), + None => base.to_string(), + } +} + +/// The `auth` string written to config for a scheme. Must match +/// `ProxyAuthScheme`'s serde encoding (asserted in tests). +fn auth_str(auth: ProxyAuthScheme) -> &'static str { + match auth { + ProxyAuthScheme::ApiKey => "api_key", + ProxyAuthScheme::Bearer => "bearer", + } +} + +/// Insert or replace `[proxy.]`'s `credential` + `auth` in the config +/// at `config_path`, preserving every other key. Round-trips through +/// `toml::Value` (comments are lost, all keys kept), under an flock so a +/// concurrent writer can't corrupt the file. +fn upsert_proxy_entry( + config_path: &Path, + provider: Provider, + credential_cmd: &str, + auth: ProxyAuthScheme, +) -> Result<()> { + let _lock = crate::fs_util::lock_sibling(config_path)?; + let mut doc = read_or_init_doc(config_path)?; + + let root = doc + .as_table_mut() + .context("config root is not a TOML table")?; + let proxy = root + .entry("proxy".to_string()) + .or_insert_with(|| toml::Value::Table(toml::Table::new())); + let proxy_tbl = proxy.as_table_mut().context("[proxy] is not a table")?; + let entry = proxy_tbl + .entry(provider.name().to_string()) + .or_insert_with(|| toml::Value::Table(toml::Table::new())); + let entry_tbl = entry + .as_table_mut() + .with_context(|| format!("[proxy.{}] is not a table", provider.name()))?; + entry_tbl.insert( + "credential".to_string(), + toml::Value::String(credential_cmd.to_string()), + ); + entry_tbl.insert( + "auth".to_string(), + toml::Value::String(auth_str(auth).to_string()), + ); + + let serialized = toml::to_string_pretty(&doc).context("Failed to serialize config")?; + // `coop proxy setup` always writes a `cmd:` indirection (not the secret), + // so 0644 matches coop's convention; a literal (BYO) credential gets 0600. + let mode = if credential_cmd.starts_with("cmd:") { + 0o644 + } else { + 0o600 + }; + crate::fs_util::atomic_write_with_mode(config_path, &serialized, mode) + .with_context(|| format!("Failed to write {}", config_path.display())) +} + +fn read_or_init_doc(path: &Path) -> Result { + if path.exists() { + let s = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read {}", path.display()))?; + toml::from_str::(&s) + .with_context(|| format!("Failed to parse {}", path.display())) + } else { + Ok(toml::Value::Table(toml::Table::new())) + } +} + +// ── status ─────────────────────────────────────────────────── + +/// Where a VM's effective credential for a provider comes from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Source { + /// A per-VM override in the instance's `proxy.json`. + Override, + /// The `[proxy.]` config default. + Default, + /// Neither configured — the proxy is off for this provider. + Off, +} + +/// The redacted resolution of one provider for one VM (or the defaults view). +#[derive(Debug, Clone, PartialEq, Eq)] +struct Resolution { + source: Source, + auth: Option, + credential: Option, +} + +fn run_status(cfg: &CoopConfig, vm: Option<&str>) -> Result<()> { + let out = &mut std::io::stdout(); + if let Some(name) = vm { + let name = InstanceName::new(name) + .with_context(|| format!("'{name}' is not a valid instance name"))?; + let inst = cfg.resolve_instance(Some(&name))?; + let state = ProxyState::load_or_default(&inst)?; + write_vm_status(out, inst.name.as_str(), &state, &cfg.proxy)?; + } else { + writeln!(out, "Credential proxy — defaults ([proxy] in config.toml):")?; + for provider in Provider::ALL { + let res = resolve(provider, None, &cfg.proxy); + writeln!(out, " {:<10} {}", provider.name(), describe(&res))?; + } + let mut overrides = Vec::new(); + for inst in cfg.list_instances()? { + match ProxyState::load_or_default(&inst) { + Ok(state) if state.anthropic.is_some() || state.openai.is_some() => { + overrides.push((inst, state)); + } + Ok(_) => {} + // Surface a corrupt override rather than silently reporting the + // VM as "no override" (which would mislead the operator). + Err(e) => tracing::warn!( + "Skipping '{}' in proxy status — unreadable proxy.json: {e:#}", + inst.name + ), + } + } + if overrides.is_empty() { + writeln!(out, "\nNo per-VM overrides.")?; + } else { + writeln!(out, "\nPer-VM overrides:")?; + for (inst, state) in overrides { + write_vm_status(out, inst.name.as_str(), &state, &cfg.proxy)?; + } + } + } + writeln!( + out, + "\nValues are cmd: references, not secrets. Each running VM in remote model mode routes \ + its agents through the proxy; the raw credential stays on the host." + )?; + Ok(()) +} + +fn write_vm_status( + out: &mut impl std::io::Write, + vm: &str, + state: &ProxyState, + cfg: &ProxyConfig, +) -> Result<()> { + writeln!(out, " {vm}:")?; + for provider in Provider::ALL { + let res = resolve(provider, Some(state), cfg); + writeln!(out, " {:<10} {}", provider.name(), describe(&res))?; + } + Ok(()) +} + +/// Resolve a provider to its redacted [`Resolution`]: per-VM override → +/// config default → off. `state` is `None` for the defaults-only view. +fn resolve(provider: Provider, state: Option<&ProxyState>, cfg: &ProxyConfig) -> Resolution { + let override_ = state.and_then(|s| match provider { + Provider::Anthropic => s.anthropic.as_ref(), + Provider::Openai => s.openai.as_ref(), + }); + let default = match provider { + Provider::Anthropic => cfg.anthropic.as_ref(), + Provider::Openai => cfg.openai.as_ref(), + }; + match override_.or(default) { + Some(up) => Resolution { + source: if override_.is_some() { + Source::Override + } else { + Source::Default + }, + auth: Some(up.auth), + credential: Some(redact_credential(up.credential.expose())), + }, + None => Resolution { + source: Source::Off, + auth: None, + credential: None, + }, + } +} + +/// A one-line human description of a [`Resolution`]. +fn describe(res: &Resolution) -> String { + let src = match res.source { + Source::Off => return "off (no default, no override)".to_string(), + Source::Override => "override", + Source::Default => "default", + }; + let auth = res.auth.map_or("?", auth_str); + let cred = res.credential.as_deref().unwrap_or("?"); + format!("{src} — {auth}, {cred}") +} + +/// Show a `cmd:` reference verbatim (it is a command, not a secret) but never +/// a literal credential a user may have hand-written into config/state. +fn redact_credential(credential: &str) -> String { + if credential.starts_with("cmd:") { + credential.to_string() + } else { + "".to_string() + } +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "tests")] +mod tests { + use super::*; + + fn config_at(dir: &Path, contents: &str) -> std::path::PathBuf { + let path = dir.join("config.toml"); + std::fs::write(&path, contents).unwrap(); + path + } + + fn upstream(cred: &str, auth: ProxyAuthScheme) -> ProxyUpstream { + ProxyUpstream { + credential: Secret::new(cred.to_string()), + auth, + } + } + + #[test] + fn auth_str_matches_serde_encoding() { + // Guard against drift from ProxyAuthScheme's serde rename. + for scheme in [ProxyAuthScheme::ApiKey, ProxyAuthScheme::Bearer] { + let serde_str = toml::Value::try_from(scheme).unwrap(); + assert_eq!(serde_str.as_str(), Some(auth_str(scheme))); + } + } + + #[test] + fn auth_for_openai_is_always_bearer() { + assert_eq!(auth_for(Provider::Openai, false), ProxyAuthScheme::Bearer); + assert_eq!(auth_for(Provider::Openai, true), ProxyAuthScheme::Bearer); + assert_eq!(auth_for(Provider::Anthropic, true), ProxyAuthScheme::ApiKey); + assert_eq!( + auth_for(Provider::Anthropic, false), + ProxyAuthScheme::Bearer + ); + } + + #[test] + fn service_for_namespaces_provider_and_vm() { + assert_eq!(service_for(Provider::Anthropic, None), "coop-anthropic"); + assert_eq!(service_for(Provider::Openai, None), "coop-openai"); + assert_eq!( + service_for(Provider::Openai, Some("dev")), + "coop-openai-dev" + ); + assert_eq!( + service_for(Provider::Anthropic, Some("test")), + "coop-anthropic-test" + ); + } + + #[test] + fn upsert_writes_provider_block_parseable_by_config() { + let tmp = tempfile::TempDir::new().unwrap(); + let path = config_at(tmp.path(), ""); + upsert_proxy_entry( + &path, + Provider::Openai, + "cmd:echo tok", + ProxyAuthScheme::Bearer, + ) + .unwrap(); + + let cfg = CoopConfig::load(&path).unwrap(); + let up = cfg.proxy.openai.unwrap(); + assert_eq!(up.credential.expose(), "cmd:echo tok"); + assert_eq!(up.auth, ProxyAuthScheme::Bearer); + // Anthropic default is untouched. + assert!(cfg.proxy.anthropic.is_none()); + } + + #[test] + fn upsert_preserves_other_provider_and_config() { + let tmp = tempfile::TempDir::new().unwrap(); + let path = config_at( + tmp.path(), + "ssh_port = 2222\n\n[proxy.anthropic]\ncredential = \"cmd:a\"\nauth = \"bearer\"\n", + ); + upsert_proxy_entry(&path, Provider::Openai, "cmd:o", ProxyAuthScheme::Bearer).unwrap(); + + let cfg = CoopConfig::load(&path).unwrap(); + assert_eq!(cfg.ssh_port.get(), 2222); + assert_eq!(cfg.proxy.anthropic.unwrap().credential.expose(), "cmd:a"); + assert_eq!(cfg.proxy.openai.unwrap().credential.expose(), "cmd:o"); + } + + #[test] + fn cmd_reference_config_is_world_readable() { + use std::os::unix::fs::PermissionsExt; + let tmp = tempfile::TempDir::new().unwrap(); + let path = config_at(tmp.path(), ""); + upsert_proxy_entry( + &path, + Provider::Anthropic, + "cmd:echo tok", + ProxyAuthScheme::Bearer, + ) + .unwrap(); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o644, "a cmd: reference is not a secret"); + } + + #[test] + fn literal_credential_config_is_owner_only() { + use std::os::unix::fs::PermissionsExt; + let tmp = tempfile::TempDir::new().unwrap(); + let path = config_at(tmp.path(), ""); + upsert_proxy_entry( + &path, + Provider::Anthropic, + "sk-ant-literal", + ProxyAuthScheme::ApiKey, + ) + .unwrap(); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "a literal secret must be owner-only"); + } + + #[test] + fn resolve_prefers_override_then_default_then_off() { + let cfg = ProxyConfig { + anthropic: Some(upstream("cmd:default", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + let state = ProxyState { + anthropic: Some(upstream("cmd:override", ProxyAuthScheme::ApiKey)), + ..Default::default() + }; + + let over = resolve(Provider::Anthropic, Some(&state), &cfg); + assert_eq!(over.source, Source::Override); + assert_eq!(over.credential.as_deref(), Some("cmd:override")); + + let def = resolve(Provider::Anthropic, None, &cfg); + assert_eq!(def.source, Source::Default); + assert_eq!(def.credential.as_deref(), Some("cmd:default")); + + let off = resolve(Provider::Openai, Some(&state), &cfg); + assert_eq!(off.source, Source::Off); + } + + #[test] + fn describe_redacts_literal_and_shows_cmd() { + let mut cfg = ProxyConfig { + openai: Some(upstream("cmd:op read x", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + let shown = describe(&resolve(Provider::Openai, None, &cfg)); + assert!( + shown.contains("cmd:op read x"), + "cmd: ref should be shown: {shown}" + ); + + cfg.openai = Some(upstream("sk-secret-literal", ProxyAuthScheme::Bearer)); + let redacted = describe(&resolve(Provider::Openai, None, &cfg)); + assert!( + !redacted.contains("sk-secret-literal"), + "literal leaked: {redacted}" + ); + assert!(redacted.contains("redacted")); + } + + #[test] + fn write_vm_status_lists_both_providers() { + let cfg = ProxyConfig { + anthropic: Some(upstream("cmd:a", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + let state = ProxyState::default(); + let mut buf = Vec::new(); + write_vm_status(&mut buf, "dev", &state, &cfg).unwrap(); + let text = String::from_utf8(buf).unwrap(); + assert!(text.contains("dev:")); + assert!(text.contains("anthropic")); + assert!(text.contains("openai")); + assert!( + text.contains("default"), + "anthropic should resolve to default: {text}" + ); + assert!(text.contains("off"), "openai should be off: {text}"); + } +} diff --git a/src/config.rs b/src/config.rs index b5ce7d5..d4ac96e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -718,6 +718,12 @@ pub struct CoopConfig { #[serde(default)] pub codex: CodexConfig, + /// Host-side credential-injecting proxy (issue #411). Opt-in: when an + /// upstream is configured, the real credential stays on the host and the + /// guest is pointed at a local proxy instead of receiving the key. + #[serde(default)] + pub proxy: ProxyConfig, + /// Literal env vars to set in the guest, independent of the host /// process environment. Merged with `env_forward` results during /// SSH setup; entries here override forwarded values (with a @@ -1513,6 +1519,61 @@ pub struct CodexConfig { pub local_model: Option, } +/// `[proxy]` — host-side credential-injecting proxy (issue #411). +/// +/// When an upstream is configured, coop runs a `coop-proxy` process on the +/// host for the lifetime of each remote-mode VM: the guest is pointed at the +/// proxy (a base-URL override) and holds only a per-instance capability +/// token, while the real credential stays on the host and is injected onto +/// outbound requests the guest never sees. Absent config means no proxy — +/// credentials are forwarded exactly as before. +/// +/// Covers Anthropic (Claude Code) and `OpenAI` (Codex); GitHub is a separate +/// slice. Each provider is an optional default; a VM can override its own +/// credential per provider (see [`crate::proxy_state`]). +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct ProxyConfig { + /// Anthropic (Claude Code) upstream. Its presence enables proxy mode for + /// Claude when the VM is in remote model mode. + #[serde(default)] + pub anthropic: Option, + + /// `OpenAI` (Codex) upstream. Its presence enables proxy mode for Codex + /// when the VM is in remote model mode. `OpenAI` API keys are injected as + /// `Authorization: Bearer`. + /// + /// Proxy mode is resolved per provider and per VM (a per-VM override can + /// enable it even when this default is absent) by + /// [`crate::proxy_state::effective_upstream`], not from this struct alone. + #[serde(default)] + pub openai: Option, +} + +/// A single proxied upstream: the real credential and how to inject it. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ProxyUpstream { + /// The real credential (an API key or a Claude `setup-token`), a plain + /// value or a `cmd:` indirection resolved via [`resolve_cmd_value`] at + /// proxy start — never written to disk, never forwarded into the guest. + pub credential: Secret, + + /// How the proxy injects the credential upstream. Defaults to `api_key` + /// (`x-api-key`); use `bearer` for a Claude `setup-token`. + #[serde(default)] + pub auth: ProxyAuthScheme, +} + +/// The header form the proxy injects the real credential as. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ProxyAuthScheme { + /// `x-api-key: ` — the Anthropic API-key form. + #[default] + ApiKey, + /// `authorization: Bearer ` — a Claude `setup-token`. + Bearer, +} + /// A local (host-side) model endpoint that `coop model local` /// materializes into guest agent config. /// @@ -2267,6 +2328,7 @@ impl Default for CoopConfig { setup: SetupConfig::default(), claude: ClaudeConfig::default(), codex: CodexConfig::default(), + proxy: ProxyConfig::default(), guest_env: BTreeMap::new(), profiles: HashMap::new(), post_start: None, @@ -2402,6 +2464,12 @@ impl Instance { self.dir.join("model.json") } + /// Per-instance proxy credential overrides (issue #411). A missing file + /// means "no override — use the `[proxy.]` defaults." + pub fn proxy_state_path(&self) -> PathBuf { + self.dir.join("proxy.json") + } + #[mutants::skip] // equivalent: default-path getter; no caller asserts the returned PathBuf pub fn devcontainer_state_path(&self) -> PathBuf { self.dir.join("devcontainer_state.json") @@ -2884,12 +2952,34 @@ mod tests { #[cfg(target_os = "linux")] fn spawn_firecracker_like() -> std::process::Child { - std::process::Command::new("bash") - .args(["-c", "exec -a firecracker-test sleep 30"]) + use std::os::unix::process::CommandExt; + // Set argv[0] to a firecracker-like name in a single execve (no shell + // `exec -a` indirection). A shell wrapper would exec twice, and reading + // /proc//cmdline during the second transition races an empty + // cmdline. `wait_for_firecracker_cmdline` then waits out the one + // remaining fork→execve gap before the process is inspected. + std::process::Command::new("sleep") + .arg0("firecracker-test") + .arg("30") .spawn() .unwrap() } + /// Wait until a spawned child's `/proc//cmdline` reflects its + /// post-`execve` argv. Between `spawn` and `execve` the cmdline is empty, so + /// process-detection assertions must not run until it settles. + #[cfg(target_os = "linux")] + fn wait_for_firecracker_cmdline(pid: u32) { + let path = format!("/proc/{pid}/cmdline"); + for _ in 0..500 { + if fs::read_to_string(&path).is_ok_and(|c| c.contains("firecracker")) { + return; + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + panic!("firecracker-like process {pid} never exposed its cmdline"); + } + #[cfg(target_os = "linux")] fn spawn_sleep() -> std::process::Child { std::process::Command::new("sleep") @@ -2945,6 +3035,7 @@ mod tests { let tmp = TempDir::new().unwrap(); let inst = test_inst("test", idx(0), tmp.path().to_path_buf()); let mut child = spawn_firecracker_like(); + wait_for_firecracker_cmdline(child.id()); fs::write(inst.pid_file_path(), child.id().to_string()).unwrap(); let running = inst.is_running(); @@ -2985,6 +3076,7 @@ mod tests { fn is_firecracker_process_true_for_firecracker_named_pid() { let mut child = spawn_firecracker_like(); let pid = child.id(); + wait_for_firecracker_cmdline(pid); let result = is_firecracker_process(pid); let _ = child.kill(); let _ = child.wait(); @@ -3454,6 +3546,63 @@ model = "" assert!(toml::from_str::(toml_str).is_err()); } + #[test] + fn proxy_disabled_by_default() { + let cfg: CoopConfig = toml::from_str("").unwrap(); + assert!(cfg.proxy.anthropic.is_none()); + assert!(cfg.proxy.openai.is_none()); + } + + #[test] + fn proxy_anthropic_parses_and_defaults_to_api_key() { + let toml_str = r#" +[proxy.anthropic] +credential = "sk-ant-secret" +"#; + let cfg: CoopConfig = toml::from_str(toml_str).unwrap(); + let up = cfg.proxy.anthropic.unwrap(); + assert_eq!(up.credential.expose(), "sk-ant-secret"); + assert_eq!(up.auth, ProxyAuthScheme::ApiKey); + } + + #[test] + fn proxy_openai_parses_with_bearer() { + let toml_str = r#" +[proxy.openai] +credential = "cmd:op read op://x" +auth = "bearer" +"#; + let cfg: CoopConfig = toml::from_str(toml_str).unwrap(); + let up = cfg.proxy.openai.unwrap(); + assert_eq!(up.credential.expose(), "cmd:op read op://x"); + assert_eq!(up.auth, ProxyAuthScheme::Bearer); + } + + #[test] + fn proxy_anthropic_parses_bearer_scheme() { + let toml_str = r#" +[proxy.anthropic] +credential = "cmd:echo tok" +auth = "bearer" +"#; + let cfg: CoopConfig = toml::from_str(toml_str).unwrap(); + let up = cfg.proxy.anthropic.unwrap(); + assert_eq!(up.auth, ProxyAuthScheme::Bearer); + // Credential is stored verbatim (the `cmd:` is resolved at proxy + // start, not config parse). + assert_eq!(up.credential.expose(), "cmd:echo tok"); + } + + #[test] + fn proxy_rejects_unknown_auth_scheme() { + let toml_str = r#" +[proxy.anthropic] +credential = "x" +auth = "basic" +"#; + assert!(toml::from_str::(toml_str).is_err()); + } + #[test] fn github_auth_deserialization() { assert!(matches!( diff --git a/src/github_pat.rs b/src/github_pat.rs index 0e8b782..fe4962b 100644 --- a/src/github_pat.rs +++ b/src/github_pat.rs @@ -405,7 +405,9 @@ fn open_browser_best_effort(url: &str) { let _ = Command::new(opener).arg(url).spawn(); } -fn read_token_no_echo() -> Result { +/// Read a pasted secret from the terminal without echoing it. Shared with +/// `coop proxy setup` (issue #411). +pub(crate) fn read_token_no_echo() -> Result { eprint!("Paste token: "); std::io::stderr().flush().ok(); let guard = EchoGuard::off(); @@ -901,7 +903,9 @@ fn print_widen_instructions(parent: &RepoSlug, expected: &[RepoSlug], failed: &[ eprintln!("Generate a fresh token, then paste it below. (Empty input aborts.)"); } -fn pick_backend() -> Result { +/// Prompt the user to choose a secret-storage backend. Shared with +/// `coop proxy setup` (issue #411). +pub(crate) fn pick_backend() -> Result { let backends = available_backends(); eprintln!("\nWhere should I store this token?"); for (i, b) in backends.iter().enumerate() { diff --git a/src/lib.rs b/src/lib.rs index c458999..871e97f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,8 @@ mod naming; mod pat_prompt; mod paths; mod port_forward; +mod proxy; +mod proxy_state; mod remote_command; mod secret_store; mod sha256_hash; @@ -70,8 +72,8 @@ use commands::{ QuickstartOpts, ResizeOpts, StartOpts, UninstallOpts, UpDevcontainerOpts, UpOpts, UpRuntimeOpts, apply_runtime_guest_env, apply_vm_overrides, cmd_agent_update, cmd_commit, cmd_destroy, cmd_devcontainer, cmd_devcontainer_check, cmd_exec, cmd_github, cmd_images, - cmd_init, cmd_list, cmd_model, cmd_profiles, cmd_quickstart, cmd_resize, cmd_restore, - cmd_shell, cmd_start, cmd_status, cmd_stop, cmd_uninstall, cmd_up, cmd_validate, + cmd_init, cmd_list, cmd_model, cmd_profiles, cmd_proxy, cmd_quickstart, cmd_resize, + cmd_restore, cmd_shell, cmd_start, cmd_status, cmd_stop, cmd_uninstall, cmd_up, cmd_validate, codex_launch_args, open_ssh_session, preflight_start_target, prepend_binary, resolve_devcontainer, resolve_devcontainer_collect, resolve_running, }; @@ -606,6 +608,11 @@ enum Commands { #[command(subcommand)] action: GithubAction, }, + /// Manage the credential-injecting proxy (issue #411) + Proxy { + #[command(subcommand)] + action: ProxyAction, + }, /// Validate configuration and check prerequisites Validate { /// Probe live state for each `[github.pat]` entry (talks to api.github.com) @@ -764,6 +771,45 @@ enum GithubAction { }, } +#[derive(Subcommand)] +enum ProxyAction { + /// Store a provider credential in a secret backend and wire it into + /// `[proxy.]` (the default) or a per-VM override. Anthropic + /// (Claude) is the default provider; pass `--openai` for Codex. + Setup { + /// Configure the `OpenAI` (Codex) upstream instead of Anthropic. `OpenAI` + /// keys are always injected as `Authorization: Bearer`. + #[arg(long, conflicts_with = "anthropic")] + openai: bool, + /// Configure the Anthropic (Claude) upstream (the default provider). + #[arg(long)] + anthropic: bool, + /// Store the credential as a per-VM override for this instance instead + /// of the global `[proxy.]` default. + #[arg( + long, + value_name = "NAME", + add = ArgValueCandidates::new(completions::instance_candidates) + )] + vm: Option, + /// Anthropic only: store an API key (`x-api-key`) instead of a Claude + /// `setup-token`. Ignored for `--openai` (always Bearer). + #[arg(long)] + api_key: bool, + }, + /// Show what each VM's agents resolve to (per-VM override → default → off), + /// with credentials redacted. + Status { + /// Show the effective resolution for a single VM instead of all. + #[arg( + long, + value_name = "NAME", + add = ArgValueCandidates::new(completions::instance_candidates) + )] + vm: Option, + }, +} + #[derive(Subcommand)] enum DevcontainerCommands { /// Parse devcontainer.json and print coop's translation report. @@ -1309,6 +1355,7 @@ pub fn run() -> Result<()> { &action.unwrap_or(ProfilesAction::List { json: false }), ), Commands::Github { action } => cmd_github(&cfg, &cli.config, action), + Commands::Proxy { action } => cmd_proxy(&cfg, &cli.config, &action), Commands::Validate { probe } => cmd_validate(&cfg, &be, probe), Commands::Init | Commands::Update { .. } diff --git a/src/model_state.rs b/src/model_state.rs index 2fd49e4..f126da9 100644 --- a/src/model_state.rs +++ b/src/model_state.rs @@ -237,6 +237,76 @@ pub fn codex_local_config(base_url: &str, model: &str) -> toml::Table { root } +/// The `~/.codex/config.toml` keys coop merges to route Codex through the +/// host-side injecting proxy (issue #411). +/// +/// Mirrors [`codex_local_config`] but for proxy mode: it points the +/// `coop_local` provider's `base_url` at the proxy and selects it via +/// `model_provider`, while pinning **no** `model` — proxy mode is transparent, +/// so Codex keeps whatever model the user configured (or its default) and only +/// its egress is redirected. Codex sends the value of `env_key` +/// ([`CODEX_LOCAL_ENV_KEY`]) as `Authorization: Bearer`; coop forwards the +/// per-instance capability token there, which the proxy verifies before +/// injecting the real credential upstream. +/// +/// The `base_url` gets an `/v1` suffix: Codex forms the request URL as +/// `{base_url}/responses`, and the proxy forwards the path verbatim to +/// `api.openai.com`, so the guest must produce `/v1/responses` (`OpenAI`'s +/// Responses endpoint) — mirroring `OpenAI`'s own `https://api.openai.com/v1` +/// provider base. +pub fn codex_proxy_config(base_url: &str) -> toml::Table { + let mut provider = toml::Table::new(); + provider.insert( + "name".to_string(), + toml::Value::String("coop credential proxy".to_string()), + ); + provider.insert( + "base_url".to_string(), + toml::Value::String(format!("{}/v1", base_url.trim_end_matches('/'))), + ); + provider.insert( + "wire_api".to_string(), + toml::Value::String("responses".to_string()), + ); + provider.insert( + "env_key".to_string(), + toml::Value::String(CODEX_LOCAL_ENV_KEY.to_string()), + ); + + let mut providers = toml::Table::new(); + providers.insert( + CODEX_LOCAL_PROVIDER.to_string(), + toml::Value::Table(provider), + ); + + let mut root = toml::Table::new(); + root.insert( + "model_provider".to_string(), + toml::Value::String(CODEX_LOCAL_PROVIDER.to_string()), + ); + root.insert("model_providers".to_string(), toml::Value::Table(providers)); + root +} + +/// The `env` block coop writes into the managed `~/.claude/settings.json` +/// to point Claude Code at the host-side injecting proxy (issue #411). +/// +/// Unlike [`claude_env_block`] (local mode), this pins **nothing** about the +/// model or request shape — proxy mode is transparent, routing real cloud +/// traffic through the host so the real credential can be injected upstream. +/// It sets only the base-URL override and the per-instance capability token +/// (`ANTHROPIC_AUTH_TOKEN`), which Claude Code sends as `Authorization: +/// Bearer` — the token the proxy verifies before injecting the real key. +pub fn claude_proxy_env_block(base_url: &str, capability_token: &str) -> BTreeMap { + let mut env = BTreeMap::new(); + env.insert("ANTHROPIC_BASE_URL".to_string(), base_url.to_string()); + env.insert( + "ANTHROPIC_AUTH_TOKEN".to_string(), + capability_token.to_string(), + ); + env +} + #[cfg(test)] #[expect(clippy::unwrap_used, reason = "tests")] mod tests { @@ -461,6 +531,21 @@ mod tests { } } + #[test] + fn claude_proxy_env_block_sets_only_base_url_and_token() { + // Proxy mode is transparent: it must NOT pin any model tier or touch + // cache-buster keys (those are local-mode concerns) — only the base + // URL and the capability token. + let env = claude_proxy_env_block("http://172.16.0.1:8788", "cap-token"); + assert_eq!(env["ANTHROPIC_BASE_URL"], "http://172.16.0.1:8788"); + assert_eq!(env["ANTHROPIC_AUTH_TOKEN"], "cap-token"); + assert_eq!( + env.len(), + 2, + "proxy env block must set nothing else: {env:?}" + ); + } + #[test] fn claude_env_block_disables_prefix_cache_busters() { // Both keys keep a local KV cache warm by stopping Claude Code @@ -490,4 +575,43 @@ mod tests { assert_eq!(provider["wire_api"].as_str().unwrap(), "responses"); assert_eq!(provider["env_key"].as_str().unwrap(), CODEX_LOCAL_ENV_KEY); } + + #[test] + fn codex_proxy_config_pins_provider_but_not_model() { + let table = codex_proxy_config("http://127.0.0.1:8900"); + // Proxy mode is transparent: no model pin, so Codex keeps its own. + assert!( + !table.contains_key("model"), + "proxy mode must not pin a model" + ); + assert_eq!( + table["model_provider"].as_str().unwrap(), + CODEX_LOCAL_PROVIDER + ); + let provider = table["model_providers"][CODEX_LOCAL_PROVIDER] + .as_table() + .unwrap(); + assert_eq!( + provider["base_url"].as_str().unwrap(), + "http://127.0.0.1:8900/v1" + ); + assert_eq!(provider["wire_api"].as_str().unwrap(), "responses"); + assert_eq!(provider["env_key"].as_str().unwrap(), CODEX_LOCAL_ENV_KEY); + assert_eq!(provider["name"].as_str().unwrap(), "coop credential proxy"); + } + + #[test] + fn codex_proxy_config_collapses_trailing_slash_before_v1() { + // A slash-terminated base must not yield `//v1`; kills the + // `trim_end_matches('/')` deletion mutant that the no-slash case + // leaves alive. + let table = codex_proxy_config("http://127.0.0.1:8900/"); + let provider = table["model_providers"][CODEX_LOCAL_PROVIDER] + .as_table() + .unwrap(); + assert_eq!( + provider["base_url"].as_str().unwrap(), + "http://127.0.0.1:8900/v1" + ); + } } diff --git a/src/proxy.rs b/src/proxy.rs new file mode 100644 index 0000000..01c1402 --- /dev/null +++ b/src/proxy.rs @@ -0,0 +1,683 @@ +//! Host-side lifecycle for the credential-injecting proxy (issue #411). +//! +//! `coop-proxy` is a separate binary (its own workspace crate) that runs on +//! the host for the lifetime of a remote-mode VM. This module resolves the +//! real credential, mints a per-instance capability token, spawns the proxy +//! process (bound on host loopback), and exposes it into the guest with a +//! per-instance `ssh -R` reverse tunnel — both tracked by PID files. +//! +//! One `coop-proxy` process (and one reverse tunnel) runs per (VM, provider): +//! Anthropic for Claude Code, `OpenAI` for Codex. The binary is +//! provider-agnostic — it injects one configured header to one fixed upstream +//! — so a provider is just a different upstream host, port, and auth scheme +//! resolved on the host. See [`docs/design/issue-411-injecting-proxy.md`]. +//! +//! Binding host loopback + reverse-tunnelling works identically on both +//! backends (Firecracker and Lima), keeps the listener off every non-loopback +//! interface, and gives each guest its own tunnel (no shared-bridge exposure). +//! The guest is pointed at `http://127.0.0.1:` and holds only the +//! capability token, which the proxy verifies before injecting the real +//! credential upstream. + +use std::fs::{self, File}; +use std::io::{Read, Write}; +use std::net::SocketAddr; +use std::os::unix::process::CommandExt; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +use anyhow::{Context, Result, bail}; +use serde::Serialize; + +use crate::backend::SshTarget; +use crate::config::{Instance, ProxyAuthScheme, ProxyUpstream, Secret, resolve_cmd_value}; + +/// The proxy binary name, expected next to the `coop` binary. +const PROXY_BIN_NAME: &str = "coop-proxy"; + +/// How long to watch the reverse-tunnel `ssh` after spawn before treating it +/// as established. With `ExitOnForwardFailure`, a refused `-R` bind makes `ssh` +/// exit well within this window (the guest is already reachable — bootstrap +/// SSH'd in moments earlier), so surviving it means the forward is bound. +const TUNNEL_READY_GRACE: Duration = Duration::from_secs(2); + +/// How long to wait for the freshly spawned proxy to accept a connection on +/// its host-loopback listener before treating the launch as failed. This +/// enforces fail-closed startup: when confinement *fails to establish* the +/// proxy exits before binding (an unsupported kernel makes Landlock's +/// `apply` bail on Linux; a missing `sandbox-exec` or a malformed Seatbelt +/// profile makes the wrapper exit non-zero on macOS), so this probe never +/// connects and the VM start aborts rather than proceeding with a dead +/// credential proxy. It confirms the proxy *bound* — the *strength* of the +/// confinement is asserted separately by `coop-proxy --jail-selftest` in the +/// integration suite, not by liveness here. +const PROXY_READY_GRACE: Duration = Duration::from_secs(5); + +/// A proxied upstream. The binary is provider-agnostic; this enum carries the +/// host-side per-provider constants (upstream host, base port, file/tunnel +/// name) so one code path serves both. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Provider { + /// Anthropic (Claude Code) → `api.anthropic.com`. + Anthropic, + /// `OpenAI` (Codex) → `api.openai.com`. + Openai, +} + +impl Provider { + /// Every provider, for teardown that must reach all of them. + pub const ALL: [Provider; 2] = [Provider::Anthropic, Provider::Openai]; + + /// Short name used in PID/log/token file names and log lines. + pub fn name(self) -> &'static str { + match self { + Provider::Anthropic => "anthropic", + Provider::Openai => "openai", + } + } + + /// The fixed upstream host. The guest cannot influence this — only the + /// request path is forwarded (closes SSRF). + fn upstream_host(self) -> &'static str { + match self { + Provider::Anthropic => "api.anthropic.com", + Provider::Openai => "api.openai.com", + } + } + + /// Base port for this provider's per-instance proxy. The actual port is + /// `base + instance index`, so concurrent VMs never collide on host + /// loopback. The instance index spans `0..=252`, so the bases are 1000 + /// apart to keep the two providers' ranges disjoint (Anthropic + /// `8788..=9040`, `OpenAI` `9788..=10040`). + fn base_port(self) -> u16 { + match self { + Provider::Anthropic => 8788, + Provider::Openai => 9788, + } + } + + /// Per-instance listen port: base + index. + fn port(self, inst: &Instance) -> u16 { + self.base_port().saturating_add(inst.index.as_u16()) + } + + /// Whether the capability token must be persisted host-side for later + /// sessions to read. Codex needs it: the token is sent as the provider's + /// bearer `env_key`, forwarded via `SendEnv` on every session (which is + /// created before bootstrap mints the token). Claude Code does not — its + /// token rides in the guest's `settings.json`, written at bootstrap — so + /// its token stays in host memory only (unchanged from slice 1). + fn persists_token(self) -> bool { + matches!(self, Provider::Openai) + } +} + +/// A running proxy and the values the guest config needs. +#[derive(Debug, Clone)] +pub struct ProxyHandle { + /// Base URL the guest is pointed at, e.g. `http://127.0.0.1:8788`. + pub base_url: String, + /// Per-instance capability token the guest presents (as a bearer / API + /// key), which the proxy verifies before injecting the real credential. + /// Wrapped so a stray `Debug` of the handle never leaks it, matching the + /// hygiene of the credential-adjacent path. + pub capability_token: Secret, +} + +/// Start the proxy for one `provider` on `inst`: resolve the credential, bind +/// the proxy on host loopback, and open a reverse SSH tunnel via `target` so +/// the guest reaches it at `127.0.0.1:`. +/// +/// **Fail closed:** if the credential cannot be resolved (or the tunnel cannot +/// be established), this returns an error and the VM start is aborted — the +/// guest never comes up on a path where the agent silently has no or the wrong +/// credential. +pub fn start_provider( + inst: &Instance, + provider: Provider, + upstream: &ProxyUpstream, + target: &SshTarget, +) -> Result { + let credential = resolve_cmd_value(upstream.credential.expose()).with_context(|| { + format!( + "Failed to resolve the {} proxy credential — aborting VM start (fail-closed); \ + the guest must never come up without the injected credential", + provider.name() + ) + })?; + + let token = mint_capability_token()?; + let port = provider.port(inst); + let listen = SocketAddr::from(([127, 0, 0, 1], port)); + let json = wire_config_json( + &listen, + &token, + provider.upstream_host(), + upstream.auth, + &credential, + )?; + + spawn_proxy(inst, provider.name(), listen, &json)?; + // Persist the capability token for providers that forward it via env on + // later sessions (Codex); Claude reads its token from the returned handle + // (settings.json) and keeps it out of host disk. Written after spawn so a + // stale token is never left pointing at a dead proxy. + if provider.persists_token() + && let Err(e) = write_token_file(inst, provider, &token) + { + stop_provider(inst, provider); + return Err(e); + } + // Expose the loopback proxy into the guest. If the tunnel fails, tear the + // proxy back down so we don't leave it orphaned (fail closed). + if let Err(e) = spawn_reverse_forward(inst, provider.name(), target, port) { + stop_provider(inst, provider); + return Err(e); + } + tracing::info!( + "Started {} credential proxy on {listen} (guest → 127.0.0.1:{port})", + provider.name() + ); + + Ok(ProxyHandle { + base_url: format!("http://127.0.0.1:{port}"), + capability_token: Secret::new(token), + }) +} + +/// Tear down the proxy process, tunnel, and token file for one `provider`. +/// Best-effort, safe to call when none were started. +pub fn stop_provider(inst: &Instance, provider: Provider) { + let name = provider.name(); + kill_pid_file(&pid_path(inst, name), "proxy"); + kill_pid_file(&fwd_pid_path(inst, name), "proxy tunnel"); + let token = token_path(inst, name); + if token.exists() + && let Err(e) = fs::remove_file(&token) + { + tracing::debug!( + "Failed to remove proxy token file {} (non-fatal): {e}", + token.display() + ); + } +} + +/// Tear down every provider's proxy for `inst` (stop/destroy). Best-effort. +pub fn stop(inst: &Instance) { + for provider in Provider::ALL { + stop_provider(inst, provider); + } +} + +/// The persisted capability token for a running `provider` proxy, if any. +/// Used to forward the Codex provider bearer on interactive sessions after +/// bootstrap has started the proxy. +pub fn read_capability_token(inst: &Instance, provider: Provider) -> Option { + let token = fs::read_to_string(token_path(inst, provider.name())).ok()?; + let token = token.trim(); + (!token.is_empty()).then(|| token.to_string()) +} + +// ── process supervision ────────────────────────────────────── + +fn spawn_proxy(inst: &Instance, name: &str, listen: SocketAddr, json: &str) -> Result<()> { + // A previous run may have crashed without stop cleanup; kill any leftover + // before binding so we don't race it for the same port. + kill_pid_file(&pid_path(inst, name), "stale proxy"); + + let bin = locate_proxy_binary()?; + let log_path = inst.dir.join(format!("proxy-{name}.log")); + let log = File::create(&log_path) + .with_context(|| format!("Failed to create proxy log {}", log_path.display()))?; + + let mut child = confined_command(&bin) + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::from(log)) + // New process group so a Ctrl-C to the coop CLI's group doesn't kill + // the proxy; it must outlive the foreground command. + .process_group(0) + .spawn() + .with_context(|| format!("Failed to spawn {}", bin.display()))?; + + let mut stdin = child + .stdin + .take() + .context("proxy child stdin unexpectedly missing")?; + stdin + .write_all(json.as_bytes()) + .context("Failed to write proxy startup config to stdin")?; + // Drop closes stdin (EOF) so the proxy finishes reading its config. + drop(stdin); + + // Fail closed: confirm the proxy bound its listener before recording it. + // When confinement (Landlock on Linux, Seatbelt on macOS) cannot be + // established the proxy exits before binding, so this probe never connects + // and the launch aborts — a credential proxy that could not be jailed never + // reaches a serving state. Kill the child on failure so it is not orphaned. + if let Err(e) = await_proxy_ready(&mut child, listen, &log_path) { + let _ = child.kill(); + let _ = child.wait(); + return Err(e); + } + + let pid = child.id(); + let pid_path = pid_path(inst, name); + if let Err(e) = fs::write(&pid_path, pid.to_string()) { + // Without a pid file `stop_provider` can never reap this proxy, and the + // std `Child` destructor does not kill it — so a running proxy holding + // the real credential would be orphaned. Kill it before failing. + let _ = child.kill(); + let _ = child.wait(); + return Err(e) + .with_context(|| format!("Failed to write proxy pid file {}", pid_path.display())); + } + // Deliberately do not wait past this point: the proxy runs for the VM's + // lifetime, and the std `Child` destructor does not kill it. + Ok(()) +} + +/// Poll until the proxy accepts a connection on its host-loopback listener, or +/// fail closed. Returns an error (carrying the proxy log) if the proxy exits +/// early or never begins serving within [`PROXY_READY_GRACE`]. +fn await_proxy_ready( + child: &mut std::process::Child, + listen: SocketAddr, + log_path: &Path, +) -> Result<()> { + let deadline = Instant::now() + PROXY_READY_GRACE; + loop { + match child.try_wait() { + Ok(Some(status)) => { + let log = fs::read_to_string(log_path).unwrap_or_default(); + bail!( + "credential proxy exited before it began serving ({status}) — its \ + confinement or bind failed; refusing to start the VM (fail-closed).\n{}", + log.trim() + ); + } + Ok(None) => {} + Err(e) => return Err(e).context("Failed to poll the credential proxy process"), + } + if std::net::TcpStream::connect_timeout(&listen, Duration::from_millis(200)).is_ok() { + return Ok(()); + } + if Instant::now() >= deadline { + let log = fs::read_to_string(log_path).unwrap_or_default(); + bail!( + "credential proxy did not begin serving on {listen} within \ + {PROXY_READY_GRACE:?} — refusing to start the VM (fail-closed).\n{}", + log.trim() + ); + } + std::thread::sleep(Duration::from_millis(100)); + } +} + +/// Build the `Command` that launches `coop-proxy` under the platform's process +/// confinement (issue #411, slice 3). +/// +/// On Linux the proxy self-confines with Landlock unconditionally (it never +/// receives a `--no-jail` opt-out from `coop`), so the command is the binary +/// itself. On macOS — which has no first-class in-process sandbox — the +/// launcher wraps it in `sandbox-exec` with a Seatbelt profile +/// ([`SEATBELT_PROFILE`]) that denies filesystem writes and program execution +/// and limits egress to the two upstream ports. `sandbox-exec` +/// `execve`-replaces itself with the proxy, so the spawned pid is the proxy's +/// and teardown is unchanged. +fn confined_command(bin: &Path) -> Command { + #[cfg(target_os = "macos")] + { + let mut cmd = Command::new("sandbox-exec"); + cmd.arg("-p").arg(SEATBELT_PROFILE).arg(bin); + cmd + } + #[cfg(not(target_os = "macos"))] + { + Command::new(bin) + } +} + +/// Seatbelt profile confining `coop-proxy` on macOS. Kept as a checked-in +/// `.sb` file so it is the single source of truth shared by the launcher +/// (here) and the integration smoke test (`sandbox-exec -f src/seatbelt-proxy.sb +/// coop-proxy --jail-selftest`). Denies filesystem writes, program exec, and +/// all egress except `:443`/`:53`; see the file for the full rationale and the +/// `sandbox-exec`-deprecation trade-off. +#[cfg(target_os = "macos")] +const SEATBELT_PROFILE: &str = include_str!("seatbelt-proxy.sb"); + +/// Establish a per-instance reverse SSH tunnel so the guest reaches the +/// host-loopback proxy at `127.0.0.1:{port}` (`ssh -R guest → host`). Detached +/// and tracked by a PID file like the proxy process, so teardown needs no SSH +/// target. `ExitOnForwardFailure` makes a bind clash on the guest a loud +/// failure rather than a silently dead tunnel. +fn spawn_reverse_forward(inst: &Instance, name: &str, target: &SshTarget, port: u16) -> Result<()> { + kill_pid_file(&fwd_pid_path(inst, name), "stale proxy tunnel"); + + let log_path = inst.dir.join(format!("proxy-{name}-fwd.log")); + let log = File::create(&log_path) + .with_context(|| format!("Failed to create proxy tunnel log {}", log_path.display()))?; + + let mut args = target.ssh_opts(); + args.extend([ + "-N".into(), + "-T".into(), + "-o".into(), + "ExitOnForwardFailure=yes".into(), + "-o".into(), + "ServerAliveInterval=30".into(), + "-R".into(), + format!("127.0.0.1:{port}:127.0.0.1:{port}"), + ]); + args.push(target.addr()); + + let mut child = Command::new("ssh") + .args(&args) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::from(log)) + // New process group so the tunnel outlives the foreground command. + .process_group(0) + .spawn() + .context("Failed to spawn the reverse SSH tunnel for the credential proxy")?; + + // Confirm the tunnel actually came up before returning Ok — otherwise the + // guest boots pointed at a dead endpoint. `ssh` was not given `-f`, so this + // child *is* the tunnel; with `ExitOnForwardFailure=yes` it exits promptly + // when the guest refuses the `-R` bind. If it survives the grace window the + // forward is bound; if it exits first, fail closed with the ssh log. + let deadline = Instant::now() + TUNNEL_READY_GRACE; + while Instant::now() < deadline { + match child.try_wait() { + Ok(Some(status)) => { + let log = fs::read_to_string(&log_path).unwrap_or_default(); + bail!( + "reverse SSH tunnel for the credential proxy exited before it was \ + established ({status}) — the guest may forbid TCP forwarding.\n{}", + log.trim() + ); + } + Ok(None) => std::thread::sleep(Duration::from_millis(100)), + Err(e) => return Err(e).context("Failed to poll the reverse SSH tunnel process"), + } + } + + let pid = child.id(); + let path = fwd_pid_path(inst, name); + if let Err(e) = fs::write(&path, pid.to_string()) { + let _ = child.kill(); + let _ = child.wait(); + return Err(e) + .with_context(|| format!("Failed to write proxy tunnel pid file {}", path.display())); + } + Ok(()) +} + +/// SIGTERM the process named by a PID file and remove the file. Best-effort; +/// safe when the file is absent or malformed. +fn kill_pid_file(path: &Path, label: &str) { + let Ok(contents) = fs::read_to_string(path) else { + return; + }; + match contents.trim().parse::() { + Ok(pid) if pid > 0 => { + // Safety: `kill` with a parsed positive pid and a constant signal. + unsafe { + libc::kill(pid, libc::SIGTERM); + } + tracing::debug!("Sent SIGTERM to {label} (pid {pid})"); + } + _ => tracing::debug!("Ignoring malformed pid file {}", path.display()), + } + if let Err(e) = fs::remove_file(path) { + tracing::debug!( + "Failed to remove pid file {} (non-fatal): {e}", + path.display() + ); + } +} + +/// Persist the capability token owner-only (0600). It is worthless off the +/// host, but kept owner-only for consistency with other per-instance state. +fn write_token_file(inst: &Instance, provider: Provider, token: &str) -> Result<()> { + let path = token_path(inst, provider.name()); + crate::fs_util::atomic_write_with_mode(&path, token, 0o600) + .with_context(|| format!("Failed to write proxy token file {}", path.display())) +} + +fn pid_path(inst: &Instance, name: &str) -> PathBuf { + inst.dir.join(format!("proxy-{name}.pid")) +} + +fn fwd_pid_path(inst: &Instance, name: &str) -> PathBuf { + inst.dir.join(format!("proxy-{name}-fwd.pid")) +} + +fn token_path(inst: &Instance, name: &str) -> PathBuf { + inst.dir.join(format!("proxy-{name}.token")) +} + +fn locate_proxy_binary() -> Result { + let exe = std::env::current_exe().context("Failed to locate the coop executable")?; + let dir = exe + .parent() + .context("coop executable has no parent directory")?; + let candidate = dir.join(PROXY_BIN_NAME); + if candidate.exists() { + return Ok(candidate); + } + bail!( + "{PROXY_BIN_NAME} not found next to coop at {} — reinstall coop; \ + the proxy ships in the same tarball as coop", + candidate.display() + ); +} + +// ── pure helpers ───────────────────────────────────────────── + +/// Mint a 256-bit capability token from the OS CSPRNG, hex-encoded. Worthless +/// off the host, so exfiltration by a compromised guest gains nothing. +fn mint_capability_token() -> Result { + let mut buf = [0u8; 32]; + let mut urandom = File::open("/dev/urandom").context("Failed to open /dev/urandom")?; + urandom + .read_exact(&mut buf) + .context("Failed to read from /dev/urandom")?; + Ok(hex::encode(buf)) +} + +/// The stdin startup blob for `coop-proxy`, matching its `ProxyConfig` shape. +/// Kept as a private mirror rather than a shared crate so the `coop` binary's +/// dependency closure stays free of the proxy's async/HTTP/TLS stack. +#[derive(Serialize)] +struct WireConfig<'a> { + listen: String, + capability_token: &'a str, + upstream_host: &'a str, + injection: WireInjection<'a>, +} + +#[derive(Serialize)] +#[serde(tag = "scheme", rename_all = "snake_case")] +enum WireInjection<'a> { + XApiKey { credential: &'a str }, + Bearer { credential: &'a str }, +} + +fn wire_config_json( + listen: &SocketAddr, + capability_token: &str, + upstream_host: &str, + auth: ProxyAuthScheme, + credential: &str, +) -> Result { + let injection = match auth { + ProxyAuthScheme::ApiKey => WireInjection::XApiKey { credential }, + ProxyAuthScheme::Bearer => WireInjection::Bearer { credential }, + }; + let wire = WireConfig { + listen: listen.to_string(), + capability_token, + upstream_host, + injection, + }; + serde_json::to_string(&wire).context("Failed to serialize proxy startup config") +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "tests")] +mod tests { + use super::*; + use crate::config::{ImageName, InstanceIndex, InstanceName}; + + fn inst_with_index(index: u16) -> Instance { + Instance { + name: InstanceName::new("t").unwrap(), + index: InstanceIndex::new(index).unwrap(), + dir: PathBuf::from("/tmp/coop-test"), + image: ImageName::new("t.img").unwrap(), + } + } + + #[test] + fn ports_are_per_instance_and_per_provider() { + assert_eq!(Provider::Anthropic.port(&inst_with_index(0)), 8788); + assert_eq!(Provider::Anthropic.port(&inst_with_index(5)), 8793); + assert_eq!(Provider::Openai.port(&inst_with_index(0)), 9788); + assert_eq!(Provider::Openai.port(&inst_with_index(5)), 9793); + } + + #[test] + fn provider_ranges_do_not_overlap() { + // 252 is the max instance index (0..=252); the Anthropic range top + // must stay strictly below the OpenAI base so no (VM, provider) pair + // ever shares a host-loopback port. + assert!(Provider::Anthropic.base_port() + 252 < Provider::Openai.base_port()); + } + + #[test] + fn upstream_hosts_are_pinned() { + assert_eq!(Provider::Anthropic.upstream_host(), "api.anthropic.com"); + assert_eq!(Provider::Openai.upstream_host(), "api.openai.com"); + } + + #[test] + fn only_openai_persists_its_token() { + // Codex forwards the token via env on later sessions, so it must be + // persisted; Claude reads its token from settings.json and keeps it in + // host memory only (unchanged from slice 1). + assert!(Provider::Openai.persists_token()); + assert!(!Provider::Anthropic.persists_token()); + } + + #[test] + fn capability_token_is_64_hex_and_random() { + let a = mint_capability_token().unwrap(); + let b = mint_capability_token().unwrap(); + assert_eq!(a.len(), 64); + assert!(a.chars().all(|c| c.is_ascii_hexdigit())); + assert_ne!(a, b, "tokens must not repeat"); + } + + #[test] + fn wire_json_api_key_shape() { + let listen: SocketAddr = "172.16.0.1:8788".parse().unwrap(); + let json = wire_config_json( + &listen, + "cap-tok", + "api.anthropic.com", + ProxyAuthScheme::ApiKey, + "sk-real", + ) + .unwrap(); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(v["listen"], "172.16.0.1:8788"); + assert_eq!(v["capability_token"], "cap-tok"); + assert_eq!(v["upstream_host"], "api.anthropic.com"); + assert_eq!(v["injection"]["scheme"], "x_api_key"); + assert_eq!(v["injection"]["credential"], "sk-real"); + } + + #[test] + fn wire_json_bearer_shape() { + let listen: SocketAddr = "172.16.0.1:8900".parse().unwrap(); + let json = wire_config_json( + &listen, + "t", + "api.openai.com", + ProxyAuthScheme::Bearer, + "sk-openai", + ) + .unwrap(); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(v["injection"]["scheme"], "bearer"); + assert_eq!(v["injection"]["credential"], "sk-openai"); + } + + #[test] + fn await_proxy_ready_fails_closed_when_proxy_exits_early() { + // A proxy that exits before binding (e.g. the jail could not be + // established) must abort the launch, not proceed. Stand in with a + // child that exits immediately; `await_proxy_ready` must return Err. + let tmp = tempfile::TempDir::new().unwrap(); + let log = tmp.path().join("proxy.log"); + std::fs::write(&log, "boom: jail could not be established\n").unwrap(); + // An address nothing will ever bind, so success can only come from the + // (never-happening) listener, not a stray connect. + let listen: SocketAddr = "127.0.0.1:1".parse().unwrap(); + let mut child = Command::new("sh").args(["-c", "exit 7"]).spawn().unwrap(); + let err = await_proxy_ready(&mut child, listen, &log).unwrap_err(); + let msg = format!("{err:#}"); + assert!( + msg.contains("exited before it began serving"), + "unexpected error: {msg}" + ); + let _ = child.wait(); + } + + #[test] + fn token_file_round_trips_and_clears() { + let tmp = tempfile::TempDir::new().unwrap(); + let mut inst = inst_with_index(0); + inst.dir = tmp.path().to_path_buf(); + + assert_eq!(read_capability_token(&inst, Provider::Openai), None); + write_token_file(&inst, Provider::Openai, "cap-123").unwrap(); + assert_eq!( + read_capability_token(&inst, Provider::Openai), + Some("cap-123".to_string()) + ); + // Other providers are independent. + assert_eq!(read_capability_token(&inst, Provider::Anthropic), None); + + stop_provider(&inst, Provider::Openai); + assert_eq!(read_capability_token(&inst, Provider::Openai), None); + } + + #[test] + fn read_capability_token_ignores_empty_and_whitespace_files() { + let tmp = tempfile::TempDir::new().unwrap(); + let mut inst = inst_with_index(0); + inst.dir = tmp.path().to_path_buf(); + + // Empty file → no token (kills a `!token.is_empty()` → `true` mutant, + // which would otherwise return `Some("")`). + write_token_file(&inst, Provider::Openai, "").unwrap(); + assert_eq!(read_capability_token(&inst, Provider::Openai), None); + + // Whitespace-only file trims to empty and is likewise ignored. + write_token_file(&inst, Provider::Openai, " \n\t ").unwrap(); + assert_eq!(read_capability_token(&inst, Provider::Openai), None); + + // Surrounding whitespace is stripped from a real token (kills a + // `.trim()`-deletion mutant, invisible to the round-trip test). + write_token_file(&inst, Provider::Openai, " cap-123\n").unwrap(); + assert_eq!( + read_capability_token(&inst, Provider::Openai), + Some("cap-123".to_string()) + ); + } +} diff --git a/src/proxy_state.rs b/src/proxy_state.rs new file mode 100644 index 0000000..9b1b548 --- /dev/null +++ b/src/proxy_state.rs @@ -0,0 +1,237 @@ +//! Per-instance credential-proxy overrides (issue #411). +//! +//! The `[proxy.]` blocks in `config.toml` are the **defaults** that +//! apply to every VM. A single VM can override the credential for a provider +//! — for per-project billing, scope, or revocation — without a growing config +//! table: the override is persisted here, one JSON file at +//! `/proxy.json`, mirroring [`crate::model_state`]. +//! +//! Resolution per provider is **override → default → off**: a per-VM override +//! wins over the config default, and if neither is set the proxy is off for +//! that provider. Whichever wins carries a `cmd:` credential reference that is +//! resolved on the host at VM start (never written to the guest); a +//! configured-but-unresolvable credential fails the VM start closed rather +//! than falling back to forwarding a raw key. +use std::fs; +use std::io::ErrorKind; + +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; + +use crate::config::{Instance, ProxyConfig, ProxyUpstream}; +use crate::proxy::Provider; + +/// Persisted per-instance credential overrides. Absent fields (the common +/// case) fall back to the `[proxy.]` config defaults. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +pub struct ProxyState { + /// Overrides the `[proxy.anthropic]` default for this VM. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub anthropic: Option, + + /// Overrides the `[proxy.openai]` default for this VM. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub openai: Option, +} + +impl ProxyState { + /// The override slot for `provider`, mutable — used by `coop proxy setup + /// --vm` to record a per-VM credential. + pub fn slot_mut(&mut self, provider: Provider) -> &mut Option { + match provider { + Provider::Anthropic => &mut self.anthropic, + Provider::Openai => &mut self.openai, + } + } + + /// The per-VM override for `provider`, if any. + fn override_for(&self, provider: Provider) -> Option<&ProxyUpstream> { + match provider { + Provider::Anthropic => self.anthropic.as_ref(), + Provider::Openai => self.openai.as_ref(), + } + } + + /// The effective upstream for `provider`: the per-VM override if set, + /// otherwise the config default, otherwise `None` (proxy off). + pub fn effective<'a>( + &'a self, + provider: Provider, + cfg: &'a ProxyConfig, + ) -> Option<&'a ProxyUpstream> { + self.override_for(provider).or(match provider { + Provider::Anthropic => cfg.anthropic.as_ref(), + Provider::Openai => cfg.openai.as_ref(), + }) + } + + /// `true` when nothing needs persisting — equivalent to "no `proxy.json` + /// on disk." + fn is_default(&self) -> bool { + self.anthropic.is_none() && self.openai.is_none() + } + + pub fn save(&self, inst: &Instance) -> Result<()> { + let path = inst.proxy_state_path(); + if self.is_default() { + // A missing file already encodes "no override"; don't leave a + // stale snapshot behind. + if path.exists() + && let Err(e) = fs::remove_file(&path) + { + tracing::debug!( + "Failed to remove default proxy state {} (non-fatal): {e}", + path.display() + ); + } + return Ok(()); + } + let json = serde_json::to_string_pretty(self).context("Failed to serialize proxy state")?; + // 0o600: an override may carry a literal (BYO) credential, and even a + // `cmd:` reference is per-VM state we keep owner-only for consistency + // with model.json. + crate::fs_util::atomic_write_with_mode(&path, &json, 0o600) + .context("Failed to write proxy.json")?; + tracing::debug!("Wrote proxy state to {}", path.display()); + Ok(()) + } + + pub fn try_load(inst: &Instance) -> Result> { + let path = inst.proxy_state_path(); + match fs::read_to_string(&path) { + Ok(content) => { + let state = serde_json::from_str(&content).context("Failed to parse proxy.json")?; + Ok(Some(state)) + } + Err(e) if e.kind() == ErrorKind::NotFound => Ok(None), + Err(e) => { + Err(anyhow::Error::new(e).context(format!("Failed to read {}", path.display()))) + } + } + } + + /// Load the saved overrides, or the empty default when none exist. + pub fn load_or_default(inst: &Instance) -> Result { + Ok(Self::try_load(inst)?.unwrap_or_default()) + } +} + +/// The effective upstream for `provider` on `inst`: per-VM override → config +/// default → `None`. A convenience over [`ProxyState::effective`] that loads +/// the persisted state. +pub fn effective_upstream( + inst: &Instance, + provider: Provider, + cfg: &ProxyConfig, +) -> Result> { + let state = ProxyState::load_or_default(inst)?; + Ok(state.effective(provider, cfg).cloned()) +} + +#[cfg(test)] +#[expect(clippy::unwrap_used, reason = "tests")] +mod tests { + use super::*; + use crate::config::{ImageName, InstanceIndex, InstanceName, ProxyAuthScheme, Secret}; + use std::path::PathBuf; + + fn inst(dir: PathBuf) -> Instance { + Instance { + name: InstanceName::new("t").unwrap(), + index: InstanceIndex::new(0).unwrap(), + dir, + image: ImageName::new("t.img").unwrap(), + } + } + + fn upstream(cred: &str, auth: ProxyAuthScheme) -> ProxyUpstream { + ProxyUpstream { + credential: Secret::new(cred.to_string()), + auth, + } + } + + #[test] + fn override_wins_over_default() { + let cfg = ProxyConfig { + anthropic: Some(upstream("cmd:default", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + let state = ProxyState { + anthropic: Some(upstream("cmd:override", ProxyAuthScheme::ApiKey)), + ..Default::default() + }; + + let eff = state.effective(Provider::Anthropic, &cfg).unwrap(); + assert_eq!(eff.credential.expose(), "cmd:override"); + assert_eq!(eff.auth, ProxyAuthScheme::ApiKey); + } + + #[test] + fn falls_back_to_default_then_off() { + let cfg = ProxyConfig { + openai: Some(upstream("cmd:default-oai", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + let state = ProxyState::default(); + + let openai = state.effective(Provider::Openai, &cfg).unwrap(); + assert_eq!(openai.credential.expose(), "cmd:default-oai"); + // No default and no override → off. + assert!(state.effective(Provider::Anthropic, &cfg).is_none()); + } + + #[test] + fn save_load_round_trip_and_default_removes_file() { + let tmp = tempfile::TempDir::new().unwrap(); + let inst = inst(tmp.path().to_path_buf()); + + let state = ProxyState { + openai: Some(upstream("cmd:x", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + state.save(&inst).unwrap(); + assert!(inst.proxy_state_path().exists()); + + let loaded = ProxyState::load_or_default(&inst).unwrap(); + assert_eq!(loaded.openai.unwrap().credential.expose(), "cmd:x"); + + // Clearing every override removes the file. + ProxyState::default().save(&inst).unwrap(); + assert!(!inst.proxy_state_path().exists()); + } + + #[test] + fn load_or_default_is_empty_when_file_missing() { + let tmp = tempfile::TempDir::new().unwrap(); + let inst = inst(tmp.path().to_path_buf()); + assert!(!inst.proxy_state_path().exists()); + + let loaded = ProxyState::load_or_default(&inst).unwrap(); + assert!(loaded.anthropic.is_none() && loaded.openai.is_none()); + // The missing-file → None path also drives effective resolution off. + let cfg = ProxyConfig::default(); + assert!( + effective_upstream(&inst, Provider::Openai, &cfg) + .unwrap() + .is_none() + ); + } + + #[test] + fn effective_upstream_loads_from_disk() { + let tmp = tempfile::TempDir::new().unwrap(); + let inst = inst(tmp.path().to_path_buf()); + let state = ProxyState { + anthropic: Some(upstream("cmd:vm", ProxyAuthScheme::Bearer)), + ..Default::default() + }; + state.save(&inst).unwrap(); + + let cfg = ProxyConfig::default(); + let eff = effective_upstream(&inst, Provider::Anthropic, &cfg) + .unwrap() + .unwrap(); + assert_eq!(eff.credential.expose(), "cmd:vm"); + } +} diff --git a/src/seatbelt-proxy.sb b/src/seatbelt-proxy.sb new file mode 100644 index 0000000..544df33 --- /dev/null +++ b/src/seatbelt-proxy.sb @@ -0,0 +1,50 @@ +;; Seatbelt (SBPL) profile confining the host-side coop-proxy on macOS +;; (issue #411, slice 3). Applied by the launcher via `sandbox-exec -p` +;; (see src/proxy.rs) and asserted by the integration suite via +;; `sandbox-exec -f src/seatbelt-proxy.sb coop-proxy --jail-selftest`. +;; +;; Deny everything by default, then allow only what the proxy legitimately +;; needs: read files (the dynamic linker, resolver config), resolve names, +;; bind its loopback listener, accept the reverse-tunnel connection, and +;; connect out only to :443 (upstream HTTPS) and :53 (DNS). No filesystem +;; writes, no exec, no other egress. The sandbox is inherited by children and +;; cannot be relaxed from inside. +;; +;; `sandbox-exec`/SBPL is officially deprecated but still functional and widely +;; relied on; Apple has shipped no CLI successor. This is the pragmatic macOS +;; analog of the Linux Landlock jail. See docs/trust-model.md. +(version 1) +(deny default) + +;; Reads stay open (dyld, dylib cache, /etc/resolv.conf, /etc/hosts). Writes +;; are denied by the default deny; the proxy only writes to its inherited +;; stderr log, which is an already-open descriptor and is unaffected. +(allow file-read*) + +;; Baseline a networked binary needs to start under (deny default). +(allow sysctl-read) +(allow system-socket) +(allow process-info* (target self)) +(allow mach-lookup + (global-name "com.apple.mDNSResponder") + (global-name "com.apple.system.opendirectoryd.libinfo") + (global-name "com.apple.system.logger")) + +;; Bind only the loopback listener and accept the host's reverse-tunnel +;; connection to it. +(allow network-bind (local ip "localhost:*")) +(allow network-inbound (local ip "localhost:*")) + +;; Egress allowlist: connect out only to the upstream HTTPS port and DNS. +;; This is port-scoped, not host-scoped — the two upstreams' identity is +;; enforced at the TLS layer in the proxy, and the guest cannot retarget them. +(allow network-outbound (remote tcp "*:443")) +(allow network-outbound (remote tcp "*:53")) +(allow network-outbound (remote udp "*:53")) +;; Local UNIX sockets used during name resolution. This is the broadest allow +;; in the profile (any local UNIX socket, not a specific path). macOS resolves +;; names via mach (allowed above), so this may be unnecessary or scopeable to a +;; specific path (e.g. path-literal "/var/run/mDNSResponder") — a candidate to +;; tighten once validated on a Mac. Kept for now so name resolution cannot +;; break; no weaker than Linux, where Landlock does not gate UNIX sockets. +(allow network-outbound (remote unix-socket)) diff --git a/src/secret_store.rs b/src/secret_store.rs index eea3c6e..d53cba0 100644 --- a/src/secret_store.rs +++ b/src/secret_store.rs @@ -48,7 +48,7 @@ pub enum Backend { LinuxSecretService, /// 1Password CLI (`op`) — used when explicitly chosen by the user. OnePassword, - /// Plain file under `~/.coop/state/github-pat/.txt`, mode 0600. + /// Plain file under `~/.coop/state//.txt`, mode 0600. File, } @@ -61,7 +61,7 @@ impl Backend { #[cfg(target_os = "linux")] Self::LinuxSecretService => "Linux Secret Service (GNOME Keyring / KWallet)", Self::OnePassword => "1Password CLI", - Self::File => "Plain file (~/.coop/state/github-pat/.txt, mode 0600)", + Self::File => "Plain file (~/.coop/state//.txt, mode 0600)", } } @@ -272,7 +272,8 @@ pub fn delete_secret( Ok(()) } Backend::File => { - let path = file_backend_path(state_dir, account); + let dir = state_dir.join(secret_subdir(service)); + let path = file_backend_path(&dir, account); if path.exists() { fs::remove_file(&path) .with_context(|| format!("Failed to remove {}", path.display()))?; @@ -304,11 +305,11 @@ pub enum CmdToken { SecretService { service: String, account: String }, /// 1Password item read via `op item get`. OnePassword { title: String }, - /// Plain-file read via `cat`. Modelled by its varying parts — the state - /// directory and the [`AccountName`] — rather than a free `PathBuf`, so - /// the canonical `/github-pat/.txt` layout holds by - /// construction and every representable value round-trips through - /// [`Display`](fmt::Display) / [`parse`](Self::parse). + /// Plain-file read via `cat`. `dir` is the per-service secret directory + /// (see [`secret_subdir`]) and `account` names the `.txt` file + /// inside it, so the `/.txt` layout holds by construction + /// and every value round-trips through [`Display`](fmt::Display) / + /// [`parse`](Self::parse). File { dir: PathBuf, account: AccountName }, } @@ -337,7 +338,7 @@ impl CmdToken { /// /// The match is the exact inverse of [`Display`](fmt::Display): a command /// that does not follow the canonical shape coop writes (including a - /// `cat` path not laid out as `…/github-pat/.txt`) falls through to + /// `cat` path not of the form `…/.txt`) falls through to /// `None` rather than being misattributed to a backend. pub fn parse(cmd: &str) -> Option { let body = cmd.strip_prefix("cmd:").map_or(cmd, str::trim_start); @@ -385,7 +386,7 @@ impl CmdToken { title: (*title).to_string(), }), ["cat", path] => { - parse_pat_file(Path::new(path)).map(|(dir, account)| Self::File { dir, account }) + parse_file(Path::new(path)).map(|(dir, account)| Self::File { dir, account }) } _ => None, } @@ -423,21 +424,17 @@ impl fmt::Display for CmdToken { } /// Decompose a `cat` path into the parts of a [`CmdToken::File`], or `None` -/// if it does not follow the canonical `/github-pat/.txt` -/// layout coop writes for the file backend. +/// if it does not follow the `/.txt` layout coop writes for the +/// file backend. /// -/// This is the exact inverse of joining those parts in -/// [`file_backend_path`]: the `.txt` suffix and `github-pat` parent are -/// stripped, and the filename stem must be a well-formed [`AccountName`]. -/// A path that fails any check is reported as `None` rather than being -/// misattributed to the file backend. -fn parse_pat_file(path: &Path) -> Option<(PathBuf, AccountName)> { +/// The exact inverse of [`file_backend_path`]: the `.txt` suffix is stripped +/// and the filename stem must be a well-formed [`AccountName`]; `dir` is the +/// file's parent (the per-service secret directory). A path that fails any +/// check is reported as `None` rather than being misattributed to the file +/// backend. +fn parse_file(path: &Path) -> Option<(PathBuf, AccountName)> { let stem = path.file_name()?.to_str()?.strip_suffix(".txt")?; - let parent = path.parent()?; - if parent.file_name()?.to_str()? != PAT_DIR { - return None; - } - let dir = parent.parent()?.to_path_buf(); + let dir = path.parent()?.to_path_buf(); let account = AccountName::new(stem).ok()?; Some((dir, account)) } @@ -570,10 +567,10 @@ fn store_file( token: &str, state_dir: &Path, ) -> Result { - let dir = state_dir.join(PAT_DIR); + let dir = state_dir.join(secret_subdir(service)); fs::create_dir_all(&dir).with_context(|| format!("Failed to create {}", dir.display()))?; set_dir_mode(&dir, 0o700)?; - let path = file_backend_path(state_dir, account); + let path = file_backend_path(&dir, account); let mut f = OpenOptions::new() .create(true) .write(true) @@ -588,16 +585,25 @@ fn store_file( f.write_all(b"\n") .with_context(|| format!("Failed to write {}", path.display()))?; } - // Suppress unused-warning for `service` when the file backend ignores it. - let _ = service; Ok(CmdToken::File { - dir: state_dir.to_path_buf(), + dir, account: account.clone(), }) } -fn file_backend_path(state_dir: &Path, account: &AccountName) -> PathBuf { - state_dir.join(PAT_DIR).join(format!("{account}.txt")) +/// The per-service secret directory under the state dir for the file +/// backend, derived by stripping coop's `coop-` service prefix — so +/// `coop-github-pat` → `github-pat` (unchanged) and `coop-anthropic` → +/// `anthropic`. Each secret kind gets its own namespace, so a proxy secret +/// never lands among the GitHub PATs. +fn secret_subdir(service: &str) -> &str { + service.strip_prefix("coop-").unwrap_or(service) +} + +/// The file holding the secret: `/.txt`, where `dir` is the +/// per-service directory from [`secret_subdir`]. +fn file_backend_path(dir: &Path, account: &AccountName) -> PathBuf { + dir.join(format!("{account}.txt")) } fn set_dir_mode(path: &Path, mode: u32) -> Result<()> { @@ -622,11 +628,20 @@ fn shell_quote(s: &str) -> String { format!("'{escaped}'") } -/// Conventional service name used across all backends. +/// Service name for the GitHub PAT secret across all backends. The file +/// backend derives its subdirectory from this via [`secret_subdir`] +/// (`github-pat`). pub const SERVICE: &str = "coop-github-pat"; -/// Directory under `/state/` holding file-backend PAT secrets. -const PAT_DIR: &str = "github-pat"; +/// Service name for the Anthropic proxy credential (issue #411); its file +/// backend subdirectory is `anthropic` (see [`secret_subdir`]). +pub const ANTHROPIC_SERVICE: &str = "coop-anthropic"; + +/// Service name for the `OpenAI` (Codex) proxy credential (issue #411); its +/// file backend subdirectory is `openai` (see [`secret_subdir`]). A per-VM +/// override appends `-` (e.g. `coop-openai-dev`) so it never collides with +/// the global default. +pub const OPENAI_SERVICE: &str = "coop-openai"; #[cfg(test)] #[expect(clippy::unwrap_used, reason = "tests")] @@ -658,7 +673,7 @@ mod tests { #[test] fn account_name_new_rejects_empty_and_slashes() { - // `new` is the parse-side smart constructor `parse_pat_file` relies + // `new` is the parse-side smart constructor `parse_file` relies // on: a `/` (single, repeated, or leading) in the filename stem would // make the path decomposition ambiguous, so it must be rejected here. // This pins what keeps the `File` round-trip unambiguous, independent @@ -745,7 +760,7 @@ mod tests { assert!(cmd.to_string().starts_with("cmd:cat ")); assert_eq!(cmd.backend(), Backend::File); // Verify the file exists with the right contents and mode. - let path = file_backend_path(tmp.path(), &acc); + let path = file_backend_path(&tmp.path().join(secret_subdir(SERVICE)), &acc); let content = std::fs::read_to_string(&path).unwrap(); assert_eq!(content.trim(), "github_pat_xyz"); let meta = std::fs::metadata(&path).unwrap(); @@ -758,7 +773,7 @@ mod tests { let acc = account("trailofbits/coop"); // A token without a trailing newline gets exactly one appended. store_file(SERVICE, &acc, "tok", tmp.path()).unwrap(); - let path = file_backend_path(tmp.path(), &acc); + let path = file_backend_path(&tmp.path().join(secret_subdir(SERVICE)), &acc); assert_eq!(std::fs::read(&path).unwrap(), b"tok\n"); } @@ -768,7 +783,7 @@ mod tests { let acc = account("trailofbits/coop"); // A token already newline-terminated is stored verbatim, not doubled. store_file(SERVICE, &acc, "tok\n", tmp.path()).unwrap(); - let path = file_backend_path(tmp.path(), &acc); + let path = file_backend_path(&tmp.path().join(secret_subdir(SERVICE)), &acc); assert_eq!(std::fs::read(&path).unwrap(), b"tok\n"); } @@ -777,7 +792,7 @@ mod tests { let tmp = tempfile::TempDir::new().unwrap(); let acc = account("trailofbits/coop"); store_file(SERVICE, &acc, "tok", tmp.path()).unwrap(); - let dir = tmp.path().join(PAT_DIR); + let dir = tmp.path().join(secret_subdir(SERVICE)); let meta = std::fs::metadata(&dir).unwrap(); assert_eq!(meta.permissions().mode() & 0o777, 0o700); } @@ -788,7 +803,7 @@ mod tests { let acc = account("x/y"); let _ = store_file(SERVICE, &acc, "token", tmp.path()).unwrap(); delete_secret(Backend::File, SERVICE, &acc, tmp.path()).unwrap(); - let path = file_backend_path(tmp.path(), &acc); + let path = file_backend_path(&tmp.path().join(secret_subdir(SERVICE)), &acc); assert!(!path.exists()); } @@ -846,15 +861,20 @@ mod tests { } #[test] - fn parse_rejects_cat_without_canonical_layout() { - // A `cat` invocation that doesn't follow the `…/github-pat/.txt` - // layout should not be reported as the File backend — otherwise - // `forget-pat` would happily try to delete a file coop never wrote. + fn parse_recognises_any_cat_txt_file() { + // The file layout is now per-service (`/.txt`), so any + // `cat /.txt` with a valid account stem is the File backend + // — deletion recomputes the path from the state dir + service, so this + // superset never causes coop to delete a file it did not write. assert_eq!( - backend_of("cmd:cat ~/some/path/secret.txt"), - None, - "cat invocation without github-pat in the path must not match File" + backend_of("cmd:cat ~/.coop/state/github-pat/x.txt"), + Some(Backend::File) + ); + assert_eq!( + backend_of("cmd:cat ~/.coop/state/anthropic/anthropic.txt"), + Some(Backend::File) ); + // A non-`.txt` suffix is still not a form coop writes. assert_eq!( backend_of("cmd:cat ~/.coop/state/github-pat/x.bin"), None, @@ -913,7 +933,7 @@ mod tests { /// collide with the canonical layout (`github-pat`, a `*.txt` stem) — /// with free text (spaces, quotes, unicode, the empty path) so the /// round-trip is exercised on the inputs most likely to confuse the - /// `parse_pat_file` decomposition, not just random bytes. The NUL byte is + /// `parse_file` decomposition, not just random bytes. The NUL byte is /// excluded; no real filesystem path carries it. fn arb_dir() -> impl Strategy { prop_oneof![ diff --git a/src/update.rs b/src/update.rs index ac2270d..385e5af 100644 --- a/src/update.rs +++ b/src/update.rs @@ -436,14 +436,20 @@ fn check_parent_writable(dir: &Path) -> Result<()> { } } -fn atomic_replace_self(new_binary: &Path) -> Result<()> { - let current = env::current_exe().context("Failed to resolve current executable path")?; - let dir = current +/// Atomically swap `new_binary` over `target`: stage a copy in the target's +/// directory, chmod + fsync it, then `rename` it into place (atomic on the +/// same filesystem, safe over a running binary on Unix). +fn atomic_replace(new_binary: &Path, target: &Path) -> Result<()> { + let dir = target .parent() - .context("Current executable has no parent directory")?; + .context("Target executable has no parent directory")?; check_parent_writable(dir)?; - let tmp = dir.join(format!(".coop-update-{}", std::process::id())); + let file_name = target + .file_name() + .and_then(|n| n.to_str()) + .context("Target executable has no file name")?; + let tmp = dir.join(format!(".{file_name}-update-{}", std::process::id())); fs::copy(new_binary, &tmp) .with_context(|| format!("Failed to stage update at {}", tmp.display()))?; fs::set_permissions(&tmp, fs::Permissions::from_mode(0o755)) @@ -454,16 +460,39 @@ fn atomic_replace_self(new_binary: &Path) -> Result<()> { .sync_all() .with_context(|| format!("Failed to fsync staged binary {}", tmp.display()))?; - fs::rename(&tmp, ¤t).with_context(|| { - format!( - "Failed to swap {} over {}", - tmp.display(), - current.display() - ) - })?; + fs::rename(&tmp, target) + .with_context(|| format!("Failed to swap {} over {}", tmp.display(), target.display()))?; Ok(()) } +fn atomic_replace_self(new_binary: &Path) -> Result<()> { + let current = env::current_exe().context("Failed to resolve current executable path")?; + atomic_replace(new_binary, ¤t) +} + +/// Replace the sibling `coop-proxy` (issue #411) from the same verified +/// tarball so it never drifts from `coop`. Fails closed: if the tarball +/// carries a proxy but the sibling cannot be written, the update aborts +/// before `coop` itself is swapped. A no-op for older releases that predate +/// the bundled proxy. +fn replace_sibling_proxy(extract_dir: &Path) -> Result<()> { + let current = env::current_exe().context("Failed to resolve current executable path")?; + replace_sibling_proxy_at(extract_dir, ¤t) +} + +/// Core of [`replace_sibling_proxy`] with the running-binary path injected so +/// the swap destination is testable without touching the real `coop` binary. +fn replace_sibling_proxy_at(extract_dir: &Path, current_exe: &Path) -> Result<()> { + let new_proxy = extract_dir.join("coop-proxy"); + if !new_proxy.exists() { + return Ok(()); + } + let dir = current_exe + .parent() + .context("Current executable has no parent directory")?; + atomic_replace(&new_proxy, &dir.join("coop-proxy")) +} + // ── Main update flow ───────────────────────────────────────────────────────── pub fn run(opts: &UpdateOpts) -> Result<()> { @@ -562,16 +591,18 @@ fn perform_update(release: &Release, triple: &str) -> Result<()> { .run() .context("Failed to extract release tarball")?; - let extracted = tmp - .path() - .join(format!("coop-{}-{triple}", release.tag)) - .join("coop"); + let extract_dir = tmp.path().join(format!("coop-{}-{triple}", release.tag)); + let extracted = extract_dir.join("coop"); ensure!( extracted.exists(), "Extracted binary not found at {}", extracted.display() ); + // Swap the sibling proxy first (from the same verified tarball) so a + // proxy-write failure aborts before coop itself is replaced, keeping the + // two in lockstep. + replace_sibling_proxy(&extract_dir)?; atomic_replace_self(&extracted) } @@ -1012,4 +1043,44 @@ mod tests { assert_eq!(release.assets[0].name, "x.tar.gz"); assert_eq!(release.assets[0].url, "https://example.com/x.tar.gz"); } + + #[test] + fn replace_sibling_proxy_is_noop_for_release_without_proxy() { + // Old release: the tarball carries no coop-proxy, so the swap returns + // before ever resolving the running binary. + let extract = tempfile::tempdir().unwrap(); + replace_sibling_proxy(extract.path()).unwrap(); + } + + #[test] + fn replace_sibling_proxy_swaps_sibling_next_to_coop() { + let extract = tempfile::tempdir().unwrap(); + fs::write(extract.path().join("coop-proxy"), b"new-proxy").unwrap(); + + let install = tempfile::tempdir().unwrap(); + let coop = install.path().join("coop"); + fs::write(&coop, b"coop-binary").unwrap(); + + replace_sibling_proxy_at(extract.path(), &coop).unwrap(); + + let sibling = install.path().join("coop-proxy"); + assert_eq!(fs::read(&sibling).unwrap(), b"new-proxy"); + assert_eq!( + fs::metadata(&sibling).unwrap().permissions().mode() & 0o777, + 0o755 + ); + } + + #[test] + fn replace_sibling_proxy_fails_closed_when_sibling_unwritable() { + // The tarball carries a proxy but the sibling cannot be written: the + // swap must return Err so perform_update never reaches + // atomic_replace_self and coop is left untouched. + let extract = tempfile::tempdir().unwrap(); + fs::write(extract.path().join("coop-proxy"), b"new-proxy").unwrap(); + + // A running-binary path whose parent directory does not exist. + let missing = extract.path().join("no-such-dir").join("coop"); + replace_sibling_proxy_at(extract.path(), &missing).unwrap_err(); + } } diff --git a/tests/integration.sh b/tests/integration.sh index f84e246..9903f7b 100755 --- a/tests/integration.sh +++ b/tests/integration.sh @@ -4085,6 +4085,236 @@ CFGEOF rm -r "$lm_dir" } +# ── Credential-injecting proxy test (issue #411, --full only) ───────── +# +# Brings a VM up in proxy mode (remote model mode + `[proxy.*]` configured) and +# asserts the non-exposure property end-to-end on both backends. There is no +# mock upstream: the proxy pins TLS to the real hosts (a security property), so +# a local mock cannot present a trusted cert. Instead the checks are hermetic +# and need no upstream — the injection-into-the-header logic is covered by the +# `coop-proxy` unit tests + `gate.rs`. Here we assert what only a live VM can: +# - the guest is wired at the proxy (Codex `config.toml`, Claude +# `settings.json`) and holds only the capability token; +# - the raw keys are suppressed even when set on the host (crown jewel), and +# Codex's `auth.json` is not staged; +# - the proxy is reachable on host loopback and enforces the capability gate; +# - `stop` tears the proxy down (the host port stops answering). +test_proxy() { + echo "" + echo "=== Phase: credential-injecting proxy (--full) ===" + + if ! command -v curl >/dev/null 2>&1; then + skip "credential-proxy test (curl not available on host)" + return + fi + # The proxy needs the `coop-proxy` binary next to `coop`. It is not a + # default workspace member (it needs cmake for aws-lc-rs), so a plain + # `cargo build` / older deploy may not have it; skip rather than fail the + # fail-closed `up` when it's absent. + if [[ ! -x "$(dirname "$BINARY")/coop-proxy" ]]; then + skip "credential-proxy test (coop-proxy not built alongside coop)" + return + fi + + local inst_name="${INSTANCE}-proxy" + local px_dir + px_dir=$(mktemp -d) + local cfg_file="$px_dir/config.toml" + # Literal fake credentials: `resolve_cmd_value` returns a non-`cmd:` value + # verbatim, so the proxy injects these. The test asserts they never reach + # the guest — bootstrap never runs the agents, so a fake key is harmless. + local openai_secret="sk-coop-it-openai-FAKE-do-not-use" + local anthropic_secret="sk-ant-coop-it-FAKE-do-not-use" + + cat >"$cfg_file" <"$tmpdir/stderr") || rc=$? + HARNESS_ERR=$(cat "$tmpdir/stderr") + return $rc + } + px_exec() { + RUST_LOG=off env -u GITHUB_TOKEN \ + OPENAI_API_KEY="$host_openai" ANTHROPIC_API_KEY="$host_anthropic" \ + "$BINARY" --config "$cfg_file" exec "$inst_name" -- "$@" \ + 2>"$tmpdir/guest_stderr" + } + + local px_ws="$tmpdir/${inst_name}-ws" + mkdir -p "$px_ws" + # Proxy mode is wired during AGENT bootstrap, so this must NOT be --no-agents. + if px up "$px_ws" --name "$inst_name" --no-devcontainer; then + STARTED_INSTANCES+=("$inst_name") + pass "up in proxy mode exits 0" + else + fail "up in proxy mode exits 0" "exit: $? stderr: $HARNESS_ERR" + rm -r "$px_dir" + return + fi + + # ── Codex (OpenAI) guest wiring ── + local codex_cfg + if codex_cfg=$(px_exec cat ./.codex/config.toml); then + if echo "$codex_cfg" | grep -q "coop_local" \ + && echo "$codex_cfg" | grep -q "responses" \ + && echo "$codex_cfg" | grep -Eq "127\.0\.0\.1:[0-9]+/v1" \ + && echo "$codex_cfg" | grep -q "COOP_LOCAL_API_KEY"; then + pass "codex config.toml points at the proxy (coop_local, responses, /v1)" + else + fail "codex config.toml points at the proxy" "got: $codex_cfg" + fi + if echo "$codex_cfg" | grep -q "$openai_secret"; then + fail "openai credential absent from codex config" "leaked into config.toml" + else + pass "openai credential absent from codex config" + fi + else + fail "codex config.toml readable" "cat failed; stderr: $(guest_stderr)" + fi + + # auth.json must not be staged in proxy mode (issue #411 §7). + if px_exec test -f ./.codex/auth.json; then + fail "codex auth.json not staged in proxy mode" "auth.json present in guest" + else + pass "codex auth.json not staged in proxy mode" + fi + + # ── Crown jewel: raw keys never enter the guest env ── + local guest_env + guest_env=$(px_exec env || true) + if echo "$guest_env" | grep -q "^COOP_LOCAL_API_KEY="; then + pass "guest holds the capability token (COOP_LOCAL_API_KEY)" + else + fail "guest holds the capability token" "COOP_LOCAL_API_KEY not in guest env" + fi + if echo "$guest_env" | grep -q "^OPENAI_API_KEY="; then + fail "OPENAI_API_KEY suppressed in guest (proxy mode)" "it was forwarded" + elif echo "$guest_env" | grep -qF "$openai_secret" \ + || echo "$guest_env" | grep -qF "$host_openai"; then + fail "raw OpenAI key absent from guest env" "a raw key value leaked" + else + pass "raw OpenAI key suppressed and absent from guest env" + fi + if echo "$guest_env" | grep -q "^ANTHROPIC_API_KEY="; then + fail "ANTHROPIC_API_KEY suppressed in guest (proxy mode)" "it was forwarded" + else + pass "ANTHROPIC_API_KEY suppressed in guest" + fi + + # ── Claude (Anthropic) guest wiring ── + local settings + if settings=$(px_exec cat ./.claude/settings.json); then + if echo "$settings" | grep -q "ANTHROPIC_BASE_URL" \ + && echo "$settings" | grep -q "ANTHROPIC_AUTH_TOKEN" \ + && ! echo "$settings" | grep -q "$anthropic_secret"; then + pass "claude settings.json points at the proxy with the capability token" + else + fail "claude settings.json points at the proxy" "got: $settings" + fi + else + fail "claude settings.json readable" "cat failed; stderr: $(guest_stderr)" + fi + + # ── The proxy is reachable on host loopback and enforces the gate ── + # The proxy binds 127.0.0.1: on the host and is reverse-tunnelled into + # the guest, so the host exercises the same listener. A request without the + # token is refused locally ("capability token"); with the token it passes + # the gate and is forwarded upstream (which fails closed at the real host — + # 502 with no egress, or an upstream auth error — but never the local + # refusal), so the presence/absence of the "capability" refusal distinguishes + # the two without needing a working upstream. + local oai_port cap_token + oai_port=$(echo "$codex_cfg" | grep -oE '127\.0\.0\.1:[0-9]+' | head -1 | cut -d: -f2 || true) + cap_token=$(echo "$guest_env" | grep '^COOP_LOCAL_API_KEY=' | cut -d= -f2- || true) + if [[ -n "$oai_port" ]]; then + local no_tok tok + no_tok=$(curl -s --max-time 10 "http://127.0.0.1:$oai_port/v1/models" || true) + tok=$(curl -s --max-time 15 "http://127.0.0.1:$oai_port/v1/models" \ + -H "Authorization: Bearer $cap_token" || true) + if echo "$no_tok" | grep -qi "capability"; then + pass "openai proxy refuses a request missing the capability token" + else + fail "openai proxy refuses a request missing the capability token" "body: $no_tok" + fi + if echo "$tok" | grep -qi "capability"; then + fail "valid capability token passes the gate" "still refused: $tok" + else + pass "valid capability token passes the gate (forwarded upstream)" + fi + else + fail "locate openai proxy port" "no 127.0.0.1: in codex config" + fi + + # ── The jail confines the host-side proxy (issue #411, slice 3) ── + # Prove the confinement coop applies to `coop-proxy` actually restricts on + # this host: a filesystem write, a program exec, and egress to a + # non-upstream port are all blocked, while the upstream port (:443) stays + # reachable. `coop-proxy --jail-selftest` runs the identical probe under + # whichever mechanism confined it — Linux self-applies Landlock; macOS + # applies the same Seatbelt profile coop uses, via `sandbox-exec` — and + # exits 0 with "=> PASS" only if all four hold. This is the host-side + # analog of the guest-side non-exposure checks above. + local proxy_bin selftest_out selftest_rc=0 + proxy_bin="$(dirname "$BINARY")/coop-proxy" + if [[ "$(uname -s)" == "Darwin" ]]; then + local sb_profile + sb_profile="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/src/seatbelt-proxy.sb" + if [[ -f "$sb_profile" ]]; then + # Pass the profile inline with -p (as coop's launcher does), not -f, + # so the smoke test exercises the same code path production uses. + selftest_out=$(sandbox-exec -p "$(cat "$sb_profile")" "$proxy_bin" --jail-selftest 2>&1) \ + || selftest_rc=$? + if [[ $selftest_rc -eq 0 ]] && echo "$selftest_out" | grep -q "=> PASS"; then + pass "proxy jail confines coop-proxy (Seatbelt: no fs-write/exec/off-list egress)" + else + fail "proxy jail confines coop-proxy (Seatbelt)" "rc=$selftest_rc out: $selftest_out" + fi + else + skip "proxy jail self-test (Seatbelt profile not found at $sb_profile)" + fi + else + selftest_out=$("$proxy_bin" --jail-selftest 2>&1) || selftest_rc=$? + if [[ $selftest_rc -eq 0 ]] && echo "$selftest_out" | grep -q "=> PASS"; then + pass "proxy jail confines coop-proxy (Landlock: no fs-write/exec/off-list egress)" + else + fail "proxy jail confines coop-proxy (Landlock)" "rc=$selftest_rc out: $selftest_out" + fi + fi + + # ── Teardown tears the proxy down ── + px stop "$inst_name" >/dev/null 2>&1 || true + if [[ -n "$oai_port" ]]; then + if curl -s --max-time 3 -o /dev/null "http://127.0.0.1:$oai_port/v1/models"; then + fail "stop tears the proxy down" "proxy still answering on 127.0.0.1:$oai_port" + else + pass "stop tears the proxy down (host port no longer answers)" + fi + fi + + px destroy "$inst_name" 2>/dev/null || true + untrack_instance "$inst_name" + rm -r "$px_dir" +} + # ── Interrupted setup test (--full only) ────────────────────── test_interrupted_setup() { @@ -4967,6 +5197,10 @@ main() { # coop model local/remote: local-model wiring lands and reverts test_local_model + # credential-injecting proxy (#411): guest wiring, key suppression, + # capability gate, teardown — Anthropic + OpenAI/Codex + test_proxy + # Interrupted setup: SIGKILL mid-build, verify clean recovery test_interrupted_setup diff --git a/tests/run-integration.sh b/tests/run-integration.sh index 947ae39..4967f1d 100755 --- a/tests/run-integration.sh +++ b/tests/run-integration.sh @@ -33,6 +33,14 @@ if [[ -z "$REMOTE_HOST" ]]; then echo "Building coop (release)..." cargo build --release --manifest-path "$PROJECT_DIR/Cargo.toml" + # coop-proxy (issue #411) is a separate workspace member — it needs cmake + # (aws-lc-rs), so it is intentionally not a default member. Build it + # best-effort next to coop so the credential-proxy phase can run; if it + # fails (e.g. cmake missing) that phase skips rather than blocking the suite. + if ! cargo build --release -p coop-proxy --manifest-path "$PROJECT_DIR/Cargo.toml"; then + echo "warning: coop-proxy build failed (cmake missing?) — proxy phase will skip" >&2 + fi + BINARY="$PROJECT_DIR/target/release/coop" exec "$TEST_SCRIPT" --binary "$BINARY" "${FORWARD_ARGS[@]+"${FORWARD_ARGS[@]}"}" fi @@ -50,11 +58,27 @@ case "$REMOTE_OS-$REMOTE_ARCH" in *) echo "Unsupported remote platform: $REMOTE_OS $REMOTE_ARCH" >&2; exit 1 ;; esac -echo "Cross-compiling for $TARGET..." +echo "Cross-compiling coop for $TARGET..." cargo build --release --target "$TARGET" \ --manifest-path "$PROJECT_DIR/Cargo.toml" LOCAL_BINARY="$PROJECT_DIR/target/$TARGET/release/coop" +LOCAL_PROXY="$PROJECT_DIR/target/$TARGET/release/coop-proxy" + +# coop-proxy (issue #411) links aws-lc-rs, which builds C via cmake — a +# cross-arch build to a foreign libc is fragile. Try a local cross-build first +# (works when the host and remote share an arch); otherwise build it natively +# ON the remote, which by definition supports its own arch. Either way the +# binary lands next to coop so `coop` can spawn it. If neither path yields one, +# the proxy phase skips rather than failing the fail-closed `up`. +build_proxy_on_remote=1 +if cargo build --release --target "$TARGET" -p coop-proxy \ + --manifest-path "$PROJECT_DIR/Cargo.toml" && [[ -f "$LOCAL_PROXY" ]]; then + build_proxy_on_remote=0 + echo "Built coop-proxy locally for $TARGET." +else + echo "Local coop-proxy cross-build unavailable — will build it natively on the remote." +fi REMOTE_DIR=$(ssh "$REMOTE_HOST" mktemp -d) trap 'ssh "$REMOTE_HOST" rm -rf "$REMOTE_DIR"' EXIT @@ -62,6 +86,29 @@ trap 'ssh "$REMOTE_HOST" rm -rf "$REMOTE_DIR"' EXIT echo "Copying binary and test script to $REMOTE_HOST:$REMOTE_DIR..." scp -q "$LOCAL_BINARY" "$TEST_SCRIPT" "$REMOTE_HOST:$REMOTE_DIR/" +if [[ "$build_proxy_on_remote" == "0" ]]; then + scp -q "$LOCAL_PROXY" "$REMOTE_HOST:$REMOTE_DIR/" +else + # Native build on the remote from a clean source snapshot of the current + # commit. Best-effort: needs cargo + cmake + a C compiler on the remote; on + # any failure the proxy phase skips (a warning is printed, the suite runs). + echo "Building coop-proxy natively on $REMOTE_HOST..." + proxy_src=$(mktemp "${TMPDIR:-/tmp}/coop-proxy-src.XXXXXX.tar.gz") + git -C "$PROJECT_DIR" archive --format=tar.gz -o "$proxy_src" HEAD + scp -q "$proxy_src" "$REMOTE_HOST:$REMOTE_DIR/coop-src.tar.gz" + rm -f "$proxy_src" + # shellcheck disable=SC2029 # $REMOTE_DIR is a mktemp path; expand client-side + ssh "$REMOTE_HOST" " + set -e + . \"\$HOME/.cargo/env\" 2>/dev/null || true + mkdir -p '$REMOTE_DIR/src' + tar xzf '$REMOTE_DIR/coop-src.tar.gz' -C '$REMOTE_DIR/src' + cd '$REMOTE_DIR/src' + cargo build --release -p coop-proxy + cp target/release/coop-proxy '$REMOTE_DIR/coop-proxy' + " || echo "warning: coop-proxy build on remote failed (needs cargo + cmake + cc) — proxy phase will skip" >&2 +fi + # Build the remote command as an array, then printf %q to safely quote for ssh. # ssh doesn't forward arbitrary env vars, so pass opt-in flags explicitly. REMOTE_CMD=()