Feature-flag restore accepts non-boolean route gates and can crash on storage failure

[Chris0Jeky]

Summary

frontend/taskdeck-web/src/store/featureFlagStore.ts trusts the shape and values of taskdeck_feature_flags after JSON parsing and performs every storage read/write without a failure boundary.

This is a source-traced finding on main 307c3b8b50bec1cb0bfaea3e570a942bcb1d4451. The store is restored both during app startup and inside the router feature gate, so these are reachable application boundaries rather than test-only helpers.

Confirmed gaps

1. Non-boolean values become route decisions. restore() spreads arbitrary parsed JSON over defaultFeatureFlags. A persisted value such as { "devTools": "false" } is returned by isEnabled() as a truthy string, even though the public contract says every flag is boolean. The same applies to objects, numbers, arrays, and unknown properties.
2. Storage reads can abort startup/navigation. localStorage.getItem executes outside the current try. Browsers can throw for storage access in restricted/private contexts, allowing App.vue or the router guard's restore() call to fail.
3. Storage writes can turn a local preference click into an exception. setFlag() and resetAll() mutate valid in-memory state, then persist() calls setItem without a boundary. A quota/security exception escapes even though the runtime choice could remain usable for the session.
4. Unknown persisted keys survive restoration. They are not part of FeatureFlags, but remain on the runtime object and participate in allEnabled through Object.values.

This concerns a local display/developer preference. It is not a server authorization boundary, and feature flags must not be treated as one.

Expected contract

• Restore only known keys whose values are actual booleans; invalid or missing values use the declared defaults.
• Unknown keys and prototype-like payload members are discarded.
• A storage read or parse failure restores safe defaults and does not abort app/router startup.
• A storage write failure does not roll back or throw away a valid in-memory choice; public method signatures remain unchanged.
• isEnabled() and allEnabled remain strictly boolean and consider only declared feature flags.
• Preserve the current storage key, defaults, route behavior for valid payloads, and profile-settings API.

Acceptance

• Vitest regressions for string/number/object/array values on known flags and unknown properties.
• A valid partial payload still overrides only its boolean keys.
• getItem failure leaves defaults and does not throw.
• setItem failure after setFlag and resetAll leaves the intended in-memory state and does not throw.
• Round-trip, router guard, App startup and Profile Settings tests remain green.
• Frontend lint, typecheck, build, full Vitest on Ubuntu and Windows, and exact-head hosted CI pass.

Audit provenance

Found during the 2026-09-21 frontend integrity pass while queue ownership PR #3349 was qualifying. Searches found no existing open issue or PR naming featureFlagStore.

[Chris0Jeky]

Fresh review residual addressed in PR #3351.

Codex found that preserving an in-memory update after localStorage.setItem failure was insufficient: a later route-triggered restore() could read the older payload and silently revert the user's choice. Test-only head b49fccaf90e17d8a11ae8280a09f04088a1f0890 covers both unsaved setFlag and unsaved resetAll.

Current final head faac2a9ab22470b136197f8b3259a3e6f73e0fd4 tracks unsaved in-memory changes, ignores stale restores while dirty, and clears the guard only after a successful write. The actual-module runner is green; hosted Self-Test and Extended are green, full Ubuntu/Windows frontend and container jobs are green, and the remaining Required CI backend/API gates are still running.