From c238f2931b5a1e7233a58aad5e93bcabed9c9eb3 Mon Sep 17 00:00:00 2001 From: Assaf Attias <49212512+attiasas@users.noreply.github.com> Date: Thu, 26 Dec 2024 16:37:33 +0200 Subject: [PATCH] Escape Job-Id for url (#272) --- go.mod | 10 +++++----- go.sum | 20 +++++++++---------- .../binary_analytics_vulnerabilities.md | 2 +- .../build_scan_analytics_vulnerabilities.md | 2 +- .../output/jobSummary/violations_analytics.md | 2 +- .../conversion/sarifparser/sarifparser.go | 5 +++-- .../sarifparser/sarifparser_test.go | 6 +++--- utils/results/output/securityJobSummary.go | 13 ++++++------ .../results/output/securityJobSummary_test.go | 2 +- 9 files changed, 32 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index d0418bad..5c08a53b 100644 --- a/go.mod +++ b/go.mod @@ -10,9 +10,9 @@ require ( github.com/jfrog/froggit-go v1.16.2 github.com/jfrog/gofrog v1.7.6 github.com/jfrog/jfrog-apps-config v1.0.1 - github.com/jfrog/jfrog-cli-core/v2 v2.57.2 - github.com/jfrog/jfrog-client-go v1.48.4 - github.com/magiconair/properties v1.8.7 + github.com/jfrog/jfrog-cli-core/v2 v2.57.5 + github.com/jfrog/jfrog-client-go v1.48.6 + github.com/magiconair/properties v1.8.9 github.com/owenrumney/go-sarif/v2 v2.3.0 github.com/stretchr/testify v1.10.0 github.com/urfave/cli v1.22.16 @@ -58,7 +58,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.7 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect - github.com/jedib0t/go-pretty/v6 v6.6.3 // indirect + github.com/jedib0t/go-pretty/v6 v6.6.5 // indirect github.com/jfrog/archiver/v3 v3.6.1 // indirect github.com/kevinburke/ssh_config v1.2.0 // indirect github.com/klauspost/compress v1.17.9 // indirect @@ -102,7 +102,7 @@ require ( go.uber.org/multierr v1.9.0 // indirect golang.org/x/crypto v0.31.0 // indirect golang.org/x/mod v0.22.0 // indirect - golang.org/x/net v0.31.0 // indirect + golang.org/x/net v0.33.0 // indirect golang.org/x/oauth2 v0.20.0 // indirect golang.org/x/sys v0.28.0 // indirect golang.org/x/term v0.27.0 // indirect diff --git a/go.sum b/go.sum index 107ac694..9c3fec37 100644 --- a/go.sum +++ b/go.sum @@ -115,8 +115,8 @@ github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= -github.com/jedib0t/go-pretty/v6 v6.6.3 h1:nGqgS0tgIO1Hto47HSaaK4ac/I/Bu7usmdD3qvs0WvM= -github.com/jedib0t/go-pretty/v6 v6.6.3/go.mod h1:zbn98qrYlh95FIhwwsbIip0LYpwSG8SUOScs+v9/t0E= +github.com/jedib0t/go-pretty/v6 v6.6.5 h1:9PgMJOVBedpgYLI56jQRJYqngxYAAzfEUua+3NgSqAo= +github.com/jedib0t/go-pretty/v6 v6.6.5/go.mod h1:Uq/HrbhuFty5WSVNfjpQQe47x16RwVGXIveNGEyGtHs= github.com/jfrog/archiver/v3 v3.6.1 h1:LOxnkw9pOn45DzCbZNFV6K0+6dCsQ0L8mR3ZcujO5eI= github.com/jfrog/archiver/v3 v3.6.1/go.mod h1:VgR+3WZS4N+i9FaDwLZbq+jeU4B4zctXL+gL4EMzfLw= github.com/jfrog/build-info-go v1.10.7 h1:10NVHYg0193gJpQft+S4WQfvYMtj5jlwwhJRvkFJtBE= @@ -127,10 +127,10 @@ github.com/jfrog/gofrog v1.7.6 h1:QmfAiRzVyaI7JYGsB7cxfAJePAZTzFz0gRWZSE27c6s= github.com/jfrog/gofrog v1.7.6/go.mod h1:ntr1txqNOZtHplmaNd7rS4f8jpA5Apx8em70oYEe7+4= github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYLipdsOFMY= github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w= -github.com/jfrog/jfrog-cli-core/v2 v2.57.2 h1:2shy1CRWm/8yf6WWfVyAW3AdmryQiI73Tkhfb62vgPE= -github.com/jfrog/jfrog-cli-core/v2 v2.57.2/go.mod h1:sgi0gw96J00Yzx2cKG5xTG/x9XD0YiJbglJOnXUeaD0= -github.com/jfrog/jfrog-client-go v1.48.4 h1:uXvBr2ebFKpBRUhWgC9TSSJe32IbSYGlbDp9tDzBcaY= -github.com/jfrog/jfrog-client-go v1.48.4/go.mod h1:2ySOMva54L3EYYIlCBYBTcTgqfrrQ19gtpA/MWfA/ec= +github.com/jfrog/jfrog-cli-core/v2 v2.57.5 h1:guVB/zPPtS8CWpNvAFPCxNvSgVra4TyX8lzs4V4+I/4= +github.com/jfrog/jfrog-cli-core/v2 v2.57.5/go.mod h1:LfKvCRXbvwgE0V6aX3/GabkzCedghXq0Y6lmsEuxr44= +github.com/jfrog/jfrog-client-go v1.48.6 h1:Pl9foa9XBaPbP3gz4wevDmF/mpfit94IQHKQKnfk3lA= +github.com/jfrog/jfrog-client-go v1.48.6/go.mod h1:2ySOMva54L3EYYIlCBYBTcTgqfrrQ19gtpA/MWfA/ec= github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88/go.mod h1:3w7q1U84EfirKl04SVQ/s7nPm1ZPhiXd34z40TNz36k= github.com/k0kubun/pp v3.0.1+incompatible/go.mod h1:GWse8YhT0p8pT4ir3ZgBbfZild3tgzSScAn6HmfYukg= github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4= @@ -154,8 +154,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/ktrysmt/go-bitbucket v0.9.80 h1:S+vZTXKx/VG5yCaX4I3Bmwo8lxWr4ifvuHdTboHTMMc= github.com/ktrysmt/go-bitbucket v0.9.80/go.mod h1:b8ogWEGxQMWoeFnT1ZE4aHIPGindI+9z/zAW/OVFjk0= -github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0VQdvPDY= -github.com/magiconair/properties v1.8.7/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= +github.com/magiconair/properties v1.8.9 h1:nWcCbLq1N2v/cpNsy5WvQ37Fb+YElfq20WJ/a8RkpQM= +github.com/magiconair/properties v1.8.9/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= github.com/manifoldco/promptui v0.9.0 h1:3V4HzJk1TtXW1MTZMP7mdlwbBpIinw3HztaIlYthEiA= github.com/manifoldco/promptui v0.9.0/go.mod h1:ka04sppxSGFAtxX0qhlYQjISsg9mR4GWtQEhdbn6Pgg= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= @@ -308,8 +308,8 @@ golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= -golang.org/x/net v0.31.0 h1:68CPQngjLL0r2AlUKiSxtQFKvzRVbnzLwMUn5SzcLHo= -golang.org/x/net v0.31.0/go.mod h1:P4fl1q7dY2hnZFxEk4pPSkDHF+QqjitcnDjUQyMM+pM= +golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I= +golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.20.0 h1:4mQdhULixXKP1rwYBW0vAijoXnkTG0BLCDRzfe1idMo= golang.org/x/oauth2 v0.20.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= diff --git a/tests/testdata/output/jobSummary/binary_analytics_vulnerabilities.md b/tests/testdata/output/jobSummary/binary_analytics_vulnerabilities.md index bc6f9bf2..90496612 100644 --- a/tests/testdata/output/jobSummary/binary_analytics_vulnerabilities.md +++ b/tests/testdata/output/jobSummary/binary_analytics_vulnerabilities.md @@ -1 +1 @@ -
44 Security issues are grouped by CVE number:	44 SCA\ No newline at end of file +❗️ 33 Critical🟡 11 Low
See the results of the scan in JFrog
44 Security issues are grouped by CVE number:	44 SCA\ No newline at end of file diff --git a/tests/testdata/output/jobSummary/build_scan_analytics_vulnerabilities.md b/tests/testdata/output/jobSummary/build_scan_analytics_vulnerabilities.md index beb4376a..904377a1 100644 --- a/tests/testdata/output/jobSummary/build_scan_analytics_vulnerabilities.md +++ b/tests/testdata/output/jobSummary/build_scan_analytics_vulnerabilities.md @@ -1 +1 @@ -❗️ 33 Critical🟡 11 Low
See the results of the scan in JFrog
24 Security Issues:	24 SCA\ No newline at end of file +🔴 3 High🟠 1 Medium⚪️ 20 Unknown
See the results of the scan in JFrog
24 Security Issues:	24 SCA\ No newline at end of file diff --git a/tests/testdata/output/jobSummary/violations_analytics.md b/tests/testdata/output/jobSummary/violations_analytics.md index 9197b43c..15766051 100644 --- a/tests/testdata/output/jobSummary/violations_analytics.md +++ b/tests/testdata/output/jobSummary/violations_analytics.md @@ -1 +1 @@ -🔴 3 High🟠 1 Medium⚪️ 20 Unknown
See the results of the scan in JFrog
watches:
watch1, watch2, watch3, watch4
watch5
23 Policy Violations:	17 Security	2 Operational	1 License	3 Secrets\ No newline at end of file +❗️ 8 Critical (2 Not Applicable)🔴 6 High🟠 3 Medium🟡 5 Low (3 Not Applicable)⚪️ 1 Unknown
See the results of the scan in JFrog
watches:
watch1, watch2, watch3, watch4
watch5
23 Policy Violations:	17 Security	2 Operational	1 License	3 Secrets\ No newline at end of file diff --git a/utils/results/conversion/sarifparser/sarifparser.go b/utils/results/conversion/sarifparser/sarifparser.go index a334285c..4dba3a6b 100644 --- a/utils/results/conversion/sarifparser/sarifparser.go +++ b/utils/results/conversion/sarifparser/sarifparser.go @@ -2,6 +2,7 @@ package sarifparser import ( "fmt" + "net/url" "os" "path/filepath" "regexp" @@ -495,7 +496,7 @@ func patchRules(platformBaseUrl string, commandType utils.CommandType, subScanTy } // Add analytics hidden pixel to the help content if needed (Github code scanning) if analytics := getAnalyticsHiddenPixel(platformBaseUrl, subScanType); rule.Help != nil && analytics != "" { - rule.Help.Markdown = utils.NewStringPtr(fmt.Sprintf("%s\n\n%s", sarifutils.GetRuleHelpMarkdown(rule), analytics)) + rule.Help.Markdown = utils.NewStringPtr(fmt.Sprintf("%s %s", sarifutils.GetRuleHelpMarkdown(rule), analytics)) } patched = append(patched, rule) } @@ -775,7 +776,7 @@ func getAnalyticsHiddenPixel(baseUrl string, resultOfSubScan utils.SubScanType) return fmt.Sprintf( "![](%sui/api/v1/u?s=1&m=2&job_id=%s&run_id=%s&git_repo=%s&type=%s)", baseUrl, - jobId, + url.PathEscape(jobId), runId, gitRepo, resultOfSubScan.String(), diff --git a/utils/results/conversion/sarifparser/sarifparser_test.go b/utils/results/conversion/sarifparser/sarifparser_test.go index 29f48a31..21158f27 100644 --- a/utils/results/conversion/sarifparser/sarifparser_test.go +++ b/utils/results/conversion/sarifparser/sarifparser_test.go @@ -364,7 +364,7 @@ func TestPatchRunsToPassIngestionRules(t *testing.T) { ), }, expectedResults: []*sarif.Run{ - sarifutils.CreateRunWithDummyResultsWithRuleInformation("", "", "rule-msg", "rule-markdown\n\n![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", "rule-msg", "rule-markdown\n\n![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", wd, + sarifutils.CreateRunWithDummyResultsWithRuleInformation("", "", "rule-msg", "rule-markdown ![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", "rule-msg", "rule-markdown ![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", wd, sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("some-msg\nGithub Actions Workflow: %s\nRun: 123\nImage: dockerImage:imageVersion\nLayer (sha256): f752cb05a39e65f231a3c47c2e08cbeac1c15e4daff0188cb129c12a3ea3049d", filepath.Join(GithubBaseWorkflowDir, "workflowFile.yml")), "some-msg", jfrogFingerprintAlgorithmName, "eda26ae830c578197aeda65a82d7f093", sarifutils.CreateDummyLocationWithPathAndLogicalLocation("", "f752cb05a39e65f231a3c47c2e08cbeac1c15e4daff0188cb129c12a3ea3049d", "layer", "algorithm", "sha256").WithPhysicalLocation( sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewSimpleArtifactLocation(filepath.Join(GithubBaseWorkflowDir, "workflowFile.yml"))), @@ -386,7 +386,7 @@ func TestPatchRunsToPassIngestionRules(t *testing.T) { ), }, expectedResults: []*sarif.Run{ - sarifutils.CreateRunWithDummyResultsWithRuleInformation("", "", "rule-msg", "rule-markdown\n\n![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", "rule-msg", "rule-markdown\n\n![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", dockerfileDir, + sarifutils.CreateRunWithDummyResultsWithRuleInformation("", "", "rule-msg", "rule-markdown ![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", "rule-msg", "rule-markdown ![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=sca)", dockerfileDir, sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("some-msg\nGithub Actions Workflow: %s\nRun: 123\nImage: dockerImage:imageVersion\nLayer (sha256): f752cb05a39e65f231a3c47c2e08cbeac1c15e4daff0188cb129c12a3ea3049d", filepath.Join(GithubBaseWorkflowDir, "workflowFile.yml")), "some-msg", jfrogFingerprintAlgorithmName, "8cbd7268a4d20f2358ba2667ebd18956", sarifutils.CreateDummyLocationWithPathAndLogicalLocation("", "f752cb05a39e65f231a3c47c2e08cbeac1c15e4daff0188cb129c12a3ea3049d", "layer", "algorithm", "sha256").WithPhysicalLocation( sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewSimpleArtifactLocation("Dockerfile")), @@ -429,7 +429,7 @@ func TestPatchRunsToPassIngestionRules(t *testing.T) { }), }, expectedResults: []*sarif.Run{ - sarifutils.CreateRunWithDummyResultsWithRuleInformation(BinarySecretScannerToolName, "[Secret in Binary found] ", "rule-msg", "rule-markdown\n\n![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=secrets)", "rule-msg", "rule-markdown\n\n![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=secrets)", wd, + sarifutils.CreateRunWithDummyResultsWithRuleInformation(BinarySecretScannerToolName, "[Secret in Binary found] ", "rule-msg", "rule-markdown ![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=secrets)", "rule-msg", "rule-markdown ![](url/ui/api/v1/u?s=1&m=2&job_id=job-id&run_id=run-id&git_repo=repo&type=secrets)", wd, sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("🔒 Found Secrets in Binary docker scanning:\nGithub Actions Workflow: %s\nRun: 123\nImage: dockerImage:imageVersion\nLayer (sha1): 9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0\nFilepath: %s\nEvidence: snippet", filepath.Join(GithubBaseWorkflowDir, "workflowFile.yml"), filepath.Join("usr", "src", "app", "server", "index.js")), "result-msg", jfrogFingerprintAlgorithmName, "e721eacf317da6090eca3522308abd28", sarifutils.CreateDummyLocationWithPathAndLogicalLocation("", "9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0", "layer", "algorithm", "sha1").WithPhysicalLocation( sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewSimpleArtifactLocation(filepath.Join(GithubBaseWorkflowDir, "workflowFile.yml"))), diff --git a/utils/results/output/securityJobSummary.go b/utils/results/output/securityJobSummary.go index e27b0e01..dffc3106 100644 --- a/utils/results/output/securityJobSummary.go +++ b/utils/results/output/securityJobSummary.go @@ -3,6 +3,7 @@ package output import ( "errors" "fmt" + "net/url" "os" "path/filepath" "sort" @@ -542,13 +543,13 @@ func getJfrogUrl(index commandsummary.Index, args ResultSummaryArgs, summary *fo } // adds analytics query params to the url if running in Github -func addAnalyticsQueryParamsIfNeeded(url string, index commandsummary.Index) string { +func addAnalyticsQueryParamsIfNeeded(platformUrl string, index commandsummary.Index) string { githubJobId := os.Getenv(utils.JfrogExternalJobIdEnv) if githubJobId == "" { // Not running in Github no need to add analytics - return url + return platformUrl } - suffixValues := []string{fmt.Sprintf("gh_job_id=%s", githubJobId)} + suffixValues := []string{fmt.Sprintf("gh_job_id=%s", url.PathEscape(githubJobId))} // Add section analytics indexValue := "gh_section=" switch index { @@ -559,10 +560,10 @@ func addAnalyticsQueryParamsIfNeeded(url string, index commandsummary.Index) str } suffixValues = append(suffixValues, indexValue) // Add the suffix to the url - if strings.Contains(url, "?") { - return fmt.Sprintf("%s%s", url, strings.Join(suffixValues, "&")) + if strings.Contains(platformUrl, "?") { + return fmt.Sprintf("%s%s", platformUrl, strings.Join(suffixValues, "&")) } - return fmt.Sprintf("%s?%s", url, strings.Join(suffixValues, "&")) + return fmt.Sprintf("%s?%s", platformUrl, strings.Join(suffixValues, "&")) } func (mg DynamicMarkdownGenerator) generateResultsMarkdown(violations bool, moreInfoUrl string, content *formats.ScanResultSummary) (markdown string) { diff --git a/utils/results/output/securityJobSummary_test.go b/utils/results/output/securityJobSummary_test.go index 7d748802..4ec435b5 100644 --- a/utils/results/output/securityJobSummary_test.go +++ b/utils/results/output/securityJobSummary_test.go @@ -485,7 +485,7 @@ func TestGenerateJobSummaryMarkdown(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { cleanUps := []func(){} if testCase.GithubEnvs { - cleanUps = append(cleanUps, clientTests.SetEnvWithCallbackAndAssert(t, utils.JfrogExternalJobIdEnv, "some-job-id")) + cleanUps = append(cleanUps, clientTests.SetEnvWithCallbackAndAssert(t, utils.JfrogExternalJobIdEnv, "some job id")) cleanUps = append(cleanUps, clientTests.SetEnvWithCallbackAndAssert(t, utils.JfrogExternalRunIdEnv, "some-run-id")) cleanUps = append(cleanUps, clientTests.SetEnvWithCallbackAndAssert(t, utils.JfrogExternalGitRepoEnv, "some-repo")) }❗️ 8 Critical (2 Not Applicable)🔴 6 High🟠 3 Medium🟡 5 Low (3 Not Applicable)⚪️ 1 Unknown
See the results of the scan in JFrog