From 124fbb63ff891dc53b3eae0eb4a0c7ea7a42bcd5 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 13:09:51 +0300 Subject: [PATCH 01/20] docs: design submission archive preflight --- docs/README.md | 1 + .../public-directory-archive-preflight.md | 365 ++++++++++++++++++ 2 files changed, 366 insertions(+) create mode 100644 docs/architecture/public-directory-archive-preflight.md diff --git a/docs/README.md b/docs/README.md index 5c07f8e..ffa4d4a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,6 +15,7 @@ This directory contains public documentation for users, contributors, and securi - [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) +- [Public Directory Archive Preflight](architecture/public-directory-archive-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-archive-preflight.md b/docs/architecture/public-directory-archive-preflight.md new file mode 100644 index 0000000..62bf0fa --- /dev/null +++ b/docs/architecture/public-directory-archive-preflight.md @@ -0,0 +1,365 @@ +# Public Directory Archive Preflight + +## Purpose + +Codex Plugin Doctor validates a skills-only ZIP before the author uploads it to the OpenAI public-directory submission portal. The archive preflight checks the packaged artifact itself, not only the source directory that produced it. + +The feature is offline, read-only, and non-executing. It never extracts archive entries to disk, starts an MCP server, invokes a system archive utility, sends a network request, authenticates to the portal, uploads a package, or claims that OpenAI will accept a submission. + +The initial archive ruleset is based on the public OpenAI submission-error reference reviewed on 2026-08-23: + +- +- + +## Command Surface + +```bash +codex-plugin-doctor doctor submission archive plugin.zip +codex-plugin-doctor doctor submission archive plugin.zip --json +codex-plugin-doctor doctor submission archive plugin.zip --markdown +codex-plugin-doctor doctor submission archive plugin.zip --output archive-report.json +codex-plugin-doctor doctor submission archive plugin.zip --require-ready +``` + +The command accepts an existing ZIP only. It does not create, normalize, repair, or rewrite an archive. + +`doctor submission ` remains unchanged. `doctor submission archive ` reuses the same automatic listing, asset, and skill checks through a read-only package abstraction after the ZIP structure passes its safety gates. + +## Target Boundary + +The archive command models the skills-only ZIP upload flow. + +The following archive contents produce portal-aligned confirmation warnings because they belong to the MCP-backed submission flow: + +- a manifest `mcpServers` declaration +- a root `.mcp.json` +- a manifest `apps` declaration +- a root `.app.json` +- `interface.screenshots` + +The report tells the author to use the portal's MCP-backed flow. The command does not validate an MCP-backed upload archive or make live MCP calls. + +These findings use severity `warn`, retain the matching portal code, and keep `readiness: manual_review_required`. The archive adapter downgrades only these exact skills-only exclusion findings; the existing v1.59 directory-preflight behavior does not change. + +Archive blockers are reserved for malformed or unsafe ZIP structures and deterministic invalid-package rules. + +## Result Contract + +The additive machine-readable surface is `doctor.submission.archive.json` with schema version `1.0.0`. + +```json +{ + "schemaVersion": "1.0.0", + "rulesetVersion": "openai-directory-archive-2026-08-23", + "status": "pass", + "readiness": "manual_review_required", + "archive": { + "fileName": "plugin.zip", + "compressedBytes": 123456, + "uncompressedBytes": 456789, + "entryCount": 24, + "rootLayout": "archive-root" + }, + "summary": { + "passed": 18, + "warnings": 0, + "blockers": 0, + "manualChecks": 3 + }, + "archiveChecks": [], + "submission": {}, + "findings": [], + "coverage": [], + "manualChecklist": [] +} +``` + +`status` covers deterministic automatic checks only: + +- `pass`: no automatic archive or nested submission blocker was found. +- `fail`: at least one automatic blocker was found. + +`readiness` remains one of: + +- `blocked` +- `manual_review_required` + +The archive command never returns `accepted`, `approved`, or an automatic ready state. + +### Exit Codes + +- `0`: the command completed, including advisory reports that contain blockers. +- `1`: blockers exist and `--require-ready` was supplied. +- `2`: command usage is invalid. + +Corrupt, truncated, encrypted, unsupported, or otherwise malformed ZIP input becomes a structured finding rather than an uncaught exception. + +## Privacy And Evidence + +Reports do not include the absolute ZIP path. The archive summary retains only a sanitized file name and numerical metadata. + +Finding evidence is limited to safe scalar locators such as: + +- `entryIndex` +- a safe package-relative entry path when the path itself passed text-safety checks +- `field` +- `count` +- `limit` +- compression method identifier + +Reports never retain unsafe raw entry names, decompressed file contents, raw manifests, prompts, descriptions, YAML, credentials, CRC payloads, or archive bytes. Text, JSON, Markdown, the GitHub Action manifest, and future signed consumers use the same redacted result model. + +## Archive Reader Architecture + +The ZIP is inspected without filesystem extraction. + +### `submission-archive-reader.ts` + +The archive reader owns ZIP-format parsing and bounded entry reads: + +- locate and validate EOCD and optional ZIP64 records +- reject multi-disk archives +- stream central-directory entries lazily +- validate central-directory boundaries and offsets +- compare central and local header names, methods, flags, sizes, and CRC metadata +- automatically inspect stored and deflate entries +- permit well-formed 32-bit and ZIP64 data descriptors +- reject encrypted entries as unreadable +- report other well-formed compression methods as unavailable coverage rather than portal failures +- stream every regular stored or deflated entry once during the archive safety phase, validating emitted size and CRC while discarding content +- expose bounded lazy reads to nested validators only after the complete archive safety phase passes +- never retain unrequested decompressed content +- never create a file, directory, symlink, process, or network request + +ZIP64 values are accepted only when they are well-formed, fit within JavaScript safe integers, and remain below the public submission limits. ZIP64 does not relax any archive budget. + +### `submission-archive-preflight.ts` + +The archive preflight owns portal-facing rules, plugin-root discovery, coverage accounting, report aggregation, and exit policy. + +### `SubmissionPackageReader` + +Directory and archive validators share a small read-only interface: + +```ts +interface SubmissionPackageReader { + list(directory: string): Promise; + stat(packagePath: string): Promise; + read(packagePath: string, maxBytes: number): Promise; +} +``` + +The interface exposes normalized package-relative paths and bounded bytes. It cannot return an absolute host path or perform a write. Existing directory behavior remains compatible while listing, asset, and skill validators gain an archive-backed reader. + +## Automatic Archive Rules + +New stable identifiers use the `plugin.submission.archive.*` namespace. When the public reference provides a portal code, it is retained separately as `portalCode`. + +### File And ZIP Limits + +- input must be a regular `.zip` file +- archive must be non-empty and parseable +- compressed ZIP size must not exceed 100 MB +- entry count must not exceed 5,000 +- one entry must not exceed 100 MiB uncompressed +- cumulative uncompressed size must not exceed 512 MiB +- encrypted entries are unreadable blockers; stored and deflated entries receive automatic content validation, while other well-formed methods receive unavailable coverage + +Declared sizes are checked before decompression. Actual emitted bytes are counted during decompression, and the reader aborts before a declared or cumulative budget can be exceeded. A small compressed payload cannot expand past the configured limit. + +The safety phase streams every regular entry that Doctor can decode, including entries that nested package validation will not request. An unused corrupt member or expansion bomb therefore cannot hide behind lazy package reads. + +### Entry Paths And Types + +An entry name must: + +- be valid supported text +- be non-empty and have no outer whitespace +- use `/`, never `\` +- be relative and contain no drive prefix +- contain no empty or `..` segment +- contain at most 20 segments including the file name + +The archive rejects: + +- exact duplicate paths +- a path that is both a file and directory +- a file path that contains child entries +- unsupported entry types, including symlinks and device entries + +Doctor emits a warning when paths collide under `NFKC` followed by ECMAScript's locale-independent `toLowerCase()` for each segment. This is a reproducible local safety signal, not a claim that the portal uses the same algorithm. The portal's unspecified case and Unicode-normalization algorithm remains `coverage: unavailable` and is never marked automatically passed. + +The public reference names a path-length limit without publishing its numeric value. The initial ruleset does not invent one. The report includes a `coverage: unavailable` item for that portal-side limit and never marks it automatically passed. + +### ZIP Encoding And Entry Metadata + +The reader follows these normative rules: + +- when general-purpose flag bit 11 is set, central and local names must be valid UTF-8 +- otherwise names are decoded as CP437; undecodable or unsupported text is rejected before it can enter evidence +- central and local raw name bytes must match exactly +- an Info-ZIP Unicode Path extra field does not override the raw-name comparison; when it would change the decoded path, portal filename-decoding parity is reported as unavailable +- general-purpose flag bit 3 requires a data descriptor after the payload +- a data-descriptor signature is optional; CRC and size widths are selected from the entry's ZIP64 state +- local CRC and size placeholders are allowed only when bit 3 is set and the descriptor supplies the authoritative values +- Unix external attributes classify symlinks and device entries by file-type bits; DOS attributes and a trailing `/` classify directories +- contradictory directory/type metadata is rejected +- each local interval runs from its local header through payload and optional descriptor +- local intervals must not overlap one another, the central directory, ZIP64 records, or EOCD; no overlap exception is permitted + +Stored and deflate are Doctor's initial reader formats. Because the public reference does not enumerate the portal's supported compression methods, other methods produce unavailable coverage instead of a portal-equivalence failure. + +### Central And Local Header Consistency + +For each entry, the reader validates: + +- local-header offset is within the archive +- central and local raw name bytes match exactly before decoding +- compression methods and relevant flags match +- sizes and CRC metadata are consistent, including data-descriptor cases +- header, payload, descriptor, ZIP64, central-directory, and EOCD ranges remain in bounds and local entry intervals never overlap +- decompressed output matches the expected CRC and size + +### Plugin Root + +The archive must contain exactly one plugin root: + +- files may be at the archive root, or +- all plugin files may be inside one top-level directory + +A top-level plugin directory cannot have siblings. + +The root must contain one recognized manifest path: + +- `.codex-plugin/plugin.json` +- `.agent-plugin/plugin.json` +- `.claude-plugin/plugin.json` + +It must also contain at least one immediate `skills//SKILL.md`. + +For `.claude-plugin` input, the report records the portal's documented normalization behavior. `.agent-plugin` remains a recognized manifest path without a normalization claim. Doctor validates the published fields it can interpret but does not invent undocumented defaults or claim byte-for-byte portal parity. + +## Nested Submission Validation + +After archive structure and budgets pass, the archive-backed reader runs the existing automatic submission checks: + +- package identity and semantic version +- public listing limits and supported text +- skills-only component exclusions +- required branding assets and bounded image validation +- `SKILL.md` identity and body checks +- optional `agents/openai.yaml` schema and contained asset references +- duplicate skill identity and aggregate skill budgets + +Nested validation retains the v1.59 manual-review boundary. Identity verification, attestations, safety scans, and any portal judgment remain manual. + +## Ruleset And Coverage Governance + +The embedded ruleset is `openai-directory-archive-2026-08-23`. + +It records: + +- official source URLs +- review date +- numeric archive limits +- Doctor-supported compression and ZIP structures +- Doctor's local collision-warning algorithm +- automatic checks +- portal-only or insufficiently documented checks + +The command does not download rule updates or scrape documentation. A ruleset update is a reviewed source change with tests and changelog coverage. + +Coverage states are: + +- `automatic`: Doctor performed the deterministic rule. +- `manual`: the item requires human or portal review. +- `unavailable`: the public source names the rule but does not define enough information for a faithful local implementation. + +An unavailable rule never becomes an automatic pass. + +The following portal warnings depend on submission history or undocumented normalization and therefore remain manual or unavailable: + +- `plugin_name_mismatch`, which requires the previously published identity +- `plugin_version_unchanged`, which requires the previously published version +- `manifest_normalized`, whose exact normalized output is portal-owned +- `developer_name_defaulted`, which depends on the selected verified identity +- `.claude-plugin` normalization details beyond the published fields Doctor can interpret + +## GitHub Action + +Archive validation is disabled by default: + +```yaml +- uses: Esquetta/CodexPluginDoctor@v1.60.0 + with: + submission-archive: ./plugin.zip + require-submission-ready: "true" +``` + +The Action emits separate JSON and Markdown archive reports under the existing report directory, exposes their paths as outputs, includes them in the Action artifact manifest, and appends the Markdown result to the step summary. + +`require-submission-ready` requires exactly one selected mode: directory `submission: "true"` or a non-empty `submission-archive`. It forwards `--require-ready` to that selected mode. Selecting both modes, or strict readiness with neither mode, records usage status `2` and produces no submission report. Existing Action defaults remain unchanged. + +## Dependency Gate + +The implementation may use one pure-JavaScript ZIP reader in lazy-entry mode. Before selection, the candidate and its complete production dependency closure must pass: + +- no native binary +- no lifecycle or install script +- no more than five newly introduced production packages in the closure +- no more than 2 MiB total unpacked installed size for that closure +- zero production audit findings from `npm audit --omit=dev --audit-level=low` +- zero listed lifecycle scripts from `npm install-scripts ls` +- no more than 100 KiB increase in the Doctor publish tarball reported by `npm pack --dry-run --json` +- support for lazy entries, stored/deflate data, CRC validation, data descriptors, and ZIP64 metadata +- deterministic malformed-input behavior under the required budgets + +The adapter remains responsible for Doctor's portal policy, central/local consistency checks, privacy, and resource budgets. A library does not replace those checks. + +A compatible dependency is a release prerequisite; the feature is not shipped without one. There is no fallback to `unzip`, PowerShell, shell wrappers, native extraction, or write-to-temp behavior. + +## Verification Contract + +Implementation is complete only when tests cover: + +- valid archive-root and single-top-level-directory packages +- stored and deflated entries +- valid ZIP64 values within all portal limits +- empty, truncated, encrypted, and multi-disk archives, plus unavailable coverage for well-formed unsupported compression +- central/local name, method, flag, CRC, size, and offset mismatch +- valid and missing data descriptors +- an unused corrupt or expansion-bomb entry that nested validation never requests +- overlapping or out-of-range headers and payloads +- path traversal, absolute paths, drive prefixes, backslashes, empty segments, deep paths, and unsafe text +- exact duplicates and file/directory conflicts, plus warning and unavailable-coverage behavior for case/Unicode-normalization collisions +- entry-count, archive-size, member-size, and total-uncompressed limits +- a compressed payload that exceeds its declared or permitted output budget +- CRC mismatch +- ambiguous root, sibling root, missing manifest, and missing skill +- `.codex-plugin`, `.agent-plugin`, and `.claude-plugin` manifest handling +- skills-only exclusions for MCP, app, and screenshot content +- parity between archive-backed and directory-backed submission checks +- text, JSON, Markdown, output contract, completion, exit codes, and output-file equality +- disabled Action behavior, each single selected mode with strict gating, both modes together, and strict gating with neither mode +- no filesystem extraction, process execution, or network request +- no absolute path, unsafe entry name, file content, credential, or decompressed-byte disclosure +- randomized malformed byte inputs that terminate without uncaught exceptions or hangs +- Windows and POSIX path semantics + +Release verification includes the complete existing suite, the archive corpus, TypeScript build, dependency audit and install-script checks, source self-scan, package-size inspection, npm pack, fresh install, and release check. + +## Baseline Portability Prerequisite + +The first implementation task fixes one existing Windows-only test portability defect: `tests/action-metadata.test.ts` compares a multiline Action block with LF-only text while a fresh Windows checkout can contain CRLF. The test will normalize line endings at its assertion boundary without changing `action.yml` or product behavior. The baseline suite must pass before archive feature code begins. + +## Out Of Scope + +- creating, normalizing, repairing, or rewriting ZIP files +- extracting ZIP entries to disk +- MCP-backed archive submission +- portal login, upload, domain verification, OAuth, or credentials +- live MCP execution +- SARIF for archive findings +- customizable archive limits or heuristic quality scoring +- undocumented portal-limit guesses +- directory-acceptance prediction or guarantee From a2b9fc9a76c574896c4f1db63a4e7616094ba2a9 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 13:30:42 +0300 Subject: [PATCH 02/20] test: normalize Action metadata newlines --- tests/action-metadata.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/action-metadata.test.ts b/tests/action-metadata.test.ts index b16d98c..e1fc8ec 100644 --- a/tests/action-metadata.test.ts +++ b/tests/action-metadata.test.ts @@ -8,6 +8,10 @@ import packageJson from "../package.json" with { type: "json" }; const execFileAsync = promisify(execFile); +function normalizeNewlines(value: string): string { + return value.replace(/\r\n/gu, "\n"); +} + async function renderActionManifest(actionMetadata: string, targetPath: string): Promise> { const start = actionMetadata.indexOf(' const fs = require("node:fs");'); const end = actionMetadata.indexOf("\n NODE", start); @@ -214,7 +218,7 @@ describe("GitHub Action metadata", () => { }); it("rejects installed-cache submission preflight requests without producing submission reports", async () => { - const actionMetadata = await readFile("action.yml", "utf8"); + const actionMetadata = normalizeNewlines(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'); From 3a46cd9d04830132f17ca371587b27355e0a85a6 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 13:40:44 +0300 Subject: [PATCH 03/20] build: add bounded ZIP reader dependencies --- package-lock.json | 55 ++++++++++++++++++++++++++++++++++++++++++++++- package.json | 5 ++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index d1f4375..24da815 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,13 +10,16 @@ "license": "MIT", "dependencies": { "fast-xml-parser": "^5.11.0", - "yaml": "^2.9.0" + "iconv-lite": "^0.7.3", + "yaml": "^2.9.0", + "yauzl": "^3.4.0" }, "bin": { "codex-plugin-doctor": "dist/cli.js" }, "devDependencies": { "@types/node": "^24.2.1", + "@types/yauzl": "^3.4.0", "tsx": "^4.20.4", "typescript": "^5.9.2", "vitest": "^3.2.4" @@ -910,6 +913,16 @@ "undici-types": "~7.16.0" } }, + "node_modules/@types/yauzl": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/@types/yauzl/-/yauzl-3.4.0.tgz", + "integrity": "sha512-NRPn5w6h8dhcnmx3YIRQcqMywY/+nND/uOkJessedcrowO3C0AssHp3tMJpxKAwOhFOo0OV1y9VtsC5hbKKBAw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@vitest/expect": { "version": "3.2.6", "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-3.2.6.tgz", @@ -1253,6 +1266,22 @@ "node": "^8.16.0 || ^10.6.0 || >=11.0.0" } }, + "node_modules/iconv-lite": { + "version": "0.7.3", + "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.3.tgz", + "integrity": "sha512-IKXpvIzjnC9XTAUbVBcMfGS0EPaIXtW6v+zr+RRp+hqULEpo0owZax6wyRwPOJbWbzjYspQwusTsfVr0ifh4uQ==", + "license": "MIT", + "dependencies": { + "safer-buffer": ">= 2.1.2 < 3.0.0" + }, + "engines": { + "node": ">=0.10.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/is-unsafe": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/is-unsafe/-/is-unsafe-2.0.0.tgz", @@ -1347,6 +1376,12 @@ "node": ">= 14.16" } }, + "node_modules/pend": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz", + "integrity": "sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==", + "license": "MIT" + }, "node_modules/picocolors": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/picocolors/-/picocolors-1.1.1.tgz", @@ -1441,6 +1476,12 @@ "fsevents": "~2.3.2" } }, + "node_modules/safer-buffer": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", + "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==", + "license": "MIT" + }, "node_modules/siginfo": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/siginfo/-/siginfo-2.0.0.tgz", @@ -1818,6 +1859,18 @@ "funding": { "url": "https://github.com/sponsors/eemeli" } + }, + "node_modules/yauzl": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-3.4.0.tgz", + "integrity": "sha512-jIH9yLR9wqr0wOS0TpBvo/g/2UgZH5qePVbjgRliiF0BYvOZyaBknKsF+x9Iht0O6sqgnB93rCICdOZFecJuDw==", + "license": "MIT", + "dependencies": { + "pend": "~1.2.0" + }, + "engines": { + "node": ">=12" + } } } } diff --git a/package.json b/package.json index 256dcf3..2afc35b 100644 --- a/package.json +++ b/package.json @@ -57,12 +57,15 @@ }, "devDependencies": { "@types/node": "^24.2.1", + "@types/yauzl": "^3.4.0", "tsx": "^4.20.4", "typescript": "^5.9.2", "vitest": "^3.2.4" }, "dependencies": { "fast-xml-parser": "^5.11.0", - "yaml": "^2.9.0" + "iconv-lite": "^0.7.3", + "yaml": "^2.9.0", + "yauzl": "^3.4.0" } } From 0ca799b4f993686bc38935c542982bc16dd7bb14 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 14:08:06 +0300 Subject: [PATCH 04/20] refactor: add submission package reader --- src/core/submission-package-reader.ts | 251 ++++++++++++++++++++++++ tests/submission-package-reader.test.ts | 216 ++++++++++++++++++++ 2 files changed, 467 insertions(+) create mode 100644 src/core/submission-package-reader.ts create mode 100644 tests/submission-package-reader.test.ts diff --git a/src/core/submission-package-reader.ts b/src/core/submission-package-reader.ts new file mode 100644 index 0000000..d458689 --- /dev/null +++ b/src/core/submission-package-reader.ts @@ -0,0 +1,251 @@ +import { + lstat, + readdir, + readFile, + realpath, + stat +} from "node:fs/promises"; +import path from "node:path"; +import type { Stats } from "node:fs"; + +export type SubmissionPackageEntryKind = "file" | "directory" | "symlink" | "other"; + +export interface SubmissionPackageEntry { + packagePath: string; + kind: SubmissionPackageEntryKind; + resolvedKind: Exclude | null; + size: number; + safeResolution: "safe" | "outside" | "unavailable"; +} + +export interface SubmissionPackageReader { + list(directory: string): Promise; + stat(packagePath: string): Promise; + read(packagePath: string, maxBytes: number): Promise; +} + +type ResolvedEntryKind = Exclude; + +const invalidPackagePathMessage = "Invalid package path."; +const invalidMaxBytesMessage = "maxBytes must be a nonnegative safe integer."; + +function packageEntryKind(stats: Stats): SubmissionPackageEntryKind { + if (stats.isFile()) { + return "file"; + } + + if (stats.isDirectory()) { + return "directory"; + } + + if (stats.isSymbolicLink()) { + return "symlink"; + } + + return "other"; +} + +function resolvedEntryKind(stats: Stats): ResolvedEntryKind { + const kind = packageEntryKind(stats); + + return kind === "symlink" ? "other" : kind; +} + +function isWithinRoot(rootPath: string, candidatePath: string): boolean { + const relativePath = path.relative(rootPath, candidatePath); + + return relativePath === "" || ( + relativePath !== ".." + && !relativePath.startsWith(`..${path.sep}`) + && !path.isAbsolute(relativePath) + ); +} + +function normalizePackagePath(packagePath: string, options: { + allowRoot: boolean; + allowTrailingSlash: boolean; +}): string { + if (typeof packagePath !== "string") { + throw new Error(invalidPackagePathMessage); + } + + if (packagePath === "") { + if (options.allowRoot) { + return ""; + } + + throw new Error(invalidPackagePathMessage); + } + + if (/[\u0000-\u001F\u007F]/u.test(packagePath) + || packagePath.startsWith("/") + || packagePath.startsWith("\\") + || /^[a-zA-Z]:/.test(packagePath) + || packagePath.includes("\\")) { + throw new Error(invalidPackagePathMessage); + } + + const pathWithoutTrailingSlash = options.allowTrailingSlash && packagePath.endsWith("/") + ? packagePath.slice(0, -1) + : packagePath; + const segments = pathWithoutTrailingSlash.split("/"); + + if (pathWithoutTrailingSlash === "" + || segments.some((segment) => segment === "" || segment === "." || segment === "..")) { + throw new Error(invalidPackagePathMessage); + } + + return segments.join("/"); +} + +function nativePathFor(rootPath: string, packagePath: string): string | null { + const candidatePath = path.resolve(rootPath, ...packagePath.split("/")); + + return isWithinRoot(rootPath, candidatePath) ? candidatePath : null; +} + +function validMaxBytes(maxBytes: number): boolean { + return Number.isSafeInteger(maxBytes) && maxBytes >= 0; +} + +export function createDirectorySubmissionPackageReader(rootPath: string): SubmissionPackageReader { + const nativeRootPath = path.resolve(rootPath); + + async function canonicalRootPath(): Promise { + try { + return await realpath(nativeRootPath); + } catch { + return null; + } + } + + async function entryFor(packagePath: string): Promise { + const candidatePath = nativePathFor(nativeRootPath, packagePath); + + if (candidatePath === null) { + return null; + } + + try { + const entryStats = await lstat(candidatePath); + const kind = packageEntryKind(entryStats); + const rootCanonicalPath = await canonicalRootPath(); + + if (rootCanonicalPath === null) { + return null; + } + + try { + const canonicalCandidatePath = await realpath(candidatePath); + const safeResolution = isWithinRoot(rootCanonicalPath, canonicalCandidatePath); + + if (!safeResolution) { + return { + packagePath, + kind, + resolvedKind: null, + size: entryStats.size, + safeResolution: "outside" + }; + } + + const targetStats = await stat(candidatePath); + + return { + packagePath, + kind, + resolvedKind: resolvedEntryKind(targetStats), + size: targetStats.size, + safeResolution: "safe" + }; + } catch { + return kind === "symlink" + ? { + packagePath, + kind, + resolvedKind: null, + size: entryStats.size, + safeResolution: "unavailable" + } + : null; + } + } catch { + return null; + } + } + + return { + async list(directory: string): Promise { + const normalizedDirectory = normalizePackagePath(directory, { + allowRoot: true, + allowTrailingSlash: true + }); + const directoryPath = normalizedDirectory === "" + ? nativeRootPath + : nativePathFor(nativeRootPath, normalizedDirectory); + + if (directoryPath === null) { + return []; + } + + try { + if (normalizedDirectory !== "") { + const directoryEntry = await entryFor(normalizedDirectory); + + if (directoryEntry?.resolvedKind !== "directory" || directoryEntry.safeResolution !== "safe") { + return []; + } + } else if (await canonicalRootPath() === null) { + return []; + } + + const childNames = await readdir(directoryPath); + const entries = await Promise.all(childNames.map(async (childName) => entryFor( + normalizedDirectory === "" ? childName : `${normalizedDirectory}/${childName}` + ))); + + return entries + .filter((entry): entry is SubmissionPackageEntry => entry !== null) + .sort((left, right) => left.packagePath.localeCompare(right.packagePath)); + } catch { + return []; + } + }, + + async stat(packagePath: string): Promise { + return entryFor(normalizePackagePath(packagePath, { + allowRoot: false, + allowTrailingSlash: false + })); + }, + + async read(packagePath: string, maxBytes: number): Promise { + const normalizedPackagePath = normalizePackagePath(packagePath, { + allowRoot: false, + allowTrailingSlash: false + }); + + if (!validMaxBytes(maxBytes)) { + throw new Error(invalidMaxBytesMessage); + } + + const entry = await entryFor(normalizedPackagePath); + + if (entry?.resolvedKind !== "file" || entry.safeResolution !== "safe" || entry.size > maxBytes) { + return null; + } + + const candidatePath = nativePathFor(nativeRootPath, normalizedPackagePath); + + if (candidatePath === null) { + return null; + } + + try { + return new Uint8Array(await readFile(candidatePath)); + } catch { + return null; + } + } + }; +} diff --git a/tests/submission-package-reader.test.ts b/tests/submission-package-reader.test.ts new file mode 100644 index 0000000..e322063 --- /dev/null +++ b/tests/submission-package-reader.test.ts @@ -0,0 +1,216 @@ +import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { + createDirectorySubmissionPackageReader, + type SubmissionPackageReader +} from "../src/core/submission-package-reader.js"; + +const temporaryDirectories: string[] = []; +const fileSymlinkIt = process.platform === "win32" ? it.skip : it; + +async function createTemporaryDirectory(prefix: string): Promise { + const directory = await mkdtemp(path.join(os.tmpdir(), prefix)); + temporaryDirectories.push(directory); + return directory; +} + +async function createReaderFixture(): Promise<{ + rootPath: string; + reader: SubmissionPackageReader; +}> { + const rootPath = await createTemporaryDirectory("submission-package-reader-"); + + await Promise.all([ + writeFile(path.join(rootPath, "z-last.txt"), "z"), + writeFile(path.join(rootPath, "a-first.txt"), "first"), + mkdir(path.join(rootPath, "nested")), + mkdir(path.join(rootPath, "empty")) + ]); + await writeFile(path.join(rootPath, "nested", "child.txt"), "nested"); + + return { + rootPath, + reader: createDirectorySubmissionPackageReader(rootPath) + }; +} + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { + recursive: true, + force: true, + maxRetries: 3, + retryDelay: 50 + }))); +}); + +describe("directory submission package reader", () => { + it("lists normalized immediate child package paths in sorted order", async () => { + const { reader } = await createReaderFixture(); + + await expect(reader.list("")) .resolves.toEqual([ + expect.objectContaining({ packagePath: "a-first.txt", kind: "file" }), + expect.objectContaining({ packagePath: "empty", kind: "directory" }), + expect.objectContaining({ packagePath: "nested", kind: "directory" }), + expect.objectContaining({ packagePath: "z-last.txt", kind: "file" }) + ]); + await expect(reader.list("nested/")) .resolves.toEqual([ + expect.objectContaining({ packagePath: "nested/child.txt", kind: "file" }) + ]); + }); + + it("does not recursively include descendants when listing a directory", async () => { + const { reader } = await createReaderFixture(); + + const entries = await reader.list(""); + + expect(entries.map((entry) => entry.packagePath)).not.toContain("nested/child.txt"); + }); + + it("returns null for missing stat and read targets", async () => { + const { reader } = await createReaderFixture(); + + await expect(reader.stat("missing.txt")).resolves.toBeNull(); + await expect(reader.read("missing.txt", 10)).resolves.toBeNull(); + }); + + it("reports file and directory kinds with safe resolution", async () => { + const { reader } = await createReaderFixture(); + + await expect(reader.stat("a-first.txt")).resolves.toEqual({ + packagePath: "a-first.txt", + kind: "file", + resolvedKind: "file", + size: 5, + safeResolution: "safe" + }); + await expect(reader.stat("nested")).resolves.toEqual({ + packagePath: "nested", + kind: "directory", + resolvedKind: "directory", + size: expect.any(Number), + safeResolution: "safe" + }); + }); + + fileSymlinkIt("reports contained symlinks without exposing their native targets", async () => { + const { rootPath, reader } = await createReaderFixture(); + await symlink(path.join(rootPath, "a-first.txt"), path.join(rootPath, "contained-link.txt"), "file"); + + const entry = await reader.stat("contained-link.txt"); + + expect(entry).toEqual({ + packagePath: "contained-link.txt", + kind: "symlink", + resolvedKind: "file", + size: expect.any(Number), + safeResolution: "safe" + }); + expect(JSON.stringify(entry)).not.toContain(rootPath); + await expect(reader.read("contained-link.txt", 5)).resolves.toEqual(new Uint8Array([102, 105, 114, 115, 116])); + }); + + it("marks an external directory junction as outside without reading target metadata", async () => { + const { rootPath, reader } = await createReaderFixture(); + const outsideRoot = await createTemporaryDirectory("submission-package-reader-outside-"); + await writeFile(path.join(outsideRoot, "private.txt"), "outside"); + await symlink(outsideRoot, path.join(rootPath, "outside-link"), "junction"); + + await expect(reader.stat("outside-link")).resolves.toEqual({ + packagePath: "outside-link", + kind: "symlink", + resolvedKind: null, + size: expect.any(Number), + safeResolution: "outside" + }); + await expect(reader.read("outside-link", 100)).resolves.toBeNull(); + }); + + fileSymlinkIt("marks broken links as unavailable", async () => { + const { rootPath, reader } = await createReaderFixture(); + await symlink(path.join(rootPath, "missing-target.txt"), path.join(rootPath, "broken-link.txt"), "file"); + + await expect(reader.stat("broken-link.txt")).resolves.toEqual({ + packagePath: "broken-link.txt", + kind: "symlink", + resolvedKind: null, + size: expect.any(Number), + safeResolution: "unavailable" + }); + }); + + it.each([ + ["../secret"], + ["nested/../a-first.txt"], + ["/absolute.txt"], + ["C:/drive.txt"], + ["\\\\server\\share\\file.txt"], + ["nested//child.txt"], + ["nested/./child.txt"], + ["nested/\u0000child.txt"], + [""] + ])("rejects unsafe stat and read package paths without host-path disclosure: %j", async (packagePath) => { + const { rootPath, reader } = await createReaderFixture(); + + await expect(reader.stat(packagePath)).rejects.toThrow("Invalid package path."); + await expect(reader.read(packagePath, 10)).rejects.toThrow("Invalid package path."); + await expect(reader.stat(packagePath)).rejects.not.toThrow(rootPath); + }); + + it.each([ + ["../secret"], + ["/absolute"], + ["C:/drive"], + ["\\\\server\\share"], + ["nested//child"], + ["nested/./child"], + ["nested/\u0000child"] + ])("rejects unsafe list package paths without host-path disclosure: %j", async (packagePath) => { + const { rootPath, reader } = await createReaderFixture(); + + await expect(reader.list(packagePath)).rejects.toThrow("Invalid package path."); + await expect(reader.list(packagePath)).rejects.not.toThrow(rootPath); + }); + + it("rejects invalid read limits and stops oversize content before reading", async () => { + const { rootPath, reader } = await createReaderFixture(); + const filePath = path.join(rootPath, "a-first.txt"); + + for (const maxBytes of [-1, 1.5, Number.POSITIVE_INFINITY, Number.MAX_SAFE_INTEGER + 1]) { + await expect(reader.read("a-first.txt", maxBytes)).rejects.toThrow( + "maxBytes must be a nonnegative safe integer." + ); + } + + await expect(reader.read("a-first.txt", 4)).resolves.toBeNull(); + await rm(filePath); + await expect(reader.read("a-first.txt", 4)).resolves.toBeNull(); + }); + + it("reads exactly at the size boundary and returns a fresh byte array", async () => { + const { reader } = await createReaderFixture(); + + const firstRead = await reader.read("a-first.txt", 5); + const secondRead = await reader.read("a-first.txt", 5); + + expect(firstRead).toBeInstanceOf(Uint8Array); + expect(firstRead && new TextDecoder().decode(firstRead)).toBe("first"); + expect(secondRead && new TextDecoder().decode(secondRead)).toBe("first"); + if (firstRead === null || secondRead === null) { + throw new Error("Expected readable fixture file."); + } + firstRead[0] = 0; + expect(secondRead[0]).toBe("f".charCodeAt(0)); + }); + + it("keeps native root paths out of successful entries and expected error messages", async () => { + const { rootPath, reader } = await createReaderFixture(); + const entries = await reader.list(""); + + expect(JSON.stringify(entries)).not.toContain(rootPath); + await expect(reader.list("../outside")).rejects.toThrow("Invalid package path."); + }); +}); From 5946c93bd76d3ec2a40e10e7ceaff5916403f207 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 14:16:41 +0300 Subject: [PATCH 05/20] fix: bind submission reader checks to files --- src/core/submission-package-reader.ts | 78 +++++++++++++++++++++++-- tests/submission-package-reader.test.ts | 3 + 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/core/submission-package-reader.ts b/src/core/submission-package-reader.ts index d458689..e6cdebc 100644 --- a/src/core/submission-package-reader.ts +++ b/src/core/submission-package-reader.ts @@ -1,7 +1,7 @@ import { lstat, + open, readdir, - readFile, realpath, stat } from "node:fs/promises"; @@ -119,6 +119,29 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis } } + async function ancestorsStayWithinRoot( + packagePath: string, + rootCanonicalPath: string + ): Promise { + const segments = packagePath.split("/"); + let ancestorPath = nativeRootPath; + + for (const segment of segments.slice(0, -1)) { + ancestorPath = path.join(ancestorPath, segment); + + try { + const canonicalAncestorPath = await realpath(ancestorPath); + if (!isWithinRoot(rootCanonicalPath, canonicalAncestorPath)) { + return false; + } + } catch { + return false; + } + } + + return true; + } + async function entryFor(packagePath: string): Promise { const candidatePath = nativePathFor(nativeRootPath, packagePath); @@ -127,14 +150,16 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis } try { - const entryStats = await lstat(candidatePath); - const kind = packageEntryKind(entryStats); const rootCanonicalPath = await canonicalRootPath(); - if (rootCanonicalPath === null) { + if (rootCanonicalPath === null + || !(await ancestorsStayWithinRoot(packagePath, rootCanonicalPath))) { return null; } + const entryStats = await lstat(candidatePath); + const kind = packageEntryKind(entryStats); + try { const canonicalCandidatePath = await realpath(candidatePath); const safeResolution = isWithinRoot(rootCanonicalPath, canonicalCandidatePath); @@ -241,10 +266,53 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis return null; } + let fileHandle: Awaited> | null = null; + try { - return new Uint8Array(await readFile(candidatePath)); + fileHandle = await open(candidatePath, "r"); + const handleStats = await fileHandle.stat(); + + if (!handleStats.isFile() + || !Number.isSafeInteger(handleStats.size) + || handleStats.size < 0 + || handleStats.size > maxBytes) { + return null; + } + + const rootCanonicalPath = await canonicalRootPath(); + const canonicalCandidatePath = await realpath(candidatePath); + const pathStats = await stat(candidatePath); + + if (rootCanonicalPath === null + || !isWithinRoot(rootCanonicalPath, canonicalCandidatePath) + || !pathStats.isFile() + || handleStats.dev !== pathStats.dev + || handleStats.ino !== pathStats.ino) { + return null; + } + + const content = Buffer.alloc(handleStats.size); + let offset = 0; + + while (offset < content.length) { + const { bytesRead } = await fileHandle.read(content, offset, content.length - offset, offset); + if (bytesRead === 0) { + return null; + } + offset += bytesRead; + } + + const probe = Buffer.alloc(1); + const { bytesRead: extraBytesRead } = await fileHandle.read(probe, 0, 1, offset); + if (extraBytesRead !== 0) { + return null; + } + + return Uint8Array.from(content); } catch { return null; + } finally { + await fileHandle?.close().catch(() => undefined); } } }; diff --git a/tests/submission-package-reader.test.ts b/tests/submission-package-reader.test.ts index e322063..dfcbbfa 100644 --- a/tests/submission-package-reader.test.ts +++ b/tests/submission-package-reader.test.ts @@ -127,6 +127,9 @@ describe("directory submission package reader", () => { safeResolution: "outside" }); await expect(reader.read("outside-link", 100)).resolves.toBeNull(); + await expect(reader.stat("outside-link/private.txt")).resolves.toBeNull(); + await expect(reader.read("outside-link/private.txt", 100)).resolves.toBeNull(); + await expect(reader.list("outside-link")).resolves.toEqual([]); }); fileSymlinkIt("marks broken links as unavailable", async () => { From 1fa471c37c4ed4e6d32661dc859d36b1919faf13 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 14:22:58 +0300 Subject: [PATCH 06/20] fix: filter unsafe submission directory entries --- src/core/submission-package-reader.ts | 18 ++++++++++++++--- tests/submission-package-reader.test.ts | 26 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/core/submission-package-reader.ts b/src/core/submission-package-reader.ts index e6cdebc..6f8734c 100644 --- a/src/core/submission-package-reader.ts +++ b/src/core/submission-package-reader.ts @@ -225,9 +225,21 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis } const childNames = await readdir(directoryPath); - const entries = await Promise.all(childNames.map(async (childName) => entryFor( - normalizedDirectory === "" ? childName : `${normalizedDirectory}/${childName}` - ))); + const entries = await Promise.all(childNames.map(async (childName) => { + const childPackagePath = normalizedDirectory === "" + ? childName + : `${normalizedDirectory}/${childName}`; + + try { + const normalizedChildPath = normalizePackagePath(childPackagePath, { + allowRoot: false, + allowTrailingSlash: false + }); + return entryFor(normalizedChildPath); + } catch { + return null; + } + })); return entries .filter((entry): entry is SubmissionPackageEntry => entry !== null) diff --git a/tests/submission-package-reader.test.ts b/tests/submission-package-reader.test.ts index dfcbbfa..a60d7ce 100644 --- a/tests/submission-package-reader.test.ts +++ b/tests/submission-package-reader.test.ts @@ -11,6 +11,7 @@ import { const temporaryDirectories: string[] = []; const fileSymlinkIt = process.platform === "win32" ? it.skip : it; +const unsafeNameIt = process.platform === "linux" ? it : it.skip; async function createTemporaryDirectory(prefix: string): Promise { const directory = await mkdtemp(path.join(os.tmpdir(), prefix)); @@ -70,6 +71,31 @@ describe("directory submission package reader", () => { expect(entries.map((entry) => entry.packagePath)).not.toContain("nested/child.txt"); }); + it("returns list entries that round-trip through stat", async () => { + const { reader } = await createReaderFixture(); + const entries = await reader.list(""); + + for (const entry of entries) { + await expect(reader.stat(entry.packagePath)).resolves.not.toBeNull(); + } + }); + + unsafeNameIt("filters unsafe native child names from package listings", async () => { + const { rootPath, reader } = await createReaderFixture(); + const unsafeNames = [ + "a\\b", + "C:entry", + `control${String.fromCharCode(1)}entry` + ]; + + await Promise.all(unsafeNames.map(async (name) => writeFile(path.join(rootPath, name), "unsafe"))); + + const listedPaths = (await reader.list("")).map((entry) => entry.packagePath); + for (const unsafeName of unsafeNames) { + expect(listedPaths).not.toContain(unsafeName); + } + }); + it("returns null for missing stat and read targets", async () => { const { reader } = await createReaderFixture(); From 992cde6262eb70e4448aeb7ca34644cf68d1c775 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 14:32:37 +0300 Subject: [PATCH 07/20] perf: bound submission directory listing --- src/core/submission-package-reader.ts | 40 ++++++++++++++++--------- tests/submission-package-reader.test.ts | 24 +++++++++++++++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/core/submission-package-reader.ts b/src/core/submission-package-reader.ts index 6f8734c..7519ef4 100644 --- a/src/core/submission-package-reader.ts +++ b/src/core/submission-package-reader.ts @@ -225,21 +225,33 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis } const childNames = await readdir(directoryPath); - const entries = await Promise.all(childNames.map(async (childName) => { - const childPackagePath = normalizedDirectory === "" - ? childName - : `${normalizedDirectory}/${childName}`; - - try { - const normalizedChildPath = normalizePackagePath(childPackagePath, { - allowRoot: false, - allowTrailingSlash: false - }); - return entryFor(normalizedChildPath); - } catch { - return null; + const entries: Array = new Array(childNames.length).fill(null); + let nextChildIndex = 0; + + async function readNextChild(): Promise { + while (nextChildIndex < childNames.length) { + const childIndex = nextChildIndex; + nextChildIndex += 1; + const childName = childNames[childIndex]; + const childPackagePath = normalizedDirectory === "" + ? childName + : `${normalizedDirectory}/${childName}`; + + try { + const normalizedChildPath = normalizePackagePath(childPackagePath, { + allowRoot: false, + allowTrailingSlash: false + }); + entries[childIndex] = await entryFor(normalizedChildPath); + } catch { + entries[childIndex] = null; + } } - })); + } + + const workerCount = Math.min(16, childNames.length); + const workers = Array.from({ length: workerCount }, async () => readNextChild()); + await Promise.all(workers); return entries .filter((entry): entry is SubmissionPackageEntry => entry !== null) diff --git a/tests/submission-package-reader.test.ts b/tests/submission-package-reader.test.ts index a60d7ce..0248c3a 100644 --- a/tests/submission-package-reader.test.ts +++ b/tests/submission-package-reader.test.ts @@ -71,6 +71,30 @@ describe("directory submission package reader", () => { expect(entries.map((entry) => entry.packagePath)).not.toContain("nested/child.txt"); }); + it("returns complete sorted listings with bounded metadata workers", async () => { + const { rootPath, reader } = await createReaderFixture(); + const bulkNames = Array.from( + { length: 300 }, + (_, index) => `bulk-${String(index).padStart(3, "0")}.txt` + ); + + for (const name of bulkNames) { + await writeFile(path.join(rootPath, name), name); + } + + const listedPaths = (await reader.list("")).map((entry) => entry.packagePath); + const expectedPaths = [ + "a-first.txt", + "empty", + "nested", + "z-last.txt", + ...bulkNames + ].sort((left, right) => left.localeCompare(right)); + + expect(listedPaths).toEqual(expectedPaths); + expect(new Set(listedPaths).size).toBe(listedPaths.length); + }); + it("returns list entries that round-trip through stat", async () => { const { reader } = await createReaderFixture(); const entries = await reader.list(""); From 0c8a906666e0dab98ede433ec83f2f7164359792 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 14:56:12 +0300 Subject: [PATCH 08/20] refactor: validate submission assets through readers --- src/core/submission-assets.ts | 87 ++++++++++++------- src/core/submission-package-reader.ts | 26 ++++-- tests/helpers/submission-memory-reader.ts | 101 ++++++++++++++++++++++ tests/submission-assets.test.ts | 77 ++++++++++++++++- tests/submission-package-reader.test.ts | 8 +- 5 files changed, 260 insertions(+), 39 deletions(-) create mode 100644 tests/helpers/submission-memory-reader.ts diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index 2cdef6f..2de53e7 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -1,11 +1,10 @@ -import { readFile, stat } from "node:fs/promises"; import path from "node:path"; import { inflateSync } from "node:zlib"; import { XMLParser, XMLValidator } from "fast-xml-parser"; -import type { DiscoveredPackage } from "../domain/types.js"; -import { resolveSafePackagePath } from "./plugin-components.js"; +import type { DiscoveredPackage, PluginManifest } from "../domain/types.js"; +import { createDirectorySubmissionPackageReader, type SubmissionPackageReader } from "./submission-package-reader.js"; import type { SubmissionFinding } from "./submission-preflight.js"; const maxAssetBytes = 5 * 1024 * 1024; @@ -350,68 +349,94 @@ function dimensionFinding(field: string, packagePath: string, format: AssetForma return null; } -async function validateAsset(rootPath: string, field: typeof assetFields[number], value: unknown): Promise { +function assetPackagePath(value: string): string | null { + if (!value.startsWith("./")) return null; + const packagePath = value.slice(2); + if (packagePath === "" + || packagePath.startsWith("/") + || /^[a-zA-Z]:/u.test(packagePath) + || packagePath.includes("\\") + || /[\u0000-\u001F\u007F]/u.test(packagePath)) { + return null; + } + const segments = packagePath.split("/"); + return segments.some((segment) => segment === "" || segment === "." || segment === "..") ? null : packagePath; +} + +async function validateAsset( + reader: SubmissionPackageReader, + 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) { + const packagePath = assetPackagePath(value); + if (packagePath === 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"); + const extension = extensions.get(path.extname(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))]; + return [finding("plugin.submission.asset.unsupported_format", "Branding asset format is unsupported.", assetEvidence(field, 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))]; + const details = await reader.stat(packagePath).catch(() => null); + if (details === null) { + return [finding("plugin.submission.asset.missing", "Branding asset is missing.", assetEvidence(field, packagePath, extension))]; + } + if (details.safeResolution !== "safe") { + return [finding("plugin.submission.asset.invalid_path", "Branding asset path is invalid.", assetEvidence(field))]; } - if (!details.isFile()) { - return [finding("plugin.submission.asset.unsupported_format", "Branding asset must be a regular file.", assetEvidence(field, resolved.packagePath, extension))]; + if (details.resolvedKind !== "file") { + return [finding("plugin.submission.asset.unsupported_format", "Branding asset must be a regular file.", assetEvidence(field, 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 })]; + return [finding("plugin.submission.asset.too_large", "Branding asset exceeds the size limit.", { ...assetEvidence(field, 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))]; + const content = await reader.read(packagePath, maxAssetBytes).catch(() => null); + if (content === null) { + return [finding("plugin.submission.asset.missing", "Branding asset cannot be read.", assetEvidence(field, 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))]; + return [finding("plugin.submission.asset.unsafe_svg", "SVG must be valid UTF-8.", assetEvidence(field, 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))]; + return [finding("plugin.submission.asset.unsafe_svg", "SVG is unsafe or lacks valid dimensions.", assetEvidence(field, packagePath, extension))]; } - const invalidDimensions = dimensionFinding(field, resolved.packagePath, extension, dimensions); + const invalidDimensions = dimensionFinding(field, 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))]; + return [finding("plugin.submission.asset.decode_failed", "Branding asset could not be decoded.", assetEvidence(field, 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))]; + return [finding("plugin.submission.asset.extension_mismatch", "Branding asset extension does not match its content.", assetEvidence(field, packagePath, decoded.format, decoded.dimensions))]; } - const invalidDimensions = dimensionFinding(field, resolved.packagePath, decoded.format, decoded.dimensions); + const invalidDimensions = dimensionFinding(field, packagePath, decoded.format, decoded.dimensions); return invalidDimensions === null ? [] : [invalidDimensions]; } -export async function validateSubmissionAssets(discoveredPackage: DiscoveredPackage): Promise { - const listing = discoveredPackage.manifest.interface; - const interfaceValues = isRecord(listing) ? listing : {}; +export async function validateSubmissionAssetsFromReader( + manifest: PluginManifest, + reader: SubmissionPackageReader +): Promise { + const interfaceValues = isRecord(manifest.interface) ? manifest.interface : {}; const findings: SubmissionFinding[] = []; - for (const field of assetFields) findings.push(...await validateAsset(discoveredPackage.rootPath, field, interfaceValues[field])); + for (const field of assetFields) findings.push(...await validateAsset(reader, field, interfaceValues[field])); return { findings }; } + +export async function validateSubmissionAssets(discoveredPackage: DiscoveredPackage): Promise { + return validateSubmissionAssetsFromReader( + discoveredPackage.manifest, + createDirectorySubmissionPackageReader(discoveredPackage.rootPath) + ); +} diff --git a/src/core/submission-package-reader.ts b/src/core/submission-package-reader.ts index 7519ef4..6f310f1 100644 --- a/src/core/submission-package-reader.ts +++ b/src/core/submission-package-reader.ts @@ -25,6 +25,7 @@ export interface SubmissionPackageReader { } type ResolvedEntryKind = Exclude; +type AncestorResolution = "safe" | "outside" | "unavailable"; const invalidPackagePathMessage = "Invalid package path."; const invalidMaxBytesMessage = "maxBytes must be a nonnegative safe integer."; @@ -122,7 +123,7 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis async function ancestorsStayWithinRoot( packagePath: string, rootCanonicalPath: string - ): Promise { + ): Promise { const segments = packagePath.split("/"); let ancestorPath = nativeRootPath; @@ -132,14 +133,14 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis try { const canonicalAncestorPath = await realpath(ancestorPath); if (!isWithinRoot(rootCanonicalPath, canonicalAncestorPath)) { - return false; + return "outside"; } } catch { - return false; + return "unavailable"; } } - return true; + return "safe"; } async function entryFor(packagePath: string): Promise { @@ -152,8 +153,21 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis try { const rootCanonicalPath = await canonicalRootPath(); - if (rootCanonicalPath === null - || !(await ancestorsStayWithinRoot(packagePath, rootCanonicalPath))) { + if (rootCanonicalPath === null) { + return null; + } + + const ancestorResolution = await ancestorsStayWithinRoot(packagePath, rootCanonicalPath); + if (ancestorResolution === "outside") { + return { + packagePath, + kind: "other", + resolvedKind: null, + size: 0, + safeResolution: "outside" + }; + } + if (ancestorResolution === "unavailable") { return null; } diff --git a/tests/helpers/submission-memory-reader.ts b/tests/helpers/submission-memory-reader.ts new file mode 100644 index 0000000..eebad25 --- /dev/null +++ b/tests/helpers/submission-memory-reader.ts @@ -0,0 +1,101 @@ +import type { + SubmissionPackageEntry, + SubmissionPackageEntryKind, + SubmissionPackageReader +} from "../../src/core/submission-package-reader.js"; + +type ResolvedEntryKind = Exclude; + +export interface MemorySubmissionPackageEntry { + content?: string | Uint8Array; + kind?: SubmissionPackageEntryKind; + resolvedKind?: ResolvedEntryKind | null; + safeResolution?: SubmissionPackageEntry["safeResolution"]; + size?: number; +} + +const invalidPackagePathMessage = "Invalid package path."; + +function normalizePackagePath( + packagePath: string, + allowRoot: boolean, + allowTrailingSlash = false +): string { + if (typeof packagePath !== "string") throw new Error(invalidPackagePathMessage); + if (packagePath === "") { + if (allowRoot) return ""; + throw new Error(invalidPackagePathMessage); + } + if (/[\u0000-\u001F\u007F]/u.test(packagePath) + || packagePath.startsWith("/") + || packagePath.startsWith("\\") + || /^[a-zA-Z]:/u.test(packagePath) + || packagePath.includes("\\")) { + throw new Error(invalidPackagePathMessage); + } + const pathWithoutTrailingSlash = allowTrailingSlash && packagePath.endsWith("/") + ? packagePath.slice(0, -1) + : packagePath; + const segments = pathWithoutTrailingSlash.split("/"); + if (segments.some((segment) => segment === "" || segment === "." || segment === "..")) { + throw new Error(invalidPackagePathMessage); + } + return pathWithoutTrailingSlash; +} + +function bytesFor(entry: MemorySubmissionPackageEntry): Uint8Array | null { + if (entry.content === undefined) return null; + if (typeof entry.content === "string") return new TextEncoder().encode(entry.content); + return Uint8Array.from(entry.content); +} + +function entryFor(packagePath: string, source: MemorySubmissionPackageEntry): SubmissionPackageEntry { + const content = bytesFor(source); + const kind = source.kind ?? (content === null ? "other" : "file"); + return { + packagePath, + kind, + resolvedKind: source.resolvedKind ?? (kind === "symlink" ? null : kind), + size: source.size ?? content?.byteLength ?? 0, + safeResolution: source.safeResolution ?? "safe" + }; +} + +export function createMemorySubmissionPackageReader( + source: Readonly> +): SubmissionPackageReader { + const entries = new Map(); + for (const [packagePath, entry] of Object.entries(source)) { + entries.set(normalizePackagePath(packagePath, false), entry); + } + + return { + async list(directory: string): Promise { + const normalizedDirectory = normalizePackagePath(directory, true, true); + const prefix = normalizedDirectory === "" ? "" : `${normalizedDirectory}/`; + return [...entries] + .filter(([packagePath]) => packagePath.startsWith(prefix) && !packagePath.slice(prefix.length).includes("/")) + .map(([packagePath, entry]) => entryFor(packagePath, entry)) + .sort((left, right) => left.packagePath.localeCompare(right.packagePath)); + }, + + async stat(packagePath: string): Promise { + const normalizedPackagePath = normalizePackagePath(packagePath, false); + const entry = entries.get(normalizedPackagePath); + return entry === undefined ? null : entryFor(normalizedPackagePath, entry); + }, + + async read(packagePath: string, maxBytes: number): Promise { + const normalizedPackagePath = normalizePackagePath(packagePath, false); + if (!Number.isSafeInteger(maxBytes) || maxBytes < 0) { + throw new Error("maxBytes must be a nonnegative safe integer."); + } + const sourceEntry = entries.get(normalizedPackagePath); + if (sourceEntry === undefined) return null; + const entry = entryFor(normalizedPackagePath, sourceEntry); + const content = bytesFor(sourceEntry); + if (entry.resolvedKind !== "file" || entry.safeResolution !== "safe" || entry.size > maxBytes || content === null) return null; + return Uint8Array.from(content); + } + }; +} diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index 11ca2b8..e983132 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -5,7 +5,9 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import type { DiscoveredPackage } from "../src/domain/types.js"; -import { type SubmissionAssetResult, validateSubmissionAssets } from "../src/core/submission-assets.js"; +import { createDirectorySubmissionPackageReader } from "../src/core/submission-package-reader.js"; +import { type SubmissionAssetResult, validateSubmissionAssets, validateSubmissionAssetsFromReader } from "../src/core/submission-assets.js"; +import { createMemorySubmissionPackageReader, type MemorySubmissionPackageEntry } from "./helpers/submission-memory-reader.js"; type AssetFiles = Record; @@ -355,3 +357,76 @@ describe("submission assets", () => { expect(source).not.toMatch(/\b(fetch|exec|spawn|http|https)\b/u); }); }); +describe("submission asset reader parity", () => { + async function expectReaderParity( + interfaceValues: Record, + files: AssetFiles = {}, + entries: Readonly> = Object.fromEntries( + Object.entries(files).map(([packagePath, content]) => [packagePath, { content }]) + ) + ): Promise { + const discoveredPackage = await packageWithAssets(interfaceValues, files); + const directoryResult = await validateSubmissionAssets(discoveredPackage); + const readerResult = await validateSubmissionAssetsFromReader(discoveredPackage.manifest, createMemorySubmissionPackageReader(entries)); + expect(readerResult.findings).toEqual(directoryResult.findings); + } + + it.each([ + ["PNG", "./logo.png", png(48, 48)], + ["JPEG", "./logo.jpg", jpeg(48, 48)], + ["WebP", "./logo.webp", webp("VP8X", 48, 48)], + ["SVG", "./logo.svg", svg('width="48" height="48"')] + ])("matches the directory wrapper for valid %s", async (_name, assetPath, content) => { + await expectReaderParity({ logo: assetPath, composerIcon: assetPath }, { [assetPath.slice(2)]: content }); + }); + + it.each([ + ["missing", { logo: "./missing.png", composerIcon: "./missing.png" }, {}], + ["non-dot-relative", { logo: "logo.png", composerIcon: "./logo.png" }, { "logo.png": png(48, 48) }], + ["lexical traversal", { logo: "./assets/../logo.png", composerIcon: "./logo.png" }, { "logo.png": png(48, 48) }], + ["unsupported extension", { logo: "./logo.gif", composerIcon: "./logo.gif" }, { "logo.gif": png(48, 48) }], + ["decode failure", { logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": new Uint8Array([1, 2, 3]) }], + ["extension mismatch", { logo: "./logo.jpg", composerIcon: "./logo.jpg" }, { "logo.jpg": png(48, 48) }], + ["non-square dimensions", { logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": png(48, 49) }], + ["unsafe SVG", { logo: "./logo.svg", composerIcon: "./logo.svg" }, { "logo.svg": '' }] + ])("matches the directory wrapper for %s", async (_name, interfaceValues, files) => { + await expectReaderParity(interfaceValues, files as AssetFiles); + }); + + it("matches directory/non-file and size-gated results", async () => { + const discoveredPackage = await packageWithAssets( + { logo: "./assets.png", composerIcon: "./oversized.png" }, + { "oversized.png": new Uint8Array(5 * 1024 * 1024 + 1) } + ); + await mkdir(path.join(discoveredPackage.rootPath, "assets.png")); + const readerResult = await validateSubmissionAssetsFromReader(discoveredPackage.manifest, createMemorySubmissionPackageReader({ + "assets.png": { kind: "directory", resolvedKind: "directory" }, + "oversized.png": { content: new Uint8Array([1]), size: 5 * 1024 * 1024 + 1 } + })); + expect(readerResult.findings).toEqual((await validateSubmissionAssets(discoveredPackage)).findings); + }); + + it("matches invalid-path evidence for exact and descendant external junctions", async () => { + const discoveredPackage = await packageWithAssets({ logo: "./outside.png", composerIcon: "./outside.png/logo.png" }); + const external = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-submission-assets-reader-outside-")); + await writeFile(path.join(external, "logo.png"), png(48, 48)); + await symlink(external, path.join(discoveredPackage.rootPath, "outside.png"), "junction"); + const result = await validateSubmissionAssetsFromReader(discoveredPackage.manifest, createDirectorySubmissionPackageReader(discoveredPackage.rootPath)); + expect(result.findings).toEqual((await validateSubmissionAssets(discoveredPackage)).findings); + expect(JSON.stringify(result)).not.toContain(external); + }); + + it("requires exact interface fields and keeps host paths and content private", async () => { + const reader = createMemorySubmissionPackageReader({ "secret.png": { content: "private-payload" } }); + await expect(validateSubmissionAssetsFromReader({ interface: null }, reader)).resolves.toEqual({ + findings: [ + expect.objectContaining({ id: "plugin.submission.asset.required", evidence: { field: "logo" } }), + expect.objectContaining({ id: "plugin.submission.asset.required", evidence: { field: "composerIcon" } }) + ] + }); + const discoveredPackage = await packageWithAssets({ logo: "./secret.png", composerIcon: "./secret.png" }, { "secret.png": "private-payload" }); + const result = await validateSubmissionAssetsFromReader(discoveredPackage.manifest, reader); + expect(JSON.stringify(result)).not.toContain(discoveredPackage.rootPath); + expect(JSON.stringify(result)).not.toContain("private-payload"); + }); +}); diff --git a/tests/submission-package-reader.test.ts b/tests/submission-package-reader.test.ts index 0248c3a..4bafdba 100644 --- a/tests/submission-package-reader.test.ts +++ b/tests/submission-package-reader.test.ts @@ -177,7 +177,13 @@ describe("directory submission package reader", () => { safeResolution: "outside" }); await expect(reader.read("outside-link", 100)).resolves.toBeNull(); - await expect(reader.stat("outside-link/private.txt")).resolves.toBeNull(); + await expect(reader.stat("outside-link/private.txt")).resolves.toEqual({ + packagePath: "outside-link/private.txt", + kind: "other", + resolvedKind: null, + size: 0, + safeResolution: "outside" + }); await expect(reader.read("outside-link/private.txt", 100)).resolves.toBeNull(); await expect(reader.list("outside-link")).resolves.toEqual([]); }); From 7a91e30a2b3c7ce565ece595377a2ec09d01ba7f Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 15:04:27 +0300 Subject: [PATCH 09/20] fix: preserve asset reader parity --- src/core/submission-assets.ts | 5 ++- tests/helpers/submission-memory-reader.ts | 7 ++++- tests/submission-assets.test.ts | 38 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index 2de53e7..47e3571 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -386,9 +386,12 @@ async function validateAsset( if (details === null) { return [finding("plugin.submission.asset.missing", "Branding asset is missing.", assetEvidence(field, packagePath, extension))]; } - if (details.safeResolution !== "safe") { + if (details.safeResolution === "outside") { return [finding("plugin.submission.asset.invalid_path", "Branding asset path is invalid.", assetEvidence(field))]; } + if (details.safeResolution !== "safe") { + return [finding("plugin.submission.asset.missing", "Branding asset is missing.", assetEvidence(field, packagePath, extension))]; + } if (details.resolvedKind !== "file") { return [finding("plugin.submission.asset.unsupported_format", "Branding asset must be a regular file.", assetEvidence(field, packagePath, extension))]; } diff --git a/tests/helpers/submission-memory-reader.ts b/tests/helpers/submission-memory-reader.ts index eebad25..7a1f734 100644 --- a/tests/helpers/submission-memory-reader.ts +++ b/tests/helpers/submission-memory-reader.ts @@ -94,7 +94,12 @@ export function createMemorySubmissionPackageReader( if (sourceEntry === undefined) return null; const entry = entryFor(normalizedPackagePath, sourceEntry); const content = bytesFor(sourceEntry); - if (entry.resolvedKind !== "file" || entry.safeResolution !== "safe" || entry.size > maxBytes || content === null) return null; + if (entry.resolvedKind !== "file" + || entry.safeResolution !== "safe" + || entry.size > maxBytes + || content === null + || content.byteLength > maxBytes + || entry.size !== content.byteLength) return null; return Uint8Array.from(content); } }; diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index e983132..450b3f1 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -416,6 +416,44 @@ describe("submission asset reader parity", () => { expect(JSON.stringify(result)).not.toContain(external); }); + it("preserves missing findings for unavailable asset links", async () => { + const result = await validateSubmissionAssetsFromReader({ + interface: { + logo: "./broken.png", + composerIcon: "./icon.png" + } + }, createMemorySubmissionPackageReader({ + "broken.png": { + kind: "symlink", + resolvedKind: null, + safeResolution: "unavailable" + }, + "icon.png": { content: png(48, 48) } + })); + + expect(result.findings).toEqual([{ + id: "plugin.submission.asset.missing", + severity: "fail", + message: "Branding asset is missing.", + evidence: { field: "logo", path: "broken.png", format: "png" } + }]); + }); + + it("keeps memory-reader byte bounds independent of declared metadata", async () => { + const reader = createMemorySubmissionPackageReader({ + "mismatch.bin": { content: new Uint8Array([1, 2]), size: 1 }, + "exact.bin": { content: new Uint8Array([3, 4]), size: 2 } + }); + + await expect(reader.read("mismatch.bin", 1)).resolves.toBeNull(); + await expect(reader.read("mismatch.bin", 2)).resolves.toBeNull(); + const first = await reader.read("exact.bin", 2); + const second = await reader.read("exact.bin", 2); + expect(first).toEqual(new Uint8Array([3, 4])); + expect(second).toEqual(new Uint8Array([3, 4])); + expect(first).not.toBe(second); + }); + it("requires exact interface fields and keeps host paths and content private", async () => { const reader = createMemorySubmissionPackageReader({ "secret.png": { content: "private-payload" } }); await expect(validateSubmissionAssetsFromReader({ interface: null }, reader)).resolves.toEqual({ From 7e44b940774e96da7c23a5beb789242cdab81307 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 15:13:16 +0300 Subject: [PATCH 10/20] fix: preserve contained asset paths --- src/core/submission-assets.ts | 9 +++++++-- tests/submission-assets.test.ts | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/core/submission-assets.ts b/src/core/submission-assets.ts index 47e3571..26e4641 100644 --- a/src/core/submission-assets.ts +++ b/src/core/submission-assets.ts @@ -359,8 +359,13 @@ function assetPackagePath(value: string): string | null { || /[\u0000-\u001F\u007F]/u.test(packagePath)) { return null; } - const segments = packagePath.split("/"); - return segments.some((segment) => segment === "" || segment === "." || segment === "..") ? null : packagePath; + const normalizedPackagePath = path.posix.normalize(packagePath); + if (normalizedPackagePath === "." + || normalizedPackagePath === "" + || normalizedPackagePath === ".." + || normalizedPackagePath.startsWith("../") + || path.posix.isAbsolute(normalizedPackagePath)) return null; + return normalizedPackagePath; } async function validateAsset( diff --git a/tests/submission-assets.test.ts b/tests/submission-assets.test.ts index 450b3f1..edcfcc2 100644 --- a/tests/submission-assets.test.ts +++ b/tests/submission-assets.test.ts @@ -380,10 +380,26 @@ describe("submission asset reader parity", () => { await expectReaderParity({ logo: assetPath, composerIcon: assetPath }, { [assetPath.slice(2)]: content }); }); + it("preserves contained dot-segment asset paths", async () => { + const content = png(48, 48); + const interfaceValues = { + logo: "./assets/../logo.png", + composerIcon: "./assets/../logo.png" + }; + const discoveredPackage = await packageWithAssets(interfaceValues, { "logo.png": content }); + const directoryResult = await validateSubmissionAssets(discoveredPackage); + const readerResult = await validateSubmissionAssetsFromReader( + discoveredPackage.manifest, + createMemorySubmissionPackageReader({ "logo.png": { content } }) + ); + + expect(directoryResult.findings).toEqual([]); + expect(readerResult.findings).toEqual([]); + }); + it.each([ ["missing", { logo: "./missing.png", composerIcon: "./missing.png" }, {}], ["non-dot-relative", { logo: "logo.png", composerIcon: "./logo.png" }, { "logo.png": png(48, 48) }], - ["lexical traversal", { logo: "./assets/../logo.png", composerIcon: "./logo.png" }, { "logo.png": png(48, 48) }], ["unsupported extension", { logo: "./logo.gif", composerIcon: "./logo.gif" }, { "logo.gif": png(48, 48) }], ["decode failure", { logo: "./logo.png", composerIcon: "./logo.png" }, { "logo.png": new Uint8Array([1, 2, 3]) }], ["extension mismatch", { logo: "./logo.jpg", composerIcon: "./logo.jpg" }, { "logo.jpg": png(48, 48) }], From e57c3f5e2e5825f1617d0a95f921681ac0f3bc75 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 15:44:48 +0300 Subject: [PATCH 11/20] refactor: validate submission skills through readers --- src/core/submission-package-reader.ts | 4 + src/core/submission-skill-metadata.ts | 283 +++++++++------------- tests/helpers/submission-memory-reader.ts | 2 + tests/submission-package-reader.test.ts | 1 + tests/submission-skill-metadata.test.ts | 81 ++++++- 5 files changed, 199 insertions(+), 172 deletions(-) diff --git a/src/core/submission-package-reader.ts b/src/core/submission-package-reader.ts index 6f310f1..d5c956a 100644 --- a/src/core/submission-package-reader.ts +++ b/src/core/submission-package-reader.ts @@ -14,6 +14,7 @@ export interface SubmissionPackageEntry { packagePath: string; kind: SubmissionPackageEntryKind; resolvedKind: Exclude | null; + resolvedPackagePath?: string; size: number; safeResolution: "safe" | "outside" | "unavailable"; } @@ -190,8 +191,11 @@ export function createDirectorySubmissionPackageReader(rootPath: string): Submis const targetStats = await stat(candidatePath); + const resolvedPackagePath = path.relative(rootCanonicalPath, canonicalCandidatePath).split(path.sep).join("/"); + return { packagePath, + ...(resolvedPackagePath === packagePath ? {} : { resolvedPackagePath }), kind, resolvedKind: resolvedEntryKind(targetStats), size: targetStats.size, diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index 55e7ef1..e3b8fa2 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -1,14 +1,15 @@ -import { lstat, opendir, 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"; +import type { DiscoveredPackage, PluginManifest } from "../domain/types.js"; +import { createDirectorySubmissionPackageReader, type SubmissionPackageEntry, type SubmissionPackageReader } from "./submission-package-reader.js"; +import type { SubmissionFinding, SubmissionPreflightReport } from "./submission-preflight.js"; -type TargetType = "skills-only" | "mcp-backed"; +type TargetType = SubmissionPreflightReport["targetType"]; type Evidence = SubmissionFinding["evidence"]; type Metadata = Record; +type AggregateRead = { kind: "source"; source: string } | { kind: "invalid" } | { kind: "budget"; nextBytes: number }; const maxSkillBytes = 1024 * 1024; const maxAgentBytes = 256 * 1024; @@ -27,27 +28,16 @@ export interface SubmissionSkillMetadataResult { 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)); +interface AggregateBudget { + bytes: number; } -function packagePath(rootPath: string, targetPath: string): string { - return path.relative(rootPath, targetPath).split(path.sep).join("/"); +function isRecord(value: unknown): value is Metadata { + return typeof value === "object" && value !== null && !Array.isArray(value); } -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 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 { @@ -73,23 +63,23 @@ function parseSafeYaml(source: string): { value: Metadata } | { 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] }; + return !match || match[2].trim().length === 0 ? null : { 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; - } +function skillsRoot(value: unknown): string | null { + if (typeof value !== "string" || !value.startsWith("./") || value.includes("\\") || /[\u0000-\u001F\u007F]/u.test(value)) return null; + const normalized = path.posix.normalize(value.slice(2)).replace(/\/$/u, ""); + return normalized === "skills" ? normalized : null; } -type AggregateRead = { kind: "source"; source: string } | { kind: "invalid" } | { kind: "budget"; nextBytes: number }; +function relativePackagePath(base: string, value: string): string | null { + if (path.posix.isAbsolute(value) || /^[A-Za-z]:[\\/]/u.test(value)) return null; + const resolved = path.posix.normalize(path.posix.join(base, value)); + return resolved === ".." || resolved.startsWith("../") || resolved === "." ? null : resolved; +} -interface AggregateBudget { - bytes: number; +function isWithinPackagePath(root: string, candidate: string): boolean { + return candidate === root || candidate.startsWith(`${root}/`); } function budgetFinding(bytes: number): SubmissionFinding { @@ -98,14 +88,20 @@ function budgetFinding(bytes: number): SubmissionFinding { }); } -async function readSafeUtf8(filePath: string, maximum: number, budget: AggregateBudget): Promise { +async function safeStat(reader: SubmissionPackageReader, packagePath: string): Promise { + return reader.stat(packagePath).catch(() => null); +} + +async function readSafeUtf8(reader: SubmissionPackageReader, packagePath: string, maximum: number, budget: AggregateBudget): Promise { + const details = await safeStat(reader, packagePath); + if (details === null || details.safeResolution !== "safe" || details.resolvedKind !== "file" || details.size > maximum) return { kind: "invalid" }; + const nextBytes = budget.bytes + details.size; + if (nextBytes > maxAggregateMetadataBytes) return { kind: "budget", nextBytes }; + const bytes = await reader.read(packagePath, maximum).catch(() => null); + if (bytes === null || bytes.byteLength !== details.size) return { kind: "invalid" }; try { - const details = await stat(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)) }; + return { kind: "source", source: new TextDecoder("utf-8", { fatal: true }).decode(bytes) }; } catch { return { kind: "invalid" }; } @@ -125,224 +121,169 @@ function isToolDescriptor(value: unknown): value is Metadata { && (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)) { +async function validateIconPath(reader: SubmissionPackageReader, skillRoot: string, skillPath: string, field: "icon_small" | "icon_large", value: unknown): Promise { + if (typeof value !== "string" || value.trim() !== value || !supportedText(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 { + const iconPath = relativePackagePath(skillRoot, value); + const details = iconPath === null ? null : await safeStat(reader, iconPath); + if (details === null || details.safeResolution !== "safe" || details.resolvedKind !== "file") { return finding("plugin.submission.skill.agent.invalid_path", "Optional agent icon path is invalid.", { path: skillPath, field }); } + return null; } -async function validateAgentFile( - rootPath: string, - skillRoot: string, - skillPath: string, - budget: AggregateBudget -): Promise { - const agentPath = path.join(skillRoot, "agents", "openai.yaml"); - let agentDetails; - try { - agentDetails = await lstat(agentPath); - } catch { - return []; - } - if (agentDetails.isSymbolicLink()) { +async function validateAgentFile(reader: SubmissionPackageReader, skillRoot: string, resolvedSkillRoot: string, budget: AggregateBudget): Promise { + const skillPath = skillRoot; + const agentPath = `${skillRoot}/agents/openai.yaml`; + const agentDetails = await safeStat(reader, agentPath); + if (agentDetails === null) return []; + if (agentDetails.kind === "symlink") { 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) })]; + if (agentDetails.safeResolution === "outside") { + return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; } - - let canonicalRoot: string; - let canonicalSkill: string; - let canonicalAgent: string; - try { - [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 (agentDetails.safeResolution !== "safe" || agentDetails.resolvedKind !== "file") { + return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a readable regular file.", { path: agentPath })]; } - if (!isWithin(canonicalRoot, canonicalAgent) || !isWithin(canonicalSkill, canonicalAgent)) { + const resolvedAgentPath = agentDetails.resolvedPackagePath ?? agentPath; + if (!isWithinPackagePath(resolvedSkillRoot, resolvedAgentPath)) { return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; } - - const source = await readSafeUtf8(agentPath, maxAgentBytes, budget); - if (source.kind === "budget") { - return [budgetFinding(source.nextBytes)]; - } + const source = await readSafeUtf8(reader, 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 })]; + return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a bounded UTF-8 regular file.", { path: agentPath, limit: maxAgentBytes })]; } 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", - "Optional agent metadata must be a safe YAML mapping.", - { path: packagePath(rootPath, agentPath) } - )]; + 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: 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) })]; + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an unsupported shape.", { path: agentPath })]; } 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" })]; + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an invalid brand color.", { path: agentPath, field: "brand_color" })]; } 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" })]; + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent metadata has an invalid default prompt.", { path: agentPath, field: "default_prompt" })]; } if (metadata.policy !== undefined && (!isRecord(metadata.policy) || rejectUnknownKeys(metadata.policy, policyKeys) || (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"))) + || 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" })]; + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent policy has an unsupported shape.", { path: agentPath, field: "policy" })]; } if (metadata.dependencies !== undefined && (!isRecord(metadata.dependencies) || rejectUnknownKeys(metadata.dependencies, dependencyKeys) - || !Array.isArray(metadata.dependencies.tools) || metadata.dependencies.tools.length === 0 - || 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" })]; + || !Array.isArray(metadata.dependencies.tools) || metadata.dependencies.tools.length === 0 || metadata.dependencies.tools.some((tool) => !isToolDescriptor(tool)))) { + return [finding("plugin.submission.skill.agent.invalid_shape", "Optional agent dependencies have an unsupported shape.", { path: agentPath, field: "dependencies" })]; } - for (const field of ["icon_small", "icon_large"] as const) { const value = metadata.interface[field]; if (value === undefined) continue; - const iconFinding = await validateIconPath(rootPath, canonicalRoot, skillRoot, skillPath, field, value); - if (iconFinding) return [iconFinding]; + const iconFinding = await validateIconPath(reader, skillRoot, skillPath, field, value); + if (iconFinding !== null) return [iconFinding]; } return []; } -export async function validateSubmissionSkillMetadata( - discoveredPackage: DiscoveredPackage, - targetType: TargetType -): Promise { - const { manifest, rootPath } = discoveredPackage; +export async function validateSubmissionSkillMetadataFromReader(manifest: PluginManifest, targetType: SubmissionPreflightReport["targetType"], reader: SubmissionPackageReader): Promise { 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 }; + 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/") { + const root = skillsRoot(manifest.skills); + if (root === null) { 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 }; + const rootDetails = await safeStat(reader, root); + if (rootDetails === null || rootDetails.safeResolution !== "safe" || rootDetails.resolvedKind !== "directory") { + return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory must be canonically contained in the package.", { path: root })], skillCount: 0 }; + } + const listed = await reader.list(root).catch(() => null); + if (listed === null) { + return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory cannot be inspected safely.", { path: root })], skillCount: 0 }; } - const findings: SubmissionFinding[] = []; const identities = new Set(); - let skillCount = 0; const budget: AggregateBudget = { bytes: 0 }; - let entries; - try { - entries = await opendir(skillsPath); - } catch { - return { findings: [finding("plugin.submission.skill.invalid_path", "Skills directory cannot be inspected safely.", { path: "skills" })], skillCount: 0 }; - } - let entryCount = 0; + let skillCount = 0; let skillDirectoryCount = 0; - for await (const entry of entries) { - entryCount += 1; + const entries = [...listed].sort((left, right) => left.packagePath.localeCompare(right.packagePath)); + for (let index = 0; index < entries.length; index += 1) { + const entry = entries[index]!; + const entryCount = index + 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; + const name = path.posix.basename(entry.packagePath); + if (path.posix.dirname(entry.packagePath) !== root || name.startsWith(".") || entry.resolvedKind !== "directory") continue; + if (entry.safeResolution !== "safe" || entry.kind === "symlink") { + findings.push(finding("plugin.submission.skill.invalid_path", "Skill directory resolves outside the declared skills directory.", { path: entry.packagePath })); + 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); - 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) })); + const skillRoot = entry.packagePath; + const skillFile = `${skillRoot}/SKILL.md`; + const fileDetails = await safeStat(reader, skillFile); + if (fileDetails?.kind === "symlink" || fileDetails?.safeResolution === "outside") { + findings.push(finding("plugin.submission.skill.invalid_path", "Skill entrypoint must not be a symbolic link.", { path: skillFile })); 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 })); + if (fileDetails === null || fileDetails.safeResolution !== "safe" || fileDetails.resolvedKind !== "file") { + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a contained regular file.", { path: skillFile })); continue; } - const source = await readSafeUtf8(skillFile, maxSkillBytes, budget); + const source = await readSafeUtf8(reader, 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 })); + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint must be a bounded UTF-8 regular file.", { path: skillFile, limit: maxSkillBytes })); continue; } 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 })); + if (split === null) { + findings.push(finding("plugin.submission.skill.invalid_file", "Skill entrypoint requires delimited frontmatter and a nonempty body.", { path: skillFile })); 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 } - )); + 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: skillFile })); continue; } - const name = parsed.value.name; + const nameValue = parsed.value.name; const description = parsed.value.description; - const normalizedName = typeof name === "string" ? name.normalize("NFKC").trim() : ""; + const normalizedName = typeof nameValue === "string" ? nameValue.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 })); + if (!supportedText(nameValue) || !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: skillFile, limit: 64 })); continue; } identities.add(normalizedName); skillCount += 1; - const agentFindings = await validateAgentFile(rootPath, skillRoot, packagePath(rootPath, skillRoot), budget); + const resolvedSkillRoot = entry.resolvedPackagePath ?? skillRoot; + const agentFindings = await validateAgentFile(reader, skillRoot, resolvedSkillRoot, 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")) { + if (targetType === "skills-only" && skillCount === 0) { 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" })); + findings.push(finding("plugin.submission.skill.invalid_file", "Declared skills must include at least one valid immediate skill entrypoint.", { path: root })); } return { findings, skillCount }; } + +export async function validateSubmissionSkillMetadata(discoveredPackage: DiscoveredPackage, targetType: TargetType): Promise { + return validateSubmissionSkillMetadataFromReader(discoveredPackage.manifest, targetType, createDirectorySubmissionPackageReader(discoveredPackage.rootPath)); +} diff --git a/tests/helpers/submission-memory-reader.ts b/tests/helpers/submission-memory-reader.ts index 7a1f734..5ceac79 100644 --- a/tests/helpers/submission-memory-reader.ts +++ b/tests/helpers/submission-memory-reader.ts @@ -10,6 +10,7 @@ export interface MemorySubmissionPackageEntry { content?: string | Uint8Array; kind?: SubmissionPackageEntryKind; resolvedKind?: ResolvedEntryKind | null; + resolvedPackagePath?: string; safeResolution?: SubmissionPackageEntry["safeResolution"]; size?: number; } @@ -55,6 +56,7 @@ function entryFor(packagePath: string, source: MemorySubmissionPackageEntry): Su return { packagePath, kind, + ...(source.resolvedPackagePath === undefined ? {} : { resolvedPackagePath: source.resolvedPackagePath }), resolvedKind: source.resolvedKind ?? (kind === "symlink" ? null : kind), size: source.size ?? content?.byteLength ?? 0, safeResolution: source.safeResolution ?? "safe" diff --git a/tests/submission-package-reader.test.ts b/tests/submission-package-reader.test.ts index 4bafdba..58f8e77 100644 --- a/tests/submission-package-reader.test.ts +++ b/tests/submission-package-reader.test.ts @@ -156,6 +156,7 @@ describe("directory submission package reader", () => { packagePath: "contained-link.txt", kind: "symlink", resolvedKind: "file", + resolvedPackagePath: "a-first.txt", size: expect.any(Number), safeResolution: "safe" }); diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 15089bf..99b1166 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -3,7 +3,8 @@ 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"; +import { validateSubmissionSkillMetadata, validateSubmissionSkillMetadataFromReader } from "../src/core/submission-skill-metadata.js"; +import { createMemorySubmissionPackageReader } from "./helpers/submission-memory-reader.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`; @@ -339,6 +340,54 @@ describe("submission skill metadata", () => { }); } + it("rejects an agents junction that resolves into a sibling skill", async () => { + const discovered = await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/other/SKILL.md": skill("other"), + "skills/other/agents/openai.yaml": agent + }); + await symlink( + path.join(discovered.rootPath, "skills", "other", "agents"), + path.join(discovered.rootPath, "skills", "check", "agents"), + "junction" + ); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + const outsideSkill = result.findings.find( + (item) => item.id === "plugin.submission.skill.agent.invalid_path" + ); + + expect(outsideSkill?.message).toBe("Optional agent metadata resolves outside its skill."); + expect(outsideSkill?.evidence).toEqual({ path: "skills/check" }); + }); + + it("rejects reader agent metadata resolved into a sibling skill", async () => { + const result = await validateSubmissionSkillMetadataFromReader( + { name: "submission-plugin", skills: "./skills" }, + "skills-only", + createMemorySubmissionPackageReader({ + skills: { kind: "directory" }, + "skills/check": { kind: "directory" }, + "skills/other": { kind: "directory" }, + "skills/check/SKILL.md": { content: skill() }, + "skills/other/SKILL.md": { content: skill("other") }, + "skills/check/agents": { kind: "directory" }, + "skills/check/agents/openai.yaml": { + content: agent, + resolvedPackagePath: "skills/other/agents/openai.yaml" + }, + "skills/other/agents": { kind: "directory" }, + "skills/other/agents/openai.yaml": { content: agent } + }) + ); + const outsideSkill = result.findings.find( + (item) => item.id === "plugin.submission.skill.agent.invalid_path" + ); + + expect(outsideSkill?.message).toBe("Optional agent metadata resolves outside its skill."); + expect(outsideSkill?.evidence).toEqual({ path: "skills/check" }); + }); + if (process.platform !== "win32") { it("rejects an in-skill agent-file symlink before parsing its content", async () => { const discovered = await packageWith("./skills", { @@ -381,3 +430,33 @@ describe("submission skill metadata", () => { .not.toMatch(/child_process|node:child_process/u); }); }); + +describe("submission skill metadata reader parity", () => { + it("matches the directory wrapper for a normalized skills root and contained agent assets", async () => { + const agentMetadata = `${agent} icon_small: ./icon.svg\n icon_large: ../../assets/logo.svg\n`; + const reader = createMemorySubmissionPackageReader({ + skills: { kind: "directory" }, + "skills/check": { kind: "directory" }, + "skills/check/SKILL.md": { content: skill() }, + "skills/check/agents": { kind: "directory" }, + "skills/check/agents/openai.yaml": { content: agentMetadata }, + "skills/check/icon.svg": { content: "icon" }, + assets: { kind: "directory" }, + "assets/logo.svg": { content: "logo" } + }); + + const fromReader = await validateSubmissionSkillMetadataFromReader( + { name: "submission-plugin", skills: "./skills/./" }, + "skills-only", + reader + ); + const fromDirectory = await validateSubmissionSkillMetadata(await packageWith("./skills", { + "skills/check/SKILL.md": skill(), + "skills/check/agents/openai.yaml": agentMetadata, + "skills/check/icon.svg": "icon", + "assets/logo.svg": "logo" + }), "skills-only"); + + expect(fromReader).toEqual(fromDirectory); + }); +}); From 34658fa2415dc080aacc619af610e19f17f72f60 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 15:57:16 +0300 Subject: [PATCH 12/20] fix: preserve agent file messages --- src/core/submission-skill-metadata.ts | 5 ++- tests/submission-skill-metadata.test.ts | 57 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index e3b8fa2..bd6ec87 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -144,9 +144,12 @@ async function validateAgentFile(reader: SubmissionPackageReader, skillRoot: str if (agentDetails.safeResolution === "outside") { return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; } - if (agentDetails.safeResolution !== "safe" || agentDetails.resolvedKind !== "file") { + if (agentDetails.safeResolution !== "safe") { return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a readable regular file.", { path: agentPath })]; } + if (agentDetails.resolvedKind !== "file") { + return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a regular file.", { path: agentPath })]; + } const resolvedAgentPath = agentDetails.resolvedPackagePath ?? agentPath; if (!isWithinPackagePath(resolvedSkillRoot, resolvedAgentPath)) { return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 99b1166..1b66c76 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -192,6 +192,63 @@ describe("submission skill metadata", () => { expect(ids(result)).toContain(expected); }); + it("reports an existing directory agent entry as a non-file through the directory wrapper", async () => { + const discovered = await packageWith("./skills", { "skills/check/SKILL.md": skill() }); + await mkdir(path.join(discovered.rootPath, "skills", "check", "agents", "openai.yaml"), { recursive: true }); + + const result = await validateSubmissionSkillMetadata(discovered, "skills-only"); + const invalidFile = result.findings.find( + (item) => item.id === "plugin.submission.skill.agent.invalid_file" + ); + + expect(invalidFile?.message).toBe("Optional agent metadata must be a regular file."); + expect(invalidFile?.evidence).toEqual({ path: "skills/check/agents/openai.yaml" }); + }); + + it("reports an existing directory agent entry as a non-file through a memory reader", async () => { + const result = await validateSubmissionSkillMetadataFromReader( + { name: "submission-plugin", skills: "./skills" }, + "skills-only", + createMemorySubmissionPackageReader({ + skills: { kind: "directory" }, + "skills/check": { kind: "directory" }, + "skills/check/SKILL.md": { content: skill() }, + "skills/check/agents": { kind: "directory" }, + "skills/check/agents/openai.yaml": { kind: "directory" } + }) + ); + const invalidFile = result.findings.find( + (item) => item.id === "plugin.submission.skill.agent.invalid_file" + ); + + expect(invalidFile?.message).toBe("Optional agent metadata must be a regular file."); + expect(invalidFile?.evidence).toEqual({ path: "skills/check/agents/openai.yaml" }); + }); + + it("reports an unavailable agent entry as unreadable through a memory reader", async () => { + const result = await validateSubmissionSkillMetadataFromReader( + { name: "submission-plugin", skills: "./skills" }, + "skills-only", + createMemorySubmissionPackageReader({ + skills: { kind: "directory" }, + "skills/check": { kind: "directory" }, + "skills/check/SKILL.md": { content: skill() }, + "skills/check/agents": { kind: "directory" }, + "skills/check/agents/openai.yaml": { + kind: "file", + resolvedKind: null, + safeResolution: "unavailable" + } + }) + ); + const invalidFile = result.findings.find( + (item) => item.id === "plugin.submission.skill.agent.invalid_file" + ); + + expect(invalidFile?.message).toBe("Optional agent metadata must be a readable regular file."); + expect(invalidFile?.evidence).toEqual({ path: "skills/check/agents/openai.yaml" }); + }); + it.each([ "policy: { products: [CHAT] }\n", "policy: { allow_implicit_invocation: false }\n", From 3e65dcc2e4ad97037059a1cb67785d93a07b8142 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 16:14:02 +0300 Subject: [PATCH 13/20] fix: validate resolved submission paths --- src/core/submission-skill-metadata.ts | 13 +++++++++++-- tests/submission-skill-metadata.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/core/submission-skill-metadata.ts b/src/core/submission-skill-metadata.ts index bd6ec87..34b5032 100644 --- a/src/core/submission-skill-metadata.ts +++ b/src/core/submission-skill-metadata.ts @@ -78,6 +78,14 @@ function relativePackagePath(base: string, value: string): string | null { return resolved === ".." || resolved.startsWith("../") || resolved === "." ? null : resolved; } +function normalizeResolvedPackagePath(value: unknown): string | null { + if (typeof value !== "string" || value === "" || value.includes("\\") || path.posix.isAbsolute(value) + || /^[A-Za-z]:/u.test(value) || /[\u0000-\u001F\u007F]/u.test(value)) return null; + const normalized = path.posix.normalize(value); + if (normalized === "" || normalized === "." || normalized === ".." || normalized.startsWith("../")) return null; + return normalized; +} + function isWithinPackagePath(root: string, candidate: string): boolean { return candidate === root || candidate.startsWith(`${root}/`); } @@ -150,8 +158,9 @@ async function validateAgentFile(reader: SubmissionPackageReader, skillRoot: str if (agentDetails.resolvedKind !== "file") { return [finding("plugin.submission.skill.agent.invalid_file", "Optional agent metadata must be a regular file.", { path: agentPath })]; } - const resolvedAgentPath = agentDetails.resolvedPackagePath ?? agentPath; - if (!isWithinPackagePath(resolvedSkillRoot, resolvedAgentPath)) { + const normalizedSkillRoot = normalizeResolvedPackagePath(resolvedSkillRoot); + const resolvedAgentPath = normalizeResolvedPackagePath(agentDetails.resolvedPackagePath ?? agentPath); + if (normalizedSkillRoot === null || resolvedAgentPath === null || !isWithinPackagePath(normalizedSkillRoot, resolvedAgentPath)) { return [finding("plugin.submission.skill.agent.invalid_path", "Optional agent metadata resolves outside its skill.", { path: skillPath })]; } const source = await readSafeUtf8(reader, agentPath, maxAgentBytes, budget); diff --git a/tests/submission-skill-metadata.test.ts b/tests/submission-skill-metadata.test.ts index 1b66c76..b6a73af 100644 --- a/tests/submission-skill-metadata.test.ts +++ b/tests/submission-skill-metadata.test.ts @@ -445,6 +445,30 @@ describe("submission skill metadata", () => { expect(outsideSkill?.evidence).toEqual({ path: "skills/check" }); }); + it("rejects reader agent metadata with a contained-looking traversal target", async () => { + const result = await validateSubmissionSkillMetadataFromReader( + { name: "submission-plugin", skills: "./skills" }, + "skills-only", + createMemorySubmissionPackageReader({ + skills: { kind: "directory" }, + "skills/check": { kind: "directory" }, + "skills/other": { kind: "directory" }, + "skills/check/SKILL.md": { content: skill() }, + "skills/check/agents": { kind: "directory" }, + "skills/check/agents/openai.yaml": { + content: agent, + resolvedPackagePath: "skills/check/agents/../../other/agents/openai.yaml" + } + }) + ); + const outsideSkill = result.findings.find( + (item) => item.id === "plugin.submission.skill.agent.invalid_path" + ); + + expect(outsideSkill?.message).toBe("Optional agent metadata resolves outside its skill."); + expect(outsideSkill?.evidence).toEqual({ path: "skills/check" }); + }); + if (process.platform !== "win32") { it("rejects an in-skill agent-file symlink before parsing its content", async () => { const discovered = await packageWith("./skills", { From 34151432c93ea5514b97b5b442b0f934d7b25701 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 17:51:20 +0300 Subject: [PATCH 14/20] feat: add bounded submission ZIP reader --- src/core/submission-archive-reader.ts | 643 ++++++++++++++++++++++++ tests/helpers/zip-fixture.ts | 148 ++++++ tests/submission-archive-reader.test.ts | 422 ++++++++++++++++ 3 files changed, 1213 insertions(+) create mode 100644 src/core/submission-archive-reader.ts create mode 100644 tests/helpers/zip-fixture.ts create mode 100644 tests/submission-archive-reader.test.ts diff --git a/src/core/submission-archive-reader.ts b/src/core/submission-archive-reader.ts new file mode 100644 index 0000000..1f8e4ca --- /dev/null +++ b/src/core/submission-archive-reader.ts @@ -0,0 +1,643 @@ +import { close, fstat, open, read, type Stats } from "node:fs"; +import { stat } from "node:fs/promises"; +import path from "node:path"; +import { Readable } from "node:stream"; + +import iconv from "iconv-lite"; +import * as yauzl from "yauzl"; + +import type { SubmissionPackageEntry, SubmissionPackageReader } from "./submission-package-reader.js"; +import type { SubmissionFinding } from "./submission-preflight.js"; + +const maxCompressedBytes = 100 * 1000 * 1000; +const maxEntries = 5_000; +const maxMemberBytes = 100 * 1024 * 1024; +const maxTotalBytes = 512 * 1024 * 1024; +const maxPathSegments = 20; +const invalidPackagePathMessage = "Invalid package path."; + +export interface SubmissionArchiveFinding extends SubmissionFinding { + id: `plugin.submission.archive.${string}`; +} + +export interface SubmissionArchiveCoverage { + id: string; + status: "automatic" | "manual" | "unavailable"; + reason: string; +} + +export interface SubmissionArchiveInspection { + fileName: string; + compressedBytes: number; + uncompressedBytes: number; + entryCount: number; + entries: readonly SubmissionPackageEntry[]; + findings: readonly SubmissionArchiveFinding[]; + coverage: readonly SubmissionArchiveCoverage[]; + reader: SubmissionPackageReader | null; +} + +interface MutableSubmissionArchiveInspection extends Omit { + entries: SubmissionPackageEntry[]; + findings: SubmissionArchiveFinding[]; + coverage: SubmissionArchiveCoverage[]; +} + +interface CheckedEntry { + index: number; + packageEntry: SubmissionPackageEntry; + crc32: number; + compressionMethod: number; + uncompressedSize: number; +} + +interface LocalHeader { + flags: number; + method: number; + crc32: number; + compressedSize: number; + uncompressedSize: number; + rawName: Buffer; + dataStart: number; + intervalEnd: number; +} + +interface LocalInterval { + start: number; + end: number; +} + +interface ArchiveMetadata { + centralStart: number; + metadataStart: number; +} + +interface ArchiveIdentity { + dev: number; + ino: number; + size: number; + mtimeMs: number; + ctimeMs: number; +} + +const crcTable = Array.from({ length: 256 }, (_, index) => { + let value = index; + for (let bit = 0; bit < 8; bit += 1) value = (value & 1) === 0 ? value >>> 1 : (value >>> 1) ^ 0xedb88320; + return value >>> 0; +}); + +function updateCrc32(value: number, content: Uint8Array): number { + let result = value; + for (const byte of content) result = (result >>> 8) ^ crcTable[(result ^ byte) & 0xff]; + return result >>> 0; +} + +function archiveFinding( + id: SubmissionArchiveFinding["id"], + message: string, + evidence?: SubmissionArchiveFinding["evidence"], + severity: SubmissionArchiveFinding["severity"] = "fail" +): SubmissionArchiveFinding { + return evidence === undefined ? { id, severity, message } : { id, severity, message, evidence }; +} + +function unavailableCoverage(reason: string): SubmissionArchiveCoverage { + return { id: "plugin.submission.archive.compression_method", status: "unavailable", reason }; +} + +function blankInspection(fileName: string, compressedBytes = 0): MutableSubmissionArchiveInspection { + return { + fileName, + compressedBytes, + uncompressedBytes: 0, + entryCount: 0, + entries: [], + findings: [], + coverage: [], + reader: null + }; +} + +function validSafeInteger(value: number): boolean { + return Number.isSafeInteger(value) && value >= 0; +} + +function openFileDescriptor(filePath: string): Promise { + return new Promise((resolve, reject) => { + open(filePath, "r", (error, fileDescriptor) => { + if (error !== null) reject(error); + else resolve(fileDescriptor); + }); + }); +} + +function statFileDescriptor(fileDescriptor: number): Promise { + return new Promise((resolve, reject) => { + fstat(fileDescriptor, (error, details) => { + if (error !== null) reject(error); + else resolve(details); + }); + }); +} + +function readFileDescriptor(fileDescriptor: number, content: Buffer, offset: number, length: number, position: number): Promise { + return new Promise((resolve, reject) => { + read(fileDescriptor, content, offset, length, position, (error, bytesRead) => { + if (error !== null) reject(error); + else resolve(bytesRead); + }); + }); +} + +function closeFileDescriptor(fileDescriptor: number): Promise { + return new Promise((resolve, reject) => { + close(fileDescriptor, (error) => { + if (error !== null) reject(error); + else resolve(); + }); + }); +} + +function archiveIdentity(details: Stats): ArchiveIdentity { + return { dev: details.dev, ino: details.ino, size: details.size, mtimeMs: details.mtimeMs, ctimeMs: details.ctimeMs }; +} + +function matchesArchiveIdentity(details: Stats, expected: ArchiveIdentity): boolean { + return details.isFile() + && details.dev === expected.dev + && details.ino === expected.ino + && details.size === expected.size + && details.mtimeMs === expected.mtimeMs + && details.ctimeMs === expected.ctimeMs; +} + +function normalizeRequestedPath(packagePath: string): string { + if (typeof packagePath !== "string" || packagePath === "" || /[\u0000-\u001F\u007F]/u.test(packagePath) + || packagePath.startsWith("/") || packagePath.startsWith("\\") || /^[a-zA-Z]:/u.test(packagePath) + || packagePath.includes("\\")) throw new Error(invalidPackagePathMessage); + const segments = packagePath.split("/"); + if (segments.some((segment) => segment === "" || segment === "." || segment === "..")) throw new Error(invalidPackagePathMessage); + return packagePath; +} + +function decodePath(rawName: Buffer, flags: number): string | null { + try { + return (flags & 0x0800) !== 0 + ? new TextDecoder("utf-8", { fatal: true }).decode(rawName) + : iconv.decode(rawName, "cp437"); + } catch { + return null; + } +} + +function normalizeArchivePath(decodedPath: string): { path: string; directory: boolean } | null { + if (decodedPath.length === 0 || decodedPath.trim() !== decodedPath || /[\u0000-\u001F\u007F]/u.test(decodedPath) + || decodedPath.startsWith("/") || decodedPath.startsWith("\\") || /^[a-zA-Z]:/u.test(decodedPath) + || decodedPath.includes("\\")) return null; + const directory = decodedPath.endsWith("/"); + const withoutTrailingSlash = directory ? decodedPath.slice(0, -1) : decodedPath; + const segments = withoutTrailingSlash.split("/"); + if (withoutTrailingSlash.length === 0 || segments.length > maxPathSegments + || segments.some((segment) => segment === "" || segment === "." || segment === "..")) return null; + return { path: segments.join("/"), directory }; +} + +function entryKind(entry: yauzl.Entry, trailingDirectory: boolean): SubmissionPackageEntry["kind"] { + const madeByUnix = entry.versionMadeBy >>> 8 === 3; + const unixType = madeByUnix ? (entry.externalFileAttributes >>> 16) & 0o170000 : 0; + const dosDirectory = (entry.externalFileAttributes & 0x10) !== 0; + if (unixType === 0o120000) return "symlink"; + if (unixType !== 0 && unixType !== 0o100000 && unixType !== 0o040000) return "other"; + if (trailingDirectory || dosDirectory || unixType === 0o040000) return "directory"; + return "file"; +} + +function hasContradictoryType(entry: yauzl.Entry, trailingDirectory: boolean, kind: SubmissionPackageEntry["kind"]): boolean { + const madeByUnix = entry.versionMadeBy >>> 8 === 3; + const unixType = madeByUnix ? (entry.externalFileAttributes >>> 16) & 0o170000 : 0; + const dosDirectory = (entry.externalFileAttributes & 0x10) !== 0; + return ((trailingDirectory || dosDirectory) && unixType === 0o100000) + || (unixType === 0o040000 && kind !== "directory") + || ((trailingDirectory || dosDirectory) && kind !== "directory"); +} + +function unicodePathChangesDecodedName(entry: yauzl.Entry, rawName: Buffer, decodedPath: string): boolean { + const field = entry.extraFields.find((candidate) => candidate.id === 0x7075); + if (field === undefined || field.data.length < 5 || field.data[0] !== 1) return false; + const expectedCrc = field.data.readUInt32LE(1); + if (expectedCrc !== crc32(rawName)) return false; + try { + return new TextDecoder("utf-8", { fatal: true }).decode(field.data.subarray(5)) !== decodedPath; + } catch { + return false; + } +} + +function crc32(content: Uint8Array): number { + return (updateCrc32(0xffffffff, content) ^ 0xffffffff) >>> 0; +} + +async function readExactly(fileDescriptor: number, position: number, length: number): Promise { + const content = Buffer.alloc(length); + let offset = 0; + while (offset < length) { + const bytesRead = await readFileDescriptor(fileDescriptor, content, offset, length - offset, position + offset); + if (bytesRead === 0) return null; + offset += bytesRead; + } + return content; +} + +function readSafeUInt64(content: Buffer, offset: number): number | null { + if (offset + 8 > content.length) return null; + const value = content.readBigUInt64LE(offset); + return value <= BigInt(Number.MAX_SAFE_INTEGER) ? Number(value) : null; +} + +function parseLocalExtraFields(content: Buffer): readonly { id: number; data: Buffer }[] | null { + const fields: { id: number; data: Buffer }[] = []; + let offset = 0; + while (offset < content.length) { + if (content.length - offset < 4) return null; + const id = content.readUInt16LE(offset); + const length = content.readUInt16LE(offset + 2); + offset += 4; + if (length > content.length - offset) return null; + fields.push({ id, data: content.subarray(offset, offset + length) }); + offset += length; + } + return fields; +} + +async function readLocalHeader( + fileDescriptor: number, + entry: yauzl.Entry, + fileSize: number +): Promise { + const offset = entry.relativeOffsetOfLocalHeader; + if (!validSafeInteger(offset) || offset + 30 > fileSize) return null; + const fixed = await readExactly(fileDescriptor, offset, 30); + if (fixed === null || fixed.readUInt32LE(0) !== 0x04034b50) return null; + const flags = fixed.readUInt16LE(6); + const method = fixed.readUInt16LE(8); + const crc = fixed.readUInt32LE(14); + const compressedSize = fixed.readUInt32LE(18); + const uncompressedSize = fixed.readUInt32LE(22); + const nameLength = fixed.readUInt16LE(26); + const extraLength = fixed.readUInt16LE(28); + const dataStart = offset + 30 + nameLength + extraLength; + if (!validSafeInteger(dataStart) || dataStart > fileSize) return null; + const rawName = await readExactly(fileDescriptor, offset + 30, nameLength); + const extra = await readExactly(fileDescriptor, offset + 30 + nameLength, extraLength); + const extraFields = extra === null ? null : parseLocalExtraFields(extra); + if (rawName === null || extraFields === null || !validSafeInteger(entry.compressedSize) || dataStart + entry.compressedSize > fileSize) return null; + + let resolvedCompressedSize = compressedSize; + let resolvedUncompressedSize = uncompressedSize; + if ((flags & 0x0008) === 0 && (compressedSize === 0xffffffff || uncompressedSize === 0xffffffff)) { + const zip64Fields = extraFields.filter((field) => field.id === 0x0001); + if (zip64Fields.length !== 1) return null; + let zip64Offset = 0; + if (uncompressedSize === 0xffffffff) { + const value = readSafeUInt64(zip64Fields[0].data, zip64Offset); + if (value === null) return null; + resolvedUncompressedSize = value; + zip64Offset += 8; + } + if (compressedSize === 0xffffffff) { + const value = readSafeUInt64(zip64Fields[0].data, zip64Offset); + if (value === null) return null; + resolvedCompressedSize = value; + } + } + + let descriptorLength = 0; + if ((flags & 0x0008) !== 0) { + const zip64Descriptor = entry.versionNeededToExtract >= 45; + const descriptor = await readExactly(fileDescriptor, dataStart + entry.compressedSize, zip64Descriptor ? 24 : 16); + if (descriptor === null) return null; + const signed = descriptor.readUInt32LE(0) === 0x08074b50; + const base = signed ? 4 : 0; + const descriptorLengthWithoutSignature = zip64Descriptor ? 20 : 12; + if (descriptor.length < base + descriptorLengthWithoutSignature + || descriptor.readUInt32LE(base) !== entry.crc32) return null; + const compressedSize = zip64Descriptor + ? readSafeUInt64(descriptor, base + 4) + : descriptor.readUInt32LE(base + 4); + const uncompressedSize = zip64Descriptor + ? readSafeUInt64(descriptor, base + 12) + : descriptor.readUInt32LE(base + 8); + if (compressedSize === null || uncompressedSize === null + || compressedSize !== entry.compressedSize || uncompressedSize !== entry.uncompressedSize) return null; + descriptorLength = (signed ? 4 : 0) + descriptorLengthWithoutSignature; + } + + return { + flags, + method, + crc32: crc, + compressedSize: resolvedCompressedSize, + uncompressedSize: resolvedUncompressedSize, + rawName, + dataStart, + intervalEnd: dataStart + entry.compressedSize + descriptorLength + }; +} + +async function readArchiveMetadata( + fileDescriptor: number, + fileSize: number +): Promise { + const tailLength = Math.min(fileSize, 0xffff + 22 + 20); + const tail = await readExactly(fileDescriptor, fileSize - tailLength, tailLength); + if (tail === null) return null; + for (let offset = tail.length - 22; offset >= 0; offset -= 1) { + if (tail.readUInt32LE(offset) !== 0x06054b50) continue; + const commentLength = tail.readUInt16LE(offset + 20); + if (offset + 22 + commentLength !== tail.length) continue; + const eocdOffset = fileSize - tailLength + offset; + const disk = tail.readUInt16LE(offset + 4); + const centralDisk = tail.readUInt16LE(offset + 6); + const entriesOnDisk = tail.readUInt16LE(offset + 8); + const entryCount = tail.readUInt16LE(offset + 10); + const centralSize32 = tail.readUInt32LE(offset + 12); + const centralStart32 = tail.readUInt32LE(offset + 16); + const zip64 = entriesOnDisk === 0xffff || entryCount === 0xffff || centralSize32 === 0xffffffff || centralStart32 === 0xffffffff; + if (!zip64) { + if (disk !== 0 || centralDisk !== 0 || entriesOnDisk !== entryCount + || centralStart32 > eocdOffset + || centralSize32 > eocdOffset - centralStart32) return null; + const signature = await readExactly(fileDescriptor, centralStart32, 4); + return signature?.readUInt32LE(0) === 0x02014b50 + ? { centralStart: centralStart32, metadataStart: centralStart32 } + : null; + } + if (eocdOffset < 20) return null; + const locator = await readExactly(fileDescriptor, eocdOffset - 20, 20); + if (locator === null || locator.readUInt32LE(0) !== 0x07064b50 || locator.readUInt32LE(4) !== 0 || locator.readUInt32LE(16) !== 1) return null; + const zip64Offset = readSafeUInt64(locator, 8); + if (zip64Offset === null || zip64Offset > eocdOffset - 20 - 56) return null; + const zip64Record = await readExactly(fileDescriptor, zip64Offset, 56); + const zip64RecordSize = zip64Record === null ? null : readSafeUInt64(zip64Record, 4); + if (zip64Record === null || zip64Record.readUInt32LE(0) !== 0x06064b50 || zip64RecordSize === null + || zip64RecordSize < 44 || zip64RecordSize > eocdOffset - 20 - zip64Offset - 12 + || zip64Record.readUInt32LE(16) !== 0 || zip64Record.readUInt32LE(20) !== 0) return null; + const zip64EntriesOnDisk = readSafeUInt64(zip64Record, 24); + const zip64Entries = readSafeUInt64(zip64Record, 32); + const centralSize = readSafeUInt64(zip64Record, 40); + const centralStart = readSafeUInt64(zip64Record, 48); + if (zip64EntriesOnDisk === null || zip64Entries === null || centralSize === null || centralStart === null + || zip64EntriesOnDisk !== zip64Entries || centralStart > zip64Offset + || centralSize > zip64Offset - centralStart) return null; + const signature = await readExactly(fileDescriptor, centralStart, 4); + return signature?.readUInt32LE(0) === 0x02014b50 + ? { centralStart, metadataStart: Math.min(centralStart, zip64Offset) } + : null; + } + return null; +} + +async function streamAndValidate( + zip: yauzl.ZipFile, + entry: yauzl.Entry, + expectedCrc: number, + expectedSize: number, + currentTotal: number +): Promise<{ actualBytes: number; totalBytes: number } | null> { + try { + const stream = await zip.openReadStreamPromise(entry); + let actualBytes = 0; + let crc = 0xffffffff; + for await (const chunk of stream as Readable) { + const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk as Uint8Array); + if (actualBytes + bytes.length > expectedSize || currentTotal + actualBytes + bytes.length > maxTotalBytes) { + stream.destroy(); + return null; + } + actualBytes += bytes.length; + crc = updateCrc32(crc, bytes); + } + return actualBytes === expectedSize && ((crc ^ 0xffffffff) >>> 0) === expectedCrc + ? { actualBytes, totalBytes: currentTotal + actualBytes } + : null; + } catch { + return null; + } +} + +async function openZip(fileDescriptor: number): Promise { + return yauzl.fromFdPromise(fileDescriptor, { + autoClose: false, + lazyEntries: true, + decodeStrings: false, + strictFileNames: false, + validateEntrySizes: false + }); +} + +function createArchiveReader(zipPath: string, checkedEntries: readonly CheckedEntry[], expectedIdentity: ArchiveIdentity): SubmissionPackageReader { + const byPath = new Map(checkedEntries.map((entry) => [entry.packageEntry.packagePath, entry])); + const packageEntries = checkedEntries.map((entry) => entry.packageEntry); + return { + async list(directory: string): Promise { + const normalizedDirectory = directory === "" ? "" : normalizeRequestedPath(directory.endsWith("/") ? directory.slice(0, -1) : directory); + const prefix = normalizedDirectory === "" ? "" : `${normalizedDirectory}/`; + return packageEntries.filter((entry) => entry.packagePath.startsWith(prefix) && !entry.packagePath.slice(prefix.length).includes("/")); + }, + async stat(packagePath: string): Promise { + return byPath.get(normalizeRequestedPath(packagePath))?.packageEntry ?? null; + }, + async read(packagePath: string, maxBytes: number): Promise { + const selected = byPath.get(normalizeRequestedPath(packagePath)); + if (!Number.isSafeInteger(maxBytes) || maxBytes < 0) throw new Error("maxBytes must be a nonnegative safe integer."); + if (selected === undefined || selected.packageEntry.resolvedKind !== "file" || selected.uncompressedSize > maxBytes) return null; + let zip: yauzl.ZipFile | null = null; + let fileDescriptor: number | null = null; + try { + fileDescriptor = await openFileDescriptor(zipPath); + if (!matchesArchiveIdentity(await statFileDescriptor(fileDescriptor), expectedIdentity)) return null; + zip = await openZip(fileDescriptor); + fileDescriptor = null; + let index = 0; + for await (const entry of zip.eachEntry()) { + if (index++ !== selected.index) continue; + const stream = await zip.openReadStreamPromise(entry); + const result = Buffer.alloc(selected.uncompressedSize); + let offset = 0; + let crc = 0xffffffff; + for await (const chunk of stream as Readable) { + const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk as Uint8Array); + if (offset + bytes.length > result.length) return null; + bytes.copy(result, offset); + offset += bytes.length; + crc = updateCrc32(crc, bytes); + } + return offset === result.length && ((crc ^ 0xffffffff) >>> 0) === selected.crc32 + ? Uint8Array.from(result) + : null; + } + return null; + } catch { + return null; + } finally { + zip?.close(); + if (fileDescriptor !== null) await closeFileDescriptor(fileDescriptor).catch(() => undefined); + } + } + }; +} + +export async function inspectSubmissionArchive(zipPath: string): Promise { + const fileName = typeof zipPath === "string" ? path.basename(zipPath) : "archive.zip"; + let details: Stats; + try { + if (typeof zipPath !== "string" || path.extname(zipPath).toLowerCase() !== ".zip") { + const report = blankInspection(fileName); + report.findings.push(archiveFinding("plugin.submission.archive.invalid_file", "Input must be a regular .zip file.")); + return report; + } + details = await stat(zipPath); + } catch { + const report = blankInspection(fileName); + report.findings.push(archiveFinding("plugin.submission.archive.invalid_file", "Input must be a readable regular .zip file.")); + return report; + } + const report = blankInspection(fileName, validSafeInteger(details.size) ? details.size : 0); + if (!details.isFile() || !validSafeInteger(details.size) || details.size === 0) { + report.findings.push(archiveFinding("plugin.submission.archive.invalid_file", "Input must be a non-empty regular .zip file.")); + return report; + } + if (details.size > maxCompressedBytes) { + report.findings.push(archiveFinding("plugin.submission.archive.too_large", "Archive exceeds the compressed size limit.", { limit: maxCompressedBytes })); + return report; + } + + let zip: yauzl.ZipFile | null = null; + let fileDescriptor: number | null = null; + try { + fileDescriptor = await openFileDescriptor(zipPath); + const openedDetails = await statFileDescriptor(fileDescriptor); + if (!matchesArchiveIdentity(openedDetails, archiveIdentity(details))) { + report.findings.push(archiveFinding("plugin.submission.archive.invalid_zip", "Archive changed while opening for inspection.")); + return report; + } + const metadata = await readArchiveMetadata(fileDescriptor, openedDetails.size); + if (metadata === null) { + report.findings.push(archiveFinding("plugin.submission.archive.range_invalid", "Archive metadata ranges are invalid.")); + return report; + } + const inspectionDescriptor = fileDescriptor; + zip = await openZip(inspectionDescriptor); + fileDescriptor = null; + if (!validSafeInteger(zip.entryCount) || zip.entryCount > maxEntries) { + report.findings.push(archiveFinding("plugin.submission.archive.entry_count", "Archive entry count exceeds the limit.", { limit: maxEntries })); + return report; + } + report.entryCount = zip.entryCount; + const checked: CheckedEntry[] = []; + const intervals: LocalInterval[] = []; + const paths = new Map(); + const normalizedPaths = new Set(); + let declaredTotal = 0; + let actualTotal = 0; + let unsupportedCompression = false; + let index = 0; + + for await (const entry of zip.eachEntry()) { + const entryIndex = index++; + const rawName = entry.fileName as unknown as Buffer; + const decodedPath = decodePath(rawName, entry.generalPurposeBitFlag); + const normalized = decodedPath === null ? null : normalizeArchivePath(decodedPath); + if (normalized === null || decodedPath === null) { + report.findings.push(archiveFinding("plugin.submission.archive.path_invalid", "Archive entry path is invalid.", { entryIndex })); + continue; + } + if (!validSafeInteger(entry.compressedSize) || !validSafeInteger(entry.uncompressedSize) + || !validSafeInteger(entry.crc32) || entry.uncompressedSize > maxMemberBytes) { + report.findings.push(archiveFinding("plugin.submission.archive.member_too_large", "Archive entry exceeds a supported size limit.", { entryIndex, limit: maxMemberBytes })); + continue; + } + declaredTotal += entry.uncompressedSize; + if (!validSafeInteger(declaredTotal) || declaredTotal > maxTotalBytes) { + report.findings.push(archiveFinding("plugin.submission.archive.total_too_large", "Archive exceeds the total uncompressed size limit.", { limit: maxTotalBytes })); + continue; + } + const local = await readLocalHeader(inspectionDescriptor, entry, details.size); + if (local === null) { + report.findings.push(archiveFinding((entry.generalPurposeBitFlag & 0x0008) !== 0 ? "plugin.submission.archive.descriptor_invalid" : "plugin.submission.archive.range_invalid", "Archive local header or descriptor is invalid.", { entryIndex })); + continue; + } + if (!rawName.equals(local.rawName) || local.flags !== entry.generalPurposeBitFlag || local.method !== entry.compressionMethod + || ((entry.generalPurposeBitFlag & 0x0008) === 0 && (local.crc32 !== entry.crc32 || local.compressedSize !== entry.compressedSize || local.uncompressedSize !== entry.uncompressedSize))) { + report.findings.push(archiveFinding("plugin.submission.archive.header_mismatch", "Archive central and local headers disagree.", { entryIndex })); + continue; + } + intervals.push({ start: entry.relativeOffsetOfLocalHeader, end: local.intervalEnd }); + const kind = entryKind(entry, normalized.directory); + if (hasContradictoryType(entry, normalized.directory, kind) || kind === "symlink" || kind === "other") { + report.findings.push(archiveFinding("plugin.submission.archive.type_unsupported", "Archive entry type is unsupported.", { entryIndex })); + continue; + } + const packageEntry: SubmissionPackageEntry = { + packagePath: normalized.path, + kind, + resolvedKind: kind, + size: entry.uncompressedSize, + safeResolution: "safe" + }; + if (paths.has(normalized.path)) { + report.findings.push(archiveFinding("plugin.submission.archive.path_duplicate", "Archive contains duplicate entry paths.", { path: normalized.path })); + continue; + } + if ([...paths.keys()].some((existing) => existing.startsWith(`${normalized.path}/`) || normalized.path.startsWith(`${existing}/`))) { + report.findings.push(archiveFinding("plugin.submission.archive.path_conflict", "Archive entry paths conflict.", { path: normalized.path })); + continue; + } + const collisionKey = normalized.path.split("/").map((segment) => segment.normalize("NFKC").toLowerCase()).join("/"); + if (normalizedPaths.has(collisionKey)) { + report.findings.push(archiveFinding("plugin.submission.archive.normalization_collision", "Archive paths collide under Doctor's local normalization check.", { path: normalized.path }, "warn")); + } + normalizedPaths.add(collisionKey); + paths.set(normalized.path, packageEntry); + if (unicodePathChangesDecodedName(entry, rawName, decodedPath)) { + report.coverage.push({ id: "plugin.submission.archive.filename_decoding", status: "unavailable", reason: "Unicode Path extra fields may change portal filename decoding." }); + } + if ((entry.generalPurposeBitFlag & 0x0001) !== 0) { + report.findings.push(archiveFinding("plugin.submission.archive.encrypted", "Encrypted archive entries cannot be inspected.", { path: normalized.path })); + continue; + } + if (kind !== "file") { + checked.push({ index: entryIndex, packageEntry, crc32: entry.crc32, compressionMethod: entry.compressionMethod, uncompressedSize: entry.uncompressedSize }); + continue; + } + if (entry.compressionMethod !== 0 && entry.compressionMethod !== 8) { + unsupportedCompression = true; + continue; + } + const streamed = await streamAndValidate(zip, entry, entry.crc32, entry.uncompressedSize, actualTotal); + if (streamed === null) { + report.findings.push(archiveFinding("plugin.submission.archive.crc_mismatch", "Archive entry content does not match declared CRC or size.", { path: normalized.path })); + continue; + } + actualTotal = streamed.totalBytes; + checked.push({ index: entryIndex, packageEntry, crc32: entry.crc32, compressionMethod: entry.compressionMethod, uncompressedSize: entry.uncompressedSize }); + } + intervals.sort((left, right) => left.start - right.start); + if (intervals.some((interval, intervalIndex) => interval.end > metadata.metadataStart + || (intervalIndex > 0 && interval.start < intervals[intervalIndex - 1].end))) { + report.findings.push(archiveFinding("plugin.submission.archive.range_invalid", "Archive entry data ranges overlap or exceed the archive.")); + } + report.uncompressedBytes = actualTotal; + report.entries = checked.map((entry) => entry.packageEntry).sort((left, right) => left.packagePath.localeCompare(right.packagePath)); + if (unsupportedCompression) report.coverage.push(unavailableCoverage("Doctor does not decode this well-formed compression method.")); + if (report.findings.some((finding) => finding.severity === "fail") || unsupportedCompression) return report; + report.reader = createArchiveReader(zipPath, checked, archiveIdentity(openedDetails)); + return report; + } catch { + report.findings.push(archiveFinding("plugin.submission.archive.invalid_zip", "Archive is malformed or truncated.")); + return report; + } finally { + zip?.close(); + if (fileDescriptor !== null) await closeFileDescriptor(fileDescriptor).catch(() => undefined); + } +} diff --git a/tests/helpers/zip-fixture.ts b/tests/helpers/zip-fixture.ts new file mode 100644 index 0000000..5a1f6c4 --- /dev/null +++ b/tests/helpers/zip-fixture.ts @@ -0,0 +1,148 @@ +import { deflateRawSync } from "node:zlib"; + +export interface ZipFixtureEntry { + name: string | Uint8Array; + content?: string | Uint8Array; + method?: 0 | 8 | number; + flags?: number; + localMethod?: 0 | 8 | number; + localFlags?: number; + descriptor?: "none" | "signed-32" | "unsigned-32" | "signed-64" | "unsigned-64"; + zip64?: boolean; + localZip64?: boolean; + centralLocalOffset?: number; + externalFileAttributes?: number; + centralName?: string | Uint8Array; + localName?: string | Uint8Array; + centralCrc32?: number; + localCrc32?: number; + centralCompressedSize?: number; + centralUncompressedSize?: number; + localCompressedSize?: number; + localUncompressedSize?: number; + extra?: Uint8Array; +} + +export interface ZipFixtureOptions { + comment?: string | Uint8Array; + diskNumber?: number; + centralDirectoryDisk?: number; + entryCountOnDisk?: number; + centralDirectoryOffset?: number; + zip64?: boolean; + zip64EocdOffset?: number; + zip64RecordSize?: number; +} + +function bytes(value: string | Uint8Array | undefined): Buffer { + if (value === undefined) return Buffer.alloc(0); + return typeof value === "string" ? Buffer.from(value, "utf8") : Buffer.from(value); +} + +function u16(value: number): Buffer { + const result = Buffer.alloc(2); + result.writeUInt16LE(value >>> 0, 0); + return result; +} + +function u32(value: number): Buffer { + const result = Buffer.alloc(4); + result.writeUInt32LE(value >>> 0, 0); + return result; +} + +function u64(value: number): Buffer { + const result = Buffer.alloc(8); + result.writeBigUInt64LE(BigInt(value)); + return result; +} + +const crcTable = Array.from({ length: 256 }, (_, index) => { + let value = index; + for (let bit = 0; bit < 8; bit += 1) value = (value & 1) === 0 ? value >>> 1 : (value >>> 1) ^ 0xedb88320; + return value >>> 0; +}); + +export function crc32(content: Uint8Array): number { + let value = 0xffffffff; + for (const byte of content) value = (value >>> 8) ^ crcTable[(value ^ byte) & 0xff]; + return (value ^ 0xffffffff) >>> 0; +} + +/** A deterministic minimal ZIP writer for reader tests; it never invokes an archive utility. */ +export function createZipFixture(entries: readonly ZipFixtureEntry[], options: ZipFixtureOptions = {}): Buffer { + const localParts: Buffer[] = []; + const centralParts: Buffer[] = []; + let localOffset = 0; + + for (const item of entries) { + const useZip64 = item.zip64 ?? options.zip64 ?? false; + const useLocalZip64 = item.localZip64 ?? false; + const payload = bytes(item.content); + const method = item.method ?? 0; + const compressed = method === 8 ? deflateRawSync(payload) : payload; + const descriptor = item.descriptor ?? "none"; + const flags = (item.flags ?? 0) | (descriptor === "none" ? 0 : 0x0008); + const localMethod = item.localMethod ?? method; + const localFlags = item.localFlags ?? flags; + const centralName = bytes(item.centralName ?? item.name); + const localName = bytes(item.localName ?? item.name); + const extra = Buffer.from(item.extra ?? []); + const crc = crc32(payload); + const centralCrc = item.centralCrc32 ?? crc; + const localCrc = descriptor === "none" ? (item.localCrc32 ?? crc) : (item.localCrc32 ?? 0); + const centralCompressedSize = item.centralCompressedSize ?? compressed.length; + const centralUncompressedSize = item.centralUncompressedSize ?? payload.length; + const localCompressedSize = descriptor === "none" ? (item.localCompressedSize ?? compressed.length) : (item.localCompressedSize ?? 0); + const localUncompressedSize = descriptor === "none" ? (item.localUncompressedSize ?? payload.length) : (item.localUncompressedSize ?? 0); + const zip64Extra = useZip64 ? Buffer.concat([ + u16(0x0001), u16(16), u64(centralUncompressedSize), u64(centralCompressedSize) + ]) : Buffer.alloc(0); + const centralExtra = Buffer.concat([zip64Extra, extra]); + const localZip64Extra = useLocalZip64 && descriptor === "none" ? Buffer.concat([ + u16(0x0001), u16(16), u64(localUncompressedSize), u64(localCompressedSize) + ]) : Buffer.alloc(0); + const localExtra = Buffer.concat([localZip64Extra, extra]); + const localHeader = Buffer.concat([ + u32(0x04034b50), u16(useLocalZip64 ? 45 : 20), u16(localFlags), u16(localMethod), u16(0), u16(0), + u32(localCrc), u32(useLocalZip64 ? 0xffffffff : localCompressedSize), u32(useLocalZip64 ? 0xffffffff : localUncompressedSize), u16(localName.length), u16(localExtra.length), localName, localExtra + ]); + const descriptorBytes = descriptor === "none" ? Buffer.alloc(0) : Buffer.concat([ + descriptor.startsWith("signed") ? u32(0x08074b50) : Buffer.alloc(0), + u32(centralCrc), + descriptor.endsWith("64") ? u64(centralCompressedSize) : u32(centralCompressedSize), + descriptor.endsWith("64") ? u64(centralUncompressedSize) : u32(centralUncompressedSize) + ]); + localParts.push(localHeader, compressed, descriptorBytes); + const centralHeader = Buffer.concat([ + u32(0x02014b50), u16(0x031e), u16(useZip64 ? 45 : 20), u16(flags), u16(method), u16(0), u16(0), + u32(centralCrc), u32(useZip64 ? 0xffffffff : centralCompressedSize), u32(useZip64 ? 0xffffffff : centralUncompressedSize), + u16(centralName.length), u16(centralExtra.length), u16(0), u16(0), u16(0), u32(item.externalFileAttributes ?? 0), u32(item.centralLocalOffset ?? localOffset), centralName, centralExtra + ]); + centralParts.push(centralHeader); + localOffset += localHeader.length + compressed.length + descriptorBytes.length; + } + + const centralDirectory = Buffer.concat(centralParts); + const centralOffset = options.centralDirectoryOffset ?? localOffset; + const prefix = Buffer.concat(localParts); + const padding = centralOffset >= prefix.length ? Buffer.alloc(centralOffset - prefix.length) : Buffer.alloc(0); + const comment = bytes(options.comment); + const zip64EocdOffset = prefix.length + padding.length + centralDirectory.length; + const zip64Records = options.zip64 ? Buffer.concat([ + u32(0x06064b50), u64(options.zip64RecordSize ?? 44), u16(45), u16(45), u32(0), u32(0), u64(entries.length), u64(entries.length), u64(centralDirectory.length), u64(centralOffset), + u32(0x07064b50), u32(0), u64(options.zip64EocdOffset ?? zip64EocdOffset), u32(1) + ]) : Buffer.alloc(0); + const eocd = Buffer.concat([ + u32(0x06054b50), u16(options.diskNumber ?? 0), u16(options.centralDirectoryDisk ?? 0), + u16(options.zip64 ? 0xffff : (options.entryCountOnDisk ?? entries.length)), u16(options.zip64 ? 0xffff : entries.length), + u32(options.zip64 ? 0xffffffff : centralDirectory.length), u32(options.zip64 ? 0xffffffff : centralOffset), u16(comment.length), comment + ]); + return Buffer.concat([prefix, padding, centralDirectory, zip64Records, eocd]); +} + +export function overwriteUInt32(buffer: Uint8Array, offset: number, value: number): Buffer { + const result = Buffer.from(buffer); + result.writeUInt32LE(value >>> 0, offset); + return result; +} diff --git a/tests/submission-archive-reader.test.ts b/tests/submission-archive-reader.test.ts new file mode 100644 index 0000000..a3de8ce --- /dev/null +++ b/tests/submission-archive-reader.test.ts @@ -0,0 +1,422 @@ +import { mkdtemp, readFile, rm, truncate, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { inspectSubmissionArchive } from "../src/core/submission-archive-reader.js"; +import { createZipFixture, crc32 } from "./helpers/zip-fixture.js"; + +const temporaryDirectories: string[] = []; + +async function inspectFixture(content: Uint8Array, name = "plugin.zip") { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-reader-")); + temporaryDirectories.push(directory); + const archivePath = path.join(directory, name); + await writeFile(archivePath, content); + return inspectSubmissionArchive(archivePath); +} + +async function inspectSparseArchive(size: number) { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-reader-size-")); + temporaryDirectories.push(directory); + const archivePath = path.join(directory, "plugin.zip"); + await writeFile(archivePath, new Uint8Array([0])); + await truncate(archivePath, size); + return inspectSubmissionArchive(archivePath); +} + +function findingIds(inspection: Awaited>): string[] { + return inspection.findings.map((finding) => finding.id); +} + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { + recursive: true, + force: true, + maxRetries: 3, + retryDelay: 50 + }))); +}); + +describe("submission archive reader", () => { + it("accepts stored and deflated entries with EOCD comments and exposes a reader only after the safety pass", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: ".codex-plugin/plugin.json", content: "{}", method: 0 }, + { name: "skills/check/SKILL.md", content: "# check", method: 8 } + ], { comment: "safe comment" })); + + expect(inspection.findings).toEqual([]); + expect(inspection.reader).not.toBeNull(); + expect(inspection.entries.map((entry) => entry.packagePath)).toEqual([ + ".codex-plugin/plugin.json", "skills/check/SKILL.md" + ]); + await expect(inspection.reader?.read("skills/check/SKILL.md", 100)).resolves.toEqual(new TextEncoder().encode("# check")); + }); + + it("rejects non-ZIP, empty, truncated, multi-disk, and encrypted inputs without throwing", async () => { + for (const [name, content] of [ + ["not-a-zip.txt", new Uint8Array([1])], + ["empty.zip", new Uint8Array()], + ["truncated.zip", createZipFixture([{ name: "a.txt", content: "a" }]).subarray(0, 15)], + ["multi.zip", createZipFixture([{ name: "a.txt", content: "a" }], { diskNumber: 1 })], + ["encrypted.zip", createZipFixture([{ name: "a.txt", content: "a", flags: 1 }])] + ] as const) { + const inspection = await inspectFixture(content, name); + expect(inspection.reader).toBeNull(); + expect(inspection.findings.length).toBeGreaterThan(0); + } + }); + + it("rejects invalid entry paths without retaining unsafe raw names", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "../private.txt", content: "secret" }, + { name: "safe\\windows.txt", content: "secret" }, + { name: "/absolute.txt", content: "secret" } + ])); + + expect(findingIds(inspection)).toContain("plugin.submission.archive.path_invalid"); + expect(JSON.stringify(inspection)).not.toContain("../private.txt"); + expect(JSON.stringify(inspection)).not.toContain("safe\\windows.txt"); + expect(JSON.stringify(inspection)).not.toContain("secret"); + expect(inspection.reader).toBeNull(); + }); + + it("rejects central and local name, method, and metadata mismatches", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "safe.txt", localName: "other.txt", content: "body" }, + { name: "method.txt", content: "body", method: 8, localCompressedSize: 2 } + ])); + + expect(findingIds(inspection)).toContain("plugin.submission.archive.header_mismatch"); + expect(inspection.reader).toBeNull(); + }); + + it("reports unsupported compression as unavailable coverage without treating the archive as malformed", async () => { + const inspection = await inspectFixture(createZipFixture([{ name: "unsupported.bin", content: "data", method: 12 }])); + + expect(inspection.findings).toEqual([]); + expect(inspection.reader).toBeNull(); + expect(inspection.coverage).toContainEqual(expect.objectContaining({ + id: "plugin.submission.archive.compression_method", + status: "unavailable" + })); + }); + + it("rejects declared CRC and output-size mismatches even for entries no nested validator requests", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "unused.txt", content: "unrequested", centralCrc32: 0x12345678, localCrc32: 0x12345678 }, + { name: "size.txt", content: "size", centralUncompressedSize: 3 } + ])); + + expect(findingIds(inspection)).toContain("plugin.submission.archive.crc_mismatch"); + expect(findingIds(inspection)).toContain("plugin.submission.archive.header_mismatch"); + expect(inspection.reader).toBeNull(); + }); + + it("rejects duplicate paths, file-directory conflicts, and unsafe type metadata", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "same.txt", content: "one" }, + { name: "same.txt", content: "two" }, + { name: "parent", content: "file" }, + { name: "parent/child.txt", content: "child" }, + { name: "link", content: "target", externalFileAttributes: 0o120777 << 16 } + ])); + + expect(findingIds(inspection)).toContain("plugin.submission.archive.path_duplicate"); + expect(findingIds(inspection)).toContain("plugin.submission.archive.path_conflict"); + expect(findingIds(inspection)).toContain("plugin.submission.archive.type_unsupported"); + expect(inspection.reader).toBeNull(); + }); + + it("supports signed and unsigned data descriptors and rejects a missing descriptor", async () => { + const valid = await inspectFixture(createZipFixture([ + { name: "signed.txt", content: "signed", descriptor: "signed-32" }, + { name: "unsigned.txt", content: "unsigned", descriptor: "unsigned-32" } + ])); + expect(valid.findings).toEqual([]); + expect(valid.reader).not.toBeNull(); + + const invalid = await inspectFixture(createZipFixture([ + { name: "missing.txt", content: "missing", descriptor: "none", flags: 0x0008 } + ])); + expect(findingIds(invalid)).toContain("plugin.submission.archive.descriptor_invalid"); + }); + + it("accepts local ZIP64 size sentinels and rejects missing or malformed required local ZIP64 data", async () => { + const name = "local-zip64.txt"; + const archive = createZipFixture([ + { name, content: "local zip64", zip64: true, localZip64: true } + ], { zip64: true }); + const valid = await inspectFixture(archive); + expect(valid.findings).toEqual([]); + expect(valid.reader).not.toBeNull(); + + const localExtraOffset = 30 + Buffer.byteLength(name); + const missing = Buffer.from(archive); + missing.writeUInt16LE(0x0002, localExtraOffset); + const missingInspection = await inspectFixture(missing); + expect(findingIds(missingInspection)).toContain("plugin.submission.archive.range_invalid"); + expect(missingInspection.reader).toBeNull(); + + const malformed = Buffer.from(archive); + malformed.writeUInt16LE(15, localExtraOffset + 2); + const malformedInspection = await inspectFixture(malformed); + expect(findingIds(malformedInspection)).toContain("plugin.submission.archive.range_invalid"); + expect(malformedInspection.reader).toBeNull(); + }); + + it("validates ZIP64, intervals, types, unused bombs, and its non-executing boundary", async () => { + const zip64 = await inspectFixture(createZipFixture([ + { name: "zip64.txt", content: "zip64", zip64: true, descriptor: "signed-64" } + ], { zip64: true })); + expect(zip64.findings).toEqual([]); + await expect(zip64.reader?.read("zip64.txt", 10)).resolves.toEqual(new TextEncoder().encode("zip64")); + + const centralOverlap = await inspectFixture(createZipFixture([{ name: "a.txt", content: "a" }], { centralDirectoryOffset: 0 })); + expect(findingIds(centralOverlap)).toContain("plugin.submission.archive.range_invalid"); + + const localOverlap = await inspectFixture(createZipFixture([ + { name: "shared.txt", content: "shared" }, + { name: "shared.txt", content: "shared", centralLocalOffset: 0 } + ])); + expect(findingIds(localOverlap)).toContain("plugin.submission.archive.range_invalid"); + + const contradictory = await inspectFixture(createZipFixture([ + { name: "regular.txt", content: "body", externalFileAttributes: (0o100644 << 16) | 0x10 } + ])); + expect(findingIds(contradictory)).toContain("plugin.submission.archive.type_unsupported"); + + const bomb = await inspectFixture(createZipFixture([ + { name: "unused-bomb.txt", content: "A".repeat(16 * 1024), method: 8, centralUncompressedSize: 1, localUncompressedSize: 1 } + ])); + expect(findingIds(bomb)).toContain("plugin.submission.archive.crc_mismatch"); + expect(JSON.stringify(bomb)).not.toContain("A".repeat(32)); + + const source = await readFile(new URL("../src/core/submission-archive-reader.ts", import.meta.url), "utf8"); + const nodeFsImports = source.match(/^import .* from "node:fs(?:\/promises)?";$/gmu) ?? []; + expect(nodeFsImports).toEqual([ + 'import { close, fstat, open, read, type Stats } from "node:fs";', + 'import { stat } from "node:fs/promises";' + ]); + expect(source).not.toMatch( + /node:child_process|\b(?:writeFile|appendFile|mkdir|rm|rename|copyFile|createWriteStream|truncate|unlink|symlink|link)\s*\(|\bfetch\s*\(/u + ); + const fetchCalls: unknown[][] = []; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (...args: unknown[]) => { + fetchCalls.push(args); + throw new Error("network must not be used"); + }) as typeof fetch; + try { + const safe = await inspectFixture(createZipFixture([{ name: "safe.txt", content: "safe" }])); + expect(safe.reader).not.toBeNull(); + expect(fetchCalls).toEqual([]); + } finally { + globalThis.fetch = originalFetch; + } + }); + + it("refuses a reader request after its inspected archive is replaced", async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-reader-binding-")); + temporaryDirectories.push(directory); + const archivePath = path.join(directory, "plugin.zip"); + await writeFile(archivePath, createZipFixture([{ name: "safe.txt", content: "inspected" }])); + const inspection = await inspectSubmissionArchive(archivePath); + await writeFile(archivePath, createZipFixture([{ name: "safe.txt", content: "tampered!" }])); + + await expect(inspection.reader?.read("safe.txt", 100)).resolves.toBeNull(); + }); + + it("rejects a ZIP64 record whose declared size cannot contain its mandatory fields", async () => { + const archive = createZipFixture([{ name: "zip64.txt", content: "zip64", zip64: true }], { zip64: true }); + const zip64RecordOffset = archive.indexOf(Buffer.from([0x50, 0x4b, 0x06, 0x06])); + archive.writeBigUInt64LE(1n, zip64RecordOffset + 4); + + const inspection = await inspectFixture(archive); + expect(findingIds(inspection)).toContain("plugin.submission.archive.range_invalid"); + }); + + it("rejects a parseable ZIP64 local interval that crosses the central directory", async () => { + const archive = createZipFixture([{ + name: "crosses.bin", + content: "", + zip64: true, + centralCompressedSize: 80, + localCompressedSize: 80 + }], { zip64: true }); + const centralDirectoryOffset = archive.indexOf(Buffer.from([0x50, 0x4b, 0x01, 0x02])); + const localDataStart = 30 + Buffer.byteLength("crosses.bin"); + expect(localDataStart).toBe(centralDirectoryOffset); + expect(localDataStart + 80).toBeLessThanOrEqual(archive.length); + expect(localDataStart + 80).toBeGreaterThan(centralDirectoryOffset); + + const inspection = await inspectFixture(archive); + expect(findingIds(inspection)).not.toContain("plugin.submission.archive.invalid_zip"); + expect(findingIds(inspection)).toContain("plugin.submission.archive.range_invalid"); + expect(inspection.reader).toBeNull(); + }); + + it("decodes CP437 entry names and rejects fatal UTF-8", async () => { + const cp437 = await inspectFixture(createZipFixture([{ name: new Uint8Array([0x82, 0x2e, 0x74, 0x78, 0x74]), content: "x" }])); + expect(cp437.entries[0]?.packagePath).toBe("é.txt"); + + const invalidUtf8 = await inspectFixture(createZipFixture([{ name: new Uint8Array([0xff]), content: "x", flags: 0x0800 }])); + expect(findingIds(invalidUtf8)).toContain("plugin.submission.archive.path_invalid"); + }); + + it("terminates deterministically for randomized malformed buffers", async () => { + for (let index = 0; index < 24; index += 1) { + const content = Uint8Array.from({ length: index + 1 }, (_, byteIndex) => (index * 37 + byteIndex * 17) & 0xff); + await expect(inspectFixture(content)).resolves.toEqual(expect.objectContaining({ reader: null })); + } + }); + + it("uses the decimal 100,000,000-byte compressed archive limit before ZIP parsing", async () => { + const atLimit = await inspectSparseArchive(100_000_000); + expect(findingIds(atLimit)).not.toContain("plugin.submission.archive.too_large"); + + const overLimit = await inspectSparseArchive(100_000_001); + expect(overLimit.findings).toContainEqual(expect.objectContaining({ + id: "plugin.submission.archive.too_large", + evidence: { limit: 100_000_000 } + })); + }); + + it("distinguishes the 5,000-entry archive limit from 5,001 entries", async () => { + const atLimitEntries = Array.from({ length: 5_000 }, (_, index) => ({ name: `entries/${index}.txt`, content: "" })); + const atLimit = await inspectFixture(createZipFixture(atLimitEntries)); + expect(findingIds(atLimit)).not.toContain("plugin.submission.archive.entry_count"); + + const overLimitEntries = Array.from({ length: 5_001 }, (_, index) => ({ name: `entries/${index}.txt`, content: "" })); + const overLimit = await inspectFixture(createZipFixture(overLimitEntries)); + expect(overLimit.findings).toContainEqual(expect.objectContaining({ + id: "plugin.submission.archive.entry_count", + evidence: { limit: 5_000 } + })); + }); + + it("distinguishes the 100-MiB declared member limit from one byte above it", async () => { + const memberLimit = 100 * 1024 * 1024; + const atLimit = await inspectFixture(createZipFixture([{ + name: "at-limit.bin", + content: "", + centralUncompressedSize: memberLimit, + localUncompressedSize: memberLimit + }])); + expect(findingIds(atLimit)).not.toContain("plugin.submission.archive.member_too_large"); + + const overLimit = await inspectFixture(createZipFixture([{ + name: "over-limit.bin", + content: "", + centralUncompressedSize: memberLimit + 1, + localUncompressedSize: memberLimit + 1 + }])); + expect(overLimit.findings).toContainEqual(expect.objectContaining({ + id: "plugin.submission.archive.member_too_large", + evidence: expect.objectContaining({ limit: memberLimit }) + })); + }); + + it("distinguishes the 512-MiB aggregate declared limit from one byte above it", async () => { + const memberLimit = 100 * 1024 * 1024; + const totalLimit = 512 * 1024 * 1024; + const declaredEntries = (lastSize: number) => [ + ...Array.from({ length: 5 }, (_, index) => ({ + name: `members/${index}.bin`, + content: "", + centralUncompressedSize: memberLimit, + localUncompressedSize: memberLimit + })), + { + name: "members/final.bin", + content: "", + centralUncompressedSize: lastSize, + localUncompressedSize: lastSize + } + ]; + const atLimit = await inspectFixture(createZipFixture(declaredEntries(totalLimit - (5 * memberLimit)))); + expect(findingIds(atLimit)).not.toContain("plugin.submission.archive.total_too_large"); + + const overLimit = await inspectFixture(createZipFixture(declaredEntries(totalLimit - (5 * memberLimit) + 1))); + expect(overLimit.findings).toContainEqual(expect.objectContaining({ + id: "plugin.submission.archive.total_too_large", + evidence: { limit: totalLimit } + })); + }); + + it("rejects drive-prefixed, empty-segment, deep, and control-character paths", async () => { + const deepPath = Array.from({ length: 21 }, (_, index) => `segment-${index}`).join("/"); + const inspection = await inspectFixture(createZipFixture([ + { name: "C:drive.txt", content: "x" }, + { name: "empty//segment.txt", content: "x" }, + { name: deepPath, content: "x" }, + { name: "control\u0001.txt", content: "x" } + ])); + + expect(findingIds(inspection).filter((id) => id === "plugin.submission.archive.path_invalid")).toHaveLength(4); + expect(JSON.stringify(inspection)).not.toContain("C:drive.txt"); + expect(inspection.reader).toBeNull(); + }); + + it("reports Unicode Path decoding ambiguity as unavailable coverage", async () => { + const rawName = Buffer.from("raw-name.txt"); + const unicodeName = Buffer.from("unicode-name.txt"); + const extra = Buffer.alloc(9 + unicodeName.length); + extra.writeUInt16LE(0x7075, 0); + extra.writeUInt16LE(5 + unicodeName.length, 2); + extra[4] = 1; + extra.writeUInt32LE(crc32(rawName), 5); + unicodeName.copy(extra, 9); + + const inspection = await inspectFixture(createZipFixture([{ name: rawName, content: "x", extra }])); + expect(inspection.findings).toEqual([]); + expect(inspection.coverage).toContainEqual(expect.objectContaining({ + id: "plugin.submission.archive.filename_decoding", + status: "unavailable" + })); + }); + + it("warns for NFKC and case path collisions without failing by that warning alone", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "A\u030A.txt", content: "first", flags: 0x0800 }, + { name: "\u00C5.txt", content: "second", flags: 0x0800 } + ])); + + expect(inspection.findings).toEqual([expect.objectContaining({ + id: "plugin.submission.archive.normalization_collision", + severity: "warn" + })]); + expect(inspection.reader).not.toBeNull(); + }); + + it("rejects local general-purpose flag and compression-method mismatches", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "flag.txt", content: "flag", flags: 0x0008, localFlags: 0, descriptor: "signed-32" }, + { name: "method.txt", content: "method", method: 8, localMethod: 0 } + ])); + + expect(findingIds(inspection).filter((id) => id === "plugin.submission.archive.header_mismatch")).toHaveLength(2); + }); + + it("accepts an unsigned ZIP64 data descriptor", async () => { + const inspection = await inspectFixture(createZipFixture([ + { name: "zip64.txt", content: "zip64", zip64: true, descriptor: "unsigned-64" } + ], { zip64: true })); + + expect(inspection.findings).toEqual([]); + expect(inspection.reader).not.toBeNull(); + }); + + it("rejects ZIP64 locator metadata overlap and malformed ZIP64 record sizes", async () => { + const locatorOverlap = await inspectFixture(createZipFixture([ + { name: "zip64.txt", content: "zip64", zip64: true } + ], { zip64: true, zip64EocdOffset: 0 })); + expect(findingIds(locatorOverlap)).toContain("plugin.submission.archive.range_invalid"); + + const shortRecord = await inspectFixture(createZipFixture([ + { name: "zip64.txt", content: "zip64", zip64: true } + ], { zip64: true, zip64RecordSize: 1 })); + expect(findingIds(shortRecord)).toContain("plugin.submission.archive.range_invalid"); + }); +}); From 4c923ab46a8c6868f7ef52184b0af044841462e6 Mon Sep 17 00:00:00 2001 From: Furkan Date: Sun, 23 Aug 2026 18:32:32 +0300 Subject: [PATCH 15/20] feat: add submission archive preflight --- src/core/submission-archive-preflight.ts | 316 +++++++++++++++++++++ src/core/submission-archive-reader.ts | 14 +- src/core/submission-archive-ruleset.ts | 63 ++++ src/core/submission-preflight.ts | 7 +- tests/submission-archive-preflight.test.ts | 266 +++++++++++++++++ tests/submission-archive-reader.test.ts | 11 + 6 files changed, 673 insertions(+), 4 deletions(-) create mode 100644 src/core/submission-archive-preflight.ts create mode 100644 src/core/submission-archive-ruleset.ts create mode 100644 tests/submission-archive-preflight.test.ts diff --git a/src/core/submission-archive-preflight.ts b/src/core/submission-archive-preflight.ts new file mode 100644 index 0000000..856bb8e --- /dev/null +++ b/src/core/submission-archive-preflight.ts @@ -0,0 +1,316 @@ +import type { PluginManifest } from "../domain/types.js"; +import { validateSubmissionAssetsFromReader } from "./submission-assets.js"; +import { + inspectSubmissionArchive, + type SubmissionArchiveCoverage, + type SubmissionArchiveFinding, + type SubmissionArchiveInspection +} from "./submission-archive-reader.js"; +import { submissionArchiveRuleset } from "./submission-archive-ruleset.js"; +import type { SubmissionPackageEntry, SubmissionPackageReader } from "./submission-package-reader.js"; +import { + type SubmissionCheck, + type SubmissionFinding, + type SubmissionManualCheck, + type SubmissionPreflightReport, + type SubmissionTargetType, + validateSubmissionListing +} from "./submission-preflight.js"; +import { submissionManualChecks } from "./submission-ruleset.js"; +import { validateSubmissionSkillMetadataFromReader } from "./submission-skill-metadata.js"; + +const manifestPaths = [".codex-plugin/plugin.json", ".agent-plugin/plugin.json", ".claude-plugin/plugin.json"] as const; +const maxManifestBytes = 1024 * 1024; + +export type SubmissionArchiveRootLayout = "archive-root" | "single-top-level-directory" | "unavailable"; + +export interface SubmissionArchivePreflightReport { + schemaVersion: "1.0.0"; + rulesetVersion: "openai-directory-archive-2026-08-23"; + status: "pass" | "fail"; + readiness: "blocked" | "manual_review_required"; + archive: { + fileName: string; + compressedBytes: number; + uncompressedBytes: number; + entryCount: number; + rootLayout: SubmissionArchiveRootLayout; + }; + summary: SubmissionPreflightReport["summary"]; + archiveChecks: readonly SubmissionCheck[]; + submission: SubmissionPreflightReport | null; + findings: readonly SubmissionArchiveFinding[] | readonly (SubmissionArchiveFinding | SubmissionFinding)[]; + coverage: readonly SubmissionArchiveCoverage[]; + manualChecklist: readonly SubmissionManualCheck[]; +} + +interface RootDiscovery { + rootPrefix: string; + rootLayout: Exclude; + manifestPath: string; + manifestFormat: typeof manifestPaths[number]; +} + +function archiveFinding( + id: SubmissionArchiveFinding["id"], + severity: SubmissionArchiveFinding["severity"], + message: string, + portalCode?: string +): SubmissionArchiveFinding { + return portalCode === undefined ? { id, severity, message } : { id, severity, message, portalCode }; +} + +function archiveCoverage(): SubmissionArchiveCoverage[] { + return submissionArchiveRuleset.portalRules.map(({ id, status, reason }) => ({ id, status, reason })); +} + +function checkStatus(findings: readonly SubmissionFinding[]): SubmissionCheck["status"] { + return findings.some((finding) => finding.severity === "fail") + ? "fail" + : findings.some((finding) => finding.severity === "warn") ? "warn" : "pass"; +} + +function submissionCheck(id: SubmissionCheck["id"], findings: SubmissionFinding[]): SubmissionCheck { + return { id, status: checkStatus(findings), findingIds: findings.map((finding) => finding.id) }; +} + +function manualChecklist(): SubmissionManualCheck[] { + return submissionManualChecks.map((item) => ({ + id: item.id, + label: item.label, + state: item.mcpOnly ? "not_applicable" : "required" + })); +} + +function reportSummary(checks: readonly SubmissionCheck[], findings: readonly SubmissionFinding[], checklist: readonly SubmissionManualCheck[]) { + return { + passed: checks.filter((check) => check.status === "pass").length, + warnings: findings.filter((finding) => finding.severity === "warn").length, + blockers: findings.filter((finding) => finding.severity === "fail").length, + manualChecks: checklist.filter((item) => item.state === "required").length + }; +} + +function normalizePath(value: string, allowRoot = false): string | null { + if (value === "" && allowRoot) return ""; + if (typeof value !== "string" || value === "" || value.startsWith("/") || value.startsWith("\\") + || /^[A-Za-z]:/u.test(value) || value.includes("\\") || /[\u0000-\u001F\u007F]/u.test(value)) return null; + const segments = value.split("/"); + return segments.some((segment) => segment === "" || segment === "." || segment === "..") ? null : segments.join("/"); +} + +function discoverRoot(entries: readonly SubmissionPackageEntry[]): { discovery: RootDiscovery | null; findings: SubmissionArchiveFinding[] } { + const paths = entries.map((entry) => entry.packagePath); + if (paths.length === 0) { + return { discovery: null, findings: [archiveFinding("plugin.submission.archive.root_missing", "fail", "Archive does not contain a plugin root.")] }; + } + const rootManifests = manifestPaths.filter((manifestPath) => paths.includes(manifestPath)); + const nestedRoots = new Set(); + for (const entryPath of paths) { + const separator = entryPath.indexOf("/"); + if (separator > 0 && manifestPaths.includes(entryPath.slice(separator + 1) as typeof manifestPaths[number])) { + nestedRoots.add(entryPath.slice(0, separator)); + } + } + if (rootManifests.length + nestedRoots.size > 1) { + return { discovery: null, findings: [archiveFinding("plugin.submission.archive.root_ambiguous", "fail", "Archive contains more than one plugin root.")] }; + } + if (rootManifests.length === 1) { + return { + discovery: { rootPrefix: "", rootLayout: "archive-root", manifestPath: rootManifests[0]!, manifestFormat: rootManifests[0]! }, + findings: [] + }; + } + if (nestedRoots.size === 0) { + return { discovery: null, findings: [archiveFinding("plugin.submission.archive.manifest_missing", "fail", "Archive root does not contain a recognized plugin manifest.")] }; + } + const hasRootFile = entries.some((entry) => !entry.packagePath.includes("/") && entry.resolvedKind === "file"); + const topLevels = new Set(paths.map((entryPath) => entryPath.split("/", 1)[0]!)); + if (hasRootFile || nestedRoots.size !== 1 || topLevels.size !== 1) { + return { discovery: null, findings: [archiveFinding("plugin.submission.archive.root_siblings", "fail", "A top-level plugin directory cannot have siblings.")] }; + } + const topLevel = [...nestedRoots][0]!; + const prefix = `${topLevel}/`; + const nestedManifests = manifestPaths.filter((manifestPath) => paths.includes(`${prefix}${manifestPath}`)); + if (nestedManifests.length === 0) { + return { discovery: null, findings: [archiveFinding("plugin.submission.archive.manifest_missing", "fail", "Archive root does not contain a recognized plugin manifest.")] }; + } + if (nestedManifests.length > 1) { + return { discovery: null, findings: [archiveFinding("plugin.submission.archive.root_ambiguous", "fail", "Archive contains more than one plugin manifest.")] }; + } + return { + discovery: { + rootPrefix: prefix, + rootLayout: "single-top-level-directory", + manifestPath: `${prefix}${nestedManifests[0]!}`, + manifestFormat: nestedManifests[0]! + }, + findings: [] + }; +} + +function rootedReader(inspection: SubmissionArchiveInspection, rootPrefix: string): SubmissionPackageReader | null { + if (inspection.reader === null) return null; + const entries = inspection.entries + .filter((entry) => entry.packagePath.startsWith(rootPrefix)) + .map((entry) => ({ ...entry, packagePath: entry.packagePath.slice(rootPrefix.length) })); + const byPath = new Map(entries.map((entry) => [entry.packagePath, entry])); + const reader = inspection.reader; + + function directoryEntry(packagePath: string): SubmissionPackageEntry | null { + if (entries.some((entry) => entry.packagePath.startsWith(`${packagePath}/`))) { + return { packagePath, kind: "directory", resolvedKind: "directory", size: 0, safeResolution: "safe" }; + } + return null; + } + + return { + async list(directory: string): Promise { + const normalizedDirectory = normalizePath(directory, true); + if (normalizedDirectory === null) return []; + const prefix = normalizedDirectory === "" ? "" : `${normalizedDirectory}/`; + const children = new Map(); + for (const entry of entries) { + if (!entry.packagePath.startsWith(prefix)) continue; + const remaining = entry.packagePath.slice(prefix.length); + if (remaining === "") continue; + const [name] = remaining.split("/", 1); + const packagePath = normalizedDirectory === "" ? name! : `${normalizedDirectory}/${name!}`; + if (remaining.includes("/")) { + children.set(packagePath, directoryEntry(packagePath)!); + } else { + children.set(packagePath, entry); + } + } + return [...children.values()].sort((left, right) => left.packagePath.localeCompare(right.packagePath)); + }, + async stat(packagePath: string): Promise { + const normalizedPath = normalizePath(packagePath); + return normalizedPath === null ? null : byPath.get(normalizedPath) ?? directoryEntry(normalizedPath); + }, + async read(packagePath: string, maxBytes: number): Promise { + const normalizedPath = normalizePath(packagePath); + if (normalizedPath === null || !byPath.has(normalizedPath)) return null; + return reader.read(`${rootPrefix}${normalizedPath}`, maxBytes); + } + }; +} + +async function parseManifest(reader: SubmissionPackageReader, manifestPath: string): Promise { + const bytes = await reader.read(manifestPath, maxManifestBytes).catch(() => null); + if (bytes === null) return null; + try { + const parsed: unknown = JSON.parse(new TextDecoder("utf-8", { fatal: true }).decode(bytes)); + return typeof parsed === "object" && parsed !== null && !Array.isArray(parsed) ? parsed as PluginManifest : null; + } catch { + return null; + } +} + +function hasImmediateSkill(entries: readonly SubmissionPackageEntry[]): boolean { + return entries.some((entry) => /^skills\/[^/]+\/SKILL\.md$/u.test(entry.packagePath) && entry.resolvedKind === "file"); +} + +function exclusionFindings(manifest: PluginManifest, entries: readonly SubmissionPackageEntry[]): SubmissionArchiveFinding[] { + const findings: SubmissionArchiveFinding[] = []; + const interfaceValue = manifest.interface; + if (manifest.mcpServers !== undefined || entries.some((entry) => entry.packagePath === ".mcp.json")) { + findings.push(archiveFinding("plugin.submission.archive.mcp_excluded", "warn", "MCP configuration requires the MCP-backed submission flow.", "mcp_configuration_excluded")); + } + if (manifest.apps !== undefined || entries.some((entry) => entry.packagePath === ".app.json")) { + findings.push(archiveFinding("plugin.submission.archive.app_excluded", "warn", "App configuration requires the MCP-backed submission flow.", "app_configuration_excluded")); + } + if (typeof interfaceValue === "object" && interfaceValue !== null && !Array.isArray(interfaceValue) + && "screenshots" in interfaceValue && (interfaceValue as Record).screenshots !== undefined) { + findings.push(archiveFinding("plugin.submission.archive.screenshot_excluded", "warn", "Screenshots require the MCP-backed submission flow.", "screenshot_configuration_excluded")); + } + return findings; +} + +async function nestedSubmission(manifest: PluginManifest, reader: SubmissionPackageReader): Promise { + const targetType: SubmissionTargetType = "skills-only"; + const listing = validateSubmissionListing(manifest as Record, targetType); + const components: SubmissionFinding[] = []; + const assets = (await validateSubmissionAssetsFromReader(manifest, reader)).findings; + const skills = (await validateSubmissionSkillMetadataFromReader(manifest, targetType, reader)).findings; + const checks = [ + submissionCheck("listing", listing), + submissionCheck("components", components), + submissionCheck("assets", assets), + submissionCheck("skills", skills) + ]; + const findings = [...listing, ...components, ...assets, ...skills]; + const manual = manualChecklist(); + const summary = reportSummary(checks, findings, manual); + return { + schemaVersion: "1.0.0", + rulesetVersion: "openai-directory-2026-08-15", + targetType, + status: summary.blockers > 0 ? "fail" : "pass", + readiness: summary.blockers > 0 ? "blocked" : "manual_review_required", + summary, + checks, + findings, + manualChecklist: manual + }; +} + +export function submissionArchiveExitCode(report: SubmissionArchivePreflightReport, requireReady: boolean): 0 | 1 { + return requireReady && report.status === "fail" ? 1 : 0; +} + +export async function buildSubmissionArchivePreflight(archivePath: string): Promise { + const inspection = await inspectSubmissionArchive(archivePath); + const coverage = [...inspection.coverage, ...archiveCoverage()]; + const manual = manualChecklist(); + const archiveFindings: SubmissionArchiveFinding[] = [...inspection.findings]; + let rootLayout: SubmissionArchiveRootLayout = "unavailable"; + let submission: SubmissionPreflightReport | null = null; + + if (!archiveFindings.some((finding) => finding.severity === "fail") && inspection.reader !== null) { + const root = discoverRoot(inspection.entries); + archiveFindings.push(...root.findings); + if (root.discovery !== null) { + rootLayout = root.discovery.rootLayout; + const reader = rootedReader(inspection, root.discovery.rootPrefix); + const manifest = reader === null ? null : await parseManifest(reader, root.discovery.manifestPath.slice(root.discovery.rootPrefix.length)); + if (manifest === null || reader === null) { + archiveFindings.push(archiveFinding("plugin.submission.archive.manifest_missing", "fail", "Plugin manifest must be a bounded UTF-8 JSON object.")); + } else { + const entries = inspection.entries + .filter((entry) => entry.packagePath.startsWith(root.discovery!.rootPrefix)) + .map((entry) => ({ ...entry, packagePath: entry.packagePath.slice(root.discovery!.rootPrefix.length) })); + if (!hasImmediateSkill(entries)) { + archiveFindings.push(archiveFinding("plugin.submission.archive.skill_missing", "fail", "Archive must contain an immediate skills//SKILL.md entrypoint.")); + } + archiveFindings.push(...exclusionFindings(manifest, entries)); + submission = await nestedSubmission(manifest, reader); + } + } + } + + const findings = [...archiveFindings, ...(submission?.findings ?? [])]; + const blockers = findings.filter((finding) => finding.severity === "fail").length; + const summary = submission === null + ? { passed: 0, warnings: findings.filter((finding) => finding.severity === "warn").length, blockers, manualChecks: manual.filter((item) => item.state === "required").length } + : { ...submission.summary, warnings: findings.filter((finding) => finding.severity === "warn").length, blockers }; + return { + schemaVersion: "1.0.0", + rulesetVersion: submissionArchiveRuleset.version, + status: blockers > 0 ? "fail" : "pass", + readiness: blockers > 0 ? "blocked" : "manual_review_required", + archive: { + fileName: inspection.fileName, + compressedBytes: inspection.compressedBytes, + uncompressedBytes: inspection.uncompressedBytes, + entryCount: inspection.entryCount, + rootLayout + }, + summary, + archiveChecks: [], + submission, + findings, + coverage, + manualChecklist: manual + }; +} diff --git a/src/core/submission-archive-reader.ts b/src/core/submission-archive-reader.ts index 1f8e4ca..eb689e9 100644 --- a/src/core/submission-archive-reader.ts +++ b/src/core/submission-archive-reader.ts @@ -15,6 +15,15 @@ const maxMemberBytes = 100 * 1024 * 1024; const maxTotalBytes = 512 * 1024 * 1024; const maxPathSegments = 20; const invalidPackagePathMessage = "Invalid package path."; +const unsafeArchiveFileName = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u202A-\u202E\u2060-\u206F\uFEFF]/u; + +function sanitizedArchiveFileName(value: unknown): string { + if (typeof value !== "string") return "archive.zip"; + const fileName = path.basename(value); + return fileName === "" || fileName.trim() !== fileName || unsafeArchiveFileName.test(fileName) + ? "archive.zip" + : fileName; +} export interface SubmissionArchiveFinding extends SubmissionFinding { id: `plugin.submission.archive.${string}`; @@ -367,6 +376,9 @@ async function readArchiveMetadata( if (disk !== 0 || centralDisk !== 0 || entriesOnDisk !== entryCount || centralStart32 > eocdOffset || centralSize32 > eocdOffset - centralStart32) return null; + if (entryCount === 0 && centralSize32 === 0 && centralStart32 === eocdOffset) { + return { centralStart: centralStart32, metadataStart: centralStart32 }; + } const signature = await readExactly(fileDescriptor, centralStart32, 4); return signature?.readUInt32LE(0) === 0x02014b50 ? { centralStart: centralStart32, metadataStart: centralStart32 } @@ -488,7 +500,7 @@ function createArchiveReader(zipPath: string, checkedEntries: readonly CheckedEn } export async function inspectSubmissionArchive(zipPath: string): Promise { - const fileName = typeof zipPath === "string" ? path.basename(zipPath) : "archive.zip"; + const fileName = sanitizedArchiveFileName(zipPath); let details: Stats; try { if (typeof zipPath !== "string" || path.extname(zipPath).toLowerCase() !== ".zip") { diff --git a/src/core/submission-archive-ruleset.ts b/src/core/submission-archive-ruleset.ts new file mode 100644 index 0000000..3cb752f --- /dev/null +++ b/src/core/submission-archive-ruleset.ts @@ -0,0 +1,63 @@ +const sources = Object.freeze([ + "https://developers.openai.com/plugins/build/plugins", + "https://developers.openai.com/plugins/deploy/submission-errors" +] as const); + +const portalRules = Object.freeze([ + Object.freeze({ id: "plugin.submission.archive.plugin_name_mismatch", status: "unavailable", reason: "Requires the previously published plugin identity.", source: "submission-errors" }), + Object.freeze({ id: "plugin.submission.archive.plugin_version_unchanged", status: "unavailable", reason: "Requires the previously published plugin version.", source: "submission-errors" }), + Object.freeze({ id: "plugin.submission.archive.manifest_normalized", status: "unavailable", reason: "The portal owns exact manifest normalization.", source: "submission-errors" }), + Object.freeze({ id: "plugin.submission.archive.developer_name_defaulted", status: "unavailable", reason: "Requires the selected verified developer identity.", source: "submission-errors" }), + Object.freeze({ id: "plugin.submission.archive.path_length", status: "unavailable", reason: "The public reference does not publish a numeric path-length limit.", source: "submission-errors" }), + Object.freeze({ id: "plugin.submission.archive.normalization_algorithm", status: "unavailable", reason: "The portal does not publish its case and Unicode collision algorithm.", source: "submission-errors" }), + Object.freeze({ id: "plugin.submission.archive.claude_format_normalized", status: "manual", reason: "The portal owns Claude-format normalization details.", source: "submission-errors" }) +] as const); + +export const submissionArchiveRuleset = Object.freeze({ + version: "openai-directory-archive-2026-08-23", + reviewedAt: "2026-08-23", + sources, + limits: Object.freeze({ + compressedBytes: 100 * 1000 * 1000, + entries: 5_000, + memberBytes: 100 * 1024 * 1024, + totalBytes: 512 * 1024 * 1024 + }), + structures: Object.freeze({ + eocdComments: "automatic", + zip64: "automatic", + dataDescriptors: "automatic" + }), + compression: Object.freeze({ + stored: "automatic", + deflate: "automatic" + }), + collisionAlgorithm: "NFKC + toLowerCase per segment", + automaticChecks: Object.freeze([ + "plugin.submission.archive.invalid_file", + "plugin.submission.archive.invalid_zip", + "plugin.submission.archive.too_large", + "plugin.submission.archive.entry_count", + "plugin.submission.archive.member_too_large", + "plugin.submission.archive.total_too_large", + "plugin.submission.archive.encrypted", + "plugin.submission.archive.header_mismatch", + "plugin.submission.archive.descriptor_invalid", + "plugin.submission.archive.range_invalid", + "plugin.submission.archive.crc_mismatch", + "plugin.submission.archive.path_invalid", + "plugin.submission.archive.path_duplicate", + "plugin.submission.archive.path_conflict", + "plugin.submission.archive.type_unsupported", + "plugin.submission.archive.normalization_collision", + "plugin.submission.archive.root_missing", + "plugin.submission.archive.root_ambiguous", + "plugin.submission.archive.root_siblings", + "plugin.submission.archive.manifest_missing", + "plugin.submission.archive.skill_missing", + "plugin.submission.archive.mcp_excluded", + "plugin.submission.archive.app_excluded", + "plugin.submission.archive.screenshot_excluded" + ] as const), + portalRules +}); diff --git a/src/core/submission-preflight.ts b/src/core/submission-preflight.ts index b15379e..5101752 100644 --- a/src/core/submission-preflight.ts +++ b/src/core/submission-preflight.ts @@ -39,7 +39,8 @@ export interface SubmissionPreflightReport { } type Evidence = SubmissionFinding["evidence"]; -type TargetType = SubmissionPreflightReport["targetType"]; +export type SubmissionTargetType = SubmissionPreflightReport["targetType"]; +type TargetType = SubmissionTargetType; 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-]+)*)?$/; @@ -167,7 +168,7 @@ function isValidHttpsUrl(value: unknown): value is string { } } -function validateListing(manifest: Record, targetType: TargetType): SubmissionFinding[] { +export function validateSubmissionListing(manifest: Record, targetType: SubmissionTargetType): SubmissionFinding[] { const findings: SubmissionFinding[] = []; validatePackage(manifest, findings); @@ -324,7 +325,7 @@ export async function buildSubmissionPreflight(targetPath: string): Promise = {}) { + return { + name: "archive-preflight", + version: "1.0.0", + skills: "./skills", + interface: { + displayName: "Archive preflight", + shortDescription: "Check a ZIP package", + longDescription: "Check an archive before submission.", + developerName: "Doctor", + category: "Developer Tools", + logo: "./assets/logo.svg", + composerIcon: "./assets/composer.svg" + }, + ...overrides + }; +} + +function packageEntries( + manifestPath = ".codex-plugin/plugin.json", + manifestValue: Record = manifest(), + prefix = "" +) { + const rooted = (entryPath: string) => prefix === "" ? entryPath : `${prefix}/${entryPath}`; + return [ + { name: rooted(manifestPath), content: JSON.stringify(manifestValue) }, + { name: rooted("assets/logo.svg"), content: svg }, + { name: rooted("assets/composer.svg"), content: svg }, + { name: rooted("skills/check/SKILL.md"), content: skill } + ]; +} + +async function reportFor(entries: ReturnType, fileName = "plugin.zip") { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-preflight-")); + temporaryDirectories.push(directory); + const archivePath = path.join(directory, fileName); + await writeFile(archivePath, createZipFixture(entries)); + return buildSubmissionArchivePreflight(archivePath); +} + +function findingIds(report: Awaited>): string[] { + return report.findings.map((finding) => finding.id); +} + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { + recursive: true, + force: true, + maxRetries: 3, + retryDelay: 50 + }))); +}); + +describe("submission archive preflight", () => { + it("aggregates a valid archive-root package through listing, asset, and skill readers", async () => { + const report = await reportFor(packageEntries()); + + expect(report).toMatchObject({ + schemaVersion: "1.0.0", + rulesetVersion: "openai-directory-archive-2026-08-23", + status: "pass", + readiness: "manual_review_required", + archive: { fileName: "plugin.zip", rootLayout: "archive-root" }, + summary: { passed: 4, warnings: 0, blockers: 0, manualChecks: 3 } + }); + expect(report.submission?.checks).toEqual([ + { id: "listing", status: "pass", findingIds: [] }, + { id: "components", status: "pass", findingIds: [] }, + { id: "assets", status: "pass", findingIds: [] }, + { id: "skills", status: "pass", findingIds: [] } + ]); + expect(report.archiveChecks).toEqual([]); + expect(submissionArchiveExitCode(report, false)).toBe(0); + expect(submissionArchiveExitCode(report, true)).toBe(0); + }); + + it.each([ + ["codex", ".codex-plugin/plugin.json"], + ["agent", ".agent-plugin/plugin.json"], + ["claude", ".claude-plugin/plugin.json"] + ])("recognizes a single top-level directory with the %s manifest path", async (_kind, manifestPath) => { + const report = await reportFor(packageEntries(manifestPath, manifest(), "plugin")); + + expect(report).toMatchObject({ status: "pass", archive: { rootLayout: "single-top-level-directory" } }); + expect(report.submission?.status).toBe("pass"); + }); + + it("reports root siblings, ambiguous manifests, and a missing immediate skill as archive blockers", async () => { + const siblings = await reportFor([ + ...packageEntries(".codex-plugin/plugin.json", manifest(), "plugin"), + { name: "README.md", content: "sibling" } + ]); + const ambiguous = await reportFor([ + ...packageEntries(), + { name: ".agent-plugin/plugin.json", content: JSON.stringify(manifest()) } + ]); + const rootAndNested = await reportFor([ + ...packageEntries(), + { name: "nested/.agent-plugin/plugin.json", content: JSON.stringify(manifest()) } + ]); + const missingSkill = await reportFor(packageEntries().filter((entry) => !String(entry.name).endsWith("SKILL.md"))); + + expect(findingIds(siblings)).toContain("plugin.submission.archive.root_siblings"); + expect(findingIds(ambiguous)).toContain("plugin.submission.archive.root_ambiguous"); + expect(findingIds(rootAndNested)).toContain("plugin.submission.archive.root_ambiguous"); + expect(rootAndNested).toMatchObject({ status: "fail", readiness: "blocked", submission: null }); + expect(findingIds(missingSkill)).toContain("plugin.submission.archive.skill_missing"); + expect([siblings, ambiguous, missingSkill].every((report) => report.status === "fail" && report.readiness === "blocked")).toBe(true); + expect(submissionArchiveExitCode(missingSkill, false)).toBe(0); + expect(submissionArchiveExitCode(missingSkill, true)).toBe(1); + }); + + it("retains exact portal codes only for warning-only skills-only exclusions", async () => { + const report = await reportFor(packageEntries(".codex-plugin/plugin.json", manifest({ + mcpServers: "./.mcp.json", + apps: "./.app.json", + interface: { ...manifest().interface as object, screenshots: ["./shot.png"] } + })).concat([ + { name: ".mcp.json", content: "{}" }, + { name: ".app.json", content: "{}" } + ])); + + expect(report.status).toBe("pass"); + expect(report.readiness).toBe("manual_review_required"); + expect(report.findings.filter((finding) => finding.id.startsWith("plugin.submission.archive.") && finding.severity === "warn")) + .toEqual(expect.arrayContaining([ + expect.objectContaining({ id: "plugin.submission.archive.mcp_excluded", portalCode: "mcp_configuration_excluded" }), + expect.objectContaining({ id: "plugin.submission.archive.app_excluded", portalCode: "app_configuration_excluded" }), + expect.objectContaining({ id: "plugin.submission.archive.screenshot_excluded", portalCode: "screenshot_configuration_excluded" }) + ])); + }); + + it("keeps portal history and normalization requirements unavailable without exposing archive paths or contents", async () => { + const sentinel = "archive-secret-sentinel"; + const report = await reportFor(packageEntries(".codex-plugin/plugin.json", manifest({ description: sentinel }))); + + expect(report.coverage).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: "plugin.submission.archive.plugin_name_mismatch", status: "unavailable" }), + expect.objectContaining({ id: "plugin.submission.archive.plugin_version_unchanged", status: "unavailable" }), + expect.objectContaining({ id: "plugin.submission.archive.manifest_normalized", status: "unavailable" }), + expect.objectContaining({ id: "plugin.submission.archive.developer_name_defaulted", status: "unavailable" }) + ])); + expect(JSON.stringify(report)).not.toContain(sentinel); + expect(JSON.stringify(report)).not.toContain("submission-archive-preflight-"); + }); + + it("preserves reader-backed listing, asset, and skill findings inside the aggregate report", async () => { + const invalidListing = await reportFor(packageEntries(".codex-plugin/plugin.json", manifest({ + interface: { ...manifest().interface as object, displayName: " " } + }))); + const missingAsset = await reportFor(packageEntries().filter((entry) => entry.name !== "assets/composer.svg")); + const invalidSkill = await reportFor(packageEntries().map((entry) => entry.name === "skills/check/SKILL.md" + ? { ...entry, content: "not a skill manifest" } + : entry)); + + expect(invalidListing.submission?.findings.map((finding) => finding.id)).toContain("plugin.submission.interface.display_name"); + expect(missingAsset.submission?.findings.map((finding) => finding.id)).toContain("plugin.submission.asset.missing"); + expect(invalidSkill.submission?.findings.map((finding) => finding.id)).toContain("plugin.submission.skill.invalid_file"); + expect([invalidListing, missingAsset, invalidSkill].every((report) => report.status === "fail" && report.readiness === "blocked")).toBe(true); + }); + + it("reports a manifest missing from the only top-level directory", async () => { + const report = await reportFor([{ name: "plugin/README.md", content: "not a manifest" }]); + + expect(findingIds(report)).toContain("plugin.submission.archive.manifest_missing"); + expect(report).toMatchObject({ status: "fail", archive: { rootLayout: "unavailable" }, submission: null }); + }); + + it("reports a structurally valid zero-entry ZIP as a missing archive root", async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-preflight-empty-")); + temporaryDirectories.push(directory); + const archivePath = path.join(directory, "empty.zip"); + await writeFile(archivePath, createZipFixture([])); + + const report = await buildSubmissionArchivePreflight(archivePath); + + expect(findingIds(report)).toEqual(["plugin.submission.archive.root_missing"]); + expect(report).toMatchObject({ + status: "fail", + readiness: "blocked", + archive: { entryCount: 0, rootLayout: "unavailable" }, + submission: null + }); + }); + + it("distinguishes an archive-root content file without a manifest from a sibling root", async () => { + const contentOnly = await reportFor([{ name: "README.md", content: "content only" }]); + const sibling = await reportFor([ + ...packageEntries(".codex-plugin/plugin.json", manifest(), "plugin"), + { name: "README.md", content: "sibling" } + ]); + + expect(findingIds(contentOnly)).toEqual(["plugin.submission.archive.manifest_missing"]); + expect(findingIds(sibling)).toContain("plugin.submission.archive.root_siblings"); + }); + + it("reports multiple archive-root content directories without a manifest as manifest_missing", async () => { + const report = await reportFor([ + { name: "assets/logo.svg", content: svg }, + { name: "skills/check/SKILL.md", content: skill } + ]); + + expect(findingIds(report)).toEqual(["plugin.submission.archive.manifest_missing"]); + }); + + it("does not treat a deep manifest-like path as an immediate nested plugin root", async () => { + const report = await reportFor([ + { name: "README.md", content: "archive-root content" }, + { name: "plugin/nested/.codex-plugin/plugin.json", content: JSON.stringify(manifest()) }, + { name: "plugin/nested/skills/check/SKILL.md", content: skill } + ]); + + expect(findingIds(report)).toEqual(["plugin.submission.archive.manifest_missing"]); + }); + + it("publishes immutable archive governance and derives portal coverage from it", async () => { + const report = await reportFor(packageEntries()); + + expect(Object.isFrozen(submissionArchiveRuleset)).toBe(true); + expect(submissionArchiveRuleset).toMatchObject({ + structures: { + eocdComments: "automatic", + zip64: "automatic", + dataDescriptors: "automatic" + }, + compression: { stored: "automatic", deflate: "automatic" }, + collisionAlgorithm: "NFKC + toLowerCase per segment" + }); + expect(submissionArchiveRuleset.automaticChecks).toContain("plugin.submission.archive.path_invalid"); + expect(submissionArchiveRuleset.automaticChecks).toEqual(expect.arrayContaining([ + "plugin.submission.archive.mcp_excluded", + "plugin.submission.archive.app_excluded", + "plugin.submission.archive.screenshot_excluded" + ])); + expect(submissionArchiveRuleset.portalRules).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: "plugin.submission.archive.plugin_name_mismatch", status: "unavailable", source: "submission-errors" }), + expect.objectContaining({ id: "plugin.submission.archive.manifest_normalized", status: "unavailable", source: "submission-errors" }) + ])); + expect(report.coverage).toEqual(expect.arrayContaining( + submissionArchiveRuleset.portalRules.map(({ id, status, reason }) => ({ id, status, reason })) + )); + }); + + it("redacts an unsafe archive basename before it reaches the report", async () => { + const sentinel = "unsafe-archive-name"; + const report = await buildSubmissionArchivePreflight(path.join(os.tmpdir(), `${sentinel}\n\u202E.zip`)); + + expect(report.archive.fileName).toBe("archive.zip"); + expect(JSON.stringify(report)).not.toContain(sentinel); + expect(JSON.stringify(report)).not.toContain("\u202E"); + }); +}); diff --git a/tests/submission-archive-reader.test.ts b/tests/submission-archive-reader.test.ts index a3de8ce..d91c7e7 100644 --- a/tests/submission-archive-reader.test.ts +++ b/tests/submission-archive-reader.test.ts @@ -54,6 +54,17 @@ describe("submission archive reader", () => { await expect(inspection.reader?.read("skills/check/SKILL.md", 100)).resolves.toEqual(new TextEncoder().encode("# check")); }); + it("accepts a structurally valid zero-entry ZIP for root-level policy diagnostics", async () => { + const inspection = await inspectFixture(createZipFixture([]), "empty.zip"); + + expect(inspection).toMatchObject({ + entryCount: 0, + entries: [], + findings: [] + }); + expect(inspection.reader).not.toBeNull(); + }); + it("rejects non-ZIP, empty, truncated, multi-disk, and encrypted inputs without throwing", async () => { for (const [name, content] of [ ["not-a-zip.txt", new Uint8Array([1])], From 7557db991ac995cd665eab1576178e46e59e23a5 Mon Sep 17 00:00:00 2001 From: Furkan Date: Mon, 24 Aug 2026 11:07:28 +0300 Subject: [PATCH 16/20] feat: expose submission archive command --- src/core/output-contract.ts | 62 ++++++++ src/core/shell-completion.ts | 23 +++ src/index.ts | 11 ++ .../render-submission-archive-report.ts | 61 ++++++++ src/run-cli.ts | 89 ++++++++++++ tests/submission-archive-command.test.ts | 133 ++++++++++++++++++ tests/submission-archive-completion.test.ts | 22 +++ tests/submission-archive-dispatch.test.ts | 51 +++++++ 8 files changed, 452 insertions(+) create mode 100644 src/reporting/render-submission-archive-report.ts create mode 100644 tests/submission-archive-command.test.ts create mode 100644 tests/submission-archive-completion.test.ts create mode 100644 tests/submission-archive-dispatch.test.ts diff --git a/src/core/output-contract.ts b/src/core/output-contract.ts index 6613580..cbd91e0 100644 --- a/src/core/output-contract.ts +++ b/src/core/output-contract.ts @@ -322,6 +322,68 @@ const publicSchemaDefinitions: Array<{ } } }, + { + id: "doctor.submission.archive.json", + command: "codex-plugin-doctor doctor submission archive --json", + required: [ + "schemaVersion", + "rulesetVersion", + "status", + "readiness", + "archive", + "summary", + "archiveChecks", + "submission", + "findings", + "coverage", + "manualChecklist" + ], + properties: { + rulesetVersion: { const: "openai-directory-archive-2026-08-23" }, + status: { type: "string", enum: ["pass", "fail"] }, + readiness: { type: "string", enum: ["blocked", "manual_review_required"] }, + archive: { + type: "object", + required: ["fileName", "compressedBytes", "uncompressedBytes", "entryCount", "rootLayout"], + properties: { + fileName: { type: "string" }, + compressedBytes: { type: "integer", minimum: 0 }, + uncompressedBytes: { type: "integer", minimum: 0 }, + entryCount: { type: "integer", minimum: 0 }, + rootLayout: { type: "string", enum: ["archive-root", "single-top-level-directory", "unavailable"] } + }, + additionalProperties: false + }, + 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 + }, + archiveChecks: { type: "array" }, + submission: { type: ["object", "null"] }, + findings: { type: "array" }, + coverage: { + type: "array", + items: { + type: "object", + required: ["id", "status", "reason"], + properties: { + id: { type: "string", pattern: "^plugin\\.submission\\.archive\\." }, + status: { type: "string", enum: ["automatic", "manual", "unavailable"] }, + reason: { type: "string" } + }, + additionalProperties: false + } + }, + manualChecklist: { type: "array" } + } + }, { 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 e38cf55..a3b7805 100644 --- a/src/core/shell-completion.ts +++ b/src/core/shell-completion.ts @@ -21,6 +21,7 @@ const topLevelCommands = [ ]; const doctorCommands = ["submission"]; +const submissionTargets = ["archive"]; const submissionFlags = ["--json", "--markdown", "--output", "--require-ready"]; const fishSubmissionCondition = "__fish_seen_subcommand_from doctor; and __fish_seen_subcommand_from submission"; @@ -34,6 +35,7 @@ function bashCompletion(): string { "", ` local commands="${topLevelCommands.join(" ")}"`, ` local doctor_commands="${doctorCommands.join(" ")}"`, + ` local submission_targets="${submissionTargets.join(" ")}"`, ` local submission_flags="${submissionFlags.join(" ")}"`, "", " case \"${prev}\" in", @@ -45,8 +47,25 @@ function bashCompletion(): string { " COMPREPLY=( $(compgen -W \"${doctor_commands}\" -- \"${cur}\") )", " return 0", " ;;", + " submission)", + " COMPREPLY=( $(compgen -W \"${submission_targets}\" -- \"${cur}\") )", + " return 0", + " ;;", " esac", "", + " if [[ ${COMP_WORDS[1]} == \"doctor\" && ${COMP_WORDS[2]} == \"submission\" && ${COMP_WORDS[3]} == \"archive\" ]]; then", + " case \"${cur}\" in", + " --*)", + " COMPREPLY=( $(compgen -W \"${submission_flags}\" -- \"${cur}\") )", + " return 0", + " ;;", + " *)", + " COMPREPLY=( $(compgen -f -- \"${cur}\") )", + " return 0", + " ;;", + " esac", + " fi", + "", " case \"${cur}\" in", " --*)", " if [[ ${COMP_WORDS[1]} == \"doctor\" && ${COMP_WORDS[2]} == \"submission\" ]]; then", @@ -82,6 +101,8 @@ function zshCompletion(): string { "", " if [[ \"$words[2]\" == \"doctor\" && \"$words[3]\" == \"submission\" ]]; then", " _arguments -C \\", + " '3:archive target:(archive)' \\", + " '4:ZIP archive:_files' \\", " '*--json[Output as JSON]' \\", " '*--markdown[Output as Markdown]' \\", " '*--output[Write to file]:file:_files' \\", @@ -114,6 +135,8 @@ function fishCompletion(): string { `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 "${fishSubmissionCondition}; and __fish_seen_subcommand_from archive" -F`, + `complete -c codex-plugin-doctor -n "${fishSubmissionCondition}; and not __fish_seen_subcommand_from archive" -a "${submissionTargets.join(" ")}" -d 'ZIP archive'`, `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 74a406d..a7009bc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -156,6 +156,12 @@ export { type SubmissionManualCheck, type SubmissionPreflightReport } from "./core/submission-preflight.js"; +export { + buildSubmissionArchivePreflight, + submissionArchiveExitCode, + type SubmissionArchivePreflightReport, + type SubmissionArchiveRootLayout +} from "./core/submission-archive-preflight.js"; export { validateSubmissionAssets, type SubmissionAssetResult @@ -170,6 +176,11 @@ export { renderSubmissionPreflightText, submissionPreflightExitCode } from "./reporting/render-submission-report.js"; +export { + renderSubmissionArchiveJson, + renderSubmissionArchiveMarkdown, + renderSubmissionArchiveText +} from "./reporting/render-submission-archive-report.js"; export { buildDoctorValidationCorpusReport, renderDoctorValidationCorpusJson, diff --git a/src/reporting/render-submission-archive-report.ts b/src/reporting/render-submission-archive-report.ts new file mode 100644 index 0000000..b73de12 --- /dev/null +++ b/src/reporting/render-submission-archive-report.ts @@ -0,0 +1,61 @@ +import type { + SubmissionArchivePreflightReport +} from "../core/submission-archive-preflight.js"; + +function upper(value: string): string { + return value.replace(/[-_]/gu, " ").toUpperCase(); +} + +function escapeMarkdown(value: string): string { + return value.replace(/[\\`*_{}\[\]()<>#+\-.!|]/gu, "\\$&"); +} + +function summary(report: SubmissionArchivePreflightReport): string[] { + return [ + `Ruleset: ${report.rulesetVersion}`, + `Archive: ${report.archive.fileName}`, + `Root layout: ${report.archive.rootLayout}`, + `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 archive review." + ]; +} + +export function renderSubmissionArchiveJson(report: SubmissionArchivePreflightReport): string { + return `${JSON.stringify(report, null, 2)}\n`; +} + +export function renderSubmissionArchiveText(report: SubmissionArchivePreflightReport): string { + const lines = ["Submission archive preflight", "============================", ...summary(report), "", "Findings"]; + lines.push(...(report.findings.length === 0 + ? [" None"] + : report.findings.map((finding) => ` ${upper(finding.severity)} ${finding.id}: ${finding.message}`))); + lines.push("", "Coverage"); + lines.push(...(report.coverage.length === 0 + ? [" None"] + : report.coverage.map((item) => ` ${upper(item.status)} ${item.id}: ${item.reason}`))); + lines.push("", "Manual checklist"); + lines.push(...report.manualChecklist.map((item) => ` ${upper(item.state)} ${item.id}: ${item.label}`)); + return `${lines.join("\n")}\n`; +} + +export function renderSubmissionArchiveMarkdown(report: SubmissionArchivePreflightReport): string { + const lines = [ + "# Submission archive preflight", + "", + ...summary(report).map((item) => `- ${escapeMarkdown(item)}`), + "", + "## Findings" + ]; + lines.push(...(report.findings.length === 0 + ? ["- None"] + : report.findings.map((finding) => `- **${escapeMarkdown(upper(finding.severity))}** ${escapeMarkdown(finding.id)}: ${escapeMarkdown(finding.message)}`))); + lines.push("", "## Coverage"); + lines.push(...(report.coverage.length === 0 + ? ["- None"] + : report.coverage.map((item) => `- ${escapeMarkdown(upper(item.status))}: ${escapeMarkdown(item.id)} — ${escapeMarkdown(item.reason)}`))); + 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`; +} diff --git a/src/run-cli.ts b/src/run-cli.ts index 42f97da..07450c8 100644 --- a/src/run-cli.ts +++ b/src/run-cli.ts @@ -83,6 +83,7 @@ import { renderDoctorOutputContract, renderDoctorOutputContractJson } from "./core/output-contract.js"; +import { buildSubmissionArchivePreflight, submissionArchiveExitCode } from "./core/submission-archive-preflight.js"; import { buildSubmissionPreflight } from "./core/submission-preflight.js"; import { buildDoctorValidationCorpusReport, @@ -231,6 +232,11 @@ 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 { + renderSubmissionArchiveJson, + renderSubmissionArchiveMarkdown, + renderSubmissionArchiveText +} from "./reporting/render-submission-archive-report.js"; import { renderSubmissionPreflightJson, renderSubmissionPreflightMarkdown, @@ -451,6 +457,7 @@ function printUsage(io: CliIo): void { ); io.writeStderr( " codex-plugin-doctor doctor submission [--json|--markdown] [--output ] [--require-ready]" + + "\n codex-plugin-doctor doctor submission archive [--json|--markdown] [--output ] [--require-ready]" ); } @@ -1664,6 +1671,70 @@ function parseSubmissionCommandArgs(args: string[]): { return { targetPath, jsonOutput, markdownOutput, outputPath, requireReady }; } +function parseSubmissionArchiveCommandArgs(args: string[]): ReturnType { + let targetPath: string | null = null; + let jsonOutput = false; + 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 === "--" && !optionsEnded) optionsEnded = true; + else if (optionsEnded) { + if (targetPath === null) targetPath = argument; + else return new CliUsageError(`Unexpected submission archive argument: ${argument}.`); + } else if (argument === "--json") { + if (jsonOutput) return new CliUsageError("Duplicate submission archive flag: --json."); + jsonOutput = true; + } else if (argument === "--markdown") { + if (markdownOutput) return new CliUsageError("Duplicate submission archive flag: --markdown."); + markdownOutput = true; + } else if (argument === "--require-ready") { + if (requireReady) return new CliUsageError("Duplicate submission archive flag: --require-ready."); + requireReady = true; + } else if (argument === "--output" || argument.startsWith("--output=")) { + if (outputPath !== null) return new CliUsageError("Duplicate submission archive flag: --output."); + const value = argument === "--output" ? args[index + 1] : argument.slice("--output=".length); + if (!value || (argument === "--output" && value.startsWith("--"))) return new CliUsageError("Missing path after --output."); + outputPath = value; + if (argument === "--output") index += 1; + } else if (argument.startsWith("--")) return new CliUsageError(`Unknown submission archive flag: ${argument}.`); + else if (targetPath === null) targetPath = argument; + else return new CliUsageError(`Unexpected submission archive argument: ${argument}.`); + } + if (targetPath === null) return new CliUsageError("Missing archive path for submission."); + if (jsonOutput && markdownOutput) return new CliUsageError("Use either --json or --markdown, not both."); + return { targetPath, jsonOutput, markdownOutput, outputPath, requireReady }; +} + +function hasArchiveSubcommandTarget(args: string[]): boolean { + let optionsEnded = false; + for (let index = 0; index < args.length; index += 1) { + const argument = args[index]; + if (optionsEnded) return true; + if (argument === "--") { + optionsEnded = true; + } else if (argument === "--output") { + index += 1; + } else if (!argument.startsWith("--")) { + return true; + } + } + return false; +} + +async function submissionTargetExists(targetPath: string): Promise { + try { + const { stat } = await import("node:fs/promises"); + await stat(targetPath); + return true; + } catch (error: unknown) { + return !(typeof error === "object" && error !== null && "code" in error + && (error as { code?: unknown }).code === "ENOENT"); + } +} + export async function runCli( args: string[], io: CliIo = defaultIo, @@ -1807,6 +1878,24 @@ export async function runCli( } if (maybePath === "submission") { + if (remainingArgs[0] === "archive") { + const useLegacyDirectoryTarget = !hasArchiveSubcommandTarget(remainingArgs.slice(1)) + && await submissionTargetExists("archive"); + if (!useLegacyDirectoryTarget) { + const parsedArchiveArgs = parseSubmissionArchiveCommandArgs(remainingArgs.slice(1)); + if (parsedArchiveArgs instanceof CliUsageError) { + io.writeStderr(parsedArchiveArgs.message); + return 2; + } + const report = await buildSubmissionArchivePreflight(parsedArchiveArgs.targetPath); + const renderedReport = parsedArchiveArgs.jsonOutput + ? renderSubmissionArchiveJson(report) + : parsedArchiveArgs.markdownOutput ? renderSubmissionArchiveMarkdown(report) : renderSubmissionArchiveText(report); + if (parsedArchiveArgs.outputPath) await writeFile(parsedArchiveArgs.outputPath, renderedReport, "utf8"); + writeExactStdout(io, renderedReport); + return submissionArchiveExitCode(report, parsedArchiveArgs.requireReady); + } + } const parsedSubmissionArgs = parseSubmissionCommandArgs(remainingArgs); if (parsedSubmissionArgs instanceof CliUsageError) { diff --git a/tests/submission-archive-command.test.ts b/tests/submission-archive-command.test.ts new file mode 100644 index 0000000..71d57ed --- /dev/null +++ b/tests/submission-archive-command.test.ts @@ -0,0 +1,133 @@ +import { mkdtemp, readFile, rm, 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 { generateCompletion } from "../src/core/shell-completion.js"; +import { runCli } from "../src/run-cli.js"; +import { createZipFixture } from "./helpers/zip-fixture.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 validSkill = `--- +name: check +description: Check archive +--- + +Check archive. +`; + +describe("doctor submission archive command", () => { + it("renders redacted JSON and writes the exact same bytes", async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-command-")); + const archivePath = path.join(directory, "plugin.zip"); + const outputPath = path.join(directory, "archive.json"); + const sentinel = "archive-content-sentinel"; + const manifest = { + name: "archive-command", version: "1.0.0", skills: "./skills", + interface: { + displayName: "Archive command", shortDescription: "Check archive", longDescription: sentinel, + developerName: "Doctor", category: "Developer Tools", logo: "./assets/logo.svg", composerIcon: "./assets/composer.svg" + } + }; + const svg = ''; + const io = createIo(); + + try { + await writeFile(archivePath, createZipFixture([ + { name: ".codex-plugin/plugin.json", content: JSON.stringify(manifest) }, + { name: "assets/logo.svg", content: svg }, + { name: "assets/composer.svg", content: svg }, + { name: "skills/check/SKILL.md", content: validSkill } + ])); + + expect(await runCli(["doctor", "submission", "archive", archivePath, "--json", "--output", outputPath], io.io)).toBe(0); + expect(io.stderr).toEqual([]); + expect(JSON.parse(io.stdout.join(""))).toMatchObject({ schemaVersion: "1.0.0", archive: { fileName: "plugin.zip" } }); + expect(await readFile(outputPath, "utf8")).toBe(io.stdout.join("")); + expect(io.stdout.join("")).not.toContain(archivePath); + expect(io.stdout.join("")).not.toContain(sentinel); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + it("exports the archive API and publishes its contract and contextual completions", async () => { + const contractIo = createIo(); + + expect(doctor.buildSubmissionArchivePreflight).toBeTypeOf("function"); + expect(doctor.renderSubmissionArchiveJson).toBeTypeOf("function"); + expect(doctor.renderSubmissionArchiveText).toBeTypeOf("function"); + expect(doctor.renderSubmissionArchiveMarkdown).toBeTypeOf("function"); + expect(doctor.submissionArchiveExitCode).toBeTypeOf("function"); + expect(await runCli(["doctor", "contract", "--json"], contractIo.io)).toBe(0); + expect(JSON.parse(contractIo.stdout.join("")).schemas).toContainEqual(expect.objectContaining({ + id: "doctor.submission.archive.json", + command: "codex-plugin-doctor doctor submission archive --json" + })); + expect(generateCompletion("bash")).toContain('local submission_targets="archive"'); + expect(generateCompletion("zsh")).toContain("'3:archive target:(archive)'"); + expect(generateCompletion("fish")).toContain('-a "archive" -d \'ZIP archive\''); + expect(generateCompletion("zsh")).toContain('[[ "$words[2]" == "doctor" && "$words[3]" == "submission" ]]'); + expect(generateCompletion("fish")).toContain('__fish_seen_subcommand_from doctor; and __fish_seen_subcommand_from submission'); + }); + + it("renders redacted text and Markdown, and keeps archive blockers advisory unless strict", async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-human-render-")); + const archivePath = path.join(directory, "safe.zip"); + const sentinel = "archive-human-secret"; + const textOutputPath = path.join(directory, "archive.txt"); + const markdownOutputPath = path.join(directory, "archive.md"); + const text = createIo(); const markdown = createIo(); const advisory = createIo(); const strict = createIo(); + try { + await writeFile(archivePath, createZipFixture([{ name: ".codex-plugin/plugin.json", content: JSON.stringify({ + name: "human-render", version: "1.0.0", skills: "./skills", interface: { displayName: "Human", shortDescription: "Human", longDescription: sentinel, developerName: "Doctor", category: "Developer Tools", logo: "./assets/logo.svg", composerIcon: "./assets/composer.svg" } + }) }, { name: "assets/logo.svg", content: '' }, { name: "assets/composer.svg", content: '' }, { name: "skills/check/SKILL.md", content: validSkill }])); + expect(await runCli(["doctor", "submission", "archive", archivePath, "--output", textOutputPath], text.io)).toBe(0); + expect(await runCli(["doctor", "submission", "archive", archivePath, "--markdown", "--output", markdownOutputPath], markdown.io)).toBe(0); + for (const output of [text.stdout.join(""), markdown.stdout.join("")]) { + expect(output).toContain("Submission archive preflight"); + expect(output).not.toContain(archivePath); + expect(output).not.toContain(sentinel); + } + expect(await readFile(textOutputPath, "utf8")).toBe(text.stdout.join("")); + expect(await readFile(markdownOutputPath, "utf8")).toBe(markdown.stdout.join("")); + const missingPath = path.join(directory, "missing.zip"); + expect(await runCli(["doctor", "submission", "archive", missingPath], advisory.io)).toBe(0); + expect(await runCli(["doctor", "submission", "archive", missingPath, "--require-ready"], strict.io)).toBe(1); + expect(advisory.stderr).toEqual([]); expect(strict.stderr).toEqual([]); + } finally { await rm(directory, { recursive: true, force: true }); } + }); + + it("keeps warning-only archives strict-ready and validates archive parser edge cases", async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-warning-")); + const archivePath = path.join(directory, "warning.zip"); + try { + await writeFile(archivePath, createZipFixture([{ name: ".codex-plugin/plugin.json", content: JSON.stringify({ + name: "warning-render", version: "1.0.0", skills: "./skills", mcpServers: "./.mcp.json", interface: { displayName: "Warning", shortDescription: "Warning", longDescription: "Warning", developerName: "Doctor", category: "Developer Tools", logo: "./assets/logo.svg", composerIcon: "./assets/composer.svg" } + }) }, { name: ".mcp.json", content: "{}" }, { name: "assets/logo.svg", content: '' }, { name: "assets/composer.svg", content: '' }, { name: "skills/check/SKILL.md", content: validSkill }])); + const warning = createIo(); + expect(await runCli(["doctor", "submission", "archive", archivePath, "--require-ready"], warning.io)).toBe(0); + expect(warning.stderr).toEqual([]); + for (const [args, expected] of [ + [["doctor", "submission", "archive"], "Missing archive path"], + [["doctor", "submission", "archive", "x.zip", "--wat"], "Unknown submission archive flag"], + [["doctor", "submission", "archive", "x.zip", "--json", "--json"], "Duplicate submission archive flag"], + [["doctor", "submission", "archive", "x.zip", "--json", "--markdown"], "Use either --json or --markdown"], + [["doctor", "submission", "archive", "--", "--literal.zip", "--require-ready"], "Unexpected submission archive argument"] + ] as const) { + const io = createIo(); + expect(await runCli([...args], io.io)).toBe(2); + expect(io.stderr.join("")).toContain(expected); + } + const delimiter = createIo(); + expect(await runCli(["doctor", "submission", "archive", "--", "--literal.zip"], delimiter.io)).toBe(0); + expect(delimiter.stderr).toEqual([]); + } finally { await rm(directory, { recursive: true, force: true }); } + }); +}); diff --git a/tests/submission-archive-completion.test.ts b/tests/submission-archive-completion.test.ts new file mode 100644 index 0000000..d06eb8c --- /dev/null +++ b/tests/submission-archive-completion.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; + +import { generateCompletion } from "../src/core/shell-completion.js"; + +describe("submission archive shell completion", () => { + it("uses standalone Bash file completion after archive while retaining scoped flags", () => { + const output = generateCompletion("bash"); + + expect(output).toContain('COMPREPLY=( $(compgen -f -- "${cur}") )'); + expect(output).not.toContain("_filedir"); + expect(output).toContain('local submission_flags="--json --markdown --output --require-ready"'); + }); + + it("uses real CLI argument positions for Zsh archive and ZIP completion", () => { + const output = generateCompletion("zsh"); + + expect(output).toContain("'3:archive target:(archive)'"); + expect(output).toContain("'4:ZIP archive:_files'"); + expect(output).not.toContain("'1:archive target:(archive)'"); + expect(output).not.toContain("'2:ZIP archive:_files'"); + }); +}); diff --git a/tests/submission-archive-dispatch.test.ts b/tests/submission-archive-dispatch.test.ts new file mode 100644 index 0000000..6383a0f --- /dev/null +++ b/tests/submission-archive-dispatch.test.ts @@ -0,0 +1,51 @@ +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import { generateCompletion } from "../src/core/shell-completion.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); } + } + }; +} + +describe("submission archive dispatch", () => { + it("preserves archive as a legacy directory target when it exists", async () => { + const directory = await mkdtemp(path.join(os.tmpdir(), "submission-archive-directory-target-")); + const previousDirectory = process.cwd(); + const { io, stdout, stderr } = createIo(); + try { + await mkdir(path.join(directory, "archive")); + process.chdir(directory); + + expect(await runCli(["doctor", "submission", "archive"], io)).toBe(0); + expect(stderr).toEqual([]); + expect(stdout.join("")).toContain("Submission preflight"); + expect(stdout.join("")).not.toContain("Submission archive preflight"); + } finally { + process.chdir(previousDirectory); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("uses archive-aware file completion after the archive target", () => { + const bash = generateCompletion("bash"); + const zsh = generateCompletion("zsh"); + const fish = generateCompletion("fish"); + + expect(bash).toContain('${COMP_WORDS[3]} == "archive"'); + expect(bash).toContain('COMPREPLY=( $(compgen -f -- "${cur}") )'); + expect(zsh).toContain("'4:ZIP archive:_files'"); + expect(fish).toContain('__fish_seen_subcommand_from archive" -F'); + }); +}); From fa1eb579d8413b65be27295a0ea9a0f735d136c9 Mon Sep 17 00:00:00 2001 From: Furkan Date: Tue, 25 Aug 2026 09:45:48 +0300 Subject: [PATCH 17/20] feat: add archive preflight to GitHub Action --- README.md | 6 + action.yml | 55 +++++- docs/guides/github-action.md | 21 +++ docs/rules/catalog.md | 31 ++++ tests/action-archive-behavior.test.ts | 249 ++++++++++++++++++++++++++ tests/action-metadata.test.ts | 32 +++- tests/public-readiness.test.ts | 13 ++ 7 files changed, 402 insertions(+), 5 deletions(-) create mode 100644 tests/action-archive-behavior.test.ts diff --git a/README.md b/README.md index dc50c68..ddd2dfa 100644 --- a/README.md +++ b/README.md @@ -143,10 +143,16 @@ codex-plugin-doctor doctor submission codex-plugin-doctor doctor submission --json codex-plugin-doctor doctor submission --markdown codex-plugin-doctor doctor submission --require-ready +codex-plugin-doctor doctor submission archive +codex-plugin-doctor doctor submission archive --json --output submission-archive.json +codex-plugin-doctor doctor submission archive --markdown --output submission-archive.md +codex-plugin-doctor doctor submission archive --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). +The archive variant accepts an existing skills-only ZIP and does not extract archive entries to disk, execute package code, or repair the archive. It reports malformed ZIP structures and deterministic package blockers; unsupported portal rules remain manual or `coverage: unavailable` rather than an automatic pass. + Output formats: - human text output diff --git a/action.yml b/action.yml index 8c58361..6fdd89e 100644 --- a/action.yml +++ b/action.yml @@ -42,6 +42,10 @@ inputs: description: Generate offline public directory submission preflight JSON and Markdown reports. required: false default: "false" + submission-archive: + description: Optional existing ZIP path for offline skills-only archive submission preflight reports. + required: false + default: "" require-submission-ready: description: Fail unless the offline submission preflight is automatically ready; this does not replace manual review. required: false @@ -183,6 +187,12 @@ outputs: submission-summary-path: description: Path to the offline submission preflight Markdown report when submission is enabled. value: ${{ steps.run-doctor.outputs.submission-summary-path }} + submission-archive-json-path: + description: Path to the offline archive submission preflight JSON report when submission-archive is configured. + value: ${{ steps.run-doctor.outputs.submission-archive-json-path }} + submission-archive-summary-path: + description: Path to the offline archive submission preflight Markdown report when submission-archive is configured. + value: ${{ steps.run-doctor.outputs.submission-archive-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 }} @@ -208,6 +218,7 @@ runs: REGISTRY_METADATA_INPUT: ${{ inputs['registry-metadata'] }} REQUIRE_REGISTRY_READINESS_INPUT: ${{ inputs['require-registry-readiness'] }} SUBMISSION_INPUT: ${{ inputs.submission }} + SUBMISSION_ARCHIVE_INPUT: ${{ inputs['submission-archive'] }} REQUIRE_SUBMISSION_READY_INPUT: ${{ inputs['require-submission-ready'] }} CORPUS_METRICS_MANIFEST_INPUT: ${{ inputs['corpus-metrics-manifest'] }} CORPUS_METRICS_BASELINE_INPUT: ${{ inputs['corpus-metrics-baseline'] }} @@ -227,14 +238,20 @@ runs: 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" + submission_archive_json_path="$report_dir/codex-plugin-doctor-submission-archive.json" + submission_archive_summary_path="$report_dir/codex-plugin-doctor-submission-archive.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" submission_state_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-submission-ran" + submission_archive_state_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-submission-archive-ran" status=0 submission_ran=false + submission_archive_ran=false submission_json_output="" submission_summary_output="" + submission_archive_json_output="" + submission_archive_summary_output="" doctor_version="$(codex-plugin-doctor --version)" mkdir -p "$report_dir" @@ -370,8 +387,11 @@ 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 + if [[ "$SUBMISSION_INPUT" == "true" && -n "$SUBMISSION_ARCHIVE_INPUT" ]]; then + echo "submission and submission-archive cannot be selected together." >&2 + record_status 2 + elif [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" && "$SUBMISSION_INPUT" != "true" && -z "$SUBMISSION_ARCHIVE_INPUT" ]]; then + echo "require-submission-ready requires exactly one submission mode." >&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 @@ -386,6 +406,16 @@ runs: 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" + elif [[ -n "$SUBMISSION_ARCHIVE_INPUT" ]]; then + submission_archive_ran=true + submission_archive_args=(doctor submission archive "$SUBMISSION_ARCHIVE_INPUT" --json --output "$submission_archive_json_path") + if [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" ]]; then + submission_archive_args+=(--require-ready) + fi + run_doctor "submission archive preflight" "${submission_archive_args[@]}" + run_doctor "submission archive summary" doctor submission archive "$SUBMISSION_ARCHIVE_INPUT" --markdown --output "$submission_archive_summary_path" + submission_archive_json_output="$submission_archive_json_path" + submission_archive_summary_output="$submission_archive_summary_path" fi if [[ "${{ inputs['review-bundle'] }}" == "true" ]]; then @@ -436,6 +466,7 @@ runs: 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_ran" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE="$submission_archive_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" @@ -448,6 +479,8 @@ runs: export CODEX_PLUGIN_DOCTOR_ACTION_REGISTRY_PATH="$registry_report_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_SUBMISSION_ARCHIVE_JSON_PATH="$submission_archive_json_output" + export CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE_SUMMARY_PATH="$submission_archive_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' @@ -487,6 +520,8 @@ runs: 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"), + submissionArchiveJson: report("submissionArchiveJson", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE_JSON_PATH"), + submissionArchiveSummary: report("submissionArchiveSummary", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE_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") } @@ -501,6 +536,7 @@ runs: printf "%s" "$status" > "$status_file" printf "%s" "$submission_ran" > "$submission_state_file" + printf "%s" "$submission_archive_ran" > "$submission_archive_state_file" { echo "status=$status" @@ -516,6 +552,8 @@ runs: echo "registry-report-path=$registry_report_path" echo "submission-json-path=$submission_json_output" echo "submission-summary-path=$submission_summary_output" + echo "submission-archive-json-path=$submission_archive_json_output" + echo "submission-archive-summary-path=$submission_archive_summary_output" echo "review-bundle-path=$review_bundle_path" echo "review-bundle-verification-path=$review_bundle_verification_path" } >> "$GITHUB_OUTPUT" @@ -529,10 +567,13 @@ 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" + submission_archive_summary_path="$report_dir/codex-plugin-doctor-submission-archive.md" status_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-status" submission_state_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-submission-ran" + submission_archive_state_file="${RUNNER_TEMP:-.}/codex-plugin-doctor-submission-archive-ran" status="unknown" submission_ran=false + submission_archive_ran=false if [[ -f "$status_file" ]]; then status="$(cat "$status_file")" @@ -542,6 +583,10 @@ runs: submission_ran="$(cat "$submission_state_file")" fi + if [[ -f "$submission_archive_state_file" ]]; then + submission_archive_ran="$(cat "$submission_archive_state_file")" + fi + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && -f "$summary_path" ]]; then cat "$summary_path" >> "$GITHUB_STEP_SUMMARY" fi @@ -550,7 +595,11 @@ runs: cat "$submission_summary_path" >> "$GITHUB_STEP_SUMMARY" fi - if [[ -n "${GITHUB_STEP_SUMMARY:-}" && ! -f "$summary_path" && ( "$submission_ran" != "true" || ! -f "$submission_summary_path" ) ]]; then + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && "$submission_archive_ran" == "true" && -f "$submission_archive_summary_path" ]]; then + cat "$submission_archive_summary_path" >> "$GITHUB_STEP_SUMMARY" + fi + + if [[ -n "${GITHUB_STEP_SUMMARY:-}" && ! -f "$summary_path" && ( "$submission_ran" != "true" || ! -f "$submission_summary_path" ) && ( "$submission_archive_ran" != "true" || ! -f "$submission_archive_summary_path" ) ]]; then { echo "## Codex Plugin Doctor" echo "" diff --git a/docs/guides/github-action.md b/docs/guides/github-action.md index 03355bb..f607a3a 100644 --- a/docs/guides/github-action.md +++ b/docs/guides/github-action.md @@ -54,6 +54,25 @@ Use the submission preflight only when a workflow needs its separate offline rep 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. +### Archive Submission Preflight + +Use archive mode for the existing skills-only ZIP that will be uploaded: + +```yaml +- uses: ./ + with: + submission-archive: ./plugin.zip + require-submission-ready: "true" +``` + +Archive mode writes `codex-plugin-doctor-submission-archive.json` and `codex-plugin-doctor-submission-archive.md` under `output-dir`, uploads them with the existing artifact directory, appends the Markdown report after the primary summary, and exposes `submission-archive-json-path` and `submission-archive-summary-path` outputs. + +It is offline, non-executing, and does not extract archive entries to disk. It forwards no runtime, network, local-network, session-lifecycle, authentication, or portal credentials. It accepts an existing ZIP only; it does not create, rewrite, repair, or upload archives. + +Choose exactly one submission mode: directory `submission: "true"` or `submission-archive`. Selecting both modes, or setting `require-submission-ready: "true"` with neither, records usage status `2` and produces no submission reports. The installed-cache guard remains for directory submission mode. + +Archive warnings identify MCP configuration, apps, or screenshots that require the MCP-backed portal flow; warnings do not make strict readiness fail. Archive blockers make `--require-ready` fail, but a non-strict report remains advisory. A passing automatic result remains `manual_review_required`, and any portal rule Doctor cannot faithfully implement remains `coverage: unavailable`; the Action never claims portal approval. + ## Recommended Workflow ```yaml @@ -145,6 +164,8 @@ The action also exposes these workflow outputs for follow-up steps: - `registry-report-path` - `submission-json-path` - `submission-summary-path` +- `submission-archive-json-path` +- `submission-archive-summary-path` - `review-bundle-path` - `review-bundle-verification-path` diff --git a/docs/rules/catalog.md b/docs/rules/catalog.md index eaff223..5fbcbc1 100644 --- a/docs/rules/catalog.md +++ b/docs/rules/catalog.md @@ -88,6 +88,37 @@ codex-plugin-doctor explain plugin.manifest.missing | `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. | +## Public Directory Archive Preflight Rules + +Archive checks validate an existing skills-only ZIP without extraction. `warn` results remain advisory; `fail` results block `--require-ready`. Portal checks whose behavior is not public remain manual or unavailable coverage rather than an automatic pass. + +| Rule ID | Severity | Meaning | +| --- | --- | --- | +| `plugin.submission.archive.invalid_file` | fail | Input is not a readable, non-empty regular ZIP file. | +| `plugin.submission.archive.invalid_zip` | fail | Archive is malformed, truncated, or changed during inspection. | +| `plugin.submission.archive.too_large` | fail | Compressed archive exceeds the supported size limit. | +| `plugin.submission.archive.entry_count` | fail | Archive entry count exceeds the supported limit. | +| `plugin.submission.archive.member_too_large` | fail | An archive member exceeds the supported size limit. | +| `plugin.submission.archive.total_too_large` | fail | Archive cumulative uncompressed size exceeds the supported limit. | +| `plugin.submission.archive.encrypted` | fail | An encrypted archive entry cannot be inspected. | +| `plugin.submission.archive.header_mismatch` | fail | Central and local ZIP headers disagree. | +| `plugin.submission.archive.descriptor_invalid` | fail | A ZIP data descriptor is invalid. | +| `plugin.submission.archive.range_invalid` | fail | ZIP metadata or entry data ranges are invalid or overlap. | +| `plugin.submission.archive.crc_mismatch` | fail | Archive entry content does not match its declared CRC or size. | +| `plugin.submission.archive.path_invalid` | fail | An archive entry path is unsafe or unsupported. | +| `plugin.submission.archive.path_duplicate` | fail | Archive contains duplicate entry paths. | +| `plugin.submission.archive.path_conflict` | fail | Archive entry paths conflict as a file and directory. | +| `plugin.submission.archive.type_unsupported` | fail | Archive entry type is unsupported. | +| `plugin.submission.archive.normalization_collision` | warn | Paths collide under Doctor's documented local normalization check. | +| `plugin.submission.archive.root_missing` | fail | Archive does not contain a plugin root. | +| `plugin.submission.archive.root_ambiguous` | fail | Archive contains more than one plugin root or manifest. | +| `plugin.submission.archive.root_siblings` | fail | A single top-level plugin directory has siblings. | +| `plugin.submission.archive.manifest_missing` | fail | Archive root has no valid recognized plugin manifest. | +| `plugin.submission.archive.skill_missing` | fail | Archive has no immediate `skills//SKILL.md` entrypoint. | +| `plugin.submission.archive.mcp_excluded` | warn | MCP configuration requires the MCP-backed submission flow. | +| `plugin.submission.archive.app_excluded` | warn | App configuration requires the MCP-backed submission flow. | +| `plugin.submission.archive.screenshot_excluded` | warn | Screenshots require the MCP-backed submission flow. | + ## MCP Rules | Rule ID | Severity | Meaning | diff --git a/tests/action-archive-behavior.test.ts b/tests/action-archive-behavior.test.ts new file mode 100644 index 0000000..6fa783f --- /dev/null +++ b/tests/action-archive-behavior.test.ts @@ -0,0 +1,249 @@ +import { execFile } from "node:child_process"; +import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { parse } from "yaml"; +import { describe, expect, it } from "vitest"; + +const execFileAsync = promisify(execFile); +const gitBash = "C:\\Program Files\\Git\\bin\\bash.exe"; + +type ActionMetadata = { + inputs: Record; + runs: { steps: Array<{ id?: string; name?: string; run?: string }> }; +}; + +type ActionRun = { + root: string; + archiveJsonPath: string; + archiveSummaryPath: string; + output: Record; + manifest: { reports: Record }; + invocations: string[][]; + readState: () => Promise<{ submission: string; archive: string; status: string }>; + runSummary: () => Promise; + cleanup: () => Promise; +}; + +function toBashPath(value: string): string { + return value.replace(/\\/gu, "/"); +} + +function renderInputs(script: string, inputs: Record): string { + return script.replace(/\$\{\{\s*inputs(?:\.([A-Za-z0-9-]+)|\[['"]([^'"]+)['"]\])\s*\}\}/gu, (_match, dotted, bracketed) => inputs[dotted ?? bracketed] ?? ""); +} + +function outputEntries(value: string): Record { + return Object.fromEntries(value.split(/\r?\n/gu).filter(Boolean).map((line) => { + const separator = line.indexOf("="); + return [line.slice(0, separator), line.slice(separator + 1)]; + })); +} + +async function loadAction(): Promise { + return parse(await readFile("action.yml", "utf8")) as ActionMetadata; +} + +async function runArchiveAction(overrides: Record = {}): Promise { + const action = await loadAction(); + const root = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-action-archive-")); + const binDirectory = path.join(root, "bin"); + const reportDirectory = path.join(root, "reports"); + const runnerDirectory = path.join(root, "runner"); + const actionOutputPath = path.join(root, "github-output"); + const actionStatePath = path.join(root, "github-state"); + const stepSummaryPath = path.join(root, "github-step-summary"); + const logPath = path.join(root, "doctor.log"); + const archivePath = path.join(root, "submission.zip"); + const defaults = Object.fromEntries(Object.entries(action.inputs).map(([key, value]) => [key, String(value.default ?? "")])); + const inputs = { + ...defaults, + path: toBashPath(path.join(root, "package")), + "output-dir": toBashPath(reportDirectory), + "step-summary": "true", + ...overrides + }; + const runDoctorScript = action.runs.steps.find((step) => step.id === "run-doctor")?.run; + const summaryScript = action.runs.steps.find((step) => step.name === "Publish Codex Plugin Doctor summary")?.run; + + if (!runDoctorScript || !summaryScript) throw new Error("Expected composite Action run-doctor and summary scripts."); + + await writeFile(path.join(root, "submission.zip"), "fixture", "utf8"); + await writeFile(path.join(root, "mock-doctor.sh"), `#!/usr/bin/env bash +set -euo pipefail +if [[ "\${1:-}" == "--version" ]]; then + printf '1.60.0\\n' + exit 0 +fi +printf '%s\\t' "$@" >> "$DOCTOR_LOG" +printf '\\n' >> "$DOCTOR_LOG" +output="" +for (( index = 1; index <= $#; index += 1 )); do + if [[ "\${!index}" == "--output" ]]; then + next=$((index + 1)) + output="\${!next}" + break + fi +done +if [[ -n "$output" ]]; then + mkdir -p "$(dirname "$output")" + if [[ " $* " == *" doctor submission archive "* ]]; then + printf '# Archive submission report\\n' > "$output" + else + printf '{}\\n' > "$output" + fi +fi +`, "utf8"); + await chmod(path.join(root, "mock-doctor.sh"), 0o755); + await mkdir(binDirectory, { recursive: true }); + await mkdir(runnerDirectory, { recursive: true }); + await writeFile(path.join(binDirectory, "codex-plugin-doctor"), `#!/usr/bin/env bash +exec "${toBashPath(path.join(root, "mock-doctor.sh"))}" "$@" +`, "utf8"); + await chmod(path.join(binDirectory, "codex-plugin-doctor"), 0o755); + + const environment = { + ...process.env, + PATH: `${toBashPath(binDirectory)}:${process.env.PATH ?? ""}`, + DOCTOR_LOG: toBashPath(logPath), + ALLOW_NETWORK_INPUT: inputs["allow-network"], + ALLOW_LOCAL_NETWORK_INPUT: inputs["allow-local-network"], + ALLOW_SESSION_LIFECYCLE_INPUT: inputs["allow-session-lifecycle"], + 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, + SUBMISSION_ARCHIVE_INPUT: inputs["submission-archive"], + 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"], + GITHUB_OUTPUT: toBashPath(actionOutputPath), + GITHUB_STATE: toBashPath(actionStatePath), + GITHUB_STEP_SUMMARY: toBashPath(stepSummaryPath), + RUNNER_TEMP: toBashPath(runnerDirectory) + }; + + try { + const runDoctorScriptPath = path.join(root, "run-doctor.sh"); + await writeFile(runDoctorScriptPath, renderInputs(runDoctorScript, inputs), "utf8"); + await chmod(runDoctorScriptPath, 0o755); + await execFileAsync(gitBash, [runDoctorScriptPath], { cwd: root, env: environment }); + const output = outputEntries(await readFile(actionOutputPath, "utf8")); + const manifest = JSON.parse(await readFile(path.join(reportDirectory, "codex-plugin-doctor-action-manifest.json"), "utf8")) as ActionRun["manifest"]; + const invocations = (await readFile(logPath, "utf8").catch(() => "")).split(/\r?\n/gu).filter(Boolean).map((line) => line.split("\t").filter(Boolean)); + + return { + root, + archiveJsonPath: path.join(reportDirectory, "codex-plugin-doctor-submission-archive.json"), + archiveSummaryPath: path.join(reportDirectory, "codex-plugin-doctor-submission-archive.md"), + output, + manifest, + invocations, + readState: async () => ({ + submission: await readFile(path.join(runnerDirectory, "codex-plugin-doctor-submission-ran"), "utf8"), + archive: await readFile(path.join(runnerDirectory, "codex-plugin-doctor-submission-archive-ran"), "utf8"), + status: await readFile(path.join(runnerDirectory, "codex-plugin-doctor-status"), "utf8") + }), + runSummary: async () => { + const summaryScriptPath = path.join(root, "publish-summary.sh"); + await writeFile(summaryScriptPath, renderInputs(summaryScript, inputs), "utf8"); + await chmod(summaryScriptPath, 0o755); + await execFileAsync(gitBash, [summaryScriptPath], { cwd: root, env: environment }); + return readFile(stepSummaryPath, "utf8"); + }, + cleanup: () => rm(root, { recursive: true, force: true }) + }; + } catch (error) { + await rm(root, { recursive: true, force: true }); + throw error; + } +} + +function submissionInvocations(run: ActionRun): string[][] { + return run.invocations.filter((invocation) => invocation[0] === "doctor" && invocation[1] === "submission"); +} + +describe("GitHub Action archive submission behavior", () => { + it("keeps archive outputs and summary empty when archive mode is disabled", async () => { + const run = await runArchiveAction(); + + try { + expect(run.output["submission-archive-json-path"]).toBe(""); + expect(run.output["submission-archive-summary-path"]).toBe(""); + expect((await run.readState()).archive).toBe("false"); + expect(run.manifest.reports.submissionArchiveJson).toEqual({ enabled: false, path: "" }); + expect(run.manifest.reports.submissionArchiveSummary).toEqual({ enabled: false, path: "" }); + expect(submissionInvocations(run)).toEqual([]); + await expect(readFile(run.archiveJsonPath, "utf8")).rejects.toThrow(); + expect(await run.runSummary()).not.toContain("Archive submission report"); + } finally { + await run.cleanup(); + } + }); + + it("runs directory submission strict mode without archive reports", async () => { + const run = await runArchiveAction({ submission: "true", "require-submission-ready": "true" }); + + try { + const submissions = submissionInvocations(run); + expect(submissions).toHaveLength(2); + expect(submissions.every((invocation) => !invocation.includes("archive"))).toBe(true); + expect(submissions[0]).toContain("--require-ready"); + expect(run.output["submission-archive-json-path"]).toBe(""); + expect((await run.readState()).archive).toBe("false"); + } finally { + await run.cleanup(); + } + }); + + it.each(["false", "true"])("runs archive-only mode and forwards strict gating only when %s", async (strict) => { + const run = await runArchiveAction({ "submission-archive": toBashPath(path.join(os.tmpdir(), "submission.zip")), "require-submission-ready": strict }); + + try { + const submissions = submissionInvocations(run); + expect(submissions).toHaveLength(2); + expect(submissions.every((invocation) => invocation.includes("archive"))).toBe(true); + expect(submissions[0].includes("--require-ready")).toBe(strict === "true"); + expect(submissions[1]).not.toContain("--require-ready"); + expect((await run.readState()).archive).toBe("true"); + expect(run.output["submission-archive-json-path"]).toBe(run.archiveJsonPath.replace(/\\/gu, "/")); + expect(run.output["submission-archive-summary-path"]).toBe(run.archiveSummaryPath.replace(/\\/gu, "/")); + expect(run.manifest.reports.submissionArchiveJson).toEqual({ enabled: true, path: run.output["submission-archive-json-path"] }); + expect(run.manifest.reports.submissionArchiveSummary).toEqual({ enabled: true, path: run.output["submission-archive-summary-path"] }); + expect(await readFile(run.archiveJsonPath, "utf8")).toContain("Archive submission report"); + expect(await run.runSummary()).toContain("Archive submission report"); + } finally { + await run.cleanup(); + } + }); + + it("rejects both submission modes without leaking archive paths or reports", async () => { + const run = await runArchiveAction({ submission: "true", "submission-archive": toBashPath(path.join(os.tmpdir(), "submission.zip")), "require-submission-ready": "true" }); + + try { + expect((await run.readState())).toMatchObject({ submission: "false", archive: "false", status: "2" }); + expect(submissionInvocations(run)).toEqual([]); + expect(run.output["submission-archive-json-path"]).toBe(""); + expect(run.output["submission-archive-summary-path"]).toBe(""); + await expect(readFile(run.archiveJsonPath, "utf8")).rejects.toThrow(); + expect(await run.runSummary()).not.toContain("Archive submission report"); + } finally { + await run.cleanup(); + } + }); + + it("rejects strict readiness without either submission mode", async () => { + const run = await runArchiveAction({ "require-submission-ready": "true" }); + + try { + expect((await run.readState())).toMatchObject({ submission: "false", archive: "false", status: "2" }); + expect(submissionInvocations(run)).toEqual([]); + expect(run.output["submission-archive-json-path"]).toBe(""); + expect(run.output["submission-archive-summary-path"]).toBe(""); + } finally { + await run.cleanup(); + } + }); +}); diff --git a/tests/action-metadata.test.ts b/tests/action-metadata.test.ts index e1fc8ec..faa98e4 100644 --- a/tests/action-metadata.test.ts +++ b/tests/action-metadata.test.ts @@ -198,8 +198,10 @@ describe("GitHub Action metadata", () => { 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('if [[ "$SUBMISSION_INPUT" == "true" && -n "$SUBMISSION_ARCHIVE_INPUT" ]]; then'); + expect(actionMetadata).toContain('echo "submission and submission-archive cannot be selected together." >&2'); + expect(actionMetadata).toContain('elif [[ "$REQUIRE_SUBMISSION_READY_INPUT" == "true" && "$SUBMISSION_INPUT" != "true" && -z "$SUBMISSION_ARCHIVE_INPUT" ]]; then'); + expect(actionMetadata).toContain('echo "require-submission-ready requires exactly one submission mode." >&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)"); @@ -217,6 +219,32 @@ describe("GitHub Action metadata", () => { expect(actionMetadata).not.toContain("submission_args+=(--allow-network"); }); + it("supports one opt-in archive submission mode without forwarding runtime consent", async () => { + const actionMetadata = normalizeNewlines(await readFile("action.yml", "utf8")); + + expect(actionMetadata).toMatch(/submission-archive:[\s\S]*?default: ""/); + expect(actionMetadata).toContain("submission-archive-json-path:"); + expect(actionMetadata).toContain("submission-archive-summary-path:"); + expect(actionMetadata).toContain('SUBMISSION_ARCHIVE_INPUT: ${{ inputs[\'submission-archive\'] }}'); + expect(actionMetadata).toContain('submission_archive_json_path="$report_dir/codex-plugin-doctor-submission-archive.json"'); + expect(actionMetadata).toContain('submission_archive_summary_path="$report_dir/codex-plugin-doctor-submission-archive.md"'); + expect(actionMetadata).toContain('submission_archive_args=(doctor submission archive "$SUBMISSION_ARCHIVE_INPUT" --json --output "$submission_archive_json_path")'); + expect(actionMetadata).toContain('run_doctor "submission archive preflight" "${submission_archive_args[@]}"'); + expect(actionMetadata).toContain('run_doctor "submission archive summary" doctor submission archive "$SUBMISSION_ARCHIVE_INPUT" --markdown --output "$submission_archive_summary_path"'); + expect(actionMetadata).toContain('if [[ "$SUBMISSION_INPUT" == "true" && -n "$SUBMISSION_ARCHIVE_INPUT" ]]; then'); + expect(actionMetadata).toContain('require-submission-ready requires exactly one submission mode.'); + expect(actionMetadata).toContain('submission_archive_args+=(--require-ready)'); + expect(actionMetadata).toContain('submissionArchiveJson: report("submissionArchiveJson", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE_JSON_PATH")'); + expect(actionMetadata).toContain('submissionArchiveSummary: report("submissionArchiveSummary", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE", "CODEX_PLUGIN_DOCTOR_ACTION_SUBMISSION_ARCHIVE_SUMMARY_PATH")'); + expect(actionMetadata).toContain('echo "submission-archive-json-path=$submission_archive_json_output"'); + expect(actionMetadata).toContain('echo "submission-archive-summary-path=$submission_archive_summary_output"'); + expect(actionMetadata).toContain('cat "$submission_archive_summary_path" >> "$GITHUB_STEP_SUMMARY"'); + expect(actionMetadata).not.toContain("SUBMISSION_ARCHIVE_RUNTIME_INPUT"); + expect(actionMetadata).not.toContain("SUBMISSION_ARCHIVE_ALLOW_NETWORK_INPUT"); + expect(actionMetadata).not.toContain("submission_archive_args+=(--runtime"); + expect(actionMetadata).not.toContain("submission_archive_args+=(--allow-network"); + }); + it("rejects installed-cache submission preflight requests without producing submission reports", async () => { const actionMetadata = normalizeNewlines(await readFile("action.yml", "utf8")); diff --git a/tests/public-readiness.test.ts b/tests/public-readiness.test.ts index d851027..9804c3f 100644 --- a/tests/public-readiness.test.ts +++ b/tests/public-readiness.test.ts @@ -216,6 +216,19 @@ describe("public repository readiness", () => { expect(readme).toContain("--markdown"); expect(readme).toContain("--require-ready"); expect(readme).toContain("manual_review_required"); + expect(readme).toContain("codex-plugin-doctor doctor submission archive "); + expect(readme).toContain("does not extract archive entries to disk"); + expect(actionGuide).toContain('submission-archive: ./plugin.zip'); + expect(actionGuide).toContain('require-submission-ready: "true"'); + expect(actionGuide).toContain("submission-archive-json-path"); + expect(actionGuide).toContain("submission-archive-summary-path"); + expect(actionGuide).toContain("exactly one submission mode"); + expect(actionGuide).toContain("does not extract archive entries to disk"); + expect(actionGuide).toContain("coverage: unavailable"); + expect(catalog).toContain("## Public Directory Archive Preflight Rules"); + expect(catalog).toContain("plugin.submission.archive.invalid_zip"); + expect(catalog).toContain("plugin.submission.archive.mcp_excluded"); + expect(catalog).toContain("plugin.submission.archive.screenshot_excluded"); expect(docsReadme).toContain("Public Directory Submission Preflight"); expect(architecture).toContain("no network requests"); expect(architecture).toContain("does not submit a package"); From e3705819d524d212d54f956b3a3cc778a9e5ae77 Mon Sep 17 00:00:00 2001 From: Furkan Date: Tue, 25 Aug 2026 10:10:04 +0300 Subject: [PATCH 18/20] test: harden submission archive boundaries --- src/core/submission-archive-reader.ts | 41 ++++++++++++++++++++++--- tests/submission-archive-reader.test.ts | 27 ++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/core/submission-archive-reader.ts b/src/core/submission-archive-reader.ts index eb689e9..0c8d3c8 100644 --- a/src/core/submission-archive-reader.ts +++ b/src/core/submission-archive-reader.ts @@ -19,7 +19,7 @@ const unsafeArchiveFileName = /[\u0000-\u001F\u007F\u2028\u2029\u200B-\u200F\u20 function sanitizedArchiveFileName(value: unknown): string { if (typeof value !== "string") return "archive.zip"; - const fileName = path.basename(value); + const fileName = path.win32.basename(path.posix.basename(value)); return fileName === "" || fileName.trim() !== fileName || unsafeArchiveFileName.test(fileName) ? "archive.zip" : fileName; @@ -278,10 +278,36 @@ function parseLocalExtraFields(content: Buffer): readonly { id: number; data: Bu return fields; } +async function readCentralEntryLayout( + fileDescriptor: number, + offset: number, + fileSize: number +): Promise<{ nextOffset: number; usesZip64Sizes: boolean } | null> { + if (!validSafeInteger(offset) || offset + 46 > fileSize) return null; + const fixed = await readExactly(fileDescriptor, offset, 46); + if (fixed === null || fixed.readUInt32LE(0) !== 0x02014b50) return null; + const compressedSize = fixed.readUInt32LE(20); + const uncompressedSize = fixed.readUInt32LE(24); + const nameLength = fixed.readUInt16LE(28); + const extraLength = fixed.readUInt16LE(30); + const commentLength = fixed.readUInt16LE(32); + const nextOffset = offset + 46 + nameLength + extraLength + commentLength; + if (!validSafeInteger(nextOffset) || nextOffset > fileSize) return null; + + const expectedZip64SizeBytes = (uncompressedSize === 0xffffffff ? 8 : 0) + (compressedSize === 0xffffffff ? 8 : 0); + if (expectedZip64SizeBytes === 0) return { nextOffset, usesZip64Sizes: false }; + const extra = await readExactly(fileDescriptor, offset + 46 + nameLength, extraLength); + const extraFields = extra === null ? null : parseLocalExtraFields(extra); + const zip64Fields = extraFields?.filter((field) => field.id === 0x0001) ?? []; + if (zip64Fields.length !== 1 || zip64Fields[0].data.length < expectedZip64SizeBytes) return null; + return { nextOffset, usesZip64Sizes: true }; +} + async function readLocalHeader( fileDescriptor: number, entry: yauzl.Entry, - fileSize: number + fileSize: number, + usesZip64Sizes: boolean ): Promise { const offset = entry.relativeOffsetOfLocalHeader; if (!validSafeInteger(offset) || offset + 30 > fileSize) return null; @@ -322,7 +348,7 @@ async function readLocalHeader( let descriptorLength = 0; if ((flags & 0x0008) !== 0) { - const zip64Descriptor = entry.versionNeededToExtract >= 45; + const zip64Descriptor = usesZip64Sizes; const descriptor = await readExactly(fileDescriptor, dataStart + entry.compressedSize, zip64Descriptor ? 24 : 16); if (descriptor === null) return null; const signed = descriptor.readUInt32LE(0) === 0x08074b50; @@ -554,9 +580,16 @@ export async function inspectSubmissionArchive(zipPath: string): Promise { expect(findingIds(invalid)).toContain("plugin.submission.archive.descriptor_invalid"); }); + it("accepts a 32-bit data descriptor when central metadata uses ZIP64-capable version 45 without ZIP64 sizes", async () => { + const archive = createZipFixture([ + { name: "version-45.txt", content: "descriptor", descriptor: "signed-32" } + ]); + const centralHeaderOffset = archive.indexOf(Buffer.from([0x50, 0x4b, 0x01, 0x02])); + archive.writeUInt16LE(45, centralHeaderOffset + 6); + + const inspection = await inspectFixture(archive); + + expect(inspection.findings).toEqual([]); + expect(inspection.reader).not.toBeNull(); + }); + + it("never exposes path separators, roots, drive names, or controls in an invalid archive report filename", async () => { + for (const [archivePath, expectedFileName] of [ + ["C:\\private\\secret.zip", "secret.zip"], + ["\\\\server\\share\\secret.zip", "secret.zip"], + ["/private/secret.zip", "secret.zip"], + ["C:\\private\\bad\u0001.zip", "archive.zip"] + ] as const) { + const inspection = await inspectSubmissionArchive(archivePath); + + expect(inspection.fileName).toBe(expectedFileName); + expect(inspection.fileName).not.toMatch(/[\\/\u0000-\u001F]/u); + } + }); + it("accepts local ZIP64 size sentinels and rejects missing or malformed required local ZIP64 data", async () => { const name = "local-zip64.txt"; const archive = createZipFixture([ From 0272677618113cfa04c03fe3afd93fde18543cb6 Mon Sep 17 00:00:00 2001 From: Furkan Date: Tue, 25 Aug 2026 10:20:34 +0300 Subject: [PATCH 19/20] chore: prepare v1.60.0 release --- CHANGELOG.md | 17 +++++++++++ README.md | 4 +-- docs/guides/github-action.md | 52 +++++++++++++++++----------------- package-lock.json | 4 +-- package.json | 2 +- tests/public-readiness.test.ts | 2 +- tests/release-check.test.ts | 4 +-- tests/release-notes.test.ts | 22 +++++++------- tests/release-sync.test.ts | 4 +-- 9 files changed, 65 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7255dc5..2ab5444 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.60.0] - 2026-08-25 + +### Added + +- added existing-ZIP Submission Archive Preflight reports through the CLI, JSON and Markdown output contracts, and opt-in GitHub Action archive inputs and reports +- added bounded no-extraction ZIP validation for the submission archive layout and supported skill metadata + +### Changed + +- archive blockers remain advisory by default and `--require-ready` enables strict readiness gating; MCP, app, and screenshot exclusions remain warnings for manual review +- kept directory submission preflight behavior unchanged + +### Security + +- validate stored and deflate ZIP entries with full safety passes for CRC, size, ZIP64, descriptors, paths, entry types, overlaps, and resource budgets +- keep archive preflight offline and non-executing: it performs no extraction, process execution, or network access and redacts paths and content from reports + ## [1.59.0] - 2026-08-17 ### Added diff --git a/README.md b/README.md index ddd2dfa..a627325 100644 --- a/README.md +++ b/README.md @@ -499,9 +499,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 - - uses: Esquetta/CodexPluginDoctor@v1.59.0 + - uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . runtime: "true" policy: codex-publish diff --git a/docs/guides/github-action.md b/docs/guides/github-action.md index f607a3a..a4e2efb 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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . registry-metadata: ./server.json require-registry-readiness: "true" @@ -42,9 +42,9 @@ The Action writes `mcp-registry-readiness.json` and exposes `registry-report-pat Use the submission preflight only when a workflow needs its separate offline report: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . submission: "true" require-submission-ready: "true" @@ -89,9 +89,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 - - uses: Esquetta/CodexPluginDoctor@v1.59.0 + - uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . runtime: "true" policy: codex-publish @@ -118,9 +118,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . sarif: "true" ``` @@ -132,9 +132,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . output-dir: doctor-ci-reports artifact-name: codex-plugin-doctor-reports @@ -174,11 +174,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 env: CODEX_PLUGIN_DOCTOR_SIGNING_KEY: ${{ secrets.CODEX_PLUGIN_DOCTOR_SIGNING_KEY }} with: - version: "1.59.0" + version: "1.60.0" path: . review-bundle: "true" review-bundle-verify: "true" @@ -209,9 +209,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . corpus-metrics-manifest: ../private-corpus/metrics.json ``` @@ -219,9 +219,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . corpus-metrics-manifest: ../private-corpus/metrics.json corpus-metrics-baseline: .doctor-baselines/corpus-metrics.json @@ -250,9 +250,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . runtime: "true" history: validation-history.jsonl @@ -272,9 +272,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . profile: publish ``` @@ -284,9 +284,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" path: . policy: codex-publish ``` @@ -298,9 +298,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.0" installed: "true" filter: github runtime: "false" @@ -311,9 +311,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.59.0 +- uses: Esquetta/CodexPluginDoctor@v1.60.0 with: - version: "1.59.0" + version: "1.60.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 24da815..7ebb8d9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codex-plugin-doctor", - "version": "1.59.0", + "version": "1.60.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codex-plugin-doctor", - "version": "1.59.0", + "version": "1.60.0", "license": "MIT", "dependencies": { "fast-xml-parser": "^5.11.0", diff --git a/package.json b/package.json index 2afc35b..d478d91 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codex-plugin-doctor", - "version": "1.59.0", + "version": "1.60.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/public-readiness.test.ts b/tests/public-readiness.test.ts index 9804c3f..0973b94 100644 --- a/tests/public-readiness.test.ts +++ b/tests/public-readiness.test.ts @@ -233,7 +233,7 @@ describe("public repository readiness", () => { 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("Esquetta/CodexPluginDoctor@v1.60.0"); expect(actionGuide).toContain('submission: "true"'); expect(actionGuide).toContain('require-submission-ready: "true"'); expect(actionGuide).toContain("submission-json-path"); diff --git a/tests/release-check.test.ts b/tests/release-check.test.ts index d22e5b8..ef8f71a 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.59.0 release version", async () => { + it("keeps package and lockfile roots on the 1.60.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.59.0"); + expect(packageJson.version).toBe("1.60.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 790e044..96104db 100644 --- a/tests/release-notes.test.ts +++ b/tests/release-notes.test.ts @@ -10,18 +10,20 @@ 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.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"); + const latestRelease = changelog.indexOf("## [1.60.0] - 2026-08-25"); + const previousRelease = changelog.indexOf("## [1.59.0] - 2026-08-17"); + const olderRelease = changelog.indexOf("## [1.58.0] - 2026-08-11"); expect(latestRelease).toBeGreaterThanOrEqual(0); expect(previousRelease).toBeGreaterThan(latestRelease); expect(olderRelease).toBeGreaterThan(previousRelease); + expect(extractReleaseSection(changelog, "1.60.0")).toContain( + "existing-ZIP Submission Archive Preflight" + ); 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"); }); it("keeps current README and Action examples pinned to the latest release", async () => { @@ -30,12 +32,12 @@ describe("extractReleaseSection", () => { 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"'); + expect(readme).toContain("Esquetta/CodexPluginDoctor@v1.60.0"); + expect(readme).toContain('version: "1.60.0"'); + expect(actionGuide).toContain("Esquetta/CodexPluginDoctor@v1.60.0"); + expect(actionGuide).toContain('version: "1.60.0"'); + expect(actionGuide).not.toContain("Esquetta/CodexPluginDoctor@v1.59.0"); + expect(actionGuide).not.toContain('version: "1.59.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 c4bd658..99f4c8f 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.59.0 stable release target", async () => { + it("uses the 1.60.0 stable release target", async () => { const packageJson = JSON.parse(await readFile("package.json", "utf8")) as { version: string; }; - expect(packageJson.version).toBe("1.59.0"); + expect(packageJson.version).toBe("1.60.0"); }); it("passes when npm, remote tag, GitHub release, and latest release match", () => { From 1d8a804aca9e387713254c1dd715f9a333112a28 Mon Sep 17 00:00:00 2001 From: Furkan Date: Tue, 25 Aug 2026 10:35:04 +0300 Subject: [PATCH 20/20] test: make Action harness portable --- tests/action-archive-behavior.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/action-archive-behavior.test.ts b/tests/action-archive-behavior.test.ts index 6fa783f..859b299 100644 --- a/tests/action-archive-behavior.test.ts +++ b/tests/action-archive-behavior.test.ts @@ -7,7 +7,7 @@ import { parse } from "yaml"; import { describe, expect, it } from "vitest"; const execFileAsync = promisify(execFile); -const gitBash = "C:\\Program Files\\Git\\bin\\bash.exe"; +const bashExecutable = process.platform === "win32" ? "C:\\Program Files\\Git\\bin\\bash.exe" : "bash"; type ActionMetadata = { inputs: Record; @@ -129,7 +129,7 @@ exec "${toBashPath(path.join(root, "mock-doctor.sh"))}" "$@" const runDoctorScriptPath = path.join(root, "run-doctor.sh"); await writeFile(runDoctorScriptPath, renderInputs(runDoctorScript, inputs), "utf8"); await chmod(runDoctorScriptPath, 0o755); - await execFileAsync(gitBash, [runDoctorScriptPath], { cwd: root, env: environment }); + await execFileAsync(bashExecutable, [runDoctorScriptPath], { cwd: root, env: environment }); const output = outputEntries(await readFile(actionOutputPath, "utf8")); const manifest = JSON.parse(await readFile(path.join(reportDirectory, "codex-plugin-doctor-action-manifest.json"), "utf8")) as ActionRun["manifest"]; const invocations = (await readFile(logPath, "utf8").catch(() => "")).split(/\r?\n/gu).filter(Boolean).map((line) => line.split("\t").filter(Boolean)); @@ -150,7 +150,7 @@ exec "${toBashPath(path.join(root, "mock-doctor.sh"))}" "$@" const summaryScriptPath = path.join(root, "publish-summary.sh"); await writeFile(summaryScriptPath, renderInputs(summaryScript, inputs), "utf8"); await chmod(summaryScriptPath, 0o755); - await execFileAsync(gitBash, [summaryScriptPath], { cwd: root, env: environment }); + await execFileAsync(bashExecutable, [summaryScriptPath], { cwd: root, env: environment }); return readFile(stepSummaryPath, "utf8"); }, cleanup: () => rm(root, { recursive: true, force: true })