diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 366e09b..ce95126 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -53,7 +53,7 @@ jobs: - name: Install dependencies with Presidio run: | uv sync --extra dev --extra presidio - uv pip install pip + uv pip install "pip>=25.3" uv run python -m spacy download en_core_web_sm - name: Run all tests diff --git a/README.md b/README.md index bea464e..ec46f6c 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,14 @@ openadapt-privacy-scan --self-test openadapt-privacy-scan --root /path/to/repo ``` +The command exits with: + +- `0` when it read every file and no rule fired. +- `1` when a rule fired or a file couldn't be read. +- `2` when the scan couldn't run: the root is missing, isn't a directory, or + can't be accessed; a directory under it can't be listed; or it has no files. + Pass `--allow-empty` if an empty tree is expected. + `[scan]` adds no packages. The scanner is stdlib, so `import openadapt_privacy.scan` works on a bare `pip install openadapt-privacy` and does not load Presidio, spaCy, or Pillow. `python -m openadapt_privacy.scan` is the same CLI. @@ -57,6 +65,10 @@ hits = scan_tree(Path(".")) # ["ohip-dashed\tfile.txt:3", ...] self_test() # 0 ok, 1 a rule stayed silent ``` +`scan_tree` raises `OSError` when the root is missing, isn't a directory, or has +a directory that can't be listed. It returns `[]` for a tree with no files, so +check `iter_scan_files(root)` first if an empty tree should fail. + `--self-test` plants fixtures under `/tmp` and exits 1 if a rule does not fire. Matching OHIP examples are not stored in the library; they're built at runtime. diff --git a/openadapt_privacy/scan.py b/openadapt_privacy/scan.py index 6e6e962..f961b10 100644 --- a/openadapt_privacy/scan.py +++ b/openadapt_privacy/scan.py @@ -13,6 +13,7 @@ from __future__ import annotations import argparse +import errno import os import re import stat @@ -101,27 +102,67 @@ def _git_files(root: Path) -> list[Path] | None: ], check=True, capture_output=True, + # English messages, so the check below can read them. + env={**os.environ, "LC_ALL": "C"}, ) except (subprocess.CalledProcessError, FileNotFoundError): return None + # git skips a directory it can't read, warns, and still exits 0, so treat a + # permission warning as a failed listing. git names paths in the tree + # relative to it. The exception is an absolute path, such as a global ignore + # file under an unreadable $HOME in a CI container: an ignore file git can't + # read hides no files. + err = out.stderr.decode("utf-8", "replace") + for line in err.splitlines(): + quoted = re.search(r"'([^']*)'", line) + outside = quoted is not None and os.path.isabs(quoted.group(1)) + if "could not open directory" in line or ( + "Permission denied" in line and not outside + ): + raise PermissionError(err.strip()) names = [n for n in out.stdout.split(b"\0") if n] return [root / n.decode("utf-8", "surrogateescape") for n in names] +def _raise(exc: OSError) -> None: + raise exc + + def _walk_files(root: Path) -> list[Path]: files: list[Path] = [] - for dirpath, dirnames, filenames in os.walk(root): + # Raise on an unreadable directory. Skipping it would report a clean scan + # of files that were never read. + for dirpath, dirnames, filenames in os.walk(root, onerror=_raise): dirnames[:] = [d for d in dirnames if d not in SKIP_DIR_NAMES] for name in filenames: files.append(Path(dirpath) / name) return files +def _is_file(path: Path) -> bool: + """Return whether ``path`` is a regular file. Raise if it can't be checked. + + ``Path.is_file()`` returns False on a permission error from Python 3.14, so + a file under a directory that can't be searched would drop out of the scan. + """ + try: + return stat.S_ISREG(path.stat().st_mode) + except OSError as exc: + if exc.errno in (errno.ENOENT, errno.ENOTDIR, errno.EBADF, errno.ELOOP): + return False + raise + + def iter_scan_files(root: Path) -> list[Path]: + """List the files to scan. Raise ``OSError`` if ``root`` can't be listed.""" + if not root.exists(): + raise FileNotFoundError(f"{root} does not exist") + if not root.is_dir(): + raise NotADirectoryError(f"{root} is not a directory") tracked = _git_files(root) if tracked is not None: - return [p for p in tracked if p.is_file()] - return [p for p in _walk_files(root) if p.is_file()] + return [p for p in tracked if _is_file(p)] + return [p for p in _walk_files(root) if _is_file(p)] def _is_binary(path: Path) -> bool: @@ -129,7 +170,9 @@ def _is_binary(path: Path) -> bool: with path.open("rb") as fh: chunk = fh.read(8192) except OSError: - return True + # Not binary as far as we know. scan_contents then reports it as + # unreadable instead of skipping it. + return False return b"\0" in chunk @@ -172,9 +215,14 @@ def scan_contents(root: Path, files: list[Path]) -> list[str]: return hits -def scan_tree(root: Path) -> list[str]: - """Return the same hit lines the clinic git scanner printed.""" - files = iter_scan_files(root) +def scan_tree(root: Path, files: list[Path] | None = None) -> list[str]: + """Return the same hit lines the clinic git scanner printed. + + Raise ``OSError`` if ``root`` is missing, is not a directory, or has a + directory that can't be listed. + """ + if files is None: + files = iter_scan_files(root) return scan_forbidden_paths(root, files) + scan_contents(root, files) @@ -257,19 +305,50 @@ def main(argv: list[str] | None = None) -> int: default=None, help="tree to scan (default: current working directory)", ) + parser.add_argument( + "--allow-empty", + action="store_true", + help="exit 0 when the tree has no files to scan (default: exit 2)", + ) args = parser.parse_args(argv) if args.self_test: return self_test() + # Exit 2 when the scan could not run, so CI can tell it apart from a + # finding (exit 1). A scan of nothing must never report clean. root = (args.root if args.root is not None else Path.cwd()).resolve() - hits = scan_tree(root) + # os.path.isdir is False, not an exception, when a parent can't be searched. + if not os.path.isdir(root): + print( + f"PHI scan error: root {root} does not exist, is not a directory, " + "or can't be accessed. Nothing was scanned.", + file=sys.stderr, + ) + return 2 + try: + files = iter_scan_files(root) + except OSError as exc: + print( + f"PHI scan error: could not list files under {root}: {exc}. " + "Nothing was scanned.", + file=sys.stderr, + ) + return 2 + if not files and not args.allow_empty: + print( + f"PHI scan error: no files to scan under {root}. " + "Pass --allow-empty if an empty tree is expected.", + file=sys.stderr, + ) + return 2 + hits = scan_tree(root, files) if hits: print("PHI / clinic-path scan failed:", file=sys.stderr) for hit in hits: print(hit, file=sys.stderr) return 1 - print(f"PHI scan clean ({len(iter_scan_files(root))} files)") + print(f"PHI scan clean ({len(files)} files)") return 0 diff --git a/pyproject.toml b/pyproject.toml index 329fb5a..450e74c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,7 +23,6 @@ classifiers = [ dependencies = [ "pillow>=10.0.0", - "pip>=25.3", "pydantic>=2.0.0", ] diff --git a/tests/test_packaging.py b/tests/test_packaging.py new file mode 100644 index 0000000..4aecc1a --- /dev/null +++ b/tests/test_packaging.py @@ -0,0 +1,31 @@ +"""Installing the library must not change the user's install tools.""" + +from __future__ import annotations + +from pathlib import Path + +from packaging.requirements import Requirement +from packaging.utils import canonicalize_name + +try: + import tomllib +except ModuleNotFoundError: # Python 3.10 + import tomli as tomllib + +ROOT = Path(__file__).resolve().parents[1] +INSTALL_TOOLS = {"pip", "setuptools", "wheel", "uv"} + + +def test_runtime_and_user_extras_do_not_require_install_tools() -> None: + project = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8"))["project"] + groups = {"dependencies": project.get("dependencies", [])} + for extra, requirements in project.get("optional-dependencies", {}).items(): + if extra != "dev": + groups[f"[{extra}]"] = requirements + offenders = [ + f"{group}: {spec}" + for group, requirements in groups.items() + for spec in requirements + if canonicalize_name(Requirement(spec).name) in INSTALL_TOOLS + ] + assert offenders == [] diff --git a/tests/test_scan.py b/tests/test_scan.py index 2a46b09..0aa0b9a 100644 --- a/tests/test_scan.py +++ b/tests/test_scan.py @@ -3,6 +3,7 @@ from __future__ import annotations import ast +import os import re import subprocess import sys @@ -210,6 +211,160 @@ def test_cli_root_exits_zero_when_clean(tmp_path: Path) -> None: assert main(["--root", str(tmp_path)]) == 0 +def test_cli_missing_root_fails_closed( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + assert main(["--root", str(tmp_path / "nope")]) == 2 + captured = capsys.readouterr() + assert "does not exist" in captured.err + assert "clean" not in captured.out + captured.err + + +def test_cli_file_root_fails_closed( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + target = tmp_path / "file.txt" + target.write_text("hello\n", encoding="utf-8") + assert main(["--root", str(target)]) == 2 + captured = capsys.readouterr() + assert "not a directory" in captured.err + assert "clean" not in captured.out + captured.err + + +@pytest.mark.parametrize("use_git", [False, True], ids=["plain-dir", "git-repo"]) +def test_cli_empty_root_fails_closed( + tmp_path: Path, capsys: pytest.CaptureFixture[str], use_git: bool +) -> None: + if use_git: + _git_init(tmp_path) + assert main(["--root", str(tmp_path)]) == 2 + captured = capsys.readouterr() + assert "no files" in captured.err + assert "clean" not in captured.out + captured.err + + +def test_cli_empty_root_passes_with_allow_empty(tmp_path: Path) -> None: + assert main(["--root", str(tmp_path), "--allow-empty"]) == 0 + + +def test_scan_tree_missing_root_raises(tmp_path: Path) -> None: + with pytest.raises(OSError): + scan_tree(tmp_path / "nope") + + +def _skip_if_root_user() -> None: + if hasattr(os, "geteuid") and os.geteuid() == 0: + pytest.skip("root can read files whatever their permissions") + + +@pytest.mark.parametrize("use_git", [False, True], ids=["plain-dir", "git-repo"]) +def test_cli_unreadable_subdirectory_fails_closed( + tmp_path: Path, capsys: pytest.CaptureFixture[str], use_git: bool +) -> None: + _skip_if_root_user() + if use_git: + _git_init(tmp_path) + (tmp_path / "ok.txt").write_text("hello\n", encoding="utf-8") + locked = tmp_path / "locked" + locked.mkdir() + (locked / "note.txt").write_text("hello\n", encoding="utf-8") + locked.chmod(0) + try: + code = main(["--root", str(tmp_path)]) + finally: + locked.chmod(0o700) + assert code == 2 + captured = capsys.readouterr() + assert "could not list files" in captured.err + assert "clean" not in captured.out + captured.err + + +@pytest.mark.parametrize("use_git", [False, True], ids=["plain-dir", "git-repo"]) +def test_cli_unsearchable_subdirectory_fails_closed( + tmp_path: Path, capsys: pytest.CaptureFixture[str], use_git: bool +) -> None: + # Read but no execute permission: the names list, but the files can't be + # opened. Python 3.14's Path.is_file() returns False here instead of raising. + _skip_if_root_user() + if use_git: + _git_init(tmp_path) + (tmp_path / "ok.txt").write_text("hello\n", encoding="utf-8") + half = tmp_path / "half" + half.mkdir() + (half / "note.txt").write_text("hello\n", encoding="utf-8") + half.chmod(0o444) + try: + code = main(["--root", str(tmp_path)]) + finally: + half.chmod(0o700) + assert code == 2 + captured = capsys.readouterr() + assert "could not list files" in captured.err + assert "clean" not in captured.out + captured.err + + +def test_cli_root_under_unsearchable_parent_fails_closed( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + _skip_if_root_user() + locked = tmp_path / "locked" + root = locked / "sub" + root.mkdir(parents=True) + (root / "ok.txt").write_text("hello\n", encoding="utf-8") + locked.chmod(0) + try: + code = main(["--root", str(root)]) + finally: + locked.chmod(0o700) + assert code == 2 + captured = capsys.readouterr() + assert "can't be accessed" in captured.err + assert "clean" not in captured.out + captured.err + + +def test_cli_unreadable_global_git_ignore_still_scans( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + # git warns "unable to access ...: Permission denied" for an unreadable + # global ignore file, but it still lists every file. That's not a failed scan. + _skip_if_root_user() + repo = tmp_path / "repo" + repo.mkdir() + _git_init(repo) + (repo / "ok.txt").write_text("hello\n", encoding="utf-8") + home = tmp_path / "home" + ignore = home / ".config" / "git" / "ignore" + ignore.parent.mkdir(parents=True) + ignore.write_text("*.txt\n", encoding="utf-8") + ignore.chmod(0) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + try: + code = main(["--root", str(repo)]) + finally: + ignore.chmod(0o600) + captured = capsys.readouterr() + assert code == 0, captured.err + assert "PHI scan clean (1 files)" in captured.out + + +def test_cli_unreadable_file_is_a_finding( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + _skip_if_root_user() + (tmp_path / "ok.txt").write_text("hello\n", encoding="utf-8") + locked = tmp_path / "locked.txt" + locked.write_text("hello\n", encoding="utf-8") + locked.chmod(0) + try: + code = main(["--root", str(tmp_path)]) + finally: + locked.chmod(0o600) + assert code == 1 + captured = capsys.readouterr() + assert "unreadable\tlocked.txt" in captured.err + + def test_module_cli_self_test() -> None: completed = subprocess.run( [sys.executable, "-m", "openadapt_privacy.scan", "--self-test"], @@ -248,3 +403,4 @@ def test_readme_keeps_presidio_caveat_and_has_scan_section() -> None: assert "## Git / CI scan (`[scan]`)" in readme assert "synthetic, not clinical" in readme assert "does not claim clinical validation" in readme + assert "--allow-empty" in readme