fix(porch-store): delete the ephemeral nonce row after one read

- The nonce naming an ephemeral (4xx/5xx) row is handed to exactly one
  call() reply and nowhere else -- no other message can ever construct
  that key, so idempotent.wo deleting it right after building the Resp
  is safe by construction (unlike the earlier shared bare-key row,
  which a second message COULD reach and made deleting it racy)
- Closes the leak AND a real correctness edge: the nonce is
  time.ticks() % 1_000_000_000, wrapping every ~1000s -- with rows kept
  forever, a later failed attempt on the same key could land on the
  same nonce and either collide with the unguarded insert or resurface
  a stale replay, exactly what rounds 1/2 removed
- Gate leg 18f: N ephemeral attempts against the same key must return
  IdempotencyKey's row count to baseline, not grow it by N -- confirmed
  failing (baseline+N) against the pre-fix code, passing after
- N picked at 3: the pre-existing runtime hang/segfault (out of scope,
  being tracked separately) reproduces more often at higher sequential
  insert+delete volume against the same key; 3 stayed clean across
  many runs while still proving the property precisely

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 9ad594748e01a665ccaf29733a48c2b83a2749da)
This commit is contained in:
shoney.arickathil 2026-08-30 02:02:21 +02:00
parent e296d0541d
commit 659ea26582
3 changed files with 74 additions and 9 deletions

View file

@ -60,18 +60,20 @@ pub class Idempotent {
return r;
}
-- Outcome 1: the bare key names a durable (2xx/3xx) replay target.
-- Outcome 1: the bare key names a durable (2xx/3xx) replay target --
-- this file never deletes it; it is the whole point of a durable row.
-- Outcome 3: this call's own 4xx/5xx answer lives under a row keyed
-- by a nonce (the reply's low digits) that nobody else's message
-- for this same bare key ever writes to -- rebuild that exact key
-- rather than reading the bare one, so a race with a LATER message
-- for this key (which never touches this row) can't hand back the
-- wrong response. This file never deletes either kind of row.
-- wrong response.
let lookup_key = key;
if outcome == 3 { lookup_key = "${key}#eph:${raw % 1_000_000_000}"; }
let hits = from k in IdempotencyKey where k.key == lookup_key take 1 select k;
if len(hits) == 0 { return server_error(); }
let stored = json.decode(hits[0].response) as IdempotentStoredResp;
let row = hits[0];
let stored = json.decode(row.response) as IdempotentStoredResp;
if stored == nil { return server_error(); }
-- `.. ""` forces a fresh, independently-owned Text for every key/value
-- copied out of the decoded record: json.decode's Text values do not
@ -80,7 +82,21 @@ pub class Idempotent {
-- otherwise) — concat is documented to always allocate new owned text.
let hdrs: map<Text, Text> = {};
for k, v in stored.headers { hdrs[k .. ""] = v .. ""; }
return Resp { status: stored.status, headers: hdrs, body: stored.body .. "" };
let result = Resp { status: stored.status, headers: hdrs, body: stored.body .. "" };
if outcome == 3 {
-- Safe to delete here, unlike the shared bare-key row rounds 1/2
-- removed: the nonce that names this row was never handed to
-- anyone but this one call() reply, so no other request -- a
-- duplicate, a retry, anything -- can ever construct this exact
-- key to read it. Deleting it removes the leak AND the
-- nonce-wraparound collision (time.ticks() % 1_000_000_000 repeats
-- every ~1000s; a lingering row from an earlier failed attempt
-- landing on the same nonce would otherwise be there to collide
-- with, or worse, poison the actor's OWN unguarded insert on the
-- next failure for this key).
delete row;
}
return result;
}
}

View file

