Files
HIPCTF2/.kilo/plans/908.md
T

8.2 KiB

Implementation Plan: Job 908 — Clear challenge state at the session boundary

1. Architectural Reconnaissance

  • Codebase style & conventions: TypeScript monorepo using NestJS (backend) + Angular 20 (frontend SPA), providedIn: 'root' signal-based stores, ChangeDetectionStrategy.OnPush, standalone components, RxJS for HTTP and SSE; backend uses TypeORM with SQLite. The repo follows the docs in /repo/docs/ (esp. architecture/frontend-structure.md, guides/challenges-board.md, api/challenges.md).
  • Data Layer: SQLite via TypeORM. The board endpoint already filters solvedByMe per caller's currentUserId (backend/src/modules/challenges/challenges.service.ts:75-125). The backend is correctsolvedByMe is computed from the JWT-authenticated user. The bug is purely on the frontend.
  • Test Framework & Structure: Jest 29 via ts-jest (configs in tests/jest.config.js). Tests live in tests/frontend/ (jsdom) and tests/backend/ (node). Run with npm test from repo root. Frontend tests favor plain TS classes / stores with manual fakes (no TestBed harness needed for the unit in question — see tests/frontend/challenges.store.spec.ts pattern).
  • Required Tools & Dependencies: No new packages. The fix is purely a code change inside existing modules.

2. Impacted Files

  • To Modify:
    • frontend/src/app/features/challenges/challenges.store.tssetMyUserId() resets whenever the cached state belongs to an unknown user (i.e. when _board !== null and prev === null or prev !== id). Add a small helper clearIfStaleOrDifferent(id) so the rule is consistent.
    • frontend/src/app/features/home/home.component.ts — inject ChallengesStore and call challengesStore.reset() in onLogout() (after auth.logout()) and in handleSessionInvalidated() (before navigating to /login).
    • tests/frontend/challenges.store.reset.spec.ts — add a regression test covering the new rule: a preloaded board with no recorded user id is cleared when PlayerB's id is set. Also keep the existing user-switch test.
  • To Create: (none)

3. Proposed Changes

3.1 Root cause

ChallengesStore is a providedIn: 'root' singleton that survives logout/login. The current setMyUserId() only resets when prev !== null && prev !== id, so two real-world cases still leak PlayerA's per-user state into PlayerB's view:

  1. PlayerB logs in fresh on a server that has never seen them before setMyUserId ran. prev === null (no user id has been recorded yet) so the guard skips reset(). If the previous user (PlayerA) had loaded a board (now stale in memory), PlayerB may flash the PlayerA board during the moment before their load() resolves. In an SSR or hot-reload scenario where ChallengesPage is constructed before any user has been associated with the singleton, the very first setMyUserId call must clear any pre-existing board.
  2. Session invalidation (logout, SSE 401/403, peer tab logout) clears the auth state but leaves the singleton cache. HomeComponent.handleSessionInvalidated() and HomeComponent.onLogout() navigate to /login without telling ChallengesStore to drop the cached board.

3.2 Backend changes

None.

3.3 Frontend changes — frontend/src/app/features/challenges/challenges.store.ts

  1. Update setMyUserId(id) so it resets whenever the cached state is for an unknown or different user. Concretely:

    setMyUserId(id: string | null): string | null {
      const prev = this._myUserId();
      const hasCachedUserData = this._board() !== null || this._selectedDetail() !== null;
      // Reset whenever the cached state belongs to a different or unknown user.
      if (hasCachedUserData && prev !== id) {
        this.reset();
      }
      this._myUserId.set(id);
      return prev;
    }
    

    Notes:

    • When prev === id (same user re-mounts), we keep the cached board — this preserves the existing tests' expectation that a re-mount does not refetch.
    • When prev === null && id !== null and no board is cached, we just record the id without reset (matches the singleton's first-use path).
    • When prev === null && id !== null and a board IS cached (orphan state), we reset — this is the new case the test must cover.
    • When prev !== null && prev !== id, we reset — already covered today.
    • When id === null (logout path) and the store has cached data, we also reset.
  2. Keep reset() as is — it already stops SSE, clears _board, _selectedDetail, _myUserId, _error, _loading, drops countdownZeroHandler, and empties solveListeners.

3.4 Frontend changes — frontend/src/app/features/home/home.component.ts

  1. Inject ChallengesStore alongside the existing userStore and auth injections in HomeComponent.

  2. In onLogout() — after await this.auth.logout() and before navigation, call this.challengesStore.reset():

    async onLogout(): Promise<void> {
      await this.auth.logout();
      this.userMenuOpen.set(false);
      this.userStore.reset();
      this.challengesStore.reset();
      await this.router.navigateByUrl('/login');
    }
    
  3. In handleSessionInvalidated() — reset the challenges store alongside userStore.reset(), before the navigation guard:

    private async handleSessionInvalidated(
      _reason: 'peer-logout' | 'sse-unauthorized',
    ): Promise<void> {
      this.userStore.reset();
      this.challengesStore.reset();
      this.changePasswordOpen.set(false);
      this.userMenuOpen.set(false);
      if (this.navigatingToLogin) return;
      if (this.router.url.startsWith('/login')) return;
      this.navigatingToLogin = true;
      try {
        await this.router.navigateByUrl('/login');
      } finally {
        this.navigatingToLogin = false;
      }
    }
    

This double-coverage (HomeComponent at the session boundary + setMyUserId at the page mount) means the store is always cleared at least once on the way out, regardless of whether logout went through the GUI or a peer-tab broadcast.

3.5 Accessibility / UX confirmation

  • ChallengeCardComponent binds [class.solved]="card.solvedByMe" and the <span *ngIf="card.solvedByMe" class="check" aria-label="Solved">✓</span>. After the fix, PlayerB's solvedByMe is per-user and falsy for Alpha Cipher. The generic "Solved ✓" therefore disappears from PlayerB's accessibility snapshot. The card retains 120 pts, 4 solves, and the difficulty pill.

4. Test Strategy

  • Target Unit Test File: tests/frontend/challenges.store.reset.spec.ts (update existing spec). Existing tests cover reset() and the user-switch path; we add the new "unknown-user" case.
  • Mocking Strategy: Pure store tests using the hand-rolled API fake pattern from tests/frontend/challenges.store.spec.ts (new ChallengesStore(api as any, { onDestroy: () => undefined } as any)). No HTTP, no Angular TestBed; no DOM visuals.
  • Key assertions (minimal — only the core success path and one regression per change):
    1. NEW: preloaded board with no recorded user id is cleared when PlayerB's id is set.
      • Load a PlayerA-shaped board (solvedByMe: true) with no prior setMyUserId() call.
      • Call setMyUserId('playerB').
      • Assert store.board() === null, findCard('alpha') === null, and store.selectedChallengeDetail() === null.
      • This is the regression that closes the original bug.
    2. Existing: setMyUserId with a different id flushes the previous board — keep as is.
    3. Existing: reset() clears board, selectedDetail, myUserId and stops any SSE — keep as is.
    4. Existing: applySubmitResponse always takes the server authoritative solvedByMe — keep as is.
  • Single command from root: npm test -- --testPathPattern=challenges.store runs the updated spec via the existing frontend Jest project; the existing npm test continues to run the whole suite.

5. Required Tools & Dependencies

  • System tools: Node 20+ (already required by setup.sh), SQLite via better-sqlite3 (already installed via root workspaces).
  • Package dependencies: None new. The fix uses only existing @angular/core (signal, DestroyRef) and TS types.
  • setup.sh impact: None — no new script, no new env var, no new migration needed.