diff --git a/apps/review-desktop/README.md b/apps/review-desktop/README.md index 058231458..5a4c976c3 100644 --- a/apps/review-desktop/README.md +++ b/apps/review-desktop/README.md @@ -220,9 +220,11 @@ Preview uses its own bundle identifier, URL scheme, CLI name, and data folders, so it can run beside stable without replacing the stable app or sharing its settings. Preview updates continue to use the preview feed. To return to stable, open the existing `Whiteboard.app` or install it from . -Squirrel renames an install to the update zip's folder name, so the client -sends its own folder name (`?bundle=`) and the feed answers with the matching -zip: `Review.app` installs stay `Review.app`, fresh installs are `Whiteboard.app`. +Squirrel renames an install to the update's executable name, so the client +sends its own bundle name (`?bundle=`) and the feed answers with a zip whose +bundle and executable carry that name (the Review copy is re-signed and +notarized on its own): `Review.app` installs stay `Review.app`, fresh installs +are `Whiteboard.app`. Builds from before the preview identity split installed as `Review.app`. Reinstall once from after the split so the diff --git a/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/abstractUpdateService.ts b/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/abstractUpdateService.ts index abc687357..238a6f65a 100644 --- a/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/abstractUpdateService.ts +++ b/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/abstractUpdateService.ts @@ -28,7 +28,7 @@ const LAST_KNOWN_VERSION_STORAGE_KEY = 'abstractUpdateService/lastKnownVersion'; export interface IUpdateURLOptions { readonly background?: boolean; readonly internalOrg?: string; - /** The .app folder name this install runs from (without .app). The feed serves the zip whose folder matches, so Squirrel never renames the install. */ + /** The .app folder name this install runs from (without .app). The feed serves the zip whose bundle and executable carry that name, so Squirrel never renames the install. */ readonly bundle?: string; } diff --git a/apps/review-desktop/scripts/notarize-macos.sh b/apps/review-desktop/scripts/notarize-macos.sh index 433af80cf..f87133f2a 100755 --- a/apps/review-desktop/scripts/notarize-macos.sh +++ b/apps/review-desktop/scripts/notarize-macos.sh @@ -167,15 +167,29 @@ xcrun stapler validate "$PACKAGED_APP" spctl -a -vv --type exec "$PACKAGED_APP" spctl -a -vv --type open --context context:primary-signature "$DMG" -# One update zip per bundle folder name still installed (release-channel.mjs -# lists them): each ships the same stapled app under that folder name, so -# Squirrel's rename-to-the-update's-name is a no-op for every install. +# One update zip per bundle name still installed (release-channel.mjs lists +# them). Squirrel renames an install to the update's CFBundleExecutable, so a +# zip meant for Review.app installs must carry an executable named Review: +# copy the stapled app, rename the executable, re-sign the outer bundle (the +# nested code keeps its signatures) and notarize that copy on its own. UPDATE_ZIPS=() while IFS=$'\t' read -r bundle artifact; do staged="$TEMP_ROOT/zips/$artifact/$bundle.app" mkdir -p "$(dirname "$staged")" ditto "$PACKAGED_APP" "$staged" zip="$ARTIFACT_DIR/$artifact-darwin-arm64-$VERSION.zip" + if [[ "$bundle" != "$PRODUCT_NAME" ]]; then + mv "$staged/Contents/MacOS/$PRODUCT_NAME" "$staged/Contents/MacOS/$bundle" + /usr/libexec/PlistBuddy -c "Set :CFBundleExecutable $bundle" "$staged/Contents/Info.plist" + codesign "${CODESIGN_DMG_ARGS[@]}" --options runtime \ + --entitlements "$CHECKOUT/build/darwin/entitlements/app.plist" "$staged" + codesign --verify --deep --strict --verbose=2 "$staged" + ditto -c -k --keepParent "$staged" "$TEMP_ROOT/zips/$artifact-notarize.zip" + submit_notarization "$TEMP_ROOT/zips/$artifact-notarize.zip" "$artifact" + xcrun stapler staple "$staged" + xcrun stapler validate "$staged" + spctl -a -vv --type exec "$staged" + fi ditto -c -k --keepParent "$staged" "$zip" UPDATE_ZIPS+=("$zip") done < <(node "$APP_DIR/scripts/release-channel.mjs" "$QUALITY") diff --git a/apps/review-desktop/scripts/release-channel.mjs b/apps/review-desktop/scripts/release-channel.mjs index 72e9fc2a7..ba0cefd5e 100644 --- a/apps/review-desktop/scripts/release-channel.mjs +++ b/apps/review-desktop/scripts/release-channel.mjs @@ -19,12 +19,12 @@ const RELEASE_IDENTITIES = Object.freeze({ }), }); -// Squirrel renames an install to the folder name inside the update zip, so a -// release ships one zip per folder name still installed: `bundle` is that -// folder name (minus .app), `artifact` prefixes the zip file. The first entry -// is what a client that does not name its folder receives: such a client -// predates the parameter, so it gets the pre-rename name. The DMG always -// carries the channel's nameShort. +// Squirrel renames an install to the update's executable name, so a release +// ships one zip per bundle name still installed, each with its executable +// named to match: `bundle` is that name (the .app folder and the executable), +// `artifact` prefixes the zip file. The first entry is what a client that does +// not name its bundle receives: such a client predates the parameter, so it +// gets the pre-rename name. The DMG always carries the channel's nameShort. const UPDATE_BUNDLES = Object.freeze({ stable: Object.freeze([ Object.freeze({ bundle: "Review", artifact: "Review" }), diff --git a/apps/review-desktop/scripts/validate-release-artifacts.mjs b/apps/review-desktop/scripts/validate-release-artifacts.mjs index c073a3f3c..7c77c3af5 100644 --- a/apps/review-desktop/scripts/validate-release-artifacts.mjs +++ b/apps/review-desktop/scripts/validate-release-artifacts.mjs @@ -54,11 +54,12 @@ export function buildManifest({ version, commit, payloads, now = new Date() }) { }; } -// Squirrel installs the zip's top-level folder under that name, so a zip whose -// folder does not match the bundle it is served to would rename the install. -export function assertZipFolder(zip, folder) { - // The full listing runs past execFileSync's default buffer, so reduce it - // to the distinct top-level names before it crosses the pipe. +// Squirrel renames an install to the update's CFBundleExecutable, so a zip +// served to .app installs must carry exactly that bundle with an +// executable of the same name; anything else renames the install. +export function assertZipBundle(zip, bundle) { + const folder = `${bundle}.app`; + const roots = new Set( execFileSync( "sh", @@ -74,6 +75,24 @@ export function assertZipFolder(zip, folder) { `${zip} must contain only ${folder}/, found ${[...roots].join(", ") || "nothing"}`, ); } + + const executable = execFileSync( + "sh", + [ + "-c", + 'unzip -p "$1" "$2/Contents/Info.plist" | plutil -extract CFBundleExecutable raw -o - -', + "sh", + zip, + folder, + ], + { encoding: "utf8" }, + ).trim(); + + if (executable !== bundle) { + throw new Error( + `${zip}: ${folder} runs ${JSON.stringify(executable)}, so Squirrel would rename installs to ${executable}.app; expected ${bundle}`, + ); + } } export function assertPackagedProduct(product, { commit, channel = "stable" }) { @@ -198,7 +217,7 @@ async function main() { run("xcrun", ["stapler", "validate", dmg]); for (const { bundle, file } of zips) { - assertZipFolder(file, `${bundle}.app`); + assertZipBundle(file, bundle); } const payloads = zips.map((zip) => ({ ...zip, sha256: sha256(zip.file) }));