Skip to content

Commit

Permalink
Make filename check more strict (pypi#14027)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
di authored and th3coop committed Jun 27, 2023
1 parent 65198aa commit e5d19a1
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 8 deletions.
34 changes: 28 additions & 6 deletions tests/unit/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -2310,31 +2310,53 @@ def test_upload_fails_with_diff_filename_same_blake2(
"400 File already exists. See /the/help/url/ for more information."
)

def test_upload_fails_with_wrong_filename(self, pyramid_config, db_request):
@pytest.mark.parametrize(
"filename, project_name",
[
# completely different
("nope-{version}.tar.gz", "something_else"),
("nope-{version}-py3-none-any.whl", "something_else"),
# starts with same prefix
("nope-{version}.tar.gz", "no"),
("nope-{version}-py3-none-any.whl", "no"),
# starts with same prefix with hyphen
("no-way-{version}.tar.gz", "no"),
("no_way-{version}-py3-none-any.whl", "no"),
],
)
def test_upload_fails_with_wrong_filename(
self, pyramid_config, db_request, metrics, filename, project_name
):
user = UserFactory.create()
pyramid_config.testing_securitypolicy(identity=user)
db_request.user = user
db_request.user_agent = "warehouse-tests/6.6.6"
EmailFactory.create(user=user)
project = ProjectFactory.create()
project = ProjectFactory.create(name=project_name)
release = ReleaseFactory.create(project=project, version="1.0")
RoleFactory.create(user=user, project=project)

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

db_request.POST = MultiDict(
{
"metadata_version": "1.2",
"name": project.name,
"version": release.version,
"filetype": "sdist",
"md5_digest": "nope!",
"md5_digest": _TAR_GZ_PKG_MD5,
"content": pretend.stub(
filename=filename,
file=io.BytesIO(b"a" * (legacy.MAX_FILESIZE + 1)),
filename=filename.format(version=release.version),
file=io.BytesIO(_TAR_GZ_PKG_TESTDATA),
type="application/tar",
),
}
)
db_request.help_url = lambda **kw: "/the/help/url/"

with pytest.raises(HTTPBadRequest) as excinfo:
legacy.file_upload(db_request)
Expand Down
14 changes: 12 additions & 2 deletions warehouse/forklift/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1208,10 +1208,20 @@ def file_upload(request):
# Ensure the filename doesn't contain any characters that are too 🌶️spicy🥵
_validate_filename(filename)

# Extract the project name from the filename and normalize it.
filename_prefix = pkg_resources.safe_name(
# For wheels, the project name is normalized and won't contain hyphens, so
# we can split on the first hyphen.
filename.partition("-")[0]
if filename.endswith(".whl")
# For source releases, we know that the version should not contain any
# hypens, so we can split on the last hypen to get the project name.
else filename.rpartition("-")[0]
).lower()

# Make sure that our filename matches the project that it is being uploaded
# to.
prefix = pkg_resources.safe_name(project.name).lower()
if not pkg_resources.safe_name(filename).lower().startswith(prefix):
if (prefix := pkg_resources.safe_name(project.name).lower()) != filename_prefix:
raise _exc_with_message(
HTTPBadRequest,
f"Start filename for {project.name!r} with {prefix!r}.",
Expand Down

0 comments on commit e5d19a1

Please sign in to comment.