Skip to content

Commit c6835f4

Browse files
committed
Update logging to redact AccessToken if provided at commandline
All API accessing methods allow users to provide an AccessToken to be used for the duration of that API call. The problem with that is that the command is also logged by default, which means that the AccessToken value might be logged in plain text to the log file. To fix this, `Write-InvocationLog` has been modified in a few ways: * Can now redact the value of specified parameters, or exclude the parameter altogether. * `AccessToken` has been configured to _always_ be redacted, and `NoStatus` has been configured to _always_ be excluded (to avoid noise) * Instead of logging the originally invoked line, as well as the individual values of the parameters, this now logs a single line with a modified version of the invocation with the substitution of parameter values performed in-place. * The `DisableParameterLogging` configuation value has been removed, as we're no longer taking up additional verbose space (we're always logging a single line), and we have to process the parameters anyway to ensure that we're excluding/redacting the necessary parameters, meaning that we can't log the invoked line no matter what.
1 parent b614f4a commit c6835f4

File tree

3 files changed

+55
-30
lines changed

3 files changed

+55
-30
lines changed

GitHubConfiguration.ps1

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,6 @@ function Set-GitHubConfiguration
9595
Specify this switch to disable the hashing of potential PII data prior to submitting the
9696
data to telemetry (if telemetry hasn't been disabled via DisableTelemetry).
9797
98-
.PARAMETER DisableParameterLogging
99-
Specify this switch to stop the module from logging every individual parameter value
100-
when it logs a function call.
101-
10298
.PARAMETER DisableSmarterObjects
10399
By deffault, this module will modify all objects returned by the API calls to update
104100
any properties that can be converted to objects (like strings for Date/Time's being
@@ -176,8 +172,6 @@ function Set-GitHubConfiguration
176172

177173
[switch] $DisablePiiProtection,
178174

179-
[switch] $DisableParameterLogging,
180-
181175
[switch] $DisableSmarterObjects,
182176

183177
[switch] $DisableTelemetry,
@@ -263,7 +257,6 @@ function Get-GitHubConfiguration
263257
'DefaultOwnerName',
264258
'DefaultRepositoryName',
265259
'DisableLogging',
266-
'DisableParameterLogging',
267260
'DisablePiiProtection',
268261
'DisableSmarterObjects',
269262
'DisableTelemetry',
@@ -593,7 +586,6 @@ function Import-GitHubConfiguration
593586
'applicationInsightsKey' = '66d83c52-3070-489b-886b-09860e05e78a'
594587
'assemblyPath' = [String]::Empty
595588
'disableLogging' = ([String]::IsNullOrEmpty($logPath))
596-
'disableParameterLogging' = $false
597589
'disablePiiProtection' = $false
598590
'disableSmarterObjects' = $false
599591
'disableTelemetry' = $false

Helpers.ps1

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,14 @@ function Write-Log
392392
}
393393
}
394394

395+
$script:alwaysRedactParametersForLogging = @(
396+
'AccessToken' # Would be a security issue
397+
)
398+
399+
$script:alwaysExcludeParametersForLogging = @(
400+
'NoStatus'
401+
)
402+
395403
function Write-InvocationLog
396404
{
397405
<#
@@ -401,43 +409,68 @@ function Write-InvocationLog
401409
.DESCRIPTION
402410
Writes a log entry for the invoke command.
403411
404-
To control whether or not this will also write out the details of every parameter,
405-
users can call 'Set-GitHubConfiguration -DisableParameterLogging:<$true|$false>'
406-
407412
The Git repo for this module can be found here: http://aka.ms/PowerShellForGitHub
408413
409414
.PARAMETER InvocationInfo
410415
The '$MyInvocationInfo' object from the calling function.
411416
417+
.PARAMETER RedactParameter
418+
An optional array of parameter names that should be logged, but their values redacted.
419+
420+
.PARAMETER ExcludeParameter
421+
An optional array of parameter names that should simply not be logged.
422+
412423
.EXAMPLE
413424
Write-InvocationLog -Invocation $MyInvocation
425+
426+
.EXAMPLE
427+
Write-InvocationLog -Invocation $MyInvocation -ExcludeParameter @('Properties', 'Metrics')
428+
429+
.NOTES
430+
The actual invocation line will not be _completely_ accurate as converted parameters will
431+
be in JSON format as opposed to PowerShell format. However, it should be sufficient enough
432+
for debugging purposes.
433+
434+
ExcludeParamater will always take precedence over RedactParameter.
414435
#>
415436
[CmdletBinding(SupportsShouldProcess)]
416437
param(
417-
[Management.Automation.InvocationInfo] $Invocation
438+
[Management.Automation.InvocationInfo] $Invocation,
439+
440+
[string[]] $RedactParameter,
441+
442+
[string[]] $ExcludeParameter
418443
)
419444

420-
Write-Log -Message "[$($Invocation.MyCommand.Module.Version)] Executing: $($Invocation.Line.Trim())" -Level Verbose
445+
$jsonConversionDepth = 20 # Seems like it should be more than sufficient
421446

422-
if (-not (Get-GitHubConfiguration -Name DisableParameterLogging))
447+
# Build up the invoked line, being sure to exclude and/or redact any values necessary
448+
$params = @()
449+
foreach ($param in $Invocation.BoundParameters.GetEnumerator())
423450
{
424-
# Get the length of all invoked parameter names so that the output can all be aligned.
425-
$maxLength = 0
426-
$Invocation.BoundParameters.Keys |
427-
ForEach-Object { $maxLength = [Math]::Max($maxLength, $_.Length + 1) }
428-
429-
$Invocation.BoundParameters.GetEnumerator() |
430-
Where-Object { ($null -ne $_) -and ($null -ne $_.Value) } |
431-
ForEach-Object {
432-
$value = $_.Value
433-
if ($null -eq $value)
434-
{
435-
$value = '$null'
436-
}
451+
if ($param.Key -in ($script:alwaysExcludeParametersForLogging + $ExcludeParameter))
452+
{
453+
continue
454+
}
437455

438-
Write-Log -Message ("{0, -$maxLength} {1}" -F ($_.Key + ":"), $value.ToString()) -Level Verbose
456+
if ($param.Key -in ($script:alwaysRedactParametersForLogging + $RedactParameter))
457+
{
458+
$params += "-$($param.Key) <redacted>"
459+
}
460+
else
461+
{
462+
if ($param.Value -is [switch])
463+
{
464+
$params += "-$($param.Key):`$$($param.Value.ToBool())"
465+
}
466+
else
467+
{
468+
$params += "-$($param.Key) $($param.Value | ConvertTo-Json -Depth $jsonConversionDepth -Compress)"
439469
}
470+
}
440471
}
472+
473+
Write-Log -Message "[$($Invocation.MyCommand.Module.Version)] Executing: $($Invocation.MyCommand) $($params -join ' ')" -Level Verbose
441474
}
442475

443476
function DeepCopy-Object

Telemetry.ps1

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ function Set-TelemetryEvent
396396
return
397397
}
398398

399-
Write-InvocationLog -Invocation $MyInvocation
399+
Write-InvocationLog -Invocation $MyInvocation -ExcludeParameter @('Properties', 'Metrics')
400400

401401
try
402402
{
@@ -498,7 +498,7 @@ function Set-TelemetryException
498498
return
499499
}
500500

501-
Write-InvocationLog -Invocation $MyInvocation
501+
Write-InvocationLog -Invocation $MyInvocation -ExcludeParameter @('Exception', 'Properties', 'NoFlush')
502502

503503
try
504504
{

0 commit comments

Comments
 (0)