Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict proxy URLs #4406

Merged
merged 19 commits into from
Jun 5, 2023
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ No changes to highlight.

## Bug Fixes:

No changes to highlight.
- Restricts the domains that can be proxied via `/proxy` route by [@abidlabs](https://github.com/abidlabs) in [PR 4406](https://github.com/gradio-app/gradio/pull/4406).

## Other Changes:

Expand Down
16 changes: 12 additions & 4 deletions gradio/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ def __init__(
self.allowed_paths = []
self.blocked_paths = []
self.root_path = ""
self.root_urls = set()

if self.analytics_enabled:
is_custom_theme = not any(
Expand All @@ -758,20 +759,22 @@ def from_config(
cls,
config: dict,
fns: list[Callable],
root_url: str | None = None,
root_url: str,
abidlabs marked this conversation as resolved.
Show resolved Hide resolved
) -> Blocks:
"""
Factory method that creates a Blocks from a config and list of functions.
Factory method that creates a Blocks from a config and list of functions. Used
internally by the gradio.external.load() method.

Parameters:
config: a dictionary containing the configuration of the Blocks.
fns: a list of functions that are used in the Blocks. Must be in the same order as the dependencies in the config.
root_url: an optional root url to use for the components in the Blocks. Allows serving files from an external URL.
root_url: an external url to use as a root URL when serving files for components in the Blocks.
"""
config = copy.deepcopy(config)
components_config = config["components"]
theme = config.get("theme", "default")
original_mapping: dict[int, Block] = {}
root_urls = set()
abidlabs marked this conversation as resolved.
Show resolved Hide resolved

def get_block_instance(id: int) -> Block:
for block_config in components_config:
Expand All @@ -783,8 +786,10 @@ def get_block_instance(id: int) -> Block:
block_config["props"].pop("type", None)
block_config["props"].pop("name", None)
style = block_config["props"].pop("style", None)
if block_config["props"].get("root_url") is None and root_url:
if block_config["props"].get("root_url") is None:
block_config["props"]["root_url"] = f"{root_url}/"
else:
root_urls.add(block_config["props"]["root_url"])
abidlabs marked this conversation as resolved.
Show resolved Hide resolved
# Any component has already processed its initial value, so we skip that step here
block = cls(**block_config["props"], _skip_init_processing=True)
if style and isinstance(block, components.IOComponent):
Expand Down Expand Up @@ -860,6 +865,8 @@ def iterate_over_children(children_list):
blocks.__name__ = "Interface"
blocks.api_mode = True

root_urls.add(root_url)
abidlabs marked this conversation as resolved.
Show resolved Hide resolved
blocks.root_urls = root_urls
return blocks

def __str__(self):
Expand Down Expand Up @@ -935,6 +942,7 @@ def render(self):
Context.root_block.fns[dependency_offset + i] = new_fn
Context.root_block.dependencies.append(dependency)
Context.root_block.temp_file_sets.extend(self.temp_file_sets)
Context.root_block.root_urls.update(self.root_urls)

if Context.block is not None:
Context.block.children.extend(self.children)
Expand Down
17 changes: 12 additions & 5 deletions gradio/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,16 @@ def get_blocks(self) -> gradio.Blocks:
raise ValueError("No Blocks has been configured for this app.")
return self.blocks

@staticmethod
def build_proxy_request(url_path):
def build_proxy_request(self, url_path):
url = httpx.URL(url_path)
is_hf_url = url.host.endswith(".hf.space")
assert self.blocks
# Don't proxy a URL unless it's a URL specifically loaded by the user using
# gr.load() to prevent SSRF or harvesting of HF tokens by malicious Spaces.
is_safe_url = any(url.host.endswith(root) for root in self.blocks.root_urls)
abidlabs marked this conversation as resolved.
Show resolved Hide resolved
if not is_safe_url:
raise PermissionError("This URL cannot be proxied.")
headers = {}
if Context.hf_token is not None and is_hf_url:
if Context.hf_token is not None:
abidlabs marked this conversation as resolved.
Show resolved Hide resolved
headers["Authorization"] = f"Bearer {Context.hf_token}"
rp_req = client.build_request("GET", url, headers=headers)
return rp_req
Expand Down Expand Up @@ -311,7 +315,10 @@ async def favicon():
@app.get("/proxy={url_path:path}", dependencies=[Depends(login_check)])
async def reverse_proxy(url_path: str):
# Adapted from: https://github.com/tiangolo/fastapi/issues/1788
rp_req = app.build_proxy_request(url_path)
try:
rp_req = app.build_proxy_request(url_path)
except PermissionError as err:
raise HTTPException(status_code=400, detail=str(err))
rp_resp = await client.send(rp_req, stream=True)
return StreamingResponse(
rp_resp.aiter_raw(),
Expand Down
8 changes: 7 additions & 1 deletion test/test_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ def ct_model(diseases, img):
assert demo.config["show_api"] is False

def test_load_from_config(self):
fake_url = "https://fake.hf.space"

def update(name):
return f"Welcome to Gradio, {name}!"

Expand All @@ -149,8 +151,12 @@ def update(name):
gr.Image().style(height=54, width=240)

config1 = demo1.get_config_file()
demo2 = gr.Blocks.from_config(config1, [update])
demo2 = gr.Blocks.from_config(config1, [update], "https://fake.hf.space")

for component in config1["components"]:
component["props"]["root_url"] = f"{fake_url}/"
config2 = demo2.get_config_file()

assert assert_configs_are_equivalent_besides_ids(config1, config2)

def test_partial_fn_in_config(self):
Expand Down
15 changes: 9 additions & 6 deletions test/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,15 @@ def test_mount_gradio_app_raises_error_if_event_queued_but_queue_disabled(self):

def test_proxy_does_not_leak_hf_token_externally(self):
gr.context.Context.hf_token = "abcdef"
r = routes.App.build_proxy_request(
"https://gradio-tests-test-loading-examples-private.hf.space/file=Bunny.obj"
)
assert "authorization" in dict(r.headers)
r = routes.App.build_proxy_request("https://google.com")
assert "authorization" not in dict(r.headers)
abidlabs marked this conversation as resolved.
Show resolved Hide resolved
app = routes.App()
interface = gr.Interface(lambda x: x, "text", "text")
app.configure_app(interface)
with pytest.raises(ValueError):
app.build_proxy_request(
"https://gradio-tests-test-loading-examples-private.hf.space/file=Bunny.obj"
)
with pytest.raises(ValueError):
app.build_proxy_request("https://google.com")
abidlabs marked this conversation as resolved.
Show resolved Hide resolved


class TestApp:
Expand Down