diff --git a/tests/test_release_artifact.py b/tests/test_release_artifact.py index 7f6aa10..441568c 100644 --- a/tests/test_release_artifact.py +++ b/tests/test_release_artifact.py @@ -68,7 +68,9 @@ class ArtifactContents(unittest.TestCase): cls.scratch = Path(tempfile.mkdtemp(prefix="bench-artifact-")).resolve() cls.tarball = cls.scratch / "bench.tar.gz" result = build_artifact(cls.tarball) - assert result.returncode == 0, result.stdout + result.stderr + if result.returncode != 0: # not assert: must survive python -O + raise RuntimeError( + f"release.sh failed:\n{result.stdout}{result.stderr}") with tarfile.open(cls.tarball) as tar: cls.members = {m.name.removeprefix("./"): m for m in tar.getmembers()} @@ -187,7 +189,9 @@ class ArtifactInstalls(unittest.TestCase): cls.scratch = Path(tempfile.mkdtemp(prefix="bench-install-")).resolve() cls.tarball = cls.scratch / "bench.tar.gz" result = build_artifact(cls.tarball) - assert result.returncode == 0, result.stdout + result.stderr + if result.returncode != 0: # not assert: must survive python -O + raise RuntimeError( + f"release.sh failed:\n{result.stdout}{result.stderr}") @classmethod def tearDownClass(cls): diff --git a/tests/test_update_from_release.py b/tests/test_update_from_release.py index df10ed8..e9bd11c 100644 --- a/tests/test_update_from_release.py +++ b/tests/test_update_from_release.py @@ -8,6 +8,7 @@ of it. python3 -m unittest discover -s tests """ +import io import os import shutil import subprocess @@ -59,7 +60,9 @@ class UpdateFromRelease(unittest.TestCase): ["bash", str(REPO / "release.sh"), "--tarball", str(cls.tarball), "--source", "example/bench"], capture_output=True, text=True) - assert result.returncode == 0, result.stdout + result.stderr + if result.returncode != 0: # not assert: must survive python -O + raise RuntimeError( + f"release.sh failed:\n{result.stdout}{result.stderr}") cls.version = (REPO / "manager" / "core" / "VERSION").read_text().strip() cls.stubs = cls.scratch / "bin" @@ -159,6 +162,43 @@ class UpdateFromRelease(unittest.TestCase): self.assertIn(f"contains core VERSION {self.version}", result.stderr) self.assertEqual(snapshot(tm), before) + def test_asset_with_escaping_member_paths_is_refused(self): + tm = self.make_install() + bad = self.scratch / "bad-members.tar.gz" + with tarfile.open(bad, "w:gz") as tar: + info = tarfile.TarInfo("../escape") + payload = b"outside\n" + info.size = len(payload) + tar.addfile(info, io.BytesIO(payload)) + before = snapshot(tm) + + result = self.run_update(tm, STUB_TARBALL=str(bad)) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("escape its own root", result.stderr) + self.assertEqual(snapshot(tm), before) + + def test_unsafe_manifest_copy_path_is_refused(self): + # A well-formed asset whose manifest reaches outside .task-manager/ + # must be refused whole — before core/ is touched. + tm = self.make_install() + workdir = self.scratch / "bad-manifest" + with tarfile.open(self.tarball) as tar: + tar.extractall(workdir) + manifest = workdir / "manager" / "core" / "release-manifest" + manifest.write_text(manifest.read_text(encoding="utf-8") + + "copy ../outside\n", encoding="utf-8") + bad = self.scratch / "bad-manifest.tar.gz" + with tarfile.open(bad, "w:gz") as tar: + tar.add(workdir, arcname=".") + before = snapshot(tm) + + result = self.run_update(tm, STUB_TARBALL=str(bad)) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("unsafe path: ../outside", result.stderr) + self.assertEqual(snapshot(tm), before) + def test_exact_tag_via_bench_ref(self): tm = self.make_install() result = self.run_update(tm, BENCH_REF=f"v{self.version}") diff --git a/update.sh b/update.sh index c04fa8b..b8e4a6c 100755 --- a/update.sh +++ b/update.sh @@ -119,6 +119,12 @@ if ! fetch_release; then fi mkdir "$tmp/dist" +# Nothing in the asset may name a path outside its own root — tar +# implementations differ on how much of that they refuse themselves. +if tar -tzf "$asset" | grep -E '^/|(^|/)\.\.(/|$)' >/dev/null; then + echo "The $tag asset contains paths that escape its own root. Refusing to install it; nothing was changed." >&2 + exit 1 +fi tar -xzf "$asset" -C "$tmp/dist" dist="$tmp/dist" manifest="$dist/manager/core/release-manifest" @@ -135,13 +141,26 @@ if [ "v$artifact_version" != "$tag" ]; then exit 1 fi +# The manifest's copy paths land under $TM verbatim, so none may reach +# outside it — checked before anything is replaced, leaving a poisoned +# manifest no partial update to hide behind. +while read -r kind path _; do + [ "$kind" = "copy" ] || continue + case "/$path/" in *//*|*/../*) + echo "Release $tag manifest names an unsafe path: $path. Refusing to install it; nothing was changed." >&2 + exit 1;; + esac +done < "$manifest" + before="$(cat "$TM/manager/core/VERSION" 2>/dev/null || echo '?')" rsync -a --delete "$dist/manager/core/" "$TM/manager/core/" # The artifact's manifest names the top-level core-owned files (class # `copy`) — read the new list, so a release adding a script updates it. while read -r kind path _; do [ "$kind" = "copy" ] || continue - [ -f "$dist/$path" ] && cp "$dist/$path" "$TM/$path" + [ -f "$dist/$path" ] || continue + mkdir -p "$TM/$(dirname "$path")" + cp "$dist/$path" "$TM/$path" done < "$manifest" chmod +x "$TM"/start.sh "$TM"/stop.sh "$TM"/update.sh 2>/dev/null || true find "$TM/manager/core/adapters" -name run -o -name wire | xargs chmod +x 2>/dev/null || true