diff --git a/.github/workflows/external-plugin-pr-quality-gates.yml b/.github/workflows/external-plugin-pr-quality-gates.yml index 3010788b8..6978738e6 100644 --- a/.github/workflows/external-plugin-pr-quality-gates.yml +++ b/.github/workflows/external-plugin-pr-quality-gates.yml @@ -214,6 +214,34 @@ jobs: .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, '''); + const escapeMarkdownTableCell = (value) => + String(value ?? '') + .replace(/\r\n?|\n/g, '\n') + .split('\n') + .map((line) => + Array.from(line, (character) => + /^[A-Za-z0-9 .-]$/.test(character) + ? character + : `&#${character.codePointAt(0)};` + ).join('') + ) + .join('
'); + const normalizeGitHubUrl = (value) => { + const raw = String(value || '').trim(); + if (!raw) { + return ''; + } + + try { + const parsed = new URL(raw); + if (parsed.protocol !== 'https:' || parsed.hostname !== 'github.com') { + return ''; + } + return parsed.toString(); + } catch { + return ''; + } + }; const TRUNCATED_OUTPUT_MARKER = '\n...output truncated...'; const truncateGateOutput = (rawOutput) => { const normalized = escapeHtml(String(rawOutput || '').trim()); @@ -243,12 +271,17 @@ jobs: const rows = checkedPlugins.length > 0 ? checkedPlugins.map((entry) => { - const name = String(entry?.name || 'unknown'); + const name = escapeMarkdownTableCell(entry?.name || 'unknown'); const quality = entry?.quality || {}; - const sourceUrl = String(entry?.source_tree_url || ''); - const locator = String(entry?.source?.sha || entry?.source?.ref || 'repository'); - const sourceCell = sourceUrl ? `[${locator}](${sourceUrl})` : locator; - return `| ${name} | ${quality.vally_lint_status || 'not_run'} | ${quality.smoke_status || 'not_run'} | ${quality.version_match_status || 'not_run'} | ${quality.canvas_structure_status || 'not_run'} | ${quality.overall_status || 'not_run'} | ${sourceCell} |`; + const sourceUrl = normalizeGitHubUrl(entry?.source_tree_url || ''); + const locator = escapeMarkdownTableCell(entry?.source?.sha || entry?.source?.ref || 'repository'); + const sourceCell = sourceUrl ? `[${locator}](<${sourceUrl}>)` : locator; + const vallyLintStatus = escapeMarkdownTableCell(quality.vally_lint_status || 'not_run'); + const smokeStatus = escapeMarkdownTableCell(quality.smoke_status || 'not_run'); + const versionMatchStatus = escapeMarkdownTableCell(quality.version_match_status || 'not_run'); + const canvasStructureStatus = escapeMarkdownTableCell(quality.canvas_structure_status || 'not_run'); + const overallStatus = escapeMarkdownTableCell(quality.overall_status || 'not_run'); + return `| ${name} | ${vallyLintStatus} | ${smokeStatus} | ${versionMatchStatus} | ${canvasStructureStatus} | ${overallStatus} | ${sourceCell} |`; }) : ['| _none_ | not_run | not_run | not_run | not_run | not_run | _n/a_ |']; const failureDetails = checkedPlugins.flatMap((entry) => { @@ -296,7 +329,9 @@ jobs: '', ] : []), - String(qualityResult.summary || '').trim() || '_No summary provided._', + String(qualityResult.summary || '').trim() + ? `
${escapeHtml(String(qualityResult.summary).trim())}
` + : '_No summary provided._', ].join('\n'); await intakeState.upsertExternalPluginIntakeComment({ diff --git a/eng/external-plugin-pr-quality-gates.mjs b/eng/external-plugin-pr-quality-gates.mjs index 85cd6d9c0..07d4a3308 100644 --- a/eng/external-plugin-pr-quality-gates.mjs +++ b/eng/external-plugin-pr-quality-gates.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node import { runExternalPluginQualityGates } from "./external-plugin-quality-gates.mjs"; +import { validateExternalPlugin } from "./external-plugin-validation.mjs"; function normalizePluginPath(pluginPath) { if (!pluginPath || pluginPath === "/") { @@ -66,13 +67,33 @@ function aggregateResultStatus(pluginResults) { }; } +function createValidationFailureQuality(errors) { + const output = errors.map((error) => `- ${error}`).join("\n"); + return { + overall_status: "fail", + vally_lint_status: "fail", + smoke_status: "not_run", + version_match_status: "not_run", + canvas_structure_status: "not_run", + failure_class: "submitter_fixes", + summary: "Plugin entry failed external.json validation. Fix the listed errors and re-run quality checks.", + vally_lint_output: output, + smoke_output: "Install smoke test skipped due to external.json validation errors.", + version_match_output: "Version match skipped due to external.json validation errors.", + canvas_structure_output: "Canvas structure check skipped due to external.json validation errors.", + }; +} + export async function runExternalPluginPrQualityGates(plugins) { if (!Array.isArray(plugins)) { throw new Error("plugins must be an array"); } const checkedPlugins = await Promise.all(plugins.map(async (plugin) => { - const quality = await runExternalPluginQualityGates(plugin); + const validation = validateExternalPlugin(plugin, "changed-plugin", { policy: "marketplace" }); + const quality = validation.errors.length > 0 + ? createValidationFailureQuality(validation.errors) + : await runExternalPluginQualityGates(plugin); return { name: plugin?.name ?? "unknown", source: plugin?.source ?? {}, diff --git a/eng/external-plugin-validation.mjs b/eng/external-plugin-validation.mjs index 846458832..0a5ce2c13 100644 --- a/eng/external-plugin-validation.mjs +++ b/eng/external-plugin-validation.mjs @@ -60,12 +60,20 @@ function validatePluginName(name, prefix, errors) { return; } - if (name.length > 50) { - errors.push(`${prefix}: "name" must be 50 characters or fewer`); + if (name.length > 64) { + errors.push(`${prefix}: "name" must be 64 characters or fewer`); } - if (!/^[a-z0-9-]+$/.test(name)) { - errors.push(`${prefix}: "name" must contain only lowercase letters, numbers, and hyphens`); + if (!/^[a-z0-9.-]+$/.test(name)) { + errors.push(`${prefix}: "name" must contain only lowercase letters, numbers, hyphens, and periods`); + } + + if (!/^[a-z0-9].*[a-z0-9]$/.test(name) && !/^[a-z0-9]$/.test(name)) { + errors.push(`${prefix}: "name" must start and end with a lowercase letter or number`); + } + + if (name.includes("--") || name.includes("..")) { + errors.push(`${prefix}: "name" must not contain consecutive hyphens or periods`); } }