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

chore(api): Added type inference and runtime validation to process.env #200

Merged
merged 7 commits into from
May 11, 2024

Conversation

jamesfrye420
Copy link
Contributor

@jamesfrye420 jamesfrye420 commented May 4, 2024

User description

Feat: Added run time zod validation and type inference to Process.env
Chore: updated .env.example with REDIS Creds
Chore: added cross-env for cross compatibility between windows and linux env vars in package.json scripts.

Fixes #199 #179 #184

Dependencies

Zod - for runtime schema Validation
cross-env - for setting variables in package.json scripts across linux and windows.

Future Improvements

TODO: To add regex validation as indicated in the Schema File of Env.

Developer's checklist

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

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.

Type

enhancement, documentation


Description

  • Integrated Zod for runtime validation of environment variables in app.module.ts, enhancing configuration robustness.
  • Defined a comprehensive Zod schema in env.schema.ts for validating and documenting expected environment variables.
  • Extended the ProcessEnv interface to type-check environment variables against the Zod schema.
  • Updated .env.example and package.json to support new environment configurations and cross-platform compatibility.
  • Added necessary dependencies (zod, cross-env) for the new features.

Changes walkthrough

Relevant files
Enhancement
app.module.ts
Integrate EnvSchema for Environment Variable Validation   

apps/api/src/app/app.module.ts

  • Integrated EnvSchema for runtime validation of environment variables
    in the configuration module.
  • Added validation options to abort early and disallow unknown
    properties.
  • +7/-1     
    env.schema.ts
    Define Zod Schema for Environment Variables                           

    apps/api/src/common/env/env.schema.ts

  • Defined a new Zod schema for environment variables.
  • Included optional and mandatory fields with detailed comments on
    handling empty strings and optional fields.
  • +60/-0   
    env.d.ts
    Extend ProcessEnv with EnvSchema Types                                     

    apps/api/src/common/env/env.d.ts

  • Extended ProcessEnv interface to include types defined in EnvSchema.
  • +8/-0     
    Configuration changes
    .env.example
    Update .env Example with Redis Variables                                 

    .env.example

    • Added example environment variables for Redis.
    +3/-0     
    Dependencies
    package.json
    Add Zod to API Package Dependencies                                           

    apps/api/package.json

    • Added zod to dependencies for schema validation.
    +2/-1     
    package.json
    Integrate cross-env for Cross-Platform Compatibility         

    package.json

  • Added cross-env to dependencies for cross-platform environment
    variable support.
  • Updated scripts to use cross-env for setting environment variables.
  • +4/-3     
    pnpm-lock.yaml
    Update Lock File with New Dependencies                                     

    pnpm-lock.yaml

    • Updated lock file with new dependencies cross-env and zod.
    +18/-0   

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

    @jamesfrye420 jamesfrye420 requested a review from rajdip-b as a code owner May 4, 2024 18:05
    @codiumai-pr-agent-free codiumai-pr-agent-free bot added type: documentation Improvements or additions to documentation type: enhancement New feature or request labels May 4, 2024
    Copy link
    Contributor

    PR Description updated to latest commit (0339e74)

    Copy link
    Contributor

    PR Review

    ⏱️ Estimated effort to review [1-5]

    3, because the PR introduces significant changes including runtime validation of environment variables, updates to configuration files, and modifications to existing TypeScript code. The complexity of the changes, especially with the integration of Zod for schema validation, requires a thorough review to ensure everything is correctly implemented and no regressions are introduced.

    🧪 Relevant tests

    Yes

    🔍 Possible issues

    Possible Bug: The Zod schema validation in env.schema.ts might not properly handle cases where environment variables are set to empty strings, which could lead to unexpected behavior if not managed correctly.

    Code Consistency: The removal of semicolons in some TypeScript files might lead to inconsistencies in coding style, which could affect maintainability.

    🔒 Security concerns

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a custom error message for environment variable validation failures.

    Consider adding a custom error message for the validation failure in the ConfigModule
    configuration. This will help developers understand what went wrong when the environment
    variables do not meet the expected schema.

    apps/api/src/app/app.module.ts [31-35]

    -validate: EnvSchema.parse,
    +validate: (config) => {
    +  try {
    +    return EnvSchema.parse(config);
    +  } catch (e) {
    +    throw new Error(`Validation error: ${e.message}`);
    +  }
    +},
     validationOptions: {
       allowUnknown: false,
       abortEarly: true
     }
     
    Add a test case for strings with leading or trailing spaces in the validation pipe tests.

    Add a test case to specifically check for strings that contain leading or trailing spaces,
    ensuring the AlphanumericReasonValidationPipe handles them correctly.

    apps/api/src/common/alphanumeric-reason-pipe.spec.ts [16-18]

    -it('should not allow strings with only spaces', () => {
    -  expect(() => pipe.transform('   ')).toThrow(BadRequestException)
    +it('should not allow strings with leading or trailing spaces', () => {
    +  expect(() => pipe.transform('  Test123  ')).toThrow(BadRequestException)
     })
     
    Bug
    Ensure no leading or trailing spaces are allowed by trimming the input in the validation pipe.

    Refactor the regular expression to ensure it does not allow leading or trailing spaces in
    the AlphanumericReasonValidationPipe. The current implementation might not fully enforce
    this requirement.

    apps/api/src/common/alphanumeric-reason-pipe.ts [6-11]

    -if (/^[a-zA-Z0-9]+(?: [a-zA-Z0-9]+)*$/.test(value)) {
    -  return value
    +if (/^[a-zA-Z0-9]+(?: [a-zA-Z0-9]+)*$/.test(value.trim())) {
    +  return value.trim()
     } else {
       throw new BadRequestException(
         'Reason must contain only alphanumeric characters and no leading or trailing spaces.'
       )
     }
     
    Security
    Enforce a minimum length for the REDIS_PASSWORD to prevent empty values.

    Add a minimum length requirement to the REDIS_PASSWORD field in the EnvSchema to ensure it
    is not empty, addressing the issue noted in the comments about Zod's behavior with empty
    strings.

    apps/api/src/common/env/env.schema.ts [26]

    -REDIS_PASSWORD: z.string().optional(),
    +REDIS_PASSWORD: z.string().min(1).optional(),
     
    Best practice
    Use specific Zod validators for email and URL fields to ensure proper formats.

    Consider using more specific Zod validators for email and URL fields in EnvSchema to
    ensure the values conform to expected formats.

    apps/api/src/common/env/env.schema.ts [23-59]

    -ADMIN_EMAIL: z.string(),
    +ADMIN_EMAIL: z.string().email(),
     WEB_FRONTEND_URL: z.string().url(),
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @rajdip-b rajdip-b changed the title Added type Inference and Runtime validation to Process.env chore(api): Added type inference and runtime validation to process.env May 5, 2024
    apps/api/package.json Outdated Show resolved Hide resolved
    apps/api/src/common/env/env.schema.ts Outdated Show resolved Hide resolved
    apps/api/src/common/env/env.schema.ts Show resolved Hide resolved
    @rajdip-b
    Copy link
    Member

    rajdip-b commented May 6, 2024

    @jamesfrye420 could you please make the 2 required changes? We can merge it asap!

    @rajdip-b
    Copy link
    Member

    /review

    Chore: updated .env.example with REDIS Creds
    Chore: added cross-env for windows and linux shell support
    Chore: Updated Prisma Client to latest
    Chore: Made SMTP creds non optional
    Chore: Removed Prisma and @prisma/client to api and migrated scripts accordingly
    Copy link

    Quality Gate Passed Quality Gate passed

    Issues
    3 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    No data about Coverage
    0.0% Duplication on New Code

    See analysis details on SonarCloud

    @rajdip-b rajdip-b merged commit 249e07d into keyshade-xyz:develop May 11, 2024
    5 checks passed
    @jamesfrye420 jamesfrye420 deleted the feat/env-inference branch May 12, 2024 13:44
    rajdip-b pushed a commit that referenced this pull request May 12, 2024
    ## [1.3.0](v1.2.0...v1.3.0) (2024-05-12)
    
    ### 🚀 Features
    
    * Add approval support ([#158](#158)) ([e09ae60](e09ae60))
    * **api:** Add configuration live update support ([#181](#181)) ([f7d6684](f7d6684))
    * **api:** Add feature to export data of a workspace ([#152](#152)) ([46833aa](46833aa))
    * **api:** Add Integration support ([#203](#203)) ([f1ae87e](f1ae87e))
    * **api:** Add note to [secure] and variable ([#151](#151)) ([2e62351](2e62351))
    * **api:** Add OAuth redirection and polished authentication ([#212](#212)) ([d2968bc](d2968bc))
    * **api:** Add support for storing and managing variables ([#149](#149)) ([963a8ae](963a8ae))
    * **api:** Added GitLab OAuth ([#188](#188)) ([4d3bbe4](4d3bbe4))
    * **api:** Added validation for reason field ([#190](#190)) ([90b8ff2](90b8ff2))
    * **api:** Create default workspace on user's creation ([#182](#182)) ([3dc0c4c](3dc0c4c))
    * **api:** Reading `port` Dynamically ([#170](#170)) ([fd46e3e](fd46e3e))
    * **auth:** Add Google OAuth ([#156](#156)) ([cf387ea](cf387ea))
    * **web:** Added waitlist ([#168](#168)) ([1084c77](1084c77))
    * **web:** Landing revamp ([#165](#165)) ([0bc723b](0bc723b))
    
    ### 🐛 Bug Fixes
    
    * **web:** alignment issue in “Collaboration made easy” section ([#178](#178)) ([df5ca75](df5ca75))
    * **workspace:** delete duplicate tailwind config ([99d922a](99d922a))
    
    ### 📚 Documentation
    
    * add contributor list ([f37569a](f37569a))
    * Add integration docs ([#204](#204)) ([406ddb7](406ddb7))
    * Added integration docs to gitbook summary ([ab37530](ab37530))
    * **api:** Add swagger docs of API key controller ([#167](#167)) ([2910476](2910476))
    * **api:** Add swagger docs of User Controller ([#166](#166)) ([fd59522](fd59522))
    * fix typo in environment-variables.md ([#163](#163)) ([48294c9](48294c9))
    * Remove supabase from docs ([#169](#169)) ([eddbce8](eddbce8))
    * **setup:** replace NX with Turbo in setup instructions ([#175](#175)) ([af8a460](af8a460))
    * Update README.md ([b59f16b](b59f16b))
    * Update running-the-api.md ([177dbbf](177dbbf))
    * Update running-the-api.md ([#193](#193)) ([3d5bcac](3d5bcac))
    
    ### 🔧 Miscellaneous Chores
    
    * Added lockfile ([60a3b9b](60a3b9b))
    * Added lockfile ([6bb512c](6bb512c))
    * **api:** Added type inference and runtime validation to `process.env` ([#200](#200)) ([249e07d](249e07d))
    * **api:** Fixed prisma script env errors ([#209](#209)) ([8762354](8762354))
    * **API:** Refactor authority check functions in API ([#189](#189)) ([e9d710d](e9d710d))
    * **api:** Refactor user e2e tests ([b38d45a](b38d45a))
    * **ci:** Disabled api stage release ([97877c4](97877c4))
    * **ci:** Update stage deployment config ([868a6a1](868a6a1))
    * **codecov:** update api-e2e project coverage ([1e90d7e](1e90d7e))
    * **dockerfile:** Fixed web dockerfile ([6134bb2](6134bb2))
    * **docker:** Optimized web Dockerfile to reduct image size ([#173](#173)) ([444286a](444286a))
    * **release:** Downgraded package version ([c173fee](c173fee))
    * **release:** Fix failing release ([#213](#213)) ([40f64f3](40f64f3))
    * **release:** Install pnpm ([1081bea](1081bea))
    * **release:** Updated release commit ([b8958e7](b8958e7))
    * **release:** Updated release commit ([e270eb8](e270eb8))
    * Update deprecated husky Install command ([#202](#202)) ([e61102c](e61102c))
    * Upgrade @million/lint from 0.0.66 to 0.0.73 ([#172](#172)) ([dd43ed9](dd43ed9))
    * **web:** Updated fly memory config ([4debc66](4debc66))
    
    ### 🔨 Code Refactoring
    
    * **api:** Made events central to workspace ([#159](#159)) ([9bc00ae](9bc00ae))
    * **api:** Migrated to cookie based authentication ([#206](#206)) ([ad6911f](ad6911f))
    * **monorepo:** Migrate from nx to turbo ([#153](#153)) ([88b4b00](88b4b00))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 1.3.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type: documentation Improvements or additions to documentation type: enhancement New feature or request
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Use cross-env to set NODE_ENV and other env variables in package.json scripts
    2 participants