Skip to content

fix(reborn): [PRODUCTION CHANGE] #5647/#5712 — tool-disclosure surface narrowed by allow-set (3 leak vectors), with regression + trust-boundary tests - #5659

Merged
henrypark133 merged 25 commits into
mainfrom
w6-disclosure-narrow
Jul 29, 2026
Merged

henrypark133 merged 25 commits into
mainfrom
w6-disclosure-narrow

Conversation

@henrypark133

@henrypark133 henrypark133 commented Jul 5, 2026 •

Copy link
Copy Markdown
Collaborator

⚠️ This PR changes production behavior (three fixes, all in the tool-disclosure security surface)

Fix 1 — #5647: bridge meta-tools survive narrowed allow-sets

CapabilitySurfaceProfileFilter stripped host-synthesized bridge ids (tool_search/tool_describe/tool_call) from narrowed-allowlist callers crossing the 32-tool bridging threshold, leaving the model a deferred catalog it couldn't query. Fixed via constructor-injected host_exempt_capability_ids (private, empty default, populated with exactly the 3 bridge ids at the single production construction site) + one permits() method mirroring the sibling filters. Unchanged: underlying-tool enforcement (proven by a zero-network-egress denial test), threshold computation, other filters.

Fix 2 — #5712: tool_search/tool_describe payloads narrowed (Closes #5712)

Review of fix 1 surfaced a deeper gap: the disclosure port ranked/described against the unnarrowed inner catalog, so a narrowed profile could read names/descriptions/schemas of tools it can't call. Fixed by threading the same CapabilitySurfaceProfileResolver production already uses for the profile filter into the disclosure decorator; tool_search/tool_describe re-resolve the caller's allow-set (before the state mutex — no lock-across-await) and filter result payloads by permits(id). Deliberate: the internal catalog stays unnarrowed so bridging-threshold semantics are unchanged; only disclosed payloads narrow. Fail-closed on resolver errors. tool_describe of a non-allowlisted id returns the byte-identical "unknown" outcome as a nonexistent id — no existence oracle. Zero ironclaw_loop_support edits.

Fix 3 — tool_search's own advertised description narrowed (f4d6456)

Third distinct leak vector, found reviewing fix 2: tool_search is always advertised to the model, and its description is a catalog index built (catalog_index_tool_search_description) from the unfiltered tool list — so a narrowed caller who could call tool_search saw every underlying tool name inlined into that description, regardless of allow-set. Fixed by making LoopCapabilityPortDecorator::decorate() async and threading an eagerly-resolved allow-set through select_active_set → advertised_bridge_tool_definitions → catalog_index_tool_search_description, so the advertised description is built from the permitted subset only. Fail-closed. Independent security review confirmed the async change touches all 3 production decorator impls + 2 test impls with no lock held across the new await, and that an unnarrowed (All) caller cannot be emptied by a resolver error (every All-path resolver is infallible; host build re-resolves and fails loudly before an emptied index reaches a live turn).

Tests (all green, 0 ignored — 15 in the disclosure suite)

All three fix designs were thermo-reviewed pre-implementation; all three implementations passed an independent 10-point security post-review (same-instance threading, fail-closed, no existence oracle, filter-before-limit, threshold untouched, async-decorate no-lock-across-await).

Known follow-up (non-blocking): the sole fallible resolver (GateBackedSubagentPromptMaterialSource::material_for_run) does real I/O, contradicting #5712's plan-review "no I/O" premise. The double-resolve (decorate + outer filter) could in principle diverge for an already-narrowed subagent caller under a transient I/O blip — pre-existing from #5712, never reachable for All callers, and fix 3 reduces the resolve-call count for the description path (N→0, pre-resolved). Tracked for a cache/single-resolve follow-up.

🤖 Generated with Claude Code

henrypark133 and others added 2 commits July 4, 2026 23:43
…losure groups

Adds an opt-in with_narrowed_capability_allow_set_for_bridged_test()
seam so a test can request a genuinely narrowed CapabilityAllowSet
while in Bridged tool-disclosure mode, instead of into_group's
existing forced CapabilityAllowSet::All workaround (which mirrors
production for every other bridged test and stays the default here).
Fails fast if the override is set without also selecting Bridged
mode, since it would otherwise silently no-op. Threaded through both
RebornIntegrationGroupBuilder and RebornIntegrationHarnessBuilder,
mirroring the existing with_tool_disclosure_bridged/off pattern.

