Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Add checkInternalAuth and checkSessionOrInternalAuth functions to enforce route-specific auth
  • Migrate tool routes to checkInternalAuth (executor-only, blocks API key access)
  • Migrate /api/function/execute and /api/providers to internal-only auth
  • Custom tools route uses checkSessionOrInternalAuth (UI + executor, no API keys)
  • A2A routes keep hybrid auth for conditional access

Type of Change

  • Bug fix

Testing

Tested manually - TypeScript compiles without errors

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 Jan 24, 2026

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 24, 2026 9:43am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 24, 2026

Greptile Summary

Implements three-tier authentication strategy to restrict API key access on internal routes:

  • Added checkInternalAuth function that blocks API keys and sessions, accepting only internal JWT (executor-to-server)
  • Added checkSessionOrInternalAuth function that accepts sessions and internal JWT but blocks API keys
  • Extracted shared resolveUserFromJwt helper to reduce code duplication across auth functions
  • Migrated 70+ tool routes from checkHybridAuth to checkInternalAuth (executor-only access)
  • Migrated /api/function/execute and /api/providers to checkInternalAuth (executor-only)
  • Migrated /api/tools/custom to checkSessionOrInternalAuth (UI + executor, no API keys)
  • Removed browser-side execution path in agent handler that relied on /api/providers endpoint
  • A2A routes and other external-facing endpoints retain checkHybridAuth for backward compatibility
  • Updated tests to mock correct auth functions

This security enhancement prevents external API key access to sensitive internal endpoints while maintaining proper access for the executor and UI.

Confidence Score: 4/5

  • This PR is safe to merge with proper testing to verify authentication flows work as expected
  • The security model is sound and the implementation is consistent across 88 files. Score is 4 (not 5) due to: (1) significant code duplication in auth functions that could be refactored, and (2) the removal of browser-side execution without clear documentation of whether this affects any existing workflows
  • Pay attention to apps/sim/lib/auth/hybrid.ts for the noted code duplication issue

Important Files Changed

Filename Overview
apps/sim/lib/auth/hybrid.ts Added checkInternalAuth and checkSessionOrInternalAuth functions with shared helper resolveUserFromJwt to enforce route-specific auth policies
apps/sim/app/api/function/execute/route.ts Migrated from checkHybridAuth to checkInternalAuth to block external access
apps/sim/app/api/providers/route.ts Migrated from checkHybridAuth to checkInternalAuth to restrict to executor-only access
apps/sim/app/api/tools/custom/route.ts Migrated from checkHybridAuth to checkSessionOrInternalAuth to allow UI and executor but block external access
apps/sim/executor/handlers/agent/agent-handler.ts Removed browser-side execution code path that relied on /api/providers endpoint

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.

85 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@waleedlatif1
Copy link
Collaborator Author

@greptile

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