Skip to content

Commit 9469b78

Browse files
authored
fix!: enforce regex for agent names (#16641)
Underscores and double hyphens are now blocked. The regex is almost the exact same as the `coder_app` `slug` regex, but uppercase characters are still permitted.
1 parent 92870f0 commit 9469b78

17 files changed

+365
-101
lines changed

coderd/database/dbauthz/dbauthz_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -3918,7 +3918,8 @@ func (s *MethodTestSuite) TestSystemFunctions() {
39183918
s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) {
39193919
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
39203920
check.Args(database.InsertWorkspaceAgentParams{
3921-
ID: uuid.New(),
3921+
ID: uuid.New(),
3922+
Name: "dev",
39223923
}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
39233924
}))
39243925
s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) {

coderd/database/dbfake/dbfake.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ func (b WorkspaceBuildBuilder) WithAgent(mutations ...func([]*sdkproto.Agent) []
9191
//nolint: revive // returns modified struct
9292
b.agentToken = uuid.NewString()
9393
agents := []*sdkproto.Agent{{
94-
Id: uuid.NewString(),
94+
Id: uuid.NewString(),
95+
Name: "dev",
9596
Auth: &sdkproto.Agent_Token{
9697
Token: b.agentToken,
9798
},

coderd/provisionerdserver/provisionerdserver.go

+16
Original file line numberDiff line numberDiff line change
@@ -1891,6 +1891,19 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid.
18911891
appSlugs = make(map[string]struct{})
18921892
)
18931893
for _, prAgent := range protoResource.Agents {
1894+
// Similar logic is duplicated in terraform/resources.go.
1895+
if prAgent.Name == "" {
1896+
return xerrors.Errorf("agent name cannot be empty")
1897+
}
1898+
// In 2025-02 we removed support for underscores in agent names. To
1899+
// provide a nicer error message, we check the regex first and check
1900+
// for underscores if it fails.
1901+
if !provisioner.AgentNameRegex.MatchString(prAgent.Name) {
1902+
if strings.Contains(prAgent.Name, "_") {
1903+
return xerrors.Errorf("agent name %q contains underscores which are no longer supported, please use hyphens instead (regex: %q)", prAgent.Name, provisioner.AgentNameRegex.String())
1904+
}
1905+
return xerrors.Errorf("agent name %q does not match regex %q", prAgent.Name, provisioner.AgentNameRegex.String())
1906+
}
18941907
// Agent names must be case-insensitive-unique, to be unambiguous in
18951908
// `coder_app`s and CoderVPN DNS names.
18961909
if _, ok := agentNames[strings.ToLower(prAgent.Name)]; ok {
@@ -2070,10 +2083,13 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid.
20702083
}
20712084

20722085
for _, app := range prAgent.Apps {
2086+
// Similar logic is duplicated in terraform/resources.go.
20732087
slug := app.Slug
20742088
if slug == "" {
20752089
return xerrors.Errorf("app must have a slug or name set")
20762090
}
2091+
// Contrary to agent names above, app slugs were never permitted to
2092+
// contain uppercase letters or underscores.
20772093
if !provisioner.AppSlugRegex.MatchString(slug) {
20782094
return xerrors.Errorf("app slug %q does not match regex %q", slug, provisioner.AppSlugRegex.String())
20792095
}

coderd/provisionerdserver/provisionerdserver_test.go

+89-1
Original file line numberDiff line numberDiff line change
@@ -1883,6 +1883,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
18831883
Name: "something",
18841884
Type: "aws_instance",
18851885
Agents: []*sdkproto.Agent{{
1886+
Name: "dev",
18861887
Auth: &sdkproto.Agent_Token{
18871888
Token: "bananas",
18881889
},
@@ -1896,14 +1897,69 @@ func TestInsertWorkspaceResource(t *testing.T) {
18961897
Name: "something",
18971898
Type: "aws_instance",
18981899
Agents: []*sdkproto.Agent{{
1900+
Name: "dev",
18991901
Apps: []*sdkproto.App{{
19001902
Slug: "a",
19011903
}, {
19021904
Slug: "a",
19031905
}},
19041906
}},
19051907
})
1906-
require.ErrorContains(t, err, "duplicate app slug")
1908+
require.ErrorContains(t, err, `duplicate app slug, must be unique per template: "a"`)
1909+
err = insert(dbmem.New(), uuid.New(), &sdkproto.Resource{
1910+
Name: "something",
1911+
Type: "aws_instance",
1912+
Agents: []*sdkproto.Agent{{
1913+
Name: "dev1",
1914+
Apps: []*sdkproto.App{{
1915+
Slug: "a",
1916+
}},
1917+
}, {
1918+
Name: "dev2",
1919+
Apps: []*sdkproto.App{{
1920+
Slug: "a",
1921+
}},
1922+
}},
1923+
})
1924+
require.ErrorContains(t, err, `duplicate app slug, must be unique per template: "a"`)
1925+
})
1926+
t.Run("AppSlugInvalid", func(t *testing.T) {
1927+
t.Parallel()
1928+
db := dbmem.New()
1929+
job := uuid.New()
1930+
err := insert(db, job, &sdkproto.Resource{
1931+
Name: "something",
1932+
Type: "aws_instance",
1933+
Agents: []*sdkproto.Agent{{
1934+
Name: "dev",
1935+
Apps: []*sdkproto.App{{
1936+
Slug: "dev_1",
1937+
}},
1938+
}},
1939+
})
1940+
require.ErrorContains(t, err, `app slug "dev_1" does not match regex`)
1941+
err = insert(db, job, &sdkproto.Resource{
1942+
Name: "something",
1943+
Type: "aws_instance",
1944+
Agents: []*sdkproto.Agent{{
1945+
Name: "dev",
1946+
Apps: []*sdkproto.App{{
1947+
Slug: "dev--1",
1948+
}},
1949+
}},
1950+
})
1951+
require.ErrorContains(t, err, `app slug "dev--1" does not match regex`)
1952+
err = insert(db, job, &sdkproto.Resource{
1953+
Name: "something",
1954+
Type: "aws_instance",
1955+
Agents: []*sdkproto.Agent{{
1956+
Name: "dev",
1957+
Apps: []*sdkproto.App{{
1958+
Slug: "Dev",
1959+
}},
1960+
}},
1961+
})
1962+
require.ErrorContains(t, err, `app slug "Dev" does not match regex`)
19071963
})
19081964
t.Run("DuplicateAgentNames", func(t *testing.T) {
19091965
t.Parallel()
@@ -1931,6 +1987,35 @@ func TestInsertWorkspaceResource(t *testing.T) {
19311987
})
19321988
require.ErrorContains(t, err, "duplicate agent name")
19331989
})
1990+
t.Run("AgentNameInvalid", func(t *testing.T) {
1991+
t.Parallel()
1992+
db := dbmem.New()
1993+
job := uuid.New()
1994+
err := insert(db, job, &sdkproto.Resource{
1995+
Name: "something",
1996+
Type: "aws_instance",
1997+
Agents: []*sdkproto.Agent{{
1998+
Name: "Dev",
1999+
}},
2000+
})
2001+
require.NoError(t, err) // uppercase is still allowed
2002+
err = insert(db, job, &sdkproto.Resource{
2003+
Name: "something",
2004+
Type: "aws_instance",
2005+
Agents: []*sdkproto.Agent{{
2006+
Name: "dev_1",
2007+
}},
2008+
})
2009+
require.ErrorContains(t, err, `agent name "dev_1" contains underscores`) // custom error for underscores
2010+
err = insert(db, job, &sdkproto.Resource{
2011+
Name: "something",
2012+
Type: "aws_instance",
2013+
Agents: []*sdkproto.Agent{{
2014+
Name: "dev--1",
2015+
}},
2016+
})
2017+
require.ErrorContains(t, err, `agent name "dev--1" does not match regex`)
2018+
})
19342019
t.Run("Success", func(t *testing.T) {
19352020
t.Parallel()
19362021
db := dbmem.New()
@@ -2007,6 +2092,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
20072092
Name: "something",
20082093
Type: "aws_instance",
20092094
Agents: []*sdkproto.Agent{{
2095+
Name: "dev",
20102096
DisplayApps: &sdkproto.DisplayApps{
20112097
Vscode: true,
20122098
VscodeInsiders: true,
@@ -2035,6 +2121,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
20352121
Name: "something",
20362122
Type: "aws_instance",
20372123
Agents: []*sdkproto.Agent{{
2124+
Name: "dev",
20382125
DisplayApps: &sdkproto.DisplayApps{},
20392126
}},
20402127
})
@@ -2059,6 +2146,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
20592146
Name: "something",
20602147
Type: "aws_instance",
20612148
Agents: []*sdkproto.Agent{{
2149+
Name: "dev",
20622150
DisplayApps: &sdkproto.DisplayApps{},
20632151
ResourcesMonitoring: &sdkproto.ResourcesMonitoring{
20642152
Memory: &sdkproto.MemoryResourceMonitor{

coderd/templateversions_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,7 @@ func TestTemplateVersionResources(t *testing.T) {
829829
Type: "example",
830830
Agents: []*proto.Agent{{
831831
Id: "something",
832+
Name: "dev",
832833
Auth: &proto.Agent_Token{},
833834
}},
834835
}, {
@@ -875,7 +876,8 @@ func TestTemplateVersionLogs(t *testing.T) {
875876
Name: "some",
876877
Type: "example",
877878
Agents: []*proto.Agent{{
878-
Id: "something",
879+
Id: "something",
880+
Name: "dev",
879881
Auth: &proto.Agent_Token{
880882
Token: uuid.NewString(),
881883
},

coderd/workspaceagents_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,8 @@ func TestWorkspaceAgentConnectRPC(t *testing.T) {
393393
Name: "example",
394394
Type: "aws_instance",
395395
Agents: []*proto.Agent{{
396-
Id: uuid.NewString(),
396+
Id: uuid.NewString(),
397+
Name: "dev",
397398
Auth: &proto.Agent_Token{
398399
Token: uuid.NewString(),
399400
},

coderd/workspacebuilds_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,7 @@ func TestWorkspaceBuildLogs(t *testing.T) {
720720
Type: "example",
721721
Agents: []*proto.Agent{{
722722
Id: "something",
723+
Name: "dev",
723724
Auth: &proto.Agent_Token{},
724725
}},
725726
}, {

coderd/workspaceresourceauth_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ func TestPostWorkspaceAuthAzureInstanceIdentity(t *testing.T) {
3333
Name: "somename",
3434
Type: "someinstance",
3535
Agents: []*proto.Agent{{
36+
Name: "dev",
3637
Auth: &proto.Agent_InstanceId{
3738
InstanceId: instanceID,
3839
},
@@ -78,6 +79,7 @@ func TestPostWorkspaceAuthAWSInstanceIdentity(t *testing.T) {
7879
Name: "somename",
7980
Type: "someinstance",
8081
Agents: []*proto.Agent{{
82+
Name: "dev",
8183
Auth: &proto.Agent_InstanceId{
8284
InstanceId: instanceID,
8385
},
@@ -164,6 +166,7 @@ func TestPostWorkspaceAuthGoogleInstanceIdentity(t *testing.T) {
164166
Name: "somename",
165167
Type: "someinstance",
166168
Agents: []*proto.Agent{{
169+
Name: "dev",
167170
Auth: &proto.Agent_InstanceId{
168171
InstanceId: instanceID,
169172
},

coderd/workspaces_test.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ func TestWorkspace(t *testing.T) {
219219
Type: "example",
220220
Agents: []*proto.Agent{{
221221
Id: uuid.NewString(),
222+
Name: "dev",
222223
Auth: &proto.Agent_Token{},
223224
}},
224225
}},
@@ -259,6 +260,7 @@ func TestWorkspace(t *testing.T) {
259260
Type: "example",
260261
Agents: []*proto.Agent{{
261262
Id: uuid.NewString(),
263+
Name: "dev",
262264
Auth: &proto.Agent_Token{},
263265
ConnectionTimeoutSeconds: 1,
264266
}},
@@ -1722,7 +1724,8 @@ func TestWorkspaceFilterManual(t *testing.T) {
17221724
Name: "example",
17231725
Type: "aws_instance",
17241726
Agents: []*proto.Agent{{
1725-
Id: uuid.NewString(),
1727+
Id: uuid.NewString(),
1728+
Name: "dev",
17261729
Auth: &proto.Agent_Token{
17271730
Token: authToken,
17281731
},
@@ -2729,7 +2732,8 @@ func TestWorkspaceWatcher(t *testing.T) {
27292732
Name: "example",
27302733
Type: "aws_instance",
27312734
Agents: []*proto.Agent{{
2732-
Id: uuid.NewString(),
2735+
Id: uuid.NewString(),
2736+
Name: "dev",
27332737
Auth: &proto.Agent_Token{
27342738
Token: authToken,
27352739
},
@@ -2951,6 +2955,7 @@ func TestWorkspaceResource(t *testing.T) {
29512955
Type: "example",
29522956
Agents: []*proto.Agent{{
29532957
Id: "something",
2958+
Name: "dev",
29542959
Auth: &proto.Agent_Token{},
29552960
Apps: apps,
29562961
}},
@@ -3025,6 +3030,7 @@ func TestWorkspaceResource(t *testing.T) {
30253030
Type: "example",
30263031
Agents: []*proto.Agent{{
30273032
Id: "something",
3033+
Name: "dev",
30283034
Auth: &proto.Agent_Token{},
30293035
Apps: apps,
30303036
}},
@@ -3068,6 +3074,7 @@ func TestWorkspaceResource(t *testing.T) {
30683074
Type: "example",
30693075
Agents: []*proto.Agent{{
30703076
Id: "something",
3077+
Name: "dev",
30713078
Auth: &proto.Agent_Token{},
30723079
}},
30733080
Metadata: []*proto.Resource_Metadata{{

enterprise/coderd/templates_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,11 @@ func TestTemplates(t *testing.T) {
161161
Name: "some",
162162
Type: "example",
163163
Agents: []*proto.Agent{{
164-
Id: "something",
164+
Id: "something",
165+
Name: "test",
165166
Auth: &proto.Agent_Token{
166167
Token: uuid.NewString(),
167168
},
168-
Name: "test",
169169
}},
170170
}, {
171171
Name: "another",

provisioner/appslug.go

-13
This file was deleted.

0 commit comments

Comments
 (0)