fix(gdc): harden local service account file loading against LFI and fix static type checking errors

This commit is contained in:
Tim Laubach 2026-07-02 15:28:48 +00:00
parent fabe5c283a
commit cee1d03f34
2 changed files with 42 additions and 1 deletions

View file

@ -155,9 +155,28 @@ class GDCGeminiConfig(OpenAILikeChatConfig):
return token
def _load_creds_from_key(self, api_key: str) -> tuple[Any, bool]:
def _allow_local_file_access(self) -> bool:
"""Whether a caller-supplied service account file path in api_key may be opened.
Gated strictly on the ``GDC_ALLOW_LOCAL_FILE_ACCESS`` environment variable.
"""
return self._read_env_bool(None, "GDC_ALLOW_LOCAL_FILE_ACCESS", default=False)
def _load_creds_from_key(self, api_key: str, model: str | None = None) -> tuple[Any, bool]:
"""Helper to safely parse service account JSON files or strings to reduce complexity."""
import google.auth
# Security hardening: Prevent local file inclusion (LFI) via api_key by default.
# Only allow opening local file paths if explicitly enabled via GDC_ALLOW_LOCAL_FILE_ACCESS env var.
allow_local_file = self._allow_local_file_access()
# Limit length to avoid OSError for 'File name too long'
if allow_local_file and len(api_key) < 2000 and os.path.exists(api_key):
with open(api_key, "r") as f:
json_obj = json.load(f)
creds, _ = google.auth.load_credentials_from_dict(json_obj)
return creds, True
try:
json_obj = json.loads(api_key)
except json.JSONDecodeError:

View file

@ -715,3 +715,25 @@ class TestCompleteGDC:
_, kwargs = mock_completion.call_args
assert kwargs["api_base"] == "https://gdc-specific.com"
@patch("google.auth.load_credentials_from_dict")
@patch("builtins.open", new_callable=MagicMock)
@patch("os.path.exists", return_value=True)
def test_load_creds_from_key_local_file_safety(self, mock_exists, mock_open, mock_load_creds):
config = GDCGeminiConfig()
fake_path = "/tmp/fake_service_account.json"
# 1. By default, local file access must be blocked (prevents LFI via api_key)
mock_open.reset_mock()
creds, is_sa = config._load_creds_from_key(fake_path, TEST_MODEL)
mock_open.assert_not_called()
# 2. When enabled via GDC_ALLOW_LOCAL_FILE_ACCESS env var, file access should be allowed
with patch.dict(os.environ, {"GDC_ALLOW_LOCAL_FILE_ACCESS": "true"}):
mock_open.reset_mock()
mock_open.return_value.__enter__.return_value.read.return_value = '{"type": "service_account"}'
mock_load_creds.return_value = (MagicMock(), None)
creds, is_sa = config._load_creds_from_key(fake_path, TEST_MODEL)
mock_open.assert_called_once_with(fake_path, "r")
assert creds is not None
assert is_sa is True