Skip to content

Update Comlink GeoHeadersMiddleware to Expose geo headers#3282

Closed
jaredvu wants to merge 5 commits intomainfrom
jv/expose-geo-headers
Closed

Update Comlink GeoHeadersMiddleware to Expose geo headers#3282
jaredvu wants to merge 5 commits intomainfrom
jv/expose-geo-headers

Conversation

@jaredvu
Copy link

@jaredvu jaredvu commented Dec 12, 2025

Changelist

Allow exposing geo headers

Test Plan

  • Tested by updating Comlink Image to include this branch's contents and accessing values on FE

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

  • Bug Fixes
    • Improved handling and exposure of geo-location headers in API responses to ensure proper CORS compliance and visibility to client applications requesting location-related information.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

Walkthrough

The geo-headers middleware now tracks geo-origin-* headers present in requests and automatically emits an Access-Control-Expose-Headers response header containing the names of detected headers. The existing header-setting logic remains intact.

Changes

Cohort / File(s) Summary
Geo-origin header exposure tracking
indexer/services/comlink/src/request-helpers/geo-headers-middleware.ts
Introduces exposedHeaders collection to track detected geo-origin-* headers and conditionally emit Access-Control-Expose-Headers with collected header names when at least one is present.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with straightforward header collection and conditional exposure logic
  • Minimal scope with no new dependencies or complex interactions introduced

Poem

🐰 A middleware hops with headers in sight,
Gathering geo-origins, left and right,
Exposing what's there with a CORS-friendly gleam,
One file, one task—a clean little dream! 🌍

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The PR description follows the template structure with Changelist and Test Plan sections, though the test plan is incomplete. Complete the Test Plan section with specific details about how the geo headers functionality was tested beyond updating the Comlink image.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: updating GeoHeadersMiddleware to expose geo headers, which matches the changeset's purpose.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jv/expose-geo-headers

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
indexer/services/comlink/src/request-helpers/geo-headers-middleware.ts (1)

3-27: Document the trusted-proxy infrastructure requirement.

This middleware assumes geo-origin-* headers originate exclusively from trusted infrastructure (e.g., reverse proxy, CDN) and does not validate them. Ensure your deployment documentation or code comments clarify that these headers must be set only by trusted proxies and cannot come directly from client requests. Consider adding a comment to the middleware stating this assumption.

Note: CORS headers are properly configured via the cors middleware in the middleware chain; Access-Control-Expose-Headers here correctly declares the geo-origin headers to the CORS layer.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c275378 and 19e5d04.

📒 Files selected for processing (1)
  • indexer/services/comlink/src/request-helpers/geo-headers-middleware.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
  • GitHub Check: call-build-ecs-service-ender / (ender) Check docker image build
  • GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
  • GitHub Check: call-build-ecs-service-comlink / (comlink) Check docker image build
  • GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
  • GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
  • GitHub Check: check-build-auxo
  • GitHub Check: check-build-bazooka
  • GitHub Check: test / run_command
  • GitHub Check: build-and-push-testnet
  • GitHub Check: build-and-push-mainnet
  • GitHub Check: run_command
  • GitHub Check: lint
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Summary

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a2a118 and fdb95ab.

📒 Files selected for processing (1)
  • .github/workflows/protocol-build-and-push-snapshot.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (33)
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
  • GitHub Check: test / run_command
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
  • GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
  • GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
  • GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
  • GitHub Check: call-build-ecs-service-comlink / (comlink) Check docker image build
  • GitHub Check: call-build-ecs-service-ender / (ender) Check docker image build
  • GitHub Check: check-build-bazooka
  • GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
  • GitHub Check: check-build-auxo
  • GitHub Check: lint
  • GitHub Check: build-and-push-testnet
  • GitHub Check: build-and-push-mainnet
  • GitHub Check: run_command
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Summary
  • GitHub Check: build-and-push-snapshot-dev
  • GitHub Check: build-and-push-snapshot-dev4
  • GitHub Check: build-and-push-snapshot-staging
  • GitHub Check: build-and-push-snapshot-dev2
🔇 Additional comments (1)
.github/workflows/protocol-build-and-push-snapshot.yml (1)

1-154: Review comment is not applicable to this file.

The workflow file is a standard GitHub Actions configuration for building and pushing Docker images. The referenced geo-headers-middleware.ts file exists in the repository at ./indexer/services/comlink/src/request-helpers/geo-headers-middleware.ts. While this workflow triggers on the jv/expose-geo-headers branch (line 6), it performs only generic build and push operations unrelated to geo-headers implementation details. PR documentation completeness and related implementation files are not directly relevant to reviewing this workflow configuration.

Likely an incorrect or invalid review comment.

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.

2 participants