Skip to content

fix(wallet): consistently use verified calls in the request dialog - #679

Merged
sea-snake merged 6 commits into
mainfrom
arshavir/compassionate-fermi-ksszck
Nov 2, 2026
Merged

sea-snake merged 6 commits into
mainfrom
arshavir/compassionate-fermi-ksszck

Conversation

@aterga

@aterga aterga commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The request dialog already loads the request itself with a verified call (getRequest(..., true)), and some operation renderers (EditAddressBookEntry, EditAsset, RemoveAddressBookEntry, RemoveUserGroup, ChangeExternalCanister) load their current values the same way. This PR applies that consistently to the rest of the data rendered inside the dialog:

  • Operation renderers: current values in detail/diff mode for EditUser, EditUserGroup, EditNamedRule, EditPermission, EditRequestPolicy, RemoveNamedRule, RemoveRequestPolicy and ManageSystemInfo (list rows keep using query calls).
  • Load next: getNextApprovableRequest is called with verifiedCall = true, since its response is rendered directly.
  • CallExternalCanister: the full-info getRequest re-fetch done in detail mode.
  • Shared components: autocompletes (users, user groups, accounts, address book, assets), named rule chips, rule tooltips and summaries (useRuleToTooltip, usePopulatedRule), group member tooltips (InteractiveUserGroup), and the address book entry specifier. RequestDialog calls the new provideVerifiedReads(), and these components read it with useVerifiedReads(), so they keep using query calls everywhere outside the dialog.
  • StationService.getNamedRule gains the verifiedCall parameter the other getters already have.

Trade-off

Reads inside the request dialog become update calls, which adds roughly a second or two when the dialog opens and when the next request is loaded. Lists and other pages are unchanged.

Testing

  • New verified-reads.composable.spec.ts covers autocomplete, rule tooltip and group member tooltip lookups inside and outside the dialog scope.
  • RequestDialog.spec.ts now asserts that load next uses a verified call.
  • Locally: vitest run (106 files, 502 tests), vue-tsc --noEmit, ESLint and Prettier all pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WAEJVi9S9JPmbMVGCycebk

The request dialog loads the request itself with a verified call, but some
of the data rendered inside it was still loaded with query calls: the
current values shown in diff views, the next request when "Load next" is
enabled, the full CallExternalCanister details, and the labels resolved by
autocompletes, named rule chips and address book specifiers.

Load these with the verified actor as well. Shared components pick this up
through a new `provideVerifiedReads`/`useVerifiedReads` pair, so they keep
using query calls everywhere outside of the dialog. `getNamedRule` gains the
`verifiedCall` parameter the other getters already have.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WAEJVi9S9JPmbMVGCycebk
Copilot AI balanced review requested due to automatic review settings October 26, 2026 16:02

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.

🟡 Changes recommended

Tooltip lookups remain unverified, and named-rule removal rows unintentionally use update calls outside the dialog.

2 open issues
What changed in this PR

This PR extends verified reads across the wallet’s request review dialog so supporting data uses the same verification approach as the request itself.

Changes:

  • Enables verified calls for operation details, full request information, and loading the next request.
  • Adds a verification scope for shared autocomplete, named-rule, and address-book components.
  • Adds scope tests and strengthens the load-next assertion.
File Description
apps/​wallet/​src/​services/​station.service.ts Adds optional verification to named-rule reads.
apps/​wallet/​src/​composables/​verified-reads.composable.ts Defines the shared verification scope.
apps/​wallet/​src/​composables/​verified-reads.composable.spec.ts Tests reads inside and outside the scope.
apps/​wallet/​src/​composables/​autocomplete.composable.ts Applies verification scope to autocomplete lookups.
apps/​wallet/​src/​components/​requests/​RequestDialog.vue Provides verification scope and verifies load-next reads.
apps/​wallet/​src/​components/​requests/​RequestDialog.spec.ts Asserts verified load-next calls.
apps/​wallet/​src/​components/​requests/​operations/​RemoveRequestPolicyOperation.vue Verifies policy detail reads.
apps/​wallet/​src/​components/​requests/​operations/​RemoveNamedRuleOperation.vue Enables verified named-rule reads.
apps/​wallet/​src/​components/​requests/​operations/​ManageSystemInfoOperation.vue Verifies system information outside list mode.
apps/​wallet/​src/​components/​requests/​operations/​EditUserOperation.vue Verifies current-user reads.
apps/​wallet/​src/​components/​requests/​operations/​EditUserGroupOperation.vue Verifies current-group reads.
apps/​wallet/​src/​components/​requests/​operations/​EditRequestPolicyOperation.vue Verifies current-policy reads.
apps/​wallet/​src/​components/​requests/​operations/​EditPermissionOperation.vue Verifies current-permission reads.
apps/​wallet/​src/​components/​requests/​operations/​EditNamedRuleOperation.vue Verifies current named-rule reads.
apps/​wallet/​src/​components/​requests/​operations/​CallExternalCanisterOperation.vue Verifies the full-information request fetch.
apps/​wallet/​src/​components/​request-policies/​specifier/​AddressBookEntrySpecifier.vue Applies verification scope to entry lookups.
apps/​wallet/​src/​components/​request-policies/​rule/​NamedRule.vue Applies verification scope to named-rule loading.
apps/​wallet/​src/​components/​request-policies/​InteractiveNamedRule.vue Applies verification scope to interactive rule loading.

