Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/19577-duplicate-package-explicit-namespace.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@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 (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.

- **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.

<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is removed, renamed or reshaped: no spec key, no export, no stored row, and no request field — `targetNamespace` keeps its name and its type. There is no old spelling that maps to a new one: a refused value is caller input the `manifest.namespace` declaration already forbade, and a namespace is the caller's choice, so no mechanical mapping could be prescribed. The refusal itself carries the remedy — it names the key, echoes the value and quotes the declaration's rule. -->
Original file line number Diff line number Diff line change
@@ -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<unknown>): Promise<any> {
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();
});
});
46 changes: 34 additions & 12 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);
Expand Down
Loading