MSSQL: read-only guard module (validateSql) with corpus tests #322

New issue

Closed

opened 2026-11-10 08:20:06 +00:00 by david · 1 comment

david commented

2026-11-10 08:20:06 +00:00

Owner

Summary

Create the guard module with validateSql(sql): a minimal, dependency-free check that allows one SELECT / WITH ... SELECT statement and rejects the rest. It catches accidents. It is not a security boundary.

Background

Decision: docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md. Read-only database credentials are the write control; this guard is defence in depth against accidents only (for example an agent attempting an UPDATE). The Stage 1 spike showed no parser reached zero false accepts, so there is no parser and no new dependency. Zero false accepts is not a requirement. [SECURITY RISK] If the login can write, a crafted query can bypass this guard. Never describe it as secure. Plan task 2.7 in extensions/mssql/docs/IMPLEMENTATION_PLAN.md; design in extensions/mssql/docs/DESIGN.md ("Minimal guard"). TDD.

Depends on: #319 (permanent corpora)

Documentation Required

None beyond the ADR and design. No parser documentation is needed.

Implementation Details

Create extensions/mssql/guard.ts exporting validateSql(sql: string): { allowed: true } | { allowed: false, reason: string }. No dependencies. Rules:

Wrap everything in try/catch. Any exception, non-string, empty or whitespace-only input, or very large input returns a rejection.
Strip, before inspecting: comments (-- and /* */, nested-comment aware), string literals (including '' escapes), and bracketed or double-quoted identifiers (including the ]] escape). Fail closed on an unterminated comment, string or identifier.
On the remaining text: allow one statement only (a single trailing ; is allowed; any other ; followed by content rejects). It must start with SELECT or WITH.
Reject whole-word (case-insensitive) INTO, EXEC, EXECUTE, OPENROWSET, OPENQUERY, OPENDATASOURCE, INSERT, UPDATE, DELETE, MERGE, DROP, ALTER, CREATE, TRUNCATE.
Keep reasons short and free of user data. Never rewrite or normalise the SQL that gets executed.

Known gaps, accepted and documented in a KNOWN_GAPS list in the test file: SELECT @var = ... assignment and UPDLOCK table hints. Neither is a write; they are left to the credentials, the row cap and the timeout. Known false rejects: an unbracketed column named like a denied keyword (for example update), FOR XML, temp tables.

Load the permanent corpora from extensions/mssql/test-data/ in guard.test.ts. Assert every must-reject case is rejected except the named known gaps, and every must-accept case is allowed (or is on a documented false-reject list). Add explicit cases: nested comments, a comment hiding a second statement, and denied keywords inside strings, comments and bracketed identifiers (allowed).

Acceptance Criteria

validateSql returns {allowed:true} only for one SELECT or WITH ... SELECT statement.
Every must-reject corpus entry is rejected, except entries on the documented KNOWN_GAPS list.
Every must-accept corpus entry is allowed, except entries on a documented false-reject list.
Nested comments are handled; unterminated comments, strings and identifiers are rejected.
Denied keywords inside strings, comments and bracketed identifiers do not cause a rejection.
SELECT INTO, EXEC, OPENROWSET, OPENQUERY, OPENDATASOURCE and stacked statements are rejected.
Exceptions, empty and non-string input all return a rejection.
The module never modifies the SQL it is given.
No new dependency is added, and no comment or doc calls the guard "secure".

Test Plan

node --test extensions/mssql/guard.test.ts. Also try by hand: SELECT 1 (allowed), SELECT 1; DROP TABLE t (rejected), /* x */ DELETE FROM t (rejected), /* a /* b */ c */ SELECT 1 (allowed), /* a /* b */ DELETE FROM t (rejected, unterminated), WITH c AS (SELECT 1) DELETE FROM t (rejected), SELECT 'update' (allowed), "" (rejected).

