[erikdarlingdata]

Claude posting for Erik Darling.

This changes tests only. One Darling live test ran the managed provisioning batch inside a scratch database. That batch revokes CONNECT from PUBLIC on the database named darling. Database privileges are shared by the whole cluster. So the shared test database lost them for every test after it.

The nightly failure was a victim. The redaction test opens a connection as a fresh role, and a fresh role has CONNECT only through PUBLIC. The culprit now aims its batch at its own scratch database. The victim grants its own role CONNECT. A new guard fails the test that does this the next time, by name.

SECTION: none (test-only)

What failed

Nightly run 37880262182 (dev b76725f), Darling PostgreSQL job: 1 failed of 30114.
PgSettingRedactionLivePostgresTests.PrimaryConninfosPassword_IsRedactedByTheRealCollector_UnderAPgMonitorOnlyRole,
42501: permission denied for database "darling" on the line that opens the test's own pg_monitor-only role.

The cause (measured, one class)

I ran 21 candidate classes one at a time against a local PostgreSQL 17 (database darling). Before and after each class I read
has_database_privilege('public', 'darling', 'CONNECT') and the database ACL. I restored PUBLIC's grant between classes. The candidates were these:

• Every class that builds or runs the provisioning batch or reaches EnsureProvisionedAsync.
• Every other member of pg-cluster-roles.
• The census allow-list classes that lift or retarget the provisioning text.

I did not measure the allow-list classes whose entry says they only feed sample statement text to a scrubber (four of them). I also did not measure MigrationUpgradeLadderLiveTests and ComposeTempFileLimitLiveTests, because their entries say they write nothing at database level. I read them instead.

Exactly one flips it, in every one of the five runs I made: PasswordKeyTablesLiveTests. ACL before {=Tc/darling,darling=CTc/darling}, after {darling=CTc/darling}. The other 20 leave it untouched, including AlertHistoryDismissTests lines 114 and 310 (the lead in the issue): it builds the text and never runs it.

The test is Provisioning_LeavesNoReadOfThePinTables_AndNoTriggerOrReferencesGrant_AfterAGrantAllRerun(managed: true). I turned on log_statement = 'ddl' and ran just that method. The server log shows REVOKE ALL ON DATABASE darling FROM PUBLIC arriving twice (the test reruns the batch) on a session whose database is darling_scratch_.... It shows none for managed: false. After the method PUBLIC reads false. The ComposeStore case does not do it, because that target leaves the revoke out.

The same measurement, plus reading, ruled out these causes: REVOKE CONNECT, ALTER DATABASE on darling, and a drop or recreate of darling. DarlingCliCommandsHostCheckTests creates it only when absent and drops only what it created. DarlingStoreUpgradeTests runs on its own cluster.

Why it showed up after #5604: inferred, not measured. #5604 moved both PasswordKeyTablesLiveTests and the redaction class from live-postgres (about sixty classes) into pg-cluster-roles (nine). Classes inside one collection run one at a time in an order xUnit picks, and I did not look into the rule.

Whether the redaction test runs before or after the revoking test depends on that order. The move changed it for CI. In my local run of the two classes together, the redaction test ran first and passed. PUBLIC ended revoked. Nothing in the suite puts the grant back, so the first test to need it after the revoke fails.

What changes

• PgSettingRedactionLivePostgresTests.cs:78 and :168: the probe role is granted CONNECT on the database the test connects to. The grant is revoked in the cleanup before the role is dropped. A role that holds a privilege cannot be dropped. The revoke is guarded for a body that failed before creating the role. Every existing assertion is unchanged. The class comment said "live-postgres collection". It now says pg-cluster-roles.
• PasswordKeyTablesLiveTests.cs:292-315: the managed case keeps the managed shape (owner, role marker, the PUBLIC revoke). Its three database-level statements are now pointed at the test's own scratch database, the way ViewerGrantReplay already does for its replays.
  • Two asserts make that checkable. The batch must contain the managed revoke before the rewrite. After the rewrite, it must contain none that names the shared database.
  • A new assertion (line 344) proves the managed revoke on the scratch database. For the managed case PUBLIC has no CONNECT there. For the compose case it keeps CONNECT. The skip for "no darling database" is gone, because the test no longer touches that database. The owner check stays.
• PublicConnectGuardAttribute.cs (new) on PgClusterRolesCollection.cs:41: an xUnit before/after attribute on the collection. Before and after every test, it reads PUBLIC's CONNECT on the database DARLING_TEST_PG names and on darling. It fails the test where a granted privilege became revoked. It compares before with after instead of demanding a grant. So a developer's own managed store (revoked on purpose) does not fail everything. It does nothing without DARLING_TEST_PG. I did not touch ScratchPostgres. Its comment says a dispose must not fail a passing test. The guard fits the collection that is the only home of this kind of test.
• PublicConnectDelta.cs (new): the comparison, store-free so a guard-stage test can pin it. PublicConnectGuardTests.cs (guard stage): the comparison. PublicConnectCatalogReadLiveTests.cs: the catalog read against a real cluster, on a scratch database, and a pin that PgClusterRolesCollection still carries the attribute.
• No product code, no Lite change (Lite has no PostgreSQL), no ScratchPostgres change.

RED proofs

• Redaction test: REVOKE ALL ON DATABASE darling FROM PUBLIC, then the old test: Failed: 1, 42501: permission denied for database "darling". The new test on the same state: Total: 1, Failed: 0. The database ACL after is {darling=CTc/darling} (no leftover grant) and no leftover role. PUBLIC restored with GRANT CONNECT, TEMPORARY ON DATABASE darling TO PUBLIC.
• Culprit: before the fix PasswordKeyTablesLiveTests leaves the probe at false. After the fix, true (ACL unchanged).
• Guard: remove only the retarget and its assertion from PasswordKeyTablesLiveTests. Then the class reports Total: 6, Failed: 1. The managed: True case fails with "... left PUBLIC without CONNECT on database "darling" (A Darling live test fails when another test leaves PUBLIC without CONNECT on the shared test database #5618) ...", and nothing else fails. With the retarget restored, 0 failed.

Test plan

• Darling.Tests builds with 0 warnings, 0 errors.
• Culprit, victim and every pg-cluster-roles member, in one process (the shape of CI): Total 56, Failed 0, Skipped 1. PUBLIC's CONNECT on darling true afterwards.
• The 21 classes in the cause section, one at a time, on the code before the fix. A class that reports Skipped skips without its own PostgreSQL.
• PublicConnect*: Total 3, Failed 0.
• *Census* Total 499, Failed 0. *Ratchet* 15 / 0. *Hygiene* 90 / 0. *Scrub* 123 / 0. *Guard* 365 / 0. Stage=Guard 1469 / 0. All on the final head after merging origin/dev.
• The full Darling.Tests suite (CI decides), and a CI-order run of the whole pg-cluster-roles collection together with the parallel collections.

Not done

Nothing was left undone. One thing to look at: the guard covers the pg-cluster-roles collection only. A class outside it that revokes PUBLIC's CONNECT is still found by the victim's name. PgClusterObjectCensusTests keeps such classes inside that collection or on its allow list. Such classes are those that name the managed batch or a database-level statement. I measured every allow-listed class except the six named under the cause, which I read instead.

Fixes #5618