Skip to content

fix: keep Slack React redirects public#234

Merged
onutc merged 1 commit intomainfrom
codex/slack-public-react-redirect
Apr 25, 2026
Merged

fix: keep Slack React redirects public#234
onutc merged 1 commit intomainfrom
codex/slack-public-react-redirect

Conversation

@onutc
Copy link
Copy Markdown
Member

@onutc onutc commented Apr 25, 2026

Summary

Slack settings redirects could send users to a Kubernetes service URL when the gateway used an internal Spritz API base URL.
This change separates browser redirects from service-to-service API calls.
The gateway can keep using the internal Spritz API URL, while React settings redirects use a public UI base URL.

What Changed

The Slack gateway now has a public React base URL for browser redirects.
It still uses the existing Spritz base URL for internal API and websocket calls.

  • Added optional SPRITZ_SLACK_REACT_BASE_URL.
  • Updated React route redirects and same-origin checks to use the React base URL.
  • Added a fallback that derives the public UI origin from the gateway public URL when the Spritz base URL is a Kubernetes service host.
  • Documented the new env var in .env.example.

Testing

The gateway tests cover the new config default and explicit React base behavior.

  • go test ./... from integrations/slack-gateway
  • git diff --check

Risks

Risk is low.
The API base URL still points at the same internal service, and only browser-facing React redirects move to the public base URL.
Cross-origin deployments can still set SPRITZ_SLACK_REACT_BASE_URL explicitly.

@onutc onutc merged commit f0bab9e into main Apr 25, 2026
7 checks passed
@onutc onutc deleted the codex/slack-public-react-redirect branch April 25, 2026 18:02
@gitrank-connector
Copy link
Copy Markdown

⭐ GitRank PR Analysis

Score: 50 points

Metric Value
Component Other (1× multiplier)
Severity P1 - High (50 base pts)
Final Score 50 × 1 = 50

Eligibility Checks

Check Status
Issue/Bug Fix
Fix Implementation
PR Documented
Tests
Lines Within Limit

Impact Summary

This PR fixes a critical configuration issue in the Slack gateway where browser redirects could inadvertently send users to internal Kubernetes service URLs when the gateway uses an internal Spritz API base URL. The fix separates browser-facing React redirects from internal service-to-service API calls by introducing an optional SPRITZ_SLACK_REACT_BASE_URL environment variable with intelligent fallback logic. The change maintains backward compatibility while preventing accidental exposure of internal infrastructure to end users.

Analysis Details

Component Classification: This PR affects the Slack gateway integration configuration and routing logic, which doesn't fit neatly into standard categorization. It's classified as OTHER since it's a cross-cutting infrastructure/configuration fix rather than a specific feature or component.

Severity Justification: This is a P1 (High) severity fix because it addresses a security/data integrity issue where users could be redirected to internal Kubernetes service URLs instead of public UI endpoints. This represents a significant risk of exposing internal infrastructure and breaking user experience in production deployments.

Eligibility Notes: Issue: True - PR clearly describes fixing a bug where Slack settings redirects could send users to internal Kubernetes URLs. Fix Implementation: True - Code changes properly implement the described solution with config separation and fallback logic. PR Linked: True - Comprehensive description with summary, changes, testing, and risk assessment. Tests: True - PR includes two new test cases (TestLoadConfigDefaultsReactBaseURLToPublicOriginWhenSpritzBaseURLIsClusterInternal and TestLoadConfigUsesExplicitReactBaseURL) plus one additional test (TestReactRouteURLUsesReactBaseURL). Tests Required: True - This is a bug fix in business logic (URL routing and configuration handling) that affects user-facing behavior and security, requiring comprehensive test coverage.


Analyzed by GitRank 🤖

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