Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ReadOnlyProviders option for session configuration files #21528

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 36 additions & 5 deletions src/System.Management.Automation/engine/InitialSessionState.cs
Expand Up @@ -504,15 +504,38 @@ internal SessionStateProviderEntry(string name, Type implementingType, string he
HelpFileName = helpFileName;
}

internal SessionStateProviderEntry(
string name,
Type implementingType,
string helpFileName,
SessionStateEntryVisibility visibility,
ScopedItemOptions options)
: base(name, visibility)
{
ImplementingType = implementingType;
HelpFileName = helpFileName;
Debug.Assert(
options is ScopedItemOptions.None or ScopedItemOptions.ReadOnly,
$"Caller should ensure that \"{nameof(options)}\" is {nameof(ScopedItemOptions.None)} or {nameof(ScopedItemOptions.ReadOnly)}.");

Options = options;
}

/// <summary>
/// Shallow-clone this object...
/// </summary>
/// <returns>The cloned object.</returns>
public override InitialSessionStateEntry Clone()
{
SessionStateProviderEntry entry = new SessionStateProviderEntry(Name, ImplementingType, HelpFileName, this.Visibility);
entry.SetPSSnapIn(this.PSSnapIn);
entry.SetModule(this.Module);
SessionStateProviderEntry entry = new(
Name,
ImplementingType,
HelpFileName,
Visibility,
Options);

entry.SetPSSnapIn(PSSnapIn);
entry.SetModule(Module);
return entry;
}

Expand All @@ -523,6 +546,10 @@ public override InitialSessionStateEntry Clone()
/// <summary>
/// </summary>
public string HelpFileName { get; }

/// <summary>
/// </summary>
public ScopedItemOptions Options { get; internal set; }
}

/// <summary>
Expand Down Expand Up @@ -3079,8 +3106,12 @@ private Exception ProcessPowerShellCommand(PowerShell psToInvoke, Runspace initi
}
else
{
cmd.Visibility = SessionStateEntryVisibility.Public;
publicCommands.Add(cmd);
// These CommandInfo implementations will throw when attempting to set Visibility
if (cmd is not ApplicationInfo and not AliasInfo and not ExternalScriptInfo)
{
cmd.Visibility = SessionStateEntryVisibility.Public;
publicCommands.Add(cmd);
}
}
}
catch (PSNotImplementedException)
Expand Down
Expand Up @@ -483,6 +483,25 @@ public string[] VisibleProviders

private string[] _visibleProviders = Array.Empty<string>();

/// <summary>
/// A list of read-only providers.
/// </summary>
[Parameter]
public string[] ReadOnlyProviders
{
get
{
return _readOnlyProviders;
}

set
{
_readOnlyProviders = value;
}
}

private string[] _readOnlyProviders = Array.Empty<string>();

/// <summary>
/// A list of aliases.
/// </summary>
Expand Down Expand Up @@ -917,6 +936,13 @@ protected override void ProcessRecord()
SessionConfigurationUtils.GetVisibilityDefault(_visibleProviders, streamWriter, this), streamWriter, _visibleProviders.Length == 0));
}

// Visible providers
if (ShouldGenerateConfigurationSnippet("ReadOnlyProviders"))
{
result.Append(SessionConfigurationUtils.ConfigFragment(ConfigFileConstants.ReadOnlyProviders, RemotingErrorIdStrings.DISCReadOnlyProvidersComment,
SessionConfigurationUtils.GetVisibilityDefault(ReadOnlyProviders, streamWriter, this), streamWriter, ReadOnlyProviders.Length == 0));
}

