From cc75dd008856d236422a2ae443bd4ae8ec00f0e2 Mon Sep 17 00:00:00 2001 From: Afri <220195275+l5yth@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:35:02 +0200 Subject: [PATCH] web: sandbox the OG capture and harden frontend escaping and lookups (#994) --- .env.example | 3 + CHANGELOG.md | 4 ++ Dockerfile | 1 + README.md | 1 + docker-compose.yml | 1 + flake.nix | 8 +++ web/Dockerfile | 1 + web/lib/potato_mesh/config.rb | 15 ++++ web/lib/potato_mesh/og_image.rb | 68 ++++++++++++++----- .../js/app/__tests__/chat-format.test.js | 9 +++ .../js/app/__tests__/federation-page.test.js | 24 +++++++ .../assets/js/app/__tests__/live-dom-model.js | 29 ++++++-- .../js/app/__tests__/role-helpers.test.js | 15 ++++ web/public/assets/js/app/chat-format.js | 3 +- web/public/assets/js/app/federation-page.js | 17 +---- .../js/app/main/__tests__/flash.test.js | 43 +++++++++++- web/public/assets/js/app/main/flash.js | 16 +++-- web/public/assets/js/app/role-helpers.js | 14 +++- web/spec/config_spec.rb | 20 ++++++ web/spec/og_image_spec.rb | 58 +++++++++++++--- 20 files changed, 293 insertions(+), 57 deletions(-) diff --git a/.env.example b/.env.example index 4851ffbe..ebc2822a 100644 --- a/.env.example +++ b/.env.example @@ -87,6 +87,9 @@ MESHTASTIC_FREQ="915MHz" # Absolute http(s) URL for the social preview image. Replaces the generated # /og-image.png for deployments without Chromium. # OG_IMAGE_URL="https://cdn.example.org/preview.png" +# Set to 1 to run the Chromium that renders /og-image.png without its sandbox (the Docker images set 1) +OG_IMAGE_NO_SANDBOX=1 + # Map center coordinates (latitude, longitude) # Berlin, Germany: 52.502889, 13.404194 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e028879..675a6175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ * Data: `TRANSPORT=udp` listens on both Meshtastic multicast groups by default, `239.0.0.69` (firmware 2.8+) and `224.0.0.69`; `MESH_UDP_GROUP` takes a comma-separated list. If you set `MESH_UDP_GROUP=224.0.0.69` before, remove it to also hear firmware 2.8+ nodes (#903) * Data: `MESHCORE_TELEMETRY_POLL_24H_EXEMPT` reaches every packaged deployment - `docker-compose.yml`, both `data/Dockerfile` stages and the NixOS module (`meshcoreTelemetryPoll24hExempt`); setting it in `.env` had no effect under Compose or NixOS (SPEC PX5) * Web: live refreshes keep map overlays, line tooltips and waypoint cards open, and repaint only the table, map or chat whose data changed (#881, SPEC DR2/DR4) +* Web: a node role or modem preset named like a built-in object property, such as `constructor`, gets its protocol's base color and its derived preset code instead of an uncolored badge or a `[FU]` tag (SPEC FE1) +* Web: a node id containing `"` no longer stops the live-update flashes of a refresh (SPEC FE2) +* Web: the Chromium that renders `/og-image.png` keeps its sandbox unless `OG_IMAGE_NO_SANDBOX=1`, which the Docker images and the Compose default set, and keeps the same-origin policy and site isolation on (SPEC FE3/FE4) +* Web: the federation page shows a numeric frequency or version instead of an empty cell (SPEC FE5) * Docs: the README documents `MESHTASTIC_PSK_B64` (SPEC PX7) * CI: the env-surface check runs in pytest whenever a deployment surface changes, and also checks names with digits, names read through helpers, and retired names (SPEC PX6/PX8) * CI: test results reach Codecov also when tests fail; workflows use codecov-action v7 and checkout v7; Dependabot opens up to 20 GitHub Actions update PRs at a time (SPEC DW1/DW2/DW4) diff --git a/Dockerfile b/Dockerfile index 4fd2436d..4550a8a5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -147,6 +147,7 @@ ENV RACK_ENV=production \ ANNOUNCEMENT="" \ EVENTS=1 \ OG_IMAGE_URL="" \ + OG_IMAGE_NO_SANDBOX=1 \ PAGES_DIR=/app/pages \ PROM_REPORT_IDS="" \ FEDERATION=1 \ diff --git a/README.md b/README.md index e09072ca..a7a616a3 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,7 @@ The web app can be configured with environment variables (defaults shown): | `MIN_THREADS` | `16` | Minimum Puma worker threads kept warm. | | `MAX_THREADS` | `96` | Maximum Puma worker threads. Each active `/api/events` SSE stream pins one thread, so keep this above your peak concurrent SSE clients plus API/ingest headroom. | | `OG_IMAGE_URL` | _unset_ | Absolute `http(s)://` URL for the social preview image; other schemes are ignored. Replaces the generated `/og-image.png`. Use HTTPS - most platforms won't render an HTTP preview. | +| `OG_IMAGE_NO_SANDBOX` | `0` | Set to `1` to run the Chromium that renders `/og-image.png` without its sandbox. The Docker images set `1`. With `0` where Chromium cannot start its sandbox, no preview is captured and `/og-image.png` serves its fallback image. | | `PAGES_DIR` | `./pages` | The directory for static, custom-content pages. | | `PROM_REPORT_IDS` | _unset_ | Comma-separated node ids to expose as per-node Prometheus gauges. Empty exports none. Opted-out nodes, and hidden clients under `PRIVATE=1`, are never exported. | diff --git a/docker-compose.yml b/docker-compose.yml index b2f61aa2..ca11af5e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -39,6 +39,7 @@ x-web-base: &web-base ANNOUNCEMENT: ${ANNOUNCEMENT:-} EVENTS: ${EVENTS:-1} OG_IMAGE_URL: ${OG_IMAGE_URL:-} + OG_IMAGE_NO_SANDBOX: ${OG_IMAGE_NO_SANDBOX:-1} PAGES_DIR: ${PAGES_DIR:-/app/pages} PROM_REPORT_IDS: ${PROM_REPORT_IDS:-} command: ["ruby", "app.rb", "-p", "41447", "-o", "0.0.0.0"] diff --git a/flake.nix b/flake.nix index 32d03f8c..f7bdebae 100644 --- a/flake.nix +++ b/flake.nix @@ -299,6 +299,12 @@ description = "Absolute http(s) URL for the social preview image, replacing the generated /og-image.png"; }; + ogImageNoSandbox = lib.mkOption { + type = lib.types.bool; + default = false; + description = "Run the Chromium that renders /og-image.png without its sandbox"; + }; + pagesDir = lib.mkOption { type = lib.types.nullOr lib.types.str; default = null; @@ -525,6 +531,8 @@ ANNOUNCEMENT = cfg.announcement; } // lib.optionalAttrs (cfg.ogImageUrl != null) { OG_IMAGE_URL = cfg.ogImageUrl; + } // lib.optionalAttrs cfg.ogImageNoSandbox { + OG_IMAGE_NO_SANDBOX = "1"; } // lib.optionalAttrs (cfg.pagesDir != null) { PAGES_DIR = cfg.pagesDir; } // lib.optionalAttrs (cfg.promReportIds != null) { diff --git a/web/Dockerfile b/web/Dockerfile index d135812b..54f0459b 100644 --- a/web/Dockerfile +++ b/web/Dockerfile @@ -144,6 +144,7 @@ ENV RACK_ENV=production \ ANNOUNCEMENT="" \ EVENTS=1 \ OG_IMAGE_URL="" \ + OG_IMAGE_NO_SANDBOX=1 \ PAGES_DIR=/app/pages \ PROM_REPORT_IDS="" \ FEDERATION=1 \ diff --git a/web/lib/potato_mesh/config.rb b/web/lib/potato_mesh/config.rb index f77a1d86..d1f3b0bd 100644 --- a/web/lib/potato_mesh/config.rb +++ b/web/lib/potato_mesh/config.rb @@ -900,6 +900,21 @@ module PotatoMesh fetch_string("OG_IMAGE_URL", nil) end + # Determine whether the +/og-image.png+ capture launches Chromium + # without its sandbox (+--no-sandbox+). + # + # Off unless +OG_IMAGE_NO_SANDBOX=1+: the capture renders the dashboard, + # which loads third-party map tiles and shows mesh-supplied text, so + # Chromium keeps its sandbox wherever it can start one. Set it where + # Chromium cannot, such as a container with neither unprivileged user + # namespaces nor a setuid +chrome-sandbox+ helper; the Docker images set + # +1+ (see {PotatoMesh::OgImage.chromium_flags}). + # + # @return [Boolean] true when OG_IMAGE_NO_SANDBOX=1 in the environment. + def og_image_no_sandbox? + fetch_string("OG_IMAGE_NO_SANDBOX", "0") == "1" + end + # Cache lifetime for runtime-generated +/og-image.png+ responses, in # seconds. Successful captures are stored on disk and reused until the # TTL elapses; the next request after expiry refreshes the cache diff --git a/web/lib/potato_mesh/og_image.rb b/web/lib/potato_mesh/og_image.rb index 1269e26f..3323ae44 100644 --- a/web/lib/potato_mesh/og_image.rb +++ b/web/lib/potato_mesh/og_image.rb @@ -187,22 +187,25 @@ module PotatoMesh Ferrum::Browser.new(browser_options) end + # Ferrum's default Chrome flags that switch off Chromium's same-origin + # policy (+disable-web-security+) and site isolation + # (+disable-site-isolation-trials+). {.chromium_flags} drops both. + SECURITY_OFF_FLAGS = %w[disable-web-security disable-site-isolation-trials].freeze + + # Site-isolation features on Ferrum's default +--disable-features+ list. + # {.chromium_flags} keeps them enabled. + SITE_ISOLATION_FEATURES = %w[site-per-process IsolateOrigins].freeze + # Build the option hash passed to +Ferrum::Browser.new+. Extracted as # a separate method so tests can verify the dimensions without # launching the browser. # - # The +--no-sandbox+ flag is required to launch Chromium as a non-root - # user inside an Alpine container without the kernel SETUID helper. - # This is only safe because the capture target is always the - # operator's own dashboard ({.serve} fetches +base_url+ from the - # +/og-image.png+ route, which derives it from the running app's - # public URL). DO NOT extend this code path to capture untrusted URLs - # — the disabled sandbox would turn a renderer-process exploit into a - # container escape. - # - # +--disable-dev-shm-usage+ avoids /dev/shm OOMs in small containers - # and +--disable-gpu+ prevents WebGL probing on machines without a - # GPU. Both are routine for headless Chromium captures. + # Ferrum's own default flags are switched off + # (+ignore_default_browser_options+): Ferrum 0.18 can add a flag to its + # defaults but not remove one, so {.chromium_flags} passes the defaults + # itself, minus the ones that disable Chromium's security model. Ferrum's + # +FERRUM_CHROME_DOCKERIZE+ therefore no longer applies; the sandbox + # follows +OG_IMAGE_NO_SANDBOX+ alone. # # @return [Hash] keyword options for Ferrum::Browser. def browser_options @@ -214,17 +217,48 @@ module PotatoMesh ], timeout: PotatoMesh::Config.og_image_navigation_timeout, process_timeout: PotatoMesh::Config.og_image_navigation_timeout, - browser_options: { - "no-sandbox": nil, - "disable-dev-shm-usage": nil, - "disable-gpu": nil, - }, + ignore_default_browser_options: true, + browser_options: chromium_flags, } browser_path = ENV["FERRUM_BROWSER_PATH"] options[:browser_path] = browser_path if browser_path && !browser_path.empty? options end + # Command-line flags for the capture's Chromium: Ferrum's default Chrome + # flags without {SECURITY_OFF_FLAGS} and without the + # {SITE_ISOLATION_FEATURES} entries of +--disable-features+. + # + # The capture renders the dashboard, which loads third-party map tiles + # and shows mesh-supplied text, so Chromium keeps its sandbox, its + # same-origin policy and its site isolation. The capture needs none of + # them off: it loads one page and takes a screenshot. + # + # +--no-sandbox+ is added only when +OG_IMAGE_NO_SANDBOX=1+ + # ({PotatoMesh::Config.og_image_no_sandbox?}), for hosts where Chromium + # cannot start its sandbox; the Docker images set it. Without the + # sandbox a renderer exploit runs with the web app's own privileges, so + # DO NOT extend this code path to capture untrusted URLs: {.serve} + # receives +base_url+ from the +/og-image.png+ route, which derives it + # from the running app's public URL. + # + # +--disable-dev-shm-usage+ avoids /dev/shm OOMs in small containers + # and +--disable-gpu+ prevents WebGL probing on machines without a + # GPU. Both are routine for headless Chromium captures. + # + # @return [Hash{String=>String, nil}] flag names without the leading + # +--+, each mapped to its value or to +nil+ for a bare switch. + def chromium_flags + require "ferrum" + flags = Ferrum::Browser::Options::Chrome.options.except(*SECURITY_OFF_FLAGS) + features = flags.fetch("disable-features", "").split(",") - SITE_ISOLATION_FEATURES + flags["disable-features"] = features.join(",") + flags["disable-dev-shm-usage"] = nil + flags["disable-gpu"] = nil + flags["no-sandbox"] = nil if PotatoMesh::Config.og_image_no_sandbox? + flags + end + # Wait for the dashboard to reach a stable state before capturing. # Network-idle timeouts are tolerated because some dashboard widgets # may continue polling indefinitely. diff --git a/web/public/assets/js/app/__tests__/chat-format.test.js b/web/public/assets/js/app/__tests__/chat-format.test.js index 2c013f88..1aec8814 100644 --- a/web/public/assets/js/app/__tests__/chat-format.test.js +++ b/web/public/assets/js/app/__tests__/chat-format.test.js @@ -187,6 +187,15 @@ test('abbreviatePreset maps known presets to codes', () => { assert.equal(abbreviatePreset('X'), 'X?'); }); +test('abbreviatePreset ignores inherited Object.prototype keys', () => { + // `constructor` is a member of every plain object, so the bare lookup in the + // preset table returned the Object constructor instead of falling through + // to the derived initials, and the chat tag read `[FU]` ("function"). + assert.equal(abbreviatePreset('constructor'), 'CO'); + assert.equal(abbreviatePreset('Constructor'), 'CO'); + assert.equal(formatChatPresetTag(extractChatMessageMetadata({ modem_preset: 'constructor' })), '[CO]'); +}); + test('derivePresetInitials falls back to segmented tokens', () => { assert.equal(derivePresetInitials('Long Moderate'), 'LM'); assert.equal(derivePresetInitials('ShortTurbo'), 'ST'); diff --git a/web/public/assets/js/app/__tests__/federation-page.test.js b/web/public/assets/js/app/__tests__/federation-page.test.js index e6df628e..89209da6 100644 --- a/web/public/assets/js/app/__tests__/federation-page.test.js +++ b/web/public/assets/js/app/__tests__/federation-page.test.js @@ -793,6 +793,30 @@ test('federation page suppresses spammy site names and truncates long names in v } }); +test('federation cells and popups render a numeric frequency and version', async () => { + // The page escaped through a private copy that returned '' for a non-string, + // so a number left the cell and the popup line empty. The shared escapeHtml + // stringifies first. + const { cleanup, tbodyEl } = createBasicFederationPageHarness(); + const markerPopups = []; + const instance = { + domain: 'num.mesh', name: 'Numeric Mesh', frequency: 868, version: 7, + latitude: 1, longitude: 1, lastUpdateTime: Math.floor(Date.now() / 1000) - 30, + }; + const fetchImpl = async () => ({ ok: true, json: async () => [instance] }); + + try { + await initializeFederationPage({ config: {}, fetchImpl, leaflet: createBasicLeafletStub({ markerPopups }) }); + + const rowHtml = tbodyEl.childNodes[0].innerHTML; + assert.match(rowHtml, /instances-col--frequency">8687Version: 7/); + } finally { + cleanup(); + } +}); + test('federation page sorts by full site names before truncating visible labels', async () => { const env = createDomEnvironment({ includeBody: true, bodyHasDarkClass: false }); const { document, createElement, registerElement, cleanup } = env; diff --git a/web/public/assets/js/app/__tests__/live-dom-model.js b/web/public/assets/js/app/__tests__/live-dom-model.js index 5364da3a..0480673b 100644 --- a/web/public/assets/js/app/__tests__/live-dom-model.js +++ b/web/public/assets/js/app/__tests__/live-dom-model.js @@ -36,7 +36,9 @@ * * Only the selector forms the dashboard and these tests use are supported: * `tag`, `#id`, `.class`, `[attr]`, `[attr="value"]`, compounds of those, the - * descendant combinator and comma lists. + * descendant combinator and comma lists. A quoted value may carry the CSS + * escapes `cssEscape` emits without `CSS.escape` (see {@link decodeCssEscapes}); + * hex escapes, and whitespace or a comma inside a value, are not supported. * * @module app/__tests__/live-dom-model */ @@ -94,7 +96,24 @@ function datasetAttribute(prop) { } /** - * Parse one compound selector (`tr.nodes-subrow[data-x="y"]`). + * Decode the CSS escapes of one quoted selector value, as a browser reads it: + * a backslash before a character that is neither a hex digit nor whitespace + * stands for that character (`\"` is `"`, `\!` is `!`). That is the only form + * `cssEscape` (`main/format-utils.js`) emits without `CSS.escape`; the + * selector grammar in {@link parseCompound} admits no other. + * + * @param {string} value Quoted value without its quotes. + * @returns {string} The value the selector compares attributes against. + */ +function decodeCssEscapes(value) { + return value.replace(/\\([\s\S])/g, '$1'); +} + +/** + * Parse one compound selector (`tr.nodes-subrow[data-x="y"]`). A quoted + * attribute value is compared with its CSS escapes decoded + * ({@link decodeCssEscapes}); a raw `"` inside it ends the value, so the rest + * of the selector fails to parse, as in a browser. * * @param {string} source Compound selector text. * @returns {{tag: ?string, id: ?string, classes: Array, attrs: Array<{name: string, value: ?string}>}} @@ -109,13 +128,15 @@ function parseCompound(source) { parts.tag = tag[1] === '*' ? null : tag[1].toUpperCase(); rest = rest.slice(tag[0].length); } - const simple = /^(?:\.([a-zA-Z0-9_-]+)|#([a-zA-Z0-9_-]+)|\[([a-zA-Z0-9_-]+)(?:="([^"]*)")?\])/; + // A quoted value runs to the first unescaped `"`; each backslash escapes + // the next character, which must not be a hex digit or whitespace. + const simple = /^(?:\.([a-zA-Z0-9_-]+)|#([a-zA-Z0-9_-]+)|\[([a-zA-Z0-9_-]+)(?:="((?:[^"\\]|\\[^0-9a-fA-F\s])*)")?\])/; while (rest.length) { const match = simple.exec(rest); if (!match) throw new Error(`live-dom-model: unsupported selector "${source}"`); if (match[1]) parts.classes.push(match[1]); else if (match[2]) parts.id = match[2]; - else parts.attrs.push({ name: match[3], value: match[4] === undefined ? null : match[4] }); + else parts.attrs.push({ name: match[3], value: match[4] === undefined ? null : decodeCssEscapes(match[4]) }); rest = rest.slice(match[0].length); } return parts; diff --git a/web/public/assets/js/app/__tests__/role-helpers.test.js b/web/public/assets/js/app/__tests__/role-helpers.test.js index bf05331a..8dd5d419 100644 --- a/web/public/assets/js/app/__tests__/role-helpers.test.js +++ b/web/public/assets/js/app/__tests__/role-helpers.test.js @@ -209,6 +209,21 @@ test('an unknown role falls back within its own protocol palette (SPEC RA9)', () assert.equal(getRoleColor('UNKNOWN_ROLE', 'meshtastic'), roleColors.CLIENT); }); +test('a role named after an Object.prototype member takes its protocol base colour (SPEC RA9)', () => { + // The palettes are plain objects, so a bare `palette[role]` lookup found + // inherited members and returned a function or an object where a colour + // belongs. No palette defines these roles, so each takes its protocol's + // base colour like any other unknown role. + assert.equal(getRoleColor('__proto__'), roleColors.CLIENT); + assert.equal(getRoleColor('constructor'), roleColors.CLIENT); + assert.equal(getRoleColor('hasOwnProperty', 'meshtastic'), roleColors.CLIENT); + assert.equal(getRoleColor('toString', 'meshcore'), meshcoreRoleColors.COMPANION); + assert.equal(getRoleColor('constructor', 'reticulum'), reticulumRoleColors.PEER); + // The badge text and the live-update fade follow the base colour. + assert.equal(getRoleTextColor('constructor'), getRoleTextColor('CLIENT')); + assert.equal(getRoleFlashColor('constructor'), 'rgba(243, 239, 116, 0.55)'); +}); + test('defaultRoleFor names each protocol base role (SPEC RA9)', () => { assert.equal(defaultRoleFor('meshtastic'), 'CLIENT'); assert.equal(defaultRoleFor('meshcore'), 'COMPANION'); diff --git a/web/public/assets/js/app/chat-format.js b/web/public/assets/js/app/chat-format.js index 650861fe..71e83fcf 100644 --- a/web/public/assets/js/app/chat-format.js +++ b/web/public/assets/js/app/chat-format.js @@ -301,7 +301,8 @@ function abbreviatePreset(preset, freqMHz = null) { return resolved.shortCode; } const token = preset.replace(/[^A-Za-z]/g, '').toLowerCase(); - if (token && PRESET_ABBREVIATIONS[token]) { + // Own keys only: `constructor` is inherited by every plain object. + if (token && Object.hasOwn(PRESET_ABBREVIATIONS, token)) { return PRESET_ABBREVIATIONS[token]; } return derivePresetInitials(preset); diff --git a/web/public/assets/js/app/federation-page.js b/web/public/assets/js/app/federation-page.js index 39cd3ebd..b652c191 100644 --- a/web/public/assets/js/app/federation-page.js +++ b/web/public/assets/js/app/federation-page.js @@ -25,6 +25,7 @@ import { mergeConfig } from './settings.js'; import { roleColors } from './role-helpers.js'; import { meshcoreIconHtml, meshtasticIconHtml, reticulumIconHtml } from './protocol-helpers.js'; import { createBasemapLayer } from './basemap-config.js'; +import { escapeHtml } from './utils.js'; import { timeAgoSuffixed } from './main/format-utils.js'; import { startRelativeTimeTicker, @@ -74,22 +75,6 @@ export function getFederationRelativeTimeTicker() { return relativeTimeTicker; } -/** - * Escape HTML special characters to prevent XSS. - * - * @param {string} str Raw string to escape. - * @returns {string} Escaped string safe for HTML insertion. - */ -function escapeHtml(str) { - if (typeof str !== 'string') return ''; - return str - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, '''); -} - /** * Format a coordinate value to fixed decimal places. * diff --git a/web/public/assets/js/app/main/__tests__/flash.test.js b/web/public/assets/js/app/main/__tests__/flash.test.js index 54347e59..0a03371c 100644 --- a/web/public/assets/js/app/main/__tests__/flash.test.js +++ b/web/public/assets/js/app/main/__tests__/flash.test.js @@ -29,6 +29,8 @@ import { emitMarkerWave, emitNodeWaves, } from '../flash.js'; +import { cssEscape } from '../format-utils.js'; +import { createLiveDocument } from '../../__tests__/live-dom-model.js'; /** A minimal element exposing a tracked classList. */ function fakeElement() { @@ -181,7 +183,8 @@ test('flashNodeTargets flashes each node row and marker', () => { markerFlashed = true; }, }; - const documentRef = { querySelectorAll: (sel) => (sel.includes('"!a"') ? [rowA] : []) }; + const queried = []; + const documentRef = { querySelectorAll: (sel) => { queried.push(sel); return [rowA]; } }; const markerByNodeId = new Map([['!a', markerA]]); const count = flashNodeTargets(['!a'], { @@ -190,6 +193,8 @@ test('flashNodeTargets flashes each node row and marker', () => { flashOptions: { schedule: () => {} }, }); + // The row selector carries the id CSS-escaped inside its quoted value. + assert.deepEqual(queried, [`[data-node-row="${cssEscape('!a')}"]`]); assert.equal(rowA.classList.contains(FLASH_CLASS), true, 'row flashed'); assert.equal(markerFlashed, true, 'marker flashed'); assert.equal(count, 2); // one row + one marker @@ -202,6 +207,42 @@ test('flashNodeTargets skips a missing document and unmapped markers', () => { assert.equal(flashNodeTargets(['!x'], { documentRef: { querySelectorAll: () => [] } }), 0); }); +test('an id containing a double quote flashes its row, message and tab without throwing', () => { + // A node id is whatever key an ingestor posted. Interpolated raw, its `"` + // closed the quoted selector value early, querySelectorAll threw a + // SyntaxError, and the refresh skipped every flash after it. The live DOM + // model parses selectors as a browser does and rejects the raw form too. + const { body, model } = createLiveDocument(); + const element = (tag, attribute, value) => { + const el = model.createElement(tag); + el.setAttribute(attribute, value); + body.appendChild(el); + return el; + }; + const row = element('tr', 'data-node-row', '!a"b'); + const prefixRow = element('tr', 'data-node-row', '!a'); + const message = element('div', 'data-message-id', '7"8'); + const tab = element('button', 'data-tab-id', 'tab"x'); + const flashOptions = { schedule: () => 0 }; + + assert.equal(flashNodeTargets(['!a"b'], { documentRef: body, flashOptions }), 1); + const messageTabId = new Map([['7"8', 'tab"x']]); + assert.equal(flashMessageTargets(['7"8'], { documentRef: body, messageTabId, flashOptions }), 2); + assert.equal(row.classList.contains(FLASH_CLASS), true, 'the node row flashed'); + assert.equal(message.classList.contains(FLASH_CLASS), true, 'the message row flashed'); + assert.equal(tab.classList.contains(FLASH_CLASS), true, 'the tab header flashed'); + assert.equal(prefixRow.classList.contains(FLASH_CLASS), false, 'only the exact id flashes'); +}); + +test('a numeric message id still flashes its row (stringified before escaping)', () => { + const { body, model } = createLiveDocument(); + const row = model.createElement('div'); + row.setAttribute('data-message-id', '42'); + body.appendChild(row); + assert.equal(flashMessageTargets([42], { documentRef: body, flashOptions: { schedule: () => 0 } }), 1); + assert.equal(row.classList.contains(FLASH_CLASS), true); +}); + test('flashNodeTargets returns 0 for nullish or non-iterable ids', () => { assert.equal(flashNodeTargets(null), 0); assert.equal(flashNodeTargets(123), 0); diff --git a/web/public/assets/js/app/main/flash.js b/web/public/assets/js/app/main/flash.js index 50411fea..38e4b168 100644 --- a/web/public/assets/js/app/main/flash.js +++ b/web/public/assets/js/app/main/flash.js @@ -28,6 +28,8 @@ * @module main/flash */ +import { cssEscape } from './format-utils.js'; + /** CSS class that drives the one-shot highlight animation. */ export const FLASH_CLASS = 'live-flash'; @@ -211,6 +213,10 @@ export function emitNodeWaves(nodeIds, options = {}) { * document or Leaflet. Each is optional — a caller on a page without the node * table (or without a map) simply passes one of them. * + * The id is CSS-escaped inside the selector's quoted value: node ids come from + * ingest, a raw `"` makes `querySelectorAll` throw, and a raw `\` changes + * what the selector matches. + * * @param {?Iterable} nodeIds Canonical node ids to flash. * @param {Object} [options] Lookups + flash overrides. * @param {?{querySelectorAll: Function}} [options.documentRef] Document to query rows in. @@ -224,7 +230,8 @@ export function flashNodeTargets(nodeIds, options = {}) { let count = 0; for (const id of nodeIds) { if (documentRef && typeof documentRef.querySelectorAll === 'function') { - count += flashElements(documentRef.querySelectorAll(`[data-node-row="${id}"]`), flashOptions); + // String() first: cssEscape returns '' for anything but a string. + count += flashElements(documentRef.querySelectorAll(`[data-node-row="${cssEscape(String(id))}"]`), flashOptions); } const marker = markerByNodeId && typeof markerByNodeId.get === 'function' ? markerByNodeId.get(id) @@ -238,7 +245,8 @@ export function flashNodeTargets(nodeIds, options = {}) { * Flash every UI target for a set of changed messages: each message's chat * row(s) (`[data-message-id=""]`, present in the Log tab and the channel * tab) and the header of each affected channel tab (`[data-tab-id=""]`) - * once (SPEC VF3). + * once (SPEC VF3). Both ids are CSS-escaped inside the selectors' quoted + * values, as in {@link flashNodeTargets}. * * @param {?Iterable} messageIds Message ids that changed. * @param {Object} [options] Lookups + flash overrides. @@ -255,7 +263,7 @@ export function flashMessageTargets(messageIds, options = {}) { const tabIds = new Set(); for (const id of messageIds) { if (canQuery) { - count += flashElements(documentRef.querySelectorAll(`[data-message-id="${id}"]`), flashOptions); + count += flashElements(documentRef.querySelectorAll(`[data-message-id="${cssEscape(String(id))}"]`), flashOptions); } const tabId = messageTabId && typeof messageTabId.get === 'function' ? messageTabId.get(id) : null; if (tabId) tabIds.add(tabId); @@ -263,7 +271,7 @@ export function flashMessageTargets(messageIds, options = {}) { // Flash each affected channel tab header exactly once. if (canQuery) { for (const tabId of tabIds) { - count += flashElements(documentRef.querySelectorAll(`[data-tab-id="${tabId}"]`), flashOptions); + count += flashElements(documentRef.querySelectorAll(`[data-tab-id="${cssEscape(String(tabId))}"]`), flashOptions); } } return count; diff --git a/web/public/assets/js/app/role-helpers.js b/web/public/assets/js/app/role-helpers.js index 49e42b8c..0146a092 100644 --- a/web/public/assets/js/app/role-helpers.js +++ b/web/public/assets/js/app/role-helpers.js @@ -321,14 +321,19 @@ export function normalizeRole(role, protocol = null) { /** * Resolve the canonical role key used for colour lookup tables. * + * Only the palette's own keys count: a role named after an `Object.prototype` + * member (`constructor`, `toString`) is not a palette key (SPEC RA9). + * * @param {*} role Raw role value from the API. + * @param {string|null|undefined} [protocol] Protocol whose base role applies + * when the value is absent. * @returns {string} Canonical role identifier. */ export function getRoleKey(role, protocol = null) { const normalized = normalizeRole(role, protocol); - if (roleColors[normalized]) return normalized; + if (Object.hasOwn(roleColors, normalized)) return normalized; const upper = normalized.toUpperCase(); - if (roleColors[upper]) return upper; + if (Object.hasOwn(roleColors, upper)) return upper; return normalized; } @@ -347,10 +352,13 @@ export function getRoleKey(role, protocol = null) { export function getRoleColor(role, protocol = null) { const colors = getRoleColors(protocol); const key = getRoleKey(role, protocol); + // Only the palette's own keys are colours: a bare `colors[key]` also finds + // inherited `Object.prototype` members (a function for `constructor`). + const own = Object.hasOwn(colors, key) ? colors[key] : null; // Fall back within the node's own palette before the Meshtastic one, so a // role-less Meshcore or Reticulum node takes its protocol's base colour // rather than Meshtastic blue. - return colors[key] || colors[defaultRoleFor(protocol)] || roleColors.CLIENT || '#3388ff'; + return own || colors[defaultRoleFor(protocol)] || roleColors.CLIENT || '#3388ff'; } /** diff --git a/web/spec/config_spec.rb b/web/spec/config_spec.rb index c11a4e88..6a103cee 100644 --- a/web/spec/config_spec.rb +++ b/web/spec/config_spec.rb @@ -942,6 +942,26 @@ RSpec.describe PotatoMesh::Config do end end + describe ".og_image_no_sandbox?" do + it "returns false when OG_IMAGE_NO_SANDBOX is unset" do + within_env("OG_IMAGE_NO_SANDBOX" => nil) do + expect(described_class.og_image_no_sandbox?).to be(false) + end + end + + it "returns false when OG_IMAGE_NO_SANDBOX=0" do + within_env("OG_IMAGE_NO_SANDBOX" => "0") do + expect(described_class.og_image_no_sandbox?).to be(false) + end + end + + it "returns true when OG_IMAGE_NO_SANDBOX=1, ignoring surrounding whitespace" do + within_env("OG_IMAGE_NO_SANDBOX" => " 1 ") do + expect(described_class.og_image_no_sandbox?).to be(true) + end + end + end + def within_env(values) original = {} values.each do |key, value| diff --git a/web/spec/og_image_spec.rb b/web/spec/og_image_spec.rb index 4a08c1ca..3adf08d5 100644 --- a/web/spec/og_image_spec.rb +++ b/web/spec/og_image_spec.rb @@ -207,6 +207,18 @@ RSpec.describe PotatoMesh::OgImage do end describe ".browser_options" do + # The command-line flags Chromium would receive for + # +described_class.browser_options+, composed by Ferrum's own command + # builder. No browser starts: the builder only needs a browser path, and + # this one does not exist. + # + # @return [Array] Chromium flags, without the binary path. + def launched_flags + require "ferrum" + options = described_class.browser_options.merge(browser_path: "/nonexistent/chromium") + Ferrum::Browser::Command.build(Ferrum::Browser::Options.new(options), SPEC_TMPDIR).to_a.drop(1) + end + it "honors the configured viewport dimensions" do options = described_class.browser_options @@ -217,20 +229,44 @@ RSpec.describe PotatoMesh::OgImage do expect(options[:headless]).to be true end - # `--no-sandbox` is required for non-root Alpine containers; removing - # it would silently break Chromium launches in production. The - # corresponding assertion lives in security review (see comment in - # OgImage.browser_options). - it "passes the --no-sandbox flag" do - options = described_class.browser_options - - expect(options[:browser_options]).to have_key(:"no-sandbox") + # Chromium keeps its sandbox unless the operator opts out; the Docker + # images opt out with OG_IMAGE_NO_SANDBOX=1 (see OgImage.chromium_flags). + it "omits --no-sandbox while OG_IMAGE_NO_SANDBOX is unset" do + original = ENV["OG_IMAGE_NO_SANDBOX"] + ENV.delete("OG_IMAGE_NO_SANDBOX") + begin + expect(launched_flags).not_to include("--no-sandbox") + ensure + ENV["OG_IMAGE_NO_SANDBOX"] = original if original + end end - it "passes the --disable-dev-shm-usage flag" do - options = described_class.browser_options + it "passes --no-sandbox when OG_IMAGE_NO_SANDBOX=1" do + original = ENV["OG_IMAGE_NO_SANDBOX"] + ENV["OG_IMAGE_NO_SANDBOX"] = "1" + begin + expect(launched_flags).to include("--no-sandbox") + ensure + if original + ENV["OG_IMAGE_NO_SANDBOX"] = original + else + ENV.delete("OG_IMAGE_NO_SANDBOX") + end + end + end - expect(options[:browser_options]).to have_key(:"disable-dev-shm-usage") + it "keeps the same-origin policy and site isolation on, and Ferrum's other defaults" do + flags = launched_flags + disabled_features = flags.find { |flag| flag.start_with?("--disable-features=") }.split("=", 2).last.split(",") + + expect(flags).not_to include("--disable-web-security", "--disable-site-isolation-trials") + expect(disabled_features).not_to include("site-per-process", "IsolateOrigins") + expect(disabled_features).to include("Translate", "MediaRouter") + expect(flags).to include("--headless", "--disable-background-networking", "--remote-allow-origins=*") + end + + it "passes the --disable-dev-shm-usage and --disable-gpu flags" do + expect(launched_flags).to include("--disable-dev-shm-usage", "--disable-gpu") end it "passes the FERRUM_BROWSER_PATH env when present" do