From ab50c5c1a87ba04c4a8d4ceee432d073fa9e9379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Blanquer=E2=80=8B?= Date: Wed, 21 Jan 2026 21:45:29 +0100 Subject: [PATCH] fix: address CodeRabbit review comments - Add path validation to /api/create-folder endpoint (forbidden system dirs) - Fix repo name extraction to handle trailing slashes in URLs - Add cleanup of partial clone directory on SSE clone failure - Remove dead code branch (unreachable message) - Fix Windows path separator detection in createNewFolder - Replace alert() with setError() for consistent error handling - Detect ssh:// URLs in addition to git@ for SSH key display - Show create folder button for both workspace types --- server/index.js | 14 ++++++++++++++ server/routes/projects.js | 15 ++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/server/index.js b/server/index.js index dd86757..1470398 100755 --- a/server/index.js +++ b/server/index.js @@ -550,6 +550,8 @@ app.get('/api/browse-filesystem', authenticateToken, async (req, res) => { } }); +const FORBIDDEN_PATHS = ['/', '/etc', '/bin', '/sbin', '/usr', '/dev', '/proc', '/sys', '/var', '/boot', '/root', '/lib', '/lib64', '/opt', '/tmp', '/run']; + app.post('/api/create-folder', authenticateToken, async (req, res) => { try { const { path: folderPath } = req.body; @@ -558,6 +560,18 @@ app.post('/api/create-folder', authenticateToken, async (req, res) => { } const homeDir = os.homedir(); const targetPath = path.resolve(folderPath.replace('~', homeDir)); + const normalizedPath = path.normalize(targetPath); + if (FORBIDDEN_PATHS.includes(normalizedPath) || normalizedPath === '/') { + return res.status(403).json({ error: 'Cannot create folders in system directories' }); + } + for (const forbidden of FORBIDDEN_PATHS) { + if (normalizedPath.startsWith(forbidden + path.sep)) { + if (forbidden === '/var' && (normalizedPath.startsWith('/var/tmp') || normalizedPath.startsWith('/var/folders'))) { + continue; + } + return res.status(403).json({ error: `Cannot create folders in system directory: ${forbidden}` }); + } + } const parentDir = path.dirname(targetPath); try { await fs.promises.access(parentDir); diff --git a/server/routes/projects.js b/server/routes/projects.js index 31df1a2..7e3c7a1 100644 --- a/server/routes/projects.js +++ b/server/routes/projects.js @@ -234,7 +234,8 @@ router.post('/create-workspace', async (req, res) => { } // Extract repo name from URL for the clone destination - const repoName = githubUrl.replace(/\.git$/, '').split('/').pop(); + const normalizedUrl = githubUrl.replace(/\/+$/, '').replace(/\.git$/, ''); + const repoName = normalizedUrl.split('/').pop() || 'repository'; const clonePath = path.join(absolutePath, repoName); // Clone the repository into a subfolder @@ -267,9 +268,7 @@ router.post('/create-workspace', async (req, res) => { return res.json({ success: true, project, - message: githubUrl - ? 'New workspace created and repository cloned successfully' - : 'New workspace created successfully' + message: 'New workspace created successfully' }); } @@ -349,7 +348,8 @@ router.get('/clone-progress', async (req, res) => { githubToken = newGithubToken; } - const repoName = githubUrl.replace(/\.git$/, '').split('/').pop(); + const normalizedUrl = githubUrl.replace(/\/+$/, '').replace(/\.git$/, ''); + const repoName = normalizedUrl.split('/').pop() || 'repository'; const clonePath = path.join(absolutePath, repoName); let cloneUrl = githubUrl; @@ -410,6 +410,11 @@ router.get('/clone-progress', async (req, res) => { } else if (lastError) { errorMessage = lastError; } + try { + await fs.rm(clonePath, { recursive: true, force: true }); + } catch (cleanupError) { + console.error('Failed to clean up after clone failure:', cleanupError); + } sendEvent('error', { message: errorMessage }); } res.end();