Repository navigation
fix(deploy): keep a NoEcho-named resource's ARN in its attributes, outputs and exports (#4849) - #4850
Merged
Merged
Conversation
…tputs and exports (#4849) A resource named from a NoEcho parameter stored its own ARN attribute, and every output and export holding that ARN, as ***, so the next deploy that re-resolved a Fn::GetAtt reader of it failed. The persist walk now exempts a copy of the record's own physical id among its attributes, the outputs bag carries the physical-name copies its passes read, and a nested stack row records a child physical id its outputs copy. An attribute or output equal to the value itself stays masked. Adds the noecho-named-producer integ fixture (edited reader on a second deploy, a consumer importing the export in a later run).
…scope the nested carry to attributes (#4849) Review round 1 on #4850: - A Fn::GetAtt of an attribute the record holds as *** re-reads the producer through the stale-attribute heal and serves the read only when it equals the record's physical id; the persist merge replaces the mask under the same predicate. Declared NoEcho attributes are never re-read. - The nested-stack carry records child ids in an attributes-only side set, so the row's Parameters keep their reference form. - Qualify the docs and changelog where the physical id is the value itself. - Tests: the repair, the declared refusal, the predicate, the nested scope and guards, and the rollback attributes negative. - Integ: structural no-replacement check, and the exports index versions are scanned too.
…one equal to it (#4849) Review round 2 on #4850: a name-identified resource's ARN (a function, a role, a table named from a NoEcho value) contains its physical id rather than equalling it. The repair now also accepts a re-read value that carries the id and is left unchanged by the producer's own redaction bag with that id exempted. Integ nits: the success line names the subscription, and the exports index scan reads only versions this run wrote.
Review round 3 on #4850: the producer's bag for this run cannot vouch for an earlier generation's secret it no longer records, so a value carrying the id is accepted only as an ARN ending in it, with nothing but partition, service, region, account and one resource-type token before it. Integ: the index scan window allows five minutes of clock skew and fails on zero index versions.
The SNS subscription provider replaces a subscription on any update (filed as #4853), so the edited deploy's subscription ARN changes by design. Phase 3 now asserts the deploy exits 0 without the redaction-mask refusal, the topic's one live subscription is the recorded one, delivers to the queue and has raw delivery on. A CDKD_TEST_NOECHO_PRODUCER_ARM=1 pre-fix arm warns through Phase 1 and skips Phase 2 so a binary without the fix fails at Phase 3.
…cer-attr-copy # Conflicts: # docs/_generated/integ-coverage.json # docs/integ-coverage.md
Owner
Author
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.
Summary
A resource named from a
NoEchotemplate parameter (an SNS topic withTopicName: !Ref Name) stored its own ARN attribute, and every output and export holding that ARN, as***. The next deploy that re-resolved a reader of the attribute (an edited subscription or grant usingFn::GetAtt Topic.TopicArn) failed withCannot resolve Topic.TopicArn ... redaction maskand rolled back; a consumer stack importing the exported ARN in a later run was refused the same way.The #4810 / #4833 rule keeps a copy of a physical id in plaintext, but it covered only the ids a reader's bag recorded. This PR closes the three places that fell outside it:
scrubResourceRecord(src/deployment/secret-redaction/redact-state.ts): a record'sattributesare redacted with the record's ownphysicalIdseeded as a physical-name copy (withPhysicalNameCopies), so a match inside it (theTopicArn) is kept. An attribute equal to the value whole (TopicName) is no copy of the id and stays***, unless the physical id IS the value (a name-identified resource), where it is stored as the id is.absorbOutputsPassSecrets(src/deployment/deploy-engine/masking.ts): the outputs bag now carriesphysicalNameCopiesOffrom each outputs pass, so an!Ref Topicoutput and its export (and the exports index, written fromredactOutputs) keep the ARN. An output of the value itself stays***.NestedStackProvider.readChildOutputsAsAttributes: a child output holding a child resource's physical id is recorded in a new attributes-only side set (attributeIdCopiesOf) on the row's bag, whichscrubResourceRecordseeds into the attributes walk alongside the own id. The row'sproperties(itsParameters) are untouched and keep their reference form.TopicArn: "***"already persisted): aFn::GetAttof an attribute the record holds as exactly***re-reads the producer through the fix(deployment): an issue-1824-era stale attribute map is never healed, and the resolver "not enriched for this resource type" message is now false for the two fixed types #1852 heal (healMaskedOwnIdAttributeinintrinsic-resolver/getatt-heal.ts, called fromresolveGetAtt's cached-attribute arm) and serves the read only when it EQUALS the record's physical id, or is an ARN ENDING in it whose prefix holds only partition, service, region, 12-digit account and at most one resource-type token (OWN_ID_ARN_PREFIX), AND is left unchanged by the producer's own redaction bag for this run with that id exempted (the bag must hold a maskable value): a function's or role'sArn(isMaskedOwnIdCopy). The shape bounds an earlier generation's secret that today's bag no longer records (security review round 2); a role ARN with a path, or anything else beside the id, keeps the old refusal. The persist merge (withHealedAttributes) replaces the mask under the same predicate, so the record is repaired. Never for an attribute the producer declaredNoEcho(a malformednoEchoAttributeNamesreads as declared), never for any other value (TopicNamestays***). The cost is one provider read, only for a masked attribute a reader asks for, memoized per record per deploy.Behavior note for the dynamic-reference class: the own-id exemption applies to the substring arm as well, so an attribute holding the record's own physical id (secret-derived name) is now stored as the id is rather than rewritten into the reference. The physical id itself is already stored in plaintext beside it, so nothing new is disclosed;
rollback-restore-secret-named-copies.test.tsis updated accordingly. Accepted residual (security review N1): a leaf EQUAL to a physical id is kept before the whole-value arm, so a different secret whose value happens to equal that id (a short handler-chosen id) is stored in plaintext rather than as its reference. No new characters reach state (the id is already there); this is the same residual #4810 accepted forRef/Fn::GetAttreads. TheNoEchoclass is still covered by the positional arm.An attribute that only contains the name (a queue's
Arn, whose physical id is its URL) is unchanged: it still stores the reference, or***for aNoEchovalue (documented as a known bound indocs/state-schema-internals.md).Tests
tests/unit/deployment/noecho-producer-attr-copy-4849.test.ts(realDeployEngine/DiffCalculator/ resolver, mocked provider): own ARN attribute kept andTopicNamemasked; a later deploy editing theFn::GetAttreader succeeds; output + export keep the ARN, the value's output stays***; negative control (a literal embedding the value stays***).tests/unit/provisioning/nested-stack-provider-output-id-copy-4849.test.ts: the nested carry, plus a negative control (an output only embedding the value records nothing and stays***).noecho-producer-attr-copy-4849.test.tsalso covers the repair (a record seeded withTopicArn: "***"is repaired by the reader edit,TopicNamestays***), a declared attribute that is not re-read into the record, andisMaskedOwnIdCopydirectly (equality only, declared and malformed declarations refused).nested-stack-provider-output-id-copy-4849.test.tscovers the attributes-only scope (the row'sParameterskeep the reference), no bound bag, and a non-object childresources.rollback-restore-secret-named-copies.test.ts: the "differs, therefore redact" attributes negative is kept with aTagattribute holding the name outside the own id.redact-state.tsown-id seed -> 2 cases red;masking.tscarry -> the outputs case red; nested carry removed -> the nested case red; nested carry into the whole bag -> the scope case red; attributes-only side set ignored by the scrub -> 2 nested cases red;resolveGetAttserve arm off -> the repair case red; persist-merge predicate off -> the repair case red; declared check off -> the predicate case red; equality relaxed to any string -> the predicate case red; containment arm off -> the name-identified engine case and the predicate case red; redaction-stability check off -> the predicate case red; maskable-bag guard off -> the predicate case red; ARN-shape check off -> the predicate case red; end-of-value check off -> the predicate case red; the producer bag dropped at the resolver or at the merge -> the name-identified engine case red.Integration
New fixture
tests/integration/noecho-named-producer/(kept out ofnoecho-parameter-masking, which PR #4848 holds, so the two PRs do not collide). It deploys a topic named from a per-runNoEchovalue with an SQS subscription readingFn::GetAtt Topic.TopicArnand an exported!Ref Topicoutput; asserts the ARN in state, outputs, export and exports index with***atTopicNameand the value's output; deploys a consumer importing the export in a separate process; then redeploys the producer with the subscription edited (raw delivery on) and asserts the deploy exits 0 with noredaction maskrefusal, the topic's one live subscription is the one state records, delivers to the queue and has raw delivery on, and state keeps the ARN withTopicNameas***. (cdkd's SNS subscription provider replaces a subscription on any update, so its ARN is not asserted unchanged; that replacement is filed as #4853.) Then it scans every object version under both stack prefixes and the exports index versions this run wrote (with a floor of one); destroys and sweeps.Integ results on 1d111f2 (coordinator):
noecho-parameter-masking,nested-stack-secret,custom-resource-noecho-nested,cross-stack-secret-import,rollback-explicit-name-rewriteand the broad set passed;noecho-named-producerfailed at Phase 3 on the "subscription ARN unchanged" premise only (the edited deploy itself succeeded), which this head fixes. Pre-fix arm (src at the merge base): red at Phase 1 (state holds***at TopicArn, output and export). WithCDKD_TEST_NOECHO_PRODUCER_ARM=1the fixture warns through Phase 1, skips Phase 2, and is expected red at Phase 3 (theCannot resolve Topic.TopicArn ... redaction maskrefusal); the arm always exits non-zero.Suggested
/run-integ:noecho-named-producer,noecho-parameter-masking,nested-stack-secret,custom-resource-noecho-nested,cross-stack-secret-import,rollback-explicit-name-rewrite.Closes #4849