Add dmr sandbox config and launch command by CoderHariswar · Pull Request #922 · docker/model-runner

CoderHariswar · 2026-05-19T12:16:01Z

Fixes #915

Describe the changes you have made in this PR -

This PR adds support for configuring and launching tools through a sandbox from the dmr wrapper.

Users can now configure the sandbox tool with:

dmr config sandbox.tool sbx

The above command writes TOML-style config

Manual Testing done using the following commands

tmpdir="$(mktemp -d)"
XDG_CONFIG_HOME="$tmpdir/config" ./dmr config sandbox.tool sbx
cat "$tmpdir/config/dmr/config.toml"

Tested launch through a fake sandbox command and outputs have been verified successfully

sourcery-ai

Hey - I've found 1 security issue

Security issues:

Detected non-static command inside Command. Audit the input to 'exec.Command'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code. (link)

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="cmd/dmr/main.go" line_range="113" />
<code_context>
	cmd := exec.Command(sandboxTool, args...)
</code_context>
<issue_to_address>
**security (go.lang.security.audit.dangerous-exec-command):** Detected non-static command inside Command. Audit the input to 'exec.Command'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds lightweight dmr config / dmr launch behaviors to the dmr CLI by persisting a sandbox tool in a user config file and executing it via os/exec.

Changes:

Added early os.Args dispatch for config and launch commands.
Implemented writing/reading sandbox.tool in a TOML-like config under the user config dir.
Added a launcher that execs the configured sandbox tool with forwarded stdio.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gemini-code-assist

Code Review

This pull request introduces new config and launch commands to manage a sandbox tool configuration. However, the current implementation contains several critical issues: the manual command handling in run() shadows existing Cobra subcommands, creating a regression; the configuration writing logic is destructive and will overwrite existing file content; and the configuration parser fails to correctly handle escaped characters in quoted strings. The reviewer recommends integrating these features into the existing Cobra command tree and using strconv.Unquote for reliable string parsing.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

sathiraumesh · 2026-05-19T14:46:34Z

+	if key != "sandbox.tool" {
+		return fmt.Errorf("unsupported config key %q", key)
+	}
+


perhaps much better if we can add a list of supported tools in the top of the file and only support here like

var allowedSandboxTools = map[string]struct{}{ "firejail": {}, "bubblewrap": {}, "bwrap": {}, }

if _, ok := allowedSandboxTools[value]; !ok { return fmt.Errorf("unsupported sandbox tool %q", sandboxTool) }

Sure but lets just start with sbx for this PR, these can be follow on PRs

@ericcurtin yes, this was my exact point, like it's better if we can specify the supported ones, both here and the launch

sathiraumesh · 2026-05-19T14:48:20Z

+	if err != nil {
+		return err
+	}
+


same thing like above can be done here for verifying supported tools

sathiraumesh · 2026-05-19T14:51:23Z

@CoderHariswar something i noticed when reviewing is. Is there any reason you didn't use the same pattern in the implementation like registering a subcommand like the serveCmd here in this file.

ericcurtin · 2026-05-19T14:58:39Z

We must ensure this works for model-cli and dmr. And test, test, test to see this works as expected

sourcery-ai

New security issues found

CoderHariswar · 2026-05-20T17:18:27Z

Thanks for the review @sathiraumesh

I was confused at first because config was already an alias for configure, so dmr config sandbox.tool sbx was getting routed to the existing configure command.

My first fix handled config directly inside cmd/dmr, but after I got stuck and asked GPT for help, I understood that this was not the right pattern here. @ericcurtin also mentioned that this should work for both model-cli and dmr.

I updated the PR to follow that approach:

moved the config support into the shared cmd/cli/commands package
registered it as a normal Cobra subcommand
kept dmr using commands.NewRootCmd
removed the old config alias from configure
restricted supported sandbox tools to sbx for this PR
added validation during config and launch
updated tests for config and launch behavior

Tested locally with:

go test ./cmd/cli/commands
go test ./cmd/dmr
go build -o dmr ./cmd/dmr

I also manually tested dmr config sandbox.tool sbx and dmr launch opencode with a fake sbx script.

ericcurtin · 2026-05-20T17:37:53Z

Thanks for the review @sathiraumesh

I was confused at first because config was already an alias for configure, so dmr config sandbox.tool sbx was getting routed to the existing configure command.

My first fix handled config directly inside cmd/dmr, but after I got stuck and asked GPT for help, I understood that this was not the right pattern here. @ericcurtin also mentioned that this should work for both model-cli and dmr.

I updated the PR to follow that approach:

moved the config support into the shared cmd/cli/commands package

registered it as a normal Cobra subcommand

kept dmr using commands.NewRootCmd

removed the old config alias from configure

restricted supported sandbox tools to sbx for this PR

added validation during config and launch

updated tests for config and launch behavior

Tested locally with:

go test ./cmd/cli/commands go test ./cmd/dmr go build -o dmr ./cmd/dmr

