Conversation
decrypto21
left a comment
There was a problem hiding this comment.
Check ordering looks right everywhere. One substantive issue.
1. productDeviceChat reuses a permission meant for something much weaker
It gates on IdentityDisclosure (runtime.rs:576) — the same slot get_user_id uses for "show this product your username" (capabilities/account.rs:513). The key is product-scoped only (truapi-platform/src/lib.rs:1399), nothing marks which capability is asking, and truapi-platform/ has no diff here. But this call binds the wallet's Chat identity and grants a standing Seal/Open oracle against any peer key the product names.
- Signing-host (
host_core.rs:597, viaframe_server.rs:177): only gate, sinceSigningHost::product_device_chat(signing_host.rs:995) checks only the session. A product with an olderget_user_idgrant gets Bind/Seal/Open with no prompt — reproduced on this branch (pre-seeded grant → proceeds, prompt count 0; no grant →Rejected). - Two-device SSO:
sso_responder.rs:934does prompt the first time, but shows "wants to know it's you" for identity binding plus an encryption oracle. Silent after that.
Worth a dedicated PermissionAuthorizationRequest variant with its own review copy, like the neighbouring AccountAccess. If the reuse is deliberate, the doc comment (truapi-platform/src/lib.rs:1029) and prompt copy should say so.
2. Minor
sso_pairing.rs:399 switches success: *success to (*success).clone() on a struct holding identity_chat_private_key. If that was for the new Drop impl, it isn't needed — box-deref-move compiles fine with Drop. Keeping the move avoids a second live copy.
# Conflicts: # rust/crates/truapi-codegen/tests/golden/wire_table.rs # rust/crates/truapi-server/src/host_logic/sso/messages.rs # rust/crates/truapi-server/src/host_logic/sso/messages/v1.rs # rust/crates/truapi-server/src/runtime/authority.rs # rust/crates/truapi-server/src/runtime/capabilities/account.rs # rust/crates/truapi-server/src/runtime/pairing_host.rs # rust/crates/truapi-server/src/runtime/pairing_host/sso_channel.rs # rust/crates/truapi-server/src/runtime/signing_host.rs # rust/crates/truapi-server/src/runtime/signing_host/sso_responder.rs
…e/chat-receive # Conflicts: # CHANGELOG.md
…y-review-709 # Conflicts: # rust/crates/truapi-codegen/tests/golden/dispatcher.rs # rust/crates/truapi-codegen/tests/golden/wire_table.rs
Keep username disclosure separate from Chat consent. Exercise approval, denial, cached consent, and revocation through local and SSO APIs; move the secret-bearing pairing result instead of cloning it.
decrypto21
left a comment
There was a problem hiding this comment.
Re-reviewed at 8de303f6e. ChatAuthority resolved, and reconcile no longer
opens the wallet before checking for payment work.
-
The app target does not compile. Seven errors, five files, all new here:
InMemoryCoinageAssetLedger.swift:188 parameter uses a private type TrUAPIChainConnectionPool.swift:298,308 parameter / result use a private type TrUAPIIdentityCandidatesTests.swift:29,44 cannot find 'SS58AddressFactory' RustRuntimeBridgeTests.swift:435 'HostThemeSubscribeItem' has no member 'dark' TrUAPINativeCoinageTests.swift:185 cannot force unwrap non-optional 'BigUInt'The first three need
privateon the method: aprivate extensionscopes to
the file, the types to the class body.RustRuntimeBridgeTestsmissed the
currentTheme()return-type change, and the two new test files have never
compiled. iOS CI stops earlier on SwiftLint (TrUAPINativeCoinage.swift:11,
418 of 350, and complexity at :155), so the compiler never runs; fixing the
lint uncovers these. Green onfeat/pvm-app-runtimeat749cf906b. -
Over SSO the peer picks which product it is checked against.
sso_service.rs:598takescalling_product_idfrom the request, and
SsoRequestContextholds onlycallandsession. It keys
PermissionsService::new(:608) and the authority request (:638), so a peer
refused under its own name succeeds under another's, and that string also
picks the keys. The local path is correct (capabilities/account.rs:398), per
native-chat-main-purse.md:156.statement_store_product_sign(:549) is no
model: it compares two fields of one request. -
read_balance(chat-v2/lib.rs:2681) accepts encodingsread_compact_u128
(:2621) rejects, and readsSendLegacy.amountandCoinageSend.total_value.
Two byte strings decode to one message, so a statement can be republished with
different bytes and the same meaning. -
The hardened suite is off.
chat-v2binds product, sender, recipient and
channel into the authenticated data;actor/receive.rs:346rejects it. The
identity route uses the older suite, byte-identical to first-contact, so an
open_chat_request_v2aimed at a peer's identity key clears decryption and
only the decoder stops it. -
Both tests pinning the wire codec version are dropped.
client.test.tsloses
uses the transport codec version for generated handshake calls, which sent a
literal 2; the replacement at :650 comparesTRUAPI_CODEC_VERSIONto itself,
so 99 stays green. It also loseslogs a protocol violation for a known pair's out-of-range message type. Both are on the base. -
CHANGELOG.md:17says codec 2; the constant is 3 and the handshake is exact.
Line 26 saysproductDeviceChat; codegen dropsproduct_, so it is
deviceChat(generated/client.ts:304).
- iOS: fix private-type visibility, stale test APIs and the BigUInt unwrap; split TrUAPINativeCoinage so SwiftLint passes and CI reaches the compiler. - SSO: document that the paired host attests calling_product_id, and pin that consent, prompts and device keys follow the attested product. - chat-v2: decode balances through the canonical compact decoder so non-canonical amounts are rejected. - native Chat: pin that first-contact plaintext is rejected on the identity route and record why the context-bound suite stays off for native peers. - TS: restore the literal codec-3 handshake test and the out-of-range message-type test. - CHANGELOG: codec 3 and account.deviceChat. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…-authority Brings #540 (now carrying latest main) under the Chat v2 authority work. Notable resolutions: - permission prompts converge on upstream's three-way PermissionDecision. Chat v2 had centralised the prompt in PermissionsService using the older bool confirm_user_action, which cannot express AllowOnce; both check_or_prompt_identity_disclosure and check_or_prompt_chat_authority now use confirm_permission, so AllowOnce authorizes one request without persisting a grant. Caught by upstream's own regression test. - PermissionsService takes a validated ProductContext, so the chat paths build one per call: a chat session serves several products, so the product stays a per-call argument rather than context state. - native.rs keeps both sides' uniffi surfaces (chat files/coinage from Chat v2, permission decisions and product operations from upstream) and adopts upstream's chain-connect registration check, with HopProvider ported onto the new register_chain signature. - de-duplicates three clean-but-wrong auto-merges: a stacked on_core_log impl, a doubled truapi_platform import list, and an unclosed injected-failure guard in the storage stub. - gates StatementRenewalTarget re-exports to the cfg its item already had, and supplies the new permission_grants adapter field on wasm. cargo check: workspace and wasm32 clean. cargo test -p truapi-server --lib: 1179 passed.
…into feat/chat-v2-product-authority
A Response that arrives after its request timed out is a normal late frame, as the transport's comments already describe, not a protocol violation. Pin both cases in the out-of-range message-type test, and keep the RFC's route-classification claim to the route the tests cover. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Building the JS packages surfaced merge damage that neither cargo nor the Rust suite could see: - the callback success arm had both sides stacked, so operation-hold tracking and chat-file bookkeeping now live in one arm, and discardChatFileCallback's tail is closed again. - one activateLocalSession protocol variant (upstream's, with the optional liteUsername) instead of two; likewise one init `role` and one activateLocalSession method on the runtime. - role stays caller-controlled: a host that asks for none still sends none, which is upstream's additive-wire rule, and its two regression tests pass again. - the test-host allowance shortcut moved to the function that actually derives and returns the secret; it had landed in the one that returns unit and takes the target from its caller. - chainConnections keeps this file's RpcConnectionEntry, which is the shape the chain handlers read. - the wasm glue test now asserts the split it documents: the production web bundle carries no signing host, the testing bundle does. bun test: truapi 271 pass, truapi-host 282 pass. wasm32 clean under --no-default-features and under wasm-signing-host,test-host.
…into fix/pr709-review-2
…into feat/chat-v2-product-authority # Conflicts: # rust/crates/truapi-platform/src/lib.rs # rust/crates/truapi-server/src/runtime.rs # rust/crates/truapi-server/src/runtime/signing_host/sso_responder.rs
…chat-v2-product-authority # Conflicts: # js/packages/truapi-host/src/web/worker-provider.test.ts
…into fix/pr709-review-2
… test Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…into feat/chat-v2-product-authority # Conflicts: # hosts/ios/polkadot-app/Modules/Products/TrUAPI/TrUAPINativeCoinage.swift
|
Thanks for the careful pass. All six addressed in 5704252, e4424be and d3dec53; verified at f296c55:
|
|
@decrypto21 ready for another look when you have time. Since the reply above, |
get_account could never answer. Resolving a product's own account reads a persisted subtree through the host, then asks the host to confirm. Neither callback had a deadline, so a host that never answered its own storage parked the request forever: no response frame and no error frame. Both are now bounded by the caller's context, falling back to the same default the SSO call below them already uses, so an unresponsive host surfaces a typed HostAccountGetError instead of hanging. The regression test stands a platform up whose core-storage reads never return. Before this change the call never settles; after it, it fails within the caller's deadline. Closes #954
# Conflicts: # rust/crates/truapi-server/src/runtime/capabilities/account.rs
Summary
Adds Chat-specific product authority above the generic PolkaVM host integration in #540. Chat cryptographic authority, device binding, request signing, sealing, and opening remain outside the generic host PR.
Authorization review follow-up
product_device_chatnow requires dedicatedChatAuthorityconsent instead of reusingIdentityDisclosure/username consentget_user_idkeeps its separate identity-disclosure permission; no legacy username grant is migrated into Chat authoritySuccessout of its box instead of cloning itMatched downstream artifacts
ddccbac617a7490dc57ed25040e460bfdc4a1b65e69940f46692f7fc5c0c0e77f910eb0aa02c7442bd9259c586ae38e62d4d87a8bcc338e721355367af1263f60327f171e38dff32Verification
The phone remains unavailable to CoreDevice, so the on-device message scenario is not verified and no message has been sent. This correction has not been deployed or released; earlier staging deployments do not contain it.
For native retests, rebuild
ios/truapi-host/scripts/rebuild.shandios/truapi-provider/scripts/rebuild.shfrom this source, then useTRUAPI_USE_LOCAL_BINARY=1andTRUAPI_PROVIDER_USE_LOCAL_BINARY=1in the consuming build. A revision-only SPM pin is not sufficient: the published prebuilt SDK artifacts were not changed.