feat: promote build-rls-003-org-roles-permissions to benchmark suite - #162
feat: promote build-rls-003-org-roles-permissions to benchmark suite#162mattrossman wants to merge 11 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Drops soft-delete and audit-log requirements. They weren't part of the actual motivation for promoting this scenario (org/role RLS complexity), and every scorer bug we hit lived in that bolted-on logic.
…d-rls-003-org-roles-permissions' into mattrossman/ai-1006-promote-build-rls-003-org-roles-permissions
Matches the checkX(ctx) pattern used elsewhere in the repo instead of imperative checks.push() calls. Also normalizes ctx.query into a non-throwing runAsUser helper so an RLS-blocked write (a real Postgres error) is a value each check judges, not an exception that silently aborts every check after it.
Seeds a user who's a viewer in org A and admin in org B, then checks that their org-B admin role doesn't leak into org A. Every seeded user previously belonged to exactly one org, so a policy that checked role without joining on org_id would have passed undetected. All tested benchmark agents still pass, but it's real coverage for a common multi-tenant RLS mistake.
Fixed a stale "active documents" reference left over from the removed soft-delete concept, and swapped an unverified PostgREST claim for a cited Supabase testing-guide reference.
Removes check comments that just restated their function name, merges asUser into runAsUser since the split had no independent caller, and clarifies the multi-org role-leak comment to name the memberships table explicitly.
barryroodt
left a comment
There was a problem hiding this comment.
Prompt rewrite reads like an actual person now, nice.
Agents already pass this across the board, which is against our typical criteria for adding benchmarks, but maybe we're okay with that here.
Fine by me to make the exception here.
On hardening, two of the 14 checks measure less than they look like:
RLS enabled on documentscan't fail anymore. The seed now enables RLS (project.sql:22) and the check reads that same flag. Repoint it at the hole you seeded: assert noSELECTpolicy withqual = 'true'survives inpg_policies.- The roster check rewards nuking access (inline).
Heads up that both of those change what's graded, so they'd need a results refresh. Your call whether that's worth it now or in a follow-up.
Also worth knowing: 7 of the 14 pass before the agent does anything. The seed turns RLS on and adds no write policies, so every "cannot insert / update / delete" is satisfied from the start.
| `SELECT user_id FROM memberships WHERE org_id = '${ORG_B}';` | ||
| ); | ||
| return { | ||
| name: "cannot see another org's membership roster", |
There was a problem hiding this comment.
Grades memberships, but the prompt only mentions documents. It's blocked-only too, so an agent that enables RLS on memberships with no policies scores the same as one that got it right. Every other negative here has a positive twin. Adding editor can still see their own org's roster makes it two-sided.
| const { rows } = await ctx.query(stripIndent` | ||
| BEGIN; | ||
| SET LOCAL ROLE authenticated; | ||
| SET LOCAL request.jwt.claim.sub = '${sub}'; |
There was a problem hiding this comment.
auth.jwt() is always {} here. lite reads auth.uid() from request.jwt.claim.sub but auth.jwt() from the JSON request.jwt.claims, which we never set, so an agent using auth.jwt() ->> 'sub' gets denied everywhere and scores wrong. One line:
SET LOCAL request.jwt.claims = '{"sub":"${sub}","role":"authenticated"}';| ctx: ToolEvalContext, | ||
| sub: string, | ||
| body: string, | ||
| finish: 'COMMIT' | 'ROLLBACK' = 'COMMIT' |
There was a problem hiding this comment.
Defaults to 'COMMIT' and checkViewerCannotInsert takes the default, so a failing agent gets a junk document committed into the scored project. Harmless today since nothing counts documents. Flip the default to 'ROLLBACK'?
| ); | ||
| return { | ||
| name: "editor cannot update another user's document", | ||
| passed: Boolean(result.error) || result.rows.length === 0, |
There was a problem hiding this comment.
Boolean(result.error) counts relation does not exist and current transaction is aborted as "RLS blocked it". Tighten to /row-level security|permission denied/i, or code === '42501'.
Promotes
build-rls-003-org-roles-permissionsto the benchmark suite and tweaks its prompt / scoring.Originally this scenario also tested the soft-delete and audit log implementation pattern, for which I found motivations FDBKIN-8277 and FDBKIN-9175 respectively, however I removed these from the scenario to keep the focus on RLS.
Other changes:
motivationmemberroster visibility, no cross-org role privilege escalationAgents already pass this across the board, which is against our typical criteria for adding benchmarks, but maybe we're okay with that here.
Preview: https://evals-git-mattrossman-ai-1006-promote-build-rls-47a093-supabase.vercel.app/?group=eval&sheet=eval&item=build-rls-003-org-roles-permissions
Closes AI-1006