Skip to content

Draft: Fix runAsUser error, migration ordering, and details during session errors #1592

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

Draft
bsquizz wants to merge 2 commits into
base: main
Choose a base branch
from
+22 −4
Draft

Draft: Fix runAsUser error, migration ordering, and details during session errors #1592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5d57c9
fix(ambient-api-server): runAsNonRoot compliance and migration ID ord…
bsquizz May 15, 2026
aad7bab
fix(backend): propagate auth errors from K8s list in ListSessions
bsquizz May 15, 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
7 changes: 6 additions & 1 deletion components/ambient-api-server/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,16 @@ ARG GIT_COMMIT=unknown
RUN \
microdnf install -y \
util-linux \
shadow-utils \
&& \
microdnf clean all
microdnf clean all \
&& \
useradd -u 1000 -M -s /sbin/nologin -r appuser

COPY --from=builder /workspace/ambient-api-server /usr/local/bin/

USER 1000

EXPOSE 8000

ENTRYPOINT ["/usr/local/bin/ambient-api-server", "serve"]
Expand Down
2 changes: 1 addition & 1 deletion components/ambient-api-server/plugins/roleBindings/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func migration() *gormigrate.Migration {

func typedFKMigration() *gormigrate.Migration {
return &gormigrate.Migration{
ID: "202505130001",
ID: "202605150001",
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the old migration ID exists in version control history or deployed environments

# Search for the old migration ID in git history
echo "=== Checking git history for old migration ID ==="
git log --all --oneline --grep="202505130001" || echo "No commits mention old ID"

# Search for migration tracking in the codebase
echo -e "\n=== Checking for migration version tracking ==="
rg -n "202505130001" --type=go

# Look for any migration state or version files
echo -e "\n=== Looking for migration state files ==="
fd -e json -e yaml -e yml migration | head -20

Repository: ambient-code/platform

Length of output: 299

🏁 Script executed:

cat -n components/ambient-api-server/plugins/roleBindings/migration.go

Repository: ambient-code/platform

Length of output: 4195

🏁 Script executed:

git log --oneline --all -- components/ambient-api-server/plugins/roleBindings/migration.go | head -20

Repository: ambient-code/platform

Length of output: 328

🏁 Script executed:

git show 5556220:components/ambient-api-server/plugins/roleBindings/migration.go | head -50

Repository: ambient-code/platform

Length of output: 1630

Fix Rollback function to propagate all errors.

The migration ID change from 202505130001 to 202605150001 is confirmed and will trigger a rerun if already deployed. While the Migrate function is idempotent (all DDL uses IF EXISTS/IF NOT EXISTS guards), the Rollback function silently discards all errors except the last one (lines 78–86). Per guidelines, errors must be propagated or explicitly collected, never discarded. If any operation fails during rollback, the failure is invisible and leaves the schema in an inconsistent state.

Collect all rollback errors instead of silencing them with _ assignment:

Suggested fix 
Rollback: func(tx *gorm.DB) error {
	var errs []error
	if err := tx.Exec(`DROP INDEX IF EXISTS idx_role_bindings_credential_id`).Error; err != nil {
		errs = append(errs, err)
	}
	if err := tx.Exec(`DROP INDEX IF EXISTS idx_role_bindings_session_id`).Error; err != nil {
		errs = append(errs, err)
	}
	// ... continue for all operations
	if len(errs) > 0 {
		return fmt.Errorf("rollback errors: %v", errs)
	}
	return nil
},
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/ambient-api-server/plugins/roleBindings/migration.go` at line 35,
The Rollback function for the migration with ID "202605150001" currently
discards errors from the tx.Exec calls (the `_` assignments in Rollback), so
update Rollback(func(tx *gorm.DB)) to collect each error into a slice (e.g., var
errs []error) by appending err when a tx.Exec(...).Error != nil for every
DROP/DDL operation, and at the end return a combined error (e.g.,
fmt.Errorf("rollback errors: %v", errs)) if len(errs)>0, otherwise return nil;
ensure every tx.Exec in Rollback is changed to check and append its error
instead of ignoring it.

Migrate: func(tx *gorm.DB) error {
// Drop the old unique index that depends on scope_id before altering columns
if err := tx.Exec(`DROP INDEX IF EXISTS idx_binding_lookup`).Error; err != nil {
Expand Down
16 changes: 14 additions & 2 deletions components/backend/handlers/sessions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,13 @@ func ListSessions(c *gin.Context) {
list, err := k8sDyn.Resource(gvr).Namespace(project).List(ctx, listOpts)
if err != nil {
log.Printf("Failed to list agentic sessions in project %s: %v", project, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list agentic sessions"})
if errors.IsUnauthorized(err) {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Token expired or invalid"})
} else if errors.IsForbidden(err) {
c.JSON(http.StatusForbidden, gin.H{"error": "Unauthorized to access project"})
} else {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list agentic sessions"})
}
return
}

Expand All @@ -650,7 +656,13 @@ func ListSessions(c *gin.Context) {
list, err = k8sDyn.Resource(gvr).Namespace(project).List(ctx, listOpts)
if err != nil {
log.Printf("Failed to list agentic sessions (continue) in project %s: %v", project, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list agentic sessions"})
if errors.IsUnauthorized(err) {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Token expired or invalid"})
} else if errors.IsForbidden(err) {
c.JSON(http.StatusForbidden, gin.H{"error": "Unauthorized to access project"})
} else {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list agentic sessions"})
}
return
}
allItems = append(allItems, list.Items...)
Expand Down
1 change: 1 addition & 0 deletions components/manifests/base/core/ambient-api-server-service.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ spec:
serviceAccountName: ambient-api-server
securityContext:
runAsNonRoot: true
runAsUser: 1000
seccompProfile:
type: RuntimeDefault
initContainers:
Expand Down