From 642bbd3592d1e5105d20f05442c41467e8c4645d Mon Sep 17 00:00:00 2001 From: iamcristi Date: Wed, 14 Oct 2026 20:42:55 +0200 Subject: [PATCH 1/2] feat: write an explicit 30 s timeout on synchronous Agent Guard hooks 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. --- src/agent_scan/guard.py | 100 ++++++++++++++---- tests/unit/test_guard.py | 217 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 290 insertions(+), 27 deletions(-) diff --git a/src/agent_scan/guard.py b/src/agent_scan/guard.py index ca64d5b1..530bf45c 100644 --- a/src/agent_scan/guard.py +++ b/src/agent_scan/guard.py @@ -65,6 +65,11 @@ _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. +HOOK_TIMEOUT_SECONDS = 30 CLAUDE_SETTINGS_PATH = Path.home() / ".claude" / "settings.json" CURSOR_HOOKS_PATH = Path.home() / ".cursor" / "hooks.json" @@ -133,6 +138,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 +695,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 +758,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 +836,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 +897,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 +941,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 +967,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 [] + ) if not actual and expected: removed[event] = expected elif actual and not expected: @@ -976,22 +1003,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*$") + + 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 +1055,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 +1076,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 @@ -1601,8 +1656,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. 30 s is also Copilot's documented default; writing it keeps the + bound explicit should that default move. + """ + return { + "type": "command", + "powershell" if IS_WINDOWS else "bash": command, + "timeoutSec": HOOK_TIMEOUT_SECONDS, + } def _copilot_hook_commands(entry: dict) -> Iterable[str]: diff --git a/tests/unit/test_guard.py b/tests/unit/test_guard.py index eaf422fe..3c143eb0 100644 --- a/tests/unit/test_guard.py +++ b/tests/unit/test_guard.py @@ -109,6 +109,19 @@ def logged_connectors(): ) +# Every synchronous guard hook is written with this explicit timeout (seconds). +HOOK_TIMEOUT = 30 + + +def _strip_timeouts(hooks: dict) -> None: + """Drop timeout keys in place, reproducing a config written before timeouts existed.""" + for entries in hooks.values(): + for item in entries: + for entry in item.get("hooks", [item]): + entry.pop("timeout", None) + entry.pop("timeoutSec", None) + + def _claude_group(command: str, matcher: str | None = None) -> dict: g: dict = {"hooks": [{"type": "command", "command": command}]} if matcher: @@ -855,7 +868,11 @@ def test_adds_flat_session_start_entry(self, tmp_path): discover_command=self.discover_command, ) - assert data["hooks"]["SessionStart"][1] == {"type": "command", COPILOT_SCRIPT_KEY: self.discover_command} + assert data["hooks"]["SessionStart"][1] == { + "type": "command", + COPILOT_SCRIPT_KEY: self.discover_command, + "timeoutSec": HOOK_TIMEOUT, + } def test_none_preserves_current_hook_shape(self, tmp_path): data, _, _ = _prepare_copilot_config( @@ -866,7 +883,8 @@ def test_none_preserves_current_hook_shape(self, tmp_path): assert all(len(data["hooks"][event]) == 1 for event in COPILOT_HOOK_EVENTS) assert all( - data["hooks"][event][0] == {"type": "command", COPILOT_SCRIPT_KEY: COPILOT_AGENT_SCAN_CMD} + data["hooks"][event][0] + == {"type": "command", COPILOT_SCRIPT_KEY: COPILOT_AGENT_SCAN_CMD, "timeoutSec": HOOK_TIMEOUT} for event in COPILOT_HOOK_EVENTS ) @@ -961,6 +979,122 @@ def test_uninstall_removes_discovery_entry(self, tmp_path): assert "hooks" not in json.loads(path.read_text()) +class TestSyncHookTimeout: + """Synchronous guard hooks carry an explicit timeout; fire-and-forget entries don't. + + Without it Claude Code and Codex wait up to 600 s for a hung forwarder on every + event, which freezes the agent whenever the hook request hangs instead of failing. + """ + + discover_command = ( + "PUSH_KEY='pk' REMOTE_HOOKS_BASE_URL='https://api.snyk.io' bash '/x/snyk-agent-guard-discover.sh'" + ) + + def test_constant_is_thirty_seconds(self): + assert guard_module.HOOK_TIMEOUT_SECONDS == HOOK_TIMEOUT + + def test_claude_guard_entries_carry_timeout_except_session_end(self, tmp_path): + with patch(f"{_G}.IS_WINDOWS", False): + settings, _, _ = _prepare_claude_config( + AGENT_SCAN_CMD, + tmp_path / "settings.json", + discover_command=self.discover_command, + ) + + for event in CLAUDE_HOOK_EVENTS: + entry = settings["hooks"][event][0]["hooks"][0] + if event == "SessionEnd": + # A per-hook timeout raises Claude Code's 1.5 s SessionEnd budget to match, + # which would delay every exit by the full timeout on a hung network. + assert entry == {"type": "command", "command": AGENT_SCAN_CMD} + else: + assert entry == {"type": "command", "command": AGENT_SCAN_CMD, "timeout": HOOK_TIMEOUT} + # Claude Code does not enforce ``timeout`` on an async hook, so none is written. + assert "timeout" not in settings["hooks"]["SessionStart"][1]["hooks"][0] + + def test_claude_windows_entry_keeps_shell_alongside_timeout(self, tmp_path): + with patch(f"{_G}.IS_WINDOWS", True): + settings, _, _ = _prepare_claude_config(AGENT_SCAN_CMD, tmp_path / "settings.json") + + assert settings["hooks"]["PreToolUse"][0]["hooks"][0] == { + "type": "command", + "command": AGENT_SCAN_CMD, + "shell": "powershell", + "timeout": HOOK_TIMEOUT, + } + + def test_codex_guard_entries_carry_timeout_on_every_event(self, tmp_path): + data, _, _ = _prepare_codex_config( + AGENT_SCAN_CMD, + tmp_path / "hooks.json", + discover_command=self.discover_command, + ) + + for event in CODEX_HOOK_EVENTS: + assert data["hooks"][event][0]["hooks"][0] == { + "type": "command", + "command": AGENT_SCAN_CMD, + "timeout": HOOK_TIMEOUT, + } + # The async discovery hook is not synchronous; it keeps its own internal caps. + assert "timeout" not in data["hooks"]["SessionStart"][1]["hooks"][0] + + def test_cursor_guard_entries_carry_timeout_except_fire_and_forget_events(self, tmp_path): + data, _, _ = _prepare_cursor_config( + AGENT_SCAN_CMD, + tmp_path / "hooks.json", + discover_command=self.discover_command, + ) + + for event in CURSOR_HOOK_EVENTS: + entry = data["hooks"][event][0] + if event in ("sessionStart", "sessionEnd"): + # Cursor documents these two as fire-and-forget: the agent loop never waits. + assert entry == {"command": AGENT_SCAN_CMD} + else: + assert entry == {"command": AGENT_SCAN_CMD, "timeout": HOOK_TIMEOUT} + assert data["hooks"]["sessionStart"][1] == {"command": self.discover_command} + + def test_copilot_entries_carry_timeout_sec_including_discovery(self, tmp_path): + data, _, _ = _prepare_copilot_config( + COPILOT_AGENT_SCAN_CMD, + tmp_path / "snyk-agent-guard.json", + discover_command=self.discover_command, + ) + + # Copilot has no async mode, so its discovery entry is synchronous too. 30 s is + # already Copilot's default; writing it keeps the bound explicit if that changes. + for event in COPILOT_HOOK_EVENTS: + assert data["hooks"][event][0]["timeoutSec"] == HOOK_TIMEOUT + assert data["hooks"]["SessionStart"][1]["timeoutSec"] == HOOK_TIMEOUT + + @pytest.mark.parametrize("client", ["claude", "cursor", "codex", "github-copilot"]) + def test_install_written_without_timeout_is_reported_modified(self, tmp_path, client): + """A pre-timeout install must show as modified on reinstall, not as up to date.""" + path = tmp_path / "cfg.json" + prepare = { + "claude": lambda p: _prepare_claude_config(AGENT_SCAN_CMD, p), + "cursor": lambda p: _prepare_cursor_config(AGENT_SCAN_CMD, p), + "codex": lambda p: _prepare_codex_config(AGENT_SCAN_CMD, p), + "github-copilot": lambda p: _prepare_copilot_config(COPILOT_AGENT_SCAN_CMD, p), + }[client] + expected_events = { + "claude": set(CLAUDE_HOOK_EVENTS) - {"SessionEnd"}, + "cursor": set(CURSOR_HOOK_EVENTS) - {"sessionStart", "sessionEnd"}, + "codex": set(CODEX_HOOK_EVENTS), + "github-copilot": set(COPILOT_HOOK_EVENTS), + }[client] + data, _, preserved = prepare(path) + _strip_timeouts(data["hooks"]) + _write_config(data, path, preserved) + + _, diff, _ = prepare(path) + + assert set(diff["modified"]) == expected_events + assert diff["added"] == {} + assert diff["removed"] == {} + + class TestWriteConfig: def test_unknown_client_rejected(self, tmp_path): with pytest.raises(ValueError, match="Unknown client: unknown"): @@ -1825,7 +1959,7 @@ def test_writes_bash_key_on_posix(self, tmp_path, monkeypatch): _setup_copilot_hooks(COPILOT_AGENT_SCAN_CMD, path) entry = json.loads(path.read_text())["hooks"]["Stop"][0] - assert entry == {"type": "command", "bash": COPILOT_AGENT_SCAN_CMD} + assert entry == {"type": "command", "bash": COPILOT_AGENT_SCAN_CMD, "timeoutSec": HOOK_TIMEOUT} def test_writes_powershell_key_on_windows(self, tmp_path, monkeypatch): monkeypatch.setattr(guard_module, "IS_WINDOWS", True) @@ -1833,7 +1967,7 @@ def test_writes_powershell_key_on_windows(self, tmp_path, monkeypatch): _setup_copilot_hooks(COPILOT_AGENT_SCAN_CMD, path) entry = json.loads(path.read_text())["hooks"]["Stop"][0] - assert entry == {"type": "command", "powershell": COPILOT_AGENT_SCAN_CMD} + assert entry == {"type": "command", "powershell": COPILOT_AGENT_SCAN_CMD, "timeoutSec": HOOK_TIMEOUT} @pytest.mark.parametrize("key", ["bash", "powershell", "command"]) def test_reinstall_replaces_an_existing_hook_under_any_script_key(self, tmp_path, key): @@ -2009,7 +2143,8 @@ def test_copilot_managed_config_has_the_documented_policy_shape(self, tmp_path): assert data["version"] == 1 assert set(data["hooks"]) == set(COPILOT_HOOK_EVENTS) assert all( - data["hooks"][event] == [{"type": "command", COPILOT_SCRIPT_KEY: COPILOT_AGENT_SCAN_CMD}] + data["hooks"][event] + == [{"type": "command", COPILOT_SCRIPT_KEY: COPILOT_AGENT_SCAN_CMD, "timeoutSec": HOOK_TIMEOUT}] for event in COPILOT_HOOK_EVENTS ) @@ -2951,6 +3086,7 @@ def test_render_none_preserves_existing_output(self, tmp_path): f"[[hooks.{event}.hooks]]", 'type = "command"', f'command = "{CODEX_AGENT_SCAN_CMD}"', + f"timeout = {HOOK_TIMEOUT}", "", ] ) @@ -3023,11 +3159,12 @@ def test_render_parse_round_trip_splits_guard_and_discovery_commands(self, tmp_p discover_command=CODEX_DISCOVER_CMD, ) - events, guard_command, discover_command = _parse_codex_requirements_toml(content) + events, guard_command, discover_command, guard_timeout = _parse_codex_requirements_toml(content) assert events == CODEX_HOOK_EVENTS assert guard_command == CODEX_AGENT_SCAN_CMD assert discover_command == CODEX_DISCOVER_CMD + assert guard_timeout == HOOK_TIMEOUT def test_render_parse_round_trip_preserves_control_characters(self, tmp_path): _, _, _, render = self._import_managed_helpers() @@ -3046,7 +3183,7 @@ def test_render_parse_round_trip_preserves_control_characters(self, tmp_path): discover_command=discover_command, ) - events, parsed_guard, parsed_discover = _parse_codex_requirements_toml(content) + events, parsed_guard, parsed_discover, _ = _parse_codex_requirements_toml(content) assert events == CODEX_HOOK_EVENTS assert parsed_guard == guard_command assert parsed_discover == discover_command @@ -3076,6 +3213,67 @@ def test_write_then_prepare_same_control_commands_has_no_modified_diff(self, tmp assert diff == {"added": {}, "modified": {}, "removed": {}} + def test_render_puts_timeout_in_guard_tables_only(self, tmp_path): + _, _, _, render = self._import_managed_helpers() + + content = render( + CODEX_AGENT_SCAN_CMD, + tmp_path / "requirements.toml", + discover_command=CODEX_DISCOVER_CMD, + ) + + assert content.count(f"timeout = {HOOK_TIMEOUT}") == len(CODEX_HOOK_EVENTS) + assert f'command = "{CODEX_AGENT_SCAN_CMD}"\ntimeout = {HOOK_TIMEOUT}\n' in content + assert f'command = "{CODEX_DISCOVER_CMD}"\nasync = true\n' in content + + def test_rendered_timeout_parses_as_an_integer(self, tmp_path): + tomllib = pytest.importorskip("tomllib") + _, _, _, render = self._import_managed_helpers() + + parsed = tomllib.loads( + render( + CODEX_AGENT_SCAN_CMD, + tmp_path / "requirements.toml", + discover_command=CODEX_DISCOVER_CMD, + ) + ) + + for event in CODEX_HOOK_EVENTS: + assert parsed["hooks"][event][0]["hooks"][0]["timeout"] == HOOK_TIMEOUT + assert "timeout" not in parsed["hooks"]["SessionStart"][1]["hooks"][0] + + def test_parse_reads_guard_timeout_back(self, tmp_path): + _, _, _, render = self._import_managed_helpers() + content = render( + CODEX_AGENT_SCAN_CMD, + tmp_path / "requirements.toml", + discover_command=CODEX_DISCOVER_CMD, + ) + + _, _, _, guard_timeout = _parse_codex_requirements_toml(content) + + assert guard_timeout == HOOK_TIMEOUT + + def test_prepare_marks_pre_timeout_install_as_modified(self, tmp_path): + path = tmp_path / "requirements.toml" + content, _ = _prepare_codex_managed_config( + CODEX_AGENT_SCAN_CMD, + path, + discover_command=CODEX_DISCOVER_CMD, + ) + path.write_text(content.replace(f"timeout = {HOOK_TIMEOUT}\n", "")) + + _, diff = _prepare_codex_managed_config( + CODEX_AGENT_SCAN_CMD, + path, + discover_command=CODEX_DISCOVER_CMD, + ) + + assert set(diff["modified"]) == set(CODEX_HOOK_EVENTS) + for event in CODEX_HOOK_EVENTS: + assert diff["modified"][event]["expected_value"][0]["timeout"] == HOOK_TIMEOUT + assert "timeout" not in diff["modified"][event]["actual_value"][0] + def test_install_writes_toml(self, tmp_path): install, _, _, _ = self._import_managed_helpers() path = tmp_path / "requirements.toml" @@ -3252,10 +3450,11 @@ def test_parse_backslash_path_no_unicode_escape(self): 'type = "command"\n' "command = \"PUSH_KEY='pk' bash 'C:\\\\Users\\\\me\\\\hooks\\\\snyk-agent-guard.sh' --client codex\"\n" ) - events, cmd, discover_cmd = _parse_codex_requirements_toml(toml) + events, cmd, discover_cmd, guard_timeout = _parse_codex_requirements_toml(toml) assert "PreToolUse" in events assert "C:\\Users\\me\\hooks\\snyk-agent-guard.sh" in cmd assert discover_cmd is None + assert guard_timeout is None def test_discovery_backslash_path_round_trips(self, tmp_path): _, _, _, render = self._import_managed_helpers() @@ -3269,7 +3468,7 @@ def test_discovery_backslash_path_round_trips(self, tmp_path): tmp_path / "requirements.toml", discover_command=discover_command, ) - _, _, parsed_discover = _parse_codex_requirements_toml(content) + _, _, parsed_discover, _ = _parse_codex_requirements_toml(content) assert parsed_discover == discover_command From 3cc7ec521348db75ab537e2fd1dc20e62a9f07ae Mon Sep 17 00:00:00 2001 From: iamcristi Date: Thu, 15 Oct 2026 11:00:15 +0200 Subject: [PATCH 2/2] chore: raise the agent-side guard hook cap 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. --- src/agent_scan/guard.py | 10 ++++++---- tests/unit/test_guard.py | 21 ++++++++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/agent_scan/guard.py b/src/agent_scan/guard.py index 530bf45c..bb225c07 100644 --- a/src/agent_scan/guard.py +++ b/src/agent_scan/guard.py @@ -68,8 +68,10 @@ # 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. -HOOK_TIMEOUT_SECONDS = 30 +# 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" @@ -1659,8 +1661,8 @@ def _copilot_command_entry(command: str) -> dict: """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. 30 s is also Copilot's documented default; writing it keeps the - bound explicit should that default move. + 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", diff --git a/tests/unit/test_guard.py b/tests/unit/test_guard.py index 3c143eb0..91b795fe 100644 --- a/tests/unit/test_guard.py +++ b/tests/unit/test_guard.py @@ -110,7 +110,9 @@ def logged_connectors(): # Every synchronous guard hook is written with this explicit timeout (seconds). -HOOK_TIMEOUT = 30 +HOOK_TIMEOUT = 65 +# The forwarder scripts' own request budget, which the agent-side cap must outlast. +_FORWARDER_BUDGET_RE = re.compile(rb"^\$?TIMEOUT_SECONDS\s*=\s*(\d+)(?=\r?$)", re.MULTILINE) def _strip_timeouts(hooks: dict) -> None: @@ -990,9 +992,18 @@ class TestSyncHookTimeout: "PUSH_KEY='pk' REMOTE_HOOKS_BASE_URL='https://api.snyk.io' bash '/x/snyk-agent-guard-discover.sh'" ) - def test_constant_is_thirty_seconds(self): + def test_constant_is_sixty_five_seconds(self): assert guard_module.HOOK_TIMEOUT_SECONDS == HOOK_TIMEOUT + @pytest.mark.parametrize("name", ["snyk-agent-guard.sh", "snyk-agent-guard.ps1"]) + def test_cap_outlasts_the_forwarder_request_budget(self, name): + """The forwarder fails open inside its own request budget, with a diagnostic. The + agent's cap must not fire first, or the user never sees it.""" + match = _FORWARDER_BUDGET_RE.search(_get_script_path(name).read_bytes()) + if match is None: + pytest.skip(f"{name} has no TIMEOUT_SECONDS on this branch") + assert int(match.group(1)) < guard_module.HOOK_TIMEOUT_SECONDS + def test_claude_guard_entries_carry_timeout_except_session_end(self, tmp_path): with patch(f"{_G}.IS_WINDOWS", False): settings, _, _ = _prepare_claude_config( @@ -1062,8 +1073,8 @@ def test_copilot_entries_carry_timeout_sec_including_discovery(self, tmp_path): discover_command=self.discover_command, ) - # Copilot has no async mode, so its discovery entry is synchronous too. 30 s is - # already Copilot's default; writing it keeps the bound explicit if that changes. + # Copilot has no async mode, so its discovery entry is synchronous too, and its 30 s + # default would kill the forwarder before the script's own request budget fires. for event in COPILOT_HOOK_EVENTS: assert data["hooks"][event][0]["timeoutSec"] == HOOK_TIMEOUT assert data["hooks"]["SessionStart"][1]["timeoutSec"] == HOOK_TIMEOUT @@ -6696,7 +6707,7 @@ def test_new_empty_old_has_guard_hooks(self): def test_nested_value_difference_is_modified(self): old_val = [{"hooks": [{"type": "command", "command": "PUSH_KEY='x' bash snyk-agent-guard.sh", "timeout": 10}]}] - new_val = [{"hooks": [{"type": "command", "command": "PUSH_KEY='x' bash snyk-agent-guard.sh", "timeout": 30}]}] + new_val = [{"hooks": [{"type": "command", "command": "PUSH_KEY='x' bash snyk-agent-guard.sh", "timeout": 65}]}] result = _compute_hooks_diff({"PreToolUse": old_val}, {"PreToolUse": new_val}) assert "PreToolUse" in result["modified"] assert result["modified"]["PreToolUse"]["expected_value"] == new_val