From 0dd0f6e648a6f525b679f4afb7a545e40d8d211e Mon Sep 17 00:00:00 2001 From: RelaxJonh <92573950+RelaxJonh@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:44:31 +0700 Subject: [PATCH 1/3] fix(agent): clamp authorization gate lock timeout to prevent OverflowError on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _ConcurrentToolAuthorizationGate uses threading.Lock.acquire(timeout=...) where timeout comes from human_wait_ceiling() (approvals.timeout + 60s). When approvals.timeout is set very large (e.g. 999999999999 to effectively disable timeouts), this overflows macOS timespec and raises: OverflowError: timestamp out of range for platform time_t The gate only needs to serialize parallel dispatch — it should never wait longer than the wedged-holder bound (_AUTHORIZATION_GATE_LOCK_TIMEOUT_S = 360s). Clamp the return value with min() so unbounded approvals.timeout cannot overflow the lock acquire. Fixes #83220 --- agent/tool_executor.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 69bcca6075..b4f3baed7e 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -154,7 +154,12 @@ def _authorization_gate_lock_timeout() -> float: try: from tools.approval import human_wait_ceiling - return human_wait_ceiling() + # Clamp to _AUTHORIZATION_GATE_LOCK_TIMEOUT_S: the gate serializes + # parallel dispatch — it should never wait longer than a wedged-holder + # timeout even when approvals.timeout is set very large (or infinite). + # On macOS, unbounded values overflow threading.Lock.acquire's timespec + # and raise OverflowError (#83220). + return min(human_wait_ceiling(), _AUTHORIZATION_GATE_LOCK_TIMEOUT_S) except Exception: return _AUTHORIZATION_GATE_LOCK_TIMEOUT_S From 4341cf1df7f6397dfb0fc0899bbe537b1d7eb314 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 15 Aug 2026 03:45:18 +0530 Subject: [PATCH 2/3] fix(approval): clamp approvals.timeout at the config-read chokepoint (#83220) Widen the salvaged gate-site clamp to the bug class. The gate min() from the contributor PR capped the gate bound at 360s, which would break the #79719 contract (gate must extend while a legitimate >360s approval prompt is answerable) and left the sibling overflow sites live: the CLI prompt thread.join, the gateway poll deadline, and human_wait_ceiling all consume the same config value. Clamp once in _get_approval_timeout() via agent.deadline.MAX_SAFE_TIMEOUT_S (1 year - semantically unbounded, platform-safe). The gate keeps its approvals.timeout-tracking behavior above 360s; 7 regression tests pin lock-acquire/thread-join safety and the gate-extension contract. Bilateral E2E: with approvals.timeout=1e20 in a real config.yaml, main crashes every consumer with OverflowError; this branch survives all 5 probes. --- agent/tool_executor.py | 13 +-- tests/tools/test_approval_timeout_overflow.py | 96 +++++++++++++++++++ tools/approval.py | 21 +++- 3 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 tests/tools/test_approval_timeout_overflow.py diff --git a/agent/tool_executor.py b/agent/tool_executor.py index b4f3baed7e..fde07af321 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -154,12 +154,13 @@ def _authorization_gate_lock_timeout() -> float: try: from tools.approval import human_wait_ceiling - # Clamp to _AUTHORIZATION_GATE_LOCK_TIMEOUT_S: the gate serializes - # parallel dispatch — it should never wait longer than a wedged-holder - # timeout even when approvals.timeout is set very large (or infinite). - # On macOS, unbounded values overflow threading.Lock.acquire's timespec - # and raise OverflowError (#83220). - return min(human_wait_ceiling(), _AUTHORIZATION_GATE_LOCK_TIMEOUT_S) + # human_wait_ceiling is platform-safety-capped (agent/deadline.py + # MAX_SAFE_TIMEOUT_S): a huge approvals.timeout can no longer overflow + # Lock.acquire's time_t on macOS (#83220). Deliberately NOT min()'d + # with _AUTHORIZATION_GATE_LOCK_TIMEOUT_S — the gate must never give + # up while a legitimate approval prompt is still answerable (#79719), + # so a configured approvals.timeout above 360s must extend the gate. + return human_wait_ceiling() except Exception: return _AUTHORIZATION_GATE_LOCK_TIMEOUT_S diff --git a/tests/tools/test_approval_timeout_overflow.py b/tests/tools/test_approval_timeout_overflow.py new file mode 100644 index 0000000000..44759f6d07 --- /dev/null +++ b/tests/tools/test_approval_timeout_overflow.py @@ -0,0 +1,96 @@ +"""Regression tests for #83220: oversized approvals.timeout must never +overflow platform wait primitives (macOS time_t OverflowError). + +The clamp lives at the single config-read site (_get_approval_timeout), so +every consumer — CLI prompt thread.join, gateway poll deadline, human-wait +ceiling, and the tool_executor authorization gate — is covered at once. +""" + +from __future__ import annotations + +import threading + +from unittest.mock import patch + +from agent.deadline import MAX_SAFE_TIMEOUT_S + + +def _with_configured_timeout(value): + return patch( + "tools.approval._get_approval_config", + return_value={"timeout": value}, + ) + + +class TestApprovalTimeoutOverflowClamp: + def test_normal_value_passes_through(self): + from tools.approval import _get_approval_timeout + + with _with_configured_timeout(300): + assert _get_approval_timeout() == 300 + + def test_oversized_value_clamped(self): + from tools.approval import _get_approval_timeout + + with _with_configured_timeout(10**18): + assert _get_approval_timeout() == int(MAX_SAFE_TIMEOUT_S) + + def test_invalid_value_falls_back_to_default(self): + from tools.approval import _get_approval_timeout + + with _with_configured_timeout("soon"): + assert _get_approval_timeout() == 300 + + def test_clamped_value_safe_for_lock_acquire(self): + # The exact primitive that crashed in #83220: Lock.acquire on macOS + # converts the relative timeout to an absolute time_t timestamp. + from tools.approval import _get_approval_timeout + + with _with_configured_timeout(10**18): + timeout = _get_approval_timeout() + lock = threading.Lock() + assert lock.acquire(timeout=timeout) # would raise OverflowError unclamped + lock.release() + + def test_clamped_value_safe_for_thread_join(self): + # Sibling crash site: the CLI prompt fallback joins the input thread + # with the configured timeout (tools/approval.py get_input path). + from tools.approval import _get_approval_timeout + + with _with_configured_timeout(10**18): + timeout = _get_approval_timeout() + t = threading.Thread(target=lambda: None) + t.start() + t.join(timeout=timeout) # would raise OverflowError unclamped + assert not t.is_alive() + + def test_human_wait_ceiling_inherits_clamp(self): + from tools.approval import HUMAN_WAIT_MARGIN_S, human_wait_ceiling + + with _with_configured_timeout(10**18): + ceiling = human_wait_ceiling() + assert ceiling == float(int(MAX_SAFE_TIMEOUT_S)) + HUMAN_WAIT_MARGIN_S + lock = threading.Lock() + assert lock.acquire(timeout=ceiling) + lock.release() + + def test_authorization_gate_timeout_safe_and_extends_with_config(self): + # The gate bound must (a) be platform-safe with an oversized config + # and (b) still EXTEND beyond the 360s fallback when approvals.timeout + # is legitimately larger — clamping it down to the fallback would + # break serialization while a real prompt is still answerable (#79719). + from agent.tool_executor import ( + _AUTHORIZATION_GATE_LOCK_TIMEOUT_S, + _authorization_gate_lock_timeout, + ) + + with _with_configured_timeout(10**18): + bound = _authorization_gate_lock_timeout() + lock = threading.Lock() + assert lock.acquire(timeout=bound) + lock.release() + + with _with_configured_timeout(3600): + bound = _authorization_gate_lock_timeout() + assert bound > _AUTHORIZATION_GATE_LOCK_TIMEOUT_S + assert bound == 3600 + 60.0 # approvals.timeout + HUMAN_WAIT_MARGIN_S diff --git a/tools/approval.py b/tools/approval.py index 775aa85972..b8fbd2988a 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -2588,6 +2588,11 @@ def human_wait_ceiling() -> float: authorization gate's serialization-lock acquire, so the two bounds cannot drift. Never call while holding ``_human_wait_lock`` — it reads the config cache. + + Platform safety: ``_get_approval_timeout`` caps at + ``agent.deadline.MAX_SAFE_TIMEOUT_S``, so this value is always safe to + hand to ``Lock.acquire(timeout=...)`` / ``Thread.join(timeout=...)`` + (#83220 macOS time_t overflow). """ return float(_get_approval_timeout()) + HUMAN_WAIT_MARGIN_S @@ -3430,11 +3435,25 @@ def _get_approval_timeout() -> int: approvals arrive as push notifications the user may not see for a couple of minutes; 60s proved too tight in practice (Telegram taps landed after the wait had already failed closed). + + Clamped to ``agent.deadline.MAX_SAFE_TIMEOUT_S`` (1 year — semantically + unbounded): a very large configured value overflows ``time_t`` inside + ``Thread.join(timeout=...)`` / ``Lock.acquire(timeout=...)`` on macOS, + and before this clamp a single oversized ``approvals.timeout`` crashed + every parallel tool batch with OverflowError (#83220). Clamping at the + single config-read site keeps every consumer (prompt join, gateway poll + deadline, human-wait ceiling, authorization gate) platform-safe at once. """ try: - return int(_get_approval_config().get("timeout", 300)) + raw = int(_get_approval_config().get("timeout", 300)) except (ValueError, TypeError): return 300 + try: + from agent.deadline import MAX_SAFE_TIMEOUT_S + + return min(raw, int(MAX_SAFE_TIMEOUT_S)) + except Exception: + return raw def _get_cron_approval_mode() -> str: From 446423d96e13d8a3bff748b9f24a538a0530ccdc Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:17:28 +0530 Subject: [PATCH 3/3] fix(approval): fail closed when the deadline import is unavailable; log clamp engagement Review round (#86412): - the except-import fallback returned the RAW oversized value, re-opening the exact macOS time_t overflow this fix prevents; it now fails closed to a finite ~1-year cap matching agent.deadline.MAX_SAFE_TIMEOUT_S - clamp engagement logs a WARNING so operators see the semantic change - new tests: float-form oversized value (YAML 1e18), warning emission, and the import-failure fail-closed path (blocked-import probe proving the result stays Lock.acquire-safe) --- ...2573950+RelaxJonh@users.noreply.github.com | 1 + tests/tools/test_approval_timeout_overflow.py | 42 +++++++++++++++++++ tools/approval.py | 16 ++++++- 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 contributors/emails/92573950+RelaxJonh@users.noreply.github.com diff --git a/contributors/emails/92573950+RelaxJonh@users.noreply.github.com b/contributors/emails/92573950+RelaxJonh@users.noreply.github.com new file mode 100644 index 0000000000..62ba5967ab --- /dev/null +++ b/contributors/emails/92573950+RelaxJonh@users.noreply.github.com @@ -0,0 +1 @@ +RelaxJonh diff --git a/tests/tools/test_approval_timeout_overflow.py b/tests/tools/test_approval_timeout_overflow.py index 44759f6d07..18d7105880 100644 --- a/tests/tools/test_approval_timeout_overflow.py +++ b/tests/tools/test_approval_timeout_overflow.py @@ -41,6 +41,48 @@ class TestApprovalTimeoutOverflowClamp: with _with_configured_timeout("soon"): assert _get_approval_timeout() == 300 + def test_oversized_float_value_clamped(self): + # YAML `1e18` arrives as a float, not an int — different int() path + # than the string/int forms; the clamp must cover it too. + from tools.approval import _get_approval_timeout + + with _with_configured_timeout(1e18): + assert _get_approval_timeout() == int(MAX_SAFE_TIMEOUT_S) + + def test_clamp_engagement_logs_warning(self, caplog): + # Capping silently changes behavior for every consumer; operators + # must see it happen. + import tools.approval as approval_mod + + with _with_configured_timeout(10**18): + with caplog.at_level("WARNING", logger=approval_mod.__name__): + approval_mod._get_approval_timeout() + assert "exceeds the platform-safe maximum" in caplog.text + + def test_deadline_import_failure_fails_closed(self, monkeypatch): + # If agent.deadline ever fails to import, the clamp must fail CLOSED + # (a finite safe cap) — returning the raw value would re-open the + # exact time_t overflow this fix exists to prevent. + import builtins + + from tools.approval import _get_approval_timeout + + real_import = builtins.__import__ + + def _blocked(name, *args, **kwargs): + if name == "agent.deadline" or name.startswith("agent.deadline."): + raise ImportError("simulated packaging failure") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", _blocked) + with _with_configured_timeout(10**18): + value = _get_approval_timeout() + assert value == 365 * 24 * 3600 + # Still platform-safe for the crashing primitive. + lock = threading.Lock() + assert lock.acquire(timeout=value) + lock.release() + def test_clamped_value_safe_for_lock_acquire(self): # The exact primitive that crashed in #83220: Lock.acquire on macOS # converts the relative timeout to an absolute time_t timestamp. diff --git a/tools/approval.py b/tools/approval.py index b8fbd2988a..ea6a1b558e 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -3451,9 +3451,21 @@ def _get_approval_timeout() -> int: try: from agent.deadline import MAX_SAFE_TIMEOUT_S - return min(raw, int(MAX_SAFE_TIMEOUT_S)) + safe_cap = int(MAX_SAFE_TIMEOUT_S) except Exception: - return raw + # Fail CLOSED: returning the raw value here would re-open the exact + # time_t overflow this clamp exists to prevent. ~1 year, matching + # agent.deadline.MAX_SAFE_TIMEOUT_S. + safe_cap = 365 * 24 * 3600 + if raw > safe_cap: + logger.warning( + "approvals.timeout=%s exceeds the platform-safe maximum; " + "clamping to %ss", + raw, + safe_cap, + ) + return safe_cap + return raw def _get_cron_approval_mode() -> str: