diff --git a/CHANGELOG.md b/CHANGELOG.md index d0bbc71..ec3e8b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed +- **Dev builds could hide stable releases from everyone else.** The update check read a single page of a hundred registry tags. Tags come back ordered by recency, so a run of development builds after a release fills that page with `X.Y.Z-dev.N` — every one of which a stable deployment filters out, leaving it with nothing and reporting no update available when one existed. It reads further pages now, but only while it has found nothing usable: the newest release is on the first page in the ordinary case, so this normally makes exactly one request as before. Bounded at five pages, so a registry that keeps offering another one cannot hang the check. + + It fails quietly and selectively, which is what makes it worth fixing ahead of the arithmetic — only people on older versions, only on the stable channel, with no error anywhere. - **One dev tag on the registry would have silenced update notices for everyone.** The tag filter was `/^\d+\.\d+\.\d+/`, unanchored, so `1.9.0-dev` passed it and then parsed to `NaN` — which made the sort comparator return `NaN`, leaving the ordering undefined and letting a prerelease surface as the newest tag, whereupon the version check correctly refused it and reported no update at all. Version tags are matched strictly now, and sorted by a comparator that understands them. - **The in-app updater could sit on `WAITING FOR SERVER` indefinitely.** Every step failed silently — both child processes discarded their output, the route answered "Update started" before checking anything could work, and a non-zero exit from `docker compose pull` simply returned — so a stack that *could not* update was indistinguishable from one still working, and the client polled every three seconds forever with no deadline. diff --git a/README.md b/README.md index 0711a99..faa8eff 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ CITY_NET/ ├── backend/ │ ├── server.js # Express entrypoint — mounts routes, starts Socket.IO │ ├── db.js # SQLite schema and migrations -│ ├── updater.js # In-app self-update — release channels selected by IMAGE_TAG alone, the same variable compose pulls with (X.Y.Z-dev tags with an optional counter, ordered so a release supersedes its own dev builds); preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change +│ ├── updater.js # In-app self-update — paginated registry tag listing so a run of dev builds cannot hide a stable release; release channels selected by IMAGE_TAG alone, the same variable compose pulls with (X.Y.Z-dev tags with an optional counter, ordered so a release supersedes its own dev builds); preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change │ ├── middleware/ │ │ └── auth.js # JWT verify middleware (admin + elevated users) │ ├── routes/ diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js index 0c600a5..fab6261 100644 --- a/backend/__tests__/updater.test.js +++ b/backend/__tests__/updater.test.js @@ -141,6 +141,119 @@ describe('dev version ordering', () => { }); }); +describe('fetchVersionTags', () => { + /** A registry serving the given pages in order. */ + const registry = (pages) => { + const calls = []; + let n = 0; + return { + calls, + https: { + request(options, cb) { + calls.push(`${options.hostname}${options.path}`); + const page = pages[n++] ?? { results: [] }; + const handlers = {}; + const upstream = { on: (evt, fn) => { handlers[evt] = fn; return upstream; } }; + const req = { + on: () => req, + end: () => { + cb(upstream); + process.nextTick(() => { + handlers.data?.(JSON.stringify(page)); + handlers.end?.(); + }); + }, + }; + return req; + }, + }, + }; + }; + + const page = (names, next = null) => ({ results: names.map((name) => ({ name })), next }); + + it('reads one page when that page already has a usable tag', async () => { + // The normal case. Later pages were updated longer ago, so there is nothing on them + // worth having once this channel has found something. + const reg = registry([page(['1.8.0', '1.8.1', 'latest'], 'https://hub.docker.com/v2/x?page=2')]); + const tags = await updater.fetchVersionTags({ https: reg.https }); + expect(tags[0]).toBe('1.8.1'); + expect(reg.calls).toHaveLength(1); + }); + + it('follows the next page when dev builds have crowded the first one out', async () => { + // The failure this fixes. A burst of dev builds after a release fills page one with + // X.Y.Z-dev tags, a stable deployment filters every one of them out, and reading a + // single page left it with nothing — reporting no update when one existed. + const devTags = Array.from({ length: 100 }, (_, i) => `1.9.0-dev.${i + 1}`); + const reg = registry([ + page(devTags, 'https://hub.docker.com/v2/repositories/x/tags?page=2'), + page(['1.8.1', '1.8.0']), + ]); + const tags = await updater.fetchVersionTags({ https: reg.https, allowDev: false }); + expect(tags[0]).toBe('1.8.1'); + expect(reg.calls).toHaveLength(2); + expect(reg.calls[1]).toContain('page=2'); + }); + + it('stops at the first page for a dev deployment, which can use those tags', async () => { + const devTags = Array.from({ length: 100 }, (_, i) => `1.9.0-dev.${i + 1}`); + const reg = registry([page(devTags, 'https://hub.docker.com/v2/x?page=2'), page(['1.8.1'])]); + const tags = await updater.fetchVersionTags({ https: reg.https, allowDev: true }); + expect(tags[0]).toBe('1.9.0-dev.100'); + expect(reg.calls).toHaveLength(1); + }); + + it('gives up after the page limit rather than following for ever', async () => { + // A bound, not an open loop: a registry that keeps offering a next page must not be + // able to hang the request. + const endless = page(['latest'], 'https://hub.docker.com/v2/x?page=n'); + const reg = registry([endless, endless, endless, endless, endless, endless, endless]); + const tags = await updater.fetchVersionTags({ https: reg.https, maxPages: 3 }); + expect(tags).toEqual([]); + expect(reg.calls).toHaveLength(3); + }); + + it('stops when the registry offers no next page', async () => { + const reg = registry([page(['latest', 'dev'])]); + const tags = await updater.fetchVersionTags({ https: reg.https }); + expect(tags).toEqual([]); + expect(reg.calls).toHaveLength(1); + }); + + it('returns tags newest first', async () => { + const reg = registry([page(['1.8.0', '1.10.0', '1.9.0'])]); + expect(await updater.fetchVersionTags({ https: reg.https })).toEqual(['1.10.0', '1.9.0', '1.8.0']); + }); + + it('rejects rather than resolving empty when the response is not JSON', async () => { + // Resolving empty would be indistinguishable from "no releases published", and the + // route would report you are up to date. + const https = { + request(options, cb) { + const handlers = {}; + const upstream = { on: (evt, fn) => { handlers[evt] = fn; return upstream; } }; + const req = { + on: () => req, + end: () => { cb(upstream); process.nextTick(() => { handlers.data?.(''); handlers.end?.(); }); }, + }; + return req; + }, + }; + await expect(updater.fetchVersionTags({ https })).rejects.toThrow(/parse/i); + }); + + it('rejects when the registry cannot be reached', async () => { + const https = { + request() { + const req = { on: (evt, fn) => { if (evt === 'error') process.nextTick(() => fn(new Error('ENOTFOUND'))); return req; }, end: () => {} }; + return req; + }, + }; + await expect(updater.fetchVersionTags({ https })).rejects.toThrow(/ENOTFOUND/); + }); +}); + describe('preflight', () => { const labels = { projectName: 'citynet', configFile: '/srv/citynet/docker-compose.yml', workingDir: '/srv/citynet' }; const allPresent = () => true; diff --git a/backend/routes/admin.js b/backend/routes/admin.js index a99677a..0e3d89b 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -222,59 +222,32 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { router.post('/check-update', authenticate, (req, res) => { if (req.user.isTemporary) return res.status(403).json({ error: 'Primary admin only' }); - const https = require('https'); const currentVersion = process.env.APP_VERSION || require('../../package.json').version; - const options = { - hostname: 'hub.docker.com', - path: '/v2/repositories/over2take/citynet-frontend/tags?page_size=100', - method: 'GET', - }; - - const request = https.request(options, (upstream) => { - let body = ''; - upstream.on('data', chunk => { body += chunk; }); - upstream.on('end', () => { - try { - const data = JSON.parse(body); - // Find version tags (skip 'latest'), sort and get highest version - // Dev builds are published as X.Y.Z-dev and ignored unless DEV=true. The - // previous filter was unanchored, so a tag like 1.9.0-dev passed it and then - // parsed to NaN, which made the comparator return NaN and left the sort - // order undefined — one dev tag on the registry could stop stable users - // being told about releases at all. - const allowDev = updater.allowsDevBuilds(); - const versionTags = data.results - ?.filter(tag => updater.isVersionTag(tag.name, allowDev)) - .map(tag => tag.name) - .sort((a, b) => updater.compareVersions(updater.parseVersion(b), updater.parseVersion(a))) || []; - const latestTag = versionTags[0] || 'unknown'; - // Strictly newer, not merely different. Comparing with !== offers a - // downgrade whenever the published tag trails the running one. - const hasUpdate = latestTag !== 'unknown' && updater.isNewerVersion(latestTag, currentVersion); - - res.json({ - current: currentVersion, - latest: latestTag, - hasUpdate, - message: hasUpdate - ? `Update available: ${currentVersion} → ${latestTag}` - : `You're up to date (${currentVersion})`, - }); - } catch (e) { - res.status(500).json({ error: 'Failed to parse Docker Hub response' }); + updater.fetchVersionTags({ allowDev: updater.allowsDevBuilds() }) + .then((versionTags) => { + const latestTag = versionTags[0] || 'unknown'; + // Strictly newer, not merely different. Comparing with !== offers a downgrade + // whenever the published tag trails the running one. + const hasUpdate = latestTag !== 'unknown' && updater.isNewerVersion(latestTag, currentVersion); + + res.json({ + current: currentVersion, + latest: latestTag, + hasUpdate, + message: hasUpdate + ? `Update available: ${currentVersion} → ${latestTag}` + : `You're up to date (${currentVersion})`, + }); + }) + .catch((err) => { + if (err.message === 'Failed to parse Docker Hub response') { + return res.status(500).json({ error: err.message }); } + res.status(502).json({ error: `Could not reach Docker Hub: ${err.message}` }); }); - }); - - request.on('error', (err) => { - res.status(502).json({ error: `Could not reach Docker Hub: ${err.message}` }); - }); - - request.end(); }); - // --- Apply Update --- router.post('/update', authenticate, (req, res) => { if (req.user.isTemporary) return res.status(403).json({ error: 'Primary admin only' }); diff --git a/backend/updater.js b/backend/updater.js index 28cfeb8..6ac48cd 100644 --- a/backend/updater.js +++ b/backend/updater.js @@ -113,6 +113,80 @@ function allowsDevBuilds(env = process.env) { return imageTag(env) === 'dev'; } +/** The registry listing the update check reads. */ +const REGISTRY_HOST = 'hub.docker.com'; +const REGISTRY_PATH = '/v2/repositories/over2take/citynet-frontend/tags?page_size=100'; + +/** + * How many pages of tags to read before giving up. + * + * Five is 500 tags, far more than this project will accumulate between releases, and a + * bound rather than an open loop so a registry misbehaving cannot hang the request. + */ +const MAX_TAG_PAGES = 5; + +/** + * Every version tag the given channel can use, newest first. + * + * Reads one page and then keeps going only while it has found nothing usable. That + * matters because the listing is ordered by recency, not by version: a burst of dev + * builds after a release fills the first page with `X.Y.Z-dev.N` tags, and a stable + * deployment filters every one of them out. Reading a single page, as this used to, + * would leave it with an empty list and report no update available — silently, only for + * people on the stable channel, and only once enough dev builds had accumulated. + * + * Normally it stops after one request, because the newest release is on the first page. + */ +function fetchVersionTags(opts = {}) { + const httpsMod = opts.https || require('https'); + const allowDev = opts.allowDev ?? false; + const maxPages = opts.maxPages ?? MAX_TAG_PAGES; + + const readPage = (host, path) => new Promise((resolve, reject) => { + const req = httpsMod.request({ hostname: host, path, method: 'GET' }, (upstream) => { + let body = ''; + upstream.on('data', (chunk) => { body += chunk; }); + upstream.on('end', () => { + try { + resolve(JSON.parse(body)); + } catch { + reject(new Error('Failed to parse Docker Hub response')); + } + }); + }); + req.on('error', reject); + req.end(); + }); + + const walk = async () => { + const found = []; + let host = REGISTRY_HOST; + let path = REGISTRY_PATH; + + for (let page = 0; page < maxPages; page++) { + const data = await readPage(host, path); + for (const tag of data?.results ?? []) { + if (isVersionTag(tag.name, allowDev)) found.push(tag.name); + } + // Anything on a later page was updated longer ago, so once this channel has + // something there is nothing to gain by reading on. + if (found.length > 0 || !data?.next) break; + + try { + const next = new URL(data.next); + host = next.hostname; + path = `${next.pathname}${next.search}`; + } catch { + break; + } + } + + return found.sort((a, b) => compareVersions(parseVersion(b), parseVersion(a))); + }; + + return walk(); +} + /** * Build the docker-run argument list for the self-update helper container. * @@ -289,6 +363,10 @@ module.exports = { isVersionTag, imageTag, allowsDevBuilds, + fetchVersionTags, + REGISTRY_HOST, + REGISTRY_PATH, + MAX_TAG_PAGES, isNewerVersion, buildUpdateHelperArgs, readComposeLabels,