fix(matching): validate source snapshot identity scope

This commit is contained in:
joshpatra committed 2026-09-27 18:33:47 -04:00
1 parent 6fdd20bb9b
commit 6008ec949a
3 files changed
+154 -4

No files matched your search

@@ -1,5 +1,6 @@
using System.Security.Cryptography;
using System.Text;
using allstarr.Core.Capabilities;
using allstarr.Core.Identity;
using allstarr.Core.Matching;
using allstarr.Core.Operations;
@@ -121,6 +122,121 @@ public sealed class PlaylistPersistenceServiceTests : IAsyncLifetime
Assert.Equal(_localTrack, Assert.Single(sourceAware).Id);
}
[Theory]
[InlineData("tenant")]
[InlineData("provider")]
[InlineData("account")]
[InlineData("hash")]
[InlineData("catalog")]
[InlineData("missing")]
[InlineData("snapshot-kind")]
public async Task SnapshotIdentity_MustMatchTheExactSourceScope(string mismatch)
{
var identity = await SeedSourceIdentityAsync(mismatch);
var input = Snapshot(1, "track-safe") with
{
ProviderTrackIdentityId = mismatch == "missing" ? Guid.CreateVersion7() : identity.Id,
ResourceKind = mismatch == "snapshot-kind" ? "album" : "track"
};
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
_matches.CaptureSnapshotAsync(Context(_userA, "principal-a"), input));
await using var db = await _factory.CreateDbContextAsync();
Assert.Empty(await db.ExternalMetadataSnapshots.ToListAsync());
var existing = await _matches.CaptureSnapshotAsync(
Context(_userA, "principal-a"), input with { ProviderTrackIdentityId = null });
await Assert.ThrowsAsync<UnauthorizedAccessException>(() =>
_matches.CaptureSnapshotAsync(Context(_userA, "principal-a"), input));
Assert.Equal(existing.Id, (await db.ExternalMetadataSnapshots.SingleAsync()).Id);
}
[Theory]
[InlineData("valid")]
[InlineData("catalog-scope")]
public async Task SnapshotIdentity_AllowsAuthorizedAccountAndCatalogLinks(string scope)
{
var identity = await SeedSourceIdentityAsync(scope);
var input = Snapshot(1, "track-safe") with { ProviderTrackIdentityId = identity.Id };
var context = Context(_userA, "principal-a");
var first = await _matches.CaptureSnapshotAsync(context, input);
var repeated = await _matches.CaptureSnapshotAsync(context, input with { ResourceKind = " Track " });
Assert.Equal(identity.Id, first.ProviderTrackIdentityId);
Assert.Equal(first.Id, repeated.Id);
await Assert.ThrowsAsync<InvalidOperationException>(() => _matches.CaptureSnapshotAsync(
context, input with { ProviderTrackIdentityId = null }));
await Assert.ThrowsAsync<InvalidOperationException>(() => _matches.CaptureSnapshotAsync(
Context(_userA, "different-principal"), input));
}
private async Task<ProviderTrackIdentityRecord> SeedSourceIdentityAsync(string variant)
{
await using var db = await _factory.CreateDbContextAsync();
var tenant = _tenant;
var owner = _userA;
if (variant == "tenant")
{
tenant = Guid.CreateVersion7();
owner = Guid.CreateVersion7();
db.Tenants.Add(new TenantRecord { Id = tenant, Slug = "foreign", Name = "Foreign", CreatedAt = _now });
var user = User(owner, "Foreign");
user.TenantId = tenant;
db.Users.Add(user);
}
var account = _accountA;
if (variant is "account" or "tenant")
{
account = Guid.CreateVersion7();
db.ProviderAccounts.Add(new ProviderAccountRecord
{
Id = account,
TenantId = tenant,
OwnerUserId = variant == "account" ? _userB : owner,
ProviderId = "fixture",
DisplayName = "Other account",
Scope = ProviderAccountScope.User,
Enabled = true,
CreatedAt = _now,
UpdatedAt = _now
});
}
var recording = new CanonicalRecordingRecord
{
Id = Guid.CreateVersion7(),
TenantId = tenant,
CreatedByUserId = owner,
IsProvisional = true,
CreatedAt = _now,
UpdatedAt = _now
};
db.CanonicalRecordings.Add(recording);
var identity = new ProviderTrackIdentityRecord
{
Id = Guid.CreateVersion7(),
TenantId = tenant,
CanonicalRecordingId = recording.Id,
ProviderId = variant == "provider" ? "other" : "fixture",
ProviderAccountId = variant is "catalog-scope" or "provider" ? null : account,
Scope = variant is "catalog-scope" or "provider" ? ProviderIdentityScope.Catalog : ProviderIdentityScope.Account,
ResourceKind = ProviderResourceKind.Track,
CatalogNamespace = variant == "catalog" ? "other" : "default",
ExternalId = variant == "hash" ? "different-track" : "track-safe",
ExternalIdHash = Hash(variant == "hash" ? "different-track" : "track-safe"),
Verification = ProviderIdentityVerification.Verified,
VerificationMethod = "source-snapshot",
DecisionVersion = 1,
VerifiedAt = _now,
CreatedAt = _now,
UpdatedAt = _now
};
db.ProviderTrackIdentities.Add(identity);
await db.SaveChangesAsync();
return identity;
}
[Fact]
public async Task OwnerTenantConcurrencyAndPayloadGuards_DenyUnsafeAccess()
{
@@ -309,10 +309,24 @@ public sealed class TrackMatchCommandService(
cancellationToken) ?? throw new UnauthorizedAccessException("The provider account is unavailable.");
await using var db = await contextFactory.CreateDbContextAsync(cancellationToken);
var resourceKind = input.ResourceKind.Trim().ToLowerInvariant();
if (input.ProviderTrackIdentityId.HasValue &&
(resourceKind != "track" || !await db.ProviderTrackIdentities.AnyAsync(item =>
item.Id == input.ProviderTrackIdentityId.Value &&
item.TenantId == actor.TenantId &&
item.ProviderId == account.Account.ProviderId &&
item.ResourceKind == ProviderResourceKind.Track &&
item.CatalogNamespace == "default" &&
item.ExternalIdHash == input.ExternalIdHash &&
(item.Scope == ProviderIdentityScope.Catalog && item.ProviderAccountId == null ||
item.Scope == ProviderIdentityScope.Account && item.ProviderAccountId == account.Account.Id),
cancellationToken)))
throw new UnauthorizedAccessException("The source identity is outside the snapshot scope.");
var existing = await db.ExternalMetadataSnapshots.AsNoTracking().SingleOrDefaultAsync(item =>
item.TenantId == actor.TenantId &&
item.ProviderAccountId == account.Account.Id &&
item.ResourceKind == input.ResourceKind &&
item.ResourceKind == resourceKind &&
item.ExternalIdHash == input.ExternalIdHash &&
item.SnapshotVersion == input.SnapshotVersion,
cancellationToken);
@@ -320,7 +334,11 @@ public sealed class TrackMatchCommandService(
{
if (!existing.PayloadSha256.Equals(input.PayloadSha256, StringComparison.Ordinal) ||
existing.OwnerUserId != actor.EffectiveUserId ||
existing.LibraryScopeId != input.LibraryScopeId)
existing.LibraryScopeId != input.LibraryScopeId ||
existing.ProviderTrackIdentityId != input.ProviderTrackIdentityId ||
existing.BackendInstanceId != context.BackendInstanceId ||
existing.BackendPrincipalId != context.VerifiedBackendPrincipalId ||
existing.Protocol != context.Protocol.ToString().ToLowerInvariant())
throw new InvalidOperationException(
"The snapshot version already exists with different immutable content or scope.");
return existing;
@@ -339,7 +357,7 @@ public sealed class TrackMatchCommandService(
BackendPrincipalId = context.VerifiedBackendPrincipalId,
Protocol = context.Protocol.ToString().ToLowerInvariant(),
ProviderId = account.Account.ProviderId,
ResourceKind = input.ResourceKind.Trim().ToLowerInvariant(),
ResourceKind = resourceKind,
ExternalIdHash = input.ExternalIdHash,
SnapshotVersion = input.SnapshotVersion,
ProviderRevision = input.ProviderRevision.Trim(),
@@ -331,7 +331,7 @@ Exit: representative Jellyfin, Navidrome/OpenSubsonic, and provider entities con
#### Stage 2 status
Checkpoint as of 2026-09-22; the alias projection changes are not yet deployed:
Checkpoint as of 2026-09-27; the catalog and alias projection changes are deployed on the testing server:
- **Storage:** The tenant-scoped recording stores provider-neutral title, version/disambiguation, duration, explicitness, and provisional status. The same graph owns artists, ordered credits, release groups, editions, release tracks, compatibility aliases, and append-only source facts. Composite tenant foreign keys block cross-tenant edges. A forward migration preserves existing IDs and routes.
- **Evidence:** `CanonicalCatalogEvidenceStore` is the only writer for aliases and source facts. It validates actor scope, normalizes source IDs and JSON, rejects remapping and hash collisions, treats repeated payloads as idempotent, and supersedes changed facts without erasing provenance. PostgreSQL qualification round-tripped the complete artist → release group → edition → release track → recording graph.
@@ -339,6 +339,8 @@ Checkpoint as of 2026-09-22; the alias projection changes are not yet deployed:
- **Atomic ingestion:** `MusicBrainzCatalogIngestService` validates the full payload before a serializable transaction, preserves separate editions, replaces provisional fields with canonical facts, and writes provenance through the shared evidence owner. Repeated input preserves IDs and creates no duplicate facts. A disposable PostgreSQL run passed 54 catalog, client, environment, migration-snapshot, and storage tests, including rollback of malformed media.
- **Discovery and refresh:** `MusicBrainzCatalogRefreshQueue` creates seven-day idempotency generations scoped by tenant, user, source, and revision. Recording discovery accepts at most 50 distinct editions. Release refresh accepts one release hierarchy and at most 64 credited artists before atomic ingestion. Both jobs preserve upstream retry delays, separate permanent hierarchy failures from transient failures, and stay outside search and playback requests. The focused lane passes 62/62 tests; a separate PostgreSQL run passes all 21 selected identity, discovery, and ingestion tests.
- **Identity projection:** Creating a recording now projects exact ISRC and MusicBrainz aliases through the shared evidence owner. Both the identity service and matching commands project accepted provider identities transactionally with account- or catalog-scoped namespaces, so the same provider ID cannot leak or collide across account boundaries. Indexed Jellyfin and Subsonic/OpenSubsonic items use a protocol-and-backend-instance namespace; two users seeing the same native item converge only when their canonical assignments agree. Conflicting historical native assignments remain unaliased rather than being silently merged. Concurrent match writers treat an alias insert race as a retryable identity write. Forward migrations backfill provider, signal, and consistent native aliases and mark recordings without an MBID provisional. The isolated PostgreSQL lanes pass all 22 unique selected identity, migration, native-index, manual-selection, automatic-fallback, concurrency, and model-snapshot tests.
- **Snapshot scope:** Snapshot capture validates a supplied provider identity against the tenant, source provider, track hash, catalog, and resolved account. Repeated captures must preserve the identity link and backend principal. PostgreSQL regressions cover foreign identities, both permitted identity scopes, and immutable retries.
- **Next reconciliation work:** Automatic rematching can move a source identity to an existing provider recording while leaving its catalog alias attached to the old recording. Fix this atomically before issuing stable canonical protocol IDs. Preserve historical decisions and manual authority; qualify two source tracks converging on one provider recording, concurrent rematches, and repeated source refresh. Generic evidence ingestion must continue to reject arbitrary alias reassignment.
- **Remaining:** Project remaining legacy source-snapshot and protocol identities into the catalog; add the relationship and image request shapes needed by Stage 3; and reconcile provisional records that begin without an MBID.
| Stage 2 checkpoint measure | Stage start | Current | Interpretation |
@@ -420,6 +422,20 @@ Exit: desktop and mobile browser suites cover the full real-API journeys with ke
Exit: every primary journey has a reproducible automated regression and a recorded live qualification, and the release commit meets the code-scanning gate. Missing credentials or skipped providers are reported as unqualified, not passed.
Live checkpoint on 2026-09-27: `6fdd20bb` passed 173 smoke checks against
Jellyfin 12.1.0 with zero failures using a listener test account. The contract
fixtures remain pinned to 12.0.0; this run does not qualify every new 12.1
operation. The run covered dashboard and
protocol login, native object parity, external search and browse, artwork,
full-song decoding, exact native audio bytes, and cached external prefix/suffix
ranges. Six checks remained blocked: three initial external range checks,
injected playlists absent from that account, playlist writes, and other
state-restoration tests. Deezer and the configured YouTube Music extension
served audio; this does not qualify every provider, account tier, or client.
The three-sample native stream run measured 34.7 ms mean Allstarr first-byte
latency against 20.4 ms direct. OIDC remains disabled pending operator setup;
its real identity-provider flow is not live-qualified.
## Code-reduction rules
Code reduction is a release objective, but deleting safety and observability is not simplification.