fix: security & correctness hardening from deep-dive audit (#98)

* fix(security): harden notify.sh against injection and config code-exec

- Parse ~/.stack-nudge/config as allowlisted STACKNUDGE_* KEY=VALUE data
  instead of `source`-ing it, so a write to that file can no longer run
  arbitrary code on every hook event.
- Pass the cwd basename (project name) and tty path into osascript via the
  environment and read them back with `system attribute`, instead of
  interpolating them into the AppleScript text — closes AppleScript/shell
  injection via a maliciously named project directory or tty path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: serialise CodexQuotaProbe reads to avoid cacheKey/cached data race

read() mutated the cacheKey/cached stored properties from a global concurrent
queue, so overlapping quota polls could race and corrupt the cache. Route
read() through a private serial queue so those properties are only ever
touched on one queue.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: harden notify.sh phrase rendering and permission FIFO

- Render voice phrases via explicit %s substitution instead of using the
  phrase as a printf format string, so stray % tokens (especially in
  user-supplied phrases.user.json entries) can't corrupt the spoken output.
- Create the permission FIFO inside a private mktemp -d directory (0700,
  CSPRNG-named) rather than a 16-bit $RANDOM /tmp path a local attacker
  could guess and race; clean up the directory in the EXIT trap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: validate prebuilt bundle and escape plist XML in install.sh

- Verify build/StackNudge.app carries an executable before removing the
  installed copy, so a corrupt artifact can't leave the user with no app.
- XML-escape labels, paths and program args written into the launchd plist
  so an install dir containing & < > can't produce a plist launchd rejects.
- Document STACKNUDGE_DEBUG in notify.conf.example.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: correctness fixes in dedup, deny-FIFO, device-flow and antigravity

- EventStore.append: abs() the dedup timestamp delta so an out-of-order event
  isn't silently dropped as a false duplicate.
- actOnSelected: write the FIFO on Deny too (not only Allow), so a Deny no
  longer leaves the agent's hook blocked until its 550s timeout.
- pollGithubSignIn: enforce the device code's expiry (RFC 8628) instead of
  polling forever, and back off by at least 5s on slow_down.
- AntigravityLocalServer: raise the semaphore backstop above URLSession's
  resource timeout so the request completion releases the wait.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: require update checksum and surface silent API/parse failures

- Updater: refuse to install when a release has no .sha256 sidecar (every
  release publishes one) instead of silently skipping checksum verification.
- GitHubAPI.parse: detect the GraphQL {errors:[...]} envelope on HTTP 200 and
  log it, rather than treating an errored response as "no PR found".
- Phrases.defaults: log when the phrase-loading subprocess fails to launch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: surface silent listener, quota and credential failures in the UI

Addresses the three remaining high-severity deep-dive findings — each a
failure that happened with no user-visible signal:

- Socket-bind failure: startListener records nav.listenerError (cleared on a
  successful bind) and the Events tab shows an orange banner, instead of the
  panel silently receiving no events with only a stderr line in the log.
- Unofficial Anthropic quota endpoint: QuotaProbe now distinguishes "had a
  token but the request/parse failed" from "no token", so the Usage tab shows
  "quota unavailable" instead of "Loading usage…" forever when the endpoint
  shape changes.
- Plaintext OAuth token: keep preferring ~/.claude/.credentials.json (it's
  intentional — avoids Claude Code's periodic Keychain re-prompt) but warn in
  Settings -> Usage when that path is active, since any same-user process can
  read the file.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: StuBehan

install.sh

@@ -49,6 +49,13 @@ if [[ "$(uname -s)" == "Darwin" ]]; then
       exit 1
     fi
   fi
+  # Don't destroy the installed copy until we've confirmed the new bundle
+  # actually carries an executable — a corrupt or incomplete artifact would
+  # otherwise leave the user with no app installed at all.
+  if ! ls "$PREBUILT_APP"/Contents/MacOS/* >/dev/null 2>&1; then
+    echo "  ✗ $PREBUILT_APP has no executable in Contents/MacOS — refusing to replace the installed app."
+    exit 1
+  fi
   rm -rf "$HOME/Applications/StackNudge.app"
   rm -rf "$HOME/Applications/stack-nudge-panel.app"  # clean up old panel binary
   cp -r "$PREBUILT_APP" "$HOME/Applications/StackNudge.app"
@@ -116,10 +123,22 @@ register_launchd_agent() {
   shift 3
   local plist_path="$HOME/Library/LaunchAgents/${label}.plist"
 
+  # Escape XML metacharacters before interpolating any value into the plist —
+  # a program arg or path containing & < > (e.g. an install dir whose name
+  # contains them) would otherwise produce a malformed plist launchd rejects.
+  xml_escape() {
+    local s="$1"
+    s="${s//&/&amp;}"; s="${s//</&lt;}"; s="${s//>/&gt;}"
+    printf '%s' "$s"
+  }
+
   local program_xml=""
   for arg in "$@"; do
-    program_xml+="        <string>${arg}</string>"$'\n'
+    program_xml+="        <string>$(xml_escape "$arg")</string>"$'\n'
   done
+  local label_xml log_xml
+  label_xml=$(xml_escape "$label")
+  log_xml=$(xml_escape "$log_path")
 
   local keep_alive_xml
   case "$keep_alive_mode" in
@@ -134,7 +153,7 @@ register_launchd_agent() {
 <plist version="1.0">
 <dict>
     <key>Label</key>
-    <string>${label}</string>
+    <string>${label_xml}</string>
     <key>ProgramArguments</key>
     <array>
 ${program_xml}    </array>
@@ -143,9 +162,9 @@ ${program_xml}    </array>
     <key>KeepAlive</key>
     ${keep_alive_xml}
     <key>StandardOutPath</key>
-    <string>${log_path}</string>
+    <string>${log_xml}</string>
     <key>StandardErrorPath</key>
-    <string>${log_path}</string>
+    <string>${log_xml}</string>
 </dict>
 </plist>
 PLIST

notify.conf.example

@@ -44,3 +44,8 @@
 # always notify regardless of focus.
 # Default: true
 #STACKNUDGE_MUTE_WHEN_FOCUSED=false
+
+# Log a debug line (to the launchd log) explaining voice decisions — useful
+# when a notification played silently and you want to know why.
+# Default: false
+#STACKNUDGE_DEBUG=true

notify.sh

@@ -9,9 +9,27 @@ AGENT="${1:-agent}"
 EVENT="${2:-stop}"
 OS="$(uname -s 2>/dev/null || echo Windows)"
 
-# Load user config (overrides defaults below).
+# Load user config (overrides defaults below). Parse as KEY=VALUE data only —
+# never `source` it, so anything able to write to this file (a compromised
+# postinstall, a stray tool) can't get arbitrary code to run on every hook
+# event. Only STACKNUDGE_* keys are recognised; anything else is ignored.
 # Copy notify.conf.example to ~/.stack-nudge/config to customise.
-[[ -f "${HOME}/.stack-nudge/config" ]] && source "${HOME}/.stack-nudge/config"
+load_stacknudge_config() {
+  local config="${HOME}/.stack-nudge/config"
+  [[ -f "$config" ]] || return 0
+  local line key value
+  while IFS= read -r line || [[ -n "$line" ]]; do
+    [[ "$line" =~ ^[[:space:]]*(export[[:space:]]+)?(STACKNUDGE_[A-Z0-9_]+)=(.*)$ ]] || continue
+    key="${BASH_REMATCH[2]}"
+    value="${BASH_REMATCH[3]}"
+    # Strip one layer of matching surrounding quotes, if present.
+    if [[ "$value" =~ ^\"(.*)\"$ ]] || [[ "$value" =~ ^\'(.*)\'$ ]]; then
+      value="${BASH_REMATCH[1]}"
+    fi
+    printf -v "$key" '%s' "$value"
+  done < "$config"
+}
+load_stacknudge_config
 
 # Read JSON piped from Claude Code hooks (contains transcript_path for Stop events).
 # Skip if stdin is a terminal (manual invocation).
@@ -221,8 +239,11 @@ voice_phrase_for() {
   fi
 
   local template="${templates[$((RANDOM % ${#templates[@]}))]}"
-  # shellcheck disable=SC2059
-  printf "$template" "$repo"
+  # Substitute the repo name into the %s placeholder ourselves rather than
+  # letting the phrase be a printf format string — a phrase (especially a
+  # user-supplied one from phrases.user.json) with stray % tokens would
+  # otherwise corrupt or truncate the spoken output.
+  printf '%s' "${template//%s/$repo}"
 }
 
 PANEL_SOCK="${HOME}/.stack-nudge/panel.sock"
@@ -305,12 +326,17 @@ detect_iterm_tab_name() {
   tty_path=$(tty 2>/dev/null) || return
   [[ -z "$tty_path" || "$tty_path" == "not a tty" ]] && return
 
-  ITERM_TAB_NAME=$(osascript <<APPLESCRIPT 2>/dev/null || true
+  # Pass the tty path through the environment and read it back with
+  # `system attribute` inside a quoted heredoc, rather than letting bash
+  # interpolate it into the script text — keeps an unusual /dev path (or a
+  # symlinked pseudo-tty name) from breaking out of the AppleScript string.
+  ITERM_TAB_NAME=$(STACKNUDGE_TTY="$tty_path" osascript <<'APPLESCRIPT' 2>/dev/null || true
+set ttyPath to system attribute "STACKNUDGE_TTY"
 tell application "iTerm2"
   repeat with w in windows
     repeat with t in tabs of w
       repeat with s in sessions of t
-        if (tty of s) is equal to "$tty_path" then
+        if (tty of s) is equal to ttyPath then
           return name of s
         end if
       end repeat
@@ -503,11 +529,17 @@ notify_macos() {
   local project_name
   project_name=$(basename "$PWD")
   if [[ -n "$process_name" ]]; then
-    win_title=$(osascript \
+    # Pass the project name through the environment and read it back inside
+    # AppleScript via `system attribute`, rather than interpolating it into the
+    # script text — a directory named `foo" & do shell script "…` would
+    # otherwise close the string and inject AppleScript/shell. process_name is
+    # safe to interpolate (it comes from the fixed bundle-id allowlist above).
+    win_title=$(STACKNUDGE_PROJECT_NAME="$project_name" osascript \
+      -e "set projectName to system attribute \"STACKNUDGE_PROJECT_NAME\"" \
       -e "tell application \"System Events\"" \
       -e "  tell process \"${process_name}\"" \
       -e "    try" \
-      -e "      get title of first window whose title contains \"${project_name}\"" \
+      -e "      get title of first window whose title contains projectName" \
       -e "    end try" \
       -e "  end tell" \
       -e "end tell" 2>/dev/null)
@@ -554,10 +586,17 @@ notify_macos() {
 # Create a unique FIFO at /tmp for the user's response. Echoes the path.
 # Returns empty if mkfifo fails.
 create_perm_fifo() {
-  local fifo
-  fifo="/tmp/stack-nudge-perm-$$-$(date +%s)-$RANDOM.fifo"
+  # Place the FIFO inside a private mktemp dir (mode 0700, CSPRNG-named) rather
+  # than a $RANDOM-suffixed /tmp path — $RANDOM is only 16-bit, so the old name
+  # was guessable, letting a local attacker pre-create the FIFO or inject a
+  # decision. The dir is removed alongside the FIFO in wait_for_permission_response.
+  local dir
+  dir=$(mktemp -d "${TMPDIR:-/tmp}/stack-nudge-perm.XXXXXXXX" 2>/dev/null) || return
+  local fifo="$dir/fifo"
   if mkfifo -m 0600 "$fifo" 2>/dev/null; then
     echo "$fifo"
+  else
+    rmdir "$dir" 2>/dev/null
   fi
 }
 
@@ -569,7 +608,7 @@ wait_for_permission_response() {
   local fifo="$1"
   local timeout=550  # Claude Code's hook timeout defaults to 600s — leave buffer
 
-  trap 'rm -f "$fifo"' EXIT
+  trap 'rm -f "$fifo"; rmdir "$(dirname "$fifo")" 2>/dev/null' EXIT
 
   local decision
   decision=$(NUDGE_FIFO="$fifo" NUDGE_TIMEOUT="$timeout" python3 - <<'PY' 2>/dev/null

panel/AntigravityLocalServer.swift

@@ -46,7 +46,10 @@ enum AntigravityLocalServer {
             status = (response as? HTTPURLResponse)?.statusCode ?? 0
             semaphore.signal()
         }.resume()
-        _ = semaphore.wait(timeout: .now() + 5)
+        // Backstop above URLSession's own resource timeout (6s) so the request's
+        // completion handler is what releases the wait — not a shorter deadline
+        // that returns while the dataTask is still running.
+        _ = semaphore.wait(timeout: .now() + 8)
         return status == 200 ? payload : nil
     }
 

panel/CodexUsage.swift

@@ -29,10 +29,15 @@ final class CodexQuotaProbe {
     private var cacheKey: String?
     private var cached: CodexQuotaSnapshot?
 
+    // Serialises read() so overlapping polls — the 60s/5min timer firing while a
+    // prior fetch is still doing disk I/O on a large ~/.codex/sessions tree —
+    // can't race on cacheKey/cached. These are only ever touched on this queue.
+    private let probeQueue = DispatchQueue(label: "stack-nudge.codex-quota")
+
     // Calls completion on the main queue. File IO runs off-main.
     func fetch(completion: @escaping (CodexQuotaSnapshot?) -> Void) {
         let dir = sessionsDir
-        DispatchQueue.global(qos: .utility).async { [weak self] in
+        probeQueue.async { [weak self] in
             let result = self?.read(dir: dir) ?? nil
             DispatchQueue.main.async { completion(result) }
         }

panel/EventStore.swift

@@ -167,7 +167,7 @@ final class EventStore: ObservableObject {
         // identical one landed within the dedup window.
         let dedupWindow: TimeInterval = 2
         if events.contains(where: { existing in
-            event.timestamp.timeIntervalSince(existing.timestamp) < dedupWindow
+            abs(event.timestamp.timeIntervalSince(existing.timestamp)) < dedupWindow
                 && existing.agent == event.agent
                 && existing.kind == event.kind
                 && existing.message == event.message

panel/GitHubAPI.swift

@@ -71,8 +71,17 @@ enum GitHubAPI {
 
     static func parse(_ json: String) -> PullRequestInfo? {
         guard let data = json.data(using: .utf8),
-              let root = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
-              let node = firstPRNode(root),
+              let root = try? JSONSerialization.jsonObject(with: data) as? [String: Any]
+        else { return nil }
+        // GraphQL returns HTTP 200 with {"data":null,"errors":[…]} on failures
+        // (bad token, rate limit, field errors). Surface them instead of
+        // silently treating the response as "no PR found".
+        if let errors = root["errors"] as? [[String: Any]], !errors.isEmpty {
+            let messages = errors.compactMap { $0["message"] as? String }.joined(separator: "; ")
+            FileHandle.standardError.write(Data("stack-nudge: GitHub GraphQL errors: \(messages)\n".utf8))
+            return nil
+        }
+        guard let node = firstPRNode(root),
               let number = node["number"] as? Int,
               let url = node["url"] as? String,
               let stateRaw = node["state"] as? String,

panel/Panel.swift

@@ -219,6 +219,9 @@ struct PanelContentView: View {
 
     private var eventsBody: some View {
         VStack(alignment: .leading, spacing: 0) {
+            if let error = nav.listenerError {
+                listenerErrorBanner(error)
+            }
             if store.events.isEmpty {
                 emptyState
             } else {
@@ -229,6 +232,21 @@ struct PanelContentView: View {
         .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
     }
 
+    private func listenerErrorBanner(_ error: String) -> some View {
+        HStack(alignment: .top, spacing: 8) {
+            Image(systemName: "exclamationmark.triangle.fill")
+                .foregroundStyle(.orange)
+            Text(error)
+                .font(.caption)
+                .foregroundStyle(.secondary)
+                .fixedSize(horizontal: false, vertical: true)
+        }
+        .padding(.horizontal, 12)
+        .padding(.vertical, 8)
+        .frame(maxWidth: .infinity, alignment: .leading)
+        .background(Color.orange.opacity(0.12))
+    }
+
     private var emptyState: some View {
         VStack(spacing: 10) {
             Image(systemName: "bell.slash")
@@ -1198,10 +1216,15 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate,
         quotaProbe.fetch { [weak self] snapshot in
             guard let self else { return }
             self.nav.quotaSyncing = false
-            guard let snapshot else { return }
-            self.nav.quota = snapshot
-            self.nav.quotaLastUpdated = Date()
-            self.evaluateQuotaThresholds(snapshot)
+            self.nav.usingPlaintextCredentials = self.quotaProbe.usingPlaintextCredentials
+            if let snapshot {
+                self.nav.quota = snapshot
+                self.nav.quotaError = nil
+                self.nav.quotaLastUpdated = Date()
+                self.evaluateQuotaThresholds(snapshot)
+            } else if self.quotaProbe.lastProbeFailed {
+                self.nav.quotaError = "Quota data unavailable — the usage endpoint may have changed."
+            }
         }
         // Codex (ChatGPT-plan) rate limits — read locally from the newest
         // rollout, no network. Independent of the Anthropic probe above so one
@@ -1638,7 +1661,8 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate,
                     userCode: response.userCode, verificationURI: response.verificationURI)
                 self.pollGithubSignIn(clientID: clientID,
                                       deviceCode: response.deviceCode,
-                                      interval: response.interval)
+                                      interval: response.interval,
+                                      deadline: Date().addingTimeInterval(TimeInterval(response.expiresIn)))
             }
         }
     }
@@ -1647,9 +1671,15 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate,
 
     // One poll per `interval` seconds. Stops if the user cancelled (state left
     // awaitingApproval). On success, stores the token and kicks a PR refresh.
-    private func pollGithubSignIn(clientID: String, deviceCode: String, interval: Int) {
+    private func pollGithubSignIn(clientID: String, deviceCode: String, interval: Int, deadline: Date) {
         DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(max(1, interval))) { [weak self] in
             guard let self, case .awaitingApproval = self.nav.githubSignIn else { return }
+            // Enforce the device code's own expiry (RFC 8628 §3.5) rather than
+            // polling forever if the endpoint never returns expired_token.
+            if Date() >= deadline {
+                self.nav.githubSignIn = .failed("Code expired — try again")
+                return
+            }
             GitHubAuth.poll(clientID: clientID, deviceCode: deviceCode) { result in
                 DispatchQueue.main.async {
                     guard case .awaitingApproval = self.nav.githubSignIn else { return }
@@ -1660,9 +1690,10 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate,
                         self.nav.githubSignIn = .idle
                         self.refreshPullRequests()
                     case .pending:
-                        self.pollGithubSignIn(clientID: clientID, deviceCode: deviceCode, interval: interval)
+                        self.pollGithubSignIn(clientID: clientID, deviceCode: deviceCode, interval: interval, deadline: deadline)
                     case .slowDown(let slower):
-                        self.pollGithubSignIn(clientID: clientID, deviceCode: deviceCode, interval: slower)
+                        // RFC 8628: back off by at least 5s; honour a larger server interval.
+                        self.pollGithubSignIn(clientID: clientID, deviceCode: deviceCode, interval: max(slower, interval + 5), deadline: deadline)
                     case .expired:
                         self.nav.githubSignIn = .failed("Code expired — try again")
                     case .denied:
@@ -2533,10 +2564,12 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate,
         let sendApproval = approve && event.hasActionButton
 
         // FIFO path = the source hook is blocking on a permission decision.
-        // Write "allow" to it and we're done — Claude Code skips its UI prompt.
-        if sendApproval, let fifo = event.fifoPath {
+        // Write the decision (allow or deny) and we're done — the agent consumes
+        // it and skips its own UI prompt. Previously only "allow" was written, so
+        // a Deny left the hook blocked until its 550s timeout.
+        if let fifo = event.fifoPath {
             DispatchQueue.global(qos: .userInitiated).async {
-                Self.writeFIFO(fifo, "allow")
+                Self.writeFIFO(fifo, approve ? "allow" : "deny")
             }
             return
         }
@@ -2780,9 +2813,13 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate,
         listener = EventListener(store: store, socketPath: socketPath)
         do {
             try listener?.start()
+            nav.listenerError = nil
         } catch {
             FileHandle.standardError.write(Data(
                 "stack-nudge-panel: listener failed: \(error)\n".utf8))
+            nav.listenerError =
+                "Event socket failed to start — agent notifications won't arrive. "
+                + "Restart StackNudge to retry. (\(error))"
         }
     }
 }

panel/PanelNav.swift

@@ -178,6 +178,19 @@ final class PanelNav: ObservableObject {
     // True while a probe is in-flight. Set by PanelController around the
     // fetch call so the UI can swap the footer status to "Syncing…".
     @Published var quotaSyncing:     Bool = false
+    // Set when a probe had a token but the request/parse failed (vs. simply
+    // having no Claude Code session). Drives the Usage tab's "quota unavailable"
+    // state so a silently-changed endpoint isn't read as "still loading".
+    // Cleared on the next successful probe.
+    @Published var quotaError:       String?
+    // True when the Claude token was read from the plaintext
+    // ~/.claude/.credentials.json rather than the Keychain — any same-user
+    // process can read that file. Drives a warning in Settings → Usage.
+    @Published var usingPlaintextCredentials: Bool = false
+    // Set when the event socket failed to bind at startup — the panel is then
+    // deaf to every agent notification. Drives the banner at the top of the
+    // Events tab so the failure isn't silent. Cleared when the socket binds.
+    @Published var listenerError:    String?
     // Per-Claude-session context-window stats. Keyed by Claude's
     // session_id UUID (NudgeEvent.claudeSessionID), populated by
     // PanelController whenever an event arrives with a transcript_path.

panel/Phrases.swift

@@ -113,7 +113,11 @@ struct PhraseStore {
         let pipe = Pipe()
         task.standardOutput = pipe
         task.standardError  = Pipe()
-        do { try task.run() } catch { return [:] }
+        do { try task.run() } catch {
+            FileHandle.standardError.write(Data(
+                "stack-nudge: failed to load phrase defaults for \(lang): \(error)\n".utf8))
+            return [:]
+        }
         let data = pipe.fileHandleForReading.readDataToEndOfFile()
         task.waitUntilExit()