Skip to content

Restrict origin#3342

Open
Kefancao wants to merge 1 commit into
mainfrom
kefan/restrict-origin
Open

Restrict origin#3342
Kefancao wants to merge 1 commit into
mainfrom
kefan/restrict-origin

Conversation

@Kefancao
Copy link
Copy Markdown
Contributor

@Kefancao Kefancao commented May 13, 2026

Changelist

[Describe or list the changes made in this PR]

Test Plan

[Describe how this PR was tested (if applicable)]

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

  • New Features

    • Added Origin allowlist validation for Turnkey endpoints to restrict requests from unauthorized origins.
    • New configuration option TURNKEY_ALLOWED_ORIGINS to specify a comma-separated list of allowed request origins.
  • Tests

    • Added comprehensive test suite for origin validation covering multiple scenarios, edge cases, and expected middleware behavior.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c05286fb-3954-49d5-a7f5-8e6cafae5132

📥 Commits

Reviewing files that changed from the base of the PR and between 5ee9766 and 65a5666.

📒 Files selected for processing (4)
  • indexer/services/comlink/__tests__/lib/origin-check.test.ts
  • indexer/services/comlink/src/config.ts
  • indexer/services/comlink/src/controllers/api/v4/turnkey-controller.ts
  • indexer/services/comlink/src/lib/origin-check.ts

📝 Walkthrough

Walkthrough

This PR adds origin header validation to Turnkey endpoints. It introduces a reusable Express middleware that parses an allowlist from configuration, performs case-insensitive exact-match validation of incoming Origin headers, and rejects non-matching origins with a 403 response while allowing requests without an Origin header to pass through.

Changes

Origin Header Allowlist Validation

Layer / File(s) Summary
Origin allowlist middleware and validation tests
indexer/services/comlink/src/lib/origin-check.ts, indexer/services/comlink/__tests__/lib/origin-check.test.ts
Implements originAllowlistMiddleware that parses comma-separated origins, normalizes to lowercase, and enforces exact-match validation with 403 rejection for disallowed origins. Tests cover empty/whitespace allowlists (no-op), case-insensitive matching, whitespace trimming, exact-match enforcement (no prefix/suffix), 403 error payload structure, and Vary: Origin header behavior.
Configuration and route integration
indexer/services/comlink/src/config.ts, indexer/services/comlink/src/controllers/api/v4/turnkey-controller.ts
Adds TURNKEY_ALLOWED_ORIGINS configuration field (comma-separated, default empty string), imports the middleware, instantiates it from config at module load, and chains it before rate limiting and validation handlers on both /turnkey/signin and /turnkey/uploadAddress POST routes.

Sequence Diagram

sequenceDiagram
  participant Client
  participant originAllowlistMiddleware
  participant Handler
  participant Response

  Client->>originAllowlistMiddleware: POST /turnkey/signin<br/>(with Origin header)
  alt Origin in allowlist
    originAllowlistMiddleware->>originAllowlistMiddleware: Vary: Origin
    originAllowlistMiddleware->>Handler: next()
    Handler->>Response: 200 OK
  else Origin not in allowlist
    originAllowlistMiddleware->>Response: 403 Forbidden<br/>(JSON error)
  else No Origin header
    originAllowlistMiddleware->>Handler: next()
    Handler->>Response: 200 OK
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops with Origin so bright,
Checking domains left and right,
Allowlists parsed with case-insensitive care,
403s bounce through the air—
Turnkey endpoints, now secure and fair! 🔐

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description follows the template structure but contains only placeholder headings with no actual content filled in under 'Changelist' or 'Test Plan' sections. Complete the Changelist section describing the added Origin allowlist middleware, config field, and Turnkey endpoint integration, and describe the testing approach (unit tests for middleware behavior).
Title check ❓ Inconclusive The title 'Restrict origin' is vague and does not provide sufficient clarity about what is being restricted or why, making it difficult for reviewers scanning history to understand the specific change. Use a more descriptive title such as 'Add Origin header allowlist validation for Turnkey endpoints' to clearly convey the primary change and its scope.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kefan/restrict-origin

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant