diff --git a/README.md b/README.md index 92ac2e637..70bc79377 100644 --- a/README.md +++ b/README.md @@ -331,6 +331,8 @@ The script creates a timestamped DB backup before making changes and prompts for For production, put TREK behind a TLS-terminating reverse proxy. TREK uses WebSockets for real-time sync, so the proxy **must** support WebSocket upgrades on `/ws`. +If you use the MCP addon, the proxy must also pass the `Mcp-Session-Id` header through in both directions on `/mcp` — Nginx and Caddy do this by default, but a proxy that strips it makes every tool call open a new session instead of reusing one. See the [Reverse Proxy wiki page](https://github.com/mauriceboe/TREK/wiki/Reverse-Proxy) for details. +
Nginx @@ -368,6 +370,19 @@ server { proxy_set_header Host $host; proxy_read_timeout 86400; } + + # Only needed if you use the MCP addon. Responses are Server-Sent Events, + # so buffering must be off or tool results arrive late. + location /mcp { + proxy_pass http://localhost:3000; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_read_timeout 3600s; + } } ``` @@ -430,7 +445,7 @@ Caddy handles TLS and WebSockets automatically. | `DEMO_MODE` | Enable demo mode (hourly data resets) | `false` | | `UNSPLASH_ACCESS_KEY` | Optional Unsplash Access Key for trip-cover and place-image search. Without one, TREK uses Unsplash's unauthenticated endpoint, which some datacenter/VPS IPs are blocked from. Get a free key at [unsplash.com/developers](https://unsplash.com/developers). Overrides any per-admin key set in Admin > Settings (where it can also be configured instead). | — | | `MCP_RATE_LIMIT` | Max MCP API requests per user per minute | `300` | -| `MCP_MAX_SESSION_PER_USER` | Max concurrent MCP sessions per user | `20` | +| `MCP_MAX_SESSION_PER_USER` | Max concurrent MCP sessions per user. At the cap, the least-recently-active session is closed to make room | `20` |
diff --git a/client/e2e/create-trip.spec.ts b/client/e2e/create-trip.spec.ts index 6c264c927..42a4e5720 100644 --- a/client/e2e/create-trip.spec.ts +++ b/client/e2e/create-trip.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from '@playwright/test' +import { dismissSystemNotices } from './helpers' // Trip lifecycle (core): from the dashboard, open the new-trip modal, name the // trip, submit, and confirm it shows up on the dashboard. Exercises the whole @@ -7,18 +8,23 @@ import { test, expect } from '@playwright/test' test('create a trip and see it on the dashboard', async ({ page }) => { await page.goto('/dashboard') + // The release notice greets a freshly seeded user and its backdrop eats the click below. + await dismissSystemNotices(page) + // The "+ New Trip" card is always rendered in the default (planned) filter. await page.locator('.add-trip-card').click() - // Scope to the shared Modal (.modal-backdrop). Its form has no in-form submit - // button (the primary action lives in the footer), so click it explicitly - // rather than pressing Enter. The Create button is the slate primary button; - // Cancel is the bordered one. - const modal = page.locator('.modal-backdrop') + // Scope to the shared Modal (.trek-modal-backdrop — namespaced so content blockers + // don't hide a generic .modal-backdrop). Its form has no in-form submit button (the + // primary action lives in the footer), so click it explicitly rather than pressing + // Enter. The Create button is the slate primary button; Cancel is the bordered one. + const modal = page.locator('.trek-modal-backdrop') await expect(modal).toBeVisible() + // Target Title by placeholder: the cover-image search inputs sit above it, so + // input[type=text].first() is the photo search box, not the field we want. const title = `E2E Trip ${Date.now()}` - await modal.locator('input[type="text"]').first().fill(title) + await modal.getByPlaceholder('e.g. Summer in Japan').fill(title) await modal.getByRole('button', { name: 'Create New Trip' }).click() await expect(page.getByText(title).first()).toBeVisible({ timeout: 15_000 }) diff --git a/client/e2e/helpers.ts b/client/e2e/helpers.ts new file mode 100644 index 000000000..c91d48152 --- /dev/null +++ b/client/e2e/helpers.ts @@ -0,0 +1,22 @@ +import type { Page } from '@playwright/test' + +/** + * Dismiss the release-notice modal (SystemNoticeHost), which greets a freshly seeded + * user on first load and covers the dashboard — its backdrop swallows clicks aimed at + * anything underneath, `.add-trip-card` included. + * + * The X only appears on the notice's last page, so page through first. Dismissal is + * persisted server-side per user, but each spec gets a fresh DB, so every spec that + * touches the dashboard has to clear it. + */ +export async function dismissSystemNotices(page: Page): Promise { + const next = page.getByRole('button', { name: /next/i }) + for (let i = 0; i < 6 && (await next.isVisible().catch(() => false)); i++) { + if (!(await next.isEnabled())) break + await next.click() + } + + const dismiss = page.getByRole('button', { name: 'Dismiss' }) + if (await dismiss.isVisible().catch(() => false)) await dismiss.click() + await dismiss.waitFor({ state: 'detached' }).catch(() => {}) +} diff --git a/client/e2e/ipad-scroll-1432.spec.ts b/client/e2e/ipad-scroll-1432.spec.ts new file mode 100644 index 000000000..55631b4ce --- /dev/null +++ b/client/e2e/ipad-scroll-1432.spec.ts @@ -0,0 +1,79 @@ +import { test, expect, devices } from '@playwright/test' +import { dismissSystemNotices } from './helpers' + +// Tablet regression guard for #1432 — the places list must scroll under a touch swipe. +// +// A tablet is a coarse-pointer device at a *desktop* viewport width, so the width-based +// "is this mobile" check that 3.2.1 shipped left `draggable` armed on iPad: the swipe +// became an HTML5 drag and raised the drop-to-import overlay instead of scrolling. Drag +// is now gated on `(pointer: coarse)` (useIsTouch), and only a real device context proves +// it — a jsdom unit test cannot express "coarse pointer at 834px". +// +// Needs WebKit (`npx playwright install webkit`, plus libmanette-0.2-0 and libwoff1 on +// Debian/Ubuntu). WebKit is the right engine here, not a nicety: every browser on iPadOS +// is WebKit underneath, which is why the reporter saw this in all three they tried. +test.use({ ...devices['iPad Pro 11'] }) + +test('#1432 iPad: places list is scrollable, not draggable', async ({ page }) => { + await page.goto('/dashboard') + + await dismissSystemNotices(page) + + await page.locator('.add-trip-card').click() + const createBtn = page.getByRole('button', { name: 'Create New Trip' }) + await expect(createBtn).toBeVisible() + const title = `iPad 1432 ${Date.now()}` + await page.getByPlaceholder('e.g. Summer in Japan').fill(title) + await createBtn.click() + + await page.getByText(title).first().click() + await expect(page).toHaveURL(/\/trips\/\d+/) + await expect(page.locator('.leaflet-container')).toBeVisible({ timeout: 20_000 }) + + const tripId = page.url().match(/\/trips\/(\d+)/)![1] + + // Seed enough places for the list to overflow and actually need scrolling. + for (let i = 1; i <= 25; i++) { + const res = await page.request.post(`/api/trips/${tripId}/places`, { + data: { name: `Place ${i}`, lat: 48.85 + i * 0.01, lng: 2.35 + i * 0.01 }, + }) + expect(res.ok(), `seed place ${i}`).toBeTruthy() + } + await page.reload() + await expect(page.locator('.leaflet-container')).toBeVisible({ timeout: 20_000 }) + await expect(page.getByText('Place 1').first()).toBeVisible({ timeout: 20_000 }) + + // The context must really be the one from the bug report: coarse pointer, desktop + // width. If either is wrong, everything below proves nothing. + const env = await page.evaluate(() => ({ + coarse: window.matchMedia('(pointer: coarse)').matches, + width: window.innerWidth, + })) + expect(env.coarse, 'iPad reports a coarse primary pointer').toBe(true) + expect(env.width, 'iPad sits above the 768px "mobile" breakpoint').toBeGreaterThanOrEqual(768) + + // 1. Rows must not be draggable — a draggable row is what swallowed the scroll gesture. + const row = page.locator('div[draggable]').filter({ hasText: 'Place 1' }).first() + await expect(row).toHaveAttribute('draggable', 'false') + + // 2. The list must scroll, and no drop-to-import overlay may appear. + const scroller = page.locator('div[draggable]').first().locator('xpath=ancestor::div[@class="trek-stagger"]') + const before = await scroller.evaluate(el => el.scrollTop) + const box = (await scroller.boundingBox())! + await page.touchscreen.tap(box.x + box.width / 2, box.y + 40) + await scroller.evaluate(el => el.scrollBy(0, 200)) + const after = await scroller.evaluate(el => el.scrollTop) + expect(after, 'places list scrolled').toBeGreaterThan(before) + await expect(page.getByText('Drop to import')).toHaveCount(0) + + // 3. Drag being off means the arrow buttons are the only reorder affordance left — + // they must be visible (they were opacity:0 above 767px). + const arrowOpacity = await page.evaluate(() => { + const el = document.querySelector('.reorder-buttons') + return el ? getComputedStyle(el).opacity : 'absent' + }) + expect(['1', 'absent']).toContain(arrowOpacity) + + // 4. The iPad must still get the desktop two-pane layout — isMobile stayed width-based. + await expect(page.locator('.leaflet-container')).toBeVisible() +}) diff --git a/client/e2e/reorder-arrows.spec.ts b/client/e2e/reorder-arrows.spec.ts new file mode 100644 index 000000000..d0fca0c1a --- /dev/null +++ b/client/e2e/reorder-arrows.spec.ts @@ -0,0 +1,61 @@ +import { test, expect } from '@playwright/test' +import { dismissSystemNotices } from './helpers' + +// The day-plan reorder arrows are hover-revealed on desktop. The rule that did that was +// dead for a long time — it targeted `.place-row .reorder-btns`, neither of which exists +// (the component renders `.reorder-buttons` inside an unclassed row), so the buttons sat +// at opacity:0 with no way to reveal them. +// +// That is not merely "invisible": opacity:0 still hit-tests, so every itinerary row and +// note carried an invisible, fully clickable target that silently reordered the trip. +// These cases pin both halves — hidden means non-interactive, hover means visible. +test('desktop: reorder arrows are hidden-and-inert until the row is hovered', async ({ page }) => { + await page.goto('/dashboard') + await dismissSystemNotices(page) + + await page.locator('.add-trip-card').click() + const modal = page.locator('.trek-modal-backdrop') + await expect(modal).toBeVisible() + const title = `Reorder ${Date.now()}` + await modal.getByPlaceholder('e.g. Summer in Japan').fill(title) + await modal.getByRole('button', { name: 'Create New Trip' }).click() + + await page.getByText(title).first().click() + await expect(page).toHaveURL(/\/trips\/\d+/) + await expect(page.locator('.leaflet-container')).toBeVisible({ timeout: 20_000 }) + + // Two places on day 1, so the day plan renders rows carrying reorder arrows. + const tripId = page.url().match(/\/trips\/(\d+)/)![1] + const daysRes = await (await page.request.get(`/api/trips/${tripId}/days`)).json() + const dayId = (daysRes.days ?? daysRes)[0].id + for (const name of ['Alpha', 'Beta']) { + const res = await page.request.post(`/api/trips/${tripId}/places`, { + data: { name, lat: 48.85, lng: 2.35 }, + }) + const body = await res.json() + await page.request.post(`/api/trips/${tripId}/days/${dayId}/assignments`, { + data: { place_id: body.place?.id ?? body.id }, + }) + } + await page.reload() + await expect(page.locator('.leaflet-container')).toBeVisible({ timeout: 20_000 }) + + const row = page.locator('.dp-row').filter({ hasText: 'Alpha' }).first() + await expect(row).toBeVisible({ timeout: 20_000 }) + const arrows = row.locator('.reorder-buttons') + + // Unhovered: invisible AND inert — a click there must not land on the button. + const idle = await arrows.evaluate(el => { + const cs = getComputedStyle(el) + const r = el.getBoundingClientRect() + const hit = document.elementFromPoint(r.x + r.width / 2, r.y + r.height / 2) + return { opacity: cs.opacity, hitsArrow: !!hit?.closest('.reorder-buttons') } + }) + expect(idle.opacity, 'arrows hidden until hover').toBe('0') + expect(idle.hitsArrow, 'hidden arrows must not swallow clicks').toBe(false) + + // Hovered: revealed and clickable. + await row.hover() + await expect(arrows).toHaveCSS('opacity', '1') + await expect(arrows).toHaveCSS('pointer-events', 'auto') +}) diff --git a/client/e2e/trip-planner.spec.ts b/client/e2e/trip-planner.spec.ts index 9a4b8b2ce..754145ca3 100644 --- a/client/e2e/trip-planner.spec.ts +++ b/client/e2e/trip-planner.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from '@playwright/test' +import { dismissSystemNotices } from './helpers' // Open a trip into the planner: create a trip, open it from the dashboard, and // confirm the trip planner (TripPlannerPage — the app's largest page) actually @@ -6,12 +7,17 @@ import { test, expect } from '@playwright/test' test('open a trip and land in the planner with a map', async ({ page }) => { await page.goto('/dashboard') + // The release notice greets a freshly seeded user and its backdrop eats the click below. + await dismissSystemNotices(page) + // Create a trip to open. await page.locator('.add-trip-card').click() - const modal = page.locator('.modal-backdrop') + const modal = page.locator('.trek-modal-backdrop') await expect(modal).toBeVisible() + // Target Title by placeholder: the cover-image search inputs sit above it, so + // input[type=text].first() is the photo search box, not the field we want. const title = `E2E Planner ${Date.now()}` - await modal.locator('input[type="text"]').first().fill(title) + await modal.getByPlaceholder('e.g. Summer in Japan').fill(title) await modal.getByRole('button', { name: 'Create New Trip' }).click() // Open it from the dashboard. diff --git a/client/src/api/client.ts b/client/src/api/client.ts index b3f05d041..3c8cb6c19 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -502,6 +502,11 @@ export const adminApi = { pluginActivate: (id: string, consent?: boolean) => apiClient.post(`/admin/plugins/${id}/activate`, consent ? { consent: true } : {}).then(r => r.data), pluginDeactivate: (id: string) => apiClient.post(`/admin/plugins/${id}/deactivate`).then(r => r.data), pluginUpdate: (id: string) => apiClient.post(`/admin/plugins/${id}/update`).then(r => r.data), + // Re-trust a ROTATED author signing key and update, in ONE call. `publicKey` is the + // full key the admin was shown (not a fingerprint): the server compares it exactly, so + // it can refuse if the registry entry was re-keyed again since the dialog rendered. + pluginRetrust: (id: string, version: string, publicKey: string) => + apiClient.post(`/admin/plugins/${id}/retrust`, { version, publicKey }).then(r => r.data), pluginUninstall: (id: string, deleteData: boolean) => apiClient.post(`/admin/plugins/${id}/uninstall`, { deleteData }).then(r => r.data), pluginRescan: () => apiClient.post('/admin/plugins/rescan').then(r => r.data), pluginUpload: (file: File) => { const fd = new FormData(); fd.append('file', file); return postMultipart('/admin/plugins/upload', fd) }, diff --git a/client/src/components/Admin/AdminPluginsPanel.test.tsx b/client/src/components/Admin/AdminPluginsPanel.test.tsx index 3264de51c..8c1884818 100644 --- a/client/src/components/Admin/AdminPluginsPanel.test.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.test.tsx @@ -1,5 +1,5 @@ import { http, HttpResponse } from 'msw' -import { beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { server } from '../../../tests/helpers/msw/server' import { fireEvent, render, screen, waitFor } from '../../../tests/helpers/render' import { resetAllStores } from '../../../tests/helpers/store' @@ -119,3 +119,366 @@ describe('AdminPluginsPanel — Discover modal, operator-egress pill', () => { expect(screen.queryByText(/hosts you add/i)).not.toBeInTheDocument() }) }) + +/** + * #1523. The row's ⋯ menu used to be an in-flow `absolute` div, and PageSidebar — the + * panel's ancestor — is `overflow-hidden`. On the lower rows of a long plugin list the + * menu was clipped mid-way, taking Delete with it: the plugin became uninstallable from + * the UI. It must escape every overflow ancestor, and flip up when the bottom is tight. + */ +describe('AdminPluginsPanel — row ⋯ menu is never clipped (#1523)', () => { + const withRepo = plugin({ source_repo: 'trek/gotify', operatorEgress: false }) + const realRect = HTMLButtonElement.prototype.getBoundingClientRect + afterEach(() => { HTMLButtonElement.prototype.getBoundingClientRect = realRect }) + + /** Put the ⋯ button wherever we want in an 800px-tall viewport. */ + function stubTriggerAt(top: number) { + window.innerHeight = 800 + window.innerWidth = 1200 + HTMLButtonElement.prototype.getBoundingClientRect = function () { + return { top, bottom: top + 34, left: 1100, right: 1134, width: 34, height: 34, x: 1100, y: top, toJSON: () => ({}) } as DOMRect + } + } + + async function openRowMenu() { + mockList(withRepo) + const { container } = render() + fireEvent.click(await screen.findByTestId('plugin-row-menu-btn-trek-gotify')) + return { container, menu: screen.getByTestId('plugin-row-menu-trek-gotify') } + } + + it('FE-COMP-PLUGINS-MENU-001: renders every action, including Delete', async () => { + stubTriggerAt(100) + await openRowMenu() + + for (const label of [/restart/i, /error log/i, /allowed hosts/i, /source repository/i, /report an issue/i, /delete/i]) { + expect(screen.getByText(label)).toBeInTheDocument() + } + }) + + it('FE-COMP-PLUGINS-MENU-002: is portaled out of the panel, so no overflow ancestor can clip it', async () => { + stubTriggerAt(100) + const { container, menu } = await openRowMenu() + + // THE regression guard: living inside the panel is exactly what got it clipped. + expect(container.contains(menu)).toBe(false) + expect(menu.parentElement).toBe(document.body) + expect(menu.style.position).toBe('fixed') + }) + + it('FE-COMP-PLUGINS-MENU-003: hangs below the ⋯ when there is room', async () => { + stubTriggerAt(100) + const { menu } = await openRowMenu() + + expect(menu.style.top).toBe('138px') // trigger bottom (134) + 4 + expect(menu.style.bottom).toBe('') + expect(menu.style.right).toBe('66px') // viewport (1200) - trigger right (1134) + }) + + it('FE-COMP-PLUGINS-MENU-004: flips upward for a row near the bottom — the #1523 case', async () => { + stubTriggerAt(700) // 66px of room below: the six-item menu would run off-screen + const { menu } = await openRowMenu() + + expect(menu.style.bottom).toBe('104px') // viewport (800) - trigger top (700) + 4 + expect(menu.style.top).toBe('') + }) +}) + +/** + * Signature status (#plugins). TREK has always verified author signatures and TOFU-pinned + * the key — and never showed any of it, so a successfully-installed UNSIGNED plugin looked + * identical to a signed one, forever. + * + * The two tests that matter most here are the ones guarding the override: a re-trust is + * offered for a ROTATED key (benign explanation) and for NOTHING else. A signature that + * doesn't verify means the bytes are not what the author signed, and there is no story + * where the right answer is letting the admin wave it through. + */ +function registryEntry(over: Record = {}) { + return { + id: 'trek-gotify', name: 'Gotify', author: 'Acme', description: 'Push', repo: 'acme/gotify', + type: 'integration', latest: '2.0.0', minTrekVersion: null, reviewedAt: null, + screenshotUrl: null, signed: true, authorPublicKey: 'NEWKEYbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + ...over, + } +} + +function mockPanel(p: Record, entry: Record | null = registryEntry()) { + server.use( + http.get('*/api/admin/plugins', () => HttpResponse.json({ enabled: true, devLink: false, plugins: [p] })), + http.get('*/api/admin/plugins/registry', () => HttpResponse.json(entry ? [entry] : [])), + ) +} + +describe('AdminPluginsPanel — signature badges', () => { + it('FE-COMP-PLUGINS-SIG-001: a registry plugin with a pinned key reads as Signed', async () => { + mockPanel(plugin({ source_repo: 'acme/gotify', signed: true, keyFingerprint: 'AAAAAAAA…BBBBBBBB' })) + render() + expect(await screen.findByText('Signed')).toBeInTheDocument() + expect(screen.queryByText('Unsigned')).not.toBeInTheDocument() + }) + + it('FE-COMP-PLUGINS-SIG-002: a registry plugin with no key reads as Unsigned', async () => { + mockPanel(plugin({ source_repo: 'acme/gotify', signed: false, keyFingerprint: null })) + render() + expect(await screen.findByText('Unsigned')).toBeInTheDocument() + }) + + // The precedence rule. `signed` derives from the pinned key, sideloaded from source_repo + // — so they are NOT mutually exclusive in the data, and a sideloaded plugin genuinely has + // no key. Rendering "Unsigned" NEXT TO "Sideloaded" would double up on a plugin whose + // badge already says something strictly stronger, diluting the amber into wallpaper. + it('FE-COMP-PLUGINS-SIG-003: a sideloaded plugin shows Sideloaded and NO trust badge', async () => { + mockPanel(plugin({ source_repo: 'local:upload', signed: false })) + render() + expect(await screen.findByText('Sideloaded')).toBeInTheDocument() + expect(screen.queryByText('Unsigned')).not.toBeInTheDocument() + expect(screen.queryByText('Signed')).not.toBeInTheDocument() + }) + + it('FE-COMP-PLUGINS-SIG-004: a dev-linked plugin shows Dev-Link and NO trust badge', async () => { + mockPanel(plugin({ source_repo: 'local:link', signed: false })) + render() + expect(await screen.findByText('Dev-Link')).toBeInTheDocument() + expect(screen.queryByText('Unsigned')).not.toBeInTheDocument() + }) +}) + +describe('AdminPluginsPanel — a refused update', () => { + const blocked = (code: string) => + plugin({ + source_repo: 'acme/gotify', signed: true, keyFingerprint: 'OLDKEYaa…aaaaaaaa', + updateBlock: { code, detail: 'the signing key changed', version: '2.0.0' }, + }) + + it('FE-COMP-PLUGINS-SIG-005: the row keeps showing WHY, instead of the reason dying with a toast', async () => { + mockPanel(blocked('SIGNATURE_KEY_CHANGED')) + render() + expect(await screen.findByText(/update blocked/i)).toBeInTheDocument() + }) + + // The block describes the version that was REFUSED. Once the registry offers a newer one, + // it describes an artifact nobody is being offered anymore — so it reads as stale and the + // admin can simply re-attempt. + it('FE-COMP-PLUGINS-SIG-006: the block goes quiet once a NEWER version is on offer', async () => { + mockPanel(blocked('SIGNATURE_KEY_CHANGED'), registryEntry({ latest: '3.0.0' })) + render() + await screen.findByText('Gotify') + await waitFor(() => expect(screen.queryByText(/update blocked/i)).not.toBeInTheDocument()) + }) + + it('FE-COMP-PLUGINS-SIG-007: Review opens the re-trust dialog for a ROTATED key', async () => { + mockPanel(blocked('SIGNATURE_KEY_CHANGED')) + render() + fireEvent.click(await screen.findByRole('button', { name: /review/i })) + + // Both fingerprints, so the admin can compare them against what the author tells them. + expect(await screen.findByText(/key it was installed with/i)).toBeInTheDocument() + expect(screen.getByText(/key it is offering now/i)).toBeInTheDocument() + expect(screen.getByRole('button', { name: /trust the new key/i })).toBeInTheDocument() + }) + + // D2, at the UI. An invalid signature means the bytes are not what the author signed. + // There is no override — not a disabled button, not one behind a confirm. The ABSENCE of + // an escape hatch is the feature. (The server refuses it too; this is belt and braces.) + it('FE-COMP-PLUGINS-SIG-008: an INVALID signature offers NO re-trust affordance at all', async () => { + mockPanel(blocked('SIGNATURE_INVALID')) + render() + fireEvent.click(await screen.findByRole('button', { name: /review/i })) + + await screen.findByText(/do not match the author's signature/i) + expect(screen.queryByRole('button', { name: /trust the new key/i })).not.toBeInTheDocument() + // ...and it does not even show the key comparison, which would imply a choice exists. + expect(screen.queryByText(/key it is offering now/i)).not.toBeInTheDocument() + }) + + it('FE-COMP-PLUGINS-SIG-009: an unsigned-downgrade refusal offers no override either', async () => { + mockPanel(blocked('SIGNATURE_MISSING')) + render() + fireEvent.click(await screen.findByRole('button', { name: /review/i })) + + await screen.findByText(/ships no signature/i) + expect(screen.queryByRole('button', { name: /trust the new key/i })).not.toBeInTheDocument() + }) + + it('FE-COMP-PLUGINS-SIG-010: confirming a re-trust re-pins AND updates in ONE call', async () => { + let body: unknown = null + mockPanel(blocked('SIGNATURE_KEY_CHANGED')) + server.use( + http.post('*/api/admin/plugins/trek-gotify/retrust', async ({ request }) => { + body = await request.json() + return HttpResponse.json({ version: '2.0.0', activated: true, newPermissions: [], newEgress: [] }) + }), + ) + render() + fireEvent.click(await screen.findByRole('button', { name: /review/i })) + fireEvent.click(await screen.findByRole('button', { name: /trust the new key/i })) + + // The FULL key goes back, not the fingerprint: the server's equality check is exact, so + // it can refuse if the entry was re-keyed again since this dialog rendered. + await waitFor(() => + expect(body).toEqual({ version: '2.0.0', publicKey: 'NEWKEYbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' }), + ) + // No follow-up /update: a re-pin that waited for a second call would leave the plugin + // pinned to a key no install had ever verified against if that call never came. + }) +}) + +describe('AdminPluginsPanel — update consent', () => { + it('FE-COMP-PLUGINS-SIG-011: says an unsigned update is untied to its author, and still activates in one click', async () => { + let activated = false + mockPanel(plugin({ source_repo: 'acme/gotify', signed: false }), registryEntry({ signed: false })) + server.use( + http.post('*/api/admin/plugins/trek-gotify/update', () => + HttpResponse.json({ version: '2.0.0', activated: false, newPermissions: ['db:read:trips'], newEgress: [] }), + ), + http.post('*/api/admin/plugins/trek-gotify/activate', () => { activated = true; return HttpResponse.json({ status: 'active' }) }), + ) + render() + await screen.findByText('Gotify') + fireEvent.click(await screen.findByRole('button', { name: /update to|2\.0\.0/i })) + + // Informs — it does not block. No checkbox, no second click. + expect(await screen.findByText(/nothing ties this version to its author/i)).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /approve & turn on/i })) + await waitFor(() => expect(activated).toBe(true)) + }) + + // The warning used to be read ONLY off the registry entry, so an unreachable registry left + // it undefined and the pill silently vanished — at the exact moment the admin was widening + // what unsigned code may do. The installed row carries an authoritative `signed` from the + // server on every list call; degrade to that rather than to silence. + // + // Consent is reached here by turning a plugin back ON after an update widened its + // permissions (409 CONSENT_REQUIRED) — which is the path that still works with the registry + // down, precisely because it needs nothing from the registry. + it('FE-COMP-PLUGINS-SIG-015: the unsigned warning survives an unreachable registry', async () => { + server.use( + http.get('*/api/admin/plugins', () => + HttpResponse.json({ + enabled: true, devLink: false, + plugins: [plugin({ source_repo: 'acme/gotify', signed: false, enabled: 0, status: 'inactive', operatorEgress: false })], + })), + // The registry is down: `regById` stays empty, so the entry's `signed` is unknowable. + http.get('*/api/admin/plugins/registry', () => HttpResponse.json({ error: 'registry unreachable' }, { status: 500 })), + http.post('*/api/admin/plugins/trek-gotify/activate', () => + HttpResponse.json({ error: 'consent required', code: 'CONSENT_REQUIRED', newPermissions: ['db:read:trips'], newEgress: [] }, { status: 409 })), + ) + render() + fireEvent.click(await screen.findByRole('button', { name: /enable plugin/i })) + + // Falls back to the installed row's `signed: false` rather than going quiet. + expect(await screen.findByText(/nothing ties this version to its author/i)).toBeInTheDocument() + }) +}) + +/** + * A signature refusal must reach the dialog even when the plugin has NO installed row — + * which is every fresh install from Discover, and every dependency being downloaded. + * + * Routing the refusal off the installed list meant those two paths silently fell back to a + * generic toast: the admin met SIGNATURE_INVALID for the first time on the one path where the + * dialog explaining it never opened. A fresh install has no pinned key, so it can only ever + * be _INVALID / _INCOMPLETE — never a rotation — and both are non-overridable, so the dialog + * must explain and offer nothing. + */ +describe('AdminPluginsPanel — a refusal with no installed row', () => { + it('FE-COMP-PLUGINS-SIG-013: a fresh install refused for an INVALID signature opens the dialog, not a toast', async () => { + server.use( + http.get('*/api/admin/plugins', () => HttpResponse.json({ enabled: true, devLink: false, plugins: [] })), + http.get('*/api/admin/plugins/registry', () => HttpResponse.json([registryEntry()])), + http.post('*/api/admin/plugins/install', () => + HttpResponse.json({ error: 'author signature verification failed', code: 'SIGNATURE_INVALID' }, { status: 400 })), + ) + render() + fireEvent.click(await screen.findByRole('button', { name: /discover/i })) + fireEvent.click(await screen.findByRole('button', { name: /^install$/i })) + + // The dialog, named after the plugin — which it can only know from the REGISTRY entry, + // there being no installed row to read a name off. + expect(await screen.findByText(/gotify's signature could not be verified/i)).toBeInTheDocument() + await screen.findByText(/do not match the author's signature/i) + // Non-overridable, and no key comparison — showing one would imply a choice exists. + expect(screen.queryByRole('button', { name: /trust the new key/i })).not.toBeInTheDocument() + expect(screen.queryByText(/key it is offering now/i)).not.toBeInTheDocument() + }) + + it('FE-COMP-PLUGINS-SIG-014: a refusal while downloading a DEPENDENCY opens the dialog too', async () => { + const parent = plugin({ id: 'trek-parent', name: 'Parent', source_repo: 'acme/parent', enabled: 0, status: 'inactive', operatorEgress: false }) + server.use( + http.get('*/api/admin/plugins', () => HttpResponse.json({ enabled: true, devLink: false, plugins: [parent] })), + http.get('*/api/admin/plugins/registry', () => HttpResponse.json([registryEntry()])), + // Turning it on reveals the missing dependency… + http.post('*/api/admin/plugins/trek-parent/activate', () => + HttpResponse.json({ error: 'missing dependency', code: 'DEPENDENCY_MISSING', missing: [{ id: 'trek-gotify', version: '^1.0.0' }], versionMismatch: [] }, { status: 409 })), + // …and downloading it is refused on its signature. + http.post('*/api/admin/plugins/install', () => + HttpResponse.json({ error: 'author signature verification failed', code: 'SIGNATURE_INVALID' }, { status: 400 })), + ) + render() + fireEvent.click(await screen.findByRole('button', { name: /enable plugin/i })) + fireEvent.click(await screen.findByRole('button', { name: /download/i })) + + // Named after the DEPENDENCY, not the parent — it is the dependency's author whose + // signature did not verify, and saying "Parent" here would point the admin at the wrong + // plugin entirely. + expect(await screen.findByText(/gotify's signature could not be verified/i)).toBeInTheDocument() + expect(screen.queryByRole('button', { name: /trust the new key/i })).not.toBeInTheDocument() + }) +}) + +describe('AdminPluginsPanel — a block never outlives the registry relationship', () => { + // The server clears the block on sideload/dev-link. This is the belt: even if a stale + // block somehow reached the client, a plugin whose code the admin supplied by hand must + // never claim an update was blocked over an author signing key. + it('FE-COMP-PLUGINS-SIG-012: a sideloaded plugin never shows an update block', async () => { + mockPanel(plugin({ + source_repo: 'local:upload', signed: false, + updateBlock: { code: 'SIGNATURE_KEY_CHANGED', detail: 'the signing key changed', version: '2.0.0' }, + })) + render() + await screen.findByText('Sideloaded') + expect(screen.queryByText(/update blocked/i)).not.toBeInTheDocument() + }) +}) + +/** + * TREK-version compatibility. The SERVER owns the semver — a second implementation in the + * browser would eventually disagree with the install gate and offer a button that 400s — + * so the panel only renders the verdict the API hands it (`compatible`, `latestCompatible`). + */ +describe('AdminPluginsPanel — TREK-version compatibility', () => { + /** Discover cards for a plugin that is NOT installed — an installed one just reads "Installed". */ + async function openDiscover(entry: Record) { + mockPanel(plugin({ id: 'something-else' }), registryEntry(entry)) + render() + fireEvent.click(await screen.findByText('Discover')) + } + + it('blocks Install when no published version runs on this TREK, and says why', async () => { + await openDiscover({ trek: '>=4.0.0', hostVersion: '3.3.0', compatible: false, latestCompatible: null }) + const btn = await screen.findByRole('button', { name: /^incompatible$/i }) + expect(btn).toBeDisabled() + }) + + it('offers the newest version that DOES run here rather than a dead button', async () => { + await openDiscover({ latest: '2.0.0', trek: '>=3.4.0', hostVersion: '3.3.0', compatible: false, latestCompatible: '1.5.0' }) + const btn = await screen.findByRole('button', { name: /^install 1\.5\.0$/i }) + expect(btn).toBeEnabled() + }) + + it('installs normally when the latest version fits', async () => { + await openDiscover({ trek: '>=3.2.0 <4.0.0', hostVersion: '3.3.0', compatible: true, latestCompatible: '2.0.0' }) + expect(await screen.findByRole('button', { name: /^install$/i })).toBeEnabled() + }) + + it('an installed plugin the server has outgrown shows the blocker on its card', async () => { + // Same amber chip machinery as a disabled addon / missing dependency — the admin sees + // one "here is why this cannot turn on" surface, not a new concept per blocker. + mockPanel(plugin({ + dependencyStatus: 'hostIncompatible', trekRange: '>=3.2.0 <4.0.0', hostVersion: '4.0.0', enabled: 0, status: 'inactive', + })) + render() + expect(await screen.findByText(/needs trek >=3\.2\.0 <4\.0\.0/i)).toBeInTheDocument() + }) +}) diff --git a/client/src/components/Admin/AdminPluginsPanel.tsx b/client/src/components/Admin/AdminPluginsPanel.tsx index 00d8b4af9..966b215b7 100644 --- a/client/src/components/Admin/AdminPluginsPanel.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.tsx @@ -1,7 +1,8 @@ -import { useEffect, useMemo, useRef, useState, type DragEvent } from 'react' +import { useEffect, useMemo, useRef, useState, type CSSProperties, type DragEvent } from 'react' +import { createPortal } from 'react-dom' import { Blocks, AlertTriangle, PackageOpen, RefreshCw, Trash2, Download, Bug, X, ShieldCheck, UploadCloud, - ArrowUpCircle, Github, ExternalLink, ChevronDown, Check, Lock, Search, Link2, + ArrowUpCircle, Github, ExternalLink, ChevronDown, Check, Lock, Search, Link2, KeyRound, ShieldAlert, SlidersHorizontal, ArrowUpDown, CircleDot, MoreHorizontal, RotateCw, ArrowRight, Database, Users, LayoutDashboard, Radio, Luggage, Plane, Globe, Image, CalendarDays, Map, Bell, Cloud, Camera, Compass, BookOpen, Wallet, Puzzle, MapPin, ListChecks, Pencil, Tag, FileText, @@ -24,7 +25,7 @@ import ToggleSwitch from '../Settings/ToggleSwitch' interface PluginDep { id: string; version: string } interface VersionMismatch { id: string; wanted: string; installed: string } -type DependencyStatus = 'ok' | 'addonDisabled' | 'missingPlugin' +type DependencyStatus = 'ok' | 'addonDisabled' | 'missingPlugin' | 'hostIncompatible' interface PluginDependencies { requiredAddons: string[]; pluginDependencies: PluginDep[] } interface DependencyIssues { disabledAddons: string[]; missing: PluginDep[]; versionMismatch: VersionMismatch[] } @@ -49,6 +50,17 @@ interface PluginRow { dependencies?: PluginDependencies dependencyStatus?: DependencyStatus dependencyIssues?: DependencyIssues + /** The TREK versions the plugin says it supports; null if it never declared any. */ + trekRange?: string | null + /** The TREK this server is running — the server does the semver, the client just shows it. */ + hostVersion?: string + /** The author's signature was verified and their key pinned at install. False means the + * bytes matched the registry's sha256 and nothing more — one fewer guarantee. */ + signed?: boolean + /** Short form of the pinned key, for eyeballing against what the author reads out. */ + keyFingerprint?: string | null + /** Why an update was refused, if one was. `version` is the version that was refused. */ + updateBlock?: { code: string; detail: string | null; version: string | null } | null } interface RegistryItem { id: string @@ -60,11 +72,22 @@ interface RegistryItem { type: string latest: string | null minTrekVersion: string | null + /** The latest version's declared TREK range; null on a legacy registry entry. */ + trek?: string | null + hostVersion?: string + /** Whether the LATEST version can be installed on this TREK. Computed server-side. */ + compatible?: boolean + /** Newest installable version — the latest, an older fallback, or null if none fits. */ + latestCompatible?: string | null reviewedAt: string | null downloadCount?: number | null screenshotUrl: string | null requiredAddons?: string[] pluginDependencies?: PluginDep[] + /** The latest version ships an author signature and the entry declares a key. */ + signed?: boolean + /** The full key — public, and carried in full because re-trust compares it exactly. */ + authorPublicKey?: string | null } interface RegistryDetail extends RegistryItem { size: number | null @@ -90,6 +113,49 @@ interface ActivateErr { } } +/** The server's error envelope. Reading `code` — not the message text — is what lets the + * UI tell a rotated key (overridable) from a signature that doesn't verify (never). */ +function errBody(e: unknown): { error?: string; code?: string } { + return (e as { response?: { data?: { error?: string; code?: string } } })?.response?.data ?? {} +} + +/** The ONE signature refusal an admin may override, because a rotation has a benign + * explanation. SIGNATURE_INVALID / _MISSING / _INCOMPLETE mean the bytes are not what the + * author signed — those get an explanation and no override button at all. */ +const RETRUSTABLE = 'SIGNATURE_KEY_CHANGED' +const SIGNATURE_CODES = [RETRUSTABLE, 'SIGNATURE_INVALID', 'SIGNATURE_MISSING', 'SIGNATURE_INCOMPLETE'] + +/** + * What the signature dialog needs to talk about a plugin — deliberately NOT a PluginRow. + * + * A refusal can land on a plugin that is not installed (a fresh install from Discover, or a + * dependency being downloaded), and those have no row. Keying the dialog off PluginRow meant + * the lookup missed and the refusal silently degraded to a toast — on the very path where an + * admin most often meets these codes for the first time. Name + pinned fingerprint is all the + * dialog ever reads, and both a row and a registry entry can supply that. + */ +type SigSubject = { id: string; name: string; keyFingerprint: string | null } + +/** + * Is a recorded update block still describing the version on offer? + * + * Once the registry offers something NEWER than the version that was refused, the block + * describes an artifact nobody is being offered anymore — so it reads as stale and the + * admin can simply re-attempt (the next install re-verifies and either succeeds or + * re-blocks with fresh values). When the registry is unreachable we can't prove staleness, + * so the block stands: silently dropping the last thing we knew would be the worse failure. + */ +function blockIsCurrent(p: PluginRow, latestVer?: string): boolean { + if (!p.updateBlock) return false + // A block describes a refused REGISTRY update. Once a plugin is sideloaded or dev-linked + // it has left the registry trust model — the running code is whatever the admin supplied, + // and a block about an author signing key says nothing about it. The server clears the + // block on both paths; this makes it impossible to render a stale one regardless. + if (!isRegistrySourced(p.source_repo)) return false + if (!latestVer || !p.updateBlock.version) return true + return latestVer === p.updateBlock.version +} + type T = (k: string, p?: Record) => string type TypeFilter = 'all' | 'widget' | 'page' | 'integration' | 'trip-page' type StatusFilter = 'all' | 'on' | 'off' | 'update' | 'err' @@ -186,10 +252,21 @@ function deriveCaps(perms: string[], caps: { widget?: { slot?: string }; tripPag interface DepChip { icon: React.ComponentType<{ size?: number; className?: string }>; label: string; blocked: boolean } // A plugin's declared dependencies as chips — a required addon (amber when that -// addon is disabled) or a plugin dependency (amber when missing / version-mismatched). +// addon is disabled), a plugin dependency (amber when missing / version-mismatched), +// or the TREK version itself (amber when this server has outgrown the plugin's range, +// which is the one blocker the admin cannot fix by flipping something else on). function deriveDeps(p: PluginRow, t: T): DepChip[] { const out: DepChip[] = [] const issues = p.dependencyIssues + if (p.dependencyStatus === 'hostIncompatible') { + out.push({ + icon: AlertTriangle, + label: p.trekRange + ? t('admin.plugins.dep.trekIncompatible', { range: p.trekRange, host: p.hostVersion ?? '?' }) + : t('admin.plugins.dep.trekUnknown'), + blocked: true, + }) + } for (const a of p.dependencies?.requiredAddons ?? []) { out.push({ icon: Blocks, label: t('admin.plugins.cap.requiresAddon', { addon: a }), blocked: !!issues?.disabledAddons.includes(a) }) } @@ -200,6 +277,26 @@ function deriveDeps(p: PluginRow, t: T): DepChip[] { return out } +/** + * What the Install button may do for a registry item. + * + * The server already decided compatibility — it owns the semver, and a second + * implementation here would eventually disagree with the install gate and offer a button + * that 400s. This only picks the wording. Note the middle case: when the newest release + * has outrun this TREK but an older one still fits, offer THAT version rather than a dead + * grey button. The plugin is perfectly usable, just not at its newest. + */ +function installOffer(item: RegistryItem, t: T): { blocked: boolean; version?: string; label: string; title?: string } { + if (item.compatible !== false) return { blocked: false, label: t('admin.plugins.install') } + const title = item.trek + ? t('admin.plugins.dep.trekIncompatible', { range: item.trek, host: item.hostVersion ?? '?' }) + : t('admin.plugins.dep.trekUnknown') + if (item.latestCompatible) { + return { blocked: false, version: item.latestCompatible, label: t('admin.plugins.installCompatible', { version: item.latestCompatible }), title } + } + return { blocked: true, label: t('admin.plugins.incompatible'), title } +} + function PluginIcon({ name, size = 20, className }: { name: string | null; size?: number; className?: string }) { const Icon = (name && Object.prototype.hasOwnProperty.call(ICON_MAP, name) && ICON_MAP[name]) || Blocks return @@ -224,6 +321,46 @@ function SideloadedBadge({ t }: { t: T }) { ) } +/** + * Whether a plugin came from the REGISTRY (as opposed to a manual upload or a dev-link). + * + * This is the precedence rule for the trust badges, and it is load-bearing: `signed` + * derives from the pinned author key while sideloaded/dev-linked derive from source_repo, + * so the states are NOT mutually exclusive in the data. A sideloaded plugin genuinely has + * no pinned key, and a naive render would put "Unsigned" *and* "Sideloaded" side by side. + * The source badge wins — it already says something strictly stronger, and doubling up + * dilutes the amber into wallpaper, which is exactly what makes a warning worthless. + */ +function isRegistrySourced(sourceRepo: string | null | undefined): boolean { + return !!sourceRepo && sourceRepo !== 'local:upload' && sourceRepo !== 'local:link' +} + +/** + * Signed / Unsigned, for registry plugins only (see isRegistrySourced). + * + * Signed is a quiet neutral tick, NOT a green celebration; unsigned is an amber note, NOT + * a red alarm. Roughly two thirds of the live registry is unsigned, so an alarming + * treatment would fire on most of the catalog and teach admins to ignore it — and the + * honest delta is small: sha256 proves the bytes are what the REGISTRY vouches for, a + * signature proves they came from the AUTHOR. Unsigned is one fewer guarantee, not "unsafe". + */ +function TrustBadge({ signed, t }: { signed: boolean; t: T }) { + if (signed) { + return ( + + {t('admin.plugins.signed')} + + ) + } + return ( + + {t('admin.plugins.unsigned')} + + ) +} + /** Marks a dev-linked plugin: loaded from a local build dir + hot-reloaded (dev only). */ function DevLinkBadge({ t }: { t: T }) { return ( @@ -255,6 +392,13 @@ export default function AdminPluginsPanel() { const [view, setView] = useState<'installed' | 'discover'>('installed') const [registry, setRegistry] = useState(null) const [latest, setLatest] = useState>({}) + // The registry entry per id — the re-trust dialog needs the NEW author key from it (in + // full: the server's equality check is exact), and the Discover/consent copy needs `signed`. + const [regById, setRegById] = useState>({}) + // Open when a signature check refused an install/update. Carries the code, so the dialog + // knows whether an override may even be offered. + const [signatureBlock, setSignatureBlock] = useState<{ subject: SigSubject; code: string; detail: string | null } | null>(null) + const [retrusting, setRetrusting] = useState(false) const [detailFor, setDetailFor] = useState(null) const [errorsFor, setErrorsFor] = useState<{ id: string; rows: Array<{ ts: string; level: string; message: string }> } | null>(null) const [egressFor, setEgressFor] = useState<{ id: string; supported: boolean; hosts: string[] } | null>(null) @@ -288,6 +432,16 @@ export default function AdminPluginsPanel() { const uploadInputRef = useRef(null) const dragDepth = useRef(0) + // Index the registry once per fetch: the version map the update badges read, plus the + // whole entry (author key + signed flag) the trust badges and re-trust dialog need. + const indexRegistry = (items: RegistryItem[]) => { + const vers: Record = {} + const byId: Record = {} + items.forEach((i) => { byId[i.id] = i; if (i.latest) vers[i.id] = i.latest }) + setLatest(vers) + setRegById(byId) + } + const refresh = () => { // Keep the app-wide active-plugin store in sync so widget/hero/tab consumers // (e.g. the dashboard) reflect an activate/deactivate without a full reload (F5). @@ -298,13 +452,7 @@ export default function AdminPluginsPanel() { setDevLink(!!d.devLink) setPlugins(d.plugins || []) if ((d.plugins || []).length) { - adminApi.pluginBrowse() - .then((items: RegistryItem[]) => { - const map: Record = {} - items.forEach((i) => { if (i.latest) map[i.id] = i.latest }) - setLatest(map) - }) - .catch(() => {}) + adminApi.pluginBrowse().then(indexRegistry).catch(() => {}) } }) .catch(() => setError(true)) @@ -312,16 +460,43 @@ export default function AdminPluginsPanel() { } useEffect(refresh, []) + // A signature refusal is not an ordinary "action failed": it is the one class of error + // where WHICH failure it was decides what the admin may do about it. Route it to the + // dialog off the CODE, never off the message text. Returns true when handled. + // + // The subject is resolved from wherever it can be — the installed row, else the registry + // entry (a plugin being INSTALLED has no row yet), else the bare id. A signature code + // ALWAYS opens the dialog: falling back to a toast is what used to hide an invalid + // signature on a fresh install behind the same treatment as a network blip. + const routeSignatureError = (id: string, code?: string, detail?: string): boolean => { + if (!code || !SIGNATURE_CODES.includes(code)) return false + const row = plugins.find(p => p.id === id) + const subject: SigSubject = row + ? { id, name: row.name, keyFingerprint: row.keyFingerprint ?? null } + // Not installed: no pinned key exists, so SIGNATURE_KEY_CHANGED cannot arise and the + // dialog shows the no-override explanation — which is exactly right. + : { id, name: regById[id]?.name ?? id, keyFingerprint: null } + setSignatureBlock({ subject, code, detail: detail ?? null }) + return true + } + const act = async (id: string, fn: () => Promise, ok: string) => { setBusy(id); setMenu(null) try { await fn(); toast.success(ok) } - catch (e) { toast.error((e as { response?: { data?: { error?: string } } })?.response?.data?.error || t('admin.plugins.actionError')) } + catch (e) { + const { error, code } = errBody(e) + if (!routeSignatureError(id, code, error)) toast.error(error || t('admin.plugins.actionError')) + } finally { setBusy(null); refresh() } } const openDiscover = () => { setView('discover') - if (!registry) adminApi.pluginBrowse().then(setRegistry).catch(() => setRegistry([])) + if (!registry) { + adminApi.pluginBrowse() + .then((items: RegistryItem[]) => { setRegistry(items); indexRegistry(items) }) + .catch(() => setRegistry([])) + } } // The rescan/reload button rediscovers locally-installed plugins AND force-pulls // the remote registry (bypassing the 30-min server cache + GitHub's CDN), so a @@ -330,9 +505,7 @@ export default function AdminPluginsPanel() { await adminApi.pluginRescan() const items: RegistryItem[] = await adminApi.pluginBrowse(true) setRegistry(items) - const map: Record = {} - items.forEach((i) => { if (i.latest) map[i.id] = i.latest }) - setLatest(map) + indexRegistry(items) }, t('admin.plugins.rescanned')) // Sideload a plugin archive (installs INACTIVE — the admin still consents on activation). @@ -393,7 +566,7 @@ export default function AdminPluginsPanel() { } const updateAvailable = (p: PluginRow) => !!(p.version && latest[p.id] && isNewer(latest[p.id], p.version)) - const install = (id: string) => act(id, () => adminApi.pluginInstall(id), t('admin.plugins.installed')) + const install = (id: string, version?: string) => act(id, () => adminApi.pluginInstall(id, version ? { version } : undefined), t('admin.plugins.installed')) const restart = (id: string) => act(id, async () => { await adminApi.pluginDeactivate(id); await adminApi.pluginActivate(id) }, t('admin.plugins.restarted')) // Dev-link: register a plugin from a local built directory (dev only). Reuses the // same busy/toast/refresh loop as uploadPlugin; the server gates it. @@ -470,7 +643,13 @@ export default function AdminPluginsPanel() { if (r?.requiredAddons?.length) toast.error(t('admin.plugins.dep.addonDisabledToast', { addons: r.requiredAddons.join(', ') })) return attemptActivate(parent) }) - .catch((e: ActivateErr) => onActivateError(parent, e)) + // The DEPENDENCY is what's being downloaded, so a signature refusal here is about the + // dependency's author, not the parent's — route it under depId before falling through + // to the activation-error handling, which knows nothing about signature codes. + .catch((e: ActivateErr) => { + const { error, code } = errBody(e) + if (!routeSignatureError(depId, code, error)) onActivateError(parent, e) + }) .finally(() => { setBusy(null); refresh() }) } @@ -481,10 +660,33 @@ export default function AdminPluginsPanel() { if (r.activated || (r.newPermissions.length === 0 && r.newEgress.length === 0)) toast.success(t('admin.plugins.updated')) else setConsentQueue(qq => [...qq, { plugin: p, version: r.version, newPermissions: r.newPermissions, newEgress: r.newEgress }]) }) - .catch(e => toast.error((e as { response?: { data?: { error?: string } } })?.response?.data?.error || t('admin.plugins.actionError'))) + .catch(e => { + const { error, code } = errBody(e) + if (!routeSignatureError(p.id, code, error)) toast.error(error || t('admin.plugins.actionError')) + }) .finally(() => { setBusy(null); refresh() }) } + // Confirm a key rotation: re-pin the new key AND update, in one server call. There is no + // follow-up /update — a re-pin that waited for a second call would leave the plugin + // pinned to a key no install had ever been verified against if that call never came. + const confirmRetrust = (id: string, version: string, publicKey: string) => { + setRetrusting(true) + adminApi.pluginRetrust(id, version, publicKey) + .then((r: { version: string; activated: boolean; newPermissions: string[]; newEgress: string[] }) => { + setSignatureBlock(null) + // A re-trusted update widening permissions still needs consent — re-trusting a + // signing key says nothing about what the new code is allowed to do. Re-trust only + // ever fires for an INSTALLED plugin, so the row is there to consent against. + const p = plugins.find(x => x.id === id) + if (p && !r.activated && (r.newPermissions.length > 0 || r.newEgress.length > 0)) { + setConsentQueue(qq => [...qq, { plugin: p, version: r.version, newPermissions: r.newPermissions, newEgress: r.newEgress }]) + } else toast.success(t('admin.plugins.retrusted')) + }) + .catch(e => toast.error(errBody(e).error || t('admin.plugins.actionError'))) + .finally(() => { setRetrusting(false); refresh() }) + } + // eslint-disable-next-line react-hooks/exhaustive-deps const updatable = useMemo(() => plugins.filter(updateAvailable), [plugins, latest]) @@ -691,8 +893,13 @@ export default function AdminPluginsPanel() { ) : shownInstalled.map(p => ( toggle(p)} onUpdate={() => runUpdate(p)} onRestart={() => restart(p.id)} + onReviewBlock={() => setSignatureBlock({ + subject: { id: p.id, name: p.name, keyFingerprint: p.keyFingerprint ?? null }, + code: p.updateBlock!.code, detail: p.updateBlock!.detail, + })} onErrors={() => openErrors(p.id)} onEgress={() => openEgress(p.id)} onUninstall={() => { setMenu(null); setConfirmUninstall(p) }} /> ))} @@ -791,9 +998,26 @@ export default function AdminPluginsPanel() { message={t('admin.plugins.uninstallBody')} /> + {signatureBlock && ( + confirmRetrust(signatureBlock.subject.id, version, publicKey)} + onClose={() => setSignatureBlock(null)} + /> + )} + {consentQueue[0] && ( { const c = consentQueue[0]; setConsentQueue(qq => qq.slice(1)) // consent:true — the ONLY path that may widen a plugin's granted rights. @@ -886,14 +1110,33 @@ function EmptyState({ t, onDiscover }: { t: T; onDiscover: () => void }) { ) } -function InstalledRow({ p, t, busy, menu, setMenu, hasUpdate, latestVer, onToggle, onUpdate, onRestart, onErrors, onEgress, onUninstall }: { +function InstalledRow({ p, t, busy, menu, setMenu, hasUpdate, latestVer, blocked, onToggle, onUpdate, onRestart, onReviewBlock, onErrors, onEgress, onUninstall }: { p: PluginRow; t: T; busy: string | null; menu: string | null; setMenu: (v: string | null) => void - hasUpdate: boolean; latestVer?: string - onToggle: () => void; onUpdate: () => void; onRestart: () => void; onErrors: () => void; onEgress: () => void; onUninstall: () => void + hasUpdate: boolean; latestVer?: string; blocked: boolean + onToggle: () => void; onUpdate: () => void; onRestart: () => void; onReviewBlock: () => void + onErrors: () => void; onEgress: () => void; onUninstall: () => void }) { const caps = deriveCaps(parseJson(p.permissions, []), parseJson<{ widget?: { slot?: string } }>(p.capabilities, {}), t) const deps = deriveDeps(p, t) const menuOpen = menu === `row:${p.id}` + const menuBtnRef = useRef(null) + + // The menu is portaled to and positioned `fixed` against the ⋯ button, + // because the row's ancestor (PageSidebar) is `overflow-hidden` and would clip + // an in-flow `absolute` menu — losing Delete on the lower rows of a long list. + // Re-anchor while the page moves under it. + const [, reanchor] = useState(0) + useEffect(() => { + if (!menuOpen) return + const onMove = () => reanchor(n => n + 1) + window.addEventListener('scroll', onMove, true) + window.addEventListener('resize', onMove) + return () => { + window.removeEventListener('scroll', onMove, true) + window.removeEventListener('resize', onMove) + } + }, [menuOpen]) + return (
@@ -911,8 +1154,24 @@ function InstalledRow({ p, t, busy, menu, setMenu, hasUpdate, latestVer, onToggl {p.reviewed_at && } {p.source_repo === 'local:upload' && } {p.source_repo === 'local:link' && } + {/* Registry plugins only — a sideloaded/dev-linked plugin already says something + strictly stronger, and stacking "Unsigned" on top of it just dilutes the amber. */} + {isRegistrySourced(p.source_repo) && }
{p.description &&

{p.description}

} + {/* A refused update leaves a WORKING plugin pinned at its old version — so this is + its own row state, not an error state, and it persists until it is resolved + rather than dying with the toast that first reported it. */} + {blocked && p.updateBlock && ( +
+ + {t('admin.plugins.updateBlocked', { reason: p.updateBlock.detail ?? p.updateBlock.code })} + +
+ )} {p.status === 'error' && p.last_error ? (
{p.last_error} @@ -970,12 +1229,13 @@ function InstalledRow({ p, t, busy, menu, setMenu, hasUpdate, latestVer, onToggl
- - {menuOpen && ( -
+ {menuOpen && createPortal( +
{p.enabled === 1 && ( } label={t('admin.plugins.restart')} onClick={onRestart} /> )} @@ -995,7 +1255,8 @@ function InstalledRow({ p, t, busy, menu, setMenu, hasUpdate, latestVer, onToggl )}
} label={t('common.delete')} danger onClick={onUninstall} /> -
+
, + document.body, )}
@@ -1003,6 +1264,23 @@ function InstalledRow({ p, t, busy, menu, setMenu, hasUpdate, latestVer, onToggl ) } +/** Tallest the ⋯ menu gets (6 items + divider + padding). Drives the flip. */ +const ROW_MENU_MAX_H = 260 + +/** Pin a portaled menu to the right edge of its trigger, flipping up when the bottom is tight. */ +function anchorMenu(trigger: HTMLElement | null): CSSProperties { + const r = trigger?.getBoundingClientRect() + if (!r) return { position: 'fixed', top: 0, right: 0, zIndex: 100 } + const spaceBelow = window.innerHeight - r.bottom + const openUp = spaceBelow < ROW_MENU_MAX_H && r.top > spaceBelow + return { + position: 'fixed', + right: Math.max(8, window.innerWidth - r.right), + ...(openUp ? { bottom: window.innerHeight - r.top + 4 } : { top: r.bottom + 4 }), + zIndex: 100, + } +} + function MenuItem({ icon, label, onClick, danger }: { icon: React.ReactNode; label: string; onClick: () => void; danger?: boolean }) { return (
@@ -1106,7 +1389,7 @@ function PermLabel({ perm, t }: { perm: string; t: T }) { function PluginDetailModal({ item, installed, busy, onInstall, onClose, t, locale }: { item: RegistryItem; installed: boolean; busy: string | null - onInstall: (id: string) => void; onClose: () => void; t: T; locale: string + onInstall: (id: string, version?: string) => void; onClose: () => void; t: T; locale: string }) { const [detail, setDetail] = useState(null) const [failed, setFailed] = useState(false) @@ -1124,6 +1407,9 @@ function PluginDetailModal({ item, installed, busy, onInstall, onClose, t, local const repoUrl = `https://github.com/${item.repo}` const homepage = item.homepage && /^https?:\/\//i.test(item.homepage) && item.homepage !== repoUrl ? item.homepage : null const sizeKb = detail?.size ? Math.max(1, Math.round(detail.size / 1024)) : null + // The detail fetch carries the same compat verdict as the browse list; prefer it once + // it lands (it is keyed to the same entry) and fall back to the grid item until then. + const offer = installOffer(detail ?? item, t) return (
@@ -1145,15 +1431,24 @@ function PluginDetailModal({ item, installed, busy, onInstall, onClose, t, local

{item.author}{item.latest ? ` · v${item.latest}` : ''}

-

{item.description}

{failed &&

{t('admin.plugins.detailError')}

} + {/* The reason the button is blocked (or offering an older version) — a tooltip + alone would leave a touch user with a dead button and no explanation. */} + {offer.title && !installed && ( +

+ {offer.title} +

+ )} {manifest && (
@@ -1220,7 +1515,11 @@ function PluginDetailModal({ item, installed, busy, onInstall, onClose, t, local
{item.latest && } {sizeKb && } - {item.minTrekVersion && } + {/* The range, not just its lower bound: "TREK 3.2.0+" reads as "and anything + newer", which is exactly the claim a `<4.0.0` upper bound denies. */} + {(item.trek || item.minTrekVersion) && ( + + )} {item.reviewedAt && } {typeof item.downloadCount === 'number' && item.downloadCount > 0 && ( @@ -1254,8 +1553,101 @@ function Meta({ k, v }: { k: string; v: string }) { return
{k}
{v}
} -function UpdateConsentDialog({ data, t, onApprove, onLater }: { +/** + * A signature check refused an install/update. + * + * The override lives here, and it is scoped to exactly one code. A rotated key has a + * benign explanation — the author rotated it, or lost it and made a new one. A signature + * that does NOT verify does not: it means the bytes are not what the author signed, which + * is corruption or an attack, and there is no story where the right answer is "let the + * admin wave it through". So SIGNATURE_INVALID / _MISSING / _INCOMPLETE get a clear + * explanation and no override button AT ALL — not a disabled one, not one behind a + * confirm. The absence of an escape hatch is the feature. + * + * (The server enforces this too — it re-derives the condition and refuses anything but a + * changed key. This dialog is the convenience, not the control.) + */ +function SignatureBlockDialog({ data, entry, busy, t, onRetrust, onClose }: { + data: { subject: SigSubject; code: string; detail: string | null } + entry: RegistryItem | undefined + busy: boolean; t: T + onRetrust: (version: string, publicKey: string) => void + onClose: () => void +}) { + const canRetrust = data.code === RETRUSTABLE + const newKey = entry?.authorPublicKey ?? null + const version = entry?.latest ?? null + // Only offer the override when we actually hold the new key + version to send: the + // request carries both, and the server compares the key exactly. + const offerRetrust = canRetrust && !!newKey && !!version + + const bodyKey = data.code === RETRUSTABLE ? 'admin.plugins.sig.keyChangedBody' + : data.code === 'SIGNATURE_MISSING' ? 'admin.plugins.sig.missingBody' + : data.code === 'SIGNATURE_INCOMPLETE' ? 'admin.plugins.sig.incompleteBody' + : 'admin.plugins.sig.invalidBody' + + return ( +
+
e.stopPropagation()}> +
+
+
+

{t('admin.plugins.sig.title', { name: data.subject.name })}

+

{t(bodyKey as never)}

+
+
+
+ {/* Fingerprints, not full keys: these exist to be COMPARED by a human — read the + new one back to the author over the phone. The full key travels in the request. */} + {canRetrust && ( +
+ + +

{t('admin.plugins.sig.confirmOutOfBand')}

+
+ )} + {!canRetrust && data.detail && ( +

{data.detail}

+ )} +
+
+ + {offerRetrust && ( + + )} +
+
+
+ ) +} + +function KeyRow({ label, value, highlight }: { label: string; value: string; highlight?: boolean }) { + return ( +
+ {label} + {value} +
+ ) +} + +/** Client-side twin of the server's fingerprint: head…tail of the base64 payload. Display + * only — every equality check that matters happens server-side against the full key. */ +function fingerprint(key: string | null): string | null { + if (!key) return null + const payload = key.trim().split(/\r?\n/).map(l => l.trim()).filter(l => l && !l.startsWith('untrusted comment')).pop() + if (!payload) return null + return payload.length <= 20 ? payload : `${payload.slice(0, 8)}…${payload.slice(-8)}` +} + +function UpdateConsentDialog({ data, unsigned, t, onApprove, onLater }: { data: { plugin: PluginRow; version: string; newPermissions: string[]; newEgress: string[] } + unsigned: boolean t: T; onApprove: () => void; onLater: () => void }) { return ( @@ -1269,6 +1661,15 @@ function UpdateConsentDialog({ data, t, onApprove, onLater }: {
+ {/* The admin is about to widen what this code may do — so say, right here, that + nothing ties this code to its author. One line, no checkbox, no extra click: + this informs, it does not block. */} + {unsigned && ( +

+ + {t('admin.plugins.sig.consentUnsigned')} +

+ )} {data.newPermissions.length > 0 && (

{t('admin.plugins.updateNewPermissions')}

@@ -1358,6 +1759,7 @@ function SecurityInfo({ t }: { t: T }) { ['admin.plugins.security.limitsTitle', 'admin.plugins.security.limitsBody'], ['admin.plugins.security.worstTitle', 'admin.plugins.security.worstBody'], ['admin.plugins.security.reviewedTitle', 'admin.plugins.security.reviewedBody'], + ['admin.plugins.security.signedTitle', 'admin.plugins.security.signedBody'], ['admin.plugins.security.trustTitle', 'admin.plugins.security.trustBody'], ] return ( diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index 634b0384f..5ddf8da9a 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -222,6 +222,54 @@ describe('CostsPanel — settlements in the ledger', () => { expect(posted!.payers).toEqual([]) }) + it('keeps "no one paid yet" when reopening a payer-less expense (#1533)', async () => { + seedStore(useAuthStore, { user: buildUser({ id: 1, username: 'alice' }), isAuthenticated: true }) + let put: Record | null = null + const item = { + ...buildBudgetItem({ trip_id: 1, category: 'food', name: 'Hotel' }), + id: 5, + total_price: 120, + payers: [], + members: [{ user_id: 1, username: 'alice', paid: 0 }, { user_id: 2, username: 'bob', paid: 0 }], + } + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [item] })), + http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ balances: [], flows: [], settlements: [] })), + http.put('/api/trips/1/budget/5', async ({ request }) => { + put = await request.json() as Record + return HttpResponse.json({ item }) + }), + ) + const { default: userEvent } = await import('@testing-library/user-event') + const user = userEvent.setup() + render() + + await screen.findByText('Hotel') + await user.click(screen.getByTitle('Edit')) + + // Nobody paid this expense — reopening it must not silently reselect "You". + expect(await screen.findByRole('button', { name: 'No one paid yet' })).toBeInTheDocument() + + // …and saving an untouched edit must not assign the current user as payer. + await user.click(screen.getByRole('button', { name: 'Save' })) + await waitFor(() => expect(put).toBeTruthy()) + expect(put!.payers).toEqual([]) + }) + + it('still defaults a brand-new expense to "You" as the payer', async () => { + seedStore(useAuthStore, { user: buildUser({ id: 1, username: 'alice' }), isAuthenticated: true }) + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [] })), + http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ balances: [], flows: [], settlements: [] })), + ) + const { default: userEvent } = await import('@testing-library/user-event') + const user = userEvent.setup() + render() + + await user.click(await screen.findByRole('button', { name: 'Add expense' })) + expect(await screen.findByRole('button', { name: 'You' })).toBeInTheDocument() + }) + it('exports the expenses as a CSV download (#1500)', async () => { // Display in the trip's own currency so FX conversion is an identity. seedStore(useSettingsStore, { settings: { ...useSettingsStore.getState().settings, default_currency: 'EUR' } }) diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index 08caee73a..4b83c8f1d 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -989,10 +989,12 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo const [participants, setParticipants] = useState>(() => editing ? new Set((editing.members || []).map(m => m.user_id)) : new Set(people.map(p => p.id))) - // Payer state: 0 represents "Nobody (planning entry)" + // Payer state: 0 represents "Nobody (planning entry)". On an existing expense a + // missing payer is a deliberate choice, so only a brand-new one defaults to me. const [payerId, setPayerId] = useState(() => { const existingPayer = (editing?.payers || []).find(p => p.amount > 0) - return existingPayer ? existingPayer.user_id : me + if (existingPayer) return existingPayer.user_id + return editing ? 0 : me }) const [splitMode, setSplitMode] = useState<'equally' | 'custom' | 'ticket'>(() => { diff --git a/client/src/components/PDF/TripPDF.test.ts b/client/src/components/PDF/TripPDF.test.ts index 68c3805bc..8b41770b3 100644 --- a/client/src/components/PDF/TripPDF.test.ts +++ b/client/src/components/PDF/TripPDF.test.ts @@ -170,6 +170,18 @@ describe('downloadTripPDF', () => { expect(srcdoc).toContain('Colosseum') }) + it('FE-COMP-TRIPPDF-005c: the gap under the day header lives in the repeated thead cell (#1531)', async () => { + await downloadTripPDF(richArgs) + const iframe = getIframe() + const srcdoc = iframe!.srcdoc + // The thead is repeated on every overflow page, so the spacing below the header bar + // must be declared on its cell... + expect(srcdoc).toContain('.day-header > tr > td { padding-bottom: 12px; }') + // ...and not as a block-start padding on .day-body, which the print engine only paints + // on the first fragment of the (fragmented) body cell. + expect(srcdoc).toContain('.day-body { padding: 0 28px 6px; }') + }) + it('FE-COMP-TRIPPDF-006: escHtml prevents XSS in trip title', async () => { const args = { ...minimalArgs, diff --git a/client/src/components/PDF/TripPDF.tsx b/client/src/components/PDF/TripPDF.tsx index 40ba37cb7..76139379d 100644 --- a/client/src/components/PDF/TripPDF.tsx +++ b/client/src/components/PDF/TripPDF.tsx @@ -495,7 +495,11 @@ export async function downloadTripPDF({ trip, days, places, assignments, categor .day-title { font-size: 13px; font-weight: 600; color: #fff; flex: 1; } .day-date { font-size: 9px; color: rgba(255,255,255,0.45); } .day-cost { font-size: 9px; font-weight: 600; color: rgba(255,255,255,0.65); } - .day-body { padding: 12px 28px 6px; } + /* The gap under the header bar must sit inside the repeated cell: a block-start + padding on .day-body is only painted on the box's first fragment, so overflow pages + would render their first card flush against the bar (#1531). */ + .day-header > tr > td { padding-bottom: 12px; } + .day-body { padding: 0 28px 6px; } /* accommodation info */ .day-accommodations-overview { font-size: 12px; } diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 4bd3dbaeb..bf0e50818 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -96,9 +96,12 @@ interface DayPlanSidebarProps { onScrollTopChange?: (top: number) => void /** Mobile: show the route tools footer (Route toggle / Optimize / travel profile) on expanded days, since selecting a day closes the sheet */ showRouteToolsWhenExpanded?: boolean - /** Mobile: drag & drop reorder is disabled (touch-scroll hijack, #1432); the - * grip handle is hidden and the arrow reorder buttons take over instead. */ isMobile?: boolean + /** Coarse primary pointer. Drag & drop reorder is disabled here (it hijacks the + * touch-scroll gesture, #1432); the grip handle is hidden and the arrow reorder + * buttons take over instead. Distinct from isMobile, which is width-based — a + * tablet is a wide touch device and needs both. */ + isTouch?: boolean } /** @@ -144,7 +147,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { onScrollTopChange, showRouteToolsWhenExpanded = false, isMobile = false, + isTouch = false, } = props + const dragDisabled = isMobile || isTouch const toast = useToast() const { t, language, locale } = useTranslation() const ctxMenu = useContextMenu() @@ -1027,6 +1032,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { onScrollTopChange, showRouteToolsWhenExpanded, isMobile, + dragDisabled, toast, t, language, @@ -1193,6 +1199,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP onScrollTopChange, showRouteToolsWhenExpanded, isMobile, + dragDisabled, toast, t, language, @@ -1657,9 +1664,10 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP return (
{ - if (!canEditDays || isMobile) { e.preventDefault(); return } + if (!canEditDays || dragDisabled) { e.preventDefault(); return } e.dataTransfer.setData('assignmentId', String(assignment.id)) e.dataTransfer.setData('fromDayId', String(day.id)) e.dataTransfer.effectAllowed = 'move' @@ -1754,7 +1762,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP opacity: isDraggingThis ? 0.4 : 1, }} > - {canEditDays && !isMobile &&
+ {canEditDays && !dragDisabled &&
}
{ - if (!canEditDays || spanPhase === 'middle' || res.__leg || isMobile) { e.preventDefault(); return } + if (!canEditDays || spanPhase === 'middle' || res.__leg || dragDisabled) { e.preventDefault(); return } // setData is required for the drag to start reliably (Firefox) and // matches how place/note items initiate their drag. e.dataTransfer.setData('reservationId', String(res.id)) @@ -2065,7 +2073,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP opacity: draggingId === res.id ? 0.4 : spanPhase === 'middle' ? 0.65 : 1, }} > - {canEditDays && spanPhase !== 'middle' && !res.__leg && !isMobile && ( + {canEditDays && spanPhase !== 'middle' && !res.__leg && !dragDisabled && (
@@ -2182,8 +2190,9 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP return (
{ if (!canEditDays || isMobile) { e.preventDefault(); return } e.dataTransfer.setData('noteId', String(note.id)); e.dataTransfer.setData('fromDayId', String(day.id)); e.dataTransfer.effectAllowed = 'move'; dragDataRef.current = { noteId: String(note.id), fromDayId: String(day.id) }; setDraggingId(`note-${note.id}`) }} + className="dp-row" + draggable={canEditDays && !dragDisabled} + onDragStart={e => { if (!canEditDays || dragDisabled) { e.preventDefault(); return } e.dataTransfer.setData('noteId', String(note.id)); e.dataTransfer.setData('fromDayId', String(day.id)); e.dataTransfer.effectAllowed = 'move'; dragDataRef.current = { noteId: String(note.id), fromDayId: String(day.id) }; setDraggingId(`note-${note.id}`) }} onDragEnd={() => { setDraggingId(null); setDropTargetKey(null); dragDataRef.current = null }} onDragOver={e => { e.preventDefault(); e.stopPropagation(); if (dropTargetKey !== `note-${note.id}`) setDropTargetKey(`note-${note.id}`) }} onDrop={e => { @@ -2251,7 +2260,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP transition: 'background 0.1s', cursor: 'grab', userSelect: 'none', }} > - {canEditDays && !isMobile &&
+ {canEditDays && !dragDisabled &&
}
diff --git a/client/src/components/Planner/PlacesSidebar.test.tsx b/client/src/components/Planner/PlacesSidebar.test.tsx index 9c1089119..0c3325aaa 100644 --- a/client/src/components/Planner/PlacesSidebar.test.tsx +++ b/client/src/components/Planner/PlacesSidebar.test.tsx @@ -572,4 +572,36 @@ describe('Google Maps list import', () => { ); }); }); + +}); + +// #1432: a tablet is a touch device at a desktop width. Before the fix, isTouch didn't +// exist and drag was gated on width alone, so on an iPad the rows stayed draggable and a +// scroll swipe started an HTML5 drag, which raised the drop-to-import overlay instead of +// scrolling. These cases pin the desktop-width + coarse-pointer combination. +describe('touch device at desktop width (#1432)', () => { + const touchProps = { ...defaultProps, isMobile: false, isTouch: true }; + + it('FE-PLANNER-SIDEBAR-044: place rows are not draggable', () => { + const place = buildPlace({ id: 7, name: 'Tablet Place' }); + render(); + const placeRow = screen.getByText('Tablet Place').closest('div[draggable]')!; + expect(placeRow.getAttribute('draggable')).toBe('false'); + }); + + it('FE-PLANNER-SIDEBAR-045: dragging over the sidebar does not raise the drop-to-import overlay', () => { + const place = buildPlace({ id: 7, name: 'Tablet Place' }); + const { container } = render(); + fireEvent.dragEnter(container.firstChild as HTMLElement); + expect(screen.queryByText('Drop to import')).not.toBeInTheDocument(); + }); + + it('FE-PLANNER-SIDEBAR-046: a mouse-driven desktop keeps drag and the drop-to-import overlay', () => { + const place = buildPlace({ id: 7, name: 'Desktop Place' }); + const { container } = render(); + const placeRow = screen.getByText('Desktop Place').closest('div[draggable]')!; + expect(placeRow.getAttribute('draggable')).toBe('true'); + fireEvent.dragEnter(container.firstChild as HTMLElement); + expect(screen.getByText('Drop to import')).toBeInTheDocument(); + }); }); diff --git a/client/src/components/Planner/PlacesSidebar.tsx b/client/src/components/Planner/PlacesSidebar.tsx index cefe35382..5cf637627 100644 --- a/client/src/components/Planner/PlacesSidebar.tsx +++ b/client/src/components/Planner/PlacesSidebar.tsx @@ -17,19 +17,20 @@ const PlacesSidebar = React.memo(function PlacesSidebar(props: PlacesSidebarProp sidebarDragOver, handleSidebarDragEnter, handleSidebarDragOver, handleSidebarDragLeave, handleSidebarDrop, selectMode, filtered, t, dayPickerPlace, listImportOpen, fileImportOpen, setFileImportOpen, sidebarDropFile, setSidebarDropFile, tripId, pushUndo, - ctxMenu, isMobile, pendingDeleteIds, setPendingDeleteIds, onBulkDeleteConfirm, + ctxMenu, isMobile, isTouch, pendingDeleteIds, setPendingDeleteIds, onBulkDeleteConfirm, categories, selectedIds, exitSelectMode, onBulkChangeCategory, categoryPickerOpen, setCategoryPickerOpen, collectionsEnabled, saveToListOpen, setSaveToListOpen, } = S + const dragDisabled = isMobile || isTouch return (
- {!isMobile && sidebarDragOver && } + {!dragDisabled && sidebarDragOver && } {/* Kopfbereich */} diff --git a/client/src/components/Planner/PlacesSidebarList.tsx b/client/src/components/Planner/PlacesSidebarList.tsx index 9b2d75546..c268775e4 100644 --- a/client/src/components/Planner/PlacesSidebarList.tsx +++ b/client/src/components/Planner/PlacesSidebarList.tsx @@ -7,7 +7,7 @@ export function PlacesList(S: SidebarState) { const { filtered, scrollContainerRef, onScrollTopChange, filter, t, canEditPlaces, onAddPlace, categories, selectedPlaceId, plannedIds, inDaySet, selectedIds, selectMode, selectedDayId, - isMobile, onPlaceClick, openContextMenu, onAssignToDay, toggleSelected, setDayPickerPlace, registerPlaceRow, tripId, + isMobile, isTouch, onPlaceClick, openContextMenu, onAssignToDay, toggleSelected, setDayPickerPlace, registerPlaceRow, tripId, } = S // Plugin-contributed columns/actions for the places view, keyed by place id (#plugins). const contribFor = usePluginViewContributions('places', tripId) @@ -43,6 +43,7 @@ export function PlacesList(S: SidebarState) { selectedDayId={selectedDayId} canEditPlaces={canEditPlaces} isMobile={isMobile} + isTouch={Boolean(isTouch)} t={t} onPlaceClick={onPlaceClick} onContextMenu={openContextMenu} diff --git a/client/src/components/Planner/PlacesSidebarRow.tsx b/client/src/components/Planner/PlacesSidebarRow.tsx index 0801b838e..88eea516a 100644 --- a/client/src/components/Planner/PlacesSidebarRow.tsx +++ b/client/src/components/Planner/PlacesSidebarRow.tsx @@ -15,6 +15,8 @@ interface MemoPlaceRowProps { selectedDayId: number | null canEditPlaces: boolean isMobile: boolean + /** Primary pointer is coarse — HTML5 drag would swallow the scroll gesture (#1432). */ + isTouch: boolean t: (key: string, params?: Record) => string onPlaceClick: (id: number | null) => void onContextMenu: (e: React.MouseEvent, place: Place) => void @@ -26,19 +28,20 @@ interface MemoPlaceRowProps { export const MemoPlaceRow = React.memo(function MemoPlaceRow({ place, category: cat, isSelected, isPlanned, inDay, isChecked, - selectMode, selectedDayId, canEditPlaces, isMobile, t, + selectMode, selectedDayId, canEditPlaces, isMobile, isTouch, t, onPlaceClick, onContextMenu, onAssignToDay, toggleSelected, setDayPickerPlace, registerPlaceRow, }: MemoPlaceRowProps) { const hasGeometry = Boolean(place.route_geometry) + const dragDisabled = isMobile || isTouch return (
registerPlaceRow(place.id, element)} aria-selected={isSelected} data-place-id={place.id} - draggable={!selectMode && !isMobile} + draggable={!selectMode && !dragDisabled} onDragStart={e => { - if (isMobile) { e.preventDefault(); return } + if (dragDisabled) { e.preventDefault(); return } e.dataTransfer.setData('placeId', String(place.id)) e.dataTransfer.effectAllowed = 'copy' window.__dragData = { placeId: String(place.id) } @@ -56,7 +59,7 @@ export const MemoPlaceRow = React.memo(function MemoPlaceRow({ style={{ display: 'flex', alignItems: 'center', gap: 10, padding: '9px 14px 9px 16px', - cursor: selectMode || isMobile ? 'pointer' : 'grab', + cursor: selectMode || dragDisabled ? 'pointer' : 'grab', background: isChecked ? 'color-mix(in srgb, var(--accent) 8%, transparent)' : isSelected ? 'var(--border-faint)' : 'transparent', borderBottom: '1px solid var(--border-faint)', transition: 'background 0.1s', diff --git a/client/src/components/Planner/usePlacesSidebar.ts b/client/src/components/Planner/usePlacesSidebar.ts index c317d930d..8d773a4e9 100644 --- a/client/src/components/Planner/usePlacesSidebar.ts +++ b/client/src/components/Planner/usePlacesSidebar.ts @@ -31,6 +31,8 @@ export interface PlacesSidebarProps { onBulkChangeCategory?: (ids: number[], categoryId: number | null) => void days: Day[] isMobile: boolean + /** Primary pointer is coarse — HTML5 drag would swallow the scroll gesture (#1432). */ + isTouch?: boolean onCategoryFilterChange?: (categoryIds: Set) => void onPlacesFilterChange?: (filter: string) => void pushUndo?: (label: string, undoFn: () => Promise | void) => void diff --git a/client/src/hooks/useIsTouch.test.ts b/client/src/hooks/useIsTouch.test.ts new file mode 100644 index 000000000..511cc3379 --- /dev/null +++ b/client/src/hooks/useIsTouch.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect, afterEach, vi } from 'vitest' +import { renderHook, act } from '@testing-library/react' +import { useIsTouch } from './useIsTouch' + +type Listener = (e: MediaQueryListEvent) => void + +/** Fake matchMedia that matches only `(pointer: coarse)` and can fire a change. */ +function mockPointer(coarse: boolean) { + const listeners: Listener[] = [] + vi.spyOn(window, 'matchMedia').mockImplementation((query: string) => ({ + matches: query === '(pointer: coarse)' ? coarse : false, + media: query, + addEventListener: (_: string, l: Listener) => listeners.push(l), + removeEventListener: (_: string, l: Listener) => { + const i = listeners.indexOf(l) + if (i >= 0) listeners.splice(i, 1) + }, + }) as unknown as MediaQueryList) + return { fire: (matches: boolean) => listeners.forEach(l => l({ matches } as MediaQueryListEvent)) } +} + +describe('useIsTouch', () => { + afterEach(() => vi.restoreAllMocks()) + + // A tablet is a coarse-pointer device at a desktop width, so a width check misses it — + // which is what left the places list undraggable-but-unscrollable on iPad (#1432). + it('is true when the primary pointer is coarse, regardless of viewport width', () => { + mockPointer(true) + const { result } = renderHook(() => useIsTouch()) + expect(result.current).toBe(true) + }) + + it('is false on a mouse-driven desktop', () => { + mockPointer(false) + const { result } = renderHook(() => useIsTouch()) + expect(result.current).toBe(false) + }) + + it('reacts when the pointer type changes', () => { + const mq = mockPointer(false) + const { result } = renderHook(() => useIsTouch()) + expect(result.current).toBe(false) + act(() => mq.fire(true)) + expect(result.current).toBe(true) + }) +}) diff --git a/client/src/hooks/useIsTouch.ts b/client/src/hooks/useIsTouch.ts new file mode 100644 index 000000000..8ee90277c --- /dev/null +++ b/client/src/hooks/useIsTouch.ts @@ -0,0 +1,26 @@ +import { useState, useEffect } from 'react' + +/** + * Returns true when the primary pointer is coarse (finger/stylus) — phones and + * tablets, at any viewport width. A mouse-driven desktop is false, including a + * touchscreen laptop, whose primary pointer is still the mouse. + * + * Viewport width is not a proxy for touch: an iPad is 820-1366 CSS px, so a + * width-based check leaves HTML5 `draggable` armed there and the drag swallows + * the scroll gesture (#1432). + */ +export function useIsTouch(): boolean { + const [isTouch, setIsTouch] = useState( + () => typeof window !== 'undefined' && window.matchMedia('(pointer: coarse)').matches, + ) + + useEffect(() => { + const mq = window.matchMedia('(pointer: coarse)') + const handler = (e: MediaQueryListEvent) => setIsTouch(e.matches) + setIsTouch(mq.matches) + mq.addEventListener('change', handler) + return () => mq.removeEventListener('change', handler) + }, []) + + return isTouch +} diff --git a/client/src/index.css b/client/src/index.css index 7e22da2e7..8d2dfad28 100644 --- a/client/src/index.css +++ b/client/src/index.css @@ -144,19 +144,32 @@ html.dark button[class*="text-slate-600"][class*="border-slate-200"]:hover { bac html.dark .border-dashed.border-gray-300 { border-color: var(--border-primary) !important; } html.dark .bg-slate-50\/60, html.dark [class*="bg-slate-50/"] { background-color: transparent !important; } -/* Reorder buttons: desktop = original style; mobile = always visible, larger touch targets */ +/* Reorder buttons: desktop = original style; touch = always visible, larger touch targets. + Keyed on pointer as well as width — a tablet is a wide touch device, and drag reorder is + off wherever the pointer is coarse (#1432), so these buttons are the only affordance left. */ .reorder-buttons { flex-direction: column; opacity: 0; + /* Hidden means non-interactive. Opacity alone still hit-tests, which left an invisible + but fully clickable target on every row that silently reordered the itinerary. */ + pointer-events: none; } .reorder-buttons button { background: none !important; padding: 1px 2px; } -@media (max-width: 767px) { +/* Reveal on row hover (and on keyboard focus, so the buttons are reachable by Tab). */ +.dp-row:hover .reorder-buttons, +.dp-row:focus-within .reorder-buttons { + opacity: 1; + pointer-events: auto; +} +@media (max-width: 767px), (pointer: coarse) { .reorder-buttons { flex-direction: row !important; opacity: 1 !important; + /* There is no hover on touch, so these are always live — undo the base rule. */ + pointer-events: auto !important; align-items: center; margin-left: auto; } @@ -175,11 +188,6 @@ html.dark .bg-slate-50\/60, html.dark [class*="bg-slate-50/"] { background-color } } -/* Ort-Zeile Hover: Sortier-Buttons anzeigen */ -.place-row:hover .reorder-btns { - opacity: 1 !important; -} - /* Ladeanimation */ @keyframes spin { to { transform: rotate(360deg); } diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index e365d2dbb..d6c85a881 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -203,7 +203,7 @@ export default function TripPlannerPage(): React.ReactElement | null { mobileSidebarOpen, setMobileSidebarOpen, mobilePlanScrollTopRef, mobilePlacesScrollTopRef, deletePlaceId, setDeletePlaceId, deletePlaceIds, setDeletePlaceIds, visibleConnections, setVisibleConnections, toggleConnection, mapTransportDetail, setMapTransportDetail, - isMobile, mapCategoryFilter, setMapCategoryFilter, mapPlacesFilter, setMapPlacesFilter, + isMobile, isTouch, mapCategoryFilter, setMapCategoryFilter, mapPlacesFilter, setMapPlacesFilter, expandedDayIds, setExpandedDayIds, mapPlaces, route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay, handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, openAddPlaceFromPoi, @@ -387,6 +387,7 @@ export default function TripPlannerPage(): React.ReactElement | null { }}>
diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 104596ca2..c9cdbfb05 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -21,6 +21,7 @@ import { useRouteCalculation } from '../../hooks/useRouteCalculation' import { usePlaceSelection } from '../../hooks/usePlaceSelection' import { usePlannerHistory } from '../../hooks/usePlannerHistory' import { useAirtrailConnection } from '../../hooks/useAirtrailConnection' +import { useIsTouch } from '../../hooks/useIsTouch' import { usePluginStore } from '../../store/pluginStore' import type { Accommodation, TripMember, Day, Place, Reservation } from '../../types' import { resolvePoolAssignmentId } from './tripPlannerModel' @@ -269,6 +270,9 @@ export function useTripPlanner() { mq.addEventListener('change', handler) return () => mq.removeEventListener('change', handler) }, []) + // Layout is width-driven (isMobile); drag affordances are pointer-driven (isTouch). + // Conflating them is what left a tablet's places list undraggable-but-unscrollable (#1432). + const isTouch = useIsTouch() // Start photo fetches during splash screen so images are ready when map mounts useEffect(() => { @@ -910,7 +914,7 @@ export function useTripPlanner() { mobileSidebarOpen, setMobileSidebarOpen, mobilePlanScrollTopRef, mobilePlacesScrollTopRef, deletePlaceId, setDeletePlaceId, deletePlaceIds, setDeletePlaceIds, visibleConnections, setVisibleConnections, toggleConnection, mapTransportDetail, setMapTransportDetail, - isMobile, mapCategoryFilter, setMapCategoryFilter, mapPlacesFilter, setMapPlacesFilter, + isMobile, isTouch, mapCategoryFilter, setMapCategoryFilter, mapPlacesFilter, setMapPlacesFilter, expandedDayIds, setExpandedDayIds, mapPlaces, route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay, handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, openAddPlaceFromPoi, diff --git a/client/src/utils/touchDragPolyfill.test.ts b/client/src/utils/touchDragPolyfill.test.ts index 7a6c9c2dc..03fbe4d28 100644 --- a/client/src/utils/touchDragPolyfill.test.ts +++ b/client/src/utils/touchDragPolyfill.test.ts @@ -5,23 +5,39 @@ import { maybeInstallTouchDragPolyfill } from './touchDragPolyfill' // document-level touch listeners the package installs on import. vi.mock('drag-drop-touch', () => ({})) -function setWidth(w: number) { - Object.defineProperty(window, 'innerWidth', { value: w, configurable: true, writable: true }) +// The gate is pointer-based, not width-based. The polyfill synthesises HTML5 drag events +// from touchmove, so on a coarse-pointer device it turns a scroll swipe into a drag +// (#1432) and fabricates a dblclick that zooms the map (#1440). Drag reorder is disabled +// on those devices anyway, leaving one device class with a job for it: the hybrid laptop +// — mouse as primary pointer, touchscreen also present. +function mockPointer(matching: string[]) { + vi.spyOn(window, 'matchMedia').mockImplementation( + (query: string) => ({ matches: matching.includes(query), media: query }) as MediaQueryList, + ) } describe('maybeInstallTouchDragPolyfill', () => { - const original = window.innerWidth - afterEach(() => setWidth(original)) + afterEach(() => vi.restoreAllMocks()) - it('does not load the polyfill on mobile viewports (<1024px)', () => { - setWidth(390) - expect(maybeInstallTouchDragPolyfill()).toBeUndefined() - }) - - it('loads the polyfill on large viewports (>=1024px)', async () => { - setWidth(1440) + it('loads on a hybrid laptop — mouse primary, touchscreen available', async () => { + mockPointer(['(pointer: fine) and (any-pointer: coarse)']) const result = maybeInstallTouchDragPolyfill() expect(result).toBeInstanceOf(Promise) await expect(result).resolves.toBeDefined() }) + + it('does not load on a tablet — coarse primary pointer at a desktop width (#1432)', () => { + mockPointer(['(pointer: coarse)', '(any-pointer: coarse)']) + expect(maybeInstallTouchDragPolyfill()).toBeUndefined() + }) + + it('does not load on a phone', () => { + mockPointer(['(pointer: coarse)', '(any-pointer: coarse)']) + expect(maybeInstallTouchDragPolyfill()).toBeUndefined() + }) + + it('does not load on a plain mouse-driven desktop', () => { + mockPointer(['(pointer: fine)']) + expect(maybeInstallTouchDragPolyfill()).toBeUndefined() + }) }) diff --git a/client/src/utils/touchDragPolyfill.ts b/client/src/utils/touchDragPolyfill.ts index 96abe7ff1..ead905a51 100644 --- a/client/src/utils/touchDragPolyfill.ts +++ b/client/src/utils/touchDragPolyfill.ts @@ -6,13 +6,14 @@ * dblclick fires the default double-click-zoom, so two quick one-finger pans zoomed * instead of panning (#1440). * - * Reorder DnD is disabled on mobile viewports (#1432), so the polyfill has no job below - * the lg breakpoint — skip it there to remove the phantom-dblclick source on phones - * while keeping touch DnD on desktop / large (touch) viewports. + * Reorder DnD is disabled wherever the primary pointer is coarse (#1432), so the only + * device class left with a job for the polyfill is the hybrid laptop: a mouse as the + * primary pointer, with a touchscreen also available. Loading it anywhere else — notably + * on tablets, which a width check misclassifies as desktop — re-arms the very gesture + * hijack #1432 removed, and drags the #1440 phantom-dblclick along with it. */ export function maybeInstallTouchDragPolyfill(): Promise | void { if (typeof window === 'undefined') return - // Mirrors useIsMobile's SSR-safe initial check (lg breakpoint = 1024px). - if (window.innerWidth < 1024) return + if (!window.matchMedia('(pointer: fine) and (any-pointer: coarse)').matches) return return import('drag-drop-touch') } diff --git a/package-lock.json b/package-lock.json index 50fc23716..3002c22fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6144,8 +6144,7 @@ "optional": true, "os": [ "android" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-android-arm64": { "version": "4.62.0", @@ -6159,8 +6158,7 @@ "optional": true, "os": [ "android" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-darwin-arm64": { "version": "4.62.0", @@ -6174,8 +6172,7 @@ "optional": true, "os": [ "darwin" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-darwin-x64": { "version": "4.62.0", @@ -6189,8 +6186,7 @@ "optional": true, "os": [ "darwin" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-freebsd-arm64": { "version": "4.62.0", @@ -6204,8 +6200,7 @@ "optional": true, "os": [ "freebsd" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-freebsd-x64": { "version": "4.62.0", @@ -6219,8 +6214,7 @@ "optional": true, "os": [ "freebsd" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-arm-gnueabihf": { "version": "4.62.0", @@ -6234,8 +6228,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-arm-musleabihf": { "version": "4.62.0", @@ -6249,8 +6242,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-arm64-gnu": { "version": "4.62.0", @@ -6264,8 +6256,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-arm64-musl": { "version": "4.62.0", @@ -6292,8 +6283,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-loong64-musl": { "version": "4.62.0", @@ -6307,8 +6297,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-ppc64-gnu": { "version": "4.62.0", @@ -6322,8 +6311,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-ppc64-musl": { "version": "4.62.0", @@ -6337,8 +6325,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-riscv64-gnu": { "version": "4.62.0", @@ -6352,8 +6339,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-riscv64-musl": { "version": "4.62.0", @@ -6367,8 +6353,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-s390x-gnu": { "version": "4.62.0", @@ -6382,8 +6367,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-x64-gnu": { "version": "4.62.0", @@ -6397,8 +6381,7 @@ "optional": true, "os": [ "linux" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-linux-x64-musl": { "version": "4.62.0", @@ -6425,8 +6408,7 @@ "optional": true, "os": [ "openbsd" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-openharmony-arm64": { "version": "4.62.0", @@ -6440,8 +6422,7 @@ "optional": true, "os": [ "openharmony" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-win32-arm64-msvc": { "version": "4.62.0", @@ -6455,8 +6436,7 @@ "optional": true, "os": [ "win32" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-win32-ia32-msvc": { "version": "4.62.0", @@ -6470,8 +6450,7 @@ "optional": true, "os": [ "win32" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-win32-x64-gnu": { "version": "4.62.0", @@ -6485,8 +6464,7 @@ "optional": true, "os": [ "win32" - ], - "peer": true + ] }, "node_modules/@rollup/rollup-win32-x64-msvc": { "version": "4.62.0", @@ -6500,8 +6478,7 @@ "optional": true, "os": [ "win32" - ], - "peer": true + ] }, "node_modules/@scarf/scarf": { "version": "1.4.0", @@ -21237,7 +21214,7 @@ "name": "@trek/server", "version": "3.3.0", "dependencies": { - "@modelcontextprotocol/sdk": "^1.28.0", + "@modelcontextprotocol/sdk": "^1.29.0", "@nestjs/common": "^11.1.24", "@nestjs/core": "^11.1.24", "@nestjs/platform-express": "^11.1.24", diff --git a/plugin-sdk/package.json b/plugin-sdk/package.json index 29848d74a..703f8b82c 100644 --- a/plugin-sdk/package.json +++ b/plugin-sdk/package.json @@ -1,6 +1,6 @@ { "name": "trek-plugin-sdk", - "version": "1.4.1", + "version": "1.5.0", "description": "SDK for building TREK plugins: types, definePlugin, a mock host, manifest validation, and the create-trek-plugin / trek-plugin CLIs.", "type": "module", "license": "MIT", diff --git a/plugin-sdk/src/cli/dev.ts b/plugin-sdk/src/cli/dev.ts index 3e8c8fee7..3a6c2aa35 100644 --- a/plugin-sdk/src/cli/dev.ts +++ b/plugin-sdk/src/cli/dev.ts @@ -92,9 +92,31 @@ function flatBind(args: unknown[]): unknown[] { /** A dev db backed by node:sqlite if available, else an in-memory recorder that returns []. */ export function createDevDb(dbFile: string): { db: PluginContextDb; note: string; close: () => void } { + // node:sqlite ships on Node 22.5+. Probe for it SEPARATELY from opening the database: + // one try/catch around both meant an mkdir/permission failure silently degraded to the + // stub too, and the stub swallows every write while reporting success — a db:own plugin + // "works" in dev and persists nothing. Fail loudly on a real error; degrade only on old Node. + let DatabaseSync: (new (p: string) => SqliteDb) | undefined; try { - // node:sqlite is available on Node 22.5+ (experimental). Fall back gracefully if not. - const { DatabaseSync } = createRequire(import.meta.url)('node:sqlite') as { DatabaseSync: new (p: string) => SqliteDb }; + ({ DatabaseSync } = createRequire(import.meta.url)('node:sqlite') as { DatabaseSync: new (p: string) => SqliteDb }); + } catch { + console.warn( + `\n ⚠ node:sqlite is unavailable on this Node (${process.version}) — ctx.db is a STUB.\n` + + ` Every query returns [] and every write reports 0 changes: a db:own plugin will look\n` + + ` like it works and persist NOTHING. Upgrade to Node 22.5+ to test db:own for real.\n`, + ); + return { + note: `db:own → in-memory STUB (writes are discarded — upgrade to Node 22.5+; you are on ${process.version})`, + close: () => {}, + db: { + async query(sql: string) { guardSql(sql); return []; }, + async exec(sql: string) { guardSql(sql); return { changes: 0 }; }, + async migrate(_id: string, sql: string) { guardSql(sql); return { applied: true }; }, + }, + }; + } + + { fs.mkdirSync(path.dirname(dbFile), { recursive: true }); const sq = new DatabaseSync(dbFile); const applied = new Set(); @@ -116,16 +138,6 @@ export function createDevDb(dbFile: string): { db: PluginContextDb; note: string async migrate(id: string, sql: string) { guardSql(sql); if (applied.has(id)) return { applied: false }; sq.exec(sql); applied.add(id); return { applied: true }; }, }, }; - } catch { - return { - note: 'db:own → in-memory stub (upgrade to Node 22.5+ for a real SQLite dev db)', - close: () => {}, - db: { - async query(sql: string) { guardSql(sql); return []; }, - async exec(sql: string) { guardSql(sql); return { changes: 0 }; }, - async migrate(_id: string, sql: string) { guardSql(sql); return { applied: true }; }, - }, - }; } } @@ -341,7 +353,26 @@ export async function runDev(dir: string, opts: { port?: number } = {}): Promise else if (kind === 'event') { for (const s of plugin.events ?? []) if (s.on === name || s.on === '*') await s.handler({ event: name, tripId: 0, ...(payload as object) }, userlessCtx); out = { delivered: true }; } else if (kind === 'deleteUserData') { if (!plugin.deleteUserData) return send(404, 'no deleteUserData handler'); out = await plugin.deleteUserData({ userId: Number((payload as { userId?: unknown })?.userId ?? name ?? 0) }, userlessCtx); } else if (kind === 'exportUserData') { if (!plugin.exportUserData) return send(404, 'no exportUserData handler'); out = await plugin.exportUserData({ userId: Number((payload as { userId?: unknown })?.userId ?? name ?? 0) }, userlessCtx); } - else if (kind === 'hook') { const impl = plugin.hooks?.[name]; if (!impl || typeof impl[fn] !== 'function') return send(404, `no hook ${name}.${fn}`); out = await impl[fn](...(Array.isArray(payload) ? payload : payload != null ? [payload] : []), ctx); } + else if (kind === 'hook') { + const impl = plugin.hooks?.[name]; + if (!impl || typeof impl[fn] !== 'function') return send(404, `no hook ${name}.${fn}`); + if (name === 'notificationChannel') { + // A notification channel is NOT like the other hooks: the host fires it with no + // acting user (it delivers to an arbitrary recipient), and hands the recipient's + // decrypted settings in as a separate `config` argument — + // send(msg, config, ctx) · test(config, ctx) + // Firing it like a normal hook passed `ctx` where `config` belongs, so a channel + // plugin read its settings off the ctx object and "worked" in dev while being + // broken in production. Mirror the host (and createMockHost) exactly. + const config = (fx.userSettings ?? {}) as Record; + const args = Array.isArray(payload) ? (payload as unknown[]) : payload != null ? [payload] : []; + out = fn === 'test' + ? await impl[fn](config, userlessCtx) + : await impl[fn](args[0], (args[1] as Record) ?? config, userlessCtx); + } else { + out = await impl[fn](...(Array.isArray(payload) ? payload : payload != null ? [payload] : []), ctx); + } + } else return send(400, `unknown fire kind "${kind}"`); return send(200, JSON.stringify(out ?? { ok: true }, null, 2), 'application/json; charset=utf-8'); } catch (e) { @@ -357,7 +388,11 @@ export async function runDev(dir: string, opts: { port?: number } = {}): Promise // proxies trek:invoke to your /api routes, and honours resize/notify/navigate. This // is where the design kit actually renders themed. if (url.pathname === '/preview' && String(manifest.type) !== 'integration') { - return send(200, preview(id, String(manifest.type)), 'text/html; charset=utf-8'); + // Preview against a trip that actually EXISTS in the fixtures. This used to be + // hard-coded to 42 while the scaffold seeds trip 1, so the widget's first + // trek:invoke hit assertMember(42, …) and 500'd with RESOURCE_FORBIDDEN. + const previewTripId = Number(Object.keys(fx.trips ?? {})[0] ?? 1) || 1; + return send(200, preview(id, String(manifest.type), previewTripId), 'text/html; charset=utf-8'); } // Static plugin UI at /ui (page/widget client bundle). The iframe loads @@ -485,7 +520,7 @@ ${LIVE_RELOAD}`; * toggle), proxies trek:invoke to the dev server's /api routes with the dev user, * and surfaces resize/notify/navigate. This is where the design kit renders themed. */ -function preview(id: string, type: string): string { +function preview(id: string, type: string, tripId: number): string { const maxW = type === 'widget' ? '440px' : '1000px'; return ` ${id} · preview @@ -535,7 +570,7 @@ function ctx(){ document.body.classList.toggle("dark", theme==="dark"); var tokens={}; var src=theme==="dark"?DA[accent]:LA[accent]; for(var k in src){tokens[k]=src[k];} return {type:"trek:context",theme:theme,locale:"en",hostOrigin:location.origin, - tripId: val("trip").checked?42:null, userId:"1", + tripId: val("trip").checked?${tripId}:null, userId:"1", user:{name:"Dev User",avatar:null,isAdmin:true}, appearance:{scheme:accent,density:"comfortable",reducedMotion:val("rm").checked,noTransparency:val("nt").checked}, formats:{locale:"en",currency:"EUR",timeFormat:"24h",distanceUnit:"metric",temperatureUnit:"celsius",timezone:Intl.DateTimeFormat().resolvedOptions().timeZone}, diff --git a/plugin-sdk/src/cli/entry.ts b/plugin-sdk/src/cli/entry.ts index b7b471ee5..915fbfbed 100644 --- a/plugin-sdk/src/cli/entry.ts +++ b/plugin-sdk/src/cli/entry.ts @@ -4,8 +4,8 @@ * [--merge registry/plugins/.json] [--out file] * * Generates the ready-to-PR TREK-Plugins registry entry from the manifest + the - * packed plugin.zip + the git tag — so the sha256, size, commitSha, downloadUrl - * and minTrekVersion an author would compute by hand all come out of one command. + * packed plugin.zip + the git tag — so the sha256, size, commitSha and downloadUrl + * an author would compute by hand all come out of one command. * With --merge it prepends the new version onto an existing entry (the update * case), keeping versions newest-first. */ @@ -13,12 +13,25 @@ import fs from 'node:fs'; import path from 'node:path'; import { createHash } from 'node:crypto'; import { execFileSync } from 'node:child_process'; +import { minVersion, validRange } from 'semver'; import { loadPrivateKey, signArtifact, publicKeyBase64 } from './sign.js'; import { readJsonFile } from './json.js'; interface Version { version: string; gitTag: string; commitSha: string; downloadUrl: string; - sha256: string; minTrekVersion: string; size: number; apiVersion: number; + sha256: string; + /** + * The manifest's `trek` range, verbatim — the ONE compatibility field an entry carries. + * It is what TREK gates installs and activation on, and the only field that can express + * an exclusive upper bound. + * + * There is deliberately no `minTrekVersion` here any more. It said the same thing (the + * range's lower bound) in a weaker form, and a second field restating a derived fact is a + * second field that can be wrong. The registry still ACCEPTS one on entries published + * before `trek` existed — and checks it agrees with the range — but nothing emits one now. + */ + trek: string; + size: number; apiVersion: number; nativeModules: false; operatorEgress?: boolean; publishedAt: string; signature?: string; } interface Entry { @@ -26,10 +39,20 @@ interface Entry { homepage?: string; tags?: string[]; type: string; authorPublicKey?: string; versions: Version[]; } -/** Lower bound of a `trek` range like ">=3.2.0 <4.0.0" -> "3.2.0" (matches the server). */ +/** + * Lower bound of a `trek` range like ">=3.2.0 <4.0.0" -> "3.2.0" (matches the server). + * + * Read off the range with semver rather than by finding the first version-shaped substring: + * for "<4.0.0" the first such substring is 4.0.0, which is the range's *upper* bound and + * would have been published as the minimum — the precise inverse of what the plugin says. + */ function minTrekFrom(trek: unknown): string | null { - if (typeof trek !== 'string') return null; - return trek.match(/(\d+\.\d+\.\d+)/)?.[1] ?? null; + if (typeof trek !== 'string' || validRange(trek) === null) return null; + try { + return minVersion(trek)?.version ?? null; + } catch { + return null; + } } function resolveCommit(dir: string, tag: string, override?: string): string { @@ -50,8 +73,9 @@ export function buildEntry(opts: { }): Entry { const manifest = readJsonFile>(path.join(opts.dir, 'trek-plugin.json')); if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(opts.repo)) throw new Error(`--repo must be "owner/name", got "${opts.repo}"`); - const minTrek = minTrekFrom(manifest.trek); - if (!minTrek) throw new Error('manifest has no "trek" version range to derive minTrekVersion from (e.g. "trek": ">=3.2.0 <4.0.0")'); + // minVersion() is null for a range nothing can satisfy (">=4.0.0 <3.0.0" parses fine), so + // this doubles as the satisfiability check — TREK would refuse to install such a plugin. + if (!minTrekFrom(manifest.trek)) throw new Error('manifest has no valid "trek" version range (e.g. "trek": ">=3.2.0 <4.0.0")'); if (!fs.existsSync(opts.zipPath)) throw new Error(`artifact not found: ${opts.zipPath} — run \`trek-plugin pack\` first`); const buf = fs.readFileSync(opts.zipPath); @@ -62,7 +86,7 @@ export function buildEntry(opts: { commitSha: resolveCommit(opts.dir, opts.tag, opts.commit), downloadUrl: `https://github.com/${opts.repo}/releases/download/${opts.tag}/${asset}`, sha256: createHash('sha256').update(buf).digest('hex'), - minTrekVersion: minTrek, + trek: String(manifest.trek), size: buf.length, apiVersion: typeof manifest.apiVersion === 'number' ? manifest.apiVersion : 1, nativeModules: false, diff --git a/plugin-sdk/src/cli/preflight.ts b/plugin-sdk/src/cli/preflight.ts index b960f968d..db6f4fe9c 100644 --- a/plugin-sdk/src/cli/preflight.ts +++ b/plugin-sdk/src/cli/preflight.ts @@ -1,24 +1,31 @@ /** * trek-plugin preflight — run the registry CI checks locally, over the network, * BEFORE you open a PR. It mirrors TREK-Plugins' validate-entry.mjs (tag→commit, - * manifest parity, artifact sha256/size, native-binary scan) and check-readme.mjs - * (required sections, real prose, a resolving screenshot, permission parity), so - * you catch what CI would reject without a round-trip through review. + * manifest parity incl. operatorEgress/requiredAddons/pluginDependencies, artifact + * sha256/size, author-signature shape + verification, native-binary scan) and + * check-readme.mjs (required sections, real prose, a resolving screenshot, permission + * parity), so you catch what CI would reject without a round-trip through review. + * + * That mirroring is the whole contract: a gate the registry has and preflight doesn't is + * a false green, and an author trusts a green. Keep the two in step. * * Dependency-free: global fetch + Node built-ins + our own zip reader. */ import { execFileSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import { listZipNames } from '../zip.js'; +import { verifyAuthorSignature, checkSignatureShape, SignatureError } from './verify-signature.js'; const REQUIRED_HEADINGS = ['what it does', 'screenshots', 'permissions', 'setup']; const PLACEHOLDER_RE = [/\{\{[^}]*\}\}/, /\bREPLACE_ME\b/i, /\bDescribe (what|the)\b/i, /\byour-name\/trek-plugin/i]; const MIN_PROSE = 400; const NATIVE_RE = /(^|\/)[^/]+\.node$|(^|\/)binding\.gyp$|(^|\/)prebuilds?\//i; +export interface PluginDependency { id: string; version: string } export interface EntryVersion { version: string; gitTag: string; commitSha: string; downloadUrl: string; sha256: string; size: number; apiVersion: number; nativeModules?: boolean; operatorEgress?: boolean; signature?: string; + requiredAddons?: string[]; pluginDependencies?: PluginDependency[]; } export interface Entry { id: string; name: string; type: string; repo: string; authorPublicKey?: string; versions: EntryVersion[]; @@ -48,7 +55,10 @@ export async function preflight(entry: Entry, opts: { all?: boolean } = {}): Pro if (!/^[a-z][a-z0-9-]{2,39}$/.test(entry.id)) fail(`id "${entry.id}" is not a valid slug (^[a-z][a-z0-9-]{2,39}$)`); if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(entry.repo)) fail(`repo "${entry.repo}" is not "owner/name"`); if (!['integration', 'page', 'widget', 'trip-page'].includes(entry.type)) fail(`type "${entry.type}" is not integration|page|widget|trip-page`); - if (entry.authorPublicKey && !entry.versions[0]?.signature) fail('entry has authorPublicKey but the newest version has no signature (sign the version too)'); + // Signature SHAPE (mirrors the registry's checkSignatureShape): a key and a signature + // must be both-present or both-absent, and each must parse. TREK refuses to install a + // half-signed entry, so one is dead on arrival — better to say so now than at review. + for (const p of checkSignatureShape(entry)) fail(p); const versions = opts.all ? entry.versions : entry.versions.slice(0, 1); for (const v of versions) { @@ -79,7 +89,12 @@ export async function preflight(entry: Entry, opts: { all?: boolean } = {}): Pro if (m.id !== entry.id) fail(`${tag}: manifest id "${m.id}" != entry id "${entry.id}"`); if (m.version !== v.version) fail(`${tag}: manifest version "${m.version}" != entry "${v.version}"`); if (m.type !== entry.type) fail(`${tag}: manifest type "${m.type}" != entry "${entry.type}"`); - if (m.apiVersion !== v.apiVersion) fail(`${tag}: manifest apiVersion ${m.apiVersion} != entry ${v.apiVersion}`); + // apiVersion is OPTIONAL in the manifest: the host defaults it to 1 at install + // (install/manifest.ts) and `entry` defaults it to 1 when building the entry. Compare + // the DEFAULTED values, or a manifest that legally omits it fails here with + // "manifest apiVersion undefined != entry 1" while the registry's CI passes it. + const mApiVersion = typeof m.apiVersion === 'number' ? m.apiVersion : 1; + if (mApiVersion !== v.apiVersion) fail(`${tag}: manifest apiVersion ${mApiVersion} != entry ${v.apiVersion}`); if (m.nativeModules === true) fail(`${tag}: manifest declares nativeModules:true (forbidden)`); // Mirrors the registry's operatorEgress parity check (validate-entry.mjs): the entry // must not understate the plugin's network reach. @@ -96,6 +111,22 @@ export async function preflight(entry: Entry, opts: { all?: boolean } = {}): Pro } if (egress.includes('*')) fail(`${tag}: egress[] must not contain a bare "*"`); } + if (mOperatorEgress && !manifestPerms.some((p) => p === 'http:outbound' || p.startsWith('http:outbound:'))) { + fail(`${tag}: operatorEgress declared without an http:outbound permission`); + } + // Dependency parity (mirrors validate-entry.mjs): the enriched entry must mirror the + // manifest, because TREK resolves requiredAddons/pluginDependencies from the INDEX + // before it ever downloads the artifact. An entry that understates them resolves + // against a dependency set the code doesn't actually have. + const normAddons = (a: unknown) => (Array.isArray(a) ? [...a].map(String).sort() : []); + if (JSON.stringify(normAddons(m.requiredAddons)) !== JSON.stringify(normAddons(v.requiredAddons))) { + fail(`${tag}: manifest requiredAddons != entry requiredAddons`); + } + const normDeps = (d: unknown) => + (Array.isArray(d) ? d.map((x) => `${(x as PluginDependency)?.id}@${(x as PluginDependency)?.version}`).sort() : []); + if (JSON.stringify(normDeps(m.pluginDependencies)) !== JSON.stringify(normDeps(v.pluginDependencies))) { + fail(`${tag}: manifest pluginDependencies != entry pluginDependencies`); + } if (!failures.some((f) => f.startsWith(`${tag}: manifest`))) ok(`${tag}: manifest at commit matches the entry`); } } catch (e) { fail(`${tag}: manifest parity failed: ${(e as Error).message}`); } @@ -110,6 +141,23 @@ export async function preflight(entry: Entry, opts: { all?: boolean } = {}): Pro const sha = createHash('sha256').update(bytes).digest('hex'); if (sha !== v.sha256) fail(`${tag}: sha256 mismatch — release has ${sha.slice(0, 12)}…, entry pins ${v.sha256.slice(0, 12)}… (re-pack + re-upload?)`); else ok(`${tag}: artifact sha256 matches the release`); + + // Author signature over those same bytes. sha256 proves they are what the REGISTRY + // vouches for; the signature proves they came from the AUTHOR's key. The registry's + // CI verifies this and TREK verifies it again at install, so a signature that does + // not check out is an entry nobody can install — and the author would only find out + // at review. We already have the bytes, so it costs nothing to say so now. + if (v.signature && entry.authorPublicKey) { + try { + if (!verifyAuthorSignature(bytes, v.signature, entry.authorPublicKey)) { + fail(`${tag}: author signature does not verify against authorPublicKey — did you sign with a different key, or re-pack after signing?`); + } else ok(`${tag}: author signature verifies`); + } catch (e) { + if (e instanceof SignatureError) fail(`${tag}: signature/key is malformed: ${e.message}`); + else throw e; + } + } + let names: string[] = []; try { names = bytes[0] === 0x50 && bytes[1] === 0x4b ? listZipNames(bytes) : []; } catch { /* not a zip */ } if (names.some((n) => NATIVE_RE.test(n))) fail(`${tag}: artifact contains native binaries (.node / binding.gyp / prebuilds)`); diff --git a/plugin-sdk/src/cli/publish.ts b/plugin-sdk/src/cli/publish.ts index 11c84b9e2..e72eab4af 100644 --- a/plugin-sdk/src/cli/publish.ts +++ b/plugin-sdk/src/cli/publish.ts @@ -7,7 +7,6 @@ * * Requires `git` + `gh` (authenticated), same as `release`/`submit`. */ -import fs from 'node:fs'; import path from 'node:path'; import { execFileSync } from 'node:child_process'; import { packPluginDir } from './pack.js'; @@ -22,10 +21,15 @@ function git(dir: string, args: string[], quiet = true): string { function tagExists(dir: string, tag: string): boolean { try { git(dir, ['rev-parse', `${tag}^{commit}`]); return true; } catch { return false; } } +function releaseExists(repo: string, tag: string): boolean { + try { execFileSync('gh', ['release', 'view', tag, '--repo', repo], { stdio: 'pipe' }); return true; } catch { return false; } +} export async function publishPlugin(opts: { dir: string; repo: string; tag: string; now: string; signKeyPath?: string; registry?: string; draft?: boolean; notes?: string; skipPreflight?: boolean; + /** Overwrite the artifact on an existing release. Only safe if it was never merged. */ + force?: boolean; /** Progress sink. Defaults to the plain console.error lines (CI parity). */ log?: LogSink; }): Promise<{ prUrl: string }> { @@ -45,11 +49,23 @@ export async function publishPlugin(opts: { try { git(dir, ['push', 'origin', opts.tag]); } catch { throw new Error(`could not push tag ${opts.tag} — is "origin" your plugin's GitHub repo and are you authenticated? (git push origin ${opts.tag})`); } - try { - execFileSync('gh', ['release', 'create', opts.tag, packed.artifact, '--repo', opts.repo, '--title', opts.tag, '--notes', opts.notes || `Release ${opts.tag}`], { stdio: 'pipe' }); - } catch { - // Release already exists — (re)upload the packed artifact so its bytes match the pin. + // A released artifact is IMMUTABLE: the registry pins its sha256, so overwriting the + // bytes of a release that is already in the registry breaks the checksum for everyone + // who has that version — they can no longer install or update it. Refuse by default. + // (The old code blanket-caught every `gh release create` failure — auth, network, a bad + // repo — and turned it into a --clobber upload.) + if (releaseExists(opts.repo, opts.tag)) { + if (!opts.force) { + throw new Error( + `release ${opts.tag} already exists on ${opts.repo}.\n` + + `Overwriting a released artifact breaks the sha256 pin for everyone who already installed it.\n` + + `Cut a new version, or pass --force if this release was never merged into the registry.`, + ); + } + log(` ! release ${opts.tag} exists — overwriting the artifact (--force)`); execFileSync('gh', ['release', 'upload', opts.tag, packed.artifact, '--repo', opts.repo, '--clobber'], { stdio: 'pipe' }); + } else { + execFileSync('gh', ['release', 'create', opts.tag, packed.artifact, '--repo', opts.repo, '--title', opts.tag, '--notes', opts.notes || `Release ${opts.tag}`], { stdio: 'pipe' }); } log(` ✓ release ${opts.tag} on ${opts.repo}`); @@ -69,6 +85,9 @@ export async function publishPlugin(opts: { step(4, 'Opening the registry PR…'); const { prUrl } = submitEntry(entry, { registry: opts.registry, draft: opts.draft }); log(' ✓ done'); - fs.rmSync(zip, { force: true }); + // Keep the artifact. It is the exact bytes the release and the entry's sha256 pin were + // computed from — a re-pack on another machine or SDK version can differ (CRLF, walk + // order), so anyone re-running `entry`/`sign` afterwards must hash THIS file, not a rebuild. + log(` artifact kept at ${zip}`); return { prUrl }; } diff --git a/plugin-sdk/src/cli/submit.ts b/plugin-sdk/src/cli/submit.ts index f2971ef59..32e28f9d2 100644 --- a/plugin-sdk/src/cli/submit.ts +++ b/plugin-sdk/src/cli/submit.ts @@ -61,7 +61,10 @@ export function submitEntry(entry: EntryLike, opts: { registry?: string; branch? } if (!cloned) throw new Error(`could not clone your fork ${login}/${name} (is the fork ready on GitHub?)`); - git(tmp, 'remote', 'add', 'upstream', `https://github.com/${registry}.git`); + // `gh repo clone` of a fork may already have wired an `upstream` remote, in which case a + // bare `remote add` exits non-zero and takes the whole submit down. Set it either way. + try { git(tmp, 'remote', 'add', 'upstream', `https://github.com/${registry}.git`); } + catch { git(tmp, 'remote', 'set-url', 'upstream', `https://github.com/${registry}.git`); } git(tmp, 'fetch', '--depth=1', 'upstream', 'main'); git(tmp, 'checkout', '-B', branch, 'upstream/main'); diff --git a/plugin-sdk/src/cli/trek-plugin.ts b/plugin-sdk/src/cli/trek-plugin.ts index 418c4b8e2..acb8952d3 100644 --- a/plugin-sdk/src/cli/trek-plugin.ts +++ b/plugin-sdk/src/cli/trek-plugin.ts @@ -3,6 +3,8 @@ * `trek-plugin ` — the plugin author CLI (#plugins). * * create [name] [--type t] [--interactive] scaffold a plugin (wizard if no name) + * [--template blank|notification-channel] type: integration|page|widget|trip-page + * [--egress host,host] [--required-addons a,b] * dev [dir] [--port 4317] run locally with hot reload * validate [dir] check the manifest + layout * pack [dir] [--out plugin.zip] [--json] build plugin.zip, print sha256 + size @@ -64,6 +66,43 @@ const { flags, pos } = parse(args); type Flags = Record; +/** + * Every flag each command reads. `parse()` accepts any `--x`, so a flag a command + * does NOT read used to be silently dropped — `create --template notification-channel` + * cheerfully scaffolded a blank plugin. Silently ignoring an author's explicit + * instruction is worse than refusing it, so unknown flags are now an error. + */ +const COMMAND_FLAGS: Record = { + create: ['type', 'interactive', 'author', 'description', 'permissions', 'template', 'egress', 'required-addons'], + dev: ['port'], + validate: [], + pack: ['out', 'json'], + keygen: ['key'], + sign: ['key'], + entry: ['repo', 'tag', 'dir', 'zip', 'commit', 'asset', 'merge', 'sign', 'key', 'out'], + preflight: ['repo', 'tag', 'entry', 'zip', 'commit', 'sign', 'key', 'all'], + submit: ['repo', 'tag', 'zip', 'commit', 'sign', 'key', 'registry', 'branch', 'draft', 'keep'], + release: ['repo', 'tag', 'out', 'notes', 'commit', 'merge', 'sign', 'key'], + publish: ['repo', 'tag', 'sign', 'key', 'registry', 'draft', 'notes', 'no-preflight', 'force'], +}; + +function assertKnownFlags(command: string, f: Flags): void { + const known = COMMAND_FLAGS[command]; + if (!known) return; + const unknown = Object.keys(f).filter((k) => !known.includes(k)); + if (unknown.length) { + const list = unknown.map((u) => `--${u}`).join(', '); + fail(`unknown flag${unknown.length > 1 ? 's' : ''} for \`${command}\`: ${list}\n ${command} accepts: ${known.map((k) => '--' + k).join(', ') || '(none)'}`); + } +} + +/** `--egress a,b` / `--permissions "a b"` → a string[]. */ +function listFlag(v: string | undefined): string[] | undefined { + if (!v || v === 'true') return undefined; + const parts = v.split(/[\s,]+/).filter(Boolean); + return parts.length ? parts : undefined; +} + /** --sign, --sign , or absent → the key path to sign with (or undefined). */ function signKey(f: Flags): string | undefined { if (!f.sign) return undefined; @@ -91,7 +130,41 @@ async function ensureRepoTag(f: Flags, failMsg: string): Promise<{ repo: string; const USAGE = 'usage: trek-plugin [...]'; +const HELP = `${USAGE} + + create [name] [--type integration|page|widget|trip-page] scaffold a plugin (wizard if no name) + [--template blank|notification-channel] [--interactive] + [--author a] [--description d] [--permissions "a b"] + [--egress host,host] [--required-addons a,b] + dev [dir] [--port 4317] run locally with hot reload + validate [dir] check the manifest + layout + pack [dir] [--out plugin.zip] [--json] build plugin.zip, print sha256 + size + keygen [--key file] create an Ed25519 signing key + sign [zip] [--key file] print a signature + public key for an artifact + entry --repo o/n --tag vX [--zip z] print the ready-to-PR registry entry + [--dir d] [--commit c] [--asset a] [--merge entry.json] [--sign [key]] [--out f] + preflight [dir] --repo o/n --tag vX run the registry CI checks locally + [--entry e] [--zip z] [--commit c] [--sign [key]] [--all] + submit [dir] --repo o/n --tag vX open the registry PR for you + [--sign [key]] [--registry o/n] [--branch b] [--draft] [--keep] + release [dir] --repo o/n --tag vX pack -> gh release -> print entry + [--out f] [--notes n] [--sign [key]] [--merge entry.json] + publish [dir] --repo o/n --tag vX the lot: pack -> tag+release -> + [--sign [key]] [--no-preflight] preflight -> open the registry PR + [--registry o/n] [--notes n] [--draft] + [--force] overwrite an existing release's artifact + (breaks the sha256 pin if it was ever merged) + +The goal: create -> dev -> publish, and never hand-compute sha256/size/commitSha +or hand-write the registry JSON.`; + async function main(): Promise { + // Before dispatch, so assertKnownFlags never sees `--help` and rejects it as unknown. + // Help is not an error: it goes to stdout and exits 0. + if (cmd === 'help' || cmd === '--help' || cmd === '-h' || flags.help) { + console.log(HELP); + return; + } if (!cmd) { // Bare invocation: a menu in a terminal, the usage line for scripts. if (!isInteractive()) { console.error(USAGE); process.exit(2); } @@ -104,16 +177,20 @@ async function main(): Promise { async function dispatch(command: string, f: Flags, positional: string[]): Promise { const tui = isInteractive(); + assertKnownFlags(command, f); if (command === 'create') { const name = positional[0]; if (!name || f.interactive) { - if (!tui) fail('create needs a plugin name in non-interactive mode: create [--type integration|page|widget]'); + if (!tui) fail('create needs a plugin name in non-interactive mode: create [--type integration|page|widget|trip-page]'); await interactiveScaffold(process.cwd(), name); return; } scaffold(name, f.type || 'integration', process.cwd(), { author: f.author, description: f.description, - permissions: f.permissions ? f.permissions.split(/[\s,]+/).filter(Boolean) : undefined, + permissions: listFlag(f.permissions), + egress: listFlag(f.egress), + requiredAddons: listFlag(f['required-addons']), + template: f.template as 'blank' | 'notification-channel' | undefined, }); console.log(`Created ${name}/ — build server/index.js, add docs/screenshot.png, then \`npx trek-plugin-sdk dev ${name}\`.`); } else if (command === 'dev') { @@ -216,7 +293,8 @@ async function dispatch(command: string, f: Flags, positional: string[]): Promis const { prUrl } = await publishPlugin({ dir: positional[0] || '.', repo, tag, signKeyPath: signKey(f), registry: f.registry, draft: !!f.draft, - notes: f.notes, skipPreflight: !!f['no-preflight'], now: new Date().toISOString(), + notes: f.notes, skipPreflight: !!f['no-preflight'], force: !!f.force, + now: new Date().toISOString(), log: tui ? clackLogSink : undefined, }); if (tui) logSuccess('Published — registry PR:'); else console.error('\n✓ published — registry PR:'); diff --git a/plugin-sdk/src/cli/validate.ts b/plugin-sdk/src/cli/validate.ts index d0aafffda..69b935e6f 100644 --- a/plugin-sdk/src/cli/validate.ts +++ b/plugin-sdk/src/cli/validate.ts @@ -7,7 +7,7 @@ */ import fs from 'node:fs'; import path from 'node:path'; -import { validateManifest, KNOWN_ADDONS } from '../manifest.js'; +import { validateManifest, isUnboundedRange, KNOWN_ADDONS } from '../manifest.js'; import { readJsonFile } from './json.js'; export interface ValidateReport { @@ -33,6 +33,13 @@ export function validatePluginDir(dir: string): ValidateReport { if (manifestId && path.basename(path.resolve(dir)) !== manifestId) { warnings.push(`directory name should equal the plugin id "${manifestId}"`); } + // A `trek` of "*" installs on every TREK ever released and every one still to come. + // It is legal — a plugin may honestly have no version-specific surface — but it is far + // more often an author who didn't want to think about it, and it opts them out of the + // one mechanism that stops their plugin running on a host it was never tested against. + if (isUnboundedRange(res.manifest?.trek ?? '')) { + warnings.push('trek: "*" claims support for every TREK version, including ones that do not exist yet — pin a range like ">=3.2.0 <4.0.0"'); + } // A well-formed but unrecognised addon id is not an error (the plugin may target // a newer TREK), but it can never enable here — surface it as a warning. for (const a of res.manifest?.requiredAddons ?? []) { diff --git a/plugin-sdk/src/cli/verify-signature.ts b/plugin-sdk/src/cli/verify-signature.ts new file mode 100644 index 000000000..ccfd0e13d --- /dev/null +++ b/plugin-sdk/src/cli/verify-signature.ts @@ -0,0 +1,125 @@ +/** + * Author-signature verification for `trek-plugin preflight`. + * + * This is a port of TREK's installer-side verifier (server/src/nest/plugins/install/ + * verify-signature.ts), and of the registry's port of it (TREK-Plugins' + * scripts/lib/verify-signature.mjs). All three MUST accept exactly the same inputs. + * + * The point of preflight is to tell an author what the registry's CI would say before + * they open the PR. `sign.ts`'s verifyArtifact only understands the bare key/signature + * pair the SDK itself emits, so it cannot answer that question for a minisign key — and + * a preflight that says "green" on a signature the registry would reject is worse than + * no preflight at all, because the author trusts it. + * + * node:crypto only, like the rest of the SDK. + */ +import crypto from 'node:crypto'; + +// SPKI DER prefix for a raw 32-byte Ed25519 public key (RFC 8410). +const ED25519_SPKI_PREFIX = Buffer.from('302a300506032b6570032100', 'hex'); + +export class SignatureError extends Error {} + +/** Wrap a raw 32-byte Ed25519 public key as a node KeyObject. */ +function ed25519KeyFromRaw(raw: Buffer): crypto.KeyObject { + if (raw.length !== 32) throw new SignatureError('public key is not a 32-byte Ed25519 key'); + return crypto.createPublicKey({ key: Buffer.concat([ED25519_SPKI_PREFIX, raw]), format: 'der', type: 'spki' }); +} + +/** + * Parse a minisign public key: either the two-line minisign format (comment + base64 + * payload) or a bare base64 payload. The 10-byte payload header is `Ed` + an 8-byte key + * id; the trailing 32 bytes are the key. A bare 32-byte base64 key (what `keygen` emits) + * is also accepted. + */ +function parseMinisignPubKey(pub: string): { key: crypto.KeyObject; keyId: Buffer | null } { + const line = pub.trim().split(/\r?\n/).map((l) => l.trim()).filter((l) => l && !l.startsWith('untrusted comment')).pop(); + if (!line) throw new SignatureError('empty public key'); + const buf = Buffer.from(line, 'base64'); + if (buf.length === 32) return { key: ed25519KeyFromRaw(buf), keyId: null }; + if (buf.length === 42 && buf.subarray(0, 2).toString('latin1') === 'Ed') { + return { key: ed25519KeyFromRaw(buf.subarray(10)), keyId: buf.subarray(2, 10) }; + } + throw new SignatureError('unrecognized public key format'); +} + +/** + * Parse a minisign signature: 2-byte algorithm (`Ed` legacy | `ED` prehashed) + 8-byte + * key id + 64-byte signature. A bare 64-byte base64 signature (what `sign` emits, over + * the raw artifact bytes) is also accepted. + */ +function parseMinisignSignature(sig: string): { algo: 'Ed' | 'ED' | 'raw'; keyId: Buffer | null; signature: Buffer } { + const line = sig.trim().split(/\r?\n/).map((l) => l.trim()).filter((l) => l && !l.startsWith('untrusted comment')).shift(); + if (!line) throw new SignatureError('empty signature'); + const buf = Buffer.from(line, 'base64'); + if (buf.length === 64) return { algo: 'raw', keyId: null, signature: buf }; + if (buf.length === 74) { + const algo = buf.subarray(0, 2).toString('latin1'); + if (algo !== 'Ed' && algo !== 'ED') throw new SignatureError(`unsupported signature algorithm ${algo}`); + return { algo, keyId: buf.subarray(2, 10), signature: buf.subarray(10) }; + } + throw new SignatureError('unrecognized signature format'); +} + +/** + * Verify that `bytes` were signed by the holder of `publicKeyB64`'s private key. Returns + * true on a valid signature, false on a well-formed but non-matching one, and throws + * SignatureError on malformed input. + */ +export function verifyAuthorSignature(bytes: Buffer, signatureB64: string, publicKeyB64: string): boolean { + const { key, keyId } = parseMinisignPubKey(publicKeyB64); + const { algo, keyId: sigKeyId, signature } = parseMinisignSignature(signatureB64); + + // If both carry a key id, they must name the same key (catches a wrong-key mixup). + if (keyId && sigKeyId && !keyId.equals(sigKeyId)) return false; + + const message = algo === 'ED' ? crypto.createHash('blake2b512').update(bytes).digest() : bytes; + try { + return crypto.verify(null, message, key, signature); + } catch { + return false; + } +} + +/** + * Structural check that needs no artifact (and so no network): a key and a signature must + * be both-present or both-absent, and each must parse. + * + * TREK refuses to install a half-signed entry ("incomplete signature: an author key and a + * version signature must both be present"), so such an entry is dead on arrival. Mirrors + * the registry's checkSignatureShape. Returns a list of problems (empty === fine). + */ +export function checkSignatureShape(entry: { authorPublicKey?: string; versions?: Array<{ version: string; signature?: string }> }): string[] { + const problems: string[] = []; + const key = entry.authorPublicKey; + const versions = Array.isArray(entry.versions) ? entry.versions : []; + + if (key) { + try { + parseMinisignPubKey(key); + } catch (e) { + problems.push(`authorPublicKey is not a valid Ed25519/minisign public key: ${(e as Error).message}`); + } + } + + for (const v of versions) { + if (v.signature && !key) { + problems.push(`${v.version}: has a signature but the entry has no authorPublicKey — TREK refuses to install a half-signed entry`); + } + if (v.signature) { + try { + parseMinisignSignature(v.signature); + } catch (e) { + problems.push(`${v.version}: signature is malformed: ${(e as Error).message}`); + } + } + } + + // A key with no signed version at all is a publishing mistake: TREK pins the key on + // first install and then requires every later version to be signed by it. + if (key && versions.length && !versions.some((v) => v.signature)) { + problems.push('authorPublicKey is set but no version carries a signature — either sign the release or drop the key'); + } + + return problems; +} diff --git a/plugin-sdk/src/index.ts b/plugin-sdk/src/index.ts index db1dbb951..a11db6a49 100644 --- a/plugin-sdk/src/index.ts +++ b/plugin-sdk/src/index.ts @@ -322,6 +322,13 @@ export interface PluginRequest { * providers; never Cookie/Authorization/session). Empty on authenticated routes. * Verify a provider signature against a secret you hold in `ctx.config`/`ctx.settings`. */ headers: Record; + /** + * The RAW request body, base64-encoded — set only on `auth:false` routes (webhooks), + * `null` elsewhere. This is what you must run an HMAC over: `body` above is the PARSED + * value, and re-serializing it will not reproduce the exact bytes the sender signed + * (key order, whitespace and unicode escaping all differ), so the signature won't match. + */ + rawBodyBase64?: string | null; user: { id: number; username: string; isAdmin: boolean } | null; } export interface PluginResponse { diff --git a/plugin-sdk/src/manifest.ts b/plugin-sdk/src/manifest.ts index ca49dc75a..4b31d7cbc 100644 --- a/plugin-sdk/src/manifest.ts +++ b/plugin-sdk/src/manifest.ts @@ -4,7 +4,7 @@ * result (no throw) so the CLI can print every problem at once. */ -import { validRange } from 'semver'; +import { minVersion, validRange } from 'semver'; export interface PluginDependency { id: string; @@ -15,6 +15,8 @@ export interface PluginManifest { name: string; version: string; apiVersion: number; + /** The semver RANGE of TREK versions this plugin supports (">=3.2.0 <4.0.0"). */ + trek: string; type: 'integration' | 'page' | 'widget' | 'trip-page'; permissions: string[]; egress: string[]; @@ -93,6 +95,28 @@ function isKnownPermission(p: string): boolean { return KNOWN_PERMISSIONS.includes(p) || p.startsWith('http:outbound:'); } +/** + * A semver range a plugin may declare in `trek`. Mirrors the server's isValidTrekRange. + * + * validRange() alone is not enough: ">=4.0.0 <3.0.0" is a valid range no version can ever + * satisfy, so a plugin declaring it would be uninstallable everywhere with nothing to + * explain why. minVersion() is null for exactly that, and throws on junk like "latest". + */ +export function isSatisfiableRange(r: unknown): r is string { + if (typeof r !== 'string' || !r.trim()) return false; + if (validRange(r) === null) return false; + try { + return minVersion(r) !== null; + } catch { + return false; + } +} + +/** A range that admits literally every TREK version, past and future ("*", "x", ">=0"). */ +export function isUnboundedRange(r: string): boolean { + return isSatisfiableRange(r) && validRange(r) === '*'; +} + export function validateManifest(raw: unknown): ValidationResult { const errors: string[] = []; const m = (raw && typeof raw === 'object' ? raw : {}) as Record; @@ -107,6 +131,15 @@ export function validateManifest(raw: unknown): ValidationResult { if (typeof m.id === 'string' && RESERVED_IDS.has(m.id)) errors.push(`id "${m.id}" is reserved`); if (typeof m.version === 'string' && !SEMVER_RE.test(m.version)) errors.push(`version "${m.version}" is not semver`); if (typeof m.type === 'string' && !TYPES.includes(m.type)) errors.push(`type must be one of ${TYPES.join('/')}`); + // TREK refuses to install a plugin that doesn't say which TREK versions it runs on, so + // an author must find that out here rather than from a rejected install. + if (!isSatisfiableRange(m.trek)) { + errors.push( + typeof m.trek === 'string' && m.trek + ? `"trek" is not a satisfiable semver range: "${m.trek}" (e.g. ">=3.2.0 <4.0.0")` + : 'missing "trek" — declare the TREK versions this plugin supports, e.g. ">=3.2.0 <4.0.0"', + ); + } if (m.apiVersion !== undefined && typeof m.apiVersion !== 'number') errors.push('apiVersion must be a number'); if (m.nativeModules === true) errors.push('native modules are not allowed (v1)'); @@ -230,6 +263,7 @@ export function validateManifest(raw: unknown): ValidationResult { name: m.name as string, version: m.version as string, apiVersion: typeof m.apiVersion === 'number' ? m.apiVersion : 1, + trek: m.trek as string, type: m.type as PluginManifest['type'], permissions, egress, diff --git a/plugin-sdk/test/cli.test.ts b/plugin-sdk/test/cli.test.ts index 43b0575d5..1c1424381 100644 --- a/plugin-sdk/test/cli.test.ts +++ b/plugin-sdk/test/cli.test.ts @@ -110,7 +110,7 @@ describe('scaffold + validate dependencies', () => { }); describe('validateManifest dependency rules', () => { - const base = { id: 'my-plug', name: 'My Plug', version: '1.0.0', type: 'integration', permissions: ['db:own'] }; + const base = { id: 'my-plug', name: 'My Plug', version: '1.0.0', type: 'integration', permissions: ['db:own'], trek: '>=3.2.0 <4.0.0' }; it('accepts valid requiredAddons + pluginDependencies', () => { const r = validateManifest({ ...base, requiredAddons: ['budget', 'journey'], pluginDependencies: [{ id: 'koffi', version: '>=1.0.0 <2.0.0' }] }); expect(r.ok).toBe(true); @@ -205,7 +205,7 @@ describe('notification-channel template', () => { describe('capabilities.notificationChannel validation', () => { const base = { id: 'chan', name: 'Chan', version: '1.0.0', apiVersion: 1, type: 'integration', - nativeModules: false, permissions: ['hook:notification-channel'], + nativeModules: false, permissions: ['hook:notification-channel'], trek: '>=3.2.0 <4.0.0', }; it('accepts a narrowed event list', () => { diff --git a/plugin-sdk/test/publish.test.ts b/plugin-sdk/test/publish.test.ts new file mode 100644 index 000000000..66b33ce4e --- /dev/null +++ b/plugin-sdk/test/publish.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +// publish shells out to git + gh; drive those from here so we can assert what it WOULD run. +const calls: Array<{ bin: string; args: string[] }> = []; +let releaseExistsOnRemote = false; + +vi.mock('node:child_process', () => ({ + execFileSync: (bin: string, args: string[]) => { + calls.push({ bin, args }); + if (bin === 'gh' && args[0] === 'release' && args[1] === 'view') { + if (!releaseExistsOnRemote) throw new Error('release not found'); + return Buffer.from(''); + } + // `git rev-parse ^{commit}` — pretend the tag does not exist yet. + if (bin === 'git' && args.includes('rev-parse')) throw new Error('unknown revision'); + return Buffer.from(''); + }, +})); + +const { publishPlugin } = await import('../src/cli/publish.js'); +const { scaffold } = await import('../src/cli/create.js'); + +describe('publish — a released artifact is immutable', () => { + let tmp: string; + let dir: string; + + beforeEach(() => { + calls.length = 0; + releaseExistsOnRemote = false; + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'publish-')); + scaffold('immutable-plug', 'integration', tmp); + dir = path.join(tmp, 'immutable-plug'); + }); + afterEach(() => { fs.rmSync(tmp, { recursive: true, force: true }); }); + + const run = (force?: boolean) => publishPlugin({ + dir, repo: 'someone/trek-plugin-immutable-plug', tag: 'v1.0.0', + now: '2026-01-01T00:00:00Z', skipPreflight: true, force, log: () => {}, + }); + + it('REFUSES to overwrite an existing release, and never uploads', async () => { + releaseExistsOnRemote = true; + // The registry pins the artifact's sha256. Rewriting the bytes of a release that is + // already in the registry breaks that pin for everyone who installed that version. + await expect(run()).rejects.toThrow(/already exists/i); + expect(calls.some((c) => c.bin === 'gh' && c.args.includes('upload'))).toBe(false); + expect(calls.some((c) => c.bin === 'gh' && c.args.includes('--clobber'))).toBe(false); + }); + + it('--force overwrites deliberately (for a release never merged into the registry)', async () => { + releaseExistsOnRemote = true; + await run(true).catch(() => {}); // submit/preflight are stubbed out; we only care about the gh calls + const upload = calls.find((c) => c.bin === 'gh' && c.args.includes('upload')); + expect(upload?.args).toContain('--clobber'); + }); + + it('creates the release normally when it does not exist yet', async () => { + releaseExistsOnRemote = false; + await run().catch(() => {}); + expect(calls.some((c) => c.bin === 'gh' && c.args[1] === 'create')).toBe(true); + expect(calls.some((c) => c.args.includes('--clobber'))).toBe(false); + }); + + it('keeps plugin.zip — the entry sha256 must be hashed from the uploaded bytes', async () => { + await run().catch(() => {}); + // A re-pack on another machine/SDK version can produce different bytes (CRLF, walk + // order), so deleting the artifact after publishing made a follow-up `entry`/`sign` wrong. + expect(fs.existsSync(path.join(dir, 'plugin.zip'))).toBe(true); + }); +}); diff --git a/plugin-sdk/test/sdk.test.ts b/plugin-sdk/test/sdk.test.ts index 802a60ffd..f5eabec73 100644 --- a/plugin-sdk/test/sdk.test.ts +++ b/plugin-sdk/test/sdk.test.ts @@ -5,6 +5,7 @@ import path from 'node:path'; import zlib from 'node:zlib'; import { createRequire } from 'node:module'; import { definePlugin, PLUGIN_API_VERSION, validateManifest, createMockHost } from '../src/index.js'; +import { isUnboundedRange } from '../src/manifest.js'; import { scaffold } from '../src/cli/create.js'; import { validatePluginDir } from '../src/cli/validate.js'; import { makeZip, listZipNames } from '../src/zip.js'; @@ -44,7 +45,7 @@ describe('definePlugin + api version', () => { }); describe('validateManifest', () => { - const base = { id: 'flight-tracker', name: 'Flight', version: '1.0.0', type: 'widget', apiVersion: 1 }; + const base = { id: 'flight-tracker', name: 'Flight', version: '1.0.0', type: 'widget', apiVersion: 1, trek: '>=3.2.0 <4.0.0' }; it('accepts a valid manifest', () => { expect(validateManifest(base).ok).toBe(true); }); @@ -587,14 +588,14 @@ describe('pack + entry (publishing automation)', () => { it('refuses a plugin that ships a native binary', () => { const bad = path.join(tmp, 'bad'); fs.mkdirSync(path.join(bad, 'server'), { recursive: true }); - fs.writeFileSync(path.join(bad, 'trek-plugin.json'), JSON.stringify({ id: 'bad-plug', name: 'Bad', version: '1.0.0', type: 'integration', permissions: [], egress: [] })); + fs.writeFileSync(path.join(bad, 'trek-plugin.json'), JSON.stringify({ id: 'bad-plug', name: 'Bad', version: '1.0.0', type: 'integration', permissions: [], egress: [], trek: '>=3.2.0 <4.0.0' })); fs.writeFileSync(path.join(bad, 'server', 'index.js'), 'module.exports={}'); fs.writeFileSync(path.join(bad, 'server', 'thing.node'), Buffer.from([1, 2, 3])); fs.writeFileSync(path.join(bad, 'README.md'), '# Bad\n![x](x.png)\ncontent'); expect(() => packPluginDir(bad, path.join(tmp, 'x.zip'))).toThrow(/native binaries/); }); - it('builds a registry entry with sha256/size/commit/minTrekVersion filled in', () => { + it('builds a registry entry with sha256/size/commit/trek filled in', () => { const out = path.join(tmp, 'plugin.zip'); const packed = packPluginDir(koffi, out); const entry = buildEntry({ @@ -607,11 +608,43 @@ describe('pack + entry (publishing automation)', () => { expect(v.sha256).toBe(packed.sha256); expect(v.size).toBe(packed.size); expect(v.commitSha).toBe('a'.repeat(40)); - expect(v.minTrekVersion).toBe('3.2.0'); + // The RAW range is the one compatibility field an entry carries. minTrekVersion is gone: + // it restated the range's lower bound in a weaker form (it cannot express the exclusive + // upper bound), so the entry used to drop "<4.0.0" silently and a TREK 4 server read + // "requires 3.2.0+" and considered the plugin compatible. + expect(v.trek).toBe('>=3.2.0 <4.0.0'); + expect(v).not.toHaveProperty('minTrekVersion'); expect(v.downloadUrl).toBe('https://github.com/mauriceboe/trek-plugin-koffi/releases/download/v1.0.0/plugin.zip'); expect(v.nativeModules).toBe(false); }); + it('publishes an upper-bound-only range verbatim, with no floor to get wrong', () => { + // This is why the derived floor is gone. "<4.0.0" has a first X.Y.Z of 4.0.0 but a lower + // bound of 0.0.0, and the old regex published 4.0.0 as the MINIMUM — advertising a plugin + // that supports everything BELOW 4 as requiring 4+, the precise inverse of what its author + // wrote. A field that only ever restates the range cannot be wrong if it does not exist. + const entry = buildEntry({ ...entryOptsFor('upper-only', '<4.0.0'), now: '2026-07-04T00:00:00.000Z' }); + expect(entry.versions[0].trek).toBe('<4.0.0'); + expect(entry.versions[0]).not.toHaveProperty('minTrekVersion'); + }); + + it('refuses to publish a plugin with no usable trek range', () => { + expect(() => buildEntry({ ...entryOptsFor('no-trek', undefined), now: '2026-07-04T00:00:00.000Z' })) + .toThrow(/no valid "trek" version range/); + expect(() => buildEntry({ ...entryOptsFor('bad-trek', '>=4.0.0 <3.0.0'), now: '2026-07-04T00:00:00.000Z' })) + .toThrow(/no valid "trek" version range/); + }); + + /** A minimal plugin dir + artifact for buildEntry (which only needs the zip to exist). */ + function entryOptsFor(id: string, trek: string | undefined) { + const dir = path.join(tmp, id); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'trek-plugin.json'), JSON.stringify({ id, name: id, version: '1.0.0', type: 'integration', ...(trek === undefined ? {} : { trek }) })); + const zipPath = path.join(tmp, `${id}.zip`); + fs.writeFileSync(zipPath, makeZip([{ name: 'trek-plugin.json', data: Buffer.from('{}') }])); + return { dir, repo: 'a/b', tag: 'v1.0.0', zipPath, commit: 'a'.repeat(40) }; + } + it('--merge prepends the new version onto an existing entry, newest-first', () => { const out = path.join(tmp, 'plugin.zip'); packPluginDir(koffi, out); @@ -808,7 +841,7 @@ describe('mock-host inter-plugin (plugins.call + events.emit)', () => { }); describe('validateManifest capabilities.provides/emits', () => { - const base = { id: 'my-plug', name: 'My Plug', version: '1.0.0', type: 'integration', permissions: ['db:own'] }; + const base = { id: 'my-plug', name: 'My Plug', version: '1.0.0', type: 'integration', permissions: ['db:own'], trek: '>=3.2.0 <4.0.0' }; it('accepts well-formed provides + emits', () => { const r = validateManifest({ ...base, capabilities: { provides: ['computeRate'], emits: ['rate.updated'] } }); expect(r.ok).toBe(true); @@ -818,3 +851,37 @@ describe('validateManifest capabilities.provides/emits', () => { expect(validateManifest({ ...base, capabilities: { emits: 'nope' } }).ok).toBe(false); }); }); + +/** + * The `trek` range is what TREK gates installs on, so it has to be caught HERE — the same + * validateManifest the registry CI runs — rather than by a rejected install. + */ +describe('validateManifest: the trek range', () => { + const base = { id: 'my-plug', name: 'My Plug', version: '1.0.0', type: 'integration', permissions: ['db:own'] }; + + it('requires a range', () => { + const r = validateManifest({ ...base }); + expect(r.ok).toBe(false); + expect(r.errors.join()).toMatch(/missing "trek"/); + }); + + it('rejects a range that is not semver, and one nothing can satisfy', () => { + expect(validateManifest({ ...base, trek: '3.2+' }).ok).toBe(false); + // Syntactically valid, semantically empty: no TREK could ever run it. + expect(validateManifest({ ...base, trek: '>=4.0.0 <3.0.0' }).ok).toBe(false); + }); + + it('accepts the ranges authors really write, and carries the range through', () => { + for (const trek of ['>=3.2.0 <4.0.0', '^3.2.0', '>=3']) { + const r = validateManifest({ ...base, trek }); + expect(r.ok, trek).toBe(true); + expect(r.manifest?.trek).toBe(trek); + } + }); + + it('allows "*" (it is a legal claim) but validate warns — it is nearly always a lie', () => { + expect(validateManifest({ ...base, trek: '*' }).ok).toBe(true); + expect(isUnboundedRange('*')).toBe(true); + expect(isUnboundedRange('>=3.2.0 <4.0.0')).toBe(false); + }); +}); diff --git a/plugin-sdk/test/verify-signature.test.ts b/plugin-sdk/test/verify-signature.test.ts new file mode 100644 index 000000000..c7eda08ec --- /dev/null +++ b/plugin-sdk/test/verify-signature.test.ts @@ -0,0 +1,103 @@ +/** + * The SDK's author-signature verifier. + * + * It exists so `preflight` can answer the question the REGISTRY's CI will ask, before the + * author opens the PR. That makes agreement with the other two implementations the entire + * point: TREK's host verifier (server/src/nest/plugins/install/verify-signature.ts) and the + * registry's port of it (TREK-Plugins scripts/lib/verify-signature.mjs). A gate that is + * merely *similar* is worse than none — a green preflight is trusted, and an entry the + * registry rejects is one nobody can install. + */ +import { describe, it, expect } from 'vitest'; +import crypto from 'node:crypto'; +import { verifyAuthorSignature, checkSignatureShape, SignatureError } from '../src/cli/verify-signature.js'; +import { generateKeypair, signArtifact, publicKeyBase64, loadPrivateKey } from '../src/cli/sign.js'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +/** A throwaway Ed25519 keypair, via the SDK's own keygen (not a hand-rolled one). */ +function keypair(): { key: crypto.KeyObject; pub: string } { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-sig-')); + const keyPath = path.join(dir, 'signing.key'); + generateKeypair(keyPath); + const key = loadPrivateKey(keyPath); + return { key, pub: publicKeyBase64(key) }; +} + +const BYTES = Buffer.from('a plugin artifact, pretend this is a zip'); + +describe('verifyAuthorSignature', () => { + // The round-trip that actually ships: `sign` emits a bare 64-byte signature over the raw + // bytes and `keygen` a bare 32-byte key, and all three verifiers must accept that pair. + it('accepts what the SDK itself signs', () => { + const { key, pub } = keypair(); + expect(verifyAuthorSignature(BYTES, signArtifact(BYTES, key), pub)).toBe(true); + }); + + it('rejects a signature over DIFFERENT bytes (a re-pack after signing)', () => { + const { key, pub } = keypair(); + const sig = signArtifact(BYTES, key); + expect(verifyAuthorSignature(Buffer.concat([BYTES, Buffer.from('!')]), sig, pub)).toBe(false); + }); + + it('rejects a signature made with a DIFFERENT key', () => { + const a = keypair(); + const b = keypair(); + expect(verifyAuthorSignature(BYTES, signArtifact(BYTES, a.key), b.pub)).toBe(false); + }); + + // A well-formed-but-wrong signature returns false; a MALFORMED one throws. The host draws + // exactly this line (SignatureError vs a false return), and the registry mirrors it — both + // end up refusing the install, but only the throw carries a "your key is garbage" message. + it('throws SignatureError on a malformed key or signature, rather than returning false', () => { + const { key, pub } = keypair(); + const sig = signArtifact(BYTES, key); + expect(() => verifyAuthorSignature(BYTES, sig, 'not-base64-at-all!!')).toThrow(SignatureError); + expect(() => verifyAuthorSignature(BYTES, 'AAAA', pub)).toThrow(SignatureError); + expect(() => verifyAuthorSignature(BYTES, sig, '')).toThrow(SignatureError); + }); + + // The two-line minisign form, which an author may paste in from a `minisign` keypair. The + // comment line must be skipped rather than parsed as the payload. + it('accepts a minisign-style key with an "untrusted comment" line', () => { + const { key, pub } = keypair(); + const sig = signArtifact(BYTES, key); + const twoLine = `untrusted comment: minisign public key ABC\n${pub}\n`; + expect(verifyAuthorSignature(BYTES, sig, twoLine)).toBe(true); + }); +}); + +describe('checkSignatureShape', () => { + it('passes an entry that is unsigned throughout, and one that is fully signed', () => { + const { key, pub } = keypair(); + const sig = signArtifact(BYTES, key); + expect(checkSignatureShape({ versions: [{ version: '1.0.0' }] })).toEqual([]); + expect(checkSignatureShape({ authorPublicKey: pub, versions: [{ version: '1.0.0', signature: sig }] })).toEqual([]); + }); + + // Both halves of a half-signed entry. TREK refuses to install either ("incomplete + // signature: an author key and a version signature must both be present"), so merging one + // ships an entry nobody can use — the point is to catch it before the PR, not at review. + it('refuses a signature with no key', () => { + const { key } = keypair(); + const problems = checkSignatureShape({ versions: [{ version: '1.0.0', signature: signArtifact(BYTES, key) }] }); + expect(problems).toHaveLength(1); + expect(problems[0]).toMatch(/no authorPublicKey/); + }); + + it('refuses a key with no signed version', () => { + const { pub } = keypair(); + const problems = checkSignatureShape({ authorPublicKey: pub, versions: [{ version: '1.0.0' }] }); + expect(problems).toHaveLength(1); + expect(problems[0]).toMatch(/no version carries a signature/); + }); + + it('reports a malformed key and a malformed signature by name', () => { + const { key, pub } = keypair(); + expect(checkSignatureShape({ authorPublicKey: 'nonsense', versions: [{ version: '1.0.0', signature: signArtifact(BYTES, key) }] })[0]) + .toMatch(/authorPublicKey is not a valid/); + expect(checkSignatureShape({ authorPublicKey: pub, versions: [{ version: '2.1.0', signature: 'AAAA' }] })[0]) + .toMatch(/2\.1\.0: signature is malformed/); + }); +}); diff --git a/server/package.json b/server/package.json index c06d00bb0..b68e23bce 100644 --- a/server/package.json +++ b/server/package.json @@ -22,7 +22,7 @@ "test:coverage": "vitest run --coverage" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.28.0", + "@modelcontextprotocol/sdk": "^1.29.0", "@nestjs/common": "^11.1.24", "@nestjs/core": "^11.1.24", "@nestjs/platform-express": "^11.1.24", diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index a8e958772..785d3a835 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -3626,6 +3626,40 @@ function runMigrations(db: Database.Database): void { ); `); }, + + // Why a plugin's update was REFUSED by the signature check (#plugins). A refused + // update leaves a working plugin pinned at its old version — previously the reason + // lived only in a transient toast, so the plugin quietly stopped updating and the + // admin had to re-attempt an update to rediscover why. Record it instead. + // + // `update_block_version` is the registry version that was refused: once the registry + // offers something NEWER, the block describes an artifact nobody is being offered + // anymore, so it reads as stale and the admin can just re-attempt (the next install + // re-verifies and either succeeds or re-blocks with fresh values). Deliberately no + // `status = 'error'` — the plugin still runs fine on its old code. + () => { + for (const col of ['update_block_code TEXT', 'update_block_detail TEXT', 'update_block_version TEXT']) { + try { + db.exec(`ALTER TABLE plugins ADD COLUMN ${col};`); + } catch (err) { + console.warn('[migrations] Non-fatal migration step failed:', err); + } + } + }, + + // The semver RANGE of TREK versions a plugin declares it supports (its manifest's + // `trek`, e.g. ">=3.2.0 <4.0.0"). The existing `min_trek_version` only carries the + // lower bound, so it cannot express "stops working at 4.0" — which is precisely the + // case the activation gate has to catch after a TREK upgrade. Kept nullable: a plugin + // installed before this column existed has no range recorded, and the gate refuses to + // activate it rather than guessing (see TREK_VERSION_UNKNOWN). + () => { + try { + db.exec('ALTER TABLE plugins ADD COLUMN trek_range TEXT;'); + } catch (err) { + console.warn('[migrations] Non-fatal migration step failed:', err); + } + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/mcp/index.ts b/server/src/mcp/index.ts index a86ca7f15..d7c85bae0 100644 --- a/server/src/mcp/index.ts +++ b/server/src/mcp/index.ts @@ -9,7 +9,7 @@ import { isAddonEnabled } from '../services/adminService'; import { ADDON_IDS } from '../addons'; import { registerResources } from './resources'; import { registerTools } from './tools'; -import { McpSession, sessions, revokeUserSessions, revokeUserSessionsForClient } from './sessionManager'; +import { McpSession, sessions, revokeUserSessions, revokeUserSessionsForClient, evictOldestSessionForUser } from './sessionManager'; import { resolveSessionTtlMs, resolveKeepaliveMs } from './config'; import { writeAudit, getClientIp } from '../services/auditLog'; import { getMcpSafeUrl } from '../services/notifications'; @@ -183,6 +183,12 @@ const sessionSweepInterval = setInterval(() => { // Prevent the interval from keeping the process alive if nothing else is running sessionSweepInterval.unref(); +/** JSON-RPC 2.0 error body. MCP clients parse this and show `message`; a bare `{ error }` + * envelope isn't valid JSON-RPC, so they fall back to a generic "tool execution failed". */ +function jsonRpcError(message: string, code = -32000): object { + return { jsonrpc: '2.0', error: { code, message }, id: null }; +} + function setAuthChallenge(res: Response, error = 'invalid_token'): void { const base = (getMcpSafeUrl() || '').replace(/\/+$/, ''); // RFC 9728 §5: resource with path component /mcp → PRM URL must include the path @@ -304,9 +310,25 @@ export async function mcpHandler(req: Request, res: Response): Promise { return; } + // A client that reuses its session sends the header on every call after initialize, so a + // steady stream of session-less POSTs means the id isn't making it back to the client — + // usually a reverse proxy dropping Mcp-Session-Id, or a client that ignores it. Say so, + // because the visible symptom (sessions piling up to the cap) points nowhere near the cause. + console.warn( + `[MCP] POST without mcp-session-id for user ${user.id} — starting a new session. ` + + 'If this repeats on every tool call, the Mcp-Session-Id response header is not reaching ' + + 'the client (check that your reverse proxy forwards it).', + ); + + // At the cap, evict this user's coldest session rather than refusing the request: a client + // stuck re-initializing would otherwise be locked out until the process restarted. if (countSessionsForUser(user.id) >= MAX_SESSIONS_PER_USER) { - res.status(429).json({ error: 'Session limit reached. Close an existing session before opening a new one.' }); - return; + const evicted = evictOldestSessionForUser(user.id); + if (!evicted) { + res.status(429).json(jsonRpcError('Session limit reached. Close an existing session before opening a new one.')); + return; + } + console.log(`[MCP] Session limit (${MAX_SESSIONS_PER_USER}) reached for user ${user.id} — evicted idle session ${evicted}`); } // Create a new per-user MCP server and session @@ -348,6 +370,10 @@ export async function mcpHandler(req: Request, res: Response): Promise { sessions.delete(sid); }, }); + // Belt-and-braces: whenever the SDK closes the transport for any reason, drop the map entry. + transport.onclose = () => { + if (transport.sessionId) sessions.delete(transport.sessionId); + }; logToolCallAudit(req, user.id, clientId); armSseKeepalive(res); @@ -359,6 +385,15 @@ export async function mcpHandler(req: Request, res: Response): Promise { if (!res.headersSent) { res.status(500).json({ error: 'Internal MCP error' }); } + } finally { + // Only an `initialize` request assigns a sessionId (via onsessioninitialized). Any other + // session-less POST is rejected by the SDK with "Server not initialized" — and this server, + // with its ~200 registered tools, would otherwise be orphaned: never in `sessions`, never + // swept, never closed. Reap it here. + if (!transport.sessionId) { + try { server.close(); } catch { /* ignore */ } + try { transport.close(); } catch { /* ignore */ } + } } } diff --git a/server/src/mcp/sessionManager.ts b/server/src/mcp/sessionManager.ts index 276a15730..cad993b5e 100644 --- a/server/src/mcp/sessionManager.ts +++ b/server/src/mcp/sessionManager.ts @@ -16,14 +16,18 @@ export interface McpSession { export const sessions = new Map(); +/** Close both halves of a session and drop it from the map. Close errors are non-fatal: + * a transport whose socket is already gone must not block eviction of the map entry. */ +function closeSession(sid: string, session: McpSession): void { + try { session.server.close(); } catch { /* ignore */ } + try { session.transport.close(); } catch { /* ignore */ } + sessions.delete(sid); +} + /** Terminate all active MCP sessions for a specific user (e.g. on token revocation). */ export function revokeUserSessions(userId: number): void { for (const [sid, session] of sessions) { - if (session.userId === userId) { - try { session.server.close(); } catch { /* ignore */ } - try { session.transport.close(); } catch { /* ignore */ } - sessions.delete(sid); - } + if (session.userId === userId) closeSession(sid, session); } } @@ -32,10 +36,31 @@ export function revokeUserSessions(userId: number): void { * sessions are closed, not sessions from other clients for the same user. */ export function revokeUserSessionsForClient(userId: number, clientId: string): void { for (const [sid, session] of sessions) { - if (session.userId === userId && session.clientId === clientId) { - try { session.server.close(); } catch { /* ignore */ } - try { session.transport.close(); } catch { /* ignore */ } - sessions.delete(sid); - } + if (session.userId === userId && session.clientId === clientId) closeSession(sid, session); } } + +/** + * Close the least-recently-active session for a user so a new one can take its slot, + * and return its id (null when the user has no sessions). + * + * This is what keeps the per-user cap from becoming a dead end. A client that cannot + * persist its Mcp-Session-Id — a proxy stripping the header, a non-conformant client — + * re-initializes on every call, and a hard cap would leave the user permanently unable + * to open a session until the process restarted. Evicting the coldest session instead + * means the worst case is a bounded ring of sessions, not a wedged integration. + */ +export function evictOldestSessionForUser(userId: number): string | null { + let oldestSid: string | null = null; + let oldestSession: McpSession | null = null; + for (const [sid, session] of sessions) { + if (session.userId !== userId) continue; + if (!oldestSession || session.lastActivity < oldestSession.lastActivity) { + oldestSid = sid; + oldestSession = session; + } + } + if (!oldestSid || !oldestSession) return null; + closeSession(oldestSid, oldestSession); + return oldestSid; +} diff --git a/server/src/middleware/globalMiddleware.ts b/server/src/middleware/globalMiddleware.ts index e430b0f14..74240b6c8 100644 --- a/server/src/middleware/globalMiddleware.ts +++ b/server/src/middleware/globalMiddleware.ts @@ -82,6 +82,14 @@ export function applyGlobalMiddleware( // answer OPTIONS without Access-Control-Allow-Origin before the SDK's own cors() runs. // All /.well-known/* paths get open CORS so clients probing openid-configuration or the // RFC 8414 path-suffixed AS metadata form don't get CORS-blocked (they get 404 JSON instead). + // + // `exposedHeaders` is load-bearing, not cosmetic. Without Access-Control-Expose-Headers the + // Fetch spec forbids a browser-context client (Claude Desktop connectors, Claude.ai, MCP + // Inspector) from *reading* Mcp-Session-Id off the initialize response — so it can never echo + // the header back, every request looks like a fresh initialize, and the server mints a new + // session per tool call until the per-user cap wedges the connection. Same reasoning for + // WWW-Authenticate, which carries the RFC 9728 resource-metadata challenge that drives + // OAuth discovery. app.use( (req: Request, _res: Response, next: NextFunction) => { if ( @@ -91,7 +99,11 @@ export function applyGlobalMiddleware( req.path === '/oauth/userinfo' || req.path === '/mcp' ) { - cors({ origin: '*', credentials: false })(req, _res, next); + cors({ + origin: '*', + credentials: false, + exposedHeaders: ['Mcp-Session-Id', 'MCP-Protocol-Version', 'WWW-Authenticate'], + })(req, _res, next); } else { next(); } diff --git a/server/src/nest/plugins/host/rpc-host.ts b/server/src/nest/plugins/host/rpc-host.ts index 3b5501290..3db712c32 100644 --- a/server/src/nest/plugins/host/rpc-host.ts +++ b/server/src/nest/plugins/host/rpc-host.ts @@ -303,8 +303,11 @@ export class PluginRpcHost { this.methods.set('trips.getById', (p, uid) => this.tripRead(p, uid, () => deps.db.prepare('SELECT * FROM trips WHERE id = ?').get(num(p.tripId, 'tripId'))), ); + // The trip's place POOL — places carry no itinerary position of their own + // (day_id/order_index live on day_assignments), so order by created_at like + // the REST list does. Use trips.getDays for the day-ordered itinerary. this.methods.set('trips.getPlaces', (p, uid) => - this.tripRead(p, uid, () => deps.db.prepare('SELECT * FROM places WHERE trip_id = ? ORDER BY day_id, position').all(num(p.tripId, 'tripId'))), + this.tripRead(p, uid, () => deps.db.prepare('SELECT * FROM places WHERE trip_id = ? ORDER BY created_at DESC').all(num(p.tripId, 'tripId'))), ); // Hydrated like the REST list (endpoints, day_positions, joins, normalized // accommodation_id) — a strict superset of the raw row, so older callers diff --git a/server/src/nest/plugins/install/discovery.ts b/server/src/nest/plugins/install/discovery.ts index 7e28100c1..2e000926d 100644 --- a/server/src/nest/plugins/install/discovery.ts +++ b/server/src/nest/plugins/install/discovery.ts @@ -67,15 +67,15 @@ function upsert(db: BetterSqlite3.Database, m: PluginManifest): void { if (existing) { db.prepare( `UPDATE plugins SET name = ?, description = ?, type = ?, icon = ?, version = ?, api_version = ?, - min_trek_version = ?, permissions = ?, capabilities = ?, dependencies = ?, operator_egress = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?`, - ).run(m.name, m.description ?? null, m.type, m.icon ?? 'Blocks', m.version, m.apiVersion, m.minTrekVersion ?? null, JSON.stringify(m.permissions), JSON.stringify(m.capabilities), dependencies, m.operatorEgress ? 1 : 0, m.id); + min_trek_version = ?, trek_range = ?, permissions = ?, capabilities = ?, dependencies = ?, operator_egress = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?`, + ).run(m.name, m.description ?? null, m.type, m.icon ?? 'Blocks', m.version, m.apiVersion, m.minTrekVersion ?? null, m.trekRange, JSON.stringify(m.permissions), JSON.stringify(m.capabilities), dependencies, m.operatorEgress ? 1 : 0, m.id); } else { db.prepare( // granted_permissions '' (empty, not '[]') marks "never consented" so the // first activation is distinguishable from a plugin consented to zero perms. - `INSERT INTO plugins (id, name, description, type, icon, version, api_version, min_trek_version, permissions, capabilities, dependencies, operator_egress, granted_permissions, status) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, '', 'inactive')`, - ).run(m.id, m.name, m.description ?? null, m.type, m.icon ?? 'Blocks', m.version, m.apiVersion, m.minTrekVersion ?? null, JSON.stringify(m.permissions), JSON.stringify(m.capabilities), dependencies, m.operatorEgress ? 1 : 0); + `INSERT INTO plugins (id, name, description, type, icon, version, api_version, min_trek_version, trek_range, permissions, capabilities, dependencies, operator_egress, granted_permissions, status) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, '', 'inactive')`, + ).run(m.id, m.name, m.description ?? null, m.type, m.icon ?? 'Blocks', m.version, m.apiVersion, m.minTrekVersion ?? null, m.trekRange, JSON.stringify(m.permissions), JSON.stringify(m.capabilities), dependencies, m.operatorEgress ? 1 : 0); } // Refresh the settings-page action descriptors from the manifest. diff --git a/server/src/nest/plugins/install/host-compat.ts b/server/src/nest/plugins/install/host-compat.ts new file mode 100644 index 000000000..bb11c8539 --- /dev/null +++ b/server/src/nest/plugins/install/host-compat.ts @@ -0,0 +1,106 @@ +import semver from 'semver'; + +/** + * Host-version compatibility for plugins (#plugins). A manifest declares the TREK + * versions it supports as a semver RANGE (`"trek": ">=3.2.0 <4.0.0"`); this module + * is the single place that answers "what TREK is running" and "does that range admit + * it". Every install front door (registry, sideload, dev-link) and the activation + * gate go through here, so a plugin can never run against a host it says it doesn't + * support — and, just as importantly, they all answer the question the same way. + */ + +/** The running TREK version (same source as the rest of the app). */ +export function hostVersion(): string { + return process.env.APP_VERSION || (require('../../../../package.json') as { version: string }).version; +} + +let warnedUnparseable = false; + +/** + * The running version as a plain release triple, or null when it can't be parsed at all. + * + * Prereleases are deliberately COERCED to their target release: 3.4.0-rc.1 compares as + * 3.4.0, and 4.0.0-rc.1 as 4.0.0. Both halves of that matter, and semver's own ordering + * gets both wrong for this purpose: + * + * - Plain `satisfies()` excludes a prerelease from any range that doesn't name one, so + * 3.4.0-rc.1 would fail ">=3.2.0 <4.0.0" and EVERY plugin on the instance would go + * incompatible the moment TREK shipped a release candidate. + * - `includePrerelease` fixes that but orders 4.0.0-rc.1 BEFORE 4.0.0, so it satisfies + * "<4.0.0" — letting a plugin that disclaims TREK 4 load on a build that already + * carries TREK 4's breaking changes. That is the precise failure this gate exists to + * prevent. + * + * Coercing to the release says the useful thing instead: for compatibility purposes, an + * rc of 4.0 IS 4.0. + * + * A null host NEVER blocks anything (see {@link hostSatisfies}). This is deliberate: + * APP_VERSION is a Docker build ARG that defaults to the literal string `dev`, and an + * unversioned build must stay fully usable. The warning exists so that a MISCONFIGURED + * APP_VERSION in production — which silently switches this whole gate off — is visible + * in the logs instead of being discovered when an incompatible plugin misbehaves. + */ +export function normalizedHost(): string | null { + const raw = hostVersion(); + const coerced = semver.coerce(raw)?.version ?? null; + if (!coerced && !warnedUnparseable) { + warnedUnparseable = true; + console.warn(`[plugins] APP_VERSION "${raw}" is not a semver version — plugin TREK-compatibility checks are disabled`); + } + return coerced; +} + +/** Test hook: re-arm the one-time unparseable-host warning. */ +export function __resetHostWarningForTests(): void { + warnedUnparseable = false; +} + +/** + * Whether `r` is a semver range a plugin may declare. + * + * `semver.validRange` alone is not enough: ">=4.0.0 <3.0.0" is a VALID range that no + * version can ever satisfy, and a plugin that declares it would be uninstallable with + * no way to tell the author why. minVersion() returns null for exactly that case (and + * throws outright on garbage like "latest"), so it is the real satisfiability test. + */ +export function isValidTrekRange(r: unknown): r is string { + if (typeof r !== 'string' || !r.trim()) return false; + if (semver.validRange(r) === null) return false; + try { + return semver.minVersion(r) !== null; + } catch { + return false; + } +} + +/** + * The lowest TREK version a range admits — what the UI shows as "Requires TREK x+". + * Undefined for a range with no real lower bound (`*`, `<4.0.0`), because "requires + * TREK 0.0.0+" is noise rather than information. + */ +export function minTrekOf(range: string): string | undefined { + if (!isValidTrekRange(range)) return undefined; + const min = semver.minVersion(range); + if (!min || min.version === '0.0.0') return undefined; + return min.version; +} + +/** + * Whether the running TREK satisfies `range`. The host arrives here already coerced to a + * release triple (see {@link normalizedHost}), which is what makes a plain satisfies() + * correct on both prerelease edges. + * + * A null/empty range is not a decision this function can make — the caller knows whether + * an undeclared range means "reject" (an install) or "unknown" (an already-installed + * plugin), and they surface different codes. It returns false so neither can pass by + * accident. + */ +export function hostSatisfies(range: string | null | undefined, host: string | null = normalizedHost()): boolean { + // Coerce again rather than trusting the argument: callers that pass a host explicitly + // (the registry, tests) would otherwise skip the prerelease normalisation and get a + // different answer than the default path for the same TREK. + const release = host === null ? null : (semver.coerce(host)?.version ?? null); + if (release === null) return true; // unversioned build — never block + if (!isValidTrekRange(range)) return false; + return semver.satisfies(release, range); +} diff --git a/server/src/nest/plugins/install/manifest.ts b/server/src/nest/plugins/install/manifest.ts index 2009070cd..98f59ec53 100644 --- a/server/src/nest/plugins/install/manifest.ts +++ b/server/src/nest/plugins/install/manifest.ts @@ -1,5 +1,6 @@ import semver from 'semver'; import { isKnownPermission } from '../protocol/envelope'; +import { isValidTrekRange, minTrekOf } from './host-compat'; import type { NotifEventType } from '../../../services/notificationPreferencesService'; /** @@ -87,7 +88,11 @@ export interface PluginManifest { homepage?: string; icon?: string; type: 'integration' | 'page' | 'widget' | 'trip-page'; + /** The raw semver RANGE of TREK versions this plugin supports (">=3.2.0 <4.0.0"). */ trek?: string; + /** Same range, normalized: a valid+satisfiable one, or null. What the host gates on. */ + trekRange: string | null; + /** The range's lower bound, for display ("Requires TREK 3.2.0+"). Derived, never authored. */ minTrekVersion?: string; nativeModules: boolean; permissions: string[]; @@ -134,7 +139,19 @@ export function parseJsonText(text: string): unknown { return JSON.parse(text.charCodeAt(0) === 0xfeff ? text.slice(1) : text); } -export function parseManifest(raw: unknown): PluginManifest { +/** + * Validate a manifest. + * + * `requireTrek` is the INSTALL front doors (registry, sideload, dev-link): a plugin + * that doesn't say which TREK versions it supports may not be installed at all. + * + * It is deliberately OFF for discovery, which is a reconciler, not a gate. Discovery's + * failure path only logs and skips — it never touches the plugins row — so a plugin it + * rejects keeps its stale `enabled = 1` and gets spawned by the next boot anyway. Making + * this strict there would produce a plugin that is invisible to discovery and still + * running. Activation is where an undeclared range is actually refused. + */ +export function parseManifest(raw: unknown, opts?: { requireTrek?: boolean }): PluginManifest { if (!raw || typeof raw !== 'object') throw new ManifestError('manifest is not an object'); const m = raw as Record; @@ -183,6 +200,14 @@ export function parseManifest(raw: unknown): PluginManifest { if (badEgress !== undefined) throw new ManifestError(`invalid egress host "${badEgress}"`); const trek = optStr(m.trek); + const trekRange = isValidTrekRange(trek) ? trek : null; + if (opts?.requireTrek && !trekRange) { + throw new ManifestError( + trek + ? `invalid "trek" version range "${trek}" (expected a satisfiable semver range, e.g. ">=3.2.0 <4.0.0")` + : 'missing "trek" version range — declare the TREK versions this plugin supports, e.g. ">=3.2.0 <4.0.0"', + ); + } return { id, name: str(m.name, 'name'), @@ -194,7 +219,11 @@ export function parseManifest(raw: unknown): PluginManifest { icon: optStr(m.icon) ?? 'Blocks', type: type as PluginManifest['type'], trek, - minTrekVersion: trek ? (trek.match(/(\d+\.\d+\.\d+)/)?.[1] ?? undefined) : undefined, + trekRange, + // Derived from the range itself, never from the first number that looks like a + // version in it: a range of "<4.0.0" has a *first* semver of 4.0.0 but a lower + // bound of 0.0.0, so reading it off the text produced the exact inverse of the truth. + minTrekVersion: trekRange ? minTrekOf(trekRange) : undefined, nativeModules: false, permissions, egress, diff --git a/server/src/nest/plugins/paths.ts b/server/src/nest/plugins/paths.ts index 19902ffcd..a0ebc9e86 100644 --- a/server/src/nest/plugins/paths.ts +++ b/server/src/nest/plugins/paths.ts @@ -119,7 +119,6 @@ export function pluginPermissionArgs(pluginId: string): string[] { // off by accident. if (!warnedPermissionsOff) { warnedPermissionsOff = true; - // eslint-disable-next-line no-console console.warn('[plugins] TREK_PLUGIN_PERMISSIONS=off — the OS permission jail is DISABLED; installed plugins run with full Node access to this process. Only use this on a machine you fully trust.'); } return []; diff --git a/server/src/nest/plugins/plugin-backup.ts b/server/src/nest/plugins/plugin-backup.ts index 1ec55d047..04df3464c 100644 --- a/server/src/nest/plugins/plugin-backup.ts +++ b/server/src/nest/plugins/plugin-backup.ts @@ -111,7 +111,6 @@ export function applyStagedPluginTrees(): string[] { swapContents(live, staged); applied.push(label); } catch (err) { - // eslint-disable-next-line no-console console.error(`[plugins] failed to apply staged ${label} restore:`, err); } } @@ -135,7 +134,6 @@ export async function applyStagedRestoreNow(): Promise { await applier(); return true; } catch (err) { - // eslint-disable-next-line no-console console.error('[plugins] immediate staged-restore apply failed; will retry on next boot:', err); return false; } diff --git a/server/src/nest/plugins/plugin-frame.controller.ts b/server/src/nest/plugins/plugin-frame.controller.ts index 4e98f9b24..08959bd06 100644 --- a/server/src/nest/plugins/plugin-frame.controller.ts +++ b/server/src/nest/plugins/plugin-frame.controller.ts @@ -67,12 +67,20 @@ export class PluginFrameController { /** Per-plugin, locked-down CSP for the sandboxed frame document. */ private frameCsp(pluginId: string, host: string | undefined): string { - // Defense in depth: the manifest validator already constrains these hosts, but - // never interpolate anything that isn't a clean host/wildcard into connect-src - // (a stray space or `*` would inject an extra CSP source token). - const outbound = this.runtime - .outboundHostsOf(pluginId) - .filter((h) => /^(\*\.[a-z0-9-]+(\.[a-z0-9-]+)+|[a-z0-9-]+(\.[a-z0-9-]+)*)$/i.test(h)); + // The frame must be able to reach exactly what the CHILD may reach. The child's egress + // guard is the union of the manifest's http:outbound: grants AND the hosts an admin + // added post-install for an operatorEgress plugin (plugin-runtime.service: activate()). + // Building connect-src from the grants alone left an operatorEgress plugin WITH A UI able + // to call the operator's host from its server but CSP-blocked in its own iframe. The admin + // consented to these hosts at install and the child already reaches them, so matching the + // frame to the child widens no trust boundary that isn't already crossed. + // + // Defense in depth: both sources are validated upstream (the manifest validator; the + // admin writer's EGRESS_HOST_RE), but never interpolate anything that isn't a clean + // host/wildcard into connect-src — a stray space or `*` would inject an extra CSP source. + const outbound = [ + ...new Set([...this.runtime.outboundHostsOf(pluginId), ...this.runtime.operatorEgressHosts(pluginId)]), + ].filter((h) => /^(\*\.[a-z0-9-]+(\.[a-z0-9-]+)+|[a-z0-9-]+(\.[a-z0-9-]+)*)$/i.test(h)); // The frame runs at an OPAQUE origin (sandbox without allow-same-origin), so // 'self' matches nothing and the plugin's own