Quote MySQL identifiers, and name the second place the engines differ - #36
Conversation
Four findings from a review of #35, which merged while I was acting on them. All nits, none of which broke the default path -- three worth fixing and one worth relabelling. IDENTIFIER QUOTING The MySQL grants interpolated ${DATABASE} bare while the Postgres statements wrapped the same variable in double quotes. `--database appdata` is a bare identifier so nothing noticed, but a hyphenated or reserved-word schema is a MySQL parse error at issuance that names neither the flag nor the value. Backtick-quoted now, escaped because a bare backtick in a double-quoted bash string starts command substitution -- which is exactly what happened when the same quoting went into the test file, and is why the assertions there are escaped too. THE SECOND ASYMMETRY The docs claimed VALID UNTIL was the one place the engines are not equivalent. It was not: the Postgres readwrite role grants CREATE ON SCHEMA public and the MySQL one grants DML only, so an ORM migration works on one and fails on the other. This one is not quietly fixable. Postgres ties DDL to ownership -- a credential can alter what it created and nothing else. MySQL grants per schema, so the same privilege would let every readwrite credential drop the whole database. The narrower grant stays, the comparison table gains a row, and a test asserts the difference in both directions the way the VALID UNTIL pair already does. That test pins the whole privilege list rather than naming the DDL keywords to reject. The first version did the latter and was circular: it caught exactly the string I had thought to write down, and the mutation I "verified" it with added that same string. Granting only CREATE walked straight through it -- confirmed by running the old assertion against that mutation and watching it pass. Anchoring on "DELETE ON" instead means any addition to the list breaks the match, whichever privilege it is. REVOCATION WAS UNDER-ASSERTED The MySQL revocation check only confirmed the credential could no longer connect, where its Postgres neighbour also confirms the role is gone from pg_roles. MySQL returns an identical "access denied" whether the password is wrong or the account does not exist, so revoking grants without dropping the user would have passed. It now reads mysql.user as root -- whose password this bootstrap never rotates, which is what makes the question askable at all. A RELABEL RATHER THAN A FIX The review flagged the vaultadmin assertions as tautological, since USERNAME defaults to vaultadmin for both engines and no branch sets root. The guard is real -- reintroducing the root shortcut anywhere fails it, which a mutation confirms -- but the section header and labels claimed engine-specific behaviour they were not testing. Relabelled to say what they check: no engine connects as root. Also fixes a state-inheritance bug introduced while adding the DDL test. The VALID UNTIL assertion reads the log of the preceding run, so adding a postgres run above it silently repointed it at the wrong engine and it began failing about MySQL while describing postgres. It sets up its own run now instead of inheriting whatever the previous section left. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3bdf27d to
d86cb3d
Compare
CI23/23 on Shim suite 57 → 60, integration 89 → 92. A circular assertion I wrote, and how it was caughtWorth recording, because the first version of the DDL test looked fine It named the keywords to reject: assert_log_lacks "mysql readwrite gets no schema-wide DDL" "CREATE, ALTER, DROP"And I "verified" it with a mutation that granted A grant of just I checked that rather than reasoning about it — restored the old The replacement pins the whole privilege list instead of enumerating what assert_log_has "mysql readwrite grants DML and nothing more" \
"GRANT SELECT, INSERT, UPDATE, DELETE ON \`appdata\`.*"Anchoring on The general lesson, which applies to the other Note on why this is a separate PR#35 merged while I was working these findings. GitHub auto-deleted the |
Follow-up to #35, which merged while I was acting on a review of it.
Four findings, all nits, none of which broke the default path — three
worth fixing and one worth relabelling.
Identifier quoting
The MySQL grants interpolated
${DATABASE}bare while the Postgresstatements wrapped the same variable in double quotes:
--database appdatais a bare identifier so nothing noticed, but ahyphenated or reserved-word schema is a MySQL parse error at issuance
naming neither the flag nor the value.
The backticks are escaped, because a bare one inside a double-quoted bash
string starts command substitution. I flagged that trap when writing the
script and then walked into it applying the same quoting to the test
file — caught on the first run, and the assertions are escaped too.
The second asymmetry
The docs claimed
VALID UNTILwas "the one place where the two enginesare not equivalent." That was false. Postgres
readwritegrantsCREATE ON SCHEMA public; MySQL grants DML only. An ORM migration workson one and fails on the other with
CREATE command denied.I did not take the obvious fix of granting MySQL the same DDL,
because it would not be equivalent — it would be broader:
GRANT CREATE, ALTER, DROP ON appdata.*would let every issued readwritecredential drop the whole database. So the narrow grant stays, the
comparison table gains a row, and a workload that genuinely needs DDL
should get a third role rather than have
readwritewidened underneathevery other consumer. Paired assertions pin it in both directions, the
way the
VALID UNTILpair already does.Revocation was under-asserted
The MySQL check only confirmed the credential could no longer connect;
its Postgres neighbour also confirms the role is gone from
pg_roles.MySQL returns an identical "access denied" whether the password is wrong
or the account does not exist — so revoking grants without dropping
the user would have passed. It now reads
mysql.useras root, whosepassword this bootstrap never rotates, which is exactly what makes the
question askable (the issued credential is scoped to
appdatapreciselyso it cannot read
mysql.user, andvaultadmin's password is gone bythen).
A relabel rather than a fix
The review called the
vaultadminassertions tautological and said theintent "is not actually guarded." The guard is real — reintroducing the
root shortcut anywhere fails it, which a mutation confirms. But the
section header and labels claimed engine-specific behaviour they were not
testing, so they now say what they check: no engine connects as root.
One bug I introduced and caught
Adding the DDL test put a Postgres run immediately above the
VALID UNTILassertion, which reads the log of the preceding run. Itsilently repointed at the wrong engine and started failing about MySQL
while describing Postgres. That section sets up its own run now instead
of inheriting whatever the previous one left behind.
Verification
Shim suite 57 → 60, all passing. Both new script behaviours are
mutation-verified: removing the identifier quoting and granting MySQL
schema-wide DDL are each caught by the assertion added for them.