feat: Recoverable keys feature for the dashboard #2110
Conversation
harshsbhat
requested review from
perkinsjr,
chronark and
mcstepp
as code owners
|
|
@harshsbhat is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new feature in the API key creation process, allowing users to set a Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@harshsbhat this currently has a frozen lock file. Preventing it from building.
Just go ahead and delete the pnpm-lock.yaml and run pnpm i and then commit the lock file. |
Done! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
internal/vault/package.json (1)
1-15: LGTM! Thepackage.jsonfile is well-structured and properly configured.The file correctly sets up the essential metadata for the
@unkey/vaultpackage, including the name, version, main entry point, and type definitions. The use of TypeScript and the inclusion of necessary dev dependencies are appropriate for the package's development.Consider adding the following enhancements to further improve the package:
- Add a
scriptssection to define common tasks like building, testing, and linting the package. For example:"scripts": { "build": "tsc", "test": "jest", "lint": "eslint src/**/*.ts" }
- Include a
README.mdfile in the package's root directory to provide documentation and usage instructions for the package. This will help users understand how to integrate and utilize the@unkey/vaultpackage effectively.apps/dashboard/lib/trpc/routers/key/create.ts (1)
1-1: Remove unused import.The
Apiimport from@/app/(app)/settings/root-keys/[keyId]/permissions/apiis not used in the changed code. Please remove it to keep the code clean.Apply this diff to remove the unused import:
-import { Api } from "@/app/(app)/settings/root-keys/[keyId]/permissions/api"; import { db, schema } from "@/lib/db";
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (5)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (5 hunks)
- apps/dashboard/lib/trpc/routers/key/create.ts (3 hunks)
- apps/dashboard/package.json (1 hunks)
- internal/vault/package.json (1 hunks)
- internal/vault/src/index.ts (1 hunks)
Additional comments not posted (13)
internal/vault/src/index.ts (5)
1-27: LGTM!The type definitions for encryption and decryption requests and responses are well-structured and follow a consistent naming convention. The separation of single and bulk operations enhances clarity and usability.
29-31: LGTM!The
RequestContexttype provides a structured way to include a uniquerequestIdwith each request, which can be useful for logging, tracing, and debugging purposes.
33-59: LGTM!The
Vaultclass and itsencryptmethod are well-implemented. The class encapsulates the encryption functionality and provides a clean interface for other modules to use. Theencryptmethod follows a standard pattern for making an HTTP request and includes appropriate error handling.
61-81: LGTM!The
encryptBulkmethod of theVaultclass is well-implemented and follows a similar pattern to theencryptmethod. It handles bulk encryption requests and includes appropriate error handling.
83-100: LGTM!The
decryptmethod of theVaultclass is well-implemented and follows a similar pattern to theencryptandencryptBulkmethods. It handles decryption requests and includes appropriate error handling.apps/dashboard/package.json (1)
56-56: LGTM!The addition of the
@unkey/vaultdependency aligns with the stated PR objectives and the feature implementation for recoverable keys. It will facilitate the necessary interaction between the dashboard and the vault.apps/dashboard/lib/trpc/routers/key/create.ts (2)
21-21: LGTM!The addition of the optional
recoverEnabledparameter to thecreateKeyprocedure is a good way to introduce the new recoverable keys feature without breaking existing functionality.
106-121: Excellent work on the key encryption and storage implementation!The conditional block for handling key encryption and storage when
recoverEnabledis true andstoreEncryptedKeysis enabled in the key auth is well-structured and follows best practices:
- Using environment variables for the vault configuration keeps sensitive information secure.
- Generating a unique request ID for each encryption operation aids in auditing and debugging.
- Storing the encrypted key and its metadata in a separate
encryptedKeystable ensures secure and efficient key recovery when needed.Great job on this implementation!
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (5)
81-81: LGTM!The addition of the
recoverEnabledproperty to the form schema is a good way to introduce the key recoverability feature. Setting the default value tofalseensures that keys are not recoverable by default, which is a good security practice.
167-167: LGTM!Adding the
recoverEnabledproperty to the form's default values with a value offalseis consistent with the addition of this property to the form schema. It ensures that the form's initial state hasrecoverEnabledset tofalse.
208-208: LGTM!Including the
recoverEnabledproperty in the object passed tokey.mutateAsyncwhen the form is submitted ensures that the value ofrecoverEnabledis passed to the server when creating a new key. This change is consistent with the addition of therecoverEnabledproperty to the form schema and default values.
297-297: LGTM!Resetting the
recoverEnabledproperty tofalsewhen the user clicks the "Create another key" button ensures that therecoverEnabledproperty is reset to its default value when creating a new key. This change is consistent with the addition of therecoverEnabledproperty to the form schema and default values.
812-859: LGTM!The addition of the key recoverability card component is consistent with the introduction of the key recoverability feature. The toggle switch allows users to easily enable/disable key recoverability. The message displayed when key recoverability is enabled is important to inform users about the security implications of this feature. Including a link to the documentation is helpful for users who want to learn more about key recoverability.
After I push the pnpm lock file for some reason it shows 17000+ lines changed. Is that supposed to look like that? |
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx
Outdated
Show resolved
Hide resolved
* Discord dark mode and fixing the link * Requested changes * Requested changes * Lint * Adding support to sidebar * remove unused imports * change the docs icon to book and remove repeating code * Consistent size * Making sure the Feedback is clickable, the feedback can be entered using the arrows and enter key on command * Revert "Making sure the Feedback is clickable, the feedback can be entered using the arrows and enter key on command" This reverts commit dc6183f. * pnpm-lock file * Fix --------- Co-authored-by: Andreas Thomas <[email protected]>
* chore(email): update styling, copy, urls * update urls * fix class name * remove unused imports, update date previews * [autofix.ci] apply automated fixes * make James the signer of all emails * add biome ignore to prevent it from breaking an import * add biome ignore to other component * Update copy and links to use Dub * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: James Perkins <[email protected]> Co-authored-by: James P <[email protected]>
* chore: Adds a five minute delay to emails Adds a five minute delay to our Welcome email * pnpm lock update * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* Update to dot com We has dot com, we should use it. * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* chore: update legacy_keys_verifyKey.ts identifiy -> identify * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* added README.md in /packages/api * added README.md in the files config in package.json
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix: default ratelimits * revert * fix: cache ratelimits on cloudflare correctly * chore: remove logs * chore: remove log * perf: remove unnecessary switch * fix: track isolate start time * test: tighten lower ratelimit threshold * fix: only cache ratelimit blocks * chore: sync lockfile * test: improve accuracy of lower limit calculation in rate limit tests * fix: address rabbit suggestions
harshsbhat
force-pushed
the
harshbhat/recoverable-keys
branch
from
5802f2c to
3a5bf01
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post
Actionable comments posted: 23
🧹 Outside diff range and nitpick comments (44)
internal/resend/src/components/signature.tsx (2)
1-3: LGTM! Consider using named import for React.The imports look good and are appropriate for the component. However, you might want to consider using a named import for React to align with modern practices:
-// biome-ignore lint/style/useImportType: not just the type -import React from "react"; +import * as React from "react";This change would eliminate the need for the linting ignore comment and align with TypeScript's preference for named imports.
9-15: LGTM! Consider using React.VFC for stricter typing.The
Signaturecomponent is well-implemented and follows React best practices. It correctly uses theSignaturePropsinterface and renders the content as expected.A minor suggestion for improvement:
-export const Signature: React.FC<SignatureProps> = ({ signedBy }) => ( +export const Signature: React.VFC<SignatureProps> = ({ signedBy }) => (Using
React.VFC(VoidFunctionComponent) instead ofReact.FCprovides stricter typing by not implicitly including thechildrenprop. This change aligns better with your component's props definition, which doesn't include children.internal/resend/src/components/layout.tsx (1)
17-37: LGTM: Well-structured Layout component with a minor suggestion.The Layout component is well-designed and follows best practices for email layouts. It effectively uses @react-email components and Tailwind classes for styling. The inclusion of a social media section is a good touch for user engagement.
Consider extracting the social media links into a separate component or configuration object. This would improve maintainability and make it easier to update the links in the future. For example:
const socialLinks = [ { name: 'X (formerly Twitter)', url: 'https://x.com/unkeydev' }, { name: 'Discord', url: 'https://unkey.link/lcMby11' }, { name: 'GitHub', url: 'https://unkey.link/cfe4hcA' }, ]; // Then in the component: {socialLinks.map((link, index) => ( <React.Fragment key={link.name}> {index > 0 && ' | '} <Link href={link.url}>{link.name}</Link> </React.Fragment> ))}This approach would make the component more flexible and easier to maintain.
internal/resend/tailwind.config.ts (1)
58-72: LGTM: Comprehensive gray palette added.The addition of a custom gray color palette is a great improvement. It provides a wide range of shades that can be used consistently across the application. The progression from light to dark is logical and includes an extra dark 950 shade, which is useful for high-contrast designs or dark mode implementations.
Consider adding a comment above the
colorsobject to explain the purpose of this custom palette and how it should be used in the project. This can help other developers understand the design system better. For example:// Custom gray palette for consistent use across the application. // Use these colors instead of default gray shades for better design consistency. colors: { // ... (rest of the colors object) }packages/api/README.md (4)
11-15: Consider adding installation instructions for other package managersWhile the npm installation instructions are clear, consider adding instructions for other popular package managers like yarn or pnpm. This would make the README more inclusive for users with different preferences or project setups.
Example addition:
## Installation npm: ```bash npm install @unkey/apiyarn:
yarn add @unkey/apipnpm:
pnpm add @unkey/api--- `17-29`: **Enhance the quickstart section with more context** The quickstart guide is informative and includes crucial security advice. To make it even more helpful for new users: 1. Provide a brief explanation of what a root key is and its importance. 2. Include a link or more specific instructions on where to find the settings for creating a new Unkey Root Key. Example addition: ```markdown ## Quickstart 1. Create a new Unkey Root Key in the [settings](https://unkey.dev/app/settings). A root key is a secure credential that grants access to manage your Unkey account programmatically. 2. Use the root key to initialize the client: ... **Important:** Always keep your root key safe and reset it if you suspect it has been compromised. The root key provides full access to your Unkey account, so it should be treated with the utmost security.
30-53: Consider adding more usage examplesThe current usage section provides a clear example of verifying a key, which is helpful. To make the README more comprehensive, consider adding examples of other common operations that users might perform with the SDK. This could include:
- Creating a new key
- Revoking a key
- Updating key permissions
- Listing keys
Example addition:
### Creating a New Key ```ts const { result, error } = await unkey.keys.create({ apiId: "api_123", prefix: "prefix", byteLength: 16, ownerId: "user_123", }); if (error) { console.error(error.message); return; } console.log("New key created:", result.key);(Add more examples as needed)
This will give users a broader understanding of the SDK's capabilities right from the README. --- `78-118`: **Add information about telemetry data collection** The configuration options section is comprehensive and well-explained. To improve transparency and user trust, consider adding a brief explanation of what telemetry data is collected when not disabled. This information helps users make an informed decision about whether to opt out. Example addition: ```markdown ### Disable Telemetry By default, we collect anonymous usage data to help improve our services. This data includes [list specific data points collected]. To opt out of anonymous telemetry data collection: ```ts const unkey = new Unkey({ rootKey: "<UNKEY_ROOT_KEY>", disableTelemetry: true, });For more information about our data collection practices, please see our Privacy Policy.
This addition provides users with more context about the telemetry feature and directs them to more detailed information if needed. </blockquote></details> <details> <summary>apps/dashboard/CHANGELOG.md (1)</summary><blockquote> `3-7`: **LGTM! Consider adding more details to the changelog entry.** The new version entry and dependency update are correctly formatted and placed. Good job on keeping the changelog up-to-date. Consider adding a brief description of the changes or improvements introduced by the @unkey/ratelimit update to version 0.4.5. This would provide more context for users and developers reading the changelog. For example: ```markdown ## 0.1.36 ### Patch Changes - Updated @unkey/ratelimit to version 0.4.5 - [Brief description of changes or improvements in this update]packages/hono/CHANGELOG.md (2)
3-8: LGTM! Consider adding a brief description of the change.The new changelog entry for version 1.4.7 is well-formatted and consistent with previous entries. It clearly indicates the patch change and the updated dependency.
To enhance the changelog's informativeness, consider adding a brief description of the change or the reason for the dependency update. For example:
## 1.4.7 ### Patch Changes - Updated dependencies [82f5b5d] - @unkey/api@0.26.2 - Brief description: [Insert reason for the update or any notable changes]This additional context can help users and developers understand the significance of each update.
1-9: Well-maintained changelog. Consider adding a "Highlights" section for major versions.The changelog is consistently formatted and effectively documents the version history and dependency updates of the @unkey/hono package. It's particularly noteworthy how diligently the @unkey/api dependency updates are tracked.
To further enhance the changelog's usefulness, consider adding a "Highlights" or "Summary of Changes" section for major version updates. This could provide a quick overview of significant changes or new features introduced in each major release, making it easier for users to understand the package's evolution at a glance.
packages/nextjs/CHANGELOG.md (1)
3-8: LGTM! Consider adding a brief description of the changes.The new changelog entry for version 0.17.7 is correctly formatted and consistent with previous entries. It accurately reflects the patch update to the
@unkey/apidependency.To improve clarity, consider adding a brief description of any notable changes or fixes introduced by this dependency update. For example:
## 0.17.7 ### Patch Changes - Updated dependencies [82f5b5d] - @unkey/api@0.26.2 - Brief description: [Insert any relevant information about the changes in @unkey/api@0.26.2]This addition would provide more context for users and developers about the nature of the update.
internal/resend/src/client.tsx (3)
27-28: LGTM! Consider extracting common email configuration.The changes to the
fromandreplyTofields improve email formatting and maintain consistency. Good job on standardizing these across all methods.Consider extracting these common email configuration fields into a separate method or constant to reduce repetition across all email sending methods. For example:
private getCommonEmailConfig() { return { from: "James from Unkey <[email protected]>", replyTo: this.replyTo }; } // Usage in methods: const result = await this.client.emails.send({ ...this.getCommonEmailConfig(), to: req.email, subject: "Your Unkey trial has ended", html, });
66-67: LGTM! Consider parameterizing the email delay.The addition of the
scheduledAtparameter is a good feature for controlling when the welcome email is sent. The email field changes are consistent with other methods, which is great.Consider parameterizing the delay for the welcome email to make it more flexible:
public async sendWelcomeEmail(req: { email: string, delayMinutes?: number }) { const delayMs = (req.delayMinutes ?? 5) * 60 * 1000; const scheduledAt = new Date(Date.now() + delayMs).toISOString(); // ... rest of the method ... }This allows the caller to specify a custom delay if needed, while still defaulting to 5 minutes if not specified.
Also applies to: 72-73, 76-76
121-122: LGTM! Consider improving error logging.The changes to the
fromandreplyTofields are consistent with the updates made to other methods, which is good for maintaining uniformity across the class.Consider improving the error logging in the catch block. Instead of just logging the error, it would be more informative to include context about the operation that failed:
} catch (error) { console.error('Error sending leaked key email:', error); // Optionally, you could also log the email address (but be cautious with PII) // console.error('Failed to send leaked key email to:', email); }This will make debugging easier if issues arise in production.
apps/api/src/pkg/middleware/metrics.ts (2)
Line range hint
126-126: Consider performance impact of request body loggingThe redacted
requestBodyis now being used in theanalytics.insertApiRequestcall, which enhances security by preventing sensitive data from being logged. However, for large requests, this could potentially impact performance.Consider the following recommendations:
- Implement a size limit for the logged request body to prevent performance issues with very large requests.
- For large requests exceeding the limit, consider logging only a summary or the first N characters.
Example implementation:
const MAX_BODY_SIZE = 1024 * 10; // 10 KB limit let truncatedRequestBody = requestBody.length > MAX_BODY_SIZE ? requestBody.slice(0, MAX_BODY_SIZE) + '... (truncated)' : requestBody; // Use truncatedRequestBody in analytics.insertApiRequestThis approach balances security needs with performance considerations.
🧰 Tools
🪛 Biome
[error] 10-11: Expected a statement but instead found '<<<<<<< HEAD'.
Expected a statement here.
(parse)
[error] 13-15: Expected a statement but instead found '=======
Expected a statement here.
(parse)
[error] 15-15: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
Line range hint
1-138: Summary of changes and recommendationsThe changes to this middleware enhance security by redacting sensitive information in the request body before logging. This is a positive improvement that aligns with best practices for handling potentially sensitive data.
Key points and recommendations:
- Resolve the merge conflict, ensuring that both the new security feature and the Cloudflare cache rate limits fix are correctly implemented.
- Consider implementing a size limit for logged request bodies to mitigate potential performance issues with large requests.
- After resolving the merge conflict, thoroughly test the middleware to ensure it functions as expected, particularly focusing on the interaction between the new request body handling and any changes from the Cloudflare cache rate limits fix.
- Update any relevant documentation to reflect the new behavior of redacting sensitive information in logged request bodies.
As a final step, consider adding a configuration option to enable/disable request body logging or adjust the maximum logged body size. This would provide flexibility for different environments (e.g., development vs. production) and use cases.
🧰 Tools
🪛 Biome
[error] 10-11: Expected a statement but instead found '<<<<<<< HEAD'.
Expected a statement here.
(parse)
[error] 13-15: Expected a statement but instead found '=======
Expected a statement here.
(parse)
[error] 15-15: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/keys.tsx (3)
101-101: LGTM! Consider applying the gap consistently.The addition of the
gap-2class improves the spacing between grid items, which is a good UI enhancement. This change aligns with the overall improvements for the new recoverable keys feature.For consistency, consider applying this gap to other similar grid layouts in the component, if any exist. This will ensure a uniform appearance throughout the UI.
Line range hint
1-143: Consider adding a recoverable key indicatorWhile this component doesn't directly implement the new recoverable keys feature, it might be beneficial to add a visual indicator for recoverable keys in the key list. This would provide users with at-a-glance information about which keys are recoverable.
You could add a new badge or icon next to recoverable keys. For example:
<div className="flex items-center col-span-3 gap-2"> {/* Existing badges */} {k.isRecoverable && ( <Badge variant="secondary"> <KeyIcon className="w-4 h-4 mr-1" /> Recoverable </Badge> )} </div>This assumes that the key object includes an
isRecoverableproperty. If it doesn't, you may need to modify the data fetching logic to include this information.
Line range hint
1-143: Enhance security awareness for recoverable keysWith the introduction of recoverable keys, it's crucial to ensure users understand the security implications. Consider adding visual cues or informational tooltips to highlight the special nature of recoverable keys.
Suggestions:
- Add a tooltip or info icon next to recoverable keys explaining their nature and security considerations.
- Include a warning message in the key creation/editing process about the security implications of recoverable keys.
- Consider implementing an audit log for actions related to recoverable keys, if not already present.
Example tooltip implementation:
import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { InfoIcon } from "lucide-react"; // Inside the key list item {k.isRecoverable && ( <Tooltip> <TooltipTrigger> <InfoIcon className="w-4 h-4 text-warning" /> </TooltipTrigger> <TooltipContent> This is a recoverable key. Ensure you understand the security implications and handle it with extra care. </TooltipContent> </Tooltip> )}These additions will help maintain a high level of security awareness among users interacting with recoverable keys.
apps/api/src/routes/legacy_keys_verifyKey.ts (1)
Deprecated route
legacy_keys_verifyKeyis still referenced in the codebase.
apps/api/src/worker.tsimportsregisterLegacyKeysVerifyKeyfrom the deprecated route.apps/api/src/routes/legacy_keys_verifyKey.test.tsstill references the deprecated route.🔗 Analysis chain
Line range hint
6-6: Verify deprecation status in relation to new recoverable keys feature.This route is marked as deprecated. Given that the PR introduces a new feature for recoverable keys, it's important to ensure that this deprecation aligns with the overall API evolution strategy.
Let's check if there are any references to this deprecated route in relation to the new feature:
Consider documenting the migration path from this deprecated route to the new recoverable keys feature in the API documentation or changelog.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to this deprecated route in files related to the new recoverable keys feature rg -i "legacy_keys_verifyKey" $(fd -t f -e ts -e tsx recoverable)Length of output: 250
apps/dashboard/app/(app)/desktop-sidebar.tsx (2)
50-65: LGTM. Consider making the icon size configurable.The
DiscordIconcomponent is well-implemented as an SVG. To enhance its reusability, consider making the icon size configurable through props. This would allow for easy adjustment in different contexts without modifying the component itself.Example:
const DiscordIcon = ({ size = 4 }: { size?: number }) => ( <svg className={`w-${size} h-${size} fill-current`} // ... rest of the SVG code > {/* ... */} </svg> );
143-153: LGTM. Consider using a custom icon for the "Docs" item as well.The changes to the
resourcesNavigationarray are good. The Discord item has been added correctly, and the icon for the "Docs" item has been updated. For consistency, you might want to consider creating a custom icon for the "Docs" item as well, similar to theDiscordIcon.const DocsIcon = () => ( // Custom SVG implementation ); // Then in resourcesNavigation { icon: DocsIcon, href: "https://unkey.dev/docs", external: true, label: "Docs", },This would provide a consistent look and feel across all resource navigation items.
apps/dashboard/lib/trpc/routers/key/create.ts (3)
Line range hint
92-195: Resolve merge conflict and approve transaction handling changes.There's a merge conflict in this section that needs to be resolved. The new version (after ">>>>>>> ca22641") with the updated transaction handling is the correct one to keep. This change improves the atomicity of the key creation and encryption process.
Please resolve the merge conflict by keeping the new version and removing the conflict markers.
226-255: LGTM: Key creation and encryption logic for recoverable keys.The implementation of the encryption logic for recoverable keys is well-structured and includes proper error handling. The condition
input.recoverEnabled && keyAuth?.storeEncryptedKeysensures that encryption only occurs when necessary.One minor suggestion:
Consider updating the error message on line 252 for clarity:
- "We are unable to store encrypt the key. Please contact support using [email protected]", + "We are unable to store the encrypted key. Please contact support using [email protected]",
Line range hint
258-272: Consider handling potential audit logging failures.Moving the audit logging outside the transaction is good for performance, but it introduces a potential inconsistency if the transaction succeeds and the audit logging fails.
Consider wrapping the
ingestAuditLogscall in a try-catch block to handle potential failures gracefully:try { await ingestAuditLogs({ // ... existing code ... }); } catch (error) { console.error("Failed to ingest audit logs:", error); // Consider implementing a retry mechanism or alerting system }This ensures that any audit logging failures are caught and logged, allowing for easier debugging and potentially implementing a retry mechanism in the future.
🧰 Tools
🪛 Biome
[error] 173-173: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 190-190: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 190-190: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 188-190: Invalid assignment to
await db .insert(schema.encryptedKeys) ======This expression cannot be assigned to
(parse)
[error] 194-194: Expected an expression but instead found '>>>'.
Expected an expression here.
(parse)
[error] 194-194: Expected an expression but instead found '>'.
Expected an expression here.
(parse)
[error] 194-194: expected
,but instead foundtransactionRemove transaction
(parse)
[error] 182-182: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend vaultRes with an underscore.(lint/correctness/noUnusedVariables)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (2)
941-946: Fix the documentation link for recoverable keys.The link to the documentation about recoverable keys is incorrectly formatted.
Please update the link as follows:
-<Link - className="font-semibold" - href={"unkey.com/docs/security/recovering-keys"} -> - unkey.com/docs/security/recovering-keys. +<Link + target="_blank" + className="font-semibold" + href="/docs/security/recovering-keys" +> + unkey.com/docs/security/recovering-keys </Link>This change ensures that:
- The link is relative to the current domain.
- It opens in a new tab (consistent with the first Card).
- The trailing period is removed from the link text.
Line range hint
1-967: Overall implementation of recoverable keys feature is good, with room for improvement.The implementation of the recoverable keys feature is generally correct and functional. However, there are a few areas that could be improved:
- Remove the duplicate "Recoverable" Card component as mentioned earlier.
- Ensure consistent formatting and naming conventions throughout the file.
- Consider extracting some of the larger Card components into separate, reusable components to improve readability and maintainability.
These improvements will enhance the overall quality and maintainability of the code.
To further improve the code structure, consider:
- Creating a separate component for the "Recoverable" Card.
- Using a custom hook to manage the form logic, which would help separate concerns and make the component more focused on rendering.
- Implementing a more robust error handling system for form submissions and API calls.
internal/resend/emails/secret_scanning_key_detected.tsx (3)
24-24: Correct the spelling of 'Github' to 'GitHub'The correct spelling is "GitHub" with a capital 'H'.
Apply this diff:
- <Text>Github found that one of your keys has been leaked. Details are as follows:</Text> + <Text>GitHub found that one of your keys has been leaked. Details are as follows:</Text>
52-55: Enhance link text for accessibilityFor better accessibility, consider using more descriptive link text instead of displaying the email address directly.
Apply this diff:
<Text> - Need help? Please reach out to{" "} - <Link href="mailto:[email protected]">[email protected]</Link> or just reply to this email. + Need help? Please reach out to our{" "} + <Link href="mailto:[email protected]">support team</Link> or just reply to this email. </Text>
61-64: Use HTTPS instead of HTTP for URLsFor security and user trust, it's recommended to use HTTPS in URLs.
Apply this diff:
SecretScanningKeyDetected.PreviewProps = { date: "Tue Oct 01 2024", // Date().toDateString source: "commit", - url: "http://unkey.com", + url: "https://unkey.com", } satisfies Props;internal/resend/emails/welcome_email.tsx (3)
31-31: Remove unnecessary whitespace strings.On lines 31 and 41, there's an unnecessary string containing a space (
" "). This extra space is not needed and can be removed to clean up the code.Apply this diff to remove the unnecessary spaces:
- {" "}Also applies to: 41-41
32-33: Ensure consistent URL formatting in links.The
hrefattributes in yourLinkcomponents use different URL formats:
https://go.unkey.comhttps://www.unkey.comhttps://unkey.comFor consistency and to prevent any potential confusion or redirection issues, consider standardizing the URLs to use a uniform format (e.g., always include or exclude
www).Also applies to: 36-38, 42-43
62-63: Manage user expectations in the postscript message.The statement "I read and reply to every single one" may become unsustainable as the user base grows. Consider adjusting the message to set realistic expectations:
- P.S. - if you have any questions or feedback, reply to this email. I read and reply to every single one. + P.S. - if you have any questions or feedback, feel free to reply to this email. I'll do my best to get back to you.internal/resend/emails/trial_ended.tsx (5)
26-27: Minor grammatical improvement in the messageThe phrase "for your workspace" is repeated and could be rephrased for better readability.
Consider updating the text:
- We hope you've enjoyed your two-week Unkey Pro trial for your workspace{" "} - <strong>{workspaceName}</strong>. + We hope you've enjoyed your two-week Unkey Pro trial of{" "} + <strong>{workspaceName}</strong>.
30-31: Enhance the clarity of the sentenceThe sentence can be made more engaging by emphasizing the continuation of Pro features.
Consider rephrasing:
- Since your trial ended, please add a payment method to keep all features of the Pro plan. + Now that your trial has ended, add a payment method to continue enjoying all the Pro plan features.
39-52: Clean up unnecessary JSX whitespaceThe use of
{" "}in lines 39, 44, and 49 is unnecessary as JSX handles spacing appropriately.Apply this diff to remove redundant whitespace:
- <li className="pb-4"> - {" "} - 1M monthly active keys included{" "} + <li className="pb-4"> + 1M monthly active keys included <span className="italic text-sm">(free users only get 1k total)</span> </li> - <li className="pb-4"> - {" "} - 150k monthly verifications included{" "} + <li className="pb-4"> + 150k monthly verifications included <span className="italic text-sm">(free users only get 2.5k per month)</span> </li> - <li className="pb-4"> - {" "} - 2.5M monthly ratelimits included{" "} + <li className="pb-4"> + 2.5M monthly ratelimits included <span className="italic text-sm">(free users only get 100k per month)</span> </li>
58-58: Consistency in list item formattingIn line 58, consider adding a period at the end of the sentence for consistency with other list items.
- Unlimited seats at no additional cost so you can invite your whole team + Unlimited seats at no additional cost so you can invite your whole team.
75-75: Optional: Add spacing around the horizontal rule for better visual separationConsider adding vertical spacing around the
<Hr />component to improve the email's readability.</Section> + <div className="py-4"> <Hr /> + </div>apps/dashboard/components/dashboard/feedback-component.tsx (3)
28-29: Use camelCase for Prop NamesThe prop
FeedbackOpenis not following the camelCase convention commonly used in JavaScript and TypeScript. Renaming it tofeedbackOpenimproves consistency and readability.Apply this diff to rename the prop:
interface FeedbackProps { variant?: FeedbackVariant; - FeedbackOpen?: boolean; + feedbackOpen?: boolean; }Update its usage accordingly:
-export const Feedback: React.FC<FeedbackProps> = ({ variant, FeedbackOpen }) => { +export const Feedback: React.FC<FeedbackProps> = ({ variant, feedbackOpen }) => { const [open, setOpen] = useState(false); useEffect(() => { - if (FeedbackOpen) { + if (feedbackOpen) { setOpen(true); } else { setOpen(false); } - }, [FeedbackOpen]); + }, [feedbackOpen]);
44-47: Clarify the Comment for Better UnderstandingThe comment explaining the necessity of the code could be more specific to help future maintainers understand why this implementation is required.
Consider updating the comment for clarity:
/** - * This was necessary cause otherwise the dialog would not close when you're clicking outside of it + * Ensuring the `open` prop and `onOpenChange` handler are set allows the dialog to close when clicking outside of it. */
83-85: Add Accessible Label to the Feedback ButtonThe feedback button lacks an
aria-label, which can hinder accessibility for users relying on screen readers. Adding anaria-labelwill improve the user experience for all users.Apply this diff to include an accessible label:
<button type="button" onClick={() => setOpen(true)} className={`flex items-center w-full ${cursorClasses}`} + aria-label="Open feedback form" > <MessagesSquare className="w-4 h-4 mr-2" /> Feedback </button>internal/resend/emails/payment_issue.tsx (2)
24-25: Improve date formatting for better readabilityThe date displayed using
Date().toDateStringresults in a format like "Tue Oct 01 2024", which may not be the most user-friendly. Consider formatting the date to a more readable format, such as "October 1, 2024", using a date formatting library likedate-fnsorMoment.js.
43-43: Consider providing a full name or title in the signatureUsing only "James" in the signature may be too informal. Consider providing a full name or including the individual's role or department to make it more professional.
🛑 Comments failed to post (23)
apps/api/src/routes/v1_ratelimit_limit.accuracy.test.ts (1)
99-103:
⚠️ Potential issueResolve merge conflict and remove conflict markers
There's an unresolved merge conflict in this section. Please resolve the conflict by keeping the new calculation for
upperLimitand removing the conflict markers.Apply this diff to resolve the merge conflict:
-<<<<<<< HEAD - const upperLimit = Math.round(exactLimit * 2.5); -======= const upperLimit = Math.round(exactLimit * 1.5); ->>>>>>> 7ffdbc4f (fix: cf cache ratelimits (#2112))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const upperLimit = Math.round(exactLimit * 1.5);🧰 Tools
🪛 Biome
[error] 100-101: Expected a statement but instead found '======='.
Expected a statement here.
(parse)
[error] 102-103: Expected a statement but instead found '>>>>>>> 7ffdbc4 (fix: cf cache ratelimits (#2112))'.
Expected a statement here.
(parse)
[error] 103-103: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
[error] 102-102: Shouldn't redeclare 'upperLimit'. Consider to delete it or rename it.
'upperLimit' is defined here:
(lint/suspicious/noRedeclare)
[error] 100-100: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend upperLimit with an underscore.(lint/correctness/noUnusedVariables)
internal/resend/src/client.tsx (1)
42-42:
⚠️ Potential issueAddress the TODO comment regarding method deprecation.
The TODO comment suggests that this method might be unused and should be deprecated.
If this method is indeed unused, consider removing it entirely to keep the codebase clean. If there's a reason to keep it, please add a comment explaining why, and consider marking it as deprecated using a proper TypeScript decorator:
/** @deprecated This method is no longer in use and will be removed in the next major version. */ @deprecated("This method is no longer in use and will be removed in the next major version.") public async sendSubscriptionEnded(req: { email: string; name: string; }): Promise<void> { // ... }Also, ensure to update any documentation or API references if you decide to deprecate or remove this method.
apps/api/src/routes/legacy_keys_verifyKey.ts (1)
51-51: 💡 Codebase verification
Found remaining instances of typo "identifiy" in multiple files
The typo "identifiy" was found in the following locations:
packages/api/src/openapi.d.ts:
* @description The name of the key, give keys a name to easily identifiy their purpose* @description The name of the key, give keys a name to easily identifiy their purpose
apps/api/src/routes/v1_keys_verifyKey.ts:
"The name of the key, give keys a name to easily identifiy their purpose",Please correct these instances by replacing "identifiy" with "identify" to ensure documentation clarity.
🔗 Analysis chain
LGTM: Typo correction improves documentation clarity.
The correction from "identifiy" to "identify" in the description of the 'name' field enhances the clarity of the API documentation. This change, while minor, contributes to maintaining high-quality documentation.
Let's check if there are any other occurrences of this typo in the file:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of "identifiy" in the file rg "identifiy" "apps/api/src/routes/legacy_keys_verifyKey.ts"Length of output: 59
Script:
#!/bin/bash # Recursively search for any instances of "identifiy" in the entire codebase rg "identifiy" --ignore-dir=node_modules --ignore-dir=dist --ignore-dir=buildLength of output: 520
Script:
#!/bin/bash # Recursively search for any instances of "identifiy" in the entire codebase rg "identifiy"Length of output: 402
apps/dashboard/lib/trpc/routers/key/create.ts (1)
3-6:
⚠️ Potential issueRemove duplicate import of 'env'.
There's a duplicate import of the 'env' module.
Please remove the duplicate import on line 6:
import { env } from "@/lib/env"; import { ingestAuditLogsTinybird } from "@/lib/tinybird"; import { rateLimitedProcedure, ratelimit } from "@/lib/trpc/ratelimitProcedure"; - import { env } from "@/lib/env"; import { TRPCError } from "@trpc/server";This will resolve the "noRedeclare" warning from the static analysis tool.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { env } from "@/lib/env"; import { ingestAuditLogsTinybird } from "@/lib/tinybird"; import { rateLimitedProcedure, ratelimit } from "@/lib/trpc/ratelimitProcedure"; import { TRPCError } from "@trpc/server";🧰 Tools
🪛 Biome
[error] 6-6: Shouldn't redeclare 'env'. Consider to delete it or rename it.
'env' is defined here:
(lint/suspicious/noRedeclare)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (1)
850-949:
⚠️ Potential issueRemove duplicate "Recoverable" Card component.
There are two nearly identical Card components for the "Recoverable" feature. This duplication is likely unintended and could lead to confusion or inconsistent behavior.
To resolve this, please remove one of the Card components. The first one (lines 850-901) seems more appropriate as it's conditionally rendered based on
checkStoreEncryptedKeys. Here's a suggested fix:
- Keep the Card component at lines 850-901.
- Remove the duplicate Card component at lines 902-949.
- Ensure that the remaining Card component correctly handles all necessary logic and UI elements for the recoverable feature.
This will eliminate redundancy and potential inconsistencies in the UI.
internal/resend/emails/subscription_ended.tsx (1)
17-17:
⚠️ Potential issueCorrect
text-semiboldtofont-semiboldThe class
text-semiboldis not a valid Tailwind CSS class. Replace it withfont-semiboldto properly apply the font weight.Apply this diff to fix the class name:
- <Heading className="font-sans text-3xl text-semibold text-center"> + <Heading className="font-sans text-3xl font-semibold text-center">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Heading className="font-sans text-3xl font-semibold text-center">internal/resend/emails/secret_scanning_key_detected.tsx (3)
27-37: 🛠️ Refactor suggestion
Remove unnecessary whitespace elements
The
{" "}elements inside the<li>tags are unnecessary and can be removed to clean up the code.Apply this diff:
<li className="pt-4"> - {" "} <strong>Source:</strong> {source} - {" "} </li> <li className="pt-4"> - {" "} <strong>Date:</strong> {date} - {" "} </li> <li className="pt-4"> - {" "} <strong>URL:</strong> {url} </li>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<strong>Source:</strong> {source} </li> <li className="pt-4"> <strong>Date:</strong> {date} </li> <li className="pt-4"> <strong>URL:</strong> {url} </li>
20-20:
⚠️ Potential issueUse correct Tailwind CSS class: replace 'text-semibold' with 'font-semibold'
The class name "text-semibold" in the
<Heading>component is not a valid Tailwind CSS class. To make the text semibold, you should use "font-semibold" instead.Apply this diff to fix the class name:
- <Heading className="font-sans text-3xl text-semibold text-center"> + <Heading className="font-sans text-3xl font-semibold text-center">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Heading className="font-sans text-3xl font-semibold text-center">
46-51: 🛠️ Refactor suggestion
Use meaningful link text instead of raw URLs
Displaying raw URLs can be less user-friendly. Consider using descriptive text for the link to improve readability.
Apply this diff:
<Text> - You can disable the Root Key in your dashboard by following our docs available at{" "} - <Link href="https://www.unkey.com/docs/security/root-keys"> - https://www.unkey.com/docs/security/root-keys - </Link> - . + You can disable the Root Key in your dashboard by following our{" "} + <Link href="https://www.unkey.com/docs/security/root-keys"> + documentation on Root Keys + </Link> + . </Text>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.You can disable the Root Key in your dashboard by following our{" "} <Link href="https://www.unkey.com/docs/security/root-keys"> documentation on Root Keys </Link> . </Text>internal/resend/emails/welcome_email.tsx (2)
59-59: 🛠️ Refactor suggestion
Enhance the feedback request about how users heard about Unkey.
Currently, you're asking users how they heard about Unkey without providing a way to respond directly. Consider adding a link to a quick survey or prompting them to reply to the email:
- <Text>Also, just curious - how did you hear about Unkey?</Text> + <Text>Also, just curious - how did you hear about Unkey? Reply to this email and let us know!</Text>Committable suggestion was skipped due to low confidence.
20-24: 🛠️ Refactor suggestion
Personalize the greeting message with the user's name.
Currently, the greeting says "Hi there!". If you have access to the user's name, consider personalizing the greeting for a more engaging experience:
- <Text>Hi there!</Text> + <Text>Hi {username}!</Text>Committable suggestion was skipped due to low confidence.
internal/resend/emails/trial_ended.tsx (2)
67-73: 🛠️ Refactor suggestion
Button styling best practices
Ensure the button's class names align with Tailwind CSS best practices, and consider extracting styles into a reusable component if used frequently.
20-20:
⚠️ Potential issueIncorrect Tailwind CSS class: Replace
text-semiboldwithfont-semiboldIn line 20, the class
text-semiboldis not a valid Tailwind CSS class for font weight. The correct class isfont-semibold.Apply this diff to fix the class name:
- <Heading className="font-sans text-3xl text-semibold text-center"> + <Heading className="font-sans text-3xl font-semibold text-center">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Heading className="font-sans text-3xl font-semibold text-center">apps/dashboard/components/dashboard/command-menu.tsx (1)
90-94:
⚠️ Potential issueRender
FeedbackComponentoutsideCommandItemto manage visibility properlyCurrently,
FeedbackComponentis rendered insideCommandItem, which may lead to unexpected behavior sinceCommandItemis intended to display menu items, not render modal components. To properly manage the visibility ofFeedbackComponent, render it outside ofCommandItemand control its visibility using theopenstate.Here is a suggested refactor:
const FeedbackCommand: React.FC = () => { const [open, setOpen] = useState(false); - return ( - <CommandItem onSelect={() => setOpen(true)}> - <FeedbackComponent variant="command" FeedbackOpen={open} /> - </CommandItem> - ); + return ( + <> + <CommandItem onSelect={() => setOpen(true)}> + <FeedbackIcon className="w-4 h-4 mr-2" /> + <span>Send Feedback</span> + </CommandItem> + <FeedbackComponent + variant="command" + FeedbackOpen={open} + onClose={() => setOpen(false)} + /> + </> + ); };Note: Ensure to import an appropriate icon for
FeedbackIcon, or replace it with suitable text to maintain consistency with other command items.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const FeedbackCommand: React.FC = () => { const [open, setOpen] = useState(false); return ( <> <CommandItem onSelect={() => setOpen(true)}> <FeedbackIcon className="w-4 h-4 mr-2" /> <span>Send Feedback</span> </CommandItem> <FeedbackComponent variant="command" FeedbackOpen={open} onClose={() => setOpen(false)} /> </> ); };apps/dashboard/components/dashboard/feedback-component.tsx (5)
33-33:
⚠️ Potential issueTypo in Variable Name
cursorClasessThere's a typo in the variable name
cursorClasess. It should becursorClassesto maintain consistent spelling and prevent potential confusion.Apply this diff to correct the typo:
-const cursorClasess = variant === "command" ? "cursor-default" : "cursor-pointer"; +const cursorClasses = variant === "command" ? "cursor-default" : "cursor-pointer"; ... <button type="button" onClick={() => setOpen(true)} - className={`flex items-center w-full ${cursorClasess}`} + className={`flex items-center w-full ${cursorClasses}`} >Also applies to: 81-81
88-89:
⚠️ Potential issueHandle Form Submission with
onSubmitCurrently, the form submission logic is attached to the submit button's
onClickhandler. It's best practice to handle form submission using the form'sonSubmitevent to ensure proper handling of form submission events, including when the user presses Enter.Apply this diff to move the submission logic to the form:
<Form {...form}> - <form> + <form + onSubmit={form.handleSubmit((data) => { + create.mutate(data); + })} + > <DialogContent> {/* ... */} </DialogContent> </form> </Form>Then, remove the
onClickhandler from the submit button:<Button type="submit" disabled={create.isLoading} - onClick={form.handleSubmit((data) => { - create.mutate(data); - })} > {create.isLoading ? <Loading /> : "Send"} </Button>Also applies to: 168-172
24-24: 🛠️ Refactor suggestion
Unnecessary Type Alias
FeedbackOpenThe type alias
FeedbackOpen = booleanis unnecessary since it's simply a boolean. You can directly usebooleanin yourFeedbackPropsinterface for clarity.Apply this diff to simplify the type:
-type FeedbackOpen = boolean; interface FeedbackProps { variant?: FeedbackVariant; - FeedbackOpen?: FeedbackOpen; + feedbackOpen?: boolean; }Committable suggestion was skipped due to low confidence.
76-76: 🛠️ Refactor suggestion
Simplify Complex
classNameStringThe
classNamestring is quite long and can be hard to read and maintain. Consider extracting it into a constant or using a utility likeclsxorclassnamesto improve readability.Example using a constant:
+const containerClassName = `w-full transition-all duration-150 group flex gap-x-2 rounded-md text-sm font-normal leading-6 items-center border border-transparent hover:bg-background-subtle hover:text-content justify-between ${commandClasses}`; return ( <div - className={`w-full transition-all duration-150 group flex gap-x-2 rounded-md text-sm font-normal leading-6 items-center border border-transparent hover:bg-background-subtle hover:text-content justify-between ${commandClasses}`} + className={containerClassName} > {/* ... */} </div> );Or using
clsxfor better management:+import clsx from "clsx"; return ( <div className={clsx( "w-full transition-all duration-150 group flex gap-x-2 rounded-md text-sm font-normal leading-6 items-center border border-transparent hover:bg-background-subtle hover:text-content justify-between", commandClasses )} > {/* ... */} </div> );Committable suggestion was skipped due to low confidence.
63-72: 💡 Codebase verification
⚠️ Potential issueMultiple Instances of
toast.errorRequire Proper Error HandlingSeveral usages of
toast.erroracross the codebase do not provide a default error message, which could lead to potential runtime errors iferr.messageis undefined. Please update these instances to include a fallback message.
- Example:
toast.error(err.message || "An unexpected error occurred.");🔗 Analysis chain
Ensure Proper Error Handling in Mutation
The error handling in the
createmutation assumes thaterr.messageexists. To prevent potential runtime errors, ensure thaterr.messageis always available or provide a default message.Apply this diff to safeguard against undefined error messages:
onError(err) { console.error(err); - toast.error(err.message); + toast.error(err.message || "An unexpected error occurred."); },Run the following script to check all usages of
toast.errorto ensure they handle errors appropriately:
Action Required: Re-run Verification with Corrected Script
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all uses of 'toast.error' handle error messages safely. # Search for 'toast.error' usages in TypeScript and TSX files rg --type-add 'typescript: *.ts' --type-add 'tsx: *.tsx' --type {typescript,tsx} 'toast\.error\(' # Expected: All usages should handle cases where 'err.message' might be undefined.Length of output: 152
Script:
#!/bin/bash # Description: Verify that all uses of 'toast.error' handle error messages safely. # Define custom types for TypeScript and TSX rg --type-add 'typescript:*.ts' --type-add 'tsx:*.tsx' --type typescript --type tsx 'toast\.error\('Length of output: 7747
apps/api/src/pkg/ratelimit/client.ts (2)
39-43:
⚠️ Potential issueUnresolved merge conflict markers found between lines 39-43
Merge conflict markers (
<<<<<<<,=======,>>>>>>>) are present in the code between lines 39 and 43. This indicates that a merge conflict has not been properly resolved. Please resolve these conflicts to ensure the code compiles and functions correctly.🧰 Tools
🪛 Biome
[error] 40-42: Expected a statement but instead found '=======
private setCache(id: string, current: number, reset: number, blocked: boolean)'.Expected a statement here.
(parse)
[error] 42-43: Expected a statement but instead found '>>>>>>> 7ffdbc4 (fix: cf cache ratelimits (#2112))'.
Expected a statement here.
(parse)
[error] 43-43: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
62-70:
⚠️ Potential issueUnresolved merge conflict markers found between lines 62-70
There are merge conflict markers (
<<<<<<<,=======,>>>>>>>) present between lines 62 and 70. These need to be resolved by selecting the correct code and removing the conflict markers.🧰 Tools
🪛 Biome
[error] 67-69: Expected a statement but instead found '=======
this.cache.set(id,'.Expected a statement here.
(parse)
[error] 69-69: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 69-70: Expected a statement but instead found '>>>>>>> 7ffdbc4 (fix: cf cache ratelimits (#2112))'.
Expected a statement here.
(parse)
[error] 70-70: numbers cannot be followed by identifiers directly after
an identifier cannot appear here
(parse)
internal/resend/emails/payment_issue.tsx (2)
19-19:
⚠️ Potential issueFix incorrect Tailwind CSS class name in Heading component
In the
classNamefor theHeadingcomponent,text-semiboldis not a valid Tailwind CSS class. The correct class for setting font weight to semi-bold isfont-semibold.Apply this diff to fix the class name:
-<Heading className="font-sans text-3xl text-semibold text-center"> +<Heading className="font-sans text-3xl font-semibold text-center">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Heading className="font-sans text-3xl font-semibold text-center">
28-35:
⚠️ Potential issueEnsure button styles render correctly in email clients
Email clients often have limited support for CSS classes and external stylesheets. To ensure the button renders consistently across different email clients, consider inlining the styles instead of relying on Tailwind CSS classes.
Apply this diff to inline the styles for the
Buttoncomponent:-<Button - href="https://app.unkey.com/settings/billing/stripe" - className="bg-gray-900 text-gray-50 rounded-lg p-3 w-2/3" -> +<Button + href="https://app.unkey.com/settings/billing/stripe" + style=" + background-color: #1f2937; /* bg-gray-900 */ + color: #f9fafb; /* text-gray-50 */ + border-radius: 0.5rem; /* rounded-lg */ + padding: 12px; /* p-3 */ + width: 66.6667%; /* w-2/3 */ + text-align: center; + text-decoration: none; + display: inline-block; + " +>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Section className="text-center py-3"> <Button href="https://app.unkey.com/settings/billing/stripe" style=" background-color: #1f2937; /* bg-gray-900 */ color: #f9fafb; /* text-gray-50 */ border-radius: 0.5rem; /* rounded-lg */ padding: 12px; /* p-3 */ width: 66.6667%; /* w-2/3 */ text-align: center; text-decoration: none; display: inline-block; " > Update payment information </Button> </Section>
harshsbhat
force-pushed
the
harshbhat/recoverable-keys
branch
from
eb9428b to
889e073
Compare
What does this PR do?
Fixes #2097
This PR introduces the feature to create recoverable keys from the dashboard itself. Adding a toggle recoverable in the dashboard creates keys if enabled encrypts the data and then stores the encrypted data. This also introduces an internal package named
@unkey/vaultwhich allows the dashboard to interact with the vault.https://www.loom.com/share/27f3a1b58e5a4d478b1510a80f679558?sid=e8faf269-7f80-4b8c-9229-7b55c547ebf2
Type of change
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Summary by CodeRabbit
New Features
recoverEnabledfeature in the key creation form, with informative descriptions about security implications.Feedbackcomponent for user feedback submission, streamlining the feedback process.Bug Fixes
recoverEnabledfeature.