Skip to content
This repository was archived by the owner on May 1, 2026. It is now read-only.

Reject symbolic links in build zip path #538

Closed
riderx wants to merge 9 commits into from
Closed

Reject symbolic links in build zip path #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dd4beb0
Reject symbolic links when zipping build input
riderx Mar 8, 2026
0345733
Handle symlinks without aborting build zipping
riderx Mar 8, 2026
5b73ad6
Add zip filter test for pnpm symlinked packages
riderx Mar 8, 2026
2d60d05
Handle broken symlinks during zip traversal
riderx Mar 9, 2026
cfdef20
Merge branch 'main' into riderx/fix-capgo-symlink-zip
riderx Mar 16, 2026
ae69aae
fix: narrow symlink zip error handling
riderx Mar 17, 2026
32a2a10
Merge branch 'main' into riderx/fix-capgo-symlink-zip
riderx Mar 18, 2026
f118e75
fix: lint
riderx Mar 18, 2026
21f4e2a
fix: only include read change
riderx Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions src/build/request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

import type { BuildCredentials, BuildOptionsPayload, BuildRequestOptions, BuildRequestResult } from '../schemas/build'
import { Buffer } from 'node:buffer'
import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'
import { existsSync, lstatSync, readdirSync, readFileSync, statSync } from 'node:fs'
import { mkdir, readFile as readFileAsync, rm, stat, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { basename, join, resolve } from 'node:path'
Expand Down Expand Up @@ -707,7 +707,29 @@ function addDirectoryToZip(
for (const item of items) {
const itemPath = join(dirPath, item)
const itemZipPath = zipPath ? `${zipPath}/${item}` : item
const stats = statSync(itemPath)
const lstats = lstatSync(itemPath)
const isSymbolicLink = lstats.isSymbolicLink()
let stats = lstats

if (isSymbolicLink) {
try {
stats = statSync(itemPath)
}
catch (error) {
const code = (error as NodeJS.ErrnoException).code
if (code === 'ENOENT' || code === 'ENOTDIR') {
// Broken symlink: skip gracefully to avoid failing the whole zip.
continue
}
throw error
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Comment on lines +866 to +878
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Keep followed symlinks inside projectDir.

Once the symlink branch resolves successfully, the later recursion walks whatever target it points to. A repo can hang ios, resources, or a matched node_modules package off a path outside the project and the CLI will zip/upload arbitrary local files. Please resolve the real path before recursing, reject targets outside the project root, and dedupe visited real paths so symlink loops cannot recurse forever.

Also applies to: 905-961

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/build/request.ts` around lines 866 - 878, When handling symbolic links in
the isSymbolicLink branch (around statSync/itemPath), resolve the link target to
its real path (using realpathSync or async equivalent), ensure the resolved path
is inside projectDir (reject and skip if it is outside), and maintain a Set of
visited real paths to dedupe and prevent recursive loops (skip if already
visited); then recurse using the resolved real path instead of the original
symlink path. Apply the same realpath/inside-project/visited-set checks to the
other symlink-handling section referenced (lines ~905-961) so external targets
are never included and symlink loops cannot cause infinite recursion.

const shouldInclude = shouldIncludeFile(
itemZipPath,
platform,
nativeDeps,
platformDir,
)

if (stats.isDirectory()) {
// Skip excluded directories
Expand Down Expand Up @@ -735,7 +757,7 @@ function addDirectoryToZip(
// 1. This directory itself should be included (matches a pattern)
// 2. This directory is a prefix of a dependency path (need to traverse to reach it)
const normalizedItemPath = itemZipPath.replace(/\\/g, '/')
const shouldRecurse = shouldIncludeFile(itemZipPath, platform, nativeDeps, platformDir)
const shouldRecurse = shouldInclude
// Ensure we can reach nested platform directories like projects/app/android.
|| platformDir === normalizedItemPath
|| platformDir.startsWith(`${normalizedItemPath}/`)
Expand All @@ -744,17 +766,24 @@ function addDirectoryToZip(
return depPath.startsWith(`${normalizedItemPath}/`) || normalizedItemPath.startsWith(`node_modules/${pkg}`)
})

// Skip unrelated symlinks instead of failing hard.
if (isSymbolicLink && !shouldRecurse)
continue

if (shouldRecurse) {
addDirectoryToZip(zip, itemPath, itemZipPath, platform, nativeDeps, platformDir)
}
}
else if (stats.isFile()) {
if (isSymbolicLink)
continue
Comment on lines +778 to +779
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop dropping required symlinked files from the build zip

This unconditional skip regresses projects that share build-critical files via symlink. statSync() has already resolved the target, but a symlinked package.json, capacitor.config.json, or shared Podfile is still omitted from the archive. The capacitor.config.json case is especially problematic: zipDirectory() only adds the JSON fallback when existsSync(configJsonPath) is false, so a symlinked config file can disappear from the zip entirely and the remote builder loses custom ios.path/android.path settings.

Useful? React with 👍 / 👎.


// Skip excluded files
if (item === '.DS_Store' || item.endsWith('.log'))
continue

// Check if we should include this file
if (shouldIncludeFile(itemZipPath, platform, nativeDeps, platformDir)) {
if (shouldInclude) {
zip.addLocalFile(itemPath, zipPath || undefined)
}
}
Expand Down
Loading
Loading