From ab568b32863dce7e7d300f3ed0d2654baba088a0 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Fri, 10 Jul 2026 19:46:20 +0200 Subject: [PATCH] archive: extract safe symlinks on old Python The daily release-tox-drift job crashed while fetching sources with: source error: unsafe archive member (link/special): osism-release-a2e4875/1.0.0/ceph.yml _safe_extract downloads each source repo as a GitHub tarball and extracts it. When tarfile.data_filter is available (Python 3.11.4+ / 3.12+) it extracts with filter="data", which permits relative symlinks that stay inside the extraction directory. The manual fallback for older Pythons, however, rejected every symlink member outright, so it was stricter than the data filter it was meant to emulate. The Zuul node runs Python 3.11.2, which predates the data_filter backport, so it takes the fallback path. The release repo -- now one of the fetched sources -- contains ~80 legitimate relative symlinks (per-version ceph.yml and openstack.yml aliases), so extraction aborted on the first one and the whole check exited non-zero. Local runs and the release-tox-drift-test job never caught this because their interpreters have data_filter. Make the fallback emulate the data filter: permit symlinks and hardlinks whose target resolves inside the extraction root, and keep rejecting links that escape (absolute or via ..) and any other special member. The final extractall is made version-robust (filter="fully_trusted" where supported, plain extractall otherwise) so it behaves identically on old and new interpreters. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi --- src/osism_drift/archive.py | 36 ++++++++++++++++++++++++++----- tests/kolla_drift/test_archive.py | 29 +++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/osism_drift/archive.py b/src/osism_drift/archive.py index 38f6840b4..926d57bea 100644 --- a/src/osism_drift/archive.py +++ b/src/osism_drift/archive.py @@ -42,14 +42,40 @@ def _safe_extract(data, dest): except tarfile.FilterError as e: raise SourceError(f"unsafe archive member: {e}") from e else: + root = dest.resolve() + + def _within(p): + return p == root or root in p.parents + for m in tar.getmembers(): target = (dest / m.name).resolve() - root = dest.resolve() - if target != root and root not in target.parents: + if not _within(target): raise SourceError(f"unsafe archive member path: {m.name}") - if not (m.isfile() or m.isdir()): - raise SourceError(f"unsafe archive member (link/special): {m.name}") - tar.extractall(dest) + # Emulate the 'data' filter for older Pythons: permit links + # whose target stays inside the extraction root (the release + # repo aliases per-version ceph.yml/openstack.yml this way), + # reject links that escape and any other special member. + if m.issym(): + link = (target.parent / m.linkname).resolve() + if not _within(link): + raise SourceError( + f"unsafe archive member (link escapes): {m.name}" + ) + elif m.islnk(): + link = (dest / m.linkname).resolve() + if not _within(link): + raise SourceError( + f"unsafe archive member (link escapes): {m.name}" + ) + elif not (m.isfile() or m.isdir()): + raise SourceError(f"unsafe archive member (special): {m.name}") + # Members are already validated above. Old Pythons (no data_filter) + # extract fully-trusted by default; newer ones need it stated so + # they don't consult the (absent) default filter. + try: + tar.extractall(dest, filter="fully_trusted") + except TypeError: + tar.extractall(dest) def _extract_snapshot(data, repo, ref): diff --git a/tests/kolla_drift/test_archive.py b/tests/kolla_drift/test_archive.py index 2024d9722..5e03dd1ee 100644 --- a/tests/kolla_drift/test_archive.py +++ b/tests/kolla_drift/test_archive.py @@ -118,8 +118,9 @@ def test_unsafe_traversal_member_rejected(): @responses.activate -def test_symlink_member_rejected_on_fallback(monkeypatch): - # Force the no-data_filter fallback path and confirm it rejects a link member. +def test_absolute_symlink_member_rejected_on_fallback(monkeypatch): + # Force the no-data_filter fallback path and confirm it rejects a link + # whose target escapes the extraction dir (absolute path). monkeypatch.delattr(tarfile, "data_filter", raising=False) cfg = _stub() link = tarfile.TarInfo("osism-r-abc/link") @@ -133,3 +134,27 @@ def test_symlink_member_rejected_on_fallback(monkeypatch): ) with pytest.raises(SourceError): archive.snapshot_dir("osism", "r", "main", cfg) + + +@responses.activate +def test_safe_relative_symlink_extracted_on_fallback(monkeypatch): + # Force the no-data_filter fallback path. A relative symlink that stays + # inside the extraction dir (as the release repo uses for its per-version + # ceph.yml / openstack.yml aliases) must be extracted, not rejected. + monkeypatch.delattr(tarfile, "data_filter", raising=False) + cfg = _stub() + link = tarfile.TarInfo("osism-release-abc/1.0.0/ceph.yml") + link.type = tarfile.SYMTYPE + link.linkname = "ceph-pacific.yml" + responses.add( + responses.GET, + "https://api.github.com/repos/osism/release/tarball/main", + body=_targz( + {"1.0.0/ceph-pacific.yml": b"pacific\n"}, + top="osism-release-abc", + extra_members=[link], + ), + status=200, + ) + d = archive.snapshot_dir("osism", "release", "main", cfg) + assert (d / "1.0.0/ceph.yml").read_bytes() == b"pacific\n"