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.

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

feat(schema, api-client): Migrate secret types and schemas to @keyshade/schema #539

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 16, 2024

User description

Description

@keyshade/schema

  • Add schemas and types for secret.
  • Add tests for secret schemas.
  • Export secret schemas from index and its types from index.types

@keyshade/api-client

  • Remove existing secret.types.
  • Change imports for secret types to @keyshade/schema.

Related to #519

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

enhancement, tests


Description

  • Migrated secret types and schemas from api-client to @keyshade/schema.
  • Removed secret type definitions from api-client.
  • Added new secret schema definitions in @keyshade/schema using zod.
  • Updated import statements in api-client to use @keyshade/schema for secret types.
  • Added comprehensive tests for the new secret schemas.

Changes walkthrough 📝

Relevant files
Enhancement
secret.ts
Update import statements for secret types                               

packages/api-client/src/controllers/secret.ts

  • Updated import statements to use @keyshade/schema for secret types.
  • +1/-1     
    secret.types.d.ts
    Remove secret type definitions                                                     

    packages/api-client/src/types/secret.types.d.ts

    • Removed all secret-related type definitions.
    +0/-124 
    index.ts
    Update export for secret schemas                                                 

    packages/schema/src/index.ts

    • Updated export statement for secret schemas.
    +2/-1     
    index.types.ts
    Update secret type exports                                                             

    packages/schema/src/index.types.ts

  • Removed direct export of secret schemas.
  • Added export for secret types from secret.types.ts.
  • +2/-3     
    secret.ts
    Remove old secret schema definitions                                         

    packages/schema/src/secret.ts

    • Removed old secret schema definitions.
    +0/-16   
    secret.ts
    Add new secret schema definitions                                               

    packages/schema/src/secret/secret.ts

    • Added new secret schema definitions using zod.
    +141/-0 
    secret.types.ts
    Add secret type definitions                                                           

    packages/schema/src/secret/secret.types.ts

    • Added type definitions inferred from secret schemas.
    +62/-0   
    Tests
    secret.spec.ts
    Add tests for secret schemas                                                         

    packages/schema/tests/secret.spec.ts

    • Added comprehensive tests for secret schemas.
    +490/-22

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    519 - Fully compliant

    Compliant requirements:

    • Created secret folder in schema package
    • Added secret.ts with zod schemas
    • Added secret.types.ts with inferred types
    • Updated index.ts exports
    • Updated api-client to use schema types
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Schema Validation
    The SecretSchema allows null values for rotateAt and note fields but doesn't validate their format when non-null. Should add validation for date format and note content.

    Type Safety
    The RollBackSecretResponseSchema expects count as string but it represents a numeric value. Consider using number type instead for better type safety.

    @muntaxir4 muntaxir4 requested a review from rajdip-b November 16, 2024 21:52
    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Nov 16, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Add length validation constraints for secret values to enforce security requirements

    Add validation for minimum length and maximum length constraints on the secret value
    field to ensure secrets meet security requirements and prevent oversized secrets.

    packages/schema/src/secret/secret.ts [22]

    -value: z.string(),
    +value: z.string().min(1).max(4096),
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding length constraints for secret values is crucial for security and system stability. This prevents empty secrets and oversized values that could cause system issues.

    9
    Add validation rules for secret names to prevent sensitive data exposure

    Add validation to ensure the secret name doesn't contain sensitive information by
    restricting special characters and enforcing a maximum length.

    packages/schema/src/secret/secret.ts [7]

    -name: z.string(),
    +name: z.string().regex(/^[a-zA-Z0-9_-]+$/).max(64),
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Restricting secret names to alphanumeric characters and reasonable length prevents accidental exposure of sensitive data and ensures consistent naming conventions.

    8
    Best practice
    Add maximum length constraint for note field to prevent abuse

    Add validation to ensure the note field has a reasonable maximum length to prevent
    potential abuse.

    packages/schema/src/secret/secret.ts [12]

    -note: z.string().nullable(),
    +note: z.string().max(1000).nullable(),
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Setting a maximum length for the note field prevents potential abuse and ensures database efficiency, while still allowing sufficient space for meaningful documentation.

    6

    💡 Need additional feedback ? start a PR chat

    @muntaxir4 muntaxir4 changed the title feat(schema, api-client): Migrate integration types and schemas to @keyshade/schema feat(schema, api-client): Migrate secret types and schemas to @keyshade/schema Nov 16, 2024
    packages/schema/src/index.types.ts Outdated Show resolved Hide resolved
    packages/schema/src/index.ts Outdated Show resolved Hide resolved
    @muntaxir4 muntaxir4 requested a review from rajdip-b November 18, 2024 11:27
    @rajdip-b rajdip-b merged commit bc3100b into keyshade-xyz:develop Nov 18, 2024
    5 checks passed
    @muntaxir4 muntaxir4 deleted the integration-schemas branch November 18, 2024 12:00
    rajdip-b pushed a commit that referenced this pull request Dec 3, 2024
    ## [2.8.0](v2.7.0...v2.8.0) (2024-12-03)
    
    ### 🚀 Features
    
    * **api:** Add workspace removal notification email template ([#476](#476)) ([40b754f](40b754f))
    * **cli:** Store `metrics_enabled` key in profile config ([#536](#536)) ([9283b22](9283b22))
    * **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([#557](#557)) ([126d024](126d024))
    * **platform:** Added screen for CREATE NEW PROJECT ([#540](#540)) ([b644633](b644633))
    * **platform:** Updated the empty state of dashboard ([#522](#522)) ([28739d9](28739d9))
    * **schema, api-client:** Migrate auth types to @keyshade/schema ([#532](#532)) ([d880098](d880098))
    * **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([#546](#546)) ([a3267de](a3267de))
    * **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([#547](#547)) ([08868c3](08868c3))
    * **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([#538](#538)) ([c468af0](c468af0))
    * **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([#539](#539)) ([bc3100b](bc3100b))
    * **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([#535](#535)) ([c24695e](c24695e))
    * **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([#545](#545)) ([0ee8f9a](0ee8f9a))
    * **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([#569](#569)) ([4398969](4398969))
    * **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([#568](#568)) ([9efbf2d](9efbf2d))
    * **schema:** Add User type inference from UserSchema ([#574](#574)) ([84c1db5](84c1db5))
    
    ### 🐛 Bug Fixes
    
    * **api:** Incorrect oauth redirect url ([58d96e5](58d96e5))
    * **platform:** Resolve loading SVG blocking input field interaction ([#571](#571)) ([30f4f65](30f4f65))
    
    ### 📚 Documentation
    
    * Add pictures to Bruno setup ([#541](#541)) ([210c0fd](210c0fd))
    * Migrate to Bruno ([#525](#525)) ([1793d92](1793d92))
    
    ### 🔧 Miscellaneous Chores
    
    * **ci:** Add script to validate schema package ([59e4280](59e4280))
    * Fixed codecov client version ([a998ae4](a998ae4))
    * **package:** Fixed tests and did housekeeping ([#544](#544)) ([40008e3](40008e3))
    * Update test coverage settings ([5b27e32](5b27e32))
    * Update Turbo to 2.3.1 ([#564](#564)) ([3a63823](3a63823))
    * **web:** Update dockerfile ([10d9cc5](10d9cc5))
    
    ### 🔨 Code Refactoring
    
    * **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([#520](#520)) ([7c8ee5d](7c8ee5d))
    muntaxir4 pushed a commit to muntaxir4/keyshade that referenced this pull request Jan 1, 2025
    ## [2.8.0](keyshade-xyz/keyshade@v2.7.0...v2.8.0) (2024-12-03)
    
    ### 🚀 Features
    
    * **api:** Add workspace removal notification email template ([keyshade-xyz#476](keyshade-xyz#476)) ([40b754f](keyshade-xyz@40b754f))
    * **cli:** Store `metrics_enabled` key in profile config ([keyshade-xyz#536](keyshade-xyz#536)) ([9283b22](keyshade-xyz@9283b22))
    * **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([keyshade-xyz#557](keyshade-xyz#557)) ([126d024](keyshade-xyz@126d024))
    * **platform:** Added screen for CREATE NEW PROJECT ([keyshade-xyz#540](keyshade-xyz#540)) ([b644633](keyshade-xyz@b644633))
    * **platform:** Updated the empty state of dashboard ([keyshade-xyz#522](keyshade-xyz#522)) ([28739d9](keyshade-xyz@28739d9))
    * **schema, api-client:** Migrate auth types to @keyshade/schema ([keyshade-xyz#532](keyshade-xyz#532)) ([d880098](keyshade-xyz@d880098))
    * **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([keyshade-xyz#546](keyshade-xyz#546)) ([a3267de](keyshade-xyz@a3267de))
    * **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([keyshade-xyz#547](keyshade-xyz#547)) ([08868c3](keyshade-xyz@08868c3))
    * **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([keyshade-xyz#538](keyshade-xyz#538)) ([c468af0](keyshade-xyz@c468af0))
    * **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([keyshade-xyz#539](keyshade-xyz#539)) ([bc3100b](keyshade-xyz@bc3100b))
    * **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([keyshade-xyz#535](keyshade-xyz#535)) ([c24695e](keyshade-xyz@c24695e))
    * **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([keyshade-xyz#545](keyshade-xyz#545)) ([0ee8f9a](keyshade-xyz@0ee8f9a))
    * **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([keyshade-xyz#569](keyshade-xyz#569)) ([4398969](keyshade-xyz@4398969))
    * **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([keyshade-xyz#568](keyshade-xyz#568)) ([9efbf2d](keyshade-xyz@9efbf2d))
    * **schema:** Add User type inference from UserSchema ([keyshade-xyz#574](keyshade-xyz#574)) ([84c1db5](keyshade-xyz@84c1db5))
    
    ### 🐛 Bug Fixes
    
    * **api:** Incorrect oauth redirect url ([58d96e5](keyshade-xyz@58d96e5))
    * **platform:** Resolve loading SVG blocking input field interaction ([keyshade-xyz#571](keyshade-xyz#571)) ([30f4f65](keyshade-xyz@30f4f65))
    
    ### 📚 Documentation
    
    * Add pictures to Bruno setup ([keyshade-xyz#541](keyshade-xyz#541)) ([210c0fd](keyshade-xyz@210c0fd))
    * Migrate to Bruno ([keyshade-xyz#525](keyshade-xyz#525)) ([1793d92](keyshade-xyz@1793d92))
    
    ### 🔧 Miscellaneous Chores
    
    * **ci:** Add script to validate schema package ([59e4280](keyshade-xyz@59e4280))
    * Fixed codecov client version ([a998ae4](keyshade-xyz@a998ae4))
    * **package:** Fixed tests and did housekeeping ([keyshade-xyz#544](keyshade-xyz#544)) ([40008e3](keyshade-xyz@40008e3))
    * Update test coverage settings ([5b27e32](keyshade-xyz@5b27e32))
    * Update Turbo to 2.3.1 ([keyshade-xyz#564](keyshade-xyz#564)) ([3a63823](keyshade-xyz@3a63823))
    * **web:** Update dockerfile ([10d9cc5](keyshade-xyz@10d9cc5))
    
    ### 🔨 Code Refactoring
    
    * **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([keyshade-xyz#520](keyshade-xyz#520)) ([7c8ee5d](keyshade-xyz@7c8ee5d))
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants