Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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,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`
Original file line number Diff line number Diff line change
@@ -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).
2 changes: 2 additions & 0 deletions agent-hub/haven/diagrams/dev-loop.prime-mermaid.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ flowchart TD
| `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` | 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. |
Expand Down
Loading
Loading