fix: CVE-2024-38821 - #41221
Conversation
WalkthroughThe Dockerfile now validates only the presence of server/mongo/server.jar and updates the associated error message. The prepare_server_artifacts.sh script removes PostgreSQL artifact retrieval and related steps, retaining only MongoDB artifact preparation with updated messages and comments. No other build logic or error handling changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Script as prepare_server_artifacts.sh
participant FS as Filesystem
participant Docker as Docker build
Note over Script: Simplified to MongoDB-only artifacts
Dev->>Script: Run artifact preparation
Script->>FS: Copy app/server/dist -> target/mongo
Script->>FS: Rename server-*.jar -> server.jar
Note over Script,FS: No PostgreSQL artifact steps
Dev->>Docker: Build image
Docker->>FS: Check server/mongo/server.jar exists
alt Mongo jar present
Docker-->Dev: Proceed with build
else Missing Mongo jar
Docker-->Dev: Fail with error (exit 1)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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. Comment Pre-merge checks✅ Passed checks (3 passed)
|
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17665653070. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
Dockerfile (1)
28-31: Use absolute path for artifact check (avoid WORKDIR assumptions).The prepare script writes to /opt/appsmith/server/mongo/server.jar. Checking server/mongo/server.jar assumes WORKDIR=/opt/appsmith; make the path explicit to prevent false negatives.
Apply:
- if ! [ -f server/mongo/server.jar ]; then - echo "Missing MongoDB server.jar file. Are you using the build script?" >&2 + if ! [ -f /opt/appsmith/server/mongo/server.jar ]; then + echo "Missing /opt/appsmith/server/mongo/server.jar. Did you run scripts/prepare_server_artifacts.sh?" >&2 exit 1 fiIf you prefer relative paths, please confirm the base image sets
WORKDIR /opt/appsmith.scripts/prepare_server_artifacts.sh (2)
12-12: Minor: keep logs ASCII-only for CI portability.Emojis can render poorly in some terminals/CI. Optional tweak below.
24-26: Optional: replace emojis with plain tags for consistent logs.-echo "✅ MongoDB artifacts prepared successfully" -echo "🗑️ PostgreSQL artifacts skipped (CVE-2024-38821 eliminated)" -echo "📁 Only MongoDB artifacts: $target/mongo/" +echo "[ok] MongoDB artifacts prepared successfully" +echo "[info] PostgreSQL artifacts skipped (CVE-2024-38821 eliminated)" +echo "[path] Only MongoDB artifacts: $target/mongo/"
| # Build MongoDB server artifacts | ||
| cp -r "app/server/dist" "$target/mongo" | ||
| mv "$target/mongo"/server-*.jar "$target/mongo/server.jar" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against 0 or >1 server-*.jar matches.
mv "$target/mongo"/server-*.jar ... is brittle if the glob matches none or multiple files.
Apply:
cp -r "app/server/dist" "$target/mongo"
-mv "$target/mongo"/server-*.jar "$target/mongo/server.jar"
+shopt -s nullglob
+jars=( "$target/mongo"/server-*.jar )
+if (( ${#jars[@]} == 1 )); then
+ mv "${jars[0]}" "$target/mongo/server.jar"
+else
+ echo "Expected exactly one server-*.jar in $target/mongo, found ${#jars[@]}: ${jars[*]}" >&2
+ exit 1
+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.
| # Build MongoDB server artifacts | |
| cp -r "app/server/dist" "$target/mongo" | |
| mv "$target/mongo"/server-*.jar "$target/mongo/server.jar" | |
| # Build MongoDB server artifacts | |
| cp -r "app/server/dist" "$target/mongo" | |
| shopt -s nullglob | |
| jars=( "$target/mongo"/server-*.jar ) | |
| if (( ${#jars[@]} == 1 )); then | |
| mv "${jars[0]}" "$target/mongo/server.jar" | |
| else | |
| echo "Expected exactly one server-*.jar in $target/mongo, found ${#jars[@]}: ${jars[*]}" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In scripts/prepare_server_artifacts.sh around lines 18–21, the mv using the glob
"server-*.jar" can fail or pick the wrong file when the glob expands to zero or
multiple matches; change the logic to enable safe globbing (e.g. set nullglob),
collect matches into an array, verify the array length is exactly 1 and exit
with a clear error if it is 0 or >1, then mv that single element to
"$target/mongo/server.jar". Ensure the script uses an explicit check and error
message instead of relying on mv's behavior.
| # PostgreSQL support has been removed - no more vulnerable artifacts | ||
| # This eliminates CVE-2024-38821 from the Docker image | ||
| echo "✅ MongoDB artifacts prepared successfully" | ||
| echo "🗑️ PostgreSQL artifacts skipped (CVE-2024-38821 eliminated)" | ||
| echo "📁 Only MongoDB artifacts: $target/mongo/" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good call removing PG artifacts for CVE mitigation; verify no lingering PG references.
Looks aligned with the Dockerfile change. Let’s ensure no PG paths/tags remain elsewhere.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find lingering references to PG artifacts/tags/paths
rg -nC2 -g '!**/node_modules/**' -P '(?i)\b(server/pg|pg/server\.jar|PG_TAG|postgresql)\b'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching repo for PostgreSQL references..."
# Search broadly (include ignored/hidden files) for common postgres/postgresql references and known artifact names.
rg -nC2 --hidden --no-ignore -i -e "postgres" -e "postgresql" -e "PG_TAG" -e "server/pg" -e "pg/server\.jar" -e "FROM postgres" -e "postgres:" || trueLength of output: 263951
Action required — Postgres artifacts remain; CVE mitigation incomplete
prepare_server_artifacts.sh removal is fine, but the image and startup still reference/install Postgres — the vulnerable artifacts will remain unless these are removed or disabled.
- Key locations to fix:
- deploy/docker/base.dockerfile (installs postgresql-14; PATH /usr/lib/postgresql/14/bin)
- deploy/docker/fs/opt/appsmith/{run-postgres.sh, pg-upgrade.sh, pg-utils.sh, entrypoint.sh — init_postgres/safe_init_postgres/create_appsmith_pg_db, supervisord/postgres.conf, postgres/appsmith_hba.conf}
- deploy/docker/fs/opt/appsmith/templates/docker.env.sh (APPSMITH_POSTGRES_DB_URL)
- deploy/helm/* (values.yaml, templates/deployment.yaml, templates/configMap.yaml — postgresql.enabled / init container)
- deploy/docker/tests/* (test-pg-utils.sh, pg-test-utils.sh, pg-upgrade tests)
- scripts/trivy_vulnerabilities_data.sh (psql usage)
Action: remove Postgres package/install and embedded startup files from the Docker image (or fully disable embedded Postgres and ensure no postgres packages are installed), then re-run vulnerability scan and grep to confirm no remaining runtime artifacts.
🤖 Prompt for AI Agents
In scripts/prepare_server_artifacts.sh around lines 22 to 26, the comment claims
PostgreSQL artifacts were skipped but the repository still installs and
references Postgres in multiple places; remove or disable those runtime
artifacts to fully mitigate CVE-2024-38821. Remove postgresql-14 installation
and any PATH additions from deploy/docker/base.dockerfile; delete or disable
run-postgres.sh, pg-upgrade.sh, pg-utils.sh, entrypoint
init_postgres/safe_init_postgres/create_appsmith_pg_db logic,
supervisord/postgres.conf and postgres/appsmith_hba.conf under
deploy/docker/fs/opt/appsmith (or ensure they are not copied into image); remove
APPSMITH_POSTGRES_DB_URL and related templates from
deploy/docker/fs/opt/appsmith/templates/docker.env.sh; update deploy/helm values
and templates to turn off postgresql.enabled and remove init
containers/configMap entries that provision Postgres; update or remove docker
tests that exercise pg utilities under deploy/docker/tests and any scripts using
psql (e.g., scripts/trivy_vulnerabilities_data.sh); after changes rebuild the
image and re-run vulnerability scan and a repo-wide grep for "postgres", "psql",
"postgresql-14" and related init scripts to confirm no Postgres binaries,
configs, or startup hooks remain in the final image.
|
Deploy-Preview-URL: https://ce-41221.dp.appsmith.com |
|
/build-deploy-preview skip-tests=false |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17666291386. |
|
Deploy-Preview-URL: https://ce-41221.dp.appsmith.com |
2 participants
Description
Before:
The appsmith-ce release image contains CVE-2024-38821 critical vulnerability.
cves_report_ce.json
After:
The current DP image doesn't contain CVE-2024-38821 after removing pg build from server.
cves_41221.txt
Fixes CVE-2024-38821
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/17725447283
Commit: 959d97e
Cypress dashboard.
Tags:
@tag.SanitySpec:
Mon, 15 Sep 2025 08:39:53 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit