Skip to content

feat: write an explicit 30 s timeout on synchronous Agent Guard hooks - #511

Open
iamcristi wants to merge 3 commits into
mainfrom
feat/guard-hook-timeout
Open

iamcristi wants to merge 3 commits into
mainfrom
feat/guard-hook-timeout

Conversation

@iamcristi

@iamcristi iamcristi commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Why

The hook forwarder scripts call agent-monitor with no request timeout (Invoke-WebRequest without -TimeoutSec, curl without --max-time). A request that hangs instead of failing, which is what a TLS-intercepting proxy, captive portal or middlebox that completes the handshake and goes silent produces, stalls the agent for the hook's default timeout on every event:

Agent Default hook timeout Field
Claude Code 600 s (30 s on UserPromptSubmit) timeout
Codex 600 s timeout
Cursor "platform default", no number documented timeout
GitHub Copilot 30 s timeoutSec

On Claude Code that is up to 10 minutes before a tool runs and 10 more after it, per tool call. Every agent documents a timed-out hook as fail-open, so an explicit cap bounds the stall without changing enforcement.

What changes

guard install now writes a 65 s timeout (HOOK_TIMEOUT_SECONDS) on every synchronous guard hook entry. 65 s sits above the forwarder scripts' 60 s request budget (#512), so a hung request fails open inside the script, with its diagnostic, before the agent kills the hook:

  • Claude Code: "timeout": 65 on every guard entry except SessionEnd. Claude Code's SessionEnd hooks share a 1.5 s budget that it raises to match a longer per-hook timeout, so a value there would delay every exit by 65 s instead of bounding a hang.
  • Codex: "timeout": 65 on all seven guard entries in hooks.json, and timeout = 65 in each guard table of the managed requirements.toml. _parse_codex_requirements_toml now returns the guard timeout as a fourth value so a managed install written before this change is reported as modified on reinstall, matching what the JSON clients get from plain dict comparison.
  • Cursor: "timeout": 65 on every guard entry except sessionStart and sessionEnd, which Cursor documents as fire-and-forget ("the agent loop does not wait for or enforce a blocking response").
  • Copilot: "timeoutSec": 65 on every entry, including the SessionStart discovery entry. Copilot has no async hooks, so that entry is synchronous too. Copilot's default is 30 s, below the scripts' 60 s budget, so the explicit value is what lets the forwarder fail open on its own terms there.

Unchanged: the async discovery entries on Claude Code and Codex (Claude Code does not enforce timeout on async: true hooks, and guard discover bounds itself at 5 s stdin / 60 s discovery / 15 s request), the hook scripts, detection and uninstall.

Existing installs will show the timeout as a modified hooks diff on their next guard install, which is the intended signal.

Docs

Caveat worth a manual check

Codex's docs do not spell out what a timed-out synchronous PreToolUse / PermissionRequest command hook does to the tool call. The only concrete sentence ("a PreToolUse callback error, timeout, or malformed response can fail the hook without blocking the tool") is in the managed MCP hooks section. Claude Code, Cursor and Copilot are explicit that a timeout does not block. Worth confirming on a real Codex install before relying on it.

Verification

  • TDD: 14 new tests in tests/unit/test_guard.py (TestSyncHookTimeout plus four Codex TOML cases) were red before the change and are green after; 5 existing exact-shape assertions and 4 parser call sites updated.
  • Full suite in a clean worktree: failure set after the change is byte-identical to the baseline from origin/main (89 entries, all environmental: e2e [binary] params without dist/, SNYK_TOKEN, and the managed-install tests that hit the real /Library/Application Support/ClaudeCode). Passed went from 2381 to 2395, exactly the new tests.
  • pre-commit run --all-files clean; local ruff 0.15 lint and format also clean on both changed files; mypy reports no errors in guard.py.

A follow-up should add a request-level timeout to the scripts themselves (-TimeoutSec / --max-time), which is the primary control; this PR is the agent-side backstop.


Note

Medium Risk
Touches hook config for all supported agents on every reinstall; mis-tuned timeouts could change stall/fail-open behavior, though changes are bounded and documented as fail-open on timeout.

Overview
guard install now writes a 65 s agent-side hook cap (HOOK_TIMEOUT_SECONDS) so hung forwarder requests cannot block Claude/Codex/Cursor for hundreds of seconds per event. The value is intentionally above the forwarder scripts’ ~60 s request budget so the script can fail open with diagnostics before the agent kills the hook.

Per-client behavior: Claude gets timeout: 65 on all guard hooks except SessionEnd (avoids stretching the shared 1.5 s exit budget). Cursor skips sessionStart / sessionEnd (documented fire-and-forget). Codex adds timeout in user hooks.json and managed requirements.toml, with _parse_codex_requirements_toml extended to read/write timeout so older installs show as modified on reinstall. Copilot sets timeoutSec: 65 on every entry, including session discovery (no async hooks).

Unit tests add TestSyncHookTimeout and update shape/diff expectations; async discovery hooks remain without these timeouts.

Reviewed by Cursor Bugbot for commit d20c075. Bugbot is set up for automated code reviews on this repo. Configure here.

A forwarder request to agent-monitor that hangs instead of failing (a proxy or
middlebox that completes the handshake and then goes silent) stalls the agent
for the hook's default timeout on every event: 600 s on Claude Code and Codex,
an undocumented platform default on Cursor. Every agent treats a timed-out hook
as fail-open, so an explicit cap bounds the stall without changing enforcement.

Per client:
- Claude Code: `timeout: 30` on every guard entry except SessionEnd, whose
  1.5 s shared budget Claude Code raises to match a longer per-hook timeout.
- Codex: `timeout: 30` on all seven guard entries in hooks.json and a
  `timeout = 30` line in each guard table of the managed requirements.toml.
  The TOML parser now reads the guard timeout back, so a managed install
  written before this change is reported as modified on reinstall, matching
  what the JSON clients already get from dict comparison.
- Cursor: `timeout: 30` on every guard entry except sessionStart and
  sessionEnd, which Cursor documents as fire-and-forget.
- Copilot: `timeoutSec: 30` on every entry, including the SessionStart
  discovery entry, since Copilot has no async hooks. 30 s is already its
  default; writing it keeps the bound explicit.

The async discovery entries on Claude Code and Codex are unchanged: Claude Code
does not enforce `timeout` on async hooks, and the discovery CLI bounds itself.
@iamcristi
iamcristi requested review from a team and marcelosousa as code owners October 14, 2026 18:43
@qodo-merge-etso

Copy link
Copy Markdown

PR Summary by Qodo

Cap synchronous Agent Guard hooks at 30 seconds

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Cap synchronous Agent Guard hook waits at 30 seconds across Claude, Codex, Cursor, and Copilot.
• Exclude nonblocking hooks where timeouts are ineffective or could delay exits.
• Flag older Codex managed installs as modified and test timeout behavior across clients.
Diagram

graph TD
  I["Guard install"] --> P{"Client type"} --> C["Claude JSON"] --> R["Agent hooks"]
  P --> D["Codex JSON/TOML"] --> R
  P --> U["Cursor JSON"] --> R
  P --> G["Copilot JSON"] --> R
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add request-level timeouts to forwarder scripts
  • ➕ Directly bounds a hung network request.
  • ➕ Can fail sooner than the agent hook limit.
  • ➖ Does not bound other causes of a stalled hook process.
  • ➖ Requires changes to the platform-specific scripts.

Recommendation: Keep the explicit client hook timeouts as an agent-side backstop, with exceptions for nonblocking hooks. Add request-level timeouts to the forwarder scripts in a follow-up; the two limits address different failure boundaries.

Files changed (2) +290 / -27

Bug fix (1) +82 / -18
guard.pyWrite bounded synchronous hooks for all four clients +82/-18

Write bounded synchronous hooks for all four clients

• Adds a shared 30-second limit to applicable Claude, Codex, and Cursor guard entries and to all Copilot entries. Managed Codex TOML now writes and parses the guard timeout so reinstall diffs identify older configurations as modified.

src/agent_scan/guard.py

Tests (1) +208 / -9
test_guard.pyCover timeout output and pre-timeout reinstall diffs +208/-9

Cover timeout output and pre-timeout reinstall diffs

• Tests client-specific timeout exceptions, Copilot discovery, Windows hook shapes, and upgrades from configurations without timeouts. Adds managed Codex TOML rendering, parsing, and modified-diff checks while updating exact-shape assertions.

tests/unit/test_guard.py

@qodo-merge-etso

qodo-merge-etso Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Some hook timeout changes go unreported 🐞 Bug ≡ Correctness
Description
_parse_codex_requirements_toml retains the timeout from only the first guard table, and
_prepare_codex_managed_config uses it as the actual timeout for every event. If a managed Codex
file has a 30-second timeout on its first guard hook but a missing or different timeout on a later
one, reinstall rewrites that later hook without reporting it as modified.
Code

src/agent_scan/guard.py[R971-974]

