feat(rule): add security rules for dangerous capabilities and host namespaces - #78
Open
nozaq wants to merge 35 commits into
Open
feat(rule): add security rules for dangerous capabilities and host namespaces#78nozaq wants to merge 35 commits into
nozaq wants to merge 35 commits into
Conversation
Add four security rules covering privilege grants the existing rules leave uncovered: - no-apparmor-unconfined: the AppArmor counterpart of no-seccomp-unconfined, for "apparmor=unconfined" (and the ":" separator Docker also accepts) in "securityOpt" or "runArgs". - no-host-namespace: a "host" value for --network/--net, --pid, --ipc, --uts, --userns, or --cgroupns in "runArgs". - no-dangerous-cap-add: individual capabilities that let a process act on the host, where no-cap-add-all only covered "ALL". Capability names are matched the way Docker matches them, case-insensitively and with or without the "CAP_" prefix. SYS_PTRACE is excluded: it is the documented way to run a debugger in a dev container and is what three other rules recommend instead of a broader grant. - no-initialize-command: "initializeCommand" is the one lifecycle command that runs on the host rather than in the container. Extract runArgsFlagValues from runArgsFindFlagValue so a rule can report every entry setting a flag, not just the first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
no-host-namespace matches the CIS Docker Benchmark's per-namespace controls (5.10 network, 5.16 process, 5.17 IPC, 5.21 UTS, 5.31 user) and the Kubernetes Pod Security Standards' Host Namespaces control; cite both. Record why the cgroup namespace has no matching control. no-dangerous-cap-add has no published denylist behind it — the capabilities it reports are a subset of what the Pod Security Standards' baseline policy disallows adding, narrowed to those that reach the host. Say so, and cite the policy the subset is taken from, so a later change to the list has a stated criterion to argue against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
It is a Kubernetes policy, not anything the Dev Container specification or Docker defines, so it does not belong among the references a reader of these rules is pointed at. no-dangerous-cap-add states its own criterion instead: a capability is reported when granting it lets a process reach the host, which is what leaves "SYS_PTRACE" off. no-host-namespace keeps the CIS Docker Benchmark, which has a control per namespace it reports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
nozaq
changed the title
no-cap-add-all compared "ALL" case-sensitively while no-dangerous-cap-add normalizes case but leaves "ALL" to it, so "capAdd": ["all"] and "--cap-add=all" were reported by neither rule even though Docker matches the name case-insensitively and grants every capability. Match it case-insensitively where it belongs. Drop NET_ADMIN from no-dangerous-cap-add: the kernel confines it to the container's own network namespace, so calling it "reconfiguring the host's network stack" was wrong, and keeping it contradicted the criterion that leaves SYS_PTRACE off. State that criterion as "reaches a kernel subsystem that is not namespaced" and name both capabilities as the standing examples of what it excludes. no-initialize-command declares no Platforms, so it also applies to Codespaces, where the command runs on the ephemeral VM rather than on a developer's own machine. Describe the machine creating the container instead of assuming which one it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Four gaps where a rule added here diverged from the one it pairs with: - require-cap-drop-all compared "ALL" case-sensitively, so "--cap-drop=all" was reported as not dropping capabilities even though Docker drops every one. It now shares capabilityIsAll with no-cap-add-all, which is why that helper moves to util.go. - no-seccomp-unconfined and no-seccomp-override matched only the "=" separator, so "seccomp:unconfined" was reported by nothing while the new AppArmor rule handled both forms. Extract securityOptValue and route all three rules through it. - no-initialize-command's "names no command" guard covered null and the empty string but not an empty argv array or an empty object of parallel commands. - no-cap-add-all quoted "ALL" in its message whatever the entry said, so a lower-case entry produced a message naming text absent from the file. Echo the entry as written, as the rules added here do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
no-cap-add-all and no-seccomp-unconfined returned on the first offending "runArgs" entry, so a second one went unreported — while the same two rules report every offending "capAdd" or "securityOpt" element, and no-seccomp-override reports every entry. Accumulate the findings instead of returning the first. no-privileged-container matched the entry "--privileged" exactly. "--privileged" is a boolean flag, so docker run also accepts "--privileged=true" (and the devcontainers CLI passes "runArgs" through verbatim), which went unreported. Parse the value the way Go parses a bool, leaving "--privileged=false" alone, and echo the entry as written in the message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
securityOptIsNoNewPrivileges accepted only the literal "true", so "no-new-privileges=1" and "=True" were reported as leaving the option unset even though Docker parses the value with strconv.ParseBool — the same parsing no-privileged-container just gained for "--privileged=<bool>". Route it through securityOptValue for the separators and ParseBool for the value, which also corrects a doc comment that described Docker as accepting only "true" and "false". Move no-seccomp-unconfined's securityOpt colon case back into the securityOpt group of its test table; it had been inserted under the runArgs comment, between two runArgs cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
The recurring defect in this PR's review was rules interpreting "runArgs" and "securityOpt" entries slightly differently from each other: case-sensitive capability names, only the "=" separator, only the bare form of a boolean option, only the first matching entry. Each fix landed in one rule and left its siblings behind. Gather the conventions in rules/dockerargs.go, with the list of them stated once at the top: flag value forms, "securityOpt" separators, boolean option values, and capability name normalization. Move runArgsFlagValues, runArgsFindFlagValue, runArgsApplicable, securityOptValue, capabilityIsAll, and normalizeCapability there from util.go and from the rule files, and add optionValue/optionEnabled so the bool parsing behind "--privileged=<bool>" and "no-new-privileges=<bool>" is one implementation rather than two. No behavior change; util.go keeps the HuJSON accessors and the mount and reference parsing, which are not Docker argument syntax. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Checked each convention in rules/dockerargs.go against docker/cli and moby rather than against the flag documentation, which turned up one form that goes unreported and one that is reported but does not exist. opts.NetworkOpt.Set reads a "--network" value as a field list once any field carries a value, taking the network from the "name" field and lower-casing each field, so "--network=name=host" and "--net=NAME=HOST" select host networking and went unreported. Add networkModeTarget for it. Only "--network" parses a field list, so every other namespace flag keeps naming its namespace directly and "--pid=name=host" stays unreported, as Docker leaves it. parseSecurityOpts splits an entry on ":" only when it holds no "=", so "seccomp:profile=custom.json" names the key "seccomp:profile" and is not a seccomp override; matching the separators by prefix reported it as one. Split the entry the way Docker does instead. Also split optionEnabled into securityOptEnabled and flagEnabled: a boolean flag takes no value from the following entry, which the shared separator argument obscured. Confirmed unchanged against the same sources: capability names are upper-cased with "CAP_" added if absent and "ALL" recognized before the prefix; "--cap-add" and "--security-opt" are ListOpts and do not split on commas; a namespace value is compared to "host" exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
opts.NetworkOpt.Set assigns the network from every "name" field it reads, so a repeated one leaves the last, while networkModeTarget returned the first. "--network=name=devnet,name=host" runs in the host network namespace and went unreported, and "name=host,name=devnet" was reported although Docker uses devnet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
networkModeTarget matched a field's key by prefix, but docker/cli trims the space around a field's key and value, so "--network=alias=web, name=host" joins the host network and went unreported while "--network=name=host, alias=web" did not — whether the rule fired turned on where the space fell. Cut each field and trim both halves, and treat a field carrying no value as leaving the value naming no network, as Docker's error does. no-seccomp-override judged each "runArgs" entry on its own rather than scanning the array for "--security-opt", so any entry that reads like a security option was flagged wherever it sat. The ":" separator this PR added widened that: "-v", "seccomp:/data" was reported as overriding the seccomp profile. Take the rule's "runArgs" path to the whole array and read the values through runArgsFlagValues, as the sibling rules do, so a value counts only where it follows "--security-opt". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Four subagents derived edge cases from docker/cli, moby and pflag without sight of this code; running the 47 they produced against the binary failed 12, nine of them one defect: every rule judged a "runArgs" entry by its shape alone, so an entry set a flag wherever it sat. Docker parses positionally, and these all went unreported by it while decolint reported them: ["-e", "--cap-add=SYS_ADMIN"] value of the preceding flag ["--name", "--cap-add", "SYS_ADMIN"] swallowed by a preceding flag ["myimage", "--cap-add=SYS_ADMIN"] after the image name ["--", "--cap-add=SYS_ADMIN"] after the terminator ["--security-opt", "--privileged"] value of the preceding flag Add runArgsFlags, which walks the array left to right as pflag does: a boolean flag never takes the following entry, every other flag always does whatever it looks like, a shorthand cluster ends at the first letter that takes a value, and "--" or the first positional ends flag parsing along with everything after it. runArgsFlagValues now filters that walk, so the five rules already reading it are fixed without changing them; no-privileged-container moves off "/runArgs/*" to join them. Two tests asserted the old behavior for a non-string entry before a flag. Docker takes a non-string as the image name, so nothing after it is a flag; they now expect no finding. Three of the twelve remain, all repeated-option precedence, and all reported where Docker would use the other value: securityOpt ["seccomp=unconfined","seccomp=builtin"] last wins runArgs ["--network","devnet","--net","host"] first wins runArgs ["--pid","host","--pid","container:x"] last wins Each needs the rule to pick an effective value rather than report every occurrence, which is a change of contract rather than a fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
The 47 cases the subagents derived from docker/cli, moby and pflag now all match. Three were left in the previous commit as a change of contract rather than a fix; they are repeated-option precedence, which decides which occurrence the container actually runs with: securityOpt ["seccomp=unconfined","seccomp=builtin"] last wins runArgs ["--network","devnet","--net","host"] first wins runArgs ["--pid","host","--pid","container:x"] last wins A repeat of these replaces the earlier value instead of adding to it, so reporting every occurrence names a value that never takes effect. Add runArgsEffectiveFlag and securityOptEffective, and have the rules report only the occurrence that decides the setting. "--network" and its "--net" alias share one list whose first entry becomes the network mode; every other namespace flag and every "securityOpt" key is overwritten, so the last wins. The accumulating flags are unaffected: "--cap-add" and "--security-opt" still report every entry, because Docker applies every one. no-docker-socket-mount had the positional defect the previous commit fixed elsewhere: it read "/runArgs/*" and judged each entry alone, so ["--name", "-v", "/var/run/docker.sock:..."] was reported although the "-v" there is --name's value. Move it onto runArgsFlags, which now also resolves shorthand flags so "-v" is read where Docker reads it. flagEnabled goes with no-privileged-container's move to the array walk in the previous commit; nothing calls it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
runArgsFlags stopped at a non-string entry, on the reasoning that "docker run" would take it as the image name and parse no flag after it. That reasoning does not hold: "runArgs" is an array of strings, so a non-string is invalid rather than positional, and it never reaches "docker run" — the devcontainer CLI spreads the array into an argv that cp.spawn rejects outright. Nothing specifies what happens, and the parse rule was invented rather than derived. Undefined input is where a linter should fail loud, not quiet: ["runArgs": [123, "--privileged"]] left the "--privileged" unreported because of a typo elsewhere in the array. Skip the entry and keep reading instead, and likewise where a non-string sits in a flag's value position — the flag took it, but the entries after it are still configuration someone wrote. The four cases asserting the old behavior now expect the finding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
"docker run [OPTIONS] IMAGE [COMMAND] [ARG...]" is documented, and flags.SetInterspersed(false) is how docker/cli enforces it, so a bare entry in "runArgs" really does end flag parsing. Stopping there is still the wrong thing for a linter to do. The devcontainer CLI appends "runArgs" ahead of the image and ahead of the flags it derives from the configuration (singleContainer.ts: the array is spread before featureArgs, entrypoint and imageName). A bare entry therefore takes the place of the image and demotes the rest — including the "--privileged", "--cap-add" and "--security-opt" the CLI generates from the configuration's own properties — to words of the container command. The file is broken, not benign. decolint already reports "privileged", "capAdd" and "securityOpt" whether or not such an entry voids them, so stopping the runArgs walk there was inconsistent as well as quiet: ["myimage", "--privileged"] reported nothing. Read on instead. The "--" terminator still stops the walk, since that one is written to mean exactly that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
The list was assembled from a summary rather than from docker/cli's
flag registration, and missed one: --use-api-socket is registered with
flags.BoolVarP in both run.go and create.go. A boolean flag left off
the list is the costly direction — the walk takes the entry after it
as its value and skips whatever that entry really was, so
["--use-api-socket", "--privileged"] reported nothing.
Re-derived both lists from every Bool and *VarP registration across
cli/command/container's opts.go, run.go and create.go. The 15 boolean
flags are now complete, and the value-taking shorthands were already
right: of the 14 shorthands docker run registers, -d, -i, -q and -t
are boolean and the other ten take a value ("c" is --cpu-shares).
Record where the lists come from so the next reader can re-check them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Reading "runArgs" positionally needed a table of which flags consume the entry after them, and a boolean flag missing from it made the walk skip that entry — a finding lost, silently. --use-api-socket was missing (cb39acf), which is the failure mode the design invites rather than a one-off: the table has to track docker/cli's flag set forever, and every drift costs a finding. What it bought was small. Of the 47 derived cases only four turn on arity, and the one that matters — ["-v", "seccomp:/data"] read as a seccomp override — is caught by asking whether the entry before a value is the flag being looked for, which needs no table at all. So read every entry, and where one can be read in more than one way return each reading: a bare "--flag" is both a boolean set to true and a flag taking the entry after it, and "-vX" is both "-vX" and "-v" set to "X". A rule looks up the flag it cares about, so a reading naming a different flag costs nothing. The "--" terminator and the image-name position go with the table; both only arise in configurations Docker would not run as written, and 677cb10 already decided to read past the latter. The cost is a finding on a value spelled exactly like a flag — ["--security-opt", "--privileged"] and ["--", "--cap-add=SYS_ADMIN"] are now reported, and require-cap-drop-all counts a "--cap-drop=ALL" in a value position as set. Seven corpus cases record that, with the reason. No maintenance, and no way to lose a finding to a stale list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
The rule reported "initializeCommand" being used at all, on the grounds that it is the one lifecycle command running outside the container. Two things undercut that. It cannot arrive unseen. The specification's merge-logic table lists every lifecycle command a Feature or image metadata may contribute, and "initializeCommand" is not among them; the Feature schema has no such property either. Unlike a "securityOpt" inherited from a base image, it is always written in the file the reader is already looking at, and its name says what it does. And it offered no fix. Every other rule in the category names a narrower way to get the same thing — "capAdd" for a capability, a user-defined network instead of the host's. The legitimate use of "initializeCommand" is host-side work before the container exists, which by definition cannot move into it, so the finding left those users with nothing to do but silence the rule. Reporting a specification-sanctioned property for being used is a preference, not a defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Five cases in no-host-namespace's table exercised shorthand entries — "-v" alone, "-e" taking the next entry, the "-it" cluster, "-eFOO=bar" — none of which the rule looks up. They were written against the walk that tracked which flag consumes which entry, to reach its cluster and value-skipping branches. 7e22ddb removed that walk, so they now assert only that an unrelated entry does not stop "--pid=host" from being found, which holds for any entry at all. The one shorthand form that still matters is a value carried in the same entry, and it matters to no-docker-socket-mount, which does look up "-v"; add "-v/var/run/docker.sock:..." there. What is left in no-host-namespace's table is about that rule: a bare flag reads as a boolean and so names no namespace, and a non-string entry is skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
dockerargs.go carries decolint's reading of Docker's own argument handling, derived from docker/cli, moby and pflag rather than from any specification, and until now it was only exercised through the rules that call it. That left the reading stated nowhere executable, made a failure ambiguous between rule and parser, and made coverage incidental — ca13d3c removed five cases from an unrelated rule's table and had to relocate one of them to keep a branch covered at all. Add rules/dockerargs_test.go, an internal test as docs_test.go already is, stating the contract per function: which readings an entry yields and why more than one, how a "securityOpt" entry splits and which separator wins, how a boolean option's value is read, which occurrence of a repeated flag decides it, how a "--network" field list resolves, and how a capability name normalizes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
"Read the value with [optionBool]" told the caller to do something without saying why, and "read" could as easily have meant "access". What matters is the contract: Value is the flag's value as written and the bare form reads as "true", so a comparison against "true" gets "--privileged=1" wrong in one direction and "--privileged=yes" wrong in the other. Say that, and that the reading taking a value from the following entry is left out because a boolean flag is never set that way. The file header still described a boolean flag as taking only the combined form, which is Docker's convention but no longer what this file encodes: 7e22ddb stopped choosing between the readings of an entry. Point at runArgsFlags for why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
The AppArmor rule reported every "apparmor=unconfined" entry while no-seccomp-unconfined, which its own doc calls its counterpart, reported only the one Docker keeps. Two ways to fix that; this takes the one that matches how the rest of the file already decides. runArgsEffectiveFlag and securityOptEffective encoded which repeat of an option Docker keeps — "--network" and its alias keep the first, every other namespace flag and every "securityOpt" key the last. That is another table of docker/cli internals whose drift loses a finding silently: read "--network" as last-wins after Docker changed to it and ["--network=devnet", "--net=host"] stops being reported at all. It is the failure mode 7e22ddb removed the arity table for. What the table bought was silence on a repeated option, and that silence is wrong for the same reason a positional entry no longer stops the walk (677cb10): the entry it hides is dead only where it stands, and live again the moment the order changes. A "seccomp=unconfined" left above a later "seccomp=builtin" is worth deleting, not worth hiding. So report every entry, as the AppArmor rule already did. Three corpus cases record the divergence from Docker, with the reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
parseMountString matched the literal "source", but docker/cli's mount parser lower-cases each field's key and the "type" value and takes "src" as an alias of "source" (opts/mount.go, MountOpt.Set). So "type=bind,src=/var/run/docker.sock,..." named the socket to Docker and nothing to decolint, while the "source=" spelling of the same mount was reported — in both "runArgs" and the "mounts" string shorthand, which is a "--mount" value. The source path itself is left as written; it is a path. Also correct runArgsFlag.Raw. It is documented as the entry the flag was written in, but the reading that takes a value from the following entry set it to that entry's text. Nothing reads it there today — runArgsBoolFlag filters those readings out — so this is a trap rather than a bug: set Raw to the flag's own entry and say that it and Node then point at different entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
runArgsFlags returns every way an entry could be read, so one entry can reach a rule twice. no-docker-socket-mount reported the same element twice for ["--mount", "--mount=type=bind,source=..."]: once as the previous flag's value, whose text still carries the "--mount=" prefix, and once as its own combined flag. parseMountString scans for "key=value" fields and tolerates the leftover prefix as one more field, so both readings matched. The sibling rules escape it only because their matchers want the whole value to have one exact shape. Fixing it per rule would leave the next one to find it again, so drop the duplicate where every rule's findings meet: two alike in path, position, rule and message say nothing the first does not. A finding differing in any field is not a duplicate and is kept, so a rule can still report two distinct problems at one position. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
TestLintDocument_DropsDuplicateFindings, added in c6f1ad2, asserted an order sort.Slice does not promise: it is not stable, so two findings sharing a line, column and rule came out in whichever order the sort left them. The test passed here and failed elsewhere. The test is the symptom; the ordering is the defect. A rule may report several findings at one position — missing-required-props reports one per absent property, all at the document root — and their order in the output varied between runs, which a diff of two CI runs or two SARIF uploads would show. Break the tie on the message. Give the test's third finding a column of its own as well, so it rests on the position order rather than on the new tiebreak. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
354a0f7 lower-cased a mount entry's type where the entry is a string, matching what docker/cli's mount parser does, and left the object form comparing it as written. So {"type": "Bind"} escaped no-bind-mount while "type=Bind,..." — the same mount, the other spelling — was reported. The devcontainer.json schema declares the object form's type as a lower-case enum, so this input is invalid there, but it is not inert: the tooling turns the object into a "--mount" value, and Docker lower-cases the type in it, so the container really does get a bind mount. Lower-case it in both, and test both spellings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
An entry Docker consumes as the value of the flag before it was read as a flag of its own, which inverted two rules: `["--label", "--cap-drop=ALL"]` silenced require-cap-drop-all even though the entry is the label's value and no capability is dropped, and the same shape silenced require-no-new-privileges. The reverse cost was a false positive wherever a value happened to be spelled like a flag. Deciding which entry a flag consumes needs every flag's arity, and a hand-kept table of that loses findings whenever it falls behind Docker. cmd/dockerflagsgen takes the table from the flags docker/cli registers instead, so it can disagree with Docker only by being older, and a flag missing from it falls back to the previous reading — the direction that offers a rule a value it never asks for rather than skipping an entry. The generator is a module of its own so docker/cli stays out of decolint's dependencies, and CI regenerates the table and fails on a diff, which turns a Docker release that moves one of these flags into a reviewable change rather than a finding that is quietly wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reading had grown two vocabularies that sat next to each other: dockerargs.go for the whole of it, and dockerRunFlag/dockerflags_gen.go for the "docker run" flag table inside it. Neither name was wrong — the file covers "securityOpt" and capability names too, which the table does not — but the pair reads as drift. Hoisting "docker" to a package name settles it: dockerargs.Flags, dockerargs.SecurityOptEnabled and dockerargs.NormalizeCapability say whose conventions these are, so nothing inside has to repeat it and the generated table is simply runflags_gen.go. runArgsApplicable stays in rules: it answers which file type "runArgs" is meaningful in, which is devcontainer knowledge rather than Docker's, and keeping it out leaves the new package depending on hujson alone. The mount helpers in util.go stay too — parseMountString is Docker's syntax but parseMountObject is the "mounts" property's, and splitting the three would scatter one mount's reading across two packages. Behavior is unchanged. BoolFlag, FindFlagValue and NamespaceValue were reached only through the rules' tests, which no longer count toward the package holding them, so they gain direct tests; total coverage is unchanged at 94.7%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nozaq
force-pushed
the
claude/rule-expansion-proposal-1401z4
branch
from
July 29, 2026 19:49
7e47a58 to
6933938
Compare
…401z4' into claude/rule-expansion-proposal-1401z4
"runArgs": ["-itv", "/var/run/docker.sock:/var/run/docker.sock"] really does bind-mount the Docker socket, but Flags read only the first letter of a cluster, so no-docker-socket-mount reported nothing — the silent false negative the package exists to avoid. Read a "-x..." entry the way pflag's parseShortArg does: a letter at a time, each a flag of its own until one takes a value, which is the rest of the entry or the entry after it when nothing is left. A letter the flag table does not hold is still read both ways. This also drops two readings that named flags Docker has none of: "-v/src:/dst" no longer reads as a bare flag of that name, and "-eFOO=bar" no longer reads as "-eFOO" set to "bar". Reword no-host-namespace's message while here. It asserted that the container shares the namespace, which is untrue of an entry a repeat of the same option supersedes: Docker keeps the first "--network" value and the last of every other one. Every occurrence is still reported, since which one is live is a property of the order alone, but the message now says what the entry asks for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
pflag gives a value written past a "=" before it gives a bare flag its default, so "-i=0" sets "-i" to "0" rather than reading as bare. Nothing stated that, or the reading of a letter that takes a value mid-cluster. The flag table records only whether a flag may be written bare, and Flags reads a bare occurrence as "true" — which is what pflag sets every such "docker run" flag to today, but not what it would set one with an optional value of its own to. Stop the generator there rather than let the table reach decolint with that reading silently wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Every rule reading "runArgs" opened with the same ten lines: cast the node to an array, check the file type, ask dockerargs for one flag's values, loop. Eight rules carried it, no-host-namespace ran the whole argv parse seven times over for one document, and a rule that got any of it wrong would be wrong on its own. Give the engine a second traversal of that one array. Its elements are positional to JSON but a "docker run" argv to the tooling, in which a flag and its value may be one element or two and one element may name several flags, so a pattern names the flag — "/runArgs/--cap-add", "/runArgs/-v" — and the rule is handed each occurrence on Node.Flag, with Node.Value the element the value is written in. The array is now parsed once per document rather than once per rule per flag. The traversal runs only for a devcontainer.json, where "runArgs" means anything, so runArgsApplicable is gone from the rules and from util.go. Every rule test passes unchanged. dockerargs keeps what a value means; what it no longer needs to export is unexported: FlagValues and FlagValue fold into FindFlagValue, which the two rules reporting a flag's absence still ask, and BoolFlag becomes Flag.Bool now that the engine does the selecting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
A pattern matched Flag.Name as written, so "/runArgs/--volume" was not handed "-v" and a rule had to spell both. no-docker-socket-mount did; the next rule to name a value-taking flag would have had to remember, and its own tests would have passed either way. That is the silent miss the flag traversal was meant to remove, not one to leave in it. Canonicalize both sides through the flag table: a pattern's flag segment and the flag a reading is visited under both become the long form, so either spelling names the flag and one occurrence still visits a rule once. "--net" is a flag Docker registers in its own right rather than a shorthand of "--network", so it keeps its own name, and no-host-namespace covering both spellings is still covering two flags. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
Both traversals descended into the array, so its elements had two paths each — their index and the flag they set — and "/runArgs/*" matched both. A rule naming it was handed one element twice under two readings: three entries produced five visits, and the entry a value-taking flag consumes, which is a value rather than a flag, arrived as a flag anyway. Let walk stop at the array and leave its elements to walkRunArgs. Under "runArgs" a segment now names a flag and nothing else, so "/runArgs/*" is every flag the array sets. The array itself is still visited, so "/runArgs" and a root "/*" reach it, and a "runArgs" nested elsewhere in the document stays an ordinary array. No rule declared an index or a wildcard there, so nothing reported changes; the corpus and every rule test are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ
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
2 participants
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 adds four new security-focused linting rules to decolint that detect container configuration patterns that compromise isolation or grant excessive privileges:
no-dangerous-cap-add: Flags individual Linux capabilities that allow processes to reach beyond the container (e.g.,SYS_ADMIN,SYS_MODULE,NET_ADMIN)no-host-namespace: Detects when containers are placed in the host's namespaces via--network=host,--pid=host, etc.no-apparmor-unconfined: Reports when AppArmor confinement is disabled viaapparmor=unconfinedno-initialize-command: Warns aboutinitializeCommandwhich runs on the host outside container isolationKey Changes
New rule implementations (
no_dangerous_cap_add.go,no_host_namespace.go,no_apparmor_unconfined.go,no_initialize_command.go):capAdd,securityOpt,initializeCommand) andrunArgsentries where applicableUtility function enhancement (
util.go):runArgsFlagValues()to return all matching flag values instead of just the first matchrunArgsFindFlagValue()which now uses the new functionRule registration (
rules.go):Documentation (
README.md):Implementation Details
no-dangerous-cap-addrule maintains a curated list of truly dangerous capabilities (those that reach the host), deliberately excluding capabilities likeSYS_PTRACEthat stay within the container's process namespaceCAP_prefixrunArgs) and devcontainer-feature.json (which does not)runArgsentries where semantically appropriatehttps://claude.ai/code/session_01NnTsBGF2nTtG8AS1osuHvJ