Repository navigation
fix(artifacts): emit ERR_VALIDATION warnings for degraded/missing inputs - #740
Closed
davidslater wants to merge 3 commits into
Closed
davidslater wants to merge 3 commits into
davidslater wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds degraded-artifact validation to prevent threat detection from silently passing with unusable inputs.
Changes:
- Emits
ERR_VALIDATIONwarnings and structured run-log events. - Fails closed when all primary inputs are unavailable.
- Adds artifact validation tests and updates existing CLI fixtures.
Show a summary per file
| File | Description |
|---|---|
pkg/artifacts/artifacts.go |
Detects degraded inputs and records metadata. |
pkg/artifacts/artifacts_test.go |
Tests artifact validation scenarios. |
cmd/threat-detect/main.go |
Emits warnings and enforces fail-closed behavior. |
cmd/threat-detect/main_test.go |
Adds valid artifact fixtures to engine tests. |
cmd/threat-detect/logfile_test.go |
Updates logging tests with valid fixtures. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Balanced
Comment on lines
+180
to
+194
| // Cross-check against HAS_PATCH: gh-aw sets this env var when the agent | ||
| // job reported a patch, so its absence here means the patch/bundle never | ||
| // made it into the artifacts directory. | ||
| if os.Getenv("HAS_PATCH") == "true" && len(arts.PatchFiles) == 0 { | ||
| arts.addWarning("patch", fmt.Sprintf( | ||
| "%s: HAS_PATCH=true was set but no aw-*.patch or aw-*.bundle file was found in %s. Detection will run without patch context.", | ||
| errCodeValidation, dir)) | ||
| } | ||
|
|
||
| // All three primary inputs missing simultaneously means the detector would | ||
| // otherwise analyze nothing and return a clean verdict: a fail-open failure | ||
| // mode in a security control. Flag it so the caller can hard-fail instead. | ||
| arts.AllPrimaryInputsMissing = (promptMissing || promptEmpty) && | ||
| (agentOutputMissing || agentOutputEmpty || agentOutputInvalid) && | ||
| len(arts.PatchFiles) == 0 |
Comment on lines
+214
to
+218
| if arts.AllPrimaryInputsMissing { | ||
| fmt.Fprintf(os.Stderr, "Error: prompt, agent output, and patch/bundle files are all missing or empty in %s; refusing to run detection on empty input.\n", artifactsDir) | ||
| logger.Error("artifacts_all_primary_inputs_missing", map[string]any{"artifacts_dir": artifactsDir}) | ||
| reason = reasonConfigError | ||
| return exitError |
| // they are visible even when this binary runs outside gh-aw's own | ||
| // "Prepare threat detection files" step. See pkg/artifacts.Load. | ||
| for _, w := range arts.Warnings { | ||
| fmt.Fprintf(os.Stderr, "::warning::%s\n", w.Message) |
davidslater
force-pushed
the
ace/01KZ72A8XZ9CVXB5ADKZEVVDR3
branch
2 times, most recently
from
August 4, 2026 22:02
36ff202 to
65f0bad
Compare
Previously pkg/artifacts.Load silently substituted sentinel strings for missing prompt.txt, agent_output.json, and patch/bundle files, so a staging bug (wrong artifact name, failed download, empty file) degraded detection to a silent no-op that returned a clean verdict — a fail-open failure mode in a security control. - Load now emits an ERR_VALIDATION warning (matching gh-aw's error-code prefix) for: missing/empty prompt.txt, missing/empty/invalid-JSON agent_output.json, and HAS_PATCH=true with no aw-*.patch/.bundle found. - Adds Artifacts.Warnings ([]ArtifactWarning) and PromptFileSize / AgentOutputFileSize fields for size-aware diagnostics. - Adds Artifacts.AllPrimaryInputsMissing: when prompt, agent output, and patch are all missing/empty simultaneously, main.go now hard-fails with config_error (exit 2) instead of proceeding to a clean pass. - main.go surfaces each warning as a ::warning:: annotation and mirrors it (plus the hard-fail) into the JSONL run log. - Adds tests for each degraded-input case in pkg/artifacts. Fixes #701 Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
- Treat zero-length/unreadable patch or bundle files as absent for both the HAS_PATCH cross-check and AllPrimaryInputsMissing, closing the loophole where an empty patch file bypassed the fail-closed check. - Escape warning messages with escapeWorkflowData before emitting them as ::warning:: annotations, since they embed the caller-supplied artifacts directory path and could otherwise forge additional workflow commands. - Add run() tests covering the empty-artifacts-directory fail-closed path (with and without --log-file), and artifacts tests for zero-length patch files. Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
davidslater
force-pushed
the
ace/01KZ72A8XZ9CVXB5ADKZEVVDR3
branch
from
August 4, 2026 22:19
65f0bad to
74665bb
Compare
Rebasing onto main's #744 (prompt-analysis degradation warnings) surfaced two tests that built empty artifact directories, which now trip the all-primary-inputs-missing hard-fail path added by this PR. Give them minimal valid prompt.txt/agent_output.json via writeMinimalArtifacts so they exercise only the prompt-template/import-tree degradation they're actually testing. Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
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.
Problem
pkg/artifacts.Loadsilently substituted sentinel strings ("No prompt file found", etc.) for missing or degraded inputs, so a staging bug (wrong artifact name, failed download, empty file) degraded detection to a no-op that passes with a clean verdict — a fail-open failure mode in a security control. Fixes #701.Changes
Loadnow emits anERR_VALIDATION-prefixed warning (matching gh-aw'serror_codes.cjsconvention) for:aw-prompts/prompt.txtagent_output.jsonHAS_PATCH=trueset in the environment but noaw-*.patch/aw-*.bundlefoundArtifactsfields:Warnings []ArtifactWarning,PromptFileSize,AgentOutputFileSize, andAllPrimaryInputsMissing.main.gonow returns exit 2 (config_error) instead of proceeding to analyze nothing and returning a clean verdict.main.gosurfaces each warning as a::warning::GitHub Actions annotation on stderr and mirrors it (plus the hard-fail) into the JSONL run log (artifact_degraded/artifacts_all_primary_inputs_missingevents).pkg/artifacts/artifacts_test.gofor each degraded-input case (missing/empty prompt, missing/empty/invalid agent output,HAS_PATCHcross-check, all-inputs-missing hard-fail, and the clean-pass case with no warnings).cmd/threat-detecttests that used empty artifact directories to include a minimal valid prompt/agent-output (via a newwriteMinimalArtifactshelper), since those tests exercise engine/sink behavior, not artifact validation, and would otherwise now hit the new hard-fail path.Out of scope (tracked separately)
prompt-template.txt/prompt-import-tree.jsonstaging (Parity: prompt-template.txt / prompt-import-tree.json are never staged, so trusted-vs-untrusted prompt analysis is silently disabled #686) andcomment-memory/analysis (Parity: comment-memory files are not analyzed (missing {COMMENT_MEMORY_FILES} prompt section and artifact discovery) #689) are not yet implemented in this repo at all, so warnings for them are deferred to those issues.Testing
make fmt-check— cleango vet ./...— cleango test ./...— all packages passmake build— succeeds