Skip to content

Security: require auth on entry/photo mutations (IDOR fix) + constant-time compare + CI - #5

Merged
davidcjw merged 1 commit into
masterfrom
security/audit-hardening
Jul 2, 2026
Merged

Security: require auth on entry/photo mutations (IDOR fix) + constant-time compare + CI#5
davidcjw merged 1 commit into
masterfrom
security/audit-hardening

Conversation

@davidcjw

@davidcjw davidcjw commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes a high-severity IDOR / public-write vulnerability on the memory mutation endpoints, hardens password/token comparison against timing attacks, and adds CI (the repo had none).

Vulnerability (HIGH — IDOR / unauthenticated write)

The mutation handlers acted on a caller-supplied entry id with no per-handler authorization:

  • src/app/api/entries/[id]/route.tsPATCH (edit a memory's description)
  • src/app/api/entries/[id]/photos/route.tsPOST (add photo), DELETE (remove photo)

The only gate was the optional SITE_PASSWORD proxy (src/proxy.ts), which returns NextResponse.next() when the password is blank (the documented default = public read). On the default config, anyone could edit or delete any entry or photo by guessing/enumerating ids — a classic Insecure Direct Object Reference, made worse by the endpoints being fully unauthenticated.

Vulnerability (LOW — non-constant-time comparison)

src/app/api/auth/login/route.ts and src/proxy.ts compared the password / session token with !== / ===, which short-circuits and can leak length/prefix information via timing.

Fix

  1. Fail-closed authorization on all writes. New requireAuth(req) helper in src/lib/auth.ts validates the owner session cookie (lj_session, the same token the login flow issues) and returns 401 when absent/invalid. Crucially, if SITE_PASSWORD is unset there is no owner session to validate, so writes are rejected (401) rather than falling open. It is called at the top of every mutation handler (entries PATCH, photos POST, photos DELETE). GET/read handlers are untouched — public reads still work.
  2. Constant-time comparison. timingSafeStringEqual uses crypto.timingSafeEqual on equal-length Buffers (with an explicit length guard, since timingSafeEqual throws on unequal lengths). The login route and proxy now use it instead of !==/===.

CI

Added .github/workflows/ci.yml (lint + build + test on Node 20, plus npm audit and gitleaks secret scan). The repo previously had no CI.

Verification

  • npm run build — passes (including the Proxy/Middleware edge bundle; node:crypto's timingSafeEqual builds fine there).
  • npm run lint — clean.
  • npm test — 38/38 pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BAMbBXXNhnUcztWtqTHzmu

…-time compare + CI

Fixes an IDOR on the write endpoints and hardens password/token comparison.

- Add requireAuth(req) in src/lib/auth.ts validating the owner session cookie
  (lj_session) and failing closed (401) when SITE_PASSWORD is unset, so public
  READ can stay open but public WRITE cannot.
- Enforce requireAuth at the top of every mutation handler:
  entries [id] PATCH, and entries [id]/photos POST and DELETE.
- Replace non-constant-time string comparisons in the login route and proxy
  with crypto.timingSafeEqual on equal-length Buffers (length-guarded).
- Add GitHub Actions CI (lint/build/test + npm audit + gitleaks); repo had none.

Claude-Session: https://claude.ai/code/session_01BAMbBXXNhnUcztWtqTHzmu
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
life-journal Ready Ready Preview, Comment Jul 2, 2026 2:32pm

@davidcjw
davidcjw merged commit 96f5358 into master Jul 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant