[prev in list] [next in list] [prev in thread] [next in thread]
List: password-store
Subject: Re: [PATCH] pass generates quite a few errors/warnings with shellcheck
From: ಚಿರ <chiraag.natara
Date: 2019-05-20 15:56:28
Message-ID: 20190520155628.GA201 () chiraag
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
So the final patch:
diff --git a/src/password-store.sh b/src/password-store.sh
index 284eabf..0fe0dc5 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,14 +426,13 @@ 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
+ grepresults="$($GPG -d "${GPG_OPTS[@]}" "$passfile" | grep --color=always "$@")" \
|| 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"
+ printf "\\e[94m%s\\e[1m%s\\e[0m:\\n" "$passfile_dir" "$passfile"
echo "$grepresults"
done < <(find -L "$PREFIX" -path '*/.git' -prune -o -iname '*.gpg' -print0)
}
@@ -490,7 +497,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 +541,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 +563,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 }
Yeah, I'll probably turn my sights on the helper scripts next ;) I _should_ be able \
to just tell shellcheck that they're bash so that it will parse them correctly.
Sincerely,
Chiraag
--
ಚಿರಾಗ್ ನಟರಾಜ್
Graduate Student at Brown University
Email: chiraag.nataraj@gmail.com
Phone: 610-350-6329
Website: http://chiraag.nataraj.us
20/05/19 08:41 ನಲ್ಲಿ, Oliver Albertini <oliver.ruben@gmail.com> \
ಬರೆದರು:
> > - 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
>
> How about this?
>
> grepresults="$($GPG -d "${GPG_OPTS[@]}" "$passfile" | grep --color=always "$@")" || \
> continue
> This way, we don't need an indented if block.
>
> This patch looks good to me, and I'm glad that people are using shellcheck.
> By the way, it might be helpful to include the path to source files in a
> comment line above the source commands, that way shellcheck can do more
> thorough checking. I haven't used that a lot, but if it takes a relative
> path, it should work.
>
>
> --
>
> Oliver
["signature.asc" (application/pgp-signature)]
_______________________________________________
Password-Store mailing list
Password-Store@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/password-store
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic