web: sandbox the OG capture and harden frontend escaping and lookups (#994)

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 \

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

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.

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');

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">868</);
+    assert.match(rowHtml, /instances-col--version mono">7</);
+    assert.match(markerPopups[0], /Frequency: 868<br>Version: 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;

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<string>, 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;

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');

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);

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, '&lt;')
-    .replace(/>/g, '&gt;')
-    .replace(/"/g, '&quot;')
-    .replace(/'/g, '&#039;');
-}
-
 /**
  * Format a coordinate value to fixed decimal places.
  *

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);

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<string>} 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="<id>"]`, present in the Log tab and the channel
  * tab) and the header of each affected channel tab (`[data-tab-id="<id>"]`)
- * once (SPEC VF3).
+ * once (SPEC VF3). Both ids are CSS-escaped inside the selectors' quoted
+ * values, as in {@link flashNodeTargets}.
  *
  * @param {?Iterable<string>} 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;

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';
 }
 
 /**

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|

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<String>] 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