- Home
- Skills
- Code Quality & Review
- Code Review — Human-Voiced PR Reviews
Works with the AI Agents you already use
Code Review — Human-Voiced PR Reviews
Reviews a pull request and writes a paste-ready, first-person review ordered by severity, with file:line evidence.
Free
Code Review — Human-Voiced PR Reviews
Example session with this skill installed
Can you review this PR before I approve it?
Title: "Add password reset request endpoint"
Description: "Adds POST /auth/password-reset. Sends an email with a reset link. Tokens expire after 1 hour."
+++ b/src/auth/passwordReset.ts
@@ -0,0 +1,29 @@
+import { Router } from "express";
+import { db } from "../db";
+import { mailer } from "../mailer";
+import { logger } from "../logger";
+
+export const passwordResetRouter = Router();
+
+passwordResetRouter.post("/auth/password-reset", async (req, res) => {
+ const { email } = req.body;
+ logger.info(`Password reset requested for ${email}`);
+
+ const user = await db.user.findUnique({ where: { email } });
+ if (!user) {
+ return res.status(404).json({ error: "No account with that email" });
+ }
+
+ const token = Math.random().toString(36).slice(2);
+ await db.passwordResetToken.create({
+ data: { userId: user.id, token },
+ });
+
+ try {
+ await mailer.send(user.email, "Reset your password", `https://app.example.com/reset?token=${token}`);
+ } catch (err) {
+ logger.error(err);
+ }
+
+ return res.json({ ok: true });
+});
+++ b/src/auth/passwordReset.test.ts
@@ -0,0 +1,4 @@
+it("sends a reset email", async () => {
+ const res = await request(app).post("/auth/password-reset").send({ email: "ana@example.com" });
+ expect(res.status).toBe(200);
+});
- Read your context and instructions
- Compiled the code review
Summary
I read the handler and the test as pasted and traced the reset flow from request to email. I didn't run the suite, because I only have the diff. The structure is fine, but the token can be predicted, the email address goes into the logs, and the one-hour expiry promised in the description isn't in the code. All three are on an account-takeover path, so this needs changes before it merges. Once those three are fixed, I'd approve with the remaining comments.
Needs to change before merge
The reset token is predictable
src/auth/passwordReset.ts:17 builds the token with Math.random().toString(36).slice(2). Math.random isn't a cryptographic generator, and its output can be predicted, which turns a reset link into an account-takeover vector. The token is also stored in plain text at line 18, so anyone with read access to that table, a backup, or a replica can reset any pending account. Generate it with crypto.randomBytes(32).toString("base64url"), store only a SHA-256 hash of it, and compare hashes when the link is used.
The email address is written to the logs
Line 10 logs Password reset requested for ${email}, which puts PII into every log line for this endpoint, including for addresses that don't belong to any account. Log the internal user.id after the lookup, or log nothing before it. The logger.error(err) at line 25 deserves a look for the same reason: many mailer errors include the recipient address in their message.
The one-hour expiry in the description isn't implemented
The description says tokens expire after 1 hour, but passwordResetToken.create at line 18 stores no expiresAt, and nothing in this diff consumes or invalidates a token. As written, a reset link works forever and can be used more than once. Add expiresAt, mark the token used on redemption, and add a test that an expired or already-used token is rejected. If the redemption side lives in another PR, point me to it and I'll check it there instead.
Worth fixing in this PR
The 404 reveals which emails have accounts
Lines 13–15 return 404 "No account with that email", so anyone can probe the endpoint to find out who is registered. Return the same 200 with a neutral message ("If this email is registered, we'll send a link") whether or not the user exists. Ideally, send the email after the response so the timing doesn't give it away either. This doesn't block the merge, but it's a small change and belongs here.
A failed send is reported as success
The catch at lines 24–26 logs the error and still returns { ok: true }. If the mailer is down, users wait for an email that never arrives, and nothing signals the outage. Keep the neutral response for the reason above, but log with user.id and an error code and emit a metric you can alert on.
email isn't validated
req.body.email goes straight into findUnique at line 12 without a check that it's a string. Validate the body with the project's schema validator and return 400 on anything else, so malformed input fails clearly instead of reaching the ORM.
Follow-ups and take-it-or-leave-it
- Fine as a follow-up: rate-limit this endpoint per IP and per address. Without a limit, it can be used to flood someone's inbox and run up the mail bill. If a global limiter already covers
/auth/*, ignore this. - Take it or leave it: move the hardcoded
https://app.example.com/resetbase URL at line 23 into config, so staging links don't point to production.
Tests
The only test checks that the status is 200. It would still pass if no token were stored, no email were sent, and the token were logged, so it doesn't prove the behavior. I'd want tests showing that an unknown address gets the same response as a known one, that the stored token is hashed and has an expiresAt, that a mailer failure still returns the neutral response and records the error, and that no log call receives the email address.
Connects securely to your tools. The creator never sees your data.
About this skill
Reviewing Code produces the kind of review a competent engineer writes after actually doing the work. Every finding is a complete sentence with file:line evidence, the cause and consequence, and a disposition: fix before merge, fix in this PR, follow-up, or take it or leave it. The review is printed in chat, safe to paste into GitHub, and written in the PR's own language.
Problems it solves: reviews that read like a filled-in form, verdicts inflated to "request changes" over preferences, missed security defects in AI-written code, and review comments that break on GitHub (#N auto-links, accidental Fixes keywords, hard-wrapped prose).
Workflow (10 steps): read the whole PR → security lens (five trust questions) → comprehension audit (can every block be defended?) → structural checks → cross-platform checks → correctness checks (enum completeness, unwrapped control-flow exceptions, failure paths) → testing checks → internal triage (Blocker / Important / Recommended / Nice-to-have) → render as prose sections → capture a learning.
Verdict rule: "Request changes" only when there is a true Blocker (security defect, PII or data loss, broken build, untested high-risk path). Important findings alone still get approved, with comments.
Inside:
references/security-checks.md: PII in logs, secrets, AI/LLM security, webhook and signature integrations, AI runtime resilience.references/structural-checks.md: file-size and complexity limits, SOLID, DRY/KISS/YAGNI, layering, error handling.references/correctness-checks.md: state machines and failure-path tests.references/grep-recipes.md: grep commands that catch common violations.references/checklist.md: the full checklist with framework notes.references/output-format.md: the voice rules and the copy-paste safety check.
Triggers on: "review this PR", "code review", "review this diff", "second opinion on this branch".
Free and MIT-licensed, part of claude-skills-czl.
Source on GitHub: this skill · full repository
How to use
Skills on Agensi run inside your AI agent. Copy the prompt, paste it into your agent, and it connects to Agensi and installs this skill for you. If a payment is needed, your agent shows it to you first.
Reviews
No reviews yet
Be one of the first to try it. Every listed skill passes our trust checks below.
Security scanned
Passed our 8-point scan before listing
Fresh listing
Recently published to Agensi
Free forever
No account required to browse
Trust & safety
Security scanned
Verified clean 11 days ago
- Free to download with an account
Requires