fix(gateway): add caller authorization to Delete / Suspend / Resume session endpoints

[sytone]

What

Closes the per-session caller-authorization gap on three session lifecycle endpoints that mirrored the gap fixed on Seal in #559:

• DELETE /api/sessions/{id} — Delete
• POST /api/sessions/{id}/suspend — Suspend
• POST /api/sessions/{id}/resume — Resume

Before this PR, any authenticated caller could destroy, suspend, or resume any other caller's session. After this PR, the same AuthorizeSessionCaller helper (introduced in #555 / #559) is invoked on all three endpoints — placed between the existing is null → 404 / NoContent guard and any state-machine guard.

Closes #558.

Why

The original conversation behind the domain-model refactor flagged inconsistent authorization across session endpoints as a key source of "slop and instability." This PR finishes the symmetric pattern: every session-modifying endpoint (Seal / PatchMetadata / Delete / Suspend / Resume) now enforces caller-ownership identically.

Changes

Two files. +228 / -1 LoC.

src/gateway/BotNexus.Gateway.Api/Controllers/SessionsController.cs (+27 / -1)

• Delete: now loads the session before deleting. Missing → idempotent NoContent (existing wire semantic preserved). Existing → AuthorizeSessionCaller(session) → DeleteAsync. Comment block explains the idempotency-vs-existence-disclosure trade-off and references [Gateway] Add per-session caller authorization to Delete / Suspend / Resume endpoints (security gap) #558.
• Suspend: AuthorizeSessionCaller check inserted between the existing is null → NotFound guard and the Status != Active → Conflict guard. Placement is deliberate — leaking "session is suspended" to a non-owner is also a small information disclosure.
• Resume: same shape as Suspend.

tests/gateway/BotNexus.Gateway.Tests/SessionsControllerTests.cs (+201 / 0)

10 new tests:

Endpoint	Tests
Delete	WhenCallerIdentityDoesNotMatchSessionCaller_Returns403_AndDoesNotDelete, WhenCallerIdentityMatchesSessionCaller_DeletesAndReturnsNoContent, WhenSessionMissing_ReturnsNoContent_PreservingIdempotency, WithMissingCallerIdentity_SkipsAuthorizationCheck
Suspend	WhenCallerIdentityDoesNotMatchSessionCaller_Returns403_AndDoesNotChangeState, WhenCallerIdentityMatchesSessionCaller_Suspends, WithMissingCallerIdentity_SkipsAuthorizationCheck
Resume	WhenCallerIdentityDoesNotMatchSessionCaller_Returns403_AndDoesNotChangeState, WhenCallerIdentityMatchesSessionCaller_Resumes, WithMissingCallerIdentity_SkipsAuthorizationCheck

Mismatched-caller tests assert both StatusCode == 403 and no state change (Delete: stillThere.ShouldNotBeNull; Suspend/Resume: session.Status unchanged from seed).

Validation

