Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
16 changes: 15 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<project.jdk.version>25</project.jdk.version>

<!-- runtime dependencies -->
<api.version>1.7.0</api.version>
<api.version>1.8.0-SNAPSHOT</api.version>
<slf4j.version>2.0.17</slf4j.version>

<!-- test dependencies -->
Expand Down Expand Up @@ -63,6 +63,20 @@
</license>
</licenses>

<repositories>
<repository>
<name>Central Portal Snapshots</name>
<id>central-portal-snapshots</id>
<url>https://central.sonatype.com/repository/maven-snapshots/</url>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
</repositories>

<dependencies>
<dependency>
<groupId>org.cryptomator</groupId>
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
import org.cryptomator.integrations.revealpath.RevealPathService;
import org.cryptomator.integrations.tray.TrayIntegrationProvider;
import org.cryptomator.integrations.uiappearance.UiAppearanceProvider;
import org.cryptomator.integrations.update.UpdateMechanism;
import org.cryptomator.macos.autostart.MacAutoStartProvider;
import org.cryptomator.macos.keychain.MacSystemKeychainAccess;
import org.cryptomator.macos.keychain.TouchIdKeychainAccess;
import org.cryptomator.macos.revealpath.OpenCmdRevealPathService;
import org.cryptomator.macos.tray.MacTrayIntegrationProvider;
import org.cryptomator.macos.uiappearance.MacUiAppearanceProvider;
import org.cryptomator.macos.update.DmgUpdateMechanism;

module org.cryptomator.integrations.mac {
requires org.cryptomator.integrations.api;
Expand All @@ -19,4 +21,5 @@
provides RevealPathService with OpenCmdRevealPathService;
provides TrayIntegrationProvider with MacTrayIntegrationProvider;
provides UiAppearanceProvider with MacUiAppearanceProvider;
provides UpdateMechanism with DmgUpdateMechanism;
}
106 changes: 106 additions & 0 deletions src/main/java/org/cryptomator/macos/update/DmgUpdateMechanism.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package org.cryptomator.macos.update;

import org.cryptomator.integrations.common.LocalizedDisplayName;
import org.cryptomator.integrations.common.OperatingSystem;
import org.cryptomator.integrations.update.DownloadUpdateInfo;
import org.cryptomator.integrations.update.DownloadUpdateMechanism;
import org.cryptomator.integrations.update.UpdateFailedException;
import org.cryptomator.integrations.update.UpdateMechanism;
import org.cryptomator.integrations.update.UpdateStep;
import org.cryptomator.macos.common.Localization;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.InterruptedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.List;
import java.util.UUID;

