diff --git a/changelog.d/unreleased/4070.security.md b/changelog.d/unreleased/4070.security.md new file mode 100644 index 000000000..c9f271bd5 --- /dev/null +++ b/changelog.d/unreleased/4070.security.md @@ -0,0 +1,19 @@ +--- +category: security +issues: + - 4070 +affected: + - src/CodeIndex/Database/DbPragmaPolicy.cs + - src/CodeIndex/Database/DbContext.cs + - src/CodeIndex/Cli/DbCommandRunner.cs + - src/CodeIndex/Cli/ReportCommandRunner.cs + - docs/sqlite-command-policy-audit.md +--- + +## English + +- **Tightened SQLite PRAGMA construction policy (#4070)** — busy timeout, cache size, mmap size, and report schema PRAGMA construction now route through shared constrained helpers, with an audit table documenting raw SQL categories. + +## 日本語 + +- **SQLite PRAGMA 構築ポリシーを強化しました (#4070)** — busy timeout、cache size、mmap size、report schema の PRAGMA 構築を制約付き shared helper 経由にし、raw SQL の分類表を追加しました。 diff --git a/docs/sqlite-command-policy-audit.md b/docs/sqlite-command-policy-audit.md new file mode 100644 index 000000000..477ab1ffb --- /dev/null +++ b/docs/sqlite-command-policy-audit.md @@ -0,0 +1,18 @@ +# SQLite Command Policy Audit + +Issue #4070 audited `CommandText` and `PRAGMA` construction in production code. The current dogfood commands are: + +```bash +dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll search --recipe dogfood-risk-patterns/raw-sql-command-text --path src/ --exclude-tests --count --group-by file --limit 80 +dotnet ./src/CodeIndex/bin/Debug/net8.0/cdidx.dll search --recipe dogfood-risk-patterns/pragma-command --path src/ --exclude-tests --count --group-by file --limit 80 +``` + +| Category | Files / examples | Policy | +|---|---|---| +| Constant SQL with parameterized values | `DbReader*`, `DbWriter`, `DiffCommandRunner`, `ExportImportCommandRunner`, `DbPathResolver`, `IndexCommandRunner*`, `PreparedCommandCache`, `SqliteConnectionPolicy`, `McpServer` | Keep values in `SqliteCommandPolicy.Add*` helpers or explicit typed parameters. | +| Dynamic query fragments with fixed internal clauses | `DbSymbolReader`, `DbReader.GraphQueries`, `DbReader.CSharpResolution`, `DbReader.References`, `DbSearchReader`, `RepoMapBuilder`, `QueryCommandRunner` | Only compose repository-owned SQL fragments such as filters, sort expressions, and CTEs; bind user values separately. | +| Dynamic identifiers and schema discovery | `DbContext`, `DbSchemaCache`, `ReportCommandRunner` | Route table/index/column names through `SqliteIdentifier.Quote` or `SqliteCommandPolicy` helpers such as `TableInfoPragmaSql`. | +| PRAGMA reads and writes | `DbContext`, `DbWriter`, `DbCommandRunner`, `DbReader.FilesStatus`, `DbSchemaCache`, `DiffCommandRunner`, `ReportCommandRunner` | Prefer constant names or specific helper methods. Runtime PRAGMA values are constrained through `DbPragmaPolicy`. | +| Migration, DDL, and diagnostics | `DbContext`, `DbWriter`, `DbDebug`, `DbCommandRunner`, `ReportCommandRunner` | Migration SQL stays repository-owned and stable; diagnostics should remain bounded and avoid raw user data. | + +Any new raw SQL construction should fit one of these categories. If it needs a new PRAGMA or identifier path, add a narrow helper instead of interpolating the final command inline. diff --git a/src/CodeIndex/Cli/DbCommandRunner.cs b/src/CodeIndex/Cli/DbCommandRunner.cs index fd4c22439..21a2c498b 100644 --- a/src/CodeIndex/Cli/DbCommandRunner.cs +++ b/src/CodeIndex/Cli/DbCommandRunner.cs @@ -966,7 +966,7 @@ private static void ApplyBusyTimeout(SqliteConnection connection, CancellationTo { cancellationToken.ThrowIfCancellationRequested(); using var cmd = SqliteConnectionPolicy.CreateCommand(connection); - cmd.CommandText = $"PRAGMA busy_timeout={DbPragmaPolicy.ReadBusyTimeoutMs(DbContext.BusyTimeoutEnvironmentVariable)}"; + cmd.CommandText = DbPragmaPolicy.ReadBusyTimeoutPragmaSql(DbContext.BusyTimeoutEnvironmentVariable); cmd.ExecuteNonQuery(); } diff --git a/src/CodeIndex/Cli/ReportCommandRunner.cs b/src/CodeIndex/Cli/ReportCommandRunner.cs index 8e999de9e..33b5279e5 100644 --- a/src/CodeIndex/Cli/ReportCommandRunner.cs +++ b/src/CodeIndex/Cli/ReportCommandRunner.cs @@ -598,7 +598,7 @@ private static HashSet LoadColumnNames(SqliteConnection connection, stri if (!LoadTableNames(connection).Contains(tableName)) return columns; using var cmd = SqliteConnectionPolicy.CreateCommand(connection); - cmd.CommandText = $"PRAGMA table_info(\"{tableName.Replace("\"", "\"\"", StringComparison.Ordinal)}\")"; + cmd.CommandText = SqliteCommandPolicy.TableInfoPragmaSql(tableName); using var reader = cmd.ExecuteReader(); while (reader.Read()) columns.Add(reader.GetString(1)); diff --git a/src/CodeIndex/Database/DbContext.cs b/src/CodeIndex/Database/DbContext.cs index dfbcc5dde..2fbce1581 100644 --- a/src/CodeIndex/Database/DbContext.cs +++ b/src/CodeIndex/Database/DbContext.cs @@ -709,7 +709,7 @@ private void ConfigureAutoVacuumForEmptyDatabase() cmd.CommandText = "SELECT COUNT(*) FROM sqlite_master WHERE name NOT LIKE 'sqlite_%'"; var objectCount = SqliteCommandPolicy.ReadInt64Scalar(cmd, "sqlite_master object count"); if (objectCount == 0) - Execute("PRAGMA auto_vacuum=INCREMENTAL"); + Execute(DbPragmaPolicy.AutoVacuumIncrementalPragmaSql); } public VacuumResult RunIncrementalVacuum(bool dryRun = false) @@ -802,7 +802,7 @@ private VacuumMetrics ReadVacuumMetrics() private void ApplyBusyTimeoutPragma() { var busyTimeoutMs = DbPragmaPolicy.ReadBusyTimeoutMs(BusyTimeoutEnvironmentVariable); - Execute($"PRAGMA busy_timeout={busyTimeoutMs}"); + Execute(DbPragmaPolicy.BusyTimeoutPragmaSql(busyTimeoutMs)); } private long? TryGetDatabaseFileSize() diff --git a/src/CodeIndex/Database/DbPragmaPolicy.cs b/src/CodeIndex/Database/DbPragmaPolicy.cs index 6a418dfc6..281b59a5c 100644 --- a/src/CodeIndex/Database/DbPragmaPolicy.cs +++ b/src/CodeIndex/Database/DbPragmaPolicy.cs @@ -5,6 +5,8 @@ namespace CodeIndex.Database; internal static class DbPragmaPolicy { + internal const string TempStoreMemoryPragmaSql = "PRAGMA temp_store=MEMORY"; + internal const string AutoVacuumIncrementalPragmaSql = "PRAGMA auto_vacuum=INCREMENTAL"; internal const int DefaultBusyTimeoutMs = 5000; internal const int MaxBusyTimeoutMs = 3_600_000; @@ -34,10 +36,10 @@ internal static void ApplyConnectionPerformancePragmas( Action execute, DbConnectionPragmaSettings settings) { - execute($"PRAGMA cache_size=-{settings.CacheSizeKb}"); - execute("PRAGMA temp_store=MEMORY"); + execute(CacheSizePragmaSql(settings.CacheSizeKb)); + execute(TempStoreMemoryPragmaSql); if (settings.MmapSizeBytes.HasValue) - execute($"PRAGMA mmap_size={settings.MmapSizeBytes.Value}"); + execute(MmapSizePragmaSql(settings.MmapSizeBytes.Value)); } internal static void ExecuteSynchronousPragmaWithFallback( @@ -65,6 +67,30 @@ internal static int ReadBusyTimeoutMs(string environmentVariable) DefaultBusyTimeoutMs, MaxBusyTimeoutMs); + internal static string ReadBusyTimeoutPragmaSql(string environmentVariable) + => BusyTimeoutPragmaSql(ReadBusyTimeoutMs(environmentVariable)); + + internal static string BusyTimeoutPragmaSql(int busyTimeoutMs) + { + if (busyTimeoutMs is < 0 or > MaxBusyTimeoutMs) + throw new ArgumentOutOfRangeException(nameof(busyTimeoutMs), busyTimeoutMs, $"SQLite busy_timeout must be between 0 and {MaxBusyTimeoutMs} milliseconds."); + return $"PRAGMA busy_timeout={busyTimeoutMs}"; + } + + internal static string CacheSizePragmaSql(int cacheSizeKb) + { + if (cacheSizeKb <= 0) + throw new ArgumentOutOfRangeException(nameof(cacheSizeKb), cacheSizeKb, "SQLite cache_size kilobytes must be positive."); + return $"PRAGMA cache_size=-{cacheSizeKb}"; + } + + internal static string MmapSizePragmaSql(long mmapSizeBytes) + { + if (mmapSizeBytes < 0) + throw new ArgumentOutOfRangeException(nameof(mmapSizeBytes), mmapSizeBytes, "SQLite mmap_size bytes must be non-negative."); + return $"PRAGMA mmap_size={mmapSizeBytes}"; + } + private static int ReadPositiveIntEnvironment(string name, int fallback, int maximum) => EnvironmentOptionParser.ReadInt32(name, fallback, minimum: 1, maximum).Value; diff --git a/tests/CodeIndex.Tests/DbConnectionPolicyTests.cs b/tests/CodeIndex.Tests/DbConnectionPolicyTests.cs index 173ed5024..2b7beae6e 100644 --- a/tests/CodeIndex.Tests/DbConnectionPolicyTests.cs +++ b/tests/CodeIndex.Tests/DbConnectionPolicyTests.cs @@ -62,6 +62,34 @@ public void DbPragmaPolicy_ApplyConnectionPerformancePragmas_SkipsMmapWhenUnavai statements); } + [Fact] + public void DbPragmaPolicy_PragmaSqlHelpers_ConstrainValues_Issue4070() + { + Assert.Equal("PRAGMA temp_store=MEMORY", DbPragmaPolicy.TempStoreMemoryPragmaSql); + Assert.Equal("PRAGMA auto_vacuum=INCREMENTAL", DbPragmaPolicy.AutoVacuumIncrementalPragmaSql); + Assert.Equal("PRAGMA cache_size=-4096", DbPragmaPolicy.CacheSizePragmaSql(4096)); + Assert.Equal("PRAGMA mmap_size=8192", DbPragmaPolicy.MmapSizePragmaSql(8192)); + Assert.Equal("PRAGMA busy_timeout=5000", DbPragmaPolicy.BusyTimeoutPragmaSql(5000)); + + Assert.Throws(() => DbPragmaPolicy.CacheSizePragmaSql(0)); + Assert.Throws(() => DbPragmaPolicy.MmapSizePragmaSql(-1)); + Assert.Throws(() => DbPragmaPolicy.BusyTimeoutPragmaSql(-1)); + Assert.Throws(() => DbPragmaPolicy.BusyTimeoutPragmaSql(DbPragmaPolicy.MaxBusyTimeoutMs + 1)); + } + + [Fact] + public void DbPragmaPolicy_ReadBusyTimeoutPragmaSql_UsesBoundedEnvironmentValue_Issue4070() + { + using var scope = CdidxEnvironment.Push(new Dictionary + { + ["CDIDX_TEST_BUSY_TIMEOUT_MS"] = "250", + }); + + Assert.Equal( + "PRAGMA busy_timeout=250", + DbPragmaPolicy.ReadBusyTimeoutPragmaSql("CDIDX_TEST_BUSY_TIMEOUT_MS")); + } + [Fact] public void DbPragmaPolicy_ReadConnectionPragmaSettings_UsesScopedEnvironmentParser() {