From 0c0f34394f521ce2006ee3a0e3a0ddcd097c0e40 Mon Sep 17 00:00:00 2001 From: jherforth Date: Sun, 25 Oct 2026 09:56:43 -0400 Subject: [PATCH] fix(plugins): /widgets/ serves only installed plugins (mrramam/HomeGlow#60) The disk fallback in /widgets/:filename and the @fastify/static registration on server/widgets/ served files that aren't in the plugins table. The fallback skipped the manifest, and the static route skipped the token and frame scripts as well. Migration 18 already imported every pre-table file, so both were dead code for real installs, and reachable only by a file placed on disk. Delete no longer unlinks a file on disk either, since nothing serves one. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/plugin-platform.md | 2 +- server/index.js | 43 ++++++++-------------------- server/tests/pluginStore.test.js | 20 +++++++++++++ 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/docs/architecture/plugin-platform.md b/docs/architecture/plugin-platform.md index 5bf9a006..b162ab03 100644 --- a/docs/architecture/plugin-platform.md +++ b/docs/architecture/plugin-platform.md @@ -689,7 +689,7 @@ New/changed persistence, all additive: | Area | File(s) | Change | | --- | --- | --- | | DB-backed plugin store ✅ | [`server/migrations/schema18-pluginsTable.js`](../../server/migrations/schema18-pluginsTable.js) | Create `plugins`; import any on-disk `widgets/*.html` + registry. **Implemented.** | -| Widget serving ✅ | `server/index.js` `/widgets/:filename` + upload/install/delete/debug routes | Serve HTML from the `plugins` table; writes are row upserts (§10.2); read-only disk fallback for pre-migration files. **Implemented**, tested in [`server/tests/pluginStore.test.js`](../../server/tests/pluginStore.test.js). | +| Widget serving ✅ | `server/index.js` `/widgets/:filename` + upload/install/delete/debug routes | Serve HTML from the `plugins` table; writes are row upserts (§10.2). Only installed plugins are served: pre-migration files were imported by migration 18, and `server/widgets/` itself is never served. **Implemented**, tested in [`server/tests/pluginStore.test.js`](../../server/tests/pluginStore.test.js). | | Storage table ✅ | [`server/migrations/schema19-pluginStorage.js`](../../server/migrations/schema19-pluginStorage.js) | Create `plugin_storage`. **Implemented.** | | Storage API ✅ | `server/index.js` `/api/plugin/v1/storage/...` routes | CRUD + atomic increment, caps, `storage: true` guard. **Implemented**, tested in [`pluginPlatform.test.js`](../../server/tests/pluginPlatform.test.js). | | Manifest handling ✅ | `server/index.js` `installPluginRow` / `extractPluginManifest` | Parse + validate embedded manifest, store in `plugins.manifest_json`. **Implemented.** | diff --git a/server/index.js b/server/index.js index c1fab665..0fdc7740 100644 --- a/server/index.js +++ b/server/index.js @@ -698,20 +698,14 @@ fastify.register(require('@fastify/static'), { maxAge: 86400000, // 1 day cache }); -// Serve static files for widgets -fastify.register(require('@fastify/static'), { - root: path.join(__dirname, 'widgets'), - prefix: '/widgets/', - decorateReply: false -}); - // Widget filenames are restricted to this charset at upload/install time; the -// serve route enforces the same rule so encoded ../ segments can never reach -// the disk-fallback path.join below (path traversal guard). +// serve route enforces the same rule. const WIDGET_FILENAME_REGEX = /^[a-zA-Z0-9-._]+$/; -// Serve widget HTML from the DB-backed plugin store (issue #105 Phase 0), with -// a read-only disk fallback for files that predate the plugins table. +// Serve widget HTML from the DB-backed plugin store (issue #105 Phase 0), and +// nothing else: every page gets its token and frame scripts injected here. +// Files that predate the plugins table were imported by migration 18, so +// server/widgets/ is never served (mrramam/HomeGlow#60). fastify.get('/widgets/:filename', async (request, reply) => { const { filename } = request.params; @@ -720,18 +714,14 @@ fastify.get('/widgets/:filename', async (request, reply) => { } try { - let content; - let pluginId = null; const row = db.prepare('SELECT id, content, plugin_id, source FROM plugins WHERE filename = ?').get(filename); - if (row) { - // A Home Assistant panel is drawn by the built-in template from its - // recipe (issue #252), so every panel gets the current template. - content = row.source === 'builder' ? haPanels.renderPanel(row.id, row.plugin_id) : row.content; - pluginId = row.plugin_id; - } else { - const filePath = path.join(__dirname, 'widgets', filename); - content = await fs.readFile(filePath, 'utf-8'); + if (!row) { + return reply.status(404).send(`Widget file not found: ${filename}`); } + // A Home Assistant panel is drawn by the built-in template from its + // recipe (issue #252), so every panel gets the current template. + let content = row.source === 'builder' ? haPanels.renderPanel(row.id, row.plugin_id) : row.content; + const pluginId = row.plugin_id; content = content.replace(/window\.location\.origin\.replace\(['"`]:\d+['"`],\s*['"`]:\d+['"`]\)/g, 'window.location.origin'); content = content.replace(/\$\{window\.location\.protocol\}\/\/\$\{window\.location\.hostname\}:\d+/g, '${window.location.origin}'); @@ -1354,16 +1344,7 @@ fastify.delete('/api/widgets/:filename', async (request, reply) => { } } - // Best-effort cleanup of a pre-migration file on disk. - let removedFromDisk = false; - try { - await fs.unlink(path.join(__dirname, 'widgets', filename)); - removedFromDisk = true; - } catch { - // Nothing on disk — expected for DB-backed plugins. - } - - if (result.changes === 0 && !removedFromDisk) { + if (result.changes === 0) { return reply.status(404).send({ error: 'Widget not found.' }); } diff --git a/server/tests/pluginStore.test.js b/server/tests/pluginStore.test.js index 077c6d35..73c0567e 100644 --- a/server/tests/pluginStore.test.js +++ b/server/tests/pluginStore.test.js @@ -137,6 +137,26 @@ test('migration imports pre-existing on-disk widgets into the plugin store', asy assert.equal(imported.name, legacyFilename.replace('.html', '')); }); +// mrramam/HomeGlow#60: only the plugins table is served, so every page gets +// its token and frame scripts. A file on disk that isn't installed isn't. +test('/widgets/ never serves a file from disk that is not an installed plugin', async () => { + const strayFilename = `stray-${process.pid}.html`; + const strayDir = path.join(widgetsDir, `stray-${process.pid}`); + fs.writeFileSync(path.join(widgetsDir, strayFilename), 'not-installed-content', 'utf-8'); + fs.mkdirSync(strayDir, { recursive: true }); + fs.writeFileSync(path.join(strayDir, 'page.html'), 'not-installed-content', 'utf-8'); + try { + for (const url of [`/widgets/${strayFilename}`, `/widgets/stray-${process.pid}/page.html`, '/widgets/README.md']) { + const res = await api(url); + assert.equal(res.status, 404, url); + assert.ok(!String(res.body).includes('not-installed-content'), url); + } + } finally { + fs.rmSync(path.join(widgetsDir, strayFilename), { force: true }); + fs.rmSync(strayDir, { recursive: true, force: true }); + } +}); + test('POST /api/widgets/upload stores a widget in the DB and serves it', async () => { const form = new FormData(); form.append('file', new Blob([uploadedHtml], { type: 'text/html' }), uploadedFilename);