Skip to content

Commit f0d9bde

Browse files
rsbhclaude
andcommitted
fix: address PR review — security and robustness fixes
- Path traversal: use safePath() guard for content file serving (dev + prod) - XSS: escape </script> in __PAGE_DATA__ JSON via \u003c replacement - SSRF: validate proxy path in apis-proxy handler - Race condition: cancel stale search fetch responses - useRef: add null initial value for React 19 compat - Error messages: hide internal details in prod error responses - Validate --adapter flag against known adapters Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 111b55a commit f0d9bde

8 files changed

Lines changed: 47 additions & 14 deletions

File tree

packages/chronicle/src/cli/commands/build.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ export const buildCommand = new Command('build')
1313
const contentDir = resolveContentDir(options.content)
1414
const outDir = path.resolve(options.outDir)
1515

16+
const VALID_ADAPTERS = ['vercel']
17+
if (options.adapter && !VALID_ADAPTERS.includes(options.adapter)) {
18+
console.error(chalk.red(`Unknown adapter: ${options.adapter}. Valid adapters: ${VALID_ADAPTERS.join(', ')}`))
19+
process.exit(1)
20+
}
21+
1622
process.env.CHRONICLE_PROJECT_ROOT = process.cwd()
1723
process.env.CHRONICLE_CONTENT_DIR = contentDir
1824

