chore(cli): Add Zod validation to parseInput function

sujal-sakpal commented 4 months ago

User description

Fixes #361

Description: Hi ,

I've added Zod validation to the profile creation command. The parseInput method now validates user input for name, apiKey, baseUrl, and setDefault fields using predefined schemas.

Changes Made:

Implemented Zod validation schemas for:

name: Must be alphanumeric with no spaces. apiKey: Should start with ks_ and contain only letters and numbers. baseUrl: Valid URL or empty. setDefault: Optional boolean. Updated parseInput to validate input before processing.

Added a basic implementation of the text function for user input collection.

Testing:

Validated user inputs to ensure correct validation. Handled cases where inputs may be missing or incorrect. Please review and let me know if you have any questions or suggestions. Thanks!

PR Type

Enhancement, Tests

Description

Added Zod validation schemas to ensure proper input validation for profile creation.
Updated the parseInput method to use the new validation schemas.
Implemented a basic text function to handle user input collection.

Changes walkthrough 📝

Relevant files

Enhancement

create.profile.ts

Add Zod validation and input parsing enhancements

apps/cli/src/commands/profile/create.profile.ts

Added Zod validation schemas for name, apiKey, baseUrl, and
setDefault.

Updated parseInput method to validate input using the defined schemas.

Implemented a basic text function for user input collection.

+44/-23

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codiumai-pr-agent-free[bot] commented 4 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Validation Logic
The validation logic for `baseUrl` allows an empty string, which might not be intended as valid input for a URL. Consider if an empty string should be valid or if it should be handled differently. Dummy Implementation
The `text` function is currently implemented with a dummy return value. Ensure to replace this with actual logic to collect user input.

codiumai-pr-agent-free[bot] commented 4 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for input validation to manage exceptions gracefully ___ Consider adding error handling for the `parse` method of the `inputSchema`. Currently, if the validation fails, it will throw an error which is not caught, potentially causing the application to crash or behave unexpectedly. [apps/cli/src/commands/profile/create.profile.ts [112]](https://github.com/keyshade-xyz/keyshade/pull/362/files#diff-71f3f4ea9fcc92ffe9e871082369710fad3f7e2120141ffa595a07d0add4c329R112-R112) ```diff -const parsedData = inputSchema.parse({ name, apiKey, baseUrl, setDefault }); +let parsedData; +try { + parsedData = inputSchema.parse({ name, apiKey, baseUrl, setDefault }); +} catch (error) { + console.error('Validation error:', error); + throw new Error('Input validation failed'); +} ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 10 Why: Adding error handling for the input validation is crucial as it prevents the application from crashing due to unhandled exceptions, thus improving the robustness and reliability of the code.	10
Possible issue	Ensure the optional baseUrl is a valid URL if provided ___ The `baseUrlSchema` should ensure that the URL is not just any string but a valid URL format. The current implementation allows an empty string or any string when the URL is optional. It's better to ensure that if a URL is provided, it must be valid. [apps/cli/src/commands/profile/create.profile.ts [73]](https://github.com/keyshade-xyz/keyshade/pull/362/files#diff-71f3f4ea9fcc92ffe9e871082369710fad3f7e2120141ffa595a07d0add4c329R73-R73) ```diff -const baseUrlSchema = z.string().url().or(z.string().length(0)).optional(); +const baseUrlSchema = z.string().url().optional(); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion improves the validation logic by ensuring that if a URL is provided, it must be valid, which enhances data integrity and prevents potential issues with invalid URLs.	8
Enhancement	✅ Replace the dummy return in the `text` function with actual input collection logic ___ Suggestion Impact: The commit replaced the dummy return in the text function with actual logic to collect user input using the `text` function with a message and placeholder. code diff: ```diff -async function text(options: { message: string, placeholder: string }): Promise { - // Implement your actual input collection logic here - return ''; // Dummy return, replace with actual user input ``` ___ The `text` function currently returns a dummy empty string, which is not practical for a real-world application. Replace the dummy return with actual logic to collect user input, or if this is placeholder code, ensure to implement it before merging. [apps/cli/src/commands/profile/create.profile.ts [86]](https://github.com/keyshade-xyz/keyshade/pull/362/files#diff-71f3f4ea9fcc92ffe9e871082369710fad3f7e2120141ffa595a07d0add4c329R86-R86) ```diff -return ''; // Dummy return, replace with actual user input +return await prompt(options.message, { default: options.placeholder }); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: This suggestion addresses a placeholder that needs to be replaced with actual logic, which is important for the functionality of the application. However, it may be less critical if the placeholder is intended for future implementation.	7
Enhancement	Expand the allowed characters in `nameSchema` to include underscores ___ The regex for `nameSchema` allows alphanumeric characters but does not permit underscores or other common characters that might be valid in certain contexts. Consider expanding the character set if applicable. [apps/cli/src/commands/profile/create.profile.ts [72]](https://github.com/keyshade-xyz/keyshade/pull/362/files#diff-71f3f4ea9fcc92ffe9e871082369710fad3f7e2120141ffa595a07d0add4c329R72-R72) ```diff -const nameSchema = z.string().regex(/^[a-zA-Z0-9]+$/, 'Name must contain only letters and numbers without spaces.'); +const nameSchema = z.string().regex(/^[a-zA-Z0-9_]+$/, 'Name must contain only letters, numbers, and underscores without spaces.'); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: Allowing underscores in the `nameSchema` can make the application more flexible and user-friendly, but it is a minor enhancement compared to other suggestions.	6

rajdip-b commented 3 months ago

@sujal-sakpal any progress on this?

keyshade-xyz / keyshade