diff --git a/services/repository/files/pathutils.go b/services/repository/files/pathutils.go new file mode 100644 index 0000000000..0adab4093b --- /dev/null +++ b/services/repository/files/pathutils.go @@ -0,0 +1,39 @@ +package files + +import ( + "fmt" + "path" + "regexp" + "strings" +) + +var fileNameComponentSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) + +// SanitizePath cleans and validates a file path +func SanitizePath(inputPath string) (string, error) { + // Normalize path separators + s := strings.ReplaceAll(inputPath, "\\", "/") + // Clean the path + s = path.Clean(s) + // Split the path components + pathComponents := strings.Split(s, "/") + // Sanitize each path component + var sanitizedComponents []string + for _, component := range pathComponents { + // Trim whitespace and apply regex sanitization + sanitizedComponent := strings.TrimSpace(fileNameComponentSanitizeRegexp.ReplaceAllString(component, "_")) + + // Skip empty components after sanitization + if sanitizedComponent != "" { + sanitizedComponents = append(sanitizedComponents, sanitizedComponent) + } + } + // Check if we have any components left after sanitization + if len(sanitizedComponents) == 0 { + return "", fmt.Errorf("path became empty after sanitization") + } + // Reconstruct the path + reconstructedPath := path.Join(sanitizedComponents...) + return reconstructedPath, nil +} + diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index ac6bdd5b53..4a313a1ba5 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -6,10 +6,8 @@ package files import ( "context" "fmt" - "html" "os" "path" - "regexp" "strings" git_model "code.gitea.io/gitea/models/git" @@ -72,8 +70,10 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use if err != nil { return fmt.Errorf("GetUploadByUUID [uuids: %v]: %w", opts.Files[i], err) } - uploads[i].Name = fileNameSanitize(html.UnescapeString(opts.FullPaths[i])) - + uploads[i].Name, err = SanitizePath(opts.FullPaths[i]) + if err != nil { + return fmt.Errorf("SanitizePath [path: %v]: %w", opts.FullPaths[i], err) + } // Check file is not lfs locked, will return nil if lock setting not enabled filepath := path.Join(opts.TreePath, uploads[i].Name) lfsLock, err := git_model.GetTreePathLock(ctx, repo.ID, filepath) @@ -165,19 +165,6 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return repo_model.DeleteUploads(ctx, uploads...) } -// From forgejo/services/repository/generate.go (but now allows /) -var fileNameSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) - -// Sanitize user input to valid OS filenames -// -// Based on https://github.com/sindresorhus/filename-reserved-regex -// Adds ".." to prevent directory traversal -func fileNameSanitize(s string) string { - // Added this because I am not sure what Windows will deliver us \ or / but we need /. - s = strings.ReplaceAll(s, "\\", "/") - return strings.TrimSpace(fileNameSanitizeRegexp.ReplaceAllString(s, "_")) -} - func copyUploadedLFSFilesIntoRepository(infos []uploadInfo, t *TemporaryUploadRepository, treePath string) error { var storeInLFSFunc func(string) (bool, error) diff --git a/tests/e2e/repo-files.test.e2e.ts b/tests/e2e/repo-files.test.e2e.ts index f99d28e943..e282e57b41 100644 --- a/tests/e2e/repo-files.test.e2e.ts +++ b/tests/e2e/repo-files.test.e2e.ts @@ -5,40 +5,85 @@ // services/repository/files/upload.go // @watch end -import {expect} from '@playwright/test'; -import {test, dynamic_id, save_visual} from './utils_e2e.ts'; +// https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md +// lies. Don't use +// make TAGS="sqlite sqlite_unlock_notify" backend +// use +// make TAGS="sqlite sqlite_unlock_notify" build +// otherwise the test fails. -test.use({user: 'user2'}); -test('drap and drop upload', async ({page}) => { +import { expect } from '@playwright/test'; +import { test, dynamic_id, save_visual } from './utils_e2e.ts'; + +test.use({ user: 'user2' }); + +test('drag and drop upload a', async ({ page }) => { const response = await page.goto(`/user2/file-uploads/_upload/main/`); expect(response?.status()).toBe(200); // Status OK const testID = dynamic_id(); - const dropzone = page.getByRole('button', {name: 'Drop files or click here to upload.'}); + const dropzone = page.getByRole('button', { name: 'Drop files or click here to upload.' }); // create the virtual files - const dataTransfer = await page.evaluateHandle(() => { + const dataTransferA = await page.evaluateHandle(() => { const dt = new DataTransfer(); - // add items in different folders - dt.items.add(new File(['Filecontent (dir1/file1.txt)'], 'dir1/file1.txt', {type: 'text/plain'})); - dt.items.add(new File(["Another file's content (double/nested/file.txt)"], 'double/nested/file.txt', {type: 'text/plain'})); - dt.items.add(new File(['Root file (root_file.txt)'], 'root_file.txt', {type: 'text/plain'})); - + dt.items.add(new File(['Filecontent (dir1/file1.txt)'], 'dir1/file1.txt', { type: 'text/plain' })); + dt.items.add(new File(["Another file's content(double / nested / file.txt)"], 'double / nested / file.txt', { type: 'text / plain' })); + dt.items.add(new File(['Root file (root_file.txt)'], 'root_file.txt', { type: 'text/plain' })); + dt.items.add(new File(['Umlaut test'], 'special/äüöÄÜÖß.txt', { type: 'text/plain' })); + dt.items.add(new File(['Unicode test'], 'special/Ʉ₦ł₵ØĐɆ.txt', { type: 'text/plain' })); return dt; }); // and drop them to the upload area - await dropzone.dispatchEvent('drop', {dataTransfer}); + await dropzone.dispatchEvent('drop', { dataTransfer: dataTransferA }); await page.getByText('new branch').click(); await save_visual(page); - await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID); - await page.getByRole('button', {name: 'Propose file change'}).click(); + await page.getByRole('textbox', { name: 'Name the new branch for this' }).fill(testID); + await page.getByRole('button', { name: 'Propose file change' }).click(); // check that nested file structure is preserved - await expect(page.getByRole('link', {name: 'dir1/file1.txt'})).toBeVisible(); - await expect(page.getByRole('link', {name: 'double/nested/file.txt'})).toBeVisible(); - await expect(page.locator('#diff-file-boxes').getByRole('link', {name: 'root_file.txt', exact: true})).toBeVisible(); + await expect(page.getByRole('link', { name: 'dir1/file1.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'double/nested/file.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'special/äüöÄÜÖß.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'special/Ʉ₦ł₵ØĐɆ.txt' })).toBeVisible(); + await expect(page.locator('#diff-file-boxes').getByRole('link', { name: 'root_file.txt', exact: true })).toBeVisible(); await save_visual(page); }); + +test('drag and drop upload b', async ({ page }) => { + const response = await page.goto(`/user2/file-uploads/_upload/main/`); + expect(response?.status()).toBe(200); // Status OK + + const testID = dynamic_id(); + const dropzone = page.getByRole('button', { name: 'Drop files or click here to upload.' }); + + // create the virtual files + const dataTransferA = await page.evaluateHandle(() => { + const dt = new DataTransfer(); + // add items in different folders + dt.items.add(new File(['1'], '../../dots.txt', { type: 'text/plain' })); + dt.items.add(new File(['2'], 'special/../../dots_vanish.txt', { type: 'text/plain' })); + dt.items.add(new File(['3'], '\\windows\\windows_slash.txt', { type: 'text/plain' })); + dt.items.add(new File(['4'], '/special/badfirstslash.txt', { type: 'text/plain' })); + dt.items.add(new File(['5'], 'special/S P A C E !.txt', { type: 'text/plain' })); + return dt; + }); + // and drop them to the upload area + await dropzone.dispatchEvent('drop', { dataTransfer: dataTransferA }); + + await page.getByText('new branch').click(); + await save_visual(page); + await page.getByRole('textbox', { name: 'Name the new branch for this' }).fill(testID); + await page.getByRole('button', { name: 'Propose file change' }).click(); + + // check that nested file structure is preserved + await expect(page.getByRole('link', { name: 'windows/windows_slash.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: '_/dots_vanish.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'special/badfirstslash.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'special/S P A C E !.txt' })).toBeVisible(); + await expect(page.locator('#diff-file-boxes').getByRole('link', { name: '_/_/dots.txt', exact: true })).toBeVisible(); + await save_visual(page); +}); \ No newline at end of file