Skip to content

Harden external plugin PR quality gate rendering and name validation - #2444

Merged
aaronpowell merged 3 commits into
mainfrom
aaronpowell-fix-external-plugin-pr-quality-gates
Jul 27, 2026
Merged

Harden external plugin PR quality gate rendering and name validation#2444
aaronpowell merged 3 commits into
mainfrom
aaronpowell-fix-external-plugin-pr-quality-gates

Conversation

@aaronpowell

Copy link
Copy Markdown
Contributor

Summary

  • sanitize and safely render external plugin PR quality gate comment output in .github/workflows/external-plugin-pr-quality-gates.yml
  • enforce Open Plugins name constraints in eng/external-plugin-validation.mjs (length, allowed chars, boundary chars, no --/..)
  • fail changed external plugin entries early in eng/external-plugin-pr-quality-gates.mjs when canonical validation fails, skipping downstream gates for invalid entries

Why

This addresses security findings surfaced in #2398, including markdown/table injection vectors in PR status comments.

Validation

  • npm run plugin:validate
  • node --test eng/external-plugin-quality-gates.test.mjs
  • npm run build

Reference: #2398

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: a79923be-c65f-4d51-8fe3-a86e05fd02f1
Copilot AI review requested due to automatic review settings July 27, 2026 05:39
@github-actions github-actions Bot added the workflow PR touches workflow automation label Jul 27, 2026

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.

Pull request overview

Hardens external plugin validation and PR quality-gate output against malformed entries and Markdown injection.

Changes:

  • Enforces Open Plugins naming constraints.
  • Skips downstream checks for invalid entries.
  • Escapes rendered PR quality-gate output.
Show a summary per file
File Description
eng/external-plugin-validation.mjs Adds plugin-name validation rules.
eng/external-plugin-pr-quality-gates.mjs Fails invalid entries before downstream gates.
.github/workflows/external-plugin-pr-quality-gates.yml Sanitizes status-comment content and links.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread .github/workflows/external-plugin-pr-quality-gates.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Copilot AI review requested due to automatic review settings July 27, 2026 05:48

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.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread .github/workflows/external-plugin-pr-quality-gates.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Copilot AI review requested due to automatic review settings July 27, 2026 05:53
@aaronpowell
aaronpowell merged commit 8e137c3 into main Jul 27, 2026
18 of 19 checks passed
@aaronpowell
aaronpowell deleted the aaronpowell-fix-external-plugin-pr-quality-gates branch July 27, 2026 05:57

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.

Review details

Comments suppressed due to low confidence (1)

.github/workflows/external-plugin-pr-quality-gates.yml:274

  • Sanitizing the name only for the table does not close the injection path. The same invalid entry.name is later passed to formatGateOutput, where escapeHtml leaves newlines and Markdown delimiters intact. A name such as bad\n\n[link](https://example.com) fails validation but still produces gate details; the blank line ends the raw HTML block and GitHub can parse the following link as Markdown. Encode all non-text characters (or at least normalize line breaks and Markdown syntax) before placing the name inside <summary>.
                  const name = escapeMarkdownTableCell(entry?.name || 'unknown');
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

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

Labels

workflow PR touches workflow automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants