Skip to content

chore: promote staging to staging-promote/5a5beec1-25040320859 (2026-04-28 11:15 UTC) - #3018

Merged
henrypark133 merged 1 commit into
mainfrom
staging-promote/2476672a-25049608611
Apr 29, 2026
Merged

henrypark133 merged 1 commit into
mainfrom
staging-promote/2476672a-25049608611

Conversation

@ironclaw-ci

@ironclaw-ci ironclaw-ci Bot commented Apr 28, 2026 •

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: 7fb41555a9e55677d1aaea29ca567a5b369c2b05..2476672a5dc88575bc67aff1fb5ef292c8e119e3
Promotion branch: staging-promote/2476672a-25049608611
Base: staging-promote/5a5beec1-25040320859
Triggered by: Staging CI batch at 2026-04-28 11:15 UTC

Commits in this batch (104):

Current commits in this promotion (1)

Current base: staging-promote/5a5beec1-25040320859
Current head: staging-promote/2476672a-25049608611
Current range: origin/staging-promote/5a5beec1-25040320859..origin/staging-promote/2476672a-25049608611

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

@github-actions github-actions Bot added scope: channel/web Web gateway channel size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 28, 2026
@claude

claude Bot commented Apr 28, 2026

Copy link
Copy Markdown

Code review

Found 7 issues:

  1. [CRITICAL:90] Secret token exposed via .to_string() conversion

    • Line 575 converts SecretString to plain String via .expose_secret().to_string(), breaking the secrecy invariant
    • If api_key is later logged, stringified, or Debug-printed, the session token leaks
    • Fix: Use .expose_secret().as_str() instead of .to_string(), or keep the type as SecretString throughout

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L573-L577

  2. [HIGH:80] Clippy await_holding_lock suppressed without fixing the underlying issue

    • Line 1115: #[allow(clippy::await_holding_lock)] suppresses the warning but the code still holds a Mutex across .await points
    • The lock is held through full HTTP request processing, risking contention and deadlock
    • Fix: Acquire lock, read/set env vars, release lock, then run the handler

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L1115-L1235

  3. [HIGH:75] Unchecked await in token retrieval creates silent failure path

    • Line 573: && let Ok(token) = session.get_token().await silently ignores retrieval errors
    • If get_token() fails after has_token() succeeds, api_key remains unset and the handler proceeds with None, resulting in a missing Authorization header (401)
    • Fix: Add explicit error handling or propagate the error

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L569-L575

  4. [MEDIUM:65] Missing timeout on SessionManager async calls in production handler

    • Lines 571-572: session.has_token().await and session.get_token().await called without timeout on critical LLM configuration endpoint
    • If SessionManager I/O stalls, the HTTP handler hangs indefinitely
    • Fix: Wrap with tokio::time::timeout(Duration::from_secs(N))

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L569-L575

  5. [MEDIUM:70] Race condition between has_token() and get_token() checks

    • Lines 571-573: Between the has_token() check and get_token() call, another task could clear the session
    • While the error is handled, the implicit assumption of synchronous state across awaits is fragile
    • Fix: Combine into a single atomic operation or add explicit synchronization

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L569-L575

  6. [MEDIUM:65] Unsafe env var manipulation in test with inadequate synchronization

    • Lines 1120-1121: unsafe { std::env::set_var() / remove_var() } with lock_env() guard can still race if lock is released before code execution
    • Fix: Ensure lock is held through the actual handler call, or use a dedicated env wrapper instead of unsafe

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L1108-L1130

  7. [MEDIUM:65] Missing error logging on session token retrieval failure

    • Line 573: Silent error handling makes debugging difficult if session token is corrupted or unavailable
    • Fix: Add debug!() logging when get_token() fails for observability

    https://github.com/anthropics/ironclaw/blob/2476672a5dc88575bc67aff1fb5ef292c8e119e3/src/channels/web/handlers/llm.rs#L569-L575


Summary: The feature is architecturally sound, but the critical secret-exposure issue at line 575 must be fixed before merge. The timeout and error handling issues (items 3-5) also need addressing per production readiness standards.

Base automatically changed from staging-promote/5a5beec1-25040320859 to main April 29, 2026 04:09
@henrypark133
henrypark133 merged commit 2476672 into main Apr 29, 2026
61 of 67 checks passed
@henrypark133
henrypark133 deleted the staging-promote/2476672a-25049608611 branch April 29, 2026 04:09

This branch had an error being deployed

1 failed and 5 inactive deployments
Ironclaw-QA / production — 2476672a Deployed Apr 28, 2026 by railway-app[bot]
ironclaw-nearai / production — 2476672a Deployed Apr 28, 2026 by railway-app[bot]
venice-ironclaw / production — 2476672a Deployed Apr 28, 2026 by railway-app[bot]
cosmose-ironclaw / production — 2476672a Deployed Apr 28, 2026 by railway-app[bot]
humble-cat / staging-cameron — 2476672a Deployed Apr 28, 2026 by railway-app[bot]
Near Foundation Ironclaw / production — 2476672a Deployed Apr 28, 2026 by railway-app[bot]
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/web Web gateway channel size: L 200-499 changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants