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/4] 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/4] 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 }, From 9d97256b344349da0a8218cb369c83e0892f0625 Mon Sep 17 00:00:00 2001 From: Vincent De Smet Date: Sat, 11 Jul 2026 17:25:39 +0800 Subject: [PATCH 3/4] feat(lib): canonical asset hashes behind the canonicalAssetHashes flag Introduces the canonical entry-framed asset hash from #322 as an opt-in FUTURE_FLAGS entry (on for new projects, opt-in for existing ones, default at the next major). The scheme is modeled on git trees and Nix NAR and frames everything that affects the emitted artifact: - files: 'F NULNUL' + content, where mode is the octal permission mask archiveSync preserves in zip external attributes - symlinks: 'L NULNUL' + target - directories: 'D NUL' including empty ones (copySync materializes them); no mode, since neither emitter preserves directory permissions Entries are framed in sorted order with /-separated relative paths, so renames, entry-boundary shifts, permission changes, empty-directory changes, file-vs-symlink swaps, and symlink retargeting all change the hash -- closing the metadata hole identified in review (0644 -> 0755 previously changed archive bytes but not the hash). The legacy scheme remains the default for existing projects and is untouched by the flag. Closes #322 Co-Authored-By: Claude Fable 5 --- packages/cdktn/src/features.ts | 19 +++ packages/cdktn/src/private/fs.ts | 100 +++++++++-- packages/cdktn/src/terraform-asset.ts | 7 +- packages/cdktn/src/terraform-module-asset.ts | 7 +- .../cdktn/test/canonical-asset-hash.test.ts | 155 ++++++++++++++++++ 5 files changed, 275 insertions(+), 13 deletions(-) create mode 100644 packages/cdktn/test/canonical-asset-hash.test.ts diff --git a/packages/cdktn/src/features.ts b/packages/cdktn/src/features.ts index 8956a8004..14cb5ec74 100644 --- a/packages/cdktn/src/features.ts +++ b/packages/cdktn/src/features.ts @@ -38,7 +38,26 @@ export const FAIL_ON_CONSTRUCTS_OUTSIDE_OF_STACKS = */ export const VALIDATE_FUNCTION_VERSIONS = "validateFunctionVersions"; +/** + * When enabled, TerraformAsset and TerraformModuleAsset compute asset hashes + * with a canonical entry-framed scheme modeled on git trees and Nix NAR: + * every entry contributes its type, permission mask (for files and + * symlinks — the bits archiveSync preserves in zip external attributes), + * relative path, payload size, and payload, in sorted order, and + * directories — including empty ones — contribute explicit records. Renames, + * entry-boundary shifts, mode changes, empty-directory changes, and + * file-vs-symlink swaps all change the hash. + * + * When disabled, the legacy content-concatenation hash is kept for + * compatibility: trees without symlinks hash byte-identically to earlier + * releases, and only symlink-bearing trees (whose hashes had to change when + * symlink following was fixed) get a tagged hash that separates symlink + * metadata from file content. + */ +export const CANONICAL_ASSET_HASHES = "canonicalAssetHashes"; + export const FUTURE_FLAGS = { [FAIL_ON_CONSTRUCTS_OUTSIDE_OF_STACKS]: "true", [VALIDATE_FUNCTION_VERSIONS]: "true", + [CANONICAL_ASSET_HASHES]: "true", }; diff --git a/packages/cdktn/src/private/fs.ts b/packages/cdktn/src/private/fs.ts index 37932e781..b527a9c45 100644 --- a/packages/cdktn/src/private/fs.ts +++ b/packages/cdktn/src/private/fs.ts @@ -115,20 +115,43 @@ export function archiveSync(src: string, dest: string) { } } +export interface HashPathOptions { + /** + * Use the canonical entry-framed hash instead of the legacy + * content-concatenation hash. Enabled through the `canonicalAssetHashes` + * feature flag. + */ + readonly canonical?: boolean; +} + /** * Compute a stable MD5 hash of a file or directory's contents. - * 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`. + * In both schemes symlinks are hashed by their metadata (path + target) + * instead of being followed, so shared targets are not double-counted and + * cycles cannot recurse; a symlink at the root itself is followed, matching + * how the asset source path is opened when the artifact is emitted. * @param src - path to a file or directory to hash + * @param options - hash scheme selection, see {@link HashPathOptions} * @returns uppercased hex digest, truncated to HASH_LEN characters */ -export function hashPath(src: string): string { +export function hashPath(src: string, options: HashPathOptions = {}): string { + const digest = options.canonical + ? canonicalHashPath(src) + : legacyHashPath(src); + return digest.slice(0, HASH_LEN).toUpperCase(); +} + +/** + * Legacy-compatible hash: file contents fold into one digest in + * directory-listing order, exactly as earlier releases did, so trees without + * symlinks keep their historical hashes. Symlink 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 + */ +function legacyHashPath(src: string): string { const content = crypto.createHash("md5"); const links = crypto.createHash("md5"); let linkCount = 0; @@ -160,13 +183,68 @@ export function hashPath(src: string): string { hashRecursion(src, "", true); if (linkCount === 0) { - return content.digest("hex").slice(0, HASH_LEN).toUpperCase(); + return content.digest("hex"); } 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(); + return outer.digest("hex"); +} + +/** + * Canonical hash, modeled on git trees and Nix NAR: every entry is framed + * with everything that affects the emitted artifact — + * + * - files: `F \0\0` + content, where `` is + * the octal permission mask (`0o7777` bits) that archiveSync + * preserves in zip external attributes, + * - symlinks: `L \0\0` + target, + * - directories (including empty ones, which copySync materializes): + * `D \0`, with no mode — neither archiveSync nor + * copySync preserves directory permissions, + * + * in sorted directory order with `/`-separated relative paths, so renames, + * entry-boundary shifts, permission changes, empty-directory changes, and + * file-vs-symlink swaps all change the digest. + * @param src - path to a file or directory to hash + */ +function canonicalHashPath(src: string): string { + const hash = crypto.createHash("md5"); + + /** + * Walk `p`, framing each entry into the enclosing hash accumulator. + * @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 the artifact is emitted + */ + function hashRecursion(p: string, relPath: string, isRoot = false) { + const stat = isRoot ? fs.statSync(p) : fs.lstatSync(p); + const mode = (stat.mode & PERM_MASK).toString(8); + if (stat.isSymbolicLink()) { + const target = fs.readlinkSync(p); + hash.update(`L ${mode} ${relPath}\0${Buffer.byteLength(target)}\0`); + hash.update(target); + } else if (stat.isFile()) { + const data = fs.readFileSync(p); + hash.update(`F ${mode} ${relPath}\0${data.length}\0`); + hash.update(data); + } else if (stat.isDirectory()) { + if (relPath) { + hash.update(`D ${relPath}\0`); + } + for (const filename of fs.readdirSync(p).sort()) { + hashRecursion( + path.resolve(p, filename), + relPath ? `${relPath}/${filename}` : filename, + ); + } + } + } + + hashRecursion(src, "", true); + return hash.digest("hex"); } /** diff --git a/packages/cdktn/src/terraform-asset.ts b/packages/cdktn/src/terraform-asset.ts index c3b94b755..6e89d79f0 100644 --- a/packages/cdktn/src/terraform-asset.ts +++ b/packages/cdktn/src/terraform-asset.ts @@ -9,6 +9,7 @@ import { hashPath, findFileAboveCwd, } from "./private/fs"; +import { CANONICAL_ASSET_HASHES } from "./features"; import { ISynthesisSession } from "./synthesize"; import { addCustomSynthesis } from "./synthesize/synthesizer"; import { TerraformStack } from "./terraform-stack"; @@ -78,7 +79,11 @@ export class TerraformAsset extends Construct { const stat = fs.statSync(this.sourcePath); const inferredType = stat.isFile() ? AssetType.FILE : AssetType.DIRECTORY; this.type = config.type ?? inferredType; - this.assetHash = config.assetHash || hashPath(this.sourcePath); + this.assetHash = + config.assetHash || + hashPath(this.sourcePath, { + canonical: !!this.node.tryGetContext(CANONICAL_ASSET_HASHES), + }); if (stat.isFile() && this.type !== AssetType.FILE) { throw assetExpectsDirectory(id, config.path); diff --git a/packages/cdktn/src/terraform-module-asset.ts b/packages/cdktn/src/terraform-module-asset.ts index b4b38c741..2f852b07d 100644 --- a/packages/cdktn/src/terraform-module-asset.ts +++ b/packages/cdktn/src/terraform-module-asset.ts @@ -10,6 +10,7 @@ import * as fs from "fs"; import { TerraformStack } from "./terraform-stack"; import { AssetType, TerraformAsset } from "./terraform-asset"; import { copySync, hashPath } from "./private/fs"; +import { CANONICAL_ASSET_HASHES } from "./features"; const TERRAFORM_MODULE_ASSET_SYMBOL = Symbol.for("cdktf.TerraformModuleAsset"); @@ -69,7 +70,11 @@ export class TerraformModuleAsset extends Construct { this.asset = new TerraformAsset(this, "asset", { path: tmpDir, type: AssetType.DIRECTORY, - assetHash: staticModuleAssetHash ?? hashPath(relativeAssetPath), + assetHash: + staticModuleAssetHash ?? + hashPath(relativeAssetPath, { + canonical: !!this.node.tryGetContext(CANONICAL_ASSET_HASHES), + }), }); } diff --git a/packages/cdktn/test/canonical-asset-hash.test.ts b/packages/cdktn/test/canonical-asset-hash.test.ts new file mode 100644 index 000000000..7fc42f1f5 --- /dev/null +++ b/packages/cdktn/test/canonical-asset-hash.test.ts @@ -0,0 +1,155 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +// Acceptance tests for the canonicalAssetHashes feature flag +// (https://github.com/open-constructs/cdk-terrain/issues/322) +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { Testing, TerraformStack, TerraformAsset, AssetType } from "../src"; +import { CANONICAL_ASSET_HASHES } from "../src/features"; +import { hashPath } from "../src/private/fs"; + +function createTempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-canonical-test-")); +} + +const canonical = (p: string) => hashPath(p, { canonical: true }); + +describe("canonical asset hash scheme", () => { + let srcDir: string; + + beforeEach(() => { + srcDir = createTempDir(); + }); + + afterEach(() => { + fs.rmSync(srcDir, { recursive: true, force: true }); + }); + + test("renaming a file changes the hash (legacy scheme cannot see renames)", () => { + fs.writeFileSync(path.join(srcDir, "a.txt"), "content"); + const before = { legacy: hashPath(srcDir), canonical: canonical(srcDir) }; + + fs.renameSync(path.join(srcDir, "a.txt"), path.join(srcDir, "b.txt")); + + expect(hashPath(srcDir)).toBe(before.legacy); + expect(canonical(srcDir)).not.toBe(before.canonical); + }); + + test("shifting bytes across entry boundaries changes the hash", () => { + fs.writeFileSync(path.join(srcDir, "a.txt"), "x"); + fs.writeFileSync(path.join(srcDir, "b.txt"), "yz"); + const before = canonical(srcDir); + + fs.writeFileSync(path.join(srcDir, "a.txt"), "xy"); + fs.writeFileSync(path.join(srcDir, "b.txt"), "z"); + + expect(canonical(srcDir)).not.toBe(before); + }); + + test("changing file permissions changes the hash (archiveSync emits them)", () => { + const file = path.join(srcDir, "run.sh"); + fs.writeFileSync(file, "#!/bin/sh\n"); + fs.chmodSync(file, 0o644); + const before = canonical(srcDir); + + fs.chmodSync(file, 0o755); + + expect(canonical(srcDir)).not.toBe(before); + }); + + test("adding or removing an empty directory changes the hash (copySync emits them)", () => { + fs.writeFileSync(path.join(srcDir, "a.txt"), "content"); + const before = canonical(srcDir); + + fs.mkdirSync(path.join(srcDir, "empty")); + const withDir = canonical(srcDir); + expect(withDir).not.toBe(before); + + fs.rmdirSync(path.join(srcDir, "empty")); + expect(canonical(srcDir)).toBe(before); + }); + + test("a regular file and a symlink with the same payload hash differently", () => { + const asFile = createTempDir(); + fs.writeFileSync(path.join(asFile, "current"), "a.txt"); + const asLink = createTempDir(); + fs.symlinkSync("a.txt", path.join(asLink, "current")); + + expect(canonical(asFile)).not.toBe(canonical(asLink)); + + fs.rmSync(asFile, { recursive: true, force: true }); + fs.rmSync(asLink, { recursive: true, force: true }); + }); + + test("does not crash on circular symlinks and sees retargeting", () => { + fs.mkdirSync(path.join(srcDir, "pkg")); + fs.writeFileSync(path.join(srcDir, "pkg", "index.js"), "module"); + fs.symlinkSync("../pkg", path.join(srcDir, "pkg", "self")); + const before = canonical(srcDir); + + fs.rmSync(path.join(srcDir, "pkg", "self")); + fs.symlinkSync("index.js", path.join(srcDir, "pkg", "self")); + + expect(canonical(srcDir)).not.toBe(before); + }); + + test("identical logical trees hash identically regardless of location", () => { + const build = (root: string) => { + fs.mkdirSync(path.join(root, "sub")); + fs.mkdirSync(path.join(root, "empty")); + const file = path.join(root, "sub", "a.txt"); + fs.writeFileSync(file, "content"); + fs.chmodSync(file, 0o644); + fs.symlinkSync("sub/a.txt", path.join(root, "link")); + }; + const otherDir = createTempDir(); + build(srcDir); + build(otherDir); + + expect(canonical(srcDir)).toBe(canonical(otherDir)); + // repeated runs over the same tree are stable + expect(canonical(srcDir)).toBe(canonical(srcDir)); + + fs.rmSync(otherDir, { recursive: true, force: true }); + }); +}); + +describe("TerraformAsset with the canonicalAssetHashes flag", () => { + let srcDir: string; + + beforeEach(() => { + srcDir = createTempDir(); + fs.writeFileSync(path.join(srcDir, "a.txt"), "content"); + }); + + afterEach(() => { + fs.rmSync(srcDir, { recursive: true, force: true }); + }); + + test("flag selects the hash scheme", () => { + // Testing.app() enables FUTURE_FLAGS by default; the off-case models an + // existing project that has not opted in + const stackOff = new TerraformStack( + Testing.app({ enableFutureFlags: false }), + "off", + ); + const assetOff = new TerraformAsset(stackOff, "asset", { + path: srcDir, + type: AssetType.DIRECTORY, + }); + + const stackOn = new TerraformStack( + Testing.app({ context: { [CANONICAL_ASSET_HASHES]: "true" } }), + "on", + ); + const assetOn = new TerraformAsset(stackOn, "asset", { + path: srcDir, + type: AssetType.DIRECTORY, + }); + + expect(assetOff.assetHash).toBe(hashPath(srcDir)); + expect(assetOn.assetHash).toBe(hashPath(srcDir, { canonical: true })); + expect(assetOff.assetHash).not.toBe(assetOn.assetHash); + }); +}); From 04555976642969ade559f358470f8eaf241bdb5f Mon Sep 17 00:00:00 2001 From: Vincent De Smet Date: Sat, 11 Jul 2026 17:27:16 +0800 Subject: [PATCH 4/4] chore(docs): document feature flags and the canonicalAssetHashes scheme Adds docs/feature-flags.md covering the FUTURE_FLAGS lifecycle and the canonicalAssetHashes flag from #323: why the legacy hash is blind to renames, entry boundaries, permissions, and empty directories; how the canonical representation frames the complete metadata model (the git-tree/Nix-NAR lesson from #322 -- payload framing alone is not canonical); the one-time migration impact of opting in; and why early adoption ahead of the next major is recommended. Co-Authored-By: Claude Fable 5 --- docs/feature-flags.md | 122 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 docs/feature-flags.md diff --git a/docs/feature-flags.md b/docs/feature-flags.md new file mode 100644 index 000000000..a702d04cb --- /dev/null +++ b/docs/feature-flags.md @@ -0,0 +1,122 @@ +# Feature Flags + +CDK Terrain uses feature flags to introduce behavior we want as the future +default without breaking existing projects. Flags are plain context keys read +from `cdktf.json` (or the `App` constructor's `context` option): + +```json +{ + "context": { + "canonicalAssetHashes": "true" + } +} +``` + +The lifecycle, defined in +[`packages/cdktn/src/features.ts`](../packages/cdktn/src/features.ts): + +- `cdktn init` writes every current `FUTURE_FLAGS` entry into new projects' + `cdktf.json`, so **new projects always get the future behavior**. +- Existing projects keep the old behavior until they **opt in** by adding the + context key themselves. +- At the next major release the flagged behavior becomes the unconditional + default and the flag is removed. + +There are deliberately no opt-_out_ flags: once a behavior is the default in +a major version, the escape hatch is gone, which keeps the code paths from +accumulating. + +## Current flags + +| Flag | Behavior when enabled | +| --------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | +| `failOnConstructsOutsideOfStacks` | Synthesis throws if a construct is scoped directly to the `App` instead of a `TerraformStack`. | +| `validateFunctionVersions` | Each stack statically validates that `Fn` functions are supported by the runtime versions declared in `targetVersions`. | +| `canonicalAssetHashes` | `TerraformAsset` / `TerraformModuleAsset` hashes use the canonical entry-framed representation described below. | + +## `canonicalAssetHashes` + +Introduced in [#323](https://github.com/open-constructs/cdk-terrain/pull/323) +(design: [#322](https://github.com/open-constructs/cdk-terrain/issues/322)). + +### Why the legacy hash is not enough + +`TerraformAsset.assetHash` decides the asset's output path, so it must change +exactly when the emitted artifact changes. The legacy scheme concatenates +file contents into one digest, which makes it blind to several +artifact-affecting properties: + +- **Renames** — `a.txt` → `b.txt` with identical content hashes identically, + but the emitted archive/directory differs. +- **Entry boundaries** — files `("x", "yz")` and `("xy", "z")` hash + identically. +- **Permissions** — `chmod 0644` → `0755` changes the zip's external + attributes (and whether your Lambda binary is executable) but not the + legacy hash. +- **Empty directories** — added or removed, they are materialized by + directory assets yet invisible to the legacy hash. + +Symlink metadata was folded into the legacy scheme in +[#321](https://github.com/open-constructs/cdk-terrain/pull/321) under a +tagged domain (fixing +[#320](https://github.com/open-constructs/cdk-terrain/issues/320) without +changing hashes for symlink-free trees), but the blind spots above are +structural: they cannot be fixed without changing every asset hash, which is +why the complete scheme is behind a feature flag. + +### The canonical representation + +The canonical scheme is modeled on git tree objects and Nix NAR +serialization. Review of the first draft +([#322](https://github.com/open-constructs/cdk-terrain/issues/322)) made an +important observation: those formats are canonical **because they serialize a +complete metadata model, not payload framing alone** — git tree entries carry +a mode (`100644` / `100755` / `120000`) and represent directories as objects; +NAR records node types and an executable marker. Borrowing only the +`(type, path, payload)` framing re-introduces exactly the permission and +empty-directory blind spots listed above. + +The adopted representation therefore frames, per entry, everything that +affects the emitted artifact: + +| entry | record | +| ----------------------- | ---------------------------------------------------------- | +| file | `F \0\0` + content | +| symlink | `L \0\0` + link target | +| directory (incl. empty) | `D \0` | + +where: + +- `` is the octal permission mask (`0o7777` bits) — precisely the bits + `archiveSync` preserves in zip external attributes. Directory records carry + no mode because neither `archiveSync` nor `copySync` preserves directory + permissions; the hash only covers what is emitted. +- Records appear in sorted directory order with `/`-separated relative + paths, independent of platform directory-listing order. +- Symlinks contribute their metadata, never their target's content, so + circular symlinks cannot recurse and shared targets are not + double-counted. A symlink at the asset root itself is followed, matching + how the source path is opened when the artifact is emitted. + +### Migration impact + +Enabling the flag changes **every** asset hash once, which changes every +asset output path (`assets///…`) and re-uploads/re-deploys the +referencing resources (e.g. Lambda functions) on the next apply. Content is +unchanged; this is a one-time path migration. `TerraformModuleAsset` users +can pin hashes across the transition with the existing +`cdktfStaticModuleAssetHash` context key if needed. + +### Why adopting it early is recommended + +The canonical scheme becomes the unconditional default at the next major +release, and the legacy scheme is removed then. Opting in early means: + +- the one-time re-deploy happens on your schedule, not the major upgrade's; +- asset hashes become trustworthy change detectors — anything that alters + the emitted artifact (content, names, modes, tree shape, symlinks) alters + the hash, and nothing else does; +- combined with the pinned entry timestamps from + [#321](https://github.com/open-constructs/cdk-terrain/pull/321), archives + are byte-reproducible, so downstream hashing such as `filebase64sha256` + stops drifting.