Credential + identity colocation (#875) - Test Plan

Field	Value
Branch / PR	fix/875-credential-colocation / PR #876
Issue	#875 (P0). Related: #846, #847, #874, #523
Env	Firestore emulator for rules + integration; dev project for the migration
Date	2026-08-14

North star

Credential material and sealed-identity material must be structurally unable to reach a browser or an LLM prompt, rather than merely not requested by today's readers. The test of success is not "the dashboard stopped fetching it" but "no client role can fetch it even if it tries".

Severity framing (for anyone reading this later)

Not a breach. Every affected path required an authenticated admin session, and passwords are scrypt (N=16384, r=8, p=1) with 32-byte per-user salts. This is defence-in-depth being spent for no operational benefit, and it converted a future admin-portal XSS from a contained problem into total credential and identity compromise.

Scenarios

1. Credential subdocs unreadable by every client role (rules, emulator)

• Do: npm run test:rules.
• Expected: the sealed location is denied to the owning merchant, an admin, a plain user, and an unauthenticated client; listing the subcollection is denied; write and delete denied for all; the parent profile stays readable by owner and admin.

Result:    [x] pass   [ ] fail   [ ] blocked
Actual:    218 passed. 9 new tests for the sealed location + 2 for the dashboard aggregation.
Evidence:  tests/rules/firestore.rules.test.js, "#875 credential subdocs are unreadable by every client role".

2. The rules tests actually bite (anti-vacuous-test check)

• Do: temporarily relax the merchant rule to allow read: if isAdmin(), re-run, then revert.
• Why: the catch-all deny would also cover these paths, so a test that passed without the explicit rule would prove nothing about the rule.
• Expected: relaxing produces failures; reverting restores green.

Result:    [x] pass   [ ] fail   [ ] blocked
Actual:    Relaxing produced EXACTLY 2 failures (the admin-read test and the list test). Reverting
           restored 216/216 at that point. So the assertions are load-bearing, not vacuous.

3. Login survives the storage move (integration, emulator, real Firestore)

• Do: npm run test:credentials:emulator.
• Why this is the scenario that matters: the change relocates the field merchant and admin login READ. A migration that succeeds while the auth path still reads the old location succeeds at locking everyone out.
• Expected: the same password verifies in BOTH layouts; the readable profile ends with no credential material; sealed beats stale legacy.

Result:    [x] pass   [ ] fail   [ ] blocked
Actual:    9 passed. Legacy layout authenticates (source 'legacy'); sealed layout authenticates
           (source 'private'); sealed WINS over a stale legacy field so a rotated password cannot be
           resurrected; writeCredential seals + strips atomically leaving businessName intact;
           idempotent on re-write; admin behaves identically; half credentials read as absent;
           unknown collection throws.
Evidence:  services/api/auth/src/services/__tests__/credentialStore.integration.test.js

4. Dashboard aggregation replaces the whole-collection read

• Do: inspect fetchDashboardStats; run the rules tests for the count query.
• Expected: pending merchants counted via getCountFromServer (no documents cross the network); admin permitted to run it, non-admin denied.

Result:    [x] pass   [ ] fail   [ ] blocked
Actual:    Converted to getCountFromServer with a where filter. Rules tests assert an admin CAN run
           the aggregation (so the fix does not silently break the dashboard) and a non-admin CANNOT.

5. Migration against dev data

• Do: dry run, review counts, back up, apply, --verify, then exercise merchant + admin login.
• Expected: legacy fields gone from readable docs, credential present in private/auth, both logins still work.

Result:    [ ] pass   [ ] fail   [x] NOT RUN - awaiting the operator's direct word
Actual:    Built and fully exercised against the emulator (scenario 3), but NOT run against dev data.
           The agent-relays rule reserves permission-sensitive actions for the operator's direct
           word, and a data mutation has no merge gate in front of it to carry a relayed
           authorization the way a rules change does. A relayed quote exists; a direct line from her
           settles it.
Planned:   node services/api/auth/scripts/migrate-credentials.mjs                      # dry run, writes nothing
           node services/api/auth/scripts/migrate-credentials.mjs --apply --backup <f> # refuses without --backup
           node services/api/auth/scripts/migrate-credentials.mjs --verify             # per-collection proof
           then: merchant login + admin login on dev, because the field they read just moved.
Rollback:  node services/api/auth/scripts/migrate-credentials.mjs --restore <f>

6. Admin user reads served from the API, not Firestore (option B)

• Do: npm run test:admin-users:emulator (real emulator + real Express routing).
• Why real routing: the projection is unit-tested separately. What matters here is everything between the projection and the wire, which is where the original bug lived. A projection that is correct but bypassed by one handler leaks exactly as badly as no projection.

Result:    [x] pass   [ ] fail   [ ] blocked
Actual:    9 passed. Assertions are made against the RAW RESPONSE TEXT, not the parsed object, so a
           sensitive value cannot hide in a nested field the assertions did not name.
           Included two regression guards rather than feature tests:
             - literal routes (/stats, /search) are not swallowed by '/:userId' if declaration
               order is ever disturbed;
             - the GitHub lookup finds a user buried behind 120 documents, which is exactly what
               the old client-side 100-doc scan could not do (#879).
Evidence:  services/api/auth/src/routes/__tests__/adminUsersRead.integration.test.js

7. Full gate

Result:    [x] pass   [ ] fail   [ ] blocked
GATE OF RECORD: assembled from SCOPED runs on a verified-idle box, not a single uninterrupted run.
           - 30 non-test checks (lint, format, openapi, headers, workflows, workspace, every
             per-service validation, incl. Admin Validation): 30 passed, 0 failed.
           - Web App Tests & Coverage: PASS (499s). Tooling Script Tests: PASS.
           - Storybook Story Tests: FAILED inside the combined --scope test run, then PASSED
             STANDALONE at 81 files / 347 tests. Explained, not waved away: see below.
Why scoped: two earlier full runs were KILLED part-way (truncated log, NO summary table) in the two
           memory-heaviest phases. Filed as #884 with the dataset. Scoped foreground runs give the
           same coverage with a readable per-scope result that fails loudly instead of vanishing.
Why the storybook red is not a real failure: sampling `free -m` every 3s showed available memory
           going from a ~5,200MB baseline to a 1,835MB trough during coverage, a ~3.4GB swing on an
           8GB box. Storybook launches chromium immediately after that trough. From a ~5GB baseline
           it passes 347/347. Same shape as CPU contention, different scarce resource.
Method:    every result read from the SUMMARY TABLE, never an exit code. A piped `validate | tail`
           reports the pipe's status, and a killed run prints no table at all, so both failure modes
           are invisible to an exit-code check. Both bit today.

8. Code review round

• Do: /code-review on the full diff, fix real findings, re-run the affected suites.

Result:    [x] pass   [ ] fail   [ ] blocked
Actual:    13 findings. TWO WERE REAL REGRESSIONS THIS PR INTRODUCED, which is the honest
           headline rather than "review passed":
           1. The credential OUTLIVED ACCOUNT DELETION. Firestore does not delete
              subcollections with their parent, so moving the hash into private/auth left it
              orphaned and retained forever when an admin or merchant account was deleted.
              The exact opposite of this issue's purpose, and against the prefer-purge axiom.
              Verified against the cascade code before fixing. Both paths now recursiveDelete.
           2. The role filter returned an EMPTY TABLE for regular users, because web signup
              writes no `role` field at all, so where('role','==','user') matched almost nobody
              while the count card above it reported a nonzero total.
           3. A FIFTH call site was missed: getUserById still read raw user documents, and the
              detail panel opens on every row click, so the stated guarantee did not hold.
           Plus: list ordering silently changed to uid order, the exact-uid search escape hatch
           was lost, and the migration would OVERWRITE A GOOD BACKUP on a second --apply.
           7 new tests, one per regression. 16 endpoint tests pass.
Filed:     #881 (pre-existing: dashboard profile-completion counts are structurally always zero
           because the plaintext fields they count are encrypted away on save). Verified against
           profileService before filing rather than trusting the report.
Deferred:  the false admin-password banner (AdminMigrationBanner.jsx, another session's file).
           MUST land before the migration runs, not before merge; the merge is harmless without
           it. Raised with the PM for sequencing rather than crossing the boundary unasked.

The users fork: decided (option B)

Resolved by the operator on 2026-08-14: server-side admin API endpoints with explicit field sets, not a mirror collection. Her reasoning is worth preserving because it generalizes: a mirror is not merely a copy, it becomes a second SOURCE that looks as though someone already decided what is safe. The next consumer (intranet search, a team-built app) reads it without making a new decision, and it grows silently as fields added for one screen are inherited by every reader. An endpoint puts the boundary at a function call, so a new consumer has to ASK, and asking is where a human decides.

The original fork, kept for context.

The users half could NOT get the credential treatment. Sensitive fields on users (phoneHash, encryptedSeed, salts) cannot simply be sealed the way credentials were: the owner's own client must read encryptedSeed and phoneSalt to derive its key, so a deny-all would break zero-knowledge login outright. The leak is that ADMIN reads whole user documents in four places, three of which need one or two fields. See the PR body for the options and the recommendation.