From ebc98f0a3f9f5aafd56b03bcdf19b194a276cb02 Mon Sep 17 00:00:00 2001 From: jubnl <66769052+jubnl@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:40:00 +0200 Subject: [PATCH] v3.4.0 (#1527) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(plugins): unknown column * fix(mcp): reuse MCP sessions instead of creating one per tool call The /mcp CORS layer never set exposedHeaders, so Access-Control-Expose-Headers was absent and browser-context MCP clients (Claude Desktop connectors, Claude.ai, MCP Inspector) could not read Mcp-Session-Id off the initialize response. Unable to echo it back, every request looked like a fresh initialize: one McpServer and one session per tool call, until the per-user cap returned a 429 and the integration died until the container was restarted. The idle sweep was not at fault — it expires on lastActivity with a 1h default, so sessions born seconds apart are nowhere near expiry, hence 'cleaned 0'. - expose Mcp-Session-Id, MCP-Protocol-Version and WWW-Authenticate - evict a user's least-recently-active session at the cap rather than refusing the request, so a client that cannot persist its session id (or a proxy that strips the header) can never wedge the server - close the McpServer/transport orphaned by every session-less non-initialize POST, which was leaked: never mapped, never swept, never closed - return the cap error as JSON-RPC so clients surface the real reason - warn on session-less POSTs to make a header-stripping proxy diagnosable * chore(deps): declare @modelcontextprotocol/sdk ^1.29.0 Matches the version already resolved in the lockfile; no dependency-tree change. * docs(mcp): add the reverse proxy specs for MCP * fix(plugins): stop the row ⋯ menu being clipped by the sidebar The menu was an in-flow `absolute` div, and its ancestor (PageSidebar) is `overflow-hidden` — which clips absolutely-positioned descendants regardless of z-index. With enough plugins installed a row sits low enough that its menu runs past the sidebar's bottom edge and gets chopped, taking Delete with it, so the plugin could no longer be uninstalled from the UI. Portal the menu to and position it `fixed` against the ⋯ button, flipping upward when the bottom is tight and re-anchoring on scroll/resize. A fixed child of has no overflow ancestor, so nothing can clip it. Closes #1523 * feat(plugins): surface author-signature status and add a scoped re-trust override TREK has always verified an author's Ed25519 signature and TOFU-pinned the key on first install, but none of it was ever shown: a successfully-installed UNSIGNED plugin looked identical to a signed one, and a signature-refused update left the plugin quietly pinned at its old version with the reason dying in a toast. Give the four refusal conditions machine-readable codes (SIGNATURE_MISSING / _INCOMPLETE / _KEY_CHANGED / _INVALID), persist a refusal on the plugin row so the admin list keeps showing it, and badge Signed/Unsigned in the list and in Discover. Only SIGNATURE_KEY_CHANGED is overridable — an author can legitimately rotate a key; a signature that does not verify means the bytes are not what the author signed, and there is no story where waving that through is right. The override re-pins and updates in ONE call (POST :id/retrust): a re-pin that waited for a follow-up /update would leave the plugin pinned to a key no install had ever been verified against if that second call never came. The artifact must still verify under the new key, so a re-trust moves the pin from one verified key to another. assertRetrustable re-derives the condition server-side, so the UI hiding the button is a convenience, not the control, and it echoes back the full key the admin was shown so a re-key since the dialog rendered is refused. The rotation is written to the admin audit log with both fingerprints — after an incident, "which key did we move from, and to what?" is the question a single key cannot answer. * fix(plugins): let a plugin's frame reach the hosts an admin added for it The frame's connect-src was built from the manifest's http:outbound grants alone, but the child's egress guard is the UNION of those and the hosts an admin added after install for an operatorEgress plugin (a self-hosted Gotify, an ntfy — hosts the author cannot know in advance). So such a plugin WITH A UI could call the operator's host from its server and was CSP-blocked in its own iframe. Match the frame to the child. The admin consented to these hosts at install and the child already reaches them, so this widens no trust boundary that isn't already crossed. Both sources stay validated on the way in, and the interpolation filter is unchanged. * fix(plugins): report a rejected events.emit on the plugin's own log stream The host can reject an emit (an undeclared event name, a rate limit). The rejection must not escape — a detached rejection crashes the child and terminally disables the plugin over one bad emit — but swallowing it silently left an author with no way to discover that `emits` was missing from their manifest. Surface it as a warning instead. * feat(plugin-sdk): expose the raw request body for webhook signature checks A webhook author must run their HMAC over the exact bytes the sender signed. `body` is the PARSED value, and re-serializing it will not reproduce those bytes — key order, whitespace and unicode escaping all differ — so the signature never matches. Document `rawBodyBase64`, which the host already sets on auth:false routes. * fix(plugin-sdk): refuse to overwrite a released artifact, and keep the packed zip A released artifact is IMMUTABLE: the registry pins its sha256, so overwriting the bytes of a release already in the registry breaks the checksum for everyone who installed that version — they can no longer install or update it. The old code blanket-caught every `gh release create` failure (auth, network, a bad repo) and turned it into a --clobber upload. Probe for the release explicitly and refuse unless --force. Also stop deleting plugin.zip on the way out. 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 THAT file, not a rebuild. * fix(plugin-sdk): don't fail submit when the fork already has an upstream remote `gh repo clone` of a fork may already have wired `upstream`, in which case a bare `remote add` exits non-zero and took the whole submit down. Set it either way. * fix(plugin-sdk): make the dev server behave like the host Three ways `dev` lied to an author about how their plugin would run in production: - ctx.db: one try/catch wrapped both the node:sqlite probe AND opening the database, so an mkdir/permission failure silently degraded to the in-memory stub — which swallows every write while reporting success. A db:own plugin "worked" in dev and persisted nothing. Probe separately: fail loudly on a real error, degrade only on old Node, and say plainly that the stub discards writes. - notificationChannel: the host fires it with no acting user and hands the recipient's decrypted settings in as a separate `config` argument — send(msg, config, ctx) / test(config, ctx). Firing it like an ordinary hook passed `ctx` where `config` belongs, so a channel plugin read its settings off ctx and was broken in production. - /preview pinned tripId 42 while the scaffold seeds trip 1, so the widget's first trek:invoke hit assertMember(42) and 500'd. Preview against a trip that exists. * fix(plugin-sdk): reject unknown flags, wire up create's new flags, and add --help `parse()` accepts any --x, so a flag a command does not read was 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, and create actually forwards --template/--egress/--required-addons. A bare --permissions used to split the string "true" into a permission literally named `true`; listFlag() now treats a valueless flag as absent. Since an unknown flag is now fatal, `--help` has to exist: it is intercepted before the flag check and prints usage on stdout with exit 0. * chore(plugins): drop three unused eslint-disable directives The no-console rule is not enabled for these files, so the directives were dead and eslint reports them as unused. * fix(plugin-sdk): bring preflight back in step with the registry's gates preflight exists to tell an author what TREK-Plugins' CI will say before they open the PR. The registry now verifies author signatures, and preflight didn't — so it drifted into the one failure mode it must never have: a false green. An author trusts a green. - Verify the signature against the artifact bytes. preflight already downloads them for the sha256 check, so this costs one call. Without it, signing with the wrong key (or re-packing after signing) sails through and is caught at review. - Check the signature SHAPE (checkSignatureShape): a key with no signed version, a signature with no key, a malformed key or signature. TREK refuses to install a half-signed entry, so such an entry is dead on arrival. - Default apiVersion to 1 before comparing. It is OPTIONAL in the manifest — install/ manifest.ts and `entry` both default it — so a manifest that legally omits it was failing preflight with "manifest apiVersion undefined != entry 1" while the registry passed it. A false RED, which teaches authors to ignore the tool. - Check requiredAddons/pluginDependencies parity, and operatorEgress without an http:outbound grant. The verifier is a port of the host's install/verify-signature.ts (the registry has its own port); sign.ts's verifyArtifact only understands the bare key/signature pair the SDK itself emits and cannot judge a minisign key. A test pins all three to the same verdicts. * feat(plugins): enforce compatibility range * chore: bump sdk version * test(e2e): repair the trip-creation specs create-trip and trip-planner have been failing for a while — long enough for three separate UI changes to drift past them, which nothing caught because CI runs vitest only and never invokes Playwright. Each failure was masking the next: - The release-notice modal greets a freshly seeded user and its backdrop swallowed the click on .add-trip-card. Added a shared dismissSystemNotices() helper (the X only shows on the notice's last page, so it has to page through first). - .modal-backdrop no longer exists — the class was namespaced to .trek-modal-backdrop so content blockers stop hiding it. - input[type=text].first() is no longer the Title field: the cover-image search inputs now sit above it, so the specs were typing the trip name into the photo search box and creating an untitled trip that never matched getByText(title). * fix(planner): gate drag & drop on pointer type, not viewport width (#1432) The 3.2.1 fix disabled drag on "mobile", but nothing in the client has ever detected touch — "mobile" was inferred from viewport width, at four independent breakpoints. A tablet is a coarse-pointer device at a *desktop* width, so an iPad (820-1366px) fell on the wrong side of all four, which is why iPhone was fixed and iPad was not: - useTripPlanner's isMobile (<768px) is what disarmed `draggable`, so on iPad rows stayed draggable and a scroll swipe became an HTML5 drag. - TripPlannerPage hardcoded isMobile={false} on the desktop PlacesSidebar, so its drop handlers and the drop-to-import overlay could never be disabled — that overlay is the reported symptom. - The arrow-button reorder fallback was revealed only below 767px, leaving iPad with no drag *and* no fallback. - touchDragPolyfill loaded drag-drop-touch at >=1024px, synthesising drags from touchmove — on a landscape iPad that re-armed the very gesture hijack 3.2.1 removed. Adds useIsTouch() ((pointer: coarse)) as a signal separate from isMobile: layout stays width-driven, so the iPad keeps the desktop two-pane planner, while every drag affordance is gated on isMobile || isTouch. The reorder arrows now show on coarse pointers, and the polyfill loads only on hybrid laptops ((pointer: fine) and (any-pointer: coarse)), which also removes the #1440 phantom-dblclick map zoom on tablets. Guarded by unit tests plus an e2e spec on real WebKit in an iPad Pro 11 context — the engine matters, since every browser on iPadOS is WebKit underneath, which is why the reporter hit this in all three they tried. * test(planner): guard the day-plan reorder arrows against phantom clicks The hover rule that reveals the reorder arrows had been dead since the TypeScript migration: it targeted `.place-row:hover .reorder-btns`, and neither class exists — the component renders `.reorder-buttons` inside an unclassed row. The buttons sat at opacity:0 on desktop with nothing to reveal them. That was not merely invisible. opacity:0 still hit-tests, so every itinerary row and day note carried an invisible, fully clickable target that silently reordered the trip: opacity: "0", visibility: "visible", pointerEvents: "auto" elementFromPoint(centre) -> BUTTON, inside .reorder-buttons The repair — pointer-events: none while hidden, plus a working .dp-row:hover/:focus-within reveal (focus-within so the buttons are also reachable by keyboard, which the file's JS-hover pattern cannot do) — lives in index.css and DayPlanSidebar.tsx. Both files also carry the #1432 drag gating, so they went with that commit rather than being split mid-file; this commit is the regression guard for them. E2E rather than unit, because jsdom does not evaluate :hover. Against the old CSS it fails on exactly the right assertion: "hidden arrows must not swallow clicks" — expected false, received true. * fix(pdf): keep the header gap on day-header overflow pages (#1531) The repeated day header (#1471) carried no gap before the first card on overflow pages: the 12px sat in .day-body's block-start padding, which a fragmented box only paints on its first fragment. Move it to the thead cell so it repeats with the header; the day's first page renders pixel-identically. * fix(budget): keep "no one paid yet" when editing an expense (#1533) The ExpenseModal payer initializer fell back to the current user whenever the edited item had no payer, so reopening an expense saved with "no one paid yet" silently reselected "You" — and re-saving then recorded the current user as the payer, corrupting the balances. An absent payer on an existing expense is a deliberate value, so only a brand-new expense defaults to me. --- README.md | 17 +- client/e2e/create-trip.spec.ts | 18 +- client/e2e/helpers.ts | 22 + client/e2e/ipad-scroll-1432.spec.ts | 79 +++ client/e2e/reorder-arrows.spec.ts | 61 +++ client/e2e/trip-planner.spec.ts | 10 +- client/src/api/client.ts | 5 + .../Admin/AdminPluginsPanel.test.tsx | 365 +++++++++++++- .../components/Admin/AdminPluginsPanel.tsx | 472 ++++++++++++++++-- .../src/components/Budget/CostsPanel.test.tsx | 48 ++ client/src/components/Budget/CostsPanel.tsx | 6 +- client/src/components/PDF/TripPDF.test.ts | 12 + client/src/components/PDF/TripPDF.tsx | 6 +- .../src/components/Planner/DayPlanSidebar.tsx | 31 +- .../components/Planner/PlacesSidebar.test.tsx | 32 ++ .../src/components/Planner/PlacesSidebar.tsx | 13 +- .../components/Planner/PlacesSidebarList.tsx | 3 +- .../components/Planner/PlacesSidebarRow.tsx | 11 +- .../components/Planner/usePlacesSidebar.ts | 2 + client/src/hooks/useIsTouch.test.ts | 46 ++ client/src/hooks/useIsTouch.ts | 26 + client/src/index.css | 22 +- client/src/pages/TripPlannerPage.tsx | 4 +- .../src/pages/tripPlanner/useTripPlanner.ts | 6 +- client/src/utils/touchDragPolyfill.test.ts | 38 +- client/src/utils/touchDragPolyfill.ts | 11 +- package-lock.json | 71 +-- plugin-sdk/package.json | 2 +- plugin-sdk/src/cli/dev.ts | 67 ++- plugin-sdk/src/cli/entry.ts | 42 +- plugin-sdk/src/cli/preflight.ts | 58 ++- plugin-sdk/src/cli/publish.ts | 31 +- plugin-sdk/src/cli/submit.ts | 5 +- plugin-sdk/src/cli/trek-plugin.ts | 84 +++- plugin-sdk/src/cli/validate.ts | 9 +- plugin-sdk/src/cli/verify-signature.ts | 125 +++++ plugin-sdk/src/index.ts | 7 + plugin-sdk/src/manifest.ts | 36 +- plugin-sdk/test/cli.test.ts | 4 +- plugin-sdk/test/publish.test.ts | 73 +++ plugin-sdk/test/sdk.test.ts | 77 ++- plugin-sdk/test/verify-signature.test.ts | 103 ++++ server/package.json | 2 +- server/src/db/migrations.ts | 34 ++ server/src/mcp/index.ts | 41 +- server/src/mcp/sessionManager.ts | 45 +- server/src/middleware/globalMiddleware.ts | 14 +- server/src/nest/plugins/host/rpc-host.ts | 5 +- server/src/nest/plugins/install/discovery.ts | 10 +- .../src/nest/plugins/install/host-compat.ts | 106 ++++ server/src/nest/plugins/install/manifest.ts | 33 +- server/src/nest/plugins/paths.ts | 1 - server/src/nest/plugins/plugin-backup.ts | 2 - .../nest/plugins/plugin-frame.controller.ts | 20 +- .../nest/plugins/plugin-runtime.service.ts | 148 +++++- server/src/nest/plugins/plugins.controller.ts | 65 ++- server/src/nest/plugins/plugins.service.ts | 60 ++- .../nest/plugins/registry/registry.service.ts | 337 +++++++++++-- server/src/nest/plugins/runtime/plugin-sdk.ts | 16 +- server/src/nest/plugins/signature-status.ts | 93 ++++ server/src/services/auditLog.ts | 1 + server/tests/integration/mcp.test.ts | 68 ++- .../integration/plugins/dev-link.test.ts | 26 +- .../plugins/plugin-runtime.test.ts | 228 ++++++++- .../integration/plugins/registry.test.ts | 431 +++++++++++++++- server/tests/unit/mcp/sessionManager.test.ts | 65 ++- server/tests/unit/plugins/discovery.test.ts | 29 +- server/tests/unit/plugins/host-compat.test.ts | 116 +++++ server/tests/unit/plugins/manifest.test.ts | 35 ++ .../tests/unit/plugins/plugin-frame.test.ts | 37 +- .../plugins/plugin-retrust.controller.test.ts | 176 +++++++ .../unit/plugins/plugins-service.test.ts | 62 ++- server/tests/unit/plugins/rpc-host.test.ts | 29 ++ shared/src/i18n/ar/admin.ts | 41 +- shared/src/i18n/br/admin.ts | 41 +- shared/src/i18n/br/settings.ts | 3 +- shared/src/i18n/cs/admin.ts | 41 +- shared/src/i18n/de/admin.ts | 44 +- shared/src/i18n/de/settings.ts | 3 +- shared/src/i18n/en/admin.ts | 41 +- shared/src/i18n/es/admin.ts | 41 +- shared/src/i18n/es/settings.ts | 3 +- shared/src/i18n/fr/admin.ts | 44 +- shared/src/i18n/fr/settings.ts | 3 +- shared/src/i18n/gr/admin.ts | 41 +- shared/src/i18n/gr/settings.ts | 3 +- shared/src/i18n/hu/admin.ts | 41 +- shared/src/i18n/hu/settings.ts | 3 +- shared/src/i18n/id/admin.ts | 41 +- shared/src/i18n/it/admin.ts | 41 +- shared/src/i18n/it/settings.ts | 3 +- shared/src/i18n/ja/admin.ts | 41 +- shared/src/i18n/ko/admin.ts | 41 +- shared/src/i18n/nl/admin.ts | 41 +- shared/src/i18n/nl/settings.ts | 3 +- shared/src/i18n/pl/admin.ts | 41 +- shared/src/i18n/pl/settings.ts | 3 +- shared/src/i18n/ru/admin.ts | 41 +- shared/src/i18n/sv/admin.ts | 41 +- shared/src/i18n/sv/settings.ts | 3 +- shared/src/i18n/tr/admin.ts | 41 +- shared/src/i18n/tr/settings.ts | 3 +- shared/src/i18n/uk/admin.ts | 41 +- shared/src/i18n/vi/admin.ts | 41 +- shared/src/i18n/vi/settings.ts | 3 +- shared/src/i18n/zh-TW/admin.ts | 37 +- shared/src/i18n/zh/admin.ts | 38 +- wiki/Environment-Variables.md | 2 +- wiki/MCP-Overview.md | 4 + wiki/MCP-Setup.md | 2 + wiki/Plugin-Development.md | 29 +- wiki/Plugin-Publishing.md | 15 +- wiki/Reverse-Proxy.md | 43 +- wiki/Troubleshooting.md | 46 +- 114 files changed, 5148 insertions(+), 425 deletions(-) create mode 100644 client/e2e/helpers.ts create mode 100644 client/e2e/ipad-scroll-1432.spec.ts create mode 100644 client/e2e/reorder-arrows.spec.ts create mode 100644 client/src/hooks/useIsTouch.test.ts create mode 100644 client/src/hooks/useIsTouch.ts create mode 100644 plugin-sdk/src/cli/verify-signature.ts create mode 100644 plugin-sdk/test/publish.test.ts create mode 100644 plugin-sdk/test/verify-signature.test.ts create mode 100644 server/src/nest/plugins/install/host-compat.ts create mode 100644 server/src/nest/plugins/signature-status.ts create mode 100644 server/tests/unit/plugins/host-compat.test.ts create mode 100644 server/tests/unit/plugins/plugin-retrust.controller.test.ts 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