From 19aea4b01f3ce5a3cd05d5a1091da5b0b3ba4af6 Mon Sep 17 00:00:00 2001 From: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com> Date: Thu, 14 Nov 2024 23:37:42 +0600 Subject: [PATCH] fix(report): handle `git@github.com` schema for misconfigs in `sarif` report (#7898) --- pkg/report/sarif.go | 38 ++++++++++++++++++- pkg/report/sarif_private_test.go | 59 +++++++++++++++++++++++++++++ pkg/report/sarif_test.go | 64 ++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 pkg/report/sarif_private_test.go diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go index 3bc3344611e2..48578c70cea9 100644 --- a/pkg/report/sarif.go +++ b/pkg/report/sarif.go @@ -346,8 +346,44 @@ 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, "\\", "/") + // 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::https:/") && !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 { + 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 } func toUri(str string) *url.URL { 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)) + }) + } +} diff --git a/pkg/report/sarif_test.go b/pkg/report/sarif_test.go index ce68fab06a8a..c3eebef5c254 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:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.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:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.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/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/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.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/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf"), + }, + }, + }, }, ColumnKind: "utf16CodeUnits", OriginalUriBaseIDs: map[string]*sarif.ArtifactLocation{