Skip to content

Provision Node.js for external Copilot threat detection - #59314

Open
pelikhan with Copilot wants to merge 4 commits into
mainfrom
copilot/update-threat-detection-setup
Open

Provision Node.js for external Copilot threat detection#59314
pelikhan with Copilot wants to merge 4 commits into
mainfrom
copilot/update-threat-detection-setup

Conversation

Copilot AI commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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

    • Add one Node.js setup step before external detector engine installation.
    • Preserve setup deduplication for engines that already provision Node.js.
  • ARC/DinD compatibility

    • Reuse shared tool-cache redirection and daemon-visible Node staging.
    • Preserve existing Copilot binary staging.
  • Custom commands

    • Propagate inherited custom engine commands into the detector context.
    • Avoid installing the standard Copilot CLI when a custom command is configured.
  • Regression coverage

    • Cover standard and ARC/DinD Copilot detectors.
    • Cover detection-specific and inherited custom commands.
    • Verify Claude and Codex do not receive duplicate Node setup.
    • Assert runtime setup ordering in compiled detection jobs.

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:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 45.4 AIC · ⌖ 8.71 AIC · ⊞ 9K ·
Comment /souschef to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix external Copilot threat detection to include Node.js setup Provision Node.js for external Copilot threat detection Sep 7, 2026
Copilot AI requested a review from pelikhan September 7, 2026 23:39
@pelikhan
pelikhan marked this pull request as ready for review September 8, 2026 02:43
Copilot AI balanced review requested due to automatic review settings September 8, 2026 02:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

return &EngineConfig{
ID: engineID,
Version: data.EngineConfig.Version,
Command: data.EngineConfig.Command,
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Ponytail Reviewer completed successfully!

Lean already. Ship.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Ponytail Reviewer for #59314

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Comment Memory

reviewed_at: 2026-09-08T03:51:32Z
review_event: REQUEST_CHANGES
top_themes:
  - external detector custom command is not passed into threat-detect execution
files_reviewed:
  - pkg/workflow/threat_detection_external.go
  - pkg/workflow/threat_detection_helpers.go
  - pkg/workflow/threat_detection_job_combinations_integration_test.go
  - pkg/workflow/threat_detection_job_runtime_test.go
  - pkg/workflow/threat_detection_external_detector_execution_test.go
  - .github/workflows/*.lock.yml
comment_count: 1

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 54 AIC · ⌖ 7.42 AIC · ⊞ 23.5K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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…

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor
🏗️ ADR Required — draft added for PR #59314

I 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

  • adr-prefetch-summary.json: requires_adr_by_default_volume: true
  • PR body: describes adding explicit Node.js setup for external Copilot threat-detection jobs, preserving setup deduplication, and handling custom engine commands
  • PR diff: adds Setup Node.js before Install GitHub Copilot CLI across generated threat-detection workflows and updates manifests accordingly
  • Existing ADR search: no ADR referenced in the PR body, and the latest ADRs on branch did not cover this decision

Architectural decision inferred from the PR

  • Decision: external Copilot threat-detection jobs should explicitly provision Node.js before Copilot CLI installation instead of relying on ambient runner state
  • Driver: threat-detection startup fails in ARC/DinD and similar chrooted environments when Node.js is unavailable inside the execution environment
  • Alternatives considered in the draft: rely on ambient Node.js, hide runtime provisioning inside Copilot install scripts, or provision Node.js for every detector engine unconditionally

Action taken

  • Added draft ADR: docs/adr/59314-provision-nodejs-for-external-copilot-threat-detection.md

Next action for the author

Please review and refine the draft ADR, especially the decision scope and trade-offs, before merging this PR.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · gpt54 · 29.7 AIC · ⌖ 12.3 AIC · ⊞ 10.1K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: engineRequiresNodeHarness correctly detects the actual need (harness script requires node), rather than hardcoding "copilot always needs it."
  • Good regression coverage: TestBuildInstallDetectionEngineForExternalDetectorStepIncludesNodeRuntime covers standard/ARC-DinD topologies, custom commands (detection-level and inherited), and asserts Claude/Codex do NOT get a duplicate Setup Node.js step — exactly the dedup risk this change introduces.
  • Real bug fix bundled in: resolveExternalDetectorEngineConfig was missing Command propagation from data.EngineConfig — this is caught and fixed, with a test case (copilot inherited custom command) exercising it.
  • ✅ Ordering assertions (Setup Node.js before 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 same extractStepName matcher 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):largefunc comment added to buildExternalDetectorExecutionStep is 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.yml files (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

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yml diffs across the affected workflows.
  • Command propagation into the detector's inherited EngineConfig looks 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

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please address the remaining blocking review on this PR, then run the pr-finisher skill and request another review. Current blocker: custom-command Copilot detections still are not passed through to threat-detect, so inherited-command workflows can fail at runtime.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 45.4 AIC · ⌖ 8.71 AIC · ⊞ 9K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please address the remaining blocking review on this PR, then run the pr-finisher skill and request another review. Current blocker: custom-command Copilot detections still are not passed t...

Addressed in commit 9976d02. External detector data now clears unsupported custom Copilot commands, so the standard Copilot CLI is installed and staged for threat-detect; regression coverage verifies both detection-specific and inherited custom-command cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

External Copilot threat detection omits Node.js setup and fails on ARC/DinD

4 participants