PowerShell transcripts should include the configuration name in the transcript header by chunqingchen · Pull Request #2890 · PowerShell/PowerShell · GitHub

Steps to reproduce

New-PSSessionConfigurationFile -Path .\myJeaConfig.pssc -TranscriptDirectory 'C:\temp\transcripts' -SessionType RestrictedRemoteServer
Register-PSSessionConfiguration -Name JEA -Path .\myJeaconfig.pssc
Enter-PSSession -ComputerName Localhost -ConfigurationName JEA

exit

Expected behavior

PowerShell transcripts currently do not log the configuration name the user used to connect to and manage the machine. For JEA scenarios, this means an auditor trying to understand how someone was able to do a certain command will not know through which endpoint the user entered and was assigned those privileges.

Suggestion is to add a new line to the transcript header similar to the following:

ConfigurationName: MyJEA

[…]
RunAs User: WinRM Virtual Users\WinRM VA_10_PRIV_priv.demo
Configuration Name: JEA
Machine: DC (Microsoft Windows NT 10.0.14393.0)
[…]

Actual behavior

[…]
RunAs User: WinRM Virtual Users\WinRM VA_10_PRIV_priv.demo
Machine: DC (Microsoft Windows NT 10.0.14393.0)
[…]

Environment data

Name Value

PSVersion 5.1.14993.1000
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.14993.1000
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

This may be more efficient for extracting the configuration name from the Shell Uri since it only allocates one string instead of multiple string objects:

string shellPrefix = System.Management.Automation.Remoting.Client.WSManNativeApi.ResourceURIPrefix;
int index = shelluri.IndexOf(shellPrefix, StringComparison.OrdinalIgnoreCase);
return (index == 0) ? shelluri.Substring(shellPrefix.Length) : string.Empty;
``` #Resolved

It would be better to parse the configuration name from the shellUri (configurationProcviderId) here rather than parse it each time later when a transcript header is written. #Resolved

You should prefer auto-properties where possible. #Resolved

Thanks Jason Shirk (@lzybkr), Paul Higinbotham (@PaulHigin) ! I have incorporated the suggested change. #Resolved

psConfigurationName [](start = 20, length = 19)

I believe that this cannot be null. Please make it default to empty string.

Paul Higinbotham (@PaulHigin) , I belive this is resolved.

Paul Higinbotham (@PaulHigin) the request has been addressed.

Chunqing Chen (@chunqingchen) Please include tests in this PR

test needs to be added

Chunqing Chen (@chunqingchen) You need to add yourself to Microsoft on GitHub

Chunqing Chen (@chunqingchen) I suspect you're already in the orgs, you just need to do this (for both PowerShell and Microsoft, though doing Microsoft is what will fix the CLA bot): https://help.github.com/articles/publicizing-or-hiding-organization-membership/

There are open comments that still need to be addressed. See: #2890 (comment)

We have been waiting on author response to PR comments for over a week.

There are open comments that still need to be addressed. See: #2890 (comment)

working on the test now

…ranscript header

resolved

Chunqing Chen (@chunqingchen) Please don't dismiss (delete) other people reviews. If you believe someone review is bad enough to be dismissed, please talk to a maintainer.

Travis Plunk (@TravisEz13) I meant the reviews are resolved. So dismiss is not the way to say it has been resolved. Got it.

Paul Higinbotham (@PaulHigin) Hi Paul, I talked with Travis and he agrees to merge the commit if you are fine. do you have any more comment about this change?

Please use more informative title.
Sample - "Transcript header".

Yes, please replace bug fix test with something more descriptive.

corrected

Please add new line.

This is recommended, but not required.

This was not fixed

Please replace -force with -Force.

corrected

Please replace aliases with full cmdlet names.

corrected

Please use Should BeLike or Should BeLikeExactly

corrected

Please replace -force with -Force.

corrected

I agree with Ilya (@iSazonov) comments. Otherwise LGTM.

I agree with @iSazonov comments. Otherwise LGTM.

I agree with Ilya (@iSazonov) comments. Otherwise LGTM.

…uration name in the transcript header #2890

Ilya (@iSazonov) Travis Plunk (@TravisEz13) comments are all resolved

Thanks!
Tests LGTM.

Great change!

…der (PowerShell#2890) * PowerShell transcripts should include the configuration name in the transcript header * adding test case for PowerShell transcripts should include the configuration name in the transcript header PowerShell#2890

Chunqing Chen (chunqingchen) requested a review from Paul Higinbotham (PaulHigin) December 15, 2016 03:39

Microsoft Contribution License Agreements (msftclas) added the cla-required label Dec 15, 2016

Chunqing Chen (chunqingchen) added cla-not-required and removed cla-required labels Dec 15, 2016

Chunqing Chen (chunqingchen) changed the title PowerShell transcripts should include the configuration name in the ranscript header #2888 PowerShell transcripts should include the configuration name in the ranscript header Dec 15, 2016

PowerShellTeam added the Review - Needed The PR is being reviewed label Dec 15, 2016

Paul Higinbotham (PaulHigin) previously requested changes Dec 15, 2016

View reviewed changes

Jason Shirk (lzybkr) reviewed Dec 15, 2016

View reviewed changes

Jason Shirk (lzybkr) changed the title PowerShell transcripts should include the configuration name in the ranscript header PowerShell transcripts should include the configuration name in the transcript header Dec 15, 2016

Paul Higinbotham (PaulHigin) reviewed Jan 3, 2017

View reviewed changes

Steve Lee (SteveL-MSFT) previously requested changes Jan 25, 2017

View reviewed changes

Mike Richmond (mirichmo) added Review - Waiting on Author and removed Review - Needed The PR is being reviewed labels Feb 17, 2017

Jason Shirk (lzybkr) assigned Travis Plunk (TravisEz13) Feb 23, 2017

Travis Plunk (TravisEz13) added cla-required and removed cla-not-required labels Feb 23, 2017

Travis Plunk (TravisEz13) closed this Feb 23, 2017

Travis Plunk (TravisEz13) reopened this Feb 23, 2017

Microsoft Contribution License Agreements (msftclas) added the cla-required label Feb 23, 2017

Travis Plunk (TravisEz13) closed this Feb 23, 2017

Travis Plunk (TravisEz13) reopened this Feb 23, 2017

Microsoft Contribution License Agreements (msftclas) added cla-not-required and removed cla-required labels Feb 23, 2017

Travis Plunk (TravisEz13) added the Stale label Mar 6, 2017

Travis Plunk (TravisEz13) removed the Review - Waiting on Author label Mar 6, 2017

PowerShell transcripts should include the configuration name in the t…

ac31745

…ranscript header

Travis Plunk (TravisEz13) previously approved these changes Mar 13, 2017

View reviewed changes

Ilya (iSazonov) reviewed Mar 15, 2017

View reviewed changes

Travis Plunk (TravisEz13) requested changes Mar 15, 2017

View reviewed changes

Travis Plunk (TravisEz13) added Review - Waiting on Author and removed Stale labels Mar 15, 2017

adding test case for PowerShell transcripts should include the config…

7d1ec00

…uration name in the transcript header #2890

Travis Plunk (TravisEz13) approved these changes Mar 15, 2017

View reviewed changes

Travis Plunk (TravisEz13) merged commit 3f274aa into PowerShell:master Mar 16, 2017

Ilya (iSazonov) removed the Review - Waiting on Author label Mar 27, 2017

PowerShell transcripts should include the configuration name in the transcript header by chunqingchen · Pull Request #2890 · PowerShell/PowerShell · GitHub

Uh oh!

PowerShell transcripts should include the configuration name in the transcript header - #2890

PowerShell transcripts should include the configuration name in the transcript header#2890Travis Plunk (TravisEz13) merged 2 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom chunqingchen:bugfix0Copy head branch name to clipboard

Conversation

Chunqing Chen (chunqingchen) commented Dec 15, 2016 • edited by joeyaiello Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Paul Higinbotham (PaulHigin) Dec 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Paul Higinbotham (PaulHigin) Dec 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jason Shirk (lzybkr) Dec 15, 2016 • edited by PaulHigin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chunqing Chen (chunqingchen) commented Dec 17, 2016 • edited by PaulHigin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chunqing Chen (chunqingchen) commented Jan 11, 2017

Uh oh!

Mike Richmond (mirichmo) commented Jan 11, 2017

Uh oh!

Steve Lee (SteveL-MSFT) left a comment

Choose a reason for hiding this comment

Uh oh!

Mike Richmond (mirichmo) commented Feb 17, 2017

Uh oh!

Joey Aiello (joeyaiello) commented Feb 17, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Feb 23, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Chunqing Chen (chunqingchen) commented Mar 7, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Mar 13, 2017

Uh oh!

Chunqing Chen (chunqingchen) commented Mar 13, 2017

Uh oh!

Chunqing Chen (chunqingchen) commented Mar 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Paul Higinbotham (PaulHigin) commented Mar 15, 2017

PowerShell transcripts should include the configuration name in the transcript header#2890
Travis Plunk (TravisEz13) merged 2 commits into
PowerShell:masterfrom
chunqingchen:bugfix0

Chunqing Chen (chunqingchen) commented Dec 15, 2016 •
edited by joeyaiello
Loading

Paul Higinbotham (PaulHigin) Dec 15, 2016 •
edited
Loading

Paul Higinbotham (PaulHigin) Dec 15, 2016 •
edited
Loading

Jason Shirk (lzybkr) Dec 15, 2016 •
edited by PaulHigin
Loading

Chunqing Chen (chunqingchen) commented Dec 17, 2016 •
edited by PaulHigin
Loading

Travis Plunk (TravisEz13) commented Mar 6, 2017 •
edited
Loading