Skip to content

chore: promote staging to staging-promote/7194808f-25051499413 (2026-04-29 03:13 UTC) - #3053

Merged
henrypark133 merged 1 commit into
mainfrom
staging-promote/8e54e51f-25089045324
Apr 29, 2026
Merged

henrypark133 merged 1 commit into
mainfrom
staging-promote/8e54e51f-25089045324

Conversation

@ironclaw-ci

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

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: 7fb41555a9e55677d1aaea29ca567a5b369c2b05..8e54e51f6201de593657fe8df57daf6efda12670
Promotion branch: staging-promote/8e54e51f-25089045324
Base: staging-promote/7194808f-25051499413
Triggered by: Staging CI batch at 2026-04-29 03:13 UTC

Commits in this batch (106):

Current commits in this promotion (0)

Current base: main
Current head: staging-promote/8e54e51f-25089045324
Current range: origin/main..origin/staging-promote/8e54e51f-25089045324

  • (no non-merge commits in range)

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(engine): centralize tool permission defaults

* fix(engine): preserve approval floors for v2 permissions

* fix(engine): address permission review cleanup

* fix(engine): avoid duplicate permission canonicalization
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel/web Web gateway channel scope: tool/builtin Built-in tools scope: config Configuration size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 29, 2026
@claude

claude Bot commented Apr 29, 2026

Copy link
Copy Markdown

Code review

Found 6 issues:

  1. [HIGH:85] Tool activation permission model change: tool_activate moved from AskEachTime to AlwaysAllow default

    This is a significant security model change. Previously, tool activation required per-action approval. The new seeded default allows silent activation by the LLM. While the test assertion (src/app.rs:183) documents this is intentional ("subgates control auth/setup"), this relaxes the approval boundary for a potentially destructive operation. Verify this aligns with security requirements and document the rationale in the commit message.

  2. [HIGH:85] HTTP tool moved to AlwaysAllow default, enabling silent SSRF vectors

    The HTTP tool was moved from AskEachTime to AlwaysAllow in the seeded defaults. This is a network-facing tool that should require explicit confirmation per-call. The LLM can now make arbitrary HTTP requests without user approval, expanding the SSRF attack surface.

  3. [HIGH:80] Inconsistent tool name normalization creates permission bypass risk

    seeded_default_permission_canonical() matches hardcoded canonical names (underscores), but effective_permission() normalizes between three variants (original, canonical, hyphenated). An attacker registering tools with confusing names could exploit the inconsistency. The old TOOL_RISK_DEFAULTS HashMap was similarly stringly-typed, but the new match-statement architecture is fragile — it requires manual sync whenever tools are added and has no compiler validation.

  4. [MEDIUM:75] Removed configured field loses visibility into user choice vs. seeded default

    ToolPermissionResolution.configured was removed. The new design conflates user-persisted permission with seeded baseline in the effective field. This loses the ability to distinguish "user explicitly chose this" from "inherited default", which may be needed for reset-to-default buttons, migration audits, or per-tenant policy enforcement.

  5. [MEDIUM:75] String allocations on every permission lookup

    effective_permission() allocates two new strings via .replace() on every call (canonical + hyphenated variants). This function is called once per tool in the filtering loop during LLM iteration. While the old LazyLock<HashMap> was O(1), the new match-based function adds string manipulation overhead.

  6. [MEDIUM:70] tool_permission_locked() helper lacks direct unit test coverage

    The new public function tool_permission_locked() is called from web handlers and extension tools but has no unit test. A regression to the empty-parameter check would only surface as integration test failures, not unit-tier catches.

Base automatically changed from staging-promote/7194808f-25051499413 to main April 29, 2026 04:09
@henrypark133
henrypark133 merged commit 8e54e51 into main Apr 29, 2026
60 of 68 checks passed
@henrypark133
henrypark133 deleted the staging-promote/8e54e51f-25089045324 branch April 29, 2026 04:09

This branch had an error being deployed

1 failed and 5 inactive deployments
cosmose-ironclaw / production — 8e54e51f Deployed Apr 29, 2026 by railway-app[bot]
ironclaw-nearai / production — 8e54e51f Deployed Apr 29, 2026 by railway-app[bot]
Ironclaw-QA / production — 8e54e51f Deployed Apr 29, 2026 by railway-app[bot]
Near Foundation Ironclaw / production — 8e54e51f Deployed Apr 29, 2026 by railway-app[bot]
venice-ironclaw / production — 8e54e51f Deployed Apr 29, 2026 by railway-app[bot]
humble-cat / staging-cameron — 8e54e51f Deployed Apr 29, 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: agent Agent core (agent loop, router, scheduler) scope: channel/web Web gateway channel scope: config Configuration scope: tool/builtin Built-in tools size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant