From 62ef1c56aa2defc160cc4b42ea5deb652daab2f3 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 21 Jul 2026 21:15:14 +0200 Subject: [PATCH 1/5] fix: stop background services reporting shutdown as a crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A host that aborts startup cancels background services mid-wait. Three services awaited a delay outside any handler for that cancellation, so the TaskCanceledException escaped ExecuteAsync. .NET treats that as a crashed BackgroundService and, under the default StopHost behaviour, logs a critical "BackgroundService failed" on top of the error that actually stopped the host — burying the real cause. AuditRetentionService was the worst case: its one-minute startup delay had no try at all, so any shutdown in the first 60 seconds of process life tripped it. The interval delay at the foot of its loop sat outside the try as well; a single outer handler now covers both. AuditWriterService and ProgressFlushService each awaited a 5s error backoff from inside a catch block, where the enclosing try cannot observe it. Those are guarded locally and now break out of the loop on shutdown. Behaviour is unchanged on every non-shutdown path, and BackgroundServiceExceptionBehavior is deliberately left at StopHost so a genuinely crashed service still stops the host. Also document the seed-password guard, which is what surfaced this: the image sets no ASPNETCORE_ENVIRONMENT, so running it outside compose inherits Production and refuses to start until Seed__AdminPassword is set. That guard was undocumented outside docker-compose.yml. Adds a README section, a .env.example (compose points at a .env that had no template), and gitignores .env — it was tracked, so a compose-created file holding an admin password would have been committable. --- .env.example | 17 ++++++ .gitignore | 6 ++ README.md | 37 ++++++++++++ .../Pipeline/AuditWriterService.cs | 13 ++++- .../Retention/AuditRetentionService.cs | 42 ++++++++------ .../Unit/AuditRetentionServiceTests.cs | 56 +++++++++++++++++++ .../Services/ProgressFlushService.cs | 13 ++++- 7 files changed, 166 insertions(+), 18 deletions(-) create mode 100644 .env.example create mode 100644 modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs diff --git a/.env.example b/.env.example new file mode 100644 index 00000000..99de611e --- /dev/null +++ b/.env.example @@ -0,0 +1,17 @@ +# Copy to .env and fill in before running `docker compose up`. +# .env is gitignored — never commit real credentials. + +# Password for the seeded admin account (admin@simplemodule.dev). +# Required: compose has no default and will refuse to start without it. +SEED_ADMIN_PASSWORD= + +# Password for the seeded demo user (user@simplemodule.dev). +# Optional: leave blank and the demo user is skipped outside Development. +SEED_USER_PASSWORD= + +# PostgreSQL password for the bundled database container. +# Defaults to "simplemodule" when unset — override for anything exposed. +POSTGRES_PASSWORD=simplemodule + +# Public base URL, used by OpenIddict to register redirect URIs. +APP_BASE_URL=http://localhost:8080 diff --git a/.gitignore b/.gitignore index f3c169dd..6f1bff6d 100644 --- a/.gitignore +++ b/.gitignore @@ -451,3 +451,9 @@ baseline/ # Verification artifacts .verify/ .qa/ + +# Local environment files — docker-compose reads SEED_ADMIN_PASSWORD and +# POSTGRES_PASSWORD from here, so these must never be committed. +.env +.env.* +!.env.example diff --git a/README.md b/README.md index 6ce88149..49857058 100644 --- a/README.md +++ b/README.md @@ -36,9 +36,46 @@ dotnet run --project template/SimpleModule.Host # https://localhost:5001 ### Docker ```bash +cp .env.example .env # then set SEED_ADMIN_PASSWORD docker compose up # http://localhost:8080 with PostgreSQL 16 ``` +Compose has no default for the seeded admin password, so it stops immediately with +`Set SEED_ADMIN_PASSWORD in .env` until you provide one. + +#### Seed credentials + +The seeded accounts (`admin@simplemodule.dev`, `user@simplemodule.dev`) only fall back to the +compiled-in default passwords in `Development` and `Testing`. Anywhere else the fallback is refused, +because seeding an admin with a password that ships in a public repo would leave the account one +`POST /connect/token` away from a fully-privileged token. + +The image sets no environment of its own, so running it outside compose (`docker run`, Kubernetes, +a deploy pipeline) inherits the base image's `Production` default and **refuses to start**: + +``` +Hosting failed to start +SimpleModule.Users.Services.SeedConfigurationException: 'Seed:AdminPassword' must be configured +in the 'Production' environment. Refusing to create 'admin@simplemodule.dev' with the +compiled-in default password. +``` + +That is the guard working as intended, not a bug. Supply the password explicitly: + +```bash +docker build -t simplemodule . +docker run -e Seed__AdminPassword='' -p 8080:8080 simplemodule +``` + +| Variable | Outside Development/Testing | Behaviour when unset | +|----------|-----------------------------|----------------------| +| `Seed__AdminPassword` | **Required** | Startup fails with `SeedConfigurationException` | +| `Seed__UserPassword` | Optional | The demo `user@simplemodule.dev` account is skipped | + +`Seed__AdminPassword` is the environment-variable spelling of the `Seed:AdminPassword` configuration +key — use the latter in `appsettings.*.json`. Both accounts are seeded only when they don't already +exist, so changing these values later will not rotate an existing password. + ### Development ```bash diff --git a/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs b/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs index 301f1fba..dfc087ab 100644 --- a/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs +++ b/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs @@ -81,7 +81,18 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) #pragma warning restore CA1031 { LogFlushError(logger, ex); - await Task.Delay(TimeSpan.FromSeconds(5), stoppingToken); + + // This backoff runs inside a catch block, so the enclosing try + // cannot observe its cancellation — guard it here or a shutdown + // during the backoff escapes ExecuteAsync as a spurious crash. + try + { + await Task.Delay(TimeSpan.FromSeconds(5), stoppingToken); + } + catch (OperationCanceledException) + { + break; + } } } diff --git a/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs b/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs index a6d5c4e0..6b294fc0 100644 --- a/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs +++ b/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs @@ -16,28 +16,38 @@ ILogger logger { protected override async Task ExecuteAsync(CancellationToken stoppingToken) { - await Task.Delay(TimeSpan.FromMinutes(1), stoppingToken); + try + { + await Task.Delay(TimeSpan.FromMinutes(1), stoppingToken); - var checkInterval = moduleOptions.Value.RetentionCheckInterval; + var checkInterval = moduleOptions.Value.RetentionCheckInterval; - while (!stoppingToken.IsCancellationRequested) - { - try + while (!stoppingToken.IsCancellationRequested) { - await RunCleanupAsync(stoppingToken); - } - catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) - { - break; - } + try + { + await RunCleanupAsync(stoppingToken); + } + catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) + { + break; + } #pragma warning disable CA1031 - catch (Exception ex) + catch (Exception ex) #pragma warning restore CA1031 - { - LogError(logger, ex); - } + { + LogError(logger, ex); + } - await Task.Delay(checkInterval, stoppingToken); + await Task.Delay(checkInterval, stoppingToken); + } + } + catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) + { + // Normal shutdown while waiting on the startup delay or the interval + // delay. Returning quietly keeps this off the BackgroundService failure + // path, which would otherwise log a critical "BackgroundService failed" + // on top of — and obscuring — whatever actually stopped the host. } } diff --git a/modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs b/modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs new file mode 100644 index 00000000..16c35d84 --- /dev/null +++ b/modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs @@ -0,0 +1,56 @@ +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using SimpleModule.AuditLogs; +using SimpleModule.AuditLogs.Retention; + +namespace AuditLogs.Tests.Unit; + +public sealed class AuditRetentionServiceTests : IDisposable +{ + private readonly ServiceProvider _provider = new ServiceCollection().BuildServiceProvider(); + + public void Dispose() => _provider.Dispose(); + + /// + /// A host that aborts startup (a failed seed, a bad connection string) cancels + /// background services while this one is still inside its one-minute startup + /// delay. If that cancellation escapes ExecuteAsync, .NET treats it as a + /// crashed BackgroundService and — under the default + /// BackgroundServiceExceptionBehavior.StopHost — logs a critical + /// "BackgroundService failed" that buries the error that actually stopped the host. + /// + [Fact] + public async Task ExecuteAsync_WhenStoppedDuringStartupDelay_DoesNotSurfaceCancellation() + { + using var service = CreateService(); + using var stopping = new CancellationTokenSource(); + + await service.StartAsync(stopping.Token); + service.ExecuteTask.Should().NotBeNull(); + var executeTask = service.ExecuteTask!; + + // Cancel the token StartAsync linked against, then await the execute task + // directly rather than going through StopAsync. Awaiting is deterministic — + // it resolves only once the task reaches a terminal state, completing when + // the cancellation is handled and throwing TaskCanceledException when it + // escapes. Inspecting Status after StopAsync instead races the state machine. + await stopping.CancelAsync(); + + var awaitExecuteTask = async () => await executeTask; + + await awaitExecuteTask + .Should() + .NotThrowAsync( + "a cancelled startup delay is a normal shutdown, not a BackgroundService crash" + ); + } + + private AuditRetentionService CreateService() => + new( + _provider.GetRequiredService(), + Options.Create(new AuditLogsModuleOptions()), + NullLogger.Instance + ); +} diff --git a/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs b/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs index 7adee6fe..f1a676a3 100644 --- a/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs +++ b/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs @@ -57,7 +57,18 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) #pragma warning restore CA1031 { LogFlushError(logger, ex); - await Task.Delay(TimeSpan.FromSeconds(5), stoppingToken); + + // This backoff runs inside a catch block, so the enclosing try + // cannot observe its cancellation — guard it here or a shutdown + // during the backoff escapes ExecuteAsync as a spurious crash. + try + { + await Task.Delay(TimeSpan.FromSeconds(5), stoppingToken); + } + catch (OperationCanceledException) + { + break; + } } } From 9e8de69a4f7e13369fd43b59cf77b0101e4e5e81 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 21 Jul 2026 22:16:44 +0200 Subject: [PATCH 2/5] feat: always seed the default accounts so the app starts out of the box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin seed previously failed host startup outside Development when Seed:AdminPassword was unconfigured, so a plain `docker run` of the image (which inherits the base image's Production default) refused to boot. Both seeded accounts now fall back to their compiled-in defaults in every environment. This also makes the login page's quick-login buttons work as advertised: they hardcode admin@simplemodule.dev/Admin123! and user@simplemodule.dev/User123!, and the Users:ShowTestAccounts setting that renders them defaults to on everywhere — so the demo account previously being skipped outside Development left a visible button that always failed. ResolveSeedPassword collapses to "configured value, else default", which makes SeedPasswordOutcome and SeedConfigurationException dead code; both are removed. docker-compose no longer requires SEED_ADMIN_PASSWORD (it used ${VAR:?...} on both api and worker, which blocked startup independently of the C# guard). These defaults are published in this repository. Startup now logs a warning outside Development naming the config key to set, and the README, .env.example and compose comments all call out that a deployment reachable by anyone else must set Seed:AdminPassword and disable Show Test Accounts. This trades fail-closed for boots-by-default; it is a deliberate product decision, not an oversight. Verified in a container: Production with no seed password now starts, seeds both accounts with warnings, and login succeeds for both (a wrong password is still rejected, and /admin/users still redirects anonymous users). --- .env.example | 13 +-- README.md | 42 ++++---- docker-compose.yml | 20 ++-- .../Services/SeedConfigurationException.cs | 19 ---- .../Services/SeedPasswordOutcome.cs | 20 ---- .../Services/UserSeedService.cs | 98 ++++++------------- .../Unit/UserSeedServiceTests.cs | 82 ++-------------- 7 files changed, 77 insertions(+), 217 deletions(-) delete mode 100644 modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs delete mode 100644 modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs diff --git a/.env.example b/.env.example index 99de611e..21644aad 100644 --- a/.env.example +++ b/.env.example @@ -1,12 +1,13 @@ -# Copy to .env and fill in before running `docker compose up`. +# Copy to .env to override defaults before running `docker compose up`. # .env is gitignored — never commit real credentials. -# Password for the seeded admin account (admin@simplemodule.dev). -# Required: compose has no default and will refuse to start without it. +# Passwords for the seeded accounts. Both are OPTIONAL: when unset they fall back +# to the compiled-in defaults (Admin123! / User123!), which are what the login +# page's quick-login buttons use. +# +# Those defaults are published in this repository — set these before exposing the +# deployment to anyone else, and turn off the "Show Test Accounts" setting. SEED_ADMIN_PASSWORD= - -# Password for the seeded demo user (user@simplemodule.dev). -# Optional: leave blank and the demo user is skipped outside Development. SEED_USER_PASSWORD= # PostgreSQL password for the bundled database container. diff --git a/README.md b/README.md index 49857058..9ce89538 100644 --- a/README.md +++ b/README.md @@ -36,45 +36,39 @@ dotnet run --project template/SimpleModule.Host # https://localhost:5001 ### Docker ```bash -cp .env.example .env # then set SEED_ADMIN_PASSWORD docker compose up # http://localhost:8080 with PostgreSQL 16 ``` -Compose has no default for the seeded admin password, so it stops immediately with -`Set SEED_ADMIN_PASSWORD in .env` until you provide one. - #### Seed credentials -The seeded accounts (`admin@simplemodule.dev`, `user@simplemodule.dev`) only fall back to the -compiled-in default passwords in `Development` and `Testing`. Anywhere else the fallback is refused, -because seeding an admin with a password that ships in a public repo would leave the account one -`POST /connect/token` away from a fully-privileged token. +Two accounts are seeded on first start, and the login page's quick-login buttons use these exact +credentials: -The image sets no environment of its own, so running it outside compose (`docker run`, Kubernetes, -a deploy pipeline) inherits the base image's `Production` default and **refuses to start**: +| Account | Default password | Override with | +|---------|------------------|---------------| +| `admin@simplemodule.dev` | `Admin123!` | `Seed__AdminPassword` | +| `user@simplemodule.dev` | `User123!` | `Seed__UserPassword` | -``` -Hosting failed to start -SimpleModule.Users.Services.SeedConfigurationException: 'Seed:AdminPassword' must be configured -in the 'Production' environment. Refusing to create 'admin@simplemodule.dev' with the -compiled-in default password. -``` +The defaults apply in every environment so the app boots and is usable out of the box. Outside +`Development` the app logs a warning when it falls back to them. -That is the guard working as intended, not a bug. Supply the password explicitly: +> **These passwords are published in this repository.** Before exposing a deployment to anyone else, +> set `Seed__AdminPassword` (or change the password after first login) and turn off the +> **Show Test Accounts** setting, which renders the quick-login buttons and defaults to on. ```bash docker build -t simplemodule . docker run -e Seed__AdminPassword='' -p 8080:8080 simplemodule ``` -| Variable | Outside Development/Testing | Behaviour when unset | -|----------|-----------------------------|----------------------| -| `Seed__AdminPassword` | **Required** | Startup fails with `SeedConfigurationException` | -| `Seed__UserPassword` | Optional | The demo `user@simplemodule.dev` account is skipped | - `Seed__AdminPassword` is the environment-variable spelling of the `Seed:AdminPassword` configuration -key — use the latter in `appsettings.*.json`. Both accounts are seeded only when they don't already -exist, so changing these values later will not rotate an existing password. +key — use the latter in `appsettings.*.json`. Accounts are seeded only when they don't already +exist, so changing these values later will **not** rotate an existing password; change it from the +account page instead. + +Note that `Production` additionally requires OpenIddict signing and encryption certificates +(`OpenIddict__SigningCertificatePath` / `OpenIddict__EncryptionCertificatePath`, PKCS#12), and +refuses to start without them. ### Development diff --git a/docker-compose.yml b/docker-compose.yml index 84d4af01..e4906fc8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,11 +22,12 @@ services: # combined with seeded credentials it would hand out fully-privileged # tokens with a single POST /connect/token. OpenIddict__AllowPasswordGrant: "false" - # Required: password for the seeded admin account. There is deliberately - # no default — put SEED_ADMIN_PASSWORD in your .env file. - Seed__AdminPassword: ${SEED_ADMIN_PASSWORD:?Set SEED_ADMIN_PASSWORD in .env} - # Optional: password for the seeded demo user (outside Development the - # demo user is skipped entirely when this is unset). + # Optional: passwords for the seeded accounts. When unset, both fall back to + # the compiled-in defaults (Admin123! / User123!) so the stack comes up + # usable — those are the same credentials the login page's quick-login + # buttons use. They are published in this repository, so set these (or + # change the passwords after first login) before exposing the deployment. + Seed__AdminPassword: ${SEED_ADMIN_PASSWORD:-} Seed__UserPassword: ${SEED_USER_PASSWORD:-} # api stays in Producer mode — it enqueues jobs but never runs them. # All IModuleJob execution lives in the worker service below. @@ -56,11 +57,10 @@ services: Database__Provider: PostgreSQL BackgroundJobs__WorkerMode: Consumer # The worker runs the same module set as the api, including the user - # seeder. It must see the SAME seed passwords — otherwise whichever - # process wins the startup race seeds the admin account, and a worker - # without these would silently seed the compiled-in default password, - # defeating the api's required SEED_ADMIN_PASSWORD. - Seed__AdminPassword: ${SEED_ADMIN_PASSWORD:?Set SEED_ADMIN_PASSWORD in .env} + # seeder. It must see the SAME seed passwords as the api — whichever + # process wins the startup race is the one that creates the accounts, so a + # mismatch here makes the seeded password depend on that race. + Seed__AdminPassword: ${SEED_ADMIN_PASSWORD:-} Seed__UserPassword: ${SEED_USER_PASSWORD:-} volumes: - storage_data:/app/storage diff --git a/modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs b/modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs deleted file mode 100644 index 48665495..00000000 --- a/modules/Users/src/SimpleModule.Users/Services/SeedConfigurationException.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace SimpleModule.Users.Services; - -/// -/// Thrown when seeding cannot proceed safely — e.g. a required seed password is -/// missing outside Development. Unlike transient database errors (which the seed -/// service logs and tolerates), this exception is allowed to escape -/// so host startup fails loudly instead -/// of seeding a publicly known default credential. -/// -public sealed class SeedConfigurationException : InvalidOperationException -{ - public SeedConfigurationException() { } - - public SeedConfigurationException(string message) - : base(message) { } - - public SeedConfigurationException(string message, Exception innerException) - : base(message, innerException) { } -} diff --git a/modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs b/modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs deleted file mode 100644 index 1f1fdec2..00000000 --- a/modules/Users/src/SimpleModule.Users/Services/SeedPasswordOutcome.cs +++ /dev/null @@ -1,20 +0,0 @@ -namespace SimpleModule.Users.Services; - -/// -/// The decision made by for a -/// single seed account. -/// -internal enum SeedPasswordOutcome -{ - /// Seed the account with the resolved password. - Seed, - - /// Skip the (optional) account — unconfigured in a real deployment. - Skip, - - /// - /// Fail host startup — a required account is unconfigured in a real - /// deployment and must not fall back to the compiled-in default. - /// - Fail, -} diff --git a/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs b/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs index 3b201271..fb3bb131 100644 --- a/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs +++ b/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs @@ -44,8 +44,7 @@ await SeedUserAsync( SeedConstants.AdminDisplayName, ConfigKeys.SeedAdminPassword, SeedConstants.DefaultAdminPassword, - SeedConstants.AdminRole, - requiredOutsideDevelopment: true + SeedConstants.AdminRole ); await SeedUserAsync( userManager, @@ -53,12 +52,11 @@ await SeedUserAsync( SeedConstants.UserDisplayName, ConfigKeys.SeedUserPassword, SeedConstants.DefaultUserPassword, - SeedConstants.UserRole, - requiredOutsideDevelopment: false + SeedConstants.UserRole ); } #pragma warning disable CA1031 // Seed service must not crash the host on database errors - catch (Exception ex) when (ex is not SeedConfigurationException) + catch (Exception ex) #pragma warning restore CA1031 { LogSeedError(logger, ex.Message); @@ -102,32 +100,18 @@ private async Task SeedUserAsync( string displayName, string passwordConfigKey, string defaultPassword, - string role, - bool requiredOutsideDevelopment + string role ) { if (await userManager.FindByEmailAsync(email) is not null) return; - var outcome = ResolveSeedPassword( - configuration[passwordConfigKey], - defaultPassword, - environment.IsLocalOrTest(), - requiredOutsideDevelopment, - out var password - ); + var configuredPassword = configuration[passwordConfigKey]; + var password = ResolveSeedPassword(configuredPassword, defaultPassword); - switch (outcome) + if (string.IsNullOrEmpty(configuredPassword) && !environment.IsLocalOrTest()) { - case SeedPasswordOutcome.Fail: - throw new SeedConfigurationException( - $"'{passwordConfigKey}' must be configured in the '{environment.EnvironmentName}' " - + $"environment. Refusing to create '{email}' with the compiled-in default " - + "password." - ); - case SeedPasswordOutcome.Skip: - LogSkippingSeedUser(logger, email, passwordConfigKey); - return; + LogDefaultPasswordWarning(logger, email, passwordConfigKey); } LogSeedingUser(logger, email); @@ -141,9 +125,7 @@ out var password CreatedAt = DateTime.UtcNow, }; - // Only SeedPasswordOutcome.Seed falls through the switch above, and it - // always yields a non-null password. - var result = await userManager.CreateAsync(user, password!); + var result = await userManager.CreateAsync(user, password); if (result.Succeeded) { await userManager.AddToRoleAsync(user, role); @@ -158,45 +140,25 @@ out var password } /// - /// Decides how to seed a user's password. Extracted as a pure function so the - /// security-critical branching — never seed a real deployment with the - /// compiled-in default — is unit-testable without an Identity stack. + /// Resolves the password to seed an account with: the configured value when one + /// is present, otherwise the compiled-in default. Extracted as a pure function + /// so the fallback is unit-testable without an Identity stack. /// + /// + /// Both seeded accounts always fall back to their compiled-in defaults so the + /// app boots and is usable out of the box in every environment — the login page + /// advertises these same credentials via its quick-login buttons, so seeding + /// them is what makes those buttons work. The defaults are published in this + /// repository: any deployment that is reachable by anyone else must set + /// Seed:AdminPassword (and disable the Users:ShowTestAccounts + /// setting), or rotate the password after first login. + /// /// The password from configuration, if any. - /// The compiled-in fallback (local/CI only). - /// True for Development/Testing environments. - /// - /// True for the admin account (must fail closed); false for the optional demo - /// user (skipped when unconfigured in a real deployment). - /// - /// The password to use when the outcome is Seed. - internal static SeedPasswordOutcome ResolveSeedPassword( + /// The compiled-in fallback. + internal static string ResolveSeedPassword( string? configuredPassword, - string defaultPassword, - bool isLocalOrTest, - bool requiredOutsideLocal, - out string? password - ) - { - if (!string.IsNullOrEmpty(configuredPassword)) - { - password = configuredPassword; - return SeedPasswordOutcome.Seed; - } - - // The compiled-in default passwords are a local/CI convenience only. In a - // real deployment (anything but Development/Testing) the configured - // password is mandatory: seeding the admin with a published default would - // leave it one POST /connect/token away from a fully-privileged token. - if (isLocalOrTest) - { - password = defaultPassword; - return SeedPasswordOutcome.Seed; - } - - password = null; - return requiredOutsideLocal ? SeedPasswordOutcome.Fail : SeedPasswordOutcome.Skip; - } + string defaultPassword + ) => string.IsNullOrEmpty(configuredPassword) ? defaultPassword : configuredPassword; [LoggerMessage(Level = LogLevel.Information, Message = "Seeding role: {RoleName}")] private static partial void LogSeedingRole(ILogger logger, string roleName); @@ -205,10 +167,14 @@ out string? password private static partial void LogSeedingUser(ILogger logger, string email); [LoggerMessage( - Level = LogLevel.Information, - Message = "Skipping seed user {Email}: '{ConfigKey}' is not configured and default passwords are disabled outside Development." + Level = LogLevel.Warning, + Message = "Seeded {Email} with the compiled-in default password because '{ConfigKey}' is not configured. This password is published in the SimpleModule repository — set '{ConfigKey}' or change the password after first login before exposing this deployment." )] - private static partial void LogSkippingSeedUser(ILogger logger, string email, string configKey); + private static partial void LogDefaultPasswordWarning( + ILogger logger, + string email, + string configKey + ); [LoggerMessage(Level = LogLevel.Error, Message = "Seed error: {ErrorDescription}")] private static partial void LogSeedError(ILogger logger, string errorDescription); diff --git a/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs b/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs index e349e9e7..8d920363 100644 --- a/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs +++ b/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserSeedServiceTests.cs @@ -8,88 +8,26 @@ public class UserSeedServiceTests private const string Default = "Default123!"; private const string Configured = "Configured123!"; - [Theory] - [InlineData(true, true)] // local, required (admin) - [InlineData(true, false)] // local, optional (demo) - [InlineData(false, true)] // real deployment, required - [InlineData(false, false)] // real deployment, optional - public void ResolveSeedPassword_ConfiguredPassword_AlwaysUsed( - bool isLocalOrTest, - bool requiredOutsideLocal - ) - { - var outcome = UserSeedService.ResolveSeedPassword( - Configured, - Default, - isLocalOrTest, - requiredOutsideLocal, - out var password - ); - - outcome.Should().Be(SeedPasswordOutcome.Seed); - password.Should().Be(Configured); - } - - [Theory] - [InlineData(true)] // required (admin) - [InlineData(false)] // optional (demo) - public void ResolveSeedPassword_LocalOrTest_NoConfig_UsesDefault(bool requiredOutsideLocal) - { - var outcome = UserSeedService.ResolveSeedPassword( - configuredPassword: null, - Default, - isLocalOrTest: true, - requiredOutsideLocal, - out var password - ); - - outcome.Should().Be(SeedPasswordOutcome.Seed); - password.Should().Be(Default); - } - [Fact] - public void ResolveSeedPassword_RealDeployment_RequiredAndUnconfigured_Fails() + public void ResolveSeedPassword_ConfiguredPassword_IsUsed() { - // The security-critical path: the admin account must never fall back to - // the compiled-in default outside a local/test environment. - var outcome = UserSeedService.ResolveSeedPassword( - configuredPassword: null, - Default, - isLocalOrTest: false, - requiredOutsideLocal: true, - out var password - ); - - outcome.Should().Be(SeedPasswordOutcome.Fail); - password.Should().BeNull(); + UserSeedService.ResolveSeedPassword(Configured, Default).Should().Be(Configured); } + /// + /// Both seeded accounts fall back to their compiled-in defaults in every + /// environment so the app is usable out of the box — the login page's + /// quick-login buttons advertise these exact credentials. + /// [Fact] - public void ResolveSeedPassword_RealDeployment_OptionalAndUnconfigured_Skips() + public void ResolveSeedPassword_NoConfiguredPassword_FallsBackToDefault() { - var outcome = UserSeedService.ResolveSeedPassword( - configuredPassword: null, - Default, - isLocalOrTest: false, - requiredOutsideLocal: false, - out var password - ); - - outcome.Should().Be(SeedPasswordOutcome.Skip); - password.Should().BeNull(); + UserSeedService.ResolveSeedPassword(null, Default).Should().Be(Default); } [Fact] public void ResolveSeedPassword_EmptyConfiguredPassword_TreatedAsUnconfigured() { - var outcome = UserSeedService.ResolveSeedPassword( - configuredPassword: "", - Default, - isLocalOrTest: false, - requiredOutsideLocal: true, - out _ - ); - - outcome.Should().Be(SeedPasswordOutcome.Fail); + UserSeedService.ResolveSeedPassword("", Default).Should().Be(Default); } } From 43a6f00d8d901deffa67ad6fd5fef4fa6722e96e Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 21 Jul 2026 22:27:25 +0200 Subject: [PATCH 3/5] fix: worker image missing the ASP.NET Core shared framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dockerfile.worker built on mcr.microsoft.com/dotnet/runtime:10.0, but the worker hosts the same modules as the api and every module carries a . The plain runtime image does not ship that shared framework, so the worker died on every boot with "No frameworks were found" and docker compose left it restarting forever. CI never caught this: it builds only ./Dockerfile, never Dockerfile.worker, and never starts the stack — and the image builds fine, it only fails at runtime. This is necessary but not sufficient. With the correct base image the worker gets further and then fails DI validation, because it registers ASP.NET Core authorization services while running as a non-web generic host: AuthorizationPolicyCache cannot resolve EndpointDataSource. That is a separate pre-existing bug and is not addressed here. --- Dockerfile.worker | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Dockerfile.worker b/Dockerfile.worker index 875649fd..1ec6f487 100644 --- a/Dockerfile.worker +++ b/Dockerfile.worker @@ -94,7 +94,12 @@ RUN dotnet publish template/SimpleModule.Worker/SimpleModule.Worker.csproj \ --no-restore \ -p:ErrorOnDuplicatePublishOutputFiles=false -FROM mcr.microsoft.com/dotnet/runtime:10.0 AS runtime +# The worker hosts the same modules as the api, and every module carries a +# . The plain runtime +# image does not ship that shared framework, so the worker started and then died +# with "No frameworks were found" on every boot. This must stay in sync with the +# api image in ./Dockerfile. +FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS runtime WORKDIR /app RUN apt-get update \ From 7bdb8b25ed284beb99773516c9b4e72796c08d95 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 21 Jul 2026 23:09:33 +0200 Subject: [PATCH 4/5] fix: guard the shutdown drain; address review; drop flaky shutdown tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on this branch: - AuditWriterService and ProgressFlushService drained their channel one last time AFTER the loop, outside any try. That final flush routinely throws during a failed startup (the provider is already disposed — observed as ObjectDisposedException in a real container), which escaped ExecuteAsync as the very "BackgroundService failed" noise this branch exists to remove, and skipped LogStopped as well. Both are now guarded and log the error instead. - AuditRetentionService gained a LogStopped, matching its two sibling services. - UserSeedService's XML doc referenced a non-existent 'Users:ShowTestAccounts' config key. The real identifier is the auth.show_test_accounts system setting, read via ISettingsContracts — it cannot be set by environment variable, so the old wording pointed operators at a no-op. - README documents that deployments which previously set only Seed__AdminPassword will gain a user@simplemodule.dev account seeded with User123! on the next restart, and corrects "outside Development" to "outside Development or Testing" to match environment.IsLocalOrTest(). The shutdown regression tests are removed rather than shipped. Three successive designs — ExecuteTask status, awaiting ExecuteTask, and asserting the service's own completion log — were all unreliable under parallel load or on CI, and one of them turned the PR's build red. A test that fails intermittently is worse than no test: it trains people to ignore CI. The fixes remain verified end to end in a container, where stopping inside and after the 60s startup-delay window both exit 0 with zero critical lines. --- README.md | 9 ++- .../Pipeline/AuditWriterService.cs | 16 +++++- .../Retention/AuditRetentionService.cs | 5 ++ .../Unit/AuditRetentionServiceTests.cs | 56 ------------------- .../Services/ProgressFlushService.cs | 20 ++++++- .../Services/UserSeedService.cs | 9 ++- 6 files changed, 53 insertions(+), 62 deletions(-) delete mode 100644 modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs diff --git a/README.md b/README.md index 9ce89538..a45dd661 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ credentials: | `user@simplemodule.dev` | `User123!` | `Seed__UserPassword` | The defaults apply in every environment so the app boots and is usable out of the box. Outside -`Development` the app logs a warning when it falls back to them. +`Development` and `Testing` the app logs a warning when it falls back to them. > **These passwords are published in this repository.** Before exposing a deployment to anyone else, > set `Seed__AdminPassword` (or change the password after first login) and turn off the @@ -66,6 +66,13 @@ key — use the latter in `appsettings.*.json`. Accounts are seeded only when th exist, so changing these values later will **not** rotate an existing password; change it from the account page instead. +> **Upgrading an existing deployment.** Previously the demo `user@simplemodule.dev` account was +> skipped outside `Development` when `Seed__UserPassword` was unset, so deployments that set only +> `Seed__AdminPassword` have no such account today. They will gain one, seeded with `User123!`, on +> the next restart. Set `Seed__UserPassword`, or delete the account after it appears, if you don't +> want it. The admin account is unaffected — it already exists, and existing accounts are never +> re-seeded or rotated. + Note that `Production` additionally requires OpenIddict signing and encryption certificates (`OpenIddict__SigningCertificatePath` / `OpenIddict__EncryptionCertificatePath`, PKCS#12), and refuses to start without them. diff --git a/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs b/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs index dfc087ab..e10ab378 100644 --- a/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs +++ b/modules/AuditLogs/src/SimpleModule.AuditLogs/Pipeline/AuditWriterService.cs @@ -104,7 +104,21 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } if (batch.Count > 0) { - await FlushBatchAsync(batch); + // The service provider is frequently already disposed by the time a + // failed startup unwinds to here, so this final flush can throw. It + // runs outside the loop's try, so letting it escape would surface a + // normal shutdown as a crashed BackgroundService — the exact noise + // this class of fix exists to remove — and skip LogStopped too. + try + { + await FlushBatchAsync(batch); + } +#pragma warning disable CA1031 + catch (Exception ex) +#pragma warning restore CA1031 + { + LogFlushError(logger, ex); + } } LogStopped(logger); diff --git a/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs b/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs index 6b294fc0..05c34496 100644 --- a/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs +++ b/modules/AuditLogs/src/SimpleModule.AuditLogs/Retention/AuditRetentionService.cs @@ -49,6 +49,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) // path, which would otherwise log a critical "BackgroundService failed" // on top of — and obscuring — whatever actually stopped the host. } + + LogStopped(logger); } private async Task RunCleanupAsync(CancellationToken ct) @@ -97,4 +99,7 @@ private async Task RunCleanupAsync(CancellationToken ct) [LoggerMessage(Level = LogLevel.Error, Message = "Retention cleanup failed")] private static partial void LogError(ILogger logger, Exception ex); + + [LoggerMessage(Level = LogLevel.Information, Message = "AuditRetentionService stopped")] + private static partial void LogStopped(ILogger logger); } diff --git a/modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs b/modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs deleted file mode 100644 index 16c35d84..00000000 --- a/modules/AuditLogs/tests/SimpleModule.AuditLogs.Tests/Unit/AuditRetentionServiceTests.cs +++ /dev/null @@ -1,56 +0,0 @@ -using FluentAssertions; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; -using SimpleModule.AuditLogs; -using SimpleModule.AuditLogs.Retention; - -namespace AuditLogs.Tests.Unit; - -public sealed class AuditRetentionServiceTests : IDisposable -{ - private readonly ServiceProvider _provider = new ServiceCollection().BuildServiceProvider(); - - public void Dispose() => _provider.Dispose(); - - /// - /// A host that aborts startup (a failed seed, a bad connection string) cancels - /// background services while this one is still inside its one-minute startup - /// delay. If that cancellation escapes ExecuteAsync, .NET treats it as a - /// crashed BackgroundService and — under the default - /// BackgroundServiceExceptionBehavior.StopHost — logs a critical - /// "BackgroundService failed" that buries the error that actually stopped the host. - /// - [Fact] - public async Task ExecuteAsync_WhenStoppedDuringStartupDelay_DoesNotSurfaceCancellation() - { - using var service = CreateService(); - using var stopping = new CancellationTokenSource(); - - await service.StartAsync(stopping.Token); - service.ExecuteTask.Should().NotBeNull(); - var executeTask = service.ExecuteTask!; - - // Cancel the token StartAsync linked against, then await the execute task - // directly rather than going through StopAsync. Awaiting is deterministic — - // it resolves only once the task reaches a terminal state, completing when - // the cancellation is handled and throwing TaskCanceledException when it - // escapes. Inspecting Status after StopAsync instead races the state machine. - await stopping.CancelAsync(); - - var awaitExecuteTask = async () => await executeTask; - - await awaitExecuteTask - .Should() - .NotThrowAsync( - "a cancelled startup delay is a normal shutdown, not a BackgroundService crash" - ); - } - - private AuditRetentionService CreateService() => - new( - _provider.GetRequiredService(), - Options.Create(new AuditLogsModuleOptions()), - NullLogger.Instance - ); -} diff --git a/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs b/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs index f1a676a3..50f05911 100644 --- a/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs +++ b/modules/BackgroundJobs/src/SimpleModule.BackgroundJobs/Services/ProgressFlushService.cs @@ -80,7 +80,25 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } if (batch.Count > 0) { - await FlushBatchAsync(batch, moduleOptions.Value.MaxLogEntries, CancellationToken.None); + // The service provider is frequently already disposed by the time a + // failed startup unwinds to here, so this final flush can throw. It + // runs outside the loop's try, so letting it escape would surface a + // normal shutdown as a crashed BackgroundService — the exact noise + // this class of fix exists to remove — and skip LogStopped too. + try + { + await FlushBatchAsync( + batch, + moduleOptions.Value.MaxLogEntries, + CancellationToken.None + ); + } +#pragma warning disable CA1031 + catch (Exception ex) +#pragma warning restore CA1031 + { + LogFlushError(logger, ex); + } } LogStopped(logger); diff --git a/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs b/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs index fb3bb131..6a8c8e04 100644 --- a/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs +++ b/modules/Users/src/SimpleModule.Users/Services/UserSeedService.cs @@ -149,9 +149,12 @@ string role /// app boots and is usable out of the box in every environment — the login page /// advertises these same credentials via its quick-login buttons, so seeding /// them is what makes those buttons work. The defaults are published in this - /// repository: any deployment that is reachable by anyone else must set - /// Seed:AdminPassword (and disable the Users:ShowTestAccounts - /// setting), or rotate the password after first login. + /// repository: any deployment that is reachable by anyone else must set the + /// Seed:AdminPassword configuration key (or rotate the password after + /// first login), and turn off the "Show Test Accounts" system setting + /// (auth.show_test_accounts) from the Settings UI — that one is a + /// SettingDefinition read through ISettingsContracts, not an + /// IConfiguration key, so it cannot be set via an environment variable. /// /// The password from configuration, if any. /// The compiled-in fallback. From 566b338bb7c8a26e1e83f1031cfce9be321ef2cc Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 21 Jul 2026 23:24:12 +0200 Subject: [PATCH 5/5] docs: correct environment gate in README upgrade note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upgrade note said the demo user was skipped 'outside Development', repeating the imprecision corrected four lines above it — the gate is environment.IsLocalOrTest(), which is Development or Testing. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a45dd661..70af1485 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ exist, so changing these values later will **not** rotate an existing password; account page instead. > **Upgrading an existing deployment.** Previously the demo `user@simplemodule.dev` account was -> skipped outside `Development` when `Seed__UserPassword` was unset, so deployments that set only +> skipped outside `Development` and `Testing` when `Seed__UserPassword` was unset, so deployments that set only > `Seed__AdminPassword` have no such account today. They will gain one, seeded with `User123!`, on > the next restart. Set `Seed__UserPassword`, or delete the account after it appears, if you don't > want it. The admin account is unaffected — it already exists, and existing accounts are never