Repository navigation
Apply the consolidation backlog: security fixes, real CI gates, maintainer docs - #96
Merged
Merged
Conversation
Defects found while applying the consolidation backlog: - Erase All Data cleared sessionStorage only. StorageService.clearAll() had no callers, so IndexedDB survived a cohort reset, contradicting Constitution VI. It now clears all three stores, and the E2E test asserts the students store is empty afterwards. - clearQuizData() stripped only `qd/` keys, leaving `qd:pin-attempts:*` PIN lockout state behind. It now strips both prefixes. - verifyInstructorPassword compared hashes with `===`. Now uses the existing constantTimeCompare. - The login-modal instructor path had no rate limiting at all, and the toolbar path never recorded failed attempts so its limiter never engaged. Both now share one policy, matching the student PIN allowance already in contracts.ts: two wrong passwords are free, the third starts the existing 2/4/8/16/30s backoff. RateLimiter gains an optional freeFailures argument, defaulting to the previous behaviour. - The toolbar unlock read `#instructor.password.hash`, an element the DITA XSL never emits, via its own hashing code. It now verifies through instructor-auth.ts against `#qd-instructor-hash` like the login modal, so the two paths cannot disagree. The duplicate reader src/config/instructor-password.ts is removed. - Stored XSS: qd-pin-reset-dialog built a message from the unescaped student name and qd-confirm-dialog renders it with unsafeHTML. Added escapeHtml() and applied it to both interpolations. - CSV export did not neutralise leading = + - @, so a student name or free-text answer became a live formula in Excel or Sheets. - The Detail column (MCQ options, numeric tolerances) was only CSS-hidden, leaving tolerances readable in view-source. Its content is now stripped to a WeakMap and restored on the instructor reveal path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87
- stories/components/qd-error-banner.stories.ts imported a component deleted in spec 012. It broke `npm run build-storybook`, which took down Chromatic (masked by continue-on-error) and the Storybook half of every PR preview (warn-and-skip). Removing it restores both. - Four modules had no importers anywhere: services/csv-export.ts, services/validation.ts, config/runtime-config.ts and components/qd-pin-create.ts, ~960 lines that misled readers about what the runtime does and skewed the untested-file count. Removed with their tests. Note this means there is no code path for the authoring-error banner described in the design doc; that is recorded as an open item rather than silently implied by a dead module. - Stories injected `#instructor.password.hash` with a full 64-char hash. The runtime reads `#qd-instructor-hash` with a 12-char truncated hash, so the stories exercised a path the product does not have. Updated to match. - feature-flags.ts pointed at a non-existent migrateStorage() helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87
- E2E was commented out of ci.yml. Restored as a default/encrypted matrix, so both storage modes are exercised on every PR. - Coverage thresholds existed in both vitest configs but CI ran test:unit, never test:coverage, so they were never evaluated and would have failed. CI now runs the coverage configs, with the thresholds set to the measured level as a ratchet to raise as tests are added. - The Chromatic job hid a broken Storybook build behind continue-on-error. Storybook now builds as a hard step, and the Chromatic upload runs only when CHROMATIC_PROJECT_TOKEN is present, reporting a notice when it is not. - test:gaps was not wired into CI. Added with a --max-gaps ratchet (new flag), and the script now accepts the flat tests/unit/<name>.test.ts layout that older tests use; it previously reported 7 tested files as untested. - playwright reporter 'html' serves a report at the end of every local run and never exits, hanging any scripted invocation. Now list + html with open:'never'. - VERSION in src/index.ts was pinned at '0.1.0-phase3.1' while package.json said 0.2.0. It is now injected from package.json at build time so it cannot drift from the release tag. - setDebugMode() is exported, so debug logging can actually be switched on; previously nothing called it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87
- src/init/ had 0% unit coverage. Added 124 tests across bootstrap, component-injector, event-coordinator, session-coordinator and global-styles, taking the directory to 99%. state-calculator.ts had no unit test; added one covering the unstarted/incomplete/complete rules and their edge cases. - Closes spec 009 T034/T035/T038: tests/e2e/encrypted-storage.spec.ts verifies records are obfuscated at rest while the instructor scores view and CSV export stay readable. Skips when ENCRYPT_STORAGE is off, matching the existing pin-authentication pattern. - specs 003 and 004 shipped without plan.md or tasks.md because they predate that convention. Added STATUS.md to each recording that, rather than leaving them looking abandoned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87
- New docs/MAINTAINERS.md: setup, repo map, where the truth lives, build/test/release commands, the things that will surprise a newcomer, known issues. There was no maintainer-facing entry point before; CLAUDE.md was the de facto one and is written for agents. - New docs/README.md: classifies every document as current, historical or generated, so a reader knows what to trust. - New docs/SECURITY.md: what this project actually does for security, with file paths and its real limits, replacing five overlapping SECURITY_*.md files (125 KB, all from one November 2025 commit) whose claims did not match the code. Those five are archived unchanged. - Technical_Design.md re-baselined against the code. 14 of its 20 checkable claims no longer held: events are on document not window and half the names were wrong, 22 components not 2, no data-qd-* flags, no window.SonarQuiz.init(), no cross-tab broadcast, no string dictionary. Section numbering kept so existing references still resolve; where something was designed but never built, it now says so. - ARCHITECTURE_FLOWS.md event table corrected; it documented a qd:instructor-login event that does not exist. - CLAUDE.md: phase status, DEBUG_MODE constant, object-store count and the erase-all description were all wrong. - README.md: "Phase 0 current" and "In active development" were nine months stale, two doc links were dead, and the bundle budget was quoted as 25 KB rather than 40 KB. - docs/CHROMATIC_SETUP.md rewritten; it claimed Chromatic was working. - Four historical review and proposal documents moved to docs/history/. - PROJECT_STATE.md gains a status column and a follow-up section recording what was applied and what is left for the owner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87
Contributor
🚀 PR Preview Deployment✅ Your changes are being built and deployed to a preview environment. 📋 Live PR Preview Links: ℹ️ About PR Previews:
📦 Production Demo (main branch): 🧪 Testing Locally: npm install
npm run build
# Open demo/quiz-index.html in your browser (use file:// or http-server)This comment will be automatically updated when you push new commits. |
The Chromatic account has lapsed, so the service is removed: the two dev dependencies, chromatic.config.json, the npm script, the Storybook addon and docs/CHROMATIC_SETUP.md. What the job was actually catching is kept and strengthened. CI still builds Storybook as a hard gate, and now does so on pull requests as well as main, so a story importing a deleted module fails before merge rather than silently breaking the Storybook half of PR previews. That was the real defect the old job hid behind continue-on-error. This leaves the project with no visual regression testing: nothing compares rendered output between commits. Recorded as an open item in docs/PROJECT_STATE.md and docs/MAINTAINERS.md, with Playwright screenshot assertions noted as the natural replacement since the E2E suite already drives every screen. Verified: typecheck, lint, format, 878 unit tests, 84 integration tests, E2E 105 passed, Storybook builds, bundle 39.10 KB. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applies every item from the restart backlog in
docs/PROJECT_STATE.md§8. One item is closed as "will not do" (Chromatic, see below) and one still needs you: taggingv0.2.0after this merges.Six commits, readable in order: security fixes, dead code removal, CI gates, tests, docs, Chromatic removal.
Defects fixed
Several were not in the original backlog. They surfaced while applying it.
StorageService.clearAll()had no callers. A cohort reset left every student record in place, contradicting Constitution VI. The E2E test now asserts the store is empty.unsafeHTML. A student registering with markup in their name got it executed in the instructor's session, same origin.=,+,-or@became a live formula when opened in Excel or Sheets.contracts.ts: two wrong passwords are free, the third starts the existing backoff.instructor-auth.ts.Gates that were decorative
test:unitrather than the coverage configs. Now enforced, set to measured levels as a ratchet.continue-on-erroron the Chromatic job. The story is gone, and the build is now a hard CI gate on pull requests as well as main, so this fails before merge next time. It also restores the Storybook half of PR previews, silently missing since June.test:gapswas not wired in, and mis-reported 7 tested files as untested. Fixed and added with a--max-gapsratchet.reporter: 'html'serves a report and never exits, hanging scripted runs.Chromatic removed
The account has lapsed, so the service is gone: two dev dependencies,
chromatic.config.json, the npm script, the Storybook addon and its setup doc. The Storybook build gate above is kept, which is what the job was catching in practice.This leaves no visual regression testing — nothing compares rendered output between commits. Recorded as an open item, with Playwright screenshot assertions noted as the natural replacement since the E2E suite already drives every screen.
Coverage and docs
src/init/went from 0% to 99% unit coverage;state-calculator.tsgained its first test. Spec 009's three open tasks are closed by a new encrypted-storage E2E spec.New
docs/MAINTAINERS.md,docs/README.md(classifies every document as current, historical or generated) anddocs/SECURITY.md, which replaces five overlappingSECURITY_*.mdfiles whose claims did not match the code.Technical_Design.mdis re-baselined: 14 of its 20 checkable claims were wrong. Four historical reviews moved todocs/history/.Verification
All run locally on this branch, after the Chromatic removal.
ENCRYPT_STORAGE=trueOne thing to watch: bundle headroom is now 0.90 KB, down from 2.39 KB. The security fixes account for most of the growth. The next feature will need to trim before it can add.
🤖 Generated with Claude Code
https://claude.ai/code/session_015u1mfVebscuZA3YDc6hc87