Agent-friendly block responses + rip non-MITM proxy path#112
Merged
Conversation
The previous block response was 403 + tiny JSON. Most LLM-agent fetch
code only checks response.ok and discards 4xx bodies, so the agent
saw a network error with no semantic info to reason about. The agent
just bailed instead of considering alternatives.
Now block responses are 200/HTML with:
- <h1>Page blocked by Calciforge security gateway</h1>
- explicit reason
- 'what this means' explanation
- suggested next steps (try other source, ask operator to allowlist)
- X-Calciforge-Blocked: true and X-Calciforge-Reason headers for
structured tooling that wants to branch without parsing HTML
Sweep call sites that previously passed the placeholder string
'Request rejected' to instead pass the actual error context (decode
failure detail, URL-substitution failure, header/body substitution
failure, etc.). The reason header is sanitised to ASCII so non-printable
or multi-byte content can't break HTTP framing.
Tests updated to assert the new 200/X-Calciforge-Blocked contract.
…st start
Plain HTTP is essentially nothing in 2026, and the non-MITM forward-proxy
path could not inspect HTTPS at all (returned 400 to CONNECT). It shipped
as silent broken protection: agents got `success` for HTTPS targets while
the gateway never saw a single byte of the encrypted traffic. MITM mode
already handles HTTPS *and* plain HTTP through one pipeline — keeping the
non-MITM path around was two code paths for one product behavior.
Changes:
- Delete `proxy.rs::intercept`, `proxy_handler`, `health_handler`,
`blocked_response`. Keep `SecurityProxy` state (scanner, audit,
credentials, config) plus the per-request helpers (substitution,
bypass match, body-mode classifier) used by the MITM handler.
- Delete `router.rs` outright. Move `vault_json_response` +
`constant_time_eq` + `VAULT_TOKEN_ENV` into `mitm.rs` (the only
surviving caller).
- Drop `mitm_enabled` from `GatewayConfig`. Update the
self-consistency test to track CA paths only.
- Collapse `main.rs` to "always serve_mitm". Add a CA fallback: if
`SECURITY_PROXY_CA_CERT/_KEY` are unset, load (or generate via
`rcgen`) a persistent self-signed CA at `/var/lib/calciforge/ca.{pem,key}`.
Key file written 0600. Operators using the install-script CA in
`/etc/calciforge/secrets` are unaffected (those env vars take
precedence). Standalone `cargo run -p security-proxy` now works.
- Delete `tests/destination_allowlist.rs`,
`tests/substitution_body_headers.rs`, `tests/vault_route.rs` — all
used the deleted axum harness.
- Add four MITM-mode replacements in `tests/mitm_https.rs`:
destination-allowlist block, ref-in-unsupported-content-type block,
vault 401 (wrong/missing bearer), vault 503 (token env unset).
Side-effect bug fix: while porting the destination-allowlist test to
MITM mode, discovered that `mitm.rs` block responses for substitution
failures echoed the resolver/allowlist error text — which contains the
literal secret name. The original axum `blocked_response` returned a
generic "Request rejected" message and logged details server-side. The
MITM path now matches that pattern (URL/header/body substitution
failures all return "Request rejected" with the detailed reason logged
at warn level).
Test count: 56 → 41 (-15 axum-mode tests, +4 MITM-mode equivalents,
+0 net coverage gap — the MITM tests cover the same scenarios via the
same intercept pipeline).
LOC delta: +575 / -1728 = -1153.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
There was a problem hiding this comment.
Pull request overview
Updates the security-proxy to make policy blocks readable/actionable to LLM agents, removes the non-MITM forward-proxy path (which couldn’t inspect HTTPS), and adds a fallback flow to auto-provision a persistent MITM CA for standalone/dev runs.
Changes:
- Switch blocked responses to an agent-friendly HTTP 200 + HTML page with
X-Calciforge-*structured headers (MITM path). - Remove the legacy axum forward-proxy router and migrate/replace key behavioral tests into MITM-mode coverage.
- Add CA cert/key resolution with auto-generation at
/var/lib/calciforge/ca.{pem,key}when not configured.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/security-proxy/src/mitm.rs | Implements agent-friendly block page + serves /health and /vault/* from MITM entrypoint; inlines vault logic. |
| crates/security-proxy/src/main.rs | Removes mitm_enabled branching; adds CA resolve/auto-generate flow and always starts MITM listener. |
| crates/security-proxy/src/proxy.rs | Deletes plain forward-proxy intercept implementation; keeps shared state + substitution helpers used by MITM. |
| crates/security-proxy/src/config.rs | Removes mitm_enabled; updates defaults/tests around CA-path consistency. |
| crates/security-proxy/src/lib.rs | Removes router module/export previously used by axum mode. |
| crates/security-proxy/src/router.rs | Deleted (legacy axum router + vault handler). |
| crates/security-proxy/tests/mitm_https.rs | Adds MITM-mode tests for destination allowlist + unsupported content-type blocking + vault route behavior. |
| crates/security-proxy/tests/vault_route.rs | Deleted (legacy axum-mode vault route tests). |
| crates/security-proxy/tests/substitution_body_headers.rs | Deleted (legacy axum-mode header/body substitution tests). |
| crates/security-proxy/tests/destination_allowlist.rs | Deleted (legacy axum-mode allowlist tests). |
Comment on lines
+241
to
+255
| std::fs::write(cert_path, cert.pem()) | ||
| .map_err(|e| anyhow::anyhow!("write CA cert {}: {e}", cert_path.display()))?; | ||
| std::fs::write(key_path, key_pair.serialize_pem()) | ||
| .map_err(|e| anyhow::anyhow!("write CA key {}: {e}", key_path.display()))?; | ||
|
|
||
| #[cfg(unix)] | ||
| { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| let mut perms = std::fs::metadata(key_path) | ||
| .map_err(|e| anyhow::anyhow!("stat CA key: {e}"))? | ||
| .permissions(); | ||
| perms.set_mode(0o600); | ||
| std::fs::set_permissions(key_path, perms) | ||
| .map_err(|e| anyhow::anyhow!("chmod 600 CA key: {e}"))?; | ||
| } |
Comment on lines
+530
to
+543
| .header("X-Calciforge-Blocked", "true") | ||
| .header( | ||
| "X-Calciforge-Reason", | ||
| reason | ||
| .chars() | ||
| .map(|c| { | ||
| if c.is_ascii() && c != '\r' && c != '\n' { | ||
| c | ||
| } else { | ||
| ' ' | ||
| } | ||
| }) | ||
| .collect::<String>(), | ||
| ) |
The Docker smoke check 'security-proxy blocks injection' was still asserting the old 403/JSON shape and broke when this PR flipped block responses to 200/HTML. Update the assertions to match the new contract (status 200, X-Calciforge-Blocked: true header, 'Page blocked by Calciforge' marker in body).
This was referenced May 2, 2026
…sation Two fixes from review feedback: 1. CA private key write race. Previously `std::fs::write` then `chmod 600` left a window where (depending on process umask) another local process could read the freshly-created key with the default-permissions metadata. New `write_key_with_restricted_perms` helper opens with `OpenOptions::mode(0o600)` so the file is created with the restricted bits already in place — no window. Non-Unix path falls back to plain write with a TODO for proper Windows ACL handling if/when this ships there. 2. `X-Calciforge-Reason` header sanitisation. Old code only replaced non-ASCII and CR/LF, so ASCII control characters like NUL (0x00), BEL (0x07), ESC (0x1B), and DEL (0x7F) could still reach the header builder. Each of those either causes the builder to reject the header (we lose the signal entirely via the fallback path) or, with CR/LF specifically, opens header injection. New `sanitize_for_header` per RFC 7230 §3.2 keeps only `field-vchar / SP / HTAB` and replaces everything else with space.
Comment on lines
+142
to
+144
| # Block responses are 200/HTML by design (see proxy::blocked_response): | ||
| # the agent-facing fetch succeeds, the body explains the block, and the | ||
| # X-Calciforge-Blocked header is the machine-readable signal. |
Comment on lines
+241
to
+249
| std::fs::write(cert_path, cert.pem()) | ||
| .map_err(|e| anyhow::anyhow!("write CA cert {}: {e}", cert_path.display()))?; | ||
|
|
||
| // Open the key file with mode 0o600 ATOMICALLY at create time so | ||
| // there is no window where the file is readable by group/world. | ||
| // (Previously we wrote then chmod'd; depending on process umask, | ||
| // another local process could have read the key in between.) | ||
| write_key_with_restricted_perms(key_path, &key_pair.serialize_pem()) | ||
| .map_err(|e| anyhow::anyhow!("write CA key {}: {e}", key_path.display()))?; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two intertwined problems were causing the security gateway to silently fail to protect agents:
1. Block responses were unreadable to LLM agents. The proxy returned
403with a tiny JSON body. Most agent fetch code only checksresponse.okand discards 4xx bodies, so the agent saw a generic network error and bailed. No information about why the page was blocked, or what to do about it.2. The non-MITM forward-proxy path delivered silent broken protection. It returned
400to every CONNECT, so all HTTPS traffic bypassed inspection entirely. In 2026 that's the entire web. The path was dead weight in practice but the default in code (mitm_enabled = false), and we just spent an hour debugging which mode each host was in. The install script already provisions and trusts a CA automatically — the "lower barrier to entry" rationale for keeping non-MITM has expired.What changes
Block response shape (commit
509e596)mitm_blocked_responsenow returns:HTTP/1.1 200 OK(soresponse.okis true and the body reaches the LLM)Content-Type: text/html; charset=utf-8X-Calciforge-Blocked: trueandX-Calciforge-Reason: <text>headers for tooling that wants structured signals without parsing HTMLThe reason header is sanitised to ASCII so non-printable / multi-byte content can't break HTTP framing. Call sites that previously passed the literal string
"Request rejected"now pass actual error context (decode failure detail, URL-substitution failure cause, header/body substitution failure cause).MITM is the only proxy mode (commit
e03e036)proxy.rs::{intercept, proxy_handler, blocked_response, health_handler}, all ofrouter.rs(build_app,vault_handler,VAULT_TOKEN_ENV).mitm.rsis now the single entry point. It already had its ownhealth_responseandvault_response;vault_json_responsewas inlined as a private helper.main.rsno longer branches onmitm_enabled.config.rsno longer has the field.tests/mitm_https.rs(destination-allowlist block, ref-in-unsupported-content-type block, vault 401, vault 503).https_mitm_destination_allowlist_blocks_disallowed_hosttest asserts the body must not echo the ref name, and caught the leak. These call sites now use a generic block message and log the detail atwarn!server-side.CA auto-provisioning
If
SECURITY_PROXY_CA_CERT/SECURITY_PROXY_CA_KEYare not set, the binary now generates a persistent self-signed CA at/var/lib/calciforge/ca.{pem,key}on first start (key file0600). Subsequent starts load the existing CA. Verified working on Librarian (231): on first start after deploy, the CA was created and the proxy came up cleanly.The install script's existing CA flow is unaffected — it pre-provisions before the binary starts, so the auto-fallback is for ad-hoc / dev usage where the binary runs standalone.
Stats
+575 / -1728)cargo check --workspaceclean too)Migration notes
SECURITY_PROXY_MITM_ENABLED=falsein old systemdEnvironment=lines is silently ignored — there is no non-MITM mode anymore. The install script already defaulted MITM on, so most installs converge automatically. Old hosts that were sitting on stale config (e.g. Librarian 231, where the dropin pre-dated PR Automate managed OpenClaw local install #107) need a re-run ofbash scripts/install.shto pick up the new systemd units and OpenClaw drop-in.Verified locally
🤖 Generated with Claude Code