From 2e74554f3635c195184118ab2c47e2fe2ef1ba37 Mon Sep 17 00:00:00 2001 From: David Linares <28559777+dlinares-linux@users.noreply.github.com> Date: Fri, 3 May 2024 21:55:22 +0900 Subject: [PATCH 1/2] Add support for migrating secret scanning resolution comments --- RELEASENOTES.md | 2 +- .../Models/GithubSecretScanningAlert.cs | 1 + src/Octoshift/Services/GithubApi.cs | 12 ++- .../Services/SecretScanningAlertService.cs | 9 +- .../Octoshift/Services/GithubApiTests.cs | 35 +++++++- .../SecretScanningAlertServiceTests.cs | 88 ++++++++++++++++++- 6 files changed, 137 insertions(+), 10 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 8b1378917..06f57f553 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1 +1 @@ - +- Add support for migrating secret scanning alert resolution comments diff --git a/src/Octoshift/Models/GithubSecretScanningAlert.cs b/src/Octoshift/Models/GithubSecretScanningAlert.cs index c442caaed..15dc92597 100644 --- a/src/Octoshift/Models/GithubSecretScanningAlert.cs +++ b/src/Octoshift/Models/GithubSecretScanningAlert.cs @@ -4,6 +4,7 @@ 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; } } diff --git a/src/Octoshift/Services/GithubApi.cs b/src/Octoshift/Services/GithubApi.cs index baca7a80f..7cb161fa3 100644 --- a/src/Octoshift/Services/GithubApi.cs +++ b/src/Octoshift/Services/GithubApi.cs @@ -870,7 +870,7 @@ public virtual async Task> 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)) { @@ -884,7 +884,14 @@ 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 }; + var payload = state == SecretScanningAlert.AlertStateOpen + ? (new { state }) + : (object)(new + { + state, + resolution, + resolution_comment = resolutionComment ?? string.Empty + }); await _client.PatchAsync(url, payload); } @@ -1129,6 +1136,7 @@ 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"], }; diff --git a/src/Octoshift/Services/SecretScanningAlertService.cs b/src/Octoshift/Services/SecretScanningAlertService.cs index 78b89b923..d53c2a4dd 100644 --- a/src/Octoshift/Services/SecretScanningAlertService.cs +++ b/src/Octoshift/Services/SecretScanningAlertService.cs @@ -70,8 +70,13 @@ public virtual async Task MigrateSecretScanningAlerts(string sourceOrg, string s _log.LogInformation( $" updating target alert:{target.Alert.Number} to state:{alert.Alert.State} and resolution:{alert.Alert.Resolution}"); - await _targetGithubApi.UpdateSecretScanningAlert(targetOrg, targetRepo, target.Alert.Number, - alert.Alert.State, alert.Alert.Resolution); + await _targetGithubApi.UpdateSecretScanningAlert( + targetOrg, + targetRepo, + target.Alert.Number, + alert.Alert.State, + alert.Alert.Resolution, + alert.Alert.ResolutionComment); _log.LogSuccess( $" target alert successfully updated to {alert.Alert.Resolution}."); } diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs index 962922ded..7cf5596ee 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs @@ -2507,6 +2507,7 @@ public async Task GetSecretScanningAlertsData() ""resolution"": null, ""resolved_by"": null, ""resolved_at"": null, + ""resolution_comment"": null, ""push_protection_bypassed"": false, ""push_protection_bypassed_by"": null, ""push_protection_bypassed_at"": null @@ -2540,6 +2541,7 @@ public async Task GetSecretScanningAlertsData() ""site_admin"": true }}, ""resolved_at"": ""2022-04-05T20:57:03"", + ""resolution_comment"": null, ""push_protection_bypassed"": false, ""push_protection_bypassed_by"": null, ""push_protection_bypassed_at"": null @@ -2561,6 +2563,7 @@ public async Task GetSecretScanningAlertsData() ""resolution"": null, ""resolved_by"": null, ""resolved_at"": null, + ""resolution_comment"": null, ""push_protection_bypassed"": false, ""push_protection_bypassed_by"": null, ""push_protection_bypassed_at"": null @@ -2601,6 +2604,7 @@ public async Task GetSecretScanningAlertsData() ""site_admin"": true }}, ""resolved_at"": ""2022-08-15T13:53:42Z"", + ""resolution_comment"": null, ""push_protection_bypassed"": false, ""push_protection_bypassed_by"": null, ""push_protection_bypassed_at"": null @@ -2740,6 +2744,7 @@ private void AssertSecretScanningData(GithubSecretScanningAlert actual, JToken e actual.State.Should().Be((string)expectedData["state"]); actual.SecretType.Should().Be((string)expectedData["secret_type"]); actual.Resolution.Should().Be((string)expectedData["resolution"]); + actual.ResolutionComment.Should().Be((string)expectedData["resolution_comment"]); actual.Secret.Should().Be((string)expectedData["secret"]); } @@ -2750,16 +2755,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 = "Risk has been accepted"; 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(x => x.ToJson() == payload.ToJson()), null)); @@ -2782,6 +2789,30 @@ public async Task UpdateSecretScanningAlert_Calls_The_Right_Endpoint_With_Payloa _githubClientMock.Verify(m => m.PatchAsync(url, It.Is(x => x.ToJson() == payload.ToJson()), null)); } + + [Fact] + public async Task UpdateSecretScanningAlert_Replaces_Null_Resolution_Comment_With_Empty_String() + { + // Arrange + const int alertNumber = 100; + const string alertState = "resolved"; + const string alertResolution = "wont_fix"; + + var url = $"https://api.github.com/repos/{GITHUB_ORG}/{GITHUB_REPO}/secret-scanning/alerts/{alertNumber}"; + var payload = new + { + state = alertState, + resolution = alertResolution, + resolution_comment = string.Empty + }; + + // Act + await _githubApi.UpdateSecretScanningAlert(GITHUB_ORG, GITHUB_REPO, alertNumber, alertState, alertResolution); + + // Assert + _githubClientMock.Verify(m => m.PatchAsync(url, It.Is(x => x.ToJson() == payload.ToJson()), null)); + } + [Fact] public async Task GetDefaultBranch_Returns_Default_Branch_Field() { diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs index 1b82a5f32..dbf1027f0 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs @@ -41,6 +41,7 @@ public async Task One_Secret_Updated() SecretType = secretType, Secret = secret, Resolution = SecretScanningAlert.ResolutionRevoked, + ResolutionComment = "Revokation explanation comment" }; var sourceLocation = new GithubSecretScanningAlertLocation() @@ -91,7 +92,8 @@ public async Task One_Secret_Updated() TARGET_REPO, 100, SecretScanningAlert.AlertStateResolved, - SecretScanningAlert.ResolutionRevoked) + SecretScanningAlert.ResolutionRevoked, + "Revokation explanation comment") ); } @@ -109,6 +111,7 @@ public async Task No_Matching_Location() SecretType = secretType, Secret = secret, Resolution = SecretScanningAlert.ResolutionRevoked, + ResolutionComment = "Revokation explanation comment" }; var sourceLocation = new GithubSecretScanningAlertLocation() @@ -159,6 +162,7 @@ public async Task No_Matching_Location() It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny()), Times.Never); } @@ -176,6 +180,7 @@ public async Task No_Matching_Secret() SecretType = secretType, Secret = secret, Resolution = SecretScanningAlert.ResolutionRevoked, + ResolutionComment = "Revokation explanation comment" }; var sourceLocation = new GithubSecretScanningAlertLocation() @@ -216,6 +221,7 @@ public async Task No_Matching_Secret() It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny()), Times.Never); } @@ -233,6 +239,7 @@ public async Task Dry_Run_Does_Not_Update() SecretType = secretType, Secret = secret, Resolution = SecretScanningAlert.ResolutionRevoked, + ResolutionComment = "Revokation explanation comment" }; var sourceLocation = new GithubSecretScanningAlertLocation() @@ -273,6 +280,7 @@ public async Task Dry_Run_Does_Not_Update() It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny()), Times.Never); } @@ -292,6 +300,7 @@ public async Task Migrates_Multiple_Alerts() SecretType = secretType, Secret = secretOne, Resolution = SecretScanningAlert.ResolutionRevoked, + ResolutionComment = "Revokation explanation comment 1" }; var sourceSecretTwo = new GithubSecretScanningAlert() @@ -301,6 +310,7 @@ public async Task Migrates_Multiple_Alerts() SecretType = secretType, Secret = secretTwo, Resolution = SecretScanningAlert.ResolutionRevoked, + ResolutionComment = "Revokation explanation comment 2" }; var sourceSecretThree = new GithubSecretScanningAlert() @@ -310,6 +320,7 @@ public async Task Migrates_Multiple_Alerts() SecretType = secretType, Secret = secretThree, Resolution = SecretScanningAlert.ResolutionFalsePositive, + ResolutionComment = "False positive explanation comment" }; var sourceLocation = new GithubSecretScanningAlertLocation() @@ -358,7 +369,8 @@ public async Task Migrates_Multiple_Alerts() TARGET_REPO, 100, SecretScanningAlert.AlertStateResolved, - SecretScanningAlert.ResolutionRevoked) + SecretScanningAlert.ResolutionRevoked, + "Revokation explanation comment 1") ); _mockTargetGithubApi.Verify(m => m.UpdateSecretScanningAlert( @@ -366,7 +378,77 @@ public async Task Migrates_Multiple_Alerts() TARGET_REPO, 300, SecretScanningAlert.AlertStateResolved, - SecretScanningAlert.ResolutionFalsePositive) + SecretScanningAlert.ResolutionFalsePositive, + "False positive explanation comment") + ); + } + + [Fact] + public async Task One_Secret_Updated_With_No_Resolution_Comment() + { + var secretType = "custom"; + var secret = "my-password"; + + // Arrange + var sourceSecret = new GithubSecretScanningAlert() + { + Number = 1, + State = SecretScanningAlert.AlertStateResolved, + SecretType = secretType, + Secret = secret, + Resolution = SecretScanningAlert.ResolutionRevoked + }; + + var sourceLocation = new GithubSecretScanningAlertLocation() + { + Path = "my-file.txt", + StartLine = 17, + EndLine = 18, + StartColumn = 22, + EndColumn = 29, + BlobSha = "abc123" + }; + + _mockSourceGithubApi.Setup(x => x.GetSecretScanningAlertsForRepository(SOURCE_ORG, SOURCE_REPO)) + .ReturnsAsync(new[] { sourceSecret }); + _mockSourceGithubApi.Setup(x => x.GetSecretScanningAlertsLocations(SOURCE_ORG, SOURCE_REPO, 1)) + .ReturnsAsync(new[] { sourceLocation }); + + var targetSecret = new GithubSecretScanningAlert() + { + Number = 100, + State = SecretScanningAlert.AlertStateOpen, + SecretType = secretType, + Secret = secret + }; + + var targetSecretLocation = new GithubSecretScanningAlertLocation() + { + Path = "my-file.txt", + StartLine = 17, + EndLine = 18, + StartColumn = 22, + EndColumn = 29, + BlobSha = "abc123" + }; + + _mockTargetGithubApi.Setup(x => x.GetSecretScanningAlertsForRepository(TARGET_ORG, TARGET_REPO)) + .ReturnsAsync(new[] { targetSecret }); + + _mockTargetGithubApi.Setup(x => x.GetSecretScanningAlertsLocations(TARGET_ORG, TARGET_REPO, 100)) + .ReturnsAsync(new[] { targetSecretLocation }); + + // Act + await _service.MigrateSecretScanningAlerts(SOURCE_ORG, SOURCE_REPO, TARGET_ORG, TARGET_REPO, false); + + // Assert + _mockTargetGithubApi.Verify(m => m.UpdateSecretScanningAlert( + TARGET_ORG, + TARGET_REPO, + 100, + SecretScanningAlert.AlertStateResolved, + SecretScanningAlert.ResolutionRevoked, + null) ); } } From ca2a407fde8e3d645e161232f7f26c2515dfd845 Mon Sep 17 00:00:00 2001 From: David Linares <28559777+dlinares-linux@users.noreply.github.com> Date: Tue, 21 May 2024 23:01:27 +0900 Subject: [PATCH 2/2] Remove test case following review --- .../SecretScanningAlertServiceTests.cs | 69 ------------------- 1 file changed, 69 deletions(-) diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs index dbf1027f0..dfb50680d 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/SecretScanningAlertServiceTests.cs @@ -382,73 +382,4 @@ public async Task Migrates_Multiple_Alerts() "False positive explanation comment") ); } - - [Fact] - public async Task One_Secret_Updated_With_No_Resolution_Comment() - { - var secretType = "custom"; - var secret = "my-password"; - - // Arrange - var sourceSecret = new GithubSecretScanningAlert() - { - Number = 1, - State = SecretScanningAlert.AlertStateResolved, - SecretType = secretType, - Secret = secret, - Resolution = SecretScanningAlert.ResolutionRevoked - }; - - var sourceLocation = new GithubSecretScanningAlertLocation() - { - Path = "my-file.txt", - StartLine = 17, - EndLine = 18, - StartColumn = 22, - EndColumn = 29, - BlobSha = "abc123" - }; - - _mockSourceGithubApi.Setup(x => x.GetSecretScanningAlertsForRepository(SOURCE_ORG, SOURCE_REPO)) - .ReturnsAsync(new[] { sourceSecret }); - _mockSourceGithubApi.Setup(x => x.GetSecretScanningAlertsLocations(SOURCE_ORG, SOURCE_REPO, 1)) - .ReturnsAsync(new[] { sourceLocation }); - - var targetSecret = new GithubSecretScanningAlert() - { - Number = 100, - State = SecretScanningAlert.AlertStateOpen, - SecretType = secretType, - Secret = secret - }; - - var targetSecretLocation = new GithubSecretScanningAlertLocation() - { - Path = "my-file.txt", - StartLine = 17, - EndLine = 18, - StartColumn = 22, - EndColumn = 29, - BlobSha = "abc123" - }; - - _mockTargetGithubApi.Setup(x => x.GetSecretScanningAlertsForRepository(TARGET_ORG, TARGET_REPO)) - .ReturnsAsync(new[] { targetSecret }); - - _mockTargetGithubApi.Setup(x => x.GetSecretScanningAlertsLocations(TARGET_ORG, TARGET_REPO, 100)) - .ReturnsAsync(new[] { targetSecretLocation }); - - // Act - await _service.MigrateSecretScanningAlerts(SOURCE_ORG, SOURCE_REPO, TARGET_ORG, TARGET_REPO, false); - - // Assert - _mockTargetGithubApi.Verify(m => m.UpdateSecretScanningAlert( - TARGET_ORG, - TARGET_REPO, - 100, - SecretScanningAlert.AlertStateResolved, - SecretScanningAlert.ResolutionRevoked, - null) - ); - } }