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

New issue

Closed

opened 2026-11-10 08:21:50 +00:00 by david · 1 comment

david commented

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

Owner

Summary

Wire extensions/mssql/index.ts: load env, return early without MSSQL_URL, refuse to load on an invalid limit, create the pool, print host/port/database only, run the write-access check (warn, never block), register the tools, and end the pool on session_shutdown.

Background

Behaviour from the design: the extension registers no tools when MSSQL_URL is absent; if MSSQL_MAX_ROWS or MSSQL_QUERY_TIMEOUT_MS is set but invalid, the extension does not load and prints an error naming the variable; at startup it prints only host, port and database, never the password or full URL ([SECURITY RISK]); the pool ends on session_shutdown. Per docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md, after the pool connects the extension runs the write-access check from #342. If the login appears to have write rights, or the check cannot verify, it prints a warning and writes a permission_warning audit entry. It never blocks: the extension still loads and registers all tools. extensions/postgres/index.ts is the model: default export (pi: ExtensionAPI) => {...}, pi.on("session_shutdown", async () => { await pool.end(); }). Stage 2 is unblocked (#318 resolved as STOP, superseded by ADR 0002). TDD with an injectable pool factory so tests need no live server.

Depends on: #328 (and #321, #324, #323, #342)

Documentation Required

docs/reference/mssql/ — https://raw.githubusercontent.com/tediousjs/node-mssql/master/README.md : node-mssql README: new sql.ConnectionPool(config), connect(), close(). Vendored on main; fetch the URL if absent.

Implementation Details

In index.ts export the default function plus a testable inner function that accepts { env, createPool }. Flow: loadEnvFile() merged with relevant process.env; no MSSQL_URL → return with no output and no tools; invalid URL → print an error that does not echo the URL and register nothing; invalid limits or trust value (from loadSettings) → print the error naming the variable and register nothing; otherwise build config with buildPoolConfig, create and connect the pool (handle connect failure with a message free of credentials), print mssql: <host>:<port>/<database>, call checkWriteAccess(pool) (#342): on write-access or unverified, print formatWarning(result) and call audit.log({ outcome: "permission_warning", ... }); then call registerMssqlTools, and register pi.on("session_shutdown", ...) to close the pool. A failure inside the permission check or the audit write must not stop tool registration.

Acceptance Criteria

No MSSQL_URL → no tools registered, no pool created.
Invalid MSSQL_MAX_ROWS / MSSQL_QUERY_TIMEOUT_MS → nothing registered and the error names the variable.
Startup output contains host, port and database only, plus any permission warning (test asserts the password and full URL are absent from all output).
All four tools are registered on a valid config, including when the login has write rights and when the permission check fails.
A write-access result prints a warning naming the rights and writes a permission_warning audit entry; no result blocks loading.
The pool is closed on session_shutdown.

Test Plan

node --test extensions/mssql/index.test.ts with a fake pi, fake pool factory, fake permission result and captured console output: cover each bullet above, including a connect failure, write rights found, and a permission-check error.

## Summary Wire `extensions/mssql/index.ts`: load env, return early without `MSSQL_URL`, refuse to load on an invalid limit, create the pool, print host/port/database only, run the write-access check (warn, never block), register the tools, and end the pool on `session_shutdown`. ## Background Behaviour from the design: the extension registers no tools when `MSSQL_URL` is absent; if `MSSQL_MAX_ROWS` or `MSSQL_QUERY_TIMEOUT_MS` is set but invalid, the extension does not load and prints an error naming the variable; at startup it prints only host, port and database, never the password or full URL ([SECURITY RISK]); the pool ends on `session_shutdown`. Per `docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md`, after the pool connects the extension runs the write-access check from #342. If the login appears to have write rights, or the check cannot verify, it prints a warning and writes a `permission_warning` audit entry. **It never blocks**: the extension still loads and registers all tools. `extensions/postgres/index.ts` is the model: default export `(pi: ExtensionAPI) => {...}`, `pi.on("session_shutdown", async () => { await pool.end(); })`. Stage 2 is unblocked (#318 resolved as STOP, superseded by ADR 0002). TDD with an injectable pool factory so tests need no live server. **Depends on:** #328 (and #321, #324, #323, #342) ## Documentation Required - `docs/reference/mssql/` — https://raw.githubusercontent.com/tediousjs/node-mssql/master/README.md : node-mssql README: `new sql.ConnectionPool(config)`, `connect()`, `close()`. Vendored on `main`; fetch the URL if absent. ## Implementation Details In `index.ts` export the default function plus a testable inner function that accepts `{ env, createPool }`. Flow: `loadEnvFile()` merged with relevant `process.env`; no `MSSQL_URL` → return with no output and no tools; invalid URL → print an error that does not echo the URL and register nothing; invalid limits or trust value (from `loadSettings`) → print the error naming the variable and register nothing; otherwise build config with `buildPoolConfig`, create and connect the pool (handle connect failure with a message free of credentials), print `mssql: <host>:<port>/<database>`, call `checkWriteAccess(pool)` (#342): on `write-access` or `unverified`, print `formatWarning(result)` and call `audit.log({ outcome: "permission_warning", ... })`; then call `registerMssqlTools`, and register `pi.on("session_shutdown", ...)` to close the pool. A failure inside the permission check or the audit write must not stop tool registration. ## Acceptance Criteria - [ ] No `MSSQL_URL` → no tools registered, no pool created. - [ ] Invalid `MSSQL_MAX_ROWS` / `MSSQL_QUERY_TIMEOUT_MS` → nothing registered and the error names the variable. - [ ] Startup output contains host, port and database only, plus any permission warning (test asserts the password and full URL are absent from all output). - [ ] All four tools are registered on a valid config, including when the login has write rights and when the permission check fails. - [ ] A write-access result prints a warning naming the rights and writes a `permission_warning` audit entry; no result blocks loading. - [ ] The pool is closed on `session_shutdown`. ## Test Plan `node --test extensions/mssql/index.test.ts` with a fake pi, fake pool factory, fake permission result and captured console output: cover each bullet above, including a connect failure, write rights found, and a permission-check error.

david added this to the MSSQL: M8 - Wiring and docs milestone

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

david added the

feature

label

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

david referenced this issue

2026-11-10 08:22:02 +00:00

MSSQL: register extension, add tests to root test script, add root README entry #330

david referenced this issue

2026-11-10 23:43:27 +00:00

MSSQL: write-access check and startup warning (never blocks) #342