Skip to content

Add additional language mode checks to PSSetMemberBinder (#202) - #28215

Open
Aditya Patwardhan (adityapatwardhan) wants to merge 1 commit into
masterfrom
binder-gh
Open

Aditya Patwardhan (adityapatwardhan) wants to merge 1 commit into
masterfrom
binder-gh

Conversation

@adityapatwardhan

Copy link
Copy Markdown
Member

PR Summary

This pull request enhances the enforcement of PowerShell's Constrained Language Mode (CLM) restrictions on setting instance members, ensuring that only NoteProperty assignments are allowed and that other property types (like ScriptProperty and AliasProperty) are properly blocked. It introduces a new helper method to centralize this logic, updates the dynamic binder to use it, and adds targeted tests to verify the behavior.

Security enforcement and property assignment logic:

  • Added a new static method SetInstanceMemberValue in PSSetMemberBinder to enforce CLM restrictions when setting instance member values, allowing only NoteProperty assignments and auditing or blocking others as appropriate.
  • Updated the dynamic binder logic in FallbackSetMember to use the new SetInstanceMemberValue method, replacing direct property assignments and ensuring consistent enforcement of CLM rules. [1] [2]
  • Improved handling in FallbackSetMember for property setting in CLM: now directly checks language mode and type, blocks or audits as needed, and logs audit messages for non-core types when enforcement is set to audit.

Reflection and infrastructure:

  • Added a cached reflection entry for the new PSSetMemberBinder.SetInstanceMemberValue method in CachedReflectionInfo to support dynamic invocation.

Testing:

  • Added new Pester tests to verify that setting ScriptProperty and AliasProperty instance members is correctly rejected in Constrained Language Mode, while ensuring language mode is reset after each test.

PR Context

PR Checklist

Copilot AI balanced review requested due to automatic review settings October 14, 2026 23:03
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Oct 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Security-boundary enforcement and audit behavior need human validation, with positive coverage still needed for permitted assignments.

1 open finding
What changed in this PR

Strengthens Constrained Language Mode enforcement for member assignments in PowerShell’s dynamic binder.

Changes:

  • Centralizes instance-member checks and updates enforcement and audit handling.
  • Adds cached reflection metadata for the helper.
  • Adds rejection tests for script and alias properties.
File Description
test/​powershell/​Modules/​Microsoft.PowerShell.Security/​ConstrainedLanguageRestriction.Tests.ps1 Tests restricted property assignments.
src/​System.Management.Automation/​engine/​runtime/​Binding/​Binders.cs Enforces language-mode restrictions during assignment.
src/​System.Management.Automation/​engine/​parser/​Compiler.cs Caches reflection metadata for the new helper.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

ExecutionContext context = LocalPipeline.GetExecutionContextFromTLS();
// Explicit `GetType() ==` instead of `is` as we only want to allow `PSNoteProperty`,
// classes that inherits from it should be excluded.
if ((context != null) && (context.LanguageMode == PSLanguageMode.ConstrainedLanguage) && (memberInfo.GetType() != typeof(PSNoteProperty)))
@iSazonov

Copy link
Copy Markdown
Collaborator

Dup #28215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants