Skip to content

fix(security): #113 — bind scheduleId to workspace (IDOR) - #124

Closed
HongmingWang-Rabbit wants to merge 1 commit into
mainfrom
fix/issue-113-schedules-idor
Closed

fix(security): #113 — bind scheduleId to workspace (IDOR)#124
HongmingWang-Rabbit wants to merge 1 commit into
mainfrom
fix/issue-113-schedules-idor

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Closes #113 (HIGH severity — cross-workspace IDOR).

Root cause

`Update` and `Delete` on `workspace_schedules` filtered by `scheduleId` only. A member of workspace A with a cached scheduleId from workspace B could mutate or delete that foreign row — the `wsAuth` middleware only gates the :id path parameter, not the sub-resource.

Fix

Bind both :id and :scheduleId in every SQL that touches a schedule:

  • Pre-update SELECT: `WHERE id = $1 AND workspace_id = $2`
  • UPDATE: `WHERE id = $1 AND workspace_id = $2`
  • DELETE: `WHERE id = $1 AND workspace_id = $2`

`RunNow` and `History` already had the correct two-key filter — no changes.

Test plan

  • 2 new sqlmock tests: `TestScheduleUpdate_RejectsCrossWorkspaceIDOR`, `TestScheduleDelete_RejectsCrossWorkspaceIDOR` — Bob attacks Alice's scheduleId → 0 rows affected → 404
  • Existing `TestRuntimeSchedule_HasSourceRuntime`, `TestList_IncludesSourceColumn` still green
  • `go test -race ./platform/...` clean locally

⚠️ CI currently blocked by GitHub Actions org spending-limit cap — not a code failure. See #110 / #119 for the same block. Verified locally.

🤖 Generated with Claude Code

ScheduleHandler.Update and Delete previously filtered the WHERE clause
on scheduleId alone, letting a member of workspace A mutate or delete
a schedule row owned by workspace B if it knew the target UUID.

Also fixes the same leak in the pre-update SELECT that read cron_expr
and timezone across workspaces.

All three queries now bind BOTH ":id" (from the wsAuth group) and
":scheduleId". Foreign rows return 404 with no leak. RunNow and History
already had the correct two-key filter — no changes there.

Two new tests drive a Bob→Alice attack via sqlmock: row counts come
back zero, handler returns 404.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

Superseded by #125 which fixes both #113 (same IDOR fix) and #120 (the PATCH unauth). Closing as the narrower of the two. I've left a regression comment on #125 about canvas drag-reposition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security(IDOR): schedules PATCH/DELETE don't bind scheduleId to workspace — cross-workspace mutation possible

1 participant