Cherry-pick upstream + post-merge code review fixes (security, dedup, MCP encryption)#2
Open
Cherry-pick upstream + post-merge code review fixes (security, dedup, MCP encryption)#2
Conversation
Resolves the issue where getSecret() silently fell back to the literal string "dev-secret" when neither DOWNLOAD_SIGNING_SECRET nor SUPABASE_SECRET_KEY was set. Because the codebase is public, that fallback let anyone forge valid /download/:token signatures against a mis-configured deployment. - Throw at first call instead of returning the hardcoded string, with a message pointing the operator at \`openssl rand -hex 32\`. - Document DOWNLOAD_SIGNING_SECRET in backend/.env.example so deployers following the README know to set it (and that it should be distinct from SUPABASE_SECRET_KEY). Closes upstream issue willchen96#7. Cherry-picked from upstream PR willchen96#21: fix(security): fail fast when download HMAC secret is missing Author: @Metbcy <https://github.com/Metbcy> Source: willchen96#21 (cherry picked from commit eb44140)
backend/src/lib/llm/claude.ts unconditionally appended every Anthropic stream event to claude-raw-stream.log on disk. That writes full conversation content (user prompts + model output) to a file in the backend's working directory, surviving restarts and untracked by any retention policy. Replace with an env-gated console.debug (DEBUG_LLM_STREAM=true) so debugging is opt-in and never persists. Cherry-picked from upstream PR willchen96#29: fix(security): remove persisted Claude raw stream log Author: @fayerman-source <https://github.com/fayerman-source> Source: willchen96#29 (cherry picked from commit 95cf296)
Several folder-mutation paths in projects.ts accepted folder IDs without first proving the folder belonged to the project being mutated: - moving folders accepted any parent folder ID - moving documents accepted any target folder ID - deleting a folder didn't verify it belonged to the project, and the follow-on document cleanup wasn't scoped to the project either Cross-project folder references are inside Mike's basic trust boundary, so each path now validates the folder against the current project before mutating. Cherry-picked from upstream PR willchen96#28: fix(projects): validate folder ownership before folder mutations Author: @fayerman-source <https://github.com/fayerman-source> Source: willchen96#28 (cherry picked from commit 7062a30)
POST /chat/create accepted any project_id from an authenticated user without verifying the caller could actually access the project, letting authenticated users create chats under arbitrary existing project IDs (app-layer project spoofing). The route now reads userEmail from the auth context and, when project_id is provided, calls checkProjectAccess() before the insert. Access denied → 404 with no row written; access allowed (owner or shared) → unchanged behaviour. Cherry-picked from upstream PR #2: Enhance chat creation endpoint to check project access using user email Author: @ryanmcdonough <https://github.com/ryanmcdonough> Source: willchen96#2 (cherry picked from commit 69c283e)
Adds OpenRouter alongside Claude and Gemini, providing access to GPT, Claude (via OpenRouter routing), and Grok models through a single unified API key. Backend gets a new openrouter.ts streaming implementation using the OpenAI-compatible interface; frontend gets the new provider in ModelToggle and the API-key input in Account → Models. Includes migration 001_add_openrouter_api_key.sql. Replaces the previously-vestigial @openrouter/sdk dependency with a fetch-based backend implementation following the same pattern as claude.ts and gemini.ts. Cherry-picked from upstream PR willchen96#11: feat: Add OpenRouter as third LLM provider Author: @becker-charles <https://github.com/becker-charles> Source: willchen96#11 (cherry picked from commit bb05dd7)
PR willchen96#11 (OpenRouter, commit 1cdbd33) introduced backend/migrations/ 001_add_openrouter_api_key.sql for existing deployments. Our init-db.sh only applied 000_one_shot_schema.sql, so a fresh stack got the column (it was added to the one-shot file too) but a docker compose down+up against an existing volume would skip it. Loop over all 0NN_*.sql files alphabetically after applying the one-shot. Each is idempotent (ADD COLUMN IF NOT EXISTS / CREATE OR REPLACE / etc.) so re-running on every container start is safe.
Adds an MCP (Model Context Protocol) Connectors feature: users can
configure external MCP servers — URL + headers, with optional OAuth
2.1 sign-in — and Mike will list/run their tools as part of the
chat tool surface.
Backend additions:
- backend/src/lib/mcp/{client,oauth,servers,types}.ts — MCP client,
OAuth flow handler, server registry, and shared types
- backend/src/routes/mcpServers.ts + mcpOauth.ts — CRUD for
user-owned MCP servers and the /mcp-oauth/* callback endpoints
- backend/src/lib/chatTools.ts — dispatches tool calls to the
configured MCP servers when the chat asks for them
- New migrations 002_user_mcp_servers.sql and
003_user_mcp_servers_oauth.sql (renumbered locally — PR willchen96#11's
001_add_openrouter_api_key.sql already occupied 001)
- New BACKEND_PUBLIC_URL env var for the OAuth callback URL
Frontend additions:
- /account/mcp settings tab for managing connectors
- McpToggleButton in the chat composer to enable/disable per-message
- Tool-call rendering in AssistantMessage showing MCP-server origin
Cherry-picked from upstream PR willchen96#32: feat(mcp): add Connectors — URL+headers and OAuth 2.1
Author: @ZachLaik <https://github.com/ZachLaik>
Source: willchen96#32
Squashed from upstream commits:
277339f feat(mcp): add user-configurable MCP servers (URL + custom headers)
fad06ac feat(mcp): rename to Connectors, prettier tool calls, observability
52749e6 feat(mcp): OAuth 2.1 sign-in for connectors
Notes:
- Migration files renamed from 001_*/002_* to 002_*/003_* to avoid
collision with PR willchen96#11's 001_add_openrouter_api_key.sql.
- backend/.env.example merge: kept both PR willchen96#21's
DOWNLOAD_SIGNING_SECRET (already in our tree from cherry-pick) and
this PR's BACKEND_PUBLIC_URL.
…AD_SIGNING_SECRET into compose Three env vars introduced by recent upstream cherry-picks need to flow through to the mike-backend container in our self-host stack: - OPENROUTER_API_KEY (PR willchen96#11) — read by lib/llm/openrouter.ts as the fallback when the user hasn't set a per-user key in account settings; matches the existing pattern for ANTHROPIC and GEMINI. - BACKEND_PUBLIC_URL (PR willchen96#32) — used to build the OAuth callback URL for MCP connectors. Defaults to http://${MIKE_HOST}:${MIKE_PORT}/backend which works for laptop traffic; OAuth flows from third-party MCP servers require this URL to actually be reachable from the public internet. - DOWNLOAD_SIGNING_SECRET (PR willchen96#21) — required at backend startup; the previous silent fallback to SUPABASE_SECRET_KEY is preserved as the compose default so existing deployments don't break, but the spec encourages using a dedicated secret. Also adds OPENROUTER_API_KEY and BACKEND_PUBLIC_URL to .env.example with comments.
Until now only user_profiles had row-level security policies; the projects, chats, and chat_messages tables had RLS disabled, meaning the anon/authenticated PostgREST roles could read/write any row. Mike's backend uses the service-role key (which bypasses RLS) so the absence wasn't visible in the API, but anyone using the publishable anon key directly against /rest/v1/* could exfiltrate or mutate arbitrary rows. Adds policies for: - projects: SELECT for owner OR email member of shared_with; INSERT/UPDATE/DELETE owner-only - chats: SELECT for owner OR member of the chat's project; INSERT requires either user-owned (no project) or project access; UPDATE/DELETE owner-only - chat_messages: SELECT/INSERT scoped by access to the parent chat (which itself flows through chats' policies) Closes upstream issue willchen96#12. Cherry-picked from upstream PR willchen96#13: Fix: willchen96#12 - RLS Author: @ryanmcdonough <https://github.com/ryanmcdonough> Source: willchen96#13 (cherry picked from commit 1e73b7a)
Major security pass over Mike's data-access surface:
Backend
- Revoke all anon/authenticated direct Supabase table access. Every
read/write now flows through service-role backend endpoints.
- Encrypt user-stored LLM API keys at rest. Adds USER_API_KEYS_-
ENCRYPTION_KEY env var and a new lib/apiKeys.ts module that
encrypts on write and decrypts on read with opportunistic upgrade
for legacy plaintext rows.
- Validate uploaded file *bytes* (not just extensions) — backend now
refuses files whose magic bytes don't match the declared mime
type, closing the trivial extension-spoof upload bypass.
- Tighten download-token signing — require an explicit
DOWNLOAD_SIGNING_SECRET (no SUPABASE_SECRET_KEY fallback).
- New /user/profile endpoint exposing has_<provider>_api_key
booleans rather than raw key strings.
- Test suite under backend/test/ covering apiKeys, access,
downloadTokens, and upload.
Frontend
- Remove frontend/src/lib/{storage.ts,supabase-server.ts} entirely
— there is no longer a frontend-side S3 client or Supabase server
client. All data access goes through the backend.
- UserProfileContext, signup, account/models, account/mcp,
ChatInput, TRChatPanel, ModelToggle, etc. all rewritten to use
the backend /user/profile endpoint and the "configured" sentinel
pattern (frontend never sees raw API keys).
- Migration 001_security_lockdown.sql revokes anon/authenticated
privileges on every public table and grants service_role only.
Cherry-picked from upstream PR willchen96#42: Harden data access, document uploads, and secret handling
Author: @kveton <https://github.com/kveton>
Source: willchen96#42
(cherry picked from commit 5605a08)
Notes on conflict resolution against earlier cherry-picks on this branch:
- PR willchen96#21 (downloadTokens fail-fast): kept PR willchen96#21's longer error message
pointing at openssl rand -hex 32; took PR willchen96#42's removal of the
SUPABASE_SECRET_KEY fallback.
- PR willchen96#29 (claude.ts stream log fix): re-applied the env-gated
DEBUG_LLM_STREAM debug listener on top of PR willchen96#42's claude.ts.
- PR #2 (chat creation access check): preserved the checkProjectAccess
guard on POST /chat/create.
- PR willchen96#11 (OpenRouter): extended PR willchen96#42's userSettings.ts encrypt/
decrypt logic to also handle openrouter_api_key; added
has_openrouter_api_key to the /user/profile shape; restored
OpenRouter rows in account/models, ChatInput, TRChatPanel,
ModelToggle and UserProfileContext using the same "configured"
sentinel pattern PR willchen96#42 introduced for Claude/Gemini.
- PR willchen96#28 (folder ownership): no semantic conflict; PR willchen96#42 auto-merged.
- PR willchen96#32 (MCP Connectors): preserved chatTools.ts MCP dispatch and
closeMcpServers finally-hook in chat.ts on top of PR willchen96#42's
effectiveProjectId rework.
- backend/.env.example: deduped DOWNLOAD_SIGNING_SECRET, added
USER_API_KEYS_ENCRYPTION_KEY.
- backend/package-lock.json: regenerated via npm install
--package-lock-only --legacy-peer-deps.
…tion ordering Two follow-ups to PR willchen96#42 (security overhaul, commit 9979566): 1. PR willchen96#42's 001_security_lockdown.sql collides with PR willchen96#11's 001_add_openrouter_api_key.sql. Rename to 004_security_lockdown.sql so it slots in after the MCP migrations (002, 003) and runs last. The lockdown only revokes anon/authenticated privileges on tables that exist when it runs — running it after the MCP tables exist means those grants are revoked too, which is the desired shape. 2. The previous init-db.sh shell-glob ('0[1-9][0-9]_*.sql' followed by '00[1-9]_*.sql') would have processed 010-099 BEFORE 001-009 if we ever crossed into double-digit migration numbers. Replace with an ls-sort-while loop that handles strict numeric order for any three-digit prefix and skips 000 explicitly.
…I_KEYS_ENCRYPTION_KEY PR willchen96#42 (security overhaul, commit 9979566) requires USER_API_KEYS_ENCRYPTION_KEY at backend startup, and PR willchen96#21 (commit 575b41d) made DOWNLOAD_SIGNING_SECRET required (no longer silently falls back to SUPABASE_SECRET_KEY). Add both to: - scripts/generate-secrets.sh — random 32-byte hex, idempotent - .env.example — empty placeholder with comment - docker-compose.yml — passed to mike-backend without an empty default (the backend should fail fast if generate-secrets.sh wasn't run, matching upstream's intent)
After PR willchen96#11 (OpenRouter) and PR willchen96#42 (security overhaul) added new deps and PR willchen96#42's conflict resolution regenerated backend/ package-lock.json with --legacy-peer-deps on the host, strict npm ci inside the container fails because the lockfile carries optional peer-dep entries (react, react-dom, scheduler — pulled in transitively, likely via @modelcontextprotocol/sdk) that the strict lockfile-vs-package.json sync check rejects. Match the frontend Dockerfile's pattern (it has done this since the original docker stack landed) by passing --legacy-peer-deps to backend's npm ci. Same pragmatic justification as the frontend case: this is a build-tooling artefact of using vanilla npm against a lockfile that was resolved with the relaxed peer-dep model.
…kens
backend/src/lib/mcp/oauth.ts had a "?? \"dev-secret\"" silent fallback
on the HMAC signing secret used to sign OAuth state tokens. Because the
codebase is public, the fallback secret is known to everyone, so any
deployment that forgot to set DOWNLOAD_SIGNING_SECRET could have its
MCP OAuth flow hijacked: an attacker mints a state token for an arbitrary
{user_id, server_id}, completes the OAuth handshake against the target
provider, and plants their own connector tokens (or hijacks a row).
This re-opened the same vulnerability commit 575b41d closed for download
tokens. lib/downloadTokens.ts already throws with a clear error when the
secret is missing; export that helper as getSigningSecret() and have
mcp/oauth.ts call it. Same throw, same message, single source of truth
for the env var.
PR willchen96#32 (commit 284890d) added the user_mcp_servers table holding MCP connector configuration, including request "headers" (typically a Bearer token) and "oauth_tokens" (refresh + access tokens) for OAuth 2.1 connectors. PR willchen96#42 (commit 9979566) introduced 004_security_ lockdown.sql which revokes anon and authenticated grants from every user-data table — but user_mcp_servers was added between PRs and slipped through. The result: a user authenticated via Supabase could query their own oauth_tokens / headers row directly through PostgREST, bypassing the "backend service-role only" pattern PR willchen96#42 established. Worse, any future regression of the per-row RLS policy would expose every users credentials, not just app data. Add the missing revoke to both 004 and the equivalent block at the bottom of 000_one_shot_schema.sql so fresh deployments are locked down too. Also alter table ... enable row level security for explicitness even though migration 002 already enables it.
Two MCP-routes findings from review:
1. The previous validateUrl in routes/mcpServers.ts allowed every HTTPS
host plus all http://localhost / 127.0.0.1 URLs. From the backend
container, single-label cluster service names ("garage", "postgres",
"gotrue", etc.) resolve to internal docker IPs, so a user could point
an MCP connector at http://garage:3900 (or http://10.0.0.5/admin) and
have the backend proxy authenticated requests through.
Add point-in-time SSRF screening: reject IPv4 literals in
0.0.0.0/8, 10/8, 127/8, 169.254/16, 172.16/12, 192.168/16; reject
IPv6 ::1, fc00::/7, fe80::/10, ::; reject single-label hostnames;
reject http:// (force https) by default. All gated behind
MCP_ALLOW_PRIVATE_HOSTS=true so laptop devs can keep the previous
permissive behaviour. Default-set MCP_ALLOW_PRIVATE_HOSTS=true in
the docker-compose stack since the laptop target is the canonical
audience; production-shape deployments should drop the flag.
Note: this is point-in-time only; a determined attacker can still
perform DNS rebinding to a private IP after validation. Closing
that loop requires per-request DNS resolution + bind-to-IP at fetch
time; flagged as a follow-up.
2. PATCH /user/mcp-servers/:id let a user change the URL on a row whose
oauth_tokens, oauth_metadata, and headers were negotiated/issued for
a different authorization server. The next call would send those
credentials to the new origin (token leak) or fail in confusing ways.
When url changes, clear oauth_tokens, oauth_metadata,
oauth_code_verifier, and headers — the user must re-supply.
Two related issues with the MCP tool-call path:
1. McpHttpClient.callTool returned the joined text content with no upper
bound. A misbehaving connector that emits multi-megabyte responses
would blow the model context window, run up token cost, and
effectively DoS the chat. The user-facing preview was capped via
truncateForPreview but the LLM-side content was not.
2. The dispatcher in chatTools.ts inferred ok-vs-error by sniffing
"MCP tool <name> " as a string prefix. Any tool legitimately
returning a string starting with that prefix would be flagged
failed; any rename of the prefix would silently break detection;
the regex chars in tool names were never escaped.
Replace the string-returning callTool with a structured
{ ok, content, truncated } result. Cap content at MAX_TOOL_CONTENT_BYTES
(default 64 KB; override via MCP_MAX_TOOL_BYTES env var) and append a
[…truncated N bytes; raise MCP_MAX_TOOL_BYTES to see more] marker so
the model knows the output was clipped. Update the LoadedMcpServer
contract in mcp/types.ts and the chatTools.ts dispatcher to consume
the new shape.
…n tabular
Two latent issues in the route-level shared_with paths:
1. tabular.ts:185 still passed a bare JS array to .contains() against a
jsonb column. PostgREST serialises JS arrays as PostgreSQL array
literals ({a,b}) which Postgres rejects when casting to jsonb with
"invalid input syntax for type json". The catch at line 197 logged it
as "shared_with column hasnt been migrated yet" and dropped the
error, so direct-share standalone tabular reviews silently never
appeared for the recipient. Same root cause as commit 653d055 for
projects/access — the tabular site was missed.
2. shared_with values are normalised to lowercase on PATCH/POST (in both
projects and tabular), but the read paths in projects.ts:33,
projects.ts:118, access.ts:148, and tabular.ts:185 passed userEmail
straight from the JWT. Auth providers (Google, Microsoft, magic-link
admins) can issue tokens with mixed-case email claims, and the case
mismatch silently makes the row invisible to those users — even though
RLS policies do use lower() in their WHERE clauses.
Lowercase userEmail at the top of each affected scope and use the
lowercased value in every .contains() / .includes() shortcut. Stored
data is unchanged.
Add DEBUG_LLM_STREAM guard around the SSE chunk console.log in openrouter.ts, mirroring the pattern already used in claude.ts. Without the guard every SSE frame was printed to stdout in production. Backfill the tool-call id when OpenRouter defers it to a later delta. Some models send the real id in chunk 2; the first delta got a synthetic "tool-<index>" fallback that was never replaced, causing tool_result messages to reference an id the model never sent. Fix the two anthropic-via-OpenRouter model slugs in models.ts: the period in claude-sonnet-4.6 and claude-opus-4.7 must be a hyphen (claude-sonnet-4-6, claude-opus-4-7) to match OpenRouter's documented routing slugs.
…ip-size guard
Fix A (PDF magic-byte scan): PDF 1.4 §3.4.1 permits %PDF- within the first
1024 bytes, not strictly at offset 0. Replaced the offset-0 equality check
with a subarray(0, 1024).includes() scan so PDFs with leading whitespace or
BOM bytes are accepted.
Fix B (DOCX zip-bomb guard): The previous guard read JSZip's private
_data.uncompressedSize field, which returns undefined for store-only
(method=0) entries, silently bypassing the limit. Replaced with an explicit
decompression loop using the public entry.async("nodebuffer") API, bailing
out as soon as the running total exceeds 100 MB.
…ider tooltip) - AuthContext: await ensureProfile() at both call sites (checkUser and onAuthStateChange) so the POST /user/profile completes before setAuthLoading(false) fires, eliminating the race where UserProfileContext could GET /user/profile and 404 before the row exists. - TabularReviewView: add openrouterApiKey to the apiKeys object passed to isModelAvailable(), matching ChatInput.tsx and TRChatPanel.tsx; without it an OpenRouter tabular model was silently blocked and the wrong modal opened. - account/models/page.tsx: replace 2-arm conditional in the disabled-model tooltip with a 3-arm one so OpenRouter models no longer incorrectly tell users to "Add a Gemini API key".
The function was duplicated verbatim in both backend/src/routes/documents.ts and backend/src/routes/projects.ts, with only minor comment divergences that would cause future fixes to silently miss one copy. Per the reviewer's finding, documents.ts is the canonical home as it is the document-focused route file and already hosts related helpers (countPdfPages, extractStructureTree). The copy in projects.ts — along with its private countPdfPages and extractStructureTree helpers — has been removed; projects.ts now imports handleDocumentUpload from ./documents instead.
…d dup The boolean→"configured" sentinel mapping for claudeApiKey, geminiApiKey, and openrouterApiKey was repeated verbatim in ChatInput.tsx, TRChatPanel.tsx, TabularReviewView.tsx, and account/models/page.tsx. This duplication directly caused the TabularReviewView regression where openrouterApiKey was missing, flagged by reviewers as Tier 3 willchen96#19. A single apiKeysFromProfile helper is now exported from modelAvailability.ts, and UserProfile is exported from UserProfileContext.tsx to type it; all four sites use the helper.
userSettings.ts had three near-identical "if stored && plaintext then re-encrypt" blocks (one per provider). routes/user.ts re-implemented the same claude→claude_api_key / encryptApiKey mapping independently. Adds to apiKeys.ts: - PROVIDER_KEY_COLUMNS / ProviderKeyColumn - the canonical list of encrypted columns - PROVIDER_KEY_COLUMN_BY_INPUT - maps frontend names to DB columns - buildPlaintextUpgrades() - opportunistic-upgrade helper used by userSettings.ts (replaces the three per-provider if-blocks with a loop) - encryptApiKeyInputs() - PATCH-path helper used by routes/user.ts (replaces the three per-provider if/encryptApiKey calls)
Three places implemented base64url encoding independently: downloadTokens.ts
and mcp/oauth.ts each had manual b64urlEncode/b64urlDecode helpers using
.replace() chains to swap +/- and /_ and pad/strip =, while apiKeys.ts
already used the native base64url encoding but still wrapped it in thin
b64url/fromB64url functions. Node 16+ supports Buffer.toString("base64url")
and Buffer.from(s, "base64url") natively, so all three manual variants are
deleted and every call site is inlined to the built-in codec directly.
Threat model: a database compromise or stolen backup of user_mcp_servers
should not yield usable bearer tokens or refresh tokens for third-party
connectors. Migration 003 explicitly deferred encryption ("Per-row
encryption is intentionally deferred to a separate hardening PR"); this
is that PR.
Storage format: serialize the JSON value, encrypt the resulting string
with the existing AES-256-GCM "enc:v1:" envelope from apiKeys.ts, and
store the ciphertext as a JSON-string scalar in the existing jsonb
column (a JSON string is itself valid jsonb, so no schema change is
needed). Encrypting the whole blob keeps the format trivial, avoids
leaking shape ("this row has a refresh_token"), and minimizes cipher
operations vs per-leaf encryption. On read we sniff
typeof === "string" && startsWith("enc:v1:") to distinguish from legacy
plaintext objects.
New helpers encryptJsonBlob / decryptJsonBlob / needsJsonBlobUpgrade
live alongside the existing per-string helpers in apiKeys.ts and reuse
them underneath. Call sites updated: DbOAuthProvider.tokens() /
saveTokens() encrypt+decrypt oauth_tokens; saveCodeVerifier() /
codeVerifier() encrypt the PKCE verifier (using the per-string helper
since it's a single text column); mcpServers.ts POST/PATCH/test
encrypt+decrypt headers; loadEnabledMcpServersForUser decrypts each
row in place and fires off a best-effort UPDATE to upgrade legacy
plaintext rows in the background, mirroring the lazy-upgrade pattern
PR willchen96#42 introduced for LLM provider keys (commit 701535b). The
upgrade write is fire-and-forget so a failing-forever encryption
write cannot block the chat hot path on every turn, but errors are
logged so they can be detected. oauth_metadata stays plaintext —
it's discovery + DCR data, not secret. If
USER_API_KEYS_ENCRYPTION_KEY is unset, encryptApiKey throws and the
write fails closed, which is the correct behavior.
Lef-F
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two layers of work on top of
main:willchen96/mike(six security/correctness fixes, OpenRouter, MCP Connectors). Original authorship preserved on every commit; PR provenance in each commit body.Layer 1 — Upstream cherry-picks
575b41dfix(security): fail fast when DOWNLOAD_SIGNING_SECRET missinge573204fix(security): remove persisted raw Claude stream log6c488bdfix(projects): validate folder ownership before mutations8a05fe2fix(chat): require project access on chat creation endpoint1cdbd33feat: Add OpenRouter as third LLM provider284890dfeat(mcp): add user-configurable Connectors with OAuth 2.1(squashed from 3)4f988f8fix(security): add RLS policies to projects, chats, chat_messages9979566fix(security): harden data access, document uploads, secret handlinggit log --format='%an'shows the original author for every cherry-picked commit. Fivechore(self-host):commits sit alongside (f4d8045,11da4b9,9f40245,c5e6141,597a219) where the cherry-picks needed the docker stack's compose / migrations / Dockerfile to learn about new env vars and migration filename collisions.Layer 2 — Code review fixes
A six-agent review of the merged tree turned up critical security holes the upstream PRs left exposed and a handful of real bugs. All landed as fix-up commits attributed to us (the upstream-attributed commits are unchanged).
Tier 1 — Critical security (
c5741fc,d5e0f74,da0bc6d,31c1817)mcp/oauth.tshad?? \"dev-secret\"fallback for the HMAC signing secret, re-opening the exact vulnerability commit575b41dhad just closed for download tokens. Forge an OAuth state token for any{user_id, server_id}and complete the MCP OAuth flow as a victim. Now reuseslib/downloadTokens.ts'sgetSigningSecret()helper — single throw-on-missing source of truth.004_security_lockdown.sqlskippeduser_mcp_servers— the table holdingheaders(Bearer tokens) andoauth_tokens. Every other user-data table was locked down behind service-role-only; the most secret table was exempt. Now revoked in both004and the matching block in000_one_shot_schema.sql.mcpServers.tsURL allowlist permitted SSRF. Allowed all HTTPS hosts and allhttp://localhost/127.0.0.1. From the backend container,http://garage:3900andhttp://10.0.0.5/adminwere reachable through. Added IP-literal screens for0/8,10/8,127/8,169.254/16,172.16/12,192.168/16, IPv6::1/fc00::/7/fe80::/10/::, and single-label hostnames. Gated behindMCP_ALLOW_PRIVATE_HOSTS=trueso laptop devs keep the permissive default. Same commit also fixes a token-leak bug: PATCH/user/mcp-servers/:idwould leturlchange while keepingoauth_tokensfrom the previous origin.MCP_MAX_TOOL_BYTEScap (default 64 KB) with a truncation marker, and replaced the fragilecontent.startsWith(\"MCP tool '<name>' \")ok-detection with a structured{ ok, content, truncated }return shape.Tier 2 — Real bugs (
908f8e1,3d46ed4,2740fa8,a96867e)tabular.ts.contains(\"shared_with\", [array])— same jsonb form bug we fixed inprojects.ts/access.ts; the catch silently logged it as "column hasn't been migrated yet", masking it. Direct-share standalone tabular reviews never appeared for recipients. Plus case-insensitive email comparisons in three sites: storedshared_withis lowercased on PATCH/POST but reads passed JWT email raw, and providers can issue mixed-case email claims.console.logper SSE chunk (production log flood), tool-call id never backfilled when the model defers it to a later delta (multi-turn tool use breaks), and anthropic-via-openrouter model IDs used4.6/4.7(period) instead of OpenRouter's4-6/4-7(hyphen) slug — every call would 404.%PDF-anywhere in first 1024 bytes). Plus the DOCX zip-bomb guard read JSZip's_data.uncompressedSizeprivate API which returnsundefinedfor store-only entries — store-compressed bombs bypassed the check. Now decompresses each entry via the publicentry.async(\"nodebuffer\")and accumulates against a 100 MB cap.ensureProfile()not awaited inAuthContext→ first-load race where a 404 makes the UI flash a fallback profile;TabularReviewViewbuiltapiKeyswithoutopenrouterApiKeyso OpenRouter tabular runs were silently blocked; tooltip on disabled model row said "Add a Gemini key" for OpenRouter models.Tier 3 — Code quality / dedup (
ca110ef,5355113,701535b,693f133,c4ff092)handleDocumentUploadwas duplicated verbatim acrossdocuments.tsandprojects.ts(-212 lines after consolidation).\"configured\"sentinel mapping was repeated four times acrossChatInput,TRChatPanel,TabularReviewView,account/models(the source of the TabularReviewView openrouter regression). ExtractedapiKeysFromProfile(profile)helper.userSettings.tsandroutes/user.tscollapsed into shared exports fromapiKeys.ts(PROVIDER_KEY_COLUMNS,buildPlaintextUpgrades,encryptApiKeyInputs).downloadTokens.ts,apiKeys.ts,mcp/oauth.ts) consolidated to Node's nativeBuffer.toString(\"base64url\")(-36 lines).003's comment):headers,oauth_tokens, andoauth_code_verifiernow go through the same AES-256-GCMenc:v1:envelope as LLM keys. Lazy-upgrade pattern: on first read, plaintext rows are decrypted as identity and a fire-and-forget UPDATE encrypts them. New helpersencryptJsonBlob/decryptJsonBlob/needsJsonBlobUpgradeinapiKeys.ts. Test suite went 11→17.Verification
Smoke-tested the merged stack end-to-end on
http://localhost:/backend/health,/auth/v1/health,/backend/projects,/backend/chat,/backend/user/mcp-serversall 200PONG4end-to-endAIzaSyDRmb...to 99-charenc:v1:...)tsc --noEmit -p backendandtsc --noEmit -p frontendboth cleannpm test17/17 passCredit
Every cherry-picked commit preserves its original author in
git log. The 13 fix-up commits in Layer 2 are attributed to us, not to the upstream authors — they're our reactions to a code review of the merged result, not changes to upstream's intent. Several of the Tier 1 / Tier 2 fixes (especially the MCPdev-secretfallback,user_mcp_serverslockdown gap, OpenRouter model-slug bugs, jsonbcontainsform in tabular) are good upstream PR candidates independently of this fork's docker stack.