From 09f7163158f9929538cb38b07be3edba55e37c24 Mon Sep 17 00:00:00 2001 From: wecbaiyk-blip Date: Sat, 9 May 2026 02:02:35 +0300 Subject: [PATCH 1/2] security: ignore symlinks by default in fsAsync.readdirRecursive readdirRecursive supports an `ignoreSymlinks` option but defaults it to false. Three of the four current callers omit the option entirely: - src/deploy/apphosting/util.ts createLocalBuildTarArchive (line 32) - src/deploy/apphosting/util.ts createSourceDeployArchive (line 98) - src/deploy/functions/prepareFunctionsUpload.ts (line 117) A symlink in the source tree pointing at /proc/self/environ, ~/.config/gcloud/application_default_credentials.json, ~/.ssh/id_rsa, or similar will be followed during App Hosting and Cloud Functions deploys and its target's contents uploaded to whatever destination the deploy targets. Largest exposure is CI workflows that extract attacker-supplied tarballs. This change flips the default to true. Callers that legitimately need symlink-following can opt in explicitly with `ignoreSymlinks: false`. The existing safe caller (src/archiveDirectory.ts) keeps working unchanged because it passes ignoreSymlinks: true explicitly. Tests updated to assert the new safer default and the explicit-opt-out path. --- src/fsAsync.spec.ts | 38 ++++++++++++++++++++++++++++---------- src/fsAsync.ts | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/src/fsAsync.spec.ts b/src/fsAsync.spec.ts index ce7cc1e369b..7ce0eb5e6c4 100644 --- a/src/fsAsync.spec.ts +++ b/src/fsAsync.spec.ts @@ -128,28 +128,46 @@ describe("fsAsync", () => { return expect(gotFileNames).to.deep.equal(expectFiles); }); - it("should ignore symlinks when option is set", async () => { - const symlinkPath = path.join(baseDir, "symlink"); + it("ignores symlinks by default (security: prevents symlink-following during deploy)", async () => { + const symlinkPath = path.join(baseDir, "default-symlink"); + fs.symlinkSync(path.join(baseDir, "visible"), symlinkPath); + + try { + const results = await fsAsync.readdirRecursive({ path: baseDir }); + const gotFileNames = results.map((r) => r.name).sort(); + const expectFiles = files.map((file) => path.join(baseDir, file)).sort(); + // Symlink MUST NOT appear in the result by default. Following symlinks + // during deploy/upload is a security boundary issue (see JSDoc on + // ReaddirRecursiveOpts.ignoreSymlinks). + expect(gotFileNames).to.deep.equal(expectFiles); + expect(gotFileNames).to.not.include(symlinkPath); + } finally { + fs.unlinkSync(symlinkPath); + } + }); + + it("includes symlinks only when ignoreSymlinks is explicitly set to false", async () => { + const symlinkPath = path.join(baseDir, "explicit-symlink"); fs.symlinkSync(path.join(baseDir, "visible"), symlinkPath); try { const resultsWithSymlinks = await fsAsync.readdirRecursive({ path: baseDir, - ignoreSymlinks: true, + ignoreSymlinks: false, }); const gotFileNamesWithSymlinks = resultsWithSymlinks.map((r) => r.name).sort(); - const expectFiles = files.map((file) => path.join(baseDir, file)).sort(); - expect(gotFileNamesWithSymlinks).to.deep.equal(expectFiles); + const expectFilesWithSymlinks = [...files, "explicit-symlink"] + .map((file) => path.join(baseDir, file)) + .sort(); + expect(gotFileNamesWithSymlinks).to.deep.equal(expectFilesWithSymlinks); const resultsWithoutSymlinks = await fsAsync.readdirRecursive({ path: baseDir, - ignoreSymlinks: false, + ignoreSymlinks: true, }); const gotFileNamesWithoutSymlinks = resultsWithoutSymlinks.map((r) => r.name).sort(); - const expectFilesWithSymlinks = [...files, "symlink"] - .map((file) => path.join(baseDir, file)) - .sort(); - expect(gotFileNamesWithoutSymlinks).to.deep.equal(expectFilesWithSymlinks); + const expectFiles = files.map((file) => path.join(baseDir, file)).sort(); + expect(gotFileNamesWithoutSymlinks).to.deep.equal(expectFiles); } finally { fs.unlinkSync(symlinkPath); } diff --git a/src/fsAsync.ts b/src/fsAsync.ts index 7bb2b36d066..b400858d3f9 100644 --- a/src/fsAsync.ts +++ b/src/fsAsync.ts @@ -3,6 +3,7 @@ import ignorePkg from "ignore"; import * as _ from "lodash"; import * as minimatch from "minimatch"; import { join, relative } from "path"; +import { logger } from "./logger"; export interface ReaddirRecursiveOpts { // The directory to recurse. @@ -14,7 +15,19 @@ export interface ReaddirRecursiveOpts { include?: string[]; // Maximum depth to recurse. maxDepth?: number; - // Ignore symlinked files or directories when traversing. + /** + * Ignore symlinked files or directories when traversing. + * + * **Defaults to `true`** for security: following symlinks during deploy/upload + * lets a path inside the source tree leak the contents of an arbitrary path + * outside it (including secrets like `/proc/self/environ`, SSH keys, or other + * credentials reachable from the deploy process). Pass `false` only if you + * have an audited reason to follow symlinks. + * + * If a symlink is encountered while this is `true`, the entry is silently + * dropped from the recursion (matching the prior `ignoreSymlinks: true` + * behavior). A `debug`-level log entry is emitted to aid debugging. + */ ignoreSymlinks?: boolean; } @@ -31,7 +44,16 @@ async function readdirRecursiveHelper(options: { }): Promise { const dirContents = readdirSync(options.path, { withFileTypes: true }); const fullPaths = dirContents - .filter((n) => !options.ignoreSymlinks || !n.isSymbolicLink()) + .filter((n) => { + if (options.ignoreSymlinks && n.isSymbolicLink()) { + logger.debug( + `[fsAsync] dropping symlink ${join(options.path, n.name)} ` + + `(set ignoreSymlinks:false to include it)`, + ); + return false; + } + return true; + }) .map((n) => join(options.path, n.name)); const filteredPaths = fullPaths.filter((p) => !options.filter(p)); const filePromises: Array> = []; @@ -63,6 +85,15 @@ async function readdirRecursiveHelper(options: { /** * Recursively read a directory. + * + * **Security note:** `ignoreSymlinks` defaults to `true`. Callers that + * upload, archive, or otherwise transport the resulting file list across + * a trust boundary (e.g. Cloud Functions deploy, App Hosting deploy, + * Hosting upload) MUST keep this default. Passing `false` causes the + * recursion to follow symlinks and may include arbitrary attacker-controlled + * targets in the produced file list (e.g. `/proc/self/environ`, + * `~/.ssh/id_rsa`, GCP application-default-credentials JSON). + * * @param options options object. * @return array of files that match. */ @@ -88,10 +119,12 @@ export async function readdirRecursive( }); }; const maxDepth = options.maxDepth && options.maxDepth > 0 ? options.maxDepth : Infinity; + // Default `ignoreSymlinks` to `true`. See JSDoc above for security rationale. + const ignoreSymlinks = options.ignoreSymlinks !== false; return await readdirRecursiveHelper({ path: options.path, filter: filter, maxDepth, - ignoreSymlinks: !!options.ignoreSymlinks, + ignoreSymlinks, }); } From ade62491312751650b32ae45ea48566a614b7a97 Mon Sep 17 00:00:00 2001 From: wecbaiyk-blip Date: Sat, 9 May 2026 02:26:00 +0300 Subject: [PATCH 2/2] trigger CLA recheck