matterhorn-chat / matterhorn

A feature-rich Unix terminal client for the Mattermost chat system
BSD 3-Clause "New" or "Revised" License
1.04k stars 76 forks source link

Quote parsing in configuration settings leads to unintended parses #761

Closed kampfflunder closed 3 weeks ago

kampfflunder commented 2 years ago

I have in config.ini:

tokencmd: security find-generic-password -s 'matterhorn' -a 'myaccount' -w

It fails on startup with:

security: SecKeychainSearchCopyNext: The specified item could not be found in the keychain.
Error loading config: Could not execute token command: failed

But if I run the same command in a terminal, it works and prints the correct token.

tokencmd: echo "mytoken"

works, but that is of course a bad solution.

MacOS 11.6.5, matterhorn 50200.15.0 from macports

jtdaugherty commented 2 years ago

Hi @kampfflunder - thanks for reporting this. Out of curiosity, are you running matterhorn within any kind of terminal multiplexer like tmux or screen? This doesn't seem likely to me to be caused by Matterhorn itself since it just runs the command you give it. Incidentally I personally use this same technique and it works for me. The only difference between your command and mine is that I don't use the -a option, but I doubt that would have a big impact on this. This behavior you are seeing makes me wonder about the environment that matterhorn is running in, such as the user account running it, or some other environmental thing that is getting in the way of keychain access.

kampfflunder commented 2 years ago

Bamm!

One must NOT use single or double quotes!

tokencmd: security find-generic-password -s 'matterhorn' -a 'myaccount' -w
tokencmd: security find-generic-password -s "matterhorn" -a "myaccount" -w

does not work, while

tokencmd: security find-generic-password -s matterhorn -a myaccount -w

does. I think at some point the parser for the ini file fails Not sure if this has to be considered a bug?

jtdaugherty commented 2 years ago

That might be a bug in the config parser library, yeah. I could see it either way because quotes can be used to quote the config values themselves, which means that quotes could be ambiguous. However, at the end of the day it's still important for people to be able to use quotes for settings like tokencmd so we should do something about it. I'm glad you found out what the problem was! I'm going to leave this open as a reminder to follow up on this.

jtdaugherty commented 3 weeks ago

I attempted to reproduce this and couldn't, so I'm going to close it. If you run into this issue on the latest version of Matterhorn, let's revisit it.