Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4e57b9f

Browse files
authored
fix: allow regular users to push files (#4500)
- As part of merging support for Template RBAC and user groups a permission check on reading files was relaxed. With the addition of admin roles on individual templates, regular users are now able to push template versions if they have inherited the 'admin' role for a template. In order to do so they need to be able to create and read their own files. Since collisions on hash in the past were ignored, this means that a regular user who pushes a template version with a file hash that collides with an existing hash will not be able to read the file (since it belongs to another user). This commit fixes the underlying problem which was that the files table had a primary key on the 'hash' column. This was not a problem at the time because only template admins and other users with similar elevated roles were able to read all files regardless of ownership. To fix this a new column and primary key 'id' has been introduced to the files table. The unique constraint has been updated to be hash+created_by. Tables (provisioner_jobs) that referenced files.hash have been updated to reference files.id. Relevant API endpoints have also been updated.
1 parenta55186c commit4e57b9f

33 files changed

+265
-94
lines changed

‎cli/templatecreate.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"unicode/utf8"
1111

1212
"github.com/briandowns/spinner"
13+
"github.com/google/uuid"
1314
"github.com/spf13/cobra"
1415
"golang.org/x/xerrors"
1516

@@ -91,7 +92,7 @@ func templateCreate() *cobra.Command {
9192
Client:client,
9293
Organization:organization,
9394
Provisioner:database.ProvisionerType(provisioner),
94-
FileHash:resp.Hash,
95+
FileID:resp.ID,
9596
ParameterFile:parameterFile,
9697
})
9798
iferr!=nil {
@@ -148,7 +149,7 @@ type createValidTemplateVersionArgs struct {
148149
Client*codersdk.Client
149150
Organization codersdk.Organization
150151
Provisioner database.ProvisionerType
151-
FileHashstring
152+
FileID uuid.UUID
152153
ParameterFilestring
153154
// Template is only required if updating a template's active version.
154155
Template*codersdk.Template
@@ -165,7 +166,7 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers
165166
req:= codersdk.CreateTemplateVersionRequest{
166167
Name:args.Name,
167168
StorageMethod:codersdk.ProvisionerStorageMethodFile,
168-
StorageSource:args.FileHash,
169+
FileID:args.FileID,
169170
Provisioner:codersdk.ProvisionerType(args.Provisioner),
170171
ParameterValues:parameters,
171172
}

‎cli/templatepull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func templatePull() *cobra.Command {
6666
latest:=versions[0]
6767

6868
// Download the tar archive.
69-
raw,ctype,err:=client.Download(ctx,latest.Job.StorageSource)
69+
raw,ctype,err:=client.Download(ctx,latest.Job.FileID)
7070
iferr!=nil {
7171
returnxerrors.Errorf("download template: %w",err)
7272
}

‎cli/templatepush.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func templatePush() *cobra.Command {
8080
Client:client,
8181
Organization:organization,
8282
Provisioner:database.ProvisionerType(provisioner),
83-
FileHash:resp.Hash,
83+
FileID:resp.ID,
8484
ParameterFile:parameterFile,
8585
Template:&template,
8686
ReuseParameters:!alwaysPrompt,

‎coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa
276276
Provisioner:template.Provisioner,
277277
Type:database.ProvisionerJobTypeWorkspaceBuild,
278278
StorageMethod:priorJob.StorageMethod,
279-
StorageSource:priorJob.StorageSource,
279+
FileID:priorJob.FileID,
280280
Input:input,
281281
})
282282
iferr!=nil {

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func New(options *Options) *API {
280280
// file content is expensive so it should be small.
281281
httpmw.RateLimitPerMinute(12),
282282
)
283-
r.Get("/{hash}",api.fileByHash)
283+
r.Get("/{fileID}",api.fileByID)
284284
r.Post("/",api.postFile)
285285
})
286286

‎coderd/coderdtest/authorize.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
142142
AssertObject:rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
143143
},
144144
"POST:/api/v2/files": {AssertAction:rbac.ActionCreate,AssertObject:rbac.ResourceFile},
145-
"GET:/api/v2/files/{hash}": {
145+
"GET:/api/v2/files/{fileID}": {
146146
AssertAction:rbac.ActionRead,
147147
AssertObject:rbac.ResourceFile.WithOwner(a.Admin.UserID.String()),
148148
},
@@ -369,7 +369,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a
369369
"{workspaceagent}":workspace.LatestBuild.Resources[0].Agents[0].ID.String(),
370370
"{buildnumber}":strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber),10),
371371
"{template}":template.ID.String(),
372-
"{hash}":file.Hash,
372+
"{fileID}":file.ID.String(),
373373
"{workspaceresource}":workspace.LatestBuild.Resources[0].ID.String(),
374374
"{workspaceapp}":workspace.LatestBuild.Resources[0].Agents[0].Apps[0].Name,
375375
"{templateversion}":version.ID.String(),

‎coderd/coderdtest/coderdtest.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ func CreateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID
383383
file,err:=client.Upload(context.Background(),codersdk.ContentTypeTar,data)
384384
require.NoError(t,err)
385385
templateVersion,err:=client.CreateTemplateVersion(context.Background(),organizationID, codersdk.CreateTemplateVersionRequest{
386-
StorageSource:file.Hash,
386+
FileID:file.ID,
387387
StorageMethod:codersdk.ProvisionerStorageMethodFile,
388388
Provisioner:codersdk.ProvisionerTypeEcho,
389389
})
@@ -431,7 +431,7 @@ func UpdateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID
431431
require.NoError(t,err)
432432
templateVersion,err:=client.CreateTemplateVersion(context.Background(),organizationID, codersdk.CreateTemplateVersionRequest{
433433
TemplateID:templateID,
434-
StorageSource:file.Hash,
434+
FileID:file.ID,
435435
StorageMethod:codersdk.ProvisionerStorageMethodFile,
436436
Provisioner:codersdk.ProvisionerTypeEcho,
437437
})

