Skip to content

fix runtime request object auth#7

Open
Jing-command wants to merge 1 commit intomainfrom
fix/runtime-task-object-auth
Open

fix runtime request object auth#7
Jing-command wants to merge 1 commit intomainfrom
fix/runtime-task-object-auth

Conversation

@Jing-command
Copy link
Copy Markdown
Owner

Summary

  • bind runtime ping and update reads to the route runtime instead of loading request rows by global ID alone
  • bind daemon ping and update result writes to the route runtime so a known request ID cannot be completed from another runtime
  • add handler regressions covering cross-workspace reads and cross-runtime result submission

Test plan

  • go -C "/Users/a1234/multica/.worktrees/runtime-task-object-auth/server" test ./internal/handler -run 'Test(PingPersistsAcrossHandlerRestart|UpdatePersistsAcrossHandlerRestart|GetPingReturnsPersistedTerminalStateWhenTimeoutUpdateLosesRace|GetUpdateReturnsPersistedTerminalStateWhenTimeoutUpdateLosesRace|GetPingTimesOutAfterHeartbeatClaimsIt|GetPingRejectsPingFromAnotherWorkspaceRuntime|GetUpdateRejectsUpdateFromAnotherWorkspaceRuntime|ReportPingResultRejectsPingBoundToAnotherRuntime|ReportUpdateResultRejectsUpdateBoundToAnotherRuntime)$'
  • make -C "/Users/a1234/multica/.worktrees/runtime-task-object-auth" test ENV_FILE=.env.worktree

🤖 Generated with Claude Code

Require runtime-scoped ping and update lookups and result writes so known global request IDs cannot cross runtime or workspace boundaries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 16, 2026 16:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens authorization around runtime “request objects” (ping/update) by binding reads and result writes to the runtime ID from the route, preventing cross-workspace reads and cross-runtime completion when a request UUID is known.

Changes:

  • Add runtime-scoped SQL queries for fetching and completing/failing runtime ping and update requests.
  • Update ping/update handlers to enforce runtime binding for reads and daemon result submissions, and to validate workspace membership via the route runtime.
  • Add regression tests covering cross-workspace reads and cross-runtime result submission rejections.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
server/pkg/db/queries/runtime_request.sql Adds ...ForRuntime query variants that scope reads/writes by (id, runtime_id).
server/pkg/db/generated/runtime_request.sql.go Regenerates sqlc output for the new runtime-scoped query methods and params.
server/internal/handler/runtime_ping.go Uses runtime-scoped ping queries and enforces runtime/workspace binding on read and result write paths.
server/internal/handler/runtime_update.go Uses runtime-scoped update queries and enforces runtime/workspace binding on read and result write paths.
server/internal/handler/handler_test.go Adds helpers + regressions for cross-workspace read and cross-runtime result submission.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1912 to +1913
getReq := withURLParam(newRequest("GET", "/api/runtimes/"+mustGetHandlerTestRuntimeID(t)+"/ping/"+created.ID, nil), "runtimeId", mustGetHandlerTestRuntimeID(t))
getReq = withURLParam(getReq, "pingId", created.ID)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mustGetHandlerTestRuntimeID(t) is called twice when building the request, which performs two separate DB lookups and repeats the same work. Store it in a local variable once (e.g., runtimeID := mustGetHandlerTestRuntimeID(t)) and reuse it for both the path and the URL param to keep the test cheaper and clearer.

Copilot uses AI. Check for mistakes.
Comment on lines +1942 to +1943
getReq := withURLParam(newRequest("GET", "/api/runtimes/"+mustGetHandlerTestRuntimeID(t)+"/update/"+created.ID, nil), "runtimeId", mustGetHandlerTestRuntimeID(t))
getReq = withURLParam(getReq, "updateId", created.ID)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mustGetHandlerTestRuntimeID(t) is called twice when building the request, resulting in duplicate DB queries for the same runtime ID. Assign it to a variable once and reuse it for both the path and the URL param to avoid unnecessary work and make the test easier to read.

Copilot uses AI. Check for mistakes.
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.

3 participants