diff --git a/.changeset/portable-normative-rules-fail-closed.md b/.changeset/portable-normative-rules-fail-closed.md new file mode 100644 index 000000000..62dbafa24 --- /dev/null +++ b/.changeset/portable-normative-rules-fail-closed.md @@ -0,0 +1,5 @@ +--- +"agent-bundle": minor +--- + +portable: fail closed on the Agent Plugins 1.0.0 normative MCP rules during ordinary builds (#307 review follow-up) — the command-form, cwd-containment, env-key-placeholder, URL-form, and header rules the pinned `mcp.schema.json` cannot express now run at plan time (`portable.mcp.{command,cwd,env,url,headers}.standard`, errors) and the Agent Plugins byte lane (`AB6035`–`AB6037`) runs over every emitted `portable/` tree in `build` and `validate --artifact`, so a standard-invalid `mcp.json` (for example `command: "bin/server"`, an escaping `cwd`, duplicate case-insensitive headers, or plain HTTP off loopback) can no longer be published, whether or not `--host-validation` is requested (the flag now only adds the `portable` host report and its `AB6038` provenance note). Containment is checked with platform-independent POSIX semantics: a `./` command or `cwd` containing backslashes or NUL is refused so a bundle built on POSIX cannot resolve outside the root on Windows. Header values are now rejected when they contain any character outside visible ASCII, space, horizontal tab, and obs-text bytes (RFC 9110 §5.5; matches Node's `validateHeaderValue`), not only CR/LF/NUL. Backward compatibility: portable bundles whose MCP servers already satisfy the standard emit byte-identical output; servers that relied on path-bearing or whitespace-bearing `command` values, escaping `cwd`, or plain-HTTP remote URLs now fail the build with a field-scoped diagnostic instead of being published. diff --git a/README.md b/README.md index 109b560cb..707137197 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ npx agent-bundle dev --root . # local workbench with live rebu `targets: ['plugin']` emits one multi-host bundle at `dist/plugin/`: `.claude-plugin/`, `.codex-plugin/`, and `.cursor-plugin/` manifests over shared `skills/`, `hooks/`, `mcp/`, and `scripts/` directories. The bundle's generated `AGENTS.md` explains how to install it into each host. Per-host layouts are available as the `claude`, `codex`, `cursor`, and `portable` targets. -The `portable` target is the [Agent Plugins open standard](https://agent-plugins.org/specification) (specification 1.0.0) adapter — the default target, and the layout Cursor, Codex, VS Code, GitHub Copilot, Kiro, and ChatGPT load natively (Claude Code consumes it only through CLI translation). It emits the closed root `plugin.json` (canonical `$schema`, `name`, `version`, `description`, plus `author`, `homepage`, `repository`, `license`, `keywords`, and reverse-domain `extensions` authored under the `portable` config key), `skills//SKILL.md`, and `mcp.json` with stdio and Streamable HTTP servers whose `args`, `env` values, and `cwd` use the standard's `${PLUGIN_ROOT}`/`${PLUGIN_DATA}` placeholders. Rules, commands, hooks, marketplaces, and client extension directories are honestly unavailable there because the v1 standard packages only skills and MCP servers. Both documents are validated against the vendored, hash-pinned 1.0.0 schemas and the normative text at plan time, after every build (`AB6011`/`AB6012`), under `validate --artifact --host-validation` (`AB6035`–`AB6038`), and by `agent-bundle doctor` for installed Cursor local plugins that declare the standard's `$schema` (`AB7320`); see [Diagnostics](docs/diagnostics.md#agent-plugins-portable-validation-ab6035ab6038). Pins live in `packages/agent-bundle/src/adapters/schemas/portable/PROVENANCE.json`; the capability table `packages/agent-bundle/src/adapters/capabilities/portable-1.0.0.json` carries a dated row for every standard feature. +The `portable` target is the [Agent Plugins open standard](https://agent-plugins.org/specification) (specification 1.0.0) adapter — the default target, and the layout Cursor, Codex, VS Code, GitHub Copilot, Kiro, and ChatGPT load natively (Claude Code consumes it only through CLI translation). It emits the closed root `plugin.json` (canonical `$schema`, `name`, `version`, `description`, plus `author`, `homepage`, `repository`, `license`, `keywords`, and reverse-domain `extensions` authored under the `portable` config key), `skills//SKILL.md`, and `mcp.json` with stdio and Streamable HTTP servers whose `args`, `env` values, and `cwd` use the standard's `${PLUGIN_ROOT}`/`${PLUGIN_DATA}` placeholders. Rules, commands, hooks, marketplaces, and client extension directories are honestly unavailable there because the v1 standard packages only skills and MCP servers. Both documents are validated against the vendored, hash-pinned 1.0.0 schemas and the normative text at plan time (`portable.mcp.*.standard`), after every ordinary build and `validate --artifact` (`AB6011`/`AB6012` plus the Agent Plugins byte lane `AB6035`–`AB6037`), under `validate --artifact --host-validation` (same lane with the `AB6038` provenance note), and by `agent-bundle doctor` for installed Cursor local plugins that declare the standard's `$schema` (`AB7320`); see [Diagnostics](docs/diagnostics.md#agent-plugins-portable-validation-ab6035ab6038). Pins live in `packages/agent-bundle/src/adapters/schemas/portable/PROVENANCE.json`; the capability table `packages/agent-bundle/src/adapters/capabilities/portable-1.0.0.json` carries a dated row for every standard feature. Claude Code language servers are declared under `claude.lspServers`; the `claude` target and the Claude half of `plugin` emit the record as plugin-root `.lsp.json`. Agent Bundle expands path tokens only in `command`, `args`, `env`, and `workspaceFolder`, and it does not include the language-server binary — install that separately so the declared command is available on `PATH`. Codex, Cursor, and the portable format do not currently receive this host-scoped configuration. diff --git a/docs/diagnostics.md b/docs/diagnostics.md index 0623c48c0..29c00fc05 100644 --- a/docs/diagnostics.md +++ b/docs/diagnostics.md @@ -92,14 +92,23 @@ Validation happens at three moments, all fail-closed: 1. **Plan time** (`agent-bundle build`/`validate`): the emitted `plugin.json` and `mcp.json` are validated against the pinned schemas before they are written (`portable.schema.plugin`, `portable.schema.mcp`), authored manifest - metadata is checked field by field (`portable.manifest..invalid`), and + metadata is checked field by field (`portable.manifest..invalid`), MCP path tokens are refused where the standard forbids them - (`portable.mcp.token.*`). These target-scoped codes are errors. + (`portable.mcp.token.*`), and the normative MCP rules the schemas cannot + express are applied to each server as it will be written + (`portable.mcp.{command,cwd,env,url,headers}.standard`: command form, cwd + containment, env-key placeholders, URL form, header names/values/casing). + These target-scoped codes are errors, so a standard-invalid server never + reaches an artifact. 2. **Artifact time** (`agent-bundle build`, `validate --artifact`): the generic target-contract pass reports a missing required document as `AB6011` and a - pinned-schema rejection as `AB6012`; `validate --artifact --host-validation` - additionally runs the byte lane below and returns a `portable` host - validation report. + pinned-schema rejection as `AB6012`, and the Agent Plugins byte lane below + (`AB6035`–`AB6037`) runs over every tree emitted by the built-in portable + adapter, so a standard-invalid layout fails the ordinary build before + publication (a tree already carrying a symlink or other unsupported entry + is reported as `AB6013` and never read by this lane). + `validate --artifact --host-validation` additionally returns the same lane + as a `portable` host validation report with the `AB6038` provenance note. 3. **Installed bytes** (`agent-bundle doctor`): a Cursor local plugin whose root `plugin.json` declares an Agent Plugins `$schema` is validated with the same byte lane and reported under `AB7320` (an error marks the entry `corrupt`). @@ -107,7 +116,7 @@ Validation happens at three moments, all fail-closed: | Code | Severity | Meaning | Recovery | | --- | --- | --- | --- | | `AB6035` | error | The root `plugin.json` is missing, or a present `plugin.json`/`mcp.json` is unreadable, not valid JSON, or rejected by its pinned Agent Plugins 1.0.0 schema (closed manifest fields, plugin name constraints, reserved `PLUGIN_ROOT`/`PLUGIN_DATA` env keys, closed server variants). | Repair the generated Agent Plugins document so it satisfies the pinned 1.0.0 schema, then rebuild. | -| `AB6036` | error | A normative-text rule the schemas cannot express is violated: `plugin.json` and `mcp.json` declare different Agent Plugins versions (§10.1); a stdio `command` is neither a bare executable name nor a bundled plugin-relative `./` file, or carries a placeholder (§7.2.1); a `./`, `${PLUGIN_ROOT}`, or `${PLUGIN_DATA}` `cwd` escapes its root after resolution (§4.1/§7.2.1); a remote `url` is not an absolute HTTP(S) URL, carries user information or a fragment, uses plain HTTP against a non-loopback host, or carries a placeholder; header names are invalid, repeat under different casing, or carry placeholders (§7.2.1); an `env` key carries a placeholder (§9.2); `skills/` or `mcp.json` is present with the wrong filesystem kind (§6.2); or a `skills//` directory has no regular `SKILL.md` (§7.1). | Repair the generated portable layout or MCP entry to satisfy the Agent Plugins 1.0.0 normative text, then rebuild. | +| `AB6036` | error | A normative-text rule the schemas cannot express is violated: `plugin.json` and `mcp.json` declare different Agent Plugins versions (§10.1); a stdio `command` is neither a bare executable name nor a bundled plugin-relative `./` file, or carries a placeholder (§7.2.1); a `./`, `${PLUGIN_ROOT}`, or `${PLUGIN_DATA}` `cwd` escapes its root after resolution (§4.1/§7.2.1); a remote `url` is not an absolute HTTP(S) URL, carries user information or a fragment, uses plain HTTP against a non-loopback host, or carries a placeholder; header names are invalid, repeat under different casing, or carry placeholders, or a header value contains anything other than visible ASCII, space, horizontal tab, or obs-text bytes (§7.2.1, RFC 9110 §5.5); an `env` key carries a placeholder (§9.2); `skills/` or `mcp.json` is present with the wrong filesystem kind (§6.2); or a `skills//` directory has no regular `SKILL.md` (§7.1). | Repair the generated portable layout or MCP entry to satisfy the Agent Plugins 1.0.0 normative text, then rebuild. | | `AB6037` | error | A symlink inside the plugin resolves outside the plugin root, or cannot be resolved at all (§4.1 containment). | Replace the escaping symlink with a file or a link that resolves inside the plugin root, then rebuild. | | `AB6038` | info | Every portable host-validation report states that Agent Plugins publishes no reference validator and names the pinned schema provenance (specification repository commit, retrieval and re-verification dates) used for local validation. | Review the pinned Agent Plugins provenance before changing the local validator contract. | diff --git a/docs/framework-mode.md b/docs/framework-mode.md index ff42d5447..6f6a67858 100644 --- a/docs/framework-mode.md +++ b/docs/framework-mode.md @@ -278,10 +278,12 @@ metadata (`author`, `homepage`, `repository`, `license`, `keywords`) and reverse-domain `extensions` are authored under the `portable` config key and land in the root `plugin.json`; omitting them leaves the manifest exactly as before. Emitted bytes are validated against the pinned schemas and the -normative text at plan time, after every build, under -`validate --artifact --host-validation`, and by `doctor` for installed Cursor -local plugins that declare the standard's `$schema` -(`AB6035`–`AB6038`, `AB7320`; see `docs/diagnostics.md`). A dogfood proof +normative text at plan time (`portable.mcp.*.standard`), by the Agent Plugins +byte lane after every ordinary build and `validate --artifact` +(`AB6035`–`AB6037`), under `validate --artifact --host-validation` (same lane +plus the `AB6038` provenance note), and by `doctor` for installed Cursor +local plugins that declare the standard's `$schema` (`AB7320`; see +`docs/diagnostics.md`). A dogfood proof against the real Cursor IDE plugin loader (discovery, skill listing, MCP launch, and three observed Cursor 3.18.25 placeholder-expansion conformance gaps) is recorded in `docs/audits/2026-09-02-agent-plugins-cursor-ide-proof.md`. diff --git a/packages/agent-bundle/src/adapters/portable-mcp-rules.ts b/packages/agent-bundle/src/adapters/portable-mcp-rules.ts new file mode 100644 index 000000000..71337058a --- /dev/null +++ b/packages/agent-bundle/src/adapters/portable-mcp-rules.ts @@ -0,0 +1,185 @@ +import { posix, win32 } from 'node:path'; + +/** + * Agent Plugins 1.0.0 normative MCP rules that the pinned `mcp.schema.json` + * cannot express (§7.2.1 command, cwd, URL and header forms; §9.2 placeholder + * scope). Pure value checks shared by portable planning (so `build` and + * `validate` fail closed before publication) and by the bytes-at-rest lane in + * `host-contracts/portable-plugin-validation.ts`. Filesystem-backed checks + * (bundled command files, symlinks) stay in the byte lane. + */ + +export interface PortableMcpRuleIssue { + /** JSON-pointer-like location under the server entry, e.g. `command`, `headers/Authorization`. */ + readonly field: string; + readonly message: string; +} + +export const portablePlaceholderPattern = /\$\{PLUGIN_(?:ROOT|DATA)\}/u; + +const headerNamePattern = /^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/u; +// RFC 9110 §5.5 field values: VCHAR, SP, HTAB and obs-text only. Mirrors Node's +// `validateHeaderValue`, which rejects every other control or non-byte character. +const forbiddenHeaderValuePattern = /[^\t\u0020-\u007E\u0080-\u00FF]/u; +const loopbackIpv4Pattern = /^127(?:\.\d{1,3}){3}$/u; + +const issue = (field: string, message: string): PortableMcpRuleIssue => Object.freeze({ field, message }); + +const isRecord = (value: unknown): value is Readonly> => + typeof value === 'object' && value !== null && !Array.isArray(value); + +const isLoopbackHost = (hostname: string): boolean => + hostname === 'localhost' || + hostname === '[::1]' || + hostname === '::1' || + loopbackIpv4Pattern.test(hostname); + +/** + * Platform-independent §4.1 containment. The emitted value is interpreted by + * the consuming host, not the build host, so backslashes and NULs are refused + * outright (a POSIX build would otherwise accept `./safe\..\..\outside` that + * Windows resolves outside the root) and the remainder is normalized with + * POSIX semantics only. + */ +const hasForbiddenPathBytes = (value: string): boolean => value.includes('\\') || value.includes('\0'); + +/** + * Lexically normalized plugin-relative path, or `undefined` when the value + * climbs above its root or is not relative. Checks the normalized path itself + * for a leading `..` rather than resolving against a fixed synthetic root: + * `./../anchor/server` must escape regardless of what the root is named. + */ +export const containedPortableRelativePath = (relativePath: string): string | undefined => { + if (hasForbiddenPathBytes(relativePath) || posix.isAbsolute(relativePath)) return undefined; + const normalized = posix.normalize(relativePath); + if (normalized === '..' || normalized.startsWith('../')) return undefined; + return normalized; +}; + +const staysInsidePosixRoot = (relativePath: string): boolean => containedPortableRelativePath(relativePath) !== undefined; + +// A nonempty win32 root also catches drive-relative forms such as `C:server`, +// which `win32.isAbsolute` reports as relative yet resolve against a per-drive cwd. +const isAnyPlatformRooted = (value: string): boolean => + posix.isAbsolute(value) || win32.isAbsolute(value) || win32.parse(value).root.length > 0; + +/** §7.2.1: a stdio command is a bare executable name or a plugin-relative `./` path. */ +export const portableCommandIssues = (command: unknown): readonly PortableMcpRuleIssue[] => { + if (typeof command !== 'string') return Object.freeze([]); + if (portablePlaceholderPattern.test(command)) { + return Object.freeze([issue( + 'command', + 'contains an Agent Plugins placeholder, but clients never expand placeholders in command (Agent Plugins 1.0.0 §7.2.1)', + )]); + } + if (command.startsWith('./')) { + if (hasForbiddenPathBytes(command) || !staysInsidePosixRoot(command)) { + return Object.freeze([issue('command', `${JSON.stringify(command)} escapes the plugin root (Agent Plugins 1.0.0 §4.1)`)]); + } + return Object.freeze([]); + } + if (command.length === 0 || /[\s/\\\0]/u.test(command) || isAnyPlatformRooted(command) || command.startsWith('.')) { + return Object.freeze([issue( + 'command', + `${JSON.stringify(command)} is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1)`, + )]); + } + return Object.freeze([]); +}; + +/** §7.2.1: cwd stays inside the plugin root or plugin data directory after resolution. */ +export const portableCwdIssues = (cwd: unknown): readonly PortableMcpRuleIssue[] => { + if (typeof cwd !== 'string') return Object.freeze([]); + const relativePart = cwd.startsWith('./') + ? cwd + : cwd.startsWith('${PLUGIN_ROOT}') + ? `.${cwd.slice('${PLUGIN_ROOT}'.length)}` + : cwd.startsWith('${PLUGIN_DATA}') + ? `.${cwd.slice('${PLUGIN_DATA}'.length)}` + : undefined; + if (relativePart === undefined) return Object.freeze([]); + if (hasForbiddenPathBytes(relativePart)) { + return Object.freeze([issue( + 'cwd', + `${JSON.stringify(cwd)} must use forward-slash separators without backslashes or NUL so every consuming platform resolves it identically (Agent Plugins 1.0.0 §4.1)`, + )]); + } + if (staysInsidePosixRoot(relativePart)) return Object.freeze([]); + const scope = cwd.startsWith('${PLUGIN_DATA}') ? 'plugin data directory' : 'plugin root'; + return Object.freeze([issue('cwd', `${JSON.stringify(cwd)} escapes its ${scope} after resolution (Agent Plugins 1.0.0 §7.2.1)`)]); +}; + +/** §9.2: placeholders never expand in env keys. */ +export const portableEnvKeyIssues = (env: unknown): readonly PortableMcpRuleIssue[] => { + if (!isRecord(env)) return Object.freeze([]); + return Object.freeze(Object.keys(env) + .filter((key) => portablePlaceholderPattern.test(key)) + .map((key) => issue( + 'env', + `key ${JSON.stringify(key)} contains an Agent Plugins placeholder, but expansion never applies to env keys (Agent Plugins 1.0.0 §9.2)`, + ))); +}; + +/** §7.2.1: remote URLs are absolute http(s), no userinfo or fragment, HTTPS off loopback, no placeholders. */ +export const portableRemoteUrlIssues = (url: unknown): readonly PortableMcpRuleIssue[] => { + if (typeof url !== 'string') return Object.freeze([]); + if (portablePlaceholderPattern.test(url)) { + return Object.freeze([issue( + 'url', + 'contains an Agent Plugins placeholder, but clients never expand placeholders in url (Agent Plugins 1.0.0 §7.2.1)', + )]); + } + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return Object.freeze([issue('url', 'must be an absolute HTTP or HTTPS URL (Agent Plugins 1.0.0 §7.2.1)')]); + } + const issues: PortableMcpRuleIssue[] = []; + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + issues.push(issue('url', 'must use the http or https scheme (Agent Plugins 1.0.0 §7.2.1)')); + } + if (parsed.username.length > 0 || parsed.password.length > 0) { + issues.push(issue('url', 'must not contain user information (Agent Plugins 1.0.0 §7.2.1)')); + } + if (url.includes('#')) { + issues.push(issue('url', 'must not contain a fragment (Agent Plugins 1.0.0 §7.2.1)')); + } + if (parsed.protocol === 'http:' && !isLoopbackHost(parsed.hostname)) { + issues.push(issue( + 'url', + `uses plain HTTP against non-loopback host ${JSON.stringify(parsed.hostname)}; non-loopback endpoints must use HTTPS (Agent Plugins 1.0.0 §7.2.1)`, + )); + } + return Object.freeze(issues); +}; + +/** §7.2.1: header names are RFC 9110 tokens, values carry no control bytes, names are case-insensitive, no placeholders. */ +export const portableHeaderIssues = (headers: unknown): readonly PortableMcpRuleIssue[] => { + if (!isRecord(headers)) return Object.freeze([]); + const issues: PortableMcpRuleIssue[] = []; + const seen = new Map(); + for (const [name, value] of Object.entries(headers)) { + const field = `headers/${name}`; + if (!headerNamePattern.test(name)) { + issues.push(issue(field, 'is not a valid HTTP header field name (Agent Plugins 1.0.0 §7.2.1)')); + } + if (typeof value === 'string' && forbiddenHeaderValuePattern.test(value)) { + issues.push(issue(field, 'is not a valid HTTP header field value: only visible ASCII, space, horizontal tab and obs-text bytes are allowed (Agent Plugins 1.0.0 §7.2.1)')); + } + if (portablePlaceholderPattern.test(name) || (typeof value === 'string' && portablePlaceholderPattern.test(value))) { + issues.push(issue(field, 'contains an Agent Plugins placeholder, but clients never expand placeholders in headers (Agent Plugins 1.0.0 §7.2.1)')); + } + const folded = name.toLowerCase(); + const previous = seen.get(folded); + if (previous !== undefined) { + issues.push(issue( + field, + `repeats header ${JSON.stringify(previous)} under different casing; header names are case-insensitive (Agent Plugins 1.0.0 §7.2.1)`, + )); + } else { + seen.set(folded, name); + } + } + return Object.freeze(issues); +}; diff --git a/packages/agent-bundle/src/adapters/portable.ts b/packages/agent-bundle/src/adapters/portable.ts index ed9984618..2bcfca850 100644 --- a/packages/agent-bundle/src/adapters/portable.ts +++ b/packages/agent-bundle/src/adapters/portable.ts @@ -21,6 +21,14 @@ import { unavailableCapability, } from './capability-state.ts'; import capabilityTable from './capabilities/portable-1.0.0.json' with { type: 'json' }; +import { + portableCommandIssues, + portableCwdIssues, + portableEnvKeyIssues, + portableHeaderIssues, + portableRemoteUrlIssues, + type PortableMcpRuleIssue, +} from './portable-mcp-rules.ts'; import schemaProvenance from './schemas/portable/PROVENANCE.json' with { type: 'json' }; import mcpSchema from './schemas/portable/mcp.schema.json' with { type: 'json' }; import pluginSchema from './schemas/portable/plugin.schema.json' with { type: 'json' }; @@ -142,6 +150,19 @@ const unsupportedTokenDiagnostic = ( const { errorDiagnostic, schemaDiagnostics } = createTargetDiagnostics(portableName, 'Portable'); +/** + * Agent Plugins 1.0.0 normative MCP rules the schema cannot express, applied + * to the values as they will be written so ordinary `build` and `validate` + * fail closed instead of deferring to `--host-validation`. + */ +const normativeRuleDiagnostics = ( + server: NormalizedMcpServer, + issues: readonly PortableMcpRuleIssue[], +): readonly Diagnostic[] => issues.map((issue) => errorDiagnostic( + `portable.mcp.${issue.field.split('/')[0] ?? issue.field}.standard`, + `Portable MCP server "${server.name}" ${issue.field} ${issue.message}.`, +)); + const hasPortableTarget = (targets: readonly string[]): boolean => targets.includes(portableName); @@ -424,6 +445,15 @@ const planMcpServer = ( return { diagnostics }; } + diagnostics.push( + ...normativeRuleDiagnostics(server, portableCommandIssues(server.command)), + ...normativeRuleDiagnostics(server, portableCwdIssues(cwd)), + ...normativeRuleDiagnostics(server, portableEnvKeyIssues(declaredEnv)), + ); + if (diagnostics.length > 0) { + return { diagnostics }; + } + return { diagnostics, value: { @@ -458,6 +488,14 @@ const planMcpServer = ( return { diagnostics }; } + diagnostics.push( + ...normativeRuleDiagnostics(server, portableRemoteUrlIssues(server.url)), + ...normativeRuleDiagnostics(server, portableHeaderIssues(server.headers)), + ); + if (diagnostics.length > 0) { + return { diagnostics }; + } + return { diagnostics, value: { diff --git a/packages/agent-bundle/src/build/validate-artifact.ts b/packages/agent-bundle/src/build/validate-artifact.ts index 2e3f1b8cd..18871a50f 100644 --- a/packages/agent-bundle/src/build/validate-artifact.ts +++ b/packages/agent-bundle/src/build/validate-artifact.ts @@ -1,6 +1,7 @@ import { lstat, readFile } from 'node:fs/promises'; import { dirname, posix, resolve } from 'node:path'; +import { portableAdapter } from '../adapters/portable.ts'; import { createDefaultRegistry, type TargetRegistry } from '../adapters/registry.ts'; import type { TargetArtifactDocumentIssue, @@ -9,6 +10,7 @@ import type { import { mcpEntryAliasPattern } from '../config/normalize.ts'; import type { Diagnostic } from '../core/diagnostics.ts'; import { dataArrayValues, isPlainDataRecord, isRecord, ownDataValue } from '../core/strict-json.ts'; +import { validatePortablePluginFiles } from '../host-contracts/portable-plugin-validation.ts'; import { agentSkillsSchemaRevision } from '../schemas/agent-skills/contract.ts'; import { artifactDiagnostic as diagnostic, @@ -433,6 +435,44 @@ const validateTargetContracts = async (options: { return Object.freeze(diagnostics); }; +/** + * Agent Plugins 1.0.0 bytes-at-rest lane (AB6035–AB6037) over every tree + * emitted by the built-in portable adapter, so a standard-invalid `mcp.json` + * or layout fails ordinary `build` and `validate --artifact` rather than only + * `--host-validation`. The lane keys on the registered adapter identity, not + * the name: an advanced registry may bind `portable` to its own adapter and + * contract, and that output is validated by its own `artifactValidation`. + * A tree that already holds a symlink or other unsupported entry (AB6013) is + * skipped: the byte lane follows `plugin.json`/`mcp.json`/`skills` with + * `stat`/`readFile`/`readdir`, so it must not touch paths whose containment + * the filesystem inspection has already refused. + */ +const validatePortableTargets = async (options: { + readonly artifactRoot: string; + readonly filesystem: ArtifactFilesystemSnapshot; + readonly manifest: ArtifactManifest; + readonly registry: TargetRegistry; +}): Promise => { + const diagnostics: Diagnostic[] = []; + for (const target of options.manifest.targets) { + if (!options.registry.has(target.name) || options.registry.get(target.name) !== portableAdapter) continue; + const prefix = `${target.name}/`; + if (!options.filesystem.files.some((file) => file.path.startsWith(prefix))) continue; + const unsupported = options.filesystem.entries.some((entry) => + (entry.path === target.name || entry.path.startsWith(prefix)) && + entry.kind !== 'directory' && + entry.kind !== 'file'); + if (unsupported) continue; + for (const entry of await validatePortablePluginFiles({ + pluginDirectory: resolve(options.artifactRoot, target.name), + target: target.name, + })) { + diagnostics.push(Object.freeze({ ...entry, message: `Target ${JSON.stringify(target.name)}: ${entry.message}` })); + } + } + return Object.freeze(diagnostics); +}; + const ownershipRecovery = artifactDiagnosticRecoveries.AB6014; const isSkillArtifactPath = (relativePath: string, skills: string | undefined): boolean => { @@ -729,6 +769,7 @@ export const validateArtifactWithSnapshot = async ( // collecting in this fixed order keeps the diagnostics sequence deterministic. const [ targetContractDiagnostics, + portableTargetDiagnostics, mcpCoherenceDiagnostics, hookCoherenceDiagnostics, emittedSkillDiagnostics, @@ -740,6 +781,12 @@ export const validateArtifactWithSnapshot = async ( manifest, registry, }), + validatePortableTargets({ + artifactRoot, + filesystem: inspection.filesystem, + manifest, + registry, + }), validateMcpCoherence({ artifactRoot, files: inspection.files, @@ -768,6 +815,7 @@ export const validateArtifactWithSnapshot = async ( ]); diagnostics.push( ...targetContractDiagnostics, + ...portableTargetDiagnostics, ...mcpCoherenceDiagnostics, ...hookCoherenceDiagnostics, ...emittedSkillDiagnostics, diff --git a/packages/agent-bundle/src/host-contracts/portable-plugin-validation.ts b/packages/agent-bundle/src/host-contracts/portable-plugin-validation.ts index 924a34d36..dc9d000f1 100644 --- a/packages/agent-bundle/src/host-contracts/portable-plugin-validation.ts +++ b/packages/agent-bundle/src/host-contracts/portable-plugin-validation.ts @@ -1,7 +1,16 @@ import { lstat, readdir, readFile, realpath, stat } from 'node:fs/promises'; -import { isAbsolute, join, normalize, relative, resolve } from 'node:path'; +import { join, relative, resolve } from 'node:path'; import capabilityTable from '../adapters/capabilities/portable-1.0.0.json' with { type: 'json' }; +import { + containedPortableRelativePath, + portableCommandIssues, + portableCwdIssues, + portableEnvKeyIssues, + portableHeaderIssues, + portableRemoteUrlIssues, + type PortableMcpRuleIssue, +} from '../adapters/portable-mcp-rules.ts'; import schemaProvenance from '../adapters/schemas/portable/PROVENANCE.json' with { type: 'json' }; import mcpSchema from '../adapters/schemas/portable/mcp.schema.json' with { type: 'json' }; import pluginSchema from '../adapters/schemas/portable/plugin.schema.json' with { type: 'json' }; @@ -71,9 +80,6 @@ const pinnedDocumentContracts = Object.freeze([ }), ]); -const placeholderPattern = /\$\{PLUGIN_(?:ROOT|DATA)\}/u; -const headerNamePattern = /^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/u; -const loopbackIpv4Pattern = /^127(?:\.\d{1,3}){3}$/u; const schemaVersionPattern = /^https:\/\/agent-plugins\.org\/schemas\/([^/]+)\//u; const recoveryFor = (code: PortableDiagnosticCode): string => { @@ -127,14 +133,14 @@ const fileKind = async (path: string): Promise<'directory' | 'file' | 'missing' }; /** - * §4.1 plugin-relative path: begins with `./`, resolves against the plugin - * root, and stays inside it after lexical normalization. Filesystem symlink - * containment is the separate §4.1 symlink lane. + * §4.1 plugin-relative path: begins with `./` and stays inside the plugin root + * after platform-independent lexical normalization (shared with the planner). + * Filesystem symlink containment is the separate §4.1 symlink lane. */ const pluginRelativeTarget = (pluginDirectory: string, value: string): string | undefined => { - if (!value.startsWith('./') || value.includes('\\') || value.includes('\0')) return undefined; - const candidate = resolve(pluginDirectory, normalize(value)); - return isInsideOrEqual(pluginDirectory, candidate) ? candidate : undefined; + if (!value.startsWith('./')) return undefined; + const contained = containedPortableRelativePath(value); + return contained === undefined ? undefined : join(pluginDirectory, contained); }; const readDocuments = async ( @@ -226,123 +232,28 @@ const versionAgreementDiagnostics = ( )]); }; -const isLoopbackHost = (hostname: string): boolean => - hostname === 'localhost' || - hostname === '[::1]' || - hostname === '::1' || - loopbackIpv4Pattern.test(hostname); +const ruleDiagnostics = ( + serverName: string, + issues: readonly PortableMcpRuleIssue[], + target: string, +): readonly Diagnostic[] => freezeDiagnostics(issues.map((entry) => diagnostic( + 'AB6036', + `mcp.json/mcpServers/${serverName}/${entry.field} ${entry.message}.`, + 'error', + target, +))); const remoteUrlDiagnostics = ( serverName: string, url: unknown, target: string, -): readonly Diagnostic[] => { - if (typeof url !== 'string') return Object.freeze([]); - const location = `mcp.json/mcpServers/${serverName}/url`; - if (placeholderPattern.test(url)) { - return freezeDiagnostics([diagnostic( - 'AB6036', - `${location} contains an Agent Plugins placeholder, but clients never expand placeholders in url (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )]); - } - let parsed: URL; - try { - parsed = new URL(url); - } catch { - return freezeDiagnostics([diagnostic( - 'AB6036', - `${location} must be an absolute HTTP or HTTPS URL (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )]); - } - const diagnostics: Diagnostic[] = []; - if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { - diagnostics.push(diagnostic( - 'AB6036', - `${location} must use the http or https scheme (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - if (parsed.username.length > 0 || parsed.password.length > 0) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} must not contain user information (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - if (url.includes('#')) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} must not contain a fragment (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - if (parsed.protocol === 'http:' && !isLoopbackHost(parsed.hostname)) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} uses plain HTTP against non-loopback host ${JSON.stringify(parsed.hostname)}; non-loopback endpoints must use HTTPS (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - return freezeDiagnostics(diagnostics); -}; +): readonly Diagnostic[] => ruleDiagnostics(serverName, portableRemoteUrlIssues(url), target); const headerDiagnostics = ( serverName: string, headers: unknown, target: string, -): readonly Diagnostic[] => { - if (!isRecord(headers)) return Object.freeze([]); - const diagnostics: Diagnostic[] = []; - const seen = new Map(); - for (const [name, value] of Object.entries(headers)) { - const location = `mcp.json/mcpServers/${serverName}/headers/${name}`; - if (!headerNamePattern.test(name)) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} is not a valid HTTP header field name (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - if (typeof value === 'string' && /[\r\n\0]/u.test(value)) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} is not a valid HTTP header field value (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - if (placeholderPattern.test(name) || (typeof value === 'string' && placeholderPattern.test(value))) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} contains an Agent Plugins placeholder, but clients never expand placeholders in headers (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - const folded = name.toLowerCase(); - const previous = seen.get(folded); - if (previous !== undefined) { - diagnostics.push(diagnostic( - 'AB6036', - `${location} repeats header ${JSON.stringify(previous)} under different casing; header names are case-insensitive (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } else { - seen.set(folded, name); - } - } - return freezeDiagnostics(diagnostics); -}; +): readonly Diagnostic[] => ruleDiagnostics(serverName, portableHeaderIssues(headers), target); const stdioDiagnostics = async ( pluginDirectory: string, @@ -350,77 +261,26 @@ const stdioDiagnostics = async ( server: Readonly>, target: string, ): Promise => { - const diagnostics: Diagnostic[] = []; const command = server['command']; - const location = `mcp.json/mcpServers/${serverName}`; - if (typeof command === 'string') { - if (placeholderPattern.test(command)) { - diagnostics.push(diagnostic( - 'AB6036', - `${location}/command contains an Agent Plugins placeholder, but clients never expand placeholders in command (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } else if (command.startsWith('./')) { - const resolved = pluginRelativeTarget(pluginDirectory, command); - if (resolved === undefined) { - diagnostics.push(diagnostic( - 'AB6036', - `${location}/command ${JSON.stringify(command)} escapes the plugin root (Agent Plugins 1.0.0 §4.1).`, - 'error', - target, - )); - } else if ((await fileKind(resolved)) !== 'file') { - diagnostics.push(diagnostic( - 'AB6036', - `${location}/command ${JSON.stringify(command)} does not resolve to a bundled regular file (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - } else if (/[\s/\\]/u.test(command) || isAbsolute(command) || command.startsWith('.')) { - diagnostics.push(diagnostic( - 'AB6036', - `${location}/command ${JSON.stringify(command)} is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - } - const cwd = server['cwd']; - if (typeof cwd === 'string') { - const relativePart = cwd.startsWith('./') - ? cwd - : cwd.startsWith('${PLUGIN_ROOT}') - ? `.${cwd.slice('${PLUGIN_ROOT}'.length)}` - : cwd.startsWith('${PLUGIN_DATA}') - ? `.${cwd.slice('${PLUGIN_DATA}'.length)}` - : undefined; - if (relativePart !== undefined) { - const anchor = join(pluginDirectory, 'anchor'); - const candidate = resolve(anchor, normalize(relativePart === '.' ? './' : relativePart)); - if (!isInsideOrEqual(anchor, candidate)) { - diagnostics.push(diagnostic( - 'AB6036', - `${location}/cwd ${JSON.stringify(cwd)} escapes its ${cwd.startsWith('${PLUGIN_DATA}') ? 'plugin data directory' : 'plugin root'} after resolution (Agent Plugins 1.0.0 §7.2.1).`, - 'error', - target, - )); - } - } - } - const env = server['env']; - if (isRecord(env)) { - for (const key of Object.keys(env)) { - if (!placeholderPattern.test(key)) continue; + const diagnostics: Diagnostic[] = [ + ...ruleDiagnostics(serverName, portableCommandIssues(command), target), + ]; + // Only the byte lane can prove a plugin-relative command is a bundled regular file. + if (typeof command === 'string' && command.startsWith('./') && diagnostics.length === 0) { + const resolved = pluginRelativeTarget(pluginDirectory, command); + if (resolved !== undefined && (await fileKind(resolved)) !== 'file') { diagnostics.push(diagnostic( 'AB6036', - `${location}/env key ${JSON.stringify(key)} contains an Agent Plugins placeholder, but expansion never applies to env keys (Agent Plugins 1.0.0 §9.2).`, + `mcp.json/mcpServers/${serverName}/command ${JSON.stringify(command)} does not resolve to a bundled regular file (Agent Plugins 1.0.0 §7.2.1).`, 'error', target, )); } } + diagnostics.push( + ...ruleDiagnostics(serverName, portableCwdIssues(server['cwd']), target), + ...ruleDiagnostics(serverName, portableEnvKeyIssues(server['env']), target), + ); return freezeDiagnostics(diagnostics); }; diff --git a/packages/agent-bundle/tests/artifact-validator.test.ts b/packages/agent-bundle/tests/artifact-validator.test.ts index 28bcbfb86..52eda8115 100644 --- a/packages/agent-bundle/tests/artifact-validator.test.ts +++ b/packages/agent-bundle/tests/artifact-validator.test.ts @@ -9,6 +9,7 @@ import { pathToFileURL } from 'node:url'; import { expect, it } from '@rstest/core'; import { createDefaultRegistry, TargetRegistry } from '../src/adapters/registry.ts'; +import { validate } from '../src/api.ts'; import { readStandardNativeHookCommands, type TargetHookContract } from '../src/adapters/hook-contract.ts'; import { validateModernMcpDocument, @@ -847,6 +848,118 @@ it('rejects a canonically rehashed script with an unsupported extension', async } }); +it('fails ordinary artifact validation when an emitted portable tree breaks the Agent Plugins normative text', async () => { + const registry = createDefaultRegistry(); + const portable = targetFromRegistry(registry, 'portable'); + const files = [ + { contents: '# Install portable-test\n', kind: 'generated' as const, path: 'portable/INSTALL.md' }, + { contents: 'export {};\n', kind: 'generated' as const, path: 'portable/install.mjs' }, + { + contents: '{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","description":"Valid portable plugin.","name":"portable-test","version":"1.0.0"}\n', + kind: 'generated' as const, + path: 'portable/plugin.json', + }, + { + contents: JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json', + mcpServers: { + headers: { + headers: { 'X-Tenant': 'a', 'x-tenant': 'b' }, + type: 'streamable-http', + url: 'http://mcp.example.test/mcp', + }, + server: { command: 'bin/server', cwd: '${PLUGIN_ROOT}/../elsewhere', type: 'stdio' }, + }, + }) + '\n', + kind: 'generated' as const, + path: 'portable/mcp.json', + }, + ]; + const root = await writeArtifact(files, true, [portable]); + + try { + const diagnostics = await validateArtifact({ artifactRoot: root, registry }); + const normative = diagnostics.filter((entry) => entry.code === 'AB6036'); + expect(normative.map((entry) => entry.message)).toEqual([ + 'Target "portable": mcp.json/mcpServers/headers/url uses plain HTTP against non-loopback host "mcp.example.test"; non-loopback endpoints must use HTTPS (Agent Plugins 1.0.0 §7.2.1).', + 'Target "portable": mcp.json/mcpServers/headers/headers/x-tenant repeats header "X-Tenant" under different casing; header names are case-insensitive (Agent Plugins 1.0.0 §7.2.1).', + 'Target "portable": mcp.json/mcpServers/server/command "bin/server" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).', + 'Target "portable": mcp.json/mcpServers/server/cwd "${PLUGIN_ROOT}/../elsewhere" escapes its plugin root after resolution (Agent Plugins 1.0.0 §7.2.1).', + ]); + expect(normative.every((entry) => entry.severity === 'error' && entry.target === 'portable' && entry.recovery !== undefined)).toBe(true); + expect(diagnostics).not.toEqual(expect.arrayContaining([expect.objectContaining({ code: 'AB6012' })])); + + // --host-validation runs host reports only over an artifact the ordinary lane + // accepted, so a failing byte lane is reported exactly once and never as a report. + const hostValidated = await validate({ artifact: root, hostValidation: true, registry, root }); + expect(hostValidated.hostValidation).toBeUndefined(); + expect(hostValidated.diagnostics.filter((entry) => entry.code === 'AB6036').map((entry) => entry.message)) + .toEqual(normative.map((entry) => entry.message)); + expect(hostValidated.diagnostics.some((entry) => entry.code === 'AB6038')).toBe(false); + } finally { + await rm(root, { force: true, recursive: true }); + } +}); + +it('does not follow a symlinked portable document into the byte lane once the inspection refused it', async () => { + const registry = createDefaultRegistry(); + const portable = targetFromRegistry(registry, 'portable'); + const root = await writeArtifact([ + { contents: '# Install portable-test\n', kind: 'generated', path: 'portable/INSTALL.md' }, + { contents: 'export {};\n', kind: 'generated', path: 'portable/install.mjs' }, + { + contents: '{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","description":"Valid portable plugin.","name":"portable-test","version":"1.0.0"}\n', + kind: 'generated', + path: 'portable/plugin.json', + }, + ], true, [portable]); + const outside = await mkdtemp(join(tmpdir(), 'agent-bundle-outside-mcp-')); + + try { + await writeFile(join(outside, 'forged-mcp.json'), JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json', + mcpServers: { forged: { command: 'bin/server', type: 'stdio' } }, + })); + await symlink(join(outside, 'forged-mcp.json'), join(root, 'portable', 'mcp.json')); + + const diagnostics = await validateArtifact({ artifactRoot: root, registry }); + expect(diagnostics).toEqual(expect.arrayContaining([ + expect.objectContaining({ code: 'AB6013', generatedPath: 'portable/mcp.json' }), + ])); + // The forged content was never read: no schema or normative finding from behind the link. + expect(diagnostics.filter((entry) => ['AB6035', 'AB6036', 'AB6037'].includes(entry.code))).toEqual([]); + } finally { + await rm(root, { force: true, recursive: true }); + await rm(outside, { force: true, recursive: true }); + } +}); + +it('leaves an advanced registry adapter that reuses the portable name to its own artifact contract', async () => { + const registry = new TargetRegistry().register({ + artifactValidation: { + documents: [{ path: 'document.json', required: true, schema: 'document' }], + schemas: [{ name: 'document', validate: validateCustomDocument }], + }, + capabilities: {}, + metadata: customMetadata, + name: 'portable', + plan: () => ({ diagnostics: [], entries: [] }), + } satisfies TargetAdapter); + const target = targetFromRegistry(registry, 'portable'); + const root = await writeArtifact([ + { contents: '{"kind":"custom"}\n', kind: 'generated', path: 'portable/document.json' }, + ], true, [target]); + + try { + const diagnostics = await validateArtifact({ artifactRoot: root, registry }); + // No AB6035 for the absent Agent Plugins plugin.json: the byte lane keys on the + // built-in adapter identity. Only the name-keyed install-surface requirement remains. + expect(diagnostics.map((entry) => entry.code).sort()).toEqual(['AB6023', 'AB6024']); + } finally { + await rm(root, { force: true, recursive: true }); + } +}); + it('admits nested project assets in the target-owned recursive asset namespace', async () => { const registry = createDefaultRegistry(); const portable = targetFromRegistry(registry, 'portable'); diff --git a/packages/agent-bundle/tests/mcp-session-service.test.ts b/packages/agent-bundle/tests/mcp-session-service.test.ts index 1e5a4382d..e42944421 100644 --- a/packages/agent-bundle/tests/mcp-session-service.test.ts +++ b/packages/agent-bundle/tests/mcp-session-service.test.ts @@ -154,7 +154,9 @@ const publishRemoteEpoch = async (root: string, id: string): Promise mcp: { servers: { http: { - headers: { Authorization: 'Bearer ${PLUGIN_DATA}' }, + // Agent Plugins §7.2.1: clients never expand placeholders in + // headers, and the build now fails closed on one (AB6036). + headers: { Authorization: 'Bearer fixture-token' }, transport: 'streamable-http', url: 'https://mcp.example.test/tools', }, @@ -1090,8 +1092,7 @@ it('opens a generated streamable HTTP server through its modern transport', asyn const httpSession = await service.open({ epochId: 'epoch-remote', serverName: 'http', target: 'portable' }); expect(http[0]?.url).toBe('https://mcp.example.test/tools'); - expect(http[0]?.headers?.Authorization).toMatch(/^Bearer \/.+/u); - expect(http[0]?.headers?.Authorization).not.toContain('${PLUGIN_DATA}'); + expect(http[0]?.headers).toEqual({ Authorization: 'Bearer fixture-token' }); await Promise.all([httpSession.close(), service.close()]); } finally { diff --git a/packages/agent-bundle/tests/portable-adapter.test.ts b/packages/agent-bundle/tests/portable-adapter.test.ts index 4758aa2e1..c7cbc8e42 100644 --- a/packages/agent-bundle/tests/portable-adapter.test.ts +++ b/packages/agent-bundle/tests/portable-adapter.test.ts @@ -440,6 +440,90 @@ it('reports unsupported portable token locations instead of silently preserving ]); }); +it('fails closed at plan time on Agent Plugins 1.0.0 normative MCP rules the schema cannot express', () => { + const provenance = { kind: 'config' as const, sourcePath: '/workspace/agent-bundle.config.ts' }; + const plan = createDefaultRegistry().get('portable').plan({ + ...plugin(), + mcpServers: [ + { command: 'bin/server', id: 'mcp:path', name: 'path-command', provenance, targets: ['portable'], transport: 'stdio' }, + { command: 'bun run', id: 'mcp:space', name: 'spaced-command', provenance, targets: ['portable'], transport: 'stdio' }, + { + command: 'node', + cwd: 'agent-bundle:path:plugin-root/../elsewhere', + id: 'mcp:cwd', + name: 'escaping-cwd', + provenance, + targets: ['portable'], + transport: 'stdio', + }, + { + command: 'node', + cwd: './safe\\..\\..\\outside', + id: 'mcp:cwd-backslash', + name: 'backslash-cwd', + provenance, + targets: ['portable'], + transport: 'stdio', + }, + { command: './bin\\..\\..\\outside', id: 'mcp:cmd-backslash', name: 'backslash-command', provenance, targets: ['portable'], transport: 'stdio' }, + { command: './../anchor/server', id: 'mcp:anchor', name: 'anchor-collision', provenance, targets: ['portable'], transport: 'stdio' }, + { + command: 'node', + cwd: 'agent-bundle:path:plugin-root/../anchor', + id: 'mcp:anchor-cwd', + name: 'anchor-cwd', + provenance, + targets: ['portable'], + transport: 'stdio', + }, + { command: 'C:\\tools\\server.exe', id: 'mcp:win-abs', name: 'windows-absolute', provenance, targets: ['portable'], transport: 'stdio' }, + { command: 'C:server', id: 'mcp:win-drive', name: 'windows-drive-relative', provenance, targets: ['portable'], transport: 'stdio' }, + { + command: 'node', + env: { '${PLUGIN_ROOT}': 'literal' }, + id: 'mcp:env', + name: 'placeholder-env-key', + provenance, + targets: ['portable'], + transport: 'stdio', + }, + { id: 'mcp:http', name: 'plain-http', provenance, targets: ['portable'], transport: 'streamable-http', url: 'http://mcp.example.test/mcp' }, + { + headers: { 'X-Tenant': 'a', 'x-tenant': 'b', 'X-Trace': 'a\u0001b' }, + id: 'mcp:headers', + name: 'bad-headers', + provenance, + targets: ['portable'], + transport: 'streamable-http', + url: 'https://mcp.example.test/mcp', + }, + { command: 'node', id: 'mcp:ok', name: 'ok', provenance, targets: ['portable'], transport: 'stdio' }, + ], + }); + + expect(plan.diagnostics.map((diagnostic) => [diagnostic.code, diagnostic.message])).toEqual([ + ['portable.mcp.command.standard', 'Portable MCP server "path-command" command "bin/server" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.command.standard', 'Portable MCP server "spaced-command" command "bun run" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.cwd.standard', 'Portable MCP server "escaping-cwd" cwd "${PLUGIN_ROOT}/../elsewhere" escapes its plugin root after resolution (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.cwd.standard', 'Portable MCP server "backslash-cwd" cwd "./safe\\\\..\\\\..\\\\outside" must use forward-slash separators without backslashes or NUL so every consuming platform resolves it identically (Agent Plugins 1.0.0 §4.1).'], + ['portable.mcp.command.standard', 'Portable MCP server "backslash-command" command "./bin\\\\..\\\\..\\\\outside" escapes the plugin root (Agent Plugins 1.0.0 §4.1).'], + ['portable.mcp.command.standard', 'Portable MCP server "anchor-collision" command "./../anchor/server" escapes the plugin root (Agent Plugins 1.0.0 §4.1).'], + ['portable.mcp.cwd.standard', 'Portable MCP server "anchor-cwd" cwd "${PLUGIN_ROOT}/../anchor" escapes its plugin root after resolution (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.command.standard', 'Portable MCP server "windows-absolute" command "C:\\\\tools\\\\server.exe" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.command.standard', 'Portable MCP server "windows-drive-relative" command "C:server" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.env.standard', 'Portable MCP server "placeholder-env-key" env key "${PLUGIN_ROOT}" contains an Agent Plugins placeholder, but expansion never applies to env keys (Agent Plugins 1.0.0 §9.2).'], + ['portable.mcp.url.standard', 'Portable MCP server "plain-http" url uses plain HTTP against non-loopback host "mcp.example.test"; non-loopback endpoints must use HTTPS (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.headers.standard', 'Portable MCP server "bad-headers" headers/x-tenant repeats header "X-Tenant" under different casing; header names are case-insensitive (Agent Plugins 1.0.0 §7.2.1).'], + ['portable.mcp.headers.standard', 'Portable MCP server "bad-headers" headers/X-Trace is not a valid HTTP header field value: only visible ASCII, space, horizontal tab and obs-text bytes are allowed (Agent Plugins 1.0.0 §7.2.1).'], + ]); + expect(plan.diagnostics.every((diagnostic) => diagnostic.severity === 'error')).toBe(true); + const mcp = plan.entries.find((entry) => entry.kind === 'write' && entry.relativePath === 'mcp.json'); + expect(JSON.parse((mcp as Extract).content)).toEqual({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json', + mcpServers: { ok: { command: 'node', env: { AGENT_BUNDLE_PLUGIN_ROOT: '${PLUGIN_ROOT}' }, type: 'stdio' } }, + }); +}); + it('reports tokens forbidden in portable URLs, headers, cwd, and environment values', () => { const plan = createDefaultRegistry().get('portable').plan({ ...plugin(), diff --git a/packages/agent-bundle/tests/portable-plugin-validation.test.ts b/packages/agent-bundle/tests/portable-plugin-validation.test.ts index dddc51c53..6267e6228 100644 --- a/packages/agent-bundle/tests/portable-plugin-validation.test.ts +++ b/packages/agent-bundle/tests/portable-plugin-validation.test.ts @@ -146,6 +146,10 @@ it('applies the normative text where the schemas are silent: commands, cwd, URLs await writeJson(join(root, 'mcp.json'), { $schema: mcpSchema, mcpServers: { + anchorCommand: { command: './../anchor/server', type: 'stdio' }, + anchorCwd: { command: 'node', cwd: '${PLUGIN_ROOT}/../anchor', type: 'stdio' }, + backslashCommand: { command: './bin\\..\\..\\outside', type: 'stdio' }, + backslashCwd: { command: 'node', cwd: './safe\\..\\..\\outside', type: 'stdio' }, escapingCommand: { command: './../outside', type: 'stdio' }, escapingCwd: { command: 'node', cwd: '${PLUGIN_ROOT}/../elsewhere', type: 'stdio' }, escapingData: { command: 'node', cwd: '${PLUGIN_DATA}/../elsewhere', type: 'stdio' }, @@ -155,6 +159,7 @@ it('applies the normative text where the schemas are silent: commands, cwd, URLs type: 'streamable-http', url: 'https://mcp.example.test/mcp', }, + driveRelativeCommand: { command: 'C:server', type: 'stdio' }, missingBundled: { command: './bin/absent', type: 'stdio' }, pathCommand: { command: 'bin/server', type: 'stdio' }, placeholderCommand: { command: '${PLUGIN_ROOT}/bin/launch', type: 'stdio' }, @@ -169,6 +174,10 @@ it('applies the normative text where the schemas are silent: commands, cwd, URLs expect(new Set(codes(diagnostics))).toEqual(new Set(['AB6036'])); expect(messages(diagnostics)).toEqual([ + 'mcp.json/mcpServers/anchorCommand/command "./../anchor/server" escapes the plugin root (Agent Plugins 1.0.0 §4.1).', + 'mcp.json/mcpServers/anchorCwd/cwd "${PLUGIN_ROOT}/../anchor" escapes its plugin root after resolution (Agent Plugins 1.0.0 §7.2.1).', + 'mcp.json/mcpServers/backslashCommand/command "./bin\\\\..\\\\..\\\\outside" escapes the plugin root (Agent Plugins 1.0.0 §4.1).', + 'mcp.json/mcpServers/backslashCwd/cwd "./safe\\\\..\\\\..\\\\outside" must use forward-slash separators without backslashes or NUL so every consuming platform resolves it identically (Agent Plugins 1.0.0 §4.1).', 'mcp.json/mcpServers/escapingCommand/command "./../outside" escapes the plugin root (Agent Plugins 1.0.0 §4.1).', 'mcp.json/mcpServers/escapingCwd/cwd "${PLUGIN_ROOT}/../elsewhere" escapes its plugin root after resolution (Agent Plugins 1.0.0 §7.2.1).', 'mcp.json/mcpServers/escapingData/cwd "${PLUGIN_DATA}/../elsewhere" escapes its plugin data directory after resolution (Agent Plugins 1.0.0 §7.2.1).', @@ -176,6 +185,7 @@ it('applies the normative text where the schemas are silent: commands, cwd, URLs 'mcp.json/mcpServers/headers/headers/Bad Header is not a valid HTTP header field name (Agent Plugins 1.0.0 §7.2.1).', 'mcp.json/mcpServers/headers/headers/x-tenant repeats header "X-Tenant" under different casing; header names are case-insensitive (Agent Plugins 1.0.0 §7.2.1).', 'mcp.json/mcpServers/headers/headers/X-Token contains an Agent Plugins placeholder, but clients never expand placeholders in headers (Agent Plugins 1.0.0 §7.2.1).', + 'mcp.json/mcpServers/driveRelativeCommand/command "C:server" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).', 'mcp.json/mcpServers/missingBundled/command "./bin/absent" does not resolve to a bundled regular file (Agent Plugins 1.0.0 §7.2.1).', 'mcp.json/mcpServers/pathCommand/command "bin/server" is neither a bare executable name nor a plugin-relative ./ path (Agent Plugins 1.0.0 §7.2.1).', 'mcp.json/mcpServers/placeholderCommand/command contains an Agent Plugins placeholder, but clients never expand placeholders in command (Agent Plugins 1.0.0 §7.2.1).', @@ -187,6 +197,36 @@ it('applies the normative text where the schemas are silent: commands, cwd, URLs ]); }); +it('rejects every forbidden control character in HTTP header values while permitting horizontal tab', async () => { + const root = await conformantBundle(); + const invalidValues: Record = { + 'X-Bell': 'a\u0007b', + 'X-Cr': 'a\rb', + 'X-Del': 'a\u007fb', + 'X-Lf': 'a\nb', + 'X-Nul': 'a\u0000b', + 'X-Soh': 'a\u0001b', + 'X-Unicode': 'a\u2014b', + 'X-Vt': 'a\u000bb', + }; + await writeJson(join(root, 'mcp.json'), { + $schema: mcpSchema, + mcpServers: { + invalid: { headers: invalidValues, type: 'streamable-http', url: 'https://mcp.example.test/mcp' }, + valid: { + headers: { 'X-ObsText': 'caf\u00e9', 'X-Tab': 'a\tb', 'X-Visible': 'Bearer token-1 ~' }, + type: 'streamable-http', + url: 'https://mcp.example.test/mcp', + }, + }, + }); + const diagnostics = await validatePortablePluginFiles({ pluginDirectory: root, target: 'portable' }); + + expect(new Set(codes(diagnostics))).toEqual(new Set(['AB6036'])); + expect(messages(diagnostics)).toEqual(Object.keys(invalidValues).map((name) => + `mcp.json/mcpServers/invalid/headers/${name} is not a valid HTTP header field value: only visible ASCII, space, horizontal tab and obs-text bytes are allowed (Agent Plugins 1.0.0 §7.2.1).`)); +}); + it('reports fixed component locations of the wrong filesystem kind and skill directories without SKILL.md', async () => { const root = await conformantBundle(); await rm(join(root, 'skills'), { recursive: true }); diff --git a/packages/agent-bundle/tests/public-api-packed.test.ts b/packages/agent-bundle/tests/public-api-packed.test.ts index 0a3c7ac18..f8d6b5250 100644 --- a/packages/agent-bundle/tests/public-api-packed.test.ts +++ b/packages/agent-bundle/tests/public-api-packed.test.ts @@ -195,7 +195,9 @@ it('invokes a prebuilt MCP server from a clean packed consumer', async () => { mcpServers: { fixture: { args: ['mcp/server.mjs'], - command: process.execPath, + // Bare executable name, as the compiler emits: Agent Plugins §7.2.1 + // forbids absolute command paths and the build now fails closed. + command: 'node', cwd: '${PLUGIN_ROOT}', type: 'stdio', },