Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions apps/desktop/src/macCodeSigning.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, expect, it } from "vitest";

import { hasDeveloperIdApplicationAuthority } from "./macCodeSigning";

describe("hasDeveloperIdApplicationAuthority", () => {
it("matches a Developer ID Application authority line", () => {
expect(
hasDeveloperIdApplicationAuthority(`Executable=/Applications/T3 Code.app/Contents/MacOS/T3 Code
Identifier=com.t3tools.t3code
Authority=Developer ID Application: T3 Tools, Inc. (ABCDE12345)
Authority=Developer ID Certification Authority
Authority=Apple Root CA`),
).toBe(true);
});

it("ignores ad hoc signatures and unsigned output", () => {
expect(
hasDeveloperIdApplicationAuthority(`Executable=/Applications/T3 Code.app/Contents/MacOS/T3 Code
Signature=adhoc`),
).toBe(false);
expect(hasDeveloperIdApplicationAuthority("code object is not signed at all")).toBe(false);
});
});
5 changes: 5 additions & 0 deletions apps/desktop/src/macCodeSigning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function hasDeveloperIdApplicationAuthority(value: string): boolean {
return value
.split(/\r?\n/)
.some((line) => line.trim().startsWith("Authority=Developer ID Application:"));
}
62 changes: 62 additions & 0 deletions apps/desktop/src/macUpdateInstaller.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import * as FS from "node:fs";
import * as OS from "node:os";
import * as Path from "node:path";

import { afterEach, describe, expect, it } from "vitest";

import {
buildMacManualUpdateInstallScript,
findFirstAppBundlePath,
resolveDownloadedMacUpdateZipPath,
} from "./macUpdateInstaller";

const tempDirs: string[] = [];

afterEach(() => {
for (const dir of tempDirs.splice(0)) {
FS.rmSync(dir, { recursive: true, force: true });
}
});

describe("resolveDownloadedMacUpdateZipPath", () => {
it("returns the downloaded zip path", () => {
expect(resolveDownloadedMacUpdateZipPath(["/tmp/update.zip", "/tmp/update.blockmap"])).toBe(
"/tmp/update.zip",
);
});

it("returns null when no zip exists", () => {
expect(resolveDownloadedMacUpdateZipPath(["/tmp/update.blockmap"])).toBeNull();
});
});

describe("findFirstAppBundlePath", () => {
it("finds an extracted app bundle recursively", () => {
const rootDir = FS.mkdtempSync(Path.join(OS.tmpdir(), "t3-mac-update-"));
tempDirs.push(rootDir);

const appPath = Path.join(rootDir, "nested", "T3 Code.app");
FS.mkdirSync(appPath, { recursive: true });

expect(findFirstAppBundlePath(rootDir)).toBe(appPath);
});
});

describe("buildMacManualUpdateInstallScript", () => {
it("builds a detached installer script with admin fallback", () => {
const script = buildMacManualUpdateInstallScript({
appPid: 123,
sourceAppPath: "/tmp/T3 Code's Update.app",
targetAppPath: "/Applications/T3 Code.app",
stagingDir: "/tmp/t3-stage",
});

expect(script).toContain("APP_PID=123");
expect(script).toContain("wait_for_app_exit");
expect(script).toContain("/usr/bin/ditto");
expect(script).toContain("/usr/bin/osascript");
expect(script).toContain(`SOURCE_APP='/tmp/T3 Code'\\''s Update.app'`);
expect(script).toContain(`TARGET_APP='/Applications/T3 Code.app'`);
expect(script).toContain('/usr/bin/open -n "$TARGET_APP"');
});
});
89 changes: 89 additions & 0 deletions apps/desktop/src/macUpdateInstaller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import * as FS from "node:fs";
import * as Path from "node:path";

