Repository navigation
feat: write an explicit 30 s timeout on synchronous Agent Guard hooks #511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,13 @@ | |
| _PERMISSION_DENIED = "__permission_denied__" | ||
| _STDIN_READ_TIMEOUT_SECONDS = 5.0 | ||
| _DISCOVERY_TIMEOUT_SECONDS = 60.0 | ||
| # Per-hook timeout written on every synchronous guard hook. Claude Code and Codex | ||
| # default to 600 s, Cursor to an undocumented platform default; a forwarder request | ||
| # that hangs rather than fails would otherwise stall the agent for that long on every | ||
| # 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 | ||
|
|
||
| CLAUDE_SETTINGS_PATH = Path.home() / ".claude" / "settings.json" | ||
| CURSOR_HOOKS_PATH = Path.home() / ".cursor" / "hooks.json" | ||
|
|
@@ -133,6 +140,9 @@ | |
| "subagentStart", | ||
| "subagentStop", | ||
| ] | ||
| # Cursor documents these two as fire-and-forget: the agent loop never waits on them, so | ||
| # they are not synchronous hooks and get no timeout. | ||
| _CURSOR_FIRE_AND_FORGET_EVENTS = {"sessionStart", "sessionEnd"} | ||
|
|
||
| COPILOT_HOOK_EVENTS = [ | ||
| "SessionStart", | ||
|
|
@@ -687,9 +697,14 @@ def _prepare_claude_config( | |
| hooks = {} | ||
|
|
||
| for event in CLAUDE_HOOK_EVENTS: | ||
| entry = {"type": "command", "command": command} | ||
| entry: dict = {"type": "command", "command": command} | ||
| if IS_WINDOWS: | ||
| entry["shell"] = "powershell" | ||
| if event != "SessionEnd": | ||
| # SessionEnd hooks share a 1.5 s budget that Claude Code raises to match a | ||
| # longer per-hook timeout, so a value here would delay every exit rather | ||
| # than bound a hang. | ||
| entry["timeout"] = HOOK_TIMEOUT_SECONDS | ||
| group: dict = {"hooks": [entry]} | ||
| if event in CLAUDE_EVENTS_WITH_MATCHER: | ||
| group["matcher"] = "*" | ||
|
|
@@ -745,8 +760,11 @@ def _prepare_cursor_config( | |
| hooks = {} | ||
|
|
||
| for event in CURSOR_HOOK_EVENTS: | ||
| entry: dict = {"command": command} | ||
| if event not in _CURSOR_FIRE_AND_FORGET_EVENTS: | ||
| entry["timeout"] = HOOK_TIMEOUT_SECONDS | ||
| existing = list(filtered.get(event, [])) | ||
| existing.append({"command": command}) | ||
| existing.append(entry) | ||
| hooks[event] = existing | ||
|
|
||
| if discover_command: | ||
|
|
@@ -820,7 +838,7 @@ def _prepare_codex_config( | |
| hooks = {} | ||
|
|
||
| for event in CODEX_HOOK_EVENTS: | ||
| entry = {"type": "command", "command": command} | ||
| entry = {"type": "command", "command": command, "timeout": HOOK_TIMEOUT_SECONDS} | ||
| existing = list(filtered.get(event, [])) | ||
| existing.append({"hooks": [entry]}) | ||
| hooks[event] = existing | ||
|
|
@@ -881,6 +899,7 @@ def _render_codex_requirements_toml( | |
| lines.append(f"[[hooks.{event}.hooks]]") | ||
| lines.append('type = "command"') | ||
| lines.append(f"command = {toml_escape(command)}") | ||
| lines.append(f"timeout = {HOOK_TIMEOUT_SECONDS}") | ||
| lines.append("") | ||
| if discover_command: | ||
| lines.append("[[hooks.SessionStart]]") | ||
|
|
@@ -924,18 +943,24 @@ def _prepare_codex_managed_config( | |
| old_events: list[str] = [] | ||
| old_guard_command: str | None = None | ||
| old_discover_command: str | None = None | ||
| old_guard_timeout: int | None = None | ||
| if path.exists(): | ||
| old_text = path.read_text() | ||
| with contextlib.suppress(UnicodeDecodeError, ValueError): | ||
| old_events, old_guard_command, old_discover_command = _parse_codex_requirements_toml(old_text) | ||
| old_events, old_guard_command, old_discover_command, old_guard_timeout = _parse_codex_requirements_toml( | ||
| old_text | ||
| ) | ||
|
|
||
| old_event_set = set(old_events) | ||
| new_event_set = set(CODEX_HOOK_EVENTS) | ||
|
|
||
| def _entries(event: str, guard: str | None, discover: str | None) -> list[dict]: | ||
| def _entries(event: str, guard: str | None, discover: str | None, timeout: int | None) -> list[dict]: | ||
| entries: list[dict] = [] | ||
| if guard is not None: | ||
| entries.append({"type": "command", "command": guard}) | ||
| guard_entry: dict = {"type": "command", "command": guard} | ||
| if timeout is not None: | ||
| guard_entry["timeout"] = timeout | ||
| entries.append(guard_entry) | ||
| if event == "SessionStart" and discover is not None: | ||
| entries.append({"type": "command", "command": discover, "async": True}) | ||
| return entries | ||
|
|
@@ -944,8 +969,12 @@ def _entries(event: str, guard: str | None, discover: str | None) -> list[dict]: | |
| modified = {} | ||
| removed = {} | ||
| for event in sorted(old_event_set | new_event_set): | ||
| expected = _entries(event, command, discover_command) if event in new_event_set else [] | ||
| actual = _entries(event, old_guard_command, old_discover_command) if event in old_event_set else [] | ||
| expected = _entries(event, command, discover_command, HOOK_TIMEOUT_SECONDS) if event in new_event_set else [] | ||
| actual = ( | ||
| _entries(event, old_guard_command, old_discover_command, old_guard_timeout) | ||
| if event in old_event_set | ||
| else [] | ||
|
Comment on lines
+973
to
+976
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Some hook timeout changes go unreported _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
|
||
| ) | ||
| if not actual and expected: | ||
| removed[event] = expected | ||
| elif actual and not expected: | ||
|
|
@@ -976,22 +1005,47 @@ def _is_discover_hook_command(command: str) -> bool: | |
| return bool(re.search(r"\bsnyk-agent-guard-discover(?:\.(?:sh|ps1))?\b", command, re.IGNORECASE)) | ||
|
|
||
|
|
||
| def _parse_codex_requirements_toml(text: str) -> tuple[list[str], str | None, str | None]: | ||
| """Extract Snyk Agent Guard events, guard command, and discovery command. | ||
| def _parse_codex_requirements_toml( | ||
| text: str, | ||
| ) -> tuple[list[str], str | None, str | None, int | None]: | ||
| """Extract Snyk Agent Guard events, guard command, discovery command, and guard timeout. | ||
|
|
||
| Only scans hook command lines containing the agent-guard detection marker. | ||
| Only scans hook command lines containing the agent-guard detection marker. The | ||
| timeout is the ``timeout`` key of the first guard hook table, or None when that table | ||
| has none, so an install written before timeouts existed diffs as modified. | ||
| """ | ||
| events: list[str] = [] | ||
| guard_command: str | None = None | ||
| discover_command: str | None = None | ||
| guard_timeout: int | None = None | ||
| guard_timeout_resolved = False | ||
| current_event: str | None = None | ||
| table_timeout: int | None = None | ||
| 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*$") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Valid timeouts appear missing on reinstall _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
|
||
|
|
||
| def close_table() -> None: | ||
| # Keys inside a table may come in any order, so the timeout is only attributed | ||
| # once the table is complete and known to hold the guard command. | ||
| nonlocal guard_timeout, guard_timeout_resolved | ||
| if table_is_guard and not guard_timeout_resolved: | ||
| guard_timeout = table_timeout | ||
| guard_timeout_resolved = True | ||
|
|
||
| for raw in text.splitlines(): | ||
| line = raw.strip() | ||
| m = header_re.match(line) | ||
| if m: | ||
| close_table() | ||
| current_event = m.group(1) | ||
| table_timeout = None | ||
| table_is_guard = False | ||
| continue | ||
| m = timeout_re.match(line) | ||
| if m and current_event: | ||
| table_timeout = int(m.group(1)) | ||
| continue | ||
| m = command_re.match(line) | ||
| if m and current_event: | ||
|
|
@@ -1003,14 +1057,17 @@ def _parse_codex_requirements_toml(text: str) -> tuple[list[str], str | None, st | |
| if _is_discover_hook_command(cmd): | ||
| if discover_command is None: | ||
| discover_command = cmd | ||
| elif guard_command is None: | ||
| guard_command = cmd | ||
| return events, guard_command, discover_command | ||
| else: | ||
| table_is_guard = True | ||
| if guard_command is None: | ||
| guard_command = cmd | ||
| close_table() | ||
| return events, guard_command, discover_command, guard_timeout | ||
|
|
||
|
|
||
| def _detect_codex_managed_install(path: Path) -> dict | None: | ||
| text = path.read_text() | ||
| events, guard_command, _ = _parse_codex_requirements_toml(text) | ||
| events, guard_command, _, _ = _parse_codex_requirements_toml(text) | ||
| if not events or guard_command is None: | ||
| return None | ||
| return _parse_command_info(guard_command, events) | ||
|
|
@@ -1021,7 +1078,7 @@ def _uninstall_codex_managed(path: Path) -> None: | |
| rich.print("[dim]No requirements.toml found. Nothing to uninstall.[/dim]") | ||
| return | ||
| text = path.read_text() | ||
| events, _, _ = _parse_codex_requirements_toml(text) | ||
| events, _, _, _ = _parse_codex_requirements_toml(text) | ||
| if not events: | ||
| rich.print("[dim]No Agent Guard hooks found.[/dim]") | ||
| return | ||
|
|
@@ -1606,8 +1663,17 @@ def _filter_claude_hooks(hooks: dict) -> dict: | |
|
|
||
|
|
||
| def _copilot_command_entry(command: str) -> dict: | ||
| """Build a Copilot command-hook entry for this platform.""" | ||
| return {"type": "command", "powershell" if IS_WINDOWS else "bash": command} | ||
| """Build a Copilot command-hook entry for this platform. | ||
|
|
||
| Copilot has no async hooks, so every entry (discovery included) is synchronous and | ||
| carries the timeout. Copilot's documented default is 30 s, below the forwarder's | ||
| request budget, so the value must be written for the script to fail open on its own. | ||
| """ | ||
| return { | ||
| "type": "command", | ||
| "powershell" if IS_WINDOWS else "bash": command, | ||
| "timeoutSec": HOOK_TIMEOUT_SECONDS, | ||
| } | ||
|
|
||
|
|
||
| def _copilot_hook_commands(entry: dict) -> Iterable[str]: | ||
|
|
||
There was a problem hiding this comment.
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_SECONDSis now 65 to outlast a 60 s forwarder budget, butsnyk-agent-guard.shandsnyk-agent-guard.ps1still callcurlandInvoke-WebRequestwith no request timeout. Copilot'stimeoutSecand Claude Code'sUserPromptSubmit30 s defaults are both raised, so a hung request stalls longer and the script never fails open with a diagnostic.Additional Locations (2)
src/agent_scan/guard.py#L1659-L1671src/agent_scan/guard.py#L702-L707Reviewed by Cursor Bugbot for commit 3cc7ec5. Configure here.