Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ The `latest` (non-prerelease) GitHub release and "Latest" badge **only move on e

This repo runs daily AW smoke tests against all three engines:

- `.github/workflows/smoke-{copilot,claude,codex}-standalone.{md,lock.yml}` — gh-aw natively downloads this repo's released `threat-detect` binary (the external detector path is the compile-time default; no `features` opt-in needed), runs it under AWF, and concludes from the structured `detection_result.json`. The `.lock.yml` files are compiled by `gh aw compile`. Note the locks conclude with gh-aw's `conclude_threat_detection.sh`, **not** `threat-detect conclude`, so they never perform the `--full-result-file` lookup and their job logs show no reasons; use `replay-detection.yml` when you need the reasons.
- `.github/workflows/smoke-{copilot,claude,codex}-standalone.{md,lock.yml}` — gh-aw natively downloads this repo's released `threat-detect` binary (the external detector path is the compile-time default; no `features` opt-in needed), runs it under AWF, and concludes from the structured `detection_result.json`. The `.lock.yml` files are compiled by `gh aw compile`. The locks call gh-aw's `conclude_threat_detection.sh`, which parses no JSON itself — it checks `threat-detect` is on `PATH` and then `exec`s `threat-detect conclude`. It passes no `--full-result-file`, so the conventional sibling is derived and the smokes' job logs **do** show reasons and `⚠️` warnings.

The detector version is **not** pinned in the locks: gh-aw emits the literal `latest`, which `install_threat_detect_binary.sh` resolves at run time to the newest **non-prerelease** detector release. Promoting a release is enough to put it in front of the smokes — no recompile. Unpromoted prereleases are never picked up automatically; test those via `replay-detection.yml` with `detector_source=release` and `detector_ref=<prerelease tag>`.

Expand Down
72 changes: 69 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,61 @@ Consequences worth knowing:
read-only detection directory does not turn a completed detection into an
infrastructure error.

#### Warnings vs reasons

The result also carries a `warnings` array, which is **not** the same as
`reasons`:

| | `reasons` | `warnings` |
|---|---|---|
| Author | The detection model | The detector itself |
| Content | Free-form explanations quoting untrusted artifact content | Fixed strings composed by the detector, embedding host-controlled paths |
| Where written | Full result only (`detection_result_full.json`) | **Both** files (`detection_result.json` and `_full.json`) |
| Uploaded? | No | Yes — safe to publish |
| Affects verdict? | Yes (present iff at least one threat is true) | **No.** A warning never sets a threat category or triggers a threat exit |
| Signal | "here is a threat and why" | "the detector could not inspect part of the input" |

A warning is recorded when an artifact channel is present but cannot be fully
inspected — for example, `HAS_PATCH=true` was set but no readable patch bundle
was found, a staged prompt or patch exists but cannot be opened, the
`comment-memory` directory could not be listed, or the optional prompt-analysis
inputs (`prompt-template.txt`, `prompt-import-tree.json`) were missing or
unusable. Readability is probed by
actually opening the file: a stat-only check succeeds on a file the detector has
no permission to read, which would report a non-empty, inspected-looking channel
for content nobody examined. Without the `warnings` array, a partially
inspectable bundle would be indistinguishable from a fully inspected clean one
on the uploaded result: the detector analyzed less than the full artifact set,
reported clean, and exited 0. Gating a run on advisory warnings is a
**host-level policy** decision; the detector deliberately does not do it, so a
staging failure cannot suppress a real finding by turning "could not inspect"
into "must fail". Each entry has three fields:

```json
"warnings": [
{
"field": "comment_memory",
"code": "ERR_VALIDATION",
"message": "Unable to read comment-memory directory at /tmp/gh-aw/threat-detection/comment-memory: permission denied"
}
]
```

`conclude` renders any warnings under a `⚠️` block distinct from both the
verdict and the reasons, so a misconfigured job is visible in the job log
without a reader having to consult GitHub Actions annotations.

One exception is worth knowing about. Warnings are advisory *within a run that
executes*. When `GH_AW_DETECTION_CONTINUE_ON_ERROR` is `false`, a warning about
a **required** input (`prompt`, `agent_output`, `patch`) is promoted to a
configuration error and the detector refuses to run degraded detection, exiting
`2` before any analysis happens. That is an infrastructure error, not a threat
verdict (exit `1`), and no result file is written — writing one would assert a
clean verdict for analysis that never ran. Warnings about *optional* artifacts,
such as `prompt_analysis`, are never promoted this way, so reporting them cannot
start failing a host that does not stage them. In the default warn mode, where
detection does run, every recorded warning appears in both result files.

#### Concluding a run (`conclude`)

In `gh-aw`-compiled workflows the detector runs inside the AWF sandbox, where the
Expand Down Expand Up @@ -535,7 +590,8 @@ explicitly treated as untrusted runtime data.
"prompt_injection": false,
"secret_leak": false,
"malicious_patch": false,
"reasons": []
"reasons": [],
"warnings": []
}
```

Expand All @@ -549,6 +605,16 @@ fail validation later. A rejected report is returned to the model as a
correctable tool error; an oversized or malformed result file is a parse error
that fails the detection closed.

`warnings` is a detector-authored, optional array of partial-inspection
findings (see [Warnings vs reasons](#warnings-vs-reasons)). It is additive and
backward-compatible: a pre-existing consumer sees the field absent on results
from an older detector, and one indexing into it always finds an array on
results from a newer detector. Each entry is a `{ "field", "code", "message" }`
object; the array is bounded at 20 entries with each `field` and `code` at most
64 characters and each `message` at most 2000 characters. A warning never sets a
threat category or causes a threat exit; see the note above on strict mode,
where a required-input warning is instead promoted to a configuration error.

### Replay workflow

Maintainers can manually run **Replay Threat Detection** from the Actions tab to rerun detection against artifacts from a prior workflow run. Provide the source repository and run ID; the workflow downloads the `agent`, `activation`, optional experiment, and optional original `detection` artifacts, normalizes them into the CLI input contract above, runs `threat-detect`, and uploads a sanitized `replay-detection-<run_id>` artifact with the manifest, file inventory, replay result, and original-result comparison.
Expand Down Expand Up @@ -634,7 +700,7 @@ This repository includes three Agentic Workflows smoke tests, one per engine:

Each runs daily and by `workflow_dispatch`. The top-level `Smoke` workflow can be dispatched manually to start all three at once. The matching `.lock.yml` files are the compiled AW workflows. gh-aw natively downloads this repo's released binary matching the runner platform (the external detector path is the compile-time default), runs it under AWF, and reads the structured `detection_result.json` to conclude. The detector version they install is resolved at run time — see [Detector Version Selection](#detector-version-selection).

**The smokes do not exercise `threat-detect conclude`.** The compiled locks conclude with gh-aw's own `conclude_threat_detection.sh`, which reads only `detection_result.json`. Because the published result deliberately carries `reasons: []`, the smokes' job logs show the verdict but no reasons: recovering those needs the `--full-result-file` lookup that only the `conclude` subcommand performs. Use [`replay-detection.yml`](#replay-workflow), which renders the reasons from the companion full result, when the explanations are what you need to see.
**The smokes do exercise `threat-detect conclude`.** The compiled locks call gh-aw's `conclude_threat_detection.sh`, which does no JSON parsing of its own: it verifies `threat-detect` is on `PATH` and then `exec`s `threat-detect conclude --result-file … --detection-log …`. Because it passes no `--full-result-file`, the conventional sibling (`detection_result_full.json`) is derived and the reasons *are* recovered into the smokes' job logs, as are any `⚠️` warnings. The full result is written next to the result file and is not uploaded — the detection artifact lists `detection_result.json` by exact filename rather than a glob, so the companion never leaves the runner. Use [`replay-detection.yml`](#replay-workflow) when you want to rerun detection against a past run's artifacts, rather than as a way to see reasons.

**Codex detection model pin.** The Codex smokes pin the detection model explicitly:

Expand Down Expand Up @@ -719,7 +785,7 @@ What the collector records per run:

- whether the run had a `detection` job at all, and which detector it used. The `Install threat-detect binary` marker step is only observable on a job that got far enough to reach it — a skipped job reports no steps at all, and a cancelled or setup-failed one reports a truncated list — so classifying per run would drop exactly the failures this report exists to measure into the "built-in" bucket and out of every rate. The evidence is therefore rolled up per **workflow**: if any run of a workflow showed the marker that day, all of its detection jobs count as external. Anything still unresolved is reported as `indeterminate_detector_runs`, never as built-in;
- the detection job's `status`/`conclusion` (`success`, `failure`, `cancelled`, `skipped`, `timed_out`, `action_required`, or still `in_progress`) and the names of any failed steps;
- whether the job published a `detection_result.json` artifact and, if so, the `prompt_injection` / `secret_leak` / `malicious_patch` verdict. The published result deliberately carries `reasons: []`, so explanations are not available here — use the [replay workflow](#replay-workflow) when you need them;
- whether the job published a `detection_result.json` artifact and, if so, the `prompt_injection` / `secret_leak` / `malicious_patch` verdict. The published result deliberately carries `reasons: []`, so explanations are not available here — use the [replay workflow](#replay-workflow) when you need them. Any `warnings` the detector recorded *are* present in the published result, since they are detector-authored (see [Warnings vs reasons](#warnings-vs-reasons));
- the `conclusion`/`reason` pairs (`threat_detected`, `agent_failure`, `parse_error`) that gh-aw itself posts to its `[aw] Detection Runs` tracking issue.

A **green** detection job that published no verdict artifact is counted separately as a soft failure: gh-aw marks detection steps `continue-on-error`, so the Actions runner rewrites their `conclusion` to `success` and step conclusions cannot reveal these.
Expand Down
25 changes: 25 additions & 0 deletions cmd/threat-detect/conclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ func (c *concluder) conclude(resultFile string) int {

// Step 4 — report and evaluate the verdict.
c.reportVerdict(result, reasons)
c.reportWarnings(result.Warnings)
if result.HasThreats() {
threats := make([]string, 0, 3)
if result.PromptInjection {
Expand Down Expand Up @@ -386,6 +387,30 @@ func (c *concluder) reportVerdict(result *detector.Result, reasons []string) {
}
}

// reportWarnings prints a distinct ⚠️ block for any detector-authored
// warnings the result carries. Warnings say "the detector could not inspect
// everything", not "a threat was found", so they are rendered separately from
// both the verdict and the reasons — visible in the job log without a reader
// having to consult the annotations. They never affect the verdict or the
// exit code.
//
// Each field is host-controlled or detector-composed (never model-authored),
// but Message embeds host-controlled paths, so every value is sanitized before
// it is echoed into the job log — the same protection the reasons block
// applies to its untrusted content.
func (c *concluder) reportWarnings(warnings []detector.ResultWarning) {
if len(warnings) == 0 {
return
}
c.info(fmt.Sprintf("⚠️ Detector warnings (%d) — artifact channels that could not be fully inspected. These do not affect the verdict.", len(warnings)))
for i, w := range warnings {
field := sanitizeLogValue(truncateRunes(w.Field, maxEchoedLineRunes))
code := sanitizeLogValue(truncateRunes(w.Code, maxEchoedLineRunes))
message := sanitizeLogValue(truncateRunes(w.Message, maxEchoedLineRunes))
c.info(fmt.Sprintf(" [%d] field=%s code=%s message=%s", i+1, field, code, message))
}
}

// reasonLogLines renders an untrusted, model-authored reason as the physical
// log lines to print. Reasons carry forensic detail — verbatim quotes of the
// triggering content, file and line references — which is only usable if it
Expand Down
113 changes: 113 additions & 0 deletions cmd/threat-detect/conclude_warnings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package main

import (
"bytes"
"path/filepath"
"strings"
"testing"
)

// TestConcludeRendersWarningsWithMarker verifies the ⚠️ warnings block appears
// in the job log, is distinct from the verdict block, and is sanitized when
// the message embeds host-controlled content.
func TestConcludeRendersWarningsWithMarker(t *testing.T) {
verdictWithWarnings := `{
"prompt_injection": false,
"secret_leak": false,
"malicious_patch": false,
"reasons": [],
"warnings": [
{
"field": "comment_memory",
"code": "ERR_VALIDATION",
"message": "Unable to read comment-memory directory at /tmp/gh-aw/threat-detection/comment-memory: permission denied"
}
]
}`
dir := t.TempDir()
resultFile := writeResultFixture(t, verdictWithWarnings)

var stdout bytes.Buffer
c := &concluder{
runDetection: "true",
warnMode: false,
githubOutput: filepath.Join(dir, "out"),
githubEnv: filepath.Join(dir, "env"),
stdout: &stdout,
fullResultDisabled: true, // don't look for a sibling full result
}
// A safe verdict + warnings must still proceed (warnings never fail the run).
if code := c.run(resultFile); code != concludeExitProceed {
t.Fatalf("safe verdict + warnings must proceed, got exit %d; log:\n%s", code, stdout.String())
}
got := stdout.String()
if !strings.Contains(got, "⚠️") {
t.Errorf("expected ⚠️ marker in log; got:\n%s", got)
}
if !strings.Contains(got, "Detector warnings (1)") {
t.Errorf("expected 'Detector warnings (1)' header; got:\n%s", got)
}
if !strings.Contains(got, "field=comment_memory") {
t.Errorf("expected structured field= rendering; got:\n%s", got)
}
if !strings.Contains(got, "code=ERR_VALIDATION") {
t.Errorf("expected structured code= rendering; got:\n%s", got)
}
if !strings.Contains(got, "do not affect the verdict") {
t.Errorf("warnings block must explain it does not affect the verdict; got:\n%s", got)
}
}

// TestConcludeWarningsSanitizedForControlChars verifies that a host-controlled
// path containing a control character cannot inject a workflow command line
// or break out of the rendered log line.
func TestConcludeWarningsSanitizedForControlChars(t *testing.T) {
// Embed a bare CR then "::error::" — a naive renderer would let it start
// a new physical line that the Actions runner interprets as an error.
verdict := `{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[],"warnings":[{"field":"comment_memory","code":"ERR_VALIDATION","message":"Unable to read /tmp/x\r::error::hijacked"}]}`
dir := t.TempDir()
resultFile := writeResultFixture(t, verdict)

var stdout bytes.Buffer
c := &concluder{
runDetection: "true",
warnMode: false,
githubOutput: filepath.Join(dir, "out"),
githubEnv: filepath.Join(dir, "env"),
stdout: &stdout,
fullResultDisabled: true,
}
if code := c.run(resultFile); code != concludeExitProceed {
t.Fatalf("run returned %d; log:\n%s", code, stdout.String())
}
got := stdout.String()
// The raw "\r::error::hijacked" sequence must NOT appear as an active
// workflow command line — sanitizeLogValue escapes control characters.
if strings.Contains(got, "\r::error::") {
t.Errorf("control char must be sanitized so it cannot forge a workflow command; got:\n%s", got)
}
}

// TestConcludeNoWarningsBlockWhenEmpty verifies the warnings section is
// silent when there are no warnings — the log should not contain a stray
// ⚠️ header for a clean run.
func TestConcludeNoWarningsBlockWhenEmpty(t *testing.T) {
dir := t.TempDir()
resultFile := writeResultFixture(t, safeVerdict)

var stdout bytes.Buffer
c := &concluder{
runDetection: "true",
warnMode: false,
githubOutput: filepath.Join(dir, "out"),
githubEnv: filepath.Join(dir, "env"),
stdout: &stdout,
fullResultDisabled: true,
}
if code := c.run(resultFile); code != concludeExitProceed {
t.Fatalf("run returned %d; log:\n%s", code, stdout.String())
}
if strings.Contains(stdout.String(), "Detector warnings") {
t.Errorf("no warnings header expected on a clean run; got:\n%s", stdout.String())
}
}
Loading
Loading