Skip to content

fix(connector-rest): a baseUrl that carries a query string sends each action to its own path - #28767

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-efkbrl
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-efkbrl

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

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=…&region=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.
Red on base, green on head, ablations, controls
  • 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&region=eu, Received /v1?api_key=TEST_PLACEHOLDER_CARRIED_QUERY&region=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).
Base readings (text join on main)
  • 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.
Package suite and type check
  • 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.

Acceptance notes

  • No origin/scheme/host checks exist in the request path today. PR feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #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.ai/code/session_01FEkQ5oht6tMAsnKGYsgbiT

… action to its own path

buildUrl parses the base and lifts its query off before joining the path,
then puts the base parameters back ahead of the action's own (inline path
query, then `query`); on a clash the action's value wins. A base without a
query sends the same URL as before, and a path still only lands under the
base's origin. The dogfood carry-forward pin gains a path-bearing dispatch.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5oht6tMAsnKGYsgbiT
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 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
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 2 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 706ac52d570096a47b64283e970bc1a8e569bc91 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7004d830fa820773336b97babcbc5acf425daefc — the merge of head a9a80de3789bc4b0c1e9e5ab9087dc7e01d3889f into base 706ac52d570096a47b64283e970bc1a8e569bc91, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7004d830fa820773336b97babcbc5acf425daefc && git checkout 7004d830fa820773336b97babcbc5acf425daefc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 706ac52d570096a47b64283e970bc1a8e569bc91 a9a80de3789bc4b0c1e9e5ab9087dc7e01d3889f && git checkout -B drift-repro 706ac52d570096a47b64283e970bc1a8e569bc91 && git merge --no-ff a9a80de3789bc4b0c1e9e5ab9087dc7e01d3889f

node scripts/docs-audit/affected-docs.mjs --json 706ac52d570096a47b64283e970bc1a8e569bc91

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-11-01T18:24Z · head a9a80de3 · #28736

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.

  1. Shape:
    • A draft against main with Fixes #28736 and a patch changeset for @objectstack/connector-rest; Clause-②: no, Metadata-protocol: none.
    • check-governed-merges --pr 28767 reads NOT governed (0 of 4 paths), +118 / -5. Landing tier S.
    • The claim (6561462816) names this PR's branch, claude/project-thread-efkbrl.
  2. Against the order:
    • buildUrl parses the base, lifts its query off, joins the path onto the base's path with one slash, then puts the base's parameters first and the action's own after them; on a clash the action's value wins, as ruled, stated in the code comment and the changeset.
    • A base without a query yields the same URL byte for byte (a 480-case sweep); a path is appended, never resolved, so it cannot leave the base's origin (pinned).
    • The dev measured that PR feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #28688 added no origin check on the request path (its hardening is read-side redaction and a save-door key refusal), so the origin pin sits on the append-only join. A base fragment keeps today's placement, as the order asked, and is reported.
    • The dogfood pin now carries a path-bearing case. Red on base (unit 10 failed, dogfood 1 failed); two ablations red only their own cases.
  3. Contract review: not owed. No spec or governed path is touched, and Clause-②: no carries no narrowing arm: a request that went to the wrong URL now goes to its own.
  4. Gates: the dev reports 60 of 61 derived gates exit 0, check:dual-build-cjs-loads NOT MEASURED locally; CI on this head was in progress with 0 red at review, and auto-merge waits for it.
  5. Overlap: no other open PR touches connector-rest at review.
  6. Landing: pr_ready + automerge_enable through the relay.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 1, 2026 18:25
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 1, 2026 18:25
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit cd0ec76 Nov 1, 2026
50 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-efkbrl branch November 1, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

connector-rest: a baseUrl that carries a query string puts the request path inside the query (buildUrl joins base + path as text)

1 participant