Add google workspace MCP to runner with necessary oauth additions

[MichaelClifford]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

Claude Code Review

Summary

This PR adds Google Workspace MCP integration with OAuth 2.0 authentication flow. The implementation adds a comprehensive OAuth handler in the backend, frontend UI for connecting Google Drive, and infrastructure changes to support credential storage. While the OAuth flow implementation is solid with HMAC-signed state tokens and CSRF protection, there are several critical security and architectural issues that must be addressed before merge.

Issues by Severity

Blocker Issues

1. Missing User Token Authentication (CRITICAL SECURITY VIOLATION)

• Location: components/backend/handlers/oauth.go:99 (GetOAuthURL)
• Issue: The GetOAuthURL endpoint does NOT use GetK8sClientsForRequest(c) to validate user authentication
• Risk: ANY unauthenticated user can generate OAuth URLs for any session, potentially stealing credentials or performing unauthorized OAuth flows
• Reference: CLAUDE.md lines 344-348, .claude/context/security-standards.md lines 9-19

2. Token Logging (SECURITY LEAK)

• Location: components/backend/handlers/oauth.go:268-271
• Issue: Access tokens and refresh tokens are assigned to callbackData struct that may be logged
• Risk: OAuth tokens logged in structured logs expose user credentials
• Required Fix: Never log the callback data struct that contains tokens. Add explicit token redaction in any log statements. Use log.Printf(tokenLen=%d, len(accessToken)) pattern
• Reference: CLAUDE.md lines 356-360, .claude/context/security-standards.md lines 21-48

3. OAuth State Secret Not Configured in Manifests

• Location: Missing from components/manifests/base/backend-deployment.yaml
• Issue: Code expects OAUTH_STATE_SECRET environment variable (oauth.go:134), but it's not defined in deployment manifests
• Risk: Backend will fail to generate/validate OAuth state tokens at runtime

Critical Issues

4. Using Backend Service Account for OAuth Secret Creation (Authorization Bypass)

• Location: components/backend/handlers/oauth.go:637-726 (storeCredentialsInSecret)
• Issue: Uses DynamicClient (backend service account) to read AgenticSession CR and create OAuth secrets WITHOUT validating user has permission to access that session
• Risk: If combined with the missing auth in GetOAuthURL, any user could potentially trigger OAuth flow for someone elses session
• Reference: CLAUDE.md lines 417-447, .claude/patterns/k8s-client-usage.md lines 99-159

5. Missing Error Context in Logs

• Location: Multiple locations in oauth.go
• Issue: Error logs don't include namespace/session context consistently
• Example: Line 256: Missing project/session info in error logs
• Reference: .claude/patterns/error-handling.md lines 8-66

6. Routes Outside ValidateProjectContext Middleware

• Location: components/backend/routes.go:39
• Issue: GetOAuthURL is registered OUTSIDE the projectGroup that has ValidateProjectContext() middleware
• Risk: No project-level RBAC validation before OAuth flow initiation
• Reference: CLAUDE.md lines 595-606

7. Unused Function (writeCredentialsToSessionPVC)

• Location: components/backend/handlers/oauth.go:592-631
• Issue: Function is defined but never called (dead code)

Major Issues

8. Frontend: No Error Handling for Failed OAuth Connection

• Location: mcp-integrations-accordion.tsx:64
• Issue: Errors are only logged to console, user gets no feedback

9. Frontend: No Verification of OAuth Success

• Location: mcp-integrations-accordion.tsx:59 (TODO comment)
• Issue: Assumes OAuth succeeded just because popup closed

10. Operator: Missing OAuth Secret Volume Mount Logic

• Location: components/operator/internal/handlers/sessions.go
• Issue: Operator creates Jobs for sessions, but doesnt add logic to mount OAuth secrets
• Impact: Session pods wont have access to OAuth credentials even after theyre created

11. Missing BACKEND_URL Configuration

• Location: components/manifests/base/backend-deployment.yaml
• Issue: OAuth callback code uses os.Getenv(BACKEND_URL) (oauth.go:150, 247) but this env var is not defined in manifests

Minor Issues

12. Inconsistent Error Messages
13. Deprecated Function Not Removed (oauth.go:510-530)
14. Frontend: Type for Response Data - implicit any type
15. Hardcoded Expiry Time (5 minutes)
16. Missing Unit Tests for OAuth functionality

Positive Highlights

