# CLOUD_AUDIT.md — RobCo U.O.S. v2.6.0 > **FILED UNDER 2.8.0 — PRODUCED-BY, NOT MEASURED-VERSION.** This document *measures* the earlier v2.6.0/v2.7.0 > codebase, but it was *produced* as a stage of the PARKED-CHAIN audit sequence — the analysis phase that became > the 2.8.0 overhaul. Planning artifacts are filed by the release whose work produced them, never by the version > they discuss. The version in the title is the code it audited, not the folder it belongs in. > **[ARCHIVE-class snapshot — not current truth.]** Per the 2.8.5 U-B1 doc-maintenance model (see `CLAUDE.md`'s > Reference Pointer Index), this doc is frozen as of the date below and is never updated to track current code. It > audited a codebase roughly 30% smaller than today's (pre-`test-console.js`, pre-OCR, pre-Diagnostic-Shell, > pre-CHASSIS/Overseer). Reuse its structural reasoning as historical input only — re-verify every measurement and > file-split proposal against current code before acting on it. > Stage A8 of the PARKED-CHAIN audit sequence. Final Phase-A audit. PLAN-ONLY. > Nothing here is implemented or committed. > Companion docs: DIEGETIC_AUDIT (A0) · PERFORMANCE_AUDIT (A1) · ACCESSIBILITY_AUDIT (A2) · > CONTENT_AUDIT (A3) · MOBILE_AUDIT (A4) · CODE_QUALITY_AUDIT (A5) · TEST_STRENGTH_AUDIT (A6) · > UI_CONSISTENCY_AUDIT (A7). > Target: bc8c2eb (v2.6.0-r1, 1078 tests / 89 suites). > > Files studied: js/state.js · js/cloud.js · js/api.js · js/ui-saves.js --- ## Executive summary The headline finding is encouraging: **every field in `let state = {}` is captured in the cloud payload** via the `JSON.parse(JSON.stringify(state))` deep copy in `saveCurrentToCloud()`. No state field is silently omitted. The Phase 6 additions (skillBooks, magazines, traits, lincolnItems) are all present. The actual issues lie one layer deeper: 1. **`sanitizeImportedContainer()` does not explicitly process 15 of the 32 state fields** — Phase 6 additions (traits, skillBooks, magazines, lincolnItems, collectibles) pass through the pull path without type-coercion or string validation. Since `escapeHtml()` is applied at render time, XSS is caught downstream, but a corrupted or tampered cloud save could inject unexpected data into these fields on load. 2. **The cloud pull round-trip is NEVER behaviorally tested** — `loadCloudSave` is presence-checked in Suite 46 but never executed in the VM sandbox with a real Firestore document. A regression breaking the sanitize → migrate → apply path would pass all 1078 tests. (A6 cross-finding.) 3. **`robco_panel_state` (panel open/close preferences) is not synced** — when a user loads a cloud save on a new device, all panel preferences are reset to defaults. This is the only user-facing preference that is NOT captured in either the cloud payload or the separate key-sync path. Low-stakes but noteworthy. 4. **`migrateState()` accepts the version string but ignores it** — all migrations are unconditional (field-presence gates, not version gates). Passing `"2.6.0"` or `"1.0"` as the version argument to `migrateState()` makes no difference. This is safe (idempotent) but means there is no mechanism to run a migration only for a specific version range — future migrations that must NOT fire for all old saves have no way to be scoped. --- ## Summary counts | Severity | Count | |---|---| | **CRITICAL** | 0 | | **HIGH** | 2 | | **MEDIUM** | 6 | | **LOW** | 5 | | **SKIP/INFO** | 5 | | **Total findings** | **18** | --- ## SECTION 1 — State Field Coverage (per-field) ### Serialization path `saveCurrentToCloud()` in `js/cloud.js:263`: ```js window.robco_v8.campaigns[window.robco_v8.activeContext] = JSON.parse(JSON.stringify(state)); ``` This is an unrestricted deep copy of the global `state` object. **Every field present in `state` at push time is included in the cloud payload** — no allow-list, no block-list, no filtering. ### Per-field table | Field | Type | Default | In cloud payload | In `sanitizeImportedContainer` | In `autoImportState` | Notes | |---|---|---|---|---|---|---| | `lvl` | int | 1 | ✅ | ✅ explicit coerce | ✅ | | | `xp` | int | 0 | ✅ | ✅ explicit coerce | ✅ | | | `hpCur` | int | 100 | ✅ | ✅ explicit coerce | ✅ | | | `hpMax` | int | 100 | ✅ | ✅ explicit coerce | ✅ | | | `s,p,e,c,i,a,l` | int×7 | 5 | ✅ | ✅ explicit coerce | ✅ clamped 1–10 | | | `caps` | int | 0 | ✅ | ✅ explicit coerce | ✅ | | | `karma` | int | 0 | ✅ | ✅ explicit coerce | ✅ | | | `rads` | int | 0 | ✅ | ✅ explicit coerce | ✅ | | | `ticks` | int | 0 | ✅ | ✅ explicit coerce | ✅ | | | `loc` | string | 'Goodsprings' | ✅ | ✅ `_str()` | ✅ | | | `la,ra,ll,rl,hd` | enum | 'OK' | ✅ | ✅ whitelist | ✅ | | | `inventory` | array | [] | ✅ | ✅ per-item coerce | ✅ | | | `ammo` | object | {} | ✅ | ⚠️ pass-through | ✅ | No coercion of values | | `squad` | array | [] | ✅ | ✅ per-member coerce | ✅ | | | `status` | array | [] | ✅ | ✅ per-effect coerce | ✅ | | | `perks` | array | [] | ✅ | ✅ per-perk name coerce | ✅ | | | `quests` | array | [] | ✅ | ✅ per-quest coerce | ✅ | | | `equipped` | object | {null×3} | ✅ | ✅ per-slot string | ✅ | | | `campaign_notes` | array | [] | ✅ | ✅ string filter | ✅ | | | `factions` | object | `_buildFactions()` | ✅ | ⚠️ pass-through | ✅ | Object of `{fame,infamy}` pairs not coerced | | `skills` | object | {all 13: 15} | ✅ | ⚠️ pass-through | ✅ clamped 0–100 | Values not coerced in sanitize | | `collectibles` | array | [] | ✅ | ⚠️ pass-through | ✅ registry-validated | Strings not sanitized on pull | | `lincolnItems` | object | {} | ✅ | ⚠️ pass-through | ✅ registry+vocab-validated | **Phase 6** — no sanitize | | `traits` | array | [] | ✅ | ⚠️ pass-through | ✅ registry-validated | **Phase 6** — no sanitize | | `skillBooks` | array | [] | ✅ | ⚠️ pass-through | ✅ registry-validated | **Phase 6** — no sanitize | | `magazines` | array | [] | ✅ | ⚠️ pass-through | ✅ registry-validated | **Phase 6** — no sanitize | | `locationHistory` | array | [] | ✅ | ⚠️ pass-through | N/A auto-generated | Not AI-mutable; can grow unboundedly | | `stats` | object | {kills:0,...} | ✅ | ⚠️ pass-through | ✅ additive | `sessionStart: Date.now()` restores old timestamp | | `campaignMode` | enum | 'standard' | ✅ | ⚠️ pass-through | ✅ enum-gated | | | `playthroughType` | enum | 'standard' | ✅ | ⚠️ pass-through | ✅ enum-gated | | | `mapView` | enum | 'auto' | ✅ | ⚠️ pass-through | ✅ enum-gated | | | `gameContext` | enum | 'FNV' | ✅ | ⚠️ pass-through | ❌ blocked by security guard | AI cannot mutate gameContext | **Verdict: ZERO fields are missing from the cloud payload.** All 32 state fields (counting SPECIAL as 7) are captured via the deep copy. The Phase 6 additions (traits, skillBooks, magazines, lincolnItems) are all present. ✓ --- ## SECTION 2 — `sanitizeImportedContainer()` Completeness **Location:** `js/api.js:289–376` `sanitizeImportedContainer()` runs on every cloud load (`loadCloudSave:511`) and file import (`ui-saves.js:262,319`). It applies `_sanitizeState()` to every campaign in the container. ### Fields explicitly sanitized: `lvl, xp, hpCur, hpMax, s, p, e, c, i, a, l, caps, rads, karma, ticks` (int coerce) · `la, ra, ll, rl, hd` (enum whitelist) · `loc` (string) · `campaign_notes` (string per entry) · `quests` (name/status/objective coerce) · `inventory` (name/qty/wgt/val coerce) · `squad` (name/hp/hpMax coerce) · `perks` (name coerce) · `status` (name/ticks/type coerce) · `equipped` (weapon/armor/headgear string) ### Fields that pass through unsanitized: `factions`, `skills`, `ammo`, `collectibles`, `lincolnItems`, `traits`, `skillBooks`, `magazines`, `locationHistory`, `stats`, `campaignMode`, `playthroughType`, `mapView`, `gameContext` --- ### CC-SAN-1 · [HIGH] · Phase 6 array fields not sanitized in `sanitizeImportedContainer` **Affected fields:** `traits`, `skillBooks`, `magazines`, `lincolnItems`, `collectibles` These are the five Phase 6 tracker fields. On cloud load, they pass through `sanitizeImportedContainer` without any type coercion or string validation. If a cloud save were tampered (or corrupted), these fields could contain arbitrary strings, objects, or non-string values. The downstream protection: - `autoImportState` validates them against the registry on the AI import path — but `autoImportState` is NOT called on cloud load. Cloud load writes directly to localStorage and reloads. - `render*()` functions use `escapeHtml()` at display time — XSS is caught here. - `migrateState()` calls `s.lincolnItems` normalization but does not sanitize string content. **Net risk:** A corrupted or tampered `traits` entry like `"<img src=x onerror=alert(1)>"` would be loaded into `state.traits`, written to localStorage, and only escaped at render. Firestore security rules restrict writes to the authenticated uid — so this is only possible via a tampered cloud document, not a cross-user attack. But it IS possible if the Firestore document is modified outside the app (e.g., via Firebase console by the user themselves, or a compromised session). **Fix — extend `_sanitizeState()` in `api.js:296`:** ```js // Add after the existing equipped block: if (Array.isArray(o.traits)) o.traits = o.traits.filter(t => t != null).map(_str); if (Array.isArray(o.skillBooks)) o.skillBooks = o.skillBooks.filter(t => t != null).map(_str); if (Array.isArray(o.magazines)) o.magazines = o.magazines.filter(t => t != null).map(_str); if (o.lincolnItems && typeof o.lincolnItems === 'object' && !Array.isArray(o.lincolnItems)) { const vocab = ['found', 'hannibal', 'leroy', 'washington']; o.lincolnItems = Object.fromEntries( Object.entries(o.lincolnItems) .filter(([, v]) => vocab.includes(v)) .map(([k, v]) => [_str(k), v]) ); } if (Array.isArray(o.collectibles)) o.collectibles = o.collectibles.filter(t => t != null).map(_str); if (o.ammo && typeof o.ammo === 'object') { o.ammo = Object.fromEntries( Object.entries(o.ammo).map(([k, v]) => [_str(k), Math.max(0, parseInt(v) || 0)]) ); } if (o.skills && typeof o.skills === 'object') { o.skills = Object.fromEntries( Object.entries(o.skills).map(([k, v]) => [k, Math.min(100, Math.max(0, parseInt(v) || 0))]) ); } ``` **Protocol 13** — this is a bug class that currently has no regression test. Add a test in Suite 44 or a new suite: ```js // CC-SAN-1-T: sanitizeImportedContainer coerces traits/skillBooks/magazines to strings const container = { campaigns: { FNV: { traits: ['<script>alert(1)</script>', 42, null, 'Wild Wasteland'], skillBooks: [{ obj: 'injection' }, 'Wasteland Survival Guide'], collectibles: [null, '<img src=x>', 'Goodsprings Snowglobe'], ammo: { '5mm': '999' } }}}; const sanitized = sanitizeImportedContainer(container); const fNV = sanitized.campaigns.FNV; assert(!fNV.traits.some(t => typeof t !== 'string'), 'traits coerced to strings'); assert(!fNV.traits.some(t => t === null), 'null traits filtered'); assert(!fNV.skillBooks.some(t => typeof t !== 'string'), 'skillBooks coerced to strings'); assert(fNV.ammo['5mm'] === 999, 'ammo values coerced to int'); ``` **File touches:** `js/api.js`, `tests/check-persistence.js`, `tests/check-persistence.ps1` **Verdict:** BUILD-NOW · **HIGH** --- ### CC-SAN-2 · [MEDIUM] · `factions` object values not coerced — fame/infamy could be strings `sanitizeImportedContainer` passes `factions` through without coercion. A save with `{ factions: { ncr: { fame: "100", infamy: "0" } } }` would load with string fame/infamy values. `adjustFaction()` then does `Math.max(0, Math.min(1000, cur + delta))` — but if `cur` is `"100"`, then `"100" + 5 === "1005"` (string concatenation), not `105`. **Fix — extend `_sanitizeState()`:** ```js if (o.factions && typeof o.factions === 'object') { o.factions = Object.fromEntries( Object.entries(o.factions).map(([k, v]) => [ k, { fame: Math.max(0, parseInt((v && v.fame) || 0) || 0), infamy: Math.max(0, parseInt((v && v.infamy) || 0) || 0), } ]) ); } ``` **File touches:** `js/api.js` **Verdict:** BUILD-NOW (7 lines) · **MEDIUM** --- ## SECTION 3 — Round-Trip Integrity ### Push → Pull path ``` saveCurrentToCloud() └─ JSON.parse(JSON.stringify(state)) → full deep copy └─ addDoc(col, { robco_v8, chat, playstyle, contentHash, ... }) loadCloudSave(docId) └─ confirm gate └─ getDoc(docRef) └─ verifySaveEnvelope(data) → version + checksum check └─ snapRollingBackup() → backup current state first └─ sanitizeImportedContainer(data.robco_v8) └─ migrateState(data.version, campaign) → per game context └─ localStorage.setItem('robco_v8', ...) └─ window.location.reload() ``` **Assessment:** The path is structurally complete. Every critical step is present and in the correct order. ✓ --- ### CC-RT-1 · [HIGH] · Cloud pull round-trip is NEVER behaviorally tested — A6 gap **Cross-reference:** A6 TEST_STRENGTH_AUDIT TS-GAP-5/6 Suite 46 (cloud saves) is 100% PRESENCE — it checks that strings like `sanitizeImportedContainer`, `migrateState`, `addDoc` appear in function bodies. `loadCloudSave` is never called in the VM sandbox with a mock Firestore document. The following behaviors are untested: - That `sanitizeImportedContainer` is actually called (vs. the `typeof ... === 'function'` guard short-circuiting silently) - That `migrateState` is called for EACH campaign context, not just the active one - That all Phase 6 fields survive the round-trip unchanged - That the `verifySaveEnvelope` confirm gate fires on checksum mismatch **Proposed behavioral test (Suite 46-B — both runners):** ```js // CC-RT-1: Full push→sanitize→migrate round-trip preserves Phase 6 fields // Build a mock Firestore document with v2.4.0 data (pre-magazines): const mockDoc = { version: '2.4.0', robco_v8: { activeContext: 'FNV', campaigns: { FNV: { lvl: 5, traits: ['Wild Wasteland'], skillBooks: ['Wasteland Survival Guide'], collectibles: ['Goodsprings Snowglobe'], // magazines deliberately absent (v2.4.0 pre-magazines) } } }, chat: [], playstyle: 'any', }; const sanitized = sanitizeImportedContainer(mockDoc.robco_v8); const migrated = migrateState(mockDoc.version, sanitized.campaigns['FNV']); // After migration: magazines field added by migrateState assert(Array.isArray(migrated.magazines), 'magazines added by migration from v2.4.0'); // Phase 6 fields preserved assert(migrated.traits[0] === 'Wild Wasteland', 'traits preserved in round-trip'); assert(migrated.skillBooks[0] === 'Wasteland Survival Guide', 'skillBooks preserved'); assert(migrated.collectibles[0] === 'Goodsprings Snowglobe', 'collectibles preserved'); ``` **File touches:** `tests/check-persistence.js`, `tests/check-persistence.ps1` **Verdict:** BUILD-NOW · **HIGH** --- ### CC-RT-2 · [MEDIUM] · `migrateState()` ignores its version parameter — unconditional migrations only **Location:** `js/state.js` — `migrateState(version, s)` function signature accepts `version` but never uses it in conditionals. All migration guards are field-presence checks (`if (!s.field)`). **Why this matters for the cloud audit:** When a cloud save with `version: "2.4.0"` is loaded, `migrateState("2.4.0", campaign)` is called. It correctly adds `magazines: []` because that field is absent. ✓ But if a future migration must fire ONLY for saves made between v2.3.0 and v2.5.0 (not for v1.x saves where the field has a different meaning, and not for current saves where it's already correct), there's no mechanism to scope it. The migration would fire for ALL saves lacking that field regardless of version. **Fix:** This is a SPEC-FIRST architecture decision — changing the migration to be version-gated would require a SCHEMA_VERSION constant and explicit version comparison. Minimal fix: document the constraint in a comment and in ARCHITECTURE.md. Add a `SCHEMA_VERSION` constant for future use. **Verdict:** SPEC-FIRST (architecture decision, safe as-is for current migrations) · **MEDIUM** --- ### CC-RT-3 · [MEDIUM] · `stats.sessionStart` is a dynamic timestamp — restores old value on cloud load **Location:** `js/state.js:17` — `stats: { kills: 0, capsEarned: 0, damageDealt: 0, sessionStart: Date.now() }` The `sessionStart` field records when the campaign session began. When a cloud save is loaded, `sessionStart` is restored to the value from when the save was made (e.g., weeks ago). The Session Statistics panel then shows time elapsed since the OLD save, not since this load. **Assessment:** This is likely intentional — session stats are per-campaign, tracking the full lifetime of the playthrough. But if the intent is "time since app was opened," the behavior is wrong. **Fix (if re-start is desired):** In the reload path after `loadCloudSave`, reset `stats.sessionStart` to `Date.now()` before `window.location.reload()`. Or: reset it in `loadUI()` on first boot. **Verdict:** SKIP (intentional per-campaign stats design) or BUILD-NOW if the owner wants per-session timing · **LOW** --- ### CC-RT-4 · [LOW] · `chat` history restores without validation on cloud load **Location:** `js/cloud.js:520-521`: ```js if (data.chat && Array.isArray(data.chat)) localStorage.setItem('robco_chat', JSON.stringify(data.chat)); ``` The chat history is restored directly from the Firestore document without sanitization. Chat entries from the AI are already escaped by `appendToChat()` at write time, so the stored content is expected to be safe. But a malformed cloud save (manually edited Firestore document) could inject HTML into the chat display. **Fix (low priority):** Add chat sanitization alongside sanitizeImportedContainer — filter non-object entries, coerce `text` and `role` fields to strings. **Verdict:** BUILD-NOW (minor addition to cloud load path, completes the sanitization story) · **LOW** --- ## SECTION 4 — Additive / Confirm-Gate Invariants (Protocol 34) | Operation | Firestore method | Confirm gate | Assessment | |---|---|---|---| | Save current to cloud (`saveCurrentToCloud`) | `addDoc` | `prompt()` for label only | ✅ Additive; non-destructive; prompt is for UX, not safety | | Sync local saves (`syncLocalSavesToCloud`) | `addDoc` | None | ✅ Additive; batch upload of local saves; no confirm needed per P34 | | Load cloud save (`loadCloudSave`) | `getDoc` (read) then `localStorage.setItem` (local write) | `confirm()` ✓ | ✅ Destructive (replaces current state); properly confirm-gated | | Delete cloud save (`deleteCloudSave`) | `deleteDoc` | `confirm()` ✓ | ✅ Destructive; properly confirm-gated | | Rename cloud save (`renameCloudSave`) | `updateDoc` (label + updatedAt only) | None | ✅ Non-destructive partial update; no confirm needed | **All Protocol 34 invariants hold.** No blind overwrites. All destructive operations are confirm-gated. ✓ --- ### CC-P34-1 · [INFO] · `loadCloudSave` timestamp warning requires TWO confirms for older saves **Location:** `js/cloud.js:471-483` When loading a cloud save older than the last local push, the user sees: 1. `confirm()` — "Load this cloud save?" (the main gate) 2. `confirm()` — "This cloud save is older than your last local push. Load anyway?" (the timestamp warning) These two confirms are in the correct order (main gate first, then warning). But the warning fires EVEN IF the user already made an informed decision. For clarity, the timestamp warning should be part of the first confirm message, not a second gate. **Verdict:** SKIP (two-confirm pattern is safe, just slightly noisy UX) · **INFO** --- ## SECTION 5 — Checksum, Version, and Rolling Backup Correctness ### Checksum `computeSaveChecksum(robco_v8, chat, playstyle)` computes FNV-1a 32-bit hash over recursively-sorted JSON of all three inputs. Included in cloud document as `contentHash`. ✓ **What the checksum covers:** - Full `robco_v8` container (all campaigns, all state fields) ✓ - Chat history ✓ - Weapon constraint playstyle (`robco_playstyle` localStorage key) ✓ **What the checksum does NOT cover:** - `robco_panel_state` (panel preferences) — intentional, this is a UI preference - `robco_gemini_key`, `robco_gemini_model` — synced separately via key-sync path - Feature flags (`robco_feature_flags`) — server-side config, not a save --- ### CC-CHK-1 · [LOW] · Checksum dedup uses a 32-bit FNV-1a hash — collision risk exists but is low **Location:** `js/state.js:30-38` FNV-1a 32-bit produces a 32-bit (4-byte) integer hash space — approximately 4.3 billion values. The probability of a collision between two distinct saves is ~1 in 4.3 billion per save pair. For a player with 10 cloud saves, the collision probability is negligible. **No action needed.** This is a dedup mechanism, not a security mechanism. A hash collision means a save is not uploaded (a no-op false dedup), not that data is lost. ✓ **Verdict:** SKIP · **INFO** --- ### Rolling Backup `snapRollingBackup()` captures `robco_v8` + `chat` + `playstyle` in a 3-slot ring before every destructive load. Called at `cloud.js:508` before `loadCloudSave` applies the cloud data. **Correctness:** ✓ The backup fires BEFORE the localStorage overwrite. ✓ The backup includes the full state container. ✓ Dedup prevents redundant backups on repeated loads. ### CC-BACK-1 · [MEDIUM] · Rolling backup is NOT snapped before `syncLocalSavesToCloud` **Location:** `js/cloud.js:343` — `syncLocalSavesToCloud()` uploads local saves to cloud but does NOT call `snapRollingBackup()` before starting. This is a sync-to-cloud operation (not a replace-local operation), so the local state is not modified — no backup needed. ✓ But `syncLocalSavesToCloud()` reads from `localStorage.getItem('robco_v8')` directly, not from `state`. If a save migration occurs mid-sync (unlikely), the local state could diverge from what was uploaded. This is an edge case. **Verdict:** SKIP (upload-only operations don't need a backup of local state) · **INFO** --- ## SECTION 6 — Per-Game-Context Isolation ### How the container works ```js window.robco_v8 = { activeContext: 'FNV', // or 'FO3' campaigns: { FNV: { ...full FNV state... }, FO3: { ...full FO3 state... }, // only present if user has played FO3 } } ``` **Push isolation:** `saveCurrentToCloud()` updates only the active context's campaign before uploading: ```js window.robco_v8.campaigns[window.robco_v8.activeContext] = JSON.parse(JSON.stringify(state)); ``` The OTHER context's campaign in `robco_v8` (from the last `saveState()` call in that context) is uploaded as-is. ✓ **Pull isolation:** `loadCloudSave()` applies `migrateState()` to EACH campaign separately: ```js Object.keys(sanitized.campaigns).forEach(ctx => { sanitized.campaigns[ctx] = migrateState(data.version, sanitized.campaigns[ctx]); }); ``` Both contexts are migrated; neither overwrites the other. ✓ --- ### CC-ISO-1 · [MEDIUM] · FO3-only fields can appear in an FNV campaign state if the AI sends them **Background:** `autoImportState()` validates `lincolnItems` against the FO3 Lincoln registry, and `traits` against the FNV trait registry. These guards are correct on the AI path. **On the cloud path:** If a cloud save was made with `state.lincolnItems` populated (FO3 campaign) and then the user switches to FNV and loads the same cloud save, `lincolnItems` will appear in the FNV campaign state. The `renderLincolnMemorabilia()` function guards against this with `if (!_activeDef().tracksLincoln) return`, so the UI won't display it. But the field IS in state for the FNV context. **Assessment:** Not a data corruption risk (the renderer ignores the field for the wrong context), but a state purity concern. On game switch, `_updateContextPanels()` hides the relevant panels but does not purge cross-context fields from state. **Fix (optional):** Clear cross-context fields on `onGameContextChange()`: ```js // When switching to FNV: state.lincolnItems = {}; // When switching to FO3: state.traits = []; state.magazines = []; ``` **Verdict:** SKIP (UI correctly ignores cross-context fields; purging would cause data loss if the user switches back) · **MEDIUM → SKIP** --- ### CC-ISO-2 · [LOW] · FNV default factions appear in a cloud save made during an FO3 playthrough `_buildFactions()` creates the same initial faction set regardless of game context. An FO3-only player's cloud save will have NV factions (NCR, Legion, etc.) with zeroed fame/infamy. These are invisible in the UI (FO3 uses Karma Center, not the faction grid) but bloat the state object. **Assessment:** Cosmetic bloat, not functional. ✓ **Verdict:** SKIP · **LOW** --- ## SECTION 7 — Out-of-State localStorage Keys These keys are stored in localStorage but NOT in `let state = {}` — therefore NOT in the cloud payload's `robco_v8` field. | Key | Purpose | Synced to cloud? | Assessment | |---|---|---|---| | `robco_v8` | Main save container (IS the cloud payload) | ✓ IS the payload | | | `robco_chat` | Chat history | ✓ via `chat` field | | | `robco_playstyle` | Weapon constraint (any/melee) | ✓ via `playstyle` field | | | `robco_gemini_key` | API key | ✓ via `saveGeminiKeyToCloud` path | | | `robco_gemini_model` | Selected Gemini model | ✓ via `saveGeminiKeyToCloud` (model included) | | | `robco_gemini_key_sync` | Key sync toggle | ✗ not synced | See CC-LS-1 | | `robco_panel_state` | Panel open/close preferences | ✗ not synced | See CC-LS-2 | | `robco_last_cloud_push` | Timestamp of last cloud push | ✗ not synced | Intentional (ephemeral) ✓ | | `robco_feature_flags` | LKG feature flags | ✗ not synced | Intentional (server-controlled) ✓ | | `robco_backup_1/2/3` | Rolling backups | ✗ not synced | Intentional (local-only emergency restore) ✓ | | `robco_backup_ptr` | Ring pointer | ✗ not synced | Intentional ✓ | | `robco_playstyle_type` | Legacy key (migrated to `state.playthroughType`) | ✗ but not needed | Migration captures it into state ✓ | --- ### CC-LS-1 · [LOW] · `robco_gemini_key_sync` toggle is NOT synced to cloud When a user signs in on a new device and loads a cloud save, the Gemini key syncs correctly (via `loadGeminiKeyFromCloud`). But the key-sync toggle (`robco_gemini_key_sync: 'true'`) is not synced — the user must re-enable it manually on the new device. **Fix:** Add `robco_gemini_key_sync` to the `saveGeminiKeyToCloud`/`loadGeminiKeyFromCloud` Firestore document (already at `users/{uid}/keystore`). Minor addition: ```js // In saveGeminiKeyToCloud(): keySync: localStorage.getItem('robco_gemini_key_sync') === 'true', // In loadGeminiKeyFromCloud(): if (typeof data.keySync === 'boolean') localStorage.setItem('robco_gemini_key_sync', data.keySync ? 'true' : 'false'); ``` **File touches:** `js/cloud.js` **Verdict:** BUILD-NOW (4-line change) · **LOW** --- ### CC-LS-2 · [MEDIUM] · `robco_panel_state` is NOT synced — panel preferences lost on cloud restore When a user loads a cloud save (or uses a new device), all panel open/close preferences are reset to defaults (desktop: all open; mobile: all closed). The user must re-collapse panels they prefer collapsed. This is the only user-facing preference that is NOT captured in either the cloud payload or the Gemini key-sync path. **Options:** 1. **Include in the cloud document** — add `panelState: localStorage.getItem('robco_panel_state')` to the Firestore document at push time, restore at load time. Minimal addition. 2. **Separate device-level sync** — panel preferences are device-specific (desktop open, mobile closed may differ by device intentionally). Treat as non-syncable. Option 2 is arguably correct — panel state may appropriately differ between a phone and a desktop. The current behavior (reset to defaults) is reasonable for new-device loads. **Verdict:** SPEC-FIRST (intentional or device-specific preference; needs owner decision) · **MEDIUM** --- ## SECTION 8 — `autoImportState()` vs Cloud Load Comparison These are two independent paths to state modification: | Aspect | AI path (`autoImportState`) | Cloud path (`loadCloudSave`) | |---|---|---| | Trigger | AI response to user message | User clicks "LOAD" from cloud picker | | Validation | Field-by-field; registry-validated | `sanitizeImportedContainer` (less granular) | | Migration | N/A (AI always sends current schema) | `migrateState(version, campaign)` | | Destructive check | Non-destructive (merges into live state) | Confirm gate (full replace) | | Backup | `window._lastStateBeforeSync` (single snapshot) | `snapRollingBackup()` (3-slot ring) | | Reload | No reload | `window.location.reload()` | | `gameContext` | Blocked (security guard) | Preserved from cloud document | | `campaignMode` | Accepted but enum-gated | Pass-through (no coercion) | | `traits/skillBooks/magazines` | Registry-validated | Pass-through (CC-SAN-1) | | Stats | ADDITIVE (increments existing values) | RESTORATIVE (replaces with saved values) | **The stats additivity asymmetry is intentional and correct:** The AI updates stats as events happen (kill = +1), while cloud restore recovers the cumulative state at save time. ✓ --- ## SECTION 9 — Proposed Protocol (for RULES.md / CLAUDE.md) ### Protocol Cloud-1 — Every New Feature Must Declare Cloud-Sync Impact When adding any new state field or feature that stores user data: **Case A — New field added to `let state = {}`:** The field is automatically captured in the cloud payload via `JSON.parse(JSON.stringify(state))`. But THREE additional steps are mandatory in the SAME commit: 1. **`migrateState()`** — add a field-presence guard: `if (!s.newField) s.newField = defaultValue;` 2. **`autoImportState()`** — add field mapping if the AI should be able to update it; OR add an explicit comment explaining why it's excluded 3. **`sanitizeImportedContainer()`** — if the field contains user-controlled strings, add explicit type coercion/whitelist in `_sanitizeState()` **Case B — New localStorage key (not in `state`):** The field is NOT captured automatically. Evaluate explicitly: - Is this a per-device preference? → Document as "intentionally not synced — device preference" - Is this a per-user/per-campaign preference? → Add to the `saveCurrentToCloud` payload AND restore in `loadCloudSave` - Is this a security secret? → Use the `saveGeminiKeyToCloud`/`loadGeminiKeyFromCloud` pattern **Case C — Feature that only mutates already-synced fields:** Document in the commit message: "Cloud impact: none — modifies existing synced field `X`." **The commit message must include one of the above declarations.** Silence is not acceptable — a new feature with no cloud-impact declaration is a protocol violation. **Checklist for any new state field:** - [ ] `let state = {}` default value added - [ ] `migrateState()` guard added - [ ] `autoImportState()` mapping added (or exclusion documented) - [ ] `sanitizeImportedContainer()` coercion added (if user-controlled strings) - [ ] Cloud-sync impact declared in commit message --- ## SECTION 10 — Proposed Gate Guard Add to **both** `tests/check-persistence.js` and `tests/check-persistence.ps1` as a new test within an existing suite (e.g., extend Suite 12 or Suite 46) or a new Suite 88 (if the UI Consistency gate guards are Suite 88): --- ### GATE-CC-1 — Every field in `let state = {}` is referenced in `migrateState()` ```js // Extract field names from state default object const STATE_FIELDS = [ 'lvl', 'xp', 'hpCur', 'hpMax', 's', 'p', 'e', 'c', 'i', 'a', 'l', 'caps', 'loc', 'rads', 'karma', 'ticks', 'la', 'ra', 'll', 'rl', 'hd', 'factions', 'skills', 'status', 'inventory', 'squad', 'campaign_notes', 'perks', 'quests', 'equipped', 'ammo', 'stats', 'locationHistory', 'gameContext', 'collectibles', 'lincolnItems', 'traits', 'skillBooks', 'magazines', 'campaignMode', 'playthroughType', 'mapView', ]; const migrateSrc = extractFunction(stateSrc, 'migrateState'); // Fields that are in the default state but don't need migration guards // (primitive fields that have been present since v1.0 and always exist): const MIGRATION_EXEMPT = ['lvl', 'xp', 'hpCur', 'hpMax', 's', 'p', 'e', 'c', 'i', 'a', 'l', 'caps', 'loc', 'rads', 'karma', 'ticks', 'la', 'ra', 'll', 'rl', 'hd', 'stats']; const needsGuard = STATE_FIELDS.filter(f => !MIGRATION_EXEMPT.includes(f)); needsGuard.forEach(field => { assert(migrateSrc.includes(`s.${field}`) || migrateSrc.includes(`'${field}'`), `migrateState references field: ${field}`); }); ``` **Purpose:** Catches the case where a new field is added to `let state = {}` but forgotten in `migrateState()`. Old saves would load with `undefined` for that field and any code that does `state.newField.push(...)` would throw. --- ### GATE-CC-2 — Every non-exempt state field is referenced in `autoImportState()` OR has an explicit exclusion comment ```js const AUTO_IMPORT_EXCLUSIONS = ['gameContext', 'mapView', '_lastSaveStr']; // documented exclusions const autoImportSrc = extractFunction(apiSrc, 'autoImportState'); const needsAutoImport = STATE_FIELDS.filter(f => !AUTO_IMPORT_EXCLUSIONS.includes(f)); needsAutoImport.forEach(field => { assert(autoImportSrc.includes(`state.${field}`) || autoImportSrc.includes(`'${field}'`), `autoImportState references or handles field: ${field}`); }); ``` **Purpose:** Catches the case where a new field is added to state but never mapped in `autoImportState()`. The AI would then never be able to update it, silently. This is fine if intentional (e.g., `campaignMode` is UI-only) but must be documented. --- ### GATE-CC-3 — `sanitizeImportedContainer` references all Phase 6 array fields (after CC-SAN-1 fix) ```js const sanitizeSrc = extractFunction(apiSrc, 'sanitizeImportedContainer'); const PHASE6_FIELDS = ['traits', 'skillBooks', 'magazines', 'lincolnItems', 'collectibles', 'ammo', 'skills', 'factions']; PHASE6_FIELDS.forEach(f => { assert(sanitizeSrc.includes(`o.${f}`) || sanitizeSrc.includes(`'${f}'`), `sanitizeImportedContainer handles field: ${f}`); }); ``` **Purpose:** Ratchet guard — once CC-SAN-1 is fixed, this gate ensures the sanitization is never removed in a refactor. Must be added in the SAME commit as the CC-SAN-1 fix. --- ### GATE-CC-4 — Cloud push path uses `addDoc`, never `setDoc` with merge:false ```js const cloudSrc = readFile('js/cloud.js'); // setDoc without merge would overwrite existing documents — Protocol 34 violation assert(!/setDoc\s*\([^)]*\)/.test(cloudSrc) || /setDoc.*merge:\s*true/.test(cloudSrc), 'cloud.js uses no blind setDoc — all writes are addDoc or updateDoc'); ``` **Purpose:** Permanently encodes the Protocol 34 additive-write invariant in the gate. A future refactor that accidentally introduces `setDoc(ref, data)` (without `{merge:true}`) would fail the build. --- ## SECTION 11 — Prioritized Build Backlog ### Tier 1 — Correctness and sanitization | ID | Finding | Severity | Fix | Files | |---|---|---|---|---| | CC-SAN-1 | Phase 6 fields not sanitized in sanitizeImportedContainer | HIGH | ~15 lines in `_sanitizeState()` | `js/api.js` + both test runners | | CC-RT-1 | Cloud pull round-trip never behaviorally tested | HIGH | New behavioral test in Suite 46 | Both test runners | | CC-SAN-2 | Factions not coerced — fame/infamy string concatenation risk | MEDIUM | 7 lines in `_sanitizeState()` | `js/api.js` | | CC-RT-4 | Chat history not sanitized on cloud load | LOW | Filter + coerce chat entries | `js/cloud.js` | ### Tier 2 — Preferences and gate guards | ID | Finding | Severity | Fix | Files | |---|---|---|---|---| | CC-LS-1 | `robco_gemini_key_sync` toggle not synced | LOW | 4 lines in `saveGeminiKeyToCloud`/`loadGeminiKeyFromCloud` | `js/cloud.js` | | CC-LS-2 | `robco_panel_state` not synced (needs owner decision) | MEDIUM | Include in cloud document or document as device-preference | `js/cloud.js` | | CC-RT-2 | `migrateState` ignores version parameter | MEDIUM | Add `SCHEMA_VERSION` constant; document constraint | `js/state.js` | | GATE-CC-1 | State field coverage guard | — | New test in Suite 46 or 88 | Both test runners | | GATE-CC-2 | autoImportState coverage guard | — | New test in Suite 46 or 88 | Both test runners | | GATE-CC-3 | sanitizeImportedContainer coverage guard | — | New test (add with CC-SAN-1 fix) | Both test runners | | GATE-CC-4 | addDoc-only invariant guard | — | New test in Suite 48 or 46 | Both test runners | ### Spec-First | ID | Finding | Verdict | |---|---|---| | CC-RT-2 | Version-gated migrations | SPEC-FIRST — requires architecture decision | | CC-LS-2 | Panel state sync | SPEC-FIRST — requires owner decision on device vs user preference | --- ## Constraint Checklist - **[DATA]** All proposed additions to `saveCurrentToCloud` and `loadCloudSave` must remain additive (use `addDoc`/`updateDoc`) and confirm-gated for destructive operations (Protocol 34) - **[AUTH]** popup-only (Protocol 30), `signInAnonymously` guarded (Protocol 31) - **[NET]** kill-switch fail-safe (Protocol 33) — `loadRemoteConfig` already fail-open ✓ - **[CACHE]** CC-SAN-1 modifies `js/api.js` (served file) → requires `CACHE_NAME` bump - **[TEST]** All gate guards (CC-1 through CC-4) require both-runner parity + Protocol 2a count sync in the same commit - **[PARITY]** Protocol 15 — every new test added to JS runner must have an exact-count equivalent in PS1 - **[NEVER]** Never bump APP_VERSION for cloud-only or test-only changes --- *End of CLOUD_AUDIT.md — final Phase-A planning audit*