Image assets (1/3): Signal K plugin — secure upload, processing, worker pool, cache - #1080
Image assets (1/3): Signal K plugin — secure upload, processing, worker pool, cache#1080dillan wants to merge 5 commits into
Conversation
First slice of user-uploaded image assets (server). ImageStore is an Express-free, unit-tested core that: - detects image type by content magic bytes (png/jpeg/webp/gif/heic/svg), not the client extension/MIME, and rejects anything else; - sanitizes SVG with DOMPurify (strips scripts/handlers/external refs) and stores it as vector, rejecting SVGs that don't survive sanitization; - validates raster decodability + dimensions via sharp with a pixel-budget guard; - stores the original bytes plus a per-image sidecar JSON of metadata (id-addressed UUIDs, no client path), and supports list/getMeta/remove with id validation. Adds the server deps (multer, sharp, heic-convert, isomorphic-dompurify) and runs all kip-plugin test files. On-demand WebP re-encode/resize + cache + worker pool and the HTTP routes build on this next.
Builds the serving layer on top of ImageStore: - image-processing: convert/resize a validated original to WebP, preserving animation (GIF/WebP) and decoding HEIC/HEIF via heic-convert; widths snapped to an allow-list; worker-count helper = n-1 CPUs (clamped to 1). - worker-pool: runs processing in a worker_threads pool (default n-1) so the SK server stays responsive; concurrent requests for the same variant are coalesced into one job. - image-worker: the pool's worker entry (sharp.concurrency(1) per worker). - ImageStore.getServable: serves sanitized SVG as-is, or an on-demand WebP variant that is cached on disk and re-served thereafter; raster is always re-encoded (raw original never served); every response carries nosniff + restrictive CSP + inline headers. Adds cacheStats()/purgeCache() for the settings UI. 22 node:test cases cover snapping, animation preservation, on-demand+cache, SVG passthrough, cache stats/purge, safe headers, and worker-pool sizing/coalescing. Fixes the plugin test script to run every *.test.cjs file.
image-router registers the /images endpoints on the plugin's Express router: - POST /images upload (multer, 10MB, login required, auth checked before parsing) - GET /images list the shared library - GET /images/:id?w= serve a cached WebP variant (raster) or sanitized SVG, with safe headers - DELETE /images/:id remove (login required) - GET/DELETE /images/cache cache size + purge (purge login required) Auth: SK's security middleware is the primary gate (as for the existing displays routes); a defensive isAuthenticatedRequest() also checks the SK-set req.skPrincipal so anonymous users are refused when security is enabled, while a server with no security configured stays permissive. Cache routes are registered before /images/:id so "cache" is not matched as an id; ids are UUID-validated. index.ts lazily builds the ImageStore + worker pool on first route use (the data dir is only known after init) and tears the pool down on stop(). 8 route tests cover auth rejection, listing, serving + headers, id validation/404, cache stats, and not-ready 503. Full plugin suite: 87 green.
…view HIGH — worker pool could permanently wedge all image serving: a worker that died via native crash/OOM/hard-exit (no 'error' event) silently left the pool, which never respawned, so queued jobs (and their HTTP requests) hung forever. The pool now detects deaths via BOTH 'error' and 'exit', rejects the in-flight job, and spawns a replacement, and a per-job timeout rejects+replaces a hung worker. (tests: crash-recovery, timeout-recovery) HIGH — HEIC decode bypassed the pixel budget: heic-convert decodes the full raster before sharp, so a 12KB/49MP HEIC forced ~632MB per serve x 7 widths. HEIC is now transcoded to a canonical WebP ONCE at upload, bounded by a strict 24MP HEIC cap, and served as cheap WebP resizes thereafter (no per-request HEIC decode). LOW — un-serveable "poison" assets: a non-HEVC HEIF / brand-spoofed AVIF detected as HEIC was stored but failed on every GET. The ingest transcode now proves decodability, so such files are rejected up front; the HEIC brand allow-list is narrowed to HEVC brands. (test: AVIF-as-HEIC) LOW — no upload quota let an authenticated user fill the shared data volume over time. Added a configurable image-count + total-bytes cap, checked before any decode work. (tests: both quotas) Full plugin suite: 92 green.
…egexes DOMPurify was only forbidding the legacy xlink:href attribute, so a modern href on <image>/<a>/<feImage> survived sanitization — an external href is a phone-home beacon in the stored SVG bytes, which is supposed to be self-safe (defense in depth beyond the serve-path sandbox CSP). Forbid 'href' as well. Internal references drawings rely on (fill="url(#g)", filter="url(#f)") are not href attributes and are preserved; added a test covering both. Also replace two regexes that contained literal control bytes (a raw BOM and a raw NUL..0x1f range) with \uFEFF / \x00-\x1f escape sequences. Behavior is identical, but the source no longer reads as a binary file to git/grep/editors.
|
This would or could be useful as an independent plugin or as an extendion of server’s applicationData scaffolding. There is nothing inherently KIP specific, is there? |
Not much Kip specific other than the widget & widget configuration. If there is enough interest I could split things out into a separate image handling plugin. I didn't go that route initially because I think it's valuable to give users the "batteries included" experience when and where possible. Making users install a plugin first adds friction. @godind Mind sharing your perspective on this discussion? |
|
I agree with both of you: I like very much the "batteries included" mindset and I'm more than happy to add a new camera display widget (it's been requested a few times over the last 2-3 years) but I also agree that the video processing engine part would be better leveraged as a server plugin/feature. That way, as Teppo says, you have applicationData and other features that could open the door to say, recording storage, re-encoding, YouTube publishing, dropbox sync and what not, without KIP running. Leave only the display and control part as KIP a widget ;) |
|
Oh and KIP has the ability to add required plugins to widgets, auto enable plugins and push config, not sure I have done the autoinstall part yet...maybe, but not a big deal to add. That way when you add the widget, KIP asks the server to install and enable it. |
|
Ok, I'll split this out. Thanks! |
|
Closing in favor of the standalone plugin. The image server engine in this PR became the standalone SK Image Signal K plugin ( |
3 participants
Image assets (1/3) — Signal K plugin: secure upload, on-demand processing, worker pool, cache
Part 1 of 3 adding user-uploaded image assets to KIP dashboards (e.g. a diagram of where safety equipment is stowed on the boat). This PR is the server side only — the Express routes the KIP plugin exposes under
/plugins/kip. The Angular client (widget + config UI) follows in PRs 2 and 3.This PR is independent and can be reviewed/merged on its own; the client PRs depend on it at runtime.
What it does
worker_threadspool (sizemax(1, nCPU-1)) runs every sharp/heic-convert job, with in-flight coalescing so concurrent requests for the same uncached variant do the work once. Per-job timeout + crash/exit recovery so a bad image can't wedge the pool.fit:'inside', withoutEnlargement:true, width snapped to a fixed allow-list[160,320,640,960,1280,1920,2560]. Animated GIF → animated WebP. SVG is served as sanitized vector (not resized).Security (untrusted uploads)
limitInputPixels/ HEIC pixel cap guard decompression bombs.USE_PROFILES:{svg,svgFilters},FORBID_TAGS:[script,foreignObject], stripson*+ external refs); rejected if emptied. Served asimage/svg+xmland only ever rendered via<img src>, where scripts don't run.Content-Type,X-Content-Type-Options: nosniff, restrictive CSP (default-src 'none'; sandbox),Content-Disposition: inline, long immutable cache.:idvalidated/^[A-Za-z0-9-]+$/(no traversal). Upload storage quota (count + total bytes) caps disk use.The malicious-upload surface went through two rounds of adversarial review. The first hardened the 4 empirically-confirmed issues it found (worker crash/timeout recovery, HEIC transcode-at-ingest with a pixel cap, upload storage quota, poison-asset rejection via decodability check + narrowed HEIC brand allow-list). A second pre-publication pass found and fixed an SVG sanitizer gap: DOMPurify was only forbidding the legacy
xlink:href, so a modernhrefon<image>/<a>/<feImage>survived as a phone-home beacon in the stored bytes — now both are forbidden (internalfill="url(#id)"/filter="url(#id)"references are preserved), with a test covering it.Endpoints (under
/plugins/kip)POST /images·GET /images·GET /images/:id?w=·DELETE /images/:id·GET /images/cache·DELETE /images/cacheNew dependencies
sharp(prebuilt ARM/Pi binaries),heic-convert(portable HEIC decode — avoids needing libheif in the sharp build),multer,isomorphic-dompurify. Worker pool uses built-inworker_threads.package-lock.jsonis regenerated: it adds the 4 deps and their platform binaries (@img/sharp-*, jsdom-related), andnpm installalso pruned some orphaned protractor/selenium-era lockfile entries that were no longer referenced bypackage.json. No devDependency or intentional dependency was changed beyond the 4 additions.lockfileVersionis unchanged (3).Tests
kip-plugin/tests/*.test.cjs(node:test): store validate/sanitize/store, on-demand generation + cache, HEIC→WebP, animated GIF→animated WebP, SVG script/onload/external-href neutralized, traversal id rejected, serving headers,?wsnap + cache, coalescing, worker crash/hang recovery, quota + poison-asset rejection, anonymous upload/delete/purge refused. Run withnpm run test:plugin.Status / gates
npm run build:plugin(tsc) clean;npm run test:plugin→ 93/93 pass.package-lock.json: the diff is large becausenpm installof the 4 deps records all platform binaries (@img/sharp-*,@esbuild/*) and also pruned a cluster of orphaned protractor/selenium/request-era entries that were no longer referenced bypackage.json. No devDependency or intentional dependency changed beyond the 4 additions,lockfileVersionstayed 3, and the lockfile is internally consistent (npm ciresolves). Happy to squash/regenerate the lockfile however you prefer on merge.This is part 1 of a 3-PR stack; the Angular client (PR 2) and config UI (PR 3) follow and depend on this at runtime.
📚 3-PR stack (review/merge in order)
PR 2 needs PR 1 at runtime; PR 3 is stacked on PR 2. Together they add the full image-assets feature.
You are viewing PR 1 of 3.