fix(amazonq): fix iam credential update logic to use custom comparator and added buffer time in cred validation #8070

parameja1 · 2025-09-17T23:33:19Z

Problem

IAM credentials is not updating in Sagemaker instances due to incorrect comparison logic which prevents credential refresh and hence users cant interact with Q chat after the initial expiration time

Solution

Add custom comparator logic and method to properly compare credentials by their actual values (accessKeyId,
secretAccessKey, sessionToken) instead of string comparison
Added 60-second expiration buffer to credential validation. similar to SSO token logic here for grace-time
Tested by building a debug artefact on a SMUS CodeEditor instance and verified q chat is triggering refresh credentials
npm run package && npm run test succeeded
https://drive.corp.amazon.com/documents/parameja@/PR-8070/IAM-Credentials-Refresh-Q-Chat.mov

Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
Your code changes must meet the guidelines in CONTRIBUTING.md.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

…r and added buffer time in cred validation

amazon-inspector-ohio · 2025-09-17T23:33:23Z

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

github-actions · 2025-09-17T23:33:35Z

This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
- Note: beta or "experiment" features that have active users should announce fixes in the changelog.
- If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

amazon-inspector-ohio · 2025-09-17T23:33:54Z

✅ I finished the code review, and didn't find any security or code quality issues.

packages/amazonq/src/lsp/auth.ts

ashishrp-aws · 2025-09-18T03:47:17Z

/retryBuilds

sdharani91 · 2025-09-18T04:45:49Z

packages/amazonq/src/lsp/auth.ts

    }

-    public updateIamCredentials = onceChanged(this._updateIamCredentials.bind(this))
+    private areCredentialsEqual(creds1: any, creds2: any): boolean {


nit: move to util as well.

Ack, let me check if it can be moved to auth related util

fixed in new commit

sdharani91 · 2025-09-18T04:46:22Z

packages/amazonq/src/lsp/auth.ts


-    public updateIamCredentials = onceChanged(this._updateIamCredentials.bind(this))
+    private areCredentialsEqual(creds1: any, creds2: any): boolean {
+        if (!creds1 && !creds2) return true


Shouldn't this be false? We want this method to return true only if the creds are not empty and contain the same values right?

No, refactored logic however when credentials object is empty, we don't want updateIAM call to be invoked right?

the wrapper controls when to invoke update iam call alone and doesn't force creating a new credential object

Okay, makes sense

sdharani91 · 2025-09-18T04:47:14Z

packages/core/src/auth/auth.ts

        if (creds !== undefined && creds.credentialsHashCode === provider.getHashCode()) {
            return creds.credentials
        }
+        return undefined


Why do we need this change?

By default the method returns undefined, added explicit statement, can this be nit?

sdharani91 · 2025-09-18T04:47:25Z

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

 import { getLogger } from '../../shared/logger/logger'
 import { asString, CredentialsProvider, CredentialsId } from '../providers/credentials'
 import { CredentialsProviderManager } from '../providers/credentialsProviderManager'
+// import { get } from 'lodash'


addressed with new commit

sdharani91 · 2025-09-18T04:49:22Z

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

-            return expiration !== undefined ? expiration >= new globals.clock.Date() : true
+            const now = new globals.clock.Date()
+            const bufferedNow = new globals.clock.Date(now.getTime() + expirationBufferMs)
+            const isValid = expiration !== undefined ? expiration >= bufferedNow : true


Why are we adding 60 seconds? Not sure I follow the buffer logic. Can you explain this via inline code comments?

Adding buffer time as a defensive programming practice here to refresh/rotate credentials before they actually expire

SSO Token expiration logic also follows similar pattern

Already added inline comment

laileni-aws · 2025-09-18T18:21:55Z

/retryBuilds

amazon-inspector-ohio · 2025-09-18T18:22:30Z

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

amazon-inspector-ohio · 2025-09-18T18:23:02Z

✅ I finished the code review, and didn't find any security or code quality issues.

parameja1 added 2 commits September 17, 2025 16:23

fix(amazonq): fix iam credential update logic to use custom comparato…

cca4e38

…r and added buffer time in cred validation

fix(amazonq): remove unused import

b64cf2b

parameja1 requested a review from a team as a code owner September 17, 2025 23:33

parameja1 added 2 commits September 17, 2025 16:44

fix(amazonq): add change log

1cd2f61

fix linting

c634328

ashishrp-aws approved these changes Sep 18, 2025

View reviewed changes

packages/amazonq/src/lsp/auth.ts Outdated Show resolved Hide resolved

refactor redundant checks

7b4b8e4

ashishrp-aws approved these changes Sep 18, 2025

View reviewed changes

sdharani91 reviewed Sep 18, 2025

View reviewed changes

parameja1 added 2 commits September 18, 2025 08:52

fix(amazonq): move method to shared util

02d4848

add missing export for util

e49ec58

sdharani91 approved these changes Sep 18, 2025

View reviewed changes

laileni-aws approved these changes Sep 18, 2025

View reviewed changes

laileni-aws enabled auto-merge (squash) September 18, 2025 18:21

laileni-aws closed this Sep 18, 2025

auto-merge was automatically disabled September 18, 2025 18:22
Pull request was closed

laileni-aws reopened this Sep 18, 2025

ci: empty commit

9614843

rli enabled auto-merge (squash) September 18, 2025 19:39

rli merged commit 42e38c6 into aws:master Sep 18, 2025
30 of 31 checks passed

fix(amazonq): fix iam credential update logic to use custom comparator and added buffer time in cred validation #8070

fix(amazonq): fix iam credential update logic to use custom comparator and added buffer time in cred validation #8070

Uh oh!

Conversation

parameja1 commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Uh oh!

amazon-inspector-ohio bot commented Sep 17, 2025

Uh oh!

github-actions bot commented Sep 17, 2025

Uh oh!

amazon-inspector-ohio bot commented Sep 17, 2025

Uh oh!

Uh oh!

ashishrp-aws commented Sep 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parameja1 Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laileni-aws commented Sep 18, 2025

Uh oh!

amazon-inspector-ohio bot commented Sep 18, 2025

Uh oh!

amazon-inspector-ohio bot commented Sep 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

parameja1 commented Sep 17, 2025 •

edited

Loading

parameja1 Sep 18, 2025 •

edited

Loading