fix(enter): parse distrobox flags after the container name (#2165)

`distrobox enter my-box --help` and any distrobox flag placed after the
container name, was passed to the container manager as the command instead
of being parsed, so it failed with a crun error. This diverged from the
original bash distrobox-enter, which keeps parsing flags after the name and
only starts the command at -e/--exec/--.

The port stopped flag parsing at the first positional (StopOnNthArg), which
cannot tell the name from the command: when --name or DBX_CONTAINER_NAME
already supplies the name, the first positional *is* the command. So the
command boundary is resolved before parsing and everything past it is handed
to urfave verbatim, letting flags be recognized wherever they appear while
the custom command is still passed through untouched. ephemeral gets the
same treatment.

Unknown flags after the name are rejected, as in bash, so a mistyped flag
surfaces instead of being silently executed inside the container.

Fixes https://github.com/89luca89/distrobox/issues/2160

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
This commit is contained in:
Luca Di Maio
2026-07-18 17:00:12 +02:00
committed by GitHub
parent 17216a5d19
commit 8561affccb
8 changed files with 410 additions and 137 deletions
+5 -1
View File
@@ -30,6 +30,10 @@ func run() error {
cmd := cli.NewRootCommand(cfg)
// PrepareArgs lets `enter`/`ephemeral` accept distrobox flags after the
// container name by splicing a "--" in front of the custom command.
args := cli.PrepareArgs(cmd, cli.ResolveArgs(os.Args))
//nolint:wrapcheck // main reports errors as-is
return cmd.Run(ctx, cli.ResolveArgs(os.Args))
return cmd.Run(ctx, args)
}
+5 -24
View File
@@ -68,7 +68,6 @@ func newEnterCommand(cfg *config.Values) *cli.Command {
},
},
UseShortOptionHandling: false,
StopOnNthArg: ptr(1),
SkipFlagParsing: false,
Action: func(ctx context.Context, cmd *cli.Command) error {
return enterAction(ctx, cmd, cfg)
@@ -82,36 +81,18 @@ func enterAction(ctx context.Context, cmd *cli.Command, cfg *config.Values) erro
return errors.New("container manager not found in context")
}
// Container name: --name flag takes priority, otherwise first positional arg.
// Everything after the container name (or after --) is the custom command.
//
// The CLI is configured with StopOnNthArg: 1, so urfave/cli stops flag
// parsing as soon as the first positional arg is seen. The trailing
// positional args (which include the custom command and any -e/--exec
// marker that came after the container name) are returned verbatim.
// --name (or DBX_CONTAINER_NAME) wins; otherwise the first positional is
// the name. PrepareArgs (parse.go) already split the command off behind a
// "--", so the tail is [name?] + command.
containerName := cmd.String("name")
args := cmd.Args().Slice()
// If the user placed -e/--exec AFTER the container name, it lands in
// the positional tail. In that case the first positional arg is still
// the container name and the custom command starts right after the
// marker. When the marker is consumed as a flag (i.e. it appeared
// before the container name) the tail is just the custom command.
markerIndex := findExecMarkerIndex(args)
var customCommand []string
switch {
case markerIndex >= 0:
// -e/--exec was placed after the container name.
if containerName == "" {
containerName = args[0]
}
customCommand = args[markerIndex+1:]
case containerName == "" && len(args) > 0:
if containerName == "" && len(args) > 0 {
containerName = args[0]
customCommand = args[1:]
default:
} else {
customCommand = args
}
+80 -67
View File
@@ -2,6 +2,7 @@ package cli
import (
"context"
"io"
"strings"
"sync"
"testing"
@@ -81,7 +82,9 @@ func runEnter(t *testing.T, argv ...string) containermanager.EnterOptions {
}
root := &cli.Command{Commands: []*cli.Command{cmd}}
full := append([]string{"distrobox"}, argv...)
// The real entrypoint (cmd/distrobox/main.go) always runs argv through
// PrepareArgs before handing it to urfave; mirror that here.
full := PrepareArgs(root, append([]string{"distrobox"}, argv...))
require.NoError(t, root.Run(context.Background(), full))
require.NotEmpty(t, spy.calls, "expected Enter to be called for argv %v", argv)
@@ -162,72 +165,82 @@ func TestEnterCommand_CustomCommandVariants(t *testing.T) {
}
}
func TestFindExecMarkerIndex(t *testing.T) {
cases := []struct {
name string
args []string
want int
}{
{
name: "empty args",
args: nil,
want: -1,
},
{
name: "no marker",
args: []string{"suse", "echo", "ciao"},
want: -1,
},
{
name: "short -e is the only arg",
args: []string{"-e"},
want: 0,
},
{
name: "long --exec is the only arg",
args: []string{"--exec"},
want: 0,
},
{
name: "-e at the start",
args: []string{"-e", "bash", "-c", "echo"},
want: 0,
},
{
name: "--exec in the middle",
args: []string{"suse", "--exec", "bash", "-c", "echo"},
want: 1,
},
{
name: "-e at the end with no command after it",
args: []string{"suse", "-e"},
want: 1,
},
{
name: "first match wins when both forms are present",
args: []string{"-e", "suse", "--exec", "echo"},
want: 0,
},
{
name: "marker-looking arg inside the custom command is not a match",
// `bash` happens to start with `b`, not `-`, so it must
// never be picked up. This guards against a future
// regression that would substring-match.
args: []string{"suse", "bash", "-e", "echo"},
want: 2,
},
{
name: "looks similar but is not the marker",
// `--exec-foo` shares a prefix with `--exec` but is a
// distinct token; it must not be treated as the marker.
args: []string{"suse", "--exec-foo", "echo"},
want: -1,
},
// TestEnterCommand_FlagsAfterContainerName is the regression suite for the
// bug this change fixes: distrobox flags placed after the bare container name
// must be parsed as distrobox flags (not swallowed into the custom command),
// mirroring the original bash distrobox-enter.
func TestEnterCommand_FlagsAfterContainerName(t *testing.T) {
t.Run("additional-flags after name is consumed", func(t *testing.T) {
opts := runEnter(t, "enter", "suse", "--additional-flags", "--env FOO=bar", "--", "printenv", "FOO")
assert.Equal(t, "suse", opts.ContainerName)
assert.Equal(t, "--env FOO=bar", opts.AdditionalFlags)
assert.Equal(t, []string{"printenv", "FOO"}, opts.CustomCommand)
})
t.Run("short -a after name, implicit command", func(t *testing.T) {
opts := runEnter(t, "enter", "suse", "-a", "--pids-limit 100", "bash")
assert.Equal(t, "suse", opts.ContainerName)
assert.Equal(t, "--pids-limit 100", opts.AdditionalFlags)
assert.Equal(t, []string{"bash"}, opts.CustomCommand)
})
t.Run("--no-tty after name is consumed", func(t *testing.T) {
opts := runEnter(t, "enter", "suse", "--no-tty", "--", "bash")
assert.Equal(t, "suse", opts.ContainerName)
assert.True(t, opts.NoTTY)
assert.Equal(t, []string{"bash"}, opts.CustomCommand)
})
// Regression for the naive StopOnNthArg:2 fix: when --name supplies the
// name, the command's own short flag (-c) must not be parsed as
// --clean-path.
t.Run("--name then command with short flag", func(t *testing.T) {
opts := runEnter(t, "enter", "--name", "suse", "bash", "-c", "echo")
assert.Equal(t, "suse", opts.ContainerName)
assert.False(t, opts.CleanPath)
assert.Equal(t, []string{"bash", "-c", "echo"}, opts.CustomCommand)
})
// A flag that follows the command word belongs to the command.
t.Run("flag after command word stays in the command", func(t *testing.T) {
opts := runEnter(t, "enter", "suse", "vim", "--help")
assert.Equal(t, "suse", opts.ContainerName)
assert.Equal(t, []string{"vim", "--help"}, opts.CustomCommand)
})
}
// runEnterRaw runs enter through PrepareArgs and returns the spy plus the Run
// error, without asserting either — for cases that intentionally stop before
// Enter (help, invalid flag).
func runEnterRaw(t *testing.T, argv ...string) (*spyContainerManager, error) {
t.Helper()
spy := &spyContainerManager{existsResult: true}
cmd := newEnterCommand(config.DefaultValues())
cmd.Before = func(ctx context.Context, _ *cli.Command) (context.Context, error) {
return context.WithValue(ctx, containerManagerKey, spy), nil
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.want, findExecMarkerIndex(tc.args))
})
}
root := &cli.Command{Commands: []*cli.Command{cmd}, Writer: io.Discard, ErrWriter: io.Discard}
full := PrepareArgs(root, append([]string{"distrobox"}, argv...))
return spy, root.Run(context.Background(), full)
}
// TestEnterCommand_NoEnterAfterName pins down that a recognized help flag
// after the name shows help, and an unrecognized flag after the name is
// rejected — in both cases without entering the container.
func TestEnterCommand_NoEnterAfterName(t *testing.T) {
t.Run("--help after name shows help, does not enter", func(t *testing.T) {
spy, err := runEnterRaw(t, "enter", "suse", "--help")
require.NoError(t, err)
assert.Empty(t, spy.calls, "expected help, not Enter")
})
t.Run("unknown flag after name errors, does not enter", func(t *testing.T) {
spy, err := runEnterRaw(t, "enter", "suse", "--frobnicate")
require.Error(t, err)
assert.Contains(t, err.Error(), "not defined")
assert.Empty(t, spy.calls, "expected error, not Enter")
})
}
+6 -24
View File
@@ -56,16 +56,10 @@ Examples:
distrobox ephemeral --root --image fedora:39
distrobox ephemeral -- bash -c "echo hello"`,
Flags: flags,
// StopOnNthArg: 1 mirrors the semantics of the original bash
// distrobox-ephemeral: every flag must appear before the first
// positional arg, and everything from the first positional arg
// onward is treated as the custom command. Without this, short
// flags inherited from `distrobox-create` (e.g. -c for --clone)
// would be eaten out of the custom command. The bare `--`
// separator still works because urfave/cli checks for it before
// honouring StopOnNthArg.
// PrepareArgs (parse.go) splits the command off behind a "--", so
// inherited create flags (e.g. -c/--clone) are parsed wherever they
// appear instead of being eaten out of the custom command.
UseShortOptionHandling: false,
StopOnNthArg: ptr(1),
SkipFlagParsing: false,
Action: func(ctx context.Context, cmd *cli.Command) error {
return ephemeralAction(ctx, cmd, cfg)
@@ -79,21 +73,9 @@ func ephemeralAction(ctx context.Context, cmd *cli.Command, cfg *config.Values)
return errors.New("container manager not found in context")
}
// The CLI is configured with StopOnNthArg: 1, so urfave/cli stops
// flag parsing as soon as it sees the first positional arg. From
// that point on, anything — including the -e/--exec marker, if the
// user placed it after a positional arg — is captured verbatim into
// the positional tail. We use findExecMarkerIndex to split the
// custom command out of the tail.
args := cmd.Args().Slice()
markerIndex := findExecMarkerIndex(args)
var customCommand []string
if markerIndex >= 0 {
customCommand = args[markerIndex+1:]
} else {
customCommand = args
}
// ephemeral has no positional name, so after PrepareArgs the whole
// positional tail is the custom command.
customCommand := cmd.Args().Slice()
opts := commands.EphemeralOptions{
CreateOptions: commands.CreateOptions{
+2 -1
View File
@@ -31,7 +31,8 @@ func runEphemeral(t *testing.T, argv ...string) containermanager.EnterOptions {
}
root := &cli.Command{Commands: []*cli.Command{cmd}}
full := append([]string{"distrobox"}, argv...)
// Mirror the real entrypoint, which runs argv through PrepareArgs.
full := PrepareArgs(root, append([]string{"distrobox"}, argv...))
require.NoError(t, root.Run(context.Background(), full))
require.NotEmpty(t, spy.calls, "expected Enter to be called for argv %v", argv)
+179 -12
View File
@@ -1,17 +1,184 @@
package cli
// findExecMarkerIndex returns the index of the first -e or --exec in
// args, or -1 if neither is present. It is used by the enter and
// ephemeral commands to recover the marker when the user placed it
// after the container name (or, for ephemeral, after the first
// positional arg) — in those cases urfave/cli leaves the marker in
// the positional tail instead of consuming it as a flag, and we need
// to know where the custom command starts.
func findExecMarkerIndex(args []string) int {
for i, arg := range args {
if arg == "-e" || arg == "--exec" {
return i
import (
"strings"
"github.com/urfave/cli/v3"
)
// PrepareArgs lets `enter` and `ephemeral` accept distrobox flags after the
// container name, like the bash distrobox-enter (which only starts the command
// at -e/--exec/--). urfave's StopOnNthArg can't express this: it stops at the
// first positional and can't tell the name from the command. So we find the
// command ourselves and splice a bare "--" in front of it, then let urfave
// parse the rest. It runs at the entrypoint because urfave has no pre-parse hook.
func PrepareArgs(root *cli.Command, args []string) []string {
if len(args) < 2 {
return args
}
// Completion appends this flag last; splicing a "--" ahead of it would
// hide it from urfave.
if args[len(args)-1] == "--generate-shell-completion" {
return args
}
// The sub-command can be preceded by global flags (e.g. --verbose).
globalArity := flagArity(root.Flags)
i := 1
for i < len(args) && args[i] != "--" && isFlagToken(args[i]) {
base, hasValue := flagBaseName(args[i])
i++
if globalArity[base] && !hasValue {
i++ // also skip the flag's value
}
}
return -1
if i >= len(args) {
return args
}
sub := root.Command(args[i])
if sub == nil || (sub.Name != "enter" && sub.Name != "ephemeral") {
return args
}
// Scan with the sub-command's flags plus the inherited globals (--verbose).
flags := append(append([]cli.Flag{}, sub.Flags...), root.Flags...)
// enter takes the first bare token as the name, unless one is already
// supplied via --name's default (DBX_CONTAINER_NAME/config). ephemeral has
// no positional name, so its first bare token is already the command.
nameSeen := true
if sub.Name == "enter" {
nameSeen = nameFlagDefault(sub.Flags) != ""
}
tail, help := splitExecCommand(flags, args[i+1:], nameSeen)
out := append([]string{}, args[:i+1]...)
if help {
// urfave would treat the container name as a help topic ("No help
// topic for '<name>'"), so drop it and just show the sub-command help.
return append(out, "--help")
}
return append(out, tail...)
}
// splitExecCommand splices a bare "--" in front of the custom command so urfave
// hands it through untouched, and reports whether help was requested. Args are
// returned unchanged when there is no command — including an unrecognized flag,
// which is left for urfave to reject.
//
// nameSeen is true when a name is already set without a positional (ephemeral,
// or enter with DBX_CONTAINER_NAME), so the first bare token is the command.
func splitExecCommand(flags []cli.Flag, args []string, nameSeen bool) ([]string, bool) {
arity := flagArity(flags)
for i := 0; i < len(args); {
arg := args[i]
switch {
case arg == "--": // urfave already isolates the command here
return args, false
case isExecMarker(arg):
// -e/--exec open the command, but before the name they are a
// harmless no-op flag that urfave consumes.
if nameSeen {
return spliceSeparator(args, i, true), false
}
i++
case isHelpToken(arg):
return nil, true
case isFlagToken(arg):
base, hasValue := flagBaseName(arg)
takesValue, known := arity[base]
if !known {
return args, false
}
if base == "name" || base == "n" {
nameSeen = true
}
i++
if takesValue && !hasValue {
i++ // skip the flag's value
}
case !nameSeen:
nameSeen = true // the first bare token is the container name
i++
default:
return spliceSeparator(args, i, false), false // command starts here
}
}
return args, false
}
// spliceSeparator inserts a "--" at index i so the args from there on are the
// verbatim command. dropMarker replaces the token at i (an -e/--exec marker)
// rather than keeping it, since the marker itself is not part of the command.
func spliceSeparator(args []string, i int, dropMarker bool) []string {
out := append([]string{}, args[:i]...)
out = append(out, "--")
if dropMarker {
return append(out, args[i+1:]...)
}
return append(out, args[i:]...)
}
// flagArity maps each flag name (long and short) to whether it consumes a
// following value, so the scanner can skip that value instead of mistaking it
// for the container name or command.
func flagArity(flags []cli.Flag) map[string]bool {
arity := make(map[string]bool)
for _, f := range flags {
takesValue := false
if dg, ok := f.(cli.DocGenerationFlag); ok {
takesValue = dg.TakesValue()
}
for _, name := range f.Names() {
arity[name] = takesValue
}
}
return arity
}
// nameFlagDefault returns the --name flag's default value, which config/env
// (DBX_CONTAINER_NAME) populate, or "" if unset.
func nameFlagDefault(flags []cli.Flag) string {
for _, f := range flags {
sf, ok := f.(*cli.StringFlag)
if ok && sf.Name == "name" {
return sf.Value
}
}
return ""
}
// flagBaseName strips leading dashes and any "=value" suffix, returning the
// bare flag name and whether an inline value was present.
func flagBaseName(tok string) (string, bool) {
base := strings.TrimLeft(tok, "-")
if name, _, found := strings.Cut(base, "="); found {
return name, true
}
return base, false
}
func isExecMarker(tok string) bool { return tok == "-e" || tok == "--exec" }
func isHelpToken(tok string) bool {
base, _ := flagBaseName(tok)
return isFlagToken(tok) && (base == "help" || base == "h")
}
// isFlagToken reports whether tok is a flag: a dash followed by another dash or
// a letter. "-", "--" and negative numbers ("-9") are not flags.
func isFlagToken(tok string) bool {
return len(tok) > 1 && tok[0] == '-' && (tok[1] == '-' || isASCIILetter(tok[1]))
}
func isASCIILetter(b byte) bool {
return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z')
}
+133
View File
@@ -0,0 +1,133 @@
package cli
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/urfave/cli/v3"
"github.com/89luca89/distrobox/pkg/config"
)
// TestPrepareArgs exercises the argv rewrite against the real, fully composed
// root command (so the flag sets and arities match production). It asserts
// where a bare "--" is spliced in to isolate the custom command, and that the
// distrobox flags placed after the container name are left for urfave to
// parse rather than swallowed into the command.
func TestPrepareArgs(t *testing.T) {
root := NewRootCommand(config.DefaultValues())
cases := []struct {
name string
in []string
want []string
}{
{
name: "flag after name is consumed, bare word opens command",
in: []string{"distrobox", "enter", "suse", "--additional-flags", "foo", "vim", "arg"},
want: []string{"distrobox", "enter", "suse", "--additional-flags", "foo", "--", "vim", "arg"},
},
{
name: "help after name drops the name so urfave shows command help",
in: []string{"distrobox", "enter", "suse", "--help"},
want: []string{"distrobox", "enter", "--help"},
},
{
name: "short -h after name behaves the same",
in: []string{"distrobox", "enter", "suse", "-h"},
want: []string{"distrobox", "enter", "--help"},
},
{
name: "implicit command",
in: []string{"distrobox", "enter", "suse", "echo", "ciao"},
want: []string{"distrobox", "enter", "suse", "--", "echo", "ciao"},
},
{
name: "-e after name becomes --",
in: []string{"distrobox", "enter", "suse", "-e", "bash", "-c", "echo"},
want: []string{"distrobox", "enter", "suse", "--", "bash", "-c", "echo"},
},
{
name: "existing -- is left untouched",
in: []string{"distrobox", "enter", "suse", "--", "bash", "-c", "echo"},
want: []string{"distrobox", "enter", "suse", "--", "bash", "-c", "echo"},
},
{
name: "--name then command with short flag (regression)",
in: []string{"distrobox", "enter", "--name", "suse", "bash", "-c", "echo"},
want: []string{"distrobox", "enter", "--name", "suse", "--", "bash", "-c", "echo"},
},
{
// A marker before the name is a no-op: it is left in place (urfave
// consumes -e/--exec as a bool flag) and the first bare token is
// still the name.
name: "-e before name is a no-op, first bare token is the name",
in: []string{"distrobox", "enter", "-e", "suse", "bash", "-c", "echo"},
want: []string{"distrobox", "enter", "-e", "suse", "--", "bash", "-c", "echo"},
},
{
name: "command word before its own flag",
in: []string{"distrobox", "enter", "suse", "vim", "--help"},
want: []string{"distrobox", "enter", "suse", "--", "vim", "--help"},
},
{
name: "unknown flag after name is left for urfave to reject",
in: []string{"distrobox", "enter", "suse", "--frobnicate"},
want: []string{"distrobox", "enter", "suse", "--frobnicate"},
},
{
name: "global flag before the subcommand",
in: []string{"distrobox", "--verbose", "enter", "suse", "bash"},
want: []string{"distrobox", "--verbose", "enter", "suse", "--", "bash"},
},
{
name: "inherited --verbose after the name is consumed",
in: []string{"distrobox", "enter", "suse", "--verbose", "bash"},
want: []string{"distrobox", "enter", "suse", "--verbose", "--", "bash"},
},
{
name: "shell completion flag is never touched",
in: []string{"distrobox", "enter", "suse", "--generate-shell-completion"},
want: []string{"distrobox", "enter", "suse", "--generate-shell-completion"},
},
{
name: "ephemeral implicit command (no positional name)",
in: []string{"distrobox", "ephemeral", "--image", "alpine", "cat", "/etc/os-release"},
want: []string{"distrobox", "ephemeral", "--image", "alpine", "--", "cat", "/etc/os-release"},
},
{
name: "ephemeral -e becomes --",
in: []string{"distrobox", "ephemeral", "-e", "bash", "-c", "echo"},
want: []string{"distrobox", "ephemeral", "--", "bash", "-c", "echo"},
},
{
name: "non-exec subcommand is left untouched",
in: []string{"distrobox", "list", "--verbose"},
want: []string{"distrobox", "list", "--verbose"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.want, PrepareArgs(root, tc.in))
})
}
}
// TestPrepareArgs_EnvContainerName covers the DBX_CONTAINER_NAME path: when a
// name is already supplied via the --name flag's default, enter must not
// consume the first bare token as the name — it is the command.
func TestPrepareArgs_EnvContainerName(t *testing.T) {
enter := &cli.Command{
Name: "enter",
Flags: []cli.Flag{
&cli.StringFlag{Name: "name", Aliases: []string{"n"}, Value: "envbox"},
&cli.BoolFlag{Name: "clean-path", Aliases: []string{"c"}},
&cli.BoolFlag{Name: "exec", Aliases: []string{"e"}},
},
}
root := &cli.Command{Name: "distrobox", Commands: []*cli.Command{enter}}
got := PrepareArgs(root, []string{"distrobox", "enter", "bash", "-c", "echo"})
assert.Equal(t, []string{"distrobox", "enter", "--", "bash", "-c", "echo"}, got)
}
-8
View File
@@ -1,8 +0,0 @@
package cli
// ptr returns a pointer to v. Go has no built-in syntax to take the
// address of a literal, so we need this tiny helper to populate pointer
// fields like urfave/cli's StopOnNthArg (a *int) — the zero value of a
// pointer is nil, and nil keeps the default behaviour, so the call site
// has to be explicit about wanting a non-nil pointer.
func ptr[T any](v T) *T { return &v }