From 911d734df90af6f149c54c9667f308116a8120f8 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 19 Feb 2026 16:56:29 +0000 Subject: [PATCH] fix: avoid re-using `AuthInstanceID` for sub agents (#22196) Parent agents were re-using AuthInstanceID when spawning child agents. This caused GetWorkspaceAgentByInstanceID to return the most recently created sub agent instead of the parent when the parent tried to refetch its own manifest. Fix by not reusing AuthInstanceID for sub agents, and updating GetWorkspaceAgentByInstanceID to filter them out entirely. --- coderd/agentapi/subagent.go | 2 +- coderd/agentapi/subagent_test.go | 46 +++++++++++++++++ coderd/database/querier_test.go | 50 +++++++++++++++++++ coderd/database/queries.sql.go | 2 + coderd/database/queries/workspaceagents.sql | 2 + .../provisionerdserver/provisionerdserver.go | 2 +- .../provisionerdserver_test.go | 48 ++++++++++++++++++ 7 files changed, 150 insertions(+), 2 deletions(-) diff --git a/coderd/agentapi/subagent.go b/coderd/agentapi/subagent.go index 9e8f9b59c9..9dc2fd745d 100644 --- a/coderd/agentapi/subagent.go +++ b/coderd/agentapi/subagent.go @@ -128,7 +128,7 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create Name: agentName, ResourceID: parentAgent.ResourceID, AuthToken: uuid.New(), - AuthInstanceID: parentAgent.AuthInstanceID, + AuthInstanceID: sql.NullString{}, Architecture: req.Architecture, EnvironmentVariables: pqtype.NullRawMessage{}, OperatingSystem: req.OperatingSystem, diff --git a/coderd/agentapi/subagent_test.go b/coderd/agentapi/subagent_test.go index 5fe78aa9c3..348992f3f6 100644 --- a/coderd/agentapi/subagent_test.go +++ b/coderd/agentapi/subagent_test.go @@ -175,6 +175,52 @@ func TestSubAgentAPI(t *testing.T) { } }) + // Context: https://github.com/coder/coder/pull/22196 + t.Run("CreateSubAgentDoesNotInheritAuthInstanceID", func(t *testing.T) { + t.Parallel() + + var ( + log = testutil.Logger(t) + clock = quartz.NewMock(t) + + db, org = newDatabaseWithOrg(t) + user, agent = newUserWithWorkspaceAgent(t, db, org) + ) + + // Given: The parent agent has an AuthInstanceID set + ctx := testutil.Context(t, testutil.WaitShort) + parentAgent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), agent.ID) + require.NoError(t, err) + require.True(t, parentAgent.AuthInstanceID.Valid, "parent agent should have an AuthInstanceID") + require.NotEmpty(t, parentAgent.AuthInstanceID.String) + + api := newAgentAPI(t, log, db, clock, user, org, agent) + + // When: We create a sub agent + createResp, err := api.CreateSubAgent(ctx, &proto.CreateSubAgentRequest{ + Name: "sub-agent", + Directory: "/workspaces/test", + Architecture: "amd64", + OperatingSystem: "linux", + }) + require.NoError(t, err) + + subAgentID, err := uuid.FromBytes(createResp.Agent.Id) + require.NoError(t, err) + + // Then: The sub-agent must NOT re-use the parent's AuthInstanceID. + subAgent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), subAgentID) + require.NoError(t, err) + assert.False(t, subAgent.AuthInstanceID.Valid, "sub-agent should not have an AuthInstanceID") + assert.Empty(t, subAgent.AuthInstanceID.String, "sub-agent AuthInstanceID string should be empty") + + // Double-check: looking up by the parent's instance ID must + // still return the parent, not the sub-agent. + lookedUp, err := db.GetWorkspaceAgentByInstanceID(dbauthz.AsSystemRestricted(ctx), parentAgent.AuthInstanceID.String) + require.NoError(t, err) + assert.Equal(t, parentAgent.ID, lookedUp.ID, "instance ID lookup should still return the parent agent") + }) + type expectedAppError struct { index int32 field string diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index e61f2f135a..fc3a34981b 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6319,6 +6319,56 @@ func TestGetWorkspaceAgentsByParentID(t *testing.T) { }) } +func TestGetWorkspaceAgentByInstanceID(t *testing.T) { + t.Parallel() + + // Context: https://github.com/coder/coder/pull/22196 + t.Run("DoesNotReturnSubAgents", func(t *testing.T) { + t.Parallel() + + // Given: A parent workspace agent with an AuthInstanceID and a + // sub-agent that shares the same AuthInstanceID. + db, _ := dbtestutil.NewDB(t) + org := dbgen.Organization(t, db, database.Organization{}) + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + Type: database.ProvisionerJobTypeTemplateVersionImport, + OrganizationID: org.ID, + }) + resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ + JobID: job.ID, + }) + + authInstanceID := fmt.Sprintf("instance-%s-%d", t.Name(), time.Now().UnixNano()) + parentAgent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{ + ResourceID: resource.ID, + AuthInstanceID: sql.NullString{ + String: authInstanceID, + Valid: true, + }, + }) + // Create a sub-agent with the same AuthInstanceID (simulating + // the old behavior before the fix). + _ = dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{ + ParentID: uuid.NullUUID{UUID: parentAgent.ID, Valid: true}, + ResourceID: resource.ID, + AuthInstanceID: sql.NullString{ + String: authInstanceID, + Valid: true, + }, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + + // When: We look up the agent by instance ID. + agent, err := db.GetWorkspaceAgentByInstanceID(ctx, authInstanceID) + require.NoError(t, err) + + // Then: The result must be the parent agent, not the sub-agent. + assert.Equal(t, parentAgent.ID, agent.ID, "instance ID lookup should return the parent agent, not a sub-agent") + assert.False(t, agent.ParentID.Valid, "returned agent should not have a parent (should be the parent itself)") + }) +} + func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { t.Helper() require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ee851174ba..2268d6ad24 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -18251,6 +18251,8 @@ WHERE auth_instance_id = $1 :: TEXT -- Filter out deleted sub agents. AND deleted = FALSE + -- Filter out sub agents, they do not authenticate with auth_instance_id. + AND parent_id IS NULL ORDER BY created_at DESC ` diff --git a/coderd/database/queries/workspaceagents.sql b/coderd/database/queries/workspaceagents.sql index e1cd648dad..7f8b53696a 100644 --- a/coderd/database/queries/workspaceagents.sql +++ b/coderd/database/queries/workspaceagents.sql @@ -17,6 +17,8 @@ WHERE auth_instance_id = @auth_instance_id :: TEXT -- Filter out deleted sub agents. AND deleted = FALSE + -- Filter out sub agents, they do not authenticate with auth_instance_id. + AND parent_id IS NULL ORDER BY created_at DESC; diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 1f527a2998..23200bfa89 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -3319,7 +3319,7 @@ func insertDevcontainerSubagent( ResourceID: resourceID, Name: dc.GetName(), AuthToken: uuid.New(), - AuthInstanceID: parentAgent.AuthInstanceID, + AuthInstanceID: sql.NullString{}, Architecture: parentAgent.Architecture, EnvironmentVariables: envJSON, Directory: dc.GetWorkspaceFolder(), diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 3ffeb8da57..6509591595 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -4072,6 +4072,54 @@ func TestInsertWorkspaceResource(t *testing.T) { }}, }, }, + { + // This test verifies that subagents created via + // devcontainers do not inherit the parent agent's + // AuthInstanceID. + // Context: https://github.com/coder/coder/pull/22196 + name: "SubAgentDoesNotInheritAuthInstanceID", + resource: &sdkproto.Resource{ + Name: "something", + Type: "aws_instance", + Agents: []*sdkproto.Agent{{ + Id: agentID.String(), + Name: "dev", + Architecture: "amd64", + OperatingSystem: "linux", + Auth: &sdkproto.Agent_InstanceId{ + InstanceId: "parent-instance-id", + }, + Devcontainers: []*sdkproto.Devcontainer{{ + Id: devcontainerID.String(), + Name: "sub", + WorkspaceFolder: "/workspace", + SubagentId: subAgentID.String(), + Apps: []*sdkproto.App{ + {Slug: "code-server", DisplayName: "VS Code", Url: "http://localhost:8080"}, + }, + }}, + }}, + }, + expectSubAgentCount: 1, + check: func(t *testing.T, db database.Store, parentAgent database.WorkspaceAgent, subAgents []database.WorkspaceAgent, _ bool) { + // Parent should have the AuthInstanceID set. + require.True(t, parentAgent.AuthInstanceID.Valid, "parent agent should have an AuthInstanceID") + require.Equal(t, "parent-instance-id", parentAgent.AuthInstanceID.String) + + require.Len(t, subAgents, 1) + subAgent := subAgents[0] + + // Sub-agent must NOT inherit the parent's AuthInstanceID. + assert.False(t, subAgent.AuthInstanceID.Valid, "sub-agent should not have an AuthInstanceID") + assert.Empty(t, subAgent.AuthInstanceID.String, "sub-agent AuthInstanceID string should be empty") + + // Looking up by the parent's instance ID must still + // return the parent, not the sub-agent. + lookedUp, err := db.GetWorkspaceAgentByInstanceID(ctx, parentAgent.AuthInstanceID.String) + require.NoError(t, err) + assert.Equal(t, parentAgent.ID, lookedUp.ID, "instance ID lookup should still return the parent agent") + }, + }, { // This test verifies the backward-compatibility behavior where a // devcontainer with a SubagentId but no apps, scripts, or envs does