david/pi-extensions-and-skills

Watch

Fork 0

MSSQL: certificate trust setting and limit validation (MSSQL_TRUST_SERVER_CERT, MSSQL_MAX_ROWS, MSSQL_QUERY_TIMEOUT_MS) #321

New issue

Closed

opened 2026-11-10 08:19:49 +00:00 by david · 2 comments

david commented

2026-11-10 08:19:49 +00:00

Owner

Summary

Parse MSSQL_TRUST_SERVER_CERT, MSSQL_MAX_ROWS and MSSQL_QUERY_TIMEOUT_MS in extensions/mssql/env.ts, refusing invalid limits.

Background

Design decisions: MSSQL_TRUST_SERVER_CERT is true or false, default false (verify the certificate); true skips the server identity check and allows interception, so only for known self-signed servers ([SECURITY RISK]). MSSQL_MAX_ROWS defaults to 1000 and MSSQL_QUERY_TIMEOUT_MS to 30000. If a limit is set but is not a positive integer, the extension must not load and must print an error naming the variable; 0 does not lift a limit. encrypt is always on and has no setting. Blocked until the Stage 1 gate (#318) passes. TDD.

Depends on: #320

Implementation Details

In extensions/mssql/env.ts add parseTrustServerCert(env), parseLimits(env) (or one loadSettings(env) returning { trustServerCert, maxRows, queryTimeoutMs } or an error). Rules:

Trust cert: unset → false; case-insensitive "true"/"false"; any other value is an error naming MSSQL_TRUST_SERVER_CERT (fail closed to verification, never silently true).
Limits: unset → default; set → must match ^[1-9][0-9]*$ and be a safe integer; else return an error "MSSQL_MAX_ROWS must be a positive integer" (variable name included, value not required). Empty string counts as set and invalid.
Return a discriminated result ({ ok: true, settings } / { ok: false, error }) so index.ts can refuse to load.

Acceptance Criteria

Defaults: false, 1000, 30000 when the variables are unset.
true/false (any case) parse; other values return an error naming the variable.
0, negatives, decimals, non-numbers, whitespace and empty strings return an error naming the variable.
Valid positive integers are accepted.
Errors never include MSSQL_URL or credentials.

Test Plan

node --test extensions/mssql/env.test.ts: table-driven cases for each variable: unset, valid, 0, -5, 1.5, abc, "", " 10 ", 1e3, huge numbers, and the trust values TRUE, yes.

## Summary Parse `MSSQL_TRUST_SERVER_CERT`, `MSSQL_MAX_ROWS` and `MSSQL_QUERY_TIMEOUT_MS` in `extensions/mssql/env.ts`, refusing invalid limits. ## Background Design decisions: `MSSQL_TRUST_SERVER_CERT` is `true` or `false`, default `false` (verify the certificate); `true` skips the server identity check and allows interception, so only for known self-signed servers ([SECURITY RISK]). `MSSQL_MAX_ROWS` defaults to 1000 and `MSSQL_QUERY_TIMEOUT_MS` to 30000. If a limit is set but is not a positive integer, the extension must not load and must print an error naming the variable; `0` does not lift a limit. `encrypt` is always on and has no setting. **Blocked until the Stage 1 gate (#318) passes.** TDD. **Depends on:** #320 ## Implementation Details In `extensions/mssql/env.ts` add `parseTrustServerCert(env)`, `parseLimits(env)` (or one `loadSettings(env)` returning `{ trustServerCert, maxRows, queryTimeoutMs }` or an error). Rules: - Trust cert: unset → `false`; case-insensitive `"true"`/`"false"`; any other value is an error naming `MSSQL_TRUST_SERVER_CERT` (fail closed to verification, never silently true). - Limits: unset → default; set → must match `^[1-9][0-9]*$` and be a safe integer; else return an error `"MSSQL_MAX_ROWS must be a positive integer"` (variable name included, value not required). Empty string counts as set and invalid. - Return a discriminated result (`{ ok: true, settings }` / `{ ok: false, error }`) so `index.ts` can refuse to load. ## Acceptance Criteria - [ ] Defaults: `false`, `1000`, `30000` when the variables are unset. - [ ] `true`/`false` (any case) parse; other values return an error naming the variable. - [ ] `0`, negatives, decimals, non-numbers, whitespace and empty strings return an error naming the variable. - [ ] Valid positive integers are accepted. - [ ] Errors never include `MSSQL_URL` or credentials. ## Test Plan `node --test extensions/mssql/env.test.ts`: table-driven cases for each variable: unset, valid, `0`, `-5`, `1.5`, `abc`, `""`, `" 10 "`, `1e3`, huge numbers, and the trust values `TRUE`, `yes`.

david added this to the MSSQL: M4 - Scaffold and config milestone

2026-11-10 08:19:49 +00:00

david added the

feature

label

2026-11-10 08:19:49 +00:00

~~david referenced this issue 2026-11-10 08:20:06 +00:00~~

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

david referenced this issue

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

MSSQL: audit log writer (.pi/mssql-audit.log, mode 0600) #323

david referenced this issue

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

MSSQL: pool config builder (encrypt on, certificate setting, timeout) #324

david referenced this issue

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

MSSQL: index.ts wiring (load env, refuse on invalid limits, pool, shutdown) #329

david commented

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

Author

Owner

Update: the "Blocked until the Stage 1 gate (#318) passes" note is superseded. The gate resolved as STOP, and docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md replaced the parser design with read-only credentials, a minimal guard and a write-access warning. Stage 2 is unblocked. The scope of this issue is unchanged.

Update: the "Blocked until the Stage 1 gate (#318) passes" note is superseded. The gate resolved as STOP, and `docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md` replaced the parser design with read-only credentials, a minimal guard and a write-access warning. Stage 2 is unblocked. The scope of this issue is unchanged.

david referenced this issue

2026-11-10 23:46:59 +00:00

MSSQL spike: write RESULT.md, apply the gate, open the Stage 1 PR #318

david referenced this issue

2026-11-11 00:04:31 +00:00

321: MSSQL: certificate trust setting and limit validation (MSSQL_TRUST_SERVER_CERT, MSSQL_MAX_ROWS, MSSQL_QUERY_TIMEOUT_MS) #345

david closed this issue

2026-11-11 00:04:33 +00:00

david commented

2026-11-11 00:04:33 +00:00

Author

Owner