RELEASE
planning/2.8.5/audits/AUDIT_registry_leak.md
sha256 64cf8505186b9f4d · 12920 bytes ·
original held in the private archive
# AUDIT — cross-game local-slot registry leak (commit `2210b57`)
**Stage 3 (Opus independent audit) · Protocol 8 · branch `dev` · audited against the running app, not the implementer's report.**
Snapshot as of 2026-07-14 — audit record, not current-truth doc.
---
## VERDICT: **PASS** — with one mandatory record correction and one recommended hardening.
The shipped fix is correct, reproduced-then-resolved in a real browser, guarded red→green, and regression-free. **However, the implementer's "it never corrupted save data — only lookups and suggestions" claim is FALSE** — I reproduced real campaign-data loss on the pre-fix code. The fix nonetheless *prevents* that loss (it closes the window entirely), so no code change is required for this commit to be correct — but the record must be corrected, and a defense-in-depth guard is recommended (see §4).
---
## (1) Is the bug genuinely fixed? — YES, reproduced then resolved in a real browser.
Independent Playwright harness (`chromium.launch()` + `file://index.html`, modelled on `render-check.mjs`), run twice: once against the shipped fix, once against `js/ui/ui-saves.js` reverted to parent `17e880d`.
**Pre-fix (`17e880d` ui-saves.js) — BUG REPRODUCED:**
```
BEFORE load: ctx=FNV regVer=2.0.0 Novac=true Megaton=false
navigation(reload) occurred: false ← in-place apply, NO reboot
AFTER load: ctx=FO3 regVer=2.0.0 Novac=true Megaton=false
VERDICT: BUG REPRODUCED (FO3 ctx but NV locations)
sample after-locations: ["188 Trading Post","Aerotech Office Park","Atomic Wrangler Casino","Bitter Springs",...]
```
state flipped to FO3 while `FALLOUT_REGISTRY` stayed NV (version `2.0.0`, Novac present, Megaton absent) — the `#locationOptions` datalist served New Vegas locations in an FO3 campaign. Exactly the owner report. **I saw the bug myself.**
**Post-fix (`2210b57`) — RESOLVED:**
```
BEFORE load: ctx=FNV regVer=2.0.0 Novac=true Megaton=false
navigation(reload) occurred: true ← reboot fires
AFTER load: ctx=FO3 regVer=2.0.0-fo3 Novac=false Megaton=true
VERDICT: FIXED (FO3 ctx + FO3 locations)
sample after-locations: ["Megaton","Rivet City","Canterbury Commons","Underworld","Paradise Falls","Big Town"]
```
The terminal reboots, boot re-selects `reg_fo3.js`/`db_fo3.js` via the `GAME_FILES` manifest (`index.html` reads `robco_v8.activeContext`), and every registry surface serves FO3 data only. Screenshot `planning/mockups/fo3/audit-crossgame-FIXED.png` shows the app booted as "PIP-BOY 3000", LVL 7 (the slot's level).
**Mechanism is sound:** the fix persists `robco_v8` **synchronously** (`localStorage.setItem` before the `setTimeout(reload, 2000)`), sets `window._loadingSave = true` first, and the `beforeunload` flush honours that flag (`ui-core.js:1091`, `state.js:1795`). So no unsaved state is lost across the reboot (Suite 95.8/95.9 footgun handled correctly — matches the `loadCloudSave`/`handleFileUpload` precedent).
---
## (2) Is the "no save corruption" claim TRUE? — **NO. It is FALSE.** A stale registry DID drop real campaign data.
This is the finding that matters most. The blast-radius narrative frames the impact as read-only ("only lookups and suggestions, never what got written"). That is wrong. **The AI-import path (`autoImportState`, `js/services/api-import.js`) validates several state arrays against `FALLOUT_REGISTRY` and DROPS every entry the registry doesn't recognise** — and it does so *on a write to `state`*:
- `collectibles` (line 676): `state.collectibles = parsed.collectibles.filter(c => _collectNames.has(c) ...)`
- `traits` (724), `skillBooks` (741), `magazines` (758): same filter-and-write pattern
- `lincolnItems` (706-710): key must be a registry name or it is dropped
With a stale registry (state=FO3, registry=NV), the campaign's own FO3 names match nothing in the NV registry, so the next AI turn — which echoes the full state every message — silently empties them and persists the result.
**Empirically reproduced on the pre-fix code**, in the real stale-registry browser state:
```
STALE-REGISTRY CORRUPTION PROBE (buggy code):
ctx after load : FO3
registry version (stale) : 2.0.0 (NV; expected 2.0.0-fo3)
FO3 collectibles in save : ["Strength","Perception"]
NV-registry has Strength : false
NV-registry has Perception: false
collectibles AFTER AI import: []
>>> DATA DROPPED BY STALE REGISTRY: YES — FO3 collectibles wiped to [] on AI turn
```
`autoImportState(JSON.stringify({collectibles:['Strength','Perception']}))` — the AI faithfully resending the campaign's two FO3 bobbleheads — wiped `state.collectibles` to `[]`. That value then persists to the `robco_v8` FO3 campaign. This is durable data loss of `collectibles`, and by the identical code path `traits` / `skillBooks` / `magazines` / `lincolnItems`.
**Reachability in the shipped bug:** the pre-fix in-place apply left the stale registry live for the *entire session* until a manual reload. Any AI message in that window triggered the drop. The owner apparently only noticed the cosmetic autocomplete symptom, but the corruption was latent the whole time.
**Does the fix prevent it? YES.** By rebooting immediately on cross-game load, the fix ensures `autoImportState` always runs with the correct registry, so the drop can never fire. I confirmed every other cross-game entry point already reboots (`onGameContextChange` → `_contextSwitching`; `loadCloudSave`/`restoreCloudSaveVersion`/`handleFileUpload` container-import → `_loadingSave`), and that the file-import *envelope/legacy* path routes through `autoImportState`, which **intentionally ignores `gameContext`** (`api-import.js:662-664`) and thus never opens a mismatch. **The corruption window is now closed on every path.**
**Bottom line:** the fix is *more* valuable than the report claims — it also closes a real save-corruption vector. But the "no corruption" line is false and must be struck from the record. Note: the user-facing `CHANGELOG.md` entry does **not** make this false claim (it is correctly scoped to "suggestions and lookups"), so no shipped document is inaccurate — the error is confined to the implementer's stage-2 narrative.
---
## (3) Blast-radius accuracy — ACCURATE (if anything, under-stated on the write path).
Every surface the commit message names was independently confirmed to read the per-game registry/DB:
| Surface | Reads | Affected by stale registry? |
| --- | --- | --- |
| location/item/quest/perk autocomplete | `registrySearch` (`initRegistryAutocomplete`) | ✅ yes |
| native LOOT | `lookupItemInDb` (`ui-render-loot.js:129`) | ✅ yes (auto-fills wrong wgt/val) |
| native THREAT | `getBestiaryNames` + `FALLOUT_REGISTRY.locations` (`api-router.js:651`) | ✅ yes |
| native CONSULT | `registrySearch` + `lookupItemInDb` (`ui-render-databank.js:210`) | ✅ yes |
| DATABANK search | `registrySearch` (`ui-render-databank.js:245`) | ✅ yes |
| ELIGIBLE PERKS survey | `FALLOUT_REGISTRY.perks` (`ui-render-databank.js:356`) | ✅ yes (not named, but folded into "every registry-backed surface") |
| **AI system context** | `databaseCSVs` attached at **`api.js:388-389`** (`systemInstruction`) | ✅ yes |
The one claim I had to chase: the commit says "the AI's own `databaseCSVs` system context." `databaseCSVs` does **not** appear in `api-directive.js` (the directive builder), so at first this looked over-claimed — but `transmitMessage` attaches `databaseCSVs` as a *second* `systemInstruction` part at `api.js:389` ("always present — guaranteed model attention"). Since `databaseCSVs` is the boot-time per-game global, a stale window would feed the AI the wrong game's entire combat DB every message. **Claim holds.**
The blast radius is therefore accurate on the read surfaces, and **under-states** the impact by omitting the `autoImportState` *write/drop* path covered in §2.
---
## (4) The new guard — red→green verified; recommended additional hardening.
**Suite 228 (6 tests) + `render-check.mjs` cross-game section — genuine red→green:**
Reverting `ui-saves.js` to `17e880d` and running the Node runner:
```
✗ 228.1 _applySlotEnvelope() calls window.location.reload()
✗ 228.2 sets window._loadingSave = true before reload
✗ 228.3 persists robco_v8.activeContext = slotCtx
✓ 228.4 still calls loadUI() for the same-game path (unchanged path — correctly stays green)
✗ 228.5 [behavioral] real _applySlotEnvelope() in a vm sandbox: reloads, no in-place loadUI, robco_v8 persisted
✓ 228.6 [behavioral] same-game load applies in place, no reload, no robco_v8 write (unchanged path)
```
Restoring the fix → all 6 green (full runner: **3213/3213**). 228.5 is the load-bearing one — it executes the *real extracted function* against a mocked boot env, so it catches the in-memory stale-global that source-grep can't. The `render-check.mjs` section is a true end-to-end proof (real reload, real `reg_nv.js`/`reg_fo3.js`, both directions, both fresh-boot baselines, and the unregressed same-game fast path). This is a real guard, not a green-only decoration.
**RECOMMENDED (non-blocking) hardening — Protocol 42 / 36b defense-in-depth:** the audit *surfaced* the `autoImportState` filter-and-drop footgun (§2). It is currently unreachable because every cross-game path reboots — but nothing *asserts* that invariant. If any future path ever leaves `state.gameContext` disagreeing with the loaded registry without a reboot, the silent data-drop returns with no guard to catch it. Per Protocol 42 ("a footgun discovered while verifying is treated like a production defect — lock the invariant"), I recommend a follow-up that either (a) guards `autoImportState` so it never drops registry-unvalidated arrays when `FALLOUT_REGISTRY`'s game ≠ `state.gameContext`, or (b) adds a static/behavioral test locking "no in-place cross-game apply exists anywhere." I did not apply this (stage 3 fixes nothing); flagging for the owner to schedule.
---
## Other checks
- **Weakened assertions? NONE.** `tests/robco-diagnostics.js` and `render-check.mjs` are purely additive (`191/0`, `307/0` — zero deletions). `ui-saves.js` is `40/2`; both removed lines are the reworded doc-comment header, no logic dropped. I grepped every changed doc/test for a loosened regex or lowered bound — none.
- **Test-count sync (Protocol 2a):** 3207→3213 everywhere (`CLAUDE.md`, `README.md`, `ARCHITECTURE.md`, `RULES.md`, `CHANGELOG.md` `[Unreleased]` header). Suite 227→228. Drift scan clean (only false positive: a SHA256 literal containing "3211"). `tests/test.html` correctly untouched — this fix doesn't change the import contract it mirrors, so no runtime suite was added.
- **Protocol 1 (cache):** `CACHE_NAME` r26→r27; `ui-saves.js` is a served file — correct.
- **Protocol 22 (extend, don't fork):** the fix reuses the exact `robco_v8` persist-then-reload pattern the other cross-game paths already use, inside the shared `_applySlotEnvelope`. `restoreSlotVersion` routes through the same core, so version-restore inherits the fix for free. No parallel apply path. ✅
- **Protocol 38 (game-agnostic):** the added block branches only on `slotCtx`/`curCtx` variables derived from the envelope/state, with the sanctioned `|| 'FNV'` fail-safe defaults. No game literals. ✅
- **Same-game load still instant:** confirmed by 228.6 behavioral (no reload, no `robco_v8` write) and by the unchanged fall-through `state = {...state, ...env.state}; loadUI()`. ✅
- **CHANGELOG accuracy (Protocol 21):** the entry is true against the running app and correctly scoped ("suggestions and lookups … matches the game you're actually in … both games, either direction"). Plain-English, no code identifiers. ✅
- **Protocol 41 (cleanup):** my scratch harnesses (`_audit_crossgame.mjs`, `_audit_corruption.mjs`) were removed; working tree is clean. The two PNGs under `planning/mockups/fo3/` are gitignored evidence (as the audit directed).
---
## Summary for the next stage
- **The reported bug is genuinely fixed** — reproduced on pre-fix code, resolved on `2210b57`, in a real browser.
- **The "no save corruption" claim is FALSE** — a stale registry demonstrably drops `collectibles`/`traits`/`skillBooks`/`magazines`/`lincolnItems` on the next AI turn (empirically wiped `["Strength","Perception"]` → `[]`). The fix *prevents* this by rebooting, so the shipped code is correct — but the risk was under-reported. Strike the "no corruption" line from the record.
- **Blast radius accurate** on reads; under-stated by omitting the write/drop path.
- **Guard is real** (red→green verified both directions).
- **No weakened assertions; all protocols (1, 2/2a, 13, 21, 22, 38, 41) satisfied.**
- **One recommended follow-up:** defense-in-depth guard on the `autoImportState` registry-mismatch drop (Protocol 42) — non-blocking.
**PASS.**
STAMP · generated for RELEASE v2.8.5 commit 06e5180 (06e51801b38a) · archive input-tree hash c07fbfbdd2e1ddeb · 754 files · no wall-clock timestamp (regenerates identically when nothing changed).