Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 0 deletions coderd/agentapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/coder/coder/v2/coderd/database/pubsub"
"github.com/coder/coder/v2/coderd/externalauth"
"github.com/coder/coder/v2/coderd/notifications"
"github.com/coder/coder/v2/coderd/portsharing"
"github.com/coder/coder/v2/coderd/prometheusmetrics"
"github.com/coder/coder/v2/coderd/tracing"
"github.com/coder/coder/v2/coderd/workspacestats"
Expand Down Expand Up @@ -90,6 +91,7 @@ type Options struct {
NetworkTelemetryHandler func(batch []*tailnetproto.TelemetryEvent)
BoundaryUsageTracker *boundaryusage.Tracker
LifecycleMetrics *LifecycleMetrics
PortSharer *atomic.Pointer[portsharing.PortSharer]

AccessURL *url.URL
AppHostname string
Expand Down Expand Up @@ -230,6 +232,7 @@ func New(opts Options, workspace database.Workspace, agent database.WorkspaceAge
Log: opts.Log,
Clock: opts.Clock,
Database: opts.Database,
PortSharer: opts.PortSharer,
}

api.BoundaryLogsAPI = &BoundaryLogsAPI{
Expand Down
44 changes: 41 additions & 3 deletions coderd/agentapi/subagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"strings"
"sync/atomic"

"github.com/google/uuid"
"github.com/sqlc-dev/pqtype"
Expand All @@ -17,6 +18,7 @@ import (
agentproto "github.com/coder/coder/v2/agent/proto"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/portsharing"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/provisioner"
"github.com/coder/quartz"
Expand All @@ -27,9 +29,10 @@ type SubAgentAPI struct {
OrganizationID uuid.UUID
AgentFn func(context.Context) (database.WorkspaceAgent, error)

Log slog.Logger
Clock quartz.Clock
Database database.Store
Log slog.Logger
Clock quartz.Clock
Database database.Store
PortSharer *atomic.Pointer[portsharing.PortSharer]
Comment thread
johnstcn marked this conversation as resolved.
}

func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.CreateSubAgentRequest) (*agentproto.CreateSubAgentResponse, error) {
Expand Down Expand Up @@ -129,6 +132,21 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
Detail: fmt.Sprintf("agent name %q does not match regex %q", agentName, provisioner.AgentNameRegex),
}
}
var template database.Template
Comment thread
johnstcn marked this conversation as resolved.
if len(req.Apps) > 0 {
workspace, err := a.Database.GetWorkspaceByAgentID(ctx, parentAgent.ID)
if err != nil {
return nil, xerrors.Errorf("get workspace by agent id: %w", err)
}

// Intentional: SubAgentAPI auth context enforces template ACL.
// Normal workspace operations depend on this.
template, err = a.Database.GetTemplateByID(ctx, workspace.TemplateID)
if err != nil {
return nil, xerrors.Errorf("get template policy: %w. If template access was recently changed, restart the workspace to refresh agent permissions", err)
}
}
Comment thread
johnstcn marked this conversation as resolved.

subAgent, err := a.Database.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
ParentID: uuid.NullUUID{Valid: true, UUID: parentAgent.ID},
Expand All @@ -155,6 +173,14 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
return nil, xerrors.Errorf("insert sub agent: %w", err)
}

// A nil PortSharer uses the AGPL default, which permits all share levels.
portSharer := portsharing.DefaultPortSharer
if a.PortSharer != nil {
if loaded := a.PortSharer.Load(); loaded != nil {
portSharer = *loaded
}
}

var appCreationErrors []*agentproto.CreateSubAgentResponse_AppCreationError
appSlugs := make(map[string]struct{})

Expand Down Expand Up @@ -198,6 +224,18 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create
}
}
sharingLevel := database.AppSharingLevel(strings.ToLower(protoSharingLevel))
// Clamp instead of rejecting so a too-permissive app share level does
// not block the sub-agent from starting.
if err := portSharer.AuthorizedLevel(template, codersdk.WorkspaceAgentPortShareLevel(sharingLevel)); err != nil {
Comment thread
johnstcn marked this conversation as resolved.
a.Log.Warn(ctx, "clamping sub-agent app sharing level to template max port sharing level",
slog.F("sub_agent_name", subAgent.Name),
slog.F("sub_agent_id", subAgent.ID),
slog.F("app_slug", slug),
slog.F("requested_share_level", sharingLevel),
slog.F("max_port_share_level", template.MaxPortSharingLevel),
slog.Error(err))
sharingLevel = template.MaxPortSharingLevel
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to surface this as a warning in the response / on the workspace agent side. But that's definitely not in scope, maybe a future improvement/refactor.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% agreed, unfortunately we don't seem to have an easy way to surface this kind of things to the user.


var openIn database.WorkspaceAppOpenIn
switch app.GetOpenIn() {
Expand Down
5 changes: 5 additions & 0 deletions coderd/database/dbauthz/dbauthz.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,11 @@ var (
User: []rbac.Permission{},
ByOrgID: map[string]rbac.OrgPermissions{
orgID.String(): {
Org: rbac.Permissions(map[string][]policy.Action{
// SubAgentAPI needs to check metadata of templates
// potentially shared via group_acl.
rbac.ResourceTemplate.Type: {policy.ActionRead},
}),
Member: rbac.Permissions(map[string][]policy.Action{
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreateAgent, policy.ActionDeleteAgent, policy.ActionUpdateAgent},
}),
Expand Down
1 change: 1 addition & 0 deletions coderd/workspaceagentsrpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) {
PublishWorkspaceAgentLogsUpdateFn: api.publishWorkspaceAgentLogsUpdate,
NetworkTelemetryHandler: api.NetworkTelemetryBatcher.Handler,
BoundaryUsageTracker: api.BoundaryUsageTracker,
PortSharer: &api.PortSharer,

AccessURL: api.AccessURL,
AppHostname: api.AppHostname,
Expand Down
Loading
Loading