Skip to content
Open
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
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import PackageDescription
let package = Package(
name: "AutoUp",
platforms: [
.macOS(.v13)
.macOS("13.3")
],
products: [
.executable(
Expand Down
136 changes: 136 additions & 0 deletions Sources/Core/Installer.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import Foundation

enum InstallerError: LocalizedError {
case noAppFound
case dmgAttachFailed(Int32)
case pkgInstallFailed(Int32)
case codesignFailed
case backupFailed

var errorDescription: String? {
switch self {
case .noAppFound:
return "Couldn't find the app in the download"
case .dmgAttachFailed(let code):
return "DMG mount failed with code \(code)"
case .pkgInstallFailed(let code):
return "PKG install failed with code \(code)"
case .codesignFailed:
return "App signature verification failed"
case .backupFailed:
return "Couldn't backup current version"
}
}
}

enum Installer {
static func installZIP(from zipURL: URL, toApplications name: String, bundleID: String, currentVersion: String) throws {
// Create backup first
let currentAppPath = "/Applications/\(name).app"
if FileManager.default.fileExists(atPath: currentAppPath) {
_ = try? SecurityChecks.backup(appPath: currentAppPath, bundleID: bundleID, version: currentVersion)

Choose a reason for hiding this comment

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

critical

The backup operation uses try?, which silences any errors. If SecurityChecks.backup fails, the installation proceeds without a successful backup. This could lead to data loss if the subsequent installation fails, as there would be no backup to roll back to. The backup operation should be allowed to throw an error to halt the installation process if it fails.

Suggested change
_ = try? SecurityChecks.backup(appPath: currentAppPath, bundleID: bundleID, version: currentVersion)
try SecurityChecks.backup(appPath: currentAppPath, bundleID: bundleID, version: currentVersion)

}

let tmp = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
try FileManager.default.createDirectory(at: tmp, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: tmp) }

_ = try run("/usr/bin/unzip", ["-qq", zipURL.path, "-d", tmp.path])
let app = try findApp(in: tmp)

// Verify codesign before installing
guard SecurityChecks.verifyCodeSign(app.path) else {
throw InstallerError.codesignFailed
}

try moveToApplications(app)
}

static func installDMG(from dmgURL: URL, bundleID: String, currentVersion: String) throws {
// Create backup first
let apps = try? FileManager.default.contentsOfDirectory(atPath: "/Applications")
let currentAppPath = apps?.first { $0.hasSuffix(".app") && Bundle(path: "/Applications/\($0)")?.bundleIdentifier == bundleID }
Comment on lines +51 to +52

Choose a reason for hiding this comment

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

medium

Finding the current application path by iterating through all files in /Applications, checking for a .app suffix, and then initializing a Bundle for each to check its bundleIdentifier is inefficient. This can be slow if the user has many applications. Additionally, this only checks the system-wide /Applications folder and not the user's ~/Applications folder. A more efficient approach might be to use Spotlight's metadata APIs (NSMetadataQuery) to find an application by its bundle ID directly.


if let appPath = currentAppPath {
let fullPath = "/Applications/\(appPath)"
_ = try? SecurityChecks.backup(appPath: fullPath, bundleID: bundleID, version: currentVersion)

Choose a reason for hiding this comment

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

critical

Similar to installZIP, the backup operation here uses try?, which silences any errors. If SecurityChecks.backup fails, the installation proceeds without a successful backup. This could lead to data loss if the subsequent installation fails. The backup operation should be allowed to throw an error to halt the installation process if it fails.

Suggested change
_ = try? SecurityChecks.backup(appPath: fullPath, bundleID: bundleID, version: currentVersion)
try SecurityChecks.backup(appPath: fullPath, bundleID: bundleID, version: currentVersion)

}

let (code, out) = try run("/usr/bin/hdiutil", ["attach", "-nobrowse", "-quiet", dmgURL.path])
guard code == 0 else {
throw InstallerError.dmgAttachFailed(code)
}

guard let mount = out.split(separator: "\t").last.map(String.init) else {
throw InstallerError.dmgAttachFailed(-1)
}

defer { _ = try? run("/usr/bin/hdiutil", ["detach", "-quiet", mount]) }

let app = try findApp(in: URL(fileURLWithPath: mount))

// Verify codesign before installing
guard SecurityChecks.verifyCodeSign(app.path) else {
throw InstallerError.codesignFailed
}

try moveToApplications(app)
}

static func installPKG(from pkgURL: URL) throws {
let (code, _) = try run("/usr/sbin/installer", ["-pkg", pkgURL.path, "-target", "/"])
guard code == 0 else {
throw InstallerError.pkgInstallFailed(code)
}
}

// MARK: - Private Helpers

private static func findApp(in dir: URL) throws -> URL {
let items = try FileManager.default.contentsOfDirectory(at: dir, includingPropertiesForKeys: nil)
if let app = items.first(where: { $0.pathExtension == "app" }) {
return app
}

// Recursive search in case of subfolders
for url in items where url.hasDirectoryPath {
if let app = try? findApp(in: url) {
return app
}
}

throw InstallerError.noAppFound
}

private static func moveToApplications(_ src: URL) throws {
let dst = URL(fileURLWithPath: "/Applications").appendingPathComponent(src.lastPathComponent)

if FileManager.default.fileExists(atPath: dst.path) {
try FileManager.default.removeItem(at: dst)
}

try FileManager.default.copyItem(at: src, to: dst)

// Remove quarantine if present
_ = SecurityChecks.removeQuarantine(dst.path)
}

@discardableResult
private static func run(_ bin: String, _ args: [String]) throws -> (Int32, String) {
let process = Process()
process.executableURL = URL(fileURLWithPath: bin)
process.arguments = args

let pipe = Pipe()
process.standardOutput = pipe
process.standardError = pipe

try process.run()
process.waitUntilExit()

let data = pipe.fileHandleForReading.readDataToEndOfFile()
let output = String(data: data, encoding: .utf8) ?? ""

return (process.terminationStatus, output)
}
Comment on lines +119 to +135

Choose a reason for hiding this comment

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

medium

The run helper function combines standardOutput and standardError into a single pipe. While convenient, this can make it difficult to parse the output of a command if it also writes to standard error. For example, hdiutil might print warnings to stderr that could interfere with parsing the mount point from stdout. It would be more robust to capture stdout and stderr separately. This would require changing the function to return stderr as well and updating the call sites.

}
64 changes: 64 additions & 0 deletions Sources/Core/Rollback.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import Foundation

enum Rollback {
static func latestBackup(bundleID: String) -> URL? {
let base = URL(fileURLWithPath: NSHomeDirectory()).appendingPathComponent("Library/Application Support/AutoUp/Backups/\(bundleID)")
guard let entries = try? FileManager.default.contentsOfDirectory(at: base, includingPropertiesForKeys: [.creationDateKey], options: .skipsHiddenFiles) else {
return nil
}

return entries.sorted { (a, b) in
let dateA = (try? a.resourceValues(forKeys: [.creationDateKey]).creationDate) ?? .distantPast
let dateB = (try? b.resourceValues(forKeys: [.creationDateKey]).creationDate) ?? .distantPast
return dateA > dateB
}.first?.appendingPathComponent("\(bundleID).app")
Copy link

Choose a reason for hiding this comment

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

Bug: Backup Filename Mismatch Causes Rollback Failure

Rollback fails because SecurityChecks.backup saves apps by their original filename, but Rollback.latestBackup expects them to be named after the bundle ID. This mismatch prevents finding backed-up applications.

Additional Locations (1)

Fix in Cursor Fix in Web

Choose a reason for hiding this comment

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

critical

The path to the backed-up application is constructed using the bundleID. However, the backup logic in SecurityChecks.backup preserves the original application's filename (e.g., AppName.app), which is not the same as the bundle ID. This mismatch will cause latestBackup(bundleID:) to return an incorrect URL, and consequently, the rollback will fail to find the backup. To fix this, after finding the latest version directory, you should list its contents to find the actual .app bundle within it instead of assuming its name is \(bundleID).app.

}

static func restoreBackup(bundleID: String, to appName: String) throws -> Bool {
guard let backupURL = latestBackup(bundleID: bundleID) else {
return false
}

let currentAppPath = "/Applications/\(appName).app"
let currentAppURL = URL(fileURLWithPath: currentAppPath)

// Remove current version
if FileManager.default.fileExists(atPath: currentAppPath) {
try FileManager.default.removeItem(at: currentAppURL)
}

// Copy backup to Applications
try FileManager.default.copyItem(at: backupURL, to: currentAppURL)
Copy link

Choose a reason for hiding this comment

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

Bug: App Replacement Risks Data Loss

The app replacement and backup logic uses an unsafe "remove then copy" pattern. This means the existing application or previous backup is removed before the new version is copied. If the copy operation fails, the original item is permanently lost, potentially leaving the user without an app or a recoverable backup.

Additional Locations (2)

Fix in Cursor Fix in Web


// Remove quarantine if present
_ = SecurityChecks.removeQuarantine(currentAppPath)

return true
}

static func listAvailableBackups(bundleID: String) -> [(version: String, date: Date)] {
let base = URL(fileURLWithPath: NSHomeDirectory()).appendingPathComponent("Library/Application Support/AutoUp/Backups/\(bundleID)")
guard let entries = try? FileManager.default.contentsOfDirectory(at: base, includingPropertiesForKeys: [.creationDateKey], options: .skipsHiddenFiles) else {
return []
}

return entries.compactMap { entry in
guard let date = try? entry.resourceValues(forKeys: [.creationDateKey]).creationDate else {
return nil
}
let version = entry.lastPathComponent
return (version: version, date: date)
}.sorted { $0.date > $1.date }
}

static func cleanOldBackups(bundleID: String, keepLatest: Int = 3) {
let backups = listAvailableBackups(bundleID: bundleID)
let toDelete = backups.dropFirst(keepLatest)

for backup in toDelete {
let backupPath = URL(fileURLWithPath: NSHomeDirectory())
.appendingPathComponent("Library/Application Support/AutoUp/Backups/\(bundleID)/\(backup.version)")
try? FileManager.default.removeItem(at: backupPath)
}
}
}
41 changes: 41 additions & 0 deletions Sources/Core/SecurityChecks.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import Foundation

enum SecurityChecks {
static func backup(appPath: String, bundleID: String, version: String) throws -> URL {
let base = URL(fileURLWithPath: NSHomeDirectory()).appendingPathComponent("Library/Application Support/AutoUp/Backups/\(bundleID)/\(version)")
try FileManager.default.createDirectory(at: base, withIntermediateDirectories: true)
let dest = base.appendingPathComponent((appPath as NSString).lastPathComponent)
if FileManager.default.fileExists(atPath: dest.path) {
try? FileManager.default.removeItem(at: dest)
}
try FileManager.default.copyItem(at: URL(fileURLWithPath: appPath), to: dest)
return dest
}

static func verifyCodeSign(_ appPath: String) -> Bool {
let task = Process()
task.executableURL = URL(fileURLWithPath: "/usr/bin/codesign")
task.arguments = ["--verify", "--deep", "--strict", appPath]
try? task.run()
task.waitUntilExit()
return task.terminationStatus == 0
Comment on lines +19 to +21

Choose a reason for hiding this comment

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

high

The try? task.run() call silently ignores potential errors during process execution, for example, if the codesign command is not found. If task.run() throws, the process is never started, and task.terminationStatus remains 0, causing the function to incorrectly return true. For a security-critical function, it's crucial to handle errors from task.run() explicitly.

Suggested change
try? task.run()
task.waitUntilExit()
return task.terminationStatus == 0
do {
try task.run()
task.waitUntilExit()
return task.terminationStatus == 0
} catch {
return false
}

}

static func getQuarantineStatus(_ appPath: String) -> Bool {
let task = Process()
task.executableURL = URL(fileURLWithPath: "/usr/bin/xattr")
task.arguments = ["-p", "com.apple.quarantine", appPath]
try? task.run()
task.waitUntilExit()
return task.terminationStatus == 0
Comment on lines +28 to +30

Choose a reason for hiding this comment

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

high

Similar to verifyCodeSign, try? task.run() is used here, which can mask errors if xattr fails to launch. If task.run() throws, task.terminationStatus will be 0, and this function will incorrectly return true. The error should be handled to ensure the function's reliability.

Suggested change
try? task.run()
task.waitUntilExit()
return task.terminationStatus == 0
do {
try task.run()
task.waitUntilExit()
return task.terminationStatus == 0
} catch {
return false
}

}

static func removeQuarantine(_ appPath: String) -> Bool {
let task = Process()
task.executableURL = URL(fileURLWithPath: "/usr/bin/xattr")
task.arguments = ["-d", "com.apple.quarantine", appPath]
try? task.run()
task.waitUntilExit()
return task.terminationStatus == 0
Comment on lines +37 to +39

Choose a reason for hiding this comment

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

high

Similar to the other functions in this file, try? task.run() is used here, which can mask errors if xattr fails to launch. This could lead to a failure to remove the quarantine attribute while the function incorrectly reports success. If task.run() throws, task.terminationStatus will be 0, and this function will incorrectly return true.

        do {
            try task.run()
            task.waitUntilExit()
            return task.terminationStatus == 0
        } catch {
            return false
        }

}
}
35 changes: 35 additions & 0 deletions Sources/Core/UpdateError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import Foundation

// Actor-Observer bias: Frame errors as situational, not user failures
struct UpdateError: LocalizedError {
let reason: String
var errorDescription: String? { reason }

static func friendly(_ error: Error) -> UpdateError {
let description = String(describing: error).lowercased()

// Use Actor-Observer bias: blame the situation, not the user
if description.contains("codesign") || description.contains("signature") {
return UpdateError(reason: "Looks like the app's signature couldn't be verified. Your previous version is safe. Try installing manually from the developer.")
}

if description.contains("permission") || description.contains("access") {
return UpdateError(reason: "Auto-Up needs permission to replace the app. Grant Full Disk Access in Settings → Privacy & Security → Privacy.")
}

if description.contains("network") || description.contains("timeout") {
return UpdateError(reason: "Network seems slow — we'll retry in 2 minutes. Your apps are still protected.")
}

if description.contains("disk") || description.contains("space") {
return UpdateError(reason: "Looks like disk space is running low. Free up some space and try again.")
}

if description.contains("dmg") || description.contains("mount") {
return UpdateError(reason: "The download file seems corrupted. We'll try downloading again automatically.")
}

// Default friendly message
return UpdateError(reason: "Update temporarily unavailable. We've kept your previous version safe. You can retry or update manually.")
}
}
10 changes: 10 additions & 0 deletions Sources/Core/Versioning.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Foundation

enum Versioning {
// Numeric-aware compare: "1.10" > "1.9", strips leading "v"
static func isNewer(_ latest: String, than current: String) -> Bool {
let a = latest.trimmingCharacters(in: .whitespacesAndNewlines).trimmingCharacters(in: CharacterSet(charactersIn: "vV"))
let b = current.trimmingCharacters(in: .whitespacesAndNewlines).trimmingCharacters(in: CharacterSet(charactersIn: "vV"))
return a.compare(b, options: [.numeric, .caseInsensitive]) == .orderedDescending
}
}
35 changes: 35 additions & 0 deletions Sources/Services/Telemetry.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import Foundation
import PostHog

enum Telemetry {
static func configure(enabled: Bool, apiKey: String) {
if enabled {
PostHogSDK.shared.setup(apiKey: apiKey, host: URL(string:"https://app.posthog.com")!)

Choose a reason for hiding this comment

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

medium

Force-unwrapping the URL with ! can lead to a crash if the URL string is ever invalid. Although it's a constant here, it's safer to handle the optional gracefully, for instance by using guard let and logging an error if the URL is invalid.

PostHogSDK.shared.optIn()
} else {
PostHogSDK.shared.optOut()
}
}

static func track(_ name: String, props: [String: Any] = [:]) {
guard UserDefaults.standard.bool(forKey: "telemetry_enabled") else { return }
Copy link

Choose a reason for hiding this comment

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

Bug: Telemetry Setting Mismatch

The Telemetry.track() function checks for the "telemetry_enabled" key in UserDefaults, but the SettingsView uses "telemetryEnabled" for the user's preference. This key mismatch means telemetry always remains disabled, regardless of the user's setting.

Fix in Cursor Fix in Web

PostHogSDK.shared.capture(event: name, properties: props)
}

// Bias-driven events for measuring UX improvements
static func trackBiasEvent(_ biasType: String, action: String, value: Any? = nil) {
var props: [String: Any] = [
"bias_type": biasType,
"action": action
]
if let value = value {
props["value"] = value
}
track("bias_interaction", props: props)
}
}

// Usage examples:
// Telemetry.trackBiasEvent("anchoring", "pricing_viewed", "yearly_selected")
// Telemetry.trackBiasEvent("loss_aversion", "security_warning_shown", securityCount)
// Telemetry.trackBiasEvent("social_proof", "user_count_viewed", 3218)
Loading