[codex] Use WebUI base URL for OAuth callback origins

[henrypark133]

Summary

• resolves hosted WebUI OAuth callback origins only from IRONCLAW_REBORN_WEBUI_BASE_URL
• feeds that hosted origin into Notion DCR/product-auth callback wiring before local listener fallback
• rejects empty normalized base URL values and cleartext public callback origins on non-loopback binds
• documents that WebUI login and product-auth OAuth flows share the Reborn WebUI base URL

Root Cause

Reborn WebUI serve wired Notion DCR OAuth callback origin from [webui].canonical_host or the local listener address. A public deployment with a wildcard/non-loopback listener and no canonical host could therefore produce provider-visible local callback origins unless the Reborn WebUI base URL was explicitly used by every OAuth surface.

Impact

Hosted WebUI login and product-auth OAuth callbacks now use the same Reborn-scoped public base URL when IRONCLAW_REBORN_WEBUI_BASE_URL is configured. Local loopback behavior is preserved when no public base URL is set.

Validation

• cargo fmt --check
• git diff --check
• cargo test -p ironclaw_reborn_cli --features webui-v2-beta webui_ -- --nocapture
• cargo clippy -p ironclaw_reborn_cli --features webui-v2-beta --all-targets -- -D warnings

Fixes #4928

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Documentation
  • Expanded WebUI OAuth/SSO environment variable guidance, including trailing-slash handling, requiring https:// for public/non-loopback deployments, and specific Notion MCP setup notes.
  • Added Railway deployment instructions to use the public hosted WebUI base URL for provider callback origins.
• Improvements
  • WebUI OAuth/SSO redirect/callback origin logic now prefers an explicitly configured hosted base URL (normalized) for public deployments.
  • Startup now fails for invalid/blank hosted base URL values, and rejects insecure public http:// origins when not running locally.
• Tests
  • Updated and extended coverage for hosted base URL precedence, validation behavior, and Notion DCR callback origin wiring.

Walkthrough

Fixes Notion MCP OAuth localhost-redirect bug on Railway (#4928) by centralizing WebUI base URL resolution in serve_sso.rs. New helpers read and normalize the explicit IRONCLAW_REBORN_WEBUI_BASE_URL env var (fail startup if blank after trim), falling back to listener-address-derived http:// when unset. Result threads through webui_oauth_callback_origin as public_base_url, short-circuiting canonical-host/loopback logic when present. Comprehensive test coverage added; docs updated for Railway and Notion MCP flows.

Changes

Railway OAuth Callback URL Fix

Layer / File(s)	Summary
Base URL resolution helpers crates/ironclaw_reborn_cli/src/commands/serve_sso.rs	Adds WEBUI_BASE_URL_ENV constant and dedicated test WEBUI_BASE_URL_ENV_LOCK. Implements webui_oauth_base_url_from_env (explicit env precedence or listener fallback), webui_public_base_url_from_env (Option-returning read), and normalize_base_url (trim + remove trailing slash, fail startup if empty). sso_startup_config_from_env delegates to new helper. Promotes is_cleartext_http_scheme to pub(crate). Updates reject_cleartext_oauth error message text.
Callback origin function and wiring crates/ironclaw_reborn_cli/src/commands/serve.rs	webui_notion_dcr_callback_origin reads and validates public base URL from IRONCLAW_REBORN_WEBUI_BASE_URL env. webui_oauth_callback_origin gains public_base_url: Option<&str> parameter: when present, sanitizes (trim, remove /), rejects empty/non-loopback cleartext http://, returns sanitized value; when absent, preserves canonical-host/loopback logic (HTTPS/HTTP selection, IPv6 brackets, loopback fallback). serve command routes through webui_notion_dcr_callback_origin. Test scaffolding adds env constant and clear_webui_env helper.
Tests for callback origin crates/ironclaw_reborn_cli/src/commands/serve.rs	Existing unit tests updated to pass None for public_base_url. New tests assert: public_base_url precedence over canonical-host/loopback, cleartext http:// rejection on non-loopback, loopback http:// preservation. New async integration test verifies Notion DCR wiring with env-derived callback origin (trimmed/sanitized). New fail-closed tests reject slash-only and cleartext http:// env values with error text assertions.
Tests for base URL resolution crates/ironclaw_reborn_cli/src/commands/serve_sso.rs	Env cleanup and serialization switched to WEBUI_BASE_URL_ENV_LOCK. New tests for validate_webui_public_base_url verify fail-closed rejection of public cleartext URLs. New comprehensive tests verify: explicit env preference, blank/only-/ rejection, integration with sso_startup_config_from_env, listener-address fallback when env unset.
Documentation README.md, .env.example, docs/reborn/deploy-reborn-cli-docker.md	README reorganizes WebUI OAuth section, expands IRONCLAW_REBORN_WEBUI_BASE_URL guidance to cover Notion MCP and product-auth flows with trailing-slash and https:// clarity. Railway deployment doc explains that Notion MCP OAuth reuses WebUI base URL for callback origins (Google remains explicit). .env.example clarifies base URL scope includes product-auth, explicitly Notion MCP.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A callback once lost to localhost's void,
now finds its way home — Railway deployed!
IRONCLAW_REBORN_WEBUI_BASE_URL steps in with grace,
trimmed and validated, the right time and place.
No more ERR_CONNECTION_REFUSED despair —
Notion OAuth breathes fresh deployed air. 🚂✨

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	Title lacks Conventional Commits format (type(scope): summary); uses bracket notation instead of standard type(scope) style.	Reformat to Conventional Commits style, e.g., 'feat(webui-oauth): use configured base URL for callback origins'.
Description check	⚠️ Warning	Description lacks required template structure: missing 'Change Type' checkboxes, 'Validation' section details, 'Reborn Trust-Boundary Checklist', and 'Review track' designation.	Complete PR description using the required template, including Security Impact, Trust-Boundary Checklist items, and explicit Review track assignment.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Changes correctly resolve #4928: webui_oauth_callback_origin now prioritizes IRONCLAW_REBORN_WEBUI_BASE_URL over canonical_host/listen_addr, reject_cleartext_oauth prevents non-loopback http origins, and Notion DCR wiring uses the new logic.
Out of Scope Changes check	✅ Passed	All changes align with #4928 scope: OAuth callback origin resolution, base URL normalization, cleartext rejection, and documentation updates. No unrelated refactoring or feature creep detected.

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

---

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

[gemini-code-assist]

Code Review

This pull request updates the WebUI OAuth base URL resolution logic to support falling back to the RAILWAY_PUBLIC_DOMAIN environment variable when IRONCLAW_REBORN_WEBUI_BASE_URL is unset, preventing hosted deployments on Railway from defaulting to local listener addresses. The reviewer suggests validating and rejecting malformed or empty base URLs with a configuration error rather than attempting to normalize them, to improve security for these sensitive inputs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[henrypark133]

Code Review (multi-agent)

Intent: Fix Railway WebUI OAuth callback origin resolution so hosted Notion/product-auth flows use the public Railway domain while preserving local fallback behavior.

Stats: 4 findings (from 5 raw, 4 after validation/dedup) across 2 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 2. Dropped during aggregation: 1 false positive.

Tests

1. Medium Missing caller-level test for Railway base URL wiring (crates/ironclaw_reborn_cli/src/commands/serve_sso.rs:55-64, confidence 88) — anchor: crates/ironclaw_reborn_cli/src/commands/serve_sso.rs:63
   The new Railway fallback is only exercised through webui_oauth_base_url_from_env. There is no test that drives sso_startup_config_from_env with a configured provider and RAILWAY_PUBLIC_DOMAIN set, so the actual startup path that feeds base_url into SSO config is still unverified.

2. Medium Normalized-empty base URL env values are not covered (crates/ironclaw_reborn_cli/src/commands/serve_sso.rs:91-107, confidence 97) — anchor: crates/ironclaw_reborn_cli/src/commands/serve_sso.rs:97
normalize_base_url can turn non-empty env input like " / " or "/" into an empty string, and webui_public_base_url_from_env currently returns that empty value instead of falling back. That leaves OAuth callback wiring with an invalid empty origin for a typo that passed non_empty_env.

Conventions

1. Medium Update the WebUI env example for the Railway base-URL fallback (.env.example:413-417, confidence 86) — anchor: CLAUDE.md:262 (no diff position — body only)
   The WebUI env example still says the only fallback after IRONCLAW_REBORN_WEBUI_BASE_URL is the bound listener address, but this branch now also consumes RAILWAY_PUBLIC_DOMAIN as a hosted base URL. CLAUDE.md:262 says .env.example is the inventory for environment variables, so the documented callback-origin precedence is now incomplete.

Approach

1. Low Railway fallback adds a second public-origin source of truth (crates/ironclaw_reborn_cli/src/commands/serve_sso.rs:91-94, confidence 64) — anchor: README.md:276-293 (body only)
   This change adds RAILWAY_PUBLIC_DOMAIN as a platform-specific source of hosted callback origins alongside the existing Reborn-scoped IRONCLAW_REBORN_WEBUI_BASE_URL. That may be the desired Railway convenience, but it is a second precedence rule operators must learn and keep documented.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_reborn_cli/src/commands/serve.rs`:
- Around line 576-578: The code in the public_base_url check does not validate
that the URL uses a secure HTTPS scheme before returning it, which creates a
security vulnerability for non-loopback deployments where HTTP (cleartext)
values could be used as OAuth callback origins. Add scheme validation to reject
cleartext HTTP URLs unless the deployment is on a loopback address (like
localhost or 127.0.0.1). Parse the URL scheme from public_base_url and only
return the trimmed base URL if it uses HTTPS for non-loopback cases, or HTTP for
loopback cases; otherwise return None to fail closed on the security check as
required for authentication-related functionality.

In `@README.md`:
- Around line 293-299: Update the documentation section in README.md that
describes the fallback behavior when IRONCLAW_REBORN_WEBUI_BASE_URL is omitted.
Clarify that the runtime precedence first checks RAILWAY_PUBLIC_DOMAIN before
falling back to the actual listener address (such as http://127.0.0.1:3000).
This will accurately reflect the current behavior and prevent misleading Railway
operators about how the base URL resolution works in their deployments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fe49e8c0-aaa1-4cb6-aae1-5915206f9788

📥 Commits

Reviewing files that changed from the base of the PR and between 1553e0d and 5dd53f1.

📒 Files selected for processing (4)

• README.md
• crates/ironclaw_reborn_cli/src/commands/serve.rs
• crates/ironclaw_reborn_cli/src/commands/serve_sso.rs
• docs/reborn/deploy-reborn-cli-docker.md

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn_cli/src/commands/serve.rs (1)

577-612: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

is_cleartext_http_scheme duplicated across modules.

Identical helper exists at serve_sso.rs:154-158. Extract to a shared location (e.g., a url_util submodule or promote serve_sso::is_cleartext_http_scheme to pub(crate)).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_reborn_cli/src/commands/serve.rs` around lines 577 - 612, The
`is_cleartext_http_scheme` function in serve.rs is identical to a function with
the same name in serve_sso.rs, creating duplication. Extract this function to a
shared location by either creating a new url_util submodule or promoting the
existing implementation in serve_sso.rs to pub(crate) visibility, then import
and use the shared version in serve.rs instead of the local duplicate
implementation. Remove the duplicate `is_cleartext_http_scheme` function
definition from the current location.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/ironclaw_reborn_cli/src/commands/serve.rs`:
- Around line 577-612: The `is_cleartext_http_scheme` function in serve.rs is
identical to a function with the same name in serve_sso.rs, creating
duplication. Extract this function to a shared location by either creating a new
url_util submodule or promoting the existing implementation in serve_sso.rs to
pub(crate) visibility, then import and use the shared version in serve.rs
instead of the local duplicate implementation. Remove the duplicate
`is_cleartext_http_scheme` function definition from the current location.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 357d9581-5a42-4dc2-8a06-c48bdf1b9f0e

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd53f1 and 5f2f41c.

📒 Files selected for processing (4)

• .env.example
• crates/ironclaw_reborn_cli/src/commands/serve.rs
• crates/ironclaw_reborn_cli/src/commands/serve_sso.rs
• docs/reborn/deploy-reborn-cli-docker.md

[henrypark133]

Code Review (multi-agent)

Intent: Ensure hosted Reborn WebUI and product-auth OAuth callbacks use IRONCLAW_REBORN_WEBUI_BASE_URL while preserving local loopback fallback.

Stats: 4 findings (from 8 raw, 4 after dedup) across 1 file. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 1.

Bugs

1. Medium Cleartext public base URL disables product auth instead of failing (crates/ironclaw_reborn_cli/src/commands/serve.rs:582-583, confidence 75) — anchor: crates/ironclaw_reborn_cli/src/commands/serve.rs:582
   When IRONCLAW_REBORN_WEBUI_BASE_URL is explicitly set to an http:// public URL on a non-loopback bind, the new product-auth callback path returns None. The caller only logs a warning and continues, so a product-auth-only deployment with no WebUI SSO provider starts without wiring Notion DCR OAuth instead of rejecting the bad hosted callback origin. Also flagged by: tests/Low.

Tests

1. Medium No caller-level test proves serve reads the WebUI base URL (crates/ironclaw_reborn_cli/src/commands/serve.rs:244-256, confidence 75) — anchor: .claude/rules/testing.md:43
   The new wiring depends on ServeCommand reading IRONCLAW_REBORN_WEBUI_BASE_URL and passing it into webui_oauth_callback_origin, but the added tests call helpers with hardcoded inputs. A regression that drops the env read, passes None, or loses invalid-base-url error context at the serve-level caller would still pass. Also flagged by: conventions/Medium, tests/Medium, maintainability/Low.

Conventions

1. Low Cleartext scheme helper is duplicated (crates/ironclaw_reborn_cli/src/commands/serve.rs:608-612, confidence 75) — anchor: CLAUDE.md:25
   The new is_cleartext_http_scheme duplicates the same scheme-normalization helper already present in serve_sso.rs, including the security-sensitive case-insensitive http handling. Keeping two private copies makes future cleartext-guard fixes easy to apply to only one OAuth path.

Local Patterns

1. Low Warning hides explicit public base URL misconfiguration (crates/ironclaw_reborn_cli/src/commands/serve.rs:259-263, confidence 75) — anchor: crates/ironclaw_reborn_cli/src/commands/serve_sso.rs:137 (no diff position — body only)
   The warning still says Notion DCR is skipped because the listener origin is not stable loopback, but the new public-base-url branch can also reach this path when IRONCLAW_REBORN_WEBUI_BASE_URL is cleartext on a non-loopback bind. If this path remains a warning rather than a fail-closed error, the diagnostic should name the explicit hosted base URL misconfiguration.

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn_cli/src/commands/serve_sso.rs (1)

93-96: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Treat an explicitly blank WebUI base URL as a startup error.

non_empty_env(WEBUI_BASE_URL_ENV) filters whitespace-only values to None, so IRONCLAW_REBORN_WEBUI_BASE_URL=" " bypasses normalize_base_url() and falls back to the listener-derived URL. That masks a bad explicit setting and can reintroduce the localhost/unspecified-listener callback path downstream.

As per coding guidelines, “Fail loud by default with ? operator for DB, IO, workspace, and settings reads” and “Fail closed for auth.”

Fail closed for present-but-blank env values

 pub(crate) fn webui_public_base_url_from_env() -> anyhow::Result<Option<String>> {
-    if let Some(raw) = non_empty_env(WEBUI_BASE_URL_ENV) {
-        return Ok(Some(normalize_base_url(WEBUI_BASE_URL_ENV, raw)?));
+    match env::var(WEBUI_BASE_URL_ENV) {
+        Ok(raw) => Ok(Some(normalize_base_url(WEBUI_BASE_URL_ENV, raw)?)),
+        Err(env::VarError::NotPresent) => Ok(None),
+        Err(env::VarError::NotUnicode(_)) => {
+            anyhow::bail!("{WEBUI_BASE_URL_ENV} must be valid UTF-8")
+        }
     }
-    Ok(None)
 }

     fn webui_public_base_url_from_env_rejects_blank_normalized_values() {
         let _guard = WEBUI_BASE_URL_ENV_LOCK.lock().expect("env lock");
         clear_sso_env();
+        // SAFETY: serialized by WEBUI_BASE_URL_ENV_LOCK; cleaned up before the guard drops.
+        unsafe {
+            std::env::set_var(WEBUI_BASE_URL_ENV, "   ");
+        }
+        assert!(
+            webui_public_base_url_from_env().is_err(),
+            "whitespace-only explicit base URL must fail closed"
+        );
+
         // SAFETY: serialized by WEBUI_BASE_URL_ENV_LOCK; cleaned up before the guard drops.
         unsafe {
             std::env::set_var(WEBUI_BASE_URL_ENV, "/");
         }

Also applies to: 510-531

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_reborn_cli/src/commands/serve_sso.rs` around lines 93 - 96,
The webui_public_base_url_from_env() function currently uses non_empty_env()
which filters whitespace-only values to None, allowing an explicitly set but
blank IRONCLAW_REBORN_WEBUI_BASE_URL environment variable to bypass validation
and fall back to a default URL. This masks a bad explicit setting and violates
the coding guidelines to fail loud for settings reads. Instead, check if the
environment variable is actually present in the environment first, and if it is
present, pass it to normalize_base_url() for validation even if it contains only
whitespace, ensuring that an explicitly set blank value triggers an error rather
than silently using a fallback. Apply the same fix pattern to similar functions
in the 510-531 range that handle environment variable validation for auth or
settings.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/ironclaw_reborn_cli/src/commands/serve_sso.rs`:
- Around line 93-96: The webui_public_base_url_from_env() function currently
uses non_empty_env() which filters whitespace-only values to None, allowing an
explicitly set but blank IRONCLAW_REBORN_WEBUI_BASE_URL environment variable to
bypass validation and fall back to a default URL. This masks a bad explicit
setting and violates the coding guidelines to fail loud for settings reads.
Instead, check if the environment variable is actually present in the
environment first, and if it is present, pass it to normalize_base_url() for
validation even if it contains only whitespace, ensuring that an explicitly set
blank value triggers an error rather than silently using a fallback. Apply the
same fix pattern to similar functions in the 510-531 range that handle
environment variable validation for auth or settings.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7d73f4bb-92ee-49c3-a694-08548cb648ba

📥 Commits

Reviewing files that changed from the base of the PR and between 5f2f41c and 632f566.

📒 Files selected for processing (2)

• crates/ironclaw_reborn_cli/src/commands/serve.rs
• crates/ironclaw_reborn_cli/src/commands/serve_sso.rs