Harden scripts against argument injection and malformed inputs

scripts/build_image.py

@@ -31,6 +31,8 @@ def main(argv: list[str] | None = None) -> int:
     data = builds.load()
     rust_digest = args.rust_image_digest
     try:
+        if args.tag:
+            common.reject_option_like(args.tag, "--tag")
         builds.assert_pair_declared(
             data, args.stellar_cli_version, f"{args.rust_version}@{rust_digest}"
         )

scripts/lib/builds.py

@@ -20,7 +20,15 @@
 
 def load(path: Path | None = None) -> dict[str, Any]:
     target = path or DEFAULT_PATH
-    return json.loads(target.read_text())
+    data = json.loads(target.read_text())
+    if not isinstance(data, dict):
+        # Every caller treats the result as a mapping (data.get(...)); a JSON
+        # scalar/array would otherwise surface as an opaque AttributeError deep
+        # in an unrelated call. Fail here with a message naming the file.
+        raise ValueError(
+            f"{target} must contain a JSON object at the top level, got {type(data).__name__}"
+        )
+    return data
 
 
 def dump(data: dict[str, Any], path: Path | None = None) -> None:
@@ -59,6 +67,10 @@ def split_entry(pin: str) -> tuple[str, str]:
     `rust:<label>` base and the published tag's `rust<label>` segment;
     the digest pins the exact base bytes.
     """
+    if not isinstance(pin, str):
+        raise ValueError(
+            f"rust base pin must be a string (expected <label>@<digest>), got {type(pin).__name__}"
+        )
     label, sep, digest = pin.partition("@")
     if not sep or not digest:
         raise ValueError(f"invalid rust base pin (expected <label>@<digest>): {pin}")

scripts/lib/common.py

@@ -32,6 +32,28 @@ def repo_root() -> Path:
     return REPO_ROOT
 
 
+def reject_option_like(value: str, name: str) -> str:
+    """Refuse a value that a child process would parse as a command-line option.
+
+    Values that reach a subprocess argv as positionals — image references, git
+    refs, git remote URLs, tags — must not begin with ``-``. Both ``git`` and
+    ``docker`` permute their argv and treat any ``-``-prefixed token as a flag
+    wherever it appears, so an attacker-influenced value like
+    ``--upload-pack=<cmd>`` or ``--privileged`` is interpreted as an injected
+    option rather than data (argument injection, up to arbitrary command
+    execution). A valid image reference, git refname, or tag never starts with
+    ``-``, so rejecting that prefix loses no legitimate input.
+
+    Returns the value unchanged when it is safe, so callers can guard inline.
+    """
+    if value.startswith("-"):
+        raise ValueError(
+            f"{name} must not begin with '-' (got {value!r}): "
+            "a leading dash would be parsed as a command-line option"
+        )
+    return value
+
+
 def step_summary(message: str) -> None:
     """Append a markdown block to the GitHub Actions step summary.
 

scripts/lib/docker_inspect.py

@@ -7,12 +7,13 @@
 
 import re
 
-from lib import runner
+from lib import common, runner
 
 _DIGEST = re.compile(r"sha256:[0-9a-f]{64}")
 
 
 def index_digest(image_ref: str) -> str:
+    common.reject_option_like(image_ref, "image reference")
     # `--format '{{.Manifest.Digest}}'` targets the top-level (index)
     # descriptor's digest. Some buildx releases print the bare digest while
     # others dump the whole descriptor struct, but the descriptor only ever
@@ -28,6 +29,7 @@ def index_digest(image_ref: str) -> str:
 
 
 def exists(image_ref: str) -> bool:
+    common.reject_option_like(image_ref, "image reference")
     result = runner.run(
         ["docker", "buildx", "imagetools", "inspect", image_ref],
         check=False,
@@ -39,4 +41,7 @@ def exists(image_ref: str) -> bool:
 def create_manifest(tag: str, *sources: str) -> None:
     if not sources:
         raise ValueError("create_manifest requires at least one source image")
+    common.reject_option_like(tag, "manifest tag")
+    for source in sources:
+        common.reject_option_like(source, "source image")
     runner.run(["docker", "buildx", "imagetools", "create", "--tag", tag, *sources])

scripts/lib/git_remote.py

@@ -4,11 +4,14 @@
 can patch one symbol per script.
 """
 
-from lib import runner
+from lib import common, runner
 
 
 def ls_remote(repo_url: str, *refspecs: str) -> str:
-    return runner.capture(["git", "ls-remote", repo_url, *refspecs])
+    common.reject_option_like(repo_url, "git remote URL")
+    for refspec in refspecs:
+        common.reject_option_like(refspec, "git refspec")
+    return runner.capture(["git", "ls-remote", "--end-of-options", repo_url, *refspecs])
 
 
 def resolve_tag_commit(repo_url: str, tag: str) -> str | None:

scripts/publish_aliases.py

@@ -41,6 +41,7 @@ def main(argv: list[str] | None = None) -> int:
 
     data = builds.load()
     try:
+        common.reject_option_like(args.registry, "--registry")
         default_pin = builds.derive_default_rust(data, args.stellar_cli_version)
         default_rust = builds.label_of(default_pin)
     except ValueError as exc:

scripts/publish_manifests.py

@@ -44,6 +44,11 @@ def main(argv: list[str] | None = None) -> int:
     args = build_parser().parse_args(argv)
     common.preflight_checks(["buildx"])
 
+    try:
+        common.reject_option_like(args.registry, "--registry")
+    except ValueError as exc:
+        common.die(str(exc))
+
     data = builds.load()
     entry = builds.find_cli(data, args.stellar_cli_version)
     if entry is None:

scripts/refresh.py

@@ -20,6 +20,7 @@
 
 import argparse
 import re
+import subprocess
 import sys
 from collections.abc import Iterable
 
@@ -91,9 +92,14 @@ def append_pins(entry: dict, labels: Iterable[str]) -> bool:
         common.log(f"  -> appending {pin}")
         entry["rust_versions"].append(pin)
         changed = True
-    entry["rust_versions"].sort(
-        key=lambda p: semver.parse(rust_keys.version_of(builds.label_of(p)))
-    )
+    # Only re-sort when we actually appended. Sorting unconditionally would
+    # rewrite (and, via the caller's os.replace(), commit) a reordering of
+    # already-published pins even on a no-op refresh, making release-prepare
+    # see a spurious change.
+    if changed:
+        entry["rust_versions"].sort(
+            key=lambda p: semver.parse(rust_keys.version_of(builds.label_of(p)))
+        )
     return changed
 
 
@@ -143,6 +149,11 @@ def main(argv: list[str] | None = None) -> int:
         data["stellar_cli_versions"].sort(key=lambda e: semver.parse(e["version"]))
     except ValueError as exc:
         common.die(str(exc))
+    except subprocess.CalledProcessError as exc:
+        # `docker buildx imagetools inspect rust:<label>` exits non-zero for an
+        # unknown/typo'd label (e.g. from --rust-versions). Report it cleanly
+        # instead of letting the CalledProcessError escape as a traceback.
+        common.die(f"failed to resolve a rust base digest via docker: {exc}")
     except OSError as exc:
         # Network failure from the Docker Hub tag lookup (urllib raises
         # URLError, an OSError) or a filesystem error.

scripts/release_body.py

@@ -9,18 +9,40 @@
 import argparse
 import io
 import json
+import re
 import sys
 from pathlib import Path
 
 from lib import builds, common, semver
 
+# The release body interpolates these metadata fields verbatim into a
+# copy-paste-runnable ```sh``` verification block. Constrain each to a charset
+# that can't smuggle shell metacharacters (e.g. $(...), backticks, ';'), so a
+# malformed/hostile meta-*.json can't plant a command in the published body.
+_DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")
+_ARCH_RE = re.compile(r"^[a-z0-9]+$")
+_TAG_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$")
+# A registry ref / repo slug as it appears in the verify commands.
+_REF_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/:-]*$")
+
+_ROW_FIELD_RE = {
+    "digest": _DIGEST_RE,
+    "arch": _ARCH_RE,
+    "tag": _TAG_RE,
+    "rust_base_key": _TAG_RE,
+}
+
 
 def load_metadata(metadata_dir: Path, expected_cli: str) -> list[dict]:
     files = sorted(metadata_dir.glob("meta-*.json"))
     if not files:
         raise ValueError(f"no meta-*.json files under {metadata_dir}")
     rows: list[dict] = []
     for f in files:
+        # A symlinked meta-*.json could resolve to an arbitrary file outside the
+        # metadata dir; only read regular files we can vouch for.
+        if f.is_symlink():
+            raise ValueError(f"metadata file {f} is a symlink; refusing to follow it")
         row = json.loads(f.read_text())
         entry_cli = row.get("stellar_cli_version") or ""
         if not entry_cli:
@@ -30,6 +52,15 @@ def load_metadata(metadata_dir: Path, expected_cli: str) -> list[dict]:
                 f"metadata file {f} has stellar_cli_version='{entry_cli}', "
                 f"expected '{expected_cli}'"
             )
+        for field, pattern in _ROW_FIELD_RE.items():
+            value = row.get(field)
+            if not isinstance(value, str) or not pattern.match(value):
+                raise ValueError(
+                    f"metadata file {f} has invalid {field}={value!r}; "
+                    f"expected to match {pattern.pattern}"
+                )
+        if "rust_version" not in row:
+            raise ValueError(f"metadata file {f} is missing the rust_version field")
         rows.append(row)
     rows.sort(key=lambda r: (semver.parse(r["rust_version"]), r["rust_base_key"], r["arch"]))
     return rows
@@ -136,6 +167,12 @@ def main(argv: list[str] | None = None) -> int:
     if not metadata_dir.is_dir():
         common.die(f"{metadata_dir} is not a directory")
 
+    # registry and repo are interpolated into the verification shell block too.
+    if not _REF_RE.match(args.registry):
+        common.die(f"--registry {args.registry!r} has unexpected characters")
+    if not _REF_RE.match(args.repo):
+        common.die(f"--repo {args.repo!r} has unexpected characters")
+
     try:
         rows = load_metadata(metadata_dir, args.stellar_cli_version)
         stellar_ref = builds.stellar_cli_ref(builds.load(), args.stellar_cli_version)

scripts/repro_test.py

@@ -100,10 +100,19 @@ def test_one_contract(image: str, workdir: Path, name: str) -> bool:
 
 
 def clone(repo: str, rev: str, workdir: Path) -> None:
+    # Both values are untrusted CLI input. git permutes its argv and treats any
+    # '-'-prefixed token as an option wherever it sits, so a rev like
+    # '--upload-pack=<cmd>' (with a file:// repo) would run an arbitrary binary.
+    # Reject the dash prefix and pin '--end-of-options' before the positionals
+    # so git can never reinterpret them as flags.
+    common.reject_option_like(repo, "repo URL")
+    common.reject_option_like(rev, "rev")
     common.log(f"cloning {repo} @ {rev} into {workdir} ...")
     runner.run(["git", "-C", str(workdir), "init", "-q"])
-    runner.run(["git", "-C", str(workdir), "remote", "add", "origin", repo])
-    runner.run(["git", "-C", str(workdir), "fetch", "--depth=1", "origin", rev, "-q"])
+    runner.run(["git", "-C", str(workdir), "remote", "add", "--", "origin", repo])
+    runner.run(
+        ["git", "-C", str(workdir), "fetch", "--depth=1", "-q", "--end-of-options", "origin", rev]
+    )
     runner.run(["git", "-C", str(workdir), "checkout", "-q", "FETCH_HEAD"])
 
 
@@ -123,6 +132,13 @@ def main(argv: list[str] | None = None) -> int:
     args = build_parser().parse_args(argv)
     common.preflight_checks(["git", "buildx"])
 
+    try:
+        common.reject_option_like(args.image, "--image")
+        common.reject_option_like(args.repo, "--repo")
+        common.reject_option_like(args.rev, "--rev")
+    except ValueError as exc:
+        common.die(str(exc))
+
     contracts = args.contracts or list(DEFAULT_CONTRACTS)
     workdir = Path(tempfile.mkdtemp(prefix="repro-test."))
     atexit.register(make_cleanup(workdir, args.keep_workdir))

scripts/smoke_test_image.py

@@ -84,6 +84,9 @@ def main(argv: list[str] | None = None) -> int:
 
     data = builds.load()
     try:
+        # args.image is fed to `docker run`/`docker inspect` as a positional; a
+        # value like '--privileged' would otherwise be parsed as a docker flag.
+        common.reject_option_like(args.image, "--image")
         parsed = rust_keys.parse(args.rust_version)
         stellar_ref = builds.stellar_cli_ref(data, args.stellar_cli_version)
     except ValueError as exc:

scripts/validate_json.py

@@ -20,9 +20,16 @@
 
 
 def iter_json_files(root: Path):
+    root_resolved = root.resolve()
     for path in sorted(root.rglob("*.json")):
         if any(part in EXCLUDED_DIRS for part in path.relative_to(root).parts):
             continue
+        # Don't follow symlinks out of the repo: a symlinked *.json (or a file
+        # reached through a symlinked directory) could resolve anywhere on
+        # disk, turning this repo lint into an arbitrary-file read. Skip any
+        # path whose real location escapes the repo tree.
+        if not path.resolve().is_relative_to(root_resolved):
+            continue
         yield path
 
 

scripts/write_metadata.py

@@ -38,6 +38,11 @@ def build_parser() -> argparse.ArgumentParser:
 def main(argv: list[str] | None = None) -> int:
     args = build_parser().parse_args(argv)
 
+    try:
+        common.reject_option_like(args.image, "--image")
+    except ValueError as exc:
+        common.die(str(exc))
+
     digest = args.digest
     if not digest:
         try:

tests/integration/test_refresh.py

@@ -1,4 +1,5 @@
 import json
+import subprocess
 from pathlib import Path
 
 import pytest
@@ -175,6 +176,36 @@ def test_unresolvable_tag_dies(monkeypatch: pytest.MonkeyPatch, staged_minimal:
         refresh.main(["--stellar-cli-version", "27.0.0", "--rust-versions", "1.95.0-slim-trixie"])
 
 
+def test_docker_failure_dies_cleanly(
+    monkeypatch: pytest.MonkeyPatch, staged_minimal: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+    # An unknown/typo'd --rust-versions label makes `docker buildx imagetools
+    # inspect` exit non-zero; the CalledProcessError must be reported via die(),
+    # not escape as an unhandled traceback.
+    _no_git(monkeypatch)
+
+    def boom(ref: str) -> str:
+        raise subprocess.CalledProcessError(1, ["docker", "buildx", "imagetools", "inspect", ref])
+
+    monkeypatch.setattr(refresh.docker_inspect, "index_digest", boom)
+    with pytest.raises(SystemExit):
+        refresh.main(["--stellar-cli-version", "26.0.0", "--rust-versions", "nonsuch-label"])
+    assert "docker" in capsys.readouterr().err
+
+
+def test_append_pins_preserves_order_on_noop() -> None:
+    # When no new pin is appended, the existing (possibly published) order must
+    # not be silently re-sorted — that would surface as a spurious change.
+    pin_a = "1.95.0-slim-trixie@sha256:" + "a" * 64
+    pin_b = "1.94.0-slim-trixie@sha256:" + "b" * 64
+    entry = {"rust_versions": [pin_a, pin_b]}  # deliberately not version-sorted
+
+    # All requested labels already present (by exact pin) → no append.
+    changed = refresh.append_pins(entry, [])
+    assert changed is False
+    assert entry["rust_versions"] == [pin_a, pin_b]
+
+
 def test_auto_picks_when_no_rust_versions(
     monkeypatch: pytest.MonkeyPatch, staged_minimal: Path, fixtures_dir: Path
 ) -> None:

tests/unit/test_build_image.py

@@ -43,6 +43,27 @@ def test_main_invokes_docker_with_build_args(
     assert f"RUST_IMAGE_DIGEST={DIGEST}" in args
 
 
+def test_main_rejects_option_like_tag(
+    monkeypatch: pytest.MonkeyPatch, minimal_builds: dict, capsys: pytest.CaptureFixture[str]
+) -> None:
+    monkeypatch.setattr(build_image.builds, "load", lambda: minimal_builds)
+    monkeypatch.setattr(build_image.common, "preflight_checks", lambda _: None)
+    monkeypatch.setattr(build_image.runner, "run", lambda *_, **__: pytest.fail("ran docker"))
+    with pytest.raises(SystemExit):
+        build_image.main(
+            [
+                "--stellar-cli-version",
+                "26.0.0",
+                "--rust-version",
+                "1.94.0-slim-trixie",
+                "--rust-image-digest",
+                DIGEST,
+                "--tag=--push",  # would reach `docker buildx build` as a flag
+            ]
+        )
+    assert "must not begin with '-'" in capsys.readouterr().err
+
+
 def test_main_respects_platform_flag(monkeypatch: pytest.MonkeyPatch, minimal_builds: dict) -> None:
     monkeypatch.setattr(build_image.builds, "load", lambda: minimal_builds)
     monkeypatch.setattr(build_image.common, "preflight_checks", lambda _: None)