Skip to content

Commit 49217ed

Browse files
authored
Make filename check more strict (#14027)
* Add a failing test * Make the test fail all the way to DID NOT RAISE * Refactor the test a bit * Add another failing test case * More test cases * Ensure filenames only have the project name No more, no less. Previously this allowed filenames that started with the project name.
1 parent b10e894 commit 49217ed

File tree

2 files changed

+40
-8
lines changed

2 files changed

+40
-8
lines changed

tests/unit/forklift/test_legacy.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2310,31 +2310,53 @@ def test_upload_fails_with_diff_filename_same_blake2(
23102310
"400 File already exists. See /the/help/url/ for more information."
23112311
)
23122312

2313-
def test_upload_fails_with_wrong_filename(self, pyramid_config, db_request):
2313+
@pytest.mark.parametrize(
2314+
"filename, project_name",
2315+
[
2316+
# completely different
2317+
("nope-{version}.tar.gz", "something_else"),
2318+
("nope-{version}-py3-none-any.whl", "something_else"),
2319+
# starts with same prefix
2320+
("nope-{version}.tar.gz", "no"),
2321+
("nope-{version}-py3-none-any.whl", "no"),
2322+
# starts with same prefix with hyphen
2323+
("no-way-{version}.tar.gz", "no"),
2324+
("no_way-{version}-py3-none-any.whl", "no"),
2325+
],
2326+
)
2327+
def test_upload_fails_with_wrong_filename(
2328+
self, pyramid_config, db_request, metrics, filename, project_name
2329+
):
23142330
user = UserFactory.create()
23152331
pyramid_config.testing_securitypolicy(identity=user)
23162332
db_request.user = user
2333+
db_request.user_agent = "warehouse-tests/6.6.6"
23172334
EmailFactory.create(user=user)
2318-
project = ProjectFactory.create()
2335+
project = ProjectFactory.create(name=project_name)
23192336
release = ReleaseFactory.create(project=project, version="1.0")
23202337
RoleFactory.create(user=user, project=project)
23212338

2322-
filename = f"nope-{release.version}.tar.gz"
2339+
storage_service = pretend.stub(store=lambda path, filepath, meta: None)
2340+
db_request.find_service = lambda svc, name=None, context=None: {
2341+
IFileStorage: storage_service,
2342+
IMetricsService: metrics,
2343+
}.get(svc)
23232344

23242345
db_request.POST = MultiDict(
23252346
{
23262347
"metadata_version": "1.2",
23272348
"name": project.name,
23282349
"version": release.version,
23292350
"filetype": "sdist",
2330-
"md5_digest": "nope!",
2351+
"md5_digest": _TAR_GZ_PKG_MD5,
23312352
"content": pretend.stub(
2332-
filename=filename,
2333-
file=io.BytesIO(b"a" * (legacy.MAX_FILESIZE + 1)),
2353+
filename=filename.format(version=release.version),
2354+
file=io.BytesIO(_TAR_GZ_PKG_TESTDATA),
23342355
type="application/tar",
23352356
),
23362357
}
23372358
)
2359+
db_request.help_url = lambda **kw: "/the/help/url/"
23382360

23392361
with pytest.raises(HTTPBadRequest) as excinfo:
23402362
legacy.file_upload(db_request)

warehouse/forklift/legacy.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,10 +1208,20 @@ def file_upload(request):
12081208
# Ensure the filename doesn't contain any characters that are too 🌶️spicy🥵
12091209
_validate_filename(filename)
12101210

1211+
# Extract the project name from the filename and normalize it.
1212+
filename_prefix = pkg_resources.safe_name(
1213+
# For wheels, the project name is normalized and won't contain hyphens, so
1214+
# we can split on the first hyphen.
1215+
filename.partition("-")[0]
1216+
if filename.endswith(".whl")
1217+
# For source releases, we know that the version should not contain any
1218+
# hypens, so we can split on the last hypen to get the project name.
1219+
else filename.rpartition("-")[0]
1220+
).lower()
1221+
12111222
# Make sure that our filename matches the project that it is being uploaded
12121223
# to.
1213-
prefix = pkg_resources.safe_name(project.name).lower()
1214-
if not pkg_resources.safe_name(filename).lower().startswith(prefix):
1224+
if (prefix := pkg_resources.safe_name(project.name).lower()) != filename_prefix:
12151225
raise _exc_with_message(
12161226
HTTPBadRequest,
12171227
f"Start filename for {project.name!r} with {prefix!r}.",

0 commit comments

Comments
 (0)