Skip to content

Commit e66e309

Browse files
fix(service-datasource): migrateCredential's drop-inline refusal names an empty stored credential as empty (#29192)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9VrDSYbFeBRpZi2Rbi1iw6B)_ Fixes #29069 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:数据源凭据迁移(drop-inline)遇到已绑定的 sys_secret 解密出来是空值时,拒绝理由现在说「凭据为空」,用的是 #29064 连接路径同一句话;真的找不到行或密钥变了时,原来那句不变。 - **为什么改**:以前两种情况共用一句「sys_secret 行丢失或加密密钥变了」,空凭据的管理员会去查错地方。 - **风险与回滚**:极低。只改拒绝文案;结果仍是 `refused`、什么都不写、内联凭据保留。回滚即 revert。 - **先看**:`packages/services/service-datasource/src/datasource-admin-service.ts` 的 `migrateCredential` 里新拆出的 `resolved === ''` 分支。 - **两分钟验证**:`pnpm --filter @objectstack/service-datasource exec vitest run src/__tests__/datasource-admin-service.test.ts`,预期 `Tests 39 passed (39)`(main 上新用例红)。 - **需要你做的**:无,席位落地。 ## What changes - `migrateCredential`'s drop-inline step (`datasource-admin-service.ts`) splits its refusal. A ref that resolves to `''` is refused with: "Datasource 'NAME' already references a stored secret, but credential 'REF' is empty (its sys_secret row holds an empty value). Removing the inline credential would leave this datasource with no working credential at all." The middle clause is the connect path's sentence from PR #29064 (`DatasourceConnectionService.connect`, the `secret === ''` branch), byte for byte. The remedy says to set the datasource's secret again. - A ref that does not resolve (`undefined`: a missing row or a decrypt failure) keeps its sentence and remedy unchanged. - Same outcome on both branches: `status: 'refused'`, no write, the inline copy kept. No new error code, status or member; the result shape is unchanged. - Changeset `.changeset/29069-migrate-credential-empty-refusal.md`: `patch` on `@objectstack/service-datasource`. ## Tests `src/__tests__/datasource-admin-service.test.ts`, `migrateCredential (#8155)` block: - **Forced test edit:** the existing "cannot be resolved" case moved from `toContain('could not be resolved')` to an exact pin of the missing-row sentence, plus its remedy, so the unchanged text is held (the card asks a test to pin both texts). - **New:** a ref whose read-back is `''` is refused with the empty-credential sentence exactly, the reason names neither "could not be resolved" nor "encryption key", the remedy says to set the secret again, the inline password stays and the only store call is the read. <details><summary>Red on base, green on head, ablation</summary> - **Red** (tests at head, source at base `fea2dbb7`): `Tests 1 failed | 38 passed (39)`; the new case received the missing-row sentence. The missing-row pin was green on base, as it should be. - **Green** (head `b213c369`): `Tests 39 passed (39)`. - **Ablation** (`scripts/ablation-replace.mjs`, the whole `resolved === ''` branch replaced by the old conflated condition `if (resolved == null || resolved === '') {`, anchor 1 -> 0, blob `144b0a89` -> `bcdb23c3`): `Tests 1 failed | 38 passed (39)`; Expected the empty sentence, Received "…but it could not be resolved or decrypted (a missing `sys_secret` row, or a changed encryption key)…". Restore: blob after restore `144b0a89` equals HEAD's, `git diff HEAD` empty. </details> ## Acceptance notes - The empty-credential clause is spelled in two places now (the connect path and this refusal). The claim's file surface is the admin service only, so no shared helper was extracted into `datasource-connection-service.ts`; each spelling is pinned by its own test. ## Verification All readings at head `b213c369`. <details><summary>Package suite, typecheck, lint</summary> - `pnpm --filter @objectstack/service-datasource test`: `Test Files 69 passed (69)`, `Tests 1153 passed (1153)`, exit 0. - `pnpm --filter @objectstack/service-datasource typecheck`: exit 0. - eslint `--no-inline-config` on the two changed source files: exit 0. Repo-wide `pnpm lint` is CI's. </details> <details><summary>Derived gates (dispatch-gates.mjs --commands, 61 families)</summary> - 60 of 61 run locally, all exit 0. `check-plugin-teardown-shape --self-test` first exited 3 on a shallow clone; after `git fetch --unshallow` it passed (48 cases). - `pnpm check:dual-build-cjs-loads` needs every package built; the full build runs and the reading goes in the os-dev-report, otherwise CI's check-run is the reading. </details> Dogfood battery: not owed (no `!` in the title, no `(narrowing)` arm). --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmP66rDL21VeBrVWovN)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent f6a04e3 commit e66e309

