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"