From bd6d77e29bc0479703070e9407f5fca7795349c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 02:17:40 +0000 Subject: [PATCH] fix(cli)!: os plugin publish parses the compiled manifest through ManifestSchema before any upload `os plugin publish` and `readOspluginManifest` read the artifact's objectstack.plugin.json with a bare JSON.parse, so a stale artifact (for example one carrying the retired `packaging: 'manifest-deps'`) reached the registry and failed only at its consumer's install. Both now ask `readCompiledPluginManifest`, the helper `os plugin install` and the boot loader use, and publish refuses an invalid manifest with the schema's issues before any request is sent. The uploaded plugin_manifest stays the artifact's own manifest, without the defaults the parse stamps. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016WV4cyHFdayr8QnKE8nszT --- ...plugin-publish-parses-compiled-manifest.md | 22 +++++++ docs/qa/platform-checklist/areas/cli.json | 14 ++-- packages/cli/src/commands/plugin/publish.ts | 66 ++++++++++++------- packages/cli/src/utils/osplugin.ts | 23 +++++-- packages/cli/test/plugin-publish.test.ts | 63 ++++++++++++++++-- .../manifest-packaging-retirement.test.ts | 2 + 6 files changed, 152 insertions(+), 38 deletions(-) create mode 100644 .changeset/27810-plugin-publish-parses-compiled-manifest.md diff --git a/.changeset/27810-plugin-publish-parses-compiled-manifest.md b/.changeset/27810-plugin-publish-parses-compiled-manifest.md new file mode 100644 index 00000000000..d185dac7a52 --- /dev/null +++ b/.changeset/27810-plugin-publish-parses-compiled-manifest.md @@ -0,0 +1,22 @@ +--- +'@objectstack/cli': minor +--- + +fix(cli)!: `os plugin publish` parses the artifact's compiled manifest through `ManifestSchema` before anything is uploaded, so a stale `.osplugin` (e.g. `packaging: 'manifest-deps'`) is refused at the publisher's terminal with the schema's prescription instead of failing later at its consumer's install + +Clause-②: no (narrowing) +Metadata-protocol: narrowed + + + +**BREAKING** accept-set narrowing at `os plugin publish` (`@objectstack/cli`). + +- **Before.** `os plugin publish` `JSON.parse`d the artifact's `objectstack.plugin.json` and checked only that `id` and `version` were present and that `integrity` was an object. An artifact built before a schema change uploaded: one carrying the retired `packaging: 'manifest-deps'` reached the registry and was refused only at its consumer's `os plugin install`. Any other compiled-manifest drift (a missing `name` or `type`, an unknown key) uploaded the same way. +- **Now.** The publish parses the compiled manifest with `readCompiledPluginManifest` (`@objectstack/runtime`), the one helper `os plugin install` and the boot-time loader already use, before any request leaves the machine. + - A manifest the schema refuses stops the publish with `The artifact's objectstack.plugin.json is not a valid plugin manifest, so nothing was published:` and one line per schema issue. For `manifest-deps` that line is the retirement prescription. Neither the package row nor the version is posted. + - The hand-rolled malformed-`integrity` refusal is gone: a malformed map is a schema issue now. `Artifact manifest is missing id or version.` is now reached only by an empty `--manifest-id`. + - A valid artifact publishes unchanged: the uploaded `plugin_manifest` is still the manifest the artifact carries, byte for byte, without the defaults the parse stamps (`scope`, `defaultDatasource`). + - `readOspluginManifest` (internal to the CLI) parses through the same helper and is now async. +- **Who sees it.** Anyone publishing an `.osplugin` whose compiled manifest `ManifestSchema` refuses: one built by an `os plugin build` from before a schema change, or one assembled by hand. + +**The remedy.** Fix the plugin's source `objectstack.plugin.json` as the refusal says (for `manifest-deps`: delete the `packaging` key; `bundled` is the default and the only strategy), rebuild the artifact with `os plugin build`, re-sign it with `os plugin sign`, and publish it again. An artifact `os plugin build` produced from a valid manifest publishes unchanged. diff --git a/docs/qa/platform-checklist/areas/cli.json b/docs/qa/platform-checklist/areas/cli.json index 326e2a3eacd..098163764db 100644 --- a/docs/qa/platform-checklist/areas/cli.json +++ b/docs/qa/platform-checklist/areas/cli.json @@ -1716,7 +1716,7 @@ "title": "os plugin build enforces the manifest at the packaging boundary: an invalid manifest is refused with located errors, `bundled` is the one packaging strategy (the retired `manifest-deps` is refused at the manifest, a native addon is refused by name), and the compiled manifest the artifact carries is readable back", "since": "v15", "status": "active", - "revision": 6, + "revision": 7, "priority": "P2", "surface": "cli", "personas": [ @@ -1772,9 +1772,9 @@ "evidence": "the three refusals with their exit codes and the listings taken after them" }, { - "clause": "the artifact carries a COMPILED manifest — the authored manifest plus `main: \"dist/index.mjs\"` and an `integrity` map of archive-relative path → sha256 SRI digest, with the manifest itself and SIGNATURE excluded from the map — and os plugin publish reads that manifest back out of the artifact BEFORE any network call, refusing 'Artifact manifest is missing id or version.' when it cannot", + "clause": "the artifact carries a COMPILED manifest — the authored manifest plus `main: \"dist/index.mjs\"` and an `integrity` map of archive-relative path → sha256 SRI digest, with the manifest itself and SIGNATURE excluded from the map — and os plugin publish reads that manifest back out of the artifact BEFORE any network call and parses it through ManifestSchema (readCompiledPluginManifest, the parse os plugin install and the boot loader share), refusing one the schema refuses with 'The artifact's objectstack.plugin.json is not a valid plugin manifest, so nothing was published:' and one line per schema issue (a missing version reads `version: …`; a pre-retirement `manifest-deps` artifact reads the retirement prescription) when it cannot", "oracle": "build", - "verify": "read the objectstack.plugin.json entry out of each archive and diff it against the authored file; then run publish against a good artifact (expect it to reach the auth refusal, proving the manifest was read) and against one whose manifest lacks a version (expect the artifact-manifest refusal, before any auth). ⚠️ The publish leg is scored on WHICH refusal arrives, not on success", + "verify": "read the objectstack.plugin.json entry out of each archive and diff it against the authored file; then run publish against a good artifact (expect it to reach the auth refusal, proving the manifest was read) and against one whose manifest lacks a version (expect the schema refusal naming `version`, before any auth). ⚠️ The publish leg is scored on WHICH refusal arrives, not on success", "evidence": "the compiled manifest of each artifact, and the two publish outputs distinguished by their refusal" }, { @@ -1798,7 +1798,7 @@ "source": [ "packages/cli/src/commands/plugin/build.ts#formatZodErrors — manifest read + named refusal; ManifestSchema.safeParse + formatZodErrors; the explicit id/version guard; packaging default and the Loaded line; entry resolution; the bundled-only esbuild call (only `@objectstack/*` external); findNativeAddons and the native-addon refusal ahead of the integrity map; computeIntegrity + the compiled manifest; the artifact write and its printed block", "packages/cli/src/utils/osplugin.ts#MANIFEST_FILENAME — MANIFEST_FILENAME = 'objectstack.plugin.json'; OSPLUGIN_EXT; computeIntegrity (manifest and SIGNATURE excluded, keys sorted); the ustar writer with mtime pinned to 0; readOspluginManifest", - "packages/cli/src/commands/plugin/publish.ts#version — artifact discovery; readOspluginManifest before any network call; the id/version refusal; the unsigned-artifact notice; the auth refusal", + "packages/cli/src/commands/plugin/publish.ts#version — artifact discovery; the compiled manifest parsed through readCompiledPluginManifest (ManifestSchema) before any network call and the schema refusal; the empty `--manifest-id` refusal; the unsigned-artifact notice; the auth refusal", "packages/cli/src/commands/plugin/install.ts#PluginInstall — the install-time re-verification the integrity negative points at (verifyIntegrity over the unpacked entries, `integrity_mismatch`; a compiled manifest ManifestSchema refuses, a malformed integrity map included, is `manifest_invalid` one step earlier through readCompiledPluginManifest; nothing installed) and the install-time tier warning; not scored here", "packages/core/src/security/plugin-artifact-integrity.ts#verifyIntegrity — the one integrity verifier, shared by the publish preflight and the install-time check; not scored here", "packages/runtime/src/installed-plugin-loader.ts#dispatchPluginRuntime — the boot-time tier dispatch point the trust-tier negative points at (`node` only; `sandbox` / `worker` refused by name; an undeclared tier refused); not scored here", @@ -1844,6 +1844,12 @@ "date": "2026-10-30", "change": "one source row re-pointed, no clause changed: os plugin install no longer has an `integrity_malformed` refusal. The compiled manifest is parsed through ManifestSchema first (readCompiledPluginManifest, the parse install and the boot loader share), so a malformed integrity map is refused as `manifest_invalid` before the integrity step; `integrity_mismatch` is unchanged. Re-verified against install.ts and compiled-plugin-manifest.ts at this PR's head, no qa-run record", "ref": "#27686" + }, + { + "revision": 7, + "date": "2026-10-30", + "change": "the compiled-manifest clause's publish half (clause and verify) and the publish.ts source row re-pointed: os plugin publish no longer hand-parses the compiled manifest. It parses it through ManifestSchema with readCompiledPluginManifest (the helper os plugin install and the boot loader share), so a manifest that lacks a version is refused as 'not a valid plugin manifest' with the schema's issue line (`version: …`), not 'Artifact manifest is missing id or version.' — that sentence is now reachable only from an empty `--manifest-id`. The uploaded plugin_manifest stays the artifact's own manifest, unchanged", + "ref": "#27810" } ] }, diff --git a/packages/cli/src/commands/plugin/publish.ts b/packages/cli/src/commands/plugin/publish.ts index 7127d9cc6cb..8846d8389b8 100644 --- a/packages/cli/src/commands/plugin/publish.ts +++ b/packages/cli/src/commands/plugin/publish.ts @@ -7,9 +7,13 @@ * Flow: * 1. Read the `.osplugin` bytes + the detached `.sig` (publisher signature). * 2. Extract the compiled `objectstack.plugin.json` from inside the - * artifact (id / version / name / runtime / permissions / integrity), - * then preflight the artifact files against the manifest's declared - * per-file `integrity` digests (ADR-0025 §3.2) — refuse on any + * artifact and parse it through `ManifestSchema` + * (`readCompiledPluginManifest`, the read `os plugin install` and the + * boot-time loader make too): a manifest the schema refuses (one built + * before a value was retired, such as `packaging: 'manifest-deps'`) is + * refused here with the schema's own words, before anything leaves the + * machine. Then preflight the artifact files against the manifest's + * declared per-file `integrity` digests (ADR-0025 §3.2) — refuse on any * mismatch / missing / extra file; an absent map skips the check * (the field is optional). * 3. POST /cloud/packages — ensure the sys_package row exists @@ -39,6 +43,7 @@ import { type ArchiveFile, } from '../../utils/osplugin.js'; import { verifyIntegrity, formatIntegrityViolation } from '@objectstack/core'; +import { readCompiledPluginManifest } from '@objectstack/runtime'; interface PostResult { ok: boolean; status: number; body: any; error?: string } @@ -103,21 +108,42 @@ export default class PluginPublish extends Command { const checksum = sha256Hex(bytes); const base64 = Buffer.from(bytes).toString('base64'); - // 2. Extract the compiled manifest from inside the artifact. ──────── - let manifest: Record; + // 2. Extract the compiled manifest from inside the artifact, through + // `ManifestSchema` (the read `os plugin install` and the boot-time + // loader make too): an artifact built before a schema change is + // refused here, with the schema's own words, not at its consumer's + // install. ⛔ No manifest rule is restated in this command. ──────── let archiveFiles: ArchiveFile[]; try { archiveFiles = readTarGz(bytes); - const entry = archiveFiles.find((f) => f.path === MANIFEST_FILENAME); - if (!entry) throw new Error(`${MANIFEST_FILENAME} not found in artifact`); - manifest = JSON.parse(Buffer.from(entry.data).toString('utf8')) as Record; } catch (err: any) { printError(`Cannot read manifest from artifact: ${err?.message ?? err}`); this.exit(1); return; } - const id = String(flags['manifest-id'] ?? manifest.id ?? '').trim(); - const version = String(manifest.version ?? '').trim(); + const read = readCompiledPluginManifest(archiveFiles); + if (!read.ok) { + if (read.reason === 'unreadable') { + printError(`Cannot read manifest from artifact: ${read.message}`); + } else { + printError(`The artifact's ${MANIFEST_FILENAME} is not a valid plugin manifest, so nothing was published:`); + for (const issue of read.issues) console.log(` • ${issue}`); + console.log('\n Fix the source manifest as above and rebuild the artifact with `os plugin build` (then re-sign it with `os plugin sign`), and publish the fresh artifact.'); + } + this.exit(1); + return; + } + const manifest = read.manifest; + // What is uploaded is the manifest the artifact carries, as it carries + // it: the parse above stamps the schema's defaults (`scope`, + // `defaultDatasource`), which would make the declared manifest differ + // from the signed bytes' own. The entry is there and is a JSON object: + // the read above answered `ok`. + const declaredManifest = JSON.parse( + Buffer.from(archiveFiles.find((f) => f.path === MANIFEST_FILENAME)!.data).toString('utf8'), + ) as Record; + const id = String(flags['manifest-id'] ?? manifest.id).trim(); + const version = manifest.version; const displayName = String(flags['display-name'] ?? manifest.name ?? id).trim(); if (!id || !version) { printError('Artifact manifest is missing id or version.'); this.exit(1); return; } printStep(`${id}@${version} (${(bytes.byteLength / 1024).toFixed(1)} KB, runtime: ${manifest.runtime ?? 'unset'})`); @@ -126,21 +152,13 @@ export default class PluginPublish extends Command { // bytes against the manifest's own declared per-file digests before // upload. Absent map = permissive by contract (the field is // `.optional()`; artifacts built before integrity computation stay - // publishable). The installing side re-verifies at unpack - // (`os plugin install`, ADR-0025 §3.5 M1); this preflight is the - // publisher's own check, and the cloud control plane runs none. - const declaredIntegrity = manifest.integrity; - if ( - declaredIntegrity !== undefined && declaredIntegrity !== null - && (typeof declaredIntegrity !== 'object' || Array.isArray(declaredIntegrity)) - ) { - printError('Artifact manifest has a malformed `integrity` map (expected an object of path → digest). Rebuild with `os plugin build`.'); - this.exit(1); - return; - } + // publishable). The map's shape is `ManifestSchema`'s, checked above. + // The installing side re-verifies at unpack (`os plugin install`, + // ADR-0025 §3.5 M1); this preflight is the publisher's own check, and + // the cloud control plane runs none. const integrityCheck = verifyIntegrity( archiveFiles, - declaredIntegrity as Record | undefined, + manifest.integrity, { exempt: [MANIFEST_FILENAME, SIGNATURE_FILENAME] }, ); if (!integrityCheck.ok) { @@ -203,7 +221,7 @@ export default class PluginPublish extends Command { version, artifact_kind: 'plugin', osplugin: base64, - plugin_manifest: manifest, + plugin_manifest: declaredManifest, artifact_checksum: checksum, is_pre_release: flags['pre-release'] || /-(alpha|beta|rc|dev|preview|staging|pr)/i.test(version), }; diff --git a/packages/cli/src/utils/osplugin.ts b/packages/cli/src/utils/osplugin.ts index bc16f6c8265..8d376856adc 100644 --- a/packages/cli/src/utils/osplugin.ts +++ b/packages/cli/src/utils/osplugin.ts @@ -38,6 +38,7 @@ import { createHash } from 'node:crypto'; import { gzipSync } from 'node:zlib'; import { readTarGz, type ArchiveFile } from '@objectstack/core'; +import type { ObjectStackManifestParsed } from '@objectstack/spec/kernel'; export { readTar, readTarGz, sha256Hex, MAX_UNPACKED_ARTIFACT_BYTES, type ArchiveFile } from '@objectstack/core'; @@ -135,11 +136,21 @@ export function createTarGz(files: ArchiveFile[]): Buffer { } /** - * Extract + parse the compiled `objectstack.plugin.json` from an `.osplugin` - * blob. Throws if the manifest is absent or not valid JSON. + * Extract the compiled `objectstack.plugin.json` from an `.osplugin` blob and + * parse it through `ManifestSchema` — `readCompiledPluginManifest`, the one + * read `os plugin install` and the boot-time loader make too. Throws with the + * helper's message when the manifest is absent, not a JSON object, or refused + * by the schema (a retired value such as `packaging: 'manifest-deps'` carries + * its prescription in that message). + * + * The helper is imported on call, not at the top: every `os plugin` command + * loads this module, and `@objectstack/runtime`'s root entry would add about + * a second to `os plugin build` and `os plugin sign`, which never read a + * compiled manifest back. */ -export function readOspluginManifest(blob: Uint8Array): Record { - const entry = readTarGz(blob).find((f) => f.path === MANIFEST_FILENAME); - if (!entry) throw new Error(`${MANIFEST_FILENAME} not found in artifact`); - return JSON.parse(Buffer.from(entry.data).toString('utf8')) as Record; +export async function readOspluginManifest(blob: Uint8Array): Promise { + const { readCompiledPluginManifest } = await import('@objectstack/runtime'); + const read = readCompiledPluginManifest(readTarGz(blob)); + if (!read.ok) throw new Error(read.message); + return read.manifest; } diff --git a/packages/cli/test/plugin-publish.test.ts b/packages/cli/test/plugin-publish.test.ts index 70a86727d6d..ffdabcf6256 100644 --- a/packages/cli/test/plugin-publish.test.ts +++ b/packages/cli/test/plugin-publish.test.ts @@ -37,16 +37,27 @@ function buildArtifact(manifestOverride: Record = manifest, ext } describe('readTarGz / readOspluginManifest (inverse of createTarGz)', () => { - it('round-trips the files and extracts the manifest', () => { + it('round-trips the files and extracts the manifest', async () => { const blob = buildArtifact(); const files = readTarGz(blob); expect(files.map((f) => f.path).sort()).toEqual([MANIFEST_FILENAME, SIGNATURE_FILENAME, 'dist/index.mjs'].sort()); expect(Buffer.from(files.find((f) => f.path === 'dist/index.mjs')!.data).toString()).toBe('export const x=1;\n'); - expect(readOspluginManifest(blob)).toMatchObject({ id: 'com.acme.demo', version: '1.2.0' }); + await expect(readOspluginManifest(blob)).resolves.toMatchObject({ id: 'com.acme.demo', version: '1.2.0' }); }); - it('throws when the manifest is absent', () => { + it('throws when the manifest is absent', async () => { const blob = new Uint8Array(createTarGz([{ path: 'dist/index.mjs', data: new Uint8Array([1, 2, 3]) }])); - expect(() => readOspluginManifest(blob)).toThrow(/not found/); + await expect(readOspluginManifest(blob)).rejects.toThrow(/not found/); + }); + it("parses through ManifestSchema: a pre-retirement manifest (packaging: 'manifest-deps') throws the schema's prescription", async () => { + const blob = buildArtifact({ ...manifest, packaging: 'manifest-deps' }); + await expect(readOspluginManifest(blob)).rejects.toThrow(/packaging: `packaging: 'manifest-deps'` was removed/); + await expect(readOspluginManifest(blob)).rejects.toThrow(/Delete the key/); + }); + it('parses through ManifestSchema: a drifted manifest throws naming each issue', async () => { + const drifted: Record = { ...manifest, integrity: ['not', 'a', 'map'] }; + delete drifted.name; + const blob = buildArtifact(drifted); + await expect(readOspluginManifest(blob)).rejects.toThrow(/is not a valid plugin manifest: .*name: .*integrity: /); }); }); @@ -101,6 +112,33 @@ describe('os plugin publish (end-to-end, mocked cloud)', () => { expect(calls[1].body.plugin_manifest).toMatchObject({ id: 'com.acme.demo', runtime: 'sandbox' }); }); + it('uploads the manifest the artifact carries, as it carries it (no schema default stamped)', async () => { + dir = await mkdtemp(join(tmpdir(), 'plugin-publish-')); + const blob = buildArtifact(); + const artifactPath = join(dir, 'com.acme.demo-1.2.0.osplugin'); + await writeFile(artifactPath, blob); + process.env.OS_CLOUD_URL = 'http://cloud.test'; + process.env.OS_CLOUD_API_KEY = 'tok_123'; + const bodies: any[] = []; + vi.stubGlobal('fetch', vi.fn(async (url: string, init: any) => { + bodies.push(JSON.parse(init.body)); + const data = url.endsWith('/versions') ? { version: '1.2.0' } : { id: 'pkg_1', created: true }; + return { ok: true, status: 200, json: async () => ({ success: true, data }), statusText: 'OK' } as any; + })); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + try { + await PluginPublish.run([artifactPath]); + } finally { + logSpy.mockRestore(); + } + expect(bodies).toHaveLength(2); + // Exactly the artifact's own manifest: `ManifestSchema` would stamp + // `scope` and `defaultDatasource`, and neither is uploaded. + expect(bodies[1].plugin_manifest).toEqual(manifest); + expect(bodies[1].plugin_manifest).not.toHaveProperty('scope'); + expect(bodies[1].plugin_manifest).not.toHaveProperty('defaultDatasource'); + }); + async function runExpectingRefusal(blob: Uint8Array): Promise<{ output: string; fetchCalls: number }> { dir = await mkdtemp(join(tmpdir(), 'plugin-publish-')); const artifactPath = join(dir, 'com.acme.demo-1.2.0.osplugin'); @@ -121,6 +159,23 @@ describe('os plugin publish (end-to-end, mocked cloud)', () => { return { output: lines.join('\n'), fetchCalls: fetchMock.mock.calls.length }; } + it("refuses a pre-retirement artifact (packaging: 'manifest-deps') with the schema's prescription, before any upload", async () => { + const { output, fetchCalls } = await runExpectingRefusal(buildArtifact({ ...manifest, packaging: 'manifest-deps' })); + expect(fetchCalls).toBe(0); // refused pre-upload — no request left the machine + expect(output).toMatch(/^ +• packaging: `packaging: 'manifest-deps'` was removed/m); + expect(output).toMatch(/Delete the key/); + expect(output).toContain('os plugin build'); + }); + + it('refuses a compiled manifest ManifestSchema refuses, naming each issue, before any upload', async () => { + const drifted: Record = { ...manifest, integrity: ['not', 'a', 'map'] }; + delete drifted.name; + const { output, fetchCalls } = await runExpectingRefusal(buildArtifact(drifted)); + expect(fetchCalls).toBe(0); + expect(output).toMatch(/^ +• name: /m); + expect(output).toMatch(/^ +• integrity: /m); + }); + it('refuses the publish before any upload when a declared digest mismatches', async () => { const tampered = { ...manifest, integrity: { 'dist/index.mjs': sriDigest(new Uint8Array(Buffer.from('other bytes'))) } }; const { output, fetchCalls } = await runExpectingRefusal(buildArtifact(tampered)); diff --git a/packages/spec/src/kernel/manifest-packaging-retirement.test.ts b/packages/spec/src/kernel/manifest-packaging-retirement.test.ts index c196e36af2c..f5c3ed705f8 100644 --- a/packages/spec/src/kernel/manifest-packaging-retirement.test.ts +++ b/packages/spec/src/kernel/manifest-packaging-retirement.test.ts @@ -255,6 +255,8 @@ describe('tree-scoped absence: nothing inside the declared radius still authors 'packages/cli/test/plugin-build-bundled-only.test.ts', // The install door's own pin authors it to assert `os plugin install` refuses it before unpacking. 'packages/cli/src/commands/plugin/install.integration.test.ts', + // The publish door's own pin authors it to assert `os plugin publish` refuses it before any upload. + 'packages/cli/test/plugin-publish.test.ts', // The boot loader's own pin authors it to assert a pre-retirement install is refused by name. 'packages/runtime/src/installed-plugin-loader.test.ts', // The `os serve` boot pin authors it to assert a pre-retirement install is refused at boot, not imported.