Skip to content

Fill test coverage gaps (replay, split-mode admin, races, edge cases) #25

@Loule95450

Description

@Loule95450

Context

test/ has 6 files (~1.3k lines) covering happy paths well. Several risky areas have no coverage.

Problem / Observation

Missing scenarios:

  • Challenge replay: post a /login/finish with the same challengeToken twice. The code deletes-on-consume (challenges.ts:64), but there's no test asserting the second call returns 401.
  • Approval status polling after token issued: no test asserts whether polling twice issues two tokens (currently it does — see related security issue).
  • Admin re-login: setUpAdmin auto-issues a session; we don't test that after /admin/auth/logout, the admin can still log in via /admin/auth/login/start.
  • Device pubkey collision across users: two users with the same devicePubkey — does the system handle the unique-or-not behavior? (The schema doesn't constrain (user_id, pubkey) to be unique either — see related issue.)
  • Concurrent appendEvent: parallel pushes from the same device — does server_seq stay strictly monotonic?
  • /admin/users pagination beyond offset 100: existing test only hits the default page size.
  • Rejection without reason: /admin/users/:id/reject with no body — currently works but no test pins it.
  • Audit log assertions: no test verifies the audit_log table receives the expected rows for any flow.
  • Migrations applied on existing DB: no test runs migrations on a DB with pre-existing rows from migration 001 to verify the approval_status DEFAULT 'approved' backfill behaves as intended.
  • Body limit / payload too large at the Fastify layer: only schema-level limits are tested.
  • CORS preflight on admin instance: admin instance shouldn't register CORS at all; covered indirectly by split-mode.test.ts but no explicit assertion.
  • trustProxy off vs. on: no test asserts that IP hashing uses the right source IP when behind a proxy.

Proposed approach

Add test/edge.test.ts covering replays, races, and pagination; add audit-log assertions to existing tests where applicable; add test/migrations.test.ts that runs migration 001 then 003 against rows seeded with status NULL and asserts they end up approved.

Acceptance criteria

  • Each bullet above has at least one test
  • Coverage report (npm run coverage) added to CI
  • CI fails below a set threshold (e.g. 80% line coverage on src/auth and src/store)

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Medium prioritytestTest coverage gap

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions