mirror of
https://github.com/DayuanJiang/next-ai-draw-io.git
synced 2026-10-12 20:49:53 +08:00
fix(server): keep users' keys at their own endpoints, and more fixes from the third review
- Bedrock: a user's AWS keys no longer go to an endpoint the server sets in AWS_ENDPOINT_URL_BEDROCK_RUNTIME / AWS_ENDPOINT_URL (read by the upgraded SDK), and admin panel keys win over AWS_BEARER_TOKEN_BEDROCK, as the Test button checks them. Checked with Bedrock. - Ollama: a server key without a base URL (admin panel, OLLAMA_API_KEY) goes to Ollama Cloud, as env.example says, instead of 127.0.0.1. - Quota: EdgeOne counts whatever key header comes along, keyless Ollama at a private address counts, and their provider texts stay in the log. - An EdgeOne server model (admin panel, ai-models.json) works: the route checked the raw provider header, which holds the name's slug. - parse-url ends downloads it does not read (too large, PDF, errors). - Desktop app: the port follows where the chats are (IndexedDB per origin) instead of a remembered port, which could hide them for good; a same-port restart tells the page to refetch the server models; a failed preset switch no longer undoes a newer choice; a presets file removed after a failed read can be saved again; .env values quoted from start to end keep their inner quotes, as dotenv reads them.
This commit is contained in:
@@ -159,6 +159,8 @@ describe("Bedrock admin panel credentials", () => {
|
||||
region: "eu-west-1",
|
||||
accessKeyId: "panel-id",
|
||||
secretAccessKey: "panel-secret",
|
||||
// The keys the Test button checked, not AWS_BEARER_TOKEN_BEDROCK
|
||||
apiKey: "",
|
||||
})
|
||||
})
|
||||
|
||||
@@ -182,6 +184,8 @@ describe("Bedrock admin panel credentials", () => {
|
||||
secretAccessKey: "client-secret",
|
||||
// The SDK would otherwise use the server's AWS_BEARER_TOKEN_BEDROCK
|
||||
apiKey: "",
|
||||
// and the server's AWS_ENDPOINT_URL_BEDROCK_RUNTIME
|
||||
baseURL: "https://bedrock-runtime.ap-northeast-1.amazonaws.com",
|
||||
})
|
||||
})
|
||||
|
||||
@@ -349,6 +353,23 @@ describe("whose keys a request uses", () => {
|
||||
)
|
||||
})
|
||||
|
||||
it("sends the server's Ollama key without a base URL to Ollama Cloud", async () => {
|
||||
// An Ollama key is an Ollama Cloud key: local Ollama has none.
|
||||
// The admin panel saves it as OLLAMA_API_KEY, without a base URL.
|
||||
process.env.OLLAMA_API_KEY = "server-key"
|
||||
const { createOllama } = await import("ollama-ai-provider-v2")
|
||||
getAIModel({ provider: "ollama", modelId: "m" })
|
||||
expect(createOllama).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({ baseURL: "https://ollama.com/api" }),
|
||||
)
|
||||
// Without a key: the SDK's local default
|
||||
delete process.env.OLLAMA_API_KEY
|
||||
getAIModel({ provider: "ollama", modelId: "m" })
|
||||
expect(vi.mocked(createOllama).mock.lastCall?.[0]).not.toHaveProperty(
|
||||
"baseURL",
|
||||
)
|
||||
})
|
||||
|
||||
it("needs a base URL with a user's Azure key", () => {
|
||||
// The SDK would otherwise read the server's AZURE_RESOURCE_NAME
|
||||
process.env.AZURE_RESOURCE_NAME = "operator-resource"
|
||||
|
||||
@@ -462,9 +462,11 @@ describe("Ollama API key security", () => {
|
||||
|
||||
expect(createOllamaMock).toHaveBeenCalledTimes(1)
|
||||
const callArgs = createOllamaMock.mock.calls[0][0]
|
||||
expect(callArgs).not.toHaveProperty("baseURL")
|
||||
// As env.example says: without OLLAMA_BASE_URL, Ollama Cloud (the
|
||||
// SDK's default is the local server, which has no keys)
|
||||
expect(callArgs).toEqual(
|
||||
expect.objectContaining({
|
||||
baseURL: "https://ollama.com/api",
|
||||
headers: { Authorization: "Bearer server-key" },
|
||||
}),
|
||||
)
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
// @vitest-environment node
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest"
|
||||
|
||||
vi.mock("electron", () => ({
|
||||
app: {
|
||||
isPackaged: true,
|
||||
getName: () => "app",
|
||||
getVersion: () => "1",
|
||||
getLocale: () => "en",
|
||||
},
|
||||
BrowserWindow: { getFocusedWindow: () => null },
|
||||
dialog: {},
|
||||
Menu: { buildFromTemplate: () => ({}), setApplicationMenu: () => {} },
|
||||
shell: {},
|
||||
}))
|
||||
|
||||
// The saved current preset, and restarts that wait until the test ends them
|
||||
const state = vi.hoisted(() => ({
|
||||
current: "A" as string | null,
|
||||
restarts: [] as Array<{ resolve: () => void; reject: (e: Error) => void }>,
|
||||
}))
|
||||
vi.mock("@/electron/main/config-manager", () => ({
|
||||
applyPresetToEnv: (id: string) => {
|
||||
state.current = id
|
||||
return { AI_PROVIDER: id }
|
||||
},
|
||||
getAllPresets: () => [],
|
||||
getCurrentPresetId: () => state.current,
|
||||
setCurrentPreset: (id: string | null) => {
|
||||
state.current = id
|
||||
return true
|
||||
},
|
||||
}))
|
||||
vi.mock("@/electron/main/next-server", () => ({
|
||||
restartNextServer: () =>
|
||||
new Promise<void>((resolve, reject) =>
|
||||
state.restarts.push({ resolve, reject }),
|
||||
),
|
||||
}))
|
||||
vi.mock("@/electron/main/menu-i18n", () => ({
|
||||
getMenuTranslations: () => new Proxy({}, { get: () => "x" }),
|
||||
getPreferredLocale: () => "en",
|
||||
}))
|
||||
vi.mock("@/electron/main/settings-window", () => ({
|
||||
showSettingsWindow: () => {},
|
||||
}))
|
||||
|
||||
import { switchPreset } from "@/electron/main/app-menu"
|
||||
|
||||
beforeEach(() => {
|
||||
state.current = "A"
|
||||
state.restarts = []
|
||||
})
|
||||
|
||||
describe("switchPreset", () => {
|
||||
it("keeps a preset chosen while a failed switch was restarting", async () => {
|
||||
const toB = switchPreset("B").catch(() => {})
|
||||
const toC = switchPreset("C")
|
||||
// B's restart fails after the user already picked C
|
||||
state.restarts[0].reject(new Error("timed out"))
|
||||
await new Promise((r) => setTimeout(r, 0))
|
||||
for (const r of state.restarts.slice(1)) r.resolve()
|
||||
await toB
|
||||
await toC
|
||||
expect(state.current).toBe("C")
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,79 @@
|
||||
// @vitest-environment node
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
|
||||
|
||||
vi.mock("@/lib/dynamo-quota-manager", () => ({
|
||||
isQuotaEnabled: () => false,
|
||||
checkAndIncrementRequest: async () => ({ allowed: true }),
|
||||
recordTokenUsage: async () => {},
|
||||
}))
|
||||
|
||||
import { POST as chat } from "@/app/api/chat/route"
|
||||
|
||||
const ENV = ["AI_MODELS_CONFIG", "AI_PROVIDER", "AI_MODEL"]
|
||||
const saved: Record<string, string | undefined> = {}
|
||||
const calls: Array<{ url: string; headers: Headers }> = []
|
||||
|
||||
beforeEach(() => {
|
||||
for (const k of ENV) saved[k] = process.env[k]
|
||||
delete process.env.AI_PROVIDER
|
||||
delete process.env.AI_MODEL
|
||||
calls.length = 0
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async (url: string, init: RequestInit) => {
|
||||
calls.push({ url: String(url), headers: new Headers(init.headers) })
|
||||
throw new Error("no network in tests")
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
for (const k of ENV) {
|
||||
if (saved[k] === undefined) delete process.env[k]
|
||||
else process.env[k] = saved[k]
|
||||
}
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
describe("EdgeOne as a server model", () => {
|
||||
it("calls the site's Edge AI function with the cookies", async () => {
|
||||
// An admin panel or ai-models.json provider: the client sends the
|
||||
// provider name's slug, not "edgeone"
|
||||
process.env.AI_MODELS_CONFIG = JSON.stringify({
|
||||
providers: [
|
||||
{
|
||||
name: "Edge Pages",
|
||||
provider: "edgeone",
|
||||
models: ["@tx/deepseek-ai/deepseek-v3-0324"],
|
||||
},
|
||||
],
|
||||
})
|
||||
const res = await chat(
|
||||
new Request("http://localhost/api/chat", {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
"x-ai-provider": "edge-pages",
|
||||
"x-selected-model-id":
|
||||
"server:edge-pages:@tx/deepseek-ai/deepseek-v3-0324",
|
||||
cookie: "eo_token=t; eo_time=1",
|
||||
},
|
||||
body: JSON.stringify({
|
||||
messages: [
|
||||
{
|
||||
id: "u1",
|
||||
role: "user",
|
||||
parts: [{ type: "text", text: "Draw two boxes" }],
|
||||
},
|
||||
],
|
||||
xml: "",
|
||||
}),
|
||||
}),
|
||||
)
|
||||
await res.text()
|
||||
expect(calls[0]?.url).toBe(
|
||||
"http://localhost/api/edgeai/chat/completions",
|
||||
)
|
||||
expect(calls[0]?.headers.get("cookie")).toBe("eo_token=t; eo_time=1")
|
||||
})
|
||||
})
|
||||
@@ -18,6 +18,13 @@ vi.mock("@/lib/dynamo-quota-manager", () => ({
|
||||
recordTokenUsage: async () => {},
|
||||
}))
|
||||
|
||||
// No DNS in tests: only loopback addresses are private
|
||||
vi.mock("@/lib/ssrf-protection", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/lib/ssrf-protection")>()),
|
||||
isPrivateUrl: async (url: string) =>
|
||||
/^https?:\/\/(127\.0\.0\.1|localhost)\b/.test(url),
|
||||
}))
|
||||
|
||||
import { POST as chat } from "@/app/api/chat/route"
|
||||
|
||||
const ENV = [
|
||||
@@ -113,6 +120,26 @@ describe("chat quota", () => {
|
||||
expect(quota.checks).toBe(2)
|
||||
})
|
||||
|
||||
it("counts EdgeOne with a key header it never reads", async () => {
|
||||
const res = await send({
|
||||
"x-ai-provider": "edgeone",
|
||||
"x-ai-api-key": "ignored",
|
||||
"x-ai-model": "@tx/deepseek-ai/deepseek-v3-0324",
|
||||
})
|
||||
expect(res.status).toBe(429)
|
||||
expect(quota.checks).toBe(1)
|
||||
})
|
||||
|
||||
it("counts Ollama at a private address, the server's network", async () => {
|
||||
const res = await send({
|
||||
"x-ai-provider": "ollama",
|
||||
"x-ai-base-url": "http://127.0.0.1:11434/api",
|
||||
"x-ai-model": "llama3.2",
|
||||
})
|
||||
expect(res.status).toBe(429)
|
||||
expect(quota.checks).toBe(1)
|
||||
})
|
||||
|
||||
it("does not count Ollama on the user's own server", async () => {
|
||||
const res = await send({
|
||||
"x-ai-provider": "ollama",
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
// @vitest-environment node
|
||||
import { existsSync, mkdtempSync, readdirSync, writeFileSync } from "node:fs"
|
||||
import {
|
||||
existsSync,
|
||||
mkdtempSync,
|
||||
readdirSync,
|
||||
rmSync,
|
||||
writeFileSync,
|
||||
} from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest"
|
||||
@@ -54,6 +60,16 @@ describe("config presets file", () => {
|
||||
expect(loadPresets().presets.map((p) => p.name)).toEqual(["Mine"])
|
||||
})
|
||||
|
||||
it("saves again once a file it could not read is gone", () => {
|
||||
createPreset({ name: "Mine", config: { AI_PROVIDER: "openai" } })
|
||||
readFails.next = true
|
||||
loadPresets()
|
||||
// The user removes the file to start over
|
||||
rmSync(presetsFile())
|
||||
createPreset({ name: "New", config: { AI_PROVIDER: "openai" } })
|
||||
expect(loadPresets().presets.map((p) => p.name)).toEqual(["New"])
|
||||
})
|
||||
|
||||
it("moves a file that is not JSON aside", () => {
|
||||
writeFileSync(presetsFile(), "{not json")
|
||||
expect(loadPresets().presets).toEqual([])
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
// @vitest-environment node
|
||||
import { mkdtempSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { afterEach, describe, expect, it, vi } from "vitest"
|
||||
|
||||
const dir = vi.hoisted(() => ({ path: "" }))
|
||||
vi.mock("electron", () => ({
|
||||
app: {
|
||||
getPath: (name: string) =>
|
||||
name === "exe" ? `${dir.path}/app/exe` : dir.path,
|
||||
getAppPath: () => `${dir.path}/app`,
|
||||
},
|
||||
}))
|
||||
|
||||
import { loadEnvFile } from "@/electron/main/env-loader"
|
||||
|
||||
const KEYS = ["T_JSON", "T_COMMENT", "T_PLAIN", "T_DOUBLE"]
|
||||
afterEach(() => {
|
||||
for (const k of KEYS) delete process.env[k]
|
||||
})
|
||||
|
||||
describe("loadEnvFile", () => {
|
||||
it("reads quoted values like dotenv", () => {
|
||||
dir.path = mkdtempSync(join(tmpdir(), "env-loader-"))
|
||||
writeFileSync(
|
||||
join(dir.path, ".env"),
|
||||
[
|
||||
// An apostrophe inside a single-quoted JSON value
|
||||
`T_JSON='{"name":"Team's models"}'`,
|
||||
`T_COMMENT="value" # a comment`,
|
||||
"T_PLAIN=plain # a comment",
|
||||
`T_DOUBLE="say "hi""`,
|
||||
].join("\n"),
|
||||
)
|
||||
loadEnvFile()
|
||||
expect(process.env.T_JSON).toBe(`{"name":"Team's models"}`)
|
||||
expect(process.env.T_COMMENT).toBe("value")
|
||||
expect(process.env.T_PLAIN).toBe("plain")
|
||||
expect(process.env.T_DOUBLE).toBe(`say "hi"`)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,41 @@
|
||||
// @vitest-environment node
|
||||
import { afterEach, describe, expect, it, vi } from "vitest"
|
||||
import { POST as parseUrl } from "@/app/api/parse-url/route"
|
||||
|
||||
// Treat every URL as public so no test hits DNS
|
||||
vi.mock("@/lib/ssrf-protection", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/lib/ssrf-protection")>()),
|
||||
isPrivateUrl: async () => false,
|
||||
}))
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
describe("POST /api/parse-url", () => {
|
||||
it("stops the download of a page announced as too large", async () => {
|
||||
let signal: AbortSignal | undefined
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async (_url: string, init: RequestInit) => {
|
||||
signal = init.signal ?? undefined
|
||||
// A body that never ends unless the request is aborted
|
||||
return new Response(new ReadableStream(), {
|
||||
headers: {
|
||||
"content-type": "text/html",
|
||||
"content-length": String(50 * 1024 * 1024),
|
||||
},
|
||||
})
|
||||
}),
|
||||
)
|
||||
const res = await parseUrl(
|
||||
new Request("http://localhost/api/parse-url", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ url: "https://example.com/huge" }),
|
||||
}),
|
||||
)
|
||||
expect(res.status).toBe(413)
|
||||
expect(signal?.aborted).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,5 @@
|
||||
// @vitest-environment node
|
||||
import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"
|
||||
import { mkdirSync, mkdtempSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest"
|
||||
@@ -35,18 +35,19 @@ vi.mock("node:net", () => ({
|
||||
import {
|
||||
findAvailablePort,
|
||||
resetAllocatedPort,
|
||||
saveServerPort,
|
||||
} from "@/electron/main/port-manager"
|
||||
|
||||
const portFile = () => join(userData.dir, "server-port.json")
|
||||
const savedPort = () => JSON.parse(readFileSync(portFile(), "utf-8")).port
|
||||
|
||||
/** One launch: pick a port, start on it, remember it if it should be */
|
||||
async function launch() {
|
||||
const port = await findAvailablePort(false)
|
||||
saveServerPort(port)
|
||||
return port
|
||||
}
|
||||
/** Chats saved under http://127.0.0.1:<port>, as Electron stores them */
|
||||
const storeData = (port: number) =>
|
||||
mkdirSync(
|
||||
join(
|
||||
userData.dir,
|
||||
"IndexedDB",
|
||||
`http_127.0.0.1_${port}.indexeddb.leveldb`,
|
||||
),
|
||||
{ recursive: true },
|
||||
)
|
||||
const launch = () => findAvailablePort(false)
|
||||
|
||||
beforeEach(() => {
|
||||
userData.dir = mkdtempSync(join(tmpdir(), "port-manager-"))
|
||||
@@ -54,44 +55,45 @@ beforeEach(() => {
|
||||
resetAllocatedPort()
|
||||
})
|
||||
|
||||
describe("saveServerPort", () => {
|
||||
it("remembers the legacy port of the first launch", async () => {
|
||||
describe("findAvailablePort", () => {
|
||||
it("uses the legacy port first, as main does", async () => {
|
||||
expect(await launch()).toBe(61337)
|
||||
storeData(61337)
|
||||
storeData(13370)
|
||||
expect(await launch()).toBe(61337)
|
||||
expect(savedPort()).toBe(61337)
|
||||
})
|
||||
|
||||
it("remembers 13370 when the system reserves the legacy port", async () => {
|
||||
// Windows excludes port ranges for Hyper-V, which can change on
|
||||
// each boot
|
||||
busy.ports[61337] = "EACCES"
|
||||
it("uses 13370 when only it has the user's chats", async () => {
|
||||
// Windows reserved 61337 when they started using the app
|
||||
storeData(13370)
|
||||
expect(await launch()).toBe(13370)
|
||||
expect(savedPort()).toBe(13370)
|
||||
})
|
||||
|
||||
it("does not remember a port used while the legacy port was in use", async () => {
|
||||
// For example the previous version still quitting after an update:
|
||||
// the user's data is under 61337, so the next launch tries it again
|
||||
it("goes back to the port with the chats once it is free", async () => {
|
||||
// The previous version still quitting after an update
|
||||
storeData(61337)
|
||||
busy.ports[61337] = "EADDRINUSE"
|
||||
expect(await launch()).toBe(13370)
|
||||
expect(existsSync(portFile())).toBe(false)
|
||||
|
||||
storeData(13370)
|
||||
busy.ports = {}
|
||||
expect(await launch()).toBe(61337)
|
||||
expect(savedPort()).toBe(61337)
|
||||
})
|
||||
|
||||
it("does not remember a fallback port", async () => {
|
||||
it("does not hide the chats for good after one reserved launch", async () => {
|
||||
// Windows reserves port ranges per boot
|
||||
storeData(61337)
|
||||
busy.ports[61337] = "EACCES"
|
||||
busy.ports[13370] = "EADDRINUSE"
|
||||
expect(await launch()).toBe(13371)
|
||||
expect(existsSync(portFile())).toBe(false)
|
||||
expect(await launch()).toBe(13370)
|
||||
storeData(13370)
|
||||
busy.ports = {}
|
||||
expect(await launch()).toBe(61337)
|
||||
})
|
||||
|
||||
it("keeps the remembered port when a launch had to use another", () => {
|
||||
// The app's data lives under the remembered port's origin; going
|
||||
// back to it once it is free brings the chats and settings back
|
||||
writeFileSync(portFile(), JSON.stringify({ port: 61337 }))
|
||||
saveServerPort(13371)
|
||||
expect(savedPort()).toBe(61337)
|
||||
it("falls back to the next ports", async () => {
|
||||
storeData(13370)
|
||||
busy.ports[13370] = "EADDRINUSE"
|
||||
expect(await launch()).toBe(61337)
|
||||
busy.ports[61337] = "EACCES"
|
||||
expect(await launch()).toBe(13371)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user