Skip to content

fix(rpc): reject alarm websiteIds outside the organization - #1112

Merged
izadoesdev merged 4 commits into
databuddy-analytics:stagingfrom
ANSHSINGH050404:fix/alarms-website-ownership
Oct 17, 2026
Merged

izadoesdev merged 4 commits into
databuddy-analytics:stagingfrom
ANSHSINGH050404:fix/alarms-website-ownership

Conversation

@ANSHSINGH050404

@ANSHSINGH050404 ANSHSINGH050404 commented Oct 9, 2026 •

Copy link
Copy Markdown

Fixes #1110

Summary

Prevent alarms from being attached to websites belonging to another organization by validating website ownership before creating or updating an alarm.

Changes

  • Add requireAlarmWebsite to validate that a website exists, is not deleted, and belongs to the same organization as the alarm.
  • Apply the ownership check to both alarm creation and updates, before any database write.
  • Preserve existing organization-level update permission semantics. Deliberately avoid the website-scoped withWorkspace variant, which would change authorization behavior by granting member roles the ability to create alarms they cannot currently create.
  • Return a uniform BAD_REQUEST for missing or foreign websites, following the existing apikeys and insight-generation precedents and avoiding a website-existence oracle.

Regression Tests

Added alarms.website-ownership.test.ts covering:

  • Rejecting foreign-organization websites on create and update.
  • Allowing websites owned by the same organization.
  • Allowing null website IDs on creation.
  • Allowing an existing alarm to be unlinked from its website during an update.

Verification

  • New regression tests: 5/5 passed
  • Related with-workspace and API-key ownership tests: 36/36 passed
  • Full repository type checks: 37/37 passed
  • Ultracite, policy-lint, and commit-format hooks: passed

Environment note: The local pre-push test suite still fails on four pre-existing apps/links shutdown tests involving Bun.spawn(process.execPath) and Windows paths containing spaces. The changes were pushed using --no-verify; CI remains the final verification gate.

AI Disclosure

Diagnosed and implemented with AI assistance (Muse Spark via OpenCode). The changes were human-verified using the checks listed above.


Summary by cubic

Fixes #1110. Alarm create and update now reject website IDs outside the alarm's organization, closing the cross-org attachment path.

  • Adds a requireAlarmWebsite check, run before any write, that verifies the website belongs to the alarm's organization and is not deleted.
  • Foreign, missing, and deleted websites now return NOT_FOUND instead of BAD_REQUEST, avoiding a website-existence oracle.
  • Keeps null websiteId allowed on create and update, so org-wide and unlinked alarms still work.
  • Adds an isolated-DB integration test suite covering foreign and deleted website rejection, same-org attachment, org-wide creation, and unlink behavior.

Written for commit 84d0749. Summary will update on new commits.

View guided diff

@vercel

vercel Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
documentation Skipped Skipped Oct 17, 2026 5:51pm UTC

@vercel
vercel Bot temporarily deployed to Preview – documentation October 9, 2026 18:16 Inactive
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: fb15c158-429d-4ac5-b38e-b9ef3d9e571a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@vercel

vercel Bot commented Oct 9, 2026

Copy link
Copy Markdown

@ANSHSINGH050404 is attempting to deploy a commit to the Databuddy OSS Team on Vercel.

A member of the Team first needs to authorize it.

@ANSHSINGH050404
ANSHSINGH050404 marked this pull request as ready for review October 10, 2026 10:52
@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Critical impact] Adds authorization check for alarm website ownership.

The ownership checks look correct, but the new tests must meet the repository’s explicit requirements before merging.

Findings

  1. P2 Tests use prohibited mocks ▶
  2. P2 Procedure cast hides its type ▶
  3. P2 Import breaks repository rules ▶

Summary

Adds requireAlarmWebsite before alarm creation and website reassignment.

  • Alarms can only link to live websites in their organization.

ANSHSINGH050404 explicitly stated that organization-level update permissions are intentional. The website-scoped alternative would grant member roles access they do not currently have.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Create or update alarm] --> B[Check existing organization permissions]
  B --> C{Website ID supplied?}
  C -->|Yes| D[Check website owner and deletion state]
  D --> E{Website matches?}
  E -->|No| F[Return BAD_REQUEST]
  E -->|Yes| G[Write alarm]
  C -->|No| G
Loading

Reviews (1) · Last reviewed commit: "fix(rpc): reject alarm websiteIds outsid..." · Reviewed by Greptile

Comment thread packages/rpc/src/routers/alarms.website-ownership.test.ts Outdated
Comment thread packages/rpc/src/routers/alarms.website-ownership.test.ts Outdated
Comment thread packages/rpc/src/routers/alarms.website-ownership.test.ts Outdated
@vercel
vercel Bot temporarily deployed to Preview – documentation October 10, 2026 14:07 Inactive
@izadoesdev
izadoesdev force-pushed the fix/alarms-website-ownership branch from 5dedc50 to 84d0749 Compare October 17, 2026 17:50
@vercel
vercel Bot temporarily deployed to Preview – documentation October 17, 2026 17:51 Inactive
@izadoesdev
izadoesdev merged commit 88cd02a into databuddy-analytics:staging Oct 17, 2026
19 of 22 checks passed

This branch was previously deployed

1 inactive deployment
Preview – documentation — 84d07490 Deployed Oct 17, 2026 by vercel[bot]
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.

2 participants