Security fixes from code review (F1-F4)#2
Merged
Conversation
… CVEs from EOL base)
- JSON-parse guard before new Function: JSON configs bypass RCE path (F1 mitigation) - HTML-escape msg in failSvg SVG body (F2, XSS in error output) - Add return after db.get err + fix ReferenceError outputFormat->fmt (F3, crash+hang) - Explicit limit on express.urlencoded (F4, DoS hardening)
There was a problem hiding this comment.
Pull request overview
This PR applies several post–code review security hardening changes across request parsing, error rendering, template rendering, and the container base image.
Changes:
- Add a strict
JSON.parsefast-path inrenderChartJsto avoid thenew Functionexecution path for JSON chart configs. - Escape error text in
failSvg, fix error-path fallthrough +ReferenceErrorin/chart/render/:key, and set an explicit body size limit forexpress.urlencoded. - Update Docker base image from Node 18 Alpine to Node 22 Alpine to reduce known CVEs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/charts.js |
Adds a JSON-parse guard to bypass new Function when input is strict JSON. |
index.js |
Adds urlencoded body size limit, HTML-escapes SVG error content, and fixes /chart/render/:key error handling + format variable usage. |
Dockerfile |
Updates runtime base image to Node 22 Alpine. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+141
to
+145
| } catch (_) { | ||
| // Not valid JSON — fall through to the new Function path below. | ||
| // Log so operators can see if unexpected JS function configs arrive. | ||
| logger.warn('chart input is not valid JSON; running via new Function (JS function config)'); | ||
| } |
Comment on lines
547
to
548
| let chartConfig = JSON.parse(row.config); | ||
| chartConfig = applyTemplateOverrides(chartConfig, req.query); |
| @@ -1,4 +1,4 @@ | |||
| FROM node:18-alpine3.17 | |||
| FROM node:22-alpine | |||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security fixes applied after code review: