Skip to content

feat(threat-detection): render detection.log in detection job with group/mask macros - #51255

Merged
pelikhan merged 8 commits into
mainfrom
copilot/create-log-file-helper
Aug 8, 2026
Merged

feat(threat-detection): render detection.log in detection job with group/mask macros#51255
pelikhan merged 8 commits into
mainfrom
copilot/create-log-file-helper

Conversation

CopilotAI commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Adds a JS helper to read detection.log and pipe it to the Actions log with secret redaction and workflow-command injection protection, then wires it into the detection job on both execution paths.

New helper: render_detection_log.cjs

  • Reads /tmp/gh-aw/threat-detection/detection.log
  • Applies redactBuiltInPatterns (defence-in-depth on top of file-level redaction by redact_secrets.cjs)
  • Masks MCP gateway tokens at the runner level via ::add-mask::
  • Wraps output in:
    • ::group::Detection Log / ::endgroup:: — collapsible section in the Actions log UI
    • ::stop-commands::<random-token> / ::<token>:: — prevents workflow-command-shaped lines in agent output from being interpreted by the runner

Detection job wiring (threat_detection_steps.go)

  • Adds buildRenderDetectionLogStep
  • Inserted as Step 7b on the inline engine path (after buildDetectionStepSummaryEchoStep)
  • Inserted as Step 11a on the external detector path (after buildExternalDetectorExecutionStep)
  • Step uses continue-on-error: true; a missing or empty log is a silent no-op

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 16.4 AIC · ⌖ 6.83 AIC · ⊞ 6.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 5.94 AIC · ⌖ 5.23 AIC · ⊞ 8.5K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.3 AIC · ⌖ 5.36 AIC · ⊞ 8.5K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/31240022087> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 17 AIC · ⌖ 5.44 AIC · ⊞ 6.1K ·

Comment /souschef to run again

…tion job render step
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title feat(threat-detection): add render_detection_log.cjs helper and detection job render stepfeat(threat-detection): render detection.log in detection job with group/mask macrosAug 8, 2026
CopilotAI requested a review from pelikhanAugust 8, 2026 00:48
Comment threadactions/setup/js/render_detection_log.cjs Outdated
Comment threadactions/setup/js/render_detection_log.cjs Outdated
…erToStdout helper
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

@pelikhanpelikhan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot move rendertostdout with mask secret and also take group title as input

…tdout.cjs
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Aug 8, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot move rendertostdout with mask secret and also take group title as input

Moved renderLogToStdout(groupTitle, content) — including the maskSecret / gateway-token logic — into a new render_log_to_stdout.cjs helper. render_detection_log.cjs now imports and delegates to it.

CopilotAI requested a review from pelikhanAugust 8, 2026 01:17
@pelikhan
pelikhan marked this pull request as ready for review August 8, 2026 01:19
CopilotAI balanced review requested due to automatic review settings August 8, 2026 01:19
@github-actions

github-actionsBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actionsBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Warning

Firewall blocked 1 domain

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

  • api.individual.githubcopilot.com

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

network:
allowed:
- defaults
- "api.individual.githubcopilot.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actionsBot commented Aug 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-actionsBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (30 additions detected).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

Copy link
Copy Markdown
Contributor

Great work on the threat-detection feature! 🎯 The new render_detection_log.cjs helper adds important security hardening with secret redaction and workflow-command injection protection.

A few things to discuss before merging:

  • Add tests — The new detection log rendering logic in render_detection_log.cjs should have unit tests covering:

    • Reading and masking MCP gateway tokens
    • Rendering the collapsible group
    • Handling edge cases (missing log file, empty log)
    • Verifying secret redaction works end-to-end
  • Lock file regeneration — This PR updates 274 .lock.yml workflow files. Were these regenerated as part of the feature, or should this be split into a separate maintenance PR?

If you'd like help adding tests for the detection log rendering, you can assign this to your AI agent:

Add unit tests for actions/render_detection_log.cjs. Create a test file at actions/__tests__/render_detection_log.test.cjs covering:
1. Reading a sample detection.log file and verifying MCP gateway token masking
2. Verifying output is wrapped in ::group:: / ::endgroup:: markers
3. Verifying output is wrapped in ::stop-commands:: / ::<token>:: markers
4. Handling missing or empty log file (should be silent no-op per continue-on-error)
5. Verifying secret redaction applies built-in patterns (credit cards, API keys, etc.)

Generated by ✅ Contribution Check · auto · 89.7 AIC · ⌖ 2.91 AIC · ⊞ 8.7K ·

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

Pull request overview

Adds secure rendering of threat-detection logs to GitHub Actions output.

Changes:

  • Adds redaction, secret masking, grouping, and command-injection protection.
  • Wires rendering into both detection execution paths.
  • Regenerates affected workflow lockfiles.
