feat(scan): per-finding evidence (matched text) — phase 2 of #80#81
Conversation
…hits Phase 2 of #80. Bumps redstamp pin to cf2ce75 (phase-1 merge: detectors now return hits[]). scanSkill already passes redstamp finding objects through, so 'hits':[{flag,match}] now flow into 'scan --json' — the exact substring each flag matched. Additive; verdicts unchanged. Suite 150/150 + evidence-flow test. Next: watch publishes evidence[] into results.json + re-reads pinned bytes to verify each match (drops unverifiable, counts evidenceMismatches — confab guard).
Phase 3 of #80. support/evidence.mjs: locate each redstamp hit's matched substring in the pinned source pieces, returning {file,line} — and DROP any that don't exist in the bytes (counted as evidenceMismatches). marketplace-watch attaches evidence:[{flag,text,file,line}] to advisoryDetail/acceptedDetail/flagged and publishes evidenceMismatches in the summary. The self-check IS the anti-confabulation guard: the feed (and site) can only show a fragment that provably exists at the linked line; a nonzero mismatch count means the detector claimed a match the source lacks — surfaced, not hidden. Tests: evidence module (locate/self-check/cap) + integration (scan→evidence→true line). Watch test stages evidence.mjs. Full suite 155/155. Manual run verified: results.json carries evidence @ real file:line, evidenceMismatches 0.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
sprayberry-reviewer
left a comment
sprayberry-reviewer
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Verdict: No blocking findings — approving. Clean, well-guarded change with a genuine confabulation safeguard; one non-blocking nit noted below.
What I checked
gh pr diff 81(full unified diff, all 7 changed/added files)support/evidence.mjs+support/marketplace-watch.mjsagainst surrounding context in a shallow clone (src/skill.mjs,src/scan.mjs,src/index.mjs,src/claude.mjs) to confirm wherescanPieces/hits/matchactually originategh pr checks 81— all 9 checks pass (CodeQL, analyze, test × 6 platforms, verify pinned skills)
Security review (the point of this PR — evidence text leaking secrets)
Traced the data flow: evidenceOf(items, skill) in support/evidence.mjs:27-38 only ever publishes h.match after locate() (support/evidence.mjs:20-25) confirms that exact substring exists verbatim in skill.scanPieces[].text — which is the static file content of the scanned marketplace skill itself (src/skill.mjs:108-114, read via fs.readFileSync), never process.env or any runtime secret value. So a hit like ${API_KEY} in the test (support/evidence.test.mjs:17) is the literal string as it appears in the skill's own SKILL.md/description — evidence of a mention, not an expanded secret. Nothing in this diff reads or forwards actual environment/credential values. An unverifiable hit (redstamp claims a match the bytes don't contain) is dropped and counted, not published (support/evidence.mjs:33-36) — this is a solid anti-confabulation guard and matches the PR's stated intent.
Correctness
No issues found in the changed logic. locate() correctly computes 1-based line numbers via a leading-newline count, empty/non-string matches are skipped before lookup (evidence.mjs:31), and the EVIDENCE_CAP truncation is tested for both the capped and uncapped paths.
Minor (non-blocking)
support/evidence.mjs:12—cap()truncates withs.slice(0, EVIDENCE_CAP - 1), a plain UTF-16 code-unit slice. If a matched string's 160-code-unit boundary lands inside a surrogate pair (e.g. an emoji or other astral character in the matched text), the truncated output would contain a lone surrogate before the…. Low likelihood given typical injection-prompt text, and not something the tests exercise — flagging only as a nit, not a blocker.
What's good
- The verify-before-publish design (
locate→ drop-if-unverifiable → count asmismatches→ surfaced in the publishedsummary.evidenceMismatches) is exactly the right shape for a scanner that must not let a detector's claim become an unverified public assertion. - New tests (
support/evidence.test.mjs,evidence-flow.test.mjs) cover the verified-hit path, the dropped/mismatched path, the length cap, and an end-to-end real-skill-dir integration case that checks the actual line number — solid coverage for new behavior. - CI is fully green across all platforms plus CodeQL.
…solve (#82) Phase 2/3 (#81) published evidence.file relative to the skill's own directory (what scanPieces uses), e.g. bare "SKILL.md" — but the site resolves a per-plugin source link from the plugin's REPO root, and a skill is usually nested under it (skills/<name>/…). A bare skill- relative file can't be appended to that link to build a working github.com/.../blob/<sha>/<file>#L<line> deep link. Thread through skillPath (the skill's own dir relative to the repo root discoverMarketplaceSkills resolved it under) and join it onto each evidence entry's file before publishing, in both corpus and legacy scan modes. Closes #80 (phase 4's actual requirement — the tree/ -> blob/#Lline swap — needs this on the data side; site-side render in a truecopy-site PR).
#83) #81 and #82 shipped user-facing behavior with no changelog entry: `scan --json` findings/advisories now carry `evidence[]`, and the watch publishes `evidenceMismatches` plus repo-relative evidence paths. Both belong under Unreleased so the next release notes are complete. Documentation only — no code or behavior change.
3 participants
Phase 2 of #80. Bumps redstamp pin to
cf2ce75(phase-1 merged: detectors returnhits[]).scanSkillpasses redstamp findings through, sohits:[{flag,match}]now appear inscan --json— the exact substring each flag matched (e.g.instruction-override → "ignore previous instructions"). Additive, verdicts unchanged. 150/150 + evidence-flow test.Next (phase 3):
marketplace-watch.mjspublishesevidence[]intoadvisoryDetail/acceptedDetail/flagged+ re-reads the pinned bytes to verify each match exists at file:line (drops unverifiable + countsevidenceMismatches— the confabulation guard). Then the site renders it.🤖 Generated with Claude Code