diff --git a/.codespellrc b/.codespellrc index 6a20d6e8..43f1415e 100644 --- a/.codespellrc +++ b/.codespellrc @@ -64,7 +64,9 @@ # ACI - Azure Container Instances abbreviation -ignore-words-list = numer,wit,aks,edn,ser,ois,gir,rouge,categor,aline,ative,afterall,deques,dateA,dateB,TE,FillIn,alle,vai,LOD,InOut,pixelX,aNULL,Wee,Sherif,queston,extenions,Vertexes,nin,FO,CAF,Parth,ans,gud,Vally,vally,ACI +# soruce - intentional misspelling for tests + +ignore-words-list = numer,wit,aks,edn,ser,ois,gir,rouge,categor,aline,ative,afterall,deques,dateA,dateB,TE,FillIn,alle,vai,LOD,InOut,pixelX,aNULL,Wee,Sherif,queston,extenions,Vertexes,nin,FO,CAF,Parth,ans,gud,Vally,vally,ACI,soruce # Skip certain files and directories diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 075101c2..fc3d43ad 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -234,13 +234,16 @@ The repository's canonical validation rules live in `eng/external-plugin-validat For entries committed to `plugins/external.json`, the current marketplace validation requires: -- `name`, `description`, and `version` -- `author.name` +- `name`, `description`, and `version` (a valid [semantic version](https://semver.org), e.g. `1.2.3`) +- `author.name` (and, when present, a valid `author.url` and `author.email`) - `repository` as an HTTPS GitHub URL - `keywords` as lowercase hyphenated tags +- `license`, when provided, is recommended to be an [SPDX identifier or expression](https://spdx.org/licenses) (e.g. `MIT`, `Apache-2.0`, `MIT OR Apache-2.0`), but a non-SPDX or proprietary license is allowed and only produces a warning - `source.source: "github"` plus `source.repo` in `owner/repo` format - optional `source.path` values of `/` for repository root, or a repository-relative folder where the plugin structure starts (do not point to `plugin.json` directly) +Validation also emits non-fatal warnings to catch mistakes early: unknown/misspelled top-level, `author`, or `source` fields; `license` values that are not recognized SPDX identifiers (non-SPDX/proprietary licenses are permitted, not rejected); and marketplace entries whose `source` omits an immutable `ref`/`sha` locator. The same `license` validation is shared with local plugin `plugin.json` manifests so both are checked consistently. + The public-submission policy builds on those rules and also requires `license` plus at least one immutable source locator: `source.ref`, `source.sha`, or both. ##### Review workflow diff --git a/eng/external-plugin-validation.mjs b/eng/external-plugin-validation.mjs index 0a5ce2c1..07f14c25 100644 --- a/eng/external-plugin-validation.mjs +++ b/eng/external-plugin-validation.mjs @@ -1,6 +1,8 @@ import fs from "fs"; import path from "path"; import { ROOT_FOLDER } from "./constants.mjs"; +import { validateLicenseField } from "./lib/license.mjs"; +import { inlineCode } from "./lib/markdown.mjs"; export const EXTERNAL_PLUGINS_FILE = path.join(ROOT_FOLDER, "plugins", "external.json"); @@ -12,6 +14,7 @@ export const EXTERNAL_PLUGIN_POLICIES = Object.freeze({ requireKeywords: true, requireLicense: false, requireImmutableLocator: false, + warnMissingImmutableLocator: true, }), publicSubmission: Object.freeze({ allowedSourceTypes: ["github"], @@ -20,9 +23,34 @@ export const EXTERNAL_PLUGIN_POLICIES = Object.freeze({ requireKeywords: true, requireLicense: true, requireImmutableLocator: true, + warnMissingImmutableLocator: false, }), }); +// Allowed keys for typo detection. Kept intentionally permissive: unknown keys +// produce warnings (not errors) so the schema stays forward-compatible. +const ALLOWED_PLUGIN_KEYS = Object.freeze([ + "name", + "description", + "version", + "author", + "repository", + "homepage", + "license", + "keywords", + "tags", + "source", +]); + +const ALLOWED_AUTHOR_KEYS = Object.freeze(["name", "url", "email"]); + +const ALLOWED_SOURCE_KEYS = Object.freeze(["source", "repo", "path", "ref", "sha"]); + +// Semantic Versioning 2.0.0 (https://semver.org). Anchored: major.minor.patch +// with optional -prerelease and +build metadata. +const SEMVER_PATTERN = + /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/; + // NOTE: Keep in sync with PLUGIN_JSON_CANDIDATES in external-plugin-quality-gates.mjs const EXTERNAL_PLUGIN_ROOT_MANIFEST_PATHS = Object.freeze([ "plugin.json", @@ -97,6 +125,12 @@ function validateVersion(version, prefix, errors) { if (version.length > 100) { errors.push(`${prefix}: "version" must be 100 characters or fewer`); } + + if (!SEMVER_PATTERN.test(version)) { + errors.push( + `${prefix}: "version" must be a valid semantic version (e.g. "1.2.3" or "1.2.3-beta.1"); see https://semver.org` + ); + } } function validateKeywords(keywords, prefix, errors, warnings, required) { @@ -164,7 +198,7 @@ function validateHttpsUrl(value, fieldName, prefix, errors, options = {}) { } } -function validateAuthor(author, prefix, errors, required) { +function validateAuthor(author, prefix, errors, warnings, required) { if (author === undefined) { if (required) { errors.push(`${prefix}: "author" is required`); @@ -184,18 +218,43 @@ function validateAuthor(author, prefix, errors, required) { if (author.url !== undefined) { validateHttpsUrl(author.url, "author.url", prefix, errors); } + + if (author.email !== undefined) { + validateEmail(author.email, "author.email", prefix, errors); + } + + validateKnownFields(author, ALLOWED_AUTHOR_KEYS, "author", prefix, warnings); } -function validateLicense(license, prefix, errors, required) { - if (license === undefined) { - if (required) { - errors.push(`${prefix}: "license" is required`); - } +function validateEmail(value, fieldName, prefix, errors) { + if (!isNonEmptyString(value)) { + errors.push(`${prefix}: "${fieldName}" must be a non-empty string`); return; } - if (!isNonEmptyString(license)) { - errors.push(`${prefix}: "license" must be a non-empty string`); + // Pragmatic email check: single "@", non-empty local part, and a dotted domain. + if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) { + errors.push(`${prefix}: "${fieldName}" must be a valid email address`); + } +} + +function validateLicense(license, prefix, errors, warnings, required) { + const result = validateLicenseField(license, { prefix, required }); + errors.push(...result.errors); + warnings.push(...result.warnings); +} + +function validateKnownFields(value, allowedKeys, scope, prefix, warnings) { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return; + } + + const allowed = new Set(allowedKeys); + const label = scope ? `${scope}.` : ""; + for (const key of Object.keys(value)) { + if (!allowed.has(key)) { + warnings.push(`${prefix}: unknown ${scope || "top-level"} field ${inlineCode(`${label}${key}`)} (possible typo)`); + } } } @@ -289,7 +348,7 @@ function validateCommitSha(sha, prefix, errors) { } } -function validateGitHubSource(source, prefix, errors, requireImmutableLocator) { +function validateGitHubSource(source, prefix, errors, warnings, policy) { if (!source || typeof source !== "object" || Array.isArray(source)) { errors.push(`${prefix}: "source" must be an object`); return; @@ -317,8 +376,15 @@ function validateGitHubSource(source, prefix, errors, requireImmutableLocator) { validateCommitSha(source.sha, prefix, errors); } - if (requireImmutableLocator && source.ref === undefined && source.sha === undefined) { - errors.push(`${prefix}: one of "source.ref" or "source.sha" is required for public external plugin submissions`); + const missingLocator = source.ref === undefined && source.sha === undefined; + if (missingLocator) { + if (policy.requireImmutableLocator) { + errors.push(`${prefix}: one of "source.ref" or "source.sha" is required for public external plugin submissions`); + } else if (policy.warnMissingImmutableLocator) { + warnings.push( + `${prefix}: "source" has no "source.ref" or "source.sha"; an immutable tag ref or commit SHA is recommended for reproducible installs` + ); + } } } @@ -338,11 +404,12 @@ export function validateExternalPlugin(plugin, index, options = {}) { validatePluginName(plugin.name, prefix, errors); validateDescription(plugin.description, prefix, errors); validateVersion(plugin.version, prefix, errors); - validateAuthor(plugin.author, prefix, errors, policy.requireAuthor); + validateAuthor(plugin.author, prefix, errors, warnings, policy.requireAuthor); validateRepository(plugin.repository, prefix, errors, policy.requireRepository); validateHomepage(plugin.homepage, prefix, errors); - validateLicense(plugin.license, prefix, errors, policy.requireLicense); + validateLicense(plugin.license, prefix, errors, warnings, policy.requireLicense); validateKeywords(plugin.keywords ?? plugin.tags, prefix, errors, warnings, policy.requireKeywords); + validateKnownFields(plugin, ALLOWED_PLUGIN_KEYS, "", prefix, warnings); if (plugin.tags !== undefined && plugin.keywords === undefined) { warnings.push(`${prefix}: prefer "keywords" over legacy "tags"`); @@ -350,12 +417,15 @@ export function validateExternalPlugin(plugin, index, options = {}) { if (!plugin.source) { errors.push(`${prefix}: "source" is required`); - } else if (typeof plugin.source === "string") { + } else if (typeof plugin.source !== "object" || Array.isArray(plugin.source)) { errors.push(`${prefix}: "source" must be an object (local file paths are not allowed for external plugins)`); - } else if (!policy.allowedSourceTypes.includes(plugin.source.source)) { - errors.push(`${prefix}: "source.source" must be one of: ${policy.allowedSourceTypes.join(", ")}`); - } else if (plugin.source.source === "github") { - validateGitHubSource(plugin.source, prefix, errors, policy.requireImmutableLocator); + } else { + validateKnownFields(plugin.source, ALLOWED_SOURCE_KEYS, "source", prefix, warnings); + if (!policy.allowedSourceTypes.includes(plugin.source.source)) { + errors.push(`${prefix}: "source.source" must be one of: ${policy.allowedSourceTypes.join(", ")}`); + } else if (plugin.source.source === "github") { + validateGitHubSource(plugin.source, prefix, errors, warnings, policy); + } } return { errors, warnings }; diff --git a/eng/external-plugin-validation.test.mjs b/eng/external-plugin-validation.test.mjs new file mode 100644 index 00000000..8144c984 --- /dev/null +++ b/eng/external-plugin-validation.test.mjs @@ -0,0 +1,245 @@ +import assert from "node:assert/strict"; +import fs from "fs"; +import { test } from "node:test"; +import { + EXTERNAL_PLUGINS_FILE, + validateExternalPlugin, + validateExternalPlugins, +} from "./external-plugin-validation.mjs"; +import { validateLicenseField } from "./lib/license.mjs"; + +function basePlugin(overrides = {}) { + return { + name: "example-plugin", + description: "An example external plugin used for validation tests.", + version: "1.2.3", + author: { name: "Example Author", url: "https://github.com/example" }, + repository: "https://github.com/example/example-plugin", + license: "MIT", + keywords: ["example", "testing"], + source: { + source: "github", + repo: "example/example-plugin", + ref: "v1.2.3", + }, + ...overrides, + }; +} + +function hasError(result, needle) { + return result.errors.some((message) => message.includes(needle)); +} + +function hasWarning(result, needle) { + return result.warnings.some((message) => message.includes(needle)); +} + +function longestBacktickRun(value) { + return Math.max(0, ...Array.from(value.matchAll(/`+/g), (match) => match[0].length)); +} + +test("valid plugin has no errors under both policies", () => { + const marketplace = validateExternalPlugin(basePlugin(), 0, { policy: "marketplace" }); + assert.deepEqual(marketplace.errors, []); + + const publicSubmission = validateExternalPlugin( + basePlugin({ source: { source: "github", repo: "example/example-plugin", ref: "v1.2.3" } }), + 0, + { policy: "publicSubmission" } + ); + assert.deepEqual(publicSubmission.errors, []); +}); + +test("version must be semver", () => { + assert.deepEqual(validateExternalPlugin(basePlugin({ version: "1.2.3" }), 0).errors, []); + // Prerelease metadata is allowed (matches committed 1.0.1161-preview1). + assert.deepEqual(validateExternalPlugin(basePlugin({ version: "1.0.1161-preview1" }), 0).errors, []); + assert.deepEqual(validateExternalPlugin(basePlugin({ version: "1.2.3-beta.1+build.5" }), 0).errors, []); + + assert.ok(hasError(validateExternalPlugin(basePlugin({ version: "1.0" }), 0), "semantic version")); + assert.ok(hasError(validateExternalPlugin(basePlugin({ version: "v1.2.3" }), 0), "semantic version")); + assert.ok(hasError(validateExternalPlugin(basePlugin({ version: "latest" }), 0), "semantic version")); +}); + +test("license accepts SPDX ids and expressions", () => { + assert.deepEqual(validateExternalPlugin(basePlugin({ license: "Apache-2.0" }), 0).errors, []); + assert.deepEqual(validateExternalPlugin(basePlugin({ license: "MIT OR Apache-2.0" }), 0).errors, []); + assert.deepEqual(validateExternalPlugin(basePlugin({ license: "(MIT OR Apache-2.0)" }), 0).errors, []); + assert.deepEqual(validateExternalPlugin(basePlugin({ license: "LicenseRef-Custom" }), 0).errors, []); + + const recognized = validateExternalPlugin(basePlugin({ license: "MIT" }), 0); + assert.equal(recognized.warnings.filter((m) => m.includes("license")).length, 0); +}); + +test("non-SPDX license is a warning, never an error", () => { + for (const license of ["SSAL-1.0", "Proprietary", "UNLICENSED", "SEE LICENSE IN LICENSE.md", "MIT OR"]) { + const result = validateExternalPlugin(basePlugin({ license }), 0); + assert.deepEqual(result.errors, [], `expected no errors for license "${license}"`); + assert.ok( + hasWarning(result, "not a recognized SPDX identifier"), + `expected a warning for license "${license}"` + ); + } +}); + +test("empty/non-string license is an error; required license is enforced", () => { + assert.ok(hasError(validateExternalPlugin(basePlugin({ license: "" }), 0), "non-empty string")); + assert.ok(hasError(validateExternalPlugin(basePlugin({ license: 42 }), 0), "non-empty string")); + + // publicSubmission requires a license. + const noLicense = basePlugin(); + delete noLicense.license; + assert.ok( + hasError(validateExternalPlugin(noLicense, 0, { policy: "publicSubmission" }), '"license" is required') + ); +}); + +test("validateLicenseField is reusable for local plugin.json", () => { + assert.deepEqual(validateLicenseField("MIT"), { errors: [], warnings: [] }); + + const proprietary = validateLicenseField("Proprietary"); + assert.deepEqual(proprietary.errors, []); + assert.equal(proprietary.warnings.length, 1); + + // Optional by default: absent license produces nothing. + assert.deepEqual(validateLicenseField(undefined), { errors: [], warnings: [] }); + assert.ok(validateLicenseField(undefined, { required: true }).errors.length === 1); +}); + +test("license grammar validates refs, parentheses, WITH exceptions, and warning sanitization", () => { + for (const license of [ + "GPL-2.0-only WITH Classpath-exception-2.0", + "LicenseRef-Custom", + "DocumentRef-spdx-tool:LicenseRef-MyLicense", + "(MIT OR Apache-2.0)", + "MIT AND Apache-2.0", + ]) { + const result = validateLicenseField(license); + assert.deepEqual(result.errors, [], `expected no errors for recognized license "${license}"`); + assert.equal(result.warnings.length, 0, `expected no warnings for recognized license "${license}"`); + } + + for (const license of [ + "LicenseRef-", + "LicenseRef-@@", + "DocumentRef-foo", + "(MIT", + "MIT)", + "MIT OR (Apache-2.0))", + "(MIT OR Apache-2.0", + "MIT WITH Apache-2.0", + "GPL-2.0-only WITH MIT", + ]) { + const result = validateLicenseField(license); + assert.deepEqual(result.errors, [], `expected malformed license "${license}" to warn, not error`); + assert.equal(result.warnings.length, 1, `expected one warning for malformed license "${license}"`); + assert.ok(hasWarning(result, "not a recognized SPDX identifier")); + } + + const injected = validateLicenseField("MIT\n\n## Injected\n| a | b |"); + assert.deepEqual(injected.errors, []); + assert.equal(injected.warnings.length, 1); + assert.ok(!injected.warnings[0].includes("\n")); + assert.ok(hasWarning(injected, "not a recognized SPDX identifier")); + + const backslashed = validateLicenseField("MIT \\ Custom \\`code\\`"); + assert.deepEqual(backslashed.errors, []); + assert.equal(backslashed.warnings.length, 1); + assert.ok(backslashed.warnings[0].includes("`` MIT \\ Custom \\`code\\` ``")); +}); + +test("license warning wraps backticks and markdown link in an unbreakable code span", () => { + const license = "x` [pwn](https://evil.example)"; + const result = validateLicenseField(license); + + assert.deepEqual(result.errors, []); + assert.equal(result.warnings.length, 1); + + const warning = result.warnings[0]; + assert.ok(warning.includes("is not a recognized SPDX identifier")); + const [, wrappedValue] = warning.match(/"license" value (.+?) is not a recognized SPDX identifier/u); + const fence = wrappedValue.match(/^`+/u)[0]; + const content = wrappedValue.slice(fence.length, -fence.length); + + assert.ok(fence.length > longestBacktickRun(content)); + assert.ok(content.includes("[pwn](https://evil.example)")); +}); + +test("author.email is validated only when present", () => { + assert.deepEqual( + validateExternalPlugin(basePlugin({ author: { name: "Example", email: "dev@example.com" } }), 0).errors, + [] + ); + assert.ok( + hasError( + validateExternalPlugin(basePlugin({ author: { name: "Example", email: "not-an-email" } }), 0), + "author.email" + ) + ); + // Absent email must not raise an error. + assert.deepEqual(validateExternalPlugin(basePlugin({ author: { name: "Example" } }), 0).errors, []); +}); + +test("unknown fields produce warnings, not errors", () => { + const topLevel = validateExternalPlugin(basePlugin({ licence: "MIT" }), 0); + assert.deepEqual(topLevel.errors, []); + assert.ok(hasWarning(topLevel, "unknown top-level field `licence`")); + + const authorTypo = validateExternalPlugin( + basePlugin({ author: { name: "Example", emial: "dev@example.com" } }), + 0 + ); + assert.ok(hasWarning(authorTypo, "unknown author field `author.emial`")); + + const sourceTypo = validateExternalPlugin( + basePlugin({ source: { source: "github", repo: "example/example-plugin", ref: "v1.2.3", branch: "main" } }), + 0 + ); + assert.ok(hasWarning(sourceTypo, "unknown source field `source.branch`")); + + // A typo in the source discriminator itself is still surfaced as an unknown + // source field (plus the discriminator error), because the unknown-field check + // now runs for every source object rather than only recognized github sources. + const sourceDiscriminatorTypo = validateExternalPlugin( + basePlugin({ source: { soruce: "github", repo: "example/example-plugin", ref: "v1.2.3" } }), + 0 + ); + assert.ok(hasWarning(sourceDiscriminatorTypo, "unknown source field `source.soruce`")); + assert.ok(hasError(sourceDiscriminatorTypo, '"source.source" must be one of')); + + // Supported fields never warn. + const clean = validateExternalPlugin(basePlugin({ homepage: "https://github.com/example/example-plugin" }), 0); + assert.equal( + clean.warnings.filter((message) => message.includes("unknown")).length, + 0 + ); +}); + +test("unknown field warning collapses malicious key markdown into one inline code span", () => { + const maliciousKey = "evil\n- injected [x](https://evil.example)"; + const result = validateExternalPlugin(basePlugin({ [maliciousKey]: true }), 0); + + assert.deepEqual(result.errors, []); + const warning = result.warnings.find((message) => message.includes("unknown top-level field")); + assert.ok(warning); + assert.ok(!warning.includes("\n")); + assert.ok(warning.includes("unknown top-level field `evil - injected [x](https://evil.example)`")); + assert.ok(warning.includes("(possible typo)")); +}); + +test("immutable locator: marketplace warns, publicSubmission errors", () => { + const noLocator = basePlugin({ source: { source: "github", repo: "example/example-plugin" } }); + + const marketplace = validateExternalPlugin(noLocator, 0, { policy: "marketplace" }); + assert.deepEqual(marketplace.errors, []); + assert.ok(hasWarning(marketplace, "immutable tag ref or commit SHA is recommended")); + + const publicSubmission = validateExternalPlugin(noLocator, 0, { policy: "publicSubmission" }); + assert.ok(hasError(publicSubmission, 'one of "source.ref" or "source.sha" is required')); +}); + +test("committed external.json passes marketplace policy with zero errors", () => { + const plugins = JSON.parse(fs.readFileSync(EXTERNAL_PLUGINS_FILE, "utf8")); + const { errors } = validateExternalPlugins(plugins, { policy: "marketplace" }); + assert.deepEqual(errors, [], `Committed external.json produced errors:\n${errors.join("\n")}`); +}); diff --git a/eng/lib/license.mjs b/eng/lib/license.mjs new file mode 100644 index 00000000..4fb9e312 --- /dev/null +++ b/eng/lib/license.mjs @@ -0,0 +1,261 @@ +// Shared license validation used by both the external plugin catalog validator +// (eng/external-plugin-validation.mjs) and the local plugin.json validator +// (eng/validate-plugins.mjs). Keeping it here avoids one validator importing from +// the other and gives license rules a single, dedicated home. +// +// The agent-plugins-spec schema (https://github.com/agentplugins/agent-plugins-spec) +// does not enforce SPDX, and plugins may legitimately use proprietary or non-OSS +// licenses. A non-SPDX license is therefore a warning, never an error. + +import { inlineCode } from "./markdown.mjs"; + +// A single SPDX license identifier token (e.g. "MIT", "Apache-2.0", "LicenseRef-Foo"). +const SPDX_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9.-]*\+?$/; +const SPDX_IDSTRING_PATTERN = /^[A-Za-z0-9.-]+$/; + +// 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([ + "0BSD", + "AGPL-3.0", + "AGPL-3.0-only", + "AGPL-3.0-or-later", + "Apache-2.0", + "BSD-2-Clause", + "BSD-3-Clause", + "BSL-1.0", + "CC-BY-4.0", + "CC-BY-SA-4.0", + "CC0-1.0", + "EPL-2.0", + "GPL-2.0", + "GPL-2.0-only", + "GPL-2.0-or-later", + "GPL-3.0", + "GPL-3.0-only", + "GPL-3.0-or-later", + "ISC", + "LGPL-2.1", + "LGPL-2.1-only", + "LGPL-2.1-or-later", + "LGPL-3.0", + "LGPL-3.0-only", + "LGPL-3.0-or-later", + "MIT", + "MPL-2.0", + "Unlicense", + "WTFPL", + "Zlib", +]); + +const KNOWN_SPDX_EXCEPTIONS = new Set([ + "Classpath-exception-2.0", + "GCC-exception-3.1", + "LLVM-exception", + "Autoconf-exception-3.0", + "Bison-exception-2.2", + "Font-exception-2.0", + "GPL-3.0-linking-exception", + "Linux-syscall-note", + "OpenSSL-exception", + "Qt-GPL-exception-1.0", +]); + +function isNonEmptyString(value) { + return typeof value === "string" && value.trim().length > 0; +} + +function isSpdxIdString(value) { + return SPDX_IDSTRING_PATTERN.test(value); +} + +function isRecognizedLicenseRef(token) { + if (token.startsWith("LicenseRef-")) { + return isSpdxIdString(token.slice("LicenseRef-".length)); + } + + if (!token.startsWith("DocumentRef-")) { + return false; + } + + const separatorIndex = token.indexOf(":LicenseRef-"); + if (separatorIndex === -1) { + return false; + } + + const documentRef = token.slice("DocumentRef-".length, separatorIndex); + const licenseRef = token.slice(separatorIndex + ":LicenseRef-".length); + return isSpdxIdString(documentRef) && isSpdxIdString(licenseRef); +} + +function isRecognizedSpdxIdToken(token) { + if (isRecognizedLicenseRef(token)) { + return true; + } + + if (!SPDX_ID_PATTERN.test(token)) { + return false; + } + + const normalized = token.endsWith("+") ? token.slice(0, -1) : token; + return KNOWN_SPDX_IDS.has(normalized); +} + +function isRecognizedSpdxExceptionToken(token) { + return isSpdxIdString(token) && KNOWN_SPDX_EXCEPTIONS.has(token); +} + +function tokenizeSpdxExpression(license) { + return license + .replace(/([()])/g, " $1 ") + .trim() + .split(/\s+/) + .filter((token) => token.length > 0); +} + +// Returns true only for a well-formed SPDX license expression composed of +// recognized identifiers joined by SPDX operators (optionally parenthesized). +// Anything else (proprietary strings, free text, unrecognized ids) returns false so +// the caller can warn without rejecting it — the plugin spec does not enforce SPDX. +export function isRecognizedSpdxExpression(license) { + if (typeof license !== "string") { + return false; + } + + const tokens = tokenizeSpdxExpression(license); + + if (tokens.length === 0) { + return false; + } + + let position = 0; + + function peek() { + return tokens[position]; + } + + function consume() { + return tokens[position++]; + } + + function parsePrimary() { + const token = peek(); + + if (token === "(") { + consume(); + if (!parseOrExpression()) { + return false; + } + if (peek() !== ")") { + return false; + } + consume(); + return true; + } + + if (!token || token === ")" || ["AND", "OR", "WITH"].includes(token.toUpperCase())) { + return false; + } + + consume(); + return isRecognizedSpdxIdToken(token); + } + + function parseSimpleExpression() { + const token = peek(); + if (!token || token === ")" || ["AND", "OR", "WITH"].includes(token.toUpperCase())) { + return false; + } + + consume(); + return isRecognizedSpdxIdToken(token); + } + + function parseWithExpression() { + if (peek() === "(") { + return parsePrimary() && peek()?.toUpperCase() !== "WITH"; + } + + if (!parseSimpleExpression()) { + return false; + } + + if (peek()?.toUpperCase() !== "WITH") { + return true; + } + + consume(); + const exceptionToken = peek(); + if (!exceptionToken || exceptionToken === ")" || ["AND", "OR", "WITH"].includes(exceptionToken.toUpperCase())) { + return false; + } + consume(); + return isRecognizedSpdxExceptionToken(exceptionToken); + } + + function parseAndExpression() { + if (!parseWithExpression()) { + return false; + } + + while (peek()?.toUpperCase() === "AND") { + consume(); + if (!parseWithExpression()) { + return false; + } + } + + return true; + } + + function parseOrExpression() { + if (!parseAndExpression()) { + return false; + } + + while (peek()?.toUpperCase() === "OR") { + consume(); + if (!parseAndExpression()) { + return false; + } + } + + return true; + } + + return parseOrExpression() && position === tokens.length; +} + +// Canonical license validation. A non-SPDX license is a warning (never an error) +// so authors may use proprietary or non-OSS licenses. Returns collected +// errors/warnings so each caller can integrate them into its own reporting. +// +// Options: +// required — when true, a missing license is an error (default false). +// prefix — optional message prefix (e.g. "external.json[3]") for context. +export function validateLicenseField(license, options = {}) { + const { required = false } = options; + const prefix = options.prefix ? `${options.prefix}: ` : ""; + const errors = []; + const warnings = []; + + if (license === undefined) { + if (required) { + errors.push(`${prefix}"license" is required`); + } + return { errors, warnings }; + } + + if (!isNonEmptyString(license)) { + errors.push(`${prefix}"license" must be a non-empty string`); + return { errors, warnings }; + } + + if (!isRecognizedSpdxExpression(license)) { + warnings.push( + `${prefix}"license" value ${inlineCode(license)} is not a recognized SPDX identifier; prefer a standard SPDX id (https://spdx.org/licenses), though non-SPDX or proprietary licenses are allowed` + ); + } + + return { errors, warnings }; +} diff --git a/eng/lib/markdown.mjs b/eng/lib/markdown.mjs new file mode 100644 index 00000000..86189049 --- /dev/null +++ b/eng/lib/markdown.mjs @@ -0,0 +1,11 @@ +export function inlineCode(value) { + const collapsed = String(value).replace(/\s+/g, " ").trim(); + const content = collapsed.length > 80 ? `${collapsed.slice(0, 77)}...` : collapsed; + const longestBacktickRun = Math.max(0, ...Array.from(content.matchAll(/`+/g), (match) => match[0].length)); + const fence = "`".repeat(longestBacktickRun + 1); + const pad = content.length === 0 || content.startsWith("`") || content.endsWith("`") ? " " : ""; + + // These values are rendered verbatim into Markdown bot comments; using a fence + // longer than any backtick run in the content prevents closing the code span. + return `${fence}${pad}${content}${pad}${fence}`; +} diff --git a/eng/lib/markdown.test.mjs b/eng/lib/markdown.test.mjs new file mode 100644 index 00000000..4b1edb5f --- /dev/null +++ b/eng/lib/markdown.test.mjs @@ -0,0 +1,25 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { inlineCode } from "./markdown.mjs"; + +test("inlineCode wraps plain values in a single-backtick code span", () => { + assert.equal(inlineCode("plain value"), "`plain value`"); +}); + +test("inlineCode uses a fence longer than the longest backtick run", () => { + assert.equal(inlineCode("a `` b"), "```a `` b```"); +}); + +test("inlineCode pads values starting or ending with a backtick", () => { + assert.equal(inlineCode("`leading"), "`` `leading ``"); + assert.equal(inlineCode("trailing`"), "`` trailing` ``"); +}); + +test("inlineCode collapses whitespace and pads empty values", () => { + assert.equal(inlineCode("a\n\t b"), "`a b`"); + assert.equal(inlineCode(" \n\t "), "` `"); +}); + +test("inlineCode truncates values to 80 characters", () => { + assert.equal(inlineCode("x".repeat(81)), `\`${"x".repeat(77)}...\``); +}); diff --git a/eng/validate-plugins.mjs b/eng/validate-plugins.mjs index 7161df06..e8cbf1a6 100755 --- a/eng/validate-plugins.mjs +++ b/eng/validate-plugins.mjs @@ -4,6 +4,7 @@ import fs from "fs"; import path from "path"; import { ROOT_FOLDER } from "./constants.mjs"; import { readExternalPlugins } from "./external-plugin-validation.mjs"; +import { validateLicenseField } from "./lib/license.mjs"; const PLUGINS_DIR = path.join(ROOT_FOLDER, "plugins"); const EXTENSIONS_DIR = path.join(ROOT_FOLDER, "extensions"); @@ -239,6 +240,12 @@ function validatePlugin(folderName) { const keywordsError = validateKeywords(plugin.keywords ?? plugin.tags); if (keywordsError) errors.push(keywordsError); + // Rule 5b: license (shared with external plugins). Non-SPDX is a warning, not an error. + const warnings = []; + const licenseResult = validateLicenseField(plugin.license, { required: false }); + errors.push(...licenseResult.errors); + warnings.push(...licenseResult.warnings); + // Rule 6: agents, commands, skills paths const specErrors = validateSpecPaths(plugin); errors.push(...specErrors); @@ -246,7 +253,7 @@ function validatePlugin(folderName) { const extensionRefErrors = validateCuratedPluginExtensionRefs(plugin); errors.push(...extensionRefErrors); - return { errors, plugin: parsedPlugin }; + return { errors, warnings, plugin: parsedPlugin }; } function validateExtensionScreenshotPath(extensionDir, pathValue, fieldName, errors) { @@ -342,7 +349,7 @@ function validatePlugins() { for (const dir of pluginDirs) { console.log(`Validating ${dir}...`); - const { errors, plugin } = validatePlugin(dir); + const { errors, warnings, plugin } = validatePlugin(dir); if (errors.length > 0) { console.error(`❌ ${dir}:`); @@ -352,6 +359,10 @@ function validatePlugins() { console.log(`✅ ${dir} is valid`); } + if (warnings?.length > 0) { + warnings.forEach((w) => console.warn(`⚠️ ${dir}: ${w}`)); + } + if (plugin?.name) { if (seenNames.has(plugin.name)) { console.error(`❌ Duplicate plugin name "${plugin.name}"`);