Skip to content

Harden docker-sbx install: remove curl|sudo sh root install #62991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 6 commits into from
Closed

Harden docker-sbx install: remove curl|sudo sh root install #62991

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cca9839
Initial plan
Copilot Sep 23, 2026
daab099
Harden sudo_docker_sbx_install.sh against curl|sudo sh supply-chain risk
Copilot Sep 23, 2026
63b999e
Merge branch 'main' into copilot/setup-security-fix
pelikhan Sep 23, 2026
820506d
Verify Docker repository signing key
Copilot Sep 23, 2026
56149e1
Merge branch 'main' into copilot/setup-security-fix
pelikhan Sep 23, 2026
3c476df
Validate all Docker key fingerprints
Copilot Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/harden-docker-sbx-install-apt-repo.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 36 additions & 2 deletions actions/setup/sh/sudo_docker_sbx_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,46 @@ set +o histexpand
#
# Usage: sudo_docker_sbx_install.sh
# No arguments required.
#
# Key notes:
# - Adds the Docker apt repository deterministically (pinned GPG key fetched to a
# file, signed-by apt source line) instead of piping a convenience script into
# a root shell. See:
# https://docs.docker.com/engine/install/ubuntu/#install-using-the-repository

set -euo pipefail

echo "::group::Install docker-sbx"
# Add Docker apt repo without installing Docker Engine (already present).
curl -fsSL https://get.docker.com | sudo REPO_ONLY=1 sh
KEYRING_PATH="/etc/apt/keyrings/docker.asc"
SOURCE_LIST="/etc/apt/sources.list.d/docker.list"
# Update only after verifying a replacement fingerprint against Docker's published signing-key documentation.
DOCKER_GPG_FINGERPRINT="9DC858229FC7DD38854AE2D88D81803C0EBFCD88"
. /etc/os-release
DISTRO_ID="${ID:-}"
DISTRO_CODENAME="${VERSION_CODENAME:-}"

if [[ -z "${DISTRO_ID}" || -z "${DISTRO_CODENAME}" ]]; then
echo "Docker apt repository requires ID and VERSION_CODENAME in /etc/os-release." >&2
exit 1
fi

echo "Adding Docker apt repository for ${DISTRO_ID} ${DISTRO_CODENAME}..."
sudo install -m 0755 -d /etc/apt/keyrings
KEY_TMP="$(mktemp)"
trap 'rm -f "${KEY_TMP}"' EXIT
# runner-guard:ignore RGS-012 -- fetches Docker's GPG signing key to a temporary file (never piped to a shell); the key fingerprint is verified before it is used to verify the signed-by apt repository below.
curl -fsSL "https://download.docker.com/linux/${DISTRO_ID}/gpg" -o "${KEY_TMP}"
KEY_FINGERPRINT="$(gpg --show-keys --with-colons "${KEY_TMP}" | awk -F: '$1 == "fpr" { print $10; exit }')"
if [[ "${KEY_FINGERPRINT}" != "${DOCKER_GPG_FINGERPRINT}" ]]; then
echo "Downloaded Docker GPG key fingerprint does not match the expected fingerprint." >&2
exit 1
fi
sudo install -m 0644 "${KEY_TMP}" "${KEYRING_PATH}"

echo "deb [arch=$(dpkg --print-architecture) signed-by=${KEYRING_PATH}] https://download.docker.com/linux/${DISTRO_ID} ${DISTRO_CODENAME} stable" \

@github-actions github-actions Bot Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hard-codes ${DISTRO_CODENAME} into the apt source without validating that Docker publishes that suite, so on runners or derivatives where the field is empty or unsupported this step now dies at apt-get update and takes every docker-sbx workflow down.

💡 Why this matters and how to harden it

The old get.docker.com path handled distro detection internally; this replacement moved that responsibility into our script but never added the guardrails. At minimum, fail fast with a clear error when VERSION_CODENAME is empty, and preferably verify that the Docker repo exposes dists/${DISTRO_ID}/dists/${DISTRO_CODENAME} before writing the source line.

Copilot AI Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 820506d: the installer now validates non-empty ID and VERSION_CODENAME values before writing the apt source, failing with a clear error when either is unavailable.

| sudo tee "${SOURCE_LIST}" > /dev/null

sudo apt-get update -qq
sudo apt-get install -y docker-sbx
sbx version
# Fix KVM permissions so the runner user can create microVMs.
Expand Down
11 changes: 9 additions & 2 deletions pkg/workflow/docker_sbx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,9 @@ func TestDockerSbxShellScriptContent(t *testing.T) {
shDir := filepath.Join(wd, "..", "..", "actions", "setup", "sh")

tests := []struct {
script string
contains []string
script string
contains []string
notContains []string
}{
{
script: "docker_sbx_kvm_check.sh",
Expand All @@ -724,7 +725,10 @@ func TestDockerSbxShellScriptContent(t *testing.T) {
script: "sudo_docker_sbx_install.sh",
contains: []string{
"sudo", "apt-get install", "docker-sbx", "sbx version", "/dev/kvm",
"DOCKER_GPG_FINGERPRINT", "gpg --show-keys", "mktemp", "signed-by=",
"VERSION_CODENAME", "download.docker.com",
},
notContains: []string{"get.docker.com", "| sudo REPO_ONLY=1 sh"},
},
{
script: "docker_sbx_daemon.sh",
Expand Down Expand Up @@ -759,6 +763,9 @@ func TestDockerSbxShellScriptContent(t *testing.T) {
for _, s := range tc.contains {
assert.Contains(t, string(content), s, "script %s must contain %q", tc.script, s)
}
for _, s := range tc.notContains {
assert.NotContains(t, string(content), s, "script %s must not contain %q", tc.script, s)
}
})
}
}
Expand Down
Loading