approval_policy.approve_change()'s signature check is gated on the signature merely being PRESENT — the exact CWE-347 shape this SAME release fixed one file over in studio_integration_send.py (27de0c), left open on the platform's own policy engine (v1.5.0)
Reproduction steps:
- workbench/primitives/approval_policy.py, approve_change() (line 627),
is the handler behind POST /api/policy/approve (session-gated —
workbench/policy_endpoints.py:17 `POST /api/policy/approve ->
handle_approve(WS, body) (session-gated)`, line 188
pol.approve_change(ws, str((body or {}).get("staging_id", "")))).
Its ONLY tamper-evidence check is gated on presence:
if staged.get("signature") and signing is not None:
if signing.verify_against_install(
staged["integrity"], staged["signature"]
) == getattr(signing, "SIG_FAIL", "signature_fail"):
return {"ok": False, "error": "staged signature invalid — refusing"}
If signature is absent from the staged JSON — stripped, nulled, or
simply never present — the ENTIRE block is skipped and execution falls
straight through to applying the staged rules as the new live policy.
- The check immediately above it,
recomputed != staged.get("integrity"),
is NOT tamper-evidence either: integrity is an UNKEYED sha256 over the
staged file's own contents (line 635-641,
`body = {k: v for k, v in staged.items() if k not in ("integrity",
"signature")}` then re-hashed). Anyone who can edit the staged JSON on
disk recomputes this trivially to match their edit — this is stated
explicitly, about the sibling bug below, in this release's own fix
comment.
- This SAME release fixed the identical shape one file over, in
studio_integration_send.py's execute path (27de0c, marcofgv, CWE-347),
which the fix's own comment describes point-for-point:
# 27de0c (marcofgv, CWE-347): verification used to be GATED on the
# signature being present — if staged.get("signature"):.
# integrity is an UNKEYED sha256 recomputed from the on-disk
# plan, so it is not tamper-evidence: anyone who edits the staged
# file recomputes it trivially. The Ed25519 signature is the only
# local tamper-evidence, so an attacker simply stripped it (set
# null / removed the key) and the whole check was skipped — a
# tampered send then passed straight to policy_gate and executed.
#
# A missing signature is now a REFUSAL, not a skip.
studio_integration_send.py's companion stage() was ALSO hardened in the
same commit so it can no longer silently persist an unsigned staging
(it now refuses to stage at all if signing.sign_block fails). Neither
half of this treatment reached approval_policy.py's stage_change()
(line 569-600, which still does if sig: staged["signature"] = sig
with no else) or approve_change().
- approval_policy.py governs something strictly more consequential than
the one send-path studio_integration_send.py protects: it IS the
platform's own policy engine — the auto_approve/require_human/block
rules every other connector write is judged against. Silently widening
it (e.g. removing a per-charge spend cap) doesn't tamper one action, it
changes what counts as "safe to auto-approve" for every future action
the policy governs.
- Run repro_approval_policy_approve_signature_skip_v150.py against a
clean v1.5.0 extraction. It drives the REAL, unmodified
stage_change()/approve_change()/load() against a real Ed25519 signing
identity (ensure_keypair() generated exactly as a real station's boot
sequence does). It stages an honest, properly-signed change — auto-
approve stripe.charge_create up to $10 — then simulates only what an
attacker with staging-dir write access would do by hand: edit the
on-disk staged JSON to remove the cap (max_amount_cents: None ==
unlimited), recompute integrity with the verifier's own unkeyed
formula (trivial, no key needed), and delete the signature key.
Then it calls the real approve_change():
staged honestly: {"auto_approve": [{"connector": "stripe",
"verb": "charge_create", ..., "max_amount_cents": 1000}], ...}
legitimately signed: True
attacker-tampered on disk: max_amount_cents -> None (unlimited)
attacker-tampered signature present: False
approve_change() result: {'ok': True, 'policy': {... 'rules':
{'auto_approve': [{..., 'max_amount_cents': None}], ...},
'signature': {'alg': 'ed25519', ...}}, ...}
live policy auto_approve[0].max_amount_cents after approve: None
live policy tampered flag: None
CONFIRMED
Expected:
approve_change() should refuse to apply a staged policy change with no
signature, exactly as studio_integration_send.py's sibling check now does
in this same release ("A missing signature is now a REFUSAL, not a skip.
Absence of evidence must never read as evidence of absence on the only
path that can touch a provider" — equally true of the only path that can
change what the platform considers safe to auto-approve).
Actual:
A staged policy change with its signature stripped is accepted, applied as
the new live policy, and RE-SIGNED by the station's own real key on the
way out — so the resulting policy.json carries a perfectly valid Ed25519
signature over content that was never actually approved through the
signed-staging flow, and no external verifier (or the operator, looking at
the live policy afterward) can tell the difference from an honestly
approved change.
Suggested fix:
Mirror studio_integration_send.py's 27de0c fix exactly, in both functions:
# stage_change(): fail closed if signing can't produce a signature —
# never persist an unsigned staging.
if signing is not None:
try:
sig = signing.sign_block(integrity)
except Exception as e:
raise RuntimeError("cannot sign the staged policy delta — "
"refusing to stage (%s)" % str(e)[:120])
if not sig:
raise RuntimeError("cannot sign the staged policy delta — "
"refusing to stage")
staged["signature"] = sig
# approve_change(): a missing signature is a REFUSAL, not a skip.
if not staged.get("signature"):
return {"ok": False, "error": "staged policy delta is unsigned — "
"refusing (the signature is the "
"tamper-evidence; re-stage this change)"}
if signing is not None:
try:
if signing.verify_against_install(
staged["integrity"], staged["signature"]) != signing.SIG_VERIFIED:
return {"ok": False, "error": "staged signature invalid — refusing"}
except Exception:
return {"ok": False, "error": "staged signature verification error — refusing"}