@OperatingSystem(OperatingSystem.Value.MAC)
@LocalizedDisplayName(bundle = "MacIntegrationsBundle", key = "org.cryptomator.macos.update.dmg.displayName")
public class DmgUpdateMechanism extends DownloadUpdateMechanism {

private static final Logger LOG = LoggerFactory.getLogger(DmgUpdateMechanism.class);

@Override
protected DownloadUpdateInfo checkForUpdate(String currentVersion, LatestVersionResponse response) {
String suffix = switch (System.getProperty("os.arch")) {
case "aarch64", "arm64" -> "arm64.dmg";
default -> "x64.dmg";
};
var updateVersion = response.latestVersion().macVersion();
var asset = response.assets().stream().filter(a -> a.name().endsWith(suffix)).findAny().orElse(null);
if (UpdateMechanism.isUpdateAvailable(updateVersion, currentVersion) && asset != null) {
return new DownloadUpdateInfo(this, updateVersion, asset);
} else {
return null;
}
}

@Override
public UpdateStep secondStep(Path workDir, Path assetPath, DownloadUpdateInfo updateInfo) {
return UpdateStep.of(Localization.get().getString("org.cryptomator.macos.update.dmg.unpacking"), () -> this.unpack(workDir, assetPath));
}

private UpdateStep unpack(Path workDir, Path assetPath) throws IOException {
// Extract Cryptomator.app from the .dmg file
String script = """
hdiutil attach "${DMG_PATH}" -mountpoint "/Volumes/Cryptomator_${MOUNT_ID}" -nobrowse -quiet &&
cp -R "/Volumes/Cryptomator_${MOUNT_ID}/Cryptomator.app" 'Cryptomator.app' &&
hdiutil detach "/Volumes/Cryptomator_${MOUNT_ID}" -quiet
""";
Comment on lines +52 to +55
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Always detach DMG and verify signature; avoid leaks on failure.

&& skips detach on copy failure, leaving the image mounted. Add trap + strict mode. Also verify codesign/notarization before proceeding.

-		String script = """
-					hdiutil attach "${DMG_PATH}" -mountpoint "/Volumes/Cryptomator_${MOUNT_ID}" -nobrowse -quiet &&
-					cp -R "/Volumes/Cryptomator_${MOUNT_ID}/Cryptomator.app" 'Cryptomator.app' &&
-					hdiutil detach "/Volumes/Cryptomator_${MOUNT_ID}" -quiet
-					""";
+		String script = """
+					set -euo pipefail
+					MOUNT_POINT="/Volumes/Cryptomator_${MOUNT_ID}"
+					cleanup() {
+						hdiutil detach "${MOUNT_POINT}" -quiet >/dev/null 2>&1 || true
+					}
+					trap cleanup EXIT
+					hdiutil attach "${DMG_PATH}" -mountpoint "${MOUNT_POINT}" -nobrowse -quiet
+					# Prefer ditto for app bundles (preserves attrs/ACLs properly):
+					ditto "${MOUNT_POINT}/Cryptomator.app" 'Cryptomator.app'
+					# Basic signature/gatekeeper check before install:
+					codesign --verify --deep --strict 'Cryptomator.app'
+					spctl --assess --type execute -vv 'Cryptomator.app'
+					trap - EXIT
+					cleanup
+					""";

If you have an expected Team ID, also assert it (e.g., via codesign -dv --verbose=4 and grep TeamIdentifier).


🏁 Script executed:

#!/bin/bash
# Locate and display content of DmgUpdateMechanism.java to inspect the update script
fd src -t f -g 'DmgUpdateMechanism.java' --exec sed -n '1,200p' {} \;

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Find DmgUpdateMechanism.java and display lines 40–80
fd 'DmgUpdateMechanism\.java' --exec cat {} \; | head -n 80 | tail -n 41

Length of output: 1965


🏁 Script executed:

#!/bin/bash
# Check for any codesign, spctl, or signature verification in the entire codebase
rg -n 'codesign|spctl|notariz' --type java

Length of output: 0


Always ensure the DMG is detached on failure and the app is properly signed before use

  • In the unpack method (src/main/java/org/cryptomator/macos/update/DmgUpdateMechanism.java), prepend your shell script with set -euo pipefail and use
    trap 'hdiutil detach "$MOUNT_POINT" -quiet || true' EXIT
    so the volume is always unmounted, even if the copy or any command fails.
  • Prefer macOS’s ditto over cp -R to copy the app bundle (it preserves extended attributes and ACLs).
  • After copying, run
    codesign --verify --deep --strict 'Cryptomator.app'  
    spctl --assess --type execute -vv 'Cryptomator.app'
    (and optionally assert the expected TeamIdentifier via codesign -dv --verbose=4), to guard against tampered or unsigned binaries.
🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/macos/update/DmgUpdateMechanism.java around
lines 52 to 55, the shell script used to mount, copy and detach the DMG must be
hardened and the app copy verified: prepend the script with strict shell options
(set -euo pipefail) and add a trap that always attempts to detach the mount
point on EXIT (using a MOUNT_POINT variable), replace cp -R with ditto to
preserve extended attributes and ACLs when copying the .app bundle, and after
copying run codesign --verify --deep --strict and spctl --assess --type execute
-vv on the copied 'Cryptomator.app' (optionally parse codesign -dv --verbose=4
to assert TeamIdentifier) so the volume is unmounted on failure and the app is
properly signed and verifiable.

var command = List.of("/bin/zsh", "-c", script);
var processBuilder = new ProcessBuilder(command);
processBuilder.directory(workDir.toFile());
processBuilder.environment().put("DMG_PATH", assetPath.toString());
processBuilder.environment().put("MOUNT_ID", UUID.randomUUID().toString());
Process p = processBuilder.start();
try {
if (p.waitFor() != 0) {
LOG.error("Failed to extract DMG, exit code: {}, output: {}", p.exitValue(), new String(p.getErrorStream().readAllBytes()));
throw new IOException("Failed to extract DMG, exit code: " + p.exitValue());
Comment on lines +52 to +65
Copy link

@coderabbitai coderabbitai bot Nov 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Mountpoint needs to be created and cleaned up in the DMG extraction script

hdiutil attach … -mountpoint expects the target directory to exist; with the current script, the very first attach fails with No such file or directory, so the update path never succeeds. In addition, the detach command is only run when the copy succeeds, leaving the DMG mounted whenever attach or copy fails. Please create/cleanup the mountpoint and ensure we always detach.

String script = """
-				hdiutil attach "${DMG_PATH}" -mountpoint "/Volumes/Cryptomator_${MOUNT_ID}" -nobrowse -quiet &&
-				cp -R "/Volumes/Cryptomator_${MOUNT_ID}/Cryptomator.app" 'Cryptomator.app' &&
-				hdiutil detach "/Volumes/Cryptomator_${MOUNT_ID}" -quiet
+				set -e
+				MOUNT_POINT="/Volumes/Cryptomator_${MOUNT_ID}"
+				cleanup() {
+					hdiutil detach "${MOUNT_POINT}" -quiet >/dev/null 2>&1 || true
+					rmdir "${MOUNT_POINT}" >/dev/null 2>&1 || true
+				}
+				trap cleanup EXIT
+				mkdir -p "${MOUNT_POINT}"
+				hdiutil attach "${DMG_PATH}" -mountpoint "${MOUNT_POINT}" -nobrowse -quiet
+				cp -R "${MOUNT_POINT}/Cryptomator.app" 'Cryptomator.app'
+				trap - EXIT
+				cleanup
 				""";
🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/macos/update/DmgUpdateMechanism.java around
lines 52 to 65, the DMG extraction script fails because the mountpoint directory
is not created and the detach is only executed on success; update the script to
(1) enable strict error handling (set -e), (2) create the mountpoint directory
(mkdir -p), (3) install a cleanup function that always tries to detach the DMG
and remove the mountpoint, registered via trap EXIT so detach runs on failure or
success, (4) perform the attach, copy, then remove the trap and invoke cleanup
on success; replace the current one-liner script with this safe flow so attach
won't fail for missing directory and the DMG is always detached.

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't fail. macOS treats /Volumes/* different from other paths.

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

🌐 Web query:

hdiutil attach mountpoint /Volumes auto create directory macOS

💡 Result:

Short answer: hdiutil will normally create a volume directory under /Volumes automatically when you attach an image. If you supply -mountpoint or -mountroot, the target directory must already exist (so create it with mkdir -p first).

Examples:

  • Let macOS pick the name under /Volumes (auto-created):
    hdiutil attach /path/to/image.dmg. [1][2]

  • Mount to a specific path (ensure the path exists first):
    mkdir -p /Volumes/MyMount
    hdiutil attach -mountpoint /Volumes/MyMount /path/to/image.dmg. [1][3]

Notes:

  • -mountroot requires that the given directory exists; -mountrandom will create randomized subdirs under an existing path. [1][2]

Sources: hdiutil man pages and Apple discussions. [1] [2] [3]


🌐 Web query:

macOS /Volumes directory autofs automount special behavior

💡 Result:

Short summary of macOS /Volumes + autofs/automount behavior and gotchas

  • /Volumes is the system directory where macOS creates mount‑point folders for attached volumes (external disks, network mounts). If a real folder with the same name already exists (often left behind after an unclean unmount), macOS will mount the volume under a different name (e.g. "Name 1"). Delete the stale folder in /Volumes to restore the expected mountpoint. [1][2]

  • Autofs/automount creates "trigger" mount points and mounts on demand. Because autofs owns the trigger nodes, automounted shares can end up mounted at autofs-controlled locations rather than the Finder's usual mount representation — this can make them appear differently in Terminal vs Finder and can break apps that expect a specific /Volumes path. [3]

  • Since Catalina (and Big Sur+) Apple split the boot disk into a read‑only system volume and a writable Data volume. The Data volume is mounted at /System/Volumes/Data and macOS uses firmlinks to present a unified filesystem. This changed where some volume data and special firmlinks live and can affect mount resolution and tooling that assumes everything lives directly under /. (Backup/clone software and automount setups sometimes need adjustments for these firmlink/mountpoint differences.) [4][5]

  • Practical consequences and troubleshooting:

    • If automounted shares don't show under /Volumes or appear "duplicated", check /Volumes for stale directories and remove them (after ensuring no data loss). [1][2]
    • Automounter mounts may require explicit maps (auto_master/auto_*), and Kerberos/credential issues can cause automounted SMB shares to appear mounted but fail to list. Check automount maps, credentials, and kerberos tickets. [3]
    • For Catalina/Big Sur+ issues (firmlink/mountpoint oddities, .VolumeIcon/.Volume‑Data behavior, backup tools), review how the Data volume is mounted at /System/Volumes/Data and adapt mount targets accordingly. [4][5]

Sources

  • Ghost/stale mount behavior and /Volumes guidance. [1][2]
  • How autofs/automount works and automounter behavior. [3]
  • Catalina/Big Sur APFS volume group, firmlinks, and /System/Volumes/Data effects. [4][5]

If you want, I can:

  • show exact files to inspect (/etc/auto_master, /etc/auto_smb, /etc/auto_*),
  • give sample automount lines that mount directly under /Volumes,
  • or help diagnose a specific automount problem if you paste your auto_master / automount map and a sample mount command.

@overheadhunter, thank you for the clarification! I found documentation indicating that -mountpoint typically requires the directory to exist first, but if you've tested this and /Volumes has special handling that makes it work without pre-creation, I'll defer to your experience.

However, there's still a separate concern about cleanup: the current script uses && chaining, which means if either hdiutil attach or cp fails, the hdiutil detach won't run, potentially leaving the DMG mounted. Would it make sense to add a trap or similar mechanism to ensure detachment even on failure? For example:

trap 'hdiutil detach "/Volumes/Cryptomator_${MOUNT_ID}" -quiet 2>/dev/null || true' EXIT
hdiutil attach "${DMG_PATH}" -mountpoint "/Volumes/Cryptomator_${MOUNT_ID}" -nobrowse -quiet
cp -R "/Volumes/Cryptomator_${MOUNT_ID}/Cryptomator.app" 'Cryptomator.app'

This would ensure the volume is always detached regardless of whether the copy succeeds or fails. What do you think?

}
LOG.debug("Update ready: {}", workDir.resolve("Cryptomator.app"));
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new InterruptedIOException("Failed to extract DMG, interrupted");
}
return UpdateStep.of(Localization.get().getString("org.cryptomator.macos.update.dmg.restarting"), () -> this.restart(workDir));
}

public UpdateStep restart(Path workDir) throws IllegalStateException, IOException {
String selfPath = ProcessHandle.current().info().command().orElse("");
String installPath;
if (selfPath.startsWith("/Applications/Cryptomator.app")) {
installPath = "/Applications/Cryptomator.app";
} else if (selfPath.contains("/Cryptomator.app/")) {
installPath = selfPath.substring(0, selfPath.indexOf("/Cryptomator.app/")) + "/Cryptomator.app";
} else {
throw new UpdateFailedException("Cannot determine destination path for Cryptomator.app, current path: " + selfPath);
}
LOG.info("Restarting to apply Update in {} now...", workDir);
String script = """
while kill -0 ${CRYPTOMATOR_PID} 2> /dev/null; do sleep 0.2; done;
if [ -d "${CRYPTOMATOR_INSTALL_PATH}" ]; then
echo "Removing old installation at ${CRYPTOMATOR_INSTALL_PATH}";
rm -rf "${CRYPTOMATOR_INSTALL_PATH}"
fi
mv 'Cryptomator.app' "${CRYPTOMATOR_INSTALL_PATH}";
open -a "${CRYPTOMATOR_INSTALL_PATH}";
""";
Files.writeString(workDir.resolve("install.sh"), script, StandardCharsets.US_ASCII, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW);
var command = List.of("bash", "-c", "/usr/bin/nohup bash install.sh >install.log 2>&1 &");
var processBuilder = new ProcessBuilder(command);
processBuilder.directory(workDir.toFile());
processBuilder.environment().put("CRYPTOMATOR_PID", String.valueOf(ProcessHandle.current().pid()));
processBuilder.environment().put("CRYPTOMATOR_INSTALL_PATH", installPath);
processBuilder.start();

return UpdateStep.EXIT;
}

}
5 changes: 4 additions & 1 deletion src/main/resources/MacIntegrationsBundle.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
org.cryptomator.macos.keychain.displayName=macOS Keychain
org.cryptomator.macos.keychain.touchIdDisplayName=Touch ID
org.cryptomator.macos.keychain.touchIdDisplayName=Touch ID
org.cryptomator.macos.update.dmg.displayName=Download .dmg file
org.cryptomator.macos.update.dmg.unpacking=Unpacking...
org.cryptomator.macos.update.dmg.restarting=Restarting...