fix: harden ReadOnly SQL validation and fix security-review findings

Directory.Packages.props

@@ -1,6 +1,11 @@
 <Project>
   <PropertyGroup>
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
+    <!-- Promotes transitive packages listed below to the pinned version —
+         needed to lift SQLitePCLRaw above the vulnerable 2.1.11 that
+         Microsoft.EntityFrameworkCore.Sqlite 10.0.8 brings in
+         (GHSA-2m69-gcr7-jv3q, NU1903 fails the lint build). -->
+    <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
   </PropertyGroup>
 
   <!-- Production dependencies -->
@@ -11,6 +16,7 @@
     <PackageVersion Include="Microsoft.Extensions.AI.OpenAI" Version="10.6.0" />
     <PackageVersion Include="Anthropic" Version="12.26.0" />
     <PackageVersion Include="Newtonsoft.Json" Version="13.0.4" />
+    <PackageVersion Include="SQLitePCLRaw.lib.e_sqlite3" Version="3.53.3" />
   </ItemGroup>
 
   <!-- Test dependencies -->

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

src/Equibles.AgentQL.EntityFrameworkCore/Query/QueryExecutor.cs

@@ -68,10 +68,6 @@ public async Task<QueryResult> 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<QueryResult> 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
                     {

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;
 }

src/Equibles.AgentQL.EntityFrameworkCore/Query/ReadOnly/ReadOnlyStatementValidator.cs

@@ -17,7 +17,10 @@
 //
 // 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<string> AllowedReadVerbs = new(StringComparer.OrdinalIgnoreCase)
@@ -27,6 +30,34 @@
         "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<string> 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<string> 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
@@ -36,34 +67,106 @@
         if (string.IsNullOrWhiteSpace(sanitizedSql))
             return null;
 
         foreach (var statement in SplitStatements(sanitizedSql))
         {
-            var violation = ValidateStatement(statement);
+            var violation = ValidateStatement(statement, depth: 0);
             if (violation != null)
                 return violation;
         }
 
         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)
     {
-        var verb = ReadIdentifier(statement, 0, out var afterVerb);
+        if (depth > MaxNestingDepth)
+            return "Statement is nested too deeply";
+
+        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, depth + 1);
+            if (violation != null)
+                return violation;
+
+            return ValidateParenthesizedTail(statement, end, depth + 1);
+        }
+
+        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;
 
         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, int depth)
+    {
+        if (depth > MaxNestingDepth)
+            return "Statement is nested too deeply";
+
+        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), depth + 1);
+        }
+
+        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)
+    private static string ValidateWithStatement(string sql, int pos, int depth)
     {
         var maybeRecursive = ReadIdentifier(sql, pos, out var afterMaybe);
         if (
@@ -113,10 +216,13 @@
                 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);
+            var nestedViolation = ValidateStatement(bodyInner, depth + 1);
             if (nestedViolation != null)
                 return nestedViolation;
             pos = bodyEnd;
@@ -130,16 +236,13 @@
             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, depth + 1);
     }
 
     // Splits the SQL into statements on top-level ';'. Quoted runs are

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");

src/Equibles.AgentQL.EntityFrameworkCore/Schema/SchemaProvider.cs

@@ -12,19 +12,24 @@ public class SchemaProvider<TContext> : ISchemaProvider
 {
     private readonly TContext _context;
     private readonly AgentQLOptions _options;
-    private string _cachedSchema;
 
     public SchemaProvider(TContext context, AgentQLOptions options)
     {
         _context = context;
         _options = options;
     }
 
-    public async Task<string> 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<string> GetSchemaDescription()
     {
-        if (!string.IsNullOrEmpty(_cachedSchema))
-            return _cachedSchema;
+        return _options.SchemaCache.GetOrCreate(typeof(TContext), BuildSchemaDescription);
+    }
 
+    private async Task<string> 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<string> GetSchemaDescription()
             AppendTableInfo(schema, table);
         }
 
-        _cachedSchema = schema.ToString();
-        return _cachedSchema;
+        return schema.ToString();
     }
 
     private bool HasIncludedEntity(ITable table)

src/Equibles.AgentQL.MicrosoftAI/Extensions/ServiceCollectionExtensions.cs

@@ -45,7 +45,10 @@
             // 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 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);
     }
 }

src/Equibles.AgentQL.MicrosoftAI/SelfCorrectingChatClient.cs

@@ -212,10 +212,15 @@ private static ChatMessage BuildReminder(string question)
     private static string ExtractQuestion(IEnumerable<ChatMessage> 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("<system-reminder>"));
+            .LastOrDefault(text =>
+                text != null
+                && !text.TrimStart().StartsWith("<system-reminder>", StringComparison.Ordinal)
+            );
     }
 }

src/Equibles.AgentQL/Configuration/AgentQLOptions.cs

@@ -14,6 +14,12 @@ public class AgentQLOptions
 
     internal Dictionary<Type, EntityConfiguration> 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<T>()
         where T : class
     {