actions / actions/runner

Runner crashes at InitializeSecretMasker on machines with Thai (th-TH) culture — culture-sensitive LastIndexOf in PowerShellPreAmpersandEscape

Open Beginner friendly
#4,686 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
6.3k
Forks
1.4k
Avg merge
1d 16h
Merged PRs (30d)
24

Description

Runner crashes at InitializeSecretMasker on machines with Thai (th-TH) culture

Every job fails before the first step runs, on a self-hosted Windows runner whose culture is th-TH.

Environment

Runner version 2.337.0
OS Windows 11 Pro 10.0.22631, Get-WinSystemLocale = th-TH
Install interactive (run.cmd), not a service
Repo secrets none (total_count: 0) — so the masked values are the ones the service sends with the job message

Symptom

Job ends in ~4 seconds with steps = 0. The check-run annotation shows:

System.ArgumentOutOfRangeException: Index and length must refer to a location within the string. (Parameter 'length')
   at System.String.ThrowSubstringArgumentOutOfRange(Int32 startIndex, Int32 length)
   at System.String.Substring(Int32 startIndex, Int32 length)
   at GitHub.DistributedTask.Logging.ValueEncoders.PowerShellPreAmpersandEscape(String value)
   at GitHub.DistributedTask.Logging.SecretMasker.AddValue(String value)
   at GitHub.Runner.Worker.Worker.InitializeSecretMasker(AgentJobRequestMessage message)
   at GitHub.Runner.Worker.Worker.RunAsync(String pipeIn, String pipeOut)

_diag/Worker_*.log confirms the culture:

Worker] Version: 2.337.0
Worker] Culture: th-TH
Worker] UI Culture: en-US
Worker] Message received.
Worker] ERR  System.ArgumentOutOfRangeException ...

Root cause

src/Sdk/DTLogging/Logging/ValueEncoders.cs:

if (!string.IsNullOrEmpty(value) && value.Contains("&"))          // Contains(string) -> ORDINAL
{
    if (value.Contains("&+"))
        secretSection = value.Substring(0, value.IndexOf("&+") + "&+".Length);      // IndexOf(string) -> CULTURE-SENSITIVE
    else
        secretSection = value.Substring(0, value.LastIndexOf("&") + "&".Length);    // LastIndexOf(string) -> CULTURE-SENSITIVE
}

String.Contains(string) is ordinal, but String.IndexOf(string) / String.LastIndexOf(string) are culture-sensitive by default. Under the Thai collation, LastIndexOf("&") returns an index one past the last character, so Substring(0, index + 1) reads past the end.

Minimal repro (.NET 9, ICU)
using System.Globalization;

foreach (var name in new[] { "th-TH", "en-US", "" })
{
    CultureInfo.CurrentCulture = name == "" ? CultureInfo.InvariantCulture : new CultureInfo(name);
    var s = "abcdef&";                       // length 7
    Console.WriteLine($"{name,-6} LastIndexOf(\"&\") = {s.LastIndexOf("&")}");
}

Output:

th-TH  LastIndexOf("&") = 7      <-- == s.Length, so Substring(0, 8) throws
en-US  LastIndexOf("&") = 6
       LastIndexOf("&") = 6      (invariant)

Same result for "abc&def&" (returns 8 for a length-8 string) and for a SAS-style URL "https://x/y?sig=AA&" (returns 19 for a length-19 string).

Any masked value that contains & is enough to kill every job on such a machine — and the values the service sends with a job message routinely contain &.

Suggested fix

Use ordinal comparisons, matching the Contains calls that guard them:

value.IndexOf("&+", StringComparison.Ordinal)
value.LastIndexOf("&", StringComparison.Ordinal)

(Or value.LastIndexOf('&') — the char overloads are already ordinal.)

A defensive clamp on the computed length would also prevent a crash in the masker from taking down the whole job, since a masking failure should never be fatal.

Workaround for affected users

Start the runner with globalization forced to invariant so the culture-sensitive path behaves ordinally:

DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1
DOTNET_SYSTEM_GLOBALIZATION_PREDEFINEDCULTURESONLY=false

Note this is inherited by job steps, which changes culture-dependent behaviour inside builds/tests — so it may need to be overridden per job (env: DOTNET_SYSTEM_GLOBALIZATION_INVARIANT: '0').

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/Sdk/DTLogging/Logging/ValueEncoders.cs at PowerShellPreAmpersandEscape, then trace its use from Worker.InitializeSecretMasker. Reproduce the reported behavior under th-TH using the minimal .NET example and verify that ampersand-containing masked values no longer cause an out-of-range exception during job initialization.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
ci-cd
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.