-
Notifications
You must be signed in to change notification settings - Fork 21
Fix/hws iam check ak info #90
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
Conversation
Reviewer's GuideExtends the HWS IAM collector to fetch each access key’s last-used details by refactoring credential retrieval and updates the user detail struct, and introduces an OPA policy that flags active access keys unused for over 365 days. Sequence diagram for fetching user and AK last-used detailssequenceDiagram
participant Collector
participant IamClient
participant UserDetail
Collector->>IamClient: ListPermanentAccessKeys(userId)
IamClient-->>Collector: Credentials list
loop For each credential
Collector->>IamClient: ShowPermanentAccessKey(AccessKey)
IamClient-->>Collector: ShowCredential (last_use_time, status)
end
Collector->>UserDetail: Populate Credentials and CredentialsDetail
Class diagram for updated UserDetail and credential retrievalclassDiagram
class UserDetail {
+KeystoneListUsersResult User
+ShowUserResult UserAttribute
+[]Credentials Credentials
+[]ShowCredential CredentialsDetail
+[]UserGroup UserGroups
+LoginProtectResult LoginProtects
+PasswordPolicyResult DomainPasswordPolicy
}
UserDetail --> "*" Credentials
UserDetail --> "*" ShowCredential
UserDetail --> "*" UserGroup
UserDetail --> LoginProtectResult
UserDetail --> PasswordPolicyResult
class IamClient
class model {
ListPermanentAccessKeysRequest
ShowPermanentAccessKeyRequest
Credentials
ShowCredential
}
IamClient --> model.ListPermanentAccessKeysRequest
IamClient --> model.ShowPermanentAccessKeyRequest
IamClient --> model.Credentials
IamClient --> model.ShowCredential
UserDetail "1" -- "*" Credentials
UserDetail "1" -- "*" ShowCredential
class Functions {
+GetUserDetail(ctx, service, res)
+listPermanentAccessKeys(ctx, cli, id)
+showPermanentAccessKey(ctx, cli, AccessKey)
}
Functions ..> IamClient
Functions ..> UserDetail
Functions ..> model.Credentials
Functions ..> model.ShowCredential
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @Lyc-heng, 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 focuses on bolstering the security posture related to Huawei Cloud IAM Access Keys. It achieves this by first enhancing the Go-based collector to gather comprehensive details, including the last usage timestamp for each Access Key. Subsequently, a new OPA rule leverages this enriched data to automatically detect and report any Access Keys that have remained inactive for more than a year, promoting proactive security measures against dormant credentials. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- Consider returning a slice of model.Credentials (instead of a *[]model.Credentials) to simplify the API and avoid unnecessary pointer indirection.
- Rename the local variable 'CredentialsDetail' to follow Go naming conventions (e.g., 'credentialsDetail') to avoid confusion with the exported struct field.
- In showPermanentAccessKey, skip appending nil entries on error so that failed credential fetches don’t propagate invalid values into the rule evaluation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider returning a slice of model.Credentials (instead of a *[]model.Credentials) to simplify the API and avoid unnecessary pointer indirection.
- Rename the local variable 'CredentialsDetail' to follow Go naming conventions (e.g., 'credentialsDetail') to avoid confusion with the exported struct field.
- In showPermanentAccessKey, skip appending nil entries on error so that failed credential fetches don’t propagate invalid values into the rule evaluation.
## Individual Comments
### Comment 1
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json:1` </location>
<code_context>
MvwxL6lnop8WR6WtsVZL
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 2
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json:1` </location>
<code_context>
dCGceTyJTRnTMekZ8Fpo
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 3
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json:1` </location>
<code_context>
ml64zq0ePvNjzGepQ3UZBaop8FoMSh2QkBWPOWggRFDxaG3pwy8NOtHug4XoUZohb6uCLSArHJdC9CqTJTZgrSBfwtSKNLF0dVhYFNugJWGJs7iAKhJCY6LBgFrG
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 4
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json:1` </location>
<code_context>
gV2kBLXcYCmavOfr0Z1g
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 5
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json:1` </location>
<code_context>
yG0nDtyNpaOcxP1v8ftA
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json
Show resolved
Hide resolved
rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json
Show resolved
Hide resolved
rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json
Show resolved
Hide resolved
rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json
Show resolved
Hide resolved
rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new feature to collect detailed access key information for Huawei Cloud IAM users and a corresponding OPA policy to identify access keys that have not been used for over a year. The changes in user.go correctly extend the data collection to include CredentialsDetail. However, there are a few high-severity issues related to efficiency in the Go collector and correctness in the OPA policy and its input data that need to be addressed.
| for _, credential := range *response.Credentials{ | ||
| credentialDetail := showPermanentAccessKey(ctx, cli, credential.Access) | ||
| credentialsDetail = append(credentialsDetail, credentialDetail) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The listPermanentAccessKeys function currently makes a separate API call to showPermanentAccessKey for each credential within the loop. This pattern can lead to an N+1 query problem, which might significantly impact performance, especially for users with a large number of access keys. Consider exploring if the Huawei Cloud IAM API offers a bulk retrieval method for access key details or if there's a more efficient way to gather this information.
rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511112036_1023138/input.json
Show resolved
Hide resolved
| } | ||
|
|
||
| user_name := input.UserAttribute.name | ||
| user_id := input.UserAttribute.domain_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user_id variable is assigned input.UserAttribute.domain_id. Typically, user_id refers to the unique identifier of the user, not the domain. It is likely that input.UserAttribute.id or input.User.id should be used here to correctly identify the user. Please verify the intended source for the user's ID and correct this assignment.
user_id := input.UserAttribute.id
j3ttt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Thank you for your contribution to CloudRec!
What About:
java)go)opa)Description:
hws Collector:Add AK details to check the last usage time of the AK.
hws rule:Add a new rule to check whether the AK last used time exceeds 365 days.
Summary by Sourcery
Add permanent access key details to Huawei Cloud collector and introduce a policy to flag keys unused for over one year
New Features:
Enhancements: