diff --git a/.github/workflows/platform-smoke.yml b/.github/workflows/platform-smoke.yml index 13f867e3..9da0cde0 100644 --- a/.github/workflows/platform-smoke.yml +++ b/.github/workflows/platform-smoke.yml @@ -10,7 +10,7 @@ on: - "scripts/native-deps-smoke.mjs" - "scripts/verify-libsql-vector.mjs" - "scripts/smoke-test.mjs" - - "scripts/copy-web-assets.mjs" + - "web/**" - ".github/workflows/platform-smoke.yml" workflow_dispatch: @@ -40,6 +40,10 @@ jobs: - name: Install source dependencies run: bun install + - name: Install web dependencies + working-directory: web + run: bun install + - name: Typecheck source run: bun run typecheck diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md index bef5050b..58aa0da2 100644 --- a/SECURITY_AUDIT.md +++ b/SECURITY_AUDIT.md @@ -2,6 +2,8 @@ **Scope:** `tickernelz/opencode-mem` @ commit `0998c69` (main). Local HTTP API, CORS policy, secret handling, SQLite layer, web UI, config/JSONC parsing, migration/cleanup services, dependencies. Read-only audit followed by targeted fixes for the three highest-severity findings. +**Note (GUI rewrite):** The Memory Explorer UI is now a Vite-bundled React 19 app under `web/` (served from `dist/web`). The previous CDN-loaded vanilla UI (`src/web/*`) and the interim Svelte UI are gone. Findings 3–4 below describe the pre-rewrite UI; current mitigations are noted under each. + ## Findings ### 1. CRITICAL — Path traversal via unauthenticated `containerTag` (fixed) @@ -16,21 +18,21 @@ A request such as `POST /api/memories {"content":"x","containerTag":"project_x_. CORS was being used as a substitute for authentication in `src/services/cors.ts`, but `isAllowedBrowserOrigin()` returned `true` whenever no `Origin` header was present — which is the case for `curl`, other local processes, and any non-browser client. Every `/api/*` handler (read/write/delete memories, full user-profile CRUD, migrations) had no session/token check at all. If `webServerHost` is set to `0.0.0.0` (a documented config option), this is reachable from the whole LAN with no auth. -**Fix:** added `src/services/auth-token.ts` — a random 256-bit token generated on first run, persisted to `~/.opencode-mem/.auth-token` (mode `0600`), required via the `x-opencode-mem-token` header on every `/api/*` request in both `web-server.ts` and `web-server-worker.ts`. The token is injected into the server-rendered `index.html` (`window.__OPENCODE_MEM_TOKEN__`) so the bundled web UI keeps working transparently (`app.js:fetchAPI` now sends the header), while a malicious cross-origin web page cannot read it (opaque/no-cors responses). The internal `checkServerAvailable()` health-check call was updated to send the token too, so the existing takeover/health-check logic still works. +**Fix:** added `src/services/auth-token.ts` — a random 256-bit token generated on first run, persisted to `~/.opencode-mem/.auth-token` (mode `0600`), required via the `x-opencode-mem-token` header on every `/api/*` request in `web-server.ts`. The token is injected into the server-rendered `index.html` (`window.__OPENCODE_MEM_TOKEN__`) so the bundled web UI keeps working transparently (`web/src/lib/api.ts` sends the header), while a malicious cross-origin web page cannot read it (opaque/no-cors responses). The internal `checkServerAvailable()` health-check call was updated to send the token too, so the existing takeover/health-check logic still works. This does not fully replace a "don't expose to `0.0.0.0` without more" warning — see recommendation below — but it closes the CSRF-style "any web page or generic local process can drive the API" gap the CORS check alone did not. -### 3. HIGH — Stored XSS via unescaped `profile.displayName` (fixed) +### 3. HIGH — Stored XSS via unescaped `profile.displayName` (fixed; superseded by React UI) -`renderUserProfile()` in `src/web/app.js` injected `profile.displayName` into `innerHTML` with no `escapeHtml()` call, unlike every other user-influenced field rendered elsewhere in the same file. `displayName` originates from `userNameOverride` in a project's `.opencode/opencode-mem.jsonc`, which is loaded automatically, with no confirmation, whenever that project is opened — so a malicious repo can plant a payload that later executes in the local Web UI when the profile tab is viewed, with same-origin `fetch()` access to the (now-authenticated, but still same-origin) API. +Historically, `renderUserProfile()` in the vanilla UI injected `profile.displayName` into `innerHTML` with no escaping. `displayName` originates from `userNameOverride` in a project's `.opencode/opencode-mem.jsonc`. -The existing `tests/web-memorytype-xss.test.ts` covers a different field (`memoryType`) that was already escaped correctly; it did not cover this sink. +**Current mitigation:** React JSX text nodes auto-escape; shared `escapeHtml()` in `web/src/lib/html.ts` covers any HTML string sinks. Markdown content goes through DOMPurify. Regression tests: `tests/web-userprofile-xss.test.ts`, `tests/web-memorytype-xss.test.ts`. -**Fix:** wrapped the value in `escapeHtml()`. Regression test: `tests/web-userprofile-xss.test.ts` (fails against the pre-fix code, passes after). +### 4. MEDIUM — Unpinned, non-SRI third-party scripts (fixed by bundling) -### 4. MEDIUM — Unpinned, non-SRI third-party scripts (not fixed — recommendation only) +Historically, the vanilla `index.html` loaded scripts from CDNs (`unpkg` / `jsdelivr`) without SRI. -`src/web/index.html` loads `lucide@latest` and `jsonrepair@latest` (no version pin) plus two pinned-but-no-`integrity` scripts from `unpkg.com`/`cdn.jsdelivr.net`. Recommend pinning exact versions and adding Subresource Integrity hashes, or self-hosting alongside the already-vendored `app.js`/`i18n.js`/`styles.css`. +**Current mitigation:** UI dependencies are installed and bundled by Vite into `dist/web` (no runtime CDN scripts). ### 5. LOW / informational — Gemini API key in URL query string (not fixed) @@ -46,11 +48,9 @@ The existing `tests/web-memorytype-xss.test.ts` covers a different field (`memor ## Remaining recommendations (not implemented in this patch) -- Pin and add SRI to the CDN-loaded scripts in `src/web/index.html` (Finding 4). - Consider warning (or refusing) at startup when `webServerHost` is set to a non-loopback address without additional network-level protection, since the token-based fix here raises the bar but a determined local/LAN attacker who can read `~/.opencode-mem/.auth-token` (or intercept the injected ` - - - - - -
-