fix(security): bind Caddy admin to local socket (GHSA-8jvv-gwqg-6vjc)#41847
Conversation
Caddy's admin endpoint now binds to a Unix socket so it is not reachable over TCP. Prometheus metrics move to a dedicated server block on :2019 so the existing Helm service-metrics keeps working. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/26465840382. |
WalkthroughThe Caddy configuration generator now derives a Unix socket path from TMP for the admin interface and creates a dedicated ChangesCaddy Admin and Metrics Configuration
Sequence Diagram(s)sequenceDiagram
participant Generator as caddy-reconfigure.mjs
participant CaddyAdminSocket as unix/CaddyAdminSocket
participant MetricsSite as :2019
Generator->>CaddyAdminSocket: write admin unix socket path (CaddyAdminSocketPath)
Generator->>MetricsSite: configure site to serve prometheus `metrics` on all routes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Deploy-Preview-URL: https://ce-41847.dp.appsmith.com |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs (1)
45-46: ⚡ Quick winDerive the admin socket path from the same temp root.
CaddyfilePathcomes fromprocess.env.TMP, but the new admin socket is hard-coded under/tmp/appsmith. Keeping both under the same base avoids hidden coupling and makes the directory creation at Line 220 cover the socket path too.♻️ Suggested cleanup
const CaddyfilePath = process.env.TMP + "/Caddyfile" +const CaddyAdminSocketPath = process.env.TMP + "/caddy.sock" ... - admin unix//tmp/appsmith/caddy.sock + admin unix/${CaddyAdminSocketPath}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs` around lines 45 - 46, The admin socket path is hard-coded as "unix//tmp/appsmith/caddy.sock" while CaddyfilePath derives its base from process.env.TMP; change the admin socket to be derived from the same TMP root (use the base from CaddyfilePath or process.env.TMP) so both use the same temp root, and update the directory-creation logic that currently creates the TMP root directory to also ensure the socket directory exists; reference the CaddyfilePath variable and replace the literal "unix//tmp/appsmith/caddy.sock" with a path built from that TMP root.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs`:
- Around line 45-46: The admin socket path is hard-coded as
"unix//tmp/appsmith/caddy.sock" while CaddyfilePath derives its base from
process.env.TMP; change the admin socket to be derived from the same TMP root
(use the base from CaddyfilePath or process.env.TMP) so both use the same temp
root, and update the directory-creation logic that currently creates the TMP
root directory to also ensure the socket directory exists; reference the
CaddyfilePath variable and replace the literal "unix//tmp/appsmith/caddy.sock"
with a path built from that TMP root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2fb25ad-b1c7-4fab-a6cf-73353e22c206
📒 Files selected for processing (1)
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs
Both CaddyfilePath and the socket now share a single source of truth (process.env.TMP) so they stay aligned if the temp root ever moves. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
/build-deploy-preview skip-tests=true recreate=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/26468640489. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs (1)
224-224:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid/un-gate the unconditional
caddy reloadincaddy-reconfigure.mjs(current--addressconcern is likely overstated)
caddy-reconfigure.mjsrunsspawnSync(... ["reload","--config", CaddyfilePath])right after writing the Caddyfile, but the container entrypoint callsnode caddy-reconfigure.mjsbefore starting Caddy—so this reload is unnecessary and can fail silently (return value isn’t checked). The specific claim that it will always hitlocalhost:2019due to missing--addressdoesn’t match howcaddy reload --configdetermines the admin endpoint (it should use the admin settings from the provided config, falling back tolocalhost:2019only when not derivable). Remove the reload fromcaddy-reconfigure.mjs, or gate it / checkspawnSyncresults and only run reload when Caddy is already running.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs` at line 224, The unconditional call to spawnSync(AppsmithCaddy, ["reload", "--config", CaddyfilePath]) in caddy-reconfigure.mjs is unsafe because the script runs before Caddy is started; remove this unconditional reload or gate it by detecting a running Caddy instance (e.g., attempt a lightweight check against the admin endpoint or test spawnSync(AppsmithCaddy, ["status"])/inspect its exit code) and only call spawnSync(... ["reload", "--config", CaddyfilePath]) when that check indicates Caddy is active; if you keep the reload, capture and check spawnSync’s return value (status, stdout/stderr) and log or surface failures instead of ignoring them (referencing spawnSync, AppsmithCaddy, and CaddyfilePath to locate the call).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs`:
- Line 224: The unconditional call to spawnSync(AppsmithCaddy, ["reload",
"--config", CaddyfilePath]) in caddy-reconfigure.mjs is unsafe because the
script runs before Caddy is started; remove this unconditional reload or gate it
by detecting a running Caddy instance (e.g., attempt a lightweight check against
the admin endpoint or test spawnSync(AppsmithCaddy, ["status"])/inspect its exit
code) and only call spawnSync(... ["reload", "--config", CaddyfilePath]) when
that check indicates Caddy is active; if you keep the reload, capture and check
spawnSync’s return value (status, stdout/stderr) and log or surface failures
instead of ignoring them (referencing spawnSync, AppsmithCaddy, and
CaddyfilePath to locate the call).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16be9311-bb45-409d-8f0d-c5ab268fd30e
📒 Files selected for processing (1)
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs
2 participants
Summary
Security fix for GHSA-8jvv-gwqg-6vjc. Caddy's admin endpoint now binds to a Unix socket instead of TCP; Prometheus metrics keep their previous port for scrape-config compatibility.
What changed
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs— admin moved to a Unix socket, dedicated:2019 { metrics }block for Prometheus, no Helm changes.🤖 Generated with Claude Code
Summary by CodeRabbit
Warning
Tests have not run on the HEAD c49fba7 yet
Tue, 26 May 2026 18:54:36 UTC