From 4ca63a82a1ced7c227974b78ba2fda455c5bb07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 12:41:57 +0000 Subject: [PATCH 01/11] add new queryid validation --- ...include_ids_with_new_queryid_validation.go | 30 +++++++++++++++++++ internal/console/flags/validate.go | 11 +++++-- internal/console/flags/validate_test.go | 20 +++++++++++++ .../new_queryid_validation/Dockerfile | 3 ++ .../new_queryid_validation/metadata.json | 12 ++++++++ .../new_queryid_validation/query.rego | 19 ++++++++++++ 6 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go create mode 100644 test/fixtures/new_queryid_validation/Dockerfile create mode 100644 test/fixtures/new_queryid_validation/metadata.json create mode 100644 test/fixtures/new_queryid_validation/query.rego diff --git a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go new file mode 100644 index 00000000000..c7c0d538191 --- /dev/null +++ b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go @@ -0,0 +1,30 @@ +package testcases + +// E2E-CLI-098 - KICS scan and ignore references +// should perform the scan successfully and return exit code 0 +// this test sample contains a different query_id +// that is not a UUID, but contains a prefix ('t:', 'p:', or 'a:') + uint64 +func init() { //nolint + testSample := TestCase{ + Name: "should perform a valid scan and return one HIGH result [E2E-CLI-098]", + Args: args{ + Args: []cmdArgs{ + []string{"scan", "-o", "/path/e2e/output", + "--output-name", "E2E_CLI_098_RESULT", + "-q", "\"/path/test/fixtures/new_queryid_validation\"", + "-p", "\"/path/test/fixtures/new_queryid_validation/Dockerfile\"", + "-i", "t:8820143918834007824", + }, + }, + ExpectedResult: []ResultsValidation{ + { + ResultsFile: "E2E_CLI_098_RESULT", + ResultsFormats: []string{"json"}, + }, + }, + }, + WantStatus: []int{50}, + } + + Tests = append(Tests, testSample) +} diff --git a/internal/console/flags/validate.go b/internal/console/flags/validate.go index 275464f6426..765f3900489 100644 --- a/internal/console/flags/validate.go +++ b/internal/console/flags/validate.go @@ -14,8 +14,15 @@ var flagValidationFuncs = flagValidationFuncsMap{ } func isQueryID(id string) bool { - re := regexp.MustCompile(`^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$`) - return re.MatchString(id) + uuidRegex := regexp.MustCompile(`^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$`) + isQueryID := uuidRegex.MatchString(id) + if !isQueryID { + // (t:|p:|a:) matches strings starting with 't:', 'p:', or 'a:' + // (\d{1,20}) ensures the numeric part has 1 to 20 digits (uint64 validation) + cxoneRegex := regexp.MustCompile(`^(t:|p:|a:)(\d{1,20})$`) + isQueryID = cxoneRegex.MatchString(id) + } + return isQueryID } func convertSliceToDummyMap(slice []string) map[string]string { diff --git a/internal/console/flags/validate_test.go b/internal/console/flags/validate_test.go index 1b8f3934b71..8b5d75d7ea1 100644 --- a/internal/console/flags/validate_test.go +++ b/internal/console/flags/validate_test.go @@ -22,6 +22,26 @@ func TestFlags_isQueryID(t *testing.T) { id: "test", expected: false, }, + { + name: "for prefix 't:' should return that query id is valid", + id: "t:12345678901234567890", + expected: true, + }, + { + name: "for prefix 'p:' should return that query id is valid", + id: "p:8820143918834007824", + expected: true, + }, + { + name: "for prefix 'a:' should return that query id is valid", + id: "a:8820143918834007824", + expected: true, + }, + { + name: "should return that query id is invalid because uint exceeds 20 length", + id: "t:123456789012345678901", + expected: false, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/test/fixtures/new_queryid_validation/Dockerfile b/test/fixtures/new_queryid_validation/Dockerfile new file mode 100644 index 00000000000..b348e64cf55 --- /dev/null +++ b/test/fixtures/new_queryid_validation/Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:2.6 +USER root +RUN npm install \ No newline at end of file diff --git a/test/fixtures/new_queryid_validation/metadata.json b/test/fixtures/new_queryid_validation/metadata.json new file mode 100644 index 00000000000..bd63573c633 --- /dev/null +++ b/test/fixtures/new_queryid_validation/metadata.json @@ -0,0 +1,12 @@ +{ + "id": "t:8820143918834007824", + "queryName": "Last User Is 'root'", + "severity": "HIGH", + "category": "Best Practices", + "descriptionText": "Leaving the last user as root can cause security risks. Change to another user after running the commands the need privileges", + "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#user", + "platform": "Dockerfile", + "descriptionID": "f445bd25", + "cwe": "250", + "oldSeverity": "MEDIUM" +} \ No newline at end of file diff --git a/test/fixtures/new_queryid_validation/query.rego b/test/fixtures/new_queryid_validation/query.rego new file mode 100644 index 00000000000..28e2dca5a04 --- /dev/null +++ b/test/fixtures/new_queryid_validation/query.rego @@ -0,0 +1,19 @@ +package Cx + +import data.generic.dockerfile as dockerLib + +CxPolicy[result] { + resource := input.document[i].command[name] + dockerLib.check_multi_stage(name, input.document[i].command) + + userCmd := [x | resource[j].Cmd == "user"; x := resource[j]] + userCmd[minus(count(userCmd), 1)].Value[0] == "root" + + result := { + "documentId": input.document[i].id, + "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, userCmd[minus(count(userCmd), 1)].Original]), + "issueType": "IncorrectValue", + "keyExpectedValue": "Last User shouldn't be root", + "keyActualValue": "Last User is root", + } +} From 5fcb3897efaeebd2c024de61387bbd36a08fa741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 13:28:44 +0000 Subject: [PATCH 02/11] fix e2e --- e2e/fixtures/schemas/result.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/fixtures/schemas/result.json b/e2e/fixtures/schemas/result.json index 80a3cdbd1c5..4d61250ea22 100644 --- a/e2e/fixtures/schemas/result.json +++ b/e2e/fixtures/schemas/result.json @@ -81,7 +81,7 @@ }, "query_id": { "type": "string", - "pattern": "^[a-f0-9]{8}-[a-f0-9]{4}-4{1}[a-f0-9]{3}-[89ab]{1}[a-f0-9]{3}-[a-f0-9]{12}$" + "pattern": "^(?[a-f0-9]{8}-[a-f0-9]{4}-4{1}[a-f0-9]{3}-[89ab]{1}[a-f0-9]{3}-[a-f0-9]{12})|(?(t:|p:|a:)(\\d{1,20}))$" }, "query_url": { "type": "string", From 07afe2f25ed4b89afb92c2d91384c4ca084e7522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 13:29:47 +0000 Subject: [PATCH 03/11] add comments --- .../e2e-cli-098_include_ids_with_new_queryid_validation.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go index c7c0d538191..373ef197f09 100644 --- a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go +++ b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go @@ -13,7 +13,8 @@ func init() { //nolint "--output-name", "E2E_CLI_098_RESULT", "-q", "\"/path/test/fixtures/new_queryid_validation\"", "-p", "\"/path/test/fixtures/new_queryid_validation/Dockerfile\"", - "-i", "t:8820143918834007824", + // QueryID 'a:123' does not exist, however, since the first one does, it should perform the scan successfully + "-i", "t:8820143918834007824,a:123", }, }, ExpectedResult: []ResultsValidation{ From ab6ab4c454f13f3fab431abd1949e73966daf8b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 13:30:45 +0000 Subject: [PATCH 04/11] typo --- .../e2e-cli-098_include_ids_with_new_queryid_validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go index 373ef197f09..da9bad787a4 100644 --- a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go +++ b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go @@ -1,6 +1,6 @@ package testcases -// E2E-CLI-098 - KICS scan and ignore references +// E2E-CLI-098 // should perform the scan successfully and return exit code 0 // this test sample contains a different query_id // that is not a UUID, but contains a prefix ('t:', 'p:', or 'a:') + uint64 From bcfa0ddb0e7037d023259852888564dd67849966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 13:33:25 +0000 Subject: [PATCH 05/11] typo --- .../e2e-cli-098_include_ids_with_new_queryid_validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go index da9bad787a4..dccdf43fe8b 100644 --- a/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go +++ b/e2e/testcases/e2e-cli-098_include_ids_with_new_queryid_validation.go @@ -1,7 +1,7 @@ package testcases // E2E-CLI-098 -// should perform the scan successfully and return exit code 0 +// should perform the scan successfully and return exit code 50 // this test sample contains a different query_id // that is not a UUID, but contains a prefix ('t:', 'p:', or 'a:') + uint64 func init() { //nolint From 365399ad17c095e812c1c9e5ebbc745cb8cc6c0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 14:32:32 +0000 Subject: [PATCH 06/11] Trigger CI From e69b865f16d9f6312e308c67b234141ebe35bfcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Thu, 16 Jan 2025 14:39:03 +0000 Subject: [PATCH 07/11] Trigger CI From 1375fa9966f1a6ceee68b1e94ff97f316d76eb59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Fri, 17 Jan 2025 14:52:19 +0000 Subject: [PATCH 08/11] Trigger CI From 48cd4e4b8e63155f58b3de95a2b08c470b8441bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Fri, 17 Jan 2025 15:53:48 +0000 Subject: [PATCH 09/11] fix codacy --- test/fixtures/new_queryid_validation/Dockerfile | 2 +- test/fixtures/new_queryid_validation/query.rego | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/fixtures/new_queryid_validation/Dockerfile b/test/fixtures/new_queryid_validation/Dockerfile index b348e64cf55..09c9ccaca40 100644 --- a/test/fixtures/new_queryid_validation/Dockerfile +++ b/test/fixtures/new_queryid_validation/Dockerfile @@ -1,3 +1,3 @@ FROM alpine:2.6 -USER root +USER guest RUN npm install \ No newline at end of file diff --git a/test/fixtures/new_queryid_validation/query.rego b/test/fixtures/new_queryid_validation/query.rego index 28e2dca5a04..36e6b8bb9a8 100644 --- a/test/fixtures/new_queryid_validation/query.rego +++ b/test/fixtures/new_queryid_validation/query.rego @@ -7,13 +7,13 @@ CxPolicy[result] { dockerLib.check_multi_stage(name, input.document[i].command) userCmd := [x | resource[j].Cmd == "user"; x := resource[j]] - userCmd[minus(count(userCmd), 1)].Value[0] == "root" + userCmd[minus(count(userCmd), 1)].Value[0] == "guest" result := { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, userCmd[minus(count(userCmd), 1)].Original]), "issueType": "IncorrectValue", - "keyExpectedValue": "Last User shouldn't be root", - "keyActualValue": "Last User is root", + "keyExpectedValue": "Last User shouldn't be guest", + "keyActualValue": "Last User is guest", } } From f44aaa7a4cbb47bfa5f3c7bae8fc1e49bcc49d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Sun, 19 Jan 2025 20:16:04 +0000 Subject: [PATCH 10/11] align with code review; fix typo --- assets/queries/dockerfile/last_user_is_root/metadata.json | 2 +- .../67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md | 2 +- test/fixtures/new_queryid_validation/metadata.json | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/queries/dockerfile/last_user_is_root/metadata.json b/assets/queries/dockerfile/last_user_is_root/metadata.json index ed4a7e9198d..3d5406d91e9 100644 --- a/assets/queries/dockerfile/last_user_is_root/metadata.json +++ b/assets/queries/dockerfile/last_user_is_root/metadata.json @@ -3,7 +3,7 @@ "queryName": "Last User Is 'root'", "severity": "HIGH", "category": "Best Practices", - "descriptionText": "Leaving the last user as root can cause security risks. Change to another user after running the commands the need privileges", + "descriptionText": "Leaving the last user as root can cause security risks. Change to another user after running the commands that need privileges", "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#user", "platform": "Dockerfile", "descriptionID": "f445bd25", diff --git a/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md b/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md index eee035c2489..4109c2b551b 100644 --- a/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md +++ b/docs/queries/dockerfile-queries/67fd0c4a-68cf-46d7-8c41-bc9fba7e40ae.md @@ -24,7 +24,7 @@ hide: - **URL:** [Github](https://github.com/Checkmarx/kics/tree/master/assets/queries/dockerfile/last_user_is_root) ### Description -Leaving the last user as root can cause security risks. Change to another user after running the commands the need privileges
+Leaving the last user as root can cause security risks. Change to another user after running the commands that need privileges
[Documentation](https://docs.docker.com/engine/reference/builder/#user) ### Code samples diff --git a/test/fixtures/new_queryid_validation/metadata.json b/test/fixtures/new_queryid_validation/metadata.json index bd63573c633..622648602b0 100644 --- a/test/fixtures/new_queryid_validation/metadata.json +++ b/test/fixtures/new_queryid_validation/metadata.json @@ -1,9 +1,9 @@ { "id": "t:8820143918834007824", - "queryName": "Last User Is 'root'", + "queryName": "Last User Is 'guest'", "severity": "HIGH", "category": "Best Practices", - "descriptionText": "Leaving the last user as root can cause security risks. Change to another user after running the commands the need privileges", + "descriptionText": "Leaving the last user as guest can cause security risks. Change to another user after running the commands that need privileges", "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#user", "platform": "Dockerfile", "descriptionID": "f445bd25", From 619157591e101ef52e98e409b90cb70bddf2d472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Ara=C3=BAjo=20Gomes?= Date: Sun, 19 Jan 2025 20:25:34 +0000 Subject: [PATCH 11/11] fix test --- assets/queries/dockerfile/last_user_is_root/metadata.json | 1 + 1 file changed, 1 insertion(+) diff --git a/assets/queries/dockerfile/last_user_is_root/metadata.json b/assets/queries/dockerfile/last_user_is_root/metadata.json index 3d5406d91e9..9826eaefbf7 100644 --- a/assets/queries/dockerfile/last_user_is_root/metadata.json +++ b/assets/queries/dockerfile/last_user_is_root/metadata.json @@ -7,6 +7,7 @@ "descriptionUrl": "https://docs.docker.com/engine/reference/builder/#user", "platform": "Dockerfile", "descriptionID": "f445bd25", + "cloudProvider": "common", "cwe": "250", "oldSeverity": "MEDIUM" } \ No newline at end of file