fix: improve mount parsing to handle Docker socket path variations

rules/no_bind_mount.go

@@ -59,7 +59,7 @@ without an error and the container starts missing the data it expects. Volume mo
 
 func checkNoBindMount(_ *linter.Context, node *linter.Node) []linter.Finding {
 	mountType, source, ok := parseMount(node.Value)
-	if !ok || mountType != "bind" || source == dockerSocketPath {
+	if !ok || mountType != "bind" || isDockerSocketSource(source) {
 		return nil
 	}
 	return []linter.Finding{{

rules/no_bind_mount_test.go

@@ -26,7 +26,20 @@ func TestNoBindMount(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-bind-mount",
 				Message: `"mounts" entry uses the "bind" type, which GitHub Codespaces silently ignores`},
 		}},
+		{"string bind mount with upper-case keys and type", `{"mounts": ["Type=Bind,Source=/host/d,dst=/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-bind-mount",
+				Message: `"mounts" entry uses the "bind" type, which GitHub Codespaces silently ignores`},
+		}},
+		{"string bind mount with a quoted field", `{"mounts": ["type=bind,\"src=/host/d\",dst=/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-bind-mount",
+				Message: `"mounts" entry uses the "bind" type, which GitHub Codespaces silently ignores`},
+		}},
+		{"string mount whose fields are not a CSV record", `{"mounts": ["type=bind,src=\"/host/d\",dst=/x"]}`, nil},
 		{"object volume mount", `{"mounts": [{"source": "vol", "target": "/data", "type": "volume"}]}`, nil},
+		{"object bind mount with upper-case type", `{"mounts": [{"source": "/host/d", "target": "/x", "type": "BIND"}]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-bind-mount",
+				Message: `"mounts" entry uses the "bind" type, which GitHub Codespaces silently ignores`},
+		}},
 		{"object mount with no type", `{"mounts": [{"source": "vol", "target": "/data"}]}`, nil},
 		{"object bind mount", `{"mounts": [{"source": "/host", "target": "/data", "type": "bind"}]}`, []linter.Issue{
 			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-bind-mount",
@@ -39,6 +52,11 @@ func TestNoBindMount(t *testing.T) {
 		{"string docker socket bind mount", `{"mounts": ["source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind"]}`, nil},
 		{"string docker socket bind mount with docker-host.sock target", `{"mounts": ["source=/var/run/docker.sock,target=/var/run/docker-host.sock,type=bind"]}`, nil},
 		{"object docker socket bind mount", `{"mounts": [{"source": "/var/run/docker.sock", "target": "/var/run/docker.sock", "type": "bind"}]}`, nil},
+		// The socket exemption must recognize every spelling no-docker-socket-mount reports, or the two
+		// rules report the same mount with contradictory findings.
+		{"string docker socket bind mount via src alias", `{"mounts": ["type=bind,src=/var/run/docker.sock,dst=/x"]}`, nil},
+		{"string docker socket bind mount with a doubled leading slash", `{"mounts": ["type=bind,src=//var/run/docker.sock,dst=/x"]}`, nil},
+		{"string docker socket bind mount with a trailing slash", `{"mounts": ["type=bind,src=/var/run/docker.sock/,dst=/x"]}`, nil},
 		{"bind mount with unrelated source named docker.sock", `{"mounts": ["source=/host/docker.sock,target=/var/run/docker.sock,type=bind"]}`, []linter.Issue{
 			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-bind-mount",
 				Message: `"mounts" entry uses the "bind" type, which GitHub Codespaces silently ignores`},

rules/no_docker_socket_mount.go

@@ -1,8 +1,6 @@
 package rules
 
 import (
-	"strings"
-
 	"github.com/bare-devcontainer/decolint/linter"
 	"github.com/tailscale/hujson"
 )
@@ -25,7 +23,7 @@ rootless daemon keeps that access inside the container.`,
 	},
 	Category:  linter.CategorySecurity,
 	FileTypes: []linter.FileType{linter.Devcontainer},
-	Paths:     []string{"/mounts/*", "/runArgs/*"},
+	Paths:     []string{"/mounts/*", "/runArgs"},
 	Example: linter.Example{
 		Bad: linter.Snippet{
 			Files: []linter.ExampleFile{
@@ -58,15 +56,15 @@ rootless daemon keeps that access inside the container.`,
 }
 
 func checkNoDockerSocketMount(_ *linter.Context, node *linter.Node) []linter.Finding {
-	if strings.HasPrefix(node.Pointer, "/mounts/") {
-		return checkDockerSocketMount(node)
+	if node.Pointer == "/runArgs" {
+		return checkDockerSocketRunArgs(node)
 	}
-	return checkDockerSocketRunArg(node)
+	return checkDockerSocketMount(node)
 }
 
 func checkDockerSocketMount(node *linter.Node) []linter.Finding {
 	_, source, ok := parseMount(node.Value)
-	if !ok || source != dockerSocketPath {
+	if !ok || !isDockerSocketSource(source) {
 		return nil
 	}
 	return []linter.Finding{{
@@ -75,29 +73,35 @@ func checkDockerSocketMount(node *linter.Node) []linter.Finding {
 	}}
 }
 
-func checkDockerSocketRunArg(node *linter.Node) []linter.Finding {
-	lit, ok := node.Value.Value.(hujson.Literal)
-	if !ok || lit.Kind() != '"' || !runArgMountsDockerSocket(lit.String()) {
-		return nil
-	}
-	return []linter.Finding{{
-		Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`,
-		Offset:  node.Value.StartOffset,
-	}}
+// dockerSocketRunArgFlags are the "runArgs" flags that can mount a host path, each paired with the
+// reader for its own value syntax. The two syntaxes are unrelated, so a value must be read only as
+// the flag introducing it, which is why this rule inspects the whole "runArgs" array rather than its
+// entries one by one.
+var dockerSocketRunArgFlags = []struct {
+	flag   string
+	source func(string) string
+}{
+	{"--mount", func(s string) string { _, source := parseMountString(s); return source }},
+	{"--volume", volumeSpecSource},
+	{"-v", volumeSpecSource},
 }
 
-// runArgMountsDockerSocket reports whether s, a single "runArgs" entry, bind-mounts the Docker
-// socket. It recognizes a "--mount"-style "key=value,..." entry with a matching "source", and a
-// "-v"/"--volume" entry, with or without an "=" before the value, whose host path is the Docker
-// socket.
-func runArgMountsDockerSocket(s string) bool {
-	if strings.Contains(s, ",") {
-		if _, source := parseMountString(strings.TrimPrefix(s, "--mount=")); source == dockerSocketPath {
-			return true
-		}
+func checkDockerSocketRunArgs(node *linter.Node) []linter.Finding {
+	arr, ok := node.Value.Value.(*hujson.Array)
+	if !ok {
+		return nil
 	}
-	for _, prefix := range []string{"--volume=", "-v="} {
-		s = strings.TrimPrefix(s, prefix)
+	var findings []linter.Finding
+	for _, f := range dockerSocketRunArgFlags {
+		for value, s := range runArgsFlagValues(arr, f.flag) {
+			if !isDockerSocketSource(f.source(s)) {
+				continue
+			}
+			findings = append(findings, linter.Finding{
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`,
+				Offset:  value.StartOffset,
+			})
+		}
 	}
-	return s == dockerSocketPath || strings.HasPrefix(s, dockerSocketPath+":")
+	return findings
 }

rules/no_docker_socket_mount_test.go

@@ -31,7 +31,28 @@ func TestNoDockerSocketMount(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-docker-socket-mount",
 				Message: `"mounts" entry bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
 		}},
+		{"string docker socket mount via src alias", `{"mounts": ["type=bind,src=/var/run/docker.sock,dst=/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-docker-socket-mount",
+				Message: `"mounts" entry bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"string docker socket mount with upper-case keys", `{"mounts": ["Type=Bind,SRC=/var/run/docker.sock,DST=/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-docker-socket-mount",
+				Message: `"mounts" entry bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"string docker socket mount with a quoted field", `{"mounts": ["type=bind,\"src=/var/run/docker.sock\",dst=/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-docker-socket-mount",
+				Message: `"mounts" entry bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"string docker socket mount with leading whitespace", `{"mounts": [" \"src=/var/run/docker.sock\",dst=/x,type=bind"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-docker-socket-mount",
+				Message: `"mounts" entry bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"object docker socket mount with upper-case type", `{"mounts": [{"source": "/var/run/docker.sock", "target": "/x", "type": "BIND"}]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 13, RuleID: "no-docker-socket-mount",
+				Message: `"mounts" entry bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
 		{"unrelated source named docker.sock", `{"mounts": ["source=/host/docker.sock,target=/var/run/docker.sock,type=bind"]}`, nil},
+		{"string mount whose fields are not a CSV record", `{"mounts": ["type=bind,src=\"/var/run/docker.sock\",dst=/x"]}`, nil},
 		{"non-string literal mount entry", `{"mounts": [123]}`, nil},
 		{"non-literal, non-object mount entry", `{"mounts": [["source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind"]]}`, nil},
 
@@ -61,7 +82,39 @@ func TestNoDockerSocketMount(t *testing.T) {
 			{Path: "devcontainer.json", Line: 1, Col: 14, RuleID: "no-docker-socket-mount",
 				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
 		}},
+		{"runArgs --mount two tokens via src alias", `{"runArgs": ["--mount", "type=bind,src=/var/run/docker.sock,dst=/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 25, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"runArgs -v host path with a doubled leading slash", `{"runArgs": ["-v", "//var/run/docker.sock:/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 20, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"runArgs -v host path with a trailing slash", `{"runArgs": ["-v", "/var/run/docker.sock/:/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 20, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"runArgs every -v is reported", `{"runArgs": ["-v", "/var/run/docker.sock:/a", "-v", "/var/run/docker.sock:/b"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 20, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+			{Path: "devcontainer.json", Line: 1, Col: 53, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
 		{"runArgs unrelated volume", `{"runArgs": ["-v", "/host/docker.sock:/var/run/docker.sock"]}`, nil},
+		// A -v value of a single field is an anonymous volume, and that field is the container path:
+		// nothing from the host is bound.
+		{"runArgs -v with only a container path", `{"runArgs": ["-v", "/var/run/docker.sock"]}`, nil},
+		// A -v value is colon-separated, so a comma in it is part of a field rather than a separator.
+		{"runArgs --volume with a comma in the container path", `{"runArgs": ["--volume", "myvol:/data,source=/var/run/docker.sock"]}`, nil},
+		{"runArgs not an array", `{"runArgs": "-v /var/run/docker.sock:/x"}`, nil},
+		{"runArgs duplicated, mounted by the first", `{"runArgs": ["-v", "/var/run/docker.sock:/x"], "runArgs": ["--init"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 20, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
+		{"runArgs duplicated, mounted by the last", `{"runArgs": ["--init"], "runArgs": ["-v", "/var/run/docker.sock:/x"]}`, []linter.Issue{
+			{Path: "devcontainer.json", Line: 1, Col: 43, RuleID: "no-docker-socket-mount",
+				Message: `"runArgs" bind-mounts the Docker socket, which grants the container root-equivalent control over the host`},
+		}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

rules/util.go

@@ -1,7 +1,9 @@
 package rules
 
 import (
+	"encoding/csv"
 	"iter"
+	"path"
 	"strings"
 
 	"github.com/bare-devcontainer/decolint/linter"
@@ -13,6 +15,16 @@ import (
 // container even though other bind mounts are ignored.
 const dockerSocketPath = "/var/run/docker.sock"
 
+// isDockerSocketSource reports whether source, a mount source, names the host's Docker socket. It is
+// the single answer to that question, so that a rule reporting the socket and a rule excusing it
+// never disagree about the same mount.
+//
+// The daemon cleans a bind mount's source before using it, so spellings such as
+// "//var/run/docker.sock" and "/var/run/docker.sock/" all reach the same socket.
+func isDockerSocketSource(source string) bool {
+	return path.Clean(source) == dockerSocketPath
+}
+
 // hasMember reports whether obj has a member named name.
 func hasMember(obj *hujson.Object, name string) bool {
 	return memberNamed(obj, name) != nil
@@ -64,53 +76,85 @@ func arrayMembers(obj *hujson.Object, name string) iter.Seq[*hujson.Array] {
 	}
 }
 
-// runArgsFindFlagValue scans arr, a "runArgs" array, for an entry that sets flag to a value accepted
-// by match. Docker accepts such a value either as a single combined "flag=value" entry or as two
-// adjacent entries, "flag" followed by "value". It returns the hujson.Value holding the matching
-// value, or nil if no entry sets flag to a value match accepts.
-func runArgsFindFlagValue(arr *hujson.Array, flag string, match func(string) bool) *hujson.Value {
-	for i := range arr.Elements {
-		lit, ok := arr.Elements[i].Value.(hujson.Literal)
-		if !ok || lit.Kind() != '"' {
-			continue
-		}
+// runArgsFlagValues yields every value that arr, a "runArgs" array, gives to flag, in order. Docker
+// accepts such a value either as a single combined "flag=value" entry or as two adjacent entries,
+// "flag" followed by "value". Each yielded pair is the hujson.Value holding the value and the value
+// itself.
+func runArgsFlagValues(arr *hujson.Array, flag string) iter.Seq2[*hujson.Value, string] {
+	return func(yield func(*hujson.Value, string) bool) {
+		for i := range arr.Elements {
+			lit, ok := arr.Elements[i].Value.(hujson.Literal)
+			if !ok || lit.Kind() != '"' {
+				continue
+			}
 
-		if v, ok := strings.CutPrefix(lit.String(), flag+"="); ok {
-			if match(v) {
-				return &arr.Elements[i]
+			if v, ok := strings.CutPrefix(lit.String(), flag+"="); ok {
+				if !yield(&arr.Elements[i], v) {
+					return
+				}
+				continue
 			}
-			continue
-		}
 
-		if lit.String() != flag || i+1 >= len(arr.Elements) {
-			continue
+			if lit.String() != flag || i+1 >= len(arr.Elements) {
+				continue
+			}
+			next, ok := arr.Elements[i+1].Value.(hujson.Literal)
+			if ok && next.Kind() == '"' && !yield(&arr.Elements[i+1], next.String()) {
+				return
+			}
 		}
-		next, ok := arr.Elements[i+1].Value.(hujson.Literal)
-		if ok && next.Kind() == '"' && match(next.String()) {
-			return &arr.Elements[i+1]
+	}
+}
+
+// runArgsFindFlagValue returns the hujson.Value holding the first value arr gives to flag that match
+// accepts, or nil if it gives flag no such value. See [runArgsFlagValues] for the entry forms it
+// recognizes.
+func runArgsFindFlagValue(arr *hujson.Array, flag string, match func(string) bool) *hujson.Value {
+	for v, s := range runArgsFlagValues(arr, flag) {
+		if match(s) {
+			return v
 		}
 	}
 	return nil
 }
 
-// parseMountString extracts the "type" and "source" fields from s, a "key=value,..." mount entry.
+// parseMountString extracts the "type" and "source" fields from s, a "--mount" value, as docker/cli
+// reads it:
+//   - the value is trimmed of surrounding whitespace and then read as a single CSV record, so a
+//     field may be quoted to protect a comma inside it;
+//   - field keys are matched case-insensitively, and "src" is an alias for "source";
+//   - the type is normalized to lower case.
+//
+// It returns empty strings for a value that is not a well-formed CSV record, which docker rejects
+// outright.
+//
+// Whitespace around a key or a value is the one deliberate deviation: docker rejects it, but reading
+// such a field anyway lets the rules name the mount the author meant rather than fall silent on a
+// value that already fails to start the container.
 func parseMountString(s string) (mountType, source string) {
-	for part := range strings.SplitSeq(s, ",") {
-		key, value, ok := strings.Cut(part, "=")
+	fields, err := csv.NewReader(strings.NewReader(strings.TrimSpace(s))).Read()
+	if err != nil {
+		return "", ""
+	}
+	for _, field := range fields {
+		key, value, ok := strings.Cut(field, "=")
 		if !ok {
 			continue
 		}
-		switch strings.TrimSpace(key) {
+		value = strings.TrimSpace(value)
+		switch strings.ToLower(strings.TrimSpace(key)) {
 		case "type":
-			mountType = strings.TrimSpace(value)
-		case "source":
-			source = strings.TrimSpace(value)
+			mountType = strings.ToLower(value)
+		case "source", "src":
+			source = value
 		}
 	}
 	return mountType, source
 }
 
-// parseMountObject extracts the "type" and "source" members from obj, a "mounts" entry.
+// parseMountObject extracts the "type" and "source" members from obj, a "mounts" entry. The member
+// names are the ones devcontainers/cli reads off the object and so are case-sensitive, but it hands
+// their values to docker as a "--mount" value, which lower-cases the type.
 func parseMountObject(obj *hujson.Object) (mountType, source string) {
 	for _, m := range obj.Members {
 		name, ok := m.Name.Value.(hujson.Literal)
@@ -123,17 +167,29 @@ func parseMountObject(obj *hujson.Object) (mountType, source string) {
 		}
 		switch name.String() {
 		case "type":
-			mountType = value.String()
+			mountType = strings.ToLower(value.String())
 		case "source":
 			source = value.String()
 		}
 	}
 	return mountType, source
 }
 
+// volumeSpecSource returns the host path or volume name that s, a "-v"/"--volume" value, mounts, or
+// "" if s names no source. A volume spec shares no syntax with a "--mount" value: its fields are
+// separated by colons and a comma is an ordinary character. A spec of a single field is an anonymous
+// volume, and that field is the container path, not a host path.
+func volumeSpecSource(s string) string {
+	source, _, ok := strings.Cut(s, ":")
+	if !ok {
+		return ""
+	}
+	return source
+}
+
 // parseMount extracts the "type" and "source" fields from v, a "mounts" entry, which may be either
-// the "key=value,..." string shorthand or an object with corresponding members. ok is false if v is
-// neither.
+// the "--mount" string shorthand (see [parseMountString]) or an object with corresponding members.
+// ok is false if v is neither.
 func parseMount(v *hujson.Value) (mountType, source string, ok bool) {
 	switch val := v.Value.(type) {
 	case hujson.Literal: