|
| 1 | +# Security Remediation Log |
| 2 | + |
| 3 | +**Date:** 2026-06-02 |
| 4 | +**Scope:** Fixes for the Critical + High findings from `SECURITY-REPORT.md` (scan 2026-06-01). |
| 5 | +**Verification:** `@ownpilot/core` + `@ownpilot/gateway` typecheck clean; 1,720 tests pass across all touched suites (incl. new regressions). core full suite 9,377 pass. |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Fixed — Critical |
| 10 | + |
| 11 | +### RCE-001 — VM sandbox escape (3 sinks) |
| 12 | +Root cause: host-realm objects injected into a `node:vm` context expose the host |
| 13 | +`Function` constructor via `.constructor`, which `codeGeneration:{strings:false}` |
| 14 | +does **not** disable. Empirically reproduced on Node v24: |
| 15 | +`Math.max['constructo'+'r']('return process')()` returned the host `process`. |
| 16 | + |
| 17 | +Fix pattern (canonical): never inject host-realm values; pass host fns under one |
| 18 | +`__host` global + data as `__argsJson`, run a bootstrap Script that rebuilds |
| 19 | +context-realm wrappers / `JSON.parse`s data into the context realm, then |
| 20 | +`delete globalThis.__host`. The context's own intrinsics (JSON/Math/Date/…) are |
| 21 | +used instead — their Function IS blocked. |
| 22 | + |
| 23 | +- `packages/gateway/src/services/extension/sandbox.ts` — worker sandbox (extensions, `claw_execute`) |
| 24 | +- `packages/gateway/src/tools/claw/lifecycle-executors.ts` — `claw_create_tool` (ran in main thread) |
| 25 | +- `packages/gateway/src/services/workflow/executors/utils.ts` — `safeVmEval` (was `structuredClone` → host-realm leak; verifier under-rated this as RCE-003 Medium, but it was escapable) |
| 26 | +- `packages/core/src/sandbox/code-validator.ts` — split-`constructor` regexes hardened (defense-in-depth) |
| 27 | + |
| 28 | +### RCE-002 — Workflow code/expression template injection |
| 29 | +Upstream node output (HTTP/LLM/webhook data) was spliced **raw** into code before |
| 30 | +execution. Fix: new `resolveCodeTemplates` JS-literal-encodes every substituted |
| 31 | +value, so data can never break into a code position. |
| 32 | + |
| 33 | +- `packages/gateway/src/services/workflow/template-resolver.ts` — new `resolveCodeTemplates` |
| 34 | +- `packages/gateway/src/services/workflow/executors/tool-llm-code.ts` — code node + transformer use it |
| 35 | + |
| 36 | +**Regression tests:** `packages/gateway/src/services/workflow/executors/utils.test.ts` (new — live escape attempts), `packages/core/src/sandbox/code-validator.test.ts` (split-string cases). |
| 37 | + |
| 38 | +--- |
| 39 | + |
| 40 | +## Fixed — High |
| 41 | + |
| 42 | +| Finding | File(s) | Fix | |
| 43 | +|---|---|---| |
| 44 | +| EXPOSE-004 / TS-002 (weak PRNG) | `channels/service-impl.ts` | DM pairing code `Math.random()` → `crypto.randomInt(100000, 1000000)` | |
| 45 | +| SSRF-001 | `tools/overrides/image.ts` | Provider-returned image URLs fetched via `safeFetch` (SSRF guard) | |
| 46 | +| PATH-001 | `routes/extensions/install.ts` | `/install` path confined to `getAllScanDirectories()` via `isWithinDirectory` (+ rejection test) | |
| 47 | +| EXPOSE-002 | `routes/database/shared.ts` | `system_settings` (gateway API keys, JWT secret, pairing keys) removed from `EXPORT_TABLES` → not exportable/CSV/importable | |
| 48 | +| CRYPTO-004 | `core/credentials/index.ts` | Legacy no-salt entries lazily re-encrypted to PBKDF2+salt on read (`migrateLegacyEntryIfNeeded`) | |
| 49 | +| DOCK-004 / IAC-002 | `docker-compose.yml` | MQTT broker ports bound to `127.0.0.1` (no LAN exposure; broker still ships `allow_anonymous true`) | |
| 50 | +| BIZ-001 | `services/agent/runner-utils.ts` | Pre-spend budget check added to `executeAgentPipeline` (shared chokepoint for Claw/Soul/Subagent), fail-open, new `BudgetExceededError` + 3 tests | |
| 51 | +| CICD-002 | `.github/workflows/release.yml` | All 9 actions pinned to full-length commit SHAs (version in trailing comment). SHAs resolved via `git ls-remote` on 2026-06-02 | |
| 52 | +| DOCK-001 / IAC-001 | `packages/ui/Dockerfile.dev` | Runs as built-in non-root `node` user (matches prod Dockerfiles) + adds a HEALTHCHECK. Not wired into the shipped compose, so no bind-mount uid impact | |
| 53 | +| DOCK-003 / IAC-004 | `docker-compose.db.yml` | Default DB password documented as local-dev-only with an explicit "set POSTGRES_PASSWORD for shared/prod" warning. Residual accepted: port is already bound to 127.0.0.1, so the DB is never LAN/remote-exposed | |
| 54 | +| CICD-001 | `.github/workflows/ci.yml`, `deploy-website.yml` | All actions pinned to commit SHAs (completes the action-pinning started in CICD-002) | |
| 55 | +| EXPOSE-001 | `db/repositories/logs.ts` | Persisted error stacks capped at 2000 chars (bounds internal-path disclosure in per-user `request_logs` exports + DB bloat). Fuller path-redaction / prod-gating left as a product decision | |
| 56 | +| WS-001 | `ws/server.ts` | Gateway `https://` self-origins added (TLS same-origin WS upgrades were rejected) + startup warning when production has no `WS_ALLOWED_ORIGINS`/`CORS_ORIGINS` configured | |
| 57 | +| API-001 | `routes/openapi.ts` | OpenAPI spec + Swagger UI disabled in production unless `ENABLE_API_DOCS=true` (hard on/off switch — deliberately avoids the delicate session-auth path). Dev unchanged | |
| 58 | + |
| 59 | +## Reviewed — intentionally NOT fixed (false-positive in context / already-correct) |
| 60 | + |
| 61 | +- **SESS-001** (session cookie `Secure` flag). Already correct: `isSecureCookie()` sets `Secure` whenever the connection is HTTPS (incl. behind a trusted proxy via `X-Forwarded-Proto`); over plain HTTP `Secure` is impossible anyway. No change. |
| 62 | +- **MASS-003** (autonomy decision `modifications` spread). No mass-assignment: the body is a strict `z.object` (unknown keys stripped), ownership is enforced (403 on mismatch), and `processDecision` only merges into `action.params` (the owner's own pending tool args, then re-assesses risk) — never into ownership/status. NB: it also revealed a *functional* bug (API field `modifications` vs manager field `modifiedParams` → modify-with-params is a silent no-op); that's a behavior fix, out of security scope. |
| 63 | + |
| 64 | + |
| 65 | +- **SSRF-002** (`tools/overrides/image.ts`, `audio.ts` — provider `base_url`/TTS/STT first-hop fetch). Wrapping these in `safeFetch` was attempted and **reverted**: `safeFetch` blocks private/localhost addresses, but (a) the local Whisper/Piper diagnostic + transcription paths legitimately hit `localhost`, and (b) this is a *privacy-first* platform where the admin-configured provider `base_url` may legitimately point at a **local** model server (OpenAI-compatible at 127.0.0.1). The `base_url` is admin-controlled, so this is self-SSRF (the verifier already rated it Medium for that reason). Forcing `safeFetch` would break local/self-hosted AI providers — a core use case. Only SSRF-001 (fetching URLs returned **inside** provider response bodies, which are always public CDN URLs) warrants the guard, and that is fixed. |
| 66 | + |
| 67 | +--- |
| 68 | + |
| 69 | +## NOT changed — needs maintainer decision (ops trade-offs) |
| 70 | + |
| 71 | +- **mosquitto.conf `allow_anonymous true`** — left as the documented local-dev default; LAN exposure already removed by the loopback port binding above. For production, set `allow_anonymous false` + `password_file` (already documented in the conf). |
| 72 | +- **Medium / Low backlog** — see `verified-findings.md`. |
| 73 | + |
| 74 | +--- |
| 75 | + |
| 76 | +## Committing note |
| 77 | + |
| 78 | +Two fixed files contain **pre-existing uncommitted work** that predates this |
| 79 | +remediation and must not be bundled into a security commit: |
| 80 | + |
| 81 | +- `services/extension/sandbox.ts` — RCE-001 fix (in `workerMain()`) is mixed with prior `ExtensionSandboxManager` trusted-identity (EXT-002) changes. |
| 82 | +- `routes/database/shared.ts` — EXPOSE-002 one-line removal is mixed with prior per-user export filtering (CSV-002). |
| 83 | + |
| 84 | +Suggested grouping when committing (use `git add -p` for the two mixed files): |
| 85 | +1. `fix(security): close vm sandbox escape (RCE-001) + workflow template injection (RCE-002)` |
| 86 | +2. `fix(security): SSRF guard, path containment, secret export, weak PRNG, KDF migration, MQTT bind, budget on autonomous runners` |
0 commit comments