Tracking: parity with gh-aw's inline threat detection before migrating workflows to the external detector

[davidslater]

Created by GitHub Ace · View Session

Context

gh-aw still runs the inline threat-detection path in most compiled workflows: the detection job executes the agentic engine directly under AWF, scrapes THREAT_DETECTION_RESULT: out of detection.log with parse_threat_detection_results.cjs, and concludes with core.setOutput/core.setFailed. The external path (features: gh-aw-detection: true) downloads this repo's threat-detect binary and concludes via conclude_threat_detection.sh → threat-detect conclude.

Most recent relevant change upstream: github/gh-aw#48940 — engine failures (agent_failure, parse_error) now render as [!WARNING] threat detection engine error instead of [!CAUTION] agentic threat detected, keyed off isToolingFailureReason(reason). That makes the reason output load-bearing for user-facing severity, which raises the bar for our conclusion contract.

We made genuinely useful improvements when we diverged (out-of-band result sink instead of log scraping, trusted-vs-untrusted prompt analysis, structured JSONL run log, self-correction retries, an explicit terminal status line). This issue tracks everything that would regress if gh-aw flipped its workflows to this repo today.

Audit scope

Reviewed against gh-aw @ main:

• pkg/workflow/threat_detection_{job,steps,inline_engine,external,config,helpers}.go
• actions/setup/js/{setup_threat_detection,parse_threat_detection_results,threat_detection_warning,messages_run_status,generate_footer}.cjs
• actions/setup/sh/{install_threat_detect_binary,conclude_threat_detection}.sh
• actions/setup/md/{threat_detection,threat_warning_request_changes_review}.md
• docs/src/content/docs/reference/threat-detection.md, ADRs 29031 / 29257 / 28160 / 38166 / 41231

Findings

Blocking — features that are silently inert or fail-open today

#	Issue	Summary
1	#686	prompt-template.txt / prompt-import-tree.json are never staged into the detection dir → the trusted-vs-untrusted prompt analysis, our flagship improvement, is a no-op in production
2	#688	CUSTOM_PROMPT / WORKFLOW_NAME / WORKFLOW_DESCRIPTION never reach the detector → safe-outputs.threat-detection.prompt is silently ignored
3	#693	parse_error is unreachable — every THREAT_DETECTION_STATUS: reason collapses to agent_failure, discarding the granularity #48940 depends on
4	#694	Conclusion logic duplicated across conclude_threat_detection.sh and threat-detect conclude; strict-mode missing-file branch writes no reason, which renders an infra failure as [!CAUTION] agentic threat detected — the exact bug #48940 fixed. Also an undocumented fail-closed divergence from ADR 29031's warn-mode resilience
5	#701	Degraded/absent artifacts produce a clean verdict with no annotation — fail-open in a security control

Detection-coverage / accuracy regressions

#	Issue	Summary
6	#689	comment-memory files are not analyzed ({COMMENT_MEMORY_FILES} section dropped)
7	#691	npm lockfile false-positive suppression guidance dropped from the prompt
8	#704	aw_info.json (activation/trigger context) and experiments/ documented but never loaded

Observability regressions

#	Issue	Summary
9	#692	threat-detect conclude emits far less diagnostic output than parse_threat_detection_results.cjs (no verdict breakdown, no input echo, no directory listing, no log stats)
10	#696	Step summary renders gh-aw's unused prompt; the detector contributes neither the prompt it used nor a verdict summary
11	#697	The structured JSONL run log (--log-file) is never enabled or uploaded
12	#698	aic token-usage parsing unverified; max-ai-credits budget not enforced on the external path

Packaging

#	Issue	Summary
13	#695	No THREAT_DETECTION_RESULT: line emitted, so gh-aw's log-scraping fallback can never recover a verdict
14	#702	gh-aw's installer supports threat-detect-darwin-* assets we never publish

Suggested sequencing

1. Parity: conclusion logic is duplicated in conclude_threat_detection.sh and threat-detect conclude, with divergent (and partly broken) semantics #694 + Parity: parse_error is unreachable on the external path — THREAT_DETECTION_STATUS reasons are collapsed to agent_failure #693 first — they define the conclusion contract everything else reports through, and Parity: conclusion logic is duplicated in conclude_threat_detection.sh and threat-detect conclude, with divergent (and partly broken) semantics #694 contains a live correctness bug.
2. Parity: prompt-template.txt / prompt-import-tree.json are never staged, so trusted-vs-untrusted prompt analysis is silently disabled #686 + Parity: CUSTOM_PROMPT / WORKFLOW_NAME / WORKFLOW_DESCRIPTION never reach threat-detect, so threat-detection.prompt is silently ignored #688 + Parity: missing ERR_VALIDATION warnings — degraded/absent artifacts silently produce a clean verdict (fail-open) #701 — restore the inputs and make degradation loud. Without these the detector is running on far less context than the inline path.
3. Parity: threat-detect emits no THREAT_DETECTION_RESULT: line, so gh-aw's detection-log fallback can never recover a verdict #695 — cheap resilience, no gh-aw change required.
4. Parity: threat-detect conclude emits far less diagnostic output than parse_threat_detection_results.cjs #692 + Parity: step summary shows gh-aw's unused prompt; threat-detect contributes no prompt or verdict summary #696 + Parity: the structured JSONL run log (--log-file) is never enabled or uploaded on the external detector path #697 — observability, so the remaining items are diagnosable from a job log.
5. Parity: comment-memory files are not analyzed (missing {COMMENT_MEMORY_FILES} prompt section and artifact discovery) #689 + Parity: npm lockfile false-positive suppression guidance dropped from the detection prompt #691 + Parity: AI-credit reporting (aic) unverified and max-ai-credits budget not enforced on the external detector path #698 + Parity: documented artifacts aw_info.json and experiments/ are never loaded or surfaced #704 + Parity: gh-aw's installer supports threat-detect-darwin-* assets that we never publish #702 — coverage, cost, and packaging.

Several items need a paired PR in github/gh-aw (staging step, execution-step env, external command flags, artifact upload paths). Those should be filed upstream and linked back here.

Definition of done

• Every item above closed or explicitly waived with rationale recorded in specs/threat-detection-spec.md.
• A side-by-side run of the same workflow on both paths produces equivalent job-log diagnostics, equivalent conclusion/reason/success outputs across the full failure matrix, and an equivalent (or better) verdict.
• specs/threat-detection-spec.md and specs/usage-spec.md updated to describe the resulting contract normatively.

[davidslater]

Suggested PR batching

Grouped by the files each item touches, so batched issues land as one reviewable change and don't conflict with each other.

---

PR 1 — Conclusion contract + diagnostics (this repo)

Issues: #694, #693, #692
Files: cmd/threat-detect/conclude.go, cmd/threat-detect/conclude_test.go, cmd/threat-detect/main.go (status constants), specs/threat-detection-spec.md

These are one change, not three. #694 moves the skip / missing-file / unreadable-file branches out of conclude_threat_detection.sh into conclude.go; #693 is the status→reason mapping that those branches need; #692 is the logging those same branches emit. Doing them separately means rewriting the same function three times, and #693's mapping table is meaningless until #694 gives conclude ownership of the missing-file path.

Fix the strict-mode "no reason written" bug (#694) here — it's the live correctness issue in the batch.

Land the mustFail / ADR-29031 decision in this PR too, since it lives in the same function.

Paired gh-aw PR (must merge first or together): reduce actions/setup/sh/conclude_threat_detection.sh to a thin threat-detect conclude invocation, passing --detection-log.

---

PR 2 — gh-aw plumbing (upstream github/gh-aw, single PR)

Issues: gh-aw halves of #686, #688, #697, #698
Files: pkg/workflow/threat_detection_steps.go, pkg/workflow/threat_detection_external.go, pkg/workflow/threat_detection_helpers.go

All four need edits to the same two functions, so one upstream PR:

• buildPrepareDetectionFilesStep — also copy prompt-template.txt + prompt-import-tree.json (Parity: prompt-template.txt / prompt-import-tree.json are never staged, so trusted-vs-untrusted prompt analysis is silently disabled #686)
• buildExternalDetectorExecutionStep — add WORKFLOW_NAME / WORKFLOW_DESCRIPTION / CUSTOM_PROMPT to the env block and the AWF passthrough (Parity: CUSTOM_PROMPT / WORKFLOW_NAME / WORKFLOW_DESCRIPTION never reach threat-detect, so threat-detection.prompt is silently ignored #688); add --log-file to the command (Parity: the structured JSONL run log (--log-file) is never enabled or uploaded on the external detector path #697)
• buildUploadDetectionArtifactStep — add the runlog path (Parity: the structured JSONL run log (--log-file) is never enabled or uploaded on the external detector path #697)
• buildExternalDetectorWorkflowData — set MaxAICredits (Parity: AI-credit reporting (aic) unverified and max-ai-credits budget not enforced on the external detector path #698)

This is the highest-leverage PR in the set — #686 and #688 cannot take effect without it. Worth filing upstream early since it's on someone else's review queue.

---

PR 3 — Artifact loading, validation, inventory (this repo)

Issues: #701, #704, detector half of #686, artifact half of #689
Files: pkg/artifacts/artifacts.go (+ tests), pkg/runlog

Every one of these adds discovery or validation to artifacts.Load. Batching avoids four sequential rewrites of the same loader and lets the artifact inventory (#704) be built once and reused by the ERR_VALIDATION warnings (#701) and the missing-template warning (#686).

Depends on PR 2 for #686 to actually observe the files, but the warning path can and should be written first — it's what proves PR 2 landed.

---

PR 4 — Prompt content (this repo)

Issues: #691, prompt half of #689
Files: pkg/detector/prompts/threat_detection.md

Both are edits to the embedded prompt in different regions. Can run in parallel with PR 3; only trivial textual overlap with #689's new section. If that split feels awkward, fold all of #689 into PR 3 and leave #691 alone here — either works.

Note #691 has an open question (whether registry.npmjs.org is reachable under AWF) that should be answered before the PR, not during review.

---

PR 5 — Output surfaces (this repo)

Issues: #695, #696, detector half of #697
Files: cmd/threat-detect/main.go, cmd/threat-detect/report.go

All three add a new emission surface driven from the same place in run() — the stdout THREAT_DETECTION_RESULT: line (#695), the --step-summary block (#696), and the --log-file default (#697). Same function, same ordering constraints around the --output write.

#695 needs no gh-aw change, so this PR delivers value even if PR 2 stalls.

---

Standalone

• Parity: AI-credit reporting (aic) unverified and max-ai-credits budget not enforced on the external detector path #698 (detector half — enforce the credit budget) — blocked on the empirical aic verification called for in the issue. Do the verification first, comment the evidence, then size the PR. Don't batch it; the answer may be "parsing already works, only the budget is missing".
• Parity: gh-aw's installer supports threat-detect-darwin-* assets that we never publish #702 (darwin assets) — touches only .github/workflows/release.yml or the spec, depending on which option is chosen. Independent of everything else.

---

Ordering

PR 2 (gh-aw)  ──┬─→ PR 3 ──→ PR 4
                └─→ PR 1
                    PR 5   (independent)
                    #702   (independent)
                    #698   (after verification)

PR 1 and PR 2 are the critical path: PR 1 contains the live [!CAUTION]-on-infra-failure bug, and PR 2 unblocks the two silently-inert features. PR 5 and #702 can be picked up in parallel by anyone.