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

Windows: credential_process cannot handle paths denoted with backslashes #2273

Closed
jordankay13 opened this issue Sep 8, 2023 · 5 comments · Fixed by #2461
Closed

Windows: credential_process cannot handle paths denoted with backslashes #2273

jordankay13 opened this issue Sep 8, 2023 · 5 comments · Fixed by #2461
Labels
bug This issue is a bug. p3 This is a minor priority issue

Comments

@jordankay13
Copy link

Describe the bug

On Windows, when using credential_process, the executable cannot be contained in a folder that starts with t or n when using backslashes in the path.

For example:

[default]
credential_process = "C:\somefolder\tools\myexe.exe"

This will fail with the error:

'C:\somefolder' is not recognized as an internal or external command, operable program or batch file.

Expected Behavior

I expected my executable in the tools folder to be executed to fetch my AWS credentials:

[default]
credential_process = "C:\somefolder\tools\myexe.exe"

Current Behavior

Folders beginning with t or n are treated as tabs or newlines when using backslashes.

In fact, the example path in these docs for Windows would hit this bug:
https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html
because "C:\Path\To\credentials.cmd" would be corrupted at the To folder.

Reproduction Steps

Setup your credentials file similar to:

[default]
credential_process = "C:\somefolder\tools\myexe.exe"

I don't have a program that I've written for this -- I'm just using an OpenTelemetry Collector, and it's written in Go:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/awskinesisexporter/exporter.go#L60

The premise is that the call to config.LoadDefaultConfig with the above credentials file results in the error.

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2/config v1.18.38

Compiler and Version used

1.20.8

Operating System and version

Windows

@jordankay13 jordankay13 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 8, 2023
@lucix-aws
Copy link
Contributor

@jordankay13 -- Do escaped backslashes exhibit the same issue?

e.g.

[default]
credential_process = "C:\\somefolder\\tools\\myexe.exe"

@lucix-aws lucix-aws added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 11, 2023
@jordankay13
Copy link
Author

jordankay13 commented Sep 11, 2023

@jordankay13 -- Do escaped backslashes exhibit the same issue?

e.g.

[default]
credential_process = "C:\\somefolder\\tools\\myexe.exe"

It looks like escaped backslashes also fails -- Using forward slashes works though (C:/somefolder/tools/myexe.exe)

@lucix-aws lucix-aws added needs-review This issue or pull request needs review from a core team member. workaround-available and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Sep 11, 2023
@RanVaknin RanVaknin added the p2 This is a standard priority issue label Sep 13, 2023
@lucix-aws lucix-aws added p3 This is a minor priority issue and removed needs-review This issue or pull request needs review from a core team member. p2 This is a standard priority issue labels Oct 11, 2023
@lucix-aws
Copy link
Contributor

I have some more context into this after recently re-writing the entire ini parser in #2365.

This manual escaping of \* sequences is something we've always done, it's not standardized behavior whatsoever.

As it so happens, in the aforementioned refactor I unintentionally removed part of that behavior. As of the current version we now only escape \n, \", and \'.

Normally we're opposed to making changes like this - https://www.hyrumslaw.com/ - but I'm reconsidering here given that there was no apparent fallout from the partial removal.

@lucix-aws
Copy link
Contributor

Will be resolved by #2461

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p3 This is a minor priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants