Fix double escaping in Build worker tar command#11250
Fix double escaping in Build worker tar command#11250nathfavour wants to merge 1 commit intoappwrite:mainfrom
Conversation
📝 WalkthroughWalkthroughModified the tar command invocation in the Builds.php file to remove Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (1)
420-430:⚠️ Potential issue | 🟡 MinorPre-existing double-quoting issue in the
mvcommand.This code block handles directories with spaces, which aligns with this PR's objective. However, line 424 has the same class of escaping issue:
escapeshellarg()already wraps arguments in single quotes, so the additional double quotes create malformed syntax likemv "'/path/with space/from'" "'/path/with space/to'".Consider fixing this for consistency with the tar command fixes:
🐛 Proposed fix
- $exit = Console::execute('mv "' . \escapeshellarg($from) . '" "' . \escapeshellarg($to) . '"', '', $stdout, $stderr); + $exit = Console::execute('mv ' . \escapeshellarg($from) . ' ' . \escapeshellarg($to), '', $stdout, $stderr);
1 participant
I noticed that the tar command in the Build worker was using both escapeshellarg and escapeshellcmd on the same variable, which caused build failures for directory paths with special characters like spaces. I've removed the redundant escaping to ensure the paths are handled correctly and securely.