Open juliusmarminge opened 2 weeks ago
vercel[bot]
commented
2 weeks ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| docs-uploadthing | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Sep 18, 2024 6:17pm |
changeset-bot[bot]
commented
2 weeks ago Latest commit: b281a5c7d9c7d2c09c6b57658556e012ddd46535
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
coderabbitai[bot]
commented
2 weeks ago The changes primarily involve the simplification of the file key generation process. The previous implementation utilized the Sqids library for encoding and randomization, while the new approach directly concatenates the appId with a user-defined file seed. The updated requirements for the file seed ensure it is unique, URL-safe, and at least 36 characters long, streamlining the overall key generation without the need for additional complexity.
| File Path | Change Summary |
|---|---|
| docs/src/app/(docs)/uploading-files/page.mdx, packages/shared/src/crypto.ts | The generateKey function was modified to simplify the key generation process by removing the encoding of appId and directly concatenating it with the encodedFileSeed. The new implementation eliminates the use of the Sqids library and focuses on ensuring the file seed is URL-safe. The verifyKey function was also removed, affecting key verification. |
In the meadow where the keys align,
A rabbit hops, feeling just fine.
With seeds so safe and bright,
The keys now shine, a pure delight!
No more shuffle, just a simple way,
To share our files and play all day! 🐇✨
packages/shared/src/crypto.ts (1)
`112-112`: **Carefully assess the impact of the change in key generation format.** The change in the `generateKey` function, which now directly concatenates `appId` and `encodedFileSeed` without encoding `appId`, could have significant implications: 1. Compatibility issues: The change in key format could lead to compatibility issues with existing systems or databases that rely on the previous format. Ensure that this change is thoroughly tested and validated against all dependent systems. 2. Security and integrity: The removal of the `verifyKey` function indicates that there is no longer a mechanism to validate the relationship between the key and the `appId`. This could have implications for security and integrity checks in the system. Consider implementing an alternative verification mechanism if necessary. 3. Key uniqueness and security: Using the raw `appId` instead of the encoded version may affect the uniqueness and security properties of the generated keys. Assess whether the new key format maintains the required level of uniqueness and security. Consider adding tests to validate these properties. To ensure system stability and compatibility, please: - Review and update any key format validation logic in the codebase. - Modify existing tests to account for the new key format. - Add new tests to verify the compatibility of the new key format with all dependent systems. - Perform integration testing to ensure that URL generation and API calls work correctly with the new key format. Run the following script to verify the usage of the `generateKey` function and its return value across the codebase:Verification successful
--- **Verification confirms removal of `verifyKey` function and potential compatibility.** Based on the verification results: 1. The `verifyKey` function has indeed been removed from the codebase, confirming the change mentioned in the original review comment. 2. No direct key format validations or comparisons were found, suggesting that the system may not rely on a specific key format for validation. This could reduce the impact of the format change. 3. Key-related types and interfaces (found in `packages/uploadthing/src/sdk/types.ts`) appear to be focused on file operations rather than the internal structure of the key itself. This suggests that the change in key format may not significantly impact these types. 4. The `generateKey` function is used in test files and the SDK, but its usage doesn't seem to depend on the internal structure of the key. Given these findings: - The removal of the `verifyKey` function and the change in key generation format appear to be intentional changes that don't severely impact the existing codebase. - The lack of key format validations and the nature of key-related types suggest that the system may be more resilient to the key format change than initially thought. However, to ensure system stability: 1. Update any remaining tests that still use the `verifyKey` function. 2. Review the `generateSignedURL` function (which wasn't found in our search) to ensure it's compatible with the new key format. 3. Perform thorough integration testing, especially for API calls using the `utapi` object, to verify that the new key format works as expected in all scenarios.Scripts executed
The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the usage of the `generateKey` function and its return value. # Test 1: Search for the function usage and its return value usage rg --type ts -A 10 'const key = .*generateKey' # Test 2: Search for any key format validations or comparisons rg --type ts 'key\.match|key\.startsWith|key\.endsWith|key\.includes' # Test 3: Look for any database operations or API calls using the key rg --type ts 'db\..*\(.*key|api\..*\(.*key' ``` Length of output: 6681 --- Script: ```shell #!/bin/bash # Description: Further investigation of key-related functions and types # Check for the current state of the verifyKey function rg --type ts 'function verifyKey' # Look for key-related type definitions or interfaces rg --type ts 'type.*Key|interface.*Key' # Investigate the generateSignedURL function rg --type ts -A 10 'function generateSignedURL' # Check for configuration files or environment variables related to key generation rg --type ts 'process\.env\..*KEY|config\..*key' ``` Length of output: 1717
pkg-pr-new[bot]
commented
2 weeks ago - [@example/minimal-appdir](https://pkg.pr.new/template/b1fcada7-f0e9-4365-abe3-37f4ac92c567) - [@example/minimal-astro-react](https://pkg.pr.new/template/1c74378e-7a49-42dd-bde4-ed5009826662) - [@example/minimal-expo](https://pkg.pr.new/template/14940cf1-601d-4a2a-b077-caa17e214b04) - [@example/minimal-nuxt](https://pkg.pr.new/template/efc4029f-2393-4247-853b-8bec323c4b19) - [@example/minimal-pagedir](https://pkg.pr.new/template/2f97fe8a-ef86-4320-b762-e4010c63c673) - [@example/minimal-solidstart](https://pkg.pr.new/template/9230d1c3-befd-4706-84a5-e2a118393031) - [@example/minimal-sveltekit](https://pkg.pr.new/template/29e19a7d-c9ea-4916-98de-72c2952a5142)
pnpm add https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/shared@960commit: b281a5c
github-actions[bot]
commented
2 weeks ago | Bundle | Size (gzip) | Visualization |
|---|---|---|
| Main | 26.03KB | No treemap on forks |
| PR (e3e14f676b8b62d4cc995e07136378917be02517) | 26.03KB | No treemap on forks |
| Diff | No change |
We decided it was too hard to port the encoding to other languages so we now do a prefix check on appId in plain text instead of the encoded value.
ref: https://github.com/pingdotgg/uploadthing-infra/pull/550
Summary by CodeRabbit
New Features
Bug Fixes
verifyKeyfunction, streamlining key management and enhancing security.