fix: improve mount parsing to handle Docker socket path variations - #89
Merged
Conversation
no-docker-socket-mount and no-bind-mount read "mounts" entries and the "runArgs" mount flags with hand-rolled parsers that neither matched docker's syntax nor kept the two flag syntaxes apart, so they missed real Docker socket mounts and reported ones that never happen. A "--mount" value is now read the way docker/cli's opts.MountOpt.Set does: the whole value is trimmed and read as one CSV record, keys are matched case-insensitively with "src" as an alias for "source", and the type is lower-cased. Object entries get the same type folding, since devcontainers/cli hands them to docker as a "--mount" value. A "-v"/"--volume" value has no syntax in common with that, so it gets its own reader: colon-separated fields where a comma is an ordinary character, and a single-field value is an anonymous volume that binds nothing from the host. Which syntax applies depends on the flag introducing the value, which an entry does not carry on its own, so the rule now inspects the whole "runArgs" array. Whether a source is the socket is now one predicate, so the rule that reports it and the rule that excuses it cannot disagree. It cleans the path first, as the daemon does before mounting, so "//var/run/docker.sock" and "/var/run/docker.sock/" are recognized too.
The conflict was the rules/util.go import block: main added "iter" for arrayMembers while this branch added "encoding/csv", "iter", and "path". Both sides made a runArgs-reading rule see more of the document, so cover the interaction: this branch moved no-docker-socket-mount from "/runArgs/*" to "/runArgs", and main made rules read every copy of a duplicated array property.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves the robustness of mount parsing in the
no-docker-socket-mountandno-bind-mountrules by properly handling path normalization and supporting the full syntax of Docker mount specifications.Key Changes
Added
isDockerSocketSource()function: Centralizes Docker socket detection by normalizing paths usingpath.Clean(), ensuring that variations like//var/run/docker.sockand/var/run/docker.sock/are correctly identified as the Docker socket.Refactored
runArgsFlagValues()as an iterator: Changed from a single-match function to an iterator that yields all values for a given flag, enabling detection of multiple Docker socket mounts in a singlerunArgsarray.Enhanced
parseMountString()for CSV compliance: Updated to properly parse--mountvalues as CSV records (supporting quoted fields with embedded commas) and added support for thesrcalias forsource. Also normalized type values to lowercase to match Docker's behavior.Added
volumeSpecSource()function: Extracts the host path from-v/--volumevalues, properly handling the colon-separated syntax and distinguishing between anonymous volumes (single field) and host mounts.Improved
parseMountObject()normalization: Now normalizes thetypefield to lowercase for consistent comparison.Refactored
checkDockerSocketRunArgs(): Now inspects the entirerunArgsarray and checks all mount-related flags (--mount,--volume,-v), reporting each problematic mount individually.Notable Implementation Details
isDockerSocketSource()function is the single source of truth for Docker socket detection, preventing disagreement between rules about the same mount.srcalias forsourcein--mountvalues is now recognized.--mountvalues allows proper handling of quoted fields containing commas.https://claude.ai/code/session_0127yGsFgejQ2hSfrYmW48GT