3 files changed

Lines changed: 57 additions & 2 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/service-datasource': patch
3+
---
4+
5+
fix(service-datasource): the credential migration's drop-inline refusal names an empty stored credential as empty
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
When a datasource already references a stored secret that decrypts to an empty value (a row sealed empty before the admin doors refused an empty `secret`), the credential migration still refuses to drop the inline copy, with the same `refused` status, but its reason now says the credential is empty, in the connect path's wording, instead of blaming a missing `sys_secret` row or a changed encryption key. A missing row or a decrypt failure keeps its sentence.

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,11 +631,41 @@ describe('migrateCredential (#8155)', () => {
631631
const result = await h.service.migrateCredential('warehouse');
632632

633633
expect(result.status).toBe('refused');
634-
expect(result.reason).toContain('could not be resolved');
634+
expect(result.reason).toBe(
635+
"Datasource 'warehouse' already references a stored secret, but it could not be resolved or "
636+
+ 'decrypted (a missing `sys_secret` row, or a changed encryption key). Removing the inline '
637+
+ 'credential would leave this datasource with no working credential at all.',
638+
);
639+
expect(result.remedy).toContain('replaces the unresolvable reference');
635640
// The inline copy is the only working credential left — it stays.
636641
expect(h.store.get('warehouse')!.config?.password).toBe('hunter2');
637642
});
638643

644+
it('refuses to drop the inline copy when the existing ref decrypts to an empty value, and says it is empty', async () => {
645+
// A row sealed empty before the admin doors refused an empty `secret`
646+
// resolves to ''. The outcome is the same refusal; the cause is named as
647+
// an empty credential, in the connect path's wording, not blamed on a
648+
// missing row or a changed key.
649+
const h = makeHarness({
650+
seed: [legacyRow({ external: { credentialsRef: 'sys_secret://sealed-empty' } })],
651+
readBackOverride: async () => '',
652+
});
653+
const result = await h.service.migrateCredential('warehouse');
654+
655+
expect(result.status).toBe('refused');
656+
expect(result.reason).toBe(
657+
"Datasource 'warehouse' already references a stored secret, but credential "
658+
+ "'sys_secret://sealed-empty' is empty (its sys_secret row holds an empty value). Removing the "
659+
+ 'inline credential would leave this datasource with no working credential at all.',
660+
);
661+
expect(result.reason).not.toContain('could not be resolved');
662+
expect(result.reason).not.toContain('encryption key');
663+
expect(result.remedy).toContain("set the datasource's secret again");
664+
// Same outcome as the missing-row branch: nothing written, inline copy kept.
665+
expect(h.store.get('warehouse')!.config?.password).toBe('hunter2');
666+
expect(h.ops).toEqual(['readSecret']);
667+
});
668+
639669
it('refuses when the host has no readable secret store', async () => {
640670
const h = makeHarness({ seed: [legacyRow()], noReadSecret: true });
641671
const result = await h.service.migrateCredential('warehouse');

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1176,7 +1176,22 @@ export class DatasourceAdminService implements IDatasourceAdminService {
11761176
// inline copy is the last cleartext copy, so it is dropped only once the
11771177
// existing ref is proved resolvable.
11781178
const resolved = await this.tryReadSecret(plan.credentialsRef);
1179-
if (resolved == null || resolved === '') {
1179+
// [#29069] An empty credential is told apart from one that is gone, in
1180+
// the connect path's wording (`DatasourceConnectionService.connect`'s
1181+
// `secret === ''` branch): a row sealed empty before the admin doors
1182+
// refused an empty `secret` decrypts to ''. The outcome is the same
1183+
// refusal — dropping the inline copy would leave nothing that works.
1184+
if (resolved === '') {
1185+
return this.refuse(
1186+
name,
1187+
`Datasource '${name}' already references a stored secret, but credential `
1188+
+ `'${plan.credentialsRef}' is empty (its sys_secret row holds an empty value). Removing the `
1189+
+ 'inline credential would leave this datasource with no working credential at all.',
1190+
'Re-enter the credential in the connection form\'s secret field to set the datasource\'s '
1191+
+ 'secret again — that rebinds it and replaces the empty one; the inline copy can then be removed.',
1192+
);
1193+
}
1194+
if (resolved == null) {
11801195
return this.refuse(
11811196
name,
11821197
`Datasource '${name}' already references a stored secret, but it could not be resolved or `

0 commit comments

Comments
 (0)