ReviewOS
Reload

also looking at this

craft-native/craft

fix(packaging): stop macOS packages relocating themselves elsewhere

#46
Merged glennmichael123 wants to merge fix/pkg-bundle-relocation into main
4 files +164 -16
.github/workflows/native-lifecycle.ymlmodified+6-3
@@ -96,8 +96,11 @@ jobs:
9696 uses: actions/upload-artifact@v4
9797 with:
9898 name: native-lifecycle-${{ matrix.name }}
99 path: |
100 artifacts/native-lifecycle/${{ matrix.name }}/report.json
101 artifacts/native-lifecycle/${{ matrix.name }}/work/packages-*/**
99 # Everything, not just report.json and the packages directory. The
100 # macOS relocation bug took a workflow dispatch to diagnose because
101 # the compiled fixtures at work/craft-lifecycle-* were outside the
102 # uploaded subtree, so the artifact alone could not answer what the
103 # runner had actually built.
104 path: artifacts/native-lifecycle/${{ matrix.name }}/**
102105 if-no-files-found: error
103106 retention-days: 90
packages/typescript/src/package.test.tsmodified+56-1
@@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from 'bun:test'
22import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
33import { tmpdir } from 'node:os'
44import { join } from 'node:path'
5import { candleArguments, codesignArguments, dmgCapacityMegabytes, dmgCreateArguments, formatPackagingCommandError, macOSInfoPlist, notarytoolArguments, packageApp, productbuildArguments, renderWixSource, shouldRetryHdiutil, windowsArchitecture, windowsExecutableName } from './package.js'
5import { candleArguments, codesignArguments, dmgCapacityMegabytes, dmgCreateArguments, formatPackagingCommandError, macOSInfoPlist, notarytoolArguments, packageApp, pkgbuildArguments, pkgbuildComponentPlist, productbuildArguments, renderWixSource, shouldRetryHdiutil, windowsArchitecture, windowsExecutableName } from './package.js'
66
77describe('Windows MSI packaging', () => {
88 it('renders a deterministic major-upgrade installer without shell interpolation', () => {
@@ -51,6 +51,61 @@ describe('Windows MSI packaging', () => {
5151 })
5252})
5353
54describe('macOS pkg relocation', () => {
55 it('pins the install path instead of letting the bundle be relocated', () => {
56 const plist = pkgbuildComponentPlist('Applications/Craft App.app')
57
58 // The one that matters. Left to pkgbuild, this is true, and the package
59 // then carries <relocate><bundle id="..."/></relocate> — which lets
60 // `installer` write the payload over any copy of that bundle the system
61 // has registered elsewhere and still exit 0. "The install was successful"
62 // with nothing at /Applications is that, and it is intermittent because it
63 // depends on what the system has indexed.
64 expect(plist).toContain('<key>BundleIsRelocatable</key>\n <false/>')
65
66 // Off so that installing an older version over a newer one is not skipped;
67 // with it on, a rollback silently becomes a no-op.
68 expect(plist).toContain('<key>BundleIsVersionChecked</key>\n <false/>')
69
70 expect(plist).toContain('<string>Applications/Craft App.app</string>')
71 expect(plist).toContain('<?xml version="1.0" encoding="UTF-8"?>')
72 })
73
74 it('escapes the bundle path rather than emitting broken XML', () => {
75 expect(pkgbuildComponentPlist('Applications/Ben & Co.app')).toContain('Applications/Ben &amp; Co.app')
76 })
77
78 it('always passes the component plist to pkgbuild', () => {
79 const args = pkgbuildArguments({
80 root: '/tmp/stage/root',
81 componentPlistPath: '/tmp/stage/component.plist',
82 identifier: 'dev.craft.lifecycle',
83 version: '1.0.0',
84 outputPath: '/tmp/out.pkg',
85 })
86 expect(args).toEqual([
87 '--root', '/tmp/stage/root',
88 '--component-plist', '/tmp/stage/component.plist',
89 '--identifier', 'dev.craft.lifecycle',
90 '--version', '1.0.0',
91 '--install-location', '/',
92 '/tmp/out.pkg',
93 ])
94
95 // Signing is additive and must not displace the component plist.
96 const signed = pkgbuildArguments({
97 root: '/tmp/stage/root',
98 componentPlistPath: '/tmp/stage/component.plist',
99 identifier: 'dev.craft.lifecycle',
100 version: '1.0.0',
101 outputPath: '/tmp/out.pkg',
102 installerIdentity: '3rd Party Mac Developer Installer: Example',
103 })
104 expect(signed).toContain('--component-plist')
105 expect(signed.slice(-3, -1)).toEqual(['--sign', '3rd Party Mac Developer Installer: Example'])
106 })
107})
108
54109describe('macOS packaging diagnostics', () => {
55110 it('retains hdiutil output in a failed package result', () => {
56111 expect(formatPackagingCommandError('hdiutil', 1, '', 'create failed - Resource temporarily unavailable\n'))
packages/typescript/src/package.tsmodified+91-12
@@ -844,6 +844,74 @@ async function createDMG(opts: {
844844 * - a signed `productbuild` submission package for the Mac App Store, which is
845845 * the only form App Store Connect accepts.
846846 */
847/**
848 * The component property list `pkgbuild` is given for the app bundle.
849 *
850 * Written out rather than left to `pkgbuild`'s inference, because its default
851 * for a bundle is `BundleIsRelocatable = true`, which puts this in the package:
852 *
853 * <relocate><bundle id="dev.example.app"/></relocate>
854 *
855 * That directive tells `installer` the payload path is only a suggestion: it
856 * looks the bundle identifier up on the target volume and, if the system has a
857 * copy of that bundle registered anywhere, writes the payload over *that* copy
858 * instead — and exits 0. The user sees "The install was successful" and finds
859 * nothing at /Applications.
860 *
861 * It is intermittent by nature, since it turns on whether the system has
862 * indexed some other copy yet, which is exactly how it behaved: the native
863 * lifecycle workflow's macOS legs failed only on slow runs, on both
864 * architectures, with `installer` reporting success and the app absent.
865 *
866 * `BundleIsVersionChecked` is off for a related reason — with it on, installing
867 * an *older* version over a newer one is skipped, which silently turns a
868 * rollback into a no-op.
869 *
870 * The `pkg-info` attribute `relocatable="false"` that appears either way is not
871 * this setting and does not govern it; only the `<relocate>` element does.
872 */
873export function pkgbuildComponentPlist(rootRelativeBundlePath: string): string {
874 return `<?xml version="1.0" encoding="UTF-8"?>
875<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
876<plist version="1.0">
877<array>
878 <dict>
879 <key>BundleHasStrictIdentifier</key>
880 <true/>
881 <key>BundleIsRelocatable</key>
882 <false/>
883 <key>BundleIsVersionChecked</key>
884 <false/>
885 <key>BundleOverwriteAction</key>
886 <string>upgrade</string>
887 <key>RootRelativeBundlePath</key>
888 <string>${xml(rootRelativeBundlePath)}</string>
889 </dict>
890</array>
891</plist>
892`
893}
894
895/** Build the `pkgbuild` argument list for a non-App-Store package. */
896export function pkgbuildArguments(opts: {
897 root: string
898 componentPlistPath: string
899 identifier: string
900 version: string
901 outputPath: string
902 installerIdentity?: string
903}): string[] {
904 return [
905 '--root', opts.root,
906 '--component-plist', opts.componentPlistPath,
907 '--identifier', opts.identifier,
908 '--version', opts.version,
909 '--install-location', '/',
910 ...(opts.installerIdentity ? ['--sign', opts.installerIdentity] : []),
911 opts.outputPath,
912 ]
913}
914
847915async function createPKG(opts: {
848916 appBundlePath: string
849917 outputPath: string
@@ -879,18 +947,29 @@ async function createPKG(opts: {
879947 // path it should land in.
880948 const tempDir = mkdtempSync(join(tmpdir(), 'craft-pkg-'))
881949 try {
882 const appsDir = join(tempDir, 'Applications')
883 mkdirSync(appsDir, { recursive: true })
884 cpSync(opts.appBundlePath, join(appsDir, basename(opts.appBundlePath)), { recursive: true })
885
886 const built = await runTool('pkgbuild', [
887 '--root', tempDir,
888 '--identifier', opts.identifier,
889 '--version', opts.version,
890 '--install-location', '/',
891 ...(opts.installerIdentity ? ['--sign', opts.installerIdentity] : []),
892 opts.outputPath,
893 ])
950 // A `root` child rather than tempDir itself, created 0755. mkdtemp makes
951 // its directory 0700, pkgbuild records the --root directory's own mode as
952 // the mode of `.`, and `.` under `--install-location /` is the target
953 // volume's root. Shipping a package that asks for `/` to be 0700 is not
954 // something to leave to the installer's discretion.
955 const root = join(tempDir, 'root')
956 const appsDir = join(root, 'Applications')
957 mkdirSync(appsDir, { recursive: true, mode: 0o755 })
958 chmodSync(root, 0o755)
959 const bundleName = basename(opts.appBundlePath)
960 cpSync(opts.appBundlePath, join(appsDir, bundleName), { recursive: true })
961
962 const componentPlistPath = join(tempDir, 'component.plist')
963 writeFileSync(componentPlistPath, pkgbuildComponentPlist(`Applications/${bundleName}`))
964
965 const built = await runTool('pkgbuild', pkgbuildArguments({
966 root,
967 componentPlistPath,
968 identifier: opts.identifier,
969 version: opts.version,
970 outputPath: opts.outputPath,
971 installerIdentity: opts.installerIdentity,
972 }))
894973 return built.success
895974 ? { success: true, outputPath: opts.outputPath }
896975 : { success: false, error: built.error }
scripts/native-lifecycle.tsmodified+11-0
@@ -155,10 +155,21 @@ async function exerciseMacOS(v1: PackageResult[], v2: PackageResult[]): Promise<
155155 const second = requireArtifact(v2, 'pkg')
156156 const app = '/Applications/craft-lifecycle.app'
157157 const executable = join(app, 'Contents', 'MacOS', 'craft-lifecycle')
158 // `installer` can report success and put the payload somewhere else entirely:
159 // a package built with bundle relocation enabled is redirected to wherever the
160 // system already has that bundle identifier registered. When that happened
161 // here the only symptom was `launch` throwing ENOENT, which says nothing about
162 // where the app went. The receipt does — `location:` is the path the payload
163 // actually landed at — so record it as its own step, into report.json and the
164 // uploaded evidence, every time.
165 const receipt = (label: string) => command(`receipt after ${label}`, ['pkgutil', '--pkg-info', 'dev.craft.lifecycle'])
166
158167 if (existsSync(app)) await command('remove pre-existing fixture', ['sudo', 'rm', '-rf', app])
159168 await command('install v1', ['sudo', 'installer', '-pkg', first, '-target', '/'])
169 await receipt('install v1')
160170 await command('launch v1', [executable], 'craft-lifecycle 1.0.0')
161171 await command('update to v2', ['sudo', 'installer', '-pkg', second, '-target', '/'])
172 await receipt('update to v2')
162173 await command('launch v2', [executable], 'craft-lifecycle 1.0.1')
163174 for (const step of macOSRollbackPlan(app, 'dev.craft.lifecycle', first))
164175 await command(step.name, step.argv)