Conversation
- Add --kiro-aws-login flag for AWS Builder ID device code flow - Add DoKiroAWSLogin function for AWS SSO OIDC authentication - Complete Kiro integration with AWS, Google OAuth, and social auth - Add kiro executor, translator, and SDK components - Update browser support for Kiro authentication flows
- Added setKiroIncognitoMode() helper function to handle Kiro auth incognito mode setting - Replaced 3 duplicate code blocks (21 lines) with single function calls (3 lines) - Kiro auth defaults to incognito mode for multi-account support - Users can override with --incognito or --no-incognito flags This addresses the code duplication noted in PR #1 review.
- Remove custom stringContains and findSubstring helper functions - Use standard library strings.Contains for better maintainability - No functional change, just cleaner code Addresses Gemini Code Assist review feedback
Summary of ChangesHello @luispater, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new feature: full integration with Kiro (AWS CodeWhisperer) and Amazon Q. It provides multiple authentication methods, including AWS Builder ID and social logins, along with the ability to import existing Kiro IDE tokens. A custom protocol handler ensures a smooth OAuth experience, and an incognito browser mode supports multi-account usage. The changes also include robust token refresh, intelligent quota fallback, and comprehensive tool calling support, all while enhancing the overall stability and configurability of the system. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive integration with Kiro (AWS CodeWhisperer), including multiple authentication methods (Google OAuth, AWS Builder ID, and token import), API execution logic with token refresh and quota fallbacks, and a custom protocol handler for an improved user experience. The changes are extensive and well-implemented, also including important concurrency and security fixes. My review focuses on opportunities for code simplification, refactoring to reduce duplication, addressing a security concern with a hardcoded secret, and correcting a test case.
| func getIFlowClientSecret() string { | ||
| if secret := os.Getenv("IFLOW_CLIENT_SECRET"); secret != "" { | ||
| return secret | ||
| } | ||
| return defaultIFlowClientSecret | ||
| } |
There was a problem hiding this comment.
The getIFlowClientSecret function falls back to a hardcoded default client secret. Hardcoding secrets is a security risk. It's recommended to remove the default value and require the IFLOW_CLIENT_SECRET environment variable to be set, or at least log a prominent warning when the default is used to discourage its use in production environments.
| func getIFlowClientSecret() string { | |
| if secret := os.Getenv("IFLOW_CLIENT_SECRET"); secret != "" { | |
| return secret | |
| } | |
| return defaultIFlowClientSecret | |
| } | |
| func getIFlowClientSecret() string { | |
| if secret := os.Getenv("IFLOW_CLIENT_SECRET"); secret != "" { | |
| return secret | |
| } | |
| log.Warn("IFLOW_CLIENT_SECRET environment variable is not set. Using the default client secret is insecure and should be avoided in production.") | |
| return defaultIFlowClientSecret | |
| } |
| } else if kiroLogin { | ||
| // For Kiro auth, default to incognito mode for multi-account support | ||
| // Users can explicitly override with --no-incognito | ||
| // Note: This config mutation is safe - auth commands exit after completion | ||
| // and don't share config with StartService (which is in the else branch) | ||
| setKiroIncognitoMode(cfg, useIncognito, noIncognito) | ||
| cmd.DoKiroLogin(cfg, options) | ||
| } else if kiroGoogleLogin { | ||
| // For Kiro auth, default to incognito mode for multi-account support | ||
| // Users can explicitly override with --no-incognito | ||
| // Note: This config mutation is safe - auth commands exit after completion | ||
| setKiroIncognitoMode(cfg, useIncognito, noIncognito) | ||
| cmd.DoKiroGoogleLogin(cfg, options) | ||
| } else if kiroAWSLogin { | ||
| // For Kiro auth, default to incognito mode for multi-account support | ||
| // Users can explicitly override with --no-incognito | ||
| setKiroIncognitoMode(cfg, useIncognito, noIncognito) | ||
| cmd.DoKiroAWSLogin(cfg, options) |
There was a problem hiding this comment.
The if/else if chain for handling different Kiro login flags (kiroLogin, kiroGoogleLogin, kiroAWSLogin) contains duplicated code. The comments and the call to setKiroIncognitoMode are repeated in each block. This can be refactored to reduce redundancy and improve maintainability by grouping the common logic.
} else if kiroLogin || kiroGoogleLogin || kiroAWSLogin {
// For Kiro auth, default to incognito mode for multi-account support
// Users can explicitly override with --no-incognito
// Note: This config mutation is safe - auth commands exit after completion
// and don't share config with StartService (which is in the else branch)
setKiroIncognitoMode(cfg, useIncognito, noIncognito)
if kiroLogin {
cmd.DoKiroLogin(cfg, options)
} else if kiroGoogleLogin {
cmd.DoKiroGoogleLogin(cfg, options)
} else if kiroAWSLogin {
cmd.DoKiroAWSLogin(cfg, options)
}
} else if kiroImport {
cmd.DoKiroImport(cfg, options)
} else {
// In cloud deploy mode without config file, just wait for shutdown signals
if isCloudDeploy && !configFileExists {| { | ||
| name: "Path traversal attempt", | ||
| email: "../../../etc/passwd", | ||
| expected: "_.__.__._etc_passwd", | ||
| }, |
There was a problem hiding this comment.
The expected value in this test case for path traversal seems incorrect. The SanitizeEmailForFilename function with the input "../../../etc/passwd" will produce "_.._.._.._etc_passwd", but the test expects "_.__.__._etc_passwd". This indicates a discrepancy between the test's expectation and the implementation's actual behavior. The test should be updated to reflect the correct output of the function to ensure its behavior is tested accurately.
| { | |
| name: "Path traversal attempt", | |
| email: "../../../etc/passwd", | |
| expected: "_.__.__._etc_passwd", | |
| }, | |
| { | |
| name: "Path traversal attempt", | |
| email: "../../../etc/passwd", | |
| expected: "_.._.._.._etc_passwd", | |
| }, |
…gresql-deployment feat(kiro): add Kiro (AWS CodeWhisperer) provider integration
1 participant
No description provided.