fix(sdk): verify agent ownership in eval routes - #3
Open
manpreet171 wants to merge 1 commit into
Open
Conversation
The eval routes accepted agent_id, eval_set_id and regression_id straight from the request and queried with the service role client, which bypasses RLS. Any valid connect key could therefore read or write eval data belonging to another user. Adds an ownsAgent() helper next to the other sdk-auth logic and uses it in the routes that were missing the check, matching what traces, command, checkpoint and execution already do.
|
@manpreet171 is attempting to deploy a commit to the tharagesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
validateConnectKey()returns a service role Supabase client, which bypasses RLS. Routes that accept anagent_idfrom the request therefore have to check ownership themselves —traces,command,checkpointandexecutionall do this.The four
evalsroutes don't. They authenticate the caller and then query using ids taken straight from the request body:GET /api/sdk/evals/regressionagent_idPATCH /api/sdk/evals/regressionregression_idPOST /api/sdk/evals/regressionPOST /api/sdk/evals/resultsPOST /api/sdk/evals/from-tracetask_idThe
GETandPATCHcases need no plan gate, so a free account is enough.Fix
Added an
ownsAgent()helper inlib/sdk-auth.tsand used it in the routes that were missing the check. Where a route accepts aneval_set_id,regression_idortask_id, that id is resolved back to its agent (or user) first, so a caller can't pair their ownagent_idwith someone else's record.Responses follow the existing convention:
403withUnauthorized agent access.Tests
tests/evals_security.test.tsmirrors the structure oftests/traces_security.test.ts. 6 of the 8 new tests fail onmainand pass with this change.Full suite: 174 passed, and
tsc --noEmitis clean.