◐ Shell
clean mode source ↗

fix(coderd): prevent cross-tenant workspace app rebinding (#26103) (#… · coder/coder@e4a7657

@@ -7464,6 +7464,178 @@ func TestWorkspaceAgentNameUniqueTrigger(t *testing.T) {

74647464

})

74657465

}

746674667467+

func TestUpsertWorkspaceAppCannotRebindAcrossWorkspaces(t *testing.T) {

7468+

t.Parallel()

7469+7470+

db, _ := dbtestutil.NewDB(t)

7471+

org := dbgen.Organization(t, db, database.Organization{})

7472+

ctx := testutil.Context(t, testutil.WaitShort)

7473+7474+

// createWorkspace builds the owner -> template -> version -> workspace chain

7475+

// and returns the workspace plus its template version so callers can create

7476+

// additional builds (and thus agents) within the same workspace.

7477+

createWorkspace := func(t *testing.T) (database.WorkspaceTable, uuid.UUID) {

7478+

t.Helper()

7479+

user := dbgen.User(t, db, database.User{})

7480+

template := dbgen.Template(t, db, database.Template{

7481+

OrganizationID: org.ID,

7482+

CreatedBy: user.ID,

7483+

})

7484+

version := dbgen.TemplateVersion(t, db, database.TemplateVersion{

7485+

TemplateID: uuid.NullUUID{Valid: true, UUID: template.ID},

7486+

OrganizationID: org.ID,

7487+

CreatedBy: user.ID,

7488+

})

7489+

workspace := dbgen.Workspace(t, db, database.WorkspaceTable{

7490+

OrganizationID: org.ID,

7491+

TemplateID: template.ID,

7492+

OwnerID: user.ID,

7493+

})

7494+

return workspace, version.ID

7495+

}

7496+7497+

// addAgent creates a build, resource, and agent for the workspace. The

7498+

// build's JobID matches the resource's JobID so the upsert's

7499+

// agent -> resource -> workspace_builds(job_id) -> workspace_id traversal

7500+

// resolves to the workspace.

7501+

addAgent := func(t *testing.T, workspace database.WorkspaceTable, versionID uuid.UUID, buildNumber int32) database.WorkspaceAgent {

7502+

t.Helper()

7503+

job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{

7504+

Type: database.ProvisionerJobTypeWorkspaceBuild,

7505+

OrganizationID: org.ID,

7506+

})

7507+

dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{

7508+

BuildNumber: buildNumber,

7509+

JobID: job.ID,

7510+

WorkspaceID: workspace.ID,

7511+

TemplateVersionID: versionID,

7512+

})

7513+

resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{

7514+

JobID: job.ID,

7515+

})

7516+

return dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{

7517+

ResourceID: resource.ID,

7518+

})

7519+

}

7520+7521+

upsertApp := func(appID, agentID uuid.UUID, slug string) (database.WorkspaceApp, error) {

7522+

return db.UpsertWorkspaceApp(ctx, database.UpsertWorkspaceAppParams{

7523+

ID: appID,

7524+

CreatedAt: dbtime.Now(),

7525+

AgentID: agentID,

7526+

Slug: slug,

7527+

DisplayName: "Code Server",

7528+

Icon: "/icon.png",

7529+

SharingLevel: database.AppSharingLevelOwner,

7530+

Health: database.WorkspaceAppHealthDisabled,

7531+

OpenIn: database.WorkspaceAppOpenInSlimWindow,

7532+

})

7533+

}

7534+7535+

// Given: two independent workspaces, each with an agent that resolves to its

7536+

// own workspace.

7537+

workspaceA, versionA := createWorkspace(t)

7538+

workspaceB, versionB := createWorkspace(t)

7539+

agentA := addAgent(t, workspaceA, versionA, 1)

7540+

agentB := addAgent(t, workspaceB, versionB, 1)

7541+7542+

gotA, err := db.GetWorkspaceByAgentID(ctx, agentA.ID)

7543+

require.NoError(t, err)

7544+

require.Equal(t, workspaceA.ID, gotA.ID)

7545+

gotB, err := db.GetWorkspaceByAgentID(ctx, agentB.ID)

7546+

require.NoError(t, err)

7547+

require.Equal(t, workspaceB.ID, gotB.ID)

7548+7549+

appID := uuid.New()

7550+

const originalSlug = "code-server"

7551+7552+

// Initial insert under workspace A's agent succeeds (no conflict).

7553+

app, err := upsertApp(appID, agentA.ID, originalSlug)

7554+

require.NoError(t, err)

7555+

require.Equal(t, appID, app.ID)

