mirror of
https://github.com/DayuanJiang/next-ai-draw-io.git
synced 2026-10-13 13:09:52 +08:00
fix(server): use the keys the user sent, and more review fixes
Found by the second PR review: - With AWS_BEARER_TOKEN_BEDROCK set on the server, a request with the user's AWS keys ran on the server's token: the Bedrock SDK prefers it. Checked with Bedrock: invalid user keys used to get an answer. - An OpenAI key with the official URL filled in (the settings form does that) went to the Responses API. Back to main's rule: a configured base URL uses Chat Completions. - A user's Ollama key went to the server's OLLAMA_BASE_URL, for chat and for the model list. Like every other provider, it goes to the user's base URL or Ollama Cloud. - The server's keyless Ollama and EdgeOne were not counted in the quota. - AI_MODEL models on the server's keys ran on any provider with a server key, not only on AI_PROVIDER. - A user's Azure key without a base URL used the server's resource name. - The admin panel's Test button failed whenever access codes were set. - DeepSeek's errors in the stream (plain text) were shown as they were, without a hint and also on the server's keys. Bedrock's throttling in the stream was not recognised as a rate limit. - The EdgeOne function accepted text/plain; x=application/json, which other sites can send without a CORS preflight. - Desktop app: a launch that found the old port taken for a moment (the previous version still quitting after an update) remembered the new port for good. The new port is kept only when Windows reserves the old one. A failed read of the presets file moved it aside as corrupt, and a save could then replace the presets. Switching presets on the same port now reloads the page. The dev launcher no longer misses a preset change made before or during a restart.
This commit is contained in:
@@ -36,6 +36,11 @@ vi.mock("@aws-sdk/credential-providers", () => ({
|
||||
fromNodeProviderChain: vi.fn(() => "node-chain"),
|
||||
}))
|
||||
|
||||
vi.mock("ollama-ai-provider-v2", () => {
|
||||
const mockProviderFn = vi.fn(() => ({ modelId: "test-model" }))
|
||||
return { createOllama: vi.fn(() => mockProviderFn) }
|
||||
})
|
||||
|
||||
vi.mock("@openrouter/ai-sdk-provider", () => {
|
||||
const mockProviderFn = vi.fn(() => ({ modelId: "test-model" }))
|
||||
return { createOpenRouter: vi.fn(() => mockProviderFn) }
|
||||
@@ -60,6 +65,8 @@ const ENV_KEYS = [
|
||||
"NEXT_AI_DRAWIO_DESKTOP",
|
||||
"SGLANG_API_KEY",
|
||||
"SGLANG_BASE_URL",
|
||||
"AZURE_RESOURCE_NAME",
|
||||
"OLLAMA_BASE_URL",
|
||||
]
|
||||
const savedEnv: Record<string, string | undefined> = {}
|
||||
|
||||
@@ -173,6 +180,8 @@ describe("Bedrock admin panel credentials", () => {
|
||||
region: "ap-northeast-1",
|
||||
accessKeyId: "client-id",
|
||||
secretAccessKey: "client-secret",
|
||||
// The SDK would otherwise use the server's AWS_BEARER_TOKEN_BEDROCK
|
||||
apiKey: "",
|
||||
})
|
||||
})
|
||||
|
||||
@@ -312,6 +321,42 @@ describe("whose keys a request uses", () => {
|
||||
expect(provider.chat).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("uses Chat Completions for any configured base URL", () => {
|
||||
// The settings form fills in the official URL for a new provider
|
||||
getAIModel({
|
||||
provider: "openai",
|
||||
apiKey: "user-key",
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
modelId: "gpt-5.5",
|
||||
})
|
||||
const provider = vi.mocked(createOpenAI).mock.results.at(-1)?.value
|
||||
expect(provider.chat).toHaveBeenCalledWith("gpt-5.5")
|
||||
})
|
||||
|
||||
it("sends a user's Ollama key to Ollama Cloud, not the server's Ollama", async () => {
|
||||
process.env.OLLAMA_BASE_URL = "http://ollama.internal:11434/api"
|
||||
const { createOllama } = await import("ollama-ai-provider-v2")
|
||||
getAIModel({ provider: "ollama", apiKey: "user-key", modelId: "m" })
|
||||
expect(createOllama).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({ baseURL: "https://ollama.com/api" }),
|
||||
)
|
||||
// Without a key: the server's Ollama
|
||||
getAIModel({ provider: "ollama", modelId: "m" })
|
||||
expect(createOllama).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: "http://ollama.internal:11434/api",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
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"
|
||||
expect(() =>
|
||||
getAIModel({ provider: "azure", apiKey: "k", modelId: "gpt-4o" }),
|
||||
).toThrow(/base URL/)
|
||||
})
|
||||
|
||||
it("needs a base URL for SGLang instead of using 127.0.0.1", () => {
|
||||
expect(() =>
|
||||
getAIModel({ provider: "sglang", apiKey: "k", modelId: "m" }),
|
||||
|
||||
@@ -20,7 +20,15 @@ vi.mock("@/lib/dynamo-quota-manager", () => ({
|
||||
|
||||
import { POST as chat } from "@/app/api/chat/route"
|
||||
|
||||
const ENV = ["AI_PROVIDER", "AI_MODEL", "OPENAI_API_KEY"]
|
||||
const ENV = [
|
||||
"AI_PROVIDER",
|
||||
"AI_MODEL",
|
||||
"OPENAI_API_KEY",
|
||||
"OLLAMA_BASE_URL",
|
||||
"OLLAMA_API_KEY",
|
||||
"AI_GATEWAY_API_KEY",
|
||||
"ALLOW_PRIVATE_URLS",
|
||||
]
|
||||
const saved: Record<string, string | undefined> = {}
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -88,4 +96,44 @@ describe("chat quota", () => {
|
||||
expect(res.status).not.toBe(429)
|
||||
expect(quota.checks).toBe(0)
|
||||
})
|
||||
|
||||
it("counts the server's keyless Ollama and EdgeOne", async () => {
|
||||
process.env.AI_PROVIDER = "ollama"
|
||||
process.env.AI_MODEL = "llama3.2"
|
||||
process.env.OLLAMA_BASE_URL = "http://ollama.internal:11434/api"
|
||||
expect((await send({})).status).toBe(429)
|
||||
expect(
|
||||
(
|
||||
await send({
|
||||
"x-ai-provider": "edgeone",
|
||||
"x-ai-model": "@tx/deepseek-ai/deepseek-v3-0324",
|
||||
})
|
||||
).status,
|
||||
).toBe(429)
|
||||
expect(quota.checks).toBe(2)
|
||||
})
|
||||
|
||||
it("does not count Ollama on the user's own server", async () => {
|
||||
const res = await send({
|
||||
"x-ai-provider": "ollama",
|
||||
"x-ai-base-url": "https://ollama.example.com/api",
|
||||
"x-ai-model": "llama3.2",
|
||||
})
|
||||
expect(res.status).not.toBe(429)
|
||||
expect(quota.checks).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe("server model allowlist", () => {
|
||||
it("runs AI_MODEL only on the server's AI_PROVIDER", async () => {
|
||||
// Another provider's server key must not run it
|
||||
process.env.AI_GATEWAY_API_KEY = "server-gateway-key"
|
||||
const res = await send({
|
||||
"x-ai-provider": "gateway",
|
||||
"x-ai-model": "gpt-5.5",
|
||||
})
|
||||
expect(res.status).toBe(400)
|
||||
expect(await res.text()).toMatch(/not available on this server/)
|
||||
expect(quota.checks).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
// @vitest-environment node
|
||||
import { existsSync, mkdtempSync, readdirSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest"
|
||||
|
||||
const userData = vi.hoisted(() => ({ dir: "" }))
|
||||
vi.mock("electron", () => ({
|
||||
app: { getPath: () => userData.dir },
|
||||
safeStorage: { isEncryptionAvailable: () => false },
|
||||
}))
|
||||
|
||||
// Make the next read of the presets file fail, like a file an antivirus
|
||||
// scanner holds on Windows
|
||||
const readFails = vi.hoisted(() => ({ next: false }))
|
||||
vi.mock("node:fs", async (importOriginal) => {
|
||||
const fs = await importOriginal<typeof import("node:fs")>()
|
||||
return {
|
||||
...fs,
|
||||
readFileSync: ((...args: Parameters<typeof fs.readFileSync>) => {
|
||||
if (readFails.next && String(args[0]).endsWith(".json")) {
|
||||
readFails.next = false
|
||||
throw Object.assign(new Error("EBUSY: resource busy"), {
|
||||
code: "EBUSY",
|
||||
})
|
||||
}
|
||||
return fs.readFileSync(...args)
|
||||
}) as typeof fs.readFileSync,
|
||||
}
|
||||
})
|
||||
|
||||
import { createPreset, loadPresets } from "@/electron/main/config-manager"
|
||||
|
||||
const presetsFile = () => join(userData.dir, "config-presets.json")
|
||||
|
||||
beforeEach(() => {
|
||||
userData.dir = mkdtempSync(join(tmpdir(), "config-manager-"))
|
||||
readFails.next = false
|
||||
})
|
||||
|
||||
describe("config presets file", () => {
|
||||
it("keeps a file it could not read for now", () => {
|
||||
createPreset({ name: "Mine", config: { AI_PROVIDER: "openai" } })
|
||||
|
||||
readFails.next = true
|
||||
expect(loadPresets().presets).toEqual([])
|
||||
expect(existsSync(presetsFile())).toBe(true)
|
||||
|
||||
// A save based on that empty read must not replace the presets
|
||||
readFails.next = true
|
||||
expect(() =>
|
||||
createPreset({ name: "New", config: { AI_PROVIDER: "openai" } }),
|
||||
).toThrow()
|
||||
expect(loadPresets().presets.map((p) => p.name)).toEqual(["Mine"])
|
||||
})
|
||||
|
||||
it("moves a file that is not JSON aside", () => {
|
||||
writeFileSync(presetsFile(), "{not json")
|
||||
expect(loadPresets().presets).toEqual([])
|
||||
expect(existsSync(presetsFile())).toBe(false)
|
||||
expect(
|
||||
readdirSync(userData.dir).some((f) =>
|
||||
f.startsWith("config-presets.json.corrupt-"),
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -26,6 +26,21 @@ describe("EdgeOne chat completions function", () => {
|
||||
env: {},
|
||||
})
|
||||
expect(res.status).toBe(400)
|
||||
// A plain-text type that only mentions JSON needs no CORS preflight
|
||||
const disguised = await onRequest({
|
||||
request: request({
|
||||
"Content-Type": "text/plain; x=application/json",
|
||||
}),
|
||||
env: {},
|
||||
})
|
||||
expect(disguised.status).toBe(400)
|
||||
const withCharset = await onRequest({
|
||||
request: request({
|
||||
"Content-Type": "application/json; charset=utf-8",
|
||||
}),
|
||||
env: {},
|
||||
})
|
||||
expect(withCharset.status).toBe(200)
|
||||
})
|
||||
|
||||
it("checks the access code when ACCESS_CODE_LIST is set", async () => {
|
||||
|
||||
@@ -229,6 +229,27 @@ describe("streamErrorText", () => {
|
||||
message: "bad key",
|
||||
})
|
||||
})
|
||||
|
||||
it("classifies a provider error sent as plain text", () => {
|
||||
// DeepSeek's SDK sends errors in the stream as a string
|
||||
const text = "Insufficient Balance for account 42"
|
||||
expect(JSON.parse(streamErrorText(text))).toEqual({
|
||||
type: "provider",
|
||||
code: "insufficient_quota",
|
||||
message: text,
|
||||
})
|
||||
expect(JSON.parse(streamErrorText(text, true)).message).not.toMatch(
|
||||
/account 42/,
|
||||
)
|
||||
})
|
||||
|
||||
it("classifies Bedrock's throttling sent in the stream", () => {
|
||||
// Bedrock's ThrottlingException as a plain object, not an API error
|
||||
const throttled = {
|
||||
message: "Too many tokens, please wait before trying again.",
|
||||
}
|
||||
expect(JSON.parse(streamErrorText(throttled)).code).toBe("rate_limited")
|
||||
})
|
||||
})
|
||||
|
||||
describe("isToolCallError", () => {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// @vitest-environment node
|
||||
import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"
|
||||
import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest"
|
||||
@@ -9,29 +9,88 @@ vi.mock("electron", () => ({
|
||||
app: { isPackaged: true, getPath: () => userData.dir },
|
||||
}))
|
||||
|
||||
import { saveServerPort } from "@/electron/main/port-manager"
|
||||
// Ports that fail to listen, with the error code
|
||||
const busy = vi.hoisted(() => ({ ports: {} as Record<number, string> }))
|
||||
vi.mock("node:net", () => ({
|
||||
default: {
|
||||
createServer: () => {
|
||||
const handlers: Record<string, (arg?: unknown) => void> = {}
|
||||
const server = {
|
||||
once: (event: string, cb: (arg?: unknown) => void) => {
|
||||
handlers[event] = cb
|
||||
return server
|
||||
},
|
||||
listen: (port: number) => {
|
||||
const code = busy.ports[port]
|
||||
if (code) handlers.error?.({ code })
|
||||
else handlers.listening?.()
|
||||
},
|
||||
close: () => {},
|
||||
}
|
||||
return server
|
||||
},
|
||||
},
|
||||
}))
|
||||
|
||||
const savedPort = () =>
|
||||
JSON.parse(readFileSync(join(userData.dir, "server-port.json"), "utf-8"))
|
||||
.port
|
||||
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
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
userData.dir = mkdtempSync(join(tmpdir(), "port-manager-"))
|
||||
busy.ports = {}
|
||||
resetAllocatedPort()
|
||||
})
|
||||
|
||||
describe("saveServerPort", () => {
|
||||
it("remembers the port of the first launch", () => {
|
||||
saveServerPort(13370)
|
||||
it("remembers the legacy port of the first launch", async () => {
|
||||
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"
|
||||
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
|
||||
busy.ports[61337] = "EADDRINUSE"
|
||||
expect(await launch()).toBe(13370)
|
||||
expect(existsSync(portFile())).toBe(false)
|
||||
|
||||
busy.ports = {}
|
||||
expect(await launch()).toBe(61337)
|
||||
expect(savedPort()).toBe(61337)
|
||||
})
|
||||
|
||||
it("does not remember a fallback port", async () => {
|
||||
busy.ports[61337] = "EACCES"
|
||||
busy.ports[13370] = "EADDRINUSE"
|
||||
expect(await launch()).toBe(13371)
|
||||
expect(existsSync(portFile())).toBe(false)
|
||||
})
|
||||
|
||||
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(
|
||||
join(userData.dir, "server-port.json"),
|
||||
JSON.stringify({ port: 61337 }),
|
||||
)
|
||||
writeFileSync(portFile(), JSON.stringify({ port: 61337 }))
|
||||
saveServerPort(13371)
|
||||
expect(savedPort()).toBe(61337)
|
||||
})
|
||||
|
||||
@@ -118,6 +118,18 @@ describe("listProviderModels", () => {
|
||||
])
|
||||
})
|
||||
|
||||
it("lists Ollama Cloud with a user's key, like chat", async () => {
|
||||
// The user's key must not go to the server's Ollama
|
||||
const { fn, calls } = answer({ models: [{ name: "gpt-oss:120b" }] })
|
||||
process.env.OLLAMA_BASE_URL = "http://ollama.internal:11434"
|
||||
try {
|
||||
await listProviderModels("ollama", { apiKey: "user-key" }, fn)
|
||||
} finally {
|
||||
delete process.env.OLLAMA_BASE_URL
|
||||
}
|
||||
expect(calls[0].url).toBe("https://ollama.com/api/tags")
|
||||
})
|
||||
|
||||
it("does not use SGLang's local address as a default", async () => {
|
||||
const { fn, calls } = answer({ data: [] })
|
||||
await expect(
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
// @vitest-environment node
|
||||
import { streamText } from "ai"
|
||||
import { afterEach, describe, expect, it, vi } from "vitest"
|
||||
import { POST as testModel } from "@/app/api/admin/test-model/route"
|
||||
import { POST as validateModel } from "@/app/api/validate-model/route"
|
||||
import { getAIModel } from "@/lib/ai-providers"
|
||||
|
||||
// No saved admin providers
|
||||
vi.mock("@/lib/admin/settings", () => ({ loadSettings: () => ({}) }))
|
||||
|
||||
// Treat every URL as public so no test hits DNS
|
||||
vi.mock("@/lib/ssrf-protection", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/lib/ssrf-protection")>()),
|
||||
@@ -158,3 +162,37 @@ describe("chat requests to a client base URL", () => {
|
||||
expect(String(error)).toMatch(/Redirects are not allowed/)
|
||||
})
|
||||
})
|
||||
|
||||
describe("the admin panel's Test button", () => {
|
||||
it("works when access codes are set", async () => {
|
||||
// The admin password stands in for the visitor access code
|
||||
process.env.ACCESS_CODE_LIST = "visitor-code"
|
||||
process.env.ADMIN_PASSWORD = "admin-pw"
|
||||
try {
|
||||
streamReply({ role: "assistant", content: "OK" })
|
||||
const res = await testModel(
|
||||
new Request("http://localhost/api/admin/test-model", {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
"x-admin-password": "admin-pw",
|
||||
},
|
||||
body: JSON.stringify({
|
||||
provider: {
|
||||
id: "p1",
|
||||
provider: "glm",
|
||||
apiKey: "key",
|
||||
models: ["glm-5"],
|
||||
},
|
||||
modelId: "glm-5",
|
||||
}),
|
||||
}),
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
expect((await res.json()).valid).toBe(true)
|
||||
} finally {
|
||||
delete process.env.ACCESS_CODE_LIST
|
||||
delete process.env.ADMIN_PASSWORD
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user