## Summary Create the `guard` module with `validateSql(sql)`: a minimal, dependency-free check that allows one `SELECT` / `WITH ... SELECT` statement and rejects the rest. It catches accidents. It is **not** a security boundary. ## Background Decision: `docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md`. Read-only database credentials are the write control; this guard is defence in depth against accidents only (for example an agent attempting an `UPDATE`). The Stage 1 spike showed no parser reached zero false accepts, so there is no parser and no new dependency. Zero false accepts is **not** a requirement. [SECURITY RISK] If the login can write, a crafted query can bypass this guard. Never describe it as secure. Plan task 2.7 in `extensions/mssql/docs/IMPLEMENTATION_PLAN.md`; design in `extensions/mssql/docs/DESIGN.md` ("Minimal guard"). TDD. **Depends on:** #319 (permanent corpora) ## Documentation Required None beyond the ADR and design. No parser documentation is needed. ## Implementation Details Create `extensions/mssql/guard.ts` exporting `validateSql(sql: string): { allowed: true } | { allowed: false, reason: string }`. No dependencies. Rules: 1. Wrap everything in try/catch. Any exception, non-string, empty or whitespace-only input, or very large input returns a rejection. 2. Strip, before inspecting: comments (`--` and `/* */`, **nested-comment aware**), string literals (including `''` escapes), and bracketed or double-quoted identifiers (including the `]]` escape). Fail closed on an unterminated comment, string or identifier. 3. On the remaining text: allow one statement only (a single trailing `;` is allowed; any other `;` followed by content rejects). It must start with `SELECT` or `WITH`. 4. Reject whole-word (case-insensitive) `INTO`, `EXEC`, `EXECUTE`, `OPENROWSET`, `OPENQUERY`, `OPENDATASOURCE`, `INSERT`, `UPDATE`, `DELETE`, `MERGE`, `DROP`, `ALTER`, `CREATE`, `TRUNCATE`. 5. Keep reasons short and free of user data. Never rewrite or normalise the SQL that gets executed. Known gaps, accepted and documented in a `KNOWN_GAPS` list in the test file: `SELECT @var = ...` assignment and `UPDLOCK` table hints. Neither is a write; they are left to the credentials, the row cap and the timeout. Known false rejects: an unbracketed column named like a denied keyword (for example `update`), `FOR XML`, temp tables. Load the permanent corpora from `extensions/mssql/test-data/` in `guard.test.ts`. Assert every must-reject case is rejected **except** the named known gaps, and every must-accept case is allowed (or is on a documented false-reject list). Add explicit cases: nested comments, a comment hiding a second statement, and denied keywords inside strings, comments and bracketed identifiers (allowed). ## Acceptance Criteria - [ ] `validateSql` returns `{allowed:true}` only for one `SELECT` or `WITH ... SELECT` statement. - [ ] Every must-reject corpus entry is rejected, except entries on the documented `KNOWN_GAPS` list. - [ ] Every must-accept corpus entry is allowed, except entries on a documented false-reject list. - [ ] Nested comments are handled; unterminated comments, strings and identifiers are rejected. - [ ] Denied keywords inside strings, comments and bracketed identifiers do not cause a rejection. - [ ] `SELECT INTO`, `EXEC`, `OPENROWSET`, `OPENQUERY`, `OPENDATASOURCE` and stacked statements are rejected. - [ ] Exceptions, empty and non-string input all return a rejection. - [ ] The module never modifies the SQL it is given. - [ ] No new dependency is added, and no comment or doc calls the guard "secure". ## Test Plan `node --test extensions/mssql/guard.test.ts`. Also try by hand: `SELECT 1` (allowed), `SELECT 1; DROP TABLE t` (rejected), `/* x */ DELETE FROM t` (rejected), `/* a /* b */ c */ SELECT 1` (allowed), `/* a /* b */ DELETE FROM t` (rejected, unterminated), `WITH c AS (SELECT 1) DELETE FROM t` (rejected), `SELECT 'update'` (allowed), `""` (rejected).

david added this to the MSSQL: M5 - Guard and audit log milestone

2026-11-10 08:20:06 +00:00

david added the

feature

label

2026-11-10 08:20:06 +00:00

david referenced this issue

2026-11-10 08:21:08 +00:00

MSSQL: mssql_query and mssql_query_one tools #326

david referenced this issue

2026-11-10 23:44:05 +00:00

MSSQL: extension skeleton, permanent corpora and dependencies #319