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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,4 @@ hack/

# Personal exports
*.csv
.worktrees/
11 changes: 11 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ repos:
files: ^components/frontend/.*\.(ts|tsx|js|jsx)$
pass_filenames: true

# ── CodeRabbit AI review ───────────────────────────────────────────
- repo: local
hooks:
- id: coderabbit-review
name: coderabbit review
entry: scripts/pre-commit/coderabbit-review.sh
language: script
always_run: true
pass_filenames: false
require_serial: true

# ── Branch protection ────────────────────────────────────────────────
- repo: local
hooks:
Expand Down
260 changes: 260 additions & 0 deletions components/backend/handlers/coderabbit_auth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
package handlers

import (
"context"
"encoding/json"
"fmt"
"log"
"net/http"
"time"

"github.com/gin-gonic/gin"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// CodeRabbitCredentials represents cluster-level CodeRabbit credentials for a user
type CodeRabbitCredentials struct {
UserID string `json:"userId"`
APIKey string `json:"apiKey"`
UpdatedAt time.Time `json:"updatedAt"`
}

// ConnectCodeRabbit handles POST /api/auth/coderabbit/connect
// Saves user's CodeRabbit credentials at cluster level
func ConnectCodeRabbit(c *gin.Context) {
// Verify user has valid K8s token (follows RBAC pattern)
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

// Verify user is authenticated and userID is valid
userID := c.GetString("userID")
if userID == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User authentication required"})
return
}
if !isValidUserID(userID) {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid user identifier"})
return
}

var req struct {
APIKey string `json:"apiKey" binding:"required"`
}

if err := c.ShouldBindJSON(&req); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}

// Validate API key with CodeRabbit
if err := ValidateCodeRabbitAPIKey(c.Request.Context(), req.APIKey); err != nil {
log.Printf("Failed to validate CodeRabbit API key for user %s: %v", userID, err)
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid CodeRabbit API key"})
return
Comment on lines +54 to +58
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t report provider outages as “invalid API key”

The validator in components/backend/handlers/integration_validation.go can fail for network/provider reasons, but Line 57 maps every error to 400 Invalid CodeRabbit API key. A transient timeout or upstream failure will reject a valid key and tell the user to rotate it. Reserve 400 for actual auth failures and return a retryable 5xx for upstream errors.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/coderabbit_auth.go` around lines 54 - 58, The
handler currently maps any error from ValidateCodeRabbitAPIKey to a 400; change
it to distinguish authentication failures from transient/provider errors by
making ValidateCodeRabbitAPIKey return a sentinel error (e.g.,
ErrInvalidCodeRabbitAPIKey) or wrap errors so you can use errors.Is(err,
ErrInvalidCodeRabbitAPIKey) in the coderabbit_auth.go handler; if errors.Is(...)
respond with 400 and the existing message, otherwise respond with a 5xx (e.g.,
502/503) indicating CodeRabbit service/unavailable and include the full error in
the server log (log.Printf) for debugging. Ensure you update
ValidateCodeRabbitAPIKey signature/implementation and the handler check to use
the sentinel/unwrap logic (ValidateCodeRabbitAPIKey, ErrInvalidCodeRabbitAPIKey)
and log the underlying error details when returning a 5xx.

}

// Store credentials
creds := &CodeRabbitCredentials{
UserID: userID,
APIKey: req.APIKey,
UpdatedAt: time.Now(),
}

if err := storeCodeRabbitCredentials(c.Request.Context(), creds); err != nil {
log.Printf("Failed to store CodeRabbit credentials for user %s: %v", userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to save CodeRabbit credentials"})
return
}

log.Printf("✓ Stored CodeRabbit credentials for user %s", userID)
c.JSON(http.StatusOK, gin.H{
"message": "CodeRabbit connected successfully",
})
}

// GetCodeRabbitStatus handles GET /api/auth/coderabbit/status
// Returns connection status for the authenticated user
func GetCodeRabbitStatus(c *gin.Context) {
// Verify user has valid K8s token
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

userID := c.GetString("userID")
if userID == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User authentication required"})
return
}

creds, err := GetCodeRabbitCredentials(c.Request.Context(), userID)
if err != nil {
if errors.IsNotFound(err) {
c.JSON(http.StatusOK, gin.H{"connected": false})
return
}
log.Printf("Failed to get CodeRabbit credentials for user %s: %v", userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to check CodeRabbit status"})
return
}

if creds == nil {
c.JSON(http.StatusOK, gin.H{"connected": false})
return
}

c.JSON(http.StatusOK, gin.H{
"connected": true,
"updatedAt": creds.UpdatedAt.Format(time.RFC3339),
})
}

// DisconnectCodeRabbit handles DELETE /api/auth/coderabbit/disconnect
// Removes user's CodeRabbit credentials
func DisconnectCodeRabbit(c *gin.Context) {
// Verify user has valid K8s token
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

userID := c.GetString("userID")
if userID == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User authentication required"})
return
}

if err := DeleteCodeRabbitCredentials(c.Request.Context(), userID); err != nil {
log.Printf("Failed to delete CodeRabbit credentials for user %s: %v", userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to disconnect CodeRabbit"})
return
}

log.Printf("✓ Deleted CodeRabbit credentials for user %s", userID)
c.JSON(http.StatusOK, gin.H{"message": "CodeRabbit disconnected successfully"})
}

// storeCodeRabbitCredentials stores CodeRabbit credentials in cluster-level Secret
func storeCodeRabbitCredentials(ctx context.Context, creds *CodeRabbitCredentials) error {
if creds == nil || creds.UserID == "" {
return fmt.Errorf("invalid credentials payload")
}

const secretName = "coderabbit-credentials"

for i := 0; i < 3; i++ { // retry on conflict
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, secretName, v1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
// Create Secret
secret = &corev1.Secret{
ObjectMeta: v1.ObjectMeta{
Name: secretName,
Namespace: Namespace,
Labels: map[string]string{
"app": "ambient-code",
"ambient-code.io/provider": "coderabbit",
},
},
Type: corev1.SecretTypeOpaque,
Data: map[string][]byte{},
}
if _, cerr := K8sClient.CoreV1().Secrets(Namespace).Create(ctx, secret, v1.CreateOptions{}); cerr != nil && !errors.IsAlreadyExists(cerr) {
return fmt.Errorf("failed to create Secret: %w", cerr)
}
// Fetch again to get resourceVersion
secret, err = K8sClient.CoreV1().Secrets(Namespace).Get(ctx, secretName, v1.GetOptions{})
Comment on lines +153 to +173
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Secret reads/writes are still using the backend service account

The handlers fetch request-scoped clients with GetK8sClientsForRequest(c), but the actual Secret operations on Lines 153, 169, 173, 192, 211, 237, and 251 all go through the package-global K8sClient. That bypasses per-request RBAC for connect/status/disconnect. Pass reqK8s into these helpers and use it for the Secret operations.
As per coding guidelines, User-facing API ops MUST use GetK8sClientsForRequest(c), never the backend service account.

Also applies to: 192-196, 211-213, 237-255

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/coderabbit_auth.go` around lines 153 - 173, The
Secret create/read/update calls are using the package-global K8sClient (e.g.,
K8sClient.CoreV1().Secrets(...)) which bypasses per-request RBAC; update the
handlers in coderabbit_auth.go to use the request-scoped client returned by
GetK8sClientsForRequest(c) (e.g., reqK8s) for all Secret operations (Get,
Create, Update) around secretName and related helpers, and change any helper
signatures that currently rely on the global K8sClient to accept and use reqK8s
instead so all connect/status/disconnect secret reads/writes go through the
per-request client.

if err != nil {
return fmt.Errorf("failed to fetch Secret after create: %w", err)
}
} else {
return fmt.Errorf("failed to get Secret: %w", err)
}
}

