diff --git a/Migration/DataProviderMigrate/Program.cs b/Migration/DataProviderMigrate/Program.cs index b2c8bba6..6e201f8b 100644 --- a/Migration/DataProviderMigrate/Program.cs +++ b/Migration/DataProviderMigrate/Program.cs @@ -218,7 +218,7 @@ MigratePhase phase schema, allowDestructive, phase, - () => PostgresSchemaInspector.Inspect(connection, "public"), + () => PostgresPolicyCatalogNormalizer.Inspect(connection: connection), ops => MigrationRunner.Apply( connection, @@ -229,7 +229,8 @@ MigratePhase phase VerifySchemaIntegrity( schema, phase, - () => PostgresSchemaInspector.Inspect(connection, "public"), + () => + PostgresPolicyCatalogNormalizer.Inspect(connection: connection), (live, target) => PostgresPolicyCatalogNormalizer.Normalize( connection, diff --git a/Migration/Nimblesite.DataProvider.Migration.Postgres/PostgresPolicyCatalogNormalizer.cs b/Migration/Nimblesite.DataProvider.Migration.Postgres/PostgresPolicyCatalogNormalizer.cs index 6fec2e6d..48390c5b 100644 --- a/Migration/Nimblesite.DataProvider.Migration.Postgres/PostgresPolicyCatalogNormalizer.cs +++ b/Migration/Nimblesite.DataProvider.Migration.Postgres/PostgresPolicyCatalogNormalizer.cs @@ -5,6 +5,13 @@ string, Nimblesite.DataProvider.Migration.Core.MigrationError >.Ok; +using SchemaOk = Outcome.Result< + Nimblesite.DataProvider.Migration.Core.SchemaDefinition, + Nimblesite.DataProvider.Migration.Core.MigrationError +>.Ok< + Nimblesite.DataProvider.Migration.Core.SchemaDefinition, + Nimblesite.DataProvider.Migration.Core.MigrationError +>; namespace Nimblesite.DataProvider.Migration.Postgres; @@ -15,6 +22,20 @@ namespace Nimblesite.DataProvider.Migration.Postgres; /// public static class PostgresPolicyCatalogNormalizer { + /// + /// Inspect the live schema with policy predicates in the same view context + /// used for desired predicates. Implements [RLS-DIFF] and issue #119. + /// PostgreSQL 15 qualifies view columns differently from pg_policies; + /// normalizing both preserves correlated references and subquery shadowing. + /// + public static SchemaResult Inspect(NpgsqlConnection connection) + { + var result = PostgresSchemaInspector.Inspect(connection: connection, schemaName: "public"); + return result is SchemaOk ok + ? Normalize(connection: connection, live: ok.Value, desired: ok.Value) + : result; + } + /// Canonicalize policies on tables that already exist. public static SchemaResult Normalize( NpgsqlConnection connection, diff --git a/Migration/Nimblesite.DataProvider.Migration.Tests/Postgres15ContainerFixture.cs b/Migration/Nimblesite.DataProvider.Migration.Tests/Postgres15ContainerFixture.cs new file mode 100644 index 00000000..c58c5bb0 --- /dev/null +++ b/Migration/Nimblesite.DataProvider.Migration.Tests/Postgres15ContainerFixture.cs @@ -0,0 +1,14 @@ +namespace Nimblesite.DataProvider.Migration.Tests; + +// Implements [RLS-DIFF]: PostgreSQL 15 deparses view and policy columns differently. +public sealed record Postgres15ContainerFixture : IAsyncLifetime +{ + private readonly PostgresContainerFixture _fixture = new(); + + public Task InitializeAsync() => _fixture.InitializeAsync(image: "postgres:15.1"); + + public Task DisposeAsync() => _fixture.DisposeAsync(); + + public NpgsqlConnection CreateDatabase(string namePrefix) => + _fixture.CreateDatabase(namePrefix: namePrefix); +} diff --git a/Migration/Nimblesite.DataProvider.Migration.Tests/Postgres15TestSuite.cs b/Migration/Nimblesite.DataProvider.Migration.Tests/Postgres15TestSuite.cs new file mode 100644 index 00000000..ec6bab1c --- /dev/null +++ b/Migration/Nimblesite.DataProvider.Migration.Tests/Postgres15TestSuite.cs @@ -0,0 +1,7 @@ +namespace Nimblesite.DataProvider.Migration.Tests; + +[CollectionDefinition(Name)] +public sealed class Postgres15TestSuite : ICollectionFixture +{ + public const string Name = "Postgres15"; +} diff --git a/Migration/Nimblesite.DataProvider.Migration.Tests/PostgresPolicyCatalogCliTests.cs b/Migration/Nimblesite.DataProvider.Migration.Tests/PostgresPolicyCatalogCliTests.cs new file mode 100644 index 00000000..67864ffc --- /dev/null +++ b/Migration/Nimblesite.DataProvider.Migration.Tests/PostgresPolicyCatalogCliTests.cs @@ -0,0 +1,205 @@ +using IntegrityOk = Outcome.Result< + System.Collections.Immutable.ImmutableArray, + Nimblesite.DataProvider.Migration.Core.MigrationError +>.Ok< + System.Collections.Immutable.ImmutableArray, + Nimblesite.DataProvider.Migration.Core.MigrationError +>; + +namespace Nimblesite.DataProvider.Migration.Tests; + +// Implements [RLS-DIFF]. Issue #119: view qualification must not become policy drift. +[Collection(Postgres15TestSuite.Name)] +public sealed class PostgresPolicyCatalogCliTests(Postgres15ContainerFixture fixture) +{ + [Theory] + [InlineData("id = 1", "id = 2", null, null, false)] + [InlineData("id = 1", "items.id = 2", "id > 0", "id > 0", true)] + [InlineData("id > 0", "id > 0", "id = 1", "id = 2", false)] + [InlineData(null, null, "id = 1", "id = 2", false)] + [InlineData("NOT (id = 1)", "NOT (id = 2)", null, null, true)] + [InlineData("abs(id) = 1", "abs(id) = 2", null, null, true)] + [InlineData("\"id\" = 1", "\"id\" = 2", null, null, true)] + [InlineData( + "EXISTS (SELECT 1 FROM public.items AS other WHERE other.id = items.id)", + "EXISTS (SELECT 1 FROM public.items AS other WHERE other.id = id)", + null, + null, + true + )] + public void ColumnPredicates_CreateReapplyAndChangeWithoutFalseIntegrityDrift( + string? initialUsing, + string? changedUsing, + string? initialCheck, + string? changedCheck, + bool sql + ) + { + using var connection = fixture.CreateDatabase(namePrefix: "policy_column_repro"); + var initial = PolicyYaml(predicate: initialUsing, check: initialCheck, sql: sql); + var changed = PolicyYaml(predicate: changedUsing, check: changedCheck, sql: sql); + AssertSuccessfulMigration( + connection: connection, + yaml: initial, + expected: "CreateRlsPolicyOperation" + ); + var before = ReadPolicy(connection: connection); + AssertSuccessfulMigration( + connection: connection, + yaml: initial, + expected: "no operations needed" + ); + AssertSuccessfulMigration( + connection: connection, + yaml: changed, + expected: "AlterRlsPolicyOperation" + ); + var after = ReadPolicy(connection: connection); + Assert.Equal( + expected: initialUsing == changedUsing, + actual: before.UsingSql == after.UsingSql + ); + Assert.Equal( + expected: initialCheck == changedCheck, + actual: before.WithCheckSql == after.WithCheckSql + ); + AssertSuccessfulMigration( + connection: connection, + yaml: changed, + expected: "no operations needed" + ); + } + + [Fact] + public void RealPredicateDrift_FailsIntegrityAndRollsBackPolicyAlteration() + { + using var connection = fixture.CreateDatabase(namePrefix: "policy_drift_rollback"); + var yaml = PolicyYaml(predicate: "id = 1", check: null, sql: false); + AssertSuccessfulMigration( + connection: connection, + yaml: yaml, + expected: "CreateRlsPolicyOperation" + ); + var desired = SchemaYamlSerializer.FromYaml(yaml: yaml); + var before = ReadPolicy(connection: connection); + var result = MigrationRunner.Apply( + connection: connection, + operations: + [ + new AlterRlsPolicyOperation( + Schema: "public", + TableName: "items", + Policy: before with + { + UsingSql = "id = 2", + } + ), + ], + generateDdl: PostgresDdlGenerator.Generate, + options: MigrationOptions.Default, + verifyBeforeCommit: () => RejectDrift(connection: connection, desired: desired) + ); + var error = Assert.IsType(@object: result); + Assert.Equal( + expected: "Schema integrity verification failed before commit", + actual: error.Value.Message + ); + Assert.Equivalent( + expected: before, + actual: ReadPolicy(connection: connection), + strict: true + ); + } + + private static bool RejectDrift(NpgsqlConnection connection, SchemaDefinition desired) + { + var live = Assert + .IsType(PostgresPolicyCatalogNormalizer.Inspect(connection: connection)) + .Value; + var normalized = Assert + .IsType( + PostgresPolicyCatalogNormalizer.Normalize( + connection: connection, + live: live, + desired: desired + ) + ) + .Value; + var mismatches = Assert + .IsType(SchemaIntegrityVerifier.Verify(live: live, desired: normalized)) + .Value; + Assert.Contains( + expected: "public.items: policy item_visible USING predicate drifted", + collection: mismatches + ); + return mismatches.IsEmpty; + } + + private static void AssertSuccessfulMigration( + NpgsqlConnection connection, + string yaml, + string expected + ) + { + var path = Path.GetTempFileName(); + try + { + File.WriteAllText(path: path, contents: yaml); + var result = MigrationCliConsole.Migrate( + schemaPath: path, + provider: "postgres", + output: connection.ConnectionString + ); + Assert.True(condition: result.ExitCode == 0, userMessage: result.Output); + Assert.Contains( + expectedSubstring: expected, + actualString: result.Output, + comparisonType: StringComparison.Ordinal + ); + } + finally + { + File.Delete(path: path); + } + } + + private static RlsPolicyDefinition ReadPolicy(NpgsqlConnection connection) => + Assert.Single( + Assert + .Single( + Assert + .IsType( + PostgresSchemaInspector.Inspect(connection: connection) + ) + .Value.Tables + ) + .RowLevelSecurity?.Policies + ?? [] + ); + + private static string PolicyYaml(string? predicate, string? check, bool sql) => + $$""" + name: nap_policy_repro + tables: + - name: items + schema: public + columns: + - name: id + type: Int + isNullable: false + primaryKey: + columns: [id] + rowLevelSecurity: + enabled: true + forced: true + policies: + - name: item_visible + permissive: true + operations: [{{(check is null ? "Select" : predicate is null ? "Insert" : "All")}}] + roles: [PUBLIC] + {{(predicate is null ? "" : $"{(sql ? "usingSql" : "using")}: '{predicate}'")}} + {{( + check is null ? "" : $"{(sql ? "withCheckSql" : "withCheck")}: '{check}'" + )}} + """; +} diff --git a/Tests.Shared/PostgresContainerFixture.cs b/Tests.Shared/PostgresContainerFixture.cs index de8ceb00..4310dc80 100644 --- a/Tests.Shared/PostgresContainerFixture.cs +++ b/Tests.Shared/PostgresContainerFixture.cs @@ -30,7 +30,10 @@ public sealed class PostgresContainerFixture : IAsyncLifetime /// /// Starts the shared container. Called once per collection fixture lifetime. /// - public async Task InitializeAsync() + public Task InitializeAsync() => InitializeAsync(image: "pgvector/pgvector:pg16"); + + /// Starts the shared fixture using the specified PostgreSQL image. + public async Task InitializeAsync(string image) { // Implements [MIG-TEST-NATIVE-POSTGRES]: use an isolated native server when configured. if ( @@ -46,7 +49,7 @@ public async Task InitializeAsync() // test that needs vector columns (VectorType). Non-vector tests are // unaffected. _container = new PostgreSqlBuilder() - .WithImage("pgvector/pgvector:pg16") + .WithImage(image) .WithDatabase("postgres") .WithUsername("test") .WithPassword("test")