fix(rules): read securityOpt entries and capabilities the way Docker does

dockerargs/capability.go

@@ -0,0 +1,17 @@
+package dockerargs
+
+import "strings"
+
+// AllCapabilities is the pseudo-capability standing for every Linux capability.
+const AllCapabilities = "ALL"
+
+// Capability returns name — a capability written in "capAdd" or given to "--cap-add"/"--cap-drop" —
+// as Docker matches it: upper-cased and prefixed with "CAP_", except for [AllCapabilities], which
+// takes no prefix.
+func Capability(name string) string {
+	c := strings.ToUpper(name)
+	if c == AllCapabilities || strings.HasPrefix(c, "CAP_") {
+		return c
+	}
+	return "CAP_" + c
+}

dockerargs/capability_test.go

@@ -0,0 +1,32 @@
+package dockerargs
+
+import "testing"
+
+func TestCapability(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		want string
+	}{
+		{"SYS_PTRACE", "CAP_SYS_PTRACE"},
+		{"sys_ptrace", "CAP_SYS_PTRACE"},
+		{"CAP_SYS_PTRACE", "CAP_SYS_PTRACE"},
+		{"cap_sys_ptrace", "CAP_SYS_PTRACE"},
+		{"", "CAP_"},
+
+		{"ALL", "ALL"},
+		{"all", "ALL"},
+		{"All", "ALL"},
+		// "ALL" is a name of its own, so prefixing it names an ordinary — and unknown — capability.
+		{"CAP_ALL", "CAP_ALL"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := Capability(tt.name); got != tt.want {
+				t.Errorf("Capability(%q) = %q, want %q", tt.name, got, tt.want)
+			}
+		})
+	}
+}

dockerargs/dockerargs.go

@@ -1,7 +1,11 @@
-// Package dockerargs reads a devcontainer.json "runArgs" array as what it becomes: the argv of the
-// "docker run" command the devcontainer tooling builds. It is the single place that knows where a
-// flag's value can be written, so a rule only has to know the values it cares about — a capability
-// name, a mount, a "securityOpt" entry — and never which entry of the array holds one.
+// Package dockerargs reads the parts of a devcontainer.json that Docker, rather than the
+// devcontainer tooling, gives meaning to:
+//
+//   - "runArgs", which becomes the argv of the "docker run" command the tooling builds. [Parse] is
+//     the single place that knows where a flag's value can be written, so a rule only has to know
+//     the values it cares about and never which entry of the array holds one.
+//   - the values themselves, whose syntax is Docker's wherever they are written: a "securityOpt"
+//     entry ([ParseSecurityOpt]), a capability name ([Capability]), a boolean ([IsTrue]).
 package dockerargs
 
 import (

dockerargs/securityopt.go

@@ -0,0 +1,50 @@
+package dockerargs
+
+import "strings"
+
+// Seccomp profile names that stand for a built-in profile rather than for the path of one to load.
+// Docker matches them exactly, so a differently cased spelling names a file.
+const (
+	// SeccompProfileDefault is the runtime's own default profile — the one a container that is
+	// given no seccomp option at all runs under.
+	SeccompProfileDefault = "builtin"
+	// SeccompProfileUnconfined turns syscall filtering off.
+	SeccompProfileUnconfined = "unconfined"
+)
+
+// noNewPrivileges is the one security option that may be written without a value.
+const noNewPrivileges = "no-new-privileges"
+
+// SecurityOpt is one "securityOpt" entry, equivalently one value given to "--security-opt".
+type SecurityOpt struct {
+	// Key names the option, e.g. "seccomp" or "no-new-privileges". Docker matches it
+	// case-sensitively, so it is kept as written.
+	Key string
+	// Value is what the entry gives the option, which for a bare "no-new-privileges" is the "true"
+	// it stands for on its own.
+	Value string
+}
+
+// ParseSecurityOpt reads s the way Docker splits it:
+//
+//   - the key and the value are separated by the first "=", or, in an entry holding none, by the
+//     first ":";
+//   - "no-new-privileges" is the one option that may be written bare, standing for "true".
+//
+// ok is false for an entry Docker rejects outright, which is any other one left without a value.
+func ParseSecurityOpt(s string) (opt SecurityOpt, ok bool) {
+	key, value, split := strings.Cut(s, "=")
+	if !split && key != noNewPrivileges {
+		key, value, split = strings.Cut(s, ":")
+	}
+	if key == noNewPrivileges {
+		if !split {
+			value = "true"
+		}
+		return SecurityOpt{Key: key, Value: value}, true
+	}
+	if !split || value == "" {
+		return SecurityOpt{}, false
+	}
+	return SecurityOpt{Key: key, Value: value}, true
+}

dockerargs/securityopt_test.go

@@ -0,0 +1,55 @@
+package dockerargs
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestParseSecurityOpt(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		s     string
+		want  SecurityOpt
+		wantO bool
+	}{
+		{"seccomp=unconfined", SecurityOpt{Key: "seccomp", Value: "unconfined"}, true},
+		{"seccomp=builtin", SecurityOpt{Key: "seccomp", Value: "builtin"}, true},
+		{"seccomp=./profile.json", SecurityOpt{Key: "seccomp", Value: "./profile.json"}, true},
+		{"apparmor=unconfined", SecurityOpt{Key: "apparmor", Value: "unconfined"}, true},
+
+		// A ":" separates the key from the value only in an entry holding no "=".
+		{"seccomp:unconfined", SecurityOpt{Key: "seccomp", Value: "unconfined"}, true},
+		{"seccomp:builtin", SecurityOpt{Key: "seccomp", Value: "builtin"}, true},
+		{"label=user:USER", SecurityOpt{Key: "label", Value: "user:USER"}, true},
+
+		// The key is matched case-sensitively, so it is kept as written.
+		{"SECCOMP=unconfined", SecurityOpt{Key: "SECCOMP", Value: "unconfined"}, true},
+		{"seccomp=BUILTIN", SecurityOpt{Key: "seccomp", Value: "BUILTIN"}, true},
+
+		{"no-new-privileges", SecurityOpt{Key: "no-new-privileges", Value: "true"}, true},
+		{"no-new-privileges=true", SecurityOpt{Key: "no-new-privileges", Value: "true"}, true},
+		{"no-new-privileges:true", SecurityOpt{Key: "no-new-privileges", Value: "true"}, true},
+		{"no-new-privileges=1", SecurityOpt{Key: "no-new-privileges", Value: "1"}, true},
+		{"no-new-privileges=false", SecurityOpt{Key: "no-new-privileges", Value: "false"}, true},
+		{"no-new-privileges=", SecurityOpt{Key: "no-new-privileges"}, true},
+		{"no-new-privileges:", SecurityOpt{Key: "no-new-privileges"}, true},
+		{"NO-NEW-PRIVILEGES", SecurityOpt{}, false},
+
+		// Every other option needs a value.
+		{"seccomp", SecurityOpt{}, false},
+		{"seccomp=", SecurityOpt{}, false},
+		{"seccomp:", SecurityOpt{}, false},
+		{"", SecurityOpt{}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.s, func(t *testing.T) {
+			t.Parallel()
+			got, gotOK := ParseSecurityOpt(tt.s)
+			if diff := cmp.Diff(tt.want, got); diff != "" || gotOK != tt.wantO {
+				t.Errorf("ParseSecurityOpt(%q) ok = %v, want %v; mismatch (-want +got):\n%s", tt.s, gotOK, tt.wantO, diff)
+			}
+		})
+	}
+}

rules/no_cap_add_all.go

@@ -63,7 +63,7 @@ func checkNoCapAddAll(ctx *linter.Context, node *linter.Node) []linter.Finding {
 	}
 
 	lit, ok := node.Value.Value.(hujson.Literal)
