Skip to content

OUT-4047: scope deleteChannelMapsByIds to the caller's portal - #131

Open
SandipBajracharya wants to merge 2 commits into
mainfrom
OUT-4047
Open

OUT-4047: scope deleteChannelMapsByIds to the caller's portal#131
SandipBajracharya wants to merge 2 commits into
mainfrom
OUT-4047

Conversation

@SandipBajracharya

Copy link
Copy Markdown
Collaborator

Problem

MapFilesService.deleteChannelMapsByIds(ids) soft-deleted channels and their file rows purely by id, with no portal guard. A service authenticated for one portal could soft-delete a channel (and all its file rows) owned by another workspace if a foreign channel id was passed — a tenant-isolation break on a destructive operation.

Not currently exploitable (both callers pre-scope ids: listFormattedChannelMap derives them from the portal-guarded getAllChannelMaps, and Sync.service.ts passes the channel being synced), but a destructive bulk method should enforce the boundary itself.

Fix

Scope both UPDATEs in the transaction to this.user.portalId:

  • channels: and(inArray(channelSync.id, ids), eq(channelSync.portalId, this.user.portalId))
  • files: and(inArray(fileFolderSync.channelSyncId, ids), eq(fileFolderSync.portalId, this.user.portalId))

Both existing callers pass owned ids, so their behavior is unchanged.

Test

New integration regression test (test/flows/channel-cascade-portal-guard.integration.test.ts):

  • a foreign-portal channel id is ignored (channel + rows left untouched)
  • the caller's own channel still cascade soft-deletes (deletedAt + status=false, file rows soft-deleted)

Verified the guard test fails without the fix (foreign channel gets deleted) and passes with it.

Verification

  • pnpm test:integration test/flows/channel-cascade-portal-guard.integration.test.ts → 2/2 pass
  • pnpm typecheck clean · pnpm lint clean

Context

Surfaced by Greptile during review of the OUT-4044 test PR (#130); split out as a standalone production fix.

🤖 Generated with Claude Code

deleteChannelMapsByIds soft-deleted channels and their file rows by id with no
portal guard, so a service for one portal could delete another workspace's
channel if a foreign id was passed. Scope both UPDATEs in the transaction to
this.user.portalId. Current callers already pass owned ids, so their behavior is
unchanged.

Adds an integration regression test: a foreign-portal channel id is ignored,
while the caller's own channel still cascade soft-deletes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 14, 2026

Copy link
Copy Markdown

OUT-4047

@vercel

vercel Bot commented Aug 14, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
dropbox-integration Ready Ready Preview Aug 14, 2026 11:14am

Request Review

@SandipBajracharya SandipBajracharya changed the title fix(OUT-4047): scope deleteChannelMapsByIds to the caller's portal OUT-4047: scope deleteChannelMapsByIds to the caller's portal Aug 14, 2026
@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR scopes channel-map soft deletion to the authenticated portal, preventing supplied foreign channel IDs from deleting another workspace’s records.

  • Adds portal predicates to both channel and file-row updates within the existing transaction.
  • Adds integration coverage for foreign, owned, and mixed-portal ID batches.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/features/sync/lib/MapFiles.service.ts Adds authenticated-portal scoping to both transactional soft-delete operations.
test/flows/channel-cascade-portal-guard.integration.test.ts Adds regression coverage showing foreign records remain untouched while owned records are cascade soft-deleted.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Service as MapFilesService
    participant DB
    Caller->>Service: deleteChannelMapsByIds(ids)
    Service->>DB: Begin transaction
    Service->>DB: Soft-delete channels matching ids + caller portal
    Service->>DB: Soft-delete file rows matching channel ids + caller portal
    DB-->>Service: Commit
    Service-->>Caller: Complete
Loading

Reviews (2): Last reviewed commit: "test(OUT-4047): cover mixed owned+foreig..." | Re-trigger Greptile

deleteChannelMapsByIds takes a batch; assert a call mixing an owned and a
foreign channel id deletes only the owned one.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai review again

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