Review Agent PRs That Parse Untrusted JSON Without a Schema

The on-call reviewer opened a quiet Monday pull request. An agent had added a payment webhook handler overnight. The diff looked tiny, and continuous integration was green.

A single new file parsed the request body as JSON. No schema file appeared anywhere in the patch. The catch block returned an empty object instead.

This pattern shows up constantly in agent-generated service code. The model wants a green typecheck and a short happy path. Untrusted bytes become trusted fields without a contract.

What this review is for

This article is a review playbook, not a framework tour. It covers agent pull requests that call JSON.parse on untrusted input. It also covers silent fallbacks that hide malformed payloads.

The reviewer learns what to trust, revert, and test. The method stays useful without any hosted coding product. A later section uses a free server only as a test runner.

The incident shape

The agent usually receives a ticket about inbound webhooks. It then invents a handler, a type, and a persistence call. The generated TypeScript often looks like the block below.

// proposed handler (agent PR) — example, not production code
export async function handleWebhook(req: Request, db: Db) {
  try {
    const payload = JSON.parse(String(req.body)) as WebhookEvent;
    await db.receipts.insert({
      id: payload.id,
      amount: payload.data.amount,
      email: payload.data.customer.email,
    });
    return { ok: true };
  } catch {
    return { ok: true, payload: {} };
  }
}

The cast tells the compiler the shape is already known. The empty catch tells operators that parse failure is success. Both lies will pass a shallow unit test without junk input.

Step 1. Isolate every parse hunk

Reviewers should not read the pull request as one story. They should list every JSON.parse and body-parser site. A unified diff is enough for this first pass.

git fetch origin pull/1842/head:pr-1842
git checkout pr-1842
git diff origin/main...HEAD -U5 -- '*.ts' '*.js'
git diff origin/main...HEAD -U0 | rg -n "JSON\.parse|req\.body|as [A-Z]"

Record each hit in a three-column review note. Column one is the file path and line. Column two is the trust boundary for those bytes.

Column three is the failure mode after a bad parse. Allowed labels are throw, empty object, null, or log.

Step 2. Classify trust before reading types

Agent comments often claim the payload is already validated. That sentence is not evidence of a runtime check. The source of the bytes is the only evidence.

Trust only these hunks during the first review pass.

1. Parse calls on fixture files written in the same patch.
2. Parse calls on constants defined inside the same patch.
3. Reuse of an existing, already reviewed schema helper.

Revert or block these hunks until poison tests exist.

1. HTTP bodies, query strings, and raw webhook bytes.
2. Queue messages and socket frames from other services.
3. Browser storage, cookies, and user-supplied CLI arguments.
4. Type assertions placed directly after JSON.parse calls.

The type assertion is not a runtime check at all. JSON.parse only proves the bytes were JSON text. It does not prove field names, types, or presence.

Step 3. Reject silent empty-object fallbacks

Empty objects are the agent's favorite repair trick. They keep CI green when a fixture is missing fields. They also drop payment events on the floor silently.

Use the decision table before arguing about style.

Failure behavior	Review action	Reason
Rethrow after logging parse error	Trust if logs omit secrets	Operators can see poison input
Return 400 with a stable error code	Trust for HTTP handlers	Callers can retry with a fix
Return {} or [] and continue	Revert	Downstream treats absence as valid
Empty catch with no log or metric	Revert	No signal and no test hook
Cast and proceed without a catch	Test, then likely revert	Invalid JSON still crashes later
Schema library with a 422 mapping	Trust after tests	Contract is explicit

The reviewer should restore a hard failure on malformed JSON. The agent may keep a typed error mapping for callers. It cannot keep a fake payload after a parse error.

// replacement contract — proposed example, not a production patch
import { WebhookEventSchema } from "./webhook-schema";

export async function handleWebhook(req: Request, db: Db) {
  const raw = String(req.body ?? "");
  let json: unknown;
  try {
    json = JSON.parse(raw);
  } catch (err) {
    throw new HttpError(400, "invalid_json", err);
  }
  const parsed = WebhookEventSchema.parse(json);
  await db.receipts.insert({
    id: parsed.id,
    amount: parsed.data.amount,
    email: parsed.data.customer.email,
  });
  return { ok: true };
}

Label this block as a proposed rewrite, not production fact. Teams should swap the schema import for their validator.

Step 4. Demand tests that send poison

Green CI on the happy fixture proves almost nothing here. The review should require four cases before any merge.

// tests/webhook-parse.test.ts — proposed review gate, not executed results
import { handleWebhook } from "../src/handle-webhook";

const db = { receipts: { insert: async () => undefined } };

test("rejects empty body", async () => {
  await expect(handleWebhook({ body: "" } as Request, db)).rejects.toMatchObject({
    status: 400,
    code: "invalid_json",
  });
});

test("rejects truncated object", async () => {
  await expect(handleWebhook({ body: '{"id":' } as Request, db)).rejects.toMatchObject({
    status: 400,
    code: "invalid_json",
  });
});

