fix(metadata-protocol): the credential carry-forward reads the row the draft save overwrites (#22128)

storedBodyForCarryForward read the stored body at the key the caller
named. For a package-less draft save of a package-owned datasource that is
neither the inherited draft nor the package-bound active row, so the
comparison fell to the code layer and the stored credential was persisted
away (measured on the real route). Both of its reads now go through
SysMetadataRepository.headAt, which also answers the binding the write
targets, so the active fallback reads the row the draft overlays.

Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
Co-authored-by: Claude <noreply@anthropic.com>

Author: claude

packages/metadata-protocol/src/protocol.ts

@@ -8586,12 +8586,20 @@ export class ObjectStackProtocolImplementation implements
      * back. Here the stakes are higher than a phantom customization — the
      * subtracted material is a credential nobody can retype from the wire.
      *
-     * Reads the row at rest through the overlay repository, whose `get()` is
-     * documented VERBATIM (no ADR-0087 conversion): the comparison must be
+     * Reads the row at rest through the overlay repository, whose rows are
+     * served VERBATIM (no ADR-0087 conversion): the comparison must be
      * against the bytes that were written, because those are the bytes the read
      * exit redacted. {@link carryForwardRedactedValues} then decides per
      * redacted path — see its docblock for the three outcomes.
      *
+     * [#22128] The row is the one THIS save overwrites, from the repository's
+     * write-address resolution ({@link SysMetadataRepository.headAt}) — ⛔ never
+     * a read at the key the caller named. A draft save that names no package is
+     * stored in the package of the item's active row (#11087); read at the
+     * package-unbound key, neither that draft nor that active row was found,
+     * the comparison fell to the code layer, and the first package-less draft
+     * save of a package-owned datasource persisted its stored credential away.
+     *
      * ⚠️ THE DRAFT FALLBACK IS LOAD-BEARING, not defensive. A `?mode=draft`
      * save of an item with no draft row yet has nothing at its own state to
      * compare against, while the body the author edited came from the ACTIVE
@@ -8621,8 +8629,9 @@ export class ObjectStackProtocolImplementation implements
     }
 
     /**
-     * The body a carry-forward compares against: the overlay row at the write's
-     * own state, the ACTIVE row for a draft save with no draft row yet (see
+     * The body a carry-forward compares against: the row the write overwrites
+     * at its own state ({@link SysMetadataRepository.headAt}), the ACTIVE row
+     * the draft overlays for a draft save with no draft row yet (see
      * {@link carryForwardRedactedCredentials} for why that fallback is
      * load-bearing), else the CODE layer. RAW in every case — the bytes the
      * read exits redacted — and read with no `try`/`catch`, for the reason the
@@ -8635,15 +8644,19 @@ export class ObjectStackProtocolImplementation implements
         state: 'draft' | 'active';
         packageId: string | null;
     }): Promise<unknown> {
-        let stored = await args.repo.get(args.ref, {
+        // [#22128] The row this save overwrites, and with no draft row yet the
+        // active row the draft overlays: the one in the package the draft is
+        // stamped into (`packageId`, the inherited binding included).
+        const target = await args.repo.headAt(args.ref, {
             state: args.state,
             packageId: args.packageId,
         });
+        let stored = target.head;
         if (!stored && args.state === 'draft') {
-            stored = await args.repo.get(args.ref, {
+            stored = (await args.repo.headAt(args.ref, {
                 state: 'active',
-                packageId: args.packageId,
-            });
+                packageId: target.packageId,
+            })).head;
         }
         // [#20552] NO overlay row at either state: the body the read served is
         // the CODE layer, so that is what the incoming body is compared with.
@@ -18065,7 +18078,7 @@ export class ObjectStackProtocolImplementation implements
         state: 'active' | 'draft',
         packageId: string | null,
     ): Promise<string | null> {
-        return (await repo.headAt(ref, { state, packageId }))?.hash ?? null;
+        return (await repo.headAt(ref, { state, packageId })).head?.hash ?? null;
     }
 
     /**

packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts

@@ -195,15 +195,17 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)',
     const repo = makeRepo(engine);
     const first = await repo.put(REF, { label: 'draft 1' }, { parentVersion: null, actor: 't', state: 'draft' as const });
 
-    const head = await repo.headAt(REF, { state: 'draft', packageId: null });
+    const { head, packageId } = await repo.headAt(REF, { state: 'draft', packageId: null });
     expect(head?.hash).toBe(first.version);
+    // The binding the put writes under: the inherited package.
+    expect(packageId).toBe('app.k9qk');
     // The key the caller named holds no draft: that read is not the parent.
     expect(await repo.get(REF, { state: 'draft', packageId: null })).toBeNull();
 
     await expect(repo.put(REF, { label: 'from null' }, { parentVersion: null, actor: 't', state: 'draft' as const }))
       .rejects.toBeInstanceOf(ConflictError);
     const second = await repo.put(REF, { label: 'draft 2' }, { parentVersion: head!.hash, actor: 't', state: 'draft' as const });
-    expect((await repo.headAt(REF, { state: 'draft' }))?.hash).toBe(second.version);
+    expect((await repo.headAt(REF, { state: 'draft' })).head?.hash).toBe(second.version);
     expect(engine.rows.filter((r) => r.state === 'draft').map((r) => r.package_id)).toEqual(['app.k9qk']);
   });
 
@@ -216,23 +218,26 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)',
       },
     ]);
     const repo = makeRepo(engine);
-    expect((await repo.headAt(REF, { state: 'draft', packageId: null }))?.hash).toBe('sha-orphan');
+    const adopted = await repo.headAt(REF, { state: 'draft', packageId: null });
+    expect(adopted.head?.hash).toBe('sha-orphan');
+    // The orphan is updated INTO the package: the put's binding is the inherited one.
+    expect(adopted.packageId).toBe('app.k9qk');
   });
 
   it('an explicit package is its own address: never another package\'s row', async () => {
     const engine = makeFakeEngine([seededActive('app.k9qk')]);
     const repo = makeRepo(engine);
     await repo.put(REF, { label: 'draft' }, { parentVersion: null, actor: 't', state: 'draft' as const });
 
-    expect(await repo.headAt(REF, { state: 'draft', packageId: 'app.other' })).toBeNull();
-    expect((await repo.headAt(REF, { state: 'draft', packageId: 'app.k9qk' }))?.hash).toBeDefined();
+    expect(await repo.headAt(REF, { state: 'draft', packageId: 'app.other' })).toEqual({ head: null, packageId: 'app.other' });
+    expect((await repo.headAt(REF, { state: 'draft', packageId: 'app.k9qk' })).head?.hash).toBeDefined();
   });
 
   it('an active address inherits nothing: the unbound key is the head, as put writes it', async () => {
     const engine = makeFakeEngine([seededActive('app.k9qk')]);
     const repo = makeRepo(engine);
-    expect(await repo.headAt(REF, { state: 'active' })).toBeNull();
-    expect((await repo.headAt(REF, { state: 'active', packageId: 'app.k9qk' }))?.hash).toBe('sha-active');
+    expect(await repo.headAt(REF, { state: 'active' })).toEqual({ head: null, packageId: null });
+    expect((await repo.headAt(REF, { state: 'active', packageId: 'app.k9qk' })).head?.hash).toBe('sha-active');
   });
 
   it('an org-scoped draft address resolves across the ADR-0005 reach, as the inheriting put does', async () => {
@@ -243,6 +248,11 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)',
       orgLabel: 'org_1',
     } as never);
     const first = await repo.put(REF, { label: 'org draft' }, { parentVersion: null, actor: 't', state: 'draft' as const });
-    expect((await repo.headAt(REF, { state: 'draft' }))?.hash).toBe(first.version);
+    expect((await repo.headAt(REF, { state: 'draft' })).head?.hash).toBe(first.version);
+  });
+
+  it('a brand-new item drafted first: no head, and the package-less binding (nothing to inherit)', async () => {
+    const repo = makeRepo(makeFakeEngine());
+    expect(await repo.headAt(REF, { state: 'draft' })).toEqual({ head: null, packageId: null });
   });
 });

packages/metadata-protocol/src/sys-metadata-repository.ts

@@ -558,28 +558,30 @@ export class SysMetadataRepository implements MetadataRepository {
    * [#22128] The stored HEAD of one write address: the row a {@link put} with
    * the same `state` and `packageId` upserts — and so the row its optimistic
    * lock judges a parent against — served as {@link get} serves a row (its
-   * version is the one {@link lockAccepts} accepts). `null` when that put
-   * would create.
-   *
-   * Ask this, never {@link get}, for the parent of a write. `get` reads a row
-   * at the key it is handed; a write's key is not always the key its caller
-   * named. A `draft` put that names no package inherits the package of the
-   * item's active row (#11087), so `get(ref, { state: 'draft', packageId:
-   * null })` reads the package-UNBOUND row while the put upserts, and locks
-   * against, the inherited one. A parent taken from that read is `null` for a
-   * draft that exists, and an unpinned (ADR-0008 last-writer-wins) save was
-   * refused 409 by its own lock.
-   *
-   * Both answers come from ONE resolution, {@link resolveWriteHead}.
+   * version is the one {@link lockAccepts} accepts), or `null` when that put
+   * would create; and the package binding that put writes under (`packageId`
+   * here: the named package, the one a package-less draft inherits, or `null`
+   * for the package-unbound row).
+   *
+   * Ask this, never {@link get}, for anything a write at an address compares
+   * against — its parent, or the stored body it carries a withheld credential
+   * forward from. `get` reads a row at the key it is handed; a write's key is
+   * not always the key its caller named. A `draft` put that names no package
+   * inherits the package of the item's active row (#11087), so
+   * `get(ref, { state: 'draft', packageId: null })` reads the package-UNBOUND
+   * row while the put upserts, and locks against, the inherited one. A parent
+   * taken from that read is `null` for a draft that exists, and an unpinned
+   * (ADR-0008 last-writer-wins) save was refused 409 by its own lock.
+   *
+   * Every answer comes from ONE resolution, {@link resolveWriteHead}.
    */
   async headAt(
     ref: MetaRef,
     opts: { state?: OverlayState; packageId?: string | null },
-  ): Promise<MetadataItem | null> {
+  ): Promise<{ head: MetadataItem | null; packageId: string | null }> {
     this.assertOpen();
-    const { row } = await this.resolveWriteHead(ref, opts.state ?? 'active', opts.packageId);
-    if (!row) return null;
-    return this.rowToItem(ref, row);
+    const { row, targetPackageId } = await this.resolveWriteHead(ref, opts.state ?? 'active', opts.packageId);
+    return { head: row ? this.rowToItem(ref, row) : null, packageId: targetPackageId };
   }
 
   /**

packages/rest/src/meta-draft-head-package-inheritance.test.ts

@@ -17,7 +17,9 @@
  *
  * `?package=all` is the second member of that class. The save door folds it
  * to the env-local overlay; the read door forwarded the literal `all`, so its
- * `version` was resolved at a package address no save writes.
+ * `version` was resolved at a package address no save writes. The credential
+ * carry-forward (#8154) is the third: it compared the served body against the
+ * row at the named key, not the row the draft save overwrites.
  *
  * Driven at the HTTP door on the real stack: a better-sqlite3 `:memory:`
  * engine, the real `sys_metadata*` objects, a real
@@ -38,6 +40,7 @@ import { ObjectQL } from '@objectstack/objectql';
 import { SqlDriver } from '@objectstack/driver-sql';
 import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
 import { SysMetadata, SysMetadataHistoryObject, SysMetadataAuditObject } from '@objectstack/platform-objects/metadata';
+import { hashSpec } from '@objectstack/metadata-core';
 import { RestServer } from './rest-server.js';
 
 /** `registry.registerObject` requires a package id (see the sibling real-stack tests). */
@@ -92,8 +95,11 @@ function makeRes() {
 /**
  * Boot the real stack with the cache off: the default cached arm of the plain
  * read publishes no OCC carriers, so the active-row pins read the uncached arm.
+ * `item` names the one item the helpers address; `permissions` the caller's
+ * system capabilities (a `datasource` is read and written under
+ * `manage_platform_settings`).
  */
-async function boot() {
+async function boot(opts: { item?: { type: string; name: string }; permissions?: string[] } = {}) {
     const engine = new ObjectQL();
     liveEngines.push(engine);
     engine.registerDriver(new SqlDriver({
@@ -112,7 +118,8 @@ async function boot() {
     const config = { api: { requireAuth: false }, metadata: { enableCache: false } };
     const rest: any = new RestServer(createMockServer() as any, protocol as any, config as any);
     // An author: `manage_metadata` saves and reads drafts.
-    rest.resolveExecCtx = async () => ({ userId: 'u_author', systemPermissions: ['manage_metadata'] });
+    const systemPermissions = opts.permissions ?? ['manage_metadata'];
+    rest.resolveExecCtx = async () => ({ userId: 'u_author', systemPermissions });
     rest.registerRoutes();
 
     const route = (method: string, path: string) => {
@@ -125,8 +132,8 @@ async function boot() {
         await route(method, routePath).handler({ method, headers: {}, query: {}, params: {}, ...req }, res);
         return res;
     };
-    const item = { type: 'view', name: 'case_grid' };
-    const path = `${META}/view/case_grid`;
+    const item = opts.item ?? { type: 'view', name: 'case_grid' };
+    const path = `${META}/${item.type}/${item.name}`;
 
     /** `GET /meta/view/case_grid`. */
     const read = (query: Record<string, string> = {}) =>
@@ -136,15 +143,31 @@ async function boot() {
         call('PUT', `${META}/:type/:name`, { path, params: item, query, headers, body });
 
     /** The stored rows of the item at one lifecycle, with their binding — the store, not the door's word for it. */
-    const stored = async (state: 'active' | 'draft') => {
-        const rows = await engine.find('sys_metadata', { where: { type: 'view', name: 'case_grid', state } });
+    const storedBodies = async (state: 'active' | 'draft') => {
+        const rows = await engine.find('sys_metadata', { where: { type: item.type, name: item.name, state } });
         return (rows ?? []).map((r: any) => ({
-            label: JSON.parse(String(r.metadata)).label as string,
+            body: JSON.parse(String(r.metadata)) as Record<string, any>,
             packageId: (r.package_id ?? null) as string | null,
         }));
     };
+    const stored = async (state: 'active' | 'draft') =>
+        (await storedBodies(state)).map(({ body, packageId }) => ({ label: body.label as string, packageId }));
+
+    /**
+     * A row as it exists at rest from before the write gates — the only way a
+     * stored credential is still at rest today, since the save door refuses
+     * one — written straight to the store, stamped as `put` stamps a row.
+     */
+    const seed = async (state: 'active' | 'draft', packageId: string | null, body: Record<string, unknown>) => {
+        const now = new Date().toISOString();
+        await engine.insert('sys_metadata', {
+            id: `seed_${state}_${packageId ?? 'unbound'}`, type: item.type, name: item.name, organization_id: null,
+            package_id: packageId, state, metadata: JSON.stringify(body), checksum: hashSpec(body, item.type),
+            version: 1, created_at: now, updated_at: now,
+        }, { context: { isSystem: true } } as any);
+    };
 
-    return { read, save, stored };
+    return { read, save, stored, storedBodies, seed };
 }
 
 const ok = (res: any) => {
@@ -267,3 +290,63 @@ describe('[#22128] `?package=all`: the read resolves `all` the way the save does
         expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]);
     }, 60_000);
 });
+
+/**
+ * [#22128] The credential carry-forward (#8154) compares the served body
+ * against the row the save OVERWRITES. It read at the key the caller named, so
+ * a package-less draft save of a package-owned item found neither the
+ * inherited draft nor the package-bound active row, compared against the code
+ * layer, and persisted the stored credential away. Measured on the real route
+ * before the fix: the stored draft's `config.url` no longer held it.
+ */
+describe('[#22128] the credential carry-forward reads the row the draft save overwrites', () => {
+    /** A fixture value standing in for the userinfo password a legacy row holds. */
+    const URL_CREDENTIAL = 'fixture-not-a-credential';
+    const DATASOURCE = { type: 'datasource', name: 'warehouse' };
+    const legacyDatasource = (label: string) => ({
+        name: 'warehouse',
+        label,
+        driver: 'postgres',
+        config: {
+            host: 'db.internal',
+            port: 5432,
+            database: 'warehouse',
+            username: 'reporting',
+            url: `postgresql://reporting:${URL_CREDENTIAL}@db.internal:5432/warehouse`,
+        },
+    });
+    const holdsCredential = (body: Record<string, any>) => String(body?.config?.url ?? '').includes(URL_CREDENTIAL);
+    const bootDatasource = () => boot({ item: DATASOURCE, permissions: ['manage_metadata', 'manage_platform_settings'] });
+
+    it('package-owned: the redacted read saved back as a package-less draft, twice, keeps the stored credential', async () => {
+        const h = await bootDatasource();
+        await h.seed('active', PKG, legacyDatasource('live'));
+
+        const served = ok(await h.read()).item;
+        expect(holdsCredential(served)).toBe(false);
+        ok(await h.save({ ...served, label: 'draft 1' }, { mode: 'draft' }));
+        const first = await h.storedBodies('draft');
+        expect(first.map((r) => r.packageId)).toEqual([PKG]);
+        expect(first.map((r) => holdsCredential(r.body))).toEqual([true]);
+
+        const servedDraft = ok(await h.read({ state: 'draft' })).item;
+        expect(holdsCredential(servedDraft)).toBe(false);
+        ok(await h.save({ ...servedDraft, label: 'draft 2' }, { mode: 'draft' }));
+        const second = await h.storedBodies('draft');
+        expect(second.map((r) => [r.body.label, r.packageId, holdsCredential(r.body)])).toEqual([['draft 2', PKG, true]]);
+        // The active row is untouched.
+        expect((await h.storedBodies('active')).map((r) => holdsCredential(r.body))).toEqual([true]);
+    }, 60_000);
+
+    it('control: env-local, the same round trip keeps the stored credential, as before', async () => {
+        const h = await bootDatasource();
+        await h.seed('active', null, legacyDatasource('live'));
+
+        const served = ok(await h.read()).item;
+        ok(await h.save({ ...served, label: 'draft 1' }, { mode: 'draft' }));
+        const servedDraft = ok(await h.read({ state: 'draft' })).item;
+        ok(await h.save({ ...servedDraft, label: 'draft 2' }, { mode: 'draft' }));
+        const drafts = await h.storedBodies('draft');
+        expect(drafts.map((r) => [r.body.label, r.packageId, holdsCredential(r.body)])).toEqual([['draft 2', null, true]]);
+    }, 60_000);
+});