Skip to content

Commit 1fb265c

Browse files
committed
fix(ledger,retention): stop retention manufacturing tamper signals — pruned-record tolerance, append grace, interior orphans, and a fold-before-delete for the public counter (#9474, #9489)
1 parent 6f3b613 commit 1fb265c

12 files changed

Lines changed: 499 additions & 32 deletions

File tree

apps/loopover-ui/content/docs/what-you-can-verify.mdx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,24 @@ hash, so any edit to history breaks the chain at a point you can locate.
2929
curl -s "https://api.loopover.ai/v1/public/decision-ledger/verify" | jq
3030
```
3131

32-
Returns `{ ok, checked, nextAfterSeq, tipSeq, tipHash, totalCount }`, and a `break` object with a
33-
`409` status if the chain is inconsistent. No API key — **anyone** can run it.
32+
Returns `{ ok, checked, nextAfterSeq, tipSeq, tipHash, totalCount, prunedRecords }`, and a `break`
33+
object with a `409` status if the chain is inconsistent. No API key — **anyone** can run it.
34+
35+
Two response fields encode deliberate, published semantics rather than tolerance for tampering:
36+
37+
- **`prunedRecords`** — decision *records* (the preimages) are retained for 180 days; ledger rows are
38+
kept forever. A ledger row whose hash-chained `createdAt` is older than that window may therefore
39+
reference a pruned record: the chain checks still hold for it, only the content re-check is
40+
impossible, and it is counted here instead of reported as `missing_record`. The tolerance keys on
41+
the *ledger row's* timestamp, which is inside the hash chain — backdating it to sneak a fresh
42+
deletion under the window breaks `row_hash_mismatch` first. What pruning genuinely gives up: the
43+
row's committed digest stays published, but only a challenger holding the original preimage can
44+
still prove a historical rewrite of it.
45+
- **Append grace (5 minutes)** — a record and its chain row are two writes moments apart, so a
46+
record younger than the grace window with no chain entry is "append in flight", not a break. Older
47+
than that, it is reported: past the verified tip as `short_tail` (the truncated-tail signature),
48+
or behind it as `unchained_record` (the failed-append signature — interior orphans are found by an
49+
anti-join over the whole record set, not just the tail).
3450

3551
<Callout variant="note">
3652
**Trust assumption: tamper-evident, externally anchored.** The check above still catches sequence

apps/loopover-ui/public/openapi.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20104,10 +20104,10 @@
2010420104
"summary": "Verify a window of the hash-chained decision ledger (resumable via afterSeq)",
2010520105
"responses": {
2010620106
"200": {
20107-
"description": "Window verified clean; nextAfterSeq is the resume cursor (null at the tip). Every response also carries tipSeq/tipHash/totalCount for third-party checkpointing."
20107+
"description": "Window verified clean; nextAfterSeq is the resume cursor (null at the tip). Every response also carries tipSeq/tipHash/totalCount for third-party checkpointing, and prunedRecords — the count of rows whose record preimage was legitimately pruned by the published retention window (chain checks still hold for them; only the content re-check is impossible, and the committed digest stays published)."
2010820108
},
2010920109
"409": {
20110-
"description": "First break found: sequence_gap | predecessor_mismatch | row_hash_mismatch | short_tail (a record exists past the verified tip with no chain entry)"
20110+
"description": "First break found: sequence_gap | predecessor_mismatch | row_hash_mismatch | missing_record | content_mismatch | short_tail (a record newer than the verified tip has no chain entry — the truncated-tail signature) | unchained_record (an INTERIOR record has no chain entry — the failed-append signature). Records younger than the 5-minute append grace window are not reported: the record insert and its chain append are two writes moments apart, and a verify landing between them is not evidence of tampering."
2011120111
}
2011220112
},
2011320113
"security": [
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
-- #9474: durable running totals for orb_pr_outcomes' CUMULATIVE public consumer.
2+
--
3+
-- #9415 gave orb_pr_outcomes a 90-day retention window, but getOrbGlobalStats SUMs the ENTIRE table and
4+
-- public-stats folds that into the homepage's all-time merged/closed/handled counters. Once rows began aging
5+
-- past 90 days (~2026-10-25 given #9415's merge date) the "all-time" numbers would have plateaued and then
6+
-- visibly DECREASED -- a published cumulative counter going backwards.
7+
--
8+
-- The retention prune now folds each about-to-be-deleted row into this table first, atomically (same batch
9+
-- transaction as the delete -- see pruneExpiredRecords' orb_pr_outcomes special case), and getOrbGlobalStats
10+
-- adds these totals to its live scan. Keyed per LOWERCASED account_login (matching the stats query's own
11+
-- LOWER() comparison; '' for a NULL login) so its excludeAccount de-dup keeps working after the raw rows are
12+
-- gone. Only rows the live query would have counted are folded: registered installations, no published
13+
-- review surface.
14+
CREATE TABLE IF NOT EXISTS orb_outcome_rollups (
15+
account_login TEXT PRIMARY KEY,
16+
merged INTEGER NOT NULL DEFAULT 0,
17+
closed INTEGER NOT NULL DEFAULT 0,
18+
total INTEGER NOT NULL DEFAULT 0,
19+
updated_at TEXT NOT NULL
20+
);
21+
22+
-- #9489/#9474: verifyDecisionLedger's completeness reconciliation now asks "does ANY ledger row vouch for
23+
-- this record" (a NOT EXISTS anti-join finding interior orphans, not just tail ones); without this index
24+
-- that is a full ledger scan per candidate record.
25+
CREATE INDEX IF NOT EXISTS decision_ledger_record_id ON decision_ledger (record_id);

src/db/retention.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ export type RetentionRule = { table: string; column: string; days: number };
1414
const DURABLE_AUDIT_EVENT_TYPES = ["github_app.pr_public_surface_published"] as const;
1515

1616
export const RETENTION_POLICY: readonly RetentionRule[] = [
17+
// #9474: MUST stay ahead of audit_events. This table's prune first FOLDS the rows it is about to delete
18+
// into the durable orb_outcome_rollups running totals (see pruneExpiredRecords' special case), and that
19+
// fold reuses getOrbGlobalStats' exact counting semantics -- including the LEFT JOIN that excludes
20+
// outcomes whose PR already published a review surface (a durable audit_events row). Both tables share a
21+
// 90-day window, so if audit_events pruned first within the same pass, the very audit rows that exclusion
22+
// needs would be gone by the time the fold ran, and the rollup would permanently over-count exactly the
23+
// PRs the live query never counted.
24+
{ table: "orb_pr_outcomes", column: "occurred_at", days: 90 },
1725
{ table: "audit_events", column: "created_at", days: 90 },
1826
{ table: "ai_usage_events", column: "created_at", days: 90 },
1927
{ table: "product_usage_events", column: "occurred_at", days: 180 },
@@ -74,7 +82,9 @@ export const RETENTION_POLICY: readonly RetentionRule[] = [
7482
{ table: "pull_request_files", column: "updated_at", days: 30 },
7583
{ table: "repo_github_totals_snapshots", column: "fetched_at", days: 30 },
7684
{ table: "recent_merged_pull_requests", column: "updated_at", days: 30 },
77-
{ table: "orb_pr_outcomes", column: "occurred_at", days: 90 },
85+
// orb_pr_outcomes was #9415's fifth entry here; #9474 moved it to the TOP of this policy (ordering
86+
// constraint documented there) and gave it a fold-before-delete so the cumulative public counter it
87+
// feeds can never shrink.
7888
// #9473: four more members of the same re-derivable/per-event class #9415 bounded, found by an audit sweep
7989
// for tables written per event with NO delete path anywhere in src/. Two have a pruned sibling, which is
8090
// what makes the omission clearly unintentional rather than a retention decision:
@@ -157,6 +167,20 @@ export const RETENTION_COMPOSITE_PK_TABLES: ReadonlySet<string> = new Set([
157167
"linked_issue_satisfaction_cache",
158168
]);
159169

170+
/**
171+
* The retention cutoff for `table` as of `nowMs` -- rows with a timestamp strictly BELOW this are eligible
172+
* for pruning -- or null when the table has no retention rule at all. #9474: exported so consumers whose
173+
* correctness depends on a table's permanence can reason about its IMPERMANENCE instead of silently assuming.
174+
* verifyDecisionLedger uses this to tell "this record was legitimately pruned by the published retention
175+
* policy" apart from "this record is missing and should not be": the distinction is keyed on the LEDGER row's
176+
* hash-chained created_at (which cannot be backdated without breaking the chain), so an operator cannot use
177+
* the tolerance to hide a fresh deletion.
178+
*/
179+
export function retentionCutoffIsoForTable(table: string, nowMs: number = Date.parse(nowIso())): string | null {
180+
const rule = RETENTION_POLICY.find((candidate) => candidate.table === table);
181+
return rule ? cutoffIso(rule.days, nowMs) : null;
182+
}
183+
160184
function pkColumnFor(table: string): string {
161185
return RETENTION_PK_COLUMN[table] ?? "rowid";
162186
}
@@ -219,6 +243,49 @@ export async function pruneExpiredRecords(
219243
continue;
220244
}
221245

246+
// #9474: orb_pr_outcomes feeds a CUMULATIVE public counter (getOrbGlobalStats -> the homepage "all-time"
247+
// merged/closed totals), so its rows must be folded into the durable orb_outcome_rollups totals in the
248+
// same transaction that deletes them -- a fold and delete that could commit separately would either
249+
// double-count (fold landed, delete didn't, next run re-folds) or under-count (delete landed, fold
250+
// didn't). One atomic batch, both statements scoped to the identical cutoff, sidesteps both. The delete
251+
// is deliberately UNBATCHED for this one table: the aging cohort is one row per fleet-wide PR terminal
252+
// per day (hundreds at most, vs the six-figure log tables the batching exists for), and a bounded delete
253+
// would reintroduce the split-commit problem for whatever the bound left behind.
254+
if (rule.table === "orb_pr_outcomes") {
255+
const batchResults = await env.DB.batch([
256+
// Fold EXACTLY the population getOrbGlobalStats counts: registered installations only, and only
257+
// outcomes whose PR never published a review surface (those are already counted by the own ledger).
258+
// Rows failing either filter are deleted WITHOUT folding -- the live query never counted them, so
259+
// folding them would make the public total jump on prune day. Keyed per lowercased account_login so
260+
// the stats query's excludeAccount de-dup keeps working against the rollup after the raw rows are gone.
261+
env.DB.prepare(
262+
`INSERT INTO orb_outcome_rollups (account_login, merged, closed, total, updated_at)
263+
SELECT LOWER(COALESCE(i.account_login, '')) AS account_login,
264+
SUM(CASE WHEN o.outcome = 'merged' THEN 1 ELSE 0 END) AS merged,
265+
SUM(CASE WHEN o.outcome = 'closed' THEN 1 ELSE 0 END) AS closed,
266+
COUNT(*) AS total,
267+
?2 AS updated_at
268+
FROM orb_pr_outcomes o
269+
JOIN orb_github_installations i ON i.installation_id = o.installation_id AND i.registered = 1
270+
LEFT JOIN audit_events ae
271+
ON ae.target_key = o.repository_full_name || '#' || o.pr_number
272+
AND ae.event_type = 'github_app.pr_public_surface_published'
273+
WHERE o.occurred_at < ?1 AND ae.id IS NULL
274+
GROUP BY LOWER(COALESCE(i.account_login, ''))
275+
ON CONFLICT(account_login) DO UPDATE SET
276+
merged = orb_outcome_rollups.merged + excluded.merged,
277+
closed = orb_outcome_rollups.closed + excluded.closed,
278+
total = orb_outcome_rollups.total + excluded.total,
279+
updated_at = excluded.updated_at`,
280+
).bind(cutoff, nowIso()),
281+
env.DB.prepare(`DELETE FROM orb_pr_outcomes WHERE occurred_at < ?1`).bind(cutoff),
282+
]);
283+
/* v8 ignore next 2 -- defensive: batch() returns exactly one result per statement on both backends, so
284+
* the `?.`/`?? 0` arms only satisfy the driver types; a missing meta degrades the COUNT, never the prune. */
285+
results.push({ table: rule.table, column: rule.column, cutoff, deleted: Number(batchResults[1]?.meta?.changes ?? 0) });
286+
continue;
287+
}
288+
222289
let deleted = 0;
223290
// Batched delete by a real indexable PK (see RETENTION_PK_COLUMN) ordered by the retention column, so
224291
// each statement is bounded AND the inner SELECT is an index range scan on Postgres, not a ctid-keyed

src/openapi/spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,8 +1641,8 @@ export function buildOpenApiSpec() {
16411641
path: "/v1/public/decision-ledger/verify",
16421642
summary: "Verify a window of the hash-chained decision ledger (resumable via afterSeq)",
16431643
responses: {
1644-
200: { description: "Window verified clean; nextAfterSeq is the resume cursor (null at the tip). Every response also carries tipSeq/tipHash/totalCount for third-party checkpointing." },
1645-
409: { description: "First break found: sequence_gap | predecessor_mismatch | row_hash_mismatch | short_tail (a record exists past the verified tip with no chain entry)" },
1644+
200: { description: "Window verified clean; nextAfterSeq is the resume cursor (null at the tip). Every response also carries tipSeq/tipHash/totalCount for third-party checkpointing, and prunedRecords — the count of rows whose record preimage was legitimately pruned by the published retention window (chain checks still hold for them; only the content re-check is impossible, and the committed digest stays published)." },
1645+
409: { description: "First break found: sequence_gap | predecessor_mismatch | row_hash_mismatch | missing_record | content_mismatch | short_tail (a record newer than the verified tip has no chain entry — the truncated-tail signature) | unchained_record (an INTERIOR record has no chain entry — the failed-append signature). Records younger than the 5-minute append grace window are not reported: the record insert and its chain append are two writes moments apart, and a verify landing between them is not evidence of tampering." },
16461646
},
16471647
});
16481648
registry.registerPath({

src/orb/outcomes.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,19 @@ export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: strin
6464
// excludeAccount de-dups an account already counted by another source. "" = include all.
6565
const exclude = (opts.excludeAccount ?? "").toLowerCase();
6666
let row: { merged: number | null; closed: number | null; total: number | null } | null;
67+
let rollup: { merged: number | null; closed: number | null; total: number | null } | null;
6768
// #8879: guard ONLY the query. A D1 error degrades to zeros, mirroring computeFleetAnalytics's try/catch
6869
// (src/orb/analytics.ts) so a failure on this join drops just the orb aggregate instead of 503-ing the entire
6970
// /v1/public/stats payload (accuracyTrend/reuseRateTrend/reviewVolumeTrend/rulePrecision) via the route catch.
7071
try {
72+
// #9474: rows older than the 90-day retention window are folded into orb_outcome_rollups by the prune
73+
// (atomically with their deletion -- see pruneExpiredRecords) precisely so THIS cumulative total never
74+
// shrinks. Rollup rows key on the lowercased account_login, so the same excludeAccount de-dup applies.
75+
rollup = await env.DB.prepare(
76+
`SELECT SUM(merged) AS merged, SUM(closed) AS closed, SUM(total) AS total FROM orb_outcome_rollups WHERE (?1 = '' OR account_login <> ?1)`,
77+
)
78+
.bind(exclude)
79+
.first<{ merged: number | null; closed: number | null; total: number | null }>();
7180
row = await env.DB.prepare(
7281
`SELECT
7382
SUM(CASE WHEN o.outcome = 'merged' THEN 1 ELSE 0 END) AS merged,
@@ -88,5 +97,10 @@ export async function getOrbGlobalStats(env: Env, opts: { excludeAccount?: strin
8897
}
8998
/* v8 ignore next -- an aggregate query always returns exactly one row; this guards the nullable .first() type only */
9099
if (!row) return { merged: 0, closed: 0, total: 0 };
91-
return { merged: row.merged ?? 0, closed: row.closed ?? 0, total: row.total ?? 0 };
100+
// The SUM() arms are genuinely reachable NULLs, not defensive: an aggregate over zero rows returns NULL.
101+
return {
102+
merged: (row.merged ?? 0) + (rollup?.merged ?? 0),
103+
closed: (row.closed ?? 0) + (rollup?.closed ?? 0),
104+
total: (row.total ?? 0) + (rollup?.total ?? 0),
105+
};
92106
}

0 commit comments

Comments
 (0)