+        actual = (
+            _entries(event, old_guard_command, old_discover_command, old_guard_timeout)
+            if event in old_event_set
+            else []
Relevance

●●● Strong

Clear correctness bug: one timeout cannot accurately represent differing per-event timeouts during
managed reinstall.

PR-#365

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The parser resolves the timeout once from the first guard table; the diff then supplies that single
value to every event. The writer replaces the file with rendered content, so a later event's
differing timeout is changed even when the diff omits it.

src/agent_scan/guard.py[1027-1033]
src/agent_scan/guard.py[970-975]
src/agent_scan/guard.py[928-934]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The managed Codex parser returns one timeout for all guard hooks, so the reinstall diff misses changes to later hooks whose timeouts differ from the first.

## Fix Focus Areas
- src/agent_scan/guard.py[1018-1063]
- src/agent_scan/guard.py[970-975]
- tests/unit/test_guard.py[3255-3274]

## Recommended Fix
Parse each guard hook's timeout alongside its event and command, then compare each event's actual entry against its expected entry. Add a test that removes or changes the timeout on only one later event.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


2. Valid timeouts appear missing on reinstall 🐞 Bug ≡ Correctness
Description
_parse_codex_requirements_toml recognizes timeout only when its value consists of bare decimal
digits followed immediately by the end of the line. If an existing managed Codex file uses valid
TOML such as timeout = 30 # guard timeout or timeout = +30, reinstall reports the hook as
modified even though its configured timeout is already 30 seconds.
Code

src/agent_scan/guard.py[1025]

+    timeout_re = re.compile(r"^timeout\s*=\s*(\d+)\s*$")
Relevance

●●● Strong

Regex rejects valid TOML timeout syntax, causing false modified reports; this is a deterministic
parser bug.

PR-#365

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added regex rejects those values, leaving table_timeout unset; the comparison treats the hook
as lacking a timeout. Existing files use this handwritten parser, while TOML parsing validates only
newly rendered content.

src/agent_scan/guard.py[1021-1025]
src/agent_scan/guard.py[1039-1046]
src/agent_scan/guard.py[945-950]
src/agent_scan/guard.py[909-914]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new timeout regex treats valid TOML integer forms and inline comments as an absent timeout, producing an inaccurate managed-install diff.

## Fix Focus Areas
- src/agent_scan/guard.py[1023-1046]
- tests/unit/test_guard.py[3237-3274]

## Recommended Fix
Read the existing timeout using TOML parsing, or support valid TOML integer syntax and inline comments in the scoped table parser. Test a managed file whose timeout has an inline comment and one using a signed integer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


Grey Divider

Context sources
✅ Compliance rules (platform): 8 rules
✅ Cross-repo context — repo relationships
Review mode: Auto: 🧠 Deep: Multiple agent-specific paths and parser changes create dense, easy-to-miss behavioral defects.

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/agent_scan/guard.py
Comment on lines +971 to +974
actual = (
_entries(event, old_guard_command, old_discover_command, old_guard_timeout)
if event in old_event_set
else []

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Some hook timeout changes go unreported 🐞 Bug ≡ Correctness

_parse_codex_requirements_toml retains the timeout from only the first guard table, and
_prepare_codex_managed_config uses it as the actual timeout for every event. If a managed Codex
file has a 30-second timeout on its first guard hook but a missing or different timeout on a later
one, reinstall rewrites that later hook without reporting it as modified.
Agent Prompt
## Issue description
The managed Codex parser returns one timeout for all guard hooks, so the reinstall diff misses changes to later hooks whose timeouts differ from the first.

## Fix Focus Areas
- src/agent_scan/guard.py[1018-1063]
- src/agent_scan/guard.py[970-975]
- tests/unit/test_guard.py[3255-3274]

## Recommended Fix
Parse each guard hook's timeout alongside its event and command, then compare each event's actual entry against its expected entry. Add a test that removes or changes the timeout on only one later event.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

Comment thread src/agent_scan/guard.py
table_is_guard = False
header_re = re.compile(r"^\[\[hooks\.([A-Za-z]+)(?:\.hooks)?\]\]\s*$")
command_re = re.compile(r'^command\s*=\s*"((?:[^"\\]|\\.)*)"\s*$')
timeout_re = re.compile(r"^timeout\s*=\s*(\d+)\s*$")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Valid timeouts appear missing on reinstall 🐞 Bug ≡ Correctness

_parse_codex_requirements_toml recognizes timeout only when its value consists of bare decimal
digits followed immediately by the end of the line. If an existing managed Codex file uses valid
TOML such as timeout = 30 # guard timeout or timeout = +30, reinstall reports the hook as
modified even though its configured timeout is already 30 seconds.
Agent Prompt
## Issue description
The new timeout regex treats valid TOML integer forms and inline comments as an absent timeout, producing an inaccurate managed-install diff.

## Fix Focus Areas
- src/agent_scan/guard.py[1023-1046]
- tests/unit/test_guard.py[3237-3274]

## Recommended Fix
Read the existing timeout using TOML parsing, or support valid TOML integer syntax and inline comments in the scoped table parser. Test a managed file whose timeout has an inline comment and one using a signed integer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

iamcristi added a commit that referenced this pull request Oct 15, 2026
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.
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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3cc7ec5. Configure here.

Comment thread src/agent_scan/guard.py
# event. Each agent treats a timed-out hook as fail-open. The value sits above the
# forwarder scripts' 60 s request budget, so a hung request fails open inside the
# script, with its diagnostic, before the agent kills the hook.
HOOK_TIMEOUT_SECONDS = 65

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Timeout raised without script request cap

Medium Severity

HOOK_TIMEOUT_SECONDS is now 65 to outlast a 60 s forwarder budget, but snyk-agent-guard.sh and snyk-agent-guard.ps1 still call curl and Invoke-WebRequest with no request timeout. Copilot's timeoutSec and Claude Code's UserPromptSubmit 30 s defaults are both raised, so a hung request stalls longer and the script never fails open with a diagnostic.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3cc7ec5. Configure here.

@iamcristi
iamcristi enabled auto-merge (squash) October 15, 2026 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant