Repository navigation
fix: bound the Agent Guard forwarder request and fail open on transport errors - #512
Merged
Merged
Conversation
The hook forwarders had no timeout at all: snyk-agent-guard.sh read stdin with cat and ran curl without --max-time; snyk-agent-guard.ps1 read stdin with ReadToEnd() and ran Invoke-WebRequest without -TimeoutSec, which is indefinite in Windows PowerShell 5.1. A stdin pipe left open, or a request that hangs instead of failing, held the hook until the agent killed it: 600 s per event on Claude Code and Codex (#510). Both scripts now resolve one timeout, 27 s by default, from SNYK_AGENT_GUARD_TIMEOUT_SECONDS (a positive integer, else the default) and apply it to every wait: the stdin read (bash `read -t` in chunks, bash 3.2 compatible; Task.Wait on ReadToEndAsync in PowerShell), curl's --connect-timeout and --max-time, and -TimeoutSec. 27 s sits above agent-monitor's ~24 s policy budget, so a slow verdict is never cut, and below the 30 s agent-side cap written by #511, so the agent sees a clean exit instead of a killed process. Transport and HTTP failures, and a stdin read that times out, now fail open for every client: exit 0, empty stdout, one diagnostic line on stderr. Copilot denies the tool call on any non-zero exit of a preToolUse hook, so the previous exit 1 turned a hung network into a denial there; Cursor blocks on non-JSON stdout but allows on no output; Claude Code and Codex ignore exit 0 with empty stdout. Usage errors (missing variables, empty payload, missing curl or base64) still exit 1. The install test event requires "success": true in stdout (#509), so an install against an unreachable endpoint still fails; _invoke_hook_script now folds the script's stderr into that failure detail so the reason is visible to the operator.
PR Summary by QodoBound Agent Guard forwarder waits and fail open on request errors
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
![qodo-merge-etso[bot]](https://avatars.githubusercontent.com/u/12959162?s=60&v=4){kind=small}
Comment on lines
+196
to
+199
| if ($timedOut) { | ||
| FailOpen "request timed out after ${timeoutSeconds}s" | ||
| } | ||
| FailOpen "request failed: $($ex.Message)" |
There was a problem hiding this comment.
2. Transport errors expose internal details 📘 Rule violation ☼ Reliability
snyk-agent-guard.ps1 interpolates $ex.Message directly into the fail-open diagnostic sent to stderr. Any non-timeout transport exception can therefore expose exception details such as request or host information to the hook consumer and to install-time error output.
Agent Prompt
## Issue description
The PowerShell forwarder sends the raw exception message to stderr, which can expose internal request or host details in user-facing hook and installation diagnostics.
## Fix Focus Areas
- src/agent_scan/hooks/snyk-agent-guard.ps1[188-199]
- src/agent_scan/guard.py[1378-1384]
## Recommended Fix
Replace the raw exception message with a generic, stable transport-failure reason while retaining only the necessary timeout or HTTP status information. Keep detailed exception data out of hook output and installation failure messages.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+177
to
+178
| $response = Invoke-WebRequest -Uri $url -Method POST -Body $bodyBytes -Headers $headers ` | ||
| -UseBasicParsing -TimeoutSec $timeoutSeconds |
There was a problem hiding this comment.
3. Windows hook can still stall on a slow response body 🐞 Bug ☼ Reliability
Invoke-WebRequest -TimeoutSec in snyk-agent-guard.ps1 bounds the connection and response-header wait but not body reads, which can remain subject to PowerShell 5.1’s default 300-second ReadWriteTimeout despite the script and CLI docs promising a timeout for the whole request. If a server sends headers and then stalls, the forwarder cannot reach FailOpen at the configured deadline, and this PR’s installed Claude hook has no explicit agent-side timeout to bound the wait.
Agent Prompt
## Issue description
On PowerShell 5.1, `Invoke-WebRequest -TimeoutSec` does not bound response-body reads, so a server that sends headers and then stalls can keep the forwarder blocked rather than reaching `FailOpen` at the configured deadline. The script header and CLI docs also incorrectly say the timeout covers the whole request.
## Fix Focus Areas
- src/agent_scan/hooks/snyk-agent-guard.ps1[10-13]
- src/agent_scan/hooks/snyk-agent-guard.ps1[177-201]
- docs/cli-reference.md[410-410]
- tests/unit/test_guard.py[2443-2455]
## Recommended Fix
Enforce the configured deadline across both response-header and body retrieval on PowerShell 5.1, and route expiry through `FailOpen`. For example, use `System.Net.HttpWebRequest` with both `.Timeout` and `.ReadWriteTimeout` set to `$timeoutSeconds*1000`, or use `System.Net.Http.HttpClient` with a timeout covering the full response and `.Wait(ms)` on the task. Add a test server that sends headers and part of a body before stalling. If full-request enforcement is not implemented, at minimum correct the script header and docs row to say that the Windows timeout covers the connection and response headers, not the body.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Windows retries the SYN on a refused loopback connect for about two seconds, so with a 2 s budget -TimeoutSec fires first and the script reports a timeout instead of a refused connection. Both are the fail-open path: widen that test's budget to 5 s and accept either reason.
SNYK_AGENT_GUARD_TIMEOUT_SECONDS let anything that can set the agent's environment (a shell profile, direnv, a project .claude/settings.json env block in a cloned repo) shrink the 27 s budget to 1 s, so every LLM-judged verdict timed out and the hook failed open. Neither the script checksum nor the config tamper check sees the environment, so the override was an unmonitored way to switch enforcement off. Both forwarders now use one constant, TIMEOUT_SECONDS=27, and read no override; the resolve step (and its subshell on every hook call) is gone. The docs drop the variable and describe the fixed 27 s limit and the fail-open behaviour under `guard install` instead. Tests: the two cases per script that wait for a timeout to fire (hanging server, stdin held open) run a test-only copy with the constant rewritten to 1-2 s; the rest run the shipped script at 27 s, which also makes the Windows refused-connect case deterministic again. New test_environment_cannot_shorten_the_timeout (bash and PowerShell) sets the variable and asserts the script is still waiting.
Drop the comments, docs sentence and tests that only described what the timeout is not, so the scripts, docs and tests describe the fixed 27 s budget and nothing else.
The budget regexes run on bytes so the patched copy keeps the shipped line endings, but Windows checks the scripts out with CRLF and `$` only matches before `\n`, so neither script's TIMEOUT_SECONDS was found there. A `(?=\r?$)` lookahead matches either line end and leaves the `\r` in place.
--max-time already covers DNS, TCP and TLS, and a connect that runs out of time exits 28 the same way, so an equal --connect-timeout next to it changed nothing.
Both forwarders read stdin as on main again (cat in bash, ReadToEnd in PowerShell); TIMEOUT_SECONDS bounds the request alone. That drops the bash 3.2 read -t loop, its timeout-versus-EOF guesswork and its per-byte reads, and the PowerShell Task.Wait.
iamcristi
changed the title
Failing open silently left a broken network or a revoked push key
invisible: Claude Code never shows stderr from a hook that exits 0, so
Agent Guard could stop validating without anyone noticing.
On every fail-open path both forwarders now write a fixed, user-facing
sentence ("Unable to reach Snyk Evo to validate agent behavior. Please
check your network and configuration and try again.") to stderr together
with the technical reason, and return it as a `systemMessage` JSON object
on stdout for the clients whose hook output defines that field, Claude
Code and Codex, which render it as a warning. Cursor and GitHub Copilot
keep empty stdout: Cursor's permission hooks block on stdout that does
not match their schema, and Copilot documents no such field. The sentence
is a constant with nothing to escape, so it is embedded verbatim; a test
pins that in both scripts.
The install test event still rejects this body, since it requires
"success": true, and its failure detail already carries stderr.
agent-monitor holds a PreToolUse for up to 25 s until the session's discovery arrives, then evaluates within its 25 s autorelease budget, so a legitimate verdict can take ~50 s. At 27 s the scripts gave up on the first MCP calls of a session, exactly where enforcement matters most. The lower bound the budget test pins moves from 25 s to 50 s. The upper bound against the agent-side cap is asserted next to that cap (#511), which moves to 65 s.
iamcristi
added a commit
that referenced
this pull request
Oct 15, 2026
The forwarder scripts now give the request 60 s (#512), because agent-monitor may legitimately take ~50 s to answer. A 30 s cap would kill the hook before the script could fail open with its diagnostic, and below Copilot's 30 s default the explicit value was redundant. A new test asserts the scripts' TIMEOUT_SECONDS stays under the cap, skipping until the forwarder budget is on the same branch.
lbeurerkellner
approved these changes
Oct 15, 2026
This was referenced Oct 15, 2026
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
2 participants
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.
Field report: #510. Agent-side half: #511 (65 s
timeouton every synchronous hook entry). This PR is the script half. Until #511 is on main, Copilot (30 s defaulttimeoutSec) and Claude Code's UserPromptSubmit (30 s default) kill the hook before its 60 s budget fires; both treat that as fail-open, so nothing is blocked, but the user-facing warning below is not shown there.Why
The forwarder scripts had no timeout anywhere. A request that hangs instead of failing (a proxy or middlebox that completes the handshake and goes silent) held the hook until the agent killed it: 600 s per event on Claude Code and Codex by default. #510 reports exactly this on Windows: three tool calls hung 12+ minutes, PreToolUse hook p95 3.6 s with a 185 s maximum. For the matching tenant, agent-monitor answered every PreToolUse in under 0.5 s on those days, so the time was lost on the client or the network.
A
timeout 60 bash …wrapper in the hook command line was considered and rejected: stock macOS ships notimeoutbinary (Homebrew coreutils has it only asgtimeout), Windows'timeout.exeonly pauses, and PowerShell has no built-in run-with-a-limit that also keeps the agent's stdin pipe. Nothing may be assumed installed on endpoints, so every bound lives inside the scripts and uses only what they already require.What changes
One constant per script,
TIMEOUT_SECONDS = 60, bounding the request:snyk-agent-guard.shcurl --max-timesnyk-agent-guard.ps1Invoke-WebRequest -TimeoutSec60 s sits above what agent-monitor may legitimately take: it holds a PreToolUse for up to 25 s until the session's discovery arrives (
SESSION_DISCOVERY_WAIT_SECONDS), then evaluates within its 25 s autorelease budget, so a verdict can take ~50 s plus network. It sits below the 65 s agent-side cap from #511, so the agent sees a clean exit rather than a killed process.Fail open for every client. On any transport failure (timeout, DNS, refused, reset) or HTTP >= 400, both scripts exit 0 and write one line to stderr,
snyk-agent-guard: <user-facing sentence> (<reason>; failing open)(curl's own stderr line is kept because it is the only place the DNS/refused/TLS reason is spelled out). Per the agents' docs: GitHub Copilot denies the tool call on any non-zero exit of apreToolUsehook, so the previousexit 1turned a hung network into a denial there; Cursor permission hooks block on non-JSON stdout but treat no output as fail-open; Claude Code and Codex ignore exit 0 with empty stdout. Usage errors (missingPUSH_KEY/REMOTE_HOOKS_BASE_URL/MACHINE_ID/--client, empty payload, missing curl or base64) still exit 1.Telling the user. Failing open silently would leave a broken network or a revoked push key invisible, since Claude Code never shows stderr from a hook that exits 0. So the fail-open path also returns a fixed sentence,
Unable to reach Snyk Evo to validate agent behavior. Please check your network and configuration and try again., as{"systemMessage": "..."}on stdout for the clients whose hook output defines that field:systemMessageper its docsThe sentence is a constant with nothing to escape, so it is embedded verbatim; a test pins that it is identical in both scripts and JSON-safe. During an outage Claude Code and Codex show the warning on each event that fails open, so a turn can produce several.
Consequence worth knowing: an invalid or revoked push key (HTTP 4xx) is now a fail-open at run time on every agent, visible as the warning above on Claude Code and Codex and only on stderr elsewhere. Install still fails loudly because the test event requires
"success": truein stdout (#509);_invoke_hook_scriptnow folds the script's stderr into that failure detail so the operator sees the reason instead of only "push key may be invalid".docs/cli-reference.mddocuments the fixed 60 s limit, the fail-open behaviour and thesystemMessagewarning underguard install. The discovery trampolines are unchanged: they never read stdin andguard discoveralready bounds itself.Docs relied on
-TimeoutSecdefault 0 = indefinite: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-webrequest?view=powershell-5.1timeoutpauses only: https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/timeout--max-time, exit 28: https://everything.curl.dev/usingcurl/timeouts.htmlpreToolUsefail-closed on non-zero exit, timeouts fail-open: https://docs.github.com/en/copilot/reference/hooks-configurationsystemMessage: https://code.claude.com/docs/en/hookssystemMessageand timeouts: https://learn.chatgpt.com/docs/hooksVerification
tests/unit/test_guard.py, red before the change, green after. Bash: hanging server, HTTP 500, connection refused, empty payload still a usage error, and a per-client case (claude-code, codex, cursor, github-copilot) pinning the JSONsystemMessageon stdout for the first two and empty stdout for the last two. The hanging-server case runs a test-only copy of the script with the constant rewritten to 2 s (_forwarder_with_budget); the rest run the shipped script at 60 s. PowerShell: the same cases, Windows-only, so thewindows-latestCI leg is the only place they execute.TestHookScriptRequestBudgetpins budget > 50 (the upper bound against the agent-side cap is asserted next to that cap, in feat: write an explicit 30 s timeout on synchronous Agent Guard hooks #511), one shared constant, the constant outside the install-time section, and no stray numeric timeout literal in either script.origin/mainbaseline (89 environmental entries: e2e[binary]params withoutdist/,SNYK_TOKEN, the managed-install tests that hit the real/Library/Application Support/ClaudeCode). Passed 2381 → 2403./bin/bash3.2 on macOS: hanging server → exit 0,curl: (28) …+ the fail-open line on stderr, 60.0 s wall clock. HTTP 500 server with each of the four clients: claude-code and codex print exactly{"systemMessage":"…"}(parses withjson.tool), cursor and github-copilot print nothing; all exit 0 with the sentence on stderr.pre-commit run --all-filesclean; local ruff 0.15 lint and format clean;shellcheckclean on the bash script; mypy reports nothing inguard.py.Note
Medium Risk
Changes synchronous hook behavior when Agent Guard is unreachable—agents proceed without validation and may show warnings—but reduces long hangs and fixes Copilot incorrectly denying tools on network errors; install still fails on bad push keys when the test event requires success.
Overview
Agent Guard hook forwarders (
snyk-agent-guard.sh/.ps1) now cap outbound requests at 60 seconds (curl --max-time/Invoke-WebRequest -TimeoutSec) so hung proxies no longer block hooks until the agent kills them.On timeout, transport errors, or HTTP ≥ 400, both scripts fail open (exit 0, one stderr line) instead of exiting 1—important for Copilot, which denies tools on non-zero hook exit. Claude Code and Codex also get a fixed
{"systemMessage":"…"}warning on stdout; Cursor and GitHub Copilot get empty stdout so invalid JSON does not block the agent. Usage errors (missing keys, empty payload) still exit 1.guard installtest-event validation inguard.pynow surfaces the forwarder’s stderr when stdout lackssuccess: true, so operators see timeout/fail-open reasons instead of only “push key may be invalid.” Revoked keys at runtime now fail open with that warning on Claude/Codex.CLI docs under
guard installdescribe the 60s limit and fail-open behavior. Tests cover bash/PowerShell fail-open paths, shared timeout/message constants, and install-time invoke stderr handling.Reviewed by Cursor Bugbot for commit 1ca6b6f. Bugbot is set up for automated code reviews on this repo. Configure here.