bank: resolveOwner() prefers the caller-supplied owner over the session principal, so 10 actions serve another customer's data

[Yaraslaut]

Summary

bank::resolveOwner() prefers a caller-supplied owner name over the
authenticated session principal, and never compares the two. Ten actions across
eight models resolve their scope through it, so an authenticated client that
names another customer in the request body is served that customer's data — and
in one case writes to it.

This is a confused-deputy: the server has the authenticated identity available
and chooses the request's assertion over it.

Found while giving bank a wire-drivable server and scenario corpus
(morph#87 / PR #470). The corpus pins the current behaviour as expect ok
so the defect is visible and a fix has a failing test to turn green — it does
not endorse it.

The mechanism

examples/bank/include/bank/core/principal.hpp:24:

/// @brief Returns @p explicitOwner if non-empty, otherwise the session principal.
[[nodiscard]] inline std::string resolveOwner(const std::string& explicitOwner) {
    return explicitOwner.empty() ? sessionPrincipal() : explicitOwner;
}

The session principal is consulted only when the caller supplies nothing.
A non-empty action.owner is taken verbatim. The file's own header comment
says these helpers are for "reading the authenticated principal from the morph
session context" — resolveOwner is the one that defeats that.

Affected actions — 10 call sites, 8 models

file:line	action
card_model.cpp:122	ListCards
customer_model.cpp:42	OpenAccount
customer_model.cpp:62	ListAccounts
notification_model.cpp:50	ListNotifications
notification_model.cpp:80	MarkAllRead
payee_model.cpp:58	ListPayees
payment_model.cpp:140	ListPayments
statement_model.cpp:21	GenerateStatement
budget_model.cpp:72	ListBudgets
loan_model.cpp:116	ListLoans

Nine are reads. MarkAllRead is a write — it mutates another owner's
notification rows.

Two aggravating details from PR #470's investigation, to be re-verified by
whoever fixes this:

• SpendingByKind consults no owner at all, so it answers a caller with no
  session.
• Id-addressed actions are not affected — those resolve the row first and
  check its owner, which is the pattern the ten above should follow. The fix
  therefore has a working in-tree model to copy rather than needing a design.

Why this matters in an example

examples/ is the ladder's teaching material — LADDER.md presents these
rungs as the reference for how a morph application is built, and bank is cited
in its intro as prior art. A scope-by-request-body pattern copied out of it
lands in real applications. The framework side is fine: morph::session
carries the authenticated principal correctly and the model simply declines to
use it.

Fix direction

Make the session principal authoritative for these ten actions. Either:

1. Ignore action.owner entirely and always scope to sessionPrincipal()
   — simplest, and correct if no legitimate caller sets it. Check the GUI and
   CLI first: if they populate owner, this changes their behaviour.
2. Keep the field but verify it: refuse when action.owner is non-empty
   and differs from the session principal. Preserves any legitimate
   administrative use, and makes the refusal explicit.

Whichever is chosen, decide deliberately what an empty session means — today
it silently scopes to "". Refusing is almost certainly right; the scenarios
pin the current answer either way.

Do not fix this by removing owner from the DTOs without checking the wire
surface, and note resolveOwner is inline in a header used by both models
and (potentially) client code.

Acceptance criteria

• A scenario in which one signed-in customer names another customer and is
  refused — the corpus already has the shape, currently pinned expect ok;
  flipping it is the test.
• The same for MarkAllRead, asserting the other owner's rows are unchanged.
• A decision recorded for SpendingByKind and for the empty-session case.
• The existing 22 bank scenarios still pass, twice.

Out of scope

• morph#87's ladder-slot question.
• Bank's server trusting the client's asserted principal at all (its AuthModel
  mints no token) — that is a separate, larger gap documented in PR bank: give it a server, and a scenario corpus over all 41 actions #470's
  README changes. This issue is narrower: even taking the asserted principal
  at face value, these ten actions ignore it.

🤖 Filed with Claude Code

https://claude.ai/code/session_01C6uB9zxdSG8qp3VNAAyFvF

[Yaraslaut]

Correction to this issue's own summary: there are two writes, not one.

The body says "Nine are reads. MarkAllRead is a write." That undercounts.
OpenAccount is also a write, and arguably the worse of the two —
customer_model.cpp:38-58:

dto::AccountInfo CustomerModel::execute(const dto::OpenAccount& action) {
    ...
    const std::string owner = resolveOwner(action.owner);
    if (owner.empty()) {
        throw Unauthorized{"no session principal to own the account"};
    }
    db::AccountRecord rec;
    db::setReference(rec.user, db::requireUserId(mapper(), owner));
    ...
    mapper().Create(rec);

So the resolved owner is not merely a filter here: it becomes the owning
user of a newly created account row. An authenticated caller naming another
customer creates an account in that customer's name.

Note the owner.empty() guard directly below resolveOwner — the code checks
that an owner exists and then trusts whichever one arrived, which is the
whole defect in miniature: the presence of the principal is validated, its
authority is not.

Two consequences for the fix:

• Both resolutions in "Fix direction" must be evaluated against a create,
  not just a query. Option 1 (ignore action.owner, always use the session
  principal) silently changes who owns rows this action creates — check the
  GUI and CLI populate owner the way you expect before taking it. Option 2
  (refuse a mismatch) is the safer default here for exactly that reason.
• The acceptance criteria need a third case: one signed-in customer naming
  another in OpenAccount must be refused, and the assertion should be that no
  row was created for the named owner — not merely that the call returned an
  error.

The corrected tally is 10 call sites across 8 models: 8 reads, 2 writes
(MarkAllRead, OpenAccount). The table in the body is otherwise accurate;
only the reads/writes sentence beneath it was wrong.

Found while documenting this defect for PR #470, whose own commit message
listed eight sites rather than ten.