-	if !ok || lit.Kind() != '"' || lit.String() != "ALL" {
+	if !ok || lit.Kind() != '"' || !isAllCapability(lit.String()) {
 		return nil
 	}
 	return []linter.Finding{{

rules/no_cap_add_all_test.go

@@ -21,6 +21,12 @@ func TestNoCapAddAll(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 27, RuleID: "no-cap-add-all",
 				Message: `"capAdd" contains "ALL", granting every Linux capability to the container`},
 		}},
+		{"capAdd with lower-case all", `{"capAdd": ["all"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-cap-add-all",
+				Message: `"capAdd" contains "ALL", granting every Linux capability to the container`},
+		}},
+		// "ALL" takes no "CAP_" prefix, so a prefixed one is an ordinary capability name.
+		{"capAdd with CAP_ALL", `{"capAdd": ["CAP_ALL"]}`, nil},
 		{"no runArgs", `{"runArgs": ["--init"]}`, nil},
 		{"runArgs without cap-add=ALL", `{"runArgs": ["--init", "--cap-add=SYS_PTRACE"]}`, nil},
 		{"runArgs with cap-add=ALL", `{"runArgs": ["--init", "--cap-add=ALL"]}`, []linter.Issue{
@@ -70,6 +76,10 @@ func TestNoCapAddAll_Feature(t *testing.T) {
 			{Path: "devcontainer-feature.json", Line: 1, Col: 27, RuleID: "no-cap-add-all",
 				Message: `"capAdd" contains "ALL", granting every Linux capability to the container`},
 		}},
+		{"capAdd with lower-case all", `{"id": "test", "capAdd": ["all"]}`, []linter.Issue{
+			{Path: "devcontainer-feature.json", Line: 1, Col: 27, RuleID: "no-cap-add-all",
+				Message: `"capAdd" contains "ALL", granting every Linux capability to the container`},
+		}},
 		// "runArgs" has no meaning in a Feature, so it's not flagged there.
 		{"runArgs with cap-add=ALL is ignored", `{"id": "test", "runArgs": ["--cap-add=ALL"]}`, nil},
 	}

rules/no_seccomp_override.go

@@ -1,8 +1,7 @@
 package rules
 
 import (
-	"strings"
-
+	"github.com/bare-devcontainer/decolint/dockerargs"
 	"github.com/bare-devcontainer/decolint/linter"
 	"github.com/tailscale/hujson"
 )
@@ -79,8 +78,10 @@ func checkNoSeccompOverride(ctx *linter.Context, node *linter.Node) []linter.Fin
 	}}
 }
 
-// securityOptOverridesSeccomp reports whether s, a single "securityOpt" entry, points seccomp at a
-// profile of its own.
+// securityOptOverridesSeccomp reports whether s, a single "securityOpt" entry, points seccomp at
+// anything other than the runtime's own default profile. Naming that default explicitly leaves the
+// container exactly where it started, so it is not an override.
 func securityOptOverridesSeccomp(s string) bool {
-	return strings.HasPrefix(s, "seccomp=")
+	profile, ok := securityOptSeccompProfile(s)
+	return ok && profile != dockerargs.SeccompProfileDefault
 }

rules/no_seccomp_override_test.go

@@ -28,6 +28,20 @@ func TestNoSeccompOverride(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-seccomp-override",
 				Message: `"securityOpt" overrides the default seccomp profile`},
 		}},
+		{"securityOpt with custom seccomp profile separated by a colon", `{"securityOpt": ["seccomp:/path/to/profile.json"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-seccomp-override",
+				Message: `"securityOpt" overrides the default seccomp profile`},
+		}},
+		// "builtin" names the runtime's own profile, the one an unset "seccomp" already selects.
+		{"securityOpt seccomp builtin", `{"securityOpt": ["seccomp=builtin"]}`, nil},
+		{"securityOpt seccomp builtin separated by a colon", `{"securityOpt": ["seccomp:builtin"]}`, nil},
+		{"securityOpt seccomp builtin upper-cased names a file", `{"securityOpt": ["seccomp=BUILTIN"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-seccomp-override",
+				Message: `"securityOpt" overrides the default seccomp profile`},
+		}},
+		{"securityOpt seccomp key upper-cased", `{"securityOpt": ["SECCOMP=/path/to/profile.json"]}`, nil},
+		{"securityOpt seccomp without a value", `{"securityOpt": ["seccomp"]}`, nil},
+		{"securityOpt seccomp with an empty value", `{"securityOpt": ["seccomp="]}`, nil},
 
 		// "runArgs"
 		{"runArgs without security-opt", `{"runArgs": ["--init", "--cap-add=SYS_PTRACE"]}`, nil},
@@ -46,6 +60,7 @@ func TestNoSeccompOverride(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 32, RuleID: "no-seccomp-override",
 				Message: `"runArgs" overrides the default seccomp profile via "--security-opt"`},
 		}},
+		{"runArgs seccomp builtin", `{"runArgs": ["--security-opt", "seccomp=builtin"]}`, nil},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

rules/no_seccomp_unconfined.go

@@ -1,6 +1,7 @@
 package rules
 
 import (
+	"github.com/bare-devcontainer/decolint/dockerargs"
 	"github.com/bare-devcontainer/decolint/linter"
 	"github.com/tailscale/hujson"
 )
@@ -52,7 +53,7 @@ func checkNoSeccompUnconfined(ctx *linter.Context, node *linter.Node) []linter.F
 		if !ok || !runArgsApplicable(ctx) {
 			return nil
 		}
-		v := runArgsFindFlagValue(arr, "security-opt", func(s string) bool { return s == "seccomp=unconfined" })
+		v := runArgsFindFlagValue(arr, "security-opt", securityOptDisablesSeccomp)
 		if v == nil {
 			return nil
 		}
@@ -63,11 +64,18 @@ func checkNoSeccompUnconfined(ctx *linter.Context, node *linter.Node) []linter.F
 	}
 
 	lit, ok := node.Value.Value.(hujson.Literal)
-	if !ok || lit.Kind() != '"' || lit.String() != "seccomp=unconfined" {
+	if !ok || lit.Kind() != '"' || !securityOptDisablesSeccomp(lit.String()) {
 		return nil
 	}
 	return []linter.Finding{{
 		Message: `"securityOpt" contains "seccomp=unconfined", disabling the container's syscall filtering`,
 		Offset:  node.Value.StartOffset,
 	}}
 }
