Sanitize direct download (#13313)

The 'direct' option in 'spacy download' is supposed to only download from our model releases repository. However, users were able to pass in a relative path, allowing download from arbitrary repositories. This meant that a service that sourced strings from user input and which used the direct option would allow users to install arbitrary packages.
This commit is contained in:
Matthew Honnibal 2024-02-20 13:17:51 +01:00 committed by GitHub
parent bff8725f4b
commit 0518c36f04
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 2 deletions

View File

@ -1,5 +1,7 @@
from wasabi import msg from wasabi import msg
# Needed for testing
from . import download as download_module # noqa: F401
from ._util import app, setup_cli # noqa: F401 from ._util import app, setup_cli # noqa: F401
from .apply import apply # noqa: F401 from .apply import apply # noqa: F401
from .assemble import assemble_cli # noqa: F401 from .assemble import assemble_cli # noqa: F401

View File

@ -1,5 +1,6 @@
import sys import sys
from typing import Optional, Sequence from typing import Optional, Sequence
from urllib.parse import urljoin
import requests import requests
import typer import typer
@ -63,6 +64,13 @@ def download(
) )
pip_args = pip_args + ("--no-deps",) pip_args = pip_args + ("--no-deps",)
if direct: if direct:
# Reject model names with '/', in order to prevent shenanigans.
if "/" in model:
msg.fail(
title="Model download rejected",
text=f"Cannot download model '{model}'. Models are expected to be file names, not URLs or fragments",
exits=True,
)
components = model.split("-") components = model.split("-")
model_name = "".join(components[:-1]) model_name = "".join(components[:-1])
version = components[-1] version = components[-1]
@ -153,7 +161,16 @@ def get_latest_version(model: str) -> str:
def download_model( def download_model(
filename: str, user_pip_args: Optional[Sequence[str]] = None filename: str, user_pip_args: Optional[Sequence[str]] = None
) -> None: ) -> None:
download_url = about.__download_url__ + "/" + filename # Construct the download URL carefully. We need to make sure we don't
# allow relative paths or other shenanigans to trick us into download
# from outside our own repo.
base_url = about.__download_url__
# urljoin requires that the path ends with /, or the last path part will be dropped
if not base_url.endswith("/"):
base_url = about.__download_url__ + "/"
download_url = urljoin(base_url, filename)
if not download_url.startswith(about.__download_url__):
raise ValueError(f"Download from {filename} rejected. Was it a relative path?")
pip_args = list(user_pip_args) if user_pip_args is not None else [] pip_args = list(user_pip_args) if user_pip_args is not None else []
cmd = [sys.executable, "-m", "pip", "install"] + pip_args + [download_url] cmd = [sys.executable, "-m", "pip", "install"] + pip_args + [download_url]
run_command(cmd) run_command(cmd)

View File

@ -12,7 +12,7 @@ from thinc.api import Config
import spacy import spacy
from spacy import about from spacy import about
from spacy.cli import info from spacy.cli import download_module, info
from spacy.cli._util import parse_config_overrides, string_to_list, walk_directory from spacy.cli._util import parse_config_overrides, string_to_list, walk_directory
from spacy.cli.apply import apply from spacy.cli.apply import apply
from spacy.cli.debug_data import ( from spacy.cli.debug_data import (
@ -1066,3 +1066,15 @@ def test_debug_data_trainable_lemmatizer_not_annotated():
def test_project_api_imports(): def test_project_api_imports():
from spacy.cli import project_run from spacy.cli import project_run
from spacy.cli.project.run import project_run # noqa: F401, F811 from spacy.cli.project.run import project_run # noqa: F401, F811
def test_download_rejects_relative_urls(monkeypatch):
"""Test that we can't tell spacy download to get an arbitrary model by using a
relative path in the filename"""
monkeypatch.setattr(download_module, "run_command", lambda cmd: None)
# Check that normal download works
download_module.download("en_core_web_sm-3.7.1", direct=True)
with pytest.raises(SystemExit):
download_module.download("../en_core_web_sm-3.7.1", direct=True)