Skip to content

fix(server): Shell escape docker login args (backport #534)#536

Merged
ssiyad merged 3 commits into
masterfrom
mergify/bp/master/pr-534
Jun 10, 2026
Merged

fix(server): Shell escape docker login args (backport #534)#536
ssiyad merged 3 commits into
masterfrom
mergify/bp/master/pr-534

Conversation

@mergify

@mergify mergify Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

This is an automatic backport of pull request #534 done by [Mergify](https://mergify.com).

@mergify mergify Bot assigned ssiyad Jun 9, 2026
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

Applies shlex.quote to all user-controlled arguments interpolated into docker CLI commands executed via subprocess.Popen(..., shell=True), closing shell injection vectors across docker login, docker pull, docker push, docker inspect, docker rmi, docker rm, and related grep pipelines.

  • All changed call sites in server.py correctly wrap dynamic arguments with shlex.quote before f-string interpolation into shell commands.
  • docker login now quotes the registry URL, username, and password; docker_inspect_manifest, docker inspect, docker rm, docker rmi, docker push, and docker pull each quote their respective image/container name arguments.

Confidence Score: 5/5

Straightforward, targeted shell-escaping fix with no logic changes — safe to merge.

Every dynamic argument passed to a shell=True docker command is now wrapped with shlex.quote. The fix is mechanical and complete across all affected call sites, with no new logic introduced.

No files require special attention.

Important Files Changed

Filename Overview
agent/server.py Adds shlex.quote to all docker command arguments (login credentials, image tags, container names, bench names) to prevent shell injection via the shell=True subprocess execution path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[docker command with user-controlled arg] --> B{shlex.quote applied?}
    B -- Before PR: No --> C[f-string interpolated directly into shell command]
    C --> D[subprocess.Popen shell=True]
    D --> E[Shell injection possible]
    B -- After PR: Yes --> F[arg shell-escaped before interpolation]
    F --> D
    D --> G[Shell injection prevented]
Loading

Reviews (2): Last reviewed commit: "fix(server): add missing shlex import" | Re-trigger Greptile

@ssiyad

ssiyad commented Jun 10, 2026

Copy link
Copy Markdown
Member

@copilot

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.


Issue 1 of 1

agent/server.py:3-10
shlex is used in every changed call but is never imported in this file. Every shlex.quote() call will raise NameError: name 'shlex' is not defined at runtime, meaning docker login, push, pull, inspect, rm, rmi, and image-ls operations will all crash instead of running. The security fix is entirely non-functional without this import.

import contextlib
import json
import os
import platform
import shlex
import shutil
import subprocess
import tempfile
import time

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown

@copilot

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
...

Fixed in the latest commit — import shlex has been added to agent/server.py.

Copilot AI requested a review from ssiyad June 10, 2026 07:38
@ssiyad ssiyad merged commit 135ed5a into master Jun 10, 2026
3 checks passed
@ssiyad ssiyad deleted the mergify/bp/master/pr-534 branch June 10, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants