From 8594dc31116204ac31ea7d862d9ec21c0a4589d7 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Thu, 24 Sep 2026 12:07:44 -0400 Subject: [PATCH 1/2] Name the executable in each update zip after its bundle Squirrel renames an install to the update's CFBundleExecutable, not to the zip's folder name (SQRLInstaller renamedTargetIfNeeded). The Review zip from #549 carried Review.app with an executable still called Whiteboard, so a 0.0.33 Review.app updating to 0.0.34 was renamed to Whiteboard.app, its Dock tile broke, and ShipIt's relaunch failed at the old path. The runner smoke passed only because its starting copy already had mismatched names, which disables the rename before that logic runs. For each zip whose bundle name differs from the build's, rename the executable to match, fix CFBundleExecutable, re-sign the outer bundle with the same identity and entitlements (nested code keeps its signatures), and notarize and staple that copy on its own. The artifact validator now rejects a zip whose executable name differs from its bundle name, which is what would have caught 0.0.34. Agent-Session: aad32659-bbbe-45b2-b163-317afc1a6a83 Agent-Session: 86ff23ef-37b3-4d9f-803a-8ceb8131e905 Agent-Session: d09f44bd-8378-4504-9cc6-dabf1d8c8bfd Agent-Session: 3ccb2131-4dfc-455e-9130-f3ab322987f0 Agent-Session: a704726b-4b5e-4b6a-86b8-558acae718b8 Agent-Session: 5705d2ed-c03d-4268-a647-86ef3e2e5719 --- apps/review-desktop/README.md | 8 +++-- .../electron-main/abstractUpdateService.ts | 2 +- apps/review-desktop/scripts/notarize-macos.sh | 20 +++++++++++-- .../scripts/release-channel.mjs | 12 ++++---- .../scripts/validate-release-artifacts.mjs | 30 +++++++++++++++---- 5 files changed, 53 insertions(+), 19 deletions(-) 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..e258f72ed 100644 --- a/apps/review-desktop/scripts/validate-release-artifacts.mjs +++ b/apps/review-desktop/scripts/validate-release-artifacts.mjs @@ -54,11 +54,11 @@ 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 +74,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 +216,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) })); From f7938facf97837209cdd58b876ae248471cfa189 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Thu, 24 Sep 2026 12:09:07 -0400 Subject: [PATCH 2/2] Add the blank line require-readable-spacing wants Agent-Session: aad32659-bbbe-45b2-b163-317afc1a6a83 Agent-Session: 86ff23ef-37b3-4d9f-803a-8ceb8131e905 Agent-Session: d09f44bd-8378-4504-9cc6-dabf1d8c8bfd Agent-Session: 3ccb2131-4dfc-455e-9130-f3ab322987f0 Agent-Session: a704726b-4b5e-4b6a-86b8-558acae718b8 Agent-Session: 5705d2ed-c03d-4268-a647-86ef3e2e5719 --- apps/review-desktop/scripts/validate-release-artifacts.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/review-desktop/scripts/validate-release-artifacts.mjs b/apps/review-desktop/scripts/validate-release-artifacts.mjs index e258f72ed..7c77c3af5 100644 --- a/apps/review-desktop/scripts/validate-release-artifacts.mjs +++ b/apps/review-desktop/scripts/validate-release-artifacts.mjs @@ -59,6 +59,7 @@ export function buildManifest({ version, commit, payloads, now = new Date() }) { // 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",