Skip to content

chore: promote staging to staging-promote/4dbb44cf-24369614290 (2026-04-14 07:31 UTC) - #2445

Merged
henrypark133 merged 1 commit into
mainfrom
staging-promote/532fc61d-24386706612
Apr 18, 2026
Merged

henrypark133 merged 1 commit into
mainfrom
staging-promote/532fc61d-24386706612

Conversation

@ironclaw-ci

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

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: a53eac5c2dec6b6cd5c08189086093fde64aa9cb..532fc61d2502c2f4299f2ba2bb27b1189f08a54a
Promotion branch: staging-promote/532fc61d-24386706612
Base: staging-promote/4dbb44cf-24369614290
Triggered by: Staging CI batch at 2026-04-14 07:31 UTC

Commits in this batch (20):

Current commits in this promotion (1)

Current base: staging-promote/4dbb44cf-24369614290
Current head: staging-promote/532fc61d-24386706612
Current range: origin/staging-promote/4dbb44cf-24369614290..origin/staging-promote/532fc61d-24386706612

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

…1963)

* feat(web): add admin management panel

* fix(web): address admin panel review findings

* fix(web): address remaining admin review feedback

* refactor(web): type admin api responses

* fix(db): aggregate admin usage summary in sql

* Add audit logging for admin privileged state-changes

Add structured tracing (warn-level) to suspend, activate, delete, and
update handlers so that privileged admin actions are recorded with the
acting admin's user_id, the action performed, and the target user.
Addresses security assessment item #1 from PR review.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix PairingStore::new() call in test after staging merge

Use PairingStore::new_noop() since the test doesn't need a real DB.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(admin): address PR #1963 review feedback

- Fix total_jobs semantics: query agent_jobs directly instead of
  counting via LEFT JOIN on llm_calls (which missed jobs without
  LLM calls). Fixed in both libSQL and PostgreSQL backends.
- Fix showConfirmModal XSS: escape message parameter internally
  instead of relying on callers to sanitize.
- Add explicit ::numeric cast to PG COALESCE(SUM(cost), 0) to
  prevent integer type inference.
- Use info! instead of warn! for successful admin audit events
  (update, suspend, activate, delete) — warn implies anomaly.
- Add missing index on llm_calls.created_at for both PG (V21
  migration) and libSQL (incremental migration 21).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address remaining admin panel review follow-ups

* fix: address review comments — query consolidation, CSP, docs, security notes

- Collapse 4 redundant llm_calls subqueries into single subquery (libsql + pg)
- Add WARNING to V21 migration about table lock risk with CONCURRENTLY note
- Add performance doc comments on admin_usage_summary full-table scan
- Add CSP and noindex meta tags to admin.html
- Add JSDoc for showConfirmModal documenting auto-escaping
- Add sessionStorage threat model security comment
- Add serde(flatten) collision risk doc on AdminUserDetailResponse
- Add TODO(#1968) for inline styles migration to CSS custom properties
- Add PG parity test stub for admin_usage_summary

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: renumber migration from V21/23 to V24 to avoid conflicts with staging

Staging added V21 (backfill_conversation_source_channel), V22
(sandbox_restart_params), and V23 (list_workspace_files_escape_like).
Renumber our llm_calls_created_at_index migration to V24 in both PG
and libSQL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback — CSP, logging, validation, dispatch-exempt, tests

- Remove 'unsafe-inline' from script-src CSP; move CSP to HTTP response header
- Change audit tracing::info! to tracing::debug! (TUI corruption)
- Add dispatch-exempt annotation on usage_summary_handler
- Add server-side input validation on users_create_handler (name length, email, role)
- Rename detailRowHtml to detailRowRawHtml with XSS safety comment
- Add real PG integration test for admin_usage_summary with non-zero data

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): skip test-only directories in no-panics check

Files under `src/**/tests/*.rs` are Rust test sub-modules included
behind `#[cfg(test)]` — they are never compiled in production builds.
The no-panics checker was flagging `.unwrap()` and `assert!()` in
helper functions at module level in these files as production code.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(admin): scope cost aggregates to 30d, drop external fonts, flatten detail response

Addresses review feedback on #1963:

- Scope all llm_calls aggregates to the 30d `since` window so the admin
  dashboard query is served by `idx_llm_calls_created_at` rather than a
  full table scan. Drops the unused all-time `total_cost` subquery from
  both libsql and postgres backends.
- Self-contain the admin SPA — remove `fonts.googleapis.com` /
  `fonts.gstatic.com` link tags from admin.html and tighten the admin
  CSP to fully same-origin. Typography degrades to the system-font
  fallback already listed in `font-family`.
- Fold `metadata` into `AdminUserInfo` (optional, skip-if-none) and
  remove the `#[serde(flatten)]` wrapper, eliminating the
  documented collision risk.
- Add regression test asserting `since` actually bounds the LLM
  aggregates (future `since` should yield zero LLM counts without
  affecting non-windowed counts).

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: ilblackdragon@gmail.com <ilblackdragon@gmail.com>
@github-actions github-actions Bot added scope: channel/web Web gateway channel scope: db Database trait / abstraction scope: db/postgres PostgreSQL backend scope: db/libsql libSQL / Turso backend scope: docs Documentation DB MIGRATION PR adds or modifies PostgreSQL or libSQL migration definitions size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 14, 2026
@claude

claude Bot commented Apr 14, 2026

Copy link
Copy Markdown

Code review

Found 7 issues:

  1. [CRITICAL:95] Column index mismatch in libSQL admin_usage_summary query

The admin_usage_summary() function mixes get_text() with index-based column access inconsistently. The function retrieves 9 columns but the mixing of accessor patterns suggests potential index misalignment. Line ~3402 in src/db/libsql/users.rs.

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/src/db/libsql/users.rs#L3398-L3410

  1. [HIGH:85] Client-side admin role check is cosmetic, relies entirely on server-side auth

The admin panel performs client-side role validation (profile.role !== 'admin'), but these checks are purely UX—the real protection is the AdminUser extractor on the server. When using OIDC proxy auth, the client doesn't send an Authorization header at all, trusting server-side session validation entirely. This is architecturally sound but depends on perfect backend auth implementation.

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/crates/ironclaw_gateway/static/admin.js#L1043-L1087

  1. [MEDIUM:92] CSP allows 'unsafe-inline' for styles, weakening XSS protection

The CSP header at server.rs permits style-src 'self' 'unsafe-inline', which partially defeats XSS protection. The code documents this as TODO (#1968) — inline styles are used throughout admin.js. This is known technical debt but still a security exposure.

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/src/channels/web/server.rs#L2829

  1. [MEDIUM:85] Email validation is insufficient for RFC 5322 compliance

The validation at src/channels/web/handlers/users.rs only checks !e.contains('@') || e.len() < 3, which accepts clearly invalid emails like a@b. When the email field is populated, it should be properly validated using a regex pattern or email validation crate. This could cause downstream issues if the database or external email services enforce stricter rules.

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/src/channels/web/handlers/users.rs#L2421-L2428

  1. [MEDIUM:85] Inefficient N+1 query pattern in users_detail_handler

The detail handler calls store.user_summary_stats(Some(&id)) after fetching the user record, creating an additional database round-trip. The list handler fetches all stats once with store.user_summary_stats(None). This inconsistency causes latency degradation under high admin panel usage. Consider pre-fetching stats or eliminating the separate call.

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/src/channels/web/handlers/users.rs#L2360-L2375

  1. [MEDIUM:80] Unbounded string concatenation in Admin Panel JavaScript

The admin.js file builds HTML through repeated string concatenation in loops (e.g., renderUsage iterates and concatenates <tr> rows). With hundreds of users or usage entries, this creates excessive temporary allocations and GC pressure. No virtual scrolling or DOM batching for large datasets. Consider building a DocumentFragment or using template literals with .join().

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/crates/ironclaw_gateway/static/admin.js#L650-L700

  1. [MEDIUM:75] Multiple scalar subqueries in admin_usage_summary cause repeated full table scans

The SQL query executes 5 separate scalar subqueries: (SELECT COUNT(*) FROM users), (SELECT COUNT(*) FROM users WHERE status = 'active'), etc. Under large user counts (10k+ users), each scan is inefficient. Consider a single multi-group aggregation query to reduce database load. Query performance scales poorly as user count grows.

https://github.com/anthropics/ironclaw/blob/532fc61d2502c2f4299f2ba2bb27b1189f08a54a/src/db/libsql/users.rs#L3360-L3410


Summary: The CRITICAL issue (column mismatch) needs immediate investigation. The MEDIUM issues (N+1 query, multiple subqueries, string concatenation) are performance/efficiency concerns. The email validation and CSP issues are known/documented but should be addressed incrementally. All auth and architecture patterns are sound.

Base automatically changed from staging-promote/4dbb44cf-24369614290 to main April 18, 2026 00:59
@henrypark133
henrypark133 merged commit 532fc61 into main Apr 18, 2026
36 of 44 checks passed
@henrypark133
henrypark133 deleted the staging-promote/532fc61d-24386706612 branch April 18, 2026 01:00

This branch had an error being deployed

1 failed and 2 inactive deployments
ironclaw-nearai / production — 532fc61d Deployed Apr 14, 2026 by railway-app[bot]
venice-ironclaw / production — 532fc61d Deployed Apr 14, 2026 by railway-app[bot]
humble-cat / staging-cameron — 532fc61d Deployed Apr 14, 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 DB MIGRATION PR adds or modifies PostgreSQL or libSQL migration definitions risk: medium Business logic, config, or moderate-risk modules scope: channel/web Web gateway channel scope: db/libsql libSQL / Turso backend scope: db/postgres PostgreSQL backend scope: db Database trait / abstraction scope: docs Documentation size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants