security: drop symlinks from Hosting deploy upload list

[wecbaiyk-blip]

Summary

src/listFiles.ts enumerates the files that firebase deploy --only hosting (and firebase hosting:channel:deploy) uploads to the Firebase Hosting CDN. It used glob({ follow: true }), and even with follow: false glob still returns symlink-to-file entries because follow only controls whether symlinked directories are descended into. Uploader then calls fs.readFile* on each entry, which follows symlinks at the OS layer, so a symlink under the public dir ends up uploading the bytes of its target.

This PR sets follow: false and adds an explicit lstatSync filter that drops any entry whose lstat reports isSymbolicLink().

Threat model

The largest exposure is CI workflows that extract attacker-supplied tarballs into the public dir before invoking firebase deploy (the classic PR-preview deploy pattern; see GitHub Security Lab on pwn requests). A tarball entry like

• public/leak -> /proc/self/environ
• public/leak -> ~/.config/gcloud/application_default_credentials.json
• public/leak -> ~/.ssh/id_rsa
• public/leak -> /run/secrets/<anything>

would otherwise end up uploaded under a path the attacker chose, on a Firebase-hosted public URL accessible to the world.

What this PR does

• src/listFiles.ts:
  • follow: true -> follow: false so glob does not descend into symlinked directories.
  • Adds an lstatSync-based filter that drops any entry whose lstat reports isSymbolicLink(). This catches the symlink-to-file entries glob still returns under follow: false.
  • Logs a debug-level entry when a symlink is dropped so users who legitimately rely on symlinks can see what was skipped.
  • Documents the threat model and the lstat-vs-stat rationale in JSDoc.
• src/listFiles.spec.ts:
  • Adds tests for symlink-to-file rejection at the top level and nested in subdirectories, and a symlinked-directory case.
  • The existing fixture-based tests are unchanged.

Backward compatibility

Behavior changes only for users whose Hosting public directory contains symlinks they want followed during deploy. That population is small; the typical public/, dist/, build/ directory contains regular files only. Users who need symlink-following can bypass listFiles by copying the dereferenced files into a real directory before deploy (e.g. cp -L), which is also the more predictable thing to do for a CDN upload.

Why this is a separate PR from #10477

PR #10477 flips the default of fsAsync.readdirRecursive. That function is used by Cloud Functions deploy and App Hosting deploy. Hosting deploy uses listFiles, which is a different module backed by glob. Both paths need symlink protection; the two fixes don't overlap.

Testing

npm run test:compile && npm test -- --grep listFiles — three new tests pass, existing fixture-based tests unchanged.

[gemini-code-assist]

Code Review

This pull request enhances security in listFiles by explicitly excluding symbolic links from the deployment list, preventing potential data leaks of files outside the source tree. It also introduces comprehensive unit tests for symlink handling. Feedback includes recommendations to use the node: prefix for built-in module imports, improving the robustness of test cleanup hooks with truthy checks, and ensuring consistent use of filesystem utilities.

[gemini-code-assist]

Inconsistent use of built-in module imports. It is recommended to use the node: prefix for all built-in modules for clarity and to follow modern Node.js conventions. Additionally, mixing namespace imports (import * as fs) with named imports (import { rmSync }) for the same module is redundant when the named export is available on the namespace object.

Suggested change

	import * as crypto from "crypto";
	import * as fs from "fs";
	import * as os from "os";
	import * as path from "path";
	import { rmSync } from "node:fs";
	import * as crypto from "node:crypto";
	import * as fs from "node:fs";
	import * as os from "node:os";
	import * as path from "node:path";

[gemini-code-assist]

The after hook should safely handle cases where tmpRoot or outsideTarget might be empty strings (e.g., if the before hook fails before they are assigned). Calling rmSync("") can throw an error, which might obscure the actual failure in the test report. Adding truthy checks makes the cleanup more robust.

    after(() => {
      if (tmpRoot) {
        fs.rmSync(tmpRoot, { recursive: true, force: true });
      }
      if (outsideTarget) {
        fs.rmSync(outsideTarget, { force: true });
      }
    });

[gemini-code-assist]

Use fs.rmSync for consistency with the namespace import recommended above.

Suggested change

	rmSync(outsideDir, { recursive: true, force: true });
	fs.rmSync(outsideDir, { recursive: true, force: true });