Skip to content

chore: promote staging to staging-promote/65062f3c-23317058602 (2026-03-19 23:06 UTC) - #1439

Merged
henrypark133 merged 1 commit into
staging-promote/65062f3c-23317058602from
staging-promote/c4ab3825-23321164063
Mar 20, 2026
Merged

henrypark133 merged 1 commit into
staging-promote/65062f3c-23317058602from
staging-promote/c4ab3825-23321164063

Conversation

@ironclaw-ci

@ironclaw-ci ironclaw-ci Bot commented Mar 19, 2026 •

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: 65062f3cc069ebbd29f6d9be874ae5eff796e43a..c4ab382522c86e7e19d55fee760b125fb1970518
Promotion branch: staging-promote/c4ab3825-23321164063
Base: staging-promote/65062f3c-23317058602
Triggered by: Staging CI batch at 2026-03-19 23:06 UTC

Commits in this batch (1):

Current commits in this promotion (1)

Current base: staging-promote/65062f3c-23317058602
Current head: staging-promote/c4ab3825-23321164063
Current range: origin/staging-promote/65062f3c-23317058602..origin/staging-promote/c4ab3825-23321164063

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

* Make hosted OAuth and MCP auth generic

* Address PR feedback and lint issues

* Suppress built-in Google secret in hosted proxy flows

* Align hosted OAuth secret suppression with proxy config

* Harden hosted OAuth callback helpers

* Tighten hosted OAuth URL rewriting
@github-actions github-actions Bot added scope: channel/cli TUI / CLI channel scope: channel/web Web gateway channel scope: llm LLM integration scope: extensions Extension management scope: docs Documentation size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 19, 2026
@claude

claude Bot commented Mar 19, 2026

Copy link
Copy Markdown

Code review

Found 8 issues:

  1. [CRITICAL:95] OAuth flow registration and lookup key mismatch

Flows are inserted into pending_oauth_flows using request.expected_state, but looked up using decoded_state.flow_id. For versioned states (ic2.*.* format), if decoding produces a different value than stored, the lookup will fail. In start_gateway_oauth_flow (line 998), flows are keyed by the raw nonce, but the callback handler retrieves them using the decoded flow_id (line 603). While the normal path should work correctly since encode_hosted_oauth_state() preserves the flow_id, the legacy fallback at lines 554-558 could cause mismatches if a state fails envelope validation.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/extensions/manager.rs#L995-L999

  1. [MEDIUM:85] Generalization of token exchange parameters loses type safety

The refactoring replaces exchange_oauth_code_with_resource(resource: Option<&str>) with exchange_oauth_code_with_params(extra_token_params: &HashMap<String, String>). While this is extensible, required parameters (like RFC 8707 resource for MCP) are no longer enforced at compile time. Callers could accidentally pass empty params where specific values are required. The function signature and struct lack documentation of required parameter contracts.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/cli/oauth_defaults.rs#L346-L356

  1. [MEDIUM:82] Legacy OAuth state validation is too loose

The fallback path in decode_hosted_oauth_state() (lines 554-558) accepts any non-empty string as a valid flow_id without further validation. This could cause log spam or confusion if attackers send crafted states that pass legacy decoding but don't exist in the registry. The new versioned format checks for empty flow_id (line 591), but legacy paths do not.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/cli/oauth_defaults.rs#L554-L558

  1. [MEDIUM:75] Downgrade attack risk on OAuth state validation

The decode_hosted_oauth_state() function accepts both the new versioned ic2.*.* envelope format and legacy instance:nonce formats without requiring cryptographic verification for the legacy formats. While nonces are still required to be pre-registered in pending_oauth_flows, a compromised or malicious OAuth provider could deliberately send legacy-format states that bypass the SHA256 checksum validation. This is a downgrade from the integrity protection of the new format.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/cli/oauth_defaults.rs#L515-L559

  1. [LOW:88] State parameter rewriting has fragile string fallback

In ExtensionManager::rewrite_oauth_state_param() (lines 1003-1006), if URL parsing fails, the code falls back to string replacement. This could corrupt query strings if expected_state appears elsewhere in the URL. For example, ...&state=abc&hint=state_is... with expected_state="state_is" would incorrectly rewrite the hint parameter.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/extensions/manager.rs#L949-L1006

  1. [LOW:86] No rate limiting on public /oauth/callback endpoint

The /oauth/callback endpoint is public and processes OAuth callbacks without per-client rate limiting (only global 30 req/60s applies). An attacker could send many OAuth callback attempts with different codes to the same flow_id, potentially saturating the in-memory pending_oauth_flows registry. The 5-minute expiry provides some protection, but explicit rate limiting on the callback handler would be more robust.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/channels/web/server.rs#L580-L614

  1. [LOW:75] OAuth state length leaks in logs

The redact_oauth_state_for_logs() function produces sha256:xxxxxx:len=N, exposing the state length. Different lengths could leak information about whether versioned (ic2.*.*) or legacy format was used, potentially aiding fingerprinting attacks.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/channels/web/server.rs#L67-L75

  1. [LOW:70] Callback URL normalization lacks input validation

The normalize_hosted_callback_url() helper (lines 893-906) performs string-based path normalization without validating the result. If the callback URL contains encoded slashes (%2F) or other special characters, the result could be invalid. URL parsing is preferred; string fallback is dangerous.

https://github.com/anthropics/ironclaw/blob/17f299a6d3aa76160de97f1d03ecc2d91f576909/src/extensions/manager.rs#L893-L906

🤖 Generated with Claude Code

@henrypark133
henrypark133 merged commit d5e08b9 into staging-promote/65062f3c-23317058602 Mar 20, 2026
55 of 56 checks passed
@henrypark133
henrypark133 deleted the staging-promote/c4ab3825-23321164063 branch March 20, 2026 17:10
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
…3321164063

chore: promote staging to staging-promote/65062f3c-23317058602 (2026-03-19 23:06 UTC)
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
…3321164063

chore: promote staging to staging-promote/840606a7-23317058602 (2026-03-19 23:06 UTC)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/cli TUI / CLI channel scope: channel/web Web gateway channel scope: docs Documentation scope: extensions Extension management scope: llm LLM integration size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant