mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
frabert/diataxis-plugin
164 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Francesco Bertolaccini
|
555dbd6068 |
feat(diataxis): add Diátaxis documentation plugin with eval suite
Adds the `diataxis` plugin: a skill and an eleven-agent dynamic workflow that
generates a four-quadrant documentation set, with the reference quadrant
produced by a real generator (Sphinx, Doxygen, rustdoc, TypeDoc) rather than
hand-written beside the code.
Also adds an eval suite under `evals/` — four fire cases and two should-NOT-fire
cases, run with `--ablation with-without` to measure uplift over an unaided
model. Full run at runs=3: mean Δ +0.26.
The suite establishes two things worth recording:
- When the skill fires, it delivers. `reference-generator-scaffolded` and
`doc-comments-in-source` pass on every fired run and on none of the nine
baseline runs; all four quadrants land in 9/9 fired runs.
- The trigger is unreliable. The parameterized form ("Document the parser
package and put the output in website/content") fired 0/3 despite being the
usage the README advertises, and the explicit form fired 2/3. Prompts that
state a need rather than name the framework fired 3/3.
Fire-case prompts explicitly authorize workflows and subagents, because eval
agents otherwise decline to use them and the plugin's only real path never
runs. Eval run output is gitignored; the cases are tracked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paweł Płatek
|
07bce8a2c8 |
Add goal-prompt plugin for copy-ready /goal commands (#248)
* Add goal-prompt plugin for copy-ready Codex /goal commands Migrated from trailofbits/codex-skills#6, converted from the Codex sidecar layout (.codex-plugin/, .agents/marketplace.json) to this repo's canonical Claude plugin structure, which Codex loads through marketplace compatibility. Both loadability checks pass. The skill drafts a goal-mode objective and pipes it through a deterministic stdlib-only formatter that collapses whitespace to one line and rejects output over the 4,000-character /goal cap, with a pytest suite covering normalization and both failure modes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add goal-prompt plugin for copy-ready /goal commands Migrated from trailofbits/codex-skills#6, converted from the Codex sidecar layout (.codex-plugin/, .agents/marketplace.json) to this repo's canonical Claude plugin structure, which Codex loads through marketplace compatibility. Works with goal mode in both Claude Code and Codex; both loadability checks pass. The skill drafts a goal-mode objective and pipes it through a deterministic stdlib-only formatter that collapses whitespace to one line and rejects output over the 4,000-character /goal cap, with a pytest suite covering normalization and both failure modes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: encode goal-mode limits and termination-contract guidance Researched both harnesses' official docs. Claude Code's /goal condition and Codex's stored objective are each capped at 4,000 characters, and Claude Code's evaluator is a transcript-only small model that cannot run tools — so a condition is judgeable only when the agent runs the check and shows the output. SKILL.md now separates the plugin's two jobs: draft a termination contract (end state not activity, stated check with transcript-visible proof, invariants including never weakening the gate, stop bound or blocked clause, AND not "or"), then format it. Platform mechanics with sources live in references/goal-mode.md. The formatter gains one deterministic non-fatal check: it warns when the objective has no numeric stop bound and no blocked clause, the documented top failure mode for goal loops. Tests cover the new detection both ways. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: fold in Trail of Bits field guidance from codex-config The codex-config README's /goal section adds three things the plugin was missing. The drafting checklist gains the work-order fields it lacked: scope to read first, and for multi-checkpoint goals a final evidence deliverable plus a progress log file for durable state. A when-to-use heuristic (an instruction repeated three turns in a row belongs in the goal; chain small goals rather than one giant one). And a security-research section hardening audit goals against reward hacking: neutral wording, threat-model scoping, demonstrated attacker preconditions, known-findings checks, per-finding human review, and second-pass validation. references/goal-mode.md gains the full work-order template, the codex exec caveat, and the missing official links. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: tighten SKILL.md and README.md Same content, less prose: SKILL.md drops the meta-commentary and keeps the checklist, security hardening, formatter contract, and example; README.md explains the two jobs in two paragraphs for a human reader. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: never invent missing goal elements Every checklist element must be grounded in the user's request, the conversation, or the repository (look up the real check command, don't guess one). When nothing grounds an element, the skill still optimizes and formats what the user provided, but reports the gap in a Missing: list after the fenced block instead of fabricating a success condition that would terminate the goal on the wrong contract. The example now shows both the grounded case and the flag-the-gaps case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: fold goal-mode reference into SKILL.md The reference file mostly restated the checklist. What survives into SKILL.md: the official doc links, the Codex feature flag and interactive-only caveat, and the Claude Code resume caveat (turn bounds silently extend across resumes), placed next to the stop-bound rule it affects. Everything else was lifecycle and mechanics detail the drafting job does not need. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: close easy-outs, keep goals small Two additions that pull in opposite directions, stated together so neither wins by default. Before formatting, reread the condition as a lazy model would and close the cheapest ways to satisfy the letter without the intent: delete-or-stub, pass-on-a-subset, game-the-gate, claim-without-running. But every constraint narrows the state space the model can explore, so prefer pairing existing checks over adding constraints, collapse to one terminating criterion when possible, and drop non-goals. Outs that cannot be closed from grounded information go in the Missing: list as warnings, never as invented constraints. The security section now leads with the collapsed pattern: one criterion referencing a THREATMODEL.md that carries scope, attacker powers, severity baseline, and known findings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: strip links and config detail from SKILL.md Skill bodies are for the drafting agent: the doc links, the Codex feature-flag and codex-exec caveats, and the fenced security-goal example added context without changing behavior. The never-invent rule loses its check-command specificity. The security section keeps only what changes the drafted text: one criterion, scoping file, neutral wording, demonstrated preconditions, per-finding review, second-pass validation. The Example section stays — Anthropic's authoring checklist calls for concrete input/output examples, and this skill's output shape is the point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: replace README wall-of-prose with a capability list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: set Paweł Płatek as author, trim CODEOWNERS Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: add with/without ablation evals Three cases, each the same bare "Improve this goal: ..." prompt run with and without the plugin so the score delta isolates what the skill adds: grounded-migration (single-line output, check command grounded in the fixture's package.json, stop clause), ungrounded-vague (no invented metrics or benchmark commands when nothing grounds "faster"; gaps flagged back), easy-out-closed (the user's grep-only success check is deletable-code-satisfiable; the goal must pair it with the fixture's real test suite). Graders judge the returned artifact against fixture contents: regex for the mechanical stop clause, LLM graders for grounding, invention, and easy-out closure. Also adds "improve" to the skill's trigger list since the eval prompts (and users) phrase it that way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: fix eval graders to judge only the /goal line Live ablation runs exposed the grader bug AGENTS.md warns about: the haiku judges failed correct answers because the Missing: list's illustrative examples ("e.g. p95 under 300ms") were read as inventions and as extra command candidates. Graders now scope judgment to the single line inside the fenced block and explicitly exempt the gap list; the README pins --judge-model sonnet since haiku cannot follow that scoping. Measured results (4 runs/arm/case): plugin arm 100% across all graders; bare arm bimodal — the deterministic stop-clause regex alone failed half its grounded-migration runs. On strong models the plugin's demonstrated value is consistency, recorded as such in the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: record baseline contamination in eval results Checked our eval logs against trailofbits/skills-internal#546 and we were hit: baseline-arm responses cite the fixture's absolute repo path and reproduce SKILL.md sentences verbatim ("scope to read first", "terminates on the wrong contract"), so the no-plugin arm read the plugin under test off disk and imitated it. Our runs were more exposed than the issue's report — the baseline had full Bash, not just ungated Read/Glob. The README now marks measured deltas as lower bounds, notes the uncontaminated baselines scored 0, and prescribes --keep-temp plus a leakage audit of baseline traces until the harness can deny the baseline Read access to the plugin directory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * goal-prompt: isolate eval fixtures from the repo, gate on contamination The ablation baseline could read the skill under test: add_dirs handed every agent an absolute path into this repo, one directory walk from SKILL.md and the graders, and baseline responses reproduced SKILL.md sentences verbatim. Contaminated baselines imitate the skill, deflating the delta to near zero. Fixtures are now generated inside the eval's temp scaffold by each case's scaffold.sh (run with --scaffold), so no agent sees a repo path; a kept-temp probe confirmed the scaffold gets its own home/, config/, and cwd/ with zero repo paths in the baseline trace. check_contamination.py fails a run whose baseline responses contain the plugin path, script name, or verbatim SKILL.md phrases, and fails when it has nothing to inspect; its pytest suite proves both directions. Clean rerun: plugin arm 1.00 everywhere; baselines 0.40/0.29/0.25; mean delta +0.69 (was +0.04 contaminated). The checker flags the old contaminated result and passes the new one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix owner * goal-prompt: run scripts through uv, fix the dead contamination marker Two fixes on top of #248. The skill's only operative command was `python3 {baseDir}/scripts/ format_goal_prompt.py`. The modern-python plugin in this same marketplace ships PATH shims that reject a bare `python3 <script>`, so the Format step failed for anyone who has it installed — and #255's narrowing does not help, because a bare script run is exactly what `uv run` replaces and stays intercepted by design. Now `uv run --no-project`, matching the form the Makefile already uses in all four of its invocations. Verified with the shim on PATH: byte-for-byte identical output to the old command run shim-free. Same fix in evals/README.md. The `scope to read first` contamination marker could never fire: SKILL.md writes `**Scope to read first**` and the match was case-sensitive. Every existing test quoted the marker's own lowercase spelling rather than the file's, so 22 tests passed over a dead marker. Matching is now case-folded, the markers are split into path and phrase groups, and two tests guard the recurrence — one asserts every phrase marker is still present in SKILL.md, the other quotes SKILL.md verbatim. Both were mutation-tested: reverting either fix fails exactly one of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
Daniel McCarney
|
4db56eff30 |
trailmark: update vector-forge skill w/ wycheproof tooling (#251)
* trailmark: update vector-forge skill w/ wycheproof tooling We landed some helpful tooling for adding/editing/replacing vector data upstream in Wycheproof that is helpful for LLMs to know about. This commit updates stale references to the Python based tooling and points to the new `vectorgen` tool/docs. * version 0.10.0 -> 0.10.1 --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
Dan Guido
|
488d37c71c |
Collapse the review into one xhigh opus tier, and stop invented evidence (#256)
* Pin the review model, raise effort to xhigh, stop invented evidence Three problems with the automated review, found while auditing what was actually reviewing PRs #253-#255. **The model was never specified.** `claude_review.sh` passed `--effort` but no `--model`, so the reviewer was whatever the CLI happened to default to at run time, and nothing in the logs said which. The CLI version was pinned to stop CI changing without a commit while the larger lever on review quality was left floating. Both tiers now pin `opus`, and the run line records model, effort and resolved CLI version. **The CLI is now deliberately unpinned**, in both this workflow and the loadability check, so they track releases as they ship. For loadability that is the point: the version worth proving plugins load against is the one users run. The logged version is what makes a surprising result attributable after the fact. **The review claimed to run things it cannot run.** Its allowlist is `gh pr` reads plus Read, Grep and Glob — no interpreter, no test runner. Across three PRs it reported a Python snippet it had "verified directly" whose regex cannot compile, a pytest suite it had "run locally" with a pass count that does not match reality, and a shell script three "independent verifiers" had supposedly executed. Every conclusion was correct and every proof was fabricated. The deep prompt already said "you cannot execute anything"; the prompt that actually runs never did. That statement moves into the shared prompt, extended to forbid reporting output no tool produced. Effort goes low -> xhigh on the tier that runs on every push, so the strongest review is the default rather than something to remember to ask for. The shared prompt also now says to rank on consequence rather than diff size, after a one-character fault that made a checker miss its own target was filed as a nit. The `fast` name is kept: it is the check name branch protection matches on, and it describes the trigger rather than the effort. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Collapse the two review tiers into one The split was a cheap pass on every push plus an xhigh adversarial pass behind a `deep-review` label. It did not survive contact: the label was never created, so the deep tier skipped 9 times and ran zero, and every review this repository has ever received came from the cheap tier. With both tiers now on opus at xhigh, the only thing left separating them was scope, and there was no reason to gate the better scope behind a label somebody has to remember to apply. So there is one job. It takes the deep tier's prompt and tools — reads beyond the diff, gets git history, checks the PR's claims against the files — and the cheap tier's `--edit-last` posting, which matters now that it runs on every push. `fast` is gone from the check name, the script's tier argument, and the concurrency key. The name was already inaccurate once effort went to xhigh, and the ruleset on main requires license/cla, Validate, Pre-commit and bats — not this check — so nothing depended on it. The elaborate label-keyed concurrency group goes too; it existed only to stop a push cancelling an in-flight deep review, and there is no second tier to collide with. Timeout follows the deep tier at 30 minutes, since xhigh on a large diff needs the room. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Address the review of this PR The new xhigh reviewer found real problems in its own configuration. **The API key was in the unpinned install step's environment.** It was there only to satisfy `if: env.ANTHROPIC_API_KEY != ''`, since `secrets` is not available in a step-level `if:`. Combined with `@latest`, that runs npm lifecycle scripts from a version nobody chose beside an org-wide credential. Presence is now reduced to a boolean in its own step, and the install step holds no secret at all — which is what makes @latest acceptable there. **The posting guard counted comments it did not write.** It matched any issue comment in the window, so a maintainer replying to the previous review could stand in for a review this run never posted: green check, no review. Measured on this PR's siblings — the old query counts 2 on #253 and #255, the new bot-only query counts 1. Also paginated, since the unfiltered call would miss a review past the first 30 comments. **`AGENTS.md` reaches the reviewer and tells it to run things.** CLAUDE.md is just `@AGENTS.md`, so it loads as project instructions telling the model to run `make check`, run `prek run -a`, and consult a `claude-code-guide` subagent — none of which it can do. That is the fabrication channel this PR exists to close, arriving by a route the prompt did not address. The prompt now names those files and says they are addressed to someone else. **validate.yml goes back to a pinned CLI.** Unpinning it was my extension, not what was asked, and the risk is misplaced: that job is a required check, so an upstream release renaming a field in `plugin list --json` reddens every open PR at once and blocks merges with no commit to explain it. The review job can go red harmlessly. This also re-aligns Codex and Claude, both pinned there again, and keeps dependabot.yml's note about "the pinned npm CLI versions" accurate. Smaller: the prompt described its allowlist as "only `gh pr` reads" while mandating `gh pr comment`, a write, and granting `git log`/`git diff`. And the comment over `MODEL` claimed an attributability the `opus` alias does not provide, since it tracks new Opus releases by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Unpin the Claude Code CLI on the loadability check too Reverses the revert two commits back, deliberately and with the argument against it on the record. The review made the case for pinning here: `Validate plugins and skills` is a required check, so a Claude Code release that breaks plugin loading reddens every open PR at once with no commit to explain it. That reading is correct about the mechanics and wrong about which failure costs more. A pin nobody remembers to bump drifts until CI is proving loadability against a version no user runs, which is the one thing this check exists to establish — and it fails silently, by passing. The loud break is the preferable failure, and it is now the chosen one rather than an oversight. Dependabot does not track npm CLIs installed this way, so the real choice was a live version or a stale one, never a maintained one. The Codex CLI beside it stays pinned at 0.146.0. Same class of manual pin and arguably the same argument applies, but unpinning another vendor's CLI was not asked for and would widen this change past its subject. dependabot.yml's note is corrected to match: singular, and naming which one is pinned and why the other is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Dan Guido
|
c199e0cc7d |
Narrow the modern-python shims to the commands uv run replaces (#255)
* Narrow the modern-python shims to the commands uv run replaces Closes #207. The shims sit on PATH, so they intercept every subprocess any tool spawns, not just what Claude types. Two of the intercepted invocations were not package management at all, and blocking them broke real tooling. `uv pip` now passes through when it carries --project, --directory or --target. Those say a tool is building an environment it owns, where `uv add` is not the available advice: prek installs every hook with `uv pip install --project / --directory <cache>`, so the refusal made `git commit` fail in any repo whose hooks need a Python environment. A bare `uv pip install requests` is still refused. `python -c`, `python -m <module>` and `python -` now reach the real interpreter. None of them resolves a script against a project's dependencies, which is what `uv run` exists to do, and `uv run python3 -` is not a drop-in replacement inside a pipeline. `python -m pip` stays intercepted, as do bare `python` and `python script.py`. Passing anything through is new for the python shim, which previously ended every branch in exit 1, so it gains the same skip-my-own-dir PATH walk the uv shim already had. That walk now uses parameter expansion rather than basename, because the one case where it must report failure is a PATH holding nothing but the shim, where shelling out to coreutils fails first with a confusing error. Verified by A/B on the two symptoms #207 reports, running each suite against the old shim and the new one: - zeroize-audit's rust-regression smoke test: FAILED at line 72 before, "Rust regression smoke checks passed." after. - prek hook installation from a cold cache: refused before, "check json Passed" after. bats goes from 19 cases to 38. Five python cases inverted rather than being deleted: the ones asserting that -c and -m are refused now assert they run. AGENTS.md's note on `make shell-suites` is corrected rather than removed — the #207 interceptions are gone, but the target still fails because variant-analysis invokes `python3 <script>.py`, which the shim intercepts by design. That one belongs to variant-analysis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Decide on the mode selector, not on argument position Two gaps in the narrowing, both from review. `uv pip install --help` documents `-t, --target <TARGET>`, so the short form has to be exempt alongside the long one. Without it the same tool-managed install was allowed or refused depending on spelling. The python shim read only $1 to find the mode selector, so `python -u -c 'code'` was refused while `python -c 'code'` ran, even though they are the same invocation. It now steps over interpreter flags to find the selector, giving `-W`, `-X` and `--check-hash-based-pycs` the two slots they take. `-u -m pip` is still refused, and so is `-u script.py`: a script path is what `uv run` replaces regardless of what precedes it. bats 38 -> 43. Both #207 regressions re-verified after the restructure: zeroize-audit's smoke test passes and prek installs hooks from a cold cache. Not fixed here, deliberately: `uv --no-progress pip install requests` still slips past the refusal, because the subcommand check reads $1 as well. Parsing that correctly means knowing which uv global flags take a value, and getting it wrong would refuse a command that works today. The failure mode is a missed nudge rather than a breakage — the real uv runs and behaves correctly — so it does not belong in a change whose purpose is to refuse less. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Dan Guido
|
d537432501 |
Pin the node test reporter in semgrep-rule-variant-creator (#254)
`node_test()` ran `node --test` with no `--test-reporter` and then read `# fail 0` and `# pass N` out of the output. Those are TAP markers, and node's default reporter is not a stable contract: node 22 emits tap when stdout is not a TTY, node 23+ emits spec, which prints `ℹ pass N`. CI pins node 22 so it stayed green, while `make check` failed 28 tests for anyone on a current node. The suites themselves were always fine. Pinning `--test-reporter=tap` fixes it. Confirmed load-bearing by reverting the flag: 28 failed, 54 passed, exactly the reported symptom, and 82 passed with it back. While here, `test_node_suites_pass` asserted `"# fail 0" in output` immediately after `assert code == 0`, which only restates the exit code. Replaced with a floor on the reported pass count, because the failure it could not see is a suite that stopped running its tests and passed anyway. Node makes that case subtle: a file containing no tests still reports `# tests 1 # pass 1`, counting the file itself, so a plain non-zero check would not have caught it either. Verified against a gutted suite, which reports 1 against a floor of 10. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Dan Guido
|
f3c8d2a73f |
Back three AGENTS.md enforcement claims with validator checks (#253)
* Back three AGENTS.md enforcement claims with validator checks AGENTS.md lists these under "What the validator enforces, so you do not have to", the heading that tells contributors and Claude to skip checking by hand. The validator did not perform any of them. - Hardcoded `/Users/…` and `/home/…` paths: moved out of the CI workflow and into find_hardcoded_paths(), so `make check` and pre-commit cover it too. Python's re has lookbehind natively, which drops the `grep -P` dependency BSD grep cannot satisfy. Same file types, same `*-shim.bats` exemption and `/path/to` and `/home/vscode` placeholders as before, and the scan carries the anti-vacuity guard across: zero files scanned is a hard failure, not a clean result. - Command files: validate_agent_frontmatter() only ever opened agent and skill files, so the "and commands" half of the allowed-tools claim was unbacked. Renamed to validate_tools_frontmatter() now that it covers all three. - subagent_type: the check returned early for a plugin with no agents/ dir and otherwise only flagged a bare name matching that plugin's own agent. It now resolves against a repo-wide agent registry, so a bare name borrowed from another plugin reports that plugin's namespace, and one that names no agent at all is reported as a dispatch that fails at runtime. Also documents three checks the validator already enforced but AGENTS.md never listed: plugin.json name matching the directory, marketplace source and description parity, and dependabot lockfiles. All three flag zero violations against the repo as it stands, verified by planting each defect and confirming the validator rejects it. Self-test goes from 34 assertions to 43, and SELF_TEST_MINIMUM is now the exact count rather than a loose floor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Catch lowercase home directories in the hardcoded-path check The pattern inherited from the CI step matched `/home/[a-z]` but `/Users/[A-Z]`, so it only caught macOS paths whose account name starts with a capital. Account short names are lowercase by convention, which means the common form went undetected: `/Users/alice/...`, and this repo's own `/Users/user/cc/skills`, all passed clean. The check was missing the thing it exists to find, and my mutation test did not catch that because I happened to plant `/Users/Someone` with a capital S — the one spelling the pattern could see. Both branches now accept either case. Widening turned up exactly one new match across the repo, and it is a false positive: c-review's SKILL.md uses "/Users/me/My Repo" to show that a path containing a space has to stay quoted. That and `/Users/Shared`, a real macOS system directory, join the placeholder list. Self-test goes 43 -> 45: the lowercase path must be caught, and `/Users/Shared` must not be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dependabot[bot]
|
2bb398210d |
deps: Bump ruff (#252)
Bumps the python-minor-patch group with 1 update in the /plugins/trailmark/skills/slicing-code-context/scripts directory: [ruff](https://github.com/astral-sh/ruff). Updates `ruff` from 0.16.1 to 0.16.2 - [Release notes](https://github.com/astral-sh/ruff/releases) - [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md) - [Commits](https://github.com/astral-sh/ruff/compare/0.16.1...0.16.2) --- updated-dependencies: - dependency-name: ruff dependency-version: 0.16.2 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: python-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
kz-tob
|
3e433ffee0 |
Exclude supply-chain eval fixtures from Dependabot scans (#245)
* Exclude supply-chain eval fixtures from Dependabot scans The supply-chain-risk-auditor evals assert on deliberately stale manifests — requests==2.19.0, flask==1.0.2, axios@0.21.0 are the findings the cases expect. A Dependabot bump would leave the cases passing with nothing left to detect. No block scans them today: the uv directories are listed explicitly, and the Cargo.toml and package.json fixtures have no matching ecosystem entry. This guards against a later edit widening that list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Update dependabot.yml added the slash * Update dependabot.yml removing slash because dependabot breaks with it --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Dan Guido
|
a551f0b5f7 |
Revise links in README 'Also see' section
Updated the 'Also see' section with new links. |
||
|
Graham Sutherland
|
04b241176f |
Rework property-based-testing skill and add eval suites. (#235)
* Rework property-based-testing skill and add eval suites. * Fix shellcheck SC2015 and exclude eval fixtures from CI pytest * Run eval self-tests under uv so `make check` survives the python3 shim `eval-self-tests` discovers harnesses repo-wide, and all three it finds today pipe a script to `python3`: property-based-testing's run.sh and effectiveness.sh, and writing-lean-proofs' run.sh. On any machine with the modern-python plugin installed, its `python3` shim rejects those calls and `make check` fails for reasons that have nothing to do with the code under test. That is the trap #207 documents, and the reason `shell-suites` is deliberately kept out of `check`. Excluding a second target is the wrong answer here — these self-tests are free and they are what makes an eval result trustworthy — so run them under `uv run --no-project`, which puts a real interpreter ahead of the shim on PATH. Harnesses should still call uv themselves. The wrapper only applies inside `make check`, and the real sweeps are invoked by hand. * Move evals-extra out of the skill directory to the plugin root `evals/` already sits at the plugin root; `evals-extra/` sat inside `skills/property-based-testing/`, so every user of the plugin shipped 900 lines of bash, a `requirements.txt` naming hypothesis, and a fixture whose tests are broken on purpose — inside the one directory the model reads guidance from. run.sh's own header notes the hazard of the model finding skill files by filesystem exploration; this removes the material it would find. Nothing in the machinery cares which of the two locations it is in: the Makefile, the CI pytest exclusion, ruff's per-file ignores and the plugin validator all match on an `evals*` prefix anywhere under `plugins/`. Verified by running eval-self-tests and validate from the new layout. `plugin_root` in both harnesses walks up one directory now instead of three. * Re-execute run.sh by path in its own self-test instead of $0 The four end-to-end assertions run the whole sweep in a subprocess by invoking `"$0"`. That is only a runnable command when the caller passed a path with a slash in it: `bash run.sh --self-test` from this directory sets `$0` to `run.sh`, which is not on PATH, so all four exited 127 while the eleven unit assertions passed. It worked by accident because the Makefile and the README both happen to pass a path. Use `$here/run.sh`, which is invocation-independent. * Report a detector that could not run as a failure, not as a non-trigger `skill_invoked` returned an exit status, and both "the model did not call the skill" and "python3 blew up" came back as 1. `check_triggered` then fell through its ladder to `no` — the one verdict the aggregator treats as a measurement. So a broken interpreter did not fail the sweep; it scored every positive session as a clean negative, and 45 sessions and $36 came back looking like a recall regression. This is not hypothetical. The modern-python plugin's `python3` shim rejects the call (#207), which is how it was found: the self-test's "skill invoked -> yes" case returned `no`. The verdict is now a printed token — `yes`, `no`, or `error:<detail>` — because an exit status cannot carry the distinction: 1 is both python3's own failure status and the detector's "not found". A healthy session whose detector failed lands in a new `crash:detector` branch, which invalidates the sweep like any other failed session and puts the interpreter's message in the NOTE column. Every python3 call in the script goes through `uv run --no-project` for the same reason, so the harness also runs correctly by hand under the shim rather than only under `make check`. That adds a uv dependency, guarded at startup alongside the existing claude CLI check. Pinned by a new assertion that points the detector at a nonexistent interpreter and asserts `crash:detector`, not `no`. 16 assertions, was 15. * Refuse to grade an effectiveness run whose patch never applied `grade()` diffs the failing tests before and after replacing canonicalize_url with an identity stub. It called `patch_codec` and never checked the result, on the assumption that `set -e` would abort — but errexit does not propagate out of a function into the command substitution `got="$(grade "$d")"` runs in. So a failed patch left the "after" suite running against the UNPATCHED fixture: before and after come out identical, nothing moves, and a suite that genuinely caught the defect is written down as `part` — "suite fails, but not on this defect". That also silently disarmed the drift guard inside patch_codec, whose entire job is to refuse to grade in exactly this situation. Its message went to stderr and the grade continued. A failed patch is now ERR, and the fixture is restored from the backup on that path. patch_codec goes through `uv run --no-project` like the rest of the repo, so the shim (#207) is not what triggers it either. Pinned by a new assertion that hands the grader a fixture with no canonicalize_url to replace and asserts ERR rather than a grade. 4 assertions, was 3. * Refuse a multi-level effort sweep while SKILL.md pins `effort:` A skill's `effort:` frontmatter overrides the session level, so the `--effort` that effectiveness.sh passes each session is ignored the moment the skill loads. SKILL.md pins `effort: low`, so the default `EFFORTS="low medium high"` ran three sessions at `low` and printed the level each one *asked* for in the EFFORT column. Nothing in the output gives that away. Three rows agreeing is also what a healthy sweep looks like when effort genuinely does not matter, which is the conclusion the table invites — and the conclusion that keeps the pin at `low` forever. It is the same shape as the failures this suite already guards against: a checker that has quietly stopped varying its independent variable reports a clean result. It also means the recorded sweep cannot be reproduced against the plugin as shipped. Either that sweep predates the pin, or it was already this artefact; there is no third reading. That matters because re-running an effort sweep is what AGENTS.md asks for whenever the model changes, and this is the check that would have been re-run. Requesting the pinned level alone is still allowed — that scores the shipped configuration and the label is true. NOPLUGIN loads no skill, so nothing overrides and a sweep there is honest. Anything else with a pin present exits 2 and names both ways forward. The `q` in the sed matters: a second `effort:` line anywhere in the file, a fenced YAML example say, would otherwise make `$pinned` multi-line and refuse even a correct `EFFORTS=low`. Both READMEs documented a bare `./evals-extra/effectiveness.sh`, which now exits 2, so they move to `EFFORTS=low` here rather than in a follow-up that would leave the docs describing a failing command in between. Known cost, not fixed here: `EFFORTS=low` is one session, where the broken sweep at least sampled the same configuration three times. run.sh:20-22 rejects n=1 for the sibling metric on the grounds that invocation is stochastic. Fixing it means a repetition knob or a different default — a change to how the eval samples rather than to what it reports, so it is left to the author. Pinned by two new assertions: a pinned skill refuses `low medium high`, and allows `low`. 6 assertions, was 4. * Check for the claude CLI below run.sh's --self-test dispatch, not above it The preflight sat at the top of the script, so it ran before the `--self-test` branch and the self-test exited 2 on any machine without Claude Code installed, having run zero of its sixteen assertions: $ env PATH=/usr/bin:/bin bash run.sh --self-test claude CLI not found: claude `claude_bin` is only swapped for the stub inside `self_test()` itself, which is far too late to matter. So the guarantee in the comment above that function — "uses a stub binary, so it costs nothing and can run in CI" — was false, and `make eval-self-tests`, and therefore `make check`, broke for any contributor without the CLI. AGENTS.md draws exactly this line for the two loadability checks: they run in CI rather than in `make check` precisely because needing the Claude Code CLI is not a reasonable local prerequisite. A self-test that claims to be free must not smuggle that requirement back in. The uv check stays above the dispatch, because the self-test genuinely needs it: the detectors run through `uv run --no-project python3`, and uv is already a prerequisite everywhere else in the repo. effectiveness.sh had this split right and was the template. Verified with `claude` absent from PATH and uv plus GNU coreutils present: all 17 assertions pass. With neither present it now stops on uv, which is the honest dependency rather than a borrowed one. Worth knowing and not fixed here: `timeout(1)` is still an undeclared dependency of both a real sweep and the self-test, and it does not exist on a stock macOS PATH. Absent, the child-sweep assertions fail with 127. CI is Linux so it is covered there, and the bash-3.2 accommodation at the end of the self-test suggests stock macOS is meant to work, so it wants either a preflight alongside uv or a documented prerequisite. Pinned by a new assertion that runs a real sweep with a nonexistent CLAUDE_BIN and asserts exit 2. The risk on the next edit is the check being deleted rather than moved, which would turn a typo'd CLAUDE_BIN into 45 crash:rc127 sessions instead of an immediate refusal. 17 assertions, was 16. * Fix sed issue on BSD * Restore refactoring.md --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> Co-authored-by: Emilio López <emilio.lopez@trailofbits.com> |
||
|
dependabot[bot]
|
4db88ee79d |
deps: Bump ruff (#246)
Bumps the python-minor-patch group with 1 update in the /plugins/trailmark/skills/slicing-code-context/scripts directory: [ruff](https://github.com/astral-sh/ruff). Updates `ruff` from 0.16.0 to 0.16.1 - [Release notes](https://github.com/astral-sh/ruff/releases) - [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md) - [Commits](https://github.com/astral-sh/ruff/compare/0.16.0...0.16.1) --- updated-dependencies: - dependency-name: ruff dependency-version: 0.16.1 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: python-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Scott Arciszewski <147527775+tob-scott-a@users.noreply.github.com> |
||
|
Paweł Płatek
|
cfea213cce |
skill-improver: remove the trash dependency and the duplicate command surface (#249)
* skill-improver: remove the trash dependency and the duplicate command surface Fixes two issues: - #244: hooks/stop-hook.sh and scripts/cancel-skill-improver.sh called `trash`, which is absent on stock Linux; under `set -e` the stop hook died before removing the state file even when the completion marker was detected, so the loop could never terminate, and the cancel escape hatch failed too. lib.sh now provides remove_state_file(), preferring trash and falling back to rm -f. The helper is not named `trash` because `command -v` resolves shell functions, so a same-named wrapper would always take the trash branch and still die. A hermetic regression suite runs both scripts on a stub PATH without trash; it fails 5/8 assertions on the old code. - #199: commands/skill-improver.md and skills/skill-improver/SKILL.md both registered as skill-improver:skill-improver, listing twice in the model-facing skill index. Commands are the legacy surface since the commands/skills unification, so the command's path-resolution and setup steps moved into SKILL.md (argument-hint, scoped Bash rule for the setup script) and the command file is gone. Invocation stays /skill-improver. SKILL.md now also tells the model to skip setup on stop-hook continuation prompts so a mid-loop re-trigger cannot start a second parallel session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * pr-review done * rm regression test --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Akshay K
|
304c81a8ce |
static-analysis: make the codeql skill's guards real and add tests (#225)
* fix(codeql): make the skill's guards real, add tests, trim the prose Every verification step piped its command to a formatter and used the pipeline's exit status, which without pipefail belongs to the formatter, not the command. Nine sites. The sharpest was the arm64e detection: `EXIT_CODE=$?` after `| tee` compared against 137, a value it could never hold, so the check underpinning Essential Principle #5 and three Rationalizations could not fire. The build workflow now runs check_db_quality.py as its Step 4 exit condition rather than describing metrics it never compared to a threshold, and suite generation aborts on zero resolved queries. Two blocks that were invalid bash — an else branch containing only comments — are fixed. Log helpers move to scripts/build_log.sh, sourced by the workflow and by the three reference docs that use run_logged. They were previously defined in one markdown file and used from three others, so those blocks failed standalone. As the skill's first .sh file it is also the first thing here that `make shell` lints. Sourcing it now checks the log is writable: under pipefail an unwritable log made tee's failure the pipeline's, so run_logged returned 1 for a build that succeeded and the method ladder walked to --build-mode=none blaming CodeQL. Suite generation moves to scripts/generate_suite.sh, which takes the mode as its argument. Both modes had been copy-pasted into two reference docs sharing some 25 lines of identical scaffolding — guards, the third-party pack loop, the excludes, the verification call — none of it lintable where it sat. The tests now run the script instead of extracting bash from markdown, and one of them fails if either doc inlines a generation block again. check_db_quality.py counts project files under the source root recorded in codeql-database.yml instead of against a hardcoded prefix list that only knew where macOS keeps its toolchain. src.zip stores each file at its absolute path minus the leading separator, so the recorded root is a prefix of the project's entries and of nothing else — verified against a database built by CodeQL 2.25.6. It also resolves the summary-versus- severity preference per extractor: a single `extractor-failures: 0` used to suppress the fallback for every other language in the database. run-analysis.md had its own database discovery loop, which SKILL.md states the workflows do not restate. It also lacked SKILL.md's `codeql resolve database` filter, so a marker file left by a failed build could be selected as a database. It now uses the canonical block. Twenty-odd snippets across language-details, threat-models, and performance-tuning created and analysed a literal `codeql.db` in the working directory — the exact shortcut SKILL.md lists as a Rationalization to Reject, and one its Success Criteria forbid. They use "$DB_NAME" under $OUTPUT_DIR. Adds six hermetic test suites needing no CodeQL install, plus the first tests that execute build_log.sh rather than reading it. They cover the shell in every markdown block, both suite generators, the quality thresholds, the .qls templates, and the exit-status claim the build ladder rests on. Corrects the run-all coverage figures against cpp-queries 1.8.0: the pack holds 515 alert queries, not 510, and run-all leaves 307 of them unrun, not 302 — 13 of those are Security/CWE queries, which the prose described as harmless refactoring metrics. Trims the skill's markdown from 2867 lines to 2748, and SKILL.md from 269 to 257. Out: the When NOT to Use section that #216 dropped repo-wide, five Essential Principles that restated their own Rationalization, two literal prompt mock-ups, three Reference Index rows duplicating the workflow table above them, three identical qlpack.yml blocks, and code fences in performance-tuning and threat-models that each carried a single flag. Replaces three approval prompts with one confirmation gate, and drops allowed-tools entries for tools that no longer exist. Bumps static-analysis to 1.3.0. * fix(codeql): address review findings on shell-block scope and suite claims Each markdown block runs in a fresh shell, so scalars and sourced functions cross it no better than arrays do. build-database.md now says to re-source build_log.sh and re-set DB_NAME in every block using run_logged: without it run_logged exits 127, the method ladder reads that as a failed build method, and it walks to --build-mode=none having never invoked CodeQL. run-analysis.md Step 4 re-establishes DB_NAME, RAW_DIR and SUITE_FILE for the same reason -- under set -u it aborted with "unbound variable" before analysis started. create-data-extensions.md was the third caller of database discovery and still used a bare find for codeql-database.yml, which selects a marker left by a build killed mid-run; the queries then return nothing and Step 3 reports coverage as adequate. It now uses find_databases.sh like the other two. quality-assessment.md sources build_log.sh in the Collect Metrics block, so a failed quality gate is recorded rather than silently dropped by a command-not-found, and the raised-threshold override logs inside the if -- it previously wrote "raised to 15%" even when the re-run still failed. generate_suite.sh no longer describes the run-all suite as every security, experimental and quality query: it imports two suites totalling 219 of the pack's 515 alert queries, as run-all-suite.md documents. Changed in the doc template too, which test_generation_scripts.py pins to the script's output. The two remaining review findings were already fixed on this branch and needed no change: quality-assessment.md assigns ERROR_RATIO from the script's JSON before reading it, and extractor_error_count() resolves the summary-versus- severity preference per extractor rather than once for the tree. * style(codeql): trim comments in the shared shell scripts build_log.sh carried more comment than code. The consequence of an unwritable log -- the ladder walking to --build-mode=none after a build that succeeded -- was stated twice within ten lines; it is stated once now. find_databases.sh and generate_suite.sh get the same treatment. Comments only: the diff contains no code lines. 435 tests pass unchanged. * feat(codeql): ship /static-analysis:codeql-build as a dynamic workflow The build is the part of this skill with real judgement and no user in it: try a method, read the failure, apply a fix, retry, escalate. That loop now runs unattended as workflows/codeql-build.js, beside the four workflows already on main. Three phases. Detect resolves the output directory and profiles the language, build system and macOS arm64e state. Build walks the ladder. Assess runs check_db_quality.py and applies the improvements from quality-assessment.md before re-running it. The ladder is deterministic and lives in the script; diagnose-fix-retry for a single rung lives in that rung's agent, where the build output it has to read already is. The agent is told not to escalate itself -- the caller owns the order, so a rung that fails is a result rather than a licence to try something else. Go and Swift never reach Method 4, which they reject outright; an arm64e Mac starts at 2m rather than spending two rungs to reach the same SIGKILL; an interpreted language does one extraction and no ladder at all. Nothing is asked. Every method failing returns no-method-succeeded, and a database that built but sits below the quality threshold returns built-below-threshold with its metrics. Whether the remaining extractor errors are confined to code nobody needs analysed is the caller's call, so the assess phase is told not to raise --max-error-ratio to make its own gate pass. A build command exiting 0 is not a database: every rung is confirmed with codeql resolve database before it counts, because finalize after a failed trace-command leaves one that resolves and holds nothing. tests/codeql_build_harness.js compiles the workflow with stubbed agents and asserts the ladder and the guards; --self-test mutates it six ways and requires every mutation to turn a scenario red. run_codeql_build_tests.sh wraps both so CI's existing shell-suite discovery runs them, with no workflow file changes. SKILL.md documents it as the unattended alternative and keeps the manual path. Database selection, analysis planning and data extensions stay in the session. * refactor(codeql): stop the docs recomputing what check_db_quality.py reports Collect Metrics parsed baseline-info.json with an inline python3 -c into BASELINE_LOC, then read .baseline_loc out of the checker's JSON into DB_LOC in the same block -- the same number, computed two ways, logged twice under two labels. The block's own comment said "Everything downstream reads $QUALITY_JSON rather than recomputing" while the recount sat sixteen lines above it. The inline parse and the print-baseline call before it are gone; the checker is the only thing that counts now, and the Quality Criteria table cites it rather than a command the doc no longer runs. Log Assessment read five variables out of Collect Metrics' shell. It runs in its own, so they expanded to empty and the log recorded "Baseline LoC:" with no number -- the same shape as the dangling ERROR_RATIO the first review found. It re-sources the helpers and re-reads the metrics, and no longer prints an expected-file count it cannot see. test_shell_blocks.py's embedded-python guard required three matches and there are two now, which is the guard working: removing the last sample of a construct must fail rather than silently leave the extractor untested. The floor moves to two, with the reason recorded. * test(codeql): drop test_suite_resolution.py, which never ran in CI Its six tests needed the CodeQL CLI and codeql/cpp-queries; CI installs neither, so every one of them reported as a skip on every run. Confirmed by reproducing CI's environment locally -- with codeql off PATH the directory gives 428 passed, 135 skipped, matching the job log exactly. Removing it leaves 428 passed, 129 skipped, and the remaining skips are per-block parametrisations of test_shell_blocks.py whose test functions do run for other blocks. What goes with it: the only check that resolved the suite templates against a real CodeQL rather than a fake one. run-all-suite.md's coverage claims -- that run-all is not the whole pack, that important-only reaches queries run-all does not -- are now prose nobody verifies, so the doc carries the command to re-derive them instead of pointing at a test file that is gone. test_generation_scripts.py's docstring no longer claims a sibling covers real-CLI resolution. * fix(codeql): Rust supports --build-mode=none; say so The language table left Rust as "check your CLI — not listed either way" and the Overview's three categories did not cover it at all, so a reader had no route for a Rust project and codeql-build.js resolved the ambiguity silently by putting Method 4 on its ladder. Settled against CodeQL 2.25.6 rather than the help text: `codeql database create --language=rust --build-mode=none` exits 0 and writes a database with `finalised: true`. Go, for contrast, fails immediately with "Go does not support the none build mode". So `--help` omits Rust the same way it omits C/C++, which the Overview already warned about; Rust joins that category and the ladder in codeql-build.js was right. * refactor(codeql): one prose copy of the quality-gate exit codes, not two check_db_quality.py's exit contract was written out four times: the script's own docstring, codeql-build.js's assess prompt, build-database.md Steps 4-5, and quality-assessment.md's Enforce the Thresholds. The first two earn it -- one is the source of truth, the other is an agent prompt that cannot read a docstring. The two prose copies are one too many. build-database.md keeps the part a reader needs at that moment (exit 1 is not a judgement call, exit 3 is) and defers the table to quality-assessment.md, which it already links and which is where someone looks for gate detail. The raised-threshold rationale there loses two lines it did not need. * refactor(codeql): assess phase reads the exit-code table instead of copying it The Assess prompt spelled out all four exit codes while its sibling phases point at a file -- Select says to read rulesets.md rather than choose from memory, and each build rung is sent to build-fixes.md. It now reads "Enforce the Thresholds" in quality-assessment.md the same way, keeping inline only the two facts that decide what it does: exit 1 is not overridable, exit 3 is a heuristic. That leaves one prose description of the contract instead of two, and a change to the script's exits reaches the agent without a second edit. Also aligns the ladder comment with the Rust finding: --help omits C/C++ and Rust, not just C/C++. * fix(codeql): pass --format=json, the flag check_db_quality.py defines The workflow's Assess phase ran `check_db_quality.py --json`. The script takes --format {text,json}, so argparse exited 2 before reading the database, and 2 is the one exit code ASSESS_SCHEMA does not describe. test_script_flags.py checks the class: it reads each script's accepted flags from its own --help and verifies every invocation across the plugin's .md and .js. The bug shipped because test_shell_blocks.py scans the skill tree and codeql-build.js sits outside it. * test(codeql): scan every block in one test instead of parametrizing over all of them Seven tests parametrized over all 65 bash blocks, which is 455 cases for seven assertions, and three of them skipped the blocks that did not qualify. That was 129 skips, and it hid a check that matched no block at all: the unpreserved-pipeline assertion had never run against the skill. Each now scans in one pass and lists every offending file:line, so a run reports all offenders rather than the first. 557 cases down to 118, none skipped. The array collector's empty case is an assertion rather than a skip. * refactor(codeql): point the workflow at build-database.md instead of restating it codeql-build.js carried its own copy of the build procedure: the arm64e detection block verbatim, the build-system command table, every method's invocation, and the output-directory logic. Both copies were live, which is how the workflow came to pass check_db_quality.py a --json flag while the doc had --format=json. Each rung now names its section of build-database.md, the way Method 2m already pointed at macos-arm64e-workaround.md. quality-assessment.md called the checker three times and re-derived its numbers with jq, unzip and grep. check_db_quality.py now reports archive_files and finalised from the two files it already reads, so one call covers the whole assessment. The fresh-shell rule was stated in five places; SKILL.md holds it once and the workflows link to it with the consequence specific to their site. test_section_pointers.py checks what this trade depends on: every "Section" in file.md pointer and every #anchor link must land on a real heading. The repo validator resolves paths, not section names, so a renamed heading would leave the pointers aimed at nothing with every check still green. Prose 1128 -> 1084 lines, codeql-build.js 390 -> 340. * fix(codeql): address PR review findings on skill paths and database selection codeql-build.js hardcoded plugins/static-analysis/skills/codeql, which only resolves in a checkout of this repo. Installed, the first build block sourced a build_log.sh that was not there and exited 127, so every rung of the ladder reported a build failure for a project that would have built. The Detect phase now resolves the directory at runtime from $CLAUDE_PLUGIN_ROOT, $CODEX_PLUGIN_ROOT, or a find over ~/.claude and ~/.codex, accepting a candidate only when scripts/build_log.sh exists. skillDir is a required schema field validated as absolute, so an unresolved path stops the run before the ladder starts. SKILL.md built FOUND_DBS in one bash fence and looped over it in the next. Each fence is a separate shell, so the metadata loop iterated zero times and the selection prompt had no language or creation time to show. The two are now one block. run-analysis.md Step 1 branched only on the zero-database case and fell through to FOUND_DBS[0] for any other count, analysing whichever database find returned first without telling the user there was a choice. It now uses the elif/else shape from create-data-extensions.md and exits with an error when DB_NAME is still unset. Two new checks cover the class rather than the instance. test_shell_blocks.py fails when a block reads an array it does not build. test_section_pointers.py fails on a repo-relative plugins/ path in the workflow, and resolves ${SKILL_DIR} pointers against the skill root so the workflow's file references stay checked: nine of them now, up from two. The rest of the review: run-all-suite.md no longer claims total coverage, which its own measurement section refutes; the analyze block expands optional flags as ${ARR[@]+"${ARR[@]}"}, since bash 3.2 treats an empty array under set -u as unbound; find_databases.sh exits 2 when codeql is absent instead of printing nothing, which auto-detection reads as "no databases, rebuild"; make -j$(nproc) falls back to sysctl -n hw.ncpu on macOS; every . build_log.sh site is || exit 1, as none of those blocks set -e; the Reference Index lists find_databases.sh and generate_suite.sh. * fix(codeql): source build_log.sh in every block that uses its helpers Each fenced block is its own Bash call, so a helper defined in an earlier one is undefined and exits 127. The build ladder reads that as a failed method and walks to the next one, reporting failure for a build that was never attempted. test_shell_blocks.py now fails any block that uses run_logged, log_step, log_cmd, log_result or LOG_FILE without sourcing build_log.sh, with fixtures pinning the detector in both directions. * fix(codeql): gate each step of the Method 3 multi-step build build_log.sh does not set -e, so the four run_logged calls ran regardless of each other: a failed trace-command still reached finalize, and the resulting database resolves while holding nothing, which the ladder reads as success. The steps now chain through if/elif, as 2m-a already does. test_shell_blocks.py fails an ungated `codeql database finalize`. * fix(codeql): stop database discovery reporting none when it found some find_databases.sh resolves each root to an absolute path, so the old -not -path '*/.*' exclusion also matched dotted ancestors: a checkout under ~/.cache or ~/.local had every database filtered out, the script printed nothing and exited 0, and the caller rebuilt from scratch. Prune dotted directories by name instead, with -mindepth 1 so a root that is itself dotted still searches. All three callers read the script through a process substitution, whose exit status is unobservable. Exit 2 (no codeql on this shell's PATH, a fresh shell per block) arrived as an empty list and was reported as "No CodeQL database found" for a project with several. Read it with command substitution and check the status. Tests cover the dotted-ancestor case, the dot-directory-below-root case the fix must not widen into, and a block scanner rejecting a process substitution around the script. * chore(static-analysis): bump version to 1.3.1 #231 bumped the plugin to 1.3.0 on main after this branch had already done the same, so merging main left HEAD and the merge base equal and the version-increment check failed. --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Eric Quintero
|
e6066e7db1 |
Rebuild supply-chain-risk-auditor around a deterministic collector (#227)
* Rebuild supply-chain-risk-auditor around a deterministic collector Replace the gh-only audit method with two stdlib-only Python scripts bundled with the skill: collect.py queries OSV, the npm and PyPI registries, the Go module proxy, deps.dev, OpenSSF Scorecard, and GitHub, and emits a JSON artifact; render.py turns it into a Markdown report of facts only. The model's job is the judgment layer on top — remediation, replacement candidates, narrative — written in report register and labeled as judgment. The old method could not deliver its own criteria: repository contributors are not registry publish rights, and gh sees no download counts or ecosystem-keyed advisories. What it measures, for npm, PyPI, and Go: - Version-matched advisories for direct dependencies, resolved from the lockfile, manifest pins, or labeled fallbacks — and for the full lockfile-resolved transitive tree (package-lock.json, uv.lock, go 1.17+ go.mod), advisories only. - Abandoned or archived upstreams, deprecated and yanked releases, npm publisher concentration, install-time script execution, and the two OpenSSF Scorecard checks that name a concrete mechanism (Dangerous-Workflow, Binary-Artifacts). Download volume, publish provenance, and security policy are reported as context, never flagged. The structure enforces its honesty rules rather than documenting them: - Every criterion resolves to assessed-clean, assessed-flagged, or unassessable-with-a-reason. Unavailable data is never evidence of risk, and every claim is bounded by a coverage table. - An empty advisory answer counts as clean only for a package proven to exist: a registry document for npm and PyPI, a module-proxy answer for Go, and for transitive lockfile entries a registry integrity hash or registry source. Everything else — private registries, git dependencies, vendored directories — is named as unverifiable with its reason, never counted clean. - Coverage must reconcile, a run that measures nothing exits non-zero instead of reporting that nothing is wrong, the renderer refuses an artifact whose flags and coverage disagree, and third-party text is escaped before it reaches a Markdown table. 85 offline tests exercise the invariants through the collector's own cache format, and the suite is mutation-checked. evals/ ships three fixtures with graded expectations; against a no-skill baseline the skill passed ~92% of skill-agnostic assertions vs ~60%, at half the wall clock, with its edge in reproducibility — the report regenerates byte-for-byte from the artifact — and self-consistency. Version 1.0.1 -> 2.0.0: method replacement. CODEOWNERS moves to @e-q. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Establish registry identity at the parse boundary; reconcile the sweep against the raw lockfile Review findings on the PR clustered at one boundary: a non-registry or malformed identity reaching the registry-keyed pipeline. Close the class, not the instances: - Dependency gains non_registry_reason, set by the parsers (npm file:/ workspace:/git/shorthand specs; PyPI direct deps whose uv.lock source is git/directory/path). One choke point in collect() marks every criterion unassessable with that reason and excludes such deps from all lookups — a same-named public package's advisories, publishers, and deprecation belong to code the project never installs. The deps stay in the report and its coverage. - Versions extracted from requirements text pass a PEP 440-shaped gate; pip-compile continuation/hash debris ("2.19.0 \") becomes unresolved instead of a version-matched claim. Measured: OSV compares garbage versions lexically, so the debris did not fail — it matched the wrong advisory ranges. - The transitive sweep excludes direct dependencies by (ecosystem, name, version), never by name: a nested copy of a direct dependency pinned at another version is this sweep's responsibility, and the name-keyed exclusion silently dropped it (measured on axios: two recovered entries, 620 -> 622). The artifact now carries a ledger counted from the raw lockfile before any exclusion — checked + unverifiable + excluded_direct must equal it, so a dropped triple fails validation instead of vanishing while the counts balance. The checked==0 guard hole is closed: zero reconciles like any number unless a reason is stated. - pip-audit runs with --no-deps --disable-pip. Measured: --no-deps alone still audited a pip-resolved transitive set, so pip was still fetching and potentially building untrusted distributions, against the tool's no-execution promise; with both flags it audits exactly the listed pins from registry metadata. Names on both sides of the cross-check are PEP 503-normalised. - Scorecard check thresholds move to model.SCORECARD_CHECKS as the single source of truth; the renderer derives its never-flags set from threshold-is-None, ending the clean-run contradiction that described the two flagging checks as "not flagged — poor precision". - Duplicate requirements prefer the runtime declaration (the dev file sorts first, so first-seen-wins reclassified production pins as build-time at the dev version); _git_commit confines refs to .git and degrades to None on undecodable content instead of crashing the run; recognised-but-unread lockfiles (yarn.lock, pnpm-lock.yaml, poetry.lock) produce a note and the docs name exactly which lockfiles are read; third-party text cannot inject links; the runtime estimate is honest. 88 tests; the four new guards (triple-keyed exclusion, non-registry choke, lockfile ledger, zero-checked reconciliation) are each mutation-checked against the exact reviewed bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Mark PEP 508 direct references as non-registry at the parse boundary `flask @ git+https://...` in pyproject.toml or requirements.txt was stripped to a bare name — `@` is a name terminator in _REQ_SPLIT, so the URL was silently discarded — and the dependency was looked up on PyPI, attributing the public package's advisories and metadata to a fork that may exist precisely to fix them. The npm path guards this at spec parsing and the PyPI path guarded it only via uv.lock's source table, so any pip-managed project walked past the choke point. Direct references are now detected in the requirement text itself and carry non_registry_reason with the URL; the existing choke point does the rest. Assert folded into the requirements parsing test and mutation-checked against the reproduction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Escape third-party text everywhere in the report, not only in tables The Method-and-caveats notes and the report header interpolated untrusted strings unescaped. Reproduced end to end: a package.json dependency key containing newlines (JSON permits them) with a file: spec produced a note that wrote a `## Summary` heading and a forged "No known advisory affects any of the 12 direct dependencies" bullet into report.md — a deliverable meant to survive being pasted into a client report. The 12-byte commit field read from the target's .git/HEAD had the same reach. Whitespace collapsing is the half that matters: it confines hostile text to the line it was interpolated into, where the worst available is inline emphasis rather than forged block structure. The mechanism was already right, so this is the missing calls plus a rename — _cell is now _safe_text, since a table-shaped name is what invited skipping it off-table. The existing table test asserted on the row it expected, which is why it never noticed these paths; the new test asserts structurally that every heading and bullet in the report came from the renderer. Both escaping calls are mutation-checked. Normal reports are byte-identical: the note templates carry no pipes or brackets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Accept v-prefixed and epoch versions; stop escaping inside code spans A regression review comparing the branch tip against its first commit found that two of the earlier review fixes had costs worth paying back. The PEP 440 version gate required a leading digit, so the legal pin `django==v3.2.0` became an unresolved version: advisories were matched against the latest release and 62 real ones for that version read as assessed_clean. PEP 440 permits the prefix, pip accepts it, and OSV matches it. The gate now accepts and strips it, so the reported version is canonical. Probing that also surfaced a longer-standing defect in the same path: extraction split on `!`, which is there for `!=` and truncated a PEP 440 epoch, so `1!2.0` was reported as the pin `1`. Extraction now ends the version at whitespace, a comma, or a semicolon, which additionally recovers the real pin from pip-compile hash lines that previously fell back to unresolved. Markdown does not process backslash escapes inside a code span, so escaping there wrote the backslashes out literally: the report title and the `Scanned:` path came out as `/tmp/pkg \[v2] \| beta`, which is not a path a reader can copy. Values inside backticks now go through _safe_code, which collapses whitespace and neutralises the one character that matters there — a backtick, which would close the span early — and leaves the rest alone. Prose and table cells keep _safe_text, so link forgery and cell escapes are unchanged. One first-party note lost its literal brackets rather than being escaped around them. Non-registry dependencies now share one unassessable reason, with the specific source in the signal value and the Method note. Embedding the source in the reason gave each dependency a unique string, which defeated the report's grouping: a 7-workspace-package fixture produced 91 near-identical bullets across 13 criteria, and the Not-assessable section went from 56 lines to 132. It is back to 56. Also: a collector-level fixture now proves the transitive ledger is sourced independently of the buckets it checks — deriving it from them made the equation true by construction and left a dropped package undetected while all tests passed. The docstring no longer claims the ledger is counted from the raw lockfile, which overstated its reach, and _locked_beyond_direct's return annotation matches its five values again. 91 tests; all four fixes mutation-checked. Real reports are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Keep the pipe escape for code spans inside table cells Splitting the escaping into prose and code-span variants dropped the pipe escape from both, but a GFM table row is split on pipes before inline spans are parsed, so a pipe inside a code span still ends the cell. A dependency named `evil|forged` put six boundaries in a five-column row: the name truncated to a bare backtick and `evil`, and every later value shifted one column right, so "none known" rendered under Other findings. Verified against a CommonMark+GFM parser that this is genuinely a third context rather than a reason to revert: inside a table cell `\|` renders as a literal pipe, while outside one the backslash survives into the output — which is the corrupted `Scanned:` path the split fixed. The three table paths now use _safe_code_cell; the bullet and header paths stay on _safe_code. The new test asserts column parity across every table in the document rather than one row in one table, counting the pipes GFM actually splits on so an escaped pipe reads as content. Per-path assertions are what let this reach three call sites at once, and what missed the notes path two commits earlier. Each of the three sites is mutation-checked independently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Check the assembled document instead of trusting every escape site Escaping was applied per interpolation site, and three successive commits each fixed one set of sites correctly while leaving siblings unguarded: the notes and header fields, then three table paths, and now the download-volume line and the informational sample. Coverage depended on whoever wrote or reviewed the diff noticing every interpolation, which is the wrong thing to rest a property on when the failure mode is a client deliverable carrying a forged all-clear. render() assembles a list of lines and joins them, and every legitimate line is appended as its own element, so two invariants are precise and cannot be violated by legitimate content: no assembled line contains a newline, and every table row carries its header's unescaped-pipe count. check_no_forged_lines enforces both immediately before the join. Block forgery needs a newline to open a new block, so the first invariant catches any site that leaks, including sites not yet written; verified by reverting each of the two newly-escaped sites, which now fails seven existing tests rather than none. The two open sites are escaped as well rather than left to the invariant: with escaping a hostile name renders harmlessly and the audit completes, while the invariant alone would let any audited repository deny its own audit by naming a dependency with a newline in it. The shared test fixture now carries a newline, a backtick, and a pipe in its default name, so every render test drives adversarial input through every path it touches. Its informational criteria carry real booleans too: they were ints, and informational_section sorts on `is True` / `is False`, so the path that interpolates names into a Without: sample had never executed in any test. That combination is why the misses kept recurring. 93 tests; both invariant loops mutation-checked. Real reports are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Akshay K
|
94abac91ca |
static-analysis: convert the semgrep scan fan-out to a dynamic workflow (#231)
* feat(static-analysis): ship /static-analysis:semgrep-scan and a scan runner Two entry points over one implementation. SKILL.md keeps its gated five-step path, where the user reviews and edits the ruleset list before anything runs. workflows/semgrep-scan.js runs the same scan end to end without stopping. Both read the same references/, so a ruleset added to rulesets.md reaches both at once. This is the shape variant-analysis uses. The workflow sits at the plugin root beside the four already on main and ships as /static-analysis:semgrep-scan. Four phases: Detect resolves the output directory and profiles languages and Pro, Select reads references/rulesets.md and writes rulesets.json, Scan runs the script, Report post-filters, merges and summarizes. Step 4 is skills/semgrep/scripts/run-scans.sh, not a fan-out of subagents. Nothing in the scan phase needs judgement: the agents would run fixed commands and report $? and a jq count. Exit codes now stay with the processes that produced them and finding counts come from the JSON they wrote, so no phase reports on work a later phase has to go behind and re-verify. --metrics=off, the --include scoping, the output-directory --exclude and the severity flags are properties of the script. Cross-language rulesets run once rather than once per language and never take --include; a ruleset already in baseline is dropped from its language; language keys fold onto a canonical name, so js and javascript are one unit; two spellings of one repository collapse to one clone. Parallelism is the script's --jobs. The workflow does not stop for ruleset approval. None of main's four ask the user anything, and invoking one with a target is the opt-in. The scan is read-only over the target -- no --autofix or --fix is ever passed, every write lands inside the output directory, and the script refuses to run when the output directory is the target. Semgrep rules are declarative YAML, so pointing --config at a cloned rule repository executes nothing from it. The gate was scope confirmation rather than protection from a dangerous action, and what ran is recorded in rulesets.json and scans.json either way. A deliberate change to a security skill's stated policy, not an oversight; the gated path remains for when the ruleset selection is the thing that matters. Fixes four defects the old prose path carried: allowed-tools omitted the tool Step 4 needed; --severity MEDIUM/HIGH/CRITICAL is rejected by semgrep, so important-only mode never ran; each scanner deleted the shared repos/ clone while others were still reading it; and the scanner agent declared its tools as a comma string where the loader expects a list. Two bugs the new suites found while being written. `eval "$cmd" &` reports exit status 1 whatever the command exited with, which marked every failed scan as a success; commands are built as an argv array and executed directly, which also leaves no quoting surface. And the target was resolved with `cd && pwd` while the output directory was not, so on any path crossing a symlink -- every path under /var on macOS -- both the equality check and the inside-the-target test missed, and the run scanned its own cloned rule repositories. tests/run_scan_tests.sh covers the script: command generation via --dry-run, and execution, exit codes and clone failures against stub semgrep and git binaries. tests/workflow-harness.js compiles the workflow with stubbed globals and asserts that a relative target, an output directory equal to the target, a dead phase and a failed scan each stop the run rather than reach the report as an empty result; --self-test mutates the workflow six ways and requires every mutation to turn a scenario red. Both are hermetic, reach no network, and CI's existing shell-suite discovery runs them, so no workflow file changes. Removes references/scanner-task-prompt.md and the no-Workflow fallback it served. There is no hand-rolled path and no second implementation in prose. * fix(static-analysis): resolve the skill dir at runtime, filter the merged SARIF, clear stale raw output Three defects flagged by kz-tob on #231. The workflow hardcoded SKILL_DIR as a repo-relative path, so every scripted command only resolved inside a checkout of this repo. A marketplace install runs with the user's own project as cwd and the scan phase would have found no run-scans.sh at all. Resolved at runtime instead, folded into the Detect phase, following the cascade variants.js already uses. Each candidate ends at scripts/run-scans.sh, which makes a stale install self-excluding: verified against the 1.2.2 install on disk, which ships merge_sarif.py and nothing else, and the glob correctly declines to bind to it. An unresolved directory throws rather than leaving an agent to compose semgrep commands by hand. In important-only mode both the workflow and scan-workflow.md told the agent to apply the scan-modes.md jq filter to the merged SARIF. That filter reads .results[].extra.metadata, which SARIF does not have, so it exits with "Cannot iterate over null" and results.sarif stayed unfiltered while the JSON side was filtered. The metadata is not recoverable from SARIF, but finding identity is: (check_id, path, start.line) and (ruleId, uri, region.startLine) match field-for-field, confirmed against real semgrep output, and it is the same triple the merge already dedups on. merge_sarif.py --important keeps the findings the JSON filter kept and fails rather than filtering if any scan has no *-important.json beside it, since a partial key set would drop real findings from the deliverable. The merge command blocks in SKILL.md and scan-workflow.md show both modes, so copying the block without reading the paragraph under it cannot produce an unfiltered deliverable. run-scans.sh never cleared raw/. merge_sarif.py globs every *.sarif there, so a rerun into a reused output directory that dropped a ruleset still merged the previous run's output for it. Tests: 58 shell assertions (+3), 46 workflow assertions (+13) with three new mutations, and 16 pytest cases for merge_sarif.py. Each fix is mutation-tested; reverting any one of them turns the suite red. * fix(static-analysis): honor a bare-path arg and guard the important-only merge * fix(static-analysis): fail the important-only post-filter loudly on a jq error * fix(static-analysis): report merge failures and exclude failed scans * fix(static-analysis): log the approved plan once and flag zero-coverage rulesets * fix(static-analysis): drop the SARIF Multitool merge path * fix(static-analysis): report SARIF files the merge could not read * fix(static-analysis): record the exclude pattern applied to every scan * fix(static-analysis): move the exclude-pattern assertions after their fixtures * fix(static-analysis): stop SIGPIPE marking a healthy rule repo as empty * test(static-analysis): pin exclusion for a target holding glob metacharacters * docs(static-analysis): describe the semgrep skill as it now runs --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Fredrik Dahlgren
|
7b9bd5f950 |
Add writing-lean-proofs plugin (#226)
* Add writing-lean-proofs plugin Structured Lean 4 proof writing and library design following Mathlib conventions: sorry-skeleton workflows, API-first definitions, lemma extraction, and an anti-pattern catalog mapped to the linters that enforce each rule. Includes a review-flow eval suite (evals/): five Lean fixtures derived from a real formal-verification project with known planted flaws and known non-flaws, natural review prompts, per-case rubrics, and a runner comparing a baseline arm against a skill arm with an LLM judge plus a deterministic no-rewrite check. The grader ships a self-test that asserts a known-bad review fails every criterion. Smoke-tested: the baseline arm reproduces the folk-advice mistake the skill corrects (calling redundant `show` lines noise); the skill arm passes 5/5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address Claude review findings - Validate the grader's "id" field before writing grades.json, so a malformed grader response fails with a clear error instead of a KeyError in the summary. - Restore title-case "When to Use"/"When NOT to Use" headings: the validator's required-section check matches them case-sensitively. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address github-actions review findings Eval harness: - checksum_tree fails when it finds zero .lean files, so the no-rewrite check can no longer pass vacuously - both arms run with --setting-sources project, so a user-level install of this skill cannot contaminate the baseline arm - a failed case is recorded and the run continues; the runner exits non-zero after printing the summary - critical steps in run_case return explicitly (errexit is suppressed when the function runs in an if-context) - self-test prints its artifact dir; the on-failure leak is intentional - case 04: dropped the trailing omega so the squeezed simp only in carry_le_one is genuinely terminal (the direction test was inverted) - case 05: overall-verdict retyped from must-flag to overall, with the grader prompt told to judge such criteria solely by pass-when text - fixture overlap removed: Felt.lean's unscored unfold replaced by a bound-free lemma; Bounds.lean's unscored Fact instance replaced by the NeZero instance ZMod.val_lt actually needs Skill content (claims verified against Mathlib docs and the ImProver paper): - lake build alone does not catch sorries (they are warnings); step 4 now includes an explicit grep gate - style linters are enabled in Mathlib's own build but off by default downstream; the opt-in is now spelled out. The show, nameCheck, and setOption linters do exist, so those attributions stand. - ImProver's 100% is on the paper's accuracy metric and holds by construction (fallback to unchanged input); now stated as such - isCompact_union does not exist in Mathlib; example is isCompact_iUnion - library-design.md now distinguishes rfl-proved API lemmas (correct) from downstream proofs needing rfl (the smell) - plugin README Contents paths fixed to be relative to the skill dir Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address second-round github-actions review findings Eval harness: - case 05 gains an engagement criterion: the review must name specific declarations from the file, so an empty "looks fine" can no longer score 5/5 by omission - the grader self-test is now two-sided: the canned bad review must fail every criterion (catches a permissive judge) and a new canned good review must pass every criterion (catches an over-strict judge) - case 01 scores the native_decide in the Fact instance; it was the most severe flaw in the fixture and previously unscored - checksum_tree excludes .claude/ so both arms fingerprint the same file set - runtime check on the isolation guard: a baseline transcript that mentions writing-lean-proofs fails the case as contaminated - results stamp includes the PID so same-second runs cannot collide - case 04: mod_add_carry_mul uses non-terminal simp [carry] (a bare simp risked "no progress"/goal-closing errors that would invalidate the criterion); rubric wording updated Skill content: - the sorry check had inverted exit status for CI use (grep exits 1 on no match); now ! grep, with the comment/docstring false-positive caveat and #print axioms alternative spelled out - anti-patterns.md documents native_decide (trust-base widening, caught by #print axioms, kernel/certificate alternatives) - llm-techniques.md sibling links demoted to plain-text mentions: AGENTS.md prohibits reference chains (file1 -> file2), which the earlier link conversion had introduced Verified: two-sided self-test passes (bad 0/7, good 7/7); live case 05 run scores baseline 2/6 vs skill 6/6 with no-rewrite passing in both arms and no contamination false-positive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: add linting, performance, and tactics references to Lean skill * Make the eval harness's own checks able to fail Five review findings, all cases of a check that reports rather than gates. - no-rewrite gates the exit status. It was written, printed, and otherwise ignored: CASE_FAILURES only counted reviewer/grader *errors*, so a reviewer that started applying its own fixes under --permission-mode acceptEdits would rewrite every fixture, print no-rewrite:fail on every line, and still exit 0 — CI would read the run as green. - Add the skill arm's mirror of the baseline contamination backstop. cp -R succeeding proves the skill is on disk, not that the CLI discovered it. If project-skill discovery under --setting-sources project ever changes, the skill arm would run bare and both arms would score alike — which is indistinguishable from the true negative "the skill provides no uplift", the one conclusion this suite exists to measure. One canary call per run asks the CLI what it can see. - Guarantee the scratch tree is removed. run_case's body moved into run_case_body so the single rm -rf covers every early return; six paths (fixture copy, skill copy, both checksums, the reviewer call, the contamination check) previously leaked a fixture copy — plus a full skill copy on the skill arm — per case. - Run the offline half of --self-test before the CLI preflight, so the schema, isolation-fixture and rubric checks work with no `claude` on PATH and no authentication — which is where you would want them, e.g. a CI shell-test suite. - Count rubric criteria with the same pattern the Python capture uses. The shell count accepted `- id: foo bar`, which the capture drops, so a malformed line surfaced later as a confusing id-mismatch diff. Also anchor linting.md's warnings-as-errors CI snippet against vacuity: Lake caches per-module artifacts and linter warnings are emitted only on recompile, so on a restored cache build.log is empty, the grep matches nothing, and the gate reports clean over live warnings and sorries — the exact failure the same file preaches against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> Co-authored-by: Marc Ilunga <marc.ilunga@trailofbits.com> |
||
|
Marcelo Morales
|
02b8a5eb8d | Add a workflow for porting Semgrep rules to other languages (#234) | ||
|
kz-tob
|
1a4ba9f1d6 | Add first-pass eval suite for yara-authoring (#219) | ||
|
Clinton Thomas
|
ea7c60e628 |
Convert audit-context-building to a dynamic workflow (#228)
* Convert audit-context-building to a dynamic workflow The skill held a fixed plan that ran the same steps over many functions and spawned subagents from prose. That is a workflow, so it is one now. workflows/audit-context.js orients, analyzes each function in its own subagent, and synthesizes a dossier. Each subagent writes its prose to audit-context/functions/ and returns a schema-validated record, so only the records reach the calling session. The skill routes to the workflow rather than analyzing inline. Also in this change: - Resolve a contradiction between SKILL.md and OUTPUT_REQUIREMENTS.md, which gave different minimum assumption counts for the same section while the agent was pointed at both. - Collapse OUTPUT_REQUIREMENTS.md and COMPLETENESS_CHECKLIST.md into one ANALYSIS_FORMAT.md. The per-function checklist existed in four places. - Drop the numeric quotas. Minimum counts of invariants, assumptions, and applications of a technique produce padding rather than analysis. - Delete commands/audit-context.md. Its --focus flag reached a skill that never accepted one, and its command name collided with the workflow's. - Add DOMAIN_NOTES.md mapping the format across smart contracts, C and C++, decompiled firmware, and web services. - Rework the worked example to cover C and Solidity. - Fix two README links to plugins that do not exist. - Rewrite user-facing text in plainer language. Add four eval cases under evals/, covering C source, Solidity, and Ghidra output. dispatches-not-inlines checks that the skill routes instead of analyzing in the caller's context; the other three check that analysis follows a call into the function being called and walks every path through it, not only the one that succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Raise dispatches-not-inlines turn limit; note measurement gaps The plugin arm was hitting max_turns 20 and being truncated before it answered, which the graders scored as a failure to route. SKILL.md points at three reference files that are read before any work starts, so it needs roughly twice the turns a bare agent uses on the same prompt. Raised to 40, and the timeout to 900s after a run hit the old 600s ceiling. Restore the imperative wording in the routing section. The plainer phrasing was not the cause of the truncation, but this is the revision that was measured, so keep it. Record in the README what is and is not known: dispatches-not-inlines has not been re-measured since the turn limit changed, the Solidity and Ghidra cases score 1.00 with no plugin loaded, and the plugin arm costs about twice the turns of a bare agent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Clinton Thomas
|
798b591aa4 |
Convert spec-to-code-compliance to a dynamic workflow (#230)
* Convert spec-to-code-compliance to a dynamic workflow
The skill held a fixed seven-phase plan and drove a subagent from prose, which
is the shape Maker Week asks us to move into a script. Phase 3 required
line-by-line YAML IR for every function in the codebase before Phase 4 could
start, so on any real target the window was exhausted partway through and the
remaining requirements got a plausible check rather than a real one.
workflows/spec-compliance.js inverts that. Requirements are extracted once, then
each one gets its own agent to hunt the code with, so no context ever holds the
whole behavioral model. Divergences go to two agents that did not produce them,
one re-reading the code and one re-reading the document, and what either refutes
is dropped. A separate agent sweeps the reverse direction, which the
requirement-driven pass cannot cover.
The removed resources are replaced by the workflow's schemas: OUTPUT_REQUIREMENTS
set minimum item counts that a spec with fewer requirements can only meet by
inventing them, alongside a "zero speculation" rule in the same file.
COMPLETENESS_CHECKLIST was a self-verification pass. IR_EXAMPLES demonstrated
YAML formats the schemas now enforce. SKILL.md keeps the judgment that stays
judgment: which verdicts matter, and when a gap is a code fix or a docs fix.
Fixes a classification bug in passing. SKILL.md called undocumented behavior
UNDOCUMENTED CODE PATH while the other two resources called it
code_stronger_than_spec, and the former was not one of the six legal match_type
values, so following the skill emitted a verdict outside its own enum.
commands/spec-compliance.md is removed: it forwarded two arguments to the skill,
required naming the spec that Phase 0 exists to discover, and would have collided
with the workflow on the same slash command.
Adds three eval cases. routes-not-inline measures Δ +1.00 for dispatch.
name-is-not-evidence and documents-contradict both measure Δ 0.00 — Opus 5
handles those unaided — and say so in their own descriptions rather than
implying coverage they do not have.
* Restore domain guidance dropped in the workflow conversion
The conversion deleted IR_EXAMPLES.md on the grounds that the workflow's schemas
replace it. The schemas replace the YAML formats it demonstrated; they do not
replace knowing that `unchecked` suspends a guarantee the calling code is written
as though it still has, or that a 0.3% fee is implemented as `amountIn * 997`
and matches nothing you can grep for. A Solidity user brought the same spec and
got a checker with no calibration for it.
audit-context-building kept its worked examples and added DOMAIN_NOTES.md for the
same cross-domain problem. Following that:
- DOMAIN_NOTES.md maps what counts as a specification, what enforcement looks
like, and where it hides across contracts, C and C++, services, and decompiled
firmware, plus scoping a check against an RFC or standard rather than a project
document. Generalizing past contracts was previously done by deleting the
"non-blockchain" exclusion and adding nothing.
- WORKED_EXAMPLE.md carries three requirements to a verdict, one per verdict
that is easy to get wrong: arithmetic that satisfies a requirement it does not
resemble, an absence whose credibility is the search record, and enforcement
present on every path but the one nobody tested.
- ANALYSIS_FORMAT.md holds the on-disk format in one place. The agent and the
workflow prompt were both describing it, which is the duplication this
conversion was meant to remove.
README gains a migration note: the 1.x command is gone, arguments inverted, PDFs
work and DOCX never did, and the report is no longer a fixed 16 sections.
* Survive an unresolvable checker agent instead of failing the phase
Running the workflow end to end for the first time failed all six requirement
checks with "agent type 'spec-to-code-compliance:spec-compliance-checker' not
found" and returned nothing salvageable. The proximate cause was a stale session
— the plugin had been installed after the session started, so its agents were not
in the registry — but a hard dependency on a namespaced agentType with no
fallback turns any resolution failure into total loss of the run.
checkRequirement now retries on the default agent with the checker's load-bearing
rules inlined, and logs once that it did. The fan-out is concurrent, so every
item in the first batch attempts the specialized agent before the flag is set;
those attempts fail at spawn without consuming tokens, and later batches skip
straight to the fallback.
The prompt no longer says "in the format your instructions define", which was
only true on the specialized path and left the fallback with no format at all.
Verified end to end against evals/documents-contradict/fixture: 26 requirements
extracted from two documents, 6 checked, 3 divergences found and none refuted —
the operator zeroing balances via reassign (critical), and the Senior-tier
collateral bypass from both directions (high). The SPEC/README fee contradiction
was reported as a documentation fix with the note that README frames it as
deliberate. The report named all 20 unchecked requirements as unknown rather than
compliant, and declined to give the unchecked fee requirements a verdict while
still reporting the contradiction as a direct observation.
Confirms the id-collision fix in
|
||
|
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> |
||
|
Thomas Chauchefoin
|
82e8e0ad77 |
insecure-defaults: convert the skill to a dynamic workflow (#224)
* insecure-defaults: convert the skill to a dynamic workflow Rewrites the plugin as a four-phase workflow: recon profiles the target, parallel sweeps collect candidates, a refuting pass adjudicates them, and a report assigns severity with coverage accounting. The skill is removed and `/insecure-defaults:audit [path]` is the only entry point, since the workflow needs the command to locate its detection corpora. It also adds offline tests: scenarios over the workflow's control flow, a mutation self-test that proves they bite, and a check that every documented example is matched by a seed pattern. This does not replace running the command end-to-end against a real codebase. Bumps to 2.0.0. * insecure-defaults: run the node suites in CI The harness and seed-coverage checks ran only when someone remembered to. CI's shell-suite discovery matches plugins/*/tests/run_*.sh, so wrap the three node invocations in run_seeds.sh and both the lint workflow and `make shell-suites` pick them up with no changes to either. Without this, adding a row to CATEGORIES without its references/<id>.json passes CI and then aborts every real run with corpus-unreadable. No setup-node step: ubuntu-latest ships Node, and the scripts are plain CommonJS with no dependencies. The command -v guard makes a missing interpreter a loud failure rather than a suite that quietly does not run. * insecure-defaults: abort when the verify phase adjudicates nothing If every verify batch died, confirmed was empty and the run returned no-findings-confirmed, which commands/audit.md considers a completed audit. Guard on unadjudicated.length === candidates.length and return verify-failed, carrying coverage so the caller sees what went unjudged. Adds a scenario for both failure shapes (all agents dead, all verdict lists empty) and two mutations covering the guard firing and over-firing. * insecure-defaults: report per-category scan counts The zero-scanned guard is on the sum, so five failed searches beside one that worked cleared it and categories_run listed all six. Add files_scanned_by_category and unsearched_categories to coverage, keyed off CATEGORIES so a dead sweep counts as 0, and have the report name them under a Not searched heading. Adds a scenario covering a searching sweep, a zero-file one and a dead one, plus three mutations. * insecure-defaults: count sweep failures against the category list The corpus-unreadable note and the seed-only log divided by the sweeps that returned, so with sweeps dead the ratio read 1/1 rather than 1/6. * insecure-defaults: drop the assertion-count floor from the harness * insecure-defaults: guard a dead report agent agent() returns null on terminal failure, so a report agent that died returned status "findings" with no report and the caller printed nothing while the findings sat in the structured return. Return report-failed with the findings and coverage, and have the command render them. * insecure-defaults: stop labelling a genuine clean run a failure Step 3 accepted findings/no-findings-confirmed and called every other status an incomplete audit, so no-candidates, the deliberate honest-negative status, told the user the run failed. It is now a per-status table. * insecure-defaults: anchor the noisiest seeds (DES|RC4|...) matched NODES and MODES, 0o?(666|777|...) matched any digits, and getMessage() matched all Java exception handling. On the Python stdlib the first two drop from 147 and 228 matching lines to 0 and 84. random. and getMessage() can't be fixed by anchoring, so they now require context: a security-material identifier near the RNG call, and concatenation into a string literal for getMessage(). seed-coverage.js confirms all 18 documented VULNERABLE examples still match. * insecure-defaults: indent the seed wrapper the way shfmt wants --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Joop van de Pol
|
9ea55c5987 |
Fix constant-time analyzer backends and trim the skill (#220)
* Add per-language triage fixtures for the constant-time analyzer One fixture per supported language, each pairing operations the analyzer reports identically but that triage must separate: a division on a private-key coefficient beside one on a public buffer length, and, where the backend detects them, a weak RNG seeding a nonce beside the same call jittering a retry delay. Expected verdicts are deliberately absent from the fixture source. These files double as evaluation input, and a verdict written next to the code is a verdict the reviewer reads instead of deriving. A later commit adds the manifest that records them. Fixture shapes are dictated by what each toolchain emits: the Go helpers are //go:noinline so each stays a distinct symbol, its main() takes input from argv so the arithmetic is not constant-folded away, and the Swift entry points use @_cdecl to keep symbol names readable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Configure ty and fix the typing weaknesses it reports `ty check` reported 50 diagnostics, all pre-existing. Most were noise from module resolution: the tests reach the modules via `sys.path.insert` and import `analyzer` rather than `ct_analyzer.analyzer`, which ty cannot know without being told. `tool.ty.environment extra-paths` tells it, dropping 32 unresolved-import reports and leaving the substantive ones visible. The rest were real, if not yet bugs: - Six compilers and several entry points annotated `list[str]`/`str` while defaulting to None. Now spelled `| None`. - `get_compiler` was annotated to require a compiler name, but its body auto-detects from the language when the name is falsy, which is how every caller uses it. - Each parser tracked functions as a bare dict, typing the values `str | int`, so the `functions[-1]["instructions"] += 1` counter present in all eight parsers read as string addition. A `ParsedFunction` TypedDict names the shape instead. Three unresolved-import reports remain by construction: analyzer.py is documented to run as a script as well as import as a package, so both the relative and top-level spellings are needed and only one can resolve. Those carry an inline ignore with the reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix analyzer backends that reported PASSED on vulnerable code Auditing each backend against real toolchain output found six of them skipping unparseable lines and reporting success. Every one was covered by tests that replay hand-written listings, which is why the drift went unnoticed; the tests added here drive the actual toolchains. Python, on 3.11 and later: - The BINARY_OP oparg map read 12 as `//`, but 12 is `^`. Floor division on a secret was missed and every constant-time XOR was reported as a division. Keyed on the operator symbol dis prints, which cannot silently renumber. - The instruction regex required a byte offset that 3.13 no longer prints, so all but the first instruction of each source line was skipped. On 3.13 `key_coef // (2 * gamma2)` came back PASSED. Every column ahead of the opcode is now optional, and the source line carries forward. PHP: `_parse_opcache_output` delegated to the VLD parser on the claim the formats were "similar enough"; they share nothing structural. `analyze()` also called the VLD parser unconditionally, so the function was dead either way. VLD is an unbundled PECL build, so this is the path most users hit: a file with intdiv, mt_rand and base64_encode reported zero findings. Adds a real parser, plus intdiv/fdiv, which perform division through a call and so emit no DIV opcode. JavaScript and TypeScript: V8 prints `67 E> 0x… @ 14 : 3e 03 04 Div a0, [4]`, not `offset : Mnemonic`, so every file parsed as zero instructions. `--no-lazy` covers functions a module exports without calling. Bytecode blocks are filtered to names the file declares, since V8 dumps node's internals identically. Source positions are byte offsets, now converted to lines except for transpiled TypeScript, where they index generated output. Go: `go build` links the runtime in, so 23 of 24 findings came from the allocator and garbage collector while the caller's code was silent. Objdump headers name the source file, so the filter is exact. Go also writes arm64 in Plan 9 syntax, where a 32-bit divide is SDIVW — absent from the table, so `int32` division, the shape of every polynomial coefficient divide, read as clean. Rust: compiled as a bin crate, so any library file failed E0601. Crypto lives in libraries. Swift: target triples were hardcoded to Apple platforms, so swiftc rejected them on Linux and no Swift file could be analyzed there. Java: the weak-RNG pattern matched only `new Random(`, missing the fully-qualified form that needs no import. All six source-level scanners treated comments as code, so `/** a / b */` counted as a division; comment bodies are now blanked while preserving offsets. Finally, the text report labelled every scripting-language error `[WARN]` while the summary counted it as an error: analyzer.py runs as `__main__` while script_analyzers imports it as `analyzer`, giving two Severity enums, so identity comparison failed. Severity is compared by value now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Record triage verdicts for every fixture and assert the pairs hold expectations.json gives each fixture's cases a verdict and the reasoning behind it: 53 cases over 13 languages, covering arithmetic everywhere, conditional branches on the native backends, weak RNG wherever a backend detects it, and PHP's encoding functions. TestTriageMatrix asserts the mechanical half — that the analyzer still reports both members of every true/false-positive pair, since the premise of the skill's triage step is that the tool cannot tell them apart. Four of its checks need no toolchain, so the matrix stays guarded on machines that cannot compile most of these languages: every supported extension has a fixture, every fixture pairs a true with a false positive, every line-based locator resolves exactly once, and no fixture leaks its verdict into a comment. The run itself fails rather than skipping when no fixture could be exercised, and names the languages it skipped. Two cases record measured behaviour rather than the intent: Rust claims no branch true positive, because rustc emits no flagged conditional branch in the tag comparison at O0 on arm64 and asserting one would encode a fixture artefact; and Go's branch false positive is its stack-growth check, which appears in every Go function and depends on nothing the caller passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Trim the constant-time-analysis skill and state its real coverage SKILL.md goes from 219 to 165 lines by removing what was duplicated or inert: a decision tree that restated the trigger bullets below it, five near-identical invocation blocks, and JVM/.NET setup copied from references/vm-compiled.md. It gains the frontmatter it was missing — allowed-tools, which it never declared and so inherited everything, and an explicit effort level. The additions come from evaluating the skill rather than from review. Runs at three effort levels all recommended fixing a secret-dependent divide by handing the compiler a constant divisor; measuring that fix showed it still emits a real divide on gcc riscv64 at every optimization level, on gcc arm64 and x86_64 at Os and Oz, and on clang arm64 at O0 and Oz. The sweep section now says so, with the table. Coverage is not uniform across backends, and a clean report means different things depending on the language, so there is a table for that too. Triage guidance now also covers the weak-RNG and encoding findings, where no operand is secret and the question is what the result is used for — seeding a nonce or jittering a retry delay. "When NOT to Use" names the constant-time-testing skill and draws the line between them: that one measures a running binary, this one reads compiler output and never executes the code. Both READMEs listed nine languages when thirteen are supported, and the skill's file tree was missing four reference files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix warning-level detectors that no source could ever match Extending the triage matrix to the comparison, lookup and encoding families found that several of those detectors were unreachable. The tables listed them, so the coverage looked complete, but no input could trigger them. - Ruby built every warning pattern as `\.<name>`, which is right for method calls like `.start_with?(` but turns the module-qualified keys into `\.base64\.encode64`. Five entries — both base64 functions, json.parse and both marshal functions — could not match any Ruby source. - JavaScript did the same to its non-method entries, so `btoa(`, `atob(`, `decodeURIComponent(` and `JSON.parse(` never matched. - The JS keyed-property mnemonics were V8 9.x spellings. Node 22 emits `GetKeyedProperty`, so secret-indexed array access — the whole point of that family — was undetectable. The named-property entry is removed rather than updated: a named access has a constant property name and cannot be indexed by a secret, so flagging it reports every `Math.trunc` call. - Java, Kotlin and C# selected patterns with if/elif chains covering three or four keys and skipping the rest, leaving the base64 and convert entries unreachable. They now share `qualified_call_pattern`, which distinguishes the two shapes the keys use: `string.equals` is a method on any receiver, while `arrays.equals` and `base64.getencoder` are members of a named type and must match only the qualified form. Case-insensitivity is embedded in the pattern because the keys are lowercase while the source spells them `Base64.getEncoder` and `.Equals`. Ruby's warning mnemonics also kept the dots from their keys, unlike every other severity and backend, so they read `BASE64.ENCODE64`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Cover the comparison, lookup and encoding families in the triage matrix The matrix covered two of the four families the analyzer implements. It now covers all four, taking it from 53 cases to 99. Each of the eight bytecode and scripting fixtures gains three pairs: an early-exit comparison of a secret tag against the same comparison on a public header, a table lookup indexed by a key byte against one indexed by a fixed offset into a public header, and a base64 encode of a decrypted session key against one of the public algorithm identifier. PHP already had the encoding pair. Native fixtures are unchanged — their instruction tables carry no comparison or load entries, and C's tag comparison is already covered as a branch. All three families are warning severity, so their cases run with `include_warnings`. That is worth noting on its own: the invocation the skill documents does not pass `--warnings`, so a reviewer following it sees none of this — including early-exit comparison, which is the most common timing bug in practice. Every case was measured against the real toolchain before being written down. Python's fixture avoids a `list[bytes]` annotation in a signature because the subscript in the annotation is itself reported at module scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Refuse an architecture the compiler cannot target Every compiler looked its architecture up with `.get()` and simply omitted the target flag when the lookup missed, while `analyze_source` still labelled the report with the architecture that was requested and applied that architecture's instruction table. So `--arch riscv64` on Swift, whose Linux map holds two entries, compiled for the host and reported `architecture: riscv64` with no findings; `--arch mips` on gcc reported `passed: true` the same way. "PASSED for riscv64" is what a reviewer writes down, so producing host output under the requested label is worse than failing. All five compilers now refuse and name what they do support. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Stop the Go symbol filter readmitting the runtime by basename The filter compares the objdump block's source path against the file under analysis, but when the realpath comparison failed it fell back to comparing basenames — and Go's runtime ships map.go, slice.go, string.go, time.go and select.go. Analyzing a user file named map.go therefore matched every `TEXT runtime.…(SB) /usr/lib/go/src/runtime/map.go` block, reported runtime.makeBucketArray among the findings, and parsed 14 functions instead of 5. That is the behaviour the filter exists to prevent, and the existing test missed it only because its fixture is named triage_go.go. objdump prints an absolute path, so the exact comparison is sufficient there; the basename fallback now applies only when the printed path is relative. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make comment blanking string-aware Blanking comments before the source-level scans stopped doc comments counting as code, but the regexes knew nothing about string literals, so a comment marker inside one blanked live code: - `const u = "http://example.com"; const k = a / b;` lost the division, and `String u = "http://x"; Random r = new Random();` reported clean — for Java, Kotlin and C# the source scan is the only detector for `new Random()`. - Ruby's `puts "tag #{a / b}"` lost everything from the interpolation on. - Worst, a lone `const marker = "/*";` plus any later `*/` anywhere in the file blanked every line in between, with no diagnostic. String and template literals are now matched first and preserved. This is still a regex rather than a lexer — unterminated literals, JavaScript regex literals, Ruby heredocs and `%w[]` are not modelled — and the docstring says so; the failure mode is a blanked or unblanked span, never a crash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test the empty-parse guards and close smaller robustness gaps The test named for PHP's "nothing parsed" refusal asserted only that the parser returned empty lists, which the pre-fix code did too, so deleting the raise in `analyze()` left the suite green. It now asserts the RuntimeError, and the Go equivalent — the symbol filter keeping nothing — is covered as well. Both were confirmed to fail with their guard removed. Four gaps that each degrade quietly: - The OPcache offset group was `\d{4}` exactly, so an op_array past 9999 opcodes stopped parsing mid-function. `functions` is non-empty by then, so the empty-parse guard cannot fire and the report reads as a partial pass. - The V8 name filter treated "could not read the compiled file" as "filtering disabled", listing hundreds of node-internal functions with nothing to distinguish that from a genuinely noisy file. Unreadable input is now an explicit None with a note on stderr. - `last_line_num` was not reset per code object, so the first instructions of a function inherited the previous function's line when dis omits one. - The triage matrix printed its skip list to stdout, where CI summaries do not show it, so an image that lost a toolchain would keep passing while the claim that all backends are exercised quietly stopped being true. Skips are now emitted per language and appear in the count. Two test-quality fixes: the matrix completeness check compared set sizes, so renaming one language while dropping another stayed green — it compares sets now, through an explicit display-name to `detect_language()` mapping. And `_have` uses `shutil.which` rather than running `--version`, because javap exits non-zero for it, which would have skipped Java and Kotlin everywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pass --warnings in the documented invocation The Quick Start ran the analyzer with no flags, which reports only error-severity findings: division, modulo and weak RNG. Four families are warning severity and stayed silent — secret-dependent branches, early-exit comparison, secret-indexed table lookups, and variable-time encoding. A reviewer following the skill exactly never saw the early-exit comparison of an authentication tag, which is the most common timing bug in practice and what Lucky Thirteen was. `--warnings` is now in the documented command, the directory sweep and the flag table, with the two consequences of turning it on stated alongside. Warnings do not affect the PASSED line, so `Result: PASSED` beside `Warnings: 6` is normal and is not a clean result. And comparison and lookup findings need their own triage question: for a lookup it is the index that must be secret, not the contents. Confirmed comparison findings have a per-language constant-time primitive, so those are tabulated rather than left as "write it branch-free"; a secret-indexed lookup gets the honest answer that no drop-in exists. Also names the plugin that owns constant-time-testing, since a reader cannot assume it is installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix the fixture path and the sdist include list Two defects that both make something unreachable: The pointer to the triage fixtures was written as a bare relative path, while every other path in SKILL.md is `{baseDir}`-prefixed. The skill runs against the user's repository, so a model following it looked for `ct_analyzer/tests/triage_samples/` inside the project being audited. The validator resolves markdown links rather than inline code, so this passed 359 reference checks while being unusable at runtime. The sdist include list named three globs covering C, Go and Rust, so a packaged install shipped none of the Python, Ruby, PHP, JS, TS, Java, C#, Kotlin or Swift samples, nor the triage fixtures and their manifest, and the tests that read them could not run from an sdist. A built sdist now carries all 27. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test that cross-compilation really targets the requested architecture Nothing covered cross-compilation. The pre-existing cross-architecture test asks for arm64, which is the host on an aarch64 machine, so it passes there without ever crossing — and a silent fall back to the host is exactly the failure `reject_arch` was added to prevent. These assert instruction names unique to the target: x86 `DIVL`/`DIVQ`/`IDIVL` and RISC-V `DIVU`/`DIVW`/`REMU` must appear, and arm64 `SDIV` must not. Dropping clang's `--target` makes both fail, which a report-shape assertion would not have caught. Go gets the same treatment for amd64, where GOARCH cross-builds because CGO is disabled. Only the paths that work are covered: on this host clang crosses to both targets and Go to amd64, while gcc, rustc and Go/riscv64 cannot cross here at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drive an explicit compiler with its own flags, and name it in the report Cross-compiling with gcc was impossible through the analyzer. `--arch x86_64` passed gcc's native ISA switch `-m64`, which the gcc on an arm64 host rejects, and the escape hatch that should have worked — handing over the cross binary — was broken: `get_compiler` treated every unrecognized `--compiler` value as clang, so `--compiler x86_64-linux-gnu-gcc` was driven with `--target=` and failed on "unrecognized command-line option". An explicit compiler is now dispatched on which compiler it actually is: by filename first, falling back to its `--version` banner, so `cc` is recognized as whatever it points at. `--compiler x86_64-linux-gnu-gcc --arch x86_64` and `--compiler riscv64-linux-gnu-gcc --arch riscv64` both work and emit that target's division instructions. Nothing is substituted on the caller's behalf. The analyzer could look for `<triple>-gcc` and use it automatically, but a Debian cross gcc is frequently a different major version than the host's, so its codegen is not the host gcc's; silently swapping binaries would put one compiler's output under another's name — the same mislabelling this commit removes. Instead, when gcc is asked for an architecture it cannot build, the error names the binary to pass, and the report records the binary that ran rather than the family, so `compiler:` is no longer "gcc" when a cross build produced the assembly. SKILL.md now states how each toolchain crosses, since "sweep more than one --arch" was not actionable for gcc users, and advises comparing against the toolchain that builds the product rather than a packaged cross build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Detect software division, and fix the armhf flags that hid it Installing the remaining cross toolchains turned up two problems that are not about missing dependencies. The arm flags were self-contradictory: `-march=armv7-a -mfloat-abi=hard` is rejected with "selected architecture lacks an FPU", because armv7-a alone specifies no FPU. Adding `-mfpu=vfpv3-d16`, Debian's armhf baseline, makes the build work. With it building, arm reported no findings at all on a fixture with three divisions. armv7-a has no hardware divider, so gcc emits `bl __aeabi_idiv` rather than a division instruction, and the analyzer matches mnemonics: a branch-with-link is not in any table, so the file read as PASSED. Software division loops over its operands, making it more operand-dependent than the instruction it replaces, so this was the worst kind of false negative. Calls to the libgcc and Arm EABI division routines are now reported by call target, which also covers __divti3 — used on x86_64 for __int128 division, which crypto code does perform. Verified against every cross target now installed: x86_64 DIVQ/IDIVL, i386 DIVL/IDIVL, arm AEABI_IDIV, riscv64 DIVU/DIVW/REMU, ppc64le DIVDU/DIVW, s390x DLGR/DSGFR, arm64 SDIV/UDIV. The ppc64le and s390x instruction tables had never been exercised before. Also stops the new "pass a cross build" hint from telling the caller to do what they just did: it is suppressed when the compiler already is a cross build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Cross-compile a freestanding probe, not a fixture that needs a cross libc CI failed on `test_clang_targets_riscv64` with "bits/libc-header-start.h file not found". The test compiled the C triage fixture, which includes <stdint.h>, so cross-compiling it needs the target's C library headers as well as the compiler. The x86_64 case passed only because the runner is x86_64, making that target native; my dev box passed riscv64 only because installing gcc-riscv64-linux-gnu had pulled in libc6-dev-riscv64-cross. So the test was asserting an environment property, not the analyzer's behaviour. Nothing about checking that a division instruction reaches the assembly needs a libc. The cross tests now use a freestanding sample with no includes, verified to compile for x86_64, riscv64 and s390x with `-nostdinc` — that is, with no headers reachable at all — while still yielding that target's division mnemonics. This also corrects the claim in SKILL.md that clang needs nothing installed to cross-compile: it needs no second compiler, but a source including libc headers still needs the target's headers. When that is what failed, the error now names the package that supplies them, which is the analyzer's own advice to sweep `--arch` being made actionable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Scott Arciszewski <147527775+tob-scott-a@users.noreply.github.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Eric Kilmer
|
ec2450d05e |
Trim dwarf-expert: single-file skill, effort level, contract test (#223)
* Trim dwarf-expert: single-file skill, effort level, contract test - Consolidate SKILL.md from four files into one (115 lines); delete reference/. Cut generic behavior rules, duplicate When-to-Use sections, a 9-line readelf reference hop, and an ASCII decision tree; fix five typos. - Fix --lookup=<offset> misuse carried over from the original: DIE at a section offset is --debug-info=<offset>; --lookup takes a program address. Verified live against llvm-dwarfdump. - Drop the invented v3-v5 version scoping from the skill, plugin.json, marketplace.json, and both READMEs. Version-specific judgment that earned its place (the -gdwarf-N pin inference, v2 surface forms) lives in the Verifying section. - Frontmatter: add effort: medium, rewrite the trigger description, extend allowed-tools with Write/Edit/WebFetch. - Add tests/test_skill_contract.py (stdlib + pytest): every dwarfdump flag documented in SKILL.md must exist in a live llvm-dwarfdump (>= 19, the verified floor for --error-display/--verify-json), plus a frontmatter contract check. Zero extracted flags or a missing tool fails rather than skips. - CI: python-tests installs llvm-19, falling back to the default llvm on future runner images. - Bump version 1.0.1 -> 1.1.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: fence-aware extraction, capability-gated resolver - body_sections() no longer strips fenced blocks from section text: fences are ignored only for heading detection, so flags documented inside examples are extracted and verified. New unit test covers both properties; a fenced --bogus-flag mutation now goes red. - Replace the parsed-version LLVM floor with a capability gate: the resolver requires --error-display/--verify-json in --help, because Apple's LLVM numbering does not track upstream and a numeric major is not comparable across toolchains. Failure lists rejected tools with their version banners. README wording matches. - Raise the extraction floor from 10 to 15 (21 flags documented). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Clinton Thomas
|
696dee5717 |
Remove debug-buttercup, re-homed in trailofbits/buttercup (#221)
The skill only makes sense inside the codebase it debugs. It hardcodes Buttercup's crs namespace, the 13 Redis stream names and 5 consumer groups from common/queues.py, the app= label selectors from its Helm charts, and its /tmp/health_check_alive liveness convention. None of that helps anyone outside that repo, and all of it goes stale when that code changes with no signal here. It now lives at .claude/skills/debug-buttercup/ in trailofbits/buttercup, next to the queue definitions and charts it documents, alongside the existing buttercup-langfuse skill. Nothing is lost. Also drops the Infrastructure section from the README, which this was the only entry under. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Scott Arciszewski <147527775+tob-scott-a@users.noreply.github.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Eric Quintero
|
8071528176 |
Remove seatbelt-sandboxer plugin (#218)
* Remove seatbelt-sandboxer plugin The skill's central justification was wrong: it taught that `file-read*` reads from any path, when a filtered `file-read*` is bounded exactly like `file-read-data`, and it never mentioned that SBPL applies the last matching rule, so a deny placed above a broad allow silently enforces nothing. Guidance that is wrong is worse than absent, because it replaces a correct instinct with a false justification, and `sandbox-exec` settles any SBPL question locally in seconds for anyone who needs one answered. With no recorded production use to weigh against four defects found in a single afternoon, the maintenance cost was not earning its keep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Update marketplace.json fix conflict resolution --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
Kevin Valerio
|
3742e65294 |
chore(ask-questions-if-underspecified): remove the plugin (#214)
The skill content is untouched since 2026-01-29. The only commits to the directory since were repo-wide sweeps (formatting in #98, codex UI metadata in #175), not maintenance. It also spends a lot of context on something you can get from one English sentence to Claude. Asking clarifying questions when a request is ambiguous is native Claude Code behaviour now, through AskUserQuestion, so an 86-line skill that mostly explains how to format multiple-choice options is paying context for a built-in. Knock-on edits: AGENTS.md used this plugin as its Basic reference example, now git-cleanup, which is a better fit anyway at five files and one self-contained SKILL.md. let-fate-decide pointed at the skill by name in its description and its When NOT to Use list, now reworded to plain "ask clarifying questions"; its description feeds skill triggering, so it gets a patch bump to 1.2.2. Co-authored-by: kevin-valerio <kevin-valerio@users.noreply.github.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> |
||
|
kz-tob
|
daa4ad03eb | Update CODEOWNERS (#236) | ||
|
Tarun Bansal
|
1256982d4d |
Remove workflow-skill-design plugin (#215)
Co-authored-by: Benjamin Samuels <1222451+bsamuels453@users.noreply.github.com> |
||
|
Samuel Moelius
|
09dfbd9153 |
Drop the "When to Use" / "When NOT to Use" requirement (#216)
* Drop the "When to Use" / "When NOT to Use" requirement * Fix inconsistency in AGENTS.md * Eliminate `REQUIRED_SKILL_SECTIONS` check in validate_plugin_metadata.py |
||
|
Dan Guido
|
ca08fc8a91 |
Commit plugin lockfiles; unblock Dependabot (#213)
* Commit plugin lockfiles so Dependabot can do something useful The uv ecosystem config added in #206 pointed at four directories that declare PEP 621 ranges and carry no lockfile. With nothing to pin, Dependabot's only available action is raising the lower bound of an already-open range — which changes nothing about what installs and only drops support for older versions. It opened five such PRs within a minute of #206 merging (#208-#212), all no-ops: the existing ranges already resolved to exactly the versions being proposed as new floors. The one directory that did have a lockfile, constant-time-analysis, produced no PR at all, because there was genuinely nothing to update. That is the whole diagnosis. Lockfiles committed for the other four. .gitignore ignored uv.lock globally, which is why they were missing; constant-time-analysis's was tracked only because it predates the rule. Now scoped to the root file (ephemeral — there is no root pyproject.toml) with plugin lockfiles explicitly allowed, matching the pattern already used for .mcp.json. Also fixes two bugs #206 introduced: - The version-increment check failed all five Dependabot PRs, and Dependabot can neither bump a plugin version nor label its own PR, so every future dependency PR would have been permanently red. Exempted by actor. - The 'no-version-bump' label was documented in AGENTS.md and wired into validate.yml but never created, so the escape hatch did not exist. Created. * Re-run CI with the no-version-bump label applied The version-increment check fired on this PR: adding uv.lock under plugins/<name>/ counts as touching those plugins. Correct behaviour — the lockfiles pin exactly what the existing ranges already resolve to, so nothing changes for anyone installing these plugins, which is what the label is for. First real use of the escape hatch created in this same PR. * Fix the three findings from this PR's review A local uv setting leaked into all four new lockfiles. /etc/uv/uv.toml on ToB machine images sets exclude-newer = "1 week", so every lock carried an [options] block with exclude-newer-span = "P1W" and pinned versions resolved a week stale — diverging from constant-time-analysis/uv.lock, which predates this PR and has no such block. Regenerated with UV_NO_CONFIG=1. That cooldown is the org's supply-chain posture and it belongs in dependabot.yml's 'cooldown: default-days: 7', where it already is; baking it into committed lockfiles was my environment leaking, not a decision. "EVERY directory here must carry a committed uv.lock" was enforced by a comment, which is precisely the anti-pattern AGENTS.md tells people to avoid. Now a validator check: it parses the uv ecosystem block out of dependabot.yml and asserts a uv.lock beside each listed directory. Scoped to that block rather than grepping for '- /plugins/...' so a future ecosystem's paths are not swept in, and it errors if the block exists but no directories parse out — otherwise the checker could inspect zero items and report clean, which is the exact failure it exists to prevent. Three self-test fixtures, and verified by deleting a real lockfile and confirming CI would go red. The Dependabot exemption keyed on github.actor, which on a synchronize event is whoever pushed. A human adding one commit to a Dependabot branch would re-arm the version check and turn the PR red — making the follow-up bump mandatory exactly where the comment says it is discretionary. Keyed on PR authorship now. |
||
|
Dan Guido
|
8ea3b6a700 |
Move the contribution checklist into machinery (#206)
* Add validator self-test, structural checks, and make check The repo documented ~53 rules in AGENTS.md and machine-enforced 6 of them. This closes the gap for the ones a machine can decide, and adds the guard that keeps the checkers honest. New error-level checks (all currently pass, so none of this blocks anyone today): agent files must use `tools:` while skills use `allowed-tools:` (the loader silently ignores the wrong key, so the restriction just does not apply); subagent_type must be namespaced or the dispatch fails at runtime; plugin dir names kebab-case and <=64 chars; plugin README present, listed rather than stat'd so `Readme.md` fails on Linux CI the way it should; semver format; the forbidden runtime sidecars AGENTS.md already banned but nothing checked; and version-increment against the base branch, which is the gap that let |
||
|
Evan Sultanik
|
0cb5f95840 |
Add open-sourcing plugin (#202)
* Add open-sourcing plugin Skill for preparing a repository for public release, generalized from the internal open-sourcing guide. Generic workflow (secrets audit, licensing, docs, CI, release automation) with a Trail of Bits policy overlay loaded via git-remote/committer-email detection. Includes per-language packaging references and readiness-check scripts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Modernize open-sourcing skill to current toolchain practice Align guidance with the current cookiecutter-python toolchain and 2026 ecosystem state, verified against primary sources: respect-existing- tooling principle (warn on stale toolchains, adopt modern defaults only when absent), SLSA provenance job in the PyPI release workflow, ty pre-1.0 pinning caveat, uv audit/interrogate, rulesets wording, Dependabot grouping and cooldown, and a new JavaScript/TypeScript reference covering npm trusted publishing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
Evan Sultanik
|
65ecafa7ec |
Add github-triage plugin (#192)
* Add github-triage plugin Triages open GitHub issues for the current repository via the gh CLI: closes already-resolved issues with comments citing the resolving PR or commit, cross-links issues with pending fix PRs, and assigns local-only priority and change-size (size/XS–XXL) estimates for everything else. All GitHub writes are gated behind a single review-and-iterate approval; priority/effort are never posted. Registers the plugin in the marketplace, README, and CODEOWNERS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add optional PR triage to github-triage skill Before issue triage, optionally clear open PRs (so merges feed the "already resolved" issue check): - incrementally merge allowlisted bot PRs and maintainer-approved PRs, one at a time, re-verifying mergeability/CI before each and confirming each landed; - spawn one read-only review subagent per never-reviewed PR, saving each review to github-pr-<number>-review.md locally (never posted). PR readiness uses verified gh --json semantics: mergeStateStatus==CLEAN (MERGEABLE alone is insufficient; UNKNOWN is never-merge), per-node statusCheckRollup (CheckRun status+conclusion vs StatusContext state), author.is_bot + trusted allowlist, and latestReviews state+authorAssociation rather than the branch-protection-driven reviewDecision. All merges gated; never --auto/--admin/force. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix github-triage bugs found in end-to-end dry run Validated the skill against trailofbits/graphtage (real issues + PRs), which surfaced four bugs: - gh repo view takes the repo positionally, not -R; fixed the default-branch and merge-method lookups. - CI readiness treated NEUTRAL/SKIPPED checks as failures, wrongly blocking mergeable Dependabot PRs (CLEAN with a NEUTRAL CodeQL run). Reworked to "CI not blocking" (hard failures + pending only), with mergeStateStatus==CLEAN as the authority. - Not-ready bot PRs were routed to the review-subagent bucket; bots are now excluded so they fall to Needs work. - Bot allowlist now normalizes gh's author.login renderings (app/dependabot and dependabot[bot]) so bot detection actually matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
Scott Arciszewski
|
841bffed0d |
trailmark: add v0.4/v0.5 support and graph-assisted security workflows (#183)
* trailmark: update skills to use v0.4.0 features * fix: resolve code review findings for PR #183 Redraws the version boundary to match the actual Trailmark release history, verified against the v0.2.0/v0.2.1/v0.2.2/v0.3.1/v0.4.0 tags of trailofbits/trailmark: P1 (misclassified APIs, fixed): - ancestors_of(), reachable_from(), entrypoint_paths_to(), nodes_with_annotation(), clear_annotations() and the diff/entrypoints CLI exist since v0.2.0 — moved from the v0.4+ list to the v0.2-safe baseline; removed needless hasattr() gates and degraded fallbacks - supported_languages()/detect_languages() (trailmark.parse) are 0.3+ modules, not v0.2-safe — annotated as such - CLI --version/version were added in 0.2.2, not 0.4 — documented as 0.2.2+ and version-probe failure semantics clarified - graph-evolution/SKILL.md reverted to main: native diff has existed since v0.2.0 with identical args, so the 0.2.x-fallback rewrite was built on a false premise (also resolves the quality-checklist contradiction flagged in review) P3 (fixed): lexical version comparison hazard noted; stale diagramming-code checklist label; README baseline list now defers to the SKILL.md Version Gate instead of keeping a second divergent copy Dismissed: 'diff CLI signatures contradict' (both forms valid — before/ after positionals accept paths or git refs, --repo/--json exist since 0.2.0); 'subgraph_edges has no edge_kinds param' (v0.4.0 signature has edge_kinds keyword); 'diagram.py fallback broken on 0.2.x' (trailmark.diagram module exists in 0.2.x; only the CLI subcommand is new); 'phantom diff_against()' (real, v0.2-safe, now in baseline) Verified: check_claude_loadability.py, check_codex_loadability.py, pre-commit hooks pass; all SKILL.md files under 500 lines Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * trailmark: add assurance workflow skills (#187) * trailmark: add assurance workflow skills * fix: drop hasattr gates on v0.2-baseline query APIs entrypoint_paths_to() and reachable_from() are in the v0.2-safe baseline, so the hasattr fallbacks were dead code — and the entrypoint_paths_to fallback indexed attack_surface() entries with entry["name"] instead of node_id, which would raise KeyError. Addresses PR #187 review feedback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(trailmark): make detect_languages import v0.2-safe in operational skills The trailmark-summary and trailmark-structural skills claimed v0.2-safe workflows but unconditionally imported trailmark.parse, a 0.3+ module. detect_languages() has existed in trailmark.query.api since v0.2.0 (kept as a deprecated alias in 0.3+), so gate the import with a fallback instead of relabeling the skills as 0.3+. Version Gate docs updated to document the v0.2-safe import path; supported_languages() remains 0.3+ with no 0.2.x equivalent. Verified against trailmark v0.2.0 source (fallback branch) and the current 0.4 line (canonical branch). Addresses review feedback on PR #183. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * trailmark: document and enable v0.5.0 features Trailmark 0.5.0 adds a PostgreSQL-oriented sql parser (node kinds schema/table/view/procedure), the stable .trailmark/links.toml configuration for cross-language/FFI/RPC/external links, repository links/proxies/type_uses edges on single-language parses, Solidity entrypoints from parser metadata (visibility/mutability/overridden-by attributes), node attributes in attack_surface() entries, TypeScript constructed-receiver resolution, and C# file-scoped namespaces. Updates the Version Gate with a v0.5+ section and a structural probe ('SCHEMA' in NodeKind.__members__ — 0.5.0 adds no new QueryEngine methods, so hasattr() cannot detect it), adds a Repository Links section and cross-boundary query recipe, extends the parser list and graph model docs, notes the reachability-vs-taint limitation, and threads the 0.5 output additions through trailmark-structural and audit-augmentation. Plugin version 0.9.0 -> 0.10.0. All version claims verified against trailofbits/trailmark v0.2.0 and v0.5.0 builds, including a live links.toml materialization test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * trailmark: add slicing-code-context skill (#203) Adds a skill for delegating focused code tasks to constrained or locally hosted models using bounded, graph-informed Trailmark source packets: - scripts/build_slice_packet.py: deterministic PEP 723 packet builder (Trailmark 0.5.x) with five selection modes, whole-unit budget admission, explicit omission accounting, path-traversal rejection, an embedded untrusted-source notice, and structured JSON errors (26 tests, including a real Trailmark integration test) - agents/code-slice-worker.md: repository-tool-free Haiku worker returning a source-cited JSON contract - SKILL.md + references/slice-packet.md: coordinator workflow, packet and worker response contracts, and validation rules Reviewed with a multi-agent Claude pass and two Codex passes; fixes from those reviews are included (doc/selection-order reconciliation, relationship deduplication, line-range anchors no longer expand to full nodes in path/entrypoint modes, background-safe worker toolset, structured io_error handling, replacement-packet expansion semantics). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
dm
|
101a102069 |
Add vulnerability triage brocards skill (#201)
Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
dependabot[bot]
|
587160fccc |
build(deps): Update opencv-python-headless requirement (#198)
Updates the requirements on [opencv-python-headless](https://github.com/opencv/opencv-python) to permit the latest version. Updates `opencv-python-headless` to 5.0.0.93 - [Release notes](https://github.com/opencv/opencv-python/releases) - [Commits](https://github.com/opencv/opencv-python/commits) --- updated-dependencies: - dependency-name: opencv-python-headless dependency-version: 5.0.0.93 dependency-type: direct:production dependency-group: all ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
dependabot[bot]
|
83f9240beb |
build(deps): Bump the all group across 1 directory with 2 updates (#204)
Bumps the all group with 2 updates in the / directory: [actions/checkout](https://github.com/actions/checkout) and [actions/setup-python](https://github.com/actions/setup-python). Updates `actions/checkout` from 7.0.0 to 7.0.1 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0...3d3c42e5aac5ba805825da76410c181273ba90b1) Updates `actions/setup-python` from 6.2.0 to 7.0.0 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/a309ff8b426b58ec0e2a45f0f869d46889d02405...5fda3b95a4ea91299a34e894583c3862153e4b97) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all - dependency-name: actions/setup-python dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: all ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
William Tan
|
3b316e6ac7 |
fix(modern-python): suggest exact uv run python so shim advice works outside projects (#196)
* fix(modern-python): suggest exact `uv run python` so the advice works outside projects The python/python3 shim suggested `uv run $cmd ...`, echoing back whichever name was invoked. For `python3` that advice is self-defeating on machines with no uv-managed interpreters: uv resolves the `python3` command through an ordinary PATH lookup, which hits the shim again and fails with the same suggestion. uv special-cases the exact command name `python` (uv >= 0.4.0) and executes its resolved interpreter directly, so always suggesting `uv run python ...` works everywhere. Reproduced on stock Debian + uv 0.11.27 (apt python3, zero managed pythons, no project): `uv run python3 script.py` fails via the shim while `uv run python script.py` succeeds, across script/-c/-m/REPL forms. Reported in #195. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(modern-python): satisfy shellcheck SC2016 in new bats assertions Escaped backticks in double quotes instead of literal backticks in single quotes, which shellcheck flags as a possible unintended non-expansion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(modern-python): requote shim suggestions and carry all arguments through Review findings on #196: the -m branch interpolated only the module name, so `python -m http.server 8000` suggested a command missing the port, and `${*}` flattened arguments without quoting, so `python -c 'print(1+1)'` suggested a command that is a bash syntax error if run verbatim (plus a trailing space inside the backticks for bare invocations). Both branches now build the suggestion from %q-requoted arguments, with regression tests for each case. Also consolidates the exact-`python` rationale into a single canonical copy in the shim's header comment; README, setup-shims.sh, and the bats file now point there instead of paraphrasing it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Andrea Cappa (zi0Black)
|
cfe5d7b161 |
Rust review plugin (#178)
* rust-review: add Rust security review plugin Add the rust-review plugin: a comprehensive Rust security review skill with clustered finders covering memory safety, concurrency/data races, panic-induced DoS, FFI/cross-language boundaries, error handling, resource handling, async runtime, and static hygiene. Includes worker, dedup-judge, fp-judge, and planner agents, SARIF generation with rule descriptions and regression tests, deterministic cluster chunking, and Codex skills mapping. Versioned at 1.0.0 and registered in the marketplace, CODEOWNERS, and root README. * c-review: backport rust-review protocol fixes and planner chunking Port the language-agnostic fixes made while building rust-review (which was ported from c-review) back into c-review: - worker/fp-judge: force findings, coverage gate, and REPORT.md to disk via Write instead of returning content in the reply (orchestrator context-bloat hardening); add a pre-complete file-existence check. - worker: move the cache-primer block below the normal self-check and pre-work budget so a non-primer worker does not start under a global "no tool calls" rule. - planner: add --max-passes-per-worker (default 4) with deterministic split_oversized_clusters chunking; skill passes the flag and documents the chunked-subset worker rule. - scripts: add test_split.py and test_generate_sarif.py regression tests. The SARIF test caught a missing RULE_DESCRIPTIONS entry for uninitialized-data, now added. Bump c-review to 1.2.0. * c-review/rust-review: validate artifacts, index-aware SARIF, protocol cleanups - Add validate_artifacts.py (+ tests) to both plugins to check worker shard, coverage, and finding files before accepting completions. - generate_sarif.py now reads the canonical findings-index.txt when present, falling back to findings/*.md only if the index is absent. - Merge the worker step-6 verification paragraphs and drop orchestrator -internal Phase 7 / plan.json jargon in favor of worker-facing stakes. - Tighten uninitialized-read-finder guidance: primitive integers still require initialization. * rust-review/c-review: per-cluster max_passes_per_worker override Lets output-heavy clusters declare a smaller manifest-level max_passes_per_worker so each expensive pass group gets its own worker, validated by a single shared cluster_max_passes_per_worker helper and honored by split_oversized_clusters via an explicit override (0 is rejected rather than silently falling back to the global cap). rust-review opts in concurrency-locking and recursion-dos; c-review ports the capability for parity. validate_artifacts now accepts grouped or repeated --claimed-count values. * rust-review: broaden bug-class coverage with capability-gated clusters Add layout-safety, input-os-safety, and info-disclosure clusters behind new has_packed_repr / has_fs_io capability gates so packed-repr, path, and pointer-exposure passes only run where they apply, and gate unsafe-only passes behind has_unsafe to cut noise on safe crates. Extend existing clusters with new bug classes: RefCell double-borrow panics, unflushed BufWriter, string-comparison bypasses, serialize_struct mismatches, nondeterminism, in-collection key mutation, and destructor-skip cleanup leaks. Fix detector regexes that missed or over-matched real Rust (packed-field borrows, RefCell try_borrow_mut, HashMap substrings, path push, packed inner attrs, fs/path probes) and add a regression test pinning them to snippets. * fix dedup * safety-net check for REPORT.md * on-disk data -> shards reconciliation * on-disk data -> shards reconciliation - v2 * ls -> glob * memory-safety gate * path validation * fix numbers/counting * rm PACKEDREF from FFI cluster prompt, it is in layout-safety * fix unsafe-boundary count * minor fixes for prompts * do not filter unknown-severity findings, just mark them as such * fix minor behavior changes in worker * Correctness: - generate_sarif: clamp startLine >=1 (`:0` produced schema-invalid SARIF) - generate_sarif: don't drop a judged survivor with blank severity - dedup-judge: Tier-2 carry-forward so a primary can't be demoted/orphaned - dedup-judge: crash-recovery unions shards with findings/*.md (empty-shard trap) Robustness: - generate_sarif: skip frontmatter-less files; add originalUriBaseIds Contracts: - SKILL: gate dedup-judge before fp-judge (prevent concurrent-spawn race) - worker: verbatim coverage cells; sub_prompt_paths omitted-not-empty; skip_subclasses reserved; Codebase comma format * improve prompts regexes, add missing deconflictions * prompt factual fixes * fix dozen of small prompt inconsistencies and add missing sections * more prompt fixes, fix retry guard in SKILL, small fixes in agents * dozen more small fixes * final regex fixes * fixes from rust to c-review * agents cannot use write tool for reports (strange cc limitation) - bypass via bash * spawnings agents is capped to 20 - explicit handling for that * fix glob -> read (glob is blocked for agents that has also bash) * fix regex patterns to work with grep * soften output requirements - they were violated anyway * consolidated clusters are no longer chunked — one worker owns the whole cluster, builds its shared Phase-A inventory once, and runs every phase * fix judge finding counting and low-severity guidance * fix metadata * small fix for skipped findings * Carry forward guard for `also_known_as` bucket * Gracefully handle parse_frontmatter error * Extend has_ffi coverage * Broader gate for has_concurrency * Update FFI-safe layout regex to support C, C+packed, and C+u32 in unsafe-boundary and dyn-trait-ffi-finder prompts * Small refine of regex patterns * Improve regex patterns for recursive type detection to include Mutex and RwLock * rm global .codex/rust-review * backport fixes to c-review * merge changes * Backport SARIF merge-survivor + malformed-frontmatter guards to c-review, mark missing locations, fix prompt-regex test extractor, and harden planner/validator scripts across both review plugins * fix pytest * fix global gitignore, adds / and ruff_cache * small fixes from pr-review * small fixes from pr-review - 2 * fix copilot finding --------- Co-authored-by: GrosQuildu <e2.8a.95@gmail.com> |
||
|
dependabot[bot]
|
39e10bd31e | build(deps): Bump actions/checkout from 6.0.3 to 7.0.0 in the all group (#191) | ||
|
Jay Little
|
ff4162dcb9 | Update fp-check with links and best practices update (#189) | ||
|
jonnydevs
|
c070b9b588 |
fix(fp-check): use correct JSON response format in stop hooks (#129)
* fix(fp-check): use correct JSON response format in stop hooks
Prompt-type stop hooks must respond with JSON. The previous prompts
instructed Claude to return plain text ('block' or 'approve'), causing
'Stop hook error: JSON validation failed' on every session end.
Updated both Stop and SubagentStop hook prompts to respond with:
- {"decision": "block", "reason": "..."} to prevent stopping
- {} to allow stopping (omitting decision field per Claude Code docs)
Bug discovered by Claude while debugging the JSON validation error
during active use of the fp-check skill.
* fix: use documented ok/reason schema for prompt hooks, bump to 1.0.2
Per https://code.claude.com/docs/en/hooks.md (Prompt-based hooks >
Response schema), prompt hooks must respond {"ok": true} to allow or
{"ok": false, "reason": "..."} to block — not {"decision": "block"}
or {}. Also bump to 1.0.2 since main already shipped 1.0.1 without
this fix; clients only update when the version increases.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Dan Guido <dan@trailofbits.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Matthew Kern
|
64a8c3f00b |
Update second-opinion Codex model to gpt-5.5 (#163)
* Update second-opinion Codex model to gpt-5.5-codex Bump primary model from gpt-5.3-codex to gpt-5.5-codex and fallback from gpt-5.2-codex to gpt-5.4-codex. Plugin version 1.6.0 → 1.7.0. * fix: correct Codex model name from gpt-5.5-codex to gpt-5.5 --------- Co-authored-by: Dan Guido <dan@trailofbits.com> |
||
|
Jamie Reid
|
5577119331 |
fix(semgrep-rule-creator): correct 404ing semgrep-docs links (#179)
* fix(semgrep-rule-creator): correct 404ing semgrep-docs links The semgrep-docs repo migrated these writing-rules pages from .md to .mdx, so the WebFetch links in SKILL.md were returning 404. Update the five affected links to their .mdx paths (pattern-syntax was already .mdx). All seven links now return HTTP 200. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Bump version to 1.2.2 in plugin.json * Update semgrep-rule-creator version to 1.2.2 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: ahpaleus <38883201+ahpaleus@users.noreply.github.com> |
||
|
dependabot[bot]
|
c910ecf739 |
Bump actions/checkout from 6.0.2 to 6.0.3 in the all group (#182)
Bumps the all group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 6.0.2 to 6.0.3 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/de0fac2e4500dabe0009e67214ff5f5447ce83dd...df4cb1c069e1874edd31b4311f1884172cec0e10) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 6.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Lixin2026
|
d5fe2e6a78 |
feat(codex): add UI metadata for skills (#175)
* feat(codex): add skill UI metadata * Use official Trail of Bits logo * fix: resolve code review findings for PR #175 Codex silently drops the icons as authored: its loader (codex-rs/core-skills resolve_asset_path) requires icon paths containing '..' to resolve under <plugin_root>/assets/, and the repo-root .codex/assets location fails that containment check. Verified empirically via codex app-server plugin/read: every iconSmall/iconLarge came back null; only brand_color applied. P1 fixed: - Vendor trail-of-bits-mark.svg into plugins/<name>/assets/ for all 38 plugins with skills and point every openai.yaml at ../../assets/trail-of-bits-mark.svg (the supported plugin-level shared asset pattern). Icons now resolve for marketplace installs too, since nothing escapes the plugin root. - Drop the .codex/ additions: .codex/skills/gh-cli/agents/ openai.yaml resolved nowhere (.codex/skills is not a Codex discovery root) and PR #173 removes the whole .codex/ tree P2 fixed: - Patch-bump all 38 touched plugins in plugin.json and marketplace.json so installed clients pick up the metadata Verified: - Static check replicating Codex's resolution algorithm: all 73 yaml files resolve under their plugin assets/ and exist - Live codex app-server probe: 71/72 loadable skills report resolved iconSmall/iconLarge and brand_color #D83A34 (claude-in-chrome-troubleshooting fails to load on main due to a pre-existing 64-char qualified-name limit, fixed by #173's rename; zeroize-audit's manifest mcpServers object is likewise a pre-existing Codex incompatibility fixed by #173) - validate_codex_skills.py, validate_plugin_metadata.py, prek all pass Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(codex): use skill-local icon assets --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
William Tan
|
f09e5c729a |
Remove legacy codex compatiblity scripts/shims. (#173)
* Remove legacy codex compatiblity scripts/shims. Codex supports claude plugins so this shouldn't be necessary. Add a script to test the plugin loadablility in both claude and codex * fix: resolve code review findings for PR #173 Review findings addressed (4 reviewers: pr-review-toolkit agents, Codex gpt-5.3-codex, direct diff review): P2 fixed: - Bump versions for the 5 substantively changed plugins in both plugin.json and marketplace.json (gh-cli 1.5.0 new skill, claude-in-chrome-troubleshooting 1.1.0 skill rename, modern-python 1.5.1 / skill-improver 1.0.3 hooks change, zeroize-audit 0.1.1 MCP config relocation) so clients pick up the changes - README Codex install: replace unpasteable /plugins slash-command block with verified CLI syntax (codex plugin marketplace add) - check_claude_loadability: parse_json_output now fails fast with command context on empty CLI output instead of returning None - check_codex_loadability: surface skipped RPC error messages in timeout failures instead of a bare TimeoutError P3 fixed: - Both checkers: error out when marketplace.json lists no plugins instead of passing vacuously Dismissed: - @latest CLI installs in validate.yml: deliberate; the check validates against the clients users actually run - select.select portability: CI-only script on ubuntu-latest - Divergent mcpServers validation between checkers: intentional; the Codex checker enforces the repo's .mcp.json convention Verified: ruff, prek, validate_plugin_metadata.py, and both loadability checks pass end-to-end (39 plugins, 74 skills, 2 MCP servers load in Claude Code and Codex) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Tim Lander
|
c94841be3d |
Fix for when CLAUDE_PLUGIN_ROOT contains spaces (#169)
* Fix for when CLAUDE_PLUGIN_ROOT contains spaces * chore(skill-improver): bump version to 1.0.2 for hooks.json fix The space-quoting fix to hooks.json is a behavioral change, but clients only pull plugin updates when the version increases. Bump plugin.json and the root marketplace.json from 1.0.1 to 1.0.2 (kept in sync) so existing users actually receive the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dan Guido <dan@trailofbits.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |