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

Consolidate and fix workspace resolution #1947

Merged

Conversation

Pwuts
Copy link
Member

@Pwuts Pwuts commented Apr 16, 2023

Background

Currently, four commands use three different ways to resolve the workspace path and all implement it themselves. Also, workspace path resolution is broken for execute_shell.

PR #1903 also fixes the immediate failure of execute_code.py and image_gen.py, but does not address the unordered approach in other files.

Changes

  • Consolidated workspace resolution functionality in autogpt/workspace.py

Documentation

No changes for users, so no docs needed.

Test Plan

Tested by letting Auto-GPT read a file in the workspace, both directly and using cat.

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

p-i-
p-i- previously approved these changes Apr 16, 2023
@Pwuts Pwuts force-pushed the fix/workspace-path-resolution branch from efe38ed to 20b3113 Compare April 16, 2023 17:15
@nponeccop nponeccop added the B7 label Apr 16, 2023
@Pwuts Pwuts requested a review from p-i- April 16, 2023 17:25
@p-i- p-i- merged commit 11620cc into Significant-Gravitas:master Apr 16, 2023
@Pwuts Pwuts mentioned this pull request Apr 16, 2023
@hdkiller
Copy link
Contributor

hdkiller commented Apr 16, 2023

I am not sure if this PR related to your implementation but worth a look.

@Pwuts
Copy link
Member Author

Pwuts commented Apr 16, 2023

@hdkiller I think that vulnerability should be fixed here. :)

@Pwuts Pwuts deleted the fix/workspace-path-resolution branch April 16, 2023 17:39
@nponeccop nponeccop mentioned this pull request Apr 17, 2023
1 task
@Pwuts Pwuts mentioned this pull request Apr 17, 2023
1 task
@Significant-Gravitas Significant-Gravitas deleted a comment from hatgit Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants