Skip to content

Commit 74b5cfb

Browse files
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.
1 parent b89dc43 commit 74b5cfb

File tree

5 files changed

+101
-1
lines changed

5 files changed

+101
-1
lines changed

coderd/agentapi/subagent.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
9191
Name: agentName,
9292
ResourceID: parentAgent.ResourceID,
9393
AuthToken: uuid.New(),
94-
AuthInstanceID: parentAgent.AuthInstanceID,
94+
AuthInstanceID: sql.NullString{},
9595
Architecture: req.Architecture,
9696
EnvironmentVariables: pqtype.NullRawMessage{},
9797
OperatingSystem: req.OperatingSystem,

coderd/agentapi/subagent_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,52 @@ func TestSubAgentAPI(t *testing.T) {
175175
}
176176
})
177177

178+
// Context: https://github.com/coder/coder/pull/22196
179+
t.Run("CreateSubAgentDoesNotInheritAuthInstanceID", func(t *testing.T) {
180+
t.Parallel()
181+
182+
var (
183+
log = testutil.Logger(t)
184+
clock = quartz.NewMock(t)
185+
186+
db, org = newDatabaseWithOrg(t)
187+
user, agent = newUserWithWorkspaceAgent(t, db, org)
188+
)
189+
190+
// Given: The parent agent has an AuthInstanceID set
191+
ctx := testutil.Context(t, testutil.WaitShort)
192+
parentAgent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), agent.ID)
193+
require.NoError(t, err)
194+
require.True(t, parentAgent.AuthInstanceID.Valid, "parent agent should have an AuthInstanceID")
195+
require.NotEmpty(t, parentAgent.AuthInstanceID.String)
196+
197+
api := newAgentAPI(t, log, db, clock, user, org, agent)
198+
199+
// When: We create a sub agent
200+
createResp, err := api.CreateSubAgent(ctx, &proto.CreateSubAgentRequest{
201+
Name: "sub-agent",
202+
Directory: "/workspaces/test",
203+
Architecture: "amd64",
204+
OperatingSystem: "linux",
205+
})
206+
require.NoError(t, err)
207+
208+
subAgentID, err := uuid.FromBytes(createResp.Agent.Id)
209+
require.NoError(t, err)
210+
211+
// Then: The sub-agent must NOT re-use the parent's AuthInstanceID.
212+
subAgent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), subAgentID)
213+
require.NoError(t, err)
214+
assert.False(t, subAgent.AuthInstanceID.Valid, "sub-agent should not have an AuthInstanceID")
215+
assert.Empty(t, subAgent.AuthInstanceID.String, "sub-agent AuthInstanceID string should be empty")
216+
217+
// Double-check: looking up by the parent's instance ID must
218+
// still return the parent, not the sub-agent.
219+
lookedUp, err := db.GetWorkspaceAgentByInstanceID(dbauthz.AsSystemRestricted(ctx), parentAgent.AuthInstanceID.String)
220+
require.NoError(t, err)
221+
assert.Equal(t, parentAgent.ID, lookedUp.ID, "instance ID lookup should still return the parent agent")
222+
})
223+
178224
type expectedAppError struct {
179225
index int32
180226
field string

coderd/database/querier_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6271,6 +6271,56 @@ func TestGetWorkspaceAgentsByParentID(t *testing.T) {
62716271
})
62726272
}
62736273

6274+
func TestGetWorkspaceAgentByInstanceID(t *testing.T) {
6275+
t.Parallel()
6276+
6277+
// Context: https://github.com/coder/coder/pull/22196
6278+
t.Run("DoesNotReturnSubAgents", func(t *testing.T) {
6279+
t.Parallel()
6280+
6281+
// Given: A parent workspace agent with an AuthInstanceID and a
6282+
// sub-agent that shares the same AuthInstanceID.
6283+
db, _ := dbtestutil.NewDB(t)
6284+
org := dbgen.Organization(t, db, database.Organization{})
6285+
job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{
6286+
Type: database.ProvisionerJobTypeTemplateVersionImport,
6287+
OrganizationID: org.ID,
6288+
})
6289+
resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
6290+
JobID: job.ID,
6291+
})
6292+
6293+
authInstanceID := fmt.Sprintf("instance-%s-%d", t.Name(), time.Now().UnixNano())
6294+
parentAgent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
6295+
ResourceID: resource.ID,
6296+
AuthInstanceID: sql.NullString{
6297+
String: authInstanceID,
6298+
Valid: true,
6299+
},
6300+
})
6301+
// Create a sub-agent with the same AuthInstanceID (simulating
6302+
// the old behavior before the fix).
6303+
_ = dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
6304+
ParentID: uuid.NullUUID{UUID: parentAgent.ID, Valid: true},
6305+
ResourceID: resource.ID,
6306+
AuthInstanceID: sql.NullString{
6307+
String: authInstanceID,
6308+
Valid: true,
6309+
},
6310+
})
6311+
6312+
ctx := testutil.Context(t, testutil.WaitShort)
6313+
6314+
// When: We look up the agent by instance ID.
6315+
agent, err := db.GetWorkspaceAgentByInstanceID(ctx, authInstanceID)
6316+
require.NoError(t, err)
6317+
6318+
// Then: The result must be the parent agent, not the sub-agent.
6319+
assert.Equal(t, parentAgent.ID, agent.ID, "instance ID lookup should return the parent agent, not a sub-agent")
6320+
assert.False(t, agent.ParentID.Valid, "returned agent should not have a parent (should be the parent itself)")
6321+
})
6322+
}
6323+
62746324
func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) {
62756325
t.Helper()
62766326
require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg)

coderd/database/queries.sql.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries/workspaceagents.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ WHERE
1717
auth_instance_id = @auth_instance_id :: TEXT
1818
-- Filter out deleted sub agents.
1919
AND deleted = FALSE
20+
-- Filter out sub agents, they do not authenticate with auth_instance_id.
21+
AND parent_id IS NULL
2022
ORDER BY
2123
created_at DESC;
2224

0 commit comments

Comments
 (0)