fix(review): 处理 PR #4 review 发现(SSRF 自动加载/ref 上限/PWA cookie/编辑 path 参/共享 schema)

cli/src/build-template.ts

@@ -21,7 +21,8 @@ const TEMPLATE_CSP = [
   "script-src 'self' 'unsafe-inline' 'unsafe-eval'",
   "object-src 'none'",
   "base-uri 'self'",
-  "frame-ancestors 'none'",
+  // Clickjacking: frame-ancestors and X-Frame-Options both require HTTP headers,
+  // not <meta>. Static templates must rely on the hosting server's headers config.
 ].join("; ")
 
 function templateCspPlugin() {

package.json

@@ -97,6 +97,7 @@
     "i18next-browser-languagedetector": "^8.2.1",
     "idb": "^8.0.3",
     "input-otp": "^1.4.2",
+    "ipaddr.js": "^2.4.0",
     "libphonenumber-js": "^1.13.3",
     "lucide-react": "^1.17.0",
     "marked": "^18.0.4",

src/components/console/ConsoleFormDialog.tsx

@@ -16,10 +16,13 @@ interface Props {
   resource: ConsoleResource
   mode: "create" | "edit"
   initialData?: Record<string, unknown> | undefined
+  /** Path params (e.g. the id the detail page was loaded with) — fallback when the
+   *  response body doesn't echo the id field. */
+  pathParams?: Record<string, string> | undefined
   onSuccess: () => void
 }
 
-export function ConsoleFormDialog({ resource, mode, initialData, onSuccess }: Props) {
+export function ConsoleFormDialog({ resource, mode, initialData, pathParams, onSuccess }: Props) {
   const { t } = useTranslation()
   const { dispatch, activeLayout } = useConsoleContext()
   const auth = useAuthContext()
@@ -41,10 +44,12 @@ export function ConsoleFormDialog({ resource, mode, initialData, onSuccess }: Pr
   const handleSubmit = async () => {
     if (!operation) return
     const body = JSON.stringify(formData)
-    const params: Record<string, string> = {}
+    // Seed with the path params the page was loaded with; the response's own id
+    // (if present) takes precedence as it's the authoritative record id.
+    const params: Record<string, string> = { ...(pathParams ?? {}) }
 
-    if (mode === "edit" && resource.idParam && initialData) {
-      const id = String(initialData[resource.idParam] ?? initialData["id"] ?? "")
+    if (mode === "edit" && resource.idParam) {
+      const id = String(initialData?.[resource.idParam] ?? initialData?.["id"] ?? pathParams?.[resource.idParam] ?? "")
       if (id) params[resource.idParam] = id
     }
 

src/components/console/templates/DetailCardTemplate.tsx

@@ -115,7 +115,7 @@ export function DetailCardTemplate({ resource, layoutOverride }: TemplateProps)
       </div>
 
       {state.subView === "edit" && data && (
-        <ConsoleFormDialog resource={resource} mode="edit" initialData={data} onSuccess={fetchDetail} />
+        <ConsoleFormDialog resource={resource} mode="edit" initialData={data} pathParams={pathParams} onSuccess={fetchDetail} />
       )}
     </div>
   )

src/hooks/use-settings.ts

@@ -1,6 +1,7 @@
 import { useEffect, useRef } from "react"
 import { useOpenAPIContext } from "@/contexts/OpenAPIContext"
 import { authTypeValue, readLegacySettingsFromLocalStorage } from "@/lib/db"
+import { isPrivateOrLocalHost } from "@/lib/openapi/url-guard"
 import type { AuthType } from "@/lib/openapi/types"
 
 interface AuthSetters {
@@ -62,9 +63,21 @@ export function useSettings(
       if (title) document.title = title
 
       if (specUrl && autoLoad) {
-        setTimeout(() => {
-          autoLoad(specUrl, baseUrl ? { baseUrlOverride: baseUrl } : undefined)
-        }, 0)
+        // A spec URL taken from the query string is attacker-controllable (share
+        // link) and auto-fetched without interaction — refuse internal/loopback
+        // hosts (SSRF). URLs the user typed or previously loaded (legacy) are exempt.
+        const fromQuery = !!paramSpecUrl && specUrl === paramSpecUrl
+        let blockedHost = false
+        if (fromQuery) {
+          try { blockedHost = isPrivateOrLocalHost(new URL(specUrl).hostname) } catch { blockedHost = true }
+        }
+        if (blockedHost) {
+          console.warn(`[apilot] refused to auto-load spec from an internal/loopback host: ${specUrl}`)
+        } else {
+          setTimeout(() => {
+            autoLoad(specUrl, baseUrl ? { baseUrlOverride: baseUrl } : undefined)
+          }, 0)
+        }
       }
     })()
 

src/lib/db.ts

@@ -733,18 +733,19 @@ export async function addHistoryEntry(entry: Omit<HistoryEntry, "id">): Promise<
   try {
     const db = await getDB()
     const originalHeaders = entry.response.requestHeaders
-    const sanitized = { ...truncateBody(entry.response) }
+    // Redact BEFORE truncating — truncateBody turns valid JSON into broken text,
+    // which makes redactBody's JSON.parse fail and skip credential masking.
+    const sanitized = { ...entry.response }
     sanitized.requestHeaders = stripSensitiveHeaders(originalHeaders)
-    // The request body is stored twice (top-level + inside response); redact both,
-    // plus request params and the response body (may contain access/refresh tokens).
     sanitized.requestBody = redactBody(sanitized.requestBody)
     sanitized.body = redactBody(sanitized.body) ?? sanitized.body
     sanitized.curlCommand = redactCurlCommand(sanitized.curlCommand, originalHeaders, entry.response.requestBody)
+    const truncated = truncateBody(sanitized)
     await db.add("history", {
       ...entry,
       requestBody: redactBody(entry.requestBody),
       requestParams: redactParams(entry.requestParams),
-      response: sanitized,
+      response: truncated,
     })
   } catch (err) {
     if ((err as DOMException)?.name === "QuotaExceededError") {
@@ -872,9 +873,11 @@ export async function getEnvironmentCredential(envId: string): Promise<Environme
 }
 
 export async function getEnvironmentRuntimes(specId: string): Promise<EnvironmentRuntime[]> {
-  const db = await getDB()
-  const profiles = await db.getAllFromIndex("environments", "specId", specId) as EnvironmentProfile[]
+  // Reuse getEnvironments so the createdAt ordering is preserved (the consumer uses
+  // [0] as the default environment when none is saved — index order ≠ creation order).
+  const profiles = await getEnvironments(specId)
   if (profiles.length === 0) return []
+  const db = await getDB()
   // Read all credentials within a single transaction instead of opening one DB
   // connection per profile.
   const tx = db.transaction("environmentCredentials", "readonly")

src/lib/openapi/generate-example.test.ts

@@ -26,6 +26,16 @@ describe("generateExample circular handling", () => {
     }
     expect(generateExample(schema)).not.toBeNull()
   })
+
+  it("does not prune a schema shared between sibling fields (non-circular reuse)", () => {
+    const shared: SchemaObject = { type: "object", properties: { value: { type: "string" } } }
+    const schema: SchemaObject = { type: "object", properties: { a: shared, b: shared } }
+    const ex = generateExample(schema) as Record<string, unknown> | null
+    expect(ex).not.toBeNull()
+    // Both siblings must retain content; the second must not be emptied as a "cycle".
+    expect((ex?.a as Record<string, unknown> | undefined)?.value).toBeDefined()
+    expect((ex?.b as Record<string, unknown> | undefined)?.value).toBeDefined()
+  })
 })
 
 describe("generateWithVariant boundaries", () => {

src/lib/openapi/generate-example.ts

@@ -277,15 +277,18 @@ export function generateWithVariant(rawSchema: SchemaObject, variantId: string):
 // Replace residual $ref nodes (left by the parser's circular:"ignore") and any
 // _circular/_unresolved-marked nodes with an empty schema, so openapi-sampler can
 // still produce a partial example instead of throwing on the first $ref.
-function pruneCircularRefs(schema: unknown, seen: WeakSet<object> = new WeakSet()): unknown {
+// `ancestors` tracks only the current recursion path (added on enter, removed on
+// leave) so a schema *shared* between siblings isn't mistaken for a cycle.
+function pruneCircularRefs(schema: unknown, ancestors: WeakSet<object> = new WeakSet()): unknown {
   if (!schema || typeof schema !== "object") return schema
-  if (Array.isArray(schema)) return schema.map(s => pruneCircularRefs(s, seen))
+  if (Array.isArray(schema)) return schema.map(s => pruneCircularRefs(s, ancestors))
   const obj = schema as Record<string, unknown>
   if (typeof obj.$ref === "string" || obj._circular || obj._unresolved) return {}
-  if (seen.has(schema)) return {}
-  seen.add(schema)
+  if (ancestors.has(schema)) return {} // genuine cycle: node reachable from itself
+  ancestors.add(schema)
   const out: Record<string, unknown> = {}
-  for (const [k, v] of Object.entries(obj)) out[k] = pruneCircularRefs(v, seen)
+  for (const [k, v] of Object.entries(obj)) out[k] = pruneCircularRefs(v, ancestors)
+  ancestors.delete(schema)
   return out
 }
 

src/lib/openapi/parser.ts

@@ -15,6 +15,7 @@ import type {
   ServerObject,
 } from "./types"
 import { isExternalRefAllowed, originOf } from "./url-guard"
+import { readResponseTextCapped } from "@/lib/fetch-utils"
 
 export const HTTP_METHODS = ["get", "post", "put", "patch", "delete", "head", "options"] as const
 export type HttpMethod = (typeof HTTP_METHODS)[number]
@@ -57,7 +58,9 @@ function buildParserOptions(allowedOrigins: string[], blocked: Set<string>): Par
       if (!res.ok) {
         throw new Error(`Failed to fetch external $ref ${file.url}: ${res.status}`)
       }
-      return await res.text()
+      // Cap external ref bodies too — a same-origin ref could otherwise return a
+      // huge response and exhaust memory before parsing.
+      return await readResponseTextCapped(res)
     },
   }
   return {

src/lib/openapi/url-guard.test.ts

@@ -18,11 +18,23 @@ describe("url-guard", () => {
         expect(isPrivateOrLocalHost(h)).toBe(true)
       }
     })
+    it("flags IPv4-mapped IPv6 in hex form (browser normalization)", () => {
+      // new URL("http://[::ffff:127.0.0.1]").hostname → "::ffff:7f00:1"
+      expect(isPrivateOrLocalHost("::ffff:7f00:1")).toBe(true)       // 127.0.0.1
+      expect(isPrivateOrLocalHost("::ffff:c0a8:1")).toBe(true)       // 192.168.0.1
+      expect(isPrivateOrLocalHost("::ffff:a9fe:a9fe")).toBe(true)    // 169.254.169.254
+      expect(isPrivateOrLocalHost("::ffff:808:808")).toBe(false)     // 8.8.8.8 — public
+    })
     it("flags internal hostnames and bare single-label names", () => {
       for (const h of ["localhost", "foo.localhost", "service.local", "db.internal", "intranet"]) {
         expect(isPrivateOrLocalHost(h)).toBe(true)
       }
     })
+    it("flags multicast addresses (IPv4 + IPv6)", () => {
+      expect(isPrivateOrLocalHost("224.0.0.1")).toBe(true)
+      expect(isPrivateOrLocalHost("239.255.255.250")).toBe(true)
+      expect(isPrivateOrLocalHost("ff02::1")).toBe(true)
+    })
     it("allows normal public hostnames", () => {
       for (const h of ["example.com", "api.example.com", "petstore.swagger.io"]) {
         expect(isPrivateOrLocalHost(h)).toBe(false)

src/lib/openapi/url-guard.ts

@@ -3,46 +3,38 @@
 // server URLs must be treated as untrusted to prevent SSRF / internal-network
 // probing from the victim's browser. The library's own safeUrlResolver is a
 // no-op in the browser, so we enforce these checks ourselves.
+//
+// IP range classification is delegated to ipaddr.js (2500M weekly downloads,
+// pure-JS, handles IPv4-mapped IPv6 in both dotted and hex forms, no CVEs).
+
+import * as ipaddr from "ipaddr.js"
+
+const NON_PUBLIC_RANGES = new Set([
+  "unspecified", "broadcast", "loopback", "private", "multicast",
+  "linkLocal", "uniqueLocal", "carrierGradeNat", "reserved",
+  "benchmarking", "amt", "as112v4", "as112v6", "ietf",
+  "6to4", "teredo", "orchid2", "droneRemoteIdProtocol",
+])
 
 /**
  * True if the hostname points at a loopback, link-local, private, or otherwise
  * internal target that an untrusted spec must not be able to reach.
  */
 export function isPrivateOrLocalHost(hostname: string): boolean {
   let h = hostname.toLowerCase().trim()
-  // Strip IPv6 brackets that URL.hostname keeps (e.g. "[::1]").
   if (h.startsWith("[") && h.endsWith("]")) h = h.slice(1, -1)
   if (!h) return true
 
-  // Hostname-based internal suffixes.
   if (h === "localhost" || h.endsWith(".localhost")) return true
   if (h.endsWith(".local") || h.endsWith(".internal") || h.endsWith(".home.arpa")) return true
 
-  // IPv4 literal.
-  const ipv4 = h.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/)
-  if (ipv4) {
-    const a = Number(ipv4[1])
-    const b = Number(ipv4[2])
-    if (a === 0 || a === 127 || a === 10) return true
-    if (a === 169 && b === 254) return true // link-local
-    if (a === 192 && b === 168) return true
-    if (a === 172 && b >= 16 && b <= 31) return true
-    if (a === 100 && b >= 64 && b <= 127) return true // CGNAT 100.64.0.0/10
-    return false
-  }
-
-  // IPv6 literal.
-  if (h.includes(":")) {
-    if (h === "::1" || h === "::") return true
-    if (h.startsWith("fc") || h.startsWith("fd")) return true // unique-local fc00::/7
-    if (h.startsWith("fe8") || h.startsWith("fe9") || h.startsWith("fea") || h.startsWith("feb")) return true // link-local fe80::/10
-    const mapped = h.match(/::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i)
-    if (mapped) return isPrivateOrLocalHost(mapped[1]!)
-    return false
+  if (ipaddr.isValid(h)) {
+    const addr = ipaddr.process(h)
+    return NON_PUBLIC_RANGES.has(addr.range())
   }
 
   // Bare single-label hostname (no dot) — likely an intranet name.
-  if (!h.includes(".")) return true
+  if (!h.includes(".") && !h.includes(":")) return true
 
   return false
 }

vite.config.ts

@@ -32,7 +32,10 @@ function cspPlugin() {
     "worker-src 'self' blob:",
     "object-src 'none'",
     "base-uri 'self'",
-    "frame-ancestors 'none'",
+    // Clickjacking: neither frame-ancestors (CSP) nor X-Frame-Options can be
+    // enforced via <meta> — both require HTTP response headers. Deployers must set
+    // `Content-Security-Policy: frame-ancestors 'none'` (or `X-Frame-Options: DENY`)
+    // in their server/CDN/Cloudflare Pages _headers config.
   ].join("; ")
   return {
     name: "apilot-csp",
@@ -62,11 +65,14 @@ export default defineConfig(({ mode }) => {
         globPatterns: ["**/*.{js,css,html,svg}"],
         runtimeCaching: [
           {
-            // Never cache credentialed spec fetches (Authorization header) — that
-            // would persist a protected API document to disk for later, possibly
-            // unauthenticated, readers. Only cache successful responses.
-            urlPattern: ({ request, url }: { request: Request; url: URL }) =>
-              !request.headers.has("authorization")
+            // Only cache cross-origin *public* specs. Same-origin specs may be
+            // cookie/session-authenticated (the Cookie header isn't visible to JS,
+            // so we can't filter on it), and Authorization-bearing requests are
+            // excluded outright. Caching a protected doc could expose it to a later,
+            // unauthenticated reader on a shared machine. Only cache 200s.
+            urlPattern: ({ request, url, sameOrigin }: { request: Request; url: URL; sameOrigin: boolean }) =>
+              !sameOrigin
+              && !request.headers.has("authorization")
               && /(?:^|\/)(?:spec|openapi|swagger|asyncapi)[^/]*\.(json|ya?ml)$/i.test(url.pathname),
             handler: "NetworkFirst",
             options: {