From a1f4161c1d6e00bd60d7bc18efa6fcb86a6dac4c Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Tue, 2 Jun 2026 10:21:04 +0000
Subject: [PATCH] fix(auth): fail closed on missing project_id claim in JWTs

Co-authored-by: mudcube <101564+mudcube@users.noreply.github.com>
---
 .jules/sentinel.md          |  5 +++++
 crates/mill-auth/src/jwt.rs | 16 ++++++----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 798ec7de6..f5d8a5699 100644
--- a/.jules/sentinel.md
+++ b/.jules/sentinel.md
@@ -2,3 +2,8 @@
 **Vulnerability:** The `OperationQueue` worker in `mill-server` executed file operations (create, write, delete, rename) using raw paths from the operation object without validating they were within the project root.
 **Learning:** Background workers that process serialized operations are a common bypass for security checks enforced at the API layer. The API layer might validate the request, but if the worker is "dumb" and blindly executes the queued operation, an internal attacker or a buggy component can exploit it.
 **Prevention:** Validation must happen at the *execution point* (in the worker), not just at the ingestion point. We introduced `validate_path` in the worker loop to enforce project root containment using `canonicalize` (handling non-existent files correctly).
+
+## 2025-05-24 - JWT Authorization Bypass via Fail-Open Fallback
+**Vulnerability:** The `validate_token_with_project` function in `mill-auth` failed open when the `project_id` claim was missing from the JWT token, returning success to maintain backward compatibility.
+**Learning:** Security controls, especially authorization checks in multi-tenant environments, must strictly fail closed. Permissive fallbacks intended for backward compatibility create easy bypasses, allowing tokens without project scoping to access project-specific resources.
+**Prevention:** Always enforce strict validation requirements and return permission denied errors if expected authorization claims are absent, rather than logging a warning and allowing access.
diff --git a/crates/mill-auth/src/jwt.rs b/crates/mill-auth/src/jwt.rs
index 8c96b0101..0110b929e 100644
--- a/crates/mill-auth/src/jwt.rs
+++ b/crates/mill-auth/src/jwt.rs
@@ -85,11 +85,10 @@ pub fn validate_token_with_project(
             )))
         }
     } else {
-        // No project_id claim, allow access (for backward compatibility)
-        tracing::warn!(
-            "JWT token missing project_id claim - this will be required in future versions"
-        );
-        Ok(true)
+        // SECURITY: Fail closed when project_id is missing to enforce strict multi-tenant authorization. Backward compatibility exceptions in authentication can lead to authorization bypasses.
+        Err(MillError::permission_denied(
+            "JWT token missing project_id claim".to_string(),
+        ))
     }
 }
 
@@ -208,10 +207,7 @@ mod tests {
         let auth_config = create_test_auth_config(false);
         let token = create_test_token(secret, None);
 
-        // Should succeed when no project_id claim is present
-        assert!(
-            validate_token_with_project(&token, secret, "any_project", &auth_config)
-                .expect("Test token should be valid")
-        );
+        // Should fail when no project_id claim is present
+        assert!(validate_token_with_project(&token, secret, "any_project", &auth_config).is_err());
     }
 }