Skip to content

fix(cron): reject CRON requests when CRON secret is not set #2343

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

Jump to bottom
Merged
waleedlatif1 merged 1 commit into from
Dec 13, 2025
Merged

fix(cron): reject CRON requests when CRON secret is not set #2343

waleedlatif1 merged 1 commit into from
Dec 13, 2025

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Dec 13, 2025

Summary

  • reject CRON requests when CRON secret is not set
    • the reason CRON_SECRET is not required and is optional is because not everyone OSS user has the cron jobs setup to run, but when we validateCronAuth we ensure that the user has it set

thanks to https://github.com/H2u8s for pointing this out

Type of Change

  • Bug fix

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 13, 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 13, 2025 1:04am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 13, 2025

Greptile Overview

Greptile Summary

This PR adds critical security validations to prevent authentication bypass when optional secrets are not configured. The changes ensure that both CRON_SECRET and INTERNAL_API_SECRET must be set before their respective authentication mechanisms will succeed.

Key Changes:

  • verifyCronAuth() now returns 401 Unauthorized when CRON_SECRET is not configured, with proper logging of unauthorized access attempts
  • validateWorkflowAccess() now validates that INTERNAL_API_SECRET exists before allowing internal secret authentication
  • Both changes follow the same security principle: optional secrets should reject requests when not configured, rather than inadvertently allowing access

The fix addresses a security vulnerability where CRON endpoints could be accessed without authentication when CRON_SECRET was not set in the environment.

Confidence Score: 5/5

  • This PR is safe to merge and fixes a critical security vulnerability
  • The changes are minimal, focused, and directly address a security issue. Both fixes follow the same pattern of validating that optional secrets exist before using them for authentication. The implementation includes proper logging and returns appropriate HTTP status codes.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/auth/internal.ts 5/5 Added security check to reject CRON requests when CRON_SECRET is not configured, preventing unauthorized access
apps/sim/app/api/workflows/middleware.ts 5/5 Added validation to ensure INTERNAL_API_SECRET is configured before allowing internal secret authentication

Sequence Diagram

sequenceDiagram
    participant Client
    participant CronEndpoint
    participant verifyCronAuth
    participant WorkflowAPI
    participant validateWorkflowAccess

    Note over Client,verifyCronAuth: Before Fix
    Client->>CronEndpoint: Request CRON job
    CronEndpoint->>verifyCronAuth: Check authorization
    verifyCronAuth-->>CronEndpoint: May proceed without proper check

    Note over Client,verifyCronAuth: After Fix
    Client->>CronEndpoint: Request CRON job
    CronEndpoint->>verifyCronAuth: Check authorization
    alt Properly configured
        verifyCronAuth-->>CronEndpoint: Verify token
        alt Token valid
            verifyCronAuth-->>CronEndpoint: Proceed
        else Token invalid
            verifyCronAuth-->>CronEndpoint: 401 Unauthorized
        end
    else Not configured
        verifyCronAuth->>verifyCronAuth: Log warning
        verifyCronAuth-->>CronEndpoint: 401 Unauthorized
    end

    Note over Client,validateWorkflowAccess: Workflow Auth After Fix
    Client->>WorkflowAPI: Request workflow
    WorkflowAPI->>validateWorkflowAccess: Validate access
    alt Internal header check passes
        validateWorkflowAccess-->>WorkflowAPI: Return workflow
    else Internal header check fails
        validateWorkflowAccess->>validateWorkflowAccess: Check API key
        validateWorkflowAccess-->>WorkflowAPI: Result
    end
Loading

greptile-apps[bot]
greptile-apps bot reviewed Dec 13, 2025
View reviewed changes
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.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit e359dc2 into staging Dec 13, 2025
10 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/vulnerability branch December 13, 2025 01:08
@waleedlatif1 waleedlatif1 mentioned this pull request Dec 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@waleedlatif1