Skip to content

security_best_practices_report.md

Latest commit

 

History

History
114 lines (85 loc) · 7.55 KB

security_best_practices_report.md

File metadata and controls

114 lines (85 loc) · 7.55 KB

Security Best Practices Report

Executive Summary

This repository is a local-only macOS SwiftUI menu bar app with a WidgetKit extension. I did not find any obvious remote-code-execution, secret-exfiltration, or network-exposure issues, largely because the app has no network stack and accepts very little untrusted input.

The main security risks are local trust-boundary issues:

  1. The app launches an external cleanup tool by absolute path without validating the target bundle identity.
  2. The main app is intentionally unsandboxed while also exposing process-management and app-launch capabilities.
  3. The "Release Memory" path can send SIGKILL to an entire process group based on broad executable-path matching.
  4. The custom URL scheme accepts any perfmonitor:// invocation and immediately activates the UI.

Method note: the requested security-best-practices skill does not ship Swift/macOS-specific reference docs, so this review uses manual Swift/macOS security analysis instead of framework-specific checklist files.

Remediation Status

  • SEC-001 mitigated in the current working tree by validating the expected DiskCleanupTool.app bundle identifier before launch and showing a feature-unavailable alert when the app is absent or untrusted.
  • SEC-003 mitigated in the current working tree by tightening Claude path matching and limiting SIGKILL to Claude-identified processes only.
  • SEC-004 mitigated in the current working tree by accepting only the exact widget deep link shape (perfmonitor://show).
  • SEC-002 remains open. Fully addressing it requires architectural work because the current app design intentionally depends on unsandboxed capabilities for process inspection and termination.

Critical

No critical findings.

High

No high-severity findings.

Medium

SEC-001: External app launch trusts a fixed filesystem path without verifying the bundle identity

Impact: clicking CLEAN UP DISK can launch arbitrary local code under the current user account if /Applications/DiskCleanupTool.app is replaced with an unexpected bundle.

  • Code: PerformanceMonitor/Views/StatsOverviewView.swift:116
  • Relevant lines:
    • let url = URL(fileURLWithPath: "/Applications/DiskCleanupTool.app")
    • NSWorkspace.shared.openApplication(at: url, configuration: NSWorkspace.OpenConfiguration())
  • Why it matters:
    • This trusts path presence alone.
    • There is no verification of bundle identifier, team identifier, code signature, or expected executable.
    • For a local utility launcher, path-only trust is a weak control.
  • Recommended fix:
    • Verify the bundle identifier before launch.
    • Prefer verifying the code signature / designated requirement if this is intended to launch a specific trusted helper app.
    • Fail closed with a user-visible error when the installed app does not match expectations.

SEC-002: The main app runs unsandboxed while exposing destructive local capabilities

Impact: if the app, a future dependency, or a future input surface is compromised, it runs with full user-level local access instead of container-scoped access.

  • Code: project.yml:35
  • Code: PerformanceMonitor/PerformanceMonitor.entitlements:5
  • Relevant lines:
    • ENABLE_APP_SANDBOX: false
    • <key>com.apple.security.app-sandbox</key><false/>
  • Why it matters:
    • The app enumerates processes, terminates apps, force-kills process groups, reads filesystem stats, and launches external apps.
    • In a non-sandboxed app, any future bug in URL handling, parsing, plug-in loading, or helper execution has a much wider blast radius.
  • Recommended fix:
    • If these capabilities are required, document the trust model explicitly and keep the unsandboxed surface as small as possible.
    • Consider moving privileged/destructive operations behind a narrower helper boundary rather than keeping the whole UI app unsandboxed.
    • Avoid adding new input surfaces to the unsandboxed target unless strictly necessary.

Low

SEC-003: Claude-process detection is path-based and kill behavior is unconditional

Impact: a local binary placed under a matching Claude-like path can be swept into the SIGKILL path, causing unintended local denial of service when the user clicks RELEASE MEMORY.

SEC-004: The custom URL scheme accepts any perfmonitor:// URL and immediately activates the UI

Impact: any local app can trigger nuisance popover activation or repeated foregrounding of the app.

  • Code: PerformanceMonitor/Info.plist:19
  • Code: PerformanceMonitor/App/PerformanceMonitorApp.swift:56
  • Why it matters:
    • The handler only checks url.scheme == "perfmonitor".
    • Host, path, and caller identity are not validated.
    • This is low severity today because the action only shows the popover, but it is still an externally triggerable local action.
  • Recommended fix:
    • Validate the exact URL shape you expect, for example perfmonitor://show.
    • Ignore unknown hosts and paths.
    • Keep the handler side-effect-free beyond the minimal UI action.

Positive Observations

  • I found no network clients, remote API calls, embedded web views, or server-facing endpoints in the repository.
  • I found no obvious secret storage, token handling, or credential persistence.
  • The widget shared store writes only basic system stats rather than sensitive user data.
  • The widget target remains sandboxed even though the main app does not.

Suggested Next Steps

  1. Fix SEC-001 first by validating the cleanup tool before launch.
  2. Decide whether the unsandboxed main app is an intentional permanent requirement; if yes, document the trust model.
  3. Harden SEC-003 before adding any more destructive process-management features.
  4. Tighten the URL handler to only accept the exact widget deep link shape.