Skip to content

validation: don't strip marks from variables during validation#37595

Merged
liamcervante merged 2 commits intomainfrom
liamcervante/validate/variable-conditions
Sep 12, 2025
Merged

validation: don't strip marks from variables during validation#37595
liamcervante merged 2 commits intomainfrom
liamcervante/validate/variable-conditions

Conversation

@liamcervante
Copy link
Copy Markdown
Contributor

There is custom logic for variable validations, explained in the surrounding comment. This was actually also stripping away sensitive and ephemeral metadata (or more accurately, just not adding it). This metadata is added by the usual approach of generating the HCL context. This meant errors during variable validation would expose sensitive values.

This PR updates the custom logic so that it only generates the value itself during the validate walk (which is the only time the custom logic is actually needed). For the validate walk, all variables are unknown anyway so the metadata doesn't matter.

Now, during other walks (eg. plan and apply) the real data is used, that does have the sensitive metadata attached. This means the sensitive metadata is no longer being exposed.

Target Release

1.13.2

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@liamcervante liamcervante added the 1.13-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Sep 11, 2025
@liamcervante liamcervante requested a review from a team as a code owner September 11, 2025 12:24
mildwonkey
mildwonkey previously approved these changes Sep 11, 2025
Copy link
Copy Markdown
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

LGTM, I tested this out locally and confirmed. Can you add a regression test for this please? A followup PR is fine no need to block the fix.

@liamcervante
Copy link
Copy Markdown
Contributor Author

A followup PR is fine no need to block the fix.

I just added it in this one, no rush to merge this as we released yesterday anyway.

Copy link
Copy Markdown
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

thank you!

@liamcervante liamcervante merged commit 20574d4 into main Sep 12, 2025
7 checks passed
@liamcervante liamcervante deleted the liamcervante/validate/variable-conditions branch September 12, 2025 14:38
@hashicorp hashicorp deleted a comment Sep 26, 2025
@github-actions
Copy link
Copy Markdown
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

1.13-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants