Skip to content

fix(windows): escape '$' before .NET regex replacement in config patching - #2962

Open
bettercalln1ck wants to merge 1 commit into
Osmantic:mainfrom
bettercalln1ck:fix/2928-hermes-config-dollar
Open

fix(windows): escape '$' before .NET regex replacement in config patching#2962
bettercalln1ck wants to merge 1 commit into
Osmantic:mainfrom
bettercalln1ck:fix/2928-hermes-config-dollar

Conversation

@bettercalln1ck

Copy link
Copy Markdown

Summary

Fixes #2928.

Update-HermesConfigFile (ods/installers/windows/phases/06-directories.ps1:392-393) interpolated $Model and $BaseUrl directly into -replace replacement strings, and the bootstrap .env rewrite at install-windows.ps1:376-377 did the same with $tierConfig.GgufFile / $tierConfig.LlmModel. .NET reads $ in a replacement as a substitution token, so a value carrying one is rewritten on its way into the file.

One correction to the issue, from actually running it. $1 and ${x} are not mangled: .NET leaves a group reference literal when the pattern has no such group, and these patterns carry no capture groups at all. The tokens that do fire are $$ and $&, and $& is the destructive one — it splices the entire matched line into the value:

value passed        resulting line                                    verdict
qwen$1-turbo    ->  default: "qwen$1-turbo"                           verbatim
qwen$$-turbo    ->  default: "qwen$-turbo"                            MANGLED
qwen$&-turbo    ->  default: "qwen  default: "old-model"-turbo"       MANGLED
qwen${x}-turbo  ->  default: "qwen${x}-turbo"                         verbatim

That third line is not just a wrong value, it is broken YAML.

Worth knowing for severity: on the Hermes path the function's own post-write verification ($verified.Contains(...)) catches the mangling and returns $false, so the installer aborts loudly rather than shipping a bad config — but WriteAllText has already run, so the corrupted file is on disk. On the .env path there is no verification, so it is genuinely silent, which is where the issue's "nothing errors" description belongs.

Fix: double $ via String.Replace('$', '$$') on all four values before substitution. The verification still compares against the raw value, which is what should land on disk.

AI Assistance

Claude Code drafted the patch and the regression test and ran the validation recorded below. The human author reviewed the diff, chose the validation, and is accountable for the change.

Release Lane

  • Stable hotfix targeting release/2.6.x
  • Mainline change targeting main
  • Next-minor work targeting the next feature/minor release
  • Not sure; reviewer should help classify

Stable hotfix reason:

n/a

Changed Surface

  • Docs only
  • Tests only
  • Dashboard UI
  • Dashboard API / host agent
  • Installer / bootstrap / lifecycle
  • Docker Compose / service manifests
  • Model routing / Hermes / capabilities
  • Network exposure / auth / proxy
  • Dependencies / runtime wiring

Risk And Validation

  • Risk level: Low
  • Validation run:
    • git diff --check
    • Markdown/link sanity for docs
    • Focused tests listed below
    • Dashboard lint/test/build
    • Extension audit / compose validation
    • Release-grade fleet or scoped hardware validation
    • Stable-lane patch validation, if targeting release/2.6.x
    • Not required because: four one-line escapes on values that were already being written to these files; behaviour is identical for any value without a '$', which is every model id and URL ODS ships today.

Commands/results:

# behaviour, under real PowerShell 7.4 (docker mcr.microsoft.com/powershell),
# driving the extracted Update-HermesConfigFile from both revisions

$ pwsh -File drive.ps1 buggy-fn.ps1      # function as it is on main
qwen$1-turbo       -> default: "qwen$1-turbo"                     [verbatim] returned=True
qwen$$-turbo       -> default: "qwen$-turbo"                      [MANGLED]  returned=False
qwen$&-turbo       -> default: "qwen  default: "old-model"-turbo" [MANGLED]  returned=False
qwen${x}-turbo     -> default: "qwen${x}-turbo"                   [verbatim] returned=True

$ pwsh -File drive.ps1 fixed-fn.ps1      # with this patch
qwen$1-turbo       -> default: "qwen$1-turbo"                     [verbatim] returned=True
qwen$$-turbo       -> default: "qwen$$-turbo"                     [verbatim] returned=True
qwen$&-turbo       -> default: "qwen$&-turbo"                     [verbatim] returned=True
qwen${x}-turbo     -> default: "qwen${x}-turbo"                   [verbatim] returned=True

$ bash tests/contracts/test-windows-hermes-config-patching.sh
...
[PASS] Hermes config patching escapes '$' before regex replacement
[PASS] Bootstrap .env patching escapes '$' before regex replacement
PASS=12 FAIL=0

# the two new assertions against the unpatched files
$ git stash push -- ods/installers/windows/phases/06-directories.ps1 ods/installers/windows/install-windows.ps1
$ bash tests/contracts/test-windows-hermes-config-patching.sh
[FAIL] Update-HermesConfigFile must double '$' in model/base_url before -replace
[FAIL] install-windows.ps1 must double '$' in GGUF_FILE/LLM_MODEL before -replace
PASS=10 FAIL=2

$ make lint
=== Shell syntax ===
=== Python compile ===
All lint checks passed.

Operational Change Check

  • This is not an operational change.
  • This is an operational change and validation is recorded above.
  • This is an operational change and validation is intentionally deferred for:

Notes For Reviewers

  • Two assertions added to the existing tests/contracts/test-windows-hermes-config-patching.sh (already in make test). They are static greps, matching how the rest of that file guards Windows code — the repo has no PowerShell test runner wired into make test, and a pwsh-dependent test would skip in every CI job that currently runs. The behavioural evidence above was produced out-of-band instead; say the word if you would rather I add a pwsh-gated test file for it.
  • The other -replace sites in these files substitute integers or literals, so they are unaffected. 06-directories.ps1:481 interpolates a fixed here-string with no $ in it.
  • No behaviour change for values without a $.

…hing

Update-HermesConfigFile interpolated $Model and $BaseUrl straight into
-replace replacement strings, and the bootstrap .env rewrite did the same with
GgufFile/LlmModel. .NET reads '$' in a replacement as a substitution token, so:

  '$&' splices the entire matched line into the value —
        default: "qwen$&-turbo"  ->  default: "qwen  default: "old-model"-turbo"
  '$$' collapses to a single '$'

Double '$' in all four values before substituting. Verified against PowerShell
7.4; '$1' and '${x}' were already safe because the patterns carry no capture
groups, so the issue's example does not reproduce — '$&' and '$$' do.
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.

Update-HermesConfigFile mangles values containing '$' via .NET group-substitution tokens

1 participant