Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 70 additions & 11 deletions apps/controller/src/drivers/configuration/model-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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 ?? {})) {
Expand Down Expand Up @@ -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<Record<string, unknown> | 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)],
Expand Down
20 changes: 20 additions & 0 deletions docs/guides/deploy/breaking-changes-archive.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
36 changes: 12 additions & 24 deletions docs/guides/deploy/breaking-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<provider>.models must be an array of model entries.`,
or `Configuration field /models/providers/<provider>/baseUrl carries a user name or password in the URL.`
`Configuration field <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

Expand Down Expand Up @@ -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.
2 changes: 1 addition & 1 deletion docs/guides/observability/demo.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions packages/contracts/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
24 changes: 19 additions & 5 deletions packages/contracts/src/preset-variables.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}
Expand All @@ -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));
Expand Down Expand Up @@ -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) {
Expand All @@ -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)),
};
}

Expand Down
5 changes: 3 additions & 2 deletions packages/occ/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4254,7 +4254,8 @@ export class OpenClawController {
namespaceId: string,
): Promise<PresetTemplate> {
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();
Expand All @@ -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.
Expand Down
28 changes: 24 additions & 4 deletions packages/occ/src/model-provider-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConfigurationValidationOptions["unresolved"]>;

// 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ?? {})) {
Expand All @@ -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)) {
Expand All @@ -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],
Expand Down
Loading
Loading