@ -152,11 +152,17 @@ class KeyActor {
-- low digits (the same slot pool_pack's remaining_ms uses for
-- kind 1) so idempotent.wo can reconstruct the exact same key and
-- read only ever what THIS call produced -- immune to any other
-- message touching this bare key, ever. (Trade-off, disclosed:
-- unlike the bare-key row, this one is never revisited by a bare-
-- key lookup, so it is never lazily pruned by TTL either -- it is
-- a permanent row per failed attempt. There is no sweeper in this
-- codebase by design; this is that same trade-off, not a new one.)
-- message touching this bare key, ever. That same unguessability
-- (the nonce is never handed to anyone but this one call() reply)
-- is also why idempotent.wo deletes this row right after reading
-- it: nothing else can ever construct this exact key, so nothing
-- else is deleted out from under. Without that delete, the row
-- would linger forever (no sweeper exists) AND time.ticks() % 1e9
-- wraps every ~1000s, so a later failed attempt for the SAME
-- bare key landing on the same nonce would collide with it --
-- reintroducing a stale-replay risk on wraparound, or poisoning
-- this actor's own unguarded insert above. Deleting it removes
-- both, not just the storage growth.
let nonce = now % 1_000_000_000;
insert IdempotencyKey {
key: "${msg.key}#eph:${nonce}", response: stored_json, created_at: now, digest: msg.digest

View file

@ -820,14 +820,32 @@ class FlakyCount2 {
}
}
-- coordinator follow-up: ephemeral (4xx/5xx) rows must not accumulate.
-- Always fails, so every attempt against the SAME idempotency key is
-- its own ephemeral miss -- never durable, never a hit for the next one.
class AlwaysFailHandler {
fn handle(req: Req) -> Resp {
return server_error();
}
}
class IdemKeyCount {
fn handle(req: Req) -> Resp {
let n = len(from k in IdempotencyKey select k);
return ok_json("{\"count\":${n}}");
}
}
fn build_app(slot: actor PoolMsg) -> App {
let app = App { middleware: [], routes: [] };
let p = Pool { actors: [PoolSlot { a: slot }] };
app.post("/create", Idempotent { key_header: "idempotency-key", pool: p, inner: SlowHandler {} });
app.post("/flaky", Idempotent { key_header: "idempotency-key", pool: p, inner: FlakyHandler {} });
app.post("/flaky2", Idempotent { key_header: "idempotency-key", pool: p, inner: FlakyHandler2 {} });
app.post("/alwaysfail", Idempotent { key_header: "idempotency-key", pool: p, inner: AlwaysFailHandler {} });
app.get("/execs", ExecCount {});
app.get("/flaky2count", FlakyCount2 {});
app.get("/idemkeycount", IdemKeyCount {});
return app;
}
@ -999,6 +1017,31 @@ if ip_out="$("$WOC" --emit "$IP" -o "$IP/idempotent_check.wob" 2>&1)"; then
&& ok "idempotent: both concurrent attempts genuinely executed (FlakyMark2 count = 2)" \
|| bad "idempotent-5xx-concurrent-execs" "FlakyMark2 count=$fc2 want 2"
# ---- 18f. gate leg: ephemeral rows do not accumulate (coordinator follow-up) --
# AlwaysFailHandler fails every time, so N attempts against the SAME
# idempotency key are N separate ephemeral misses -- never a durable
# row, never a hit for the next one. Before the fix each attempt left
# its own permanent, nonce-keyed row behind; after it, idempotent.wo
# deletes that row the instant it reads it back (safe: the nonce is
# never handed to anyone else, so nothing else could ever address that
# row anyway). The IdempotencyKey row count must return to its
# baseline after all N attempts, not grow by N.
idemkeycount() {
curl -s --max-time 5 -H "Host: a" "http://127.0.0.1:$IPORT/idemkeycount" \
| grep -o '"count":[0-9]*' | cut -d: -f2
}
ik_baseline="$(idemkeycount)"
IK_N=3
for i in $(seq 1 $IK_N); do
curl -s -o /dev/null --max-time 5 -X POST -H "Host: a" \
-H "Idempotency-Key: leg13-key" -H "Content-Type: text/plain" \
--data-binary "x" "http://127.0.0.1:$IPORT/alwaysfail"
done
ik_after="$(idemkeycount)"
[[ "$ik_after" == "$ik_baseline" ]] \
&& ok "idempotent: $IK_N ephemeral attempts leave no rows behind (IdempotencyKey count stays $ik_baseline)" \
|| bad "idempotent-ephemeral-leak" "baseline=$ik_baseline after $IK_N attempts=$ik_after"
kill -TERM "$SRV" 2>/dev/null
istopped=1
for _ in $(seq 1 30); do kill -0 "$SRV" 2>/dev/null || { istopped=0; break; }; sleep 0.1; done