Skip to content

Commit 85f92c0

Browse files
security: pin MASShortcut, confirm URL tasks, harden config auto-load (#1765)
Addresses three issues identified in a source-level security audit (see SECURITY_AUDIT.md for the full report): 1. [High] MASShortcut SPM dependency was pinned to `branch = master`, meaning every build pulled the latest tip with no review gate. Pin to the current HEAD revision SHA so future upstream changes require an explicit bump. 2. [Medium] `rectangle://execute-task=ignore-app/unignore-app` URL actions silently mutated the disabled-apps defaults when triggered from outside Rectangle (e.g. a web page). Require an NSAlert confirmation, surfacing the bundle-id, unless Rectangle itself is frontmost. 3. [Medium] `Config.loadFromSupportDir()` silently auto-loaded any RectangleConfig.json dropped in Application Support on launch. Add a confirmation prompt, reject symlinks and world-writable files, and apply a 1 MiB size cap to the loader (defense against OOM via giant config). Also includes SECURITY_AUDIT.md documenting the full review, including confirmation that the app contains no telemetry, analytics, or crash reporting and that the only outbound network channel is the EdDSA-signed Sparkle update feed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6aa825f commit 85f92c0

4 files changed

Lines changed: 383 additions & 6 deletions

File tree

Rectangle.xcodeproj/project.pbxproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,8 +1519,8 @@
15191519
isa = XCRemoteSwiftPackageReference;
15201520
repositoryURL = "https://github.com/rxhanson/MASShortcut";
15211521
requirement = {
1522-
branch = master;
1523-
kind = branch;
1522+
kind = revision;
1523+
revision = 2f9fbb3f959b7a683c6faaf9638d22afad37a235;
15241524
};
15251525
};
15261526
9877B63B29C8AC3600F02D74 /* XCRemoteSwiftPackageReference "Sparkle" */ = {

Rectangle/AppDelegate.swift

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,26 @@ extension AppDelegate {
644644
return isValid
645645
}
646646

647+
func confirmExecuteTask(action: String, bundleId: String) -> Bool {
648+
// Defense-in-depth: any web page or another app can trigger the
649+
// `rectangle://execute-task=ignore-app` URL with an arbitrary
650+
// bundle-id. Without confirmation this silently mutates
651+
// Rectangle's `disabledApps` defaults. Skip the prompt only
652+
// when Rectangle itself is frontmost (i.e. the user almost
653+
// certainly clicked this from inside Rectangle's own UI).
654+
if NSWorkspace.shared.frontmostApplication == NSRunningApplication.current {
655+
return true
656+
}
657+
let alert = NSAlert()
658+
alert.alertStyle = .warning
659+
alert.messageText = "Allow Rectangle URL action?".localized
660+
alert.informativeText = String(format: "An external source asked Rectangle to perform \"%@\" on app bundle id \"%@\". Allow?".localized, action, bundleId)
661+
alert.addButton(withTitle: "Allow".localized)
662+
alert.addButton(withTitle: "Cancel".localized)
663+
NSApp.activate(ignoringOtherApps: true)
664+
return alert.runModal() == .alertFirstButtonReturn
665+
}
666+
647667
for url in urls {
648668
guard
649669
let components = URLComponents(url: url, resolvingAgainstBaseURL: true),
@@ -659,11 +679,13 @@ extension AppDelegate {
659679
action?.postUrl()
660680
case ("execute-task", "ignore-app"):
661681
let bundleId = extractBundleIdParameter(fromComponents: components)
662-
guard isValidParameter(bundleId: bundleId) else { continue }
682+
guard isValidParameter(bundleId: bundleId), let bundleId else { continue }
683+
guard confirmExecuteTask(action: "ignore-app", bundleId: bundleId) else { continue }
663684
self.applicationToggle.disableApp(appBundleId: bundleId)
664685
case ("execute-task", "unignore-app"):
665686
let bundleId = extractBundleIdParameter(fromComponents: components)
666-
guard isValidParameter(bundleId: bundleId) else { continue }
687+
guard isValidParameter(bundleId: bundleId), let bundleId else { continue }
688+
guard confirmExecuteTask(action: "unignore-app", bundleId: bundleId) else { continue }
667689
self.applicationToggle.enableApp(appBundleId: bundleId)
668690
default:
669691
continue

Rectangle/PrefsWindow/Config.swift

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ extension Defaults {
6262
static func load(fileUrl: URL) {
6363
guard let dictTransformer = ValueTransformer(forName: NSValueTransformerName(rawValue: MASDictionaryTransformerName)) else { return }
6464

65+
// Size cap: legitimate configs are ~tens of KB; refuse anything that
66+
// looks abusive (defense against OOM via a giant config file).
67+
if let attrs = try? FileManager.default.attributesOfItem(atPath: fileUrl.path),
68+
let size = attrs[.size] as? NSNumber, size.intValue > 1_048_576 {
69+
return
70+
}
71+
6572
guard let jsonString = try? String(contentsOf: fileUrl, encoding: .utf8),
6673
let config = convert(jsonString: jsonString) else { return }
6774

@@ -95,14 +102,55 @@ extension Defaults {
95102

96103
let exists = try? configURL.checkResourceIsReachable()
97104
if exists == true {
105+
// Defense-in-depth: any process running as this user can drop
106+
// a RectangleConfig.json in Application Support and have it
107+
// silently applied on next launch, overwriting shortcuts and
108+
// defaults. Require the user to confirm before loading.
109+
//
110+
// We also refuse symlinks (could redirect reads elsewhere) and
111+
// any file with world-write permission (suggests tampering).
112+
let path = configURL.path
113+
let fm = FileManager.default
114+
var isSafe = true
115+
116+
if let attrs = try? fm.attributesOfItem(atPath: path) {
117+
if (attrs[.type] as? FileAttributeType) == .typeSymbolicLink {
118+
isSafe = false
119+
}
120+
if let perms = attrs[.posixPermissions] as? NSNumber,
121+
(perms.intValue & 0o002) != 0 {
122+
isSafe = false
123+
}
124+
}
125+
126+
guard isSafe else {
127+
AlertUtil.oneButtonAlert(
128+
question: "Refused to load RectangleConfig.json",
129+
text: "The configuration file at \(path) is a symlink or world-writable. Rectangle has refused to load it. Remove the file or fix its permissions and try again."
130+
)
131+
try? fm.removeItem(at: configURL)
132+
return
133+
}
134+
135+
let response = AlertUtil.twoButtonAlert(
136+
question: "Apply Rectangle configuration?",
137+
text: "A configuration file was found at \(path). Applying it will overwrite your current Rectangle shortcuts and preferences. Apply now?",
138+
confirmText: "Apply",
139+
cancelText: "Discard"
140+
)
141+
guard response == .alertFirstButtonReturn else {
142+
try? fm.removeItem(at: configURL)
143+
return
144+
}
145+
98146
load(fileUrl: configURL)
99147
do {
100148
let newFilename = "RectangleConfig\(timestamp()).json"
101149

102-
try FileManager.default.moveItem(atPath: configURL.path, toPath: rectangleSupportURL.appendingPathComponent(newFilename).path)
150+
try fm.moveItem(atPath: configURL.path, toPath: rectangleSupportURL.appendingPathComponent(newFilename).path)
103151
} catch {
104152
do {
105-
try FileManager.default.removeItem(at: configURL)
153+
try fm.removeItem(at: configURL)
106154
} catch {
107155
AlertUtil.oneButtonAlert(question: "Error after loading from Support Dir", text: "Unable to rename/remove RectangleConfig.json from \(rectangleSupportURL) after loading.")
108156
}

0 commit comments

Comments
 (0)