Fix: IDOR attachment validation

[tembo]

Summary

Fixed IDOR vulnerability in message attachments by validating that each storageId belongs to the authenticated user before accepting or resolving file URLs.

Vulnerability Details

Message attachments were accepted from clients without verifying ownership of the referenced storage files. This allowed attackers to reference arbitrary storageId values and retrieve signed URLs for other users' files.

Changes Made

• Added validateAttachmentOwnership(): Validates each attachment's storageId against the fileUploads table to ensure:

  • The file exists in the user's uploads
  • The file belongs to the authenticated user
  • The file has not been soft-deleted
  • Maximum 20 attachments per message (abuse prevention)

• Added getVerifiedStorageIds(): For the list query path, returns only storageIds verified to belong to the requesting user, preventing URL generation for unauthorized files

• Updated insertOrUpdateMessage(): Calls validateAttachmentOwnership() before inserting/updating messages with attachments

• Updated list query: Filters attachment URLs to only include verified storageIds, returning null for unauthorized files

Files Modified

• apps/server/convex/messages.ts
  • Added security validation functions
  • Added ownership checks in message send path (lines 875-883)
  • Added ownership verification in message list path (lines 152-162)

Security Impact

• Prevents unauthorized file access via IDOR attacks
• Ensures only file owners can reference their uploads in messages
• Maintains backward compatibility with existing message structure

Want tembo to make any changes? Add a review or comment with @tembo and i'll get back to work!

---

Summary by cubic

Fixed an IDOR in message attachments by verifying storageId ownership and only generating URLs for files owned by the authenticated user. Prevents unauthorized access to other users’ uploads.

• Bug Fixes
  • On insert/update: validate attachment storageIds against fileUploads (exists, owned by user, not soft-deleted) and enforce a 20-attachment limit.
  • On list: verify storageIds before generating URLs; return null for unverified files.

^{Written for commit 72fa493. Summary will update on new commits.}

[railway-app]

This PR was not deployed automatically as @tembo[bot] does not have access to the Railway project.

In order to get automatic PR deploys, please add @tembo[bot] to your workspace on Railway.

[tembo]

Requesting review from @leoisadev1 who has experience with the following files modified in this PR:

• apps/server/convex/messages.ts

[cubic-dev-ai]

2 issues found across 1 file

Confidence score: 2/5

• High risk due to a likely security bypass in apps/server/convex/messages.ts where missing userId skips attachment ownership validation, enabling IDOR if callers omit it.
• Distinct error messages in apps/server/convex/messages.ts can leak file existence/ownership, enabling storage ID enumeration and privacy exposure.
• Pay close attention to apps/server/convex/messages.ts - ensure ownership checks always run and error messages are normalized.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/server/convex/messages.ts">

<violation number="1" location="apps/server/convex/messages.ts:779">
P2: Information leakage: distinct error messages for "not found", "not owned", and "deleted" allow an attacker to enumerate storage IDs and learn about other users' files. Use a single generic error message for all three cases to prevent information disclosure.</violation>

<violation number="2" location="apps/server/convex/messages.ts:881">
P1: Security bypass: attachment ownership validation is silently skipped when `userId` is undefined. Since `userId` is an optional parameter, any caller that omits it will bypass the IDOR check entirely. This should throw an error if attachments are present but `userId` is missing, rather than silently proceeding.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: Security bypass: attachment ownership validation is silently skipped when userId is undefined. Since userId is an optional parameter, any caller that omits it will bypass the IDOR check entirely. This should throw an error if attachments are present but userId is missing, rather than silently proceeding.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/messages.ts, line 881:

<comment>Security bypass: attachment ownership validation is silently skipped when `userId` is undefined. Since `userId` is an optional parameter, any caller that omits it will bypass the IDOR check entirely. This should throw an error if attachments are present but `userId` is missing, rather than silently proceeding.</comment>

<file context>
@@ -788,6 +875,13 @@ async function insertOrUpdateMessage(
+
+	// SECURITY: Validate attachment ownership before inserting/updating message
+	// This prevents IDOR where a user references another user's storageId
+	if (args.attachments && args.attachments.length > 0 && args.userId) {
+		await validateAttachmentOwnership(ctx, args.attachments, args.userId);
+	}
</file context>

Suggested change

	if (args.attachments && args.attachments.length > 0 && args.userId) {
	await validateAttachmentOwnership(ctx, args.attachments, args.userId);
	if (args.attachments && args.attachments.length > 0) {
	if (!args.userId) {
	throw new Error("userId is required when sending attachments");
	}
	await validateAttachmentOwnership(ctx, args.attachments, args.userId);

[cubic-dev-ai]

P2: Information leakage: distinct error messages for "not found", "not owned", and "deleted" allow an attacker to enumerate storage IDs and learn about other users' files. Use a single generic error message for all three cases to prevent information disclosure.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/messages.ts, line 779:

<comment>Information leakage: distinct error messages for "not found", "not owned", and "deleted" allow an attacker to enumerate storage IDs and learn about other users' files. Use a single generic error message for all three cases to prevent information disclosure.</comment>

<file context>
@@ -736,6 +743,86 @@ type ErrorData = {
+			.withIndex("by_storage", (q) => q.eq("storageId", attachment.storageId))
+			.unique();
+
+		if (!fileUpload) {
+			throw new Error(
+				"Unauthorized: attachment references a file that does not exist in your uploads.",
</file context>

Suggested change

	if (!fileUpload) {
	throw new Error(
	"Unauthorized: attachment references a file that does not exist in your uploads.",
	);
	}
	if (fileUpload.userId !== userId) {
	throw new Error(
	"Unauthorized: you do not own the referenced attachment file.",
	);
	}
	if (fileUpload.deletedAt) {
	throw new Error(
	"Attachment references a file that has been deleted.",
	if (!fileUpload || fileUpload.userId !== userId || fileUpload.deletedAt) {
	throw new Error(
	"Unauthorized: invalid attachment reference.",
	);
	}

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
osschat-web	Error		Feb 14, 2026 2:51am