Skip to content

chore(audit): weekly cloud audit — scanner false-negatives + proposal prompt-injection guard#114

Open
tzone85 wants to merge 1 commit into
mainfrom
fix/audit-cloud-2026-07-06
Open

chore(audit): weekly cloud audit — scanner false-negatives + proposal prompt-injection guard#114
tzone85 wants to merge 1 commit into
mainfrom
fix/audit-cloud-2026-07-06

Conversation

@tzone85

@tzone85 tzone85 commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Weekly automated security/quality audit on main. Four parallel category audits (concurrency, silent failures, event-sourcing wiring, security) ran with adversarial verification of every candidate. Concurrency and event-sourcing wiring came back clean (68/68 event types are handled in Project()). Two real, subtle defects survived verification and are fixed here.

Findings

1. Security scanners silently pass when a scan never runs

File: internal/security/scanners.go · parseNpmAudit, parseSemgrep

Why it's a real bug: Scanner.Run discards the process exit code (out, _ := cmd.CombinedOutput()) and relies solely on a JSON parse error to mark a scanner failed. But two tools emit a valid-JSON error envelope when they fail to scan anything:

  • npm audit with no package-lock.json (a common state for a freshly-checked-out worktree) exits non-zero and prints {"error":{"code":"ENOLOCK",...}}. parseNpmAudit unmarshalled into a struct with only a Vulnerabilities field → nil map → ([], nil).
  • semgrep on a config/rule-fetch failure prints {"errors":[...],"results":[]}. parseSemgrep read only results([], nil).

In both cases RunScanners placed the scanner in ran (clean), not failed. Consequences: under security.require_scanners=true a story that should block on incomplete coverage does not; and vxd security scan --min ... reports a clean CI exit code when the dependency/SAST pass never inspected anything. This directly defeats the invariant stated in RunScanners' own doc comment — "a failed scan must never masquerade as a clean one."

Fix: parseNpmAudit returns an error when the top-level error key is present; parseSemgrep returns an error when it produced zero results but reported errors (a run with results plus a benign per-file parse warning is left alone so its findings still count). RunScanners then correctly classifies these as failed (coverage lost).

2. Proposal drafting feeds scraped text to the LLM without injection screening

File: internal/improve/proposal.go · DraftProposal, BuildProposalPrompt

Why it's a real bug: Opportunity records are built entirely from scraped third-party content — HN who-is-hiring comments (parseHNComment), Algora/arc.dev bounties and jobs (parseMarkdownBounties/parseMarkdownJobs). DraftProposal interpolated opp.Title/Company/Budget/Skills/Notes verbatim into a claude -p prompt with no injection screening and no data-boundary framing. The sibling LLM call sites in the same subsystem already guard: research.go runs DetectPromptInjection on every finding before the LLM, and analyzer.go/implementer.go wrap untrusted fields in <untrusted_content> tags. The proposal path did neither on its inputs; its only defense (sanitizeProposal) runs on the LLM output and merely prepends a warning — it does not stop injected instructions from steering the draft, which is a client-facing document.

Fix: DraftProposal now hard-rejects an opportunity whose free-text fields match a known injection pattern (mirroring research.go), and BuildProposalPrompt wraps each scraped field in <untrusted_content kind="..."> boundaries as defense in depth.

Tests added

  • internal/security/coverage_gaps_test.go: TestParseNpmAudit_ErrorEnvelopeIsAFailure, TestParseSemgrep_ErrorsNoResultsIsAFailure (error envelope → failure; clean/partial runs still pass).
  • internal/improve/proposal_injection_test.go: injection detected in each scraped field; clean opportunity passes; prompt wraps all fields in untrusted-content boundaries.

No new CLI commands or config fields, so no documentation-coverage changes are required (TestDocCoverage_* unaffected; security.require_scanners is already documented).

Verification

  • go build ./...pass
  • go vet ./...pass (whole tree)
  • go test ./internal/security/... ./internal/improve/...pass (the changed packages)
  • make doc-coveragepass
  • go test ./... -count=1 → the two changed packages pass. Three pre-existing, environment-caused failures remain in packages this PR does not touch, all confirmed identical on baseline main with these changes stashed: internal/figma/TestResolveToken_UnreadableFileIsDistinguished (0000-perm test is meaningless as root), internal/engine conflict-rebase tests (TestRebaseWithResolution_* — sandbox git config/version), and internal/repolearn/TestCountFilesInDir_NonexistentDir. Not introduced by this change.
  • golangci-lint and govulncheck could not run: both the pre-installed binaries and a fresh @latest build refuse the target — "the Go language version (go1.24/go1.25) used to build the tool is lower than the targeted Go version (1.26.4)". go1.26 is brand new and the lint/vuln tooling hasn't shipped a compatible release yet. This is an environment/toolchain-skew limitation, not a code issue; per the audit fallback, build + vet + test were used instead. This change adds no new dependencies, so it introduces no new vulnerability surface for govulncheck to assess.

🤖 Generated with Claude Code

https://claude.ai/code/session_014RfcAntkTRbDFA7iqvM27C


Generated by Claude Code

… prompt-injection guard

Two verified defects found by the weekly security/quality audit:

1. Security scanner false-negative (internal/security/scanners.go):
   parseNpmAudit and parseSemgrep accepted valid-JSON *error envelopes*
   (npm's {"error":{...}} on ENOLOCK; semgrep's {"errors":[...],"results":[]}
   on config-fetch failure) as zero-findings clean runs. RunScanners then
   classified the scanner as "ran clean" instead of "failed", so under
   security.require_scanners=true a story would NOT block on the coverage
   loss it is meant to block on, and `vxd security scan` CI exit codes would
   report clean when the scan never ran — defeating the module's stated
   invariant that "a failed scan must never masquerade as a clean one".
   Both parsers now detect the error envelope and return an error.

2. Proposal prompt-injection gap (internal/improve/proposal.go):
   Opportunity records are built entirely from scraped third-party content
   (HN who-is-hiring comments, Algora/arc.dev bounties+jobs). DraftProposal
   interpolated opp.Title/Company/Budget/Skills/Notes verbatim into a Claude
   prompt with no injection screening and no data-boundary framing — unlike
   the sibling research/analyzer/implementer paths, which run
   DetectPromptInjection and wrap untrusted text in <untrusted_content> tags.
   The only existing defense (sanitizeProposal) runs on the LLM *output*.
   DraftProposal now hard-rejects an opportunity whose free-text fields match
   an injection pattern, and BuildProposalPrompt wraps each scraped field in
   <untrusted_content> boundaries (defense in depth).

Tests: parseNpmAudit/parseSemgrep error-envelope regression tests
(coverage_gaps_test.go), opportunity injection screening + prompt framing
tests (proposal_injection_test.go).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RfcAntkTRbDFA7iqvM27C
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants