From 6d6526bcf6e0eedce9f445afef2928dc32ec6c31 Mon Sep 17 00:00:00 2001 From: Emily Bettencourt Date: Wed, 1 Apr 2026 11:12:27 -0700 Subject: [PATCH] Fix ResourceWarning: unclosed file in credential provider subprocess Use subprocess.DEVNULL instead of subprocess.PIPE for stdin (which was never used) and wrap Popen in a context manager so stdout/stderr pipes are automatically closed. Fixes https://github.com/microsoft/artifacts-keyring/issues/103 --- src/artifacts_keyring/plugin.py | 73 ++++++++++++++++----------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/src/artifacts_keyring/plugin.py b/src/artifacts_keyring/plugin.py index 5954019..58ddd3f 100644 --- a/src/artifacts_keyring/plugin.py +++ b/src/artifacts_keyring/plugin.py @@ -106,7 +106,7 @@ def _get_credentials_from_credential_provider(self, url, is_retry): os.environ[self._NON_INTERACTIVE_VAR_NAME] and \ str(os.environ[self._NON_INTERACTIVE_VAR_NAME]).lower() == "true" - proc = Popen( + with Popen( self.exe + [ "-Uri", url, "-IsRetry", str(is_retry), @@ -114,41 +114,40 @@ def _get_credentials_from_credential_provider(self, url, is_retry): "-CanShowDialog", "True", "-OutputFormat", "Json" ], - stdin=subprocess.PIPE, + stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE - ) - - # Read all standard error first, which may either display - # errors from the credential provider or instructions - # from it for Device Flow authentication. - for stderr_line in iter(proc.stderr.readline, b''): - line = stderr_line.decode("utf-8", "ignore") - sys.stderr.write(line) - sys.stderr.flush() - - proc.wait() - - if proc.returncode != 0: - stderr = proc.stderr.read().decode("utf-8", "ignore") - - error_msg = "Failed to get credentials: process with PID {pid} exited with code {code}".format( - pid=proc.pid, code=proc.returncode - ) - if stderr.strip(): - error_msg += "; additional error message: {error}".format(error=stderr) - else: - error_msg += "; no additional error message available, see Credential Provider logs above for details." - raise RuntimeError(error_msg) - - try: - # stdout is expected to be UTF-8 encoded JSON, so decoding errors are not ignored here. - payload = proc.stdout.read().decode("utf-8") - except ValueError: - raise RuntimeError("Failed to get credentials: the Credential Provider's output could not be decoded using UTF-8.") - - try: - parsed = json.loads(payload) - return parsed["Username"], parsed["Password"] - except ValueError: - raise RuntimeError("Failed to get credentials: the Credential Provider's output could not be parsed as JSON.") + ) as proc: + # Read all standard error first, which may either display + # errors from the credential provider or instructions + # from it for Device Flow authentication. + for stderr_line in iter(proc.stderr.readline, b''): + line = stderr_line.decode("utf-8", "ignore") + sys.stderr.write(line) + sys.stderr.flush() + + proc.wait() + + if proc.returncode != 0: + stderr = proc.stderr.read().decode("utf-8", "ignore") + + error_msg = "Failed to get credentials: process with PID {pid} exited with code {code}".format( + pid=proc.pid, code=proc.returncode + ) + if stderr.strip(): + error_msg += "; additional error message: {error}".format(error=stderr) + else: + error_msg += "; no additional error message available, see Credential Provider logs above for details." + raise RuntimeError(error_msg) + + try: + # stdout is expected to be UTF-8 encoded JSON, so decoding errors are not ignored here. + payload = proc.stdout.read().decode("utf-8") + except ValueError: + raise RuntimeError("Failed to get credentials: the Credential Provider's output could not be decoded using UTF-8.") + + try: + parsed = json.loads(payload) + return parsed["Username"], parsed["Password"] + except ValueError: + raise RuntimeError("Failed to get credentials: the Credential Provider's output could not be parsed as JSON.")