Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • added auth checks for various routes reusing the checkHybridAuth helper
  • mysql and postgres query validation to prevent injections
  • csp improvements to only allow localhost on dev

Type of Change

  • Other: Security

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 19, 2025

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 19, 2025 8:35am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 19, 2025

Greptile Summary

This PR strengthens security across authentication, authorization, and SQL injection prevention. The changes centralize authentication logic through a new checkHybridAuth helper, add comprehensive permission checks to API routes, and implement query validation for database operations.

Key improvements:

  • Centralized authentication via checkHybridAuth supporting session, API tokens, and internal JWT
  • Added authorization checks to jobs, execution logs, memory, and template endpoints using workflow/workspace permissions
  • Implemented SQL/NoSQL injection prevention with query validation and identifier sanitization for MySQL, PostgreSQL, and MongoDB
  • Updated CSP configuration to restrict localhost URLs to development environment only
  • Created reusable permission helpers in lib/templates/permissions.ts for cleaner authorization code

Security concerns:

  • MySQL and PostgreSQL WHERE clauses accept raw strings that are validated but not fully parameterized, creating potential injection vectors if user-controlled values are embedded in WHERE conditions (e.g., id = ${userInput}). While dangerous patterns are blocked, this is less secure than fully parameterized queries.

Confidence Score: 4/5

  • This PR significantly improves security but has WHERE clause parameterization issues in SQL utilities that should be addressed
  • Score reflects strong authentication/authorization improvements and comprehensive NoSQL injection prevention, but MySQL/PostgreSQL WHERE clause handling uses string concatenation instead of full parameterization, which is a security risk even with pattern validation
  • Pay close attention to apps/sim/app/api/tools/mysql/utils.ts and apps/sim/app/api/tools/postgresql/utils.ts for WHERE clause parameterization issues

Important Files Changed

Filename Overview
apps/sim/lib/auth/hybrid.ts New centralized authentication helper supporting session, API key, and internal JWT auth - clean implementation with proper error handling
apps/sim/app/api/tools/mysql/utils.ts Added comprehensive SQL injection prevention with query validation, identifier sanitization, and WHERE clause validation - good security patterns but validateWhereClause is not used in all query builders
apps/sim/app/api/tools/postgresql/utils.ts Added PostgreSQL-specific injection prevention with proper parameterization and WHERE clause validation - uses parameterized queries correctly
apps/sim/lib/core/security/csp.ts Updated CSP to only allow localhost URLs in development mode - prevents production security issues
apps/sim/app/api/jobs/[jobId]/route.ts Replaced manual auth code with checkHybridAuth and added proper workflow/task ownership verification before returning results
apps/sim/app/api/logs/execution/[executionId]/route.ts Added authentication and workspace permission checks via database joins - ensures only authorized users can access execution logs
apps/sim/app/api/memory/[id]/route.ts Added authentication and granular read/write access control for memory operations with proper workflow ownership checks
apps/sim/app/api/templates/[id]/route.ts Added comprehensive permission checks using verifyCreatorPermission and verifyWorkflowAccess helpers - ensures only authorized users can modify/delete templates

Sequence Diagram

sequenceDiagram
    participant Client
    participant APIRoute
    participant AuthModule
    participant Database
    participant PermModule

    Client->>APIRoute: HTTP Request
    APIRoute->>AuthModule: Validate credentials
    AuthModule->>Database: Check authentication
    Database-->>AuthModule: Authentication result
    
    alt Authentication passes
        AuthModule-->>APIRoute: User identity confirmed
        APIRoute->>PermModule: Verify resource access
        PermModule->>Database: Check permissions
        Database-->>PermModule: Permission result
        
        alt Has permission
            PermModule-->>APIRoute: Access granted
            APIRoute-->>Client: Success response
        else No permission
            PermModule-->>APIRoute: Access denied
            APIRoute-->>Client: 403 Forbidden
        end
    else Authentication fails
        AuthModule-->>APIRoute: Invalid credentials
        APIRoute-->>Client: 401 Unauthorized
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (4)

  1. apps/sim/app/api/tools/mysql/utils.ts, line 79-89 (link)

    logic: WHERE clause is embedded as raw string rather than parameterized. While validateWhereClause blocks many injection patterns, this approach is less secure than parameterized queries. If the WHERE clause contains user values like id = 123, those values should be parameterized too.

    Consider accepting WHERE clause as structured params:

  2. apps/sim/app/api/tools/mysql/utils.ts, line 92-99 (link)

    logic: Same WHERE clause parameterization issue as in buildUpdateQuery. Raw string WHERE clauses allow user-controlled values to be embedded directly in SQL, even though dangerous patterns are blocked.

  3. apps/sim/app/api/tools/postgresql/utils.ts, line 149-171 (link)

    logic: WHERE clause is embedded as raw string in the query. While validateWhereClause blocks injection patterns, user values in the WHERE clause (like id = 123) aren't parameterized, creating a potential injection vector.

  4. apps/sim/app/api/tools/postgresql/utils.ts, line 173-189 (link)

    logic: Same WHERE clause parameterization issue as in executeUpdate.

17 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 3a33ec9 into staging Dec 19, 2025
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/authentication branch December 19, 2025 08:44
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