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
Merged

Restrict proxy URLs #4406

merged 19 commits into from
Jun 5, 2023

Conversation

abidlabs
Copy link
Member

@abidlabs abidlabs commented Jun 2, 2023

Restricts proxy URLs to those that a user has loaded through gr.load().

Solves two issues:

(1) Prevents users from carrying out SSRF (using other people's Spaces to make requests through the /proxy route)
(2) Prevents harvesting of a Space's HF tokens

cc @akx

@gradio-pr-bot
Copy link
Collaborator

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-4406-all-demos

@abidlabs abidlabs marked this pull request as draft June 2, 2023 17:16
gradio/blocks.py Show resolved Hide resolved
gradio/routes.py Outdated Show resolved Hide resolved
gradio/routes.py Outdated Show resolved Hide resolved
gradio/routes.py Outdated Show resolved Hide resolved
abidlabs and others added 5 commits June 2, 2023 11:31
Co-authored-by: Aarni Koskela <akx@iki.fi>
Co-authored-by: Aarni Koskela <akx@iki.fi>
gradio/routes.py Outdated Show resolved Hide resolved
@abidlabs
Copy link
Member Author

abidlabs commented Jun 2, 2023

Suggestions should be addressed now @akx, please feel free to re-review!

@abidlabs abidlabs marked this pull request as ready for review June 2, 2023 19:16
test/test_routes.py Outdated Show resolved Hide resolved
test/test_routes.py Outdated Show resolved Hide resolved
gradio/blocks.py Outdated Show resolved Hide resolved
gradio/blocks.py Outdated Show resolved Hide resolved
gradio/blocks.py Show resolved Hide resolved
abidlabs and others added 6 commits June 2, 2023 12:48
Co-authored-by: Aarni Koskela <akx@iki.fi>
Co-authored-by: Aarni Koskela <akx@iki.fi>
@abidlabs
Copy link
Member Author

abidlabs commented Jun 4, 2023

Tests have been updated -- @freddyaboulton @dawoodkhan82 or @aliabid94 would you be able to give this a review so that we can merge in?

Copy link
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thanks @abidlabs and @akx !

The changes look good to me! It looks like we're constructing the URLs incorrectly on main so we can fix that in another PR.

image

@abidlabs
Copy link
Member Author

abidlabs commented Jun 5, 2023

The changes look good to me! It looks like we're constructing the URLs incorrectly on main so we can fix that in another PR.

Hmm interesting, let me see what's happening there

@abidlabs
Copy link
Member Author

abidlabs commented Jun 5, 2023

Ok let me merge this in and then I'll look at the file path issue in a separate PR. Thanks again for the reviews @akx and @freddyaboulton!

@abidlabs abidlabs merged commit 3796761 into main Jun 5, 2023
@abidlabs abidlabs deleted the further-restrict-proxy branch June 5, 2023 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants