Refactor hot-spots (012): foundational dedup + quiz-overlay XSS fix + answer-reveal/instructor-auth consolidation#93
Merged
Conversation
Wave-1 zero-behavior-change dedup that later slices build on: - Add INSTRUCTOR_SHOW_ANSWERS_KEY constant; replace the 'qd/instructor/showAnswers' magic string across quiz-table, qd-instructor, session, bootstrap, event-coordinator (T003) - Add utils/page-id.ts getPageIdFromUrl(); replace the inline pathname->filename->strip-.html parses in bootstrap (x2) and event-coordinator, with unit tests (T004/T005) - Add clearBadges(link) helper in home-badges; replace 3 duplicated badge-stripping loops (T006) - Add createEmptyStudentRecord(session) in storage-service; replace the two duplicated new-record literals in loadStudentRecord (T007) - Extract spinnerStyles into components/shared-styles.ts and adopt in qd-migration-dialog (T008; divergent error/button styles left intact to preserve exact rendering per the behavior-preservation rule) DoD: typecheck, lint, 791 unit + 67 integration tests, format, build, size-check (36.95KB gzip) all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tK4JXdkC4s9rYYywjYNiv
Security/duplication fixes (Phase 4, partial): - Fix innerHTML XSS in the quiz instructor overlay (FR-004): student name/answer now rendered via textContent + element construction, so any HTML they contain is inert text. Adds characterization + XSS-regression integration tests (T012/T013/T020). - Extract enhancers/instructor-answer-reveal.ts as the single shared, security-sensitive answer-reveal path; bootstrap (initial-load, adds qd-quiz-instructor class) and event-coordinator (post-login) now both call it. Behavior preserved exactly via an addInstructorClass option. Adds characterization tests (T014/T019). Export QuizTableMetadata type for the shared enhancer. DoD: typecheck, lint, 791 unit + 75 integration tests, format, build, size-check (36.87KB gzip) all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tK4JXdkC4s9rYYywjYNiv
- Add services/auth/instructor-auth.ts (hashPassword, getExpectedInstructorHash, verifyInstructorPassword) as the single source of instructor-password hashing (SHA-256 + 12-char truncation), with unit tests (T016/T017, FR-008). - Route qd-login.handleInstructorLogin and qd-migration-dialog.validatePassword through it, deleting the duplicated crypto. Instructor-hash config reads now go through the shared module (partial T018). DoD: typecheck, lint, 800 unit + 75 integration tests, format, build, size-check (36.94KB gzip) all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tK4JXdkC4s9rYYywjYNiv
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. |
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.
Summary
Implements the first, independently-shippable slices of feature 012-code-review (
/speckit.implement 12) — Phases 1–3 in full and the high-value security/duplication work of Phase 4 (US2). Every slice is behavior-preserving (the one sanctioned exception being the quiz-overlay XSS fix) and gated by the full Definition of Done.The work follows the spec's own incremental-delivery strategy (SC-006): each commit is green and mergeable on its own.
What's included
Phase 1 — Baseline (T001–T002)
quickstart.md: 782 unit + 67 integration tests; 36.71 KB gzip bundle (limit 40 KB); the 8 files >400 lines.Phase 2 — Foundational consolidation (T003–T008, no behavior change)
INSTRUCTOR_SHOW_ANSWERS_KEYconstant replaces the'qd/instructor/showAnswers'magic string in 5 modules.utils/page-id.tsgetPageIdFromUrl()replaces 3 inline pathname→filename→strip-.htmlparses (with unit tests).clearBadges(link)helper replaces 3 duplicated badge-stripping loops.createEmptyStudentRecord(session)replaces the two duplicated new-record literals.spinnerStylesextracted intocomponents/shared-styles.ts(adopted by the migration dialog).Phase 3 — Maintainer report verified (T009–T011)
code-review-report.mdlists everysrc/**file >400 lines with line count/severity/decomposition, addresses all five hot-spot criteria with file/line refs, and provides a lowest-risk→highest-payoff sequence. Reconciled against the T002 baseline.Phase 4 — Security & duplication (US2, partial)
textContent/element construction instead ofinnerHTML, so injected HTML is inert text. Characterization + XSS-regression integration tests added (T012/T013/T020).enhancers/instructor-answer-reveal.ts): bothbootstrap.ts(initial-load) andevent-coordinator.ts(post-login) now call one function; behavior preserved exactly via anaddInstructorClassoption. Characterization tests added (T014/T019).instructor-auth(services/auth/instructor-auth.ts): one SHA-256 + 12-char hashing implementation, consumed by bothqd-loginandqd-migration-dialog; duplicated crypto deleted; instructor-hash config reads routed through it (T016/T017, partial T018).Verification
Final Definition of Done — all green:
typecheck,lint,format:checkcleanbuildOK; bundle 36.94 KB gzip (within the 40 KB budget, no regression)Deferred to follow-up
The remaining tasks are large and security-sensitive; rushing them would risk the behavior regressions the spec's golden rule guards against. They are best delivered as further independent slices:
AuthServiceextraction collapsing the ~100-linehandleStudentLogin/retryLoginAfterMigrationduplicate and routing the remaining dbName/title config reads (these are entangled with custom error semantics, so they belong with the AuthService move).indexeddb.ts,quiz-table.ts,analysis-table.ts,bootstrap.ts,session.ts,qd-login.ts) below ~400 lines.🤖 Generated with Claude Code
Generated by Claude Code