Skip to content

fix!: enforce regex for agent names #16641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 20, 2025
Merged

fix!: enforce regex for agent names #16641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2c3a14
fix!: enforce regex for agent names
deansheather Feb 20, 2025
d247016
fixup! fix!: enforce regex for agent names
deansheather Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion coderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3918,7 +3918,8 @@ func (s *MethodTestSuite) TestSystemFunctions() {
s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) {
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
check.Args(database.InsertWorkspaceAgentParams{
ID: uuid.New(),
ID: uuid.New(),
Name: "dev",
}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
}))
s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) {
Expand Down
3 changes: 2 additions & 1 deletion coderd/database/dbfake/dbfake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ func (b WorkspaceBuildBuilder) WithAgent(mutations ...func([]*sdkproto.Agent) []
//nolint: revive // returns modified struct
b.agentToken = uuid.NewString()
agents := []*sdkproto.Agent{{
Id: uuid.NewString(),
Id: uuid.NewString(),
Name: "dev",
Auth: &sdkproto.Agent_Token{
Token: b.agentToken,
},
Expand Down
16 changes: 16 additions & 0 deletions coderd/provisionerdserver/provisionerdserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1891,6 +1891,19 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid.
appSlugs = make(map[string]struct{})
)
for _, prAgent := range protoResource.Agents {
// Similar logic is duplicated in terraform/resources.go.
if prAgent.Name == "" {
return xerrors.Errorf("agent name cannot be empty")
}
// In 2025-02 we removed support for underscores in agent names. To
// provide a nicer error message, we check the regex first and check
// for underscores if it fails.
if !provisioner.AgentNameRegex.MatchString(prAgent.Name) {
if strings.Contains(prAgent.Name, "_") {
return xerrors.Errorf("agent name %q contains underscores which are no longer supported, please use hyphens instead (regex: %q)", prAgent.Name, provisioner.AgentNameRegex.String())
}
return xerrors.Errorf("agent name %q does not match regex %q", prAgent.Name, provisioner.AgentNameRegex.String())
}
// Agent names must be case-insensitive-unique, to be unambiguous in
// `coder_app`s and CoderVPN DNS names.
if _, ok := agentNames[strings.ToLower(prAgent.Name)]; ok {
Expand Down Expand Up @@ -2070,10 +2083,13 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid.
}

for _, app := range prAgent.Apps {
// Similar logic is duplicated in terraform/resources.go.
slug := app.Slug
if slug == "" {
return xerrors.Errorf("app must have a slug or name set")
}
// Contrary to agent names above, app slugs were never permitted to
// contain uppercase letters or underscores.
if !provisioner.AppSlugRegex.MatchString(slug) {
return xerrors.Errorf("app slug %q does not match regex %q", slug, provisioner.AppSlugRegex.String())
}
Expand Down
90 changes: 89 additions & 1 deletion coderd/provisionerdserver/provisionerdserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1883,6 +1883,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
Auth: &sdkproto.Agent_Token{
Token: "bananas",
},
Expand All @@ -1896,14 +1897,69 @@ func TestInsertWorkspaceResource(t *testing.T) {
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
Apps: []*sdkproto.App{{
Slug: "a",
}, {
Slug: "a",
}},
}},
})
require.ErrorContains(t, err, "duplicate app slug")
require.ErrorContains(t, err, `duplicate app slug, must be unique per template: "a"`)
err = insert(dbmem.New(), uuid.New(), &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev1",
Apps: []*sdkproto.App{{
Slug: "a",
}},
}, {
Name: "dev2",
Apps: []*sdkproto.App{{
Slug: "a",
}},
}},
})
require.ErrorContains(t, err, `duplicate app slug, must be unique per template: "a"`)
})
t.Run("AppSlugInvalid", func(t *testing.T) {
t.Parallel()
db := dbmem.New()
job := uuid.New()
err := insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
Apps: []*sdkproto.App{{
Slug: "dev_1",
}},
}},
})
require.ErrorContains(t, err, `app slug "dev_1" does not match regex`)
err = insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
Apps: []*sdkproto.App{{
Slug: "dev--1",
}},
}},
})
require.ErrorContains(t, err, `app slug "dev--1" does not match regex`)
err = insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
Apps: []*sdkproto.App{{
Slug: "Dev",
}},
}},
})
require.ErrorContains(t, err, `app slug "Dev" does not match regex`)
})
t.Run("DuplicateAgentNames", func(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -1931,6 +1987,35 @@ func TestInsertWorkspaceResource(t *testing.T) {
})
require.ErrorContains(t, err, "duplicate agent name")
})
t.Run("AgentNameInvalid", func(t *testing.T) {
t.Parallel()
db := dbmem.New()
job := uuid.New()
err := insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "Dev",
}},
})
require.NoError(t, err) // uppercase is still allowed
err = insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev_1",
}},
})
require.ErrorContains(t, err, `agent name "dev_1" contains underscores`) // custom error for underscores
err = insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev--1",
}},
})
require.ErrorContains(t, err, `agent name "dev--1" does not match regex`)
})
t.Run("Success", func(t *testing.T) {
t.Parallel()
db := dbmem.New()
Expand Down Expand Up @@ -2007,6 +2092,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
DisplayApps: &sdkproto.DisplayApps{
Vscode: true,
VscodeInsiders: true,
Expand Down Expand Up @@ -2035,6 +2121,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
DisplayApps: &sdkproto.DisplayApps{},
}},
})
Expand All @@ -2059,6 +2146,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
DisplayApps: &sdkproto.DisplayApps{},
ResourcesMonitoring: &sdkproto.ResourcesMonitoring{
Memory: &sdkproto.MemoryResourceMonitor{
Expand Down
4 changes: 3 additions & 1 deletion coderd/templateversions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ func TestTemplateVersionResources(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Name: "dev",
Auth: &proto.Agent_Token{},
}},
}, {
Expand Down Expand Up @@ -875,7 +876,8 @@ func TestTemplateVersionLogs(t *testing.T) {
Name: "some",
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Id: "something",
Name: "dev",
Auth: &proto.Agent_Token{
Token: uuid.NewString(),
},
Expand Down
3 changes: 2 additions & 1 deletion coderd/workspaceagents_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,8 @@ func TestWorkspaceAgentConnectRPC(t *testing.T) {
Name: "example",
Type: "aws_instance",
Agents: []*proto.Agent{{
Id: uuid.NewString(),
Id: uuid.NewString(),
Name: "dev",
Auth: &proto.Agent_Token{
Token: uuid.NewString(),
},
Expand Down
1 change: 1 addition & 0 deletions coderd/workspacebuilds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ func TestWorkspaceBuildLogs(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Name: "dev",
Auth: &proto.Agent_Token{},
}},
}, {
Expand Down
3 changes: 3 additions & 0 deletions coderd/workspaceresourceauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestPostWorkspaceAuthAzureInstanceIdentity(t *testing.T) {
Name: "somename",
Type: "someinstance",
Agents: []*proto.Agent{{
Name: "dev",
Auth: &proto.Agent_InstanceId{
InstanceId: instanceID,
},
Expand Down Expand Up @@ -78,6 +79,7 @@ func TestPostWorkspaceAuthAWSInstanceIdentity(t *testing.T) {
Name: "somename",
Type: "someinstance",
Agents: []*proto.Agent{{
Name: "dev",
Auth: &proto.Agent_InstanceId{
InstanceId: instanceID,
},
Expand Down Expand Up @@ -164,6 +166,7 @@ func TestPostWorkspaceAuthGoogleInstanceIdentity(t *testing.T) {
Name: "somename",
Type: "someinstance",
Agents: []*proto.Agent{{
Name: "dev",
Auth: &proto.Agent_InstanceId{
InstanceId: instanceID,
},
Expand Down
11 changes: 9 additions & 2 deletions coderd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ func TestWorkspace(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: uuid.NewString(),
Name: "dev",
Auth: &proto.Agent_Token{},
}},
}},
Expand Down Expand Up @@ -259,6 +260,7 @@ func TestWorkspace(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: uuid.NewString(),
Name: "dev",
Auth: &proto.Agent_Token{},
ConnectionTimeoutSeconds: 1,
}},
Expand Down Expand Up @@ -1722,7 +1724,8 @@ func TestWorkspaceFilterManual(t *testing.T) {
Name: "example",
Type: "aws_instance",
Agents: []*proto.Agent{{
Id: uuid.NewString(),
Id: uuid.NewString(),
Name: "dev",
Auth: &proto.Agent_Token{
Token: authToken,
},
Expand Down Expand Up @@ -2729,7 +2732,8 @@ func TestWorkspaceWatcher(t *testing.T) {
Name: "example",
Type: "aws_instance",
Agents: []*proto.Agent{{
Id: uuid.NewString(),
Id: uuid.NewString(),
Name: "dev",
Auth: &proto.Agent_Token{
Token: authToken,
},
Expand Down Expand Up @@ -2951,6 +2955,7 @@ func TestWorkspaceResource(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Name: "dev",
Auth: &proto.Agent_Token{},
Apps: apps,
}},
Expand Down Expand Up @@ -3025,6 +3030,7 @@ func TestWorkspaceResource(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Name: "dev",
Auth: &proto.Agent_Token{},
Apps: apps,
}},
Expand Down Expand Up @@ -3068,6 +3074,7 @@ func TestWorkspaceResource(t *testing.T) {
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Name: "dev",
Auth: &proto.Agent_Token{},
}},
Metadata: []*proto.Resource_Metadata{{
Expand Down
4 changes: 2 additions & 2 deletions enterprise/coderd/templates_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ func TestTemplates(t *testing.T) {
Name: "some",
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Id: "something",
Name: "test",
Auth: &proto.Agent_Token{
Token: uuid.NewString(),
},
Name: "test",
}},
}, {
Name: "another",
Expand Down
13 changes: 0 additions & 13 deletions provisioner/appslug.go
View file
Open in desktop

This file was deleted.

Loading
Loading