594 lines
23 KiB
Markdown
594 lines
23 KiB
Markdown
# Security Audit — Officer.dev Pi-Monorepo
|
||
|
||
**Date:** 2026-02-25
|
||
**Scope:** Full-stack audit of server, frontend, and infrastructure code.
|
||
|
||
---
|
||
|
||
## Executive Summary
|
||
|
||
The codebase has solid foundations (JWT + argon2 auth, WebAuthn passkeys, token blacklisting, Docker-sandboxed terminals, role-based access control). However, several critical and high-severity vulnerabilities exist — primarily around CORS misconfiguration, weak secret management, unauthenticated admin routes, XSS via `rehype-raw`, and token exposure in URLs.
|
||
|
||
**Totals:** 5 Critical, 13 High, 14 Medium, 9 Low, 5 Info
|
||
|
||
---
|
||
|
||
## CRITICAL
|
||
|
||
### C1. Weak JWT Secret — `"officer"`
|
||
|
||
**Files:** `.env` (line 2), `src/servers/jwt.ts` (lines 2, 25)
|
||
|
||
The live `.env` contains `JWT_SECRET="officer"` — a single dictionary word. The `.env.example` uses `"change-me"`. The server uses a non-null assertion (`JWT_SECRET!`) with no startup validation for length or complexity.
|
||
|
||
**Impact:** Complete authentication bypass. Anyone who knows/guesses the secret can forge JWTs for any user including Super Admin.
|
||
|
||
**Remediation:**
|
||
- Generate a cryptographically random secret: `openssl rand -base64 32`
|
||
- Add a startup guard: throw if `JWT_SECRET` is absent or shorter than 32 characters
|
||
- Update `.env.example` with a placeholder: `JWT_SECRET=<generate with: openssl rand -base64 32>`
|
||
|
||
---
|
||
|
||
### C2. Global CORS Wildcard on All API Routes
|
||
|
||
**File:** `src/servers/hono.ts` (lines 36–42)
|
||
|
||
```ts
|
||
cors({ origin: '*', allowHeaders: ['Content-Type', 'Authorization'] })
|
||
```
|
||
|
||
Applied to every route. The `Authorization` header allowlist means any website can make cross-origin requests using a victim's token (obtained via XSS or other leaks).
|
||
|
||
**Impact:** Full cross-origin request forgery against all API endpoints for users tricked into visiting a malicious page.
|
||
|
||
**Remediation:** Replace `origin: '*'` with the allowlist from `origin-validation.ts` (`ALLOWED_ORIGINS`). Use a dynamic origin validation function in the CORS middleware.
|
||
|
||
---
|
||
|
||
### C3. Server-Settings Routes Entirely Unauthenticated
|
||
|
||
**Files:** `src/servers/hono.ts` (lines 46–48), `src/servers/api/server-settings/server-settings.ts`
|
||
|
||
The `serverSettingsRouter` is mounted on the **public** router (outside `protectedRouter`). All sub-routes — SMTP, TTS, STT, OCR, SearXNG, Pi-Mono, Claude Code, OpenCode, applications, resources — have no auth checks. Any unauthenticated request can read settings (including masked credentials), write new settings, and trigger software installs.
|
||
|
||
**Impact:** Full server configuration takeover by any unauthenticated user.
|
||
|
||
**Remediation:** Move `serverSettingsRouter` inside `protectedRouter`. Add Super Admin role checks on all write operations. Exempt only `GET /api/server-settings/onboarding-complete` as public.
|
||
|
||
---
|
||
|
||
### C4. Unauthenticated Dev-Server WebSocket Proxy
|
||
|
||
**Files:** `src/server.tsx` (lines 114–139), `src/servers/api/dev-server/router.ts` (lines 241–268), `src/servers/hono.ts` (line 49)
|
||
|
||
The dev-server proxy (`/api/dev-server-proxy/*`) is mounted on the public router. WebSocket upgrades carry no authentication. The HTTP proxy has no auth gate either. Additionally, the Referer header fallback allows probing any running dev server.
|
||
|
||
**Impact:** Unauthenticated access to dev-servers spawned for any user, which may expose internal APIs, HMR endpoints, or local services.
|
||
|
||
**Remediation:** Require a valid JWT (via query param) before upgrading WebSocket connections. Apply `userMiddleware` to the HTTP proxy router.
|
||
|
||
---
|
||
|
||
### C5. AI Chat Messages Rendered with `rehype-raw` — LLM-Driven XSS
|
||
|
||
**File:** `src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx` (lines 132, 181)
|
||
|
||
```tsx
|
||
<ReactMarkdown rehypePlugins={[rehypeRaw]}>{assistantText}</ReactMarkdown>
|
||
```
|
||
|
||
AI-generated assistant responses containing HTML tags (`<script>`, `<iframe>`, `<img onerror=...>`) are rendered verbatim into the DOM.
|
||
|
||
**Impact:** Stored XSS in the chat interface. Prompt injection via uploaded files, tool results, or a compromised AI provider can execute arbitrary JavaScript — exfiltrating the Bearer token from localStorage.
|
||
|
||
**Remediation:** Remove `rehype-raw` from chat rendering. If basic HTML is needed, use `rehype-sanitize` with an allowlist of safe tags (no `<script>`, `<iframe>`, event handlers).
|
||
|
||
---
|
||
|
||
## HIGH
|
||
|
||
### H1. Bearer Token Stored in `localStorage`
|
||
|
||
**Files:** `src/workspaces/hooks/src/useAuth/useAuth.ts` (lines 56, 84, 99), `usePasskeys.ts` (line 44)
|
||
|
||
The primary 30-day authentication token is stored in `localStorage`, readable by any JavaScript on the page origin — including via XSS.
|
||
|
||
**Impact:** Combined with any XSS vector (C5, H5, H6), an attacker can exfiltrate the token and hijack the session.
|
||
|
||
**Remediation:** Use `HttpOnly` cookies for the Bearer token so it is inaccessible to JavaScript.
|
||
|
||
---
|
||
|
||
### H2. Bearer Token Exposed in WebSocket/Media URL Query Strings
|
||
|
||
**Files:** `src/workspaces/officerdev/src/hooks/usePiChat.ts` (line 58), `src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx` (line 43), `src/workspaces/officerdev/src/apps/FileViewer/file-types.ts` (lines 144–148), `src/server.tsx` (line 87), `src/servers/_middlewares/user-middleware.ts` (line 32)
|
||
|
||
The full 30-day JWT appears in URLs for WebSocket connections, raw file downloads, and media transcoding.
|
||
|
||
**Impact:** Tokens are logged by servers, proxies, CDNs; stored in browser history; sent in `Referer` headers; visible to browser extensions and analytics scripts.
|
||
|
||
**Remediation:** For WebSocket: send the token as the first message after connection. For media: use short-lived single-use tokens generated by a dedicated endpoint (e.g., 60-second TTL).
|
||
|
||
---
|
||
|
||
### H3. `rehype-raw` Used in Multiple Content-Rendering Contexts
|
||
|
||
**Files:**
|
||
- `src/apps/officer-web/Screens/Dashboard/CapabilityPage.tsx` (line 354)
|
||
- `src/apps/officer-web/Screens/Dashboard/Plans/index.tsx` (line 5)
|
||
- `src/apps/officer-web/Screens/Dashboard/Automation/CapabilityDetailView.tsx` (line 5)
|
||
- `src/apps/officer-web/Screens/Dashboard/Settings/ResourceSettings/Resources.tsx` (line 5)
|
||
- `src/workspaces/components/MarkdownEditor.tsx` (line 98)
|
||
- `src/workspaces/officerdev/src/apps/FileViewer/renderers/MarkdownRenderer.tsx` (line 43)
|
||
|
||
Markdown from user-editable files (skills, tasks, processes, arbitrary filesystem files) is rendered with `rehype-raw`, allowing embedded HTML/script injection.
|
||
|
||
**Impact:** Stored XSS. A malicious Markdown file opened in the file viewer or capability page executes arbitrary JavaScript.
|
||
|
||
**Remediation:** Add `rehype-sanitize` alongside `rehype-raw` in all locations, or remove `rehype-raw` entirely for untrusted content.
|
||
|
||
---
|
||
|
||
### H4. Email HTML Rendered with `allow-same-origin` Sandbox
|
||
|
||
**File:** `src/apps/officer-web/Screens/Dashboard/Email/EmailReader.tsx` (lines 25–41)
|
||
|
||
HTML email bodies are written into an iframe via `document.write()` with `sandbox="allow-same-origin"`. While `allow-scripts` is not set (blocking script execution), CSS injection and phishing forms can still render.
|
||
|
||
**Impact:** Email phishing attacks rendered within the Officer UI. CSS-based data exfiltration possible.
|
||
|
||
**Remediation:** Change to `sandbox=""` (no attributes). Consider running email HTML through DOMPurify server-side.
|
||
|
||
---
|
||
|
||
### H5. Mass Email Template — Unsanitized `dangerouslySetInnerHTML`
|
||
|
||
**Files:** `src/workspaces/emailer/emails/MassEmail.tsx` (line 23), `src/workspaces/components/MarkdownEditor.tsx` (line 41)
|
||
|
||
```tsx
|
||
<div dangerouslySetInnerHTML={{ __html: greetingHtml + content }} />
|
||
```
|
||
|
||
User names are interpolated via `String.replace()` without HTML encoding. A user with `<script>` in their name triggers XSS in the admin's email preview.
|
||
|
||
**Impact:** Stored XSS in admin email previews.
|
||
|
||
**Remediation:** HTML-encode all interpolated user data. Use DOMPurify on the final output.
|
||
|
||
---
|
||
|
||
### H6. `applyGlobals` — Arbitrary JavaScript Injection
|
||
|
||
**File:** `src/workspaces/injector/apply-changes/apply-globals.ts` (lines 11–13)
|
||
|
||
```ts
|
||
script.innerHTML = `;(function(){${globalJavascript}})();`;
|
||
document.body.appendChild(script);
|
||
```
|
||
|
||
A/B test experiment configuration injects arbitrary JavaScript into every user's browser session.
|
||
|
||
**Impact:** If a non-admin can influence the `globalJavascript` payload, it's instant code execution across the organization.
|
||
|
||
**Remediation:** Restrict write access to experiment configurations to Admin/Owner/Super Admin. Audit API endpoints that set `globalJavascript`.
|
||
|
||
---
|
||
|
||
### H7. Unsanitized URL Passed to `yt-dlp` and `git clone`
|
||
|
||
**File:** `src/servers/api/file-browser/router.ts` (lines 901–944)
|
||
|
||
Both `yt-dlp` and `git clone` are invoked with user-supplied URLs without scheme validation. `git clone file:///etc/passwd` reads local files. `yt-dlp` accepts plugin URLs and SSRF targets.
|
||
|
||
**Impact:** Local file exfiltration via `file://`, SSRF to internal services, potential exploitation via git remote helpers.
|
||
|
||
**Remediation:** Validate URLs to allow only `http://` and `https://` schemes. Add `--` argument terminator and `--no-config` for yt-dlp. Add `--no-local` for git.
|
||
|
||
---
|
||
|
||
### H8. File Browser Exposes Host Filesystem to Super Admin
|
||
|
||
**File:** `src/servers/api/file-browser/router.ts` (lines 54–61)
|
||
|
||
Super Admin can access `~` (host homedir) and `officer.dev` (project root), giving read/write access to `.env`, auth store files, API keys, and the entire host filesystem.
|
||
|
||
**Impact:** A compromised Super Admin account (or forged JWT — trivial with C1) has full host filesystem access.
|
||
|
||
**Remediation:** Restrict roots to `DATA_PATH` only. Remove `~` and `officer.dev` roots or restrict them to read-only with path filtering.
|
||
|
||
---
|
||
|
||
### H9. Rate Limiting Disabled Outside Production
|
||
|
||
**File:** `src/servers/_middlewares/rate-limiter.ts` (lines 27–28, 45–46)
|
||
|
||
All rate limiters are bypassed unless `PUBLIC_BUILD_ENV` is `production` or `staging`. The `.env` doesn't set this variable, so rate limiting is off by default.
|
||
|
||
**Impact:** Unrestricted brute-force attacks on passwords, verification codes, and passkey challenges in any deployment that doesn't explicitly set `PUBLIC_BUILD_ENV=production`.
|
||
|
||
**Remediation:** Enable rate limiting by default. Only disable when `PUBLIC_BUILD_ENV === 'test'`.
|
||
|
||
---
|
||
|
||
### H10. Rate Limiter Uses Spoofable `X-Forwarded-For`
|
||
|
||
**File:** `src/servers/_middlewares/rate-limiter.ts` (line 34)
|
||
|
||
The default key is the raw `X-Forwarded-For` header, which is client-controlled and trivially spoofed.
|
||
|
||
**Impact:** Complete rate limit bypass by setting any `X-Forwarded-For` value.
|
||
|
||
**Remediation:** Use the actual socket IP (Bun provides this) or validate `X-Forwarded-For` against a trusted proxy list.
|
||
|
||
---
|
||
|
||
### H11. Terminal Sidecar Users Get Passwordless Sudo
|
||
|
||
**File:** `src/servers/api/terminal/entrypoint.sh` (line 24)
|
||
|
||
```sh
|
||
echo "$USERNAME ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/terminal-user
|
||
```
|
||
|
||
**Impact:** Full root inside container. If Docker sandboxing is bypassed or misconfigured, this escalates to host root.
|
||
|
||
**Remediation:** Remove NOPASSWD sudo entirely or restrict to specific commands.
|
||
|
||
---
|
||
|
||
### H12. Google OAuth State Parameter Not Signed
|
||
|
||
**File:** `src/servers/api/integrations/integrations.ts` (lines 134–150)
|
||
|
||
The OAuth state is just `base64(JSON)` — no HMAC. The `origin` query parameter is user-controlled and embedded in the state. The callback is a public endpoint.
|
||
|
||
**Impact:** OAuth CSRF. An attacker can craft a state parameter to bind their Google account to a victim's email.
|
||
|
||
**Remediation:** Sign the state with HMAC using `JWT_SECRET`. Derive `redirectUri` from `PUBLIC_URL`, not from the query parameter.
|
||
|
||
---
|
||
|
||
### H13. `maxRequestBodySize` Set to 50 GB
|
||
|
||
**File:** `src/server.tsx` (line 144)
|
||
|
||
**Impact:** DoS by exhausting server memory/disk with a single request.
|
||
|
||
**Remediation:** Set a reasonable limit (100 MB–1 GB). Apply per-route size limits for file uploads.
|
||
|
||
---
|
||
|
||
## MEDIUM
|
||
|
||
### M1. Rate Limiter Only Counts Failed Requests
|
||
|
||
**File:** `src/servers/_middlewares/rate-limiter.ts` (lines 59–67)
|
||
|
||
Only 4xx responses increment the counter. Successful auths reset the window.
|
||
|
||
**Impact:** Attackers can interleave successful/failed attempts to effectively bypass rate limits.
|
||
|
||
**Remediation:** Count all requests toward the limit, regardless of response code.
|
||
|
||
---
|
||
|
||
### M2. Password Reset Tokens Not Blacklisted After Use
|
||
|
||
**File:** `src/servers/api/auth/reset-password.ts`
|
||
|
||
The same reset link can be used multiple times within its validity window.
|
||
|
||
**Impact:** Reset link replay — an intercepted link works even after the user has already reset their password.
|
||
|
||
**Remediation:** Blacklist the token's JTI immediately after first use.
|
||
|
||
---
|
||
|
||
### M3. Password Validation Skipped in Non-Production
|
||
|
||
**File:** `src/servers/api/auth/change-password.ts` (lines 8–13)
|
||
|
||
```ts
|
||
if (isProduction) validatePassword(newPassword);
|
||
```
|
||
|
||
**Impact:** Weak/empty passwords accepted if `PUBLIC_BUILD_ENV` is unset in production.
|
||
|
||
**Remediation:** Always validate password complexity regardless of environment.
|
||
|
||
---
|
||
|
||
### M4. User Settings PUT — No Schema Validation or Size Limit
|
||
|
||
**File:** `src/servers/api/settings/settings.ts` (lines 43–51)
|
||
|
||
The entire request body is written verbatim to the settings JSON file.
|
||
|
||
**Impact:** Disk exhaustion, injection of arbitrary keys that may affect server-side logic (e.g., `tts.voice`).
|
||
|
||
**Remediation:** Apply a size limit (64 KB) and schema validation for known fields only.
|
||
|
||
---
|
||
|
||
### M5. SSRF via Scrape Endpoint (Playwright)
|
||
|
||
**File:** `src/servers/api/scrape/scrape.ts` (lines 31–45)
|
||
|
||
The scrape endpoint navigates Playwright to any user-supplied URL without validation.
|
||
|
||
**Impact:** SSRF to internal network, cloud metadata (`169.254.169.254`), and localhost services. Full HTML returned.
|
||
|
||
**Remediation:** Validate scheme (`http://`/`https://` only), resolve hostname, block private IP ranges.
|
||
|
||
---
|
||
|
||
### M6. SSRF via User-Configured OCR/TTS/STT URLs
|
||
|
||
**File:** `src/servers/api/file-browser/router.ts` (lines 509–545)
|
||
|
||
External API URLs come from server configuration (writable via unauthenticated C3 routes). An attacker can point them to internal services.
|
||
|
||
**Impact:** SSRF to internal network via AI provider proxy.
|
||
|
||
**Remediation:** Fix C3 first. Then validate configured URLs don't resolve to private/loopback ranges.
|
||
|
||
---
|
||
|
||
### M7. OCR Sends Arbitrary Files to External Vision API
|
||
|
||
**File:** `src/servers/api/file-browser/router.ts` (lines 481–555)
|
||
|
||
MIME type derived from extension only. No file type validation or size limit.
|
||
|
||
**Impact:** Data exfiltration of non-image files (config, keys) to third-party AI providers.
|
||
|
||
**Remediation:** Validate magic bytes, enforce max size, restrict allowed extensions.
|
||
|
||
---
|
||
|
||
### M8. `resend-verification` Leaks User Existence
|
||
|
||
**File:** `src/servers/api/auth/resend-verification.ts` (line 13)
|
||
|
||
Returns distinct responses for "user not found" vs "already verified" with no rate limiting.
|
||
|
||
**Impact:** Automated user enumeration.
|
||
|
||
**Remediation:** Return the same success response regardless. Add rate limiting.
|
||
|
||
---
|
||
|
||
### M9. API Keys + `JWT_SECRET` Leaked to AI Subprocess
|
||
|
||
**File:** `src/servers/api/pi/pi-bridge.ts` (lines 234–235, 281–283)
|
||
|
||
The unsandboxed path spreads `process.env` (including `JWT_SECRET`, DB credentials, SMTP passwords) into every Pi process.
|
||
|
||
**Impact:** A prompt-injected AI model can exfiltrate all server secrets.
|
||
|
||
**Remediation:** Explicitly enumerate allowed environment variables. Never pass `JWT_SECRET` or DB credentials to AI subprocesses.
|
||
|
||
---
|
||
|
||
### M10. Auth Store Has No File Locking
|
||
|
||
**File:** `src/databases/officer_db/src/store.ts`
|
||
|
||
In-memory arrays with async file writes. No mutex or write queue.
|
||
|
||
**Impact:** Concurrent writes (signups, signouts) can lose data or corrupt the JSON file.
|
||
|
||
**Remediation:** Add a write lock/queue. Consider SQLite for the auth store.
|
||
|
||
---
|
||
|
||
### M11. JSON Parse Errors Silently Swallowed in Body Parser
|
||
|
||
**File:** `src/servers/_middlewares/body-parser.ts` (lines 27–29)
|
||
|
||
```ts
|
||
} catch (ex) {
|
||
// throw errors.BAD_REQUEST('Invalid request body');
|
||
}
|
||
```
|
||
|
||
**Impact:** Malformed request bodies become `{}`, bypassing input validation in handlers that don't check required fields.
|
||
|
||
**Remediation:** Uncomment the `throw errors.BAD_REQUEST(...)` line.
|
||
|
||
---
|
||
|
||
### M12. Module-Level Token Singleton Race Condition
|
||
|
||
**File:** `src/workspaces/hooks/src/useClient.ts` (lines 2–14)
|
||
|
||
`theToken` is a module-level variable set once per `createClient()` call. Stale after logout.
|
||
|
||
**Impact:** Authenticated requests may continue after logout in race conditions.
|
||
|
||
**Remediation:** Read the token fresh from storage in `getHeaders()` rather than caching in a module variable.
|
||
|
||
---
|
||
|
||
### M13. No Content Security Policy
|
||
|
||
**Files:** `src/apps/officer-web/index.html`, `src/servers/hono.ts`
|
||
|
||
No CSP header or meta tag. Every XSS vulnerability has maximum impact.
|
||
|
||
**Impact:** No defense-in-depth against script injection.
|
||
|
||
**Remediation:** Add a CSP header at the server level. At minimum: `default-src 'self'; script-src 'self'; object-src 'none'; base-uri 'self'`.
|
||
|
||
---
|
||
|
||
### M14. Gmail Token Cache Is Global, Not Per-User
|
||
|
||
**File:** `src/servers/queue/handlers/gmail-sync.ts` (lines 40–46)
|
||
|
||
Module-level `cachedAccessToken` shared across all users.
|
||
|
||
**Impact:** One user's Gmail access token may be used for another user's API calls.
|
||
|
||
**Remediation:** Scope the cache by user email using a `Map`.
|
||
|
||
---
|
||
|
||
## LOW
|
||
|
||
### L1. `TEST_USERS` Backdoor Mechanism in Signin
|
||
|
||
**File:** `src/servers/api/auth/signin.ts` (lines 11, 23, 33)
|
||
|
||
Currently empty, but the code allows bypassing password and passkey verification for listed user IDs.
|
||
|
||
**Remediation:** Remove the mechanism entirely.
|
||
|
||
---
|
||
|
||
### L2. API Keys and OAuth Tokens Stored Unencrypted at Rest
|
||
|
||
**Files:** `src/servers/api/server-settings/pi-mono.ts` (line 10), `src/databases/officer_db/src/store.ts`
|
||
|
||
All provider API keys, Google OAuth tokens, and passkey data stored as plaintext JSON.
|
||
|
||
**Remediation:** Encrypt sensitive values at rest using a key derived from a separate `ENCRYPTION_KEY` env variable.
|
||
|
||
---
|
||
|
||
### L3. Sensitive Credentials Returned to Frontend Unnecessarily
|
||
|
||
**Files:** `src/apps/officer-web/Screens/Dashboard/Settings/IntegrationsSettings/GoogleOAuthConfig.tsx` (line 201), `SMTPSection.tsx`
|
||
|
||
Google OAuth `clientSecret` and SMTP passwords are fetched to frontend state on every settings page load.
|
||
|
||
**Remediation:** Return masked values for display. Only require the full value when updating.
|
||
|
||
---
|
||
|
||
### L4. Host `$HOME` Path Leaked in API Response
|
||
|
||
**File:** `src/servers/api/pi/rest.ts` (line 34)
|
||
|
||
```ts
|
||
return ctx.json({ models, providerNames, hostHome: process.env.HOME ?? '' });
|
||
```
|
||
|
||
**Remediation:** Remove `hostHome` from the response or restrict to Super Admin.
|
||
|
||
---
|
||
|
||
### L5. Verification Codes Not Cleared from URL
|
||
|
||
**Files:** `src/apps/officer-web/Screens/Authentication/Verify.tsx` (line 19), `useResetPassword.ts` (line 14)
|
||
|
||
Codes remain in browser history and `Referer` headers.
|
||
|
||
**Remediation:** Call `history.replaceState(null, '', window.location.pathname)` after reading the code.
|
||
|
||
---
|
||
|
||
### L6. Missing HTTP Security Headers
|
||
|
||
No `X-Content-Type-Options`, `X-Frame-Options`, `Referrer-Policy`, `Permissions-Policy`, or `Strict-Transport-Security` headers.
|
||
|
||
**Remediation:** Add a security headers middleware to `hono.ts`.
|
||
|
||
---
|
||
|
||
### L7. Package Install Endpoints Lack Role Restriction
|
||
|
||
**Files:** `src/servers/api/server-settings/claude-code.ts` (line 24), `opencode.ts` (line 47), `pi-mono.ts` (line 275)
|
||
|
||
Three endpoints run `curl | bash` or `npm install -g` on the host. Even after fixing C3, any authenticated user (not just Super Admin) can trigger installs.
|
||
|
||
**Remediation:** Restrict to Super Admin. Add integrity checks (SHA256 pinning).
|
||
|
||
---
|
||
|
||
### L8. Token Accepted via URL `officerToken` Parameter
|
||
|
||
**File:** `src/workspaces/hooks/src/useAuth/useAuth.ts` (line 27)
|
||
|
||
Token in URL is stored permanently in browser history and sent in `Referer` headers. Not cleared from the URL after consumption.
|
||
|
||
**Remediation:** Use short-lived single-use tokens for embedding. Call `history.replaceState` to remove from URL bar.
|
||
|
||
---
|
||
|
||
### L9. Bootstrap Race Condition
|
||
|
||
**File:** `src/servers/api/auth/auth.ts` (lines 35–36)
|
||
|
||
Two concurrent bootstrap requests could create two Super Admins before `getUserCount()` reflects the first.
|
||
|
||
**Remediation:** Add a mutex or atomic check-and-insert.
|
||
|
||
---
|
||
|
||
## INFO
|
||
|
||
### I1. No Automated Security Test Coverage
|
||
|
||
No tests exist for auth flows, authorization, rate limiting, or path traversal guards.
|
||
|
||
### I2. Stack Traces Logged to Console
|
||
|
||
**File:** `src/servers/hono.ts` (lines 89–90) — full stack traces logged. If logs are user-accessible, this reveals internal paths.
|
||
|
||
### I3. `passwordChangedAt` Not Checked in WebSocket Upgrade
|
||
|
||
**File:** `src/server.tsx` (lines 86–112) — existing WebSocket connections are not invalidated after password change.
|
||
|
||
### I4. Docker Container Map File Has No Integrity Protection
|
||
|
||
**File:** `src/servers/api/terminal/websocket.ts` (line 30) — plain JSON at a known path, tamperable to redirect terminal sessions.
|
||
|
||
### I5. `preinstall` Script Rejects Non-Node 22 but Project Uses Bun
|
||
|
||
**File:** `package.json` (line 11) — inconsistent runtime enforcement.
|
||
|
||
---
|
||
|
||
## Prioritized Remediation Plan
|
||
|
||
### Immediate (do first)
|
||
|
||
1. **Rotate JWT secret** (C1) — generate a 256-bit random value, add startup length check
|
||
2. **Fix CORS** (C2) — replace `origin: '*'` with `ALLOWED_ORIGINS` allowlist
|
||
3. **Protect server-settings routes** (C3) — move inside `protectedRouter` + Super Admin role check
|
||
4. **Protect dev-server proxy** (C4) — require JWT for WebSocket upgrades and HTTP proxy
|
||
5. **Remove `rehype-raw` from chat** (C5) — or add `rehype-sanitize` with strict allowlist
|
||
|
||
### Short-term (next sprint)
|
||
|
||
6. **Move token to `HttpOnly` cookies** (H1, H2) — eliminates XSS token theft and URL leakage
|
||
7. **Add `rehype-sanitize` everywhere** (H3) — all Markdown rendering of user content
|
||
8. **Validate URLs for git-clone/yt-dlp/scrape** (H7, M5) — allow only `http://`/`https://`
|
||
9. **Enable rate limiting by default** (H9) — disable only in `test` environment
|
||
10. **Fix rate limiter key** (H10) — use socket IP, not `X-Forwarded-For`
|
||
11. **Sign OAuth state** (H12) — HMAC with `JWT_SECRET`
|
||
12. **Reduce `maxRequestBodySize`** (H13) — 100 MB or less
|
||
13. **Uncomment body parser error** (M11) — fail loudly on malformed JSON
|
||
|
||
### Medium-term
|
||
|
||
14. **Add Content Security Policy** (M13)
|
||
15. **Add HTTP security headers** (L6)
|
||
16. **Blacklist password reset tokens after use** (M2)
|
||
17. **Always validate password complexity** (M3)
|
||
18. **Scope Gmail token cache per user** (M14)
|
||
19. **Restrict file browser roots** (H8) — remove `~` and `officer.dev` access
|
||
20. **Filter AI subprocess environment** (M9) — never pass `JWT_SECRET` or DB credentials
|
||
21. **Add schema validation to settings PUT** (M4)
|
||
22. **Restrict install endpoints to Super Admin** (L7)
|
||
|
||
### Longer-term
|
||
|
||
23. Encrypt sensitive values at rest (L2)
|
||
24. Add security test coverage (I1)
|
||
25. Implement structured logging with configurable verbosity (I2)
|
||
26. Consider SQLite for auth store (M10)
|
||
27. Remove `TEST_USERS` mechanism (L1)
|