fix(qa): bind TEST_PUBKEY as a SQLite parameter instead of interpolating it

[anupamme]

Closes #1977. Supersedes #1952. Follow-up filed as #1983.

What §10.2 did

q="SELECT COUNT(*) FROM transmissions WHERE from_node = '$TEST_PUBKEY';"
qq=$(printf %q "$q")
if ! count=$(ssh_t "docker exec … sqlite3 … $qq" 2>/dev/null); then
  count=$(ssh_t "sqlite3 … $qq" 2>/dev/null || echo "")
fi

The injection is not reachable today — TEST_PUBKEY is hex-gated and the
script exit 2s before the SQL is built. The problem is that the SQL layer's
safety rests entirely on that outer gate rather than on the SQL layer itself.
#1952 proposed doubling embedded quotes; that is string escaping, not
parameterisation, which is why it was withdrawn in favour of this.

What this does

Per the four points in the sign-off on #1977:

1. Bind the value. A constant SELECT and a bound :pubkey, fed to
sqlite3 on stdin. The SQL no longer crosses the remote shell as a command
word, so there is no printf %q on the query at all any more.

Why hex rather than .parameter set :pk '<value>'. Dot-command arguments
are split on whitespace, so a payload containing a space produces too many
arguments — and sqlite3 responds by printing the .parameter help to
stdout, exiting 0, and leaving :pk unbound. COUNT(*) then
returns 0, which reads exactly like a passing security fix. -bail does not
catch it. Verified on 3.51.0:

$ printf ".parameter set :pk '' OR 1=1 --'\nSELECT COUNT(*) FROM transmissions WHERE from_node = :pk;\n" \
    | sqlite3 -bail ptest.db
.parameter CMD ...       Manage SQL parameter bindings     # <- help, on stdout
   …
0                                                          # <- :pk never bound
$ echo $?
0

.parameter set :pk 1+1 also binds the integer 2 — the value is evaluated
as an SQL expression and only falls back to a text literal when evaluation
fails. So interpolating into the .parameter set line trades one hazard for
another.

Hex-encoding removes the quoting layer instead of adding one: the value is
bound as cast(x'<hex>' as text), so its contribution to the SQL text is
drawn from the alphabet [0-9a-f] only. Nothing to quote, no tokenizer arity
hazard, and it holds for arbitrary input rather than only for hex-gated
input — which is the point.

Verified against a fixture table holding two rows, one of them deadbeef:

value	result	exit
deadbeef, bound as cast(x'6465616462656566' as text)	1	0
' OR 1=1 --, bound the same way	0	0
' OR 1=1 --, interpolated the current way	2 (whole table)	0
query against a DB with no transmissions table, -bail	Parse error … no such table on stderr	1

2. Probe the capability, not a version. resolve_sqlite_runner binds
corescope-probe-ok and asserts it comes back — a round trip, not a bare
.parameter init, so the positive control runs against the operator's actual
binary rather than one we pin. If neither the container nor the host qualifies,
it fails loudly and names what is needed:

  ❌ retain-failed: no sqlite3 able to bind a parameter on the target
     tried: docker exec -i corescope-prod sqlite3, then sqlite3 on runner@example
     need:  the sqlite3 CLI reachable over ssh, supporting '.parameter set'
OCI runtime exec failed: exec: "sqlite3": executable file not found in $PATH
bash: line 1: sqlite3: command not found

There is deliberately no interpolating fallback. That would leave the
vulnerable path in place under a nicer name.

3. The hex gate is kept, with its comment updated to say why: for the SQL
layer it is now defence in depth rather than the only guard. Redundant is not
the same as wrong.

4. The exit status and stderr survive. -batch -bail -init /dev/null -noheader -list (stop at the first SQL error; ignore the operator's
~/.sqliterc, where a stray .mode would make the count unparseable; stdout
is exactly the number). Query stderr is captured and printed on failure rather
than sent to /dev/null, so a broken query is distinguishable from a
legitimately empty result. Probe stderr is collected too, and printed only if
both probes fail — the container miss is the known-normal case, so surfacing
it on every run would be noise.

Also fixed

An existing double-count in §10.2: the TARGET_DB_PATH unset branch
incremented $fails and then left count="", so the generic branch
incremented it a second time for the same failure. read_retain_count now
gives §10.2 exactly one increment point. Opportunistic cleanup in a file
already being touched (AGENTS.md line 318).

Tests

New qa/scripts/test-blacklist-sql.sh, wired into the go-test job. 24
assertions, modelled on scripts/staging/test-disk-monitor.sh.

Both directions are asserted, because a zero from a command that failed proves
nothing:

• Positive control — deadbeef still returns its row (1, exit 0), and so
  does cafebabe; an absent pubkey returns 0.
• Negative — ' OR 1=1 -- returns 0 while the table demonstrably holds
  2 rows, and the old interpolated form is asserted to leak all 2. That last
  assertion is what makes the 0 above worth something.
• Error surfacing — the same SQL against a DB with no transmissions table
  exits non-zero with a message on stderr and nothing on stdout.
• Alphabet — sql_hex_literal output matches ^x'[0-9a-f]*'$ for the SQL
  payloads, a backslash, $(id) / backticks, an embedded newline, héllo, and
  a 4096-byte repetitive string. That last one is a regression guard for od -v: without the flag od collapses repeated identical lines to *.
• run_sqlite with no resolved runner refuses rather than guessing.

Group 2 skips loudly (rather than silently) if sqlite3 is not on PATH; group
1 needs no sqlite3 and always runs.

Mutation-tested — each of these breaks the suite, so the assertions have
teeth:

mutation	caught by
restore full interpolation	injection payload → 0 rows — expected '0' got '2'
naive .parameter set '%s'	expected '0' got '.parameter CMD ...'
drop od -v	alphabet failure on *, plus expected '8192' got '33'

Commit 1 is a behaviour-neutral refactor that moves the imperative body into
main() behind a BASH_SOURCE guard, so the test can source the script and
exercise individual helpers. Same idiom as scripts/staging/disk-monitor.sh:99.

Verification

• bash qa/scripts/test-blacklist-sql.sh → 24 passed, 0 failed
• bash -n on both scripts
• All three runtime paths exercised end to end with PATH shims for
  ssh/docker/sqlite3 against a real fixture DB: success
  (sqlite3 runner: host, count 2), query failure (classified message +
  Parse error … no such table, fails=1), and no-capability (the loud block
  above, both probe stderrs, fails=1 — not 2)
• The new step lands inside go-test, which runs when changes.outputs.code == 'true'; qa/scripts/*.sh does not match that job's
  ^docs/|[.]md$|^LICENSE$ documentation filter, so it is not skipped

Deliberately out of scope

• The docker exec branch is dead on current images → filed as blacklist-test.sh §10.2: the docker exec sqlite3 branch can never succeed — the app image has no sqlite3 #1983. The
  app container has no sqlite3 at all: Dockerfile:15 is pure-Go SQLite with
  no CGO, and the apk add installs only mosquitto mosquitto-clients supervisor caddy wget. So the host "fallback" is the only path that has ever
  executed, silently, because both branches discarded stderr. This change keeps
  both branches and merely makes the outcome visible (sqlite3 runner: … on
  every run).
• -readonly on the target DB. Tempting, and verified compatible with
  .parameter (the binding table lives in the TEMP database), but a WAL
  database needing journal recovery can refuse a read-only open. Adding it here
  risks exactly the "trades an unreachable injection for a script that does not
  run" outcome flagged in the fix: fix security issue in blacklist-test.sh #1952 thread. Worth its own issue.
• The other 2>/dev/null sites in this file, which also sit awkwardly with
  qa/README.md's "Don't silence stderr". Only the §10.2 lines named in the
  sign-off are touched.

🤖 Generated with Claude Code