Show a summary per file
FileDescription
actions/setup/js/render_detection_log.cjsReads and redacts detection logs.
actions/setup/js/render_detection_log.test.cjsTests rendering behavior.
actions/setup/js/render_log_to_stdout.cjsEmits protected grouped output.
pkg/workflow/threat_detection_steps.goAdds the rendering step to detection jobs.
.github/workflows/*.lock.ymlRegenerates affected workflows with the new step.

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

actions/setup/js/render_detection_log.test.cjs:116

  • This assertion also expects the obsolete detection-log- prefix, while the renderer emits render-<hex>. As written, the trailing-newline test fails even when the marker is correctly placed on its own line.
  • Files reviewed: 275/275 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment threadactions/setup/js/render_detection_log.test.cjs Outdated
Comment threadactions/setup/js/render_detection_log.cjs Outdated
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Quick triage nudge for this PR.

Please refresh the branch if GitHub can update it cleanly, verify the remaining blocking reviews are now resolved or ready for dismissal, and run the pr-finisher skill before handing back to maintainers.

Open items (newest first):

  • blocking review from @github-actions: confirm the previously requested stop-token test fixes are fully satisfied by the latest commit and clear any stale review state
  • blocking review from @github-actions: confirm helper extraction coverage is complete and no maintainer-facing follow-up remains
  • re-check whether branch refresh is still needed after the latest author follow-up

Run: https://github.com/github/gh-aw/actions/runs/31235166066

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 5.94 AIC · ⌖ 5.23 AIC · ⊞ 8.5K ·
Comment /souschef to run again

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

Copy link
Copy Markdown
Collaborator

@copilot cap size to a reasonable size before cutting out file content

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Aug 8, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cap size to a reasonable size before cutting out file content

Added in the latest commit. render_detection_log.cjs now defines MAX_LOG_BYTES = 1 MiB and uses fs.readSync to read only the first 1 MiB when the file exceeds that limit, emitting a core.warning with the actual vs. capped size. Files within the limit continue to use readFileSync as before. A new test covers the truncation path.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot render log from file as a helper function, including file size capping

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Aug 8, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot render log from file as a helper function, including file size capping

Done in the latest commit. Extracted renderLogFromFile(filePath, groupTitle) from main() — it encapsulates the file existence check, lstatGuard, size capping (with truncation warning), readFileSync/readSync, and redactBuiltInPatterns + renderLogToStdout calls. main() now simply delegates to it. The function is exported for reuse. Three new tests cover the custom group title, missing-file no-op, and truncation paths.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Quick triage nudge for this PR.

Please refresh the branch if GitHub can update it cleanly, verify the responded review threads are ready for resolution, address any remaining failed checks, and run the pr-finisher skill before handing back to maintainers.

Open items (newest first):

Branch refresh was requested.

Run: https://github.com/github/gh-aw/actions/runs/31238095490

Generated by 👨🍳 PR Sous Chef
Comment /souschef to run again

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.3 AIC · ⌖ 5.36 AIC · ⊞ 8.5K ·
Comment /souschef to run again

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

CopilotAI commented Aug 8, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Quick triage nudge for this PR.

Please refresh the branch if GitHub can update it cleanly, verify the responded review threads are ready for resolution, address any remaining failed checks, ...

Fixed in the latest commit — render_detection_log.cjs now passes Prettier formatting checks. All 15 tests still pass.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Quick triage nudge for this PR.

Please re-check reviewer feedback state, confirm whether any maintainer-facing follow-up still remains, and run the pr-finisher skill before handing back to maintainers.

Open items (newest first):

  • no remaining unresolved review threads detected
  • no recorded failed checks in compact candidate data
  • verify whether any stale blocking review state can now be cleared by maintainers

Branch refresh was requested.

Run: https://github.com/github/gh-aw/actions/runs/31240022087

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 17 AIC · ⌖ 5.44 AIC · ⊞ 6.1K ·
Comment /souschef to run again

CopilotAI commented Aug 8, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Quick triage nudge for this PR.

Please re-check reviewer feedback state, confirm whether any maintainer-facing follow-up still remains, and run the pr-finisher skill before handing back to...

All open items are addressed. Local validation is green: make fmt, JS lint (prettier), full Go unit test suite, and all 392 JS test files (12,802 tests) pass. The pre-existing Go lint warnings in codemod_bash_allowlist_unsupported_engine.go are unrelated to this PR. No uncommitted changes remain. CI needs to be re-triggered by a maintainer (close/reopen or push) to confirm green on the current HEAD before merge.

@pelikhan
pelikhan merged commit 84579ec into mainAug 8, 2026
1 check passed
@pelikhan
pelikhan deleted the copilot/create-log-file-helper branch August 8, 2026 05:35
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.86.2

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@gh-aw-bot@pelikhan