Repository navigation
fix(auth): close team-ownership takeover and cross-team datasource inserts - #91
Merged
Merged
Conversation
…serts - invite_team_member read the team with the caller's token. A non-member got nothing back, so the owner check was skipped and the member + any role (incl. owner) was inserted with the admin secret. The team is now read with the admin secret and the caller must be its owner or a portal admin. - create_team (the GraphQL action) made any caller OWNER of a new team with the default datasources. User-invoked calls now require a portal admin; the users-insert event trigger (no session) is unchanged. - datasources insert check was always true because user_id is preset to the caller, so anyone could insert a datasource into any team. A row must now belong to a team the caller is a member of, or be personal (team_id null). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
@coderabbitai review |
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.
Security review 2026-10-08 (finding C-02 follow-ups). Three authorization holes, each closed without changing any existing data.
1.
invite_team_member→ any user could make themselves owner of any team (Critical)rpc/inviteTeamMember.jsloaded the team with the caller's token. Users can only see teams they belong to, so a non-member gotnull,members?.lengthwas falsy and the owner check was skipped. The member row and the caller-chosen role (includingowner) were then inserted with the admin secret. Anyone signed in to dbx who knew a team id could become its owner, write raw-SQL models and read that team's datasource.Fix: read the team with the admin secret. Allow the invite only if the caller is an owner of that team or a portal admin (
isPortalAdmin, the existing helper). A missing team is refused.2.
create_teamaction → any user became OWNER of a new team with the default datasources (High)Fix: user-invoked calls (with session variables) require a portal admin. The
usersinsert event trigger arrives with no session and is unchanged. Tenant onboarding via sign-in is a separate follow-up.3.
datasourcesinsert permission was always true (High)The check
_or: [user_id = caller, team member]always passed becauseuser_idis preset to the caller. Anyone could insert a datasource, pointing at any host, into another tenant's team.Fix: a row must belong to a team the caller is a member of, or be personal (
team_idnull,user_id= caller). Legitimate inserts into your own team and personal datasources are unchanged.What changes for users
Not in this PR
reconcileOneTeam), so no change is needed there.Tests
services/actions/src/rpc/__tests__/teamAuthz.test.js: owner / member / outsider / missing team / empty team / portal admin, pluscreate_teamuser vs event trigger. 9/9 pass.node --test→ 62/62 pass.tables.yamlparses; the new check renders as intended.Deploy
Needs a new
synmetrix-actionsimage and thehasura-migrationsjob (metadata apply), then tag bumps in fraiosinfra/k8s/data/synmetrix.🤖 Generated with Claude Code