enroll: R14 — || die guards on remaining unguarded commands#186
Merged
Conversation
Six findings, all LOW/MEDIUM severity: - MEDIUM: minisign -G had no || die; non-zero exit fired set -e before the [[ -s ]] guard (from R13) could run, leaving no remediation hint. - LOW: mkdir -p / chmod 700 on SOURCEOS_DIR used && chaining with no || die; split into two statements each with || die. - LOW: install -m 600 for COMPOSE_ENV had no || die; added with disk/perm hint. - LOW: printf → .tmp && mv for AGE_PUB and KATELLO_ADMIN_PW_FILE had no || die on the combined write; added || die at end of each && chain. - LOW: cat > nix-cache-info.tmp <<EOF and its mv had no || die; wrapped heredoc in a group command and added || die to both the write and the mv.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round 14 audit findings
minisign -G(step 8)|| die— non-zero exit firesset -ebefore the[[ -s ]]guard from R13 can print a diagnosticmkdir -p / chmod 700(step 0)&&-chained with no|| die; split into two statements each with guardinstall -m 600(step 4)|| dieon.envcopyprintf → .tmp && mvforage.pub(step 3)|| dieprintf → .tmp && mvforkatello-admin-pw(step 4)cat > nix-cache-info.tmp <<EOF+mv(step 8)|| die; heredoc wrapped in group commandChanges
minisign -G ... || die "..."with diagnose hintmkdir -p ... || die+chmod 700 ... || die(two lines)install -m 600 ... || dieprintf ... && mv ... || diefor both atomic password file writes{ cat > .tmp <<EOF ... EOF } || die+mv .tmp final || diefor nix-cache-infoAll six changes are mechanical
|| dieadditions to commands that were already correct but lacked operator-visible diagnostics on failure.Test plan
bash -n scripts/enroll.shpasses (verified)if [[ ! -f ]]or fresh-generate branches)