Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
16 changes: 6 additions & 10 deletions crates/mill-auth/src/jwt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
))
Comment on lines +89 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Enforce project claims on the websocket auth path

This fail-closed branch does not protect the production websocket authentication path: rg validate_token_with_project shows no production callers, while crates/mill-transport/src/ws.rs:208-220 still decodes JWTs directly and returns Ok(response) after only warning when project_id is absent; handle_initialize then accepts the client-supplied project at ws.rs:459-460. In deployments using websocket auth, a valid unscoped token can therefore still connect and choose any project despite this helper rejecting the same token in tests.

Useful? React with πŸ‘Β / πŸ‘Ž.

}
}

Expand Down Expand Up @@ -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());
}
}
Loading