From b007b7173585d331c7ab2a3e36d9baa3167f0a88 Mon Sep 17 00:00:00 2001 From: nbayati <99771966+nbayati@users.noreply.github.com> Date: Wed, 24 Jun 2026 13:45:39 -0700 Subject: [PATCH 1/4] fix(auth): handle PermissionError on workload certificates to avoid startup hang and crash In sandboxed environments (such as when running the gcloud CLI within a snap package manager sandbox), AppArmor rules can block reading the workload credentials directory or certificate files, raising a PermissionError. Previously, `os.path.exists()` caught this PermissionError and returned `False`. The library interpreted this as the files not being ready yet (e.g. due to a startup race) and entered a 30-second retry loop before failing with a RefreshError. This change: 1. Updates `_is_certificate_file_ready` to use `os.stat` and propagate `PermissionError`. 2. Catches `PermissionError` immediately during certificate lookup, logging a warning and falling back to unbound tokens without retrying or crashing. 3. Adds corresponding unit tests. --- .../google/auth/_agent_identity_utils.py | 17 +++++- .../tests/test_agent_identity_utils.py | 52 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/packages/google-auth/google/auth/_agent_identity_utils.py b/packages/google-auth/google/auth/_agent_identity_utils.py index b57c7bc82b52..aa7103c7883c 100644 --- a/packages/google-auth/google/auth/_agent_identity_utils.py +++ b/packages/google-auth/google/auth/_agent_identity_utils.py @@ -59,7 +59,15 @@ def _is_certificate_file_ready(path): """Checks if a file exists and is not empty.""" - return path and os.path.exists(path) and os.path.getsize(path) > 0 + if not path: + return False + try: + return os.path.getsize(path) > 0 + except PermissionError: + # Propagate PermissionError to let caller handle it (fail-fast or fallback) + raise + except OSError: + return False def get_agent_identity_certificate_path(): @@ -148,6 +156,13 @@ def get_agent_identity_certificate_path(): ) has_logged_cert_warning = True + except PermissionError as e: + _LOGGER.warning( + "Permission denied when accessing certificate config or certificate file: %s. " + "Token binding protection cannot be enabled. Falling back to unbound tokens.", + e, + ) + return None except (IOError, ValueError, KeyError) as e: if cert_config_path and os.path.exists(cert_config_path): # If the file exists but has invalid JSON or is unreadable, diff --git a/packages/google-auth/tests/test_agent_identity_utils.py b/packages/google-auth/tests/test_agent_identity_utils.py index 50a47367b9d7..1d8f5a19e687 100644 --- a/packages/google-auth/tests/test_agent_identity_utils.py +++ b/packages/google-auth/tests/test_agent_identity_utils.py @@ -54,6 +54,19 @@ def test_parse_certificate(self, mock_load_cert): mock_load_cert.assert_called_once_with(b"cert_bytes") assert result == mock_load_cert.return_value + @mock.patch("os.path.getsize") + def test_is_certificate_file_ready_permission_error(self, mock_getsize): + mock_getsize.side_effect = PermissionError("Permission denied") + with pytest.raises(PermissionError): + _agent_identity_utils._is_certificate_file_ready("/path/to/cert") + + @mock.patch("os.path.getsize") + def test_is_certificate_file_ready_os_error(self, mock_getsize): + mock_getsize.side_effect = OSError("Not found") + # Should swallow the OSError and return False + result = _agent_identity_utils._is_certificate_file_ready("/path/to/cert") + assert result is False + def test__is_agent_identity_certificate_invalid(self): cert = _agent_identity_utils.parse_certificate(NON_AGENT_IDENTITY_CERT_BYTES) assert not _agent_identity_utils._is_agent_identity_certificate(cert) @@ -395,6 +408,45 @@ def test_get_agent_identity_certificate_path_no_config_well_known_polling_timeou assert mock_sleep.call_count == len(_agent_identity_utils._POLLING_INTERVALS) + @mock.patch("time.sleep") + @mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready") + @mock.patch("os.path.exists") + def test_get_agent_identity_certificate_path_permission_error_well_known( + self, mock_exists, mock_is_ready, mock_sleep, monkeypatch + ): + monkeypatch.delenv( + environment_vars.GOOGLE_API_CERTIFICATE_CONFIG, raising=False + ) + mock_exists.return_value = True + mock_is_ready.side_effect = PermissionError("Permission denied") + + # It should fail-fast and return None immediately + result = _agent_identity_utils.get_agent_identity_certificate_path() + assert result is None + mock_sleep.assert_not_called() + + @mock.patch("time.sleep") + @mock.patch("os.path.exists") + def test_get_agent_identity_certificate_path_permission_error_config( + self, mock_exists, mock_sleep, tmpdir, monkeypatch + ): + config_path = tmpdir.join("config.json") + monkeypatch.setenv( + environment_vars.GOOGLE_API_CERTIFICATE_CONFIG, str(config_path) + ) + # Mock os.path.exists so ECP workstation fail-fast is not triggered + mock_exists.return_value = True + + # Mocking open to raise PermissionError + mock_open = mock.mock_open() + mock_open.side_effect = PermissionError("Permission denied") + + with mock.patch("builtins.open", mock_open): + result = _agent_identity_utils.get_agent_identity_certificate_path() + + assert result is None + mock_sleep.assert_not_called() + @mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path") def test_get_and_parse_agent_identity_certificate_opted_out( self, mock_get_path, monkeypatch From 981a60a5ef6e044c75eaeb37631acd4b8e2eee45 Mon Sep 17 00:00:00 2001 From: Negar Bayati Date: Thu, 25 Jun 2026 16:49:17 +0000 Subject: [PATCH 2/4] fix(auth): restrict certificate readiness check to regular files --- .../google/auth/_agent_identity_utils.py | 9 +++++++-- .../tests/test_agent_identity_utils.py | 20 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/google-auth/google/auth/_agent_identity_utils.py b/packages/google-auth/google/auth/_agent_identity_utils.py index aa7103c7883c..42e5236d41e3 100644 --- a/packages/google-auth/google/auth/_agent_identity_utils.py +++ b/packages/google-auth/google/auth/_agent_identity_utils.py @@ -19,6 +19,7 @@ import logging import os import re +import stat import time from urllib.parse import quote, urlparse @@ -58,11 +59,15 @@ def _is_certificate_file_ready(path): - """Checks if a file exists and is not empty.""" + """Checks if a file exists, is a regular file, and is not empty.""" if not path: return False try: - return os.path.getsize(path) > 0 + # Check if the path points to a regular file and is not empty. + # stat.S_ISREG is used instead of os.path.isfile to avoid swallowing + # PermissionError exceptions, which the caller needs to propagate. + st = os.stat(path) + return stat.S_ISREG(st.st_mode) and st.st_size > 0 except PermissionError: # Propagate PermissionError to let caller handle it (fail-fast or fallback) raise diff --git a/packages/google-auth/tests/test_agent_identity_utils.py b/packages/google-auth/tests/test_agent_identity_utils.py index 1d8f5a19e687..8d558bc3e9d1 100644 --- a/packages/google-auth/tests/test_agent_identity_utils.py +++ b/packages/google-auth/tests/test_agent_identity_utils.py @@ -54,19 +54,27 @@ def test_parse_certificate(self, mock_load_cert): mock_load_cert.assert_called_once_with(b"cert_bytes") assert result == mock_load_cert.return_value - @mock.patch("os.path.getsize") - def test_is_certificate_file_ready_permission_error(self, mock_getsize): - mock_getsize.side_effect = PermissionError("Permission denied") + @mock.patch("google.auth._agent_identity_utils.os.stat") + def test_is_certificate_file_ready_permission_error(self, mock_stat): + mock_stat.side_effect = PermissionError("Permission denied") with pytest.raises(PermissionError): _agent_identity_utils._is_certificate_file_ready("/path/to/cert") - @mock.patch("os.path.getsize") - def test_is_certificate_file_ready_os_error(self, mock_getsize): - mock_getsize.side_effect = OSError("Not found") + @mock.patch("google.auth._agent_identity_utils.os.stat") + def test_is_certificate_file_ready_os_error(self, mock_stat): + mock_stat.side_effect = OSError("Not found") # Should swallow the OSError and return False result = _agent_identity_utils._is_certificate_file_ready("/path/to/cert") assert result is False + @mock.patch("google.auth._agent_identity_utils.os.stat") + def test_is_certificate_file_ready_not_a_file(self, mock_stat): + import stat + + mock_stat.return_value = mock.MagicMock(st_mode=stat.S_IFDIR, st_size=4096) + result = _agent_identity_utils._is_certificate_file_ready("/path/to/cert") + assert result is False + def test__is_agent_identity_certificate_invalid(self): cert = _agent_identity_utils.parse_certificate(NON_AGENT_IDENTITY_CERT_BYTES) assert not _agent_identity_utils._is_agent_identity_certificate(cert) From 5a7ce2c031c69161502864435f2942a5ff03755b Mon Sep 17 00:00:00 2001 From: Negar Bayati Date: Thu, 25 Jun 2026 20:03:34 +0000 Subject: [PATCH 3/4] test(auth): mock os.path.exists locally to avoid test runner side effects --- .../google-auth/tests/compute_engine/test__mtls.py | 6 +++--- .../google-auth/tests/test_agent_identity_utils.py | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/google-auth/tests/compute_engine/test__mtls.py b/packages/google-auth/tests/compute_engine/test__mtls.py index 2effa29bbdc2..501a90a264a7 100644 --- a/packages/google-auth/tests/compute_engine/test__mtls.py +++ b/packages/google-auth/tests/compute_engine/test__mtls.py @@ -78,13 +78,13 @@ def test__parse_mds_mode_invalid(monkeypatch): _mtls._parse_mds_mode() -@mock.patch("os.path.exists") +@mock.patch("google.auth.compute_engine._mtls.os.path.exists") def test__certs_exist_true(mock_exists, mock_mds_mtls_config): mock_exists.return_value = True assert _mtls._certs_exist(mock_mds_mtls_config) is True -@mock.patch("os.path.exists") +@mock.patch("google.auth.compute_engine._mtls.os.path.exists") def test__certs_exist_false(mock_exists, mock_mds_mtls_config): mock_exists.return_value = False assert _mtls._certs_exist(mock_mds_mtls_config) is False @@ -101,7 +101,7 @@ def test__certs_exist_false(mock_exists, mock_mds_mtls_config): ("default", False, False), ], ) -@mock.patch("os.path.exists") +@mock.patch("google.auth.compute_engine._mtls.os.path.exists") def test_should_use_mds_mtls( mock_exists, monkeypatch, mtls_mode, certs_exist, expected_result ): diff --git a/packages/google-auth/tests/test_agent_identity_utils.py b/packages/google-auth/tests/test_agent_identity_utils.py index 8d558bc3e9d1..1ba79932c6c3 100644 --- a/packages/google-auth/tests/test_agent_identity_utils.py +++ b/packages/google-auth/tests/test_agent_identity_utils.py @@ -251,7 +251,7 @@ def test_get_agent_identity_certificate_path_workstation_fail_fast( assert result is None @mock.patch("time.sleep") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") def test_get_agent_identity_certificate_path_cert_not_found( self, mock_exists, mock_sleep, tmpdir, monkeypatch ): @@ -341,7 +341,7 @@ def test_get_agent_identity_certificate_path_workload_config_missing_cert_path( mock_sleep.assert_not_called() @mock.patch("time.sleep") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") @mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready") def test_get_agent_identity_certificate_path_no_config_but_has_well_known_dir( self, mock_is_ready, mock_exists, mock_sleep, monkeypatch @@ -361,7 +361,7 @@ def test_get_agent_identity_certificate_path_no_config_but_has_well_known_dir( mock_sleep.assert_not_called() @mock.patch("time.sleep") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") def test_get_agent_identity_certificate_path_no_config_no_well_known_dir( self, mock_exists, mock_sleep, monkeypatch ): @@ -379,7 +379,7 @@ def test_get_agent_identity_certificate_path_no_config_no_well_known_dir( mock_sleep.assert_not_called() @mock.patch("time.sleep") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") @mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready") def test_get_agent_identity_certificate_path_no_config_well_known_polling_success( self, mock_is_ready, mock_exists, mock_sleep, monkeypatch @@ -398,7 +398,7 @@ def test_get_agent_identity_certificate_path_no_config_well_known_polling_succes assert mock_sleep.call_count == 1 @mock.patch("time.sleep") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") @mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready") def test_get_agent_identity_certificate_path_no_config_well_known_polling_timeout( self, mock_is_ready, mock_exists, mock_sleep, monkeypatch @@ -418,7 +418,7 @@ def test_get_agent_identity_certificate_path_no_config_well_known_polling_timeou @mock.patch("time.sleep") @mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") def test_get_agent_identity_certificate_path_permission_error_well_known( self, mock_exists, mock_is_ready, mock_sleep, monkeypatch ): @@ -434,7 +434,7 @@ def test_get_agent_identity_certificate_path_permission_error_well_known( mock_sleep.assert_not_called() @mock.patch("time.sleep") - @mock.patch("os.path.exists") + @mock.patch("google.auth._agent_identity_utils.os.path.exists") def test_get_agent_identity_certificate_path_permission_error_config( self, mock_exists, mock_sleep, tmpdir, monkeypatch ): From 022d2ae48d38bf3b639c0c4096307d758c64c388 Mon Sep 17 00:00:00 2001 From: Negar Bayati Date: Thu, 25 Jun 2026 20:25:51 +0000 Subject: [PATCH 4/4] fix(auth): fallback to unbound tokens on certificate permission or read errors --- .../google/auth/_agent_identity_utils.py | 13 +++++++++++-- .../tests/test_agent_identity_utils.py | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/google-auth/google/auth/_agent_identity_utils.py b/packages/google-auth/google/auth/_agent_identity_utils.py index 42e5236d41e3..849ac1c846e6 100644 --- a/packages/google-auth/google/auth/_agent_identity_utils.py +++ b/packages/google-auth/google/auth/_agent_identity_utils.py @@ -225,8 +225,17 @@ def get_and_parse_agent_identity_certificate(): if not cert_path: return None - with open(cert_path, "rb") as cert_file: - cert_bytes = cert_file.read() + try: + with open(cert_path, "rb") as cert_file: + cert_bytes = cert_file.read() + except OSError as e: + _LOGGER.warning( + "Failed to read agent identity certificate file at %s: %s. " + "Token binding protection cannot be enabled. Falling back to unbound tokens.", + cert_path, + e, + ) + return None return parse_certificate(cert_bytes) diff --git a/packages/google-auth/tests/test_agent_identity_utils.py b/packages/google-auth/tests/test_agent_identity_utils.py index 1ba79932c6c3..5488e418e365 100644 --- a/packages/google-auth/tests/test_agent_identity_utils.py +++ b/packages/google-auth/tests/test_agent_identity_utils.py @@ -499,6 +499,23 @@ def test_get_and_parse_agent_identity_certificate_success( mock_parse_certificate.assert_called_once_with(b"cert_bytes") assert result == mock_parse_certificate.return_value + @mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path") + def test_get_and_parse_agent_identity_certificate_file_read_error( + self, mock_get_path, monkeypatch + ): + monkeypatch.setenv( + environment_vars.GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES, + "true", + ) + mock_get_path.return_value = "/fake/cert.pem" + mock_open = mock.mock_open() + mock_open.side_effect = PermissionError("Permission denied") + + with mock.patch("builtins.open", mock_open): + result = _agent_identity_utils.get_and_parse_agent_identity_certificate() + + assert result is None + def test_get_cached_cert_fingerprint_no_cert(self): with pytest.raises(ValueError, match="mTLS connection is not configured."): _agent_identity_utils.get_cached_cert_fingerprint(None)