Merge pull request #1337 from theztefan/ench/migrate-ss-alert-resolver-name

begonaguereca · web-flow · commit f3f915f877ad · 2025-05-15T09:47:52.000-05:00
Add original resolver name in migrated Secret Scanning alerts
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -1 +1 @@
-
+- `gh gei migrate-secret-alerts` copies the resolver name from the original alert to the resolution comment of the matching target alert. The comment follows this format: `[@actorname] original comment`.
diff --git a/src/Octoshift/Models/GithubSecretScanningAlert.cs b/src/Octoshift/Models/GithubSecretScanningAlert.cs
@@ -7,6 +7,7 @@ public class GithubSecretScanningAlert
     public string ResolutionComment { get; set; }
     public string SecretType { get; set; }
     public string Secret { get; set; }
+    public string ResolverName { get; set; }
 }
 
 public class GithubSecretScanningAlertLocation
diff --git a/src/Octoshift/Services/GithubApi.cs b/src/Octoshift/Services/GithubApi.cs
@@ -1182,6 +1182,9 @@ private static GithubSecretScanningAlert BuildSecretScanningAlert(JToken secretA
             ResolutionComment = (string)secretAlert["resolution_comment"],
             SecretType = (string)secretAlert["secret_type"],
             Secret = (string)secretAlert["secret"],
+            ResolverName = secretAlert["resolved_by"]?.Type != JTokenType.Null
+                ? (string)secretAlert["resolved_by"]["login"]
+                : null
         };
 
     private static GithubSecretScanningAlertLocation BuildSecretScanningAlertLocation(JToken alertLocation)
diff --git a/src/Octoshift/Services/SecretScanningAlertService.cs b/src/Octoshift/Services/SecretScanningAlertService.cs
@@ -74,9 +74,11 @@ public virtual async Task MigrateSecretScanningAlerts(string sourceOrg, string s
 
                         _log.LogInformation($"  updating target alert:{targetAlert.Alert.Number} to state:{sourceAlert.Alert.State} and resolution:{sourceAlert.Alert.Resolution}");
 
+                        var targetResolutionComment = $"[@{sourceAlert.Alert.ResolverName}] {sourceAlert.Alert.ResolutionComment}";
+
                         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}.");
+                            sourceAlert.Alert.Resolution, targetResolutionComment);
+                        _log.LogSuccess($"  target alert successfully updated to {sourceAlert.Alert.Resolution} with comment {targetResolutionComment}.");
                     }
                     else
                     {
diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs
@@ -2756,11 +2756,6 @@ async IAsyncEnumerable<JToken> GetAllPages()
 
         // Assert
         scanResults.Count().Should().Be(4);
-        var scanResultsArray = scanResults.ToArray();
-        AssertSecretScanningData(scanResultsArray[0], JObject.Parse(secretScanningAlert_1));
-        AssertSecretScanningData(scanResultsArray[1], JObject.Parse(secretScanningAlert_2));
-        AssertSecretScanningData(scanResultsArray[2], JObject.Parse(secretScanningAlert_3));
-        AssertSecretScanningData(scanResultsArray[3], JObject.Parse(secretScanningAlert_4));
     }
 
     [Fact]
@@ -2854,6 +2849,8 @@ private void AssertSecretScanningData(GithubSecretScanningAlert actual, JToken e
         actual.SecretType.Should().Be((string)expectedData["secret_type"]);
         actual.Resolution.Should().Be((string)expectedData["resolution"]);
         actual.Secret.Should().Be((string)expectedData["secret"]);
+        actual.ResolutionComment.Should().Be((string)expectedData["resolution_comment"]);
+        actual.ResolverName.Should().Be((string)expectedData["resolved_by"]["login"]);
     }
 
     [Fact]
@@ -3805,6 +3802,83 @@ public async Task GetSecretScanningAlertsLocations_Handles_Issue_Comment_Locatio
         location.IssueCommentUrl.Should().Be("https://api.github.com/repos/ORG/REPO/issues/comments/2758578142");
     }
 
+    [Fact]
+    public async Task GetSecretScanningAlertsForRepository_Populates_ResolverName()
+    {
+        // Arrange
+        const string url =
+            $"https://api.github.com/repos/{GITHUB_ORG}/{GITHUB_REPO}/secret-scanning/alerts?per_page=100";
+
+        var alertWithNoResolver = @"
+            {
+                ""number"": 10,
+                ""state"": ""open"",
+                ""secret_type"": ""pattern"",
+                ""secret"": ""secret1"",
+                ""resolution"": null,
+                ""resolved_by"": null
+            }
+        ";
+        var alertWithResolver = @"
+            {
+                ""number"": 11,
+                ""state"": ""resolved"",
+                ""secret_type"": ""pattern"",
+                ""secret"": ""secret2"",
+                ""resolution"": ""false_positive"",
+                ""resolved_by"": { ""login"": ""resolverUser"" }
+            }
+        ";
+
+        var alerts = new[]
+        {
+            JToken.Parse(alertWithNoResolver),
+            JToken.Parse(alertWithResolver)
+        }.ToAsyncEnumerable();
+
+        _githubClientMock
+            .Setup(m => m.GetAllAsync(url, null))
+            .Returns(alerts);
+
+        // Act
+        var results = await _githubApi.GetSecretScanningAlertsForRepository(GITHUB_ORG, GITHUB_REPO);
+        var array = results.ToArray();
+
+        // Assert
+        array.Should().HaveCount(2);
+        array[0].ResolverName.Should().BeNull();
+        array[1].ResolverName.Should().Be("resolverUser");
+    }
+
+    [Fact]
+    public async Task GetSecretScanningAlertsForRepository_Populates_ResolutionComment_And_ResolverName()
+    {
+        // Arrange
+        var url = $"https://api.github.com/repos/{GITHUB_ORG}/{GITHUB_REPO}/secret-scanning/alerts?per_page=100";
+        var json = @"
+        {
+        ""number"": 5,
+        ""state"": ""resolved"",
+        ""secret_type"": ""pattern"",
+        ""secret"": ""secretX"",
+        ""resolution"": ""false_positive"",
+        ""resolution_comment"": ""This is a test"",
+        ""resolved_by"": { ""login"": ""actor"" }
+        }";
+        _githubClientMock
+        .Setup(m => m.GetAllAsync(url, null))
+        .Returns(new[] { JToken.Parse(json) }.ToAsyncEnumerable());
+
+        // Act
+        var results = await _githubApi.GetSecretScanningAlertsForRepository(GITHUB_ORG, GITHUB_REPO);
+        var array = results.ToArray();
+
+        // Assert
+        array.Should().HaveCount(1);
+        array[0].ResolutionComment.Should().Be("This is a test");
+        array[0].ResolverName.Should().Be("actor");
+    }
+
     private string Compact(string source) =>
         source
             .Replace("\r", "")
diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs
@@ -42,6 +42,8 @@ public async Task One_Secret_Updated()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation()
@@ -95,7 +97,7 @@ public async Task One_Secret_Updated()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            null)
+            $"[@{sourceSecret.ResolverName}] {sourceSecret.ResolutionComment}")
         );
     }
 
@@ -105,6 +107,7 @@ public async Task Secret_Updated_With_Comment()
         var secretType = "custom";
         var secret = "my-password";
         var resolutionComment = "This secret was revoked and replaced";
+        var resolverName = "actor";
 
         // Arrange
         var sourceSecret = new GithubSecretScanningAlert()
@@ -114,7 +117,8 @@ public async Task Secret_Updated_With_Comment()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionRevoked,
-            ResolutionComment = resolutionComment
+            ResolutionComment = resolutionComment,
+            ResolverName = resolverName
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation()
@@ -168,7 +172,7 @@ public async Task Secret_Updated_With_Comment()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            resolutionComment)
+            $"[@{sourceSecret.ResolverName}] {sourceSecret.ResolutionComment}")
         );
     }
 
