Skip to content

Give the legacy audit caller evidence and keep the default audit to verdicts - #26

Open
ccarpene-blk wants to merge 1 commit into
mainfrom
fix/legacy-audit-evidence
Open

ccarpene-blk wants to merge 1 commit into
mainfrom
fix/legacy-audit-evidence

Conversation

@ccarpene-blk

Copy link
Copy Markdown
Collaborator

The legacy audit used by the default prompt set saw only the files its findings cited. A route guard or middleware registered beside a handler was invisible to it, so a claim such as a missing authorization check could be confirmed even though the guard exists in a file the auditor never received. The same audit could also return new findings of its own, which bypassed discovery and arrived without the batch context needed to avoid repeating other batches' claims.

Caller evidence for every audit batch

  • Retrieval now runs for every audit batch, legacy or v2 (internal/cli/scan_audit.go:171). Each batch receives up to about 24K tokens of excerpts with real line numbers, taken round-robin across the batch's claims and never repeating a file the batch already includes.
  • References to the flagged function are retrieved with route registrations first. Local definitions of other names in the same statement, such as middleware registered with the handler, are added up to eight per claim (internal/auditctx/auditctx.go:102, :498).
  • A legacy batch shrinks the retrieval budget so the prompt fits the model's context limit after reserving output tokens (internal/cli/scan_audit.go:238). If the rendered context would still exceed the limit, the batch is sent with an omission note in place of the excerpts (:265). The claims' own files are always sent.
  • When retrieval leaves out context that may decide a claim, the batch says so in plain text, for example Audit context omitted: ambiguous caller reference <name> in <file> or Audit context omitted: unresolved or external caller reference <name> in <file>, so a guard that was not retrieved is not mistaken for a guard that does not exist. @/ and ~/ aliases and directory index files resolve to scanned files; other import prefixes are treated as external. The notes are listed in README.md:864.

Audit additions in the default set

  • Audit prompts gain an allow_new_findings setting (internal/llm/prompt.go:87, :797-798). When it is false, the audit request uses a schema whose new_findings array is described as required to be empty (internal/llm/schema.go:175-178); the array is kept for provider compatibility. Any additions a model returns anyway are discarded before verdicts are applied, and their count is recorded as execution.auditAdditionsDropped in the run metadata (internal/cli/scan_audit.go:375, internal/sarif/metadata.go:102), omitted when zero.
  • prompts/default/audit.yaml:1 sets allow_new_findings: false. Sets that omit the key keep the previous behavior, and a v2 audit has no additions, so the setting has no effect there.
  • README.md:322 and the execution row of the SARIF property table document the setting and the counter.

Release note

  • A scan with the default prompt set no longer reports audit-originated findings (auditStatus: "new"). Its audit only issues verdicts on analysis findings. Other v1 prompt sets are unchanged unless they set allow_new_findings: false.
  • Legacy audit batches now include retrieved caller context, so their prompts are larger and their verdicts can change where a guard or middleware is visible in a caller.

Validation

  • internal/auditctx/caller_regression_test.go covers mounted and bare middleware references, Go wrappers, neighbouring registrations that must not be pulled in, external imports that must not resolve to a local namesake, import-path boundaries, omission notes and their deduplication.
  • TestLegacyAuditRetrievesMountedGuardWithinBudget (internal/cli/audit_caller_context_test.go:17) runs a legacy audit end to end and checks that the guard is retrieved within the context budget.
  • TestAuditDisablesUnsolicitedAdditions (internal/cli/audit_recovery_test.go) uses a model endpoint that ignores the empty-array instruction and checks the schema sent, the results kept and the dropped count with the setting both on and off; schema tests cover the existing-claims schema.
  • Both changes were also exercised with real-binary scans using the default prompt set against a local mock provider.
  • make build, go test -race -count=1 ./..., go vet ./... and git diff --check pass; lint is unchanged from main.

Merge order with the #18–#20 stack

This PR targets main and conflicts with the open audit-integrity stack (#18 onward) in the legacy audit context code. #19 moves source extraction into internal/sourceindex, so if this lands first, the stack needs the caller lookup ported into internal/sourceindex with the claim-citation step kept ahead of retrieval. That port has been prepared and validated locally (full race suite, vet and lint pass, with a regression test for cited ranges reaching the legacy audit prompt) and can be applied when the stack is restacked.

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.

1 participant