Repository navigation
Provision Node.js for external Copilot threat detection - #59314
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Custom-command Copilot detections currently fail because no executable is installed or passed to the detector.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Node.js provisioning and ARC/DinD staging for external Copilot threat-detection jobs.
Changes:
- Adds and deduplicates Node.js runtime setup.
- Propagates custom engine commands and adds regression coverage.
- Regenerates affected workflow lock files.
- Critical blocker: inherited custom commands suppress Copilot installation but are not passed to
threat-detect, causing runtime failure.
File summaries
| File | Description |
|---|---|
pkg/workflow/threat_detection_job_runtime_test.go |
Tests runtime setup and custom commands. |
pkg/workflow/threat_detection_job_combinations_integration_test.go |
Tests compiled setup ordering. |
pkg/workflow/threat_detection_helpers.go |
Inherits custom commands; contains the critical blocker. |
pkg/workflow/threat_detection_external.go |
Emits Node.js and ARC/DinD setup. |
.github/workflows/workflow-skill-extractor.lock.yml |
Adds detection Node.js setup. |
.github/workflows/workflow-health-manager.lock.yml |
Adds detection Node.js setup. |
.github/workflows/workflow-generator.lock.yml |
Adds detection Node.js setup. |
.github/workflows/weekly-editors-health-check.lock.yml |
Adds detection Node.js setup. |
.github/workflows/weekly-blog-post-writer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/visual-regression-checker.lock.yml |
Adds detection Node.js setup. |
.github/workflows/unbloat-docs.lock.yml |
Adds detection Node.js setup. |
.github/workflows/ubuntu-image-analyzer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/tidy.lock.yml |
Adds detection Node.js setup. |
.github/workflows/test-quality-sentinel.lock.yml |
Adds detection Node.js setup. |
.github/workflows/technical-doc-writer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/stale-pr-cleanup.lock.yml |
Adds detection Node.js setup. |
.github/workflows/smoke-issues.lock.yml |
Adds detection Node.js setup. |
.github/workflows/slide-deck-maintainer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/security-compliance.lock.yml |
Adds detection Node.js setup. |
.github/workflows/repository-quality-improver.lock.yml |
Adds detection Node.js setup. |
.github/workflows/refiner.lock.yml |
Adds detection Node.js setup. |
.github/workflows/pr-triage-agent.lock.yml |
Adds detection Node.js setup. |
.github/workflows/pr-sous-chef.lock.yml |
Adds detection Node.js setup. |
.github/workflows/pr-nitpick-reviewer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/pr-description-caveman.lock.yml |
Adds detection Node.js setup. |
.github/workflows/pr-code-quality-reviewer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/plan.lock.yml |
Adds detection Node.js setup. |
.github/workflows/mergefest.lock.yml |
Adds detection Node.js setup. |
.github/workflows/mcp-inspector.lock.yml |
Adds detection Node.js setup. |
.github/workflows/jsweep.lock.yml |
Adds detection Node.js setup. |
.github/workflows/glossary-maintainer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/firewall-escape.lock.yml |
Adds detection Node.js setup. |
.github/workflows/evoskill-evolver.lock.yml |
Adds detection Node.js setup. |
.github/workflows/eslint-miner.lock.yml |
Adds detection Node.js setup. |
.github/workflows/draft-pr-cleanup.lock.yml |
Adds detection Node.js setup. |
.github/workflows/docs-noob-tester.lock.yml |
Adds detection Node.js setup. |
.github/workflows/dictation-prompt.lock.yml |
Adds detection Node.js setup. |
.github/workflows/dev-hawk.lock.yml |
Adds detection Node.js setup. |
.github/workflows/design-decision-gate.lock.yml |
Adds detection Node.js setup. |
.github/workflows/deployment-incident-monitor.lock.yml |
Adds detection Node.js setup. |
.github/workflows/dependabot-burner.lock.yml |
Adds detection Node.js setup. |
.github/workflows/delight.lock.yml |
Adds detection Node.js setup. |
.github/workflows/dataflow-pr-discussion-dataset.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-workflow-updater.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-trajectory-grader-implementer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-team-status.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-syntax-error-quality.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-squid-image-scan.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-spec-coverage-kiro.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-spdd-spec-planner.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-security-observability.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-secrets-analysis.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-schema-audit-cursor.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-safe-output-integrator.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-repo-chronicle.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-regression-audit-kiro.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-pr-review-cursor.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-performance-summary.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-news.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-multi-device-docs-tester.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-model-inventory.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-mcp-concurrency-analysis.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-issues-report.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-hippo-learn.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-graft-intelligence.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-geo-optimizer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-function-namer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-formal-spec-verifier.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-firewall-report.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-file-diet.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-experiment-report.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-compiler-threat-spec-optimizer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-compiler-quality.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-community-attribution.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-code-metrics.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-byok-ollama-test.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-assign-issue-to-user.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-ambient-context-optimizer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-agent-of-the-day-blog-writer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/daily-action-setup-security-audit.lock.yml |
Adds detection Node.js setup. |
.github/workflows/craft.lock.yml |
Adds detection Node.js setup. |
.github/workflows/copilot-pr-prompt-analysis.lock.yml |
Adds detection Node.js setup. |
.github/workflows/copilot-pr-nlp-analysis.lock.yml |
Adds detection Node.js setup. |
.github/workflows/copilot-pr-merged-report.lock.yml |
Adds detection Node.js setup. |
.github/workflows/copilot-opt.lock.yml |
Adds detection Node.js setup. |
.github/workflows/copilot-cli-deep-research.lock.yml |
Adds detection Node.js setup. |
.github/workflows/contribution-check.lock.yml |
Adds detection Node.js setup. |
.github/workflows/constraint-solving-potd.lock.yml |
Adds detection Node.js setup. |
.github/workflows/code-simplifier.lock.yml |
Adds detection Node.js setup. |
.github/workflows/code-scanning-fixer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/cli-consistency-checker.lock.yml |
Adds detection Node.js setup. |
.github/workflows/ci-coach.lock.yml |
Adds detection Node.js setup. |
.github/workflows/breaking-change-checker.lock.yml |
Adds detection Node.js setup. |
.github/workflows/architecture-guardian.lock.yml |
Adds detection Node.js setup. |
.github/workflows/archie.lock.yml |
Adds detection Node.js setup. |
.github/workflows/agentic-token-audit.lock.yml |
Adds detection Node.js setup. |
.github/workflows/agent-persona-explorer.lock.yml |
Adds detection Node.js setup. |
.github/workflows/agent-performance-analyzer.lock.yml |
Adds detection Node.js setup. |
Review details
- Files reviewed: 116/116 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
The Node.js provisioning fix is headed in the right direction, but the custom-command Copilot path is still broken in the external detector execution flow.
Blocking theme
When engine.command is inherited for threat detection, this patch correctly suppresses the standard Copilot CLI install step, but buildThreatDetectCommand still launches threat-detect with only --engine copilot. That means the detector never learns about the custom binary path and still tries to execute copilot from PATH, which is missing by design in the inherited-command case.
The added tests mostly validate install-step shape and ordering, not the generated detector command, so they currently miss the actual regression surface.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 54 AIC · ⌖ 7.42 AIC · ⊞ 23.5K
Comment /review to run again
Comments that could not be inline-anchored
pkg/workflow/threat_detection_external.go:433
Inheriting engine.command into the external detector config fixes only half of the custom-command path: buildThreatDetectCommand still invokes threat-detect --engine copilot, so the detector never receives /opt/custom/copilot and falls back to looking for copilot on PATH. In the inherited-command case you explicitly skip installing the standard CLI, which turns this into a deterministic runtime failure for every custom-command workflow.
<details><summary>💡 Why this blocks the change…
🏗️ ADR Required — draft added for PR #59314I enforced the design-decision gate for this PR because the prefetch summary shows 130 added lines in default business logic directories, which is above the 100-line threshold. Evidence used
Architectural decision inferred from the PR
Action taken
Next action for the authorPlease review and refine the draft ADR, especially the decision scope and trade-offs, before merging this PR.
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /codebase-design to the core logic changes in threat_detection_external.go, threat_detection_helpers.go, and nodejs.go. This is a well-scoped, well-tested infra fix.
📋 Key Themes & Highlights
Positive Highlights
- ✅ Root cause fix, not a patch:
engineRequiresNodeHarnesscorrectly detects the actual need (harness script requiresnode), rather than hardcoding "copilot always needs it." - ✅ Good regression coverage:
TestBuildInstallDetectionEngineForExternalDetectorStepIncludesNodeRuntimecovers standard/ARC-DinD topologies, custom commands (detection-level and inherited), and asserts Claude/Codex do NOT get a duplicateSetup Node.jsstep — exactly the dedup risk this change introduces. - ✅ Real bug fix bundled in:
resolveExternalDetectorEngineConfigwas missingCommandpropagation fromdata.EngineConfig— this is caught and fixed, with a test case (copilot inherited custom command) exercising it. - ✅ Ordering assertions (
Setup Node.jsbefore CLI install, ARC/DinD redirect → node setup → daemon-path staging) are asserted with explicit index comparisons rather than just substring presence — good defense against future ordering regressions. - ✅ Deduplication logic (
installStepsContainNodeSetup) reuses the sameextractStepNamematcher as the JobManager's duplicate-step validator, so the guard can't drift from what the validator flags.
Minor Observations (non-blocking)
- The
(nolint/redacted):largefunccomment added tobuildExternalDetectorExecutionStepis on an unrelated, pre-existing function. Worth double-checking in a follow-up that this suppression isn't masking function growth that should eventually be split, per/codebase-design's "deep modules, not just larger ones" principle — but not a blocker for this PR since the function itself wasn't touched substantively. - 116 changed files are almost entirely regenerated
.lock.ymlfiles (expected/mechanical); the actual logic change is small and isolated to 4 Go files, which is the right shape for a fix like this.
Build (go build ./pkg/workflow/...) and all touched tests (go test ./pkg/workflow/ -run "ThreatDetection|InstallDetectionEngine") pass locally.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 97.9 AIC · ⌖ 14.7 AIC · ⊞ 10.3K
Comment /matt to run again
There was a problem hiding this comment.
Reviewed the Node.js provisioning change for external Copilot threat detection (threat_detection_external.go, threat_detection_helpers.go) plus tests.
- Confirmed
go build ./...succeeds and the new/targeted tests (TestBuildInstallDetectionEngineForExternalDetectorStepIncludesNodeRuntime,TestBuildDetectionJobStepsCodex*) pass. - Verified Node.js setup is injected exactly once, ordered before engine install steps, and correctly deduplicated for engines that already bundle Node (Claude/Codex), including custom-command paths.
- ARC/DinD tool-cache redirect + daemon-visible Node staging are only emitted when Node setup is actually present, matching the compiled
.lock.ymldiffs across the affected workflows. Commandpropagation into the detector's inheritedEngineConfiglooks correct and covered by a dedicated test case.
This is an internal Go compiler/runtime-provisioning change with no UI surface, so the Impeccable UX/audit modes don't apply; I did a standard correctness/security review instead. No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 72.8 AIC · ⌖ 13.3 AIC · ⊞ 8.3K
|
@copilot please address the remaining blocking review on this PR, then run the Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in commit |
@copilot Please take the next forward-progress pass on PR #59314.
Failed checks noted in the compact queue:
Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
@copilot Please take the next forward-progress pass on PR #59314.
Failed checks noted in the compact queue:
|
PR finisher triage completed. The custom-command blocker is addressed in |
The review thread has a substantive follow-up on current HEAD from |
|
Great work! 🚀 This PR fixes the external Copilot threat-detection Node.js provisioning issue (#59235) with solid regression coverage across standard, ARC/DinD, and custom-command scenarios. The lock-file updates are focused and the test plan is clear. This looks ready for review. Note: As a trusted core team contribution by the Copilot SWE Agent, this follows the project's agentic development workflow perfectly.
|
|
@copilot please address the current blockers on this PR: Failed checks:
Also refresh the branch if needed and run the Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
@copilot Please take the next forward-progress pass on PR #59314.
Failed checks noted in the compact queue:
Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
Rechecked PR #59314 at |
Investigated step 7. The logged |
|
🎉 This pull request is included in a new release. Release: |
External Copilot threat-detection jobs omitted Node.js setup, causing AWF startup failures when the runner’s ambient Node installation was unavailable inside the chroot, particularly on ARC/DinD.
Runtime provisioning
ARC/DinD compatibility
Custom commands
Regression coverage
Run: https://github.com/github/gh-aw/actions/runs/34185177055
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
hub.lumenfield.workTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Branch refresh requested from run https://github.com/github/gh-aw/actions/runs/34190324308
Warning
Firewall blocked 2 domains
The following domains were blocked by the firewall during workflow execution:
hub.lumenfield.workregistry.npmjs.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
hub.lumenfield.workTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Warning
Firewall blocked 2 domains
The following domains were blocked by the firewall during workflow execution:
api.hub.lumenfield.workhub.lumenfield.work[!TIP]
api.hub.lumenfield.workis blocked because GitHub API access uses the built-in GitHub tools by default. Instead of addingapi.hub.lumenfield.worktonetwork.allowed, usetools.github.mode: gh-proxyfor direct pre-authenticated GitHub CLI access without requiring network access toapi.github.com:See GitHub Tools for more information on
gh-proxymode.To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.