chore: backoffice run dist with environment variables #2643
Conversation
|
WalkthroughThe recent changes enhance the production setup of the backoffice application by introducing a global 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: 0
Outside diff range, codebase verification and nitpick comments (1)
apps/backoffice-v2/global.d.ts (1)
1-3: Document the globalenvvariable.Consider adding comments to explain the purpose and usage of the global
envvariable. This will help other developers understand its role and how it should be used.+// Global environment variable used for configuration settings. declare global { export var env: { [key: string]: any }; }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- 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)
Files skipped from review due to trivial changes (1)
- apps/backoffice-v2/public/config.js
Additional comments not posted (5)
apps/backoffice-v2/src/common/env/env.ts (2)
Line range hint
13-18:
LGTM! Ensure error handling is robust.The code correctly handles invalid environment variables by logging an error and throwing an exception. Ensure that this approach aligns with the application's error handling strategy.
13-13: Verify the impact of usingglobalThis.env.The switch from
import.meta.envtoglobalThis.envmay affect how environment variables are managed. Ensure thatglobalThis.envis correctly populated in all environments where this code runs.apps/backoffice-v2/index.html (1)
11-11: Verify the loading and security of/config.js.Ensure that
/config.jsis correctly loaded and does not introduce security vulnerabilities such as cross-site scripting (XSS). Consider using integrity checks or other security measures.apps/backoffice-v2/src/main.tsx (2)
19-19: Verify the import statement with a query string.The import statement includes a query string (
?url). Ensure that this is intentional and supported by your build system or bundler.
55-58: Verify the necessity of//@ts-ignoreand the impact of modifyingglobalThis.Using
//@ts-ignoresuppresses TypeScript checks, which might hide potential issues. Ensure that this is necessary and verify the impact of modifyingglobalThis.envto avoid unintended side effects.
| @@ -8,6 +8,7 @@ | |||
| <link rel="manifest" href="/manifest.webmanifest" /> | |||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |||
| <title>Ballerine - Backoffice</title> | |||
| <script type="text/javascript" src="/config.js"></script> | |||
There was a problem hiding this comment.
I'm not sure this is needed since env.ts is imported very early on and the config file is imported there.
There was a problem hiding this comment.
I think this is needed else the config changes are not getting rendered
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/backoffice-v2/public/config.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/backoffice-v2/public/config.js
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/backoffice-v2/public/config.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/backoffice-v2/public/config.js
pratapalakshmi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/backoffice-v2/Dockerfile (1 hunks)
- apps/backoffice-v2/entrypoint.sh (1 hunks)
Additional context used
Shellcheck
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] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 34-34: In POSIX sh, [[ ]] is undefined.
(SC3010)
Additional comments not posted (6)
apps/backoffice-v2/Dockerfile (4)
25-26: Setting the working directory.The
WORKDIR /appdirective is a good practice to ensure that all subsequent commands run within the/appdirectory.
29-30: Copying the entrypoint script.The
COPYcommand ensures thatentrypoint.shis available in the production image, which is essential for initializing the container.
33-34: Granting execution permissions.The
RUN chmod a+x /app/entrypoint.shcommand is necessary to make the entrypoint script executable.
37-38: Setting the entrypoint.The
ENTRYPOINT [ "/app/entrypoint.sh" ]directive allows for customized container initialization, enhancing flexibility and control over the startup process.apps/backoffice-v2/entrypoint.sh (2)
39-51: Dynamic configuration file creation.The use of a heredoc to write environment variables to
config.jsis an effective way to generate dynamic configuration files.
53-54: Handling CMD command with exec.Using
exec "$@"ensures that the CMD command is executed properly, maintaining the intended behavior of the container.
apps/backoffice-v2/entrypoint.sh
Outdated
| if [[ -z "$VITE_DOMAIN" ]] | ||
| then | ||
| VITE_DOMAIN="localhost:3000" | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_API_KEY" ]] | ||
| then | ||
| VITE_API_KEY="secret" | ||
| fi | ||
|
|
||
| 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_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_POLLING_INTERVAL=10 | ||
| fi |
There was a problem hiding this comment.
Use POSIX-compliant syntax for conditional checks.
Replace [[ ]] with [ ] for compatibility with POSIX sh.
-if [[ -z "$VITE_DOMAIN" ]]
+if [ -z "$VITE_DOMAIN" ]Apply similar changes to all other conditional checks in the script.
Also applies to: 29-37
Tools
Shellcheck
[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)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/backoffice-v2/entrypoint.sh (1 hunks)
Additional context used
Shellcheck
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] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 34-34: In POSIX sh, [[ ]] is undefined.
(SC3010)
Additional comments not posted (6)
apps/backoffice-v2/entrypoint.sh (6)
3-27: Also applies to: 29-37Tools
Shellcheck
[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)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
13-16: LGTM!The logic for setting the default value of
VITE_AUTH_ENABLEDis correct.Tools
Shellcheck
[warning] 13-13: In POSIX sh, [[ ]] is undefined.
(SC3010)
19-22: LGTM!The logic for setting the default value of
VITE_MOCK_SERVERis correct.Tools
Shellcheck
[warning] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
24-27: LGTM!The logic for setting the default value of
VITE_POLLING_INTERVALis correct.Tools
Shellcheck
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
39-51: LGTM!The generated
config.jsfile correctly sets up the globalenvobject with the necessary environment variables.
53-54: LGTM!Using
exec "$@"provides flexibility to execute any command passed to the script.
| if [[ -z "$VITE_API_KEY" ]] | ||
| then | ||
| VITE_API_KEY="secret" | ||
| fi |
There was a problem hiding this comment.
Use a more secure method to set the API key.
Hardcoding the API key as secret is not recommended. Consider using a secure vault or secrets manager to store and retrieve the API key.
Tools
Shellcheck
[warning] 8-8: In POSIX sh, [[ ]] is undefined.
(SC3010)
apps/backoffice-v2/public/config.js
Outdated
| globalThis.env = { | ||
| VITE_API_URL: "http://google.com", | ||
| VITE_API_KEY: "secret", | ||
| VITE_AUTH_ENABLED: true, | ||
| VITE_MOCK_SERVER: false, | ||
| VITE_POLLING_INTERVAL: 10, | ||
| VITE_ASSIGNMENT_POLLING_INTERVAL: 5, | ||
| VITE_FETCH_SIGNED_URL: false, | ||
| VITE_ENVIRONMENT_NAME: "local", | ||
| MODE: "production" | ||
| }; |
There was a problem hiding this comment.
should use default value if preset, if case someone set his value while running the docker container so you should consider the provided value:
globalThis.env = {
VITE_API_URL: import.meta.env.VITE_API_URL || 'http://google.com',
VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID || 'kyb_parent_kyc_session_example',
VITE_API_KEY: import.meta.env.VITE_API_KEY || 'secret',
VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'local',
VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN || '12345678-1234-1234-1234-123456789012',
VITE_SENTRY_AUTH_TOKEN: import.meta.env.VITE_SENTRY_AUTH_TOKEN || '',
VITE_SENTRY_DSN: import.meta.env.VITE_SENTRY_DSN || '',
};
fad5a3c to
754da10
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (8)
apps/backoffice-v2/global.d.ts (1)
1-3: Make the environment object readonly to prevent accidental modifications.The environment configuration should be immutable to prevent runtime modifications that could lead to unexpected behavior.
declare global { - export var env: { [key: string]: any }; + export var env: Readonly<EnvConfig>; }apps/backoffice-v2/public/config.js (1)
1-9: Consider adding environment variable validationThe current implementation silently falls back to default values. Consider adding validation to ensure required environment variables are properly set during initialization.
Add a validation function:
const validateEnv = (env) => { const required = ['VITE_API_URL', 'VITE_API_KEY']; const missing = required.filter(key => !env[key] || env[key].startsWith('<REQUIRED_')); if (missing.length > 0) { console.error(`Missing required environment variables: ${missing.join(', ')}`); throw new Error('Missing required environment variables'); } }; globalThis.env = { // ... your existing env config ... }; validateEnv(globalThis.env);apps/backoffice-v2/Dockerfile (2)
37-38: Consider using ENTRYPOINT and CMD in shell form for better signal handling.The current exec form (
[ "/app/entrypoint.sh" ]) might not properly handle signals. Consider using shell form to ensure proper signal propagation to the nginx process.-ENTRYPOINT [ "/app/entrypoint.sh" ] -CMD ["nginx", "-g", "daemon off;"] +ENTRYPOINT /app/entrypoint.sh +CMD nginx -g "daemon off;"
29-38: Consider security improvements for the entrypoint setup.A few security-related suggestions:
- Consider using a non-root user for running nginx
- Add health checks to ensure the configuration is properly generated
Example improvements:
+# Add after line 33 +RUN adduser -D -H -s /sbin/nologin nginx-user +USER nginx-user +# Add before ENTRYPOINT +HEALTHCHECK --interval=30s --timeout=3s \ + CMD wget --quiet --tries=1 --spider http://localhost:80/ || exit 1apps/backoffice-v2/src/common/env/env.ts (1)
Line range hint
13-24: Consider enhancing error handling for production environments.The current error handling throws an error, which might not be ideal for production. Consider adding error reporting and graceful fallbacks.
if (_env.success === false) { terminal.error('❌ Invalid environment variables:\n', ...formatErrors(_env.error.format())); + // Report error to monitoring service + if (process.env.NODE_ENV === 'production') { + // Log to external service + console.error('Environment initialization failed:', _env.error); + // Use fallback values where possible + return defaultProductionConfig; + } throw new Error('Invalid environment variables'); }apps/backoffice-v2/index.html (1)
11-11: Consider adding error handling for config.js loadingWhile the config.js import is necessary for rendering as confirmed in the previous discussion, we should ensure graceful handling of loading failures.
Consider adding error handling:
- <script type="text/javascript" src="/config.js"></script> + <script type="text/javascript" src="/config.js" onerror="console.error('Failed to load configuration. Please refresh the page or contact support if the issue persists.');"></script>apps/backoffice-v2/entrypoint.sh (1)
24-32: Add validation for polling interval values.The polling intervals should be validated to ensure they're within reasonable bounds to prevent performance issues.
if [[ -z "$VITE_POLLING_INTERVAL" ]] then VITE_POLLING_INTERVAL=10 +else + # Ensure minimum 5 seconds to prevent excessive polling + if [ "$VITE_POLLING_INTERVAL" -lt 5 ]; then + VITE_POLLING_INTERVAL=5 + fi fi if [[ -z "$VITE_ASSIGNMENT_POLLING_INTERVAL" ]] then VITE_ASSIGNMENT_POLLING_INTERVAL=5 +else + # Ensure minimum 3 seconds to prevent excessive polling + if [ "$VITE_ASSIGNMENT_POLLING_INTERVAL" -lt 3 ]; then + VITE_ASSIGNMENT_POLLING_INTERVAL=3 + fi fi🧰 Tools
🪛 Shellcheck
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
apps/backoffice-v2/src/main.tsx (1)
Line range hint
1-61: Consider separating environment initializationThe current file handles multiple responsibilities including app initialization, environment setup, and React rendering. Consider extracting the environment initialization into a separate module (e.g.,
initializeEnvironment.ts) that runs before any other initialization.This would:
- Improve separation of concerns
- Make environment validation more centralized
- Ensure environment is properly set up before any other code runs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
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)
🧰 Additional context used
🪛 Shellcheck
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] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 34-34: In POSIX sh, [[ ]] is undefined.
(SC3010)
🔇 Additional comments (6)
apps/backoffice-v2/Dockerfile (2)
25-26: LGTM: Working directory setup follows best practices.
The WORKDIR directive is appropriately placed before operations that need the working directory context.
29-30: Verify entrypoint.sh existence and content.
While the script copying and permission setting look correct, we should verify:
- The existence of entrypoint.sh in the source location
- The script's content for security considerations
Also applies to: 33-34
✅ Verification successful
The entrypoint.sh script has been verified and appears secure
The script exists and its content has been examined. It performs the following safe operations:
- Sets default values for environment variables if not provided
- Generates a runtime config.js with environment variables
- Uses proper shell scripting practices with environment variable checks
- Executes the passed command safely using
exec "$@"
No security concerns were identified in the script's implementation. The COPY instruction in the Dockerfile is appropriate for this use case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if entrypoint.sh exists and examine its content
fd -t f "entrypoint.sh" -x cat {}
Length of output: 1077
apps/backoffice-v2/src/common/env/env.ts (2)
13-13: Review security implications of global environment exposure.
Moving environment variables to globalThis.env makes them globally accessible. This could expose sensitive configuration data to client-side scripts and browser dev tools.
#!/bin/bash
# Check for potentially sensitive environment variables in schema
rg -A 5 "schema.*string.*" apps/backoffice-v2/src/common/env/schema.ts
# Look for any security-related comments or patterns
rg -i "secret|key|token|password|credential" apps/backoffice-v2/src/common/env/schema.tsConsider:
- Splitting environment variables into public/private
- Using runtime API endpoints for sensitive configurations
- Implementing proper security headers (e.g., CSP) to restrict script access
13-13:
Verify environment variable initialization timing.
The switch from import.meta.env to globalThis.env changes when environment variables become available - from build-time to runtime. This could lead to race conditions if the code executes before the config.js script loads.
Consider adding an initialization guard:
-const _env = EnvSchema.safeParse(globalThis.env);
+const getEnv = () => {
+ if (!globalThis.env) {
+ throw new Error('Environment variables not initialized. Ensure config.js is loaded.');
+ }
+ return EnvSchema.safeParse(globalThis.env);
+};
+const _env = getEnv();apps/backoffice-v2/entrypoint.sh (1)
53-54: LGTM: Proper command execution handling.
The use of exec "$@" is the correct pattern for Docker entrypoint scripts, allowing proper signal handling and process management.
apps/backoffice-v2/src/main.tsx (1)
20-20: Verify config.js availability and loading mechanism
The import of config.js using Vite's URL import might cause issues in production if the file is not properly generated during the build process.
Let's verify the config file setup:
✅ Verification successful
Config.js import and setup is correctly implemented
The verification shows that:
- The
config.jsis properly included inindex.htmlvia a script tag before the main application code. - The
entrypoint.shcorrectly generates theconfig.jsfile at runtime in/usr/share/nginx/html/config.jswith all necessary environment variables. - The import with
?urlinmain.tsxis correct as it ensures the file is included in the build and available at runtime.
The implementation follows a proper pattern where:
- Environment variables are injected at container startup via
entrypoint.sh - The config file is loaded before the application code
- The Vite URL import ensures the file is included in the build
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify config.js existence and content in public directory
# Expected: Find config.js and check its content structure
# Check if config.js exists in public directory
fd -t f "config.js" -x cat {} \;
# Check if the HTML file properly includes config.js
fd -t f "index.html" -x grep -l "config.js" {} \;
Length of output: 11134
Script:
#!/bin/bash
# Let's check the actual content of the index.html and look for config.js references
cat ./apps/backoffice-v2/index.html
# Let's also check the actual config.js in the backoffice-v2 directory
fd -t f "config.js" apps/backoffice-v2 -x cat {} \;
# Let's check if there are any build scripts or Dockerfile that might generate config.js
rg -l "config.js" --type sh
Length of output: 2226
Script:
#!/bin/bash
# Let's check the entrypoint.sh to understand how config.js is generated
cat apps/backoffice-v2/entrypoint.sh
# Let's also check if there are any other files that might be involved in config.js generation
rg -l "config\.js" --type typescript --type javascript
Length of output: 1172
| declare global { | ||
| export var env: { [key: string]: any }; | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Migration from import.meta.env to globalThis.env is incomplete
There are still several instances of import.meta.env usage in the codebase that need to be migrated to globalThis.env:
apps/backoffice-v2/src/main.tsx: Usingimport.meta.env.VITE_API_URLapps/backoffice-v2/public/config.js: Multiple environment variables still usingimport.meta.env:VITE_API_URLVITE_KYB_DEFINITION_IDVITE_API_KEYVITE_ENVIRONMENT_NAMEVITE_DEFAULT_EXAMPLE_TOKENVITE_SENTRY_AUTH_TOKENVITE_SENTRY_DSN
🔗 Analysis chain
Verify the environment variable usage across the codebase.
Since this is part of a migration from import.meta.env to globalThis.env, let's ensure all usages are updated consistently.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining import.meta.env usage that needs to be migrated
echo "Checking for remaining import.meta.env usage..."
rg "import\.meta\.env" apps/backoffice-v2/
echo "Checking current globalThis.env usage..."
rg "globalThis\.env" apps/backoffice-v2/
Length of output: 1425
🛠️ Refactor suggestion
Improve type safety by using a specific interface instead of any.
Using { [key: string]: any } bypasses TypeScript's type checking benefits. Consider defining a specific interface for your environment variables with proper types.
declare global {
- export var env: { [key: string]: any };
+ export interface EnvConfig {
+ API_URL: string;
+ NODE_ENV: 'development' | 'production' | 'test';
+ // Add other environment variables with specific types
+ }
+ export var env: EnvConfig;
}Committable suggestion skipped: line range outside the PR's diff.
| globalThis.env = { | ||
| VITE_API_URL: import.meta.env.VITE_API_URL || 'http://google.com', | ||
| VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID || 'kyb_parent_kyc_session_example', | ||
| VITE_API_KEY: import.meta.env.VITE_API_KEY || 'secret', | ||
| VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'local', | ||
| VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN || '12345678-1234-1234-1234-123456789012', | ||
| VITE_SENTRY_AUTH_TOKEN: import.meta.env.VITE_SENTRY_AUTH_TOKEN || '', | ||
| VITE_SENTRY_DSN: import.meta.env.VITE_SENTRY_DSN || '', | ||
| }; |
There was a problem hiding this comment.
Security: Avoid exposing sensitive default values in public files
The configuration file contains sensitive default values (API key, token) and is located in the public directory, which could pose security risks in production environments.
Consider these improvements:
- Remove sensitive default values
- Use placeholder values that clearly indicate they need to be replaced
- Move this configuration to a non-public location
globalThis.env = {
- VITE_API_URL: import.meta.env.VITE_API_URL || 'http://google.com',
- VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID || 'kyb_parent_kyc_session_example',
- VITE_API_KEY: import.meta.env.VITE_API_KEY || 'secret',
- VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'local',
- VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN || '12345678-1234-1234-1234-123456789012',
+ VITE_API_URL: import.meta.env.VITE_API_URL || '<REQUIRED_API_URL>',
+ VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID || '<REQUIRED_KYB_DEFINITION_ID>',
+ VITE_API_KEY: import.meta.env.VITE_API_KEY || '<REQUIRED_API_KEY>',
+ VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'development',
+ VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN || '<REQUIRED_TOKEN>',
VITE_SENTRY_AUTH_TOKEN: import.meta.env.VITE_SENTRY_AUTH_TOKEN || '',
VITE_SENTRY_DSN: import.meta.env.VITE_SENTRY_DSN || '',
};📝 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.
| globalThis.env = { | |
| VITE_API_URL: import.meta.env.VITE_API_URL || 'http://google.com', | |
| VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID || 'kyb_parent_kyc_session_example', | |
| VITE_API_KEY: import.meta.env.VITE_API_KEY || 'secret', | |
| VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'local', | |
| VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN || '12345678-1234-1234-1234-123456789012', | |
| VITE_SENTRY_AUTH_TOKEN: import.meta.env.VITE_SENTRY_AUTH_TOKEN || '', | |
| VITE_SENTRY_DSN: import.meta.env.VITE_SENTRY_DSN || '', | |
| }; | |
| globalThis.env = { | |
| VITE_API_URL: import.meta.env.VITE_API_URL || '<REQUIRED_API_URL>', | |
| VITE_KYB_DEFINITION_ID: import.meta.env.VITE_KYB_DEFINITION_ID || '<REQUIRED_KYB_DEFINITION_ID>', | |
| VITE_API_KEY: import.meta.env.VITE_API_KEY || '<REQUIRED_API_KEY>', | |
| VITE_ENVIRONMENT_NAME: import.meta.env.VITE_ENVIRONMENT_NAME || 'development', | |
| VITE_DEFAULT_EXAMPLE_TOKEN: import.meta.env.VITE_DEFAULT_EXAMPLE_TOKEN || '<REQUIRED_TOKEN>', | |
| VITE_SENTRY_AUTH_TOKEN: import.meta.env.VITE_SENTRY_AUTH_TOKEN || '', | |
| VITE_SENTRY_DSN: import.meta.env.VITE_SENTRY_DSN || '', | |
| }; |
💡 Codebase verification
Let me analyze the results and generate a final response based on the findings.
Environment variables are properly used, but some hardcoded values need attention
The verification revealed several areas that need attention:
- A hardcoded API endpoint in
domains/workflows/mock-service-worker/workflows/workflows.controller.ts:http://localhost:3000/api/internal/workflows/${id} - Hardcoded URLs in
common/env/schema.ts:VITE_API_URLdefault value is set to'https://api-dev.ballerine.io/v2' - Several hardcoded URLs in
domains/chat/chatbot-opengpt.tsxfor logos and links - A hardcoded model URL in
common/hooks/useFaceComparison/useFaceComparison.tsx:'https://justadudewhohacks.github.io/face-api.js/models'
The rest of the hardcoded URLs appear to be in mock data, story files, or SVG namespaces which are acceptable.
🔗 Analysis chain
Verify environment variable usage across the application
Let's ensure these environment variables are properly used throughout the application and that there are no hardcoded values that should be using these configurations instead.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for potential hardcoded values that should use environment variables instead
# Test: Search for hardcoded URLs that might need to use VITE_API_URL
echo "Checking for hardcoded URLs..."
rg -g '!{*.md,*.json,*.lock}' -i "http[s]?://[a-zA-Z0-9./?=_%:-]+" apps/backoffice-v2/src/
# Test: Search for hardcoded API keys or tokens
echo "Checking for potential hardcoded secrets..."
rg -g '!{*.md,*.json,*.lock}' -i "api[_-]?key|secret|token" apps/backoffice-v2/src/
# Test: Verify environment variable usage
echo "Checking environment variable usage..."
rg "import\.meta\.env\.VITE_" apps/backoffice-v2/src/
Length of output: 14917
| @@ -8,6 +8,7 @@ | |||
| <link rel="manifest" href="/manifest.webmanifest" /> | |||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |||
| <title>Ballerine - Backoffice</title> | |||
| <script type="text/javascript" src="/config.js"></script> | |||
There was a problem hiding this comment.
💡 Codebase verification
Environment variables exposure requires attention
The config.js file exposes several sensitive configuration variables that should be handled more securely:
VITE_API_KEY: Exposing API keys in client-side code poses a security riskVITE_SENTRY_AUTH_TOKEN: Sentry authentication tokens should never be exposed to the clientVITE_DEFAULT_EXAMPLE_TOKEN: Even example/default tokens should not be hardcoded in client-side code
Recommendations:
- Move sensitive configuration to server-side environment variables
- Use server endpoints to handle authenticated API calls instead of exposing API keys
- Remove any hardcoded tokens, even if they are just examples
- Consider implementing proper authentication flow instead of relying on exposed tokens
🔗 Analysis chain
Review security implications of exposing environment variables
The inclusion of environment variables in a client-side config.js file requires careful consideration of security implications. Ensure that sensitive configuration data (API keys, secrets, internal URLs) are not exposed.
Let's verify what environment variables are being exposed:
Consider implementing:
- A backend proxy for sensitive operations instead of exposing credentials
- Environment-specific configuration filtering during build
Let me check the config.js file content to verify what environment variables are being exposed:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential sensitive data in environment variable handling
# Check entrypoint.sh for environment variable exposure
echo "Checking entrypoint.sh for environment variables:"
rg -i "export|env\." apps/backoffice-v2/entrypoint.sh
# Check for sensitive patterns in environment variable usage
echo -e "\nChecking for potentially sensitive patterns:"
rg -i "key|token|secret|password|credential" apps/backoffice-v2/
Length of output: 107207
Script:
#!/bin/bash
# Check the content of config.js
cat apps/backoffice-v2/public/config.js
Length of output: 613
| #!/usr/bin/env sh | ||
|
|
There was a problem hiding this comment.
Update shebang to match script requirements.
The script uses bash-specific features but declares itself as a sh script. Either:
- Change shebang to
#!/usr/bin/env bashto explicitly require bash, or - Make the script POSIX-compliant by replacing
[[with[
-#!/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 | |
| #!/usr/bin/env bash | |
| #!/usr/bin/env sh | ||
|
|
||
| if [[ -z "$VITE_DOMAIN" ]] | ||
| then | ||
| VITE_DOMAIN="http://localhost:3000" | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_API_KEY" ]] | ||
| then | ||
| VITE_API_KEY="secret" | ||
| fi | ||
|
|
||
| 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_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_POLLING_INTERVAL=10 | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_ASSIGNMENT_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_ASSIGNMENT_POLLING_INTERVAL=5 | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_FETCH_SIGNED_URL" ]] | ||
| then | ||
| VITE_FETCH_SIGNED_URL=false | ||
| fi | ||
|
|
||
| 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 | ||
|
|
||
| # Handle CMD command | ||
| exec "$@" |
There was a problem hiding this comment.
💡 Codebase verification
File permissions need to be fixed for Docker entrypoint
The script integration has issues that need to be addressed:
- The entrypoint script currently has permissions
644(-rw-r--r--) but needs to be executable (755) - While the Dockerfile sets permissions with
chmod a+x, the file in the repository should be executable to maintain consistency
To fix:
- Run
chmod +x apps/backoffice-v2/entrypoint.shand commit the changes - Consider adding a
.gitattributesfile to maintain executable permissions
🔗 Analysis chain
Verify Docker integration and file permissions.
Let's ensure the script is properly integrated into the Docker setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the script is referenced in Dockerfile and has correct permissions
echo "Checking Dockerfile configuration..."
rg -l "COPY.*entrypoint.sh" && \
rg -l "RUN chmod \+x.*entrypoint.sh" && \
rg -l "ENTRYPOINT.*entrypoint.sh"
# Check if nginx configuration exists for serving the config.js
echo "Checking nginx configuration..."
fd -g "*nginx*.conf" -x grep -l "config.js"
Length of output: 298
Script:
#!/bin/bash
# Let's check the actual Dockerfile content to verify the integration
echo "Checking Dockerfile content:"
cat apps/backoffice-v2/Dockerfile
# Let's also check if there are any nginx configuration files
echo -e "\nListing nginx configuration files:"
fd -g "*nginx*.conf"
# Check if the entrypoint.sh is executable
echo -e "\nChecking file permissions:"
ls -l apps/backoffice-v2/entrypoint.sh
Length of output: 1202
🧰 Tools
🪛 Shellcheck
[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)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 34-34: In POSIX sh, [[ ]] is undefined.
(SC3010)
| //@ts-ignore | ||
| globalThis.env = globalThis.env || { | ||
| API_URL: import.meta.env.VITE_API_URL, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and environment variable handling
The current implementation has several concerns:
- Type safety is bypassed with @ts-ignore
- Direct mutation of globalThis could lead to runtime conflicts
- Missing validation for required environment variables
- No error handling for missing variables
Consider this type-safe alternative:
-//@ts-ignore
-globalThis.env = globalThis.env || {
- API_URL: import.meta.env.VITE_API_URL,
-};
+interface AppEnv {
+ API_URL: string;
+ // Add other environment variables as needed
+}
+
+const validateEnv = (env: Partial<AppEnv>): AppEnv => {
+ if (!env.API_URL) {
+ throw new Error('VITE_API_URL is required but not provided');
+ }
+ return env as AppEnv;
+};
+
+declare global {
+ var env: AppEnv;
+}
+
+globalThis.env = validateEnv({
+ API_URL: import.meta.env.VITE_API_URL,
+});This solution:
- Adds proper TypeScript types
- Validates required environment variables
- Provides clear error messages for missing variables
- Makes the code more maintainable and type-safe
📝 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.
| //@ts-ignore | |
| globalThis.env = globalThis.env || { | |
| API_URL: import.meta.env.VITE_API_URL, | |
| }; | |
| interface AppEnv { | |
| API_URL: string; | |
| // Add other environment variables as needed | |
| } | |
| const validateEnv = (env: Partial<AppEnv>): AppEnv => { | |
| if (!env.API_URL) { | |
| throw new Error('VITE_API_URL is required but not provided'); | |
| } | |
| return env as AppEnv; | |
| }; | |
| declare global { | |
| var env: AppEnv; | |
| } | |
| globalThis.env = validateEnv({ | |
| API_URL: import.meta.env.VITE_API_URL, | |
| }); |
pratapalakshmi
force-pushed
the
chore/run/time/dist/backoffice
branch
from
39d369a to
f48d792
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
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)
✅ Files skipped from review due to trivial changes (1)
- apps/backoffice-v2/global.d.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/backoffice-v2/src/main.tsx
- apps/backoffice-v2/index.html
- apps/backoffice-v2/src/common/env/env.ts
- apps/backoffice-v2/public/config.js
- apps/backoffice-v2/Dockerfile
🧰 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] 19-19: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 34-34: In POSIX sh, [[ ]] is undefined.
(SC3010)
🔇 Additional comments (1)
apps/backoffice-v2/entrypoint.sh (1)
1-2: Update shebang to match script requirements.
The script uses bash-specific features but declares itself as a sh script.
| if [[ -z "$VITE_DOMAIN" ]] | ||
| then | ||
| VITE_DOMAIN="http://localhost:3000" | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_API_KEY" ]] | ||
| then | ||
| VITE_API_KEY="secret" | ||
| fi | ||
|
|
||
| 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_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_POLLING_INTERVAL=10 | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_ASSIGNMENT_POLLING_INTERVAL" ]] | ||
| then | ||
| VITE_ASSIGNMENT_POLLING_INTERVAL=5 | ||
| fi | ||
|
|
||
| if [[ -z "$VITE_FETCH_SIGNED_URL" ]] | ||
| then | ||
| VITE_FETCH_SIGNED_URL=false | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for critical environment variables.
The script should validate the format and values of critical variables like VITE_DOMAIN and VITE_POLLING_INTERVAL.
if [[ -z "$VITE_DOMAIN" ]]
then
VITE_DOMAIN="http://localhost:3000"
+else
+ # Validate URL format
+ if ! [[ "$VITE_DOMAIN" =~ ^https?:// ]]; then
+ echo "Error: VITE_DOMAIN must start with http:// or https://"
+ exit 1
+ fi
fi
if [[ -z "$VITE_POLLING_INTERVAL" ]]
then
VITE_POLLING_INTERVAL=10
+else
+ # Validate numeric value and range
+ if ! [[ "$VITE_POLLING_INTERVAL" =~ ^[0-9]+$ ]] || \
+ [ "$VITE_POLLING_INTERVAL" -lt 1 ] || \
+ [ "$VITE_POLLING_INTERVAL" -gt 3600 ]; then
+ echo "Error: VITE_POLLING_INTERVAL must be a number between 1 and 3600"
+ exit 1
+ fi
fiCommittable suggestion skipped: line range outside the PR's diff.
🧰 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)
[warning] 24-24: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 29-29: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 34-34: In POSIX sh, [[ ]] is undefined.
(SC3010)
| 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.
💡 Codebase verification
Add security headers and configure content type for config.js
The nginx configuration needs to be enhanced with security headers and proper content type handling for the config.js file:
- Add the following to the nginx configuration:
location /config.js { add_header Content-Type application/javascript; add_header X-Content-Type-Options nosniff; add_header Cache-Control no-store; }
- Consider adding other security headers like
X-Frame-Options,X-XSS-Protection, andContent-Security-Policy
🔗 Analysis chain
Add security headers and content type for config.js.
The config file might be accessible via direct URL. Ensure proper security headers are set in the nginx configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if nginx configuration exists and has proper security headers
fd -g "*nginx*.conf" -x grep -l "config\.js"
fd -g "*nginx*.conf" -x grep -l "add_header X-Content-Type-Options nosniff"
Length of output: 124
Script:
#!/bin/bash
# Let's try a broader search for nginx configurations and security headers
fd nginx
fd -e conf
rg -l "config\.js"
rg -l "add_header.*nosniff"
Length of output: 1647
Script:
#!/bin/bash
# Let's examine the nginx configuration files we found
cat apps/backoffice-v2/example.nginx.conf
Length of output: 322
| # Handle CMD command | ||
| exec "$@" |
There was a problem hiding this comment.
Validate and sanitize command input.
The exec "$@" command executes arbitrary commands without validation. Consider:
- Validating allowed commands
- Implementing command whitelisting
+# Define allowed commands
+ALLOWED_COMMANDS=("nginx" "node" "npm" "serve")
+
+# Validate command
+if [ $# -gt 0 ]; then
+ command_name=$(basename "$1")
+ if ! [[ " ${ALLOWED_COMMANDS[@]} " =~ " ${command_name} " ]]; then
+ echo "Error: Command '$command_name' is not allowed"
+ exit 1
+ fi
+fi
+
# Handle CMD command
exec "$@"Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation