Skip to content

Commit 75106a8

Browse files
chriscrosstalkclaude
authored andcommitted
fix(security): path traversal and SSRF protections from pre-launch audit
Fixes 4 high-severity findings from a comprehensive security audit: 1. Path traversal on ZIM file delete — resolve()+startsWith() containment 2. Path traversal on Map file delete — same pattern 3. Path traversal on docs read — same pattern (already used in rag_service) 4. SSRF on download endpoints — block private/internal IPs, require TLD Also adds assertNotPrivateUrl() to content update endpoints. Full audit report attached as admin/docs/security-audit-v1.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b9dd32b commit 75106a8

File tree

8 files changed

+367
-15
lines changed

8 files changed

+367
-15
lines changed

admin/app/controllers/collection_updates_controller.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { CollectionUpdateService } from '#services/collection_update_service'
22
import {
3+
assertNotPrivateUrl,
34
applyContentUpdateValidator,
45
applyAllContentUpdatesValidator,
56
} from '#validators/common'
@@ -13,12 +14,16 @@ export default class CollectionUpdatesController {
1314

1415
async applyUpdate({ request }: HttpContext) {
1516
const update = await request.validateUsing(applyContentUpdateValidator)
17+
assertNotPrivateUrl(update.download_url)
1618
const service = new CollectionUpdateService()
1719
return await service.applyUpdate(update)
1820
}
1921

2022
async applyAllUpdates({ request }: HttpContext) {
2123
const { updates } = await request.validateUsing(applyAllContentUpdatesValidator)
24+
for (const update of updates) {
25+
assertNotPrivateUrl(update.download_url)
26+
}
2227
const service = new CollectionUpdateService()
2328
return await service.applyAllUpdates(updates)
2429
}

admin/app/controllers/maps_controller.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { MapService } from '#services/map_service'
22
import {
3+
assertNotPrivateUrl,
34
downloadCollectionValidator,
45
filenameParamValidator,
56
remoteDownloadValidator,
@@ -25,12 +26,14 @@ export default class MapsController {
2526

2627
async downloadBaseAssets({ request }: HttpContext) {
2728
const payload = await request.validateUsing(remoteDownloadValidatorOptional)
29+
if (payload.url) assertNotPrivateUrl(payload.url)
2830
await this.mapService.downloadBaseAssets(payload.url)
2931
return { success: true }
3032
}
3133

3234
async downloadRemote({ request }: HttpContext) {
3335
const payload = await request.validateUsing(remoteDownloadValidator)
36+
assertNotPrivateUrl(payload.url)
3437
const filename = await this.mapService.downloadRemote(payload.url)
3538
return {
3639
message: 'Download started successfully',
@@ -52,6 +55,7 @@ export default class MapsController {
5255
// For providing a "preflight" check in the UI before actually starting a background download
5356
async downloadRemotePreflight({ request }: HttpContext) {
5457
const payload = await request.validateUsing(remoteDownloadValidator)
58+
assertNotPrivateUrl(payload.url)
5559
const info = await this.mapService.downloadRemotePreflight(payload.url)
5660
return info
5761
}

admin/app/controllers/zim_controller.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ZimService } from '#services/zim_service'
22
import {
3+
assertNotPrivateUrl,
34
downloadCategoryTierValidator,
45
filenameParamValidator,
56
remoteDownloadWithMetadataValidator,
@@ -25,6 +26,7 @@ export default class ZimController {
2526

2627
async downloadRemote({ request }: HttpContext) {
2728
const payload = await request.validateUsing(remoteDownloadWithMetadataValidator)
29+
assertNotPrivateUrl(payload.url)
2830
const { filename, jobId } = await this.zimService.downloadRemote(payload.url)
2931

3032
return {

admin/app/services/docs_service.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,19 @@ export class DocsService {
6666

6767
const filename = _filename.endsWith('.md') ? _filename : `${_filename}.md`
6868

69-
const fileExists = await getFileStatsIfExists(path.join(this.docsPath, filename))
69+
// Prevent path traversal — resolved path must stay within the docs directory
70+
const basePath = path.resolve(this.docsPath)
71+
const fullPath = path.resolve(path.join(this.docsPath, filename))
72+
if (!fullPath.startsWith(basePath + path.sep)) {
73+
throw new Error('Invalid document slug')
74+
}
75+
76+
const fileExists = await getFileStatsIfExists(fullPath)
7077
if (!fileExists) {
7178
throw new Error(`File not found: ${filename}`)
7279
}
7380

74-
const fileStream = await getFile(path.join(this.docsPath, filename), 'stream')
81+
const fileStream = await getFile(fullPath, 'stream')
7582
if (!fileStream) {
7683
throw new Error(`Failed to read file stream: ${filename}`)
7784
}

admin/app/services/map_service.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
getFile,
1414
ensureDirectoryExists,
1515
} from '../utils/fs.js'
16-
import { join } from 'path'
16+
import { join, resolve, sep } from 'path'
1717
import urlJoin from 'url-join'
1818
import { RunDownloadJob } from '#jobs/run_download_job'
1919
import logger from '@adonisjs/core/services/logger'
@@ -404,7 +404,13 @@ export class MapService implements IMapService {
404404
fileName += '.pmtiles'
405405
}
406406

407-
const fullPath = join(this.baseDirPath, 'pmtiles', fileName)
407+
const basePath = resolve(join(this.baseDirPath, 'pmtiles'))
408+
const fullPath = resolve(join(basePath, fileName))
409+
410+
// Prevent path traversal — resolved path must stay within the storage directory
411+
if (!fullPath.startsWith(basePath + sep)) {
412+
throw new Error('Invalid filename')
413+
}
408414

409415
const exists = await getFileStatsIfExists(fullPath)
410416
if (!exists) {

admin/app/services/zim_service.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
listDirectoryContents,
1717
ZIM_STORAGE_PATH,
1818
} from '../utils/fs.js'
19-
import { join } from 'path'
19+
import { join, resolve, sep } from 'path'
2020
import { WikipediaOption, WikipediaState } from '../../types/downloads.js'
2121
import vine from '@vinejs/vine'
2222
import { wikipediaOptionsFileSchema } from '#validators/curated_collections'
@@ -332,7 +332,13 @@ export class ZimService {
332332
fileName += '.zim'
333333
}
334334

335-
const fullPath = join(process.cwd(), ZIM_STORAGE_PATH, fileName)
335+
const basePath = resolve(join(process.cwd(), ZIM_STORAGE_PATH))
336+
const fullPath = resolve(join(basePath, fileName))
337+
338+
// Prevent path traversal — resolved path must stay within the storage directory
339+
if (!fullPath.startsWith(basePath + sep)) {
340+
throw new Error('Invalid filename')
341+
}
336342

337343
const exists = await getFileStatsIfExists(fullPath)
338344
if (!exists) {

admin/app/validators/common.ts

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,39 @@
11
import vine from '@vinejs/vine'
22

3+
/**
4+
* Checks whether a URL points to a private/internal network address.
5+
* Used to prevent SSRF — the server should not fetch from localhost,
6+
* private RFC1918 ranges, link-local, or cloud metadata endpoints.
7+
*
8+
* Throws an error if the URL is internal/private.
9+
*/
10+
export function assertNotPrivateUrl(urlString: string): void {
11+
const parsed = new URL(urlString)
12+
const hostname = parsed.hostname.toLowerCase()
13+
14+
const privatePatterns = [
15+
/^localhost$/,
16+
/^127\.\d+\.\d+\.\d+$/,
17+
/^10\.\d+\.\d+\.\d+$/,
18+
/^172\.(1[6-9]|2\d|3[01])\.\d+\.\d+$/,
19+
/^192\.168\.\d+\.\d+$/,
20+
/^169\.254\.\d+\.\d+$/, // Link-local / cloud metadata
21+
/^0\.0\.0\.0$/,
22+
/^\[::1\]$/,
23+
/^\[?fe80:/i,
24+
/^\[?fd[0-9a-f]{2}:/i, // Unique local IPv6
25+
]
26+
27+
if (privatePatterns.some((re) => re.test(hostname))) {
28+
throw new Error(`Download URL must not point to a private/internal address: ${hostname}`)
29+
}
30+
}
31+
332
export const remoteDownloadValidator = vine.compile(
433
vine.object({
534
url: vine
635
.string()
7-
.url({
8-
require_tld: false, // Allow local URLs
9-
})
36+
.url()
1037
.trim(),
1138
})
1239
)
@@ -15,9 +42,7 @@ export const remoteDownloadWithMetadataValidator = vine.compile(
1542
vine.object({
1643
url: vine
1744
.string()
18-
.url({
19-
require_tld: false, // Allow local URLs
20-
})
45+
.url()
2146
.trim(),
2247
metadata: vine
2348
.object({
@@ -34,9 +59,7 @@ export const remoteDownloadValidatorOptional = vine.compile(
3459
vine.object({
3560
url: vine
3661
.string()
37-
.url({
38-
require_tld: false, // Allow local URLs
39-
})
62+
.url()
4063
.trim()
4164
.optional(),
4265
})

0 commit comments

Comments
 (0)