Review effort: Balanced

Rule tooltips and summaries resolve user, group and named rule names
through `populateRule`, which always used query calls. Pass the dialog's
verified reads flag through it, so these lookups match the rest of the
dialog and keep using query calls elsewhere.

Also keep the RemoveNamedRule request list rows on query calls; only its
detail view needs the verified read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WAEJVi9S9JPmbMVGCycebk
@aterga
aterga marked this pull request as ready for review November 2, 2026 12:09
@aterga
aterga requested a review from a team as a code owner November 2, 2026 12:09
@sea-snake
sea-snake enabled auto-merge (squash) November 2, 2026 12:11
@zeropath-ai

zeropath-ai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

✅ No security or compliance issues detected. Reviewed everything up to 8398e81.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► apps/wallet/src/components/request-policies/InteractiveNamedRule.vue
    Use verified reads when fetching named rules
► apps/wallet/src/components/request-policies/rule/NamedRule.vue
    Use verified reads in NamedRule fetch
► apps/wallet/src/components/request-policies/specifier/AddressBookEntrySpecifier.vue
    Pass verifiedReads to address book entries fetch
Enhancement ► apps/wallet/src/components/requests/RequestDialog.spec.ts
    Adjust expectation for getNextApprovableRequest call with verified flag
Enhancement ► apps/wallet/src/components/requests/RequestDialog.vue
    Provide verified reads context for shared components inside dialog
Enhancement ► apps/wallet/src/components/requests/operations/CallExternalCanisterOperation.vue
    Call getRequest with verified flag
► apps/wallet/src/components/requests/operations/EditNamedRuleOperation.vue
    Pass verified flag when fetching named rule
Enhancement ► apps/wallet/src/components/requests/operations/EditPermissionOperation.vue
    Fetch permission with verified flag
► apps/wallet/src/components/requests/operations/EditRequestPolicyOperation.vue
    Fetch request policy with verified flag
Enhancement ► apps/wallet/src/components/requests/operations/EditUserGroupOperation.vue
    Fetch user group with verified flag
Enhancement ► apps/wallet/src/components/requests/operations/EditUserOperation.vue
    Fetch user with verified flag set to true
► apps/wallet/src/components/requests/operations/ManageSystemInfoOperation.vue
    Fetch system info with isListMode influence (pass boolean)
Enhancement ► apps/wallet/src/components/requests/operations/RemoveNamedRuleOperation.vue
    Fetch named rule with verified flag depending on list mode
► apps/wallet/src/components/requests/operations/RemoveRequestPolicyOperation.vue
    Fetch request policy with verified flag
Enhancement ► apps/wallet/src/components/users/InteractiveUserGroup.vue
    Use verified reads for fetching group users tooltip data
Enhancement ► apps/wallet/src/composables/autocomplete.composable.ts
    Integrate verified reads into various autocomplete fetch calls (user groups, users, accounts, address book, assets)
Enhancement ► apps/wallet/src/composables/request-policies.composable.ts
    Propagate verified reads through policy population and tooltips
Enhancement ► apps/wallet/src/composables/verified-reads.composable.ts
    Add new composable to provide/use verified reads context
► apps/wallet/src/composables/verified-reads.composable.spec.ts
    New test file for verified reads behavior

`InteractiveUserGroup` lists a group's members on hover and is rendered
inside the request dialog through rule summaries. Pass the dialog's verified
reads flag to its `listUsers` call, like the other shared components, so it
keeps using query calls elsewhere.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WAEJVi9S9JPmbMVGCycebk
@sea-snake
sea-snake merged commit c7ea54a into main Nov 2, 2026
29 checks passed
@sea-snake
sea-snake deleted the arshavir/compassionate-fermi-ksszck branch November 2, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants