chore/deploy-sh-hardening: improved failure handling - #72
Merged
Conversation
cover that the API-key pepper existence check treated every read failure as absence, so expired credentials read as 'not found' and entered the generate branch — only a genuine ParameterNotFound does now, and any other failure aborts naming the real cause, with a comment recording that the absence of overwrite on the store call is a deliberate safety property protecting existing API-key hashes. Stack deploys now classify a failed attempt before retrying: deterministic client-side errors fail fast with the root cause line instead of burning a guaranteed-identical retry, the container runtime is re-checked before every attempt so a mid-run disappearance is named precisely, and the diff step drops an unsupported flag and names its failures instead of swallowing them. A self-contained harness under scripts proves each behaviour with PATH-shimmed fakes, including the exact credential-failure and runtime-disappearance shapes from the two incidents that motivated this.
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.
Summary
Mitigates two deploy failure path edge cases: deploy.sh reporting confident states its inputs didn't support.
--overwriteon the store call is documented as a deliberate safety property (replacing the pepper invalidates every issued API-key hash).spawnSync docker ENOENT) fail fast with the root cause; CloudFormation transients keep the single retry. The container runtime is re-checked before every attempt, so a mid-deploy Docker disappearance is named precisely.cdk diffdrops the unsupported--allflag and names its failures instead of swallowing them with|| true.Testing
scripts/test-deploy-sh.sh(tracked via a .gitignore exception): 16/16 assertions using PATH-shimmed fake aws/cdk/docker binaries, including the exact credential-failure and runtime-disappearance shapes from both incidents. The harness is bite-proven - reverting either fix fails it by name. Default execution is byte-equivalent: all changes are function-internal; the test seam is opt-in.bash -nclean; secret value verified never to reach stout or logs.Notes
No infrastructure delta. Behaviour changes only on failure paths.