diff --git a/src/install/PackageManager/processDependencyList.rs b/src/install/PackageManager/processDependencyList.rs index e0cbfae08021..f99d215820f5 100644 --- a/src/install/PackageManager/processDependencyList.rs +++ b/src/install/PackageManager/processDependencyList.rs @@ -179,7 +179,18 @@ impl PackageManager { } _ => false, }; - if !(from_lockfile && moved) { + if !from_lockfile { + return existing; + } + if existing.resolution.tag.is_tarball() { + // A row that records no scripts leaves them to the installed package.json. + if existing.scripts.filled { + existing.scripts = package.scripts; + } + (existing.meta).set_has_install_script(package.meta.has_install_script()); + self.lockfile.packages.set(existing_id as usize, existing); + } + if !moved { return existing; } if existing.resolution.tag.is_tarball() && existing.meta.integrity.tag.is_supported() { @@ -198,6 +209,10 @@ impl PackageManager { existing.dependencies = package.dependencies; existing.resolutions = package.resolutions; existing.bin = package.bin; + if existing.resolution.tag.is_git() && package.meta.has_install_script() { + // The same commit: only a lockfile from before `hasInstallScript` lacks it. + (existing.meta).set_has_install_script(true); + } if data.integrity.tag.is_supported() { existing.meta.integrity = data.integrity; } diff --git a/test/cli/install/bun-install-tarball-integrity.test.ts b/test/cli/install/bun-install-tarball-integrity.test.ts index db452ac39553..5189cfa44969 100644 --- a/test/cli/install/bun-install-tarball-integrity.test.ts +++ b/test/cli/install/bun-install-tarball-integrity.test.ts @@ -1,7 +1,8 @@ import { file, spawn } from "bun"; +import { install_test_helpers } from "bun:internal-for-testing"; import { afterAll, beforeAll, describe, expect, it, setDefaultTimeout } from "bun:test"; import { realpath, rm, writeFile } from "fs/promises"; -import { bunExe, bunEnv as env, readdirSorted, tempDir } from "harness"; +import { bunExe, bunEnv as env, githubTarball, hostedGitRepository, readdirSorted, tempDir } from "harness"; import { createHash, randomBytes } from "node:crypto"; import { gzipSync } from "node:zlib"; import { dirname, join } from "path"; @@ -15,6 +16,8 @@ import { type TestContext, } from "./dummy.registry"; +const { parseLockfile } = install_test_helpers; + setDefaultTimeout(1000 * 60 * 5); beforeAll(() => { @@ -540,11 +543,12 @@ describe.concurrent.each( (["hoisted", "isolated"] as const).map(linker => [kind, linker] as const), ), )("%s tarball cache entries (%s)", (kind, linker) => { - async function tarball(exported: string, manifest: object = {}, name = "pkg") { + async function tarball(exported: string, manifest: object = {}, name = "pkg", files: Record = {}) { const tgz = await new Bun.Archive( { "package/package.json": JSON.stringify({ name, version: "1.0.0", ...manifest }), "package/index.js": `module.exports = ${JSON.stringify(exported)};\n`, + ...Object.fromEntries(Object.entries(files).map(([path, text]) => [`package/${path}`, text])), }, { compress: "gzip" }, ).bytes(); @@ -955,6 +959,183 @@ describe.concurrent.each( expect(await file(join(siblings, "dep", "index.js")).text()).toBe('module.exports = "dep";\n'); }); + // The row of a tarball that is read again describes what was read: its lifecycle scripts, which only + // bun.lockb records, and whether it has one, which both lockfiles record. + const writes = (hook: "preinstall" | "postinstall", says: string) => ({ [hook]: `echo ${says} > ran.txt` }); + const trusting = { trustedDependencies: ["pkg"] }; + /** True if the install said that it did not run a lifecycle script of `pkg`, which is not trusted. */ + const reportsBlocked = (stdout: string) => stdout.includes("Blocked 1 postinstall"); + /** What the lifecycle script of the installed `pkg` wrote, or null if none ran. */ + async function ranIn(box: ReturnType, project: string) { + const wrote = file(join(box.root, project, "node_modules", "pkg", "ran.txt")); + return (await wrote.exists()) ? await wrote.text() : null; + } + /** Makes `project` save `lockfile`, with more in its package.json. */ + async function configure( + box: ReturnType, + project: string, + lockfile: "bun.lock" | "bun.lockb", + manifest: object = {}, + ) { + await writeFile( + join(box.root, project, "package.json"), + JSON.stringify({ name: "app", dependencies: { pkg: box.spec }, ...manifest }), + ); + await writeFile( + join(box.root, project, "bunfig.toml"), + Bun.TOML.stringify({ install: { linker, saveTextLockfile: lockfile === "bun.lock" } }), + ); + } + /** The second item of the row of `pkg` in bun.lock. */ + const locked = async (box: ReturnType, project: string) => + (Bun.JSONC.parse(await box.lockfile(project)) as any).packages.pkg[1]; + // The same code takes what was read for a `file:` row and for a URL row. + const fileOnly = it.skipIf(kind === "URL"); + + fileOnly.each( + (["bun.lock", "bun.lockb"] as const).flatMap(lockfile => + ( + [ + ["adds", {}, writes("postinstall", "v2"), "v2\n"], + ["changes", writes("preinstall", "v1"), writes("preinstall", "v2"), "v2\n"], + ["drops", writes("postinstall", "v1"), {}, null], + ] as const + ).map(row => [lockfile, ...row] as const), + ), + )( + "with %s, bun update runs what the new build has when it %s a lifecycle script", + async (lockfile, verb, from, to, wrote) => { + using box = sandbox(`tarball-scripts-${verb}`, { app: {} }); + await configure(box, "app", lockfile, trusting); + + await box.put("app", await tarball("v1", { scripts: from })); + await box.ok("app", "install"); + expect(await ranIn(box, "app")).toBe(verb === "adds" ? null : "v1\n"); + + await box.put("app", await tarball("v2", { scripts: to })); + await box.ok("app", "update"); + expect(await box.installed("app")).toBe('module.exports = "v2";\n'); + expect(await ranIn(box, "app")).toBe(wrote); + // What the installs that follow read. + if (lockfile === "bun.lockb") { + expect(parseLockfile(join(box.root, "app")).packages[1].scripts).toEqual(to); + } else { + expect(await locked(box, "app")).toEqual(verb === "drops" ? {} : { hasInstallScript: true }); + } + }, + ); + + it("each command that takes the new bytes records whether they have a lifecycle script", async () => { + using box = sandbox("tarball-has-install-script", { app: {}, fresh: {} }); + let build = await tarball("v1"); + + await box.put("app", build); + await box.ok("app", "install"); + expect(await locked(box, "app")).toEqual({}); + + // A binding.gyp is built by `node-gyp rebuild`, unless the package.json says `gypfile: false`. + for (const [command, scripted, manifest, files] of [ + [["update"], true, { scripts: { postinstall: "echo built" } }, {}], + [["install", "--force"], false, { gypfile: false }, { "binding.gyp": "{}" }], + [["add", box.spec], true, { scripts: { preinstall: "echo built" } }, {}], + [["update", "pkg"], false, {}, {}], + [["update"], true, {}, { "binding.gyp": "{}" }], + ] as const) { + build = await tarball(command.join(" "), manifest, "pkg", files); + await box.put("app", build); + const { stdout } = await box.ok("app", ...command); + expect({ reported: reportsBlocked(stdout), locked: await locked(box, "app") }).toEqual({ + reported: scripted, + locked: scripted ? { hasInstallScript: true } : {}, + }); + } + + // The same bun.lock as in a project that never had another build. + await box.put("fresh", build); + await box.ok("fresh", "install"); + expect(await box.lockfile("app")).toBe(await box.lockfile("fresh")); + }); + + // `bun pm untrusted` and `bun pm trust` read the scripts that bun.lockb records. + it("bun.lockb records the scripts of the new build of a package that is not trusted", async () => { + using box = sandbox("tarball-lockb-untrusted", { app: {}, fresh: {} }); + await configure(box, "app", "bun.lockb"); + + await box.put("app", await tarball("v1")); + await box.ok("app", "install"); + + await box.put("app", await tarball("v2", { scripts: writes("postinstall", "v2") })); + expect(reportsBlocked((await box.ok("app", "update")).stdout)).toBe(true); + expect((await box.ok("app", "pm", "untrusted")).stdout).toContain("[postinstall]: echo v2 > ran.txt"); + expect(reportsBlocked((await box.reinstall("app")).stdout)).toBe(true); + + // The row that was loaded from bun.lockb is saved in bun.lock. + const v3 = await tarball("v3", { scripts: writes("postinstall", "v3") }); + await box.put("app", v3); + expect(reportsBlocked((await box.ok("app", "update", "--save-text-lockfile")).stdout)).toBe(true); + await box.put("fresh", v3); + await box.ok("fresh", "install"); + expect(await box.lockfile("app")).toBe(await box.lockfile("fresh")); + }); + + // An older bun.lock, and the same bytes. The tarball is read again for each of these commands. + fileOnly.each([ + ["no integrity", false], + ["the integrity and no hasInstallScript", true], + ] as const)("each command records the lifecycle script of a row that has %s", async (_, pinned) => { + using box = sandbox("tarball-older-lockfile", { app: {} }); + const build = await tarball("v1", { scripts: writes("postinstall", "v1") }); + await box.put("app", build); + + for (const command of [["update"], ["update", "pkg"], ["install", "--force"], ["add", box.spec]]) { + await rm(join(box.root, "app", "node_modules"), { recursive: true, force: true }); + await writeFile( + join(box.root, "app", "bun.lock"), + JSON.stringify({ + lockfileVersion: 1, + configVersion: 1, + workspaces: { "": { name: "app", dependencies: { pkg: box.spec } } }, + packages: { pkg: [`pkg@${box.resolution}`, {}, ...(pinned ? [build.integrity] : [])] }, + }), + ); + const { stdout } = await box.ok("app", ...command); + expect({ + reported: reportsBlocked(stdout), + locked: (Bun.JSONC.parse(await box.lockfile("app")) as any).packages.pkg, + }).toEqual({ + reported: true, + locked: [`pkg@${box.resolution}`, { hasInstallScript: true }, build.integrity], + }); + } + }); + + // bun.lock records no scripts: they are read from the installed package.json, which a patch can edit. + fileOnly("bun update runs the lifecycle script that a patch edited, as bun install does", async () => { + using box = sandbox("tarball-scripts-patched", { app: {} }); + const manifest = (says: string) => + JSON.stringify({ name: "pkg", version: "1.0.0", scripts: writes("postinstall", says) }); + const noNewline = "\\ No newline at end of file"; + await configure(box, "app", "bun.lock", { + ...trusting, + patchedDependencies: { [`pkg@${box.resolution}`]: "pkg.patch" }, + }); + await writeFile( + join(box.root, "app", "pkg.patch"), + `diff --git a/package.json b/package.json\n--- a/package.json\n+++ b/package.json\n@@ -1 +1 @@\n-${manifest("tarball")}\n${noNewline}\n+${manifest("patched")}\n${noNewline}\n`, + ); + + await box.put("app", await tarball("v1", { scripts: writes("postinstall", "tarball") })); + await box.ok("app", "install"); + await box.reinstall("app"); + expect(await ranIn(box, "app")).toBe("patched\n"); + + await box.put("app", await tarball("v2", { scripts: writes("postinstall", "tarball") })); + await box.ok("app", "update"); + expect(await box.installed("app")).toBe('module.exports = "v2";\n'); + expect(await file(join(box.root, "app", "node_modules", "pkg", "package.json")).text()).toBe(manifest("patched")); + expect(await ranIn(box, "app")).toBe("patched\n"); + }); + // `--no-verify` skips the hash check of a download (a `file:` tarball is always checked). // Unverified bytes never get a cache entry named after the hash in bun.lock: their entry // is named after the URL and that hash together, and only `--no-verify` installs look there. @@ -1129,6 +1310,100 @@ describe.concurrent.each( }); }); +// `bun update` reads a git or GitHub dependency again when its commit is the same, so that what it depends +// on resolves again. Nothing of the package is new then. Only a bun.lock from before `hasInstallScript` was +// written lacks it. +describe.concurrent("a git or GitHub row that bun update reads again", () => { + const manifest = (scripts: object) => JSON.stringify({ name: "pkg", version: "1.0.0", scripts }); + + /** A project that depends on `pkg`. */ + function project(name: string, dependency: string, environment: Record) { + const dir = tempDir(name, { "package.json": JSON.stringify({ name: "app", dependencies: { pkg: dependency } }) }); + const cache = join(String(dir), "cache"); + const lockfile = () => file(join(String(dir), "bun.lock")).text(); + const locked = async () => (Bun.JSONC.parse(await lockfile()) as any).packages.pkg[1]; + return { + cache, + locked, + async ok(...args: string[]) { + await using proc = spawn({ + cmd: [bunExe(), ...args], + cwd: String(dir), + env: { ...env, ...environment, BUN_INSTALL_CACHE_DIR: cache }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect({ stdout, exitCode }).toEqual({ stdout: expect.any(String), exitCode: 0 }); + }, + /** Makes bun.lock one from before `hasInstallScript` was written. */ + async forget() { + await writeFile( + join(String(dir), "bun.lock"), + (await lockfile()).replace('{ "hasInstallScript": true }', "{}"), + ); + expect(await locked()).toEqual({}); + }, + [Symbol.dispose]: () => dir[Symbol.dispose](), + }; + } + const gitRepository = (scripts: object) => + hostedGitRepository(["https://gitlab.com/group/pkg"], { "package.json": manifest(scripts) }); + + it("records the lifecycle script of a git dependency", async () => { + using repo = await gitRepository({ postinstall: "echo built" }); + using app = project("git-has-install-script", "gitlab:group/pkg", repo.env); + + await app.ok("install"); + expect(await app.locked()).toEqual({ hasInstallScript: true }); + await app.forget(); + await app.ok("update"); + expect(await app.locked()).toEqual({ hasInstallScript: true }); + }); + + it("records the lifecycle script of a GitHub dependency", async () => { + const archive = await githubTarball("gh-owner-gh-repo-0badc0d", { + "package.json": manifest({ postinstall: "echo built" }), + }); + await using github = Bun.serve({ + port: 0, + fetch: req => + new URL(req.url).pathname === "/repos/gh-owner/gh-repo/tarball/0badc0d" + ? new Response(archive) + : new Response("not found", { status: 404 }), + }); + using app = project("github-has-install-script", "gh-owner/gh-repo#0badc0d", { + GITHUB_API_URL: github.url.origin, + }); + + await app.ok("install"); + expect(await app.locked()).toEqual({ hasInstallScript: true }); + await app.forget(); + await app.ok("update"); + expect(await app.locked()).toEqual({ hasInstallScript: true }); + }); + + // node_modules links to the files of the checkout in the cache, so a lifecycle script can edit them. + it("does not clear the flag of a git dependency whose checkout was edited", async () => { + using repo = await gitRepository({}); + using app = project("git-edited-checkout", "gitlab:group/pkg", repo.env); + const checkout = join(app.cache, `@G@${repo.sha}`, "package.json"); + + await app.ok("install"); + expect(await app.locked()).toEqual({}); + + // `bun update` reads the package.json of that checkout. + await writeFile(checkout, manifest({ postinstall: "echo built" })); + await app.ok("update"); + expect(await app.locked()).toEqual({ hasInstallScript: true }); + + await writeFile(checkout, manifest({})); + await app.ok("update"); + expect(await app.locked()).toEqual({ hasInstallScript: true }); + }); +}); + describe.concurrent.each(["hoisted", "isolated"] as const)("tarball integrity mismatch (%s)", linker => { // Regression test for #29646 — with the isolated linker, a SHA-512 mismatch // during the resolve-phase tarball extract left `task_queue` /