Skip to content

Security: stop the Connections widget printing the address it failed to reach #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SandObserver merged 1 commit into from
Aug 10, 2026
Merged

Security: stop the Connections widget printing the address it failed to reach #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- The Books, Now Playing and Backup widgets now show why they failed, such as a
missing API key or a rejected password, instead of a generic error.

### Security

- A Connections tile that cannot reach a service now says so in plain words
instead of printing the address it tried, which put an internal host and port
on the dashboard.

## [1.5.2] - 2026-08-09

### Changed
Expand Down
17 changes: 15 additions & 2 deletions api/test/connections-url.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const assert = require('node:assert/strict');

const data = require(path.join(__dirname, '../../ui/widgets/connections/data.js'));
const { normBase } = data;
const { WidgetError, KIND } = require('../src/api-error');

/* ── the normaliser ───────────────────────────────────────────────────────── */

Expand Down Expand Up @@ -56,14 +57,26 @@ test('the string "undefined" never appears in the result', () => {

/* ── through the widget ───────────────────────────────────────────────────── */

/* fetchJSON is passed in, so a call can be observed without any network. */
/* fetchJSON is passed in, so a call can be observed without any network. The
rest of the context is the surface widget-data.js really hands a data
function: ctx.fail throws a WidgetError, which is what marks a message as one
the widget wrote rather than one that escaped from Node. */
function run(config, endpoint = 'map') {
const requested = [];
const fetchJSON = async url => {
requested.push(url);
return { status: 200, data: {} };
};
return data({ config, endpoint, fetchJSON }).then(result => ({ result, requested }));
const ctx = {
config,
endpoint,
fetchJSON,
KIND,
fail: (message, opts) => {
throw new WidgetError(message, opts);
},
};
return data(ctx).then(result => ({ result, requested }));
}

test('a service with no address is not fetched at all', async () => {
Expand Down
40 changes: 32 additions & 8 deletions api/test/widget-error.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,12 @@ test('every widget frontend shows the message the server vouched for', () => {
still throws, and the sanitiser is right to replace it. What is banned is a
sentence written for the user that never reaches them.

connections is exempt and has to be. It reports several services in one
result, catches each service's throw itself, and writes the message into that
service's own `error` field, which is an error inside a successful response
rather than a failed one. Its sentences do reach the user, by the other route
the convention allows. */
const THROWS_INTERNALLY = new Set(['connections']);

connections used to be exempt, because it catches its own throws and reports
each service inside a successful result. It uses ctx.fail too now, which is
what lets its catch tell a sentence it wrote from one Node threw. */
test('no widget reports a user-facing failure with a plain Error', () => {
const offenders = [];
for (const [name, p] of dataFiles) {
if (THROWS_INTERNALLY.has(name)) continue;
const src = fs.readFileSync(p, 'utf8').replace(/\/\*[\s\S]*?\*\//g, '');
for (const m of src.matchAll(/throw new Error\(([^)]*)\)/g)) {
offenders.push(`${name}: throw new Error(${m[1].slice(0, 50)})`);
Expand Down Expand Up @@ -227,3 +222,32 @@ test('no widget rebuilds the base-URL normalisation ctx already provides', () =>
}
assert.deepEqual(offenders, [], `Use ctx.normalizeBase:\n${offenders.join('\n')}`);
});

/* An error field inside a successful result is allowed, and is how a widget
reporting several services marks the one that failed. It bypasses the
api-error sanitiser entirely, though: the response is a 200, so nothing
rewrites what goes in it.

A caught error's message names what it failed to reach, so writing one into a
result puts an internal host and port on the dashboard. The field has to be
given a phrase the widget chose. */
test('no widget writes a caught message into a successful result', () => {
const offenders = [];
for (const [name, p] of dataFiles) {
const src = fs
.readFileSync(p, 'utf8')
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/ctx\.log\.\w+\([^;]*?\);/g, '');
for (const m of src.matchAll(/(\w+)\.error\s*=\s*([^;]+);/g)) {
const value = m[2];
if (/\be\.(message|stack)\b|String\(e\)/.test(value)) {
offenders.push(`${name}: ${m[0].trim().slice(0, 60)}`);
}
}
}
assert.deepEqual(
offenders,
[],
`A caught message reaches the browser in a 200 body. Map it to a phrase first:\n${offenders.join('\n')}`,
);
});
68 changes: 44 additions & 24 deletions ui/widgets/connections/data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,24 @@ const MAP_DEFAULT_COLOR = {
};

/* Raw GET: Conduit exposes Prometheus-style text that needs the unparsed body. */
/* What a failed service shows on the tile. This string travels in a successful
response, so the api-error sanitiser never sees it: a caught error's message
names the host and port it failed to reach. Our own sentence when we wrote
one, a phrase from the error code otherwise. */
const TIMEOUT_CODES = new Set(['ETIMEDOUT', 'ESOCKETTIMEDOUT', 'UND_ERR_HEADERS_TIMEOUT']);
function errorText(e) {
if (e && e.vouchedMessage) return e.vouchedMessage;
/* Already a sentence this repo wrote, and the reason is the point of it. */
if (e && e.name === 'SsrfBlockedError') return e.message;
const code = e && e.code;
if (code === 'ECONNREFUSED') return 'Connection refused';
if (code === 'ENOTFOUND' || code === 'EAI_AGAIN') return 'Host not found';
if (TIMEOUT_CODES.has(code)) return 'Timed out';
if (code === 'ECONNRESET' || code === 'EPIPE') return 'Connection lost';
if (typeof code === 'string' && (code.startsWith('CERT_') || code.includes('_CERT_'))) return 'Certificate rejected';
return 'Unreachable';
}

function authErr(r) {
return r.status === 401 || r.status === 403;
}
Expand Down Expand Up @@ -130,13 +148,14 @@ function parseConduitText(raw) {
return { regions, limit, connected, live };
}

const run = async function ({ config, endpoint, fetchJSON }) {
if (endpoint === 'vpn') return vpnView(config, fetchJSON);
return mapView(config, fetchJSON);
const run = async function (ctx) {
if (ctx.endpoint === 'vpn') return vpnView(ctx);
return mapView(ctx);
};

/* ── VPN view ── */
async function vpnView(config, fetchJSON) {
async function vpnView(ctx) {
const { config, fetchJSON } = ctx;
const vpn = config.vpn || {};
const svc = vpn.service || 'gluetun';
const out = {
Expand All @@ -151,13 +170,13 @@ async function vpnView(config, fetchJSON) {
try {
if (svc === 'gluetun') {
const base = normBase(vpn.url);
if (!base) throw new Error('No control server URL configured');
if (!base) ctx.fail('No control server URL configured', { kind: ctx.KIND.INVALID });
const headers = vpn.apiKey ? { 'X-API-Key': vpn.apiKey } : {};
let ipRes = null;
try {
ipRes = await fetchJSON(base + '/v1/publicip/ip', { headers, timeout: 7000 });
} catch (e) {
out.error = e.code || e.message || 'unreachable';
out.error = errorText(e);
}
if (ipRes) {
if (authErr(ipRes)) out.error = 'Auth required — set the API key';
Expand Down Expand Up @@ -192,12 +211,12 @@ async function vpnView(config, fetchJSON) {
}
} else {
const base = normBase(vpn.url).replace(/\/+$/, '');
if (!base) throw new Error('No management API URL configured');
if (!base) ctx.fail('No management API URL configured', { kind: ctx.KIND.INVALID });
const apiBase = /\/api$/.test(base) ? base : base + '/api';
const headers = { Authorization: `Token ${vpn.token || ''}`, Accept: 'application/json' };
const r = await fetchJSON(apiBase + '/peers', { headers, timeout: 8000 });
if (authErr(r)) throw new Error('Auth failed — check the access token');
if (r.status >= 400) throw new Error('Management API HTTP ' + r.status);
if (authErr(r)) ctx.fail('Auth failed — check the access token', { kind: ctx.KIND.AUTH });
if (r.status >= 400) ctx.fail('Management API HTTP ' + r.status);
const peers = Array.isArray(r.data) ? r.data : [];
const connected = peers.filter(p => p && p.connected);
out.peersTotal = peers.length;
Expand All @@ -219,14 +238,15 @@ async function vpnView(config, fetchJSON) {
}
}
} catch (e) {
out.error = e.message;
out.error = errorText(e);
}

return out;
}

/* ── Map view ── */
async function mapView(config, fetchJSON) {
async function mapView(ctx) {
const { config, fetchJSON } = ctx;
const wc = config || {};
const services = (Array.isArray(wc.services) ? wc.services : []).filter(s => s && s.enabled !== false && s.url);
const results = await Promise.all(
Expand All @@ -242,15 +262,15 @@ async function mapView(config, fetchJSON) {
try {
/* Same as the VPN path above: say the address is missing rather than
letting a doomed request report itself as a network fault. */
if (!base) throw new Error('No URL configured');
if (!base) ctx.fail('No URL configured', { kind: ctx.KIND.INVALID });
if (s.type === 'conduit') {
const r = await fetchJSON(new URL('/metrics', base).href, { raw: true });
if (r.status >= 400) throw new Error('HTTP ' + r.status);
if (r.status >= 400) ctx.fail('HTTP ' + r.status);
Object.assign(o, { kind: 'regions' }, parseConduitText(r.data));
} else if (s.type === 'gluetun') {
const r = await fetchJSON(base + '/v1/publicip/ip', { headers: s.apiKey ? { 'X-API-Key': s.apiKey } : {} });
if (r.status === 401) throw new Error('Auth required — set the API key');
if (r.status >= 400) throw new Error('HTTP ' + r.status);
if (r.status === 401) ctx.fail('Auth required — set the API key', { kind: ctx.KIND.AUTH });
if (r.status >= 400) ctx.fail('HTTP ' + r.status);
const d = r.data || {};
const L = d.location || d.loc;
o.kind = 'point';
Expand All @@ -265,8 +285,8 @@ async function mapView(config, fetchJSON) {
const r = await fetchJSON(base + '/api/peers', {
headers: s.token ? { Authorization: 'Token ' + s.token } : {},
});
if (authErr(r)) throw new Error('Auth required — check the token');
if (r.status >= 400) throw new Error('HTTP ' + r.status);
if (authErr(r)) ctx.fail('Auth required — check the token', { kind: ctx.KIND.AUTH });
if (r.status >= 400) ctx.fail('HTTP ' + r.status);
const peers = Array.isArray(r.data) ? r.data : [];
const regions = {};
let conn = 0;
Expand Down Expand Up @@ -294,8 +314,8 @@ async function mapView(config, fetchJSON) {
headers: { 'Content-Type': 'application/json', Authorization: s.apiKey ? 'Bearer ' + s.apiKey : '' },
body,
});
if (authErr(r)) throw new Error('Auth required — check the API key');
if (r.status >= 400) throw new Error('HTTP ' + r.status);
if (authErr(r)) ctx.fail('Auth required — check the API key', { kind: ctx.KIND.AUTH });
if (r.status >= 400) ctx.fail('HTTP ' + r.status);
const rows = (r.data && r.data.results) || [];
const regions = {};
let total = 0;
Expand All @@ -317,10 +337,10 @@ async function mapView(config, fetchJSON) {
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ username: s.username || '', password: s.password || '' }),
});
if (authErr(lg)) throw new Error('Auth required — check username/password');
if (lg.status >= 400) throw new Error('Login HTTP ' + lg.status);
if (authErr(lg)) ctx.fail('Auth required — check username/password', { kind: ctx.KIND.AUTH });
if (lg.status >= 400) ctx.fail('Login HTTP ' + lg.status);
const token = lg.data && lg.data.token;
if (!token) throw new Error('Login failed');
if (!token) ctx.fail('Login failed', { kind: ctx.KIND.AUTH });
const end = Date.now(),
start = end - 7 * 24 * 3600 * 1000;
const r = await fetchJSON(
Expand All @@ -333,7 +353,7 @@ async function mapView(config, fetchJSON) {
end,
{ headers: { Authorization: 'Bearer ' + token } },
);
if (r.status >= 400) throw new Error('HTTP ' + r.status);
if (r.status >= 400) ctx.fail('HTTP ' + r.status);
const rows = Array.isArray(r.data) ? r.data : [];
const regions = {};
let total = 0;
Expand All @@ -353,7 +373,7 @@ async function mapView(config, fetchJSON) {
o.error = 'Unsupported service type';
}
} catch (e) {
o.error = e.message || String(e);
o.error = errorText(e);
}
return o;
}),
Expand Down
Loading