Skip to content

fix(presets): refuse url userinfo in proxy, channel, mcp and proxy env fields, an empty baseUrl host and non-string url variables - #2350

Merged
freeqaz merged 4 commits into
mainfrom
fix-1255
Oct 13, 2026
Merged

freeqaz merged 4 commits into
mainfrom
fix-1255

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Three low-severity gaps in Configuration and Preset save checks.

URL userinfo in other native URL fields. Since #2347, saves refuse a user name or password only in a model baseUrl and request.proxy.url (urlHoldsUserinfo). These other native URL fields carry the same credential, because a proxy sends it as Proxy-Authorization, and they still saved it:

Field On main
proxy.proxyUrl (Gateway egress proxy) not refused
channels.<channel>.proxy, channels.<channel>.accounts.<id>.proxy not refused. The Kubernetes channel proxy rules in #2334 and #2344 check the Installation's channels.proxyUrl and its directory proxy, not the Configuration.
mcp.servers.<name>.url, nodeHost.mcp.servers.<name>.url not refused
HTTPS_PROXY, HTTP_PROXY, ALL_PROXY (any case) in env / env.vars not refused. requireHarnessCredentialBinding checks only model provider credential names.

Empty baseUrl host. A Preset baseUrl of https://{{ vars.h }}/v1 with h empty (an empty default or an empty launch input) renders https:///v1. The URL parser reads v1 as the host, and Configuration create accepted it.

Non-string variable as the whole baseUrl or api. baseUrl: "{{ vars.n }}" or api: "{{ vars.n }}" with a number or boolean variable saved, then failed every launch.

Change

Userinfo

  • validateModelCredentialReferences (model-auth.ts) refuses userinfo in the fields above on every write. It uses the shared urlHoldsUserinfo check with the same variable handling: the Preset text around {{ vars.x }} placeholders, ${VAR} references and :- fallbacks.
  • The refusal is the fix(presets): find url userinfo when a variable fills the host or port #2347 message. It names the field by JSON pointer and never echoes the value: Configuration field /env/vars/HTTPS_PROXY carries a user name or password in the URL. Remove it, or make the whole URL a ${NAME} reference.
  • Proxy environment covers any *_PROXY name in any case (HTTPS_PROXY, all_proxy, npm_config_proxy) except NO_PROXY. A name that waits on a Preset variable ({{ vars.x }}_PROXY) is matched on the text around the variable. A value without a scheme is also checked as http://<value>, which is how curl and git read it.

Empty host

  • isRuntimeUrl (model-provider-settings.ts) refuses an http(s) URL whose written authority is empty: three or more slashes or backslashes after the scheme.

Non-string variables

  • presetTemplateDefaultsWithPlaceholders also returns unresolvedType(path), the declared type of an unfilled variable that makes up the whole value at that path.
  • This is passed as the optional ConfigurationValidationOptions.unresolvedType, and Preset admission now refuses a number or boolean baseUrl or api with the existing message.

Stored documents and docs

  • Stored documents are left alone. A Configuration an older release saved with any of these still reads and can be edited. Saving it unchanged is refused, and deployment refuses the userinfo cases.
  • The 2026-10-13 model credential breaking-change notice covers the new refusals. To stay under the word cap, the oldest notice (2026-10-12, demo NetworkPolicy ports) moves to the archive page.

Verification

  • New integration test Configuration and Preset saves refuse a user name or password in proxy, channel proxy, MCP server and proxy environment URLs:
    • Configuration create, Configuration update and Preset save each refuse all 13 field cases. Six more Preset cases use variables, including a variable in the proxy environment name.
    • A refused edit leaves the stored Configuration unchanged, and no response contains the sentinel.
    • These still save: whole references, fallbacks without userinfo, NO_PROXY, an @ in a path, and variable hosts.
    • An older stored document still reads.
  • New integration test Preset and Configuration saves refuse a baseUrl or api that a variable fills with no host or no string:
    • Configuration create refuses four empty-authority spellings.
    • Preset save refuses an empty host default and number or boolean whole baseUrl or api variables, including on a model row.
    • A host variable without a default still saves. Its launch render is refused when the input is empty and accepted when filled.
    • A number variable in contextWindow still saves.
  • Every refused case was checked against the origin/main sources. All of them were accepted there, and all are refused with this change.
  • These suites pass, 442/442 in total (the docs link to the moved notice now points at the archive page):
    • tests/integration/configuration-controller.test.mjs
    • tests/integration/presets-controller.test.mjs
    • tests/conformance/kubernetes-configuration.test.mjs
    • tests/conformance/kubernetes-compute.test.mjs
    • tests/conformance/presets.test.mjs
  • The new tests and the fix(presets): find url userinfo when a variable fills the host or port #2347 test, pinned to 2 CPUs: 20/20.
  • tsc --build, plus eslint and prettier on the touched files: clean.
  • Two independent bypass reviews (percent-encoding, control characters, backslashes, fallbacks, variable keys, prototype keys, case and scheme tricks): no in-scope bypass. Their nits are applied.

Risks

…oxy env fields

Finding 1255: since #2347 only model baseUrl and request.proxy.url were checked
for a user name or password. Configuration and Preset saves now also refuse one
in proxy.proxyUrl, channels.<channel>.proxy and its accounts' proxy, MCP server
urls (mcp and nodeHost.mcp), and *_PROXY variables in env and env.vars (any
case; a value without a scheme is read as http). Stored documents still read.
Moves the oldest breaking-changes notice to the archive to make room.
…rl or api variables

Finding 1256: an http(s) baseUrl whose written authority is empty
(https:///v1, as an empty Preset variable leaves it) parsed with its first
path segment as the host; it is now refused on every write.
Finding 1257: a number or boolean Preset variable that is the whole baseUrl
or api saved and failed every launch. The Preset render now reports the
declared type of a whole-value variable (unresolvedType) and the model
provider rule refuses a non-string one at save.
Review nit on 1255: a proxy environment name that waits on a Preset variable
({{ vars.x }}_PROXY) is matched as the text around it. The notice names the
new fields and that the next deployment fails until edited.
…ploy

Review nit: deployment re-checks the native credential slots, not the model
provider baseUrl rule, so a stored empty-host baseUrl still deploys. Name the
baseUrl refusal message and the steps for findings 1256 and 1257.
@freeqaz
freeqaz requested a review from a team as a code owner October 13, 2026 17:17
@clawsweeper

clawsweeper Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review blocked

Automated review did not run, so no review verdict was produced.

Reason: The input-safety check rejected material in this revision. No detected value, path, or scanner output is reproduced here.

Automatic review is on hold, including after source changes. Request a fresh re-review after resolving the failure; maintainers can also release the hold through an intentional scanner-policy update.

Next step: If this is a genuine credential, remove and rotate it. If it is an intentional test fixture, a maintainer must review and qualify it.

View the workflow run.

The notice moved to breaking-changes-archive.md to make room; the docs build
refused the stale anchor.

This branch was successfully deployed

1 active deployment
integration-qa-pr — 05938a34 Deployed Oct 13, 2026 by freeqaz via QA advisory (compose) #684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant