Runner crashes at InitializeSecretMasker on machines with Thai (th-TH) culture — culture-sensitive LastIndexOf in PowerShellPreAmpersandEscape
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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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