Skip to content

fix: preserve literal RPM repository URL arguments #580

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

Open
soulteary wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: preserve literal RPM repository URL arguments #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,16 @@ jobs:
make build/${{ matrix.channel }}/rootless-install.sh \
${{ matrix.stable_latest && format('STABLE_LATEST={0}', matrix.stable_latest) || '' }}
FORCE_ROOTLESS_INSTALL=1 ./build/${{ matrix.channel }}/rootless-install.sh

# Execute repository commands against inert mocks; never contact a mirror.
test-repository-url-quoting:
runs-on: ubuntu-latest
timeout-minutes: 2
steps:
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3
- name: Check installer syntax
run: sh -n install.sh
- name: Check RPM repository URL arguments (offline only)
run: |
python3 scripts/test-repository-url-quoting.py
TEST_SHELL=bash python3 scripts/test-repository-url-quoting.py
15 changes: 15 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ To verify that the install script works amongst the supported operating systems
make shellcheck
```

### Offline RPM repository URL regression checks

Run the repository URL argument checks with Python 3:

```shell
python3 scripts/test-repository-url-quoting.py
TEST_SHELL=bash python3 scripts/test-repository-url-quoting.py
```

The suite executes the installer's generated shell commands against inert dnf5,
dnf and yum mocks. It checks exact URL arguments, literal special characters,
mirror precedence, staging defaults and both channels. It makes no network
requests, installs no packages and never writes host repository files. These
checks do not validate live RPM repositories or package-manager compatibility.

## Legal
*Brought to you courtesy of our legal counsel. For more context,
please see the [NOTICE](NOTICE) document in this repo.*
Expand Down
26 changes: 23 additions & 3 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,25 @@ command_exists() {
command -v "$@" > /dev/null 2>&1
}

# Quote one argument for commands executed through the POSIX shell in $sh_c.
# A closing quote preserves trailing newlines when the result is captured.
shell_quote() (
value=$1
printf "'"
while :; do
case "$value" in
*"'"*)
printf '%s' "${value%%\'*}" "'\''"
value=${value#*\'}
;;
*)
printf "%s'" "$value"
break
;;
esac
done
)

# version_gte checks if the version specified in $VERSION is at least the given
# SemVer (Maj.Minor[.Patch]), or CalVer (YY.MM) version.It returns 0 (success)
# if $VERSION is either unset (=latest) or newer or equal than the specified
Expand Down Expand Up @@ -676,13 +695,14 @@ do_install() {
;;
centos|fedora|rhel|rocky)
repo_file_url="$DOWNLOAD_URL/linux/$lsb_dist/$REPO_FILE"
repo_file_url_quoted=$(shell_quote "$repo_file_url")
(
if ! is_dry_run; then
set -x
fi
if command_exists dnf5; then
$sh_c "dnf -y -q --setopt=install_weak_deps=False install dnf-plugins-core"
$sh_c "dnf5 config-manager addrepo --overwrite --save-filename=docker-ce.repo --from-repofile='$repo_file_url'"
$sh_c "dnf5 config-manager addrepo --overwrite --save-filename=docker-ce.repo --from-repofile=$repo_file_url_quoted"

if [ "$CHANNEL" != "stable" ]; then
$sh_c "dnf5 config-manager setopt \"docker-ce-*.enabled=0\""
Expand All @@ -692,7 +712,7 @@ do_install() {
elif command_exists dnf; then
$sh_c "dnf -y -q --setopt=install_weak_deps=False install dnf-plugins-core"
$sh_c "rm -f /etc/yum.repos.d/docker-ce.repo /etc/yum.repos.d/docker-ce-staging.repo"
$sh_c "dnf config-manager --add-repo $repo_file_url"
$sh_c "dnf config-manager --add-repo $repo_file_url_quoted"

if [ "$CHANNEL" != "stable" ]; then
$sh_c "dnf config-manager --set-disabled \"docker-ce-*\""
Expand All @@ -702,7 +722,7 @@ do_install() {
else
$sh_c "yum -y -q install yum-utils"
$sh_c "rm -f /etc/yum.repos.d/docker-ce.repo /etc/yum.repos.d/docker-ce-staging.repo"
$sh_c "yum-config-manager --add-repo $repo_file_url"
$sh_c "yum-config-manager --add-repo $repo_file_url_quoted"

if [ "$CHANNEL" != "stable" ]; then
$sh_c "yum-config-manager --disable \"docker-ce-*\""
Expand Down
152 changes: 152 additions & 0 deletions scripts/test-repository-url-quoting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
#!/usr/bin/env python3
"""Offline checks that execute RPM repository commands against inert mocks."""
import json
import os
from pathlib import Path
import shutil
import subprocess
import tempfile

ROOT = Path(__file__).resolve().parents[1]
INSTALLER = Path(os.environ.get("INSTALLER_UNDER_TEST", ROOT / "install.sh"))
TEST_SHELL = shutil.which(os.environ.get("TEST_SHELL", "sh"))
if TEST_SHELL is None:
raise SystemExit("ERROR: TEST_SHELL was not found")

# Keep the installer unchanged except for deferring its final entry point.
source = INSTALLER.read_text()
if not source.endswith("\ndo_install\n"):
raise SystemExit("ERROR: expected the installer's final do_install entry point")

mock_source = """#!/usr/bin/env python3
import json
import os
from pathlib import Path
import sys

command = Path(sys.argv[0]).name
args = sys.argv[1:]
with open(os.environ["MOCK_ARGUMENT_LOG"], "a") as log:
log.write(json.dumps({"command": command, "args": args}) + "\\n")
if command == "rm":
expected = ["-f", "/etc/yum.repos.d/docker-ce.repo", "/etc/yum.repos.d/docker-ce-staging.repo"]
if args != expected:
raise SystemExit("ERROR: unexpected removal command in offline test")
elif command not in ("dnf5", "dnf", "yum", "yum-config-manager"):
raise SystemExit("ERROR: forbidden command executed in offline test: " + command)
"""

wrapper_source = """#!/bin/sh
set -e
. "$TEST_INSTALLER"
# Control host detection only; repository command generation and sh -c execution
# still come from the actual installer.
id() { printf '%s\\n' root; }
get_distribution() { printf '%s\\n' "$TEST_DISTRO"; }
is_wsl() { false; }
check_forked() { :; }
command_exists() {
case "$1" in
dnf5) [ "$TEST_MANAGER" = dnf5 ] ;;
dnf) [ "$TEST_MANAGER" != yum ] ;;
docker|lsb_release) return 1 ;;
*) command -v "$@" >/dev/null 2>&1 ;;
esac
}
dist_version=44
do_install
"""

with tempfile.TemporaryDirectory(prefix="docker-install-repository-url-") as temp:
directory = Path(temp)
mock_bin = directory / "bin"
mock_bin.mkdir()
production = directory / "install-functions.sh"
production.write_text(source[:-len("do_install\n")])
wrapper = directory / "run-installer.sh"
wrapper.write_text(wrapper_source)
mock = directory / "mock-command"
mock.write_text(mock_source)
mock.chmod(0o755)
for command in ("dnf5", "dnf", "yum", "yum-config-manager", "rm",
"curl", "wget", "sudo", "su", "docker", "systemctl"):
(mock_bin / command).symlink_to(mock)
# Exercise the chosen shell for both the installer and its inner sh -c.
(mock_bin / "sh").symlink_to(TEST_SHELL)
log = directory / "arguments.jsonl"
substitution_marker = directory / "substitution-executed"
backtick_marker = directory / "backtick-executed"
default_url = "https://download.docker.com"
special = 'space & single\' double" $HOME $(touch "$TEST_SUBSTITUTION_MARKER") ' + chr(96) + 'touch "$TEST_BACKTICK_MARKER"' + chr(96)
cases = [
("default", {}, [], default_url, "docker-ce.repo"),
("custom URL", {"DOWNLOAD_URL": "https://mirror.invalid/" + special},
[], "https://mirror.invalid/" + special, "docker-ce.repo"),
("custom repo file", {"REPO_FILE": special + ".repo"},
[], default_url, special + ".repo"),
("URL and repo file", {"DOWNLOAD_URL": "https://mirror.invalid/" + special,
"REPO_FILE": special + ".repo"},
[], "https://mirror.invalid/" + special, special + ".repo"),
("newline characters", {"DOWNLOAD_URL": "https://mirror.invalid/a\nb\t",
"REPO_FILE": "line\nfile.repo\n"},
[], "https://mirror.invalid/a\nb\t", "line\nfile.repo\n"),
("staging default", {"DOWNLOAD_URL": "https://download-stage.docker.com"},
[], "https://download-stage.docker.com", "docker-ce-staging.repo"),
("staging explicit repo", {"DOWNLOAD_URL": "https://download-stage.docker.com",
"REPO_FILE": special + ".repo"},
[], "https://download-stage.docker.com", special + ".repo"),
("mirror overrides custom URL", {"DOWNLOAD_URL": "https://mirror.invalid/" + special,
"REPO_FILE": special + ".repo"},
["--mirror", "Aliyun"], "https://mirrors.aliyun.com/docker-ce",
special + ".repo"),
]
checks = 0
for manager in ("dnf5", "dnf", "yum"):
distro = "centos" if manager == "yum" else "fedora"
for channel in ("stable", "test"):
for label, overrides, arguments, base_url, repo_file in cases:
log.write_text("")
substitution_marker.unlink(missing_ok=True)
backtick_marker.unlink(missing_ok=True)
# Discard inherited VERSION, mirror, dry-run and installer settings.
environment = {
"PATH": str(mock_bin) + os.pathsep + os.environ["PATH"],
"HOME": str(directory),
"TEST_INSTALLER": str(production),
"TEST_MANAGER": manager,
"TEST_DISTRO": distro,
"MOCK_ARGUMENT_LOG": str(log),
"TEST_SUBSTITUTION_MARKER": str(substitution_marker),
"TEST_BACKTICK_MARKER": str(backtick_marker),
**overrides,
}
result = subprocess.run(
[TEST_SHELL, str(wrapper), "--setup-repo", "--channel", channel, *arguments],
env=environment, text=True, capture_output=True, timeout=10)
name = f"{manager}/{channel}/{label}"
if result.returncode:
raise SystemExit(f"FAIL (offline): {name}\n{result.stdout}{result.stderr}")
calls = [json.loads(line) for line in log.read_text().splitlines()]
expected_url = f"{base_url}/linux/{distro}/{repo_file}"
if manager == "dnf5":
expected = {"command": "dnf5", "args": ["config-manager", "addrepo",
"--overwrite", "--save-filename=docker-ce.repo",
"--from-repofile=" + expected_url]}
repository_calls = [call for call in calls if call["command"] == "dnf5"
and "addrepo" in call["args"]]
else:
expected = {"command": "dnf" if manager == "dnf" else "yum-config-manager",
"args": (["config-manager"] if manager == "dnf" else []) +
["--add-repo", expected_url]}
repository_calls = [call for call in calls if "--add-repo" in call["args"]]
if repository_calls != [expected]:
raise SystemExit(f"FAIL (offline): {name}: URL arguments changed\n"
f"expected={expected!r}\nactual={repository_calls!r}")
if substitution_marker.exists() or backtick_marker.exists():
raise SystemExit(f"FAIL (offline): {name}: command substitution executed")
if any("docker-ce" in call["args"] and "install" in call["args"] for call in calls):
raise SystemExit(f"FAIL (offline): {name}: setup-repo installed Docker")
checks += 1
print(f"PASS (offline): {manager}: exact URL argument and no command substitution")
print(f"PASS (offline): {checks} RPM repository cases with {TEST_SHELL}")
print("No network requests, package installation, or host repository changes were performed.")