Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ enum LocalProcessRunner {
"PCH_PINNED_WHITELIST",
"PCH_STORAGE_DU_TIMEOUT",
"PCH_STORAGE_TOTAL_DU_BUDGET",
"PCH_STORAGE_WATCH_APP_BUNDLE",
"PCH_STORAGE_WATCH_SCRIPT",
"PCH_STORAGE_WATCH_SHA256",
"VT_API_KEY",
Expand Down
12 changes: 11 additions & 1 deletion macos/Modore/Sources/Modore/Services/StorageWatchService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,20 @@ enum StorageWatchService {
return values
}

/// `expectedAppBundlePath` must match what `schedule.sh` actually writes
/// into ProgramArguments. That script has emitted a
/// `PCH_STORAGE_WATCH_APP_BUNDLE=` entry unconditionally since the watch
/// notification moved under the app's own identity, but this expectation
/// was never updated to match, so the exact-array comparison below could
/// never succeed: every freshly installed plist was judged `.stale`, the
/// toggle reported failure, and the UI showed the watch as off while
/// launchd had in fact loaded the job.
static func runtimeState(
protocolValues: [String: String],
expectedWatcherURL: URL,
expectedWatcherSHA256: String? = nil,
expectedHomeURL: URL = FileManager.default.homeDirectoryForCurrentUser
expectedHomeURL: URL = FileManager.default.homeDirectoryForCurrentUser,
expectedAppBundlePath: String = Bundle.main.bundleURL.path
) -> StorageWatchRuntimeState {
guard let plistPath = protocolValues["plist"], plistPath.hasPrefix("/") else {
return .stale
Expand All @@ -201,6 +210,7 @@ enum StorageWatchService {
"PATH=\(LocalProcessRunner.safeSystemPath)",
"LANG=en_US.UTF-8",
"LC_ALL=en_US.UTF-8",
"PCH_STORAGE_WATCH_APP_BUNDLE=\(expectedAppBundlePath)",
"/bin/bash",
"-p",
"-c",
Expand Down
28 changes: 26 additions & 2 deletions macos/Modore/Sources/Modore/Views/SpaceGoalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ struct SpaceGoalWorkspaceList: View {

private var achievableGB: Double { Self.achievableGB(storage) }

/// Upper bound for the goal slider. SwiftUI's Slider divides the range by
/// `step` and fatals with "max stride must be positive" on a zero-width
/// range, so `1...max(achievableGB, 1)` hard-crashed the whole page
/// whenever the cleanable total was greater than zero but at or below
/// 1GB (one small npm cache is enough). Rounding up and flooring at 2
/// keeps the range provably wider than its lower bound.
private var goalUpperBoundGB: Double { max(achievableGB.rounded(.up), 2) }

/// A whole-GB goal picker is meaningless below 1GB, and that is exactly
/// the range where a degenerate slider used to crash -- show the real
/// achievable total instead of a control the user cannot move.
private var supportsGoalSlider: Bool { achievableGB >= 1 }

private var selection: [StorageItem] {
SpaceGoalSelection.select(from: storage.cleanupCandidates, targetGB: targetGB)
}
Expand Down Expand Up @@ -92,9 +105,13 @@ struct SpaceGoalWorkspaceList: View {
@ViewBuilder
private var goalPicker: some View {
VStack(alignment: .leading, spacing: 8) {
Slider(value: $targetGB, in: 1...max(achievableGB, 1), step: 1)
if supportsGoalSlider {
Slider(value: $targetGB, in: 1...goalUpperBoundGB, step: 1)
}
HStack {
Text("목표: \(String(format: "%.0f", targetGB))GB")
Text(supportsGoalSlider
? "목표: \(String(format: "%.0f", targetGB))GB"
: "정리 가능한 용량이 1GB 미만이라 목표를 나눌 수 없습니다.")
.font(.callout.weight(.medium))
Spacer()
Text("정리 가능 총합 \(String(format: "%.1f", achievableGB))GB")
Expand All @@ -103,6 +120,13 @@ struct SpaceGoalWorkspaceList: View {
}
}
.padding(.vertical, 4)
// A rescan can shrink what is cleanable while this tab stays on
// screen; @State survives that, so an old goal could sit outside the
// new range (slider pinned at its end, header quoting a goal the
// track cannot reach).
.onChange(of: goalUpperBoundGB) { newUpperBound in
targetGB = min(max(targetGB, 1), newUpperBound)
}
}

private static func achievableGB(_ storage: StorageSnapshot) -> Double {
Expand Down
38 changes: 27 additions & 11 deletions macos/Modore/Tests/ModoreTests/CleanupSafetyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ final class CleanupSafetyTests: XCTestCase {
withIntermediateDirectories: true
)
try "#!/bin/bash\nexit 0\n".write(to: expectedWatcher, atomically: true, encoding: .utf8)
let appBundlePath = "/Applications/Modore.app"

func writePlist(watcher: URL, extraEnvironment: Bool = false) throws {
let watcherData = (try? Data(contentsOf: watcher)) ?? Data(watcher.path.utf8)
Expand All @@ -265,6 +266,9 @@ final class CleanupSafetyTests: XCTestCase {
"PATH=\(LocalProcessRunner.safeSystemPath)",
"LANG=en_US.UTF-8",
"LC_ALL=en_US.UTF-8",
// schedule.sh writes this entry unconditionally; a fixture
// without it is not a plist this product can actually produce.
"PCH_STORAGE_WATCH_APP_BUNDLE=\(appBundlePath)",
"/bin/bash",
"-p",
"-c",
Expand Down Expand Up @@ -299,7 +303,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)

// Installing replaces the stale definition with the current signed
Expand All @@ -308,7 +313,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .current)

try "#!/bin/bash\nexit 99\n".write(
Expand All @@ -319,7 +325,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)
try "#!/bin/bash\nexit 0\n".write(
to: expectedWatcher,
Expand All @@ -333,7 +340,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: mismatchedLoadedValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)

try FileManager.default.setAttributes(
Expand All @@ -343,7 +351,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)
try FileManager.default.setAttributes(
[.posixPermissions: 0o600],
Expand All @@ -354,15 +363,17 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)

let mutableWatcher = root.appendingPathComponent("Application Support/Modore/runtime/scripts/storage_watch.sh")
try writePlist(watcher: mutableWatcher)
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)

let outsidePlist = root.appendingPathComponent("outside.plist")
Expand All @@ -371,19 +382,22 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)

// Uninstall must remove the entry rather than merely unload it.
try FileManager.default.removeItem(at: plistURL)
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: protocolValues,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .absent)
}

func testStorageWatchRejectsOversizedPlistAndSymlinkedParent() throws {
let appBundlePath = "/Applications/Modore.app"
let root = FileManager.default.temporaryDirectory
.appendingPathComponent("pch-watch-bounds-\(UUID().uuidString)")
defer { try? FileManager.default.removeItem(at: root) }
Expand All @@ -403,7 +417,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: values,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)

try FileManager.default.removeItem(at: launchAgents)
Expand All @@ -418,7 +433,8 @@ final class CleanupSafetyTests: XCTestCase {
XCTAssertEqual(StorageWatchService.runtimeState(
protocolValues: values,
expectedWatcherURL: expectedWatcher,
expectedHomeURL: root
expectedHomeURL: root,
expectedAppBundlePath: appBundlePath
), .stale)
}

Expand Down
71 changes: 71 additions & 0 deletions macos/Modore/Tests/ModoreTests/SpaceGoalViewRangeTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import SwiftUI
import XCTest
@testable import Modore

/// The goal slider's range is the one place in this view that can take the
/// whole page down: SwiftUI's Slider divides the range by `step` and calls
/// `fatalError("max stride must be positive")` on a zero-width range, which
/// is not catchable. `1...max(achievableGB, 1)` collapsed to `1...1` for any
/// cleanable total in (0, 1]GB -- a single small npm cache -- so the 목표 tab
/// hard-crashed the app. These assert the bound is always strictly above the
/// lower bound, and that the sub-1GB case doesn't render a slider at all.
final class SpaceGoalViewRangeTests: XCTestCase {
private func snapshot(cleanableGB: [Double]) -> StorageSnapshot {
let candidates = cleanableGB.enumerated().map { index, size in
[
"risk": "warning",
"kind": "cache",
"label": "cache-\(index)",
"sizeGB": size,
"path": "/tmp/cache-\(index)",
"action": "정리",
"note": "",
"measureStatus": "ok",
"cleanupId": "npm_cache",
] as [String: Any]
}
return StorageSnapshot(json: [
"volume": [
"mount": "/", "freeGB": 30, "usedGB": 70,
"totalGB": 100, "usePercent": 70, "risk": "safe",
],
"cleanupCandidates": candidates,
])!
}

@MainActor
private func renderGoalTab(cleanableGB: [Double]) {
let view = SpaceGoalWorkspaceList(storage: snapshot(cleanableGB: cleanableGB))
.environmentObject(ScanModel())
let host = NSHostingView(rootView: view)
host.frame = NSRect(x: 0, y: 0, width: 640, height: 480)
host.layoutSubtreeIfNeeded()
_ = host.fittingSize
}

/// The exact crash: cleanable total greater than zero but at or below 1GB.
@MainActor
func testRendersWithASubOneGigabyteCleanableTotal() {
renderGoalTab(cleanableGB: [0.5])
}

@MainActor
func testRendersWhenEveryCandidateMeasuresZero() {
renderGoalTab(cleanableGB: [0, 0])
}

@MainActor
func testRendersAtExactlyOneGigabyte() {
renderGoalTab(cleanableGB: [1.0])
}

@MainActor
func testRendersWithAnOrdinaryMultiGigabyteTotal() {
renderGoalTab(cleanableGB: [1.5, 1.5, 1.4])
}

@MainActor
func testRendersWithNoCandidatesAtAll() {
renderGoalTab(cleanableGB: [])
}
}
44 changes: 38 additions & 6 deletions scripts/login_items.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,18 @@ current_login_item_names() {
"$OSASCRIPT_BIN" -e 'tell application "System Events" to get the name of every login item' 2>/dev/null
}

# 0 = present, 1 = confirmed absent, 2 = could not determine.
#
# "Could not determine" must never collapse into "absent". A failed System
# Events query and an item that is genuinely gone are indistinguishable from
# the exit status alone, and treating the first as the second made this
# script report a persistence item as removed while it was still installed --
# the exact class of silent false success the post-delete recheck exists to
# prevent. An empty list from a *successful* query is still a real answer
# (this Mac has zero login items), so only the query failing yields 2.
login_item_exists() {
local target="$1" names entry
names="$(current_login_item_names)" || return 1
names="$(current_login_item_names)" || return 2
IFS=',' read -ra parts <<< "$names"
# A Mac with zero login items yields an empty array, and macOS's bash 3.2
# treats "${parts[@]}" on an empty array as unbound under set -u.
Expand All @@ -94,8 +103,16 @@ login_item_exists() {
}

cmd_preview() {
local target="$1"
if ! login_item_exists "$target"; then
local target="$1" presence
login_item_exists "$target"
presence=$?
if [[ "$presence" -eq 2 ]]; then
# No token may be issued off a reading we could not actually take.
emit "status" "blocked"
emit "name" "$target"
return 1
fi
if [[ "$presence" -ne 0 ]]; then
emit "status" "not_found"
emit "name" "$target"
return 1
Expand Down Expand Up @@ -209,7 +226,17 @@ cmd_execute() {
return 1
fi

if ! login_item_exists "$target"; then
local presence
login_item_exists "$target"
presence=$?
if [[ "$presence" -eq 2 ]]; then
# Cannot read the current list, so we can neither confirm the item is
# there nor claim it is gone. Refuse rather than delete blind.
emit "status" "blocked"
emit "name" "$target"
return 1
fi
if [[ "$presence" -ne 0 ]]; then
# Removed some other way (System Settings, the app itself) between
# preview and execute. The desired end state already holds.
emit "status" "already_gone"
Expand All @@ -222,8 +249,13 @@ cmd_execute() {
"$OSASCRIPT_BIN" -e "tell application \"System Events\" to delete login item \"$escaped\"" >/dev/null 2>&1

# A clean osascript exit only means the command was accepted, not that
# the item is actually gone. Re-read the real list before reporting ok.
if login_item_exists "$target"; then
# the item is actually gone. Re-read the real list before reporting ok --
# and only a successful read proving absence counts. A failed re-read
# leaves the outcome unknown, which is a failure to report removal, not
# a removal.
login_item_exists "$target"
presence=$?
if [[ "$presence" -ne 1 ]]; then
emit "status" "failed"
emit "name" "$target"
return 1
Expand Down
18 changes: 18 additions & 0 deletions scripts/network_watch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ sample_listen() {
"$LSOF_BIN" -nP -iTCP -sTCP:LISTEN 2>/dev/null || true
}

# lsof exits non-zero with no output both when it fails and when nothing
# matches, and `|| true` cannot tell those apart -- so an empty closing
# sample is indistinguishable from "the window was quiet". Reporting 0 new
# connections off a sample we may never have taken is a false all-clear on a
# security surface, so the closing LISTEN set is used as the liveness probe:
# a real Mac always has listening sockets (launchd/rapportd/mDNSResponder),
# making an empty closing list overwhelmingly a failed read rather than a
# genuine state. The opening sample needs no such probe -- an empty opening
# baseline just makes everything look new, which errs toward over-reporting.
closing_sample_looks_unreadable() {
[[ ! -s "$WORKSPACE/second_listen" ]]
}

# 테스트는 네 표본 파일을 직접 주입한다. 실제 lsof 표는 재현할 수 없으므로,
# 델타 계산을 고정된 입력으로 검증한다.
inject_or_empty() {
Expand Down Expand Up @@ -110,6 +123,11 @@ emit "version" "$PROTOCOL_VERSION"
emit "operation" "network-watch"
emit "windowSeconds" "$WINDOW_SECONDS"

if closing_sample_looks_unreadable; then
emit "error" "관찰 종료 시점의 네트워크 목록을 읽지 못했습니다."
exit 0
fi

# lsof 열: COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME (ESTABLISHED 행은
# NAME 뒤에 "(ESTABLISHED)"가 하나 더 붙어 총 10 필드). NAME(9번째 필드)이
# established는 "LOCAL->REMOTE", listen은 "ADDR:PORT" 형태다.
Expand Down
Loading