From 5e843ab81ddede118275c45e22ae0ccaa1ed5701 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 08:53:40 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): duplicatePackage parses an explicit targetNamespace through the manifest namespace declaration An explicit `targetNamespace` was taken raw and spliced into every copied object name, while the derived default already had to satisfy the namespace charset. Both branches now pass one `ManifestSchema.shape.namespace` parse before anything is scanned or minted, refusing with the declaration's own sentence and the status-derived 400 VALIDATION_ERROR the derived branch already answered. Claude-Session: https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr Co-authored-by: Claude --- ...77-duplicate-package-explicit-namespace.md | 16 ++ ...duplicate-package-target-namespace.test.ts | 181 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 46 +++-- 3 files changed, 231 insertions(+), 12 deletions(-) create mode 100644 .changeset/19577-duplicate-package-explicit-namespace.md create mode 100644 packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts diff --git a/.changeset/19577-duplicate-package-explicit-namespace.md b/.changeset/19577-duplicate-package-explicit-namespace.md new file mode 100644 index 00000000000..74ffd5f148c --- /dev/null +++ b/.changeset/19577-duplicate-package-explicit-namespace.md @@ -0,0 +1,16 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): `duplicatePackage` parses an explicit `targetNamespace` through the manifest namespace declaration instead of taking it raw (#19577) + +Clause-②: no + +`ObjectStackProtocolImplementation.duplicatePackage` (and so `POST /api/v1/packages/:id/duplicate`, which forwards the body's `targetNamespace` verbatim) resolved its target namespace as `request.targetNamespace ?? deriveNamespaceFromPackageId(request.targetPackageId)`. The derived default already had to satisfy the namespace charset; the explicit value crossed no gate at all. That value is written as the copy's `manifest.namespace` and spliced into every copied object name as `${namespace}_${short}`, so `targetNamespace: 'my-ns'` minted `my-ns_ticket` — a name the object declaration (`/^[a-z_][a-z0-9_]*$/`) refuses — under a manifest namespace the manifest declaration refuses. + +- **One parse for both branches.** Whichever branch answered, the resolved namespace is now parsed by `ManifestSchema.shape.namespace` (`@objectstack/spec/kernel`) — the declaration itself, by reference, not a copied regex — before the source rows are scanned and before the target package record is minted, so a refusal never leaves an empty shell behind. +- **Refused, not sanitised.** An explicit value the declaration refuses is refused; it is never rewritten the way the derivation sanitises an id, because a copy landing under a namespace the caller did not write is a silent rewrite. +- **The sentence is the declaration's.** The refusal names the key and echoes the value, then carries the declaration's own rule text: `Invalid package namespace 'my-ns' on \`targetNamespace\`. Namespace must be 2-20 chars, lowercase alphanumeric + underscore. …`. The derived branch's refusal (an id whose final segment cannot carry the charset) now carries the same declaration sentence after its `Pass \`targetNamespace\` explicitly.` remedy, replacing a reworded one. +- **No new error code.** Both refusals throw with `statusCode: 400` and no `code`, so an HTTP boundary answers `400 VALIDATION_ERROR`, the status-derived code the derived branch already answered. + +**If you are refused:** pass a `targetNamespace` of 2–20 characters that starts with a lowercase letter and continues with lowercase letters, digits or underscores (`leave_copy`, not `leave-copy`), or omit it and let the door derive one from `targetPackageId`. Every conforming value duplicates exactly as before. diff --git a/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts b/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts new file mode 100644 index 00000000000..f26368850f8 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19577] `duplicatePackage` parses an EXPLICIT `targetNamespace` through the + * same declaration its derived default already had to satisfy. + * + * --------------------------------------------------------------------------- + * The gap these pins close + * --------------------------------------------------------------------------- + * The target namespace is spliced into every copied object name + * (`${targetNs}_${short}`) and written as the copy's `manifest.namespace`. + * #19417 made the DERIVED default go through `deriveNamespaceFromPackageId`, + * which sanitises toward the namespace charset and answers `null` when nothing + * valid comes out. The explicit branch of the same `??` was left raw: + * `targetNamespace: 'my-ns'` minted `my-ns_ticket`, a name the object + * declaration (`/^[a-z_][a-z0-9_]*$/`) refuses, and stored `my-ns` as a + * manifest namespace the manifest declaration refuses. + * + * --------------------------------------------------------------------------- + * What is pinned, and how + * --------------------------------------------------------------------------- + * - The refusal is asserted as the ENVELOPE an HTTP boundary answers with — + * `resolveThrownHttpError` (`@objectstack/types`), the one function both + * package doors call — so `status` and `code` are read the way the wire reads + * them, and `declaredCode` absent proves no new code was minted: the explicit + * branch answers the same status-derived code the derived branch does. + * - The rule sentence is read off the DECLARATION + * (`ManifestSchema.shape.namespace`) rather than retyped here: a pin that + * restated it would stay green on a reworded second sentence for one rule. + * - Every refusal also asserts that nothing was minted or scanned, and every + * refusal is answered by a lit control on a conforming explicit namespace + * that still duplicates under the names it prefixes — a refusal pin alone + * cannot tell "the rule is enforced" from "this door stopped duplicating". + */ +import { describe, it, expect, vi } from 'vitest'; +import { ManifestSchema } from '@objectstack/spec/kernel'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** Explicit namespaces the declaration refuses, one per reason it refuses them. */ +const REFUSED = [ + ['a hyphen — the Studio-shaped value the card names', 'my-ns'], + ['an uppercase letter', 'MyNs'], + ['a leading digit', '1leave'], + ['a leading underscore', '_leave'], + ['a single character — below the 2-char floor', 'l'], + ['21 characters — above the 20-char ceiling', 'abcdefghijklmnopqrstu'], + ['surrounding whitespace — parsed raw, never trimmed', ' leave2 '], + ['the empty string', ''], +] as const; + +/** Explicit namespaces the declaration admits — the lit controls. */ +const ADMITTED = [ + ['a plain word', 'leave2'], + ['an inner underscore', 'leave_copy'], + ['exactly 20 characters — the ceiling itself', 'abcdefghijklmnopqrst'], + ['exactly 2 characters — the floor itself', 'lv'], +] as const; + +/** A duplicate-door harness: one source package holding one object row. */ +function makeDuplicateImpl(sourceId = 'com.example.leave') { + const rows = [{ + id: 'r_1', + type: 'object', + name: 'leave_ticket', + organization_id: null, + package_id: sourceId, + state: 'active', + metadata: JSON.stringify({ name: 'leave_ticket', label: 'Ticket' }), + }]; + const installed: any[] = []; + const engine: any = { + find: vi.fn(async () => rows), + registry: { + getPackage: vi.fn(() => ({ + manifest: { id: sourceId, name: 'Leave', namespace: 'leave', version: '1.0.0' }, + })), + installPackage: vi.fn((manifest: any) => { + installed.push(manifest); + return { manifest, status: 'installed', enabled: true }; + }), + }, + }; + const impl = new ObjectStackProtocolImplementation(engine as never, () => new Map()); + const saveMetaItem = vi.spyOn(impl, 'saveMetaItem' as never); + (saveMetaItem as any).mockResolvedValue({ success: true } as never); + return { impl, engine, installed, saveMetaItem }; +} + +/** The thrown refusal, or a failure naming what happened instead. */ +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + throw new Error('expected the call to be refused, but it resolved'); +} + +/** The declaration's own sentence for a value — read, never retyped. */ +function declarationSentence(value: string): string { + const parsed = ManifestSchema.shape.namespace.safeParse(value); + expect(parsed.success, `the fixture '${value}' must be one the declaration refuses`).toBe(false); + return parsed.error!.issues[0]!.message; +} + +describe('[#19577] duplicatePackage refuses an explicit targetNamespace the declaration refuses', () => { + for (const [why, ns] of REFUSED) { + it(`refuses ${why} (${JSON.stringify(ns)}) before anything is minted`, async () => { + const { impl, engine, installed, saveMetaItem } = makeDuplicateImpl(); + const err = await refusalOf(() => (impl as any).duplicatePackage({ + sourcePackageId: 'com.example.leave', + targetPackageId: 'com.example.leave-copy', + targetNamespace: ns, + })); + + // The envelope, as the HTTP boundary resolves it: a 400 carrying + // the status-derived code — the same answer the derived branch's + // refusal gets — and no producer-minted code beside it. + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(400); + expect(envelope.code).toBe('VALIDATION_ERROR'); + expect(envelope.declaredCode).toBeUndefined(); + + // The first sentence names the key the caller wrote and echoes the + // value; the rule that follows is the DECLARATION's sentence. + expect(err.message.startsWith(`Invalid package namespace '${ns}' on \`targetNamespace\`.`)).toBe(true); + expect(err.message).toContain(declarationSentence(ns)); + + // ⭐ Nothing was minted and nothing was scanned: the refusal + // precedes the manifest write AND the copy loop. The manifest write + // sits inside a best-effort `catch {}`, so a refusal raised only + // there would be swallowed and reported as `success: true`. + expect(installed).toHaveLength(0); + expect(engine.registry.installPackage).not.toHaveBeenCalled(); + expect(engine.find).not.toHaveBeenCalled(); + expect(saveMetaItem).not.toHaveBeenCalled(); + }); + } +}); + +describe('[#19577] lit control — a conforming explicit targetNamespace still duplicates under it', () => { + for (const [why, ns] of ADMITTED) { + it(`${why} ('${ns}') is the copy's namespace and prefixes every copied object name`, async () => { + expect(ManifestSchema.shape.namespace.safeParse(ns).success).toBe(true); + const { impl, installed, saveMetaItem } = makeDuplicateImpl(); + const res: any = await (impl as any).duplicatePackage({ + sourcePackageId: 'com.example.leave', + targetPackageId: 'com.example.leave-copy', + targetNamespace: ns, + }); + expect(res.success).toBe(true); + expect(res.copiedCount).toBe(1); + expect(installed).toHaveLength(1); + expect(installed[0].namespace).toBe(ns); + const written = (saveMetaItem as any).mock.calls.map((c: any[]) => c[0].name); + expect(written).toEqual([`${ns}_ticket`]); + }); + } +}); + +describe('[#19577] the derived branch refuses through the same parse', () => { + it('an id with no derivable namespace answers the same envelope, with the declaration\'s sentence', async () => { + const { impl, installed, engine } = makeDuplicateImpl(); + const err = await refusalOf(() => (impl as any).duplicatePackage({ + sourcePackageId: 'com.example.leave', + // Admitted by the id pattern; a single-letter final segment cannot + // carry the namespace charset. + targetPackageId: 'com.example.a', + })); + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(400); + expect(envelope.code).toBe('VALIDATION_ERROR'); + expect(envelope.declaredCode).toBeUndefined(); + expect(err.message.startsWith("Cannot derive a package namespace from 'com.example.a'.")).toBe(true); + expect(err.message).toContain('`targetNamespace`'); + expect(err.message).toContain(declarationSentence('')); + expect(installed).toHaveLength(0); + expect(engine.find).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 9fb0b9d541b..74ee23265ed 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -19371,22 +19371,44 @@ export class ObjectStackProtocolImplementation implements // under the SOURCE's names — the collision the re-namespacing exists to // prevent. // - // ⛔ An explicitly declared `targetNamespace` still wins untouched: it - // is the caller's decision and this seam is about the DEFAULT. + // [#19577] ⭐ AN EXPLICIT `targetNamespace` STILL WINS OVER THE DEFAULT, + // BUT IT IS PARSED, NOT TAKEN RAW. Whichever branch answered, the value + // becomes the copy's `manifest.namespace` AND the prefix of every copied + // object name, so both branches pass ONE parse before anything is + // minted — `ManifestSchema.shape.namespace`, the declaration itself, + // never a hand-copied regex. An explicit value was the more reachable + // hole of the two: it is pure caller input, and `targetNamespace: + // 'my-ns'` minted `my-ns_ticket`, a name the object declaration refuses. + // + // ⛔ A refused explicit value is REFUSED, never sanitised the way the + // derivation sanitises an id: it is what the caller wrote, and a copy + // quietly landing under a namespace nobody wrote is the silent rewrite + // this door exists not to perform. + // + // The sentence is the DECLARATION's, surfaced — the discipline the id + // gate above follows — and this door adds only the key it read and the + // value that arrived. The throw carries `statusCode: 400` and no `code`, + // exactly like the id refusal above, so an HTTP boundary answers the + // status-derived `VALIDATION_ERROR` on both branches (`resolveThrownHttpError`). const sourceNs: string = (srcPkg?.manifest?.namespace as string) ?? (deriveNamespaceFromPackageId(request.sourcePackageId) ?? ''); - const targetNs: string | null = - request.targetNamespace ?? deriveNamespaceFromPackageId(request.targetPackageId); - if (!targetNs) { - // Reachable only for an id the pattern admits but the namespace - // charset cannot carry (a single-letter final segment), or for an - // explicit `targetNamespace: ''`. Loud, with the remedy — never a - // copy renamed with an empty prefix. + const explicitNs = request.targetNamespace; + const targetNs: string | null = explicitNs ?? deriveNamespaceFromPackageId(request.targetPackageId); + // `?? ''` because the declaration is `.optional()`: an ABSENT value + // passes it, and a derivation that produced nothing must not. + const declaredTargetNs = ManifestSchema.shape.namespace.safeParse(targetNs ?? ''); + if (targetNs == null || !declaredTargetNs.success) { + const rule = declaredTargetNs.error?.issues[0]?.message ?? 'See `manifest.namespace`'; throw Object.assign( new Error( - `Cannot derive a package namespace from '${request.targetPackageId}'. ` - + 'Pass `targetNamespace` explicitly — a lowercase letter followed by ' - + '1–19 letters, digits or underscores.', + explicitNs != null + ? `Invalid package namespace '${String(explicitNs)}' on \`targetNamespace\`. ${rule}. ` + + 'It becomes the copy\'s `manifest.namespace` and the prefix of every copied object name.' + // Reachable only for an id the pattern admits but the + // namespace charset cannot carry (a single-letter final + // segment) — never a copy renamed with an empty prefix. + : `Cannot derive a package namespace from '${request.targetPackageId}'. ` + + `Pass \`targetNamespace\` explicitly. ${rule}.`, ), { statusCode: 400 }, ); From a27d6d9348277ae8e8e276712eed7a391e389962 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 09:36:45 +0000 Subject: [PATCH 2/2] chore(changeset): declare the explicit-targetNamespace refusal a narrowing (minor) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An explicit targetNamespace outside the manifest.namespace declaration used to be accepted verbatim and is now refused, so the changeset declares Clause-② no (narrowing), ships minor, carries the BREAKING banner naming the refused values, and states its ADR-0087 disposition. Claude-Session: https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr Co-authored-by: Claude --- .changeset/19577-duplicate-package-explicit-namespace.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.changeset/19577-duplicate-package-explicit-namespace.md b/.changeset/19577-duplicate-package-explicit-namespace.md index 74ffd5f148c..d6943a913d4 100644 --- a/.changeset/19577-duplicate-package-explicit-namespace.md +++ b/.changeset/19577-duplicate-package-explicit-namespace.md @@ -1,10 +1,12 @@ --- -'@objectstack/metadata-protocol': patch +'@objectstack/metadata-protocol': minor --- fix(metadata-protocol): `duplicatePackage` parses an explicit `targetNamespace` through the manifest namespace declaration instead of taking it raw (#19577) -Clause-②: no +Clause-②: no (narrowing) + +**BREAKING for callers of `duplicatePackage` / `POST /api/v1/packages/:id/duplicate`** — an explicit `targetNamespace` outside the `manifest.namespace` declaration (`/^[a-z][a-z0-9_]{1,19}$/`) is now refused with a `400` before anything is copied, where it used to be accepted verbatim. Refused now: a hyphen or an uppercase letter (`my-ns`, `MyNs`), a leading digit or underscore (`1leave`, `_leave`), a single character (`l`), more than 20 characters, and surrounding whitespace. Some of these (`l`, `_leave`, a 21-character value) still yield legal object names, so they used to be copied, under a `manifest.namespace` the declaration refuses. Every conforming value — and every call that omits `targetNamespace` — duplicates exactly as before. `ObjectStackProtocolImplementation.duplicatePackage` (and so `POST /api/v1/packages/:id/duplicate`, which forwards the body's `targetNamespace` verbatim) resolved its target namespace as `request.targetNamespace ?? deriveNamespaceFromPackageId(request.targetPackageId)`. The derived default already had to satisfy the namespace charset; the explicit value crossed no gate at all. That value is written as the copy's `manifest.namespace` and spliced into every copied object name as `${namespace}_${short}`, so `targetNamespace: 'my-ns'` minted `my-ns_ticket` — a name the object declaration (`/^[a-z_][a-z0-9_]*$/`) refuses — under a manifest namespace the manifest declaration refuses. @@ -14,3 +16,5 @@ Clause-②: no - **No new error code.** Both refusals throw with `statusCode: 400` and no `code`, so an HTTP boundary answers `400 VALIDATION_ERROR`, the status-derived code the derived branch already answered. **If you are refused:** pass a `targetNamespace` of 2–20 characters that starts with a lowercase letter and continues with lowercase letters, digits or underscores (`leave_copy`, not `leave-copy`), or omit it and let the door derive one from `targetPackageId`. Every conforming value duplicates exactly as before. + +