Sign up
View stargazersSign up

verify: verify.sh accepts a number with a leading zero that Package Verification rejects as Invalid JSON#1922

opened10d agobyMMatt Donovan

Note

Claude Code responding on behalf of Matt.

Defect: the script shipped inside an evidence package passes a record that spells a number with a leading zero, which the app fails as unreadable.

What: A signed record rewritten so that a number reads 01 instead of 1 still passes the shipped script, while the app rejects the same package; no value changes, but the two checkers disagree. It was kept out of the member-chain change because the problem predates it and the fix needs a strict reading rule the script's JSON tool does not have.

Where it is and how to reproduce it

Where: src/main/services/verifyScript.ts:366 Reproduce: Export an unshared Case, rewrite "index":1 as "index":01 on line 2 of manifest.jsonl, run sh verify.sh (exit 0), then verify the package in the app (Invalid JSON at index 1). The same holds for "exhibitNumber":2 as 02 in a member chain under #1880.

1jq -c . <<<'{"index":01}' # jq 1.7 prints {"index":1}; JSON.parse throws

Generated by Claude Code

MMatt Donovan6d ago

Note

Written by claude-opus-5-5 via Claude Code on behalf of Matt.

This was generated by AI during triage.

Triage: bug, evidence-affecting, ready for an agent. Reproduced on current main.

The checking script shipped inside every evidence package passes a package that the app fails as unreadable. Rewriting a record's number as 01 instead of 1 gave a full pass from the script and an "Invalid JSON" failure from the app, both for an ordinary Case and for a member's record in a Shared Case. No value changes, so the edit hides nothing, but the two checkers disagree on the same package.

Leading zeros are one case of a wider gap. The script reads each record with a tool that accepts several number spellings the app rejects, and a second record on the same line. Scope is widened to that whole set, at the maintainer's direction during triage. The brief below is the contract for the work.

Agent brief

Agent Brief

Category: bug Summary: verify.sh must reject, as Invalid JSON, every signed chain line that the package verifier's JSON.parse rejects and jq accepts, so the two checkers never disagree on whether a line is readable.

Current behavior: The shipped verify.sh decides whether a signed chain line is valid JSON by running it through jq empty, then reads its fields through jq. The package verifier parses the same line with JSON.parse and reports Invalid JSON when that throws. jq is more lenient than JSON.parse, and recomputes each Entry Hash from its own normalized reading, so a line spelled the lenient way keeps a matching hash and passes every script step. The script already closes two lenient cases by hand, a byte-order mark at the start of a line (bom_led) and a NUL byte (nul_line), and has no check for the cases below.

Spellings jq 1.8.2 accepts and JSON.parse rejects (checked during triage):

Line contentjq reads it as
01, 00, -01, 01.5 (leading zero)1, 0, -1, 1.5
+1 (leading plus)1
1., .5, 1.e5 (bare decimal point)1, 0.5, 1E+5
nan, NaN, -nannull
Infinity, infinity, -Infinity±1.7976931348623157e+308
{"a":1}{"b":2} (two values on one line)two values

Desired behavior: For every line verify.sh reads as a Manifest Entry, in the Case manifest and in every member chain of a Shared Case package, the script fails that line as Invalid JSON whenever JSON.parse would throw on it for any reason in the table above. It reports the line where it reports a byte-order mark or NUL byte today, with Invalid JSON in the finding. A line that JSON.parse accepts is never newly rejected: strings that contain these spellings as text (for example a URL or title holding 01 or NaN) still pass. The check must not depend on jq's own number parsing, which has differed between jq releases.

Key interfaces:

  • The shipped script is generated as a template string, VERIFY_SCRIPT. The new check sits beside bom_led and nul_line and runs wherever they run: the Case manifest's step 4 loop and check_chain for member chains (step 4a).
  • The package verifier's verdict is the reference: the chain check that calls JSON.parse and returns broken(i, 'Invalid JSON'). It does not change.
  • The script stays POSIX sh plus the tools it already requires (jq, openssl, the sha256 tool). No new required tool.

Acceptance criteria:

  • In the conformance table of the test that runs both shipped verifiers against one real package, a row for each spelling in the table above, applied to a Case manifest line (with the unsigned index rehashed), shows the package verifier and verify.sh both failing, with verify.sh printing Invalid JSON at step 4.
  • A Shared Case export test rewrites a member chain line's index as 01, and verify.sh reports FAIL [step 4a] <chain> (member <code>): entry 1: Invalid JSON, matching the package verifier's chain-broken ... Invalid JSON.
  • At least one other spelling from the table (NaN or a second value on the line) is covered on the member chain path too.
  • A line whose string values contain 01, +1, .5, NaN and Infinity as text still passes both checkers.
  • Every existing verify.sh, runbook and Shared Case export test passes unmodified, and a package from an unmodified Case still reads PASS.

Verification path:

  • pnpm test tests/main/services/verifyRunbookExecution.test.ts
  • pnpm test tests/main/services/sharedCaseExport.test.ts
  • pnpm preflight, which must be green at the PR head. These tests need jq and openssl. CI requires jq, so the rows cannot be skipped there.

Starting points (as of main 4f2b4387, may move):

  • src/main/services/verifyScript.ts: bom_led and nul_line, check_chain, and the step 4 manifest loop.
  • src/shared/verify/manifestChain.ts: the JSON.parse the verdict must match.
  • tests/main/services/verifyRunbookExecution.test.ts: the conformance table, whose NUL-byte and byte-order-mark rows are the pattern to follow.
  • tests/main/services/sharedCaseExport.test.ts: the member-chain byte-order-mark test is the pattern for the Shared Case row.

Evidence-affecting: yes. verifyScript.ts is on the blocking tier of the evidence path list, and the change alters a verification verdict.

Out of scope:

  • Number spellings both checkers accept but write back differently, such as 1E+5 or -0, where jq keeps the literal and canonical JSON does not. If one of them changes a recomputed Entry Hash, record it in the PR's findings and do not fix it here.
  • JSON files the script reads other than chain lines (export-entry.json, evidence.json). If one shows the same disagreement, record it in the PR's findings.
  • Any change to the package verifier, the runbook prose beyond what names the new check, or the JSON.parse reading rule.