Skip to content

Conversation

@robbat2
Copy link

@robbat2 robbat2 commented Dec 11, 2024

Redoing #1 work - starting with test suite before any changes.

Signed-off-by: Robin H. Johnson robbat2@gentoo.org

Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Please use consistent coding style, fix whitespace and add error handling, for a start.

@robbat2 robbat2 requested a review from mgorny December 16, 2024 06:56
@robbat2
Copy link
Author

robbat2 commented Dec 16, 2024

@mgorny you can recheck it; it does pass on woodpecker btw

Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
@robbat2
Copy link
Author

robbat2 commented Dec 16, 2024

What format-checker do you recommend then since neither shfmt-3.1.2 nor shellcheck-0.10.0 are catching these, and I missed two inconsistencies only.

@robbat2 robbat2 requested a review from mgorny December 16, 2024 07:14
@mgorny
Copy link
Member

mgorny commented Dec 16, 2024

I have never used a formatter for shell code.

export AUTOSIGN_TMPDIR GNUPGHOME
echo "Testing tmpdir is ${AUTOSIGN_TMPDIR}"

umask 077 && mkdir -p "${GNUPGHOME}"
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

echo "Testing tmpdir is ${AUTOSIGN_TMPDIR}"

umask 077 && mkdir -p "${GNUPGHOME}"
[[ -n ${AUTOSIGN_NO_SEND_KEYS} ]] || echo "keyserver ${LOCAL_KEYSERVER}" >"${GNUPGHOME}"/gpg.conf
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

# this is to ensure there are other secret keys, and we aren't just lucky with the
# code picking a random key that was the correct one for the key.
for f in {1..4}; do
gpg --batch --passphrase '' --quick-gen-key "Dummy Key 0x$(printf %04x "${f}")"
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

}

echo "Generating authority keys ..."
gpg -q --batch --passphrase '' --quick-gen-key "${authuid_foo}"
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

echo "Generating authority keys ..."
gpg -q --batch --passphrase '' --quick-gen-key "${authuid_foo}"
authfpr_foo=$(
gpg --with-colon --list-secret-keys "${authuid_foo}" | gpg_colon_get_sec_fpr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
gpg --with-colon --list-secret-keys "${authuid_foo}" | gpg_colon_get_sec_fpr
gpg_colon_get_sec_fpr < <(gpg --with-colon --list-secret-keys "${authuid_foo}")

Also no error handling.

gpg --with-colons --check-sig \
--trusted-key "${authfpr_foo}" \
--trusted-key "${authfpr_bar}" \
>"${AUTOSIGN_TMPDIR}"/verification-${n}.txt
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

gpg --with-colons --check-sig \
--trusted-key "${authfpr_foo}" \
--trusted-key "${authfpr_bar}" \
>"${AUTOSIGN_TMPDIR}"/verification-${n}.txt
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

<(grep -E -e '^uid:.:.*\<a[^@]+@gentoo.org>:' <"${AUTOSIGN_TMPDIR}"/verification-03.txt) \
<(grep -E -e '^uid:.:.*\<a[^@]+@gentoo.org>:' <"${AUTOSIGN_TMPDIR}"/verification-04.txt) \
-I '^tru:' \
>"${AUTOSIGN_TMPDIR}"/diff-03-04.txt
Copy link
Member

Choose a reason for hiding this comment

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

No error handling.

<(grep -E -e '^uid:.:.*\<a[^@]+@gentoo.org>:' <"${AUTOSIGN_TMPDIR}"/verification-04.txt) \
-I '^tru:' \
>"${AUTOSIGN_TMPDIR}"/diff-03-04.txt
cat "${AUTOSIGN_TMPDIR}"/diff-03-04.txt
Copy link
Member

Choose a reason for hiding this comment

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

Is that debug?

Comment on lines +157 to +158
AUTOSIGN_TMPDIR=${AUTOSIGN_TMPDIR}/${n}-foo/ AUTOSIGN_GPG_LOCAL_FPR=${authfpr_foo} bash ./autosign.bash
AUTOSIGN_TMPDIR=${AUTOSIGN_TMPDIR}/${n}-bar/ AUTOSIGN_GPG_LOCAL_FPR=${authfpr_bar} bash ./autosign.bash
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to check for unsuccessful return here too.

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.

2 participants