feat(server)!: require an explicit refAccess on createTenantStore

[bokelley]

Addresses the third item in #2428 — the one that needed a product decision rather than a patch.

Why

refAccess defaulted to 'ref-routed', under which accounts.resolve returns whatever tenant the buyer's ref names without consulting the authenticated principal (tenant-store.ts:217-235).

That default is correct for an agency hub whose single credential legitimately spans tenants — the model the helper was designed for, documented in its own header. It is a cross-tenant spend hole for a hub whose tenants are unrelated clients, because resolve is the account path for create_media_buy and update_media_buy, not just reads.

Nothing in the code can tell those two deployments apart. So any default silently picks one, and the wrong pick is invisible until someone notices another tenant's budget moving.

To be clear about what this is not: the previous behavior was not a bug. It was documented in three places, has a fail-closed alternative shipped, and has a characterization test named "default ('ref-routed') resolves a cross-tenant ref without checking the caller". A codex-security scan flagged it as a vulnerability; on the code, that was refuted. The problem is the default, and the docs.

What changed

refAccess is required. Neither behavior changed; only the choice is now explicit.

	resolve with a ref naming another tenant	Use when
'auth-scoped'	returns null → ACCOUNT_NOT_FOUND	Tenants are unrelated clients. Most hubs.
'ref-routed'	returns that tenant's Account	One credential is supposed to span tenants

Migration is one line per call site. Considered and rejected: flipping the default to 'auth-scoped', which would break agency-hub adopters quietly — their calls start returning null and they debug it as a resolution bug. A required field breaks them at tsc, before they ship.

The type alone isn't enough, so the helper also throws at construction on a missing or unrecognized value. A JS adopter or an as any cast would otherwise fall through to the permissive branch silently — the exact failure the field exists to prevent.

The docs were the larger half

skills/build-holdco-agent/SKILL.md is the file an adopter actually follows. Its front-matter promises "per-tenant data isolation", its § "Account resolution + tenant-isolation gate" presents createTenantStore as the answer — and it mentioned refAccess zero times. Its "What the helper guarantees" list covers upsertRow / syncGovernanceRow / non-writability and omits resolve entirely.

So an adopter could follow the skill end to end, verify every guarantee it lists, and ship cross-tenant resolve believing they had isolation. Worse, the guarantee they did verify — the per-entry gate — covers sync_accounts / sync_governance only, which is a genuinely separate decision from refAccess.

That skill now documents the choice with a decision table, states the consequence of getting it wrong, and gains a "What the helper does NOT guarantee" section. skills/cross-cutting.md no longer describes the helper as an unqualified "built-in isolation gate", and decisioning/account.ts's header no longer says resolve "is NOT an isolation gate by default" (there is no default now).

Verification

• 12,690 pass, 0 fail on the full suite. No other construction site broke.
• Two new tests cover the runtime refusal (omitted, and unrecognized value).
• The characterization test is renamed off "default" and passes 'ref-routed' explicitly; the four 'auth-scoped' tests are unchanged.
• examples/hello_seller_adapter_multi_tenant.ts takes 'auth-scoped' — each buyer agent_url maps to exactly one home tenant there, so a ref naming another must fail closed. Its storyboard tests pass unchanged, which confirms the example never relied on cross-tenant resolution.

Changeset is major. Timed for the 13.0.0-rc line, and createTenantStore is Preview / 6.x surface, so the bar for a required-field change is lower than it would be for stable API — but it's worth checking how many adopters are on this helper before merging.

Not in scope

The other two items in #2428 are ordinary fixes, unaffected by this and still open: the test-controller bridge admitting on a buyer-supplied sandbox marker when only resolveAccount is wired, and the comply controller whose ADCP_SANDBOX / ADCP_COMPLY_CONTROLLER_UNGATED env flags suppress the warning rather than gating anything.

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Approve

Approve — security-hardening PR making createTenantStore's refAccess required with no default, backed by a fail-closed construction guard.

Verified clean:

• Guard logic, resolve path, and new throw paths (with test coverage)
• Major changeset correctly reflects the required-param flip / breaking change
• Call-site sweep and example/doc updates are consistent
• No witness-not-translator or transport-client concerns

No blocking or Medium findings reported by the reviewer.

Gate checks: gated_paths is false, so row 2 does not apply despite review_decision: REVIEW_REQUIRED. No no-auto-approve team match, high_risk false, no prior decision. No findings at any severity → falls through to row 9.