Merge pull request #338 from Molecule-AI/fix/issue-328-transcript-fail-closed

fix(security): /transcript fails closed when auth token missing (#328)

workspace-template/main.py

@@ -45,6 +45,11 @@ def get_machine_ip() -> str:  # pragma: no cover
         return "127.0.0.1"
 
 
+# Re-exported from transcript_auth for the inline /transcript handler.
+# Separate module keeps the security-critical gate import-light + unit-testable.
+from transcript_auth import transcript_authorized as _transcript_authorized
+
+
 async def main():  # pragma: no cover
     workspace_id = os.environ.get("WORKSPACE_ID", "workspace-default")
     config_path = os.environ.get("WORKSPACE_CONFIG_PATH", "/configs")
@@ -293,12 +298,19 @@ async def main():  # pragma: no cover
         # Require workspace bearer token — the same token issued at registration
         # and stored in /configs/.auth_token. Any container on molecule-monorepo-net
         # could otherwise read the full session log. Closes #287.
+        #
+        # #328: fail CLOSED when the token file is unavailable. get_token()
+        # returns None during the bootstrap window (first register hasn't
+        # completed), if /configs/.auth_token was deleted, or on OSError.
+        # The old `if expected:` guard treated all three cases as "skip
+        # auth" — an unauthenticated container on the same Docker network
+        # could read the entire session log during that window. Deny
+        # instead. The platform's TranscriptHandler acquires the token
+        # during registration, so once the bootstrap completes it always
+        # has a valid credential to present.
         from platform_auth import get_token
-        expected = get_token()
-        if expected:
-            auth_header = request.headers.get("Authorization", "")
-            if auth_header != f"Bearer {expected}":
-                return JSONResponse({"error": "unauthorized"}, status_code=401)
+        if not _transcript_authorized(get_token(), request.headers.get("Authorization", "")):
+            return JSONResponse({"error": "unauthorized"}, status_code=401)
         try:
             since = int(request.query_params.get("since", "0"))
             limit = int(request.query_params.get("limit", "100"))

workspace-template/tests/test_transcript_auth.py

@@ -0,0 +1,47 @@
+"""Tests for the #328 fix — /transcript endpoint must fail-CLOSED when
+the workspace auth token is not yet on disk.
+
+Prior behaviour (regressed in #287): `if expected:` skipped the auth
+check when `get_token()` returned None, so any container on
+`molecule-monorepo-net` could read the full session log during the
+bootstrap window. The fix lifts the guard into transcript_auth.py for
+testability.
+"""
+
+from transcript_auth import transcript_authorized
+
+
+def test_missing_token_fails_closed():
+    # #328 regression: None token MUST return False (was the fail-open bug).
+    assert transcript_authorized(None, "Bearer anything") is False
+
+
+def test_empty_token_fails_closed():
+    # Empty string is as-bad-as None — also a fail-closed case.
+    assert transcript_authorized("", "Bearer anything") is False
+
+
+def test_valid_bearer_passes():
+    assert transcript_authorized("tok-123", "Bearer tok-123") is True
+
+
+def test_wrong_bearer_fails():
+    assert transcript_authorized("tok-123", "Bearer other-token") is False
+
+
+def test_missing_header_fails_even_when_expected_is_set():
+    # Empty auth header (not sent at all) must fail — client forgot.
+    assert transcript_authorized("tok-123", "") is False
+
+
+def test_case_sensitive_bearer_prefix():
+    # Strict equality matches platform wsauth.BearerTokenFromHeader
+    # which is also case-sensitive on the "Bearer " prefix. Documenting
+    # the behavior so a future refactor is a conscious choice.
+    assert transcript_authorized("tok-123", "bearer tok-123") is False
+
+
+def test_extra_whitespace_in_header_fails():
+    # Strict equality — accidental double space between Bearer and token
+    # must fail so an adversary can't test fuzzed variations.
+    assert transcript_authorized("tok-123", "Bearer  tok-123") is False

workspace-template/transcript_auth.py

@@ -0,0 +1,30 @@
+"""Auth gate for the /transcript Starlette route.
+
+Extracted from main.py so the security-critical logic is unit-testable
+without standing up the full uvicorn/a2a/httpx import stack.
+
+#328: the route must fail CLOSED when the expected token is unavailable
+(bootstrap window, missing file, OSError). The previous implementation
+treated a missing token as "skip auth entirely" — any container on the
+same Docker network could read the session log during provisioning.
+"""
+
+
+def transcript_authorized(expected_token: str | None, auth_header: str) -> bool:
+    """Return True iff /transcript should serve the request.
+
+    Args:
+        expected_token: the workspace's registered bearer token, or None
+            if `/configs/.auth_token` is absent / unreadable.
+        auth_header: raw value of the Authorization request header.
+
+    Behavior:
+        - None/empty expected → fail closed (401). This is the #328 fix;
+          a missing token file is an auth failure, not a bypass.
+        - Non-empty expected: strict equality check against "Bearer <tok>".
+          Bearer prefix is case-sensitive (matches the platform's
+          wsauth.BearerTokenFromHeader contract).
+    """
+    if not expected_token:
+        return False
+    return auth_header == f"Bearer {expected_token}"