chore: fix some small issue for deploy #164
Conversation
…guration validation
…URL handling for HTTPS
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request makes a series of changes across backend, frontend, Docker, and build configurations. It modifies the default mail behavior in the authentication service and adjusts email confirmation checks to respect mail service settings. New optional database configuration variables and a default port are introduced, while the Vite server settings are updated. Docker Compose and Traefik configurations are added for reverse proxy management and TLS security. Frontend components now use dynamic URL protocol constants, and dependency import paths for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthService
User->>AuthService: Attempt login with unconfirmed email
AuthService->>AuthService: Check email confirmation & mail service (isMailEnabled)
alt Mail service enabled
AuthService-->>User: Throw error (email not confirmed)
else Mail service disabled
AuthService-->>User: Proceed with login
end
sequenceDiagram
participant Client
participant RunProjectRoute as RunProject Route
participant Docker as Docker Service
Client->>RunProjectRoute: Request to build and run project
RunProjectRoute->>RunProjectRoute: Check TLS value
alt TLS enabled
RunProjectRoute->>Docker: Execute Docker run with websecure entry and TLS labels (network: docker_traefik_network)
else TLS disabled
RunProjectRoute->>Docker: Execute Docker run with web entry (network: docker_traefik_network)
end
Docker-->>Client: Container started
Possibly related PRs
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 1
🧹 Nitpick comments (8)
backend/template/react-ts/vite.config.ts (1)
33-33: Consider security implications of allowing all hosts.Setting
allowedHosts: trueallows connections from any host to your Vite dev server. While this may be necessary for certain deployment scenarios (like Docker or proxied environments), it opens up your development server to potential cross-origin attacks.If possible, consider specifying only the hosts that need access:
- allowedHosts: true, + allowedHosts: ['your-domain.com', 'localhost'],backend/src/auth/auth.service.ts (1)
46-49: Update the outdated comment to match the new default value.The comment on line 46 still mentions defaulting to 'true', but the implementation now defaults to 'false'.
- // Read the MAIL_ENABLED environment variable, default to 'true' + // Read the MAIL_ENABLED environment variable, default to 'false'backend/src/config/env.validation.ts (1)
4-24: Consider adding validation for connection URL as an alternative.Many database systems also support connection URLs (e.g., DATABASE_URL). Consider adding support for this alongside the individual configuration parameters to provide flexibility in deployment scenarios.
export class EnvironmentVariables { // Database Configuration - all optional @IsOptional() @IsString() DB_HOST?: string; @IsOptional() @IsPort() DB_PORT?: string; @IsOptional() @IsString() DB_USERNAME?: string; @IsOptional() @IsString() DB_PASSWORD?: string; @IsOptional() @IsString() DB_DATABASE?: string; + // Alternative database configuration via URL + @IsOptional() + @IsString() + DATABASE_URL?: string;frontend/src/app/api/runProject/route.ts (1)
151-169: Good implementation of conditional TLS configuration.The conditional Docker run command structure correctly sets different Traefik configurations based on whether TLS is enabled:
- With TLS: Uses
websecureentrypoint and enables TLS- Without TLS: Uses
webentrypoint without TLS settingsAlso correctly updates the network name from
codefox_traefik_networktodocker_traefik_networkin both configurations.One suggestion: Consider extracting the common parts of both Docker run commands to reduce code duplication.
- let runCommand; - if (TLS) { - runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ - -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ - -l "traefik.http.routers.${subdomain}.entrypoints=websecure" \ - -l "traefik.http.routers.${subdomain}.tls=true" \ - -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ - --network=docker_traefik_network -p ${exposedPort}:5173 \ - -v "${directory}:/app" \ - ${imageName}`; - } else { - runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ - -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ - -l "traefik.http.routers.${subdomain}.entrypoints=web" \ - -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ - --network=docker_traefik_network -p ${exposedPort}:5173 \ - -v "${directory}:/app" \ - ${imageName}`; - } + const entrypoint = TLS ? "websecure" : "web"; + const tlsConfig = TLS ? '-l "traefik.http.routers.${subdomain}.tls=true" \\' : ''; + const runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ + -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ + -l "traefik.http.routers.${subdomain}.entrypoints=${entrypoint}" \ + ${tlsConfig} + -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ + --network=docker_traefik_network -p ${exposedPort}:5173 \ + -v "${directory}:/app" \ + ${imageName}`;frontend/src/utils/const.ts (2)
1-14: Well-structured URL protocol prefix determination.The constant correctly implements logic to determine the protocol prefix based on the environment:
- Uses HTTPS when in a browser with HTTPS protocol
- Respects the TLS environment variable setting
- Defaults to HTTPS for security if not explicitly configured otherwise
Small suggestion: Consider using strict equality (
===) when comparing with environment variable values:- : process.env.TLS == 'false' + : process.env.TLS === 'false'
16-22: Consistent TLS detection implementation.The TLS constant correctly determines if the environment should use TLS based on:
- Browser protocol checks
- Environment variable configuration
Small suggestion: Consider making the environment variable check consistent with the URL_PROTOCOL_PREFIX implementation by using the negative case:
- : process.env.TLS == 'true'; + : process.env.TLS !== 'false';This would make the default behavior consistent (both default to secure) when the environment variable is not explicitly set.
frontend/src/components/chat/code-engine/web-view.tsx (1)
17-23: Consider using more specific types thanany.While the component extraction is good for separation of concerns, using
anytype forcurProjectandgetWebUrlloses type safety. Consider creating proper interfaces or types for these props.- curProject: any; - getWebUrl: any; + curProject: { projectPath: string; [key: string]: any }; + getWebUrl: (projectPath: string) => Promise<{ domain: string }>;docker/traefik-config/services.yml (1)
57-67: Consider expanding CORS configuration for security.The CORS configuration allows several methods and headers but might need additional settings like:
accessControlAllowOriginto restrict which domains can access resourcesaccessControlMaxAgeto control caching of preflight requestsaccessControlAllowCredentialsif authentication is neededcors: headers: + accessControlAllowOrigin: + - "https://codefox.net" + - "https://*.codefox.net" accessControlAllowMethods: - GET - POST - PUT - DELETE - OPTIONS accessControlAllowHeaders: - Content-Type - Authorization + accessControlMaxAge: 3600 + accessControlAllowCredentials: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
backend/src/auth/auth.service.ts(2 hunks)backend/src/config/env.validation.ts(1 hunks)backend/template/react-ts/vite.config.ts(1 hunks)codefox-common/package.json(1 hunks)docker/docker-compose.pord.yml(1 hunks)docker/traefik-config/services.yml(1 hunks)docker/traefik-config/tls.yml(1 hunks)frontend/.env.example(1 hunks)frontend/src/app/api/runProject/route.ts(2 hunks)frontend/src/components/chat/code-engine/project-context.tsx(3 hunks)frontend/src/components/chat/code-engine/web-view.tsx(5 hunks)frontend/src/components/root/expand-card.tsx(3 hunks)frontend/src/utils/const.ts(1 hunks)llm-server/src/downloader/model-downloader.ts(1 hunks)llm-server/src/downloader/universal-status.ts(1 hunks)llm-server/src/downloader/universal-utils.ts(1 hunks)llm-server/src/llm-provider.ts(1 hunks)llm-server/tsconfig.json(2 hunks)
✅ Files skipped from review due to trivial changes (7)
- llm-server/src/downloader/universal-utils.ts
- llm-server/src/downloader/universal-status.ts
- docker/traefik-config/tls.yml
- llm-server/src/downloader/model-downloader.ts
- llm-server/src/llm-provider.ts
- docker/docker-compose.pord.yml
- frontend/.env.example
🔇 Additional comments (22)
llm-server/tsconfig.json (2)
10-10: Clarifying "rootDir" Addition
The addition of"rootDir": "./src"undercompilerOptionsclearly designates the project's source directory. This improves module resolution and ensures that the TypeScript compiler correctly maps the source files, which is especially beneficial for deployment and build consistency.
19-21: Streamlined "paths" Configuration
Removing the custom mappings for"codefox-common"in favor of a universal"*": ["node_modules/*"]simplifies the module resolution process. Verify that all import statements throughout the codebase have been adjusted accordingly as per the updated structure, ensuring that there are no unresolved module references.backend/src/auth/auth.service.ts (1)
195-197: Good enhancement to account for mail service state.This change intelligently makes email confirmation conditional on whether the mail service is enabled. This ensures that when mail is disabled, users can still log in without email confirmation, which is the expected behavior.
backend/src/config/env.validation.ts (3)
5-23: Well-structured database configuration options with proper validation.The addition of optional database configuration variables with appropriate validation decorators is a good practice. This enhances flexibility while maintaining type safety and validation.
26-26: Good addition of default port value.Setting a default port (8000) is a sensible enhancement that makes the application more user-friendly by not requiring this configuration when using the standard port.
1-1: Ensure imports are properly organized.You've correctly added the
IsPortvalidator to handle the new DB_PORT validation. The imports are appropriately organized.codefox-common/package.json (2)
1-10: Notice Removal of the "exports" FieldThe "exports" property has been completely removed from the package.json. While this simplification may streamline module resolution, it is a breaking change for any consumers that relied on the exported paths for ES modules, CommonJS, or type definitions. Please verify that downstream projects or build tools do not depend on this field and update their import paths if necessary.
12-12: Enhanced Build Script with Cleanup CommandThe updated "build" script now begins with
rimraf dist &&to ensure that stale build files are cleared before new builds are generated. This proactive cleanup helps prevent issues related to outdated files. Ensure that this change has been tested in all environments where the build process runs.frontend/src/app/api/runProject/route.ts (1)
10-10: Import TLS constant to support secure and non-secure environments.Good addition for making the Docker configuration protocol-aware.
frontend/src/components/root/expand-card.tsx (3)
7-7: Import protocol constant for dynamic URL construction.Appropriate addition to make the URLs protocol-aware.
41-42: Dynamic URL protocol implementation.Good replacement of hardcoded "http://" with the dynamic URL_PROTOCOL_PREFIX.
124-124: Dynamic URL protocol implementation.Good replacement of hardcoded "http://" with the dynamic URL_PROTOCOL_PREFIX.
frontend/src/components/chat/code-engine/project-context.tsx (3)
23-23: Import protocol constant for dynamic URL construction.Appropriate addition to make the URLs protocol-aware.
281-281: Dynamic URL protocol implementation in getWebUrl.Good replacement of hardcoded protocol with the dynamic URL_PROTOCOL_PREFIX.
443-443: Dynamic URL protocol implementation in pollChatProject.Good replacement of hardcoded protocol with the dynamic URL_PROTOCOL_PREFIX.
frontend/src/components/chat/code-engine/web-view.tsx (4)
15-15: Good addition of URL protocol constant.Using a constant for the URL protocol prefix is a better practice than hardcoding "http://" throughout the code, allowing for flexibility in switching between protocols.
47-47: Good refactoring to use URL protocol constant.Replacing hardcoded "http://" with the imported constant is a good practice.
58-58: Consistent use of URL protocol constant.Nice consistency in using the imported protocol constant throughout the component.
248-265: Improved error handling with proper loading states.The refactored
WebPreviewcomponent now handles context initialization better by showing a loading message instead of throwing an error, which improves user experience.docker/traefik-config/services.yml (3)
1-26: Good router configuration with appropriate priorities.The router configuration follows a sensible structure with proper prioritization:
- Frontend (priority 10)
- Backend/graphql (priority 20)
- Subdomains (priority 30)
This ensures requests are routed to the correct services based on hostname and path.
36-49:❓ Verification inconclusive
Verify Docker host resolution across environments.
The configuration uses
host.docker.internalwhich works on Docker Desktop for Windows and Mac but might require additional configuration in Linux environments.
🏁 Script executed:
#!/bin/bash # Check if running in a Docker container and if host.docker.internal is resolvable if grep -q docker /proc/1/cgroup; then echo "Running inside Docker container" # Try to resolve host.docker.internal if getent hosts host.docker.internal > /dev/null; then echo "host.docker.internal is resolvable" else echo "host.docker.internal is NOT resolvable" echo "In Linux environments, you may need to add '--add-host=host.docker.internal:host-gateway' to your docker run command" echo "or add 'extra_hosts: - \"host.docker.internal:host-gateway\"' in docker-compose.yml" fi else echo "Not running inside a Docker container" fiLength of output: 164
Attention: Verify Docker Host Resolution in Linux Environments
The test script ran in a non-container environment, so it didn't confirm whether
host.docker.internalis resolvable within a Linux container. As noted,host.docker.internaltypically works on Docker Desktop for Windows and Mac but might not resolve on Linux without additional configuration. Please ensure that when deploying on Linux you provide the necessary host mapping (e.g., using--add-host=host.docker.internal:host-gatewayor addingextra_hosts: - "host.docker.internal:host-gateway"in your docker-compose file).
- Action: Test the resolution of
host.docker.internalinside a Linux container and update the Docker run or docker-compose configuration accordingly if the resolution fails.
46-49:✅ Verification successful
Verify noop service endpoint.
The noop service points to
localhost:9000, but it's not clear what service (if any) is running on that port. Make sure this is intentional and that the port is either unused or running a service that can handle these redirected requests.
🏁 Script executed:
#!/bin/bash # Check if anything is listening on port 9000 if command -v ss > /dev/null; then echo "Checking if any service is listening on port 9000 (using ss):" ss -tuln | grep ":9000" elif command -v netstat > /dev/null; then echo "Checking if any service is listening on port 9000 (using netstat):" netstat -tuln | grep ":9000" else echo "Neither ss nor netstat found. Unable to check port 9000." fi echo "Note: For the 'noop' service, it's common to use a non-existent endpoint since" echo "requests to this service only occur after a redirect middleware is applied."Length of output: 496
Noop Service Endpoint Confirmed as Intentional
Based on the verification, no service is listening on port 9000—which is expected behavior for a noop endpoint that only routes requests after the redirect middleware is applied. The configuration in
docker/traefik-config/services.yml(lines 46–49) is intentionally set to point to a non-existent service, so no changes are required.
docker/traefik-config/services.yml
Outdated
| redirect-all: | ||
| rule: 'hostregexp(`{host:.+}`)' | ||
| entrypoints: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix case inconsistency in rule directive.
There's a case inconsistency in the rule directive that might cause issues as Traefik configuration is often case-sensitive.
- rule: 'hostregexp(`{host:.+}`)'
+ rule: 'HostRegexp(`{host:.+}`)'📝 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.
| redirect-all: | |
| rule: 'hostregexp(`{host:.+}`)' | |
| entrypoints: | |
| redirect-all: | |
| rule: 'HostRegexp(`{host:.+}`)' | |
| entrypoints: |
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Updated login flow: Email confirmation is now conditional, allowing sign-in when email functionality is disabled. - Introduced dynamic protocol handling for project previews and container deployments, enabling seamless HTTP/HTTPS transitions. - Added secure reverse proxy and TLS configurations to enhance connectivity and security. - **Chores** - Expanded configuration options for database and service settings. - Improved the build process with enhanced cleanup and modern networking adjustments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced file upload validation for project photos, ensuring only allowed image formats (JPEG, PNG, WebP) are accepted and that files do not exceed 5MB. - **Refactor** - Streamlined file handling during photo updates by centralizing validation logic, resulting in clearer error handling and improved reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
New Features
Refactor
Chores