mirror of
https://github.com/coder/coder.git
synced 2026-06-07 06:58:17 +00:00
fix: clamp template port sharing level in SubAgentAPI (#26061)
Fixes an issue where sub-agent apps created via CreateSubAgent would
bypass the check for the template's max port sharing level:
- Clamps dynamically inserted `workspace_apps` to the template max
sharing level in `coderd.agentapi.SubAgentAPI`.
- Emits a warning when clamping occurs.
- Adds unit test coverage for the max sharing level matrix.
- Adds an integration-ish test through the devcontainer sub-agent client
path.
> 🤖 Generated by Coder Agents with guidance from a human.
This commit is contained in:
@@ -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"
|
||||
@@ -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
|
||||
@@ -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{
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/sqlc-dev/pqtype"
|
||||
@@ -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"
|
||||
@@ -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]
|
||||
}
|
||||
|
||||
func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.CreateSubAgentRequest) (*agentproto.CreateSubAgentResponse, error) {
|
||||
@@ -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
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
subAgent, err := a.Database.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
|
||||
ID: uuid.New(),
|
||||
ParentID: uuid.NullUUID{Valid: true, UUID: parentAgent.ID},
|
||||
@@ -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{})
|
||||
|
||||
@@ -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 {
|
||||
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
|
||||
}
|
||||
|
||||
var openIn database.WorkspaceAppOpenIn
|
||||
switch app.GetOpenIn() {
|
||||
|
||||
Reference in New Issue
Block a user