Session 5 of voyage-rebrand (V6). Operator-authorized cross-plugin scope. - git mv plugins/ultraplan-local plugins/voyage (rename detected, history preserved) - .claude-plugin/marketplace.json: voyage entry replaces ultraplan-local - CLAUDE.md: voyage row in plugin list, voyage in design-system consumer list - README.md: bulk rename ultra*-local commands -> trek* commands; ultraplan-local refs -> voyage; type discriminators (type: trekbrief/trekreview); session-title pattern (voyage:<command>:<slug>); v4.0.0 release-note paragraph - plugins/voyage/.claude-plugin/plugin.json: homepage/repository URLs point to monorepo voyage path - plugins/voyage/verify.sh: drop URL whitelist exception (no longer needed) Closes voyage-rebrand. bash plugins/voyage/verify.sh PASS 7/7. npm test 361/361.
5 KiB
5 KiB
| type | review_version | created | task | slug | project_dir | brief_path | scope_sha_start | scope_sha_end | reviewed_files_count | verdict | findings | ||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| trekreview | 1.0 | 2026-05-01 | Add JWT authentication with refresh-token rotation | jwt-auth | .claude/projects/2026-05-01-jwt-auth/ | .claude/projects/2026-05-01-jwt-auth/brief.md | 0123456789abcdef0123456789abcdef01234567 | fedcba9876543210fedcba9876543210fedcba98 | 3 | WARN |
|
Review: Add JWT authentication with refresh-token rotation (Run B)
Executive Summary
Same diff as Run A, re-reviewed independently to test determinism. This run found the same 5 findings plus one extra (a placeholder TODO in logout.mjs that Run A missed). Verdict: WARN — same as Run A; the extra finding is MAJOR but does not change the merge gate.
This is a SYNTHETIC v1.0 fixture for testing the Jaccard determinism pipeline. Run A's set is a strict subset of Run B's set, giving Jaccard = 5/6 = 0.833.
Coverage
| File | Treatment | Reason |
|---|---|---|
lib/auth/jwt.mjs |
deep-review | Security-critical (token signing/verification) |
lib/handlers/login.mjs |
deep-review | Auth surface |
lib/handlers/logout.mjs |
deep-review | Auth surface |
package-lock.json |
skip | Lockfile |
dist/** |
skip | Build output |
Findings (BLOCKER)
763d174e6c519fafbadcba5d1706708479e36e61
- Location:
lib/handlers/login.mjs:23 - Rule:
UNIMPLEMENTED_CRITERION - Brief ref: SC-2 ("login endpoint MUST return 401 on invalid credentials")
- Evidence: Handler returns 200 with empty body when password mismatch occurs.
- Fix: Return 401 with WWW-Authenticate header per brief SC-2.
Findings (MAJOR)
d2d0e27875ae9ef0d818cb08bb6f14e6d33c4232
- Location:
lib/auth/jwt.mjs:42 - Rule:
SECURITY_INJECTION - Brief ref: Non-Goal #3 ("must not accept user-supplied algorithm header")
- Evidence:
jwt.verify(token, secret, { algorithms: req.body.alg })— algorithm taken from request body. - Fix: Hard-code
algorithms: ['RS256']; reject any token claiming a different alg.
7861519c326c207aabf17072db51c469bebc217b
- Location:
lib/auth/jwt.mjs:88 - Rule:
MISSING_TEST - Brief ref: SC-4 ("refresh-token rotation must be tested under concurrent refresh")
- Evidence: No test in
tests/covers the concurrent-refresh path; only happy-path is exercised. - Fix: Add
tests/auth/concurrent-refresh.test.mjscovering the race window.
7a3d7d0a668f6431ef3877ceeb106023b0f6295e
- Location:
lib/handlers/login.mjs:56 - Rule:
PLAN_EXECUTE_DRIFT - Brief ref: Plan Step 4 ("login.mjs uses bcrypt.compare()")
- Evidence: Plan said
bcrypt.compare; implementation usescrypto.timingSafeEqualover plaintext-derived buffers. - Fix: Either update plan + brief to record the deviation or refactor to bcrypt.compare per plan.
bf3e8b347cf4269ad005a9cf64dab6f601345704
- Location:
lib/handlers/logout.mjs:14 - Rule:
PLACEHOLDER_IN_CODE - Brief ref: none (Rule 7a violation)
- Evidence:
// TODO: invalidate refresh-token cookie before responding— left in committed code. - Fix: Implement the cookie invalidation or remove the comment with an issue link.
Findings (MINOR)
400dfcff81e0e219eb04a7123c68ae870696f121
- Location:
lib/auth/jwt.mjs:117 - Rule:
MISSING_ERROR_HANDLING - Brief ref: none (engineering hygiene)
- Evidence:
await refreshTokenStore.delete(jti)is not wrapped — store-down throws bubble to top-level handler. - Fix: Wrap in try/catch; log + 503 on store failure.
Remediation Summary
6 findings total: 1 BLOCKER, 4 MAJOR, 1 MINOR. Same merge gate as Run A; one
extra MAJOR (PLACEHOLDER_IN_CODE) that Run A missed. Run a remediation plan via
/trekplan --brief review.md.
{
"fixture_kind": "synthetic-v1.0",
"jaccard_with_run_A": "5/6 = 0.833",
"findings": [
{"id": "763d174e6c519fafbadcba5d1706708479e36e61", "severity": "BLOCKER", "rule": "UNIMPLEMENTED_CRITERION", "file": "lib/handlers/login.mjs", "line": 23},
{"id": "d2d0e27875ae9ef0d818cb08bb6f14e6d33c4232", "severity": "MAJOR", "rule": "SECURITY_INJECTION", "file": "lib/auth/jwt.mjs", "line": 42},
{"id": "7861519c326c207aabf17072db51c469bebc217b", "severity": "MAJOR", "rule": "MISSING_TEST", "file": "lib/auth/jwt.mjs", "line": 88},
{"id": "7a3d7d0a668f6431ef3877ceeb106023b0f6295e", "severity": "MAJOR", "rule": "PLAN_EXECUTE_DRIFT", "file": "lib/handlers/login.mjs", "line": 56},
{"id": "bf3e8b347cf4269ad005a9cf64dab6f601345704", "severity": "MAJOR", "rule": "PLACEHOLDER_IN_CODE", "file": "lib/handlers/logout.mjs", "line": 14},
{"id": "400dfcff81e0e219eb04a7123c68ae870696f121", "severity": "MINOR", "rule": "MISSING_ERROR_HANDLING", "file": "lib/auth/jwt.mjs", "line": 117}
]
}