From d3a59621bfe84255ede32d4f662fa6df6532d585 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 11 Sep 2026 22:04:50 -0300 Subject: [PATCH] fix(electron): relativize standalone symlink targets for Windows manifest verify (#11979) (#13251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged as part of the 39-PR owner batch of 2026-09-11, validated as a unit. Boarded into one consolidated worktree cut from `release/v3.8.51` with the other 38 — zero conflicts between them. - ESLint over every changed file: no errors (the only finding was one suppression entry the batch emptied, pruned on #13243) - `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, within baseline); `check:changelog-integrity` OK - complexity 2821 / baseline 3218 and cognitive-complexity 1272 / baseline 1437 — both under baseline - 256 assertions green: 246 under node:test and 10 under vitest, which is where `tests/unit/**/*.test.tsx` actually runs - `check-file-size`: `chatCore.ts` rebaselined 6144 → 6146 for #13278 and #13276, annotated and landed on #13243 ⚠️ base-red inherited: #12732 — the provider count (356 in the docs vs the 358 the modules define) and `open-sse/utils/stream.ts` at 3115 > frozen 3098 both reproduce on the pure tip with zero contribution from this batch. --- ...standalone-manifest-symlink-portability.md | 1 + scripts/build/standaloneManifest.mjs | 63 ++++++- scripts/build/standaloneTarball.mjs | 17 +- ...alone-manifest-symlink-portability.test.ts | 175 ++++++++++++++++++ 4 files changed, 250 insertions(+), 6 deletions(-) create mode 100644 changelog.d/fixes/11979-standalone-manifest-symlink-portability.md create mode 100644 tests/unit/build/standalone-manifest-symlink-portability.test.ts diff --git a/changelog.d/fixes/11979-standalone-manifest-symlink-portability.md b/changelog.d/fixes/11979-standalone-manifest-symlink-portability.md new file mode 100644 index 0000000000..2425114cdd --- /dev/null +++ b/changelog.d/fixes/11979-standalone-manifest-symlink-portability.md @@ -0,0 +1 @@ +- fix(electron): relativize standalone-bundle symlink targets so Stage 8 manifest verification stops failing on Windows (#11979) diff --git a/scripts/build/standaloneManifest.mjs b/scripts/build/standaloneManifest.mjs index 19a3eb8288..3751dd6c5e 100644 --- a/scripts/build/standaloneManifest.mjs +++ b/scripts/build/standaloneManifest.mjs @@ -31,6 +31,47 @@ async function sha256File(filePath) { }); } +/** + * Rewrite a symlink target so it is anchored to the tree being packed/verified + * instead of the machine that happened to create it (issue #11979). + * + * `npm`'s bin-links usually produce a target already relative to the + * symlink's own directory (e.g. `../semver/bin/semver.js`), which survives an + * archive/restore round trip unchanged on every OS. Some `npm ci` legs + * (observed on the ubuntu web-build runner) instead emit an ABSOLUTE target + * tied to that machine's checkout path. An absolute target is inherently + * non-portable: POSIX restores it as a dangling symlink once the packing + * machine's path is gone, and Windows' `CreateSymbolicLink` rewrites a + * leading `/` into a drive-relative path on read-back, so a byte-for-byte + * comparison against the recorded value fails outright. + * + * A relative target has no such ambiguity, so an absolute target is resolved + * against `rootDir` and re-expressed relative to the symlink's own directory + * -- the same portable shape `npm install` already produces natively. + * + * @param {string} rootDir absolute path to the tree root + * @param {string} entryRelPath the symlink's own path, relative to rootDir (posix-separated) + * @param {string} rawTarget the raw string from fs.readlinkSync + * @returns {{ok: true, value: string} | {ok: false, reason: string}} + */ +export function normalizeSymlinkTarget(rootDir, entryRelPath, rawTarget) { + if (!path.isAbsolute(rawTarget)) { + return { ok: true, value: rawTarget }; + } + const rootResolved = path.resolve(rootDir); + const resolvedTarget = path.resolve(rawTarget); + const relFromRoot = path.relative(rootResolved, resolvedTarget); + if (relFromRoot === "" || relFromRoot.startsWith("..") || path.isAbsolute(relFromRoot)) { + return { + ok: false, + reason: `symlink ${entryRelPath} target escapes the tree root: ${rawTarget}`, + }; + } + const symlinkDir = path.dirname(path.join(rootResolved, ...entryRelPath.split("/"))); + const relFromSymlink = path.relative(symlinkDir, resolvedTarget).split(path.sep).join("/"); + return { ok: true, value: relFromSymlink }; +} + function walkDir(root, current, entries) { const children = fs.readdirSync(current, { withFileTypes: true }); // Sort for determinism: manifest of the same tree is byte-identical. @@ -39,7 +80,11 @@ function walkDir(root, current, entries) { const abs = path.join(current, child.name); const rel = path.relative(root, abs).split(path.sep).join("/"); if (child.isSymbolicLink()) { - entries.push({ path: rel, symlink: fs.readlinkSync(abs) }); + const normalized = normalizeSymlinkTarget(root, rel, fs.readlinkSync(abs)); + if (!normalized.ok) { + throw new Error(`standalone manifest: ${normalized.reason}`); + } + entries.push({ path: rel, symlink: normalized.value }); } else if (child.isDirectory()) { walkDir(root, abs, entries); } else if (child.isFile()) { @@ -101,9 +146,19 @@ export async function verifyStandaloneManifest(rootDir, manifest) { if (!stat.isSymbolicLink()) { errors.push(`${entry.path}: expected symlink, found regular entry`); } else { - const target = fs.readlinkSync(abs); - if (target !== entry.symlink) { - errors.push(`${entry.path}: symlink target ${target} != ${entry.symlink}`); + // Normalize BOTH sides before comparing: the manifest's recorded + // value is already relative for a tree built after #11979, but an + // older manifest (or a restoring OS that still hands back an + // absolute string) is re-anchored here too, so the comparison never + // depends on which machine happened to produce which string. + const actual = normalizeSymlinkTarget(rootDir, entry.path, fs.readlinkSync(abs)); + const expected = normalizeSymlinkTarget(rootDir, entry.path, entry.symlink); + if (!actual.ok) { + errors.push(`${entry.path}: ${actual.reason}`); + } else if (!expected.ok) { + errors.push(`${entry.path}: manifest ${expected.reason}`); + } else if (actual.value !== expected.value) { + errors.push(`${entry.path}: symlink target ${actual.value} != ${expected.value}`); } } continue; diff --git a/scripts/build/standaloneTarball.mjs b/scripts/build/standaloneTarball.mjs index 94afbc0334..a5a1c8f1b0 100644 --- a/scripts/build/standaloneTarball.mjs +++ b/scripts/build/standaloneTarball.mjs @@ -21,6 +21,7 @@ import fs from "node:fs"; import path from "node:path"; import { once } from "node:events"; import { createGunzip, createGzip } from "node:zlib"; +import { normalizeSymlinkTarget } from "./standaloneManifest.mjs"; const BLOCK = 512; @@ -88,7 +89,16 @@ function* walkFiles(root, current = root) { const abs = path.join(current, child.name); const rel = path.relative(root, abs).split(path.sep).join("/"); if (child.isSymbolicLink()) { - yield { rel, symlink: fs.readlinkSync(abs) }; + // Same portability normalization as the manifest (issue #11979): an + // absolute symlink target survives this exact tree on the packing + // machine, but not the tar round trip to another OS/checkout path. + // Packing the relative form here is what makes the *restored* symlink + // actually resolve, not just what makes the manifest comparison match. + const normalized = normalizeSymlinkTarget(root, rel, fs.readlinkSync(abs)); + if (!normalized.ok) { + throw new Error(`standalone tarball: ${normalized.reason}`); + } + yield { rel, symlink: normalized.value }; } else if (child.isDirectory()) { yield* walkFiles(root, abs); } else if (child.isFile()) { @@ -343,7 +353,10 @@ export async function extractTarGz(archiveFile, destDir) { if (linkname.length === 0) throw new Error(`symlink entry ${name} has empty target`); fs.mkdirSync(path.dirname(target), { recursive: true }); fs.rmSync(target, { force: true }); - fs.symlinkSync(linkname, target); + // Standalone node_modules symlinks are always file symlinks (npm + // bin-links, package aliasing); an explicit type hint removes + // Windows' undocumented auto-detect ambiguity for CreateSymbolicLink. + fs.symlinkSync(linkname, target, "file"); } else if (typeflag === "1") { const sourceAbs = safeJoin(destDir, linkname); fs.mkdirSync(path.dirname(target), { recursive: true }); diff --git a/tests/unit/build/standalone-manifest-symlink-portability.test.ts b/tests/unit/build/standalone-manifest-symlink-portability.test.ts new file mode 100644 index 0000000000..21e9359c0f --- /dev/null +++ b/tests/unit/build/standalone-manifest-symlink-portability.test.ts @@ -0,0 +1,175 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +/** + * Regression guard for issue #11979 (Stage 8 shared standalone bundle fails + * manifest verification on Windows because symlink targets were stored as + * absolute, packing-machine-specific paths). + * + * npm ci on the ubuntu web-build leg is observed (run 33238093090) to + * produce at least one ABSOLUTE .bin symlink target (an npm/bin-links + * implementation detail) instead of the RELATIVE target a local `npm + * install` produces for the identical file + * (node_modules/global-agent/node_modules/.bin/semver -> ../semver/bin/semver.js). + * An absolute target is not portable: it dangles once the packing machine's + * path is gone (silent false-positive on POSIX) and Windows' + * CreateSymbolicLink rewrites it to a drive-relative path on read-back + * (outright verification failure) -- both symptoms share the same root + * cause of never normalizing to a relative, tree-anchored form. + */ + +const manifestMod = await import("../../../scripts/build/standaloneManifest.mjs"); +const { buildStandaloneManifest, verifyStandaloneManifest, normalizeSymlinkTarget } = + manifestMod as typeof manifestMod & { + buildStandaloneManifest: ( + rootDir: string + ) => Promise<{ version: number; entries: { path: string; symlink?: string }[] }>; + verifyStandaloneManifest: ( + rootDir: string, + manifest: unknown + ) => Promise<{ ok: true } | { ok: false; errors: string[] }>; + normalizeSymlinkTarget: ( + rootDir: string, + entryRelPath: string, + rawTarget: string + ) => { ok: true; value: string } | { ok: false; reason: string }; + }; + +function tmpDir(prefix: string): string { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); +} + +/** Build the exact shape from the report: nested node_modules .bin symlink. */ +function makeTreeWithAbsoluteBinSymlink(root: string): void { + const semverDir = path.join( + root, + "node_modules", + "global-agent", + "node_modules", + "semver", + "bin" + ); + fs.mkdirSync(semverDir, { recursive: true }); + fs.writeFileSync(path.join(semverDir, "semver.js"), "#!/usr/bin/env node\n// fake semver cli\n"); + + const binDir = path.join(root, "node_modules", "global-agent", "node_modules", ".bin"); + fs.mkdirSync(binDir, { recursive: true }); + // Mirrors what the ubuntu web-build leg actually produced: an ABSOLUTE + // symlink target tied to that machine's checkout path. + fs.symlinkSync(path.join(semverDir, "semver.js"), path.join(binDir, "semver")); +} + +test("#11979: buildStandaloneManifest relativizes an absolute .bin symlink target", async () => { + const packRoot = tmpDir("standalone-pack-"); + makeTreeWithAbsoluteBinSymlink(packRoot); + + const manifest = await buildStandaloneManifest(packRoot); + const entry = manifest.entries.find((e) => e.path.endsWith("node_modules/.bin/semver")); + assert.ok(entry, "manifest must record the .bin/semver symlink entry"); + assert.ok(entry!.symlink, "entry must be recorded as a symlink"); + assert.equal( + path.isAbsolute(entry!.symlink!), + false, + "recorded target must be relativized, not the packing-machine absolute path" + ); + assert.equal( + entry!.symlink, + "../semver/bin/semver.js", + "must match the portable form npm install already produces locally for this exact file" + ); + + fs.rmSync(packRoot, { recursive: true, force: true }); +}); + +test("#11979: a relativized manifest restores to a working (non-dangling) symlink", async () => { + const packRoot = tmpDir("standalone-pack-"); + makeTreeWithAbsoluteBinSymlink(packRoot); + const manifest = await buildStandaloneManifest(packRoot); + const entry = manifest.entries.find((e) => e.path.endsWith("node_modules/.bin/semver"))!; + + // packRoot no longer exists once the archive is shipped to another + // machine/leg -- only the tar + manifest travel. + fs.rmSync(packRoot, { recursive: true, force: true }); + + // Simulate restoring the identical relative tree under a different + // absolute root, exactly what extractTarGz now does: it recreates each + // symlink verbatim from the (now-relativized) manifest string. + const restoredRoot = tmpDir("standalone-restore-"); + const semverDir2 = path.join( + restoredRoot, + "node_modules", + "global-agent", + "node_modules", + "semver", + "bin" + ); + fs.mkdirSync(semverDir2, { recursive: true }); + fs.writeFileSync( + path.join(semverDir2, "semver.js"), + "#!/usr/bin/env node\n// fake semver cli\n" + ); + const binDir = path.join(restoredRoot, "node_modules", "global-agent", "node_modules", ".bin"); + fs.mkdirSync(binDir, { recursive: true }); + fs.symlinkSync(entry.symlink!, path.join(binDir, "semver")); + + const verdict = await verifyStandaloneManifest(restoredRoot, manifest); + const restoredBin = path.join(binDir, "semver"); + + assert.equal( + fs.existsSync(restoredBin), + true, + "restored .bin/semver must resolve to the co-located semver.js" + ); + assert.deepEqual(verdict, { ok: true }, "a genuinely portable restored tree must verify clean"); + + fs.rmSync(restoredRoot, { recursive: true, force: true }); +}); + +test("#11979: relative-target comparison is unaffected when both sides already match (scope boundary)", async () => { + // The fix is about portability of the *recorded string*, not about + // resolving the symlink on disk -- a relative target that matches the + // manifest still verifies even if the referenced file happens to be + // missing on this particular tree. Documented here so a future change + // does not assume verifyStandaloneManifest performs fs resolution. + const restoredRoot = tmpDir("standalone-restore-"); + const binDir = path.join(restoredRoot, "node_modules", "global-agent", "node_modules", ".bin"); + fs.mkdirSync(binDir, { recursive: true }); + fs.symlinkSync("../semver/bin/semver.js", path.join(binDir, "semver")); + // Note: unlike the previous test, semver.js is never created here, so the + // relative target is a real dangling reference on the restored tree. + + const manifest = { + version: 1, + entries: [ + { + path: "node_modules/global-agent/node_modules/.bin/semver", + bytes: 0, + sha256: "", + symlink: "../semver/bin/semver.js", + }, + ], + }; + + const verdict = await verifyStandaloneManifest(restoredRoot, manifest); + // The string-form comparison still matches (both sides are the same + // relative string), which is correct: the manifest layer's job is + // portability of the *recorded* target, not filesystem resolution -- + // this asserts that guarantee is unaffected by the fix. + assert.deepEqual(verdict, { ok: true }); + + fs.rmSync(restoredRoot, { recursive: true, force: true }); +}); + +test("#11979: normalizeSymlinkTarget rejects an absolute target that escapes the tree root", () => { + const rootDir = tmpDir("standalone-root-"); + const result = normalizeSymlinkTarget( + rootDir, + "node_modules/.bin/semver", + "/completely/unrelated/path/semver.js" + ); + assert.equal(result.ok, false); + fs.rmSync(rootDir, { recursive: true, force: true }); +});