From 49bc34de9c42f3d8208dc63084b4aacc3416c510 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Mon, 11 Nov 2024 11:04:16 +0600 Subject: [PATCH 1/7] fix(sarif): overwrite `git@github.com` + `.git` --- pkg/report/sarif.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index 3bc3344611e2..0c996f9b2159 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -346,8 +346,18 @@ func ToPathUri(input string, resultClass types.ResultClass) string { return clearURI(input) } +// clearURI clears URI for misconfigs func clearURI(s string) string { - return strings.ReplaceAll(strings.ReplaceAll(s, "\\", "/"), "git::https:/", "") + s = strings.ReplaceAll(s, "\\", "/") + if strings.HasPrefix(s, "git::https:/") { + s = strings.ReplaceAll(s, "git::https:/", "") + } else if strings.HasPrefix(s, "git@github.com:") { + // cf. https://developer.hashicorp.com/terraform/language/modules/sources#github + s = strings.ReplaceAll(s, "git@github.com:", "github.com/") + s = strings.ReplaceAll(s, ".git", "") + } + + return s } func toUri(str string) *url.URL { From c6138f43f0443db09a4762a9cf821b31f73ea990 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Mon, 11 Nov 2024 11:04:27 +0600 Subject: [PATCH 2/7] test: add test for `git@github.com` schema --- pkg/report/sarif_test.go | 64 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/pkg/report/sarif_test.go b/pkg/report/sarif_test.go index ce68fab06a8a..e70f2b51a07f 100644 --- a/pkg/report/sarif_test.go +++ b/pkg/report/sarif_test.go @@ -588,6 +588,44 @@ func TestReportWriter_Sarif(t *testing.T) { }, }, }, + { + Target: "git@github.com:rbs-path/terraform-modules.git/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf", + Class: types.ClassConfig, + Type: ftypes.Terraform, + Misconfigurations: []types.DetectedMisconfiguration{ + { + Type: "Terraform Security Check", + ID: "AVD-GCP-0007", + AVDID: "AVD-GCP-0007", + Title: "Service accounts should not have roles assigned with excessive privileges", + Description: "Service accounts should have a minimal set of permissions assigned in order to do their job. They should never have excessive access as if compromised, an attacker can escalate privileges and take over the entire account.", + Message: "Service account is granted a privileged role.", + Query: "data..", + Resolution: "Limit service account access to minimal required set", + Severity: "HIGH", + PrimaryURL: "https://avd.aquasec.com/misconfig/avd-gcp-0007", + References: []string{ + "https://cloud.google.com/iam/docs/understanding-roles", + "https://avd.aquasec.com/misconfig/avd-gcp-0007", + }, + Status: "Fail", + CauseMetadata: ftypes.CauseMetadata{ + StartLine: 91, + EndLine: 91, + Occurrences: []ftypes.Occurrence{ + { + Resource: "google_project_iam_member.workload_identity_sa_bindings[\"roles/storage.admin\"]", + Filename: "git@github.com:rbs-path/terraform-modules.git/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf", + Location: ftypes.Location{ + StartLine: 87, + EndLine: 93, + }, + }, + }, + }, + }, + }, + }, }, }, want: &sarif.Report{ @@ -655,6 +693,32 @@ func TestReportWriter_Sarif(t *testing.T) { }, }, }, + { + RuleID: lo.ToPtr("AVD-GCP-0007"), + RuleIndex: lo.ToPtr(uint(0)), + Level: lo.ToPtr("error"), + Message: *sarif.NewTextMessage("Artifact: github.com/rbs-path/terraform-modules/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf\nType: terraform\nVulnerability AVD-GCP-0007\nSeverity: HIGH\nMessage: Service account is granted a privileged role.\nLink: [AVD-GCP-0007](https://avd.aquasec.com/misconfig/avd-gcp-0007)"), + Locations: []*sarif.Location{ + { + PhysicalLocation: sarif.NewPhysicalLocation(). + WithArtifactLocation( + &sarif.ArtifactLocation{ + URI: lo.ToPtr("github.com/rbs-path/terraform-modules/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf"), + URIBaseId: lo.ToPtr("ROOTPATH"), + }, + ). + WithRegion( + &sarif.Region{ + StartLine: lo.ToPtr(91), + StartColumn: lo.ToPtr(1), + EndLine: lo.ToPtr(91), + EndColumn: lo.ToPtr(1), + }, + ), + Message: sarif.NewTextMessage("github.com/rbs-path/terraform-modules/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf"), + }, + }, + }, }, ColumnKind: "utf16CodeUnits", OriginalUriBaseIDs: map[string]*sarif.ArtifactLocation{ From 3ebdecfdafc7d5baf56816c30338cbf23db59a82 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Wed, 13 Nov 2024 16:09:57 +0600 Subject: [PATCH 3/7] fix: remove `.git` for `git::https:/` --- pkg/report/sarif.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index 0c996f9b2159..2f736893e4ea 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -351,6 +351,7 @@ func clearURI(s string) string { s = strings.ReplaceAll(s, "\\", "/") if strings.HasPrefix(s, "git::https:/") { s = strings.ReplaceAll(s, "git::https:/", "") + s = strings.ReplaceAll(s, ".git", "") } else if strings.HasPrefix(s, "git@github.com:") { // cf. https://developer.hashicorp.com/terraform/language/modules/sources#github s = strings.ReplaceAll(s, "git@github.com:", "github.com/") From 58cc321eb5eb9e50cde93684bb71cf30588ab75a Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 14 Nov 2024 15:05:48 +0600 Subject: [PATCH 4/7] fix: replase `?ref=` to `/tree/` for GitHub links --- pkg/report/sarif.go | 5 ++++- pkg/report/sarif_test.go | 10 +++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index 2f736893e4ea..75f2e7f5ec49 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -353,9 +353,12 @@ func clearURI(s string) string { s = strings.ReplaceAll(s, "git::https:/", "") s = strings.ReplaceAll(s, ".git", "") } else if strings.HasPrefix(s, "git@github.com:") { - // cf. https://developer.hashicorp.com/terraform/language/modules/sources#github + // build GitHub url format + // e.g. `git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf` -> `github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf` + // cf. https://github.com/aquasecurity/trivy/issues/7897 s = strings.ReplaceAll(s, "git@github.com:", "github.com/") s = strings.ReplaceAll(s, ".git", "") + s = strings.ReplaceAll(s, "?ref=", "/tree/") } return s diff --git a/pkg/report/sarif_test.go b/pkg/report/sarif_test.go index e70f2b51a07f..c3eebef5c254 100644 --- a/pkg/report/sarif_test.go +++ b/pkg/report/sarif_test.go @@ -589,7 +589,7 @@ func TestReportWriter_Sarif(t *testing.T) { }, }, { - Target: "git@github.com:rbs-path/terraform-modules.git/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf", + Target: "git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf", Class: types.ClassConfig, Type: ftypes.Terraform, Misconfigurations: []types.DetectedMisconfiguration{ @@ -615,7 +615,7 @@ func TestReportWriter_Sarif(t *testing.T) { Occurrences: []ftypes.Occurrence{ { Resource: "google_project_iam_member.workload_identity_sa_bindings[\"roles/storage.admin\"]", - Filename: "git@github.com:rbs-path/terraform-modules.git/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf", + Filename: "git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf", Location: ftypes.Location{ StartLine: 87, EndLine: 93, @@ -697,13 +697,13 @@ func TestReportWriter_Sarif(t *testing.T) { RuleID: lo.ToPtr("AVD-GCP-0007"), RuleIndex: lo.ToPtr(uint(0)), Level: lo.ToPtr("error"), - Message: *sarif.NewTextMessage("Artifact: github.com/rbs-path/terraform-modules/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf\nType: terraform\nVulnerability AVD-GCP-0007\nSeverity: HIGH\nMessage: Service account is granted a privileged role.\nLink: [AVD-GCP-0007](https://avd.aquasec.com/misconfig/avd-gcp-0007)"), + Message: *sarif.NewTextMessage("Artifact: github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf\nType: terraform\nVulnerability AVD-GCP-0007\nSeverity: HIGH\nMessage: Service account is granted a privileged role.\nLink: [AVD-GCP-0007](https://avd.aquasec.com/misconfig/avd-gcp-0007)"), Locations: []*sarif.Location{ { PhysicalLocation: sarif.NewPhysicalLocation(). WithArtifactLocation( &sarif.ArtifactLocation{ - URI: lo.ToPtr("github.com/rbs-path/terraform-modules/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf"), + URI: lo.ToPtr("github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf"), URIBaseId: lo.ToPtr("ROOTPATH"), }, ). @@ -715,7 +715,7 @@ func TestReportWriter_Sarif(t *testing.T) { EndColumn: lo.ToPtr(1), }, ), - Message: sarif.NewTextMessage("github.com/rbs-path/terraform-modules/modules/compute/aws-lambda?ref=v3.8.0/.terraform/modules/cleanup/modules/compute/aws-lambda/cloudwatch.tf"), + Message: sarif.NewTextMessage("github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf"), }, }, }, From 80496e195b85d489cc1778e72dff9df74f01fbae Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 14 Nov 2024 15:56:45 +0600 Subject: [PATCH 5/7] feat: add other source types --- pkg/report/sarif.go | 27 ++++++++++++--- pkg/report/sarif_private_test.go | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 pkg/report/sarif_private_test.go diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index 75f2e7f5ec49..bfdcc799555d 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -349,16 +349,35 @@ func ToPathUri(input string, resultClass types.ResultClass) string { // clearURI clears URI for misconfigs func clearURI(s string) string { s = strings.ReplaceAll(s, "\\", "/") - if strings.HasPrefix(s, "git::https:/") { - s = strings.ReplaceAll(s, "git::https:/", "") - s = strings.ReplaceAll(s, ".git", "") - } else if strings.HasPrefix(s, "git@github.com:") { + // cf. https://developer.hashicorp.com/terraform/language/modules/sources + switch { + case strings.HasPrefix(s, "git@github.com:"): // build GitHub url format // e.g. `git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf` -> `github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf` // cf. https://github.com/aquasecurity/trivy/issues/7897 s = strings.ReplaceAll(s, "git@github.com:", "github.com/") s = strings.ReplaceAll(s, ".git", "") s = strings.ReplaceAll(s, "?ref=", "/tree/") + case strings.HasPrefix(s, "git::ssh://"): + // `"`git::ssh://username@example.com/storage.git` -> `example.com/storage.git` + if _, u, ok := strings.Cut(s, "@"); ok { + s = u + } + s = strings.ReplaceAll(s, ".git", "") + case strings.HasPrefix(s, "git::"): + // `git::https://example.com/vpc.git` -> `https://example.com/vpc` + s = strings.TrimPrefix(s, "git::") + s = strings.ReplaceAll(s, ".git", "") + case strings.HasPrefix(s, "hg::"): + // `hg::http://example.com/vpc.hg` -> `http://example.com/vpc` + s = strings.TrimPrefix(s, "hg::") + s = strings.ReplaceAll(s, ".hg", "") + case strings.HasPrefix(s, "s3::"): + // `s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip` -> `https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip` + s = strings.TrimPrefix(s, "s3::") + case strings.HasPrefix(s, "gcs::"): + // `gcs::https://www.googleapis.com/storage/v1/modules/foomodule.zipp` -> `https://www.googleapis.com/storage/v1/modules/foomodule.zip` + s = strings.TrimPrefix(s, "gcs::") } return s diff --git a/pkg/report/sarif_private_test.go b/pkg/report/sarif_private_test.go new file mode 100644 index 000000000000..b9384599f7b0 --- /dev/null +++ b/pkg/report/sarif_private_test.go @@ -0,0 +1,59 @@ +package report + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_clearURI(t *testing.T) { + test := []struct { + name string + uri string + want string + }{ + { + name: "https", + uri: "bitbucket.org/hashicorp/terraform-consul-aws", + want: "bitbucket.org/hashicorp/terraform-consul-aws", + }, + { + name: "github", + uri: "git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf", + want: "github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf", + }, + { + name: "git", + uri: "git::https://example.com/storage.git?ref=51d462976d84fdea54b47d80dcabbf680badcdb8", + want: "https://example.com/storage?ref=51d462976d84fdea54b47d80dcabbf680badcdb8", + }, + { + name: "git ssh", + uri: "git::ssh://username@example.com/storage.git", + want: "example.com/storage", + }, + { + name: "hg", + uri: "hg::http://example.com/vpc.hg?ref=v1.2.0", + want: "http://example.com/vpc?ref=v1.2.0", + }, + { + name: "s3", + uri: "s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip", + want: "https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip", + }, + { + name: "gcs", + uri: "gcs::https://www.googleapis.com/storage/v1/modules/foomodule.zip", + want: "https://www.googleapis.com/storage/v1/modules/foomodule.zip", + }, + } + + for _, tt := range test { + t.Run(tt.name, func(t *testing.T) { + got := clearURI(tt.uri) + require.Equal(t, tt.want, got) + require.NotNil(t, toUri(got)) + }) + } +} From 4ed1befdfa9c7d4a91503523e44e7b7c4be59744 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 14 Nov 2024 16:23:52 +0600 Subject: [PATCH 6/7] fix: add `git::https:/` --- pkg/report/sarif.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index bfdcc799555d..88f5200acef7 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -358,6 +358,9 @@ func clearURI(s string) string { s = strings.ReplaceAll(s, "git@github.com:", "github.com/") s = strings.ReplaceAll(s, ".git", "") s = strings.ReplaceAll(s, "?ref=", "/tree/") + case strings.HasPrefix(s, "git::https:/"): + s = strings.TrimPrefix(s, "git::https:/") + s = strings.ReplaceAll(s, ".git", "") case strings.HasPrefix(s, "git::ssh://"): // `"`git::ssh://username@example.com/storage.git` -> `example.com/storage.git` if _, u, ok := strings.Cut(s, "@"); ok { From 5abb1cae1ede16d18783b31d83c345ebe2755eb9 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 14 Nov 2024 16:42:17 +0600 Subject: [PATCH 7/7] fix: `git::https:/` --- pkg/report/sarif.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index 88f5200acef7..48578c70cea9 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -358,7 +358,7 @@ func clearURI(s string) string { s = strings.ReplaceAll(s, "git@github.com:", "github.com/") s = strings.ReplaceAll(s, ".git", "") s = strings.ReplaceAll(s, "?ref=", "/tree/") - case strings.HasPrefix(s, "git::https:/"): + case strings.HasPrefix(s, "git::https:/") && !strings.HasPrefix(s, "git::https://"): s = strings.TrimPrefix(s, "git::https:/") s = strings.ReplaceAll(s, ".git", "") case strings.HasPrefix(s, "git::ssh://"):