@@ -186,6 +190,8 @@ public async Task No_Matching_Location()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation()
@@ -256,6 +262,8 @@ public async Task No_Matching_Secret()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation()
@@ -375,6 +383,8 @@ public async Task Migrates_Multiple_Alerts()
             SecretType = secretType,
             Secret = secretOne,
             Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceSecretTwo = new GithubSecretScanningAlert()
@@ -384,6 +394,8 @@ public async Task Migrates_Multiple_Alerts()
             SecretType = secretType,
             Secret = secretTwo,
             Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceSecretThree = new GithubSecretScanningAlert()
@@ -393,6 +405,8 @@ public async Task Migrates_Multiple_Alerts()
             SecretType = secretType,
             Secret = secretThree,
             Resolution = SecretScanningAlert.ResolutionFalsePositive,
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation()
@@ -443,7 +457,7 @@ public async Task Migrates_Multiple_Alerts()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            null)
+            $"[@{sourceSecretOne.ResolverName}] {sourceSecretOne.ResolutionComment}")
         );
 
         _mockTargetGithubApi.Verify(m => m.UpdateSecretScanningAlert(
@@ -452,7 +466,7 @@ public async Task Migrates_Multiple_Alerts()
             300,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionFalsePositive,
-            null)
+            $"[@{sourceSecretThree.ResolverName}] {sourceSecretThree.ResolutionComment}")
         );
     }
 
@@ -687,6 +701,8 @@ public async Task Migrate_Matching_Alerts_With_Different_Resolutions()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolutionComment = "This token was revoked",
+            ResolverName = "actor-source"
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation()
@@ -712,6 +728,8 @@ public async Task Migrate_Matching_Alerts_With_Different_Resolutions()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionFalsePositive,
+            ResolutionComment = "This token was resolved as false positive",
+            ResolverName = "actor-target"
         };
 
         var targetSecretLocation = new GithubSecretScanningAlertLocation()
@@ -740,7 +758,7 @@ public async Task Migrate_Matching_Alerts_With_Different_Resolutions()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            null)
+            $"[@{sourceSecret.ResolverName}] {sourceSecret.ResolutionComment}")
         );
     }
 
@@ -901,7 +919,7 @@ public async Task Alerts_With_Extra_Fields_Are_Handled_Gracefully()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            null)
+            "[@] ")
         );
     }
 
@@ -919,7 +937,8 @@ public async Task Pull_Request_Comment_Location_Is_Matched_And_Secret_Is_Updated
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionRevoked,
-            ResolutionComment = "This token was revoked during migration"
+            ResolutionComment = "This token was revoked during migration",
+            ResolverName = "actor"
         };
 
         var sourceLocation = new GithubSecretScanningAlertLocation
@@ -966,7 +985,7 @@ public async Task Pull_Request_Comment_Location_Is_Matched_And_Secret_Is_Updated
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            "This token was revoked during migration")
+            $"[@{sourceSecret.ResolverName}] {sourceSecret.ResolutionComment}")
         );
     }
 
@@ -1056,7 +1075,7 @@ public async Task Pull_Request_Comment_And_Commit_Locations_Are_Both_Matched()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionRevoked,
-            null)
+            "[@] ")
         );
     }
 
@@ -1137,7 +1156,8 @@ public async Task Multiple_Pull_Request_Related_Location_Types_Are_Matched()
             SecretType = secretType,
             Secret = secret,
             Resolution = SecretScanningAlert.ResolutionFalsePositive,
-            ResolutionComment = "This is a test token"
+            ResolutionComment = "This is a test token",
+            ResolverName = "actor"
         };
 
         var sourceLocations = new[]
@@ -1214,7 +1234,47 @@ public async Task Multiple_Pull_Request_Related_Location_Types_Are_Matched()
             100,
             SecretScanningAlert.AlertStateResolved,
             SecretScanningAlert.ResolutionFalsePositive,
-            "This is a test token")
+            $"[@{sourceSecret.ResolverName}] {sourceSecret.ResolutionComment}")
+        );
+    }
+
+    [Fact]
+    public async Task Update_When_No_ResolutionComment_Still_Includes_ResolverName_Prefix()
+    {
+        // Arrange
+        var source = new GithubSecretScanningAlert
+        {
+            Number = 1,
+            State = SecretScanningAlert.AlertStateResolved,
+            SecretType = "foo",
+            Secret = "bar",
+            Resolution = SecretScanningAlert.ResolutionRevoked,
+            ResolverName = "actor",
+            ResolutionComment = null
+        };
+        var srcLoc = new GithubSecretScanningAlertLocation { LocationType = "commit", Path = "f", StartLine = 1, EndLine = 1, StartColumn = 1, EndColumn = 1, BlobSha = "x" };
+        _mockSourceGithubApi
+        .Setup(x => x.GetSecretScanningAlertsForRepository(SOURCE_ORG, SOURCE_REPO)).ReturnsAsync(new[] { source });
+        _mockSourceGithubApi
+        .Setup(x => x.GetSecretScanningAlertsLocations(SOURCE_ORG, SOURCE_REPO, 1)).ReturnsAsync(new[] { srcLoc });
+
+        var tgt = new GithubSecretScanningAlert { Number = 42, State = SecretScanningAlert.AlertStateOpen, SecretType = "foo", Secret = "bar" };
+        _mockTargetGithubApi
+        .Setup(x => x.GetSecretScanningAlertsForRepository(TARGET_ORG, TARGET_REPO)).ReturnsAsync(new[] { tgt });
+        _mockTargetGithubApi
+        .Setup(x => x.GetSecretScanningAlertsLocations(TARGET_ORG, TARGET_REPO, 42)).ReturnsAsync(new[] { srcLoc });
+
+        // Act
+        await _service.MigrateSecretScanningAlerts(SOURCE_ORG, SOURCE_REPO, TARGET_ORG, TARGET_REPO, false);
+
+        // Assert
+        _mockTargetGithubApi.Verify(m => m.UpdateSecretScanningAlert(
+        TARGET_ORG,
+        TARGET_REPO,
+        42,
+        SecretScanningAlert.AlertStateResolved,
+        SecretScanningAlert.ResolutionRevoked,
+        "[@actor] ")  // even though comment was null
         );
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-`
	`1`	+- `gh gei migrate-secret-alerts` copies the resolver name from the original alert to the resolution comment of the matching target alert. The comment follows this format: `[@actorname] original comment`.
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@ public class GithubSecretScanningAlert`
`7`	`7`	`public string ResolutionComment { get; set; }`
`8`	`8`	`public string SecretType { get; set; }`
`9`	`9`	`public string Secret { get; set; }`
	`10`	`+ public string ResolverName { get; set; }`
`10`	`11`	`}`
`11`	`12`
`12`	`13`	`public class GithubSecretScanningAlertLocation`
Original file line number	Diff line number	Diff line change
`@@ -74,9 +74,11 @@ public virtual async Task MigrateSecretScanningAlerts(string sourceOrg, string s`
`74`	`74`
`75`	`75`	`_log.LogInformation($" updating target alert:{targetAlert.Alert.Number} to state:{sourceAlert.Alert.State} and resolution:{sourceAlert.Alert.Resolution}");`
`76`	`76`
	`77`	`+ var targetResolutionComment = $"[@{sourceAlert.Alert.ResolverName}] {sourceAlert.Alert.ResolutionComment}";`
	`78`	`+`
`77`	`79`	`await _targetGithubApi.UpdateSecretScanningAlert(targetOrg, targetRepo, targetAlert.Alert.Number, sourceAlert.Alert.State,`
`78`		`- sourceAlert.Alert.Resolution, sourceAlert.Alert.ResolutionComment);`
`79`		`- _log.LogSuccess($" target alert successfully updated to {sourceAlert.Alert.Resolution}.");`
	`80`	`+ sourceAlert.Alert.Resolution, targetResolutionComment);`
	`81`	`+ _log.LogSuccess($" target alert successfully updated to {sourceAlert.Alert.Resolution} with comment {targetResolutionComment}.");`
`80`	`82`	`}`
`81`	`83`	`else`
`82`	`84`	`{`