Repository navigation
fix(policy): evaluate account bindings against the wrap-time path - #120
Merged
Merged
Conversation
Boka44
force-pushed
the
fix/freeze-policy-path
branch
from
October 12, 2026 10:54
3e4fbdb to
278651a
Compare
Two ways a path-bound rule could miss: the proxy re-read account.path on every call (a later change could dodge an active rule), and path was writable through the proxy (a spoofed path could make a future getAccount() skip the wrap). Capture the path at wrap time for evaluation, and refuse path/_path writes through the proxy. Closes tetherto#101
Boka44
force-pushed
the
fix/freeze-policy-path
branch
from
October 12, 2026 11:03
278651a to
9d05a12
Compare
|
Hi @Boka44, thanks for picking this up. This is the fix from my #112 (opened 4 Oct, closed today): the same branch name, the path captured at wrap time on the enforcement context, and the If you go with this version, could you add me as a co-author on the commit? I ran the four attack tests from #112 ( |
AlonzoRicardo
suggested changes
Oct 12, 2026
path is a prototype getter on real accounts, so the set/defineProperty traps did not actually stop spoofing (reassignment, prototype swap, or returning the raw account from an added method) from making a later getAccount() skip the wrap. Record each raw account's path in a WeakMap the first time WDK sees it and evaluate against that. Covers the spoof routes and the simulate path.
|
Thanks @Boka44. e656146 covers what @AlonzoRicardo raised, and it is the same first-sight WeakMap I had on a branch built on your 9d05a12, so I'm not offering that code. Two gaps are still open in the tests:
I added a test for each on top of your branch, in one commit that changes only tests: bookingminds@600ccf3
One heads-up: e656146 removes the traps that #121 edits, so the two PRs now conflict in For the record, my #112 had the same gap that only the traps covered. Credit to @AlonzoRicardo for finding it. |
The .d.ts typedef was missing path and still described account as read for the derivation path.
1 task done
Add a test asserting simulate.getXProtocol evaluates against the wrap-time path; it fails if the mirror reverts to re-reading account.path.
AlonzoRicardo
approved these changes
Oct 16, 2026
AlonzoRicardo
approved these changes
Oct 16, 2026
This was referenced Oct 19, 2026
jonathunne
pushed a commit
to jonathunne/wdk
that referenced
this pull request
Nov 11, 2026
The policy engine is opt-in and imports nothing from the rest of core,
only zod, yet every install of @tetherto/wdk ships it. It moves to
tetherto/wdk-policies and is published as @tetherto/wdk-policies, a
plugin that attaches to a WDK instance. Core keeps one middleware hook
so the plugin can still govern every account the instance hands out.
Removed:
- src/policy/, types/src/policy/ and docs/policy-exclusions-audit.md.
- WDK#registerPolicy, WDK#getPolicyExclusions, the constructor options
maxConditionTimeoutMs and policyExclusions, _applyPolicies, and the
engine calls in dispose.
- From index.js: PolicyViolationError, PolicyConfigurationError,
DEFAULT_POLICY_EXCLUSIONS, DENIAL_CODES and the policy typedefs.
- tests/wdk-policy.test.js, which moves with the engine.
- The policy sections of README.md and AGENTS.md, replaced by a
pointer to the new package.
- zod from dependencies.
Changed, in _runMiddlewares:
- A middleware receives a second argument, { blockchain, index, path }.
index is absent for getAccountByPath; path is the account's full
derivation path.
- A middleware's non-nullish return value replaces the account. The
next middleware, and the caller of getAccount, get the replacement.
- Middlewares run after _registerProtocols, where the engine used to
wrap the account, so the account a middleware returns already has
the protocol getters.
Kept from the engine fixes that landed before the move:
- tetherto#117: protocols are still built when a getter is called, through
account._buildProtocol(category, label, subject). The plugin passes
its governed account as the subject, so a protocol registered on the
account is built with it. A protocol registered with
wdk.registerProtocol is still built with the original account.
- tetherto#120: path is the one recorded the first time WDK saw the account,
not a fresh read of account.path, so a later change to the account
cannot change the path middlewares receive.
New typedef MiddlewareContext. MiddlewareFunction becomes
(account, context) => Promise<account | void>. The middleware tests
expect the context argument and gain two replacement cases, a
first-seen path case, and two cases for how _buildProtocol treats a
middleware's wrapper.
A middleware that returns a value by accident now replaces the
account.
Version 1.0.0-beta.21.
Refs tetherto#122
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Account-scope policies are resolved by the account's derivation
path. Two gaps let a path-bound rule miss:account.pathon every intercepted call, so a change to the account's path after the proxy was built could make an active rule stop matching.account.pathon every retrieval. Because the wallet manager caches account instances, if the path changed after the account was first governed, a latergetAccount()could read the changed path, decide the account is ungoverned, and hand back an unwrapped account.Motivation and Context
pathwhen the proxy is created (the same value already used to decide whether to wrap) and store it on the enforcement context. Each intercepted call evaluates account-scope bindings against that capturedpathinstead of re-readingaccount.path.pathin aWeakMapthe first time it is seen (in_applyPolicies), and use that first-seen value for the wrapping decision on every later retrieval. On a real accountpathis a prototype getter the proxy cannot stop from being reassigned, so this first-sight capture — rather than proxyset/definePropertytraps — is what keeps a latergetAccount()from being handed a spoofed, unwrapped account.Related Issue
Closes #101
Type of change