From fdf17acba2bd5650e29d7c512b9190b1c83da97c Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:10:51 +0100 Subject: [PATCH 1/9] fix(query): validate parenthesized statement heads in ReadOnly whitelist A statement starting with '(' carried no readable verb and was waved through the whitelist unchecked. Parenthesized heads are now unwrapped and validated recursively, set-operation tails (UNION/INTERSECT/EXCEPT) validate their right-hand side as a statement, and any statement whose shape cannot be read is rejected instead of assumed harmless. Also guards the CTE walker against unterminated body parens, which previously threw a negative-length substring error. --- .../ReadOnly/ReadOnlyStatementValidator.cs | 114 ++++++++++++++++-- ...ExecutorReadOnlyStatementWhitelistTests.cs | 35 ++++++ .../ReadOnlyStatementValidatorTests.cs | 79 ++++++++++++ 3 files changed, 216 insertions(+), 12 deletions(-) diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs index e7fad38..84237f3 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs @@ -17,7 +17,10 @@ namespace Equibles.AgentQL.EntityFrameworkCore.Query.ReadOnly; // // CTE bodies are validated recursively — `WITH x AS (INSERT INTO ...) // SELECT * FROM x` is rejected because PG's writable CTEs would otherwise -// be a trivial bypass. +// be a trivial bypass. Parenthesized query heads (`(SELECT ...)`, alone or +// combined via UNION / INTERSECT / EXCEPT) are unwrapped and validated the +// same way; a statement whose shape cannot be read is rejected, never +// waved through. internal static class ReadOnlyStatementValidator { private static readonly HashSet AllowedReadVerbs = new(StringComparer.OrdinalIgnoreCase) @@ -27,6 +30,34 @@ internal static class ReadOnlyStatementValidator "TABLE", }; + // Set operations that may combine a parenthesized query head with a + // right-hand side, which is then validated as its own statement. + private static readonly HashSet SetOperationKeywords = new( + StringComparer.OrdinalIgnoreCase + ) + { + "UNION", + "INTERSECT", + "EXCEPT", + "MINUS", + }; + + // Clauses that may legally trail a parenthesized query head (ORDER BY / + // LIMIT / OFFSET / FETCH / FOR ...). Everything after them is + // expression-level content — the same trust level as the tail of a plain + // SELECT statement, which the whitelist never inspects either: an + // expression position cannot hold a DML/DDL statement in any dialect. + private static readonly HashSet TrailingClauseKeywords = new( + StringComparer.OrdinalIgnoreCase + ) + { + "ORDER", + "LIMIT", + "OFFSET", + "FETCH", + "FOR", + }; + // Returns null when the SQL is acceptable; otherwise a human-readable // explanation of which statement was rejected. The whole batch is // rejected on the first violation — partial execution under ReadOnly is @@ -48,9 +79,32 @@ public static string Validate(string sanitizedSql) private static string ValidateStatement(string statement) { - var verb = ReadIdentifier(statement, 0, out var afterVerb); + var pos = SkipWhitespace(statement, 0); + if (pos >= statement.Length) + return "Statement is empty"; + + // A parenthesized head — `(SELECT ...)`, optionally followed by a set + // operation or a trailing clause — hides its verb inside the parens. + // Unwrap and validate the inner statement recursively, then vet what + // follows the closing paren. Waving these through unread would be a + // fail-open hole in the whitelist. + if (statement[pos] == '(') + { + var end = SkipBalancedParens(statement, pos); + if (end - pos < 2 || statement[end - 1] != ')') + return "Statement has an unterminated parenthesized query"; + + var inner = statement.Substring(pos + 1, end - pos - 2); + var violation = ValidateStatement(inner); + if (violation != null) + return violation; + + return ValidateParenthesizedTail(statement, end); + } + + var verb = ReadIdentifier(statement, pos, out var afterVerb); if (verb == null) - return null; + return $"Statement starting with '{statement[pos]}' is not permitted in ReadOnly mode"; if (AllowedReadVerbs.Contains(verb)) return null; @@ -61,6 +115,42 @@ private static string ValidateStatement(string statement) return ValidateWithStatement(statement, afterVerb); } + // Validates what follows a parenthesized query head. Only two + // continuations are read-shaped: a set operation whose right-hand side is + // validated as its own statement, or a trailing clause keyword. Anything + // else is rejected. + private static string ValidateParenthesizedTail(string sql, int pos) + { + pos = SkipWhitespace(sql, pos); + if (pos >= sql.Length) + return null; + + var keyword = ReadIdentifier(sql, pos, out var afterKeyword); + if (keyword == null) + return "Statement content after a parenthesized query is not permitted in ReadOnly mode"; + + if (SetOperationKeywords.Contains(keyword)) + { + pos = afterKeyword; + var modifier = ReadIdentifier(sql, pos, out var afterModifier); + if ( + modifier != null + && ( + modifier.Equals("ALL", StringComparison.OrdinalIgnoreCase) + || modifier.Equals("DISTINCT", StringComparison.OrdinalIgnoreCase) + ) + ) + pos = afterModifier; + + return ValidateStatement(sql.Substring(pos)); + } + + if (TrailingClauseKeywords.Contains(keyword)) + return null; + + return $"'{keyword.ToUpperInvariant()}' after a parenthesized query is not permitted in ReadOnly mode"; + } + // Walks the CTE definitions of a WITH-led statement, recursively // validating each CTE body, then validates the outer body verb. private static string ValidateWithStatement(string sql, int pos) @@ -113,8 +203,11 @@ private static string ValidateWithStatement(string sql, int pos) return "WITH statement is malformed"; // Recursively validate the CTE body as its own statement, then - // skip past the parens to continue the walk. + // skip past the parens to continue the walk. An unterminated body + // paren is malformed, not something to guess at. var bodyEnd = SkipBalancedParens(sql, pos); + if (bodyEnd - pos < 2 || sql[bodyEnd - 1] != ')') + return "WITH statement is malformed"; var bodyInner = sql.Substring(pos + 1, bodyEnd - pos - 2); var nestedViolation = ValidateStatement(bodyInner); if (nestedViolation != null) @@ -130,16 +223,13 @@ private static string ValidateWithStatement(string sql, int pos) break; } - var outerVerb = ReadIdentifier(sql, pos, out _); - if (outerVerb == null) + var body = sql.Substring(pos); + if (string.IsNullOrWhiteSpace(body)) return "WITH statement has no body"; - if ( - outerVerb.Equals("WITH", StringComparison.OrdinalIgnoreCase) - || AllowedReadVerbs.Contains(outerVerb) - ) - return ValidateStatement(sql.Substring(pos)); - return $"WITH ... {outerVerb.ToUpperInvariant()} is not permitted in ReadOnly mode"; + // The outer body is validated as its own statement — it may itself be + // a WITH, an allowed read verb, or a parenthesized query. + return ValidateStatement(body); } // Splits the SQL into statements on top-level ';'. Quoted runs are diff --git a/tests/Equibles.AgentQL.IntegrationTests/QueryExecutorReadOnlyStatementWhitelistTests.cs b/tests/Equibles.AgentQL.IntegrationTests/QueryExecutorReadOnlyStatementWhitelistTests.cs index e31eccd..5c713f1 100644 --- a/tests/Equibles.AgentQL.IntegrationTests/QueryExecutorReadOnlyStatementWhitelistTests.cs +++ b/tests/Equibles.AgentQL.IntegrationTests/QueryExecutorReadOnlyStatementWhitelistTests.cs @@ -63,6 +63,15 @@ public QueryExecutorReadOnlyStatementWhitelistTests(PostgresFixture fixture) )] [InlineData("WITH x AS (UPDATE \"Customers\" SET \"Name\" = 'M' RETURNING *) SELECT * FROM x")] [InlineData("WITH x AS (DELETE FROM \"Customers\" RETURNING *) SELECT * FROM x")] + // Parenthesized heads must be unwrapped and validated, not waved through + // because no verb is readable at position zero. + [InlineData("(INSERT INTO \"Customers\" (\"Name\", \"Tier\") VALUES ('M', 0))")] + [InlineData( + "(WITH x AS (INSERT INTO \"Customers\" (\"Name\", \"Tier\") VALUES ('M', 0) RETURNING *) SELECT * FROM x)" + )] + [InlineData( + "(SELECT 1) UNION (INSERT INTO \"Customers\" (\"Name\", \"Tier\") VALUES ('M', 0) RETURNING 1)" + )] public async Task Execute_ReadOnlyForbiddenStatement_NeutralizedAndDoesNotExecute(string sql) { await using var executorContext = Fixture.CreateContext(); @@ -108,6 +117,13 @@ public async Task Execute_ReadOnlyForbiddenStatement_NeutralizedAndDoesNotExecut // accepted even when its data mentions a forbidden keyword. [InlineData("SELECT 'please UPDATE your settings' AS message")] [InlineData("SELECT \"Name\" AS \"UPDATE_TIME\" FROM \"Customers\"")] + // Parenthesized read shapes are valid PostgreSQL and must survive the + // unwrap-and-validate path. + [InlineData("(SELECT 1 AS x)")] + [InlineData("((SELECT 1 AS x))")] + [InlineData("(SELECT 1 AS x) UNION ALL (SELECT 2)")] + [InlineData("(SELECT 1 AS x) ORDER BY 1 LIMIT 1")] + [InlineData("(WITH y AS (SELECT 1 AS a) SELECT * FROM y)")] public async Task Execute_ReadOnlyAllowedStatement_Succeeds(string sql) { await using var executorContext = Fixture.CreateContext(); @@ -148,4 +164,23 @@ public async Task Execute_QuotedCteName_ExecutesAndReturnsRows_NotNeutralized() // Seed has two Lisbon bookings and one Tokyo booking. Convert.ToInt32(result.Data![0]["BookingCount"]).Should().Be(2); } + + [Fact] + public async Task Execute_ParenthesizedSelect_ExecutesAndReturnsRows_NotNeutralized() + { + // A neutralized statement also returns Success=true with empty data — + // this proves a parenthesized head actually reaches the database and + // returns a row rather than being silently rejected. + await using var context = Fixture.CreateContext(); + var executor = new QueryExecutor( + context, + new AgentQLOptions { ReadOnly = true }, + NullLogger>.Instance + ); + + var result = await executor.Execute("(SELECT COUNT(*) AS c FROM \"Customers\")"); + + result.Success.Should().BeTrue(result.ErrorMessage); + result.Data.Should().HaveCount(1); + } } diff --git a/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs b/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs index 4485a88..d69740a 100644 --- a/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs +++ b/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs @@ -221,6 +221,10 @@ public void Validate_LeadingSemicolons_AreSkipped() [InlineData("WITH x")] [InlineData("WITH x AS")] [InlineData("WITH x AS (SELECT 1)")] + // Unterminated body parens must be reported as malformed, not crash the + // CTE walker with a negative-length substring. + [InlineData("WITH x AS (")] + [InlineData("WITH x AS (SELECT 1")] public void Validate_MalformedWith_ReturnsViolation(string sql) { // Each of these is missing either a CTE name, the AS keyword, the @@ -228,4 +232,79 @@ public void Validate_MalformedWith_ReturnsViolation(string sql) // guess and treats them as malformed. ReadOnlyStatementValidator.Validate(sql).Should().NotBeNull(); } + + [Theory] + [InlineData("(SELECT 1)")] + [InlineData("((SELECT 1))")] + [InlineData("(SELECT 1) UNION SELECT 2")] + [InlineData("(SELECT 1) UNION ALL (SELECT 2)")] + [InlineData("(SELECT 1) UNION DISTINCT (SELECT 2)")] + [InlineData("(SELECT 1) INTERSECT (SELECT 2)")] + [InlineData("(SELECT 1) EXCEPT (SELECT 2)")] + [InlineData("(SELECT 1) UNION (SELECT 2) UNION (SELECT 3)")] + [InlineData("(SELECT 1 AS a) ORDER BY a")] + [InlineData("(SELECT 1 AS a) LIMIT 1")] + [InlineData("(SELECT 1 AS a) ORDER BY a LIMIT 1 OFFSET 0")] + [InlineData("(VALUES (1), (2))")] + [InlineData("(TABLE foo)")] + [InlineData("(WITH x AS (SELECT 1) SELECT * FROM x)")] + public void Validate_ParenthesizedReadShapes_ReturnsNull(string sql) + { + // A parenthesized head hides its verb inside the parens — the + // validator must unwrap and accept read-shaped content rather than + // reject every paren-led statement outright. + ReadOnlyStatementValidator.Validate(sql).Should().BeNull(); + } + + [Theory] + [InlineData("(INSERT INTO foo VALUES (1))", "INSERT")] + [InlineData("((DELETE FROM foo))", "DELETE")] + [InlineData("(WITH x AS (INSERT INTO foo VALUES (1) RETURNING *) SELECT * FROM x)", "INSERT")] + [InlineData("(SELECT 1) UNION (DELETE FROM foo RETURNING 1)", "DELETE")] + [InlineData("(SELECT 1) UNION ALL DELETE FROM foo", "DELETE")] + [InlineData("(SELECT 1) INTERSECT (UPDATE foo SET a = 1 RETURNING a)", "UPDATE")] + public void Validate_WriteInsideParenthesizedStatement_IsRejected(string sql, string verb) + { + // Before parenthesized heads were unwrapped, a statement starting + // with '(' carried no readable verb and was waved through — a + // fail-open hole in the whitelist. These lock the fail-closed + // behavior in. + var violation = ReadOnlyStatementValidator.Validate(sql); + violation.Should().NotBeNull(); + violation.Should().Contain(verb); + } + + [Theory] + [InlineData("(SELECT 1")] // unterminated paren + [InlineData("(SELECT 1) INTO foo")] // non-read continuation after the head + [InlineData("(SELECT 1) )")] // stray content after the head + [InlineData("1")] // no readable verb at all + [InlineData("?")] + public void Validate_UnreadableStatementShape_IsRejected(string sql) + { + // The whitelist is fail-closed: a statement whose shape cannot be + // read as one of the permitted forms is rejected, never assumed + // harmless. + ReadOnlyStatementValidator.Validate(sql).Should().NotBeNull(); + } + + [Fact] + public void Validate_WithOuterBodyParenthesized_IsAccepted() + { + // PG accepts a parenthesized outer body after the CTE list. + ReadOnlyStatementValidator + .Validate("WITH x AS (SELECT 1) (SELECT * FROM x)") + .Should() + .BeNull(); + } + + [Fact] + public void Validate_WithOuterBodyParenthesizedWrite_IsRejected() + { + var violation = ReadOnlyStatementValidator.Validate( + "WITH x AS (SELECT 1) (INSERT INTO foo VALUES (1))" + ); + violation.Should().NotBeNull(); + violation.Should().Contain("INSERT"); + } } From 1157fda57395159883930a2b65ceb9931c74b3ea Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:11:09 +0100 Subject: [PATCH 2/9] fix(query): use provider-default transaction isolation instead of ReadUncommitted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReadUncommitted allowed dirty reads on SQL Server in ReadOnly mode — uncommitted data that may later be rolled back, which the LLM would present as fact. The executor now uses the provider's default isolation (ReadCommitted on PostgreSQL/SQL Server). SQLite is the exception: its default maps to BEGIN IMMEDIATE, which acquires a write lock that PRAGMA query_only itself refuses, so the SQLite enforcer requests a deferred BEGIN via a new ReadOnlyIsolationLevel hint on the enforcer interface. --- .../Query/QueryExecutor.cs | 17 ++++++++++------- .../Query/ReadOnly/IReadOnlySessionEnforcer.cs | 8 ++++++++ .../ReadOnly/SqliteReadOnlySessionEnforcer.cs | 8 ++++++++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Query/QueryExecutor.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Query/QueryExecutor.cs index 443e424..8b392c8 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Query/QueryExecutor.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Query/QueryExecutor.cs @@ -68,10 +68,6 @@ public async Task Execute(string sql) } } - var isolationLevel = _options.ReadOnly - ? IsolationLevel.ReadUncommitted - : IsolationLevel.ReadCommitted; - var enforcer = _options.ReadOnly ? ReadOnlySessionEnforcerFactory.Resolve(_context.Database.ProviderName) : NullReadOnlySessionEnforcer.Instance; @@ -96,9 +92,16 @@ public async Task Execute(string sql) try { - await using var transaction = await _context.Database.BeginTransactionAsync( - isolationLevel - ); + // Provider-default isolation (ReadCommitted on PostgreSQL / + // SQL Server) unless the ReadOnly enforcer needs a + // specific level (SQLite). ReadUncommitted — formerly used + // for all ReadOnly queries — allows dirty reads on SQL + // Server: uncommitted data that may be rolled back, which + // the LLM would then present as fact. + var isolationLevel = _options.ReadOnly ? enforcer.ReadOnlyIsolationLevel : null; + await using var transaction = isolationLevel.HasValue + ? await _context.Database.BeginTransactionAsync(isolationLevel.Value) + : await _context.Database.BeginTransactionAsync(); try { diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/IReadOnlySessionEnforcer.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/IReadOnlySessionEnforcer.cs index c3ece11..aed2628 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/IReadOnlySessionEnforcer.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/IReadOnlySessionEnforcer.cs @@ -1,3 +1,4 @@ +using System.Data; using System.Data.Common; namespace Equibles.AgentQL.EntityFrameworkCore.Query.ReadOnly; @@ -21,4 +22,11 @@ internal interface IReadOnlySessionEnforcer // DBMS — the executor logs this message once so operators see what is // and is not guaranteed, and what to do about it. string Limitation => null; + + // Isolation level for the executor's wrapper transaction in ReadOnly + // mode; null means the provider default. SQLite must override this: + // Microsoft.Data.Sqlite maps the default (Serializable) to BEGIN + // IMMEDIATE, which acquires a write lock that PRAGMA query_only itself + // refuses — ReadUncommitted maps to a plain deferred BEGIN instead. + IsolationLevel? ReadOnlyIsolationLevel => null; } diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/SqliteReadOnlySessionEnforcer.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/SqliteReadOnlySessionEnforcer.cs index ae1352d..dcca8f4 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/SqliteReadOnlySessionEnforcer.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/SqliteReadOnlySessionEnforcer.cs @@ -1,3 +1,4 @@ +using System.Data; using System.Data.Common; namespace Equibles.AgentQL.EntityFrameworkCore.Query.ReadOnly; @@ -23,6 +24,13 @@ internal sealed class SqliteReadOnlySessionEnforcer : IReadOnlySessionEnforcer private SqliteReadOnlySessionEnforcer() { } + // Microsoft.Data.Sqlite maps Serializable (its default) to BEGIN + // IMMEDIATE, which acquires a write lock — refused once query_only is + // set. ReadUncommitted maps to a plain deferred BEGIN, which is what a + // read-only wrapper transaction needs; without a shared cache it has no + // dirty-read semantics on SQLite. + public IsolationLevel? ReadOnlyIsolationLevel => IsolationLevel.ReadUncommitted; + public Task Apply(DbConnection connection) => Execute(connection, "PRAGMA query_only = 1"); public Task Reset(DbConnection connection) => Execute(connection, "PRAGMA query_only = 0"); From 1e4ca26f314820d13e38f77fb22f3d4952b0a3b2 Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:11:09 +0100 Subject: [PATCH 3/9] perf(schema): cache the schema description for the application lifetime SchemaProvider is scoped, so its instance-level cache re-ran the full EF model walk (plus a connection open for server info) on every request scope. The cache now lives on the AgentQLOptions singleton, keyed by options lifetime, with a semaphore so the factory runs once under concurrent first calls. --- .../Schema/SchemaProvider.cs | 16 ++++++---- .../Configuration/AgentQLOptions.cs | 6 ++++ .../Configuration/SchemaDescriptionCache.cs | 29 +++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs index 8b73d0d..448de17 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs @@ -12,7 +12,6 @@ public class SchemaProvider : ISchemaProvider { private readonly TContext _context; private readonly AgentQLOptions _options; - private string _cachedSchema; public SchemaProvider(TContext context, AgentQLOptions options) { @@ -20,11 +19,17 @@ public SchemaProvider(TContext context, AgentQLOptions options) _options = options; } - public async Task GetSchemaDescription() + // Cached on the options singleton, not on this scoped instance — the + // schema depends only on the EF model and the options, so one model walk + // (plus one connection open for server info) serves the whole application + // lifetime instead of every request scope. + public Task GetSchemaDescription() { - if (!string.IsNullOrEmpty(_cachedSchema)) - return _cachedSchema; + return _options.SchemaCache.GetOrCreate(BuildSchemaDescription); + } + private async Task BuildSchemaDescription() + { var schema = new StringBuilder(); schema.AppendLine("=== DATABASE SCHEMA ==="); schema.AppendLine("Generated by Entity Framework Core using Code First approach"); @@ -61,8 +66,7 @@ public async Task GetSchemaDescription() AppendTableInfo(schema, table); } - _cachedSchema = schema.ToString(); - return _cachedSchema; + return schema.ToString(); } private bool HasIncludedEntity(ITable table) diff --git a/src/Equibles.AgentQL/Configuration/AgentQLOptions.cs b/src/Equibles.AgentQL/Configuration/AgentQLOptions.cs index 12079f7..116c068 100644 --- a/src/Equibles.AgentQL/Configuration/AgentQLOptions.cs +++ b/src/Equibles.AgentQL/Configuration/AgentQLOptions.cs @@ -14,6 +14,12 @@ public class AgentQLOptions internal Dictionary EntityConfigurations { get; } = new(); + // The generated schema description depends only on the EF model and this + // options instance, so the cache shares the options' lifetime (a singleton + // in DI) instead of the scoped SchemaProvider's — one model walk per + // application, not per request scope. + internal SchemaDescriptionCache SchemaCache { get; } = new(); + public EntityConfiguration Entity() where T : class { diff --git a/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs b/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs new file mode 100644 index 0000000..9eeba4c --- /dev/null +++ b/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs @@ -0,0 +1,29 @@ +namespace Equibles.AgentQL.Configuration; + +// Application-lifetime cache for the generated schema description. Owned by +// AgentQLOptions (see its SchemaCache property) so scoped SchemaProvider +// instances share one cached result per container; distinct options instances +// (separate containers, or tests) never share it. The semaphore ensures the +// factory runs once even under concurrent first calls. +internal sealed class SchemaDescriptionCache +{ + private readonly SemaphoreSlim _gate = new(1, 1); + private volatile string _value; + + public async Task GetOrCreate(Func> factory) + { + if (_value != null) + return _value; + + await _gate.WaitAsync(); + try + { + _value ??= await factory(); + return _value; + } + finally + { + _gate.Release(); + } + } +} From 1b389a6dc77c4dc7caafa93ec0ae7242c4212952 Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:11:21 +0100 Subject: [PATCH 4/9] fix(microsoft-ai): apply MaxOutputTokens and honor custom Anthropic endpoint AgentQLChatOptions.MaxOutputTokens was documented but never read; it is now applied through ConfigureOptions when the per-request ChatOptions leaves it unset. The Anthropic factory also honors an explicitly configured Endpoint (BaseUrl) instead of silently ignoring it, while keeping the SDK default when none is set. --- .../Extensions/ServiceCollectionExtensions.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Equibles.AgentQL.MicrosoftAI/Extensions/ServiceCollectionExtensions.cs b/src/Equibles.AgentQL.MicrosoftAI/Extensions/ServiceCollectionExtensions.cs index 7f67f1a..0a293dd 100644 --- a/src/Equibles.AgentQL.MicrosoftAI/Extensions/ServiceCollectionExtensions.cs +++ b/src/Equibles.AgentQL.MicrosoftAI/Extensions/ServiceCollectionExtensions.cs @@ -45,7 +45,10 @@ public static IServiceCollection AddAgentQLChat( // Self-correction is added before function invocation so it ends up // the outer wrapper around the whole tool loop (see // ChatClientBuilderExtensions.UseAgentQLSelfCorrection). + // ConfigureOptions fills MaxOutputTokens only when the caller's + // per-request ChatOptions left it unset. return new ChatClientBuilder(innerClient) + .ConfigureOptions(chat => chat.MaxOutputTokens ??= options.MaxOutputTokens) .UseAgentQLSelfCorrection(selfCorrectionOptions) .UseFunctionInvocation() .Build(); @@ -65,7 +68,14 @@ private static IChatClient CreateOpenAIClient(AgentQLChatOptions options) private static IChatClient CreateAnthropicClient(AgentQLChatOptions options) { - var client = new AnthropicClient { ApiKey = options.ApiKey }; + // Only override BaseUrl when explicitly configured — the SDK's own + // default base address differs from GetEndpoint()'s OpenAI-style + // default, so falling back to GetEndpoint() here would break the + // out-of-the-box Anthropic setup. + var client = string.IsNullOrEmpty(options.Endpoint) + ? new AnthropicClient { ApiKey = options.ApiKey } + : new AnthropicClient { ApiKey = options.ApiKey, BaseUrl = options.Endpoint }; + return client.AsIChatClient(options.ModelName); } } From 085fff59527eca1e83f5d518da21d96d3f5fb862 Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:11:21 +0100 Subject: [PATCH 5/9] fix(microsoft-ai): only skip injected reminders when extracting the question ExtractQuestion skipped any user message merely containing the literal '' text. Injected reminders always start with the tag, so matching on the prefix keeps genuine user questions that mention the tag in the running. --- .../SelfCorrectingChatClient.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Equibles.AgentQL.MicrosoftAI/SelfCorrectingChatClient.cs b/src/Equibles.AgentQL.MicrosoftAI/SelfCorrectingChatClient.cs index 7a782e5..5f967a9 100644 --- a/src/Equibles.AgentQL.MicrosoftAI/SelfCorrectingChatClient.cs +++ b/src/Equibles.AgentQL.MicrosoftAI/SelfCorrectingChatClient.cs @@ -212,10 +212,15 @@ private static ChatMessage BuildReminder(string question) private static string ExtractQuestion(IEnumerable messages) { // Skip any reminder this guard injected on a prior (replayed) turn so the - // restated question is the real user question, not a reminder. + // restated question is the real user question, not a reminder. Injected + // reminders always start with the tag (see BuildReminder), so a genuine + // user question that merely mentions the tag is not skipped. return messages .Where(m => m.Role == ChatRole.User) .Select(m => m.Text) - .LastOrDefault(text => text != null && !text.Contains("")); + .LastOrDefault(text => + text != null + && !text.TrimStart().StartsWith("", StringComparison.Ordinal) + ); } } From aed19711a4618dd73971d49b9bf87e9fad1c9703 Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:11:22 +0100 Subject: [PATCH 6/9] docs(readme): document exclusion caveat and parenthesized-head validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit States explicitly that include/exclude is schema hiding, not access control — the executor does not block SQL referencing excluded tables or columns, so sensitive data must be protected at the database. Also updates the ReadOnly whitelist description for the parenthesized-head validation. --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2f623b8..f9d708d 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ Passed via `configureAgentQL` when calling `AddAgentQL` or `AddAgentQLChat`: With `ReadOnly = true` the executor applies three layers of defense: -1. **Statement whitelist (provider-agnostic).** Before the connection is opened, the SQL is parsed and any statement that is not a `SELECT`, `VALUES`, `TABLE`, or `WITH` with a read-shaped body (CTEs validated recursively) is silently neutralized — the result returns `Success=true` with empty data, the write never reaches the database. Every DML, DDL, transaction-control, permission, session-settings, anonymous-block, and stored-procedure call is refused at this layer. +1. **Statement whitelist (provider-agnostic).** Before the connection is opened, the SQL is parsed and any statement that is not a `SELECT`, `VALUES`, `TABLE`, or `WITH` with a read-shaped body (CTEs validated recursively) is silently neutralized — the result returns `Success=true` with empty data, the write never reaches the database. Parenthesized query heads (`(SELECT ...)`, alone or combined via `UNION`/`INTERSECT`/`EXCEPT`) are unwrapped and validated the same way, and any statement whose shape cannot be read is rejected rather than assumed harmless. Every DML, DDL, transaction-control, permission, session-settings, anonymous-block, and stored-procedure call is refused at this layer. 2. **DBMS session read-only (where the provider supports it).** The connection's session is flipped to read-only so the database itself refuses any write that did somehow get past the whitelist — including writes attempted under an implicit autocommit transaction after an embedded `COMMIT`. 3. **End-of-query rollback.** The executor's transaction rolls back at the end of every query regardless of outcome. @@ -250,6 +250,8 @@ Each level overrides the one above it: Primary keys and discriminator columns are always included regardless of configuration. +> **Exclusion is schema hiding, not access control.** Include/exclude only affects the schema description shown to the model — the query executor does not parse or block SQL that references excluded tables or columns. A model that guesses (or is prompt-injected into trying) `SELECT "Ssn" FROM "Customers"` will get real data back, and database error messages returned to the model can confirm that hidden columns exist. Treat exclusion as a token-budget and relevance tool; protect genuinely sensitive data at the database itself — column grants, views, or the least-privileged principal described under **ReadOnly mode**. + ## AI Providers ### OpenAI From 8b4da795e31b702341ba299d89e7995e11295bff Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:19:01 +0100 Subject: [PATCH 7/9] fix(query): bound recursion depth in ReadOnly statement validation The paren-unwrap, set-operation and CTE recursion had no depth guard, so a MaxSqlLength-sized run of nested parens could recurse thousands of frames deep and kill the process with an uncatchable StackOverflow. Depth is now capped at 64 nested query heads; deeper input is rejected with the usual silent-neutralization contract. --- .../ReadOnly/ReadOnlyStatementValidator.cs | 33 +++++++++++++------ .../ReadOnlyStatementValidatorTests.cs | 20 +++++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs index 84237f3..cfe158a 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs @@ -69,7 +69,7 @@ public static string Validate(string sanitizedSql) foreach (var statement in SplitStatements(sanitizedSql)) { - var violation = ValidateStatement(statement); + var violation = ValidateStatement(statement, depth: 0); if (violation != null) return violation; } @@ -77,8 +77,18 @@ public static string Validate(string sanitizedSql) return null; } - private static string ValidateStatement(string statement) + // Every recursive hop (paren unwrap, set-operation tail, CTE body, WITH + // outer body) increments depth. The cap bounds stack usage: without it a + // MaxSqlLength-sized run of nested parens recurses thousands of frames + // deep and a StackOverflowException kills the process uncatchably. No + // legitimate read query nests query heads anywhere near this deep. + private const int MaxNestingDepth = 64; + + private static string ValidateStatement(string statement, int depth) { + if (depth > MaxNestingDepth) + return "Statement is nested too deeply"; + var pos = SkipWhitespace(statement, 0); if (pos >= statement.Length) return "Statement is empty"; @@ -95,11 +105,11 @@ private static string ValidateStatement(string statement) return "Statement has an unterminated parenthesized query"; var inner = statement.Substring(pos + 1, end - pos - 2); - var violation = ValidateStatement(inner); + var violation = ValidateStatement(inner, depth + 1); if (violation != null) return violation; - return ValidateParenthesizedTail(statement, end); + return ValidateParenthesizedTail(statement, end, depth + 1); } var verb = ReadIdentifier(statement, pos, out var afterVerb); @@ -112,15 +122,18 @@ private static string ValidateStatement(string statement) if (!verb.Equals("WITH", StringComparison.OrdinalIgnoreCase)) return $"Statement '{verb.ToUpperInvariant()}' is not permitted in ReadOnly mode"; - return ValidateWithStatement(statement, afterVerb); + return ValidateWithStatement(statement, afterVerb, depth); } // Validates what follows a parenthesized query head. Only two // continuations are read-shaped: a set operation whose right-hand side is // validated as its own statement, or a trailing clause keyword. Anything // else is rejected. - private static string ValidateParenthesizedTail(string sql, int pos) + private static string ValidateParenthesizedTail(string sql, int pos, int depth) { + if (depth > MaxNestingDepth) + return "Statement is nested too deeply"; + pos = SkipWhitespace(sql, pos); if (pos >= sql.Length) return null; @@ -142,7 +155,7 @@ private static string ValidateParenthesizedTail(string sql, int pos) ) pos = afterModifier; - return ValidateStatement(sql.Substring(pos)); + return ValidateStatement(sql.Substring(pos), depth + 1); } if (TrailingClauseKeywords.Contains(keyword)) @@ -153,7 +166,7 @@ private static string ValidateParenthesizedTail(string sql, int pos) // Walks the CTE definitions of a WITH-led statement, recursively // validating each CTE body, then validates the outer body verb. - private static string ValidateWithStatement(string sql, int pos) + private static string ValidateWithStatement(string sql, int pos, int depth) { var maybeRecursive = ReadIdentifier(sql, pos, out var afterMaybe); if ( @@ -209,7 +222,7 @@ private static string ValidateWithStatement(string sql, int pos) if (bodyEnd - pos < 2 || sql[bodyEnd - 1] != ')') return "WITH statement is malformed"; var bodyInner = sql.Substring(pos + 1, bodyEnd - pos - 2); - var nestedViolation = ValidateStatement(bodyInner); + var nestedViolation = ValidateStatement(bodyInner, depth + 1); if (nestedViolation != null) return nestedViolation; pos = bodyEnd; @@ -229,7 +242,7 @@ private static string ValidateWithStatement(string sql, int pos) // The outer body is validated as its own statement — it may itself be // a WITH, an allowed read verb, or a parenthesized query. - return ValidateStatement(body); + return ValidateStatement(body, depth + 1); } // Splits the SQL into statements on top-level ';'. Quoted runs are diff --git a/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs b/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs index d69740a..d1aa02e 100644 --- a/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs +++ b/tests/Equibles.AgentQL.UnitTests/Query/ReadOnly/ReadOnlyStatementValidatorTests.cs @@ -288,6 +288,26 @@ public void Validate_UnreadableStatementShape_IsRejected(string sql) ReadOnlyStatementValidator.Validate(sql).Should().NotBeNull(); } + [Fact] + public void Validate_DeeplyNestedParens_IsRejectedNotStackOverflow() + { + // Recursion depth is bounded — a MaxSqlLength-sized run of nested + // parens must produce a clean violation, not an uncatchable + // StackOverflowException that kills the process. + var sql = new string('(', 2000) + "SELECT 1" + new string(')', 2000); + + ReadOnlyStatementValidator.Validate(sql).Should().NotBeNull(); + } + + [Fact] + public void Validate_ModeratelyNestedParens_IsAccepted() + { + // Realistic nesting stays well under the depth cap. + var sql = new string('(', 10) + "SELECT 1" + new string(')', 10); + + ReadOnlyStatementValidator.Validate(sql).Should().BeNull(); + } + [Fact] public void Validate_WithOuterBodyParenthesized_IsAccepted() { From f3c3680796c8fb32a42821aed2ec78eed225b04e Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:19:02 +0100 Subject: [PATCH 8/9] fix(schema): key the schema cache by DbContext type DI hands the last-registered AgentQLOptions singleton to every provider, so a host registering two contexts would have seen one context's cached schema served for the other. The cache dictionary is now keyed by the context CLR type; a failed build caches nothing and is retried. --- .../Schema/SchemaProvider.cs | 2 +- .../Configuration/SchemaDescriptionCache.cs | 25 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs b/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs index 448de17..c9c1f60 100644 --- a/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs +++ b/src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs @@ -25,7 +25,7 @@ public SchemaProvider(TContext context, AgentQLOptions options) // lifetime instead of every request scope. public Task GetSchemaDescription() { - return _options.SchemaCache.GetOrCreate(BuildSchemaDescription); + return _options.SchemaCache.GetOrCreate(typeof(TContext), BuildSchemaDescription); } private async Task BuildSchemaDescription() diff --git a/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs b/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs index 9eeba4c..7efcbe0 100644 --- a/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs +++ b/src/Equibles.AgentQL/Configuration/SchemaDescriptionCache.cs @@ -1,25 +1,34 @@ +using System.Collections.Concurrent; + namespace Equibles.AgentQL.Configuration; // Application-lifetime cache for the generated schema description. Owned by // AgentQLOptions (see its SchemaCache property) so scoped SchemaProvider // instances share one cached result per container; distinct options instances -// (separate containers, or tests) never share it. The semaphore ensures the -// factory runs once even under concurrent first calls. +// (separate containers, or tests) never share it. Keyed by DbContext CLR type +// because DI hands the last-registered options singleton to every provider — +// a host that registers two contexts must not see one context's schema served +// for the other. The semaphore ensures a factory runs once even under +// concurrent first calls; a failed factory caches nothing and is retried. internal sealed class SchemaDescriptionCache { + private readonly ConcurrentDictionary _values = new(); private readonly SemaphoreSlim _gate = new(1, 1); - private volatile string _value; - public async Task GetOrCreate(Func> factory) + public async Task GetOrCreate(Type contextType, Func> factory) { - if (_value != null) - return _value; + if (_values.TryGetValue(contextType, out var cached)) + return cached; await _gate.WaitAsync(); try { - _value ??= await factory(); - return _value; + if (_values.TryGetValue(contextType, out cached)) + return cached; + + var value = await factory(); + _values[contextType] = value; + return value; } finally { From 341ea323e019b3316c3e0c69cce331600748347d Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Tue, 7 Jul 2026 16:23:37 +0100 Subject: [PATCH 9/9] chore(deps): pin SQLitePCLRaw.lib.e_sqlite3 above vulnerable 2.1.11 Microsoft.EntityFrameworkCore.Sqlite 10.0.8 transitively brings in SQLitePCLRaw.lib.e_sqlite3 2.1.11, flagged by NuGet audit with a known high-severity vulnerability (GHSA-2m69-gcr7-jv3q), which fails the warnings-as-errors lint build. Enables central transitive pinning and lifts the native lib to 3.53.3 (versioned by SQLite release; the managed bundle/core stay on 2.1.x and accept it). --- Directory.Packages.props | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Directory.Packages.props b/Directory.Packages.props index bc1dd88..b227cd7 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -1,6 +1,11 @@ true + + true @@ -11,6 +16,7 @@ +