Skip to content

fix(coderd): add strict org ID joins for provisioner job metadata #16588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 17, 2025
Merged

fix(coderd): add strict org ID joins for provisioner job metadata #16588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b30914
fix(coderd): add strict org ID joins for provisioner job metadata
mafredri Feb 17, 2025
f1d5cda
overkill?
mafredri Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions coderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3354,11 +3354,11 @@ func (s *MethodTestSuite) TestExtraMethods() {
dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{ID: wbID, WorkspaceID: w.ID, TemplateVersionID: tv.ID, JobID: j2.ID})

ds, err := db.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(context.Background(), database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
OrganizationID: uuid.NullUUID{Valid: true, UUID: org.ID},
OrganizationID: org.ID,
})
s.NoError(err, "get provisioner jobs by org")
check.Args(database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
OrganizationID: uuid.NullUUID{Valid: true, UUID: org.ID},
OrganizationID: org.ID,
}).Asserts(j1, policy.ActionRead, j2, policy.ActionRead).Returns(ds)
}))
}
Expand Down
2 changes: 1 addition & 1 deletion coderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4221,7 +4221,7 @@ func (q *FakeQuerier) GetProvisionerJobsByOrganizationAndStatusWithQueuePosition
for _, rowQP := range rowsWithQueuePosition {
job := rowQP.ProvisionerJob

if arg.OrganizationID.Valid && job.OrganizationID != arg.OrganizationID.UUID {
if job.OrganizationID != arg.OrganizationID {
continue
}
if len(arg.Status) > 0 && !slices.Contains(arg.Status, job.JobStatus) {
Expand Down
42 changes: 33 additions & 9 deletions coderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 19 additions & 4 deletions coderd/database/queries/provisionerdaemons.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ JOIN
LEFT JOIN
provisioner_jobs current_job ON (
current_job.worker_id = pd.id
AND current_job.organization_id = pd.organization_id
AND current_job.completed_at IS NULL
)
LEFT JOIN
Expand All @@ -69,28 +70,42 @@ LEFT JOIN
provisioner_jobs
WHERE
worker_id = pd.id
AND organization_id = pd.organization_id
AND completed_at IS NOT NULL
ORDER BY
completed_at DESC
LIMIT 1
)
AND previous_job.organization_id = pd.organization_id
)
-- Current job information.
LEFT JOIN
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
LEFT JOIN
-- We should always have a template version, either explicitly or implicitly via workspace build.
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
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
AND current_version.organization_id = pd.organization_id
)
LEFT JOIN
templates current_template ON current_template.id = current_version.template_id
templates current_template ON (
current_template.id = current_version.template_id
AND current_template.organization_id = pd.organization_id
)
-- Previous job information.
LEFT JOIN
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
LEFT JOIN
-- We should always have a template version, either explicitly or implicitly via workspace build.
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
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
AND previous_version.organization_id = pd.organization_id
)
LEFT JOIN
templates previous_template ON previous_template.id = previous_version.template_id
templates previous_template ON (
previous_template.id = previous_version.template_id
AND previous_template.organization_id = pd.organization_id
)
WHERE
pd.organization_id = @organization_id::uuid
AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pd.id = ANY(@ids::uuid[]))
Expand Down
17 changes: 13 additions & 4 deletions coderd/database/queries/provisionerjobs.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,23 @@ LEFT JOIN
LEFT JOIN
workspace_builds wb ON wb.id = CASE WHEN pj.input ? 'workspace_build_id' THEN (pj.input->>'workspace_build_id')::uuid END
LEFT JOIN
workspaces w ON wb.workspace_id = w.id
workspaces w ON (
w.id = wb.workspace_id
AND w.organization_id = pj.organization_id
)
LEFT JOIN
-- We should always have a template version, either explicitly or implicitly via workspace build.
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
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
AND tv.organization_id = pj.organization_id
)
LEFT JOIN
templates t ON tv.template_id = t.id
templates t ON (
t.id = tv.template_id
AND t.organization_id = pj.organization_id
)
WHERE
(sqlc.narg('organization_id')::uuid IS NULL OR pj.organization_id = @organization_id)
pj.organization_id = @organization_id::uuid
AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pj.id = ANY(@ids::uuid[]))
AND (COALESCE(array_length(@status::provisioner_job_status[], 1), 0) = 0 OR pj.job_status = ANY(@status::provisioner_job_status[]))
AND (@tags::tagset = 'null'::tagset OR provisioner_tagset_contains(pj.tags::tagset, @tags::tagset))
Expand Down
2 changes: 1 addition & 1 deletion coderd/provisionerjobs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (api *API) handleAuthAndFetchProvisionerJobs(rw http.ResponseWriter, r *htt
}

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