Fix secret nesting amplification for write-only properties on refresh - #3113
Closed
yukimotochern wants to merge 1 commit into
Closed
Fix secret nesting amplification for write-only properties on refresh#3113yukimotochern wants to merge 1 commit into
yukimotochern wants to merge 1 commit into
Conversation
Refreshing a resource that has a secret value under a write-only path (e.g. AWS::Synthetics::Canary runConfig/environmentVariables) doubled the value's secret wrapper depth on every refresh: 1, 2, 4, 8, 16, ... Each extra wrapper roughly doubles the JSON-escaped payload in the encrypted state, so a 36-character secret grew to ~438 KiB per occurrence after four refreshes and kept doubling, eventually OOM-killing refresh. Two defects combined to cause this: 1. preserveSecretWrapper re-wrapped values that were already secret. Write-only values restored from old inputs by AddWriteOnlyOutputFallbacks are already secret, so each refresh added the old input's wrapper depth on top of the existing wrappers. 2. addWriteOnlyFallbacks aliased the old-input subtree into the new output state instead of copying it, so the re-wrap in (1) also wrote through into the parsed old inputs, which then flowed into the new __inputs checkpoint - making the growth compound across refreshes. Fix preserveSecretWrapper to never wrap an already-secret value, clone write-only fallback values, and collapse consecutive secret wrappers at the checkpoint read/write boundary (CollapseConsecutiveSecrets) so that states already poisoned by earlier versions heal on their next refresh instead of doubling further. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
PR is now waiting for a maintainer to run the acceptance tests. |
corymhall
added a commit
that referenced
this pull request
Aug 3, 2026
## Problem Refreshing a resource with a secret under a write-only path could double its secret-wrapper depth on every refresh: `1 → 2 → 4 → 8`. The refresh path reapplied secret markers to values that were already secret, and restored subtrees aliased the old inputs, allowing nested values to flow into the next `__inputs` checkpoint. ## Solution - Restore write-only values into raw CloudControl state before reapplying secret markers, treating secret-wrapped checkpoint ancestors as transparent for path lookup. - Make secret preservation idempotent and clone restored write-only subtrees. - Collapse consecutive wrappers only within write-only fallbacks, healing affected state from `4 → 1` without rewriting unrelated checkpoint values. Based on the investigation and initial fix in #3113 by @yukimotochern. This version moves healing from the global checkpoint boundary to the write-only fallback path. ## Validation - Added repeated-refresh and targeted-healing tests based on Synthetics Canary environment variables. - Added direct provider `Read` coverage for a secret parent containing a nested write-only value, plus array fallback coverage. - Ran `mise exec -- make lint` and `mise exec -- make test_provider_fast`. --------- Co-authored-by: cyan <cyan.chen@nearme.jp>
Member
|
@yukimotochern thanks for the PR! I modified your solutions slightly and merged it in #3115. I also kicked of a release so a new version should be out soon. |
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
3 participants
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
Refreshing a resource with a secret value under a write-only path (e.g.
AWS::Synthetics::CanaryrunConfig/environmentVariables) doubles the value's secret wrapper depth on every refresh: 1 → 2 → 4 → 8 → 16 → … Each wrapper roughly doubles the JSON-escaped payload in the encrypted state, so a 36-character secret grew to ~438 KiB per occurrence after four refreshes on a production stack, and eventuallypulumi refreshwas OOM-killed.Two defects combine:
preserveSecretWrapperre-wraps values that are already secret. Write-only values restored from old inputs byAddWriteOnlyOutputFallbacksare already secret, so each refresh stacks the old wrapper depth on top.addWriteOnlyFallbacksaliases the old-input subtree into the new output state instead of copying, so the re-wrap also writes through into the parsed old inputs and flows into the new__inputscheckpoint — compounding across refreshes.Fix
preserveSecretWrappernever wraps an already-secret value.addWriteOnlyFallbacksclones restored values instead of aliasing old inputs.CollapseConsecutiveSecretscollapsesSecret(Secret(x))→Secret(x)at the checkpoint read/write boundary (ParseCheckpointObject/CheckpointPropertyMap), so states already poisoned by earlier versions heal on their next refresh.Tests
__inputsover 20 simulated refreshes (fails with depth 2 on master at iteration 0).PreserveSecretWrappers, and the no-alias behavior ofAddWriteOnlyOutputFallbacks.🤖 Generated with Claude Code