+
+// securityOptDisablesSeccomp reports whether s, a single "securityOpt" entry, turns syscall
+// filtering off.
+func securityOptDisablesSeccomp(s string) bool {
+	profile, ok := securityOptSeccompProfile(s)
+	return ok && profile == dockerargs.SeccompProfileUnconfined
+}

rules/no_seccomp_unconfined_test.go

@@ -24,6 +24,14 @@ func TestNoSeccompUnconfined(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-seccomp-unconfined",
 				Message: `"securityOpt" contains "seccomp=unconfined", disabling the container's syscall filtering`},
 		}},
+		{"securityOpt seccomp unconfined separated by a colon", `{"securityOpt": ["seccomp:unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-seccomp-unconfined",
+				Message: `"securityOpt" contains "seccomp=unconfined", disabling the container's syscall filtering`},
+		}},
+		{"securityOpt seccomp builtin", `{"securityOpt": ["seccomp=builtin"]}`, nil},
+		{"securityOpt seccomp key upper-cased", `{"securityOpt": ["SECCOMP=unconfined"]}`, nil},
+		{"securityOpt seccomp profile upper-cased", `{"securityOpt": ["seccomp=UNCONFINED"]}`, nil},
+		{"securityOpt seccomp without a value", `{"securityOpt": ["seccomp"]}`, nil},
 
 		// "runArgs"
 		{"runArgs without security-opt", `{"runArgs": ["--init", "--cap-add=SYS_PTRACE"]}`, nil},
@@ -33,6 +41,11 @@ func TestNoSeccompUnconfined(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 32, RuleID: "no-seccomp-unconfined",
 				Message: `"runArgs" contains "--security-opt seccomp=unconfined", disabling the container's syscall filtering`},
 		}},
+		{"runArgs seccomp unconfined separated by a colon", `{"runArgs": ["--security-opt", "seccomp:unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 32, RuleID: "no-seccomp-unconfined",
+				Message: `"runArgs" contains "--security-opt seccomp=unconfined", disabling the container's syscall filtering`},
+		}},
+		{"runArgs seccomp builtin", `{"runArgs": ["--security-opt", "seccomp=builtin"]}`, nil},
 		{"runArgs security-opt consumed as another flag's value", `{"runArgs": ["--label", "--security-opt=seccomp=unconfined"]}`, nil},
 		{"runArgs bare seccomp entry names no flag", `{"runArgs": ["seccomp=unconfined"]}`, nil},
 		{"runArgs seccomp unconfined combined", `{"runArgs": ["--security-opt=seccomp=unconfined"]}`, []linter.Issue{

rules/require_cap_drop_all_test.go

@@ -22,6 +22,11 @@ func TestRequireCapDropAll(t *testing.T) {
 		{"runArgs with cap-drop=ALL", `{"runArgs": ["--cap-drop=ALL"]}`, nil},
 		{"runArgs with cap-drop ALL two tokens", `{"runArgs": ["--cap-drop", "ALL"]}`, nil},
 		{"runArgs with lower-case all", `{"runArgs": ["--cap-drop=all"]}`, nil},
+		// "ALL" takes no "CAP_" prefix, so a prefixed one is an ordinary capability name.
+		{"runArgs with cap-drop=CAP_ALL", `{"runArgs": ["--cap-drop=CAP_ALL"]}`, []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`},
+		}},
 		{"runArgs with cap-drop consumed as another flag's value", `{"runArgs": ["--label", "--cap-drop=ALL"]}`, []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`},

rules/require_no_new_privileges.go

@@ -1,6 +1,7 @@
 package rules
 
 import (
+	"github.com/bare-devcontainer/decolint/dockerargs"
 	"github.com/bare-devcontainer/decolint/linter"
 	"github.com/tailscale/hujson"
 )
@@ -67,13 +68,10 @@ func checkRequireNoNewPrivileges(_ *linter.Context, node *linter.Node) []linter.
 	}}
 }
 
-// securityOptIsNoNewPrivileges reports whether s, a single "securityOpt" entry, sets
-// "no-new-privileges". Docker treats the bare keyword as well as an explicit "=true" or ":true"
-// value as enabling it; "=false" or ":false" leaves it disabled.
+// securityOptIsNoNewPrivileges reports whether s, a single "securityOpt" entry, turns on
+// "no-new-privileges". Its value is a boolean, read as [dockerargs.IsTrue] describes, and the
+// option is the one that may also be written bare.
 func securityOptIsNoNewPrivileges(s string) bool {
-	switch s {
-	case "no-new-privileges", "no-new-privileges=true", "no-new-privileges:true":
-		return true
-	}
-	return false
+	opt, ok := dockerargs.ParseSecurityOpt(s)
+	return ok && opt.Key == "no-new-privileges" && dockerargs.IsTrue(opt.Value)
 }

rules/require_no_new_privileges_test.go

@@ -24,10 +24,25 @@ func TestRequireNoNewPrivileges(t *testing.T) {
 		{"securityOpt bare", `{"securityOpt": ["no-new-privileges"]}`, nil},
 		{"securityOpt equals true", `{"securityOpt": ["no-new-privileges=true"]}`, nil},
 		{"securityOpt colon true", `{"securityOpt": ["no-new-privileges:true"]}`, nil},
+		// The value is a boolean, not the literal "true": Docker reads it with strconv.ParseBool.
+		{"securityOpt equals 1", `{"securityOpt": ["no-new-privileges=1"]}`, nil},
+		{"securityOpt colon T", `{"securityOpt": ["no-new-privileges:T"]}`, nil},
 		{"securityOpt equals false", `{"securityOpt": ["no-new-privileges=false"]}`, []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`},
 		}},
+		{"securityOpt equals 0", `{"securityOpt": ["no-new-privileges=0"]}`, []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`},
+		}},
+		{"securityOpt key upper-cased", `{"securityOpt": ["NO-NEW-PRIVILEGES=true"]}`, []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`},
+		}},
+		{"securityOpt key upper-cased and bare", `{"securityOpt": ["NO-NEW-PRIVILEGES"]}`, []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`},
+		}},
 		{"securityOpt with unrelated option", `{"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`},
@@ -44,6 +59,7 @@ func TestRequireNoNewPrivileges(t *testing.T) {
 		}},
 		{"runArgs two tokens", `{"runArgs": ["--security-opt", "no-new-privileges"]}`, nil},
 		{"runArgs combined", `{"runArgs": ["--security-opt=no-new-privileges=true"]}`, nil},
+		{"runArgs combined 1", `{"runArgs": ["--security-opt=no-new-privileges=1"]}`, nil},
 		{"runArgs security-opt consumed as another flag's value", `{"runArgs": ["--label", "--security-opt=no-new-privileges"]}`, []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`},