Repository navigation
Commit cd0ec76
fix(connector-rest): a baseUrl that carries a query string sends each action to its own path (#28767)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XHVWgh9oMbncfSuTB7i56uG)_
Fixes #28736
Clause-②: no — no accepted value or public member changes; a
path-bearing action on a base URL with a query now reaches its own path.
Metadata-protocol: none
## 维护者速读
- 改了什么:REST 连接器在 `baseUrl` 自带查询参数时(例如
`https://api.example.com/v1?region=eu`),动作的路径不再被拼进查询串里,而是拼在基址路径后面:`/items`
发往 `/v1/items?region=eu`。基址的查询参数保留在前,动作自己的参数(写在 path 里的,或 `query` 里的,含
api-key 的查询参数)跟在后面;同名时以动作的值为准。
- 为什么改:原来基址和路径按文本拼接,`?region=eu` 加 `/items` 得到 `?region=eu/items`,请求打到
`/v1`,作者只看到对方 404 或错数据,毫无线索。带固定查询参数的基址是常见的服务商写法(`?api-version=…`)。
- 风险与回滚:不带查询参数的基址发出的 URL 与原来逐字节一致(480
组输入对照全同);路径仍只能落在基址的源(origin)之下,`//other.example` 或完整 URL
仍是路径的一段,有测试钉住。回滚即还原本 PR。
- 先看:`packages/connectors/connector-rest/src/rest-connector.ts` 的
`buildUrl`(查询参数先摘下、再拼路径、再合并,同名动作优先);`src/rest-connector.test.ts` 新增的
"base URL with a query string" 一节。
- 两分钟验证:`pnpm --filter @objectstack/connector-rest exec vitest run
src/rest-connector.test.ts`,预期 24/24 通过(其中新增 14 例,10 例在 main 上为红)。
- 需要你做的:无,席位落地。
## What changes
- **A base URL with a query string.** `buildUrl` now parses the base,
lifts its query off, joins the action path onto the base's path exactly
as before (trailing slashes of the base dropped, one slash between), and
puts the base's query parameters back first, then the action's own:
those written inline in `path`, then `query` (an api-key `paramName`
rides `query`). So `https://api.example.com/v1?region=eu` plus `/items`
sends `https://api.example.com/v1/items?region=eu` (on `main`:
`https://api.example.com/v1?region=eu/items`, which goes to `/v1`).
- **On a clash the action's value wins.** A parameter the action sets
(in `query` or inline in `path`) replaces the base's value. Picked over
refusing because refusing would break a working action the day its base
gains the parameter; stated in the code comment and the changeset.
- **Unchanged.** A base without a query sends byte-for-byte the URL it
sent before. A path can only land under the base's origin: it is
appended after the base's path, never resolved against it, so
`//evil.example/x`, `https://evil.example/x`, a backslash form or
`/@evil.example` stay path segments (pinned, for a base with and without
a query). A base fragment keeps the place the text join gave it
(measured below).
- **Dogfood.** The carry-forward case of
`connector-meta-door-entry-and-url-hardening.dogfood.test.ts` dispatched
with an empty path only to avoid this bug; it now also dispatches
`/probe` through the materialized instance and pins
`/v1/probe?api_key=…®ion=eu` at the upstream.
## Tests
- **Unit, `rest-connector.test.ts`, new describe "base URL with a query
string" (14 cases):** a path, a path without a leading slash, a base
with a trailing slash, a base with no path, several base parameters, an
empty path; action `query` merged after the base query; a clash with
`query` and a clash with an inline path query (action wins); an api-key
query parameter beside the base query; the base fragment's place; a path
never leaving the base origin (base with and without a query, five
hostile path shapes); a base without a query sending the text join's URL
(five shapes including `//evil.example/x` and an inline path query).
- **Dogfood:** the carry-forward case gains the path-bearing dispatch.
<details><summary>Red on base, green on head, ablations,
controls</summary>
- Red (unit): `rest-connector.ts` restored to base `7a48bf73`: `10
failed | 14 passed (24)`, the ten query-base cases; the empty-path,
fragment, no-query origin and no-query byte-identity cases and every
existing case pass on base. Restored with `git checkout HEAD`, `git diff
HEAD` empty.
- Red (dogfood): `rest-connector.ts` at base,
`@objectstack/connector-rest` rebuilt (new code absent from `dist/`): `1
failed | 8 passed (9)`, Expected
`/v1/probe?api_key=TEST_PLACEHOLDER_CARRIED_QUERY®ion=eu`, Received
`/v1?api_key=TEST_PLACEHOLDER_CARRIED_QUERY®ion=eu/probe`. Restored
and rebuilt (new code back in `dist/`), tree clean.
- Green on head: unit `24 passed (24)`; package suite `5 passed` files,
`57 passed` tests; dogfood file `9 passed (9)`.
- Ablation 1 (`scripts/ablation-replace.mjs`: the base query is not
lifted, `base.search = base.search;`): `10 failed | 14 passed (24)`, the
same ten query-base cases only. Restored: blob `9180629b` == HEAD, `git
diff HEAD` empty.
- Ablation 2 (the inline-path clash rule off: base parameters kept even
when the path sets them): `1 failed | 23 passed (24)`, the inline clash
case only. Restored: blob == HEAD, `git diff HEAD` empty.
- Control sweep (one-off script against the built `dist/`, not a kept
test): 10 query-free bases (incl. uppercase host, `//v1//`, dot
segments, userinfo, explicit default port, a fragment, an encoded space)
× 16 paths (incl. `//evil.example/x`, an absolute URL, a backslash form,
`/../x`, `?only=1`, `/items?`, `//`) × 3 `query` values = 480 cases: new
URL == the base text join's URL in all 480. Positive control: adding one
query base to the same sweep makes 45 of its 48 cases differ (the 3
identical are the empty path).
</details>
<details><summary>Base readings (text join on main)</summary>
- `https://api.example.com/v1?region=eu` + `/items` →
`https://api.example.com/v1?region=eu/items` (request goes to `/v1`).
- `…/v1?region=eu` + `/items` with `query: { region: 'us' }` →
`https://api.example.com/v1?region=us` (the path is lost entirely).
- `…/v1#frag` + `/items` → `https://api.example.com/v1#frag/items`;
`…/v1?region=eu#frag` + `/items` → `…/v1?region=eu#frag/items`. Both
unchanged on head: the path still lands inside a base fragment, so the
wire request goes to `/v1`.
- `…/v1` + `//evil.example/x` →
`https://api.example.com/v1//evil.example/x`; `…/v1` +
`https://evil.example/x` →
`https://api.example.com/v1/https://evil.example/x`. No path left the
base host on main either; unchanged on head.
</details>
<details><summary>Package suite and type check</summary>
- `pnpm --filter @objectstack/connector-rest build && pnpm --filter
@objectstack/connector-rest typecheck`: exit 0 (`check-dts-emitted` 2/2,
`check:test-typecheck` OK).
- `pnpm --filter @objectstack/connector-rest test`: `5 passed` files,
`57 passed` tests.
</details>
## Acceptance notes
- **No origin/scheme/host checks exist in the request path today.** PR
#28688's URL hardening is read-side redaction of credential parts and
the save-door refusal of undeclared entry keys (`packages/spec`);
neither sits in `buildUrl` or the `rest` provider, and none is touched
here. What stops a path from changing the origin is the append-only
join, unchanged, and now pinned for both base shapes.
- **A base fragment still swallows the path** (kept as measured, per the
order). Is a `#` in a `baseUrl` ever meaningful for an HTTP API? Not
handled here; with the seat.
- A path starting `//` keeps its double slash after the base path, as on
main; "one slash between" holds for every path that does not itself
start with two slashes.
## Verification
Every local reading above is on head `a9a80de3`. Derived gate families
(`node scripts/pm/dispatch-gates.mjs --commands`, reconciled with
`--ran`): 61 derived, 60 run with exit 0, 1 not measured
(`check:dual-build-cjs-loads`, exit 3: its prerequisite, every package's
`dist/`, is not met in this checkout). `check-plugin-teardown-shape
--self-test` first answered exit 3 (its pinned fixture commit was
outside the shallow clone); after `git fetch --depth=1` of that commit
it answered exit 0 (48 cases). `check-nul-bytes`: OK. The repo-wide
lint, `test:repo` and the full farm are CI's. Full dogfood not run: no
`!` in the title and no `(narrowing)` arm. Overlap: no open PR touches
`packages/connectors/connector-rest/` or the dogfood pin (14 open code
PRs' heads diffed against their merge bases).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5oht6tMAsnKGYsgbiT
Co-authored-by: Claude <noreply@anthropic.com>1 parent 52de5a5 commit cd0ec76
4 files changed
Lines changed: 118 additions & 5 deletions
File tree
- .changeset
- packages
- connectors/connector-rest/src
- qa/dogfood/test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
89 | 89 | | |
90 | 90 | | |
91 | 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 | + | |
92 | 167 | | |
93 | 168 | | |
94 | 169 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
103 | | - | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
104 | 121 | | |
105 | | - | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
106 | 125 | | |
107 | | - | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
108 | 134 | | |
109 | 135 | | |
110 | 136 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
267 | 267 | | |
268 | 268 | | |
269 | 269 | | |
270 | | - | |
271 | | - | |
| 270 | + | |
272 | 271 | | |
| 272 | + | |
273 | 273 | | |
274 | 274 | | |
275 | 275 | | |
0 commit comments