Skip to content

fix(auth): close team-ownership takeover and cross-team datasource inserts - #91

Merged
doktorhelgi merged 1 commit into
mainfrom
fix/team-authz-c02
Oct 8, 2026
Merged

doktorhelgi merged 1 commit into
mainfrom
fix/team-authz-c02

Conversation

@doktorhelgi

Copy link
Copy Markdown

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.js loaded the team with the caller's token. Users can only see teams they belong to, so a non-member got null, members?.length was falsy and the owner check was skipped. The member row and the caller-chosen role (including owner) 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_team action → 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 users insert event trigger arrives with no session and is unchanged. Tenant onboarding via sign-in is a separate follow-up.

3. datasources insert permission was always true (High)

The check _or: [user_id = caller, team member] always passed because user_id is 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_id null, user_id = caller). Legitimate inserts into your own team and personal datasources are unchanged.

What changes for users

  • Inviting: only team owners (as today in the UI) and portal admins.
  • The "create team" button now fails for non-portal-admins.
  • Nothing else; no data migration.

Not in this PR

  • Sign-in provisioning and the users-insert trigger still give new users their own team as owner with the default datasources. Changing that affects tenant onboarding and is planned separately.
  • The real C-02 fix (per-team ClickHouse credentials / row policies) needs a design.
  • The default-models cron already skips teams without a partition (reconcileOneTeam), so no change is needed there.

Tests

  • New services/actions/src/rpc/__tests__/teamAuthz.test.js: owner / member / outsider / missing team / empty team / portal admin, plus create_team user vs event trigger. 9/9 pass.
  • Full actions suite: node --test → 62/62 pass.
  • tables.yaml parses; the new check renders as intended.

Deploy

Needs a new synmetrix-actions image and the hasura-migrations job (metadata apply), then tag bumps in fraios infra/k8s/data/synmetrix.

🤖 Generated with Claude Code

…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>
@doktorhelgi

Copy link
Copy Markdown
Author

@coderabbitai review

@doktorhelgi
doktorhelgi merged commit ac06bf7 into main Oct 8, 2026
4 checks passed
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.

1 participant