mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
fix(backend): SessionTenantLookup is claimed first-writer-wins at registration — the sessionId→tenantId row that every Global Admin/Reader cross-tenant session resolve trusts was an unconditional last-writer-wins upsert AFTER the session write keyed by the client-supplied sessionId, so any tenant's device (self-service onboarding) that knew a victim session GUID (portal deep links, Teams/Slack notifications, progress-portal URLs) could re-point the mapping at its own tenant and have every no-tenantId GA read and GA write (globaladmin annotation lane, restore fallback) silently served from a forged session; StoreSessionAsync now claims the row create-only BEFORE any Sessions/SessionsIndex write (409 → read owner: same tenant = re-registration, foreign tenant = SessionTenantConflictException → 409 session_owner_mismatch without disclosing the owner + SessionTenantConflict ops event, nothing written, not fail-soft), the legacy self-heal write is Add-only, ResolveSessionScopeAsync(requireGlobalAdmin) corroborates the resolved tenant against its Sessions row (silent, no operator confirmation), and scripts/Migration/Backfill-SessionTenantLookup.ps1 closed the pre-table window (13509 sessions, 8795 rows added, verify dry-run added=0); no tombstone on deletion by design
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -39,7 +39,9 @@ Every table in `Constants.TableNames` must belong to at least one deletion lifec
|
||||
- Unknown session → `TargetTenantId`, leaving each endpoint's own not-found semantics (404 / empty 200 / 400) in charge.
|
||||
- Delegated (MSP) callers deliberately get NO fallback — extending it to `AllowedTenantIds` is a pending user decision.
|
||||
|
||||
The lookup row is written at session registration (fail-soft), deleted by the cascade one step before the tombstone, and rebuildable from Sessions (not in the critical-backup set). The former lazy triggers (`events.Count == 0` → scan) are gone: empty-but-owned results no longer pay a table scan, and endpoints resolve upfront.
|
||||
The lookup row is **claimed at registration, first-writer-wins** (`ClaimSessionTenantLookupAsync`, create-only `AddEntity` BEFORE any Sessions/SessionsIndex write): on 409 the existing owner is read back — same tenant ⇒ normal re-registration, different tenant ⇒ `SessionTenantConflictException` ⇒ `409 Conflict` with `ErrorCode=session_owner_mismatch` (owning tenant not disclosed) plus a `SessionTenantConflict` ops event, and nothing is written. The claim is deliberately NOT fail-soft. Before this, the row was an unconditional last-writer-wins upsert after the session write, so any tenant's device (self-service onboarding) that knew a victim session GUID (portal deep links, Teams/Slack notifications, progress-portal URLs) could re-point the mapping at its own tenant and have every no-`tenantId` Global Admin/Reader read — and GA writes such as the `globaladmin` annotation lane — silently served from a forged session. Session ids are 122-bit random, so a genuine cross-tenant collision does not occur; a conflict is an attack or an agent bug and fails loudly. The legacy self-heal write is Add-only too (an existing claim is the authority), and `scripts/Migration/Backfill-SessionTenantLookup.ps1` closes the pre-table window once (idempotent, Add-only) so no legacy session id is claimable. The row is deleted by the cascade one step before the tombstone (no tombstone is kept — a freed id gives a foreign tenant nothing a fresh GUID would not) and is rebuildable from Sessions (not in the critical-backup set). The former lazy triggers (`events.Count == 0` → scan) are gone: empty-but-owned results no longer pay a table scan, and endpoints resolve upfront.
|
||||
|
||||
`requireGlobalAdmin: true` additionally corroborates the resolved tenant against its Sessions row (one point-read): a mapping whose session does not exist in the resolved tenant keeps `TargetTenantId`, so a stale or poisoned row can never steer a GA write — silently, without any operator confirmation step (a "confirm tenant GUID" dialog was rejected as unusable). Read paths keep the single point-read; their own session read is the corroboration (404).
|
||||
|
||||
Related enforcement added with the migration: `RecomputeTriggerGate` — `?reanalyze=true` and `?rescan=true` on the MemberRead session routes are actions (delete + rewrite stored results) and now 403 for Viewer / Global Reader / cross-tenant tenant roles (Global Admin or own-tenant Admin/Operator only).
|
||||
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
# Log
|
||||
|
||||
## 2026-08-30 (15)
|
||||
|
||||
* **Update**: `backend/lifecycle-manifests-and-session-scope.md` — SessionTenantLookup is now claimed first-writer-wins at registration (create-only Add BEFORE any session write; foreign owner ⇒ 409 `session_owner_mismatch` + `SessionTenantConflict` ops event, nothing written; not fail-soft). Closes the poisoning path where any tenant's device could re-point a victim sessionId's mapping at its own tenant and hijack every no-tenantId Global Admin/Reader read and GA write (annotation lane). `ResolveSessionScopeAsync(requireGlobalAdmin: true)` corroborates the resolved tenant against its Sessions row (silent — no operator confirmation). Self-heal write is Add-only; `scripts/Migration/Backfill-SessionTenantLookup.ps1` closes the legacy window once. No tombstone on deletion (deliberate: unbounded growth for no gain).
|
||||
|
||||
## 2026-08-30 (14)
|
||||
|
||||
* **Update**: `agent/decision-engine.md` — new invariant: every payload-derived string stored into `DecisionState` is bounded to 256 chars by `FactStringBounds` before any dedupe comparison (RealmJoin package id/version/scope, failed app ids, `HelloOutcome`, `ImeMatchedPatternId`, `EspAdvisoryFailureCategory`, RJ product version/channel). Backend `ReducerVerifier` now replays decode-fold-discard instead of materialising all decoded signals, and `ISignalRepository.QueryBySessionAsync` carries a cumulative `PayloadJson` budget (`SignalQueryLimits`, 32 M chars) reflected in the `truncated` flag of `sessions/{id}/signals` and `sessions/{id}/reducer-verification`.
|
||||
|
||||
@@ -8,6 +8,7 @@ using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared;
|
||||
using AutopilotMonitor.Shared.DataAccess;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Moq;
|
||||
using Xunit;
|
||||
@@ -96,10 +97,12 @@ public class SessionScopeResolutionTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RequireGlobalAdmin_StillResolves_ForGlobalAdmin()
|
||||
public async Task RequireGlobalAdmin_StillResolves_ForGlobalAdmin_WhenTheSessionRowCorroboratesTheLookup()
|
||||
{
|
||||
var repo = new Mock<ISessionRepository>();
|
||||
repo.Setup(r => r.ResolveSessionTenantIdAsync(SessionId)).ReturnsAsync(OtherTenant);
|
||||
repo.Setup(r => r.GetSessionAsync(OtherTenant, SessionId))
|
||||
.ReturnsAsync(new SessionSummary { SessionId = SessionId, TenantId = OtherTenant });
|
||||
var ctx = new RequestContext { TenantId = HomeTenant, TargetTenantId = HomeTenant, IsGlobalAdmin = true };
|
||||
|
||||
var result = await ctx.ResolveSessionScopeAsync(repo.Object, SessionId, requireGlobalAdmin: true);
|
||||
@@ -107,6 +110,38 @@ public class SessionScopeResolutionTests
|
||||
Assert.Equal(OtherTenant, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RequireGlobalAdmin_KeepsTargetTenant_WhenTheLookupPointsAtATenantWithoutTheSessionRow()
|
||||
{
|
||||
// A lookup row whose tenant does not hold the Sessions row (poisoned or stale mapping)
|
||||
// must never steer a GA WRITE into that tenant — silently keep the target tenant so the
|
||||
// endpoint's own not-found handling answers; no operator confirmation dialog.
|
||||
var repo = new Mock<ISessionRepository>();
|
||||
repo.Setup(r => r.ResolveSessionTenantIdAsync(SessionId)).ReturnsAsync(OtherTenant);
|
||||
repo.Setup(r => r.GetSessionAsync(OtherTenant, SessionId)).ReturnsAsync((SessionSummary?)null);
|
||||
var ctx = new RequestContext { TenantId = HomeTenant, TargetTenantId = HomeTenant, IsGlobalAdmin = true };
|
||||
|
||||
var result = await ctx.ResolveSessionScopeAsync(repo.Object, SessionId, requireGlobalAdmin: true);
|
||||
|
||||
Assert.Equal(HomeTenant, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ReadPath_DoesNotCorroborate_TheLookupWithASessionRead()
|
||||
{
|
||||
// Reads keep the single point-read contract: the endpoint's own session read is the
|
||||
// corroboration (404 when the row is missing in the resolved tenant).
|
||||
var repo = new Mock<ISessionRepository>(MockBehavior.Strict);
|
||||
repo.Setup(r => r.ResolveSessionTenantIdAsync(SessionId)).ReturnsAsync(OtherTenant);
|
||||
var ctx = new RequestContext { TenantId = HomeTenant, TargetTenantId = HomeTenant, IsGlobalAdmin = true };
|
||||
|
||||
var result = await ctx.ResolveSessionScopeAsync(repo.Object, SessionId);
|
||||
|
||||
Assert.Equal(OtherTenant, result);
|
||||
repo.VerifyAll();
|
||||
repo.VerifyNoOtherCalls();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DelegatedCaller_GetsNoFallback_ByDesign()
|
||||
{
|
||||
@@ -151,14 +186,31 @@ public class SessionScopeResolutionTests
|
||||
var result = await harness.Sut.ResolveSessionTenantIdAsync(SessionId);
|
||||
|
||||
Assert.Equal(OtherTenant, result);
|
||||
// Self-heal: the lookup row is written so the next resolve is a point-read.
|
||||
harness.Lookup.Verify(t => t.UpsertEntityAsync(
|
||||
// Self-heal: the lookup row is written ADD-ONLY so the next resolve is a point-read and
|
||||
// a claim that landed meanwhile is never overwritten.
|
||||
harness.Lookup.Verify(t => t.AddEntityAsync(
|
||||
It.Is<TableEntity>(e => e.PartitionKey == SessionId
|
||||
&& e.RowKey == "tenant"
|
||||
&& e.GetString("TenantId") == OtherTenant),
|
||||
It.IsAny<TableUpdateMode>(),
|
||||
It.IsAny<CancellationToken>()),
|
||||
Times.Once);
|
||||
harness.Lookup.Verify(t => t.UpsertEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()),
|
||||
Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SelfHeal_SwallowsAConcurrentClaim_AndStillReturnsTheScannedTenant()
|
||||
{
|
||||
var indexRow = new TableEntity(OtherTenant, "0000000000000000001_" + SessionId) { ["SessionId"] = SessionId };
|
||||
var harness = new ResolveHarness(lookupRow: null, indexRows: new[] { indexRow });
|
||||
harness.Lookup
|
||||
.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new RequestFailedException(409, "EntityAlreadyExists"));
|
||||
|
||||
var result = await harness.Sut.ResolveSessionTenantIdAsync(SessionId);
|
||||
|
||||
Assert.Equal(OtherTenant, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -169,8 +221,8 @@ public class SessionScopeResolutionTests
|
||||
var result = await harness.Sut.ResolveSessionTenantIdAsync(SessionId);
|
||||
|
||||
Assert.Null(result);
|
||||
harness.Lookup.Verify(t => t.UpsertEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()),
|
||||
harness.Lookup.Verify(t => t.AddEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()),
|
||||
Times.Never);
|
||||
}
|
||||
|
||||
@@ -246,7 +298,7 @@ public class SessionScopeResolutionTests
|
||||
.ThrowsAsync(new RequestFailedException(404, "Not Found"));
|
||||
}
|
||||
Lookup
|
||||
.Setup(t => t.UpsertEntityAsync(It.IsAny<TableEntity>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()))
|
||||
.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(Mock.Of<Response>());
|
||||
|
||||
var page = Page<TableEntity>.FromValues(indexRows ?? Array.Empty<TableEntity>(), null, Mock.Of<Response>());
|
||||
|
||||
+6
@@ -386,6 +386,7 @@ public class StoreSessionReregistrationPreserveTests
|
||||
private sealed class Harness
|
||||
{
|
||||
public Mock<TableClient> Sessions { get; }
|
||||
public Mock<TableClient> Lookup { get; } = new();
|
||||
public TableStorageService Sut { get; }
|
||||
public TableEntity? Written { get; private set; }
|
||||
|
||||
@@ -423,8 +424,13 @@ public class StoreSessionReregistrationPreserveTests
|
||||
return Task.FromResult(new Mock<Response>().Object);
|
||||
});
|
||||
|
||||
// Ownership claim (first-writer-wins lookup) — Add succeeds: this tenant owns the id.
|
||||
Lookup.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new Mock<Response>().Object);
|
||||
|
||||
var mockServiceClient = new Mock<TableServiceClient>();
|
||||
mockServiceClient.Setup(s => s.GetTableClient(Constants.TableNames.Sessions)).Returns(Sessions.Object);
|
||||
mockServiceClient.Setup(s => s.GetTableClient(Constants.TableNames.SessionTenantLookup)).Returns(Lookup.Object);
|
||||
// Events / SessionsIndex intentionally unconfigured → null client → SUT swallows.
|
||||
|
||||
Sut = new TableStorageService(mockServiceClient.Object, NullLogger<TableStorageService>.Instance);
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Azure;
|
||||
using Azure.Data.Tables;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Moq;
|
||||
using Xunit;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// SessionTenantLookup ownership claim in <see cref="TableStorageService.StoreSessionAsync"/>.
|
||||
/// The lookup (PK = client-supplied sessionId → owning tenantId) is the authority behind every
|
||||
/// global-scope cross-tenant session resolve. It used to be an unconditional last-writer-wins
|
||||
/// upsert AFTER the Sessions row, so any tenant's device could re-point a victim sessionId at
|
||||
/// its own tenant. Now it is a create-only claim BEFORE any session write: a foreign owner
|
||||
/// refuses the registration (409, nothing written), the same owner re-registers as before.
|
||||
/// </summary>
|
||||
public class StoreSessionTenantLookupClaimTests
|
||||
{
|
||||
private const string TenantA = "11111111-1111-1111-1111-111111111111";
|
||||
private const string TenantB = "44444444-4444-4444-4444-444444444444";
|
||||
private const string SessionId = "22222222-2222-2222-2222-222222222222";
|
||||
|
||||
private static SessionRegistration Registration(string tenantId) => new()
|
||||
{
|
||||
TenantId = tenantId,
|
||||
SessionId = SessionId,
|
||||
StartedAt = new DateTime(2026, 1, 1, 8, 0, 0, DateTimeKind.Utc),
|
||||
SerialNumber = "SN-1",
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public async Task FreshSessionId_ClaimsTheLookup_BeforeWritingTheSessionRow()
|
||||
{
|
||||
var harness = new Harness(existingOwner: null);
|
||||
|
||||
var ok = await harness.Sut.StoreSessionAsync(Registration(TenantA));
|
||||
|
||||
Assert.True(ok);
|
||||
Assert.Equal(new[] { "lookup-add", "session-write" }, harness.Order);
|
||||
Assert.Equal(TenantA, harness.ClaimedTenant);
|
||||
harness.Lookup.Verify(t => t.UpsertEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SameTenantReregistration_KeepsTheExistingClaim_AndStoresNormally()
|
||||
{
|
||||
var harness = new Harness(existingOwner: TenantA.ToUpperInvariant());
|
||||
|
||||
var ok = await harness.Sut.StoreSessionAsync(Registration(TenantA));
|
||||
|
||||
Assert.True(ok);
|
||||
Assert.Contains("session-write", harness.Order);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ForeignTenant_IsRefused_AndNothingIsWritten()
|
||||
{
|
||||
// Attacker in tenant B registers the victim's (tenant A) sessionId.
|
||||
var harness = new Harness(existingOwner: TenantA);
|
||||
|
||||
var ex = await Assert.ThrowsAsync<SessionTenantConflictException>(
|
||||
() => harness.Sut.StoreSessionAsync(Registration(TenantB)));
|
||||
|
||||
Assert.Equal(SessionId, ex.SessionId);
|
||||
Assert.Equal(TenantB, ex.RequestedTenantId);
|
||||
Assert.Equal(TenantA, ex.OwningTenantId);
|
||||
// The mapping still points at A, and B never got a Sessions row for the id.
|
||||
Assert.DoesNotContain("session-write", harness.Order);
|
||||
harness.Lookup.Verify(t => t.UpsertEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
harness.Lookup.Verify(t => t.UpdateEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<ETag>(), It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ClaimStorageFailure_IsNotFailSoft_TheSessionIsNotStored()
|
||||
{
|
||||
var harness = new Harness(existingOwner: null);
|
||||
harness.Lookup.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new RequestFailedException(503, "ServerBusy"));
|
||||
|
||||
var ok = await harness.Sut.StoreSessionAsync(Registration(TenantA));
|
||||
|
||||
Assert.False(ok);
|
||||
Assert.DoesNotContain("session-write", harness.Order);
|
||||
}
|
||||
|
||||
private sealed class Harness
|
||||
{
|
||||
public Mock<TableClient> Sessions { get; } = new();
|
||||
public Mock<TableClient> Lookup { get; } = new();
|
||||
public List<string> Order { get; } = new();
|
||||
public string? ClaimedTenant { get; private set; }
|
||||
public TableStorageService Sut { get; }
|
||||
|
||||
public Harness(string? existingOwner)
|
||||
{
|
||||
// Sessions: no existing row; Add captures the write order.
|
||||
Sessions.Setup(t => t.GetEntityAsync<TableEntity>(
|
||||
It.IsAny<string>(), SessionId, It.IsAny<IEnumerable<string>>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new RequestFailedException(404, "ResourceNotFound"));
|
||||
Sessions.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.Returns<TableEntity, CancellationToken>((_, _) =>
|
||||
{
|
||||
Order.Add("session-write");
|
||||
return Task.FromResult(new Mock<Response>().Object);
|
||||
});
|
||||
|
||||
if (existingOwner is null)
|
||||
{
|
||||
Lookup.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.Returns<TableEntity, CancellationToken>((e, _) =>
|
||||
{
|
||||
Order.Add("lookup-add");
|
||||
ClaimedTenant = e.GetString("TenantId");
|
||||
return Task.FromResult(new Mock<Response>().Object);
|
||||
});
|
||||
}
|
||||
else
|
||||
{
|
||||
Lookup.Setup(t => t.AddEntityAsync(It.IsAny<TableEntity>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new RequestFailedException(409, "EntityAlreadyExists"));
|
||||
Lookup.Setup(t => t.GetEntityAsync<TableEntity>(
|
||||
SessionId, "tenant", It.IsAny<IEnumerable<string>>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(Response.FromValue(
|
||||
new TableEntity(SessionId, "tenant") { ["TenantId"] = existingOwner }, Mock.Of<Response>()));
|
||||
}
|
||||
|
||||
var serviceClient = new Mock<TableServiceClient>();
|
||||
serviceClient.Setup(s => s.GetTableClient(Constants.TableNames.Sessions)).Returns(Sessions.Object);
|
||||
serviceClient.Setup(s => s.GetTableClient(Constants.TableNames.SessionTenantLookup)).Returns(Lookup.Object);
|
||||
// Events / SessionsIndex intentionally unconfigured → null client → SUT swallows.
|
||||
Sut = new TableStorageService(serviceClient.Object, NullLogger<TableStorageService>.Instance);
|
||||
}
|
||||
}
|
||||
}
|
||||
+43
-3
@@ -38,6 +38,7 @@ namespace AutopilotMonitor.Functions.Functions.Sessions
|
||||
private readonly SessionDeletionGuard _deletionGuard;
|
||||
private readonly AdminConfigurationService _adminConfigService;
|
||||
private readonly SessionOwnerBindingObserver _ownerBinding;
|
||||
private readonly OpsEventService _opsEvents;
|
||||
|
||||
public RegisterSessionFunction(
|
||||
ILogger<RegisterSessionFunction> logger,
|
||||
@@ -54,7 +55,8 @@ namespace AutopilotMonitor.Functions.Functions.Sessions
|
||||
BootstrapSessionService bootstrapSessionService,
|
||||
WebhookNotificationService webhookNotificationService,
|
||||
SessionDeletionGuard deletionGuard,
|
||||
SessionOwnerBindingObserver ownerBinding)
|
||||
SessionOwnerBindingObserver ownerBinding,
|
||||
OpsEventService opsEvents)
|
||||
{
|
||||
_logger = logger;
|
||||
_sessionRepo = sessionRepo;
|
||||
@@ -71,6 +73,7 @@ namespace AutopilotMonitor.Functions.Functions.Sessions
|
||||
_webhookNotificationService = webhookNotificationService;
|
||||
_deletionGuard = deletionGuard;
|
||||
_ownerBinding = ownerBinding;
|
||||
_opsEvents = opsEvents;
|
||||
}
|
||||
|
||||
[Function("RegisterSession")]
|
||||
@@ -194,8 +197,26 @@ namespace AutopilotMonitor.Functions.Functions.Sessions
|
||||
var ownerDecision = _ownerBinding.Observe(
|
||||
req, registration.TenantId, registration.SessionId, guardSessionRow, validation, "agent/register-session");
|
||||
|
||||
// Store session in Azure Table Storage
|
||||
var stored = await _sessionRepo.StoreSessionAsync(registration, ownerDecision.OwnerToStamp);
|
||||
// Store session in Azure Table Storage. The sessionId → tenantId lookup claim inside is
|
||||
// first-writer-wins: a sessionId already owned by another tenant is refused with 409 so
|
||||
// no tenant can poison the cross-tenant resolver (SessionTenantLookup) that Global
|
||||
// Admin/Reader session reads and GA writes trust.
|
||||
bool stored;
|
||||
try
|
||||
{
|
||||
stored = await _sessionRepo.StoreSessionAsync(registration, ownerDecision.OwnerToStamp);
|
||||
}
|
||||
catch (SessionTenantConflictException conflict)
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"RegisterSession refused: session {SessionId} is owned by another tenant — requested tenant={TenantId} thumbprint={Thumbprint}",
|
||||
registration.SessionId, registration.TenantId, validation.CertificateThumbprint);
|
||||
_ = _opsEvents.RecordSessionTenantConflictAsync(
|
||||
registration.TenantId, registration.SessionId, conflict.OwningTenantId,
|
||||
validation.CertificateThumbprint, registration.AgentVersion, "agent/register-session")
|
||||
.ContinueWith(t => _logger.LogWarning(t.Exception?.InnerException, "Fire-and-forget RecordSessionTenantConflictAsync failed"), TaskContinuationOptions.OnlyOnFaulted);
|
||||
return new RegisterSessionOutput { HttpResponse = await WriteSessionTenantConflictAsync(req) };
|
||||
}
|
||||
|
||||
if (!stored)
|
||||
{
|
||||
@@ -440,6 +461,25 @@ namespace AutopilotMonitor.Functions.Functions.Sessions
|
||||
registration.StartedAt = sanitized;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 409 for a sessionId already owned by another tenant. Carries
|
||||
/// <see cref="Constants.AgentErrorCodes.SessionOwnerMismatch"/> — the agent's correct reaction
|
||||
/// is the same as for an owner-binding refusal: rotate the SessionId and register afresh.
|
||||
/// The owning tenant is deliberately NOT disclosed to the caller.
|
||||
/// </summary>
|
||||
private static async Task<HttpResponseData> WriteSessionTenantConflictAsync(HttpRequestData req)
|
||||
{
|
||||
var response = req.CreateResponse(HttpStatusCode.Conflict);
|
||||
await response.WriteAsJsonAsync(new RegisterSessionResponse
|
||||
{
|
||||
Success = false,
|
||||
Message = "Session id is already registered to another tenant; register with a new session id.",
|
||||
RegisteredAt = DateTime.UtcNow,
|
||||
ErrorCode = Constants.AgentErrorCodes.SessionOwnerMismatch,
|
||||
});
|
||||
return response;
|
||||
}
|
||||
|
||||
private static async Task<HttpResponseData> WriteSessionLockedAsync(HttpRequestData req, SessionDeletionLockedException locked)
|
||||
{
|
||||
var response = req.CreateResponse(HttpStatusCode.Gone);
|
||||
|
||||
@@ -176,10 +176,18 @@ public static class RequestContextExtensions
|
||||
return ctx.TargetTenantId;
|
||||
|
||||
var resolvedTenantId = await sessionRepo.ResolveSessionTenantIdAsync(sessionId);
|
||||
if (resolvedTenantId != null && !string.Equals(resolvedTenantId, ctx.TargetTenantId, StringComparison.OrdinalIgnoreCase))
|
||||
return resolvedTenantId;
|
||||
if (resolvedTenantId == null || string.Equals(resolvedTenantId, ctx.TargetTenantId, StringComparison.OrdinalIgnoreCase))
|
||||
return ctx.TargetTenantId;
|
||||
|
||||
return ctx.TargetTenantId;
|
||||
// WRITE paths corroborate the lookup against the Sessions row of the resolved tenant:
|
||||
// the lookup is first-writer-wins at registration (StoreSessionAsync), but a mapping
|
||||
// whose session does not exist must never steer a Global Admin write into that tenant.
|
||||
// Silent for the operator — a miss just keeps the target tenant, so the endpoint's own
|
||||
// not-found handling answers.
|
||||
if (requireGlobalAdmin && await sessionRepo.GetSessionAsync(resolvedTenantId, sessionId) == null)
|
||||
return ctx.TargetTenantId;
|
||||
|
||||
return resolvedTenantId;
|
||||
}
|
||||
|
||||
/// <summary>Gets the correlation ID for this request (set by CorrelationIdMiddleware).</summary>
|
||||
|
||||
@@ -298,6 +298,20 @@ namespace AutopilotMonitor.Functions.Services
|
||||
/// re-enroll-without-wipe shape (same device, new certificate identity, old session id);
|
||||
/// <c>serialMatch=false</c> is a foreign device naming somebody else's session.
|
||||
/// </summary>
|
||||
/// <summary>
|
||||
/// A device/bootstrap-authenticated registration named a sessionId whose SessionTenantLookup
|
||||
/// row belongs to ANOTHER tenant (first-writer-wins claim in <c>StoreSessionAsync</c>).
|
||||
/// Session ids are random GUIDs — this is a forged registration or an agent bug, never a
|
||||
/// legitimate collision. The registration was refused with 409. Recorded against the
|
||||
/// requesting tenant; the owning tenant is only in the details.
|
||||
/// </summary>
|
||||
public Task RecordSessionTenantConflictAsync(
|
||||
string requestedTenantId, string sessionId, string owningTenantId, string? certificateThumbprint, string? agentVersion, string endpoint)
|
||||
=> WriteAsync(OpsEventCategory.Security, "SessionTenantConflict", OpsEventSeverity.Warning,
|
||||
$"Registration refused on {endpoint}: session {sessionId} is owned by another tenant (409)",
|
||||
requestedTenantId, "System.SessionTenantLookup",
|
||||
new { sessionId, owningTenantId, certificateThumbprint, agentVersion, endpoint });
|
||||
|
||||
public Task RecordSessionOwnerMismatchAsync(
|
||||
string tenantId, string sessionId, string outcome, string callerKind, string ownerKind,
|
||||
bool serialMatch, string? agentVersion, string endpoint)
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
using System;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Services
|
||||
{
|
||||
/// <summary>
|
||||
/// Thrown by <c>TableStorageService.StoreSessionAsync</c> when a registration names a sessionId
|
||||
/// whose SessionTenantLookup row is already owned by a DIFFERENT tenant. The lookup is the
|
||||
/// authority every global-scope cross-tenant session resolve trusts, so its write is
|
||||
/// first-writer-wins: a tenant can never overwrite (poison) another tenant's mapping.
|
||||
/// Session ids are 122-bit random — a genuine cross-tenant collision does not happen, so a
|
||||
/// conflict is either a forged registration or an agent bug, and both must fail loudly
|
||||
/// (409 Conflict) instead of silently retargeting the platform-operator data plane.
|
||||
/// </summary>
|
||||
public class SessionTenantConflictException : InvalidOperationException
|
||||
{
|
||||
public string SessionId { get; }
|
||||
public string RequestedTenantId { get; }
|
||||
public string OwningTenantId { get; }
|
||||
|
||||
public SessionTenantConflictException(string sessionId, string requestedTenantId, string owningTenantId)
|
||||
: base($"Session {sessionId} is already registered to another tenant; registration for tenant {requestedTenantId} refused")
|
||||
{
|
||||
SessionId = sessionId;
|
||||
RequestedTenantId = requestedTenantId;
|
||||
OwningTenantId = owningTenantId;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -459,6 +459,14 @@ namespace AutopilotMonitor.Functions.Services
|
||||
|
||||
try
|
||||
{
|
||||
// Ownership claim FIRST, before any Sessions/SessionsIndex write: the sessionId →
|
||||
// tenantId lookup is first-writer-wins. A registration whose sessionId is already
|
||||
// mapped to another tenant is refused here (SessionTenantConflictException → 409),
|
||||
// so no tenant can poison the cross-tenant resolver and no foreign-owned sessionId
|
||||
// ever gets a row in the caller's tenant. Not fail-soft: without the claim the
|
||||
// session must not exist.
|
||||
await ClaimSessionTenantLookupAsync(registration.SessionId, registration.TenantId);
|
||||
|
||||
var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.Sessions);
|
||||
|
||||
// If the agent restarts with the same session ID, preserve timeline/progress fields
|
||||
@@ -818,13 +826,14 @@ namespace AutopilotMonitor.Functions.Services
|
||||
// Dual-write: upsert into SessionsIndex for time-sorted listing
|
||||
await UpsertSessionIndexAsync(entity, startedAt);
|
||||
|
||||
// Dual-write: sessionId → tenantId point-read lookup (fail-soft; legacy
|
||||
// sessions heal lazily in ResolveSessionTenantIdAsync).
|
||||
await UpsertSessionTenantLookupAsync(registration.SessionId, registration.TenantId);
|
||||
|
||||
_logger.LogInformation($"Stored session {registration.SessionId}");
|
||||
return true;
|
||||
}
|
||||
catch (SessionTenantConflictException)
|
||||
{
|
||||
// Caller maps this to 409 — never swallow it into a generic "store failed".
|
||||
throw;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogError(ex, $"Failed to store session {registration.SessionId}");
|
||||
@@ -1634,8 +1643,9 @@ namespace AutopilotMonitor.Functions.Services
|
||||
var scanned = await ScanSessionsIndexForTenantIdAsync(sessionId);
|
||||
if (scanned != null)
|
||||
{
|
||||
// Self-heal: next resolve for this legacy session is a point-read.
|
||||
await UpsertSessionTenantLookupAsync(sessionId, scanned);
|
||||
// Self-heal: next resolve for this legacy session is a point-read. Add-only —
|
||||
// a row that landed meanwhile (a registration claim) is the authority, never ours.
|
||||
await TryAddSessionTenantLookupAsync(sessionId, scanned);
|
||||
}
|
||||
return scanned;
|
||||
}
|
||||
@@ -1664,24 +1674,75 @@ namespace AutopilotMonitor.Functions.Services
|
||||
}
|
||||
}
|
||||
|
||||
private static TableEntity NewSessionTenantLookupRow(string sessionId, string tenantId)
|
||||
=> new TableEntity(sessionId, SessionTenantLookupRowKey) { ["TenantId"] = tenantId };
|
||||
|
||||
/// <summary>
|
||||
/// Writes the sessionId → tenantId lookup row (fail-soft: resolution still works via
|
||||
/// the scan fallback when this write is lost).
|
||||
/// Claims the sessionId → tenantId lookup row for <paramref name="tenantId"/> (first-writer-wins).
|
||||
/// Create-only Add; on 409 the existing owner is read and must equal the caller's tenant
|
||||
/// (normal re-registration), otherwise <see cref="SessionTenantConflictException"/>.
|
||||
/// NOT fail-soft: a lost claim means a session without a trustworthy owner mapping, so
|
||||
/// storage errors propagate and the registration fails as a whole.
|
||||
/// </summary>
|
||||
private async Task UpsertSessionTenantLookupAsync(string sessionId, string tenantId)
|
||||
private async Task ClaimSessionTenantLookupAsync(string sessionId, string tenantId)
|
||||
{
|
||||
var lookupClient = _tableServiceClient.GetTableClient(Constants.TableNames.SessionTenantLookup);
|
||||
|
||||
// Two attempts cover the only benign race: the row is deleted (retention cascade)
|
||||
// between our 409 and the read-back — the second Add then simply succeeds.
|
||||
for (var attempt = 0; attempt < 2; attempt++)
|
||||
{
|
||||
try
|
||||
{
|
||||
await lookupClient.AddEntityAsync(NewSessionTenantLookupRow(sessionId, tenantId));
|
||||
return;
|
||||
}
|
||||
catch (RequestFailedException ex) when (ex.Status == 409)
|
||||
{
|
||||
// Existing claim — re-registration by the owner, or a foreign tenant.
|
||||
}
|
||||
|
||||
string? owner;
|
||||
try
|
||||
{
|
||||
var existing = await lookupClient.GetEntityAsync<TableEntity>(sessionId, SessionTenantLookupRowKey);
|
||||
owner = existing.Value.GetString("TenantId");
|
||||
}
|
||||
catch (RequestFailedException ex) when (ex.Status == 404)
|
||||
{
|
||||
continue; // deleted in between — retry the Add
|
||||
}
|
||||
|
||||
if (string.Equals(owner, tenantId, StringComparison.OrdinalIgnoreCase))
|
||||
return;
|
||||
|
||||
_logger.LogWarning(
|
||||
"SessionTenantLookup claim refused: session {SessionId} is owned by tenant {OwningTenantId}, registration from tenant {RequestedTenantId} rejected",
|
||||
sessionId, owner, tenantId);
|
||||
throw new SessionTenantConflictException(sessionId, tenantId, owner ?? string.Empty);
|
||||
}
|
||||
|
||||
throw new InvalidOperationException($"SessionTenantLookup claim for session {sessionId} did not converge");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Self-heal write for legacy sessions (pre-lookup-table): Add-only, so it can never
|
||||
/// overwrite an existing claim; fail-soft because resolution still works via the scan.
|
||||
/// </summary>
|
||||
private async Task TryAddSessionTenantLookupAsync(string sessionId, string tenantId)
|
||||
{
|
||||
try
|
||||
{
|
||||
var lookupClient = _tableServiceClient.GetTableClient(Constants.TableNames.SessionTenantLookup);
|
||||
var row = new TableEntity(sessionId, SessionTenantLookupRowKey)
|
||||
{
|
||||
["TenantId"] = tenantId
|
||||
};
|
||||
await lookupClient.UpsertEntityAsync(row);
|
||||
await lookupClient.AddEntityAsync(NewSessionTenantLookupRow(sessionId, tenantId));
|
||||
}
|
||||
catch (RequestFailedException ex) when (ex.Status == 409)
|
||||
{
|
||||
// Already claimed — the existing row is the authority.
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogWarning(ex, "Failed to upsert SessionTenantLookup for {SessionId}", sessionId);
|
||||
_logger.LogWarning(ex, "Failed to add SessionTenantLookup for {SessionId}", sessionId);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -259,7 +259,9 @@ namespace AutopilotMonitor.Shared
|
||||
/// SESSION-OWNER-BINDING: the session named by the request is bound to a different
|
||||
/// device identity than the caller presented. The agent's correct reaction is to
|
||||
/// rotate its SessionId and register afresh — NOT to count it as an auth failure.
|
||||
/// Reserved in stage 1 (shadow); emitted only once enforcement is switched on.
|
||||
/// Owner-binding: reserved in stage 1 (shadow), emitted once enforcement is on.
|
||||
/// Already emitted by RegisterSession (409) when the session id is claimed by
|
||||
/// another tenant (SessionTenantLookup first-writer-wins).
|
||||
/// </summary>
|
||||
public const string SessionOwnerMismatch = "session_owner_mismatch";
|
||||
}
|
||||
|
||||
@@ -103,6 +103,10 @@ const OPS_EVENT_TYPES: Record<string, string[]> = {
|
||||
// different device identity (SessionOwnerBindingObserver, throttled 1h per session+outcome).
|
||||
// Stage 1 observes only. Dual-register per memory feedback_ops_event_types_dual_register.
|
||||
"SessionOwnerMismatch",
|
||||
// A registration named a sessionId already owned by ANOTHER tenant (first-writer-wins
|
||||
// SessionTenantLookup claim) and was refused with 409. Dual-register per memory
|
||||
// feedback_ops_event_types_dual_register.
|
||||
"SessionTenantConflict",
|
||||
"EmbeddedCertExpiringSoon",
|
||||
"EmbeddedCertExpiringUrgent",
|
||||
"EmbeddedCertExpired",
|
||||
|
||||
Reference in New Issue
Block a user