-
Notifications
You must be signed in to change notification settings - Fork 74
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
openhands issue resolver #343
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new GitHub Actions workflow file named Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/openhands-resolver.yml (2)
4-13
: Consider adding label filters for optimizationThe workflow currently triggers on any label being added. To prevent unnecessary workflow runs, consider filtering for specific labels that require OpenHands resolution.
issues: types: [labeled] + if: contains(github.event.label.name, 'openhands') || contains(github.event.label.name, 'ai-assist') pull_request: types: [labeled] + if: contains(github.event.label.name, 'openhands') || contains(github.event.label.name, 'ai-assist')
24-25
: Document the configuration variablesThe
OPENHANDS_MACRO
andOPENHANDS_MAX_ITER
variables should be documented to explain their purpose and acceptable values.Consider adding a comment block:
# Configuration: # OPENHANDS_MACRO: The trigger phrase for the OpenHands agent (default: @openhands-agent) # OPENHANDS_MAX_ITER: Maximum number of iterations for the resolution process (default: 50)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/openhands-resolver.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/openhands-resolver.yml (1)
15-18
: Verify required permissions scope
The workflow grants broad write permissions. Let's verify if all these permissions are actually required by the reusable workflow.
✅ Verification successful
Let me gather more information about how these permissions are used in the workflow.
Let me check the rest of the workflow file to see all the actions being performed.
All requested permissions are necessary for the workflow's functionality
Based on the workflow analysis, all the granted permissions are justified:
contents: write
: Required for checking out code and creating branches/commitspull-requests: write
: Needed for creating draft PRs and commenting on pull requestsissues: write
: Used for commenting on issues and pull requests with status updates
The permissions align with the principle of least privilege as they are actively used by the workflow for:
- Creating and managing pull requests
- Commenting on issues and PRs with start messages and results
- Committing changes and creating branches
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the permissions required by the reusable workflow
# Fetch and examine the reusable workflow file
curl -s https://raw.githubusercontent.com/All-Hands-AI/OpenHands/main/.github/workflows/openhands-resolver.yml | grep -A 10 "permissions:"
Length of output: 386
Script:
#!/bin/bash
# Check how these permissions are used in the workflow
curl -s https://raw.githubusercontent.com/All-Hands-AI/OpenHands/main/.github/workflows/openhands-resolver.yml | grep -A 50 "jobs:"
Length of output: 2234
Script:
#!/bin/bash
# Get the full workflow file to analyze all actions
curl -s https://raw.githubusercontent.com/All-Hands-AI/OpenHands/main/.github/workflows/openhands-resolver.yml | grep -A 200 "jobs:"
Length of output: 9045
PAT_TOKEN: ${{ secrets.PAT_TOKEN }} | ||
PAT_USERNAME: ${{ secrets.PAT_USERNAME }} | ||
LLM_MODEL: ${{ secrets.LLM_MODEL }} | ||
LLM_API_KEY: ${{ secrets.LLM_API_KEY }} | ||
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} |
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.
💡 Codebase verification
Security improvements needed for workflow secrets
Based on the verification results, several security concerns need to be addressed:
- The
openhands-resolver.yml
workflow has broad permissions (contents: write
,pull-requests: write
,issues: write
) but lacks environment protection rules. - The
PAT_TOKEN
andLLM_API_KEY
are used without environment restrictions or rotation policies. - Other workflows properly use
read-all
permissions, but thepkgdown
job has specificcontents: write
permission.
Recommendations:
- Define a protected environment for the OpenHands resolver workflow
- Restrict the
PAT_TOKEN
permissions to the minimum required scope - Implement a secret rotation policy for
LLM_API_KEY
- Consider using OIDC federation instead of long-lived PAT tokens
🔗 Analysis chain
Review security considerations for sensitive secrets
The workflow uses several sensitive secrets. Consider the following security recommendations:
- Ensure
PAT_TOKEN
has minimal required permissions - Consider adding environment protection rules for production environments
- Verify if
LLM_API_KEY
requires rotation policies
Let me gather more information about how these tokens are used across the workflows.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for environment protection rules
# Check if any environment protection rules exist
gh api repos/{owner}/{repo}/environments --jq '.[].protection_rules'
# Check for any existing secret rotation workflows
rg -l "GITHUB_TOKEN|PAT_TOKEN|API_KEY" .github/workflows/
Length of output: 371
Script:
#!/bin/bash
# Check the context and permissions of token usage in workflows
rg -A 5 -B 5 "GITHUB_TOKEN|PAT_TOKEN|API_KEY" .github/workflows/
# Check if there are any environment definitions
rg -l "environment:" .github/workflows/
# Check for any permissions definitions in workflows
rg -A 5 "permissions:" .github/workflows/
Length of output: 5329
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.
@coderabbitai What is Define a protected environment for the OpenHands resolver workflow?
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.
@zachmayer, defining a protected environment for the OpenHands resolver workflow involves setting up an environment in your GitHub repository to add additional security measures for workflows that use sensitive permissions and secrets.
By creating a protected environment, you can:
- Require manual approvals before a job can access the environment and its secrets.
- Restrict branch access, specifying which branches can use the environment.
- Set up protection rules like required reviewers or wait timers.
Here's how you can set it up:
-
Create a new environment:
- Go to your repository's Settings > Environments.
- Click on "New environment" and name it (e.g.,
openhands-production
).
-
Configure protection rules (optional but recommended):
- Add required reviewers who must approve before the workflow can access secrets.
- Restrict which branches can deploy to this environment.
- Set wait timers if needed.
-
Add secrets to the environment:
- Instead of storing sensitive secrets in repository-level secrets, add them to your new environment.
- This ensures that only workflows referencing this environment can access these secrets.
-
Modify your workflow to use the environment:
- Update your workflow file to specify the environment and remove the explicit
secrets
mapping since secrets will be inherited from the environment.
- Update your workflow file to specify the environment and remove the explicit
name: Resolve Issue with OpenHands
on:
# your triggers here
permissions:
contents: write
pull-requests: write
issues: write
jobs:
call-openhands-resolver:
environment: openhands-production # Reference your protected environment here
uses: All-Hands-AI/OpenHands/.github/workflows/openhands-resolver.yml@main
with:
macro: ${{ vars.OPENHANDS_MACRO || '@openhands-agent' }}
max_iterations: ${{ vars.OPENHANDS_MAX_ITER || 50 }}
# Remove the 'secrets' section; secrets are accessed from the environment
By associating your job with the openhands-production
environment, GitHub Actions will enforce the protection rules you've set, enhancing the security for workflows with elevated permissions or sensitive data.
Let me know if you have any questions or need further assistance!
https://github.com/All-Hands-AI/OpenHands/blob/main/openhands/resolver/README.md