Hello pass(1)! I'm using vis(1) (https://github.com/martanne/vis) and whenever I try to edit my passwords I get stumped:
[kousu@requiem ~]$ EDITOR=vis pass edit lol New password not saved. Linux vi(1) too: [kousu@requiem ~]$ EDITOR=vi pass edit lol New password not saved. But these editors all work: [kousu@requiem ~]$ EDITOR=vim pass edit lol [master 4c687b5] Edit password for lol using vim. 1 file changed, 0 insertions(+), 0 deletions(-) rewrite lol.gpg (100%) [kousu@requiem ~]$ EDITOR=nano pass edit lol [master b8dc02b] Edit password for lol using nano. 1 file changed, 0 insertions(+), 0 deletions(-) rewrite lol.gpg (100%) [kousu@requiem ~]$ EDITOR=mg pass edit lol [master 28e9d7c] Edit password for lol using mg. 1 file changed, 0 insertions(+), 0 deletions(-) rewrite lol.gpg (100%) [kousu@requiem ~]$ I've installed pass(1) from ArchLinux repos: [kousu@requiem pass]$ pacman -Qi pass Name : pass Version : 1.7.1-1 Description : Stores, retrieves, generates, and synchronizes passwords securely Architecture : any URL : https://www.passwordstore.org Licenses : GPL2 Groups : None Provides : passmenu Depends On : xclip bash gnupg tree Optional Deps : git: for Git support [installed] dmenu: for passmenu [installed] qrencode: for QR code support Required By : None Optional For : None Conflicts With : passmenu Replaces : passmenu Installed Size : 45.00 KiB Packager : Lukas Fleischer <[email protected]> Build Date : Fri 14 Apr 2017 04:31:38 AM EDT Install Date : Mon 22 Jan 2018 04:23:59 PM EST Install Reason : Explicitly installed Install Script : No Validated By : Signature (ignore the install date; I tried reinstalling to see if the bug was just me, but I've actually had it installed since August or earlier). `pass edit` is: cmd_edit() { ... tmpdir #Defines $SECURE_TMPDIR local tmp_file="$(mktemp -u "$SECURE_TMPDIR/XXXXXX")-${path//\//-}.txt" ... ${EDITOR:-vi} "$tmp_file" # < -- the file is deleted here --> [[ -f $tmp_file ]] || die "New password not saved." What's going on is that `tmpdir` says tmpdir() { ... local template="$PROGRAM.XXXXXXXXXXXXX" if [[ -d /dev/shm && -w /dev/shm && -x /dev/shm ]]; then SECURE_TMPDIR="$(mktemp -d "/dev/shm/$template")" remove_tmpfile() { rm -rf "$SECURE_TMPDIR" } trap remove_tmpfile INT TERM EXIT ... And that trap is running *when vi exits*. I don't know why vis(1) and vi(1) trigger it but the others don't, and why all the other intermediate commands don't also trigger it, but they do. Here's a refined test case script which reproduces the behaviour: ``` #!/bin/sh # test_traps.sh # debugging why pass(1) can't edit any passwords; it always says "Password not saved" # I suspect it's because 'trap remove_tmpfile' is running when the editor quits, even though it's meant to run when pass(1) quits tmpdir() { SECURE_TMPDIR="$(mktemp -d "/dev/shm/test_traps.XXXXX")" remove_tmpfile() { echo "remove_tmpfile" rm -rf "$SECURE_TMPDIR" } trap remove_tmpfile INT TERM EXIT # this runs twice: once immediately after vi gets done, and once at the end of the script #trap remove_tmpfile EXIT # behaves sensible; even cleans up on INT when INT triggers EXIT #trap remove_tmpfile TERM # this *never* runs #trap remove_tmpfile INT # this runs *after* vi, and only after vi } tmpdir tmpfile=$SECURE_TMPDIR/test date > "$tmpfile" echo $tmpfile cat $tmpfile sleep 3 echo "launching editor" ${EDITOR:-vi} $SECURE_TMPDIR/test echo "done editor" # this should *succeed*: the file should be readable here echo $tmpfile cat $tmpfile # but after this script, the file should be gone, so test with test_traps.sh ; ls -l /dev/shm/test_traps.* ``` Here is how this behaves for me. It seems, actually, that this may be a bashism, since under sh(1) `remove_tmpfile` only runs once, but under bash(1) it runs twice: [kousu@requiem pass]$ bash test_traps.sh /dev/shm/test_traps.ge5j6/test Mon Jan 22 16:42:16 EST 2018 vi remove_tmpfile done vi /dev/shm/test_traps.ge5j6/test cat: /dev/shm/test_traps.ge5j6/test: No such file or directory remove_tmpfile [kousu@requiem pass]$ sh test_traps.sh /dev/shm/test_traps.Noqry/test Mon Jan 22 16:41:54 EST 2018 vi done vi /dev/shm/test_traps.Noqry/test Mon Jan 22 16:41:54 EST 2018 remove_tmpfile I think a sufficient patch is just to *not* trap INT or TERM; EXIT should be enough. In my testing it seems to be enough. Another patch might be to make sure to run bash in posixly-correct mode. Here is my patch below (and attached too). I'm not sure if this is the correct way to submit a patch; I'm too used to github :$. If you want it in a different format, let me know. [kousu@requiem password-store]$ cat 0001-Don-t-trap-INT-or-TERM-they-are-redundant-and-can-br.patch >From cefb03e4fb75abdd878d96e1982e9723e8e7f280 Mon Sep 17 00:00:00 2001 From: kousu <[email protected]> Date: Mon, 22 Jan 2018 16:52:06 -0500 Subject: [PATCH] Don't trap INT or TERM; they are redundant and can break `pass edit`. Some EDITORs, notably Linux vi(1), which is the fallback default in pass, apparently send INT when they exit, and when pass is run under bash (which is also its default)--if you have /dev/shm/ available--bash catches this and cleans up your edited password file *before* it can be reencrypted and saved. This only happens with `pass edit`; none of the other commands combine tmpdir and $EDITOR. --- src/password-store.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/password-store.sh b/src/password-store.sh index e3cd145..99d2d65 100755 --- a/src/password-store.sh +++ b/src/password-store.sh @@ -203,7 +203,7 @@ tmpdir() { remove_tmpfile() { rm -rf "$SECURE_TMPDIR" } - trap remove_tmpfile INT TERM EXIT + trap remove_tmpfile EXIT else [[ $warn -eq 1 ]] && yesno "$(cat <<-_EOF Your system does not have /dev/shm, which means that it may @@ -218,7 +218,7 @@ tmpdir() { find "$SECURE_TMPDIR" -type f -exec $SHRED {} + rm -rf "$SECURE_TMPDIR" } - trap shred_tmpfile INT TERM EXIT + trap shred_tmpfile EXIT fi } -- 2.16.0 _______________________________________________ Password-Store mailing list [email protected] https://lists.zx2c4.com/mailman/listinfo/password-store
