Skip to content

feat(rules): add no-positional-run-arg, for a runArgs entry that displaces the image name #79

Description

@nozaq

Problem

A runArgs entry that is not an option silently breaks the container, and no rule reports it.

docker run's synopsis is docker run [OPTIONS] IMAGE [COMMAND] [ARG...] (docker/cli cli/command/container/run.go, Use:), enforced by flags.SetInterspersed(false) in the same file: the first entry that is not an option is the image name, and nothing after it is parsed as an option.

The devcontainer CLI spreads runArgs into the argument list ahead of the image and ahead of the options it derives from the configuration (devcontainers/cli src/spec-node/singleContainer.ts):

... mounts, labels, containerEnv, containerUserArgs, podmanArgs,
...config.runArgs,      ← the user's runArgs
...extraRunArgs,
...featureArgs,         ← where "privileged", "capAdd", "securityOpt" and "init" become options
...entrypoint,          ← --entrypoint /bin/sh
imageName,              ← the container's actual image
...cmd

So a bare entry in runArgs:

  1. takes the place of the image name, so Docker pulls and runs whatever that entry spells;
  2. demotes everything after it — including the --privileged, --cap-add and --security-opt derived from the configuration's own properties — to words of the container's command, so those settings are silently not applied;
  3. reaches Docker unvalidated: the CLI passes the array through to dockerPtyCLI(...args) with no checking.

The failure is either "unable to find image <entry>" or, worse, a container built from an unrelated image with none of the configured hardening.

-- in runArgs is the same defect by a different route: it ends option parsing, so the options the CLI appends after it are never applied.

Proposed rule

ID no-positional-run-arg
Description disallow a "runArgs" entry that is not an option, which Docker takes as the image name
Category correctness — the configuration does not do what it says, so this should be on by default
FileTypes linter.Devcontainer
Paths {"/runArgs"}
Platforms none (applies everywhere)

Report only the first such entry: once option parsing has ended, every later entry is a command word, and reporting each would be noise about one defect.

Suggested messages:

  • positional: "runArgs" entry %q is not an option, so it takes the place of the image name and the options after it are not applied
  • terminator: "runArgs" contains "--", which ends option parsing, so the options the tooling adds after it are not applied

Example for the rule's Example field:

// Bad
{
  "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
  "runArgs": ["--init", "bash"]
}

// Good
{
  "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
  "runArgs": ["--init"]
}

An Example.Note explaining that the container's command belongs in overrideCommand/lifecycle scripts rather than in runArgs would help, since "set the shell" is the usual reason someone puts a bare word there.

Implementation

This rule owns its own walk. runArgsFlags in rules/dockerargs.go deliberately does not model argument position: it reads every entry and returns each way an entry could be read, because tracking which flag consumes which entry needs a table of every docker run flag's arity, and a flag missing from that table loses findings silently (see 7e22ddb, and cb39acf for the bug that prompted it).

That trade-off is right for the security rules, which ask "is this option set anywhere", but it is the wrong shape for this rule, which is about parsing order. Give it a small local walk that answers only its own question — where does option parsing end — rather than reintroducing arity into the shared helpers:

  • an entry that is exactly -- ends parsing;
  • an entry not starting with - ends parsing and is the image name;
  • everything before that is options.

Arity does affect precision here (["--name", "bash"] is a container name, not an image name), so accept a small false-positive risk in the same direction the rest of the file already does, and say so in the rule's own comment. Reporting a bare word that is really another flag's value is a finding on a configuration nobody should have written either way.

Tests

rules/no_positional_run_arg_test.go, table-driven in the style of the neighbouring rule tests:

runArgs reported
["--init", "--privileged"] no
["--cap-add=SYS_PTRACE"] no
["--init", "bash"] yes, at "bash"
["bash", "--init"] yes, at "bash" only
["--init", "bash", "sleep"] yes, at "bash" only
["--", "--privileged"] yes, at "--"
[123, "--init"] no — a non-string entry is invalid rather than positional, and the shared walk skips it (2fd89c3)
["--cap-add", "SYS_PTRACE"] decide and document: without arity this reads as a positional. Listing the handful of flags whose values are bare words, or accepting the finding, are both defensible — say which and why

Also needed: registration in rules/rules.go. The docs and dogfooding CI jobs cover the generated rule page and the repository's own configuration.

References

Context

Came out of the Docker argument-handling verification in #78, which established the behaviour this rule reports but left the rule itself out of scope.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions