Parse MAX_FILE_SIZE, and document what else caps an upload

Follow-up to #233, which made the upload ceiling configurable — the right call,
500MB is genuinely too low for video.

An environment variable is a string, so the value reached multer's
limits.fileSize as text where a number is expected. That survives some
comparisons through coercion and misbehaves in others, which is the worst kind
of bug to find later; the line directly above it already used parseInt for the
same reason. It is parsed properly now, and a suffix is accepted — someone
raising a limit for video is choosing "about 2GB", and 2147483648 is easy to
mistype by a factor of ten.

An unparseable value falls back to the default rather than becoming NaN or
zero. Either would reject every upload on the instance, from a typo in an env
file, with nothing on screen to explain it.

The documentation matters as much as the code here. MAX_FILE_SIZE is the LAST
limit in the chain: nginx caps the request body with client_max_body_size and
returns 413 before the app is reached — our own deployment sets 500M — and
Cloudflare caps uploads per plan at the edge. Raising the variable alone often
changes nothing, so the README now says so, with the nginx directive and a note
that an upload failing with nothing in the server log never reached the server.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL

README.md

@@ -136,8 +136,29 @@ Schema migrations run automatically on first boot — no manual migration comman
 | `PING_TIMEOUT` | Socket.IO Engine.IO pong wait (ms). Lower = faster dead-socket detection; higher = more forgiving of laggy clients. | `30000` |
 | `HEARTBEAT_INTERVAL` | App-level offline-checker frequency (ms). How often the server sweeps the device list looking for stale heartbeats. | `10000` |
 | `HEARTBEAT_TIMEOUT` | How long without an app-level heartbeat (ms) before marking a device offline. Raise for slow/jittery networks. | `45000` |
+| `MAX_FILE_SIZE` | Largest upload the server will accept. Bytes, or a suffix (`2GB`, `1500MB`). **A reverse proxy caps this independently** — see below. | `500MB` |
 | `COMMAND_QUEUE_TTL_MS` | How long the server holds commands and playlist-updates for a device that's offline at emit time (ms). Flushed in order on reconnect within this window; dropped past TTL. | `30000` |
 
+#### Raising the upload limit
+
+`MAX_FILE_SIZE` sets what **the application** accepts. It is usually not the only limit, and it
+is the last one in the chain — so raising it on its own often changes nothing and the upload
+still fails with a `413`:
+
+- **nginx** (or any reverse proxy) caps the request body with `client_max_body_size`. The
+  default is 1MB, and a typical signage deployment sets 500M. Raise it to match:
+
+  ```nginx
+  client_max_body_size 2048M;   # must be >= MAX_FILE_SIZE
+  ```
+
+- **Cloudflare** caps uploads per plan (100MB on Free/Pro at the time of writing) and returns
+  413 at the edge, before your server is involved. Large uploads need a plan that allows them,
+  or a route that bypasses the proxy.
+
+If an upload fails and nothing appears in the server log, the request never reached the app —
+check the proxy first.
+
 ### Optional Integrations
 
 All integrations are optional. The app works fully without any of them.

server/config.js

@@ -20,6 +20,8 @@ function parseBillingRateTable(raw) {
   return null;
 }
 
+const { parseSize } = require('./lib/parse-size');
+
 module.exports = {
   port: process.env.PORT || 3001,
   httpsPort: process.env.HTTPS_PORT || 3443,
@@ -59,7 +61,13 @@ module.exports = {
   // #148 Item 4: TCP SO_KEEPALIVE idle delay — OS-level dead-peer probing independent of the
   // app ping, so a half-open TCP can't persist indefinitely.
   tcpKeepAliveMs: parseInt(process.env.TCP_KEEPALIVE_MS) || 20000,
-  maxFileSize:  process.env.MAX_FILE_SIZE || 500 * 1024 * 1024, // 500MB
+  // Upload ceiling, #233. Accepts bytes or a suffix (MAX_FILE_SIZE=2GB). An env var is a
+  // string, so this must be parsed rather than used directly — multer's limits.fileSize wants a
+  // number, and an unparseable value falls back to the default rather than becoming NaN, which
+  // would reject every upload. NOTE: a reverse proxy caps the request body independently
+  // (nginx client_max_body_size, and any CDN in front) and returns 413 before the app is
+  // reached, so raising this alone is not enough — see the README.
+  maxFileSize: parseSize(process.env.MAX_FILE_SIZE, 500 * 1024 * 1024), // default 500MB
   thumbnailWidth: 320,
   screenshotQuality: 70,
   // SSL: drop your Cloudflare Origin cert + key in certs/ folder

server/lib/parse-size.js

@@ -0,0 +1,37 @@
+'use strict';
+
+// Parse a human-written size into bytes.
+//
+// Environment variables are always STRINGS. `process.env.MAX_FILE_SIZE || default` therefore
+// yields the string "2000000000", which then travels into multer's limits.fileSize where a
+// number is expected — it happens to survive some comparisons through coercion and misbehaves
+// in others, which is the worst kind of bug to chase.
+//
+// A plain byte count is also unfriendly for this particular setting: the person raising an
+// upload limit for video is choosing "about 2GB", and 2147483648 is easy to mistype by a
+// factor of ten. So a suffix is accepted as well.
+//
+// Deliberately strict: anything unparseable returns the fallback rather than 0 or NaN. A typo
+// in an env var must not silently become "reject every upload", which is what a NaN limit or a
+// zero would do.
+
+const UNITS = { b: 1, kb: 1024, mb: 1024 ** 2, gb: 1024 ** 3, tb: 1024 ** 4 };
+
+function parseSize(value, fallback) {
+  if (value == null || value === '') return fallback;
+  if (typeof value === 'number') return Number.isFinite(value) && value > 0 ? Math.floor(value) : fallback;
+
+  const raw = String(value).trim().toLowerCase().replace(/\s+/g, '');
+  // Bare digits are bytes, which is what the variable meant before suffixes were understood.
+  if (/^\d+$/.test(raw)) {
+    const n = Number(raw);
+    return n > 0 ? n : fallback;
+  }
+  const m = raw.match(/^(\d+(?:\.\d+)?)(b|kb|mb|gb|tb)$/);
+  if (!m) return fallback;
+  const n = parseFloat(m[1]);
+  if (!Number.isFinite(n) || n <= 0) return fallback;
+  return Math.floor(n * UNITS[m[2]]);
+}
+
+module.exports = { parseSize };

server/test/max-file-size-config.test.js

@@ -0,0 +1,76 @@
+'use strict';
+
+// #233: the upload ceiling was hard-coded at 500MB, which is too low for video. Making it an
+// env var is right, but env vars are STRINGS — `process.env.MAX_FILE_SIZE || default` hands
+// multer's limits.fileSize a string where it wants a number. That survives some comparisons by
+// coercion and misbehaves in others, which is the worst kind of bug to track down later.
+//
+// The other half is that a bad value must not become NaN or 0. Either would reject every upload
+// on the instance, from a typo in an env file, with nothing on screen explaining why. Falling
+// back to the documented default is the only safe reading of an unparseable limit.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const { parseSize } = require('../lib/parse-size');
+
+const MB = 1024 * 1024;
+const GB = 1024 * MB;
+const DEFAULT = 500 * MB;
+
+test('THE BUG: the result is a number, never the raw string', () => {
+  const v = parseSize('2000000000', DEFAULT);
+  assert.equal(typeof v, 'number', 'multer wants a number');
+  assert.equal(v, 2000000000);
+});
+
+test('a bare number is bytes, which is what the variable meant before suffixes', () => {
+  assert.equal(parseSize('1500000000', DEFAULT), 1500000000);
+  assert.equal(parseSize(1500000000, DEFAULT), 1500000000);
+});
+
+test('a suffix is understood, because nobody should have to type 2147483648', () => {
+  assert.equal(parseSize('2GB', DEFAULT), 2 * GB);
+  assert.equal(parseSize('2gb', DEFAULT), 2 * GB);
+  assert.equal(parseSize('1500MB', DEFAULT), 1500 * MB);
+  assert.equal(parseSize('750kb', DEFAULT), 750 * 1024);
+  assert.equal(parseSize('900b', DEFAULT), 900);
+});
+
+test('spacing and case do not matter', () => {
+  assert.equal(parseSize(' 2 GB ', DEFAULT), 2 * GB);
+  assert.equal(parseSize('2Gb', DEFAULT), 2 * GB);
+});
+
+test('a fractional size is allowed and floored to whole bytes', () => {
+  assert.equal(parseSize('1.5GB', DEFAULT), Math.floor(1.5 * GB));
+  assert.equal(Number.isInteger(parseSize('1.5GB', DEFAULT)), true);
+});
+
+test('THE SAFETY RULE: an unparseable value falls back, it does not become NaN or 0', () => {
+  // A NaN or zero limit rejects every upload on the instance. A typo in an env file must not
+  // do that silently.
+  for (const bad of ['nonsense', '2 gigabytes', 'GB', '-5MB', '0', '0GB', '', '   ', null, undefined, NaN, {}]) {
+    const v = parseSize(bad, DEFAULT);
+    assert.equal(v, DEFAULT, `${JSON.stringify(bad)} should fall back`);
+    assert.ok(Number.isFinite(v) && v > 0);
+  }
+});
+
+test('the shipped default is unchanged at 500MB', () => {
+  // Raising the ceiling for everyone silently is not the point of this change.
+  delete process.env.MAX_FILE_SIZE;
+  delete require.cache[require.resolve('../config')];
+  const config = require('../config');
+  assert.equal(config.maxFileSize, DEFAULT);
+  assert.equal(typeof config.maxFileSize, 'number');
+});
+
+test('config honours the env var end to end', () => {
+  process.env.MAX_FILE_SIZE = '3GB';
+  delete require.cache[require.resolve('../config')];
+  const config = require('../config');
+  assert.equal(config.maxFileSize, 3 * GB);
+  assert.equal(typeof config.maxFileSize, 'number');
+  delete process.env.MAX_FILE_SIZE;
+  delete require.cache[require.resolve('../config')];
+});