[PATCH] pass generates quite a few errors/warnings with shellcheck

ಚಿರಾಗ್ ನಟರಾಜ್ chiraag.nataraj at gmail.com
Sun May 19 17:08:05 CEST 2019


Updated patch to take care of one more warning (the [[ $? -ne 0 ]] && continue bit).

diff --git a/src/password-store.sh b/src/password-store.sh
index 284eabf..c964032 100755
--- a/src/password-store.sh
+++ b/src/password-store.sh
@@ -6,10 +6,11 @@
 umask "${PASSWORD_STORE_UMASK:-077}"
 set -o pipefail
 
-GPG_OPTS=( $PASSWORD_STORE_GPG_OPTS "--quiet" "--yes" "--compress-algo=none" "--no-encrypt-to" )
+read -r -a PASSWORD_STORE_GPG_OPTS_ARR <<< "$PASSWORD_STORE_GPG_OPTS"
+GPG_OPTS=( "${PASSWORD_STORE_GPG_OPTS_ARR[@]}" "--quiet" "--yes" "--compress-algo=none" "--no-encrypt-to" )
 GPG="gpg"
 export GPG_TTY="${GPG_TTY:-$(tty 2>/dev/null)}"
-which gpg2 &>/dev/null && GPG="gpg2"
+command -v gpg2 &>/dev/null && GPG="gpg2"
 [[ -n $GPG_AGENT_INFO || $GPG == "gpg2" ]] && GPG_OPTS+=( "--batch" "--use-agent" )
 
 PREFIX="${PASSWORD_STORE_DIR:-$HOME/.password-store}"
@@ -58,7 +59,8 @@ die() {
 verify_file() {
 	[[ -n $PASSWORD_STORE_SIGNING_KEY ]] || return 0
 	[[ -f $1.sig ]] || die "Signature for $1 does not exist."
-	local fingerprints="$($GPG $PASSWORD_STORE_GPG_OPTS --verify --status-fd=1 "$1.sig" "$1" 2>/dev/null | sed -n 's/^\[GNUPG:\] VALIDSIG \([A-F0-9]\{40\}\) .* \([A-F0-9]\{40\}\)$/\1\n\2/p')"
+	local fingerprints;
+	fingerprints="$($GPG "${PASSWORD_STORE_GPG_OPTS_ARR[@]}" --verify --status-fd=1 "$1.sig" "$1" 2>/dev/null | sed -n 's/^\[GNUPG:\] VALIDSIG \([A-F0-9]\{40\}\) .* \([A-F0-9]\{40\}\)$/\1\n\2/p')"
 	local fingerprint found=0
 	for fingerprint in $PASSWORD_STORE_SIGNING_KEY; do
 		[[ $fingerprint =~ ^[A-F0-9]{40}$ ]] || continue
@@ -106,7 +108,8 @@ set_gpg_recipients() {
 
 reencrypt_path() {
 	local prev_gpg_recipients="" gpg_keys="" current_keys="" index passfile
-	local groups="$($GPG $PASSWORD_STORE_GPG_OPTS --list-config --with-colons | grep "^cfg:group:.*")"
+	local groups;
+	groups="$($GPG "${PASSWORD_STORE_GPG_OPTS_ARR[@]}" --list-config --with-colons | grep "^cfg:group:.*")"
 	while read -r -d "" passfile; do
 		[[ -L $passfile ]] && continue
 		local passfile_dir="${passfile%/*}"
@@ -119,14 +122,15 @@ reencrypt_path() {
 		set_gpg_recipients "$passfile_dir"
 		if [[ $prev_gpg_recipients != "${GPG_RECIPIENTS[*]}" ]]; then
 			for index in "${!GPG_RECIPIENTS[@]}"; do
-				local group="$(sed -n "s/^cfg:group:$(sed 's/[\/&]/\\&/g' <<<"${GPG_RECIPIENTS[$index]}"):\\(.*\\)\$/\\1/p" <<<"$groups" | head -n 1)"
+				local group;
+				group="$(sed -n "s/^cfg:group:$(sed 's/[\/&]/\\&/g' <<<"${GPG_RECIPIENTS[$index]}"):\\(.*\\)\$/\\1/p" <<<"$groups" | head -n 1)"
 				[[ -z $group ]] && continue
 				IFS=";" eval 'GPG_RECIPIENTS+=( $group )' # http://unix.stackexchange.com/a/92190
 				unset "GPG_RECIPIENTS[$index]"
 			done
-			gpg_keys="$($GPG $PASSWORD_STORE_GPG_OPTS --list-keys --with-colons "${GPG_RECIPIENTS[@]}" | sed -n 's/^sub:[^:]*:[^:]*:[^:]*:\([^:]*\):[^:]*:[^:]*:[^:]*:[^:]*:[^:]*:[^:]*:[a-zA-Z]*e[a-zA-Z]*:.*/\1/p' | LC_ALL=C sort -u)"
+			gpg_keys="$($GPG "${PASSWORD_STORE_GPG_OPTS_ARR[@]}" --list-keys --with-colons "${GPG_RECIPIENTS[@]}" | sed -n 's/^sub:[^:]*:[^:]*:[^:]*:\([^:]*\):[^:]*:[^:]*:[^:]*:[^:]*:[^:]*:[^:]*:[a-zA-Z]*e[a-zA-Z]*:.*/\1/p' | LC_ALL=C sort -u)"
 		fi
-		current_keys="$(LC_ALL=C $GPG $PASSWORD_STORE_GPG_OPTS -v --no-secmem-warning --no-permission-warning --decrypt --list-only --keyid-format long "$passfile" 2>&1 | sed -n 's/^gpg: public key is \([A-F0-9]\+\)$/\1/p' | LC_ALL=C sort -u)"
+		current_keys="$(LC_ALL=C $GPG "${PASSWORD_STORE_GPG_OPTS_ARR[@]}" -v --no-secmem-warning --no-permission-warning --decrypt --list-only --keyid-format long "$passfile" 2>&1 | sed -n 's/^gpg: public key is \([A-F0-9]\+\)$/\1/p' | LC_ALL=C sort -u)"
 
 		if [[ $gpg_keys != "$current_keys" ]]; then
 			echo "$passfile_display: reencrypting to ${gpg_keys//$'\n'/ }"
@@ -173,11 +177,13 @@ clip() {
 	# variable. Specifically, it cannot store nulls nor (non-trivally) store
 	# trailing new lines.
 	pkill -f "^$sleep_argv0" 2>/dev/null && sleep 0.5
-	local before="$("${paste_cmd[@]}" 2>/dev/null | $BASE64)"
+	local before;
+	before="$("${paste_cmd[@]}" 2>/dev/null | $BASE64)"
 	echo -n "$1" | "${copy_cmd[@]}" || die "Error: Could not copy data to the clipboard"
 	(
 		( exec -a "$sleep_argv0" bash <<<"trap 'kill %1' TERM; sleep '$CLIP_TIME' & wait" )
-		local now="$("${paste_cmd[@]}" | $BASE64)"
+		local now;
+		now="$("${paste_cmd[@]}" | $BASE64)"
 		[[ $now != $(echo -n "$1" | $BASE64) ]] && before="$now"
 
 		# It might be nice to programatically check to see if klipper exists,
@@ -232,7 +238,7 @@ tmpdir() {
 		)"
 		SECURE_TMPDIR="$(mktemp -d "${TMPDIR:-/tmp}/$template")"
 		shred_tmpfile() {
-			find "$SECURE_TMPDIR" -type f -exec $SHRED {} +
+			find "$SECURE_TMPDIR" -type f -exec "$SHRED" {} +
 			rm -rf "$SECURE_TMPDIR"
 		}
 		trap shred_tmpfile EXIT
@@ -342,14 +348,15 @@ cmd_init() {
 		rmdir -p "${gpg_id%/*}" 2>/dev/null
 	else
 		mkdir -v -p "$PREFIX/$id_path"
-		printf "%s\n" "$@" > "$gpg_id"
-		local id_print="$(printf "%s, " "$@")"
+		printf "%s\\n" "$@" > "$gpg_id"
+		local id_print;
+		id_print="$(printf "%s, " "$@")"
 		echo "Password store initialized for ${id_print%, }${id_path:+ ($id_path)}"
 		git_add_file "$gpg_id" "Set GPG id to ${id_print%, }${id_path:+ ($id_path)}."
 		if [[ -n $PASSWORD_STORE_SIGNING_KEY ]]; then
 			local signing_keys=( ) key
 			for key in $PASSWORD_STORE_SIGNING_KEY; do
-				signing_keys+=( --default-key $key )
+				signing_keys+=( --default-key "$key" )
 			done
 			$GPG "${GPG_OPTS[@]}" "${signing_keys[@]}" --detach-sign "$gpg_id" || die "Could not sign .gpg_id."
 			key="$($GPG --verify --status-fd=1 "$gpg_id.sig" "$gpg_id" 2>/dev/null | sed -n 's/^\[GNUPG:\] VALIDSIG [A-F0-9]\{40\} .* \([A-F0-9]\{40\}\)$/\1/p')"
@@ -385,7 +392,7 @@ cmd_show() {
 			echo "$pass" | $BASE64 -d
 		else
 			[[ $selected_line =~ ^[0-9]+$ ]] || die "Clip location '$selected_line' is not a number."
-			pass="$($GPG -d "${GPG_OPTS[@]}" "$passfile" | tail -n +${selected_line} | head -n 1)" || exit $?
+			pass="$($GPG -d "${GPG_OPTS[@]}" "$passfile" | tail -n +"${selected_line}" | head -n 1)" || exit $?
 			[[ -n $pass ]] || die "There is no password to put on the clipboard at line ${selected_line}."
 			if [[ $clip -eq 1 ]]; then
 				clip "$pass" "$path"
@@ -410,7 +417,8 @@ cmd_show() {
 cmd_find() {
 	[[ $# -eq 0 ]] && die "Usage: $PROGRAM $COMMAND pass-names..."
 	IFS="," eval 'echo "Search Terms: $*"'
-	local terms="*$(printf '%s*|*' "$@")"
+	local terms;
+	terms="*$(printf '%s*|*' "$@")"
 	tree -C -l --noreport -P "${terms%|*}" --prune --matchdirs --ignore-case "$PREFIX" | tail -n +2 | sed -E 's/\.gpg(\x1B\[[0-9]+m)?( ->|$)/\1\2/g'
 }
 
@@ -418,15 +426,16 @@ cmd_grep() {
 	[[ $# -lt 1 ]] && die "Usage: $PROGRAM $COMMAND [GREPOPTIONS] search-string"
 	local passfile grepresults
 	while read -r -d "" passfile; do
-		grepresults="$($GPG -d "${GPG_OPTS[@]}" "$passfile" | grep --color=always "$@")"
-		[[ $? -ne 0 ]] && continue
-		passfile="${passfile%.gpg}"
-		passfile="${passfile#$PREFIX/}"
-		local passfile_dir="${passfile%/*}/"
-		[[ $passfile_dir == "${passfile}/" ]] && passfile_dir=""
-		passfile="${passfile##*/}"
-		printf "\e[94m%s\e[1m%s\e[0m:\n" "$passfile_dir" "$passfile"
-		echo "$grepresults"
+		if grepresults="$($GPG -d "${GPG_OPTS[@]}" "$passfile" | grep --color=always "$@")";
+        	then
+			passfile="${passfile%.gpg}"
+			passfile="${passfile#$PREFIX/}"
+			local passfile_dir="${passfile%/*}/"
+			[[ $passfile_dir == "${passfile}/" ]] && passfile_dir=""
+			passfile="${passfile##*/}"
+			printf "\\e[94m%s\\e[1m%s\\e[0m:\\n" "$passfile_dir" "$passfile"
+			echo "$grepresults"
+        	fi
 	done < <(find -L "$PREFIX" -path '*/.git' -prune -o -iname '*.gpg' -print0)
 }
 
@@ -490,7 +499,8 @@ cmd_edit() {
 	set_git "$passfile"
 
 	tmpdir #Defines $SECURE_TMPDIR
-	local tmp_file="$(mktemp -u "$SECURE_TMPDIR/XXXXXX")-${path//\//-}.txt"
+	local tmp_file;
+	tmp_file="$(mktemp -u "$SECURE_TMPDIR/XXXXXX")-${path//\//-}.txt"
 
 	local action="Add"
 	if [[ -f $passfile ]]; then
@@ -533,7 +543,7 @@ cmd_generate() {
 
 	[[ $inplace -eq 0 && $force -eq 0 && -e $passfile ]] && yesno "An entry already exists for $path. Overwrite it?"
 
-	read -r -n $length pass < <(LC_ALL=C tr -dc "$characters" < /dev/urandom)
+	read -r -n "$length" pass < <(LC_ALL=C tr -dc "$characters" < /dev/urandom)
 	[[ ${#pass} -eq $length ]] || die "Could not generate password from /dev/urandom."
 	if [[ $inplace -eq 0 ]]; then
 		echo "$pass" | $GPG -e "${GPG_RECIPIENT_ARGS[@]}" -o "$passfile" "${GPG_OPTS[@]}" || die "Password encryption aborted."
@@ -555,7 +565,7 @@ cmd_generate() {
 	elif [[ $qrcode -eq 1 ]]; then
 		qrcode "$pass" "$path"
 	else
-		printf "\e[1mThe generated password for \e[4m%s\e[24m is:\e[0m\n\e[1m\e[93m%s\e[0m\n" "$path" "$pass"
+		printf "\\e[1mThe generated password for \\e[4m%s\\e[24m is:\\e[0m\\n\\e[1m\\e[93m%s\\e[0m\\n" "$path" "$pass"
 	fi
 }
 

- Chiraag
-- 
ಚಿರಾಗ್ ನಟರಾಜ್
Graduate Student at Brown University
Email: chiraag.nataraj at gmail.com
Phone: 610-350-6329
Website: http://chiraag.nataraj.us

On 18/05/19 17:22, Oliver Albertini wrote:
> >
> > I'm surprised that shellcheck didn't complain about lines 26, 58, 73,
> > 96, 98, 141 (of the patch, don't know its correspondences with the code).
> 
> 
> Most of those are inside `[[` tests, so they don't require quoting (word
> splitting doesn't happen inside `[[`). The other case is a for loop, which
> shellcheck doesn't warn about (in fact if you quote it, it will give
> you warning
> SC2066 <https://github.com/koalaman/shellcheck/wiki/SC2066>). It assumes
> you will rely on word splitting to get a list of args.
> 
> Chiraag, I'm a bit confused why the switch to using array notation for
> `$PASSWORD_STORE_GPG_OPTS`. That's not going to be an array, but just a
> string that can be set in the environment. Also, the quoting would pass the
> users opts as one string, instead of separate strings to `gpg`. Consider
> this example:
> 
> $ var="-n foo"
> $ echo $var
> foo$ echo "$var"
> -n foo
> 
> We either want to not quote, or use something like `mapfile` or `read -a`
> to put the opts into an array. I hesitate to use `mapfile` since that
> wasn't around in bash 3, which is what ships in macOS these days (many are
> using bash 5 now!). But `read -a` is a good alternative.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/password-store/attachments/20190519/824c889f/attachment-0001.asc>


More information about the Password-Store mailing list