fix(security): harden AAO application boundaries

[bokelley]

What

Remediates the actionable findings from the latest Codex Security CLI review of the AdCP 3.2 work. All findings were in the AAO website/Addie application under server/src; none required changes to the underlying protocol or SDK.

• require current WorkOS organization membership and direct user/org ban checks for mutable MCP requests and sponsorship changes
• harden Slack/Addie prompt boundaries by principal-scoping stored history and fencing user-editable channel context
• constrain session bridging, remote logo/RSS/manifest fetching, and public agent-card validation against cross-origin, SSRF, body-size, queue, and timeout abuse
• require current owner/admin authority for new membership charges
• persist one immutable checkout attempt per organization, with stable Stripe idempotency, safe retries, and exact completion semantics

Why

Several application flows treated signed or previously stored context as current authority, allowed untrusted remote content or conversation history to cross trust boundaries too freely, or performed billing decisions before an external write without a durable replay record. The changes move authorization to the point of mutation and make resource and payment operations bounded and retry-safe.

Impact

• Adds migration 546_membership_checkout_attempts.sql.
• Ordinary organization members can no longer initiate a membership invoice or checkout; current owners/admins can.
• All actionable scan findings are remediated; the LLM prompt-boundary item is addressed as defense in depth.

Validation

• Code, security, and Node.js testing expert reviews: no remaining blockers
• TypeScript typecheck: passed
• Migration tests: passed
• Root test suite: 1,043 passed
• Changed-area server tests: passed
• Full server suite: 6,270 passed and 30 skipped; two unrelated parallel-only admin-route 404 flakes passed 20/20 when rerun together in isolation
• git diff --check: clean

The local pre-commit hook's duplicate full-server-suite step exceeded its fixed 240-second wrapper after the tests had already been exercised manually; GitHub CI is the merge authority for this PR.

[aao-secretariat]

Ladon verdict: Approve

Approve — app-only security hardening under server/src/** with strong test coverage.

What I checked

• No protocol surface touched (static/schemas/source/**, docs/reference/**, dist/**, task/compliance assets) → no changeset, spec-drift, oneOf, immutability, or 3.1.x-patch gates apply.
• Migration 546 is additive (new membership_checkout_attempts table, FK to organizations.workos_organization_id PK); judged as production-facing and sound.
• Checkout idempotency-key handoff collapses concurrent Stripe writes to one stored key; referral consumed exactly once.
• Agent-card semaphore FIFO slot transfer — no leak; bounded-response cancellation and safeFetch SSRF handling verified.
• high_risk: true reasons are all (modified) app files plus one (added) migration — no deletions. No medium finding lands on a high-risk modified file, so row 5 does not fire.

Findings

• 1 medium finding (MCP principal gate calls WorkOS on the default unbounded client with no timeout, principal-authorization.ts:51). This is an operability/timeout gap on an external call — a legitimate medium, but a single medium with no escalation category (not data-loss/schema/infra).

Decision path: No critical/high (row 1 no). gated_paths false (row 2 no). No deletions (row 3 no). Sole medium is not data-loss/schema/infra category (row 4 no). No medium on a modified high-risk file (row 5 no). No prior escalation (row 6 no). No no-auto-approve team match (row 7 no). Only 1 medium, <3 (row 8 no). Falls through to row 9 → approve.

Medium findings

• server/src/mcp/principal-authorization.ts:51 — MCP principal gate calls WorkOS on default client with no timeout on the hot path (worth a follow-up: add a bounded timeout).

Medium findings

• server/src/mcp/principal-authorization.ts:51 — MCP principal gate calls WorkOS on default client with no timeout on the hot path

[aao-secretariat]

Ladon verdict: Approve

Approve — application-only security-hardening PR with no blocking findings.

Scope check: touches only server/src/** plus tests. No static/schemas/source/**, docs/reference/**, dist/**, task definitions, or package.json version — so no changeset/schema-coherence/oneOf/immutability/patch-eligibility rule applies, and this is not a protocol decision-class change. No no-auto-approve team match; gated_paths is false.

High-risk flag is true, but the only high-risk match that is not a (modified) existing file is the new migration 546_membership_checkout_attempts.sql (added) — inherent low risk. The modified high-risk files (addie/**, routes/**) carry no medium-or-higher findings, so per the change-kind guidance they are presumed safe (row 5 does not fire without a medium finding, and no medium exists).

Reviewer verified the load-bearing logic: agent-card semaphore is a balanced token-passing gate with bounded waiter queue + timeout (no slot leak); MCP principal auth fails closed (503 on dependency error, 403 on denial) with rate limiter ordered ahead of WorkOS/DB fan-out; membership checkout claims run under per-org advisory lock with immutable payload-hash-bound Stripe idempotency key (replay avoids a second write, completion single-shot so referral discounts can't double-consume). Operational readiness holds: 20s Stripe client timeout, safeFetch+AbortSignal on new outbound fetches, stream readers cancel oversized bodies before buffering.

No Critical/High/Medium findings survived verification. Decision table: rows 1–8 do not fire (no critical/high, gated_paths false, no deletion, no medium, no prior escalation, no team gate, fewer than three mediums) → row 9 approve.

[aao-secretariat]

Ladon verdict: Approve

Approve — no blocking or medium findings.

This PR swaps the WorkOS client in authorizeMCPPrincipal from getWorkos() to the bounded getPipesWorkos() (10s timeout, maxRetries: 0) for the per-request membership check, with the test mock updated to match. The caller in routes.ts already fails closed (503 on error), so bounding the SDK budget keeps the 503 inside the edge request window instead of after retry fan-out — a clean, well-justified operability change.

No protocol surface (static/schemas/source/**, docs/reference/**, dist/**) is touched, so schema↔docs coherence, changeset, oneOf, and artifact-immutability gates do not apply. The added migration 546_membership_checkout_attempts.sql is a new file (inherently low risk on its own); the reviewer surfaced no concerns on it or any other high-risk-path file.

Decision-table walk: no critical/high findings (row 1 n/a); gated_paths false (row 2 n/a); no deletions in high-risk reasons (row 3 n/a); no medium findings at all (rows 4/5/8 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve. high_risk is true only via modified/added files with zero findings, which is not an escalation trigger.

[aao-secretariat]

Ladon verdict: Approve

Approve — clean hardening delta over a prior clean pass, no blocking or medium findings.

What I checked:

• The delta since the prior approve (head 3c316f1) is three small hardening edits plus a test mock rename:
  • thread-utils.ts — fence-tag neutralizer switched from replace('<') to replaceAll('<'), closing a case where a single regex match spanning two < only escaped the first. Correct hardening.
  • principal-authorization.ts — per-request WorkOS membership check moved to getPipesWorkos() (10s timeout, maxRetries:0), consistent with the bounded-interactive intent and the fail-closed 503 handler in routes.ts.
  • membership-checkout-attempt.ts — comment + lgtm suppression on a non-secret SHA256 fingerprint.
  • Test mock renamed getWorkos→getPipesWorkos to match.
• No static/schemas/source/**, docs/reference/**, or dist/** changes — no schema↔docs coherence, changeset-scope, oneOf, or immutability concerns apply.
• Migration 546_membership_checkout_attempts.sql is an added file (not modified/deleted); high-risk flag on it is scaffolding, not escalation-worthy per the added-file rule.
• No high-risk (deleted) reasons; no medium findings on any (modified) high-risk file.

Gate checks: No no-auto-approve team match. gated_paths: false (row 2 does not apply despite review_decision: REVIEW_REQUIRED). No critical/high/medium findings. Prior decision was approve, so sticky escalation (row 6) does not apply. Falls through to row 9 → approve.

[aao-secretariat]

Ladon verdict: Approve

Approve — clean mechanical rename, no blocking or medium findings.

The delta is a mechanical rename: hashMembershipCheckoutPayload → fingerprintMembershipCheckoutPayload, replacing the SHA-256 digest with a deterministic JSON.stringify of a fixed-shape immutable payload, plus the payload_hash → payload_fingerprint column rename in migration 546. Both the migration and the checkout-attempt module are new files introduced in this same unreleased PR, so the in-place edit strands no data. Equality semantics are preserved and all call/test sites were moved in lockstep.

Checked:

• Migration 546 is an added file (not a modification to an existing released migration); high-risk flag is (added) scaffolding, not escalation-worthy.
• No static/schemas/source/** changes — no schema↔docs coherence, oneOf, or changeset-scope concerns.
• No released-artifact (dist/**) modifications.
• No no-auto-approve team match; gated_paths is false; no protocol/breaking-class change.

Reviewer surfaced no critical/high/medium findings. No decision-table row 1–8 fires, so this falls through to row 9 → approve. Prior decision on the earlier head was also approve, consistent with this clean pass.