Skip to content

Commit c1889d0

Browse files
f0sseljohnstcn
f0ssel
and
johnstcn
authored
fix: clamp template port sharing level in SubAgentAPI (#26061) (#26318)
Backport of #26061 to `release/2.29` (SEC-80 / CODAGT-479). Conflict resolution: `CreateSubAgent` on this branch predates the terraform-defined devcontainer (req.Id) flow, so only the template fetch and port-share clamp were applied. Dropped `BoundaryUsageTracker`/`LifecycleMetrics` options that do not exist on 2.29. > 🤖 Backport prepared by Coder Agents on behalf of @f0ssel. --------- Co-authored-by: Cian Johnston <cian@coder.com>
1 parent 2044599 commit c1889d0

5 files changed

Lines changed: 565 additions & 3 deletions

File tree

‎coderd/agentapi/api.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/coder/coder/v2/coderd/database/pubsub"
2525
"github.com/coder/coder/v2/coderd/externalauth"
2626
"github.com/coder/coder/v2/coderd/notifications"
27+
"github.com/coder/coder/v2/coderd/portsharing"
2728
"github.com/coder/coder/v2/coderd/prometheusmetrics"
2829
"github.com/coder/coder/v2/coderd/tracing"
2930
"github.com/coder/coder/v2/coderd/workspacestats"
@@ -83,6 +84,7 @@ type Options struct {
8384
PublishWorkspaceUpdateFn func(ctx context.Context, userID uuid.UUID, event wspubsub.WorkspaceEvent)
8485
PublishWorkspaceAgentLogsUpdateFn func(ctx context.Context, workspaceAgentID uuid.UUID, msg agentsdk.LogsNotifyMessage)
8586
NetworkTelemetryHandler func(batch []*tailnetproto.TelemetryEvent)
87+
PortSharer *atomic.Pointer[portsharing.PortSharer]
8688

8789
AccessURL *url.URL
8890
AppHostname string
@@ -216,6 +218,7 @@ func New(opts Options, workspace database.Workspace) *API {
216218
Log: opts.Log,
217219
Clock: opts.Clock,
218220
Database: opts.Database,
221+
PortSharer: opts.PortSharer,
219222
}
220223

221224
// Start background cache refresh loop to handle workspace changes

‎coderd/agentapi/subagent.go‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"errors"
99
"fmt"
1010
"strings"
11+
"sync/atomic"
1112

1213
"github.com/google/uuid"
1314
"github.com/sqlc-dev/pqtype"
@@ -19,6 +20,7 @@ import (
1920
agentproto "github.com/coder/coder/v2/agent/proto"
2021
"github.com/coder/coder/v2/coderd/database"
2122
"github.com/coder/coder/v2/coderd/database/dbauthz"
23+
"github.com/coder/coder/v2/coderd/portsharing"
2224
"github.com/coder/coder/v2/codersdk"
2325
"github.com/coder/coder/v2/provisioner"
2426
)
@@ -29,9 +31,10 @@ type SubAgentAPI struct {
2931
AgentID uuid.UUID
3032
AgentFn func(context.Context) (database.WorkspaceAgent, error)
3133

32-
Log slog.Logger
33-
Clock quartz.Clock
34-
Database database.Store
34+
Log slog.Logger
35+
Clock quartz.Clock
36+
Database database.Store
37+
PortSharer *atomic.Pointer[portsharing.PortSharer]
3538
}
3639

3740
func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.CreateSubAgentRequest) (*agentproto.CreateSubAgentResponse, error) {
@@ -84,6 +87,21 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
8487
displayApps = append(displayApps, app)
8588
}
8689

90+
var template database.Template
91+
if len(req.Apps) > 0 {
92+
workspace, err := a.Database.GetWorkspaceByAgentID(ctx, parentAgent.ID)
93+
if err != nil {
94+
return nil, xerrors.Errorf("get workspace by agent id: %w", err)
95+
}
96+
97+
// Intentional: SubAgentAPI auth context enforces template ACL.
98+
// Normal workspace operations depend on this.
99+
template, err = a.Database.GetTemplateByID(ctx, workspace.TemplateID)
100+
if err != nil {
101+
return nil, xerrors.Errorf("get template policy: %w. If template access was recently changed, restart the workspace to refresh agent permissions", err)
102+
}
103+
}
104+
87105
subAgent, err := a.Database.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
88106
ID: uuid.New(),
89107
ParentID: uuid.NullUUID{Valid: true, UUID: parentAgent.ID},
@@ -110,6 +128,14 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
110128
return nil, xerrors.Errorf("insert sub agent: %w", err)
111129
}
112130

131+
// A nil PortSharer uses the AGPL default, which permits all share levels.
132+
portSharer := portsharing.DefaultPortSharer
133+
if a.PortSharer != nil {
134+
if loaded := a.PortSharer.Load(); loaded != nil {
135+
portSharer = *loaded
136+
}
137+
}
138+
113139
var appCreationErrors []*agentproto.CreateSubAgentResponse_AppCreationError
114140
appSlugs := make(map[string]struct{})
115141

@@ -153,6 +179,18 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
153179
}
154180
}
155181
sharingLevel := database.AppSharingLevel(strings.ToLower(protoSharingLevel))
182+
// Clamp instead of rejecting so a too-permissive app share level does
183+
// not block the sub-agent from starting.
184+
if err := portSharer.AuthorizedLevel(template, codersdk.WorkspaceAgentPortShareLevel(sharingLevel)); err != nil {
185+
a.Log.Warn(ctx, "clamping sub-agent app sharing level to template max port sharing level",
186+
slog.F("sub_agent_name", subAgent.Name),
187+
slog.F("sub_agent_id", subAgent.ID),
188+
slog.F("app_slug", slug),
189+
slog.F("requested_share_level", sharingLevel),
190+
slog.F("max_port_share_level", template.MaxPortSharingLevel),
191+
slog.Error(err))
192+
sharingLevel = template.MaxPortSharingLevel
193+
}
156194

157195
var openIn database.WorkspaceAppOpenIn
158196
switch app.GetOpenIn() {

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ var (
399399
User: []rbac.Permission{},
400400
ByOrgID: map[string]rbac.OrgPermissions{
401401
orgID.String(): {
402+
Org: rbac.Permissions(map[string][]policy.Action{
403+
// SubAgentAPI needs to check metadata of templates
404+
// potentially shared via group_acl.
405+
rbac.ResourceTemplate.Type: {policy.ActionRead},
406+
}),
402407
Member: rbac.Permissions(map[string][]policy.Action{
403408
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreateAgent, policy.ActionDeleteAgent},
404409
}),

‎coderd/workspaceagentsrpc.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) {
146146
PublishWorkspaceUpdateFn: api.publishWorkspaceUpdate,
147147
PublishWorkspaceAgentLogsUpdateFn: api.publishWorkspaceAgentLogsUpdate,
148148
NetworkTelemetryHandler: api.NetworkTelemetryBatcher.Handler,
149+
PortSharer: &api.PortSharer,
149150

150151
AccessURL: api.AccessURL,
151152
AppHostname: api.AppHostname,

0 commit comments

Comments
 (0)