Plan Before Code With Role Split

Act as a senior software engineer working as a planner first and a builder second. You must not write any implementation code until I approve your plan.

Task: Prepare a plan for this change to my codebase: Add a password reset flow to our Express and PostgreSQL web app. Users should request a reset link by email, and the link should expire after 30 minutes. The link must work only once, and the existing login route must not change.. Relevant context: Node 20 with Express 4, Postgres accessed through the pg library, and emails sent via SendGrid in services/mailer.js. Users are stored in a users table with id, email and password_hash. Authentication lives in routes/auth.js and tests use Jest with Supertest..

Steps:
1. As the Planner, restate the goal in one sentence and list any assumptions or questions that could change the approach.
2. Name the files, functions or tables you expect to touch, and give a numbered implementation plan with small, testable steps.
3. As the Reviewer, switch roles and challenge your own plan: list the three most likely risks, missed edge cases or simpler alternatives.
4. Revise the plan to address that critique, then stop and ask me to approve it.

Output:
Use the headings Goal, Assumptions, Plan, Review, Revised Plan. Finish with the single line "Awaiting approval to start coding." Include no code blocks.

# Goal Add a self-service password reset flow in which a user requests a reset link by email, the link expires after 30 minutes and works only once, and the existing login route stays unchanged. # Assumptions - Questions that could change the approach: 1. Should the reset link point to a frontend page (SPA) or a server-rendered page? This determines whether I add a GET route with a form or only a JSON API. 2. Do you have a migrations tool (node-pg-migrate, Knex, plain SQL files), or should I add a plain SQL migration file? 3. Which hashing library does routes/auth.js use (bcrypt, bcryptjs, argon2)? I will reuse it so the new hashes match the login check. 4. Are sessions or tokens (JWT) used for login? If so, should a successful reset invalidate existing sessions? 5. Is there any existing rate limiting middleware (e.g. express-rate-limit)? Is adding a dependency acceptable? 6. Does a SendGrid template exist for this email, or should I send a simple text/HTML message from services/mailer.js? 7. Is there a minimum password policy I should enforce? - Working assumptions if you don't answer: - JSON API only. The email link points to a configurable frontend URL (an environment variable) with the token in the query string. - A plain SQL migration file is acceptable. - Tokens are random 32-byte values from Node's crypto module. Only a SHA-256 hash is stored in the database. - The request endpoint always returns the same response whether or not the email exists, to prevent account enumeration. - Login in routes/auth.js remains untouched. New routes go in a new file, mounted separately. # Plan Files and tables I expect to touch: - New table: password_reset_tokens (id, user_id, token_hash, expires_at, used_at, created_at) - New migration file for that table - New: routes/passwordReset.js - New: services/passwordResetService.js (token creation and consumption logic) - Edit: services/mailer.js (add a sendPasswordResetEmail function) - Edit: the app entry file (mount the new router only) - New tests: tests/passwordReset.test.js - Config: environment variables for the frontend reset URL and the token TTL - Not touched: the login handler in routes/auth.js Steps: 1. Write the migration for password_reset_tokens with a foreign key to users, a unique index on token_hash, and an index on user_id. Verify it applies and rolls back cleanly. 2. Add the service function to create a token: generate random bytes, store the SHA-256 hash with expires_at set to now plus 30 minutes, and return the raw token. Unit test it. 3. Add sendPasswordResetEmail to the mailer, building the link from the configured base URL. Test it with SendGrid mocked. 4. Add POST /password-reset/request. Look up the user by lowercased email, create a token and send the email if found, and always return the same 200 response. Test both the known and unknown email cases. 5. Add the service function to consume a token: hash the submitted value, then in a single transaction run an atomic UPDATE that sets used_at only where the hash matches, used_at is null, and expires_at is in the future. If it matches no row, reject. Otherwise update the user's password_hash. 6. Add POST /password-reset/confirm taking token and new password. Validate input, call the consume function, and return a generic error for invalid, expired or used tokens. Test the success, expired, reused and malformed cases. 7. Mount the router in the app entry file and confirm via a regression test that existing login tests pass without modification. 8. Run the full Jest suite and review the diff to confirm routes/auth.js is unchanged. # Review Three most likely risks and gaps: 1. **Race conditions and token accumulation.** Two concurrent confirm requests could both succeed if the check and the update are separate. Also, a user who requests several links leaves several valid tokens active, which widens the attack surface. 2. **Abuse and information leaks.** Without rate limiting, the request endpoint can be used to spam a user's inbox, and response timing can still reveal whether an email exists, since sending mail only happens for real accounts. 3. **Incomplete post-reset security.** Existing sessions or tokens stay valid after a password change, and the user gets no notice if someone else changed their password. A simpler alternative, signing a stateless token with the current password hash as part of the secret, would avoid the new table but is harder to make strictly one-time and to audit, so I would keep the table. # Revised Plan Changes from the critique: 1. In step 2, invalidate earlier unused tokens for the user (set used_at) when a new one is issued, so only the latest link works. Step 5 keeps the single atomic UPDATE with the password change in the same transaction, and I add a concurrency test that fires two confirms at once and expects exactly one success. 2. In step 4, send the email asynchronously after responding (or without awaiting it, with errors logged) so response time does not reveal whether the account exists. Add per-IP and per-email rate limiting, using express-rate-limit if you approve the dependency, or a small in-house limiter otherwise. 3. In step 6, after a successful reset, invalidate the user's existing sessions if the app has a session store, and send a "your password was changed" notification email through the mailer. Enforce the password policy you specify, defaulting to a minimum length of 8 characters. 4. Add a cleanup step: a small function that deletes tokens expired for more than a day, runnable from a scheduled job or on request, so the table does not grow unbounded. 5. Never log raw tokens or full reset URLs, and keep the token out of any error messages. The step order is otherwise unchanged: migration, token creation, mailer, request route, consume logic, confirm route, mounting and regression, then a full test run and diff check that routes/auth.js is unmodified. Please answer any of the questions above that differ from my working assumptions, or reply with approval to proceed on them. Awaiting approval to start coding.