Payments 7/8: Security hardening of payment authorization boundaries (guardian multisig invoker fix + auth audit) #144

Problem / Context

contracts/guardian_multisig/src/lib.rs's confirm_veto contains a literal placeholder instead of real caller attribution:

pub fn confirm_veto(env: Env, governance_contract: Address, proposal_id: u32) {
    let caller = env.current_contract_address(); // placeholder — real impl uses invoker

(line 79-80). env.current_contract_address() returns the multisig contract's own address, not the calling owner's — meaning veto confirmations are not actually attributed to distinct owners today. This breaks the N-of-M threshold guarantee that the Guardian Multisig exists to provide: per the README's own risk model, the Guardian's veto over Strategist proposals is one of only two privileged, fund-adjacent authority paths in the protocol, and it is currently non-functional as a distinct-owner threshold check.

Separately, #138 changes the storage model and call patterns across all four tier vaults (asset-scoped keys, standardized signatures). Every payment-moving require_auth() boundary in the router-to-vault-to-token call chain should get a dedicated security pass once that structural change lands, rather than treating authorization correctness as an implicit side effect of a feature PR.

Objective

Close the guardian-multisig authorization gap, and verify that every payment-moving code path enforces the correct, minimal authorization boundary consistent with the protocol's stated non-custodial design.

Technical approach

• Fix confirm_veto to use the actual transaction invoker (Soroban's auth-context / invoker mechanism, or require the caller to pass and require_auth() their own address), and verify that address is a registered owner before recording a confirmation.
• Add tests proving: a non-owner cannot confirm; the same owner cannot double-count toward the threshold by calling repeatedly; the veto dispatches to Governance::veto exactly once, exactly at the threshold — not before, not repeatedly after.
• Audit the require_auth() chain across the (post-Payments 1/8: Fix VaultRouter <-> tier-vault ABI mismatch and unify asset-scoped accounting #138) vault interfaces: confirm that VaultRouter.deposit/withdraw/early_exit's user.require_auth() cannot be bypassed by calling a tier vault directly. Today, tier vaults gate their mutating functions on admin.require_auth() where admin = VaultRouter (see contracts/vault_flex/src/lib.rs line 37 and the equivalent in vault_l3) — verify this pattern holds, unchanged in spirit, across all four vaults after Payments 1/8: Fix VaultRouter <-> tier-vault ABI mismatch and unify asset-scoped accounting #138's refactor, so a caller can't skip router-level checks (the pause flag, the asset allowlist) by invoking a vault's deposit/withdraw directly.
• Review the two-hop transfer pattern (user -> vault on deposit, vault -> user on withdraw) for re-entrancy exposure under Soroban's host-function model — extending the internal audit's own stated follow-up ("Review of Governance execution path for re-entrancy... under Soroban's host-function model," audit/internal-2026-01.md line 74) to the payment path specifically, not just governance.
• Document the authorization model explicitly as part of this PR: who can call what, and what each require_auth() call actually proves at each hop of the payment path. This currently exists only implicitly in code with no single reference.

Detailed scope

contracts/guardian_multisig/src/lib.rs, contracts/vault_router, contracts/vault_flex/vault_l3/vault_l6/vault_l12 (authorization-boundary verification, building on #138's structural changes — not a re-implementation), and a new authorization-boundary document (either extending audit/ or a new SECURITY.md-adjacent note).

Important edge cases and failure scenarios

• Owner removed or rotated mid-veto-proposal — confirm_veto must not allow a stale confirmation count to persist incorrectly against the new owner set.
• Regression coverage: existing tests must be extended so they fail against the current placeholder behavior, proving the fix actually changes the outcome rather than just adding parallel, unused code.
• A malicious or buggy adjacent contract attempting to call a tier vault's deposit/withdraw directly (bypassing VaultRouter) must be rejected by the vault's own admin-gated require_auth().
• Guardian pause/unpause (contracts/vault_router/src/lib.rs lines 114-133) is itself authorization-critical — confirm it remains callable only by the registered Guardian address after Payments 1/8: Fix VaultRouter <-> tier-vault ABI mismatch and unify asset-scoped accounting #138's changes, with no incidental widening of who can trigger it.

Dependencies

Depends on #138 — the authorization-boundary audit needs to review the final, post-refactor vault interfaces, not the current mismatched ones.

Acceptance criteria

• confirm_veto correctly attributes confirmations to the actual calling owner; a dedicated test suite proves non-owner and duplicate-owner confirmations are both rejected.
• A committed authorization-boundary review covers every payment-moving entrypoint: deposit, withdraw, early_exit, relock, pause, unpause, harvest, confirm_veto.
• No payment-moving contract function is callable without its intended require_auth() check, verified by tests that attempt to bypass each one directly.

Definition of done

PR merged; the guardian_multisig placeholder is removed and replaced with a real, tested invoker check; the authorization-boundary document is committed; cargo test is green including the new negative-authorization tests.