Skip to content

feat(spec)!: a URL is judged as a URL parser cleans it — surrounding whitespace trimmed, tab and newline characters removed — with one userinfo grammar at both doors; a credential query parameter is refused in any string of a connector entry - #29875

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/sec-hardening-i
Nov 3, 2026

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Clause-②: no (narrowing)
Metadata-protocol: narrowed
Skills-docs: none — no connector entry that skills/**, the hand-written content/docs/**, examples/** or packages/apps/** author holds a credential query parameter in any string under providerConfig or auth, and none of their datasource or connector URLs carries a credential part (scanned: 0 connector or datasource literals affected; the shipped OpenAPI documents those apps reference by path: 0), so nothing they teach changes; each refusal carries its own prescription.

维护者速读

  • 改了什么:连接器实例与数据源驱动配置里的值按 URL 解析器的读法判定:先去掉首尾的 C0 控制字符与空格,并删除值中任意位置的制表符、换行符与回车符,再判定——userinfo 与查询参数两半、连接器与数据源两扇门都如此;连接器的 userinfo 改用数据源已有的同一套语法(权限部分以 scheme:// 或无 scheme 的 // 起始);连接器条目 providerConfig 与 auth 下任意字符串里带值的凭据类查询参数一律拒绝。保存门 422 INVALID_METADATA,定位到该字符串;两侧读取在写入判定的原始位置改写,其余字符原样返回。
  • 为什么改:维护者裁决 D:两扇门一条值级规则,目的是让检查看到的就是使用方解析器看到的。删除制表/换行字符与连接器沿用数据源的 scheme 可选语法,正是这同一条规则;二者超出裁决的字面文本,席位已作为否决窗口呈报维护者(经契约复核档审核为既裁执行)。
  • 风险与回滚:破坏性收窄(spec minor,一条 ADR-0087 D3 条目 url-credential-judged-trimmed-and-in-any-connector-string)。升级后在校验、构建产物加载与保存时被拒的有:连接器 providerConfig/auth 的任意字符串里带值的凭据查询参数;按解析器清洗后(去首尾空白、删除制表/换行字符)含凭据部分的 URL;连接器中无 scheme 的 // 权限部分所带的 userinfo。仓内 examples/apps/skills/docs 实测 0 处受影响。已存行不重判;读取隐去凭据部分,原样保存保留库值。回滚即 revert 本 PR。
  • 先看:packages/spec/src/data/driver/url-boundary.ts(唯一的边界解析:按解析器清洗并保留到原始串的偏移映射,userinfo 区间供两扇门共用)、packages/spec/src/integration/connector-url-credentials.ts(connectorUrlWithoutUserinfo 同时供写判与读改写;查询参数判定作用于任意字符串)、packages/spec/src/data/datasource-credential-redaction.ts(redactUrlPassword 按写入侧同一区间改写)。
  • 两分钟验证:pnpm --filter @objectstack/spec exec vitest run src/integration/connector-entry-url-credentials.test.ts src/data/datasource-credential-redaction.test.ts,预期全部通过。
  • 需要你做的:无,席位落地。

What changes

One value-level rule at both doors — a URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string of the entry.

  • Cleaned as a URL parser cleans it. A value is judged after trimming its leading and trailing C0 control characters and spaces and removing every tab, newline and carriage return in it — both halves, both doors. Each query pair is cleaned before its name is read. The userinfo is read on the cleaned string and rewritten at the matching offsets of the authored string.
  • One userinfo grammar at both doors. The connector entry locates a userinfo with urlUserinfoSpan, the scheme-optional grammar the datasource doors already use: an authority starts at scheme:// or a scheme-relative //.
  • Any string. The connector entry's credential query parameter rule runs the single value-level parse the datasource doors already run (urlCredentialQueryParams) on every string under providerConfig and auth, scheme or none. Same names, same located refusal, never echoing the value.
  • The read rewrites exactly what the write refuses. packages/spec/src/data/driver/url-boundary.ts (internal, not re-exported) holds the one boundary parse: urlTrimmedBounds and urlUserinfoSpan, offsets into the authored string. The datasource write accessors (urlUserinfoPassword, urlUserinfoUsername) read the userinfo there and the datasource read (redactUrlPassword) rewrites the password there. On the connector side connectorUrlWithoutUserinfo is both the write door's userinfo judgement (does it change the value?) and the read's rewrite, and the query pairs go through the shared credentialQueryParamOf. Every other character, the surrounding ones included, is served as stored. At this head the boundary parse is urlAsParsed (the cleaned string with a map back to raw offsets) and urlUserinfoSpan (raw offsets, the cleaned userinfo, the mapped colon); connectorUrlWithoutUserinfo removes the whole userinfo and its @ at those offsets.
  • Unchanged. No rule keyed on a provider key name; ConnectorSchema keeps its posture; header and environment maps are judged by neither door; stored rows are never re-judged, a save of the served form keeps the stored value, and an edit stores the edit. No public export added or removed (check:api-surface unchanged).
  • ADR-0087 D3 entry 18.url-credential-judged-trimmed-and-in-any-connector-string.ts; changeset @objectstack/spec minor, BREAKING, with its FROM → TO and the registration marker.

Tests

  • packages/spec/src/integration/connector-entry-url-credentials.test.ts — new block: refused at the string (userinfo behind surrounding whitespace, api_key/client_secret in a URL behind whitespace, token and the instance's paramName in a string with no scheme, signature in a string that is not a URL, an auth.tokenUrl behind whitespace); CONTROL accepts (clean URLs with surrounding whitespace, scheme-less and non-URL strings holding none, empty credential values); the read rewrites the same position with surrounding bytes kept and the served form passes the write door; refused ⇔ rewritten over providerConfig.baseUrl, a non-URL providerConfig string and auth.tokenUrl.

  • packages/spec/src/integration/connector-credential-redaction.test.ts — the pin that held a non-URL string with a credential parameter untouched pinned the removed boundary; it is split into a still-untouched CONTROL (a non-URL string holding none) and the new rewrite pin.

  • packages/spec/src/data/driver/driver-credential-refusal.test.ts — per URL-bearing driver key (postgres, mysql, mongo, turso url, turso syncUrl): user:password@ refused behind each of five surroundings, and a bare username behind each not refused for a password; urlUserinfoPassword judges the trimmed string.

  • packages/spec/src/data/datasource-credential-redaction.test.ts — the datasource read rewrites the password at the position the write door judges, keeps the username and every surrounding character, and the served form passes the write door; CONTROL strings come back byte for byte; redactDatasourceConfig names url.

  • Dogfood, both HTTP save doors: connector-meta-door-url-and-nested-keys.dogfood.test.ts gains a block on the existing showcase boot (five 422 refusals at PUT /api/v1/meta/connector/:name with nothing stored; a credential-free URL with surrounding whitespace saves, materializes ready and dispatches; a seeded row is served with the parts removed at the judged position to a member and an administrator; a save of the served form keeps the stored values). New datasource-meta-door-url-credential-trimmed.dogfood.test.ts (three 422 refusals at PUT /api/v1/meta/datasource/:name with nothing stored; a bare username and a clean URL save as written; a seeded row is served with the password removed and its surrounding characters kept; the stored row untouched).

  • Test names state the rule. Every placeholder is fake and every assertion on one is its absence from what a door served.

  • connector-entry-url-credentials.test.ts — a second block: refused at the string for a credential name holding a tab or newline (URL and non-URL string), a userinfo behind a scheme or // holding a tab or carriage return, a scheme-relative userinfo; CONTROL accepts for an ordinary name holding a tab, a credential value of only tab/newline characters, a cleaned scheme with no userinfo, a scheme-relative authority with no userinfo, an @ in a scheme-relative path; the read rewrites at the matching authored offsets; refused ⇔ rewritten.

  • driver-credential-refusal.test.ts — per URL-bearing driver key: user:password@ refused with a tab, newline or carriage return inside the scheme or the password; CONTROL a bare username and a password of only that character; per query driver: the parameter refused with such a character inside its name, CONTROL a value of only such characters; the shared parses read pairs and userinfo with them deleted.

  • datasource-credential-redaction.test.ts — the read removes the password at the mapped offsets and a credential pair whose name holds such a character, keeping the rest of the query; CONTROL strings unchanged.

  • Dogfood, both save doors: connector — 422 for a credential name holding a tab, a scheme holding a tab, a scheme-relative userinfo; CONTROL an ordinary name holding a tab saves, materializes and is sent as the parser reads it, a scheme-relative URL with no userinfo saves. Datasource postgres — 422 for a password name holding a tab and a scheme holding a tab; CONTROL a bare username and an ordinary name each with a tab save as written; a seeded row is served with its credential parts removed at the judged positions.

Acceptance notes

  • Basis for the two additions beyond the ruled text. The maintainer's ruling D is "one value-level rule at both doors"; its stated purpose is that the check sees what the consuming parser sees. Removing tab/newline characters before judging, and the connector using the datasource's scheme-optional userinfo grammar, are that same rule. They narrow beyond the ruled literal text, and the seat has put that to the maintainer as a veto window.

  • Consumers. The cleaning matches the WHATWG parse that pg-connection-string, mysql2 and the mongodb driver apply to these values; for @libsql/core, which applies none, it only refuses at publish a value the client rejects at connect.

  • Cost measured. A non-URL string under providerConfig holding a credential parameter with a value is now refused. Over examples/, packages/apps/, skills/, content/docs/ and apps/docs/ (git grep for a credential name after ? or & with a value): 3 hits, none in a connector entry (a storage-service doc comment and two auth-docs fetch calls); the three OpenAPI documents packages/apps and the showcase reference by path: 0 hits.

  • auth endpoints are URLs by their own declared format, so a string with no scheme there is already refused by that format; the any-string rule matters under providerConfig.

  • The other package this diff touches is packages/qa/dogfood (private, tests only).

Verification

Head 49b7049edb (origin/main at 9edd6e783 merged in).

  • Local gate subset: dispatch-gates --commands derived 81 commands, each exit captured before any pipe; --ran: 81 derived, 81 run, 0 NOT-MEASURED. check:type-check-debt exit 0 (0 raw tsc errors). check:api-surface: public API surface + factory signatures unchanged. check:authorable-surface, check:strictness-ledger, check:spec-changes, check:upgrade-guide, check:dual-build-cjs-loads exit 0. check:generated: All 14 generated artifacts are up to date.
  • Typecheck: spec and dogfood exit 0. Four touched spec files 463/463. Full pnpm --filter @objectstack/spec test: 807 files, 24224 passed, 1 todo, exit 0. Two dogfood pin files 51/51. Full pnpm --filter @objectstack/dogfood test: 410 passed and 3 skipped files, 3400 passed and 28 skipped tests, exit 0.
  • Ablation, one shot per leg through scripts/ablation-replace.mjs, each restored to blob == HEAD with git diff HEAD empty: round 1 — leg A (no trim) 34/395 red, leg B (scheme gate on the query half) 6/395 red; round 2 — leg C (tab/newline removal removed) 51/463 red, split into query half only 13 and userinfo half only 38; leg D (scheme-optional authority removed) 9/463 red.
  • CI on 49b7049edb: 55 success, 9 skipped, 0 failed.

…a credential query parameter in any string of a connector entry

WIP: implementation; tests, D3 entry and changeset follow.

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
…th connector doors and both datasource doors

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
…nd any-string credential rules

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
… the connector and datasource save doors

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation protocol:data tests tooling labels Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 20 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/connectors.mdx (via /api/v1/automation/connectors (route, a path literal in acceptanceCriteria), /meta/connector/:name (route, a path literal in surface))
  • content/docs/kernel/cluster.mdx (via /api/v1/automation/connectors (route, a path literal in acceptanceCriteria), /meta/connector/:name (route, a path literal in surface))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via /api/v1/automation/connectors (route, a path literal in acceptanceCriteria))
  • content/docs/releases/v17/17-6.mdx (via /meta/connector/:name (route, a path literal in surface))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 8 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 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; 103 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 — 168 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 9edd6e783b48ad5e326060e4c4c35040f159deac → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 381b7f25131f2376ea7d1ff57575f06026b8f3e7 — the merge of head 49b7049edb9149a0bec96768e320ad9b86ed3db8 into base 9edd6e783b48ad5e326060e4c4c35040f159deac, 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 381b7f25131f2376ea7d1ff57575f06026b8f3e7 && git checkout 381b7f25131f2376ea7d1ff57575f06026b8f3e7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9edd6e783b48ad5e326060e4c4c35040f159deac 49b7049edb9149a0bec96768e320ad9b86ed3db8 && git checkout -B drift-repro 9edd6e783b48ad5e326060e4c4c35040f159deac && git merge --no-ff 49b7049edb9149a0bec96768e320ad9b86ed3db8

node scripts/docs-audit/affected-docs.mjs --json 9edd6e783b48ad5e326060e4c4c35040f159deac

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 9edd6e783b48ad5e326060e4c4c35040f159deac → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…ewline characters too; one userinfo grammar at both doors

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
…both connector and datasource doors

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
…try and changeset state it

Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/xl and removed size/l labels Nov 3, 2026
@objectstack-fleet objectstack-fleet Bot changed the title feat(spec)!: a URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string of a connector entry feat(spec)!: a URL is judged as a URL parser cleans it — surrounding whitespace trimmed, tab and newline characters removed — with one userinfo grammar at both doors; a credential query parameter is refused in any string of a connector entry Nov 3, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Contract-review-claim: session_01QSeKYn5CmmdShCJifm6WYj · head 49b7049 · 2026-11-03T11:30Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 49b7049edb9149a0bec96768e320ad9b86ed3db8
Local-runs: none
Scope: packages/spec/src/data/datasource-credential-redaction.ts, packages/spec/src/data/driver/common.zod.ts, packages/spec/src/data/driver/url-boundary.ts, packages/spec/src/integration/connector-credential-redaction.ts, packages/spec/src/integration/connector-url-credentials.ts, packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts + .changeset/url-credential-judged-trimmed-and-in-any-connector-string.md, packages/spec/src/integration/connector.zod.ts (the entry refinement and connectorUrlCredentialRefusal), packages/spec/src/data/datasource.zod.ts (the mongo credentialsRef arm), packages/services/service-datasource/src/default-datasource-driver-factory.ts (buildMongoAuth, the username injection), packages/spec/src/data/driver/pg-url-grammar.server.ts, docs/audits/2026-07-unknown-key-strictness-ledger.md (the integration/ and datasource.zod.ts rows), PR #29573's body (the ruling text it quotes), and the five spec test files and two dogfood files of the diff, read for what they pin

Inputs: the PR body as rewritten for this round, the file list, the net diff against main at the merge-base 9edd6e783b, the check-runs on this head, ruling D verbatim, and the earlier independent review's two "within the ruling" findings. The tree was judged, not the body's account of it.

① Derived judgments

Accept-set changes, per door and half, read off the tree.

  • Datasource, userinfo half (credentialFreeUrl → urlUserinfoPassword → urlUserinfoSpan): the password is read on the cleaned string — both ends trimmed of U+0000–U+0020, every ASCII tab, LF and CR deleted anywhere — at RFC 3986's authority and last-@ boundaries. Narrowing: a URL whose non-empty password is reachable only after that cleaning is now refused; on main the anchored regex missed it. Unchanged: a bare username, an empty password, and any string with no //.
  • Datasource, query half (urlCredentialQueryParams → credentialQueryParamOf): each raw pair has tab, LF and CR deleted before the = split and the decoded, case-folded name match. This half applies no end-trim (the ruling names trimming for the userinfo half only). The one divergence from the parser's reading: a last pair whose value is non-empty only by characters the parser trims at the string's end is refused here where the parser reads no credential. Over-refusal in the safe direction, no exposure.
  • Connector, userinfo half (connectorUrlWithoutUserinfo): the same urlUserinfoSpan, so two narrowings against main: the cleaning, and an authority that starts at a scheme-relative //. Still the whole userinfo (feat(spec)!: an authored connector URL holds no credential part; its retryConfig and actions refuse an undeclared key #29573's posture), on every string under providerConfig and auth outside the four withheld maps.
  • Connector, query half: the URL_SCHEME_RE gate is gone; urlCredentialQueryParams runs on every walked string. Narrowing: a non-URL string holding a credential-named pair with a value is refused.

Read/write symmetry ("refused ⇔ rewritten").

  • Connector: the write refuses iff connectorUrlWithoutUserinfo(value) !== value or a pair matches; the read rewrites with the same two functions on the same string. Identical predicate, exact.
  • Datasource, userinfo: the read removes [colon, @) at the raw offsets; the write refuses iff the cleaned password is non-empty. "Rewritten ⇒ refused" fails in exactly one case, pre-existing on main and flagged by the dev: an empty password (user:@host) is rewritten to user@host while the write accepts it. Credential-neutral; the served form passes the write door, and service-datasource's restore compares against redactUrlCredentials(stored), so a save of the served form keeps the stored value. Not a defect of this PR. The prose on redactUrlCredentialQueryParams saying redactUrlPassword "also leaves" that form is wrong on main and still wrong here (no pin asserts it either way) — prose, should-fix.
  • Query, both doors: write and read filter the same raw pairs through the same credentialQueryParamOf. The userinfo always precedes the first ? (the authority is anchored at the string's cleaned start and ends before any ?), so the read's order — userinfo first, then pairs — cannot change the pair set.
  • Offset mapping: rawOffsets[i] is the authored index of cleaned character i, with one sentinel at text.length; start, end (the @) and colon are all taken through it, so a deletion in the scheme, between // and the username, inside the username or inside the password shifts nothing. Walked by hand on each of those, on an empty userinfo (start === end, only the @ goes), on a userinfo ending the string, and on a multi-@ password. The sentinel is never indexed on a reachable path. No off-by-one found; the served form passes the write door in every case walked.

Accessor semantics. urlUserinfoUsername and urlUserinfoPassword now answer the cleaned component (percent-encoding still preserved). Consumers at this head: credentialFreeUrl (presence only); the datasource.zod.ts mongo credentialsRef arm (presence — it now agrees with the client on a URL behind surrounding whitespace, where main refused "names no user"); and buildMongoAuth, which injects as auth.username the username the mongodb client itself reads from the same URL after its own WHATWG cleaning — an alignment, not a drift. The "RAW component" sentence on urlUserinfoUsername now means percent-encoding only; urlUserinfo's note states the cleaning. Prose, not blocking.

Check-runs on 49b7049edb. 0 failed; every gate that ran is green, 9 skipped by their own conditions. The mergeable_state: unstable reading is accounted for by Check Changeset (workflow PR Automation): run 41618651092 success at 10:24Z on this head; after the body rewrite two further pull_request runs on this same head started at 11:29–11:30Z — 41627905511 cancelled (superseded by its successor) and 41628005836, in progress when the review began and completed success at 11:32Z. Both belong to this PR; the only non-green conclusion on the head is that superseded cancellation, and the gate's newest reading is green. The vercel, fly-io, cloudflare-workers-and-pages, claude and objectstack-fleet suites show queued with no runs — third-party app suites, not gates of this repo.

② Semver level

  • @objectstack/spec minor with BREAKING: right for a declared accept-set narrowing under ADR-0087. The changeset carries Clause-②: no (narrowing), Metadata-protocol: narrowed, the registration marker and a FROM → TO that the tree refuses and accepts as written.
  • D3 entry 18.url-credential-judged-trimmed-and-in-any-connector-string.ts: surface names both doors and every entry door; reason states all four narrowings (trim, tab/newline deletion, one userinfo grammar, any connector string), the bare-username carve-out and the deliberate absence of a mechanical rewrite; replacement and acceptanceCriteria are actionable. Each sentence was checked against the code; the amended wording matches the tree. The 18. prefix is the series prefix the other entries use.
  • Metadata-protocol: narrowed is correct: both save doors accept less; the carry-forward contract is unchanged (datasource restore mirrors the one composite redactUrlCredentials; the connector re-grafts by redactedKeys and the withheld maps are untouched).
  • No public export added or removed: url-boundary.ts and connector-url-credentials.ts are internal; the removed URL_SCHEME_RE / URL_USERINFO_RE had no consumer on main outside the connector redactor (the view.zod.ts constant of the same name is its own). The strictness ledger's integration/ and datasource.zod.ts rows describe shapes this PR does not touch; no row changes.

③ Boundary flags

  • Tab/LF/CR deleted anywhere before judging — within ruling D's purpose. The tree's own pg-url-grammar.server.ts records that pg-connection-string parses through new URL(str, base), i.e. a WHATWG parse, and the mysql2 and mongodb notes in common.zod.ts point the same way; a password the parser reads only after deletion was accepted on main. Note also that the pg grammar arm already refuses a leading space for postgres (its space re-encoding makes the value relative) but not a leading tab, LF, CR or other C0 character, and that arm is server-only — so this value-level rule is the only guard in the browser build. Accept.
  • One scheme-optional userinfo grammar at the connector — the weaker leg, as the body declares. Cost on the tree: any string under providerConfig or auth whose cleaned form starts with // and holds an @ before any /, ? or # is refused as a userinfo; prose or an argument-list element can take that shape. Benefit: a scheme-relative reference is read with its userinfo by every consumer that resolves it. The refusal is located and names the fix; the body's in-repo scan found 0 hits. Within the ruling; it stays the veto window the body puts to the maintainer. Not blocking. Should-fix candidate, the maintainer's call: on the scheme-relative arm only, require the host part to hold no whitespace, which a URL parser rejects as a host — the purpose "the check sees what the parser sees" would then hold on both sides of that arm.
  • Open question, escalated: special-scheme strings. A string with a special scheme (http, https, ws, wss, ftp, file) whose authority is not introduced by // is judged by neither door at this head — the shared grammar requires // — so such a value is stored and served; no built-in provider sends it (they fetch, and fetch refuses a userinfo). Leaving it out of this PR is right: it would be a third reading of the grammar, outside ruling D's "one grammar at both doors" text, and no datasource driver uses a special scheme, so it would in effect be connector-only. Symmetry holds there (neither refused nor rewritten); it is an accept-set question, not an asymmetry, and not a FAIL. It needs a maintainer ruling on whether the shared authority grammar admits a special scheme's slash-tolerant forms at both doors.
  • libsql. The body's claim that @libsql/core applies no cleaning cannot be verified from this tree (no installed client to read; nothing was run). It is not load-bearing for the verdict: the tree's own measured note is that libsql reads ?authToken= by the exact decoded key and refuses a userinfo, so no value libsql opens is refused by this PR (see ④).

④ Design judgment

  • Off-by-one on deletions before the colon or the @: none; see the offset walk in ①.
  • A pair whose name collides with a credential name only after cleaning: the write refuses it and the read removes the raw pair whole; both go through credentialQueryParamOf, and the parser's URLSearchParams reads the same name. Consistent.
  • Percent-encoded tab (%09): neither the parser nor this code deletes it (deletion precedes percent-decoding on both sides); the decoded name then differs from the credential name on both sides. Consistent; nothing to fix.
  • A value valid for libsql but now refused: a turso URL behind surrounding whitespace, or with a tab, LF or CR in its scheme, holding a userinfo was accepted on main and is refused now; libsql opens neither (its scheme must start the string, and the turso contract has refused a userinfo since [Decision] URL-embedded credentials (user:password@host in driver config.url) remain a live cleartext door after #7990 — refuse at publish, or accept as residual risk? #8082). A ?authToken= name that matches only after deleting such a character is refused now and was never read by libsql, which matches the exact decoded key. No working value lost.
  • Performance of cleaning every string under providerConfig: urlAsParsed builds a character-by-character copy plus a per-character offset array for every walked string — an inline OpenAPI document's strings included — on every entry parse and every served read, the clean common case included; main's scheme gate skipped most strings. Bounded by metadata size, not on a request path; acceptable. Should-fix, non-blocking: a fast path when the string needs no cleaning (identity offsets), or a //-presence check before building the map.
  • Tests: names state the rule; refusals name parts and position, never the value; every placeholder is fake. Pinned: refused ⇔ rewritten at both doors, the mapped offsets, CONTROLs for a bare username, an empty credential value, a tab in an ordinary name, a scheme-relative authority without userinfo, an @ in a scheme-relative path; both HTTP save doors in dogfood with nothing stored on refusal and the stored row never rewritten.
  • Must-fix: none.

Implemented-by: claude/sec-hardening-i
Reviewed-by: session_01QSeKYn5CmmdShCJifm6WYj

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

席位核对

ACCEPT · head 49b7049edb9149a0bec96768e320ad9b86ed3db8 · 2026-11-03T11:43Z

  • Shape: 13 files, +953/−82, under 3000, not governed. Spec minor with a BREAKING banner, Clause-②: no (narrowing), Metadata-protocol: narrowed. One ADR-0087 D3 entry.
  • Rule held (the maintainer's ruling D, quoted in the PR body): one value-level rule at both doors. A value is judged as a URL parser cleans it. The credential query half is judged on every string of a connector entry. The read rewrites exactly what the write refuses. No rule is keyed on provider key names.
  • Contract review: comment 6602879406, Served-tier: CONTRACT_REVIEW_TIER, Head-sha equal to this head, VERDICT PASS, no must-fix. It judged both additions beyond the literal ruled text to be within the ruling (tab/newline removal; the connector using the datasource's scheme-optional userinfo grammar), with the second as the maintainer's veto window. An earlier independent review reached the same reading.
  • Check-runs at this head: 58 success, 13 skipped, 1 cancelled (a superseded Check Changeset run; the later run on this head is success), 0 failed. Skips judged by name; check-expected-skips.mjs NOT MEASURED. mergeable_state clean.
  • Accepted deviations: a clean origin/main merge before the patch; urlUserinfoUsername/Password now answer the cleaned component (the review read every consumer); the empty-password read rewrite is unchanged from main; the additional ablation legs.
  • Out of scope, routed by the security seat: the special-scheme userinfo shapes, escalated to the maintainer; the review's should-fixes (a no-cleaning fast path in urlAsParsed, two prose corrections, an optional host-whitespace check on the // arm), which go to a follow-up order.
  • Landing: ready, then auto-merge.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 3, 2026 11:44
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 3, 2026 11:44
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit cfd237f Nov 3, 2026
72 of 73 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/sec-hardening-i branch November 3, 2026 12:06
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 protocol:data size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant