Skip to content

fix(presets): read git config proxy values and mcp credential headers as credentials - #2371

Merged
freeqaz merged 3 commits into
mainfrom
fix-1276-1278
Oct 13, 2026
Merged

freeqaz merged 3 commits into
mainfrom
fix-1276-1278

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Two credential gaps in Configuration and Preset saves (findings 1276 and 1278):

  • An env map with GIT_CONFIG_KEY_0=http.proxy and GIT_CONFIG_VALUE_0=u:pw@proxy.example:3128 saved. The scheme-less proxy read was keyed on the variable name only (*_PROXY), but git reads GIT_CONFIG_VALUE_<n> as the value of the key GIT_CONFIG_KEY_<n> names, and git and curl send a proxy's userinfo as Proxy-Authorization.
  • mcp.servers.*.headers and nodeHost.mcp.servers.*.headers, which OpenClaw marks sensitive, accepted a literal Authorization: Bearer .... Model provider headers already refuse the same names.

Change

  • validateModelCredentialReferences reads a GIT_CONFIG_VALUE_<n> as a proxy (scheme-less, as http) when GIT_CONFIG_KEY_<n> names http.proxy, https.proxy, http.<url>.proxy or remote.<name>.proxy, in any case. Keys pair across the env maps OpenClaw merges into one environment: env, env.vars and every skill's (the Gateway's process), and the Docker sandbox defaults' with each Agent's (one Agent's keys do not pair with another's). Names are trimmed, as OpenClaw trims them.
  • GIT_CONFIG_PARAMETERS is parsed into its 'key'='value' (and older 'key=value') pairs, and each value whose key names a proxy gets the same check.
  • A key that is a whole ${VAR} reference, or that waits on a Preset variable, can name a proxy when the text around it allows one ({{ vars.k }}, http.{{ vars.u }}, {{ vars.s }}.proxy); user.{{ vars.k }} cannot. A Preset variable name whose index waits on a variable (GIT_CONFIG_VALUE_{{ vars.n }}, GIT_CONFIG_VALUE{{ vars.n }}) pairs with every index.
  • core.gitProxy is not matched: it names a command for git://, not a URL (credentials in commands are finding 1277's class).
  • MCP server headers on the Gateway and on nodes apply the model provider credential header rule (isCredentialHeaderName, including Preset names around a variable): a credential header must hold ${NAME}, or Bearer ${NAME} for an Authorization header. Writes only; the stored-read path is unchanged. The refusal names the field, never the value.
  • The new proxy reads go through the existing userinfo checks, so fix(presets): bound the url userinfo checks on every url field #2359's budget accounting covers them; the key match is linear.
  • Breaking-changes sentence in the 2026-10-13 model credential header entry; configuration-driver flow changelog.

Verification

  • node --test tests/integration/configuration-controller.test.mjs tests/conformance/configuration-occ.test.mjs tests/conformance/kubernetes-configuration.test.mjs tests/conformance/filesystem-configuration.test.mjs: 147/147 pass.
  • The two new tests (33 subtests) fail on main (all 31 refusal cases) and pass with the fix; 20/20 runs pinned to 2 CPUs.
  • An adversarial review probed key spellings, index forms, cross-map pairs, Preset variables, GIT_CONFIG_PARAMETERS, MCP header names and values, and the budget (the new key match and parser are linear); its bypasses and one over-refusal it found are covered by tests here; Preset-only leftovers are tracked as a low follow-up.
  • Accept cases: ordinary pairs (user.email with an @ value, a proxy without userinfo, a whole ${GIT_PROXY} reference, url.<base>.insteadOf, http.proxyAuthMethod, a core.gitProxy command with user@host), ordinary GIT_CONFIG_PARAMETERS, a proxy key in an unrelated process's map, and MCP headers holding references or naming no credential (including a number).
  • tsc --build tsconfig.json, eslint and prettier clean on touched files.

Risks

  • Low. A value with an @ at an index whose key is a whole ${VAR} reference or Preset variable, or in a map merged with one whose key at that index is a proxy, is now refused; write it as a ${NAME} reference.
  • MCP server configs with a literal credential header no longer save; the notice tells operators to use a reference.
  • Pairing follows OpenClaw's merges as listed; maps it does not merge (hooks, MCP server, local service, exec secret provider, install policy) pair only within themselves. A whole-variable Preset name ({{ vars.name }}) is still checked only once filled, as before.

… as credentials

A GIT_CONFIG_VALUE_<n> whose GIT_CONFIG_KEY_<n> names a git proxy (http.proxy,
https.proxy, http.<url>.proxy, remote.<name>.proxy, core.gitProxy) is now read
as a proxy, so a scheme-less user name or password in it is refused at save
(finding 1276). MCP server headers on the Gateway and nodeHost apply the model
provider credential header rule and must hold references (finding 1278).
@freeqaz
freeqaz requested a review from a team as a code owner October 13, 2026 22:50
@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.

…arameters

Review follow-up for finding 1276:
- keys pair across the maps OpenClaw merges (env, env.vars and every
  skill's; the Docker sandbox defaults and each Agent's)
- a key that is a whole reference, or waits on a Preset variable at
  either end, can name a proxy (wildcard-aware match)
- Preset names such as GIT_CONFIG_VALUE{{ vars.n }} pair with every index
- names are trimmed, as OpenClaw trims them
- GIT_CONFIG_PARAMETERS 'key'='value' and 'key=value' pairs are checked
- core.gitProxy is dropped: it names a command, not a URL
OpenClaw runs each Agent's sandbox with the defaults' environment and its
own, so one Agent's proxy key no longer refuses another Agent's ordinary
git config value (re-review over-refusal).
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 13, 2026 23:13 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 13, 2026 23:13 — with GitHub Actions Inactive
@freeqaz
freeqaz merged commit 474a9b7 into main Oct 13, 2026
45 checks passed
@freeqaz
freeqaz deleted the fix-1276-1278 branch October 13, 2026 23:21

This branch was previously deployed

1 inactive deployment
integration-qa-pr — d1b54e2e Deployed Oct 13, 2026 by freeqaz via QA advisory (kubernetes) #746
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