Skip to content

fix(core): see mariadb, mycli, pgcli and redis-cli as the database clients they are - #127

Open
Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/see-every-database-client
Open

Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/see-every-database-client

Conversation

@Cedric921

Copy link
Copy Markdown
Contributor

Closes #40.

What this changes

The statement grammar in sql.ts:11-16 was keyed on four binary names, so the same statement was destructive through one client and an ordinary shell line through its drop-in replacement:

mysql -e "DROP DATABASE prod"     ->  mysql.drop        destructive
mariadb -e "DROP DATABASE prod"   ->  shell.execute     normal
mycli -e "DROP DATABASE prod"     ->  shell.execute     normal
psql -c "DROP TABLE users"        ->  psql.drop         destructive
pgcli -c "DROP TABLE users"       ->  shell.execute     normal
redis-cli FLUSHALL                ->  shell.execute     normal

mariadb and mycli now read a statement the way mysql does, and pgcli the way psql does. Each keeps its own name in the action — mariadb.drop, not mysql.drop — so a rule can still name one client, and each gets a table of its own so an interactive session is a write like mysql is.

redis-cli takes its command as argv rather than as a statement, so it needs a table instead of a grammar.

Classes in the new tables, named as the contributing guide asks

redis-cli — destructive: FLUSHALL, FLUSHDB, DEL, UNLINK. Write: SET, CONFIG SET, and a bare interactive session. Read: GET, KEYS, SCAN, INFO.

mariadb, mycli, pgcli — write for -e/-c and for an interactive session, exactly as mysql and psql already are. The statement itself still decides the real class through the SQL path, which is how mariadb -e "SELECT 1" comes out read.

Nothing moved in an existing table.

One shared change

verbsIgnoreCase is a new optional field on VerbTable, set only by redis-cli, because the protocol answers to FLUSHALL and flushall alike. matchVerb lowercases argv for matching when it is set, and nothing else: the target is still read from argv as it was typed. Spelling the eleven redis verbs twice each was the alternative, and the next person adding one would have added it once.

How it was verified

Seventeen cases across resolve.test.ts and sql.test.ts, all of which fail on main:

  • each drop-in client resolves the destructive statement to its own .drop
  • redis-cli FLUSHALL, flushall and FlushDb all reach the same lowercase action
  • CONFIG SET and SET are writes, GET and INFO reads
  • mariadb -e "SELECT 1" stays read, so the alias did not blanket everything as a write
  • a bare mariadb and a bare redis-cli are writes, as mysql already was
Test Files  286 passed (286)
     Tests  8469 passed (8469)

pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all clean, on Node 24.

Checklist

  • pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all pass
  • Behaviour change ships with a test
  • No any, no magic values, no console.* outside cli-output.ts
  • If this touches the decision path: still deterministic — one more table and one grammar alias, no model, network or randomness
  • If this changes a verb table: the classes are named above; four tables are added and none already there is touched
  • If this changes a command, flag or file it writes: n/a; scan now lists four more CLIs, which the changeset names

@Cedric921

Copy link
Copy Markdown
Contributor Author

The red Dependency audit here is not this change — all twelve test legs pass, and the audit fails the same way on a clean main.

It is GHSA-vfj7-8cjw-p6xm on braces, which the advisory says is patched in >=3.0.4. That version was never published: pnpm view braces dist-tags answers { latest: '3.0.3' }, and 3.0.3 is the vulnerable one. So there is nothing to override to, and pnpm audit --audit-level high cannot pass on any branch right now. braces is a devDependency only and reaches nothing the four packages publish.

I tried bumping the dependency that pulls it before reporting. @changesets/cli@3 does drop the chain, but knip reaches the same micromatch@4.0.8 on its own, so the advisory stays:

before:  braces <- micromatch <- fast-glob <- globby <- @manypkg/get-packages <- @changesets/cli
after:   braces <- micromatch <- fast-glob <- knip

I reverted that rather than push a major bump of the tool pnpm ship runs for a fix that is not one.

Written up with the evidence in #129, where the remaining choices are policy ones — an ignore entry, a qualified gate, or merging past it — which seemed yours to make rather than mine to guess at. This PR needs nothing.

This branch has not been deployed

No deployments
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.

intercept: mariadb, redis-cli, pgcli and mycli are not recognized as database clients

1 participant