run/time/env/variables #2996
run/time/env/variables #2996
Conversation
|
WalkthroughThis pull request introduces a comprehensive update to environment variable management across multiple applications. The changes involve adding a new Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (8)
deploy/docker-compose-build.yml (1)
81-85: Maintain consistent environment variable naming.The
MODEvariable doesn't follow theVITE_prefix convention used by other variables. This inconsistency could lead to confusion.Consider renaming
MODEtoVITE_MODEfor consistency:- MODE: ${MODE} + VITE_MODE: ${VITE_MODE:-development}apps/kyb-app/global.d.ts (1)
1-3: Consider creating shared type definitions.This declaration is identical across multiple applications. Consider creating a shared types package to maintain consistency and reduce duplication.
Create a shared types package that can be imported by all applications:
// packages/shared-types/env.d.ts export interface EnvConfig { VITE_API_URL: string; VITE_API_KEY: string; VITE_ENVIRONMENT_NAME: string; VITE_KYB_DEFINITION_ID?: string; // kyb-app specific VITE_DEFAULT_EXAMPLE_TOKEN?: string; // kyb-app specific VITE_SENTRY_AUTH_TOKEN?: string; VITE_SENTRY_DSN?: string; [key: string]: string | undefined; } declare global { export var env: EnvConfig; }apps/kyb-app/index.html (1)
10-10: Consider adding integrity hash and defer/async attributes.The config.js script is loaded synchronously without integrity verification. Consider:
- Adding an integrity hash to prevent script tampering
- Using defer/async if possible to improve page load performance
apps/backoffice-v2/Dockerfile (1)
29-30: Consider using COPY --chmod instead of separate RUN chmod.While the current implementation is correct, you can optimize the Dockerfile by combining the COPY and chmod operations.
-COPY --from=dev /app/entrypoint.sh /app/entrypoint.sh -RUN chmod a+x /app/entrypoint.sh; +COPY --from=dev --chmod=755 /app/entrypoint.sh /app/entrypoint.shAlso applies to: 33-34
apps/workflows-dashboard/entrypoint.sh (1)
3-22: Add input validation for environment variables.The script sets default values but doesn't validate the format or content of provided values.
if [[ -z "$VITE_DOMAIN" ]] then VITE_DOMAIN="http://localhost:3000" +else + # Validate domain format + if ! echo "$VITE_DOMAIN" | grep -qE '^https?://[^/]+$'; then + echo "Error: VITE_DOMAIN must be a valid HTTP/HTTPS URL" + exit 1 + fi fiapps/kyb-app/entrypoint.sh (1)
1-46: Reduce code duplication across entrypoint scripts.This script shares similar code with other entrypoint scripts. Consider creating a shared script.
Create a shared script at the root level:
- Move common functions to
scripts/generate-config.sh- Source it from individual entrypoint scripts
- Only keep app-specific configurations in entrypoint scripts
apps/backoffice-v2/index.html (1)
11-11: Add defer attribute to config.js script.The script might block rendering. Consider adding the defer attribute if the configuration is not needed immediately.
- <script type="text/javascript" src="/config.js"></script> + <script type="text/javascript" src="/config.js" defer></script>apps/workflows-dashboard/package.json (1)
11-11: Use cross-platform clean command.While cleaning the dist directory before build is good practice,
rm -rfisn't cross-platform. Consider:
- Using
rimraffor cross-platform compatibility- Moving clean step to a separate script for better maintainability
- "build": "rm -rf dist && tsc && vite build", + "clean": "rimraf dist", + "build": "npm run clean && tsc && vite build",Don't forget to add rimraf to devDependencies:
"devDependencies": { + "rimraf": "^5.0.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/backoffice-v2/.env.example(1 hunks)apps/backoffice-v2/Dockerfile(1 hunks)apps/backoffice-v2/entrypoint.sh(1 hunks)apps/backoffice-v2/global.d.ts(1 hunks)apps/backoffice-v2/index.html(1 hunks)apps/backoffice-v2/public/config.js(1 hunks)apps/backoffice-v2/src/common/env/env.ts(1 hunks)apps/backoffice-v2/src/main.tsx(2 hunks)apps/kyb-app/Dockerfile(1 hunks)apps/kyb-app/entrypoint.sh(1 hunks)apps/kyb-app/global.d.ts(1 hunks)apps/kyb-app/index.html(1 hunks)apps/kyb-app/public/config.js(1 hunks)apps/kyb-app/src/common/utils/request.ts(1 hunks)apps/workflows-dashboard/Dockerfile(1 hunks)apps/workflows-dashboard/entrypoint.sh(1 hunks)apps/workflows-dashboard/global.d.ts(1 hunks)apps/workflows-dashboard/index.html(1 hunks)apps/workflows-dashboard/package.json(1 hunks)apps/workflows-dashboard/public/config.js(1 hunks)apps/workflows-dashboard/src/lib/request/request.ts(1 hunks)deploy/.env(1 hunks)deploy/docker-compose-build.yml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/backoffice-v2/.env.example
- apps/workflows-dashboard/index.html
- apps/workflows-dashboard/public/config.js
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: lint
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
deploy/docker-compose-build.yml (1)
81-85: Verify environment variable validation.Ensure that the
entrypoint.shscript properly validates these environment variables before starting the application.Run the following script to check the validation in entrypoint.sh:
✅ Verification successful
Environment variables are properly validated with defaults
The
entrypoint.shscript includes appropriate validation for all environment variables:
- Checks for empty values using
-ztest- Sets development-friendly defaults:
- VITE_DOMAIN → "http://localhost:3000"
- MODE → "development"
- VITE_ENVIRONMENT_NAME → "local"
- VITE_IMAGE_LOGO_URL → empty string
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if entrypoint.sh validates the new environment variables # Search for environment variable validation in entrypoint.sh rg -A 5 'VITE_DOMAIN|VITE_ENVIRONMENT_NAME|VITE_IMAGE_LOGO_URL|MODE' ../apps/workflows-dashboard/entrypoint.shLength of output: 194
Script:
#!/bin/bash # Find entrypoint.sh files echo "=== Searching for entrypoint.sh files ===" fd entrypoint.sh # Search for environment variable validation in Dockerfile and related files echo -e "\n=== Searching for environment variables in Docker files ===" rg -A 5 'VITE_DOMAIN|VITE_ENVIRONMENT_NAME|VITE_IMAGE_LOGO_URL|MODE' -g 'Dockerfile*' -g 'docker-compose*.yml' # Search for environment variable validation patterns in the codebase echo -e "\n=== Searching for environment variable validation patterns ===" rg -A 5 'process\.env\.(VITE_DOMAIN|VITE_ENVIRONMENT_NAME|VITE_IMAGE_LOGO_URL|MODE)' -g '*.{js,ts,jsx,tsx}'Length of output: 1740
Script:
#!/bin/bash # Check content of workflows-dashboard entrypoint.sh echo "=== Content of workflows-dashboard/entrypoint.sh ===" cat apps/workflows-dashboard/entrypoint.sh # Look for common validation patterns echo -e "\n=== Checking for validation patterns ===" rg -A 5 '(check|validate|require).*(env|environment)' apps/workflows-dashboard/Length of output: 891
apps/workflows-dashboard/global.d.ts (1)
1-3: Same type safety concerns as other applications.See previous comments about creating shared type definitions.
apps/backoffice-v2/src/common/env/env.ts (1)
13-13: Ensure environment variables are available before validationThe code assumes
globalThis.envis already initialized, but there's no guarantee of initialization order.Add initialization check and provide better error message:
-const _env = EnvSchema.safeParse(globalThis.env); +if (!globalThis.env) { + throw new Error('Environment variables not initialized. Ensure config.js is loaded before env.ts'); +} +const _env = EnvSchema.safeParse(globalThis.env);apps/backoffice-v2/src/main.tsx (1)
20-20: Verify config.js loading orderThe config.js import might not complete before the environment variables are accessed.
Consider moving the import to a separate initialization function:
async function initializeEnv() { await import('../public/config.js?url'); if (!globalThis.env) { throw new Error('Environment variables not initialized'); } }apps/kyb-app/src/common/utils/request.ts (1)
7-8: 🛠️ Refactor suggestionRemove @ts-ignore by adding proper type definitions.
The @ts-ignore comment suggests missing type definitions for
globalThis.env. This should be addressed by adding proper type definitions in a global.d.ts file.✅ Verification successful
Remove @ts-ignore as type definition exists
The
@ts-ignorecomment is unnecessary sinceglobalThis.envis already typed inglobal.d.ts. While the type could be more specific than{ [key: string]: any }, it's valid and allows the current usage.Optional improvement: Consider making the type definition more specific by listing expected environment variables:
declare global { export var env: { VITE_API_URL?: string; // ... other env vars }; }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if proper type definitions exist for globalThis.env echo "Checking for type definitions..." fd -t f "global.d.ts" "apps/kyb-app/"Length of output: 135
Script:
#!/bin/bash # Check contents of global.d.ts for globalThis.env definitions cat "apps/kyb-app/global.d.ts"Length of output: 91
apps/workflows-dashboard/Dockerfile (1)
26-27: Verify entrypoint.sh existence and content.The Dockerfile assumes entrypoint.sh exists in the dev stage. Please ensure:
- The script exists in the correct location
- The script contains proper environment variable handling
Also applies to: 30-31, 34-35
✅ Verification successful
✓ Entrypoint script verified and properly configured
The entrypoint.sh script exists in the correct location and properly handles environment variables with appropriate defaults. The script correctly generates the runtime configuration and handles container commands.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if entrypoint.sh exists and verify its content echo "Checking for entrypoint.sh..." fd -t f "entrypoint.sh" "apps/workflows-dashboard/"Length of output: 157
Script:
#!/bin/bash # Check content of entrypoint.sh cat apps/workflows-dashboard/entrypoint.shLength of output: 597
apps/kyb-app/Dockerfile (2)
12-13: Verify build output and PATH modification.The build step has been added, but please ensure:
- The build output directory matches the COPY command in the prod stage
- The PATH modification is necessary (it might be redundant as node_modules/.bin is typically already in PATH)
Also applies to: 15-15
✅ Verification successful
Build configuration and PATH modification are correctly implemented
The build output directory (
/app/dist) matches the COPY command in the prod stage, and the PATH modification is necessary for the dev stage to ensure local binaries are accessible during development.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if build output directory exists and matches COPY command echo "Checking build configuration..." fd -t f "package.json" "apps/kyb-app/" -x jq .scripts.build {}Length of output: 155
Script:
#!/bin/bash # Check vite config and Dockerfile echo "Checking vite configuration..." fd -t f "vite.config" "apps/kyb-app/" echo -e "\nChecking Dockerfile prod stage..." cat apps/kyb-app/DockerfileLength of output: 842
27-28: Ensure entrypoint.sh configuration matches workflows-dashboard.The entrypoint configuration mirrors workflows-dashboard, which is good for consistency. Verify that:
- The environment variables handled in both entrypoint scripts are consistent
- The scripts follow the same error handling patterns
Also applies to: 31-32, 35-36
✅ Verification successful
Entrypoint scripts are structurally consistent but have application-specific variables
The scripts follow identical patterns for environment variable handling and error management. The differences in environment variables (kyb-app handles Sentry configuration while workflows-dashboard handles MODE and logo URL) are justified by different application requirements rather than inconsistency issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Compare entrypoint.sh files between applications echo "Comparing entrypoint.sh files..." fd -t f "entrypoint.sh" "apps/" -x md5sum {}Length of output: 320
Script:
#!/bin/bash echo "=== kyb-app/entrypoint.sh ===" cat apps/kyb-app/entrypoint.sh echo -e "\n=== workflows-dashboard/entrypoint.sh ===" cat apps/workflows-dashboard/entrypoint.shLength of output: 1673
apps/backoffice-v2/Dockerfile (2)
25-26: LGTM! Working directory setup follows Docker best practices.The WORKDIR directive is appropriately used to set up the container's working environment.
37-38: Verify entrypoint.sh exists in the dev stage.The Dockerfile copies entrypoint.sh from the dev stage, but we need to ensure it exists.
✅ Verification successful
Entrypoint script exists and is correctly located
The entrypoint.sh file exists at apps/backoffice-v2/entrypoint.sh, confirming it's available for the Dockerfile build process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if entrypoint.sh exists in the source code fd -t f "entrypoint.sh$" apps/backoffice-v2/Length of output: 79
| environment: | ||
| VITE_DOMAIN: ${VITE_DOMAIN} | ||
| VITE_ENVIRONMENT_NAME: ${VITE_ENVIRONMENT_NAME} | ||
| VITE_IMAGE_LOGO_URL: ${VITE_IMAGE_LOGO_URL} | ||
| MODE: ${MODE} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add default values and document environment variables.
The new environment variables lack default values and documentation. This could lead to deployment issues if variables are not properly set.
Apply this diff to add defaults and comments:
environment:
- VITE_DOMAIN: ${VITE_DOMAIN}
- VITE_ENVIRONMENT_NAME: ${VITE_ENVIRONMENT_NAME}
- VITE_IMAGE_LOGO_URL: ${VITE_IMAGE_LOGO_URL}
- MODE: ${MODE}
+ # Domain for API endpoints
+ VITE_DOMAIN: ${VITE_DOMAIN:-localhost}
+ # Environment name (e.g., development, staging, production)
+ VITE_ENVIRONMENT_NAME: ${VITE_ENVIRONMENT_NAME:-development}
+ # URL for the logo image
+ VITE_IMAGE_LOGO_URL: ${VITE_IMAGE_LOGO_URL:-/logo.png}
+ # Application mode
+ MODE: ${MODE:-development}📝 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.
| environment: | |
| VITE_DOMAIN: ${VITE_DOMAIN} | |
| VITE_ENVIRONMENT_NAME: ${VITE_ENVIRONMENT_NAME} | |
| VITE_IMAGE_LOGO_URL: ${VITE_IMAGE_LOGO_URL} | |
| MODE: ${MODE} | |
| environment: | |
| # Domain for API endpoints | |
| VITE_DOMAIN: ${VITE_DOMAIN:-localhost} | |
| # Environment name (e.g., development, staging, production) | |
| VITE_ENVIRONMENT_NAME: ${VITE_ENVIRONMENT_NAME:-development} | |
| # URL for the logo image | |
| VITE_IMAGE_LOGO_URL: ${VITE_IMAGE_LOGO_URL:-/logo.png} | |
| # Application mode | |
| MODE: ${MODE:-development} |
| declare global { | ||
| export var env: { [key: string]: any }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a strongly typed interface instead of any.
Using { [key: string]: any } reduces type safety. Consider creating a specific interface that defines the expected environment variables and their types.
+interface EnvConfig {
+ VITE_API_URL: string;
+ VITE_API_KEY: string;
+ VITE_ENVIRONMENT_NAME: string;
+ VITE_SENTRY_AUTH_TOKEN?: string;
+ VITE_SENTRY_DSN?: string;
+ [key: string]: string | undefined; // for other env vars
+}
+
declare global {
- export var env: { [key: string]: any };
+ export var env: EnvConfig;
}📝 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.
| declare global { | |
| export var env: { [key: string]: any }; | |
| } | |
| interface EnvConfig { | |
| VITE_API_URL: string; | |
| VITE_API_KEY: string; | |
| VITE_ENVIRONMENT_NAME: string; | |
| VITE_SENTRY_AUTH_TOKEN?: string; | |
| VITE_SENTRY_DSN?: string; | |
| [key: string]: string | undefined; // for other env vars | |
| } | |
| declare global { | |
| export var env: EnvConfig; | |
| } |
apps/kyb-app/public/config.js
Outdated
| globalThis.env = { | ||
| VITE_API_URL: 'http://localhost:3000/api/v1/', | ||
| VITE_KYB_DEFINITION_ID: 'kyb_parent_kyc_session_example', | ||
| VITE_API_KEY: 'secret', | ||
| VITE_ENVIRONMENT_NAME: 'local', | ||
| VITE_DEFAULT_EXAMPLE_TOKEN: '12345678-1234-1234-1234-123456789012', | ||
| VITE_SENTRY_AUTH_TOKEN: '', | ||
| VITE_SENTRY_DSN: '', | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add runtime validation for environment variables.
Consider adding runtime validation to ensure all required environment variables are present and have the correct format.
// validation.ts
import { z } from 'zod';
const envSchema = z.object({
VITE_API_URL: z.string().url(),
VITE_API_KEY: z.string().min(1),
VITE_ENVIRONMENT_NAME: z.enum(['development', 'staging', 'production']),
VITE_KYB_DEFINITION_ID: z.string().min(1),
VITE_DEFAULT_EXAMPLE_TOKEN: z.string().optional(),
VITE_SENTRY_AUTH_TOKEN: z.string().optional(),
VITE_SENTRY_DSN: z.string().optional(),
});
export function validateEnv(env: unknown) {
return envSchema.parse(env);
}💡 Codebase verification
Security: Remove hardcoded sensitive values but use build-time injection instead of process.env
The security concern about hardcoded sensitive values is valid, but the suggested fix needs adjustment:
-
Found multiple config files with hardcoded API keys and tokens:
apps/kyb-app/public/config.jsapps/backoffice-v2/public/config.js
-
Instead of using
process.env(which won't work in browser), use build-time environment injection:
globalThis.env = {
- VITE_API_URL: 'http://localhost:3000/api/v1/',
- VITE_KYB_DEFINITION_ID: 'kyb_parent_kyc_session_example',
- VITE_API_KEY: 'secret',
- VITE_ENVIRONMENT_NAME: 'local',
- VITE_DEFAULT_EXAMPLE_TOKEN: '12345678-1234-1234-1234-123456789012',
+ VITE_API_URL: import.meta.env.VITE_API_URL,
+ VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID,
+ VITE_API_KEY: import.meta.env.VITE_API_KEY,
+ VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'development',
+ VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN,
VITE_SENTRY_AUTH_TOKEN: '',
VITE_SENTRY_DSN: '',
};🔗 Analysis chain
Security: Remove sensitive information from source control.
Several security concerns in the default configuration:
- API key should never be committed, even if it's a development value
- Example tokens should be clearly marked as non-production
- Localhost URL might cause issues in different environments
Consider this safer approach:
globalThis.env = {
- VITE_API_URL: 'http://localhost:3000/api/v1/',
- VITE_KYB_DEFINITION_ID: 'kyb_parent_kyc_session_example',
- VITE_API_KEY: 'secret',
- VITE_ENVIRONMENT_NAME: 'local',
- VITE_DEFAULT_EXAMPLE_TOKEN: '12345678-1234-1234-1234-123456789012',
+ VITE_API_URL: process.env.VITE_API_URL || '',
+ VITE_KYB_DEFINITION_ID: process.env.VITE_KYB_DEFINITION_ID || '',
+ VITE_API_KEY: process.env.VITE_API_KEY || '',
+ VITE_ENVIRONMENT_NAME: process.env.VITE_ENVIRONMENT_NAME || 'development',
+ VITE_DEFAULT_EXAMPLE_TOKEN: process.env.VITE_DEFAULT_EXAMPLE_TOKEN || 'example-only-do-not-use',
VITE_SENTRY_AUTH_TOKEN: '',
VITE_SENTRY_DSN: '',
};Let's verify if sensitive information exists in other configuration files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential hardcoded secrets in config files
echo "Checking for potential secrets in config files..."
rg -g '*.js' -g '*.ts' -g '*.json' -i '(secret|token|key|password|credential)' apps/
Length of output: 66330
apps/backoffice-v2/public/config.js
Outdated
| globalThis.env = { | ||
| VITE_API_URL: 'http://localhost:3000/api/v1/internal', | ||
| VITE_KYB_DEFINITION_ID: 'kyb_parent_kyc_session_example', | ||
| VITE_API_KEY: 'secret', | ||
| VITE_ENVIRONMENT_NAME: 'local', | ||
| VITE_DEFAULT_EXAMPLE_TOKEN: '12345678-1234-1234-1234-123456789012', | ||
| VITE_SENTRY_AUTH_TOKEN: '', | ||
| VITE_SENTRY_DSN: '', | ||
| }; |
There was a problem hiding this comment.
Critical: Remove hardcoded sensitive values from client-side code
Storing sensitive values like API keys and URLs directly in client-side JavaScript poses several security risks:
- Values are exposed in source code
- Configuration can't be changed without rebuilding
- Different environments need different values
Consider these alternatives:
- Use environment variable substitution at build time
- Inject values at runtime through server-side rendered script
- Move sensitive values to secure backend endpoints
Example implementation using runtime injection:
globalThis.env = {
VITE_API_URL: '{{VITE_API_URL}}',
VITE_KYB_DEFINITION_ID: '{{VITE_KYB_DEFINITION_ID}}',
VITE_API_KEY: '{{VITE_API_KEY}}',
VITE_ENVIRONMENT_NAME: '{{VITE_ENVIRONMENT_NAME}}',
VITE_DEFAULT_EXAMPLE_TOKEN: '{{VITE_DEFAULT_EXAMPLE_TOKEN}}',
VITE_SENTRY_AUTH_TOKEN: '{{VITE_SENTRY_AUTH_TOKEN}}',
VITE_SENTRY_DSN: '{{VITE_SENTRY_DSN}}',
};| //@ts-ignore | ||
| baseURL: globalThis.env.VITE_API_URL, |
There was a problem hiding this comment.
Fix type safety and ensure consistent environment variable access
- The
@ts-ignoredirective bypasses type checking, which could lead to runtime errors - Inconsistent environment variable access: using
globalThis.envfor baseURL butimport.meta.envin the interceptor
Consider these improvements:
- Add proper TypeScript types for globalThis.env
- Use consistent environment variable access method
- Add runtime checks for required environment variables
- //@ts-ignore
- baseURL: globalThis.env.VITE_API_URL,
+ baseURL: globalThis.env?.VITE_API_URL ?? throw new Error('VITE_API_URL is required'),Also update the interceptor to use the same access method:
- config.headers['Authorization'] = `Api-Key ${import.meta.env.VITE_API_KEY}`;
+ config.headers['Authorization'] = `Api-Key ${globalThis.env?.VITE_API_KEY}`;Committable suggestion skipped: line range outside the PR's diff.
apps/backoffice-v2/entrypoint.sh
Outdated
| if [[ -z "$VITE_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_POLLING_INTERVAL=10 | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_ASSIGNMENT_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_ASSIGNMENT_POLLING_INTERVAL=5 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for numeric environment variables.
The polling intervals should be validated to ensure:
- Values are numeric
- Values are within reasonable bounds
- ASSIGNMENT_POLLING_INTERVAL should not exceed POLLING_INTERVAL
if [[ -z "$VITE_POLLING_INTERVAL" ]]
then
VITE_POLLING_INTERVAL=10
+else
+ if ! [[ "$VITE_POLLING_INTERVAL" =~ ^[0-9]+$ ]]; then
+ echo "Error: VITE_POLLING_INTERVAL must be a positive integer"
+ exit 1
+ fi
+ if (( VITE_POLLING_INTERVAL < 5 || VITE_POLLING_INTERVAL > 300 )); then
+ echo "Error: VITE_POLLING_INTERVAL must be between 5 and 300 seconds"
+ exit 1
+ fi
fiApply similar validation to VITE_ASSIGNMENT_POLLING_INTERVAL.
Committable suggestion skipped: line range outside the PR's diff.
apps/backoffice-v2/entrypoint.sh
Outdated
| #!/usr/bin/env sh | ||
|
|
||
| if [[ -z "$VITE_DOMAIN" ]] | ||
| then | ||
| VITE_DOMAIN="http://localhost:3000" | ||
| fi |
There was a problem hiding this comment.
Fix shell compatibility and review default domain.
- The script uses
shbut contains bash-specific syntax ([[]]). Either change the shebang to#!/bin/bashor use POSIX-compatible[for better portability. - The default domain points to localhost which might not be suitable for production environments.
-#!/usr/bin/env sh
+#!/usr/bin/env bash📝 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.
| #!/usr/bin/env sh | |
| if [[ -z "$VITE_DOMAIN" ]] | |
| then | |
| VITE_DOMAIN="http://localhost:3000" | |
| fi | |
| #!/usr/bin/env bash | |
| if [[ -z "$VITE_DOMAIN" ]] | |
| then | |
| VITE_DOMAIN="http://localhost:3000" | |
| fi |
apps/backoffice-v2/entrypoint.sh
Outdated
| cat << EOF > /usr/share/nginx/html/config.js | ||
| globalThis.env = { | ||
| VITE_API_URL: "$VITE_DOMAIN/api/v1/internal", | ||
| VITE_API_KEY: "$VITE_API_KEY", | ||
| VITE_AUTH_ENABLED: "$VITE_AUTH_ENABLED", | ||
| VITE_MOCK_SERVER: "$VITE_MOCK_SERVER", | ||
| VITE_POLLING_INTERVAL: "$VITE_POLLING_INTERVAL", | ||
| VITE_ASSIGNMENT_POLLING_INTERVAL: "$VITE_ASSIGNMENT_POLLING_INTERVAL", | ||
| VITE_FETCH_SIGNED_URL: "$VITE_FETCH_SIGNED_URL", | ||
| VITE_ENVIRONMENT_NAME: "local", | ||
| MODE: "production" | ||
| } | ||
| EOF |
There was a problem hiding this comment.
Security: Protect sensitive data and improve configuration handling.
Several issues with the config.js generation:
- Sensitive data (API key) is exposed to the client
- Environment variable values aren't escaped, potentially breaking the JS
- Environment name and mode are hardcoded, ignoring actual environment settings
cat << EOF > /usr/share/nginx/html/config.js
globalThis.env = {
VITE_API_URL: "$VITE_DOMAIN/api/v1/internal",
- VITE_API_KEY: "$VITE_API_KEY",
+ // Don't expose API key to client
VITE_AUTH_ENABLED: "$VITE_AUTH_ENABLED",
VITE_MOCK_SERVER: "$VITE_MOCK_SERVER",
VITE_POLLING_INTERVAL: "$VITE_POLLING_INTERVAL",
VITE_ASSIGNMENT_POLLING_INTERVAL: "$VITE_ASSIGNMENT_POLLING_INTERVAL",
VITE_FETCH_SIGNED_URL: "$VITE_FETCH_SIGNED_URL",
- VITE_ENVIRONMENT_NAME: "local",
- MODE: "production"
+ VITE_ENVIRONMENT_NAME: "${VITE_ENVIRONMENT_NAME:-local}",
+ MODE: "${MODE:-production}"
}
EOF📝 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.
| cat << EOF > /usr/share/nginx/html/config.js | |
| globalThis.env = { | |
| VITE_API_URL: "$VITE_DOMAIN/api/v1/internal", | |
| VITE_API_KEY: "$VITE_API_KEY", | |
| VITE_AUTH_ENABLED: "$VITE_AUTH_ENABLED", | |
| VITE_MOCK_SERVER: "$VITE_MOCK_SERVER", | |
| VITE_POLLING_INTERVAL: "$VITE_POLLING_INTERVAL", | |
| VITE_ASSIGNMENT_POLLING_INTERVAL: "$VITE_ASSIGNMENT_POLLING_INTERVAL", | |
| VITE_FETCH_SIGNED_URL: "$VITE_FETCH_SIGNED_URL", | |
| VITE_ENVIRONMENT_NAME: "local", | |
| MODE: "production" | |
| } | |
| EOF | |
| cat << EOF > /usr/share/nginx/html/config.js | |
| globalThis.env = { | |
| VITE_API_URL: "$VITE_DOMAIN/api/v1/internal", | |
| // Don't expose API key to client | |
| VITE_AUTH_ENABLED: "$VITE_AUTH_ENABLED", | |
| VITE_MOCK_SERVER: "$VITE_MOCK_SERVER", | |
| VITE_POLLING_INTERVAL: "$VITE_POLLING_INTERVAL", | |
| VITE_ASSIGNMENT_POLLING_INTERVAL: "$VITE_ASSIGNMENT_POLLING_INTERVAL", | |
| VITE_FETCH_SIGNED_URL: "$VITE_FETCH_SIGNED_URL", | |
| VITE_ENVIRONMENT_NAME: "${VITE_ENVIRONMENT_NAME:-local}", | |
| MODE: "${MODE:-production}" | |
| } | |
| EOF |
apps/backoffice-v2/entrypoint.sh
Outdated
| if [[ -z "$VITE_API_KEY" ]] | ||
| then | ||
| VITE_API_KEY="secret" | ||
| fi |
There was a problem hiding this comment.
Security: Remove hardcoded API key and add validation.
Hardcoding a default API key value is a security risk. The script should:
- Fail fast if no API key is provided in production
- Validate the API key format/length
if [[ -z "$VITE_API_KEY" ]]
then
- VITE_API_KEY="secret"
+ if [[ "${MODE}" == "production" ]]; then
+ echo "Error: VITE_API_KEY must be set in production"
+ exit 1
+ fi
+ VITE_API_KEY="development-only-key"
fiCommittable suggestion skipped: line range outside the PR's diff.
apps/backoffice-v2/entrypoint.sh
Outdated
| if [[ -z "$VITE_AUTH_ENABLED" ]] | ||
| then | ||
| VITE_AUTH_ENABLED=true | ||
| fi | ||
|
|
||
|
|
||
| if [[ -z "$VITE_MOCK_SERVER" ]] | ||
| then | ||
| VITE_MOCK_SERVER=false | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize boolean environment variables.
The script should normalize boolean values to prevent issues with string comparison:
- Convert input to lowercase
- Validate against known boolean values
- Normalize output to "true"/"false"
if [[ -z "$VITE_AUTH_ENABLED" ]]
then
VITE_AUTH_ENABLED=true
+else
+ # Convert to lowercase
+ VITE_AUTH_ENABLED=$(echo "$VITE_AUTH_ENABLED" | tr '[:upper:]' '[:lower:]')
+ # Validate and normalize
+ case "$VITE_AUTH_ENABLED" in
+ true|1|yes) VITE_AUTH_ENABLED=true ;;
+ false|0|no) VITE_AUTH_ENABLED=false ;;
+ *) echo "Error: VITE_AUTH_ENABLED must be a boolean value"; exit 1 ;;
+ esac
fiApply similar changes to other boolean variables (VITE_MOCK_SERVER, VITE_FETCH_SIGNED_URL).
📝 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.
| if [[ -z "$VITE_AUTH_ENABLED" ]] | |
| then | |
| VITE_AUTH_ENABLED=true | |
| fi | |
| if [[ -z "$VITE_MOCK_SERVER" ]] | |
| then | |
| VITE_MOCK_SERVER=false | |
| fi | |
| if [[ -z "$VITE_AUTH_ENABLED" ]] | |
| then | |
| VITE_AUTH_ENABLED=true | |
| else | |
| # Convert to lowercase | |
| VITE_AUTH_ENABLED=$(echo "$VITE_AUTH_ENABLED" | tr '[:upper:]' '[:lower:]') | |
| # Validate and normalize | |
| case "$VITE_AUTH_ENABLED" in | |
| true|1|yes) VITE_AUTH_ENABLED=true ;; | |
| false|0|no) VITE_AUTH_ENABLED=false ;; | |
| *) echo "Error: VITE_AUTH_ENABLED must be a boolean value"; exit 1 ;; | |
| esac | |
| fi | |
| if [[ -z "$VITE_MOCK_SERVER" ]] | |
| then | |
| VITE_MOCK_SERVER=false | |
| else | |
| # Convert to lowercase | |
| VITE_MOCK_SERVER=$(echo "$VITE_MOCK_SERVER" | tr '[:upper:]' '[:lower:]') | |
| # Validate and normalize | |
| case "$VITE_MOCK_SERVER" in | |
| true|1|yes) VITE_MOCK_SERVER=true ;; | |
| false|0|no) VITE_MOCK_SERVER=false ;; | |
| *) echo "Error: VITE_MOCK_SERVER must be a boolean value"; exit 1 ;; | |
| esac | |
| fi |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/workflows-dashboard/entrypoint.sh (2)
1-2: 🛠️ Refactor suggestionAdd shell safety options.
Add shell safety options to prevent common shell script issues.
#!/usr/bin/env sh +set -euo pipefail
25-32:⚠️ Potential issueSecurity: Avoid exposing sensitive configuration in client-side JavaScript.
The script generates a client-side JavaScript file with environment variables. This approach might expose sensitive configuration to client-side code.
Consider:
- Only exposing necessary non-sensitive variables to the client
- Using server-side configuration for sensitive data
- Implementing proper API endpoints for required dynamic configuration
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/workflows-dashboard/entrypoint.sh(1 hunks)apps/workflows-dashboard/src/lib/request/request.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/workflows-dashboard/src/lib/request/request.ts
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/workflows-dashboard/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test_windows
- GitHub Check: build (windows-latest)
- GitHub Check: test_linux
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
- GitHub Check: spell_check
🔇 Additional comments (1)
apps/workflows-dashboard/entrypoint.sh (1)
34-35: LGTM!The command execution follows container entrypoint best practices by using
exec "$@"to properly handle signals and process management.
| if [[ -n "$VITE_DOMAIN" ]] | ||
| then | ||
| VITE_API_URL="$VITE_DOMAIN/api/v1/" | ||
| fi | ||
|
|
||
| if [[ -n "$MODE" ]] | ||
| then | ||
| MODE="$MODE" | ||
| fi | ||
|
|
||
| if [[ -n "$VITE_IMAGE_LOGO_URL" ]] | ||
| then | ||
| VITE_IMAGE_LOGO_URL="$VITE_IMAGE_LOGO_URL" | ||
| fi | ||
|
|
||
|
|
||
| if [[ -n "$VITE_ENVIRONMENT_NAME" ]] | ||
| then | ||
| VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix POSIX compatibility and simplify variable handling.
- The [[ ]] syntax is not POSIX compliant. Use [ ] for better compatibility.
- The MODE variable block is redundant as it just assigns the variable to itself.
-if [[ -n "$VITE_DOMAIN" ]]
+if [ -n "$VITE_DOMAIN" ]
then
VITE_API_URL="$VITE_DOMAIN/api/v1/"
fi
-if [[ -n "$MODE" ]]
-then
- MODE="$MODE"
-fi
-if [[ -n "$VITE_IMAGE_LOGO_URL" ]]
+if [ -n "$VITE_IMAGE_LOGO_URL" ]
then
VITE_IMAGE_LOGO_URL="$VITE_IMAGE_LOGO_URL"
fi
-if [[ -n "$VITE_ENVIRONMENT_NAME" ]]
+if [ -n "$VITE_ENVIRONMENT_NAME" ]
then
VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME"
fi📝 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.
| if [[ -n "$VITE_DOMAIN" ]] | |
| then | |
| VITE_API_URL="$VITE_DOMAIN/api/v1/" | |
| fi | |
| if [[ -n "$MODE" ]] | |
| then | |
| MODE="$MODE" | |
| fi | |
| if [[ -n "$VITE_IMAGE_LOGO_URL" ]] | |
| then | |
| VITE_IMAGE_LOGO_URL="$VITE_IMAGE_LOGO_URL" | |
| fi | |
| if [[ -n "$VITE_ENVIRONMENT_NAME" ]] | |
| then | |
| VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" | |
| fi | |
| if [ -n "$VITE_DOMAIN" ] | |
| then | |
| VITE_API_URL="$VITE_DOMAIN/api/v1/" | |
| fi | |
| if [ -n "$VITE_IMAGE_LOGO_URL" ] | |
| then | |
| VITE_IMAGE_LOGO_URL="$VITE_IMAGE_LOGO_URL" | |
| fi | |
| if [ -n "$VITE_ENVIRONMENT_NAME" ] | |
| then | |
| VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
pratapalakshmi
force-pushed
the
run/time/env/variables
branch
from
9b28afe to
3d8ddef
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/workflows-dashboard/entrypoint.sh (4)
1-2: Enhance Shell Safety OptionsConsider adding shell safety options immediately after the shebang to improve script robustness. For example, add:
+set -euo pipefailThis catches unset variables, errors, and failures in pipelines.
8-11: Eliminate Redundant MODE Block & Use POSIX TestThe block checking
MODEis redundant since it simply assigns the variable to itself. Also, update the test for POSIX compliance. You could remove this block entirely unless a transformation is intended:-if [ -n "$MODE" ] -then - MODE="$MODE" -fiIf no processing is needed, it's best to remove these lines.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
13-16: Refactor VITE_IMAGE_LOGO_URL Check for POSIX CompatibilitySimilarly, update the test to use POSIX-compliant syntax. Also, the assignment is a no-op. Consider removing the redundant block if no processing is required:
-if [[ -n "$VITE_IMAGE_LOGO_URL" ]] -then - VITE_IMAGE_LOGO_URL="$VITE_IMAGE_LOGO_URL" -fi +if [ -n "$VITE_IMAGE_LOGO_URL" ] +then + : # Variable already set; no reassignment needed. +fiAlternatively, remove the conditional block if it serves no purpose.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
19-22: Refactor VITE_ENVIRONMENT_NAME CheckApply the same POSIX syntax fix for this block and consider its necessity:
-if [[ -n "$VITE_ENVIRONMENT_NAME" ]] -then - VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" -fi +if [ -n "$VITE_ENVIRONMENT_NAME" ] +then + : # No transformation; consider removing if redundant. +fiThis ensures consistency and compatibility.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/workflows-dashboard/entrypoint.sh(1 hunks)apps/workflows-dashboard/src/lib/request/request.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/workflows-dashboard/src/lib/request/request.ts
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/workflows-dashboard/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: lint
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/workflows-dashboard/entrypoint.sh (3)
25-32: Review Client-Side Exposure of Environment VariablesThe script writes environment variables to a JavaScript file (
config.js), making them accessible in client-side code. Please verify that none of the environment variables (e.g.,VITE_API_URL,VITE_ENVIRONMENT_NAME,MODE,VITE_IMAGE_LOGO_URL) contain sensitive information. If any variable is sensitive, consider masking it or moving its handling to the server side.
34-35: Confirm Execution ParametersThe use of
exec "$@"is standard for passing the CMD arguments. Ensure that any command-line arguments (potentially provided via Docker CMD) are safely and correctly handled by the script.
3-7: 🛠️ Refactor suggestionEnsure POSIX Compatibility for Environment Checks (VITE_DOMAIN)
The use of
[[ ... ]]is not POSIX compliant. Replace it with[ ... ]. For instance:-if [[ -n "$VITE_DOMAIN" ]] +if [ -n "$VITE_DOMAIN" ] then - VITE_API_URL="$VITE_DOMAIN/api/v1/" + VITE_API_URL="$VITE_DOMAIN/api/v1/" fiThis alteration improves compatibility with
/usr/bin/env sh.Likely invalid or redundant comment.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/kyb-app/entrypoint.sh (1)
35-44:⚠️ Potential issueSensitive Data Exposure in Client-side Configuration
The generated
/usr/share/nginx/html/config.js(lines 35-44) exposes several environment variables (e.g.,VITE_API_KEY,VITE_SENTRY_AUTH_TOKEN, andVITE_SENTRY_DSN) to the client-side. This replicates concerns from previous reviews regarding the exposure of sensitive data. Please ensure that either these values are safe to expose or consider removing/masking them in the client configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/kyb-app/entrypoint.sh(1 hunks)apps/kyb-app/src/common/utils/request.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/kyb-app/src/common/utils/request.ts
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/kyb-app/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 9-9: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 14-14: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 20-20: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 25-25: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 30-30: In POSIX sh, [[ ]] is undefined.
(SC3010)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/kyb-app/entrypoint.sh (1)
46-47: CMD Execution HandlingThe use of
exec "$@"(lines 46-47) to forward the CMD arguments is correctly implemented and allows proper container command execution.
| if [[ -n "$VITE_DOMAIN" ]] | ||
| then | ||
| VITE_API_URL="$VITE_DOMAIN/api/v1/" | ||
| fi | ||
|
|
||
|
|
||
| if [[ -n "$VITE_API_KEY" ]] | ||
| then | ||
| VITE_API_KEY="$VITE_API_KEY" | ||
| fi | ||
|
|
||
| if [[ -n "$VITE_ENVIRONMENT_NAME" ]] | ||
| then | ||
| VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" | ||
| fi | ||
|
|
||
|
|
||
| if [[ -n "$VITE_DEFAULT_EXAMPLE_TOKEN" ]] | ||
| then | ||
| VITE_DEFAULT_EXAMPLE_TOKEN="$VITE_DEFAULT_EXAMPLE_TOKEN" | ||
| fi | ||
|
|
||
| if [[ -n "$VITE_SENTRY_AUTH_TOKEN" ]] | ||
| then | ||
| VITE_SENTRY_AUTH_TOKEN="$VITE_SENTRY_AUTH_TOKEN" | ||
| fi | ||
|
|
||
| if [[ -n "$VITE_SENTRY_DSN" ]] | ||
| then | ||
| VITE_SENTRY_DSN="$VITE_SENTRY_DSN" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
POSIX Compatibility Issue: Replace Non-POSIX Conditionals
The script’s shebang (#!/usr/bin/env sh, line 1) indicates a POSIX shell, but the conditional checks use the non-POSIX [[ ... ]] syntax. This will cause incompatibility issues in pure POSIX environments. Please update all conditionals (lines 3, 9, 14, 20, 25, and 30) to use [ ... ] instead. For example:
- if [[ -n "$VITE_DOMAIN" ]]
+ if [ -n "$VITE_DOMAIN" ]Apply similar changes for the other if blocks.
📝 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.
| if [[ -n "$VITE_DOMAIN" ]] | |
| then | |
| VITE_API_URL="$VITE_DOMAIN/api/v1/" | |
| fi | |
| if [[ -n "$VITE_API_KEY" ]] | |
| then | |
| VITE_API_KEY="$VITE_API_KEY" | |
| fi | |
| if [[ -n "$VITE_ENVIRONMENT_NAME" ]] | |
| then | |
| VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" | |
| fi | |
| if [[ -n "$VITE_DEFAULT_EXAMPLE_TOKEN" ]] | |
| then | |
| VITE_DEFAULT_EXAMPLE_TOKEN="$VITE_DEFAULT_EXAMPLE_TOKEN" | |
| fi | |
| if [[ -n "$VITE_SENTRY_AUTH_TOKEN" ]] | |
| then | |
| VITE_SENTRY_AUTH_TOKEN="$VITE_SENTRY_AUTH_TOKEN" | |
| fi | |
| if [[ -n "$VITE_SENTRY_DSN" ]] | |
| then | |
| VITE_SENTRY_DSN="$VITE_SENTRY_DSN" | |
| fi | |
| if [ -n "$VITE_DOMAIN" ] | |
| then | |
| VITE_API_URL="$VITE_DOMAIN/api/v1/" | |
| fi | |
| if [ -n "$VITE_API_KEY" ] | |
| then | |
| VITE_API_KEY="$VITE_API_KEY" | |
| fi | |
| if [ -n "$VITE_ENVIRONMENT_NAME" ] | |
| then | |
| VITE_ENVIRONMENT_NAME="$VITE_ENVIRONMENT_NAME" | |
| fi | |
| if [ -n "$VITE_DEFAULT_EXAMPLE_TOKEN" ] | |
| then | |
| VITE_DEFAULT_EXAMPLE_TOKEN="$VITE_DEFAULT_EXAMPLE_TOKEN" | |
| fi | |
| if [ -n "$VITE_SENTRY_AUTH_TOKEN" ] | |
| then | |
| VITE_SENTRY_AUTH_TOKEN="$VITE_SENTRY_AUTH_TOKEN" | |
| fi | |
| if [ -n "$VITE_SENTRY_DSN" ] | |
| then | |
| VITE_SENTRY_DSN="$VITE_SENTRY_DSN" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 9-9: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 14-14: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 20-20: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 25-25: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 30-30: In POSIX sh, [[ ]] is undefined.
(SC3010)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/backoffice-v2/entrypoint.sh (1)
3-37: Refactor Environment Variable Checks
Multiple conditionals (lines 3–37) use the bash-specific[[ … ]]operator. After changing the shebang, these checks will work as intended. In addition, for maintainability you might consider refactoring these repeated checks into a function or loop. For example, if many environment variables are processed similarly, a loop over a list of variable names could reduce duplication and simplify future updates.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 18-18: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 23-23: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 28-28: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 33-33: In POSIX sh, [[ ]] is undefined.
(SC3010)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/backoffice-v2/entrypoint.sh(1 hunks)apps/backoffice-v2/src/common/env/env.ts(1 hunks)apps/backoffice-v2/src/main.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/backoffice-v2/src/main.tsx
- apps/backoffice-v2/src/common/env/env.ts
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/backoffice-v2/entrypoint.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 18-18: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 23-23: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 28-28: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 33-33: In POSIX sh, [[ ]] is undefined.
(SC3010)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/backoffice-v2/entrypoint.sh (3)
52-53: Verify Execution Flow
The script ends withexec "$@", which is a standard pattern to hand over control to the command provided as arguments. Ensure that any necessary pre-execution checks or setups (such as the validations discussed in earlier reviews for numeric values) are completed before this step.
1-1: 🛠️ Refactor suggestion
⚠️ Potential issueUpdate Shebang for Shell Compatibility
The script is currently using#!/usr/bin/env sh, yet it employs bash‑specific constructs (such as[[ … ]]). This can lead to errors when running in a strictly POSIX shell. I recommend updating the shebang to bash.-#!/usr/bin/env sh +#!/usr/bin/env bashLikely invalid or redundant comment.
38-50: 🛠️ Refactor suggestionEnhance Configuration File Generation
The script generates/usr/share/nginx/html/config.jsby directly injecting environment variables. Consider the following improvements:
- Security: Avoid exposing sensitive values like
VITE_API_KEYif they are not meant for client-side consumption.- Flexibility: Instead of hardcoding
VITE_ENVIRONMENT_NAMEandMODE, use the environment variables with sensible defaults (e.g., use"${VITE_ENVIRONMENT_NAME:-local}"and"${MODE:-production}"). This makes the configuration adaptable to various deployment environments.- VITE_ENVIRONMENT_NAME: "local", - MODE: "production" + VITE_ENVIRONMENT_NAME: "${VITE_ENVIRONMENT_NAME:-local}", + MODE: "${MODE:-production}"[security]
Likely invalid or redundant comment.
Summary by CodeRabbit
Configuration Updates
config.jsscript.Build Process Improvements
Infrastructure Changes