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/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/haven/diagrams/dev-loop.prime-mermaid.md b/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md index 3d04a76..6ef154d 100644 --- a/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md +++ b/agent-hub/haven/diagrams/dev-loop.prime-mermaid.md @@ -55,7 +55,7 @@ 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` | 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. | 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' })); + }); +});