From 290acdb59c7efad48db5c2ca7918b7b411536c6b Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Fri, 8 May 2026 13:35:41 -0400 Subject: [PATCH] fix(auth): address PR review comments for Google Workspace OAuth - Secure token file permissions (0o600) in dashboard callback handler - Validate refresh_token presence after code exchange - HTML-escape all dynamic values in callback pages (XSS prevention) - Raise error when only placeholder credentials are available - Fix docstring to match actual behavior (no standalone fallback) - Validate OAuth state parameter in headless mode - Reduce client_id log exposure to 8 chars - Use configurable port for dashboard redirect URI (app.state.bound_port) - Read HERMES_DASHBOARD_PORT env var instead of hardcoding 9119 --- agent/google_workspace_oauth.py | 16 +++++++++++++--- hermes_cli/auth.py | 12 +++++++++++- hermes_cli/web_server.py | 24 +++++++++++++++++++----- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/agent/google_workspace_oauth.py b/agent/google_workspace_oauth.py index 1a51d54c2b..13adc781ce 100644 --- a/agent/google_workspace_oauth.py +++ b/agent/google_workspace_oauth.py @@ -253,6 +253,16 @@ def get_client_credentials() -> Tuple[str, str]: if _NOUS_CLIENT_ID and _NOUS_CLIENT_ID != "PLACEHOLDER.apps.googleusercontent.com": return _NOUS_CLIENT_ID, _NOUS_CLIENT_SECRET + # Placeholder credentials are not usable — raise so callers get a clear error + if _NOUS_CLIENT_ID.startswith("PLACEHOLDER"): + raise GoogleWorkspaceOAuthError( + "No usable Google OAuth client credentials found.\n" + "The bundled Nous Research app is not yet verified. Please either:\n" + " 1. Set HERMES_GOOGLE_WORKSPACE_CLIENT_ID and HERMES_GOOGLE_WORKSPACE_CLIENT_SECRET env vars\n" + " 2. Place a google_client_secret.json in ~/.hermes/\n", + code="google_workspace_oauth_no_client", + ) + # If we only have the placeholder, still return it — caller can decide # whether to proceed or error out. return _NOUS_CLIENT_ID, _NOUS_CLIENT_SECRET @@ -706,8 +716,7 @@ def run_oauth_login() -> dict: saves the token. This function polls the dashboard session until it's approved. - Falls back to a standalone local HTTP server on port 8098 if the - dashboard is not reachable. + If the dashboard is not running, exits with instructions to start it or use --no-browser mode. Returns: The saved token data dict. @@ -727,7 +736,8 @@ def run_oauth_login() -> dict: ) # Try the dashboard flow first (preferred — single source of truth) - dashboard_url = "http://127.0.0.1:9119" + dashboard_port = os.environ.get("HERMES_DASHBOARD_PORT", "9119") + dashboard_url = f"http://127.0.0.1:{dashboard_port}" use_dashboard = _is_dashboard_running(dashboard_url) if use_dashboard: diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 6e99a74cd7..fc934da60c 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1874,14 +1874,24 @@ def login_google_workspace_command(args) -> None: # Extract code from URL if user pasted the full redirect URL code = callback_input + returned_state = None if callback_input.startswith("http"): from urllib.parse import parse_qs, urlparse parsed = urlparse(callback_input) params = parse_qs(parsed.query) + if "error" in params: + print(f"Error from Google: {params['error'][0]}") + raise SystemExit(1) if "code" not in params: print("Error: No 'code' parameter found in the pasted URL.") raise SystemExit(1) code = params["code"][0] + returned_state = params.get("state", [None])[0] + + # Validate state if available + if returned_state and returned_state != state: + print("Error: OAuth state mismatch. This may be a code from a different session.") + raise SystemExit(1) try: token_data = exchange_code(code, state, code_verifier, redirect_uri="http://localhost:1") @@ -1891,7 +1901,7 @@ def login_google_workspace_command(args) -> None: else: # Interactive flow — opens browser, waits for callback print("Starting Google Workspace PKCE login...") - print(f" Client ID: {client_id[:20]}...") + print(f" Client ID: {client_id[:8]}…") print(f" Token will be saved to: {credentials_path()}") print() try: diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 15c63a887c..06531d68e7 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -12,6 +12,7 @@ Usage: import asyncio import base64 import hashlib +import html import hmac import importlib.util import json @@ -2157,7 +2158,9 @@ def _start_google_workspace_pkce() -> Dict[str, Any]: sess["client_secret"] = client_secret # Use localhost:9119 callback so the dashboard catches the redirect automatically - redirect_uri = "http://localhost:9119/auth/google/callback" + # Use the actual bound port if available, default to 9119 + bound_port = getattr(app.state, "bound_port", 9119) + redirect_uri = f"http://localhost:{bound_port}/auth/google/callback" sess["redirect_uri"] = redirect_uri params = { @@ -2242,6 +2245,12 @@ def _submit_google_workspace_pkce(session_id: str, code_input: str) -> Dict[str, sess["error_message"] = "No access token returned" return {"ok": False, "status": "error", "message": sess["error_message"]} + if not refresh_token: + with _oauth_sessions_lock: + sess["status"] = "error" + sess["error_message"] = "No refresh token returned. Ensure 'access_type=offline' and 'prompt=consent' are set." + return {"ok": False, "status": "error", "message": sess["error_message"]} + # Save token in google_token.json format (compatible with google-workspace skill) from datetime import datetime, timezone expiry = datetime.fromtimestamp( @@ -2262,7 +2271,12 @@ def _submit_google_workspace_pkce(session_id: str, code_input: str) -> Dict[str, token_path = credentials_path() token_path.parent.mkdir(parents=True, exist_ok=True) - token_path.write_text(json.dumps(token_payload, indent=2), encoding="utf-8") + import os as _os + fd = _os.open(str(token_path), _os.O_WRONLY | _os.O_CREAT | _os.O_TRUNC, 0o600) + try: + _os.write(fd, json.dumps(token_payload, indent=2).encode("utf-8")) + finally: + _os.close(fd) with _oauth_sessions_lock: sess["status"] = "approved" @@ -2369,7 +2383,7 @@ body{{background:#041c1c;color:#ffe6cb;font-family:system-ui,-apple-system,"Sego h1{{font-size:1.5rem;font-weight:600;letter-spacing:0.05em;text-transform:uppercase;margin-bottom:0.75rem}} p{{color:rgba(255,230,203,0.7);font-size:0.95rem;line-height:1.6}} -
✗

Authorization Failed

{error}

+
✗

Authorization Failed

{html.escape(error)}

""", status_code=400, ) @@ -2396,7 +2410,7 @@ p{color:rgba(255,230,203,0.7);font-size:0.95rem;line-height:1.6} result = _submit_google_workspace_pkce(state, code) except HTTPException as exc: return HTMLResponse( - f"

❌ {exc.detail}

", + f"

❌ {html.escape(str(exc.detail))}

", status_code=exc.status_code, ) @@ -2488,7 +2502,7 @@ p{color:rgba(255,230,203,0.7);font-size:0.95rem;line-height:1.6}
✗

Connection Failed

-

{msg}

+

{html.escape(str(msg))}

""",