GENERATED VIEW · PRIVATE ARCHIVE · DO NOT SERVE
RobCo Industries · Archive Museum

AUDIT_registry_leak.md



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).