Files
Aaron Powell be7a1cf734 Harden external plugin validation: semver, SPDX, email, and unknown-field checks (#2445)
* Harden external plugin validation: semver, SPDX, email, unknown fields

Extend the canonical external-plugin validator with Open-Plugins-aligned
rules, reusing the shared validation functions rather than duplicating checks:

- version: enforce Semantic Versioning (allows prerelease/build metadata)
- license: validate SPDX identifiers/expressions; warn (not error) on
  well-formed-but-unrecognized ids so existing entries like SSAL-1.0 pass
- author.email: validate format when present
- unknown-field detection: warn on typo'd top-level/author/source keys
- immutable locator: marketplace warns when source lacks ref/sha;
  publicSubmission keeps the existing hard error

Add eng/external-plugin-validation.test.mjs (node:test) covering each rule
plus a regression that committed external.json passes marketplace policy
with zero errors. Update CONTRIBUTING.md accordingly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728

* Make license non-SPDX a warning and share license check with local plugins

The agent-plugins-spec schema does not enforce SPDX, and plugins may use
proprietary/non-OSS licenses. Relax license validation so any non-empty
license string that isn't a recognized SPDX identifier/expression produces a
warning rather than an error.

Extract the license check into a reusable validateLicenseField() and apply it
to both external plugins and local plugin.json manifests via
eng/validate-plugins.mjs, so licenses are validated consistently in one place.

Update tests and CONTRIBUTING.md accordingly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728

* Extract shared license validation into eng/lib/license.mjs

Move the SPDX constants and license validation (validateLicenseField,
isRecognizedSpdxExpression) out of external-plugin-validation.mjs into a
dedicated eng/lib/license.mjs module. Both the external plugin catalog
validator and the local plugin.json validator now import license logic
from this neutral shared module instead of one validator importing from
the other. Behavior is unchanged; tests import from the new home.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728

* Fix SPDX license parsing correctness and warning-message injection

Rewrite isRecognizedSpdxExpression as a recursive-descent SPDX license
expression parser to address PR review feedback:

- Validate LicenseRef-/DocumentRef- grammar (non-empty idstring; DocumentRef
  requires the ":LicenseRef-<id>" suffix) instead of accepting any token with
  that prefix.
- Parse parentheses with balance and placement checks rather than stripping
  them, so malformed groupings like "(MIT", "MIT)", and "MIT OR (Apache-2.0))"
  are no longer silently treated as valid.
- Treat WITH as a distinct operator whose right operand must be a known SPDX
  license exception, so "MIT WITH Apache-2.0" and "GPL-2.0-only WITH MIT" are
  rejected while "GPL-2.0-only WITH Classpath-exception-2.0" is recognized.
- Sanitize the untrusted license value in the non-SPDX warning (collapse
  whitespace, truncate, escape backticks, wrap as inline code) since the
  warning is rendered verbatim into a Markdown bot comment during intake.

Malformed SPDX stays a warning (never an error) so proprietary and non-OSS
license strings remain allowed. Adds targeted grammar and sanitization tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728

* Complete backslash escaping and run source unknown-field check for all sources

Address two PR review findings:

- eng/lib/license.mjs: sanitizeForMessage now escapes backslashes before
  backticks so the inline-code warning value is fully escaped, resolving a
  CodeQL "incomplete string escaping" alert. A raw backslash in a license
  value is now doubled rather than left to combine with a following escaped
  backtick.
- eng/external-plugin-validation.mjs: move the source unknown-field check out
  of validateGitHubSource into the outer source-object branch so it runs for
  every object source and then dispatch by source.source. A typo in the source
  discriminator (e.g. "soruce") or an unsupported source type now still
  surfaces the unknown-field warning instead of being silently skipped.

Adds regression tests for both behaviors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728

* Harden Markdown safety of validation warnings via inlineCode helper

External-plugin validation warnings are rendered verbatim as Markdown
list items in the intake bot comment, so untrusted values interpolated
into them are an injection vector.

Add eng/lib/markdown.mjs exporting inlineCode(), which wraps a value in
a code span fenced with more backticks than any run in the content (and
pads leading/trailing backticks). This cannot be broken out of, unlike
the previous backslash escaping, which does not work inside code spans.

- lib/license.mjs: replace sanitizeForMessage with inlineCode for the
  non-SPDX license warning.
- external-plugin-validation.mjs: wrap unknown-field keys (attacker
  controlled JSON object keys) with inlineCode so newlines/Markdown in a
  key can no longer inject a new list item.
- Add inlineCode unit tests and injection-neutralization tests for both
  the license value and unknown-field key paths.

Addresses PR review comments 3679379606 and 3679434134.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728

* Fixing codespell

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eaa5eed6-5b65-4b28-9904-24f380d26728
2026-07-30 14:35:49 +10:00

76 lines
3.1 KiB
Plaintext

[codespell]
# Ignore intentional misspellings used as examples and technical terms
# numer - intentional example typo in add-educational-comments.prompt.md
# wit - proper technical term/name (sardonic wit, Gilfoyle character trait)
# aks - Azure Kubernetes Service (AKS) abbreviation
# edn - Extensible Data Notation (Clojure data format)
# ser - serialization abbreviation
# ois - ObjectInputStream abbreviation in Java
# gir - valid abbreviation/technical term
# rouge - Rouge is a syntax highlighter (not "rogue")
# categor - TypeScript template literal in website/src/scripts/pages/skills.ts:70 (categor${...length > 1 ? "ies" : "y"})
# aline - proper name (Aline Ávila, contributor)
# ative - part of "Declarative Agents" in TypeSpec M365 Copilot documentation (collections/typespec-m365-copilot.collection.md)
# dateA, dateB - variable names used in sorting comparison functions
# TE - HTTP transfer coding header
# alle - Finnish word meaning "under/below" (not "all" or "alley")
# vai - Finnish word meaning "or"
# FillIn - pdftk-server skill reference file available permission
# LOD - Level of Detail
# InOut - template property in skills/game-engine/assets/2d-platform-game.md
# pixelX - template variable in skill/game-engine/assets/simple-2d-engine.md
# aNULL - HTTPS configuration cipher string
# Wee, Sherif - proper name (Wee, Sherif, contributor names should not be flagged as typos)
# queston - intentional misspelling example in skills/arize-dataset/SKILL.md demonstrating typo detection in field names
# extenions - intentional misspelled key name used in plugin validators to detect invalid manifests
# nin - MongoDB $nin operator in security instructions NoSQL injection detection regex
# Vertexes - FreeCAD shape sub-elements used as property of obj.Shape
# FO - tasklist option /FO to format running task output
# CAF - Microsoft Cloud Adoption Framework acronym
# ans - bash and powershell variable short for answer
# GUD - "Guideline" identifier prefix in the create-implementation-plan skill spec (alongside REQ, SEC, CON, PAT, etc.)
# Vally/vally - Name of product
# ACI - Azure Container Instances abbreviation
# 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
# *.tm7 - MTM DataContract exports; embedded base64 icon blobs trigger false positives
skip = .git,node_modules,package-lock.json,*.lock,website/build,website/.docusaurus,.all-contributorrc,./skills/geofeed-tuner/assets/*.json,./skills/geofeed-tuner/references/*.txt,./plugins/fastah-ip-geo-tools/skills/geofeed-tuner/assets/*.json,./plugins/fastah-ip-geo-tools/skills/geofeed-tuner/references/*.txt,./extensions/arcade-canvas/game/phaser.min.js,./extensions/pr-artifact-explorer/assets/asciinema-player.min.js,./extensions/pr-artifact-explorer/assets/asciinema-player-worker.min.js,./extensions/pr-artifact-explorer/assets/primer-*.css,./extensions/flight-map-canvas/game/vendor/three.min.js,*.tm7