fix: validate upload filename character set in file_svc by deacon-mp · Pull Request #3267 · mitre/caldera

deacon-mp · 2026-03-15T19:55:28Z

Reject filenames with path traversal, null bytes, or special characters.

Description

(insert summary)

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

Copilot

Pull request overview

This PR aims to harden file upload handling in FileSvc by adding a filename validator and applying it during multipart uploads, with accompanying tests.

Changes:

Added FileSvc._validate_filename to restrict filenames to a safe ASCII character set and block traversal/null bytes.
Integrated filename validation into save_multipart_file_upload before saving uploaded files.
Added a new security-focused test module for filename validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`app/service/file_svc.py`	Adds `_validate_filename` and uses it during multipart upload processing to filter unsafe filenames.
`tests/security/test_filename_validation.py`	Introduces tests covering allowed/disallowed filenames for the new validator.

Comments suppressed due to low confidence (1)

app/service/file_svc.py:125

On invalid filenames this code logs a warning and continues, but still returns a 200 OK at the end. If the goal is to reject unsafe uploads, consider failing the request (e.g., 400 Bad Request) and stopping processing rather than silently skipping invalid parts; otherwise clients may believe an upload succeeded when it was discarded.

                if not self._validate_filename(filename):
                    self.log.warning('Invalid filename rejected: %s', repr(filename))
                    continue
                await self.save_file(filename, bytes(await field.read()), target_dir,
                                     encrypt=encrypt, encoding=headers.get('x-file-encoding'))
                self.log.debug('Uploaded file %s/%s' % (target_dir, filename))
            return web.Response()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/service/file_svc.py

                _, filename = os.path.split(field.filename)
+                if not self._validate_filename(filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(filename))
+                    continue


app/service/file_svc.py

+        if not re.match(r'^[a-zA-Z0-9._\-]+$', name):
+            return False
+        return True


tests/security/test_filename_validation.py

+import unittest
+from app.service.file_svc import FileSvc
+
+
+class TestFilenameValidation(unittest.TestCase):
+    def test_valid_filenames(self):
+        self.assertTrue(FileSvc._validate_filename('test.ps1'))
+        self.assertTrue(FileSvc._validate_filename('my-payload_v2.exe'))
+        self.assertTrue(FileSvc._validate_filename('agent.bin'))
+
+    def test_invalid_path_traversal(self):
+        self.assertFalse(FileSvc._validate_filename('../../../etc/passwd'))
+        self.assertFalse(FileSvc._validate_filename('..\\windows\\system32'))
+
+    def test_invalid_null_byte(self):
+        self.assertFalse(FileSvc._validate_filename('test\x00.ps1'))
+
+    def test_invalid_special_chars(self):
+        self.assertFalse(FileSvc._validate_filename('test file.ps1'))
+        self.assertFalse(FileSvc._validate_filename('test;rm -rf /.ps1'))
+        self.assertFalse(FileSvc._validate_filename('<script>.js'))
+
+    def test_empty_filename(self):
+        self.assertFalse(FileSvc._validate_filename(''))
+
+    def test_too_long_filename(self):
+        self.assertFalse(FileSvc._validate_filename('a' * 256))
+        self.assertTrue(FileSvc._validate_filename('a' * 255))
+
+
+if __name__ == '__main__':
+    unittest.main()


app/service/file_svc.py

@@ -101,6 +116,9 @@ async def save_multipart_file_upload(self, request, target_dir, encrypt=True):
                if not field:
                    break
                _, filename = os.path.split(field.filename)
+                if not self._validate_filename(filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(filename))
+                    continue
                await self.save_file(filename, bytes(await field.read()), target_dir,
                                     encrypt=encrypt, encoding=headers.get('x-file-encoding'))


Copilot

Pull request overview

This PR tightens multipart upload handling by adding server-side validation for uploaded filenames in FileSvc, aiming to prevent path traversal and disallowed characters from being used in field.filename.

Changes:

Added _validate_filename() with a strict allowed-character policy and checks for traversal patterns and null bytes.
Enforced filename validation in save_multipart_file_upload() before calling os.path.split() and saving.
Added a new security-focused test module covering valid/invalid filename cases and the traversal-bypass scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`app/service/file_svc.py`	Adds filename validation logic and applies it during multipart uploads.
`tests/security/test_filename_validation.py`	Adds tests for allowed/disallowed filenames and traversal bypass prevention.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/service/file_svc.py

+        if not name or len(name) > 255:
+            return False
+        if '\x00' in name:
+            return False
+        if '..' in name or '/' in name or '\\' in name:
+            return False
+        if not _SAFE_FILENAME_RE.fullmatch(name):
+            return False
+        return True


app/service/file_svc.py

+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(field.filename))
+                    continue


app/service/file_svc.py

                if not field:
                    break
+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(field.filename))


tests/security/test_filename_validation.py

+    'test;rm -rf /.ps1',
+    '<script>.js',
+    '',
+    'a' * 256,


Reject filenames with path traversal, null bytes, or special characters.

…sal bypass; precompile regex; convert tests to pytest

A single '.' passes the safe-character regex but is not a valid upload filename. Add an explicit check and a parametrized test case.

Copilot

Pull request overview

Adds stricter filename validation to the legacy multipart upload path (FileSvc.save_multipart_file_upload) to prevent path traversal and unsafe filenames, and introduces targeted security tests for the validation logic.

Changes:

Add _validate_filename and a safe-character regex in FileSvc.
Validate field.filename before os.path.split() in multipart uploads and skip invalid names.
Add pytest coverage for valid/invalid filename cases and the traversal “split bypass” scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`app/service/file_svc.py`	Introduces filename validation and applies it to multipart uploads prior to splitting the path.
`tests/security/test_filename_validation.py`	Adds unit tests for filename validation, including traversal bypass regression coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/service/file_svc.py

                if not field:
                    break
+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(field.filename))


app/service/file_svc.py

                    break
+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(field.filename))
+                    continue


app/service/file_svc.py

+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %s', repr(field.filename))
+                    continue
                _, filename = os.path.split(field.filename)
                await self.save_file(filename, bytes(await field.read()), target_dir,


…r stall

Copilot

Pull request overview

Adds server-side validation for multipart upload filenames in FileSvc to prevent path traversal and disallowed characters, and introduces unit tests to verify acceptance/rejection behavior.

Changes:

Introduce _validate_filename (regex + traversal/null-byte checks) in app/service/file_svc.py.
Enforce filename validation before os.path.split() in save_multipart_file_upload.
Add a new tests/security/test_filename_validation.py test suite for valid/invalid filenames and the traversal-bypass scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
app/service/file_svc.py	Adds filename validation and applies it to multipart uploads before splitting/saving.
tests/security/test_filename_validation.py	Adds unit tests covering safe filenames, unsafe characters, and traversal bypass via `os.path.split()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/service/file_svc.py

                    break
+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %r', field.filename)
+                    await field.read()  # consume the part to allow reader to advance


app/service/file_svc.py

+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %r', field.filename)
+                    await field.read()  # consume the part to allow reader to advance
+                    continue


…ently skipping

Copilot

Pull request overview

This PR hardens the legacy /file/upload multipart upload flow by introducing filename validation to prevent path traversal, null bytes, and disallowed characters.

Changes:

Added a _validate_filename helper to enforce a strict allowed character set and reject traversal/path separators.
Applied filename validation to multipart uploads before os.path.split() to prevent traversal bypasses.
Added unit tests covering valid/invalid filename cases and traversal bypass scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`app/service/file_svc.py`	Adds `_validate_filename` and enforces it during multipart upload handling.
`tests/security/test_filename_validation.py`	Adds tests for filename validation rules and traversal bypass patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/service/file_svc.py

+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %r', field.filename)
+                    raise web.HTTPBadRequest(reason='Invalid filename: disallowed characters or path traversal')


tests/security/test_filename_validation.py

+def test_path_traversal_not_bypassed_by_split():
+    """Validate that traversal sequences in the full field.filename are rejected
+    BEFORE os.path.split() strips them — the critical fix for the path traversal bypass.
+    os.path.split('../../../etc/passwd') returns ('../../..', 'passwd');
+    validating only 'passwd' would incorrectly pass."""
+    traversal_filenames = [
+        '../../../etc/passwd',
+        '../../secret.yml',
+        'uploads/../../../etc/shadow',
+    ]
+    for raw_name in traversal_filenames:
+        assert not FileSvc._validate_filename(raw_name), \
+            f'Expected {raw_name!r} to be rejected by _validate_filename (traversal bypass risk)'


…vel test for invalid filename

Copilot

Pull request overview

Adds stricter filename validation for multipart uploads to prevent path traversal and unsafe characters, with regression/security test coverage.

Changes:

Introduces FileSvc._validate_filename() and a safe filename regex.
Rejects multipart upload parts with invalid field.filename by raising HTTPBadRequest.
Adds targeted tests for filename validation and a regression test for multipart uploads.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
app/service/file_svc.py	Adds filename allowlist validation and enforces it during multipart upload handling.
tests/services/test_file_svc.py	Adds regression test ensuring invalid multipart filenames are rejected with `HTTPBadRequest`.
tests/security/test_filename_validation.py	Adds parameterized tests covering valid/invalid filename cases and traversal bypass scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/service/file_svc.py

+        if name == '.' or '..' in name or '/' in name or '\\' in name:
+            return False


app/service/file_svc.py

                    break
+                if not self._validate_filename(field.filename):
+                    self.log.warning('Invalid filename rejected: %r', field.filename)
+                    raise web.HTTPBadRequest(reason='Invalid filename: disallowed characters or path traversal')


sonarqubecloud · 2026-03-16T01:23:29Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

* fix: validate upload filename character set in file_svc Reject filenames with path traversal, null bytes, or special characters. * fix: validate field.filename before os.path.split() to prevent traversal bypass; precompile regex; convert tests to pytest * fix: flake8 style fixes * fix: reject '.' as a filename and add test coverage for dot-only names A single '.' passes the safe-character regex but is not a valid upload filename. Add an explicit check and a parametrized test case. * fix: consume rejected multipart part before continue to prevent reader stall * fix: return 400 Bad Request on invalid upload filename instead of silently skipping * fix: re-raise HTTPException so HTTPBadRequest propagates; add HTTP-level test for invalid filename

deacon-mp requested a review from Copilot March 15, 2026 19:55

Copilot started reviewing on behalf of deacon-mp March 15, 2026 19:55 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

deacon-mp force-pushed the fix/filename-validation branch 2 times, most recently from 7644575 to 7c544be Compare March 15, 2026 23:19

deacon-mp requested a review from Copilot March 15, 2026 23:45

Copilot started reviewing on behalf of deacon-mp March 15, 2026 23:46 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

deacon-mp added 4 commits March 15, 2026 19:52

fix: validate upload filename character set in file_svc

b49d899

Reject filenames with path traversal, null bytes, or special characters.

fix: validate field.filename before os.path.split() to prevent traver…

fd1acde

…sal bypass; precompile regex; convert tests to pytest

fix: flake8 style fixes

ebc9737

fix: reject '.' as a filename and add test coverage for dot-only names

0666ba3

A single '.' passes the safe-character regex but is not a valid upload filename. Add an explicit check and a parametrized test case.

deacon-mp requested a review from Copilot March 15, 2026 23:54

deacon-mp force-pushed the fix/filename-validation branch from c384f75 to 0666ba3 Compare March 15, 2026 23:54

Copilot started reviewing on behalf of deacon-mp March 15, 2026 23:55 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

fix: consume rejected multipart part before continue to prevent reade…

6f87584

…r stall

deacon-mp requested a review from Copilot March 16, 2026 00:06

Copilot started reviewing on behalf of deacon-mp March 16, 2026 00:06 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

fix: return 400 Bad Request on invalid upload filename instead of sil…

41b5a62

…ently skipping

deacon-mp requested a review from Copilot March 16, 2026 00:43

Copilot started reviewing on behalf of deacon-mp March 16, 2026 00:47 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

fix: re-raise HTTPException so HTTPBadRequest propagates; add HTTP-le…

044b7c0

…vel test for invalid filename

deacon-mp requested a review from Copilot March 16, 2026 01:14

Copilot AI reviewed Mar 16, 2026

View reviewed changes

Copilot started reviewing on behalf of deacon-mp March 16, 2026 01:21 View session

deacon-mp merged commit 973ed61 into master Mar 16, 2026
19 checks passed

deacon-mp deleted the fix/filename-validation branch March 16, 2026 01:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: validate upload filename character set in file_svc#3267

fix: validate upload filename character set in file_svc#3267
deacon-mp merged 7 commits intomasterfrom
fix/filename-validation

deacon-mp commented Mar 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

sonarqubecloud bot commented Mar 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if name == '.' or '..' in name or '/' in name or '\\' in name:
		return False

Conversation

deacon-mp commented Mar 15, 2026

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sonarqubecloud bot commented Mar 16, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants