fix(detection): prepend npm PATH setup to threat-detect command so engine CLIs are found in AWF chroot#40876
Conversation
…nd in AWF chroot Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
/smoke-claude |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
There was a problem hiding this comment.
Pull request overview
This PR fixes a threat-detection execution issue when running inside the AWF chroot by ensuring npm-installed engine CLIs (e.g., claude, codex) are discoverable on PATH before threat-detect spawns the engine subprocess.
Changes:
- Prepends
GetNpmBinPathSetup()to the external detector’sthreat-detectcommand so hosted toolcachebin/directories are onPATHinside the AWF chroot. - Extends the threat-detection isolation test to assert that the compiled detection job includes the npm toolcache PATH setup.
- Recompiles workflow lock files so generated
run:commands include the PATH setup.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/threat_detection.go | Prepends npm toolcache PATH setup to the AWF threat-detect command so engine CLIs are found in the chroot. |
| pkg/workflow/threat_detection_isolation_test.go | Adds an assertion intended to verify the compiled detection step includes the PATH setup. |
| .github/workflows/test-quality-sentinel.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/test-project-url-default.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/test-dispatcher.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/test-create-pr-error-handling.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-workflow-call.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-workflow-call-with-inputs.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-update-cross-repo-pr.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-test-tools.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-temporary-id.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-service-ports.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-project.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-pi.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-otel-backends.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-opencode.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-multi-pr.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-gemini.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-crush.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-copilot.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-copilot-sdk.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-copilot-arm.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-copilot-aoai-entra.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-copilot-aoai-apikey.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-codex.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-claude.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-call-workflow.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-antigravity.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-agent-scoped-approved.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-agent-public-none.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-agent-public-approved.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-agent-all-none.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/smoke-agent-all-merged.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/github-remote-mcp-auth-test.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/docs-noob-tester.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-repo-chronicle.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-rendering-scripts-verifier.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-reliability-review.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-performance-summary.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-observability-report.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-news.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-multi-device-docs-tester.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-model-inventory.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-mcp-concurrency-analysis.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-max-ai-credits-test.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-issues-report.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-hippo-learn.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-geo-optimizer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-function-namer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-formal-spec-verifier.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-file-diet.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-fact.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-experiment-report.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-doc-updater.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-doc-healer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-credit-limit-test.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-compiler-threat-spec-optimizer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-compiler-quality.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-community-attribution.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-code-metrics.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-cli-tools-tester.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-cli-performance.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-choice-test.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-caveman-optimizer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-cache-strategy-analyzer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-byok-ollama-test.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-awf-spec-compiler-surfacing.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-aw-cross-repo-compile-check.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-astrostylelite-markdown-spellcheck.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-architecture-diagram.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-ambient-context-optimizer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-agentrx-trace-optimizer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/daily-agent-of-the-day-blog-writer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/craft.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-session-insights.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-pr-merged-report.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-opt.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/copilot-agent-analysis.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/contribution-check.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/constraint-solving-potd.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/commit-changes-analyzer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/code-scanning-fixer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/cloclo.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/cli-version-checker.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/cli-consistency-checker.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/claude-code-user-docs-review.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/ci-doctor.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/ci-coach.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/chaos-pr-bundle-fuzzer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/changeset.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/breaking-change-checker.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/brave.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/blog-auditor.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/aw-failure-investigator.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/avenger.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/auto-triage-issues.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/audit-workflows.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/artifacts-summary.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/architecture-guardian.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/archie.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/approach-validator.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/api-consumption-report.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/agent-performance-analyzer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/agent-persona-explorer.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
| .github/workflows/ab-testing-advisor.lock.yml | Recompiled: detection execution step now includes npm toolcache PATH setup. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 111/111 changed files
- Comments generated: 1
| // The AWF execution step must prepend npm PATH setup so npm-installed engine CLIs | ||
| // (e.g. claude, codex) are found by threat-detect inside the AWF chroot. | ||
| if !strings.Contains(detectionSection, "RUNNER_TOOL_CACHE") { | ||
| t.Error("External detector AWF step must prepend npm PATH setup (RUNNER_TOOL_CACHE) so engine CLIs are on PATH") | ||
| } |
|
Smoke Test: Claude — Run 27987751554 Core #1-12: ✅ all passed Overall: PARTIAL (all executed tests passed) Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
💥 [THE END] — Illustrated by Smoke Claude · 81.6 AIC · ⌖ 35.3 AIC · ⊞ 8.6K
| // Prepend npm PATH setup so that npm-installed engine CLIs (e.g. claude, codex) | ||
| // can be found inside the AWF container's chroot environment. threat-detect | ||
| // invokes the engine binary as a subprocess and relies on PATH to locate it. | ||
| npmPathSetup := GetNpmBinPathSetup() |
There was a problem hiding this comment.
Nice — caching GetNpmBinPathSetup() in a local before formatting keeps the command construction readable.
| npmPathSetup := GetNpmBinPathSetup() | ||
| threatDetectCmd := fmt.Sprintf( | ||
| "threat-detect --engine %s --output %s %s", | ||
| "%s && threat-detect --engine %s --output %s %s", |
There was a problem hiding this comment.
Good use of '%s &&' to chain PATH setup before threat-detect — consistent with the engine execution steps.
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #40876 does not have the 'implementation' label and has only 12 new lines of code (threshold: 100) in business logic directories. |
🧪 Test Quality Sentinel Report✅ Test Quality Score: 100/100 — Excellent
📊 Metrics & Test Classification (1 test analyzed)
Go: 1 ( Verdict
|
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 86.2 AIC · ⌖ 7.08 AIC · ⊞ 6.5K
|
|
||
| // The AWF execution step must prepend npm PATH setup so npm-installed engine CLIs | ||
| // (e.g. claude, codex) are found by threat-detect inside the AWF chroot. | ||
| if !strings.Contains(detectionSection, "RUNNER_TOOL_CACHE") { |
There was a problem hiding this comment.
[/tdd] The assertion checks for presence of RUNNER_TOOL_CACHE but not ordering — a future refactor that accidentally moved the PATH setup after threat-detect would still pass this test.
💡 Suggested ordering check
pathSetupIdx := strings.Index(detectionSection, "RUNNER_TOOL_CACHE")
threatDetectIdx := strings.Index(detectionSection, "threat-detect --engine")
if pathSetupIdx == -1 {
t.Error("External detector AWF step must prepend npm PATH setup (RUNNER_TOOL_CACHE) so engine CLIs are on PATH")
} else if threatDetectIdx == -1 || pathSetupIdx >= threatDetectIdx {
t.Error("npm PATH setup (RUNNER_TOOL_CACHE) must appear before 'threat-detect --engine' in the AWF step")
}This enforces the prepend invariant the code comment describes, not just that the substring exists somewhere in the section.
There was a problem hiding this comment.
The logic change is correct and consistent with the pattern used across all other engine implementations (claude_engine.go, codex_engine.go, copilot_engine_execution.go, etc.). The 109 lock file regenerations are the expected mechanical output.
Review details
What was changed
buildExternalDetectorExecutionStep now prepends GetNpmBinPathSetup() to the threat-detect command so npm-installed engine CLIs (claude, codex, etc.) are on PATH inside the AWF chroot when threat-detect invokes them as subprocesses.
Shell chain analysis
The generated command ends with ... || true && threat-detect --engine .... Shell &&/|| associate left-to-right, so this parses as ((GOROOT_check || true) && threat-detect), which is always correct — threat-detect runs in both the GOROOT-set and GOROOT-unset cases. The critical find-based PATH export before it runs unconditionally via semicolons and is not part of this chain.
Finding
One non-blocking concern posted inline: the new test assertion checks for RUNNER_TOOL_CACHE presence but not that it precedes threat-detect --engine in the generated output — the "prepend" semantics the comment describes are not enforced by the test itself.
🔎 Code quality review by PR Code Quality Reviewer · 77.1 AIC · ⌖ 7.31 AIC · ⊞ 5.1K
| // The AWF execution step must prepend npm PATH setup so npm-installed engine CLIs | ||
| // (e.g. claude, codex) are found by threat-detect inside the AWF chroot. | ||
| if !strings.Contains(detectionSection, "RUNNER_TOOL_CACHE") { | ||
| t.Error("External detector AWF step must prepend npm PATH setup (RUNNER_TOOL_CACHE) so engine CLIs are on PATH") |
There was a problem hiding this comment.
Test does not enforce ordering: the assertion verifies RUNNER_TOOL_CACHE is present anywhere in detectionSection, but not that it appears before threat-detect --engine. A refactoring that accidentally reordered the PATH setup to after the command would not be caught.
💡 Suggested fix
Follow the same strings.Index ordering pattern already used a few lines down for parse_detection_token_usage vs detection_conclusion:
runnerToolCacheIdx := strings.Index(detectionSection, "RUNNER_TOOL_CACHE")
threatDetectIdx := strings.Index(detectionSection, "threat-detect --engine")
if runnerToolCacheIdx == -1 || threatDetectIdx == -1 || runnerToolCacheIdx >= threatDetectIdx {
t.Error("External detector AWF step must prepend npm PATH setup (RUNNER_TOOL_CACHE) before threat-detect --engine")
}This makes the "prepend" semantics load-bearing in the test, matching how ordering is enforced elsewhere in this same test function.
When
features: gh-aw-detection: trueis set,threat-detectruns inside the AWF chroot and spawns the engine CLI (e.g.claude) as a subprocess — but the hostedtoolcache bin dirs were not on PATH, causing:Changes
pkg/workflow/threat_detection.go— InbuildExternalDetectorExecutionStep, prependGetNpmBinPathSetup()tothreatDetectCmdso the hostedtoolcache bin directories (populated bynpm install -g) are on PATH beforethreat-detectexecutes. This mirrors the same pattern all engine execution steps already use.pkg/workflow/threat_detection_isolation_test.go—TestExternalDetectorPathnow assertsRUNNER_TOOL_CACHEPATH setup is present in the compiled detection execution step.✨ PR Review Safe Output Test - Run 27987751554
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.comSee Network Configuration for more information.