Skip to content

fix(backend): API keys access user integrations via SA annotation #901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Gkrumbach07 merged 3 commits into from
Mar 13, 2026
Merged

fix(backend): API keys access user integrations via SA annotation #901

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b862874
fix(backend): resolve userID from SA annotation for API key auth
Mar 12, 2026
33c6e28
refactor(backend): deduplicate ExtractServiceAccountFromAuth into ser…
Gkrumbach07 Mar 12, 2026
1237c27
fix(backend): use TokenReview for SA identity, reject empty userID on…
Gkrumbach07 Mar 13, 2026
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
9 changes: 5 additions & 4 deletions components/backend/handlers/permissions.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,11 @@ func CreateProjectKey(c *gin.Context) {
Namespace: projectName,
Labels: map[string]string{"app": "ambient-access-key"},
Annotations: map[string]string{
"ambient-code.io/key-name": req.Name,
"ambient-code.io/description": req.Description,
"ambient-code.io/created-at": time.Now().Format(time.RFC3339),
"ambient-code.io/role": role,
"ambient-code.io/key-name": req.Name,
"ambient-code.io/description": req.Description,
"ambient-code.io/created-at": time.Now().Format(time.RFC3339),
"ambient-code.io/role": role,
"ambient-code.io/created-by-user-id": c.GetString("userID"),
},
},
}
Expand Down
73 changes: 73 additions & 0 deletions components/backend/server/server.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@
package server

import (
"encoding/base64"
"encoding/json"
"fmt"
"log"
"os"
"strings"

"github.com/gin-contrib/cors"
"github.com/gin-gonic/gin"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// RouterFunc is a function that can register routes on a Gin router
Expand Down Expand Up @@ -158,6 +161,76 @@ func forwardedIdentityMiddleware() gin.HandlerFunc {
if v := c.GetHeader("X-Forwarded-Access-Token"); v != "" {
c.Set("forwardedAccessToken", v)
}

// Fallback: if userID is still empty, try to resolve it from the
// ServiceAccount's created-by-user-id annotation. This enables API
// key-authenticated requests to inherit the creating user's identity
// so that integration credentials (GitHub, Jira, GitLab) are accessible.
if c.GetString("userID") == "" {
if ns, saName, ok := extractServiceAccountFromBearer(c); ok && K8sClient != nil {
sa, err := K8sClient.CoreV1().ServiceAccounts(ns).Get(c.Request.Context(), saName, v1.GetOptions{})
if err == nil && sa.Annotations != nil {
if uid := sa.Annotations["ambient-code.io/created-by-user-id"]; uid != "" {
c.Set("userID", uid)
}
}
Comment on lines +169 to +176
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Do not treat a mutable ServiceAccount annotation as authoritative user identity.

Once the token is verified, this block copies ambient-code.io/created-by-user-id straight into userID without checking that the ServiceAccount is platform-managed. These ServiceAccounts are created in-project with the caller-scoped Kubernetes client in components/backend/handlers/permissions.go, so this annotation is not a strong trust boundary. Any annotated ServiceAccount token can become a user-integration identity; at minimum gate this on a platform-managed marker, and preferably move the mapping to server-owned or tamper-evident state.

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

In `@components/backend/server/server.go` around lines 169 - 176, The code
currently trusts the ServiceAccount annotation
"ambient-code.io/created-by-user-id" and sets c.Set("userID", ...) directly (in
the block using K8sClient and resolveServiceAccountFromToken), which allows
caller-controlled SA annotations to impersonate users; change this by requiring
a platform-managed marker on the ServiceAccount before accepting its created-by
annotation (e.g. check for a trusted annotation/label such as an
"ambient-code.io/platform-managed" marker or other server-controlled flag on
sa.Annotations/sa.Labels), and if that marker is absent do not set
c.Set("userID") from the SA annotation — instead record the mapping in
server-owned/tamper-evident state (or reject/ignore the annotation) and only
populate c.Set("userID") after verifying the SA is platform-managed or after
consulting the server-owned mapping.

}
}

c.Next()
}
}

// extractServiceAccountFromBearer extracts namespace and SA name from a Bearer
// JWT's "sub" claim (format: "system:serviceaccount:<ns>:<name>").
// Also checks the X-Remote-User header for the same format.
func extractServiceAccountFromBearer(c *gin.Context) (string, string, bool) {
// Check X-Remote-User header (OpenShift OAuth proxy format)
if remoteUser := c.GetHeader("X-Remote-User"); remoteUser != "" {
const prefix = "system:serviceaccount:"
if strings.HasPrefix(remoteUser, prefix) {
parts := strings.SplitN(strings.TrimPrefix(remoteUser, prefix), ":", 2)
if len(parts) == 2 {
return parts[0], parts[1], true
}
}
}

// Standard Authorization Bearer JWT parsing
rawAuth := c.GetHeader("Authorization")
parts := strings.SplitN(rawAuth, " ", 2)
if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") {
return "", "", false
}
token := strings.TrimSpace(parts[1])
Comment on lines +188 to +193
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Query-string API-key auth still misses this fallback.

This resolver only reads Authorization, but ?token= is promoted into that header later in components/backend/handlers/middleware.go inside ValidateProjectContext. Because forwardedIdentityMiddleware() runs first, websocket/agent callers using query-token auth will still reach this path with empty userID, so the new integration fallback never runs for them.

Possible fix 
 func Run(registerRoutes RouterFunc) error {
 	// Setup Gin router with custom logger that redacts tokens
 	r := gin.New()
 	r.Use(gin.Recovery())
+	r.Use(func(c *gin.Context) {
+		if c.GetHeader("Authorization") == "" && c.GetHeader("X-Forwarded-Access-Token") == "" {
+			if qp := strings.TrimSpace(c.Query("token")); qp != "" {
+				c.Request.Header.Set("Authorization", "Bearer "+qp)
+			}
+		}
+		c.Next()
+	})
 	r.Use(gin.LoggerWithFormatter(func(param gin.LogFormatterParams) string {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/backend/server/server.go` around lines 188 - 193, The
Authorization header parsing in server.go only checks
c.GetHeader("Authorization") (rawAuth) and thus misses query-string ?token=
values used by websocket/agent callers; update the logic in the same function
that computes token (using rawAuth, parts, token) to fallback to the query param
(e.g., c.Query("token") or r.URL.Query().Get("token")) when rawAuth is empty or
parts length != 2, so token gets populated from the ?token= value (this keeps
behavior consistent with ValidateProjectContext and
forwardedIdentityMiddleware); ensure you still trim/validate the token string
before returning.

if token == "" {
return "", "", false
}
segs := strings.Split(token, ".")
if len(segs) < 2 {
return "", "", false
}
payloadB64 := segs[1]
if m := len(payloadB64) % 4; m != 0 {
payloadB64 += strings.Repeat("=", 4-m)
}
data, err := base64.URLEncoding.DecodeString(payloadB64)
if err != nil {
return "", "", false
}
var payload map[string]interface{}
if err := json.Unmarshal(data, &payload); err != nil {
return "", "", false
}
sub, _ := payload["sub"].(string)
const prefix = "system:serviceaccount:"
if !strings.HasPrefix(sub, prefix) {
return "", "", false
}
rest := strings.TrimPrefix(sub, prefix)
parts2 := strings.SplitN(rest, ":", 2)
if len(parts2) != 2 {
return "", "", false
}
return parts2[0], parts2[1], true
}
Loading