test("rejects missing amount", async () => {
  const body = JSON.stringify({
    id: "evt_1",
    data: { customer: { email: "a@b.c" } },
  });
  await expect(handleWebhook({ body } as Request, db)).rejects.toMatchObject({
    status: 400,
  });
});

test("inserts only after schema success", async () => {
  const insert = jest.fn(async () => undefined);
  const body = JSON.stringify({
    id: "evt_1",
    data: { amount: 1999, customer: { email: "a@b.c" } },
  });
  await handleWebhook({ body } as Request, { receipts: { insert } });
  expect(insert).toHaveBeenCalledWith({
    id: "evt_1",
    amount: 1999,
    email: "a@b.c",
  });
});

These tests are a proposed review gate, not executed results. They are unlabeled measurements and should be run locally.

Step 5. Scan the diff with a small harness

Human reviewers miss second parse sites in utility files. A short scanner keeps the classification list honest.

// tools/scan-json-parse.mjs — proposed local harness
import { execSync } from "node:child_process";

const diff = execSync("git diff origin/main...HEAD", { encoding: "utf8" });
const hits = [];
let file = "";
for (const line of diff.split("\n")) {
  if (line.startsWith("+++ b/")) file = line.slice(6);
  if (!line.startsWith("+") || line.startsWith("+++")) continue;
  const text = line.slice(1);
  if (/JSON\.parse\s*\(/.test(text)) {
    hits.push({ file, kind: "parse", text });
  }
  if (/catch\s*\([^)]*\)\s*\{\s*\}/.test(text)) {
    hits.push({ file, kind: "empty-catch", text });
  }
  if (/as any|as unknown as|as [A-Z][A-Za-z0-9_]+/.test(text) && /parse/.test(text)) {
    hits.push({ file, kind: "cast-after-parse", text });
  }
}
if (hits.length === 0) {
  console.log("no parse hunks in diff");
  process.exit(0);
}
for (const hit of hits) {
  console.log(`${hit.kind}\t${hit.file}\t${hit.text.trim()}`);
}
process.exit(2);

Run the scanner from the repository root after fetch.

node tools/scan-json-parse.mjs
echo $?

Exit code two means classification work remains unfinished. The script does not parse TypeScript grammar at all. It only flags added hunks that need a human label.

Suggested review comments

Paste short comments on the parse hunk, not the whole file. Three comments cover most agent webhook diffs of this shape.

Revert the empty catch. Malformed JSON must fail the handler.
JSON.parse plus a type assertion is not a schema.
Add tests for truncated JSON and a missing amount field.

Each comment is a command, not a discussion prompt. The authoring agent can act on commands without extra chat.

Where a free hosted runner fits

Some teams draft first review notes with a coding model. That step is optional for any team using this playbook. The scanner and poison tests already stand on their own.

MonkeyCode is an open-source coding assistant with free model access. Disclosure: This article was prepared as part of MonkeyCode's product outreach. It also offers a free server option for running checks.

The project documents a 10 million free-token allotment for model access. This article does not name models or remaining quota. It also does not report latency, hardware, or uptime claims.

A reviewer can paste classified hunks into a free model session. The prompt should ask for revert-or-test notes only. The model must not invent files missing from the diff.

Review only the added hunks below.
For each hunk, answer trust, revert, or test.
Forbid empty-object fallbacks on JSON.parse failures.
Do not invent files that are not in the diff.

The free server is a clean place to run the scanner. It can also run the four poison tests after clone. Keep the model away from production secrets and live keys.

The webhook fixture must use fake emails and fake ids. Curious readers can try that optional loop on the free server.

What to trust after the rewrite

Trust a schema parse that throws a mapped HTTP error. Trust tests that send truncated JSON and missing fields. Trust logs that record error class without secret headers.

Do not trust agent comments that simply say validated. Do not trust a WebhookEvent cast beside JSON.parse. Do not trust an ok flag returned from catch.

Limitations

The scanner is a regex pass over unified diffs. It will miss parse wrappers hidden behind local helpers. It will also flag JSON.parse inside generated snapshots.

The playbook assumes TypeScript or JavaScript HTTP services. It does not cover protobuf, Avro, or XML partner payloads. It does not measure model quality or token consumption.

The four tests do not prove idempotency or replay safety. Payment webhooks still need signature checks outside this article. Skipping signatures is a separate revert, not a style note.

Who should not use this approach

Do not apply the empty-object rule to closed generated fixtures. Do not run untrusted branches as root on a shared server. Do not paste live webhook payloads into a hosted model prompt.

Do not treat a model review comment as merge approval. Teams without JSON at the trust boundary can skip this. They still need a different review for SQL and files.

Close

Agent pull requests often hide contract work inside one parse. The contract belongs in a schema, a 400, and poison tests. The reviewer who classifies hunks first reverts less blindly.