From 27343cf8b574c3afca6c6d6a5d0a84e28d85ba4d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 07:46:28 +0000 Subject: [PATCH 1/7] feat(spec): judge a URL after trimming surrounding whitespace; judge 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 --- .../data/datasource-credential-redaction.ts | 44 +++++----- packages/spec/src/data/driver/common.zod.ts | 39 ++++----- packages/spec/src/data/driver/url-boundary.ts | 81 +++++++++++++++++++ .../connector-credential-redaction.ts | 21 +++-- .../integration/connector-url-credentials.ts | 66 ++++++++++----- 5 files changed, 179 insertions(+), 72 deletions(-) create mode 100644 packages/spec/src/data/driver/url-boundary.ts diff --git a/packages/spec/src/data/datasource-credential-redaction.ts b/packages/spec/src/data/datasource-credential-redaction.ts index b491b004f62..7aaf44ac281 100644 --- a/packages/spec/src/data/datasource-credential-redaction.ts +++ b/packages/spec/src/data/datasource-credential-redaction.ts @@ -110,6 +110,7 @@ import { MONGO_OPTIONS_CREDENTIAL_PATHS, } from './driver/common.zod'; import { getDriverConfigSchema, resolveDriverId } from './driver/config-registry.zod'; +import { urlUserinfoSpan } from './driver/url-boundary'; // The canonical spellings and former aliases MOVED to `driver/common.zod.ts` // with the nested-position fix: the write door's passthrough walk @@ -388,35 +389,36 @@ export function redactableConfigKeys(driver: unknown): string[] { return [...new Set([...canonical, ...FORMER_CREDENTIAL_ALIASES, ...retired])]; } -/** - * `scheme://[user[:password]@]rest`. Anchored, and every class excludes `/?#` - * so a password-looking substring in a path or query cannot be mistaken for one - * — `https://host/a:b@c` has no userinfo and must come back untouched. - * - * The password group deliberately ALLOWS `@` and is greedy, which (with - * backtracking) makes the match end at the LAST `@` before the path — the - * userinfo boundary RFC 3986 actually defines. A lazier class stopping at the - * first `@` would split `postgres://u:p@ss@host/db` after `p`, leave `ss@host` - * in place, and publish a fragment of the password while looking redacted. - * Such a URL is malformed (a literal `@` in userinfo must be `%40`), which is - * precisely why it must not be the case that decides how much leaks. - * - * The write door's detector (`urlUserinfoPassword` in `driver/common.zod.ts`) - * draws the same boundaries; now that both live in this package, the pin - * keeping them aligned is `datasource-credential-redaction.test.ts`. - */ -const URL_USERINFO_RE = /^([a-z][a-z0-9+.\-]*:\/\/)([^/?#@:]*)(:[^/?#]*)@/i; - /** * Strip the password component from a URL's userinfo, preserving the scheme, - * the username, and everything from the host onward. + * the username, and everything from the host onward — every other character + * of the string, the surrounding ones included, byte for byte. + * + * The userinfo is located by the write door's own boundary parse + * (`urlUserinfoSpan` in `driver/url-boundary.ts`, which `urlUserinfoPassword` + * in `driver/common.zod.ts` reads too), so the read rewrites exactly what the + * write refuses: the string is judged trimmed of its leading and trailing C0 + * control characters and spaces, as a URL parser reads it; a scheme-relative + * `//` authority counts; the userinfo ends at the LAST `@` in the authority — + * a lazier boundary stopping at the first `@` would split + * `postgres://u:p@ss@host/db` after `p`, leave `ss@host` in place, and publish + * a fragment of the password while looking redacted (such a URL is malformed — + * a literal `@` in userinfo must be `%40` — which is precisely why it must not + * be the case that decides how much leaks); and the password starts after the + * FIRST `:` in the userinfo. A `:` or `@` in a path or query is never userinfo + * — `https://host/a:b@c` comes back untouched. The pin keeping the two halves + * aligned is `datasource-credential-redaction.test.ts`. * * Returns the input unchanged when there is nothing to strip, which is what * makes "did this value change?" a usable test for whether a credential was * present. */ export function redactUrlPassword(value: string): string { - return value.replace(URL_USERINFO_RE, (_m, scheme: string, user: string) => `${scheme}${user}@`); + const span = urlUserinfoSpan(value); + if (span === undefined) return value; + const colon = value.indexOf(':', span.start); + if (colon === -1 || colon >= span.end) return value; + return `${value.slice(0, colon)}${value.slice(span.end)}`; } /** diff --git a/packages/spec/src/data/driver/common.zod.ts b/packages/spec/src/data/driver/common.zod.ts index 99ecf71f9b3..60f817998d8 100644 --- a/packages/spec/src/data/driver/common.zod.ts +++ b/packages/spec/src/data/driver/common.zod.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; +import { urlUserinfoSpan } from './url-boundary'; /** * Shared building blocks for the per-driver `datasource.config` shapes. @@ -187,23 +188,17 @@ export const URL_EMBEDDED_CREDENTIAL_REFUSED = (key: string): string => /** * The userinfo component of a URL-ish string, or `undefined` when the string - * carries none — the ONE boundary parse behind both userinfo accessors - * ({@link urlUserinfoPassword}, {@link urlUserinfoUsername}), deliberately - * unexported so no caller can depend on a third reading of the grammar. - * - * Deliberately NOT `new URL()`: real DSNs take forms WHATWG parsing rejects or - * mangles (postgres/mongo multi-host `user:pass@h1:5432,h2:5432/db` throws - * `ERR_INVALID_URL`, bare `:memory:`, `file:` paths), and a parse that throws - * on the exact inputs it must judge would fail open. The boundaries below are - * RFC 3986's, and match the read-path redactor (`redactUrlPassword` in this - * package's `data/datasource-credential-redaction.ts`, moved from - * `service-datasource` by #8300) so the write door refuses precisely the - * material the read door redacts: - * - * - the authority is what follows `//` (scheme-relative included), up to the - * first `/`, `?` or `#` — a `:` or `@` in a path or query is never userinfo; - * - userinfo ends at the LAST `@` in the authority (a malformed literal `@` - * inside a password must not decide how much of it goes unjudged). + * carries none — what both userinfo accessors ({@link urlUserinfoPassword}, + * {@link urlUserinfoUsername}) split, deliberately unexported so no caller can + * depend on a third reading of the grammar. + * + * The boundaries are `urlUserinfoSpan`'s (`./url-boundary.ts`), the ONE + * userinfo boundary parse the read-path redactor (`redactUrlPassword` in this + * package's `data/datasource-credential-redaction.ts`) rewrites at too, so the + * write door refuses precisely the material the read door redacts: RFC 3986's + * authority and last-`@` boundaries, read on the string trimmed of its + * leading and trailing C0 control characters and spaces, as a URL parser + * reads it (why WHATWG parsing itself is refused is on that parse). * * A string with no `//` (sqlite/turso `file:` paths, `:memory:`) has no * authority and answers `undefined`. The FIRST `:` inside the returned @@ -211,14 +206,8 @@ export const URL_EMBEDDED_CREDENTIAL_REFUSED = (key: string): string => * accessors, not here. */ function urlUserinfo(value: string): string | undefined { - const scheme = /^(?:[a-z][a-z0-9+.-]*:)?\/\//i.exec(value); - if (!scheme) return undefined; - const rest = value.slice(scheme[0].length); - const end = rest.search(/[/?#]/); - const authority = end === -1 ? rest : rest.slice(0, end); - const at = authority.lastIndexOf('@'); - if (at === -1) return undefined; - return authority.slice(0, at); + const span = urlUserinfoSpan(value); + return span === undefined ? undefined : value.slice(span.start, span.end); } /** diff --git a/packages/spec/src/data/driver/url-boundary.ts b/packages/spec/src/data/driver/url-boundary.ts new file mode 100644 index 00000000000..ed9fd13d5ba --- /dev/null +++ b/packages/spec/src/data/driver/url-boundary.ts @@ -0,0 +1,81 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Where a URL-ish authored string starts and ends, and where its userinfo + * sits — the boundary parse every URL-credential judge in this package reads. + * Internal: imported by the datasource write door's userinfo accessors + * (`./common.zod.ts`), the datasource read redactor + * (`../datasource-credential-redaction.ts`) and the connector URL judge + * (`../../integration/connector-url-credentials.ts`), never re-exported. + * + * ## Surrounding whitespace + * + * A URL parser (the WHATWG URL Standard, which `fetch`, `new URL()` and the + * clients built on them follow) strips leading and trailing C0 control + * characters and spaces — U+0000 to U+0020 — before it reads the string. So a + * URL is judged where the parser reads it: on the string trimmed of those + * characters ({@link urlTrimmedBounds}). A judge that read the untrimmed + * string would answer for a value no client ever opens. + * + * The bounds are offsets into the authored string, not a trimmed copy, so a + * read redactor rewrites the credential part at the position the write door + * judged and leaves every other character, the surrounding ones included, + * byte for byte. + */ + +/** U+0000 to U+0020: the C0 control characters and space a URL parser strips from both ends. */ +function isC0ControlOrSpace(code: number): boolean { + return code <= 0x20; +} + +/** + * `[start, end)` of `value` with its leading and trailing C0 control + * characters and spaces left out — the span a URL parser reads (module note). + * `start === end` for a string holding nothing else. + */ +export function urlTrimmedBounds(value: string): readonly [start: number, end: number] { + let start = 0; + let end = value.length; + while (start < end && isC0ControlOrSpace(value.charCodeAt(start))) start += 1; + while (end > start && isC0ControlOrSpace(value.charCodeAt(end - 1))) end -= 1; + return [start, end]; +} + +/** `scheme://` or a scheme-relative `//`: where an authority starts. */ +const AUTHORITY_START_RE = /^(?:[a-z][a-z0-9+.-]*:)?\/\//i; + +/** + * `[start, end)` of the userinfo in `value`, or `undefined` when it carries + * none — the ONE userinfo boundary parse of the datasource doors: the write + * door's accessors (`urlUserinfoPassword`, `urlUserinfoUsername`) read the + * userinfo at these offsets, and the read redactor (`redactUrlPassword`) + * rewrites the password at them, so the read rewrites exactly what the write + * refuses. + * + * Deliberately NOT `new URL()`: real DSNs take forms WHATWG parsing rejects or + * mangles (postgres/mongo multi-host `user:pass@h1:5432,h2:5432/db` throws + * `ERR_INVALID_URL`, bare `:memory:`, `file:` paths), and a parse that throws + * on the exact inputs it must judge would fail open. The boundaries are + * RFC 3986's, read on the trimmed span ({@link urlTrimmedBounds}): + * + * - the authority is what follows `//` (scheme-relative included), up to the + * first `/`, `?` or `#` — a `:` or `@` in a path or query is never userinfo; + * - userinfo ends at the LAST `@` in the authority (a malformed literal `@` + * inside a password must not decide how much of it goes unjudged). + * + * A string with no `//` (sqlite/turso `file:` paths, `:memory:`) has no + * authority and answers `undefined`. + */ +export function urlUserinfoSpan(value: string): { start: number; end: number } | undefined { + const [start, end] = urlTrimmedBounds(value); + const url = value.slice(start, end); + const scheme = AUTHORITY_START_RE.exec(url); + if (!scheme) return undefined; + const rest = url.slice(scheme[0].length); + const authorityEnd = rest.search(/[/?#]/); + const authority = authorityEnd === -1 ? rest : rest.slice(0, authorityEnd); + const at = authority.lastIndexOf('@'); + if (at === -1) return undefined; + const userinfoStart = start + scheme[0].length; + return { start: userinfoStart, end: userinfoStart + at }; +} diff --git a/packages/spec/src/integration/connector-credential-redaction.ts b/packages/spec/src/integration/connector-credential-redaction.ts index fa79044245a..7de3460ace8 100644 --- a/packages/spec/src/integration/connector-credential-redaction.ts +++ b/packages/spec/src/integration/connector-credential-redaction.ts @@ -72,9 +72,12 @@ * `providerConfig.spec` (`openapi`), and any other URL a provider keeps at * an OBJECT depth of `providerConfig`; and the endpoints an instance-auth * kind declares (`auth.tokenUrl`, `auth.authorizationUrl`, `auth.resource`) - * and any URL a kind declares later. A string is judged when it starts - * with `scheme://`. What a credential part is, and the walk that finds - * every URL holding one, live in ONE place, + * and any URL a kind declares later. The userinfo is judged on a string + * that, trimmed of its leading and trailing C0 control characters and + * spaces as a URL parser trims it, starts with `scheme://`, and is + * rewritten at that trimmed position; a credential query parameter is + * judged in any string. What a credential part is, and the walk that finds + * every string holding one, live in ONE place, * `./connector-url-credentials.ts`, which the write door's refinement * runs too: an authored URL holding one is refused, so only a row stored * before that refusal can carry one to this read. @@ -160,8 +163,8 @@ import { redactUrlCredentialQueryParams } from '../data/datasource-credential-re import { CONNECTOR_CREDENTIAL_MAP_PATHS, CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES, - URL_USERINFO_RE, connectorCredentialQueryParamsOf, + connectorUrlWithoutUserinfo, connectorUrlCredentialPositionOf, findConnectorUrlCredentials, } from './connector-url-credentials'; @@ -181,9 +184,15 @@ export { CONNECTOR_CREDENTIAL_MAP_PATHS }; */ export { CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES }; -/** One URL with its userinfo and credential query parameters removed; the input when there is none. */ +/** + * One URL with its userinfo and credential query parameters removed; the input + * when there is none. Each half is the write door's own judgement applied: the + * userinfo is removed where the write door judged it, on the trimmed span + * ({@link connectorUrlWithoutUserinfo}), and the query pairs by the parse the + * write door's `urlCredentialQueryParams` reads, in any string. + */ function redactConnectorUrl(value: string, params: readonly string[]): string { - return redactUrlCredentialQueryParams(value.replace(URL_USERINFO_RE, '$1'), params); + return redactUrlCredentialQueryParams(connectorUrlWithoutUserinfo(value), params); } /** One instance-auth kind as the union declares it: its literal-valued keys, and every key it declares. */ diff --git a/packages/spec/src/integration/connector-url-credentials.ts b/packages/spec/src/integration/connector-url-credentials.ts index a2b5fc4d72c..cfdf5fbfb7f 100644 --- a/packages/spec/src/integration/connector-url-credentials.ts +++ b/packages/spec/src/integration/connector-url-credentials.ts @@ -7,10 +7,10 @@ * the read redactor (`./connector-credential-redaction.ts`), never exported on * its own. * - * ## The URLs judged + * ## The strings judged * - * Every string that starts with `scheme://` under the two places an entry - * keeps a URL ({@link CONNECTOR_URL_ROOTS}): + * Every string under the two places an entry keeps a URL + * ({@link CONNECTOR_URL_ROOTS}): * * - `providerConfig` — the provider's own vocabulary: `baseUrl` (`rest`, * `openapi`), `transport.url` (`mcp` over http), a URL `spec` (`openapi`), @@ -25,14 +25,19 @@ * ## The credential parts * * - **the userinfo**, whole: `https://user:pass@host`, and `https://@host`, - * the form several HTTP APIs read as a token; - * - **each credential query parameter** with a non-empty value: the names in + * the form several HTTP APIs read as a token. Judged on the string trimmed + * of its leading and trailing C0 control characters and spaces + * (`urlTrimmedBounds`, `data/driver/url-boundary.ts`) — where a URL parser + * reads it — when that trimmed string starts with `scheme://`; + * - **each credential query parameter** with a non-empty value, in ANY + * judged string, whether or not it starts with a scheme: the names in * {@link CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES}, plus the - * `auth.paramName` an `api-key` instance itself names. A pair is judged by - * `credentialQueryParamOf` (`data/driver/common.zod.ts`), the boundary - * definition the datasource doors share: the query runs from the first `?` - * to the fragment, names match case-insensitively on the percent-decoded - * key, and an empty value carries nothing. + * `auth.paramName` an `api-key` instance itself names. The query is read by + * `urlCredentialQueryParams` (`data/driver/common.zod.ts`), the single + * value-level parse the datasource doors run too, and a pair is judged by + * `credentialQueryParamOf` beside it: the query runs from the first `?` to + * the fragment, names match case-insensitively on the percent-decoded key, + * and an empty value carries nothing. * * ## One traversal, two doors * @@ -47,6 +52,7 @@ */ import { CREDENTIAL_URL_QUERY_PARAM_NAMES, urlCredentialQueryParams } from '../data/driver/common.zod'; +import { urlTrimmedBounds } from '../data/driver/url-boundary'; /** * The header and environment maps a provider sends upstream — withheld WHOLE by @@ -89,15 +95,30 @@ export const CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES: readonly string[] = Obj 'x-api-key', ]); -/** `scheme://` — the strings judged as URLs. */ -export const URL_SCHEME_RE = /^[a-z][a-z0-9+.\-]*:\/\//i; - /** * `scheme://userinfo@` — the whole userinfo. Every class excludes `/?#`, so the * `@` must sit in the authority; the greedy class backtracks to the LAST `@` - * before the path, the userinfo boundary RFC 3986 defines. + * before the path, the userinfo boundary RFC 3986 defines. Run on the trimmed + * span only ({@link connectorUrlWithoutUserinfo}). */ -export const URL_USERINFO_RE = /^([a-z][a-z0-9+.\-]*:\/\/)[^/?#]*@/i; +const URL_USERINFO_RE = /^([a-z][a-z0-9+.\-]*:\/\/)[^/?#]*@/i; + +/** + * `value` with its userinfo removed, or `value` itself when it holds none — + * the userinfo half's ONE judgement, which the write door asks (does this + * change the value?) and the read redactor applies, so the read rewrites + * exactly what the write refuses. The userinfo is judged on the span a URL + * parser reads — `value` trimmed of its leading and trailing C0 control + * characters and spaces (`urlTrimmedBounds`) — and removed there, every + * other character of `value`, the surrounding ones included, kept byte for + * byte. + */ +export function connectorUrlWithoutUserinfo(value: string): string { + const [start, end] = urlTrimmedBounds(value); + const url = value.slice(start, end); + const rewritten = url.replace(URL_USERINFO_RE, '$1'); + return rewritten === url ? value : `${value.slice(0, start)}${rewritten}${value.slice(end)}`; +} const isPlainRecord = (v: unknown): v is Record => !!v && typeof v === 'object' && !Array.isArray(v); @@ -126,13 +147,17 @@ export interface ConnectorUrlCredentialParts { params: string[]; } -/** The credential parts `value` holds, or `undefined` when it is not a URL or holds none. */ +/** + * The credential parts `value` holds, or `undefined` when it holds none: the + * userinfo on the trimmed span ({@link connectorUrlWithoutUserinfo}), and the + * credential query parameters in any string, scheme or none (see the module + * note). + */ export function connectorUrlCredentialPartsOf( value: string, params: readonly string[], ): ConnectorUrlCredentialParts | undefined { - if (!URL_SCHEME_RE.test(value)) return undefined; - const userinfo = URL_USERINFO_RE.test(value); + const userinfo = connectorUrlWithoutUserinfo(value) !== value; const found = urlCredentialQueryParams(value, params); return userinfo || found.length > 0 ? { userinfo, params: found } : undefined; } @@ -146,8 +171,9 @@ export interface ConnectorUrlCredential extends ConnectorUrlCredentialParts { } /** - * Every URL under {@link CONNECTOR_URL_ROOTS} that holds a credential part, at - * any depth — inside arrays and under keys whose own name carries a dot too — + * Every string under {@link CONNECTOR_URL_ROOTS} that holds a credential part + * (see the module note), at any depth — inside arrays and under keys whose own + * name carries a dot too — * except inside the maps of {@link CONNECTOR_CREDENTIAL_MAP_PATHS}, whatever a * map position holds. The one traversal both doors run (see the module note): * the write door refuses every URL returned, the read redactor withholds each From 722e914bc671e96d168ed1799607cc2b7d0fbbf4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 08:05:12 +0000 Subject: [PATCH 2/7] test(spec): pin the trimmed-URL and any-string credential rules at both connector doors and both datasource doors Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .../datasource-credential-redaction.test.ts | 50 ++++++++++ .../driver/driver-credential-refusal.test.ts | 40 ++++++++ .../connector-credential-redaction.test.ts | 13 ++- .../connector-entry-url-credentials.test.ts | 97 +++++++++++++++++++ 4 files changed, 198 insertions(+), 2 deletions(-) diff --git a/packages/spec/src/data/datasource-credential-redaction.test.ts b/packages/spec/src/data/datasource-credential-redaction.test.ts index 64a54f205b5..209b6cfb91f 100644 --- a/packages/spec/src/data/datasource-credential-redaction.test.ts +++ b/packages/spec/src/data/datasource-credential-redaction.test.ts @@ -263,6 +263,56 @@ describe('write-door alignment: redactUrlPassword removes exactly what urlUserin }); }); +describe('the datasource read rewrites a URL\'s userinfo password exactly where the write door judges it', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const CR = String.fromCharCode(0x0d); + const NUL = String.fromCharCode(0x00); + + // [stored, served]: the URL is judged trimmed of its leading and trailing C0 + // control characters and spaces, as a URL parser reads it, and the password + // is removed at that trimmed position — every other character, the + // surrounding ones included, is served byte for byte. + const TRIMMED: Array<[string, string]> = [ + [' mysql://svc:hunter2@db:3306/app', ' mysql://svc@db:3306/app'], + [`${TAB}postgresql://svc:hunter2@db:5432/app`, `${TAB}postgresql://svc@db:5432/app`], + [`${NUL}mongodb://svc:hunter2@db:27017/app${LF}`, `${NUL}mongodb://svc@db:27017/app${LF}`], + [` postgres://svc:p@ss@host/db${CR}`, ` postgres://svc@host/db${CR}`], + ]; + // [stored, served]: the write door's own userinfo boundaries — a + // scheme-relative authority, and a userinfo that ends at the LAST `@` of the + // authority whatever precedes its first `:`. + const BOUNDARIES: Array<[string, string]> = [ + ['//svc:hunter2@db:5432/app', '//svc@db:5432/app'], + [' //svc:hunter2@db:5432/app', ' //svc@db:5432/app'], + ['postgres://svc@corp:hunter2@db:5432/app', 'postgres://svc@corp@db:5432/app'], + ]; + + it.each([...TRIMMED, ...BOUNDARIES])('%j is served as %j, which the write door accepts', (stored, served) => { + expect(urlUserinfoPassword(stored), 'the write door refuses the stored URL').toBeDefined(); + expect(redactUrlPassword(stored)).toBe(served); + expect(urlUserinfoPassword(served), 'the served URL passes the write door').toBeUndefined(); + expect(urlUserinfoUsername(served), 'the username is kept').toBe(urlUserinfoUsername(stored)); + }); + + it.each([ + ' mysql://svc@db:3306/app', + `${TAB}postgresql://db:5432/app${LF}`, + ' //svc@db/app', + ' a:b@c ', + ` https://host/a:b@c${CR}`, + ])('CONTROL: %j holds no password, passes the write door, and is served byte for byte', (url) => { + expect(urlUserinfoPassword(url)).toBeUndefined(); + expect(redactUrlPassword(url)).toBe(url); + }); + + it('redactDatasourceConfig serves a stored URL behind surrounding whitespace with its password removed, naming `url`', () => { + const { config, redactedKeys } = redactDatasourceConfig('mysql', { url: ' mysql://svc:hunter2@db:3306/app' }); + expect(config).toEqual({ url: ' mysql://svc@db:3306/app' }); + expect(redactedKeys).toEqual(['url']); + }); +}); + describe('redactUrlCredentialQueryParams — the read half: a credential query parameter is never served back', () => { it('strips the credential pair whole and serves the parameter-absent shape', () => { expect(redactUrlCredentialQueryParams('libsql://x.turso.io?authToken=eyJhbGci.x.y')) diff --git a/packages/spec/src/data/driver/driver-credential-refusal.test.ts b/packages/spec/src/data/driver/driver-credential-refusal.test.ts index 712a30b44d3..d675c0c7495 100644 --- a/packages/spec/src/data/driver/driver-credential-refusal.test.ts +++ b/packages/spec/src/data/driver/driver-credential-refusal.test.ts @@ -317,6 +317,32 @@ describe.each(URL_FAMILY)('$name — URL-embedded credential refusal', (f) => { expect(f.schema.parse(config)).toEqual(before.data); } }); + + describe('a URL is judged after trimming its leading and trailing C0 control characters and spaces', () => { + /** The userinfo refusal at `f.key` — other refusals at the same key (a driver's own URL grammar) are not this rule's. */ + const userinfoRefusalAt = (config: Record) => { + const result = f.schema.safeParse(config); + if (result.success) return undefined; + return result.error.issues.find( + (i) => i.path.join('.') === f.key && i.message.includes('embeds a password in its userinfo'), + ); + }; + const SURROUNDINGS: Array<[label: string, lead: string, trail: string]> = [ + ['a leading space', ' ', ''], + ['a leading tab', String.fromCharCode(0x09), ''], + ['a leading NUL', String.fromCharCode(0x00), ''], + ['a trailing newline', '', String.fromCharCode(0x0a)], + ['a leading space and a trailing carriage return', ' ', String.fromCharCode(0x0d)], + ]; + + it.each(SURROUNDINGS)('refuses `user:password@host` behind %s', (_label, lead, trail) => { + expect(userinfoRefusalAt(f.make(`${lead}${f.sample('svc:hunter2@')}${trail}`))).toBeDefined(); + }); + + it.each(SURROUNDINGS)('CONTROL: a bare username (`user@host`) behind %s carries no password and is not refused for one', (_label, lead, trail) => { + expect(userinfoRefusalAt(f.make(`${lead}${f.sample('svc@')}${trail}`))).toBeUndefined(); + }); + }); }); /** @@ -509,6 +535,20 @@ describe('urlUserinfoPassword — the shared value-level parse', () => { expect(urlUserinfoPassword('//u@h/db')).toBeUndefined(); }); + it('a URL is judged trimmed of its leading and trailing C0 control characters and spaces, as a URL parser reads it', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const NUL = String.fromCharCode(0x00); + expect(urlUserinfoPassword(' mysql://u:p@h:3306/db')).toBe('p'); + expect(urlUserinfoPassword(`${TAB}postgresql://u:p@h/db`)).toBe('p'); + expect(urlUserinfoPassword(`${NUL}mongodb://u:p@h/db${LF}`)).toBe('p'); + expect(urlUserinfoPassword(' //u:p@h/db ')).toBe('p'); + // CONTROL: the same strings with no password, and a bare word, carry none. + expect(urlUserinfoPassword(' mysql://u@h:3306/db')).toBeUndefined(); + expect(urlUserinfoPassword(`${TAB}postgresql://h/db${LF}`)).toBeUndefined(); + expect(urlUserinfoPassword(' a:b@c ')).toBeUndefined(); + }); + it('userinfo ends at the LAST `@`, so a malformed literal-`@` password is caught whole', () => { // Mirrors the read-path redactor's boundary (`redactUrlPassword`, now in // this package's `data/datasource-credential-redaction.ts` — #8300): a URL diff --git a/packages/spec/src/integration/connector-credential-redaction.test.ts b/packages/spec/src/integration/connector-credential-redaction.test.ts index e6ab490e1bf..ec284834169 100644 --- a/packages/spec/src/integration/connector-credential-redaction.test.ts +++ b/packages/spec/src/integration/connector-credential-redaction.test.ts @@ -215,18 +215,27 @@ describe('the connector redactor — credentials inside a URL under `providerCon expect(served.auth).toEqual({ type: 'api-key', credentialRef: 'VENDOR_KEY', paramName: 'appid' }); }); - it('a URL with no credential part, and a string that is not a URL, are untouched', () => { + it('a URL with no credential part, and a string that is not a URL holding none, are untouched', () => { const item = { ...base, providerConfig: { baseUrl: 'https://api.vendor.example/v1?region=eu', - note: `call ?token=${P(38)} by hand`, + note: 'call the regional endpoint by hand', include: ['search'], }, }; expect(redactConnectorItem(item)).toEqual({ item, redactedKeys: [] }); }); + it('a credential query parameter is rewritten out of any string, a string that is not a URL included', () => { + const { item: served, redactedKeys } = at({ + baseUrl: 'https://api.vendor.example/v1?region=eu', + note: `call ?page=2&token=${P(38)}`, + }); + expect(redactedKeys).toEqual(['providerConfig.note']); + expect((served.providerConfig as Record).note).toBe('call ?page=2'); + }); + it('withholds whole an array holding a URL with credential parts, at any depth', () => { const { item: served, redactedKeys } = at({ servers: [`https://u:${P(39)}@a.example`, 'https://b.example'], diff --git a/packages/spec/src/integration/connector-entry-url-credentials.test.ts b/packages/spec/src/integration/connector-entry-url-credentials.test.ts index 21b8ac99bb5..c017da893e4 100644 --- a/packages/spec/src/integration/connector-entry-url-credentials.test.ts +++ b/packages/spec/src/integration/connector-entry-url-credentials.test.ts @@ -224,3 +224,100 @@ describe('an authored connector URL holds no credential part', () => { }); }); }); + +// One value-level rule at both connector doors: a URL is judged where a URL +// parser reads it — after its leading and trailing C0 control characters and +// spaces are trimmed (the WHATWG URL Standard's first step) — and a credential +// query parameter is refused in ANY string under `providerConfig` and `auth`, +// whether or not it starts with a scheme. The read rewrites exactly what the +// write refuses, at the position the write judged, and serves every other +// character byte for byte. +describe('a URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string of the entry', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const CR = String.fromCharCode(0x0d); + const NUL = String.fromCharCode(0x00); + const CLEAN = 'https://api.vendor.example.com/v1'; + const withNote = (note: string) => ({ ...rest(CLEAN), providerConfig: { baseUrl: CLEAN, note } }); + + describe('REFUSED, located at the string', () => { + it.each([ + ['a userinfo password in a URL behind a leading space', rest(` https://user:${PLACEHOLDER}@api.vendor.example.com/v1`), 'providerConfig.baseUrl'], + ['a userinfo token in a URL behind a leading tab', rest(`${TAB}https://${PLACEHOLDER}@api.vendor.example.com/v1`), 'providerConfig.baseUrl'], + ['a userinfo in a URL between a leading NUL and a trailing newline', rest(`${NUL}https://u:${PLACEHOLDER}@api.vendor.example.com/v1${LF}`), 'providerConfig.baseUrl'], + ['`api_key` in the query of a URL behind a leading space', rest(` https://api.vendor.example.com/v1?api_key=${PLACEHOLDER}`), 'providerConfig.baseUrl'], + ['`token` in the query of a string with no scheme', rest(`api.vendor.example.com/v1?page=2&token=${PLACEHOLDER}`), 'providerConfig.baseUrl'], + ['`signature` in the query of a string that is not a URL', withNote(`see ?signature=${PLACEHOLDER}`), 'providerConfig.note'], + [ + 'the instance\'s own `auth.paramName` in the query of a string with no scheme', + rest(`api.vendor.example.com/v1?vendor_key=${PLACEHOLDER}`, { type: 'api-key', credentialRef: 'VENDOR_KEY', paramName: 'vendor_key' }), + 'providerConfig.baseUrl', + ], + ['a userinfo in `auth.tokenUrl` behind a leading space', oauth2({ tokenUrl: ` https://c:${PLACEHOLDER}@auth.vendor.example.com/token` }), 'auth.tokenUrl'], + ['`client_secret` in the query of an `auth.tokenUrl` behind a leading tab', oauth2({ tokenUrl: `${TAB}https://auth.vendor.example.com/token?client_secret=${PLACEHOLDER}` }), 'auth.tokenUrl'], + ] as const)('%s', (_label, entry, at) => { + const { located, messages } = refusals(entry); + expect(located).toEqual([['custom', at]]); + expect(messages[0]).toContain(`\`${at}\``); + expect(messages[0]).toContain('auth: { type, credentialRef }'); + expect(messages[0]).not.toContain(PLACEHOLDER); + }); + }); + + describe('CONTROL: a string holding no credential part is accepted, surrounding whitespace or not', () => { + it.each([ + ['a URL behind a leading space', rest(` ${CLEAN}`)], + ['a URL with an ordinary query between a leading tab and a trailing carriage return', rest(`${TAB}${CLEAN}?region=eu${CR}`)], + ['a string with no scheme and an ordinary query', rest('api.vendor.example.com/v1?region=eu')], + ['a string with no scheme whose credential parameter is empty', rest('api.vendor.example.com/v1?token=')], + ['a string that is not a URL', withNote('Billing API, EU region')], + ['a string that is not a URL whose credential parameter is empty', withNote('for the next page append ?page=2&token=')], + ] as const)('%s', (_label, entry) => accepts(entry)); + }); + + describe('the read rewrites exactly what the write refuses, at the position the write judged', () => { + it.each([ + [` https://user:${PLACEHOLDER}@api.vendor.example.com/v1`, ' https://api.vendor.example.com/v1'], + [`${TAB}https://${PLACEHOLDER}@api.vendor.example.com/v1?page=2${LF}`, `${TAB}https://api.vendor.example.com/v1?page=2${LF}`], + [` https://api.vendor.example.com/v1?api_key=${PLACEHOLDER}`, ' https://api.vendor.example.com/v1'], + [` https://u:${PLACEHOLDER}@api.vendor.example.com/v1?region=eu&token=${PLACEHOLDER} `, ' https://api.vendor.example.com/v1?region=eu'], + [`api.vendor.example.com/v1?page=2&token=${PLACEHOLDER}`, 'api.vendor.example.com/v1?page=2'], + ] as const)('%j is served as %j', (stored, servedUrl) => { + const entry = rest(stored); + expect(refusals(entry).located).toEqual([['custom', 'providerConfig.baseUrl']]); + const { item: served, redactedKeys } = redactConnectorItem(entry); + expect(redactedKeys).toEqual(['providerConfig.baseUrl']); + expect((served.providerConfig as Record).baseUrl).toBe(servedUrl); + // The served form is what the write door accepts: nothing it judges is left. + accepts(served); + }); + + it('a credential query parameter in a string that is not a URL is rewritten the same way', () => { + const entry = withNote(`see ?page=2&signature=${PLACEHOLDER}`); + const { item: served, redactedKeys } = redactConnectorItem(entry); + expect(redactedKeys).toEqual(['providerConfig.note']); + expect((served.providerConfig as Record).note).toBe('see ?page=2'); + accepts(served); + }); + + // `auth` endpoints are URLs by their own declared format, so a string with + // no scheme is judged at a `providerConfig` position only. + it.each([ + [` ${CLEAN}`, true], + [`${TAB}${CLEAN}?region=eu${CR}`, true], + [` https://user:${PLACEHOLDER}@api.vendor.example.com/v1`, true], + [`${NUL}https://${PLACEHOLDER}@api.vendor.example.com/v1`, true], + [` https://api.vendor.example.com/v1?apikey=${PLACEHOLDER}`, true], + ['api.vendor.example.com/v1?token=', false], + [`api.vendor.example.com/v1?token=${PLACEHOLDER}`, false], + [`see ?page=2&token=${PLACEHOLDER}`, false], + ] as const)('refused ⇔ rewritten: %j', (value, hasScheme) => { + const entries = [rest(value), withNote(value), ...(hasScheme ? [oauth2({ tokenUrl: value })] : [])]; + for (const entry of entries) { + const refused = !DeclarativeConnectorEntrySchema.safeParse(entry).success; + const rewritten = redactConnectorItem(entry as Record).redactedKeys.length > 0; + expect(refused, 'refused ⇔ rewritten').toBe(rewritten); + } + }); + }); +}); From 73b183b6dc89fb39373841f69ec71f449d37755a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 08:06:21 +0000 Subject: [PATCH 3/7] docs(spec): the ADR-0087 D3 entry and changeset for the trimmed-URL and any-string credential rules Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- ...ged-trimmed-and-in-any-connector-string.md | 33 +++++++++++++++++++ ...ged-trimmed-and-in-any-connector-string.ts | 33 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 .changeset/url-credential-judged-trimmed-and-in-any-connector-string.md create mode 100644 packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts diff --git a/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md b/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md new file mode 100644 index 00000000000..cf4a32a24a8 --- /dev/null +++ b/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md @@ -0,0 +1,33 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec)!: a URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string of a connector entry + +Clause-②: no (narrowing) +Metadata-protocol: narrowed + + + +**BREAKING** accept-set narrowing at the two metadata doors that judge a URL credential: an authored connector instance (`DeclarativeConnectorEntrySchema` — `defineStack({ connectors })`, `os validate` / `os compile`, the artifact load, `PUT /api/v1/meta/connector/:name`) and a datasource driver config's URL keys (`config.url` on postgres, mysql, mongo and turso, turso `config.syncUrl`) at every door that parses the datasource. + +- **One value-level rule at both doors.** A URL is judged where a URL parser reads it: after its leading and trailing C0 control characters and spaces are trimmed. A connector URL holding a userinfo, and a datasource URL holding a userinfo password, are refused whatever surrounds them; a bare username in a datasource URL stays accepted. +- **A credential query parameter is refused in any string of a connector entry.** The connector query rule is the single value-level parse the datasource doors already run, applied to every string under `providerConfig` and `auth` — whether or not it starts with a scheme. Same names as before (`api_key`, `token`, `access_token`, `key`, `secret`, `signature` and the rest of the connector list, matched case-insensitively on the decoded name, plus the instance's own `auth.paramName`), same located refusal (`422 INVALID_METADATA` at the save door, one issue per string, naming the parts, never the value). +- **Read.** The connector read redactor and the datasource read redactor rewrite exactly what the write refuses, at the position the write judged, and serve every other character as stored. The datasource read locates a userinfo password by the write door's own boundary parse. +- **Unchanged.** `ConnectorSchema` (a plugin's `registerConnector` def) keeps its posture; a header or environment map under `providerConfig` is still judged by neither door; a stored row is never re-judged: a save of its served form keeps the stored value, and an edit of the served value stores exactly the edit. + +## FROM → TO + +```ts +// before — parsed green; now refused at providerConfig.note +{ name: 'billing_api', label: 'Billing API', type: 'api', provider: 'rest', + providerConfig: { baseUrl: 'https://api.billing.example.com/v1', note: 'sandbox: append ?api_key=… to try it' }, + auth: { type: 'none' } }, + +// after — the note holds no credential, and the key is a reference +{ name: 'billing_api', label: 'Billing API', type: 'api', provider: 'rest', + providerConfig: { baseUrl: 'https://api.billing.example.com/v1', note: 'sandbox: the key is BILLING_API_KEY' }, + auth: { type: 'api-key', credentialRef: 'BILLING_API_KEY', paramName: 'api_key' } }, +``` + +For a datasource, keep the URL credential-free (`mysql://app@db.internal:3306/app`) and bind the password through the Setup → Datasources connection form's secret field, or reference it with `external.credentialsRef`. There is deliberately no automatic rewrite: the value in the string may be the only copy of the secret. The upgrade guide entry is `url-credential-judged-trimmed-and-in-any-connector-string`. diff --git a/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts b/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts new file mode 100644 index 00000000000..51e47b1e665 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts @@ -0,0 +1,33 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'url-credential-judged-trimmed-and-in-any-connector-string', + surface: 'the URL credential rule at the two metadata doors that judge one: connector entries AUTHORED through ' + + 'defineStack `connectors:`, `os validate` / `os compile`, the artifact load and `PUT /meta/connector/:name` — ' + + 'every string under `providerConfig` and `auth` — and a datasource driver config\'s URL keys (`config.url` on ' + + 'postgres, mysql, mongo and turso, and turso `config.syncUrl`) at every door that parses the datasource', + replacement: 'keep every authored URL credential-free, whatever surrounds it, and keep a credential query ' + + 'parameter out of every connector string, URL or not: for a connector, name the secret with `auth: { type, ' + + 'credentialRef }` and set its value under that name; for a datasource, bind the secret through the Setup → ' + + 'Datasources connection form\'s secret field (or reference it with `external.credentialsRef`) and keep a bare ' + + 'username in the URL if the driver needs one. Delete the credential from the string.', + reason: + 'A URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string ' + + 'of the entry. A URL parser trims the leading and trailing C0 control characters and spaces of a string before ' + + 'it reads it, so the userinfo rule now reads the string where the parser does, at both doors: a connector URL ' + + 'holding a userinfo, and a datasource URL holding a userinfo password, are refused whatever whitespace or ' + + 'control characters surround them (a bare username in a datasource URL stays accepted). And the connector ' + + 'entry\'s credential query parameter rule is one value-level parse — the one the datasource doors already run — ' + + 'applied to every string under `providerConfig` and `auth`, whether or not it starts with a scheme. Both read ' + + 'redactors rewrite exactly what the write refuses, at the position the write judged, and serve every other ' + + 'character as stored. There is deliberately no mechanical rewrite: the value in the string may be the only copy ' + + 'of the secret. A row stored before this refusal keeps working: a save of the served form keeps the stored ' + + 'value, and an edit of the served value stores the edit.', + acceptanceCriteria: + 'Every authored connector entry parses through `DeclarativeConnectorEntrySchema` with no issue located at a ' + + 'string under `providerConfig` or `auth`, and every authored datasource parses through `DatasourceSchema` with ' + + 'no issue at `config.url` or `config.syncUrl`; each secret that sat in such a string is set under its reference ' + + 'instead, and the connector still materializes (`GET /api/v1/automation/connectors` reports it `ready`).', +}; From d7d0531a3bf242f57319f3f03aa5166b052d8604 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 08:08:23 +0000 Subject: [PATCH 4/7] test(dogfood): pin the trimmed-URL and any-string credential rules at the connector and datasource save doors Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- ...a-door-url-and-nested-keys.dogfood.test.ts | 86 ++++++++++++ ...oor-url-credential-trimmed.dogfood.test.ts | 132 ++++++++++++++++++ 2 files changed, 218 insertions(+) create mode 100644 packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts diff --git a/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts b/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts index ce6b510bf3b..174fe0eac55 100644 --- a/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts +++ b/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts @@ -28,6 +28,13 @@ // same row is kept (CONTROL). A `retryConfig` the author leaves out is // removed with its stored keys, as before (CONTROL), and one stored as a // non-object is still kept at rest by a save that leaves it out (CONTROL). +// - ONE VALUE-LEVEL RULE: a URL is judged after trimming its leading and +// trailing C0 control characters and spaces, where a URL parser reads it, +// and a credential query parameter is refused in any string under +// `providerConfig` and `auth`, scheme or none. The read rewrites exactly +// that, at the position the write door judged, and a save of the served +// form keeps the stored values; a credential-free URL with surrounding +// whitespace saves and dispatches (CONTROL). // // Every credential below is an obviously-fake placeholder, and every assertion // on one is its absence from what a door served or stored, or its presence at @@ -412,4 +419,83 @@ describe('connector metadata doors: an authored connector keeps no credential in expect(atRest.retryConfig).toBe(PLACEHOLDERS.nested); }); }); + + describe('one value-level rule: a URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const TRIM_ROW = 'cun_trim_stored'; + const TRIM_CONTROL = 'cun_trim_control'; + const storedTokenUrl = () => ` https://cun:${PLACEHOLDERS.authEndpoint}@auth.vendor.example.com/token${LF}`; + const storedNote = () => `sandbox ?region=eu&token=${PLACEHOLDERS.urlQuery}`; + + beforeAll(async () => { + // A row stored before the refusal: a clean save, then the system context + // writes the credential parts over it. + const res = await stack.apiAs(admin, 'PUT', `/meta/connector/${TRIM_ROW}`, oauth2(TRIM_ROW)); + expect(res.status, await res.clone().text()).toBe(200); + const [row] = await stored(TRIM_ROW); + const seeded = { + ...oauth2(TRIM_ROW, { tokenUrl: storedTokenUrl() }), + providerConfig: { baseUrl: `${upstreamUrl}/v1`, note: storedNote() }, + }; + await ql.update('sys_metadata', { id: row.id, metadata: JSON.stringify(seeded) }, { context: SYSTEM }); + }); + + describe('WRITE: refused at the string, and nothing is stored', () => { + const cases: Array<[label: string, name: string, body: () => Record, at: string]> = [ + ['a userinfo password in `providerConfig.baseUrl` behind a leading space', 'cun_t_userinfo', + () => instance('cun_t_userinfo', { providerConfig: { baseUrl: ` https://u:${PLACEHOLDERS.urlPassword}@api.vendor.example.com/v1` } }), 'providerConfig.baseUrl'], + ['`api_key` in the `providerConfig.baseUrl` query behind a leading tab', 'cun_t_api_key', + () => instance('cun_t_api_key', { providerConfig: { baseUrl: `${TAB}${upstreamUrl}/v1?api_key=${PLACEHOLDERS.urlQuery}` } }), 'providerConfig.baseUrl'], + ['`token` in the query of a `providerConfig.baseUrl` with no scheme', 'cun_t_no_scheme', + () => instance('cun_t_no_scheme', { providerConfig: { baseUrl: `api.vendor.example.com/v1?token=${PLACEHOLDERS.urlQuery}` } }), 'providerConfig.baseUrl'], + ['a credential query parameter in a `providerConfig` string that is not a URL', 'cun_t_note', + () => instance('cun_t_note', { providerConfig: { baseUrl: `${upstreamUrl}/v1`, note: `sandbox ?token=${PLACEHOLDERS.urlQuery}` } }), 'providerConfig.note'], + ['a userinfo in `auth.tokenUrl` behind a leading space', 'cun_t_token_url', + () => oauth2('cun_t_token_url', { tokenUrl: ` https://c:${PLACEHOLDERS.authEndpoint}@auth.vendor.example.com/token` }), 'auth.tokenUrl'], + ]; + + it.each(cases)('%s', async (_label, name, body, at) => { + await expectRefused(name, body(), at); + expect(await stored(name), `${name}: nothing stored`).toEqual([]); + }); + }); + + it('CONTROL: a credential-free URL behind a leading space, and a string that is not a URL holding none, save; the instance materializes ready and dispatches', async () => { + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/${TRIM_CONTROL}`, instance(TRIM_CONTROL, { + providerConfig: { baseUrl: ` ${upstreamUrl}/v1?region=eu`, note: 'sandbox ?region=eu&token=' }, + })); + expect(put.status, await put.clone().text()).toBe(200); + await waitReady(TRIM_CONTROL); + const handler = automation.resolveConnectorAction(TRIM_CONTROL, 'request'); + expect(handler, `${TRIM_CONTROL} is materialized with its request action`).toBeTypeOf('function'); + const before = received.length; + const out = await handler({ method: 'GET', path: '/probe' }, {}); + expect(out?.status, JSON.stringify(out)).toBe(200); + expect(received.length).toBe(before + 1); + expect(received[received.length - 1]).toBe('/v1/probe?region=eu'); + }); + + it('READ: the stored row is served with each credential part removed at the position the write door judged, every other character as stored', async () => { + for (const token of [member, admin]) { + const { text, item } = await read(token, TRIM_ROW); + expect(holdsAPlaceholder(text)).toBe(false); + expect(item.auth.tokenUrl).toBe(` https://auth.vendor.example.com/token${LF}`); + expect(item.providerConfig.note).toBe('sandbox ?region=eu'); + } + const atRest = bodyOf((await stored(TRIM_ROW))[0]); + expect(atRest.auth.tokenUrl, 'the stored row is never redacted').toBe(storedTokenUrl()); + expect(atRest.providerConfig.note).toBe(storedNote()); + }); + + it('CARRY-FORWARD: a save of the served form keeps each stored value', async () => { + const served = authorable((await read(admin, TRIM_ROW)).item); + const res = await stack.apiAs(admin, 'PUT', `/meta/connector/${TRIM_ROW}`, { ...served, label: 'CUN trim (edited)' }); + expect(res.status, await res.clone().text()).toBe(200); + const atRest = bodyOf((await stored(TRIM_ROW))[0]); + expect(atRest.label).toBe('CUN trim (edited)'); + expect(atRest.auth.tokenUrl).toBe(storedTokenUrl()); + expect(atRest.providerConfig.note).toBe(storedNote()); + }); + }); }); diff --git a/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts b/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts new file mode 100644 index 00000000000..99c024a2a0a --- /dev/null +++ b/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The datasource metadata door `PUT /api/v1/meta/datasource/:name` and its +// read `GET /api/v1/meta/datasource/:name`, on a booted stack: a driver +// config URL is judged after trimming its leading and trailing C0 control +// characters and spaces — where a URL parser reads it — at both doors: +// +// - WRITE: a `config.url` holding a userinfo password is refused with the +// `422 INVALID_METADATA` envelope located at `config.url`, whatever +// surrounds it, and nothing is stored. +// - CONTROL: a bare username in the same position, and a credential-free +// URL, save and are stored exactly as written. +// - READ: a stored row whose URL holds a userinfo password (seeded through +// the system context, the only way such a row is written now) is served +// with the password removed at the position the write door judges, every +// other character — the surrounding ones included — as stored. The stored +// row is never touched: redaction is a serving act. +// +// Every credential below is an obviously-fake placeholder, and every +// assertion on one is its absence from what a door served or stored, or its +// presence at rest on a seeded row. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; + +const fixture = defineStack({ + manifest: { + id: 'com.dogfood.datasource-url-credential-trimmed', namespace: 'duct', version: '0.0.0', type: 'app', + name: 'Datasource URL credential fixture', description: 'An app with one object, for its metadata door.', + }, + objects: [ + ObjectSchema.create({ + name: 'duct_note', label: 'Note', pluralLabel: 'Notes', + fields: { name: Field.text({ label: 'Name', required: true }) }, + }), + ], +} as never); + +const PLACEHOLDER = 'TEST_PLACEHOLDER_DUCT_URL_PASSWORD'; +const TAB = String.fromCharCode(0x09); +const LF = String.fromCharCode(0x0a); +const SYS = { isSystem: true }; +const SEEDED = 'duct_seeded_row'; + +type Answer = { status: number; json: any; text: string }; + +describe('datasource metadata door: a driver config URL is judged after trimming surrounding whitespace, at the write and at the read', () => { + let stack: VerifyStack; + let ql: any; + let token = ''; + + const call = async (method: string, path: string, payload?: unknown): Promise => { + const res = await stack.apiAs(token, method, path, payload); + const text = await res.text(); + let json: any = {}; + try { json = JSON.parse(text); } catch { /* not JSON */ } + return { status: res.status, json, text }; + }; + const storedRows = async (name: string): Promise>> => + ql.find('sys_metadata', { where: { type: 'datasource', name }, context: SYS }); + const storedBody = async (name: string): Promise> => { + const [row] = await storedRows(name); + const raw = row?.metadata; + return (typeof raw === 'string' ? JSON.parse(raw) : raw) as Record; + }; + const body = (name: string, driver: string, url: string) => ({ name, label: `DUCT ${name}`, driver, config: { url } }); + + beforeAll(async () => { + stack = await bootStack(fixture); + token = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + // A row stored before the refusal: a clean save, then the system context + // writes the password over it. + const clean = await call('PUT', `/meta/datasource/${SEEDED}`, body(SEEDED, 'mysql', ' mysql://svc@db.internal:3306/app')); + expect(clean.status, clean.text).toBe(200); + const [row] = await storedRows(SEEDED); + const seeded = body(SEEDED, 'mysql', ` mysql://svc:${PLACEHOLDER}@db.internal:3306/app${LF}`); + await ql.update('sys_metadata', { id: row!.id, metadata: JSON.stringify(seeded) }, { context: SYS }); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + }); + + describe('WRITE: a URL holding a userinfo password is refused whatever surrounds it, and nothing is stored', () => { + it.each([ + ['a mysql `config.url` behind a leading space', 'duct_w_mysql', 'mysql', ` mysql://svc:${PLACEHOLDER}@db.internal:3306/app`], + ['a postgres `config.url` behind a leading tab', 'duct_w_postgres', 'postgres', `${TAB}postgresql://svc:${PLACEHOLDER}@db.internal:5432/app`], + ['a mongo `config.url` between a leading space and a trailing newline', 'duct_w_mongo', 'mongodb', ` mongodb://svc:${PLACEHOLDER}@db.internal:27017/app${LF}`], + ])('%s', async (_label, name, driver, url) => { + const answer = await call('PUT', `/meta/datasource/${name}`, body(name, driver, url)); + expect(answer.status, answer.text.slice(0, 300)).toBe(422); + expect(answer.json.code ?? answer.json.error?.code).toBe('INVALID_METADATA'); + const issues: any[] = answer.json.issues ?? answer.json.error?.issues ?? []; + const atUrl = issues.filter((i) => (Array.isArray(i.path) ? i.path.join('.') : i.path) === 'config.url'); + expect(atUrl.some((i) => String(i.message).includes('embeds a password in its userinfo')), answer.text.slice(0, 600)).toBe(true); + expect(answer.text.includes(PLACEHOLDER), 'the refusal echoes no credential').toBe(false); + expect(await storedRows(name), `${name}: nothing stored`).toEqual([]); + }); + }); + + describe('CONTROL: a bare username or a credential-free URL saves and is stored as written', () => { + it.each([ + ['a bare username behind a leading space', 'duct_c_bare_user', 'mysql', ' mysql://svc@db.internal:3306/app'], + ['a credential-free URL between a leading tab and a trailing newline', 'duct_c_clean', 'mysql', `${TAB}mysql://db.internal:3306/app${LF}`], + ])('%s', async (_label, name, driver, url) => { + const answer = await call('PUT', `/meta/datasource/${name}`, body(name, driver, url)); + expect(answer.status, answer.text.slice(0, 300)).toBe(200); + expect((await storedBody(name)).config.url).toBe(url); + }); + }); + + describe('READ: a stored URL\'s password is withheld at the position the write door judges', () => { + it('guard the guard: the stored row holds the seeded password', async () => { + expect((await storedBody(SEEDED)).config.url).toBe(` mysql://svc:${PLACEHOLDER}@db.internal:3306/app${LF}`); + }); + + it('the item read serves the URL with the password removed and every other character as stored', async () => { + const read = await call('GET', `/meta/datasource/${SEEDED}`); + expect(read.status, read.text.slice(0, 300)).toBe(200); + expect(read.text.includes(PLACEHOLDER), 'the read withholds the password').toBe(false); + const item = (read.json.item ?? read.json.data?.item ?? read.json.data ?? read.json) as Record; + expect(item.config.url).toBe(` mysql://svc@db.internal:3306/app${LF}`); + }); + + it('the stored row is never redacted: redaction is a serving act', async () => { + expect((await storedBody(SEEDED)).config.url).toContain(PLACEHOLDER); + }); + }); +}); From e05109d51636b58e8141e332108509c835b22ac1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:19:35 +0000 Subject: [PATCH 5/7] feat(spec): judge a URL as a URL parser cleans it, deleting tab and newline characters too; one userinfo grammar at both doors Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .../data/datasource-credential-redaction.ts | 22 ++-- packages/spec/src/data/driver/common.zod.ts | 22 ++-- packages/spec/src/data/driver/url-boundary.ts | 122 +++++++++++++----- .../connector-credential-redaction.ts | 15 ++- .../integration/connector-url-credentials.ts | 48 +++---- 5 files changed, 142 insertions(+), 87 deletions(-) diff --git a/packages/spec/src/data/datasource-credential-redaction.ts b/packages/spec/src/data/datasource-credential-redaction.ts index 7aaf44ac281..a4614a7eb84 100644 --- a/packages/spec/src/data/datasource-credential-redaction.ts +++ b/packages/spec/src/data/datasource-credential-redaction.ts @@ -397,9 +397,11 @@ export function redactableConfigKeys(driver: unknown): string[] { * The userinfo is located by the write door's own boundary parse * (`urlUserinfoSpan` in `driver/url-boundary.ts`, which `urlUserinfoPassword` * in `driver/common.zod.ts` reads too), so the read rewrites exactly what the - * write refuses: the string is judged trimmed of its leading and trailing C0 - * control characters and spaces, as a URL parser reads it; a scheme-relative - * `//` authority counts; the userinfo ends at the LAST `@` in the authority — + * write refuses: the string is judged as a URL parser reads it — trimmed of + * its leading and trailing C0 control characters and spaces, with every ASCII + * tab, LF and CR deleted — and rewritten at the matching offsets of the + * authored string; a scheme-relative `//` authority counts; the userinfo ends + * at the LAST `@` in the authority — * a lazier boundary stopping at the first `@` would split * `postgres://u:p@ss@host/db` after `p`, leave `ss@host` in place, and publish * a fragment of the password while looking redacted (such a URL is malformed — @@ -415,10 +417,8 @@ export function redactableConfigKeys(driver: unknown): string[] { */ export function redactUrlPassword(value: string): string { const span = urlUserinfoSpan(value); - if (span === undefined) return value; - const colon = value.indexOf(':', span.start); - if (colon === -1 || colon >= span.end) return value; - return `${value.slice(0, colon)}${value.slice(span.end)}`; + if (span === undefined || span.colon === -1) return value; + return `${value.slice(0, span.colon)}${value.slice(span.end)}`; } /** @@ -430,9 +430,11 @@ export function redactUrlPassword(value: string): string { * `credentialQueryParamOf` (`driver/common.zod.ts`), so the two doors share * ONE boundary definition instead of the comment-level alignment the userinfo * pair needed while they lived in different packages. Consequences it - * inherits: names match case-insensitively on the percent-decoded key, and an - * EMPTY value carries no secret and is left in place (the query twin of - * `user:@host`, which `redactUrlPassword` also leaves). + * inherits: every ASCII tab, LF and CR is deleted from a pair before it is + * read, as a URL parser deletes them; names match case-insensitively on the + * percent-decoded key; and an EMPTY value carries no secret and is left in + * place (the query twin of `user:@host`, which `redactUrlPassword` also + * leaves). * * The whole pair is removed rather than masked: a masked token would * round-trip through the wizard as a literal new credential and then be diff --git a/packages/spec/src/data/driver/common.zod.ts b/packages/spec/src/data/driver/common.zod.ts index 60f817998d8..87688843321 100644 --- a/packages/spec/src/data/driver/common.zod.ts +++ b/packages/spec/src/data/driver/common.zod.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; -import { urlUserinfoSpan } from './url-boundary'; +import { urlUserinfoSpan, withoutAsciiTabOrNewline } from './url-boundary'; /** * Shared building blocks for the per-driver `datasource.config` shapes. @@ -196,9 +196,10 @@ export const URL_EMBEDDED_CREDENTIAL_REFUSED = (key: string): string => * userinfo boundary parse the read-path redactor (`redactUrlPassword` in this * package's `data/datasource-credential-redaction.ts`) rewrites at too, so the * write door refuses precisely the material the read door redacts: RFC 3986's - * authority and last-`@` boundaries, read on the string trimmed of its - * leading and trailing C0 control characters and spaces, as a URL parser - * reads it (why WHATWG parsing itself is refused is on that parse). + * authority and last-`@` boundaries, read on the string as a URL parser reads + * it — trimmed of its leading and trailing C0 control characters and spaces, + * with every ASCII tab, LF and CR deleted — and the userinfo answered is that + * cleaned text (why WHATWG parsing itself is refused is on that parse). * * A string with no `//` (sqlite/turso `file:` paths, `:memory:`) has no * authority and answers `undefined`. The FIRST `:` inside the returned @@ -206,8 +207,7 @@ export const URL_EMBEDDED_CREDENTIAL_REFUSED = (key: string): string => * accessors, not here. */ function urlUserinfo(value: string): string | undefined { - const span = urlUserinfoSpan(value); - return span === undefined ? undefined : value.slice(span.start, span.end); + return urlUserinfoSpan(value)?.userinfo; } /** @@ -619,6 +619,9 @@ function decodeQueryKey(raw: string): string { * `URLSearchParams` — what `pg-connection-string` reads — draws the same * lines): * + * - every ASCII tab, LF and CR is deleted from the pair before it is read, + * as a URL parser deletes them from the whole URL (`url-boundary.ts`), so a + * name is judged as the client reads it; * - a pair with no `=`, or an empty value, carries NO secret (`?authToken=` * is the query twin of `user:@host`, which #8082 accepts); * - the key is percent-decoded before matching (`auth%54oken` IS `authToken` @@ -630,9 +633,10 @@ export function credentialQueryParamOf( rawPair: string, params: readonly string[], ): string | undefined { - const splitIdx = rawPair.indexOf('='); - if (splitIdx < 0 || splitIdx === rawPair.length - 1) return undefined; - const key = decodeQueryKey(rawPair.slice(0, splitIdx)).toLowerCase(); + const pair = withoutAsciiTabOrNewline(rawPair); + const splitIdx = pair.indexOf('='); + if (splitIdx < 0 || splitIdx === pair.length - 1) return undefined; + const key = decodeQueryKey(pair.slice(0, splitIdx)).toLowerCase(); return params.find((param) => param.toLowerCase() === key); } diff --git a/packages/spec/src/data/driver/url-boundary.ts b/packages/spec/src/data/driver/url-boundary.ts index ed9fd13d5ba..58bf0e4bb8d 100644 --- a/packages/spec/src/data/driver/url-boundary.ts +++ b/packages/spec/src/data/driver/url-boundary.ts @@ -1,62 +1,112 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Where a URL-ish authored string starts and ends, and where its userinfo + * A URL-ish authored string as a URL parser reads it, and where its userinfo * sits — the boundary parse every URL-credential judge in this package reads. - * Internal: imported by the datasource write door's userinfo accessors - * (`./common.zod.ts`), the datasource read redactor + * Internal: imported by the datasource write door's userinfo accessors and + * query-pair judgement (`./common.zod.ts`), the datasource read redactor * (`../datasource-credential-redaction.ts`) and the connector URL judge * (`../../integration/connector-url-credentials.ts`), never re-exported. * - * ## Surrounding whitespace + * ## The string a URL parser reads * * A URL parser (the WHATWG URL Standard, which `fetch`, `new URL()` and the - * clients built on them follow) strips leading and trailing C0 control - * characters and spaces — U+0000 to U+0020 — before it reads the string. So a - * URL is judged where the parser reads it: on the string trimmed of those - * characters ({@link urlTrimmedBounds}). A judge that read the untrimmed - * string would answer for a value no client ever opens. + * clients built on them follow) cleans a string before it reads it: it trims + * the leading and trailing C0 control characters and spaces — U+0000 to + * U+0020 — and deletes every ASCII tab, LF and CR anywhere in what is left. + * So a URL is judged as the parser reads it: on the cleaned string + * ({@link urlAsParsed}). A judge that read the authored string as written + * would answer for a value no client ever opens. * - * The bounds are offsets into the authored string, not a trimmed copy, so a - * read redactor rewrites the credential part at the position the write door - * judged and leaves every other character, the surrounding ones included, - * byte for byte. + * The cleaned string carries a map back to the authored one, so a read + * redactor rewrites the credential part at the position the write door + * judged — in the authored string — and leaves every other character, the + * surrounding and deleted ones included, byte for byte. */ -/** U+0000 to U+0020: the C0 control characters and space a URL parser strips from both ends. */ +/** U+0000 to U+0020: the C0 control characters and space a URL parser trims from both ends. */ function isC0ControlOrSpace(code: number): boolean { return code <= 0x20; } +/** ASCII tab, LF and CR: what a URL parser deletes from anywhere in a URL. */ +function isAsciiTabOrNewline(code: number): boolean { + return code === 0x09 || code === 0x0a || code === 0x0d; +} + /** - * `[start, end)` of `value` with its leading and trailing C0 control - * characters and spaces left out — the span a URL parser reads (module note). - * `start === end` for a string holding nothing else. + * `value` with every ASCII tab, LF and CR deleted — the parser's deletion + * applied to a piece of a URL (a query pair), where the trim of the whole + * string does not reach. The input itself when it holds none. */ -export function urlTrimmedBounds(value: string): readonly [start: number, end: number] { +export function withoutAsciiTabOrNewline(value: string): string { + let out = ''; + let changed = false; + for (let i = 0; i < value.length; i += 1) { + if (isAsciiTabOrNewline(value.charCodeAt(i))) changed = true; + else out += value[i]; + } + return changed ? out : value; +} + +/** A string as a URL parser reads it, mapped back to the authored string. */ +export interface UrlAsParsed { + /** The cleaned string: trimmed, and with every ASCII tab, LF and CR deleted. */ + readonly text: string; + /** + * For each index of {@link UrlAsParsed.text}, the offset in the authored + * string the character came from; one more entry, at `text.length`, is the + * authored offset where the trimmed span ends. + */ + readonly rawOffsets: readonly number[]; +} + +/** `value` cleaned as a URL parser cleans it (module note), with the offset map back to `value`. */ +export function urlAsParsed(value: string): UrlAsParsed { let start = 0; let end = value.length; while (start < end && isC0ControlOrSpace(value.charCodeAt(start))) start += 1; while (end > start && isC0ControlOrSpace(value.charCodeAt(end - 1))) end -= 1; - return [start, end]; + let text = ''; + const rawOffsets: number[] = []; + for (let i = start; i < end; i += 1) { + if (isAsciiTabOrNewline(value.charCodeAt(i))) continue; + text += value[i]; + rawOffsets.push(i); + } + rawOffsets.push(end); + return { text, rawOffsets }; } /** `scheme://` or a scheme-relative `//`: where an authority starts. */ const AUTHORITY_START_RE = /^(?:[a-z][a-z0-9+.-]*:)?\/\//i; +/** Where the userinfo of an authored string sits, and what a URL parser reads there. */ +export interface UrlUserinfoSpan { + /** Offset in the authored string where the userinfo starts. */ + start: number; + /** Offset in the authored string of the `@` that ends the userinfo. */ + end: number; + /** The userinfo as a URL parser reads it (cleaned; percent-encoding preserved). */ + userinfo: string; + /** Offset in the authored string of the userinfo's FIRST `:`, or `-1` when it has none. */ + colon: number; +} + /** - * `[start, end)` of the userinfo in `value`, or `undefined` when it carries - * none — the ONE userinfo boundary parse of the datasource doors: the write - * door's accessors (`urlUserinfoPassword`, `urlUserinfoUsername`) read the - * userinfo at these offsets, and the read redactor (`redactUrlPassword`) - * rewrites the password at them, so the read rewrites exactly what the write - * refuses. + * The userinfo of `value`, or `undefined` when it carries none — the ONE + * userinfo boundary parse of both metadata doors that judge a URL credential: + * the datasource write door's accessors (`urlUserinfoPassword`, + * `urlUserinfoUsername`) read {@link UrlUserinfoSpan.userinfo}, the datasource + * read redactor (`redactUrlPassword`) rewrites the password at the offsets, + * and the connector entry judges and rewrites the whole userinfo at them — so + * each read rewrites exactly what its write refuses. * * Deliberately NOT `new URL()`: real DSNs take forms WHATWG parsing rejects or * mangles (postgres/mongo multi-host `user:pass@h1:5432,h2:5432/db` throws * `ERR_INVALID_URL`, bare `:memory:`, `file:` paths), and a parse that throws * on the exact inputs it must judge would fail open. The boundaries are - * RFC 3986's, read on the trimmed span ({@link urlTrimmedBounds}): + * RFC 3986's, read on the cleaned string ({@link urlAsParsed}): * * - the authority is what follows `//` (scheme-relative included), up to the * first `/`, `?` or `#` — a `:` or `@` in a path or query is never userinfo; @@ -66,16 +116,22 @@ const AUTHORITY_START_RE = /^(?:[a-z][a-z0-9+.-]*:)?\/\//i; * A string with no `//` (sqlite/turso `file:` paths, `:memory:`) has no * authority and answers `undefined`. */ -export function urlUserinfoSpan(value: string): { start: number; end: number } | undefined { - const [start, end] = urlTrimmedBounds(value); - const url = value.slice(start, end); - const scheme = AUTHORITY_START_RE.exec(url); +export function urlUserinfoSpan(value: string): UrlUserinfoSpan | undefined { + const { text, rawOffsets } = urlAsParsed(value); + const scheme = AUTHORITY_START_RE.exec(text); if (!scheme) return undefined; - const rest = url.slice(scheme[0].length); + const rest = text.slice(scheme[0].length); const authorityEnd = rest.search(/[/?#]/); const authority = authorityEnd === -1 ? rest : rest.slice(0, authorityEnd); const at = authority.lastIndexOf('@'); if (at === -1) return undefined; - const userinfoStart = start + scheme[0].length; - return { start: userinfoStart, end: userinfoStart + at }; + const userinfoStart = scheme[0].length; + const userinfo = authority.slice(0, at); + const colon = userinfo.indexOf(':'); + return { + start: rawOffsets[userinfoStart]!, + end: rawOffsets[userinfoStart + at]!, + userinfo, + colon: colon === -1 ? -1 : rawOffsets[userinfoStart + colon]!, + }; } diff --git a/packages/spec/src/integration/connector-credential-redaction.ts b/packages/spec/src/integration/connector-credential-redaction.ts index 7de3460ace8..5c484a93f1e 100644 --- a/packages/spec/src/integration/connector-credential-redaction.ts +++ b/packages/spec/src/integration/connector-credential-redaction.ts @@ -72,12 +72,13 @@ * `providerConfig.spec` (`openapi`), and any other URL a provider keeps at * an OBJECT depth of `providerConfig`; and the endpoints an instance-auth * kind declares (`auth.tokenUrl`, `auth.authorizationUrl`, `auth.resource`) - * and any URL a kind declares later. The userinfo is judged on a string - * that, trimmed of its leading and trailing C0 control characters and - * spaces as a URL parser trims it, starts with `scheme://`, and is - * rewritten at that trimmed position; a credential query parameter is - * judged in any string. What a credential part is, and the walk that finds - * every string holding one, live in ONE place, + * and any URL a kind declares later. A string is judged as a URL parser + * reads it — trimmed of its leading and trailing C0 control characters + * and spaces, with every ASCII tab, LF and CR deleted: the userinfo of an + * authority that starts at `scheme://` or a scheme-relative `//`, + * rewritten at the matching position of the authored string; and a + * credential query parameter in any string. What a credential part is, + * and the walk that finds every string holding one, live in ONE place, * `./connector-url-credentials.ts`, which the write door's refinement * runs too: an authored URL holding one is refused, so only a row stored * before that refusal can carry one to this read. @@ -187,7 +188,7 @@ export { CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES }; /** * One URL with its userinfo and credential query parameters removed; the input * when there is none. Each half is the write door's own judgement applied: the - * userinfo is removed where the write door judged it, on the trimmed span + * userinfo is removed where the write door judged it * ({@link connectorUrlWithoutUserinfo}), and the query pairs by the parse the * write door's `urlCredentialQueryParams` reads, in any string. */ diff --git a/packages/spec/src/integration/connector-url-credentials.ts b/packages/spec/src/integration/connector-url-credentials.ts index cfdf5fbfb7f..801f6ba4899 100644 --- a/packages/spec/src/integration/connector-url-credentials.ts +++ b/packages/spec/src/integration/connector-url-credentials.ts @@ -25,10 +25,12 @@ * ## The credential parts * * - **the userinfo**, whole: `https://user:pass@host`, and `https://@host`, - * the form several HTTP APIs read as a token. Judged on the string trimmed - * of its leading and trailing C0 control characters and spaces - * (`urlTrimmedBounds`, `data/driver/url-boundary.ts`) — where a URL parser - * reads it — when that trimmed string starts with `scheme://`; + * the form several HTTP APIs read as a token. Located by `urlUserinfoSpan` + * (`data/driver/url-boundary.ts`), the one userinfo grammar the datasource + * doors read too: on the string as a URL parser reads it — trimmed of its + * leading and trailing C0 control characters and spaces, with every ASCII + * tab, LF and CR deleted — whose authority starts at `scheme://` or a + * scheme-relative `//`; * - **each credential query parameter** with a non-empty value, in ANY * judged string, whether or not it starts with a scheme: the names in * {@link CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES}, plus the @@ -36,8 +38,9 @@ * `urlCredentialQueryParams` (`data/driver/common.zod.ts`), the single * value-level parse the datasource doors run too, and a pair is judged by * `credentialQueryParamOf` beside it: the query runs from the first `?` to - * the fragment, names match case-insensitively on the percent-decoded key, - * and an empty value carries nothing. + * the fragment, every ASCII tab, LF and CR is deleted from a pair before it + * is read, names match case-insensitively on the percent-decoded key, and + * an empty value carries nothing. * * ## One traversal, two doors * @@ -52,7 +55,7 @@ */ import { CREDENTIAL_URL_QUERY_PARAM_NAMES, urlCredentialQueryParams } from '../data/driver/common.zod'; -import { urlTrimmedBounds } from '../data/driver/url-boundary'; +import { urlUserinfoSpan } from '../data/driver/url-boundary'; /** * The header and environment maps a provider sends upstream — withheld WHOLE by @@ -96,28 +99,17 @@ export const CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES: readonly string[] = Obj ]); /** - * `scheme://userinfo@` — the whole userinfo. Every class excludes `/?#`, so the - * `@` must sit in the authority; the greedy class backtracks to the LAST `@` - * before the path, the userinfo boundary RFC 3986 defines. Run on the trimmed - * span only ({@link connectorUrlWithoutUserinfo}). - */ -const URL_USERINFO_RE = /^([a-z][a-z0-9+.\-]*:\/\/)[^/?#]*@/i; - -/** - * `value` with its userinfo removed, or `value` itself when it holds none — - * the userinfo half's ONE judgement, which the write door asks (does this - * change the value?) and the read redactor applies, so the read rewrites - * exactly what the write refuses. The userinfo is judged on the span a URL - * parser reads — `value` trimmed of its leading and trailing C0 control - * characters and spaces (`urlTrimmedBounds`) — and removed there, every - * other character of `value`, the surrounding ones included, kept byte for - * byte. + * `value` with its whole userinfo and the `@` that ends it removed, or `value` + * itself when it holds none — the userinfo half's ONE judgement, which the + * write door asks (does this change the value?) and the read redactor applies, + * so the read rewrites exactly what the write refuses. The userinfo is located + * by `urlUserinfoSpan` on the string as a URL parser reads it, and removed at + * the matching offsets of `value`: every other character, the surrounding and + * the parser-deleted ones included, is kept byte for byte. */ export function connectorUrlWithoutUserinfo(value: string): string { - const [start, end] = urlTrimmedBounds(value); - const url = value.slice(start, end); - const rewritten = url.replace(URL_USERINFO_RE, '$1'); - return rewritten === url ? value : `${value.slice(0, start)}${rewritten}${value.slice(end)}`; + const span = urlUserinfoSpan(value); + return span === undefined ? value : `${value.slice(0, span.start)}${value.slice(span.end + 1)}`; } const isPlainRecord = (v: unknown): v is Record => @@ -149,7 +141,7 @@ export interface ConnectorUrlCredentialParts { /** * The credential parts `value` holds, or `undefined` when it holds none: the - * userinfo on the trimmed span ({@link connectorUrlWithoutUserinfo}), and the + * userinfo ({@link connectorUrlWithoutUserinfo}), and the * credential query parameters in any string, scheme or none (see the module * note). */ From f78b7d59af6d6db02c412787e3e905c79abb9c6d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:20:50 +0000 Subject: [PATCH 6/7] test(spec): pin the cleaned-URL rule and the one userinfo grammar at both connector and datasource doors Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .../datasource-credential-redaction.test.ts | 36 +++++++++ .../driver/driver-credential-refusal.test.ts | 55 ++++++++++++++ .../connector-entry-url-credentials.test.ts | 73 +++++++++++++++++++ 3 files changed, 164 insertions(+) diff --git a/packages/spec/src/data/datasource-credential-redaction.test.ts b/packages/spec/src/data/datasource-credential-redaction.test.ts index 209b6cfb91f..5dbc15500cb 100644 --- a/packages/spec/src/data/datasource-credential-redaction.test.ts +++ b/packages/spec/src/data/datasource-credential-redaction.test.ts @@ -306,6 +306,42 @@ describe('the datasource read rewrites a URL\'s userinfo password exactly where expect(redactUrlPassword(url)).toBe(url); }); + // [stored, served]: every ASCII tab, LF and CR is deleted before the URL is + // judged, as a URL parser deletes them; the password is removed at the + // matching offsets of the authored string, and every other character — + // the deleted ones outside the password included — is served as stored. + const CLEANED: Array<[string, string]> = [ + [`postgre${TAB}sql://svc:hunter2@db:5432/app`, `postgre${TAB}sql://svc@db:5432/app`], + [`postgresql:/${LF}/svc:hunter2@db:5432/app`, `postgresql:/${LF}/svc@db:5432/app`], + [`postgresql://s${CR}vc:hun${TAB}ter2@db:5432/app`, `postgresql://s${CR}vc@db:5432/app`], + ]; + + it.each(CLEANED)('%j is served as %j, which the write door accepts', (stored, served) => { + expect(urlUserinfoPassword(stored), 'the write door refuses the stored URL').toBeDefined(); + expect(redactUrlPassword(stored)).toBe(served); + expect(urlUserinfoPassword(served), 'the served URL passes the write door').toBeUndefined(); + expect(urlUserinfoUsername(served), 'the username is kept').toBe(urlUserinfoUsername(stored)); + }); + + it('a credential query parameter whose name holds a tab or newline is removed whole, the rest of the query as stored', () => { + for (const char of [TAB, LF, CR]) { + const stored = `postgresql://svc@db:5432/app?sslmode=require&pass${char}word=hunter2&connect_timeout=10`; + expect(urlCredentialQueryParams(stored, CREDENTIAL_URL_QUERY_PARAM_NAMES)).toEqual(['password']); + const served = redactUrlCredentials(stored); + expect(served).toBe('postgresql://svc@db:5432/app?sslmode=require&connect_timeout=10'); + expect(urlCredentialQueryParams(served, CREDENTIAL_URL_QUERY_PARAM_NAMES)).toEqual([]); + } + }); + + it.each([ + `postgre${TAB}sql://svc@db:5432/app`, + `postgresql://svc:${TAB}@db:5432/app?ssl${LF}mode=require`, + ])('CONTROL: %j holds no credential once cleaned and keeps its query', (url) => { + expect(urlUserinfoPassword(url)).toBeUndefined(); + expect(urlCredentialQueryParams(url, CREDENTIAL_URL_QUERY_PARAM_NAMES)).toEqual([]); + expect(redactUrlCredentialQueryParams(url)).toBe(url); + }); + it('redactDatasourceConfig serves a stored URL behind surrounding whitespace with its password removed, naming `url`', () => { const { config, redactedKeys } = redactDatasourceConfig('mysql', { url: ' mysql://svc:hunter2@db:3306/app' }); expect(config).toEqual({ url: ' mysql://svc@db:3306/app' }); diff --git a/packages/spec/src/data/driver/driver-credential-refusal.test.ts b/packages/spec/src/data/driver/driver-credential-refusal.test.ts index d675c0c7495..42975d4169a 100644 --- a/packages/spec/src/data/driver/driver-credential-refusal.test.ts +++ b/packages/spec/src/data/driver/driver-credential-refusal.test.ts @@ -342,6 +342,25 @@ describe.each(URL_FAMILY)('$name — URL-embedded credential refusal', (f) => { it.each(SURROUNDINGS)('CONTROL: a bare username (`user@host`) behind %s carries no password and is not refused for one', (_label, lead, trail) => { expect(userinfoRefusalAt(f.make(`${lead}${f.sample('svc@')}${trail}`))).toBeUndefined(); }); + + // A URL parser also deletes every ASCII tab, LF and CR anywhere in it. + const INSIDE: Array<[label: string, char: string]> = [ + ['a tab', String.fromCharCode(0x09)], + ['a newline', String.fromCharCode(0x0a)], + ['a carriage return', String.fromCharCode(0x0d)], + ]; + /** `url` with `char` inserted after its scheme's first letter. */ + const insideScheme = (url: string, char: string) => `${url.slice(0, 1)}${char}${url.slice(1)}`; + + it.each(INSIDE)('refuses `user:password@host` with %s inside its scheme, and inside its password', (_label, char) => { + expect(userinfoRefusalAt(f.make(insideScheme(f.sample('svc:hunter2@'), char)))).toBeDefined(); + expect(userinfoRefusalAt(f.make(f.sample(`svc:hun${char}ter2@`)))).toBeDefined(); + }); + + it.each(INSIDE)('CONTROL: a bare username with %s inside the scheme, and a password of only that character, carry no password', (_label, char) => { + expect(userinfoRefusalAt(f.make(insideScheme(f.sample('svc@'), char)))).toBeUndefined(); + expect(userinfoRefusalAt(f.make(f.sample(`svc:${char}@`)))).toBeUndefined(); + }); }); }); @@ -433,6 +452,18 @@ describe.each(QUERY_FAMILY)('$name — URL query-parameter credential refusal', expect(refusalAt(f.make(f.sample(`?${f.param}=\${DB_TOKEN}`)))).toBeDefined(); }); + it('refuses the parameter whose name holds a tab, newline or carriage return — a URL parser deletes them', () => { + for (const code of [0x09, 0x0a, 0x0d]) { + const char = String.fromCharCode(code); + const name = `${f.param.slice(0, 2)}${char}${f.param.slice(2)}`; + expect(refusalAt(f.make(f.sample(`?${name}=hunter2`))), JSON.stringify(name)).toBeDefined(); + } + }); + + it('CONTROL: a value of only tab and newline characters is empty once a URL parser deletes them', () => { + expect(refusalAt(f.make(f.sample(`?${f.param}=${String.fromCharCode(0x09)}${String.fromCharCode(0x0a)}`)))).toBeUndefined(); + }); + it('accepts an EMPTY value — the query twin of `user:@host` carries no secret', () => { expect(refusalAt(f.make(f.sample(`?${f.param}=`)))).toBeUndefined(); }); @@ -500,6 +531,17 @@ describe('urlCredentialQueryParams — the shared value-level parse', () => { expect(urlCredentialQueryParams('libsql://h?auth%zzoken=x', TURSO)).toEqual([]); }); + it('a pair is read with every ASCII tab, LF and CR deleted, as a URL parser deletes them — before percent-decoding', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const CR = String.fromCharCode(0x0d); + expect(urlCredentialQueryParams(`libsql://h?auth${TAB}Token=x`, TURSO)).toEqual(['authToken']); + expect(urlCredentialQueryParams(`libsql://h?tls=1&a${LF}uthToken=x`, TURSO)).toEqual(['authToken']); + expect(urlCredentialQueryParams(`libsql://h?auth%5${CR}4oken=x`, TURSO)).toEqual(['authToken']); + // CONTROL: a value of only such characters is empty once they are deleted. + expect(urlCredentialQueryParams(`libsql://h?authToken=${TAB}${CR}${LF}`, TURSO)).toEqual([]); + }); + it('names match case-insensitively (the at-rest rationale on CREDENTIAL_URL_QUERY_PARAMS)', () => { expect(urlCredentialQueryParams('libsql://h?AUTHTOKEN=x', TURSO)).toEqual(['authToken']); expect(urlCredentialQueryParams('libsql://h?authtoken=x', TURSO)).toEqual(['authToken']); @@ -549,6 +591,19 @@ describe('urlUserinfoPassword — the shared value-level parse', () => { expect(urlUserinfoPassword(' a:b@c ')).toBeUndefined(); }); + it('a URL is judged with every ASCII tab, LF and CR deleted, as a URL parser deletes them', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const CR = String.fromCharCode(0x0d); + expect(urlUserinfoPassword(`postgre${TAB}sql://u:p@h/db`)).toBe('p'); + expect(urlUserinfoPassword(`postgresql:/${LF}/u:p@h/db`)).toBe('p'); + expect(urlUserinfoPassword(`postgresql://u:p${CR}w@h/db`)).toBe('pw'); + expect(urlUserinfoUsername(`mongodb://a${TAB}pp@h/db`)).toBe('app'); + // CONTROL: a password of only such characters is empty, and a bare username carries none. + expect(urlUserinfoPassword(`postgresql://u:${TAB}@h/db`)).toBeUndefined(); + expect(urlUserinfoPassword(`postgre${TAB}sql://u@h/db`)).toBeUndefined(); + }); + it('userinfo ends at the LAST `@`, so a malformed literal-`@` password is caught whole', () => { // Mirrors the read-path redactor's boundary (`redactUrlPassword`, now in // this package's `data/datasource-credential-redaction.ts` — #8300): a URL diff --git a/packages/spec/src/integration/connector-entry-url-credentials.test.ts b/packages/spec/src/integration/connector-entry-url-credentials.test.ts index c017da893e4..1e23e91cce7 100644 --- a/packages/spec/src/integration/connector-entry-url-credentials.test.ts +++ b/packages/spec/src/integration/connector-entry-url-credentials.test.ts @@ -321,3 +321,76 @@ describe('a URL is judged after trimming surrounding whitespace; a credential qu }); }); }); + +// The same rule, as a URL parser cleans a string: it also deletes every ASCII +// tab, LF and CR anywhere in it, so a value is judged with them deleted. And +// one userinfo grammar serves both doors: an authority starts at `scheme://` +// or a scheme-relative `//`. +describe('a URL is judged as a URL parser cleans it: tab and newline characters deleted, and one userinfo grammar at both doors', () => { + const TAB = String.fromCharCode(0x09); + const LF = String.fromCharCode(0x0a); + const CR = String.fromCharCode(0x0d); + const CLEAN = 'https://api.vendor.example.com/v1'; + const withNote = (note: string) => ({ ...rest(CLEAN), providerConfig: { baseUrl: CLEAN, note } }); + + describe('REFUSED, located at the string', () => { + it.each([ + ['a credential query parameter whose name holds a tab', rest(`${CLEAN}?page=2&api${TAB}_key=${PLACEHOLDER}`), 'providerConfig.baseUrl'], + ['a credential query parameter whose name holds a newline, in a string that is not a URL', withNote(`see ?to${LF}ken=${PLACEHOLDER}`), 'providerConfig.note'], + ['a userinfo behind a scheme that holds a tab', rest(`ht${TAB}tps://u:${PLACEHOLDER}@api.vendor.example.com/v1`), 'providerConfig.baseUrl'], + ['a userinfo behind a `//` that holds a carriage return', rest(`https:/${CR}/${PLACEHOLDER}@api.vendor.example.com/v1`), 'providerConfig.baseUrl'], + ['a userinfo in a scheme-relative authority', rest(`//u:${PLACEHOLDER}@api.vendor.example.com/v1`), 'providerConfig.baseUrl'], + ['a userinfo in a scheme-relative authority, in a string that is not the base URL', withNote(`//${PLACEHOLDER}@hooks.vendor.example.com`), 'providerConfig.note'], + ] as const)('%s', (_label, entry, at) => { + const { located, messages } = refusals(entry); + expect(located).toEqual([['custom', at]]); + expect(messages[0]).toContain(`\`${at}\``); + expect(messages[0]).toContain('auth: { type, credentialRef }'); + expect(messages[0]).not.toContain(PLACEHOLDER); + }); + }); + + describe('CONTROL: a value holding no credential part once cleaned is accepted', () => { + it.each([ + ['an ordinary query parameter whose name holds a tab', rest(`${CLEAN}?reg${TAB}ion=eu`)], + ['a credential query parameter whose value is only tab and newline characters', rest(`${CLEAN}?token=${TAB}${LF}`)], + ['a scheme that holds a tab, with no userinfo', rest(`ht${TAB}tps://api.vendor.example.com/v1`)], + ['a scheme-relative authority with no userinfo', rest('//api.vendor.example.com/v1')], + ['an `@` in the path of a scheme-relative string', rest('//api.vendor.example.com/v1/users/@me')], + ] as const)('%s', (_label, entry) => accepts(entry)); + }); + + describe('the read rewrites exactly what the write refuses, at the matching position of the authored string', () => { + it.each([ + [`${CLEAN}?page=2&api${TAB}_key=${PLACEHOLDER}`, `${CLEAN}?page=2`], + [`ht${TAB}tps://u:${PLACEHOLDER}@api.vendor.example.com/v1`, `ht${TAB}tps://api.vendor.example.com/v1`], + [`https:/${CR}/${PLACEHOLDER}@api.vendor.example.com/v1`, `https:/${CR}/api.vendor.example.com/v1`], + [`//u:${PLACEHOLDER}@api.vendor.example.com/v1`, '//api.vendor.example.com/v1'], + [` //u:${PLACEHOLDER}@api.vendor.example.com/v1?to${LF}ken=${PLACEHOLDER}&page=2`, ' //api.vendor.example.com/v1?page=2'], + [`https://u${TAB}:${PLACEHOLDER}@api.vendor.example.com/v1`, CLEAN], + ] as const)('%j is served as %j', (stored, servedUrl) => { + const entry = rest(stored); + expect(refusals(entry).located).toEqual([['custom', 'providerConfig.baseUrl']]); + const { item: served, redactedKeys } = redactConnectorItem(entry); + expect(redactedKeys).toEqual(['providerConfig.baseUrl']); + expect((served.providerConfig as Record).baseUrl).toBe(servedUrl); + accepts(served); + }); + + it.each([ + `${CLEAN}?reg${TAB}ion=eu`, + `${CLEAN}?token=${TAB}`, + `${CLEAN}?api${TAB}_key=${PLACEHOLDER}`, + `ht${TAB}tps://${PLACEHOLDER}@api.vendor.example.com/v1`, + '//api.vendor.example.com/v1', + `//u:${PLACEHOLDER}@api.vendor.example.com/v1`, + `see ?to${CR}ken=${PLACEHOLDER}`, + ])('refused ⇔ rewritten: %j', (value) => { + for (const entry of [rest(value), withNote(value)]) { + const refused = !DeclarativeConnectorEntrySchema.safeParse(entry).success; + const rewritten = redactConnectorItem(entry as Record).redactedKeys.length > 0; + expect(refused, 'refused ⇔ rewritten').toBe(rewritten); + } + }); + }); +}); From 49b7049edb9149a0bec96768e320ad9b86ed3db8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:21:46 +0000 Subject: [PATCH 7/7] test(dogfood): pin the cleaned-URL rule at both save doors; the D3 entry and changeset state it Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- ...ged-trimmed-and-in-any-connector-string.md | 5 +-- ...a-door-url-and-nested-keys.dogfood.test.ts | 28 +++++++++++++++ ...oor-url-credential-trimmed.dogfood.test.ts | 35 ++++++++++++++++++- ...ged-trimmed-and-in-any-connector-string.ts | 19 +++++----- 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md b/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md index cf4a32a24a8..f7dd85ac52a 100644 --- a/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md +++ b/.changeset/url-credential-judged-trimmed-and-in-any-connector-string.md @@ -2,7 +2,7 @@ '@objectstack/spec': minor --- -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 after trimming surrounding whitespace and removing tab and newline characters, as a URL parser does; a credential query parameter is refused in any string of a connector entry Clause-②: no (narrowing) Metadata-protocol: narrowed @@ -11,7 +11,8 @@ Metadata-protocol: narrowed **BREAKING** accept-set narrowing at the two metadata doors that judge a URL credential: an authored connector instance (`DeclarativeConnectorEntrySchema` — `defineStack({ connectors })`, `os validate` / `os compile`, the artifact load, `PUT /api/v1/meta/connector/:name`) and a datasource driver config's URL keys (`config.url` on postgres, mysql, mongo and turso, turso `config.syncUrl`) at every door that parses the datasource. -- **One value-level rule at both doors.** A URL is judged where a URL parser reads it: after its leading and trailing C0 control characters and spaces are trimmed. A connector URL holding a userinfo, and a datasource URL holding a userinfo password, are refused whatever surrounds them; a bare username in a datasource URL stays accepted. +- **One value-level rule at both doors.** A value is judged as a URL parser reads it: after its leading and trailing C0 control characters and spaces are trimmed and every tab, newline and carriage return in it is removed. A connector URL holding a userinfo, and a datasource URL holding a userinfo password, are refused as the parser reads them; a bare username in a datasource URL stays accepted. The same cleaning applies to each query pair before its name is read, at both doors. +- **One userinfo grammar at both doors.** The connector entry locates a userinfo with the grammar the datasource doors already use: an authority starts at `scheme://` or at a scheme-relative `//`. - **A credential query parameter is refused in any string of a connector entry.** The connector query rule is the single value-level parse the datasource doors already run, applied to every string under `providerConfig` and `auth` — whether or not it starts with a scheme. Same names as before (`api_key`, `token`, `access_token`, `key`, `secret`, `signature` and the rest of the connector list, matched case-insensitively on the decoded name, plus the instance's own `auth.paramName`), same located refusal (`422 INVALID_METADATA` at the save door, one issue per string, naming the parts, never the value). - **Read.** The connector read redactor and the datasource read redactor rewrite exactly what the write refuses, at the position the write judged, and serve every other character as stored. The datasource read locates a userinfo password by the write door's own boundary parse. - **Unchanged.** `ConnectorSchema` (a plugin's `registerConnector` def) keeps its posture; a header or environment map under `providerConfig` is still judged by neither door; a stored row is never re-judged: a save of its served form keeps the stored value, and an edit of the served value stores exactly the edit. diff --git a/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts b/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts index 174fe0eac55..9a199548e49 100644 --- a/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts +++ b/packages/qa/dogfood/test/connector-meta-door-url-and-nested-keys.dogfood.test.ts @@ -453,6 +453,12 @@ describe('connector metadata doors: an authored connector keeps no credential in () => instance('cun_t_note', { providerConfig: { baseUrl: `${upstreamUrl}/v1`, note: `sandbox ?token=${PLACEHOLDERS.urlQuery}` } }), 'providerConfig.note'], ['a userinfo in `auth.tokenUrl` behind a leading space', 'cun_t_token_url', () => oauth2('cun_t_token_url', { tokenUrl: ` https://c:${PLACEHOLDERS.authEndpoint}@auth.vendor.example.com/token` }), 'auth.tokenUrl'], + ['`api_key` in the `providerConfig.baseUrl` query, a tab inside its name', 'cun_t_key_tab', + () => instance('cun_t_key_tab', { providerConfig: { baseUrl: `${upstreamUrl}/v1?api${TAB}_key=${PLACEHOLDERS.urlQuery}` } }), 'providerConfig.baseUrl'], + ['a userinfo in `providerConfig.baseUrl`, a tab inside its scheme', 'cun_t_scheme_tab', + () => instance('cun_t_scheme_tab', { providerConfig: { baseUrl: `ht${TAB}tps://u:${PLACEHOLDERS.urlPassword}@api.vendor.example.com/v1` } }), 'providerConfig.baseUrl'], + ['a userinfo in a scheme-relative `providerConfig.baseUrl`', 'cun_t_scheme_relative', + () => instance('cun_t_scheme_relative', { providerConfig: { baseUrl: `//u:${PLACEHOLDERS.urlPassword}@api.vendor.example.com/v1` } }), 'providerConfig.baseUrl'], ]; it.each(cases)('%s', async (_label, name, body, at) => { @@ -476,6 +482,28 @@ describe('connector metadata doors: an authored connector keeps no credential in expect(received[received.length - 1]).toBe('/v1/probe?region=eu'); }); + it('CONTROL: an ordinary query parameter with a tab inside its name saves and dispatches as a URL parser reads it; a scheme-relative URL with no userinfo saves', async () => { + const name = 'cun_trim_tab_control'; + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/${name}`, instance(name, { + providerConfig: { baseUrl: `${upstreamUrl}/v1?reg${TAB}ion=eu` }, + })); + expect(put.status, await put.clone().text()).toBe(200); + await waitReady(name); + const handler = automation.resolveConnectorAction(name, 'request'); + expect(handler, `${name} is materialized with its request action`).toBeTypeOf('function'); + const before = received.length; + const out = await handler({ method: 'GET', path: '/probe' }, {}); + expect(out?.status, JSON.stringify(out)).toBe(200); + expect(received.length).toBe(before + 1); + expect(received[received.length - 1]).toBe('/v1/probe?region=eu'); + // Saved disabled: the pin is the door's answer, not a materialization. + const relative = await stack.apiAs(admin, 'PUT', '/meta/connector/cun_trim_relative_control', instance('cun_trim_relative_control', { + enabled: false, + providerConfig: { baseUrl: '//api.vendor.example.com/v1' }, + })); + expect(relative.status, await relative.clone().text()).toBe(200); + }); + it('READ: the stored row is served with each credential part removed at the position the write door judged, every other character as stored', async () => { for (const token of [member, admin]) { const { text, item } = await read(token, TRIM_ROW); diff --git a/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts b/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts index 99c024a2a0a..5f89ad93809 100644 --- a/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts +++ b/packages/qa/dogfood/test/datasource-meta-door-url-credential-trimmed.dogfood.test.ts @@ -3,7 +3,8 @@ // The datasource metadata door `PUT /api/v1/meta/datasource/:name` and its // read `GET /api/v1/meta/datasource/:name`, on a booted stack: a driver // config URL is judged after trimming its leading and trailing C0 control -// characters and spaces — where a URL parser reads it — at both doors: +// characters and spaces and deleting every tab, newline and carriage return +// in it — as a URL parser reads it — at both doors: // // - WRITE: a `config.url` holding a userinfo password is refused with the // `422 INVALID_METADATA` envelope located at `config.url`, whatever @@ -42,7 +43,9 @@ const PLACEHOLDER = 'TEST_PLACEHOLDER_DUCT_URL_PASSWORD'; const TAB = String.fromCharCode(0x09); const LF = String.fromCharCode(0x0a); const SYS = { isSystem: true }; +const CR = String.fromCharCode(0x0d); const SEEDED = 'duct_seeded_row'; +const SEEDED_PG = 'duct_seeded_pg_row'; type Answer = { status: number; json: any; text: string }; @@ -66,6 +69,7 @@ describe('datasource metadata door: a driver config URL is judged after trimming return (typeof raw === 'string' ? JSON.parse(raw) : raw) as Record; }; const body = (name: string, driver: string, url: string) => ({ name, label: `DUCT ${name}`, driver, config: { url } }); + const storedPgUrl = () => `postgre${TAB}sql://svc:${PLACEHOLDER}@db.internal:5432/app?sslmode=require&pass${CR}word=${PLACEHOLDER}`; beforeAll(async () => { stack = await bootStack(fixture); @@ -78,6 +82,10 @@ describe('datasource metadata door: a driver config URL is judged after trimming const [row] = await storedRows(SEEDED); const seeded = body(SEEDED, 'mysql', ` mysql://svc:${PLACEHOLDER}@db.internal:3306/app${LF}`); await ql.update('sys_metadata', { id: row!.id, metadata: JSON.stringify(seeded) }, { context: SYS }); + const cleanPg = await call('PUT', `/meta/datasource/${SEEDED_PG}`, body(SEEDED_PG, 'postgres', 'postgresql://svc@db.internal:5432/app')); + expect(cleanPg.status, cleanPg.text).toBe(200); + const [pgRow] = await storedRows(SEEDED_PG); + await ql.update('sys_metadata', { id: pgRow!.id, metadata: JSON.stringify(body(SEEDED_PG, 'postgres', storedPgUrl())) }, { context: SYS }); }, 180_000); afterAll(async () => { @@ -89,6 +97,7 @@ describe('datasource metadata door: a driver config URL is judged after trimming ['a mysql `config.url` behind a leading space', 'duct_w_mysql', 'mysql', ` mysql://svc:${PLACEHOLDER}@db.internal:3306/app`], ['a postgres `config.url` behind a leading tab', 'duct_w_postgres', 'postgres', `${TAB}postgresql://svc:${PLACEHOLDER}@db.internal:5432/app`], ['a mongo `config.url` between a leading space and a trailing newline', 'duct_w_mongo', 'mongodb', ` mongodb://svc:${PLACEHOLDER}@db.internal:27017/app${LF}`], + ['a postgres `config.url` with a tab inside its scheme', 'duct_w_pg_scheme_tab', 'postgres', `postgre${TAB}sql://svc:${PLACEHOLDER}@db.internal:5432/app`], ])('%s', async (_label, name, driver, url) => { const answer = await call('PUT', `/meta/datasource/${name}`, body(name, driver, url)); expect(answer.status, answer.text.slice(0, 300)).toBe(422); @@ -101,10 +110,24 @@ describe('datasource metadata door: a driver config URL is judged after trimming }); }); + it('WRITE: a postgres `config.url` whose `password` query parameter holds a tab inside its name is refused, and nothing is stored', async () => { + const name = 'duct_w_pg_key_tab'; + const answer = await call('PUT', `/meta/datasource/${name}`, body(name, 'postgres', `postgresql://svc@db.internal:5432/app?pass${TAB}word=${PLACEHOLDER}`)); + expect(answer.status, answer.text.slice(0, 300)).toBe(422); + expect(answer.json.code ?? answer.json.error?.code).toBe('INVALID_METADATA'); + const issues: any[] = answer.json.issues ?? answer.json.error?.issues ?? []; + const atUrl = issues.filter((i) => (Array.isArray(i.path) ? i.path.join('.') : i.path) === 'config.url'); + expect(atUrl.some((i) => String(i.message).includes('`?password=`')), answer.text.slice(0, 600)).toBe(true); + expect(answer.text.includes(PLACEHOLDER), 'the refusal echoes no credential').toBe(false); + expect(await storedRows(name), `${name}: nothing stored`).toEqual([]); + }); + describe('CONTROL: a bare username or a credential-free URL saves and is stored as written', () => { it.each([ ['a bare username behind a leading space', 'duct_c_bare_user', 'mysql', ' mysql://svc@db.internal:3306/app'], ['a credential-free URL between a leading tab and a trailing newline', 'duct_c_clean', 'mysql', `${TAB}mysql://db.internal:3306/app${LF}`], + ['a postgres bare username with a tab inside the scheme', 'duct_c_pg_scheme_tab', 'postgres', `postgre${TAB}sql://svc@db.internal:5432/app`], + ['a postgres ordinary query parameter with a tab inside its name', 'duct_c_pg_key_tab', 'postgres', `postgresql://svc@db.internal:5432/app?connect${TAB}_timeout=10`], ])('%s', async (_label, name, driver, url) => { const answer = await call('PUT', `/meta/datasource/${name}`, body(name, driver, url)); expect(answer.status, answer.text.slice(0, 300)).toBe(200); @@ -125,8 +148,18 @@ describe('datasource metadata door: a driver config URL is judged after trimming expect(item.config.url).toBe(` mysql://svc@db.internal:3306/app${LF}`); }); + it('a stored postgres URL is served with its password and its `password` query parameter removed where the write door judges them, every other character as stored', async () => { + expect((await storedBody(SEEDED_PG)).config.url, 'guard the guard').toBe(storedPgUrl()); + const read = await call('GET', `/meta/datasource/${SEEDED_PG}`); + expect(read.status, read.text.slice(0, 300)).toBe(200); + expect(read.text.includes(PLACEHOLDER), 'the read withholds both').toBe(false); + const item = (read.json.item ?? read.json.data?.item ?? read.json.data ?? read.json) as Record; + expect(item.config.url).toBe(`postgre${TAB}sql://svc@db.internal:5432/app?sslmode=require`); + }); + it('the stored row is never redacted: redaction is a serving act', async () => { expect((await storedBody(SEEDED)).config.url).toContain(PLACEHOLDER); + expect((await storedBody(SEEDED_PG)).config.url).toBe(storedPgUrl()); }); }); }); diff --git a/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts b/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts index 51e47b1e665..f5c94701bd8 100644 --- a/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts +++ b/packages/spec/src/migrations/entries/semantic/18.url-credential-judged-trimmed-and-in-any-connector-string.ts @@ -8,19 +8,22 @@ export const entry: SemanticMigration = { 'defineStack `connectors:`, `os validate` / `os compile`, the artifact load and `PUT /meta/connector/:name` — ' + 'every string under `providerConfig` and `auth` — and a datasource driver config\'s URL keys (`config.url` on ' + 'postgres, mysql, mongo and turso, and turso `config.syncUrl`) at every door that parses the datasource', - replacement: 'keep every authored URL credential-free, whatever surrounds it, and keep a credential query ' + + replacement: 'keep every authored URL credential-free as a URL parser reads it, and keep a credential query ' + 'parameter out of every connector string, URL or not: for a connector, name the secret with `auth: { type, ' + 'credentialRef }` and set its value under that name; for a datasource, bind the secret through the Setup → ' + 'Datasources connection form\'s secret field (or reference it with `external.credentialsRef`) and keep a bare ' + 'username in the URL if the driver needs one. Delete the credential from the string.', reason: - 'A URL is judged after trimming surrounding whitespace; a credential query parameter is refused in any string ' + - 'of the entry. A URL parser trims the leading and trailing C0 control characters and spaces of a string before ' + - 'it reads it, so the userinfo rule now reads the string where the parser does, at both doors: a connector URL ' + - 'holding a userinfo, and a datasource URL holding a userinfo password, are refused whatever whitespace or ' + - 'control characters surround them (a bare username in a datasource URL stays accepted). And the connector ' + - 'entry\'s credential query parameter rule is one value-level parse — the one the datasource doors already run — ' + - 'applied to every string under `providerConfig` and `auth`, whether or not it starts with a scheme. Both read ' + + 'A URL is judged after trimming surrounding whitespace and removing tab and newline characters, as a URL parser ' + + 'does; a credential query parameter is refused in any string of the entry. A URL parser trims the leading and ' + + 'trailing C0 control characters and spaces of a string and removes every tab, newline and carriage return in it ' + + 'before it reads it, so both halves of the rule now read the value as the parser does, at both doors: a ' + + 'connector URL holding a userinfo, and a datasource URL holding a userinfo password, are refused as the parser ' + + 'reads them (a bare username in a datasource URL stays accepted), and each query pair is cleaned the same way ' + + 'before its name is read. One userinfo grammar serves both doors: an authority starts at `scheme://` or at a ' + + 'scheme-relative `//`. And the connector entry\'s credential query parameter rule is one value-level parse — the ' + + 'one the datasource doors already run — applied to every string under `providerConfig` and `auth`, whether or ' + + 'not it starts with a scheme. Both read ' + 'redactors rewrite exactly what the write refuses, at the position the write judged, and serve every other ' + 'character as stored. There is deliberately no mechanical rewrite: the value in the string may be the only copy ' + 'of the secret. A row stored before this refusal keeps working: a save of the served form keeps the stored ' +