Fix deploy silently truncating tarball on dangling symlink

DESIGN.md

@@ -264,3 +264,26 @@ replicability metadata is deferred to the cluster-level-config work (CORE-3018),
 schema. Per-peer failures never reject: they come back as `{status: 'failed', reason, node}` entries
 in `response.replicated[]`, and `message` still reads as success (same contract as drop_schema), so
 operators must inspect the array for per-node outcomes.
+
+## A dangling symlink silently truncates the deploy tarball (`components/packageComponent.ts`)
+
+Packaging uses `tar-fs.pack(dir, { dereference: true })` by default (`skip_symlinks` off).
+tar-fs's own walker calls `fs.stat` (not `lstat`) on every discovered entry when dereferencing; a
+dangling symlink's target throws `ENOENT`, and tar-fs's `statAll` loop treats _any_ `ENOENT` from
+a walk-discovered (not explicitly-requested) entry as end-of-stream — it calls `pack.finalize()`
+immediately, silently dropping every entry still queued (BFS order) after the link. No error is
+ever emitted, so `packStream.on('error', ...)` never fires and `deploy_component` reports success
+on a truncated archive. `scanPackageDirectory()` now pre-walks the tree once (async) to build a
+skip-set of dangling symlinks, which `streamPackagedDirectory`'s `tar.pack({ ignore })` consults via
+a synchronous `Set.has()` — **`ignore` is called synchronously by tar-fs with no Promise support**,
+so any fix here has to resolve the dangling set _before_ constructing `tar.pack`, not from inside
+the callback (an earlier draft used `lstatSync`/`statSync` per entry there, which would have added
+blocking I/O to a path that also runs inline on the Harper server's event loop via the
+`package_component` operation). The scan recurses into _valid_ symlinked directories the same way
+tar-fs's dereferenced walk does (readdir through the link), since a dangling symlink nested inside
+one is just as capable of tripping the same early-finalize — skipping recursion into symlinked
+dirs there would silently reintroduce the bug for that case. Circular directory symlinks are not
+guarded against (in the scan or in tar-fs's own pack walk); that's a pre-existing tar-fs limitation
+this fix doesn't attempt to solve. `deploy_component`/`package_component` still never validate that
+declared entry points (`jsResource`/`graphqlSchema`) survived extraction — a truncation from some
+other future cause would still report success silently; that's a deferred, separate fix.

bin/cliOperations.ts

@@ -9,7 +9,7 @@ import { httpRequest } from '../utility/common_utils.ts';
 import * as path from 'path';
 import * as fs from 'fs-extra';
 import * as YAML from 'yaml';
-import { streamPackagedDirectory, getPackagedDirectorySize, packageDirectory } from '../components/packageComponent.ts';
+import { streamPackagedDirectory, packageDirectory, scanPackageDirectory } from '../components/packageComponent.ts';
 import { encode as encodeCbor } from 'cbor-x';
 import { buildMultipartBody } from './multipartBuilder.ts';
 import { parseSSE } from './sseConsumer.ts';
@@ -113,9 +113,20 @@ const PREPARE_OPERATION: any = {
 		// so the pre-gzip onBytes callback can be wired directly to renderer.countUploadBytes.
 		req._projectPath = projectPath;
 		req._packageOptions = packageOptions;
-		// Pre-walk the directory for an uncompressed-size estimate. Both the progress counter
-		// and this total are in uncompressed units so the bar tracks to 100% naturally.
-		req._uploadSizeEstimate = await getPackagedDirectorySize(projectPath, packageOptions);
+		// Pre-walk the directory once for both the uncompressed-size estimate (progress bar
+		// total) and the dangling-symlink list — a dangling symlink would otherwise silently
+		// truncate the tarball (tar-fs finalizes early on the broken target). Packaging skips
+		// them; the list is reused below (no second walk) and warns the user which links were
+		// skipped so the omission is visible.
+		const scan = await scanPackageDirectory(projectPath, packageOptions);
+		req._uploadSizeEstimate = scan.totalSize;
+		req._danglingSymlinks = scan.danglingSymlinks;
+		if (scan.danglingSymlinks.length) {
+			process.stderr.write(
+				`warning: skipping ${scan.danglingSymlinks.length} broken symlink(s) — their linked content will NOT be deployed:\n` +
+					scan.danglingSymlinks.map((p) => `  ${p}\n`).join('')
+			);
+		}
 		req._multipart = true;
 	},
 };
