Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion src/System.Management.Automation/engine/PSConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.IO;
using System.Management.Automation.Internal;
using System.Security;
using System.Text;
using System.Threading;

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

configData = serializer.Deserialize<JObject>(jsonReader) ?? emptyConfig;
}
catch (Exception exc) when (
scope == ConfigScope.CurrentUser &&
(exc is IOException || exc is UnauthorizedAccessException || exc is SecurityException || exc is JsonException))
Comment on lines +415 to +417
{
// The per-user configuration file is unreadable (e.g. OneDrive cloud file provider
// not running, permission denied, ACL denied, or corrupt/malformed JSON). Rather
// than failing pwsh startup, fall back to defaults and emit a warning so the user
// knows the file was skipped. See https://github.com/PowerShell/PowerShell/issues/27370.
//
// We intentionally do NOT apply this fallback to ConfigScope.AllUsers: that file is
// admin-owned and on non-Windows platforms is the only source for security-relevant
// policies (ScriptBlockLogging, Transcription, ConsoleSessionConfiguration, etc.).
// If we cannot prove the admin's intent we must fail closed.
TryWriteConfigWarning(StringUtil.Format(PSConfigurationStrings.CanNotReadConfigurationFile, fileName, exc.Message));

configData = emptyConfig;
}
catch (Exception exc)
{
throw PSTraceSource.NewInvalidOperationException(exc, PSConfigurationStrings.CanNotConfigurationFile, args: fileName);
Expand All @@ -435,12 +453,47 @@ private T ReadValueFromFile<T>(ConfigScope scope, string key, T defaultValue = d

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

return defaultValue;
}

/// <summary>
/// Best-effort emission of a warning when the per-user configuration file (or one of its
/// values) can't be read. <see cref="ReadValueFromFile"/> is invoked very early in pwsh
/// startup -- before any host, runspace, or PowerShell warning stream exists -- so we
/// write directly to <see cref="Console.Error"/>. This matches existing startup-time
/// stderr writes in ConsoleHost (e.g. CannotLoadPSReadline). Centralizing this here
/// keeps formatting consistent and gives future hosts a single seam to swap the channel.
/// Failures inside the writer itself are swallowed so logging issues never block startup.
/// </summary>
private static void TryWriteConfigWarning(string message)
{
try
{
Console.Error.WriteLine("WARNING: " + message);
}
catch
{
// Best-effort warning; never let logging failures block startup.
}
}

private static FileStream OpenFileStreamWithRetry(string fullPath, FileMode mode, FileAccess access, FileShare share)
{
const int MaxTries = 5;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,10 @@
<data name="CanNotConfigurationFile" xml:space="preserve">
<value>PowerShell has stopped working because of a security issue: Cannot read the configuration file: {0}</value>
</data>
<data name="CanNotReadConfigurationFile" xml:space="preserve">
<value>Cannot read the PowerShell configuration file '{0}': {1}. Falling back to default settings.</value>
</data>
<data name="CanNotReadConfigurationValue" xml:space="preserve">
<value>Cannot read setting '{0}' from PowerShell configuration file '{1}': {2}. Falling back to default for this setting.</value>
</data>
</root>
119 changes: 116 additions & 3 deletions test/xUnit/csharp/test_PSConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,28 @@ public void SetupConfigFile5()
CreateBrokenConfigFile(currentUserConfigFile);
}

public void SetupBrokenCurrentUserConfigOnly()
{
CleanupConfigFiles();

// Only the per-user config file is broken; the system-wide file is absent.
// Used to verify the #27370 fail-open path for ConfigScope.CurrentUser.
CreateBrokenConfigFile(currentUserConfigFile);
}

public void SetupTypeMismatchedCurrentUserConfig()
{
CleanupConfigFiles();

// Per-user config is syntactically valid JSON, but PowerShellPolicies has the
// wrong nested shape: ScriptExecution should be an object but is a string.
// This triggers JsonSerializationException inside jToken.ToObject<T>() and
// exercises the per-key fail-open path for #27370.
File.WriteAllText(
currentUserConfigFile,
"{ \"PowerShellPolicies\": { \"ScriptExecution\": \"not-an-object\" } }");
}

private static void CreateBrokenConfigFile(string fileName)
{
File.WriteAllText(fileName, "[abbra");
Expand Down Expand Up @@ -978,14 +1000,105 @@ public void Utils_GetPolicySetting_BothConfigFilesNotExist()
fixture.CompareConsoleSessionConfiguration(consoleSessionConfiguration, null);
}

[Fact, Priority(11)]
public void PowerShellConfig_GetPowerShellPolicies_BrokenSystemConfig()
[Fact]
[Priority(11)]
public void PowerShellConfig_BrokenSystemConfig_Throws_BrokenCurrentUserConfig_FallsBack()
{
fixture.SetupConfigFile5();
fixture.ForceReadingFromFile();

// Broken system-wide config still hard-fails (admin-owned, security-relevant).
Assert.Throws<System.Management.Automation.PSInvalidOperationException>(() => PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.AllUsers));
Assert.Throws<System.Management.Automation.PSInvalidOperationException>(() => PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser));