packages/chronicle/src/components/ui/search.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,27 @@ interface SearchResult {
1818
function useSearch(query: string) {
1919
const [results, setResults] = useState<SearchResult[]>([]);
2020
const [isLoading, setIsLoading] = useState(false);
21-
const timerRef = useRef<ReturnType<typeof setTimeout>>();
21+
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
2222

2323
useEffect(() => {
24-
clearTimeout(timerRef.current);
24+
let cancelled = false;
25+
if (timerRef.current) clearTimeout(timerRef.current);
2526
timerRef.current = setTimeout(async () => {
2627
setIsLoading(true);
2728
try {
2829
const params = new URLSearchParams();
2930
if (query) params.set("query", query);
3031
const res = await fetch(`/api/search?${params}`);
31-
setResults(await res.json());
32+
if (!cancelled) setResults(await res.json());
3233
} catch {
33-
setResults([]);
34+
if (!cancelled) setResults([]);
3435
}
35-
setIsLoading(false);
36+
if (!cancelled) setIsLoading(false);
3637
}, 100);
37-
return () => clearTimeout(timerRef.current);
38+
return () => {
39+
cancelled = true;
40+
if (timerRef.current) clearTimeout(timerRef.current);
41+
};
3842
}, [query]);
3943

4044
return { results, isLoading };

packages/chronicle/src/server/dev.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createReadStream } from 'fs'
55
import path from 'path'
66
import chalk from 'chalk'
77
import { createViteConfig } from './vite-config'
8+
import { safePath } from './utils/safe-path'
89

910
export interface DevServerOptions {
1011
port: number
@@ -38,8 +39,8 @@ export async function startDevServer(options: DevServerOptions) {
3839
}
3940

4041
// Serve static files from content dir (skip .md/.mdx)
41-
const contentFile = path.join(contentDir, decodeURIComponent(url.split('?')[0]))
42-
if (!url.endsWith('.md') && !url.endsWith('.mdx')) {
42+
const contentFile = safePath(contentDir, url)
43+
if (contentFile && !url.endsWith('.md') && !url.endsWith('.mdx')) {
4344
try {
4445
const stat = await fsPromises.stat(contentFile)
4546
if (stat.isFile()) {
@@ -119,7 +120,8 @@ export async function startDevServer(options: DevServerOptions) {
119120
template = await vite.transformIndexHtml(url, template)
120121

121122
// Embed page data for client hydration
122-
const dataScript = `<script>window.__PAGE_DATA__ = ${JSON.stringify(embeddedData)}</script>`
123+
const safeJson = JSON.stringify(embeddedData).replace(/</g, '\\u003c')
124+
const dataScript = `<script>window.__PAGE_DATA__ = ${safeJson}</script>`
123125
template = template.replace('<!--head-outlet-->', `<!--head-outlet-->${dataScript}`)
124126

125127
const { render } = await vite.ssrLoadModule(path.resolve(root, 'src/server/entry-server.tsx'))

packages/chronicle/src/server/entry-prod.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { loadConfig } from '@/lib/config'
99
import { loadApiSpecs } from '@/lib/openapi'
1010
import { getPage, loadPageComponent, buildPageTree } from '@/lib/source'
1111
import { handleRequest } from './request-handler'
12+
import { safePath } from './utils/safe-path'
1213

1314
export { render, matchRoute, loadConfig, loadApiSpecs, getPage, loadPageComponent, buildPageTree }
1415

@@ -45,8 +46,8 @@ export async function startServer(options: { port: number; distDir: string }) {
4546

4647
// Serve static files from content dir (skip .md/.mdx)
4748
const contentDir = process.env.CHRONICLE_CONTENT_DIR || process.cwd()
48-
const contentFile = path.join(contentDir, decodeURIComponent(url.split('?')[0]))
49-
if (!url.endsWith('.md') && !url.endsWith('.mdx')) {
49+
const contentFile = safePath(contentDir, url)
50+
if (contentFile && !url.endsWith('.md') && !url.endsWith('.mdx')) {
5051
try {
5152
const stat = await fsPromises.stat(contentFile)
5253
if (stat.isFile()) {
@@ -77,7 +78,7 @@ export async function startServer(options: { port: number; distDir: string }) {
7778
} catch (e) {
7879
console.error(e)
7980
res.statusCode = 500
80-
res.end((e as Error).message)
81+
res.end('Internal Server Error')
8182
}
8283
})
8384

packages/chronicle/src/server/entry-vercel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ export default async function handler(req: IncomingMessage, res: ServerResponse)
2323
} catch (e) {
2424
console.error(e)
2525
res.statusCode = 500
26-
res.end((e as Error).message)
26+
res.end('Internal Server Error')
2727
}
2828
}

packages/chronicle/src/server/handlers/apis-proxy.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ export async function handleApisProxy(req: Request): Promise<Response> {
2020
return Response.json({ error: `Unknown spec: ${specName}` }, { status: 404 })
2121
}
2222

23+
// Validate path doesn't contain protocol or escape the base URL
24+
if (/^[a-z]+:\/\//i.test(path) || path.includes('..')) {
25+
return Response.json({ error: 'Invalid path' }, { status: 400 })
26+
}
27+
2328
const url = spec.server.url + path
2429

2530
try {

packages/chronicle/src/server/request-handler.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ export async function handleRequest(url: string, options: RequestHandlerOptions)
5151

5252
const html = render(url, { config, tree, page: pageData, apiSpecs })
5353

54-
const dataScript = `<script>window.__PAGE_DATA__ = ${JSON.stringify(embeddedData)}</script>`
54+
const safeJson = JSON.stringify(embeddedData).replace(/</g, '\\u003c')
55+
const dataScript = `<script>window.__PAGE_DATA__ = ${safeJson}</script>`
5556
const finalHtml = template
5657
.replace('<!--head-outlet-->', `<!--head-outlet-->${dataScript}`)
5758
.replace('<!--ssr-outlet-->', html)
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import path from 'path'
2+
3+
/**
4+
* Resolve a URL path within a base directory, preventing path traversal.
5+
* Returns null if the resolved path escapes the base directory.
6+
*/
7+
export function safePath(baseDir: string, urlPath: string): string | null {
8+
const decoded = decodeURIComponent(urlPath.split('?')[0])
9+
const resolved = path.resolve(baseDir, '.' + decoded)
10+
if (!resolved.startsWith(path.resolve(baseDir) + path.sep) && resolved !== path.resolve(baseDir)) {
11+
return null
12+
}
13+
return resolved
14+
}

0 commit comments

Comments
 (0)