Dataset builders download only into a directory you own

scripts/build_breadth_v1.py

@@ -35,7 +35,7 @@
 from kev.data import materialize  # noqa: E402
 from kev.model import fits, load_tokenizer, user_tokens  # noqa: E402
 from kev.suite import ADMISSION_TOKENIZER as TOKENIZER, GIT_LIMIT, SERVING_CONTEXT_8K as SERVING_CONTEXT, digest, normalise_text, read_json, read_jsonl, text_digest, write_json, write_jsonl  # noqa: E402
-from scripts.build_devtools_v1 import deal_groups, fetch, hub_file, line_safe, parquet_rows, round_robin, state_key  # noqa: E402
+from scripts.build_devtools_v1 import deal_groups, fetch, hub_file, line_safe, parquet_rows, private_dir, round_robin, state_key  # noqa: E402
 
 VERSION = SEED = "breadth-v1"
 PARTITIONS = ("development", "test")
@@ -915,7 +915,7 @@ def main():
     ap.add_argument("--only", help="comma-separated datasets (a dry run: prints counts, writes nothing)")
     a = ap.parse_args()
     out, raw = Path(a.out), Path(a.raw)
-    raw.mkdir(parents=True, exist_ok=True)
+    raw = private_dir(raw)
     if out.exists(): raise FileExistsError(out)
     tok = load_tokenizer(*TOKENIZER)
     only = set(a.only.split(",")) if a.only else None

scripts/build_devtools_v1.py

@@ -55,7 +55,7 @@
 The ids and keys also order candidates (deduplication keeps the first by id), so a new version selects different records;
 a version that must keep devtools-v1's evaluation groups out of its training partition has to check that by group_id.
 """
-import argparse, csv, difflib, hashlib, io, json, random, re, subprocess, sys, tarfile, zipfile
+import argparse, csv, difflib, hashlib, io, json, os, random, re, stat, subprocess, sys, tarfile, tempfile, zipfile
 from collections import Counter, defaultdict
 from pathlib import Path
 
@@ -286,13 +286,32 @@ def hub_file(repo, revision, filename):
     return Path(hf_hub_download(repo, filename, repo_type="dataset", revision=revision))
 
 
+def private_dir(path):
+    """The directory downloads land in: created 0700 if missing, and on POSIX it must be a real directory (a symlink is
+    refused, not followed) owned by this user with no group/other write bits. A shared or writable cache lets another
+    local user plant a file where a download lands."""
+    path = Path(path)
+    path.mkdir(mode=0o700, parents=True, exist_ok=True)
+    if os.name != "posix": return path
+    st = path.lstat()
+    if not stat.S_ISDIR(st.st_mode) or st.st_uid != os.getuid() or st.st_mode & 0o022:
+        raise ValueError(f"{path} is not a private download directory (owner uid {st.st_uid}, mode {stat.filemode(st.st_mode)}): "
+                         f"fix it with chmod go-w {path} or pass --raw <a directory of your own>")
+    return path
+
+
 def fetch(url, path, sha256=None, md5=None):
     path = Path(path)
+    private_dir(path.parent)
     if not path.exists():
-        path.parent.mkdir(parents=True, exist_ok=True)
         print(f"downloading {url} -> {path}", flush=True)
-        subprocess.run(["curl", "-sSfL", "-o", str(path) + ".part", url], check=True)
-        Path(str(path) + ".part").rename(path)
+        fd, temp = tempfile.mkstemp(dir=path.parent, prefix=f".{path.name}.", suffix=".part")
+        os.close(fd)
+        try:
+            subprocess.run(["curl", "-sSfL", "-o", temp, url], check=True)
+            os.replace(temp, path)
+        finally:
+            Path(temp).unlink(missing_ok=True)
     if md5 and hashlib.md5(path.read_bytes()).hexdigest() != md5: raise ValueError(f"md5 mismatch: {path}")
     if sha256 and digest(path) != sha256: raise ValueError(f"sha256 mismatch: {path}")
     return path
@@ -734,7 +753,7 @@ def main():
     ap.add_argument("--resolve-licences", action="store_true", help="refresh the CodeReviewer project licences with `gh api` into scripts/devtools_v1_licences.json")
     ap.add_argument("--reproduce-v1", action="store_true", help="rebuild the frozen devtools-v1 byte for byte: its CodeReviewer ids, state keys and admission check (module docstring)")
     a = ap.parse_args()
-    raw = Path(a.raw)
+    raw = private_dir(a.raw)
     if a.resolve_licences: return resolve_licences(raw)
     if not a.out: ap.error("--out is required")
     out = Path(a.out)

tests/test_devtools_v1.py

@@ -1,7 +1,7 @@
 """The devtools-v1 builder's label mappings and selection rules (scripts/build_devtools_v1.py) on small synthetic inputs.
 No weights, no network. Run: uv run python -m pytest tests/test_devtools_v1.py -q
 """
-import random
+import os, random
 from collections import Counter
 from pathlib import Path
 
@@ -236,3 +236,60 @@ def test_a_new_build_admits_commits_on_their_final_record():
     assert len(labels) >= 6 and sum(labels) == len(labels) // 2
     assert all("message_match" not in x["questions"] and "shown_message" not in x["_meta"] for x in pool)   # a redo starts from clean candidates
     assert choose("commitpackft", commits(), 8, everyone, "seed", final_ok=line_safe)[0] == new
+
+
+POSIX_ONLY = pytest.mark.skipif(os.name != "posix", reason="the download-dir checks are POSIX lstat/mode rules")
+
+
+@POSIX_ONLY
+def test_private_dir_accepts_owned_dirs_and_creates_missing(tmp_path):
+    """A missing directory is created 0700; an existing user-owned directory (even 0755, as older caches were made)
+    passes; a symlink to one does not."""
+    import stat
+    from scripts.build_devtools_v1 import private_dir
+    new = private_dir(tmp_path / "fresh" / "nested")
+    assert new.is_dir() and stat.S_IMODE(new.lstat().st_mode) == 0o700
+    owned = tmp_path / "owned"; owned.mkdir(mode=0o755)
+    assert private_dir(owned) == owned
+    link = tmp_path / "link"; link.symlink_to(owned, target_is_directory=True)
+    with pytest.raises(ValueError, match="not a private download directory"):
+        private_dir(link)
+
+
+@POSIX_ONLY
+def test_fetch_downloads_through_a_private_temp_file(tmp_path):
+    """fetch writes to a securely created .<name>.XXXXXX.part inside the private directory and os.replace()s it over the
+    target: a planted <name>.part symlink is left alone where the old code's curl -o wrote through it onto the victim."""
+    import hashlib
+    from scripts.build_devtools_v1 import fetch
+    src = tmp_path / "src"; src.write_bytes(b"payload")
+    raw = tmp_path / "raw"; raw.mkdir(mode=0o700)
+    victim = tmp_path / "victim"; victim.write_text("keep me", encoding="utf-8")
+    (raw / "data.bin.part").symlink_to(victim)
+    target = fetch(src.as_uri(), raw / "data.bin", sha256=hashlib.sha256(b"payload").hexdigest())
+    assert target.read_bytes() == b"payload"
+    assert victim.read_text(encoding="utf-8") == "keep me" and (raw / "data.bin.part").is_symlink()
+    assert sorted(p.name for p in raw.iterdir()) == ["data.bin", "data.bin.part"]   # no temp files remain
+
+
+@POSIX_ONLY
+def test_fetch_refuses_a_writable_directory(tmp_path):
+    """A group/world-writable download directory is refused before anything is fetched (that is where the planted
+    .part symlink attack lived)."""
+    from scripts.build_devtools_v1 import fetch
+    src = tmp_path / "src"; src.write_bytes(b"payload")
+    shared = tmp_path / "shared"; shared.mkdir(); shared.chmod(0o777)
+    with pytest.raises(ValueError, match="not a private download directory"):
+        fetch(src.as_uri(), shared / "data.bin")
+    assert not list(shared.iterdir())
+
+
+@POSIX_ONLY
+def test_fetch_a_failed_download_leaves_nothing(tmp_path):
+    """curl failing leaves neither the temp file nor the target behind."""
+    import subprocess
+    from scripts.build_devtools_v1 import fetch
+    raw = tmp_path / "raw"; raw.mkdir(mode=0o700)
+    with pytest.raises(subprocess.CalledProcessError):
+        fetch((tmp_path / "missing").as_uri(), raw / "gone.bin")
+    assert not (raw / "gone.bin").exists() and not list(raw.iterdir())