fix(coderd): scope provisioner module file downloads to the daemon's org (#26635)

This commit is contained in:
Jon Ayers
2026-06-24 12:09:22 -05:00
committed by GitHub
parent 9defdb4af2
commit 1961908ca7
9 changed files with 246 additions and 0 deletions
+10
View File
@@ -5697,6 +5697,16 @@ func (q *querier) GetWorkspacesForWorkspaceMetrics(ctx context.Context) ([]datab
return q.db.GetWorkspacesForWorkspaceMetrics(ctx)
}
func (q *querier) HasTemplateVersionsUsingCachedModuleFileInOrg(ctx context.Context, arg database.HasTemplateVersionsUsingCachedModuleFileInOrgParams) (bool, error) {
// This query authorizes provisioner module-file downloads. The caller
// must be able to read files in the target organization; the actual
// tenant isolation comes from the organization_id filter in the query.
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceFile.InOrg(arg.OrganizationID)); err != nil {
return false, err
}
return q.db.HasTemplateVersionsUsingCachedModuleFileInOrg(ctx, arg)
}
func (q *querier) HydrateAgentChatsContext(ctx context.Context, arg database.HydrateAgentChatsContextParams) error {
// System-level operation: an agent context push fans hydration out
// across every not-yet-pinned chat for the agent, so it authorizes at
+5
View File
@@ -2674,6 +2674,11 @@ func (s *MethodTestSuite) TestTemplate() {
dbm.EXPECT().GetTemplateVersionTerraformValues(gomock.Any(), tv.ID).Return(val, nil).AnyTimes()
check.Args(tv.ID).Asserts(t, policy.ActionRead)
}))
s.Run("HasTemplateVersionsUsingCachedModuleFileInOrg", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
arg := database.HasTemplateVersionsUsingCachedModuleFileInOrgParams{FileID: uuid.New(), OrganizationID: uuid.New()}
dbm.EXPECT().HasTemplateVersionsUsingCachedModuleFileInOrg(gomock.Any(), arg).Return(true, nil).AnyTimes()
check.Args(arg).Asserts(rbac.ResourceFile.InOrg(arg.OrganizationID), policy.ActionRead).Returns(true)
}))
s.Run("GetTemplateVersionVariables", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
t1 := testutil.Fake(s.T(), faker, database.Template{})
tv := testutil.Fake(s.T(), faker, database.TemplateVersion{TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}})
+8
View File
@@ -3874,6 +3874,14 @@ func (m queryMetricsStore) GetWorkspacesForWorkspaceMetrics(ctx context.Context)
return r0, r1
}
func (m queryMetricsStore) HasTemplateVersionsUsingCachedModuleFileInOrg(ctx context.Context, arg database.HasTemplateVersionsUsingCachedModuleFileInOrgParams) (bool, error) {
start := time.Now()
r0, r1 := m.s.HasTemplateVersionsUsingCachedModuleFileInOrg(ctx, arg)
m.queryLatencies.WithLabelValues("HasTemplateVersionsUsingCachedModuleFileInOrg").Observe(time.Since(start).Seconds())
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "HasTemplateVersionsUsingCachedModuleFileInOrg").Inc()
return r0, r1
}
func (m queryMetricsStore) HydrateAgentChatsContext(ctx context.Context, arg database.HydrateAgentChatsContextParams) error {
start := time.Now()
r0 := m.s.HydrateAgentChatsContext(ctx, arg)
+15
View File
@@ -7242,6 +7242,21 @@ func (mr *MockStoreMockRecorder) GetWorkspacesForWorkspaceMetrics(ctx any) *gomo
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspacesForWorkspaceMetrics", reflect.TypeOf((*MockStore)(nil).GetWorkspacesForWorkspaceMetrics), ctx)
}
// HasTemplateVersionsUsingCachedModuleFileInOrg mocks base method.
func (m *MockStore) HasTemplateVersionsUsingCachedModuleFileInOrg(ctx context.Context, arg database.HasTemplateVersionsUsingCachedModuleFileInOrgParams) (bool, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "HasTemplateVersionsUsingCachedModuleFileInOrg", ctx, arg)
ret0, _ := ret[0].(bool)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// HasTemplateVersionsUsingCachedModuleFileInOrg indicates an expected call of HasTemplateVersionsUsingCachedModuleFileInOrg.
func (mr *MockStoreMockRecorder) HasTemplateVersionsUsingCachedModuleFileInOrg(ctx, arg any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasTemplateVersionsUsingCachedModuleFileInOrg", reflect.TypeOf((*MockStore)(nil).HasTemplateVersionsUsingCachedModuleFileInOrg), ctx, arg)
}
// HydrateAgentChatsContext mocks base method.
func (m *MockStore) HydrateAgentChatsContext(ctx context.Context, arg database.HydrateAgentChatsContextParams) error {
m.ctrl.T.Helper()
+5
View File
@@ -975,6 +975,11 @@ type sqlcQuerier interface {
GetWorkspacesByTemplateID(ctx context.Context, templateID uuid.UUID) ([]WorkspaceTable, error)
GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]GetWorkspacesEligibleForTransitionRow, error)
GetWorkspacesForWorkspaceMetrics(ctx context.Context) ([]GetWorkspacesForWorkspaceMetricsRow, error)
// Reports whether the given file is referenced as cached module files by any
// template version in the given organization. Used to authorize provisioner
// module-file downloads so a daemon cannot read another organization's cached
// Terraform module source.
HasTemplateVersionsUsingCachedModuleFileInOrg(ctx context.Context, arg HasTemplateVersionsUsingCachedModuleFileInOrgParams) (bool, error)
// Stamps the pinned hash and error on every not-yet-hydrated chat for
// an agent (context_aggregate_hash IS NULL) and copies the agent's
// current context resources onto those chats in the same statement, so
+27
View File
@@ -27698,6 +27698,33 @@ func (q *sqlQuerier) GetTemplateVersionTerraformValues(ctx context.Context, temp
return i, err
}
const hasTemplateVersionsUsingCachedModuleFileInOrg = `-- name: HasTemplateVersionsUsingCachedModuleFileInOrg :one
SELECT EXISTS (
SELECT 1
FROM template_version_terraform_values tvtv
JOIN template_versions tv
ON tv.id = tvtv.template_version_id
WHERE tvtv.cached_module_files = $1::uuid
AND tv.organization_id = $2::uuid
)
`
type HasTemplateVersionsUsingCachedModuleFileInOrgParams struct {
FileID uuid.UUID `db:"file_id" json:"file_id"`
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
}
// Reports whether the given file is referenced as cached module files by any
// template version in the given organization. Used to authorize provisioner
// module-file downloads so a daemon cannot read another organization's cached
// Terraform module source.
func (q *sqlQuerier) HasTemplateVersionsUsingCachedModuleFileInOrg(ctx context.Context, arg HasTemplateVersionsUsingCachedModuleFileInOrgParams) (bool, error) {
row := q.db.QueryRowContext(ctx, hasTemplateVersionsUsingCachedModuleFileInOrg, arg.FileID, arg.OrganizationID)
var exists bool
err := row.Scan(&exists)
return exists, err
}
const insertTemplateVersionTerraformValuesByJobID = `-- name: InsertTemplateVersionTerraformValuesByJobID :exec
INSERT INTO
template_version_terraform_values (
@@ -23,3 +23,17 @@ VALUES
@updated_at,
@provisionerd_version
);
-- name: HasTemplateVersionsUsingCachedModuleFileInOrg :one
-- Reports whether the given file is referenced as cached module files by any
-- template version in the given organization. Used to authorize provisioner
-- module-file downloads so a daemon cannot read another organization's cached
-- Terraform module source.
SELECT EXISTS (
SELECT 1
FROM template_version_terraform_values tvtv
JOIN template_versions tv
ON tv.id = tvtv.template_version_id
WHERE tvtv.cached_module_files = @file_id::uuid
AND tv.organization_id = @organization_id::uuid
);
@@ -1584,6 +1584,26 @@ func (s *server) DownloadFile(request *proto.FileRequest, stream proto.DRPCProvi
if file.CreatedBy != uuid.Nil || file.Mimetype != tarMimeType {
return fail(xerrors.Errorf("file %s is not a modules file", fid))
}
// Ensure the requested module file belongs to a template version in
// this provisioner daemon's organization. Without this, any
// authenticated provisioner could download cached module archives
// (Terraform source) belonging to other organizations (ANT-2026-22440).
ok, err := s.Database.HasTemplateVersionsUsingCachedModuleFileInOrg(ctx, database.HasTemplateVersionsUsingCachedModuleFileInOrgParams{
FileID: fid,
OrganizationID: s.OrganizationID,
})
if err != nil {
return fail(xerrors.Errorf("authorize module file: %w", err))
}
if !ok {
s.Logger.Warn(ctx, "module file download rejected: file not referenced by any template version in daemon org",
slog.F("file_id", fid),
slog.F("organization_id", s.OrganizationID),
)
// Use the same error as the metadata check above so the handler
// does not confirm the existence of files in other organizations.
return fail(xerrors.Errorf("file %s is not a modules file", fid))
}
default:
return fail(xerrors.Errorf("unsupported file upload type: %s", request.UploadType))
}
@@ -2,6 +2,7 @@ package provisionerdserver_test
import (
"context"
crand "crypto/rand"
"database/sql"
"encoding/json"
"io"
@@ -25,6 +26,8 @@ import (
"golang.org/x/xerrors"
"google.golang.org/protobuf/types/known/timestamppb"
"storj.io/drpc"
"storj.io/drpc/drpcmux"
"storj.io/drpc/drpcserver"
"cdr.dev/slog/v3"
"cdr.dev/slog/v3/sloggers/slogtest"
@@ -52,6 +55,7 @@ import (
"github.com/coder/coder/v2/coderd/usage/usagetypes"
"github.com/coder/coder/v2/coderd/wspubsub"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/drpcsdk"
"github.com/coder/coder/v2/provisionerd/proto"
"github.com/coder/coder/v2/provisionersdk"
sdkproto "github.com/coder/coder/v2/provisionersdk/proto"
@@ -5420,3 +5424,141 @@ func newFakeUsageInserter() (*coderdtest.UsageInserter, *atomic.Pointer[usage.In
poitr.Store(&inserter)
return fake, poitr
}
// serveProvisionerDaemon serves the provisioner daemon server over an
// in-memory pipe and returns a connected client, mirroring how coderd serves
// in-memory provisioner daemons. This exercises the real DRPC streaming path
// instead of a hand-rolled mock stream.
func serveProvisionerDaemon(t *testing.T, srv proto.DRPCProvisionerDaemonServer) proto.DRPCProvisionerDaemonClient {
t.Helper()
clientPipe, serverPipe := drpcsdk.MemTransportPipe()
t.Cleanup(func() {
_ = clientPipe.Close()
_ = serverPipe.Close()
})
mux := drpcmux.New()
require.NoError(t, proto.DRPCRegisterProvisionerDaemon(mux, srv))
server := drpcserver.NewWithOptions(mux, drpcserver.Options{
Manager: drpcsdk.DefaultDRPCOptions(nil),
})
ctx, cancel := context.WithCancel(context.Background())
closed := make(chan struct{})
go func() {
defer close(closed)
_ = server.Serve(ctx, serverPipe)
}()
t.Cleanup(func() {
cancel()
<-closed
})
return proto.NewDRPCProvisionerDaemonClient(clientPipe)
}
// insertModuleFile inserts a system-created (CreatedBy=uuid.Nil) tar file and
// links it as the cached module files of a template version in the given
// organization, returning the file.
func insertModuleFile(t *testing.T, db database.Store, orgID uuid.UUID, data []byte) database.File {
t.Helper()
ctx := testutil.Context(t, testutil.WaitShort)
user := dbgen.User(t, db, database.User{})
template := dbgen.Template(t, db, database.Template{
OrganizationID: orgID,
CreatedBy: user.ID,
})
jobID := uuid.New()
version := dbgen.TemplateVersion(t, db, database.TemplateVersion{
OrganizationID: orgID,
CreatedBy: user.ID,
TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true},
JobID: jobID,
})
// Insert the file directly rather than via dbgen.File: the helper treats a
// zero CreatedBy as "unset" and replaces it with a random UUID, but module
// files must be system-created (CreatedBy=uuid.Nil) to match the handler's
// metadata check.
file, err := db.InsertFile(ctx, database.InsertFileParams{
ID: uuid.New(),
Hash: uuid.NewString(),
CreatedAt: dbtime.Now(),
CreatedBy: uuid.Nil,
Mimetype: "application/x-tar",
Data: data,
})
require.NoError(t, err)
err = db.InsertTemplateVersionTerraformValuesByJobID(ctx, database.InsertTemplateVersionTerraformValuesByJobIDParams{
JobID: version.JobID,
CachedPlan: []byte("{}"),
CachedModuleFiles: uuid.NullUUID{UUID: file.ID, Valid: true},
UpdatedAt: dbtime.Now(),
})
require.NoError(t, err)
return file
}
// TestDownloadFile verifies that a provisioner daemon cannot download cached
// module archives belonging to other organizations (ANT-2026-22440), while
// still being able to download module files from its own organization.
func TestDownloadFile(t *testing.T) {
t.Parallel()
t.Run("RejectsOtherOrgModuleFile", func(t *testing.T) {
t.Parallel()
// The server is scoped to the default organization (org A).
srv, db, _, daemon := setup(t, false, &overrides{
externalAuthConfigs: []*externalauth.Config{{}},
})
ctx := testutil.Context(t, testutil.WaitMedium)
client := serveProvisionerDaemon(t, srv)
// Create a module file belonging to a different organization (org B).
otherOrg := dbgen.Organization(t, db, database.Organization{})
require.NotEqual(t, daemon.OrganizationID, otherOrg.ID)
moduleData := make([]byte, sdkproto.ChunkSize*2)
// crand.Read never returns an error as of Go 1.24.
_, _ = crand.Read(moduleData)
file := insertModuleFile(t, db, otherOrg.ID, moduleData)
stream, err := client.DownloadFile(ctx, &proto.FileRequest{
FileId: file.ID.String(),
UploadType: sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES,
})
require.NoError(t, err)
// The handler must reject the cross-org download with an error rather
// than streaming the file's contents.
_, err = provisionersdk.HandleReceivingDataUpload(stream)
require.Error(t, err)
require.ErrorContains(t, err, "is not a modules file")
})
t.Run("AllowsSameOrgModuleFile", func(t *testing.T) {
t.Parallel()
// The server is scoped to the default organization (org A).
srv, db, _, daemon := setup(t, false, &overrides{
externalAuthConfigs: []*externalauth.Config{{}},
})
ctx := testutil.Context(t, testutil.WaitMedium)
client := serveProvisionerDaemon(t, srv)
moduleData := make([]byte, sdkproto.ChunkSize*2+512)
// crand.Read never returns an error as of Go 1.24.
_, _ = crand.Read(moduleData)
file := insertModuleFile(t, db, daemon.OrganizationID, moduleData)
stream, err := client.DownloadFile(ctx, &proto.FileRequest{
FileId: file.ID.String(),
UploadType: sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES,
})
require.NoError(t, err)
builder, err := provisionersdk.HandleReceivingDataUpload(stream)
require.NoError(t, err)
data, err := builder.Complete()
require.NoError(t, err)
require.Equal(t, moduleData, data)
})
}