fix: attest delegate-to-delegate caller via MessageOrigin::Delegate by sanity · Pull Request #3865 · freenet/freenet-core

sanity · 2026-04-13T21:32:24Z

Problem

When delegate A sends a message to delegate B via
OutboundDelegateMsg::SendDelegateMessage, B's process() is invoked with
origin = None. B has no way to learn which delegate (if any) called it.
The dispatch site at crates/core/src/contract.rs:479 explicitly passed
None and the comment acknowledged the gap:

origin_contract is None for inter-delegate delivery since the message
originates from another delegate, not from a contract attestation context.

This blocks any trust policy that depends on caller delegate identity, and
prevents the permission prompt UI added in #3857 from showing the actual
caller for delegate-to-delegate RequestUserInput requests.

Approach

Add a Delegate(DelegateKey) variant to freenet-stdlib::MessageOrigin
(see freenet/freenet-stdlib#65) and plumb a runtime-attested caller delegate
key through the inter-delegate dispatch path so the receiver sees
Some(MessageOrigin::Delegate(caller_key)) instead of None.

ContractExecutor::execute_delegate_request gains a
caller_delegate: Option<&DelegateKey> parameter alongside the existing
origin_contract. The two are mutually exclusive at the call sites:

Web app → delegate (via WebSocket): origin_contract = Some(...),
caller_delegate = None. Receiver sees MessageOrigin::WebApp(contract_id).
Delegate A → delegate B (SendDelegateMessage): origin_contract = None,
caller_delegate = Some(&A_key). Receiver sees
MessageOrigin::Delegate(A_key). The runtime fills this in from the
caller's executing context (delegate_key is bound from req.key() —
trustworthy because the runtime sets it, not from caller-controlled WASM
output) — so the calling delegate cannot forge it.

Origin resolution lives in a new free function resolve_message_origin so
the precedence rules are unit-testable directly:

caller_delegate (Delegate variant) — wins unconditionally.
origin_contract (WebApp variant).
DELEGATE_INHERITED_ORIGINS lookup (inherited WebApp).

A debug_assert! enforces the mutual-exclusion invariant on the two
arguments. The wasm_runtime layer matches the new MessageOrigin::Delegate(_)
variant explicitly: only WebApp propagates contract access into
origin_contracts; Delegate carries identity only. The
#[non_exhaustive] catch-all logs a tracing::warn! so any future variant
that reaches the fail-closed default is visible during the PR that adds it
rather than silently denying contract access in production.

Compatibility

The companion stdlib change is a source-level break (variant added,
#[non_exhaustive]) but not a wire-format break — bincode discriminants
for existing variants are unchanged, and stdlib pins both WebApp and
Delegate byte layouts in delegate_interface.rs tests. MessageOrigin
is delegate-only, so:

Contracts: unaffected entirely. Contracts never see MessageOrigin.
Deployed delegates that receive only WebApp or None origins:
unaffected. They continue to deserialize identically.
Deployed delegates that begin receiving inter-delegate calls: would
fail to deserialize Delegate(..). This requires another delegate to
call them via SendDelegateMessage, which no production delegate
exercises today (river chat delegate, ghostkey, etc.).

A delegate that wants to start participating in inter-delegate messaging
after this lands must be rebuilt against stdlib 0.5.x.

Testing

resolve_message_origin_tests (new, 4 unit tests) — pin the precedence
rules directly: caller_delegate wins over origin_contract, origin_contract
alone yields WebApp, neither + no inherited yields None, caller_delegate
also overrides an inherited WebApp origin (documents revocation semantics).
test_inbound_app_message_propagates_delegate_origin (new) — calls
runtime.inbound_app_message with Some(MessageOrigin::Delegate(caller_a))
and asserts the test delegate's process() echoes that exact caller key
back via a new origin_delegate_key_bytes field on
DelegateMessageReceived. The in-band msg.sender is set to a different
key so the assertion cannot pass by accident if the receiver confuses
msg.sender with the runtime-attested origin.
The existing test_delegate_emits_send_delegate_message,
test_delegate_receives_delegate_message, and
test_delegate_to_delegate_roundtrip tests continue to pass.
All 2241 lib tests in cargo test -p freenet --lib pass locally.
cargo clippy --locked -- -D warnings clean.

Why didn't CI catch this?

The bug was a missing capability, not a regression — origin = None was
returning a documented limitation. CI couldn't catch it because the contract
surface didn't exist yet. The new resolve_message_origin_tests now fail
without the caller_delegate plumbing, so any regression that drops the
attestation will be caught at the unit-test level.

Pre-merge checklist (release coordination)

⚠️ Draft — must not merge until the items below are checked off.

There are three freenet-stdlib references that currently point at
the feat-message-origin-delegate branch on github via git = ... so CI
can build this PR before stdlib publishes. All three must be removed
before merge:

feat!: add MessageOrigin::Delegate variant for inter-delegate caller attestation freenet-stdlib#65 published to crates.io as 0.5.0
Root Cargo.toml: delete the [patch.crates-io] block (regular
freenet-stdlib = "0.5.0" from crates.io takes over).
apps/freenet-ping/Cargo.toml: delete the [patch.crates-io] block
(added in this PR because the nested ping workspace transitively
requires freenet-stdlib = "0.5.0" via the freenet path-dep, and
Cargo's [patch.crates-io] is not inherited by nested workspaces).
tests/test-delegate-messaging/Cargo.toml: replace the
git = ..., branch = ... line with version = "0.5.0", features = ["contract"].
All affected Cargo.lock files regenerated against crates.io
riverctl member list smoke test against a gateway running the
rebuilt binary (per .claude/rules/bug-prevention-patterns.md — "Protocol
enum changes" rule learned from the v0.2.11 streaming incident). River's
current source does not reference MessageOrigin so source-compat risk
is nil, but the runtime smoke test should still happen.
Audit tests/test-delegate-capabilities/src/lib.rs — it pattern-matches
MessageOrigin exhaustively against only the WebApp arm. That crate
currently pins freenet-stdlib = "0.3.5" in its own workspace, so it does
not break this PR's build, but the moment its stdlib bumps to 0.5 it fails
to compile. Either update in this PR or file a follow-up issue.
Mark PR ready for review (un-draft) once the items above are done

Fixes

Closes #3860

[AI-assisted - Claude]

Plumbs a runtime-attested caller delegate key through the inter-delegate dispatch path so the receiving delegate's `process()` sees `Some(MessageOrigin::Delegate(caller_key))` instead of `None`. This unblocks trust policies and permission prompts that depend on caller delegate identity (see PR #3857 for the prompt UI that already renders this variant). The `ContractExecutor::execute_delegate_request` API gains a `caller_delegate: Option<&DelegateKey>` parameter alongside the existing `origin_contract`. At most one is `Some` at any call site: - Web app → delegate (via WebSocket): `origin_contract = Some(...)`, `caller_delegate = None`. Receiver sees `MessageOrigin::WebApp(contract_id)`. - Delegate A → delegate B (via SendDelegateMessage): `origin_contract = None`, `caller_delegate = Some(&A_key)`. Receiver sees `MessageOrigin::Delegate(A_key)`. The runtime fills this in — the calling delegate cannot forge it. In `delegate_request`'s `ApplicationMessages` branch, `caller_delegate` takes precedence over both the direct contract attestation and the `DELEGATE_INHERITED_ORIGINS` fallback. The wasm_runtime layer is updated to match the new `MessageOrigin::Delegate(_)` variant and to fail closed on any future variant: only `WebApp` propagates contract access into `origin_contracts`; `Delegate` carries identity only. Regression test: `test_inbound_app_message_propagates_delegate_origin` calls `runtime.inbound_app_message` with `Some(MessageOrigin::Delegate(caller_key))` and asserts the test delegate's `process()` echoes that exact key back via a new `origin_delegate_key_bytes` field on `DelegateMessageReceived`. The in-band `msg.sender` is deliberately set to a different key in the test so the assertion cannot pass by accident if the receiver confuses `msg.sender` with the runtime-attested origin. The test delegate at `tests/test-delegate-messaging` is repointed at the local freenet-stdlib worktree carrying the new variant; this `path = ...` must be replaced with the published 0.5.0 from crates.io before merge. The same caveat applies to the workspace-level `[patch.crates-io]` override. Also fixes a non-exhaustive-pattern site in `tests/operations.rs` that matched on `MessageOrigin` without a wildcard arm — required now that the upstream enum is `#[non_exhaustive]`. Closes #3860 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Addresses review feedback on freenet-core#3865: 1. Extract origin resolution into `resolve_message_origin` (free function) so the precedence rules can be unit-tested directly without standing up a full `Executor`. Three reviewers (testing, code-first, skeptical) all flagged that the prior regression test bypassed `Executor::delegate_request` entirely — it called `runtime.inbound_app_message` with a hand-built `Some(MessageOrigin::Delegate(..))`, so reverting `contract.rs:481` from `Some(delegate_key)` back to `None` would NOT have failed the test. The new `resolve_message_origin_tests` cover: - `caller_delegate_takes_precedence_over_origin_contract` — pins the precedence rule in case anyone reorders the branches. - `origin_contract_alone_yields_webapp` — preserves the historical web-app-driven dispatch behavior. - `no_arguments_and_no_inherited_yields_none` — None pass-through. - `caller_delegate_overrides_inherited_origin` — documents the deliberate "inter-delegate calls revoke inherited contract access" semantics from the new `MessageOrigin::Delegate` rustdoc. 2. `debug_assert!` on the mutual-exclusion invariant (`!(origin_contract.is_some() && caller_delegate.is_some())`) in `Executor::delegate_request`. The doc comment already states "at most one of these two arguments may be Some"; the assertion turns the convention into a tripwire so a future call site that violates it fails loudly in debug/test builds. In release builds the precedence below silently picks `caller_delegate` (fail-safe in the direction of "least surprising attestation"). 3. `tracing::warn!` in the `non_exhaustive` catch-all of `wasm_runtime::delegate::Runtime::inbound_app_message`. Future `MessageOrigin` variants reach the fail-closed default arm because the compiler requires it; logging makes the gap visible during the PR that adds the new variant rather than silently denying contract access in production. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

CI on PR #3865 was failing 5 jobs (Clippy, Simulation, Unit & Integration, NAT Validation, Windows) at `cargo metadata` because both `path = ...` references to the local freenet-stdlib worktree resolve only on the author's machine — the sibling `../../freenet-stdlib-3860/rust` directory does not exist on CI runners. Swap both for git pins on the stdlib branch: - Root `Cargo.toml` `[patch.crates-io]`: now `git = ".../freenet-stdlib", branch = "feat-message-origin-delegate"`. Cargo's patch supports git sources and CI can fetch the branch. - `tests/test-delegate-messaging/Cargo.toml`: same swap. The test fixture is in its own workspace and is NOT covered by the root `[patch.crates-io]`, so it must point at the git source directly. Both pins are still flagged with prominent "MUST be removed before merge" comments referencing #3860. They will be replaced with `version = "0.5.0"` once stdlib publishes to crates.io. The pre-merge checklist on PR #3865 already tracks this swap. Lockfiles regenerated: `freenet-stdlib v0.5.0` and `freenet-macros v0.2.0` now resolve from `https://github.com/freenet/freenet-stdlib?branch=feat-message-origin-delegate` in both Cargo.lock and tests/test-delegate-messaging/Cargo.lock. Verified locally: `cargo build -p freenet`, `cargo clippy --locked -- -D warnings`, and `cargo test -p freenet --lib resolve_message_origin` all pass against the git pin. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

… CI NAT Validation and Unit & Integration jobs on PR #3865 were still failing after the previous CI fix because the freenet-ping nested workspace could not resolve `freenet-stdlib = "^0.5.0"`. Root cause: `apps/freenet-ping/contracts/ping` is built via `cd apps/freenet-ping/contracts/ping && fdev build --features contract`, which shells out to `cargo build` and walks UP to find the closest workspace — that's `apps/freenet-ping/Cargo.toml`, NOT the freenet-core root workspace. The ping workspace pulls in `freenet-core` via the path dep on `app/`, which transitively requires `freenet-stdlib = "0.5.0"`. Cargo's `[patch.crates-io]` section is NOT inherited by nested workspaces, so the override added to the root Cargo.toml in the previous commit had no effect here. Without the override, the nested workspace tries to fetch 0.5.0 from crates.io, doesn't find it, and fails with "failed to select a version for the requirement freenet-stdlib = ^0.5.0". Fix: duplicate the same `[patch.crates-io] freenet-stdlib = { git = ..., branch = ... }` block into `apps/freenet-ping/Cargo.toml`. This is the only nested workspace that path-deps freenet-core (verified with `grep -rln 'freenet = .*path = ' apps/ tests/`). Verified locally: cd apps/freenet-ping/contracts/ping && \ cargo build --target wasm32-unknown-unknown --features contract now succeeds (previously: same `failed to select a version` error CI hit). The new override is flagged with the same "MUST be removed before merge" comment block as the root one, and the PR pre-merge checklist already covers swapping git pins to `version = "0.5.0"` once stdlib publishes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

freenet-stdlib 0.5.0 is now on crates.io (from PR #65 merged to main). Removes all three temporary `[patch.crates-io]` git overrides and the test-delegate-messaging git pin, replacing them with the standard crates.io dependency. All three Cargo.lock files regenerated to resolve freenet-stdlib v0.5.0 from the registry instead of the github branch. Removed: - Root `Cargo.toml`: `[patch.crates-io] freenet-stdlib = { git = ... }` - `apps/freenet-ping/Cargo.toml`: same `[patch.crates-io]` block - `tests/test-delegate-messaging/Cargo.toml`: `git = ..., branch = ...` swapped for `version = "0.5.0", features = ["contract"]` Verified locally: workspace builds clean, `cargo clippy --locked -- -D warnings` clean, all 4 `resolve_message_origin_tests` pass, all 37 delegate tests pass. This is the pre-merge cleanup pass on the freenet-core#3865 checklist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…stive arms stdlib 0.6.0 (freenet/freenet-stdlib#66) adds `#[non_exhaustive]` to five wire-boundary enums (`InboundDelegateMsg`, `UpdateData`, `DelegateError`, `ContractError`, `APIVersion`) and pins their bincode wire formats with new test coverage. The bump is source-level breaking but wire-compatible: deployed contracts and delegates compiled against any earlier 0.x stdlib continue to deserialize identically because bincode discriminants for existing variants are byte-identical (now permanently locked by the new pin tests). This commit fixes every freenet-core / fdev / test-fixture match site that the compiler flagged after the bump: - `wasm_runtime/delegate.rs` (2 sites): `inbound_msg_name` lookup gets an "Unknown" sentinel; the `for msg in inbound` loop forwards future variants through the same generic exec path so a delegate built against a newer stdlib can still receive them. Annotated `#[allow(clippy::wildcard_enum_match_arm)]` with a comment because expanding the wildcard into `pat | _` would defeat the non_exhaustive safety net. - `client_events.rs` (1 site): UPDATE conversion to 'static rejects unknown UpdateData variants with an explicit `Error::Node` (forwards a "rebuild freenet-core against the stdlib emitting this variant" diagnostic to the client) rather than silently dropping the payload. - `contract.rs` (2 sites): delegate UPDATE path falls into the existing "unsupported variant" warn+reject branch; the `ContractHandlerEvent:: UpdateQuery` path stays `unreachable!()` because the producer at the edge is now responsible for rejecting unknowns. - `node/request_router.rs` (1 site): hash dedup uses sentinel discriminant 255 for unknown variants so dedup keys stay distinct. - `operations/update.rs` (1 site): state-extraction returns `None` for unknowns (treated as "no state to extract"). - `contract/executor/mock_wasm_runtime.rs` (1 site): mock-only path ignores unknowns for the merge. - `tests/test-contract-mock-aligned/src/lib.rs` (1 site): test fixture ignores unknowns. - `crates/fdev/src/commands.rs` (2 sites): `extract_update_bytes` returns an empty slice for unknown variants; `describe_update_variant` returns "unknown". Test fixture lockfile bump: `tests/test-delegate-messaging/Cargo.toml` already pinned 0.5.0 (the published version at the time of the prior commit); now bumped to 0.6.0 to match. Adds `.claude/rules/git-workflow.md` guidance: stdlib-first release policy (don't use [patch.crates-io] git overrides for cross-repo coordination — publish stdlib first, then open the consumer PR) and a walk-through of the non_exhaustive bump pattern so the next person who adds a wire-boundary variant doesn't have to rediscover the cheap-win shape from scratch. Verified locally: full freenet-core lib test suite passes (2245 tests, 4 more than baseline — the `resolve_message_origin_tests` precedence suite from the prior commit), `cargo clippy --locked -- -D warnings` clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…dlib 0.6.0 Six integration test sites in `tests/operations.rs` and `tests/connectivity.rs` exhaustively matched `UpdateData` and broke after the stdlib 0.6.0 bump (which made `UpdateData` `#[non_exhaustive]`). The previous commit only caught lib-target match sites because `cargo build -p freenet` doesn't build the test target. Local repro: `cargo build --tests -p freenet`. Each site gets a `| _` arm appended to the existing `UpdateData::Delta | StateAndDelta | Related*` group, so unexpected and future variants share the same "unexpected update type" warn or `bail!()`. Behavior is unchanged for known variants. Verified locally: cargo build --tests -p freenet # clean cargo build --tests -p freenet --all-features # clean cargo clippy --locked -- -D warnings # clean cargo test -p freenet --lib # 2245 passed Lesson logged in `.claude/rules/git-workflow.md`: when bumping stdlib across a `#[non_exhaustive]` boundary, run `cargo build --tests` (not just `cargo build -p freenet`) before pushing — the lib target won't catch integration test exhaustive matches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-14T02:46:45Z

Good. The #[allow(clippy::wildcard_enum_match_arm)] is present at the appropriate site. Now let me compile the full review.

Rule Review: Fix: PR regression test doesn't pin the key integration fix at `contract.rs`

Rules checked: git-workflow.md, code-style.md, testing.md, operations.md, contracts.md
Files reviewed: 22 (across all 4 commits on this branch)

Warnings

crates/core/src/contract.rs:485 — The specific bug site is not pinned by any test. The core fix changes execute_delegate_request(target_req, None) → execute_delegate_request(target_req, None, Some(delegate_key)). Neither new test catches a revert of this line: test_inbound_app_message_propagates_delegate_origin calls runtime.inbound_app_message directly with a hand-constructed Some(MessageOrigin::Delegate(caller_a)), bypassing the contract.rs dispatch path entirely; and the four resolve_message_origin_tests call the extracted helper directly with Some(&caller) as an argument, so they also don't exercise whether contract.rs actually builds that Some(delegate_key). Reverting line 485 from Some(delegate_key) back to None leaves all tests green. An integration test that calls through the full contract.rs inter-delegate dispatch (e.g. building a synthetic SendDelegateMessage outbound, running it through the executor, and asserting the receiving delegate's process() receives MessageOrigin::Delegate(caller_key)) is needed to close this gap. (rule: testing.md — fix: PR without regression test)

Info

No info-level findings.

Rule review against .claude/rules/. WARNING findings block merge. ⚠️ 1 warning(s) — fix or add review-override label

sanity and others added 5 commits April 13, 2026 16:31

sanity mentioned this pull request Apr 14, 2026

feat!: harden wire-boundary enums with #[non_exhaustive] + pin tests freenet/freenet-stdlib#66

Merged

sanity and others added 2 commits April 13, 2026 21:08

sanity marked this pull request as ready for review April 14, 2026 02:42

sanity added this pull request to the merge queue Apr 14, 2026

Merged via the queue into main with commit 96c871c Apr 14, 2026
12 of 13 checks passed

sanity deleted the fix-3860 branch April 14, 2026 02:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: attest delegate-to-delegate caller via MessageOrigin::Delegate#3865

fix: attest delegate-to-delegate caller via MessageOrigin::Delegate#3865
sanity merged 7 commits intomainfrom
fix-3860

sanity commented Apr 13, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

sanity commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Approach

Compatibility

Testing

Why didn't CI catch this?

Pre-merge checklist (release coordination)

Fixes

Uh oh!

github-actions Bot commented Apr 14, 2026

Rule Review: Fix: PR regression test doesn't pin the key integration fix at contract.rs

Warnings

Info

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sanity commented Apr 13, 2026 •

edited

Loading

Rule Review: Fix: PR regression test doesn't pin the key integration fix at `contract.rs`