Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove AWS credential provider settings on bmx write #461

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Conversation

cfbao
Copy link
Member

@cfbao cfbao commented Jul 19, 2024

Why

An AWS profile can only have one credential provider configured.
Not removing other credential providers' settings when bmx write runs could lead to a broken or at least confusing AWS profile.

Ticket

https://desire2learn.atlassian.net/browse/VUL-402

Comment on lines -138 to +137
return File.Exists( path ) ? parser.ReadFile( path ) : null;
return File.Exists( path ) ? parser.ReadFile( path, Utf8 ) : null;
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated - the default encoding is ASCII - what if I want to write Chinese comments in my AWS config file

Copy link
Member

Choose a reason for hiding this comment

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

Huh, I figured C# is for the most part UTF8 by default in places? Or is it just the FileSystem API that is like this?

> System.Text.Encoding.Default
UTF8Encoding.UTF8EncodingSealed

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

It'd be disastrous if C#/.NET runtime did this.

Copy link
Member

Choose a reason for hiding this comment

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

👍 I suppose that makes sense for them. INI doesn't have a spec around it and it could be used by old timey things that except files to be ASCII?

Copy link
Member Author

@cfbao cfbao Jul 19, 2024

Choose a reason for hiding this comment

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

I don't think it makes sense at all actually. UTF-8 is ASCII compatible.

@cfbao cfbao marked this pull request as ready for review July 19, 2024 13:43
@cfbao cfbao requested a review from a team as a code owner July 19, 2024 13:43
@cfbao cfbao merged commit cae7c4a into main Jul 19, 2024
9 checks passed
@cfbao cfbao deleted the rm-cred-settings branch July 19, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants