MSSQL: write extensions/mssql/README.md (setup, tools, risks) #331

New issue

Closed

opened 2026-11-10 08:22:15 +00:00 by david · 1 comment

david commented

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

Owner

Summary

Write extensions/mssql/README.md covering setup, the tools, and every security and compliance caveat of the extension.

Background

The extension gives the agent SQL Server query access. Read-only database credentials are the write control (docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md). A minimal guard catches accidents, and a startup check warns (never blocks) if the login can write. Users must be told these limits plainly. Use Australian English; tag risks inline with [SECURITY RISK], [COMPLIANCE RISK] and [BEST PRACTICE]. Do not describe the guard as "secure" or as a "write block". Stage 2 is unblocked (#318 resolved as STOP, superseded by ADR 0002).

Depends on: #330

Implementation Details

The README must contain these sections and statements:

Setup: .env variables and defaults: MSSQL_URL (mssql://user:pass@host:1433/db, required, port default 1433), MSSQL_TRUST_SERVER_CERT (true/false, default false), MSSQL_MAX_ROWS (default 1000), MSSQL_QUERY_TIMEOUT_MS (default 30000); invalid limits stop the extension loading; encrypt is always on. Keep credentials in .env or a vault; never hard-code them.
Tools: mssql_query, mssql_query_one, mssql_list_tables, mssql_table_schema, with what each does (schema tools use fixed parameterised queries and INFORMATION_SCHEMA only shows objects the login can access).
[BEST PRACTICE] Use a read-only login. This is the real write control. Give the DBA checklist: db_datareader only; no db_datawriter, db_owner, db_ddladmin or server roles; no EXECUTE on procedures that write; access limited to the schemas or views needed. Mention ApplicationIntent=ReadOnly against a replica where one exists.
[SECURITY RISK] The guard is a minimal accident check, not a security boundary. If the login can write, a crafted query can bypass it.
Write-access warning: what the startup check looks at (sysadmin, db_owner, db_datawriter, db_ddladmin, database-level INSERT, UPDATE, DELETE, ALTER, EXECUTE), that it warns and never blocks, that it also writes a permission_warning audit entry, and that table-level grants and ownership chaining are not detected, so a clean result does not prove the login is read-only.
A read-only login does not stop expensive reads or locking hints (UPDLOCK, HOLDLOCK); the row cap and timeout limit the impact.
MSSQL_TRUST_SERVER_CERT=true skips the server identity check; only for known self-signed servers.
Audit log: location .pi/mssql-audit.log, contents (timestamp, SQL, outcome, reason, row count, duration, truncated, and permission warnings), mode 0600, no rotation, and how to delete or archive it; never contains the password or URL.
[COMPLIANCE RISK] The log and query results can hold patron data (Privacy Act 1988, APPs); grant least-privilege access; add .pi/ to .gitignore; check retention with the privacy team. Recommend a qualified practitioner review and a cross-check against ISO 27001 and Essential Eight controls before production use.
Some valid SELECT statements are rejected (table hints, FOR XML, temp tables, unbracketed identifiers named like a denied keyword). Known accepted gaps: SELECT @var = ... and UPDLOCK hints.
Row cap and timeout, and the limits of stream-and-cancel (long time to first row on sort/aggregate, cancel not instant).
Tests use a fake pool and do not cover a live SQL Server.
Link to docs/DESIGN.md and ADR 0002.

Acceptance Criteria

Every section and statement above is present.
The risk tags appear inline against the right items.
The documented defaults match the code.
The README recommends adding .pi/ to .gitignore.
It names read-only credentials as the write control, and does not call the guard "secure" or a "write block".
The permission-check limits are stated.

Test Plan

Follow the setup section on a clean machine with a test SQL Server and confirm the extension loads. Cross-check each default in the README against env.ts. Review the risk list against docs/DESIGN.md "Risks and accepted residuals".

## Summary Write `extensions/mssql/README.md` covering setup, the tools, and every security and compliance caveat of the extension. ## Background The extension gives the agent SQL Server query access. **Read-only database credentials are the write control** (`docs/adr/0002-mssql-read-only-credentials-with-minimal-guard.md`). A minimal guard catches accidents, and a startup check warns (never blocks) if the login can write. Users must be told these limits plainly. Use Australian English; tag risks inline with `[SECURITY RISK]`, `[COMPLIANCE RISK]` and `[BEST PRACTICE]`. Do not describe the guard as "secure" or as a "write block". Stage 2 is unblocked (#318 resolved as STOP, superseded by ADR 0002). **Depends on:** #330 ## Implementation Details The README must contain these sections and statements: - **Setup:** `.env` variables and defaults: `MSSQL_URL` (`mssql://user:pass@host:1433/db`, required, port default 1433), `MSSQL_TRUST_SERVER_CERT` (`true`/`false`, default `false`), `MSSQL_MAX_ROWS` (default 1000), `MSSQL_QUERY_TIMEOUT_MS` (default 30000); invalid limits stop the extension loading; `encrypt` is always on. Keep credentials in `.env` or a vault; never hard-code them. - **Tools:** `mssql_query`, `mssql_query_one`, `mssql_list_tables`, `mssql_table_schema`, with what each does (schema tools use fixed parameterised queries and `INFORMATION_SCHEMA` only shows objects the login can access). - **[BEST PRACTICE] Use a read-only login. This is the real write control.** Give the DBA checklist: `db_datareader` only; no `db_datawriter`, `db_owner`, `db_ddladmin` or server roles; no `EXECUTE` on procedures that write; access limited to the schemas or views needed. Mention `ApplicationIntent=ReadOnly` against a replica where one exists. - **[SECURITY RISK]** The guard is a minimal accident check, not a security boundary. If the login can write, a crafted query can bypass it. - **Write-access warning:** what the startup check looks at (sysadmin, `db_owner`, `db_datawriter`, `db_ddladmin`, database-level `INSERT`, `UPDATE`, `DELETE`, `ALTER`, `EXECUTE`), that it warns and never blocks, that it also writes a `permission_warning` audit entry, and that table-level grants and ownership chaining are not detected, so a clean result does not prove the login is read-only. - A read-only login does not stop expensive reads or locking hints (`UPDLOCK`, `HOLDLOCK`); the row cap and timeout limit the impact. - `MSSQL_TRUST_SERVER_CERT=true` skips the server identity check; only for known self-signed servers. - **Audit log:** location `.pi/mssql-audit.log`, contents (timestamp, SQL, outcome, reason, row count, duration, truncated, and permission warnings), mode 0600, no rotation, and how to delete or archive it; never contains the password or URL. - **[COMPLIANCE RISK]** The log and query results can hold patron data (Privacy Act 1988, APPs); grant least-privilege access; add `.pi/` to `.gitignore`; check retention with the privacy team. Recommend a qualified practitioner review and a cross-check against ISO 27001 and Essential Eight controls before production use. - Some valid `SELECT` statements are rejected (table hints, `FOR XML`, temp tables, unbracketed identifiers named like a denied keyword). Known accepted gaps: `SELECT @var = ...` and `UPDLOCK` hints. - Row cap and timeout, and the limits of stream-and-cancel (long time to first row on sort/aggregate, cancel not instant). - Tests use a fake pool and do not cover a live SQL Server. - Link to `docs/DESIGN.md` and ADR 0002. ## Acceptance Criteria - [ ] Every section and statement above is present. - [ ] The risk tags appear inline against the right items. - [ ] The documented defaults match the code. - [ ] The README recommends adding `.pi/` to `.gitignore`. - [ ] It names read-only credentials as the write control, and does not call the guard "secure" or a "write block". - [ ] The permission-check limits are stated. ## Test Plan Follow the setup section on a clean machine with a test SQL Server and confirm the extension loads. Cross-check each default in the README against `env.ts`. Review the risk list against `docs/DESIGN.md` "Risks and accepted residuals".

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

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

david added the

documentation

label

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

david referenced this issue

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

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

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