• Excellent CSRF Protection with HMAC-signed state tokens
• State Token Expiry prevents replay attacks
• OwnerReferences Set Correctly for auto-cleanup
• Proper OAuth Scopes for Google Drive
• Good User Experience with popup window
• Secret Structure matches workspace-mcp expectations
• Error Handling in Token Exchange

Recommendations

Immediate (Before Merge)

1. Fix Blocker 1 - Add user token authentication to GetOAuthURL
2. Fix Blocker 2 - Add token redaction throughout oauth.go
3. Fix Blocker 3 - Add OAUTH_STATE_SECRET to manifests
4. Fix Critical 4 - Validate user session ownership before storing credentials
5. Fix Critical 6 - Move OAuth endpoint inside authenticated route group

Short-term (Next PR)

6. Add comprehensive unit tests for OAuth flow
7. Implement frontend verification of OAuth success
8. Add operator logic to mount OAuth secrets in session pods
9. Add user-facing error notifications
10. Add BACKEND_URL to deployment manifests

Long-term Improvements

11. Rate limiting on OAuth endpoints
12. Audit logging for OAuth credential operations
13. OAuth token refresh logic in MCP server
14. Monitoring/alerting for failed OAuth flows

Security Review Summary

Overall Security Grade: C (Needs Improvement)

Strengths: Strong CSRF protection, proper OwnerReferences, state token expiration

Critical Gaps: Missing user authentication on OAuth URL generation, potential token logging, authorization bypass in credential storage

Recommendation: DO NOT MERGE until blockers 1, 2, and 3 are resolved. The missing authentication on GetOAuthURL is a critical security vulnerability that could allow unauthorized OAuth flows.

---

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

[github-actions]

Claude Code Review

This PR adds Google Workspace MCP integration with OAuth2 flow. Overall architecture is sound, but there are critical security issues that must be addressed before merge.

🚫 Blocker Issues

1. No User Token Authentication in GetOAuthURL (oauth.go:98)

• VIOLATION: Does NOT use GetK8sClientsForRequest(c) for user authentication
• IMPACT: Any unauthenticated user can generate OAuth URLs for any project/session
• REQUIRED: Add user token validation and RBAC check before generating OAuth URL

2. OAuth Secrets in Environment Variables (backend-deployment.yaml:101-110)

• VIOLATION: GOOGLE_OAUTH_CLIENT_SECRET exposed as env var
• SECURITY RISK: Secrets visible in pod spec and kubectl describe
• REQUIRED: Use volume mounts for secrets instead

3. Hardcoded Namespace in storeOAuthCallback (oauth.go:381, 436)

• VIOLATION: Uses package-level Namespace variable
• IMPACT: Cross-tenant leakage potential
• REQUIRED: Pass namespace as parameter

4. Frontend Manual fetch() Instead of React Query (mcp-integrations-accordion.tsx:30-32)

• VIOLATION: Direct fetch() call violates React Query requirement
• REQUIRED BY: CLAUDE.md line 1037
• REQUIRED: Create React Query hook in services/queries/oauth.ts

🔴 Critical Issues

5. Missing Error Context in Logs - Multiple locations lack project/session context
6. No TypeScript Types - Missing OAuth API response types
7. No Loading/Error States - Frontend missing error notifications
8. Token Logging Validation - Need explicit token redaction checks

🟡 Major Issues

9. No Tests - 725 lines of backend code with zero tests
10. Commented Dead Code - 137 lines of commented code (lines 493-630)
11. No Input Validation - projectName/sessionName not validated
12. OAUTH_STATE_SECRET Not Documented - Missing from integration docs

✅ Positive Highlights

• Excellent HMAC state validation with constant-time comparison
• Proper OwnerReference pattern
• Good secret labeling
• Comprehensive OAuth provider abstraction

Recommendations

Priority 1 (REQUIRED FOR MERGE):

1. Add user token authentication to GetOAuthURL
2. Move OAuth secrets to volume mounts
3. Add RBAC validation
4. Fix hardcoded namespace
5. Convert frontend to React Query

Priority 2 (BEFORE PRODUCTION):
6. Add comprehensive tests
7. Add input validation
8. Document OAUTH_STATE_SECRET
9. Remove commented code

Overall: The OAuth implementation shows good security awareness (HMAC, state expiry), but the critical authentication bypass in GetOAuthURL blocks this from merging.

Estimated Effort: ~4-6 hours for Priority 1 fixes + tests.

---

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.