diff --git a/agent-hub/evidence/implementer/2026-09-28/fix-v2-register-missing-await-plan.md b/agent-hub/evidence/implementer/2026-09-28/fix-v2-register-missing-await-plan.md new file mode 100644 index 0000000..e4250a2 --- /dev/null +++ b/agent-hub/evidence/implementer/2026-09-28/fix-v2-register-missing-await-plan.md @@ -0,0 +1,129 @@ +# 2026-09-28 — fix-v2-register-missing-await (plan + diff) + +- Worker: implementer +- Version: 0.1.0 +- Node: `fix-v2-register-missing-await` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Issue: [#156](https://github.com/datvt243/resume-nodejs-api/issues/156) — POST /api/v2/auth/register always fails — missing await on bcryptGenerateSalt +- Branch: `156-post-apiv2-authregister` (base `staging`) +- Task (verbatim): "fix bug #152 tới #156" — this note covers #156, the last of the same 5-issue batch as #152/#153/#154/#155, each processed as its own implementer→verifier round. + +## Hub bytes before: 88066 + +## Node lookup + +Matched the existing PENDING node `fix-v2-register-missing-await` directly +(task resolves to GitHub issue #156, filed against this exact node). + +## Bookkeeping-gap finding, with a twist (read before writing anything) + +Different shape from the other 4 nodes in this batch: the file the node +(and the issue) names, `src/api/v1/auth/services/register.ts`, **does not +exist anywhere in the repo today**: + +``` +$ find src/api -iname "*.ts" +(no output) +``` + +It was removed by `consolidate-v1-v2-auth` (issue #77, SEALED +2026-08-29), which merged the separate v1/v2 auth implementations into +one shared `src/auth/auth.service.ts` + `src/auth/auth.controller.ts`. +Confirmed `src/routers/api/v2/auth.route.ts` today: + +```ts +import { authRegister, authLogin } from '@/auth/auth.controller'; +router.post('/register', authRegister); +router.post('/login', authLogin); +``` + +— the exact same `authRegister` controller v1 uses, which calls +`handlerRegister` (`src/auth/auth.service.ts:48`): + +```ts +const bcryptPwd = await bcryptGenerateSalt(password); +``` + +`await` is present and correct. This isn't quite the same "the exact +diff already shipped in commit `f355e2f`" story as the other 4 nodes in +this batch — it's one level more thorough: the **entire buggy code +path was deleted and replaced** by the v1/v2 consolidation, and the +replacement was never buggy in the first place (it's the same +`handlerRegister` v1 has always used, which has always awaited +correctly). So there's no old commit to cite for "this exact line was +fixed" — instead the fix is that the vulnerable file doesn't exist +anymore, full stop. + +Existing coverage already indirectly proves the `await` is correct: +`auth.service.test.ts`'s `handlerRegister` "should register successfully" +test mocks `bcryptGenerateSalt` with `mockResolvedValue(mockHash)` and +asserts `CandidateModel.create` was called with `password: mockHash` (the +plain string) — if the `await` were missing, `bcryptPwd` would be the +Promise object itself, and that assertion would fail (a Promise never +deep-equals a string). This is not a "happy path that could pass by +accident" (issue #156's own phrasing) — it's a real, load-bearing +assertion. + +The one real gap: **nothing proved `/api/v2/auth/register` actually +reaches this already-correct, already-tested handler**, as opposed to +some other, possibly-still-broken code path. That's what this diff adds. + +## Diff (smallest diff — no `src/` production code, 1 new test file) + +- `src/__tests__/auth/v2AuthRoute.test.ts` (new) — inspects the real + Express router object exported by `src/routers/api/v2/auth.route.ts` + (`router.stack`) and asserts its `/register` route's handler is + literally the same `authRegister` function reference imported from + `@/auth/auth.controller` — not a re-implementation, not a stale/dead + copy, the exact same code v1 uses and `auth.service.test.ts` already + covers. + +## Command + +``` +npm test +``` +Output (verbatim tail): +``` +Test Suites: 27 passed, 27 total +Tests: 141 passed, 141 total +Snapshots: 0 total +Time: 6.786 s, estimated 7 s +Ran all test suites. +``` +(141 = 140 (post-#157-merge baseline on `staging`) + 1 new test, in a new +suite file, so +1 suite. Same pre-existing, unrelated harness exit +warning as prior notes — not a failure.) + +``` +npm run build +``` +Output: `tsc` clean, `copy` step ran with no errors. + +## Acceptance + +| Criterion | Evidence | +|---|---| +| Trace to exactly one diagram node | `fix-v2-register-missing-await` | +| Smallest diff | 1 new test file only, 0 production `src/` changes (the buggy file no longer exists — removed by `consolidate-v1-v2-auth`/#77, 2026-08-29) | +| `POST /api/v2/auth/register` succeeds end-to-end, stored password is a real bcrypt hash | `auth.service.test.ts`'s existing `handlerRegister` test (pre-existing, cited not re-run) proves the hash is awaited correctly; this diff's new test proves v2's route reaches that exact handler | +| A regression test covers this call site directly, not just a happy-path that could pass by accident | `v2AuthRoute.test.ts` — asserts the literal function reference, would fail if v2 ever pointed at a different/broken handler again | +| Exact test command run + output read back | `npm test` output above; `npm run build` clean | +| Evidence note written | This file | + +## Live end-to-end — not performed, said honestly + +Same sandbox constraint as the other nodes in this batch: no `.env`, no +local MongoDB, Docker daemon unreachable — no way to curl a real `POST +/api/v2/auth/register` end-to-end in this environment. The route-wiring +test plus the existing `handlerRegister` unit test together prove the +same thing a live curl would (real handler, real awaited hash), without +requiring a live DB round-trip. + +## Seal gate + +No outward-facing action taken (no commit/push). Only a local file +write: 1 new test file under `src/__tests__/`. Pending verifier. + +## Status + +`sealed_pending_verifier` diff --git a/agent-hub/evidence/verifier/2026-09-28/fix-v2-register-missing-await-seal.md b/agent-hub/evidence/verifier/2026-09-28/fix-v2-register-missing-await-seal.md new file mode 100644 index 0000000..c404fff --- /dev/null +++ b/agent-hub/evidence/verifier/2026-09-28/fix-v2-register-missing-await-seal.md @@ -0,0 +1,94 @@ +# 2026-09-28 — fix-v2-register-missing-await (verifier verdict) + +- Worker: verifier +- Node: `fix-v2-register-missing-await` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- New PM status: SEALED (was PENDING) + +## Isolation proof + +Dispatched as a fresh subagent via the Agent tool with the task +description "Independent verifier pass for +fix-v2-register-missing-await" — no memory of the implementer session +that produced the diff under review; everything below was re-derived +from the evidence note and the repo itself, not recalled. + +## Reasoning + +**Criterion (a) — POST /api/v2/auth/register succeeds end-to-end, stored +password is a real bcrypt hash, not `[object Promise]`.** +Independently confirmed the note's central claim by reading source +directly (not just trusting the note's citations): +- `find src/api -iname "*.ts"` → no output. The file the diagram node and + issue #156 name (`src/api/v1/auth/services/register.ts`) does not + exist anywhere in the repo. +- `src/routers/api/v2/auth.route.ts` imports `authRegister` from + `@/auth/auth.controller` and wires it to `router.post('/register', ...)` + — the identical controller v1 uses. +- `src/auth/auth.service.ts` `handlerRegister`: `const bcryptPwd = await + bcryptGenerateSalt(password);` — `await` present and correct. +- `src/__tests__/auth/auth.service.test.ts` "should register + successfully with new email" mocks `bcryptGenerateSalt` with + `mockResolvedValue(mockHash)` and asserts + `CandidateModel.create` was called with `password: mockHash` (the + plain string). A missing `await` would leave `bcryptPwd` as a pending + Promise object, which never deep-equals `mockHash` under + `toHaveBeenCalledWith` — so this pre-existing test is genuinely + load-bearing for criterion (a), not incidental. Confirmed by reading + the test file directly, not the note's paraphrase alone. + +**Criterion (b) — a regression test covers this call site directly, not +just a happy-path integration test that could pass by accident.** +Read `src/__tests__/auth/v2AuthRoute.test.ts` in full. It imports the +real `v2AuthRouter` and the real `authRegister`, finds the `/register` +layer in `router.stack`, and asserts +`registerLayer.route.stack[0].handle` is the literal `authRegister` +function reference via `.toBe()` (identity, not deep-equal) — this +would fail if v2's route were ever repointed at a different or +reintroduced-buggy handler. Combined with the pre-existing +`handlerRegister` unit test above, the two together directly cover both +"the right handler runs" and "that handler awaits the hash correctly" — +not a single broad happy-path test that could pass by accident. + +**Test command / output.** Note's command is `npm test`, matching +`doctrine/MEMORY.md`'s documented `npm test` = `jest +--passWithNoTests`. Output in the note (141/141, 27 suites) is not +truncated or redacted. Additionally re-ran the two directly relevant +suites myself for extra confidence: +`npx jest src/__tests__/auth/v2AuthRoute.test.ts +src/__tests__/auth/auth.service.test.ts` → 2 suites, 14 tests, all +passed, including both cited assertions. + +**Forbidden states scan.** +- `ADHOC_WORK` — node exists on the diagram, worker identity declared. N/A. +- `NO_EVIDENCE` — note written at the cited path. N/A. +- `EDIT_UNVERIFIED` — test output was read back in the note and + independently reproduced here. N/A. +- `CODE_IN_HAVEN` — `find agent-hub/haven -iname "*.ts" -o -iname "*.py" + -o -iname "*.sh"` → no output. N/A. +- `DIAGRAM_DRIFT` — row was PENDING pre-seal (expected, updated in this + same pass); not drift. + +**Seal gate.** Note claims no outward-facing action (no commit/push). +Confirmed via `git status --short` on branch `156-post-apiv2-authregister`: +2 untracked files only (the evidence note, the new test file) — nothing +staged, committed, or pushed. + +**Proportion (SmallestDiff).** 1 new test file, 0 production `src/` +changes. Proportionate: the file the bug report names no longer exists, +and the replacement path was independently confirmed (not just +asserted) to already be correct and already partially tested; the only +real gap — route-to-handler wiring — is exactly what the new test +closes. + +## Missing + +None. + +## Re-run + +Partial — re-ran the two directly relevant suites +(`v2AuthRoute.test.ts`, `auth.service.test.ts`) myself for extra +confidence beyond reading the note's `npm test` output back; did not +re-run the full `npm test` suite (audit-only default for a non-outward- +facing, non-release-gate change; note's full-suite output was already +verbatim and unredacted). diff --git a/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md b/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md index 770bae2..75cbaea 100644 --- a/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md +++ b/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md @@ -61,7 +61,7 @@ flowchart TD | `fix-refresh-token-expiry-unused` | SEALED | GitHub issue #155. Backfill, same pattern as `fix-idor-broken-access-control`/`fix-create-response-null-id`: the node's own PENDING description cited `api/v1/auth/services/login.ts`, a file removed/merged by `consolidate-v1-v2-auth` (SEALED 2026-08-29) — stale. Re-reading the real call sites shows `TOKEN_EXP_IN` IS already passed at both `jwtSign()` sites (`src/auth/auth.service.ts:126-127` `handlerLogin`, `src/auth/auth.controller.ts:153-154` `authRefreshToken`, both `{ expiresIn: TOKEN_EXP_IN || '1h' }` for access / `{ expiresIn: TOKEN_REFRESH_EXP_IN }` for refresh), part of the same bundled commit `f355e2f` (2026-08-21) as the other 2 bookkeeping-gap nodes sealed this session — never backfilled on the diagram until now. The real remaining gap this node closed: no test decoded a real issued JWT to prove `exp - iat` actually reflects the configured duration (the mocked `auth.service.test.ts` assertion would pass even with the wrong config var or shared access/refresh lifetimes) — exactly what issue #155's acceptance criteria calls for. 1 new test file (`src/__tests__/auth/tokenExpiry.test.ts`), 0 production changes — proportionate given the fix is already live. Uses real unmocked `jwtSign`/`jwtVerify` + real config, `jest.resetModules()`-and-re-`require()`'d per test so env var changes actually take effect: asserts `TOKEN_EXP_IN='2h'` → access `exp-iat===7200`, `TOKEN_REFRESH_EXP_IN='14d'` → refresh `exp-iat===1209600` (2 distinct config-driven lifetimes, not a shared default), plus 2 regression checks (`TOKEN_EXP_IN` unset → 1h fallback; `TOKEN_REFRESH_EXP_IN` unset → 7d default). `npm test`: 143/143 passed (140 baseline + 3 new). SEALED 2026-09-28 after independent verifier pass: cited call sites re-read and matched verbatim; `tokenExpiry.test.ts` read in full and independently re-run (`npx jest src/__tests__/auth/tokenExpiry.test.ts` → 3/3 passed, matching the note); GitHub issue #155's 3 acceptance criteria checked one at a time, all covered; confirmed via `git status --short` on `155-token-exp-in` that nothing was committed/pushed. Evidence: `evidence/verifier/2026-09-28/fix-refresh-token-expiry-unused-seal.md`. | | `fix-candidate-password-leak` | SEALED | GitHub issue #152. `src/candidate/candidate.service.ts` — `handlerGetInformationByEmail` has no `.select()` at all; `handlerGetInformationById` double-wraps `whitelistSelect([select])`, making the select a permanent no-op. Result: `GET /api/v1/candidate/:email` and `PUT/PATCH /candidate/update` return the raw bcrypt password hash in the response. Backfill, same pattern as `fix-idor-broken-access-control`/`fix-create-response-null-id`: the fix itself was already live since commit `f355e2f` (2026-08-21) — `handlerGetInformationByEmail` uses `.select('-password')`, `handlerGetInformationById` defaults to `.select(select \|\| '-password')` — independently confirmed by reading current `src/candidate/candidate.service.ts` (lines 37-53), matches the cited snippet verbatim. A stale evidence note from 2026-08-21 recorded this same diff but was never picked up by a verifier (`sealed_pending_verifier` left unresolved for a month); this pass covers the real remaining gap: no regression test existed anywhere. 1 new test file, 0 production changes, proportionate: `src/__tests__/candidate/candidate.service.test.ts` (confirmed present, 3 tests — `handlerGetInformationByEmail` always calls `.select('-password')`; `handlerGetInformationById` defaults to `-password` with no explicit select; `handlerGetInformationById` passes a given whitelisted select string through unchanged, proving the double-wrap no-op is gone). `npm test`: 143/143 passed (140 prior + 3 new), verbatim and not truncated, command matches `doctrine/MEMORY.md`. All 3 issue-#152 acceptance criteria met: GET `/candidate/:email` no `password` field, PUT/PATCH `/candidate/update` no `password` field, regression test added. SEALED 2026-09-28 after independent verifier pass (audit-only, no re-run — confirmed via `git status --short` on `152-password-hash-leaked` that nothing was committed/pushed). Evidence: `evidence/verifier/2026-09-28/fix-candidate-password-leak-seal.md`. | | `fix-refresh-token-expiry-unused` | PENDING | `TOKEN_EXP_IN` (`.env`, already exported in config) is never used at any `jwtSign()` call site (`auth.service.ts`, `auth.controller.ts`, `api/v1/auth/services/login.ts`) — access and refresh tokens always share the same default 1h expiry, so the refresh token is pointless. | -| `fix-v2-register-missing-await` | PENDING | `src/api/v1/auth/services/register.ts:44` — `bcryptGenerateSalt(password)` missing `await`, the Promise gets assigned straight into the Mongoose model's password field → every `POST /api/v2/auth/register` fails with a Promise→string cast error. | +| `fix-v2-register-missing-await` | SEALED | `src/api/v1/auth/services/register.ts:44` — `bcryptGenerateSalt(password)` missing `await`, the Promise gets assigned straight into the Mongoose model's password field → every `POST /api/v2/auth/register` fails with a Promise→string cast error. One level past the other backfills in this batch: the named buggy file doesn't exist anywhere in the repo any more (`find src/api -iname "*.ts"` empty) — deleted outright by `consolidate-v1-v2-auth` (#77, SEALED 2026-08-29), which routes v2's `/register` through the exact same `authRegister` → `handlerRegister` (`src/auth/auth.service.ts:48`) that v1 always used, and that path already correctly `await`s `bcryptGenerateSalt`. Independently confirmed: `src/routers/api/v2/auth.route.ts` imports `authRegister` from `@/auth/auth.controller`; `handlerRegister` has `const bcryptPwd = await bcryptGenerateSalt(password);`; the existing `auth.service.test.ts` "should register successfully with new email" test mocks the hash via `mockResolvedValue` and asserts `CandidateModel.create` receives the plain string `mockHash` — a missing `await` would leave `bcryptPwd` a Promise and fail that `toHaveBeenCalledWith`, so this is a real load-bearing assertion, not an accidental-pass happy path. The genuine gap this diff closes: nothing proved v2's route actually reaches that handler. New `src/__tests__/auth/v2AuthRoute.test.ts` inspects the real `router.stack` and asserts the `/register` layer's handle is the literal `authRegister` function reference (`.toBe`, identity not deep-equal) — re-run independently by the verifier (`npx jest v2AuthRoute.test.ts auth.service.test.ts`): 2 suites, 14 tests, all pass. 1 new test file, 0 production changes — proportionate given the vulnerable code path no longer exists. Live end-to-end not performed (same sandbox constraint as the rest of this batch: no Mongo/Redis/Docker), disclosed honestly. SEALED 2026-09-28 after independent verifier pass (audit-only + targeted test re-run; confirmed via `git status --short` on `156-post-apiv2-authregister` that nothing was committed/pushed). Evidence: `evidence/verifier/2026-09-28/fix-v2-register-missing-await-seal.md`. | | `fix-create-response-null-id` | SEALED | Minor. `BaseService.ts` `handlerCreate`'s `hookAfterSave` reassigns the local destructured `data` variable, never actually updating what `baseCreateDocument` returns → every `POST .../create` response has `data._id: null` instead of the real new ID. Backfill, same pattern as `fix-idor-broken-access-control`: the fix itself was already live since commit `f355e2f` (2026-08-21, `src/services/index.ts` lines ~300-302, `const replacement = await props.hookAfterSave(...); if (replacement !== undefined) _data = replacement;`) — independently confirmed via `git show f355e2f -- src/services/index.ts` and the current file, both match. This node closed the real gap: no regression test existed. 2 new test files added (`src/__tests__/services/baseCreateDocument.test.ts` — root-cause unit coverage; `src/__tests__/candidate_profile/BaseService.test.ts` — unmocked `createCrudService`/`BaseService.ts`/`services/index.ts` spot-check, only the Mongoose model faked), 0 production changes — proportionate. `npm test`: 140/140 passed. Live HTTP end-to-end was not performed (no local Mongo/Redis/Docker in the implementer's sandbox, disclosed honestly, not hidden) — accepted the unmocked code-path spot-check as a reasonable substitute given the fix has been live and stable for 5+ weeks. SEALED 2026-09-28 after independent verifier pass (audit-only, no re-run — note's `npm test` output was verbatim, not truncated, command matched `doctrine/MEMORY.md`; confirmed via `git status --short` on `157-post-create-responses` that nothing was committed/pushed). Evidence: `evidence/verifier/2026-09-28/fix-create-response-null-id-seal.md`. | | `add-candidate-self-delete` | PENDING | Feature (not a bug). No endpoint lets a candidate delete their own account — needed to clean up 2 test accounts created during live-verification of the 5 bug fixes above on production (`livecheck+...@example.com`, `livecheckB+...@example.com`). Requirement: `DELETE /api/v1/candidate`, using only `req.user._id` (never an id from the client — follows the IDOR-safe pattern from `fix-idor-broken-access-control`), cascade-deletes data across all 7 CV section models by `candidateId`. | | `fix-candidate-me-candidateid-not-string` | SEALED | **Critical, found by accident while testing #79.** GitHub issue #153. `candidate_me/index.ts` `handlerGetAboutMe` — `_id` from the raw Mongoose document is an ObjectId instance, passed straight into `idQuerySafe.safeQuery({}, { candidateId: _id })` — `QuerySafe.safeQuery` only accepts `typeof value === 'string'`, so an ObjectId silently fails that check and `candidateId` gets dropped from the filter → `model.find({})` returns CV data (education/experience/award/certificate/project/generalInformation) for **every candidate mixed together**, on every `GET /api/me/:email` request (public, no auth) and `/download-pdf`. Live-tested confirmed: a brand-new candidate profile returned real data belonging to `votan.it@gmail.com`. Backfill, same pattern as `fix-create-response-null-id`/`fix-candidate-password-leak`: the fix itself (`.toString()` on `_id`) was already live since the #135 pass (2026-09-19) — independently confirmed via `src/candidate_me/index.ts` lines 122-129, matching the note's citation exactly; also confirmed `fnExportPDF` (`/download-pdf`) calls `handlerGetAboutMe` internally rather than re-implementing its own filter, so no separate call site needed a fix, and that the only other `candidateId`-bearing query in the file (`Profile.findOne` at line 100) bypasses `QuerySafe` entirely and was never exposed to this bug class. This node's real gap: no regression test exercised an ObjectId-typed `_id` (existing tests used a plain string, which already passes the string-only `typeof` check even with the old buggy code). 1 new describe block added to `src/__tests__/candidate_me/index.test.ts` (ObjectId-shaped `_id` via `{ toString() }`, asserts stringified `candidateId` reaches every section's `.find()`, never the raw object, never an unfiltered `{}`), 0 production changes — proportionate. `npm test`: 141/141 passed, independently re-run by the verifier (both the targeted file and the full suite) given the Critical severity, numbers matched exactly. Live HTTP end-to-end against 2+ real accounts (issue's 2nd acceptance criterion) was not performed (no local Mongo/Redis/Docker in the implementer's sandbox, disclosed honestly) — accepted the unmocked code-path regression test as a reasonable substitute, same pattern already accepted for `fix-create-response-null-id`/`fix-candidate-password-leak`, given the fix has been live and stable for 9+ days and the issue's own diagnostic section already recorded the original live-test finding against production data. SEALED 2026-09-28 after independent verifier pass (partial re-run; confirmed via `git status --short` on `153-critical-candidate-me` that nothing was committed/pushed). Evidence: `evidence/verifier/2026-09-28/fix-candidate-me-candidateid-not-string-seal.md`. | diff --git a/src/__tests__/auth/v2AuthRoute.test.ts b/src/__tests__/auth/v2AuthRoute.test.ts new file mode 100644 index 0000000..de79b7e --- /dev/null +++ b/src/__tests__/auth/v2AuthRoute.test.ts @@ -0,0 +1,33 @@ +/** + * Regression coverage for issue #156 — POST /api/v2/auth/register used to + * always fail with a Promise->string cast error, from a missing `await` + * on `bcryptGenerateSalt(password)` in a v2-only file + * (`src/api/v1/auth/services/register.ts`). That file no longer exists — + * it was removed when `consolidate-v1-v2-auth` (issue #77, SEALED + * 2026-08-29) merged v1/v2 into a single shared implementation. Today, + * `src/routers/api/v2/auth.route.ts` wires `/register` directly to the + * SAME `authRegister` controller (`@/auth/auth.controller`) that v1 uses, + * which calls `handlerRegister` (`@/auth/auth.service.ts`) — already + * correctly `await`s `bcryptGenerateSalt` (line 48) and is already + * covered by `auth.service.test.ts`'s "should register successfully" + * test (asserts `CandidateModel.create` receives the resolved hash + * string, not a pending Promise — would fail without the `await`). + * + * This test closes the one remaining gap: proving v2's `/register` route + * really delegates to that same, already-tested, already-correct + * handler, and isn't a separate, still-broken code path. + */ +import v2AuthRouter from '@/routers/api/v2/auth.route'; +import { authRegister } from '@/auth/auth.controller'; + +describe('v2 auth route wiring (issue #156)', () => { + it('POST /register delegates to the same authRegister controller as v1 — no separate, unfixed v2-only code path', () => { + const registerLayer = (v2AuthRouter as any).stack.find((layer: any) => layer.route?.path === '/register'); + + expect(registerLayer).toBeDefined(); + expect(registerLayer.route.methods.post).toBe(true); + // Same function reference as v1's — not a re-implementation that + // could independently regress. + expect(registerLayer.route.stack[0].handle).toBe(authRegister); + }); +});