trailmark: add assurance workflow skills#187
Conversation
|
@claude review once |
| if hasattr(engine, "entrypoint_paths_to"): | ||
| entry_paths = engine.entrypoint_paths_to(node_id) | ||
| else: | ||
| entry_paths = [] | ||
| for entry in engine.attack_surface(): | ||
| for path in engine.paths_between(entry["name"], node_id): | ||
| entry_paths.append(path) | ||
| ``` |
There was a problem hiding this comment.
🔴 The new query-recipes.md and neighborhood-patterns.md reference docs feature-gate entrypoint_paths_to() and reachable_from() with hasattr(), but both methods are explicitly listed in the v0.2-safe baseline in plugins/trailmark/skills/trailmark/SKILL.md:84-91 — not v0.4+. The else branches are therefore dead on every supported Trailmark install, and the entrypoint_paths_to fallback is broken anyway: it iterates engine.attack_surface() and indexes each entry with entry["name"], but every other Trailmark reference in this repo (trailmark/references/query-patterns.md:38-39, genotoxic/references/graph-analysis.md:75) treats those entries as node_id-keyed, so on the v0.2 build the gate is meant to support the fallback would raise KeyError: 'name'. Fix: drop the hasattr gates for these two methods in both files (which deletes the broken fallback entirely); keep hasattr only for genuine v0.4+ APIs like connect_subgraphs, subgraph_edges, generic_parameters, type_references, and augment_binary.
Extended reasoning...
Where this fires
plugins/trailmark/skills/trailmark-finding-triage/references/query-recipes.md:30-37:
if hasattr(engine, "entrypoint_paths_to"):
entry_paths = engine.entrypoint_paths_to(node_id)
else:
entry_paths = []
for entry in engine.attack_surface():
for path in engine.paths_between(entry["name"], node_id):
entry_paths.append(path)And at query-recipes.md:68-71:
if hasattr(engine, "reachable_from"):
downstream = engine.reachable_from(node_id)
else:
downstream = []And a duplicate gate at plugins/trailmark/skills/trailmark-variant-neighborhood/references/neighborhood-patterns.md:32-33:
if hasattr(engine, "entrypoint_paths_to"):
paths = engine.entrypoint_paths_to(seed)Why the gates are wrong
The authoritative version-gate list lives in the same PR at plugins/trailmark/skills/trailmark/SKILL.md:84-91, which enumerates the v0.2-safe baseline:
QueryEngine.from_directory(), callers_of(), callees_of(), paths_between(), ancestors_of(), reachable_from(), entrypoint_paths_to(), complexity_hotspots(), attack_surface(), summary(), to_json(), preanalysis(), ...
Both entrypoint_paths_to() and reachable_from() are explicitly on that list. The genuine v0.4+ additions (per SKILL.md:98-101) are connect_subgraphs(), subgraph_edges(), generic_parameters(), type_references(), and augment_binary() — and query-recipes.md itself opens (line 4) with "Feature-gate optional Trailmark 0.4 APIs with hasattr()", so the file is self-contradictory.
Why the reachable_from else branch is harmful
When the (always-true) hasattr check ever returns False (e.g. a proxied/monkey-patched engine, an older 0.1.x install, a misconfigured test double) the else branch silently sets downstream = []. The very next paragraph instructs the reader to "Flag downstream sinks involving value transfer, authorization or role decisions, persistence or state writes, …". An empty downstream looks identical to "no downstream sinks found", which would push a triage verdict toward Deprioritize when it should be Promote or Needs manual review.
Why the entrypoint_paths_to else branch is broken
The fallback iterates engine.attack_surface() and indexes each entry with entry["name"], but every other Trailmark reference in this repo agrees that attack_surface() returns dicts keyed by node_id:
plugins/trailmark/skills/trailmark/references/query-patterns.md:38-39—for ep in engine.attack_surface(): print(f"{ep['node_id']}: {ep['trust_level']} ({ep['kind']})")plugins/trailmark/skills/genotoxic/references/graph-analysis.md:75—surface_by_id = {ep["node_id"]: ep for ep in engine.attack_surface()}plugins/trailmark/skills/trailmark-structural/SKILL.md:119uses the same shape.
Step-by-step proof for the KeyError
Assume a user follows this recipe against a Trailmark build where entrypoint_paths_to is somehow missing (the exact case the fallback exists for):
hasattr(engine, "entrypoint_paths_to")→False, so control flows into theelsebranch.engine.attack_surface()returns the standard list of dicts, e.g.[{"node_id": "src/api.py:handler", "trust_level": "untrusted_external", "kind": "http_route"}, ...](per the contract used by every sibling skill).- The loop body evaluates
entry["name"]on the first entry. - The dict has no
"name"key → Python raisesKeyError: 'name'beforepaths_betweenis ever called. - The fallback the gate was added to support crashes immediately, defeating its purpose, and teaches readers (and any AI agent copying the recipe) an incorrect API contract.
Suggested fix
Drop the hasattr gates for both v0.2-baseline methods in both files (deleting the buggy attack_surface() fallback entirely):
entry_paths = engine.entrypoint_paths_to(node_id)
...
downstream = engine.reachable_from(node_id)Keep hasattr only for actual v0.4+ APIs (connect_subgraphs, subgraph_edges, generic_parameters, type_references, augment_binary) — which is the pattern trailmark/SKILL.md and audit-augmentation/SKILL.md already model.
Summary
Combines the follow-up Trailmark assurance workflow additions from PRs #184, #185, and #186 into one PR on top of #183's
trailmark-0.4baseline.This intentionally uses a single minor version bump for the Trailmark plugin:
0.8.3 -> 0.9.00.10.0or0.11.0bumpsAdded skills
trailmark-finding-triage: graph-assisted triage for one candidate finding, SARIF result, weAudit annotation, suspicious function, or report excerpt.trailmark-review-gate: graph-level branch/PR/fix review gate for new entrypoints, tainted paths, privilege-boundary drift, blast-radius growth, and related structural regressions.trailmark-variant-neighborhood: graph-neighborhood expansion around a seed finding to produce ranked variant-analysis candidates.Other updates
plugins/trailmark/.claude-plugin/plugin.jsonand root.claude-plugin/marketplace.json.Base branch
This PR targets
trailmark-0.4because it depends on #183. After #183 merges, this PR can be retargeted tomainif needed.Verification
uv run python3 .github/scripts/validate_plugin_metadata.pyuv run python3 .github/scripts/check_claude_loadability.pyuv run python3 .github/scripts/check_codex_loadability.pygit diff --cached --checkbefore commitAll passed locally.