// Alias definitions
if ((_aliasDefinitions == null) || (_aliasDefinitions.Length == 0))
{
Expand Down Expand Up @@ -1371,6 +1397,12 @@ public string[] VisibleProviders

private string[] _visibleProviders = Array.Empty<string>();

/// <summary>
/// A list of read-only providers.
/// </summary>
[Parameter]
public string[] ReadOnlyProviders { get; set; } = Array.Empty<string>();

/// <summary>
/// Scripts to process.
/// </summary>
Expand Down Expand Up @@ -1664,6 +1696,10 @@ protected override void ProcessRecord()
result.Append(SessionConfigurationUtils.ConfigFragment(ConfigFileConstants.VisibleProviders, RemotingErrorIdStrings.DISCVisibleProvidersComment,
SessionConfigurationUtils.GetVisibilityDefault(_visibleProviders, streamWriter, this), streamWriter, _visibleProviders.Length == 0));

// Read-only providers
result.Append(SessionConfigurationUtils.ConfigFragment(ConfigFileConstants.ReadOnlyProviders, RemotingErrorIdStrings.DISCReadOnlyProvidersComment,
SessionConfigurationUtils.GetVisibilityDefault(ReadOnlyProviders, streamWriter, this), streamWriter, ReadOnlyProviders.Length == 0));

// Scripts to process
string resultData = (_scriptsToProcess.Length > 0) ? SessionConfigurationUtils.CombineStringArray(_scriptsToProcess) : "'C:\\ConfigData\\InitScript1.ps1', 'C:\\ConfigData\\InitScript2.ps1'";
result.Append(SessionConfigurationUtils.ConfigFragment(ConfigFileConstants.ScriptsToProcess, RemotingErrorIdStrings.DISCScriptsToProcessComment,
Expand Down
Expand Up @@ -967,6 +967,7 @@ internal static class ConfigFileConstants
internal static readonly string VisibleCmdlets = "VisibleCmdlets";
internal static readonly string VisibleFunctions = "VisibleFunctions";
internal static readonly string VisibleProviders = "VisibleProviders";
internal static readonly string ReadOnlyProviders = "ReadOnlyProviders";
internal static readonly string VisibleExternalCommands = "VisibleExternalCommands";

internal static readonly ConfigTypeEntry[] ConfigFileKeys = new ConfigTypeEntry[] {
Expand Down Expand Up @@ -1004,6 +1005,7 @@ internal static class ConfigFileConstants
new ConfigTypeEntry(VisibleCmdlets, new ConfigTypeEntry.TypeValidationCallback(StringArrayTypeValidationCallback)),
new ConfigTypeEntry(VisibleFunctions, new ConfigTypeEntry.TypeValidationCallback(StringArrayTypeValidationCallback)),
new ConfigTypeEntry(VisibleProviders, new ConfigTypeEntry.TypeValidationCallback(StringArrayTypeValidationCallback)),
new ConfigTypeEntry(ReadOnlyProviders, new ConfigTypeEntry.TypeValidationCallback(StringArrayTypeValidationCallback)),
new ConfigTypeEntry(VisibleExternalCommands, new ConfigTypeEntry.TypeValidationCallback(StringArrayTypeValidationCallback)),
};

Expand Down Expand Up @@ -1402,6 +1404,7 @@ internal static class DISCUtils
"VisibleFunctions",
"VisibleExternalCommands",
"VisibleProviders",
"ReadOnlyProviders",
"ScriptsToProcess",
"AliasDefinitions",
"FunctionDefinitions",
Expand Down Expand Up @@ -1987,6 +1990,7 @@ public override InitialSessionState GetInitialSessionState(PSSenderInfo senderIn
bool functionVisibilityApplied = IsNonDefaultVisibilitySpecified(ConfigFileConstants.VisibleFunctions);
bool aliasVisibilityApplied = IsNonDefaultVisibilitySpecified(ConfigFileConstants.VisibleAliases);
bool providerVisibilityApplied = IsNonDefaultVisibilitySpecified(ConfigFileConstants.VisibleProviders);
bool readonlyProvidersApplied = IsNonDefaultVisibilitySpecified(ConfigFileConstants.ReadOnlyProviders);
bool processDefaultSessionStateVisibility = false;

if (!string.IsNullOrEmpty(initialSessionState))
Expand Down Expand Up @@ -2029,28 +2033,45 @@ public override InitialSessionState GetInitialSessionState(PSSenderInfo senderIn
}
}

if (readonlyProvidersApplied)
{
string[] providers = TryGetStringArray(_configHash[ConfigFileConstants.ReadOnlyProviders]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code below this for providerVisibilityApplied uses a case-insensitive hashtable which looks unnecessary to me? The asymmetry between this and the latter would create confusion for future people reading the code. Seems like we either need to either make them the same or add a comment why they are different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code below this for providerVisibilityApplied uses a case-insensitive hashtable which looks unnecessary to me? The asymmetry between this and the latter would create confusion for future people reading the code. Seems like we either need to either make them the same or add a comment why they are different.

Thanks Steve! Yep the hashset seems to do nothing at all, I went ahead and removed it there as well.


if (providers != null)
{
foreach (string provider in providers)
{
if (!string.IsNullOrEmpty(provider))
{
// Look up providers from provider name including wildcards.
var providersFound = iss.Providers.LookUpByName(provider);

foreach (SessionStateProviderEntry providerFound in providersFound)
{
providerFound.Options = ScopedItemOptions.ReadOnly;
}
}
}
}
}

// Add providers
if (providerVisibilityApplied)
{
string[] providers = TryGetStringArray(_configHash[ConfigFileConstants.VisibleProviders]);

if (providers != null)
{
System.Collections.Generic.HashSet<string> addedProviders = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (string provider in providers)
{
if (!string.IsNullOrEmpty(provider))
{
// Look up providers from provider name including wildcards.
var providersFound = iss.Providers.LookUpByName(provider);

foreach (var providerFound in providersFound)
foreach (SessionStateProviderEntry providerFound in providersFound)
{
if (!addedProviders.Contains(providerFound.Name))
{
addedProviders.Add(providerFound.Name);
providerFound.Visibility = SessionStateEntryVisibility.Public;
}
providerFound.Visibility = SessionStateEntryVisibility.Public;
}
}
}
Expand Down Expand Up @@ -2955,7 +2976,8 @@ internal static class DISCFileValidation
"VariableDefinitions",
"VisibleExternalCommands",
"VisibleFunctions",
"VisibleProviders"
"VisibleProviders",
"ReadOnlyProviders",
};
#else
private static readonly HashSet<string> SupportedConfigOptions = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
Expand Down Expand Up @@ -2985,7 +3007,8 @@ internal static class DISCFileValidation
"VariableDefinitions",
"VisibleExternalCommands",
"VisibleFunctions",
"VisibleProviders"
"VisibleProviders",
"ReadOnlyProviders",
};
#endif

Expand Down
16 changes: 15 additions & 1 deletion src/System.Management.Automation/namespaces/ProviderBase.cs
Expand Up @@ -938,6 +938,21 @@ protected internal virtual void StopProcessing()
SessionStateStrings.IContentCmdletProvider_NotSupported);
}

Collection<SessionStateProviderEntry> providerEntries =
Context.ExecutionContext.InitialSessionState.Providers[Context.ProviderInstance.ProviderInfo.Name];

foreach (SessionStateProviderEntry providerEntry in providerEntries)
{
if (providerEntry.Options.HasFlag(ScopedItemOptions.ReadOnly))
{
throw new SessionStateUnauthorizedAccessException(
Context.ProviderInstance.ProviderInfo.Name,
SessionStateCategory.CmdletProvider,
nameof(SessionStateStrings.ProviderNotWritable),
SessionStateStrings.ProviderNotWritable);
}
}

// Call interface method

return contentProvider.GetContentWriter(path);
Expand Down Expand Up @@ -1984,4 +1999,3 @@ public void WriteError(ErrorRecord errorRecord)
}

#pragma warning restore 56506

Expand Up @@ -1062,6 +1062,9 @@ All WinRM sessions connected to PowerShell session configurations, such as Micro
<data name="DISCVisibleProvidersComment" xml:space="preserve">
<value>Providers to make visible when applied to a session</value>
</data>
<data name="DISCReadOnlyProvidersComment" xml:space="preserve">
<value>Providers to make read-only when applied to a session</value>
</data>
<data name="DISCVisibleExternalCommandsComment" xml:space="preserve">
<value>External commands (scripts and applications) to make visible when applied to a session</value>
</data>
Expand Down
Expand Up @@ -342,6 +342,9 @@
<data name="AliasNotWritable" xml:space="preserve">
<value>Alias is not writeable because alias {0} is read-only or constant and cannot be written to.</value>
</data>
<data name="ProviderNotWritable" xml:space="preserve">
<value>Cannot write to provider {0} because it is read-only.</value>
</data>
<data name="FunctionNotWritable" xml:space="preserve">
<value>Cannot write to function {0} because it is read-only or constant.</value>
</data>
Expand Down
Expand Up @@ -707,6 +707,7 @@ namespace PowershellTestConfigNamespace
VisibleVariables = "*"
LanguageMode = "RestrictedLanguage"
ExecutionPolicy = "AllSigned"
ReadOnlyProviders = 'Environment'
}
}
}
Expand Down Expand Up @@ -785,7 +786,8 @@ namespace PowershellTestConfigNamespace
-LanguageMode $parmMap.LanguageMode `
-ExecutionPolicy $parmMap.ExecutionPolicy `
-ScriptsToProcess $parmMap.ScriptsToProcess `
-GUID $parmMap.GUID
-GUID $parmMap.GUID `
-ReadOnlyProviders $parmMap.ReadOnlyProviders

# Verify the generated config file using the Test-PSSessionConfigurationFile
$result = Test-PSSessionConfigurationFile -Path $configFilePath -Verbose
Expand Down Expand Up @@ -847,3 +849,50 @@ finally
$WarningPreference = $originalWarningPreference
}

Describe 'Test PSSession configuration file options' -Tags CI {
BeforeAll {
$ConfigFilePath = Join-Path $TestDrive "TestPSSessionConfigurationFile.pssc"
$PSFileName = $IsWindows ? 'pwsh.exe' : 'pwsh'
$PSPath = Join-Path -Path $PSHOME -ChildPath $psFileName
}

It 'ReadOnlyProviders cannot be written to.' {
$configParams = @{
SchemaVersion = '2.0.0.0'
GUID = [guid]::NewGuid()
Author = 'User'
SessionType = 'RestrictedRemoteServer'
LanguageMode = 'ConstrainedLanguage'
VisibleAliases = '*'
VisibleCmdlets = '*'
VisibleFunctions = '*'
VisibleExternalCommands = '*'
VisibleProviders = 'FileSystem', 'Registry', 'Environment', 'Function'
ReadOnlyProviders = 'Environment'
}

if (Test-Path -LiteralPath $ConfigFilePath) {
Remove-Item -LiteralPath $ConfigFilePath
}

try {
{ New-PSSessionConfigurationFile @configParams -Path $ConfigFilePath } | Should -Not -Throw
$env:READONLY_PROVIDER_TESTING_VALUE = '1'

{
$results = & $PSPath -NonInteractive -NoProfile -ConfigurationFile $ConfigFilePath { $env:READONLY_PROVIDER_TESTING_VALUE = '2' } 2>&1
foreach ($result in $results) {
if ($result -is [System.Management.Automation.ErrorRecord]) {
throw $result
}
}
} | Should -Throw -ErrorId ProviderNotWritable
} finally {
$env:READONLY_PROVIDER_TESTING_VALUE = ''
if (Test-Path -LiteralPath $ConfigFilePath) {
Remove-Item -LiteralPath $ConfigFilePath
}
}

}
}