Skip to content

Commit 8f25cee

Browse files
fix(service-datasource)!: a datasource create may not name a stored credential reference; the settings handle door is pinned as it holds (#27761)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DXVCzWzmBkGHAYQZPucaktq)_ Fixes #27722 domain:server#3 · os-elon-musk · session_016WV4cy83ynufEsjN8PwPoh (card thread) ## 维护者速读 - 改了什么:数据源管理入口的「新建」不再接受请求里自带的 `external.credentialsRef`,一律以 400 拒绝,且不写入任何东西;新建记录只保存它自己这次 `secret` 封存后得到的那个引用。设置服务的密文句柄写入口经实测本来就关着(每个值都重新封存、从不收引用),本 PR 只给它加钉。 - 为什么改:卡上要求按 #27582 的同一条规则审两个凭据引用写入口:「非系统写入只能封存新值,或保留本记录自己的引用」。实测数据源新建入口会原样保存请求带来的引用,不符合规则;编辑入口与设置入口符合。 - 风险与回滚:拒绝发生在任何记录、密文或连接池写入之前;无论引用指向什么,答复都相同,消息里不出现引用值。控制台的数据源表单从不发送这个键,不受影响。编辑行为不变。回滚只需 revert 本 PR。 - 先看:`packages/services/service-datasource/src/datasource-admin-service.ts`(`createDatasource` 开头的 `assertNoDraftCredentialsRef`),两个新测试文件。 - 两分钟验证:`pnpm --filter @objectstack/service-datasource exec vitest run src/__tests__/datasource-credentials-ref-own-reference.test.ts`(10/10)与 `pnpm --filter @objectstack/service-settings exec vitest run src/settings-secret-handle-own-reference.pin.test.ts`(2/2)。 - 需要你做的:无需操作;本 PR 是收窄(narrowing),落地前需一次契约复审,由席位安排。 ## What changes The card audits two credential-reference write doors against the rule #27582 set (PR #27695): **a non-system write may only seal a new value or keep its own record's stored reference.** **Door 2, datasource `external.credentialsRef` (fixed).** The admin door's create (`POST /api/v1/datasources`, `IDatasourceAdminService.createDatasource`) now refuses a draft that names `external.credentialsRef`: - A create has no stored reference of its own, so the only reference it may store is the one its own `secret` is bound under. - The refusal is thrown before anything is written (no record, no secret, no pool), as the route's `400`. The message names the key and never its value, and the answer is the same whatever the value names. - Every caller of this door is a signed-in operator through the admin routes; nothing system-side writes through it, so there is no pass-through to keep. - The update path already keeps the record's own reference unless a supplied secret rewraps it. It is pinned here, unchanged. **Door 1, settings-service credential handles (holds, pinned).** `setMany` seals every value it is handed for a declared-encrypted key and stores the handle that seal minted, so no caller can make the stored handle one it did not mint. The pin names a live handle another organization's write minted. The #27695 helper (`SecretRefWriteError`, `encryptSecretFields`) is not shareable here: it lives in an `@objectstack/objectql` internal module that nothing re-exports, this package does not depend on objectql, and its judge reads a record's stored column, which a create does not have. The refusal here is the door's own, in the shape its sibling validation refusals already take. ## Why The card's audit, measured on `origin/main` at 3702be4: door 2's create stored a draft's `external.credentialsRef` as given; door 2's update and door 1 did not let a caller set the stored reference. Only the door that measured open is changed. ## Verification - New pins: `service-datasource` `datasource-credentials-ref-own-reference.test.ts` (10 cases) and `service-settings` `settings-secret-handle-own-reference.pin.test.ts` (2 cases). - Ablations, each restored: removing the create refusal reds the 5 refusal cases; letting an update keep the patch's reference reds the 2 update pins; letting the settings door store a submitted value as the handle reds both settings pins. - Package suites: `service-datasource` 1021/1021, `service-settings` 1016/1016. `pnpm typecheck` in both packages: exit 0. - `node scripts/pm/dispatch-gates.mjs --commands` at the committed head, plus `pnpm lint`: 61 commands derived. 60 exit 0 (`check:engine-double-contract` first flagged the settings pin's secret-store fake as an engine double; the fake was trimmed to the store members the pin uses, and the gate re-runs exit 0). `check:dual-build-cjs-loads` exits 3 (prerequisite: a whole-tree build); it is re-run after `pnpm build` and reported in the report comment. `pnpm lint`: exit 0. Clause-②: no (narrowing) — the datasource admin door's create refuses a draft that names `external.credentialsRef`, where it stored the value as given before. Nothing is widened. Metadata-protocol: none The changeset is `patch` on `@objectstack/service-datasource` with the BREAKING banner and its ADR-0087 disposition. `service-settings` gains a test file only and publishes nothing. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: no. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the maintainer brief excepted). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016WV4cy83ynufEsjN8PwPoh Co-authored-by: Claude <noreply@anthropic.com>
1 parent a0413fc commit 8f25cee

5 files changed

Lines changed: 341 additions & 1 deletion

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/service-datasource': patch
3+
---
4+
5+
fix(service-datasource)!: a datasource create may not name a stored credential reference; it stores only the one its own secret is bound under (#27722)
6+
7+
Clause-②: no (narrowing) — the datasource admin door's create refuses a draft that names `external.credentialsRef`, where it stored the value as given before. Nothing is widened.
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a create draft value the admin door used to store as given is refused. The fix for a caller is to send the credential as `secret` and leave the key out, which the refusal says; there is nothing for a migration to rewrite and no tombstone. -->
11+
12+
**BREAKING** accept-set narrowing on the datasource admin door's create (`POST /api/v1/datasources`, `IDatasourceAdminService.createDatasource`), the same rule #27582 set for `secret` fields: a write may only seal a new value or keep its own record's stored reference.
13+
14+
- **Before.** A create draft's `external.credentialsRef` was stored as given, beside or instead of the reference a supplied `secret` is bound under.
15+
- **Now.** A create has no stored reference of its own, so a draft that names `external.credentialsRef` is refused with `400` before anything is written: no record, no secret, no pool. The message names the key and never the value, and the answer is the same whatever the value names. The only reference a create stores is the one its own `secret` is bound under.
16+
- **Unchanged.** An update keeps the record's own reference unless a supplied `secret` rewraps it, as before. A create with a `secret` and no `external.credentialsRef` binds it as before. The Console's datasource form never sends the key, so it is unaffected.
17+
- **Who sees it.** A caller of the admin door's create that sends `external.credentialsRef`.
18+
19+
**The remedy.** Send the credential as `secret` and leave `external.credentialsRef` out; the door binds it and stores the reference.
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #27722 — the datasource admin door applies the #27582 own-reference rule to
5+
* `external.credentialsRef`: **a write may only seal a new value or keep its
6+
* own record's stored reference.**
7+
*
8+
* The door's callers are the admin routes (`POST` / `PATCH /datasources`), a
9+
* signed-in operator every time; nothing system-side writes through this
10+
* service, so there is no pass-through to keep here.
11+
*
12+
* - **create** — a new record has no reference of its own to keep, so a draft
13+
* that names `external.credentialsRef` is refused, with one answer whatever
14+
* it names, before anything is written. The credential arrives as `secret`
15+
* and the door binds it; that reference is the only one a create stores.
16+
* - **update** — the record keeps the reference it already holds unless a
17+
* supplied `secret` rewraps it. A patch that names any other reference never
18+
* reaches the stored record.
19+
*
20+
* The harness is the service's own config seam, so each case reads exactly
21+
* what the door persisted, bound, removed and registered.
22+
*/
23+
24+
import { describe, expect, it } from 'vitest';
25+
import {
26+
DatasourceAdminService,
27+
type DatasourceAdminServiceConfig,
28+
type StoredDatasource,
29+
} from '../datasource-admin-service.js';
30+
31+
/** A reference some other write minted — the kind a caller must not store. */
32+
const OTHER_REF = 'sys_secret:sec_minted_elsewhere';
33+
34+
function makeHarness(seed: StoredDatasource[] = []) {
35+
const records: StoredDatasource[] = seed.map((r) => ({ ...r }));
36+
const secrets = new Map<string, string>([[OTHER_REF, 'other-credential']]);
37+
const ops: string[] = [];
38+
let seq = 0;
39+
40+
const config: DatasourceAdminServiceConfig = {
41+
probe: async () => ({ ok: true }),
42+
listDatasourceRecords: async () => records.map((r) => ({ ...r })),
43+
getDatasourceRecord: async (name) => {
44+
const r = records.find((x) => x.name === name);
45+
return r ? { ...r } : undefined;
46+
},
47+
putDatasourceRecord: async (record) => {
48+
ops.push(`put:${record.name}`);
49+
const idx = records.findIndex((r) => r.name === record.name);
50+
if (idx >= 0) records[idx] = { ...record };
51+
else records.push({ ...record });
52+
},
53+
deleteDatasourceRecord: async (name) => {
54+
ops.push(`delete:${name}`);
55+
const idx = records.findIndex((r) => r.name === name);
56+
if (idx >= 0) records.splice(idx, 1);
57+
},
58+
writeSecret: async (input) => {
59+
const ref = `sys_secret:sec_minted_${++seq}`;
60+
ops.push(`writeSecret:${ref}`);
61+
secrets.set(ref, input.value);
62+
return ref;
63+
},
64+
removeSecret: async (ref) => {
65+
ops.push(`removeSecret:${ref}`);
66+
secrets.delete(ref);
67+
},
68+
readSecret: async (ref) => secrets.get(ref),
69+
countBoundObjects: async () => 0,
70+
listObjectsBoundTo: async () => [],
71+
registerPool: (record) => {
72+
ops.push(`registerPool:${record.name}`);
73+
},
74+
unregisterPool: (name) => {
75+
ops.push(`unregisterPool:${name}`);
76+
},
77+
};
78+
79+
const service = new DatasourceAdminService(config);
80+
const stored = (name: string) => records.find((r) => r.name === name);
81+
return { service, stored, secrets, ops };
82+
}
83+
84+
const DRAFT = {
85+
name: 'reporting',
86+
driver: 'postgres',
87+
config: { host: 'db.internal', database: 'analytics' },
88+
};
89+
90+
describe('#27722 — a create may not carry a stored credential reference', () => {
91+
it.each([
92+
['a reference another write minted', OTHER_REF],
93+
['a reference that names nothing', 'sys_secret:sec_never_minted'],
94+
['a value in no reference shape', 'not-a-reference'],
95+
])('refuses %s, writing nothing', async (_label, ref) => {
96+
const { service, stored, secrets, ops } = makeHarness();
97+
98+
await expect(
99+
service.createDatasource({ ...DRAFT, external: { credentialsRef: ref } }),
100+
).rejects.toThrow(/external\.credentialsRef/);
101+
102+
expect(stored('reporting')).toBeUndefined();
103+
expect(ops).toEqual([]);
104+
expect(secrets.get(OTHER_REF)).toBe('other-credential');
105+
});
106+
107+
it('answers the same whatever the reference names, and never echoes it', async () => {
108+
const messages: string[] = [];
109+
for (const ref of [OTHER_REF, 'sys_secret:sec_never_minted', 'not-a-reference']) {
110+
const { service } = makeHarness();
111+
const err = await service
112+
.createDatasource({ ...DRAFT, external: { credentialsRef: ref } })
113+
.catch((e: unknown) => e as Error);
114+
expect(err).toBeInstanceOf(Error);
115+
expect((err as Error).message).not.toContain(ref);
116+
messages.push((err as Error).message);
117+
}
118+
expect(new Set(messages).size).toBe(1);
119+
});
120+
121+
it('refuses it beside a supplied secret too: the door mints the only reference a create stores', async () => {
122+
const { service, stored, ops } = makeHarness();
123+
124+
await expect(
125+
service.createDatasource(
126+
{ ...DRAFT, external: { credentialsRef: OTHER_REF } },
127+
{ value: 'fresh-credential' },
128+
),
129+
).rejects.toThrow(/external\.credentialsRef/);
130+
131+
expect(stored('reporting')).toBeUndefined();
132+
expect(ops).toEqual([]);
133+
});
134+
135+
it('stores the reference its own secret was sealed under', async () => {
136+
const { service, stored, secrets } = makeHarness();
137+
138+
await service.createDatasource(DRAFT, { value: 'fresh-credential' });
139+
140+
const ref = stored('reporting')?.external?.credentialsRef;
141+
expect(ref).toMatch(/^sys_secret:sec_minted_/);
142+
expect(ref).not.toBe(OTHER_REF);
143+
expect(secrets.get(ref!)).toBe('fresh-credential');
144+
});
145+
146+
it('still creates a record that names no reference at all', async () => {
147+
const { service, stored } = makeHarness();
148+
149+
await service.createDatasource({ ...DRAFT, schemaMode: 'external', external: { allowWrites: false } });
150+
151+
expect(stored('reporting')).toBeDefined();
152+
expect(stored('reporting')?.external?.credentialsRef).toBeUndefined();
153+
});
154+
});
155+
156+
describe('#27722 — an update keeps its own reference and stores no other', () => {
157+
const OWN_REF = 'sys_secret:sec_own';
158+
159+
it('keeps the record\'s own reference when a patch names another', async () => {
160+
const { service, stored, secrets, ops } = makeHarness([
161+
{ ...DRAFT, origin: 'runtime', external: { credentialsRef: OWN_REF } },
162+
]);
163+
164+
await service.updateDatasource('reporting', { external: { credentialsRef: OTHER_REF } });
165+
166+
expect(stored('reporting')?.external?.credentialsRef).toBe(OWN_REF);
167+
expect(ops).not.toContain(`removeSecret:${OTHER_REF}`);
168+
expect(secrets.get(OTHER_REF)).toBe('other-credential');
169+
});
170+
171+
it('stores no reference on a record that holds none when a patch names one', async () => {
172+
const { service, stored } = makeHarness([{ ...DRAFT, origin: 'runtime' }]);
173+
174+
await service.updateDatasource('reporting', { external: { credentialsRef: OTHER_REF } });
175+
176+
expect(stored('reporting')?.external?.credentialsRef).toBeUndefined();
177+
});
178+
179+
it('replaces its own reference only with the one a supplied secret is sealed under', async () => {
180+
const { service, stored, secrets, ops } = makeHarness([
181+
{ ...DRAFT, origin: 'runtime', external: { credentialsRef: OWN_REF } },
182+
]);
183+
184+
await service.updateDatasource(
185+
'reporting',
186+
{ external: { credentialsRef: OTHER_REF } },
187+
{ value: 'rotated-credential' },
188+
);
189+
190+
const ref = stored('reporting')?.external?.credentialsRef;
191+
expect(ref).toMatch(/^sys_secret:sec_minted_/);
192+
expect(secrets.get(ref!)).toBe('rotated-credential');
193+
expect(ops).toContain(`removeSecret:${OWN_REF}`);
194+
expect(ops).not.toContain(`removeSecret:${OTHER_REF}`);
195+
expect(secrets.get(OTHER_REF)).toBe('other-credential');
196+
});
197+
});

‎packages/services/service-datasource/src/contracts/datasource-admin-service.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,9 @@ export interface IDatasourceAdminService {
160160

161161
/**
162162
* Persist a new runtime datasource (`origin: 'runtime'`, environment-scoped).
163-
* Rejects when a code-defined datasource of the same name exists.
163+
* Rejects when a code-defined datasource of the same name exists, and rejects
164+
* a draft that names `external.credentialsRef`: the only reference a create
165+
* stores is the one a supplied `secret` is bound under.
164166
*/
165167
createDatasource(input: DatasourceDraft, secret?: SecretInput): Promise<DatasourceSummary>;
166168

‎packages/services/service-datasource/src/datasource-admin-service.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,7 @@ export class DatasourceAdminService implements IDatasourceAdminService {
649649

650650
async createDatasource(input: DatasourceDraft, secret?: SecretInput): Promise<DatasourceSummary> {
651651
this.assertValidName(input?.name);
652+
this.assertNoDraftCredentialsRef(input);
652653
if (!input.driver) throw new Error('A driver is required to create a datasource.');
653654
this.assertValidConfig(input.driver, input.config);
654655
// The wizard is a publish door for `pool` too (#5714). Rejected BEFORE the
@@ -1175,6 +1176,29 @@ export class DatasourceAdminService implements IDatasourceAdminService {
11751176
}
11761177
}
11771178

1179+
/**
1180+
* [#27722] The #27582 rule at this door: a write may only seal a new value or
1181+
* keep its own record's stored reference. A create has no record of its own,
1182+
* so the only `external.credentialsRef` it stores is the one a supplied
1183+
* `secret` is bound under, and a draft that names one is refused. One answer
1184+
* whatever it names: the message names the key and never its value, and
1185+
* nothing about the reference is looked up. Thrown before anything is
1186+
* written (a `400` at the route).
1187+
*
1188+
* An update needs no such refusal: it keeps the stored reference unless a
1189+
* supplied secret rewraps it, whatever the patch's `external` says.
1190+
*/
1191+
private assertNoDraftCredentialsRef(input: DatasourceDraft): void {
1192+
const external = input.external as { credentialsRef?: unknown } | undefined;
1193+
if (external?.credentialsRef === undefined) return;
1194+
throw new Error(
1195+
`Invalid datasource '${input.name}': \`external.credentialsRef\` is not accepted on create. `
1196+
+ 'It names a credential already stored, and only the reference this door binds a supplied `secret` '
1197+
+ 'under can be stored on a new datasource. Send the credential as `secret`, and leave '
1198+
+ '`external.credentialsRef` out. Nothing was written.',
1199+
);
1200+
}
1201+
11781202
private toRecord(input: DatasourceDraft): StoredDatasource {
11791203
return {
11801204
name: input.name,
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #27722 — the settings write door against the #27582 own-reference rule:
5+
* **a write may only seal a new value or keep its own record's stored
6+
* reference.**
7+
*
8+
* Measured, not changed: this door never takes a reference from its caller at
9+
* all. For a declared-encrypted key, `setMany` seals whatever value it is
10+
* handed and stores the handle that seal minted; the submitted value is never
11+
* the stored handle, whatever shape it has. These pins hold that answer, so a
12+
* later change that let a caller-supplied value stand in for the stored handle
13+
* reds here.
14+
*
15+
* Each case names a live handle another organization's write minted, so the
16+
* pin measures a real stored handle rather than one that names nothing.
17+
*
18+
* The writer is an enforced caller (the settings routes' context) holding the
19+
* namespace's write capability. The in-process caller takes the same path:
20+
* the door has no reference-bearing branch for any caller.
21+
*/
22+
23+
import { randomBytes } from 'node:crypto';
24+
import { describe, expect, it } from 'vitest';
25+
import type { SettingsManifest } from '@objectstack/spec/system';
26+
import { SettingsService } from './settings-service.js';
27+
import { LocalCryptoProvider } from './local-crypto-provider.js';
28+
import type { SettingsContext, SettingsSecretStore } from './settings-service.types.js';
29+
30+
const manifest: SettingsManifest = {
31+
namespace: 'sms',
32+
version: 1,
33+
label: 'SMS',
34+
scope: 'tenant',
35+
readPermission: 'setup.access',
36+
writePermission: 'setup.write',
37+
specifiers: [
38+
{ type: 'password', key: 'auth_token', label: 'Auth token', required: false, encrypted: true },
39+
],
40+
};
41+
42+
const asOrg = (tenantId: string): SettingsContext => ({
43+
tenantId,
44+
userId: `admin_${tenantId}`,
45+
enforced: true,
46+
permissions: ['setup.access', 'setup.write'],
47+
});
48+
49+
function boot() {
50+
const rows = new Map<string, Record<string, unknown>>();
51+
const secretStore: SettingsSecretStore = {
52+
async insert(row) { rows.set(row.id, { ...row }); return { id: row.id }; },
53+
async get(id) { return (rows.get(id) as any) ?? null; },
54+
async update(id, patch) { rows.set(id, { ...rows.get(id), ...patch }); },
55+
};
56+
const svc = new SettingsService({
57+
env: {},
58+
cryptoProvider: new LocalCryptoProvider({ key: randomBytes(32) }),
59+
secretStore,
60+
});
61+
svc.registerManifest(manifest);
62+
return { svc, rows };
63+
}
64+
65+
/** Org A holds a live secret; returns the handle its write minted. */
66+
async function seedOtherOrganization(svc: SettingsService, rows: Map<string, unknown>) {
67+
const before = new Set(rows.keys());
68+
await svc.set('sms', 'auth_token', 'org-a-credential', asOrg('org_a'));
69+
const minted = [...rows.keys()].filter((id) => !before.has(id));
70+
expect(minted).toHaveLength(1);
71+
return minted[0];
72+
}
73+
74+
describe('#27722 — the settings write door stores only a handle its own write minted', () => {
75+
it.each([
76+
['the bare handle', (h: string) => h],
77+
['a `secret:` reference to it', (h: string) => `secret:${h}`],
78+
])('seals %s as a new value rather than storing it as the handle', async (_label, spell) => {
79+
const { svc, rows } = boot();
80+
const foreign = await seedOtherOrganization(svc, rows);
81+
const foreignRow = { ...(rows.get(foreign) as Record<string, unknown>) };
82+
const submitted = spell(foreign);
83+
84+
const before = new Set(rows.keys());
85+
await svc.set('sms', 'auth_token', submitted, asOrg('org_b'));
86+
const minted = [...rows.keys()].filter((id) => !before.has(id));
87+
88+
// One new seal, under a handle that is not the submitted one.
89+
expect(minted).toHaveLength(1);
90+
expect(minted[0]).not.toBe(foreign);
91+
// Org B reads back exactly what it wrote — never the credential the
92+
// named handle holds.
93+
expect((await svc.get<string>('sms', 'auth_token', asOrg('org_b'))).value).toBe(submitted);
94+
// The other organization's handle and its value are untouched.
95+
expect(rows.get(foreign)).toEqual(foreignRow);
96+
expect((await svc.get<string>('sms', 'auth_token', asOrg('org_a'))).value).toBe('org-a-credential');
97+
});
98+
});

0 commit comments

Comments
 (0)