Skip to content

fixtures/review/bad: the route fixture claims a PII leak its code cannot produce #11

Description

@chiibitsu

What it is

fixtures/review/bad/src/app/api/items/route.ts describes two planted defects. It contains one.

The second is stated in its own comment:

if (error) {
  // Second planted defect, deliberately quieter: the caller's email lands in the log.
  console.error("items query failed for", data, error.message);

No email can reach that line.

Why it cannot happen

  1. data is null whenever error is set — standard @supabase/supabase-js v2 contract. The interesting operand is always null on the error branch.
  2. items has no email column. From nextjs-supabase-template's supabase/migrations/20260901000000_orgs_items.sql, the columns are id, org_id, title, body, created_by uuid, created_at. No address anywhere, and created_by is a uuid.

So the line only ever prints null plus a generic Postgrest error string. The comment asserts a PII leak; the code has none.

How it surfaced

The draft reviewer read this fixture in nextjs-supabase-template#6 and correctly dropped the claim, citing the migration and the null-on-error contract, then kept only the unscoped-query finding. That was the verification bar working in the direction that costs a reviewer a finding — so this is not a reviewer bug.

But it means the fixture is not the thing it says it is. A planted-failure file whose stated defect is absent is the fixture equivalent of a check whose message claims more than the check asserts — a defect this toolkit's own REVIEW.md names explicitly. A future reviewer that dutifully reports both defects would be marked correct for inventing one.

Proposed fix — a fixture change, not a comment edit

Pick one, and the first is better:

  1. Make the line actually carry PII. Give the fixture a shape where an identifier reaches the log — e.g. log a caller-derived value on the error branch, or have the fixture select a joined column that is an address. Then the comment is true and the fixture tests two defect shapes, which is more valuable than one.
  2. Delete the claim and leave the unscoped query as the single planted defect.

Do not just soften the comment's wording. The comment is not the problem; the mismatch is. Option 1 preserves the fixture's intent — a quieter second defect the reviewer might miss — which is worth keeping if the reviewer is meant to be tested on PII-in-logs at all.

Priority

Lower than #10. This one makes a fixture weaker than advertised; it does not let a real defect through a green check.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions