jinghui/hermes-webui
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Release — approval no-run mirror masking fix (#7086) (#7125)

Browse Source 
* fix(approval): stamp mirror token when creating no-run legacy mirror (#6008 follow-up)

In in-process (legacy) mode, a guarded command parks an _ApprovalEntry in
_gateway_queues whose .data has no run_id. submit_gateway_pending_mirror()
mirrors it into _pending for the UI in the `elif not exact_local_entry`
branch, but that branch never stamped _GATEWAY_MIRROR_TOKEN on the mirror.

_resolve_approval_legacy() binds a no-run mirror back to the parked
_ApprovalEntry via a token match (pending_token == gateway_token). Without
the mirror token the match fails, resolve_gateway_pending_local() is never
called and entry.event.set() never fires, so the agent thread stays blocked
even though the endpoint returns ok:true and the UI closes the card. Only a
~1.5s reconcile fallback re-creates the card with a token -> the FIRST click
on "Allow once" silently does nothing.

Stamp the token from the live gateway head at mirror-creation time, matching
the exact-local branch, so the first click unblocks the agent.

Adds a regression test reproducing the scenario.

* fix(approval): resolve non-head no-run mirrors by scanning all live producers

The first-click fix stamps the mirror token from the mirror's own producer.
But _resolve_approval_legacy() previously matched that token only against the
queue head (`_gateway_queues[sid][0]`), so a non-head mirror (multiple parked
producers, #7093) could not be resolved: the head's token differs from the
selected producer's, the match failed, and the agent thread stayed blocked.

Scan every live producer in _gateway_queues for a matching
_webui_mirror_token or approval_id, and stamp the resolved producer's
approval_id before delegating to resolve_gateway_pending_local(). Registered
producers keep head/tail ordering, so exact-producer isolation is preserved.

Adds tests/test_issue_legacy_first_click_repro.py::test_legacy_non_head_
producer_unblocks_only_its_producer, which reproduces the non-head scenario.

* fix(approval): fail closed on no-run mirror token binding (818fd2fd follow-up)

submit_gateway_pending_mirror() previously fell back to "first unclaimed
token" or the queue head when a no-run mirror's identity (request_id/
approval_id) didn't match any live producer. That let approving a
stale/foreign mirror silently resolve a different, unrelated live
producer than the one the user actually saw.

Bind the token only on an explicit identity match; otherwise leave the
mirror tokenless so it can't borrow another producer's slot. A tokenless
orphan is still created automatically for the legitimate #7093 case
(no live producer at all) via reconcile_gateway_pending_mirror_locked.

Adds two adversarial regression tests (stale request_id vs. a live
sibling, and an identity-less mirror with multiple producers parked) and
fixes an unrelated test that omitted request_id, unlike every real
_ApprovalEntry.

* fix(approval): don't let an unmatched no-run mirror mask a live producer

Follow-up to 63087a40. Failing closed on token binding stopped stale
mirror A from executing live producer B, but left A parked in _pending:
reconcile then deferred to the pre-existing no-run mirror and skipped
tokenizing B, so B never surfaced as the head. The real pending approval
became unactionable (409 in gateway mode, ok:true resolving nothing in
local mode) while an unresolvable card sat in front of it.

reconcile_gateway_pending_mirror_locked() now always tokenizes every live
no-run producer and derives live_token from the authoritative head
whenever _gateway_queues holds producers, so an unmatched tokenless
mirror is discarded instead of suppressing the real one. Retained
missing-producer mirrors survive only while no producer is live, keeping
the #7093 tokenless-orphan boundary intact.

Tests assert the displayed == resolved invariant: the authoritative
producer is the head, responding with the stale/identity-less id fails
without waking anything, and approving the displayed entry wakes exactly
its own producer. Three test_gateway_approval_runs_api.py cases that
encoded the old masking behavior are updated to the new invariant.

* docs(approval): correct reconcile comment on which mirrors survive

The comment added in 9c2ca104 claimed only the authoritative head's mirror
may survive while a producer is live. That contradicts the code it
documents and the non-head regression test: the `entry_token in
live_local_tokens` branch keeps a mirror bound to ANY live no-run
producer, which is exactly what makes a non-head mirror resolvable
(test_legacy_non_head_producer_unblocks_only_its_producer).

State the actual rule instead — a mirror survives only if bound to some
live producer's own token, the head's via live_token or a non-head
producer's via live_local_tokens — so unmatched and tokenless copies are
discarded without implying non-head mirrors are dropped too. Comment
only; no behavior change.

* Release — approval no-run mirror masking fix (#7086)

---------

Co-authored-by: VHSgunzo <wosganzo@gmail.com>
Co-authored-by: n <a@n>
This commit is contained in:
nesquena-hermesVHSgunzon
 authored and GitHub committed 2026-08-18 13:11:07 +00:00
1 parent cced4f3509
commit 59023b784c
5 files changed
+503 -28
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

CHANGELOG.md
+1
View File
@@ -5,6 +5,7 @@
### Fixed
- **A parked no-run approval can no longer mask the real pending approval when a gateway producer is live.** No-run "legacy" approval mirrors are now stamped with a token derived from their own live producer, and whenever a session's gateway queue has live producers the reconciler tokenizes from the authoritative head and discards any unmatched or tokenless mirror instead of letting it linger — so a stale card can't sit in front of the genuine pending approval (previously gateway mode returned 409 and permanently retained the stale mirror, and local mode reported success while resolving nothing). Tokenless-orphan retention is preserved for the no-producer case, and responding to a stale id fails without waking a different producer. Thanks @VHSgunzo. (#7086)
- **Toggling YOLO from an approval card no longer hangs the gateway run or leaks across a session switch.** The approval card's YOLO branch now takes the same single `gateway_yolo_handoff(sid)` as the Runs dispatcher (revalidating the exact mirror after acquisition and holding it through settlement, with no nested-lock path), and the client captures one immutable owner tuple (card-owner session, approval id, load generation) at click time — failing closed without a POST when the visible card is not the active session, and fencing every post-response mutation (card hide, pending clears, stale-clear requery, controls, toast, status) on that exact tuple so a stale card can't project its result into the successor session. Thanks @dss539. (#6945)
- **Git operations no longer flash a console window on Windows.** Every git subprocess boundary (agent runtime, rollback, updates, workspace, worktrees) now routes through a shared `windows_hide_flags()` helper that passes `CREATE_NO_WINDOW` on Win32 and stays a no-op on POSIX, so short-lived git commands no longer pop a visible console window on Windows while keeping captured stdout/stderr intact. Thanks @ya1yi2fa3. (#6534)
api/route_approvals.py
+88 -11
View File
@@ -240,20 +240,26 @@ def reconcile_gateway_pending_mirror_locked(session_key: str) -> tuple[dict | No
live_head_entry = live_gateway_queue[0] if live_gateway_queue else None
live_head_data = getattr(live_head_entry, "data", None) or {}
has_no_run_mirror = any(
_is_gateway_mirror_entry(entry)
and not str(entry.get("run_id") or "").strip()
for entry in queue_list
)
live_run_id = str(live_head_data.get("run_id") or "").strip()
live_has_token = bool(live_head_data.get(_GATEWAY_ENTRY_DATA_TOKEN_KEY))
# Tokenize EVERY live no-run producer, and derive `live_token` from the
# authoritative head, whenever `_gateway_queues[session_key]` has live
# producers. This deliberately does NOT defer to a pre-existing no-run
# mirror: an unmatched/tokenless mirror A (which the fail-closed binding in
# submit_gateway_pending_mirror leaves deliberately unbound) must never
# suppress the live producer's token, or A masks the real pending approval
# B — B never surfaces as the head and can't be actioned, while responding
# to A resolves nothing. While any producer is live, a mirror survives only
# if it is bound to some live producer's own token — the head's via
# `live_token`, a non-head producer's via `live_local_tokens` (which is what
# keeps a non-head mirror resolvable) — so unmatched and tokenless copies
# are discarded instead of masking a real one. Tokenless-orphan retention is
# reserved for the genuine no-producer case (#7093), which lands here with an
# empty `live_gateway_queue` and therefore a `None` `live_token` anyway.
live_local_tokens: set[str] = set()
for live_entry in live_gateway_queue:
live_data = getattr(live_entry, "data", None) or {}
if str(live_data.get("run_id") or "").strip():
continue
live_entry_token = str(live_data.get(_GATEWAY_ENTRY_DATA_TOKEN_KEY) or "").strip()
if live_entry_token or not has_no_run_mirror:
live_entry_token = _gateway_mirror_entry_token(live_entry) or ""
if live_entry_token:
live_local_tokens.add(live_entry_token)
@@ -262,7 +268,6 @@ def reconcile_gateway_pending_mirror_locked(session_key: str) -> tuple[dict | No
live_token = (
_gateway_mirror_entry_token(live_head_entry)
if live_head_entry and live_head_data
and (live_run_id or live_has_token or not has_no_run_mirror)
else None
)
if live_token and live_run_id and not str(live_head_data.get("approval_id") or "").strip():
@@ -314,7 +319,14 @@ def reconcile_gateway_pending_mirror_locked(session_key: str) -> tuple[dict | No
changed = True
continue
if entry.get(_GATEWAY_MIRROR_RETAINED):
# A retained mirror is one whose own producer had already vanished when
# the user responded (the missing-producer 409 kept visible until an
# explicit teardown). It survives ONLY while no producer is live: once
# `_gateway_queues[session_key]` holds a real producer again, an
# unresolvable retained mirror must not mask it, so fall through to the
# normal matching below (which keeps it if it still matches a live
# token and discards it otherwise).
if entry.get(_GATEWAY_MIRROR_RETAINED) and not live_gateway_queue:
rebuilt.append(entry)
continue
@@ -557,7 +569,30 @@ def _gateway_mirrored_pending_run_id(session_key: str, approval_id: str) -> str
def submit_gateway_pending_mirror(session_key: str, approval: dict) -> tuple[dict | None, int]:
"""Mirror the live gateway head into WebUI polling state under a typed tag."""
"""Mirror the live gateway head into WebUI polling state under a typed tag.
Every mirrored entry describes one pending approval to the UI. Run-backed
mirrors carry ``run_id`` (remote gateway runs) and are bound to the parked
``_ApprovalEntry`` via ``approval_id``. No-run mirrors, which represent an
in-process (legacy) approval parked in ``_gateway_queues``, have no
``run_id`` and instead bind back to the live entry through
``_GATEWAY_MIRROR_TOKEN``: ``_resolve_approval_legacy()`` matches
``pending[_GATEWAY_MIRROR_TOKEN]`` against the live entry's
``_webui_mirror_token`` before it will call ``resolve_gateway_pending_local()``
to unblock the agent thread. A no-run mirror's token is stamped from its
OWN live producer — resolved via the mirror's ``request_id``/
``approval_id`` — and from nothing else. If no live producer's identity
matches, the mirror is left tokenless (fail closed): guessing ownership
from "first unclaimed token" or the queue head would let approving THIS
(possibly stale/foreign) mirror resolve a DIFFERENT live producer than
the one the user actually saw, which is an approval-integrity violation,
not a convenience. A tokenless mirror is only wired up automatically when
there is truly no live producer at all (#7093); ``reconcile_gateway_
pending_mirror_locked`` binds a fresh, correctly-bound mirror to the
authoritative live head on its own. Without a token, THIS specific click
returns ``ok:true`` and the card clears without unblocking any producer —
the correct producer's own card reappears on the next reconcile.
"""
with _lock:
run_id = str(approval.get("run_id") or "").strip()
approval_id = str(approval.get("approval_id") or "").strip()
@@ -683,9 +718,51 @@ def submit_gateway_pending_mirror(session_key: str, approval: dict) -> tuple[dic
if no_run_mirror:
approval["approval_id"] = str(no_run_mirror.get("approval_id") or approval_id).strip()
elif not _gateway_pending_mirror_locked(session_key, approval_id=approval_id):
# Stamp the mirror token from the mirror's OWN live producer so
# the first respond can link this mirror to the right
# _ApprovalEntry in _gateway_queues. Without it,
# _resolve_approval_legacy cannot match the no-run mirror to its
# gateway entry (both token fields are empty) and the agent
# thread is never unblocked on the first click (#6008 legacy).
# We MUST NOT blindly take the live head: for a non-head mirror
# (multiple parked producers, #7093 exact-producer isolation)
# the head belongs to a sibling, and stamping its token would
# bind the mirror to the wrong entry. Only an explicit
# request_id/approval_id match may bind a token.
#
# FAIL CLOSED when no producer's identity matches: never infer
# ownership from "first unclaimed token" or the queue head. A
# stale/foreign approval (mismatched request_id, or no
# identity at all) that borrows another live producer's token
# would let approving THIS mirror resolve a DIFFERENT producer
# than the one the user actually saw — an approval-integrity
# violation, not a convenience (found in review of 818fd2fd).
# A tokenless orphan is only legitimate when there is no live
# producer at all (#7093); reconcile_gateway_pending_mirror_locked
# binds a real mirror to the authoritative live head on its own.
live_queue_for_mirror = _gateway_queues.get(session_key) or []
request_id_for_mirror = str(approval.get("request_id") or "").strip()
mirror_producer = None
if request_id_for_mirror or approval_id:
for cand in live_queue_for_mirror:
cand_data = getattr(cand, "data", None) or {}
if (request_id_for_mirror and
str(cand_data.get("request_id") or "").strip() == request_id_for_mirror):
mirror_producer = cand
break
if (approval_id and not request_id_for_mirror and
str(cand_data.get("approval_id") or "").strip() == approval_id):
mirror_producer = cand
break
mirror_token = (
_gateway_mirror_entry_token(mirror_producer)
if mirror_producer is not None else None
)
mirror_entry = dict(approval)
mirror_entry["approval_id"] = approval_id
mirror_entry[_GATEWAY_MIRROR_FLAG] = True
if mirror_token:
mirror_entry[_GATEWAY_MIRROR_TOKEN] = mirror_token
_normalize_pending_queue_locked(session_key).append(mirror_entry)
head, total, _changed = reconcile_gateway_pending_mirror_locked(session_key)
_approval_sse_notify_locked(session_key, head, total)
api/routes.py
+18 -3
View File
@@ -24613,10 +24613,25 @@ def _resolve_approval_legacy(sid: str, approval_id: str, choice: str, run_id: st
if gw_approval_id == approval_id and (not run_id or gw_run_id == run_id):
local_gateway_approval_id = approval_id
elif not run_id and found_target and pending:
# The no-run mirror may belong to a NON-head producer
# (multiple parked entries, #7093). The queue head's own
# token won't match a non-head mirror, so scan every live
# producer for a token/approval_id match instead of only
# comparing against `_gateway_queues[0]`.
pending_token = str(pending.get(_GATEWAY_MIRROR_TOKEN) or "").strip()
gateway_token = str(gw_data.get("_webui_mirror_token") or "").strip()
if pending_token and pending_token == gateway_token:
gw_data["approval_id"] = approval_id
matched_data = None
for _cand in gw_queue:
_cand_data = getattr(_cand, "data", None) or {}
_cand_token = str(_cand_data.get("_webui_mirror_token") or "").strip()
if pending_token and _cand_token == pending_token:
matched_data = _cand_data
break
if (str(_cand_data.get("approval_id") or "").strip() == approval_id
and not str(_cand_data.get("run_id") or "").strip()):
matched_data = _cand_data
break
if matched_data is not None:
matched_data["approval_id"] = approval_id
local_gateway_approval_id = approval_id
# Notify SSE subscribers of the new head (or empty state) so the UI
# surfaces any trailing approvals that were queued behind this one
tests/test_gateway_approval_runs_api.py
+46 -14
View File
@@ -1073,10 +1073,19 @@ def test_legacy_gateway_approval_without_run_gets_browser_visible_id():
try:
approvals._gateway_queues[sid] = [SimpleNamespace(data={"command": "legacy"})]
approval = {"command": "legacy"}
approvals.submit_gateway_pending_mirror(sid, approval)
head, total = approvals.submit_gateway_pending_mirror(sid, approval)
assert approval.get("approval_id")
queue = approvals._pending.get(sid) or []
assert queue[0]["approval_id"] == approval["approval_id"]
# The browser is handed the RETURNED head — both production callers do
# `{**(head or approval_data), "pending_count": total}` — so the head's
# id is the browser-visible one, and it must be the id actually sitting
# in the polling queue or the respond call targets nothing. While a
# producer is live that id is the authoritative producer's, not the
# submitted copy's: an unmatched copy must not linger and mask it.
assert head is not None
assert total == 1
assert str(head["approval_id"]).strip()
assert queue[0]["approval_id"] == head["approval_id"]
finally:
approvals._pending.pop(sid, None)
approvals._gateway_queues.pop(sid, None)
@@ -1102,13 +1111,31 @@ def test_legacy_gateway_approval_without_run_keeps_its_own_id_beside_local_head(
and not str(entry.get("run_id") or "").strip()
]
assert mirrors
assert mirrors[-1]["approval_id"] == approval["approval_id"]
# The no-run mirror keeps an id of its own, distinct from the unrelated
# local pending entry's. While a producer is live that id comes from the
# authoritative producer rather than the submitted copy, so assert the
# distinctness this test is about instead of copy-identity.
assert str(mirrors[-1]["approval_id"]).strip()
assert mirrors[-1]["approval_id"] != "local-id"
finally:
approvals._pending.pop(sid, None)
approvals._gateway_queues.pop(sid, None)
def test_legacy_gateway_approval_without_run_keeps_its_own_id_beside_local_gateway_head():
def test_legacy_gateway_approval_without_run_yields_to_live_local_gateway_head():
"""An unmatched no-run mirror must not linger beside a live local producer.
Previously a submitted copy whose identity matched no live producer
("remote-id" here, against a parked producer carrying "local-id") was kept
in `_pending` alongside the producer. That copy is unresolvable — nothing
in `_gateway_queues` answers to its id — and while it sat there it
suppressed the producer's token, so the real pending approval never
surfaced as the head and could not be actioned (responding to the copy
returned 409 in gateway mode and `ok:true` resolving nothing in local
mode). Only the authoritative producer's mirror may survive while a
producer is live; tokenless-orphan retention is reserved for the genuine
no-producer case (#7093).
"""
import api.route_approvals as approvals
ta = pytest.importorskip(
"tools.approval",
@@ -1119,21 +1146,26 @@ def test_legacy_gateway_approval_without_run_keeps_its_own_id_beside_local_gatew
approvals._pending.pop(sid, None)
approvals._gateway_queues.pop(sid, None)
try:
approvals._gateway_queues[sid] = [ta._ApprovalEntry({
entry = ta._ApprovalEntry({
"approval_id": "local-id",
"command": "local-head",
})]
})
approvals._gateway_queues[sid] = [entry]
approval = {"approval_id": "remote-id", "command": "legacy"}
approvals.submit_gateway_pending_mirror(sid, approval)
head, total = approvals.submit_gateway_pending_mirror(sid, approval)
assert approval["approval_id"] == "remote-id"
queue = approvals._pending.get(sid) or []
mirrors = [
entry for entry in queue
if entry.get(approvals._GATEWAY_MIRROR_FLAG)
and not str(entry.get("run_id") or "").strip()
and entry.get("approval_id") == "remote-id"
]
assert mirrors
# The unmatched copy must be gone, not parked next to the producer.
assert not [
item for item in queue
if item.get("approval_id") == "remote-id"
], "an unmatched no-run mirror must not mask the live local producer"
# The authoritative producer is what the user sees and can action.
assert head is not None
assert total == 1
assert head["approval_id"] == "local-id"
assert head["command"] == "local-head"
assert queue[0]["approval_id"] == "local-id"
finally:
approvals._pending.pop(sid, None)
approvals._gateway_queues.pop(sid, None)
tests/test_issue_legacy_first_click_repro.py
+350
View File
@@ -0,0 +1,350 @@
"""Reproduction for: first approval click silently fails in legacy (in-process) mode.
BuggY branch reached by submit_gateway_pending_mirror():
`elif not exact_local_entry` (route_approvals.py). This branch runs when:
* run_id is empty, AND
* the incoming approval dict is NOT the same object as a live entry's data
and its approval_id does not match any live entry's data.approval_id.
In that branch the mirror is created WITHOUT `_GATEWAY_MIRROR_TOKEN`. Later,
_resolve_approval_legacy() must bind the no-run mirror back to the parked
_ApprovalEntry via token match (routes.py `elif not run_id and found_target
and pending:` -> `pending_token == gateway_token`). Without the mirror token
the match fails, resolve_gateway_pending_local() is never called,
live.event.set() is never invoked -> the agent thread stays blocked even
though the endpoint returns ok:true and the UI clears the card (only a ~1.5s
reconcile fallback re-creates the card -> requiring a 2nd click).
The fix stamps `_GATEWAY_MIRROR_TOKEN` from the live head when creating the
no-run mirror, so the FIRST click unblocks the agent.
"""
from __future__ import annotations
import copy
import threading
import uuid
from types import SimpleNamespace
def _live_head_data(approval_id: str = "", run_id: str = ""):
return {
"command": "rm -rf /tmp/test",
"description": "Delete temporary files",
"pattern_key": "dangerous_command",
"pattern_keys": ["dangerous_command"],
"approval_id": approval_id,
"run_id": run_id,
"choices": ["once", "session", "always", "deny"],
}
def test_legacy_no_run_mirror_first_click_unblocks_agent():
"""First 'once' click on a no-run legacy mirror must set the live agent event."""
from api import routes
from api import route_approvals as ra
sid = "sess-first-click-" + uuid.uuid4().hex[:8]
# Live _ApprovalEntry parked by the in-process legacy agent: NO run_id,
# NO approval_id yet (UUID is assigned to the mirror). The incoming mirror
# approval is a SEPARATE dict (copy) so exact_local_entry stays None and we
# land in the buggy `elif not exact_local_entry` branch.
live = SimpleNamespace(
data=_live_head_data(approval_id="", run_id=""),
event=threading.Event(),
result=None,
)
# _ApprovalEntry.__init__ always stamps a request_id via
# `data.setdefault("request_id", uuid.uuid4().hex)`, and notify_cb is
# handed `dict(entry.data)` — a copy that carries it. Mirror that
# contract here so the mirror's identity match (request_id/approval_id)
# can find this producer; without it, the fail-closed fallback (no
# inferring ownership from "first unclaimed"/head) would correctly
# refuse to bind a token, but that never happens for a real entry.
live.data["request_id"] = "req-" + uuid.uuid4().hex[:8]
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
ra._gateway_queues.setdefault(sid, []).append(live)
try:
mirror_approval = copy.deepcopy(live.data)
mirror_approval["approval_id"] = "appr-" + uuid.uuid4().hex[:8]
head, _total = ra.submit_gateway_pending_mirror(sid, mirror_approval)
with ra._lock:
mirrored = ra._pending[sid][0]
approval_id = mirrored["approval_id"]
# Verify we actually reached the no-run mirror branch.
assert mirrored.get(ra._GATEWAY_MIRROR_FLAG) is True
assert not str(mirrored.get("run_id") or "").strip()
# User clicks "Allow once" through the real legacy resolution path.
ok = routes._resolve_approval_legacy(sid, approval_id, "once")
# The hard requirement: the parked agent thread must be unblocked.
assert ok is True
assert live.event.is_set(), (
"BUG: first click returned ok but the agent thread was never "
"unblocked (live entry event not set). Need to build the token "
"link at mirror creation."
)
assert live.result == "once"
finally:
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
def test_legacy_non_head_producer_unblocks_only_its_producer():
"""A no-run mirror for a NON-head producer must unblock that producer, not the head."""
from api import routes
from api import route_approvals as ra
sid = "sess-first-click-nonhead-" + uuid.uuid4().hex[:8]
head = SimpleNamespace(
data=_live_head_data(approval_id="", run_id=""),
event=threading.Event(),
result=None,
)
non_head = SimpleNamespace(
data=_live_head_data(approval_id="", run_id=""),
event=threading.Event(),
result=None,
)
# _ApprovalEntry.__init__ stamps a request_id; copy that contract so the
# request_id fallback (#7093) links the mirror to its own producer.
head.data["request_id"] = "req-head-" + uuid.uuid4().hex[:8]
non_head.data["request_id"] = "req-nonhead-" + uuid.uuid4().hex[:8]
assert non_head.data["request_id"] != head.data["request_id"]
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
ra._gateway_queues.setdefault(sid, []).extend([head, non_head])
try:
# Mirror the NON-head producer (it is not the queue head).
mirror_approval = copy.deepcopy(non_head.data)
ra.submit_gateway_pending_mirror(sid, mirror_approval)
with ra._lock:
# The head is mirrored too; pick the mirror bound to the NON-head
# producer by its token.
non_head_token = str(non_head.data.get("_webui_mirror_token") or "").strip()
matches = [
m for m in ra._pending[sid]
if m.get(ra._GATEWAY_MIRROR_FLAG)
and not str(m.get("run_id") or "").strip()
and str(m.get(ra._GATEWAY_MIRROR_TOKEN) or "").strip() == non_head_token
]
assert len(matches) == 1, f"expected exactly one non-head mirror, got {len(matches)}"
non_head_mirror = matches[0]
approval_id = non_head_mirror["approval_id"]
assert str(non_head_mirror.get(ra._GATEWAY_MIRROR_TOKEN) or "").strip(), (
"no-run mirror must carry a token to be resolvable on the first click"
)
ok = routes._resolve_approval_legacy(sid, approval_id, "once")
assert ok is True
# The NON-HEAD producer must be the one unblocked.
assert non_head.event.is_set(), (
"BUG: click on a non-head mirror did not unblock its own producer. "
"_resolve_approval_legacy must match the mirror token against every "
"live producer, not only the queue head."
)
assert non_head.result == "once"
# The HEAD producer must remain untouched.
assert not head.event.is_set()
assert head.result is None
finally:
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
def test_legacy_stale_request_id_does_not_resolve_unrelated_live_producer():
"""A no-run mirror whose identity matches NO live producer must neither
resolve nor mask a different, unrelated live producer.
Regression for two defects on the same boundary:
* the approval-integrity violation found in review of 818fd2fd — the
old fallback bound an unmatched mirror to "the first unclaimed token"
(or the queue head), so approving a stale/foreign command A could
silently execute an unrelated live command B;
* the liveness/masking hole the fail-closed fix opened in its place —
the unmatched tokenless mirror A was still appended and retained,
suppressing B's token so the real pending approval never surfaced and
could not be actioned.
The invariant asserted here is displayed == resolvable == resolved: the
authoritative live producer is what the UI shows, responding to the stale
id fails without waking anything, and approving the displayed entry wakes
exactly its own producer.
"""
from api import routes
from api import route_approvals as ra
sid = "sess-stale-reqid-" + uuid.uuid4().hex[:8]
producer_b = SimpleNamespace(
data=_live_head_data(approval_id="", run_id=""),
event=threading.Event(),
result=None,
)
producer_b.data["command"] = "rm -rf /tmp/b-command"
producer_b.data["request_id"] = "req-b-" + uuid.uuid4().hex[:8]
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
ra._gateway_queues.setdefault(sid, []).append(producer_b)
try:
# A stale/foreign approval copy: a distinct request_id that matches
# no live producer (e.g. a duplicate resubmit after B replaced A, or
# a cross-session replay).
stale_approval_a = copy.deepcopy(producer_b.data)
stale_approval_a["command"] = "rm -rf /tmp/a-command-DIFFERENT"
stale_approval_a["request_id"] = "req-a-stale-" + uuid.uuid4().hex[:8]
stale_approval_a["approval_id"] = "appr-a-" + uuid.uuid4().hex[:8]
stale_approval_id = stale_approval_a["approval_id"]
head, total = ra.submit_gateway_pending_mirror(sid, stale_approval_a)
# The stale copy must not be displayed, and above all must not mask B.
with ra._lock:
pending_now = list(ra._pending.get(sid) or [])
assert all(
m.get("command") != stale_approval_a["command"] for m in pending_now
), (
"BUG: the unmatched stale mirror A was retained; it masks live "
"producer B, which can then never surface or be actioned."
)
assert head is not None, "live producer B must still surface a head"
assert head["command"] == producer_b.data["command"], (
"BUG: the displayed head is not the authoritative live producer."
)
assert total == 1
b_approval_id = str(head["approval_id"]).strip()
assert b_approval_id and b_approval_id != stale_approval_id
# Responding with A's stale id must fail closed — and must NOT wake B.
ok_stale = routes._resolve_approval_legacy(sid, stale_approval_id, "once")
assert ok_stale is False, (
"a stale approval_id must not report success"
)
assert not producer_b.event.is_set(), (
"BUG: approving unrelated stale command A resolved live "
"producer B instead."
)
assert producer_b.result is None
# Approving the entry the user was actually shown wakes exactly it.
ok_b = routes._resolve_approval_legacy(sid, b_approval_id, "once")
assert ok_b is True
assert producer_b.event.is_set(), (
"BUG: approving the displayed head did not unblock its producer."
)
assert producer_b.result == "once"
finally:
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
def test_legacy_identityless_mirror_with_multiple_producers_does_not_bind_to_head():
"""A tokenless, identity-less no-run mirror submitted while multiple
producers are parked must neither bind to the queue head nor mask it.
Regression for the approval-integrity violation found in review of
818fd2fd (inferring ownership from the queue head let approving an
unmatched mirror resolve the wrong producer) AND for the masking hole the
fail-closed fix opened (the unmatched mirror was retained and suppressed
the authoritative head's token). Asserts the same displayed == resolved
invariant as the stale-id case above, with two producers parked.
"""
from api import routes
from api import route_approvals as ra
sid = "sess-identityless-multi-" + uuid.uuid4().hex[:8]
head = SimpleNamespace(
data=_live_head_data(approval_id="", run_id=""),
event=threading.Event(),
result=None,
)
non_head = SimpleNamespace(
data=_live_head_data(approval_id="", run_id=""),
event=threading.Event(),
result=None,
)
head.data["command"] = "rm -rf /tmp/head-command"
head.data["request_id"] = "req-head-" + uuid.uuid4().hex[:8]
non_head.data["command"] = "rm -rf /tmp/nonhead-command"
non_head.data["request_id"] = "req-nonhead-" + uuid.uuid4().hex[:8]
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)
ra._gateway_queues.setdefault(sid, []).extend([head, non_head])
try:
# A copy carrying no identity at all: no request_id, no approval_id.
# Must NOT be inferred as belonging to `head`.
identityless_copy = {
"command": "rm -rf /tmp/mystery-command",
"description": "mystery",
"pattern_key": "dangerous_command",
"pattern_keys": ["dangerous_command"],
"choices": ["once", "session", "always", "deny"],
}
mirror_head, total = ra.submit_gateway_pending_mirror(sid, identityless_copy)
identityless_approval_id = str(identityless_copy["approval_id"]).strip()
# The identity-less copy must not be displayed, and must not suppress
# the authoritative head's mirror.
with ra._lock:
pending_now = list(ra._pending.get(sid) or [])
assert all(
m.get("command") != "rm -rf /tmp/mystery-command" for m in pending_now
), (
"BUG: an identity-less mirror was retained while producers were "
"parked; it masks the authoritative head."
)
assert mirror_head is not None
assert mirror_head["command"] == head.data["command"], (
"BUG: the displayed head is not the authoritative live producer."
)
assert total == 1
head_approval_id = str(mirror_head["approval_id"]).strip()
assert head_approval_id and head_approval_id != identityless_approval_id
# Responding with the identity-less id must wake nobody.
ok_identityless = routes._resolve_approval_legacy(
sid, identityless_approval_id, "once"
)
assert ok_identityless is False
assert not head.event.is_set(), (
"BUG: an identity-less mirror resolved the queue HEAD instead "
"of failing closed."
)
assert not non_head.event.is_set()
assert head.result is None
assert non_head.result is None
# Approving the displayed head resolves exactly that producer and
# leaves the non-head producer parked.
ok_head = routes._resolve_approval_legacy(sid, head_approval_id, "once")
assert ok_head is True
assert head.event.is_set()
assert head.result == "once"
assert not non_head.event.is_set()
assert non_head.result is None
finally:
with ra._lock:
ra._gateway_queues.pop(sid, None)
ra._pending.pop(sid, None)