function shellQuote(value: string): string {
return `'${value.replaceAll("'", `'\\''`)}'`;
}

export function resolveDownloadedMacUpdateZipPath(
downloadedFiles: ReadonlyArray<string>,
): string | null {
for (const file of downloadedFiles) {
if (file.toLowerCase().endsWith(".zip")) {
return file;
}
}
return null;
}

export function findFirstAppBundlePath(rootDir: string): string | null {
const queue = [rootDir];

while (queue.length > 0) {
const currentDir = queue.shift();
if (!currentDir) {
continue;
}

for (const entry of FS.readdirSync(currentDir, { withFileTypes: true })) {
const entryPath = Path.join(currentDir, entry.name);
if (entry.isDirectory() && entry.name.endsWith(".app")) {
return entryPath;
}
if (entry.isDirectory()) {
queue.push(entryPath);
}
}
}

return null;
}

export function buildMacManualUpdateInstallScript(args: {
appPid: number;
sourceAppPath: string;
targetAppPath: string;
stagingDir: string;
}): string {
const sourceAppPath = shellQuote(args.sourceAppPath);
const targetAppPath = shellQuote(args.targetAppPath);
const stagingDir = shellQuote(args.stagingDir);

return `#!/bin/sh
set -eu
APP_PID=${args.appPid}
SOURCE_APP=${sourceAppPath}
TARGET_APP=${targetAppPath}
STAGING_DIR=${stagingDir}

cleanup() {
/bin/rm -rf "$STAGING_DIR"
}

trap cleanup EXIT

wait_for_app_exit() {
while /bin/kill -0 "$APP_PID" 2>/dev/null; do
/bin/sleep 0.2
done
}

install_update() {
/bin/rm -rf "$TARGET_APP"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium]

The generated installer removes the currently installed bundle before verifying the new bundle has been copied successfully, so a copy failure (for example disk full or I/O error during ditto) can leave users with no runnable app at the target path. This creates a real failure-mode regression in the unsigned-update path. Use a two-phase replace: copy to a temporary sibling path, verify success, then atomically swap (or move) into place and only then remove the old bundle. ```sh

apps/desktop/src/macUpdateInstaller.ts

install_update() {
/bin/rm -rf "$TARGET_APP"
/usr/bin/ditto "$SOURCE_APP" "$TARGET_APP"
}

/usr/bin/ditto "$SOURCE_APP" "$TARGET_APP"
}

wait_for_app_exit

if ! install_update >/dev/null 2>&1; then
export SOURCE_APP TARGET_APP
/usr/bin/osascript <<'APPLESCRIPT'
set sourceApp to system attribute "SOURCE_APP"
set targetApp to system attribute "TARGET_APP"
do shell script "/bin/rm -rf " & quoted form of targetApp & " && /usr/bin/ditto " & quoted form of sourceApp & " " & quoted form of targetApp with administrator privileges
APPLESCRIPT
Comment on lines +71 to +84
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Don't remove the installed app before the replacement copy has succeeded.

Both install paths delete "$TARGET_APP" before running ditto. If the copy then fails (corrupt archive, disk full, interrupted copy), the current bundle is already gone and there is nothing left to relaunch. Copy into a sibling temp bundle first, then swap it into place only after the new bundle is complete, keeping the old bundle as rollback until the move succeeds.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/macUpdateInstaller.ts` around lines 71 - 84, The
install_update routine currently removes TARGET_APP before copying, risking loss
if ditto fails; change install_update (and the osascript fallback) to copy
SOURCE_APP into a sibling temporary bundle (e.g., TARGET_APP+".tmp" or use
mktemp) first, verify the copy completed successfully, then atomically
rename/move the temp bundle over TARGET_APP (or use /bin/mv) to replace it; keep
TARGET_APP until the move succeeds so the old app is available for rollback, and
ensure wait_for_app_exit is still invoked before the final swap; update
references to SOURCE_APP and TARGET_APP in both the shell and osascript paths
accordingly.

fi

/usr/bin/open -n "$TARGET_APP"
`;
}
Loading
Loading