Cherry-pick upstream + post-merge code review fixes (security, dedup, MCP encryption)
From the PR description
Summary
Two layers of work on top of main:
- Cherry-picked 8 upstream PRs from
willchen96/mike(six security/correctness fixes, OpenRouter, MCP Connectors). Original authorship preserved on every commit; PR provenance in each commit body. - Applied a 13-commit code review of the merged result - 4 critical security fixes the upstream PRs left exposed (or introduced), 8 real bugs, and 5 code-quality consolidations including encrypting MCP credentials at rest with the same AES-256-GCM layer PR #42 added for LLM keys.
Layer 1 - Upstream cherry-picks
| Commit | PR | Author | What |
|---|---|---|---|
575b41d |
#21 | @Metbcy | fix(security): fail fast when DOWNLOAD_SIGNING_SECRET missing |
e573204 |
#29 | @fayerman-source | fix(security): remove persisted raw Claude stream log |
6c488bd |
#28 | @fayerman-source | fix(projects): validate folder ownership before mutations |
8a05fe2 |
#2 | @ryanmcdonough | fix(chat): require project access on chat creation endpoint |
1cdbd33 |
#11 | @becker-charles | feat: Add OpenRouter as third LLM provider |
284890d |
#32 | @ZachLaik | feat(mcp): add user-configurable Connectors with OAuth 2.1 (squashed from 3) |
4f988f8 |
#13 | @ryanmcdonough | fix(security): add RLS policies to projects, chats, chat_messages |
9979566 |
#42 | @kveton | fix(security): harden data access, document uploads, secret handling |
git log --format='%an' shows the original author for every cherry-picked commit. Five chore(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.- No size cap on MCP tool output. A misbehaving connector returning multi-megabyte responses would blow the LLM context window, run up token cost, and DoS the chat. Added
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.- Three OpenRouter bugs in PR #11: unconditional
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 magic-byte check pinned to offset 0 rejected legitimate PDFs with a leading newline (PDF spec allows
%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. - Three frontend bugs from PR #42:
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).- Boolean→
\"configured\"sentinel mapping was repeated four times acrossChatInput,TRChatPanel,TabularReviewView,account/models(the source of the TabularReviewView openrouter regression). ExtractedapiKeysFromProfile(profile)helper. - Three-fold provider-key encrypt/decrypt mapping in
userSettings.tsandroutes/user.tscollapsed into shared exports fromapiKeys.ts(PROVIDER_KEY_COLUMNS,buildPlaintextUpgrades,encryptApiKeyInputs). - Three different b64url implementations (
downloadTokens.ts,apiKeys.ts,mcp/oauth.ts) consolidated to Node's nativeBuffer.toString(\"base64url\")(-36 lines). - MCP credential at-rest encryption (the deferred-by-design item in migration
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:
- All long-lived containers healthy; both init containers exited 0
/backend/health,/auth/v1/health,/backend/projects,/backend/chat,/backend/user/mcp-serversall 200- Gemini chat streamed
PONG4end-to-end - Existing plaintext Gemini API key opportunistically encrypted on first authenticated request (DB row went from 39-char
AIzaSyDRmb...to 99-charenc:v1:...) tsc --noEmit -p backendandtsc --noEmit -p frontendboth cleannpm test17/17 pass
Credit
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 MCP dev-secret fallback, user_mcp_servers lockdown gap, OpenRouter model-slug bugs, jsonb contains form in tabular) are good upstream PR candidates independently of this fork's docker stack.
Our analysis
Cherry-pick eight upstream PRs and harden the merged result — read the full analysis →
Think the analysis missed something the PR description covers?
Capture this PR into my fork
Download a Markdown prompt that tells Claude how to port every
commit in this PR into your working tree. Run it via
claude -p < capture-pull-2.md from
inside the repo you want the changes in.