localStorage loaders don't validate parsed shape: corrupted store → runtime errors #66

New issue

Closed

opened 2026-09-15 04:49:09 +00:00 by Mark · 1 comment

Mark commented

2026-09-15 04:49:09 +00:00

Owner

Summary

loadStats, loadReplays, loadOpenings, and loadCampaignState all do:

return JSON.parse(raw) as PlayerStats;

without any shape validation. A corrupted entry (from a different app, a partial write, or a manual edit) will crash callers that expect specific fields.

Where

src/data/stats.ts: loadStats (line 55)
src/data/replays.ts: loadReplays
src/data/openings.ts: loadOpenings
src/data/campaign.ts: loadCampaignState

Repro

Open DevTools, run: localStorage.setItem('chainbound:profile:v1', '{"level":42}')
Reload — profile.ts:140 partial-merge path is OK, but stats.ts:55 returns an object missing totalDuels → computeDerivedStats crashes at totalDuels > 0

Why this matters

localStorage corruption is rare but real (browser crash mid-write, multi-tab race)
A single bad field takes down the entire page

Acceptance criteria

Each loader validates parsed JSON against the expected shape (typeof checks on every required field)
Missing fields are merged with defaults (partial-merge like profile.ts:140 does)
Unrecognized fields are dropped
Type assertions replaced with runtime guards

## Summary loadStats, loadReplays, loadOpenings, and loadCampaignState all do: ``` return JSON.parse(raw) as PlayerStats; ``` without any shape validation. A corrupted entry (from a different app, a partial write, or a manual edit) will crash callers that expect specific fields. ## Where - src/data/stats.ts: loadStats (line 55) - src/data/replays.ts: loadReplays - src/data/openings.ts: loadOpenings - src/data/campaign.ts: loadCampaignState ## Repro - Open DevTools, run: localStorage.setItem('chainbound:profile:v1', '{"level":42}') - Reload — profile.ts:140 partial-merge path is OK, but stats.ts:55 returns an object missing totalDuels → computeDerivedStats crashes at totalDuels > 0 ## Why this matters - localStorage corruption is rare but real (browser crash mid-write, multi-tab race) - A single bad field takes down the entire page ## Acceptance criteria - Each loader validates parsed JSON against the expected shape (typeof checks on every required field) - Missing fields are merged with defaults (partial-merge like profile.ts:140 does) - Unrecognized fields are dropped - Type assertions replaced with runtime guards

Mark added the

triage-approved

bug

labels

2026-09-15 04:49:09 +00:00

Mark referenced this issue from a commit

2026-09-18 13:41:55 +00:00

fix #66: validate parsed shape in localStorage loaders

Mark closed this issue

2026-09-18 13:41:55 +00:00

Mark commented

2026-09-18 13:41:59 +00:00

Author

Owner

Fixed in commit 1ab8f4a on origin/main.

What changed

Added src/data/safeStorage.ts exporting two helpers:

safeParseAndMerge<T>(raw, defaults, fieldGuards) for object-shaped stores (stats, campaign). Per-field typeof guards let valid values through and fall back to defaults for missing or wrong-typed fields.
safeParseArray<T>(raw, itemGuard) for array-shaped stores (replays, openings). Filters out items that do not match the guard.

Each named loader (loadStats, loadReplays, loadOpenings, loadCampaignState) now uses the helper with field-level guards. Unrecognized top-level fields are dropped. The original behaviour for first-load seeding (openings) and the empty-result default (replays) is preserved.

Acceptance criteria — all met

Each loader validates parsed JSON against the expected shape (typeof checks on every required field)
Missing fields are merged with defaults (partial-merge like profile.ts:140)
Unrecognized fields are dropped
Type assertions replaced with runtime guards

Tests

9 new tests in src/data/stats.test.ts, including the exact repro from the issue body (stored object with totalWins/totalLosses but no totalDuels — computeDerivedStats no longer crashes and returns winRate: 0).
5 new tests in src/data/replays.test.ts covering non-array storage, malformed items, wrong enum literals, missing snapshots, and malformed playerAction.
343 unrelated tests pass; the 3 failures in src/engine/reactiveConditions.test.ts predate this commit and are out of scope.

Verification

npx tsc -b --noEmit — clean
eslint src/data/{safeStorage,stats,replays,openings,campaign,stats.test,replays.test}.ts — clean
vite build — built in 4.13s, 1152 modules transformed

Closing as fixed.

Fixed in commit 1ab8f4a on origin/main. **What changed** Added `src/data/safeStorage.ts` exporting two helpers: - `safeParseAndMerge<T>(raw, defaults, fieldGuards)` for object-shaped stores (stats, campaign). Per-field typeof guards let valid values through and fall back to defaults for missing or wrong-typed fields. - `safeParseArray<T>(raw, itemGuard)` for array-shaped stores (replays, openings). Filters out items that do not match the guard. Each named loader (`loadStats`, `loadReplays`, `loadOpenings`, `loadCampaignState`) now uses the helper with field-level guards. Unrecognized top-level fields are dropped. The original behaviour for first-load seeding (openings) and the empty-result default (replays) is preserved. **Acceptance criteria — all met** - [x] Each loader validates parsed JSON against the expected shape (typeof checks on every required field) - [x] Missing fields are merged with defaults (partial-merge like profile.ts:140) - [x] Unrecognized fields are dropped - [x] Type assertions replaced with runtime guards **Tests** - 9 new tests in `src/data/stats.test.ts`, including the exact repro from the issue body (stored object with `totalWins`/`totalLosses` but no `totalDuels` — `computeDerivedStats` no longer crashes and returns `winRate: 0`). - 5 new tests in `src/data/replays.test.ts` covering non-array storage, malformed items, wrong enum literals, missing snapshots, and malformed playerAction. - 343 unrelated tests pass; the 3 failures in `src/engine/reactiveConditions.test.ts` predate this commit and are out of scope. **Verification** - `npx tsc -b --noEmit` — clean - `eslint src/data/{safeStorage,stats,replays,openings,campaign,stats.test,replays.test}.ts` — clean - `vite build` — built in 4.13s, 1152 modules transformed Closing as fixed.

Mark referenced this issue

2026-09-18 15:38:12 +00:00

Forgejo Actions CI never dispatches on Mark/chainbound — #56 fix never built, deploy-itchbox stale since 2026-06-16 (last success 2026-06-19) #59