feat(rules): add container privilege and isolation rules

README.md

@@ -154,7 +154,7 @@ runs without configuration; the rest are `off` until you enable them:
 | Category | Default | Rules |
 | --- | --- | --- |
 | [`correctness`](https://bare-devcontainer.github.io/decolint/rules/#correctness) | `error` | 13 |
-| [`security`](https://bare-devcontainer.github.io/decolint/rules/#security) | `off` | 8 |
+| [`security`](https://bare-devcontainer.github.io/decolint/rules/#security) | `off` | 11 |
 | [`reproducibility`](https://bare-devcontainer.github.io/decolint/rules/#reproducibility) | `off` | 4 |
 | [`style`](https://bare-devcontainer.github.io/decolint/rules/#style) | `off` | 2 |
 <!-- /decolint:categories -->

dockerargs/network.go

@@ -0,0 +1,51 @@
+package dockerargs
+
+import (
+	"encoding/csv"
+	"regexp"
+	"strings"
+)
+
+// NetworkHost is the network mode that puts the container in the host's network namespace.
+const NetworkHost = "host"
+
+// networkFieldList matches a "--network" value docker/cli reads as a field list rather than as a
+// network name. It is docker/cli's own regexp, applied unanchored as it applies it, so one unspaced
+// "key=value" anywhere makes the whole value a list: "name = host" names a network, where
+// "alias=web, name = host " is a list whose "name" field holds one.
+var networkFieldList = regexp.MustCompile(`\w+=\w+(,\w+=\w+)*`)
+
+// NetworkTarget returns the network a "--network" or "--net" value names. Docker takes either the
+// network itself ("host") or a comma-separated field list in which "name" holds it
+// ("name=host,alias=web"), the fields being a CSV record as a mount entry's are. It lower-cases a
+// field's key and value and trims the space around them.
+//
+// A field list Docker rejects for a reason of its own — an unknown field key, an address that does
+// not parse, a field written without a key or without a value — is read here for whatever its
+// "name" field holds rather than treated as naming nothing. The value already fails to start the
+// container, and reading it lets a rule name the network the author asked for instead of falling
+// silent on it, as [IsTrue] reads a boolean flag Docker would reject.
+//
+// The result is "" for a list naming no network at all and for one the CSV reader cannot read,
+// neither of which says what was meant. A list naming several yields the last, which is the one
+// Docker is left holding.
+func NetworkTarget(value string) string {
+	if !networkFieldList.MatchString(value) {
+		return value
+	}
+	fields, err := csv.NewReader(strings.NewReader(value)).Read()
+	if err != nil {
+		return ""
+	}
+	target := ""
+	for _, field := range fields {
+		key, name, ok := strings.Cut(field, "=")
+		if !ok {
+			continue
+		}
+		if strings.ToLower(strings.TrimSpace(key)) == "name" {
+			target = strings.ToLower(strings.TrimSpace(name))
+		}
+	}
+	return target
+}

dockerargs/network_test.go

@@ -0,0 +1,47 @@
+package dockerargs
+
+import "testing"
+
+func TestNetworkTarget(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		value, want string
+	}{
+		{`host`, "host"},
+		{`devnet`, "devnet"},
+		// Docker compares the network mode exactly, so a differently cased spelling is a network name.
+		{`HOST`, "HOST"},
+		{`name=host`, "host"},
+		{`NAME=HOST`, "host"},
+		{`alias=web,name=host`, "host"},
+		{`alias=web, name = host `, "host"},
+		{`name=devnet,name=host`, "host"},
+		{`name=host,name=devnet`, "devnet"},
+		{`alias=host`, ""},
+		// Docker rejects each of these outright, so nothing runs; the value still says which network
+		// was asked for, and saying so beats going quiet on it.
+		{`name=host,web`, "host"},
+		{`name=host,foo=bar`, "host"},
+		{`name=host,ip=notanip`, "host"},
+		{`name=host,=x`, "host"},
+		// The field-list reading is chosen by an unspaced "key=value" appearing somewhere, so a value
+		// whose every "=" has space around it is a network name that happens to contain one.
+		{`name = host`, "name = host"},
+		{`name =host`, "name =host"},
+		// The fields are a CSV record, so a field may be quoted and a record the reader rejects names
+		// no network.
+		{`"name=host"`, "host"},
+		{`"name=host",alias=web`, "host"},
+		{` name=host `, "host"},
+		{`name=host,"alias=web`, ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.value, func(t *testing.T) {
+			t.Parallel()
+			if got := NetworkTarget(tt.value); got != tt.want {
+				t.Errorf("NetworkTarget(%q) = %q, want %q", tt.value, got, tt.want)
+			}
+		})
+	}
+}

dockerargs/securityopt.go

@@ -12,6 +12,10 @@ const (
 	SeccompProfileUnconfined = "unconfined"
 )
 
+// AppArmorProfileUnconfined removes the container's AppArmor profile, as
+// [SeccompProfileUnconfined] removes its seccomp one.
+const AppArmorProfileUnconfined = "unconfined"
+
 // noNewPrivileges is the one security option that may be written without a value.
 const noNewPrivileges = "no-new-privileges"
 

rules/no_apparmor_unconfined.go

@@ -0,0 +1,80 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/bare-devcontainer/decolint/dockerargs"
+	"github.com/bare-devcontainer/decolint/linter"
+	"github.com/tailscale/hujson"
+)
+
+// NoApparmorUnconfined reports a devcontainer.json or devcontainer-feature.json that disables
+// AppArmor confinement, either via the "securityOpt" property or, in a devcontainer.json, a
+// "--security-opt apparmor=unconfined" entry in "runArgs". It is the AppArmor counterpart of
+// [NoSeccompUnconfined]: both remove a mandatory confinement layer the runtime applies by default.
+var NoApparmorUnconfined = &linter.Rule{
+	ID:          "no-apparmor-unconfined",
+	Description: `disallow disabling AppArmor confinement via a devcontainer.json's or Feature's "securityOpt" property, or a "--security-opt apparmor=unconfined" entry in a devcontainer.json's "runArgs"`,
+	LongDescription: `A container runtime applies its own AppArmor profile ("docker-default" for Docker) to every container on
+a host that has AppArmor enabled, restricting what the container may do to the host's filesystem,
+capabilities, and network. "apparmor=unconfined" removes that profile outright, so the only thing left
+between a process in the container and the host is the discretionary access control the container's user
+is already subject to. The setting is usually copied from instructions for running nested containers or a
+debugger, both of which have narrower settings that work.`,
+	References: []string{
+		`https://containers.dev/implementors/json_reference/#general-properties`,
+		`https://docs.docker.com/engine/security/apparmor/`,
+	},
+	Category:  linter.CategorySecurity,
+	FileTypes: []linter.FileType{linter.Devcontainer, linter.Feature},
+	Paths:     []string{"/securityOpt/*", "/runArgs/--security-opt"},
+	Example: linter.Example{
+		Bad: linter.Snippet{
+			Files: []linter.ExampleFile{
+				{Path: `devcontainer.json`, Content: `{
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
+  "securityOpt": ["apparmor=unconfined"]
+}
+`},
+			},
+		},
+		Good: linter.Snippet{
+			Files: []linter.ExampleFile{
+				{Path: `devcontainer.json`, Content: `{
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
+  "capAdd": ["SYS_PTRACE"]
+}
+`},
+			},
+		},
+	},
+	Check: checkNoApparmorUnconfined,
+}
+
+func checkNoApparmorUnconfined(_ *linter.Context, node *linter.Node) []linter.Finding {
+	if node.Arg != nil {
+		if !securityOptDisablesAppArmor(node.Arg.Value) {
+			return nil
+		}
+		return []linter.Finding{{
+			Message: fmt.Sprintf(`"runArgs" contains "--security-opt %s", disabling the container's AppArmor confinement`, node.Arg.Value),
+			Offset:  node.Value.StartOffset,
+		}}
+	}
+
+	lit, ok := node.Value.Value.(hujson.Literal)
+	if !ok || lit.Kind() != '"' || !securityOptDisablesAppArmor(lit.String()) {
+		return nil
+	}
+	return []linter.Finding{{
+		Message: fmt.Sprintf(`"securityOpt" contains %q, disabling the container's AppArmor confinement`, lit.String()),
+		Offset:  node.Value.StartOffset,
+	}}
+}
+
+// securityOptDisablesAppArmor reports whether s, a single "securityOpt" entry, removes the
+// container's AppArmor profile.
+func securityOptDisablesAppArmor(s string) bool {
+	opt, ok := dockerargs.ParseSecurityOpt(s)
+	return ok && opt.Key == "apparmor" && opt.Value == dockerargs.AppArmorProfileUnconfined
+}

rules/no_apparmor_unconfined_test.go

@@ -0,0 +1,83 @@
+package rules_test
+
+import (
+	"testing"
+
+	"github.com/bare-devcontainer/decolint/linter"
+	"github.com/bare-devcontainer/decolint/rules"
+)
+
+func TestNoApparmorUnconfined(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		src  string
+		want []linter.Issue
+	}{
+		{"no securityOpt property", `{"name": "test"}`, nil},
+		{"securityOpt without apparmor", `{"securityOpt": ["no-new-privileges"]}`, nil},
+		{"securityOpt with a custom apparmor profile", `{"securityOpt": ["apparmor=my-profile"]}`, nil},
+		{"securityOpt with apparmor=unconfined", `{"securityOpt": ["apparmor=unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-apparmor-unconfined",
+				Message: `"securityOpt" contains "apparmor=unconfined", disabling the container's AppArmor confinement`},
+		}},
+		{"securityOpt with apparmor:unconfined", `{"securityOpt": ["apparmor:unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-apparmor-unconfined",
+				Message: `"securityOpt" contains "apparmor:unconfined", disabling the container's AppArmor confinement`},
+		}},
+		{"seccomp=unconfined is a different confinement", `{"securityOpt": ["seccomp=unconfined"]}`, nil},
+		{"no runArgs", `{"runArgs": ["--init"]}`, nil},
+		{"runArgs without apparmor", `{"runArgs": ["--security-opt", "seccomp=unconfined"]}`, nil},
+		{"runArgs with security-opt apparmor=unconfined", `{"runArgs": ["--security-opt=apparmor=unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 14, RuleID: "no-apparmor-unconfined",
+				Message: `"runArgs" contains "--security-opt apparmor=unconfined", disabling the container's AppArmor confinement`},
+		}},
+		{"runArgs with security-opt apparmor=unconfined two tokens", `{"runArgs": ["--security-opt", "apparmor=unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 32, RuleID: "no-apparmor-unconfined",
+				Message: `"runArgs" contains "--security-opt apparmor=unconfined", disabling the container's AppArmor confinement`},
+		}},
+		{"every offending entry is reported", `{"runArgs": ["--security-opt=apparmor=unconfined", "--security-opt=apparmor:unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 14, RuleID: "no-apparmor-unconfined",
+				Message: `"runArgs" contains "--security-opt apparmor=unconfined", disabling the container's AppArmor confinement`},
+			{Path: "devcontainer.json", Line: 1, Col: 52, RuleID: "no-apparmor-unconfined",
+				Message: `"runArgs" contains "--security-opt apparmor:unconfined", disabling the container's AppArmor confinement`},
+		}},
+		{"both securityOpt and runArgs", `{"securityOpt": ["apparmor=unconfined"], "runArgs": ["--security-opt=apparmor=unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 18, RuleID: "no-apparmor-unconfined",
+				Message: `"securityOpt" contains "apparmor=unconfined", disabling the container's AppArmor confinement`},
+			{Path: "devcontainer.json", Line: 1, Col: 54, RuleID: "no-apparmor-unconfined",
+				Message: `"runArgs" contains "--security-opt apparmor=unconfined", disabling the container's AppArmor confinement`},
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assertIssues(t, rules.NoApparmorUnconfined, linter.SeverityWarn, tt.src, tt.want)
+		})
+	}
+}
+
+func TestNoApparmorUnconfined_Feature(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		src  string
+		want []linter.Issue
+	}{
+		{"no securityOpt property", `{"id": "test", "version": "1.0.0", "name": "test"}`, nil},
+		{"securityOpt with apparmor=unconfined", `{"id": "test", "securityOpt": ["apparmor=unconfined"]}`, []linter.Issue{
+			{Path: "devcontainer-feature.json", Line: 1, Col: 32, RuleID: "no-apparmor-unconfined",
+				Message: `"securityOpt" contains "apparmor=unconfined", disabling the container's AppArmor confinement`},
+		}},
+		// "runArgs" has no meaning in a Feature, so it's not flagged there.
+		{"runArgs with apparmor=unconfined is ignored", `{"id": "test", "runArgs": ["--security-opt=apparmor=unconfined"]}`, nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assertIssuesAt(t, rules.NoApparmorUnconfined, linter.SeverityWarn, "devcontainer-feature.json", linter.Feature, tt.src, tt.want)
+		})
+	}
+}

rules/no_dangerous_cap_add.go

@@ -0,0 +1,112 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/bare-devcontainer/decolint/dockerargs"
+	"github.com/bare-devcontainer/decolint/linter"
+	"github.com/tailscale/hujson"
+)
+
+// dangerousCapabilities maps each Linux capability that lets a process act on the host rather than
+// on the container to what it allows. The keys are as [dockerargs.Capability] returns a name, so a
+// capability written in any of its spellings is found here. None of them is in a container runtime's default set, so one
+// appearing in "capAdd" was granted deliberately.
+//
+// The list is deliberately narrower than "every capability a container does not need": adding one
+// means arguing that granting it reaches past the container, which for these means reaching a
+// kernel subsystem that is not namespaced. A capability the kernel confines to the container's own
+// namespaces belongs elsewhere however privileged it sounds — "SYS_PTRACE" (process namespace) and
+// "NET_ADMIN" (network namespace) are the standing examples, and sharing the host's namespaces is
+// what [NoHostNamespace] reports.
+//
+// "ALL" is not here: granting every capability at once is what [NoCapAddAll] reports.
+var dangerousCapabilities = map[string]string{
+	"CAP_AUDIT_CONTROL":   "allows reconfiguring the kernel's audit subsystem, which is not namespaced",
+	"CAP_BPF":             "allows loading BPF programs into the host kernel",
+	"CAP_DAC_READ_SEARCH": "bypasses file read permission checks and allows opening files by handle, outside the container's filesystem",
+	"CAP_MAC_ADMIN":       "allows changing the host's mandatory access control policy",
+	"CAP_MAC_OVERRIDE":    "bypasses the host's mandatory access control policy",
+	"CAP_PERFMON":         "grants access to the kernel's performance monitoring interfaces, which observe the whole host",
+	"CAP_SYSLOG":          "allows reading the host kernel's log, which discloses kernel addresses",
+	"CAP_SYS_ADMIN":       "grants a broad range of administrative operations, including mounting filesystems",
+	"CAP_SYS_BOOT":        "allows rebooting the host",
+	"CAP_SYS_MODULE":      "allows loading modules into the host kernel",
+	"CAP_SYS_RAWIO":       "allows raw access to the host's I/O ports and memory devices",
+	"CAP_SYS_TIME":        "allows setting the clock, which the container shares with the host",
+}
+
+// NoDangerousCapAdd reports a devcontainer.json or devcontainer-feature.json that grants a Linux
+// capability which lets a process reach past the container, either via the "capAdd" property or, in
+// a devcontainer.json, a "--cap-add" entry in "runArgs". Unlike [NoCapAddAll], which only flags
+// granting every capability at once, this rule flags the individual capabilities in
+// dangerousCapabilities.
+var NoDangerousCapAdd = &linter.Rule{
+	ID:          "no-dangerous-cap-add",
+	Description: `disallow granting a Linux capability that lets a process act on the host, e.g. "SYS_ADMIN" or "SYS_MODULE", via the "capAdd" property or a "--cap-add" entry in a devcontainer.json's "runArgs"`,
+	LongDescription: `Container runtimes withhold the capabilities that let a process act on the host rather than on the
+container, and "capAdd" adds them back one at a time. Each capability this rule reports is on its own
+enough to reach past the container — loading a module into the host kernel, opening a file by handle
+outside the mounted filesystem, rebooting the machine — and none of them is granted by default, so one
+that appears here was asked for. Grant only what the workload actually fails without.
+
+A capability the kernel confines to the container's own namespaces is not reported, however privileged
+it sounds: "SYS_PTRACE" reaches no further than the container's process namespace, and "NET_ADMIN" no
+further than its network namespace. What makes those dangerous is sharing the host's namespaces, which
+is a separate rule.`,
+	References: []string{
+		`https://containers.dev/implementors/json_reference/#general-properties`,
+		`https://docs.docker.com/engine/security/#linux-kernel-capabilities`,
+		`https://man7.org/linux/man-pages/man7/capabilities.7.html`,
+	},
+	Category:  linter.CategorySecurity,
+	FileTypes: []linter.FileType{linter.Devcontainer, linter.Feature},
+	Paths:     []string{"/capAdd/*", "/runArgs/--cap-add"},
+	Example: linter.Example{
+		Bad: linter.Snippet{
+			Files: []linter.ExampleFile{
+				{Path: `devcontainer.json`, Content: `{
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
+  "capAdd": ["SYS_ADMIN"]
+}
+`},
+			},
+		},
+		Good: linter.Snippet{
+			Files: []linter.ExampleFile{
+				{Path: `devcontainer.json`, Content: `{
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
+  "capAdd": ["SYS_PTRACE"]
+}
+`},
+			},
+		},
+	},
+	Check: checkNoDangerousCapAdd,
+}
+
+func checkNoDangerousCapAdd(_ *linter.Context, node *linter.Node) []linter.Finding {
+	if node.Arg != nil {
+		effect, dangerous := dangerousCapabilities[dockerargs.Capability(node.Arg.Value)]
+		if !dangerous {
+			return nil
+		}
+		return []linter.Finding{{
+			Message: fmt.Sprintf(`"runArgs" contains "--cap-add=%s", which %s`, node.Arg.Value, effect),
+			Offset:  node.Value.StartOffset,
+		}}
+	}
+
+	lit, ok := node.Value.Value.(hujson.Literal)
+	if !ok || lit.Kind() != '"' {
+		return nil
+	}
+	effect, dangerous := dangerousCapabilities[dockerargs.Capability(lit.String())]
+	if !dangerous {
+		return nil
+	}
+	return []linter.Finding{{
+		Message: fmt.Sprintf(`"capAdd" contains %q, which %s`, lit.String(), effect),
+		Offset:  node.Value.StartOffset,
+	}}
+}