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+74677639func TestGetWorkspaceAgentsByParentID(t *testing.T) {
74687640 t.Parallel()
74697641