Skip to content

Harden CloudFormation variable and Ref resolvers#248

Merged
GrahamCampbell merged 2 commits intomainfrom
backport-a-cf-resolver-hardening
May 3, 2026
Merged

Harden CloudFormation variable and Ref resolvers#248
GrahamCampbell merged 2 commits intomainfrom
backport-a-cf-resolver-hardening

Conversation

@GrahamCampbell
Copy link
Copy Markdown
Contributor

Summary

  • Treat omitted CloudFormation Outputs, Exports, and StackResourceSummaries as empty arrays.
  • Fix the user-facing Ref resolution error message grammar.
  • Add focused resolver coverage for optional response arrays and pagination.

Tests

  • npx mocha --config "test/mocha/unit.cjs" "test/unit/lib/configuration/variables/sources/instance-dependent/get-cf.test.js" "test/unit/lib/plugins/aws/utils/resolve-cf-import-value.test.js" "test/unit/lib/plugins/aws/utils/resolve-cf-ref-value.test.js"
  • npm run lint
  • npm run prettier-check
  • git diff --check

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens CloudFormation-backed variable and intrinsic resolvers against optional/missing response arrays, improving robustness and error clarity in AWS pagination scenarios.

Changes:

  • Treat missing StackResourceSummaries, Exports, and Outputs fields as empty arrays to avoid runtime crashes.
  • Improve the user-facing Ref resolution error message grammar.
  • Add unit coverage for pagination where intermediate pages omit optional arrays, plus coverage for stacks with no outputs.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/plugins/aws/utils/resolve-cf-ref-value.js Avoids crashing when StackResourceSummaries is omitted; updates Ref error message.
lib/plugins/aws/utils/resolve-cf-import-value.js Avoids crashing when Exports is omitted during listExports pagination.
lib/configuration/variables/sources/instance-dependent/get-cf.js Treats missing Outputs as an empty list when resolving ${cf:...} variables.
test/unit/lib/plugins/aws/utils/resolve-cf-ref-value.test.js Adds pagination coverage for pages that omit StackResourceSummaries and verifies updated error message.
test/unit/lib/plugins/aws/utils/resolve-cf-import-value.test.js Adds pagination coverage for pages that omit Exports.
test/unit/lib/configuration/variables/sources/instance-dependent/get-cf.test.js Adds coverage for stacks that return no Outputs.
Comments suppressed due to low confidence (1)

lib/plugins/aws/utils/resolve-cf-import-value.js:11

  • When continuing pagination, the recursive call replaces sdkParams with { NextToken: ... }, which drops any other params the caller may have provided. Consider preserving existing params when paginating (e.g., merge NextToken into the current params) so pagination remains correct if additional request params are ever introduced/used.
    const targetExportMeta = (result.Exports || []).find((exportMeta) => exportMeta.Name === name);
    if (targetExportMeta) return targetExportMeta.Value;
    if (result.NextToken) {
      return resolveCfImportValue(provider, name, { NextToken: result.NextToken });
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/plugins/aws/utils/resolve-cf-ref-value.js Outdated
Comment thread lib/plugins/aws/utils/resolve-cf-ref-value.js Outdated
@GrahamCampbell GrahamCampbell merged commit 2a69031 into main May 3, 2026
4 checks passed
@GrahamCampbell GrahamCampbell deleted the backport-a-cf-resolver-hardening branch May 3, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants