Repository navigation
fix(service-datasource)!: the datasource admin doors refuse an empty secret instead of sealing it over the working credential - #29064
Conversation
…secret instead of sealing it over the working credential POST /api/v1/datasources and PATCH /api/v1/datasources/:name took "secret": "" as a value: a create sealed an empty credential, and an edit minted a new sys_secret holding nothing, repointed credentialsRef at it and removed the working row. Both now refuse it 400 DATASOURCE_ADMIN_ERROR naming `secret` before anything is written. null, an absent secret and a whitespace-only value are unchanged. A row sealed empty before still fails closed at connect; its status reason now says the credential is empty instead of blaming a missing row or a changed encryption key. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FEkQ5mTYb1BTJNRqf3bdJ8
📓 Docs Drift Check4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 3 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f5e5b7c61b68cf2c561265913a3c495249e60a3e && git checkout f5e5b7c61b68cf2c561265913a3c495249e60a3e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c708de67b2c00c095afbf5fb36c6c093dbe85458 66a746c6f45bcac47f811b0408ad2e341306d7fa && git checkout -B drift-repro c708de67b2c00c095afbf5fb36c6c093dbe85458 && git merge --no-ff 66a746c6f45bcac47f811b0408ad2e341306d7fa
node scripts/docs-audit/affected-docs.mjs --json c708de67b2c00c095afbf5fb36c6c093dbe85458 |
|
domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC Contract-review-claim: Taken by the dispatching seat. Face Generated by Claude Code |
|
domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC Contract reviewServed-tier: Rendered by the seat's subagent, read-only, at 2026-11-02T06:49Z. Inputs: card #28928 (body + 3 comments: triage 6569553065, claim 6569714758, os-dev-report 6570874270), PR #29064 (body, 5-file list, Check-runs on the head at review time (45): completed success — Auto Label, Test UI, Dogfood Regression Gate, Plan Test Core shards, Lint · scoped · query-options erasure, Type Check · source gates, Lint · PM dispatch-gates, Lint · scoped · tooling self-tests, filter, Check Changeset, Governed Surface Queue Guard, Lint · ESLint, Check PR Size, Flag docs affected by code changes, PR-body guards. Skipped (path-filtered): the UI/docs/dogfood-CLI/temporal/opt-in families. Still in progress: Test Core (1/8 … 8/8), Build Core, Live E2E UI, Type Check · workspace, Type Check · debt ledger, Type Check · consumer gates, Lint · scoped · slot lookup & corpus, Lint · docs & code guards, Lint · repo & tooling gates, Lint · CI, release & engine gates. No failure or cancellation on the head. The seat lands only once the in-progress families conclude. ① Derived judgmentsEach accept-set / public-surface change the diff implies, read in the tree:
② Semver level
③ Boundary flags
④ Design judgmentThe narrowing read against the published spec on
Must-fix: none. Implemented-by: VERDICT: PASS Generated by Claude Code |
|
domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC 席位核对 · ACCEPT ·
|
| item | reading |
|---|---|
| Shape | Title carries !; base main, Fixes #28928. 5 files (+265 / −1): the empty-secret refusal in datasource-admin-service.ts beside the doors' undeclared-key refusals, the connect path's empty-credential status reason in datasource-connection-service.ts, two test files and a @objectstack/service-datasource patch changeset (BREAKING banner, ADR-0087 not-required marker). Metadata-protocol: none, which the review judged right: secret is the doors' own channel, not a metadata key. No packages/spec/**, no gate, no new error code. |
| Rule | Create and update refuse "secret": "" (and {value: ""}) 400 DATASOURCE_ADMIN_ERROR naming secret, before any read or write: no new sys_secret row, no repoint, no delete. null and an absent secret keep the stored credential; a whitespace-only secret is a usable value as sent and is sealed, per the seat's ruling. A stored credential that decrypts to empty is now named as empty in the status reason. |
| Review | No must-fix. Its one should-fix is the dev's own out-of-scope finding, carried by this seat as #29069 (migrateCredential's drop-inline refusal text). Its two observations (an empty secret on the test door, malformed secret shapes failing closed at encrypt) are pre-existing and outside this narrowing. |
| Evidence | 4 pins red on base, 5 controls green both ways, head 111/111; ablation red, restore clean. service-datasource 1152/1152, typecheck, scoped lint, 62/62 derived gates, type-check-debt OK, dogfood battery 383 files at the head. CI on head is still running; auto-merge waits for it. |
| Landing | 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay, with every check green or an expected skip. |
Generated by Claude Code
Requested by Leehom · project thread
Fixes #28928
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no file under
skills/**orcontent/docs/**teaches what an emptysecretdoes at the datasource admin doors. The fail-closed paragraph inexternal-datasources.mdx(§4 Credentials) stays true: an empty stored credential still fails closed at connect.BREAKING accept-set narrowing at the datasource admin doors (
@objectstack/service-datasource): an emptysecretis refused.维护者速读
POST /api/v1/datasources和PATCH /api/v1/datasources/NAME收到空字符串secret(""或{ "value": "" })时,现在回400 DATASOURCE_ADMIN_ERROR,消息点名secret,任何写入之前就拒绝:不新建 sys_secret,不改credentialsRef,不删旧行。另外,旧数据里已经存成空值的凭据,连接时的状态原因现在说「凭据为空」,不再误说「sys_secret 行丢失或加密密钥变了」。secret会被当成轮换:新建一个空值的 sys_secret,把引用指过去,再删掉能用的旧行。正在用的连接池照常跑,直到下次连接才坏,而旧凭据已经没了(Datasources: PATCH with an emptysecretreplaces the stored credential with an empty one, and the status reason then blames a missing secret row #28928,QA 记录 QA run · priority:P0 (23/23) · 7fa9873f · 2026-11-02 · 21 PASS / 2 PARTIAL / 0 FAIL / 0 BLOCKED / 0 NOT-RUN #28927)。null、不传secret、真实凭据、全空格值的行为都不变。控制台本来就只在字段有值时才发secret,所以不受影响。回滚即 revert。packages/services/service-datasource/src/datasource-admin-service.ts的assertSecretNotEmpty及它在createDatasource/updateDatasource里的两处调用;datasource-connection-service.ts里新拆出的secret === ''分支。pnpm --filter @objectstack/service-datasource exec vitest run src/__tests__/datasource-empty-secret-refusal.test.ts,预期Tests 8 passed (8)(main 上 3 条拒绝用例红,5 条对照两边都绿)。What changes
assertSecretNotEmpty(heading, secret)(module function indatasource-admin-service.ts) throws when the normalizedsecrethasvalue === ''.createDatasourcecalls it right after the name check, andupdateDatasourceright after the undeclared-key and inline-ssl.keychecks, so it runs before any secret is read, written or removed and before the record is stored. The route answers it as it answers those two checks:400 DATASOURCE_ADMIN_ERROR, messageInvalid datasource 'NAME'.then "Thesecretis empty. Send the credential to store, or leavesecretout (or null) to keep the stored one. Nothing was written." Only the empty string is judged, so the answer never echoes a credential.VALIDATION_ERROR. Their existing refusals for a body (Datasources: POST /api/v1/datasources accepts and silently drops a top-level password or connectionString that os compile refuses, answering 201 #28549 / Datasources: the test door and every undeclared top-level key still bypass the refusal DatasourceSchema gives (the rest of #28549) #28612 undeclared keys, Datasources: the admin create and edit doors drop the schema-declaredssl,autoConnectanddescription, so a datasource created there loses its TLS settings #28633 inlinessl.key) are a plainErrorthrown first thing in the service and answered400 DATASOURCE_ADMIN_ERRORbybadRequest; this reuses that exactly (ruling 3: no new error code). Only these two doors callcreateDatasource/updateDatasource.nulland an absentsecretkeep the stored credential (splitSecretalready mapsnullto absent). A whitespace-only value is not refused: step 1 shows it is sealed andresolve()returns it unchanged, so it is a usable value as sent.POST /api/v1/datasources/teststores nothing and is untouched.resolve()returns''for it.DatasourceConnectionService.connectnow answers that case on its own: stillfailed-credentials, still no driver built, reason "credential 'REF' is empty (its sys_secret row holds an empty value); set the datasource's secret again". A missing row or a decrypt failure (undefined) keeps the old reason..changeset/28928-empty-datasource-secret-refused.md:patchon@objectstack/service-datasource(non-protocol), both declaration lines, BREAKING banner, the remedy, and the ADR-0087 markernot-required (no-migration-prescription).packages/spec/**, no new gate.Why
Clause-②: no (narrowing). One body that answered 201 / 200 now answers 400; no answer member, code or exported type changes.Step 1: measurement on main
Real routes over the real
DatasourceAdminServiceon in-memory stores, basef7c545ad:secret""credentialsRefto it'', repointscredentialsRef, removes the working row" "nullcredentialsRefresolve()returns the sealed string as-is, so""yields''and" "yields three spaces. Callers: none sends an emptysecreton purpose. The console'sDatasourceResourcePage.tsx(objectuiui/packages/app-shell) setsbody.secretonly when the field is truthy, on save and on test; the client SDK has no datasource create/update; hotcrm has no caller.Tests
New
packages/services/service-datasource/src/__tests__/datasource-empty-secret-refusal.test.ts, through the real routes over the real service (the harness ofdatasource-admin-declared-keys-kept.test.ts), recording every seam call:"secret": ""and with{ "value": "" }: 400,DATASOURCE_ADMIN_ERROR, message namessecret; the stored record (itscredentialsRefincluded) equals the seed, no secret written, no seam call at all (no put, no removeSecret, no pool)."secret": "": the same refusal; nothing stored, no secret written, no seam call.secret: nulland an absentsecreton PATCH change no credential;secret: nullon POST creates with no ref.One new case in
datasource-connection-service.test.ts(D3 block): a resolver returning''fails closed with a reason that names the empty credential and contains neither "could not be resolved", "missing sys_secret row" nor "encryption key".Red on base, green on head, ablation
f7c545ad, the tests at head):Tests 4 failed | 107 passed (111)across the two files. The three refusal pins received{ status: 200, success: true }; the status-reason pin received "credential 'sys_secret:abc' could not be resolved or decrypted (missing sys_secret row, or the encryption key changed)". All five controls green.Tests 111 passed (111).scripts/ablation-replace.mjs, the guard line ofassertSecretNotEmptyreplaced byreturn;):Tests 3 failed | 5 passed (8), exactly the three refusal pins. Restore: blob after restore equals HEAD's (1933a09e),git diff HEADempty.Acceptance notes
migrateCredential's drop-inline refusal (datasource-admin-service.ts, theresolved == null || resolved === ''check) still says the existing ref "could not be resolved or decrypted (a missingsys_secretrow, or a changed encryption key)" when the row decrypts to empty. Its outcome (refuse to drop the last inline copy) is right; only the cause text conflates."secret": ""to mean "no change" now gets the 400; the remedy is in the message and the changeset.Verification
All readings at head
66a746c6(one commit onorigin/main00d0668d, after PR #28955 merged). Pins red/green and the ablation were taken before the restart, on basef7c545ad; the restart moved no file this diff touches.Package suite, typecheck, lint
pnpm --filter @objectstack/service-datasource test:Test Files 69 passed (69),Tests 1152 passed (1152), exit 0.pnpm --filter @objectstack/service-datasource typecheck(tsc oversrc, tests included): exit 0.pnpm lintcommand scoped topackages/services/service-datasource: exit 0.Derived gates (`dispatch-gates.mjs --commands`, 62 families)
check-adr-0087-registration --base origin/mainandcheck-changeset-no-majorincluded;dispatch-gates --ran:62 derived, 62 run, 0 NOT-MEASURED. Record:pm-orders/server/28928-gates-ran.txt.check:type-check-debt: OK,--re-measure: OK.pnpm lintand the CI-only families are CI's; see the PR's check-runs.Dogfood battery (owed:
!title and the(narrowing)arm), once at the final head afterpnpm turbo run build --filter=@objectstack/dogfood...(exit 0):pnpm --filter @objectstack/dogfood test,Test Files 383 passed | 3 skipped (386),Tests 3163 passed | 28 skipped (3191), exit 0.Generated by Claude Code