if secret.Data == nil {
secret.Data = map[string][]byte{}
}

b, err := json.Marshal(creds)
if err != nil {
return fmt.Errorf("failed to marshal credentials: %w", err)
}
secret.Data[creds.UserID] = b

if _, uerr := K8sClient.CoreV1().Secrets(Namespace).Update(ctx, secret, v1.UpdateOptions{}); uerr != nil {
if errors.IsConflict(uerr) {
continue // retry
}
return fmt.Errorf("failed to update Secret: %w", uerr)
}
return nil
}
return fmt.Errorf("failed to update Secret after retries")
}

// GetCodeRabbitCredentials retrieves cluster-level CodeRabbit credentials for a user
func GetCodeRabbitCredentials(ctx context.Context, userID string) (*CodeRabbitCredentials, error) {
if userID == "" {
return nil, fmt.Errorf("userID is required")
}

const secretName = "coderabbit-credentials"

secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, secretName, v1.GetOptions{})
if err != nil {
return nil, err
}

if secret.Data == nil || len(secret.Data[userID]) == 0 {
return nil, nil // User hasn't connected CodeRabbit
}

var creds CodeRabbitCredentials
if err := json.Unmarshal(secret.Data[userID], &creds); err != nil {
return nil, fmt.Errorf("failed to parse credentials: %w", err)
}

return &creds, nil
}

// DeleteCodeRabbitCredentials removes CodeRabbit credentials for a user
func DeleteCodeRabbitCredentials(ctx context.Context, userID string) error {
if userID == "" {
return fmt.Errorf("userID is required")
}

const secretName = "coderabbit-credentials"

for i := 0; i < 3; i++ { // retry on conflict
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, secretName, v1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
return nil // Secret doesn't exist, nothing to delete
}
return fmt.Errorf("failed to get Secret: %w", err)
}

if secret.Data == nil || len(secret.Data[userID]) == 0 {
return nil // User's credentials don't exist
}

delete(secret.Data, userID)

if _, uerr := K8sClient.CoreV1().Secrets(Namespace).Update(ctx, secret, v1.UpdateOptions{}); uerr != nil {
if errors.IsConflict(uerr) {
continue // retry
}
return fmt.Errorf("failed to update Secret: %w", uerr)
}
return nil
}
return fmt.Errorf("failed to update Secret after retries")
}
Loading
Loading