fix(api): enforce same-partner ownership on partner-wide script writes (#3262 follow-up) - #3272
Merged
Merged
Conversation
#3262 review) Review follow-ups on the partner-wide script write gate: - Defense-in-depth: the PUT/DELETE guards for partner-wide scripts (org_id NULL, partner_id set) checked the capability but relied on RLS row invisibility alone for cross-partner isolation. Add an explicit app-layer same-partner ownership check answering 404 (not 403 — do not leak that the script id exists), mirroring the rescope path's currentScope.partnerId comparison. - Extract the duplicated two-guard block from the PUT and DELETE handlers into one partnerWideScriptWriteError helper (with the ownership check inside) so the rules can never drift between routes. - RescopeAuth.partnerOrgAccess: make the key required (value may still be undefined) so future resolveRescopeTarget callers are forced by the compiler to thread the partner-wide capability through. The cross-partner rescope test now expects 404/"Script not found" instead of the previous existence-leaking 403; new tests prove the app layer alone rejects cross-partner PUT and DELETE with 404. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
9e987d6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b201278.breeze-9te.pages.dev |
| Branch Preview URL: | https://fix-3262-followup-partner-ow.breeze-9te.pages.dev |
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
1 participant
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.
Follow-up to #3263, which was merged just before this reviewed hardening commit reached the PR branch (it landed on the fork moments after the squash-merge — see #3263 (comment)).
Same diff as fork commit
1cfe0e8, cherry-picked onto main:script.partnerId !== auth.partnerIdnow returns 404 (no existence leak) instead of relying solely on RLS row-invisibility. Defense-in-depth — no exploit today, but protects against future refactors that load scripts under a system DB context (the [api] Mass agent reconnect can poison the Postgres pool (idle-in-transaction timeouts on config_policy_assignments) → heartbeat 500s + login outage #1105 pattern).RescopeAuth.partnerOrgAccessis now a required key, so future callers ofresolveRescopeTargetcan't omit it and silently compile into fail-closed 403s for genuine full-partner admins.partnerWideScriptWriteErrorhelper so PUT and DELETE can't drift.One pre-existing test updated: cross-partner rescope now expects 404 rather than the old 403 "not owned by your partner" (that message was the existence leak).
Verified:
scripts.test.ts47 passed | 2 skipped (two new cross-partner denial tests);tsc --noEmitclean on apps/api.🤖 Generated with Claude Code