diff --git a/apps/controller/src/drivers/configuration/model-auth.ts b/apps/controller/src/drivers/configuration/model-auth.ts index e88e1a719f..bae3faff9b 100644 --- a/apps/controller/src/drivers/configuration/model-auth.ts +++ b/apps/controller/src/drivers/configuration/model-auth.ts @@ -19,6 +19,10 @@ const pointer = (...segments: readonly (string | number)[]): string => const urlCredentialMessage = (path: string): string => `Configuration field ${path} carries a user name or password in the URL. Remove it, or make the whole URL a \${NAME} reference.`; +// Proxy environment variables, in any case (`HTTPS_PROXY`, `all_proxy`, `npm_config_proxy`). +// `NO_PROXY` lists hosts, not a proxy. +const PROXY_ENVIRONMENT = /^(?!no_proxy$)(?:\w*_)?proxy$/i; + // The provider headers refused as literals before the 2026-10-12 notice. const STORED_CREDENTIAL_HEADER = /^(?:authorization|api-key|x-api-key)$/i; @@ -31,7 +35,9 @@ const STORED_CREDENTIAL_HEADER = /^(?:authorization|api-key|x-api-key)$/i; * Driver refuses to read one. A document an older release saved with a literal in a slot * refused since still reads, so its operator can see and replace it; every write and * deployment checks all slots. A Preset save passes `unresolved`, so a credential header name that - * waits on a variable is found as on Kubernetes Compute (`isCredentialHeaderName`). + * waits on a variable is found as on Kubernetes Compute (`isCredentialHeaderName`). Writes also + * refuse a user name or password in a native URL (`urlHoldsUserinfo`): model and proxy URLs, + * channel proxies, MCP server URLs and proxy environment variables. */ export function validateModelCredentialReferences( values: OpenClawConfigurationDocument, @@ -64,6 +70,26 @@ export function validateModelCredentialReferences( } throw new ModelCredentialValueError(path); }; + // The refusal names the field, never its value. A Preset save passes `unresolved`, the text + // around each unfilled variable at that path. + const refuseUrlUserinfo = ( + path: readonly (string | number)[], + value: unknown, + schemeless = false, + ): void => { + if (typeof value !== "string") { + return; + } + const [head = "", ...tail] = options.unresolved?.(path) ?? [value]; + if ( + urlHoldsUserinfo(value, [head, ...tail]) || + (schemeless && + !/^[\0- ]*[A-Za-z][A-Za-z0-9+.-]*:\/\//.test(head) && + urlHoldsUserinfo(value, [`http://${head}`, ...tail])) + ) { + throw new ModelCredentialValueError(pointer(...path), urlCredentialMessage); + } + }; const providers = asRecord(asRecord(values.models)?.providers); for (const [providerName, provider] of Object.entries(providers ?? {})) { @@ -129,20 +155,53 @@ export function validateModelCredentialReferences( validateReference(tls?.[field], pointer(...tlsPath, field)); } } - for (const [urlPath, url] of [ - [[...path, "baseUrl"], row?.baseUrl], - [[...path, "request", "proxy", "url"], proxy?.url], - ] as const) { - // A proxy URL with userinfo is a credential (undici sends it as Proxy-Authorization), and - // so is a model endpoint's (fetch refuses a URL that holds one), even where a variable - // or an environment reference fills its host or port (finding 1250). - if (typeof url === "string" && urlHoldsUserinfo(url, options.unresolved?.(urlPath))) { - throw new ModelCredentialValueError(pointer(...urlPath), urlCredentialMessage); + // A proxy URL with userinfo is a credential (undici sends it as Proxy-Authorization), and + // so is a model endpoint's (fetch refuses a URL that holds one), even where a variable + // or an environment reference fills its host or port (finding 1250). + refuseUrlUserinfo([...path, "baseUrl"], row?.baseUrl); + refuseUrlUserinfo([...path, "request", "proxy", "url"], proxy?.url); + } + } + const env = asRecord(values.env); + if (!stored) { + // The other native URLs a password can ride in (finding 1255): the Gateway's egress proxy, + // every channel's proxy and its accounts', and MCP server URLs, on the Gateway and on nodes. + refuseUrlUserinfo(["proxy", "proxyUrl"], asRecord(values.proxy)?.proxyUrl); + for (const [channel, settings] of Object.entries(asRecord(values.channels) ?? {})) { + const config = asRecord(settings); + refuseUrlUserinfo(["channels", channel, "proxy"], config?.proxy); + for (const [account, accountSettings] of Object.entries(asRecord(config?.accounts) ?? {})) { + refuseUrlUserinfo( + ["channels", channel, "accounts", account, "proxy"], + asRecord(accountSettings)?.proxy, + ); + } + } + for (const at of [["mcp"], ["nodeHost", "mcp"]]) { + const mcp = at.reduce | undefined>( + (parent, key) => asRecord(parent?.[key]), + values, + ); + for (const [server, settings] of Object.entries(asRecord(mcp?.servers) ?? {})) { + refuseUrlUserinfo([...at, "servers", server, "url"], asRecord(settings)?.url); + } + } + // Proxy environment (HTTPS_PROXY, http_proxy, ALL_PROXY, ...) reaches the Gateway and every + // process it starts. Clients such as curl and git read a value without a scheme as http. On a + // Preset save, a name that waits on a variable is matched as the text around it + // (`{{ vars.scheme }}_PROXY`), as for credential header names. + for (const [prefix, settings] of [ + [["env"], env], + [["env", "vars"], asRecord(env?.vars)], + ] as const) { + for (const name of Object.keys(settings ?? {})) { + const literal = options.unresolved?.([...prefix, name], true)?.join(""); + if ([name, literal].some((text) => text !== undefined && PROXY_ENVIRONMENT.test(text))) { + refuseUrlUserinfo([...prefix, name], settings?.[name], true); } } } } - const env = asRecord(values.env); for (const [prefix, settings] of [ [["env"], env], [["env", "vars"], asRecord(env?.vars)], diff --git a/docs/guides/deploy/breaking-changes-archive.md b/docs/guides/deploy/breaking-changes-archive.md index 1c2e67b383..ad2d7339b5 100644 --- a/docs/guides/deploy/breaking-changes-archive.md +++ b/docs/guides/deploy/breaking-changes-archive.md @@ -8,6 +8,26 @@ newest first. Steps marked _untested_ have not been run against a real Installation. When this page nears the word limit, its oldest notices move to a new page named for the date they precede. +## 2026-10-12: demo NetworkPolicy ports must be canonical decimals + +**What breaks.** `helm upgrade` of the observability demo fails when +`cluster.port` or `occ.metricsPort` is not a decimal integer from 1 through +65535 with no leading zero. A saved string `"010"` used to install, and +Kubernetes applied port 8. The upgrade now stops before it writes that policy. + +**Who is affected.** Demo releases whose saved values use a leading zero or a +port outside that range. Defaults 443 and 9464, and an ordinary decimal such as +6443, upgrade unchanged. + +**How to tell.** Helm stops in `prometheus.yaml` with +`cluster.port must be an integer TCP port from 1 to 65535`, or the same +sentence for `occ.metricsPort`. + +**Steps.** Set the field to the decimal Kubernetes already applied (`8` for a +saved `"010"`) and upgrade again. The NetworkPolicy port stays that decimal. +The [demo install](../observability/demo.md#install-private-backends) states the +same contract. + ## 2026-10-12: cluster and execution API ports must be canonical decimals **What breaks.** Helm now refuses `cluster.port` and `executionCluster.apiPort` diff --git a/docs/guides/deploy/breaking-changes.md b/docs/guides/deploy/breaking-changes.md index ca4ed7ae00..e589664773 100644 --- a/docs/guides/deploy/breaking-changes.md +++ b/docs/guides/deploy/breaking-changes.md @@ -112,18 +112,26 @@ variables follow. A later change finds a user name or password in a `baseUrl` or `request.proxy.url` whose host or port is a variable or `${VAR}` (or in its `:-` fallback), refuses a variable as the whole user name, and names an invalid first header character by code point: `not one that starts with U+007B`. A -Preset saved with one still reads but cannot launch: edit it. +Preset saved with one still reads but cannot launch: edit it. A later change +refuses one in `proxy.proxyUrl`, a channel or channel account `proxy`, an MCP +server `url`, and `env` proxy variables such as `HTTPS_PROXY` in any case; a +`baseUrl` with an empty host (`https:///v1`, as an empty variable leaves it); +and a number or boolean variable as the whole `baseUrl` or `api`. -**Who is affected.** Configurations with such a header, a `baseUrl` with a +**Who is affected.** Configurations with such a header, such a URL with a user name or password, or a `models` record, `null` or string on any provider. +A stored Configuration with such a user name or password does not deploy until +it is edited. **How to tell.** The 2026-10-12 messages, `Configuration setting models.providers..models must be an array of model entries.`, -or `Configuration field /models/providers//baseUrl carries a user name or password in the URL.` +`Configuration field carries a user name or password in the URL.`, +or `... must be an absolute http or https URL.` **Steps.** Follow the 2026-10-12 steps for the header. Write `models` as a list of model objects, or remove it. Remove the user name and password from -`baseUrl`, or make the whole URL a `${NAME}` reference. +the URL, or make the whole URL a `${NAME}` reference. Give a `baseUrl` its host, +and use a string variable for a whole `baseUrl` or `api`. ## 2026-10-13: sign-in and repository CIDRs must be canonical decimals @@ -316,23 +324,3 @@ they check the binding. Nothing new is refused. **How to tell.** The `400` says the method must be `runtime`. **Steps.** Set `harnessAuth` to `{ "method": "runtime" }`. - -## 2026-10-12: demo NetworkPolicy ports must be canonical decimals - -**What breaks.** `helm upgrade` of the observability demo fails when -`cluster.port` or `occ.metricsPort` is not a decimal integer from 1 through -65535 with no leading zero. A saved string `"010"` used to install, and -Kubernetes applied port 8. The upgrade now stops before it writes that policy. - -**Who is affected.** Demo releases whose saved values use a leading zero or a -port outside that range. Defaults 443 and 9464, and an ordinary decimal such as -6443, upgrade unchanged. - -**How to tell.** Helm stops in `prometheus.yaml` with -`cluster.port must be an integer TCP port from 1 to 65535`, or the same -sentence for `occ.metricsPort`. - -**Steps.** Set the field to the decimal Kubernetes already applied (`8` for a -saved `"010"`) and upgrade again. The NetworkPolicy port stays that decimal. -The [demo install](../observability/demo.md#install-private-backends) states the -same contract. diff --git a/docs/guides/observability/demo.md b/docs/guides/observability/demo.md index 97ee67628a..09cfad8bf3 100644 --- a/docs/guides/observability/demo.md +++ b/docs/guides/observability/demo.md @@ -175,7 +175,7 @@ Grafana stops working until you start it again. `"010"` used to install, and Kubernetes applied port 8. Upgrade now rejects that spelling before it changes the policy. Set the field to the applied decimal, `8` in that example, and rerun the block; the NetworkPolicy port stays 8. See -the [port notice](../deploy/breaking-changes.md#2026-10-12-demo-networkpolicy-ports-must-be-canonical-decimals). +the [port notice](../deploy/breaking-changes-archive.md#2026-10-12-demo-networkpolicy-ports-must-be-canonical-decimals). For a custom release, read its generated Service names with `helm get manifest RELEASE -n NAMESPACE` before adapting the port-forward command diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index ea39097d8a..fd612fbb52 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -1755,6 +1755,13 @@ export interface ConfigurationValidationOptions { path: readonly (string | number)[], key?: boolean, ) => readonly string[] | undefined; + /** + * The declared type of an unfilled variable that is the whole value at `path`, or undefined: + * a number or boolean fills a field that takes text with no string. + */ + readonly unresolvedType?: ( + path: readonly (string | number)[], + ) => "string" | "number" | "boolean" | "password" | undefined; } export interface ComputeDriver extends Driver { diff --git a/packages/contracts/src/preset-variables.mjs b/packages/contracts/src/preset-variables.mjs index f3e6b83fc2..fae8a12b18 100644 --- a/packages/contracts/src/preset-variables.mjs +++ b/packages/contracts/src/preset-variables.mjs @@ -123,7 +123,8 @@ function tokens(text, definitions, path) { return parts; } -// `located(literals, text)` receives the literal text around the placeholders left in `text`. +// `located(literals, text, type)` receives the literal text around the placeholders left in +// `text`, and the declared type of a variable that is the whole value. function substitute(text, definitions, inputs, path, key, partial, located) { const parts = tokens(text, definitions, path); const refs = parts.filter((part) => typeof part !== "string"); @@ -173,7 +174,7 @@ function substitute(text, definitions, inputs, path, key, partial, located) { } } if (literals.length > 1) { - located?.(literals, result); + located?.(literals, result, whole ? definitions[refs[0].name].type : undefined); } return result; } @@ -184,13 +185,22 @@ function placeholderLocation(path, key) { return JSON.stringify([key ? "key" : "value", ...path]); } +// Where admission finds the declared type of a variable without a value that is a whole value. +function wholeVariableLocation(path) { + return JSON.stringify(["type", ...path]); +} + // `placeholders` collects admission's placeholder locations; `at` is the path in // configuration.values (undefined outside it). function walk(value, definitions, inputs, path, keys, partial, placeholders, at) { const located = (key) => placeholders && at - ? (literals, text) => - placeholders.set(placeholderLocation(key ? [...at, text] : at, key), literals) + ? (literals, text, type) => { + placeholders.set(placeholderLocation(key ? [...at, text] : at, key), literals); + if (type !== undefined) { + placeholders.set(wholeVariableLocation(at), type); + } + } : undefined; if (typeof value === "string") { return substitute(value, definitions, inputs, path, false, partial, located(false)); @@ -329,11 +339,14 @@ export function presetTemplateDefaults(template) { * once filled. `unresolved(path, key)` returns the literal text around the placeholders of the * string at `path` (an object key: `key` true, `path` ending in the rendered key), or undefined. * Escaped tokens and defaults that read like tokens are literal text: elsewhere they are checked - * as written, and beside a placeholder they are part of its literal text. + * as written, and beside a placeholder they are part of its literal text. `unresolvedType(path)` + * returns the declared type of such a variable that is the whole value at `path`: a number or + * boolean variable fills it with no string (finding 1257). * @param {import('./presets.ts').PresetTemplate} template * @returns {{ * settings: import('./presets.ts').PresetLaunchSettings, * unresolved: (path: readonly (string | number)[], key?: boolean) => readonly string[] | undefined, + * unresolvedType: (path: readonly (string | number)[]) => import('./presets.ts').PresetVariable['type'] | undefined, * }} */ export function presetTemplateDefaultsWithPlaceholders(template) { @@ -342,6 +355,7 @@ export function presetTemplateDefaultsWithPlaceholders(template) { return { settings, unresolved: (path, key = false) => placeholders.get(placeholderLocation(path, key)), + unresolvedType: (path) => placeholders.get(wholeVariableLocation(path)), }; } diff --git a/packages/occ/src/index.ts b/packages/occ/src/index.ts index fdc4f3527f..01fd66d058 100644 --- a/packages/occ/src/index.ts +++ b/packages/occ/src/index.ts @@ -4254,7 +4254,8 @@ export class OpenClawController { namespaceId: string, ): Promise { const template = normalizePresetTemplate(input, namespaceId); - const { settings, unresolved } = presetTemplateDefaultsWithPlaceholders(template); + const { settings, unresolved, unresolvedType } = + presetTemplateDefaultsWithPlaceholders(template); const values = settings.configuration?.values; if (values !== undefined) { const driver = this.configurationDriver(); @@ -4269,7 +4270,7 @@ export class OpenClawController { // Only text the render put a placeholder into waits on a variable, located by its path: an // escaped token is literal text that stays in the launch render, so it is checked as // written (finding 1214), even where it reads like a placeholder elsewhere (finding 1220). - const options: ConfigurationValidationOptions = { unresolved }; + const options: ConfigurationValidationOptions = { unresolved, unresolvedType }; validateModelProviderSettings(values, options); this.validateComputeConfiguration(values, options); // Validation owns native credential rules; Preset CRUD never creates runtime resources. diff --git a/packages/occ/src/model-provider-settings.ts b/packages/occ/src/model-provider-settings.ts index 4d7bb0cc6a..fd6dadfb33 100644 --- a/packages/occ/src/model-provider-settings.ts +++ b/packages/occ/src/model-provider-settings.ts @@ -51,9 +51,14 @@ const holdsEnvironmentReference = (value: unknown): boolean => // Text its writer fills in later: `unresolved` locates an unfilled Preset variable's text. type Unresolved = NonNullable; +// An http(s) URL whose written authority is empty (`https:///v1`, as an empty Preset variable +// for the host renders it) parses with the first path segment as its host (finding 1256). const isRuntimeUrl = (value: unknown): boolean => { const url = typeof value === "string" && URL.canParse(value) ? new URL(value) : undefined; - return url?.protocol === "http:" || url?.protocol === "https:"; + return ( + (url?.protocol === "http:" || url?.protocol === "https:") && + !/^[\0- ]*https?:[/\\]{3}/i.test(String(value).replaceAll(/[\t\n\r]/g, "")) + ); }; // Where a variable sits in a baseUrl with a literal http(s) scheme, and the one token it stands @@ -266,8 +271,17 @@ function validateSettings( path: readonly (string | number)[], provider: boolean, unresolved: Unresolved, + unresolvedType: ConfigurationValidationOptions["unresolvedType"], ) { const { baseUrl, api } = settings; + // A number or boolean variable that is the whole baseUrl or api fills it with no string, which + // fails every launch (finding 1257). + for (const field of ["baseUrl", "api"] as const) { + const type = unresolvedType?.([...path, field]); + if (type !== undefined && type !== "string") { + throw new ModelProviderSettingError(pointer(...path, field), field); + } + } // The runtime resolves an environment reference before it validates the value. const environment = holdsEnvironmentReference; // On a provider, the runtime treats a blank baseUrl as unset and uses a built-in @@ -319,7 +333,7 @@ export function providerModelsListError(key: string): ConfigurationHarnessError */ export function validateModelProviderSettings( values: OpenClawConfigurationDocument, - { unresolved = () => undefined }: ConfigurationValidationOptions = {}, + { unresolved = () => undefined, unresolvedType }: ConfigurationValidationOptions = {}, ): void { const providers = asRecord(asRecord(values.models)?.providers); for (const [providerName, provider] of Object.entries(providers ?? {})) { @@ -329,7 +343,7 @@ export function validateModelProviderSettings( } const path = ["models", "providers", providerName] as const; const setting = keyedSettingPath("models.providers", providerName); - validateSettings(settings, path, true, unresolved); + validateSettings(settings, path, true, unresolved, unresolvedType); const models = settings.models; // The list first, as the Kubernetes rule does, so both name the same setting. if (models !== undefined && !Array.isArray(models)) { @@ -339,7 +353,13 @@ export function validateModelProviderSettings( for (const [index, model] of (Array.isArray(models) ? models : []).entries()) { const modelSettings = asRecord(model); if (modelSettings !== undefined) { - validateSettings(modelSettings, [...path, "models", index], false, unresolved); + validateSettings( + modelSettings, + [...path, "models", index], + false, + unresolved, + unresolvedType, + ); validateHeaderNames( modelSettings, [...path, "models", index], diff --git a/tests/integration/configuration-controller.test.mjs b/tests/integration/configuration-controller.test.mjs index e5ced08ef7..ea01cd6194 100644 --- a/tests/integration/configuration-controller.test.mjs +++ b/tests/integration/configuration-controller.test.mjs @@ -8,7 +8,10 @@ import test from "node:test"; import { resolveApprovedHarness as resolveApprovedDevelopmentHarness } from "../../apps/controller/src/composition/production-harness.ts"; import { createControllerApp } from "../../apps/controller/src/index.ts"; import { InMemoryAuditSink } from "../../packages/audit/src/index.ts"; -import { admitLoggingConfiguration } from "../../packages/contracts/src/index.ts"; +import { + admitLoggingConfiguration, + renderPresetTemplate, +} from "../../packages/contracts/src/index.ts"; import { NativeIAMDriver } from "../../packages/iam/src/index.ts"; import { InMemoryPlatformState, OpenClawController } from "../../packages/occ/src/index.ts"; import { createReadyComputeDriver } from "../helpers/development.mjs"; @@ -1833,6 +1836,264 @@ test("Without Kubernetes Compute, Configuration and Preset saves refuse what eve ); }); +test("Configuration and Preset saves refuse a user name or password in proxy, channel proxy, MCP server and proxy environment URLs", async (t) => { + // Finding 1255: #2347 refused URL userinfo only in model baseUrl and request.proxy.url. The + // Gateway's egress proxy, channel proxies, MCP server URLs and proxy environment variables + // carry the same credential (a proxy sends it as Proxy-Authorization), and saved. + const root = await mkdtemp(join(tmpdir(), "occ-url-userinfo-")); + t.after(() => rm(root, { recursive: true, force: true })); + const configurationDriver = new FilesystemConfigurationDriver(root); + const computeDriver = createReadyComputeDriver("compute-url-userinfo"); + const { namespace, ...context } = await fixture({ configurationDriver, computeDriver }); + await context.controller.transact((state) => + state.namespaces.transitionNamespaceStatus(namespace.id, "provisioning", "ready"), + ); + const configurations = `/namespaces/${namespace.id}/configurations`; + const presets = `/namespaces/${namespace.id}/presets`; + const sentinel = `synthetic-url-userinfo-${randomUUID()}`; + const message = (field) => + `Configuration field ${field} carries a user name or password in the URL. Remove it, or make the whole URL a \${NAME} reference.`; + const userinfo = `http://u:${sentinel}@proxy.example:3128`; + const refused = [ + [{ proxy: { enabled: true, proxyUrl: userinfo } }, "/proxy/proxyUrl"], + [{ channels: { telegram: { proxy: userinfo } } }, "/channels/telegram/proxy"], + [ + { channels: { discord: { proxy: `socks5://u:${sentinel}@proxy:1080` } } }, + "/channels/discord/proxy", + ], + [ + { channels: { telegram: { accounts: { ops: { proxy: userinfo } } } } }, + "/channels/telegram/accounts/ops/proxy", + ], + [ + { mcp: { servers: { docs: { url: `https://u:${sentinel}@mcp.example.test/mcp` } } } }, + "/mcp/servers/docs/url", + ], + [ + { nodeHost: { mcp: { servers: { docs: { url: `https://${sentinel}@mcp.example.test` } } } } }, + "/nodeHost/mcp/servers/docs/url", + ], + [{ env: { vars: { HTTPS_PROXY: userinfo } } }, "/env/vars/HTTPS_PROXY"], + [{ env: { vars: { http_proxy: userinfo } } }, "/env/vars/http_proxy"], + [{ env: { ALL_PROXY: userinfo } }, "/env/ALL_PROXY"], + [{ env: { vars: { all_proxy: userinfo } } }, "/env/vars/all_proxy"], + // curl and git read a proxy without a scheme as http. + [{ env: { vars: { HTTPS_PROXY: `${sentinel}@proxy.example:3128` } } }, "/env/vars/HTTPS_PROXY"], + // As in #2347: an environment reference in the port, or a fallback that holds userinfo. + [ + { channels: { telegram: { proxy: `http://u:${sentinel}@proxy.example:\${PROXY_PORT}` } } }, + "/channels/telegram/proxy", + ], + [ + { proxy: { proxyUrl: `\${EGRESS_PROXY:-http://u:${sentinel}@proxy.example:3128}` } }, + "/proxy/proxyUrl", + ], + ]; + // A Configuration to update: a refused edit leaves it as it was. + const original = await request(context.app, "POST", configurations, { + body: { + kind: "agent", + values: { env: { vars: { HTTPS_PROXY: "http://proxy.example:3128" } } }, + }, + }); + assert.equal(original.status, 201, JSON.stringify(original.body)); + const item = `${configurations}/${original.body.data.id}`; + for (const [values, field] of refused) { + for (const [method, path, body] of [ + ["POST", configurations, { kind: "agent", values }], + ["PATCH", item, { values }], + ["POST", presets, { name: "Refused", template: { configuration: { values } } }], + ]) { + const response = await request(context.app, method, path, { body }); + assert.equal(response.status, 400, `${method} ${path}: ${JSON.stringify(response.body)}`); + assert.equal(response.body.error.message, message(field), `${method} ${path}`); + assert.equal(JSON.stringify(response.body).includes(sentinel), false); + } + } + const read = await request(context.app, "GET", item); + assert.equal(read.status, 200, JSON.stringify(read.body)); + assert.deepEqual(read.body.data.values, original.body.data.values); + assert.deepEqual(await readdir(join(root, namespace.id)), [`${original.body.data.id}.json`]); + assert.equal( + (await readFile(join(root, namespace.id, `${original.body.data.id}.json`), "utf8")).includes( + sentinel, + ), + false, + ); + assert.deepEqual((await request(context.app, "GET", presets)).body.data, []); + + // A Preset variable as the host, port or the whole URL is checked once filled; literal userinfo + // beside it, or a variable as the whole user name, is refused at save. + const variables = { host: { type: "string" }, port: { type: "string" }, url: { type: "string" } }; + for (const [values, field] of [ + [{ proxy: { proxyUrl: `http://u:${sentinel}@{{ vars.host }}:3128` } }, "/proxy/proxyUrl"], + [ + { channels: { discord: { proxy: `http://u:${sentinel}@proxy.example:{{ vars.port }}` } } }, + "/channels/discord/proxy", + ], + [ + { mcp: { servers: { docs: { url: "https://{{ vars.url }}@{{ vars.host }}/mcp" } } } }, + "/mcp/servers/docs/url", + ], + [{ env: { vars: { HTTPS_PROXY: `u:${sentinel}@{{ vars.host }}` } } }, "/env/vars/HTTPS_PROXY"], + [ + { env: { vars: { HTTPS_PROXY: `${sentinel}@{{ vars.host }}:3128` } } }, + "/env/vars/HTTPS_PROXY", + ], + // A variable in the name is matched as the text around it. + [ + { env: { vars: { "{{ vars.host }}_PROXY": `http://u:${sentinel}@proxy.example:3128` } } }, + "/env/vars/{{ vars.host }}_PROXY", + ], + ]) { + const response = await request(context.app, "POST", presets, { + body: { name: "Refused", template: { variables, configuration: { values } } }, + }); + assert.equal(response.status, 400, JSON.stringify(response.body)); + assert.equal(response.body.error.message, message(field)); + assert.equal(JSON.stringify(response.body).includes(sentinel), false); + } + + // A proxy without userinfo, a whole reference, a fallback without userinfo, NO_PROXY and an + // `@` in a path still save. + const accepted = { + proxy: { enabled: true, proxyUrl: "${EGRESS_PROXY:-http://proxy.example:3128}" }, + channels: { + telegram: { proxy: "http://10.42.0.15:3128", accounts: { ops: { proxy: "${OPS_PROXY}" } } }, + discord: { proxy: "socks5://proxy.example:1080" }, + }, + mcp: { servers: { docs: { url: "https://mcp.example.test/v1/@scope/mcp" } } }, + env: { + HTTP_PROXY: "proxy.example:3128", + vars: { HTTPS_PROXY: "${CORP_PROXY}", NO_PROXY: "localhost,.svc,10.0.0.0/8" }, + }, + }; + const created = await request(context.app, "POST", configurations, { + body: { kind: "agent", values: accepted }, + }); + assert.equal(created.status, 201, JSON.stringify(created.body)); + assert.deepEqual(created.body.data.values, accepted); + const templated = await request(context.app, "POST", presets, { + body: { + name: "Templated", + template: { + variables, + configuration: { + values: { + ...accepted, + proxy: { proxyUrl: "http://{{ vars.host }}:{{ vars.port }}" }, + channels: { telegram: { proxy: "{{ vars.url }}" } }, + env: { vars: { HTTPS_PROXY: "{{ vars.host }}:3128", ALL_PROXY: "{{ vars.url }}" } }, + }, + }, + }, + }, + }); + assert.equal(templated.status, 201, JSON.stringify(templated.body)); + + // A document an older release stored with one still reads, so its operator can edit it. + const path = join(root, namespace.id, `${original.body.data.id}.json`); + const stale = { proxy: { proxyUrl: userinfo }, env: { vars: { HTTPS_PROXY: userinfo } } }; + const current = JSON.parse(await readFile(path, "utf8")); + await writeFile(path, JSON.stringify({ ...current, values: stale })); + const staleRead = await request(context.app, "GET", item); + assert.equal(staleRead.status, 200, JSON.stringify(staleRead.body)); + assert.deepEqual(staleRead.body.data.values, stale); + const resaved = await request(context.app, "PATCH", item, { body: { values: stale } }); + assert.equal(resaved.status, 400, JSON.stringify(resaved.body)); + assert.equal(resaved.body.error.message, message("/proxy/proxyUrl")); +}); + +test("Preset and Configuration saves refuse a baseUrl or api that a variable fills with no host or no string", async (t) => { + // Finding 1256: `https://{{ vars.h }}/v1` filled empty renders `https:///v1`, whose first path + // segment the URL parser reads as the host. Finding 1257: a number or boolean variable that is + // the whole baseUrl or api saved, and failed every launch. + const root = await mkdtemp(join(tmpdir(), "occ-preset-fill-")); + t.after(() => rm(root, { recursive: true, force: true })); + const configurationDriver = new FilesystemConfigurationDriver(root); + const computeDriver = createReadyComputeDriver("compute-preset-fill"); + const { namespace, ...context } = await fixture({ configurationDriver, computeDriver }); + await context.controller.transact((state) => + state.namespaces.transitionNamespaceStatus(namespace.id, "provisioning", "ready"), + ); + const configurations = `/namespaces/${namespace.id}/configurations`; + const presets = `/namespaces/${namespace.id}/presets`; + const gateway = (settings) => ({ models: { providers: { gateway: settings } } }); + const baseUrl = (field = "baseUrl") => + `Configuration field /models/providers/gateway/${field} must be an absolute http or https URL.`; + const api = + "Configuration field /models/providers/gateway/api must name a model API the runtime supports, such as openai-responses, openai-completions or anthropic-messages."; + + // An empty written authority, however it is spelled, is refused on Configuration create. + for (const [settings, message] of [ + [{ baseUrl: "https:///v1" }, baseUrl()], + [{ baseUrl: " HTTP:\\\\\\v1" }, baseUrl()], + [{ baseUrl: "https:/\t//v1" }, baseUrl()], + [ + { models: [{ id: "m", baseUrl: "http:////models.example.test/v1" }] }, + baseUrl("models/0/baseUrl"), + ], + ]) { + const response = await request(context.app, "POST", configurations, { + body: { kind: "agent", values: gateway(settings) }, + }); + assert.equal(response.status, 400, JSON.stringify(response.body)); + assert.equal(response.body.error.message, message); + } + + const template = (variables, settings) => ({ + variables, + configuration: { values: gateway(settings) }, + }); + const hostVariable = { baseUrl: "https://{{ vars.h }}/v1" }; + for (const [variables, settings, message] of [ + // An empty default is a literal empty host. + [{ h: { type: "string", default: "" } }, hostVariable, baseUrl()], + [{ n: { type: "number" } }, { baseUrl: "{{ vars.n }}" }, baseUrl()], + [{ b: { type: "boolean" } }, { baseUrl: "{{ vars.b }}" }, baseUrl()], + [{ n: { type: "number" } }, { api: "{{ vars.n }}" }, api], + [ + { b: { type: "boolean" } }, + { models: [{ id: "m", baseUrl: "{{ vars.b }}" }] }, + baseUrl("models/0/baseUrl"), + ], + ]) { + const response = await request(context.app, "POST", presets, { + body: { name: "Refused", template: template(variables, settings) }, + }); + assert.equal(response.status, 400, JSON.stringify(response.body)); + assert.equal(response.body.error.message, message); + } + assert.deepEqual((await request(context.app, "GET", presets)).body.data, []); + + // A host variable without a default saves; filled empty at launch, its Configuration is refused. + const variables = { h: { type: "string" }, n: { type: "number" }, u: { type: "string" } }; + const values = { + ...hostVariable, + models: [{ id: "m", baseUrl: "{{ vars.u }}", contextWindow: "{{ vars.n }}" }], + }; + const saved = await request(context.app, "POST", presets, { + body: { name: "Filled at launch", template: template(variables, values) }, + }); + assert.equal(saved.status, 201, JSON.stringify(saved.body)); + const launch = (inputs) => + request(context.app, "POST", configurations, { + body: { + kind: "agent", + ...renderPresetTemplate(saved.body.data.template, inputs).configuration, + }, + }); + const empty = await launch({ h: "", n: 1000, u: "https://models.example.test/v1" }); + assert.equal(empty.status, 400, JSON.stringify(empty.body)); + assert.equal(empty.body.error.message, baseUrl()); + const filled = await launch({ + h: "models.example.test", + n: 1000, + u: "https://models.example.test/v1", + }); + assert.equal(filled.status, 201, JSON.stringify(filled.body)); +}); + test("A stored Configuration with request credentials an older release accepted reads, is repaired by an edit, and does not deploy", async (t) => { // The 2026-10-12 notice: an older release saved model credentials in a provider's request // transport settings. After the upgrade the bundled Driver must still read and replace that