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

Secret Scanning Alerts migration - update to new location types #1306

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion RELEASENOTES.md
Original file line number Diff line number Diff line change
@@ -1 +1 @@
- Update validation error messages for `gh bbs2gh migrate-repo` command when generating an archive is not required.
- Updated Secret Scanning Alerts migration (`gh gei migrate-secret-alerts`) command to match on all location types. Now includes: issues, pull requests, issues.
13 changes: 13 additions & 0 deletions src/Octoshift/Models/GithubSecretScanningAlert.cs
Original file line number Diff line number Diff line change
@@ -4,16 +4,29 @@ public class GithubSecretScanningAlert
public int Number { get; set; }
public string State { get; set; }
public string Resolution { get; set; }
public string ResolutionComment { get; set; }
public string SecretType { get; set; }
public string Secret { get; set; }
}

public class GithubSecretScanningAlertLocation
{
public string LocationType { get; set; }
public string Path { get; set; }
public int StartLine { get; set; }
public int EndLine { get; set; }
public int StartColumn { get; set; }
public int EndColumn { get; set; }
public string BlobSha { get; set; }
public string IssueTitleUrl { get; set; }
public string IssueBodyUrl { get; set; }
public string IssueCommentUrl { get; set; }
public string DiscussionTitleUrl { get; set; }
public string DiscussionBodyUrl { get; set; }
public string DiscussionCommentUrl { get; set; }
public string PullRequestTitleUrl { get; set; }
public string PullRequestBodyUrl { get; set; }
public string PullRequestCommentUrl { get; set; }
public string PullRequestReviewUrl { get; set; }
public string PullRequestReviewCommentUrl { get; set; }
}
17 changes: 15 additions & 2 deletions src/Octoshift/Services/GithubApi.cs
Original file line number Diff line number Diff line change
@@ -908,7 +908,7 @@ public virtual async Task<IEnumerable<GithubSecretScanningAlertLocation>> GetSec
.ToListAsync();
}

public virtual async Task UpdateSecretScanningAlert(string org, string repo, int alertNumber, string state, string resolution = null)
public virtual async Task UpdateSecretScanningAlert(string org, string repo, int alertNumber, string state, string resolution = null, string resolutionComment = null)
{
if (!SecretScanningAlert.IsOpenOrResolved(state))
{
@@ -922,7 +922,7 @@ public virtual async Task UpdateSecretScanningAlert(string org, string repo, int

var url = $"{_apiUrl}/repos/{org.EscapeDataString()}/{repo.EscapeDataString()}/secret-scanning/alerts/{alertNumber}";

object payload = state == SecretScanningAlert.AlertStateOpen ? new { state } : new { state, resolution };
object payload = state == SecretScanningAlert.AlertStateOpen ? new { state } : new { state, resolution, resolution_comment = resolutionComment };
await _client.PatchAsync(url, payload);
}

@@ -1179,19 +1179,32 @@ private static GithubSecretScanningAlert BuildSecretScanningAlert(JToken secretA
Number = (int)secretAlert["number"],
State = (string)secretAlert["state"],
Resolution = (string)secretAlert["resolution"],
ResolutionComment = (string)secretAlert["resolution_comment"],
SecretType = (string)secretAlert["secret_type"],
Secret = (string)secretAlert["secret"],
};

private static GithubSecretScanningAlertLocation BuildSecretScanningAlertLocation(JToken alertLocation) =>
new()
{
LocationType = (string)alertLocation["type"],
Path = (string)alertLocation["details"]["path"],
StartLine = (int)alertLocation["details"]["start_line"],
EndLine = (int)alertLocation["details"]["end_line"],
StartColumn = (int)alertLocation["details"]["start_column"],
EndColumn = (int)alertLocation["details"]["end_column"],
BlobSha = (string)alertLocation["details"]["blob_sha"],
IssueTitleUrl = (string)alertLocation["details"]["issue_title_url"],
IssueBodyUrl = (string)alertLocation["details"]["issue_body_url"],
IssueCommentUrl = (string)alertLocation["details"]["issue_comment_url"],
DiscussionTitleUrl = (string)alertLocation["details"]["discussion_title_url"],
DiscussionBodyUrl = (string)alertLocation["details"]["discussion_body_url"],
DiscussionCommentUrl = (string)alertLocation["details"]["discussion_comment_url"],
PullRequestTitleUrl = (string)alertLocation["details"]["pull_request_title_url"],
PullRequestBodyUrl = (string)alertLocation["details"]["pull_request_body_url"],
PullRequestCommentUrl = (string)alertLocation["details"]["pull_request_comment_url"],
PullRequestReviewUrl = (string)alertLocation["details"]["pull_request_review_url"],
PullRequestReviewCommentUrl = (string)alertLocation["details"]["pull_request_review_comment_url"],
};

private static CodeScanningAnalysis BuildCodeScanningAnalysis(JToken codescan) =>
186 changes: 98 additions & 88 deletions src/Octoshift/Services/SecretScanningAlertService.cs
Original file line number Diff line number Diff line change
@@ -18,130 +18,140 @@ public SecretScanningAlertService(GithubApi sourceGithubApi, GithubApi targetGit
_log = logger;
}

// Iterate over all source alerts by looping through the dictionary with each key (SecretType, Secret) and
// try to find a matching alert in the target repository based on the same key
// If potential match is found we compare the locations of the alerts and if they match a matching AlertWithLocations is returned
public virtual async Task MigrateSecretScanningAlerts(string sourceOrg, string sourceRepo, string targetOrg,
string targetRepo, bool dryRun)
string targetRepo, bool dryRun)
{
_log.LogInformation(
$"Migrating Secret Scanning Alerts from '{sourceOrg}/{sourceRepo}' to '{targetOrg}/{targetRepo}'");
_log.LogInformation($"Migrating Secret Scanning Alerts from '{sourceOrg}/{sourceRepo}' to '{targetOrg}/{targetRepo}'");

var sourceAlerts = await GetAlertsWithLocations(_sourceGithubApi, sourceOrg, sourceRepo);
var targetAlerts = await GetAlertsWithLocations(_targetGithubApi, targetOrg, targetRepo);
var sourceAlertsDict = await GetAlertsWithLocations(_sourceGithubApi, sourceOrg, sourceRepo);
var targetAlertsDict = await GetAlertsWithLocations(_targetGithubApi, targetOrg, targetRepo);

_log.LogInformation($"Source {sourceOrg}/{sourceRepo} secret alerts found: {sourceAlerts.Count}");
_log.LogInformation($"Target {targetOrg}/{targetRepo} secret alerts found: {targetAlerts.Count}");
_log.LogInformation($"Source {sourceOrg}/{sourceRepo} secret alerts found: {sourceAlertsDict.Count}");
_log.LogInformation($"Target {targetOrg}/{targetRepo} secret alerts found: {targetAlertsDict.Count}");

_log.LogInformation("Matching secret resolutions from source to target repository");
foreach (var alert in sourceAlerts)

foreach (var kvp in sourceAlertsDict)
{
_log.LogInformation($"Processing source secret {alert.Alert.Number}");
var sourceKey = kvp.Key;
var sourceAlerts = kvp.Value;

if (SecretScanningAlert.IsOpen(alert.Alert.State))
foreach (var sourceAlert in sourceAlerts)
{
_log.LogInformation(" secret alert is still open, nothing to do");
continue;
}
_log.LogInformation($"Processing source secret {sourceAlert.Alert.Number}");

_log.LogInformation(" secret is resolved, looking for matching secret in target...");
var target = MatchTargetSecret(alert, targetAlerts);

if (target == null)
{
_log.LogWarning(
$" failed to locate a matching secret to source secret {alert.Alert.Number} in {targetOrg}/{targetRepo}");
continue;
}
if (SecretScanningAlert.IsOpen(sourceAlert.Alert.State))
{
_log.LogInformation(" secret alert is still open, nothing to do");
continue;
}

_log.LogInformation(
$" source secret alert matched alert to {target.Alert.Number} in {targetOrg}/{targetRepo}.");
_log.LogInformation(" secret is resolved, looking for matching secret in target...");

if (alert.Alert.Resolution == target.Alert.Resolution && alert.Alert.State == target.Alert.State)
{
_log.LogInformation(" source and target alerts are already aligned.");
continue;
}

if (dryRun)
{
_log.LogInformation(
$" executing in dry run mode! Target alert {target.Alert.Number} would have been updated to state:{alert.Alert.State} and resolution:{alert.Alert.Resolution}");
continue;
}
if (targetAlertsDict.TryGetValue(sourceKey, out var potentialTargets))
{
var targetAlert = potentialTargets.FirstOrDefault(target => DoAllLocationsMatch(sourceAlert.Locations, target.Locations));

_log.LogInformation(
$" updating target alert:{target.Alert.Number} to state:{alert.Alert.State} and resolution:{alert.Alert.Resolution}");
if (targetAlert != null)
{
_log.LogInformation($" source secret alert matched to {targetAlert.Alert.Number} in {targetOrg}/{targetRepo}.");

await _targetGithubApi.UpdateSecretScanningAlert(targetOrg, targetRepo, target.Alert.Number,
alert.Alert.State, alert.Alert.Resolution);
_log.LogSuccess(
$" target alert successfully updated to {alert.Alert.Resolution}.");
}
}
if (sourceAlert.Alert.Resolution == targetAlert.Alert.Resolution && sourceAlert.Alert.State == targetAlert.Alert.State)
{
_log.LogInformation(" source and target alerts are already aligned.");
continue;
}

private AlertWithLocations MatchTargetSecret(AlertWithLocations source, List<AlertWithLocations> targets)
{
AlertWithLocations matched = null;
if (dryRun)
{
_log.LogInformation($" executing in dry run mode! Target alert {targetAlert.Alert.Number} would have been updated to state:{sourceAlert.Alert.State} and resolution:{sourceAlert.Alert.Resolution}");
continue;
}

foreach (var target in targets)
{
if (matched != null)
{
break;
}
_log.LogInformation($" updating target alert:{targetAlert.Alert.Number} to state:{sourceAlert.Alert.State} and resolution:{sourceAlert.Alert.Resolution}");

if (source.Alert.SecretType == target.Alert.SecretType
&& source.Alert.Secret == target.Alert.Secret)
{
_log.LogVerbose(
$"Secret type and value match between source:{source.Alert.Number} and target:{source.Alert.Number}");
var locationMatch = true;
foreach (var sourceLocation in source.Locations)
{
locationMatch = IsMatchedSecretAlertLocation(sourceLocation, target.Locations);
if (!locationMatch)
await _targetGithubApi.UpdateSecretScanningAlert(targetOrg, targetRepo, targetAlert.Alert.Number, sourceAlert.Alert.State,
sourceAlert.Alert.Resolution, sourceAlert.Alert.ResolutionComment);
_log.LogSuccess($" target alert successfully updated to {sourceAlert.Alert.Resolution}.");
}
else
{
break;
_log.LogWarning($" failed to locate a matching secret to source secret {sourceAlert.Alert.Number} in {targetOrg}/{targetRepo}");
}
}

if (locationMatch)
else
{
matched = target;
_log.LogWarning($" Failed to locate a matching secret to source secret {sourceAlert.Alert.Number} in {targetOrg}/{targetRepo}");
}
}
}
}

return matched;
[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0075: Conditional expression can be simplified", Justification = "Want to keep guard for better performance.")]
private bool DoAllLocationsMatch(GithubSecretScanningAlertLocation[] sourceLocations, GithubSecretScanningAlertLocation[] targetLocations)
{
// Preflight check: Compare the number of locations;
// If the number of locations don't match we can skip the detailed comparison as the alerts can't be considered equal
return sourceLocations.Length != targetLocations.Length
? false
: sourceLocations.All(sourceLocation => IsLocationMatched(sourceLocation, targetLocations));
}

private bool IsMatchedSecretAlertLocation(GithubSecretScanningAlertLocation sourceLocation,
GithubSecretScanningAlertLocation[] targetLocations)
private bool IsLocationMatched(GithubSecretScanningAlertLocation sourceLocation, GithubSecretScanningAlertLocation[] targetLocations)
{
// We cannot guarantee the ordering of things with the locations and the APIs, typically they would match, but cannot be sure
// so we need to iterate over all the targets to ensure a match
return targetLocations.Any(
target => sourceLocation.Path == target.Path
&& sourceLocation.StartLine == target.StartLine
&& sourceLocation.EndLine == target.EndLine
&& sourceLocation.StartColumn == target.StartColumn
&& sourceLocation.EndColumn == target.EndColumn
&& sourceLocation.BlobSha == target.BlobSha
// Technically this wil hold, but only if there is not commit rewriting going on, so we need to make this last one optional for now
// && sourceDetails.CommitSha == target.Details.CommitSha)
);
return targetLocations.Any(targetLocation => AreLocationsEqual(sourceLocation, targetLocation));
}

// Check if the locations of the source and target alerts match exactly
// We compare the type of location and the corresponding fields based on the type
// Each type has different fields that need to be compared for equality so we use a switch statement
// Note: Discussions are commented out as we don't miggate them currently
[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0075: Conditional expression can be simplified", Justification = "Want to keep guard for better performance.")]
private bool AreLocationsEqual(GithubSecretScanningAlertLocation sourceLocation, GithubSecretScanningAlertLocation targetLocation)
{
return sourceLocation.LocationType != targetLocation.LocationType
? false
: sourceLocation.LocationType switch
{
"commit" or "wiki_commit" => sourceLocation.Path == targetLocation.Path &&
sourceLocation.StartLine == targetLocation.StartLine &&
sourceLocation.EndLine == targetLocation.EndLine &&
sourceLocation.StartColumn == targetLocation.StartColumn &&
sourceLocation.EndColumn == targetLocation.EndColumn &&
sourceLocation.BlobSha == targetLocation.BlobSha,
"issue_title" => sourceLocation.IssueTitleUrl == targetLocation.IssueTitleUrl,
"issue_body" => sourceLocation.IssueBodyUrl == targetLocation.IssueBodyUrl,
"issue_comment" => sourceLocation.IssueCommentUrl == targetLocation.IssueCommentUrl,
"pull_request_title" => sourceLocation.PullRequestTitleUrl == targetLocation.PullRequestTitleUrl,
"pull_request_body" => sourceLocation.PullRequestBodyUrl == targetLocation.PullRequestBodyUrl,
"pull_request_comment" => sourceLocation.PullRequestCommentUrl == targetLocation.PullRequestCommentUrl,
"pull_request_review" => sourceLocation.PullRequestReviewUrl == targetLocation.PullRequestReviewUrl,
"pull_request_review_comment" => sourceLocation.PullRequestReviewCommentUrl == targetLocation.PullRequestReviewCommentUrl,
_ => false
};
}

private async Task<List<AlertWithLocations>> GetAlertsWithLocations(GithubApi api, string org, string repo)
// Getting alerts with locations from a repository and building a dictionary with a key (SecretType, Secret)
// and value List of AlertWithLocations
// This method is used to get alerts from both source and target repositories
private async Task<Dictionary<(string SecretType, string Secret), List<AlertWithLocations>>>
GetAlertsWithLocations(GithubApi api, string org, string repo)
{
var alerts = await api.GetSecretScanningAlertsForRepository(org, repo);
var results = new List<AlertWithLocations>();
var alertsWithLocations = new List<AlertWithLocations>();
foreach (var alert in alerts)
{
var locations =
await api.GetSecretScanningAlertsLocations(org, repo, alert.Number);
results.Add(new AlertWithLocations { Alert = alert, Locations = locations.ToArray() });
var locations = await api.GetSecretScanningAlertsLocations(org, repo, alert.Number);
alertsWithLocations.Add(new AlertWithLocations { Alert = alert, Locations = locations.ToArray() });
}

return results;
// Build the dictionary keyed by SecretType and Secret
return alertsWithLocations
.GroupBy(alert => (alert.Alert.SecretType, alert.Alert.Secret))
.ToDictionary(group => group.Key, group => group.ToList());
}
}

6 changes: 4 additions & 2 deletions src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs
Original file line number Diff line number Diff line change
@@ -2863,16 +2863,18 @@ public async Task UpdateSecretScanningAlert_Calls_The_Right_Endpoint_With_Payloa
const int alertNumber = 100;
const string alertState = "resolved";
const string alertResolution = "wont_fix";
const string alertResolutionComment = "This is a false positive";

var url = $"https://api.github.com/repos/{GITHUB_ORG}/{GITHUB_REPO}/secret-scanning/alerts/{alertNumber}";
var payload = new
{
state = alertState,
resolution = alertResolution
resolution = alertResolution,
resolution_comment = alertResolutionComment
};

// Act
await _githubApi.UpdateSecretScanningAlert(GITHUB_ORG, GITHUB_REPO, alertNumber, alertState, alertResolution);
await _githubApi.UpdateSecretScanningAlert(GITHUB_ORG, GITHUB_REPO, alertNumber, alertState, alertResolution, alertResolutionComment);

// Assert
_githubClientMock.Verify(m => m.PatchAsync(url, It.Is<object>(x => x.ToJson() == payload.ToJson()), null));
Loading
Loading