Harden external plugin validation: semver, SPDX, email, and unknown-field checks

[aaronpowell]

Summary

Extends the canonical external-plugin validator (eng/external-plugin-validation.mjs) with additional Open-Plugins-aligned validation rules. All checks reuse the shared validation functions rather than duplicating logic in scripts/workflows.

Origin: #2398. Follows #2444 (which landed the markdown/table-injection and stricter plugin.name hardening).

Validation enhancements

1. Semver version — enforces Semantic Versioning (major.minor.patch with optional -prerelease / +build metadata); non-semver → error. Accepts the existing committed 1.0.1161-preview1.
2. SPDX license — validates SPDX identifiers and expressions (AND/OR/WITH, parentheses, trailing +, LicenseRef-*). Malformed syntax → error; a well-formed but unrecognized identifier → warning.
3. author.email — format-validated only when the field is present.
4. Unknown-field detection — warns on typo'd/unknown top-level, author, and source keys (forward-compatible; warnings, not errors).
5. Immutable locator (evaluation of item 5) — kept the marketplace policy non-fatal because ~12 committed entries have no source.ref/source.sha. Added a new warnMissingImmutableLocator policy flag that warns for marketplace entries lacking an immutable locator, while publicSubmission retains its existing hard error.

Non-breaking decisions

• Unknown fields and the marketplace missing-immutable-locator case are warnings, not errors — this keeps npm run build / plugin:validate green and stays forward-compatible with future supported fields.
• SPDX validation warns (rather than errors) on well-formed-but-unrecognized identifiers, specifically so the already-committed SSAL-1.0 entry continues to pass marketplace validation.

Test coverage

New eng/external-plugin-validation.test.mjs (node:test, 9 tests) covering:

• semver accept/reject cases (incl. prerelease/build);
• SPDX ids, expressions, LicenseRef-*, warning on unknown id, errors on malformed;
• author.email present-only validation;
• unknown-field warnings for top-level/author/source;
• immutable-locator warning (marketplace) vs error (publicSubmission);
• a regression asserting the committed plugins/external.json passes the marketplace policy with zero errors.

Verification

• node --test eng/external-plugin-validation.test.mjs → 9 pass
• node --test eng/external-plugin-quality-gates.test.mjs → existing 7 pass
• npm run plugin:validate → valid (surfaces the intended warnings)
• npm run build → marketplace regenerates, no new errors
• bash eng/fix-line-endings.sh → line endings normalized

Docs

Updated CONTRIBUTING.md external-plugin section to document semver version, SPDX license, optional author.email, unknown-field warnings, and the marketplace immutable-locator warning.

[Copilot]

Pull request overview

Hardens canonical external-plugin metadata validation and documents the expanded rules.

Changes:

• Adds semver, SPDX, email, unknown-field, and immutable-locator validation.
• Adds nine validator tests, including committed-data regression coverage.
• Updates contributor documentation.

Show a summary per file

File	Description
eng/external-plugin-validation.mjs	Implements new validation rules and warnings.
eng/external-plugin-validation.test.mjs	Tests validator behavior and marketplace compatibility.
CONTRIBUTING.md	Documents external-plugin requirements and warnings.

Review details

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

[Copilot]

Review details

Comments suppressed due to low confidence (1)

eng/external-plugin-validation.mjs:343

• The license check does not actually distinguish malformed SPDX syntax from an unknown but well-formed identifier. Every parse failure becomes a warning (the test even includes MIT OR), while the recognizer also strips parentheses and treats WITH like AND/OR; this lets unbalanced expressions pass and warns on valid exception expressions such as GPL-2.0-only WITH Classpath-exception-2.0. This contradicts the stated contract that malformed syntax is an error. Please parse the SPDX grammar separately from identifier recognition, report syntax failures as errors, and reserve warnings for syntactically valid unknown IDs.

  if (!isRecognizedSpdxExpression(license)) {
    warnings.push(
      `${prefix}"license" value "${license}" is not a recognized SPDX identifier; prefer a standard SPDX id (https://spdx.org/licenses), though non-SPDX or proprietary licenses are allowed`
    );

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

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

[Copilot]

Review details

Comments suppressed due to low confidence (3)

eng/lib/license.mjs:230

• Backslashes do not escape backticks inside a Markdown code span. These warning strings are inserted directly into external-plugin intake comments (eng/external-plugin-intake.mjs:889-905), so a submitted license containing a backtick can close this span and render attacker-controlled Markdown (for example, a link), reintroducing the injection class this validation is meant to harden. Use a code-span fence longer than every backtick run in the value and add a regression case containing an embedded backtick.

function sanitizeForMessage(value) {
  const collapsed = String(value).replace(/\s+/g, " ").trim();
  const truncated = collapsed.length > 80 ? `${collapsed.slice(0, 77)}...` : collapsed;
  return `\`${truncated.replace(/`/g, "\\`")}\``;

eng/lib/license.mjs:261

• This single boolean conflates malformed SPDX syntax with a well-formed but unknown identifier, so malformed expressions such as MIT OR, unmatched parentheses, and invalid WITH clauses only warn. That contradicts the PR contract that malformed SPDX expressions are errors; the new tests at lines 118-133 explicitly codify the opposite behavior. Preserve a distinct grammar result and add malformed expressions to errors, reserving warnings for syntactically valid but unrecognized/non-SPDX license values.

  if (!isRecognizedSpdxExpression(license)) {
    warnings.push(
      `${prefix}"license" value ${sanitizeForMessage(license)} is not a recognized SPDX identifier; prefer a standard SPDX id (https://spdx.org/licenses), though non-SPDX or proprietary licenses are allowed`
    );

eng/lib/license.mjs:16

• The curated list causes false warnings for valid SPDX identifiers that are omitted here—for example, Artistic-2.0 is on the official SPDX License List but is reported as unrecognized. The exception list has the same drift problem. Use the canonical SPDX license/exception data (or a maintained SPDX expression parser) rather than a small hand-maintained subset so valid licenses are not mislabeled.

// Curated set of common SPDX license identifiers. Not exhaustive: unrecognized
// but well-formed identifiers produce a warning rather than an error.
const KNOWN_SPDX_IDS = new Set([

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

[Copilot]

Review details

Comments suppressed due to low confidence (2)

eng/lib/license.mjs:144

• The recursive parser has no input-length or nesting-depth bound. An untrusted license containing thousands of opening parentheses repeatedly enters parseOrExpression here until Node throws RangeError: Maximum call stack size exceeded, crashing the external-plugin intake workflow rather than returning a warning. Reject overlong expressions and/or enforce a small explicit nesting limit before recursing.

    if (token === "(") {
      consume();
      if (!parseOrExpression()) {

eng/lib/license.mjs:154

• Normalizing every candidate operator with toUpperCase() makes malformed expressions such as MIT or Apache-2.0, MIT and Apache-2.0, and lowercase with count as recognized SPDX and suppresses the intended warning. SPDX operators are case-sensitive uppercase tokens; compare them exactly throughout this parser.

    if (!token || token === ")" || ["AND", "OR", "WITH"].includes(token.toUpperCase())) {

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

[Copilot]

Review details

Comments suppressed due to low confidence (2)

eng/lib/license.mjs:230

• Backslashes do not escape backticks inside GitHub/CommonMark code spans. A submitted value such as x followed by Markdown can therefore close this span and render attacker-controlled links or formatting when the warning is forwarded to the intake comment (external-plugin-intake.mjs:969-970). Choose a backtick delimiter longer than any run in the value instead of prefixing backticks with \.

  return `\`${truncated.replace(/\\/g, "\\\\").replace(/`/g, "\\`")}\``;

eng/lib/license.mjs:16

• This small curated set causes valid official SPDX identifiers outside it (for example, Artistic-2.0) to be reported as “not a recognized SPDX identifier.” That makes the new SPDX validation produce false warnings and will drift further as the official list evolves. Use the official SPDX license data (or an SPDX expression package) rather than treating a hand-picked subset as the recognition source.

// Curated set of common SPDX license identifiers. Not exhaustive: unrecognized
// but well-formed identifiers produce a warning rather than an error.
const KNOWN_SPDX_IDS = new Set([

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

[Copilot]

Review details

Comments suppressed due to low confidence (3)

eng/lib/license.mjs:113

• Tokenization removes syntax information required to validate an SPDX expression: it accepts every whitespace kind and inserts separators around parentheses. Consequently malformed values such as MIT OR(Apache-2.0) and a multiline MIT\nOR\nApache-2.0 are reported as recognized, although SPDX requires ASCII spaces around operators and a single-line expression. Preserve token boundaries or reject invalid whitespace/operator adjacency before parsing.

function tokenizeSpdxExpression(license) {
  return license
    .replace(/([()])/g, " $1 ")
    .trim()
    .split(/\s+/)
    .filter((token) => token.length > 0);

eng/lib/license.mjs:14

• idstring must contain at least one alphanumeric character, but this pattern accepts punctuation-only values. As a result, malformed references such as LicenseRef-... and DocumentRef---:LicenseRef-... are classified as recognized SPDX references and emit no warning. Require at least one letter or digit.

This issue also appears on line 108 of the same file.

const SPDX_IDSTRING_PATTERN = /^[A-Za-z0-9.-]+$/;

eng/lib/license.mjs:146

• Parenthesis nesting is parsed recursively without any input-length or depth bound. Because license is attacker-controlled issue/PR data, a value with a few thousand nested parentheses can exhaust the JavaScript call stack and abort validation instead of returning a warning. Add a conservative length/nesting guard or use an iterative parser.

    if (token === "(") {
      consume();
      if (!parseOrExpression()) {

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

eng/lib/license.mjs:18

• This curated allowlist marks many valid SPDX identifiers (for example MIT-0, BlueOak-1.0.0, Python-2.0, and Artistic-2.0) as “not recognized”; the similarly curated exception set also omits valid exceptions such as 389-exception and FLTK-exception. That creates false warnings for standards-compliant license expressions. Use the complete SPDX license/exception data (preferably from a maintained SPDX data source or parser) rather than a small hand-maintained subset.

// Curated set of common SPDX license identifiers. Not exhaustive: unrecognized
// but well-formed identifiers produce a warning rather than an error.
const KNOWN_SPDX_IDS = new Set([

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Medium