Skip to content

chore: promote staging to staging-promote/ee6f5cd6-23354122351 (2026-03-20 19:41 UTC)#1483

Open
ironclaw-ci[bot] wants to merge 1 commit intostaging-promote/ee6f5cd6-23354122351from
staging-promote/d3b69e7b-23359661011
Open

chore: promote staging to staging-promote/ee6f5cd6-23354122351 (2026-03-20 19:41 UTC)#1483
ironclaw-ci[bot] wants to merge 1 commit intostaging-promote/ee6f5cd6-23354122351from
staging-promote/d3b69e7b-23359661011

Conversation

@ironclaw-ci
Copy link
Contributor

@ironclaw-ci ironclaw-ci bot commented Mar 20, 2026

Auto-promotion from staging CI

Batch range: c4ab382522c86e7e19d55fee760b125fb1970518..d3b69e7be35217ebb6f6ce9fb3547402c862797e
Promotion branch: staging-promote/d3b69e7b-23359661011
Base: staging-promote/ee6f5cd6-23354122351
Triggered by: Staging CI batch at 2026-03-20 19:41 UTC

Commits in this batch (17):

Current commits in this promotion (1)

Current base: staging-promote/ee6f5cd6-23354122351
Current head: staging-promote/d3b69e7b-23359661011
Current range: origin/staging-promote/ee6f5cd6-23354122351..origin/staging-promote/d3b69e7b-23359661011

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

* Fix CI approval flows and stale fixtures

* Backfill approval thread mapping across channels
@github-actions github-actions bot added scope: agent Agent core (agent loop, router, scheduler) scope: docs Documentation size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 20, 2026
@claude
Copy link

claude bot commented Mar 20, 2026

Code review

Found 8 issues:

  1. [CRITICAL:100] Cross-channel approval thread hijacking - authorization bypass
    An attacker can approve/deny tool requests on threads they didn't initiate by supplying the target thread UUID to the approval endpoint. The code at line 1039 only checks if a thread exists in the session without verifying channel authorization context or ownership of the approval request. This allows potential escalation of privileges across channels.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1033-L1051

  1. [CRITICAL:95] TOCTOU race condition in approval thread resolution
    The code checks thread existence and mutates session state (lines 1040-1041), then drops the lock before completing the async register_thread() call (lines 1042-1050). This violates the session locking invariant documented in src/agent/CLAUDE.md:127 and creates a window where concurrent operations could modify session state after the mutations but before the thread is registered. The entire check-mutate-register sequence must complete while holding the write lock.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1038-L1051

  1. [HIGH:70] Incomplete fallback logic for non-existent approval threads
    When an approval targets a UUID that doesn't exist in the local session, the code falls back to resolve_thread() with the same UUID string (line 1053). But resolve_thread() expects either None or a conversation_scope value, not a UUID. This fallback may not correctly handle or create the intended thread.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1052-L1062

  1. [MEDIUM:85] Unnecessary repeated UUID parsing in hot path
    The approval thread UUID is parsed from string on every approval submission via Uuid::parse_str(thread_id).ok() (lines 1022-1031). For high-frequency approval workloads, this repeated parsing could accumulate. Consider caching the parsed UUID or parsing once during message deserialization.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1022-L1031

  1. [MEDIUM:80] Lock contention in approval thread resolution
    Multiple sequential lock acquisitions: get_or_create_session() write lock, session.lock(), then register_thread() with additional locks (lines 1036-1050). The explicit drop(sess) hints at intentional lock management but suggests the lock pattern needs review. Under high approval load, this sequential locking could cause contention.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1036-L1050

  1. [MEDIUM:70] Missing architectural trait for approval submission routing
    The code hardcodes a type check for Submission::ExecApproval { .. } | Submission::ApprovalResponse { .. } to detect approval submissions that can bypass normal thread resolution (lines 1022-1031). Per CLAUDE.md architecture guidelines, trait-based design is preferred over hardcoding conditionals. A method like submission.can_target_existing_thread() would make the intent explicit.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1022-L1031

  1. [MEDIUM:75] Test URLs changed to use public IPs
    Test URLs in config tests changed from https://custom.example.com to https://8.8.8.8 and similar. While these are test fixtures, using public IP addresses (Google DNS) in test mocking could accidentally expose real network calls if test isolation fails. Use RFC 5737 TEST-NET addresses (192.0.2.x) or localhost instead.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/config/embeddings.rs#L302
https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/config/llm.rs#L858

  1. [LOW:70] Unnecessary UTC time fetch on every approval thread reuse
    Line 1041 calls chrono::Utc::now() every time an existing approval thread is reused. This syscall is unnecessary for functionality and could accumulate under high approval frequency. Consider deferring to periodic heartbeat updates.

https://github.com/anthropics/ironclaw/blob/067ff01a2780fd91354b7902b4743316293beba0/src/agent/agent_loop.rs#L1040-L1041

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: agent Agent core (agent loop, router, scheduler) scope: docs Documentation size: L 200-499 changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant