Skip to content

Commit b613dd7

Browse files
adityapatwardhanadityapatwardhan
authored
[release/v7.6.7] Tolerate IO/JSON errors reading the per-user powershell.config.json (#28084)
1 parent e8bfcaa commit b613dd7

3 files changed

Lines changed: 176 additions & 4 deletions

File tree

‎src/System.Management.Automation/engine/PSConfiguration.cs‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.IO;
77
using System.Management.Automation.Internal;
8+
using System.Security;
89
using System.Text;
910
using System.Threading;
1011

@@ -411,6 +412,23 @@ private T ReadValueFromFile<T>(ConfigScope scope, string key, T defaultValue = d
411412

412413
configData = serializer.Deserialize<JObject>(jsonReader) ?? emptyConfig;
413414
}
415+
catch (Exception exc) when (
416+
scope == ConfigScope.CurrentUser &&
417+
(exc is IOException || exc is UnauthorizedAccessException || exc is SecurityException || exc is JsonException))
418+
{
419+
// The per-user configuration file is unreadable (e.g. OneDrive cloud file provider
420+
// not running, permission denied, ACL denied, or corrupt/malformed JSON). Rather
421+
// than failing pwsh startup, fall back to defaults and emit a warning so the user
422+
// knows the file was skipped. See https://github.com/PowerShell/PowerShell/issues/27370.
423+
//
424+
// We intentionally do NOT apply this fallback to ConfigScope.AllUsers: that file is
425+
// admin-owned and on non-Windows platforms is the only source for security-relevant
426+
// policies (ScriptBlockLogging, Transcription, ConsoleSessionConfiguration, etc.).
427+
// If we cannot prove the admin's intent we must fail closed.
428+
TryWriteConfigWarning(StringUtil.Format(PSConfigurationStrings.CanNotReadConfigurationFile, fileName, exc.Message));
429+
430+
configData = emptyConfig;
431+
}
414432
catch (Exception exc)
415433
{
416434
throw PSTraceSource.NewInvalidOperationException(exc, PSConfigurationStrings.CanNotConfigurationFile, args: fileName);
@@ -435,12 +453,47 @@ private T ReadValueFromFile<T>(ConfigScope scope, string key, T defaultValue = d
435453

436454
if (configData != emptyConfig && configData.TryGetValue(key, StringComparison.OrdinalIgnoreCase, out JToken jToken))
437455
{
438-
return jToken.ToObject<T>(serializer) ?? defaultValue;
456+
try
457+
{
458+
return jToken.ToObject<T>(serializer) ?? defaultValue;
459+
}
460+
catch (JsonException exc) when (scope == ConfigScope.CurrentUser)
461+
{
462+
// The per-user JSON parsed successfully, but the value at <key> can't be
463+
// materialized as T (wrong type, malformed sub-document, etc.). Fall back
464+
// to the caller's default for this setting and emit a warning, mirroring
465+
// the file-level fallback above. AllUsers stays fail-closed for
466+
// security-relevant policies.
467+
// See https://github.com/PowerShell/PowerShell/issues/27370.
468+
TryWriteConfigWarning(StringUtil.Format(PSConfigurationStrings.CanNotReadConfigurationValue, key, fileName, exc.Message));
469+
return defaultValue;
470+
}
439471
}
440472

441473
return defaultValue;
442474
}
443475

476+
/// <summary>
477+
/// Best-effort emission of a warning when the per-user configuration file (or one of its
478+
/// values) can't be read. <see cref="ReadValueFromFile"/> is invoked very early in pwsh
479+
/// startup -- before any host, runspace, or PowerShell warning stream exists -- so we
480+
/// write directly to <see cref="Console.Error"/>. This matches existing startup-time
481+
/// stderr writes in ConsoleHost (e.g. CannotLoadPSReadline). Centralizing this here
482+
/// keeps formatting consistent and gives future hosts a single seam to swap the channel.
483+
/// Failures inside the writer itself are swallowed so logging issues never block startup.
484+
/// </summary>
485+
private static void TryWriteConfigWarning(string message)
486+
{
487+
try
488+
{
489+
Console.Error.WriteLine("WARNING: " + message);
490+
}
491+
catch
492+
{
493+
// Best-effort warning; never let logging failures block startup.
494+
}
495+
}
496+
444497
private static FileStream OpenFileStreamWithRetry(string fullPath, FileMode mode, FileAccess access, FileShare share)
445498
{
446499
const int MaxTries = 5;

‎src/System.Management.Automation/resources/PSConfigurationStrings.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,10 @@
120120
<data name="CanNotConfigurationFile" xml:space="preserve">
121121
<value>PowerShell has stopped working because of a security issue: Cannot read the configuration file: {0}</value>
122122
</data>
123+
<data name="CanNotReadConfigurationFile" xml:space="preserve">
124+
<value>Cannot read the PowerShell configuration file '{0}': {1}. Falling back to default settings.</value>
125+
</data>
126+
<data name="CanNotReadConfigurationValue" xml:space="preserve">
127+
<value>Cannot read setting '{0}' from PowerShell configuration file '{1}': {2}. Falling back to default for this setting.</value>
128+
</data>
123129
</root>

‎test/xUnit/csharp/test_PSConfiguration.cs‎

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,28 @@ public void SetupConfigFile5()
382382
CreateBrokenConfigFile(currentUserConfigFile);
383383
}
384384

385+
public void SetupBrokenCurrentUserConfigOnly()
386+
{
387+
CleanupConfigFiles();
388+
389+
// Only the per-user config file is broken; the system-wide file is absent.
390+
// Used to verify the #27370 fail-open path for ConfigScope.CurrentUser.
391+
CreateBrokenConfigFile(currentUserConfigFile);
392+
}
393+
394+
public void SetupTypeMismatchedCurrentUserConfig()
395+
{
396+
CleanupConfigFiles();
397+
398+
// Per-user config is syntactically valid JSON, but PowerShellPolicies has the
399+
// wrong nested shape: ScriptExecution should be an object but is a string.
400+
// This triggers JsonSerializationException inside jToken.ToObject<T>() and
401+
// exercises the per-key fail-open path for #27370.
402+
File.WriteAllText(
403+
currentUserConfigFile,
404+
"{ \"PowerShellPolicies\": { \"ScriptExecution\": \"not-an-object\" } }");
405+
}
406+
385407
private static void CreateBrokenConfigFile(string fileName)
386408
{
387409
File.WriteAllText(fileName, "[abbra");
@@ -978,14 +1000,105 @@ public void Utils_GetPolicySetting_BothConfigFilesNotExist()
9781000
fixture.CompareConsoleSessionConfiguration(consoleSessionConfiguration, null);
9791001
}
9801002

981-
[Fact, Priority(11)]
982-
public void PowerShellConfig_GetPowerShellPolicies_BrokenSystemConfig()
1003+
[Fact]
1004+
[Priority(11)]
1005+
public void PowerShellConfig_BrokenSystemConfig_Throws_BrokenCurrentUserConfig_FallsBack()
9831006
{
9841007
fixture.SetupConfigFile5();
9851008
fixture.ForceReadingFromFile();
9861009

1010+
// Broken system-wide config still hard-fails (admin-owned, security-relevant).
9871011
Assert.Throws<System.Management.Automation.PSInvalidOperationException>(() => PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.AllUsers));
988-
Assert.Throws<System.Management.Automation.PSInvalidOperationException>(() => PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser));
1012+
1013+
// Broken per-user config falls back to defaults (#27370).
1014+
PowerShellPolicies currentUserPolicies = PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser);
1015+
Assert.Null(currentUserPolicies);
1016+
}
1017+
1018+
[Fact]
1019+
[Priority(12)]
1020+
public void PowerShellConfig_BrokenCurrentUserConfig_FallsBackToDefaults()
1021+
{
1022+
fixture.SetupBrokenCurrentUserConfigOnly();
1023+
fixture.ForceReadingFromFile();
1024+
1025+
// ExecutionPolicy returns null (caller falls back to GP / registry / Restricted).
1026+
string execPolicy = PowerShellConfig.Instance.GetExecutionPolicy(ConfigScope.CurrentUser, "Microsoft.PowerShell");
1027+
Assert.Null(execPolicy);
1028+
1029+
// Module path returns null (caller uses built-in defaults).
1030+
string modulePath = PowerShellConfig.Instance.GetModulePath(ConfigScope.CurrentUser);
1031+
Assert.Null(modulePath);
1032+
1033+
// Experimental features list is empty.
1034+
string[] features = PowerShellConfig.Instance.GetExperimentalFeatures();
1035+
Assert.Empty(features);
1036+
}
1037+
1038+
[Fact]
1039+
[Priority(13)]
1040+
public void PowerShellConfig_TypeMismatchInCurrentUserConfig_FallsBackToDefaults()
1041+
{
1042+
// Verifies the per-key fail-open path: the file parses as JSON, but a value
1043+
// can't be materialized as its target type. Without this fallback the
1044+
// JsonSerializationException would propagate out of GetPolicySetting and
1045+
// crash startup just like #27370.
1046+
fixture.SetupTypeMismatchedCurrentUserConfig();
1047+
fixture.ForceReadingFromFile();
1048+
1049+
PowerShellPolicies policies = PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser);
1050+
Assert.Null(policies);
1051+
}
1052+
1053+
[Fact]
1054+
[Priority(14)]
1055+
public void PowerShellConfig_BrokenCurrentUserConfig_EmitsWarningToStderr()
1056+
{
1057+
// A regression in the warning path would still let startup succeed (this is the
1058+
// whole point of the fail-open) -- but the user would silently lose their config
1059+
// with no indication. This test pins the user-visible warning so that doesn't
1060+
// happen quietly.
1061+
fixture.SetupBrokenCurrentUserConfigOnly();
1062+
fixture.ForceReadingFromFile();
1063+
1064+
string stderr = CaptureStderr(() =>
1065+
PowerShellConfig.Instance.GetExecutionPolicy(ConfigScope.CurrentUser, "Microsoft.PowerShell"));
1066+
1067+
Assert.Contains("WARNING:", stderr);
1068+
Assert.Contains("Falling back to default settings", stderr);
1069+
}
1070+
1071+
[Fact]
1072+
[Priority(15)]
1073+
public void PowerShellConfig_TypeMismatchInCurrentUserConfig_EmitsWarningToStderr()
1074+
{
1075+
// Pin the user-visible warning for the per-key fail-open path as well.
1076+
fixture.SetupTypeMismatchedCurrentUserConfig();
1077+
fixture.ForceReadingFromFile();
1078+
1079+
string stderr = CaptureStderr(() =>
1080+
PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser));
1081+
1082+
Assert.Contains("WARNING:", stderr);
1083+
Assert.Contains("PowerShellPolicies", stderr);
1084+
Assert.Contains("Falling back to default for this setting", stderr);
1085+
}
1086+
1087+
private static string CaptureStderr(Action action)
1088+
{
1089+
var captured = new StringWriter();
1090+
TextWriter originalError = Console.Error;
1091+
try
1092+
{
1093+
Console.SetError(captured);
1094+
action();
1095+
}
1096+
finally
1097+
{
1098+
Console.SetError(originalError);
1099+
}
1100+
1101+
return captured.ToString();
9891102
}
9901103
}
9911104
}

0 commit comments

Comments
 (0)