mssql: the query runs as a read-only login, one line, no variables; sqlcmd is installed (hq #193)
Proven on a throwaway server: as the administrator a caller's $(SQLCMDPASSWORD) returned the sa password, and a line beginning ':!!' ran a program in the tools container. The statement now runs as mesh_mssql_reader (CONNECT ANY DATABASE, SELECT ALL USER SECURABLES), with substitution off (-x), after the module's own text on the first line, and a line break is refused. go-sqlcmd v1.10.0 is installed at a pinned digest: the image never had sqlcmd, so every mssql tool failed with spawn sqlcmd ENOENT.
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
// What holds mssql_query to being read-only (novox/hq issue 193): a caller's statement runs as the
|
||||
// reader login and never as the administrator, with sqlcmd's variable substitution off, on one line
|
||||
// that follows the module's own — a line break is refused before sqlcmd starts — and with no
|
||||
// transaction wrapped around it as text. Without the reader's password the statement is refused.
|
||||
//
|
||||
// sqlcmd is a fake on PATH that records each call's login, flags and text. That the reader cannot
|
||||
// write is the server's to enforce and was proven against a real server; this holds the module to
|
||||
// asking for it. Run against the compiled module (npm test builds first), the way the runtime loads it.
|
||||
|
||||
import { test, before, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { MssqlClient, READER } from "../dist/client.js";
|
||||
|
||||
let dir: string;
|
||||
let log: string;
|
||||
const originalPath = process.env.PATH;
|
||||
|
||||
before(async () => {
|
||||
dir = await mkdtemp(join(tmpdir(), "mssql-reader-"));
|
||||
log = join(dir, "calls.jsonl");
|
||||
await writeFile(join(dir, "sqlcmd"), `#!/usr/bin/env node
|
||||
const fs = require("node:fs");
|
||||
const args = process.argv.slice(2);
|
||||
const at = (flag) => args[args.indexOf(flag) + 1];
|
||||
fs.appendFileSync(${JSON.stringify(log)}, JSON.stringify({
|
||||
user: at("-U"), database: at("-d"), sql: at("-Q"), noVariables: args.includes("-x"),
|
||||
password: process.env.SQLCMDPASSWORD,
|
||||
}) + "\\n");
|
||||
const sql = at("-Q");
|
||||
if (/FROM sys.server_principals/.test(sql)) process.stdout.write("");
|
||||
else if (/FOR JSON/.test(sql)) process.stdout.write('[{"name":"alpha","n":1}]\\n');
|
||||
`);
|
||||
await chmod(join(dir, "sqlcmd"), 0o755);
|
||||
process.env.PATH = `${dir}:${originalPath}`;
|
||||
});
|
||||
|
||||
after(async () => {
|
||||
process.env.PATH = originalPath;
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function calls(): Promise<Record<string, unknown>[]> {
|
||||
const text = await readFile(log, "utf8").catch(() => "");
|
||||
await writeFile(log, "");
|
||||
return text.split("\n").filter(Boolean).map((line) => JSON.parse(line));
|
||||
}
|
||||
|
||||
const conn = { host: "127.0.0.1", port: 1433, user: "sa", password: "admin-secret" };
|
||||
|
||||
test("a caller's statement runs as the reader, without variables, on the module's first line", async () => {
|
||||
const client = new MssqlClient({ ...conn, readerPassword: "reader-secret" });
|
||||
const result = await client.readOnlyQuery("inventory", "SELECT '$(SQLCMDPASSWORD)' AS p");
|
||||
|
||||
const asked = (await calls()).at(-1)!;
|
||||
assert.equal(asked.user, READER, "the statement never runs as the administrator");
|
||||
assert.equal(asked.password, "reader-secret");
|
||||
assert.equal(asked.noVariables, true, "no $(NAME) is substituted in a caller's text");
|
||||
const [first] = String(asked.sql).split("\n");
|
||||
assert.ok(first.startsWith("SET NOCOUNT ON; SELECT '$(SQLCMDPASSWORD)'"), "the caller's text never begins a line");
|
||||
assert.doesNotMatch(String(asked.sql), /BEGIN TRANSACTION|ROLLBACK/, "no transaction wrapped around it as text");
|
||||
assert.deepEqual(result.rows, [{ name: "alpha", n: 1 }]);
|
||||
assert.equal(result.command, "SELECT");
|
||||
});
|
||||
|
||||
test("a line break in a caller's statement is refused before sqlcmd starts", async () => {
|
||||
const client = new MssqlClient({ ...conn, readerPassword: "reader-secret" });
|
||||
for (const sql of ["SELECT 1\n:!! id", "SELECT 1\r\n:!! id", "SELECT 1\r:!! id"]) {
|
||||
await assert.rejects(client.readOnlyQuery("inventory", sql), /must be one line/);
|
||||
}
|
||||
assert.deepEqual(await calls(), []);
|
||||
});
|
||||
|
||||
test("the reader is made as the administrator, kept out of sysadmin, and granted only reading", async () => {
|
||||
const client = new MssqlClient({ ...conn, readerPassword: "reader-secret" });
|
||||
await client.readOnlyQuery("inventory", "SELECT 1 AS x");
|
||||
await client.readOnlyQuery("inventory", "SELECT 2 AS x");
|
||||
|
||||
const made = await calls();
|
||||
const asAdmin = made.filter((c) => c.user === "sa").map((c) => String(c.sql));
|
||||
assert.ok(asAdmin.some((s) => s.startsWith(`CREATE LOGIN [${READER}]`)));
|
||||
assert.ok(asAdmin.some((s) => /ALTER SERVER ROLE sysadmin DROP MEMBER/.test(s)));
|
||||
assert.ok(asAdmin.includes(`GRANT CONNECT ANY DATABASE TO [${READER}]`));
|
||||
assert.ok(asAdmin.includes(`GRANT SELECT ALL USER SECURABLES TO [${READER}]`));
|
||||
assert.equal(asAdmin.filter((s) => s.startsWith("CREATE LOGIN")).length, 1, "made once, not per call");
|
||||
assert.equal(made.filter((c) => c.user === READER).length, 2);
|
||||
});
|
||||
|
||||
test("without the reader's password the statement is refused, and nothing runs as the administrator", async () => {
|
||||
const client = new MssqlClient(conn);
|
||||
await assert.rejects(client.readOnlyQuery("inventory", "SELECT 1"), /refused rather than run as the administrator/);
|
||||
assert.deepEqual(await calls(), []);
|
||||
});
|
||||
Reference in New Issue
Block a user