Repository navigation
Commit f3d84b3
fix(service-datasource)!: refuse a top-level password or connectionString at the datasource admin write doors (#28607)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XCtDq7uGdxAmnoCoqT9Rx4h)_
Fixes #28549
Clause-②: no (narrowing) — a create or edit body that carries `password`
or `connectionString` at its top level, which `POST /api/v1/datasources`
answered 201 (and `PATCH /api/v1/datasources/NAME` 200) with the key
silently dropped, is refused `400 DATASOURCE_ADMIN_ERROR` before
anything is written. Nothing is widened.
Metadata-protocol: none
Skills-docs: none — no page under `skills/**` or `content/docs` teaches
the admin write 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 write doors
(`@objectstack/service-datasource`).
## 维护者速读
- **改了什么**:数据源管理接口的创建(`POST /api/v1/datasources`)和编辑(`PATCH
/api/v1/datasources/NAME`)现在会拒收写在请求体顶层的 `password` 或
`connectionString`:400
`DATASOURCE_ADMIN_ERROR`,点名这个键,不回显值,什么都不写入。拒收文字直接取自 `DatasourceSchema`
自己的报错,也就是 `os compile` 打印的那一段(password → 用秘密字段或
`external.credentialsRef` 绑定;connectionString → 放进 `config.url`)。
- **为什么改**:卡 #28549(分诊 6554155790):以前这两个键被悄悄丢掉、接口照样答
201,作者拿到一个连不上的数据源且没有任何解释,而构建门对同一个数据源会点名拒收。
- **风险与回滚**:只收窄这两个顶层键;用 `secret` 字段传凭据、编辑时保留已绑定的
`external.credentialsRef`、`config.password` 的原有拒收都不变。控制台表单只发
`label/driver/schemaMode/config/external/secret`,不会触发。回滚即 revert。
-
**先看**:`packages/services/service-datasource/src/datasource-admin-service.ts`(新加的
`assertNoTopLevelCredential` 及其在 create / update
里的两处调用);`packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts`;`.changeset/28549-datasource-top-level-credential.md`。
- **两分钟验证**:`pnpm --filter @objectstack/service-datasource exec vitest
run src/__tests__/datasource-admin-top-level-credential.test.ts` → 10/10
通过。
- **需要你做的**:无,席位落地(收窄需先过合约评审)。
## What changes
**The doors**
(`packages/services/service-datasource/src/datasource-admin-service.ts`).
A new private `assertNoTopLevelCredential(name, body)` runs at the start
of both write doors, before anything is read from or written to the
store:
- `createDatasource`: right after the name rule and the draft
`external.credentialsRef` refusal.
- `updateDatasource`: right after the 404 and the code-defined refusal,
before the merge. It runs on every patch, a `label`/`active`-only edit
included: that exemption spares a row stored before a gate, while this
judges a credential the author is writing now.
It looks for the keys in `TOP_LEVEL_CREDENTIAL_KEYS` (`password`,
`connectionString`) on the body. When one is present it asks
`DatasourceSchema` for its refusal of those key names, with `null`
standing in for every value, and keeps the root `unrecognized_keys`
message. It then throws `Invalid datasource 'NAME'.` plus that message,
the heading the existing record judgement (`assertValidRecord`) uses.
The route turns the throw into `400 DATASOURCE_ADMIN_ERROR`, as it does
for every refusal from this service.
**One spelling.** The prescription is not copied anywhere. The message
is the strict shape's own refusal from
`packages/spec/src/data/datasource.zod.ts` (the `password` and
`connectionString` guidance rows), the same text `os compile` prints.
The pin asserts the door's message equals `Invalid datasource 'NAME'.`
plus `DatasourceSchema`'s message derived inside the test, so a
re-inlined copy that differs by a word goes red.
**No credential stored or echoed.** Only key names reach the judgement;
the values never do. The refusal is thrown before `writeSecret`,
`putDatasourceRecord` or any pool seam runs. The tests read the store,
the secret store and the op log afterwards, and check that the answer
holds neither the placeholder password nor the DSN host.
## Tests
New file `src/__tests__/datasource-admin-top-level-credential.test.ts`.
It drives the real routes over `HonoHttpServer` with the real
`DatasourceAdminService` on an in-memory record store and secret store,
using the `entitled-caller.fixture` caller. Every credential is an
obvious placeholder (`PLACEHOLDER_NOT_A_SECRET`, `db.example.invalid`).
- **Refusals** (each asserts status 400 + `error.code`
`DATASOURCE_ADMIN_ERROR` + `success: false`, the exact message, the
value absent, and nothing stored, minted or registered):
- POST with a top-level `password` (the card's repro 1); the message
also names `external.credentialsRef`.
- POST with a top-level `connectionString` carrying URL userinfo (repro
2).
- POST with both keys: one answer naming both.
- POST with a top-level `password` beside a `secret`.
- PATCH with a top-level `password`, and with a top-level
`connectionString`: the stored record is unchanged.
- PATCH that is `label`-only plus a `password`.
- **Controls**:
- POST with the credential as `secret`: 201, a `sys_secret:` ref bound
at `external.credentialsRef`, the placeholder held only in the secret
store.
- PATCH on a record with a bound `external.credentialsRef`: 200, ref
kept.
- POST with `config.password`: still refused with the driver contract's
`Invalid configuration for driver 'postgres'.` naming `config.password`,
unchanged.
<details><summary>Red first, green, ablation</summary>
- **Red first** (test written and run against the unchanged base
service): `Tests 7 failed | 3 passed (10)`. The four POST refusals were
`expected { status: 201, success: true, … } to deeply equal { status:
400, success: false, … }`, and the three PATCH refusals answered
`status: 200`. The three controls were green.
- **Green** at the fix: `Tests 10 passed (10)`.
- **Ablation** (`node scripts/ablation-replace.mjs`, anchor ` if
(present.length === 0) return;` → ` if (present.length >= 0) return;`,
which turns the check off at both call sites): anchor 1 → 0, replacement
0 → 1, blob `fae370d6fc02` → `7e11b57f76be`. Result: the 7 refusal cases
red again (POST back to 201, PATCH back to 200) and the 3 controls
green. Restore: blob equals HEAD (`fae370d6fc02`), `git diff HEAD`
empty. The test imports the service from `src/`, so no build sits
between the mutation and the run.
- `pnpm --filter @objectstack/service-datasource test`: Test Files 67
passed (67), Tests 1110 passed (1110). `pnpm --filter
@objectstack/service-datasource typecheck` exit 0, and `tsc --listFiles`
includes the new test file.
</details>
## Acceptance notes
- **Which keys.** The build door's strict shape refuses every undeclared
top-level key. Its credential prescription ("must never be inlined …
bind through `external.credentialsRef`") is given for `password`.
`connectionString` gets "belongs inside `config` … `config: { url: …
}`", and a `config.url` with userinfo is then refused by the driver
contract, which points at the secret. The doors take each key's text
verbatim from the schema, so `connectionString` answers with the
`config.url` prescription, not the `credentialsRef` one. That is "the
same prescription the build door gives" for that key.
- **Other top-level keys these doors still drop silently** (read from
`toRecord` and the update merge, which project only `name`, `label`,
`driver`, `schemaMode`, `config`, `external`, `pool`, `active`): the
other connection keys the schema has guidance for (`url`, `host`,
`port`, `database`, `user`, `username`, `filename`), `connector`, the
retired blocks (`readReplicas`, `capabilities`, `retryPolicy`,
`healthCheck`, …), and any other unknown key. A top-level `url` can
carry userinfo exactly as `connectionString` does. Out of scope by the
order ("a general unknown-key refusal for every top-level key is out of
scope"). A closed body would also have to tolerate what a client
round-trips from `GET /datasources/NAME` (`origin`, `hasSecret`,
`redactedConfigKeys`, `definedIn`).
- **The unsaved-draft probe** (`POST /api/v1/datasources/test`,
`testConnection`) projects through the same `toRecord` and still ignores
a top-level `password`: the probe then fails to connect with the
driver's own error, not a silent green. It is not a write door, so it is
left as it was.
- **Control premise.** The order's control "a valid body with
`external.credentialsRef` still answers 201" does not hold on `main`:
since the own-reference rule, create refuses a draft that names
`external.credentialsRef`. The controls are a create whose credential
arrives as `secret` and is bound at `external.credentialsRef` (201), and
an edit that keeps a bound `external.credentialsRef` (200).
- **Producers measured.** The Console's datasource editor
(`ui/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx`,
`draftBody`) sends `label`, `driver`, `schemaMode`, `config`,
`external`, `secret` and never a top-level credential. `packages/client`
exposes only the `datasources/NAME/external/*` family.
## Verification
All readings on head `0cc981aa` (the only commit; nothing landed after
the union ran).
- **Derived gate families:** `node scripts/pm/dispatch-gates.mjs
--commands --repo objectstack-ai/objectstack` → 61 families, each run
with its exit code captured before any pipe (stdin from `/dev/null`).
`--ran` reconciliation: `✓ dispatch-gates --ran: 61 derived famil(ies)
accounted for — 61 run, 0 NOT-MEASURED`. Two first reads were container
prerequisites, re-read after meeting them:
- `check-plugin-teardown-shape.mjs --self-test` exit 3 → `git fetch
--depth=1 origin 621a487…` → exit 0 (48 cases pass).
- `check:dual-build-cjs-loads` exit 3 → full `pnpm build
--concurrency=2` (117/117 tasks) → exit 0.
- **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);
`node scripts/check-nul-bytes.mjs` exit 0; `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 `@objectstack/dogfood^...` build, under the lock, `pnpm --filter
@objectstack/dogfood test` → Test Files 364 passed | 3 skipped (367),
Tests 2934 passed | 28 skipped (2962); no failure.
- **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_01FEkQ5moUefysT3ba6SpdYY
Co-authored-by: Claude <noreply@anthropic.com>1 parent d5da45b commit f3d84b3
3 files changed
Lines changed: 340 additions & 0 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 | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 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 | + | |
89 | 100 | | |
90 | 101 | | |
91 | 102 | | |
| |||
667 | 678 | | |
668 | 679 | | |
669 | 680 | | |
| 681 | + | |
670 | 682 | | |
671 | 683 | | |
672 | 684 | | |
| |||
728 | 740 | | |
729 | 741 | | |
730 | 742 | | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
731 | 747 | | |
732 | 748 | | |
733 | 749 | | |
| |||
1244 | 1260 | | |
1245 | 1261 | | |
1246 | 1262 | | |
| 1263 | + | |
| 1264 | + | |
| 1265 | + | |
| 1266 | + | |
| 1267 | + | |
| 1268 | + | |
| 1269 | + | |
| 1270 | + | |
| 1271 | + | |
| 1272 | + | |
| 1273 | + | |
| 1274 | + | |
| 1275 | + | |
| 1276 | + | |
| 1277 | + | |
| 1278 | + | |
| 1279 | + | |
| 1280 | + | |
| 1281 | + | |
| 1282 | + | |
| 1283 | + | |
| 1284 | + | |
| 1285 | + | |
| 1286 | + | |
| 1287 | + | |
| 1288 | + | |
| 1289 | + | |
| 1290 | + | |
| 1291 | + | |
1247 | 1292 | | |
1248 | 1293 | | |
1249 | 1294 | | |
| |||
0 commit comments