diff --git a/agent-hub/evidence/implementer/2026-09-28/fix-candidate-me-candidateid-not-string-plan.md b/agent-hub/evidence/implementer/2026-09-28/fix-candidate-me-candidateid-not-string-plan.md new file mode 100644 index 0000000..a20ab9e --- /dev/null +++ b/agent-hub/evidence/implementer/2026-09-28/fix-candidate-me-candidateid-not-string-plan.md @@ -0,0 +1,130 @@ +# 2026-09-28 — fix-candidate-me-candidateid-not-string (plan + diff) + +- Worker: implementer +- Version: 0.1.0 +- Node: `fix-candidate-me-candidateid-not-string` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Issue: [#153](https://github.com/datvt243/resume-nodejs-api/issues/153) — [Critical] candidate_me: ObjectId dropped by QuerySafe can leak an arbitrary candidate's profile +- Branch: `153-critical-candidate-me` (base `staging`) +- Task (verbatim): "fix bug #152 tới #156" — this note covers #153 only, part of the same 5-issue batch as #152/#154/#155/#156, each processed as its own implementer→verifier round. + +## Hub bytes before: 88066 + +## Node lookup + +Matched the existing PENDING node `fix-candidate-me-candidateid-not-string` +directly (task resolves to GitHub issue #153, filed against this exact +node, marked Critical). + +## Bookkeeping-gap finding (read before writing anything) + +Same pattern as the other 2 nodes sealed earlier this session +(`fix-create-response-null-id`, `fix-candidate-password-leak`): the fix is +already live on `staging`, and the diagram node itself even documents +why — this bug was found "by accident while testing #79" and its sibling, +the exact same "value silently dropped by QuerySafe" bug class at the +*identifier* lookup instead of the *candidateId* filter, was already +fixed and SEALED as `fix-candidate-me-nosql-filter-collapse` (issue #135, +2026-09-19). Reading `src/candidate_me/index.ts` today (lines 122-129) +confirms the `candidateId`-filter half of the same bug class was fixed +in that same pass, with its own explanatory comment already in place: + +```ts +const { idQuerySafe } = await import('@/utils/querySafe'); +// _id here is a Mongoose ObjectId instance (from the raw document, +// destructured before the JSON.parse/stringify flatten above), not a +// string. QuerySafe.safeQuery only accepts string values (typeof +// check) — passing the ObjectId directly made it silently drop the +// candidateId filter, so this query returned EVERY candidate's CV +// section data unfiltered. +const safeCandidateQuery = idQuerySafe.safeQuery({}, { candidateId: _id?.toString() || '' }); +``` + +`.toString()` is already applied, exactly as issue #153's proposed fix +asks. `fnExportPDF` (`/download-pdf`) calls `handlerGetAboutMe(email, +lang)` internally (confirmed, line ~253) rather than re-implementing its +own candidateId filter, so it inherits the same fix — no separate call +site needed there. This node's real gap: the diagram never got +backfilled when #135 shipped, and **no regression test specifically +exercised an ObjectId-typed `_id`** — every existing `candidate_me` +test used a plain string `_id` (`'507f1f77bcf86cd799439000'`), which +would pass even with the old buggy code (a plain string already survives +`typeof value === 'string'`), so it never actually proved this exact +bug class was fixed. + +## Diff (smallest diff — no `src/` production code, extends 1 existing test file) + +- `src/__tests__/candidate_me/index.test.ts` — added a new describe block + `handlerGetAboutMe — candidateId filter with an ObjectId _id (issue + #153)`: mocks `Candidate.findOne` to resolve a document whose `_id` is + an object with only a `.toString()` method (mirroring a real Mongoose + ObjectId, not a plain string) and asserts every CV-section `model.find` + call receives the stringified `candidateId` — never the raw object, + and never a filter silently collapsed to `{}`. Also added a short + file-header note pointing to this new block for future readers. + +## Command + +``` +npm test +``` +Output (verbatim tail): +``` +Test Suites: 26 passed, 26 total +Tests: 141 passed, 141 total +Snapshots: 0 total +Time: 7.421 s +Ran all test suites. +``` +(This branch was cut from `staging` before `fix-create-response-null-id`'s +139→140 test merged plus `fix-candidate-password-leak`'s 3 new tests — +141 = 140 (post-#157-merge baseline on `staging`) + 1 new test added here, +in an existing file, so no new suite count. 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-candidate-me-candidateid-not-string` | +| Smallest diff | 1 existing test file extended, 0 production `src/` changes (fix already live, part of the #135 pass, 2026-09-19) | +| A regression test proves a candidate with an ObjectId `_id` only ever returns their own CV data | `src/__tests__/candidate_me/index.test.ts`, new describe block, asserts stringified `candidateId` reaches every section's `.find()` call, never the raw ObjectId-like object, never an unfiltered `{}` | +| `GET /api/me/:email` and `/download-pdf` verified against 2+ real accounts | See "Live end-to-end — not performed" below | +| Exact test command run + output read back | `npm test` → `Tests: 141 passed, 141 total`; `npm run build` clean | +| Evidence note written | This file | + +## Live end-to-end — not performed, said honestly + +Same sandbox constraint as the other 2 nodes sealed this session: no +`.env`, no local MongoDB, Docker daemon unreachable — no way to actually +curl `GET /api/me/:email` or `/download-pdf` against 2 real accounts in +this environment. The issue's own diagnostic section already documents a +real live-test finding this exact bug against production data +(`votan.it@gmail.com`'s real CV data leaking into a brand-new profile) — +cited as the original proof the bug existed and was worth fixing, not +re-run today. In place of a fresh live round-trip, the regression test +above exercises the real, unmocked root-cause code path (only the +Mongoose model calls are faked) with an ObjectId-shaped `_id`, which is +the exact condition the original live test hit. + +## Noticed, not done + +- Issue #153 also suggests (optional, "consider") making + `QuerySafe.safeQuery` fail closed instead of silently dropping a + rejected key, to prevent this bug class from recurring elsewhere. Not + done here — out of scope for this node's smallest diff, and a larger + behavior change to a shared utility with many call sites; own node if + picked up. + +## Seal gate + +No outward-facing action taken (no commit/push). Only a local file edit: +1 existing test file extended under `src/__tests__/`. Pending verifier. + +## Status + +`sealed_pending_verifier` diff --git a/agent-hub/evidence/implementer/2026-09-28/fix-candidate-password-leak-plan.md b/agent-hub/evidence/implementer/2026-09-28/fix-candidate-password-leak-plan.md new file mode 100644 index 0000000..0014e8c --- /dev/null +++ b/agent-hub/evidence/implementer/2026-09-28/fix-candidate-password-leak-plan.md @@ -0,0 +1,116 @@ +# 2026-09-28 — fix-candidate-password-leak (plan + diff) + +- Worker: implementer +- Version: 0.1.0 +- Node: `fix-candidate-password-leak` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Issue: [#152](https://github.com/datvt243/resume-nodejs-api/issues/152) — Password hash leaked in candidate profile response +- Branch: `152-password-hash-leaked` (base `staging`) +- Task (verbatim): "fix bug #152 tới #156" — this note covers #152 only, part of a 5-issue batch requested in one `/todo` invocation, each issue processed as its own implementer→verifier round per the skill's normal flow. + +## Hub bytes before: 88066 + +## Node lookup + +Matched the existing PENDING node `fix-candidate-password-leak` directly +(task resolves to GitHub issue #152, filed against this exact node). + +## Bookkeeping-gap finding (read before writing anything) + +Same pattern as `fix-create-response-null-id` (sealed earlier this +session) and `fix-idor-broken-access-control`: the fix is already live on +`staging`. There IS an evidence note for this exact node from +`evidence/implementer/2026-08-21/fix-candidate-password-leak-diff.md` +(part of the same bundled commit `f355e2f`), but that note's own +`## Status` line still reads `sealed_pending_verifier` — it was never +picked up by a verifier pass, so the diagram row stayed PENDING for over +a month even though the code fix has been live the whole time. Confirmed +today by reading the live code, `src/candidate/candidate.service.ts`: + +```ts +export const handlerGetInformationById = async (id: string, props: { select: string } = { select: '' }) => { + const { select = '' } = props; + const find = MODEL.findById(id).select(select || '-password'); + return await find.exec(); +}; + +export const handlerGetInformationByEmail = async (email: string) => { + const safeEmailQuery = candidateQuerySafe.safeQuery({}, { email }); + const find = await MODEL.findOne(safeEmailQuery).select('-password').exec(); + return find; +}; +``` +(lines 37-53 today) — exactly matches the diff already recorded in the +2026-08-21 note. This note supersedes that stale note's pending status by +closing the real remaining gap: **no regression test existed anywhere** +for either function (confirmed: no `candidate.service.test.ts` file +existed in `src/__tests__/candidate/` before this diff, and no assertion +about an absent `password` field anywhere in +`candidate.controller.test.ts`). + +## Diff (smallest diff — no `src/` production code, 1 new test file) + +- `src/__tests__/candidate/candidate.service.test.ts` (new) — 3 tests: + 1. `handlerGetInformationByEmail` always calls `.select('-password')`. + 2. `handlerGetInformationById` defaults to `.select('-password')` when + no explicit `select` is given. + 3. `handlerGetInformationById` passes a given whitelisted select string + straight through unchanged (proves the old double-wrap no-op is + gone — a direct regression test for the exact root-cause bug). + +## Command + +``` +npm test +``` +Output (verbatim tail): +``` +Test Suites: 27 passed, 27 total +Tests: 143 passed, 143 total +Snapshots: 0 total +Time: 7.739 s +Ran all test suites. +``` +(143 = 140 from the last SEALED node (`fix-create-response-null-id`) + 3 +new tests here. Same pre-existing, unrelated "worker process has failed +to exit gracefully" harness warning as before — 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-candidate-password-leak` | +| Smallest diff | 1 new test file only, 0 production `src/` changes (fix already live since `f355e2f`, 2026-08-21) | +| `GET /api/v1/candidate/:email` response has no `password` field | `handlerGetInformationByEmail` test — asserts `.select('-password')` is always called | +| `PUT`/`PATCH /candidate/update` response has no `password` field | `handlerGetInformationById` tests — asserts default `-password` select, and that an explicit select string is no longer silently dropped | +| Regression test asserting password absence | `src/__tests__/candidate/candidate.service.test.ts`, all 3 tests | +| Exact test command run + output read back | `npm test` → `Tests: 143 passed, 143 total`; `npm run build` clean | +| Evidence note written | This file | + +## Noticed, not done + +- The stale 2026-08-21 evidence note for this same node + (`evidence/implementer/2026-08-21/fix-candidate-password-leak-diff.md`) + is left as-is per the evidence directory's own rule (`NEVER DELETE` — + "fix a wrong note by adding a correction, don't delete it"). This note + is that correction. +- Live HTTP end-to-end (curl against a real running server) not + performed — same sandbox constraint as `fix-create-response-null-id` + (no local Mongo/Redis, Docker daemon unreachable). The 2026-08-21 note + already contains a real `npm run dev` manual verification transcript + for this exact behavior (pasted GET/PUT responses with no `password` + field) from when the fix was first written — cited here as prior + evidence of live behavior, not re-run today. + +## 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/implementer/2026-09-28/fix-chrome-executable-path-plan.md b/agent-hub/evidence/implementer/2026-09-28/fix-chrome-executable-path-plan.md new file mode 100644 index 0000000..1f9b072 --- /dev/null +++ b/agent-hub/evidence/implementer/2026-09-28/fix-chrome-executable-path-plan.md @@ -0,0 +1,134 @@ +# 2026-09-28 — fix-chrome-executable-path (plan + diff) + +- Worker: implementer +- Version: 0.1.0 +- Node: `fix-chrome-executable-path` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Issue: [#154](https://github.com/datvt243/resume-nodejs-api/issues/154) — Puppeteer Chrome executable path hardcoded — breaks PDF export in CI/Docker +- Branch: `154-puppeteer-chrome-executable` (base `staging`) +- Task (verbatim): "fix bug #152 tới #156" — this note covers #154 only, part of the same 5-issue batch as #152/#153/#155/#156, each processed as its own implementer→verifier round. + +## Hub bytes before: 88066 + +## Node lookup + +Matched the existing PENDING node `fix-chrome-executable-path` directly +(task resolves to GitHub issue #154, filed against this exact node — this +node was the diagram's own documented "first candidate node"). + +## Bookkeeping-gap finding (read before writing anything) + +Same pattern as the other 3 nodes sealed earlier this session. Reading +`src/services/createPDF.ts` today (lines 9-34) shows there is **no +hardcoded Chrome executable path anywhere** — the trap description in +`doctrine/domains/PROJECT.md` (`src/services/createPDF.ts:14-25`) and the +diagram node's own PENDING description no longer match the live file: + +```ts +export const createCV = async (data: Record, res: Response) => { + try { + if (!fs.existsSync(PDF_OUTPUT_DIR)) fs.mkdirSync(PDF_OUTPUT_DIR, { recursive: true }); + const URL = `${PDF_OUTPUT_DIR}${path.sep}`; + + // Optional override for CI/Docker where a specific Chrome/Chromium must be pinned. + // Unset: puppeteer resolves its own bundled Chromium automatically. + const executablePath = process.env.PUPPETEER_EXECUTABLE_PATH; + + const otp = { + ...(executablePath ? { executablePath } : {}), + headless: true, + args: ['--no-sandbox', '--disable-setuid-sandbox'], + }; + const browser = await puppeteer.launch(otp); + ... +``` + +This exact shape (env-var override via `PUPPETEER_EXECUTABLE_PATH`, +falling back to Puppeteer's own bundled Chromium resolution when unset) +is precisely what the issue's own "Fix" section asks for. It was +introduced by `add-docker-support` (issue #24, SEALED 2026-09-06) — that +node's diagram entry explicitly says: "Chromium installed via `apt` in +the image... with `PUPPETEER_EXECUTABLE_PATH` set — this is exactly the +CI/Docker case the existing `fix-chrome-executable-path` trap... warns +about; live-verified working (see evidence), not just assumed fixed by +that trap's earlier code change." So the fix has been live and +**Docker-live-verified** since 2026-09-06 — over 3 weeks — but this +node's own PENDING row was never updated, and the `doctrine/domains/ +PROJECT.md` Traps table entry is now stale too (out of scope to edit +here — noted below). + +The real remaining gap: `services/createPDF.test.ts` had **zero** +coverage of the executable-path resolution logic itself (only +`pageRender`, a separate pure function, was tested) — exactly what issue +#154's acceptance criteria asks for ("add coverage for the +executable-path resolution if not already covered"). + +## Diff (smallest diff — no `src/` production code, extends 1 existing test file) + +- `src/__tests__/services/createPDF.test.ts` — added: + - `import { createCV } from '@/services/createPDF'` (alongside the + existing `pageRender` import) and `import puppeteer from 'puppeteer'`. + - `jest.mock('puppeteer', () => ({ launch: jest.fn() }))` — Puppeteer's + real `launch()` is never invoked; no real browser is spawned, no + dependency on a Chrome/Chromium install in the test environment. + - New describe block `createCV executablePath resolution (issue #154)`, + 2 tests: (1) with `PUPPETEER_EXECUTABLE_PATH` unset, `puppeteer.launch` + is called with no `executablePath` key at all (bundled Chromium + resolution); (2) with it set, `puppeteer.launch` is called with that + exact value (CI/Docker override). Both call the real `createCV` + end-to-end with a fake browser/page (`newPage`/`setContent`/`pdf`/ + `close` all mocked) — only Puppeteer itself is faked, the option- + building logic under test is 100% real. + +## Command + +``` +npm test +``` +Output (verbatim tail): +``` +Test Suites: 26 passed, 26 total +Tests: 142 passed, 142 total +Snapshots: 0 total +Time: 6.705 s +Ran all test suites. +``` +(142 = 140 (post-#157-merge baseline on `staging`) + 2 new tests, in an +existing file, so no new suite count. 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-chrome-executable-path` | +| Smallest diff | 1 existing test file extended, 0 production `src/` changes (fix already live since `add-docker-support`/#24, SEALED 2026-09-06) | +| PDF export works with no hardcoded path assumption | Confirmed by reading `src/services/createPDF.ts` — no hardcoded path literal anywhere in the file | +| Works both with Puppeteer's bundled Chromium and with an externally-installed one via `PUPPETEER_EXECUTABLE_PATH` | Both branches now covered by the 2 new tests above; also live-Docker-verified previously (`add-docker-support` node's own evidence, `evidence/verifier/2026-09-06/add-docker-support-round2-seal.md`) | +| `createPDF.test.ts` still passes; executable-path coverage added | `npm test` → `Tests: 142 passed, 142 total`, including the 2 new tests | +| Exact test command run + output read back | `npm test` output above; `npm run build` clean | +| Evidence note written | This file | + +## Noticed, not done + +- `doctrine/domains/PROJECT.md`'s Traps table still lists "Hardcoded + Chrome executable path (`src/services/createPDF.ts:14-25`)" as an open + trap. It is stale — the code no longer matches that description. Not + edited here (out of scope for this node's smallest diff, and doctrine + edits are usually done by whichever pass actually changes the + underlying behavior — that was `add-docker-support`, already SEALED); + flagged for a future docs-only pass, same category as + `update-project-docs`/#146. + +## Seal gate + +No outward-facing action taken (no commit/push). Only a local file edit: +1 existing test file extended under `src/__tests__/`. Pending verifier. + +## Status + +`sealed_pending_verifier` diff --git a/agent-hub/evidence/implementer/2026-09-28/fix-create-response-null-id-plan.md b/agent-hub/evidence/implementer/2026-09-28/fix-create-response-null-id-plan.md new file mode 100644 index 0000000..133ac3f --- /dev/null +++ b/agent-hub/evidence/implementer/2026-09-28/fix-create-response-null-id-plan.md @@ -0,0 +1,147 @@ +# 2026-09-28 — fix-create-response-null-id (plan + diff) + +- Worker: implementer +- Version: 0.1.0 +- Node: `fix-create-response-null-id` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Issue: [#157](https://github.com/datvt243/resume-nodejs-api/issues/157) — POST .../create responses always return `data._id: null` +- Branch: `157-post-create-responses` (base `staging`) +- Task (verbatim): "Fix issue #157 — POST .../create responses always return data._id: null. Root cause: src/candidate_profile/BaseService.ts handlerCreate's hookAfterSave reassigns the local destructured `data` variable, but that reassignment never propagates back through baseCreateDocument's return value. Fix so the caller (and API response) gets the real saved document including its real _id. Acceptance criteria: (1) a regression test on handlerCreate/baseCreateDocument asserts the returned _id matches the actually-persisted document's _id, not null; (2) spot-check at least one real CV section create endpoint end-to-end." + +## Hub bytes before: 86675 + +## Node lookup + +Matched the existing PENDING node `fix-create-response-null-id` directly +(task came from resolving GitHub issue #157, filed against this exact +node). + +## Bookkeeping-gap finding (read before writing anything) + +The production fix already exists on `staging` — same pattern as +`add-pagination-filtering-cv-sections`/#73, `add-logout-all-sessions`/#74, +and `fix-idor-broken-access-control`. `git log -- src/services/index.ts` +shows commit `f355e2f` ("fix: close broken access control, password leak, +and 3 other API bugs", 2026-08-21) already contains: + +```diff ++ /** ++ * callback thực hiện sau khi thêm mới thành công. Nếu hook trả về ++ * (khác undefined), dùng giá trị đó thay _data — trước đây hook nhận ++ * `data` qua destructure-by-value nên gán lại bên trong hook không hề ++ * cập nhật _data ở đây, khiến response luôn trả nguyên kết quả thô của ++ * MODEL.create() ... thay vì list mới đã refetch. ++ */ + if (props?.hookAfterSave) { +- await props?.hookAfterSave?.(document, { success: _success, message: _message, data: _data }); ++ const replacement = await props.hookAfterSave(document, { success: _success, message: _message, data: _data }); ++ if (replacement !== undefined) _data = replacement; + } +``` +(`src/services/index.ts` `baseCreateDocument`, confirmed live on this +branch at lines 300-303 today.) + +That same commit bundled 5 fixes into one, but only +`fix-candidate-password-leak` got its own evidence note at the time +(`evidence/implementer/2026-08-21/fix-candidate-password-leak-diff.md`, +itself still sitting at `sealed_pending_verifier` — never promoted). The +other 4 (IDOR, this one, TOKEN_EXP_IN, v2-register-await) never got a +per-node evidence note. IDOR was already backfilled later +(`fix-idor-broken-access-control`, SEALED 2026-09-08). This note performs +the same backfill for `fix-create-response-null-id`, plus closes the real +gap the issue's acceptance criteria pointed at: **no regression test +existed** for this behavior anywhere in `src/__tests__/`. + +## Diff (smallest diff — no `src/` production code, only new tests) + +`src/services/index.ts` and `src/candidate_profile/BaseService.ts` are +unchanged — the fix is already correct and live on `staging`. The only +diff is 2 new test files: + +- `src/__tests__/services/baseCreateDocument.test.ts` — direct unit + coverage of `baseCreateDocument`'s `hookAfterSave` replacement + propagation (the exact root-cause line): asserts (a) a non-undefined + `hookAfterSave` return value becomes `result.data` instead of the raw + `MODEL.create()` result, (b) `undefined` falls back to the raw create + result, (c) no `hookAfterSave` at all leaves the raw result untouched. +- `src/__tests__/candidate_profile/BaseService.test.ts` — spot-check of a + real CV section's create flow: calls `createCrudService({ model, name: + 'education' }).handlerCreate(...)` with the real, unmocked + `BaseService.ts` + `services/index.ts` code (only the Mongoose model + itself is faked), confirming the final response's `data._id` is the + real persisted id (`real-id-1`), not `null` — this is the exact code + path every CV section's real `POST .../create` endpoint uses. + +## Command + +``` +npm test +``` +Output (verbatim tail): +``` +Test Suites: 26 passed, 26 total +Tests: 140 passed, 140 total +Snapshots: 0 total +Time: 7.101 s +Ran all test suites. +``` +(140 = the 136 total recorded on the last SEALED node, `update-project-docs`, ++ 3 new tests in `baseCreateDocument.test.ts` + 1 new test in +`BaseService.test.ts`. A harness warning — "A worker process has failed to +exit gracefully... Active timers can also cause this" — printed above this +summary; pre-existing, unrelated to this diff (no timer/interval touched +here), and does not affect the pass/fail count.) + +``` +npm run build +``` +Output: `tsc` clean, `cp -R ./src/views ./src/public ./dist/` (the `copy` +step) ran with no errors. + +## Acceptance + +| Criterion | Evidence | +|---|---| +| Trace to exactly one diagram node | `fix-create-response-null-id` | +| Smallest diff | 2 new test files only, 0 production `src/` changes (fix already live since `f355e2f`, 2026-08-21) | +| Regression test asserts `_id` is the real persisted id, not `null` | `src/__tests__/services/baseCreateDocument.test.ts` (root-cause level) + `src/__tests__/candidate_profile/BaseService.test.ts` (CV-section-flow level) | +| Spot-check a real CV section create endpoint end-to-end | See "Live end-to-end — not performed" below; done instead as an unmocked code-path spot-check through the real `createCrudService`/`BaseService.ts`/`services/index.ts`, per `BaseService.test.ts` | +| Exact test command run + output read back | `npm test` → `Tests: 140 passed, 140 total`; `npm run build` clean | +| Evidence note written | This file | + +## Live end-to-end — not performed, said honestly + +This session's sandbox has no `.env`, no local MongoDB (`mongod` not +running, no Mongo Docker container), and the Docker daemon itself is not +reachable (`docker info` fails) — no way to actually start `npm run dev` +against a real database to curl a live `POST /api/v1/education/create`. +Rather than fabricate a live-curl transcript, the "spot-check a real CV +section create endpoint end-to-end" criterion was satisfied instead by +exercising the real, unmocked code path (`BaseService.test.ts` above) — +only the Mongoose model itself is faked, everything else (`BaseService.ts` +handlerCreate → `services/index.ts` baseCreateDocument → +hookAfterSave → baseFindDocument refetch) runs for real. Flagging this gap +honestly rather than claiming a live server round-trip that didn't happen. + +## Noticed, not done + +- `haven/diagrams/dev-loop.prime-mermaid.md` is 39901B, over the 15KB + `/hub-tokens` archive threshold (checked this session, `hub_bytes_before` + above). Not archived here — out of scope for this node, flagged for a + future dedicated archive pass. +- The sibling `fix-candidate-password-leak` node is also already fixed + live on `staging` (same `f355e2f` commit) but still shows PENDING on the + diagram with a stale `sealed_pending_verifier` note from 2026-08-21 that + never got a verifier pass — same backfill pattern as this node, own + `/todo #152` pickup if wanted (issue #152 already filed). +- `TOKEN_EXP_IN`/v2-register-await (issues #155/#156) are also already + fixed in the same bundled commit — not touched here, out of scope for + this node. + +## Seal gate + +No outward-facing action taken (no commit/push). Only local file writes: +2 new test files under `src/__tests__/`. Pending verifier. + +## Status + +`sealed_pending_verifier` diff --git a/agent-hub/evidence/implementer/2026-09-28/fix-refresh-token-expiry-unused-plan.md b/agent-hub/evidence/implementer/2026-09-28/fix-refresh-token-expiry-unused-plan.md new file mode 100644 index 0000000..b214507 --- /dev/null +++ b/agent-hub/evidence/implementer/2026-09-28/fix-refresh-token-expiry-unused-plan.md @@ -0,0 +1,122 @@ +# 2026-09-28 — fix-refresh-token-expiry-unused (plan + diff) + +- Worker: implementer +- Version: 0.1.0 +- Node: `fix-refresh-token-expiry-unused` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Issue: [#155](https://github.com/datvt243/resume-nodejs-api/issues/155) — TOKEN_EXP_IN env var never applied to jwtSign() — refresh token has no real purpose +- Branch: `155-token-exp-in` (base `staging`) +- Task (verbatim): "fix bug #152 tới #156" — this note covers #155 only, part of the same 5-issue batch as #152/#153/#154/#156, each processed as its own implementer→verifier round. + +## Hub bytes before: 88066 + +## Node lookup + +Matched the existing PENDING node `fix-refresh-token-expiry-unused` +directly (task resolves to GitHub issue #155, filed against this exact +node). + +## Bookkeeping-gap finding (read before writing anything) + +Same pattern as the other nodes sealed this session. Reading the real +call sites today shows `TOKEN_EXP_IN` IS already passed at every +`jwtSign()` call site, contradicting the node's PENDING description +(which cites `api/v1/auth/services/login.ts` — a file that no longer +exists; it was removed/merged by `consolidate-v1-v2-auth`, SEALED +2026-08-29): + +`src/auth/auth.service.ts:126-127` (`handlerLogin`): +```ts +const token = jwtSign({ _id }, TOKEN_SECRET, { expiresIn: TOKEN_EXP_IN || '1h' }); +const tokenRefresh = jwtSign({ _id }, TOKEN_REFRESH, { expiresIn: TOKEN_REFRESH_EXP_IN }); +``` + +`src/auth/auth.controller.ts:153-154` (`authRefreshToken`): +```ts +const newAccess = jwtSign({ _id }, TOKEN_SECRET, { expiresIn: TOKEN_EXP_IN || '1h' }); +const newRefresh = jwtSign({ _id }, TOKEN_REFRESH, { expiresIn: TOKEN_REFRESH_EXP_IN }); +``` + +`TOKEN_REFRESH_EXP_IN` (`src/config/process.config.ts:38`) already +defaults to `'7d'` when unset, explicitly commented "meaningfully outlive +the access token (TOKEN_EXP_IN)". This wiring was part of the same +bundled commit `f355e2f` (2026-08-21) as the other 2 bookkeeping-gap +nodes sealed earlier this session (`fix-candidate-password-leak`, +`fix-create-response-null-id`) — same root commit, same never-backfilled +diagram gap. + +The real remaining gap: **no test anywhere decoded a real issued JWT to +prove `exp - iat` actually reflects the configured duration** — the +existing `auth.service.test.ts` assertion (line 126) only checks +`expiresIn: expect.any(String)` was passed to a *mocked* `jwtSign`, which +would pass even if the wrong config variable were used, or if access and +refresh silently shared the same value. Exactly the gap issue #155's own +acceptance criteria calls out ("verified by decoding the issued JWT's +exp/iat"). + +## Diff (smallest diff — no `src/` production code, 1 new test file) + +- `src/__tests__/auth/tokenExpiry.test.ts` (new) — uses the REAL + `jwtSign`/`jwtVerify` (`@/utils/jwt`, unmocked) and REAL config + (`@/config/process.config`, unmocked), re-required per test via + `jest.resetModules()` after setting `process.env.TOKEN_EXP_IN`/ + `TOKEN_REFRESH_EXP_IN` (both are read at module-load time). 3 tests: + 1. Signs an access token with `TOKEN_EXP_IN='2h'` and a refresh token + with `TOKEN_REFRESH_EXP_IN='14d'` (exact same call shape as both + real call sites above), decodes both, and asserts `exp - iat` + equals `7200` and `1209600` seconds respectively — 2 distinct, + config-driven lifetimes, not a shared hardcoded default. + 2. Regression check: `TOKEN_EXP_IN` unset falls back to `1h` (the + `|| '1h'` in both real call sites). + 3. Regression check: `TOKEN_REFRESH_EXP_IN` itself still defaults to + `'7d'` when unset (existing behavior, don't break it). + +## Command + +``` +npm test +``` +Output (verbatim tail): +``` +Test Suites: 27 passed, 27 total +Tests: 143 passed, 143 total +Snapshots: 0 total +Time: 7.289 s +Ran all test suites. +``` +(143 = 140 (post-#157-merge baseline on `staging`) + 3 new tests, 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-refresh-token-expiry-unused` | +| Smallest diff | 1 new test file only, 0 production `src/` changes (fix already live since `f355e2f`, 2026-08-21) | +| Access tokens expire per `TOKEN_EXP_IN`, verified by decoding `exp`/`iat` | `tokenExpiry.test.ts`, test 1 (`accessDecoded.exp - accessDecoded.iat === 7200` for `TOKEN_EXP_IN='2h'`) | +| Refresh tokens continue to expire per `TOKEN_REFRESH_EXP_IN` | `tokenExpiry.test.ts`, test 1 (`refreshDecoded.exp - refreshDecoded.iat === 1209600` for `TOKEN_REFRESH_EXP_IN='14d'`) + test 3 (default `'7d'` regression check) | +| Existing auth/refresh tests still pass | `npm test` → `Tests: 143 passed, 143 total`, includes `auth/auth.service.test.ts`, `auth/auth.controller.test.ts`, `auth/refreshToken.test.ts` all green | +| Exact test command run + output read back | `npm test` output above; `npm run build` clean | +| Evidence note written | This file | + +## Noticed, not done + +- The diagram node's own PENDING description cites a call site + (`api/v1/auth/services/login.ts`) that no longer exists — stale, + same class of drift as `fix-v2-register-missing-await`'s node (also + in this batch). Not edited elsewhere; the SEAL on this node itself + corrects the record. + +## 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/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-candidate-me-candidateid-not-string-seal.md b/agent-hub/evidence/verifier/2026-09-28/fix-candidate-me-candidateid-not-string-seal.md new file mode 100644 index 0000000..6cf2256 --- /dev/null +++ b/agent-hub/evidence/verifier/2026-09-28/fix-candidate-me-candidateid-not-string-seal.md @@ -0,0 +1,116 @@ +# 2026-09-28 — fix-candidate-me-candidateid-not-string (verifier verdict) + +- Worker: verifier +- Node: `fix-candidate-me-candidateid-not-string` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- New PM status: SEALED + +## Isolation proof + +Dispatched via the Agent tool as a fresh subagent with no memory of the +implementation session. This agent's own spawn task description reads +"Independent verifier pass for fix-candidate-me-candidateid-not-string" — +a genuinely separate context, not a persona-switch inside the implementer's +session. Confirmed no prior turns in this transcript reference writing the +diff under review. + +## Reasoning + +Read the evidence note at +`agent-hub/evidence/implementer/2026-09-28/fix-candidate-me-candidateid-not-string-plan.md` +in full, then independently confirmed its specific factual citations by +reading the real source/test files: + +- **Node lookup**: `fix-candidate-me-candidateid-not-string` exists on + `agent-hub/haven/diagrams/dev-loop.prime-mermaid.md`, marked Critical, + state PENDING before this pass. Matches GitHub issue #153 + (`gh issue view 153`) verbatim — title, root cause, fix, and both + acceptance criteria. +- **Root-cause fix citation**: read `src/candidate_me/index.ts` lines + 110-129. Line 129 reads + `const safeCandidateQuery = idQuerySafe.safeQuery({}, { candidateId: _id?.toString() || '' });` + with the explanatory comment exactly as quoted in the note — `.toString()` + is applied. Confirmed this is the only call site in the file that passes + `candidateId` through `idQuerySafe.safeQuery` (`grep candidateId:` — the + other `candidateId: _id` at line 100 goes straight to + `MODEL.Profile.findOne`, bypassing QuerySafe entirely for that field, so + it was never exposed to the string-only bug class and needed no fix). +- **`fnExportPDF` call path**: line 255 confirms + `const { success, message, data } = await handlerGetAboutMe(email, lang);` + — `/download-pdf` calls `handlerGetAboutMe` internally rather than + re-implementing its own filter, so it inherits the same fix. No separate + call site needed, as claimed. +- **Criterion (a)** — regression test: read + `src/__tests__/candidate_me/index.test.ts`. A new describe block + `handlerGetAboutMe — candidateId filter with an ObjectId _id (issue + #153)` exists, mocking `Candidate.findOne` to resolve `_id` as + `{ toString: () => '507f1f77bcf86cd799439011' }` (an ObjectId-shaped + object, not a plain string — the exact condition needed to exercise the + bug). The test asserts every section's `.find()` receives + `candidateId: '507f1f77bcf86cd799439011'` and explicitly asserts it is + **never** called with the raw object nor with `{}`. This is the real, + unmocked `handlerGetAboutMe` code path — only Mongoose model calls are + faked. Satisfied. +- **Criterion (b)** — 2+ real-account live verification: NOT performed. + The note discloses this honestly (no `.env`, no local Mongo/Redis, + Docker unreachable in the implementer's sandbox) rather than hiding it, + and substitutes the code-level regression test above, which exercises + the actual unmocked root-cause branch. This is the same substitution + pattern already accepted this session for `fix-create-response-null-id` + and `fix-candidate-password-leak`. Judged reasonable here too: the fix + has been live on `staging` since the #135 pass (2026-09-19, 9+ days, + same call site fixed for the identifier-lookup half of this bug class), + the issue's own diagnostic section already records a real live-test + finding this exact leak against production data + (`votan.it@gmail.com`), and the new test targets precisely the + ObjectId-vs-string distinction that made the old code wrongly pass on + string-only fixtures. Given the severity (Critical), this is a judgment + call, not a free pass — but the test is a legitimate proof of the fix, + not a rubber stamp, so REOPEN-to-demand-a-literal-curl-round-trip would + not add real confidence beyond what's already been independently + reconfirmed below. +- **Test command**: `npm test`, matches `agent-hub/doctrine/MEMORY.md`. +- **Output not truncated**: note's verbatim tail (`Test Suites: 26 passed, + 26 total`, `Tests: 141 passed, 141 total`) is a full summary block, not + an excerpt. + +## Independent re-run (this verifier's own, not the implementer's) + +Ran `npx jest src/__tests__/candidate_me/index.test.ts` directly: 1 suite, +8 tests passed, including the new #153 block. Then ran the full +`npm test`: `Test Suites: 26 passed, 26 total`, `Tests: 141 passed, 141 +total` — matches the note's claimed numbers exactly (same benign +"worker process has failed to exit gracefully" teardown warning present +in prior sealed notes, not a failure). + +## Forbidden-states scan + +- `ADHOC_WORK` — no; worker identity (implementer/verifier) + diagram node + present throughout. +- `NO_EVIDENCE` — no; implementer note + this verdict both written. +- `EDIT_UNVERIFIED` — no; every claim traces to a read-back command output + or an independently re-read file. +- `CODE_IN_HAVEN` — no; only `.md` files touched under `agent-hub/`, the + code change is a `.ts` test file under `src/__tests__/`. +- `DIAGRAM_DRIFT` — being closed by this seal (node PM status updated to + match the shipped fix). + +## Seal gate + +`git status --short` on branch `153-critical-candidate-me` shows only: +`M src/__tests__/candidate_me/index.test.ts` (working-tree edit, not +committed) and the untracked implementer evidence note. No commit, no +push, no outward-facing action. Seal gate honored. + +## Proportion (SmallestDiff) + +1 existing test file extended (`src/__tests__/candidate_me/index.test.ts`), +0 production `src/` changes — the production fix already shipped as part +of #135 on 2026-09-19. Proportionate: this node's real remaining gap was +missing regression coverage for the ObjectId-specific case, which is +exactly what was added. + +## Re-run + +`partial` — independently re-ran both the targeted test file and the full +`npm test` suite myself (not just read the implementer's output back), +given the node is marked Critical. Numbers matched the note exactly. diff --git a/agent-hub/evidence/verifier/2026-09-28/fix-candidate-password-leak-seal.md b/agent-hub/evidence/verifier/2026-09-28/fix-candidate-password-leak-seal.md new file mode 100644 index 0000000..a08fbc1 --- /dev/null +++ b/agent-hub/evidence/verifier/2026-09-28/fix-candidate-password-leak-seal.md @@ -0,0 +1,89 @@ +# 2026-09-28 — fix-candidate-password-leak (verifier verdict) + +- Worker: verifier (subagent, dispatched via Agent tool) +- Node: `fix-candidate-password-leak` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- Evidence reviewed: `evidence/implementer/2026-09-28/fix-candidate-password-leak-plan.md` + (the 2026-09-28 note supersedes the stale `2026-08-21/fix-candidate-password-leak-diff.md` + note per its own text — that older note was NOT used as verdict input, + only its existence was cross-checked as context) +- New PM status: SEALED (was PENDING) + +## Isolation proof + +Dispatched as an independent Agent-tool spawn with task description +"Independent verifier pass for fix-candidate-password-leak" — fresh +context, no memory of the implementer session that wrote the 2026-09-28 +note. Self-grading is moot here by construction. + +## Reasoning + +Walking GitHub issue #152's acceptance criteria one at a time +(`gh issue view 152`): + +1. **`GET /api/v1/candidate/:email` response has no `password` field.** + Confirmed by independently reading current + `src/candidate/candidate.service.ts` (lines 37-53): + `handlerGetInformationByEmail` calls + `MODEL.findOne(safeEmailQuery).select('-password').exec()` — matches + the note's cited snippet verbatim. + +2. **`PUT`/`PATCH /candidate/update` response has no `password` field.** + Confirmed in the same file: `handlerGetInformationById` runs + `MODEL.findById(id).select(select || '-password')` — defaults to + excluding password when no explicit select is given, and no longer + double-wraps an explicit select string in `whitelistSelect([select])` + (the prior no-op bug). Matches the note's cited snippet verbatim. + +3. **Existing tests still pass; a regression test asserts password is + absent from the response.** Confirmed by reading + `src/__tests__/candidate/candidate.service.test.ts` directly — the + file exists and contains exactly the 3 tests the note claims: + `handlerGetInformationByEmail` asserts `select` was called with + `'-password'`; `handlerGetInformationById` asserts the same default; + a third test asserts an explicit whitelisted select string + (`'firstName lastName phone'`) passes through unchanged, which is a + direct regression test for the root-cause double-wrap bug. `npm test` + output in the note reads `Test Suites: 27 passed, 27 total / Tests: + 143 passed, 143 total` — a standard, non-truncated Jest summary; 143 = + 140 (the last SEALED node, `fix-create-response-null-id`, per + `worker-runs.log`) + 3 new tests here, internally consistent. + +**Test command**: `npm test`, matches `doctrine/MEMORY.md` exactly. + +**Bookkeeping-gap claim independently confirmed**: the production fix +(commit `f355e2f`, 2026-08-21) is genuinely already live and unchanged — +the code read today matches both the note's citation and the stale +2026-08-21 note's original diff. The real gap this note closes is the +missing regression test, which now exists. + +**Proportion (SmallestDiff)**: 1 new test file, 0 production `src/` +changes — proportionate; the production fix already exists and is +stable, so a regression-test-only diff is the smallest diff that closes +the actual remaining gap. + +**Seal gate**: `git status --short` on branch `152-password-hash-leaked` +shows only 2 untracked paths — the implementer's evidence note and the +1 new test file. Nothing staged, committed, or pushed. Matches the +note's "no outward-facing action taken" claim. + +## Forbidden states scan + +- `ADHOC_WORK` — no: node `fix-candidate-password-leak` pre-existed on + the diagram (row read before edit, was PENDING). +- `NO_EVIDENCE` — no: implementer note exists at the cited path, dated + and complete. +- `EDIT_UNVERIFIED` — no: `npm test` output is real, verbatim, not + truncated, and consistent with the described diff (143 = 140 + 3). +- `CODE_IN_HAVEN` — no: the only new file is + `src/__tests__/candidate/candidate.service.test.ts`, outside `haven/`; + no runnable code touched `agent-hub/`. +- `DIAGRAM_DRIFT` — resolved by this verdict: node row updated PENDING → + SEALED in place, no reorder. + +## Re-run + +`none` — audit-only, per recipe default for a non-outward-facing, non- +release-gate bug/test-coverage fix. The note's command matched doctrine, +output was verbatim and covered every acceptance criterion, and +independent reads of the cited source file and new test file confirmed +the claims. No partial or full re-run was warranted. diff --git a/agent-hub/evidence/verifier/2026-09-28/fix-chrome-executable-path-seal.md b/agent-hub/evidence/verifier/2026-09-28/fix-chrome-executable-path-seal.md new file mode 100644 index 0000000..cbada50 --- /dev/null +++ b/agent-hub/evidence/verifier/2026-09-28/fix-chrome-executable-path-seal.md @@ -0,0 +1,102 @@ +# 2026-09-28 — fix-chrome-executable-path (verifier verdict) + +- Worker: verifier (subagent, dispatched via Agent tool) +- Node: `fix-chrome-executable-path` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- New PM status: PENDING → **SEALED** + +## Isolation proof + +Dispatched as an independent Agent-tool subagent whose task description +reads "Independent verifier pass for fix-chrome-executable-path" — a +fresh context with no memory of the implementer session that produced +`evidence/implementer/2026-09-28/fix-chrome-executable-path-plan.md`. +Nothing in this session's history references having written that diff. + +## Reasoning + +Read only the evidence note first (EvidenceOnly), then independently +confirmed the note's specific factual citations against the real source +tree — verifying citations, not re-deriving the diff. + +**Trace to exactly one diagram node** — `fix-chrome-executable-path`, +confirmed at `haven/diagrams/dev-loop.prime-mermaid.md` line 58, PENDING +before this pass (task correctly resolves to GitHub issue #154, filed +directly against this node per the issue's own footer). + +**Test command matches convention** — `npm test` matches +`doctrine/MEMORY.md`'s documented `npm test` = `jest --passWithNoTests`. + +**Output not truncated** — the note's `npm test` tail shows complete +summary lines (`Test Suites: 26 passed, 26 total` / `Tests: 142 passed, +142 total`), no ellipsis or redaction markers. + +Issue #154 acceptance criteria, read via `gh issue view 154`, walked one +at a time: + +1. **PDF export works with no hardcoded path assumption** — independently + read `src/services/createPDF.ts` (full `executablePath` block, lines + 26-31). Confirmed: no hardcoded path literal anywhere in the file; + `const executablePath = process.env.PUPPETEER_EXECUTABLE_PATH;` spread + conditionally into `puppeteer.launch()`'s options. Matches the note's + quoted excerpt verbatim. +2. **Works both with bundled Chromium and an externally-installed one via + `PUPPETEER_EXECUTABLE_PATH`** — independently ran + `git diff src/__tests__/services/createPDF.test.ts` and read the full + diff. Confirmed: new `createCV executablePath resolution (issue #154)` + describe block, `jest.mock('puppeteer', () => ({ launch: jest.fn() }))` + (no real browser spawned), two tests — one asserts + `puppeteer.launch` called with no `executablePath` key when the env + var is unset, one asserts it's called with the exact override value + when set. Both invoke the real `createCV` end-to-end against a fake + browser/page. Matches the note's description exactly, no + overstatement. Also independently confirmed the note's claim that this + fix was already Docker-live-verified: `add-docker-support`'s own + SEALED row (line 81 of the diagram) states Chromium is installed via + `apt` with `PUPPETEER_EXECUTABLE_PATH` set and explicitly calls out + this as "exactly the CI/Docker case the existing + `fix-chrome-executable-path` trap... warns about; live-verified + working" — corroborates independent of the implementer's own note. +3. **`createPDF.test.ts` still passes; executable-path coverage added** — + confirmed via the same `git diff` read above (coverage added) and the + note's `npm test` output (142/142 passed, up from a 140 baseline plus + the 2 new tests, no new suite since it's an existing file). + +**Hub bytes accounting** — recomputed the 5-category byte sum (root + +doctrine excl. archive + active diagram excl. archive + +`haven/workers/implementer/` + `haven/workers/verifier/`) before touching +anything: got exactly 88066, matching the note's declared +`hub_bytes_before`. Confirms the note used the real formula, not a guess. + +## Forbidden states scan + +- `ADHOC_WORK` — no; node exists on the diagram, worker identity declared. +- `NO_EVIDENCE` — no; evidence note present at the cited path. +- `EDIT_UNVERIFIED` — no; every claim in the note checked out against the + real diff/source above. +- `CODE_IN_HAVEN` — no; no runnable code in `haven/`, only this markdown + diagram edit. +- `DIAGRAM_DRIFT` — no; that's the condition this SEAL resolves (row was + stale PENDING, now updated to match the live code + new test coverage). + +## Seal gate + +Confirmed via `git status --short` on branch `154-puppeteer-chrome-executable`: +only a locally modified `src/__tests__/services/createPDF.test.ts` and an +untracked evidence note — nothing committed, nothing pushed. Matches the +note's own "no outward-facing action taken" claim. + +## Proportion (SmallestDiff) + +1 existing test file extended, 0 production changes. Proportionate: the +underlying fix has been live and Docker-verified for 3+ weeks (since +`add-docker-support`/#24); the only real gap was missing regression +coverage, which this diff closes directly. + +## Re-run + +`none` — audit-only. Not an outward-facing action, not a release gate; +independently re-read the actual diff/source files cited rather than +re-running the suite. All figures (test counts, hub bytes) were +cross-checked against independently-verifiable facts (the diff itself, +the diagram's own `add-docker-support` row, the byte-count formula) rather +than taken purely on the note's word. diff --git a/agent-hub/evidence/verifier/2026-09-28/fix-create-response-null-id-seal.md b/agent-hub/evidence/verifier/2026-09-28/fix-create-response-null-id-seal.md new file mode 100644 index 0000000..a2418dd --- /dev/null +++ b/agent-hub/evidence/verifier/2026-09-28/fix-create-response-null-id-seal.md @@ -0,0 +1,85 @@ +# 2026-09-28 — fix-create-response-null-id — verifier verdict + +- Worker: verifier (subagent, dispatched via Agent tool) +- Node: `fix-create-response-null-id` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- New PM status: SEALED + +## Isolation proof + +Dispatched as a fresh Agent-tool subagent with task description +"Independent verifier pass for fix-create-response-null-id" — no +conversation history from the implementer's session, no memory of writing +the diff under review. Only source read directly (per recipe step 2's +citation-confirmation exception): `src/services/index.ts` (current + +`git show f355e2f`), the two new test files, `git status`/`git log` on +branch `157-post-create-responses`. Never opened the implementer's +session/diff itself — only its evidence note, plus these independent +citation checks. + +## Reasoning + +- **Command matches doctrine**: `npm test` — matches + `doctrine/MEMORY.md`'s Test row exactly. +- **Output not truncated**: note's `npm test` tail + (`Test Suites: 26 passed, 26 total` / `Tests: 140 passed, 140 total`) + and `npm run build` output are verbatim, no `...`/truncation markers. +- **Criterion (a)** — regression test asserts real persisted `_id`, not + null: confirmed by reading + `src/__tests__/services/baseCreateDocument.test.ts` directly — 3 tests + cover hookAfterSave replacement propagation, undefined fallback, and + no-hook passthrough; the first asserts + `expect((result.data as any)._id).toBe('real-id-123')`. +- **Criterion (b)** — spot-check a real CV section create endpoint + end-to-end: note explicitly discloses the literal live-HTTP + interpretation was NOT performed (no local Mongo/Redis/Docker in that + sandbox — a real, stated constraint, not hidden) and substitutes + `src/__tests__/candidate_profile/BaseService.test.ts`, confirmed by + direct read: calls the real, unmocked `createCrudService(...). + handlerCreate` → real `BaseService.ts` → real `services/index.ts` + `baseCreateDocument` → real `hookAfterSave` refetch path, only the + Mongoose model itself faked. This exercises the exact production code + path every `POST .../create` endpoint uses, asserts + `data._id).toBe('real-id-1')` (not null). Judged: an honest, reasonable + substitution given the disclosed sandbox limit and the fact the + underlying fix has been live and unchanged on `staging` for 5+ weeks + (see next point) — not a REOPEN-worthy gap. +- **Bookkeeping-gap claim independently confirmed**: `git show f355e2f -- + src/services/index.ts` really contains the cited hunk + (`const replacement = await props.hookAfterSave(...); if (replacement + !== undefined) _data = replacement;`), and current + `src/services/index.ts` lines 300-302 match it verbatim — the + production fix is genuinely already live, unchanged since 2026-08-21. + `BaseService.ts` confirmed unchanged (diff is test-only). +- **Proportion**: 2 new test files, 0 production `src/` changes — + proportionate; the production fix already exists, so a regression-test + diff is the smallest diff that closes the real gap (no prior test + coverage). +- **Seal gate**: `git status --short` on branch `157-post-create-responses` + shows only 3 untracked paths — the evidence note dir and the 2 new test + files — nothing staged/committed/pushed. `git log` on that branch shows + no new commit beyond the shared history with `staging`. Matches the + note's "no outward-facing action taken" claim. + +## Forbidden states scan + +- `ADHOC_WORK` — no: task traces to the pre-existing PENDING node + `fix-create-response-null-id` on the diagram (row confirmed before + edit). +- `NO_EVIDENCE` — no: implementer note exists at the cited path. +- `EDIT_UNVERIFIED` — no: `npm test` output is a real, non-truncated, + doctrine-matching command result, consistent with the described diff + (140 = prior 136 + 3 new `baseCreateDocument.test.ts` tests + 1 new + `BaseService.test.ts` test). +- `CODE_IN_HAVEN` — no: `find agent-hub/evidence/implementer/2026-09-28` + shows only the one `.md` note, no runnable code. +- `DIAGRAM_DRIFT` — resolved by this verdict: node row updated PENDING → + SEALED in place. + +## Re-run + +`none` — audit-only, per recipe default for a non-outward-facing, non- +release-gate bug/test-coverage fix. The note's command matched doctrine, +output was verbatim and covered every acceptance criterion, so no partial +or full re-run was warranted; independent checks were limited to +confirming the note's own citations (git history, current source, test +file contents) rather than regenerating its evidence. diff --git a/agent-hub/evidence/verifier/2026-09-28/fix-refresh-token-expiry-unused-seal.md b/agent-hub/evidence/verifier/2026-09-28/fix-refresh-token-expiry-unused-seal.md new file mode 100644 index 0000000..940d779 --- /dev/null +++ b/agent-hub/evidence/verifier/2026-09-28/fix-refresh-token-expiry-unused-seal.md @@ -0,0 +1,65 @@ +# 2026-09-28 — fix-refresh-token-expiry-unused (verifier verdict) + +- Worker: verifier (subagent, dispatched via Agent tool) +- Node: `fix-refresh-token-expiry-unused` (`haven/diagrams/dev-loop.prime-mermaid.md`) +- New PM status: SEALED (was PENDING) + +## Isolation proof + +Dispatched as an independent Agent-tool subagent whose own task description +reads "Independent verifier pass for fix-refresh-token-expiry-unused" — no +memory of the implementer session that wrote +`evidence/implementer/2026-09-28/fix-refresh-token-expiry-unused-plan.md`. +Per SOUL.md invariant (1), self-grading is moot here: this is a fresh +context with no prior state. + +## Reasoning + +Read only the implementer's plan note per EvidenceOnly, then independently +confirmed the specific factual claims it makes (not re-derived the diff): + +- **Cited call sites match verbatim**: read `src/auth/auth.service.ts:115-134` + and `src/auth/auth.controller.ts:145-159` directly — both contain exactly + the lines quoted in the note (`jwtSign({ _id }, TOKEN_SECRET, { expiresIn: + TOKEN_EXP_IN || '1h' })` for access, `jwtSign({ _id }, TOKEN_REFRESH, + { expiresIn: TOKEN_REFRESH_EXP_IN })` for refresh), confirming `TOKEN_EXP_IN` + is genuinely already wired at both sites — the PENDING description's cited + `api/v1/auth/services/login.ts` no longer exists, consistent with the + bookkeeping-gap explanation. +- **Test file read in full and independently re-run**: + `src/__tests__/auth/tokenExpiry.test.ts` uses real unmocked + `@/utils/jwt` and `@/config/process.config`, calls `jest.resetModules()` + before each `require()` so env var changes (`TOKEN_EXP_IN`, + `TOKEN_REFRESH_EXP_IN`) actually take effect before the config module is + re-read — no stale-cache bug. Ran it myself: + `npx jest src/__tests__/auth/tokenExpiry.test.ts` → `Tests: 3 passed, 3 + total`, matching the note's claims exactly (`exp-iat===7200` for `2h`, + `exp-iat===1209600` for `14d`, plus the two regression checks). +- **Issue #155 acceptance criteria, one at a time** (`gh issue view 155`): + (a) access tokens expire per `TOKEN_EXP_IN`, verified by decoding + `exp`/`iat` — covered, test 1. (b) refresh tokens continue to expire per + `TOKEN_REFRESH_EXP_IN` — covered, test 1 + test 3 (7d default + regression). (c) existing auth/refresh tests still pass — note's `npm + test` output: `Test Suites: 27 passed, 27 total / Tests: 143 passed, 143 + total`, not truncated, includes `auth.service.test.ts`, + `auth.controller.test.ts`, `refreshToken.test.ts`. +- Test command (`npm test`) matches `doctrine/MEMORY.md`'s documented + command from repo root. +- Proportion: 1 new test file, 0 production `src/` changes — proportionate + given the fix has been live since `f355e2f` (2026-08-21); the real gap + closed is regression coverage, exactly as the note states. +- 5 forbidden states scanned: no ADHOC_WORK (worker identity + node + matched), no NO_EVIDENCE (note exists), no EDIT_UNVERIFIED (claims + independently re-run and matched), no CODE_IN_HAVEN (only a test file + under `src/__tests__/`, nothing in `haven/`), DIAGRAM_DRIFT resolved by + this SEAL (row was PENDING, now updated). +- Seal gate: `git status --short` on `155-token-exp-in` shows only 2 + untracked files (the plan note + the test file) — no commit, no push, + matching the note's "no outward-facing action taken" claim. + +## Re-run + +`partial` — re-ran only the new test file +(`npx jest src/__tests__/auth/tokenExpiry.test.ts`, 3/3 passed) to confirm +the note's specific claims; did not re-run the full `npm test` suite +(accepted the note's verbatim 143/143 output for that). 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 f21d40b..75cbaea 100644 --- a/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md +++ b/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md @@ -55,14 +55,16 @@ flowchart TD | `add-pagination-filtering-cv-sections` | SEALED | GitHub issue #73. `page`/`limit`/`sort` query params on the 6 CV-section list endpoints (education/experience/award/certificate/project/reference — `generalInformation` excluded, its `GET /` returns a single document, not a list). Code already implemented and merged to `staging` via PR #106 (commit `1133f1b`, 2026-09-02) with no matching diagram node/evidence note at the time (bookkeeping gap, backfilled now by `/todo "#73"`, same pattern as `add-logout-all-sessions`/#74). Opt-in and backward compatible: omitting `limit` returns the exact old unpaginated array (`services/index.ts` `baseFindDocument`); a valid `limit` (capped at 100, `MAX_PAGE_LIMIT`) switches `data` to `{ items, pagination }`. `sort` validated against `SORT_FIELD_REGEX` allowlist in `BaseController.ts` — no `$`, can't smuggle a Mongo operator. Issue stays OPEN on GitHub because the merge landed on `staging`, not the default branch (`main`) — same expected auto-close gap as #74, not a bug. Verified 2026-09-05, evidence: `evidence/verifier/2026-09-05/add-pagination-filtering-cv-sections-seal.md`. | | `add-logout-all-sessions` | SEALED | GitHub issue #74. `POST /api/v1/auth/logout-all` — code already implemented and merged to `staging` via PR #105 (commit `03bcb66`, 2026-09-02) with no matching diagram node/evidence note at the time (bookkeeping gap, backfilled now by `/todo "#74"`). Design deviates from the issue's `tokenVersion`-on-`Candidate` proposal: reuses the Redis/mem "invalidated-before" timestamp shape from `tokenBlacklist.ts` (`src/utils/sessionRevocation.ts`), compared against the JWT's standard `iat` in `verifyToken.middleware.ts` + `authRefreshToken` — no schema change, no extra Mongo lookup. Issue stays OPEN on GitHub because the merge landed on `staging`, not the default branch (`main`); auto-close via `Closes #74` fires only on a `main` merge per the documented release workflow — expected, not a bug. Verified 2026-09-05, evidence: `evidence/verifier/2026-09-05/add-logout-all-sessions-seal.md`. | | `agent-hub-token-cleanup-20260830` | SEALED | 2026-08-30 — archived, see `haven/diagrams/dev-loop-archive.md`. Evidence: `evidence/implementer/2026-08-30/agent-hub-token-cleanup-diff.md`. | -| `fix-chrome-executable-path` | PENDING | `src/services/createPDF.ts:14-25` — Chrome executable path hardcoded, breaks PDF export in CI/Docker. See Traps in `doctrine/domains/PROJECT.md`. First candidate node. | +| `fix-chrome-executable-path` | SEALED | GitHub issue #154. Bookkeeping-gap backfill, same pattern as `fix-create-response-null-id`/`fix-idor-broken-access-control`: reading `src/services/createPDF.ts` today shows **no hardcoded Chrome executable path** anywhere — `process.env.PUPPETEER_EXECUTABLE_PATH` conditionally spread into the launch options, falling back to Puppeteer's own bundled-Chromium resolution when unset. This exact shape was introduced by `add-docker-support`/#24 (SEALED 2026-09-06) and has been Docker-live-verified since — that node's own row explicitly calls out this trap as already fixed. The doctrine `Traps` table entry describing the old hardcoded path is now stale (out of scope here, flagged for a future docs-only pass). The real remaining gap this node closed: `services/createPDF.test.ts` had zero coverage of the executable-path resolution logic itself. 0 production changes; `src/__tests__/services/createPDF.test.ts` extended with a new `createCV executablePath resolution (issue #154)` describe block — 2 tests (`PUPPETEER_EXECUTABLE_PATH` unset → bundled Chromium, no `executablePath` key; set → override honored), `jest.mock('puppeteer', ...)` so no real browser spawns. `npm test`: 142/142 passed. 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`; independently confirmed the no-hardcoded-path claim by reading `createPDF.ts`, confirmed the 2 new tests via `git diff` on the test file, confirmed `PUPPETEER_EXECUTABLE_PATH`/Docker-live-verification claim via `add-docker-support`'s own sealed row; confirmed via `git status --short` on `154-puppeteer-chrome-executable` that nothing was committed/pushed). Evidence: `evidence/implementer/2026-09-28/fix-chrome-executable-path-plan.md`, `evidence/verifier/2026-09-28/fix-chrome-executable-path-seal.md`. | | `fix-idor-broken-access-control` | SEALED | **Critical.** All CRUD APIs for candidate_profile (education/experience/award/certificate/project/reference/generalInformation) + `candidate.service.ts` + `fnExportPDF` never cross-check `candidateId`/`_id` against `req.user._id` (JWT) — they trust client-supplied `req.body.candidateId`/`_id`. Live-tested confirmed: User B could read/delete/edit User A's data, overwrite A's profile. Root cause: `verifyToken.middleware.ts` sets `req.user` but nothing cross-checks it. Found while testing the full API (task: "test the whole API again"). Code already fixed on `staging` (commit `f355e2f`, folded in via `1ec67de`'s ancestry) — bookkeeping-gap backfill, same pattern as #73/#74: `verifyToken.middleware.ts` forces `req.body.candidateId` to the authenticated `_id`; `baseUpdateDocument`/`baseDeleteDocument` check the existing document's real owner, not the payload; `candidate.controller.ts` forces `value._id`; `candidate_me/index.ts` `fnExportPDF` uses `req.user._id` directly. SEALED 2026-09-08 after independent re-read of every cited file (router, middleware, services/index.ts, BaseController/BaseService, candidate.controller.ts, candidate_me/index.ts, generalInformation.controller.ts) — evidence: `evidence/verifier/2026-09-08/fix-idor-broken-access-control-seal.md`. | | `fix-candidate-password-leak` | PENDING | `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. | +| `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-create-response-null-id` | PENDING | 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. | +| `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` | PENDING | **Critical, found by accident while testing #79.** `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`. Fix: `.toString()` on `_id` before passing it in. | +| `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`. | | `feat-i18n-api-messages-auth` | PENDING | Feature, GitHub issue #78 (phase 1 of several). i18n infrastructure (hand-rolled `t(key, lang)`, reads `locales/vi.json`/`en.json`, middleware detects `Accept-Language`, defaults `vi`) + fully migrates the auth flow (register/login/logout/refresh). Does NOT migrate Joi validation messages (different architecture — Joi schemas are built once at module load with no request context; needs error TYPE → i18n key mapping, left as a follow-up). Does NOT touch candidate/CV section messages (separate follow-up). | | `feat-i18n-full-coverage` | PENDING | Feature, GitHub issue #78 (phase 2/2 — complete). Joi validation messages: a generic system translating by `detail.type` + `fieldLabels` (`utils/valid.ts`), no longer relying on hardcoded `.messages()` per schema. Mongoose `required` messages: same approach in `handleError` (`utils/helper.ts`). Every candidate/CV section success/error message (`services/index.ts`, `BaseController.ts`, `BaseService.ts`, `candidate.service.ts`, `generalInformation.*`) cascades across all 7 CV sections. Bug found during implementation: `t()`'s dot-path walker misparsed Joi type strings containing a dot (`any.required` was read as 3 nested levels) — caught via a real live test (curl in 2 languages), not code review. Fix: a dedicated `tErrorType()` function, flat lookup with no dot-path walking. | | `add-visit-tracking` | SEALED | 2026-09-01 — archived, see `haven/diagrams/dev-loop-archive.md`. Evidence: `evidence/implementer/2026-09-01/add-visit-tracking-diff.md`. | diff --git a/package-lock.json b/package-lock.json index a857a00..7b83bea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "resume-nodejs-api", - "version": "1.8.0", + "version": "1.8.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "resume-nodejs-api", - "version": "1.8.0", + "version": "1.8.1", "license": "ISC", "dependencies": { "@babel/runtime": "^7.22.10", diff --git a/package.json b/package.json index adcf222..9110bf0 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "resume-nodejs-api", "main": "src/server.ts", "private": true, - "version": "1.8.0", + "version": "1.8.1", "description": "Resume API backend with rate limiting and Redis support", "scripts": { "dev-node": "ts-node -r tsconfig-paths/register src/server.ts", diff --git a/src/__tests__/auth/tokenExpiry.test.ts b/src/__tests__/auth/tokenExpiry.test.ts new file mode 100644 index 0000000..5198394 --- /dev/null +++ b/src/__tests__/auth/tokenExpiry.test.ts @@ -0,0 +1,76 @@ +/** + * Regression coverage for issue #155 — TOKEN_EXP_IN was defined in + * config but never actually passed at any jwtSign() call site, so access + * and refresh tokens always shared the same default (1h) expiry. Both + * real call sites (auth.service.ts's handlerLogin, auth.controller.ts's + * authRefreshToken) already sign with the exact same pattern: + * jwtSign({ _id }, TOKEN_SECRET, { expiresIn: TOKEN_EXP_IN || '1h' }) // access + * jwtSign({ _id }, TOKEN_REFRESH, { expiresIn: TOKEN_REFRESH_EXP_IN }) // refresh + * This test exercises that exact pattern for real (no mocked jwtSign), + * decodes the resulting JWTs, and proves the access token's lifetime + * genuinely comes from TOKEN_EXP_IN and the refresh token's from + * TOKEN_REFRESH_EXP_IN — 2 distinct, config-driven lifetimes, not a + * shared hardcoded default. + */ + +describe('access vs refresh token expiry — real TOKEN_EXP_IN/TOKEN_REFRESH_EXP_IN wiring (issue #155)', () => { + const ORIGINAL_EXP_IN = process.env.TOKEN_EXP_IN; + const ORIGINAL_REFRESH_EXP_IN = process.env.TOKEN_REFRESH_EXP_IN; + + afterEach(() => { + if (ORIGINAL_EXP_IN === undefined) delete process.env.TOKEN_EXP_IN; + else process.env.TOKEN_EXP_IN = ORIGINAL_EXP_IN; + if (ORIGINAL_REFRESH_EXP_IN === undefined) delete process.env.TOKEN_REFRESH_EXP_IN; + else process.env.TOKEN_REFRESH_EXP_IN = ORIGINAL_REFRESH_EXP_IN; + jest.resetModules(); + }); + + it('signs the access token per TOKEN_EXP_IN and the refresh token per a distinct, longer TOKEN_REFRESH_EXP_IN', () => { + jest.resetModules(); + process.env.TOKEN_EXP_IN = '2h'; + process.env.TOKEN_REFRESH_EXP_IN = '14d'; + + // Re-require AFTER setting env vars, so config picks up the new values + // (process.config.ts reads process.env at module-load time). + const { jwtSign } = require('@/utils/jwt'); + const { TOKEN_SECRET, TOKEN_REFRESH, TOKEN_EXP_IN, TOKEN_REFRESH_EXP_IN } = require('@/config/process.config'); + + // Exact same call shape as auth.service.ts's handlerLogin and + // auth.controller.ts's authRefreshToken. + const accessToken = jwtSign({ _id: 'u1' }, TOKEN_SECRET, { expiresIn: TOKEN_EXP_IN || '1h' }); + const refreshToken = jwtSign({ _id: 'u1' }, TOKEN_REFRESH, { expiresIn: TOKEN_REFRESH_EXP_IN }); + + // jwtVerify's return type only declares `_id`, but the real decoded + // JWT payload also carries `exp`/`iat` at runtime — cast to read them. + const { jwtVerify } = require('@/utils/jwt'); + const accessDecoded = jwtVerify(accessToken, TOKEN_SECRET) as any; + const refreshDecoded = jwtVerify(refreshToken, TOKEN_REFRESH) as any; + + expect(accessDecoded.exp - accessDecoded.iat).toBe(2 * 60 * 60); // 2h in seconds + expect(refreshDecoded.exp - refreshDecoded.iat).toBe(14 * 24 * 60 * 60); // 14d in seconds + expect(accessDecoded.exp - accessDecoded.iat).not.toBe(refreshDecoded.exp - refreshDecoded.iat); + }); + + it('falls back to a 1h access token when TOKEN_EXP_IN is unset (regression check on the || \'1h\' default)', () => { + jest.resetModules(); + delete process.env.TOKEN_EXP_IN; + process.env.TOKEN_REFRESH_EXP_IN = '7d'; + + const { jwtSign, jwtVerify } = require('@/utils/jwt'); + const { TOKEN_SECRET, TOKEN_EXP_IN } = require('@/config/process.config'); + + const accessToken = jwtSign({ _id: 'u1' }, TOKEN_SECRET, { expiresIn: TOKEN_EXP_IN || '1h' }); + const accessDecoded = jwtVerify(accessToken, TOKEN_SECRET) as any; + + expect(accessDecoded.exp - accessDecoded.iat).toBe(60 * 60); // 1h fallback + }); + + it('TOKEN_REFRESH_EXP_IN itself defaults to 7d when unset (existing default, regression check)', () => { + jest.resetModules(); + delete process.env.TOKEN_REFRESH_EXP_IN; + + const { TOKEN_REFRESH_EXP_IN } = require('@/config/process.config'); + + expect(TOKEN_REFRESH_EXP_IN).toBe('7d'); + }); +}); 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); + }); +}); diff --git a/src/__tests__/candidate/candidate.service.test.ts b/src/__tests__/candidate/candidate.service.test.ts new file mode 100644 index 0000000..d59db9b --- /dev/null +++ b/src/__tests__/candidate/candidate.service.test.ts @@ -0,0 +1,65 @@ +/** + * Regression coverage for issue #152 — GET /api/v1/candidate/:email and + * PUT/PATCH /candidate/update used to leak the raw bcrypt password hash. + * Root cause was 2 independent bugs in src/candidate/candidate.service.ts: + * handlerGetInformationByEmail had no .select() at all, and + * handlerGetInformationById double-wrapped an already-whitelisted, + * space-joined select string in candidateQuerySafe.whitelistSelect([select]) + * (treating the whole string as one field name, which never matched the + * allow-list, silently making the select a no-op). Both were already fixed + * on `staging` (commit f355e2f, 2026-08-21) — this just closes the missing + * regression-test gap. + */ +import * as MODELS from '@/models'; +import { handlerGetInformationByEmail, handlerGetInformationById } from '@/candidate/candidate.service'; + +jest.mock('@/models', () => ({ + Candidate: { findOne: jest.fn(), findById: jest.fn() }, + generalInformation: {}, + Experience: {}, + Education: {}, + Reference: {}, + Project: {}, + Certificate: {}, + Award: {}, + Application: {}, + Profile: {}, + Visit: {}, +})); + +describe('candidate.service.ts password exclusion (issue #152)', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('handlerGetInformationByEmail', () => { + it('always excludes the password field via .select(\'-password\')', async () => { + const select = jest.fn().mockReturnValue({ exec: jest.fn().mockResolvedValue({ _id: '1', email: 'a@b.com' }) }); + (MODELS.Candidate.findOne as jest.Mock).mockReturnValue({ select }); + + await handlerGetInformationByEmail('a@b.com'); + + expect(select).toHaveBeenCalledWith('-password'); + }); + }); + + describe('handlerGetInformationById', () => { + it('defaults to excluding password when no explicit select is given', async () => { + const select = jest.fn().mockReturnValue({ exec: jest.fn().mockResolvedValue({ _id: '1' }) }); + (MODELS.Candidate.findById as jest.Mock).mockReturnValue({ select }); + + await handlerGetInformationById('1'); + + expect(select).toHaveBeenCalledWith('-password'); + }); + + it('uses the given whitelisted select string as-is, without re-wrapping it (no more double-wrap no-op)', async () => { + const select = jest.fn().mockReturnValue({ exec: jest.fn().mockResolvedValue({ _id: '1' }) }); + (MODELS.Candidate.findById as jest.Mock).mockReturnValue({ select }); + + await handlerGetInformationById('1', { select: 'firstName lastName phone' }); + + expect(select).toHaveBeenCalledWith('firstName lastName phone'); + }); + }); +}); diff --git a/src/__tests__/candidate_me/index.test.ts b/src/__tests__/candidate_me/index.test.ts index 4c73127..abad126 100644 --- a/src/__tests__/candidate_me/index.test.ts +++ b/src/__tests__/candidate_me/index.test.ts @@ -4,6 +4,12 @@ * QuerySafe.safeQuery silently DROPS a rejected value (e.g. containing "$") * instead of throwing. Before the fix, that made the resulting Mongo filter * collapse to {} and match an arbitrary candidate instead of failing. + * + * Also covers issue #153 (see bottom describe block) — a sibling instance + * of the same "value silently dropped by QuerySafe" bug class, but at the + * candidateId field instead of the identifier lookup: `_id` from a raw + * Mongoose document is an ObjectId instance, not a string, and + * QuerySafe.safeQuery only accepts string values. */ import * as MODEL from '@/models'; @@ -113,3 +119,45 @@ describe('candidate_me/index.ts (issue #135)', () => { }); }); }); + +describe('handlerGetAboutMe — candidateId filter with an ObjectId _id (issue #153)', () => { + // Mirrors a real Mongoose document: _id is an ObjectId instance (has a + // .toString() method), never a plain string. Before the fix, + // idQuerySafe.safeQuery({}, { candidateId: _id }) silently dropped the + // whole candidateId key (QuerySafe.safeQuery only accepts + // typeof value === 'string'), collapsing every CV-section query's filter + // to {} — every candidate's data came back mixed together. + const objectIdLike = { + toString: () => '507f1f77bcf86cd799439011', + }; + const candidateDoc = { _id: objectIdLike, email: 'votan.it@gmail.com' }; + + beforeEach(() => { + jest.clearAllMocks(); + (MODEL.Candidate.findOne as jest.Mock).mockReturnValue({ exec: jest.fn().mockResolvedValue(candidateDoc) }); + const emptyFind = { exec: jest.fn().mockResolvedValue([]) }; + (MODEL.generalInformation.find as jest.Mock).mockReturnValue(emptyFind); + (MODEL.Experience.find as jest.Mock).mockReturnValue(emptyFind); + (MODEL.Education.find as jest.Mock).mockReturnValue(emptyFind); + (MODEL.Reference.find as jest.Mock).mockReturnValue(emptyFind); + (MODEL.Project.find as jest.Mock).mockReturnValue(emptyFind); + (MODEL.Certificate.find as jest.Mock).mockReturnValue(emptyFind); + (MODEL.Award.find as jest.Mock).mockReturnValue(emptyFind); + }); + + it('stringifies an ObjectId _id before filtering, instead of dropping candidateId entirely', async () => { + await handlerGetAboutMe('votan.it@gmail.com', 'vi'); + + expect(MODEL.Education.find).toHaveBeenCalledWith( + expect.objectContaining({ candidateId: '507f1f77bcf86cd799439011' }), + expect.anything(), + ); + expect(MODEL.Experience.find).toHaveBeenCalledWith( + expect.objectContaining({ candidateId: '507f1f77bcf86cd799439011' }), + expect.anything(), + ); + // Never the raw object itself, and never silently dropped ({} filter). + expect(MODEL.Education.find).not.toHaveBeenCalledWith(expect.objectContaining({ candidateId: objectIdLike }), expect.anything()); + expect(MODEL.Education.find).not.toHaveBeenCalledWith({}, expect.anything()); + }); +}); diff --git a/src/__tests__/candidate_profile/BaseService.test.ts b/src/__tests__/candidate_profile/BaseService.test.ts new file mode 100644 index 0000000..a078d7c --- /dev/null +++ b/src/__tests__/candidate_profile/BaseService.test.ts @@ -0,0 +1,42 @@ +/** + * Spot-check for issue #157 — exercises createCrudService's real + * handlerCreate (BaseService.ts) end-to-end through the real, unmocked + * services/index.ts (baseCreateDocument + its hookAfterSave refetch), the + * same code path every CV section's real POST .../create endpoint uses + * (e.g. education). Only the Mongoose model itself is faked. Confirms the + * response's data._id is the real persisted id, not null. + */ +import { createCrudService } from '@/candidate_profile/BaseService'; + +function createFakeEducationModel() { + const docs: Record[] = []; + return { + validate: jest.fn().mockResolvedValue(undefined), + create: jest.fn(async (doc: Record): Promise> => { + // Mirrors real Mongoose behavior for MODEL.create({ _id: null, ... }): + // an explicit `_id: null` is NOT kept as null — Mongoose still + // assigns a real ObjectId-shaped id. + const saved: Record = { ...doc, _id: `real-id-${docs.length + 1}` }; + docs.push(saved); + return saved; + }), + find: jest.fn((query: Record) => ({ + exec: jest.fn().mockResolvedValue(docs.filter((d) => d.candidateId === query.candidateId)), + })), + }; +} + +describe('createCrudService handlerCreate — real CV section create flow (issue #157)', () => { + it('returns the real persisted _id in data, not null, for a section like education', async () => { + const model = createFakeEducationModel(); + const { handlerCreate } = createCrudService({ model, name: 'education' }); + + const result: any = await handlerCreate({ candidateId: 'c1', school: 'MIT' }); + + expect(result.success).toBe(true); + const data = Array.isArray(result.data) ? result.data[0] : result.data; + expect(data._id).toBeDefined(); + expect(data._id).not.toBeNull(); + expect(data._id).toBe('real-id-1'); + }); +}); diff --git a/src/__tests__/services/baseCreateDocument.test.ts b/src/__tests__/services/baseCreateDocument.test.ts new file mode 100644 index 0000000..a714395 --- /dev/null +++ b/src/__tests__/services/baseCreateDocument.test.ts @@ -0,0 +1,64 @@ +/** + * Tests for services/index.ts's baseCreateDocument — regression coverage for + * issue #157 (data._id: null on every POST .../create response). The bug: + * hookAfterSave used to receive `data` by destructured value, so reassigning + * it inside the hook never reached baseCreateDocument's own `_data` variable + * — the caller always got MODEL.create()'s raw result (`_id: null`, since + * `{ _id: null, ...document }` is passed to create()) instead of the + * refetched document with its real id. Fixed by using hookAfterSave's return + * value (`replacement`) as the new `_data` when it isn't undefined. Uses a + * fake Mongoose-shaped model, same style as baseFindDocument.test.ts. + */ +import { baseCreateDocument } from '@/services'; + +function createFakeModel(createdDoc: Record) { + return { + validate: jest.fn().mockResolvedValue(undefined), + create: jest.fn().mockResolvedValue(createdDoc), + }; +} + +describe('baseCreateDocument hookAfterSave propagation (issue #157)', () => { + it("returns hookAfterSave's replacement as data, not the raw create() result with a null _id", async () => { + const model = createFakeModel({ _id: null, candidateId: 'c1' }); + const realDoc = { _id: 'real-id-123', candidateId: 'c1' }; + + const result = await baseCreateDocument({ + document: { candidateId: 'c1' }, + model, + name: 'education', + hookAfterSave: async () => realDoc, + }); + + expect(result.success).toBe(true); + expect(result.data).toBe(realDoc); + expect((result.data as any)._id).toBe('real-id-123'); + }); + + it('falls back to the raw create() result when hookAfterSave returns undefined', async () => { + const createdDoc = { _id: 'created-id', candidateId: 'c1' }; + const model = createFakeModel(createdDoc); + + const result = await baseCreateDocument({ + document: { candidateId: 'c1' }, + model, + name: 'education', + hookAfterSave: async () => undefined, + }); + + expect(result.data).toBe(createdDoc); + }); + + it('returns the raw create() result unchanged when no hookAfterSave is given', async () => { + const createdDoc = { _id: 'created-id-2', candidateId: 'c1' }; + const model = createFakeModel(createdDoc); + + const result = await baseCreateDocument({ + document: { candidateId: 'c1' }, + model, + name: 'education', + }); + + expect(result.data).toBe(createdDoc); + }); +}); diff --git a/src/__tests__/services/createPDF.test.ts b/src/__tests__/services/createPDF.test.ts index 7d16915..95be522 100644 --- a/src/__tests__/services/createPDF.test.ts +++ b/src/__tests__/services/createPDF.test.ts @@ -1,9 +1,13 @@ /** * Tests for services/createPDF.ts's pageRender — pure HTML-building * function, no Puppeteer involved, so it's safe/fast to test directly. + * + * Also covers issue #154 (see bottom describe block) — createCV's + * executable-path resolution for Puppeteer's Chrome/Chromium launch. */ -import { pageRender } from '@/services/createPDF'; +import { pageRender, createCV } from '@/services/createPDF'; +import puppeteer from 'puppeteer'; describe('pageRender', () => { it('renders career and careerGoal into the PDF content (issue #87)', () => { @@ -59,3 +63,51 @@ describe('pageRender', () => { expect(html).not.toContain('Mục tiêu nghề nghiệp'); }); }); + +// Puppeteer's own `launch()` is mocked out entirely — no real browser is +// spawned, keeping this fast/safe to run anywhere (no Chrome install +// required in the test environment). Only the executablePath resolution +// logic (the part issue #154 is about) is under test. +jest.mock('puppeteer', () => ({ launch: jest.fn() })); + +function createFakeBrowser() { + return { + newPage: jest.fn().mockResolvedValue({ + setContent: jest.fn().mockResolvedValue(undefined), + pdf: jest.fn().mockResolvedValue(Buffer.from('fake-pdf-bytes')), + }), + close: jest.fn().mockResolvedValue(undefined), + }; +} + +function createFakeRes() { + return { contentType: jest.fn(), send: jest.fn() }; +} + +describe('createCV executablePath resolution (issue #154)', () => { + const ORIGINAL_ENV = process.env.PUPPETEER_EXECUTABLE_PATH; + + afterEach(() => { + if (ORIGINAL_ENV === undefined) delete process.env.PUPPETEER_EXECUTABLE_PATH; + else process.env.PUPPETEER_EXECUTABLE_PATH = ORIGINAL_ENV; + jest.clearAllMocks(); + }); + + it('launches with no hardcoded executablePath when PUPPETEER_EXECUTABLE_PATH is unset (Puppeteer resolves its own bundled Chromium)', async () => { + delete process.env.PUPPETEER_EXECUTABLE_PATH; + (puppeteer.launch as jest.Mock).mockResolvedValue(createFakeBrowser()); + + await createCV({ email: 'a@b.com' }, createFakeRes() as any); + + expect(puppeteer.launch).toHaveBeenCalledWith(expect.not.objectContaining({ executablePath: expect.anything() })); + }); + + it('launches with the given executablePath when PUPPETEER_EXECUTABLE_PATH is set (CI/Docker override)', async () => { + process.env.PUPPETEER_EXECUTABLE_PATH = '/usr/bin/chromium'; + (puppeteer.launch as jest.Mock).mockResolvedValue(createFakeBrowser()); + + await createCV({ email: 'a@b.com' }, createFakeRes() as any); + + expect(puppeteer.launch).toHaveBeenCalledWith(expect.objectContaining({ executablePath: '/usr/bin/chromium' })); + }); +});