-
Notifications
You must be signed in to change notification settings - Fork 1
Fix MCP env aliasing, explicit missing-env errors, and add memory smoke diagnostics #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| export type PandoraMcpEnvStatus = { ok: true; value: string; envVar: string } | { ok: false; envVar: string; aliases: string[]; message: string }; | ||
|
|
||
| function firstPresent(env: Partial<NodeJS.ProcessEnv>, names: string[]): PandoraMcpEnvStatus { | ||
| const found = names.find((name) => Boolean(env[name])); | ||
| if (found) return { ok: true, value: String(env[found]), envVar: found }; | ||
| return { ok: false, envVar: names[0], aliases: names.slice(1), message: `Missing server env: ${names[0]}` }; | ||
| } | ||
|
|
||
| export function getPandoraMcpBearerSecret(env: Partial<NodeJS.ProcessEnv> = process.env) { | ||
| return firstPresent(env, ["PANDORA_MCP_TOKEN", "PANDORA_MCP_API_KEY", "PANDORA_API_KEY", "MEMORY_API_KEY"]); | ||
| } | ||
|
|
||
| export function getPandoraMcpDbKey(env: Partial<NodeJS.ProcessEnv> = process.env) { | ||
| return firstPresent(env, ["PANDORA_MCP_DB_KEY", "PANDORA_MEMORY_BRIDGE_DB_KEY", "SUPABASE_SERVICE_ROLE_KEY"]); | ||
| } | ||
|
Comment on lines
+13
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift DB key fallback widens blast radius to full service-role privileges.
Consider making the service-role fallback opt-in (e.g., a separate explicit flag) rather than a silent alias, so misconfigured deployments fail loudly instead of quietly running with elevated privileges. 🤖 Prompt for AI Agents |
||
|
|
||
| export function getPandoraSupabaseUrl(env: Partial<NodeJS.ProcessEnv> = process.env) { | ||
| return firstPresent(env, ["NEXT_PUBLIC_SUPABASE_URL", "SUPABASE_URL"]); | ||
| } | ||
|
|
||
| export function presentEnvName(env: Partial<NodeJS.ProcessEnv>, names: string[]) { | ||
| return names.find((name) => Boolean(env[name])) ?? null; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import { createClient } from "@supabase/supabase-js"; | ||
| import { getPandoraMcpDbKey, getPandoraSupabaseUrl, presentEnvName } from "../lib/services/pandora-mcp-env"; | ||
| import { analyzeMemoryCandidatesTool, captureAdaptiveMemoryTool, captureMemoryEventTool, createSessionDigestTool, distillContextPackTool, getAdaptiveContextTool, getLatestContextPackTool, getMemoryContextTool, getOpenLoopsTool } from "../lib/services/pandora-mcp-tools"; | ||
| import type { MemoryBridgeDbClient } from "../lib/services/memory-bridge-service"; | ||
|
|
||
| const seed = "Creative workflow preference: preserve continuity and user feedback for future writing."; | ||
| const ns = (process.env.PANDORA_MEMORY_SMOKE_NAMESPACE === "au" ? "au" : "real_life") as "real_life" | "au"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Default namespace risks polluting/leaking real data.
Default to the safe test namespace and require explicit opt-in to touch 🔧 Proposed fix-const ns = (process.env.PANDORA_MEMORY_SMOKE_NAMESPACE === "au" ? "au" : "real_life") as "real_life" | "au";
+const ns = (process.env.PANDORA_MEMORY_SMOKE_NAMESPACE === "real_life" ? "real_life" : "au") as "real_life" | "au";Also applies to: 21-35 🤖 Prompt for AI Agents |
||
| const userId = process.env.PANDORA_MCP_USER_ID ?? process.env.PANDORA_MEMORY_BRIDGE_USER_ID ?? ""; | ||
| const env = process.env; | ||
| const required = ["PANDORA_ENABLE_MCP", "PANDORA_ENABLE_MCP_CAPTURE", "PANDORA_ENABLE_MCP_DISTILLATION", "PANDORA_ENABLE_EMBEDDINGS", "PANDORA_ENABLE_MODEL_CALLS", "PANDORA_ENABLE_SEMANTIC_RETRIEVAL", "OPENAI_API_KEY", "EMBEDDING_MODEL", "LLM_MODEL", "PANDORA_EMBEDDING_MODEL", "PANDORA_MEMORY_EMBEDDING_MODEL"]; | ||
| const aliases = { apiKey: ["PANDORA_MCP_TOKEN", "PANDORA_MCP_API_KEY", "PANDORA_API_KEY", "MEMORY_API_KEY"], dbKey: ["PANDORA_MCP_DB_KEY", "PANDORA_MEMORY_BRIDGE_DB_KEY", "SUPABASE_SERVICE_ROLE_KEY"], url: ["NEXT_PUBLIC_SUPABASE_URL", "SUPABASE_URL"], anon: ["NEXT_PUBLIC_SUPABASE_ANON_KEY", "SUPABASE_ANON_KEY", "NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY"] }; | ||
| function flag(name: string) { return env[name] === "true" ? "enabled" : "disabled"; } | ||
| function out(name: string, ok: boolean, detail = "") { console.log(`${ok ? "PASS" : "FAIL"} ${name}${detail ? ` - ${detail}` : ""}`); if (!ok) process.exitCode = 1; } | ||
| async function step(name: string, fn: () => Promise<unknown>, optional = false) { try { const r = await fn(); const text = JSON.stringify(r); const blocked = text.includes('"ok":false') || text.includes('Invalid API key'); out(name, optional || !blocked, text.slice(0, 500)); } catch (e) { out(name, false, e instanceof Error ? e.message : String(e)); } } | ||
| async function main() { | ||
| console.log("Pandora memory diagnostics (secret values redacted)"); | ||
| for (const k of required) console.log(`ENV ${k}: ${env[k] ? "present" : "missing"}${k.startsWith("PANDORA_ENABLE") ? ` (${flag(k)})` : ""}`); | ||
| for (const [label, names] of Object.entries(aliases)) console.log(`ENV ${label}: ${presentEnvName(env, names) ?? "missing"}`); | ||
| console.log(`provider: ${env.OPENAI_API_KEY ? "openai_configured" : "deterministic/no_provider"}`); | ||
| console.log(`distillation: ${flag("PANDORA_ENABLE_MCP_DISTILLATION")}; embeddings: ${flag("PANDORA_ENABLE_EMBEDDINGS")}; model_calls: ${flag("PANDORA_ENABLE_MODEL_CALLS")}`); | ||
| const url = getPandoraSupabaseUrl(env); const db = getPandoraMcpDbKey(env); | ||
| if (!url.ok || !db.ok || !userId) { out("bootstrap", false, [!url.ok && url.message, !db.ok && db.message, !userId && "Missing server env: PANDORA_MCP_USER_ID or PANDORA_MEMORY_BRIDGE_USER_ID"].filter(Boolean).join("; ")); return; } | ||
| const client = createClient(url.value, db.value, { auth: { autoRefreshToken: false, persistSession: false } }) as unknown as MemoryBridgeDbClient; | ||
| const principal = { ok: true as const, authType: "mcp_bearer_token" as const, userId }; | ||
| await step("supabase connection/service-role memory table read", async () => client.from("memory_events").select("id").eq("user_id", userId).eq("namespace", ns).limit(1)); | ||
| await step("analyze_memory_candidates", async () => analyzeMemoryCandidatesTool(client, principal, { namespace: ns, text: seed, source: "memory_smoke", mode: "candidate_only" }, env)); | ||
| await step("capture_adaptive_memory candidate_only", async () => captureAdaptiveMemoryTool(client, principal, { namespace: ns, text: seed, source: "memory_smoke", mode: "candidate_only" }, env)); | ||
| await step("capture_adaptive_memory auto_capture_allowed", async () => captureAdaptiveMemoryTool(client, principal, { namespace: ns, text: seed, source: "memory_smoke", mode: "auto_capture_allowed" }, env)); | ||
| await step("capture_memory_event", async () => captureMemoryEventTool(client, principal, { namespace: ns, raw_text: seed, source: "memory_smoke", sensitivity: "low" }, env), env.PANDORA_ENABLE_MCP_CAPTURE !== "true"); | ||
|
Comment on lines
+26
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| await step("get_memory_context", async () => getMemoryContextTool(client, principal, { namespace: ns, query: seed, max_items: 5 })); | ||
| await step("get_adaptive_context", async () => getAdaptiveContextTool(client, principal, { namespace: ns, query: seed, max_items: 5 })); | ||
| await step("create_session_digest", async () => createSessionDigestTool(client, principal, { namespace: ns, source: "memory_smoke", transcript_or_summary: seed }, env), env.PANDORA_ENABLE_MCP_CAPTURE !== "true" || env.PANDORA_ENABLE_MCP_DISTILLATION !== "true"); | ||
| await step("get_latest_context_pack", async () => getLatestContextPackTool(client, principal, { namespace: ns })); | ||
| await step("distill master context pack", async () => distillContextPackTool(client, principal, { namespace: ns, pack_type: "master" }, env), env.PANDORA_ENABLE_MCP_DISTILLATION !== "true"); | ||
| await step("retrieve open loops", async () => getOpenLoopsTool(client, principal, { namespace: ns })); | ||
| } | ||
| main().catch((e) => { out("memory smoke", false, e instanceof Error ? e.message : String(e)); }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { getPandoraMcpBearerSecret, getPandoraMcpDbKey, getPandoraSupabaseUrl } from "@/lib/services/pandora-mcp-env"; | ||
| import { resolvePandoraMcpPrincipal } from "@/lib/services/mcp-auth"; | ||
|
|
||
| function req(token: string) { return new Request("https://example.com/api/mcp", { headers: { authorization: `Bearer ${token}` } }); } | ||
|
|
||
| describe("Pandora MCP environment aliases", () => { | ||
| it("accepts deployed API key aliases and service-role db key aliases", () => { | ||
| expect(getPandoraMcpBearerSecret({ PANDORA_MCP_API_KEY: "tool-key" })).toMatchObject({ ok: true, envVar: "PANDORA_MCP_API_KEY" }); | ||
| expect(getPandoraMcpDbKey({ SUPABASE_SERVICE_ROLE_KEY: "service-role" })).toMatchObject({ ok: true, envVar: "SUPABASE_SERVICE_ROLE_KEY" }); | ||
| expect(getPandoraSupabaseUrl({ SUPABASE_URL: "https://example.supabase.co" })).toMatchObject({ ok: true, envVar: "SUPABASE_URL" }); | ||
| }); | ||
|
|
||
| it("reports a missing server env instead of generic invalid API key when no MCP token is configured", () => { | ||
| const result = resolvePandoraMcpPrincipal(req("tool-key"), { PANDORA_ENABLE_MCP: "true", PANDORA_MCP_USER_ID: "user-1", SUPABASE_SERVICE_ROLE_KEY: "service-role" }); | ||
| expect(result).toMatchObject({ ok: false, code: "mcp_token_env_missing", message: "Missing server env: PANDORA_MCP_TOKEN" }); | ||
| }); | ||
|
|
||
| it("reports disabled capture and distillation with explicit env names", async () => { | ||
| const { requireMcpCaptureEnabled, requireMcpDistillationEnabled } = await import("@/lib/services/mcp-auth"); | ||
| expect(requireMcpCaptureEnabled({ PANDORA_ENABLE_MCP_CAPTURE: "false" }).message).toBe("capture_disabled: PANDORA_ENABLE_MCP_CAPTURE is not true"); | ||
| expect(requireMcpDistillationEnabled({ PANDORA_ENABLE_MCP_DISTILLATION: "false" }).message).toBe("distillation_disabled: PANDORA_ENABLE_MCP_DISTILLATION is not true"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing bearer-secret env blocks OAuth authentication entirely.
resolvePandoraMcpPrincipalreturnsmcp_token_env_missingat Line 27 as soon asgetPandoraMcpBearerSecretfails, before ever checkingsuppliedTokenor reaching the OAuth branch (Line 35). Since OAuth (verifyPandoraMcpOAuthAccessToken) verifies against a completely independent secret, a deployment that intentionally relies solely on OAuth (noPANDORA_MCP_TOKEN/PANDORA_MCP_API_KEY/PANDORA_API_KEY/MEMORY_API_KEYset) will have every request — including ones bearing a perfectly valid OAuth access token — rejected with a "missing server env" error. OAuth-only auth becomes impossible.Reorder so OAuth is attempted whenever the static secret doesn't match/isn't configured, and only report
mcp_token_env_missingas the final fallback when both mechanisms fail.🔧 Proposed fix
export function resolvePandoraMcpPrincipal(request: Request, env: Partial<NodeJS.ProcessEnv> = process.env): PandoraMcpPrincipal { if (env.PANDORA_ENABLE_MCP !== "true") return { ok: false, status: 403, code: "mcp_disabled", message: "Pandora MCP is disabled." }; const configuredToken = getPandoraMcpBearerSecret(env); const suppliedToken = bearerToken(request); - if (!configuredToken.ok) return { ok: false, status: 403, code: "mcp_token_env_missing", message: configuredToken.message }; if (!suppliedToken) return { ok: false, status: 401, code: "mcp_token_missing", message: "MCP bearer token is required." }; - if (safeEqual(suppliedToken, configuredToken.value)) { + if (configuredToken.ok && safeEqual(suppliedToken, configuredToken.value)) { if (!env.PANDORA_MCP_USER_ID) return { ok: false, status: 403, code: "mcp_user_id_missing", message: "Missing server env: PANDORA_MCP_USER_ID" }; const dbKey = getPandoraMcpDbKey(env); if (!dbKey.ok) return { ok: false, status: 403, code: "mcp_db_key_missing", message: dbKey.message }; return { ok: true, authType: "mcp_bearer_token", userId: env.PANDORA_MCP_USER_ID }; } const oauth = verifyPandoraMcpOAuthAccessToken(suppliedToken, env); - if (!oauth.ok) return { ok: false, status: 401, code: "mcp_token_invalid", message: "MCP bearer token is invalid." }; - if (!env.PANDORA_MCP_USER_ID) return { ok: false, status: 403, code: "mcp_user_id_missing", message: "Missing server env: PANDORA_MCP_USER_ID" }; - const dbKey = getPandoraMcpDbKey(env); - if (!dbKey.ok) return { ok: false, status: 403, code: "mcp_db_key_missing", message: dbKey.message }; - return { ok: true, authType: "mcp_oauth_access_token", userId: oauth.payload.user_id }; + if (oauth.ok) { + if (!env.PANDORA_MCP_USER_ID) return { ok: false, status: 403, code: "mcp_user_id_missing", message: "Missing server env: PANDORA_MCP_USER_ID" }; + const dbKey = getPandoraMcpDbKey(env); + if (!dbKey.ok) return { ok: false, status: 403, code: "mcp_db_key_missing", message: dbKey.message }; + return { ok: true, authType: "mcp_oauth_access_token", userId: oauth.payload.user_id }; + } + if (!configuredToken.ok) return { ok: false, status: 403, code: "mcp_token_env_missing", message: configuredToken.message }; + return { ok: false, status: 401, code: "mcp_token_invalid", message: "MCP bearer token is invalid." }; }📝 Committable suggestion
🤖 Prompt for AI Agents