Skip to content

unmark object ID or Name for formatting#29397

Merged
jbardin merged 1 commit intomainfrom
jbardin/format-id-name-marks
Aug 17, 2021
Merged

unmark object ID or Name for formatting#29397
jbardin merged 1 commit intomainfrom
jbardin/format-id-name-marks

Conversation

@jbardin
Copy link
Copy Markdown
Member

@jbardin jbardin commented Aug 17, 2021

It's possible that an "identifying" attribute could have marks. Make sure we don't return sensitive values as identifying attributes, and unmark the entire value when converting to a string.

Fixes #29374

@jbardin jbardin requested a review from a team August 17, 2021 14:06
@jbardin jbardin force-pushed the jbardin/format-id-name-marks branch from 0c249e6 to 8c68c7d Compare August 17, 2021 14:06
@jbardin jbardin added the 1.0-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Aug 17, 2021
@alisdair
Copy link
Copy Markdown
Contributor

I'm not sure about this solution. Since the identifying attributes heuristic is out of the user's control, displaying a name/ID marked as sensitive seems dangerous to me.

What if instead of this, we check if the identifying attribute value is sensitive, and ignore it if so?

@jbardin
Copy link
Copy Markdown
Member Author

jbardin commented Aug 17, 2021

The first check here is obj.HasMark(marks.Sensitive) to ignore the value, but I left the unmark call in order to remove any possible other marks in the future.

@alisdair
Copy link
Copy Markdown
Contributor

Unless I'm misunderstanding, that will check if the containing object itself has the sensitive mark, but not the enclosed value (i.e. the name/ID string).

@jbardin
Copy link
Copy Markdown
Member Author

jbardin commented Aug 17, 2021

Oh, you are correct! I had some refactoring I discarded and that check no longer makes sense. I'll drop sensitive values individually.

@jbardin jbardin force-pushed the jbardin/format-id-name-marks branch from 8c68c7d to 8c08c30 Compare August 17, 2021 16:13
@jbardin jbardin force-pushed the jbardin/format-id-name-marks branch from 8c08c30 to a48b024 Compare August 17, 2021 16:24
@jbardin jbardin merged commit a94155d into main Aug 17, 2021
@jbardin jbardin deleted the jbardin/format-id-name-marks branch August 17, 2021 18:59
@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 Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

1.0-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.

Terraform crashed

2 participants