Fold in cross_reviewer (GPT-4.1) auth/trifecta findings

Cross-family review caught defense-in-depth gaps:
- CR-1: validate aud at edge AND server-side (per-tier authorizer doesn't replace
  design.md §2.5 server enforcement); alarm on wrong-audience tokens.
- CR-6: finance-audience token must not reach Gmail/Calendar even with a Google token.
- CR-2: verify+enforce received sub is the Google Workspace sub, not a Salesforce id.
- CR-3: pre-token Lambda fails closed on cross-audience scope.
- CR-5: pre-token + group-sync are the auth SPOF — alarms + group-claim freshness bound.
- CR-4/CR-7: restrict per-client Cognito scopes; WAF is defense-in-depth only.
Reflected in §0.1 B3/B4, §1h tests, §4 monitoring, §5 cross-review log.
This commit is contained in:
Adam Moussa 2026-06-11 12:53:33 -04:00
parent 74e834a7c4
commit 52c58a5a0a

View file

@ -109,13 +109,17 @@ Agentforce per-user path (D4) is OpenAPI/Apex, not MCP. Decision:
- **Ship the OpenAPI adapter now** — Agentforce **External Service (OpenAPI) actions** are the primary transport;
use **Apex `@InvocableMethod`** only for actions needing request/response shaping (e.g. extra redaction,
pagination).
- **One facade per trust tier (B3-resolved — the audience boundary stays in infrastructure, not in shared
handlers).** Deploy a **separate API Gateway + Cognito authorizer per server**: `sh-mcp-ops` facade validates
`aud=sh-mcp-ops`, `sh-mcp-finance` facade validates `aud=sh-mcp-finance`. A token minted for ops is rejected at
the finance gateway's authorizer (and vice-versa) — the audience-binding rejection design.md §2.5 requires,
enforced at the edge. A **shared AWS WAF web ACL** fronts both. This is deliberately NOT one shared gateway with
audience checks pushed into application code: keeping per-tier audiences at separate authorizers preserves the
trust-tier firewall even if a handler bug slips through.
- **One facade per trust tier (B3-resolved — the audience boundary stays in infrastructure).** Deploy a
**separate API Gateway + Cognito authorizer per server**: `sh-mcp-ops` facade validates `aud=sh-mcp-ops`,
`sh-mcp-finance` facade validates `aud=sh-mcp-finance`. A token minted for ops is rejected at the finance
gateway's authorizer (and vice-versa). A **shared AWS WAF web ACL** fronts both (WAF is rate-limit/IP
defense-in-depth ONLY, never an authz boundary — CR-7).
- **Audience validated at the edge AND the server (defense in depth — CR-1, design.md §2.5).** The per-tier
authorizer is NOT the only check: every server **independently re-validates issuer + `aud` + required scope on
every call** (design.md §2.5 makes server-side enforcement authoritative — the edge does not replace it). The
per-tier gateway adds an early-reject layer; it does not move the boundary off the server. **Alarm on any token
presented to the wrong audience** (a finance-aud token at the ops endpoint, or vice-versa) — that signature
means either a misconfig or an attack.
- **Defer the MCP adapter** until a **named trigger**: (a) Claude Code / IDE consumption becomes a real recurring
workflow (not nice-to-have), **or** (b) Salesforce native remote-MCP per-user binding reaches GA (then MCP-native
could also collapse the Agentforce-side OpenAPI facade). When triggered, the MCP adapter is a thin add over the
@ -143,10 +147,23 @@ credential that requests **only its tier's scopes**. Mechanism (specified, testa
per-agent credential, not per-agent action assignment (which the plan concedes is "a convenience, never the
boundary," §1h).
- The Cognito pre-token Lambda must therefore scope the minted scopes to the requested `aud` (resource server),
not blanket-union across all audiences. **This is a per-server IAM/authz behavior → mandatory cross-review
not blanket-union across all audiences, and **fail closed**: if a scope from another audience would ever appear
in a token, reject and alarm (CR-3). **This is a per-server IAM/authz behavior → mandatory cross-review
(GPT-4.1) before commit** (design.md §8/§10).
- Tested by: "per-agent credential mints only its tier's scopes" (a token obtained via the Exec agent's
credential never contains `finance:read`) — added to the §1h security suite.
- **Backend `sub` validation (CR-2).** Do not assume Salesforce's External Credential forwards the Google `sub`
unchanged — verify in Phase-0a that the JWT our endpoint receives carries the **Google Workspace `sub`** (not a
Salesforce/Cognito-internal id), and have each server **reject any token whose `sub` is not a valid Workspace
user**. Force short token lifetimes; watch for Salesforce-side token caching/reuse across agents.
- **Gmail/Calendar segregation is defense-in-depth, not just credential-shaped (CR-6).** The Finance agent's
credential never carries `gmail:self`/`calendar:self`, but the servers must **also** refuse Gmail/Calendar tool
calls unless the token carries the matching `*:self` scope AND the Google token is ABAC-partitioned by `sub`
(design.md §2.4) — so a finance-audience token can never reach a mailbox even if something upstream misfires.
- **Pre-token + group-sync are the auth SPOF (CR-5).** Alarm on group-sync failure (design.md §2.3 already) AND on
pre-token Lambda error rate / any cross-audience-scope event; enforce a hard freshness bound on group claims so a
stale sync can't silently widen scope.
- Tested by (§1h): "per-agent credential mints only its tier's scopes" (a token via the Exec credential never
contains `finance:read`); "wrong-audience token rejected at edge AND server"; "finance token cannot reach a
Gmail/Calendar tool"; "pre-token fails closed on a cross-audience scope."
**Dropped claim:** the "BYOLLM = ~30% fewer Einstein Requests" figure was **not corroborated** by any verified
source — removed from cost modeling.
@ -175,6 +192,20 @@ and FIX is addressed below; this revision supersedes the pre-audit text wherever
| **N1/N2/N3** | §1f ingestion pinned to **S3 → Data Cloud**; ALARM-only monitoring for the facades + repointed `notion-sync`; Salesforce DevName/kebab boundary noted. |
| **Q1/Q2/Q3** | **G15** (Slack plan supports Employee Agents; per-employee Agentforce licensing; `$User.GoogleGroups`/`$Session.Channel` existence) — all unverified-load-bearing, on the §6 verify gate. |
**Cross-family review (GPT-4.1 `cross_reviewer`, 2026-06-11) — auth/trifecta sections.** Run per the mandatory
cross-review gate for auth/IAM design (Fable is Claude-family, not a substitute). Findings folded in:
- **CR-1 (BLOCK):** validate `aud` at the edge **AND** server-side (defense in depth) — the per-tier authorizer
does not replace design.md §2.5 server-side enforcement; alarm on wrong-audience tokens. → §0.1 B3, §1h.
- **CR-6 (BLOCK):** a finance-audience token must not reach Gmail/Calendar even with a valid Google token — backend
refuses unless `*:self` scope present + Google token ABAC-partitioned by `sub`. → §0.1 B4, §1h.
- **CR-2 (FIX):** verify + enforce that the received `sub` is the Google Workspace `sub` (not a Salesforce/Cognito
id); reject otherwise; watch Salesforce token caching. → §0.1 B4, Phase-0a.
- **CR-3 (FIX):** pre-token Lambda fails closed + alarms if any cross-audience scope would appear. → §0.1 B4, §1h.
- **CR-5 (FIX):** pre-token + group-sync are the auth SPOF — alarms + freshness bound on group claims. → §0.1 B4, §4.
- **CR-4/CR-7 (NIT):** restrict each Cognito app client's allowed scopes; WAF is defense-in-depth only. → §0.1.
Re-run `cross_reviewer` on the concrete pre-token Lambda + IAM/Cognito resource-server config once written
(design.md §10 asked for the real artifacts).
---
## 1. Platform-level design
@ -375,10 +406,13 @@ real home:
deprecating each bot (design.md §6, §7.3 parity gate).
**Core security tests (authoritative, transport-agnostic, in `sh-mcp`, design.md §7.3):**
**audience-binding rejection at the per-tier facade** (an `aud=sh-mcp-ops` token rejected by the `sh-mcp-finance`
gateway authorizer, and vice-versa — B3); per-tool **server-side** scope enforcement; **per-agent credential
mints only its tier's scopes** (a token obtained via the Lauren Exec app client never contains `finance:read`,
even though the person holds it — B4); deny-list **hard revocation**; minimal-scope Google client (a `gmail:self`
**audience-binding rejection at the per-tier facade AND re-validated server-side** (an `aud=sh-mcp-ops` token
rejected by the `sh-mcp-finance` gateway authorizer *and* by the finance server itself — B3/CR-1, defense in
depth); per-tool **server-side** scope enforcement; **per-agent credential mints only its tier's scopes** (a
token obtained via the Lauren Exec app client never contains `finance:read`, even though the person holds it —
B4); **pre-token fails closed on a cross-audience scope** (CR-3); **a finance-audience token cannot reach a
Gmail/Calendar tool** (CR-6); **backend rejects a token whose `sub` is not a valid Google Workspace user** (CR-2);
deny-list **hard revocation**; minimal-scope Google client (a `gmail:self`
token can't mint a Calendar token); **per-user refresh-token ABAC isolation**; **finance PII redaction**
(bank/routing/card/SSN masked before egress) **while leaving vendor names/contacts UNMASKED** (legacy Alex
deliberately left names unmasked — Trust Layer must not re-mask them, Gap G3); per-tool rate limit + per-session
@ -621,6 +655,9 @@ MCP layer) are unchanged — those stay locked.
only, no OK/no-data).
- Repointed **`notion-sync`** carries the design.md ALARM-on-sync-failure through to the dual-feed (two-alarm
pattern: Errors≥1 missing=notBreaching; Invocations<1 over cadence missing=breaching).
- **Auth SPOF (CR-5):** alarm on **group-sync Lambda** failure (design.md §2.3) and on **pre-token Lambda** error
rate + any **cross-audience-scope** event; enforce a freshness bound on group claims so a stale sync can't widen
scope silently. ALARM action only → site-alerts.
**Jira (INFRA project — no migration epic exists yet; related done: INFRA-92 AOSS lockdown, INFRA-37 reminder
removal):**