Reproduction steps:
- server/db/schema.sql:228 declares
approval_id TEXT PRIMARY KEY— the table
has no composite (tenant_id, approval_id) uniqueness constraint, only a
non-unique index on (tenant_id, status). create_approval_request_v1() lets
the caller supply approval_id directly (it's in _ALLOWED_APPROVAL_FIELDS_V1,
approval_store.py:17-18) and falls back to a tenant-salted
make_stable_id_v1() only when the caller omits it (line 43). The write is
INSERT OR REPLACE INTO approval_requests (...) (line 50).
- Tenant A creates a real pending approval gating a $50,000 wire transfer,
explicitly naming its own approval_id:
execute_within_tenant_boundary_v1(TENANT_A, store.create_approval_request_v1,
{"approval_id":"appr_shared_id_0001", "workflow_run_id":"wfr_ACME_REAL_WIRE_TRANSFER",
"action_id":"wire_transfer_50000_usd", "risk_tier":"R1",
"proposed_action":{"type":"wire_transfer","amount_usd":50000,"to":"acme_vendor"},
"status":"pending"})
- Tenant B (unrelated) creates its own, unrelated approval, choosing the SAME
approval_id string (a realistic collision: a short human-chosen id, a replay
of a leaked/observed id format, or simply guessing the platform's own
"appr_" + hex convention):
execute_within_tenant_boundary_v1(TENANT_B, store.create_approval_request_v1,
{"approval_id":"appr_shared_id_0001", "action_id":"send_marketing_email",
"risk_tier":"R3", "proposed_action":{"type":"send_email"}, "status":"approved"})
- Tenant A looks up its own approval by the id it created; check the raw table.
- Run repro_approval_store_cross_tenant_pk_hijack_v131_server.py against a
clean v1.3.1 extraction (real, unmodified approval_store.py + schema.sql,
a real temp-file SQLite db, no network). Output:
Tenant A created: tenant_alpha_acme_corp appr_shared_id_0001 pending {...}
Tenant B created: tenant_beta_unrelated_llc appr_shared_id_0001 approved
Tenant A now sees: None
raw row: {'approval_id': 'appr_shared_id_0001', 'tenant_id': 'tenant_beta_unrelated_llc',
'status': 'approved', 'proposed_action_json': '{"template":"newsletter",...}'}
CONFIRMED — Tenant B's write silently REPLACED Tenant A's pending
$50,000 wire-transfer approval record.
Expected:
The store's own tenant boundary is meant to isolate tenants completely — every
read path checked (get_approval_request_v1, update_approval_status_v1) bindsWHERE tenant_id = ? AND approval_id = ? and raises when no tenant context is
active. A WRITE from tenant B must never be able to touch, replace, or even
observe the existence of a row belonging to tenant A. This is an approval-GATE
table specifically — a real governed action (a wire transfer) sits behind the
record that got silently overwritten.
Actual:
The conflict target for INSERT OR REPLACE is the bare approval_id primary
key, with tenant_id merely one of the columns being written, not part of the
uniqueness constraint. A second tenant's insert with a colliding id doesn't
error, doesn't append a second row, and doesn't get rejected by any
application-level ownership check (none exists) — it physically replaces the
row. Tenant A's own pending wire-transfer approval vanishes from ITS
tenant-scoped view entirely, and the surviving row belongs to tenant B, already
marked "approved". (The reverse direction is just as real: an attacker tenant
could instead target a victim's KNOWN approval_id — e.g. observed in a webhook,
log line, or shared receipt — and overwrite it with their own record before the
real approval resolves, or flip an existing pending record's status by
replaying create_approval_request_v1 with status="approved" and matching
fields, since the function does not distinguish "create" from "replace
existing".)
Suggested fix:
Make the tenant boundary part of the actual uniqueness constraint, not just a
column and a read-side filter:
ALTER: PRIMARY KEY (tenant_id, approval_id) -- composite, in schema.sql
and change the write to target that composite key:
INSERT INTO approval_requests (...) VALUES (...)
ON CONFLICT(tenant_id, approval_id) DO UPDATE SET ...
so a colliding approval_id from a DIFFERENT tenant creates its own independent
row instead of replacing another tenant's. Additionally, create_approval_request_v1
should refuse to silently overwrite an EXISTING row for the same (tenant_id,
approval_id) when one is already present with a different status/workflow_run_id
— a second "create" for an id that already exists should be a conflict error,
not a silent replace, even within the same tenant.