fix: handle potentially undefined env variables in nginx template#41648
fix: handle potentially undefined env variables in nginx template#41648leno23 wants to merge 1 commit intoappsmithorg:releasefrom
Conversation
WalkthroughRefactored Nginx configuration template to use variable-based substitution instead of direct placeholder interpolation in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/client/docker/templates/nginx-app.conf.template`:
- Around line 46-48: The template currently exposes the OTLP license key via the
variable set $appsmith_new_relic_otlp_license_key
(APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY), which must not be sent to the browser;
replace uses of APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY with the New Relic browser
monitoring key environment variable (e.g.
APPSMITH_NEW_RELIC_BROWSER_MONITORING_KEY) and update the template variable name
accordingly (e.g. set $appsmith_new_relic_browser_key) wherever
$appsmith_new_relic_otlp_license_key appears (lines near the current set
$appsmith_new_relic_otlp_license_key and the similar block at lines ~66-68),
ensuring the OTLP key is never exposed client-side.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00a22aa0-b7a4-4a3c-ab6e-0780c329ebb9
📒 Files selected for processing (1)
app/client/docker/templates/nginx-app.conf.template
| set $appsmith_new_relic_otlp_license_key '${APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY}'; | ||
| set $appsmith_new_relic_otel_exporter_otlp_endpoint '${APPSMITH_NEW_RELIC_OTEL_EXPORTER_OTLP_ENDPOINT}'; | ||
| set $appsmith_frontend_tracing_url '${APPSMITH_FRONTEND_TRACING_URL}'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is a New Relic OTLP license key safe to expose in browser-delivered frontend code? Please cite official New Relic documentation and the recommended key type for browser telemetry ingestion.
💡 Result:
No, a New Relic OTLP license key (account license key or ingest key) is not safe to expose in browser-delivered frontend code. OTLP ingest uses the account license key as the api-key header, which allows sending arbitrary telemetry data to your New Relic account. Exposing it client-side risks abuse, as evidenced by security concerns raised when such keys are exposed in source code. New Relic documentation explicitly distinguishes: license keys are for most data ingest (except browser/mobile), while browser telemetry uses a separate, safer browser key (browser_monitoring_key), which is designed for client-side exposure in the browser agent JavaScript snippet. There is no official support or recommendation for using OTLP directly from browsers; browser monitoring uses proprietary endpoints and the browser key, not OTLP/license key.
Citations:
- 1: https://docs.newrelic.com/docs/opentelemetry/best-practices/opentelemetry-otlp-attributes-processing-update/
- 2: https://docs.newrelic.com/docs/opentelemetry/best-practices/opentelemetry-otlp/
- 3: https://docs.newrelic.com/docs/browser/browser-monitoring/configuration/browser-license-key-app-id/
- 4: https://docs.newrelic.com/docs/apis/intro-apis/new-relic-api-keys/
- 5: https://docs.newrelic.com/docs/opentelemetry/best-practices/opentelemetry-otlp-troubleshooting/
Replace OTLP license key with New Relic browser-specific key for frontend exposure.
Lines 46 and 66 expose APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY to the browser. OTLP license keys are account credentials that allow arbitrary telemetry ingestion and must never be exposed client-side. Use the browser-specific browser_monitoring_key instead, designed for safe client-side exposure and recommended by New Relic for browser telemetry.
Also applies to: 66-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/docker/templates/nginx-app.conf.template` around lines 46 - 48,
The template currently exposes the OTLP license key via the variable set
$appsmith_new_relic_otlp_license_key (APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY),
which must not be sent to the browser; replace uses of
APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY with the New Relic browser monitoring key
environment variable (e.g. APPSMITH_NEW_RELIC_BROWSER_MONITORING_KEY) and update
the template variable name accordingly (e.g. set
$appsmith_new_relic_browser_key) wherever $appsmith_new_relic_otlp_license_key
appears (lines near the current set $appsmith_new_relic_otlp_license_key and the
similar block at lines ~66-68), ensuring the OTLP key is never exposed
client-side.
1 participant
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit