fix(enroll): seventh audit — atomic pw write, grep trap, key validation, CV promotion guard#179
Merged
Merged
Conversation
…on, CV promotion guard - Atomic KATELLO_ADMIN_PW_FILE write: printf > .tmp then mv; interrupted write previously left an empty file with no recovery path (password only recoverable from .env manually) - Fix two remaining `docker compose` (v2) occurrences in die/warn messages that audit #6 missed: container count die and Foreman timeout warn block - Guard SIGNING_PUBKEY grep against pipefail abort: `grep -v` exits 1 when every line matches the invert pattern; with set -o pipefail this fires set -e before the empty guard can print a diagnostic message; add `|| true` within the $() so the guard always runs - Validate nix-store key file sizes after generation: `[[ -s KEY && -s PUB ]]` immediately after nix-store --generate-binary-cache-key; a zero-byte secret key would cause harmonia to start but fail all signing silently - Validate minisign .minisig is non-empty after signing: `[[ -s SIG ]]` after minisign -S; some minisign versions exit 0 on edge-case failures - Broaden minisign -S die message: previously blamed only passphrase; now lists corrupt key, unreadable permissions, and disk-full as causes - CV promotion container liveness check: `docker ps --filter name=katello-foreman` before hammer calls; a stopped container caused docker exec to silently fail (2>/dev/null) and || branch to print "Already at env" — looked like success but promotion was silently skipped - Rename `env` loop variable to `_promote_env`: avoids collision with the common `env` command/variable name in the for-loop scope
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.
Findings and fixes
echo > KATELLO_ADMIN_PW_FILEnon-atomic — interrupted write leaves empty file, password unrecoverable except from.envprintf > .tmp && mvdocker compose(v2) occurrences in die/warn messages missed by audit #6docker-composegrep -vin SIGNING_PUBKEY extraction exits 1 on pathological pub file →set -efires before the empty guard can print a diagnostic[[ -s KEY && -s PUB ]]validation afternix-store --generate-binary-cache-key[[ -s SIG ]]afterminisign -S— some versions exit 0 on failure[[ -s "${MINISIGN_CACHE_INFO_SIG}" ]]after signingminisign -S || diemessage blames passphrase for any failure (corrupt key, perms, disk full)docker execfailure masked as "Already at env" —2>/dev/nullon stopped container looks like successfor env in candidate stable—envis a common variable/command name; shadowing risk withset -u_promote_envTest plan
bash -n scripts/enroll.sh— no syntax errorsKATELLO_ADMIN_PW_FILEwrite → re-run regenerates fileset -esilent abort)nix-store --generate-binary-cache-key, zero out HARMONIA_KEY →[[ -s ]]guard fireskatello-foremancontainer before step 11 → CV promotion block warns and continues rather than printing false "Already at"