Skip to content

fix: run ops psql as trakrf-migrate, add psql-super and a QUERY argument (TRA-1105) - #164

Merged
mikestankavich merged 5 commits into
mainfrom
fix/tra-1105-ops-psql-connects-as-superuser-which-sil
Aug 19, 2026
Merged

fix: run ops psql as trakrf-migrate, add psql-super and a QUERY argument (TRA-1105)#164
mikestankavich merged 5 commits into
mainfrom
fix/tra-1105-ops-psql-connects-as-superuser-which-sil

Conversation

@mikestankavich

@mikestankavich mikestankavich commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

just psql connected as the postgres superuser, so any DDL hand-run through it created a postgres-owned object in the trakrf schema. Those objects are permanently un-replaceable by the migrate role — CREATE OR REPLACE, DROP and ALTER … OWNER TO all require ownership — so a later migration aborts partway and leaves the golang-migrate ledger dirty. Because the migrate Job is an ArgoCD PreSync hook, the Deployment is then never updated: the old pod keeps serving while ArgoCD reports Healthy and CI stays green. That is how preview wedged in TRA-1104.

What changed

New db_psql helper in scripts/ops-lib.sh runs psql through env PGOPTIONS='-c role=<role>', which applies the equivalent of SET ROLE at connect time. Connection auth is unchanged (in-pod peer auth as postgres over the unix socket — the only credential available without threading a password in). What changes is the role the session runs as, and therefore who owns anything it creates.

Recipe Role Prod guard
just psql ENV [QUERY] trakrf-migrate none (unchanged)
just psql-super ENV [QUERY] postgres superuser confirm_prod, fails closed without a tty

psql also gained the QUERY argument the v1.4.0 release needed: empty opens an interactive shell, non-empty runs psql -c with ON_ERROR_STOP=1 and no tty. The → namespace/pod banner moved to stderr so stdout stays pipeable.

QUERY reaches the shell via just's quote(), not "{{ QUERY }}" — just substitutes into the recipe body textually, so SQL containing a quoted identifier ("trakrf-migrate") would otherwise terminate the string and mangle the statement.

Verification

Reproduced the TRA-1104 wedge against a throwaway postgres:16, then confirmed the fix live on both environments.

Ownership, the thing that actually matters:

 function |     owner
----------+----------------
 new_path | trakrf-migrate     <- created via the new path
 old_path | postgres           <- created via the old path

A later migration running as trakrf-migrate:

CREATE OR REPLACE FUNCTION trakrf.new_path() ...  -> CREATE FUNCTION  (rc=0)
CREATE OR REPLACE FUNCTION trakrf.old_path() ...  -> ERROR: must be owner of function old_path  (rc=1)

Live against real CNPG clusters — the ticket's own table, inverted:

$ just psql preview "SELECT current_user, session_user, ... usesuper;"
  current_user  | session_user | usesuper
----------------+--------------+----------
 trakrf-migrate | postgres     | f

The docs/releasing.md step 0 invocation, which previously failed with Justfile does not contain recipe `-c` :

$ just psql prod "SELECT version, dirty FROM trakrf.schema_migrations;"
 version | dirty
---------+-------
      39 | f

Also verified live: banner is on stderr (stdout clean under 2>/dev/null); SQL with quotes and a semicolon survives intact; psql-super preview yields postgres/usesuper=t; psql-super prod fails closed without a tty and YES=1 bypasses as documented.

Correction found by testing: I had documented RESET ROLE as the escape back to superuser. It is not — the role arrives in the startup packet, so it becomes the session default that RESET returns to. SET ROLE postgres is the actual escape. Both the code comment and docs/ops.md say so now. This makes the guardrail slightly stronger than written: an incidental RESET ROLE will not silently restore superuser.

Commits

  1. chore: narrow the .claude/ gitignore (matching trakrf/docs) and track .claude/csw.json + .claude/csw-validate.sh. infra ignored all of .claude/, so csw fell back to defaults: tracker none, worktreeDir .worktrees (which CLAUDE.md calls a fresh-clone footgun), and an empty validate command. .claude/ was audited before flipping the rule — it held nothing but those two files.
  2. fix: the psql change above, plus 12 new ops-lib tests.
  3. ci: run scripts/test-ops-lib.sh. It was never wired into CI — its header asks you to run it by hand, which is discipline rather than a gate.
  4. chore: stop csw-validate mutating the terraform lockfiles.
  5. fix: stop a multi-line QUERY spilling out of a recipe comment (see below).

The multi-line bug, found late

Running the real findOwnershipDrift sweep through the new recipe — the exact use case QUERY exists for — failed with psql: line 1116: SELECT: command not found.

Cause: the explanatory comment above the QUERY assignment itself contained an interpolation of QUERY. just expands interpolations inside shebang-recipe comments like anywhere else, so the value was pasted into the comment too. A single-line query stayed behind the leading # and looked fine; a multi-line one spilled past it and ran as shell. Every earlier test passed because they were all single-line.

Fixed by rewording the comment so it contains no interpolation braces, plus two regression tests per recipe (via just --dry-run and a stub kubectl on PATH): the rendered body must parse under bash -n, and the query must reach psql as one byte-for-byte intact argument. Verified by reintroducing the bug — both fail for psql, neither for psql-super.

With it repaired, the full ownership sweep runs through just psql and both environments audit clean (0 drifted objects in schema trakrf on preview and prod). Note that sweep must run as psql, not psql-super: it filters on pg_has_role(CURRENT_USER, ...) and a superuser is implicitly a member of every role, so it would report a false clean.

Scope notes

  • Acceptance item 4 is cross-repo and is NOT in this PR. Deleting the "TEMPORARY WORKAROUND" block from platform's docs/releasing.md step 0 is a trakrf/platform change; re-dispatch there. A scope-split comment is on TRA-1105.
  • docs/db-migration.md, docs/backups.md and docs/prod-cutover.md still contain raw psql -U postgres kubectl calls. Left deliberately: those are DROP SCHEMA recovery paths that legitimately need superuser, not the hand-run-DDL hazard this closes.
  • Interactive use on a real terminal was the one path CI cannot cover — confirmed by Mike on preview for both recipes (just psqltrakrf-migrate with a trakrf=> prompt; just psql-superpostgres). Note \conninfo reports session_user and so says postgres in both modes; SELECT current_user, or the => vs =# prompt, is the check that distinguishes them.

Refs TRA-1105

Mike Stankavich added 4 commits August 19, 2026 12:50
infra ignored all of .claude/, so agent workflow config could not be
shared and every csw run fell back to built-in defaults: tracker 'none'
(no Linear), worktreeDir '.worktrees' (which CLAUDE.md explicitly calls a
fresh-clone footgun), and an empty validate command — a green run that
proved nothing.

Narrow the ignore to .claude/worktrees/, settings.local.json and *.lock,
matching trakrf/docs, and track:

  .claude/csw.json         tracker/base branch/validate, same shape as
                           the configs already in trakrf/{docs,platform}
  .claude/csw-validate.sh  local mirror of .github/workflows/ci.yml, so
                           CI failures surface before the PR

Audited .claude/ before flipping the rule: it held nothing but these two
files, so no scratch or credential material becomes trackable.

Refs: TRA-1105
just psql connected as the postgres superuser, so any DDL hand-run
through it created a postgres-owned object in the trakrf schema. Those
objects are permanently un-replaceable by the migrate role — CREATE OR
REPLACE, DROP and ALTER .. OWNER TO all require ownership — so a later
migration aborts partway and leaves the golang-migrate ledger dirty.
Because the migrate Job is an ArgoCD PreSync hook the Deployment is then
never updated: the old pod keeps serving while ArgoCD reports Healthy and
CI stays green. That is how preview wedged in TRA-1104.

New db_psql helper in ops-lib.sh runs psql through
env PGOPTIONS='-c role=<role>', which applies the equivalent of SET ROLE
at connect time. Connection auth is unchanged (in-pod peer auth as
postgres over the unix socket — the only credential available without
threading a password in); what changes is the role the session runs as,
and therefore who owns anything it creates.

  just psql ENV [QUERY]        role trakrf-migrate (default)
  just psql-super ENV [QUERY]  raw postgres superuser, opt-in

psql also gained the QUERY argument the v1.4.0 release needed: empty
opens an interactive shell, non-empty runs psql -c with ON_ERROR_STOP=1
and no tty. The banner moved to stderr so stdout stays pipeable. Without
it, docs/releasing.md step 0 could not be run as documented and operators
fell back to a raw kubectl exec — the superuser path this change closes.

QUERY reaches the shell via just's quote(), not "{{ QUERY }}": just
substitutes into the recipe body textually, so SQL containing a quoted
identifier would otherwise terminate the string and mangle the statement.

psql-super gates prod behind confirm_prod and warns on stderr in both
environments. It is a guardrail, not a boundary — session_user is still
postgres, so SET ROLE postgres escapes deliberately. Plain RESET ROLE
does not, since the role arrives in the startup packet and becomes the
session default.

Verified against a real postgres:16: PGOPTIONS yields
current_user=trakrf-migrate/usesuper=f, a function created through the
new path is owned by trakrf-migrate and CREATE OR REPLACE by the migrate
role succeeds, while one created through the old path is owned by
postgres and fails with 'must be owner of function' — the TRA-1104 wedge.

12 new ops-lib tests cover role selection, tty mode, ON_ERROR_STOP,
argument integrity for SQL with spaces and quotes, and arg validation.

Refs: TRA-1105
scripts/test-ops-lib.sh was never wired into CI — its own header asks you
to run it by hand after changing ops-lib.sh, which is discipline rather
than a gate. This change adds 12 tests to that file covering db_psql, so
make them load-bearing.

Pure bash with a stubbed kubectl: no cluster, no credentials, no matrix.

Refs: TRA-1105
tofu init records provider hashes for the current platform into the
tracked .terraform.lock.hcl files. CI does this in a throwaway checkout
and never notices; run locally it left four modified lockfiles behind,
which a later 'git add -A' would sweep into an unrelated commit — caught
exactly that way on this branch.

Note which lockfiles are clean before the tofu loop and restore those
afterwards. A lockfile already edited on purpose is left alone.

Refs: TRA-1105
The explanatory comment above the QUERY assignment in the psql recipe
itself contained an interpolation of QUERY. just expands interpolations
inside shebang-recipe comments exactly as it does anywhere else, so the
value was pasted into the comment too. A single-line query stayed behind
the leading # and nothing looked wrong; a multi-line one spilled past it
and its remaining lines ran as shell:

  psql: line 1116: SELECT: command not found

Found by running the real findOwnershipDrift sweep from platform through
the new recipe — the exact use case the QUERY argument exists for, and
one that no single-line test could have surfaced.

Reword the comment so it never contains interpolation braces, and say so
explicitly, since the failure is invisible until a value spans lines.

Two regression tests per recipe, driven through  and a
stub kubectl on PATH: the rendered body must parse under , and
the query must reach psql as one byte-for-byte intact argument. Verified
by reintroducing the bug — both fail for psql and neither for psql-super.

Both environments audit clean through the repaired path (0 drifted
objects in schema trakrf on preview and prod).

Refs: TRA-1105
@mikestankavich
mikestankavich merged commit c83d9a4 into main Aug 19, 2026
20 checks passed
@mikestankavich
mikestankavich deleted the fix/tra-1105-ops-psql-connects-as-superuser-which-sil branch August 19, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant