From 1aaa405194f4bf53735d131de32c9aaa754504d1 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 16 Aug 2026 14:51:21 +0300 Subject: [PATCH 01/24] docs: design public directory submission preflight --- docs/README.md | 1 + .../public-directory-submission-preflight.md | 255 ++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 docs/architecture/public-directory-submission-preflight.md diff --git a/docs/README.md b/docs/README.md index 4242f07..5c07f8e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -14,6 +14,7 @@ This directory contains public documentation for users, contributors, and securi - [Remote MCP Transport Reliability](architecture/remote-mcp-transport-reliability.md) - [MCP Registry Readiness](architecture/mcp-registry-readiness.md) - [MCP Registry Publication Preflight](architecture/mcp-registry-publication-preflight.md) +- [Public Directory Submission Preflight](architecture/public-directory-submission-preflight.md) - [Real-World Corpus Quality Metrics](architecture/real-world-corpus-quality-metrics.md) - [Corpus Metrics Regression Diff](architecture/corpus-metrics-regression-diff.md) diff --git a/docs/architecture/public-directory-submission-preflight.md b/docs/architecture/public-directory-submission-preflight.md new file mode 100644 index 0000000..e54072b --- /dev/null +++ b/docs/architecture/public-directory-submission-preflight.md @@ -0,0 +1,255 @@ +# Public Directory Submission Preflight + +## Purpose + +Codex Plugin Doctor will provide an offline preflight for packages intended for the OpenAI public directory. The preflight helps authors find deterministic packaging and listing problems before they enter the submission portal. + +The command does not submit a package and does not claim that OpenAI will accept it. Portal review, developer verification, policy review, OAuth review, and other checks that require external state remain explicit manual work. + +The ruleset is based on the OpenAI plugin packaging, app review, and submission error documentation available on 2026-08-15: + +- +- +- + +## Command Surface + +```bash +codex-plugin-doctor doctor submission +codex-plugin-doctor doctor submission --json +codex-plugin-doctor doctor submission --markdown +codex-plugin-doctor doctor submission --require-ready +``` + +The command is additive. Existing `check`, runtime, audit, registry, and release behavior remains unchanged. + +`doctor submission` is always static and non-executing: + +- no network requests +- no MCP server startup +- no child-process execution +- no portal API access +- no domain verification +- no OAuth credential handling + +The command classifies the package deterministically from its declarations. A package that declares an MCP server or app connection is `mcp-backed`; every other package is `skills-only`. + +The two reported targets are: + +- `skills-only`: the package contains only public-directory-compatible skills and metadata. +- `mcp-backed`: the package declares an MCP server or an app connection. + +## Result Model + +The machine-readable contract is `doctor.submission.json` with schema version `1.0.0`. + +```json +{ + "schemaVersion": "1.0.0", + "rulesetVersion": "openai-directory-2026-08-15", + "targetType": "skills-only", + "status": "pass", + "readiness": "manual_review_required", + "summary": { + "passed": 12, + "warnings": 0, + "blockers": 0, + "manualChecks": 3 + }, + "checks": [], + "findings": [], + "manualChecklist": [] +} +``` + +`status` reports only deterministic automatic checks: + +- `pass`: no automatic blocker was found. +- `fail`: at least one automatic blocker was found. + +`readiness` prevents an offline result from being confused with submission approval: + +- `blocked`: an automatic blocker exists. +- `manual_review_required`: automatic checks passed, but portal or reviewer checks remain. + +The command never emits an automatic `ready` or `accepted` state. + +### Exit Codes + +- `0`: automatic checks passed, including when manual review remains. +- `1`: automatic blockers exist and `--require-ready` was supplied. +- `2`: command usage is invalid. + +Without `--require-ready`, a completed preflight returns `0` even when automatic blockers are reported. This permits advisory adoption. Output still reports `status: "fail"` and `readiness: "blocked"`. + +## Automatic Rules + +New stable finding identifiers use the `plugin.submission.*` namespace. Existing validation rule identifiers do not change. Portal error codes, when known, are stored separately as `portalCode` and never replace the Doctor identifier. + +### Listing And Identity + +The preflight validates: + +- package name and semantic version +- display name at most 30 characters +- short description at most 30 characters +- long description at most 4,000 characters +- developer name at most 80 characters +- supported category value +- no more than 20 capability entries +- each capability at most 120 characters and one line +- no more than 3 unique starter prompts +- each starter prompt at most 128 characters and one line +- no starter prompt containing an `@mention` +- no control or invisible characters in listing text + +For `mcp-backed` targets, the following fields are required: + +- website URL +- support URL +- privacy policy URL +- terms of service URL + +Each URL must: + +- use HTTPS +- contain no embedded credentials +- be at most 1,024 characters + +Unknown listing fields produce a warning when they can be ignored safely. Malformed expected fields produce deterministic findings instead of exceptions. + +### Assets And Component Integrity + +The preflight requires `logo` and `composerIcon` assets. Each asset must: + +- resolve to a regular file contained within the package root after canonical path resolution +- use PNG, JPEG, WebP, or SVG content +- be no larger than 5 MiB +- be square +- have dimensions from 48 through 4,096 pixels +- match its declared file extension +- decode safely within fixed resource limits + +Raster metadata and SVG dimensions are parsed without executing external tools. SVG handling uses safe XML parsing and rejects external entities or remote references needed for validation. + +When `.app.json` is referenced, the preflight validates the documented shared-package connection-mapping shape. A structurally valid app manifest is not treated as proof of public-directory eligibility. + +A `skills-only` target that declares screenshot components receives a submission blocker. A package that declares MCP or app components is classified as `mcp-backed` instead. + +### Skill Metadata + +For each skill, `skills/*/agents/openai.yaml` is optional. When present, it is parsed as data with a bounded safe YAML schema. The preflight validates supported fields including: + +- `interface.display_name` +- `interface.short_description` +- `interface.icons` +- `interface.brand_color` +- `interface.default_prompt` +- `policy.products` +- `policy.allow_implicit_invocation` +- `dependencies.tools` + +The validator also checks: + +- all referenced icon paths remain within the package root +- skill identities are unique +- combined `plugin:skill` identities remain within the documented length limit +- malformed or mixed-shape metadata fails deterministically + +YAML aliases, custom tags, executable types, and unbounded structures are rejected. Metadata content is never executed. + +## Manual Review Checklist + +The report separates portal-only or judgment-based work from automatic checks. Depending on target type, the manual checklist includes: + +- developer or business identity verification +- required attestations +- skill safety review +- MCP demonstration video +- exactly 5 positive and 3 negative MCP tests +- release notes +- production domain verification +- current tool security scan +- tool annotation accuracy and justification +- OAuth reviewer credentials + +Manual items have a state such as `required` or `not_applicable`; they never receive an automatic `passed` state merely because local files exist. + +## Privacy And Evidence + +Findings expose only the minimum evidence needed to locate a problem: + +- field names +- counts and limits +- package-relative paths +- normalized check identifiers + +Reports must not include full prompts, descriptions, credentials, file contents, absolute package roots, or decoded asset data. Text, JSON, Markdown, GitHub Action artifacts, and future report consumers share the same redacted result model. + +## Ruleset Governance + +The initial embedded ruleset is `openai-directory-2026-08-15`. + +Each ruleset records: + +- a stable version identifier +- source URLs +- the date the sources were reviewed +- deterministic automatic constraints +- the manual checklist definitions + +The command does not fetch rule updates at runtime. A ruleset update is a reviewed source change with tests and changelog coverage. This keeps identical package inputs reproducible in local development and CI. + +## Architecture + +The implementation will use four focused core modules: + +- `submission-ruleset.ts`: immutable rules and manual-check definitions +- `submission-preflight.ts`: package classification and result orchestration +- `submission-assets.ts`: bounded asset inspection +- `submission-skill-metadata.ts`: safe `agents/openai.yaml` parsing and validation + +Text, JSON, and Markdown renderers consume the same domain result. Public TypeScript exports and output contracts are additive. + +Small pure-JavaScript dependencies may be introduced only when necessary for safe YAML, raster metadata, or SVG parsing. Native modules, heavy dependency trees, postinstall requirements, and install-time code execution are not acceptable for this feature. + +## GitHub Action + +Submission preflight is opt-in: + +```yaml +- uses: Esquetta/CodexPluginDoctor@v1.59.0 + with: + submission: "true" + require-submission-ready: "true" +``` + +`submission` runs the offline preflight and publishes its reports with the existing Action artifacts. `require-submission-ready` applies the strict automatic-blocker exit gate. Existing Action defaults remain unchanged. + +## Verification Contract + +Implementation is complete only when tests cover: + +- valid and invalid `skills-only` and `mcp-backed` fixtures +- listing limits, duplicate prompts, and Unicode edge cases +- `.app.json` shape and duplicate connection identities +- safe YAML shapes and path traversal in `agents/openai.yaml` +- asset magic bytes, extension mismatch, dimensions, square ratio, and size limit +- URL requirements +- manual checks never becoming automatic passes +- text, JSON, Markdown, exit-code, and output-contract behavior +- absence of network and child-process activity +- package-relative evidence on Windows and POSIX-style inputs +- opt-in GitHub Action behavior with unchanged defaults + +Release verification includes targeted tests, the full test suite, TypeScript build, corpus checks, dependency audit, package-content inspection, source security self-scan, and the existing release check. + +## Out Of Scope + +- portal authentication or submission +- domain verification actions +- OAuth flow testing or credential storage +- MCP execution or live tool calls +- runtime verification of tool annotations +- judging demonstration or test quality +- predicting or claiming OpenAI directory acceptance From 622e846675104fab6ce0b73a1dcfd4161d06969e Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 16 Aug 2026 15:11:16 +0300 Subject: [PATCH 02/24] docs: clarify app manifest preflight boundary --- docs/architecture/public-directory-submission-preflight.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/public-directory-submission-preflight.md b/docs/architecture/public-directory-submission-preflight.md index e54072b..9fcc5b1 100644 --- a/docs/architecture/public-directory-submission-preflight.md +++ b/docs/architecture/public-directory-submission-preflight.md @@ -132,7 +132,7 @@ The preflight requires `logo` and `composerIcon` assets. Each asset must: Raster metadata and SVG dimensions are parsed without executing external tools. SVG handling uses safe XML parsing and rejects external entities or remote references needed for validation. -When `.app.json` is referenced, the preflight validates the documented shared-package connection-mapping shape. A structurally valid app manifest is not treated as proof of public-directory eligibility. +When `.app.json` is referenced, the preflight validates only the publicly documented package boundary: the declaration points to the root `.app.json`, resolves to a contained regular file, and contains parseable JSON. The public documentation does not define the internal registered-connection mapping schema, so the preflight does not invent or enforce one. A valid local app file is not treated as proof of public-directory eligibility. A `skills-only` target that declares screenshot components receives a submission blocker. A package that declares MCP or app components is classified as `mcp-backed` instead. @@ -232,7 +232,7 @@ Implementation is complete only when tests cover: - valid and invalid `skills-only` and `mcp-backed` fixtures - listing limits, duplicate prompts, and Unicode edge cases -- `.app.json` shape and duplicate connection identities +- `.app.json` root path, regular-file, containment, and JSON parsing rules - safe YAML shapes and path traversal in `agents/openai.yaml` - asset magic bytes, extension mismatch, dimensions, square ratio, and size limit - URL requirements From 2884212a9b140b04ee962937f31055b57cb47a4b Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 16 Aug 2026 15:24:31 +0300 Subject: [PATCH 03/24] chore: update vulnerable nanoid transitive --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5322d27..01421e6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1218,9 +1218,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.16", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", - "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { From 59844defb9adfbdf947f2df88afb82caab799e6e Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 10:12:46 +0300 Subject: [PATCH 04/24] feat: add submission listing preflight --- src/core/submission-preflight.ts | 313 ++++++++++++++++++++++++++++ src/core/submission-ruleset.ts | 39 ++++ tests/submission-preflight.test.ts | 323 +++++++++++++++++++++++++++++ 3 files changed, 675 insertions(+) create mode 100644 src/core/submission-preflight.ts create mode 100644 src/core/submission-ruleset.ts create mode 100644 tests/submission-preflight.test.ts diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts new file mode 100644 index 0000000..f47d02f --- /dev/null +++ b/src/core/submission-preflight.ts @@ -0,0 +1,313 @@ +import { readFile, realpath, stat } from "node:fs/promises"; +import path from "node:path"; + +import { discoverPackage } from "./discover-package.js"; +import { submissionManualChecks, submissionRuleset } from "./submission-ruleset.js"; + +export interface SubmissionFinding { + id: `plugin.submission.${string}`; + severity: "warn" | "fail"; + message: string; + portalCode?: string; + evidence?: Record; +} + +export interface SubmissionCheck { + id: "listing" | "components" | "assets" | "skills"; + status: "pass" | "warn" | "fail"; + findingIds: string[]; +} + +export interface SubmissionManualCheck { + id: string; + label: string; + state: "required" | "not_applicable"; +} + +export interface SubmissionPreflightReport { + schemaVersion: "1.0.0"; + rulesetVersion: "openai-directory-2026-08-15"; + targetType: "skills-only" | "mcp-backed"; + status: "pass" | "fail"; + readiness: "blocked" | "manual_review_required"; + summary: { passed: number; warnings: number; blockers: number; manualChecks: number }; + checks: SubmissionCheck[]; + findings: SubmissionFinding[]; + manualChecklist: SubmissionManualCheck[]; +} + +type Evidence = SubmissionFinding["evidence"]; +type TargetType = SubmissionPreflightReport["targetType"]; + +const packageNamePattern = /^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/; +const semverPattern = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/; +const singleLineUnsupported = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u202E\u2060-\u206F\uFEFF]/u; +const multilineUnsupported = /[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u202E\u2060-\u206F\uFEFF]/u; +const knownInterfaceFields = new Set([ + "displayName", "shortDescription", "longDescription", "developerName", "category", + "capabilities", "websiteURL", "supportURL", "privacyPolicyURL", "termsOfServiceURL", + "brandColor", "brandColorDark", "defaultPrompt", "composerIcon", "logo", "screenshots" +]); + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function finding( + id: `plugin.submission.${string}`, + severity: SubmissionFinding["severity"], + message: string, + evidence?: Evidence +): SubmissionFinding { + return evidence === undefined ? { id, severity, message } : { id, severity, message, evidence }; +} + +function invalidField(field: string, evidence?: Evidence): SubmissionFinding { + return finding(`plugin.submission.interface.${field}`, "fail", "Listing field is invalid.", { + field, + ...evidence + }); +} + +function hasValidText(value: unknown, maximum: number, multiline = false): value is string { + return typeof value === "string" + && value.trim().length > 0 + && value.length <= maximum + && !(multiline ? multilineUnsupported : singleLineUnsupported).test(value); +} + +function validateRequiredText( + source: Record, + property: string, + field: string, + limit: number, + multiline: boolean, + findings: SubmissionFinding[] +): void { + if (!hasValidText(source[property], limit, multiline)) { + findings.push(invalidField(field, { limit })); + } +} + +function validatePackage(manifest: Record, findings: SubmissionFinding[]): void { + const name = manifest.name; + if (typeof name !== "string" || name.length > submissionRuleset.limits.packageName || !packageNamePattern.test(name)) { + findings.push(finding("plugin.submission.package.name", "fail", "Package name is invalid.", { + field: "name", limit: submissionRuleset.limits.packageName + })); + } + + const version = manifest.version; + if (typeof version !== "string" || version.length > submissionRuleset.limits.version || !semverPattern.test(version)) { + findings.push(finding("plugin.submission.package.version", "fail", "Package version is invalid.", { + field: "version", limit: submissionRuleset.limits.version + })); + } +} + +function validateCapabilities(value: unknown, findings: SubmissionFinding[]): void { + if (value === undefined) { + return; + } + if (!Array.isArray(value) || value.length > submissionRuleset.limits.capabilities) { + findings.push(invalidField("capabilities", { limit: submissionRuleset.limits.capabilities })); + } + if (!Array.isArray(value)) { + return; + } + if (value.some((capability) => !hasValidText(capability, submissionRuleset.limits.capability))) { + findings.push(invalidField("capability", { limit: submissionRuleset.limits.capability })); + } +} + +function normalizePrompt(prompt: string): string { + return prompt.normalize("NFKC").trim().replace(/\s+/gu, " "); +} + +function validateDefaultPrompt(value: unknown, findings: SubmissionFinding[]): void { + if (value === undefined) { + return; + } + const prompts = typeof value === "string" ? [value] : value; + if (!Array.isArray(prompts) || prompts.length > submissionRuleset.limits.starterPrompts) { + findings.push(invalidField("default_prompt", { limit: submissionRuleset.limits.starterPrompts })); + return; + } + const normalized = new Set(); + const invalid = prompts.some((prompt) => { + if (!hasValidText(prompt, submissionRuleset.limits.starterPrompt) || prompt.includes("@")) { + return true; + } + const normalizedPrompt = normalizePrompt(prompt); + if (normalized.has(normalizedPrompt)) { + return true; + } + normalized.add(normalizedPrompt); + return false; + }); + if (invalid) { + findings.push(invalidField("default_prompt", { limit: submissionRuleset.limits.starterPrompt })); + } +} + +function isValidHttpsUrl(value: unknown): value is string { + if (typeof value !== "string" || value.length === 0 || value.length > submissionRuleset.limits.url + || value.trim() !== value || singleLineUnsupported.test(value)) { + return false; + } + try { + const parsed = new URL(value); + return parsed.protocol === "https:" && parsed.hostname.length > 0 + && parsed.username.length === 0 && parsed.password.length === 0 && parsed.hash.length === 0; + } catch { + return false; + } +} + +function validateListing(manifest: Record, targetType: TargetType): SubmissionFinding[] { + const findings: SubmissionFinding[] = []; + validatePackage(manifest, findings); + + const listing = manifest.interface; + if (!isRecord(listing)) { + findings.push(finding("plugin.submission.interface.required", "fail", "Listing interface mapping is required.", { field: "interface" })); + return findings; + } + + validateRequiredText(listing, "displayName", "display_name", submissionRuleset.limits.displayName, false, findings); + validateRequiredText(listing, "shortDescription", "short_description", submissionRuleset.limits.shortDescription, false, findings); + validateRequiredText(listing, "longDescription", "long_description", submissionRuleset.limits.longDescription, true, findings); + validateRequiredText(listing, "developerName", "developer_name", submissionRuleset.limits.developerName, false, findings); + validateRequiredText(listing, "category", "category", 80, false, findings); + if (typeof listing.category === "string" && !submissionRuleset.categories.includes(listing.category as never)) { + findings.push(invalidField("category")); + } + validateCapabilities(listing.capabilities, findings); + validateDefaultPrompt(listing.defaultPrompt, findings); + + if (targetType === "mcp-backed") { + for (const property of ["websiteURL", "supportURL", "privacyPolicyURL", "termsOfServiceURL"]) { + if (!isValidHttpsUrl(listing[property])) { + findings.push(finding("plugin.submission.interface.url", "fail", "Required listing URL is invalid.", { + field: property, limit: submissionRuleset.limits.url + })); + } + } + } + + for (const key of Object.keys(listing)) { + if (!knownInterfaceFields.has(key)) { + findings.push(finding("plugin.submission.interface.unknown_field", "warn", "Listing interface contains an unsupported field.", { field: key })); + } + } + return findings; +} + +function isWithin(rootPath: string, candidatePath: string): boolean { + const relative = path.relative(rootPath, candidatePath); + return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); +} + +async function validateApp(rootPath: string, declaration: unknown): Promise { + if (declaration === undefined) { + return []; + } + const invalid = () => [finding("plugin.submission.component.app", "fail", "App declaration must reference a contained parseable root file.", { path: ".app.json" })]; + if (declaration !== "./.app.json") { + return invalid(); + } + const candidatePath = path.resolve(rootPath, ".app.json"); + try { + const [canonicalRoot, canonicalCandidate, details] = await Promise.all([ + realpath(rootPath), + realpath(candidatePath), + stat(candidatePath) + ]); + if (!isWithin(canonicalRoot, canonicalCandidate) || !details.isFile() || details.size > 5 * 1024 * 1024) { + return invalid(); + } + JSON.parse(await readFile(candidatePath, "utf8")); + return []; + } catch { + return invalid(); + } +} + +function checkStatus(findings: SubmissionFinding[]): SubmissionCheck["status"] { + if (findings.some((item) => item.severity === "fail")) { + return "fail"; + } + return findings.some((item) => item.severity === "warn") ? "warn" : "pass"; +} + +function check(id: SubmissionCheck["id"], findings: SubmissionFinding[]): SubmissionCheck { + return { id, status: checkStatus(findings), findingIds: findings.map((item) => item.id) }; +} + +function checklist(targetType: TargetType): SubmissionManualCheck[] { + return submissionManualChecks.map((item) => ({ + id: item.id, + label: item.label, + state: item.mcpOnly && targetType === "skills-only" ? "not_applicable" : "required" + })); +} + +function invalidPackageReport(): SubmissionPreflightReport { + const findings = [finding("plugin.submission.package.invalid", "fail", "Plugin manifest is missing or invalid.")]; + const checks = [check("listing", findings), check("components", []), check("assets", []), check("skills", [])]; + return { + schemaVersion: "1.0.0", + rulesetVersion: submissionRuleset.version, + targetType: "skills-only", + status: "fail", + readiness: "blocked", + summary: { passed: 3, warnings: 0, blockers: 1, manualChecks: 3 }, + checks, + findings, + manualChecklist: checklist("skills-only") + }; +} + +export async function buildSubmissionPreflight(targetPath: string): Promise { + const discovered = await discoverPackage(targetPath); + if (!discovered || !isRecord(discovered.manifest)) { + return invalidPackageReport(); + } + + const manifest = discovered.manifest; + const targetType: TargetType = manifest.mcpServers !== undefined || manifest.apps !== undefined + ? "mcp-backed" + : "skills-only"; + const listingFindings = validateListing(manifest, targetType); + const componentFindings = await validateApp(discovered.rootPath, manifest.apps); + const interfaceValue = manifest.interface; + if (targetType === "skills-only" && isRecord(interfaceValue) && interfaceValue.screenshots !== undefined) { + componentFindings.push(finding("plugin.submission.component.excluded", "warn", "Screenshots are excluded for skills-only submissions.", { field: "screenshots" })); + } + const findings = [...listingFindings, ...componentFindings]; + const checks = [ + check("listing", listingFindings), + check("components", componentFindings), + check("assets", []), + check("skills", []) + ]; + const blockers = findings.filter((item) => item.severity === "fail").length; + const manualChecklist = checklist(targetType); + + return { + schemaVersion: "1.0.0", + rulesetVersion: submissionRuleset.version, + targetType, + status: blockers > 0 ? "fail" : "pass", + readiness: blockers > 0 ? "blocked" : "manual_review_required", + summary: { + passed: checks.filter((item) => item.status === "pass").length, + warnings: findings.filter((item) => item.severity === "warn").length, + blockers, + manualChecks: manualChecklist.filter((item) => item.state === "required").length + }, + checks, + findings, + manualChecklist + }; +} diff --git a/src/core/submission-ruleset.ts b/src/core/submission-ruleset.ts new file mode 100644 index 0000000..5fbe257 --- /dev/null +++ b/src/core/submission-ruleset.ts @@ -0,0 +1,39 @@ +export const submissionRuleset = Object.freeze({ + version: "openai-directory-2026-08-15", + reviewedAt: "2026-08-15", + sources: Object.freeze([ + "https://developers.openai.com/plugins/build/plugins", + "https://developers.openai.com/plugins/deploy/app-review", + "https://developers.openai.com/plugins/deploy/submission-errors" + ]), + limits: Object.freeze({ + packageName: 64, + version: 64, + displayName: 30, + shortDescription: 30, + longDescription: 4000, + developerName: 80, + capabilities: 20, + capability: 120, + starterPrompts: 3, + starterPrompt: 128, + url: 1024 + }), + categories: Object.freeze([ + "Productivity", "Creativity", "Developer Tools", "Business & Operations", + "Data & Analytics", "Communication", "Education & Research", "Security", + "Finance", "Healthcare", "Travel", "Entertainment", "Other" + ]) +}); + +export const submissionManualChecks = Object.freeze([ + Object.freeze({ id: "developer-business-identity", label: "Developer and business identity", mcpOnly: false }), + Object.freeze({ id: "attestations", label: "Required attestations", mcpOnly: false }), + Object.freeze({ id: "skill-safety-scan", label: "Skill safety scan", mcpOnly: false }), + Object.freeze({ id: "demo-video", label: "Demo video", mcpOnly: true }), + Object.freeze({ id: "tool-tests", label: "Exactly 5 positive and 3 negative tool tests", mcpOnly: true }), + Object.freeze({ id: "release-notes", label: "Release notes", mcpOnly: true }), + Object.freeze({ id: "production-domain-verification", label: "Production domain verification and current tool scan", mcpOnly: true }), + Object.freeze({ id: "tool-annotations", label: "Tool annotations and justifications", mcpOnly: true }), + Object.freeze({ id: "oauth-reviewer-credentials", label: "OAuth reviewer credentials", mcpOnly: true }) +]); diff --git a/tests/submission-preflight.test.ts b/tests/submission-preflight.test.ts new file mode 100644 index 0000000..81888bc --- /dev/null +++ b/tests/submission-preflight.test.ts @@ -0,0 +1,323 @@ +import { mkdir, mkdtemp, symlink, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; + +import { buildSubmissionPreflight } from "../src/core/submission-preflight.js"; +import { submissionRuleset } from "../src/core/submission-ruleset.js"; + +const validManifest = { + name: "submission-preflight", + version: "1.2.3", + skills: "./skills", + interface: { + displayName: "Submission helper", + shortDescription: "Check a submission", + longDescription: "A local submission checker.\nIt remains offline.", + developerName: "Example Developer", + category: "Developer Tools", + capabilities: ["Checks public listing metadata"], + defaultPrompt: "Check my plugin submission" + } +}; + +async function writePackage(manifest: unknown, files: Record = {}): Promise { + const directory = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-")); + const manifestDirectory = path.join(directory, ".codex-plugin"); + + await mkdir(manifestDirectory, { recursive: true }); + await writeFile(path.join(manifestDirectory, "plugin.json"), JSON.stringify(manifest), "utf8"); + + for (const [relativePath, content] of Object.entries(files)) { + const filePath = path.join(directory, relativePath); + + await mkdir(path.dirname(filePath), { recursive: true }); + await writeFile(filePath, content, "utf8"); + } + + return directory; +} + +function findingIds(report: Awaited>): string[] { + return report.findings.map((finding) => finding.id); +} + +function validMcpManifest(overrides: Record = {}) { + return { + ...validManifest, + mcpServers: "./.mcp.json", + interface: { + ...validManifest.interface, + websiteURL: "https://example.com", + supportURL: "https://example.com/support", + privacyPolicyURL: "https://example.com/privacy", + termsOfServiceURL: "https://example.com/terms" + }, + ...overrides + }; +} + +describe("submission preflight", () => { + it("publishes the immutable directory ruleset", () => { + expect(submissionRuleset).toEqual({ + version: "openai-directory-2026-08-15", + reviewedAt: "2026-08-15", + sources: [ + "https://developers.openai.com/plugins/build/plugins", + "https://developers.openai.com/plugins/deploy/app-review", + "https://developers.openai.com/plugins/deploy/submission-errors" + ], + limits: { + packageName: 64, + version: 64, + displayName: 30, + shortDescription: 30, + longDescription: 4000, + developerName: 80, + capabilities: 20, + capability: 120, + starterPrompts: 3, + starterPrompt: 128, + url: 1024 + }, + categories: [ + "Productivity", "Creativity", "Developer Tools", "Business & Operations", + "Data & Analytics", "Communication", "Education & Research", "Security", + "Finance", "Healthcare", "Travel", "Entertainment", "Other" + ] + }); + expect(Object.isFrozen(submissionRuleset)).toBe(true); + }); + + it.each([ + ["skills-only", validManifest], + ["mcp-backed", validMcpManifest()], + ["mcp-backed", { ...validManifest, apps: "./.app.json" }], + ["mcp-backed", { ...validManifest, mcpServers: null }], + ["mcp-backed", { ...validManifest, apps: null }] + ] as const)("classifies declarations by presence as %s", async (targetType, manifest) => { + const report = await buildSubmissionPreflight(await writePackage(manifest)); + + expect(report.targetType).toBe(targetType); + }); + + it("returns a manual-review-ready skills-only report without side effects", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch"); + const report = await buildSubmissionPreflight(await writePackage(validManifest)); + + expect(report).toMatchObject({ + schemaVersion: "1.0.0", + rulesetVersion: "openai-directory-2026-08-15", + targetType: "skills-only", + status: "pass", + readiness: "manual_review_required", + summary: { passed: 4, warnings: 0, blockers: 0, manualChecks: 3 } + }); + expect(report.checks).toEqual([ + { id: "listing", status: "pass", findingIds: [] }, + { id: "components", status: "pass", findingIds: [] }, + { id: "assets", status: "pass", findingIds: [] }, + { id: "skills", status: "pass", findingIds: [] } + ]); + expect(report.manualChecklist).toEqual([ + { id: "developer-business-identity", label: "Developer and business identity", state: "required" }, + { id: "attestations", label: "Required attestations", state: "required" }, + { id: "skill-safety-scan", label: "Skill safety scan", state: "required" }, + { id: "demo-video", label: "Demo video", state: "not_applicable" }, + { id: "tool-tests", label: "Exactly 5 positive and 3 negative tool tests", state: "not_applicable" }, + { id: "release-notes", label: "Release notes", state: "not_applicable" }, + { id: "production-domain-verification", label: "Production domain verification and current tool scan", state: "not_applicable" }, + { id: "tool-annotations", label: "Tool annotations and justifications", state: "not_applicable" }, + { id: "oauth-reviewer-credentials", label: "OAuth reviewer credentials", state: "not_applicable" } + ]); + expect(report.manualChecklist.every((item) => item.state !== "passed")).toBe(true); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("blocks missing and malformed manifests without exposing the package path", async () => { + const missing = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-missing-")); + const malformed = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-malformed-")); + + await mkdir(path.join(malformed, ".codex-plugin")); + await writeFile(path.join(malformed, ".codex-plugin", "plugin.json"), "{", "utf8"); + + for (const target of [missing, malformed]) { + const report = await buildSubmissionPreflight(target); + const invalid = report.findings.find((finding) => finding.id === "plugin.submission.package.invalid"); + + expect(report).toMatchObject({ status: "fail", readiness: "blocked", targetType: "skills-only" }); + expect(invalid).toMatchObject({ severity: "fail" }); + expect(JSON.stringify(report)).not.toContain(target); + } + }); + + it.each([ + ["package name", { ...validManifest, name: "a".repeat(64) }, false], + ["package name one over", { ...validManifest, name: "a".repeat(65) }, true], + ["version", { ...validManifest, version: "1.2.3+" + "a".repeat(58) }, false], + ["version one over", { ...validManifest, version: "1.2.3+" + "a".repeat(59) }, true], + ["invalid package name", { ...validManifest, name: "bad name" }, true], + ["invalid strict semver", { ...validManifest, version: "01.2.3" }, true], + ["malformed package fields", { ...validManifest, name: [], version: {} }, true] + ])("enforces %s boundaries", async (_caseName, manifest, invalid) => { + const report = await buildSubmissionPreflight(await writePackage(manifest)); + + expect(report.status === "fail").toBe(invalid); + }); + + it.each([ + ["displayName", "a".repeat(30), false], + ["displayName", "a".repeat(31), true], + ["shortDescription", "a".repeat(30), false], + ["shortDescription", "a".repeat(31), true], + ["longDescription", "a".repeat(4000), false], + ["longDescription", "a".repeat(4001), true], + ["developerName", "a".repeat(80), false], + ["developerName", "a".repeat(81), true] + ])("enforces %s length boundary", async (field, value, invalid) => { + const report = await buildSubmissionPreflight(await writePackage({ + ...validManifest, + interface: { ...validManifest.interface, [field]: value } + })); + + expect(report.status === "fail").toBe(invalid); + }); + + it("rejects malformed listing mappings, blank fields, forbidden newlines and invisible controls", async () => { + const cases: Array<{ interface: unknown; expected: string }> = [ + { interface: null, expected: "plugin.submission.interface.required" }, + { interface: { ...validManifest.interface, displayName: " \t " }, expected: "plugin.submission.interface.display_name" }, + { interface: { ...validManifest.interface, shortDescription: "line\nbreak" }, expected: "plugin.submission.interface.short_description" }, + { interface: { ...validManifest.interface, developerName: "a\r\nb" }, expected: "plugin.submission.interface.developer_name" }, + { interface: { ...validManifest.interface, category: "Developer\u2028Tools" }, expected: "plugin.submission.interface.category" }, + { interface: { ...validManifest.interface, displayName: "zero\u200Bwidth" }, expected: "plugin.submission.interface.display_name" }, + { interface: { ...validManifest.interface, longDescription: "bidi\u202Etext" }, expected: "plugin.submission.interface.long_description" }, + { interface: { ...validManifest.interface, longDescription: "acceptable\r\nmultiline" }, expected: "" } + ]; + + for (const testCase of cases) { + const report = await buildSubmissionPreflight(await writePackage({ ...validManifest, interface: testCase.interface })); + + if (testCase.expected) { + expect(findingIds(report)).toContain(testCase.expected); + } else { + expect(report.status).toBe("pass"); + } + } + }); + + it("validates categories, capabilities, prompts and redacts listing content", async () => { + const sentinel = "listing-content-sentinel"; + const report = await buildSubmissionPreflight(await writePackage({ + ...validManifest, + interface: { + ...validManifest.interface, + category: "Not a category", + capabilities: ["valid", " ", "line\nbreak", "a".repeat(121), ...Array.from({ length: 17 }, (_, index) => `c${index}`)], + defaultPrompt: [" Prompt", "Ｐｒｏｍｐｔ ", `@app ${sentinel}`, "one too many"] + } + })); + + expect(findingIds(report)).toEqual(expect.arrayContaining([ + "plugin.submission.interface.category", + "plugin.submission.interface.capabilities", + "plugin.submission.interface.capability", + "plugin.submission.interface.default_prompt" + ])); + expect(JSON.stringify(report)).not.toContain(sentinel); + }); + + it.each([ + ["20 capabilities", Array.from({ length: 20 }, (_, index) => index === 0 ? "a".repeat(120) : `capability ${index}`), ["one", "two", "a".repeat(128)], true], + ["21 capabilities", Array.from({ length: 21 }, (_, index) => `capability ${index}`), ["one"], false], + ["prompt one over", ["capability"], ["a".repeat(129)], false], + ["four prompts", ["capability"], ["one", "two", "three", "four"], false] + ])("accepts only bounded capability and prompt entries for %s", async (_caseName, capabilities, defaultPrompt, valid) => { + const report = await buildSubmissionPreflight(await writePackage({ + ...validManifest, + interface: { ...validManifest.interface, capabilities, defaultPrompt } + })); + + expect(report.status === "pass").toBe(valid); + }); + + it("requires valid MCP listing URLs and never returns their contents", async () => { + const sentinel = "url-secret-sentinel"; + const report = await buildSubmissionPreflight(await writePackage(validMcpManifest({ + interface: { + ...validMcpManifest().interface, + websiteURL: `https://user:${sentinel}@example.com/path#fragment`, + supportURL: "http://example.com", + privacyPolicyURL: " ", + termsOfServiceURL: "https://example.com/\u200B" + } + }))); + + expect(findingIds(report)).toContain("plugin.submission.interface.url"); + expect(JSON.stringify(report)).not.toContain(sentinel); + expect(report.manualChecklist.filter((item) => item.state === "required")).toHaveLength(9); + }); + + it("warns for unknown interface fields without retaining values and excludes skills-only screenshots", async () => { + const sentinel = "unknown-value-sentinel"; + const report = await buildSubmissionPreflight(await writePackage({ + ...validManifest, + interface: { ...validManifest.interface, unknownSubmissionField: sentinel, screenshots: ["./shot.png"] } + })); + + expect(findingIds(report)).toEqual(expect.arrayContaining([ + "plugin.submission.interface.unknown_field", + "plugin.submission.component.excluded" + ])); + expect(report.status).toBe("pass"); + expect(report.summary).toMatchObject({ warnings: 2, blockers: 0, passed: 2 }); + expect(JSON.stringify(report)).not.toContain(sentinel); + }); + + it.each([ + ["missing app", "./.app.json", {}, "plugin.submission.component.app"], + ["wrong path", "./apps/app.json", { "apps/app.json": "{}" }, "plugin.submission.component.app"], + ["wrong type", [], {}, "plugin.submission.component.app"], + ["traversal", "../.app.json", { "../.app.json": "{}" }, "plugin.submission.component.app"], + ["invalid JSON", "./.app.json", { ".app.json": "{" }, "plugin.submission.component.app"] + ])("blocks %s at the app boundary", async (_caseName, apps, files, expected) => { + const report = await buildSubmissionPreflight(await writePackage({ ...validMcpManifest(), apps }, files)); + + expect(findingIds(report)).toContain(expected); + }); + + it.each(["null", "[]", "42", "true", "\"a primitive\"", "{}"]) ("accepts parseable %s app JSON without inferring its schema", async (contents) => { + const report = await buildSubmissionPreflight(await writePackage({ ...validMcpManifest(), apps: "./.app.json" }, { ".app.json": contents })); + + expect(report.status).toBe("pass"); + }); + + it("rejects an app file whose canonical path escapes the package", async () => { + const target = await writePackage({ ...validMcpManifest(), apps: "./.app.json" }); + const external = path.join(os.tmpdir(), `submission-app-escape-${Date.now()}.json`); + + await writeFile(external, "{}", "utf8"); + try { + await symlink(external, path.join(target, ".app.json"), "file"); + } catch (error: unknown) { + if ((error as NodeJS.ErrnoException).code === "EPERM") { + return; + } + throw error; + } + + const report = await buildSubmissionPreflight(target); + + expect(findingIds(report)).toContain("plugin.submission.component.app"); + expect(JSON.stringify(report)).not.toContain(external); + }); + + it("rejects a root app declaration that points at a directory", async () => { + const target = await writePackage({ ...validMcpManifest(), apps: "./.app.json" }); + await mkdir(path.join(target, ".app.json")); + + const report = await buildSubmissionPreflight(target); + + expect(findingIds(report)).toContain("plugin.submission.component.app"); + }); +}); From 94b4a94d49ed4f44f97940714d69e3955fb924d0 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 10:16:39 +0300 Subject: [PATCH 05/24] fix: block skills-only submission screenshots --- src/core/submission-preflight.ts | 2 +- tests/submission-preflight.test.ts | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts index f47d02f..6856df6 100644 --- a/src/core/submission-preflight.ts +++ b/src/core/submission-preflight.ts @@ -282,7 +282,7 @@ export async function buildSubmissionPreflight(targetPath: string): Promise { expect(report.manualChecklist.filter((item) => item.state === "required")).toHaveLength(9); }); - it("warns for unknown interface fields without retaining values and excludes skills-only screenshots", async () => { + it("warns for unknown interface fields without retaining values and blocks skills-only screenshots", async () => { const sentinel = "unknown-value-sentinel"; const report = await buildSubmissionPreflight(await writePackage({ ...validManifest, @@ -269,8 +269,11 @@ describe("submission preflight", () => { "plugin.submission.interface.unknown_field", "plugin.submission.component.excluded" ])); - expect(report.status).toBe("pass"); - expect(report.summary).toMatchObject({ warnings: 2, blockers: 0, passed: 2 }); + expect(report.status).toBe("fail"); + expect(report.readiness).toBe("blocked"); + expect(report.summary).toMatchObject({ warnings: 1, blockers: 1, passed: 2 }); + expect(report.findings.find((finding) => finding.id === "plugin.submission.component.excluded")) + .toMatchObject({ severity: "fail" }); expect(JSON.stringify(report)).not.toContain(sentinel); }); From 52648764e885a3b9f65d221a59cb445cb26a47c9 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 10:23:20 +0300 Subject: [PATCH 06/24] fix: fail closed on submission target input --- src/core/submission-preflight.ts | 9 ++++++- tests/submission-preflight.test.ts | 43 +++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts index 6856df6..b244c27 100644 --- a/src/core/submission-preflight.ts +++ b/src/core/submission-preflight.ts @@ -213,6 +213,7 @@ async function validateApp(rootPath: string, declaration: unknown): Promise [finding("plugin.submission.component.app", "fail", "App declaration must reference a contained parseable root file.", { path: ".app.json" })]; + const invalidPath = () => [finding("plugin.submission.app.invalid_path", "fail", "App declaration resolves outside the package.", { path: ".app.json" })]; if (declaration !== "./.app.json") { return invalid(); } @@ -223,7 +224,10 @@ async function validateApp(rootPath: string, declaration: unknown): Promise 5 * 1024 * 1024) { + if (!isWithin(canonicalRoot, canonicalCandidate)) { + return invalidPath(); + } + if (!details.isFile() || details.size > 5 * 1024 * 1024) { return invalid(); } JSON.parse(await readFile(candidatePath, "utf8")); @@ -269,6 +273,9 @@ function invalidPackageReport(): SubmissionPreflightReport { } export async function buildSubmissionPreflight(targetPath: string): Promise { + if (typeof targetPath !== "string") { + return invalidPackageReport(); + } const discovered = await discoverPackage(targetPath); if (!discovered || !isRecord(discovered.manifest)) { return invalidPackageReport(); diff --git a/tests/submission-preflight.test.ts b/tests/submission-preflight.test.ts index e621b23..337460a 100644 --- a/tests/submission-preflight.test.ts +++ b/tests/submission-preflight.test.ts @@ -151,6 +151,18 @@ describe("submission preflight", () => { } }); + it("fails closed for a non-string public target input", async () => { + const report = await buildSubmissionPreflight(null as unknown as string); + + expect(report).toMatchObject({ + targetType: "skills-only", + status: "fail", + readiness: "blocked" + }); + expect(findingIds(report)).toEqual(["plugin.submission.package.invalid"]); + expect(JSON.stringify(report)).not.toContain("null"); + }); + it.each([ ["package name", { ...validManifest, name: "a".repeat(64) }, false], ["package name one over", { ...validManifest, name: "a".repeat(65) }, true], @@ -295,23 +307,30 @@ describe("submission preflight", () => { expect(report.status).toBe("pass"); }); - it("rejects an app file whose canonical path escapes the package", async () => { - const target = await writePackage({ ...validMcpManifest(), apps: "./.app.json" }); - const external = path.join(os.tmpdir(), `submission-app-escape-${Date.now()}.json`); + if (process.platform !== "win32") { + it("rejects a file symlink whose canonical app path escapes the package", async () => { + const target = await writePackage({ ...validMcpManifest(), apps: "./.app.json" }); + const external = path.join(os.tmpdir(), `submission-app-escape-${Date.now()}.json`); - await writeFile(external, "{}", "utf8"); - try { + await writeFile(external, "{}", "utf8"); await symlink(external, path.join(target, ".app.json"), "file"); - } catch (error: unknown) { - if ((error as NodeJS.ErrnoException).code === "EPERM") { - return; - } - throw error; - } + + const report = await buildSubmissionPreflight(target); + + expect(findingIds(report)).toContain("plugin.submission.app.invalid_path"); + expect(JSON.stringify(report)).not.toContain(external); + }); + } + + it("rejects a junction whose canonical app path escapes the package before file classification", async () => { + const target = await writePackage({ ...validMcpManifest(), apps: "./.app.json" }); + const external = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-app-escape-")); + + await symlink(external, path.join(target, ".app.json"), "junction"); const report = await buildSubmissionPreflight(target); - expect(findingIds(report)).toContain("plugin.submission.component.app"); + expect(findingIds(report)).toContain("plugin.submission.app.invalid_path"); expect(JSON.stringify(report)).not.toContain(external); }); From 3897a1cd2e980dda37a608114e84825c271d3ece Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 10:39:22 +0300 Subject: [PATCH 07/24] feat: validate submission branding assets --- package-lock.json | 123 +++++++++++++++ package.json | 3 + src/core/submission-assets.ts | 246 +++++++++++++++++++++++++++++ src/core/submission-preflight.ts | 6 +- tests/submission-assets.test.ts | 171 ++++++++++++++++++++ tests/submission-preflight.test.ts | 18 ++- 6 files changed, 562 insertions(+), 5 deletions(-) create mode 100644 src/core/submission-assets.ts create mode 100644 tests/submission-assets.test.ts diff --git a/package-lock.json b/package-lock.json index 01421e6..64fae44 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,6 +8,9 @@ "name": "codex-plugin-doctor", "version": "1.58.0", "license": "MIT", + "dependencies": { + "fast-xml-parser": "^5.11.0" + }, "bin": { "codex-plugin-doctor": "dist/cli.js" }, @@ -470,6 +473,18 @@ "dev": true, "license": "MIT" }, + "node_modules/@nodable/entities": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@nodable/entities/-/entities-3.0.0.tgz", + "integrity": "sha512-8L9xFeTYKhm49xfIypoe2W5wV1m/3Z58kT+7kR9A8OyFxcPduI4VmxaUMQyKYrRjUoLLSXv6EKKID5Tvj9cUVw==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/nodable" + } + ], + "license": "MIT" + }, "node_modules/@rollup/rollup-android-arm-eabi": { "version": "4.60.2", "resolved": "https://registry.npmjs.org/@rollup/rollup-android-arm-eabi/-/rollup-android-arm-eabi-4.60.2.tgz", @@ -1009,6 +1024,18 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/anynum": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/anynum/-/anynum-1.0.1.tgz", + "integrity": "sha512-N6//FLET/tXYNM/F6ABca1oH6fWB+KlTt909Le28WMDBk8oaT4vY17DCrwg2MvmuqUKt3Ni4N5dGJ/EoBgcO6A==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT" + }, "node_modules/assertion-error": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/assertion-error/-/assertion-error-2.0.1.tgz", @@ -1153,6 +1180,45 @@ "node": ">=12.0.0" } }, + "node_modules/fast-xml-builder": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.3.1.tgz", + "integrity": "sha512-pIM/1n3ntFXKYrUZwW7QCK0gAW7XY+wzj1YMIV3tLDvPj/V+zTGJK5e3/4WJfwj0qWw2ElNXiTixda/R+3YSug==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "dependencies": { + "path-expression-matcher": "^1.6.2", + "xml-naming": "^0.3.0" + } + }, + "node_modules/fast-xml-parser": { + "version": "5.11.0", + "resolved": "https://registry.npmjs.org/fast-xml-parser/-/fast-xml-parser-5.11.0.tgz", + "integrity": "sha512-9IGxMqvqLOnqP+Egi1nqDHKv5k8aZ7r9n558enxcucmyVGEBNPAU+MOg/8jPIS7rO7sSq4gFm1/nHtiaubMruw==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "dependencies": { + "@nodable/entities": "^3.0.0", + "fast-xml-builder": "^1.2.0", + "is-unsafe": "^2.0.0", + "path-expression-matcher": "^1.6.2", + "strnum": "^2.4.2", + "xml-naming": "^0.3.0" + }, + "bin": { + "fxparser": "src/cli/cli.js" + } + }, "node_modules/fdir": { "version": "6.5.0", "resolved": "https://registry.npmjs.org/fdir/-/fdir-6.5.0.tgz", @@ -1186,6 +1252,18 @@ "node": "^8.16.0 || ^10.6.0 || >=11.0.0" } }, + "node_modules/is-unsafe": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/is-unsafe/-/is-unsafe-2.0.0.tgz", + "integrity": "sha512-2LdV822R+wmI86unXA93WCFpL6g+av8ynWk0nrHyJqGop5VoocYsSLFgN8jrfalT6iGeLNM4KXuVSsULP53kEA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT" + }, "node_modules/js-tokens": { "version": "9.0.1", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-9.0.1.tgz", @@ -1236,6 +1314,21 @@ "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" } }, + "node_modules/path-expression-matcher": { + "version": "1.6.2", + "resolved": "https://registry.npmjs.org/path-expression-matcher/-/path-expression-matcher-1.6.2.tgz", + "integrity": "sha512-enSlaiat05iasnzmgNxRj8reFdj3puY2QpNgP1aPIaVfT6nn9ICuPoFlKHk8EN22HcwewshO+mN2DGbkCEOtqQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "engines": { + "node": ">=14.0.0" + } + }, "node_modules/pathe": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/pathe/-/pathe-2.0.3.tgz", @@ -1391,6 +1484,21 @@ "url": "https://github.com/sponsors/antfu" } }, + "node_modules/strnum": { + "version": "2.4.2", + "resolved": "https://registry.npmjs.org/strnum/-/strnum-2.4.2.tgz", + "integrity": "sha512-rDG3Ah4TV0k1hWvLSzkZtMmLN9+eS+h3knq4MP6A42Y3Yh5qGNnOUs1jJkoSr8FG5dsL28c7KgkIBzSEykqtuw==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "dependencies": { + "anynum": "^1.0.1" + } + }, "node_modules/tinybench": { "version": "2.9.0", "resolved": "https://registry.npmjs.org/tinybench/-/tinybench-2.9.0.tgz", @@ -1679,6 +1787,21 @@ "engines": { "node": ">=8" } + }, + "node_modules/xml-naming": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/xml-naming/-/xml-naming-0.3.0.tgz", + "integrity": "sha512-ghig2TBE/H11aOVgmahA3MhimvkBr6JIYknH/Dhdk10nXwdbIqBJsbfMxpvFPG8bAw77gN29aQWvKpmVoPlvPQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "engines": { + "node": ">=16.0.0" + } } } } diff --git a/package.json b/package.json index c2caf1a..43c0aef 100644 --- a/package.json +++ b/package.json @@ -60,5 +60,8 @@ "tsx": "^4.20.4", "typescript": "^5.9.2", "vitest": "^3.2.4" + }, + "dependencies": { + "fast-xml-parser": "^5.11.0" } } diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts new file mode 100644 index 0000000..d768b66 --- /dev/null +++ b/src/core/submission-assets.ts @@ -0,0 +1,246 @@ +import { readFile, stat } from "node:fs/promises"; +import path from "node:path"; + +import { XMLParser, XMLValidator } from "fast-xml-parser"; + +import type { DiscoveredPackage } from "../domain/types.js"; +import { resolveSafePackagePath } from "./plugin-components.js"; +import type { SubmissionFinding } from "./submission-preflight.js"; + +const maxAssetBytes = 5 * 1024 * 1024; +const minimumDimension = 48; +const maximumDimension = 4096; +const assetFields = ["logo", "composerIcon"] as const; +const extensions = new Map([[".png", "png"], [".jpg", "jpeg"], [".jpeg", "jpeg"], [".webp", "webp"], [".svg", "svg"]] as const); +const sofMarkers = new Set([0xc0, 0xc1, 0xc2, 0xc3, 0xc5, 0xc6, 0xc7, 0xc9, 0xca, 0xcb, 0xcd, 0xce, 0xcf]); +const numeric = /^[+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?$/; + +type AssetFormat = "png" | "jpeg" | "webp" | "svg"; +type Dimensions = { width: number; height: number }; +type Evidence = SubmissionFinding["evidence"]; + +function finding( + id: `plugin.submission.asset.${string}`, + message: string, + evidence: Evidence +): SubmissionFinding { + return { id, severity: "fail", message, evidence }; +} + +function assetEvidence(field: string, packagePath?: string, format?: AssetFormat, dimensions?: Dimensions): Evidence { + return { + field, + ...(packagePath === undefined ? {} : { path: packagePath }), + ...(format === undefined ? {} : { format }), + ...(dimensions === undefined ? {} : dimensions) + }; +} + +function readPng(buffer: Uint8Array): Dimensions | null { + if (buffer.length < 24 || ![137, 80, 78, 71, 13, 10, 26, 10].every((byte, index) => buffer[index] === byte) + || buffer[8] !== 0 || buffer[9] !== 0 || buffer[10] !== 0 || buffer[11] !== 13 + || buffer[12] !== 73 || buffer[13] !== 72 || buffer[14] !== 68 || buffer[15] !== 82) { + return null; + } + const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); + return { width: view.getUint32(16), height: view.getUint32(20) }; +} + +function readJpeg(buffer: Uint8Array): Dimensions | null { + if (buffer.length < 4 || buffer[0] !== 0xff || buffer[1] !== 0xd8) { + return null; + } + const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); + let offset = 2; + for (let steps = 0; steps < 128 && offset < buffer.length; steps += 1) { + while (offset < buffer.length && buffer[offset] === 0xff) { + offset += 1; + } + if (offset >= buffer.length) { + return null; + } + const marker = buffer[offset++]; + if (marker === 0xd9 || marker === 0xda) { + return null; + } + if (marker === 0x01 || (marker >= 0xd0 && marker <= 0xd7)) { + continue; + } + if (offset + 2 > buffer.length) { + return null; + } + const length = view.getUint16(offset); + if (length < 2 || offset + length > buffer.length) { + return null; + } + if (sofMarkers.has(marker)) { + const start = offset + 2; + if (start + 5 > offset + length) { + return null; + } + return { height: view.getUint16(start + 1), width: view.getUint16(start + 3) }; + } + offset += length; + } + return null; +} + +function fourCc(buffer: Uint8Array, offset: number): string { + return String.fromCharCode(...buffer.slice(offset, offset + 4)); +} + +function readWebp(buffer: Uint8Array): Dimensions | null { + if (buffer.length < 20 || fourCc(buffer, 0) !== "RIFF" || fourCc(buffer, 8) !== "WEBP") { + return null; + } + const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); + const riffSize = view.getUint32(4, true); + if (riffSize !== buffer.length - 8) { + return null; + } + const chunkType = fourCc(buffer, 12); + const chunkLength = view.getUint32(16, true); + const payload = 20; + if (chunkLength > buffer.length - payload) { + return null; + } + if (chunkType === "VP8X") { + if (chunkLength < 10) return null; + const width = 1 + buffer[payload + 4] + (buffer[payload + 5] << 8) + (buffer[payload + 6] << 16); + const height = 1 + buffer[payload + 7] + (buffer[payload + 8] << 8) + (buffer[payload + 9] << 16); + return { width, height }; + } + if (chunkType === "VP8L") { + if (chunkLength < 5 || buffer[payload] !== 0x2f) return null; + const packed = view.getUint32(payload + 1, true); + return { width: (packed & 0x3fff) + 1, height: ((packed >>> 14) & 0x3fff) + 1 }; + } + if (chunkType === "VP8 ") { + if (chunkLength < 10 || buffer[payload + 3] !== 0x9d || buffer[payload + 4] !== 0x01 || buffer[payload + 5] !== 0x2a) return null; + return { width: view.getUint16(payload + 6, true) & 0x3fff, height: view.getUint16(payload + 8, true) & 0x3fff }; + } + return null; +} + +function rasterAsset(buffer: Uint8Array): { format: Exclude; dimensions: Dimensions } | null { + const candidates: Array<[Exclude, (source: Uint8Array) => Dimensions | null]> = [ + ["png", readPng], ["jpeg", readJpeg], ["webp", readWebp] + ]; + for (const [format, reader] of candidates) { + const dimensions = reader(buffer); + if (dimensions !== null) return { format, dimensions }; + } + return null; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function numberFrom(value: unknown): number | null { + if (typeof value !== "string" || !numeric.test(value)) return null; + const number = Number(value); + return Number.isFinite(number) ? number : null; +} + +function svgDimensions(content: string): Dimensions | null { + if (/ part === null)) return null; + return { width: parts[2]!, height: parts[3]! }; + } + const width = numberFrom(root["@_width"]); + const height = numberFrom(root["@_height"]); + return width === null || height === null ? null : { width, height }; + } catch { + return null; + } +} + +function dimensionFinding(field: string, packagePath: string, format: AssetFormat, dimensions: Dimensions): SubmissionFinding | null { + const evidence = assetEvidence(field, packagePath, format, dimensions); + if (!Number.isInteger(dimensions.width) || !Number.isInteger(dimensions.height) || dimensions.width <= 0 || dimensions.height <= 0) { + return finding("plugin.submission.asset.dimensions", "Asset dimensions must be positive integers.", evidence); + } + if (dimensions.width !== dimensions.height) { + return finding("plugin.submission.asset.not_square", "Asset dimensions must be square.", evidence); + } + if (dimensions.width < minimumDimension || dimensions.width > maximumDimension) { + return finding("plugin.submission.asset.dimensions", "Asset dimensions are outside the allowed range.", { ...evidence, limit: minimumDimension }); + } + return null; +} + +async function validateAsset(rootPath: string, field: typeof assetFields[number], value: unknown): Promise { + if (value === undefined) { + return [finding("plugin.submission.asset.required", "Branding asset is required.", assetEvidence(field))]; + } + if (typeof value !== "string") { + return [finding("plugin.submission.asset.invalid_path", "Branding asset path is invalid.", assetEvidence(field))]; + } + const resolved = await resolveSafePackagePath(rootPath, value); + if (resolved === null) { + return [finding("plugin.submission.asset.invalid_path", "Branding asset path is invalid.", assetEvidence(field))]; + } + const extension = extensions.get(path.extname(resolved.packagePath).toLowerCase() as ".png" | ".jpg" | ".jpeg" | ".webp" | ".svg"); + if (extension === undefined) { + return [finding("plugin.submission.asset.unsupported_format", "Branding asset format is unsupported.", assetEvidence(field, resolved.packagePath))]; + } + let details; + try { + details = await stat(resolved.path); + } catch { + return [finding("plugin.submission.asset.missing", "Branding asset is missing.", assetEvidence(field, resolved.packagePath, extension))]; + } + if (!details.isFile()) { + return [finding("plugin.submission.asset.unsupported_format", "Branding asset must be a regular file.", assetEvidence(field, resolved.packagePath, extension))]; + } + if (details.size > maxAssetBytes) { + return [finding("plugin.submission.asset.too_large", "Branding asset exceeds the size limit.", { ...assetEvidence(field, resolved.packagePath, extension), limit: maxAssetBytes })]; + } + let content: Uint8Array; + try { + content = await readFile(resolved.path); + } catch { + return [finding("plugin.submission.asset.missing", "Branding asset cannot be read.", assetEvidence(field, resolved.packagePath, extension))]; + } + if (extension === "svg") { + let source: string; + try { + source = new TextDecoder("utf-8", { fatal: true }).decode(content); + } catch { + return [finding("plugin.submission.asset.unsafe_svg", "SVG must be valid UTF-8.", assetEvidence(field, resolved.packagePath, extension))]; + } + const dimensions = svgDimensions(source); + if (dimensions === null) { + return [finding("plugin.submission.asset.unsafe_svg", "SVG is unsafe or lacks valid dimensions.", assetEvidence(field, resolved.packagePath, extension))]; + } + const invalidDimensions = dimensionFinding(field, resolved.packagePath, extension, dimensions); + return invalidDimensions === null ? [] : [invalidDimensions]; + } + const decoded = rasterAsset(content); + if (decoded === null) { + return [finding("plugin.submission.asset.decode_failed", "Branding asset could not be decoded.", assetEvidence(field, resolved.packagePath, extension))]; + } + if (decoded.format !== extension) { + return [finding("plugin.submission.asset.extension_mismatch", "Branding asset extension does not match its content.", assetEvidence(field, resolved.packagePath, decoded.format, decoded.dimensions))]; + } + const invalidDimensions = dimensionFinding(field, resolved.packagePath, decoded.format, decoded.dimensions); + return invalidDimensions === null ? [] : [invalidDimensions]; +} + +export async function validateSubmissionAssets(discoveredPackage: DiscoveredPackage): Promise<{ findings: SubmissionFinding[] }> { + const listing = discoveredPackage.manifest.interface; + const interfaceValues = isRecord(listing) ? listing : {}; + const result = await Promise.all(assetFields.map((field) => validateAsset(discoveredPackage.rootPath, field, interfaceValues[field]))); + return { findings: result.flat() }; +} diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts index b244c27..81db784 100644 --- a/src/core/submission-preflight.ts +++ b/src/core/submission-preflight.ts @@ -2,6 +2,7 @@ import { readFile, realpath, stat } from "node:fs/promises"; import path from "node:path"; import { discoverPackage } from "./discover-package.js"; +import { validateSubmissionAssets } from "./submission-assets.js"; import { submissionManualChecks, submissionRuleset } from "./submission-ruleset.js"; export interface SubmissionFinding { @@ -287,15 +288,16 @@ export async function buildSubmissionPreflight(targetPath: string): Promise item.severity === "fail").length; diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts new file mode 100644 index 0000000..c6bd5d8 --- /dev/null +++ b/tests/submission-assets.test.ts @@ -0,0 +1,171 @@ +import { mkdir, mkdtemp, readFile, symlink, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import type { DiscoveredPackage } from "../src/domain/types.js"; +import { validateSubmissionAssets } from "../src/core/submission-assets.js"; + +type AssetFiles = Record; + +const png = (width: number, height: number) => { + const data = new Uint8Array(24); + data.set([137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82]); + new DataView(data.buffer).setUint32(16, width); + new DataView(data.buffer).setUint32(20, height); + return data; +}; + +const jpeg = (width: number, height: number) => { + const data = new Uint8Array(23); + data.set([0xff, 0xd8, 0xff, 0xc0, 0, 17, 8]); + const view = new DataView(data.buffer); + view.setUint16(7, height); + view.setUint16(9, width); + data.set([3, 1, 17, 0, 2, 17, 0, 3, 17, 0], 11); + return data; +}; + +const webp = (variant: "VP8X" | "VP8L" | "VP8", width: number, height: number) => { + const payload = variant === "VP8X" ? new Uint8Array(10) : variant === "VP8L" ? new Uint8Array(5) : new Uint8Array(10); + const view = new DataView(payload.buffer); + if (variant === "VP8X") { + payload.set([0, 0, 0, 0]); + payload[4] = (width - 1) & 0xff; payload[5] = ((width - 1) >>> 8) & 0xff; payload[6] = ((width - 1) >>> 16) & 0xff; + payload[7] = (height - 1) & 0xff; payload[8] = ((height - 1) >>> 8) & 0xff; payload[9] = ((height - 1) >>> 16) & 0xff; + } else if (variant === "VP8L") { + payload[0] = 0x2f; + const packed = (width - 1) | ((height - 1) << 14); + view.setUint32(1, packed, true); + } else { + payload.set([0, 0, 0, 0x9d, 0x01, 0x2a]); + view.setUint16(6, width, true); view.setUint16(8, height, true); + } + const data = new Uint8Array(20 + payload.length); + data.set([82, 73, 70, 70]); + new DataView(data.buffer).setUint32(4, data.length - 8, true); + data.set([87, 69, 66, 80, ...variant.padEnd(4, " ").split("").map((character) => character.charCodeAt(0))], 8); + new DataView(data.buffer).setUint32(16, payload.length, true); + data.set(payload, 20); + return data; +}; + +const svg = (attributes: string) => ``; + +async function packageWithAssets(interfaceValues: Record, files: AssetFiles = {}): Promise { + const rootPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-assets-")); + await mkdir(path.join(rootPath, ".codex-plugin")); + for (const [relativePath, content] of Object.entries(files)) { + const filePath = path.join(rootPath, relativePath); + await mkdir(path.dirname(filePath), { recursive: true }); + await writeFile(filePath, content); + } + return { rootPath, manifestPath: path.join(rootPath, ".codex-plugin", "plugin.json"), manifest: { interface: interfaceValues } } as DiscoveredPackage; +} + +async function findingIds(interfaceValues: Record, files: AssetFiles = {}): Promise { + return (await validateSubmissionAssets(await packageWithAssets(interfaceValues, files))).findings.map((finding) => finding.id); +} + +describe("submission assets", () => { + it.each([ + ["PNG", "./logo.png", png(48, 48)], + ["JPEG", "./logo.jpg", jpeg(48, 48)], + ["WebP VP8X", "./logo.webp", webp("VP8X", 48, 48)], + ["WebP VP8L", "./logo.webp", webp("VP8L", 48, 48)], + ["WebP VP8", "./logo.webp", webp("VP8", 48, 48)], + ["largest PNG", "./logo.png", png(4096, 4096)], + ["SVG viewBox", "./logo.svg", svg('viewBox="0 0 48 48"')], + ["SVG dimensions", "./logo.svg", svg('width="48" height="48"')] + ])("accepts a valid %s asset", async (_name, assetPath, content) => { + expect(await findingIds({ logo: assetPath, composerIcon: assetPath }, { [assetPath.slice(2)]: content })).toEqual([]); + }); + + it("requires both branding assets", async () => { + expect(await findingIds({}, {})).toEqual(["plugin.submission.asset.required", "plugin.submission.asset.required"]); + }); + + it.each([undefined, "logo.png", "../logo.png", "./../logo.png", "/logo.png", 42])("rejects invalid asset paths", async (value) => { + const ids = await findingIds({ logo: value, composerIcon: "./icon.png" }, { "icon.png": png(48, 48) }); + expect(ids).toContain(value === undefined ? "plugin.submission.asset.required" : "plugin.submission.asset.invalid_path"); + }); + + it.each([ + ["missing", "./missing.png", {}], + ["directory", "./assets", { assets: "" }], + ["empty", "./logo.png", { "logo.png": new Uint8Array() }], + ["unsupported extension", "./logo.gif", { "logo.gif": png(48, 48) }], + ["unsupported content", "./logo.png", { "logo.png": new Uint8Array([1, 2, 3]) }], + ["extension mismatch", "./logo.jpg", { "logo.jpg": png(48, 48) }], + ["malformed PNG IHDR", "./logo.png", { "logo.png": (() => { const image = png(48, 48); image[11] = 12; return image; })() }], + ["truncated png", "./logo.png", { "logo.png": png(48, 48).slice(0, 20) }], + ["overflow dimensions", "./logo.png", { "logo.png": png(0xffffffff, 48) }], + ["rectangle", "./logo.png", { "logo.png": png(48, 49) }], + ["one pixel under", "./logo.png", { "logo.png": png(47, 47) }], + ["one pixel over", "./logo.png", { "logo.png": png(4097, 4097) }] + ])("rejects %s", async (_name, assetPath, files) => { + const ids = await findingIds({ logo: assetPath, composerIcon: assetPath }, files as AssetFiles); + expect(ids.some((id) => id.startsWith("plugin.submission.asset."))).toBe(true); + }); + + it("rejects a directory, truncated JPEG and malformed WebP", async () => { + const directoryPackage = await packageWithAssets({ logo: "./assets", composerIcon: "./icon.png" }, { "icon.png": png(48, 48) }); + await mkdir(path.join(directoryPackage.rootPath, "assets")); + expect((await validateSubmissionAssets(directoryPackage)).findings.map((finding) => finding.id)) + .toContain("plugin.submission.asset.unsupported_format"); + expect(await findingIds({ logo: "./logo.jpg", composerIcon: "./logo.jpg" }, { "logo.jpg": new Uint8Array([0xff, 0xd8, 0xff, 0xc0, 0, 17]) })) + .toContain("plugin.submission.asset.decode_failed"); + const malformed = webp("VP8X", 48, 48); + new DataView(malformed.buffer).setUint32(4, 0, true); + expect(await findingIds({ logo: "./logo.webp", composerIcon: "./logo.webp" }, { "logo.webp": malformed })) + .toContain("plugin.submission.asset.decode_failed"); + }); + + it("rejects an oversized asset before decoding it", async () => { + const ids = await findingIds({ logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": new Uint8Array(5 * 1024 * 1024 + 1) }); + expect(ids).toEqual(["plugin.submission.asset.too_large", "plugin.submission.asset.too_large"]); + }); + + it.each([ + ["invalid XML", ""], + ["missing dimensions", ""], + ["units", svg('width="48px" height="48px"')], + ["percent", svg('width="100%" height="100%"')], + ["nonpositive", svg('width="0" height="48"'), "plugin.submission.asset.dimensions"], + ["rectangle", svg('width="48" height="49"'), "plugin.submission.asset.not_square"], + ["doctype", ''], + ["entity", ''], + ["external href", ''], + ["relative external href", ''], + ["external xlink", '', "plugin.submission.asset.unsafe_svg"] + ].map(([name, content, expected = "plugin.submission.asset.unsafe_svg"]) => [name, content, expected] as const))("rejects unsafe SVG %s", async (_name, content, expected) => { + const ids = await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": content }); + expect(ids).toContain(expected); + }); + + it("rejects invalid UTF-8 SVG", async () => { + const ids = await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": new Uint8Array([0xc3, 0x28]) }); + expect(ids).toContain("plugin.submission.asset.unsafe_svg"); + }); + + it("rejects a junction that canonically escapes the package", async () => { + const assetPackage = await packageWithAssets({ logo: "./outside/logo.png", composerIcon: "./icon.png" }, { "icon.png": png(48, 48) }); + const external = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-assets-escape-")); + await writeFile(path.join(external, "logo.png"), png(48, 48)); + await symlink(external, path.join(assetPackage.rootPath, "outside"), "junction"); + const findings = await validateSubmissionAssets(assetPackage); + expect(findings.findings.map((finding) => finding.id)).toContain("plugin.submission.asset.invalid_path"); + expect(JSON.stringify(findings)).not.toContain(external); + }); + + it("keeps findings package-relative and its validator offline", async () => { + const assetPackage = await packageWithAssets({ logo: "./secret.png", composerIcon: "./secret.png" }, { "secret.png": new Uint8Array([1]) }); + const result = await validateSubmissionAssets(assetPackage); + expect(JSON.stringify(result)).not.toContain(assetPackage.rootPath); + expect(JSON.stringify(result)).not.toContain("secret.png\u0000"); + expect(JSON.stringify(result)).toContain('"path":"secret.png"'); + const source = await readFile(new URL("../src/core/submission-assets.ts", import.meta.url), "utf8"); + expect(source).not.toMatch(/\b(fetch|exec|spawn|http|https)\b/u); + }); +}); diff --git a/tests/submission-preflight.test.ts b/tests/submission-preflight.test.ts index 337460a..4f3d2f8 100644 --- a/tests/submission-preflight.test.ts +++ b/tests/submission-preflight.test.ts @@ -16,23 +16,35 @@ const validManifest = { longDescription: "A local submission checker.\nIt remains offline.", developerName: "Example Developer", category: "Developer Tools", + logo: "./assets/logo.png", + composerIcon: "./assets/composer-icon.png", capabilities: ["Checks public listing metadata"], defaultPrompt: "Check my plugin submission" } }; -async function writePackage(manifest: unknown, files: Record = {}): Promise { +const validPng = new Uint8Array([ + 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, + 0, 0, 0, 48, 0, 0, 0, 48 +]); + +const validAssetFiles: Record = { + "assets/logo.png": validPng, + "assets/composer-icon.png": validPng +}; + +async function writePackage(manifest: unknown, files: Record = {}): Promise { const directory = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-")); const manifestDirectory = path.join(directory, ".codex-plugin"); await mkdir(manifestDirectory, { recursive: true }); await writeFile(path.join(manifestDirectory, "plugin.json"), JSON.stringify(manifest), "utf8"); - for (const [relativePath, content] of Object.entries(files)) { + for (const [relativePath, content] of Object.entries({ ...validAssetFiles, ...files })) { const filePath = path.join(directory, relativePath); await mkdir(path.dirname(filePath), { recursive: true }); - await writeFile(filePath, content, "utf8"); + await writeFile(filePath, content); } return directory; From a4f20007cccf2f2321bf45d5a087fbbc4a3c6b89 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 10:47:56 +0300 Subject: [PATCH 08/24] fix: reject truncated submission images --- src/core/submission-assets.ts | 44 +++++++++++++++++++++++--- tests/submission-assets.test.ts | 50 +++++++++++++++++++++++++----- tests/submission-preflight.test.ts | 36 ++++++++++++++++++--- 3 files changed, 114 insertions(+), 16 deletions(-) diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index d768b66..15ba760 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -19,6 +19,10 @@ type AssetFormat = "png" | "jpeg" | "webp" | "svg"; type Dimensions = { width: number; height: number }; type Evidence = SubmissionFinding["evidence"]; +export interface SubmissionAssetResult { + findings: SubmissionFinding[]; +} + function finding( id: `plugin.submission.asset.${string}`, message: string, @@ -36,14 +40,44 @@ function assetEvidence(field: string, packagePath?: string, format?: AssetFormat }; } +function crc32(buffer: Uint8Array): number { + let crc = 0xffffffff; + for (const byte of buffer) { + crc ^= byte; + for (let bit = 0; bit < 8; bit += 1) { + crc = (crc >>> 1) ^ (0xedb88320 & -(crc & 1)); + } + } + return (crc ^ 0xffffffff) >>> 0; +} + function readPng(buffer: Uint8Array): Dimensions | null { - if (buffer.length < 24 || ![137, 80, 78, 71, 13, 10, 26, 10].every((byte, index) => buffer[index] === byte) - || buffer[8] !== 0 || buffer[9] !== 0 || buffer[10] !== 0 || buffer[11] !== 13 - || buffer[12] !== 73 || buffer[13] !== 72 || buffer[14] !== 68 || buffer[15] !== 82) { + if (buffer.length < 33 || ![137, 80, 78, 71, 13, 10, 26, 10].every((byte, index) => buffer[index] === byte)) { return null; } const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); - return { width: view.getUint32(16), height: view.getUint32(20) }; + let dimensions: Dimensions | null = null; + let sawIdat = false; + let offset = 8; + for (let chunks = 0; chunks < 128 && offset + 12 <= buffer.length; chunks += 1) { + const length = view.getUint32(offset); + const typeOffset = offset + 4; + const dataOffset = offset + 8; + if (length > buffer.length - dataOffset - 4) return null; + const dataEnd = dataOffset + length; + if (crc32(buffer.slice(typeOffset, dataEnd)) !== view.getUint32(dataEnd)) return null; + const type = fourCc(buffer, typeOffset); + if (offset === 8) { + if (type !== "IHDR" || length !== 13) return null; + dimensions = { width: view.getUint32(dataOffset), height: view.getUint32(dataOffset + 4) }; + } else if (type === "IDAT") { + sawIdat ||= length > 0; + } else if (type === "IEND") { + return length === 0 && sawIdat && dataEnd + 4 === buffer.length ? dimensions : null; + } + offset = dataEnd + 4; + } + return null; } function readJpeg(buffer: Uint8Array): Dimensions | null { @@ -238,7 +272,7 @@ async function validateAsset(rootPath: string, field: typeof assetFields[number] return invalidDimensions === null ? [] : [invalidDimensions]; } -export async function validateSubmissionAssets(discoveredPackage: DiscoveredPackage): Promise<{ findings: SubmissionFinding[] }> { +export async function validateSubmissionAssets(discoveredPackage: DiscoveredPackage): Promise { const listing = discoveredPackage.manifest.interface; const interfaceValues = isRecord(listing) ? listing : {}; const result = await Promise.all(assetFields.map((field) => validateAsset(discoveredPackage.rootPath, field, interfaceValues[field]))); diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index c6bd5d8..8a21f08 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -1,19 +1,45 @@ import { mkdir, mkdtemp, readFile, symlink, writeFile } from "node:fs/promises"; +import { deflateSync } from "node:zlib"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; import type { DiscoveredPackage } from "../src/domain/types.js"; -import { validateSubmissionAssets } from "../src/core/submission-assets.js"; +import { type SubmissionAssetResult, validateSubmissionAssets } from "../src/core/submission-assets.js"; type AssetFiles = Record; +const crc32 = (data: Uint8Array) => { + let crc = 0xffffffff; + for (const byte of data) { + crc ^= byte; + for (let bit = 0; bit < 8; bit += 1) crc = (crc >>> 1) ^ (0xedb88320 & -(crc & 1)); + } + return (crc ^ 0xffffffff) >>> 0; +}; + +const pngChunk = (type: string, content: Uint8Array) => { + const chunk = new Uint8Array(content.length + 12); + const view = new DataView(chunk.buffer); + view.setUint32(0, content.length); + chunk.set(type.split("").map((character) => character.charCodeAt(0)), 4); + chunk.set(content, 8); + view.setUint32(content.length + 8, crc32(chunk.slice(4, content.length + 8))); + return chunk; +}; + const png = (width: number, height: number) => { - const data = new Uint8Array(24); - data.set([137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82]); - new DataView(data.buffer).setUint32(16, width); - new DataView(data.buffer).setUint32(20, height); - return data; + const header = new Uint8Array(13); + const view = new DataView(header.buffer); + view.setUint32(0, width); view.setUint32(4, height); + header.set([1, 0, 0, 0, 0], 8); + const scanlines = new Uint8Array((1 + Math.ceil(width / 8)) * height); + const idat = new Uint8Array(deflateSync(scanlines)); + const parts = [new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10]), pngChunk("IHDR", header), pngChunk("IDAT", idat), pngChunk("IEND", new Uint8Array())]; + const image = new Uint8Array(parts.reduce((size, part) => size + part.length, 0)); + let offset = 0; + for (const part of parts) { image.set(part, offset); offset += part.length; } + return image; }; const jpeg = (width: number, height: number) => { @@ -64,7 +90,8 @@ async function packageWithAssets(interfaceValues: Record, files } async function findingIds(interfaceValues: Record, files: AssetFiles = {}): Promise { - return (await validateSubmissionAssets(await packageWithAssets(interfaceValues, files))).findings.map((finding) => finding.id); + const result: SubmissionAssetResult = await validateSubmissionAssets(await packageWithAssets(interfaceValues, files)); + return result.findings.map((finding) => finding.id); } describe("submission assets", () => { @@ -85,6 +112,15 @@ describe("submission assets", () => { expect(await findingIds({}, {})).toEqual(["plugin.submission.asset.required", "plugin.submission.asset.required"]); }); + it("rejects a PNG whose declared IHDR is truncated", async () => { + const truncated = new Uint8Array(24); + truncated.set([137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82]); + new DataView(truncated.buffer).setUint32(16, 48); + new DataView(truncated.buffer).setUint32(20, 48); + expect(await findingIds({ logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": truncated })) + .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); + }); + it.each([undefined, "logo.png", "../logo.png", "./../logo.png", "/logo.png", 42])("rejects invalid asset paths", async (value) => { const ids = await findingIds({ logo: value, composerIcon: "./icon.png" }, { "icon.png": png(48, 48) }); expect(ids).toContain(value === undefined ? "plugin.submission.asset.required" : "plugin.submission.asset.invalid_path"); diff --git a/tests/submission-preflight.test.ts b/tests/submission-preflight.test.ts index 4f3d2f8..99e8b6c 100644 --- a/tests/submission-preflight.test.ts +++ b/tests/submission-preflight.test.ts @@ -1,6 +1,7 @@ import { mkdir, mkdtemp, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { deflateSync } from "node:zlib"; import { describe, expect, it, vi } from "vitest"; import { buildSubmissionPreflight } from "../src/core/submission-preflight.js"; @@ -23,10 +24,37 @@ const validManifest = { } }; -const validPng = new Uint8Array([ - 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, - 0, 0, 0, 48, 0, 0, 0, 48 -]); +const crc32 = (data: Uint8Array) => { + let crc = 0xffffffff; + for (const byte of data) { + crc ^= byte; + for (let bit = 0; bit < 8; bit += 1) crc = (crc >>> 1) ^ (0xedb88320 & -(crc & 1)); + } + return (crc ^ 0xffffffff) >>> 0; +}; + +const pngChunk = (type: string, content: Uint8Array) => { + const chunk = new Uint8Array(content.length + 12); + const view = new DataView(chunk.buffer); + view.setUint32(0, content.length); + chunk.set(type.split("").map((character) => character.charCodeAt(0)), 4); + chunk.set(content, 8); + view.setUint32(content.length + 8, crc32(chunk.slice(4, content.length + 8))); + return chunk; +}; + +const validPng = (() => { + const header = new Uint8Array(13); + const view = new DataView(header.buffer); + view.setUint32(0, 48); view.setUint32(4, 48); + header.set([1, 0, 0, 0, 0], 8); + const idat = new Uint8Array(deflateSync(new Uint8Array((1 + Math.ceil(48 / 8)) * 48))); + const parts = [new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10]), pngChunk("IHDR", header), pngChunk("IDAT", idat), pngChunk("IEND", new Uint8Array())]; + const image = new Uint8Array(parts.reduce((size, part) => size + part.length, 0)); + let offset = 0; + for (const part of parts) { image.set(part, offset); offset += part.length; } + return image; +})(); const validAssetFiles: Record = { "assets/logo.png": validPng, From 68fe7ae9bb4f0ef2a20e8a73a038375f8aebc9f4 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 11:03:14 +0300 Subject: [PATCH 09/24] fix: harden submission image decoding --- src/core/submission-assets.ts | 93 ++++++++++++++++++++++++++++++--- tests/submission-assets.test.ts | 70 +++++++++++++++++++++++-- 2 files changed, 151 insertions(+), 12 deletions(-) diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index 15ba760..88b5e30 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -1,5 +1,6 @@ import { readFile, stat } from "node:fs/promises"; import path from "node:path"; +import { inflateSync } from "node:zlib"; import { XMLParser, XMLValidator } from "fast-xml-parser"; @@ -51,13 +52,41 @@ function crc32(buffer: Uint8Array): number { return (crc ^ 0xffffffff) >>> 0; } +function pngScanlines(width: number, height: number, bitDepth: number, channels: number, interlace: number): number[] | null { + if (!Number.isInteger(width) || !Number.isInteger(height) || width < 1 || height < 1 + || width > maximumDimension || height > maximumDimension) { + return null; + } + const bytesPerRow = (pixels: number) => Math.ceil((pixels * bitDepth * channels) / 8); + if (interlace === 0) return Array.from({ length: height }, () => bytesPerRow(width)); + if (interlace !== 1) return null; + const passes = [[0, 0, 8, 8], [4, 0, 8, 8], [0, 4, 4, 8], [2, 0, 4, 4], [0, 2, 2, 4], [1, 0, 2, 2], [0, 1, 1, 2]] as const; + const rows: number[] = []; + for (const [left, top, horizontal, vertical] of passes) { + const passWidth = width <= left ? 0 : Math.ceil((width - left) / horizontal); + const passHeight = height <= top ? 0 : Math.ceil((height - top) / vertical); + for (let row = 0; row < passHeight; row += 1) rows.push(bytesPerRow(passWidth)); + } + return rows; +} + +function validPngEncoding(bitDepth: number, colorType: number, compression: number, filter: number, interlace: number): boolean { + const supportedBitDepths: Record = { + 0: [1, 2, 4, 8, 16], 2: [8, 16], 3: [1, 2, 4, 8], 4: [8, 16], 6: [8, 16] + }; + return compression === 0 && filter === 0 && (interlace === 0 || interlace === 1) + && supportedBitDepths[colorType]?.includes(bitDepth) === true; +} + function readPng(buffer: Uint8Array): Dimensions | null { if (buffer.length < 33 || ![137, 80, 78, 71, 13, 10, 26, 10].every((byte, index) => buffer[index] === byte)) { return null; } const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); let dimensions: Dimensions | null = null; - let sawIdat = false; + let scanlines: number[] | null = null; + const idatParts: Uint8Array[] = []; + let idatBytes = 0; let offset = 8; for (let chunks = 0; chunks < 128 && offset + 12 <= buffer.length; chunks += 1) { const length = view.getUint32(offset); @@ -70,10 +99,33 @@ function readPng(buffer: Uint8Array): Dimensions | null { if (offset === 8) { if (type !== "IHDR" || length !== 13) return null; dimensions = { width: view.getUint32(dataOffset), height: view.getUint32(dataOffset + 4) }; + const bitDepth = buffer[dataOffset + 8]; + const colorType = buffer[dataOffset + 9]; + if (!validPngEncoding(bitDepth, colorType, buffer[dataOffset + 10], buffer[dataOffset + 11], buffer[dataOffset + 12])) return null; + const channels = colorType === 0 || colorType === 3 ? 1 : colorType === 2 ? 3 : colorType === 4 ? 2 : 4; + scanlines = pngScanlines(dimensions.width, dimensions.height, bitDepth, channels, buffer[dataOffset + 12]); + if (scanlines === null) return null; } else if (type === "IDAT") { - sawIdat ||= length > 0; + idatParts.push(buffer.slice(dataOffset, dataEnd)); + idatBytes += length; } else if (type === "IEND") { - return length === 0 && sawIdat && dataEnd + 4 === buffer.length ? dimensions : null; + if (length !== 0 || dimensions === null || scanlines === null || idatBytes === 0 || dataEnd + 4 !== buffer.length) return null; + const expectedBytes = scanlines.reduce((total, rowBytes) => total + rowBytes + 1, 0); + const idat = new Uint8Array(idatBytes); + let idatOffset = 0; + for (const part of idatParts) { idat.set(part, idatOffset); idatOffset += part.length; } + try { + const output = inflateSync(idat, { maxOutputLength: expectedBytes }); + if (output.length !== expectedBytes) return null; + let outputOffset = 0; + for (const rowBytes of scanlines) { + if (output[outputOffset] > 4) return null; + outputOffset += rowBytes + 1; + } + return outputOffset === output.length ? dimensions : null; + } catch { + return null; + } } offset = dataEnd + 4; } @@ -85,6 +137,7 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { return null; } const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); + let dimensions: Dimensions | null = null; let offset = 2; for (let steps = 0; steps < 128 && offset < buffer.length; steps += 1) { while (offset < buffer.length && buffer[offset] === 0xff) { @@ -94,7 +147,7 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { return null; } const marker = buffer[offset++]; - if (marker === 0xd9 || marker === 0xda) { + if (marker === 0xd9) { return null; } if (marker === 0x01 || (marker >= 0xd0 && marker <= 0xd7)) { @@ -112,7 +165,27 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { if (start + 5 > offset + length) { return null; } - return { height: view.getUint16(start + 1), width: view.getUint16(start + 3) }; + dimensions = { height: view.getUint16(start + 1), width: view.getUint16(start + 3) }; + } + if (marker === 0xda) { + if (dimensions === null) return null; + const componentCount = buffer[offset + 2]; + if (componentCount < 1 || length !== 6 + (componentCount * 2)) return null; + offset += length; + for (let entropySteps = 0; entropySteps < buffer.length && offset < buffer.length; entropySteps += 1) { + if (buffer[offset] !== 0xff) { + offset += 1; + continue; + } + if (offset + 1 >= buffer.length) return null; + const entropyMarker = buffer[offset + 1]; + if (entropyMarker === 0x00 || (entropyMarker >= 0xd0 && entropyMarker <= 0xd7)) { + offset += 2; + continue; + } + return entropyMarker === 0xd9 && offset + 2 === buffer.length ? dimensions : null; + } + return null; } offset += length; } @@ -135,7 +208,8 @@ function readWebp(buffer: Uint8Array): Dimensions | null { const chunkType = fourCc(buffer, 12); const chunkLength = view.getUint32(16, true); const payload = 20; - if (chunkLength > buffer.length - payload) { + const paddedLength = chunkLength + (chunkLength % 2); + if (paddedLength !== buffer.length - payload) { return null; } if (chunkType === "VP8X") { @@ -178,9 +252,12 @@ function numberFrom(value: unknown): number | null { } function svgDimensions(content: string): Dimensions | null { - if (/ part === null)) return null; return { width: parts[2]!, height: parts[3]! }; } diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index 8a21f08..b7faae9 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -33,7 +33,7 @@ const png = (width: number, height: number) => { const view = new DataView(header.buffer); view.setUint32(0, width); view.setUint32(4, height); header.set([1, 0, 0, 0, 0], 8); - const scanlines = new Uint8Array((1 + Math.ceil(width / 8)) * height); + const scanlines = new Uint8Array(width > 4096 || height > 4096 ? 1 : (1 + Math.ceil(width / 8)) * height); const idat = new Uint8Array(deflateSync(scanlines)); const parts = [new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10]), pngChunk("IHDR", header), pngChunk("IDAT", idat), pngChunk("IEND", new Uint8Array())]; const image = new Uint8Array(parts.reduce((size, part) => size + part.length, 0)); @@ -42,8 +42,20 @@ const png = (width: number, height: number) => { return image; }; -const jpeg = (width: number, height: number) => { - const data = new Uint8Array(23); +const pngWithIdat = (idat: Uint8Array) => { + const header = new Uint8Array(13); + const view = new DataView(header.buffer); + view.setUint32(0, 48); view.setUint32(4, 48); + header.set([1, 0, 0, 0, 0], 8); + const parts = [new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10]), pngChunk("IHDR", header), pngChunk("IDAT", idat), pngChunk("IEND", new Uint8Array())]; + const image = new Uint8Array(parts.reduce((size, part) => size + part.length, 0)); + let offset = 0; + for (const part of parts) { image.set(part, offset); offset += part.length; } + return image; +}; + +const jpegSof = (width: number, height: number) => { + const data = new Uint8Array(21); data.set([0xff, 0xd8, 0xff, 0xc0, 0, 17, 8]); const view = new DataView(data.buffer); view.setUint16(7, height); @@ -52,6 +64,17 @@ const jpeg = (width: number, height: number) => { return data; }; +const jpegWithSos = (width: number, height: number, includeEoi: boolean) => { + const sof = jpegSof(width, height); + const data = new Uint8Array(sof.length + 15 + (includeEoi ? 2 : 0)); + data.set(sof); + data.set([0xff, 0xda, 0, 12, 3, 1, 0, 2, 0, 3, 0, 0, 63, 0, 0], sof.length); + if (includeEoi) data.set([0xff, 0xd9], data.length - 2); + return data; +}; + +const jpeg = (width: number, height: number) => jpegWithSos(width, height, true); + const webp = (variant: "VP8X" | "VP8L" | "VP8", width: number, height: number) => { const payload = variant === "VP8X" ? new Uint8Array(10) : variant === "VP8L" ? new Uint8Array(5) : new Uint8Array(10); const view = new DataView(payload.buffer); @@ -67,7 +90,7 @@ const webp = (variant: "VP8X" | "VP8L" | "VP8", width: number, height: number) = payload.set([0, 0, 0, 0x9d, 0x01, 0x2a]); view.setUint16(6, width, true); view.setUint16(8, height, true); } - const data = new Uint8Array(20 + payload.length); + const data = new Uint8Array(20 + payload.length + (payload.length % 2)); data.set([82, 73, 70, 70]); new DataView(data.buffer).setUint32(4, data.length - 8, true); data.set([87, 69, 66, 80, ...variant.padEnd(4, " ").split("").map((character) => character.charCodeAt(0))], 8); @@ -103,6 +126,7 @@ describe("submission assets", () => { ["WebP VP8", "./logo.webp", webp("VP8", 48, 48)], ["largest PNG", "./logo.png", png(4096, 4096)], ["SVG viewBox", "./logo.svg", svg('viewBox="0 0 48 48"')], + ["SVG comma viewBox", "./logo.svg", svg('viewBox="0,0,48,48"')], ["SVG dimensions", "./logo.svg", svg('width="48" height="48"')] ])("accepts a valid %s asset", async (_name, assetPath, content) => { expect(await findingIds({ logo: assetPath, composerIcon: assetPath }, { [assetPath.slice(2)]: content })).toEqual([]); @@ -121,6 +145,28 @@ describe("submission assets", () => { .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); }); + it("rejects a CRC-valid PNG with invalid IDAT zlib data", async () => { + expect(await findingIds({ logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": pngWithIdat(new Uint8Array([1, 2, 3])) })) + .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); + }); + + it("rejects an unpadded odd-length VP8L payload", async () => { + expect(await findingIds({ logo: "./logo.webp", composerIcon: "./logo.webp" }, { "logo.webp": webp("VP8L", 48, 48).slice(0, -1) })) + .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); + }); + + it("rejects a JPEG scan without terminal EOI", async () => { + expect(await findingIds({ logo: "./logo.jpg", composerIcon: "./logo.jpg" }, { "logo.jpg": jpegWithSos(48, 48, false) })) + .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); + }); + + it("rejects a JPEG with an invalid SOS header", async () => { + const malformed = jpegWithSos(48, 48, true); + malformed[25] = 0; + expect(await findingIds({ logo: "./logo.jpg", composerIcon: "./logo.jpg" }, { "logo.jpg": malformed })) + .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); + }); + it.each([undefined, "logo.png", "../logo.png", "./../logo.png", "/logo.png", 42])("rejects invalid asset paths", async (value) => { const ids = await findingIds({ logo: value, composerIcon: "./icon.png" }, { "icon.png": png(48, 48) }); expect(ids).toContain(value === undefined ? "plugin.submission.asset.required" : "plugin.submission.asset.invalid_path"); @@ -180,6 +226,22 @@ describe("submission assets", () => { expect(ids).toContain(expected); }); + it.each([ + ["CSS import", ''], + ["CSS url", ''], + ["inline CSS url", ''], + ["non-fragment CSS url", ''], + ["case and whitespace import", ''] + ])("rejects SVG %s remote CSS", async (_name, content) => { + expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": content })) + .toEqual(["plugin.submission.asset.unsafe_svg", "plugin.submission.asset.unsafe_svg"]); + }); + + it("allows fragment-only SVG CSS URLs", async () => { + expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": '' })) + .toEqual([]); + }); + it("rejects invalid UTF-8 SVG", async () => { const ids = await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": new Uint8Array([0xc3, 0x28]) }); expect(ids).toContain("plugin.submission.asset.unsafe_svg"); From fb0859d788d3ef89d61b791f89b5bce747bacbc6 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 11:10:48 +0300 Subject: [PATCH 10/24] fix: bound submission image resources --- src/core/submission-assets.ts | 65 ++++++++++++++++++++++++++++++--- tests/submission-assets.test.ts | 41 +++++++++++++++++++++ 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index 88b5e30..c79e95d 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -9,6 +9,7 @@ import { resolveSafePackagePath } from "./plugin-components.js"; import type { SubmissionFinding } from "./submission-preflight.js"; const maxAssetBytes = 5 * 1024 * 1024; +const MAX_DECODED_PNG_BYTES = 72 * 1024 * 1024; const minimumDimension = 48; const maximumDimension = 4096; const assetFields = ["logo", "composerIcon"] as const; @@ -111,6 +112,7 @@ function readPng(buffer: Uint8Array): Dimensions | null { } else if (type === "IEND") { if (length !== 0 || dimensions === null || scanlines === null || idatBytes === 0 || dataEnd + 4 !== buffer.length) return null; const expectedBytes = scanlines.reduce((total, rowBytes) => total + rowBytes + 1, 0); + if (expectedBytes > MAX_DECODED_PNG_BYTES) return null; const idat = new Uint8Array(idatBytes); let idatOffset = 0; for (const part of idatParts) { idat.set(part, idatOffset); idatOffset += part.length; } @@ -251,18 +253,68 @@ function numberFrom(value: unknown): number | null { return Number.isFinite(number) ? number : null; } +function normalizeCssEscapes(source: string): string | null { + if (source.length > maxAssetBytes) return null; + let normalized = ""; + for (let index = 0; index < source.length; index += 1) { + if (source[index] !== "\\") { + normalized += source[index]; + continue; + } + index += 1; + if (index >= source.length) return null; + if (/[0-9A-Fa-f]/u.test(source[index])) { + let digits = ""; + while (digits.length < 6 && index < source.length && /[0-9A-Fa-f]/u.test(source[index])) { + digits += source[index++]; + } + const codePoint = Number.parseInt(digits, 16); + if (codePoint === 0 || codePoint > 0x10ffff || (codePoint >= 0xd800 && codePoint <= 0xdfff)) return null; + normalized += String.fromCodePoint(codePoint); + if (source[index] === "\r" && source[index + 1] === "\n") index += 1; + else if (/[ \t\r\n\f]/u.test(source[index] ?? "")) index += 1; + index -= 1; + continue; + } + normalized += source[index]; + } + return normalized; +} + +function isSafeCss(source: string): boolean { + const css = normalizeCssEscapes(source); + if (css === null || /@\s*import\b/iu.test(css)) return false; + for (const match of css.matchAll(/url\s*\(\s*(?:(["'])(.*?)\1|([^)]*))\s*\)/giu)) { + if (!/^#[^\s]*$/u.test((match[2] ?? match[3] ?? "").trim())) return false; + } + return true; +} + +function inspectSvgValue(value: unknown, context: "css" | "href" | undefined = undefined): boolean { + if (typeof value === "string") { + if (context === "css") return isSafeCss(value); + return context !== "href" || value.trim() === "" || /^#[^\s]*$/u.test(value.trim()); + } + if (Array.isArray(value)) return value.every((item) => inspectSvgValue(item, context)); + if (!isRecord(value)) return true; + return Object.entries(value).every(([key, item]) => { + const childContext = context === "css" || key === "style" || key === "@_style" + ? "css" + : key === "@_href" || key === "@_xlink:href" ? "href" : undefined; + return inspectSvgValue(item, childContext); + }); +} + function svgDimensions(content: string): Dimensions | null { - if (/ { const listing = discoveredPackage.manifest.interface; const interfaceValues = isRecord(listing) ? listing : {}; - const result = await Promise.all(assetFields.map((field) => validateAsset(discoveredPackage.rootPath, field, interfaceValues[field]))); - return { findings: result.flat() }; + const findings: SubmissionFinding[] = []; + for (const field of assetFields) findings.push(...await validateAsset(discoveredPackage.rootPath, field, interfaceValues[field])); + return { findings }; } diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index b7faae9..2199d87 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -54,6 +54,18 @@ const pngWithIdat = (idat: Uint8Array) => { return image; }; +const pngWithHeader = (width: number, height: number, bitDepth: number, colorType: number, idat: Uint8Array) => { + const header = new Uint8Array(13); + const view = new DataView(header.buffer); + view.setUint32(0, width); view.setUint32(4, height); + header.set([bitDepth, colorType, 0, 0, 0], 8); + const parts = [new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10]), pngChunk("IHDR", header), pngChunk("IDAT", idat), pngChunk("IEND", new Uint8Array())]; + const image = new Uint8Array(parts.reduce((size, part) => size + part.length, 0)); + let offset = 0; + for (const part of parts) { image.set(part, offset); offset += part.length; } + return image; +}; + const jpegSof = (width: number, height: number) => { const data = new Uint8Array(21); data.set([0xff, 0xd8, 0xff, 0xc0, 0, 17, 8]); @@ -150,6 +162,12 @@ describe("submission assets", () => { .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); }); + it("rejects a PNG whose declared decoded raster exceeds 72 MiB before inflation", async () => { + const compressedByte = new Uint8Array(deflateSync(new Uint8Array([0]))); + expect(await findingIds({ logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": pngWithHeader(4096, 4096, 16, 6, compressedByte) })) + .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); + }); + it("rejects an unpadded odd-length VP8L payload", async () => { expect(await findingIds({ logo: "./logo.webp", composerIcon: "./logo.webp" }, { "logo.webp": webp("VP8L", 48, 48).slice(0, -1) })) .toEqual(["plugin.submission.asset.decode_failed", "plugin.submission.asset.decode_failed"]); @@ -237,11 +255,34 @@ describe("submission assets", () => { .toEqual(["plugin.submission.asset.unsafe_svg", "plugin.submission.asset.unsafe_svg"]); }); + it("rejects XML-character-reference and CSS-escaped remote imports", async () => { + const content = ''; + expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": content })) + .toEqual(["plugin.submission.asset.unsafe_svg", "plugin.submission.asset.unsafe_svg"]); + }); + + it("consumes CRLF after CSS hexadecimal escapes", async () => { + const content = ''; + expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": content })) + .toEqual(["plugin.submission.asset.unsafe_svg", "plugin.submission.asset.unsafe_svg"]); + }); + + it("rejects XML-character-reference href values after parsing", async () => { + const content = ''; + expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": content })) + .toEqual(["plugin.submission.asset.unsafe_svg", "plugin.submission.asset.unsafe_svg"]); + }); + it("allows fragment-only SVG CSS URLs", async () => { expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": '' })) .toEqual([]); }); + it("allows inline SVG CSS without a remote reference", async () => { + expect(await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": '' })) + .toEqual([]); + }); + it("rejects invalid UTF-8 SVG", async () => { const ids = await findingIds({ logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": new Uint8Array([0xc3, 0x28]) }); expect(ids).toContain("plugin.submission.asset.unsafe_svg"); From e753c3e0a0d17cff67c3a361b5755610970eb436 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 12:31:57 +0300 Subject: [PATCH 11/24] feat: validate submission skill metadata --- package-lock.json | 18 +- package.json | 3 +- src/core/submission-preflight.ts | 6 +- src/core/submission-skill-metadata.ts | 257 ++++++++++++++++++++++++ tests/submission-preflight.test.ts | 29 ++- tests/submission-skill-metadata.test.ts | 204 +++++++++++++++++++ 6 files changed, 510 insertions(+), 7 deletions(-) create mode 100644 src/core/submission-skill-metadata.ts create mode 100644 tests/submission-skill-metadata.test.ts diff --git a/package-lock.json b/package-lock.json index 64fae44..2f65ee2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,8 @@ "version": "1.58.0", "license": "MIT", "dependencies": { - "fast-xml-parser": "^5.11.0" + "fast-xml-parser": "^5.11.0", + "yaml": "^2.9.0" }, "bin": { "codex-plugin-doctor": "dist/cli.js" @@ -1802,6 +1803,21 @@ "engines": { "node": ">=16.0.0" } + }, + "node_modules/yaml": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.0.tgz", + "integrity": "sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==", + "license": "ISC", + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } } } } diff --git a/package.json b/package.json index 43c0aef..88c0035 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "vitest": "^3.2.4" }, "dependencies": { - "fast-xml-parser": "^5.11.0" + "fast-xml-parser": "^5.11.0", + "yaml": "^2.9.0" } } diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts index 81db784..4c756d3 100644 --- a/src/core/submission-preflight.ts +++ b/src/core/submission-preflight.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { discoverPackage } from "./discover-package.js"; import { validateSubmissionAssets } from "./submission-assets.js"; import { submissionManualChecks, submissionRuleset } from "./submission-ruleset.js"; +import { validateSubmissionSkillMetadata } from "./submission-skill-metadata.js"; export interface SubmissionFinding { id: `plugin.submission.${string}`; @@ -289,16 +290,17 @@ export async function buildSubmissionPreflight(targetPath: string): Promise item.severity === "fail").length; const manualChecklist = checklist(targetType); diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts new file mode 100644 index 0000000..f9160da --- /dev/null +++ b/src/core/submission-skill-metadata.ts @@ -0,0 +1,257 @@ +import { lstat, readdir, readFile, realpath, stat } from "node:fs/promises"; +import path from "node:path"; + +import { isAlias, isNode, parseDocument, visit } from "yaml"; + +import type { DiscoveredPackage } from "../domain/types.js"; +import type { SubmissionFinding } from "./submission-preflight.js"; + +type TargetType = "skills-only" | "mcp-backed"; +type Evidence = SubmissionFinding["evidence"]; +type Metadata = Record; + +const maxSkillBytes = 1024 * 1024; +const maxAgentBytes = 256 * 1024; +const unsupportedText = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u202E\u2060-\u206F\uFEFF]/u; +const interfaceKeys = new Set(["display_name", "short_description"]); +const policyKeys = new Set(["products", "allow_implicit_invocation"]); +const dependencyKeys = new Set(["tools"]); +const agentKeys = new Set(["interface", "icon_small", "icon_large", "brand_color", "default_prompt", "policy", "dependencies"]); + +export interface SubmissionSkillMetadataResult { + findings: SubmissionFinding[]; + skillCount: number; +} + +function isRecord(value: unknown): value is Metadata { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function isWithin(rootPath: string, candidatePath: string): boolean { + const relative = path.relative(rootPath, candidatePath); + return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); +} + +function packagePath(rootPath: string, targetPath: string): string { + return path.relative(rootPath, targetPath).split(path.sep).join("/"); +} + +function finding( + id: `plugin.submission.skill.${string}`, + message: string, + evidence?: Evidence +): SubmissionFinding { + return evidence === undefined + ? { id, severity: "fail", message } + : { id, severity: "fail", message, evidence }; +} + +function supportedText(value: unknown, limit = Number.MAX_SAFE_INTEGER): value is string { + return typeof value === "string" && value.trim().length > 0 && value.length <= limit && !unsupportedText.test(value); +} + +function parseSafeYaml(source: string): { value: Metadata } | { error: "yaml" | "shape" } { + const document = parseDocument(source, { schema: "core", uniqueKeys: true, strict: true, prettyErrors: false }); + let alias = false; + let nonCoreTag = false; + visit(document, (_key, node) => { + if (isAlias(node)) alias = true; + if (isNode(node) && node.tag !== undefined && !node.tag.startsWith("tag:yaml.org,2002:")) nonCoreTag = true; + }); + if (document.errors.length > 0 || alias || nonCoreTag) return { error: "yaml" }; + try { + const value = document.toJS({ maxAliasCount: 0 }); + return isRecord(value) ? { value } : { error: "shape" }; + } catch { + return { error: "yaml" }; + } +} + +function splitSkillFile(source: string): { frontmatter: string; body: string } | null { + const match = /^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)([\s\S]*)$/u.exec(source); + if (!match || match[2].trim().length === 0) return null; + return { frontmatter: match[1], body: match[2] }; +} + +async function safeDirectory(rootPath: string, skillsPath: string): Promise<{ canonicalRoot: string; canonicalSkills: string } | null> { + try { + const [canonicalRoot, canonicalSkills, details] = await Promise.all([realpath(rootPath), realpath(skillsPath), stat(skillsPath)]); + return details.isDirectory() && isWithin(canonicalRoot, canonicalSkills) ? { canonicalRoot, canonicalSkills } : null; + } catch { + return null; + } +} + +async function readSafeUtf8(filePath: string, maximum: number): Promise { + try { + const details = await stat(filePath); + if (!details.isFile() || details.size > maximum) return null; + return new TextDecoder("utf-8", { fatal: true }).decode(await readFile(filePath)); + } catch { + return null; + } +} + +function rejectUnknownKeys(value: Metadata, allowed: Set): boolean { + return Object.keys(value).some((key) => !allowed.has(key)); +} + +async function validateAgentFile( + rootPath: string, + skillRoot: string, + skillPath: string +): Promise { + const agentPath = path.join(skillRoot, "agents", "openai.yaml"); + try { + await lstat(agentPath); + } catch { + return []; + } + + let canonicalSkill: string; + let canonicalAgent: string; + try { + [canonicalSkill, canonicalAgent] = await Promise.all([realpath(skillRoot), realpath(agentPath)]); + } catch { + return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a readable regular file.", { path: packagePath(rootPath, agentPath) })]; + } + if (!isWithin(canonicalSkill, canonicalAgent)) { + return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; + } + + const source = await readSafeUtf8(agentPath, maxAgentBytes); + if (source === null) { + return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a bounded UTF-8 regular file.", { path: packagePath(rootPath, agentPath), limit: maxAgentBytes })]; + } + const parsed = parseSafeYaml(source); + if ("error" in parsed) { + return [finding( + parsed.error === "yaml" ? "plugin.submission.skill.agent.invalid_yaml" : "plugin.submission.skill.agent.invalid_shape", + "Optional agent metadata must be a safe YAML mapping.", + { path: packagePath(rootPath, agentPath) } + )]; + } + + const metadata = parsed.value; + if (rejectUnknownKeys(metadata, agentKeys) || !isRecord(metadata.interface) || rejectUnknownKeys(metadata.interface, interfaceKeys) + || !supportedText(metadata.interface.display_name) || !supportedText(metadata.interface.short_description)) { + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an unsupported shape.", { path: packagePath(rootPath, agentPath) })]; + } + if (metadata.brand_color !== undefined && (typeof metadata.brand_color !== "string" || !/^#[0-9A-Fa-f]{6}$/u.test(metadata.brand_color))) { + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an invalid brand color.", { path: packagePath(rootPath, agentPath), field: "brand_color" })]; + } + if (metadata.default_prompt !== undefined && !supportedText(metadata.default_prompt)) { + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an invalid default prompt.", { path: packagePath(rootPath, agentPath), field: "default_prompt" })]; + } + if (metadata.policy !== undefined && (!isRecord(metadata.policy) || rejectUnknownKeys(metadata.policy, policyKeys) + || !Array.isArray(metadata.policy.products) || metadata.policy.products.length === 0 + || new Set(metadata.policy.products).size !== metadata.policy.products.length + || metadata.policy.products.some((product) => product !== "CHAT" && product !== "CODEX") + || typeof metadata.policy.allow_implicit_invocation !== "boolean")) { + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent policy has an unsupported shape.", { path: packagePath(rootPath, agentPath), field: "policy" })]; + } + if (metadata.dependencies !== undefined && (!isRecord(metadata.dependencies) || rejectUnknownKeys(metadata.dependencies, dependencyKeys) + || !Array.isArray(metadata.dependencies.tools) || metadata.dependencies.tools.length === 0 + || new Set(metadata.dependencies.tools).size !== metadata.dependencies.tools.length + || metadata.dependencies.tools.some((tool) => !supportedText(tool)))) { + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent dependencies have an unsupported shape.", { path: packagePath(rootPath, agentPath), field: "dependencies" })]; + } + + for (const field of ["icon_small", "icon_large"] as const) { + const value = metadata[field]; + if (value === undefined) continue; + if (typeof value !== "string" || !value.startsWith("./")) { + return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field })]; + } + const iconPath = path.resolve(skillRoot, value); + try { + const [canonicalIcon, details] = await Promise.all([realpath(iconPath), stat(iconPath)]); + if (!details.isFile() || !isWithin(canonicalSkill, canonicalIcon)) throw new Error("unsafe icon"); + } catch { + return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field })]; + } + } + return []; +} + +export async function validateSubmissionSkillMetadata( + discoveredPackage: DiscoveredPackage, + targetType: TargetType +): Promise { + const { manifest, rootPath } = discoveredPackage; + if (manifest.skills === undefined) { + return targetType === "skills-only" + ? { findings: [finding("plugin.submission.skill.required", "Skills-only submissions require a valid skill.")], skillCount: 0 } + : { findings: [], skillCount: 0 }; + } + if (manifest.skills !== "./skills" && manifest.skills !== "./skills/") { + return { findings: [finding("plugin.submission.skill.invalid_manifest", "Skills must be declared as the root ./skills directory.", { field: "skills" })], skillCount: 0 }; + } + + const skillsPath = path.resolve(rootPath, manifest.skills); + const safe = await safeDirectory(rootPath, skillsPath); + if (!safe) { + return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory must be canonically contained in the package.", { path: "skills" })], skillCount: 0 }; + } + + let entries; + try { + entries = await readdir(skillsPath, { withFileTypes: true }); + } catch { + return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory cannot be inspected safely.", { path: "skills" })], skillCount: 0 }; + } + const findings: SubmissionFinding[] = []; + const identities = new Set(); + let skillCount = 0; + for (const entry of entries.filter((candidate) => !candidate.name.startsWith(".") && candidate.isDirectory())) { + const skillRoot = path.join(skillsPath, entry.name); + const skillFile = path.join(skillRoot, "SKILL.md"); + const relativeSkillPath = packagePath(rootPath, skillFile); + let canonicalSkill: string; + try { + canonicalSkill = await realpath(skillRoot); + if (!isWithin(safe.canonicalSkills, canonicalSkill)) throw new Error("unsafe skill root"); + } catch { + findings.push(finding("plugin.submission.skill.invalid_path", "Skill directory resolves outside the declared skills directory.", { path: packagePath(rootPath, skillRoot) })); + continue; + } + const source = await readSafeUtf8(skillFile, maxSkillBytes); + if (source === null) { + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a bounded UTF-8 regular file.", { path: relativeSkillPath, limit: maxSkillBytes })); + continue; + } + const split = splitSkillFile(source); + if (!split) { + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint requires delimited frontmatter and a nonempty body.", { path: relativeSkillPath })); + continue; + } + const parsed = parseSafeYaml(split.frontmatter); + if ("error" in parsed) { + findings.push(finding( + parsed.error === "yaml" ? "plugin.submission.skill.invalid_yaml" : "plugin.submission.skill.invalid_shape", + "Skill frontmatter must be a safe YAML mapping.", + { path: relativeSkillPath } + )); + continue; + } + const name = parsed.value.name; + const description = parsed.value.description; + const normalizedName = typeof name === "string" ? name.normalize("NFKC").trim() : ""; + const pluginName = typeof manifest.name === "string" ? manifest.name.normalize("NFKC").trim() : ""; + if (!supportedText(name) || !supportedText(description, 1024) || normalizedName.length === 0 + || `${pluginName}:${normalizedName}`.length > 64 || identities.has(normalizedName)) { + findings.push(finding("plugin.submission.skill.identity", "Skill identity metadata is invalid or duplicated.", { path: relativeSkillPath, limit: 64 })); + continue; + } + identities.add(normalizedName); + skillCount += 1; + findings.push(...await validateAgentFile(rootPath, skillRoot, packagePath(rootPath, skillRoot))); + } + if (targetType === "skills-only" && skillCount === 0 && !findings.some((item) => item.id === "plugin.submission.skill.required")) { + findings.push(finding("plugin.submission.skill.required", "Skills-only submissions require at least one valid skill.", { count: 0 })); + } + if (targetType === "mcp-backed" && skillCount === 0 && findings.length === 0) { + findings.push(finding("plugin.submission.skill.invalid_file", "Declared skills must include at least one valid immediate skill entrypoint.", { path: "skills" })); + } + return { findings, skillCount }; +} diff --git a/tests/submission-preflight.test.ts b/tests/submission-preflight.test.ts index 99e8b6c..7048a1e 100644 --- a/tests/submission-preflight.test.ts +++ b/tests/submission-preflight.test.ts @@ -60,15 +60,22 @@ const validAssetFiles: Record = { "assets/logo.png": validPng, "assets/composer-icon.png": validPng }; +const validSkillFiles: Record = { + "skills/check/SKILL.md": "---\nname: check\ndescription: Check a plugin submission\n---\n\nCheck the plugin submission.\n" +}; -async function writePackage(manifest: unknown, files: Record = {}): Promise { +async function writePackage( + manifest: unknown, + files: Record = {}, + includeSkill = true +): Promise { const directory = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-")); const manifestDirectory = path.join(directory, ".codex-plugin"); await mkdir(manifestDirectory, { recursive: true }); await writeFile(path.join(manifestDirectory, "plugin.json"), JSON.stringify(manifest), "utf8"); - for (const [relativePath, content] of Object.entries({ ...validAssetFiles, ...files })) { + for (const [relativePath, content] of Object.entries({ ...validAssetFiles, ...(includeSkill ? validSkillFiles : {}), ...files })) { const filePath = path.join(directory, relativePath); await mkdir(path.dirname(filePath), { recursive: true }); @@ -174,6 +181,22 @@ describe("submission preflight", () => { expect(fetchSpy).not.toHaveBeenCalled(); }); + it("integrates bounded skill metadata into the skills check without requiring skills for MCP-only packages", async () => { + const missingSkill = await buildSubmissionPreflight(await writePackage(validManifest, {}, false)); + const mcpOnly = await buildSubmissionPreflight(await writePackage({ + ...validMcpManifest(), + skills: undefined + }, {}, false)); + + expect(missingSkill.checks.find((item) => item.id === "skills")).toMatchObject({ + status: "fail", + findingIds: expect.arrayContaining(["plugin.submission.skill.invalid_path"]) + }); + expect(missingSkill).toMatchObject({ status: "fail", readiness: "blocked" }); + expect(mcpOnly.checks.find((item) => item.id === "skills")).toEqual({ id: "skills", status: "pass", findingIds: [] }); + expect(mcpOnly).toMatchObject({ status: "pass", readiness: "manual_review_required" }); + }); + it("blocks missing and malformed manifests without exposing the package path", async () => { const missing = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-missing-")); const malformed = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-malformed-")); @@ -204,7 +227,7 @@ describe("submission preflight", () => { }); it.each([ - ["package name", { ...validManifest, name: "a".repeat(64) }, false], + ["package name", { ...validMcpManifest({ skills: undefined }), name: "a".repeat(64) }, false], ["package name one over", { ...validManifest, name: "a".repeat(65) }, true], ["version", { ...validManifest, version: "1.2.3+" + "a".repeat(58) }, false], ["version one over", { ...validManifest, version: "1.2.3+" + "a".repeat(59) }, true], diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts new file mode 100644 index 0000000..187702c --- /dev/null +++ b/tests/submission-skill-metadata.test.ts @@ -0,0 +1,204 @@ +import { mkdir, mkdtemp, readFile, symlink, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; + +import { validateSubmissionSkillMetadata } from "../src/core/submission-skill-metadata.js"; + +const skill = (name = "check", description = "Checks plugin metadata") => `---\nname: ${name}\ndescription: ${description}\n---\n\nUse the checker.\n`; +const agent = `interface:\n display_name: Check\n short_description: Check plugin metadata\n`; + +async function packageWith( + skills: unknown, + files: Record = {}, + name = "submission-plugin" +) { + const rootPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-skills-")); + const manifestPath = path.join(rootPath, ".codex-plugin", "plugin.json"); + await mkdir(path.dirname(manifestPath), { recursive: true }); + await writeFile(manifestPath, JSON.stringify({ name, skills }), "utf8"); + for (const [relativePath, contents] of Object.entries(files)) { + const target = path.join(rootPath, relativePath); + await mkdir(path.dirname(target), { recursive: true }); + await writeFile(target, contents); + } + return { rootPath, manifestPath, manifest: { name, skills } }; +} + +function ids(result: Awaited>) { + return result.findings.map((finding) => finding.id); +} + +describe("submission skill metadata", () => { + it("requires a declared valid skill for skills-only targets", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith(undefined), "skills-only"); + + expect(ids(result)).toEqual(["plugin.submission.skill.required"]); + expect(result.skillCount).toBe(0); + }); + + it("allows an MCP-backed target with no skills declaration", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith(undefined), "mcp-backed"); + + expect(result).toEqual({ findings: [], skillCount: 0 }); + }); + + it.each(["skills", "./skills/child", "../skills", [], null])("rejects invalid skills declaration %j", async (skills) => { + const result = await validateSubmissionSkillMetadata(await packageWith(skills), "mcp-backed"); + + expect(ids(result)).toContain("plugin.submission.skill.invalid_manifest"); + }); + + it("accepts an immediate skill with no optional agent metadata", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill() + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + + it("inspects only non-hidden immediate skill directories", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/.ignored/SKILL.md": "not valid frontmatter" + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + + it("rejects a visible immediate skill directory without a regular entrypoint", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/.keep": "" + }), "skills-only"); + + expect(ids(result)).toEqual(expect.arrayContaining([ + "plugin.submission.skill.invalid_file", + "plugin.submission.skill.required" + ])); + }); + + it("accepts complete safe agent metadata", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills/", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `${agent}icon_small: ./icon.png\nicon_large: ./icon.png\nbrand_color: \"#123ABC\"\ndefault_prompt: Check this plugin\npolicy:\n products: [CHAT, CODEX]\n allow_implicit_invocation: true\ndependencies:\n tools: [read_file]\n`, + "skills/check/icon.png": "not-inspected-as-image" + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + + it.each([ + ["missing frontmatter", "name: check\ndescription: text\n", "plugin.submission.skill.invalid_file"], + ["frontmatter root array", "---\n- name: check\n---\nbody\n", "plugin.submission.skill.invalid_shape"], + ["alias", "---\nname: &name check\ndescription: *name\n---\nbody\n", "plugin.submission.skill.invalid_yaml"], + ["custom tag", "---\nname: !custom check\ndescription: text\n---\nbody\n", "plugin.submission.skill.invalid_yaml"], + ["duplicate key", "---\nname: check\nname: again\ndescription: text\n---\nbody\n", "plugin.submission.skill.invalid_yaml"], + ["blank metadata", "---\nname: \" \"\ndescription: \" \"\n---\nbody\n", "plugin.submission.skill.identity"], + ["missing body", "---\nname: check\ndescription: text\n---\n", "plugin.submission.skill.invalid_file"] + ] as const)("rejects %s", async (_caseName, contents, expected) => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": contents + }), "skills-only"); + + expect(ids(result)).toContain(expected); + }); + + it("rejects invalid UTF-8 and oversized skill files", async () => { + const invalidUtf8 = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": new Uint8Array([0xff, 0xfe]) + }), "skills-only"); + const oversized = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": `${skill()}${"x".repeat(1024 * 1024)}` + }), "skills-only"); + + expect(ids(invalidUtf8)).toContain("plugin.submission.skill.invalid_file"); + expect(ids(oversized)).toContain("plugin.submission.skill.invalid_file"); + }); + + it("uses normalized unique plugin and skill identities at the 64 character boundary", async () => { + const packageName = "p".repeat(58); + const boundary = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill("c".repeat(5)) + }, packageName), "skills-only"); + const duplicate = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/one/SKILL.md": skill("café"), + "skills/two/SKILL.md": skill("cafe\u0301") + }), "skills-only"); + const oneOver = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill("c".repeat(6)) + }, packageName), "skills-only"); + + expect(boundary.findings).toEqual([]); + expect(ids(duplicate)).toContain("plugin.submission.skill.identity"); + expect(ids(oneOver)).toContain("plugin.submission.skill.identity"); + }); + + it.each([ + ["missing interface", "policy: { products: [CHAT], allow_implicit_invocation: true }\ndependencies: { tools: [read_file] }\n", "plugin.submission.skill.agent.invalid_shape"], + ["wrong interface", "interface: []\n", "plugin.submission.skill.agent.invalid_shape"], + ["malformed yaml", "interface: [\n", "plugin.submission.skill.agent.invalid_yaml"], + ["alias", "interface: &meta { display_name: Check, short_description: Check }\npolicy: *meta\n", "plugin.submission.skill.agent.invalid_yaml"], + ["custom tag", "interface: !custom { display_name: Check, short_description: Check }\n", "plugin.submission.skill.agent.invalid_yaml"], + ["unsupported key", `${agent}unexpected: value\n`, "plugin.submission.skill.agent.invalid_shape"], + ["policy shape", `${agent}policy: { products: [CHAT, CHAT], allow_implicit_invocation: yes }\n`, "plugin.submission.skill.agent.invalid_shape"], + ["dependency shape", `${agent}dependencies: { tools: [read_file, \"\"] }\n`, "plugin.submission.skill.agent.invalid_shape"] + ] as const)("rejects agent metadata with %s", async (_caseName, contents, expected) => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": contents + }), "skills-only"); + + expect(ids(result)).toContain(expected); + }); + + it("rejects oversized and invalid UTF-8 agent metadata", async () => { + const invalidUtf8 = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": new Uint8Array([0xff, 0xfe]) + }), "skills-only"); + const oversized = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `${agent}${"x".repeat(256 * 1024)}` + }), "skills-only"); + + expect(ids(invalidUtf8)).toContain("plugin.submission.skill.agent.invalid_file"); + expect(ids(oversized)).toContain("plugin.submission.skill.agent.invalid_file"); + }); + + it("rejects agent file paths that escape a skill and does not expose their contents", async () => { + const sentinel = "agent-secret-sentinel"; + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `${agent}icon_small: ../${sentinel}.png\n` + }), "skills-only"); + + expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_path"); + expect(JSON.stringify(result)).not.toContain(sentinel); + }); + + it("rejects a junction that makes the skills directory escape the package", async () => { + const discovered = await packageWith("./skills"); + const external = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-skills-escape-")); + await mkdir(path.join(external, "check")); + await writeFile(path.join(external, "check", "SKILL.md"), skill(), "utf8"); + await symlink(external, path.join(discovered.rootPath, "skills"), "junction"); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + + expect(ids(result)).toContain("plugin.submission.skill.invalid_path"); + expect(JSON.stringify(result)).not.toContain(external); + }); + + it("does not fetch, spawn processes, or expose untrusted skill content", async () => { + const sentinel = "skill-secret-sentinel"; + const fetchSpy = vi.spyOn(globalThis, "fetch"); + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": `---\nname: check\ndescription: ${sentinel}\n---\nbody\n` + }), "skills-only"); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(JSON.stringify(result)).not.toContain(sentinel); + expect(await readFile(new URL("../src/core/submission-skill-metadata.ts", import.meta.url), "utf8")) + .not.toMatch(/child_process|node:child_process/u); + }); +}); From 0fa89dca81b73498ae97dae07bed671d43301fb6 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 12:39:39 +0300 Subject: [PATCH 12/24] fix: align submission skill metadata schema --- src/core/submission-skill-metadata.ts | 32 ++++++++--- tests/submission-skill-metadata.test.ts | 73 ++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index f9160da..62a1d09 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -13,10 +13,10 @@ type Metadata = Record; const maxSkillBytes = 1024 * 1024; const maxAgentBytes = 256 * 1024; const unsupportedText = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u202E\u2060-\u206F\uFEFF]/u; -const interfaceKeys = new Set(["display_name", "short_description"]); +const interfaceKeys = new Set(["display_name", "short_description", "icon_small", "icon_large", "brand_color", "default_prompt"]); const policyKeys = new Set(["products", "allow_implicit_invocation"]); const dependencyKeys = new Set(["tools"]); -const agentKeys = new Set(["interface", "icon_small", "icon_large", "brand_color", "default_prompt", "policy", "dependencies"]); +const agentKeys = new Set(["interface", "policy", "dependencies"]); export interface SubmissionSkillMetadataResult { findings: SubmissionFinding[]; @@ -137,17 +137,17 @@ async function validateAgentFile( || !supportedText(metadata.interface.display_name) || !supportedText(metadata.interface.short_description)) { return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an unsupported shape.", { path: packagePath(rootPath, agentPath) })]; } - if (metadata.brand_color !== undefined && (typeof metadata.brand_color !== "string" || !/^#[0-9A-Fa-f]{6}$/u.test(metadata.brand_color))) { + if (metadata.interface.brand_color !== undefined && (typeof metadata.interface.brand_color !== "string" || !/^#[0-9A-Fa-f]{6}$/u.test(metadata.interface.brand_color))) { return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an invalid brand color.", { path: packagePath(rootPath, agentPath), field: "brand_color" })]; } - if (metadata.default_prompt !== undefined && !supportedText(metadata.default_prompt)) { + if (metadata.interface.default_prompt !== undefined && !supportedText(metadata.interface.default_prompt)) { return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an invalid default prompt.", { path: packagePath(rootPath, agentPath), field: "default_prompt" })]; } if (metadata.policy !== undefined && (!isRecord(metadata.policy) || rejectUnknownKeys(metadata.policy, policyKeys) - || !Array.isArray(metadata.policy.products) || metadata.policy.products.length === 0 - || new Set(metadata.policy.products).size !== metadata.policy.products.length - || metadata.policy.products.some((product) => product !== "CHAT" && product !== "CODEX") - || typeof metadata.policy.allow_implicit_invocation !== "boolean")) { + || (metadata.policy.products !== undefined && (!Array.isArray(metadata.policy.products) || metadata.policy.products.length === 0 + || new Set(metadata.policy.products).size !== metadata.policy.products.length + || metadata.policy.products.some((product) => product !== "CHAT" && product !== "CODEX"))) + || (metadata.policy.allow_implicit_invocation !== undefined && typeof metadata.policy.allow_implicit_invocation !== "boolean"))) { return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent policy has an unsupported shape.", { path: packagePath(rootPath, agentPath), field: "policy" })]; } if (metadata.dependencies !== undefined && (!isRecord(metadata.dependencies) || rejectUnknownKeys(metadata.dependencies, dependencyKeys) @@ -158,7 +158,7 @@ async function validateAgentFile( } for (const field of ["icon_small", "icon_large"] as const) { - const value = metadata[field]; + const value = metadata.interface[field]; if (value === undefined) continue; if (typeof value !== "string" || !value.startsWith("./")) { return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field })]; @@ -215,6 +215,20 @@ export async function validateSubmissionSkillMetadata( findings.push(finding("plugin.submission.skill.invalid_path", "Skill directory resolves outside the declared skills directory.", { path: packagePath(rootPath, skillRoot) })); continue; } + try { + const details = await lstat(skillFile); + if (details.isSymbolicLink()) { + findings.push(finding("plugin.submission.skill.invalid_path", "Skill entrypoint must not be a symbolic link.", { path: relativeSkillPath })); + continue; + } + if (!details.isFile() || !isWithin(canonicalSkill, await realpath(skillFile))) { + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a contained regular file.", { path: relativeSkillPath })); + continue; + } + } catch { + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a contained regular file.", { path: relativeSkillPath })); + continue; + } const source = await readSafeUtf8(skillFile, maxSkillBytes); if (source === null) { findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a bounded UTF-8 regular file.", { path: relativeSkillPath, limit: maxSkillBytes })); diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 187702c..239f420 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, readFile, symlink, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, symlink, unlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; @@ -77,13 +77,23 @@ describe("submission skill metadata", () => { ])); }); - it("accepts complete safe agent metadata", async () => { + it("rejects agent fields at the top level", async () => { const result = await validateSubmissionSkillMetadata(await packageWith("./skills/", { "skills/check/SKILL.md": skill(), "skills/check/agents/openai.yaml": `${agent}icon_small: ./icon.png\nicon_large: ./icon.png\nbrand_color: \"#123ABC\"\ndefault_prompt: Check this plugin\npolicy:\n products: [CHAT, CODEX]\n allow_implicit_invocation: true\ndependencies:\n tools: [read_file]\n`, "skills/check/icon.png": "not-inspected-as-image" }), "skills-only"); + expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_shape"); + }); + + it("accepts complete safe agent metadata inside interface", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills/", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: ./icon.png\n icon_large: ./icon.png\n brand_color: \"#123ABC\"\n default_prompt: Check this plugin\npolicy:\n products: [CHAT, CODEX]\n allow_implicit_invocation: true\ndependencies:\n tools: [read_file]\n`, + "skills/check/icon.png": "not-inspected-as-image" + }), "skills-only"); + expect(result).toEqual({ findings: [], skillCount: 1 }); }); @@ -151,6 +161,19 @@ describe("submission skill metadata", () => { expect(ids(result)).toContain(expected); }); + it.each([ + "policy: { products: [CHAT] }\n", + "policy: { allow_implicit_invocation: false }\n", + "policy: {}\n" + ])("accepts partial policy metadata %j", async (policy) => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `${agent}${policy}` + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + it("rejects oversized and invalid UTF-8 agent metadata", async () => { const invalidUtf8 = await validateSubmissionSkillMetadata(await packageWith("./skills", { "skills/check/SKILL.md": skill(), @@ -169,7 +192,7 @@ describe("submission skill metadata", () => { const sentinel = "agent-secret-sentinel"; const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { "skills/check/SKILL.md": skill(), - "skills/check/agents/openai.yaml": `${agent}icon_small: ../${sentinel}.png\n` + "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: ../${sentinel}.png\n` }), "skills-only"); expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_path"); @@ -189,6 +212,50 @@ describe("submission skill metadata", () => { expect(JSON.stringify(result)).not.toContain(external); }); + if (process.platform !== "win32") { + it("rejects a SKILL.md symlink that escapes its skill without reading external content", async () => { + const sentinel = "external-skill-secret"; + const discovered = await packageWith("./skills", { "skills/check/SKILL.md": skill() }); + const external = path.join(os.tmpdir(), `codex-plugin-doctor-external-${Date.now()}.md`); + const skillFile = path.join(discovered.rootPath, "skills", "check", "SKILL.md"); + await writeFile(external, `---\nname: check\ndescription: ${sentinel}\n---\nbody\n`, "utf8"); + await unlink(skillFile); + await symlink(external, skillFile, "file"); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + + expect(ids(result)).toEqual(expect.arrayContaining([ + "plugin.submission.skill.invalid_path", + "plugin.submission.skill.required" + ])); + expect(JSON.stringify(result)).not.toContain(sentinel); + expect(JSON.stringify(result)).not.toContain(external); + }); + } + + if (process.platform === "win32") { + it("rejects a SKILL.md junction without following its reparse target", async () => { + const sentinel = "external-junction-secret"; + const discovered = await packageWith("./skills", { "skills/check/SKILL.md": skill() }); + const external = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-external-skill-junction-")); + const skillFile = path.join(discovered.rootPath, "skills", "check", "SKILL.md"); + await writeFile(path.join(external, "secret.md"), sentinel, "utf8"); + await unlink(skillFile); + await symlink(external, skillFile, "junction"); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + + expect(ids(result)).toEqual(expect.arrayContaining([ + "plugin.submission.skill.required" + ])); + expect(ids(result)).toEqual(expect.arrayContaining([ + expect.stringMatching(/^plugin\.submission\.skill\.invalid_(path|file)$/u) + ])); + expect(JSON.stringify(result)).not.toContain(sentinel); + expect(JSON.stringify(result)).not.toContain(external); + }); + } + it("does not fetch, spawn processes, or expose untrusted skill content", async () => { const sentinel = "skill-secret-sentinel"; const fetchSpy = vi.spyOn(globalThis, "fetch"); From 0ea08c3cccd3b423f62557ff34f26d569ac900f5 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 12:54:18 +0300 Subject: [PATCH 13/24] fix: support official skill tool descriptors --- src/core/submission-skill-metadata.ts | 64 +++++++++++++----- tests/submission-skill-metadata.test.ts | 89 ++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 17 deletions(-) diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index 62a1d09..5f648ad 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -16,6 +16,7 @@ const unsupportedText = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u2 const interfaceKeys = new Set(["display_name", "short_description", "icon_small", "icon_large", "brand_color", "default_prompt"]); const policyKeys = new Set(["products", "allow_implicit_invocation"]); const dependencyKeys = new Set(["tools"]); +const toolDescriptorKeys = new Set(["type", "value", "description", "transport", "url"]); const agentKeys = new Set(["interface", "policy", "dependencies"]); export interface SubmissionSkillMetadataResult { @@ -96,26 +97,68 @@ function rejectUnknownKeys(value: Metadata, allowed: Set): boolean { return Object.keys(value).some((key) => !allowed.has(key)); } +function isToolDescriptor(value: unknown): value is Metadata { + return isRecord(value) && !rejectUnknownKeys(value, toolDescriptorKeys) + && (value.type === "mcp" || value.type === "cli") + && supportedText(value.value) + && (value.description === undefined || supportedText(value.description)) + && (value.transport === undefined || supportedText(value.transport)) + && (value.url === undefined || supportedText(value.url)); +} + +async function validateIconPath( + rootPath: string, + canonicalRoot: string, + skillRoot: string, + skillPath: string, + field: "icon_small" | "icon_large", + value: unknown +): Promise { + if (typeof value !== "string" || value.trim() !== value || !supportedText(value) + || path.isAbsolute(value) || /^[A-Za-z]:[\\/]/u.test(value)) { + return finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field }); + } + const iconPath = path.resolve(skillRoot, value); + if (!isWithin(rootPath, iconPath)) { + return finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field }); + } + try { + const [canonicalIcon, details] = await Promise.all([realpath(iconPath), stat(iconPath)]); + if (!details.isFile() || !isWithin(canonicalRoot, canonicalIcon)) throw new Error("unsafe icon"); + return null; + } catch { + return finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field }); + } +} + async function validateAgentFile( rootPath: string, skillRoot: string, skillPath: string ): Promise { const agentPath = path.join(skillRoot, "agents", "openai.yaml"); + let agentDetails; try { - await lstat(agentPath); + agentDetails = await lstat(agentPath); } catch { return []; } + if (agentDetails.isSymbolicLink()) { + return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata must not be a symbolic link.", { path: skillPath })]; + } + if (!agentDetails.isFile()) { + return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a regular file.", { path: packagePath(rootPath, agentPath) })]; + } + let canonicalRoot: string; let canonicalSkill: string; let canonicalAgent: string; try { - [canonicalSkill, canonicalAgent] = await Promise.all([realpath(skillRoot), realpath(agentPath)]); + [canonicalRoot, canonicalSkill, canonicalAgent] = await Promise.all([realpath(rootPath), realpath(skillRoot), realpath(agentPath)]); } catch { return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a readable regular file.", { path: packagePath(rootPath, agentPath) })]; } - if (!isWithin(canonicalSkill, canonicalAgent)) { + if (!isWithin(canonicalRoot, canonicalAgent) || !isWithin(canonicalSkill, canonicalAgent)) { return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; } @@ -152,24 +195,15 @@ async function validateAgentFile( } if (metadata.dependencies !== undefined && (!isRecord(metadata.dependencies) || rejectUnknownKeys(metadata.dependencies, dependencyKeys) || !Array.isArray(metadata.dependencies.tools) || metadata.dependencies.tools.length === 0 - || new Set(metadata.dependencies.tools).size !== metadata.dependencies.tools.length - || metadata.dependencies.tools.some((tool) => !supportedText(tool)))) { + || metadata.dependencies.tools.some((tool) => !isToolDescriptor(tool)))) { return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent dependencies have an unsupported shape.", { path: packagePath(rootPath, agentPath), field: "dependencies" })]; } for (const field of ["icon_small", "icon_large"] as const) { const value = metadata.interface[field]; if (value === undefined) continue; - if (typeof value !== "string" || !value.startsWith("./")) { - return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field })]; - } - const iconPath = path.resolve(skillRoot, value); - try { - const [canonicalIcon, details] = await Promise.all([realpath(iconPath), stat(iconPath)]); - if (!details.isFile() || !isWithin(canonicalSkill, canonicalIcon)) throw new Error("unsafe icon"); - } catch { - return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field })]; - } + const iconFinding = await validateIconPath(rootPath, canonicalRoot, skillRoot, skillPath, field, value); + if (iconFinding) return [iconFinding]; } return []; } diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 239f420..3190801 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -7,6 +7,7 @@ import { validateSubmissionSkillMetadata } from "../src/core/submission-skill-me const skill = (name = "check", description = "Checks plugin metadata") => `---\nname: ${name}\ndescription: ${description}\n---\n\nUse the checker.\n`; const agent = `interface:\n display_name: Check\n short_description: Check plugin metadata\n`; +const toolDescriptors = `dependencies:\n tools:\n - type: mcp\n value: figma\n description: Figma design tools\n transport: streamable_http\n url: https://example.com/mcp\n - type: cli\n value: adb\n description: Android device bridge\n`; async function packageWith( skills: unknown, @@ -90,7 +91,7 @@ describe("submission skill metadata", () => { it("accepts complete safe agent metadata inside interface", async () => { const result = await validateSubmissionSkillMetadata(await packageWith("./skills/", { "skills/check/SKILL.md": skill(), - "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: ./icon.png\n icon_large: ./icon.png\n brand_color: \"#123ABC\"\n default_prompt: Check this plugin\npolicy:\n products: [CHAT, CODEX]\n allow_implicit_invocation: true\ndependencies:\n tools: [read_file]\n`, + "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: ./icon.png\n icon_large: ./icon.png\n brand_color: \"#123ABC\"\n default_prompt: Check this plugin\npolicy:\n products: [CHAT, CODEX]\n allow_implicit_invocation: true\n${toolDescriptors}`, "skills/check/icon.png": "not-inspected-as-image" }), "skills-only"); @@ -151,7 +152,9 @@ describe("submission skill metadata", () => { ["custom tag", "interface: !custom { display_name: Check, short_description: Check }\n", "plugin.submission.skill.agent.invalid_yaml"], ["unsupported key", `${agent}unexpected: value\n`, "plugin.submission.skill.agent.invalid_shape"], ["policy shape", `${agent}policy: { products: [CHAT, CHAT], allow_implicit_invocation: yes }\n`, "plugin.submission.skill.agent.invalid_shape"], - ["dependency shape", `${agent}dependencies: { tools: [read_file, \"\"] }\n`, "plugin.submission.skill.agent.invalid_shape"] + ["string dependency", `${agent}dependencies: { tools: [read_file] }\n`, "plugin.submission.skill.agent.invalid_shape"], + ["unsupported descriptor", `${agent}dependencies: { tools: [{ type: mcp, value: figma, unsupported: no }] }\n`, "plugin.submission.skill.agent.invalid_shape"], + ["blank descriptor", `${agent}dependencies: { tools: [{ type: cli, value: \" \" }] }\n`, "plugin.submission.skill.agent.invalid_shape"] ] as const)("rejects agent metadata with %s", async (_caseName, contents, expected) => { const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { "skills/check/SKILL.md": skill(), @@ -174,6 +177,59 @@ describe("submission skill metadata", () => { expect(result).toEqual({ findings: [], skillCount: 1 }); }); + it("accepts MCP and CLI tool descriptors without retaining their values", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `${agent}${toolDescriptors}` + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + + it("accepts package-root and relative icon assets", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: assets/local.svg\n icon_large: ../../assets/logo.svg\n`, + "skills/check/assets/local.svg": "local", + "assets/logo.svg": "root" + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + + it("accepts dot-prefixed and parent-relative icon assets inside the package", async () => { + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: ./assets/local.svg\n icon_large: ../.shared/icon.svg\n`, + "skills/check/assets/local.svg": "local", + "skills/.shared/icon.svg": "shared" + }), "skills-only"); + + expect(result).toEqual({ findings: [], skillCount: 1 }); + }); + + it("rejects icon paths outside the package without retaining the path", async () => { + const sentinel = "icon-path-secret"; + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `interface:\n display_name: Check\n short_description: Check plugin metadata\n icon_small: ../../../${sentinel}.svg\n` + }), "skills-only"); + + expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_path"); + expect(JSON.stringify(result)).not.toContain(sentinel); + }); + + it("redacts invalid tool descriptor values, URLs, and descriptions", async () => { + const sentinel = "tool-descriptor-secret"; + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": `${agent}dependencies:\n tools:\n - type: invalid\n value: ${sentinel}\n description: ${sentinel}\n url: https://${sentinel}.example\n` + }), "skills-only"); + + expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_shape"); + expect(JSON.stringify(result)).not.toContain(sentinel); + }); + it("rejects oversized and invalid UTF-8 agent metadata", async () => { const invalidUtf8 = await validateSubmissionSkillMetadata(await packageWith("./skills", { "skills/check/SKILL.md": skill(), @@ -256,6 +312,35 @@ describe("submission skill metadata", () => { }); } + if (process.platform !== "win32") { + it("rejects an in-skill agent-file symlink before parsing its content", async () => { + const discovered = await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/metadata.yaml": agent + }); + const agentPath = path.join(discovered.rootPath, "skills", "check", "agents", "openai.yaml"); + await symlink(path.join(discovered.rootPath, "skills", "check", "agents", "metadata.yaml"), agentPath, "file"); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + + expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_path"); + }); + } + + if (process.platform === "win32") { + it("rejects an agents junction that escapes the skill", async () => { + const discovered = await packageWith("./skills", { "skills/check/SKILL.md": skill() }); + const external = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-external-agents-junction-")); + await writeFile(path.join(external, "openai.yaml"), agent, "utf8"); + await symlink(external, path.join(discovered.rootPath, "skills", "check", "agents"), "junction"); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + + expect(ids(result)).toContain("plugin.submission.skill.agent.invalid_path"); + expect(JSON.stringify(result)).not.toContain(external); + }); + } + it("does not fetch, spawn processes, or expose untrusted skill content", async () => { const sentinel = "skill-secret-sentinel"; const fetchSpy = vi.spyOn(globalThis, "fetch"); From 42f6e0ddc5b99a61d4158700a98db05c54cc57e6 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 12:57:35 +0300 Subject: [PATCH 14/24] fix: reject mixed skill tool descriptors --- src/core/submission-skill-metadata.ts | 1 + tests/submission-skill-metadata.test.ts | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index 5f648ad..e0e325c 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -100,6 +100,7 @@ function rejectUnknownKeys(value: Metadata, allowed: Set): boolean { function isToolDescriptor(value: unknown): value is Metadata { return isRecord(value) && !rejectUnknownKeys(value, toolDescriptorKeys) && (value.type === "mcp" || value.type === "cli") + && (value.type !== "cli" || (value.transport === undefined && value.url === undefined)) && supportedText(value.value) && (value.description === undefined || supportedText(value.description)) && (value.transport === undefined || supportedText(value.transport)) diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 3190801..451d5f7 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -154,7 +154,9 @@ describe("submission skill metadata", () => { ["policy shape", `${agent}policy: { products: [CHAT, CHAT], allow_implicit_invocation: yes }\n`, "plugin.submission.skill.agent.invalid_shape"], ["string dependency", `${agent}dependencies: { tools: [read_file] }\n`, "plugin.submission.skill.agent.invalid_shape"], ["unsupported descriptor", `${agent}dependencies: { tools: [{ type: mcp, value: figma, unsupported: no }] }\n`, "plugin.submission.skill.agent.invalid_shape"], - ["blank descriptor", `${agent}dependencies: { tools: [{ type: cli, value: \" \" }] }\n`, "plugin.submission.skill.agent.invalid_shape"] + ["blank descriptor", `${agent}dependencies: { tools: [{ type: cli, value: \" \" }] }\n`, "plugin.submission.skill.agent.invalid_shape"], + ["CLI transport", `${agent}dependencies: { tools: [{ type: cli, value: adb, transport: streamable_http }] }\n`, "plugin.submission.skill.agent.invalid_shape"], + ["CLI URL", `${agent}dependencies: { tools: [{ type: cli, value: adb, url: https://example.com }] }\n`, "plugin.submission.skill.agent.invalid_shape"] ] as const)("rejects agent metadata with %s", async (_caseName, contents, expected) => { const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { "skills/check/SKILL.md": skill(), From 9744644574abc561905432ca66220ef7535befb2 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 13:10:57 +0300 Subject: [PATCH 15/24] feat: expose submission preflight command --- src/core/output-contract.ts | 76 +++++++++++ src/core/shell-completion.ts | 16 ++- src/index.ts | 25 ++++ src/reporting/render-submission-report.ts | 94 +++++++++++++ src/run-cli.ts | 86 ++++++++++++ tests/completion.test.ts | 7 + tests/contract-command.test.ts | 28 ++++ tests/submission-command.test.ts | 152 ++++++++++++++++++++++ 8 files changed, 483 insertions(+), 1 deletion(-) create mode 100644 src/reporting/render-submission-report.ts create mode 100644 tests/submission-command.test.ts diff --git a/src/core/output-contract.ts b/src/core/output-contract.ts index 316d2fd..6613580 100644 --- a/src/core/output-contract.ts +++ b/src/core/output-contract.ts @@ -246,6 +246,82 @@ const publicSchemaDefinitions: Array<{ } } }, + { + id: "doctor.submission.json", + command: "codex-plugin-doctor doctor submission --json", + required: [ + "schemaVersion", + "rulesetVersion", + "targetType", + "status", + "readiness", + "summary", + "checks", + "findings", + "manualChecklist" + ], + properties: { + rulesetVersion: { const: "openai-directory-2026-08-15" }, + targetType: { type: "string", enum: ["skills-only", "mcp-backed"] }, + status: { type: "string", enum: ["pass", "fail"] }, + readiness: { type: "string", enum: ["blocked", "manual_review_required"] }, + summary: { + type: "object", + required: ["passed", "warnings", "blockers", "manualChecks"], + properties: { + passed: { type: "integer", minimum: 0 }, + warnings: { type: "integer", minimum: 0 }, + blockers: { type: "integer", minimum: 0 }, + manualChecks: { type: "integer", minimum: 0 } + }, + additionalProperties: false + }, + checks: { + type: "array", + items: { + type: "object", + required: ["id", "status", "findingIds"], + properties: { + id: { type: "string", enum: ["listing", "components", "assets", "skills"] }, + status: { type: "string", enum: ["pass", "warn", "fail"] }, + findingIds: { type: "array", items: { type: "string" } } + }, + additionalProperties: false + } + }, + findings: { + type: "array", + items: { + type: "object", + required: ["id", "severity", "message"], + properties: { + id: { type: "string", pattern: "^plugin\\.submission\\." }, + severity: { type: "string", enum: ["warn", "fail"] }, + message: { type: "string" }, + portalCode: { type: "string" }, + evidence: { + type: "object", + additionalProperties: { type: ["string", "number", "boolean", "null"] } + } + }, + additionalProperties: false + } + }, + manualChecklist: { + type: "array", + items: { + type: "object", + required: ["id", "label", "state"], + properties: { + id: { type: "string" }, + label: { type: "string" }, + state: { type: "string", enum: ["required", "not_applicable"] } + }, + additionalProperties: false + } + } + } + }, { id: "doctor.installed.check.json", command: "codex-plugin-doctor check --installed --json", diff --git a/src/core/shell-completion.ts b/src/core/shell-completion.ts index 3de1376..eaa51a2 100644 --- a/src/core/shell-completion.ts +++ b/src/core/shell-completion.ts @@ -20,6 +20,8 @@ const topLevelCommands = [ "config" ]; +const doctorCommands = ["submission"]; + function bashCompletion(): string { return [ "_codex_plugin_doctor() {", @@ -29,17 +31,22 @@ function bashCompletion(): string { " prev=\"${COMP_WORDS[COMP_CWORD-1]}\"", "", ` local commands="${topLevelCommands.join(" ")}"`, + ` local doctor_commands="${doctorCommands.join(" ")}"`, "", " case \"${prev}\" in", " codex-plugin-doctor)", " COMPREPLY=( $(compgen -W \"${commands}\" -- \"${cur}\") )", " return 0", " ;;", + " doctor)", + " COMPREPLY=( $(compgen -W \"${doctor_commands}\" -- \"${cur}\") )", + " return 0", + " ;;", " esac", "", " case \"${cur}\" in", " --*)", - " local flags=\"--json --output --runtime --policy --help\"", + " local flags=\"--json --markdown --output --require-ready --runtime --policy --help\"", " COMPREPLY=( $(compgen -W \"${flags}\" -- \"${cur}\") )", " return 0", " ;;", @@ -64,11 +71,15 @@ function zshCompletion(): string { " typeset -A opt_args", "", ` local commands=(${topLevelCommands.join(" ")})`, + ` local doctor_commands=(${doctorCommands.join(" ")})`, "", " _arguments -C \\", " '1:command:(${commands})' \\", + " '2:doctor command:(${doctor_commands})' \\", " '*--json[Output as JSON]' \\", + " '*--markdown[Output as Markdown]' \\", " '*--output[Write to file]:file:_files' \\", + " '*--require-ready[Fail when automatic checks are blocked]' \\", " '*--runtime[Enable runtime probes]' \\", " '*--policy[Apply policy]:policy:(codex-publish mcp-strict security)'", "}", @@ -84,7 +95,10 @@ function fishCompletion(): string { `complete -c codex-plugin-doctor -n "not __fish_seen_subcommand_from ${topLevelCommands.join(" ")}" -a "${topLevelCommands.join(" ")}"`, "complete -c codex-plugin-doctor -s h -l help -d 'Show help'", "complete -c codex-plugin-doctor -l json -d 'Output as JSON'", + "complete -c codex-plugin-doctor -l markdown -d 'Output as Markdown'", "complete -c codex-plugin-doctor -l output -d 'Write to file' -r", + "complete -c codex-plugin-doctor -l require-ready -d 'Fail when automatic checks are blocked'", + `complete -c codex-plugin-doctor -n "__fish_seen_subcommand_from doctor; and not __fish_seen_subcommand_from ${doctorCommands.join(" ")}" -a "${doctorCommands.join(" ")}"`, "complete -c codex-plugin-doctor -l runtime -d 'Enable runtime probes'", "complete -c codex-plugin-doctor -l policy -d 'Apply policy' -x -a 'codex-publish mcp-strict security'", "" diff --git a/src/index.ts b/src/index.ts index 834d524..74a406d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -145,6 +145,31 @@ export { type OutputContractRule, type OutputContractSchema } from "./core/output-contract.js"; +export { + submissionRuleset, + submissionManualChecks +} from "./core/submission-ruleset.js"; +export { + buildSubmissionPreflight, + type SubmissionCheck, + type SubmissionFinding, + type SubmissionManualCheck, + type SubmissionPreflightReport +} from "./core/submission-preflight.js"; +export { + validateSubmissionAssets, + type SubmissionAssetResult +} from "./core/submission-assets.js"; +export { + validateSubmissionSkillMetadata, + type SubmissionSkillMetadataResult +} from "./core/submission-skill-metadata.js"; +export { + renderSubmissionPreflightJson, + renderSubmissionPreflightMarkdown, + renderSubmissionPreflightText, + submissionPreflightExitCode +} from "./reporting/render-submission-report.js"; export { buildDoctorValidationCorpusReport, renderDoctorValidationCorpusJson, diff --git a/src/reporting/render-submission-report.ts b/src/reporting/render-submission-report.ts new file mode 100644 index 0000000..35ff7bc --- /dev/null +++ b/src/reporting/render-submission-report.ts @@ -0,0 +1,94 @@ +import type { + SubmissionCheck, + SubmissionFinding, + SubmissionPreflightReport +} from "../core/submission-preflight.js"; + +function upper(value: string): string { + return value.replace(/[-_]/gu, " ").toUpperCase(); +} + +function findingsByCheck(report: SubmissionPreflightReport): Array<{ + check: SubmissionCheck; + findings: SubmissionFinding[]; +}> { + const findings = new Map( + report.findings.map((finding) => [finding.id, finding]) + ); + + return report.checks.map((check) => ({ + check, + findings: check.findingIds.flatMap((id) => { + const finding = findings.get(id); + return finding ? [finding] : []; + }) + })); +} + +function escapeMarkdown(value: string): string { + return value.replace(/[\\`*_{}\[\]()<>#+\-.!|]/gu, "\\$&"); +} + +function textSummary(report: SubmissionPreflightReport): string[] { + return [ + `Ruleset: ${report.rulesetVersion}`, + `Target: ${report.targetType}`, + `Automatic status: ${upper(report.status)}`, + `Readiness: ${upper(report.readiness)}`, + `Summary: ${report.summary.passed} passed, ${report.summary.warnings} warnings, ${report.summary.blockers} blockers, ${report.summary.manualChecks} manual checks`, + "Manual review is required; automatic checks do not complete directory review." + ]; +} + +export function renderSubmissionPreflightJson(report: SubmissionPreflightReport): string { + return `${JSON.stringify(report, null, 2)}\n`; +} + +export function renderSubmissionPreflightText(report: SubmissionPreflightReport): string { + const lines = ["Submission preflight", "====================", ...textSummary(report), "", "Findings by check"]; + + for (const { check, findings } of findingsByCheck(report)) { + lines.push(`${check.id} (${upper(check.status)})`); + lines.push(...(findings.length === 0 + ? [" None"] + : findings.map((finding) => ` ${upper(finding.severity)} ${finding.id}: ${finding.message}`))); + } + + lines.push("", "Manual checklist"); + lines.push(...report.manualChecklist.map((item) => ` ${upper(item.state)} ${item.id}: ${item.label}`)); + return `${lines.join("\n")}\n`; +} + +export function renderSubmissionPreflightMarkdown(report: SubmissionPreflightReport): string { + const lines = [ + "# Submission preflight", + "", + `- Ruleset: ${escapeMarkdown(report.rulesetVersion)}`, + `- Target: ${escapeMarkdown(report.targetType)}`, + `- Automatic status: ${escapeMarkdown(upper(report.status))}`, + `- Readiness: ${escapeMarkdown(upper(report.readiness))}`, + `- Summary: ${report.summary.passed} passed, ${report.summary.warnings} warnings, ${report.summary.blockers} blockers, ${report.summary.manualChecks} manual checks`, + "", + "Manual review is required; automatic checks do not complete directory review.", + "", + "## Findings by check" + ]; + + for (const { check, findings } of findingsByCheck(report)) { + lines.push("", `### ${escapeMarkdown(check.id)} (${escapeMarkdown(upper(check.status))})`); + lines.push(...(findings.length === 0 + ? ["- None"] + : findings.map((finding) => `- **${escapeMarkdown(upper(finding.severity))}** ${escapeMarkdown(finding.id)}: ${escapeMarkdown(finding.message)}`))); + } + + lines.push("", "## Manual checklist"); + lines.push(...report.manualChecklist.map((item) => `- ${escapeMarkdown(upper(item.state))}: ${escapeMarkdown(item.id)} — ${escapeMarkdown(item.label)}`)); + return `${lines.join("\n")}\n`; +} + +export function submissionPreflightExitCode( + report: SubmissionPreflightReport, + requireReady: boolean +): 0 | 1 { + return requireReady && report.status === "fail" ? 1 : 0; +} diff --git a/src/run-cli.ts b/src/run-cli.ts index ab73ccf..75aaf9c 100644 --- a/src/run-cli.ts +++ b/src/run-cli.ts @@ -83,6 +83,7 @@ import { renderDoctorOutputContract, renderDoctorOutputContractJson } from "./core/output-contract.js"; +import { buildSubmissionPreflight } from "./core/submission-preflight.js"; import { buildDoctorValidationCorpusReport, renderDoctorValidationCorpusJson, @@ -230,6 +231,12 @@ import { import { renderRuleExplanation } from "./reporting/render-rule-explanation.js"; import { renderSarifReport } from "./reporting/render-sarif-report.js"; import { renderTextReport } from "./reporting/render-text-report.js"; +import { + renderSubmissionPreflightJson, + renderSubmissionPreflightMarkdown, + renderSubmissionPreflightText, + submissionPreflightExitCode +} from "./reporting/render-submission-report.js"; import { applyPolicyToDepAudit, applyPolicyToDoctorConfig, @@ -310,6 +317,14 @@ const defaultIo: CliIo = { } }; +function writeExactStdout(io: CliIo, message: string): void { + if (io === defaultIo) { + process.stdout.write(message); + return; + } + io.writeStdout(message); +} + class CliUsageError extends Error {} function parseRuntimeSandbox( @@ -434,6 +449,9 @@ function printUsage(io: CliIo): void { io.writeStderr( "Remote MCP runtime flags (check, mcp, release check, and doctor release-evidence): --runtime --allow-network [--allow-local-network] [--allow-session-lifecycle] [--require-remote-reliability]\n --allow-session-lifecycle requires --runtime --allow-network and can terminate a remote session.\n --require-remote-reliability requires --runtime --allow-network, blocks non-pass reliability, and does not grant network consent." ); + io.writeStderr( + " codex-plugin-doctor doctor submission [--json|--markdown] [--output ] [--require-ready]" + ); } const suppressUsageText = [ @@ -1587,6 +1605,51 @@ function parseSelectedFixActionIndexes( : null; } +function parseSubmissionCommandArgs(args: string[]): { + targetPath: string; + jsonOutput: boolean; + markdownOutput: boolean; + outputPath: string | null; + requireReady: boolean; +} | CliUsageError { + let targetPath: string | null = null; + let jsonOutput = false; + let markdownOutput = false; + let outputPath: string | null = null; + let requireReady = false; + + for (let index = 0; index < args.length; index += 1) { + const argument = args[index]; + + if (argument === "--json") { + if (jsonOutput) return new CliUsageError("Duplicate submission flag: --json."); + jsonOutput = true; + } else if (argument === "--markdown") { + if (markdownOutput) return new CliUsageError("Duplicate submission flag: --markdown."); + markdownOutput = true; + } else if (argument === "--require-ready") { + if (requireReady) return new CliUsageError("Duplicate submission flag: --require-ready."); + requireReady = true; + } else if (argument === "--output") { + if (outputPath !== null) return new CliUsageError("Duplicate submission flag: --output."); + const value = args[index + 1]; + if (!value || value.startsWith("--")) return new CliUsageError("Missing path after --output."); + outputPath = value; + index += 1; + } else if (argument.startsWith("--")) { + return new CliUsageError(`Unknown submission flag: ${argument}.`); + } else if (targetPath === null) { + targetPath = argument; + } else { + return new CliUsageError(`Unexpected submission argument: ${argument}.`); + } + } + + if (targetPath === null) return new CliUsageError("Missing target path for submission."); + if (jsonOutput && markdownOutput) return new CliUsageError("Use either --json or --markdown, not both."); + return { targetPath, jsonOutput, markdownOutput, outputPath, requireReady }; +} + export async function runCli( args: string[], io: CliIo = defaultIo, @@ -1729,6 +1792,29 @@ export async function runCli( return 0; } + if (maybePath === "submission") { + const parsedSubmissionArgs = parseSubmissionCommandArgs(remainingArgs); + + if (parsedSubmissionArgs instanceof CliUsageError) { + io.writeStderr(parsedSubmissionArgs.message); + return 2; + } + + const report = await buildSubmissionPreflight(parsedSubmissionArgs.targetPath); + const renderedReport = parsedSubmissionArgs.jsonOutput + ? renderSubmissionPreflightJson(report) + : parsedSubmissionArgs.markdownOutput + ? renderSubmissionPreflightMarkdown(report) + : renderSubmissionPreflightText(report); + + if (parsedSubmissionArgs.outputPath) { + await writeFile(parsedSubmissionArgs.outputPath, renderedReport, "utf8"); + } + + writeExactStdout(io, renderedReport); + return submissionPreflightExitCode(report, parsedSubmissionArgs.requireReady); + } + if (maybePath === "mcp") { const targetPath = remainingArgs[0] && !remainingArgs[0].startsWith("--") ? remainingArgs[0] diff --git a/tests/completion.test.ts b/tests/completion.test.ts index abd8815..372feb3 100644 --- a/tests/completion.test.ts +++ b/tests/completion.test.ts @@ -26,6 +26,9 @@ describe("shell completion", () => { expect(output).toContain("check"); expect(output).toContain("audit"); expect(output).toContain("init-git-hooks"); + expect(output).toContain("submission"); + expect(output).toContain("--markdown"); + expect(output).toContain("--require-ready"); }); it("generates zsh completion script", () => { @@ -34,6 +37,8 @@ describe("shell completion", () => { expect(output).toContain("#compdef codex-plugin-doctor"); expect(output).toContain("_arguments"); expect(output).toContain("check"); + expect(output).toContain("submission"); + expect(output).toContain("--markdown"); }); it("generates fish completion script", () => { @@ -42,6 +47,8 @@ describe("shell completion", () => { expect(output).toContain("complete -c codex-plugin-doctor"); expect(output).toContain("__fish_seen_subcommand_from"); expect(output).toContain("codex-publish"); + expect(output).toContain("submission"); + expect(output).toContain("-l markdown"); }); }); diff --git a/tests/contract-command.test.ts b/tests/contract-command.test.ts index 0d3fcac..72ed675 100644 --- a/tests/contract-command.test.ts +++ b/tests/contract-command.test.ts @@ -74,6 +74,11 @@ describe("doctor contract command", () => { id: "doctor.check.json", command: "codex-plugin-doctor check --json" }), + expect.objectContaining({ + id: "doctor.submission.json", + command: "codex-plugin-doctor doctor submission --json", + outputKind: null + }), expect.objectContaining({ id: "doctor.installed.check.json", command: "codex-plugin-doctor check --installed --json" @@ -226,6 +231,9 @@ describe("doctor contract command", () => { const checkSchema = output.schemas.find( (surface: { id: string }) => surface.id === "doctor.check.json" ); + const submissionSchema = output.schemas.find( + (surface: { id: string }) => surface.id === "doctor.submission.json" + ); const mcpSchema = output.schemas.find( (surface: { id: string }) => surface.id === "doctor.mcp.json" ); @@ -248,6 +256,26 @@ describe("doctor contract command", () => { "summary", "findings" ]); + expect(submissionSchema.schema.required).toEqual([ + "schemaVersion", + "rulesetVersion", + "targetType", + "status", + "readiness", + "summary", + "checks", + "findings", + "manualChecklist" + ]); + expect(submissionSchema.schema.properties).toMatchObject({ + targetType: { enum: ["skills-only", "mcp-backed"] }, + status: { enum: ["pass", "fail"] }, + readiness: { enum: ["blocked", "manual_review_required"] }, + summary: { type: "object" }, + checks: { type: "array" }, + findings: { type: "array" }, + manualChecklist: { type: "array" } + }); expect(mcpSchema.schema.required).toEqual([ "schemaVersion", "kind", diff --git a/tests/submission-command.test.ts b/tests/submission-command.test.ts new file mode 100644 index 0000000..96afc00 --- /dev/null +++ b/tests/submission-command.test.ts @@ -0,0 +1,152 @@ +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import * as doctor from "../src/index.js"; +import { runCli } from "../src/run-cli.js"; + +function createIo() { + const stdout: string[] = []; + const stderr: string[] = []; + + return { + stdout, + stderr, + io: { + writeStdout(message: string) { stdout.push(message); }, + writeStderr(message: string) { stderr.push(message); } + } + }; +} + +const validManifest = { + name: "submission-command", + version: "1.0.0", + skills: "./skills", + interface: { + displayName: "Submission check", + shortDescription: "Check directory data", + longDescription: "submission-description-sentinel", + developerName: "Example Developer", + category: "Developer Tools", + logo: "./assets/logo.svg", + composerIcon: "./assets/composer-icon.svg", + defaultPrompt: "submission-prompt-sentinel" + } +}; + +const validSvg = ''; + +async function writeSubmissionPackage(blocked = false): Promise { + const target = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-command-")); + const manifest = blocked + ? { + ...validManifest, + interface: { + ...validManifest.interface, + websiteURL: "https://user:url-credential-sentinel@example.com" + }, + mcpServers: "./.mcp.json" + } + : validManifest; + + await mkdir(path.join(target, ".codex-plugin"), { recursive: true }); + await mkdir(path.join(target, "assets"), { recursive: true }); + await mkdir(path.join(target, "skills", "check"), { recursive: true }); + await writeFile(path.join(target, ".codex-plugin", "plugin.json"), JSON.stringify(manifest), "utf8"); + await writeFile(path.join(target, "assets", "logo.svg"), validSvg, "utf8"); + await writeFile(path.join(target, "assets", "composer-icon.svg"), validSvg, "utf8"); + await writeFile( + path.join(target, "skills", "check", "SKILL.md"), + "---\nname: check\ndescription: skill-description-sentinel\n---\n\nSkill body tool-value-sentinel\n", + "utf8" + ); + + return target; +} + +function expectRedacted(output: string, target: string): void { + expect(output).not.toContain(target); + expect(output).not.toContain("submission-description-sentinel"); + expect(output).not.toContain("submission-prompt-sentinel"); + expect(output).not.toContain("url-credential-sentinel"); + expect(output).not.toContain("skill-description-sentinel"); + expect(output).not.toContain("tool-value-sentinel"); +} + +describe("doctor submission command", () => { + it("renders a valid package as text without crossing the manual review boundary", async () => { + const target = await writeSubmissionPackage(); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["doctor", "submission", target], io); + const output = stdout.join(""); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(output).toContain("Ruleset: openai-directory-2026-08-15"); + expect(output).toContain("Target: skills-only"); + expect(output).toContain("Automatic status: PASS"); + expect(output).toContain("Readiness: MANUAL REVIEW REQUIRED"); + expect(output).toContain("Manual checklist"); + expect(output.toLowerCase()).not.toMatch(/accepted|approved|ready for directory/); + expectRedacted(output, target); + }); + + it.each([ + ["--json", "\"schemaVersion\": \"1.0.0\""], + ["--markdown", "# Submission preflight"] + ])("renders %s output and writes the exact same bytes", async (flag, expected) => { + const target = await writeSubmissionPackage(true); + const outputPath = path.join(target, `submission${flag === "--json" ? ".json" : ".md"}`); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["doctor", "submission", target, flag, "--output", outputPath], io); + const output = stdout.join(""); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(output).toContain(expected); + expect(await readFile(outputPath, "utf8")).toBe(output); + expectRedacted(output, target); + }); + + it("returns a blocking exit only when --require-ready is selected", async () => { + const target = await writeSubmissionPackage(true); + const advisory = createIo(); + const required = createIo(); + + expect(await runCli(["doctor", "submission", target], advisory.io)).toBe(0); + expect(await runCli(["doctor", "submission", target, "--require-ready"], required.io)).toBe(1); + expect(advisory.stderr).toEqual([]); + expect(required.stderr).toEqual([]); + }); + + it.each([ + [[], "Missing target path"], + [["--json", "--markdown"], "Use either --json or --markdown"], + [["--json", "--json"], "Duplicate submission flag"], + [["--output"], "Missing path after --output"], + [["--wat"], "Unknown submission flag"], + [["one", "two"], "Unexpected submission argument"] + ])("rejects invalid arguments %#", async (suffix, message) => { + const target = suffix.length === 0 ? [] : [await writeSubmissionPackage(), ...suffix]; + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["doctor", "submission", ...target], io); + + expect(exitCode).toBe(2); + expect(stdout).toEqual([]); + expect(stderr.join("")).toContain(message); + }); + + it("exports the submission report API", () => { + expect(doctor.submissionRuleset.version).toBe("openai-directory-2026-08-15"); + expect(doctor.buildSubmissionPreflight).toBeTypeOf("function"); + expect(doctor.renderSubmissionPreflightJson).toBeTypeOf("function"); + expect(doctor.renderSubmissionPreflightText).toBeTypeOf("function"); + expect(doctor.renderSubmissionPreflightMarkdown).toBeTypeOf("function"); + expect(doctor.submissionPreflightExitCode).toBeTypeOf("function"); + }); +}); From 83d465f74e6485f8589775427ecb8db2da7cc1cc Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 13:24:19 +0300 Subject: [PATCH 16/24] fix: scope submission CLI flags --- src/core/shell-completion.ts | 26 +++++++++++++++++++++----- src/run-cli.ts | 16 +++++++++++++++- tests/completion.test.ts | 19 ++++++++++++------- tests/submission-command.test.ts | 32 +++++++++++++++++++++++++++++++- 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/core/shell-completion.ts b/src/core/shell-completion.ts index eaa51a2..e38cf55 100644 --- a/src/core/shell-completion.ts +++ b/src/core/shell-completion.ts @@ -21,6 +21,8 @@ const topLevelCommands = [ ]; const doctorCommands = ["submission"]; +const submissionFlags = ["--json", "--markdown", "--output", "--require-ready"]; +const fishSubmissionCondition = "__fish_seen_subcommand_from doctor; and __fish_seen_subcommand_from submission"; function bashCompletion(): string { return [ @@ -32,6 +34,7 @@ function bashCompletion(): string { "", ` local commands="${topLevelCommands.join(" ")}"`, ` local doctor_commands="${doctorCommands.join(" ")}"`, + ` local submission_flags="${submissionFlags.join(" ")}"`, "", " case \"${prev}\" in", " codex-plugin-doctor)", @@ -46,7 +49,11 @@ function bashCompletion(): string { "", " case \"${cur}\" in", " --*)", - " local flags=\"--json --markdown --output --require-ready --runtime --policy --help\"", + " if [[ ${COMP_WORDS[1]} == \"doctor\" && ${COMP_WORDS[2]} == \"submission\" ]]; then", + " COMPREPLY=( $(compgen -W \"${submission_flags}\" -- \"${cur}\") )", + " return 0", + " fi", + " local flags=\"--json --output --runtime --policy --help\"", " COMPREPLY=( $(compgen -W \"${flags}\" -- \"${cur}\") )", " return 0", " ;;", @@ -73,13 +80,20 @@ function zshCompletion(): string { ` local commands=(${topLevelCommands.join(" ")})`, ` local doctor_commands=(${doctorCommands.join(" ")})`, "", + " if [[ \"$words[2]\" == \"doctor\" && \"$words[3]\" == \"submission\" ]]; then", + " _arguments -C \\", + " '*--json[Output as JSON]' \\", + " '*--markdown[Output as Markdown]' \\", + " '*--output[Write to file]:file:_files' \\", + " '*--require-ready[Fail when automatic checks are blocked]'", + " return", + " fi", + "", " _arguments -C \\", " '1:command:(${commands})' \\", " '2:doctor command:(${doctor_commands})' \\", " '*--json[Output as JSON]' \\", - " '*--markdown[Output as Markdown]' \\", " '*--output[Write to file]:file:_files' \\", - " '*--require-ready[Fail when automatic checks are blocked]' \\", " '*--runtime[Enable runtime probes]' \\", " '*--policy[Apply policy]:policy:(codex-publish mcp-strict security)'", "}", @@ -95,9 +109,11 @@ function fishCompletion(): string { `complete -c codex-plugin-doctor -n "not __fish_seen_subcommand_from ${topLevelCommands.join(" ")}" -a "${topLevelCommands.join(" ")}"`, "complete -c codex-plugin-doctor -s h -l help -d 'Show help'", "complete -c codex-plugin-doctor -l json -d 'Output as JSON'", - "complete -c codex-plugin-doctor -l markdown -d 'Output as Markdown'", "complete -c codex-plugin-doctor -l output -d 'Write to file' -r", - "complete -c codex-plugin-doctor -l require-ready -d 'Fail when automatic checks are blocked'", + `complete -c codex-plugin-doctor -n "${fishSubmissionCondition}" -l json -d 'Output as JSON'`, + `complete -c codex-plugin-doctor -n "${fishSubmissionCondition}" -l markdown -d 'Output as Markdown'`, + `complete -c codex-plugin-doctor -n "${fishSubmissionCondition}" -l output -d 'Write to file' -r`, + `complete -c codex-plugin-doctor -n "${fishSubmissionCondition}" -l require-ready -d 'Fail when automatic checks are blocked'`, `complete -c codex-plugin-doctor -n "__fish_seen_subcommand_from doctor; and not __fish_seen_subcommand_from ${doctorCommands.join(" ")}" -a "${doctorCommands.join(" ")}"`, "complete -c codex-plugin-doctor -l runtime -d 'Enable runtime probes'", "complete -c codex-plugin-doctor -l policy -d 'Apply policy' -x -a 'codex-publish mcp-strict security'", diff --git a/src/run-cli.ts b/src/run-cli.ts index 75aaf9c..42f97da 100644 --- a/src/run-cli.ts +++ b/src/run-cli.ts @@ -1617,11 +1617,20 @@ function parseSubmissionCommandArgs(args: string[]): { let markdownOutput = false; let outputPath: string | null = null; let requireReady = false; + let optionsEnded = false; for (let index = 0; index < args.length; index += 1) { const argument = args[index]; - if (argument === "--json") { + if (argument === "--" && !optionsEnded) { + optionsEnded = true; + } else if (optionsEnded) { + if (targetPath === null) { + targetPath = argument; + } else { + return new CliUsageError(`Unexpected submission argument: ${argument}.`); + } + } else if (argument === "--json") { if (jsonOutput) return new CliUsageError("Duplicate submission flag: --json."); jsonOutput = true; } else if (argument === "--markdown") { @@ -1636,6 +1645,11 @@ function parseSubmissionCommandArgs(args: string[]): { if (!value || value.startsWith("--")) return new CliUsageError("Missing path after --output."); outputPath = value; index += 1; + } else if (argument.startsWith("--output=")) { + if (outputPath !== null) return new CliUsageError("Duplicate submission flag: --output."); + const value = argument.slice("--output=".length); + if (!value) return new CliUsageError("Missing path after --output."); + outputPath = value; } else if (argument.startsWith("--")) { return new CliUsageError(`Unknown submission flag: ${argument}.`); } else if (targetPath === null) { diff --git a/tests/completion.test.ts b/tests/completion.test.ts index 372feb3..bafeaa0 100644 --- a/tests/completion.test.ts +++ b/tests/completion.test.ts @@ -18,7 +18,7 @@ function createIo() { describe("shell completion", () => { describe("generateCompletion", () => { - it("generates bash completion script", () => { + it("scopes bash submission flags to doctor submission", () => { const output = generateCompletion("bash"); expect(output).toContain("_codex_plugin_doctor"); @@ -27,28 +27,33 @@ describe("shell completion", () => { expect(output).toContain("audit"); expect(output).toContain("init-git-hooks"); expect(output).toContain("submission"); - expect(output).toContain("--markdown"); - expect(output).toContain("--require-ready"); + expect(output).toContain('local submission_flags="--json --markdown --output --require-ready"'); + expect(output).toContain('local flags="--json --output --runtime --policy --help"'); + expect(output).toContain('${COMP_WORDS[1]} == "doctor"'); }); - it("generates zsh completion script", () => { + it("scopes zsh submission flags to doctor submission", () => { const output = generateCompletion("zsh"); expect(output).toContain("#compdef codex-plugin-doctor"); expect(output).toContain("_arguments"); expect(output).toContain("check"); expect(output).toContain("submission"); - expect(output).toContain("--markdown"); + expect(output).toContain('[[ "$words[2]" == "doctor" && "$words[3]" == "submission" ]]'); + expect(output).toContain("'*--require-ready[Fail when automatic checks are blocked]'"); + expect(output).toContain("'*--runtime[Enable runtime probes]'"); }); - it("generates fish completion script", () => { + it("scopes fish submission flags to doctor submission", () => { const output = generateCompletion("fish"); expect(output).toContain("complete -c codex-plugin-doctor"); expect(output).toContain("__fish_seen_subcommand_from"); expect(output).toContain("codex-publish"); expect(output).toContain("submission"); - expect(output).toContain("-l markdown"); + expect(output).toContain('__fish_seen_subcommand_from doctor; and __fish_seen_subcommand_from submission'); + expect(output).toContain('-l require-ready'); + expect(output).not.toContain('complete -c codex-plugin-doctor -l require-ready'); }); }); diff --git a/tests/submission-command.test.ts b/tests/submission-command.test.ts index 96afc00..50d283a 100644 --- a/tests/submission-command.test.ts +++ b/tests/submission-command.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; @@ -123,12 +123,42 @@ describe("doctor submission command", () => { expect(required.stderr).toEqual([]); }); + it("accepts dash-prefixed paths after -- and treats later flags as positionals", async () => { + const accepted = createIo(); + const extra = createIo(); + + expect(await runCli(["doctor", "submission", "--", "--literal-target"], accepted.io)).toBe(0); + expect(accepted.stderr).toEqual([]); + expect(await runCli(["doctor", "submission", "--", "--literal-target", "--require-ready"], extra.io)).toBe(2); + expect(extra.stderr.join("")).toContain("Unexpected submission argument: --require-ready."); + }); + + it("accepts --output= when the output filename begins with dashes", async () => { + const target = await writeSubmissionPackage(); + const outputPath = `--submission-output-${Date.now()}.json`; + const { io, stdout, stderr } = createIo(); + + try { + const exitCode = await runCli( + ["doctor", "submission", target, "--json", `--output=${outputPath}`], + io + ); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(await readFile(outputPath, "utf8")).toBe(stdout.join("")); + } finally { + await rm(outputPath, { force: true }); + } + }); + it.each([ [[], "Missing target path"], [["--json", "--markdown"], "Use either --json or --markdown"], [["--json", "--json"], "Duplicate submission flag"], [["--output"], "Missing path after --output"], [["--wat"], "Unknown submission flag"], + [["--wat", "--", "--literal-target"], "Unknown submission flag"], [["one", "two"], "Unexpected submission argument"] ])("rejects invalid arguments %#", async (suffix, message) => { const target = suffix.length === 0 ? [] : [await writeSubmissionPackage(), ...suffix]; From 4e59f13126e1bf5158ef85010190da1333750e64 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 13:32:09 +0300 Subject: [PATCH 17/24] feat: add submission preflight to GitHub Action --- README.md | 15 +++++++++++- action.yml | 45 ++++++++++++++++++++++++++++++++-- docs/guides/github-action.md | 19 ++++++++++++++ docs/rules/catalog.md | 43 ++++++++++++++++++++++++++++++++ tests/action-metadata.test.ts | 30 +++++++++++++++++++++++ tests/public-readiness.test.ts | 37 ++++++++++++++++++++++++++++ 6 files changed, 186 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6f04fe9..b195c3f 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,19 @@ version is an immutable collision and must be replaced by a version bump. The command is advisory: it never authenticates, publishes, or changes npm or Registry records. See [MCP Registry Publication Preflight](./docs/architecture/mcp-registry-publication-preflight.md). +### Public Directory Submission Preflight + +Check a plugin package for deterministic public-directory submission issues without contacting a portal: + +```bash +codex-plugin-doctor doctor submission +codex-plugin-doctor doctor submission --json +codex-plugin-doctor doctor submission --markdown +codex-plugin-doctor doctor submission --require-ready +``` + +The preflight is offline and non-executing: it does not submit a package, make network requests, start MCP servers, verify domains, or handle OAuth credentials. Its automatic `status` is `pass` or `fail`; a passing automatic result remains `manual_review_required` until portal-only review is complete. It never claims directory acceptance. See [Public Directory Submission Preflight](./docs/architecture/public-directory-submission-preflight.md). + Output formats: - human text output @@ -494,7 +507,7 @@ jobs: review-bundle-verify: "true" ``` -The action writes `codex-plugin-doctor-summary.md`, `codex-plugin-doctor-report.json`, `codex-plugin-doctor-action-manifest.json`, optional `codex-plugin-doctor.sarif`, optional validation corpus and quality metrics reports, optional `output-contract.json`, and optional signed `review-bundle/` files to `codex-plugin-doctor-reports`, appends the Markdown report to the GitHub Actions step summary, uploads the report directory as an artifact, and then returns the real validation exit code. Review bundle generation requires a signing key environment variable such as `CODEX_PLUGIN_DOCTOR_SIGNING_KEY`. For runtime probing, SARIF output, corpus quality regression gates, corpus and contract artifacts, review bundle artifacts, installed plugin cache checks, CI policy presets, and pinned release examples, see [GitHub Action Usage](./docs/guides/github-action.md). +The action writes `codex-plugin-doctor-summary.md`, `codex-plugin-doctor-report.json`, `codex-plugin-doctor-action-manifest.json`, optional `codex-plugin-doctor.sarif`, optional validation corpus and quality metrics reports, optional `output-contract.json`, and optional signed `review-bundle/` files to `codex-plugin-doctor-reports`, appends the Markdown report to the GitHub Actions step summary, uploads the report directory as an artifact, and then returns the real validation exit code. Set `submission: "true"` to add offline submission preflight reports; set `require-submission-ready: "true"` only with that opt-in to make automatic blockers fail the job. Review bundle generation requires a signing key environment variable such as `CODEX_PLUGIN_DOCTOR_SIGNING_KEY`. For runtime probing, SARIF output, corpus quality regression gates, corpus and contract artifacts, review bundle artifacts, installed plugin cache checks, CI policy presets, and pinned release examples, see [GitHub Action Usage](./docs/guides/github-action.md). To self-test this repository after cloning it: diff --git a/action.yml b/action.yml index 07ca8db..9b633de 100644 --- a/action.yml +++ b/action.yml @@ -38,6 +38,14 @@ inputs: description: Fail unless the configured local Registry metadata receives a pass result. required: false default: "false" + submission: + description: Generate offline public directory submission preflight JSON and Markdown reports. + required: false + default: "false" + require-submission-ready: + description: Fail unless the offline submission preflight is automatically ready; this does not replace manual review. + required: false + default: "false" installed: description: Validate plugins from the local Codex plugin cache. required: false @@ -169,6 +177,12 @@ outputs: registry-report-path: description: Path to the MCP Registry readiness JSON report when registry-metadata is configured. value: ${{ steps.run-doctor.outputs.registry-report-path }} + submission-json-path: + description: Path to the offline submission preflight JSON report when submission is enabled. + value: ${{ steps.run-doctor.outputs.submission-json-path }} + submission-summary-path: + description: Path to the offline submission preflight Markdown report when submission is enabled. + value: ${{ steps.run-doctor.outputs.submission-summary-path }} review-bundle-path: description: Path to the generated review bundle directory when review-bundle is enabled. value: ${{ steps.run-doctor.outputs.review-bundle-path }} @@ -193,6 +207,8 @@ runs: REQUIRE_REMOTE_RELIABILITY_INPUT: ${{ inputs['require-remote-reliability'] }} REGISTRY_METADATA_INPUT: ${{ inputs['registry-metadata'] }} REQUIRE_REGISTRY_READINESS_INPUT: ${{ inputs['require-registry-readiness'] }} + SUBMISSION_INPUT: ${{ inputs.submission }} + REQUIRE_SUBMISSION_READY_INPUT: ${{ inputs['require-submission-ready'] }} CORPUS_METRICS_MANIFEST_INPUT: ${{ inputs['corpus-metrics-manifest'] }} CORPUS_METRICS_BASELINE_INPUT: ${{ inputs['corpus-metrics-baseline'] }} CORPUS_METRICS_FAIL_ON_REGRESSION_INPUT: ${{ inputs['corpus-metrics-fail-on-regression'] }} @@ -209,6 +225,8 @@ runs: output_contract_path="$report_dir/output-contract.json" action_manifest_path="$report_dir/codex-plugin-doctor-action-manifest.json" registry_report_path="$report_dir/mcp-registry-readiness.json" + submission_json_path="$report_dir/codex-plugin-doctor-submission.json" + submission_summary_path="$report_dir/codex-plugin-doctor-submission.md" review_bundle_path="$report_dir/${{ inputs['review-bundle-dir'] }}" review_bundle_verification_path="$report_dir/review-bundle-verification.json" status_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-status" @@ -348,6 +366,18 @@ runs: run_doctor "MCP Registry readiness" "${registry_args[@]}" fi + if [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" && "$SUBMISSION_INPUT" != "true" ]]; then + echo "require-submission-ready requires submission." >&2 + record_status 2 + elif [[ "$SUBMISSION_INPUT" == "true" ]]; then + submission_args=(doctor submission "${{ inputs.path }}" --json --output "$submission_json_path") + if [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" ]]; then + submission_args+=(--require-ready) + fi + run_doctor "submission preflight" "${submission_args[@]}" + run_doctor "submission summary" doctor submission "${{ inputs.path }}" --markdown --output "$submission_summary_path" + fi + if [[ "${{ inputs['review-bundle'] }}" == "true" ]]; then signing_key_env="${{ inputs['signing-key-env'] }}" @@ -395,6 +425,7 @@ runs: export CODEX_PLUGIN_DOCTOR_ACTION_CORPUS_METRICS_DIFF="$([[ -n "$CORPUS_METRICS_BASELINE_INPUT" ]] && echo true || echo false)" export CODEX_PLUGIN_DOCTOR_ACTION_CONTRACT="${{ inputs.contract }}" export CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY="$([[ -n "$REGISTRY_METADATA_INPUT" ]] && echo true || echo false)" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION="$SUBMISSION_INPUT" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE="${{ inputs['review-bundle'] }}" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFY="${{ inputs['review-bundle-verify'] }}" export CODEX_PLUGIN_DOCTOR_ACTION_SUMMARY_PATH="$summary_path" @@ -405,6 +436,8 @@ runs: export CODEX_PLUGIN_DOCTOR_ACTION_CORPUS_METRICS_DIFF_PATH="$corpus_metrics_diff_path" export CODEX_PLUGIN_DOCTOR_ACTION_CONTRACT_PATH="$output_contract_path" export CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY_PATH="$registry_report_path" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH="$submission_json_path" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH="$submission_summary_path" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_PATH="$review_bundle_path" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFICATION_PATH="$review_bundle_verification_path" node <<'NODE' @@ -437,6 +470,8 @@ runs: corpusMetricsDiff: report("corpusMetricsDiff", "CODEX_PLUGIN_DOCTOR_ACTION_CORPUS_METRICS_DIFF", "CODEX_PLUGIN_DOCTOR_ACTION_CORPUS_METRICS_DIFF_PATH"), contract: report("contract", "CODEX_PLUGIN_DOCTOR_ACTION_CONTRACT", "CODEX_PLUGIN_DOCTOR_ACTION_CONTRACT_PATH"), registryReport: report("registryReport", "CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY", "CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY_PATH"), + submissionJson: report("submissionJson", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH"), + submissionSummary: report("submissionSummary", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH"), reviewBundle: report("reviewBundle", "CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE", "CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_PATH"), reviewBundleVerification: report("reviewBundleVerification", "CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFY", "CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFICATION_PATH") } @@ -463,6 +498,8 @@ runs: echo "output-contract-path=$output_contract_path" echo "action-manifest-path=$action_manifest_path" echo "registry-report-path=$registry_report_path" + echo "submission-json-path=$submission_json_path" + echo "submission-summary-path=$submission_summary_path" echo "review-bundle-path=$review_bundle_path" echo "review-bundle-verification-path=$review_bundle_verification_path" } >> "$GITHUB_OUTPUT" @@ -475,6 +512,7 @@ runs: report_dir="${{ inputs['output-dir'] }}" summary_path="$report_dir/codex-plugin-doctor-summary.md" + submission_summary_path="$report_dir/codex-plugin-doctor-submission.md" status_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-status" status="unknown" @@ -484,10 +522,13 @@ runs: if [[ -n "${GITHUB_STEP_SUMMARY:-}" && -f "$summary_path" ]]; then cat "$summary_path" >> "$GITHUB_STEP_SUMMARY" - exit 0 fi - if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && -f "$submission_summary_path" ]]; then + cat "$submission_summary_path" >> "$GITHUB_STEP_SUMMARY" + fi + + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && ! -f "$summary_path" && ! -f "$submission_summary_path" ]]; then { echo "## Codex Plugin Doctor" echo "" diff --git a/docs/guides/github-action.md b/docs/guides/github-action.md index cb0cc4e..ff2d827 100644 --- a/docs/guides/github-action.md +++ b/docs/guides/github-action.md @@ -37,6 +37,23 @@ Use local Registry metadata gating when the repository contains a `server.json` The Action writes `mcp-registry-readiness.json` and exposes `registry-report-path`. This path is checked locally; the Action does not inspect the live Registry and does not grant network access. `require-registry-readiness` requires `registry-metadata` and blocks warnings as well as failures. +## Public Directory Submission Preflight + +Use the submission preflight only when a workflow needs its separate offline report: + +```yaml +- uses: Esquetta/CodexPluginDoctor@v1.59.0 + with: + version: "1.59.0" + path: . + submission: "true" + require-submission-ready: "true" +``` + +`submission` writes `codex-plugin-doctor-submission.json` and `codex-plugin-doctor-submission.md` under `output-dir`, uploads them with the existing artifact directory, appends the Markdown report after the primary summary, and exposes `submission-json-path` and `submission-summary-path` outputs. It does not require runtime or network access, forwards neither runtime nor network consent, and does not start MCP servers or use portal, domain-verification, or OAuth credentials. + +The report's automatic status is separate from manual review: a passing automatic result is still `manual_review_required`, not portal approval. The Action never submits a package or claims acceptance. `require-submission-ready` makes automatic blockers fail the Action status; it requires `submission: "true"`, otherwise the Action records usage status `2` without running a submission command. + ## Recommended Workflow ```yaml @@ -126,6 +143,8 @@ The action also exposes these workflow outputs for follow-up steps: - `output-contract-path` - `action-manifest-path` - `registry-report-path` +- `submission-json-path` +- `submission-summary-path` - `review-bundle-path` - `review-bundle-verification-path` diff --git a/docs/rules/catalog.md b/docs/rules/catalog.md index 0114d94..0f3c7b1 100644 --- a/docs/rules/catalog.md +++ b/docs/rules/catalog.md @@ -42,6 +42,49 @@ codex-plugin-doctor explain plugin.manifest.missing | `plugin.heuristic.skill_description.too_long` | warn | Skill description is likely too verbose. | | `plugin.skill.asset_reference.missing` | warn | Skill references a missing local support asset such as `scripts/...`, `templates/...`, `assets/...`, or `examples/...`. | +## Public Directory Submission Preflight Rules + +| Rule ID | Severity | Meaning | +| --- | --- | --- | +| `plugin.submission.package.invalid` | fail | Plugin manifest is missing or invalid for submission preflight. | +| `plugin.submission.package.name` | fail | Plugin package name is invalid for the listing. | +| `plugin.submission.package.version` | fail | Plugin version is not valid semantic versioning. | +| `plugin.submission.interface.required` | fail | Listing interface metadata is required. | +| `plugin.submission.interface.display_name` | fail | Listing display name is missing or invalid. | +| `plugin.submission.interface.short_description` | fail | Listing short description is missing or invalid. | +| `plugin.submission.interface.long_description` | fail | Listing long description is missing or invalid. | +| `plugin.submission.interface.developer_name` | fail | Listing developer name is missing or invalid. | +| `plugin.submission.interface.category` | fail | Listing category is missing or unsupported. | +| `plugin.submission.interface.capabilities` | fail | Listing capabilities are not a valid bounded list. | +| `plugin.submission.interface.capability` | fail | A listing capability is invalid. | +| `plugin.submission.interface.default_prompt` | fail | Listing starter prompts are invalid or duplicated. | +| `plugin.submission.interface.url` | fail | A required listing URL is invalid. | +| `plugin.submission.interface.unknown_field` | warn | Listing metadata includes an unsupported field. | +| `plugin.submission.component.app` | fail | App declaration must reference a contained parseable root file. | +| `plugin.submission.component.excluded` | fail | A component is not allowed for this submission target type. | +| `plugin.submission.app.invalid_path` | fail | App declaration resolves outside the package. | +| `plugin.submission.asset.required` | fail | A required branding asset is missing. | +| `plugin.submission.asset.invalid_path` | fail | Branding asset path is invalid. | +| `plugin.submission.asset.missing` | fail | Branding asset cannot be found or read. | +| `plugin.submission.asset.unsupported_format` | fail | Branding asset format or file type is unsupported. | +| `plugin.submission.asset.too_large` | fail | Branding asset exceeds the size limit. | +| `plugin.submission.asset.unsafe_svg` | fail | SVG branding asset is unsafe or invalid. | +| `plugin.submission.asset.decode_failed` | fail | Branding asset cannot be decoded safely. | +| `plugin.submission.asset.extension_mismatch` | fail | Branding asset extension does not match its content. | +| `plugin.submission.asset.dimensions` | fail | Branding asset dimensions are outside the allowed range. | +| `plugin.submission.asset.not_square` | fail | Branding asset must be square. | +| `plugin.submission.skill.required` | fail | Skills-only submission requires a valid skill. | +| `plugin.submission.skill.invalid_manifest` | fail | Skills declaration must use the root skills directory. | +| `plugin.submission.skill.invalid_path` | fail | Skill path is not safely contained in the package. | +| `plugin.submission.skill.invalid_file` | fail | Skill entrypoint is not a valid contained file. | +| `plugin.submission.skill.invalid_yaml` | fail | Skill frontmatter YAML is invalid or unsafe. | +| `plugin.submission.skill.invalid_shape` | fail | Skill frontmatter has an unsupported shape. | +| `plugin.submission.skill.identity` | fail | Skill identity metadata is invalid or duplicated. | +| `plugin.submission.skill.agent.invalid_path` | fail | Optional agent metadata path is invalid. | +| `plugin.submission.skill.agent.invalid_file` | fail | Optional agent metadata is not a valid contained file. | +| `plugin.submission.skill.agent.invalid_yaml` | fail | Optional agent metadata YAML is invalid or unsafe. | +| `plugin.submission.skill.agent.invalid_shape` | fail | Optional agent metadata has an unsupported shape. | + ## MCP Rules | Rule ID | Severity | Meaning | diff --git a/tests/action-metadata.test.ts b/tests/action-metadata.test.ts index dd5eebc..ed456e0 100644 --- a/tests/action-metadata.test.ts +++ b/tests/action-metadata.test.ts @@ -126,6 +126,36 @@ describe("GitHub Action metadata", () => { expect(actionMetadata).toContain("registryReport: report("); }); + it("supports opt-in offline submission preflight reports with strict readiness gating", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + + expect(actionMetadata).toMatch(/submission:[\s\S]*?default: "false"/); + expect(actionMetadata).toMatch(/require-submission-ready:[\s\S]*?default: "false"/); + expect(actionMetadata).toContain("submission-json-path:"); + expect(actionMetadata).toContain("submission-summary-path:"); + expect(actionMetadata).toContain('SUBMISSION_INPUT: ${{ inputs.submission }}'); + expect(actionMetadata).toContain('REQUIRE_SUBMISSION_READY_INPUT: ${{ inputs[\'require-submission-ready\'] }}'); + expect(actionMetadata).toContain('submission_json_path="$report_dir/codex-plugin-doctor-submission.json"'); + expect(actionMetadata).toContain('submission_summary_path="$report_dir/codex-plugin-doctor-submission.md"'); + expect(actionMetadata).toContain('if [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" && "$SUBMISSION_INPUT" != "true" ]]; then'); + expect(actionMetadata).toContain('echo "require-submission-ready requires submission." >&2'); + expect(actionMetadata).toContain("record_status 2"); + expect(actionMetadata).toContain('submission_args=(doctor submission "${{ inputs.path }}" --json --output "$submission_json_path")'); + expect(actionMetadata).toContain("submission_args+=(--require-ready)"); + expect(actionMetadata).toContain('run_doctor "submission preflight" "${submission_args[@]}"'); + expect(actionMetadata).toContain('run_doctor "submission summary" doctor submission "${{ inputs.path }}" --markdown --output "$submission_summary_path"'); + expect(actionMetadata).toContain('submissionJson: report("submissionJson", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH")'); + expect(actionMetadata).toContain('submissionSummary: report("submissionSummary", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH")'); + expect(actionMetadata).toContain('echo "submission-json-path=$submission_json_path"'); + expect(actionMetadata).toContain('echo "submission-summary-path=$submission_summary_path"'); + expect(actionMetadata).toContain('cat "$submission_summary_path" >> "$GITHUB_STEP_SUMMARY"'); + expect(actionMetadata).toContain('run_doctor "check" "${args[@]}" "${history_args[@]}" --no-animations'); + expect(actionMetadata).not.toContain("SUBMISSION_RUNTIME_INPUT"); + expect(actionMetadata).not.toContain("SUBMISSION_ALLOW_NETWORK_INPUT"); + expect(actionMetadata).not.toContain("submission_args+=(--runtime"); + expect(actionMetadata).not.toContain("submission_args+=(--allow-network"); + }); + it("documents loopback-only consent without permitting private or reserved ranges", async () => { const actionMetadata = await readFile("action.yml", "utf8"); const actionUsage = await readFile("docs/guides/github-action.md", "utf8"); diff --git a/tests/public-readiness.test.ts b/tests/public-readiness.test.ts index 6c8b13d..1783941 100644 --- a/tests/public-readiness.test.ts +++ b/tests/public-readiness.test.ts @@ -203,4 +203,41 @@ describe("public repository readiness", () => { expect(releaseGating).toContain("--require-registry-readiness"); expect(security).toContain("Registry publication proves namespace control"); }); + + it("documents offline public directory submission preflight without implying portal approval", async () => { + const readme = await readText("README.md"); + const docsReadme = await readText("docs/README.md"); + const architecture = await readText("docs/architecture/public-directory-submission-preflight.md"); + const actionGuide = await readText("docs/guides/github-action.md"); + const catalog = await readText("docs/rules/catalog.md"); + + expect(readme).toContain("codex-plugin-doctor doctor submission "); + expect(readme).toContain("--json"); + expect(readme).toContain("--markdown"); + expect(readme).toContain("--require-ready"); + expect(readme).toContain("manual_review_required"); + expect(docsReadme).toContain("Public Directory Submission Preflight"); + expect(architecture).toContain("no network requests"); + expect(architecture).toContain("does not submit a package"); + expect(architecture).toContain("manual_review_required"); + expect(actionGuide).toContain("Esquetta/CodexPluginDoctor@v1.59.0"); + expect(actionGuide).toContain('submission: "true"'); + expect(actionGuide).toContain('require-submission-ready: "true"'); + expect(actionGuide).toContain("submission-json-path"); + expect(actionGuide).toContain("submission-summary-path"); + expect(actionGuide).toContain("does not require runtime or network access"); + expect(actionGuide).toContain("requires `submission: \"true\"`"); + expect(actionGuide).not.toMatch(/internal (implementation )?plan/i); + expect(actionGuide).not.toMatch(/portal acceptance|automatic manual pass/i); + + for (const [id, severity] of [ + ["plugin.submission.package.invalid", "fail"], + ["plugin.submission.interface.unknown_field", "warn"], + ["plugin.submission.asset.extension_mismatch", "fail"], + ["plugin.submission.skill.agent.invalid_yaml", "fail"], + ["plugin.submission.skill.required", "fail"] + ]) { + expect(catalog).toContain(`| \`${id}\` | ${severity} |`); + } + }); }); From add17583df0b602b83ef69a1cbcc2fed39f73269 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 13:44:09 +0300 Subject: [PATCH 18/24] fix: guard submission Action outputs --- action.yml | 31 ++++++++++++++++++++++++------- tests/action-metadata.test.ts | 30 ++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/action.yml b/action.yml index 9b633de..e167c1b 100644 --- a/action.yml +++ b/action.yml @@ -230,7 +230,11 @@ runs: review_bundle_path="$report_dir/${{ inputs['review-bundle-dir'] }}" review_bundle_verification_path="$report_dir/review-bundle-verification.json" status_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-status" + submission_state_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-submission-ran" status=0 + submission_ran=false + submission_json_output="" + submission_summary_output="" doctor_version="$(codex-plugin-doctor --version)" mkdir -p "$report_dir" @@ -369,13 +373,19 @@ runs: if [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" && "$SUBMISSION_INPUT" != "true" ]]; then echo "require-submission-ready requires submission." >&2 record_status 2 + elif [[ "$SUBMISSION_INPUT" == "true" && "${{ inputs.installed }}" == "true" ]]; then + echo "Submission preflight requires a single package path, not installed-cache mode." >&2 + record_status 2 elif [[ "$SUBMISSION_INPUT" == "true" ]]; then + submission_ran=true submission_args=(doctor submission "${{ inputs.path }}" --json --output "$submission_json_path") if [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" ]]; then submission_args+=(--require-ready) fi run_doctor "submission preflight" "${submission_args[@]}" run_doctor "submission summary" doctor submission "${{ inputs.path }}" --markdown --output "$submission_summary_path" + submission_json_output="$submission_json_path" + submission_summary_output="$submission_summary_path" fi if [[ "${{ inputs['review-bundle'] }}" == "true" ]]; then @@ -425,7 +435,7 @@ runs: export CODEX_PLUGIN_DOCTOR_ACTION_CORPUS_METRICS_DIFF="$([[ -n "$CORPUS_METRICS_BASELINE_INPUT" ]] && echo true || echo false)" export CODEX_PLUGIN_DOCTOR_ACTION_CONTRACT="${{ inputs.contract }}" export CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY="$([[ -n "$REGISTRY_METADATA_INPUT" ]] && echo true || echo false)" - export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION="$SUBMISSION_INPUT" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION="$submission_ran" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE="${{ inputs['review-bundle'] }}" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFY="${{ inputs['review-bundle-verify'] }}" export CODEX_PLUGIN_DOCTOR_ACTION_SUMMARY_PATH="$summary_path" @@ -436,8 +446,8 @@ runs: export CODEX_PLUGIN_DOCTOR_ACTION_CORPUS_METRICS_DIFF_PATH="$corpus_metrics_diff_path" export CODEX_PLUGIN_DOCTOR_ACTION_CONTRACT_PATH="$output_contract_path" export CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY_PATH="$registry_report_path" - export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH="$submission_json_path" - export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH="$submission_summary_path" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH="$submission_json_output" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH="$submission_summary_output" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_PATH="$review_bundle_path" export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFICATION_PATH="$review_bundle_verification_path" node <<'NODE' @@ -485,6 +495,7 @@ runs: NODE printf "%s" "$status" > "$status_file" + printf "%s" "$submission_ran" > "$submission_state_file" { echo "status=$status" @@ -498,8 +509,8 @@ runs: echo "output-contract-path=$output_contract_path" echo "action-manifest-path=$action_manifest_path" echo "registry-report-path=$registry_report_path" - echo "submission-json-path=$submission_json_path" - echo "submission-summary-path=$submission_summary_path" + echo "submission-json-path=$submission_json_output" + echo "submission-summary-path=$submission_summary_output" echo "review-bundle-path=$review_bundle_path" echo "review-bundle-verification-path=$review_bundle_verification_path" } >> "$GITHUB_OUTPUT" @@ -514,21 +525,27 @@ runs: summary_path="$report_dir/codex-plugin-doctor-summary.md" submission_summary_path="$report_dir/codex-plugin-doctor-submission.md" status_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-status" + submission_state_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-submission-ran" status="unknown" + submission_ran=false if [[ -f "$status_file" ]]; then status="$(cat "$status_file")" fi + if [[ -f "$submission_state_file" ]]; then + submission_ran="$(cat "$submission_state_file")" + fi + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && -f "$summary_path" ]]; then cat "$summary_path" >> "$GITHUB_STEP_SUMMARY" fi - if [[ -n "${GITHUB_STEP_SUMMARY:-}" && -f "$submission_summary_path" ]]; then + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && "$submission_ran" == "true" && -f "$submission_summary_path" ]]; then cat "$submission_summary_path" >> "$GITHUB_STEP_SUMMARY" fi - if [[ -n "${GITHUB_STEP_SUMMARY:-}" && ! -f "$summary_path" && ! -f "$submission_summary_path" ]]; then + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && ! -f "$summary_path" && ( "$submission_ran" != "true" || ! -f "$submission_summary_path" ) ]]; then { echo "## Codex Plugin Doctor" echo "" diff --git a/tests/action-metadata.test.ts b/tests/action-metadata.test.ts index ed456e0..21c1052 100644 --- a/tests/action-metadata.test.ts +++ b/tests/action-metadata.test.ts @@ -146,8 +146,8 @@ describe("GitHub Action metadata", () => { expect(actionMetadata).toContain('run_doctor "submission summary" doctor submission "${{ inputs.path }}" --markdown --output "$submission_summary_path"'); expect(actionMetadata).toContain('submissionJson: report("submissionJson", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH")'); expect(actionMetadata).toContain('submissionSummary: report("submissionSummary", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH")'); - expect(actionMetadata).toContain('echo "submission-json-path=$submission_json_path"'); - expect(actionMetadata).toContain('echo "submission-summary-path=$submission_summary_path"'); + expect(actionMetadata).toContain('echo "submission-json-path=$submission_json_output"'); + expect(actionMetadata).toContain('echo "submission-summary-path=$submission_summary_output"'); expect(actionMetadata).toContain('cat "$submission_summary_path" >> "$GITHUB_STEP_SUMMARY"'); expect(actionMetadata).toContain('run_doctor "check" "${args[@]}" "${history_args[@]}" --no-animations'); expect(actionMetadata).not.toContain("SUBMISSION_RUNTIME_INPUT"); @@ -156,6 +156,32 @@ describe("GitHub Action metadata", () => { expect(actionMetadata).not.toContain("submission_args+=(--allow-network"); }); + it("rejects installed-cache submission preflight requests without producing submission reports", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + + expect(actionMetadata).toContain('elif [[ "$SUBMISSION_INPUT" == "true" && "${{ inputs.installed }}" == "true" ]]; then'); + expect(actionMetadata).toContain('echo "Submission preflight requires a single package path, not installed-cache mode." >&2'); + expect(actionMetadata).toContain('record_status 2'); + expect(actionMetadata).toContain('elif [[ "$SUBMISSION_INPUT" == "true" ]]; then\n submission_ran=true\n submission_args=(doctor submission "${{ inputs.path }}" --json --output "$submission_json_path")'); + expect(actionMetadata).toContain('submission_json_output=""'); + expect(actionMetadata).toContain('submission_summary_output=""'); + expect(actionMetadata).toContain('submission_json_output="$submission_json_path"'); + expect(actionMetadata).toContain('submission_summary_output="$submission_summary_path"'); + }); + + it("leaves optional submission paths empty unless submission reports actually ran", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + + expect(actionMetadata).toContain('submission_ran=false'); + expect(actionMetadata).toContain('export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION="$submission_ran"'); + expect(actionMetadata).toContain('export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_JSON_PATH="$submission_json_output"'); + expect(actionMetadata).toContain('export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_SUMMARY_PATH="$submission_summary_output"'); + expect(actionMetadata).toContain('echo "submission-json-path=$submission_json_output"'); + expect(actionMetadata).toContain('echo "submission-summary-path=$submission_summary_output"'); + expect(actionMetadata).toContain('submission_ran="$(cat "$submission_state_file")"'); + expect(actionMetadata).toContain('if [[ -n "${GITHUB_STEP_SUMMARY:-}" && "$submission_ran" == "true" && -f "$submission_summary_path" ]]; then'); + }); + it("documents loopback-only consent without permitting private or reserved ranges", async () => { const actionMetadata = await readFile("action.yml", "utf8"); const actionUsage = await readFile("docs/guides/github-action.md", "utf8"); From 4f90fdc28d76eb2dd0aeea8aa64ba20974aa557e Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 13:53:57 +0300 Subject: [PATCH 19/24] test: harden submission preflight boundaries --- tests/submission-command.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/submission-command.test.ts b/tests/submission-command.test.ts index 50d283a..9cea31a 100644 --- a/tests/submission-command.test.ts +++ b/tests/submission-command.test.ts @@ -47,7 +47,8 @@ async function writeSubmissionPackage(blocked = false): Promise { ...validManifest.interface, websiteURL: "https://user:url-credential-sentinel@example.com" }, - mcpServers: "./.mcp.json" + mcpServers: "./.mcp.json", + apps: "./.app.json" } : validManifest; @@ -57,6 +58,9 @@ async function writeSubmissionPackage(blocked = false): Promise { await writeFile(path.join(target, ".codex-plugin", "plugin.json"), JSON.stringify(manifest), "utf8"); await writeFile(path.join(target, "assets", "logo.svg"), validSvg, "utf8"); await writeFile(path.join(target, "assets", "composer-icon.svg"), validSvg, "utf8"); + if (blocked) { + await writeFile(path.join(target, ".app.json"), '{"name":"app-json-sentinel"}', "utf8"); + } await writeFile( path.join(target, "skills", "check", "SKILL.md"), "---\nname: check\ndescription: skill-description-sentinel\n---\n\nSkill body tool-value-sentinel\n", @@ -71,6 +75,7 @@ function expectRedacted(output: string, target: string): void { expect(output).not.toContain("submission-description-sentinel"); expect(output).not.toContain("submission-prompt-sentinel"); expect(output).not.toContain("url-credential-sentinel"); + expect(output).not.toContain("app-json-sentinel"); expect(output).not.toContain("skill-description-sentinel"); expect(output).not.toContain("tool-value-sentinel"); } From 896baff0f2d8188fc64fa9007b1e5f8d04e461fb Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 14:00:52 +0300 Subject: [PATCH 20/24] fix: redact absolute Action target paths --- action.yml | 7 +++++- tests/action-metadata.test.ts | 44 ++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/action.yml b/action.yml index e167c1b..78bcc47 100644 --- a/action.yml +++ b/action.yml @@ -452,8 +452,13 @@ runs: export CODEX_PLUGIN_DOCTOR_ACTION_REVIEW_BUNDLE_VERIFICATION_PATH="$review_bundle_verification_path" node <<'NODE' const fs = require("node:fs"); + const path = require("node:path"); const enabled = (name) => process.env[name] === "true"; + const displayTargetPath = (value) => { + if (path.isAbsolute(value) || path.win32.isAbsolute(value)) return "[absolute-path-redacted]"; + return value === "" ? "" : path.posix.normalize(value.replace(/\\/gu, "/")); + }; const report = (key, enabledEnv, pathEnv) => ({ enabled: enabled(enabledEnv), path: process.env[pathEnv] || "" @@ -467,7 +472,7 @@ runs: reportDirectory: process.env.CODEX_PLUGIN_DOCTOR_ACTION_REPORT_DIR || "", artifactName: process.env.CODEX_PLUGIN_DOCTOR_ACTION_ARTIFACT_NAME || "", target: { - path: process.env.CODEX_PLUGIN_DOCTOR_ACTION_PATH || "", + path: displayTargetPath(process.env.CODEX_PLUGIN_DOCTOR_ACTION_PATH || ""), installed: enabled("CODEX_PLUGIN_DOCTOR_ACTION_INSTALLED"), runtime: enabled("CODEX_PLUGIN_DOCTOR_ACTION_RUNTIME") }, diff --git a/tests/action-metadata.test.ts b/tests/action-metadata.test.ts index 21c1052..14eb2f1 100644 --- a/tests/action-metadata.test.ts +++ b/tests/action-metadata.test.ts @@ -1,8 +1,50 @@ -import { readFile } from "node:fs/promises"; +import { execFile } from "node:child_process"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; import packageJson from "../package.json" with { type: "json" }; +const execFileAsync = promisify(execFile); + +async function renderActionManifest(actionMetadata: string, targetPath: string): Promise> { + const start = actionMetadata.indexOf(' const fs = require("node:fs");'); + const end = actionMetadata.indexOf("\n NODE", start); + const manifestDirectory = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-action-manifest-")); + const manifestPath = path.join(manifestDirectory, "manifest.json"); + const script = actionMetadata.slice(start, end).replace(/^ /gmu, ""); + + try { + await execFileAsync(process.execPath, ["-e", script], { + env: { + ...process.env, + CODEX_PLUGIN_DOCTOR_ACTION_MANIFEST_PATH: manifestPath, + CODEX_PLUGIN_DOCTOR_ACTION_PATH: targetPath + } + }); + return JSON.parse(await readFile(manifestPath, "utf8")) as Record; + } finally { + await rm(manifestDirectory, { recursive: true, force: true }); + } +} + describe("GitHub Action metadata", () => { + it("redacts absolute POSIX, drive, and UNC target paths from the Action manifest", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + + for (const targetPath of ["/absolute-posix-sentinel", "C:\\\\absolute-drive-sentinel", "\\\\server-sentinel\\share"]) { + const manifest = await renderActionManifest(actionMetadata, targetPath); + const target = manifest.target as { path: string }; + + expect(JSON.stringify(manifest)).not.toContain(targetPath); + expect(target.path).toBe("[absolute-path-redacted]"); + } + + const relative = await renderActionManifest(actionMetadata, "plugins\\\\relative-target"); + expect((relative.target as { path: string }).path).toBe("plugins/relative-target"); + }); + it("exposes a composite action that installs and runs codex-plugin-doctor", async () => { const actionMetadata = await readFile("action.yml", "utf8"); From fa958abb760a73adfa9bbc6b44228cc35da9bd18 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 14:15:09 +0300 Subject: [PATCH 21/24] fix: redact file URI Action targets --- action.yml | 2 +- tests/action-metadata.test.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/action.yml b/action.yml index 78bcc47..8c58361 100644 --- a/action.yml +++ b/action.yml @@ -456,7 +456,7 @@ runs: const enabled = (name) => process.env[name] === "true"; const displayTargetPath = (value) => { - if (path.isAbsolute(value) || path.win32.isAbsolute(value)) return "[absolute-path-redacted]"; + if (/^file:/iu.test(value) || path.isAbsolute(value) || path.win32.isAbsolute(value)) return "[absolute-path-redacted]"; return value === "" ? "" : path.posix.normalize(value.replace(/\\/gu, "/")); }; const report = (key, enabledEnv, pathEnv) => ({ diff --git a/tests/action-metadata.test.ts b/tests/action-metadata.test.ts index 14eb2f1..b16d98c 100644 --- a/tests/action-metadata.test.ts +++ b/tests/action-metadata.test.ts @@ -45,6 +45,21 @@ describe("GitHub Action metadata", () => { expect((relative.target as { path: string }).path).toBe("plugins/relative-target"); }); + it("redacts file URI target paths from the Action manifest", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + + for (const [targetPath, sentinel] of [ + ["file:///uri-sentinel", "uri-sentinel"], + ["file://server-sentinel/share", "server-sentinel"], + ["FiLe:///case-sentinel", "case-sentinel"] + ]) { + const manifest = await renderActionManifest(actionMetadata, targetPath); + + expect(JSON.stringify(manifest)).not.toContain(sentinel); + expect((manifest.target as { path: string }).path).toBe("[absolute-path-redacted]"); + } + }); + it("exposes a composite action that installs and runs codex-plugin-doctor", async () => { const actionMetadata = await readFile("action.yml", "utf8"); From 03e145c51a81515df2bcfe855aeabf940071e7e9 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 14:24:24 +0300 Subject: [PATCH 22/24] release: prepare v1.59.0 --- CHANGELOG.md | 17 +++++++++++++ README.md | 4 +-- docs/guides/github-action.md | 48 ++++++++++++++++++------------------ package-lock.json | 4 +-- package.json | 2 +- tests/release-check.test.ts | 4 +-- tests/release-notes.test.ts | 24 +++++++++++++++--- tests/release-sync.test.ts | 4 +-- 8 files changed, 70 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e214353..7255dc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to `codex-plugin-doctor` are documented here. This changelog groups the shipped work into product-level release blocks instead of repeating every low-level git diff in isolation. +## [1.59.0] - 2026-08-17 + +### Added + +- added offline `doctor submission ` reports in text, JSON, and Markdown, with advisory defaults and `--require-ready` as the strict automatic blocker gate +- added automatic public-directory listing and classification, root `.app.json` boundary checks, bounded branding asset validation, and safe skill and `openai.yaml` identity and metadata validation +- added opt-in GitHub Action submission inputs, artifacts, and step-summary output while preserving existing defaults + +### Changed + +- kept portal-only review items explicit: automatic passing results still require manual review and do not submit a package or claim directory acceptance + +### Security + +- bounded raster, SVG, and YAML parsing; canonical containment; no process execution or network access; and redacted evidence with absolute Action paths +- added `yaml` and `fast-xml-parser` parsing dependencies without package scripts; includes the transitive `nanoid` remediation + ## [1.58.0] - 2026-08-11 ### Added diff --git a/README.md b/README.md index b195c3f..dc50c68 100644 --- a/README.md +++ b/README.md @@ -493,9 +493,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 - - uses: Esquetta/CodexPluginDoctor@v1.58.0 + - uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . runtime: "true" policy: codex-publish diff --git a/docs/guides/github-action.md b/docs/guides/github-action.md index ff2d827..03355bb 100644 --- a/docs/guides/github-action.md +++ b/docs/guides/github-action.md @@ -27,9 +27,9 @@ The Action transfers these boolean inputs through environment-backed shell varia Use local Registry metadata gating when the repository contains a `server.json` intended for publication: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . registry-metadata: ./server.json require-registry-readiness: "true" @@ -70,9 +70,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 - - uses: Esquetta/CodexPluginDoctor@v1.58.0 + - uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . runtime: "true" policy: codex-publish @@ -99,9 +99,9 @@ Every action run also writes `codex-plugin-doctor-action-manifest.json`. The man Use SARIF when repository security tooling should ingest validation findings. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . sarif: "true" ``` @@ -113,9 +113,9 @@ The action writes `codex-plugin-doctor.sarif` into `output-dir`. Uploading it to Use artifact and summary controls when the workflow needs custom retention or wants to disable generated report uploads. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . output-dir: doctor-ci-reports artifact-name: codex-plugin-doctor-reports @@ -153,11 +153,11 @@ The action also exposes these workflow outputs for follow-up steps: Use review bundle artifacts when a pull request or release workflow should preserve signed runtime approval, runtime policy, attestation, and release evidence handoff files. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 env: CODEX_PLUGIN_DOCTOR_SIGNING_KEY: ${{ secrets.CODEX_PLUGIN_DOCTOR_SIGNING_KEY }} with: - version: "1.58.0" + version: "1.59.0" path: . review-bundle: "true" review-bundle-verify: "true" @@ -188,9 +188,9 @@ The CLI can produce badge output for release notes, README automation, or a stat Use a private corpus metrics manifest to measure reviewed precision, recall, and false-positive share in CI. The action writes only the public-safe metrics report into its artifact directory; snapshots, manifest contents, local paths, and review notes are not copied. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . corpus-metrics-manifest: ../private-corpus/metrics.json ``` @@ -198,9 +198,9 @@ Use a private corpus metrics manifest to measure reviewed precision, recall, and This writes `corpus-metrics.json`. To compare the result with a retained report and fail the job on regression: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . corpus-metrics-manifest: ../private-corpus/metrics.json corpus-metrics-baseline: .doctor-baselines/corpus-metrics.json @@ -229,9 +229,9 @@ The history file is newline-delimited JSON. Store it as an artifact, cache, or r The composite action can also append history directly: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . runtime: "true" history: validation-history.jsonl @@ -251,9 +251,9 @@ Use profiles when a consuming workflow needs a named validation policy instead o The composite action can pass profiles directly: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . profile: publish ``` @@ -263,9 +263,9 @@ The composite action can pass profiles directly: Use policy presets when a workflow should apply one of the opinionated release gates without adding a local `.codex-doctor.json`. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" path: . policy: codex-publish ``` @@ -277,9 +277,9 @@ Supported policy values are `codex-publish`, `mcp-strict`, and `security`. The C Use installed-cache mode only in environments where Codex plugins are already available on the runner. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" installed: "true" filter: github runtime: "false" @@ -290,9 +290,9 @@ Use installed-cache mode only in environments where Codex plugins are already av Pin both the action ref and npm package version for reproducible CI: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.58.0 +- uses: Esquetta/CodexPluginDoctor@v1.59.0 with: - version: "1.58.0" + version: "1.59.0" ``` Use `version: "latest"` only when the consuming repository intentionally wants automatic CLI upgrades. diff --git a/package-lock.json b/package-lock.json index 2f65ee2..d1f4375 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codex-plugin-doctor", - "version": "1.58.0", + "version": "1.59.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codex-plugin-doctor", - "version": "1.58.0", + "version": "1.59.0", "license": "MIT", "dependencies": { "fast-xml-parser": "^5.11.0", diff --git a/package.json b/package.json index 88c0035..256dcf3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codex-plugin-doctor", - "version": "1.58.0", + "version": "1.59.0", "description": "CLI-first validator for Codex plugins, skills, and MCP package surfaces with runtime MCP protocol validation.", "type": "module", "main": "./dist/index.js", diff --git a/tests/release-check.test.ts b/tests/release-check.test.ts index 5a37bcf..d22e5b8 100644 --- a/tests/release-check.test.ts +++ b/tests/release-check.test.ts @@ -12,7 +12,7 @@ import { } from "../scripts/release-check.mjs"; describe("release check registry version gate", () => { - it("keeps package and lockfile roots on the 1.58.0 release version", async () => { + it("keeps package and lockfile roots on the 1.59.0 release version", async () => { const packageJson = JSON.parse(await readFile("package.json", "utf8")) as { version: string; }; @@ -21,7 +21,7 @@ describe("release check registry version gate", () => { packages: { "": { version: string } }; }; - expect(packageJson.version).toBe("1.58.0"); + expect(packageJson.version).toBe("1.59.0"); expect(packageLock.version).toBe(packageJson.version); expect(packageLock.packages[""].version).toBe(packageJson.version); }); diff --git a/tests/release-notes.test.ts b/tests/release-notes.test.ts index 27fd0ec..790e044 100644 --- a/tests/release-notes.test.ts +++ b/tests/release-notes.test.ts @@ -10,16 +10,32 @@ import { describe("extractReleaseSection", () => { it("records the latest release and restores the two shipped release sections", async () => { const changelog = await readFile("CHANGELOG.md", "utf8"); - const latestRelease = changelog.indexOf("## [1.58.0] - 2026-08-11"); - const previousRelease = changelog.indexOf("## [1.57.0] - 2026-08-08"); - const olderRelease = changelog.indexOf("## [1.56.0] - 2026-08-02"); + const latestRelease = changelog.indexOf("## [1.59.0] - 2026-08-17"); + const previousRelease = changelog.indexOf("## [1.58.0] - 2026-08-11"); + const olderRelease = changelog.indexOf("## [1.57.0] - 2026-08-08"); expect(latestRelease).toBeGreaterThanOrEqual(0); expect(previousRelease).toBeGreaterThan(latestRelease); expect(olderRelease).toBeGreaterThan(previousRelease); + expect(extractReleaseSection(changelog, "1.59.0")).toContain( + "offline `doctor submission `" + ); expect(extractReleaseSection(changelog, "1.58.0")).toContain("current official MCP layouts"); expect(extractReleaseSection(changelog, "1.57.0")).toContain("npm pack dry-run"); - expect(extractReleaseSection(changelog, "1.56.0")).toContain("doctor size "); + }); + + it("keeps current README and Action examples pinned to the latest release", async () => { + const [readme, actionGuide] = await Promise.all([ + readFile("README.md", "utf8"), + readFile("docs/guides/github-action.md", "utf8") + ]); + + expect(readme).toContain("Esquetta/CodexPluginDoctor@v1.59.0"); + expect(readme).toContain('version: "1.59.0"'); + expect(actionGuide).toContain("Esquetta/CodexPluginDoctor@v1.59.0"); + expect(actionGuide).toContain('version: "1.59.0"'); + expect(actionGuide).not.toContain("Esquetta/CodexPluginDoctor@v1.58.0"); + expect(actionGuide).not.toContain('version: "1.58.0"'); }); it("extracts the matching version section from the changelog", () => { diff --git a/tests/release-sync.test.ts b/tests/release-sync.test.ts index 809bd8b..c4bd658 100644 --- a/tests/release-sync.test.ts +++ b/tests/release-sync.test.ts @@ -5,12 +5,12 @@ import { describe, expect, it } from "vitest"; import { evaluateReleaseSync } from "../src/release/release-sync.js"; describe("evaluateReleaseSync", () => { - it("uses the 1.58.0 stable release target", async () => { + it("uses the 1.59.0 stable release target", async () => { const packageJson = JSON.parse(await readFile("package.json", "utf8")) as { version: string; }; - expect(packageJson.version).toBe("1.58.0"); + expect(packageJson.version).toBe("1.59.0"); }); it("passes when npm, remote tag, GitHub release, and latest release match", () => { From 7c1124554be610012ba6b12769b7ed3a58f034b8 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 14:48:09 +0300 Subject: [PATCH 23/24] fix: accept standard submission image containers --- src/core/submission-assets.ts | 59 ++++++++++++++++++--------------- tests/submission-assets.test.ts | 51 ++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index c79e95d..2cdef6f 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -140,8 +140,10 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { } const view = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength); let dimensions: Dimensions | null = null; + let scans = 0; let offset = 2; - for (let steps = 0; steps < 128 && offset < buffer.length; steps += 1) { + for (let steps = 0; steps < 512 && offset < buffer.length; steps += 1) { + if (buffer[offset] !== 0xff) return null; while (offset < buffer.length && buffer[offset] === 0xff) { offset += 1; } @@ -150,7 +152,7 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { } const marker = buffer[offset++]; if (marker === 0xd9) { - return null; + return dimensions !== null && scans > 0 && offset === buffer.length ? dimensions : null; } if (marker === 0x01 || (marker >= 0xd0 && marker <= 0xd7)) { continue; @@ -173,6 +175,7 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { if (dimensions === null) return null; const componentCount = buffer[offset + 2]; if (componentCount < 1 || length !== 6 + (componentCount * 2)) return null; + scans += 1; offset += length; for (let entropySteps = 0; entropySteps < buffer.length && offset < buffer.length; entropySteps += 1) { if (buffer[offset] !== 0xff) { @@ -185,9 +188,9 @@ function readJpeg(buffer: Uint8Array): Dimensions | null { offset += 2; continue; } - return entropyMarker === 0xd9 && offset + 2 === buffer.length ? dimensions : null; + break; } - return null; + continue; } offset += length; } @@ -207,29 +210,33 @@ function readWebp(buffer: Uint8Array): Dimensions | null { if (riffSize !== buffer.length - 8) { return null; } - const chunkType = fourCc(buffer, 12); - const chunkLength = view.getUint32(16, true); - const payload = 20; - const paddedLength = chunkLength + (chunkLength % 2); - if (paddedLength !== buffer.length - payload) { - return null; - } - if (chunkType === "VP8X") { - if (chunkLength < 10) return null; - const width = 1 + buffer[payload + 4] + (buffer[payload + 5] << 8) + (buffer[payload + 6] << 16); - const height = 1 + buffer[payload + 7] + (buffer[payload + 8] << 8) + (buffer[payload + 9] << 16); - return { width, height }; - } - if (chunkType === "VP8L") { - if (chunkLength < 5 || buffer[payload] !== 0x2f) return null; - const packed = view.getUint32(payload + 1, true); - return { width: (packed & 0x3fff) + 1, height: ((packed >>> 14) & 0x3fff) + 1 }; - } - if (chunkType === "VP8 ") { - if (chunkLength < 10 || buffer[payload + 3] !== 0x9d || buffer[payload + 4] !== 0x01 || buffer[payload + 5] !== 0x2a) return null; - return { width: view.getUint16(payload + 6, true) & 0x3fff, height: view.getUint16(payload + 8, true) & 0x3fff }; + let dimensions: Dimensions | null = null; + let offset = 12; + for (let chunks = 0; chunks < 128 && offset + 8 <= buffer.length; chunks += 1) { + const chunkType = fourCc(buffer, offset); + const chunkLength = view.getUint32(offset + 4, true); + const payload = offset + 8; + const paddedLength = chunkLength + (chunkLength % 2); + if (paddedLength > buffer.length - payload) return null; + let detected: Dimensions | null = null; + if (chunkType === "VP8X") { + if (chunkLength < 10) return null; + detected = { + width: 1 + buffer[payload + 4] + (buffer[payload + 5] << 8) + (buffer[payload + 6] << 16), + height: 1 + buffer[payload + 7] + (buffer[payload + 8] << 8) + (buffer[payload + 9] << 16) + }; + } else if (chunkType === "VP8L") { + if (chunkLength < 5 || buffer[payload] !== 0x2f) return null; + const packed = view.getUint32(payload + 1, true); + detected = { width: (packed & 0x3fff) + 1, height: ((packed >>> 14) & 0x3fff) + 1 }; + } else if (chunkType === "VP8 ") { + if (chunkLength < 10 || buffer[payload + 3] !== 0x9d || buffer[payload + 4] !== 0x01 || buffer[payload + 5] !== 0x2a) return null; + detected = { width: view.getUint16(payload + 6, true) & 0x3fff, height: view.getUint16(payload + 8, true) & 0x3fff }; + } + dimensions ??= detected; + offset = payload + paddedLength; } - return null; + return offset === buffer.length ? dimensions : null; } function rasterAsset(buffer: Uint8Array): { format: Exclude; dimensions: Dimensions } | null { diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index 2199d87..11ca2b8 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -66,9 +66,9 @@ const pngWithHeader = (width: number, height: number, bitDepth: number, colorTyp return image; }; -const jpegSof = (width: number, height: number) => { +const jpegSof = (width: number, height: number, marker = 0xc0) => { const data = new Uint8Array(21); - data.set([0xff, 0xd8, 0xff, 0xc0, 0, 17, 8]); + data.set([0xff, 0xd8, 0xff, marker, 0, 17, 8]); const view = new DataView(data.buffer); view.setUint16(7, height); view.setUint16(9, width); @@ -87,6 +87,24 @@ const jpegWithSos = (width: number, height: number, includeEoi: boolean) => { const jpeg = (width: number, height: number) => jpegWithSos(width, height, true); +const jpegSos = (component: number, spectralStart = 0, spectralEnd = 63) => new Uint8Array([0xff, 0xda, 0, 8, 1, component, 0, spectralStart, spectralEnd, 0]); + +const jpegDht = () => new Uint8Array([0xff, 0xc4, 0, 20, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + +const concat = (...parts: Uint8Array[]) => { + const result = new Uint8Array(parts.reduce((length, part) => length + part.length, 0)); + let offset = 0; + for (const part of parts) { result.set(part, offset); offset += part.length; } + return result; +}; + +const progressiveMultiScanJpeg = (width: number, height: number) => concat( + jpegSof(width, height, 0xc2), + jpegSos(1, 0, 0), new Uint8Array([0x11, 0xff, 0x00, 0xff, 0xd0, 0x22]), + jpegDht(), + jpegSos(1, 1, 63), new Uint8Array([0x33, 0xff, 0xd9]) +); + const webp = (variant: "VP8X" | "VP8L" | "VP8", width: number, height: number) => { const payload = variant === "VP8X" ? new Uint8Array(10) : variant === "VP8L" ? new Uint8Array(5) : new Uint8Array(10); const view = new DataView(payload.buffer); @@ -111,6 +129,25 @@ const webp = (variant: "VP8X" | "VP8L" | "VP8", width: number, height: number) = return data; }; +const webpChunk = (type: string, payload: Uint8Array) => { + const chunk = new Uint8Array(8 + payload.length + (payload.length % 2)); + chunk.set(type.split("").map((character) => character.charCodeAt(0)), 0); + new DataView(chunk.buffer).setUint32(4, payload.length, true); + chunk.set(payload, 8); + return chunk; +}; + +const webpWithTrailingExif = () => { + const base = webp("VP8L", 48, 48); + const chunks = concat(base.slice(12), webpChunk("EXIF", new Uint8Array([1]))); + const data = new Uint8Array(12 + chunks.length); + data.set([82, 73, 70, 70]); + new DataView(data.buffer).setUint32(4, data.length - 8, true); + data.set([87, 69, 66, 80], 8); + data.set(chunks, 12); + return data; +}; + const svg = (attributes: string) => ``; async function packageWithAssets(interfaceValues: Record, files: AssetFiles = {}): Promise { @@ -148,6 +185,16 @@ describe("submission assets", () => { expect(await findingIds({}, {})).toEqual(["plugin.submission.asset.required", "plugin.submission.asset.required"]); }); + it("accepts a structurally complete progressive multi-scan JPEG", async () => { + expect(await findingIds({ logo: "./logo.jpg", composerIcon: "./logo.jpg" }, { "logo.jpg": progressiveMultiScanJpeg(48, 48) })) + .toEqual([]); + }); + + it("accepts a WebP image chunk followed by padded EXIF metadata", async () => { + expect(await findingIds({ logo: "./logo.webp", composerIcon: "./logo.webp" }, { "logo.webp": webpWithTrailingExif() })) + .toEqual([]); + }); + it("rejects a PNG whose declared IHDR is truncated", async () => { const truncated = new Uint8Array(24); truncated.set([137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82]); From 2a8dd3e5b0d19feea60bdb0c5f223a51b023338c Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 17 Aug 2026 14:53:39 +0300 Subject: [PATCH 24/24] fix: bound submission package inputs --- .../public-directory-submission-preflight.md | 3 +- docs/rules/catalog.md | 3 + src/core/submission-preflight.ts | 43 +++++++++- src/core/submission-skill-metadata.ts | 78 ++++++++++++++----- tests/public-readiness.test.ts | 5 +- tests/submission-preflight.test.ts | 14 ++++ tests/submission-skill-metadata.test.ts | 25 ++++++ 7 files changed, 148 insertions(+), 23 deletions(-) diff --git a/docs/architecture/public-directory-submission-preflight.md b/docs/architecture/public-directory-submission-preflight.md index 9fcc5b1..648ed47 100644 --- a/docs/architecture/public-directory-submission-preflight.md +++ b/docs/architecture/public-directory-submission-preflight.md @@ -142,7 +142,8 @@ For each skill, `skills/*/agents/openai.yaml` is optional. When present, it is p - `interface.display_name` - `interface.short_description` -- `interface.icons` +- `interface.icon_small` +- `interface.icon_large` - `interface.brand_color` - `interface.default_prompt` - `policy.products` diff --git a/docs/rules/catalog.md b/docs/rules/catalog.md index 0f3c7b1..eaff223 100644 --- a/docs/rules/catalog.md +++ b/docs/rules/catalog.md @@ -47,6 +47,7 @@ codex-plugin-doctor explain plugin.manifest.missing | Rule ID | Severity | Meaning | | --- | --- | --- | | `plugin.submission.package.invalid` | fail | Plugin manifest is missing or invalid for submission preflight. | +| `plugin.submission.package.too_large` | fail | Plugin manifest exceeds the submission preflight size limit. | | `plugin.submission.package.name` | fail | Plugin package name is invalid for the listing. | | `plugin.submission.package.version` | fail | Plugin version is not valid semantic versioning. | | `plugin.submission.interface.required` | fail | Listing interface metadata is required. | @@ -80,6 +81,8 @@ codex-plugin-doctor explain plugin.manifest.missing | `plugin.submission.skill.invalid_yaml` | fail | Skill frontmatter YAML is invalid or unsafe. | | `plugin.submission.skill.invalid_shape` | fail | Skill frontmatter has an unsupported shape. | | `plugin.submission.skill.identity` | fail | Skill identity metadata is invalid or duplicated. | +| `plugin.submission.skill.too_many` | fail | Skills directory exceeds the bounded submission preflight entry or skill limit. | +| `plugin.submission.skill.budget_exceeded` | fail | Skill metadata exceeds the aggregate submission preflight size limit. | | `plugin.submission.skill.agent.invalid_path` | fail | Optional agent metadata path is invalid. | | `plugin.submission.skill.agent.invalid_file` | fail | Optional agent metadata is not a valid contained file. | | `plugin.submission.skill.agent.invalid_yaml` | fail | Optional agent metadata YAML is invalid or unsafe. | diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts index 4c756d3..b15379e 100644 --- a/src/core/submission-preflight.ts +++ b/src/core/submission-preflight.ts @@ -1,7 +1,7 @@ -import { readFile, realpath, stat } from "node:fs/promises"; +import { lstat, readFile, realpath, stat } from "node:fs/promises"; import path from "node:path"; -import { discoverPackage } from "./discover-package.js"; +import type { DiscoveredPackage, PluginManifest } from "../domain/types.js"; import { validateSubmissionAssets } from "./submission-assets.js"; import { submissionManualChecks, submissionRuleset } from "./submission-ruleset.js"; import { validateSubmissionSkillMetadata } from "./submission-skill-metadata.js"; @@ -50,6 +50,7 @@ const knownInterfaceFields = new Set([ "capabilities", "websiteURL", "supportURL", "privacyPolicyURL", "termsOfServiceURL", "brandColor", "brandColorDark", "defaultPrompt", "composerIcon", "logo", "screenshots" ]); +const maxSubmissionManifestBytes = 1024 * 1024; function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); @@ -274,11 +275,47 @@ function invalidPackageReport(): SubmissionPreflightReport { }; } +function oversizedPackageReport(): SubmissionPreflightReport { + const findings = [finding("plugin.submission.package.too_large", "fail", "Plugin manifest exceeds the submission preflight size limit.", { + path: ".codex-plugin/plugin.json", limit: maxSubmissionManifestBytes + })]; + const checks = [check("listing", findings), check("components", []), check("assets", []), check("skills", [])]; + return { + schemaVersion: "1.0.0", + rulesetVersion: submissionRuleset.version, + targetType: "skills-only", + status: "fail", + readiness: "blocked", + summary: { passed: 3, warnings: 0, blockers: 1, manualChecks: 3 }, + checks, + findings, + manualChecklist: checklist("skills-only") + }; +} + +async function discoverSubmissionPackage(targetPath: string): Promise { + const rootPath = path.resolve(targetPath); + const manifestPath = path.join(rootPath, ".codex-plugin", "plugin.json"); + try { + const linkDetails = await lstat(manifestPath); + if (!linkDetails.isFile()) return null; + const details = await stat(manifestPath); + if (details.size > maxSubmissionManifestBytes) return "too_large"; + const content = new TextDecoder("utf-8", { fatal: true }).decode(await readFile(manifestPath)); + return { rootPath, manifestPath, manifest: JSON.parse(content) as PluginManifest }; + } catch { + return null; + } +} + export async function buildSubmissionPreflight(targetPath: string): Promise { if (typeof targetPath !== "string") { return invalidPackageReport(); } - const discovered = await discoverPackage(targetPath); + const discovered = await discoverSubmissionPackage(targetPath); + if (discovered === "too_large") { + return oversizedPackageReport(); + } if (!discovered || !isRecord(discovered.manifest)) { return invalidPackageReport(); } diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index e0e325c..55e7ef1 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -1,4 +1,4 @@ -import { lstat, readdir, readFile, realpath, stat } from "node:fs/promises"; +import { lstat, opendir, readFile, realpath, stat } from "node:fs/promises"; import path from "node:path"; import { isAlias, isNode, parseDocument, visit } from "yaml"; @@ -12,6 +12,9 @@ type Metadata = Record; const maxSkillBytes = 1024 * 1024; const maxAgentBytes = 256 * 1024; +const maxAggregateMetadataBytes = 16 * 1024 * 1024; +const maxDirectoryEntries = 256; +const maxSkillDirectories = 100; const unsupportedText = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u202E\u2060-\u206F\uFEFF]/u; const interfaceKeys = new Set(["display_name", "short_description", "icon_small", "icon_large", "brand_color", "default_prompt"]); const policyKeys = new Set(["products", "allow_implicit_invocation"]); @@ -83,13 +86,28 @@ async function safeDirectory(rootPath: string, skillsPath: string): Promise<{ ca } } -async function readSafeUtf8(filePath: string, maximum: number): Promise { +type AggregateRead = { kind: "source"; source: string } | { kind: "invalid" } | { kind: "budget"; nextBytes: number }; + +interface AggregateBudget { + bytes: number; +} + +function budgetFinding(bytes: number): SubmissionFinding { + return finding("plugin.submission.skill.budget_exceeded", "Skill metadata exceeds the aggregate submission preflight size limit.", { + count: bytes, limit: maxAggregateMetadataBytes + }); +} + +async function readSafeUtf8(filePath: string, maximum: number, budget: AggregateBudget): Promise { try { const details = await stat(filePath); - if (!details.isFile() || details.size > maximum) return null; - return new TextDecoder("utf-8", { fatal: true }).decode(await readFile(filePath)); + if (!details.isFile() || details.size > maximum) return { kind: "invalid" }; + const nextBytes = budget.bytes + details.size; + if (nextBytes > maxAggregateMetadataBytes) return { kind: "budget", nextBytes }; + budget.bytes = nextBytes; + return { kind: "source", source: new TextDecoder("utf-8", { fatal: true }).decode(await readFile(filePath)) }; } catch { - return null; + return { kind: "invalid" }; } } @@ -135,7 +153,8 @@ async function validateIconPath( async function validateAgentFile( rootPath: string, skillRoot: string, - skillPath: string + skillPath: string, + budget: AggregateBudget ): Promise { const agentPath = path.join(skillRoot, "agents", "openai.yaml"); let agentDetails; @@ -163,11 +182,14 @@ async function validateAgentFile( return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; } - const source = await readSafeUtf8(agentPath, maxAgentBytes); - if (source === null) { + const source = await readSafeUtf8(agentPath, maxAgentBytes, budget); + if (source.kind === "budget") { + return [budgetFinding(source.nextBytes)]; + } + if (source.kind === "invalid") { return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a bounded UTF-8 regular file.", { path: packagePath(rootPath, agentPath), limit: maxAgentBytes })]; } - const parsed = parseSafeYaml(source); + const parsed = parseSafeYaml(source.source); if ("error" in parsed) { return [finding( parsed.error === "yaml" ? "plugin.submission.skill.agent.invalid_yaml" : "plugin.submission.skill.agent.invalid_shape", @@ -229,16 +251,30 @@ export async function validateSubmissionSkillMetadata( return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory must be canonically contained in the package.", { path: "skills" })], skillCount: 0 }; } + const findings: SubmissionFinding[] = []; + const identities = new Set(); + let skillCount = 0; + const budget: AggregateBudget = { bytes: 0 }; let entries; try { - entries = await readdir(skillsPath, { withFileTypes: true }); + entries = await opendir(skillsPath); } catch { return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory cannot be inspected safely.", { path: "skills" })], skillCount: 0 }; } - const findings: SubmissionFinding[] = []; - const identities = new Set(); - let skillCount = 0; - for (const entry of entries.filter((candidate) => !candidate.name.startsWith(".") && candidate.isDirectory())) { + let entryCount = 0; + let skillDirectoryCount = 0; + for await (const entry of entries) { + entryCount += 1; + if (entryCount > maxDirectoryEntries) { + findings.push(finding("plugin.submission.skill.too_many", "Skills directory exceeds the submission preflight entry limit.", { count: entryCount, limit: maxDirectoryEntries })); + break; + } + if (entry.name.startsWith(".") || !entry.isDirectory()) continue; + skillDirectoryCount += 1; + if (skillDirectoryCount > maxSkillDirectories) { + findings.push(finding("plugin.submission.skill.too_many", "Skills directory exceeds the submission preflight skill limit.", { count: skillDirectoryCount, limit: maxSkillDirectories })); + break; + } const skillRoot = path.join(skillsPath, entry.name); const skillFile = path.join(skillRoot, "SKILL.md"); const relativeSkillPath = packagePath(rootPath, skillFile); @@ -264,12 +300,16 @@ export async function validateSubmissionSkillMetadata( findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a contained regular file.", { path: relativeSkillPath })); continue; } - const source = await readSafeUtf8(skillFile, maxSkillBytes); - if (source === null) { + const source = await readSafeUtf8(skillFile, maxSkillBytes, budget); + if (source.kind === "budget") { + findings.push(budgetFinding(source.nextBytes)); + break; + } + if (source.kind === "invalid") { findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a bounded UTF-8 regular file.", { path: relativeSkillPath, limit: maxSkillBytes })); continue; } - const split = splitSkillFile(source); + const split = splitSkillFile(source.source); if (!split) { findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint requires delimited frontmatter and a nonempty body.", { path: relativeSkillPath })); continue; @@ -294,7 +334,9 @@ export async function validateSubmissionSkillMetadata( } identities.add(normalizedName); skillCount += 1; - findings.push(...await validateAgentFile(rootPath, skillRoot, packagePath(rootPath, skillRoot))); + const agentFindings = await validateAgentFile(rootPath, skillRoot, packagePath(rootPath, skillRoot), budget); + findings.push(...agentFindings); + if (agentFindings.some((item) => item.id === "plugin.submission.skill.budget_exceeded")) break; } if (targetType === "skills-only" && skillCount === 0 && !findings.some((item) => item.id === "plugin.submission.skill.required")) { findings.push(finding("plugin.submission.skill.required", "Skills-only submissions require at least one valid skill.", { count: 0 })); diff --git a/tests/public-readiness.test.ts b/tests/public-readiness.test.ts index 1783941..d851027 100644 --- a/tests/public-readiness.test.ts +++ b/tests/public-readiness.test.ts @@ -234,8 +234,11 @@ describe("public repository readiness", () => { ["plugin.submission.package.invalid", "fail"], ["plugin.submission.interface.unknown_field", "warn"], ["plugin.submission.asset.extension_mismatch", "fail"], + ["plugin.submission.package.too_large", "fail"], ["plugin.submission.skill.agent.invalid_yaml", "fail"], - ["plugin.submission.skill.required", "fail"] + ["plugin.submission.skill.required", "fail"], + ["plugin.submission.skill.too_many", "fail"], + ["plugin.submission.skill.budget_exceeded", "fail"] ]) { expect(catalog).toContain(`| \`${id}\` | ${severity} |`); } diff --git a/tests/submission-preflight.test.ts b/tests/submission-preflight.test.ts index 7048a1e..62db558 100644 --- a/tests/submission-preflight.test.ts +++ b/tests/submission-preflight.test.ts @@ -214,6 +214,20 @@ describe("submission preflight", () => { } }); + it("bounds the submission manifest before decoding or parsing it", async () => { + const target = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-large-manifest-")); + const sentinel = "submission-manifest-secret"; + await mkdir(path.join(target, ".codex-plugin")); + await writeFile(path.join(target, ".codex-plugin", "plugin.json"), sentinel.padEnd(1024 * 1024 + 1, "x"), "utf8"); + + const report = await buildSubmissionPreflight(target); + + expect(report).toMatchObject({ status: "fail", readiness: "blocked" }); + expect(findingIds(report)).toEqual(["plugin.submission.package.too_large"]); + expect(JSON.stringify(report)).not.toContain(sentinel); + expect(JSON.stringify(report)).not.toContain(target); + }); + it("fails closed for a non-string public target input", async () => { const report = await buildSubmissionPreflight(null as unknown as string); diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 451d5f7..15089bf 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -67,6 +67,31 @@ describe("submission skill metadata", () => { expect(result).toEqual({ findings: [], skillCount: 1 }); }); + it("limits immediate non-hidden skill directories without exposing their paths", async () => { + const sentinel = "too-many-skill-secret"; + const files = Object.fromEntries(Array.from({ length: 101 }, (_, index) => [ + `skills/check-${index}/SKILL.md`, + skill(`check-${index}`, `Checks ${sentinel}`) + ])); + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", files), "skills-only"); + + expect(ids(result).filter((id) => id === "plugin.submission.skill.too_many")).toHaveLength(1); + expect(JSON.stringify(result)).not.toContain(sentinel); + }); + + it("limits aggregate skill metadata bytes before reading more content", async () => { + const sentinel = "aggregate-skill-secret"; + const body = sentinel.padEnd(1024 * 1024 - 80, "x"); + const files = Object.fromEntries(Array.from({ length: 17 }, (_, index) => [ + `skills/check-${index}/SKILL.md`, + `---\nname: check-${index}\ndescription: Check aggregate metadata\n---\n\n${body}\n` + ])); + const result = await validateSubmissionSkillMetadata(await packageWith("./skills", files), "skills-only"); + + expect(ids(result).filter((id) => id === "plugin.submission.skill.budget_exceeded")).toHaveLength(1); + expect(JSON.stringify(result)).not.toContain(sentinel); + }); + it("rejects a visible immediate skill directory without a regular entrypoint", async () => { const result = await validateSubmissionSkillMetadata(await packageWith("./skills", { "skills/check/.keep": ""