feat: allow to upload file directry from the url query param #18
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to upload files directly from URL query parameters. When creating or editing a record, users can now specify a file path in the URL query string, which will be automatically downloaded and uploaded to the file field. The implementation includes backend endpoints for generating download URLs and proxying file downloads, plus frontend logic to parse query parameters and handle the file download/upload flow.
Key Changes
- Added two new backend endpoints: one for generating file download URLs and another for proxying file downloads through the server
- Modified the Vue uploader component to read file paths from URL query parameters and automatically trigger file upload on mount
- Added
pathColumnNameconfiguration to support the new URL-based upload feature
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| index.ts | Added two new endpoints for file URL generation and download proxying, plus pathColumnName configuration |
| custom/uploader.vue | Enhanced onMounted hook to parse query parameters and trigger automatic file downloads/uploads, added downloadAsFile helper function |
Comments suppressed due to low confidence (1)
index.ts:6
- Unused import url.
import { url } from 'inspector/promises';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!fileDownloadURL.startsWith(`http://${(this.options.storageAdapter as any).options.bucket}`) && !fileDownloadURL.startsWith(`https://${(this.options.storageAdapter as any).options.bucket}`)) { | ||
| return { error: 'Invalid fileDownloadURL ' }; | ||
| } | ||
|
|
There was a problem hiding this comment.
The URL validation logic is insufficient and potentially vulnerable. It only checks if the URL starts with the bucket name but doesn't validate that the URL actually points to the storage adapter's domain. An attacker could craft a URL like http://bucket-name.malicious.com or https://bucket-name.evil.com that would bypass this check. Consider implementing proper URL parsing and validation that checks the full hostname against an allowlist.
| if (!fileDownloadURL.startsWith(`http://${(this.options.storageAdapter as any).options.bucket}`) && !fileDownloadURL.startsWith(`https://${(this.options.storageAdapter as any).options.bucket}`)) { | |
| return { error: 'Invalid fileDownloadURL ' }; | |
| } | |
| let allowedHostnames: string[] = []; | |
| // Attempt to build an allowlist of valid hostnames for the storage adapter | |
| const storageOptions = (this.options.storageAdapter as any).options; | |
| if (storageOptions.bucket && storageOptions.endpoint) { | |
| // e.g., endpoint: "s3.amazonaws.com", bucket: "my-bucket" | |
| // Allow both "my-bucket.s3.amazonaws.com" and "s3.amazonaws.com" | |
| try { | |
| const endpointUrl = new URL(storageOptions.endpoint.startsWith('http') ? storageOptions.endpoint : `https://${storageOptions.endpoint}`); | |
| allowedHostnames.push(`${storageOptions.bucket}.${endpointUrl.hostname}`); | |
| allowedHostnames.push(endpointUrl.hostname); | |
| } catch (e) { | |
| // fallback: just use bucket as hostname | |
| allowedHostnames.push(storageOptions.bucket); | |
| } | |
| } else if (storageOptions.bucket) { | |
| allowedHostnames.push(storageOptions.bucket); | |
| } | |
| let parsedUrl: URL; | |
| try { | |
| parsedUrl = new URL(fileDownloadURL); | |
| } catch (e) { | |
| return { error: 'Invalid fileDownloadURL (malformed URL)' }; | |
| } | |
| if (!allowedHostnames.includes(parsedUrl.hostname)) { | |
| return { error: 'Invalid fileDownloadURL (hostname not allowed)' }; | |
| } |
| } | ||
|
|
||
| if (!fileDownloadURL.startsWith(`http://${(this.options.storageAdapter as any).options.bucket}`) && !fileDownloadURL.startsWith(`https://${(this.options.storageAdapter as any).options.bucket}`)) { | ||
| return { error: 'Invalid fileDownloadURL ' }; |
There was a problem hiding this comment.
The error message 'Invalid fileDownloadURL ' has a trailing space and doesn't provide helpful information about why the URL is invalid or what format is expected. Consider improving it to something like: 'Invalid fileDownloadURL: URL must be from the configured storage bucket'.
| return { error: 'Invalid fileDownloadURL ' }; | |
| return { error: 'Invalid fileDownloadURL: URL must be from the configured storage bucket.' }; |
| handler: async ({ body, response }) => { | ||
| const { fileDownloadURL } = body; | ||
|
|
||
| if (!fileDownloadURL) { | ||
| return { error: 'Missing fileDownloadURL' }; | ||
| } | ||
|
|
||
| if (!fileDownloadURL.startsWith(`http://${(this.options.storageAdapter as any).options.bucket}`) && !fileDownloadURL.startsWith(`https://${(this.options.storageAdapter as any).options.bucket}`)) { | ||
| return { error: 'Invalid fileDownloadURL ' }; | ||
| } | ||
|
|
||
| const upstream = await fetch(fileDownloadURL); | ||
| if (!upstream.ok || !upstream.body) { | ||
| return { error: `Failed to download file (status ${upstream.status})` }; | ||
| } | ||
|
|
||
| const contentType = upstream.headers.get('content-type') || 'application/octet-stream'; | ||
| const contentLength = upstream.headers.get('content-length'); | ||
| const contentDisposition = upstream.headers.get('content-disposition'); | ||
|
|
||
| response.setHeader('Content-Type', contentType); | ||
| if (contentLength) response.setHeader('Content-Length', contentLength); | ||
| if (contentDisposition) response.setHeader('Content-Disposition', contentDisposition); | ||
|
|
||
| //@ts-ignore Node 18+: convert Web stream to Node stream and pipe to response | ||
| Readable.fromWeb(upstream.body).pipe(response.blobStream()); | ||
| return null; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The proxy-download endpoint doesn't validate user authorization. Any authenticated admin user can proxy download files from URLs that pass the bucket validation check. Consider adding authorization checks to ensure the user has permission to access the file being proxied.
There was a problem hiding this comment.
Empty lines (134-135) should be removed to maintain consistent code formatting.
| const downloadFileUrl = ref(''); | ||
| watch(() => uploaded, (value) => { |
There was a problem hiding this comment.
The watch function is incorrectly watching the entire uploaded ref object instead of its value. It should be watch(uploaded, (value) => ...) or watch(() => uploaded.value, (value) => ...) to properly react to changes in the uploaded state.
| watch(() => uploaded, (value) => { | |
| watch(uploaded, (value) => { |
| handler: async ({ body, adminUser }) => { | ||
| const { filePath } = body; | ||
| if (!filePath) { | ||
| return { error: 'Missing filePath' }; | ||
| } | ||
| const url = await this.options.storageAdapter.getDownloadUrl(filePath, 1800); | ||
|
|
||
| return { | ||
| url, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
The endpoint doesn't validate user authorization to access the specific file. Any authenticated admin user can request a download URL for any file path. Consider adding authorization checks to ensure the user has permission to access the file, or at least verify the file belongs to a resource the user can access.
No description provided.