Reproduction steps:
- workbench/primitives/approval_router.py's receive_incoming() (line 282),
invoked by the Relay poll handler for any incoming approval_request
event, writes the request's own request_id field directly into a path
with NO charset/prefix validation:
line 313: p = os.path.join(_pending_dir(ws), body["request_id"] + ".json")
line 314-317: write via a .tmp file, then os.replace(tmp, p) — a FULL
file replacement (open/truncate + atomic rename), not a merge.
build_request() (the intended constructor, line 145) always prefixes
request_id with "areq_", but nothing on the RECEIVING side enforces that
shape — this is the same class of gap as team_jobs.py (finding #2 in this
file) and, historically, bugs_found_v0.99.txt #13/#14: a value the minter
always prefixes safely, but a receiver trusts unprefixed and unchecked.
- _verify(body, requester_pubkey) (called just before the write) only
proves the body was signed by WHOEVER holds the private key matching the
requester_pubkey embedded IN THAT SAME BODY — i.e. it proves internal
self-consistency, not that the sender is a known or trusted party. Nothing
requires build_request() to have produced the body; an attacker can craft
a raw dict by hand, set request_id to anything (including a path-traversal
string), and sign it with their OWN, freshly-generated, entirely unrelated
Ed25519 keypair.
- Per this module's own docstring, "both keys publicly resolvable via
marketplace users" — the attacker needs no relationship to the victim
station, no team membership, no shared secret: only the victim's public
pubkey (by design, publicly resolvable) to put in approver_pubkey.
- _intake_policy(ws) (line 251) returns has_policy=False when the station
has neither an approval_allowlist.json file nor a team manifest — a
realistic default for many installs, especially any not yet in a team.
receive_incoming's gate is `if has_policy and requester_pubkey not in
allow: refuse` — when has_policy is False, this condition is False, so
NOTHING refuses the sender; the request is filed regardless, merely
tagged _sender_trust: "unverified (no allowlist/team configured)".
- Run repro_approval_router_request_id_traversal_v131.py against a clean
v1.3.1 extraction. It drives the REAL, unmodified receive_incoming() with
two independently-generated Ed25519 keypairs (no shared state) standing in
for the victim station and an unrelated attacker:
victim's has_policy: False
target file BEFORE: {"approved_by":"studio:operator","sealed":true,
"note":"REAL PLAN PIN -- not an approval request"}
receive_incoming() ack result: True
target file AFTER: {"schema":"railcall_approval_request.v1",
"request_id":"../pins/critical","requester_pubkey":"a9ed92...",
..., "_sender_trust":"unverified (no allowlist/team configured)"}
CONFIRMED — original content GONE, replaced entirely by the attacker's
request document.
Expected:
An id received from an external party, however strongly signed, must be
validated to a fixed charset/shape before it is ever joined into a
filesystem path — exactly the discipline this platform already applies
elsewhere (workflow_id via the _WF_ID regex one file over in team_jobs.py;
envelope_id's forced "env_" prefix in team_mesh). A cryptographic signature
proves WHO wrote a document; it says nothing about whether a FIELD INSIDE
that document is safe to use as a path component — those are orthogonal
properties, and this code conflates them.
Actual:
Any party — not merely an untrusted team member, but literally anyone who
can push a message through the shared Relay service (per the module's own
"publicly resolvable" trust model) — can fully overwrite an arbitrary
existing *.json file on a station that has not configured a team or
allowlist, with zero prior relationship to that station. This is a
FULL FILE REPLACEMENT (not a merge like the team_jobs.py sibling), so the
blast radius per hit is total content loss/replacement of whatever file the
traversal targets — plan pins, receipts, team/manifest.json, or
(depending on path depth reachable from WS/approval_pending/)keys.local.json if the traversal can reach it.
Suggested fix:
Validate request_id (and, defensively, every other peer-supplied id this
module later uses as a path component — decide_and_push's request_id
parameter at line 341, and _decisions_dir writes at line 418) against the
minter's own fixed shape before it is ever joined into a path:
import re
_REQUEST_ID = re.compile(r"^areq_[0-9]{8}T[0-9]{6}Z_[0-9a-f]{8}$")
...
def receive_incoming(ws, event):
body = event.get("body") or {}
if body.get("schema") != REQUEST_SCHEMA:
return True
rid = str(body.get("request_id") or "")
if not _REQUEST_ID.match(rid):
return True # malformed id — ack + drop, never touches a path
...
Separately, treat "no allowlist/team configured" as a reason to be MORE
conservative about writing peer-supplied data to disk, not less — an
unverified sender's request should, at minimum, never be trusted with a
raw filesystem write regardless of what the sender chose to put in any
field, which the id-validation fix above achieves as a side effect but is
worth stating as the module's own invariant.