search

ptsochantaris / trailer

Managing Pull Requests and Issues For GitHub & GitHub Enterprise
https://github.com/ptsochantaris/trailer
Other
1.16k stars 67 forks source link

Reduce Personal Access Token exposure #470

Closed rahim closed 1 year ago

rahim commented 1 year ago

The Personal Access Token used by Trailer is somewhat sensitive given the broad permissions it requires.

At the moment the token is shown in the Preferences UI in the clear

Screenshot 2023-02-15 at 12 45 48

And can be found on disk by anything with access to the files in the user's Library.

ie

$ sqlite3 ~/Library/Application\ Support/com.housetrip.Trailer/Trailer.sqlite 'SELECT ZAUTHTOKEN FROM ZAPISERVER;'
ghp_xxxxxxxxxxxxxxxxxxxxx

Storing this in the user's Keychain would keep this behind some level of access control and off disk in unencrypted form.

Obscuring the existing token in the Preferences UI until the user provided further authentication (eg TouchID) would further protect it.

ptsochantaris commented 1 year ago

You are totally right and this change is long overdue - will add 👍

ptsochantaris commented 1 year ago

Here is a test build with this feature enabled. It should, in theory, migrate your token(s) to the keychain and remove them from the database, as well as hide them from the UI. You can always find the tokens in the Keychain. Note that this isn't quite polished yet - for instance if you deny Keychain access I suspect things will break :) But I wanted to get some feedback in the meantime if you find you have the time to test it out.

[edit: removed temporarily, need to address a bug]

ptsochantaris commented 1 year ago

Ok, fixed ;) Sorry about that!

[edit: removed, updated build below]

ptsochantaris commented 1 year ago

A fresh binary that won't auto-update when 1.8.7 is out shortly, because otherwise anyone using the above test build would lose their tokens :)

Trailer-188.zip

rahim commented 1 year ago

Thank you @ptsochantaris, this is great news. I'll test this out shortly and let you know how I get on.

rahim commented 1 year ago

Tried that that build out and it went well - it seamlessly (and silently) transitioned to the Keychain storage, the token is no longer shown in the UI.

Repeating my SQLITE query above no longer returns the token. 🎉

ptsochantaris commented 1 year ago

Fantastic news - thanks for testing! I'll send this out as part of the next update 👍

ptsochantaris commented 1 year ago

Version 1.8.8 is now up and contains this feature. Will close this issue now, but please feel free to open new issues related to this or other problems/suggestions.