Skip to content

Commit 2b30914

Browse files
mafredrimafredri
committed
fix(coderd): add strict org ID joins for provisioner job metadata
References #16558
1 parent 46e04c6 commit 2b30914

File tree

6 files changed

+63
-21
lines changed

6 files changed

+63
-21
lines changed

‎coderd/database/dbauthz/dbauthz_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -3354,11 +3354,11 @@ func (s *MethodTestSuite) TestExtraMethods() {
33543354
dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{ID: wbID, WorkspaceID: w.ID, TemplateVersionID: tv.ID, JobID: j2.ID})
33553355

33563356
ds, err := db.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(context.Background(), database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
3357-
OrganizationID: uuid.NullUUID{Valid: true, UUID: org.ID},
3357+
OrganizationID: org.ID,
33583358
})
33593359
s.NoError(err, "get provisioner jobs by org")
33603360
check.Args(database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
3361-
OrganizationID: uuid.NullUUID{Valid: true, UUID: org.ID},
3361+
OrganizationID: org.ID,
33623362
}).Asserts(j1, policy.ActionRead, j2, policy.ActionRead).Returns(ds)
33633363
}))
33643364
}

‎coderd/database/dbmem/dbmem.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -4221,7 +4221,7 @@ func (q *FakeQuerier) GetProvisionerJobsByOrganizationAndStatusWithQueuePosition
42214221
for _, rowQP := range rowsWithQueuePosition {
42224222
job := rowQP.ProvisionerJob
42234223

4224-
if arg.OrganizationID.Valid && job.OrganizationID != arg.OrganizationID.UUID {
4224+
if job.OrganizationID != arg.OrganizationID {
42254225
continue
42264226
}
42274227
if len(arg.Status) > 0 && !slices.Contains(arg.Status, job.JobStatus) {

‎coderd/database/queries.sql.go

+30-9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/provisionerdaemons.sql

+16-4
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,29 @@ LEFT JOIN
8080
workspace_builds current_build ON current_build.id = CASE WHEN current_job.input ? 'workspace_build_id' THEN (current_job.input->>'workspace_build_id')::uuid END
8181
LEFT JOIN
8282
-- We should always have a template version, either explicitly or implicitly via workspace build.
83-
template_versions current_version ON current_version.id = CASE WHEN current_job.input ? 'template_version_id' THEN (current_job.input->>'template_version_id')::uuid ELSE current_build.template_version_id END
83+
template_versions current_version ON (
84+
current_version.id = CASE WHEN current_job.input ? 'template_version_id' THEN (current_job.input->>'template_version_id')::uuid ELSE current_build.template_version_id END
85+
AND current_version.organization_id = pd.organization_id
86+
)
8487
LEFT JOIN
85-
templates current_template ON current_template.id = current_version.template_id
88+
templates current_template ON (
89+
current_template.id = current_version.template_id
90+
AND current_template.organization_id = pd.organization_id
91+
)
8692
-- Previous job information.
8793
LEFT JOIN
8894
workspace_builds previous_build ON previous_build.id = CASE WHEN previous_job.input ? 'workspace_build_id' THEN (previous_job.input->>'workspace_build_id')::uuid END
8995
LEFT JOIN
9096
-- We should always have a template version, either explicitly or implicitly via workspace build.
91-
template_versions previous_version ON previous_version.id = CASE WHEN previous_job.input ? 'template_version_id' THEN (previous_job.input->>'template_version_id')::uuid ELSE previous_build.template_version_id END
97+
template_versions previous_version ON (
98+
previous_version.id = CASE WHEN previous_job.input ? 'template_version_id' THEN (previous_job.input->>'template_version_id')::uuid ELSE previous_build.template_version_id END
99+
AND previous_version.organization_id = pd.organization_id
100+
)
92101
LEFT JOIN
93-
templates previous_template ON previous_template.id = previous_version.template_id
102+
templates previous_template ON (
103+
previous_template.id = previous_version.template_id
104+
AND previous_template.organization_id = pd.organization_id
105+
)
94106
WHERE
95107
pd.organization_id = @organization_id::uuid
96108
AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pd.id = ANY(@ids::uuid[]))

‎coderd/database/queries/provisionerjobs.sql

+13-4
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,23 @@ LEFT JOIN
148148
LEFT JOIN
149149
workspace_builds wb ON wb.id = CASE WHEN pj.input ? 'workspace_build_id' THEN (pj.input->>'workspace_build_id')::uuid END
150150
LEFT JOIN
151-
workspaces w ON wb.workspace_id = w.id
151+
workspaces w ON (
152+
w.id = wb.workspace_id
153+
AND w.organization_id = pj.organization_id
154+
)
152155
LEFT JOIN
153156
-- We should always have a template version, either explicitly or implicitly via workspace build.
154-
template_versions tv ON tv.id = CASE WHEN pj.input ? 'template_version_id' THEN (pj.input->>'template_version_id')::uuid ELSE wb.template_version_id END
157+
template_versions tv ON (
158+
tv.id = CASE WHEN pj.input ? 'template_version_id' THEN (pj.input->>'template_version_id')::uuid ELSE wb.template_version_id END
159+
AND tv.organization_id = pj.organization_id
160+
)
155161
LEFT JOIN
156-
templates t ON tv.template_id = t.id
162+
templates t ON (
163+
t.id = tv.template_id
164+
AND t.organization_id = pj.organization_id
165+
)
157166
WHERE
158-
(sqlc.narg('organization_id')::uuid IS NULL OR pj.organization_id = @organization_id)
167+
pj.organization_id = @organization_id::uuid
159168
AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pj.id = ANY(@ids::uuid[]))
160169
AND (COALESCE(array_length(@status::provisioner_job_status[], 1), 0) = 0 OR pj.job_status = ANY(@status::provisioner_job_status[]))
161170
AND (@tags::tagset = 'null'::tagset OR provisioner_tagset_contains(pj.tags::tagset, @tags::tagset))

‎coderd/provisionerjobs.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (api *API) handleAuthAndFetchProvisionerJobs(rw http.ResponseWriter, r *htt
130130
}
131131

132132
jobs, err := api.Database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx, database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
133-
OrganizationID: uuid.NullUUID{UUID: org.ID, Valid: true},
133+
OrganizationID: org.ID,
134134
Status: slice.StringEnums[database.ProvisionerJobStatus](status),
135135
Limit: sql.NullInt32{Int32: limit, Valid: limit > 0},
136136
IDs: ids,

0 commit comments

Comments
 (0)