You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
feat(detector): require forensic, locatable reasons for detections (#852)
* feat(detector): require forensic, locatable reasons for detections
Reasons are the only artifact a maintainer sees in the Actions log, so
they must be enough to find and remove the root cause (or judge a false
positive) without access to the artifacts.
- Add a "Reason Requirements (Forensic Detail)" section to the detection
prompt: one reason per distinct finding, tagged by category, with
LOCATION, EVIDENCE (verbatim quote), ORIGIN, WHY and REMEDIATION.
- Prompt injection reasons must quote the triggering passage verbatim
with line numbers and name the untrusted region and actor it came from.
- Secret-leak reasons must never reproduce the credential: mask it with
type prefix and length, and instead supply provenance (type, artifact,
source variable/step, and the sink it was headed to) plus the evasion
technique used.
- Malicious-patch reasons must name the patch, target file and hunk, the
added lines, and dependency name/version/registry host plus the
concrete indicator that fired.
- Raise the per-reason bound from 1000 to 2000 characters so evidence
fits, and render multi-line reasons across real log lines with a gutter
prefix so quoted evidence stays copy-pasteable without letting an
untrusted line emit a workflow command.
- Update spec (TD-10d, TD-10b, TD-20d) and README to match.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
* fix(detector): transport reasons out of band of the shell
Review flagged that the new verbatim-evidence requirement pushes
attacker-authored text onto a shell command line: the engine invokes
threat_detection_result through Bash, so evidence containing $(...),
backticks, or quotes passed via --reason is expanded or executed before
the tool sees it. Prompt-level quoting guidance is not a boundary.
- Add --reasons-file to report-result, reading a JSON array of reason
strings via detector.ReadReasonsFile. The model writes the file with
its file-editing tool, so evidence never reaches a command line; a
malformed file is a correctable parse error, not an executed command.
Entries are bounded by the same validateRawResult rules as --reason.
- Rewrite the prompt's Response Format to mandate the file transport for
anything quoting artifact content, explain why, and restrict --reason
to short self-authored text. Update the self-correction instruction.
- Extract resultToolScript so the wrapper's use of double-quoted "$@"
(no re-splitting or globbing) is documented and tested.
- Add TD-10e requiring a non-shell transport and forbidding prompt-only
quoting guidance as the sole protection.
- Tests: end-to-end invocation through /bin/sh and the provisioned
wrapper with hostile evidence ($(...), backticks, ;, |, globs, quotes,
newlines) asserting no canary file is created and the reason survives
byte-for-byte; wrapper argument-forwarding test; reasons-file
round-trip and malformed-input cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
* fix(detector): close two gaps found in re-review
Legacy workflow-command marker reached the job log (cmd/threat-detect):
the Actions runner accepts "##[command]data" in addition to "::command::",
and locates it with an unanchored IndexOf (ActionCommand.TryParse) rather
than the TrimStart+StartsWith used for "::" (TryParseV2). A marker
anywhere inside a line is therefore live, so the reason gutter — which
only guarantees a line never *starts* with "::" — could not neutralize
it. Reachable commands include add-mask (redacts arbitrary log text) and
stop-commands (suppresses every later command, including this program's
own threat annotation, letting attacker-authored evidence hide the very
finding it caused). Escape "##[" in sanitizeLogValue, covering all four
untrusted echo paths (reasons, filenames, detection-log lines, the
annotation message). Verified the new tests fail without the fix.
Reasons transport was unreachable on Claude (pkg/engine): the prompt
requires reasons to be written to a file with a file-writing tool, but
claudeArgs granted only Bash when the result sink is provisioned — so on
Claude the model's only options were a heredoc or a --reason argument,
i.e. exactly the shell-expansion surface the transport removes, leaving
prompt wording as the sole protection (contra TD-10e). Grant Write and
Edit alongside Bash; this adds no capability, since Bash can already
write files. Add a test pinning the grant to the transport.
Also provision THREAT_DETECTION_REASONS_FILE next to the result sink
(a directory every engine can reach — Copilot gets it via --add-dir)
instead of naming a hardcoded /tmp path the engine may be refused, and
delete any reasons file left by a previous attempt so a retry cannot
report stale reasons. Spec TD-10e/TD-20d and README updated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
---------
Co-authored-by: GitHub Ace <githubnext@users.noreply.github.com>
Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
Copy file name to clipboardExpand all lines: cmd/threat-detect/main.go
+1-1Lines changed: 1 addition & 1 deletion
Original file line number
Diff line number
Diff line change
@@ -38,7 +38,7 @@ const (
38
38
39
39
detectionCorrectionPrefix="Your previous response did not record a verdict"
40
40
detectionCorrectionMessage="The threat_detection_result command was not run, or it reported an error and exited before a verdict was recorded."
41
-
detectionCorrectionInstruction="Run the threat_detection_result command exactly once with --prompt-injection, --secret-leak, and --malicious-patch each set to true or false, plus a --reason for every threat set to true."
41
+
detectionCorrectionInstruction="Run the threat_detection_result command exactly once with --prompt-injection, --secret-leak, and --malicious-patch each set to true or false. When any of them is true, use your file-writing tool to write your reasons as a JSON array of strings to the path in $THREAT_DETECTION_REASONS_FILE and pass it with --reasons-file; do not paste quoted artifact content onto the command line."
42
42
promptAnalysisValidationCode="ERR_VALIDATION"
43
43
44
44
// maxInventoryEntries bounds the artifact inventory printed to stderr so a
Copy file name to clipboardExpand all lines: cmd/threat-detect/report.go
+14-2Lines changed: 14 additions & 2 deletions
Original file line number
Diff line number
Diff line change
@@ -43,12 +43,14 @@ func runReport(args []string) int {
43
43
secretLeakbool
44
44
maliciousPatchbool
45
45
reasonsstringSliceFlag
46
+
reasonsFilestring
46
47
resultFilestring
47
48
)
48
49
fs.BoolVar(&promptInjection, "prompt-injection", false, "Whether a prompt injection threat was detected (required)")
49
50
fs.BoolVar(&secretLeak, "secret-leak", false, "Whether a secret leak threat was detected (required)")
50
51
fs.BoolVar(&maliciousPatch, "malicious-patch", false, "Whether a malicious patch threat was detected (required)")
51
-
fs.Var(&reasons, "reason", "Reason explaining a detected threat (repeatable)")
52
+
fs.Var(&reasons, "reason", "Reason explaining a detected threat (repeatable; use --reasons-file for text quoting artifact content)")
53
+
fs.StringVar(&reasonsFile, "reasons-file", "", "Path to a file containing a JSON array of reason strings; the shell-free way to report reasons that quote artifact content")
52
54
fs.StringVar(&resultFile, "result-file", os.Getenv("THREAT_DETECTION_RESULT_FILE"), "Path to the result sink file (defaults to env THREAT_DETECTION_RESULT_FILE)")
0 commit comments