From c17f3eb564e2541833ac06faebfa71bf4be873cd Mon Sep 17 00:00:00 2001 From: mforce Date: Sun, 23 Aug 2026 23:41:12 -0700 Subject: [PATCH 1/8] fix(api): print the farm code from bootstrap-admin (#589) --- .../Cli/BootstrapAdminCliCommand.cs | 8 +++- .../Identity/FirstRunAdminService.cs | 40 ++++++++++++++----- .../BootstrapAdminCommandTests.cs | 35 ++++++++++++++++ 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs index daffb1992..4be1b1bd4 100644 --- a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs +++ b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs @@ -65,8 +65,14 @@ await Console.Out.WriteLineAsync( // journald, a platform log pipeline) may still capture it — treat // the value as sensitive, same as the recover-admin runbook // instructs (#265 review). + // #589 — the FARM CODE is named explicitly because it is the one field + // the operator must type and has no other way to discover; the account + // GUID is diagnostic only. Without this line the command ends with + // "log in with this now" and they cannot: #532 made the farm code a + // required login input and never swept this caller (AGENTS.md #394). await Console.Out.WriteLineAsync( - $"First-run admin provisioned: {outcome.Email} (account {outcome.AccountId})."); + $"First-run admin provisioned: {outcome.Email} on farm {outcome.Slug} " + + $"(account {outcome.AccountId})."); await Console.Out.WriteLineAsync($"Temporary password: {outcome.TemporaryPassword}"); await Console.Out.WriteLineAsync( "Log in with this now — the app requires a new password before anything else works."); diff --git a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs index d25563a73..3a6dac4be 100644 --- a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs +++ b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs @@ -118,10 +118,19 @@ public async Task> ProvisionAsync( // the schema is current (MigrateAsync above already ran) — this is // defense-in-depth against a hand-rolled/partially-restored schema, // not the expected path. Read-only, so it stays outside the lock. - var accountExists = await db.Accounts + // #589 — select the SLUG instead of testing existence. The operator cannot + // sign in without the farm code (#532 made it a required login input), and + // this query already runs on this path, so reading the value here costs no + // extra round trip. A null result still means "no such account" and takes + // the same failure path as before, so the guard below is unchanged in + // meaning. Deliberately NOT a "default-farm" literal: that value lives in + // 20260818235944_AddAccountSlug.cs and a second copy would drift. + var accountSlug = await db.Accounts .IgnoreQueryFilters() - .AnyAsync(a => a.Id == accountId, ct); - if (!accountExists) + .Where(a => a.Id == accountId) + .Select(a => a.Slug) + .SingleOrDefaultAsync(ct); + if (accountSlug is null) return Result.Failure(Error.Validation( "Bootstrap.AccountMissing", "The default account does not exist. This should never happen against a schema this " + @@ -149,7 +158,7 @@ await db.Database.ExecuteSqlInterpolatedAsync( $"SELECT pg_advisory_lock({AdvisoryLockClassId}, {AdvisoryLockObjectId})", ct); try { - return await ProvisionUnderLockAsync(accountId, email.Trim(), ct); + return await ProvisionUnderLockAsync(accountId, accountSlug, email.Trim(), ct); } finally { @@ -211,7 +220,7 @@ private async Task TryCleanupAsync(Func cleanup, string what) } private async Task> ProvisionUnderLockAsync( - Guid accountId, string email, CancellationToken ct) + Guid accountId, string accountSlug, string email, CancellationToken ct) { // Idempotency: an Owner already existing in the default account means // first-run provisioning already happened. Re-checked HERE (not just @@ -332,7 +341,8 @@ private async Task> ProvisionUnderLockAsync( } await transaction.CommitAsync(token); - return Result.Success(FirstRunAdminOutcome.Provisioned(email, accountId, password)); + return Result.Success( + FirstRunAdminOutcome.Provisioned(email, accountId, accountSlug, password)); }, ct); } @@ -366,11 +376,21 @@ SELECT 1 FROM pg_locks """).SingleAsync(ct); } +// #589 — Slug is NULLABLE and sits beside the other nullable fields deliberately. +// This record is not a mirror of AccountProvisionOutcome: that one has no no-op +// path, so all its fields are non-null, whereas AlreadyProvisioned() here returns +// a value with nothing populated. Copying its non-nullable `string Slug` across +// would make the idempotent branch unrepresentable. public sealed record FirstRunAdminOutcome( - bool WasAlreadyProvisioned, string? Email, Guid? AccountId, string? TemporaryPassword) + bool WasAlreadyProvisioned, + string? Email, + Guid? AccountId, + string? Slug, + string? TemporaryPassword) { - public static FirstRunAdminOutcome AlreadyProvisioned() => new(true, null, null, null); + public static FirstRunAdminOutcome AlreadyProvisioned() => new(true, null, null, null, null); - public static FirstRunAdminOutcome Provisioned(string email, Guid accountId, string password) => - new(false, email, accountId, password); + public static FirstRunAdminOutcome Provisioned( + string email, Guid accountId, string slug, string password) => + new(false, email, accountId, slug, password); } diff --git a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs index 9f024bf89..2ba43b1dd 100644 --- a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs @@ -67,6 +67,22 @@ public async Task FirstRun_CreatesOwner_PrintsPasswordOnlyToStdout_AndSetsMustCh Assert.True(0 == exitCode, $"expected exit 0, got {exitCode}. stdout={stdout} stderr={stderr}"); Assert.Contains(email, stdout); Assert.Contains("Temporary password:", stdout); + // #589 — assert the FARM CODE is printed, and read the expected value + // from the Accounts row rather than a "default-farm" literal: a + // hardcoded copy here would pass even if the command printed a + // constant instead of the real slug, and would need editing if the + // seed value ever changed. #532 made the farm code a required login + // input, so without this line the operator cannot sign in. + using (var assertScope = _factory.Services.CreateScope()) + { + var defaultAccountSlug = await assertScope.ServiceProvider + .GetRequiredService() + .Accounts.IgnoreQueryFilters() + .Where(a => a.Id == SeedDefaults.AccountId) + .Select(a => a.Slug) + .SingleAsync(); + Assert.Contains($"on farm {defaultAccountSlug} ", stdout); + } // The password itself never reaches stderr (nothing does on success). Assert.Equal(string.Empty, stderr); // #273 — the actual "never the logger" guarantee, asserted directly: @@ -129,6 +145,25 @@ public async Task Rerun_AfterFirstRunSucceeded_IsIdempotent_PrintsNoSecret_AndEx Assert.Single(owners, u => u.AccountId == SeedDefaults.AccountId); } + [Fact] + public async Task Rerun_WhenAlreadyProvisioned_DoesNotPrintTheFarmCode() + { + // Reuses the idempotent-rerun setup: first run provisions the Owner, + // second run is a no-op where WasAlreadyProvisioned is true. + var email = $"bootstrap-farmcode-{Guid.NewGuid():N}@test.local"; + var first = await RunBootstrapCommandAsync(email); + Assert.True(0 == first.ExitCode, $"first run: expected exit 0, got {first.ExitCode}. stderr={first.Stderr}"); + + var second = await RunBootstrapCommandAsync(email); + Assert.True(0 == second.ExitCode, $"rerun: expected exit 0, got {second.ExitCode}. stderr={second.Stderr}"); + Assert.Contains("already provisioned", second.Stdout, StringComparison.OrdinalIgnoreCase); + // #589 — the idempotent path must stay silent about the farm code: no + // provisioning happened, so no "on farm" line may appear. This is the + // mutant that keeps the AlreadyProvisioned() branch honest — if that + // branch ever started populating Slug and printing it, this reds. + Assert.DoesNotContain(" on farm ", second.Stdout, StringComparison.Ordinal); + } + [Fact] public async Task MissingEmailFlag_ExitsNonZeroWithClearMessage() { From 759fd3fa7b4449507725fca284590c19c76fec2b Mon Sep 17 00:00:00 2001 From: mforce Date: Sun, 23 Aug 2026 23:47:29 -0700 Subject: [PATCH 2/8] fix(tests): update the tenant-bypass allow-list for the new signature (#589) `tenant-bypass-allowlist.json` pins `ProvisionUnderLockAsync`'s exact symbol signature, and #589 added an `accountSlug` parameter to it, so the enumerating guard from #536 went red. The entry now matches; the justification is unchanged because the reason for the bypass is unchanged (first-run bootstrap runs at an unresolved tenant by design, #283). This was a defect in the runbook, not in the implementation: the file was left out of the permitted-edit list because the driver authored a signature change without grepping for readers of that symbol. The implementer correctly refused to touch a file outside its allow-list and flagged the decision instead. Verified the entry was corrected rather than the guard widened: renaming the parameter reddens TenantBypass again (1 failed / 15 passed); restoring it is green (16 passed). Full suite 1942 passed, 0 failed. --- .../TenantBypass/Data/tenant-bypass-allowlist.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Cluckwork.Application.Tests/TenantBypass/Data/tenant-bypass-allowlist.json b/tests/Cluckwork.Application.Tests/TenantBypass/Data/tenant-bypass-allowlist.json index 40c210c45..45e68949c 100644 --- a/tests/Cluckwork.Application.Tests/TenantBypass/Data/tenant-bypass-allowlist.json +++ b/tests/Cluckwork.Application.Tests/TenantBypass/Data/tenant-bypass-allowlist.json @@ -145,7 +145,7 @@ "justification": "Caller of AccountSlugLookup.ResolveAsync (a forwarding wrapper that carries the IgnoreQueryFilters). Operator CLI suspends an account by global slug; no tenant is resolved at this point. The caller is flagged by the wrapper-forwarding walk (review P1-3) — the wrapper and its caller both need entries." }, { - "symbol": "Cluckwork.Infrastructure.Identity.FirstRunAdminService.ProvisionUnderLockAsync(Guid accountId, string email, CancellationToken ct)", + "symbol": "Cluckwork.Infrastructure.Identity.FirstRunAdminService.ProvisionUnderLockAsync(Guid accountId, string accountSlug, string email, CancellationToken ct)", "file": "src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs", "justification": "Caller of HoldsProvisioningLockAsync (a forwarding wrapper). First-run admin bootstrap runs at unresolved tenant by design (#283); the provisioning lock is a process-level advisory lock, not tenant-scoped." }, From 439077d20fba04a7bc45b90f50a7da3704d42531 Mon Sep 17 00:00:00 2001 From: mforce Date: Mon, 24 Aug 2026 07:40:38 -0700 Subject: [PATCH 3/8] fix(api): print the farm code from recover-admin too, and pin it against a varied slug (#589) --- docs/runbooks/break-glass-account-recovery.md | 7 +- docs/runbooks/first-admin-provisioning.md | 14 +- .../Cli/BootstrapAdminCliCommand.cs | 5 +- .../Cli/RecoverAdminCliCommand.cs | 4 +- .../Identity/AdminRecoveryService.cs | 11 +- .../BootstrapAdminCommandTests.cs | 140 ++++++++++++------ .../RecoverAdminCommandTests.cs | 52 +++++++ 7 files changed, 174 insertions(+), 59 deletions(-) diff --git a/docs/runbooks/break-glass-account-recovery.md b/docs/runbooks/break-glass-account-recovery.md index d705d9356..78d5b6409 100644 --- a/docs/runbooks/break-glass-account-recovery.md +++ b/docs/runbooks/break-glass-account-recovery.md @@ -87,11 +87,14 @@ docker compose -f deploy/docker-compose.yml exec app \ ### Expected output (exit code `0`) ``` -Break-glass reset complete for owner@thefarm.example (account 0000000a-...). All existing sessions were revoked. +Break-glass reset complete for owner@thefarm.example on farm (account 0000000a-...). All existing sessions were revoked. Temporary password: <20-char one-time password> Log in with this now and change it immediately (Account → change password). ``` +The **farm code** before the account GUID is the required #532 sign-in input and +is printed nowhere else, so copy it from stdout along with the password. + The temporary password is written to **stdout only** (never the logger/OTLP), so it does not persist in structured logs. Capture it from the terminal, hand it to the user over a trusted channel, and have them rotate it on first login. @@ -169,7 +172,7 @@ Nothing is changed on a failure. ## Post-recovery verification (drill this on staging before you need it) -1. The user logs in with the temporary password → **succeeds**. +1. The user logs in with the temporary password **and the printed farm code** → **succeeds**. 2. The **old** password → **rejected**. 3. Any previously open session (a still-loaded browser tab) can no longer refresh → signed out. 4. `SELECT action, actor_email, reason, occurred_at_utc FROM audit_events WHERE action = 'User.BreakGlassReset' ORDER BY occurred_at_utc DESC LIMIT 1;` shows the row with your reason. diff --git a/docs/runbooks/first-admin-provisioning.md b/docs/runbooks/first-admin-provisioning.md index 43cb5e39a..e12d6fa6d 100644 --- a/docs/runbooks/first-admin-provisioning.md +++ b/docs/runbooks/first-admin-provisioning.md @@ -21,12 +21,15 @@ Base data — the default account, roles, default egg grades, packed-unit conversions — ships inside the EF migrations, so it is already there. **No credential is ever baked into the repo**, so the admin user is not. -All three forms below share the same three properties: +All three forms below share the same properties: - the generated one-time password goes to **stdout only** — never the application logger or the OTLP pipeline. A host's stdout collector (docker logs, journald, a platform log pipeline) may still capture it, so treat that output as sensitive while the password is valid; +- the printed output also carries the **farm code** (`on farm `) — #532 made + it a required sign-in input, and it is printed nowhere else, so copy it from + stdout along with the password; - first sign-in shows a **Set your password** screen and refuses everything else until you pick your own; - a re-run against an already-provisioned account is a safe no-op. @@ -93,8 +96,8 @@ against a plaintext local Postgres (the #261/#262 TLS floor). ## Verify -1. Sign in with the printed password. The SPA must show **Set your password** and - refuse every other screen. +1. Sign in with the printed password **and the printed farm code**. The SPA must + show **Set your password** and refuse every other screen. 2. Set a password. The app releases the rest of the UI. 3. Re-run the same command. Expected: a no-op — no new user, no new password. @@ -113,7 +116,8 @@ Safe on a scratch database only. 1. `docker compose -f deploy/docker-compose.dev.yml down -v && … up -d` — a database with no Owner. -2. Run form 3. Expected: a password on stdout, exit `0`. -3. Sign in; expect **Set your password**; set one. +2. Run form 3. Expected: a password and the farm code on stdout, exit `0`. +3. Sign in with the printed password **and farm code**; expect **Set your + password**; set one. 4. Run form 3 again. Expected: no-op, exit `0`, no second password. 5. Update **Last drilled** above. diff --git a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs index 4be1b1bd4..f8158b4de 100644 --- a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs +++ b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs @@ -68,8 +68,9 @@ await Console.Out.WriteLineAsync( // #589 — the FARM CODE is named explicitly because it is the one field // the operator must type and has no other way to discover; the account // GUID is diagnostic only. Without this line the command ends with - // "log in with this now" and they cannot: #532 made the farm code a - // required login input and never swept this caller (AGENTS.md #394). + // "log in with this now" and they cannot: #532 changed the login + // contract (farm code became a required input) and this caller was + // missed. await Console.Out.WriteLineAsync( $"First-run admin provisioned: {outcome.Email} on farm {outcome.Slug} " + $"(account {outcome.AccountId})."); diff --git a/src/Cluckwork.Api/Cli/RecoverAdminCliCommand.cs b/src/Cluckwork.Api/Cli/RecoverAdminCliCommand.cs index 1014fdf08..c329c42d9 100644 --- a/src/Cluckwork.Api/Cli/RecoverAdminCliCommand.cs +++ b/src/Cluckwork.Api/Cli/RecoverAdminCliCommand.cs @@ -65,8 +65,8 @@ await Console.Error.WriteLineAsync( // rotate it on first login, as the runbook instructs (#265 review). var outcome = recovered.Value; await Console.Out.WriteLineAsync( - $"Break-glass reset complete for {outcome.Email} (account {outcome.AccountId}). " + - "All existing sessions were revoked."); + $"Break-glass reset complete for {outcome.Email} on farm {outcome.Slug} " + + $"(account {outcome.AccountId}). All existing sessions were revoked."); await Console.Out.WriteLineAsync($"Temporary password: {outcome.TemporaryPassword}"); await Console.Out.WriteLineAsync( "Log in with this now and change it immediately (Account → change password)."); diff --git a/src/Cluckwork.Infrastructure/Identity/AdminRecoveryService.cs b/src/Cluckwork.Infrastructure/Identity/AdminRecoveryService.cs index 7663622a6..f0b136d29 100644 --- a/src/Cluckwork.Infrastructure/Identity/AdminRecoveryService.cs +++ b/src/Cluckwork.Infrastructure/Identity/AdminRecoveryService.cs @@ -151,9 +151,16 @@ public async Task> RecoverAsync( return Result.Failure(reset.Error); await transaction.CommitAsync(token); - return Result.Success(new AdminRecoveryResult(target.Email!, target.AccountId, password)); + return Result.Success(new AdminRecoveryResult(target.Email!, target.AccountId, lockedAccount.Slug, password)); }, ct); } } -public sealed record AdminRecoveryResult(string Email, Guid AccountId, string TemporaryPassword); +// #589 — Slug is a plain NON-NULLABLE string, unlike FirstRunAdminOutcome's +// nullable one. That record has a no-op path (AlreadyProvisioned returns a +// value with nothing populated) so every field must be nullable there; +// AdminRecoveryResult has no no-op path — recovery always ran — so every field +// is populated and none are nullable. The slug is read off `lockedAccount` +// (already loaded FOR UPDATE in the transaction, no new query) and printed by +// recover-admin because #532 made the farm code a required login input. +public sealed record AdminRecoveryResult(string Email, Guid AccountId, string Slug, string TemporaryPassword); diff --git a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs index 2ba43b1dd..7d550bcce 100644 --- a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs @@ -62,45 +62,51 @@ public async Task FirstRun_CreatesOwner_PrintsPasswordOnlyToStdout_AndSetsMustCh { var email = $"bootstrap-{Guid.NewGuid():N}@test.local"; - var (exitCode, stdout, stderr) = await RunBootstrapCommandAsync(email); + // #589 (codex P2) — the account slug is VARIED before the command runs + // so the farm-code assertion discriminates. Every bootstrap test + // database carries the same migration-seeded "default-farm", so reading + // the slug back from the DB and asserting THAT constant would still pass + // a regression that printed the literal "default-farm". Pinning the + // varied value forces the command to actually read the account row. + // Restored in `finally` because the tests in this class share `_factory`'s + // one database. + var variedSlug = "varied-farm-x7q2"; + var originalSlug = await OverrideDefaultAccountSlugAsync(variedSlug); + try + { + var (exitCode, stdout, stderr) = await RunBootstrapCommandAsync(email); - Assert.True(0 == exitCode, $"expected exit 0, got {exitCode}. stdout={stdout} stderr={stderr}"); - Assert.Contains(email, stdout); - Assert.Contains("Temporary password:", stdout); - // #589 — assert the FARM CODE is printed, and read the expected value - // from the Accounts row rather than a "default-farm" literal: a - // hardcoded copy here would pass even if the command printed a - // constant instead of the real slug, and would need editing if the - // seed value ever changed. #532 made the farm code a required login - // input, so without this line the operator cannot sign in. - using (var assertScope = _factory.Services.CreateScope()) + Assert.True(0 == exitCode, $"expected exit 0, got {exitCode}. stdout={stdout} stderr={stderr}"); + Assert.Contains(email, stdout); + Assert.Contains("Temporary password:", stdout); + // #589 — assert the FARM CODE is printed AND that it is the VARIED + // value set above, not the seed "default-farm" (see the method + // comment). #532 made the farm code a required login input, so + // without this line the operator cannot sign in. + Assert.Contains($"on farm {variedSlug} ", stdout); + // The password itself never reaches stderr (nothing does on success). + Assert.Equal(string.Empty, stderr); + // #273 — the actual "never the logger" guarantee, asserted directly: + // Serilog's own Console sink ALSO writes to this process's stdout (the + // "Console" WriteTo in appsettings.json), so a stray structured log + // line would land in this SAME captured stream, not go missing — this + // is precisely how a regression here would be observable. NOT "exactly + // 3 lines": a legitimate operational Serilog line (e.g. the connection- + // string warning RecoverAdminCommandTests hits in Production) can + // legally appear here too, so the invariant is narrower and precise — + // the password appears EXACTLY ONCE in the whole capture (the one + // explicit Console.Out line), and never inside a Serilog line (which + // always opens with its outputTemplate's "[HH:mm:ss LVL]" bracket). + var tempPassword = ExtractTemporaryPassword(stdout); + Assert.Equal(1, CountOccurrences(stdout, tempPassword)); + Assert.DoesNotContain( + stdout.Split('\n', StringSplitOptions.RemoveEmptyEntries), + line => line.TrimStart().StartsWith('[') && line.Contains(tempPassword)); + } + finally { - var defaultAccountSlug = await assertScope.ServiceProvider - .GetRequiredService() - .Accounts.IgnoreQueryFilters() - .Where(a => a.Id == SeedDefaults.AccountId) - .Select(a => a.Slug) - .SingleAsync(); - Assert.Contains($"on farm {defaultAccountSlug} ", stdout); + await SetDefaultAccountSlugAsync(originalSlug); } - // The password itself never reaches stderr (nothing does on success). - Assert.Equal(string.Empty, stderr); - // #273 — the actual "never the logger" guarantee, asserted directly: - // Serilog's own Console sink ALSO writes to this process's stdout (the - // "Console" WriteTo in appsettings.json), so a stray structured log - // line would land in this SAME captured stream, not go missing — this - // is precisely how a regression here would be observable. NOT "exactly - // 3 lines": a legitimate operational Serilog line (e.g. the connection- - // string warning RecoverAdminCommandTests hits in Production) can - // legally appear here too, so the invariant is narrower and precise — - // the password appears EXACTLY ONCE in the whole capture (the one - // explicit Console.Out line), and never inside a Serilog line (which - // always opens with its outputTemplate's "[HH:mm:ss LVL]" bracket). - var tempPassword = ExtractTemporaryPassword(stdout); - Assert.Equal(1, CountOccurrences(stdout, tempPassword)); - Assert.DoesNotContain( - stdout.Split('\n', StringSplitOptions.RemoveEmptyEntries), - line => line.TrimStart().StartsWith('[') && line.Contains(tempPassword)); using var scope = _factory.Services.CreateScope(); var users = scope.ServiceProvider.GetRequiredService>(); @@ -151,17 +157,32 @@ public async Task Rerun_WhenAlreadyProvisioned_DoesNotPrintTheFarmCode() // Reuses the idempotent-rerun setup: first run provisions the Owner, // second run is a no-op where WasAlreadyProvisioned is true. var email = $"bootstrap-farmcode-{Guid.NewGuid():N}@test.local"; - var first = await RunBootstrapCommandAsync(email); - Assert.True(0 == first.ExitCode, $"first run: expected exit 0, got {first.ExitCode}. stderr={first.Stderr}"); + // #589 (codex P2) — same varied-slug discipline as the first-run test: + // the re-run bans the account's ACTUAL (varied) slug value, not one + // phrasing of an "on farm" line and not the seed constant — a broken + // re-run that emitted "Farm code: " under any label must red. + // Restored in `finally` because the tests share `_factory`'s database. + var variedSlug = "varied-farm-x7q2"; + var originalSlug = await OverrideDefaultAccountSlugAsync(variedSlug); + try + { + var first = await RunBootstrapCommandAsync(email); + Assert.True(0 == first.ExitCode, $"first run: expected exit 0, got {first.ExitCode}. stderr={first.Stderr}"); - var second = await RunBootstrapCommandAsync(email); - Assert.True(0 == second.ExitCode, $"rerun: expected exit 0, got {second.ExitCode}. stderr={second.Stderr}"); - Assert.Contains("already provisioned", second.Stdout, StringComparison.OrdinalIgnoreCase); - // #589 — the idempotent path must stay silent about the farm code: no - // provisioning happened, so no "on farm" line may appear. This is the - // mutant that keeps the AlreadyProvisioned() branch honest — if that - // branch ever started populating Slug and printing it, this reds. - Assert.DoesNotContain(" on farm ", second.Stdout, StringComparison.Ordinal); + var second = await RunBootstrapCommandAsync(email); + Assert.True(0 == second.ExitCode, $"rerun: expected exit 0, got {second.ExitCode}. stderr={second.Stderr}"); + Assert.Contains("already provisioned", second.Stdout, StringComparison.OrdinalIgnoreCase); + // #589 — the idempotent path must stay silent about the farm code: + // no provisioning happened, so the slug value may not appear at all, + // whatever the phrasing. Bans the VARIED value itself, so a mutant + // that appends the slug to the already-provisioned message reds. + // This is the mutant that keeps the AlreadyProvisioned() branch honest. + Assert.DoesNotContain(variedSlug, second.Stdout, StringComparison.Ordinal); + } + finally + { + await SetDefaultAccountSlugAsync(originalSlug); + } } [Fact] @@ -260,6 +281,33 @@ public async Task ConcurrentInvocations_OnlyOneCreatesAnOwner_TheOtherIsACleanNo Assert.Contains(owner.Email, new[] { emailA, emailB }); } + // #589 — set the default account's Slug to a value that is NOT the + // migration-seeded "default-farm", and restore it. Raw SQL (not EF + // SaveChanges) is deliberate: Slug is immutable in the domain (#531, + // "no ChangeSlug"), and a raw UPDATE bypasses both the domain guard and any + // interceptor — exactly what a test that fabricates a distinct farm code + // needs. `Override` returns the original so a `finally` can restore it. + private async Task OverrideDefaultAccountSlugAsync(string newSlug) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var original = await db.Accounts.IgnoreQueryFilters() + .Where(a => a.Id == SeedDefaults.AccountId) + .Select(a => a.Slug) + .SingleAsync(); + await db.Database.ExecuteSqlInterpolatedAsync( + $"UPDATE \"Accounts\" SET \"Slug\" = {newSlug} WHERE \"Id\" = {SeedDefaults.AccountId}"); + return original; + } + + private async Task SetDefaultAccountSlugAsync(string slug) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + await db.Database.ExecuteSqlInterpolatedAsync( + $"UPDATE \"Accounts\" SET \"Slug\" = {slug} WHERE \"Id\" = {SeedDefaults.AccountId}"); + } + private static string ExtractTemporaryPassword(string stdout) { const string marker = "Temporary password:"; diff --git a/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs index ed329a19e..22f3e85e1 100644 --- a/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs @@ -5,6 +5,8 @@ namespace Cluckwork.Api.IntegrationTests; using System.Diagnostics; using Cluckwork.Api.IntegrationTests.Infrastructure; using Cluckwork.Application.Common; +using Cluckwork.Infrastructure.Persistence; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; // #265 — the `recover-admin` break-glass command is a real CLI dispatch branch @@ -95,6 +97,31 @@ public async Task RecoverAdmin_InProduction_ResetsPassword_PrintsWorkingTemporar "the old seeded password must be rejected after recovery"); } + // #589 — recover-admin must print the farm code (a required #532 login + // input) the same way bootstrap-admin does. Same varied-slug discipline as + // the bootstrap tests: the expected value is NOT the migration-seeded + // "default-farm" constant, so a regression that printed a literal would red + // here. recover-admin reads the slug off the account row inside its own + // transaction (AdminRecoveryService, no new query). Restored in `finally` + // because the tests in this class share the fixture's one database. + [Fact] + public async Task RecoverAdmin_PrintsTheFarmCodeFromTheAccountRow() + { + var variedSlug = "varied-farm-x7q2"; + var originalSlug = await OverrideAccountSlugAsync(variedSlug); + try + { + var (exitCode, stdout, stderr) = await RunAsync( + $"--email {_factory.AdminEmail} --reason integration-drill"); + Assert.True(0 == exitCode, $"expected exit 0, got {exitCode}. stdout={stdout} stderr={stderr}"); + Assert.Contains($"on farm {variedSlug} ", stdout); + } + finally + { + await SetAccountSlugAsync(originalSlug); + } + } + // #316 review — the OTLP guard validates during service registration, which // runs BEFORE CliDispatcher. Enforcing it for a one-off verb meant a // plaintext Otlp:Endpoint aborted recover-admin with an unhandled exception @@ -157,6 +184,31 @@ public async Task RecoverAdmin_UnderALeastPrivilegeDmlOnlyRole_StillRecovers() Assert.Contains("Temporary password:", stdout); } + // #589 — vary the fixture's account Slug (the admin lives on + // SeedDefaults.AccountId) and restore it, same raw-SQL rationale as the + // BootstrapAdminCommandTests helpers. Slug is immutable in the domain, so + // only a raw UPDATE can fabricate a distinct farm code. + private async Task OverrideAccountSlugAsync(string newSlug) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var original = await db.Accounts.IgnoreQueryFilters() + .Where(a => a.Id == SeedDefaults.AccountId) + .Select(a => a.Slug) + .SingleAsync(); + await db.Database.ExecuteSqlInterpolatedAsync( + $"UPDATE \"Accounts\" SET \"Slug\" = {newSlug} WHERE \"Id\" = {SeedDefaults.AccountId}"); + return original; + } + + private async Task SetAccountSlugAsync(string slug) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + await db.Database.ExecuteSqlInterpolatedAsync( + $"UPDATE \"Accounts\" SET \"Slug\" = {slug} WHERE \"Id\" = {SeedDefaults.AccountId}"); + } + private static string ExtractTemporaryPassword(string stdout) { const string marker = "Temporary password:"; From a892f62a36987b222e9ef34dbb2f6044e975a90a Mon Sep 17 00:00:00 2001 From: mforce Date: Mon, 24 Aug 2026 08:07:20 -0700 Subject: [PATCH 4/8] docs(runbooks): point at list-accounts for farm-code recovery, and correct a guard comment (#589) --- docs/runbooks/break-glass-account-recovery.md | 6 ++++-- docs/runbooks/first-admin-provisioning.md | 7 ++++--- .../Identity/FirstRunAdminService.cs | 12 +++++++++--- .../BootstrapAdminCommandTests.cs | 12 ++++++++---- .../RecoverAdminCommandTests.cs | 8 +++++--- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/docs/runbooks/break-glass-account-recovery.md b/docs/runbooks/break-glass-account-recovery.md index 78d5b6409..1c24c97f5 100644 --- a/docs/runbooks/break-glass-account-recovery.md +++ b/docs/runbooks/break-glass-account-recovery.md @@ -92,8 +92,10 @@ Temporary password: <20-char one-time password> Log in with this now and change it immediately (Account → change password). ``` -The **farm code** before the account GUID is the required #532 sign-in input and -is printed nowhere else, so copy it from stdout along with the password. +The **farm code** before the account GUID is the required #532 sign-in input, so +copy it from stdout along with the password. If its output is ever lost, the +read-only `list-accounts` verb prints every farm's code and is the supported way +to recover it. The temporary password is written to **stdout only** (never the logger/OTLP), so it does not persist in structured logs. Capture it from the terminal, hand it to diff --git a/docs/runbooks/first-admin-provisioning.md b/docs/runbooks/first-admin-provisioning.md index e12d6fa6d..2f52d8bde 100644 --- a/docs/runbooks/first-admin-provisioning.md +++ b/docs/runbooks/first-admin-provisioning.md @@ -27,9 +27,10 @@ All three forms below share the same properties: logger or the OTLP pipeline. A host's stdout collector (docker logs, journald, a platform log pipeline) may still capture it, so treat that output as sensitive while the password is valid; -- the printed output also carries the **farm code** (`on farm `) — #532 made - it a required sign-in input, and it is printed nowhere else, so copy it from - stdout along with the password; +- a first-run provisioning also prints the **farm code** (`on farm `) — #532 + made it a required sign-in input, so copy it from stdout along with the password. + A re-run (the no-op bullet below) deliberately does **not** reprint it; to recover + a code whose output was lost, run the read-only `list-accounts` verb. - first sign-in shows a **Set your password** screen and refuses everything else until you pick your own; - a re-run against an already-provisioned account is a safe no-op. diff --git a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs index 3a6dac4be..e74a74096 100644 --- a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs +++ b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs @@ -121,9 +121,15 @@ public async Task> ProvisionAsync( // #589 — select the SLUG instead of testing existence. The operator cannot // sign in without the farm code (#532 made it a required login input), and // this query already runs on this path, so reading the value here costs no - // extra round trip. A null result still means "no such account" and takes - // the same failure path as before, so the guard below is unchanged in - // meaning. Deliberately NOT a "default-farm" literal: that value lives in + // extra round trip. A null result now takes the failure path, so this guard + // also FAILS CLOSED on a row whose slug is missing — not just on a missing + // row. The NULL-slug case only arises on a hand-rolled/partially-restored + // schema, which is precisely this guard's premise: a migrated database marks + // Slug .IsRequired() (AccountConfiguration), so it can never be NULL there. + // On that hand-rolled schema the diagnostic below ("The default account does + // not exist") is slightly imprecise, but failing closed is right — a farm + // with no discoverable code cannot be signed into. Deliberately NOT a + // "default-farm" literal: that value lives in // 20260818235944_AddAccountSlug.cs and a second copy would drift. var accountSlug = await db.Accounts .IgnoreQueryFilters() diff --git a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs index 7d550bcce..77d3d3402 100644 --- a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs @@ -105,7 +105,9 @@ public async Task FirstRun_CreatesOwner_PrintsPasswordOnlyToStdout_AndSetsMustCh } finally { - await SetDefaultAccountSlugAsync(originalSlug); + // Restore is asserted (rows affected == 1) so the isolation the + // finally provides is actually verified, not just claimed. + Assert.Equal(1, await SetDefaultAccountSlugAsync(originalSlug)); } using var scope = _factory.Services.CreateScope(); @@ -181,7 +183,9 @@ public async Task Rerun_WhenAlreadyProvisioned_DoesNotPrintTheFarmCode() } finally { - await SetDefaultAccountSlugAsync(originalSlug); + // Restore is asserted (rows affected == 1) so the isolation the + // finally provides is actually verified, not just claimed. + Assert.Equal(1, await SetDefaultAccountSlugAsync(originalSlug)); } } @@ -300,11 +304,11 @@ await db.Database.ExecuteSqlInterpolatedAsync( return original; } - private async Task SetDefaultAccountSlugAsync(string slug) + private async Task SetDefaultAccountSlugAsync(string slug) { using var scope = _factory.Services.CreateScope(); var db = scope.ServiceProvider.GetRequiredService(); - await db.Database.ExecuteSqlInterpolatedAsync( + return await db.Database.ExecuteSqlInterpolatedAsync( $"UPDATE \"Accounts\" SET \"Slug\" = {slug} WHERE \"Id\" = {SeedDefaults.AccountId}"); } diff --git a/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs index 22f3e85e1..1d1c716a6 100644 --- a/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/RecoverAdminCommandTests.cs @@ -118,7 +118,9 @@ public async Task RecoverAdmin_PrintsTheFarmCodeFromTheAccountRow() } finally { - await SetAccountSlugAsync(originalSlug); + // Restore is asserted (rows affected == 1) so the isolation the + // finally provides is actually verified, not just claimed. + Assert.Equal(1, await SetAccountSlugAsync(originalSlug)); } } @@ -201,11 +203,11 @@ await db.Database.ExecuteSqlInterpolatedAsync( return original; } - private async Task SetAccountSlugAsync(string slug) + private async Task SetAccountSlugAsync(string slug) { using var scope = _factory.Services.CreateScope(); var db = scope.ServiceProvider.GetRequiredService(); - await db.Database.ExecuteSqlInterpolatedAsync( + return await db.Database.ExecuteSqlInterpolatedAsync( $"UPDATE \"Accounts\" SET \"Slug\" = {slug} WHERE \"Id\" = {SeedDefaults.AccountId}"); } From f9454d23b3d9e107a1b4b795ecd601eeed87f86a Mon Sep 17 00:00:00 2001 From: mforce Date: Mon, 24 Aug 2026 08:33:54 -0700 Subject: [PATCH 5/8] fix(tests): close the re-run farm-code guard gap and drop stale exclusivity claims (#589) --- specs/product/GLOSSARY.md | 6 +++-- .../Cli/BootstrapAdminCliCommand.cs | 9 ++++--- .../Identity/FirstRunAdminService.cs | 25 +++++++++++-------- .../BootstrapAdminCommandTests.cs | 14 +++++++---- web/src/i18n/en.ts | 3 ++- web/src/i18n/es.ts | 12 ++++----- web/src/i18n/tl.ts | 3 ++- 7 files changed, 42 insertions(+), 30 deletions(-) diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index 2d7f5468c..76fe2803e 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -986,7 +986,8 @@ data**: they ship inside the EF migrations themselves (no runtime seeder, no **not**: an operator runs the offline `bootstrap-admin --email
` command once, which generates a random password, creates the Owner with it, and writes the password to **stdout only** — never the application logger or -the OTLP pipeline. A host's stdout collector (docker logs, journald, a +the OTLP pipeline — and prints the **farm code** alongside it, which #532 made +a required sign-in input. A host's stdout collector (docker logs, journald, a platform log pipeline) may still capture it, so that output must be treated as sensitive while the password is valid. Re-running the command against an already-provisioned account is a safe no-op (no second Owner, no password @@ -1041,7 +1042,8 @@ reset, self-service needs the current password, and an Owner cannot reset their own account — so without this the only recourse is direct database surgery. An operator with server access runs the offline `recover-admin` command, which resets the password to a freshly generated temporary one, **clears any lockout**, -rotates the security stamp, and revokes every session. It is audited as +rotates the security stamp, and revokes every session, printing the temporary +password and the farm code to stdout. It is audited as `User.BreakGlassReset` (distinct from `User.PasswordSet`, and recording the reason + the host it ran from) so the reset is conspicuous in the audit log. The temporary password must be changed on first login. diff --git a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs index f8158b4de..44010256f 100644 --- a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs +++ b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs @@ -65,10 +65,11 @@ await Console.Out.WriteLineAsync( // journald, a platform log pipeline) may still capture it — treat // the value as sensitive, same as the recover-admin runbook // instructs (#265 review). - // #589 — the FARM CODE is named explicitly because it is the one field - // the operator must type and has no other way to discover; the account - // GUID is diagnostic only. Without this line the command ends with - // "log in with this now" and they cannot: #532 changed the login + // #589 — the FARM CODE is named explicitly because it is the one field the + // operator must type and this command's output is the fastest source of + // it (the read-only `list-accounts` verb can recover it later if lost); + // the account GUID is diagnostic only. Without this line the command ends + // with "log in with this now" and they cannot: #532 changed the login // contract (farm code became a required input) and this caller was // missed. await Console.Out.WriteLineAsync( diff --git a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs index e74a74096..ac24138b2 100644 --- a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs +++ b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs @@ -121,20 +121,23 @@ public async Task> ProvisionAsync( // #589 — select the SLUG instead of testing existence. The operator cannot // sign in without the farm code (#532 made it a required login input), and // this query already runs on this path, so reading the value here costs no - // extra round trip. A null result now takes the failure path, so this guard - // also FAILS CLOSED on a row whose slug is missing — not just on a missing - // row. The NULL-slug case only arises on a hand-rolled/partially-restored - // schema, which is precisely this guard's premise: a migrated database marks - // Slug .IsRequired() (AccountConfiguration), so it can never be NULL there. - // On that hand-rolled schema the diagnostic below ("The default account does - // not exist") is slightly imprecise, but failing closed is right — a farm - // with no discoverable code cannot be signed into. Deliberately NOT a - // "default-farm" literal: that value lives in - // 20260818235944_AddAccountSlug.cs and a second copy would drift. + // extra round trip. Projecting `(string?)a.Slug` is load-bearing: without it, + // EF's shaper throws `InvalidCastException: Column 'Slug' is null.` before + // SingleOrDefaultAsync returns, so a NULL slug row would bypass this guard and + // surface as a raw cast error from the CLI's catch-all instead. With the cast, + // a NULL slug row returns null and falls into the `if (accountSlug is null)` + // failure below, giving the operator the intended "The default account does + // not exist" message. That NULL-slug case only arises on a + // hand-rolled/partially-restored schema, which is precisely this guard's + // premise (a migrated database marks Slug .IsRequired() in AccountConfiguration, + // so it can never be NULL there). Failing closed is right — a farm with no + // discoverable code cannot be signed into. Deliberately NOT a "default-farm" + // literal: that value lives in 20260818235944_AddAccountSlug.cs and a second + // copy would drift. var accountSlug = await db.Accounts .IgnoreQueryFilters() .Where(a => a.Id == accountId) - .Select(a => a.Slug) + .Select(a => (string?)a.Slug) .SingleOrDefaultAsync(ct); if (accountSlug is null) return Result.Failure(Error.Validation( diff --git a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs index 77d3d3402..ac935c424 100644 --- a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs @@ -174,12 +174,16 @@ public async Task Rerun_WhenAlreadyProvisioned_DoesNotPrintTheFarmCode() var second = await RunBootstrapCommandAsync(email); Assert.True(0 == second.ExitCode, $"rerun: expected exit 0, got {second.ExitCode}. stderr={second.Stderr}"); Assert.Contains("already provisioned", second.Stdout, StringComparison.OrdinalIgnoreCase); - // #589 — the idempotent path must stay silent about the farm code: - // no provisioning happened, so the slug value may not appear at all, - // whatever the phrasing. Bans the VARIED value itself, so a mutant - // that appends the slug to the already-provisioned message reds. - // This is the mutant that keeps the AlreadyProvisioned() branch honest. + // #589 — the idempotent path must stay silent about the farm code: no + // provisioning happened. TWO bans, because the value ban alone cannot + // catch the real mutant. AlreadyProvisioned() returns Slug = null, so a + // re-run branch that prints the slug prints an EMPTY string, which the + // varied-value ban below can never see; the "on farm" phrase ban is what + // catches that one-file mutant, while the value ban catches the stronger + // two-file mutant that also populates the slug. (These two are what keep + // the AlreadyProvisioned() branch honest.) Assert.DoesNotContain(variedSlug, second.Stdout, StringComparison.Ordinal); + Assert.DoesNotContain("on farm", second.Stdout, StringComparison.OrdinalIgnoreCase); } finally { diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index aede86588..7e1828adb 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -2227,7 +2227,8 @@ export const en = { // one-time password. signingInFirstRun: "First sign-in on a brand-new farm. There is no default password — an operator runs a " - + "one-time setup command that prints a temporary one. Sign in with it and you'll immediately land on a " + + "one-time setup command that prints a temporary password and the farm code. Sign in with both and " + + "you'll immediately land on a " + "Set your password screen instead of the normal app; nothing else works until you pick " + "your own password there. This is separate from an ordinary Change password. Until that setup " + "step has been run, trying to sign in tells you so and points you at whoever administers the server, " diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index 712c2e814..ef621c68c 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -1776,12 +1776,12 @@ export const es = { // machine-drafted (#283) — pending native review. signingInFirstRun: "Primer inicio de sesión en una granja nueva. No hay una contraseña predeterminada — " - + "un operador ejecuta un comando de configuración único que imprime una contraseña temporal. Inicie " - + "sesión con ella y llegará de inmediato a una pantalla de Establecer su contraseña en " - + "lugar de la aplicación normal; nada más funciona hasta que elija su propia contraseña allí. Esto es " - + "distinto de un Cambio de contraseña ordinario. Hasta que se realice ese paso de " - + "configuración, al intentar iniciar sesión se le indica esto y se le remite a quien administra el " - + "servidor, en lugar de afirmar que sus datos eran incorrectos.", + + "un operador ejecuta un comando de configuración único que imprime una contraseña temporal y el código " + + "de la granja. Inicie sesión con ambos y llegará de inmediato a una pantalla de Establecer su " + + "contraseña en lugar de la aplicación normal; nada más funciona hasta que elija su propia " + + "contraseña allí. Esto es distinto de un Cambio de contraseña ordinario. Hasta que se realice " + + "ese paso de configuración, al intentar iniciar sesión se le indica esto y se le remite a quien " + + "administra el servidor, en lugar de afirmar que sus datos eran incorrectos.", // #308 (machine-drafted, pending native review) // #308/#356 (machine-drafted, pending native review) signingInStepUp: diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index f821fabd5..6a16f0250 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -1845,7 +1845,8 @@ export const tl = { signingInFirstRun: "Unang pag-sign in sa isang bagong-bagong farm. Walang default na password — " + "nagpapatakbo ang isang operator ng isang beses na setup command na naglilimbag ng pansamantalang " - + "password. Mag-sign in gamit ito at diretso kang mapupunta sa isang screen ng Itakda ang iyong " + + "password at ang code ng bukid. Mag-sign in gamit ang dalawa at diretso kang mapupunta sa isang screen ng " + + "Itakda ang iyong " + "password sa halip na ang normal na app; wala pang gumagana hangga't hindi ka pumipili ng " + "sarili mong password doon. Iba ito sa karaniwang Pagbabago ng password. Hangga't hindi pa " + "naisasagawa ang hakbang na iyon sa setup, sinasabi ito sa iyo kapag sinubukan mong mag-sign in at " From 351d47c36f9af8e4db4d62a875b5fcfc392eec7b Mon Sep 17 00:00:00 2001 From: mforce Date: Mon, 24 Aug 2026 10:16:42 -0700 Subject: [PATCH 6/8] docs(web): tell a user who does not know their farm code who to ask (#589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codex asked for the in-app glossary to document the CLI discovery and recovery sources for the farm code. Half accepted. The gap is real: a signed-out user who does not know their farm code gets no guidance from the glossary entry that defines it. But the suggested fix is wrong for this audience — the in-app glossary is read by SPA users, who have no shell and cannot run `list-accounts` or read `bootstrap-admin` output. Pointing them at CLI verbs would be a new false implication, which is the exact defect class this PR has spent three rounds removing. The sentence added instead points them at the person who can look it up. Mirrored into es and tl with matching meaning. The other half of the finding is rejected with evidence, on the thread: `auditSystemActors` explains what audit-log actor names mean and makes no claim about what those commands print, so nothing in it became false. --- web/src/i18n/en.ts | 3 ++- web/src/i18n/es.ts | 3 ++- web/src/i18n/tl.ts | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 7e1828adb..4c9da0ef5 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -2869,7 +2869,8 @@ export const en = { + "says which one you mean. It is lowercase and it does not change. The sign-in screen " + "remembers the last 10 farms you have signed in with on this device, most recent " + "first, and offers them so you usually do not have to retype the code. A link such " - + "as /login?farm= takes over and fills the code in for you, without showing the list.", + + "as /login?farm= takes over and fills the code in for you, without showing the list. " + + "If you do not know your farm's code, ask your administrator — they can look it up.", glossaryFarmProvisioningTerm: "Farm provisioning", glossaryFarmProvisioningDef: diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index ef621c68c..a54232ee1 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -2447,7 +2447,8 @@ export const es = { + "cambia. La pantalla de inicio de sesión recuerda las últimas 10 granjas con las que " + "ha iniciado sesión en este dispositivo, la más reciente primero, y se las ofrece " + "para que normalmente no tenga que volver a escribir el código. Un enlace como " - + "/login?farm= se impone y rellena el código por usted, sin mostrar la lista.", + + "/login?farm= se impone y rellena el código por usted, sin mostrar la lista. " + + "Si no sabe el código de su granja, pregunte a su administrador: puede consultarlo.", // #533 (machine-drafted, pending native review) glossaryFarmProvisioningTerm: "Aprovisionamiento de granja", diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index 6a16f0250..a75966b17 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -2539,7 +2539,8 @@ export const tl = { + "nagbabago. Naaalala ng screen ng sign in ang huling 10 bukid na naka-sign in ka sa " + "device na ito, pinakabago muna, at inaalok ang mga ito para hindi mo na kailangang " + "i-type ang code. Ang link tulad ng /login?farm= ang mananaig at awtomatikong " - + "pinupuno ang code para sa iyo, nang hindi ipinapakita ang listahan.", + + "pinupuno ang code para sa iyo, nang hindi ipinapakita ang listahan. " + + "Kung hindi mo alam ang code ng bukid mo, tanungin ang iyong administrator: matitingnan nila ito.", // #533 (machine-drafted, pending native review) glossaryFarmProvisioningTerm: "Pag-provision ng bukid", From 050f432af63425a4045a30740af791b80c601d70 Mon Sep 17 00:00:00 2001 From: mforce Date: Mon, 24 Aug 2026 10:44:40 -0700 Subject: [PATCH 7/8] fix(api): drop a cast that changed nothing, and say what the guard actually does (#589) --- .../Cli/BootstrapAdminCliCommand.cs | 10 ++++--- .../Identity/FirstRunAdminService.cs | 29 ++++++++++--------- .../BootstrapAdminCommandTests.cs | 7 +++-- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs index 44010256f..d7f414a99 100644 --- a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs +++ b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs @@ -65,10 +65,12 @@ await Console.Out.WriteLineAsync( // journald, a platform log pipeline) may still capture it — treat // the value as sensitive, same as the recover-admin runbook // instructs (#265 review). - // #589 — the FARM CODE is named explicitly because it is the one field the - // operator must type and this command's output is the fastest source of - // it (the read-only `list-accounts` verb can recover it later if lost); - // the account GUID is diagnostic only. Without this line the command ends + // #589 — the FARM CODE is named explicitly because it is the one of the + // three login inputs the operator has no other source for: the email and + // password are the operator's own, while the farm code is furnished by a + // command. This command's output is the fastest source of it (the + // read-only `list-accounts` verb can recover it later if lost); the + // account GUID is diagnostic only. Without this line the command ends // with "log in with this now" and they cannot: #532 changed the login // contract (farm code became a required input) and this caller was // missed. diff --git a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs index ac24138b2..924bd7568 100644 --- a/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs +++ b/src/Cluckwork.Infrastructure/Identity/FirstRunAdminService.cs @@ -121,23 +121,24 @@ public async Task> ProvisionAsync( // #589 — select the SLUG instead of testing existence. The operator cannot // sign in without the farm code (#532 made it a required login input), and // this query already runs on this path, so reading the value here costs no - // extra round trip. Projecting `(string?)a.Slug` is load-bearing: without it, - // EF's shaper throws `InvalidCastException: Column 'Slug' is null.` before - // SingleOrDefaultAsync returns, so a NULL slug row would bypass this guard and - // surface as a raw cast error from the CLI's catch-all instead. With the cast, - // a NULL slug row returns null and falls into the `if (accountSlug is null)` - // failure below, giving the operator the intended "The default account does - // not exist" message. That NULL-slug case only arises on a - // hand-rolled/partially-restored schema, which is precisely this guard's - // premise (a migrated database marks Slug .IsRequired() in AccountConfiguration, - // so it can never be NULL there). Failing closed is right — a farm with no - // discoverable code cannot be signed into. Deliberately NOT a "default-farm" - // literal: that value lives in 20260818235944_AddAccountSlug.cs and a second - // copy would drift. + // extra round trip. A MISSING ROW returns null and takes the + // Bootstrap.AccountMissing failure path below. A row whose slug is NULL — + // only possible on a hand-rolled or partially-restored schema, since + // AccountConfiguration marks Slug .IsRequired() — throws + // InvalidCastException ("Column 'Slug' is null.") before SingleOrDefaultAsync + // returns; the CLI's catch-all turns that into "Bootstrap failed: …" and exit + // 1. That is still fail-closed, but via the catch-all, NOT via this guard, so + // the operator sees an EF error rather than the friendly message. Recorded + // as a known limitation: an earlier attempt to make the null-slug case reach + // the friendly path by projecting `(string?)a.Slug` changed nothing — the + // cast is an identity conversion whose nullability comes from the model + // (a migrated database never has a NULL slug), not from the C# annotation. + // Deliberately NOT a "default-farm" literal: that value lives in + // 20260818235944_AddAccountSlug.cs and a second copy would drift. var accountSlug = await db.Accounts .IgnoreQueryFilters() .Where(a => a.Id == accountId) - .Select(a => (string?)a.Slug) + .Select(a => a.Slug) .SingleOrDefaultAsync(ct); if (accountSlug is null) return Result.Failure(Error.Validation( diff --git a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs index ac935c424..9c2ff9644 100644 --- a/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/BootstrapAdminCommandTests.cs @@ -156,8 +156,11 @@ public async Task Rerun_AfterFirstRunSucceeded_IsIdempotent_PrintsNoSecret_AndEx [Fact] public async Task Rerun_WhenAlreadyProvisioned_DoesNotPrintTheFarmCode() { - // Reuses the idempotent-rerun setup: first run provisions the Owner, - // second run is a no-op where WasAlreadyProvisioned is true. + // Reuses the idempotent-rerun setup: this class shares `_factory`'s one + // database and xUnit does not order tests, so the first run merely + // guarantees an Owner exists — provisioning it if no earlier test in this + // class already did — and the second run is a no-op where + // WasAlreadyProvisioned is true. var email = $"bootstrap-farmcode-{Guid.NewGuid():N}@test.local"; // #589 (codex P2) — same varied-slug discipline as the first-run test: // the re-run bans the account's ACTUAL (varied) slug value, not one From fcce1041e5451877fb500cb82aade21271d9e57f Mon Sep 17 00:00:00 2001 From: mforce Date: Mon, 24 Aug 2026 11:05:13 -0700 Subject: [PATCH 8/8] docs(api): stop the farm-code comment contradicting itself (#589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codex P3. The comment said `list-accounts` can recover the farm code and then, four lines later, that without this output line the operator "cannot" sign in. Both cannot be true. Rephrased as what it actually is: printing the code here removes a recovery step, it is not the only route. Also notes that the SPA can supply the code from a remembered value or a /login?farm= link once either exists — which on a first run neither does, so this output is still the practical source. This is the third correction to this one comment block. The first two each fixed an overstatement and introduced another. --- src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs index d7f414a99..cf2dda2a4 100644 --- a/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs +++ b/src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs @@ -68,12 +68,14 @@ await Console.Out.WriteLineAsync( // #589 — the FARM CODE is named explicitly because it is the one of the // three login inputs the operator has no other source for: the email and // password are the operator's own, while the farm code is furnished by a - // command. This command's output is the fastest source of it (the - // read-only `list-accounts` verb can recover it later if lost); the - // account GUID is diagnostic only. Without this line the command ends - // with "log in with this now" and they cannot: #532 changed the login - // contract (farm code became a required input) and this caller was - // missed. + // command. Printing it here removes a recovery step rather than being the + // only route — the read-only `list-accounts` verb also prints it, and the + // SPA can supply it from a remembered code or a /login?farm= link once + // either exists (on a first run, neither does). The account GUID is + // diagnostic only. Without this line the command still ends with "log in + // with this now" while withholding one of the three things that takes: + // #532 changed the login contract (farm code became a required input) and + // this caller was missed. await Console.Out.WriteLineAsync( $"First-run admin provisioned: {outcome.Email} on farm {outcome.Slug} " + $"(account {outcome.AccountId}).");