Skip to content

Commit

Permalink
Merge pull request #1027 from github/timrogers/sanitiize-logs-urlencoded
Browse files Browse the repository at this point in the history
Improve log secret sanitization to remove secrets that have been URL encoded
  • Loading branch information
timrogers authored Jun 19, 2023
2 parents 3441f64 + 66b0181 commit 89276c7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
3 changes: 2 additions & 1 deletion RELEASENOTES.md
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
- Warn when the `--ssh-port` argument for `bbs2gh migrate-repo` and `bbs2gh generate-script` is set to the default port for Git operations
- Improve log sanitization to also remove secret values that have been URL encoded
- Warn when the `--ssh-port` argument for `bbs2gh migrate-repo` and `bbs2gh generate-script` is set to the default port for Git operations
3 changes: 2 additions & 1 deletion src/Octoshift/Services/OctoLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ private string MaskSecrets(string msg)

foreach (var secret in _secrets.Where(x => x is not null))
{
result = result.Replace(secret, "***");
result = result.Replace(secret, "***")
.Replace(Uri.EscapeDataString(secret), "***");
}

return result;
Expand Down
14 changes: 14 additions & 0 deletions src/OctoshiftCLI.Tests/Octoshift/Services/OctoLoggerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,39 @@ public OctoLoggerTests()
public void Secrets_Should_Be_Masked_From_Logs_And_Console()
{
var secret = "purplemonkeydishwasher";
var urlEncodedSecret = Uri.EscapeDataString(secret);

_octoLogger.RegisterSecret(secret);

_octoLogger.Verbose = false;
_octoLogger.LogInformation($"Don't tell anybody that {secret} is my password");
_octoLogger.LogInformation($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password");
_octoLogger.LogVerbose($"Don't tell anybody that {secret} is my password");
_octoLogger.LogVerbose($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password");
_octoLogger.LogWarning($"Don't tell anybody that {secret} is my password");
_octoLogger.LogWarning($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password");
_octoLogger.LogSuccess($"Don't tell anybody that {secret} is my password");
_octoLogger.LogSuccess($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password");
_octoLogger.LogError($"Don't tell anybody that {secret} is my password");
_octoLogger.LogError($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password");
_octoLogger.LogError(new OctoshiftCliException($"Don't tell anybody that {secret} is my password"));
_octoLogger.LogError(new OctoshiftCliException($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password"));
_octoLogger.LogError(new InvalidOperationException($"Don't tell anybody that {secret} is my password"));
_octoLogger.LogError(new InvalidOperationException($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password"));

_octoLogger.Verbose = true;
_octoLogger.LogVerbose($"Don't tell anybody that {secret} is my password");
_octoLogger.LogVerbose($"Don't tell anyone that {urlEncodedSecret} is my URL encoded password");

_consoleOutput.Should().NotContain(secret);
_logOutput.Should().NotContain(secret);
_verboseLogOutput.Should().NotContain(secret);
_consoleError.Should().NotContain(secret);

_consoleOutput.Should().NotContain(urlEncodedSecret);
_logOutput.Should().NotContain(urlEncodedSecret);
_verboseLogOutput.Should().NotContain(urlEncodedSecret);
_consoleError.Should().NotContain(urlEncodedSecret);
}

[Fact]
Expand Down

0 comments on commit 89276c7

Please sign in to comment.