Skip to content

Conversation

@AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Dec 10, 2025

Note

This PR builds on top of #12

What

  • Add a tip in README.md about starting the command with a space to not have the key leak in the user's shell history
  • Update the expected argument for unlock:
    • The base64: prefix (introduced in Add support for base64:… arg for unlock #10) is no longer needed to provide the base64-encoded key during unlock <base64key>, which makes more sense since that's the use case that local developers will use the most (copying the Base64-encoded key from the Secret Store to unlock their local working copy with it)
    • Drop support for reading the binary key from a file. In practice, this use case would be the less used one (and one we wouldn't recommend, as having the binary key around in a file makes it easy to accidentally be committed, or be read by another user on the machine if not properly protected, etc), so it felt not only odd but also sending the wrong signal/suggestion to have it as the default for this unlock argument. Reading from a file is still possible via < shell redirection anyway.

Testing

# Store the base64 key you copied to your clipboard in an env var
$ export GIT_CONCEAL_KEY=$(pbpaste)
# Test passing the base64 key directly
$ git conceal lock
$ git conceal unlock $(pbpaste)
$ git conceal lock
$ git conceal unlock $GIT_CONCEAL_KEY
# Test passing the env var name
$ git conceal lock
$ git conceal unlock env:GIT_CONCEAL_KEY
# Test passing it via stdin (raw binary)
$ git conceal lock
$ echo "$GIT_CONCEAL_KEY" | base64 -d | git conceal unlock -
$ git conceal lock
$ echo "$GIT_CONCEAL_KEY" | base64 -d >~/git-conceal-key.bin
$ git conceal unlock - < ~/git-conceal-key.bin
$ rm ~/git-conceal-key.bin

echo "c3VwcG9zZWRseS15b3VyLWJpbmFyeS1zZWNyZXRrZXk=" | base64 -d | git-conceal unlock -
# Option 1: Provide the Base64-encoded key directly as command line argument.
# Only use locally, as on CI this could leak the key in logs.
# Tip: start your command with a space to avoid it (and thus the key) being added to your shell's history
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@iangmaia iangmaia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

I've tested lock / unlock in PCAndroid, with both a file < redirection and the base64 string 👍

@AliSoftware
Copy link
Contributor Author

Thanks for the review 👍

FYI: Since this is built on top of #12 itself built on top of #11, I'll probably wait for those other 2 to be reviews/approved/merged to merge that chain of PR in order (as opposed to "polluting" those other PRs' diff with the diff of this PR if I were to merge it now)

 - Remove the syntax that was allowing to provide the key from a file.
 - Make the default syntax be for passing the key as base64 directly as argument (without requiring the `base64:` prefix for that case anymore.

Rationale:

It was never a good idea to provide a built-in way to read the binary key from a file IMHO, because that would suggest that having the key laying around in a random file on disk was a good idea to begin with (which can be OK… as long as the key file is properly protected). For example, we wouldn't want to incite people to write the key in a file just to be able to `git-conceal unlock` with it… and then accidentally commit that key file.

Reading from a file is still possible using `-` as the argument to read from `stdin`, then using shell redirection syntax `<file` to feed the content of the file as stdin. So even in the unlikely case that someone would want to provide the key via a file, they can still do that.

And since having the base64 key directly will be the most common use case for `unlock` on local machines (when developers get the key from the secret store and copy/paste it to the `unlock` command), that feels more fitting for this to be the default command / most basic syntax (i.e. without requiring a `base64:` prefix for it)
Base automatically changed from files-reorg to trunk December 11, 2025 18:42
@AliSoftware AliSoftware merged commit defdb9b into trunk Dec 11, 2025
4 checks passed
@AliSoftware AliSoftware deleted the better-unlock-arg branch December 11, 2025 18:42
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.

3 participants