From 4b15b0166d686bed478dfaa8df2eddd5eae039c6 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sun, 13 Sep 2026 16:16:01 +0200 Subject: [PATCH] fix(labels): enforce per-tenant uniqueness on NormalizedName (#8112) * fix(labels): enforce per-tenant uniqueness on NormalizedName Finish the unused Label.NormalizedName contract the same way Secrets does: unique (TenantId, NormalizedName) in EF, fail-closed Memory saves, and keep NormalizedName in sync with Name. Co-authored-by: Sipke Schoorstra * fix(labels): clone Memory label reads so a rejected rename cannot persist Find/List handed out live store refs. Labels.Update mutates Name (and NormalizedName) on that instance before Save; a uniqueness rejection then left the stored row already renamed. Clone-on-read matches Memory identity stores from #8108. Co-authored-by: Sipke Schoorstra * fix(labels): stamp null TenantId, fail-loud on leftover duplicates, cap names at 255 Follow the Architect steer for Greptile P1s: do not auto-delete duplicate labels; UPDATE Labels SET TenantId = '' WHERE TenantId IS NULL on every provider before CreateIndex; keep SQL Server/Oracle filtered unique indexes; HasMaxLength(255) on Name and NormalizedName with no silent truncate. Co-authored-by: Sipke Schoorstra * fix(labels): fail-loud preflight for leftover keys and over-length names No silent dedupe: every provider lists leftover (TenantId, NormalizedName) keys and aborts before CreateIndex. SQL Server/Oracle keep their filtered unique indexes. PostgreSQL/SQLite keep provider column types. Providers that narrow Name/NormalizedName to 255 preflight over-length Ids first. Memory uniqueness semantics are unchanged. Co-authored-by: Sipke Schoorstra * test(labels): match Oracle filtered-index escape in migration assertion Co-authored-by: Sipke Schoorstra --------- Co-authored-by: Cursor Agent --- .../Endpoints/Labels/Update/Mappers.cs | 1 - src/modules/Elsa.Labels/Entities/Label.cs | 13 ++ .../Services/InMemoryLabelStore.cs | 81 +++++++- ...33332_PerTenantLabelUniqueness.Designer.cs | 100 ++++++++++ ...20260913133332_PerTenantLabelUniqueness.cs | 172 +++++++++++++++++ .../LabelsElsaDbContextModelSnapshot.cs | 14 +- ...33340_PerTenantLabelUniqueness.Designer.cs | 101 ++++++++++ ...20260913133340_PerTenantLabelUniqueness.cs | 174 +++++++++++++++++ .../LabelsElsaDbContextModelSnapshot.cs | 15 +- ...33325_PerTenantLabelUniqueness.Designer.cs | 100 ++++++++++ ...20260913133325_PerTenantLabelUniqueness.cs | 66 +++++++ .../LabelsElsaDbContextModelSnapshot.cs | 8 +- ...33317_PerTenantLabelUniqueness.Designer.cs | 101 ++++++++++ ...20260913133317_PerTenantLabelUniqueness.cs | 157 +++++++++++++++ .../LabelsElsaDbContextModelSnapshot.cs | 15 +- ...33258_PerTenantLabelUniqueness.Designer.cs | 96 ++++++++++ ...20260913133258_PerTenantLabelUniqueness.cs | 63 ++++++ .../LabelsElsaDbContextModelSnapshot.cs | 8 +- .../Modules/Labels/Configurations.cs | 10 + .../Entities/LabelNameMaxLengthTests.cs | 27 +++ .../InMemoryLabelStoreUniquenessTests.cs | 180 ++++++++++++++++++ .../LabelNormalizedNameUniquenessTests.cs | 40 ++++ .../LabelPerTenantUniquenessMigrationTests.cs | 101 ++++++++++ 23 files changed, 1620 insertions(+), 23 deletions(-) create mode 100644 src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Labels/20260913133332_PerTenantLabelUniqueness.Designer.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.MySql/Migrations/Labels/20260913133332_PerTenantLabelUniqueness.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Labels/20260913133340_PerTenantLabelUniqueness.Designer.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.Oracle/Migrations/Labels/20260913133340_PerTenantLabelUniqueness.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Labels/20260913133325_PerTenantLabelUniqueness.Designer.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Labels/20260913133325_PerTenantLabelUniqueness.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Labels/20260913133317_PerTenantLabelUniqueness.Designer.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Labels/20260913133317_PerTenantLabelUniqueness.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Labels/20260913133258_PerTenantLabelUniqueness.Designer.cs create mode 100644 src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Labels/20260913133258_PerTenantLabelUniqueness.cs create mode 100644 test/unit/Elsa.Labels.UnitTests/Entities/LabelNameMaxLengthTests.cs create mode 100644 test/unit/Elsa.Labels.UnitTests/Services/InMemoryLabelStoreUniquenessTests.cs create mode 100644 test/unit/Elsa.Persistence.EFCore.UnitTests/LabelNormalizedNameUniquenessTests.cs create mode 100644 test/unit/Elsa.Persistence.EFCore.UnitTests/LabelPerTenantUniquenessMigrationTests.cs diff --git a/src/modules/Elsa.Labels/Endpoints/Labels/Update/Mappers.cs b/src/modules/Elsa.Labels/Endpoints/Labels/Update/Mappers.cs index bc1ca9181..ce8eb9177 100644 --- a/src/modules/Elsa.Labels/Endpoints/Labels/Update/Mappers.cs +++ b/src/modules/Elsa.Labels/Endpoints/Labels/Update/Mappers.cs @@ -26,7 +26,6 @@ internal class LabelMapper : Mapper e.Color = r.Color?.Trim(); e.Description = r.Description?.Trim(); e.Name = r.Name.Trim(); - e.NormalizedName = r.Name.ToLowerInvariant(); return e; } } \ No newline at end of file diff --git a/src/modules/Elsa.Labels/Entities/Label.cs b/src/modules/Elsa.Labels/Entities/Label.cs index f6dd24e22..2f2ae4b0d 100644 --- a/src/modules/Elsa.Labels/Entities/Label.cs +++ b/src/modules/Elsa.Labels/Entities/Label.cs @@ -7,6 +7,12 @@ namespace Elsa.Labels.Entities; /// public class Label : Entity { + /// + /// Shared maximum for and . + /// Matches the MySQL unique-index limit that every provider's schema must honor. + /// + public const int NameMaxLength = 255; + private string _name = default!; public string Name @@ -14,6 +20,13 @@ public class Label : Entity get => _name; set { + if (value.Length > NameMaxLength) + { + throw new ArgumentException( + $"Label name cannot exceed {NameMaxLength} characters.", + nameof(Name)); + } + _name = value; NormalizedName = value.ToLowerInvariant(); } diff --git a/src/modules/Elsa.Labels/Services/InMemoryLabelStore.cs b/src/modules/Elsa.Labels/Services/InMemoryLabelStore.cs index a5e64795f..242443a72 100644 --- a/src/modules/Elsa.Labels/Services/InMemoryLabelStore.cs +++ b/src/modules/Elsa.Labels/Services/InMemoryLabelStore.cs @@ -38,9 +38,14 @@ public class InMemoryLabelStore : ILabelStore /// public Task SaveAsync(Label record, CancellationToken cancellationToken = default) { - ApplyCurrentTenant(record); lock (_labelStore.Sync) + { + ApplyCurrentTenant(record); + SyncNormalizedName(record); + EnsureNormalizedNameAvailable(record, [record]); _labelStore.Save(record, x => x.Id); + } + return Task.CompletedTask; } @@ -49,11 +54,20 @@ public class InMemoryLabelStore : ILabelStore { var list = records.ToList(); - foreach (var record in list) - ApplyCurrentTenant(record); - lock (_labelStore.Sync) + { + foreach (var record in list) + { + ApplyCurrentTenant(record); + SyncNormalizedName(record); + } + + foreach (var record in list) + EnsureNormalizedNameAvailable(record, list); + _labelStore.SaveMany(list, x => x.Id); + } + return Task.CompletedTask; } @@ -90,7 +104,8 @@ public class InMemoryLabelStore : ILabelStore /// public Task FindByIdAsync(string id, CancellationToken cancellationToken = default) { - return Task.FromResult(FindVisibleLabel(id)); + var found = FindVisibleLabel(id); + return Task.FromResult(found is null ? null : Clone(found)); } /// @@ -98,20 +113,38 @@ public class InMemoryLabelStore : ILabelStore { var query = _labelStore.List().AsQueryable().WhereVisibleToTenant(CurrentTenantId).OrderBy(x => x.Name); var page = query.ToPage(pageArgs); - return Task.FromResult(page); + return Task.FromResult(Page.Of(page.Items.Select(Clone).ToList(), page.TotalCount)); } /// public Task> FindManyByIdAsync(IEnumerable ids, CancellationToken cancellationToken) { var idList = ids.ToList(); - var records = _labelStore.Query(query => query.WhereVisibleToTenant(CurrentTenantId).Where(x => idList.Contains(x.Id))); - return Task.FromResult(records); + var records = _labelStore.Query(query => query.WhereVisibleToTenant(CurrentTenantId).Where(x => idList.Contains(x.Id))) + .Select(Clone) + .ToList(); + return Task.FromResult>(records); } private Label? FindVisibleLabel(string id) => _labelStore.Query(query => query.WhereVisibleToTenant(CurrentTenantId).Where(x => x.Id == id)).FirstOrDefault(); + /// + /// Clone-on-read, matching Memory identity stores. Labels.Update does Find → mutate + /// Name (which writes NormalizedName) → Save. Without a copy, a rejected rename + /// would already have mutated the stored row. + /// + private static Label Clone(Label label) => + new() + { + Id = label.Id, + TenantId = label.TenantId, + Name = label.Name, + NormalizedName = label.NormalizedName, + Description = label.Description, + Color = label.Color + }; + private bool IsVisible(Entity entity) => TenantVisibility.IsVisible(entity.TenantId, CurrentTenantId); private string CurrentTenantId => _tenantAccessor?.TenantId ?? Tenant.DefaultTenantId; @@ -123,4 +156,36 @@ public class InMemoryLabelStore : ILabelStore entity.TenantId ??= _tenantAccessor.TenantId; } + + private static void SyncNormalizedName(Label record) => + record.NormalizedName = record.Name.ToLowerInvariant(); + + /// + /// Memory counterpart of the EF unique index on (TenantId, NormalizedName). + /// Same-Id upserts are allowed so a row can rename itself. Incoming batch rows + /// replace same-Id store rows, so those store rows are ignored here. + /// + private void EnsureNormalizedNameAvailable(Label record, IReadOnlyCollection