Skip to content

Commit 19aea4b

Browse files
authoredNov 14, 2024··
fix(report): handle git@github.com schema for misconfigs in sarif report (#7898)
1 parent bdfcc19 commit 19aea4b

File tree

3 files changed

+160
-1
lines changed

3 files changed

+160
-1
lines changed
 

‎pkg/report/sarif.go

+37-1
Original file line numberDiff line numberDiff line change
@@ -346,8 +346,44 @@ func ToPathUri(input string, resultClass types.ResultClass) string {
346346
return clearURI(input)
347347
}
348348

349+
// clearURI clears URI for misconfigs
349350
func clearURI(s string) string {
350-
return strings.ReplaceAll(strings.ReplaceAll(s, "\\", "/"), "git::https:/", "")
351+
s = strings.ReplaceAll(s, "\\", "/")
352+
// cf. https://developer.hashicorp.com/terraform/language/modules/sources
353+
switch {
354+
case strings.HasPrefix(s, "git@github.com:"):
355+
// build GitHub url format
356+
// 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`
357+
// cf. https://github.com/aquasecurity/trivy/issues/7897
358+
s = strings.ReplaceAll(s, "git@github.com:", "github.com/")
359+
s = strings.ReplaceAll(s, ".git", "")
360+
s = strings.ReplaceAll(s, "?ref=", "/tree/")
361+
case strings.HasPrefix(s, "git::https:/") && !strings.HasPrefix(s, "git::https://"):
362+
s = strings.TrimPrefix(s, "git::https:/")
363+
s = strings.ReplaceAll(s, ".git", "")
364+
case strings.HasPrefix(s, "git::ssh://"):
365+
// `"`git::ssh://username@example.com/storage.git` -> `example.com/storage.git`
366+
if _, u, ok := strings.Cut(s, "@"); ok {
367+
s = u
368+
}
369+
s = strings.ReplaceAll(s, ".git", "")
370+
case strings.HasPrefix(s, "git::"):
371+
// `git::https://example.com/vpc.git` -> `https://example.com/vpc`
372+
s = strings.TrimPrefix(s, "git::")
373+
s = strings.ReplaceAll(s, ".git", "")
374+
case strings.HasPrefix(s, "hg::"):
375+
// `hg::http://example.com/vpc.hg` -> `http://example.com/vpc`
376+
s = strings.TrimPrefix(s, "hg::")
377+
s = strings.ReplaceAll(s, ".hg", "")
378+
case strings.HasPrefix(s, "s3::"):
379+
// `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`
380+
s = strings.TrimPrefix(s, "s3::")
381+
case strings.HasPrefix(s, "gcs::"):
382+
// `gcs::https://www.googleapis.com/storage/v1/modules/foomodule.zipp` -> `https://www.googleapis.com/storage/v1/modules/foomodule.zip`
383+
s = strings.TrimPrefix(s, "gcs::")
384+
}
385+
386+
return s
351387
}
352388

353389
func toUri(str string) *url.URL {

‎pkg/report/sarif_private_test.go

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package report
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func Test_clearURI(t *testing.T) {
10+
test := []struct {
11+
name string
12+
uri string
13+
want string
14+
}{
15+
{
16+
name: "https",
17+
uri: "bitbucket.org/hashicorp/terraform-consul-aws",
18+
want: "bitbucket.org/hashicorp/terraform-consul-aws",
19+
},
20+
{
21+
name: "github",
22+
uri: "git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf",
23+
want: "github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf",
24+
},
25+
{
26+
name: "git",
27+
uri: "git::https://example.com/storage.git?ref=51d462976d84fdea54b47d80dcabbf680badcdb8",
28+
want: "https://example.com/storage?ref=51d462976d84fdea54b47d80dcabbf680badcdb8",
29+
},
30+
{
31+
name: "git ssh",
32+
uri: "git::ssh://username@example.com/storage.git",
33+
want: "example.com/storage",
34+
},
35+
{
36+
name: "hg",
37+
uri: "hg::http://example.com/vpc.hg?ref=v1.2.0",
38+
want: "http://example.com/vpc?ref=v1.2.0",
39+
},
40+
{
41+
name: "s3",
42+
uri: "s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip",
43+
want: "https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip",
44+
},
45+
{
46+
name: "gcs",
47+
uri: "gcs::https://www.googleapis.com/storage/v1/modules/foomodule.zip",
48+
want: "https://www.googleapis.com/storage/v1/modules/foomodule.zip",
49+
},
50+
}
51+
52+
for _, tt := range test {
53+
t.Run(tt.name, func(t *testing.T) {
54+
got := clearURI(tt.uri)
55+
require.Equal(t, tt.want, got)
56+
require.NotNil(t, toUri(got))
57+
})
58+
}
59+
}

‎pkg/report/sarif_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,44 @@ func TestReportWriter_Sarif(t *testing.T) {
588588
},
589589
},
590590
},
591+
{
592+
Target: "git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf",
593+
Class: types.ClassConfig,
594+
Type: ftypes.Terraform,
595+
Misconfigurations: []types.DetectedMisconfiguration{
596+
{
597+
Type: "Terraform Security Check",
598+
ID: "AVD-GCP-0007",
599+
AVDID: "AVD-GCP-0007",
600+
Title: "Service accounts should not have roles assigned with excessive privileges",
601+
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.",
602+
Message: "Service account is granted a privileged role.",
603+
Query: "data..",
604+
Resolution: "Limit service account access to minimal required set",
605+
Severity: "HIGH",
606+
PrimaryURL: "https://avd.aquasec.com/misconfig/avd-gcp-0007",
607+
References: []string{
608+
"https://cloud.google.com/iam/docs/understanding-roles",
609+
"https://avd.aquasec.com/misconfig/avd-gcp-0007",
610+
},
611+
Status: "Fail",
612+
CauseMetadata: ftypes.CauseMetadata{
613+
StartLine: 91,
614+
EndLine: 91,
615+
Occurrences: []ftypes.Occurrence{
616+
{
617+
Resource: "google_project_iam_member.workload_identity_sa_bindings[\"roles/storage.admin\"]",
618+
Filename: "git@github.com:terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v4.2.0/main.tf",
619+
Location: ftypes.Location{
620+
StartLine: 87,
621+
EndLine: 93,
622+
},
623+
},
624+
},
625+
},
626+
},
627+
},
628+
},
591629
},
592630
},
593631
want: &sarif.Report{
@@ -655,6 +693,32 @@ func TestReportWriter_Sarif(t *testing.T) {
655693
},
656694
},
657695
},
696+
{
697+
RuleID: lo.ToPtr("AVD-GCP-0007"),
698+
RuleIndex: lo.ToPtr(uint(0)),
699+
Level: lo.ToPtr("error"),
700+
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)"),
701+
Locations: []*sarif.Location{
702+
{
703+
PhysicalLocation: sarif.NewPhysicalLocation().
704+
WithArtifactLocation(
705+
&sarif.ArtifactLocation{
706+
URI: lo.ToPtr("github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf"),
707+
URIBaseId: lo.ToPtr("ROOTPATH"),
708+
},
709+
).
710+
WithRegion(
711+
&sarif.Region{
712+
StartLine: lo.ToPtr(91),
713+
StartColumn: lo.ToPtr(1),
714+
EndLine: lo.ToPtr(91),
715+
EndColumn: lo.ToPtr(1),
716+
},
717+
),
718+
Message: sarif.NewTextMessage("github.com/terraform-aws-modules/terraform-aws-s3-bucket/tree/v4.2.0/main.tf"),
719+
},
720+
},
721+
},
658722
},
659723
ColumnKind: "utf16CodeUnits",
660724
OriginalUriBaseIDs: map[string]*sarif.ArtifactLocation{

0 commit comments

Comments
 (0)
Please sign in to comment.