🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

Commit d6ea2bd

Browse files
jdomeracki-codersreya
jdomeracki-coder
and
sreya
authored
fix(coderd): scope provisioner module file downloads to the daemon's org (#26635) (#27883)
Backport of #26635 Original PR: #26635 — fix(coderd): scope provisioner module file downloads to the daemon's org Merge commit: 1961908 Requested by: @jdomeracki-coder <details> <summary>Conflict resolution notes</summary> The cherry-pick conflicted in generated database files because the new `HasTemplateVersionsUsingCachedModuleFileInOrg` query sits adjacent to chatd methods (`HydrateAgentChatsContext`, `IncrementChatGenerationAttempt`) that do not exist on `release/2.34`. Resolved by keeping only the new query and its generated bindings in `querier.go`, `dbmetrics/querymetrics.go`, `dbmock/dbmock.go`, and `dbauthz/dbauthz.go`, then running `gofmt`. Affected packages (`./coderd/database/...`, `./coderd/provisionerdserver/...`) build successfully. </details> --- _Opened by Coder Agents on behalf of @jdomeracki-coder._ --------- Co-authored-by: Jon Ayers <jon@coder.com>
1 parent d70055a commit d6ea2bd

9 files changed

Lines changed: 246 additions & 0 deletions

File tree

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5386,6 +5386,16 @@ func (q *querier) GetWorkspacesForWorkspaceMetrics(ctx context.Context) ([]datab
53865386
return q.db.GetWorkspacesForWorkspaceMetrics(ctx)
53875387
}
53885388

5389+
func (q *querier) HasTemplateVersionsUsingCachedModuleFileInOrg(ctx context.Context, arg database.HasTemplateVersionsUsingCachedModuleFileInOrgParams) (bool, error) {
5390+
// This query authorizes provisioner module-file downloads. The caller
5391+
// must be able to read files in the target organization; the actual
5392+
// tenant isolation comes from the organization_id filter in the query.
5393+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceFile.InOrg(arg.OrganizationID)); err != nil {
5394+
return false, err
5395+
}
5396+
return q.db.HasTemplateVersionsUsingCachedModuleFileInOrg(ctx, arg)
5397+
}
5398+
53895399
func (q *querier) InsertAIBridgeInterception(ctx context.Context, arg database.InsertAIBridgeInterceptionParams) (database.AIBridgeInterception, error) {
53905400
return insert(q.log, q.auth, rbac.ResourceAibridgeInterception.WithOwner(arg.InitiatorID.String()), q.db.InsertAIBridgeInterception)(ctx, arg)
53915401
}

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,6 +2442,11 @@ func (s *MethodTestSuite) TestTemplate() {
24422442
dbm.EXPECT().GetTemplateVersionTerraformValues(gomock.Any(), tv.ID).Return(val, nil).AnyTimes()
24432443
check.Args(tv.ID).Asserts(t, policy.ActionRead)
24442444
}))
2445+
s.Run("HasTemplateVersionsUsingCachedModuleFileInOrg", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
2446+
arg := database.HasTemplateVersionsUsingCachedModuleFileInOrgParams{FileID: uuid.New(), OrganizationID: uuid.New()}
2447+
dbm.EXPECT().HasTemplateVersionsUsingCachedModuleFileInOrg(gomock.Any(), arg).Return(true, nil).AnyTimes()
2448+
check.Args(arg).Asserts(rbac.ResourceFile.InOrg(arg.OrganizationID), policy.ActionRead).Returns(true)
2449+
}))
24452450
s.Run("GetTemplateVersionVariables", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
24462451
t1 := testutil.Fake(s.T(), faker, database.Template{})
24472452
tv := testutil.Fake(s.T(), faker, database.TemplateVersion{TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}})

‎coderd/database/dbmetrics/querymetrics.go‎

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go‎

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/templateversionterraformvalues.sql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,17 @@ VALUES
2323
@updated_at,
2424
@provisionerd_version
2525
);
26+
27+
-- name: HasTemplateVersionsUsingCachedModuleFileInOrg :one
28+
-- Reports whether the given file is referenced as cached module files by any
29+
-- template version in the given organization. Used to authorize provisioner
30+
-- module-file downloads so a daemon cannot read another organization's cached
31+
-- Terraform module source.
32+
SELECT EXISTS (
33+
SELECT 1
34+
FROM template_version_terraform_values tvtv
35+
JOIN template_versions tv
36+
ON tv.id = tvtv.template_version_id
37+
WHERE tvtv.cached_module_files = @file_id::uuid
38+
AND tv.organization_id = @organization_id::uuid
39+
);

‎coderd/provisionerdserver/provisionerdserver.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,6 +1584,26 @@ func (s *server) DownloadFile(request *proto.FileRequest, stream proto.DRPCProvi
15841584
if file.CreatedBy != uuid.Nil || file.Mimetype != tarMimeType {
15851585
return fail(xerrors.Errorf("file %s is not a modules file", fid))
15861586
}
1587+
// Ensure the requested module file belongs to a template version in
1588+
// this provisioner daemon's organization. Without this, any
1589+
// authenticated provisioner could download cached module archives
1590+
// (Terraform source) belonging to other organizations (ANT-2026-22440).
1591+
ok, err := s.Database.HasTemplateVersionsUsingCachedModuleFileInOrg(ctx, database.HasTemplateVersionsUsingCachedModuleFileInOrgParams{
1592+
FileID: fid,
1593+
OrganizationID: s.OrganizationID,
1594+
})
1595+
if err != nil {
1596+
return fail(xerrors.Errorf("authorize module file: %w", err))
1597+
}
1598+
if !ok {
1599+
s.Logger.Warn(ctx, "module file download rejected: file not referenced by any template version in daemon org",
1600+
slog.F("file_id", fid),
1601+
slog.F("organization_id", s.OrganizationID),
1602+
)
1603+
// Use the same error as the metadata check above so the handler
1604+
// does not confirm the existence of files in other organizations.
1605+
return fail(xerrors.Errorf("file %s is not a modules file", fid))
1606+
}
15871607
default:
15881608
return fail(xerrors.Errorf("unsupported file upload type: %s", request.UploadType))
15891609
}

‎coderd/provisionerdserver/provisionerdserver_test.go‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package provisionerdserver_test
22

