Skip to content

fix: fail the PHI scan closed and drop the pip runtime dependency - #21

Merged
abrichr merged 3 commits into
mainfrom
fix/ledger-2026-10-10
Oct 14, 2026
Merged

abrichr merged 3 commits into
mainfrom
fix/ledger-2026-10-10

Conversation

@abrichr

@abrichr abrichr commented Oct 10, 2026

Copy link
Copy Markdown
Member

openadapt-privacy-scan reported "PHI scan clean" and exited 0 when it hadn't read anything: a missing root, a root that's a file, an empty directory, and directories or files it couldn't open all passed the gate. The scan now exits 2 when it can't run and reports a file it can't read as a finding. This change also removes pip>=25.3 from the runtime dependencies, so installing the library no longer upgrades the user's pip.

Fixes

Problem Change Regression test Ref
The PHI scan printed "PHI scan clean (0 files)" and exited 0 when --root was missing, was a file, or was an empty directory. It also skipped directories it couldn't list and files it couldn't open, and still reported clean. main() exits 2 with "PHI scan error: ... Nothing was scanned." when the root is missing, isn't a directory, or can't be accessed; when a directory under it can't be listed; or when there are no files to scan. A new --allow-empty flag accepts an empty tree. os.walk now raises on an unreadable directory, and a git ls-files permission warning about a path in the tree fails the listing (warnings about an unreadable global ignore file don't). Each listed path is checked with os.stat, so a permission error isn't read as "not a file" on Python 3.14. A file that can't be opened is reported as unreadable (exit 1). iter_scan_files and scan_tree raise OSError for a missing or non-directory root. The README lists exit codes 0, 1, and 2. tests/test_scan.py: test_cli_missing_root_fails_closed, test_cli_file_root_fails_closed, test_cli_empty_root_fails_closed, test_cli_empty_root_passes_with_allow_empty, test_scan_tree_missing_root_raises, test_cli_unreadable_subdirectory_fails_closed, test_cli_unsearchable_subdirectory_fails_closed, test_cli_root_under_unsearchable_parent_fails_closed, test_cli_unreadable_file_is_a_finding, test_cli_unreadable_global_git_ignore_still_scans B123
pip>=25.3 was a runtime dependency, so installing openadapt-privacy (or a package that depends on it) upgraded the user's pip. The library never uses pip. Remove pip>=25.3 from [project].dependencies. The CI job that downloads the spaCy model, which is the only step that needs pip, installs pip>=25.3 itself. tests/test_packaging.py::test_runtime_and_user_extras_do_not_require_install_tools B124

Not fixed here

  • scan_tree() still returns an empty list for a tree with no files, because callers use it as a plain function, not a gate. The README now says to check iter_scan_files(root) first when an empty tree should fail.
  • Other OpenAdapt packages may declare the same pip requirement. They weren't checked in this change.
  • Running the scanner on this repository still exits 1, because of two PNG assets and an MRN-shaped test string. That's unchanged.

How it was tested

  • Every new scan test except test_cli_unreadable_global_git_ignore_still_scans fails on main, where the scan exits 0, and passes on this branch. That test covers a case main already handles: it makes sure a warning about an unreadable global git ignore file doesn't stop a scan that listed every file.
  • test_cli_unsearchable_subdirectory_fails_closed and test_cli_root_under_unsearchable_parent_fails_closed also pass on Python 3.14, where Path.is_file() hides permission errors.
  • The packaging test fails on main, where it finds pip>=25.3, and passes on this branch. The built wheel's metadata has no pip requirement.
  • Full suite without Presidio: 120 passed and 2 skipped on Python 3.10 and 3.11; 120 passed and 3 skipped on Python 3.12. The scanner and packaging tests pass on Python 3.14.
  • ruff check passes on the changed files. The 5 ruff check . errors in scripts/generate_demo_images.py, tests/test_fixtures.py, and tests/test_loaders.py are also on main.
  • uv build produces the sdist and wheel, and scripts/check_source_boundary.py --require-dist passes.

Before merging

  • Merging this to main runs the release workflow, which publishes openadapt-privacy 1.1.1 to PyPI. Downstream packages stop pulling in pip only after 1.1.1 is published.
  • Check every clinic CI job that runs openadapt-privacy-scan. A missing, empty, or inaccessible --root now fails with exit 2 where it used to pass. Add --allow-empty only to a job that's expected to scan an empty tree.

🤖 Generated with Claude Code

abrichr and others added 3 commits October 9, 2026 23:58
openadapt-privacy-scan printed "PHI scan clean (0 files)" and exited 0
when --root did not exist, pointed at a file, or pointed at an empty
directory. It also skipped directories and files it could not read and
still reported clean. A mistyped path in CI turned the gate into a
silent pass.

The scan now exits 2 when it cannot run: the root is missing or is not a
directory, a directory under it cannot be listed (from os.walk or from
git ls-files, which warns and exits 0), or it has no files to scan.
--allow-empty opts in to an empty tree. A file that cannot be opened is
reported as an unreadable finding (exit 1) instead of being skipped as
binary. scan_tree() raises OSError for a missing or non-directory root.
The clean message counts the files that were scanned, and the README
lists the exit codes.

Ledger: B123

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pyproject.toml listed "pip>=25.3" as a runtime dependency. The package
never imports or runs pip, but installing openadapt-privacy (or any
package that depends on it) into an environment with an older pip
upgraded the user's pip, and it conflicted with environments that pin
pip on purpose.

Remove pip from the runtime dependencies. Keep the pip>=25.3 floor in
the one CI step that installs pip for the spaCy model download. A new
test checks that the runtime dependencies and user extras do not list
pip, setuptools, wheel, or uv.

Ledger: B124

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Treat a git permission warning as a failed listing only when it names a
  path in the tree. An unreadable global ignore file under $HOME no longer
  turns a complete scan into "Nothing was scanned" (exit 2).
- Check each listed path with os.stat and raise on a permission error.
  On Python 3.14, Path.is_file() returns False instead, so files under a
  directory without search permission dropped out and the scan reported clean.
- Exit 2, not a traceback, when the root's parent can't be searched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr
abrichr merged commit 08e1dd7 into main Oct 14, 2026
4 checks passed
@abrichr
abrichr deleted the fix/ledger-2026-10-10 branch October 14, 2026 06:28
abrichr added a commit that referenced this pull request Oct 18, 2026
* fix: fail the PHI scan when the root is missing, empty, or unreadable

openadapt-privacy-scan printed "PHI scan clean (0 files)" and exited 0
when --root did not exist, pointed at a file, or pointed at an empty
directory. It also skipped directories and files it could not read and
still reported clean. A mistyped path in CI turned the gate into a
silent pass.

The scan now exits 2 when it cannot run: the root is missing or is not a
directory, a directory under it cannot be listed (from os.walk or from
git ls-files, which warns and exits 0), or it has no files to scan.
--allow-empty opts in to an empty tree. A file that cannot be opened is
reported as an unreadable finding (exit 1) instead of being skipped as
binary. scan_tree() raises OSError for a missing or non-directory root.
The clean message counts the files that were scanned, and the README
lists the exit codes.

Ledger: B123

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: stop requiring pip at install time

pyproject.toml listed "pip>=25.3" as a runtime dependency. The package
never imports or runs pip, but installing openadapt-privacy (or any
package that depends on it) into an environment with an older pip
upgraded the user's pip, and it conflicted with environments that pin
pip on purpose.

Remove pip from the runtime dependencies. Keep the pip>=25.3 floor in
the one CI step that installs pip for the spaCy model download. A new
test checks that the runtime dependencies and user extras do not list
pip, setuptools, wheel, or uv.

Ledger: B124

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: keep the scan's fail-closed checks precise across Python versions

- Treat a git permission warning as a failed listing only when it names a
  path in the tree. An unreadable global ignore file under $HOME no longer
  turns a complete scan into "Nothing was scanned" (exit 2).
- Check each listed path with os.stat and raise on a permission error.
  On Python 3.14, Path.is_file() returns False instead, so files under a
  directory without search permission dropped out and the scan reported clean.
- Exit 2, not a traceback, when the root's parent can't be searched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant