113 lines
7.0 KiB
Markdown
113 lines
7.0 KiB
Markdown
---
|
|
status: resolved
|
|
opened: 2026-10-02
|
|
located-in: [mesh-catalog modules/postgres/client.ts (readOnlyQuery)]
|
|
fixed-by: [mesh-catalog PR 209 (postgres), mesh-catalog PR 210 (mssql)]
|
|
amended-design:
|
|
---
|
|
|
|
# 193 — The store seat's read-only query is read-only by convention, and its answer is unreadable
|
|
|
|
## What was observed
|
|
|
|
Asking the store seat's `query` verb for a count through the console returned this. Rows are each
|
|
wrapped in an object under a key named `BEGIN`: the column name, then the value, then the word
|
|
`ROLLBACK`. A query returning nothing gave the column name and `ROLLBACK` alone. The answer to
|
|
`select count(*) as n from <table>` was:
|
|
|
|
> `rows: [ {BEGIN: "n"}, {BEGIN: "46"}, {BEGIN: "ROLLBACK"} ]`
|
|
|
|
A reader can work it out. A program cannot, and a query with two columns loses which value belongs to
|
|
which.
|
|
|
|
## Why this is here
|
|
|
|
**The cause is the same line that makes the query read-only.** The holder's tool sends
|
|
`BEGIN TRANSACTION READ ONLY; <the caller's statement>; ROLLBACK;` to the command-line client as one
|
|
string. The client prints a command tag for each of the three statements, and the parser takes the
|
|
first line, `BEGIN`, as the header.
|
|
|
|
**And it is not read-only.** [ADR 0159](../../02-DECISIONS/0159-a-tool-call-names-the-machine-and-a-holder-serves-its-seats-verbs.md)
|
|
decided the store seat's `query` verb is "one read-only statement against one database". The only
|
|
thing enforcing that is the wrapping transaction, and the caller's statement is pasted inside it as
|
|
text. A statement that begins by ending the transaction (a commit, then anything) runs whatever
|
|
follows it outside the read-only transaction, with the holder's own role, the administrative one that creates every
|
|
consumer's role and database. A rule stated in a decision and enforced by string concatenation is enforced by nothing.
|
|
|
|
*This is read from the code, not tried against the live store, and it should not be tried there.*
|
|
The lab bed is where it gets proven.
|
|
|
|
Every caller with `invokes` on the store seat's `query` can do this. The console has `invokes: ["*"]`,
|
|
so that includes anyone logged in on a machine running the console.
|
|
|
|
## What a fix looks like
|
|
|
|
- **One statement, refused otherwise.** Send the caller's statement alone, through the client's
|
|
single-statement path (the extended protocol takes one statement per call and refuses more). The
|
|
read-only property then comes from the session, not from text around the statement.
|
|
- **Read-only by role, not by transaction.** Run the verb as a role that can only read, granted
|
|
`pg_read_all_data`, not as the administrative role. A statement that escapes every wrapper still cannot write.
|
|
- **Rows as rows.** Parse the client's output with the column names it returns, or use a driver
|
|
instead of the command-line client, so a row is an object keyed by its columns.
|
|
- **The check 0159 lacks:** a test that sends a commit followed by a write and asserts the write is
|
|
refused and nothing changed. Another asserts a two-column row comes back keyed by both columns.
|
|
|
|
## Proven, 2026-10-02
|
|
|
|
On a throwaway server — the same engine image, no network, reached over a socket — the module's code
|
|
from the catalogue's main branch ran `COMMIT; COPY (select 1) TO PROGRAM '<a command>'` and **the
|
|
command ran on the database host** as the server's own user. `COMMIT; DROP TABLE t` executed the drop
|
|
outside the read-only transaction; the wrapper's own trailing rollback happened to undo it, which a
|
|
caller ending their statement with a commit of their own would get past (not tried). Nothing was tried
|
|
against the live store.
|
|
|
|
The fix (mesh-catalog PR 209) runs the caller's statement as a login granted `pg_read_all_data` and
|
|
nothing else, read-only by its role and its session, with a password the mesh mints as one of the
|
|
module's own secrets; without that password the call is refused rather than run as the admin. On the
|
|
same throwaway server every escape above, and `SET ROLE`, `RESET SESSION AUTHORIZATION`, turning
|
|
read-only off, creating a table, altering the role and reading a server file, is refused; a plain
|
|
select comes back keyed by its columns. One attempt — turning the transaction's read-only off, then
|
|
deleting — got past the first layer and was stopped by the second, which is why both exist.
|
|
|
|
**Not answered by the statement-count fix proposed above.** The command-line client sends one string
|
|
in one message, so several statements still arrive together. They are harmless as the reader, and
|
|
refusing them is left to whoever moves the module to a driver.
|
|
|
|
## The same hole, elsewhere — and two worse ones
|
|
|
|
The `mssql` module wrapped a caller's statement the same way (`BEGIN TRANSACTION; … ROLLBACK;` as its
|
|
administrator) for its `mssql_query` tool. Its command-line client added two holes of its own. Both
|
|
were proven on a throwaway server, running the client the way the module ran it:
|
|
|
|
- **It substitutes `$(NAME)` from its environment into the caller's text**, and the administrator's
|
|
password is in that environment. Selecting it as a string returned the password.
|
|
- **It reads a line beginning `:!!` as a command that starts a program**, in the container that holds
|
|
the administrator's password and the module's bus credentials. Its switch for refusing such commands
|
|
makes the shipped version ignore the statement entirely, so the switch cannot be the guard.
|
|
|
|
None of it was reachable on the live mesh, for a reason that is a defect of its own: the runtime image
|
|
never installed the client, so every mssql tool failed (`spawn sqlcmd ENOENT`). The fix (mesh-catalog
|
|
PR 210) installs the client at a pinned digest and runs the caller's statement as a login that can
|
|
connect and read and do nothing else. Substitution is off. The statement must be one line, placed after
|
|
the module's own text, so no line of it can begin a command; a line break is refused before the client
|
|
starts. On the throwaway server, writes, `xp_cmdshell`, impersonating the administrator, and joining
|
|
the administrators' role were all refused, and the variable came back as the literal text.
|
|
|
|
**The general lesson**, worth more than either module: *a command-line client is an interpreter with
|
|
its own syntax, and a caller's text handed to it is a program in that syntax as well as in SQL.* A
|
|
module that passes a caller's text to a client has two languages to defend, and a transaction drawn
|
|
around the text defends neither.
|
|
|
|
## Resolved, 2026-10-02
|
|
|
|
Both pull requests merged, built and pushed to the two machines that run each module. Checked live, on
|
|
every copy, by asking each one who it is:
|
|
|
|
- the store seat's `query`, and postgres's own tool on each machine, answer as the reader login —
|
|
not a superuser, in a read-only transaction — with rows keyed by their columns;
|
|
- mssql's tool, on each machine, answers as its reader login, outside the administrators' role, and
|
|
returns `$(SQLCMDPASSWORD)` as the literal text it is. Its tools work for the first time.
|
|
|
|
The escapes themselves were tried only on the throwaway servers above; on the live mesh the check is
|
|
the identity a statement runs as, which is what makes every escape a statement that the login cannot do.
|