Skip to content

Avoid decorating ErrorRecord during CliXml serialization#27528

Open
KirtiRamchandani wants to merge 2 commits into
PowerShell:masterfrom
KirtiRamchandani:fix/clixml-errorrecord-decoration
Open

Avoid decorating ErrorRecord during CliXml serialization#27528
KirtiRamchandani wants to merge 2 commits into
PowerShell:masterfrom
KirtiRamchandani:fix/clixml-errorrecord-decoration

Conversation

@KirtiRamchandani
Copy link
Copy Markdown

PR Summary

Fixes #25854.

ConvertTo-CliXml and Export-CliXml serialize ErrorRecord instances by adding remoting-specific note properties such as ErrorCategory_Activity and InvocationInfo. The serializer was adding those properties to the caller's PSObject wrapper, which caused the original ErrorRecord to gain serialization-only extended members after serialization completed.

This change serializes remoting records through a wrapper with local instance members, preserving the generated CliXml fields without decorating the caller's input object.

PR Context

PSObject.Copy() alone still shares the base object's resurrection-table key, so note properties added to the copy are visible on wrappers around the same base object. The new helper resets the serialization wrapper's instance-member collection before adding remoting-only properties, while preserving existing visible instance members for serialization.

The same path is used for informational records, so the isolation is applied there too.

PR Checklist

  • Tests pass

Tests

  • Reproduced before fix: serializing an ErrorRecord increased extended members from 1 to 13 and added ErrorCategory_*, InvocationInfo, SerializeExtendedInfo, and related serialization notes.
  • Added regression coverage for both Export-Clixml and ConvertTo-CliXml.
  • Start-PSBuild -Configuration Debug -SMAOnly
  • Manual verification after fix: ConvertTo-CliXml and Export-Clixml added no extended members to the original ErrorRecord.
  • Manual verification after fix: generated CliXml still contains ErrorCategory_Category and ConvertFrom-CliXml round-trips the error information.
  • Start-PSPester -Path .\test\powershell\Modules\Microsoft.PowerShell.Utility\clixml.tests.ps1 -Terse -ThrowOnFailure -SkipTestToolBuild -BinDir <publish> (19 passed)
  • git diff --check (line-ending warning only)

@KirtiRamchandani KirtiRamchandani requested a review from a team as a code owner May 25, 2026 04:29
Copilot AI review requested due to automatic review settings May 25, 2026 04:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR prevents Export-Clixml / ConvertTo-CliXml remoting serialization from mutating the input ErrorRecord (and InformationalRecord) by adding remoting-only members to the original object, and adds regression coverage for ErrorRecord.

Changes:

  • Add Pester tests asserting ErrorRecord does not gain new extended members after CliXml serialization.
  • Update serialization logic to serialize a remoting-only wrapper PSObject instead of mutating the original PSObject for ErrorRecord / InformationalRecord.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
test/powershell/Modules/Microsoft.PowerShell.Utility/clixml.tests.ps1 Adds regression tests and a helper to compare extended member sets pre/post serialization.
src/System.Management.Automation/engine/serialization.cs Introduces a wrapper-copy strategy for remoting serialization to avoid persisting remoting-only members on the source object.

Comment on lines +195 to +198
$filePath = Join-Path $subFilePath 'error.xml'
Write-Error foo 2>$null
$errorRecord = $Error[0]
$before = Get-ExtendedMemberName $errorRecord
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 16245c451: the Export-Clixml test now captures the produced ErrorRecord directly through New-TestErrorRecord instead of reading $Error[0].

Comment on lines +255 to +257
Write-Error foo 2>$null
$errorRecord = $Error[0]
$before = Get-ExtendedMemberName $errorRecord
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 16245c451: the ConvertTo-CliXml test now uses the same deterministic ErrorRecord capture helper instead of depending on global error state.

Comment on lines +39 to +43
function Get-ExtendedMemberName {
param([object] $InputObject)

@($InputObject | Get-Member -View Extended | Select-Object -ExpandProperty Name)
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 16245c451: the helper is now named Get-ExtendedMemberNames to match its collection result.

Comment on lines +1492 to +1498
foreach (PSMemberInfo member in source.InstanceMembers)
{
if (!member.IsHidden)
{
target.Members.Add(member);
}
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 16245c451: the remoting serialization wrapper now adds member.Copy() to target.InstanceMembers, so the wrapper owns independent member instances.

Comment on lines 1541 to 1547
InformationalRecord informationalRecord = mshSource.ImmediateBaseObject as InformationalRecord;
if (informationalRecord != null)
{
mshSource = GetRemotingSerializationTarget(mshSource);
informationalRecord.ToPSObjectForRemoting(mshSource);
isInformationalRecord = true;
break;
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 16245c451: added matching Export-Clixml and ConvertTo-CliXml regression coverage for InformationRecord inputs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ConvertTo-Clixml and Export-CliXml inappropriately decorate their input objects with NoteProperty members introduced for serialization

2 participants