No behavior change for any existing test: default None resolves to
today's forced-All path byte-for-byte.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nder a narrowed allow-set

Adds #[ignore]d bridged_mode_survives_narrowed_capability_allow_set,
using the new harness seam: Bridged mode + a >32-tool GithubIssueTools
catalog + a narrowed allow-set (github.get_repo only, not forced All).
CapabilitySurfaceProfileFilter runs outside (after)
ToolDisclosureCapabilityDecorator, so the synthetic ironclaw.tool_search
bridge id — not a real granted capability — gets stripped by the
narrowed allow-set, leaving the model with zero tools.

Verified RED at authoring time: fails at
assert_model_tools_contains(TOOL_SEARCH_NAME) with "saw []", not at
harness construction or the secondary assertion. Ships #[ignore]d
(delivery option (a)): both fix directions in issue #5647 are real
trust-boundary changes to a security-relevant filter, not one-liners,
and deserve their own review rather than a bundled coverage-lane PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 06:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 5, 2026 06:51 Destroyed
@github-actions github-actions Bot added size: XS < 10 changed lines (excluding docs) risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Jul 5, 2026
@coderabbitai

coderabbitai Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Bridged tool-disclosure now supports per-turn caller-controlled capability allow-set narrowing, including bridged tool-search indexing and descriptions.
    • Added support for host-exempt capability IDs that stay visible even when narrowing is enabled.
  • Bug Fixes

    • Non-allowlisted bridged discovery/search results are no longer dispatched or disclosed.
    • Tool-describe for non-allowlisted tools now consistently reports as unknown.
  • Tests

    • Expanded bridged-mode regression coverage and added new harness assertions for narrowed descriptions and tool-result envelopes.
  • Refactor

    • Updated capability-port decoration to support async decorator behavior.

Walkthrough

Adds host-exempt bridge capability IDs, threads allow-set narrowing through bridged disclosure/runtime wiring, and extends bridged integration builders and tests for narrowed allow-list behavior.

Changes

Bridged allow-set narrowing

Layer / File(s) Summary
Surface filter and async decoration
crates/ironclaw_loop_support/src/capability_surface_filter.rs, crates/ironclaw_loop_support/src/capability_port.rs, crates/ironclaw_reborn/src/runtime.rs
Adds host-exempt capability IDs, switches loop capability decoration to async, and updates the deny/decorator test implementations.
Tool disclosure allow-set plumbing
crates/ironclaw_reborn/src/tool_disclosure.rs, crates/ironclaw_reborn/src/tool_disclosure_port.rs, crates/ironclaw_reborn/src/loop_driver_host.rs
Threads allow-sets through catalog discovery, bridge advertisement, active-set selection, ranking, and per-turn disclosure, and exempts host bridge IDs in the host filter.
Bridged harness seam
tests/integration/support/group_options.rs, tests/integration/support/group.rs, tests/integration/support/builder.rs
Adds bridged-only allow-set passthrough in the integration harness and group builders, with validation and default initialization.
Integration regression coverage
tests/integration/support/assertions.rs, tests/integration/tool_disclosure.rs
Adds exclusion assertions and bridged disclosure regression tests for narrowed bridge visibility, search output, and describe behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

  • nearai/ironclaw#5149: Introduced the bridged tool-disclosure machinery that this PR narrows with allow-sets.
  • nearai/ironclaw#5649: Added earlier bridged disclosure test coverage and harness wiring extended here.

Suggested reviewers: ilblackdragon, serrrfirat

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The body is detailed, but it does not follow the required template and omits major sections like Change Type, Validation, Security Impact, and Rollback Plan. Rework the description to match the template headings and fill in the missing required sections, especially Change Type, Validation, Security Impact, and Rollback Plan.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title uses Conventional Commits style and accurately summarizes the allow-set narrowing and regression tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a testing seam (narrowed_bridged_allow_set) to allow integration tests to override the CapabilityAllowSet used in Bridged tool disclosure mode. It includes a fail-fast guard to prevent misuse when the override is configured without enabling bridged mode, and adds an ignored integration test pinning issue #5647 where a narrowed allow-set incorrectly strips synthetic bridge IDs. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/support/builder.rs`:
- Around line 125-129: The `RebornIntegrationHarnessBuilder` is storing bridged
capability allow-set entries as raw `&'static str` instead of a domain type,
which breaks the “convert at the boundary” rule. Update the
`narrowed_bridged_allow_set` field and related builder flow to use
`CapabilityId` (or the appropriate domain wrapper) internally, then convert back
only when passing into
`RebornIntegrationGroupBuilder::with_narrowed_capability_allow_set_for_bridged_test`
in the `group_options.rs` handoff. Make the same adjustment anywhere the field
is initialized, carried through, or forwarded so the builder stays consistently
typed.

In `@tests/integration/support/group.rs`:
- Around line 674-677: The `Group` construction is cloning
`narrowed_bridged_allow_set` unnecessarily even though `self` is already being
consumed and the field is not used afterward. Update the `allow_set`
initialization in the `Group`/builder code to move
`self.narrowed_bridged_allow_set` directly into
`unwrap_or(CapabilityAllowSet::All)` instead of calling `clone`, so the value is
taken by move and the extra allocation is avoided.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e26728c0-7569-42d5-b38d-ada9d70c4fee

📥 Commits

Reviewing files that changed from the base of the PR and between 85c02c2 and 8df40bc.

📒 Files selected for processing (4)
  • tests/integration/support/builder.rs
  • tests/integration/support/group.rs
  • tests/integration/support/group_options.rs
  • tests/integration/tool_disclosure.rs

Comment thread tests/integration/support/builder.rs Outdated
Comment thread tests/integration/support/group.rs
@railway-app

railway-app Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5659 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ❌ Build Failed (View Logs) Web Jul 29, 2026 at 2:09 am

@github-actions

github-actions Bot commented Jul 5, 2026 •

Copy link
Copy Markdown
Contributor

Coverage ratchet

Ratchet mode: ENFORCING

RATCHET PASS: global
  observed: 85.78% (314996 / 367207 lines)
  floor:    80.81% (tolerance 0.5pp -> effective floor 80.31%)
  denominator: 367207 lines now vs 377084 at floor capture (-9877 lines, -2.62%) — not a material change

⚠️ 2 Reborn crate(s) have 0 int-tier coverage (target: 0) — ironclaw_prompt_envelope, ironclaw_scripts

Reborn integration-tier coverage

Line coverage (Reborn crates): 85.78% — 314996 / 367207 lines

Per-crate breakdown (60 crates, lowest-covered first)
Crate Line % Covered / Total
ironclaw_prompt_envelope 0% 0 / 88
ironclaw_scripts 0% 0 / 345
ironclaw_process_sandbox 33.91% 118 / 348
ironclaw_host_ingress 42.5% 17 / 40
ironclaw_event_projections 43.51% 684 / 1572
ironclaw_observability 61.54% 16 / 26
ironclaw_authorization 63.02% 610 / 968
ironclaw_memory 64.41% 959 / 1489
ironclaw_telegram_v2_adapter 69.69% 731 / 1049
ironclaw_trust 73.21% 664 / 907
ironclaw_wasm_limiter 74.6% 47 / 63
ironclaw_filesystem 74.64% 4829 / 6470
ironclaw_extractors 74.72% 538 / 720
ironclaw_capabilities 75.41% 2879 / 3818
ironclaw_projects 76.48% 400 / 523
ironclaw_mcp 76.6% 779 / 1017
ironclaw_reborn_cli 78.36% 10782 / 13760
ironclaw_wasm 79.72% 735 / 922
ironclaw_llm 80.14% 22266 / 27783
ironclaw_memory_native 81.02% 3299 / 4072
ironclaw_auth 81.88% 6679 / 8157
ironclaw_first_party_extensions 82.38% 6682 / 8111
ironclaw_processes 83.3% 933 / 1120
ironclaw_host_api 83.7% 9722 / 11615
ironclaw_reborn_identity 83.8% 450 / 537
ironclaw_events 84.06% 1687 / 2007
ironclaw_operator 84.41% 5561 / 6588
ironclaw_secrets 84.53% 2797 / 3309
ironclaw_reborn_config 85.23% 2101 / 2465
ironclaw_skills 85.27% 4493 / 5269
ironclaw_extension_host 85.34% 19110 / 22393
ironclaw_telegram_extension 85.4% 1299 / 1521
ironclaw_run_state 85.77% 458 / 534
ironclaw_reborn_composition 85.88% 24760 / 28832
ironclaw_triggers 85.92% 2783 / 3239
ironclaw_network 85.97% 913 / 1062
ironclaw_reborn_event_store 86.17% 1246 / 1446
ironclaw_webui 86.37% 11171 / 12934
ironclaw_hooks 86.72% 9949 / 11472
ironclaw_common 86.99% 1772 / 2037
ironclaw_approvals 87.07% 1542 / 1771
ironclaw_extensions 87.24% 4798 / 5500
ironclaw_threads 87.36% 4912 / 5623
ironclaw_product 87.48% 20324 / 23233
ironclaw_reborn_traces 88.13% 11987 / 13601
ironclaw_turns 88.41% 14546 / 16453
ironclaw_host_runtime 88.93% 19947 / 22429
ironclaw_slack_extension 89.26% 2027 / 2271
ironclaw_reborn_openai_compat 89.32% 3780 / 4232
ironclaw_conversations 90.03% 3171 / 3522
ironclaw_resources 90.84% 4474 / 4925
ironclaw_event_streams 91.24% 1063 / 1165
ironclaw_runner 91.63% 17642 / 19253
ironclaw_loop_host 92.01% 16650 / 18095
ironclaw_attachments 93.06% 630 / 677
ironclaw_outbound 93.91% 4101 / 4367
ironclaw_agent_loop 94.51% 10056 / 10640
ironclaw_safety 95.22% 3941 / 4139
ironclaw_first_party_extension_ports 95.62% 3672 / 3840
ironclaw_runtime_policy 96.56% 814 / 843

This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors.

Exemptions (3 entry/entries excluded from the accounting above)
Module / Crate Reason Issue
crate: ironclaw_embeddings v1-only: consumed only by root ironclaw (src/app.rs, src/tools/builtin/memory.rs, src/workspace/mod.rs, src/config/{mod,embeddings}.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_gateway v1-only: consumed only by root ironclaw (src/channels/web/platform/static_files.rs, src/channels/web/handlers/frontend.rs); no crates/* dependents. Covered by "Tests (Legacy)". #5657
crate: ironclaw_tui v1-only: consumed only by root ironclaw (src/main.rs, src/channels/tui.rs); no crates/* dependents. Crate's own doc comment confirms it bridges INTO v1, not Reborn. Covered by "Tests (Legacy)". #5657

@henrypark133
henrypark133 marked this pull request as draft July 5, 2026 07:05
@henrypark133

Copy link
Copy Markdown
Collaborator Author

Converting to draft: production fix for #5647 is being added to this PR (design under review). The #[ignore] comes off the test when the fix lands — this PR will not merge with a skipped test. Title/body will be updated to flag the production behavior change.

Compress #5647 RED-pin doc comments (field docs, misuse-guard note,
test doc comment) to dense 1-3 line notes carrying the crux + issue
ref, per repo comment-economy convention. No test logic or #[ignore]
attribute changed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 5, 2026 07:13 Destroyed
… allow-sets

PRODUCTION BEHAVIOR CHANGE: a narrowed-allowlist caller (subagent /
capability-surface profile) crossing the bridged tool-disclosure
threshold now keeps the synthetic ironclaw.* bridge ids
(tool_search/tool_describe/tool_call) for both disclosure AND
invocation, instead of shipping the model zero tools.
CapabilitySurfaceProfileFilter gains a constructor-injected
host-exempt id set (empty via new() — every existing call site
unchanged), folded into one private permits() mirroring the
Visible/Deny sibling filters; the sole production construction site
(loop_driver_host.rs) supplies tool_disclosure::bridge_capability_ids().

Trust boundary unchanged and tested: bridged/forgiving dispatch
resolves to the REAL underlying capability id, which the allow-set
still gates. New denial test proves a non-allowlisted underlying tool
never dispatches (zero network egress) for a narrowed profile; a
filter unit test pins bypass-without-widening. The #5647 RED pin is
un-ignored with zero test-body edits. Mutation-verified both ways:
exemption reverted -> regression test red ("saw []"); enforcement
broken -> denial test red ("saw 1" egress).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@henrypark133 henrypark133 changed the title test(reborn): RED-pin #5647 — bridged disclosure must survive a narrowed allow-set fix(reborn): [PRODUCTION CHANGE] #5647 — bridge meta-tools survive narrowed allow-sets, with regression + trust-boundary tests Jul 5, 2026
@henrypark133
henrypark133 marked this pull request as ready for review July 5, 2026 07:46
Copilot AI review requested due to automatic review settings July 5, 2026 07:46
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 5, 2026 07:46 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added size: M 50-199 changed lines and removed size: XS < 10 changed lines (excluding docs) labels Jul 5, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30394e5cd6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/ironclaw_reborn/src/loop_driver_host.rs Outdated
…Id at the boundary

Harness builder now converts &str -> CapabilityId in its public setter
and carries the domain type through to group_options (which takes
CapabilityId directly); drop a needless clone in into_group where self
is consumed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ironloopai

ironloopai Bot commented Jul 6, 2026 •

Copy link
Copy Markdown
Contributor

⏳ IronLoop Review Status

Head: 85bbf891b82f208b089ab7f582571653c632bacf
Result: No reviewer jobs are scheduled yet.
Next: Run @ironloopai review to start reviewers.
Updated: 2026-07-07T23:00:43.306Z

Current reviewers:

Reviewer State Verdict Findings Last update
none Queued N/A No reviewer jobs scheduled yet. N/A
Reviewer summaries
Reviewer Detail
none No reviewer jobs scheduled yet.
Recent activity
Time Reviewer State Detail
N/A N/A Waiting No progress events recorded yet.
Available commands
  • @ironloopai agents
  • @ironloopai review
  • @ironloopai review --agent <agent-id-or-alias>
  • @ironloopai status
Run metadata

Admission: webhook accepted the request and IronLoop persisted review state before this projection.

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supplemental security finding

A late specialist result identified one additional high-confidence authorization issue at the captured head. This is a follow-up to review 4801698963.

  1. High Bridge ID exemptions allow colliding extension capabilities (crates/ironclaw_runner/src/loop_driver_host.rs:2551, confidence 88) — anchor: crates/ironclaw_runner/src/loop_driver_host.rs:2551.
    The profile filter exempts capabilities solely by matching one of the synthetic bridge IDs. Extension capability IDs are user-controlled, and the extension lifecycle's reserved-capability set does not include these synthetic IDs, so an extension can declare ironclaw.tool_search (or another bridge ID). Its real provider tool can then bypass the allow-set and be invoked despite not being permitted; tool-disclosure catalog filtering also misclassifies the collision as a bridge.

    Fix: Reserve all bridge capability IDs for extensions, and apply exemptions only to host-synthesized bridge definitions.

Comment thread crates/ironclaw_runner/src/loop_driver_host.rs
Copilot AI review requested due to automatic review settings July 28, 2026 21:27
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 28, 2026 21:27 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added size: XL 500+ changed lines and removed size: L 200-499 changed lines labels Jul 28, 2026

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (multi-agent)

Intent: Narrow tool-disclosure surfaces to caller allow-sets while preserving bridge functionality and threshold semantics, with fail-closed behavior and regression/trust-boundary coverage.

Stats: 5 findings (from 6 raw, 5 after dedup) across 4 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 2.

The High finding is a merge blocker because a denied tool's schema can still be exposed through the describe-first bridge. The remaining findings are coverage, efficiency, and maintainability follow-ups.

logic

  1. High Auto-schema path bypasses the allow-set (crates/ironclaw_runner/src/tool_disclosure_port.rs:982-985, confidence 98) — anchor: crates/ironclaw_runner/src/tool_disclosure_port.rs:982
    The new allow-set check covers explicit tool_describe calls, but describe-first responses still return the full catalog entry for a non-allowlisted tool. A malformed direct call can therefore trigger invoke_describe_first, which exposes its name, capability ID, description, and schema through the host-exempt tool_describe bridge.

unnecessary-deferral

  1. Medium Exclude disallowed tools before token-budget selection (crates/ironclaw_runner/src/tool_disclosure.rs:589-589, confidence 93) — anchor: crates/ironclaw_runner/src/tool_disclosure.rs:589
    The deferred selector still builds core definitions and promoted candidates from the full catalog, so an allowlisted caller's disallowed core schemas consume the token threshold and max-tools budget. The outer surface filter removes those definitions only afterward, causing permitted promoted tools to be unnecessarily deferred and requiring extra tool-search/schema round trips.

missing-integration

  1. Medium No test proves allowlisted tool_describe still succeeds (crates/ironclaw_runner/src/tool_disclosure_port.rs:987-988, confidence 95) — anchor: crates/ironclaw_runner/src/tool_disclosure_port.rs:987
    The new allow-set check can incorrectly reject every tool_describe target while the existing tests cover only denied targets. No narrowed-profile integration test describes an allowlisted tool and verifies that its schema is returned. (no diff position — body only)
  2. Medium No test proves allowlisted tool_call still dispatches (crates/ironclaw_runner/src/tool_disclosure_port.rs:1126-1131, confidence 95) — anchor: crates/ironclaw_runner/src/tool_disclosure_port.rs:1129
    The new allow-set gate can incorrectly make all deferred tool_call targets unresolved; current narrowed-profile integration coverage tests only denial and existence-oracle behavior. There is no test that an allowlisted target passes this check and reaches the underlying dispatch path. (no diff position — body only)

tier-1-#4j-duplicate-profile-factory

  1. Medium Avoid duplicating capability-profile factory logic (crates/ironclaw_runner/src/runtime.rs:892-924, confidence 78) — anchor: crates/ironclaw_runner/src/loop_driver_host.rs:137
    This adds a second factory that independently resolves the allow-set and applies CapabilitySurfaceProfileFilter, duplicating the existing SurfaceFilteringCapabilityPortFactory in loop_driver_host.rs. These parallel construction paths can drift when profile filtering changes, especially because one also composes disclosure decorators.

Comment thread crates/ironclaw_runner/src/tool_disclosure_port.rs
Comment thread crates/ironclaw_runner/src/tool_disclosure.rs
Comment thread crates/ironclaw_runner/src/runtime.rs
Copilot AI review requested due to automatic review settings July 29, 2026 00:43
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 29, 2026 00:43 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 29, 2026 02:09
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 29, 2026 02:09 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (multi-agent)

Intent: Narrow tool-disclosure surfaces and bridge behavior by caller allow-sets, preventing three capability-leak vectors with regression and trust-boundary tests.

Stats: 3 findings (from 4 raw, 3 after overlap dedup) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewer fan-out hit the local agent limit; remaining lenses were completed in the parent fallback. Body-only: 1.

The High finding is a merge blocker: a denied capability's schema can still be disclosed through the describe-first bridge. The remaining findings are performance and coverage follow-ups.

Security

  1. High Auto-schema responses bypass the caller allow-set (crates/ironclaw_runner/src/tool_disclosure_port.rs:1107-1137, confidence 98) — anchor: crates/ironclaw_runner/src/tool_disclosure_port.rs:1107
    The new allow-set gate in allowed_tool_call_target prevents an unauthorized target from dispatching, but a malformed call to an allowlisted deferred tool enters register_describe_first and is returned under the host-exempt tool_describe bridge. invoke_describe_first then looks up the catalog entry and returns its description, required fields, and full parameters without checking self.allow_set. A narrowed caller can therefore learn the schema of a denied capability through the describe-first recovery path.

Tests

  1. Medium Exported bridge ID list lacks an exact contract test (crates/ironclaw_runner/src/tool_disclosure_bridge.rs:9-11, confidence 85) — anchor: crates/ironclaw_runner/src/tool_disclosure_bridge.rs:10
    The new bridge_capability_ids() export is consumed by production filtering and extension reservation, but the changed module has no direct contract test that all three canonical IDs—tool_search, tool_describe, and tool_call—are present. Indirect coverage of tool_search alone would not catch drift that leaves one bridge unreserved or unenforced.

Performance

  1. Low Recompute full catalog metrics on every tool-definition request (crates/ironclaw_runner/src/tool_disclosure_port.rs:207-208, confidence 96) — anchor: crates/ironclaw_runner/src/tool_disclosure_port.rs:208
    Each model request can call tool_definitions twice, and effective_metrics now scans the entire catalog and performs an allow-set lookup for every entry each time. This replaces the previously cached total-token value and adds O(n) work to the hot model path, even when debug logging is disabled. (no diff position — body only)

Comment thread crates/ironclaw_runner/src/tool_disclosure_port.rs
Comment thread crates/ironclaw_runner/src/tool_disclosure_bridge.rs
Copilot AI review requested due to automatic review settings July 29, 2026 17:20
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5659 July 29, 2026 17:20 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@henrypark133
henrypark133 merged commit 83418d5 into main Jul 29, 2026
61 of 62 checks passed
@henrypark133
henrypark133 deleted the w6-disclosure-narrow branch July 29, 2026 17:42
@serrrfirat serrrfirat mentioned this pull request Aug 6, 2026
24 of 29 tasks
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
…osure surface narrowed by allow-set (3 leak vectors), with regression + trust-boundary tests (nearai#5659)

* test(reborn): narrowed capability allow-set override for bridged-disclosure groups

Adds an opt-in with_narrowed_capability_allow_set_for_bridged_test()
seam so a test can request a genuinely narrowed CapabilityAllowSet
while in Bridged tool-disclosure mode, instead of into_group's
existing forced CapabilityAllowSet::All workaround (which mirrors
production for every other bridged test and stays the default here).
Fails fast if the override is set without also selecting Bridged
mode, since it would otherwise silently no-op. Threaded through both
RebornIntegrationGroupBuilder and RebornIntegrationHarnessBuilder,
mirroring the existing with_tool_disclosure_bridged/off pattern.

No behavior change for any existing test: default None resolves to
today's forced-All path byte-for-byte.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(reborn): red-pin nearai#5647 — bridged disclosure strips tool_search under a narrowed allow-set

Adds #[ignore]d bridged_mode_survives_narrowed_capability_allow_set,
using the new harness seam: Bridged mode + a >32-tool GithubIssueTools
catalog + a narrowed allow-set (github.get_repo only, not forced All).
CapabilitySurfaceProfileFilter runs outside (after)
ToolDisclosureCapabilityDecorator, so the synthetic ironclaw.tool_search
bridge id — not a real granted capability — gets stripped by the
narrowed allow-set, leaving the model with zero tools.

Verified RED at authoring time: fails at
assert_model_tools_contains(TOOL_SEARCH_NAME) with "saw []", not at
harness construction or the secondary assertion. Ships #[ignore]d
(delivery option (a)): both fix directions in issue nearai#5647 are real
trust-boundary changes to a security-relevant filter, not one-liners,
and deserve their own review rather than a bundled coverage-lane PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: trim comments to context-economy rule

Compress nearai#5647 RED-pin doc comments (field docs, misuse-guard note,
test doc comment) to dense 1-3 line notes carrying the crux + issue
ref, per repo comment-economy convention. No test logic or #[ignore]
attribute changed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(reborn): nearai#5647 — bridge meta-tool ids survive narrowed capability allow-sets

PRODUCTION BEHAVIOR CHANGE: a narrowed-allowlist caller (subagent /
capability-surface profile) crossing the bridged tool-disclosure
threshold now keeps the synthetic ironclaw.* bridge ids
(tool_search/tool_describe/tool_call) for both disclosure AND
invocation, instead of shipping the model zero tools.
CapabilitySurfaceProfileFilter gains a constructor-injected
host-exempt id set (empty via new() — every existing call site
unchanged), folded into one private permits() mirroring the
Visible/Deny sibling filters; the sole production construction site
(loop_driver_host.rs) supplies tool_disclosure::bridge_capability_ids().

Trust boundary unchanged and tested: bridged/forgiving dispatch
resolves to the REAL underlying capability id, which the allow-set
still gates. New denial test proves a non-allowlisted underlying tool
never dispatches (zero network egress) for a narrowed profile; a
filter unit test pins bypass-without-widening. The nearai#5647 RED pin is
un-ignored with zero test-body edits. Mutation-verified both ways:
exemption reverted -> regression test red ("saw []"); enforcement
broken -> denial test red ("saw 1" egress).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(test-support): type narrowed bridged allow-set as CapabilityId at the boundary

Harness builder now converts &str -> CapabilityId in its public setter
and carries the domain type through to group_options (which takes
CapabilityId directly); drop a needless clone in into_group where self
is consumed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(reborn): narrow tool_search/tool_describe payloads by the caller's allow-set

PRODUCTION BEHAVIOR CHANGE: under a narrowed CapabilityAllowSet, the
tool-disclosure bridge no longer discloses metadata (names, descriptions,
schemas) for non-allowlisted capabilities. ToolDisclosureCapabilityPort sits
inside CapabilitySurfaceProfileFilter, so its catalog is the full unnarrowed
base surface; tool_search results and tool_describe lookups previously
served that catalog to every caller. The port now holds the SAME
CapabilitySurfaceProfileResolver the composition root uses for the profile
filter and lazily re-resolves the caller's allow-set (before the turn_state
sync mutex) to filter search results and gate describe. A non-allowlisted
tool_describe target reads as "unknown" — identical to a nonexistent name —
so existence itself is not disclosed.

Unchanged: unnarrowed (All) callers see the full catalog; the 32-tool
bridging-threshold computation still runs against the unnarrowed surface;
invocation enforcement (outer profile-filter permits recheck) and the
3-bridge-id host-exempt set (nearai#5647) are untouched. Zero edits to
ironclaw_loop_support.

Also fixes the stale setter index in the group_options.rs module doc
(review nit on nearai#5659).

Closes nearai#5712

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(reborn): tighten nearai#5712 field-doc to comment-economy convention

Post-implementation review nit — the surface_resolver field doc on
ToolDisclosureCapabilityDecorator ran 4 lines; repo convention is
1-2 dense lines (invariant + issue ref).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(reborn): narrow tool_search's own advertised description by allow-set

PRODUCTION CHANGE: closes a third metadata-leak vector flagged on PR nearai#5659
(ironloopai, 2026-07-06T17:34): the tool_search bridge's advertised
`description` doubles as an always-on catalog index of every discoverable
tool name (`catalog_index_tool_search_description`). It was built from the
full, unnarrowed catalog inside the sync `tool_definitions()` path, so a
narrowed capability allow-set (subagent flavor, scheduled-trigger surface,
etc.) still read every tool name straight out of tool_search's own
description — bypassing the nearai#5712/88eb669da narrowing already applied to
tool_search/tool_describe results, since the bridge id is host-exempt from
the outer CapabilitySurfaceProfileFilter (nearai#5647) and nothing else touches
that description text.

Root cause: `tool_definitions()` is a synchronous LoopCapabilityPort method
with no `.await` point, so the async-resolved CapabilityAllowSet could never
reach it under the old lazy-per-call resolve pattern. Fix: make
`LoopCapabilityPortDecorator::decorate` async (3 production + 2 test impls,
already-async caller) so ToolDisclosureCapabilityDecorator can resolve the
allow-set once, eagerly, before any turn/port method runs — fails closed to
an empty allow-set on resolve error. This also deletes the "MUST be awaited
before turn_state() locks its sync mutex" ordering hazard the old lazy
resolve required.

Threaded the allow-set through select_active_set /
advertised_bridge_tool_definitions / catalog_index_tool_search_description /
CapabilityCatalog::discoverable_tool_names so the index only lists
allow-set-permitted tool names. New unit test
(tool_search_description_is_narrowed_by_allow_set) and integration test
(bridged_mode_tool_search_description_is_narrowed_by_allow_set) pin the
narrowing; both were verified red against the pre-fix code before the fix
was restored.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(reborn): report seen tool names when description-exclusion target is absent

Mirrors assert_model_tools_contains's missing-name diagnostic so a
bridged-surface failure shows which tools were actually sent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(reborn): update tool_disclosure failure category for nearai#5692 rename

Post-merge fix: main's nearai#5692 recoverability-stack refactor made the
abort path report the precise ModelErrorClass instead of falling
through to the generic model_error default. Fail-closed behavior
(Failed status, zero network egress) is unchanged — only the category
label narrowed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(reborn): single-resolve tool-disclosure allow-set + narrow test/doc gaps

Addresses 5 PR review findings on the disclosure-narrowing seam:

- Decorator no longer owns a CapabilitySurfaceProfileResolver: the
  host-build boundary (create_host) resolves the caller's allow-set
  exactly once and primes it into both the tool-disclosure decorator
  and the CapabilitySurfaceProfileFilter, so a transient resolver
  failure can no longer let the two observe different profiles.
- Extend the narrowed tool_search description test with a positive
  assertion (allowlisted tool present), not just the negative one.
- Extend the narrowed tool_describe test to compare the full persisted
  ToolResultReferenceEnvelope for a non-allowlisted vs a nonexistent
  target, modulo the run-scoped result_ref, closing an existence-oracle
  gap a safe_summary substring check alone would miss.
- Cite the production wiring the narrowed-allow-set harness seam mirrors.
- Fix a stale doc comment: the allow-set-filtered tool index is stable
  per (CapabilitySurfaceVersion, allow_set) pair, not per surface
  version alone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(runner): clear primed disclosure state on factory failure (nearai#5659)

* fix(runner): close bridged tool existence oracle (nearai#5659)

* refactor(runner): pass disclosure profile directly (nearai#5659)

* docs(runner): update disclosure decorator reference (nearai#5659)

* fix(runner): reserve disclosure bridge capability ids (nearai#5659)

* fix(runner): budget disclosure from effective surface (nearai#5659)

* test(runner): lock disclosure authorization contracts (nearai#5659)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-5659 — fe19d917 Deployed Jul 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: low Changes to docs, tests, or low-risk modules size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tool_search discloses full unnarrowed capability catalog under narrowed CapabilityAllowSet

2 participants