feat(detector): add a threat_detection_result reporting command to replace post-hoc transcript parsing#239
Open
Copilot wants to merge 3 commits into
Open
Conversation
16 tasks
Copilot
AI
changed the title
[WIP] Add threat detection result reporting command
feat(detector): add a threat_detection_result reporting command to replace post-hoc transcript parsing
Jun 1, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an in-session, out-of-band result reporting channel for the agentic CLI engine path, reducing reliance on post-hoc transcript scraping and enabling early termination once a validated verdict is recorded.
Changes:
- Adds an internal
report-resultsubcommand plus a provisionedthreat_detection_resultwrapper that records validated results to a per-run sink file. - Extends CLI-engine execution to watch the sink and cancel the subprocess as soon as a valid verdict is written (treating the resulting kill as success when a sink verdict exists).
- Adds detector helpers for atomic result-file write/read and updates prompts/docs to prefer the in-session command with a legacy transcript fallback.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the in-session threat_detection_result command, sink env var, and early termination behavior. |
| pkg/engine/tool.go | Adds wrapper provisioning (threat_detection_result) and sink polling cancellation logic. |
| pkg/engine/tool_test.go | Tests wrapper provisioning, cleanup, sink watcher cancellation, and sink-based error suppression. |
| pkg/engine/engine.go | Extends Engine.Analyze with sink options and implements early termination + killed-process suppression. |
| pkg/engine/engine_test.go | Updates Claude args tests and adds coverage for optional --allowed-tools Bash. |
| pkg/detector/result.go | Adds result-file helpers (atomic write/read) and report-field helpers. |
| pkg/detector/result_file_test.go | Adds tests for write/read roundtrip, error cases, and field validation helpers. |
| pkg/detector/prompts/threat_detection.md | Updates response format to call threat_detection_result once, with legacy fallback. |
| DEVGUIDE.md | Documents the new command, sink env var, early termination, and fallback parsing behavior. |
| cmd/threat-detect/report.go | Implements the internal report-result subcommand and validation/idempotency behavior. |
| cmd/threat-detect/report_test.go | Adds tests for valid write, invalid inputs, missing config, and idempotency. |
| cmd/threat-detect/main.go | Dispatches report-result before global flags; provisions sink and prefers sink result over transcript parsing. |
| cmd/threat-detect/main_test.go | Adds integration-style tests ensuring sink is preferred and early termination occurs. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 13/13 changed files
- Comments generated: 4
Comment on lines
+62
to
+65
| if resultFile == "" { | ||
| fmt.Fprintln(os.Stderr, "THREAT_DETECTION_RESULT_ERROR: no result sink configured; THREAT_DETECTION_RESULT_FILE is unset.") | ||
| return reportExitConfig | ||
| } |
Comment on lines
+86
to
+89
| if err := detector.WriteResultFile(resultFile, result); err != nil { | ||
| fmt.Fprintf(os.Stderr, "THREAT_DETECTION_RESULT_ERROR: failed to record result: %v.\n", err) | ||
| return reportExitConfig | ||
| } |
Comment on lines
+184
to
+188
| if r.Reasons == nil { | ||
| r.Reasons = []string{} | ||
| } | ||
| data, err := json.MarshalIndent(r, "", " ") | ||
| if err != nil { |
Comment on lines
+33
to
+50
| fs := flag.NewFlagSet("report-result", flag.ContinueOnError) | ||
| var ( | ||
| promptInjection bool | ||
| secretLeak bool | ||
| maliciousPatch bool | ||
| reasons stringSliceFlag | ||
| resultFile string | ||
| ) | ||
| fs.BoolVar(&promptInjection, "prompt-injection", false, "Whether a prompt injection threat was detected (required)") | ||
| fs.BoolVar(&secretLeak, "secret-leak", false, "Whether a secret leak threat was detected (required)") | ||
| fs.BoolVar(&maliciousPatch, "malicious-patch", false, "Whether a malicious patch threat was detected (required)") | ||
| fs.Var(&reasons, "reason", "Reason explaining a detected threat (repeatable)") | ||
| fs.StringVar(&resultFile, "result-file", os.Getenv("THREAT_DETECTION_RESULT_FILE"), "Path to the result sink file (defaults to env THREAT_DETECTION_RESULT_FILE)") | ||
|
|
||
| if err := fs.Parse(args); err != nil { | ||
| fmt.Fprintf(os.Stderr, "THREAT_DETECTION_RESULT_ERROR: %v. Re-run threat_detection_result with corrected values.\n", err) | ||
| return reportExitInvalid | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The agentic detection path relied on scraping a single free-form
THREAT_DETECTION_RESULT:{...}line out of the engine transcript after the subprocess exited. This is uncorrectable in-session (high false-positive parse failures, "multiple conflicting" hard errors) and gives the model no signal that it's done — so runs burn the full time/token budget waiting for the model to stop or for a timeout.This adds an in-session, validated, out-of-band reporting channel for the CLI engine path; the
/reflectstructured-output path and the legacy transcript line are unchanged.Reporting command
threat-detect report-resultsubcommand (cmd/threat-detect/report.go), dispatched inmain()before global flag parsing. Validates synchronously, writes canonical JSON to the sink fromTHREAT_DETECTION_RESULT_FILE, and prints a stop signal. Exit codes:0recorded,2invalid input,3no sink configured.THREAT_DETECTION_RESULT_ERROR: …and records nothing, so the model can fix and retry.Detector helpers (
pkg/detector/result.go)WriteResultFile(atomic temp+rename,0o600),ReadResultFile,BuildResultFromReport,ValidateReportFields— all reuse the existing schema/validateRawResult.ParseResultand its tests untouched.Engine provisioning + early termination (
pkg/engine)Engine.AnalyzegainsAnalyzeOptions{ResultSinkPath}.provisionResultToolwrites athreat_detection_resultwrapper ontoPATHand sets the sink env;watchResultSinkpolls the sink and cancels the subprocess the moment a valid verdict lands. A killed process is treated as success when a valid sink result exists.cmd.WaitDelaybounds stdout drain so cancellation isn't blocked by descendants holding the pipe.--allowed-tools Bashonly when the sink is enabled (headless mode won't run shell tools otherwise); engines that can't run the wrapper degrade to the legacy line.CLI wiring (
cmd/threat-detect/main.go)analyzeWithRetriesprovisions a per-run sink, prefersReadResultFileoverParseResult, and falls back to transcript scraping + the existing correction prompt only when the sink is absent/invalid.Prompt + docs
THREAT_DETECTION_RESULT_RECORDED, with the legacy line documented as a fallback. README/DEVGUIDE document the command, env var, and early-termination behavior.Open questions
--json/stdin) for engines that prefer emitting JSON over flags.--allowed-tools Bashfor Claude is unverified against a live Claude CLI; the line fallback covers it regardless.