From 2df508037d8e5546389197bbf65463662833d61f Mon Sep 17 00:00:00 2001 From: alexeybe1kin <210597588+alexeybe1kin@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:27:08 +0200 Subject: [PATCH] fix: every pending Inbox card can be rejected or dismissed Rejecting or dismissing grants nothing, so it no longer depends on the card being reviewable or its origin being intact. A card whose arguments, origin or publication cannot be shown used to stay in the Inbox for good (hunt toolgate-2 F14). Approving still needs a reviewable card with an intact origin. --- toolgate/core/approvals.py | 34 ++++++++++++++-------------- toolgate/core/owner_channel.py | 8 ++++--- toolgate/tests/test_owner_channel.py | 13 ++++++++--- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/toolgate/core/approvals.py b/toolgate/core/approvals.py index 3d39679..a755edc 100644 --- a/toolgate/core/approvals.py +++ b/toolgate/core/approvals.py @@ -318,25 +318,25 @@ def decide_request(request_id: str, status: str, actor: str, note: str = "", *, if record.get("status") != "pending": raise ValueError(f"request is already {record.get('status')}") decision: dict[str, object] = {"actor": actor, "note": note, "at": _now()} - if record.get("kind") == "verification": + if record.get("kind") == "verification" and status == "approved": + # Only a yes needs an intact origin; a no or a dismissal closes any pending card. if not _verification_origin_valid(conn, record): raise ValueError("Approval was not issued intact by ToolGate; request fresh confirmation") - if status == "approved": - binding = record.get("payload", {}).get("binding", {}) - if "expires_at" not in binding: - raise ValueError("Approval expiry is invalid; request fresh confirmation") - if binding["expires_at"] is None: - # The window in force now stays with this approval; a later change never - # moves it. - from toolgate.core import tool_settings - decision["run_window_minutes"] = tool_settings.run_window(conn) - else: - try: - expires_at = datetime.fromisoformat(binding["expires_at"]) - except (TypeError, ValueError) as exc: - raise ValueError("Approval expiry is invalid; request fresh confirmation") from exc - if expires_at.tzinfo is None or expires_at <= datetime.now(UTC): - raise ValueError("Approval has expired; request fresh confirmation") + binding = record.get("payload", {}).get("binding", {}) + if "expires_at" not in binding: + raise ValueError("Approval expiry is invalid; request fresh confirmation") + if binding["expires_at"] is None: + # The window in force now stays with this approval; a later change never + # moves it. + from toolgate.core import tool_settings + decision["run_window_minutes"] = tool_settings.run_window(conn) + else: + try: + expires_at = datetime.fromisoformat(binding["expires_at"]) + except (TypeError, ValueError) as exc: + raise ValueError("Approval expiry is invalid; request fresh confirmation") from exc + if expires_at.tzinfo is None or expires_at <= datetime.now(UTC): + raise ValueError("Approval has expired; request fresh confirmation") record["status"] = status record["decision"] = decision _save_request(conn, record) diff --git a/toolgate/core/owner_channel.py b/toolgate/core/owner_channel.py index 036adb9..4d83cac 100644 --- a/toolgate/core/owner_channel.py +++ b/toolgate/core/owner_channel.py @@ -223,10 +223,12 @@ def guard(db, record): if record.get("kind") != "verification": raise OwnerError("not_found", "Owner request not found.", 404) _gone(record) + if status != "approved": + # Saying no grants nothing, so every pending card can be closed, even one whose + # arguments, origin or publication cannot be shown; it never stays in the Inbox. + return view = project(db, record) - permitted = (view["reviewable"] if status == "approved" else - view["unavailable_reason"] in {None, "expired", "unsupported_subject", "publication_unavailable"}) - if not view["approval"]["origin_valid"] or not permitted: + if not view["approval"]["origin_valid"] or not view["reviewable"]: raise OwnerError("not_reviewable", "This exact action cannot be approved from this view.") try: record = approvals.decide_request(identity, status, "gateway-owner", note, guard=guard) diff --git a/toolgate/tests/test_owner_channel.py b/toolgate/tests/test_owner_channel.py index 324cbae..d31df6c 100644 --- a/toolgate/tests/test_owner_channel.py +++ b/toolgate/tests/test_owner_channel.py @@ -86,14 +86,16 @@ def test_fixed_projection_has_full_args_but_no_nonce_originating_key_or_unknown_ @pytest.mark.parametrize("args", [{"text": "x" * 32768}, {str(i): i for i in range(1100)}, {"invalid": float("nan")}, {"n": 9007199254740993}, {"nested": [{"n": -9007199254740993}]}]) -def test_withheld_arguments_never_authorize_a_decision(gate, args): +@pytest.mark.parametrize("no", ["rejected", "dismissed"]) +def test_withheld_arguments_never_approve_but_the_card_can_be_closed(gate, args, no): request = mint(gate, args) view = detail(gate, request["id"]).json() assert view["action"]["args"] is None and not view["reviewable"] assert view["unavailable_reason"] == "arguments_unavailable" - for status in ("approved", "rejected", "dismissed"): - assert decide(gate, request["id"], status).status_code == 409 + assert decide(gate, request["id"], "approved").status_code == 409 assert cp.get("request", request["id"])["status"] == "pending" + assert decide(gate, request["id"], no).status_code == 200 + assert cp.get("request", request["id"])["status"] == no def test_origin_tampering_between_review_and_decision_fails_closed(gate): @@ -108,6 +110,9 @@ def test_origin_tampering_between_review_and_decision_fails_closed(gate): view = detail(gate, request["id"]).json() assert view["unavailable_reason"] == "invalid_origin" and view["action"]["args"] is None assert cp.get("request", request["id"])["status"] == "pending" + # Saying no grants nothing, so a card with a broken origin still leaves the Inbox. + assert decide(gate, request["id"], "rejected").status_code == 200 + assert cp.get("request", request["id"])["status"] == "rejected" def test_legacy_malformed_binding_is_read_only_not_server_error(gate): @@ -116,6 +121,8 @@ def test_legacy_malformed_binding_is_read_only_not_server_error(gate): view = detail(gate, record["id"]) assert view.status_code == 200 and view.json()["unavailable_reason"] == "invalid_origin" assert decide(gate, record["id"]).status_code == 409 + assert decide(gate, record["id"], "dismissed").status_code == 200 + assert cp.get("request", record["id"])["status"] == "dismissed" def test_expired_pending_remains_pending_but_can_only_be_rejected_or_dismissed(gate, monkeypatch):