fix(tools): quote Daytona sync paths before remote mkdir - #6072
Closed
Dusk1e wants to merge 1 commit into
Closed
Conversation
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While exploring the Daytona environment integration, I found myself tracing the initial synchronization process that Hermes performs to mirror local skills and credentials into the sandbox. Everything appeared standard on the surface, but as I dug deeper into the _upload_if_changed method, something caught my eye.
I noticed that before any file is uploaded, the system has to ensure the remote parent directory exists. It does this by building a quick mkdir -p shell command. However, the path was being dropped directly into the command string without any defensive quoting.
This led me to wonder: what if a skill file wasn't named as expected? Upon closer inspection, I realized that an attacker who could control a skill's filename—perhaps by submitting a malicious repository—could inject shell metacharacters. The plot thickened when I realized that Hermes syncs credentials into the same sandbox. An injected ; or & in a filename could allow a command to run and exfiltrate those very secrets before the backend even finished initializing.
To secure this boundary, I’ve introduced proper shell quoting using shlex.quote() and added the POSIX -- flag to the mkdir call. This ensures that every directory path is treated strictly as data, no matter how "evil" its name might be.
I've included a regression test that mimics a malicious skill path to prove the fix works. The test demonstrates that a path containing a command injection payload is now safely neutralized and treated as a literal string by the Daytona backend.