Skip to content

🛡️ Sentinel: Fix IDOR and SQL Wildcard Enumeration vulnerabilities #524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ngoiyaeric wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: Fix IDOR and SQL Wildcard Enumeration vulnerabilities #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2026-02-16 - IDOR and Wildcard Enumeration in Server Actions
**Vulnerability:** IDOR in `getChatMessages` and `updateDrawingContext` allowed unauthorized access/modification to chat data. `searchUsers` was susceptible to wildcard enumeration.
**Learning:** Exported server actions in Next.js ('use server') are public endpoints and must implement their own authentication and authorization checks, even if they are only intended for use in protected pages.
**Prevention:** Always retrieve the current user session within the server action and verify ownership/permissions before performing database operations. Escape special SQL characters in `LIKE`/`ILIKE` patterns from user input.
Comment on lines +1 to +4
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix markdown lint warnings (MD022, MD041).

Static analysis flags two issues: the first line should be a top-level heading (#), and headings need surrounding blank lines.

Proposed fix 
-## 2026-02-16 - IDOR and Wildcard Enumeration in Server Actions
-**Vulnerability:** IDOR in `getChatMessages` and `updateDrawingContext` allowed unauthorized access/modification to chat data. `searchUsers` was susceptible to wildcard enumeration.
-**Learning:** Exported server actions in Next.js ('use server') are public endpoints and must implement their own authentication and authorization checks, even if they are only intended for use in protected pages.
-**Prevention:** Always retrieve the current user session within the server action and verify ownership/permissions before performing database operations. Escape special SQL characters in `LIKE`/`ILIKE` patterns from user input.
+# Sentinel Security Notes
+
+## 2026-02-16 - IDOR and Wildcard Enumeration in Server Actions
+
+**Vulnerability:** IDOR in `getChatMessages` and `updateDrawingContext` allowed unauthorized access/modification to chat data. `searchUsers` was susceptible to wildcard enumeration.
+
+**Learning:** Exported server actions in Next.js ('use server') are public endpoints and must implement their own authentication and authorization checks, even if they are only intended for use in protected pages.
+
+**Prevention:** Always retrieve the current user session within the server action and verify ownership/permissions before performing database operations. Escape special SQL characters in `LIKE`/`ILIKE` patterns from user input.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## 2026-02-16 - IDOR and Wildcard Enumeration in Server Actions
**Vulnerability:** IDOR in `getChatMessages` and `updateDrawingContext` allowed unauthorized access/modification to chat data. `searchUsers` was susceptible to wildcard enumeration.
**Learning:** Exported server actions in Next.js ('use server') are public endpoints and must implement their own authentication and authorization checks, even if they are only intended for use in protected pages.
**Prevention:** Always retrieve the current user session within the server action and verify ownership/permissions before performing database operations. Escape special SQL characters in `LIKE`/`ILIKE` patterns from user input.
# Sentinel Security Notes
## 2026-02-16 - IDOR and Wildcard Enumeration in Server Actions
**Vulnerability:** IDOR in `getChatMessages` and `updateDrawingContext` allowed unauthorized access/modification to chat data. `searchUsers` was susceptible to wildcard enumeration.
**Learning:** Exported server actions in Next.js ('use server') are public endpoints and must implement their own authentication and authorization checks, even if they are only intended for use in protected pages.
**Prevention:** Always retrieve the current user session within the server action and verify ownership/permissions before performing database operations. Escape special SQL characters in `LIKE`/`ILIKE` patterns from user input.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)

[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

[warning] 1-1: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🤖 Prompt for AI Agents 
In @.jules/sentinel.md around lines 1 - 4, Change the first line into a
top-level heading by prefixing it with "# " (e.g., "# 2026-02-16 - IDOR and
Wildcard Enumeration in Server Actions") and add blank lines immediately before
and after each heading (including the main heading and any "##" or "###"
headings) so they are surrounded by blank lines; also ensure the
`<details>`/`<summary>` block is separated from adjacent headings/content with
blank lines to satisfy MD022 and MD041.

36 changes: 28 additions & 8 deletions lib/actions/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,20 @@ export async function getChatMessages(chatId: string): Promise<DrizzleMessage[]>
console.warn('getChatMessages called without chatId');
return [];
}

const userId = await getCurrentUserIdOnServer();
if (!userId) {
throw new Error('Unauthorized: Authentication required');
}

try {
// Check if the user has access to this chat
const chat = await getChat(chatId, userId);
if (!chat) {
console.warn(`Unauthorized access attempt to chat ${chatId} by user ${userId}`);
return [];
}
Comment on lines +60 to +72
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Good IDOR fix — but mixed error-handling contract may surprise callers.

The authentication and authorization checks correctly prevent unauthorized access to chat messages. However, the function has an inconsistent contract:

  • Unauthenticated → throw (line 63), which escapes the try/catch at line 66.
  • Unauthorized (no chat access) → silently returns [] (line 71).
  • DB error → caught and returns [] (line 77).

A caller expecting DrizzleMessage[] will get an unhandled exception for unauthenticated requests but an empty array for everything else. Consider either returning [] consistently (aligning with the learned pattern that the chats API route returns empty for unauthenticated users) or throwing consistently.

Option A: return [] consistently (matches existing UX pattern) 
   const userId = await getCurrentUserIdOnServer();
   if (!userId) {
-    throw new Error('Unauthorized: Authentication required');
+    console.warn('getChatMessages: User not authenticated');
+    return [];
   }

Based on learnings, the app/api/chats/route.ts endpoint returns { chats: [] } for unauthenticated users instead of a 401 — a similar pattern here would keep the contract uniform.

🤖 Prompt for AI Agents 
In `@lib/actions/chat.ts` around lines 60 - 72, The current behavior is
inconsistent: getCurrentUserIdOnServer() throws for unauthenticated but
getChat(chatId, userId) returns [] for unauthorized/DB errors; change to a
consistent return-of-empty-array policy by replacing the thrown Error from the
unauthenticated check with an early return of [] (instead of throw), keep the
existing try/catch around getChat, and ensure any logging (e.g., the
console.warn for unauthorized access) remains but does not change the return
contract; update callers/tests that assume an exception to expect
DrizzleMessage[] from this action.


return dbGetMessagesByChatId(chatId);
} catch (error) {
console.error(`Error fetching messages for chat ${chatId} in getChatMessages:`, error);
Expand Down Expand Up @@ -127,15 +140,22 @@ export async function updateDrawingContext(chatId: string, contextData: { drawnF
return { error: 'User not authenticated' };
}

const newDrawingMessage: DbNewMessage = {
userId: userId,
chatId: chatId,
role: 'data',
content: JSON.stringify(contextData),
createdAt: new Date(),
};

try {
// Check if the user has access to this chat and owns it
const chat = await getChat(chatId, userId);
if (!chat || chat.userId !== userId) {
console.warn(`Unauthorized drawing context update attempt for chat ${chatId} by user ${userId}`);
return { error: 'Unauthorized: Ownership required to update drawing context' };
}

const newDrawingMessage: DbNewMessage = {
userId: userId,
chatId: chatId,
role: 'data',
content: JSON.stringify(contextData),
createdAt: new Date(),
};

const savedMessage = await dbCreateMessage(newDrawingMessage);
if (!savedMessage) {
throw new Error('Failed to save drawing context message.');
Expand Down
10 changes: 9 additions & 1 deletion lib/actions/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,26 @@ export async function searchUsers(query: string) {
noStore();
if (!query) return [];

// Prevent DoS and long pattern matching
if (query.length > 255) {
throw new Error('Search query too long');
}
Comment on lines +192 to +195
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Good addition of input length validation.

The length check correctly guards against excessively long patterns. One minor inconsistency: an empty query silently returns [] (line 190), but an over-length query throws. Consider returning [] here as well for a uniform caller experience, or document the throwing behavior so callers know to handle it.

🤖 Prompt for AI Agents 
In `@lib/actions/users.ts` around lines 192 - 195, The input-length check
currently throws an Error when query.length > 255, causing inconsistent behavior
because the empty-query path returns [] earlier; update the logic in the same
function that references query (replace the throw new Error('Search query too
long') branch) to return [] for over-length queries instead of throwing so
callers receive a uniform [] response, or alternatively add
documentation/comments to the function explaining that it will throw for
too-long queries if you prefer keeping the throw (choose one approach and apply
it to the query length check).


const userId = await getCurrentUserIdOnServer();
if (!userId) {
throw new Error('Unauthorized');
}

try {
// Escape special PostgreSQL characters to prevent wildcard enumeration
const escapedQuery = query.replace(/[%_\\]/g, '\\$&');

const result = await db.select({
id: users.id,
email: users.email,
})
.from(users)
.where(ilike(users.email, `%${query}%`))
.where(ilike(users.email, `%${escapedQuery}%`))
.limit(10);

return result;
Expand Down