‎coderd/database/databasefake/databasefake.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,12 +316,24 @@ func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error {
316316
returnsql.ErrNoRows
317317
}
318318

319-
func (q*fakeQuerier)GetFileByHash(_ context.Context,hashstring) (database.File,error) {
319+
func (q*fakeQuerier)GetFileByHashAndCreator(_ context.Context,arg database.GetFileByHashAndCreatorParams) (database.File,error) {
320320
q.mutex.RLock()
321321
deferq.mutex.RUnlock()
322322

323323
for_,file:=rangeq.files {
324-
iffile.Hash==hash {
324+
iffile.Hash==arg.Hash&&file.CreatedBy==arg.CreatedBy {
325+
returnfile,nil
326+
}
327+
}
328+
return database.File{},sql.ErrNoRows
329+
}
330+
331+
func (q*fakeQuerier)GetFileByID(_ context.Context,id uuid.UUID) (database.File,error) {
332+
q.mutex.RLock()
333+
deferq.mutex.RUnlock()
334+
335+
for_,file:=rangeq.files {
336+
iffile.ID==id {
325337
returnfile,nil
326338
}
327339
}
@@ -1901,6 +1913,7 @@ func (q *fakeQuerier) InsertFile(_ context.Context, arg database.InsertFileParam
19011913

19021914
//nolint:gosimple
19031915
file:= database.File{
1916+
ID:arg.ID,
19041917
Hash:arg.Hash,
19051918
CreatedAt:arg.CreatedAt,
19061919
CreatedBy:arg.CreatedBy,
@@ -2085,7 +2098,7 @@ func (q *fakeQuerier) InsertProvisionerJob(_ context.Context, arg database.Inser
20852098
InitiatorID:arg.InitiatorID,
20862099
Provisioner:arg.Provisioner,
20872100
StorageMethod:arg.StorageMethod,
2088-
StorageSource:arg.StorageSource,
2101+
FileID:arg.FileID,
20892102
Type:arg.Type,
20902103
Input:arg.Input,
20912104
}

‎coderd/database/dump.sql

Lines changed: 8 additions & 4 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
BEGIN;
2+
3+
-- Add back the storage_source column. This must be nullable temporarily.
4+
ALTERTABLE provisioner_jobs ADD COLUMN storage_sourcetext;
5+
6+
-- Set the storage_source to the hash of the files.id reference.
7+
UPDATE
8+
provisioner_jobs
9+
SET
10+
storage_source=files.hash
11+
FROM
12+
files
13+
WHERE
14+
provisioner_jobs.file_id=files.id;
15+
16+
-- Now that we've populated storage_source drop the file_id column.
17+
ALTERTABLE provisioner_jobs DROP COLUMN file_id;
18+
-- We can set the storage_source column as NOT NULL now.
19+
ALTERTABLE provisioner_jobs ALTER COLUMN storage_sourceSETNOT NULL;
20+
21+
-- Delete all the duplicate rows where hashes collide.
22+
-- We filter on 'id' to ensure only 1 unique row.
23+
DELETEFROM
24+
files a
25+
USING
26+
files b
27+
WHERE
28+
a.created_by<b.created_by
29+
AND
30+
a.hash=b.hash;
31+
32+
-- Drop the primary key on files.id.
33+
ALTERTABLE files DROPCONSTRAINT files_pkey;
34+
-- Drop the id column.
35+
ALTERTABLE files DROP COLUMN id;
36+
-- Drop the unique constraint on hash + owner.
37+
ALTERTABLE files DROPCONSTRAINT files_hash_created_by_key;
38+
-- Set the primary key back to hash.
39+
ALTERTABLE files ADDPRIMARY KEY (hash);
40+
41+
COMMIT;
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
-- This migration updates the files table to move the unique
2+
-- constraint to be hash + created_by. This is necessary to
3+
-- allow regular users who have been granted admin to a specific
4+
-- template to be able to push and read files used for template
5+
-- versions they create.
6+
-- Prior to this collisions on file.hash were not an issue
7+
-- since users who could push files could also read all files.
8+
--
9+
-- This migration also adds a 'files.id' column as the primary
10+
-- key. As a side effect the provisioner_jobs must now reference
11+
-- the files.id column since the 'hash' column is now ambiguous.
12+
BEGIN;
13+
14+
-- Drop the primary key on hash.
15+
ALTERTABLE files DROPCONSTRAINT files_pkey;
16+
17+
-- Add an 'id' column and designate it the primary key.
18+
ALTERTABLE files ADD COLUMN
19+
id uuidNOT NULLPRIMARY KEY DEFAULT gen_random_uuid ();
20+
21+
-- Update the constraint to include the user who created it.
22+
ALTERTABLE files ADD UNIQUE(hash, created_by);
23+
24+
-- Update provisioner_jobs to include a file_id column.
25+
-- This must be temporarily nullable.
26+
ALTERTABLE provisioner_jobs ADD COLUMN file_id uuid;
27+
28+
-- Update all the rows to point to key in the files table.
29+
UPDATE provisioner_jobs
30+
SET
31+
file_id=files.id
32+
FROM
33+
files
34+
WHERE
35+
provisioner_jobs.storage_source=files.hash;
36+
37+
-- Enforce NOT NULL on file_id now.
38+
ALTERTABLE provisioner_jobs ALTER COLUMN file_idSETNOT NULL;
39+
-- Drop storage_source since it is no longer useful for anything.
40+
ALTERTABLE provisioner_jobs DROP COLUMN storage_source;
41+
42+
COMMIT;

‎coderd/database/models.go

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp