Security audit: pin MASShortcut, confirm URL tasks, harden config auto-load#1765
Merged
Merged
Conversation
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>
Owner
|
Thanks! I appreciate the audit. I tested out the additional checks and they looked good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is the result of a source-level security audit of Rectangle. The full report is included as
SECURITY_AUDIT.mdat the repo root and explains the methodology, every finding (with severity), what was changed, and what was deliberately left for follow-up.TL;DR — outbound data
The audit confirms what the README implies: Rectangle contains no telemetry, no analytics, and no crash reporting. The only outbound network channel is the EdDSA-signed Sparkle update feed at
https://rectangleapp.com/downloads/updates.xml(system profiling is off by default —SUSendProfileInfo/SUEnableSystemProfilingare not set).Fixes in this PR
[High] Supply chain — pin
MASShortcut.Rectangle.xcodeproj/project.pbxprojhadMASShortcutpinned tobranch = master, so every build silently pulled the latest tip with no review gate. Pinned to the current HEAD commit (2f9fbb3f959b7a683c6faaf9638d22afad37a235) viakind = revision. Recommend follow-up: tag MASShortcut releases upstream and switch toupToNextMajorVersion, matching how Sparkle is configured.[Medium] URL handler — confirm
execute-taskactions.rectangle://execute-task?name=ignore-app&app-bundle-id=...(andunignore-app) previously mutated the disabled-apps defaults silently from any URL source, including a web page. Added anNSAlertconfirmation showing the requested action and bundle-id. The confirmation is skipped only when Rectangle itself is frontmost, so the in-app/Stream-Deck/Shortcuts.app workflows are unchanged.execute-actionis intentionally left alone — those actions are reversible and silent execution is the documented contract.[Medium]
Config.loadFromSupportDir()— confirm + harden. On every launch Rectangle auto-loaded any~/Library/Application Support/Rectangle/RectangleConfig.jsonand applied it (overwriting shortcuts and the entire defaults set), so any process running as the user could rewrite Rectangle config silently. Now:load(fileUrl:)so both auto-load and the explicit Import button are protected from OOM via a giant config file.Not changed in this PR (low severity, documented in
SECURITY_AUDIT.md)codesign --force --deepin.github/workflows/build.yml(Apple-deprecated; tangentiallyruns-on: macos-26is also not a valid runner label).SECURITY.mdwith a disclosure address.Verification
plutil -lintreports the modifiedproject.pbxprojis valid; brace count is balanced.xcodebuildarchive build was attempted from the audit machine (no Xcode + no signing identity). Maintainer CI will exercise the full build on this PR.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com