Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d5fe00f
Merge pull request #151 from datvt243/release/v1.8.0
datvt243 Sep 26, 2026
e599a16
test(services): add regression coverage for POST .../create returning…
datvt243 Sep 27, 2026
c28d31c
Merge pull request #166 from datvt243/157-post-create-responses
datvt243 Sep 27, 2026
fd5a958
test(candidate): add regression coverage proving password hash is nev…
datvt243 Sep 27, 2026
a2ec824
Merge pull request #167 from datvt243/152-password-hash-leaked
datvt243 Sep 27, 2026
0dc9b77
test(candidate_me): add regression coverage for ObjectId candidateId …
datvt243 Sep 27, 2026
04ccc28
Merge pull request #168 from datvt243/153-critical-candidate-me
datvt243 Sep 27, 2026
247983c
test(services): add coverage for Puppeteer executablePath resolution
datvt243 Sep 27, 2026
2faa537
Merge pull request #169 from datvt243/154-puppeteer-chrome-executable
datvt243 Sep 27, 2026
6cfba0a
test(auth): add regression coverage proving access/refresh token expi…
datvt243 Sep 27, 2026
a318e76
Merge remote-tracking branch 'origin/staging' into 155-token-exp-in
datvt243 Sep 27, 2026
6faec85
Merge pull request #170 from datvt243/155-token-exp-in
datvt243 Sep 27, 2026
5437632
test(auth): add regression coverage proving v2 register reaches the f…
datvt243 Sep 27, 2026
66a9299
Merge remote-tracking branch 'origin/staging' into 156-post-apiv2-aut…
datvt243 Sep 27, 2026
4b84729
Merge pull request #171 from datvt243/156-post-apiv2-authregister
datvt243 Sep 27, 2026
0f87eb4
chore(release): bump version to v1.8.1
datvt243 Sep 27, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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`
Original file line number Diff line number Diff line change
@@ -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`
Original file line number Diff line number Diff line change
@@ -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<string, any>, 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`
Loading
Loading