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

Code Executor should by default use a temp directory for working directory rather than the current directory. #6041

Open
1 task done
ekzhu opened this issue Mar 20, 2025 · 2 comments
Labels
code-execution execute generated code help wanted Extra attention is needed proj-extensions
Milestone

Comments

@ekzhu
Copy link
Collaborator

ekzhu commented Mar 20, 2025

Confirmation

  • I confirm that I am a maintainer and so can use this template. If I am not, I understand this issue will be closed and I will be asked to use a different template.

Issue body

Right now, all code executors under python/packages/autogen-ext/src/autogen_ext/code_executors are defaulting to "." -- the current directory, as the working directory for reading and writing files.

This introduces a security issue as the current directory may contain code files and other data you don't want to expose to a model.

To fix this, the code executors' constructor parameter's default for work_dir should be None, and a temp directory should be created upon the start() method is called, and the temp directory is closed when stop() method is called.

For backward compatibility for LocalCommandLineExecutor and ACADynamicSessionsCodeExecutor, which didn't have start() and stop() methods prior to #6040, we should keep using "." as the default work directory if the start() method was not called and work directory was not provided by the user, and emit a DeprecationWarning to reminder user of the best practice.

For temporary directory, see Python's tempfile: https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory

@ekzhu ekzhu added code-execution execute generated code help wanted Extra attention is needed proj-extensions labels Mar 20, 2025
@ekzhu ekzhu added this to the 0.4.x-python milestone Mar 20, 2025
@federicovilla55
Copy link
Contributor

As the current implementation of CodeExecutor subclasses lack a standardized way to manage working directories
so they default to "."a new executor constructor could include the param work_dir: Optional[Union[Path, str]] = None.
Then in the executor's start() method, if no work_dir is provided, a temporary directory will be created using tempfile.TemporaryDirectory() as described in https://docs.python.org/3/library/tempfile.html.
The work_dir contains the directory the the executor uses if specified by the user in the constructor, otherwise it contains a python-defined temporary directory that is defined after the code executor is started (start()).
For backward compatibility, if neither a work_dir is provided nor the executor is started the executor defaults to the current standard directory, so ".". Plus a DeprecationWarning should be emitted to encourage updating the usage.

Should more complex work_dir validation check (e.g. user has right permissions to access files in a directory) be added? (So something more complex than mkdir(exist_ok=True))

Happy to hear any feedback or suggestion!

@ekzhu
Copy link
Collaborator Author

ekzhu commented Mar 21, 2025

Should more complex work_dir validation check (e.g. user has right permissions to access files in a directory) be added? (So something more complex than mkdir(exist_ok=True))

If the directory provided by the caller doesn't exist it should be an error. I think for now it is the caller's responsibility to set right permissions for the work directory if provided, otherwise the default should be a temp directory managed by the executor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-execution execute generated code help wanted Extra attention is needed proj-extensions
Projects
None yet
Development

No branches or pull requests

2 participants