diff --git a/rules/require_cap_drop_all.go b/rules/require_cap_drop_all.go index 8802969..ec54e53 100644 --- a/rules/require_cap_drop_all.go +++ b/rules/require_cap_drop_all.go @@ -56,7 +56,7 @@ func checkRequireCapDropAll(_ *linter.Context, node *linter.Node) []linter.Findi return nil } - if arr, ok := arrayMember(obj, "runArgs"); ok { + for arr := range arrayMembers(obj, "runArgs") { if runArgsFindFlagValue(arr, "--cap-drop", func(s string) bool { return s == "ALL" }) != nil { return nil } diff --git a/rules/require_cap_drop_all_test.go b/rules/require_cap_drop_all_test.go index 29871f4..91a1b3c 100644 --- a/rules/require_cap_drop_all_test.go +++ b/rules/require_cap_drop_all_test.go @@ -34,6 +34,15 @@ func TestRequireCapDropAll(t *testing.T) { Message: `"ALL" is not set via "runArgs", leaving the container with its default Linux capabilities`}, }}, + // duplicate members: a JSON parser keeps only the last copy, so every copy is read + {"runArgs duplicated, set by the last", `{"runArgs": ["--init"], "runArgs": ["--cap-drop=ALL"]}`, nil}, + {"runArgs duplicated, set by the first", `{"runArgs": ["--cap-drop=ALL"], "runArgs": ["--init"]}`, nil}, + {"runArgs duplicated, one copy not an array", `{"runArgs": "--cap-drop=ALL", "runArgs": ["--cap-drop", "ALL"]}`, nil}, + {"runArgs duplicated, set by neither", `{"runArgs": ["--init"], "runArgs": ["--cap-add=SYS_PTRACE"]}`, []linter.Issue{ + {Path: "devcontainer.json", Line: 1, Col: 1, RuleID: "require-cap-drop-all", + Message: `"ALL" is not set via "runArgs", leaving the container with its default Linux capabilities`}, + }}, + // document root {"root is an array, not an object", `[]`, nil}, } diff --git a/rules/require_no_new_privileges.go b/rules/require_no_new_privileges.go index 6908c91..0068974 100644 --- a/rules/require_no_new_privileges.go +++ b/rules/require_no_new_privileges.go @@ -55,7 +55,7 @@ func checkRequireNoNewPrivileges(_ *linter.Context, node *linter.Node) []linter. if stringArrayContains(obj, "securityOpt", securityOptIsNoNewPrivileges) { return nil } - if arr, ok := arrayMember(obj, "runArgs"); ok { + for arr := range arrayMembers(obj, "runArgs") { if runArgsFindFlagValue(arr, "--security-opt", securityOptIsNoNewPrivileges) != nil { return nil } diff --git a/rules/require_no_new_privileges_test.go b/rules/require_no_new_privileges_test.go index 454619f..39061c3 100644 --- a/rules/require_no_new_privileges_test.go +++ b/rules/require_no_new_privileges_test.go @@ -53,6 +53,22 @@ func TestRequireNoNewPrivileges(t *testing.T) { Message: `"no-new-privileges" is not set via "securityOpt" or "runArgs", allowing container processes to gain additional privileges`}, }}, + // duplicate members: a JSON parser keeps only the last copy, so every copy is read + {"securityOpt duplicated, set by the last", `{"securityOpt": [], "securityOpt": ["no-new-privileges"]}`, nil}, + {"securityOpt duplicated, set by the first", `{"securityOpt": ["no-new-privileges"], "securityOpt": []}`, nil}, + {"securityOpt duplicated, one copy not an array", `{"securityOpt": "no-new-privileges", "securityOpt": ["no-new-privileges"]}`, nil}, + {"securityOpt duplicated, set by neither", `{"securityOpt": [], "securityOpt": ["seccomp=unconfined"]}`, []linter.Issue{ + {Path: "devcontainer.json", Line: 1, Col: 1, RuleID: "require-no-new-privileges", + Message: `"no-new-privileges" is not set via "securityOpt" or "runArgs", allowing container processes to gain additional privileges`}, + }}, + {"runArgs duplicated, set by the last", `{"runArgs": ["--init"], "runArgs": ["--security-opt=no-new-privileges"]}`, nil}, + {"runArgs duplicated, set by the first", `{"runArgs": ["--security-opt=no-new-privileges"], "runArgs": ["--init"]}`, nil}, + {"runArgs duplicated, one copy not an array", `{"runArgs": "--security-opt=no-new-privileges", "runArgs": ["--security-opt", "no-new-privileges"]}`, nil}, + {"runArgs duplicated, set by neither", `{"runArgs": ["--init"], "runArgs": ["--cap-drop=ALL"]}`, []linter.Issue{ + {Path: "devcontainer.json", Line: 1, Col: 1, RuleID: "require-no-new-privileges", + Message: `"no-new-privileges" is not set via "securityOpt" or "runArgs", allowing container processes to gain additional privileges`}, + }}, + // document root {"root is an array, not an object", `[]`, nil}, } diff --git a/rules/util.go b/rules/util.go index e74c092..0590d27 100644 --- a/rules/util.go +++ b/rules/util.go @@ -1,6 +1,7 @@ package rules import ( + "iter" "strings" "github.com/bare-devcontainer/decolint/linter" @@ -27,42 +28,40 @@ func memberNamed(obj *hujson.Object, name string) *hujson.ObjectMember { return nil } -// stringArrayContains reports whether obj has a member named name whose value is an array -// containing a string element for which match returns true. It returns false if obj has no such -// member or the member's value is not an array. +// stringArrayContains reports whether any array member of obj named name (see [arrayMembers]) +// contains a string element for which match returns true. func stringArrayContains(obj *hujson.Object, name string, match func(string) bool) bool { - for _, m := range obj.Members { - nameLit, ok := m.Name.Value.(hujson.Literal) - if !ok || nameLit.String() != name { - continue - } - arr, ok := m.Value.Value.(*hujson.Array) - if !ok { - return false - } + for arr := range arrayMembers(obj, name) { for _, elem := range arr.Elements { lit, ok := elem.Value.(hujson.Literal) if ok && lit.Kind() == '"' && match(lit.String()) { return true } } - return false } return false } -// arrayMember returns obj's member named name as an array. ok is false if obj has no such member or -// the member's value is not an array. -func arrayMember(obj *hujson.Object, name string) (arr *hujson.Array, ok bool) { - for _, m := range obj.Members { - nameLit, isLit := m.Name.Value.(hujson.Literal) - if !isLit || nameLit.String() != name { - continue +// arrayMembers yields the value of every member of obj named name that is an array, skipping the +// members whose value is not one. A malformed object may repeat a name: JSON parsers keep a single +// copy — the last one — but decolint reads them all rather than report as unset something the +// document plainly sets. +func arrayMembers(obj *hujson.Object, name string) iter.Seq[*hujson.Array] { + return func(yield func(*hujson.Array) bool) { + for _, m := range obj.Members { + nameLit, ok := m.Name.Value.(hujson.Literal) + if !ok || nameLit.String() != name { + continue + } + arr, ok := m.Value.Value.(*hujson.Array) + if !ok { + continue + } + if !yield(arr) { + return + } } - arr, ok = m.Value.Value.(*hujson.Array) - return arr, ok } - return nil, false } // runArgsFindFlagValue scans arr, a "runArgs" array, for an entry that sets flag to a value accepted