fix(wafv2): decode SearchStringBase64 and rename reference-statement Arn to the SDK ARN - #1405
Merged
Merged
Conversation
go-to-k
force-pushed
the
fix/1389-wafv2-search-string-base64
branch
from
August 9, 2026 05:58
bb7820d to
e5ffba2
Compare
go-to-k
force-pushed
the
fix/1389-wafv2-search-string-base64
branch
from
August 9, 2026 06:04
e5ffba2 to
30eebbe
Compare
…Arn to the SDK ARN WAFv2WebACLProvider forwarded the CFn Rules blob raw. The AWS SDK v3 serializer drops unknown members, and the Rules tree carries exactly two spellings CFn uses that the SDK model does not (all 154 CFn keys were diffed against the SDK member set, so this is the complete list): - ByteMatchStatement.SearchStringBase64 exists ONLY in CloudFormation; the SDK carries a single SearchString blob member. The key was dropped and CreateWebACL failed validation on the missing required SearchString. - IPSetReferenceStatement / RegexPatternSetReferenceStatement / RuleGroupReferenceStatement spell the reference ARN `Arn` in CFn and `ARN` in the SDK, where it is also required. Same drop, same loud create failure — so any WebACL using a reference statement was broken regardless of base64. Both leaves are nestable, so one recursive walk handles them, covering every member the SDK Statement union declares as nested: NotStatement.Statement, And/OrStatement.Statements[], and the ScopeDownStatement of both RateBasedStatement and ManagedRuleGroupStatement. The walk rebuilds every level, so the caller's properties object is never mutated. Plain SearchString values are left untouched: the serializer accepts a string at a blob member and encodes it, which is the existing working behavior. A truthy non-array Rules value (an unresolved intrinsic) is now passed through rather than defaulted to []. Defaulting it would make update() a silent UpdateWebACL that wipes every rule and reports success, where the previous code handed the value to the SDK and failed loudly. PreParseTextTransformations, Monetize and PriceMultiplier have no member in the installed SDK model, so no mapping exists to write -- the fix is an SDK bump, after which the spellings already match. They cannot go in unhandledByDesign either, which is top-level property granularity, and all three nest inside the genuinely-handled Rules. Their drop is made loud with a warning naming them on create and update instead of staying silent. The wafv2 integ fixture now exercises all three divergence sites -- a base64 ByteMatchStatement under the rate-based rule's ScopeDownStatement, one under a NotStatement, and an IPSetReferenceStatement under an AndStatement -- so the deploy itself is the regression signal: pre-fix CreateWebACL rejects the stack. Closes #1389
go-to-k
force-pushed
the
fix/1389-wafv2-search-string-base64
branch
from
August 9, 2026 06:53
30eebbe to
6c24643
Compare
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 9, 2026
## [0.278.7](v0.278.6...v0.278.7) (2026-08-09) ### Bug Fixes * **wafv2:** decode SearchStringBase64 and rename reference-statement Arn to the SDK ARN ([#1405](#1405)) ([615c33b](615c33b))
|
🎉 This PR is included in version 0.278.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
WAFv2WebACLProviderforwarded the CFnRulesblob raw. The AWS SDK v3 serializer drops unknown members, and theRulestree carries exactly two spellings CFn uses that the SDK model does not — all 154 CFn keys in theCfnWebACLtree were diffed against the SDK schema member set, so this is the complete list:ByteMatchStatement.SearchStringBase64SearchStringblob)CreateWebACLfails on the missing requiredSearchStringArnonIPSetReferenceStatement/RegexPatternSetReferenceStatement/RuleGroupReferenceStatementARN(required)CreateWebACLfails on the missing requiredARNThe second row was not in the filed issue — a reviewer found it by diffing the whole tree, and it is the wider breakage: any WebACL using an IPSet / RegexPatternSet / RuleGroup reference statement failed today, base64 or not. Both are the same class and both leaves are nestable, so one walk fixes them together.
What changed
Statementunion declares as nested:NotStatement.Statement,AndStatement.Statements[],OrStatement.Statements[], and theScopeDownStatementof bothRateBasedStatementandManagedRuleGroupStatement.RuleGroupReferenceStatementis deliberately not a recursion point (it declares no nested statement member); it appears only in the ARN rename list. Applied on create and update; every level is rebuilt, so the caller'spropertiesobject is never mutated (pinned by a test).SearchStringBase64decodes to aUint8Array, matching the declared member type. The issue flagged a latent string-vs-Uint8Arrayrisk; it is resolved from the resolved serializer source rather than assumed — the blob member is schema code 21, and@aws-sdk/core's JSON body serializer base64-encodes both aUint8Arrayand a plainstringat a blob member. PlainSearchStringvalues are therefore correct as-is and left untouched.SearchStringBase64wins. CFn treats them as mutually exclusive and rejects such a template, so any choice is arbitrary; the explicit-encoding form is the only one that can express non-UTF-8 bytes, so honoring it never loses information.Rulesnow passes through instead of defaulting to[]. Defaulting a truthy non-array (an unresolved intrinsic) would makeupdate()a silentUpdateWebACLthat wipes every rule and reports success; the previous code handed the value to the SDK and failed loudly, and that is preserved. AbsentRulesstill defaults to[].PreParseTextTransformations/Monetize/PriceMultiplierhave no member in the installed SDK, so there is no mapping to write — the fix is an SDK bump, after which the spellings already match. They cannot go inunhandledByDesign, which is top-level property granularity, and all three nest inside the genuinely-handledRules. Their drop is made loud with a warning naming them on create and update instead of staying silent.Test plan
Ruleshandling 1, and no-op'ing the warn helper 2. The remaining tests are deliberate construction-green guards (plainSearchStringuntouched, absent / falsyRules, no mutation of the caller's object, no spurious warning, an already-ARN-spelled value left alone)./run-integ wafv2, us-east-1). The fixture now exercises all three divergence sites — a base64ByteMatchStatementunder the rate-based rule'sScopeDownStatement, one under aNotStatement, and anIPSetReferenceStatementunder anAndStatement— so the deploy itself is the regression signal: pre-fixCreateWebACLrejects the stack. Deploy 9/9 created;GetWebACLround-tripped all three (Y2RrZC1zY29wZWQ=,Y2RrZC1ibG9ja2Vk, and the IPSet ARN underARN); destroy 7 deleted / 2DeletionPolicy: Retain/ 0 errors; account swept clean.Known gaps (reviewer-surfaced, deliberately not widened into this PR)
readCurrentStatehas no reverse mapping, socdkd driftreports permanent phantomRulesdrift — AWS returnsARNand a decodedUint8ArraySearchString, while state holdsArn/SearchStringBase64. Tracked in (AWS::WAFv2::WebACL: phantom drift on every ByteMatchStatement — readCurrentState returns the SDK Uint8Array SearchString unmapped #1403), which this work widened: the reverse map must be BASELINE-driven (only the template knows whether it usedSearchStringorSearchStringBase64), and it now also has to un-renameARN. Not folded in here because the cheaper alternative — agetDriftUnknownPathsentry forRules— trades a false positive for never reporting real rule drift, and the repo argues that trade deserves an explicit decision.Buffer-vs-Uint8Arraymutation turns 7 red), and the exact bytes were confirmed by hand againstGetWebACLon every verification run. Closing it mechanically means converting this standard-flow fixture to averify.sh.RateBasedStatement.ScopeDownStatement; the other three recursion points are covered for the base64 conversion only. Both conversions ride the sametoSdkStatementframe, so they cannot regress independently.Follow-ups
ByteMatchStatement(AWS::WAFv2::WebACL: phantom drift on every ByteMatchStatement — readCurrentState returns the SDK Uint8Array SearchString unmapped #1403):readCurrentStatereturns the AWSRulesverbatim, whereGetWebACLdeserializesSearchStringback into aUint8Arraywhile state holds the template shape. It already fires for the plainSearchStringform; this PR makes the base64 form newly deployable and therefore newly able to hit it. A genuinely separate concern, so it is not widened into this diff.wafv2fixture is not re-runnable (test(integ): thewafv2fixture is not re-runnable — its RETAIN ApiGateway CloudWatch role collides on the next deploy #1407): itsapigateway.RestApiCloudWatch role isDeletionPolicy: Retainunder a fixed name and collides with the next CREATE. Hit live during this verification and cleaned up by hand; pre-existing, not caused by this PR.handledPropertiescan lie (audit:handledPropertiescan lie — a property declared handled but named on NO API call passes the property-coverage pre-flight #1404) — proposed critic, from the sibling ECR work in the same session.AWS::WAFv2::WebACLis not inNESTED_KEY_TARGETS, so the nested-key critic would not have caught either divergence. That target expansion is tracked in (audit: nested-key critic blind spots found by the 0809 sweep — mixed-case SDK models, file-global literal heuristic, missing targets, selective sub-key forwards #1393), and theArncase is a good argument for it — a mechanical critic would have found it without a reviewer.Closes #1389