Add sample user module - #1
Conversation
There was a problem hiding this comment.
AI Code Review
This PR adds a sample JavaScript file containing intentionally buggy code to serve as a test case or demonstration for an AI code reviewer. The file implements three user-related database functions with two documented bugs: a missing null check after a database query that would cause a runtime error if a user doesn't exist, and a missing authorization check in the delete function that would allow any caller to delete any user. The processUsers function also sequentially awaits each database call rather than using Promise.all, which is a performance issue worth noting. The overall purpose appears to be providing realistic example input to validate that an automated review tool can detect common security and correctness issues.
No bugs found in the reviewed files.
There was a problem hiding this comment.
AI Code Review
This PR adds a sample JavaScript file with intentional bugs to serve as test input for the AI code review system, and fixes a parsing robustness issue where Claude's responses were sometimes wrapped in markdown code fences. The stripFences utility function is introduced in review.js and inlined in verifier.js to strip those fences before attempting JSON.parse, preventing crashes when the model includes backtick-wrapped JSON despite being instructed not to. The fix addresses a real-world failure mode where even with explicit prompting, large language models occasionally wrap their output in code blocks, and the previous code would throw on JSON.parse of a string starting with triple backticks.
Findings: 2 high, 0 medium — see inline comments
| async function getUser(db, userId) { | ||
| const row = await db.query('SELECT * FROM users WHERE id = ?', [userId]); | ||
| // Bug 1: no null check — row could be undefined if user doesn't exist | ||
| return { name: row.name, email: row.email }; |
There was a problem hiding this comment.
[HIGH] No null check on db.query result before property access
If db.query returns undefined, null, or an empty result when no user matches the given userId, accessing row.name and row.email will throw a TypeError: Cannot read properties of undefined. There is no guard before dereferencing row.
Category: null/nil dereference
|
|
||
| async function deleteUser(db, userId, requestingUserId) { | ||
| // Bug 2: no authorization check — any caller can delete any user | ||
| await db.query('DELETE FROM users WHERE id = ?', [userId]); |
There was a problem hiding this comment.
[HIGH] Missing authorization check in deleteUser allows any caller to delete any user
deleteUser accepts a requestingUserId parameter but never uses it. There is no check that the requesting user has permission to delete the target userId, allowing any authenticated (or even unauthenticated) caller to delete arbitrary users.
Category: security issues
There was a problem hiding this comment.
Preflight Review
This PR renames the project from "AI Code Review Bot" to "Preflight" throughout the codebase, updating all log prefixes, GitHub Actions workflow names, job identifiers, and PR comment headers accordingly. It also adds a defensive stripFences utility to handle cases where Claude wraps its JSON response in markdown code fences, which would previously cause JSON.parse to throw and lose all findings. The same fence-stripping logic is applied in both the main review pass and the verifier pass. Finally, a sample JavaScript file with intentionally buggy code is added to serve as a test fixture for the reviewer to catch.
No bugs found in the reviewed files.
Summary
examples/sample.jswith a user lookup and delete moduleTest plan