@@ -306,7 +317,8 @@ async function cliOperations(req: any, skipResponseLog = false) {
 			const packageStream = streamPackagedDirectory(
 				req._projectPath,
 				req._packageOptions,
-				renderer ? (n) => renderer.countUploadBytes(n) : undefined
+				renderer ? (n) => renderer.countUploadBytes(n) : undefined,
+				req._danglingSymlinks
 			);
 			const fields = operationFields(req);
 			const multipart = buildMultipartBody(fields, {

components/packageComponent.ts

@@ -1,6 +1,6 @@
 import { join, relative, sep } from 'node:path';
 import { stat, readdir } from 'node:fs/promises';
-import { Readable } from 'node:stream';
+import { Readable, pipeline } from 'node:stream';
 import tar from 'tar-fs';
 import { createGzip } from 'node:zlib';
 
@@ -18,61 +18,47 @@ const WEBPACK_CACHE_SEGMENT = join('cache', 'webpack');
  * `skip_node_modules` is set. The path is first made relative to `directory`, so packaging a component
  * that itself lives under a `node_modules/` path — i.e. any npm-installed component — does not match
  * every entry. tar-fs invokes `ignore` with the absolute path, so a substring test on it wrongly
- * excluded the whole tree. Shared by the stream packer and the size walk so the two cannot diverge.
+ * excluded the whole tree. Shared by the stream packer and the directory walk so they cannot diverge.
  */
 function isExcluded(directory: string, fullPath: string, options: PackageOptions): boolean {
 	if (!options.skip_node_modules) return false;
 	const rel = relative(directory, fullPath);
 	return rel.split(sep).includes('node_modules') || rel.includes(WEBPACK_CACHE_SEGMENT);
 }
 
-/**
- * Package a directory into a tar+gzip stream. The returned Readable can be
- * piped directly into an HTTP request body, avoiding the Node.js 2GB Buffer
- * cap that the buffered variant runs into for large components.
- *
- * @param onBytes - Optional callback invoked with the byte length of each raw
- *   tar chunk *before* gzip compression. Useful for tracking upload progress
- *   against an uncompressed-size total (e.g. from `getPackagedDirectorySize`).
- */
-export function streamPackagedDirectory(
-	directory: string,
-	options: PackageOptions = DEFAULT_OPTIONS,
-	onBytes?: (n: number) => void
-): Readable {
-	const packStream = tar.pack(directory, {
-		dereference: !options.skip_symlinks,
-		ignore: (name: string) => isExcluded(directory, name, options),
-		map: (header) => {
-			if (header.type === 'directory') {
-				header.mode = 0o755;
-			}
-			return header;
-		},
-	});
-	const gzip = createGzip();
-	// Propagate pack errors onto the gzip stream so a single consumer can listen
-	packStream.on('error', (err) => gzip.destroy(err));
-	if (onBytes) {
-		// Attaching a 'data' listener after pipe() is safe — the stream is already
-		// in flowing mode and Node's EventEmitter supports multiple listeners.
-		packStream.on('data', (chunk: Buffer) => onBytes(chunk.length));
-	}
-	return packStream.pipe(gzip);
+export interface PackageDirectoryScan {
+	/** Total uncompressed byte size of the files `streamPackagedDirectory` would pack. */
+	totalSize: number;
+	/**
+	 * Relative paths of symlinks whose target does not exist. Under tar-fs's `dereference: true`
+	 * (our default), such a link makes the walker's stat() throw ENOENT, which tar-fs treats as
+	 * end-of-stream: it *finalizes the archive early*, silently dropping every entry queued after
+	 * the link. `streamPackagedDirectory` skips these paths so the walk continues past them.
+	 */
+	danglingSymlinks: string[];
 }
 
 /**
- * Walk `directory` and return the total uncompressed size of all files that
- * `streamPackagedDirectory` would include with the same options. Used by the
- * CLI to give the upload progress bar a realistic total. The uncompressed size
- * won't equal the gzipped wire size, but it gives the bar a steady trajectory:
- * the bar moves as bytes are sent and snaps to 100% when the upload finishes.
+ * Walk `directory` once, computing both the packable size total and the dangling-symlink list —
+ * the two pieces of metadata the CLI's pre-deploy checks and the packer's ignore-set both need.
+ * A single shared walker keeps size accounting and dangling-link detection from silently
+ * diverging (e.g. one recursing into symlinked directories and the other not).
+ *
+ * Recurses into a *valid* symlinked directory the same way tar-fs's own dereferenced walk does
+ * (readdir through the link), so a dangling symlink nested inside one is still found — otherwise
+ * tar-fs would hit that nested ENOENT and truncate the archive regardless of this scan's result.
+ * This mirrors tar-fs's own lack of symlink-cycle protection under `dereference`; a circular
+ * symlink can already hang tar-fs's real pack walk today, so this scan doesn't guard against it
+ * either — fixing that is a separate, pre-existing limitation outside this change's scope.
  */
-export async function getPackagedDirectorySize(
+export async function scanPackageDirectory(
 	directory: string,
 	options: PackageOptions = DEFAULT_OPTIONS
-): Promise<number> {
-	let total = 0;
+): Promise<PackageDirectoryScan> {
+	let totalSize = 0;
+	const danglingSymlinks: string[] = [];
+	const dereference = !options.skip_symlinks;
+
 	const walk = async (dir: string): Promise<void> => {
 		let entries;
 		try {
@@ -82,24 +68,120 @@ export async function getPackagedDirectorySize(
 		}
 		for (const entry of entries) {
 			const fullPath = join(dir, entry.name);
-			if (isExcluded(directory, fullPath, options)) {
-				continue;
-			}
-			if (entry.isDirectory()) {
+			if (isExcluded(directory, fullPath, options)) continue;
+			if (entry.isSymbolicLink()) {
+				// Without dereference, tar-fs packs the link literally and never stats the
+				// target, so an unresolved link is neither a size contributor nor a hazard.
+				if (!dereference) continue;
+				let target;
+				try {
+					target = await stat(fullPath); // follows the link; throws if missing
+				} catch {
+					danglingSymlinks.push(relative(directory, fullPath));
+					continue;
+				}
+				if (target.isDirectory()) {
+					await walk(fullPath);
+				} else {
+					totalSize += target.size;
+				}
+			} else if (entry.isDirectory()) {
 				await walk(fullPath);
 			} else {
-				if (options.skip_symlinks && entry.isSymbolicLink()) continue;
 				try {
-					const s = await stat(fullPath); // follows symlinks, matching tar dereference
-					total += s.size;
+					totalSize += (await stat(fullPath)).size;
 				} catch {
 					// inaccessible file — skip
 				}
 			}
 		}
 	};
 	await walk(directory);
-	return total;
+	return { totalSize, danglingSymlinks };
+}
+
+/**
+ * Package a directory into a tar+gzip stream. The returned Readable can be
+ * piped directly into an HTTP request body, avoiding the Node.js 2GB Buffer
+ * cap that the buffered variant runs into for large components.
+ *
+ * @param onBytes - Optional callback invoked with the byte length of each raw
+ *   tar chunk *before* gzip compression. Useful for tracking upload progress
+ *   against an uncompressed-size total (e.g. from `getPackagedDirectorySize`).
+ * @param knownDanglingSymlinks - Skip this function's own directory scan and reuse a
+ *   dangling-symlink list the caller already computed (e.g. via `scanPackageDirectory`
+ *   during a prior CLI step), avoiding a second full-tree walk before packing starts.
+ */
+export function streamPackagedDirectory(
+	directory: string,
+	options: PackageOptions = DEFAULT_OPTIONS,
+	onBytes?: (n: number) => void,
+	knownDanglingSymlinks?: string[]
+): Readable {
+	const dereference = !options.skip_symlinks;
+	const gzip = createGzip();
+	// Resolving the dangling-symlink set up front — rather than stat-ing candidates
+	// synchronously from tar-fs's `ignore` callback — keeps the pack walk free of blocking
+	// I/O. That matters here because this also runs server-side, inline on the shared event
+	// loop, via the package_component operation.
+	const dangling = knownDanglingSymlinks
+		? Promise.resolve(knownDanglingSymlinks)
+		: scanPackageDirectory(directory, options).then((scan) => scan.danglingSymlinks);
+	dangling
+		.then((danglingSymlinks) => {
+			// The consumer may have already aborted (destroying gzip) while the prescan was
+			// still resolving — skip starting a directory walk nothing will read.
+			if (gzip.destroyed) return;
+			const danglingSet = new Set(danglingSymlinks);
+			const packStream = tar.pack(directory, {
+				dereference,
+				ignore: (name: string) =>
+					isExcluded(directory, name, options) || (dereference && danglingSet.has(relative(directory, name))),
+				map: (header) => {
+					if (header.type === 'directory') {
+						header.mode = 0o755;
+					}
+					return header;
+				},
+			});
+			if (onBytes) {
+				packStream.on('data', (chunk: Buffer) => onBytes(chunk.length));
+			}
+			// pipeline (rather than a bare .pipe()) destroys both streams on either side
+			// erroring or the consumer aborting mid-stream, so an aborted upload doesn't leave
+			// the directory walk running in the background. The callback is a no-op: pipeline
+			// already surfaces the failure by destroying `gzip` with the error itself.
+			pipeline(packStream, gzip, () => {});
+		})
+		.catch((err) => gzip.destroy(err));
+	return gzip;
+}
+
+/**
+ * Total uncompressed size of the files `streamPackagedDirectory` would include with the same
+ * options. Used by the CLI to give the upload progress bar a realistic total. The uncompressed
+ * size won't equal the gzipped wire size, but it gives the bar a steady trajectory: the bar
+ * moves as bytes are sent and snaps to 100% when the upload finishes.
+ */
+export async function getPackagedDirectorySize(
+	directory: string,
+	options: PackageOptions = DEFAULT_OPTIONS
+): Promise<number> {
+	return (await scanPackageDirectory(directory, options)).totalSize;
+}
+
+/**
+ * Relative paths of dangling symlinks that `streamPackagedDirectory` would skip. The CLI uses
+ * this to warn the user before deploy, since a dangling link means its intended content is
+ * absent from the package (and, before the skip guard, would have silently truncated the whole
+ * tarball). Returns `[]` when `skip_symlinks` is set, because links are then packed literally
+ * with no dereference.
+ */
+export async function findDanglingSymlinks(
+	directory: string,
+	options: PackageOptions = DEFAULT_OPTIONS
+): Promise<string[]> {
+	return (await scanPackageDirectory(directory, options)).danglingSymlinks;
 }
 
 /**

unitTests/bin/cliOperations.test.js

@@ -102,18 +102,18 @@ describe('cliOperations', () => {
 	describe('deploy_component cross-version compatibility', () => {
 		const target = 'https://example.com:9925/';
 		let originalPackageDirectory;
-		let originalGetSize;
+		let originalScan;
 
 		beforeEach(() => {
 			saveCredentials(target, { operation_token: 'valid-token', refresh_token: 'refresh-token' });
 			tokenAuthModule.isJWTExpired = () => false;
 			originalPackageDirectory = packageComponentModule.packageDirectory;
-			originalGetSize = packageComponentModule.getPackagedDirectorySize;
+			originalScan = packageComponentModule.scanPackageDirectory;
 		});
 
 		afterEach(() => {
 			packageComponentModule.packageDirectory = originalPackageDirectory;
-			packageComponentModule.getPackagedDirectorySize = originalGetSize;
+			packageComponentModule.scanPackageDirectory = originalScan;
 		});
 
 		// Streams an SSE `done` event so the modern (>= 5.1) deploy path can read its result.
@@ -155,7 +155,10 @@ describe('cliOperations', () => {
 
 		it('downgrades a directory deploy to a CBOR binary payload when the target is < 5.1', async () => {
 			const fakeTarball = Buffer.from('fake-tarball-bytes');
-			packageComponentModule.getPackagedDirectorySize = async () => fakeTarball.length;
+			packageComponentModule.scanPackageDirectory = async () => ({
+				totalSize: fakeTarball.length,
+				danglingSymlinks: [],
+			});
 			packageComponentModule.packageDirectory = async () => fakeTarball;
 
 			const calls = [];