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

Jump to bottom

Upload files from a URL #3386

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Upload files from a URL #3386

wants to merge 2 commits into from

Conversation

Floppy
Copy link
Collaborator

@Floppy Floppy commented Jan 13, 2025 •
edited
Loading

WIP to resolve #2376

Implements the url_meta Uppy companion API, but still needs the url_get. See #2376 for background.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 13, 2025
View reviewed changes
app/controllers/uppy_companion_controller.rb
@@ -0,0 +1,21 @@
class UppyCompanionController < ApplicationController
skip_forgery_protection

Check failure

Code scanning / CodeQL

CSRF protection weakened or disabled High

Potential CSRF vulnerability due to forgery protection being disabled or weakened.

Copilot Autofix AI 11 days ago

To fix the problem, we need to re-enable CSRF protection for the UppyCompanionController by removing the skip_forgery_protection call. This will ensure that all actions within the controller require a valid CSRF token, mitigating the risk of CSRF attacks.

  1. Remove the skip_forgery_protection call on line 2.
  2. Ensure that the protect_from_forgery method is used with the with: :exception option to raise an exception on an invalid CSRF token.
Suggested changeset 1
app/controllers/uppy_companion_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/uppy_companion_controller.rb b/app/controllers/uppy_companion_controller.rb
--- a/app/controllers/uppy_companion_controller.rb
+++ b/app/controllers/uppy_companion_controller.rb
@@ -1,3 +1,3 @@
 class UppyCompanionController < ApplicationController
-  skip_forgery_protection
+  protect_from_forgery with: :exception
 
EOF
@@ -1,3 +1,3 @@
class UppyCompanionController < ApplicationController
skip_forgery_protection
protect_from_forgery with: :exception

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
app/controllers/uppy_companion_controller.rb
def url_meta
authorize :model, :new?
# Do HEAD request for url
response = Faraday.head params[:url]

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The URL of this request depends on a
user-provided value
Loading
.

Copilot Autofix AI 11 days ago

To fix the problem, we need to ensure that the user-provided URL is validated against a list of authorized URLs or restricted to a particular host or URL prefix. This can be achieved by maintaining a list of allowed hosts and checking if the user-provided URL belongs to one of these hosts before making the request.

  1. Create a list of allowed hosts.
  2. Parse the user-provided URL and check if its host is in the list of allowed hosts.
  3. If the host is not allowed, return an error response.
  4. If the host is allowed, proceed with the request.
Suggested changeset 1
app/controllers/uppy_companion_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/uppy_companion_controller.rb b/app/controllers/uppy_companion_controller.rb
--- a/app/controllers/uppy_companion_controller.rb
+++ b/app/controllers/uppy_companion_controller.rb
@@ -1 +1,3 @@
+require 'uri'
+
 class UppyCompanionController < ApplicationController
@@ -5,10 +7,16 @@
     authorize :model, :new?
-    # Do HEAD request for url
-    response = Faraday.head params[:url]
-    render json: {
-      name: response.headers["filename"],
-      type: response.headers["content-type"],
-      size: response.headers["content-length"],
-      status_code: response.status
-    }
+    url = params[:url]
+    uri = URI.parse(url)
+    if ALLOWED_HOSTS.include?(uri.host)
+      # Do HEAD request for url
+      response = Faraday.head url
+      render json: {
+        name: response.headers["filename"],
+        type: response.headers["content-type"],
+        size: response.headers["content-length"],
+        status_code: response.status
+      }
+    else
+      render status: :bad_request, json: { error: "Invalid URL" }
+    end
   end
@@ -17,4 +25,10 @@
     authorize :model, :new?
-    Faraday.get params[:url]
-    render status: :internal_server_error, json: {}
+    url = params[:url]
+    uri = URI.parse(url)
+    if ALLOWED_HOSTS.include?(uri.host)
+      Faraday.get url
+      render status: :ok, json: {}
+    else
+      render status: :bad_request, json: { error: "Invalid URL" }
+    end
   end
EOF
@@ -1 +1,3 @@
require 'uri'

class UppyCompanionController < ApplicationController
@@ -5,10 +7,16 @@
authorize :model, :new?
# Do HEAD request for url
response = Faraday.head params[:url]
render json: {
name: response.headers["filename"],
type: response.headers["content-type"],
size: response.headers["content-length"],
status_code: response.status
}
url = params[:url]
uri = URI.parse(url)
if ALLOWED_HOSTS.include?(uri.host)
# Do HEAD request for url
response = Faraday.head url
render json: {
name: response.headers["filename"],
type: response.headers["content-type"],
size: response.headers["content-length"],
status_code: response.status
}
else
render status: :bad_request, json: { error: "Invalid URL" }
end
end
@@ -17,4 +25,10 @@
authorize :model, :new?
Faraday.get params[:url]
render status: :internal_server_error, json: {}
url = params[:url]
uri = URI.parse(url)
if ALLOWED_HOSTS.include?(uri.host)
Faraday.get url
render status: :ok, json: {}
else
render status: :bad_request, json: { error: "Invalid URL" }
end
end
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
app/controllers/uppy_companion_controller.rb

def url_get
authorize :model, :new?
Faraday.get params[:url]

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The URL of this request depends on a
user-provided value
Loading
.

Copilot Autofix AI 11 days ago

To fix the problem, we need to ensure that the user-provided URL is validated against a list of authorized URLs or restricted to a particular host or URL prefix. This can be achieved by maintaining a list of allowed domains and checking if the user-provided URL belongs to one of these domains before making the request.

We will:

  1. Define a list of allowed domains.
  2. Parse the user-provided URL and check if its host is in the list of allowed domains.
  3. Only proceed with the request if the URL is valid.
Suggested changeset 1
app/controllers/uppy_companion_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/uppy_companion_controller.rb b/app/controllers/uppy_companion_controller.rb
--- a/app/controllers/uppy_companion_controller.rb
+++ b/app/controllers/uppy_companion_controller.rb
@@ -1 +1,3 @@
+require 'uri'
+
 class UppyCompanionController < ApplicationController
@@ -5,10 +7,16 @@
     authorize :model, :new?
-    # Do HEAD request for url
-    response = Faraday.head params[:url]
-    render json: {
-      name: response.headers["filename"],
-      type: response.headers["content-type"],
-      size: response.headers["content-length"],
-      status_code: response.status
-    }
+    url = params[:url]
+    uri = URI.parse(url)
+    if ALLOWED_DOMAINS.include?(uri.host)
+      # Do HEAD request for url
+      response = Faraday.head url
+      render json: {
+        name: response.headers["filename"],
+        type: response.headers["content-type"],
+        size: response.headers["content-length"],
+        status_code: response.status
+      }
+    else
+      render status: :bad_request, json: { error: "Invalid URL" }
+    end
   end
@@ -17,4 +25,10 @@
     authorize :model, :new?
-    Faraday.get params[:url]
-    render status: :internal_server_error, json: {}
+    url = params[:url]
+    uri = URI.parse(url)
+    if ALLOWED_DOMAINS.include?(uri.host)
+      Faraday.get url
+      render status: :ok, json: {}
+    else
+      render status: :bad_request, json: { error: "Invalid URL" }
+    end
   end
EOF
@@ -1 +1,3 @@
require 'uri'

class UppyCompanionController < ApplicationController
@@ -5,10 +7,16 @@
authorize :model, :new?
# Do HEAD request for url
response = Faraday.head params[:url]
render json: {
name: response.headers["filename"],
type: response.headers["content-type"],
size: response.headers["content-length"],
status_code: response.status
}
url = params[:url]
uri = URI.parse(url)
if ALLOWED_DOMAINS.include?(uri.host)
# Do HEAD request for url
response = Faraday.head url
render json: {
name: response.headers["filename"],
type: response.headers["content-type"],
size: response.headers["content-length"],
status_code: response.status
}
else
render status: :bad_request, json: { error: "Invalid URL" }
end
end
@@ -17,4 +25,10 @@
authorize :model, :new?
Faraday.get params[:url]
render status: :internal_server_error, json: {}
url = params[:url]
uri = URI.parse(url)
if ALLOWED_DOMAINS.include?(uri.host)
Faraday.get url
render status: :ok, json: {}
else
render status: :bad_request, json: { error: "Invalid URL" }
end
end
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@Floppy Floppy added the feature User-facing features and product enhancements label Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature User-facing features and product enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upload from URL
1 participant
@Floppy