Skip to content

fix(ownership): fail closed on WASM default-scope fallback - #2465

Merged
ilblackdragon merged 3 commits into
nearai:stagingfrom
G7CNF:codex/issue-2069-wasm-default-fallback
Apr 18, 2026
Merged

ilblackdragon merged 3 commits into
nearai:stagingfrom
G7CNF:codex/issue-2069-wasm-default-fallback

Conversation

@G7CNF

@G7CNF G7CNF commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #2069
Refs #2070

What changed

  • WASM tool credential resolution now fails closed instead of borrowing the default scope for admin users.
  • The tool-facing error now names the user and tells them to configure the missing credential with ironclaw secrets set.
  • WASM channel startup no longer falls back to legacy default broadcast metadata when the owner scope has no stored value.
  • Added regressions proving we only read the configured owner scope and do not probe default.

Notes

  • #2070 overlaps on the same ownership boundary. The setup docs already document config.owner_id as the correct owner scope for instance-level channel secrets, so this PR keeps the runtime fix focused on the unsafe fallback behavior.

@github-actions github-actions Bot added scope: channel/wasm WASM channel runtime scope: tool/wasm WASM tool sandbox size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 14, 2026

@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 removes legacy fallback logic for broadcast metadata to ensure strict owner-scoping and hardens WASM tool credential resolution by denying default scope fallbacks. It also improves error reporting for missing credentials and introduces a mock settings store for testing. However, the mock's has_settings method is inconsistently implemented compared to other methods in the store and should be updated to correctly verify the presence of stored values.

Comment thread src/channels/wasm/wrapper.rs Outdated

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: fail-closed ownership fix looks correct

I verified the two ownership-scope changes in the checked-out code and did not find a blocker.

Positives:

  • WASM tool credential resolution now uses DefaultFallback::Denied, so the runtime no longer borrows default-scoped secrets when a user-specific secret is missing.
  • WASM channel startup only reads owner-scoped broadcast metadata and no longer probes default.
  • The new regressions cover both the tool path and the channel-startup path, which is the right caller-level coverage for this boundary.

Residual risk:

  • Approval is based on code inspection plus the new targeted tests; I did not run the full suite from this review pass.

@ilblackdragon
ilblackdragon merged commit a619c47 into nearai:staging Apr 18, 2026
14 checks passed
@ilblackdragon ilblackdragon mentioned this pull request Apr 18, 2026
5 of 7 tasks
@henrypark133 henrypark133 mentioned this pull request Apr 21, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
* fix(wasm): fail closed on default-scope fallback

* fix(wasm): implement settings-store has_settings mock
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: channel/wasm WASM channel runtime scope: tool/wasm WASM tool sandbox size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ownership(wasm): remove "default" credential fallback in WASM tool execution

3 participants