Skip to content
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

fix(platform): Fix marketplace leaking secrets #8281

Merged
merged 7 commits into from
Oct 8, 2024

Conversation

aarushik93
Copy link
Contributor

Background

Marketplace wasn't implemented with secret hiding. It downloads the agent as is and then submits it as is, with secrets in cleartext.

Changes 🏗️

Made the graph endpoint hideable. Added query param to hide secrets, which should hide the input data in the nodes when fetching

Testing 🔍

Note

Only for the new autogpt platform, currently in autogpt_platform/

  • Create from scratch and execute an agent with at least 3 blocks
  • Import an agent from file upload, and confirm it executes correctly
  • Upload agent to marketplace
  • Import an agent from marketplace and confirm it executes correctly
  • Edit an agent from monitor, and confirm it executes correctly

@aarushik93 aarushik93 requested a review from a team as a code owner October 8, 2024 13:38
@aarushik93 aarushik93 requested review from Pwuts, Torantulino, ntindle, Swiftyos and majdyz and removed request for a team, Pwuts and Torantulino October 8, 2024 13:38
@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end labels Oct 8, 2024
@github-actions github-actions bot added the size/m label Oct 8, 2024
Copy link

qodo-merge-pro bot commented Oct 8, 2024

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
While the PR aims to hide secrets, the implementation in _hide_secrets_in_input method uses a predefined list of sensitive keys. This approach might not catch all types of secrets, especially if they have custom or uncommon names. Consider implementing a more robust secret detection mechanism or allowing custom configuration of sensitive keys.

⚡ Recommended focus areas for review

Potential Bug
The _hide_secrets_in_input method uses a hardcoded list of sensitive keys. This approach might miss some custom or less common sensitive information.

Unused Variable
The undefined parameter is passed to api.getGraph(), but it's not clear what this parameter represents or if it's necessary.

Code Duplication
The query string construction is duplicated. Consider refactoring this into a separate method for better maintainability.

Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit c03332e
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/6705600c73493c0008203597

Swiftyos
Swiftyos previously approved these changes Oct 8, 2024
autogpt_platform/backend/backend/data/graph.py Outdated Show resolved Hide resolved
Co-authored-by: Reinier van der Leer <pwuts@agpt.co>
@aarushik93 aarushik93 requested review from Pwuts and Swiftyos October 8, 2024 14:33
Copy link
Member

@Pwuts Pwuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@aarushik93 aarushik93 merged commit bc1df92 into master Oct 8, 2024
16 checks passed
@aarushik93 aarushik93 deleted the aarushikansal/fix-marketplace-secrets branch October 8, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform/backend AutoGPT Platform - Back end platform/frontend AutoGPT Platform - Front end Review effort [1-5]: 3 size/m
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants