Skip to content

fix(stepfunctions-tasks): mapFromJsonPath produces invalid M.$.$ key for tokenized JsonPath#37965

Open
Khangdang1690 wants to merge 1 commit into
aws:mainfrom
Khangdang1690:fix/21149-dynamodb-map-from-json-path
Open

fix(stepfunctions-tasks): mapFromJsonPath produces invalid M.$.$ key for tokenized JsonPath#37965
Khangdang1690 wants to merge 1 commit into
aws:mainfrom
Khangdang1690:fix/21149-dynamodb-map-from-json-path

Conversation

@Khangdang1690
Copy link
Copy Markdown

Issue # (if applicable)

Closes #21149.

Reason for this change

DynamoAttributeValue.mapFromJsonPath(JsonPath.stringAt('$.Item.M')) synthesizes the invalid CloudFormation key "M.$.$": "$.Item.M" instead of "M.$": "$.Item.M". Step Functions rejects the resulting state machine at deploy time. The bug is at packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts:225 — the factory hardcodes the key as 'M.$', but the Step Functions JsonPath renderer at json-path.ts:218-225 auto-appends .$ to any key whose value is a JsonPath token, producing the double suffix.

Sibling factories fromString ("S.$") and listFromJsonPath ("L.$") work correctly because they store the bare type letter ({ S: value }, { L: value }) and let the renderer add .$ exactly once.

The issue was filed in 2022 and has remained reproducible through 2025 per follow-up comments. @kaizencc identified the bug location in 2022 ("we are adding at least one $... I bet we don't want to always add $") but the fix never landed.

Description of changes

  • One-line fix in aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts:225 — change new DynamoAttributeValue({ 'M.$': value }) to new DynamoAttributeValue({ M: value }). This matches listFromJsonPath exactly.

  • Unit test rewrite in aws-stepfunctions-tasks/test/dynamodb/shared-types.test.ts (the 'map from json path' case). The pre-existing test passed a literal '$.path' string rather than wrapping in sfn.JsonPath.stringAt('$.path'). jsonPathString() only detects CDK JsonPathToken instances, so the literal-string call bypassed the buggy code path entirely — letting the test pass while real users (who pass JsonPath.stringAt(...)) hit the bug. The updated test now wraps the input via sfn.JsonPath.stringAt(m), mirroring the correct pattern already used by the 'from list with json path' test, and asserts the expected 'M.$': m rendering. Pre-fix, this assertion sees 'M.$.$': m and fails — so the test is a real regression guard.

Backwards compatibility. Per AGENTS.md on validation-that-breaks-existing-input: "Input that was accepted by synth but always failed at deploy time → feature flag NOT required; fail-fast synth-time validation is preferred." The buggy synth output M.$.$ is rejected by Step Functions at deploy time, so any app currently calling mapFromJsonPath(JsonPath.stringAt(...)) is already broken — the fix only changes synth for code paths that never deploy.

Edge case worth flagging for reviewers: mapFromJsonPath called with a literal '$.path' (no JsonPath.stringAt wrap) previously emitted "M.$": "$.path", which Step Functions could resolve as a path lookup. After this fix it emits "M": "$.path" (a literal string). listFromJsonPath already exhibits the same behavior — bare { L: value } only renders to "L.$" if the value is a JsonPathToken. The JSDoc @param value Json path that specifies state input to be used implies a token-encoded JsonPath, so this aligns the API with both its sibling and its documented contract. Calling out explicitly so a maintainer can weigh in if a feature flag is preferred.

Alternative considered and rejected: refactoring renderString to detect already-suffixed keys (endsWith('.$')) and avoid double-suffixing. Rejected as too broad — it would touch every *FromJsonPath factory and the central renderer, when the bug is local to one factory.

Describe any new or updated permissions being added

None.

Description of how you validated changes

  • Unit tests pass: npx jest --testPathPattern aws-stepfunctions-tasks/test/dynamodb/shared-types → 25/25 pass, including the updated 'map from json path' case.
  • Regression sweep: npx jest --testPathPattern aws-stepfunctions-tasks/test/dynamodb → 34/34 pass across delete-item, get-item, put-item, update-item, shared-types.
  • Regression sweep on the renderer it depends on: npx jest --testPathPattern aws-stepfunctions/test/fields.test → 36/36 pass.
  • No integ-test change — there is no integ test under packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/ that currently exercises mapFromJsonPath. If the PR linter requires an integ snapshot change, requesting exemption: this fixes a synth-time bug whose output is rejected by Step Functions at deploy time, and there is no existing integ test exercising the affected factory.

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…for tokenized JsonPath

DynamoAttributeValue.mapFromJsonPath hardcoded the key as 'M.$', but the
Step Functions JsonPath renderer auto-appends '.$' to any key whose value
is a JsonPath token. The result was the invalid key 'M.$.$', which is
rejected by Step Functions at deploy time. Sibling factories
(listFromJsonPath, fromString) store the bare type letter and let the
renderer add the '.$' exactly once.

This change drops the hardcoded suffix so mapFromJsonPath behaves
consistently with listFromJsonPath. The existing unit test was bypassing
the bug by passing a literal '$.path' string instead of wrapping in
JsonPath.stringAt; it now wraps the input the same way the listFromJsonPath
test does.

fixes aws#21149
@github-actions github-actions Bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. effort/small Small work item – less than a day of effort p2 labels May 21, 2026
Copy link
Copy Markdown
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

✅ A exemption request has been requested. Please wait for a maintainer's review.

@Khangdang1690
Copy link
Copy Markdown
Author

Exemption Request

This PR fixes a synth-time bug — DynamoAttributeValue.mapFromJsonPath(JsonPath.stringAt(...)) synthesizes the invalid CloudFormation key "M.$.$", which Step Functions rejects at deploy time. No existing integration test under packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/ calls mapFromJsonPath, so there is no integ snapshot to update.

Adding a new integ test for this fix would not provide meaningful additional coverage over the unit test rewrite at shared-types.test.ts:203. The fix is purely a synth-time key rendering correction in a factory method — the existing unit test now wraps the input in sfn.JsonPath.stringAt(...) (matching the established pattern used by the listFromJsonPath test at shared-types.test.ts:266) and asserts the synthesized key is "M.$" rather than "M.$.$". Pre-fix, that assertion sees "M.$.$" and fails, so the test is a genuine regression guard for the bug. A deploy-time integ test would assert nothing the unit test doesn't already assert.

Also note: the unrelated pr-triage-manager job failure is a pre-existing infrastructure issue ("Reviews may only be requested from collaborators..."). The same failure appears on my prior PR #37963 and is not caused by this change.

@aws-cdk-automation aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. effort/small Small work item – less than a day of effort p2 pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(stepfunctions-tasks): DynamoDB attribute map synth output

2 participants