diff --git a/specs/012-code-review/quickstart.md b/specs/012-code-review/quickstart.md index d257e2f..bf08ecd 100644 --- a/specs/012-code-review/quickstart.md +++ b/specs/012-code-review/quickstart.md @@ -73,6 +73,24 @@ For every extraction: | SC-005 (tests green, bundle OK) | full DoD command block passes; `size-check` under 40KB | | SC-006 (independently shippable) | each slice merges on its own with green CI | +## Baseline (regression budget) — recorded T001/T002 + +Captured before refactor (2026-06-19): + +- **Tests**: 782 unit + 67 integration passing; typecheck/lint/format clean. +- **Bundle**: 146.12 KB min / **36.71 KB gzip** (limit 40 KB; 3.29 KB headroom). This is the regression budget — no slice may exceed 40 KB gzip. +- **Files >400 lines (SC-002 targets)**: + | File | Lines | + |------|-------| + | src/components/qd-login.ts | 983 | + | src/enhancers/quiz-table.ts | 807 | + | src/services/storage/indexeddb.ts | 759 | + | src/enhancers/analysis-table.ts | 637 | + | src/components/qd-migration-dialog.ts | 519 | + | src/init/bootstrap.ts | 490 | + | src/services/session.ts | 443 | + | src/types/contracts.ts | 411 (FROZEN — out of scope) | + ## Out of scope (do not touch) - `src/types/contracts.ts` (frozen). - `home-badges` → Lit conversion (breaks progressive enhancement). diff --git a/specs/012-code-review/tasks.md b/specs/012-code-review/tasks.md index 6e22714..0eedc10 100644 --- a/specs/012-code-review/tasks.md +++ b/specs/012-code-review/tasks.md @@ -30,8 +30,8 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be **Purpose**: Confirm the baseline is green so every later slice can be verified against it. -- [ ] T001 Run the full Definition of Done baseline (`npm run typecheck && npm run lint && npm run test:unit && npm run test:integration && npm run format:check && npm run build && npm run size-check`) and record the current bundle size as the regression budget in `specs/012-code-review/quickstart.md` notes -- [ ] T002 [P] Capture current file line counts for all in-scope modules (`wc -l` for the 8 target files) as the before/after baseline for SC-002 +- [X] T001 Run the full Definition of Done baseline (`npm run typecheck && npm run lint && npm run test:unit && npm run test:integration && npm run format:check && npm run build && npm run size-check`) and record the current bundle size as the regression budget in `specs/012-code-review/quickstart.md` notes +- [X] T002 [P] Capture current file line counts for all in-scope modules (`wc -l` for the 8 target files) as the before/after baseline for SC-002 --- @@ -41,12 +41,12 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be **⚠️ CRITICAL**: Complete before US2/US3/US4 extraction work begins. -- [ ] T003 [P] Add `INSTRUCTOR_SHOW_ANSWERS` constant to `STORAGE_KEYS` in `src/utils/storage-helpers.ts` (or the existing keys module) and replace the `'qd/instructor/showAnswers'` magic string in `src/services/session.ts`, `src/init/bootstrap.ts`, and `src/init/event-coordinator.ts` -- [ ] T004 [P] Create `src/utils/page-id.ts` exporting `getPageIdFromUrl(url?)` and replace the inline pathname→filename→strip-`.html` parses in `src/init/bootstrap.ts` (×2) and `src/init/event-coordinator.ts` -- [ ] T005 [P] Add unit test `tests/unit/page-id.test.ts` for `getPageIdFromUrl` (root, nested, query string, hash, missing `.html`) -- [ ] T006 [P] Add `clearBadges(link)` helper in `src/enhancers/home-badges.ts` and replace the three duplicated badge-stripping loops -- [ ] T007 [P] Add `createEmptyStudentRecord(session)` to `src/services/storage-service.ts` and replace the two duplicated new-record literals in `loadStudentRecord` -- [ ] T008 Consolidate shared styles (`button.primary`, `.error-message`, `.button-row`, spinner, modal-overlay) into `src/components/qd-instructor/shared-styles.ts` and import them in `qd-migration-dialog.ts`, `qd-pin-create.ts`, and `qd-login.ts` +- [X] T003 [P] Add `INSTRUCTOR_SHOW_ANSWERS` constant to `STORAGE_KEYS` in `src/utils/storage-helpers.ts` (or the existing keys module) and replace the `'qd/instructor/showAnswers'` magic string in `src/services/session.ts`, `src/init/bootstrap.ts`, and `src/init/event-coordinator.ts` +- [X] T004 [P] Create `src/utils/page-id.ts` exporting `getPageIdFromUrl(url?)` and replace the inline pathname→filename→strip-`.html` parses in `src/init/bootstrap.ts` (×2) and `src/init/event-coordinator.ts` +- [X] T005 [P] Add unit test `tests/unit/page-id.test.ts` for `getPageIdFromUrl` (root, nested, query string, hash, missing `.html`) +- [X] T006 [P] Add `clearBadges(link)` helper in `src/enhancers/home-badges.ts` and replace the three duplicated badge-stripping loops +- [X] T007 [P] Add `createEmptyStudentRecord(session)` to `src/services/storage-service.ts` and replace the two duplicated new-record literals in `loadStudentRecord` +- [X] T008 Consolidate shared styles (`button.primary`, `.error-message`, `.button-row`, spinner, modal-overlay) into `src/components/qd-instructor/shared-styles.ts` and import them in `qd-migration-dialog.ts`, `qd-pin-create.ts`, and `qd-login.ts` **Checkpoint**: Shared helpers/constants/styles in place; no observable behavior change; all tests green. @@ -60,9 +60,9 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be > This story is the primary deliverable and is already produced. Tasks below verify it against the spec's acceptance criteria. -- [ ] T009 [US1] Verify `specs/012-code-review/code-review-report.md` lists every `src/**` file >400 lines with line count, severity, and a decomposition recommendation (FR-001 / SC-001), reconciling against the T002 baseline -- [ ] T010 [P] [US1] Verify the report addresses all five hot-spot criteria (UI/logic coupling, deep nesting, extractable components, Lit candidates, oversized files) each with specific file/line references (FR-002) -- [ ] T011 [P] [US1] Verify the report includes a recommended execution order ranked lowest-risk → highest-payoff (FR-003) +- [X] T009 [US1] Verify `specs/012-code-review/code-review-report.md` lists every `src/**` file >400 lines with line count, severity, and a decomposition recommendation (FR-001 / SC-001), reconciling against the T002 baseline +- [X] T010 [P] [US1] Verify the report addresses all five hot-spot criteria (UI/logic coupling, deep nesting, extractable components, Lit candidates, oversized files) each with specific file/line references (FR-002) +- [X] T011 [P] [US1] Verify the report includes a recommended execution order ranked lowest-risk → highest-payoff (FR-003) **Checkpoint**: Report validated; MVP deliverable complete. @@ -76,18 +76,18 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be ### Tests for User Story 2 (write FIRST, confirm red/green) ⚠️ -- [ ] T012 [P] [US2] Characterization test `tests/integration/quiz-instructor-overlay.test.ts` asserting current rendered student-answer overlay content and structure (baseline before XSS fix) -- [ ] T013 [P] [US2] Add XSS regression test in `tests/integration/quiz-instructor-overlay.test.ts`: a student answer/name containing `'; + await storageService.saveStudentRecord( + makeStudent(maliciousName, 'RN0001', maliciousAnswer, false), + ); + + const table = createMCQTable(); + container.appendChild(table); + const metadata = enhanceWithMetadata(table); + + await showStudentAnswersForTable(table, metadata); + + const overlay = table.querySelector('.qd-student-answers'); + expect(overlay).toBeTruthy(); + + // No live elements were created from the student-controlled strings. + expect(overlay?.querySelector('img')).toBeNull(); + expect(overlay?.querySelector('script')).toBeNull(); + + // The raw markup survives verbatim as text content. + const nameSpan = overlay?.querySelector('.qd-student-name'); + const answerSpan = overlay?.querySelector('.qd-student-answer-text'); + expect(nameSpan?.textContent).toContain(maliciousName); + expect(answerSpan?.textContent).toBe(maliciousAnswer); + }); +}); diff --git a/tests/unit/instructor-auth.test.ts b/tests/unit/instructor-auth.test.ts new file mode 100644 index 0000000..2be4495 --- /dev/null +++ b/tests/unit/instructor-auth.test.ts @@ -0,0 +1,83 @@ +/** + * Unit tests for shared instructor-password authentication (T016). + * + * Covers SHA-256 + 12-char-truncation hashing and verification against the + * configured hash element (FR-008). + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { + hashPassword, + getExpectedInstructorHash, + verifyInstructorPassword, +} from '../../src/services/auth/instructor-auth.js'; +import { CONFIG_IDS } from '../../src/config/dom-config-reader.js'; + +function setConfiguredHash(value: string | null): void { + const existing = document.getElementById(CONFIG_IDS.instructorHash); + existing?.remove(); + if (value === null) return; + const span = document.createElement('span'); + span.id = CONFIG_IDS.instructorHash; + span.textContent = value; + document.body.appendChild(span); +} + +describe('instructor-auth', () => { + afterEach(() => { + setConfiguredHash(null); + }); + + describe('hashPassword', () => { + it('returns a 12-character hex digest', async () => { + const hash = await hashPassword('secret'); + expect(hash).toMatch(/^[0-9a-f]{12}$/); + }); + + it('is deterministic for the same input', async () => { + expect(await hashPassword('secret')).toBe(await hashPassword('secret')); + }); + + it('differs for different inputs', async () => { + expect(await hashPassword('secret')).not.toBe(await hashPassword('other')); + }); + + it('matches the first 12 chars of the full SHA-256 of "secret"', async () => { + // Known SHA-256("secret") = 2bb80d537b1da3e3... + expect(await hashPassword('secret')).toBe('2bb80d537b1d'); + }); + }); + + describe('getExpectedInstructorHash', () => { + it('returns the trimmed configured hash', () => { + setConfiguredHash(' abc123def456 '); + expect(getExpectedInstructorHash()).toBe('abc123def456'); + }); + + it('returns empty string when the element is missing', () => { + setConfiguredHash(null); + expect(getExpectedInstructorHash()).toBe(''); + }); + }); + + describe('verifyInstructorPassword', () => { + beforeEach(() => { + setConfiguredHash(null); + }); + + it('returns true when the password matches the configured hash', async () => { + setConfiguredHash(await hashPassword('letmein')); + expect(await verifyInstructorPassword('letmein')).toBe(true); + }); + + it('returns false when the password does not match', async () => { + setConfiguredHash(await hashPassword('letmein')); + expect(await verifyInstructorPassword('wrong')).toBe(false); + }); + + it('returns false when no hash is configured', async () => { + setConfiguredHash(null); + expect(await verifyInstructorPassword('anything')).toBe(false); + }); + }); +}); diff --git a/tests/unit/utils/page-id.test.ts b/tests/unit/utils/page-id.test.ts new file mode 100644 index 0000000..2ae192b --- /dev/null +++ b/tests/unit/utils/page-id.test.ts @@ -0,0 +1,44 @@ +/** + * Unit tests for page-id extraction utility + */ + +import { describe, it, expect } from 'vitest'; +import { getPageIdFromUrl } from '../../../src/utils/page-id.js'; + +describe('getPageIdFromUrl', () => { + it('extracts the page id from a root-level filename', () => { + expect(getPageIdFromUrl('/quiz-index.html')).toBe('quiz-index'); + }); + + it('extracts the page id from a nested path', () => { + expect(getPageIdFromUrl('/training/topics/gram-1.html')).toBe('gram-1'); + }); + + it('ignores a query string', () => { + expect(getPageIdFromUrl('/topics/gram-1.html?attempt=2')).toBe('gram-1'); + }); + + it('ignores a hash fragment', () => { + expect(getPageIdFromUrl('/topics/gram-1.html#question-3')).toBe('gram-1'); + }); + + it('ignores both query string and hash fragment', () => { + expect(getPageIdFromUrl('gram-1.html?x=1#top')).toBe('gram-1'); + }); + + it('strips a .htm extension as well as .html', () => { + expect(getPageIdFromUrl('/legacy/gram-1.htm')).toBe('gram-1'); + }); + + it('returns the filename unchanged when there is no .html extension', () => { + expect(getPageIdFromUrl('/topics/gram-1')).toBe('gram-1'); + }); + + it('returns an empty string for a trailing-slash path', () => { + expect(getPageIdFromUrl('/topics/')).toBe(''); + }); + + it('handles a bare filename with no leading slash', () => { + expect(getPageIdFromUrl('analysis-examples.html')).toBe('analysis-examples'); + }); +});