From 9d7a61e6bdd38847a5dd669da58fc86a4e1700e9 Mon Sep 17 00:00:00 2001 From: Roberto Catalano Date: Tue, 25 Aug 2026 11:58:50 +0200 Subject: [PATCH] fix: fail closed on CLI input, and confirm the full footprint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Usability-review blocker 2. Three ways a typo produced a LARGER deployment than asked, all through the same hole: the arg parser's `*) shift;;` discarded anything it didn't recognize, profile/team values were validated only for the workshop action, and with no resolved target execution fell through to `cdk deploy --all --require-approval never` with no confirmation. deploy --profile greenfied → flags silently skipped → full deploy, A2A on deploy --stakc identity → flag discarded → full deploy deploy --stack identity → "No stacks match" AFTER bootstrap already ran Fixes: - Unknown options and the --opt=value form are errors naming the valid set. - --profile / --team values are validated for every action, not just workshop. - --stack values must be full stack names (prefix-checked before any AWS call, with `deploy.sh ls` as the pointer). - A deploy or destroy with NO target now prints the plan first — account, region, config source (platform.yaml / workshop.env / defaults), and the exact stack list from `cdk ls` — and asks. --yes skips the prompt; NON_INTERACTIVE=1 implies --yes, so CI flows are unchanged. Checks (n) and (o) in check-deploy-config.sh: (n) invokes the REAL script and asserts every bad input above exits non-zero at parse/validation time, pre-credentials, and that --yes parses as a flag rather than dying as an unknown option; (o) drives the extracted confirm_footprint through all four paths (--yes skip, NON_INTERACTIVE skip, answer-n abort, answer-y proceed) with npx stubbed. Verified: 17 self-checks green, workshop --dry-run end-to-end green, shellcheck clean, check-workshop-flow green. --- scripts/check-deploy-config.sh | 42 +++++++++++++++++++ scripts/deploy.sh | 77 ++++++++++++++++++++++++++++++---- 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/scripts/check-deploy-config.sh b/scripts/check-deploy-config.sh index 2b2bc46..f3d0f9e 100755 --- a/scripts/check-deploy-config.sh +++ b/scripts/check-deploy-config.sh @@ -242,4 +242,46 @@ grep -q -- "put-secret-value --secret-id my-corp/entra-secret" "$TMP/aws.args" \ unset -f aws prompt_idp echo "PASS: a configured IdP secret name is reused, not duplicated" +# (n) invalid CLI input fails closed, before any AWS call. A typo'd option +# used to be silently discarded; with no resolved target the run escalated to +# `cdk deploy --all --require-approval never`. These invoke the REAL script: +# every rejection below happens at parse/validation time, pre-credentials. +run_deploy() { (cd "$REPO_ROOT" && "$BASH" scripts/deploy.sh "$@") } + +out=$(run_deploy deploy --bogus-flag 2>&1) && fail "unknown option was accepted" +echo "$out" | grep -q -- "--bogus-flag" || fail "unknown option not named: $out" + +out=$(run_deploy deploy --stack=identity 2>&1) && fail "--stack=NAME form was accepted" + +out=$(run_deploy deploy --profile greenfied 2>&1) && fail "misspelled profile was accepted" +echo "$out" | grep -q "greenfield" || fail "valid profiles not listed: $out" + +out=$(run_deploy deploy --team agents 2>&1) && fail "unknown team was accepted" + +out=$(run_deploy deploy --stack identity 2>&1) && fail "short stack name was accepted" +echo "$out" | grep -q -- "-identity" || fail "full-name hint missing: $out" + +# --yes must parse as a flag (not hit the unknown-option arm): with it present +# the run must get PAST parsing and die on the misspelled profile instead. +out=$(run_deploy deploy --yes --profile greenfied 2>&1) && fail "--yes+bad profile accepted" +echo "$out" | grep -q "Unknown profile" || fail "--yes not parsed as a flag: $out" +echo "PASS: invalid CLI input fails closed before any AWS call" + +# (o) the full-footprint gate: --yes and NON_INTERACTIVE skip it; an answer +# of anything but y aborts with a non-zero exit and no mutation. +eval "$(sed -n '/^confirm_footprint()/,/^}/p' "$SCRIPT_DIR/deploy.sh")" +log_header() { :; }; log_warn() { :; }; log_error() { :; } +# shellcheck disable=SC2329 # invoked from inside the eval'd function +npx() { echo "stack-a"; echo "stack-b"; } +CONTEXT_ARGS=(); PLATFORM_CONFIG="$TMP/absent.yaml" +# shellcheck disable=SC2034 # YES/NON_INTERACTIVE are read by the eval'd function +YES=1 && confirm_footprint deploy || fail "--yes did not skip the gate" +YES=0 NON_INTERACTIVE=1 confirm_footprint deploy || fail "NON_INTERACTIVE did not imply --yes" +# shellcheck disable=SC2034 +NON_INTERACTIVE=0 +( YES=0 confirm_footprint destroy <<< "n" ) >/dev/null 2>&1 && fail "answering n did not abort" +( YES=0 confirm_footprint destroy <<< "y" ) >/dev/null 2>&1 || fail "answering y did not proceed" +unset -f npx +echo "PASS: full-footprint gate honors --yes / NON_INTERACTIVE and aborts on n" + echo "OK: all deploy-config checks passed" diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 41d7845..cf68f93 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -563,6 +563,35 @@ build_context_args() { return 0 } +# ═══════════════════════════════════════════════════════════════ +# Full-footprint confirmation (deploy --all / destroy --all) +# ═══════════════════════════════════════════════════════════════ +# A deploy or destroy with no target touches EVERYTHING the app synthesizes, +# so it shows the account, the config source, and the exact stack list first. +# --yes skips the prompt; NON_INTERACTIVE=1 implies --yes (CI compatibility). +confirm_footprint() { + # $1 = verb ("deploy" | "destroy") + [ "$YES" = "1" ] && return 0 + [ "${NON_INTERACTIVE:-0}" = "1" ] && return 0 + echo "" + log_header "Plan — $1 the FULL footprint" + log_info "Account: ${ACCOUNT_ID:-unknown} Region: ${AWS_REGION:-us-east-1} Prefix: ${PREFIX}" + local src="built-in defaults" + [ -f "$CONFIG_FILE" ] && src="workshop.env" + [ -f "$PLATFORM_CONFIG" ] && src="platform.yaml" + log_info "Config source: $src" + log_info "Stacks:" + JSII_SILENCE_WARNING_UNTESTED_NODE_VERSION=1 \ + npx cdk ls "${CONTEXT_ARGS[@]}" 2>/dev/null | sed 's/^/ /' \ + || log_warn "(could not synthesize the stack list)" + local ans + read -rp "Proceed to $1 ALL of the above? [y/N]: " ans + case "$ans" in + [Yy]*) : ;; + *) log_error "Aborted before any change. Re-run with --yes to skip this prompt."; exit 1 ;; + esac +} + # ═══════════════════════════════════════════════════════════════ # Stack Deploy Loop (shared by the deploy and workshop actions) # ═══════════════════════════════════════════════════════════════ @@ -757,6 +786,7 @@ TEAM="" MODULE="" FROM_MODULE="" DRY_RUN=0 +YES=0 require_flag_value() { # $1 = flag name, $2 = number of remaining args after the flag if [ "$2" -lt 2 ]; then @@ -772,18 +802,46 @@ while [[ $# -gt 0 ]]; do --module) require_flag_value "--module" "$#"; MODULE="$2"; shift 2 ;; --from) require_flag_value "--from" "$#"; FROM_MODULE="$2"; shift 2 ;; --dry-run) DRY_RUN=1; shift ;; - *) shift ;; + --yes) YES=1; shift ;; + *) + # Fail closed: `--stakc identity` or `--stack=identity` used to be + # silently discarded here, leaving no target and escalating to + # `cdk deploy --all`. A typo must never deploy more than asked. + log_error "Unknown option or argument: '$1'" + log_error "Valid options: --stack --profile --team --module --from --dry-run --yes" + log_error "(values are space-separated: --stack NAME, not --stack=NAME)" + exit 1 + ;; esac done -# Workshop action: default and validate the profile against the guided sequences. -if [ "$ACTION" = "workshop" ]; then - PROFILE="${PROFILE:-greenfield}" - if [ -z "${PROFILE_MODULES[$PROFILE]:-}" ]; then - log_error "Unknown profile: '$PROFILE'. Valid profiles: ${!PROFILE_MODULES[*]}" - exit 1 - fi +# Workshop action: default the profile. +[ "$ACTION" = "workshop" ] && PROFILE="${PROFILE:-greenfield}" + +# ── Fail closed on VALUES, for every action ── +# A misspelled --profile used to be validated only for the workshop action: +# on plain deploy it skipped its feature flags without a word and fell +# through to `cdk deploy --all` — a typo produced a LARGER deployment. +if [ -n "$PROFILE" ] && [ -z "${PROFILE_FLAGS[$PROFILE]:-}" ]; then + log_error "Unknown profile: '$PROFILE'. Valid profiles: ${!PROFILE_FLAGS[*]}" + exit 1 +fi +if [ -n "$TEAM" ] && [ -z "${TEAM_MAP[$TEAM]:-}" ]; then + log_error "Unknown team: '$TEAM'. Valid teams: ${!TEAM_MAP[*]}" + exit 1 fi +# --stack takes full stack names; a short name like 'identity' would only +# fail deep inside cdk ("No stacks match") after bootstrap has already run. +for _s in ${STACK_FILTER:-}; do + case "$_s" in + "$PREFIX"-*) : ;; + *) + log_error "Unknown stack: '$_s' — stacks are full names like ${PREFIX}-identity." + log_error "List them with: $0 ls" + exit 1 + ;; + esac +done # Apply profile flags if [ -n "$PROFILE" ] && [ -n "${PROFILE_FLAGS[$PROFILE]:-}" ]; then @@ -853,6 +911,7 @@ case "$ACTION" in # shellcheck disable=SC2086 # stack list is space-separated by design deploy_stacks $CDK_STACKS else + confirm_footprint deploy log_step "Deploying all stacks..." JSII_SILENCE_WARNING_UNTESTED_NODE_VERSION=1 \ npx cdk deploy --all --require-approval never "${CONTEXT_ARGS[@]}" 2>&1 || { @@ -922,6 +981,7 @@ case "$ACTION" in destroy) log_header "Destroying" + [ -z "${CDK_STACKS:-}" ] && confirm_footprint destroy # No stacks selected means --all. Failures are no longer swallowed: a # refused destroy is the guard against cascading, so it has to be seen. # shellcheck disable=SC2086 # stack list is space-separated by design @@ -962,6 +1022,7 @@ case "$ACTION" in echo " --module N Workshop module number (3|4|5|6|7|8|9|A|B|C|D|E)" echo " --from MODULE (workshop) Start at this module in the profile sequence" echo " --dry-run (workshop) Print each module's stacks + verify command; no AWS calls" + echo " --yes Skip the full-footprint confirmation (deploy/destroy with no target)" echo "" echo "Environment Variables:" echo " NON_INTERACTIVE=1 Skip all prompts"