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.

Sign up for GitHub

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

Do not send HF token to other domains via /proxy #4368

Merged
merged 5 commits into from
May 31, 2023
Merged

Do not send HF token to other domains via /proxy #4368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
45b5f11
test
abidlabs May 30, 2023
a5c5def
fix
abidlabs May 30, 2023
4ce3069
changelog
abidlabs May 30, 2023
6df296d
fix, add warnings
abidlabs May 31, 2023
013b670
Merge branch 'main' into proxy-token
abidlabs May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Make `Blocks.load` behave like other event listeners (allows chaining `then` off of it) [@anentropic](https://github.com/anentropic/) in [PR 4304](https://github.com/gradio-app/gradio/pull/4304)
- Respect `interactive=True` in output components of a `gr.Interface` by [@abidlabs](https://github.com/abidlabs) in [PR 4356](https://github.com/gradio-app/gradio/pull/4356).
- Remove unused frontend code by [@akx](https://github.com/akx) in [PR 4275](https://github.com/gradio-app/gradio/pull/4275)
- Do not send HF token to other domains via `/proxy` route by [@abidlabs](https://github.com/abidlabs) in [PR 4368](https://github.com/gradio-app/gradio/pull/4368).

## Other Changes:

Expand Down
2 changes: 1 addition & 1 deletion gradio/blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,7 +1448,7 @@ def load(
Parameters:
name: Class Method - the name of the model (e.g. "gpt2" or "facebook/bart-base") or space (e.g. "flax-community/spanish-gpt2"), can include the `src` as prefix (e.g. "models/facebook/bart-base")
src: Class Method - the source of the model: `models` or `spaces` (or leave empty if source is provided as a prefix in `name`)
api_key: Class Method - optional access token for loading private Hugging Face Hub models or spaces. Find your token here: https://huggingface.co/settings/tokens
api_key: Class Method - optional access token for loading private Hugging Face Hub models or spaces. Find your token here: https://huggingface.co/settings/tokens. Warning: only provide this if you are loading a trusted private Space as it can be read by the Space you are loading.
alias: Class Method - optional string used as the name of the loaded model instead of the default name (only applies if loading a Space running Gradio 2.x)
fn: Instance Method - the function to wrap an interface around. Often a machine learning model's prediction function. Each parameter of the function corresponds to one input component, and the function should return a single value or a tuple of values, with each element in the tuple corresponding to one output component.
inputs: Instance Method - List of gradio.components to use as inputs. If the function takes no inputs, this should be an empty list.
Expand Down
2 changes: 1 addition & 1 deletion gradio/external.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def load(
name: the name of the model (e.g. "gpt2" or "facebook/bart-base") or space (e.g. "flax-community/spanish-gpt2"), can include the `src` as prefix (e.g. "models/facebook/bart-base")
src: the source of the model: `models` or `spaces` (or leave empty if source is provided as a prefix in `name`)
api_key: Deprecated. Please use the `hf_token` parameter instead.
hf_token: optional access token for loading private Hugging Face Hub models or spaces. Find your token here: https://huggingface.co/settings/tokens
hf_token: optional access token for loading private Hugging Face Hub models or spaces. Find your token here: https://huggingface.co/settings/tokens. Warning: only provide this if you are loading a trusted private Space as it can be read by the Space you are loading.
alias: optional string used as the name of the loaded model instead of the default name (only applies if loading a Space running Gradio 2.x)
Returns:
a Gradio Blocks object for the given model
Expand Down
2 changes: 1 addition & 1 deletion gradio/interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def load(
Parameters:
name: the name of the model (e.g. "gpt2" or "facebook/bart-base") or space (e.g. "flax-community/spanish-gpt2"), can include the `src` as prefix (e.g. "models/facebook/bart-base")
src: the source of the model: `models` or `spaces` (or leave empty if source is provided as a prefix in `name`)
api_key: optional access token for loading private Hugging Face Hub models or spaces. Find your token here: https://huggingface.co/settings/tokens
api_key: optional access token for loading private Hugging Face Hub models or spaces. Find your token here: https://huggingface.co/settings/tokens. Warning: only provide this if you are loading a trusted private Space as it can be read by the Space you are loading.
alias: optional string used as the name of the loaded model instead of the default name (only applies if loading a Space running Gradio 2.x)
Returns:
a Gradio Interface object for the given model
Expand Down
16 changes: 11 additions & 5 deletions gradio/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,16 @@ def get_blocks(self) -> gradio.Blocks:
raise ValueError("No Blocks has been configured for this app.")
return self.blocks

@staticmethod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could just as well not be a @staticmethod so it's easy to override in a subclass..?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what scenario would this be overridden? This should be a static method as it has no reference to the object, it's just here for organizational purposes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To have an application be able to customize a forwarded request without having to copy-paste as much code.

Copy link
Member Author

@abidlabs abidlabs May 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I haven't really seen the App class subclassed. Is it subclassed in Automatic1111 webui? If you have a concrete use case, feel free to share, otherwise I'll keep as is

def build_proxy_request(url_path):
url = httpx.URL(url_path)
is_hf_url = url.host.endswith(".hf.space")
headers = {}
if Context.hf_token is not None and is_hf_url:
headers["Authorization"] = f"Bearer {Context.hf_token}"
rp_req = client.build_request("GET", url, headers=headers)
return rp_req

@staticmethod
def create_app(
blocks: gradio.Blocks, app_kwargs: Dict[str, Any] | None = None
Expand Down Expand Up @@ -300,11 +310,7 @@ 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
url = httpx.URL(url_path)
headers = {}
if Context.hf_token is not None:
headers["Authorization"] = f"Bearer {Context.hf_token}"
rp_req = client.build_request("GET", url, headers=headers)
rp_req = app.build_proxy_request(url_path)
rp_resp = await client.send(rp_req, stream=True)
return StreamingResponse(
rp_resp.aiter_raw(),
Expand Down
2 changes: 1 addition & 1 deletion test/test_external.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def test_multiple_spaces_one_private(self):
def test_loading_files_via_proxy_works(self):
api_key = "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" # Intentionally revealing this key for testing purposes
io = gr.load(
"spaces/gradio-tests/test-loading-examples-private", api_key=api_key
"spaces/gradio-tests/test-loading-examples-private", hf_token=api_key
)
assert io.theme.name == "default"
app, _, _ = io.launch(prevent_thread_lock=True)
Expand Down
9 changes: 9 additions & 0 deletions test/test_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,15 @@ def test_mount_gradio_app_raises_error_if_event_queued_but_queue_disabled(self):

demo.close()

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)


class TestApp:
def test_create_app(self):
Expand Down