From 0a4145f8c325e5f869f042dd92a92e856972a148 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Sat, 1 Feb 2025 15:22:20 +0100 Subject: [PATCH 1/3] _subprocess: avoid leaking pipe FDs Signed-off-by: William Woodruff --- pip_audit/_subprocess.py | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/pip_audit/_subprocess.py b/pip_audit/_subprocess.py index fb021a4c..504f6d48 100644 --- a/pip_audit/_subprocess.py +++ b/pip_audit/_subprocess.py @@ -34,10 +34,6 @@ def run(args: Sequence[str], *, log_stdout: bool = False, state: AuditState = Au the process's `stdout` stream as a string. """ - # Run the process with unbuffered I/O, to make the poll-and-read loop below - # more responsive. - process = Popen(args, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - # NOTE(ww): We frequently run commands inside of ephemeral virtual environments, # which have long absolute paths on some platforms. These make for confusing # state updates, so we trim the first argument down to its basename. @@ -47,22 +43,25 @@ def run(args: Sequence[str], *, log_stdout: bool = False, state: AuditState = Au stdout = b"" stderr = b"" - # NOTE: We use `poll()` to control this loop instead of the `read()` call - # to prevent deadlocks. Similarly, `read(size)` will return an empty bytes - # once `stdout` hits EOF, so we don't have to worry about that blocking. - while not terminated: - terminated = process.poll() is not None - stdout += process.stdout.read() # type: ignore - stderr += process.stderr.read() # type: ignore - state.update_state( - f"Running {pretty_args}", - stdout.decode(errors="replace") if log_stdout else None, - ) + # Run the process with unbuffered I/O, to make the poll-and-read loop below + # more responsive. + with Popen(args, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: + # NOTE: We use `poll()` to control this loop instead of the `read()` call + # to prevent deadlocks. Similarly, `read(size)` will return an empty bytes + # once `stdout` hits EOF, so we don't have to worry about that blocking. + while not terminated: + terminated = process.poll() is not None + stdout += process.stdout.read() # type: ignore + stderr += process.stderr.read() # type: ignore + state.update_state( + f"Running {pretty_args}", + stdout.decode(errors="replace") if log_stdout else None, + ) - if process.returncode != 0: - raise CalledProcessError( - f"{pretty_args} exited with {process.returncode}", - stderr=stderr.decode(errors="replace"), - ) + if process.returncode != 0: + raise CalledProcessError( + f"{pretty_args} exited with {process.returncode}", + stderr=stderr.decode(errors="replace"), + ) return stdout.decode("utf-8", errors="replace") From ae1cb3601148180e1089ef4fdac614a180fdaf07 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Sun, 2 Feb 2025 01:16:31 +0100 Subject: [PATCH 2/3] _cli: remove argparse.FileType use This API always leaks a file handle. Signed-off-by: William Woodruff --- pip_audit/_cli.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pip_audit/_cli.py b/pip_audit/_cli.py index 70186ba4..0c6f9f11 100644 --- a/pip_audit/_cli.py +++ b/pip_audit/_cli.py @@ -209,7 +209,7 @@ def _parser() -> argparse.ArgumentParser: # pragma: no cover dep_source_args.add_argument( "-r", "--requirement", - type=argparse.FileType("r"), + type=Path, metavar="REQUIREMENT", action="append", dest="requirements", @@ -465,9 +465,12 @@ def audit() -> None: # pragma: no cover source: DependencySource if args.requirements is not None: - req_files: list[Path] = [Path(req.name) for req in args.requirements] + for req in args.requirements: + if not req.exists(): + _fatal(f"invalid requirements input: {req}") + source = RequirementSource( - req_files, + args.requirements, require_hashes=args.require_hashes, no_deps=args.no_deps, disable_pip=args.disable_pip, From 1cda1b1e38e145ed5a020600d9b5b6a8b54e1e75 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Sun, 2 Feb 2025 01:21:25 +0100 Subject: [PATCH 3/3] record changes Signed-off-by: William Woodruff --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7574bb2a..7058a09a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ All versions prior to 0.0.9 are untracked. * Auditing a fully-pinned requirements file with `--disable-pip` now allows for duplicates, so long as the duplicates don't have conflicting specifier sets ([#749](https://github.com/pypa/pip-audit/pull/749)) +* Fixed two sources of unnecessary resource leaks when doing file I/O + ([#878](https://github.com/pypa/pip-audit/pull/878)) ## [2.7.3]