mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-19 08:33:56 -06:00
9823aaf595
A partner reselling this platform reported that white-labelling changed the sidebar
title and the browser tab, and nothing else. Three fixes, in the order they matter to
them.
THE APK FILENAME, which they called the highest priority and which is a commercial
leak rather than a cosmetic one: every download landed on their customer's disk as
"ScreenTinker.apk", naming the upstream product — and where to buy it directly — to
the people they were selling to. /download/apk now resolves branding by DOMAIN, since
that route is unauthenticated and has no workspace to read, which is also exactly how
a reseller deploys: their own hostname, their own brand.
The name is sanitised through a whitelist, in lib/brand-filename.js so it can be
tested. That is security code, not cosmetics: brand_name is arbitrary operator text
landing in a Content-Disposition header, where a quote ends the filename parameter
early and a CR/LF ends the header line entirely. The tests are mostly hostile input.
ADMIN-CREATED USERS ARE VERIFIED. POST /api/admin/users left email_verified at the
schema default of 0, so every admin-provisioned user met a "Please confirm your email
address" banner they could not dismiss — and on an instance with no SMTP, could never
clear. Operators were fixing it by editing the database by hand. An address typed in
by an administrator is as verified as this system can make it. Note the test fixture
had drifted from the real schema and lacked the column entirely; adding it there is
what let the fix be tested at all.
THE HARDCODED STRINGS. Nine user-facing strings named the product — setup steps, the
empty-dashboard hint, onboarding, sign-in errors. They are translated strings, so the
substitution belongs in the translation layer: they now say {brandName}, and i18n.js
fills it in inside format(), so every t() call gets it without threading a variable
through several hundred call sites. Read at CALL time, not captured, so a workspace
switch shows the new brand rather than the one cached at module load. 43 strings across
7 locales; the default is the product's own name, so an un-branded install is unchanged.
Deliberately NOT changed, because substituting a brand there would be wrong rather than
incomplete:
- the White Label brand_name input's placeholder, which shows the default when empty;
- the install-statistics explanation, which describes what the upstream project can
and cannot see, and is not about the reseller's brand;
- the widget security warning, which describes the privileges of this software; that
is copy worth changing deliberately rather than by regex.
Full suite 1779 pass / 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014kfhrUPit5MCqxeTQyqr56
70 lines
3 KiB
JavaScript
70 lines
3 KiB
JavaScript
'use strict';
|
|
|
|
/*
|
|
* The brand name on its way into a Content-Disposition header (#292).
|
|
*
|
|
* Two things are being protected. The commercial one: a reseller's customer must not receive a file
|
|
* called ScreenTinker.apk. The security one, which matters more: brand_name is arbitrary
|
|
* operator-supplied text, and a quote or a newline in it would break out of the header — so the
|
|
* cases below are mostly hostile input, not brand names anyone would choose.
|
|
*/
|
|
|
|
const { test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const { brandToFilenameStem } = require('../lib/brand-filename');
|
|
|
|
test('an ordinary brand name comes through recognisably', () => {
|
|
assert.equal(brandToFilenameStem('BoldSignage'), 'BoldSignage');
|
|
assert.equal(brandToFilenameStem('Bold Signage'), 'BoldSignage');
|
|
assert.equal(brandToFilenameStem('Bold-Media_Group.v2'), 'Bold-Media_Group.v2');
|
|
});
|
|
|
|
test('nothing configured falls back to the product name', () => {
|
|
for (const empty of ['', ' ', null, undefined]) {
|
|
assert.equal(brandToFilenameStem(empty), 'ScreenTinker');
|
|
}
|
|
});
|
|
|
|
test('a name that sanitises away entirely still yields a usable filename', () => {
|
|
// Otherwise the download would be called ".apk", which some browsers refuse to save at all.
|
|
assert.equal(brandToFilenameStem('日本語'), 'ScreenTinker');
|
|
assert.equal(brandToFilenameStem('***'), 'ScreenTinker');
|
|
});
|
|
|
|
test('⚠️ a quote cannot break out of the header', () => {
|
|
// attachment; filename="<HERE>" — a quote would end the parameter and let the rest be parsed
|
|
// as further header syntax.
|
|
const out = brandToFilenameStem('Acme" ; filename="evil');
|
|
assert.ok(!out.includes('"'), `a quote survived: ${out}`);
|
|
assert.ok(!out.includes(';'), `a semicolon survived: ${out}`);
|
|
assert.ok(!out.includes(' '), `a space survived: ${out}`);
|
|
});
|
|
|
|
test('⚠️ CR/LF cannot inject another header', () => {
|
|
const out = brandToFilenameStem('Acme\r\nSet-Cookie: admin=1');
|
|
assert.ok(!/[\r\n]/.test(out), `a line break survived: ${out}`);
|
|
assert.equal(out, 'AcmeSet-Cookieadmin1');
|
|
});
|
|
|
|
test('⚠️ path separators cannot escape the filename', () => {
|
|
assert.ok(!brandToFilenameStem('../../etc/passwd').includes('/'));
|
|
assert.ok(!brandToFilenameStem('..\\..\\windows').includes('\\'));
|
|
// Leading dots would also make it a hidden file rather than a download.
|
|
assert.ok(!brandToFilenameStem('...Acme').startsWith('.'));
|
|
});
|
|
|
|
test('accents are transliterated rather than deleted', () => {
|
|
// "Café Media" losing its é is fine; losing the whole word is not.
|
|
assert.equal(brandToFilenameStem('Café Média'), 'CafeMedia');
|
|
});
|
|
|
|
test('an absurdly long name is bounded', () => {
|
|
const out = brandToFilenameStem('A'.repeat(500));
|
|
assert.equal(out.length, 64, 'filenames have limits on every filesystem worth naming');
|
|
});
|
|
|
|
test('a trailing dot is removed', () => {
|
|
// Windows rejects a trailing dot outright, and "Acme..apk" is what naive concatenation gives.
|
|
assert.equal(brandToFilenameStem('Acme.'), 'Acme');
|
|
});
|