diff --git a/code-reviews/README.md b/code-reviews/README.md index ea303e1..ee2fb99 100644 --- a/code-reviews/README.md +++ b/code-reviews/README.md @@ -30,7 +30,8 @@ code-reviews/ All 19 modules were reviewed at commit `9c60592` (241 findings: 6 Critical, 46 High, 100 Medium, 89 Low). The tables below track what remains **open** as findings are -resolved and re-triaged. +resolved and re-triaged; findings discovered after the baseline are appended to their +module file and counted in **Total**. | Severity | Open findings | |----------|---------------| @@ -61,7 +62,7 @@ resolved and re-triaged. | [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/3/4/4 | 11 | 11 | | [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-16 | `9c60592` | 0/4/4/3 | 11 | 11 | | [SiteRuntime](SiteRuntime/findings.md) | 2026-05-16 | `9c60592` | 0/3/8/5 | 16 | 16 | -| [StoreAndForward](StoreAndForward/findings.md) | 2026-05-16 | `9c60592` | 0/2/4/6 | 12 | 13 | +| [StoreAndForward](StoreAndForward/findings.md) | 2026-05-16 | `9c60592` | 0/2/4/6 | 12 | 14 | | [TemplateEngine](TemplateEngine/findings.md) | 2026-05-16 | `9c60592` | 0/5/5/4 | 14 | 14 | ## Pending Findings diff --git a/code-reviews/StoreAndForward/findings.md b/code-reviews/StoreAndForward/findings.md index 6a412de..2e9c996 100644 --- a/code-reviews/StoreAndForward/findings.md +++ b/code-reviews/StoreAndForward/findings.md @@ -470,3 +470,41 @@ invalid-JSON payloads. **Resolution** _Unresolved._ + +### StoreAndForward-014 — Storage does not create its SQLite database directory + +| | | +|--|--| +| Severity | Medium | +| Category | Error handling & resilience | +| Status | Resolved | +| Location | `src/ScadaLink.StoreAndForward/StoreAndForwardStorage.cs:26` | + +**Found 2026-05-16** while verifying the store-and-forward fixes — this defect was +not part of the original baseline review. + +**Description** + +`StoreAndForwardStorage.InitializeAsync` opened a `SqliteConnection` against the +configured `SqliteDbPath` (default `./data/store-and-forward.db`) without ensuring the +parent directory exists. SQLite creates the database *file* on demand but not its +*directory*, so when `data/` does not already exist the connection fails to open with +`SQLite Error 14: 'unable to open database file'`. Every site-host boot therefore failed +in any environment whose working directory has no `data/` folder — the cause of the six +failing `SiteActorPathTests` (the host's `RegisterSiteActors` aborts at +`StoreAndForwardService.StartAsync`). Production masked it because `data/` is created by +the Docker image / deployment. + +**Recommendation** + +Create the parent directory of a file-backed SQLite database before opening it. + +**Resolution** + +Resolved 2026-05-16. `InitializeAsync` now calls a new `EnsureDatabaseDirectoryExists` +helper that parses the connection string with `SqliteConnectionStringBuilder` and, for a +file-backed database, creates the parent directory if it is missing (in-memory databases +and bare filenames are skipped). Regression test +`InitializeAsync_FileInMissingDirectory_CreatesDirectory` fails against the pre-fix code; +all six `SiteActorPathTests` now pass. Fixed by the commit whose message references +`StoreAndForward-014`. diff --git a/src/ScadaLink.StoreAndForward/StoreAndForwardStorage.cs b/src/ScadaLink.StoreAndForward/StoreAndForwardStorage.cs index 0749cc1..89e4ec1 100644 --- a/src/ScadaLink.StoreAndForward/StoreAndForwardStorage.cs +++ b/src/ScadaLink.StoreAndForward/StoreAndForwardStorage.cs @@ -25,6 +25,8 @@ public class StoreAndForwardStorage /// public async Task InitializeAsync() { + EnsureDatabaseDirectoryExists(); + await using var connection = new SqliteConnection(_connectionString); await connection.OpenAsync(); @@ -53,6 +55,32 @@ public class StoreAndForwardStorage _logger.LogInformation("Store-and-forward SQLite storage initialized"); } + /// + /// Ensures the directory for a file-backed SQLite database exists. SQLite creates + /// the database file on demand but not its parent directory, so a configured path + /// such as "./data/store-and-forward.db" fails to open ("unable to open database + /// file") when the "data" directory does not yet exist. In-memory databases and + /// bare filenames in the working directory have no directory to create and are + /// skipped. + /// + private void EnsureDatabaseDirectoryExists() + { + var builder = new SqliteConnectionStringBuilder(_connectionString); + if (builder.Mode == SqliteOpenMode.Memory) + return; + + var dataSource = builder.DataSource; + if (string.IsNullOrEmpty(dataSource) || dataSource == ":memory:") + return; + + var directory = System.IO.Path.GetDirectoryName(System.IO.Path.GetFullPath(dataSource)); + if (!string.IsNullOrEmpty(directory) && !System.IO.Directory.Exists(directory)) + { + System.IO.Directory.CreateDirectory(directory); + _logger.LogInformation("Created store-and-forward database directory: {Directory}", directory); + } + } + /// /// WP-9: Enqueues a new message with Pending status. /// diff --git a/tests/ScadaLink.StoreAndForward.Tests/StoreAndForwardStorageTests.cs b/tests/ScadaLink.StoreAndForward.Tests/StoreAndForwardStorageTests.cs index e30c25e..ac7ef80 100644 --- a/tests/ScadaLink.StoreAndForward.Tests/StoreAndForwardStorageTests.cs +++ b/tests/ScadaLink.StoreAndForward.Tests/StoreAndForwardStorageTests.cs @@ -221,4 +221,31 @@ public class StoreAndForwardStorageTests : IAsyncLifetime, IDisposable Status = StoreAndForwardMessageStatus.Pending }; } + + [Fact] + public async Task InitializeAsync_FileInMissingDirectory_CreatesDirectory() + { + // SQLite creates the database file on demand but not its parent directory; + // the storage must create the directory itself or OpenAsync fails with + // "unable to open database file" (the cause of the SiteActorPathTests failures). + var directory = Path.Combine(Path.GetTempPath(), "sf-storage-test-" + Guid.NewGuid().ToString("N")); + var dbPath = Path.Combine(directory, "store-and-forward.db"); + Assert.False(Directory.Exists(directory)); + + try + { + var storage = new StoreAndForwardStorage( + $"Data Source={dbPath}", NullLogger.Instance); + + await storage.InitializeAsync(); + + Assert.True(Directory.Exists(directory)); + Assert.True(File.Exists(dbPath)); + } + finally + { + if (Directory.Exists(directory)) + Directory.Delete(directory, recursive: true); + } + } }