From 85be6cf6742d1d4ac720ce75d40f0e39157c60b2 Mon Sep 17 00:00:00 2001 From: mukktinaadh <159904553+mukktinaadh@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:36:45 +0530 Subject: [PATCH] fix(auth): remove hardcoded fallback session cookie signing secret `AUTH_COOKIE_SECRET` fell back to the committed literal `"your_cookie_signing_secret"` whenever the environment variable was unset. That value signs the session cookie JWT in `_encode_session_cookie` and verifies it in `_decode_session_cookie`, so a deployment running without the variable accepts session cookies forged with a key that is public in the source tree. Verified against this commit: before the change, a cookie signed with the committed literal decoded to a valid `Session`; afterwards the same cookie raises `AuthException`. The literal now appears only in the regression test, to prove it can no longer verify anything. - Read `AUTH_COOKIE_SECRET` with no default. - When unset, generate an ephemeral per-process secret via `secrets.token_hex(32)` and warn. Local development keeps working without a committed signing key; sessions signed with the ephemeral value are dropped on restart and are not shared across replicas, which the warning states explicitly. - Drop the now-dead "unsafe AUTH_COOKIE_SECRET" check. Fixes #1446 --- app/api/agentops/auth/environment.py | 25 +++++-- app/api/tests/auth/test_session_cookie.py | 84 +++++++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 app/api/tests/auth/test_session_cookie.py diff --git a/app/api/agentops/auth/environment.py b/app/api/agentops/auth/environment.py index b7b2fd604..f48b349ac 100644 --- a/app/api/agentops/auth/environment.py +++ b/app/api/agentops/auth/environment.py @@ -1,10 +1,28 @@ import os +import secrets from agentops.api.log_config import logger # generate an AUTH_COOKIE_SECRET with: # import secrets; print(secrets.token_hex(32)) -_DEV_AUTH_COOKIE_SECRET = "your_cookie_signing_secret" -AUTH_COOKIE_SECRET = os.getenv("AUTH_COOKIE_SECRET", _DEV_AUTH_COOKIE_SECRET) +# +# This value signs the session cookie JWT, so it must be unique and high-entropy +# per deployment. Never fall back to a committed literal: a signing key that is +# present in the source tree lets anyone forge a session cookie that passes +# signature verification, and the fallback would silently be used whenever the +# variable is unset. +AUTH_COOKIE_SECRET = os.getenv("AUTH_COOKIE_SECRET") + +if not AUTH_COOKIE_SECRET: + # Fall back to an ephemeral, per-process secret so local development works + # without a committed signing key. Cookies signed with this value are + # invalidated on restart and are not valid across replicas, so it is only + # suitable for development. + AUTH_COOKIE_SECRET = secrets.token_hex(32) + logger.warning( + "[agentops.auth.environment] AUTH_COOKIE_SECRET is not set; generated a " + "random per-process secret. Sessions will not survive a restart and " + "will not be shared across replicas. Set AUTH_COOKIE_SECRET in production." + ) AUTH_COOKIE_NAME = os.getenv("AUTH_COOKIE_NAME", "session_id") AUTH_JWT_ALGO = "HS256" # this is for our internal JWT on the session cookie @@ -24,9 +42,6 @@ SUPABASE_JWT_SECRET: str = os.getenv("JWT_SECRET_KEY") -if AUTH_COOKIE_SECRET == _DEV_AUTH_COOKIE_SECRET: - logger.warning("[agentops.auth.environment] Using an unsafe AUTH_COOKIE_SECRET") - if not SUPABASE_JWT_SECRET: logger.warning("[agentops.auth.environment] No JWT_SECRET_KEY set") diff --git a/app/api/tests/auth/test_session_cookie.py b/app/api/tests/auth/test_session_cookie.py new file mode 100644 index 000000000..24545cb37 --- /dev/null +++ b/app/api/tests/auth/test_session_cookie.py @@ -0,0 +1,84 @@ +""" +Tests for the session cookie JWT helpers. + +The session cookie is signed with `AUTH_COOKIE_SECRET`. These tests pin two +properties: + +1. A cookie signed with the literal that used to be committed as the fallback + secret must NOT verify, so the known-key forgery path stays closed. +2. Encoding and decoding a session cookie round-trips the session ID. +""" + +from unittest.mock import patch +from uuid import uuid4 + +import jwt +import pytest + +from agentops.auth import views as auth_views +from agentops.auth.environment import AUTH_COOKIE_SECRET, AUTH_JWT_ALGO +from agentops.auth.exceptions import AuthException +from agentops.auth.session import Session + +# The literal that used to be the committed fallback in `auth/environment.py`. +# It is now only referenced here, to prove it can no longer verify a cookie. +LEGACY_DEV_SECRET = "your_cookie_signing_secret" + + +def _forge_cookie(session_id: str, secret: str) -> str: + """Mint a session cookie signed with an arbitrary secret.""" + return jwt.encode({"session_id": session_id}, secret, algorithm=AUTH_JWT_ALGO) + + +def test_committed_fallback_secret_is_not_in_use(): + """The signing secret must never be the value that was committed to the repo.""" + assert AUTH_COOKIE_SECRET != LEGACY_DEV_SECRET + assert AUTH_COOKIE_SECRET + + +def test_cookie_signed_with_legacy_secret_is_rejected(): + """A forged cookie signed with the old committed key must not decode.""" + forged = _forge_cookie(str(uuid4()), LEGACY_DEV_SECRET) + + with pytest.raises(AuthException, match="Could not decode internal session JWT."): + auth_views._decode_session_cookie(forged) + + +def test_cookie_signed_with_legacy_secret_is_rejected_even_for_known_session(): + """Signature verification happens before the session lookup, so a forged + cookie cannot be redeemed even when it names a session that exists.""" + session = Session(session_id=uuid4(), user_id=uuid4()) + forged = _forge_cookie(str(session.session_id), LEGACY_DEV_SECRET) + + with patch.object(auth_views.Session, "get", return_value=session): + with pytest.raises(AuthException, match="Could not decode internal session JWT."): + auth_views._decode_session_cookie(forged) + + +def test_session_cookie_round_trip(): + """A cookie signed with the configured secret decodes back to its session.""" + session = Session(session_id=uuid4(), user_id=uuid4()) + cookie = auth_views._encode_session_cookie(session) + + with patch.object(auth_views.Session, "get", return_value=session) as get_session: + assert auth_views._decode_session_cookie(cookie) == session + + get_session.assert_called_once_with(str(session.session_id)) + + +def test_decoding_unknown_session_returns_none(): + """A well-signed cookie for a session that no longer exists returns None.""" + session = Session(session_id=uuid4(), user_id=uuid4()) + cookie = auth_views._encode_session_cookie(session) + + with patch.object(auth_views.Session, "get", return_value=None): + assert auth_views._decode_session_cookie(cookie) is None + + +def test_cookie_payload_only_contains_session_id(): + """The cookie carries the session ID and nothing else.""" + session = Session(session_id=uuid4(), user_id=uuid4()) + cookie = auth_views._encode_session_cookie(session) + + decoded = jwt.decode(cookie, AUTH_COOKIE_SECRET, algorithms=[AUTH_JWT_ALGO]) + assert decoded == {"session_id": str(session.session_id)}