1Password / shell-plugins

Seamless authentication for every tool in your terminal.
https://developer.1password.com/docs/cli/shell-plugins/
MIT License
506 stars 163 forks source link

1Password mysql shell plugin breaks additional CLI arguments #406

Open minaguib opened 7 months ago

minaguib commented 7 months ago

Platform or tool

mysql

Desired or expected behavior

Invoking mysql additional arguments should work as if invoking native mysql binary

Current behavior

Supplying additional arguments while using the 1Password mysql shell plugin integration returns error:

mysql: [ERROR] unknown variable 'defaults-file=/var/folders/hp/7hxtb9_968d3vj4q13z28vjc0000gn/T/1PasswordShellPlugins-413974828/my.cnf'.

Relevant log output

I investigated a bit, and given an invocation like mysql dbname

The plugin prepares the temporary file and appends it to the CLI arguments like so:

/opt/homebrew/opt/mysql-client/bin/mysql dbname --defaults-file=/var/folders/hp/7hxtb9_968d3vj4q13z28vjc0000gn/T/1PasswordShellPlugins-413974828/my.cnf

mysql: [ERROR] unknown variable 'defaults-file=/var/folders/hp/7hxtb9_968d3vj4q13z28vjc0000gn/T/1PasswordShellPlugins-413974828/my.cnf'.

It appears however that the position matters to the underlying mysql binary, and arguments like --defaults-file need to come before additional arguments (like dbname or -e foo.sql or -A etc...)

This order works:

/opt/homebrew/opt/mysql-client/bin/mysql --defaults-file=/var/folders/hp/7hxtb9_968d3vj4q13z28vjc0000gn/T/1PasswordShellPlugins-413974828/my.cnf dbname

op CLI version

2.23.0

nr-dbuckwalter commented 7 months ago

@minaguib I was facing the same issue and have sumbitted a fairly simple PR to address it (works for me!). Can you give it a try as well?

minaguib commented 7 months ago

@nr-dbuckwalter Looks good on my end - thanks :)

For reference, for anyone else who wants to try:

# Clone Damon's branch:
git clone git@github.com:nr-dbuckwalter/shell-plugins.git -b fix-mysql-arg-order

# Build the mysql plugin locally
cd shell-plugins
make mysql/build

# Validate it got installed here:
$ ls -la ~/.config/op/plugins/local/
-rwxr-xr-x  1 mina  staff  20390690 30 Nov 16:19 mysql

# Test it - you'll get an ugly warning due to the above setup till properly released
$ mysql --version
#############################################################################
# WARNING: 'mysql' is not from the official registry.                       #
# Only proceed if you are the developer of 'mysql'.                         #
# Otherwise, delete the file at /Users/mina/.config/op/plugins/local/mysql. #
#############################################################################
mysql  Ver 8.2.0 for macos14.0 on arm64 (Homebrew)
gccervonebport commented 5 days ago

@nr-dbuckwalter Thanks so much for your work on this - now mysql is fully useable. And, I was able to get a plugin working for mysqldump too - which I really needed!

-= G =-