Skip to content

Fix workspace name uniqueness to be scoped per user #1007

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

Open
derekhiggins wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −2
Open

Fix workspace name uniqueness to be scoped per user #1007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions components/ambient-api-server/openapi/openapi.projects.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ components:
required:
- name
properties:
owner_user_id:
type: string
description: User ID of the project owner (set automatically by the server)
readOnly: true
name:
type: string
display_name:
Expand Down
4 changes: 4 additions & 0 deletions components/ambient-api-server/plugins/projects/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/ambient-code/platform/components/ambient-api-server/pkg/api/openapi"
"github.com/openshift-online/rh-trex-ai/pkg/api/presenters"
"github.com/openshift-online/rh-trex-ai/pkg/auth"
"github.com/openshift-online/rh-trex-ai/pkg/errors"
"github.com/openshift-online/rh-trex-ai/pkg/handlers"
"github.com/openshift-online/rh-trex-ai/pkg/services"
Expand Down Expand Up @@ -36,6 +37,9 @@ func (h projectHandler) Create(w http.ResponseWriter, r *http.Request) {
Action: func() (interface{}, *errors.ServiceError) {
ctx := r.Context()
projectModel := ConvertProject(project)
if username := auth.GetUsernameFromContext(ctx); username != "" {
projectModel.OwnerUserId = username
}
Comment on lines +40 to +42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing validation when username is unavailable.

If GetUsernameFromContext returns an empty string, the project is created with owner_user_id = "". This defeats the per-user uniqueness goal since all such projects share the empty-owner namespace. Although auth middleware should block unauthenticated requests, defensively returning an error here prevents silent creation of orphaned projects.

Proposed fix 
-			if username := auth.GetUsernameFromContext(ctx); username != "" {
-				projectModel.OwnerUserId = username
+			username := auth.GetUsernameFromContext(ctx)
+			if username == "" {
+				return nil, errors.Unauthorized("unable to determine project owner from request context")
 			}
+			projectModel.OwnerUserId = username
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if username := auth.GetUsernameFromContext(ctx); username != "" {
projectModel.OwnerUserId = username
}
username := auth.GetUsernameFromContext(ctx)
if username == "" {
return nil, errors.Unauthorized("unable to determine project owner from request context")
}
projectModel.OwnerUserId = username
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/ambient-api-server/plugins/projects/handler.go` around lines 40 -
42, The code sets projectModel.OwnerUserId from auth.GetUsernameFromContext(ctx)
without checking for an empty string, allowing creation of projects with
owner_user_id == ""; update the project creation handler (the function that
calls auth.GetUsernameFromContext and assigns projectModel.OwnerUserId) to
validate the returned username and, if it is empty, return an appropriate error
response (e.g., HTTP 401/400) and abort creation instead of assigning an empty
owner; ensure the validation occurs before any DB insert and reference
GetUsernameFromContext and projectModel.OwnerUserId when adding the check.

projectModel, err := h.project.Create(ctx, projectModel)
if err != nil {
return nil, err
Expand Down
39 changes: 39 additions & 0 deletions components/ambient-api-server/plugins/projects/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,42 @@ func migration() *gormigrate.Migration {
},
}
}

func ownerUserIdMigration() *gormigrate.Migration {
migrateStatements := []string{
// Add owner_user_id column with a default empty string for existing rows
`ALTER TABLE projects ADD COLUMN IF NOT EXISTS owner_user_id TEXT NOT NULL DEFAULT ''`,
// Drop the old unique index on name only
`DROP INDEX IF EXISTS idx_projects_name`,
// Create composite unique index on (owner_user_id, name)
`CREATE UNIQUE INDEX IF NOT EXISTS idx_owner_name ON projects(owner_user_id, name)`,
}
rollbackStatements := []string{
// Drop the composite unique index
`DROP INDEX IF EXISTS idx_owner_name`,
// Recreate the old unique index on name only
`CREATE UNIQUE INDEX IF NOT EXISTS idx_projects_name ON projects(name)`,
// Drop the owner_user_id column
`ALTER TABLE projects DROP COLUMN IF EXISTS owner_user_id`,
}
Comment on lines +32 to +48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider data backfill for existing projects.

Existing projects will have owner_user_id = '' after migration. If ownership data can be derived (e.g., from audit logs or created_by fields), consider backfilling during migration or as a follow-up task. Otherwise, these orphaned projects share a single namespace and may conflict with future user-created projects named identically.

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

In `@components/ambient-api-server/plugins/projects/migration.go` around lines 32
- 48, The migration currently adds owner_user_id as NOT NULL with default ''
which leaves orphaned projects and can cause future uniqueness conflicts; in
ownerUserIdMigration, change the flow to add owner_user_id as NULLABLE first,
then run a backfill UPDATE (e.g., set owner_user_id = created_by or derived from
audit/logs) for rows where that data exists, then ALTER TABLE to set NOT NULL
and add DEFAULT if you want enforcement, and adjust the
migrateStatements/rollbackStatements accordingly; if no reliable backfill
exists, keep owner_user_id nullable and update the unique index logic (or use
COALESCE) to avoid grouping all empty values into one namespace.

Comment on lines +41 to +48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Rollback may fail if duplicate names exist.

If users create projects with identical names (under different owner_user_id values) after this migration runs, the rollback will fail at line 45 when attempting to recreate the single-column unique index on name. Document this limitation or add a check/warning in the rollback path.

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

In `@components/ambient-api-server/plugins/projects/migration.go` around lines 41
- 48, The rollback block building rollbackStatements (including `DROP INDEX IF
EXISTS idx_owner_name`, `CREATE UNIQUE INDEX IF NOT EXISTS idx_projects_name ON
projects(name)`, and `ALTER TABLE projects DROP COLUMN IF EXISTS owner_user_id`)
can fail if duplicate project names exist across different `owner_user_id`
values; update the rollback in migration.go to detect duplicates before
attempting to recreate `idx_projects_name` and either (a) abort with a clear
error/warning listing offending `name` values and instructing manual cleanup, or
(b) skip/guard the `CREATE UNIQUE INDEX` step when duplicates are present;
implement this by running a pre-check query (SELECT name, COUNT(*) FROM projects
GROUP BY name HAVING COUNT(*) > 1) and branching the rollback to log/return a
helpful error or omit the index-create statement accordingly.


return &gormigrate.Migration{
ID: "202603240001",
Migrate: func(tx *gorm.DB) error {
for _, stmt := range migrateStatements {
if err := tx.Exec(stmt).Error; err != nil {
return err
}
}
return nil
},
Rollback: func(tx *gorm.DB) error {
for _, stmt := range rollbackStatements {
if err := tx.Exec(stmt).Error; err != nil {
return err
}
}
return nil
},
}
}
5 changes: 3 additions & 2 deletions components/ambient-api-server/plugins/projects/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import (

type Project struct {
api.Meta
Name string `json:"name" gorm:"uniqueIndex;not null"`
OwnerUserId string `json:"owner_user_id" gorm:"uniqueIndex:idx_owner_name;not null"`
Name string `json:"name" gorm:"uniqueIndex:idx_owner_name;not null"`
DisplayName *string `json:"display_name"`
Description *string `json:"description"`
Labels *string `json:"labels"`
Expand All @@ -27,7 +28,7 @@ func (l ProjectList) Index() ProjectIndex {
}

func (d *Project) BeforeCreate(tx *gorm.DB) error {
d.ID = d.Name
d.ID = api.NewID()
return nil
Comment on lines 30 to 32
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for any code that might construct project IDs from names
rg -n -C2 'project.*[Ii][Dd].*[Nn]ame|[Nn]ame.*project.*[Ii][Dd]' --type go

Repository: ambient-code/platform

Length of output: 1790

🏁 Script executed:

#!/bin/bash
# Try to list files in the project to understand structure
git ls-files components/ambient-api-server/plugins/projects/ 2>/dev/null | head -20

Repository: ambient-code/platform

Length of output: 1888

🏁 Script executed:

#!/bin/bash
# Search for previous version of the model.go file or any git history
# First, check what files exist and search for ID-related patterns
find . -name "*.go" -path "*projects*" -type f 2>/dev/null | head -10

Repository: ambient-code/platform

Length of output: 732

🏁 Script executed:

#!/bin/bash
# Search for NewID function definition to understand its nature
rg -n "func NewID|type.*NewID" --type go 2>/dev/null | head -20

Repository: ambient-code/platform

Length of output: 47

🏁 Script executed:

#!/bin/bash
# Search for Project struct definition and BeforeCreate
rg -n "type.*Project|BeforeCreate" --type go -A 3 2>/dev/null | head -40

Repository: ambient-code/platform

Length of output: 3038

🏁 Script executed:

#!/bin/bash
# List all go files in projects directory to see actual structure
ls -la components/ambient-api-server/plugins/projects/*.go 2>/dev/null

Repository: ambient-code/platform

Length of output: 1718

🏁 Script executed:

#!/bin/bash
# Search for ID assignment patterns in projects
rg -n "d\.ID\s*=" --type go 2>/dev/null | grep -i project

Repository: ambient-code/platform

Length of output: 229

🏁 Script executed:

#!/bin/bash
# Read the complete model.go file
cat -n components/ambient-api-server/plugins/projects/model.go

Repository: ambient-code/platform

Length of output: 1471

🏁 Script executed:

#!/bin/bash
# Search for git history of model.go to see what changed
git log --oneline -20 -- components/ambient-api-server/plugins/projects/model.go 2>/dev/null

Repository: ambient-code/platform

Length of output: 124

🏁 Script executed:

#!/bin/bash
# Search for any documentation or tests mentioning Project ID generation
rg -n "NewID|project.*id|ID.*project" --type go -i components/ambient-api-server/plugins/projects/ 2>/dev/null | head -30

Repository: ambient-code/platform

Length of output: 4225

🏁 Script executed:

#!/bin/bash
# Search for any external API contracts or client code that might use Project IDs
rg -n "project.*id|construct.*id|derive.*id" --type go -i 2>/dev/null | grep -v ".pb.go" | head -20

Repository: ambient-code/platform

Length of output: 2339

🏁 Script executed:

#!/bin/bash
# Get the actual diff for the commit that touched model.go
git show f246df6 -- components/ambient-api-server/plugins/projects/model.go 2>/dev/null | head -50

Repository: ambient-code/platform

Length of output: 1889

🏁 Script executed:

#!/bin/bash
# Search for any comments in the codebase about ID generation
rg -n "deterministic|construct.*id|derive.*id|from.*name" --type go -i 2>/dev/null | head -20

Repository: ambient-code/platform

Length of output: 2329

🏁 Script executed:

#!/bin/bash
# Search git log more broadly for projects changes
git log --oneline -50 -- components/ambient-api-server/plugins/projects/ 2>/dev/null

Repository: ambient-code/platform

Length of output: 124

🏁 Script executed:

#!/antml/bash
# Look for any test files that might indicate ID expectations
rg -n "NewID|project.*id|SetID" --type go components/ambient-api-server/plugins/projects/ -A 2 2>/dev/null | grep -i test

Repository: ambient-code/platform

Length of output: 47

🏁 Script executed:

#!/bin/bash
# Search for release notes, changelog, or migration guides
find . -type f \( -name "CHANGELOG*" -o -name "RELEASE*" -o -name "MIGRATION*" -o -name "*.md" \) 2>/dev/null | head -20

Repository: ambient-code/platform

Length of output: 812

🏁 Script executed:

#!/bin/bash
# Search for any comments or documentation about the ID change
rg -n "unique.*id|breaking.*change|project.*id" --type md -i 2>/dev/null | head -30

Repository: ambient-code/platform

Length of output: 4840

🏁 Script executed:

#!/bin/bash
# Check if the commit mentions anything about documentation
git show f246df6 --stat 2>/dev/null | head -20

Repository: ambient-code/platform

Length of output: 968

Project ID generation changed from deterministic to UUID—ensure release notes document this breaking change.

The commit "Fix workspace name uniqueness to be scoped per user" changed Project ID generation from d.ID = d.Name to d.ID = api.NewID(). This breaks any external integrations that relied on deriving Project IDs from names without querying the API. Document this in release notes and provide migration guidance for affected clients.

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

In `@components/ambient-api-server/plugins/projects/model.go` around lines 30 -
32, The Project.BeforeCreate method now sets d.ID = api.NewID() instead of
deriving IDs from d.Name, which is a breaking change for clients that inferred
project IDs; update the release notes to call out this change and add migration
guidance: document that Project IDs are now opaque UUIDs (referencing
Project.BeforeCreate and api.NewID()), advise clients to stop deriving IDs from
names and to fetch IDs via the Projects API, provide an example migration path
(map old name-based IDs to new IDs by querying existing projects or running a
one-time server-side migration), and note any compatibility window or
deprecation timeline for name-derived IDs.

}

Expand Down
1 change: 1 addition & 0 deletions components/ambient-api-server/plugins/projects/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,5 @@ func init() {
})

db.RegisterMigration(migration())
db.RegisterMigration(ownerUserIdMigration())
}