fix: validate path for getHomeDirectory

[ivikash]

Problem

• Users with misconfigured env variables -HOME or USERPROFILE or HOMEPATH AWS_SHARED_CREDENTIALS_FILE or AWS_CONFIG_FILE are unable to login into Amazon Q and AWS Toolkits extensions.

Solution

• Added a isValidPath function to ensure a valid path is used for authentication

TODO

• Update fs to fsCommon because the web tests are failing. This entails updating all functions to use Promises
• Updating createVueEntries function to allow developers to build on *NIX and Windows.
• Add Logging

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[ivikash]

/runIntegrationTests

[ivikash]

/runIntegrationtests

[ivikash]

/runIntegrationtests

[ivikash]

/retryBuilds

[justinmk3]

Users on windows are unable to login to AWS Toolkits and Amazon Q extension on Windows because env variable: HOME is set incorrectly

It's not really a Windows specific issue. Any system with a misconfigured $HOME env var could have this issue.

[ivikash]

/runIntegrationTests

[justinmk3]

this would never be hit in web-mode because getHomeDirectory returns early for web-mode

[ivikash]

vscode only provide Thenable aka Promise based api. To allow this in web, it will require updating all touch points with a Promise based API.

https://github.com/microsoft/vscode/blob/main/src/vscode-dts/vscode.d.ts#L9045

[justinmk3]

why are we using resolve() here but not in the result returned by getHomeDirectory(). that means this is checking something different than would be returned. it probably is a signal that getHomeDirectory() should return a `resolve() result.

more generally, please think about how code can make decisions once, which are then used for the rest of the codepath, instead of making many decisions in multiple different places.

[ivikash]

Would you recommend something like this?

packages/core/src/auth/credentials/sharedCredentialsFile.ts

-    if (env.AWS_SHARED_CREDENTIALS_FILE && isValidPath(env.AWS_SHARED_CREDENTIALS_FILE)) {
-        return env.AWS_SHARED_CREDENTIALS_FILE
+    if (env.AWS_SHARED_CREDENTIALS_FILE && isValidPath(resolve(env.AWS_SHARED_CREDENTIALS_FILE))) {
+        return resolve(env.AWS_SHARED_CREDENTIALS_FILE)
     }

-    if (env.AWS_CONFIG_FILE && isValidPath(env.AWS_CONFIG_FILE)) {
-        return env.AWS_CONFIG_FILE
+    if (env.AWS_CONFIG_FILE && isValidPath(resolve(env.AWS_CONFIG_FILE))) {
+        return resolve(env.AWS_CONFIG_FILE)
     }

packages/core/src/shared/systemUtilities.ts

-        if (env.HOME && isValidPath(env.HOME)) {
-            return env.HOME
+        if (env.HOME && isValidPath(path.resolve(env.HOME))) {
+            return path.resolve(env.HOME)
         }
-        if (env.USERPROFILE && isValidPath(env.USERPROFILE)) {
-            return env.USERPROFILE
+        if (env.USERPROFILE && isValidPath(path.resolve(env.USERPROFILE))) {
+            return path.resolve(env.USERPROFILE)
         }
-        if (env.HOMEPATH && isValidPath(env.HOMEPATH)) {
+        if (env.HOMEPATH && isValidPath(path.resolve(env.HOMEPATH))) {
             const homeDrive: string = env.HOMEDRIVE || 'C:'
 
-            return path.join(homeDrive, env.HOMEPATH)
+            return path.join(homeDrive, path.resolve(env.HOMEPATH))
         }

[ivikash]

Or skipping the resolve like

-    if (path && path.length !== 0 && fs.existsSync(_path.resolve(path))) {
+    if (path && path.length !== 0 && fs.existsSync(path)) {

I'd prefer the former where we check isValidPath(path.resolve(env.HOME)) and return path.resolve(env.HOME)

[justinmk3]

stubbing isValidPath in various tests that happen to current hit the getHomeDirectory() codepath, means that future tests might run into the same issue.

[ivikash]

Given that in unit tests we are validating an actual path, indeed it will require stubbing in future as well. What would be your recommendation?