diff --git a/docs/BUG_HISTORY.md b/docs/BUG_HISTORY.md index 376ede77..4f4bb82d 100644 --- a/docs/BUG_HISTORY.md +++ b/docs/BUG_HISTORY.md @@ -2489,3 +2489,19 @@ - 相关记录：BUG-145 - 复发自：无（staging bootstrap deadlock 的独立 health contract 根因） - 修复版本：待 follow-up commit / gate + +## BUG-146 | staging 后台写请求把 Caddy 上游协议误当公开来源 + +- 状态：resolved（local candidate，未 push / deploy） +- 首次发现：2026-08-07 +- 最近更新：2026-08-07 +- 影响面：所有经 `requireAdminMutation` 的后台写接口、`admin.staging.jyotisha.chat` 模型管理写操作；普通 staging 用户域名、其他后台功能的通用 `ReasonActionModal` 与 production 未改动。 +- 用户现象：管理员在独立后台域名提交 `POST /api/admin/models` 时，浏览器 `Origin` 为 HTTPS 公开后台域名，但 Caddy 转发后的 Route Handler 请求 URL 使用上游 HTTP 协议，旧校验因此返回 403 `请求来源不可信`。 +- 触发条件：请求经 staging Caddy `reverse_proxy web:3000` 进入 Next.js，公开 origin 与上游 `request.url` 协议不同；旧 `isSameOriginAdminMutation` 只比较这两个 origin，未核对 Caddy 提供的公开 Host/Proto 头。 +- 根因：共享后台 mutation guard 把应用上游 URL 当作浏览器公开来源真值，没有结合既有 `ADMIN_USER_ORIGIN`、原始 `Host` 与 Caddy 的 `X-Forwarded-Host` / `X-Forwarded-Proto`；因此合法后台请求被拒绝，同时也不能安全地仅信任任意 forwarded host。 +- 修复：`requireAdminMutation` 统一调用可测试的共享来源策略；配置 `ADMIN_USER_ORIGIN` 时要求浏览器 Origin 精确匹配、`Host` 与规范化后的 forwarded host 一致、公开 host/proto 精确匹配后台 origin，缺失、歧义、畸形或冲突的 forwarded 值全部 fail closed；未配置后台 origin 的既有直连环境继续使用严格 same-origin fallback。复用 identity host 规范化 helper，未新增同义 env，staging Caddy 继续在用户域名对 `/admin*` 与 `/api/admin/*` 返回 404。模型管理同时删除 saveProvider/saveDraft/publish/rollback 的客户端“操作原因”字段与交互，服务端分别注入固定中文审计说明后继续传给原 DB procedure 的非空 reason 参数；通用 `ReasonActionModal` 未改动。 +- 验证：`npx tsx --test tests/admin-http-origin.test.ts tests/admin-model-management-ui-contract.test.ts tests/identity-host-routing.test.ts tests/admin-reauth.test.ts tests/health-deployment.test.ts`（34 passed）；相关 ESLint、`git diff --check` 与最终差异审查见本提交验证记录。 +- 防复发：后台 mutation 来源测试必须同时覆盖合法 admin Origin + 公开 host/proto、错误 Origin、普通 staging host、Host/forwarded host 冲突、逗号多值、畸形 host、缺失 proto 与非法 `ADMIN_USER_ORIGIN`；模型管理合同必须拒绝客户端 reason，并确认四个固定审计说明仍传入现有 procedure。 +- 相关记录：BUG-134、BUG-138、BUG-139 +- 复发自：无 +- 修复版本：本地候选提交（未 push / deploy） diff --git a/frontend/src/app/api/admin/models/route.ts b/frontend/src/app/api/admin/models/route.ts index c9a735d3..20f857ed 100644 --- a/frontend/src/app/api/admin/models/route.ts +++ b/frontend/src/app/api/admin/models/route.ts @@ -28,7 +28,6 @@ const providerSchema = z.object({ baseUrl: z.string().url().startsWith("https://").nullable().optional(), apiKey: z.string().max(4096).optional(), enabled: z.boolean(), - reason: z.string().trim().min(1).max(500), }).strict(); const settingsSchema = z.record(z.string(), z.unknown()).superRefine((value, context) => { if (modelSettingsContainSecrets(value)) { @@ -52,20 +51,26 @@ const draftSchema = z.object({ isDefault: z.boolean(), fallbackModelId: z.string().regex(/^[a-z0-9][a-z0-9._-]{0,63}$/).nullable().optional(), settings: settingsSchema.default({}), - reason: z.string().trim().min(1).max(500), }).strict(); const actionSchema = z.discriminatedUnion("action", [ providerSchema, draftSchema, z.object({ action: z.literal("test"), versionId: z.string().uuid() }).strict(), - z.object({ action: z.literal("publish"), versionId: z.string().uuid(), reason: z.string().trim().min(1).max(500) }).strict(), - z.object({ action: z.literal("rollback"), configId: z.string().uuid(), targetVersion: z.number().int().positive(), reason: z.string().trim().min(1).max(500) }).strict(), + z.object({ action: z.literal("publish"), versionId: z.string().uuid() }).strict(), + z.object({ action: z.literal("rollback"), configId: z.string().uuid(), targetVersion: z.number().int().positive() }).strict(), ]).superRefine((value, context) => { if (value.action === "saveDraft" && value.isDefault && !value.enabled) { context.addIssue({ code: "custom", path: ["isDefault"], message: "默认模型必须启用" }); } }); +const modelMutationAuditReasons = { + saveProvider: "保存模型供应商", + saveDraft: "保存模型草稿", + publish: "发布模型版本", + rollback: "回滚模型版本", +} as const; + type ProviderRow = { id: string; code: string; @@ -178,8 +183,11 @@ export async function POST(request: Request) { : "models.write"; const session = await requireAdminMutation(request, permission); const rid = requestId(request); + const mutation: AdminModelMutation = body.data.action === "test" + ? body.data + : { ...body.data, reason: modelMutationAuditReasons[body.data.action] }; return await handleAdminModelMutation( - body.data as AdminModelMutation, + mutation, { actorUserId: session.user.id, requestId: rid }, { queryRows: (sql, values) => queryAdminRows>(sql, values), diff --git a/frontend/src/components/admin/model-management.tsx b/frontend/src/components/admin/model-management.tsx index 3b51113a..dee61d59 100644 --- a/frontend/src/components/admin/model-management.tsx +++ b/frontend/src/components/admin/model-management.tsx @@ -24,7 +24,6 @@ import { import { useCallback, useEffect, useState } from "react"; import { adminRequestJson, type AdminIdentity } from "@/lib/admin/providers"; -import { ReasonActionModal } from "./reason-action-modal"; import { formatAdminDate } from "./resource-table"; const { Text } = Typography; @@ -76,7 +75,6 @@ type ProviderForm = { type ModelForm = Omit & { versionId?: string; settingsJson: string; - reason: string; }; type ModelsPayload = { data: ModelVersion[]; total: number; providers: Provider[] }; @@ -121,7 +119,6 @@ export default function ModelManagement() { const [discoveredModels, setDiscoveredModels] = useState([]); const [actingId, setActingId] = useState(null); const [versionAction, setVersionAction] = useState(null); - const [pendingProvider, setPendingProvider] = useState | null>(null); const [filters, setFilters] = useState({}); const canWrite = Boolean(identity?.permissions.includes("models.write")); const canTest = Boolean(identity?.permissions.includes("models.test")); @@ -187,7 +184,6 @@ export default function ModelManagement() { isDefault: model.isDefault, fallbackModelId: model.fallbackModelId, settingsJson: JSON.stringify(model.settings, null, 2), - reason: "", } : { modelId: "", providerId: providerId ?? providers[0]?.id, @@ -203,37 +199,32 @@ export default function ModelManagement() { isDefault: false, fallbackModelId: null, settingsJson: "{}", - reason: "", }); setModelOpen(true); } - function prepareProviderSave(values: ProviderForm) { + async function saveProvider(values: ProviderForm) { const apiKey = values.apiKey?.trim(); - setPendingProvider({ - action: "saveProvider", - id: editingProvider?.id ?? null, - name: values.name.trim(), - providerType: values.providerType, - baseUrl: values.providerType === "openai-compatible" ? values.baseUrl?.trim() : null, - ...(apiKey ? { apiKey } : {}), - enabled: values.enabled, - }); - } - - async function saveProvider(reason: string) { - if (!pendingProvider) return; setSaving(true); try { await adminRequestJson("/api/admin/models", { method: "POST", - body: JSON.stringify({ ...pendingProvider, reason }), + body: JSON.stringify({ + action: "saveProvider", + id: editingProvider?.id ?? null, + name: values.name.trim(), + providerType: values.providerType, + baseUrl: values.providerType === "openai-compatible" ? values.baseUrl?.trim() : null, + ...(apiKey ? { apiKey } : {}), + enabled: values.enabled, + }), }); message.success("供应商配置已保存"); - setPendingProvider(null); setProviderOpen(false); providerForm.resetFields(); await load(); + } catch (error) { + message.error(error instanceof Error ? error.message : "保存失败"); } finally { setSaving(false); } @@ -296,7 +287,6 @@ export default function ModelManagement() { isDefault: values.isDefault, fallbackModelId: values.fallbackModelId?.trim() || null, settings, - reason: values.reason.trim(), }), }); message.success("模型草稿已保存"); @@ -320,14 +310,18 @@ export default function ModelManagement() { } } - async function submitVersionAction(reason: string) { + async function submitVersionAction() { if (!versionAction) return; const { action, model } = versionAction; - await act(action === "publish" - ? { action, versionId: model.id, reason } - : { action, configId: model.configId, targetVersion: model.version, reason }, - action === "publish" ? "模型已发布" : "模型已回滚", model.id); - setVersionAction(null); + try { + await act(action === "publish" + ? { action, versionId: model.id } + : { action, configId: model.configId, targetVersion: model.version }, + action === "publish" ? "模型已发布" : "模型已回滚", model.id); + setVersionAction(null); + } catch (error) { + message.error(error instanceof Error ? error.message : action === "publish" ? "发布失败" : "回滚失败"); + } } async function testVersion(item: ModelVersion) { @@ -411,8 +405,8 @@ export default function ModelManagement() { - providerForm.submit()} onCancel={() => { setProviderOpen(false); providerForm.resetFields(); }} destroyOnHidden> - form={providerForm} layout="vertical" onFinish={prepareProviderSave}> + providerForm.submit()} onCancel={() => { setProviderOpen(false); providerForm.resetFields(); }} destroyOnHidden> + form={providerForm} layout="vertical" onFinish={saveProvider}> : null} @@ -454,25 +448,23 @@ export default function ModelManagement() { ({ validator(_, value) { return value && !getFieldValue("enabled") ? Promise.reject(new Error("默认模型必须启用")) : Promise.resolve(); } })]}> - - setPendingProvider(null)} - onSubmit={saveProvider} - /> - setVersionAction(null)} - onSubmit={submitVersionAction} - /> + onOk={() => void submitVersionAction()} + destroyOnHidden + > + + {versionAction?.action === "rollback" + ? "确认将此历史版本恢复为新的已发布版本？" + : "确认发布此模型版本？"} + + ; } diff --git a/frontend/src/lib/admin/auth-policy.ts b/frontend/src/lib/admin/auth-policy.ts index 99edfdfc..32cf0710 100644 --- a/frontend/src/lib/admin/auth-policy.ts +++ b/frontend/src/lib/admin/auth-policy.ts @@ -1,6 +1,7 @@ import { createHmac, timingSafeEqual } from "node:crypto"; import type { IdentityUser } from "@/modules/identity/contracts"; +import { normalizeIdentityHost } from "@/modules/identity/host"; export type AdminRole = | "owner" @@ -95,6 +96,67 @@ export function isSameOriginAdminMutation( } } +function singleForwardedValue(value: string | null): string | null { + const normalized = value?.trim(); + return normalized && !normalized.includes(",") ? normalized : null; +} + +function configuredAdminOrigin(value: string): URL | null { + try { + const url = new URL(value); + const isLocalhost = url.hostname === "localhost" || url.hostname.endsWith(".localhost"); + if ( + (url.protocol !== "https:" + && !(isLocalhost && url.protocol === "http:")) + || url.username + || url.password + || url.pathname !== "/" + || url.search + || url.hash + ) { + return null; + } + return url; + } catch { + return null; + } +} + +export function isTrustedAdminMutationRequest( + request: Request, + adminOriginValue?: string, +): boolean { + const origin = request.headers.get("origin"); + const configuredValue = adminOriginValue?.trim(); + if (!configuredValue) { + return isSameOriginAdminMutation(origin, request.url); + } + + const adminOrigin = configuredAdminOrigin(configuredValue); + if (!adminOrigin || origin !== adminOrigin.origin) return false; + + const hasForwardedHost = request.headers.has("x-forwarded-host"); + const hasForwardedProto = request.headers.has("x-forwarded-proto"); + if (!hasForwardedHost && !hasForwardedProto) { + return isSameOriginAdminMutation(origin, request.url); + } + + const forwardedHostValue = request.headers.get("x-forwarded-host"); + const forwardedProtoValue = request.headers.get("x-forwarded-proto"); + + const host = normalizeIdentityHost(request.headers.get("host")); + const forwardedHost = normalizeIdentityHost(forwardedHostValue); + const forwardedProto = singleForwardedValue(forwardedProtoValue)?.toLowerCase(); + return Boolean( + host + && forwardedHost + && forwardedProto + && host === forwardedHost + && forwardedHost === adminOrigin.host.toLowerCase() + && `${forwardedProto}:` === adminOrigin.protocol, + ); +} + export function resolveAdminMfaStatus( required: boolean, enrolled: boolean, diff --git a/frontend/src/lib/admin/http.ts b/frontend/src/lib/admin/http.ts index f591e392..a2c06b33 100644 --- a/frontend/src/lib/admin/http.ts +++ b/frontend/src/lib/admin/http.ts @@ -10,7 +10,7 @@ import { import { ADMIN_MFA_PROOF_COOKIE, HIGH_RISK_ADMIN_PROOF_COOKIE, - isSameOriginAdminMutation, + isTrustedAdminMutationRequest, resolveAdminMfaStatus, verifyAdminMfaProof, verifyHighRiskAdminProof, @@ -43,7 +43,7 @@ export function requestId(request: Request): string { } export async function requireAdminMutation(request: Request, permission: AdminPermission) { - if (!isSameOriginAdminMutation(request.headers.get("origin"), request.url)) { + if (!isTrustedAdminMutationRequest(request, process.env.ADMIN_USER_ORIGIN)) { throw new AdminAuthorizationError("请求来源不可信", 403); } return requirePermission(permission, request.headers); diff --git a/frontend/src/modules/identity/host.ts b/frontend/src/modules/identity/host.ts index d0a50968..810a59f8 100644 --- a/frontend/src/modules/identity/host.ts +++ b/frontend/src/modules/identity/host.ts @@ -9,7 +9,7 @@ export interface IdentityAuthHandlers { POST: IdentityRequestHandler; } -function normalizeHost(value: string | null): string | null { +export function normalizeIdentityHost(value: string | null): string | null { if (!value || value !== value.trim() || /[\s,@/\\]/.test(value)) return null; try { @@ -33,7 +33,7 @@ export function resolveIdentitySurface( hostHeader: string | null, config: SelfHostedIdentityConfig, ): "user" | "admin" | null { - const host = normalizeHost(hostHeader); + const host = normalizeIdentityHost(hostHeader); if (!host) return null; if (host === new URL(config.userOrigin).host.toLowerCase()) return "user"; diff --git a/frontend/tests/admin-contracts.test.ts b/frontend/tests/admin-contracts.test.ts index 6c4a0d02..b7ba9967 100644 --- a/frontend/tests/admin-contracts.test.ts +++ b/frontend/tests/admin-contracts.test.ts @@ -225,7 +225,7 @@ test("Refine dependencies and same-origin admin data provider are present", () = }); test("administrator writes require scoped email OTP reauthentication without mandatory MFA", () => { - assert.match(adminHttp, /isSameOriginAdminMutation/); + assert.match(adminHttp, /isTrustedAdminMutationRequest\(request, process\.env\.ADMIN_USER_ORIGIN\)/); assert.match(administratorsRoute, /requireHighRiskAdminMutation\(request, "admin\.users\.manage_roles"\)/); assert.match(adminHttp, /requireAdminMutation\(request, permission\)[\s\S]*verifyHighRiskAdminProof/); assert.doesNotMatch(adminHttp, /requireAdminMfaIfRequired/); diff --git a/frontend/tests/admin-http-origin.test.ts b/frontend/tests/admin-http-origin.test.ts new file mode 100644 index 00000000..cb56edfe --- /dev/null +++ b/frontend/tests/admin-http-origin.test.ts @@ -0,0 +1,123 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { isTrustedAdminMutationRequest } from "../src/lib/admin/auth-policy.ts"; + +const adminOrigin = "https://admin.staging.jyotisha.chat"; +const userOrigin = "https://staging.jyotisha.chat"; + +function request( + url: string, + headers: HeadersInit, +): Request { + return new Request(url, { method: "POST", headers }); +} + +test("trusted proxy admin origin accepts the configured host and protocol", () => { + const proxied = request("http://admin.staging.jyotisha.chat/api/admin/models", { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat", + "x-forwarded-proto": "https", + }); + + assert.equal(isTrustedAdminMutationRequest(proxied, adminOrigin), true); +}); + +test("configured admin origin rejects wrong browser origins", () => { + const proxied = request("http://admin.staging.jyotisha.chat/api/admin/models", { + origin: "https://evil.example", + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat", + "x-forwarded-proto": "https", + }); + + assert.equal(isTrustedAdminMutationRequest(proxied, adminOrigin), false); +}); + +test("ordinary staging host cannot call admin mutations", () => { + const userHost = request(`${userOrigin}/api/admin/models`, { + origin: userOrigin, + host: "staging.jyotisha.chat", + "x-forwarded-host": "staging.jyotisha.chat", + "x-forwarded-proto": "https", + }); + const forgedForwardedHost = request(`${userOrigin}/api/admin/models`, { + origin: adminOrigin, + host: "staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat", + "x-forwarded-proto": "https", + }); + + assert.equal(isTrustedAdminMutationRequest(userHost, adminOrigin), false); + assert.equal(isTrustedAdminMutationRequest(forgedForwardedHost, adminOrigin), false); +}); + +test("malformed or ambiguous forwarded origins fail closed", () => { + const cases: HeadersInit[] = [ + { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat, evil.example", + "x-forwarded-proto": "https", + }, + { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat", + "x-forwarded-proto": "https, http", + }, + { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat/path", + "x-forwarded-proto": "https", + }, + { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "", + "x-forwarded-proto": "https", + }, + { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat", + }, + ]; + for (const headers of cases) { + assert.equal( + isTrustedAdminMutationRequest( + request("http://admin.staging.jyotisha.chat/api/admin/models", headers), + adminOrigin, + ), + false, + ); + } +}); + +test("direct same-origin requests retain the legacy fallback when no admin origin is configured", () => { + const direct = request("https://admin.example/api/admin/models", { + origin: "https://admin.example", + }); + + assert.equal(isTrustedAdminMutationRequest(direct), true); +}); + +test("invalid configured admin origins fail closed", () => { + const proxied = request("http://admin.staging.jyotisha.chat/api/admin/models", { + origin: adminOrigin, + host: "admin.staging.jyotisha.chat", + "x-forwarded-host": "admin.staging.jyotisha.chat", + "x-forwarded-proto": "https", + }); + + for (const configured of [ + "not-an-origin", + `${adminOrigin}/path`, + "ftp://admin.staging.jyotisha.chat", + "http://admin.staging.jyotisha.chat", + ]) { + assert.equal(isTrustedAdminMutationRequest(proxied, configured), false); + } +}); diff --git a/frontend/tests/admin-model-management-ui-contract.test.ts b/frontend/tests/admin-model-management-ui-contract.test.ts index 6f7fa068..d03d2c73 100644 --- a/frontend/tests/admin-model-management-ui-contract.test.ts +++ b/frontend/tests/admin-model-management-ui-contract.test.ts @@ -7,6 +7,11 @@ const component = readFileSync( "utf8", ); const globals = readFileSync(new URL("../src/app/globals.css", import.meta.url), "utf8"); +const route = readFileSync(new URL("../src/app/api/admin/models/route.ts", import.meta.url), "utf8"); +const mutationHandler = readFileSync( + new URL("../src/lib/admin/model-mutation-handler.ts", import.meta.url), + "utf8", +); test("provider form keeps codes server-owned and API keys write-only", () => { assert.doesNotMatch(component, /name="code"|code:\s*values\.code/); @@ -31,11 +36,22 @@ test("model discovery stays searchable with a manual provider model fallback", ( assert.match(component, /!values\.label/); }); -test("model mutations keep audit reasons without requesting email verification", () => { - assert.doesNotMatch(component, /reauthPermission=[^\n]*models\./); - assert.doesNotMatch(component, /验证并(?:保存|获取)/); - assert.match(component, /title="保存模型供应商"[\s\S]*okText="保存"[\s\S]*onSubmit=\{saveProvider\}/); - assert.match(component, /title=\{versionAction\?\.action[\s\S]*onSubmit=\{submitVersionAction\}/); +test("model mutations omit client reasons while the server keeps fixed audit reasons", () => { + assert.doesNotMatch(component, /ReasonActionModal|name="reason"|reason:\s*(?:values\.reason|reason)/); + assert.doesNotMatch(component, /reauthPermission=[^\n]*models\.|验证并(?:保存|获取)/); + assert.match(component, /open=\{providerOpen\} okText="保存"[\s\S]*onFinish=\{saveProvider\}/); + assert.doesNotMatch(component, /继续验证/); + assert.match(component, / void submitVersionAction\(\)\}/); + + assert.doesNotMatch(route, /reason:\s*z\.string/); + for (const reason of ["保存模型供应商", "保存模型草稿", "发布模型版本", "回滚模型版本"]) { + assert.match(route, new RegExp(reason)); + } + assert.match(route, /reason:\s*modelMutationAuditReasons\[body\.data\.action\]/); + assert.match(mutationHandler, /admin_save_model_provider[\s\S]*a\.reason/); + assert.match(mutationHandler, /admin_save_model_draft[\s\S]*a\.reason/); + assert.match(mutationHandler, /admin_publish_model[\s\S]*a\.reason/); + assert.match(mutationHandler, /admin_rollback_model[\s\S]*a\.reason/); }); test("provider rows expose a direct add-model entry with the provider preselected", () => {