33
import (
44
"context"
5+
crand "crypto/rand"
56
"database/sql"
67
"encoding/json"
78
"io"
@@ -25,6 +26,8 @@ import (
2526
"golang.org/x/xerrors"
2627
"google.golang.org/protobuf/types/known/timestamppb"
2728
"storj.io/drpc"
29+
"storj.io/drpc/drpcmux"
30+
"storj.io/drpc/drpcserver"
2831

2932
"cdr.dev/slog/v3"
3033
"cdr.dev/slog/v3/sloggers/slogtest"
@@ -52,6 +55,7 @@ import (
5255
"github.com/coder/coder/v2/coderd/usage/usagetypes"
5356
"github.com/coder/coder/v2/coderd/wspubsub"
5457
"github.com/coder/coder/v2/codersdk"
58+
"github.com/coder/coder/v2/codersdk/drpcsdk"
5559
"github.com/coder/coder/v2/provisionerd/proto"
5660
"github.com/coder/coder/v2/provisionersdk"
5761
sdkproto "github.com/coder/coder/v2/provisionersdk/proto"
@@ -5420,3 +5424,141 @@ func newFakeUsageInserter() (*coderdtest.UsageInserter, *atomic.Pointer[usage.In
54205424
poitr.Store(&inserter)
54215425
return fake, poitr
54225426
}
5427+
5428+
// serveProvisionerDaemon serves the provisioner daemon server over an
5429+
// in-memory pipe and returns a connected client, mirroring how coderd serves
5430+
// in-memory provisioner daemons. This exercises the real DRPC streaming path
5431+
// instead of a hand-rolled mock stream.
5432+
func serveProvisionerDaemon(t *testing.T, srv proto.DRPCProvisionerDaemonServer) proto.DRPCProvisionerDaemonClient {
5433+
t.Helper()
5434+
clientPipe, serverPipe := drpcsdk.MemTransportPipe()
5435+
t.Cleanup(func() {
5436+
_ = clientPipe.Close()
5437+
_ = serverPipe.Close()
5438+
})
5439+
mux := drpcmux.New()
5440+
require.NoError(t, proto.DRPCRegisterProvisionerDaemon(mux, srv))
5441+
server := drpcserver.NewWithOptions(mux, drpcserver.Options{
5442+
Manager: drpcsdk.DefaultDRPCOptions(nil),
5443+
})
5444+
ctx, cancel := context.WithCancel(context.Background())
5445+
closed := make(chan struct{})
5446+
go func() {
5447+
defer close(closed)
5448+
_ = server.Serve(ctx, serverPipe)
5449+
}()
5450+
t.Cleanup(func() {
5451+
cancel()
5452+
<-closed
5453+
})
5454+
return proto.NewDRPCProvisionerDaemonClient(clientPipe)
5455+
}
5456+
5457+
// insertModuleFile inserts a system-created (CreatedBy=uuid.Nil) tar file and
5458+
// links it as the cached module files of a template version in the given
5459+
// organization, returning the file.
5460+
func insertModuleFile(t *testing.T, db database.Store, orgID uuid.UUID, data []byte) database.File {
5461+
t.Helper()
5462+
ctx := testutil.Context(t, testutil.WaitShort)
5463+
5464+
user := dbgen.User(t, db, database.User{})
5465+
template := dbgen.Template(t, db, database.Template{
5466+
OrganizationID: orgID,
5467+
CreatedBy: user.ID,
5468+
})
5469+
jobID := uuid.New()
5470+
version := dbgen.TemplateVersion(t, db, database.TemplateVersion{
5471+
OrganizationID: orgID,
5472+
CreatedBy: user.ID,
5473+
TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true},
5474+
JobID: jobID,
5475+
})
5476+
// Insert the file directly rather than via dbgen.File: the helper treats a
5477+
// zero CreatedBy as "unset" and replaces it with a random UUID, but module
5478+
// files must be system-created (CreatedBy=uuid.Nil) to match the handler's
5479+
// metadata check.
5480+
file, err := db.InsertFile(ctx, database.InsertFileParams{
5481+
ID: uuid.New(),
5482+
Hash: uuid.NewString(),
5483+
CreatedAt: dbtime.Now(),
5484+
CreatedBy: uuid.Nil,
5485+
Mimetype: "application/x-tar",
5486+
Data: data,
5487+
})
5488+
require.NoError(t, err)
5489+
err = db.InsertTemplateVersionTerraformValuesByJobID(ctx, database.InsertTemplateVersionTerraformValuesByJobIDParams{
5490+
JobID: version.JobID,
5491+
CachedPlan: []byte("{}"),
5492+
CachedModuleFiles: uuid.NullUUID{UUID: file.ID, Valid: true},
5493+
UpdatedAt: dbtime.Now(),
5494+
})
5495+
require.NoError(t, err)
5496+
return file
5497+
}
5498+
5499+
// TestDownloadFile verifies that a provisioner daemon cannot download cached
5500+
// module archives belonging to other organizations (ANT-2026-22440), while
5501+
// still being able to download module files from its own organization.
5502+
func TestDownloadFile(t *testing.T) {
5503+
t.Parallel()
5504+
5505+
t.Run("RejectsOtherOrgModuleFile", func(t *testing.T) {
5506+
t.Parallel()
5507+
5508+
// The server is scoped to the default organization (org A).
5509+
srv, db, _, daemon := setup(t, false, &overrides{
5510+
externalAuthConfigs: []*externalauth.Config{{}},
5511+
})
5512+
ctx := testutil.Context(t, testutil.WaitMedium)
5513+
client := serveProvisionerDaemon(t, srv)
5514+
5515+
// Create a module file belonging to a different organization (org B).
5516+
otherOrg := dbgen.Organization(t, db, database.Organization{})
5517+
require.NotEqual(t, daemon.OrganizationID, otherOrg.ID)
5518+
5519+
moduleData := make([]byte, sdkproto.ChunkSize*2)
5520+
// crand.Read never returns an error as of Go 1.24.
5521+
_, _ = crand.Read(moduleData)
5522+
file := insertModuleFile(t, db, otherOrg.ID, moduleData)
5523+
5524+
stream, err := client.DownloadFile(ctx, &proto.FileRequest{
5525+
FileId: file.ID.String(),
5526+
UploadType: sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES,
5527+
})
5528+
require.NoError(t, err)
5529+
5530+
// The handler must reject the cross-org download with an error rather
5531+
// than streaming the file's contents.
5532+
_, err = provisionersdk.HandleReceivingDataUpload(stream)
5533+
require.Error(t, err)
5534+
require.ErrorContains(t, err, "is not a modules file")
5535+
})
5536+
5537+
t.Run("AllowsSameOrgModuleFile", func(t *testing.T) {
5538+
t.Parallel()
5539+
5540+
// The server is scoped to the default organization (org A).
5541+
srv, db, _, daemon := setup(t, false, &overrides{
5542+
externalAuthConfigs: []*externalauth.Config{{}},
5543+
})
5544+
ctx := testutil.Context(t, testutil.WaitMedium)
5545+
client := serveProvisionerDaemon(t, srv)
5546+
5547+
moduleData := make([]byte, sdkproto.ChunkSize*2+512)
5548+
// crand.Read never returns an error as of Go 1.24.
5549+
_, _ = crand.Read(moduleData)
5550+
file := insertModuleFile(t, db, daemon.OrganizationID, moduleData)
5551+
5552+
stream, err := client.DownloadFile(ctx, &proto.FileRequest{
5553+
FileId: file.ID.String(),
5554+
UploadType: sdkproto.DataUploadType_UPLOAD_TYPE_MODULE_FILES,
5555+
})
5556+
require.NoError(t, err)
5557+
5558+
builder, err := provisionersdk.HandleReceivingDataUpload(stream)
5559+
require.NoError(t, err)
5560+
data, err := builder.Complete()
5561+
require.NoError(t, err)
5562+
require.Equal(t, moduleData, data)
5563+
})
5564+
}

0 commit comments

Comments
 (0)