mirror of
https://codeberg.org/davrot/forgejo.git
synced 2025-07-24 22:00:02 +02:00
Compare commits
9 commits
bec006b2eb
...
fb9e663660
Author | SHA1 | Date | |
---|---|---|---|
fb9e663660 | |||
b3e64faf8e | |||
c920c21f31 | |||
68c860d717 | |||
3e664400f8 | |||
![]() |
74e5ca6c8e | ||
![]() |
d2753f9845 | ||
![]() |
5da912f0a0 | ||
![]() |
8723da989d |
4 changed files with 313 additions and 131 deletions
|
@ -21,22 +21,25 @@ func SanitizePath(inputPath string) (string, error) {
|
|||
return "", fmt.Errorf("path starts with / : %s", inputPath)
|
||||
}
|
||||
|
||||
// Clean the path
|
||||
s = path.Clean(s)
|
||||
// Make path temporarily absolute to ensure proper cleaning of all components
|
||||
temp := "/" + s
|
||||
|
||||
// Clean the path (this will properly handle ../.. components when absolute)
|
||||
temp = path.Clean(temp)
|
||||
|
||||
// Remove the leading / to make it relative again
|
||||
s = strings.TrimPrefix(temp, "/")
|
||||
|
||||
// If the cleaned path is empty or becomes root, it's invalid
|
||||
if s == "" {
|
||||
return "", fmt.Errorf("path resolves to root or becomes empty after cleaning")
|
||||
}
|
||||
|
||||
// Split the path components
|
||||
pathComponents := strings.Split(s, "/")
|
||||
// Sanitize each path component
|
||||
var sanitizedComponents []string
|
||||
for _, component := range pathComponents {
|
||||
// There is no reason why there should be a path segment with ..
|
||||
if component == ".." {
|
||||
return "", fmt.Errorf("path contains directory traversal: %s", s)
|
||||
}
|
||||
// There is no reason why there should be a path segment with .
|
||||
if component == "." {
|
||||
return "", fmt.Errorf("path contains directory traversal: %s", s)
|
||||
}
|
||||
|
||||
// Trim whitespace and apply regex sanitization
|
||||
sanitizedComponent := strings.TrimSpace(fileNameComponentSanitizeRegexp.ReplaceAllString(component, "_"))
|
||||
|
||||
|
|
|
@ -75,6 +75,8 @@
|
|||
</select>
|
||||
</div>
|
||||
</div>
|
||||
<button id="commit-button" type="submit" class="ui primary button">{{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}}</button>
|
||||
<button id="commit-button" type="submit" class="ui primary button">
|
||||
{{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}}
|
||||
</button>
|
||||
<a class="ui button red" href="{{$.BranchLink}}/{{PathEscapeSegments .TreePath}}">{{ctx.Locale.Tr "repo.editor.cancel"}}</a>
|
||||
</div>
|
||||
|
|
|
@ -5,135 +5,102 @@
|
|||
// templates/repo/editor/**
|
||||
// web_src/js/features/common-global.js
|
||||
// routers/web/web.go
|
||||
// services/repository/files/upload.go
|
||||
// services/repository/files/**
|
||||
// @watch end
|
||||
|
||||
import {expect} from '@playwright/test';
|
||||
import {test, dynamic_id, save_visual} from './utils_e2e.ts';
|
||||
import {test, dynamic_id} from './utils_e2e.ts';
|
||||
|
||||
test.use({user: 'user2'});
|
||||
|
||||
interface TestCase {
|
||||
description: string;
|
||||
files: string[];
|
||||
}
|
||||
|
||||
async function doUpload({page}, testCase: TestCase) {
|
||||
await page.goto(`/user2/file-uploads/_upload/main/`);
|
||||
const testID = dynamic_id();
|
||||
const dropzone = page.getByRole('button', {name: 'Drop files or click here to upload.'});
|
||||
|
||||
// create the virtual files
|
||||
const dataTransfer = await page.evaluateHandle((testCase: TestCase) => {
|
||||
const dt = new DataTransfer();
|
||||
for (const filename of testCase.files) {
|
||||
dt.items.add(new File([`File content of ${filename}`], filename, {type: 'text/plain'}));
|
||||
}
|
||||
return dt;
|
||||
}, testCase);
|
||||
// and drop them to the upload area
|
||||
await dropzone.dispatchEvent('drop', {dataTransfer});
|
||||
|
||||
await page.getByText('new branch').click();
|
||||
|
||||
await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID);
|
||||
// ToDo: Potential race condition: We do not currently wait for the upload to complete.
|
||||
// See https://codeberg.org/forgejo/forgejo/pulls/6687#issuecomment-5068272 and
|
||||
// https://codeberg.org/forgejo/forgejo/issues/5893#issuecomment-5068266 for details.
|
||||
// Workaround is to wait (the uploads are just a few bytes and usually complete instantly)
|
||||
//
|
||||
// eslint-disable-next-line playwright/no-wait-for-timeout
|
||||
await page.waitForTimeout(100);
|
||||
|
||||
await page.getByRole('button', {name: 'Propose file change'}).click();
|
||||
}
|
||||
|
||||
test.describe('Drag and Drop upload', () => {
|
||||
test('normal and special characters', async ({page}) => {
|
||||
const response = await page.goto(`/user2/file-uploads/_upload/main/`);
|
||||
expect(response?.status()).toBe(200); // Status OK
|
||||
const goodTestCases: TestCase[] = [
|
||||
{
|
||||
description: 'normal and special characters',
|
||||
files: [
|
||||
'dir1/file1.txt',
|
||||
'double/nested/file.txt',
|
||||
'special/äüöÄÜÖß.txt',
|
||||
'special/Ʉ₦ł₵ØĐɆ.txt',
|
||||
],
|
||||
},
|
||||
{
|
||||
description: 'strange paths and spaces',
|
||||
files: [
|
||||
'..dots.txt',
|
||||
'.dots.preserved.txt',
|
||||
'special/S P A C E !.txt',
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
const testID = dynamic_id();
|
||||
const dropzone = page.getByRole('button', {name: 'Drop files or click here to upload.'});
|
||||
// actual good tests based on definition above
|
||||
for (const testCase of goodTestCases) {
|
||||
test(`good: ${testCase.description}`, async ({page}) => {
|
||||
await doUpload({page}, testCase);
|
||||
|
||||
// create the virtual files
|
||||
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(['Umlaut test'], 'special/äüöÄÜÖß.txt', {type: 'text/plain'}));
|
||||
dt.items.add(new File(['Unicode test'], 'special/Ʉ₦ł₵ØĐɆ.txt', {type: 'text/plain'}));
|
||||
return dt;
|
||||
// check that nested file structure is preserved
|
||||
for (const filename of testCase.files) {
|
||||
await expect(page.locator('#diff-file-boxes').getByRole('link', {name: filename})).toBeVisible();
|
||||
}
|
||||
});
|
||||
// and drop them to the upload area
|
||||
await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA});
|
||||
}
|
||||
|
||||
await page.getByText('new branch').click();
|
||||
const badTestCases: TestCase[] = [
|
||||
{
|
||||
description: 'broken path slash in front',
|
||||
files: [
|
||||
'/special/badfirstslash.txt',
|
||||
],
|
||||
},
|
||||
{
|
||||
description: 'broken path with traversal',
|
||||
files: [
|
||||
'../baddots.txt',
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
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.getByRole('link', {name: 'special/äüöÄÜÖß.txt'})).toBeVisible();
|
||||
await expect(page.getByRole('link', {name: 'special/Ʉ₦ł₵ØĐɆ.txt'})).toBeVisible();
|
||||
// Since this is a file in root, there two links with the same label
|
||||
// we take the on in #diff-file-tree
|
||||
await expect(page.locator('#diff-file-boxes').getByRole('link', {name: 'root_file.txt'})).toBeVisible();
|
||||
});
|
||||
|
||||
test('strange paths and spaces', 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'], '.dots.vanish.txt', {type: 'text/plain'}));
|
||||
dt.items.add(new File(['3'], 'special/S P A C E !.txt', {type: 'text/plain'}));
|
||||
return dt;
|
||||
// actual bad tests based on definition above
|
||||
for (const testCase of badTestCases) {
|
||||
test(`bad: ${testCase.description}`, async ({page}) => {
|
||||
await doUpload({page}, testCase);
|
||||
await expect(page.getByText('Failed to upload files to')).toBeVisible();
|
||||
});
|
||||
// and drop them to the upload area
|
||||
await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA});
|
||||
|
||||
await page.getByText('new branch').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
|
||||
// Since this is a file in root, there two links with the same label
|
||||
// we take the on in #diff-file-tree
|
||||
await expect(page.locator('#diff-file-boxes').getByRole('link', {name: '.dots.vanish.txt'})).toBeVisible();
|
||||
await expect(page.getByRole('link', {name: 'special/S P A C E !.txt'})).toBeVisible();
|
||||
// Since this is a file in root, there two links with the same label
|
||||
// we take the on in #diff-file-tree
|
||||
await expect(page.locator('#diff-file-boxes').getByRole('link', {name: '..dots.txt'})).toBeVisible();
|
||||
});
|
||||
|
||||
test('broken path slash in front', 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'], '/special/badfirstslash.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 page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID);
|
||||
await page.getByRole('button', {name: 'Propose file change'}).click();
|
||||
|
||||
await expect(page.getByText('Failed to upload files to')).toBeVisible();
|
||||
|
||||
await save_visual(page);
|
||||
});
|
||||
|
||||
test('broken path with traversal', 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'], '../baddots.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 page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID);
|
||||
await page.getByRole('button', {name: 'Propose file change'}).click();
|
||||
|
||||
await expect(page.getByText('Failed to upload files to')).toBeVisible();
|
||||
|
||||
await save_visual(page);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
210
tests/integration/pathutils_test.go
Normal file
210
tests/integration/pathutils_test.go
Normal file
|
@ -0,0 +1,210 @@
|
|||
// Copyright 2019 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
package integration
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"forgejo.org/services/repository/files"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestSanitizePath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
expected string
|
||||
expectError bool
|
||||
}{
|
||||
// Valid paths
|
||||
{
|
||||
name: "simple valid path",
|
||||
input: "folder/file.txt",
|
||||
expected: "folder/file.txt",
|
||||
},
|
||||
{
|
||||
name: "single file",
|
||||
input: "file.txt",
|
||||
expected: "file.txt",
|
||||
},
|
||||
{
|
||||
name: "nested path",
|
||||
input: "a/b/c/file.txt",
|
||||
expected: "a/b/c/file.txt",
|
||||
},
|
||||
|
||||
// Path normalization
|
||||
{
|
||||
name: "backslash to forward slash",
|
||||
input: "folder\\file.txt",
|
||||
expected: "folder/file.txt",
|
||||
},
|
||||
{
|
||||
name: "mixed separators",
|
||||
input: "folder\\subfolder/file.txt",
|
||||
expected: "folder/subfolder/file.txt",
|
||||
},
|
||||
{
|
||||
name: "double separators",
|
||||
input: "folder//file.txt",
|
||||
expected: "folder/file.txt",
|
||||
},
|
||||
{
|
||||
name: "trailing slash",
|
||||
input: "folder/file.txt/",
|
||||
expected: "folder/file.txt",
|
||||
},
|
||||
{
|
||||
name: "dot segments",
|
||||
input: "folder/./file.txt",
|
||||
expected: "folder/file.txt",
|
||||
},
|
||||
{
|
||||
name: "parent directory references",
|
||||
input: "folder/../other/file.txt",
|
||||
expected: "other/file.txt",
|
||||
},
|
||||
|
||||
// Character sanitization
|
||||
{
|
||||
name: "illegal characters replaced",
|
||||
input: "file<name>.txt",
|
||||
expected: "file_name_.txt",
|
||||
},
|
||||
{
|
||||
name: "multiple illegal characters",
|
||||
input: "file:name|with?bad*chars.txt",
|
||||
expected: "file_name_with_bad_chars.txt",
|
||||
},
|
||||
{
|
||||
name: "quotes in filename",
|
||||
input: `file"name.txt`,
|
||||
expected: "file_name.txt",
|
||||
},
|
||||
{
|
||||
name: "control characters",
|
||||
input: "file\x00\x01name.txt",
|
||||
expected: "file__name.txt",
|
||||
},
|
||||
|
||||
// Whitespace handling
|
||||
{
|
||||
name: "leading whitespace",
|
||||
input: " file.txt",
|
||||
expected: "file.txt",
|
||||
},
|
||||
{
|
||||
name: "trailing whitespace",
|
||||
input: "file.txt ",
|
||||
expected: "file.txt",
|
||||
},
|
||||
{
|
||||
name: "whitespace in path components",
|
||||
input: " folder / file.txt ",
|
||||
expected: "folder/file.txt",
|
||||
},
|
||||
|
||||
// Edge cases that should return errors
|
||||
{
|
||||
name: "path starts with slash",
|
||||
input: "/folder/file.txt",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "empty string",
|
||||
input: "",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "only separators",
|
||||
input: "///",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "only illegal characters",
|
||||
input: "<>:\"|?*",
|
||||
expected: "_______",
|
||||
},
|
||||
{
|
||||
name: "only whitespace",
|
||||
input: " ",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "path that resolves to root",
|
||||
input: "../..",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "path that goes above root",
|
||||
input: "folder/../../..",
|
||||
expectError: true,
|
||||
},
|
||||
|
||||
// Complex combinations
|
||||
{
|
||||
name: "complex path with multiple issues",
|
||||
input: "folder\\with:illegal|chars/normal_file.txt",
|
||||
expected: "folder/with_illegal_chars/normal_file.txt",
|
||||
},
|
||||
{
|
||||
name: "unicode characters preserved",
|
||||
input: "folder/файл.txt",
|
||||
expected: "folder/файл.txt",
|
||||
},
|
||||
{
|
||||
name: "dots and extensions",
|
||||
input: "file.name.with.dots.txt",
|
||||
expected: "file.name.with.dots.txt",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result, err := files.SanitizePath(tt.input)
|
||||
|
||||
if tt.expectError {
|
||||
require.Error(t, err, "expected error for input %q", tt.input)
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err, "unexpected error for input %q", tt.input)
|
||||
assert.Equal(t, tt.expected, result, "SanitizePath(%q) should return expected result", tt.input)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSanitizePathErrorMessages tests that error messages are informative
|
||||
func TestSanitizePathErrorMessages(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
expectedError string
|
||||
}{
|
||||
{
|
||||
name: "path starts with slash",
|
||||
input: "/test/path",
|
||||
expectedError: "path starts with / : /test/path",
|
||||
},
|
||||
{
|
||||
name: "path that resolves to root",
|
||||
input: "../..",
|
||||
expectedError: "path resolves to root or becomes empty after cleaning",
|
||||
},
|
||||
{
|
||||
name: "empty after sanitization",
|
||||
input: "",
|
||||
expectedError: "path resolves to root or becomes empty after cleaning",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
_, err := files.SanitizePath(tt.input)
|
||||
require.Error(t, err, "expected error for input %q", tt.input)
|
||||
assert.Equal(t, tt.expectedError, err.Error(), "error message for %q should match expected", tt.input)
|
||||
})
|
||||
}
|
||||
}
|
Loading…
Add table
Add a link
Reference in a new issue