fix: fail closed on CLI input, and confirm the full footprint - #37
Merged
Conversation
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.
|
Commit: Security Scan Results
|
# Conflicts: # scripts/check-deploy-config.sh
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
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.
What
Usability-review blocker 2: invalid CLI input silently escalated to "deploy everything". Three inputs, one hole — the arg parser's
*) shift ;;discarded anything unrecognized, profile/team values were validated only for theworkshopaction, and with no resolved target execution fell through tocdk deploy --all --require-approval neverwith no confirmation:deploy --profile greenfieddeploy --stakc identity/--stack=identitydeploy --team agentsdeploy --stack identity(short name)Fixes
--opt=valueform are errors that name the valid option set.--profile/--teamvalues are validated for every action, listing the valid values.--stackvalues must be full stack names — prefix-checked before any AWS call, withdeploy.sh lsas the pointer.deployordestroywith no target prints the plan first — account, region, config source (platform.yaml/workshop.env/ defaults), and the exact stack list fromcdk ls— then asks.--yesskips the prompt;NON_INTERACTIVE=1implies--yes, so CI flows are byte-for-byte unchanged.Checks
Two new self-checks in
check-deploy-config.sh:deploy.shand asserts every bad input above exits non-zero at parse/validation time — before credentials are ever checked — and that--yesparses as a flag rather than dying as an unknown option.confirm_footprintthrough all four paths (--yesskip,NON_INTERACTIVEskip, answer-nabort with non-zero exit, answer-yproceed) withnpxstubbed.Verified
17 self-checks green,
workshop --dry-runend-to-end green, shellcheck clean, workshop-flow checks green. No shared files with #35 or #36 — any merge order works.