Skip to content

Commit f538ad8

Browse files
authored
fix(auth): handle PermissionError on workload certificates to avoid startup hang and crash (googleapis#17568)
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. Fixes b/522957573
1 parent 0f4bfed commit f538ad8

3 files changed

Lines changed: 122 additions & 25 deletions

File tree

packages/google-auth/google/auth/_agent_identity_utils.py

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,15 @@
1616

1717
import base64
1818
import hashlib
19-
import logging
2019
import os
2120
import re
21+
import stat
2222
import time
2323
from urllib.parse import quote, urlparse
24+
import warnings
2425

2526
from google.auth import environment_vars, exceptions
2627

27-
_LOGGER = logging.getLogger(__name__)
28-
2928
CRYPTOGRAPHY_NOT_FOUND_ERROR = (
3029
"The cryptography library is required for certificate-based authentication."
3130
"Please install it with `pip install google-auth[cryptography]`."
@@ -58,8 +57,20 @@
5857

5958

6059
def _is_certificate_file_ready(path):
61-
"""Checks if a file exists and is not empty."""
62-
return path and os.path.exists(path) and os.path.getsize(path) > 0
60+
"""Checks if a file exists, is a regular file, and is not empty."""
61+
if not path:
62+
return False
63+
try:
64+
# Check if the path points to a regular file and is not empty.
65+
# stat.S_ISREG is used instead of os.path.isfile to avoid swallowing
66+
# PermissionError exceptions, which the caller needs to propagate.
67+
st = os.stat(path)
68+
return stat.S_ISREG(st.st_mode) and st.st_size > 0
69+
except PermissionError:
70+
# Propagate PermissionError to let caller handle it (fail-fast or fallback)
71+
raise
72+
except OSError:
73+
return False
6374

6475

6576
def get_agent_identity_certificate_path():
@@ -141,26 +152,28 @@ def get_agent_identity_certificate_path():
141152

142153
# Log a warning on the first failed attempt to load the certificate file
143154
if not has_logged_cert_warning:
144-
_LOGGER.warning(
145-
"Certificate file not ready at %s. Retrying until startup timeout (up to %s seconds total)...",
146-
target_path,
147-
_TOTAL_TIMEOUT,
155+
warnings.warn(
156+
f"Certificate file not ready at {target_path}. Retrying until startup timeout (up to {_TOTAL_TIMEOUT} seconds total)..."
148157
)
149158
has_logged_cert_warning = True
150159

160+
except PermissionError as e:
161+
warnings.warn(
162+
f"Permission denied when accessing certificate config or certificate file: {e}. "
163+
"Token binding protection cannot be enabled. Falling back to unbound tokens."
164+
)
165+
return None
151166
except (IOError, ValueError, KeyError) as e:
152167
if cert_config_path and os.path.exists(cert_config_path):
153168
# If the file exists but has invalid JSON or is unreadable,
154169
# we assume it is in its final format and fail-fast by returning None.
155170
return None
156171

157172
if not has_logged_config_warning and cert_config_path:
158-
_LOGGER.warning(
159-
"Certificate config file not found or incomplete: %s (from %s "
160-
"environment variable). Retrying until startup timeout (up to %s seconds total)...",
161-
e,
162-
environment_vars.GOOGLE_API_CERTIFICATE_CONFIG,
163-
_TOTAL_TIMEOUT,
173+
warnings.warn(
174+
f"Certificate config file not found or incomplete: {e} (from "
175+
f"{environment_vars.GOOGLE_API_CERTIFICATE_CONFIG} environment variable). "
176+
f"Retrying until startup timeout (up to {_TOTAL_TIMEOUT} seconds total)..."
164177
)
165178
has_logged_config_warning = True
166179
pass
@@ -212,8 +225,15 @@ def get_and_parse_agent_identity_certificate():
212225
if not cert_path:
213226
return None
214227

215-
with open(cert_path, "rb") as cert_file:
216-
cert_bytes = cert_file.read()
228+
try:
229+
with open(cert_path, "rb") as cert_file:
230+
cert_bytes = cert_file.read()
231+
except PermissionError as e:
232+
warnings.warn(
233+
f"Failed to read agent identity certificate file at {cert_path}: {e}. "
234+
"Token binding protection cannot be enabled. Falling back to unbound tokens."
235+
)
236+
return None
217237

218238
return parse_certificate(cert_bytes)
219239

packages/google-auth/tests/compute_engine/test__mtls.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,13 @@ def test__parse_mds_mode_invalid(monkeypatch):
7878
_mtls._parse_mds_mode()
7979

8080

81-
@mock.patch("os.path.exists")
81+
@mock.patch("google.auth.compute_engine._mtls.os.path.exists")
8282
def test__certs_exist_true(mock_exists, mock_mds_mtls_config):
8383
mock_exists.return_value = True
8484
assert _mtls._certs_exist(mock_mds_mtls_config) is True
8585

8686

87-
@mock.patch("os.path.exists")
87+
@mock.patch("google.auth.compute_engine._mtls.os.path.exists")
8888
def test__certs_exist_false(mock_exists, mock_mds_mtls_config):
8989
mock_exists.return_value = False
9090
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):
101101
("default", False, False),
102102
],
103103
)
104-
@mock.patch("os.path.exists")
104+
@mock.patch("google.auth.compute_engine._mtls.os.path.exists")
105105
def test_should_use_mds_mtls(
106106
mock_exists, monkeypatch, mtls_mode, certs_exist, expected_result
107107
):

packages/google-auth/tests/test_agent_identity_utils.py

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,27 @@ def test_parse_certificate(self, mock_load_cert):
6565
mock_load_cert.assert_called_once_with(b"cert_bytes")
6666
assert result == mock_load_cert.return_value
6767

68+
@mock.patch("google.auth._agent_identity_utils.os.stat")
69+
def test_is_certificate_file_ready_permission_error(self, mock_stat):
70+
mock_stat.side_effect = PermissionError("Permission denied")
71+
with pytest.raises(PermissionError):
72+
_agent_identity_utils._is_certificate_file_ready("/path/to/cert")
73+
74+
@mock.patch("google.auth._agent_identity_utils.os.stat")
75+
def test_is_certificate_file_ready_os_error(self, mock_stat):
76+
mock_stat.side_effect = OSError("Not found")
77+
# Should swallow the OSError and return False
78+
result = _agent_identity_utils._is_certificate_file_ready("/path/to/cert")
79+
assert result is False
80+
81+
@mock.patch("google.auth._agent_identity_utils.os.stat")
82+
def test_is_certificate_file_ready_not_a_file(self, mock_stat):
83+
import stat
84+
85+
mock_stat.return_value = mock.MagicMock(st_mode=stat.S_IFDIR, st_size=4096)
86+
result = _agent_identity_utils._is_certificate_file_ready("/path/to/cert")
87+
assert result is False
88+
6889
def test__is_agent_identity_certificate_invalid(self):
6990
cert = _agent_identity_utils.parse_certificate(NON_AGENT_IDENTITY_CERT_BYTES)
7091
assert not _agent_identity_utils._is_agent_identity_certificate(cert)
@@ -268,7 +289,7 @@ def test_get_agent_identity_certificate_path_workstation_fail_fast(
268289
assert result is None
269290

270291
@mock.patch("time.sleep")
271-
@mock.patch("os.path.exists")
292+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
272293
def test_get_agent_identity_certificate_path_cert_not_found(
273294
self, mock_exists, mock_sleep, tmpdir, monkeypatch
274295
):
@@ -358,7 +379,7 @@ def test_get_agent_identity_certificate_path_workload_config_missing_cert_path(
358379
mock_sleep.assert_not_called()
359380

360381
@mock.patch("time.sleep")
361-
@mock.patch("os.path.exists")
382+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
362383
@mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready")
363384
def test_get_agent_identity_certificate_path_no_config_but_has_well_known_dir(
364385
self, mock_is_ready, mock_exists, mock_sleep, monkeypatch
@@ -378,7 +399,7 @@ def test_get_agent_identity_certificate_path_no_config_but_has_well_known_dir(
378399
mock_sleep.assert_not_called()
379400

380401
@mock.patch("time.sleep")
381-
@mock.patch("os.path.exists")
402+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
382403
def test_get_agent_identity_certificate_path_no_config_no_well_known_dir(
383404
self, mock_exists, mock_sleep, monkeypatch
384405
):
@@ -396,7 +417,7 @@ def test_get_agent_identity_certificate_path_no_config_no_well_known_dir(
396417
mock_sleep.assert_not_called()
397418

398419
@mock.patch("time.sleep")
399-
@mock.patch("os.path.exists")
420+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
400421
@mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready")
401422
def test_get_agent_identity_certificate_path_no_config_well_known_polling_success(
402423
self, mock_is_ready, mock_exists, mock_sleep, monkeypatch
@@ -415,7 +436,7 @@ def test_get_agent_identity_certificate_path_no_config_well_known_polling_succes
415436
assert mock_sleep.call_count == 1
416437

417438
@mock.patch("time.sleep")
418-
@mock.patch("os.path.exists")
439+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
419440
@mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready")
420441
def test_get_agent_identity_certificate_path_no_config_well_known_polling_timeout(
421442
self, mock_is_ready, mock_exists, mock_sleep, monkeypatch
@@ -433,6 +454,45 @@ def test_get_agent_identity_certificate_path_no_config_well_known_polling_timeou
433454

434455
assert mock_sleep.call_count == len(_agent_identity_utils._POLLING_INTERVALS)
435456

457+
@mock.patch("time.sleep")
458+
@mock.patch("google.auth._agent_identity_utils._is_certificate_file_ready")
459+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
460+
def test_get_agent_identity_certificate_path_permission_error_well_known(
461+
self, mock_exists, mock_is_ready, mock_sleep, monkeypatch
462+
):
463+
monkeypatch.delenv(
464+
environment_vars.GOOGLE_API_CERTIFICATE_CONFIG, raising=False
465+
)
466+
mock_exists.return_value = True
467+
mock_is_ready.side_effect = PermissionError("Permission denied")
468+
469+
# It should fail-fast and return None immediately
470+
result = _agent_identity_utils.get_agent_identity_certificate_path()
471+
assert result is None
472+
mock_sleep.assert_not_called()
473+
474+
@mock.patch("time.sleep")
475+
@mock.patch("google.auth._agent_identity_utils.os.path.exists")
476+
def test_get_agent_identity_certificate_path_permission_error_config(
477+
self, mock_exists, mock_sleep, tmpdir, monkeypatch
478+
):
479+
config_path = tmpdir.join("config.json")
480+
monkeypatch.setenv(
481+
environment_vars.GOOGLE_API_CERTIFICATE_CONFIG, str(config_path)
482+
)
483+
# Mock os.path.exists so ECP workstation fail-fast is not triggered
484+
mock_exists.return_value = True
485+
486+
# Mocking open to raise PermissionError
487+
mock_open = mock.mock_open()
488+
mock_open.side_effect = PermissionError("Permission denied")
489+
490+
with mock.patch("builtins.open", mock_open):
491+
result = _agent_identity_utils.get_agent_identity_certificate_path()
492+
493+
assert result is None
494+
mock_sleep.assert_not_called()
495+
436496
@mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path")
437497
def test_get_and_parse_agent_identity_certificate_opted_out(
438498
self, mock_get_path, monkeypatch
@@ -501,6 +561,23 @@ def test_get_and_parse_agent_identity_certificate_use_client_cert_invalid(
501561
assert result is None
502562
mock_get_path.assert_not_called()
503563

564+
@mock.patch("google.auth._agent_identity_utils.get_agent_identity_certificate_path")
565+
def test_get_and_parse_agent_identity_certificate_file_read_error(
566+
self, mock_get_path, monkeypatch
567+
):
568+
monkeypatch.setenv(
569+
environment_vars.GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES,
570+
"true",
571+
)
572+
mock_get_path.return_value = "/fake/cert.pem"
573+
mock_open = mock.mock_open()
574+
mock_open.side_effect = PermissionError("Permission denied")
575+
576+
with mock.patch("builtins.open", mock_open):
577+
result = _agent_identity_utils.get_and_parse_agent_identity_certificate()
578+
579+
assert result is None
580+
504581
def test_get_cached_cert_fingerprint_no_cert(self):
505582
with pytest.raises(ValueError, match="mTLS connection is not configured."):
506583
_agent_identity_utils.get_cached_cert_fingerprint(None)

0 commit comments

Comments
 (0)