fix(lib)!: preserve symlinks in TerraformAsset walkers

[vincenthsh]

Description

Closes #320.

TerraformAsset walked source trees with fs.statSync, which follows symlinks. On symlinked trees (e.g. pnpm's node_modules) this meant:

• Archive bloat: a target reachable through N symlink paths was copied N times into the zip (15 MB → 34 MB on a real Lambda bundle for byte-identical logical content).
• No symlink fidelity: archives contained zero symlink entries — links became full copies.
• Hard synth crash: circular symlinks (legitimate in pnpm trees) hit ELOOP. Notably this crashed for every asset type, not just ARCHIVE, because hashPath runs in the TerraformAsset constructor.

This is a fork regression, not inherited from cdktf: upstream archives with archiver, which walks with lstat and emits real symlink entries (cycles are structurally impossible there). The regression entered when archiver was replaced by yazl in #95 — yazl has no symlink awareness at all (addFile stats-and-follows) — and survived the yazl → fflate rewrite in #148, which kept the hand-rolled statSync walk.

The fix

All three walkers in packages/cdktn/src/private/fs.ts now use lstatSync and treat symlinks first-class, restoring archiver/cdktf parity:

• archiveSync emits real symlink entries: the readlink target as STORED entry data with S_IFLNK | perms in the unix external attributes and version-made-by host = Unix — exactly what archiver produces and what Info-ZIP unzip, Go archive/zip, and the AWS Lambda runtime (verified live on nodejs22.x in TerraformAsset: AssetType.ARCHIVE follows symlinks (statSync), duplicating content and crashing on cyclic links #320) recreate as symlinks. Never recursing through links makes cycles unreachable by construction. fflate 0.8.2 supports this natively via per-file [data, { os, attrs, level }] tuples (attrs must be caller-pre-shifted, (mode << 16) >>> 0).
• hashPath hashes a symlink by its target path instead of following it — retargeting a link still changes the asset hash, but shared targets are no longer double-counted and cycles no longer crash the constructor.
• copySync (AssetType.DIRECTORY) recreates symlinks with fs.symlinkSync.

Two latent zip-metadata gaps fixed along the way (both also archiver/cdktf parity):

• Regular-file unix modes are now preserved (executable bits were silently stripped — matters for Lambda binaries and .bin scripts).
• Entry mtimes are pinned to a fixed 1980 date, making archives byte-reproducible across synths (fflate defaults mtime to Date.now(), so every synth previously produced different zip bytes — perpetual drift for anyone hashing the archive, e.g. filebase64sha256). The pin uses the local-time Date constructor deliberately: fflate encodes DOS dates from local getters and rejects years < 1980, so a UTC midnight date would underflow to 1979 in timezones west of UTC.

Prior art considered and rejected: terraform-provider-archive always dereferences and has no cycle detection — it accumulated the opt-in exclude_symlink_directories band-aid (v2.4.0, hashicorp/terraform-provider-archive#183; follow-up defect fixed in v2.4.2, #298) and still ELOOPs on cycles. Reverting to archiver would reintroduce the execSync child-process bridge that #148 removed (its API is async-only).

Hash compatibility (updated after review — downscoped)

Review surfaced a domain collision in the first cut of the hashPath change (file bytes and symlink-target bytes shared one unframed stream). Per the requested downscope this PR now carries only the compatibility-preserving resolution (reviewer's option 3):

• the legacy content hash is kept byte-identical for symlink-free trees (proven by a test that recomputes the historical md5 independently);
• symlink metadata (relative path + target) is framed into a separate digest and combined under a tagged outer hash only when symlinks exist — so hashes still change only for symlink-bearing trees, which today either bloat 2-3× or crash outright.

The canonical entry-framed scheme and its canonicalAssetHashes feature flag moved to the stack: design issue #322 → implementation #323 → docs #324.

TerraformModuleAsset stays in scope per review: it shares the corrected copier (its private copier sent directory symlinks into copyFileSync → EISDIR) with a regression test.

Testing

• packages/cdktn/test/archive-symlink.test.ts: 10 tests — the issue's repros (file/dir symlink preservation via system-unzip round-trip + lstat, dedup of shared targets, ELOOP on cycles for both archiveSync and hashPath, hash-changes-on-retarget), copySync symlink recreation, executable-bit preservation, and byte-identical archives across runs. 5 of the 6 core repros fail on main.
• Full cdktn package suite green (453 passed; the one pre-existing matchers.test.ts → toPlanSuccessfully failure shells out to a real terraform plan and fails identically without this change).
• Issue TerraformAsset: AssetType.ARCHIVE follows symlinks (statSync), duplicating content and crashing on cyclic links #320's literal repro script against the built lib: zipinfo shows lrwxr-xr-x ... stor entries for link-a/link-b, one payload copy (538-byte zip vs 3× 200 KiB before), and the cyclic tree archives to a 120-byte zip instead of aborting synth.

Checklist

• I have updated the PR title to match CDKTN's style guide
• I have run the linter on my code locally
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if applicable
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works if applicable
• New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

[so0k]

flagged as breaking change as it changes the checksums / SHA created by the TerraformAsset Archiver, and I agree that is should not be put behind a feature flag because:

Asset hashes change only for trees that contain symlinks — trees that today either bloat 2-3× or crash outright.

Altho I'm not sure if we want this (or it should be possible to opt-in / default opt-out to this mtime behavior)

Entry mtimes are pinned to a fixed 1980 date, making archives byte-reproducible across synths (fflate defaults mtime to Date.now(), so every synth previously produced different zip bytes — perpetual drift for anyone hashing the archive, e.g. filebase64sha256). The pin uses the local-time Date constructor deliberately: fflate encodes DOS dates from local getters and rejects years < 1980, so a UTC midnight date would underflow to 1979 in timezones west of UTC.

[sakul-learning]

Review summary

The symlink fix itself is well targeted, and the new tests pass the behavior-oriented/value assessment: each core test exercises an observable regression rather than implementation text. In particular, the extraction tests prove real symlink fidelity; the shared-target size test catches archive materialization/bloat; the circular-link tests protect all three walkers from ELOOP; retargeting checks asset invalidation; and the directory-copy test proves links are recreated rather than dereferenced. The executable-mode test protects a real Lambda/CLI artifact property, while the byte-identical archive test protects deterministic output (subject to resolving the maintainer tradeoff already raised about forcing mtimes). These tests are included by the normal cdktn Nx/Jest target, and the configured full run passed: 39 suites, 454 tests, and 288 snapshots. The focused validations suite and nx build cdktn also passed.

I found one blocking correctness issue in the new hash representation: regular-file bytes and symlink-target bytes occupy the same unframed hash domain, so materially different artifacts can receive the same assetHash and synthesized path. See the inline comment for options and compatibility tradeoffs.

I also think this small PR should cover TerraformModuleAsset's separate source copier. packages/cdktn/src/terraform-module-asset.ts:136-144 sends every non-directory, including a directory symlink, to copyFileSync, which can fail with EISDIR before the corrected TerraformAsset walkers are reached. Reusing the corrected helper (or adding equivalent symlink handling) plus one module-asset regression would address the same class of synthesis failures without materially expanding the PR. GitHub cannot attach a review comment to those unchanged lines, so this item is recorded here and alongside the shared copier.

No workflow files changed, and I found no security concern in the touched code. Ponytail/artifact-value pass: the core regression tests earn their maintenance cost; no speculative abstraction or low-signal static test was added.

[vincenthsh]

Altho I'm not sure if we want this (or it should be possible to opt-in / default opt-out to this mtime behavior)

Keeping the pin, on prior art: Go module zips do exactly this — the golang.org/x/mod/zip format spec says "File permissions and timestamps are also ignored" (entries get the zero DOS date; the format bans symlinks outright, so timestamps are its only nondeterminism to kill). Python wheels likewise clamp entry mtimes to 1980 for reproducibility (the zip format can't represent earlier dates). The reproducible-builds ecosystem treats embedded wall-clock timestamps in archives as a defect.

Two things bound the blast radius here: assetHash (and therefore the asset path) comes from hashPath over the source tree, never from the zip bytes, so the pin changes no paths; and the previous behavior was fflate's Date.now() default — meaning nobody can be depending on meaningful mtimes inside these archives today, because every synth already produced fresh ones. The pin only removes noise for anyone hashing the artifact (filebase64sha256 on the archive path currently drifts on every synth).

If someone does turn up with a real need for source mtimes we can add a knob then, but I'd rather not grow the JSII surface for it preemptively.

[sakul-learning]

Requested downscope: keep option 3 here; move canonical hashing out

The compatibility-preserving option 3 now resolves the blocker from the earlier review: trees without symlinks keep their historical content hash, while symlink-bearing trees combine separately framed symlink metadata under a tagged outer digest. That is sufficient for #320 and should remain in this PR.

Please remove the additional canonicalAssetHashes implementation, feature flag, and canonical-only tests from this PR. I opened #322 for the implementer to take as that independently reviewable, feature-flagged follow-up:

#322

The reason to separate it is not only scope. The current canonical representation has a metadata hole: it frames file/symlink type, relative path, payload length, and payload, but regular-file records omit the mode that archiveSync preserves, and directories contribute no records. I reproduced 0644 → 0755 producing identical canonical hashes while producing different archive bytes; adding an empty directory is likewise invisible even where the asset copier emits it.

The Git/Nix precedent did not expose this problem because their canonical formats serialize more than payload framing:

• Git tree entries include mode, name, and object ID; executable files (100755), ordinary files (100644), symlinks (120000), and subtrees are structurally distinct.
• Nix NAR represents directory entries/node types and an executable marker.

So the framing pattern was borrowed without the complete metadata model that makes those formats canonical. #322 captures the needed contract and regressions for modes, empty directories, paths, entry boundaries, and symlink metadata.

The TerraformModuleAsset copier fix should stay in this PR. It is not speculative canonical-hash work: its existing copier is another active walker over the same asset source class, and a directory symlink reaches copyFileSync and fails with EISDIR before the corrected TerraformAsset walkers run. Reusing the shared symlink-aware copier is a small deletion/consolidation that fixes the same concrete synthesis-failure class and now has a regression test.

[vincenthsh]

Downscoped as requested — the branch was force-pushed so this PR is now exactly: the three symlink-aware walkers, the option-3 compatibility-preserving hash, and the shared TerraformModuleAsset copier with its EISDIR regression (kept in scope per your note). The canonicalAssetHashes flag, canonical scheme, and canonical-only tests are gone from this diff.

The follow-up is a stack, review-ready:

• feat(lib): canonical asset hashes behind the canonicalAssetHashes feature flag #323 implements feat(lib): define a complete canonical asset-hash representation #322 on top of this branch (only its last commit is new). It adopts your metadata-model finding directly: file and symlink records now frame the octal permission mask (0o7777 bits — exactly what archiveSync emits into zip external attributes), and directories — including empty ones, which copySync materializes — contribute explicit D <relPath>\0 records (no mode, since neither emitter preserves directory permissions). Every acceptance criterion in feat(lib): define a complete canonical asset-hash representation #322 has a dedicated test, including your reproduced 0644 → 0755 case and empty-directory visibility.
• chore(docs): document feature flags and the canonicalAssetHashes scheme #324 (stacked on feat(lib): canonical asset hashes behind the canonicalAssetHashes feature flag #323) adds docs/feature-flags.md, documenting the flag lifecycle, why canonical hashing is the recommended future default, and the complete-metadata-model lesson from your review.

Thanks for the git/Nix observation — you were right that the first draft borrowed the framing without the object model that makes those formats canonical.

[sakul-learning]

Approved after the downscope and successful checks on ff817b2. The option-3 hash fix and TerraformModuleAsset copier regression are in scope; canonical hashing remains correctly split into the stacked follow-up.

Documentation follow-up for open-constructs/cdk-terrain-docs: please explain the deterministic ZIP timestamp policy and cite golang.org/x/mod/zip as precedent (its format says “File permissions and timestamps are also ignored”). Phrase the comparison precisely: TerraformAsset archives similarly avoid meaningful source timestamps by pinning ZIP mtimes, but unlike Go module ZIPs they preserve regular-file permission bits and support symlinks.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.