I also manually tested dmr config sandbox.tool sbx and dmr launch opencode with a fake sbx script.

We want to do a brand new "config", "git config"-style, using toml, the old "configure" we should deprecate it's kinda strange, it only lives as long as the daemon

CoderHariswar · 2026-05-20T17:41:29Z

Got it @ericcurtin for this PR I focused on sandbox.tool with sbx as the only supported sandbox tool.

sathiraumesh · 2026-05-21T09:05:48Z

-		Short:   "Manage model runtime configurations",
-		Hidden:  true,
+		Use:    "configure [--context-size=<n>] [--speculative-draft-model=<model>] [--hf_overrides=<json>] [--gpu-memory-utilization=<float>] [--mode=<mode>] [--think] [--keep-alive=<duration>] MODEL [-- <runtime-flags...>]",
+		Short:  "Manage model runtime configurations",


I was hoping this wouldn't be removed we could keep this like this without removing if not it would break the style of dmr config or docker model config style .

What i suggest is we register it as a sub command in config. so one can do dmr config sandbox.tool ... or docker model config sandbox.tool

like c.AddCommand(newSandboxConfigCmd()) (note that sandbox.tool is not lke a cmd style format but not sure in this case there is any other approach)

sathiraumesh · 2026-05-21T09:10:48Z

 			if ca, ok := containerApps[app]; ok {
 				return launchContainerApp(cmd, ca, ep.container, image, port, detach, appArgs, dryRun)
 			}
+


can remove these line space changes which i think is not part of changes

sathiraumesh · 2026-05-21T09:18:06Z

 }

+func launchSandboxedHostApp(cmd *cobra.Command, sandboxTool, app string, appArgs []string, dryRun bool) error {
+	if err := validateSandboxTool(sandboxTool); err != nil {


much better if we can retrieve so everything related to the sandbox happens inside the sandbox.go

err, validatedSbxTool = `validateSandboxTool(sandboxTool) if err != nil { return err }

Then remove the switch ... and directly

if dryRun { cmd.Printf("sbx %s\n", strings.Join(args, " ")) return nil } launchCmd := exec.Command("sbx", args...) launchCmd.Stdin = os.Stdin launchCmd.Stdout = os.Stdout launchCmd.Stderr = os.Stderr return launchCmd.Run()

sathiraumesh · 2026-05-21T09:19:05Z

+	}
+}
+
+func validateSandboxTool(tool string) error {


can change as above mentioned suggestions

sathiraumesh · 2026-05-21T11:07:41Z

 		newShowCmd(),
 		newComposeCmd(),
 		newLaunchCmd(),
+		newSandboxConfigCmd(),


we don't need to do this if we do as a configs sub command

Should be newConfigCmd, this will be a generic config system, sandbox is just one of the things

sathiraumesh · 2026-05-21T11:14:15Z

Probably you might need to check if sbx is not configured it fail tests locally. When i tried in my computer it did

Add dmr sandbox config and launch command

2be8943

Copilot AI review requested due to automatic review settings May 19, 2026 12:16

sourcery-ai Bot reviewed May 19, 2026

View reviewed changes

Comment thread cmd/dmr/main.go Outdated

Copilot AI reviewed May 19, 2026

View reviewed changes

Comment thread cmd/dmr/main.go Outdated

Comment thread cmd/dmr/main.go Outdated

Comment thread cmd/dmr/main.go Outdated

Comment thread cmd/dmr/main.go Outdated

gemini-code-assist Bot reviewed May 19, 2026

View reviewed changes

Comment thread cmd/dmr/main.go Outdated

Comment thread cmd/dmr/main.go Outdated

Comment thread cmd/dmr/main.go Outdated

Comment thread cmd/dmr/main.go Outdated

Potential fix for pull request finding

934299f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

sathiraumesh suggested changes May 19, 2026

View reviewed changes

CoderHariswar added 2 commits May 20, 2026 22:37

Add dmr sandbox config and launch command

e1e3fb1

Move sandbox config and launch to shared CLI commands

064d96b

sourcery-ai Bot reviewed May 20, 2026

View reviewed changes

Comment thread cmd/cli/commands/launch.go Outdated

CoderHariswar requested a review from sathiraumesh May 20, 2026 17:28

Use static sandbox command for sbx launch

93c3726

Update CLI reference docs

6334d03

sathiraumesh reviewed May 21, 2026

View reviewed changes

Conversation

CoderHariswar commented May 19, 2026

Describe the changes you have made in this PR -

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathiraumesh commented May 19, 2026

Uh oh!

ericcurtin commented May 19, 2026

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CoderHariswar commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericcurtin commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CoderHariswar commented May 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathiraumesh commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CoderHariswar commented May 20, 2026 •

edited

Loading

ericcurtin commented May 20, 2026 •

edited

Loading

sathiraumesh commented May 21, 2026 •

edited

Loading