Unescape lsof \x20 process names; fix empty-first-sample suppression - #73
Merged
Conversation
Two bugs in lsof output handling, both confirmed against this
machine's real output rather than hypothesized:
1. lsof escapes a space inside a COMMAND name as literal "\x20"
("Codex " -> "Codex\x20") -- that escaping is what keeps whitespace
field-splitting correct, but passed through verbatim it leaked into
the UI: the main scan's network/listeningPorts sections (present
since the JXA port, "Codex\x20"/"Manus\x20" in this machine's real
scan_result.json) and network_watch.sh's rows (#72). Both now
unescape it, and drop the trailing space left by lsof's 9-character
truncation -- invisible in the UI while still splitting dedup keys.
2. network_watch.sh's awk used the FNR==NR idiom to separate the two
samples. With an empty first sample -- a real shape, since lsof
exits 1 with no output both on failure and on zero matches, and
`|| true` swallows it -- FNR==NR misreads the second file as the
first, so every connection made during the window was registered as
"already seen" and reporting went silent. Found because the new
escaping test's fixture happened to use an empty first listen
sample. Replaced with POSIX inter-file variable assignment
(building=1 file1 building=0 file2), which holds even for empty
files; with no baseline, over-reporting is the safe direction.
All three new tests proven against the pre-fix code via stash/restore.
5 tasks
heznpc
added a commit
that referenced
this pull request
Aug 13, 2026
…74) Adversarial review of #60-#73 (six parallel reviewers, findings then reproduced by hand before fixing). Four defects that made the app either crash or state something untrue: login_items.sh reported removal SUCCESS when the check itself failed. login_item_exists returned the same status for "not a login item" and "the System Events query failed", so a failed read at execute time emitted already_gone/ok with exit 0. Reproduced end to end: preview issues a token normally, System Events then fails, execute reports "ok" -- while the login item is still installed. The app maps that to success and announces removal, so a security tool affirmatively told the owner a persistence mechanism was gone while it was still there. Now tri-state: only a successful read may assert absence; an unreadable list is blocked, and a failed post-delete recheck is failed, never ok. SpaceGoalView hard-crashed the 목표 tab. Slider(in: 1...max(x, 1)) collapses to 1...1 whenever the cleanable total is above zero but at or below 1GB -- one small npm cache -- and SwiftUI fatals with "max stride must be positive". Reproduced as a real crash in a hosted view, not inferred. The range is now provably wider than its lower bound, and below 1GB the slider is replaced by the achievable total. The storage watch could never be turned on, from two independent causes, both predating this range but only diagnosable together: PCH_STORAGE_WATCH_APP_BUNDLE was never in allowedEnvironmentOverrides, so sanitizedEnvironment threw and the spawn was refused; and once that was fixed, StorageWatchService's expectedArguments still omitted the entry schedule.sh actually writes, so the installed plist failed the exact-array comparison and every install was judged stale. The Swift unit test could not catch that because its fixture was built from the same wrong list the implementation used. Verified on a live machine: the toggle now turns on and the app's reported state matches launchd for the first time. A cross-language guard now compares schedule.sh's and Swift's argument lists so they cannot drift apart again. network_watch.sh reported a false all-clear when the closing lsof sample failed: `|| true` cannot distinguish failure from zero matches, so an unread sample looked like a quiet window. The closing LISTEN set is the liveness probe (every real Mac has listening sockets); an empty one now reports an error instead of "0 new connections". Every new test was proven against the pre-fix code first. Co-authored-by: Heznpc <[email protected]>
5 tasks
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
1 participant
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.
Summary
Two bugs in lsof output handling, both confirmed against this machine's real output rather than hypothesized. Found while preparing review targets after #72 merged -- re-reading the real lsof capture from that PR's verification (
Codex\x20 1142 ...) surfaced the first, and the first's new test fixture incidentally surfaced the second.1.
\x20-escaped process names leaked verbatim into the UIlsof escapes a space inside a COMMAND name as literal
\x20("Codex "→Codex\x20) -- that escaping is what keeps whitespace field-splitting correct, but passed through verbatim it leaked into user-visible output in two places:network/listeningPortssections -- present since the JXA port, i.e. long before this session. This machine's realscan_result.jsoncontainedCodex\x20andManus\x20as process names, which render as-is on the security page and feed rule matching against process names.network_watch.sh's observation rows (Add on-demand CPU/network observation window (Phase 5-1) #72, this session).Both now unescape
\x20and drop the trailing space left by lsof's 9-character COMMAND truncation (invisible in the UI while still splitting dedup keys). Confirmed against real output that lsof drops a partial escape rather than truncating mid-sequence, so the complete-\x20replacement is sufficient.2. An empty first sample silently suppressed all new-connection reports
network_watch.sh's awk used theFNR==NRidiom to separate the two samples. With an empty first sample -- a real production shape, since lsof exits 1 with no output both on failure and on zero matches, and the sampler's|| trueswallows it --FNR==NRmisreads the second file as the first: every connection made during the window got registered as "already seen" and reporting went silent. Exactly the silent-failure class this whole effort has been closing. Replaced with POSIX inter-file variable assignment (building=1 file1 building=0 file2), which holds even for empty files. With no baseline a row can't be distinguished from a genuinely new one, and over-reporting is the safe direction.Test plan
pytest tests/-- 361 passed (3 new tests: escaped-name unescaping innetwork_watch.shand in the JXA scan parsers, plus a dedicated empty-first-sample regression test)swift test-- 157 passed;release_smoke.py --check-only-- okraw_facts.jsonnetwork section now readsCodex/Manuswhere it previously readCodex\x20/Manus\x20, with no escaped or trailing-space names remaining🤖 Generated with Claude Code