7556+

require.Equal(t, agentA.ID, app.AgentID)

7557+

require.Equal(t, originalSlug, app.Slug)

7558+7559+

// Upserting the same app id onto workspace B's agent is rejected because the

7560+

// existing row and the incoming agent resolve to different workspaces. The

7561+

// guard updates zero rows, so the :one query returns sql.ErrNoRows.

7562+

_, err = upsertApp(appID, agentB.ID, "hijacked")

7563+

require.ErrorIs(t, err, sql.ErrNoRows)

7564+7565+

// The app remains bound to workspace A's agent, unchanged.

7566+

appsA, err := db.GetWorkspaceAppsByAgentID(ctx, agentA.ID)

7567+

require.NoError(t, err)

7568+

require.Len(t, appsA, 1)

7569+

require.Equal(t, appID, appsA[0].ID)

7570+

require.Equal(t, agentA.ID, appsA[0].AgentID)

7571+

require.Equal(t, originalSlug, appsA[0].Slug)

7572+7573+

// Workspace B's agent has no app.

7574+

appsB, err := db.GetWorkspaceAppsByAgentID(ctx, agentB.ID)

7575+

require.NoError(t, err)

7576+

require.Empty(t, appsB)

7577+7578+

// A legitimate rebuild of workspace A produces a new agent (agent IDs are

7579+

// regenerated every build). Rebinding the persistent app to it succeeds

7580+

// because both agents resolve to workspace A.

7581+

agentA2 := addAgent(t, workspaceA, versionA, 2)

7582+

app, err = upsertApp(appID, agentA2.ID, "code-server-v2")

7583+

require.NoError(t, err)

7584+

require.Equal(t, agentA2.ID, app.AgentID)

7585+

require.Equal(t, "code-server-v2", app.Slug)

7586+7587+

appsA2, err := db.GetWorkspaceAppsByAgentID(ctx, agentA2.ID)

7588+

require.NoError(t, err)

7589+

require.Len(t, appsA2, 1)

7590+

require.Equal(t, appID, appsA2[0].ID)

7591+7592+

// Set up a template-import agent. It is intentionally not associated with

7593+

// a workspace build, so it resolves to no workspace.

7594+

importJob := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{

7595+

Type: database.ProvisionerJobTypeTemplateVersionImport,

7596+

OrganizationID: org.ID,

7597+

})

7598+

importResource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{

7599+

JobID: importJob.ID,

7600+

})

7601+

importAgent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{

7602+

ResourceID: importResource.ID,

7603+

})

7604+

_, err = db.GetWorkspaceByAgentID(ctx, importAgent.ID)

7605+

require.ErrorIs(t, err, sql.ErrNoRows, "import agent must not resolve to a workspace")

7606+7607+

// An app that already belongs to a workspace cannot be rebound to a

7608+

// template-import agent. Otherwise a second update could move it from

7609+

// the import agent to a different workspace.

7610+

_, err = upsertApp(appID, importAgent.ID, "hijacked-by-import")

7611+

require.ErrorIs(t, err, sql.ErrNoRows)

7612+7613+

appsA2, err = db.GetWorkspaceAppsByAgentID(ctx, agentA2.ID)

7614+

require.NoError(t, err)

7615+

require.Len(t, appsA2, 1)

7616+

require.Equal(t, appID, appsA2[0].ID)

7617+

require.Equal(t, agentA2.ID, appsA2[0].AgentID)

7618+

require.Equal(t, "code-server-v2", appsA2[0].Slug)

7619+7620+

appsImport, err := db.GetWorkspaceAppsByAgentID(ctx, importAgent.ID)

7621+

require.NoError(t, err)

7622+

require.Empty(t, appsImport)

7623+7624+

_, err = upsertApp(appID, agentB.ID, "hijacked-after-import")

7625+

require.ErrorIs(t, err, sql.ErrNoRows)

7626+7627+

unownedAppID := uuid.New()

7628+

_, err = upsertApp(unownedAppID, importAgent.ID, "import-app")

7629+

require.NoError(t, err)

7630+7631+

// An app whose existing agent belongs to a template-import job resolves to

7632+

// no workspace, so rebinding it is permitted. It is not a cross-tenant

7633+

// victim.

7634+

rebound, err := upsertApp(unownedAppID, agentA.ID, "import-app")

7635+

require.NoError(t, err)

7636+

require.Equal(t, agentA.ID, rebound.AgentID)

7637+

}

7638+74677639

func TestGetWorkspaceAgentsByParentID(t *testing.T) {

74687640

t.Parallel()

74697641