Files
trailofbits__skills/plugins/variant-analysis/tests/README.md
T
Nuno Sabino 4822dc3876 variant-analysis: convert the skill to a dynamic workflow (#232)
* Converted skill into a dynamic workflow. Still working on the tests

* Added gradio test with injected vulns

* Fix grader

* Fix trailing whitespaces

* Bump version number

* Remove trailing whitespaces from a git patch...

* Run pre-commit

* Add claude evals

* Address PR claude review

* variant-analysis: fix problems found by testing #232 before release (#237)

* variant-analysis: fix three workflow defects found in a cold run

Prose args killed the run on the first line. The model wrote
`bug: ...; root: /path; lang: python` instead of an object, and the invocation
died with `args.bug is required` before a single agent started. Parse that
shape, and say in whenToUse that args is a JSON object.

The baseline command was not shell-safe. The pattern went through
JSON.stringify, which looks like quoting but yields a double-quoted string where
$(...) and backticks still expand -- and the pattern is model-generated from
codebase content. The root was not quoted at all, so any path with a space broke
the command. Single-quote both.

The sweep had no size floor. It spawned 25 agents against a 5-file fixture,
re-reading in parallel what one agent holds at once. The eval's own negative
result already said so: five small synthetic codebases showed no difference
between the workflow and the skill alone because the fan-out had nothing to buy.
Below 40 source files, sweep two axes in one round -- 7 agents on the same
fixture. The baseline gate reports the file count, and a single-round sweep is
now reported as the deliberate bound it is rather than as a truncated one.

The report stage now has to emit `**Location:**` fields. Without them the
grader falls through to a permissive path its own docstring calls over-counting,
which is what happened on the cold run: a real report scored through the
fallback and nothing said so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* variant-analysis: score construct spans, not line proximity

A cold run scored a correct report as wrong. The report flagged a helper at
lines 4 and 7 of a file whose safe site began at line 10; LINE_WINDOW=30
credited it as the safe site being reported as real, and the run failed. Two
different functions three lines apart, conflated.

Ground truth now records a `span` per site -- the function's real line range --
and a reported location has to fall inside it. verify_fixtures.py fails if a
span stops containing its own anchor line, so a stale hand-edit cannot
reintroduce the failure silently. LINE_WINDOW drops 30 -> 12 as the fallback for
entries carrying no span.

Line-less mentions now lean opposite ways for recall and precision, and both
directions favour not failing a run that did the work. A report naming the right
file without a line is still credited for recall. It is no longer treated as
claiming the decoy: the decoy's file in the real fixture also holds a genuine
upstream finding, so any run reporting the real one without a line number was
marked as having flagged the decoy.

Three self-tests added, all reduced from the cold run. Both fixes were
mutation-checked: reverting the span logic and reverting require_line each fail
the suite.

Also removeprefix("./") for lstrip("./"), which took a character set and ate the
leading dot of paths like .github/scripts/x.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* variant-analysis: surface loose scoring, plumb --strict-decoy, parse the workflow

summarize.py prints a `loose` column counting runs scored through score.py's
permissive fallback. A score built on it is worth less than one built on
location fields, and that was invisible.

--strict-decoy was documented in the README and implemented in score.py but
unreachable from eval.sh, which exited 2 on the unknown option. Plumbed through.
The usage header also advertised `--codebase go`, left over from the five
synthetic codebases; gradio is the only one, and passing both modes needs
quoting.

run_fixtures.sh now runs `node --check` on the workflow. It is the only
JavaScript in the repo and nothing in CI parses it, so a syntax error would
surface only inside a paid eval.sh run. Skipped, not failed, where node is
absent.

setup-gradio.sh reported "the checkout is not at $SHA" for any failed
apply --check, including a checkout at the right SHA whose patch is already
partly applied -- reachable, since the unpatched probe only looks at one of the
three files. Name both causes and the recovery.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* variant-analysis: drop eval graders no arm can fail, correct the firing claim

The skill-not-fired graders on cases 06-07 set `arm: both`, which makes them
scored, and neither arm can fail them: the baseline arm has no plugin so Skill
never fires, and the with-plugin arm does not fire on these shapes either. The
suite's own guidance says a grader no arm can fail is worth deleting rather than
reweighting. The type: llm grader on each case carries the real check.

The "skill does not fire" limitation was overstated as a property of the skill.
A 9-run cold run across three prompt shapes locates the actual cause: it fires
2/3 on a conversational prompt and 3/3 on the description's trigger language
when there is a codebase on disk, and 0/3 on an inline candidate panel -- which
is the shape of every case in this directory. With nothing to sweep, declining
the skill is arguably correct. Giving these cases files on disk would fix the
saturated delta and the trigger rate at once; that is the highest-value change
left here and it is not a small one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* variant-analysis: describe the trigger that actually fires

The skill description was generic where the measured trigger is specific: a bug
just found in a named file, and the question of where else it occurs. It now
leads with that situation and names the bare conversational form, which is what
fired 2/3 in a cold run. The old description was diagnosed as the reason the
skill never fired; it was not, but it was still vague.

The README's entry-point table claimed the skill is "best for a narrow search
where you want a say in each generalization" and triggers on its own. Measured
on a real codebase, Claude reaches for the workflow in 4 of 5 firing runs and
the skill in 1 of 9 -- so ask for the skill by name if you want to weigh in.
Also records the size floor, and that args is a JSON object.

tests/README.md documents spans, the recall/precision asymmetry on line-less
mentions, and the loose column.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Document dynamic workflow layout; variant-analysis 2.0.1

AGENTS.md described only skills/<skill>/workflows/, the prose step-by-step kind,
so the plugin-root workflows/*.js layout that ships as /<plugin>:<workflow> was
undocumented -- and variant-analysis is the first plugin in the repo to use it.
Names both, says which one a "Phase 1 / for each / repeat until" SKILL.md
belongs in, and records that ${CLAUDE_PLUGIN_ROOT} is unavailable inside a
workflow script.

Version bumped 2.0.0 -> 2.0.1 since these are behavioural changes on top of an
unmerged 2.0.0. Squash it back to 2.0.0 if you would rather ship one version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* variant-analysis: close gaps found by review of the fix PR

A line-less claim on the safe site's file fell into the gap between the strict
accusation check and the permissive examined check: it stayed out of
decoy_reported_as_real (correct -- it names no line), matched `known`
permissively so it dropped out of unreviewed_findings, and then satisfied
decoy_examined_and_ruled_out. A run was credited with correctly ruling out the
site it had just listed under Findings, and passed even under --strict-decoy.
Now surfaced as decoy_claimed_without_line, kept visible in
unreviewed_findings, and it blocks the ruled-out credit without counting as a
false positive.

The small-tree bound could drop expansion axes with no record in the artifact.
With axesPerRound=2 and one round, a 6-axis root cause left four
generalizations unattempted and only the live progress log said so; REPORT.md
was indistinguishable from an exhausted sweep. The report prompt and the return
value now carry swept/total axes and name the unswept ones.

Spans are exact def..return, which left no room for a decorator directly above
a def. RECALL_PAD=3 covers that on the recall side only; the safe site gets no
slack, since padding it walks back into the conflation the spans fixed.

verify_fixtures.py now requires a span on every entry and validates the range.
Without that, a dropped span silently reverted the grader to a proximity window
with a green suite, while ground-truth's own comment documented a guarantee that
no longer held.

source_file_count was `rg --files | wc -l`, which counts assets and fixtures.
A 25-source-file project behind 300 fixtures reported 325 and missed the floor
it was built for. The prompt and the schema now ask for source files only.

`node --check` runs against an .mjs copy. On a .js file whose first statement is
`export`, it only passes on Node ~22.7+, and lint.yml pins no Node version.

Also: pinned the extraction-mode labels as constants with a self-test, so
renaming one cannot leave summarize.py's loose column reading zero forever;
fixed a self-test fixture whose span did not contain its own anchor line, a
shape verify_fixtures.py now rejects; dropped a dead condition in parseArgs;
third-person skill description per AGENTS.md.

score.py self-test 16 -> 18 checks, summarize.py 6 -> 7. The label-rename and
span-removal mutations were both confirmed to fail the suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Fix node check. Claude workflows have syntax like top-level returns that will trip the linter

* Add trailing newline

---------

Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com>
Co-authored-by: Clinton Thomas <1033162+KernelClint@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 09:13:41 -04:00

6.9 KiB

Variant-analysis eval

One large real codebase — gradio at a pinned commit — with two lookalike command-injection vulnerabilities and one decoy injected by patch.

The hunt is seeded with the first vulnerability only. Finding the second is what the eval measures; the decoy measures whether precision survives the search that finds it.

Role Location What it is
Seed gradio/processing_utils.py:1110 extract_video_thumbnail() interpolates an uploaded video path into an ffmpeg command string, subprocess.run(..., shell=True)
Variant gradio/flagging.py:351 archive_flagged_media() interpolates a flagged-sample label into os.system()
Decoy gradio/screen_recording_utils.py:14 probe_recording_duration() — same subprocess.run, same kind of user-controlled media path, argv list form with no shell

The variant is deliberately hard to reach from the seed: different module, different sink API. Grepping for the seed's exact sink (subprocess.run / shell=True) never finds it. Reaching it requires generalizing to "user data reaches a shell" — which is the abstraction ladder the skill teaches and the axis fan-out the workflow parallelizes.

The hypothesis under test: the workflow finds the variant and still rules out the decoy; the skill alone finds only the seed.

Running

./setup-gradio.sh              # fetch + patch (~283 MB, one time)
./run_fixtures.sh              # free, offline — verifies the fixture, grader, and workflow syntax
./eval.sh                      # calls Claude: both modes
./eval.sh --mode workflow      # skip the baseline
./eval.sh --runs 3             # variance estimate
./eval.sh --strict-decoy       # also require the decoy in the ruled-out section
./eval.sh --keep               # keep the work dir even on a pass
./setup-gradio.sh --clean      # delete the checkout

A passing eval.sh deletes its own work dir; a failing one keeps it, because that is when the transcripts are worth reading. --keep keeps it either way, and a dir passed with --out is never deleted. --mode accepts only workflow and baseline: a typo is an error rather than a silent fallback to the baseline prompt, and a run with no workflow arm is reported as measuring nothing instead of passing green.

The codebase is not vendored — 772 Python files at 283 MB, which is the point. Only gradio-vulns.patch and ground-truth.json live in this repo; work/ is gitignored.

eval.sh runs Claude inside the checkout rather than a per-run copy, and writes the report to an absolute path outside it. Copying the tree twice per run would cost more than the eval. After every run the fixture is re-verified, so an agent that edits the code it was asked to audit fails the sweep instead of silently changing what later runs are graded against.

What "pass" means

The workflow mode must:

  1. find every unseeded variant (new_variants_found == new_variants_total),
  2. not report the decoy as real, and
  3. score at least as well as the baseline (skill only, no workflow).

--strict-decoy additionally requires the decoy to appear in the report's ruled-out section — evidence the sweep was broad enough to surface it and triage sharp enough to reject it, rather than the search simply never reaching it.

Scoring keys on new variants rather than total true positives on purpose. The seed is handed to the run, and real reports differ on where it goes: some list it under ## Findings, others under ## Original Vulnerability per the template. Both are correct, so counting the total measured formatting rather than detection.

Design notes

The SHA is pinned. Ground truth records absolute line numbers. Track main and the patch stops applying, the line numbers drift, and the eval grades against the wrong code. verify_fixtures.py fails if the checkout has moved.

The grader reads the artifact, not the transcript. A run that describes finding two variants but writes no report exits 3 (ungradeable), not 0 true positives. Conflating those is how an eval starts reporting success for runs that did nothing.

A finding is a **Location:** declaration inside a non-refuted block. Scraping every path in the Findings section over-counts badly — it picks up entry points named while tracing data flow, and rows in triage tables the report itself refuted. Both mistakes were observed against real reports before this was fixed.

The workflow's report stage is instructed to emit those fields, because the fallback is what runs when it does not. score.py reports which path it used as extraction_mode, and summarize.py prints a loose column counting the runs that fell back — a score built on the permissive path is worth less than one built on location fields, and that used to be invisible.

Ground truth carries a span, not just a line. A construct is a line range — the function it lives in. Matching on proximity to a single line conflates neighbours: a cold run reported a helper at lines 4 and 7 of a file whose safe site began at line 10, and a 30-line window scored it as the safe site being reported as real. Spans are exact, and verify_fixtures.py fails if a span stops containing its own anchor line.

Line-less mentions lean opposite ways for recall and precision. A report naming the right file without a line is credited for recall, but is not treated as claiming the decoy. Both directions favour not failing a run that did the work — and the strict half matters because the decoy's file also holds a genuine upstream finding, so any run that reported the real one without a line number used to be marked as having flagged the decoy.

run_fixtures.sh is CI-safe; eval.sh is not. make shell-suites executes every plugins/*/tests/*/run_*.sh it finds, so the expensive half is deliberately named outside that pattern. run_fixtures.sh also exits 0 with a notice when the codebase has not been fetched, so CI passes on a machine that never ran setup.

Caveats

  • This is one test. Five smaller synthetic codebases were tried first and showed no difference between modes — they were small enough that a single context handled them, so the fan-out had nothing to buy. That is why the eval moved to one large real codebase. One test is thin, but it is testing something the previous five were not.
  • The vulnerabilities are injected, not real. They are written to sit plausibly beside existing code (gradio already shells out to ffmpeg), but a hunt is still looking for something planted rather than something that arose naturally.
  • Results vary between runs. These are LLM runs, not deterministic tests. Use --runs 3 or more before drawing a conclusion, and treat a single run as a smoke test.
  • The baseline still has the skill and its references. The comparison is workflow orchestration versus the same knowledge without it, which is the intended contrast.