Repository navigation
Commit 819b101
fix(service-datasource)!: refuse every top-level key DatasourceSchema refuses at the datasource test, create and update doors (#28632)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X1Tmz1BBW7x71pyEcvjiBAW)_
Fixes #28612
Clause-②: no (narrowing) — three doors refuse top-level keys they
silently dropped: `POST /api/v1/datasources/test` (a top-level
`password` / `connectionString`, and any other key `DatasourceSchema`
refuses), `POST /api/v1/datasources` and `PATCH
/api/v1/datasources/NAME` (every key the schema refuses, beyond the two
credential keys PR #28607 closed). Each now answers `400
DATASOURCE_ADMIN_ERROR` before anything is read, written or probed.
Nothing is widened.
Metadata-protocol: none
Skills-docs: none — no page under `skills/**` or `content/docs` teaches
the admin request body; the refusal text is `DatasourceSchema`'s own,
which already ships with the spec and does not move.
**BREAKING** accept-set narrowing at the datasource admin doors
(`@objectstack/service-datasource`).
**Built on PR #28607** (same file, landed as `f3d84b33`): this branch
started from its head and has merged `origin/main` since, so the diff is
only this change.
## 维护者速读
- **改了什么**:数据源管理接口的三个入口(连接测试 `POST /api/v1/datasources/test`、创建 `POST
/api/v1/datasources`、编辑 `PATCH /api/v1/datasources/NAME`)现在会拒收请求体顶层任何
`DatasourceSchema` 不认识的键:400
`DATASOURCE_ADMIN_ERROR`,点名这个键,不回显值,什么都不写入、也不发起探测。哪些键算"不认识"、提示怎么说,都在运行时直接取自
`DatasourceSchema` 自己的报错(即 `os compile` 打印的那段),服务里没有手写清单。
- **为什么改**:卡 #28612(#28607 合约评审 6555221422 的两条 should-fix):连接测试会悄悄丢掉顶层
`password`,测的其实是另一个连接;三个入口都会悄悄丢掉
`url`、`host`、`type`、`readReplicas`、拼错的键等,接口照样答成功。
- **风险与回滚**:schema 声明过的键(`origin`、`description`、`ssl`、`autoConnect`
等)照常通过;`secret` 由路由先拆走,仍是唯一的凭据通道。控制台、`packages/client`、CLI、QA 清单与
dogfood 都不发被拒的键。唯一可能受影响的是把 `GET` 的应答(含 `hasSecret`、`redactedConfigKeys`
等)原样 PATCH 回来的客户端,仓内实测没有这样的生产者。回滚即 revert。
-
**先看**:`packages/services/service-datasource/src/datasource-admin-service.ts`(`assertNoUndeclaredTopLevelKey`
与 `testConnection`
开头的新调用);`packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts`(#28612
的三个 describe);`.changeset/28612-datasource-undeclared-top-level-key.md`。
- **两分钟验证**:`pnpm --filter @objectstack/service-datasource exec vitest
run src/__tests__/datasource-admin-top-level-credential.test.ts` → 25/25
通过。
- **需要你做的**:无,席位落地(收窄需先过合约评审)。
## What changes
**One check, three doors**
(`packages/services/service-datasource/src/datasource-admin-service.ts`).
PR #28607's private `assertNoTopLevelCredential(name, body)` and its
hand list `TOP_LEVEL_CREDENTIAL_KEYS` become
`assertNoUndeclaredTopLevelKey(heading, body)`, with no list:
- It takes the body's own top-level key names (those whose value is not
`undefined`), gives each the value `null`, parses that skeleton through
`DatasourceSchema`, and keeps only the strict shape's root
`unrecognized_keys` refusal. So the key set is exactly what the schema
refuses at run time, and the message is the schema's own (its `guidance`
rows for `password`, `connectionString`, `url`, `host`, `connector`, the
retired blocks, its alias table for `type` and the like, and its generic
unknown-key text for anything else). Values never reach the parse, so
the answer cannot echo a credential.
- `testConnection` (the unsaved-draft probe): first statement, before
the driver requirement, the config judgement, the record judgement and
the probe. It throws, so the route answers `400 DATASOURCE_ADMIN_ERROR`
(the card asks for that code), under the heading this door already gives
a draft it refuses: `This connection draft is not a valid datasource.`
- `createDatasource` and `updateDatasource`: the same position as
#28607's call (create: after the name rule and the draft
`credentialsRef` refusal; update: after the 404 and the code-defined
refusal, before the merge, on every patch including a
`label`/`active`-only one), under the heading `Invalid datasource
'NAME'.`
**Door-only fields.** The service's `DatasourceDraft` declares `name`,
`label`, `driver`, `schemaMode`, `config`, `external`, `pool`, `active`:
all declared by the schema. The one field the doors accept on purpose
that the schema does not carry is **`secret`**, the credential channel:
the route's `splitSecret` removes it before the service sees the draft,
so it never reaches the check (controls below, on all three doors). No
other door-only field exists.
**Keys the schema declares pass.** `origin`, `description`, `ssl`,
`autoConnect`, `name` on an edit, and the metadata protection fields are
declared, so the key check lets them through even where the projection
leaves them out (`origin` is still stamped `runtime` by the door). That
is the card's scope: refuse what the schema refuses.
## Tests
Three new `describe` blocks in
`src/__tests__/datasource-admin-top-level-credential.test.ts`, same
harness as #28607 (real routes over `HonoHttpServer`, real
`DatasourceAdminService` on in-memory stores,
`entitled-caller.fixture`). The probe seam now records `probe` in the op
log, so "nothing probed" is read, not assumed.
- **Test door refusals** (status 400 + `error.code`
`DATASOURCE_ADMIN_ERROR` + `success: false`, exact message = heading +
`DatasourceSchema`'s refusal derived in the test, value absent, op log
empty so no probe ran): top-level `password`, `connectionString` (URL
userinfo), `foo`, and `password` beside a `secret`.
- **Create refusals** (same envelope, exact message, nothing stored or
minted): `foo`, `url` (URL userinfo), `type` (the schema's alias row),
`readReplicas` (a retired block), and `foo` + `host` + `password` in one
answer.
- **Edit refusals** (same envelope, exact message, stored record
unchanged): `foo` with a `config`, `foo` on a `label`-only edit,
`hasSecret` (a `GET` answer key) on a `label`-only edit.
- **Controls**: the test door still probes a valid draft whose
credential arrives as `secret` (200, `ok: true`, one probe); create with
`description` and `origin` (declared) answers 201 and stores `origin:
'runtime'`; an edit carrying `name` and `origin` answers 200. #28607's
controls (secret-bound create 201, edit keeping its bound ref 200,
`config.password` refusal) stay green.
<details><summary>Red first, green, ablation</summary>
- **Red first** (the new cases run against PR #28607's head `0cc981aa`,
service unchanged): `Tests 12 failed | 13 passed (25)`. The test door
answered `expected { status: 200, success: true, … } to deeply equal {
status: 400, success: false, … }` (the probe ran without the key),
create answered `status: 201`, edit answered `status: 200`. All
controls, #28607's included, were green.
- **Green** at the fix: `Tests 25 passed (25)`.
- **Ablation**, one door at a time (`node scripts/ablation-replace.mjs`,
each call to `assertNoUndeclaredTopLevelKey` replaced by `void input;` /
`void patch;`; the test imports the service from `src/`, so no build
sits between the mutation and the run):
- test door: anchor 1 → 0, blob `30943306cc1c` → `ef800604f535`; `Tests
4 failed | 21 passed`, exactly the four test-door refusals.
- create: anchor 1 → 0, blob `30943306cc1c` → `32f0578239a4`; `Tests 9
failed | 16 passed`, the four #28549 POST refusals and the five #28612
POST refusals.
- update: anchor 1 → 0, blob `30943306cc1c` → `3c4b56a5464b`; `Tests 6
failed | 19 passed`, the three #28549 PATCH refusals and the three
#28612 PATCH refusals.
- each restore: blob after restore equals HEAD (`30943306cc1c`), `git
diff HEAD` empty.
</details>
## Acceptance notes
- **The `GET` round-trip keys are refused, not tolerated.** #28607's
contract review noted that a general closed body "must tolerate the GET
round-trip keys `origin`, `hasSecret`, `redactedConfigKeys`,
`definedIn`". Measured: `origin` is declared by the schema and passes.
The other three (and the list row's `status` / `statusReason`) are not
declared, so the card's item 2 ("every top-level key `DatasourceSchema`
refuses") refuses them, and no producer in the repo sends them back: the
Console editor's `draftBody` builds a fresh body (`label`, `driver`,
`schemaMode`, `config`, `external`, `secret`) and its test call sends
`driver`, `config`, `secret`; `packages/client` has no
create/update/test method; the CLI datasource commands are read-only;
the QA checklist bodies and the dogfood suites send declared keys only.
Tolerating them would be a consumer-side allowance for keys nothing
writes. The changeset's remedy line says not to send them back.
- **Keys the schema declares that the doors still drop.** `description`,
`ssl`, `autoConnect` (and the protection fields) pass the key check and
are then left out by `toRecord` / the update merge, as before. They are
accepted by the schema, so they are outside this card ("refuse what the
schema refuses"); reported to the seat as an out-of-scope finding.
- **Producers measured** (order's stop-and-report check): none sends a
top-level key the schema refuses. Searched:
`ui/packages/app-shell/src/views/metadata-admin/datasource/`,
`packages/client/src`, `packages/cli/src`, `packages/qa/dogfood/test`,
`docs/qa/platform-checklist/areas`, `examples/`, `apps/`, `skills/`,
`content/docs`.
- **Test door answer shape.** The probe's existing refusals (no driver,
bad config, invalid record) answer `200` with `result.ok: false`. This
one answers `400 DATASOURCE_ADMIN_ERROR`, as the card's item 1 asks
("the same `DATASOURCE_ADMIN_ERROR`"): a key that changes what is being
tested is a malformed request, not a failed connection.
- **The #28549 changeset** says "Other unknown top-level keys are still
dropped as before". That changeset is not edited; this PR's changeset
carries the superseding sentence.
## Verification
All readings on head `f1438f52` (the merge of `origin/main` at
`de3fafcb`, after #28607 landed as `f3d84b33`; the diff against `main`
is the three files above).
- **Package:** `pnpm --filter @objectstack/service-datasource test` →
Test Files 67 passed (67), Tests 1125 passed (1125). `pnpm --filter
@objectstack/service-datasource typecheck` exit 0; `tsc --listFiles`
includes the test file.
- **Derived gate families:** `node scripts/pm/dispatch-gates.mjs
--commands --repo objectstack-ai/objectstack` → 61 families (same list
before and after the merge), each run after a full `pnpm build
--concurrency=2` (117/117) with its exit code captured before any pipe
and stdin from `/dev/null`: 61 × exit 0. `--ran`: `✓ dispatch-gates
--ran: 61 derived famil(ies) accounted for — 61 run, 0 NOT-MEASURED`.
- **Hand gates:** `node scripts/check-commit-card-trailers.mjs --range
origin/main..HEAD` exit 0; `node scripts/check-platform-checklist.mjs
--changed-from origin/main` exit 0 (no anchored file in the change set);
`pnpm exec eslint --no-inline-config
packages/services/service-datasource` exit 0, no findings. The repo-wide
`pnpm lint` is CI's.
- **Full dogfood** (owed by the `!` and the `(narrowing)` arm), after
the full build, under the lock, `pnpm --filter @objectstack/dogfood
test` → Test Files 1 failed | 364 passed | 3 skipped (368), Tests 1
failed | 2938 passed | 28 skipped (2967). The one failure is not this
change: `route-ledger-live-mount-parity.dogfood.test.ts` reports
`runtime/route-ledger.ts: GET /packages/:id/customizations — LEDGERED
BUT NOT MOUNTED`, a runtime route that arrived with #28595 (`519089e1`)
in the merged `main`. This diff touches only `service-datasource`.
Reported to the seat; not fixed here.
- **CI** families (Test Core incl. `test:repo`, the lint farm, the
type-check lanes) are read off this PR's check-runs by the seat.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5nv4Jd9m4jHpZgRJVT
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent c90a0b4 commit 819b101
3 files changed
Lines changed: 177 additions & 33 deletions
File tree
- .changeset
- packages/services/service-datasource/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
54 | 57 | | |
55 | 58 | | |
56 | 59 | | |
| |||
275 | 278 | | |
276 | 279 | | |
277 | 280 | | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | 89 | | |
101 | 90 | | |
102 | 91 | | |
| |||
635 | 624 | | |
636 | 625 | | |
637 | 626 | | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
638 | 635 | | |
639 | 636 | | |
640 | 637 | | |
| |||
678 | 675 | | |
679 | 676 | | |
680 | 677 | | |
681 | | - | |
| 678 | + | |
682 | 679 | | |
683 | 680 | | |
684 | 681 | | |
| |||
740 | 737 | | |
741 | 738 | | |
742 | 739 | | |
743 | | - | |
744 | | - | |
745 | | - | |
746 | | - | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
747 | 744 | | |
748 | 745 | | |
749 | 746 | | |
| |||
1261 | 1258 | | |
1262 | 1259 | | |
1263 | 1260 | | |
1264 | | - | |
1265 | | - | |
1266 | | - | |
1267 | | - | |
1268 | | - | |
| 1261 | + | |
| 1262 | + | |
| 1263 | + | |
| 1264 | + | |
| 1265 | + | |
| 1266 | + | |
| 1267 | + | |
| 1268 | + | |
| 1269 | + | |
| 1270 | + | |
| 1271 | + | |
| 1272 | + | |
| 1273 | + | |
| 1274 | + | |
| 1275 | + | |
| 1276 | + | |
1269 | 1277 | | |
1270 | | - | |
1271 | | - | |
1272 | | - | |
1273 | | - | |
1274 | | - | |
1275 | | - | |
| 1278 | + | |
| 1279 | + | |
| 1280 | + | |
1276 | 1281 | | |
1277 | | - | |
1278 | | - | |
1279 | | - | |
1280 | | - | |
| 1282 | + | |
| 1283 | + | |
1281 | 1284 | | |
1282 | 1285 | | |
1283 | 1286 | | |
| |||
1286 | 1289 | | |
1287 | 1290 | | |
1288 | 1291 | | |
1289 | | - | |
| 1292 | + | |
1290 | 1293 | | |
1291 | 1294 | | |
1292 | 1295 | | |
| |||
0 commit comments