Skip to content

Commit 8b3b5cc

Browse files
davidslaterGitHub AceCopilot
authored
feat(cli): add step-summary blocks for rendered prompt and verdict (#739)
* feat(cli): add step-summary blocks for rendered prompt and verdict Adds `--step-summary` to `threat-detect` (writes the prompt actually rendered, plus resolved engine/model/retries, as a collapsible block) and to `threat-detect conclude` (writes a verdict block with per-field booleans, reasons, conclusion, and reason code for every terminal outcome including skipped). Both default to $GITHUB_STEP_SUMMARY, are best-effort (a write failure never changes the exit code), and bound the prompt size to stay within the shared per-job step summary budget. Closes #696 Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com> * fix(cli): escape untrusted content and reject step-summary path collisions Addresses review feedback on #739: - HTML-escape and render the rendered prompt inside <pre><code> instead of a Markdown fence, since the embedded artifact content can contain a closing fence and spoof the job summary. - HTML-escape and render verdict reasons inside <pre><code> for the same reason, since reasons are engine-generated from untrusted artifacts. - Add rejectPathCollisions and use it to reject --step-summary aliasing --log-file/--output in `threat-detect`, and --result-file/ $GITHUB_OUTPUT/$GITHUB_ENV in `threat-detect conclude`, so a misconfigured alias fails closed instead of corrupting another destination. 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> Co-authored-by: Copilot <copilot@github.com>
1 parent c31765e commit 8b3b5cc

11 files changed

Lines changed: 614 additions & 20 deletions

File tree

README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ threat-detect [flags] <artifacts-dir>
6363
- `--custom-prompt-file` — Path to a file with additional detection instructions. Takes precedence over `--custom-prompt` and `CUSTOM_PROMPT`
6464
- `--output` — Path to write JSON result (defaults to stdout)
6565
- `--log-file` — Path to write structured JSONL run logs (one JSON object per line). Env: `THREAT_DETECTION_LOG_FILE`; defaults to `detection-runlog.jsonl` beside `--output`
66+
- `--step-summary` — Path to append the rendered prompt (engine/model/retries plus the prompt actually sent, including the resolved prompt-analysis section) as a collapsible block in the job step summary. Defaults to `GITHUB_STEP_SUMMARY`
6667
- `--retries` — Retries for malformed detection outputs. Default: `1` (env: `THREAT_DETECTION_RETRIES`)
6768
- `--version` — Print version and exit
6869

@@ -171,6 +172,12 @@ host-side reason:
171172
| `config_error` | `agent_failure` |
172173
| absent / unrecognized / log unreadable | `agent_failure` ("Detection result file not found at: <path>") |
173174

175+
`conclude` also accepts `--step-summary <path>` (defaulting to
176+
`GITHUB_STEP_SUMMARY`) to append a collapsible verdict block to the job step
177+
summary: per-field booleans (`prompt_injection`, `secret_leak`,
178+
`malicious_patch`), the reasons list, the resolved `conclusion`
179+
(`success`/`warning`/`failure`/`skipped`), and the reason code.
180+
174181
`conclude` writes a verbose, self-contained diagnostic section to the job log:
175182
banners framing the section, the environment inputs and resolved paths, and the
176183
per-field verdict breakdown (`prompt_injection`/`secret_leak`/`malicious_patch`)

cmd/threat-detect/conclude.go

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,12 @@ func runConclude(args []string) int {
8787
fs.SetOutput(os.Stderr)
8888
var (
8989
resultFile string
90+
stepSummary string
9091
detectionLog string
9192
logFile string
9293
)
9394
fs.StringVar(&resultFile, "result-file", defaultConcludeResultFile, "Path to the structured detection_result.json verdict file")
95+
fs.StringVar(&stepSummary, "step-summary", os.Getenv("GITHUB_STEP_SUMMARY"), "Path to append the verdict to the job step summary (defaults to env GITHUB_STEP_SUMMARY)")
9496
fs.StringVar(&detectionLog, "detection-log", "", "Path to the detection run's captured log, consulted to refine agent_failure/parse_error and to render diagnostics when the result file is missing (default: <result-file dir>/detection.log)")
9597
fs.StringVar(&logFile, "log-file", os.Getenv("THREAT_DETECTION_LOG_FILE"), "Path to write JSONL run logs (env: THREAT_DETECTION_LOG_FILE)")
9698
if err := fs.Parse(args); err != nil {
@@ -103,6 +105,23 @@ func runConclude(args []string) int {
103105
detectionLog = filepath.Join(filepath.Dir(resultFile), defaultDetectionLogName)
104106
}
105107

108+
githubOutput := os.Getenv("GITHUB_OUTPUT")
109+
githubEnv := os.Getenv("GITHUB_ENV")
110+
111+
// Reject collisions among the run's independently-written destinations:
112+
// --step-summary must not alias the structured result file (which would be
113+
// overwritten with Markdown after being read) or the GitHub Actions command
114+
// files (which the runner may reject outright if polluted with Markdown).
115+
if err := rejectPathCollisions(
116+
namedPath{"--result-file", resultFile},
117+
namedPath{"--step-summary", stepSummary},
118+
namedPath{"$GITHUB_OUTPUT", githubOutput},
119+
namedPath{"$GITHUB_ENV", githubEnv},
120+
); err != nil {
121+
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
122+
return concludeExitFail
123+
}
124+
106125
// The JSONL log is opened with O_TRUNC, so it must not alias an input this
107126
// command reads. Aliasing --result-file would erase the verdict (yielding a
108127
// bogus parse_error) and aliasing the detection log would erase the failure
@@ -153,8 +172,9 @@ func runConclude(args []string) int {
153172
warnMode: os.Getenv("GH_AW_DETECTION_CONTINUE_ON_ERROR") != "false",
154173
executionFailed: os.Getenv("DETECTION_AGENTIC_EXECUTION_OUTCOME") == "failure",
155174
executionOutcome: os.Getenv("DETECTION_AGENTIC_EXECUTION_OUTCOME"),
156-
githubOutput: os.Getenv("GITHUB_OUTPUT"),
157-
githubEnv: os.Getenv("GITHUB_ENV"),
175+
githubOutput: githubOutput,
176+
githubEnv: githubEnv,
177+
stepSummary: stepSummary,
158178
detectionLog: detectionLog,
159179
logger: logger,
160180
stdout: os.Stdout,
@@ -173,6 +193,7 @@ type concluder struct {
173193

174194
githubOutput string // path of $GITHUB_OUTPUT (may be empty)
175195
githubEnv string // path of $GITHUB_ENV (may be empty)
196+
stepSummary string // path of $GITHUB_STEP_SUMMARY (may be empty)
176197
detectionLog string // detection run log path; empty derives the sibling default
177198
logger *runlog.Logger
178199
stdout io.Writer
@@ -221,6 +242,7 @@ func (c *concluder) conclude(resultFile string) int {
221242
c.setOutput("success", "true")
222243
c.setOutput("reason", "")
223244
c.exportVariable("GH_AW_DETECTION_REASON", "")
245+
c.writeVerdictSummary(nil, "skipped", "")
224246
c.info("✅ Detection skipped — no threats to evaluate.")
225247
c.logger.Info("conclude_outcome", map[string]any{
226248
"conclusion": "skipped",
@@ -251,16 +273,16 @@ func (c *concluder) conclude(resultFile string) int {
251273

252274
if errors.Is(err, fs.ErrNotExist) {
253275
reason, code := c.detectionFailureReason()
254-
return c.fail(reason, fmt.Sprintf("%s: ❌ Detection result file not found at: %s", code, resultFile))
276+
return c.fail(nil, reason, fmt.Sprintf("%s: ❌ Detection result file not found at: %s", code, resultFile))
255277
}
256278
var pathErr *fs.PathError
257279
if errors.As(err, &pathErr) {
258280
reason, code := c.detectionFailureReason()
259-
return c.fail(reason, fmt.Sprintf("%s: ❌ Detection result file unreadable at %s: %v", code, resultFile, err))
281+
return c.fail(nil, reason, fmt.Sprintf("%s: ❌ Detection result file unreadable at %s: %v", code, resultFile, err))
260282
}
261283
c.info("💡 This usually means the AI engine did not record a verdict in the expected format.")
262284
c.info(` Expected content: {"prompt_injection":bool,"secret_leak":bool,"malicious_patch":bool,"reasons":[...]}`)
263-
return c.fail("parse_error", fmt.Sprintf("%s: ❌ Failed to parse detection result file %s: %v", errCodeParse, resultFile, err))
285+
return c.fail(nil, "parse_error", fmt.Sprintf("%s: ❌ Failed to parse detection result file %s: %v", errCodeParse, resultFile, err))
264286
}
265287

266288
c.info("✔️ Structured result file found and parsed successfully.")
@@ -282,7 +304,7 @@ func (c *concluder) conclude(resultFile string) int {
282304
if len(result.Reasons) > 0 {
283305
message += "\nReasons: " + strings.Join(result.Reasons, "; ")
284306
}
285-
return c.fail("threat_detected", message)
307+
return c.fail(result, "threat_detected", message)
286308
}
287309

288310
c.info("✅ No security threats detected. Safe outputs may proceed.")
@@ -291,6 +313,7 @@ func (c *concluder) conclude(resultFile string) int {
291313
c.setOutput("success", "true")
292314
c.setOutput("reason", "")
293315
c.exportVariable("GH_AW_DETECTION_REASON", "")
316+
c.writeVerdictSummary(result, "success", "")
294317
c.logger.Info("conclude_outcome", map[string]any{
295318
"conclusion": "success",
296319
"reason": "",
@@ -599,7 +622,7 @@ func lastDetectionStatusReason(path string) string {
599622
// - In warn mode and not mustFail, emit a warning, set conclusion=warning, and
600623
// let the job proceed (exit 0).
601624
// - Otherwise set conclusion=failure, emit an error, and fail closed (exit 1).
602-
func (c *concluder) fail(reason, message string) int {
625+
func (c *concluder) fail(result *detector.Result, reason, message string) int {
603626
mustFail := c.executionFailed && (reason == "agent_failure" || reason == "parse_error")
604627
c.setOutput("reason", reason)
605628
c.exportVariable("GH_AW_DETECTION_REASON", reason)
@@ -608,6 +631,7 @@ func (c *concluder) fail(reason, message string) int {
608631
c.setOutput("conclusion", "warning")
609632
c.exportVariable("GH_AW_DETECTION_CONCLUSION", "warning")
610633
c.setOutput("success", "false")
634+
c.writeVerdictSummary(result, "warning", reason)
611635
c.logger.Error("conclude_outcome", map[string]any{
612636
"conclusion": "warning",
613637
"reason": reason,
@@ -621,6 +645,7 @@ func (c *concluder) fail(reason, message string) int {
621645
c.setOutput("conclusion", "failure")
622646
c.exportVariable("GH_AW_DETECTION_CONCLUSION", "failure")
623647
c.setOutput("success", "false")
648+
c.writeVerdictSummary(result, "failure", reason)
624649
c.logger.Error("conclude_outcome", map[string]any{
625650
"conclusion": "failure",
626651
"reason": reason,
@@ -631,6 +656,15 @@ func (c *concluder) fail(reason, message string) int {
631656
return concludeExitFail
632657
}
633658

659+
// writeVerdictSummary appends the verdict block to the job step summary,
660+
// logging (but not failing on) any write error since the summary is a
661+
// best-effort diagnostic aid, not part of the conclude contract.
662+
func (c *concluder) writeVerdictSummary(result *detector.Result, conclusion, reasonCode string) {
663+
if err := detector.AppendStepSummary(c.stepSummary, detector.FormatVerdictSummary(result, conclusion, reasonCode)); err != nil {
664+
fmt.Fprintf(os.Stderr, "conclude: failed to write step summary: %v\n", err)
665+
}
666+
}
667+
634668
// setOutput appends a step output to $GITHUB_OUTPUT. Values are single-line
635669
// tokens, so the simple name=value form is sufficient and unambiguous.
636670
func (c *concluder) setOutput(name, value string) {

cmd/threat-detect/conclude_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,67 @@ func TestConcludeUnreadableFileIsAgentFailure(t *testing.T) {
366366
}
367367
}
368368

369+
func TestConcludeWritesVerdictStepSummary(t *testing.T) {
370+
dir := t.TempDir()
371+
stepSummaryPath := filepath.Join(dir, "step_summary.md")
372+
resultFile := writeResultFixture(t, threatVerdict)
373+
374+
var stdout bytes.Buffer
375+
c := &concluder{
376+
runDetection: "true",
377+
warnMode: false,
378+
githubOutput: filepath.Join(dir, "out"),
379+
githubEnv: filepath.Join(dir, "env"),
380+
stepSummary: stepSummaryPath,
381+
stdout: &stdout,
382+
}
383+
if code := c.run(resultFile); code != concludeExitFail {
384+
t.Fatalf("exit code = %d, want %d (stdout: %s)", code, concludeExitFail, stdout.String())
385+
}
386+
387+
data, err := os.ReadFile(stepSummaryPath)
388+
if err != nil {
389+
t.Fatalf("ReadFile(stepSummaryPath) error = %v", err)
390+
}
391+
summary := string(data)
392+
for _, want := range []string{
393+
"<summary>Threat Detection Verdict</summary>",
394+
"| Prompt Injection | true |",
395+
"| Conclusion | failure |",
396+
"| Reason Code | threat_detected |",
397+
"jailbreak attempt",
398+
} {
399+
if !strings.Contains(summary, want) {
400+
t.Errorf("step summary missing %q; got:\n%s", want, summary)
401+
}
402+
}
403+
}
404+
405+
func TestConcludeSkippedWritesVerdictStepSummary(t *testing.T) {
406+
dir := t.TempDir()
407+
stepSummaryPath := filepath.Join(dir, "step_summary.md")
408+
409+
var stdout bytes.Buffer
410+
c := &concluder{
411+
runDetection: "false",
412+
githubOutput: filepath.Join(dir, "out"),
413+
githubEnv: filepath.Join(dir, "env"),
414+
stepSummary: stepSummaryPath,
415+
stdout: &stdout,
416+
}
417+
if code := c.run(filepath.Join(dir, "detection_result.json")); code != concludeExitProceed {
418+
t.Fatalf("exit code = %d, want %d (stdout: %s)", code, concludeExitProceed, stdout.String())
419+
}
420+
421+
data, err := os.ReadFile(stepSummaryPath)
422+
if err != nil {
423+
t.Fatalf("ReadFile(stepSummaryPath) error = %v", err)
424+
}
425+
if !strings.Contains(string(data), "| Conclusion | skipped |") {
426+
t.Errorf("step summary missing skipped conclusion; got:\n%s", string(data))
427+
}
428+
}
429+
369430
func TestConcludeThreatMessageEscaped(t *testing.T) {
370431
dir := t.TempDir()
371432
resultFile := writeResultFixture(t, threatVerdict)
@@ -1042,3 +1103,41 @@ func TestConcludeUnopenableLogFileIsConfigError(t *testing.T) {
10421103
t.Fatalf("runConclude() = %d, want %d", code, concludeExitFail)
10431104
}
10441105
}
1106+
1107+
func TestRunConcludeRejectsStepSummaryCollidingWithResultFile(t *testing.T) {
1108+
dir := t.TempDir()
1109+
resultFile := writeResultFixture(t, safeVerdict)
1110+
1111+
t.Setenv("RUN_DETECTION", "true")
1112+
t.Setenv("GITHUB_OUTPUT", filepath.Join(dir, "out"))
1113+
t.Setenv("GITHUB_ENV", filepath.Join(dir, "env"))
1114+
1115+
code := runConclude([]string{"--result-file", resultFile, "--step-summary", resultFile})
1116+
if code != concludeExitFail {
1117+
t.Fatalf("runConclude() = %d, want %d (fail closed on collision)", code, concludeExitFail)
1118+
}
1119+
// The result file must survive untouched — a collision must be rejected
1120+
// before anything is written, so the structured verdict is never clobbered.
1121+
data, err := os.ReadFile(resultFile)
1122+
if err != nil {
1123+
t.Fatalf("ReadFile(resultFile) error = %v", err)
1124+
}
1125+
if string(data) != safeVerdict {
1126+
t.Fatalf("result file was modified: got %q, want %q", string(data), safeVerdict)
1127+
}
1128+
}
1129+
1130+
func TestRunConcludeRejectsStepSummaryCollidingWithGithubOutput(t *testing.T) {
1131+
dir := t.TempDir()
1132+
resultFile := writeResultFixture(t, safeVerdict)
1133+
shared := filepath.Join(dir, "shared")
1134+
1135+
t.Setenv("RUN_DETECTION", "true")
1136+
t.Setenv("GITHUB_OUTPUT", shared)
1137+
t.Setenv("GITHUB_ENV", filepath.Join(dir, "env"))
1138+
1139+
code := runConclude([]string{"--result-file", resultFile, "--step-summary", shared})
1140+
if code != concludeExitFail {
1141+
t.Fatalf("runConclude() = %d, want %d (fail closed on collision)", code, concludeExitFail)
1142+
}
1143+
}

cmd/threat-detect/logfile_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,50 @@ func TestRunRejectsLogFileCollidingWithOutput(t *testing.T) {
296296
}
297297
}
298298

299+
func TestRunRejectsStepSummaryCollidingWithOutput(t *testing.T) {
300+
artifactsDir := t.TempDir()
301+
shared := filepath.Join(t.TempDir(), "same.json")
302+
303+
code, stderr := runWithTestArgsCapture(t, []string{
304+
"threat-detect",
305+
"-output", shared,
306+
"-step-summary", shared,
307+
artifactsDir,
308+
}, nil)
309+
310+
if code != exitError {
311+
t.Fatalf("run() exit code = %d, want %d", code, exitError)
312+
}
313+
if !strings.Contains(stderr, "--step-summary") || !strings.Contains(stderr, "must not point to the same file") {
314+
t.Fatalf("stderr missing collision error, got:\n%s", stderr)
315+
}
316+
if _, err := os.Stat(shared); !os.IsNotExist(err) {
317+
t.Fatalf("expected no file to be written, stat err = %v", err)
318+
}
319+
}
320+
321+
func TestRunRejectsStepSummaryCollidingWithLogFile(t *testing.T) {
322+
artifactsDir := t.TempDir()
323+
shared := filepath.Join(t.TempDir(), "same.jsonl")
324+
325+
code, stderr := runWithTestArgsCapture(t, []string{
326+
"threat-detect",
327+
"-log-file", shared,
328+
"-step-summary", shared,
329+
artifactsDir,
330+
}, nil)
331+
332+
if code != exitError {
333+
t.Fatalf("run() exit code = %d, want %d", code, exitError)
334+
}
335+
if !strings.Contains(stderr, "--step-summary") || !strings.Contains(stderr, "--log-file") {
336+
t.Fatalf("stderr missing collision error mentioning both flags, got:\n%s", stderr)
337+
}
338+
if _, err := os.Stat(shared); !os.IsNotExist(err) {
339+
t.Fatalf("expected no file to be written, stat err = %v", err)
340+
}
341+
}
342+
299343
func TestRunRejectsDefaultLogCollidingThroughDanglingOutputSymlink(t *testing.T) {
300344
artifactsDir := t.TempDir()
301345
outputDir := t.TempDir()

cmd/threat-detect/main.go

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ func run() (code int) {
107107
promptFile string
108108
outputJSON string
109109
logFile string
110+
stepSummary string
110111
workflowName string
111112
workflowDescription string
112113
customPrompt string
@@ -125,6 +126,7 @@ func run() (code int) {
125126
flag.StringVar(&promptFile, "prompt-template", "", "Path to custom prompt template (defaults to built-in)")
126127
flag.StringVar(&outputJSON, "output", "", "Path to write JSON result (defaults to stdout)")
127128
flag.StringVar(&logFile, "log-file", os.Getenv("THREAT_DETECTION_LOG_FILE"), "Path to write JSONL run logs (env: THREAT_DETECTION_LOG_FILE)")
129+
flag.StringVar(&stepSummary, "step-summary", os.Getenv("GITHUB_STEP_SUMMARY"), "Path to append the rendered prompt to the job step summary (defaults to env GITHUB_STEP_SUMMARY)")
128130
flag.StringVar(&workflowName, "workflow-name", "", "Workflow name for the prompt (overrides WORKFLOW_NAME)")
129131
flag.StringVar(&workflowDescription, "workflow-description", "", "Workflow description for the prompt (overrides WORKFLOW_DESCRIPTION)")
130132
flag.StringVar(&customPrompt, "custom-prompt", "", "Additional detection instructions appended to the prompt (overrides CUSTOM_PROMPT)")
@@ -158,19 +160,17 @@ func run() (code int) {
158160
logFile = dir + "detection-runlog.jsonl"
159161
}
160162

161-
// Reject a --log-file that collides with --output: they are opened and
162-
// truncated independently, so sharing an inode would interleave the JSONL
163-
// trace and the result JSON and corrupt both while still reporting success.
164-
if logFile != "" && outputJSON != "" {
165-
if same, err := samePath(logFile, outputJSON); err != nil {
166-
fmt.Fprintf(os.Stderr, "Error resolving output paths: %v\n", err)
167-
reason = reasonConfigError
168-
return exitError
169-
} else if same {
170-
fmt.Fprintf(os.Stderr, "Error: --log-file and --output must not point to the same file (%q)\n", logFile)
171-
reason = reasonConfigError
172-
return exitError
173-
}
163+
// Reject collisions among the run's independently-written destinations
164+
// (--log-file, --output, --step-summary): each is opened and written on
165+
// its own, so aliasing any two would interleave or clobber their content.
166+
if err := rejectPathCollisions(
167+
namedPath{"--log-file", logFile},
168+
namedPath{"--output", outputJSON},
169+
namedPath{"--step-summary", stepSummary},
170+
); err != nil {
171+
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
172+
reason = reasonConfigError
173+
return exitError
174174
}
175175

176176
// Open the JSONL run log when configured or derived. A failure here is a
@@ -307,6 +307,16 @@ func run() (code int) {
307307
"custom_prompt_bytes": len(arts.CustomPrompt),
308308
})
309309

310+
// Surface the prompt actually rendered by threat-detect (including the
311+
// resolved prompt-analysis section and engine/model/retries) to the job
312+
// step summary. gh-aw's own prompt-rendering step summary reflects a
313+
// template threat-detect never receives, so this is the only summary that
314+
// reflects what was actually sent to the engine.
315+
if err := detector.AppendStepSummary(stepSummary, detector.FormatPromptSummary(engine.Canonical(engineID), model, retries, prompt)); err != nil {
316+
fmt.Fprintf(os.Stderr, "Warning: failed to write prompt step summary: %v\n", err)
317+
logger.Error("step_summary_write_failed", map[string]any{"stage": "prompt", "error": err.Error()})
318+
}
319+
310320
// Create engine
311321
eng, err := engine.New(engineID, model)
312322
if err != nil {

0 commit comments

Comments
 (0)