• Build: 0 warnings / 0 errors under TreatWarningsAsErrors.
• Targeted: SessionsControllerTests 67/67 passed (was 57 — +10).
• Full suite: BotNexus.Gateway.Tests 1832 passed / 1 skipped / 0 failed (was 1822). 1 known flake in BotNexus.Cron.Tests (timing-sensitive) — passed on re-run.
• Architecture fence: AgentKindArchitectureTests 5/5 (allowlist still 3 entries from feat(gateway): migrate SessionsController.Seal off SessionId.IsSubAgent substring (Phase 5 / F-6 step 2b) #559).

Critique sweep

Three independent models reviewed the change before this PR was opened. All findings either adopted, accepted as documented trade-off, or split out into follow-up.

Agent	Model	Verdict	Findings
authz-siblings-security	gpt-5.5	1 LOW	Existence oracle on Delete (204 missing vs 403 mismatched). Accepted as documented trade-off; see below.
authz-siblings-plan-vs-impl	claude-opus-4.7	1 MEDIUM	Delete was missing the standard WithMissingCallerIdentity_SkipsAuthorizationCheck test that all 5 sibling endpoints carry. Adopted — added in commit.
authz-siblings-bug-hunt	gpt-5.3-codex	1 "BLOCKING" + 3 non-blocking	"BLOCKING" was the same Delete oracle as security/LOW — rejected (see below). 3 non-blocking items split off to follow-up issue #560.

On the Delete 204/403 existence oracle (cross-reviewer disagreement)

The bug-hunt agent flagged this as BLOCKING; security agent flagged it as LOW; plan-vs-impl agent explicitly analyzed it and judged the design choice sound. I am rejecting the BLOCKING label and accepting the trade-off, with these reasons:

1. The oracle already exists on GET endpoints: GET /api/sessions/{id}/metadata returns 404 for missing and 403 for "exists but mismatched". The same existence channel is already accessible to any authenticated caller; closing only DELETE closes no information channel. (Source: plan-vs-impl agent's analysis.)
2. Session IDs are GUIDs: probing is impractical at scale.
3. The proposed fix (return 204 on authz failure) silently swallows authorization failure, breaking the principle of least surprise for legitimate callers who hit the wrong session. The UX regression is worse than the disclosure delta.
4. RFC 7231 §4.3.5: DELETE idempotency is the established HTTP semantic. Pre-PR wire contract on main already returned NoContent for missing sessions.
5. A unified existence-disclosure policy across ALL session endpoints — if desired — belongs in a separate design RFC, not a one-off DELETE fix. Tracked separately.

In-code comment block at SessionsController.cs:274-278 and test comment at SessionsControllerTests.cs:104-107 document this trade-off explicitly so future maintainers see it.

Other findings — adopted

• Plan-vs-impl MEDIUM — added Delete_WithMissingCallerIdentity_SkipsAuthorizationCheck test (commit-amended). Mirrors the same test on Suspend / Resume / Seal / GetMetadata / PatchMetadata.

Other findings — split to follow-up

• Bug-hunt non-blocking [WIP] Port nanobot to C# with extensible environment support #1 (store-level conditional delete API for stricter TOCTOU): scope creep, not a current vulnerability. Security agent verified CallerId can't be tampered with in practice (set once via ??=).
• Bug-hunt non-blocking security: guard Swagger, add daily secret scanning workflow, fix gitignore #2 (Suspend/Resume tests rely on InMemoryStore reference aliasing for "state unchanged"): known InMemoryStore limitation. Store-level round-trip coverage already exists in SessionStoreExistenceQueryTests + SessionModelWave2Tests.
• Bug-hunt non-blocking Add focused coverage for audio transcription handler and wire it into solution tests #3 (read endpoints Get / List / ListSubAgents / GetHistory / KillSubAgent also lack caller authz): filed as [Gateway] Add per-session caller authorization to read endpoints (Get / List / ListSubAgents / GetHistory / KillSubAgent) #560 for follow-up PR.

Mutation-resistance evidence

The new mismatched-caller tests catch the following mutations:

Mutation	Caught by
Remove AuthorizeSessionCaller call entirely	Delete: stillThere.ShouldNotBeNull after 403 attempt. Suspend/Resume: session.Status unchanged after 403 attempt.
Invert if (... is not null) to is null	Matching-caller tests fail (return shape mismatch).
Move authz after state-machine guard	Mismatched-caller Resume test seeds Status = Suspended; the test asserts 403 (not 409) — would catch reordering.
Move authz before is null guard	Idempotent-missing-DELETE test asserts NoContent (not 403); would catch reordering.

Related

• Predecessor: feat(gateway): migrate SessionsController.Seal off SessionId.IsSubAgent substring (Phase 5 / F-6 step 2b) #559 (Seal authz fold-in).
• Follow-up: [Gateway] Add per-session caller authorization to read endpoints (Get / List / ListSubAgents / GetHistory / KillSubAgent) #560 (read-endpoint authz on Get / List / ListSubAgents / GetHistory / KillSubAgent).
• Domain-model refactor umbrella: original conversation traced from [Platform] Primitive obsession Phase 2 -- create JobId, ChannelId, RunId value objects and remediate #501.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com