Conversation
* Create dev-image.yml * Add test-local target for local testing setup Added test-local target to Makefile for local testing.
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Corrects mistranslation of device timestamp labels and fixes product name reference in site tunnel settings.
Updates the German translation to use "Standort" (site) instead of "Seite" (page) for consistency with the site context.
Summary of ChangesHello @Lokowitz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on modernizing and streamlining the application's Docker build and deployment pipeline. Key changes include adopting custom, more controlled Docker base images, centralizing database migration and build configurations for Drizzle ORM, and introducing a dedicated entrypoint script for robust application startup. These updates aim to improve consistency, security, and developer experience by simplifying local testing and aligning with newer language standards. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request overhauls the Docker build process, simplifies npm scripts, and introduces a container entrypoint script, enhancing security by running as a non-root user and adding a health check. However, a significant security concern is the insecure configuration in the new entrypoint.mjs script, which sets NODE_ENV to 'development' for production, potentially leading to information disclosure through verbose error messages and source code exposure. Additionally, a backup file Dockerfile.bak should be removed, the Dockerfile has redundant dependency installations, the health check script hardcodes a port, and the entrypoint script could better capture migration logs.
| @@ -0,0 +1,73 @@ | |||
| FROM dhi.io/node:24-alpine3.23-dev AS builder | |||
There was a problem hiding this comment.
This file appears to be a backup of the Dockerfile and should not be committed to the version control system. Please remove it from the repository.
| }); | ||
| } | ||
|
|
||
| process.env.NODE_ENV = 'development'; |
There was a problem hiding this comment.
Setting NODE_ENV to development in a production entrypoint.mjs script is insecure. This configuration can lead to information disclosure through verbose error messages (e.g., stack traces, file paths, environment variables) and disables critical security and performance optimizations. It should be set to 'production' for production environments.
| process.env.NODE_ENV = 'development'; | |
| process.env.NODE_ENV = 'production'; |
| async function startServer() { | ||
| return new Promise((resolve, reject) => { | ||
| console.log('Starting server with source maps enabled...'); | ||
| const server = spawn('node', ['--enable-source-maps', '/app/dist/server.mjs'], { |
There was a problem hiding this comment.
Enabling source maps in production via the --enable-source-maps flag can expose the original source code structure and logic through stack traces. When combined with NODE_ENV=development, this significantly increases the risk of information disclosure, making it easier for an attacker to understand the application's internal workings and identify further vulnerabilities.
| const server = spawn('node', ['--enable-source-maps', '/app/dist/server.mjs'], { | |
| const server = spawn('node', ['/app/dist/server.mjs'], { |
| RUN apk add --no-cache \ | ||
| g++ \ | ||
| make \ | ||
| python3 \ | ||
| tzdata |
There was a problem hiding this comment.
The builder stage and this runner stage both install the same set of build dependencies (g++, make, python3) using apk add. This redundancy increases build time and image size. To optimize this, you could create a common base stage that installs these dependencies once, and then have both builder and runner stages use it via FROM <base_stage_name>. This will leverage Docker's layer caching more effectively.
Example:
FROM dhi.io/node:24-alpine3.23-dev AS base
# Install build dependencies once
RUN apk add --no-cache \
g++ \
make \
python3
FROM base AS builder
# ... rest of builder stage, no need for apk add
FROM base AS runner
# ... rest of runner stage, no need for apk add| async function runMigrations() { | ||
| return new Promise((resolve, reject) => { | ||
| console.log('Running migrations...'); | ||
| const migrations = spawn('node', ['/app/dist/migrations.mjs']); |
There was a problem hiding this comment.
The runMigrations function spawns a child process for migrations but doesn't capture its standard output or error streams. This means any logs or error details from the migration script will be lost, making it difficult to debug failures. You should pipe the child's stdout and stderr to the parent process, similar to how you've done it for the startServer function.
| const migrations = spawn('node', ['/app/dist/migrations.mjs']); | |
| const migrations = spawn('node', ['/app/dist/migrations.mjs'], { stdio: 'inherit' }); |
| const req = http.get('http://localhost:3001/api/v1/', (res) => { | ||
| process.exit(res.statusCode === 200 ? 0 : 1); | ||
| }); | ||
| req.on('error', () => process.exit(1)); |
There was a problem hiding this comment.
The API port 3001 is hardcoded in the health check URL. This makes the health check brittle. If the API port is changed in the configuration, this script will fail. It's better to use an environment variable to specify the port, which can be set in the Dockerfile or at runtime.
| const req = http.get('http://localhost:3001/api/v1/', (res) => { | |
| process.exit(res.statusCode === 200 ? 0 : 1); | |
| }); | |
| req.on('error', () => process.exit(1)); | |
| const port = process.env.API_PORT || 3001; | |
| const req = http.get(`http://localhost:${port}/api/v1/`, (res) => { | |
| process.exit(res.statusCode === 200 ? 0 : 1); | |
| }); | |
| req.on('error', () => process.exit(1)); |
|
Warning This image may contain unchecked and breaking changes. Only use on own risk. 👋 Thanks for your PR! SQLITE Image: Postgresql Image: |
3 participants
Community Contribution License Agreement
By creating this pull request, I grant the project maintainers an unlimited,
perpetual license to use, modify, and redistribute these contributions under any terms they
choose, including both the AGPLv3 and the Fossorial Commercial license terms. I
represent that I have the right to grant this license for all contributed content.
Description
How to test?