From 782681f9043ed8ae5a7ec88da787660d0fb165f0 Mon Sep 17 00:00:00 2001 From: Zyrix Date: Mon, 25 May 2026 03:58:52 +0300 Subject: [PATCH] fix(google_chat): harden oauth credential persistence with atomic private writes (#24788) --- plugins/platforms/google_chat/oauth.py | 72 ++++++++++++++++------- tests/gateway/test_google_chat.py | 79 ++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 22 deletions(-) diff --git a/plugins/platforms/google_chat/oauth.py b/plugins/platforms/google_chat/oauth.py index 7c54726b8ad..d18aaab0cb6 100644 --- a/plugins/platforms/google_chat/oauth.py +++ b/plugins/platforms/google_chat/oauth.py @@ -61,6 +61,8 @@ import json import logging import os import re +import secrets +import stat import subprocess import sys from pathlib import Path @@ -89,6 +91,8 @@ except (ModuleNotFoundError, ImportError): except ValueError: return str(home) +from utils import atomic_replace + def _hermes_home() -> Path: """Resolve HERMES_HOME at call time (NOT module import). @@ -296,14 +300,11 @@ def list_authorized_emails() -> List[str]: def _persist_credentials(creds: Any, token_path: Path) -> None: - """Atomic-ish JSON write of refreshed credentials.""" + """Persist refreshed credentials atomically with private permissions.""" try: - token_path.parent.mkdir(parents=True, exist_ok=True) - token_path.write_text( - json.dumps( - _normalize_authorized_user_payload(json.loads(creds.to_json())), - indent=2, - ) + _write_private_json( + token_path, + _normalize_authorized_user_payload(json.loads(creds.to_json())), ) except Exception: logger.debug( @@ -325,6 +326,38 @@ def _normalize_authorized_user_payload(payload: dict) -> dict: return normalized +def _write_private_json(path: Path, data: Any) -> None: + """Atomically write JSON with 0o600 permissions where supported.""" + path.parent.mkdir(parents=True, exist_ok=True) + try: + os.chmod(path.parent, 0o700) + except OSError: + pass + + tmp_path = path.with_suffix(f".tmp.{os.getpid()}.{secrets.token_hex(4)}") + try: + fd = os.open( + str(tmp_path), + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + stat.S_IRUSR | stat.S_IWUSR, + ) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(data, fh, indent=2, ensure_ascii=False) + fh.flush() + os.fsync(fh.fileno()) + atomic_replace(tmp_path, path) + try: + os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) + except OSError: + pass + finally: + try: + if tmp_path.exists(): + tmp_path.unlink() + except OSError: + pass + + def _ensure_deps() -> None: """Check deps available; install if not; exit on failure.""" try: @@ -402,25 +435,21 @@ def store_client_secret(path: str) -> None: sys.exit(1) target = _client_secret_path() - target.parent.mkdir(parents=True, exist_ok=True) - target.write_text(json.dumps(data, indent=2)) + _write_private_json(target, data) print(f"OK: Client secret saved to {target}") def _save_pending_auth(*, state: str, code_verifier: str, email: Optional[str] = None) -> None: pending = _pending_auth_path(email) - pending.parent.mkdir(parents=True, exist_ok=True) - pending.write_text( - json.dumps( - { - "state": state, - "code_verifier": code_verifier, - "redirect_uri": _REDIRECT_URI, - "email": email or "", - }, - indent=2, - ) + _write_private_json( + pending, + { + "state": state, + "code_verifier": code_verifier, + "redirect_uri": _REDIRECT_URI, + "email": email or "", + }, ) @@ -548,8 +577,7 @@ def exchange_auth_code(code: str, email: Optional[str] = None) -> None: token_payload["scopes"] = granted_scopes token_path = _token_path(email) - token_path.parent.mkdir(parents=True, exist_ok=True) - token_path.write_text(json.dumps(token_payload, indent=2)) + _write_private_json(token_path, token_payload) _pending_auth_path(email).unlink(missing_ok=True) print(f"OK: Authenticated. Token saved to {token_path}") diff --git a/tests/gateway/test_google_chat.py b/tests/gateway/test_google_chat.py index aee1f41e6f2..b7590278503 100644 --- a/tests/gateway/test_google_chat.py +++ b/tests/gateway/test_google_chat.py @@ -1516,6 +1516,13 @@ class TestSetupFilesSlashCommand: class TestUserOAuthHelper: + @staticmethod + def _assert_private_json_file(path, expected): + assert json.loads(path.read_text(encoding="utf-8")) == expected + assert list(path.parent.glob(f"{path.stem}.tmp.*")) == [] + if os.name != "nt": + assert (path.stat().st_mode & 0o777) == 0o600 + def test_load_user_credentials_returns_none_when_no_token(self, tmp_path, monkeypatch): """Missing token file is the expected no-op case (user hasn't run /setup-files yet). Must NOT raise.""" @@ -1610,6 +1617,78 @@ class TestUserOAuthHelper: assert a != legacy assert "google_chat_user_oauth_pending" in str(a.parent) + def test_persist_credentials_writes_private_json(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from plugins.platforms.google_chat.oauth import _persist_credentials, _token_path + + creds = type( + "Creds", + (), + { + "to_json": lambda self: json.dumps( + { + "client_id": "cid", + "client_secret": "secret", + "refresh_token": "rtok", + "token": "atok", + } + ) + }, + )() + + path = _token_path("alice@example.com") + _persist_credentials(creds, path) + + self._assert_private_json_file( + path, + { + "client_id": "cid", + "client_secret": "secret", + "refresh_token": "rtok", + "token": "atok", + "type": "authorized_user", + }, + ) + + def test_store_client_secret_writes_private_json(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + src = tmp_path / "client_secret.json" + payload = {"installed": {"client_id": "cid", "client_secret": "secret"}} + src.write_text(json.dumps(payload), encoding="utf-8") + + from plugins.platforms.google_chat.oauth import ( + _client_secret_path, + store_client_secret, + ) + + store_client_secret(str(src)) + + self._assert_private_json_file(_client_secret_path(), payload) + + def test_save_pending_auth_writes_private_json(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from plugins.platforms.google_chat.oauth import ( + _REDIRECT_URI, + _pending_auth_path, + _save_pending_auth, + ) + + _save_pending_auth( + state="state-123", + code_verifier="verifier-abc", + email="alice@example.com", + ) + + self._assert_private_json_file( + _pending_auth_path("alice@example.com"), + { + "state": "state-123", + "code_verifier": "verifier-abc", + "redirect_uri": _REDIRECT_URI, + "email": "alice@example.com", + }, + ) + class TestPerUserAttachmentRouting: """The bot must use the *requesting user's* OAuth token when sending