// Broken per-user config falls back to defaults (#27370).
PowerShellPolicies currentUserPolicies = PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser);
Assert.Null(currentUserPolicies);
}

[Fact]
[Priority(12)]
public void PowerShellConfig_BrokenCurrentUserConfig_FallsBackToDefaults()
{
fixture.SetupBrokenCurrentUserConfigOnly();
fixture.ForceReadingFromFile();

// ExecutionPolicy returns null (caller falls back to GP / registry / Restricted).
string execPolicy = PowerShellConfig.Instance.GetExecutionPolicy(ConfigScope.CurrentUser, "Microsoft.PowerShell");
Assert.Null(execPolicy);

// Module path returns null (caller uses built-in defaults).
string modulePath = PowerShellConfig.Instance.GetModulePath(ConfigScope.CurrentUser);
Assert.Null(modulePath);

// Experimental features list is empty.
string[] features = PowerShellConfig.Instance.GetExperimentalFeatures();
Assert.Empty(features);
}

[Fact]
[Priority(13)]
public void PowerShellConfig_TypeMismatchInCurrentUserConfig_FallsBackToDefaults()
{
// Verifies the per-key fail-open path: the file parses as JSON, but a value
// can't be materialized as its target type. Without this fallback the
// JsonSerializationException would propagate out of GetPolicySetting and
// crash startup just like #27370.
fixture.SetupTypeMismatchedCurrentUserConfig();
fixture.ForceReadingFromFile();

PowerShellPolicies policies = PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser);
Assert.Null(policies);
}

[Fact]
[Priority(14)]
public void PowerShellConfig_BrokenCurrentUserConfig_EmitsWarningToStderr()
{
// A regression in the warning path would still let startup succeed (this is the
// whole point of the fail-open) -- but the user would silently lose their config
// with no indication. This test pins the user-visible warning so that doesn't
// happen quietly.
fixture.SetupBrokenCurrentUserConfigOnly();
fixture.ForceReadingFromFile();

string stderr = CaptureStderr(() =>
PowerShellConfig.Instance.GetExecutionPolicy(ConfigScope.CurrentUser, "Microsoft.PowerShell"));

Assert.Contains("WARNING:", stderr);
Assert.Contains("Falling back to default settings", stderr);
}

[Fact]
[Priority(15)]
public void PowerShellConfig_TypeMismatchInCurrentUserConfig_EmitsWarningToStderr()
{
// Pin the user-visible warning for the per-key fail-open path as well.
fixture.SetupTypeMismatchedCurrentUserConfig();
fixture.ForceReadingFromFile();

string stderr = CaptureStderr(() =>
PowerShellConfig.Instance.GetPowerShellPolicies(ConfigScope.CurrentUser));

Assert.Contains("WARNING:", stderr);
Assert.Contains("PowerShellPolicies", stderr);
Assert.Contains("Falling back to default for this setting", stderr);
}

private static string CaptureStderr(Action action)
{
var captured = new StringWriter();
TextWriter originalError = Console.Error;
try
{
Console.SetError(captured);
action();
}
finally
{
Console.SetError(originalError);
}

return captured.ToString();
}
}
}
Loading