Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/policy/policy-account-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ import PolicyViolationError, { PolicyConfigurationError } from './policy-error.j
* The per-account state every enforced method closes over.
*
* @typedef {Object} EnforcementContext
* @property {IWalletAccount} account - The raw account, read for its derivation path when resolving account-scope bindings.
* @property {IWalletAccount} account - The raw account the proxy wraps.
* @property {IWalletAccountReadOnly} readOnlyAccount - The read-only view handed to condition functions as `context.account`.
* @property {string} blockchain - The wallet identifier (the same string passed to `registerWallet`).
* @property {string | undefined} path - The derivation path captured at wrap time, used to resolve account-scope bindings. Captured here rather than re-read from `account.path` on each call so a later change to `account.path` cannot make a path-bound rule miss.
* @property {number | undefined} index - The index passed to `wdk.getAccount(wallet, index)`, when the account
Comment thread
Boka44 marked this conversation as resolved.
Resolved
* was retrieved that way.
* @property {PolicyEngine} engine - The engine that evaluates each intercepted call.
Expand Down Expand Up @@ -174,7 +175,7 @@ export async function createPolicyEnforcedAccount (account, { blockchain, path,

const readOnlyAccount = await account.toReadOnlyAccount()

const ctx = { account, readOnlyAccount, blockchain, index, engine }
const ctx = { account, readOnlyAccount, blockchain, path, index, engine }

const substitutions = new Map()

Expand Down Expand Up @@ -251,7 +252,7 @@ function buildEnforcedMethod (name, boundOriginal, ctx) {
args: forwardedArgs
})

const verdict = await ctx.engine._evaluateContext(context, { path: ctx.account.path, index: ctx.index })
const verdict = await ctx.engine._evaluateContext(context, { path: ctx.path, index: ctx.index })

if (verdict.outcome === 'BLOCK') {
throw new PolicyViolationError({
Expand Down Expand Up @@ -370,7 +371,7 @@ function buildSimulateMirror (methodNames, ctx) {
args
})

return ctx.engine._simulateContext(context, { path: ctx.account.path, index: ctx.index })
return ctx.engine._simulateContext(context, { path: ctx.path, index: ctx.index })
Comment thread
Boka44 marked this conversation as resolved.
Resolved
}
}

Expand All @@ -395,7 +396,7 @@ function buildSimulateMirror (methodNames, ctx) {
args
})

return ctx.engine._simulateContext(context, { path: ctx.account.path, index: ctx.index })
return ctx.engine._simulateContext(context, { path: ctx.path, index: ctx.index })
}
}

Expand Down
17 changes: 16 additions & 1 deletion src/wdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ export default class WDK {

/** @private */
this._decoratedAccounts = new WeakSet()

/** @private */
this._accountPaths = new WeakMap()
}

/**
Expand Down Expand Up @@ -341,7 +344,19 @@ export default class WDK {

/** @private */
async _applyPolicies (account, { blockchain, index }) {
return this._policyEngine.applyPoliciesTo(account, { blockchain, path: account.path, index })
// Record each raw account's derivation path the first time WDK sees it, and
// route policy evaluation through that. `account.path` is a prototype getter
// on real wallet accounts, so re-reading it lets a caller spoof the path
// (reassigning it, swapping the prototype, or returning the raw account from
// an added method) and make a later `getAccount()` skip the wrap.
let path = this._accountPaths.get(account)

if (path === undefined) {
path = account.path
this._accountPaths.set(account, path)
}

return this._policyEngine.applyPoliciesTo(account, { blockchain, path, index })
}

/** @private */
Expand Down
102 changes: 102 additions & 0 deletions tests/wdk-policy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2911,6 +2911,108 @@ wdk.registerPolicy({
}
})
})

describe('account path binding integrity', () => {
const allowAll = { id: 'allow-all', name: 'allow-all', scope: 'project', rules: [{ name: 'a', operation: '*', action: 'ALLOW', conditions: [] }] }
const freeze = { id: 'freeze', name: 'freeze', scope: 'account', wallet: 'ethereum', accounts: [PATH_DEFAULT], rules: [{ name: 'deny', operation: 'sendTransaction', action: 'DENY', conditions: [] }] }

test('a path-bound DENY is evaluated against the wrap-time path even if the account path changes afterwards', async () => {
const raw = buildAccount()
getAccountMock.mockResolvedValue(raw)

wdk
.registerWallet('ethereum', WalletManagerMock, {})
.registerPolicy(allowAll)
.registerPolicy(freeze)

const account = await wdk.getAccount('ethereum', 0)

raw.path = "0'/0/9"

const err = await catchAsync(() => account.sendTransaction({ to: RECIPIENT, value: 1n }))

expect(err.policyId).toBe('freeze')
})

test('simulate is evaluated against the wrap-time path even if the account path changes afterwards', async () => {
const raw = buildAccount()
getAccountMock.mockResolvedValue(raw)

wdk
.registerWallet('ethereum', WalletManagerMock, {})
.registerPolicy(allowAll)
.registerPolicy(freeze)

const account = await wdk.getAccount('ethereum', 0)

raw.path = "0'/0/9"

const result = await account.simulate.sendTransaction({ to: RECIPIENT, value: 1n })

expect(result.decision).toBe('DENY')
expect(result.policy_id).toBe('freeze')
})

test('simulate.getXProtocol is evaluated against the wrap-time path even if the account path changes afterwards', async () => {
class MySwapProtocol extends SwapProtocol {
async swap () { return DUMMY_SWAP_RESULT }
}

const freezeSwap = { id: 'freeze', name: 'freeze', scope: 'account', wallet: 'ethereum', accounts: [PATH_DEFAULT], rules: [{ name: 'deny', operation: 'swap', action: 'DENY', conditions: [] }] }

const raw = buildAccount()
getAccountMock.mockResolvedValue(raw)

wdk
.registerWallet('ethereum', WalletManagerMock, {})
.registerProtocol('ethereum', 'velora', MySwapProtocol, {})
.registerPolicy(allowAll)
.registerPolicy(freezeSwap)

const account = await wdk.getAccount('ethereum', 0)

raw.path = "0'/0/9"

const result = await account.simulate.getSwapProtocol('velora').swap({ tokenIn: 'A', tokenOut: 'B', tokenInAmount: 1n })

expect(result.decision).toBe('DENY')
expect(result.policy_id).toBe('freeze')
})

test('a later getAccount() stays governed after the account path is reassigned', async () => {
const raw = buildAccount()
getAccountMock.mockResolvedValue(raw)

wdk.registerWallet('ethereum', WalletManagerMock, {}).registerPolicy(freeze)

await wdk.getAccount('ethereum', 0)

raw.path = "0'/0/9"

const account = await wdk.getAccount('ethereum', 0)
const err = await catchAsync(() => account.sendTransaction({ to: RECIPIENT, value: 1n }))

expect(err.policyId).toBe('freeze')
})

test('a later getAccount() stays governed after the prototype path getter is swapped', async () => {
const raw = buildAccount()
delete raw.path
Object.setPrototypeOf(raw, { get path () { return PATH_DEFAULT } })
getAccountMock.mockResolvedValue(raw)

wdk.registerWallet('ethereum', WalletManagerMock, {}).registerPolicy(freeze)

await wdk.getAccount('ethereum', 0)

Object.setPrototypeOf(raw, { get path () { return "0'/0/9" } })

const account = await wdk.getAccount('ethereum', 0)
const err = await catchAsync(() => account.sendTransaction({ to: RECIPIENT, value: 1n }))

expect(err.policyId).toBe('freeze')
})
})
// -------------------------------------------------------------------------
// Method coverage: deny-by-default proxy + exclusion set
// -------------------------------------------------------------------------
Expand Down
6 changes: 5 additions & 1 deletion types/src/policy/policy-account-proxy.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function createPolicyEnforcedAccount(account: IWalletAccount, { blockchai
*/
export type EnforcementContext = {
/**
* - The raw account, read for its derivation path when resolving account-scope bindings.
* - The raw account the proxy wraps.
*/
account: IWalletAccount;
/**
Expand All @@ -48,6 +48,10 @@ export type EnforcementContext = {
* - The wallet identifier (the same string passed to `registerWallet`).
*/
blockchain: string;
/**
* - The derivation path captured at wrap time, used to resolve account-scope bindings. Captured here rather than re-read from `account.path` on each call so a later change to `account.path` cannot make a path-bound rule miss.
*/
path: string | undefined;
/**
* - The index passed to `wdk.getAccount(wallet, index)`, when the account was retrieved that way.
*/
Expand Down
Loading