fix(ownership): remove silent cross-tenant credential fallback

[henrypark133]

Summary

• Remove silent cross-tenant credential fallback in WASM tool execution — resolve_host_credentials() no longer falls back to "default" scope when a user's credential is missing. Returns Err(ToolError::NotAuthorized) with an actionable message instead.
• Fix resolve_websocket_identify_message() to use the channel's owner_scope_id instead of hardcoded "default".
• Document legacy broadcast metadata fallback with removal tracking and setup.rs boot-time ownership model.

Note: The broadcast metadata legacy fallback in load_broadcast_metadata() is documented but NOT removed — tracked in #2100.

Addresses #2069 (broadcast metadata fallback deferred — see #2100)
Addresses #2070 (broadcast metadata fallback deferred — see #2100)

Test plan

• test_resolve_host_credentials_no_cross_tenant_fallback — credential under "default" does NOT leak to another user
• test_resolve_host_credentials_missing_secret_returns_error — missing cred returns NotAuthorized with credential + user name
• test_resolve_host_credentials_no_store_with_credentials_errors — no store + required creds returns error
• test_resolve_host_credentials_skips_urlpath_credentials — UrlPath creds skipped without error
• test_resolve_websocket_identify_message_uses_owner_scope — websocket uses owner scope, not "default"
• All 4,349+ existing tests pass
• cargo clippy --all --all-features — zero warnings
• grep get_decrypted("default" src/ — zero hits in production code

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Removes silent cross-tenant credential fallback in WASM tool execution to enforce per-user secret scoping, and fixes WASM channel websocket identify secret resolution to use the channel owner scope rather than a hardcoded "default".

Changes:

• Make resolve_host_credentials() return Result and error with ToolError::NotAuthorized when required secrets can’t be resolved (no "default" fallback).
• Pass owner_scope_id into websocket identify resolution and use it for get_decrypted() instead of "default".
• Add ownership-model documentation for channel boot-time secret lookups and document the legacy broadcast-metadata fallback.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/tools/wasm/wrapper.rs	Removes "default" credential fallback; returns NotAuthorized for missing credentials and adds regression tests.
src/channels/wasm/wrapper.rs	Uses owner_scope_id when resolving websocket identify secrets; documents legacy broadcast-metadata fallback and adds regression test.
src/channels/wasm/setup.rs	Documents why boot-time channel secrets are resolved under config.owner_id (instance-level ownership).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gemini-code-assist]

Code Review

This pull request implements a stricter ownership model for WASM channels and tools, ensuring that credentials are resolved within the correct scope and removing insecure cross-tenant fallbacks. Key changes include updating websocket identification to use the channel's owner scope and modifying tool credential resolution to return a NotAuthorized error instead of silently skipping missing secrets or falling back to a global 'default' user. Review feedback suggests refining the logic in resolve_host_credentials to ensure that UrlPath credentials (which do not require the secrets store) do not trigger authorization errors and that error messages distinguish between missing and expired secrets.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/tools/wasm/wrapper.rs:97

• ResolvedHostCredential now derives Debug, but it contains secret_value (the raw decrypted credential). Any accidental {:?} logging/panic of this struct (or a parent struct containing it) would leak secrets into logs/test output. Consider removing Debug or implementing a custom redacting Debug impl that omits/obfuscates secret_value (and ideally headers/query params too, since they may embed the same secret).

/// Pre-resolved credential for host-based injection.
///
/// Built before each WASM execution by decrypting secrets from the store.
/// Applied per-request by matching the URL host against `host_patterns`.
/// WASM tools never see the raw secret values.
#[derive(Debug)]
struct ResolvedHostCredential {
    /// Host patterns this credential applies to (e.g., "www.googleapis.com").
    host_patterns: Vec<String>,
    /// Headers to add to matching requests (e.g., "Authorization: Bearer ...").
    headers: HashMap<String, String>,
    /// Query parameters to add to matching requests.
    query_params: HashMap<String, String>,
    /// Raw secret value for redaction in error messages.
    secret_value: String,
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[nickpismenkov]

lgtm!

[claude]

Code review

Found 4 issues:

1. [HIGH:75] Incomplete security fix scope: Tool-level credentials now properly fail on missing credentials, but resolve_channel_host_credentials() at line 4073 in src/channels/wasm/wrapper.rs still silently skips missing credentials (line 4114: continue;). While channels are instance-owned (lower risk than user-scoped tools), the silent skip pattern should be addressed consistently across the codebase. Reference: https://github.com/anthropics/ironclaw/blob/c890d07a1/src/tools/wasm/wrapper.rs#L4070-L4114

2. [HIGH:75] Missing custom Debug impl for channel credentials: Added a custom Debug impl for ResolvedHostCredential in src/tools/wasm/wrapper.rs (lines 100-112) that redacts secrets, but the same struct in src/channels/wasm/wrapper.rs (line 86) has only #[derive(Clone)]. This asymmetry could lead to secret leakage if channel credentials are printed or logged. Reference: https://github.com/anthropics/ironclaw/blob/4d73c31ee/src/channels/wasm/wrapper.rs#L86

3. [MEDIUM:75] Referenced spec document missing: The module documentation in src/channels/wasm/setup.rs references docs/superpowers/specs/2026-04-01-ownership-model-design.md but this file does not exist. This will confuse maintainers trying to understand the ownership model rationale. Reference: https://github.com/anthropics/ironclaw/blob/5700a0c84/src/channels/wasm/setup.rs#L17

4. [MEDIUM:50] Breaking change in error behavior: This PR changes tool execution from silently returning empty credentials (leading to 401s from APIs) to explicitly failing with ToolError::NotAuthorized. While the new behavior is better UX (actionable error messages), existing tools and automation that expect silent failures may break. The change is correct, but deployment should include migration guidance.

Positive findings:

• ✅ Error propagation is correct: .await? at line 1170 properly propagates errors through the Tool trait
• ✅ Error mapping is granular and actionable: distinguishes NotFound/Expired/AccessDenied with specific guidance
• ✅ UrlPath credentials correctly excluded from store requirements (lines 1497-1509)
• ✅ Regression tests are comprehensive: cross-tenant fallback properly verified as blocked
• ✅ CLAUDE.md compliance: removed warn!() logging that corrupts REPL; comments follow "non-obvious logic only" principle
• ✅ Websocket identify message fix passes owner_scope_id correctly with test coverage
• ✅ No production .unwrap() or .expect() introduced; test assertions properly justified with "safety: test code only" comments
• ✅ Legacy migration path clearly documented with issue reference ownership(channels): remove legacy "default" broadcast metadata fallback #2100
• ✅ No blocking in async path; fully async credential resolution

🤖 Generated with Claude Code