Skip to content

fix(flags): validate persisted route-gate values - #3351

Merged
Chris0Jeky merged 8 commits into
mainfrom
audit/3350-feature-flag-storage-boundary
Sep 23, 2026
Merged

Chris0Jeky merged 8 commits into
mainfrom
audit/3350-feature-flag-storage-boundary

Conversation

@Chris0Jeky

@Chris0Jeky Chris0Jeky commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Closes #3350.

Final qualification and merge

Merged at edd07b7a514fe98b5e3764e3b3b7838153a8be62 from exact source head ebe289e0b89f99bb60cc5f3e965ef89a08c54f37.

Required CI 35676336663 completed successfully across all 18 jobs, including both frontend lint/typecheck/build/coverage jobs, both backend and API platforms, E2E, migrations, containers and security. Self-Test 35676336098 and Extended 35676336594 succeeded. All three review threads are resolved; final continuation review 5297205871 inspected the complete four-file diff and the final recovered-load-error correction.

Before merge, GitHub's synthetic merge 5d15cbb5dac3ce56e554fb27a6633a64b73e1a7c bound this head to main f180c225168e8d4ca417cd5a379363c55d4a4c37 without conflict. The cited CI qualifies the source head, not a separate execution of that newly generated merge ref. No local Vue execution is claimed by the continuation review.

Summary

Hardens the local feature-flag persistence boundary used during app startup, router gating and Profile Settings:

  • restores only declared keys with actual boolean values;
  • arrays, primitives and unknown properties cannot become route gates;
  • storage read/parse failure restores defaults without aborting startup/navigation;
  • storage write failure preserves the valid in-memory setFlag or resetAll result;
  • an unsaved in-memory choice cannot be overwritten by stale readable storage during later route-triggered restores;
  • a later successful write clears the unsaved-state guard;
  • persistenceError exposes failures in Profile Settings and clears on recovery;
  • isEnabled and allEnabled consider only declared flags.

The storage key, defaults, valid payload behavior and existing store calls remain compatible. Feature flags remain local presentation/developer preferences, not a server authorization boundary.

Historical test-first evidence

Test-only head a22c2acaa96a0db4736af0a56646687f15897307 ran canonical Node 24 qualification. Ubuntu lint, typecheck, production build and PWA validation passed before JUnit recorded 7,158 tests, exactly six intended shape/storage-boundary failures and zero errors. Initial correction 9246b9744174b76b0b0418b1929d443b54bd154b then passed its full matrix.

Codex review identified failed writes being overwritten by a subsequent restore. Test-only b49fccaf90e17d8a11ae8280a09f04088a1f0890 added separate unsaved setFlag/resetAll regressions. A supplemental actual-module runner reproduced the stale overwrite before the correction and preserved the in-memory choice afterward. Head faac2a9 was an intermediate checkpoint, not the final qualified head.

Later review corrections added visible persistence failure feedback and recovery clearing, with the dedicated store and view regressions present in final head ebe289e0.

No backend, DTO, schema, migration, dependency, workflow, control-plane or human-action change.

@Chris0Jeky
Chris0Jeky marked this pull request as ready for review September 21, 2026 17:25

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9246b97441

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread frontend/taskdeck-web/src/store/featureFlagStore.ts Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-22T01:42:46.142116Z ebe289e Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Chris0Jeky
Chris0Jeky marked this pull request as draft September 21, 2026 17:34
@Chris0Jeky
Chris0Jeky marked this pull request as ready for review September 21, 2026 18:56
@Chris0Jeky
Chris0Jeky force-pushed the audit/3350-feature-flag-storage-boundary branch 2 times, most recently from 7920993 to e880496 Compare September 21, 2026 20:16
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

Please review exact current head e880496 against base fb830ed. This is the one current-head review request after the main-base requalification. Focus on the PR's changed ownership and stale-settlement behavior, and report only merge-blocking correctness or security defects.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: e880496b36

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Chris0Jeky
Chris0Jeky force-pushed the audit/3350-feature-flag-storage-boundary branch from e880496 to 48e325a Compare September 21, 2026 22:06
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48e325a4e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread frontend/taskdeck-web/src/store/featureFlagStore.ts Outdated
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7668a30398

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread frontend/taskdeck-web/src/store/featureFlagStore.ts
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 8161b04446

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Chris0Jeky
Chris0Jeky force-pushed the audit/3350-feature-flag-storage-boundary branch from 8161b04 to ca7ade3 Compare September 21, 2026 23:24
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: ca7ade33c8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Chris0Jeky
Chris0Jeky force-pushed the audit/3350-feature-flag-storage-boundary branch from ca7ade3 to a0cbf1c Compare September 22, 2026 00:06
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: a0cbf1ce5a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

Please review exact current head ee8fc83 against base e09f10f after the #3349 merge and rebase. feature-flag persistence proof is 64/64 and typecheck passes. Report only merge-blocking correctness or security defects.

@Chris0Jeky
Chris0Jeky force-pushed the audit/3350-feature-flag-storage-boundary branch from a0cbf1c to ee8fc83 Compare September 22, 2026 00:49
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review

Please review exact current head ee8fc83 against base e09f10f. The rebase/merge-base proof is 64/64 and typecheck passes. Report only merge-blocking correctness or security defects.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: ee8fc83fed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Chris0Jeky
Chris0Jeky force-pushed the audit/3350-feature-flag-storage-boundary branch from ee8fc83 to ebe289e Compare September 22, 2026 01:35
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

@codex review Please review exact current head ebe289e against base a62d1ec after #3310 merged and this PR was rebased. Focused feature-flag proof is 64/64 across the three suites and Vue typecheck passes. Qwen's two storage-guard notes were source-validated as non-blocking; report only merge-blocking correctness or security defects.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: ebe289e0b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator verification: real route-gate bug, correctly fixed. On main, restore() spreads unvalidated localStorage JSON into flags (featureFlagStore.ts line 34): a tampered devTools true-string is truthy and passes the router guard at router/index.ts line 453, and false-strings stick flags on. The PR normalizes to declared booleans only. Persist/restore failure paths are also real (quota throws, blocked-storage getItem throws outside try on main) and the fix handles both with surfaced error state plus the hasUnsavedChanges guard so stale storage cannot clobber an unsaved update. Prior Codex P1/P2 threads were maintainer-verified fixed and the final state matches. Tests discriminate against defaults on every type-confusion case. Full CI green. Recommend merge once mergeable resolves.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh continuation review of all four changed files at actual head ebe289e0b89f99bb60cc5f3e965ef89a08c54f37 (the body still named faac2a9): known-key boolean normalization excludes unexpected payload shapes; failed writes retain session state across route-triggered restores; persistenceError exposes unsaved state in Profile Settings and clears on successful persistence/restore. Reviewed the final recovered-load-error commit and its focused regression. All three prior review threads are resolved, and their corrections are present in this head. No blocking defect found in this bounded local-preference slice.

Freshly inspected exact-head required workflow 35676336663: all 18 jobs succeeded, including both frontend lint/typecheck/build/coverage jobs, both backend and API platforms, E2E, migrations, containers and security. Self-Test 35676336098 and Extended 35676336594 also succeeded. The current GitHub merge ref 5d15cbb5dac3ce56e554fb27a6633a64b73e1a7c binds this head to current main f180c225168e8d4ca417cd5a379363c55d4a4c37 without conflict. The CI run qualifies the exact source head; it is not a claim that this newly generated merge ref was separately executed.

No new code or tests were authored in this review, and no local Vue execution is claimed. Ready for merge under the user's explicit maintenance request; feature flags remain presentation preferences, not server authorization.

@Chris0Jeky
Chris0Jeky merged commit edd07b7 into main Sep 23, 2026
36 checks passed
@Chris0Jeky
Chris0Jeky deleted the audit/3350-feature-flag-storage-boundary branch September 23, 2026 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

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

1 participant