From 08bc3dfa1b8cd034d39654074d8e5010fa48613a Mon Sep 17 00:00:00 2001 From: Vincent De Smet Date: Sat, 11 Jul 2026 15:00:34 +0800 Subject: [PATCH 1/2] fix(lib): preserve symlinks in TerraformAsset walkers archiveSync, hashPath, and copySync all walked with fs.statSync, which follows symlinks: shared targets were duplicated N times into archives (15 MB -> 34 MB on a real pnpm Lambda bundle), symlink entries were never emitted, and circular links (legitimate in pnpm node_modules) crashed synth with ELOOP -- for every asset type, since hashPath runs in the TerraformAsset constructor. Walk with lstatSync instead and treat symlinks first-class, restoring the behavior cdktf gets from archiver (dropped in #95 when archiver was replaced by yazl, kept broken through the fflate rewrite in #148): - archiveSync emits real symlink entries (readlink target as STORED data, S_IFLNK unix mode in the external attrs, host os Unix), now also preserves regular-file modes (executable bits were stripped), and pins entry mtimes to a fixed date so archives are byte-reproducible (fflate defaulted to Date.now() per synth) - hashPath hashes a symlink by its target path, so retargeting still changes the asset hash without following (or double-counting) links - copySync recreates symlinks for AssetType.DIRECTORY assets Fixes #320 Co-Authored-By: Claude Fable 5 --- packages/cdktn/src/private/fs.ts | 77 +++++-- packages/cdktn/test/archive-symlink.test.ts | 229 ++++++++++++++++++++ 2 files changed, 293 insertions(+), 13 deletions(-) create mode 100644 packages/cdktn/test/archive-symlink.test.ts diff --git a/packages/cdktn/src/private/fs.ts b/packages/cdktn/src/private/fs.ts index 561885b6e..847f55acd 100644 --- a/packages/cdktn/src/private/fs.ts +++ b/packages/cdktn/src/private/fs.ts @@ -3,11 +3,33 @@ import * as fs from "fs"; import * as path from "path"; import * as crypto from "crypto"; -import { zipSync } from "fflate"; +import { strToU8, zipSync } from "fflate"; +import type { ZipOptions } from "fflate"; import { assetCanNotCreateZipArchive } from "../errors"; const HASH_LEN = 32; +// Unix file-type bits for zip external attributes (st_mode upper nibble) +const S_IFLNK = 0o120000; +const S_IFREG = 0o100000; +const PERM_MASK = 0o7777; +// "version made by" host byte; unzip only honors unix mode attrs when set +const ZIP_OS_UNIX = 3; +// Pinned so archives are byte-reproducible across synths. fflate encodes +// DOS dates from local-time getters and rejects years outside 1980-2099, +// so this must be a local-time 1980 date (a UTC one underflows to 1979 in +// timezones west of UTC). +const ZIP_ENTRY_MTIME = new Date(1980, 0, 1); + +/** + * Zip external attributes for a unix mode: file-type and permission bits + * belong in the high 16 bits of the 32-bit field. + * @param mode - unix st_mode bits (type | permissions) + */ +function zipAttrs(mode: number): number { + return (mode << 16) >>> 0; +} + // Full implementation at https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/copy-sync.js /** * Copy a file or directory. The directory can have contents and subfolders. @@ -21,11 +43,12 @@ export function copySync(src: string, dest: string) { */ function copyItem(p: string) { const sourcePath = path.resolve(src, p); - const stat = fs.statSync(sourcePath); - if (stat.isFile()) { + const stat = fs.lstatSync(sourcePath); + if (stat.isSymbolicLink()) { + fs.symlinkSync(fs.readlinkSync(sourcePath), path.resolve(dest, p)); + } else if (stat.isFile()) { fs.copyFileSync(sourcePath, path.resolve(dest, p)); - } - if (stat.isDirectory()) { + } else if (stat.isDirectory()) { walkSubfolder(p); } } @@ -51,20 +74,42 @@ export function copySync(src: string, dest: string) { */ export function archiveSync(src: string, dest: string) { try { - const files: Record = {}; + const files: Record = {}; const walk = (dir: string, prefix: string) => { for (const entry of fs.readdirSync(dir)) { const full = path.join(dir, entry); const zipPath = prefix ? `${prefix}/${entry}` : entry; - if (fs.statSync(full).isDirectory()) { + const stat = fs.lstatSync(full); + if (stat.isSymbolicLink()) { + // Store the link target as the entry data with S_IFLNK attrs so + // extractors recreate the symlink instead of a copy of the target. + // Never recursing through links also makes cycles unreachable. + files[zipPath] = [ + strToU8(fs.readlinkSync(full)), + { + os: ZIP_OS_UNIX, + attrs: zipAttrs(S_IFLNK | (stat.mode & PERM_MASK)), + level: 0, + }, + ]; + } else if (stat.isDirectory()) { walk(full, zipPath); } else { - files[zipPath] = fs.readFileSync(full); + files[zipPath] = [ + fs.readFileSync(full), + { + os: ZIP_OS_UNIX, + attrs: zipAttrs(S_IFREG | (stat.mode & PERM_MASK)), + }, + ]; } } }; walk(src, ""); - fs.writeFileSync(dest, zipSync(files, { level: 9 })); + fs.writeFileSync( + dest, + zipSync(files, { level: 9, mtime: ZIP_ENTRY_MTIME }), + ); } catch (err: any) { throw assetCanNotCreateZipArchive(src, dest, err); } @@ -82,11 +127,17 @@ export function hashPath(src: string): string { /** * Walk `p`, feeding any file contents into the enclosing hash accumulator. + * Symlinks are hashed by their target path instead of being followed, so + * shared targets are not double-counted and cycles cannot recurse. * @param p - path to walk + * @param isRoot - follow a symlink only at the root, matching how the + * asset's own path is resolved when it is read */ - function hashRecursion(p: string) { - const stat = fs.statSync(p); - if (stat.isFile()) { + function hashRecursion(p: string, isRoot = false) { + const stat = isRoot ? fs.statSync(p) : fs.lstatSync(p); + if (stat.isSymbolicLink()) { + hash.update(fs.readlinkSync(p)); + } else if (stat.isFile()) { hash.update(fs.readFileSync(p)); } else if (stat.isDirectory()) { fs.readdirSync(p).forEach((filename) => @@ -95,7 +146,7 @@ export function hashPath(src: string): string { } } - hashRecursion(src); + hashRecursion(src, true); return hash.digest("hex").slice(0, HASH_LEN).toUpperCase(); } diff --git a/packages/cdktn/test/archive-symlink.test.ts b/packages/cdktn/test/archive-symlink.test.ts new file mode 100644 index 000000000..796d4888e --- /dev/null +++ b/packages/cdktn/test/archive-symlink.test.ts @@ -0,0 +1,229 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +// Repros for https://github.com/open-constructs/cdk-terrain/issues/320 +// AssetType.ARCHIVE follows symlinks: content is duplicated per link path, +// no symlink entries are preserved, and circular links crash synth (ELOOP). +import * as crypto from "crypto"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { execSync } from "child_process"; +import { archiveSync, copySync, hashPath } from "../src/private/fs"; + +function createTempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-symlink-test-")); +} + +/** + * Extracts a zip with the system `unzip`, which restores entries carrying + * unix S_IFLNK mode bits as real symlinks on disk. + */ +function extractZip(zipPath: string): string { + const extractDir = createTempDir(); + execSync(`unzip -o "${zipPath}" -d "${extractDir}"`, { stdio: "ignore" }); + return extractDir; +} + +describe("archiveSync symlink handling (#320)", () => { + let srcDir: string; + let destDir: string; + + beforeEach(() => { + srcDir = createTempDir(); + destDir = createTempDir(); + }); + + afterEach(() => { + fs.rmSync(srcDir, { recursive: true, force: true }); + fs.rmSync(destDir, { recursive: true, force: true }); + }); + + test("preserves a file symlink as a symlink entry", () => { + fs.writeFileSync(path.join(srcDir, "file.txt"), "content"); + fs.symlinkSync("file.txt", path.join(srcDir, "alias.txt")); + + const zipPath = path.join(destDir, "output.zip"); + archiveSync(srcDir, zipPath); + + const extracted = extractZip(zipPath); + const alias = path.join(extracted, "alias.txt"); + expect(fs.lstatSync(alias).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(alias)).toBe("file.txt"); + expect(fs.readFileSync(alias, "utf-8")).toBe("content"); + }); + + test("preserves directory symlinks instead of recursing into the target", () => { + // pnpm-style layout: two links share one real package + fs.mkdirSync(path.join(srcDir, "real-pkg")); + fs.writeFileSync(path.join(srcDir, "real-pkg", "index.js"), "module"); + fs.symlinkSync("real-pkg", path.join(srcDir, "link-a")); + fs.symlinkSync("real-pkg", path.join(srcDir, "link-b")); + + const zipPath = path.join(destDir, "output.zip"); + archiveSync(srcDir, zipPath); + + const extracted = extractZip(zipPath); + expect(fs.lstatSync(path.join(extracted, "link-a")).isSymbolicLink()).toBe( + true, + ); + expect(fs.readlinkSync(path.join(extracted, "link-a"))).toBe("real-pkg"); + // the target's content must exist exactly once, under its real path + expect( + fs.readFileSync(path.join(extracted, "real-pkg", "index.js"), "utf-8"), + ).toBe("module"); + // and NOT as a materialized copy beneath the link paths + expect(fs.lstatSync(path.join(extracted, "link-a")).isDirectory()).toBe( + false, + ); + }); + + test("does not duplicate shared symlink targets into the archive", () => { + // incompressible payload so duplication shows up in the zip size + const payload = crypto.randomBytes(200 * 1024); + fs.mkdirSync(path.join(srcDir, "real-pkg")); + fs.writeFileSync(path.join(srcDir, "real-pkg", "blob.bin"), payload); + fs.symlinkSync("real-pkg", path.join(srcDir, "link-a")); + fs.symlinkSync("real-pkg", path.join(srcDir, "link-b")); + + const zipPath = path.join(destDir, "output.zip"); + archiveSync(srcDir, zipPath); + + // one stored copy ≈ 200 KiB; the buggy walk stores three (~600 KiB) + expect(fs.statSync(zipPath).size).toBeLessThan(250 * 1024); + }); + + test("does not crash on circular symlinks (ELOOP)", () => { + // pnpm trees legitimately contain self-referential links + fs.mkdirSync(path.join(srcDir, "pkg")); + fs.writeFileSync(path.join(srcDir, "pkg", "index.js"), "module"); + fs.symlinkSync("../pkg", path.join(srcDir, "pkg", "self")); + + const zipPath = path.join(destDir, "output.zip"); + expect(() => archiveSync(srcDir, zipPath)).not.toThrow(); + + const extracted = extractZip(zipPath); + expect( + fs.lstatSync(path.join(extracted, "pkg", "self")).isSymbolicLink(), + ).toBe(true); + }); +}); + +describe("hashPath symlink handling (#320)", () => { + let srcDir: string; + + beforeEach(() => { + srcDir = createTempDir(); + }); + + afterEach(() => { + fs.rmSync(srcDir, { recursive: true, force: true }); + }); + + test("does not crash on circular symlinks (ELOOP)", () => { + // hashPath runs in the TerraformAsset constructor for EVERY asset type, + // so this crash hits before archiving even starts + fs.mkdirSync(path.join(srcDir, "pkg")); + fs.writeFileSync(path.join(srcDir, "pkg", "index.js"), "module"); + fs.symlinkSync("../pkg", path.join(srcDir, "pkg", "self")); + + expect(() => hashPath(srcDir)).not.toThrow(); + }); + + test("hash changes when a symlink is retargeted", () => { + fs.writeFileSync(path.join(srcDir, "a.txt"), "aaa"); + fs.writeFileSync(path.join(srcDir, "b.txt"), "bbb"); + fs.symlinkSync("a.txt", path.join(srcDir, "current")); + const before = hashPath(srcDir); + + fs.rmSync(path.join(srcDir, "current")); + fs.symlinkSync("b.txt", path.join(srcDir, "current")); + const after = hashPath(srcDir); + + expect(before).not.toBe(after); + }); +}); + +describe("copySync symlink handling (#320)", () => { + let srcDir: string; + let destDir: string; + + beforeEach(() => { + srcDir = createTempDir(); + destDir = createTempDir(); + }); + + afterEach(() => { + fs.rmSync(srcDir, { recursive: true, force: true }); + fs.rmSync(destDir, { recursive: true, force: true }); + }); + + test("recreates symlinks instead of copying their targets", () => { + fs.mkdirSync(path.join(srcDir, "real-pkg")); + fs.writeFileSync(path.join(srcDir, "real-pkg", "index.js"), "module"); + fs.symlinkSync("real-pkg", path.join(srcDir, "link-a")); + + copySync(srcDir, destDir); + + const link = path.join(destDir, "link-a"); + expect(fs.lstatSync(link).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(link)).toBe("real-pkg"); + expect(fs.readFileSync(path.join(link, "index.js"), "utf-8")).toBe( + "module", + ); + }); + + test("does not crash on circular symlinks (ELOOP)", () => { + fs.mkdirSync(path.join(srcDir, "pkg")); + fs.writeFileSync(path.join(srcDir, "pkg", "index.js"), "module"); + fs.symlinkSync("../pkg", path.join(srcDir, "pkg", "self")); + + expect(() => copySync(srcDir, destDir)).not.toThrow(); + expect( + fs.lstatSync(path.join(destDir, "pkg", "self")).isSymbolicLink(), + ).toBe(true); + }); +}); + +describe("archiveSync zip metadata (#320 follow-ups)", () => { + let srcDir: string; + let destDir: string; + + beforeEach(() => { + srcDir = createTempDir(); + destDir = createTempDir(); + }); + + afterEach(() => { + fs.rmSync(srcDir, { recursive: true, force: true }); + fs.rmSync(destDir, { recursive: true, force: true }); + }); + + test("preserves the executable bit on regular files", () => { + const script = path.join(srcDir, "run.sh"); + fs.writeFileSync(script, "#!/bin/sh\necho hi\n"); + fs.chmodSync(script, 0o755); + + const zipPath = path.join(destDir, "output.zip"); + archiveSync(srcDir, zipPath); + + const extracted = extractZip(zipPath); + const mode = fs.statSync(path.join(extracted, "run.sh")).mode; + expect(mode & 0o111).not.toBe(0); + }); + + test("produces byte-identical archives across runs", async () => { + fs.mkdirSync(path.join(srcDir, "sub")); + fs.writeFileSync(path.join(srcDir, "a.txt"), "aaa"); + fs.writeFileSync(path.join(srcDir, "sub", "b.txt"), "bbb"); + fs.symlinkSync("a.txt", path.join(srcDir, "link")); + + const first = path.join(destDir, "first.zip"); + const second = path.join(destDir, "second.zip"); + archiveSync(srcDir, first); + // entry mtimes are pinned, so wall-clock time must not leak into bytes + await new Promise((resolve) => setTimeout(resolve, 1100)); + archiveSync(srcDir, second); + + expect(fs.readFileSync(first).equals(fs.readFileSync(second))).toBe(true); + }); +}); From ff817b2e6bd3843f1425a261b164a1997454ef1d Mon Sep 17 00:00:00 2001 From: Vincent De Smet Date: Sat, 11 Jul 2026 17:22:01 +0800 Subject: [PATCH 2/2] fix(lib): frame symlink metadata in asset hashes, share the symlink-aware copier Review follow-ups on #321, downscoped per review to the compatibility-preserving hash only (canonical hashing moved to #322): - hashPath fed symlink targets and file contents into one unframed stream, so a file containing 'foo' and a symlink targeting 'foo' could produce the same assetHash for materially different artifacts. The scheme now keeps the legacy content hash byte-identical for symlink-free trees and, only when symlinks exist, combines it with separately-framed symlink metadata (relative path + target) under a tagged outer hash. - TerraformModuleAsset dropped its private copier, which routed directory symlinks into copyFileSync (EISDIR) before the fixed walkers could run; it now shares the symlink-aware copySync. Co-Authored-By: Claude Fable 5 --- packages/cdktn/src/private/fs.ts | 43 +++++++++++++------ packages/cdktn/src/terraform-module-asset.ts | 18 +------- packages/cdktn/test/archive-symlink.test.ts | 27 ++++++++++++ .../cdktn/test/terraform-module-asset.test.ts | 40 ++++++++++++++++- 4 files changed, 98 insertions(+), 30 deletions(-) diff --git a/packages/cdktn/src/private/fs.ts b/packages/cdktn/src/private/fs.ts index 847f55acd..37932e781 100644 --- a/packages/cdktn/src/private/fs.ts +++ b/packages/cdktn/src/private/fs.ts @@ -117,37 +117,56 @@ export function archiveSync(src: string, dest: string) { /** * Compute a stable MD5 hash of a file or directory's contents. - * Directories are hashed by recursively folding each file's contents into - * the same digest, in directory-listing order. + * File contents fold into one digest in directory-listing order, exactly as + * earlier releases did, so trees without symlinks keep their historical + * hashes. Symlinks are hashed by their metadata (path + target) instead of + * being followed, so shared targets are not double-counted and cycles cannot + * recurse; that metadata goes into a second digest, and only when symlinks + * exist are the two combined under a tagged outer hash — the tag keeps + * symlink metadata in a separate domain from file bytes, so a file + * containing `foo` can never collide with a symlink targeting `foo`. * @param src - path to a file or directory to hash * @returns uppercased hex digest, truncated to HASH_LEN characters */ export function hashPath(src: string): string { - const hash = crypto.createHash("md5"); + const content = crypto.createHash("md5"); + const links = crypto.createHash("md5"); + let linkCount = 0; /** - * Walk `p`, feeding any file contents into the enclosing hash accumulator. - * Symlinks are hashed by their target path instead of being followed, so - * shared targets are not double-counted and cycles cannot recurse. + * Walk `p`, feeding file contents and symlink metadata into the enclosing + * accumulators. * @param p - path to walk + * @param relPath - path of `p` relative to the walk root, `/`-separated * @param isRoot - follow a symlink only at the root, matching how the * asset's own path is resolved when it is read */ - function hashRecursion(p: string, isRoot = false) { + function hashRecursion(p: string, relPath: string, isRoot = false) { const stat = isRoot ? fs.statSync(p) : fs.lstatSync(p); if (stat.isSymbolicLink()) { - hash.update(fs.readlinkSync(p)); + links.update(`${relPath}\0${fs.readlinkSync(p)}\0`); + linkCount++; } else if (stat.isFile()) { - hash.update(fs.readFileSync(p)); + content.update(fs.readFileSync(p)); } else if (stat.isDirectory()) { fs.readdirSync(p).forEach((filename) => - hashRecursion(path.resolve(p, filename)), + hashRecursion( + path.resolve(p, filename), + relPath ? `${relPath}/${filename}` : filename, + ), ); } } - hashRecursion(src, true); - return hash.digest("hex").slice(0, HASH_LEN).toUpperCase(); + hashRecursion(src, "", true); + if (linkCount === 0) { + return content.digest("hex").slice(0, HASH_LEN).toUpperCase(); + } + const outer = crypto.createHash("md5"); + outer.update("cdktn/asset-hash/symlinks/v1\0"); + outer.update(content.digest("hex")); + outer.update(links.digest("hex")); + return outer.digest("hex").slice(0, HASH_LEN).toUpperCase(); } /** diff --git a/packages/cdktn/src/terraform-module-asset.ts b/packages/cdktn/src/terraform-module-asset.ts index db13e000d..b4b38c741 100644 --- a/packages/cdktn/src/terraform-module-asset.ts +++ b/packages/cdktn/src/terraform-module-asset.ts @@ -9,7 +9,7 @@ import * as os from "os"; import * as fs from "fs"; import { TerraformStack } from "./terraform-stack"; import { AssetType, TerraformAsset } from "./terraform-asset"; -import { hashPath } from "./private/fs"; +import { copySync, hashPath } from "./private/fs"; const TERRAFORM_MODULE_ASSET_SYMBOL = Symbol.for("cdktf.TerraformModuleAsset"); @@ -127,19 +127,3 @@ export function findLowestCommonPath(paths: string[]): string | undefined { const relativePath = path.relative(process.cwd(), absolutePathPrefix); return relativePath === "" ? "." : relativePath; } - -/** - * Copies a file or directory recursively - * @param from - * @param to - */ -function copySync(from: string, to: string) { - if (fs.lstatSync(from).isDirectory()) { - fs.mkdirSync(to, { recursive: true }); - for (const file of fs.readdirSync(from)) { - copySync(path.join(from, file), path.join(to, file)); - } - } else { - fs.copyFileSync(from, to); - } -} diff --git a/packages/cdktn/test/archive-symlink.test.ts b/packages/cdktn/test/archive-symlink.test.ts index 796d4888e..838f0f6b0 100644 --- a/packages/cdktn/test/archive-symlink.test.ts +++ b/packages/cdktn/test/archive-symlink.test.ts @@ -141,6 +141,33 @@ describe("hashPath symlink handling (#320)", () => { expect(before).not.toBe(after); }); + + test("a regular file and a symlink with the same payload hash differently", () => { + // both trees contain a `current` entry whose payload bytes are `a.txt` + const asFile = createTempDir(); + fs.writeFileSync(path.join(asFile, "current"), "a.txt"); + const asLink = createTempDir(); + fs.symlinkSync("a.txt", path.join(asLink, "current")); + + expect(hashPath(asFile)).not.toBe(hashPath(asLink)); + + fs.rmSync(asFile, { recursive: true, force: true }); + fs.rmSync(asLink, { recursive: true, force: true }); + }); + + test("trees without symlinks keep their legacy hash byte-for-byte", () => { + fs.mkdirSync(path.join(srcDir, "sub")); + fs.writeFileSync(path.join(srcDir, "a.txt"), "aaa"); + fs.writeFileSync(path.join(srcDir, "sub", "b.txt"), "bbb"); + + // the historical scheme: file contents folded into one md5 in + // directory-listing order, uppercased + const legacy = crypto.createHash("md5"); + legacy.update("aaa"); + legacy.update("bbb"); + + expect(hashPath(srcDir)).toBe(legacy.digest("hex").toUpperCase()); + }); }); describe("copySync symlink handling (#320)", () => { diff --git a/packages/cdktn/test/terraform-module-asset.test.ts b/packages/cdktn/test/terraform-module-asset.test.ts index 303b0fc07..e7c09250d 100644 --- a/packages/cdktn/test/terraform-module-asset.test.ts +++ b/packages/cdktn/test/terraform-module-asset.test.ts @@ -3,9 +3,47 @@ * SPDX-License-Identifier: MPL-2.0 */ -import { findLowestCommonPath } from "../src/terraform-module-asset"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { Testing } from "../src/testing"; +import { TerraformStack } from "../src/terraform-stack"; +import { + findLowestCommonPath, + TerraformModuleAsset, +} from "../src/terraform-module-asset"; describe("TerraformModuleAsset", () => { + describe("module source copying (#320)", () => { + let moduleDir: string; + + beforeEach(() => { + moduleDir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-module-")); + fs.writeFileSync(path.join(moduleDir, "main.tf"), ""); + fs.mkdirSync(path.join(moduleDir, "shared")); + fs.writeFileSync(path.join(moduleDir, "shared", "vars.tf"), ""); + // a directory symlink previously hit copyFileSync -> EISDIR + fs.symlinkSync("shared", path.join(moduleDir, "link")); + }); + + afterEach(() => { + fs.rmSync(moduleDir, { recursive: true, force: true }); + }); + + it("copies module sources containing directory symlinks", () => { + const app = Testing.app({ + context: { + cdktfRelativeModules: [path.relative(process.cwd(), moduleDir)], + }, + }); + const stack = new TerraformStack(app, "stack"); + + expect( + () => new TerraformModuleAsset(stack, "module-asset"), + ).not.toThrow(); + }); + }); + describe("findLowestCommonPath", () => { it.each([ { paths: [], expected: undefined },