Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redact ?token= querystring parameter from GHES <3.8 archive URLs in logs #1152

Merged
merged 8 commits into from
Nov 10, 2023

Conversation

timrogers
Copy link
Contributor

@timrogers timrogers commented Oct 24, 2023

Existing code allows us to redact secrets in the logs to stop them being outputted to the shell and to log files. Secret strings are registered during execution, and then replaced in the output.

This PR also allows classes using OctoLogger to register regular expressions to replace, and begins redacting GHES download tokens for GHES <3.8 using a simple regular expression.

Fixes #1151.

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

@@ -156,5 +155,17 @@ public virtual void LogSuccess(string msg)
Console.ResetColor();
}

public virtual void RegisterSecret(string secret) => _secrets.Add(secret);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also avoid this refactor, and keep a separate list of _secrets which we replace as well as the "redaction patterns". That's probably a safer approach to avoid surprises - but I'd love to hear what y'all think.

@github-actions
Copy link

github-actions bot commented Oct 24, 2023

Unit Test Results

785 tests   785 ✔️  21s ⏱️
    1 suites      0 💤
    1 files        0

Results for commit 062d0c2.

♻️ This comment has been updated with latest results.

… logs

Existing code allows us to redact secrets in the logs outputted
to the shell and to log files. Secret strings are registered,
and then replaced in the output.

This PR also allows classes using `OctoLogger` to register
regular expressions to replace, and begins redacting
GHES download tokens for GHES <3.8.

Fixes #1151.
@@ -188,6 +189,8 @@ public override MigrateRepoCommandHandler BuildHandler(MigrateRepoCommandArgs ar
var ghesVersionChecker = ghesVersionCheckerFactory.Create(ghesApi);
var warningsCountLogger = sp.GetRequiredService<WarningsCountLogger>();

log.AddRedactionPattern(new Regex("\\?token=[A-Z0-9]{29}"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have manually tested that this works as expected on GHES 3.7, and the URL is not logged.

@timrogers timrogers marked this pull request as ready for review October 31, 2023 14:32
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
gei 79% 71% 504
bbs2gh 79% 74% 638
ado2gh 84% 80% 607
Octoshift 86% 76% 1224
Summary 83% (6596 / 7961) 75% (1495 / 1984) 2973

@timrogers timrogers merged commit 393f771 into main Nov 10, 2023
@timrogers timrogers deleted the timrogers/fix-1151 branch November 10, 2023 04:16
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.

Redact tokens from logs when migrating from GHES
2 participants