fix: Fn::GetAtt with a nested intrinsic crashes on dump (#120)#122
Open
earfman wants to merge 2 commits into
Open
fix: Fn::GetAtt with a nested intrinsic crashes on dump (#120)#122earfman wants to merge 2 commits into
earfman wants to merge 2 commits into
Conversation
GetAtt collapses its list into the dotted short form via ".".join(value), which assumes every element is a string. A GetAtt with a nested intrinsic (e.g. Fn::FindInMap, allowed by AWS::LanguageExtensions) is an ODict, so the join raises TypeError on dump. Collapse to the dotted short form only when all elements are strings; otherwise emit the long-form Fn::GetAtt mapping, which round-trips and is valid CloudFormation.
Author
Coretexa gate report —
|
| check | result | by | detail |
|---|---|---|---|
repro |
✓ pass | verifier-cfnflip |
clean clone: nested-intrinsic GetAtt → TypeError: sequence item 0: expected str instance, ODict found, at both operand positions |
fix-works |
✓ pass | verifier-cfnflip |
after patch, #120 template dumps to long-form Fn::GetAtt: with nested !FindInMap; no crash |
round-trip |
✓ pass | verifier-cfnflip |
dumped output reloads with no error and is byte-identical on re-dump; to_json(to_yaml(x)) structure-equal |
naive-fix-rejected |
✓ pass | verifier-cfnflip |
independently confirmed the !GetAtt [ !FindInMap ] short-tag form FAILS to load (construct_getatt reads raw scalar nodes) — long form is required, not optional |
no-over-reach |
✓ pass | verifier-cfnflip |
all-string GetAtt still dotted; Ref/Sub elements, GetAtt-in-GetAtt, first/second-intrinsic all round-trip |
full-suite |
✓ pass | verifier-cfnflip |
baseline 91 pass / 1 fail → patched 91 pass / 1 fail (dumper hunk only); the 1 fail is a pre-existing env-only test_cli_with_version, present in baseline. With the new test file: 93 pass. |
All required checks have passing evidence: repro, fix-works, round-trip, no-over-reach, full-suite
Verification
--- VERIFIED 2026-07-13 by verifier-cfnflip (independent session, not the doer) ---
Gate 1: reproduced the crash on a fresh clone with own template (both operand positions).
Gate 2: the FIRST fix attempt (dumper-only, emit !GetAtt short-tag sequence) stopped the dump
crash but produced YAML that cfn-flip's own loader could not re-parse. Caught here and
rejected; corrected to emit the long-form Fn::GetAtt mapping, which round-trips.
Gate 3: patch touches only the crashing branch (+14/-1); all-string path byte-for-byte unchanged.
Gate 4: adversarial inputs (Sub element, deep nesting, mixed positions, empty list) all safe;
the one 3-part dotted-form asymmetry is a PRE-EXISTING library quirk, unchanged by the fix.
Verified by a second agent · claimer ≠ verifier · coretexa.dev
Contributor
|
Nice to see a code written 100% by humans years ago got a fix from AI. Considering it's a minor fix, we humans did a good job on this app. |
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.
Issue #, if available: #120
Description of changes:
What
Fn::GetAttwhose elements include a nested intrinsic (e.g.Fn::FindInMapinsideGetAtt, whichAWS::LanguageExtensionsallows) crashes on dump:Fixes #120.
Why
fn_representer(cfn_flip/yaml_dumper.py) collapses aGetAttlist into the dotted short form via".".join(value), which assumes every element is a string. A nested intrinsic is anODict, so the join raises. This is a valid CloudFormation shape, so cfn-flip shouldn't crash on it.The fix
Collapse to the dotted short form only when all elements are strings; otherwise emit the long-form
Fn::GetAttmapping.The long form is used deliberately rather than the
!GetAtt [ ... ]short-tag sequence: the loader'sconstruct_getattreads short-form!GetAttelements as raw scalar node values ([s.value for s in node.value]), so a!GetAttshort tag wrapping a nested!FindInMapfails to load. The long-formFn::GetAttmapping round-trips cleanly through the existing machinery and is valid CloudFormation.Tests
Added
tests/test_getatt_intrinsic.py:test_flip_to_yaml_with_getatt_containing_intrinsic—GetAttwith a nestedFindInMapemits long form and round-trips throughto_json(to_yaml(x)). Fails onmasterwith the exactTypeError; passes with this change.test_flip_to_yaml_getatt_all_strings_still_dotted— guards the common case: an all-stringGetAttstill collapses to!GetAtt 'Res.Attr'.Full suite: 91 → 93 pass (2 new tests). Scope: the single crashing branch (+14/-1) plus a two-test file; no behavior change for all-string
GetAtt, other intrinsics, or any non-GetAttpath.This change was prepared with AI assistance and independently verified against the project's full test suite before submission — reproduced on a clean clone, checked for round-trip correctness and sibling-case regressions, and confirmed to fail without the change.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.