Skip to content

🛡️ Sentinel: [CRITICAL] Fix Missing Authorization Checks #107

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
daggerstuff wants to merge 1 commit into
base: staging
Choose a base branch
from
Open

🛡️ Sentinel: [CRITICAL] Fix Missing Authorization Checks #107

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
1 change: 1 addition & 0 deletions .Jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
## 2026-03-27 - Prevent SQL Error Leakage | Vulnerability: Information Exposure through Error Messages | Learning: Database error traces bubbling up to users can leak schema and path information. | Prevention: Catch database exceptions and return generic error messages (e.g. 500 Internal Server Error) to the client while logging the detailed exception server-side.
## 2026-03-29 - Server-Side Logging of Database Errors | Vulnerability: Insufficient Logging and Monitoring | Learning: Swallowing database errors without logging them hides potential malicious activity like SQL injection attempts. | Prevention: Always log database exception details using `logger.error` on the backend before returning a sanitized generic error to the client.
## 2026-03-31 - Enforce API Scope Authorization | Vulnerability: Missing Authorization | Learning: Endpoints that fetch an authenticated user dependency (`get_current_active_user_or_api_key`) do not automatically enforce permission scopes. | Prevention: Always verify the required `PermissionLevel` (e.g., `READ`, `WRITE`) against the user's or API key's `scopes` before executing the endpoint logic.
9 changes: 9 additions & 0 deletions api/dataset_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ async def list_datasets(
current_auth_entity: Any = Depends(get_current_active_user_or_api_key),
):
"""List all available datasets (tables in the database)."""
if PermissionLevel.READ not in current_auth_entity["scopes"]:
raise HTTPException(status_code=403, detail="Insufficient permissions to list datasets")
Comment on lines +139 to +140
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

READ checks depend on a broken user-scope source and can fail for JWT users

These checks assume current_auth_entity["scopes"] is always valid, but Line 105 builds scopes from user.permissions. Based on the auth model (security/api_authentication.py:106-121), permissions are role-derived, so this can throw for token-auth users and produce 500s instead of clean auth decisions.

🔧 Proposed fix 
 async def get_current_active_user_or_api_key(
     request: Request, api_key: Optional[str] = Depends(api_key_header)
 ):
@@
     user = getattr(request.state, "authenticated_user", None)
     if user:
+        scopes = auth_system.role_permissions.get(user.role, [])
         return {
             "username": user.username,
-            "scopes": user.permissions,
+            "scopes": scopes,
             "auth_type": "user_token",
         }
-    if PermissionLevel.READ not in current_auth_entity["scopes"]:
+    scopes = current_auth_entity.get("scopes") or []
+    if PermissionLevel.READ not in scopes:
         raise HTTPException(status_code=403, detail="Insufficient permissions to list datasets")

Apply the same scopes = ...get("scopes") or [] guard to the checks in get_dataset_metadata and query_dataset.

Also applies to: 202-204, 267-269

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/dataset_api.py` around lines 139 - 140, The permission checks directly
index current_auth_entity["scopes"] which can be missing for token-auth users;
change each check in list-datasets, get_dataset_metadata, and query_dataset to
first read scopes = current_auth_entity.get("scopes") or [] and then test
PermissionLevel.READ in scopes (and similarly for other PermissionLevel checks)
so missing scopes default to an empty list and return a proper 403 instead of
causing a 500.


datasets = []
try:
conn = get_db_connection()
Expand Down Expand Up @@ -196,6 +199,9 @@ async def get_dataset_metadata(
current_auth_entity: Any = Depends(get_current_active_user_or_api_key),
):
"""Get metadata (schema) for a specific dataset (table)."""
if PermissionLevel.READ not in current_auth_entity["scopes"]:
raise HTTPException(status_code=403, detail="Insufficient permissions to read dataset metadata")

conn = None
try:
# Validate input format immediately to prevent any SQL injection attempts
Expand Down Expand Up @@ -258,6 +264,9 @@ async def query_dataset(
"""
Query data from a specific dataset (table) with optional filters and pagination.
"""
if PermissionLevel.READ not in current_auth_entity["scopes"]:
raise HTTPException(status_code=403, detail="Insufficient permissions to query dataset")

conn = None
try:
# Validate input format immediately
Expand Down