benchmark: add OWASP starter corpus and fix EXAMPLE.json glob bug

[visit2rahul]

Fixes #24, closes #23.

Summary

• EXAMPLE.json was included in the *.json glob used by load_all_benchmarks(). Its placeholder commit hashes do not exist on GitHub, so python -m local_harness.benchmark.run always failed at the clone phase on a fresh checkout. Renamed to EXAMPLE.json.template so it is excluded from the glob.
• Added _validate_benchmarks() in run.py to catch missing required fields, duplicate finding_id values, and malformed source_code URLs before any network I/O -- replacing cryptic KeyError/IndexError crashes with clear upfront errors.
• Added a real starter corpus of 6 verified findings across 3 OWASP intentionally-vulnerable apps, each pinned to a specific commit hash with exact file and line references. The benchmark now runs end-to-end out of the box.

Corpus added

File	Findings
juice-shop.json	SQLInjection (routes/login.ts:34), DOMXss (search-result.component.ts:143)
WebGoat.json	PathTraversal via fullName field (ProfileUploadBase.java:51), Zip Slip (ProfileZipSlip.java)
NodeGoat.json	NoSQLInjection via $where clause (allocations-dao.js:78-79), InsecureDirectObjectReference (allocations.js:16)

Test plan

• python -m local_harness.benchmark.run --scan-only on a fresh checkout no longer fails at clone phase
• Introducing a finding with a missing field or bad source_code URL prints a clear error and exits before cloning
• EXAMPLE.json.template is not loaded by the benchmark runner

[CLAassistant]

All committers have signed the CLA.

[schenksj]

Thanks for this — the corpus research is genuinely good work. I verified the findings independently and they hold up: all three pinned commits exist, and the line references are accurate (juice-shop routes/login.ts:34 is exactly the unparameterized sequelize.query template literal; NodeGoat allocations-dao.js:78-79 is exactly the $where interpolation, with the commented-out parseInt remediation directly above it as described). The descriptions are specific and not hand-waved.

Two blocking issues before this can merge, though.

1. The validator rejects this PR's own corpus

The test-plan boxes are unchecked, and python -m local_harness.benchmark.run doesn't get off the ground on this branch:

Error: benchmark corpus validation failed:
  WebGoat.json[0]: duplicate finding_id 'VULN-001' (also in NodeGoat.json)
  WebGoat.json[1]: duplicate finding_id 'VULN-002' (also in NodeGoat.json)
  juice-shop.json[0]: duplicate finding_id 'VULN-001' (also in NodeGoat.json)
  juice-shop.json[1]: duplicate finding_id 'VULN-002' (also in NodeGoat.json)

sys.exit(1). _validate_benchmarks() tracks seen_ids globally across files, but all three new corpus files number from VULN-001. The benchmark fails earlier than before — during validation instead of clone.

To be clear on which side is wrong: the per-file numbering is right. Real reports start at 001 and sequence monotonically, and the corpus should keep that. The uniqueness check should be scoped per file.

2. Scoping the validator per-file exposes a silent scoring bug (pre-existing)

This is the more serious one. state["judgments"] is a flat dict keyed by bare finding_id (run.py:236, 286, 315, 335), spanning every repo in a run. finding_history.py:40-52 keys history the same way.

That keying is not new — it's identical on the base commit and unchanged by this PR. It's dormant on main only because main ships a single corpus file whose IDs happen to be globally sequential across repos (VULN-001 NodeGoat / VULN-002 juice-shop / VULN-003 WebGoat). This PR is the first to ship multiple files and the first to restart numbering per file, which arms it.

With the validator relaxed to per-file scoping, the corpus loads — and then:

REAL benchmark findings in corpus: 6
TALLY reports:                     2
detection_rate denominator:        2

surviving rows:
  VULN-001 SQLInjection    juice-shop
  VULN-002 DOMXss          juice-shop

All four WebGoat and NodeGoat findings are silently overwritten by juice-shop's, and the detection rate is computed over 2 instead of 6. A loud exit(1) is much safer than this — it quietly corrupts the number the harness exists to produce.

(Verified by driving the real corpus through deduplicate_targets(), writing judgments with the same keying run.py uses, then calling the real generate_tally().)

Fix is internal namespacing, not renaming findings. Judgments already carry benchmark_file and scan_target, so a composite key is small. Prototyped:

TALLY total: 6 (expected 6)
  NodeGoat.json    VULN-001 NoSQLInjection
  WebGoat.json     VULN-001 PathTraversal
  juice-shop.json  VULN-001 SQLInjection   ...

Sites: the five judgment read/write points in run.py, finding_history.py:40-57, and tally.py:49 (it currently uses the map key as the display finding_id, so judgments need to carry finding_id explicitly). Any existing finding_history.json / state file is keyed by bare ID and should be invalidated, or history will smear across repos.

Smaller items

• _validate_benchmarks calls finding.keys() with no dict check, so a JSON array of strings raises AttributeError instead of the clean error the function exists to provide.
• Please rebase onto main. This branch predates ci: run all project tests on every PR via GitHub Actions #27, has no .github/workflows/, and no CI run has ever executed against it — so nothing here has been checked automatically.
• The README schema table says finding_id is a "Unique label" without qualifying the scope. The code requires globally unique; the natural convention is per-report. Worth stating explicitly so the next contributor doesn't rediscover this.

Test coverage

No tests are added, and the existing suite can't catch any of this: every case in test_benchmark_run.py monkeypatches BENCHMARK_DIR to a tmp fixture, so the real shipped corpus is never loaded. The fixture also uses globally-unique F1/F2/F3 across its two files, which is exactly why the collision stayed invisible.

All of this is checkable offline — validation runs before any network I/O and _validate_benchmarks is pure, so no clone and no judge/LLM is needed. I wrote a tests/test_corpus_integrity.py covering the real corpus plus the validator branches; it runs in 0.05s with no network and no model, and goes RED on exactly the three defects above. Happy to hand it over if useful.

@emeth- — you know this area far better than I do, particularly the judgment/history keying and whether the composite-key change should ride along here or land as its own PR ahead of it. Could you take a look? The ordering matters: if the corpus merges before the keying fix, the benchmark silently reports 2 of 6.

[emeth-]

Thanks @schenksj - agree with most everything you said, just adding one piece of intent that isn't visible from the code.

finding_id is globally unique by design - it's meant to be a stable, CVE-like handle, not a per-report line number, which is why --findings <id> is well-defined. The original implementation of this tool assumes globally-unique IDs as well. So on # 1, the three files restarting at 001 are what's out of step, not the check - I'd suggest tapping into your composite key concept encoded into the corpus instead of the code (e.g. NODEGOAT-001/002, WEBGOAT-001/002, JUICE-001/002) and keep the validator global rather than scope it per-file.

On # 2, your analysis is why I'd avoid the per-file relaxation. The flat keying is only safe while IDs are unique, and the validator is what enforces that - so I'd keep the guardrail, renumber the six findings, and #2 can't fire. No keying change needed for correctness, and it unblocks the PR. The full composite key proposal is reasonable defense-in-depth, but I'd land it as its own PR rather than a prerequisite; the ordering risk you flagged goes away once the corpus is globally numbered.

Couple of smaller things: the README should probably state the scope outright ("globally unique across the whole corpus"). And the test_corpus_integrity.py you offered is a good idea, just pointed at global uniqueness instead of per-file. The findings.keys() guard and the rebase/CI note both look right to me.

[visit2rahul]

Thank you @schenksj and @emeth-.

Went with @emeth-'s direction -- prefixed all six IDs (NODEGOAT-001/002, WEBGOAT-001/002, JUICE-001/002) and left the validator global. Cleaner than touching the keying code, and the duplicate check now enforces it.

Also added the isinstance guard, updated the README to say "globally unique across the entire corpus" with a note on the prefixing convention, and added tests/test_corpus_integrity.py with coverage on the real corpus and the validator edge cases. Rebased onto main so CI runs.

Please review as your time permits.