diff --git a/CHANGELOG.md b/CHANGELOG.md index 9df2309..8ff9a56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ All notable changes to `sendmsg` are documented here. +## 5.2.0 + +### Added +- **`--list-groups`:** Lists the Signal groups the account belongs to, with their names, IDs, member counts, and blocked status (via `GET /v1/groups/{number}`). This is the sanctioned way to discover the group IDs that `--recipients` and CSV rows expect. Errors out clearly when no real account is configured. +- **Test suite and CI:** A pytest suite (`tests/`) covering phone/group detection, path expansion, attachment validation, Signal payload construction, CSV dispatch (BOM, blank rows, skips, dry run, `--json`), and CLI argument validation, plus a GitHub Actions workflow running it on Python 3.10–3.13. +- **Commas in CSV messages:** Quoted message fields (`"Hi, there"`) are parsed per standard CSV rules as before, and messages with *unquoted* commas are now repaired automatically — the extra fragments are merged back into the `message` column (columns before and after it are preserved) with a warning recommending quoting. Previously an unquoted comma silently shifted every subsequent column, leaking message text into `account`, `service`, etc. Rows with fewer fields than the header are also tolerated (missing trailing columns are treated as empty). +- **Dry-run validation:** `--dry-run` now flags rows with missing recipients and verifies that `file`/`voice` paths exist (reported as `[NOT FOUND]` and listed in the summary notes), so problems surface before a real run. + +### Fixed +- **Voice messages: removed the non-existent `voice` API flag.** signal-cli-rest-api's `/v2/send` has no voice-note field; the flag sent since 5.0.0 was silently ignored and the audio was always delivered as a regular audio attachment. The payload and documentation now reflect reality. (`--voice` still works — the audio is sent as its own message — but whether it renders as a playable voice note depends on the receiving client.) +- **UTF-8 BOM no longer misroutes CSV rows.** CSVs are read with `utf-8-sig`. Previously a BOM (written by Excel and many editors) corrupted the first header to `\ufeffmethod`, making every row's method appear empty — which defaulted to `signal` and silently sent explicit `sms` rows via Signal. +- **Formatted phone numbers are no longer misdetected as group IDs.** Spaces, dashes, dots, and parentheses are stripped before phone-vs-group detection and before sending, so `+1 (800) 555-1212` and `555-867-5309` are routed as direct messages in E.164 form instead of down the group path. +- **`--delay abc` no longer crashes.** The global `--delay` is parsed by argparse as a float, producing a clean usage error instead of a `ValueError` traceback. +- **`--json` output is machine-parseable.** Per-row progress lines now go to stderr when `--json` is set, so stdout contains only the JSON summary and can be piped to `jq` etc. The summary also gains a `blank` count. +- **`--sms` sends to every `--to` recipient.** Previously only the first was sent and the rest were silently dropped; the SMS path now fans out like the Signal path and recipients are `+`-normalized. +- **Rows without a recipient are skipped and reported** instead of being POSTed with an empty recipient and failing with a confusing API error. +- **CSV summary accounting is exact.** Blank rows are counted and reported (`blank` in `--json`, a `Blank` line in the text summary), and the blank/actionable test is shared between the pre-flight count and the row loop, so a row containing only a `file` value can no longer be counted as actionable yet silently skipped. `Total = blank + success + skipped + failed` always holds. +- **SMS retries only on timeout.** Non-zero `imsg` exits (bad recipient, Messages not signed in, ...) are permanent and now fail fast instead of being retried three times with backoff. +- **`--link-signal` timeout exits non-zero**, so scripts can detect a failed link. +- **`--list-signal` and other GET calls now retry** transient failures (429/5xx, network errors) with backoff, matching the send path. + ## 5.1.1 ### Added diff --git a/README.md b/README.md index 2ee0110..3e7caf4 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ It supports individual direct messages, Signal group broadcasts, file attachment └─────────────────────────────────────────────────────────┘ ``` -**Signal path:** `sendmsg` → JSON HTTP POST → `signal-cli-rest-api` → Docker container → Signal network. Attachments are base64-encoded inline in the JSON request body (the API does not accept multipart uploads). Voice messages are sent the same way with the API's `voice` flag set so they render as playable voice notes. +**Signal path:** `sendmsg` → JSON HTTP POST → `signal-cli-rest-api` → Docker container → Signal network. Attachments are base64-encoded inline in the JSON request body (the API does not accept multipart uploads). Voice messages are sent the same way, as an audio attachment in a message of its own — the REST API has no dedicated voice-note flag, so how the audio renders (inline player vs. file) is up to the receiving client. **SMS/iMessage path:** `sendmsg` → subprocess call → `imsg` CLI → macOS Messages framework → carrier/Apple. @@ -177,6 +177,7 @@ Messaging methods (choose one): Management commands (choose one): --list-signal List linked Signal accounts + --list-groups List Signal groups and their IDs (use --account to pick the account) --link-signal Link a new device to your Signal account --show-config Print resolved configuration and where each value came from ``` @@ -210,8 +211,10 @@ sendmsg --signal --to +18885551212 --attach ~/report.pdf ### Voice Messages -Send an audio file as a Signal **voice note** (a playable voice message, -not a file attachment) with `--voice`: +Send an audio file as a Signal **voice message** with `--voice`. The audio is +delivered as an audio attachment in its own message (the Signal REST API has +no dedicated voice-note field, so whether it renders as an inline playable +voice note or as an audio file depends on the recipient's client): ``` # Send a voice message @@ -267,6 +270,10 @@ sendmsg --csv messages.csv --yes # Skip the large-batch confirmation # List linked Signal accounts sendmsg --list-signal +# List Signal groups and their IDs (for --recipients and CSV rows) +sendmsg --list-groups +sendmsg --list-groups --account +18885551212 + # Link a new device sendmsg --link-signal sendmsg --link-signal --name "my-laptop" @@ -291,11 +298,12 @@ Create a CSV file with the following columns: | `account` | No | Signal account phone number (defaults to `$SIGNAL_ACCOUNT` / config value) | | `service` | Varies | SMS service: `imessage` or `sms`. **Required** on `sms` rows; ignored for `signal`. | | `file` | No | Path to an attachment file. `~` and environment variables are expanded. | -| `voice` | No | Path to an audio file to send as a Signal **voice note**. Signal only. | +| `voice` | No | Path to an audio file to send as a Signal **voice message**. Signal only. | | `delay` | No | Seconds to wait after this row is sent (overrides the global `--delay`). | **Notes** +- **Commas in the message are fine.** The standard way is to quote the field: `signal,+1888,Alice,"Hi, how are you?",,,,,`. If a message contains *unquoted* commas, `sendmsg` detects that the row has more fields than the header, merges the extra fragments back into the `message` column (keeping the columns before and after it intact), and prints a warning suggesting quoting. Fields that themselves contain quotes follow normal CSV rules (`"She said ""hi"", then left"`). - `signal` rows may be **attachment-only** or **voice-only**: if a `file` or `voice` is present, the `message` may be left blank. `sms` rows always require a `message`. - The `voice` column is **Signal only**. An `sms` row with a `voice` value is skipped and reported, since SMS/iMessage has no voice-note concept. - A voice note is sent as its own message; if a row has both `voice` and `file`, they are delivered as separate messages. @@ -487,10 +495,21 @@ After processing all rows, a summary is printed: --- +## Testing + +The repository ships with a pytest suite (`tests/test_sendmsg.py`) that runs entirely offline — no Signal container, no macOS, and no real config needed: + +```bash +pip install pytest +python3 -m pytest tests/ +``` + +See [TESTING.md](TESTING.md) for details on selecting subsets of tests, how the suite stubs out sends, and how CI runs it. + ## Changelog See [CHANGELOG.md](CHANGELOG.md) for the full version history. --- -*For support or issues, see * +*For support or issues, see * \ No newline at end of file diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 0000000..0f4959b --- /dev/null +++ b/TESTING.md @@ -0,0 +1,98 @@ +# Testing sendmsg + +The test suite lives in `tests/test_sendmsg.py` and covers phone/group +detection, path expansion, attachment validation, Signal payload +construction, CSV parsing and dispatch (BOM handling, comma repair, blank +rows, skips, dry run, `--json` output), and CLI argument validation. + +## Requirements + +- Python 3.10 or newer +- `pytest` (the only test dependency — sendmsg itself is stdlib-only) + +```bash +pip install pytest +# or on a system where pip refuses to touch system packages (Homebrew, Debian): +pip install pytest --break-system-packages +# or keep it isolated: +python3 -m venv .venv && source .venv/bin/activate && pip install pytest +``` + +## Running the tests + +Run from the repository root (the tests locate the `sendmsg` script +relative to their own path, so the working directory doesn't actually +matter — but the root is the natural place): + +```bash +python3 -m pytest tests/ +``` + +Expected output looks like: + +``` +..................................................................... [100%] +69 passed in 0.14s +``` + +### Useful variations + +```bash +python3 -m pytest tests/ -v # one line per test, with names +python3 -m pytest tests/ -q # terse summary only +python3 -m pytest tests/ -k csv # only tests matching "csv" +python3 -m pytest tests/ -k "bom or comma" # match multiple keywords +python3 -m pytest tests/test_sendmsg.py::TestPhoneNumbers # one test class +python3 -m pytest tests/test_sendmsg.py::TestCsv::test_quoted_commas_in_message # one test +python3 -m pytest tests/ -x # stop at the first failure +python3 -m pytest tests/ --lf # re-run only what failed last time +``` + +## What the tests do NOT need + +- **No running signal-cli-rest-api container.** All network calls + (`signal_rest_post`, `signal_rest_get`) are replaced with fakes; the + suite asserts on the payloads sendmsg *would* send. +- **No macOS / no `imsg`.** SMS sends are stubbed the same way, so the + suite runs identically on Linux and macOS. +- **No real config.** The suite pins `SIGNAL_REST_URL` and + `SIGNAL_ACCOUNT` environment variables *before* importing the script, + so your real `~/.sendmsg.conf` and shell environment can't leak into + (or be touched by) test runs. Nothing is ever actually sent. + +## How the script gets imported + +`sendmsg` has no `.py` extension, so the suite loads it with +`importlib`'s `SourceFileLoader` (see the top of `tests/test_sendmsg.py`). +The loaded module is a normal Python module — tests call its functions +(`normalize_account`, `send_one_signal`, `cmd_csv` via `main()`, ...) +directly and monkeypatch its globals. + +Two consequences worth knowing: + +1. Module-level code (config loading) runs at import time — that's why + the env vars are pinned first. +2. If you rename or move the `sendmsg` script, update `SCRIPT_PATH` at + the top of the test file. + +## Continuous integration + +`.github/workflows/ci.yml` runs on every push and pull request: +a compile check (`python -m py_compile sendmsg`) followed by the full +suite on Python 3.10, 3.11, 3.12, and 3.13. A green run on your branch +means the same command that CI uses passed: + +```bash +python -m py_compile sendmsg && python3 -m pytest tests/ -v +``` + +## Adding tests + +- Pure helpers (parsing, normalization, validation) get direct + parametrized tests — see `TestPhoneNumbers` for the pattern. +- Anything that would send goes through the stubbing helpers: + `run_csv()` for CSV behavior (returns exit code, stdout, stderr, and + the captured Signal/SMS calls) and the `captured_posts` fixture for + payload-level assertions. +- When fixing a bug, add a test that fails on the old behavior first — + most tests in the suite carry a comment naming the bug they pin down. \ No newline at end of file diff --git a/sendmsg b/sendmsg index 88b678d..8d0c8dc 100755 --- a/sendmsg +++ b/sendmsg @@ -9,11 +9,12 @@ Usage: sendmsg --sms --to +18005551212 --text "Hey" --service imessage --file ~/photo.png sendmsg --signal --to +18005551212 --text "Multi" --text "more text" sendmsg --signal --to +18005551212 --attach ~/doc.pdf ~/pic.jpg - sendmsg --signal --to +18005551212 --voice ~/note.m4a # Send a voice message + sendmsg --signal --to +18005551212 --voice ~/note.m4a # Send an audio/voice message sendmsg --csv messages.csv # Bulk send from CSV file sendmsg --csv messages.csv --delay 2 # Wait N seconds between sends sendmsg --csv messages.csv --dry-run # Preview without sending sendmsg --list-signal # Show linked Signal accounts + sendmsg --list-groups # Show Signal groups and their IDs sendmsg --link-signal # Link a new device to your existing Signal account CSV format (messages.csv): @@ -39,7 +40,7 @@ import urllib.error import urllib.parse import urllib.request -VERSION = "5.1.1" +VERSION = "5.2.0" CONFIG_PATHS = [ os.path.expanduser("~/.sendmsg.conf"), @@ -137,9 +138,23 @@ def expand_path(path): return os.path.expanduser(os.path.expandvars(path.strip())) +# Characters commonly used to format phone numbers. These must be stripped +# before phone-vs-group detection: '+1 (800) 555-1212' is a phone number, but +# without stripping it fails the all-digits test and is mis-routed as a +# Signal group ID (the same class of bug as issue #3). +_PHONE_FORMAT_CHARS = str.maketrans("", "", " \t-.()") + + +def strip_phone_separators(value): + """Remove spaces, dashes, dots, and parentheses from a phone number.""" + return value.translate(_PHONE_FORMAT_CHARS) + + def normalize_account(account): """Ensure an account/phone number carries a leading '+'. + Formatting characters (spaces, dashes, dots, parentheses) are stripped + so the number is in the E.164-ish form the Signal REST API expects. Group IDs (starting with 'group.') are returned unchanged. """ if not account: @@ -147,6 +162,7 @@ def normalize_account(account): account = account.strip() if account.startswith("group."): return account + account = strip_phone_separators(account) if not account.startswith("+"): account = "+" + account return account @@ -155,17 +171,20 @@ def normalize_account(account): def looks_like_phone_number(value): """Return True if value looks like a phone number. - Accepts numbers with an explicit '+', and also bare all-digit strings - (which a user may type without the leading '+'). This is the inverse - test used by is_group_id, and the two must agree on what a bare number - is so a phone number is never mis-routed as a group. + Accepts numbers with an explicit '+', bare all-digit strings, and + numbers containing common formatting characters ('+1 (800) 555-1212', + '555-867-5309'). This is the inverse test used by is_group_id, and the + two must agree on what a phone number is so a number is never + mis-routed as a group. """ if not value: return False - value = value.strip() + value = strip_phone_separators(value.strip()) + if not value: + return False if value.startswith("+"): return value[1:].isdigit() and len(value) > 1 - # A bare run of digits (optionally long) is a phone number, not a group. + # A bare run of digits is a phone number, not a group. return value.isdigit() @@ -285,12 +304,16 @@ def send_one_signal(account, recipient, message, group_id=None, ) return False - # A voice note is sent as its own message (a voice note cannot be mixed - # with arbitrary file attachments in a single send). + # A voice message is sent as its own message, separate from any other + # file attachments. NOTE: signal-cli-rest-api's /v2/send has no + # voice-note field (a previous release sent "voice": true, which the + # API silently ignored), so the audio is delivered as a regular audio + # attachment. How it renders (inline player vs. file) is up to the + # receiving client. overall_ok = True if has_voice: - voice_payload = {"number": account, "recipients": [target], "voice": True} + voice_payload = {"number": account, "recipients": [target]} if message and not has_attachment: # Attach the text to the voice message if there are no other files. voice_payload["message"] = message @@ -361,15 +384,9 @@ def send_one_sms(to, message, service, file=None): print(f"✅ SMS/iMessage sent to {to} (service: {service.upper()})") return True - if attempt < max_attempts: - wait = backoff * attempt - print( - f"⚠️ SMS send to {to} failed (exit {result.returncode}); retrying in " - f"{wait:.0f}s (attempt {attempt}/{max_attempts})...", - file=sys.stderr, - ) - time.sleep(wait) - continue + # A non-zero exit is almost always permanent (bad recipient, Messages + # not signed in, etc.); retrying just burns time. Only timeouts — + # a hung Messages app — are treated as transient and retried above. print(f"❌ SMS send failed (exit {result.returncode}): {result.stderr.strip()}", file=sys.stderr) return False @@ -466,22 +483,48 @@ def signal_rest_post(endpoint, payload=None, files=None): def signal_rest_get(endpoint): - """GET a JSON response from the signal-cli-rest-api.""" + """GET a JSON response from the signal-cli-rest-api. + + Transient failures (429/5xx, network errors) are retried with backoff, + matching the resilience of the POST helper. + """ url = f"{SIGNAL_REST_URL.rstrip('/')}{endpoint}" - req = urllib.request.Request(url, method="GET") - try: - with urllib.request.urlopen(req, timeout=15) as resp: - data = resp.read().decode("utf-8") - if data.strip(): - return json.loads(data) + max_attempts = 3 + backoff = 2.0 + for attempt in range(1, max_attempts + 1): + req = urllib.request.Request(url, method="GET") + try: + with urllib.request.urlopen(req, timeout=15) as resp: + data = resp.read().decode("utf-8") + if data.strip(): + return json.loads(data) + return None + except urllib.error.HTTPError as e: + body = e.read().decode("utf-8", errors="replace") + if e.code in (429, 500, 502, 503, 504) and attempt < max_attempts: + wait = backoff * attempt + print( + f"⚠️ Signal REST HTTP {e.code}; retrying in {wait:.0f}s " + f"(attempt {attempt}/{max_attempts})...", + file=sys.stderr, + ) + time.sleep(wait) + continue + print(f"❌ Signal REST error: HTTP {e.code} — {body[:500]}", file=sys.stderr) return None - except urllib.error.HTTPError as e: - body = e.read().decode("utf-8", errors="replace") - print(f"❌ Signal REST error: HTTP {e.code} — {body[:500]}", file=sys.stderr) - return None - except urllib.error.URLError as e: - print(f"❌ Cannot reach Signal REST API at {SIGNAL_REST_URL}: {e.reason}", file=sys.stderr) - return None + except urllib.error.URLError as e: + if attempt < max_attempts: + wait = backoff * attempt + print( + f"⚠️ Cannot reach Signal REST API ({e.reason}); retrying in " + f"{wait:.0f}s (attempt {attempt}/{max_attempts})...", + file=sys.stderr, + ) + time.sleep(wait) + continue + print(f"❌ Cannot reach Signal REST API at {SIGNAL_REST_URL}: {e.reason}", file=sys.stderr) + return None + return None # --------------------------------------------------------------------------- @@ -568,13 +611,18 @@ def cmd_sms(args): else: print(f"⚠️ Warning: SMS supports one attachment; ignoring extra file: {ef}", file=sys.stderr) - ok = send_one_sms( - to=args.to[0], - message=msg_text, - service=args.service, - file=chosen_file, - ) - if not ok: + # Fan out to every --to recipient, matching the Signal path. Previously + # only args.to[0] was sent and extra recipients were silently dropped. + all_ok = True + for to in args.to: + ok = send_one_sms( + to=normalize_account(to), + message=msg_text, + service=args.service, + file=chosen_file, + ) + all_ok = all_ok and ok + if not all_ok: sys.exit(1) @@ -591,28 +639,83 @@ def cmd_csv(args): # Global delay from --delay applies between every send unless a row # specifies its own delay (which takes precedence). - global_delay = float(args.delay) if args.delay else 0.0 + global_delay = args.delay if args.delay else 0.0 - with open(filepath, newline="", encoding="utf-8") as csvfile: - reader = csv.DictReader(csvfile) + # With --json the summary on stdout must be the ONLY stdout output so it + # can be piped to a parser; per-row progress then goes to stderr. + progress = sys.stderr if getattr(args, "json_output", False) else sys.stdout + + # 'utf-8-sig' transparently strips a UTF-8 BOM. Excel and many editors + # write one; under plain 'utf-8' it corrupts the first header to + # '\ufeffmethod', making every row's method look empty — which then + # defaulted to 'signal' and silently misrouted explicit 'sms' rows. + # restkey collects overflow fields (see repair below); restval="" fills + # missing trailing columns so short rows never produce None values. + with open(filepath, newline="", encoding="utf-8-sig") as csvfile: + reader = csv.DictReader(csvfile, restkey="_overflow", restval="") + fieldnames = reader.fieldnames or [] rows = list(reader) if not rows: print("❌ Error: CSV file is empty (no data rows).", file=sys.stderr) sys.exit(1) - # Count the rows that will actually be attempted (non-blank). - actionable = [ - r for r in rows - if (r.get("method", "").strip() - or r.get("message", "").strip() - or r.get("recipient", "").strip() - or r.get("file", "").strip() - or r.get("voice", "").strip()) - ] + def repair_overflow(row): + """Merge unquoted-comma overflow back into the message field. + + Standard CSV quoting ('"Hi, there"') is fully supported and needs no + repair. But a message containing UNQUOTED commas splits into extra + fields, shifting every column after 'message' (so part of the text + lands in 'account', the delay in 'voice', etc.) and corrupting the + row silently. When a row has more fields than the header, the only + sane interpretation is that the extras belong to the free-text + message column: keep the columns before 'message' from the front, + the columns after it from the back, and rejoin everything in the + middle with commas. + + Returns (row, n_extra_fields_merged). + """ + overflow = row.pop("_overflow", None) + if not overflow or "message" not in fieldnames: + return row, 0 + idx = fieldnames.index("message") + n_after = len(fieldnames) - idx - 1 + raw = [str(row.get(h) or "") for h in fieldnames] + [str(x) for x in overflow] + merged = ",".join(raw[idx:len(raw) - n_after]) + repaired = dict(zip(fieldnames, raw[:idx] + [merged] + raw[len(raw) - n_after:])) + return repaired, len(overflow) + + repair_notes = [] + for idx0, row in enumerate(rows): + repaired, n_extra = repair_overflow(row) + if n_extra: + rows[idx0] = repaired + note = ( + f"Row {idx0 + 1}: message contains unquoted commas; merged " + f"{n_extra} extra field(s) back into the message. Quote the " + f'field ("...") to silence this.' + ) + repair_notes.append(note) + print(f"⚠️ {note}", file=sys.stderr) + + def row_is_blank(r): + """True if a row has no content in any actionable column. + + Used both for the pre-flight count and the per-row skip so the two + can never disagree (previously a row with only a 'file' value was + counted as actionable but then silently skipped as blank). + """ + return not any( + (r.get(col) or "").strip() + for col in ("method", "recipient", "message", "file", "voice") + ) + + actionable = [r for r in rows if not row_is_blank(r)] + blank = len(rows) - len(actionable) if args.dry_run: - print(f"🔎 Dry run: {len(actionable)} row(s) would be processed from {filepath}\n") + print(f"🔎 Dry run: {len(actionable)} row(s) would be processed from {filepath}\n", + file=progress) elif len(actionable) > CSV_CONFIRM_THRESHOLD and not args.yes: try: resp = input( @@ -629,17 +732,17 @@ def cmd_csv(args): failed = 0 skipped = 0 previewed = 0 - errors = [] + errors = list(repair_notes) for i, row in enumerate(rows, 1): - method = row.get("method", "").strip().lower() - message = row.get("message", "").strip() - to = row.get("recipient", "").strip() - name = row.get("name", "").strip() - voice_path = row.get("voice", "").strip() or None + method = (row.get("method") or "").strip().lower() + message = (row.get("message") or "").strip() + to = (row.get("recipient") or "").strip() + name = (row.get("name") or "").strip() + voice_path = (row.get("voice") or "").strip() or None - # Skip blank rows - if not method and not message and not to and not voice_path: + # Skip blank rows (same test as the pre-flight count above). + if row_is_blank(row): continue if not method: @@ -662,6 +765,16 @@ def cmd_csv(args): print(f"⚠️ Row {i}: invalid delay '{delay_str}' — ignoring", file=sys.stderr) row_delay = None + # A recipient is always required. Previously a recipient-less signal + # row was passed through and POSTed with an empty recipients entry, + # producing a confusing API error (and a dry run reported it as + # sendable). + if not to: + print(f"⚠️ Row {i}: missing recipient — skipping", file=sys.stderr) + skipped += 1 + errors.append(f"Row {i}: missing recipient") + continue + # A message is required UNLESS there is an attachment or a voice note # (Signal can send attachment-only / voice-only messages). SMS still # requires text. @@ -698,21 +811,32 @@ def cmd_csv(args): label = f"{name} ({to})" if name else to or "(group)" prefix = "[DRY] " if args.dry_run else "" - print(f"{prefix}[{i}/{total}] {method.upper()} → {label}") + print(f"{prefix}[{i}/{total}] {method.upper()} → {label}", file=progress) if args.dry_run: - # Report what would happen without sending. + # Report what would happen without sending, and validate the + # attachment paths so problems surface BEFORE a real run. extras = [] if file_path: - extras.append(f"file={file_path}") + expanded = expand_path(file_path) + if os.path.isfile(expanded): + extras.append(f"file={file_path}") + else: + extras.append(f"file={file_path} [NOT FOUND]") + errors.append(f"Row {i}: attachment not found: {expanded}") if voice_path: - extras.append(f"voice={voice_path}") + expanded = expand_path(voice_path) + if os.path.isfile(expanded): + extras.append(f"voice={voice_path}") + else: + extras.append(f"voice={voice_path} [NOT FOUND]") + errors.append(f"Row {i}: voice file not found: {expanded}") if method == "signal" and is_group_id(to): extras.append("group") if method == "sms": extras.append(f"service={service}") if extras: - print(f" ({', '.join(extras)})") + print(f" ({', '.join(extras)})", file=progress) previewed += 1 continue @@ -752,6 +876,7 @@ def cmd_csv(args): if getattr(args, "json_output", False): summary = { "total": total, + "blank": blank, "success": success, "skipped": skipped, "failed": failed, @@ -767,6 +892,8 @@ def cmd_csv(args): print("📊 CSV Send Summary" + (" (DRY RUN)" if args.dry_run else "")) print("=" * 50) print(f" Total rows: {total}") + if blank: + print(f" ⬜ Blank: {blank}") if args.dry_run: print(f" 🔎 Would send: {previewed}") else: @@ -901,6 +1028,51 @@ def cmd_link_signal(args): return print("⚠️ Linking timed out (no new account detected). Please try again.", file=sys.stderr) + sys.exit(1) + + +def cmd_list_groups(args): + """List the Signal groups the account belongs to, with their IDs. + + Uses GET /v1/groups/{number}. This is the sanctioned way to discover the + group IDs that CSV rows and --recipients expect. + """ + account = normalize_account(args.account or SIGNAL_DEFAULT_ACCOUNT) + if account == PLACEHOLDER_ACCOUNT: + print( + f"❌ Error: no Signal account configured (still the placeholder " + f"{PLACEHOLDER_ACCOUNT}). Set SIGNAL_ACCOUNT, set " + f"signal_default_account in ~/.sendmsg.conf, or pass --account.", + file=sys.stderr, + ) + sys.exit(1) + + endpoint = f"/v1/groups/{urllib.parse.quote(account, safe='')}" + result = signal_rest_get(endpoint) + if result is None: + print(f"❌ Failed to list groups for {account}.", file=sys.stderr) + sys.exit(1) + + # Shape-tolerant: expect a list of group dicts, but don't crash on + # variations. Known keys across API versions: name, id ('group.'), + # internal_id, members, blocked. + groups = result if isinstance(result, list) else result.get("groups", []) if isinstance(result, dict) else [] + if not groups: + print(f"No Signal groups found for {account}.") + return + + print(f"Signal groups for {account}:") + for g in groups: + if not isinstance(g, dict): + print(f" • {g}") + continue + gname = g.get("name") or "(unnamed)" + gid = g.get("id") or g.get("internal_id") or "(no id)" + members = g.get("members") + member_note = f", {len(members)} member(s)" if isinstance(members, list) else "" + blocked_note = " [blocked]" if g.get("blocked") else "" + print(f" • {gname}{blocked_note}{member_note}") + print(f" id: {gid}") # --------------------------------------------------------------------------- @@ -922,6 +1094,7 @@ def main(): " %(prog)s --csv messages.csv --delay 2\n" " %(prog)s --csv messages.csv --dry-run\n" " %(prog)s --list-signal\n" + " %(prog)s --list-groups\n" " %(prog)s --link-signal --name 'my-automation'\n" "\n" "CSV columns: method, recipient, name, message, account, service, file, voice, delay\n" @@ -962,6 +1135,8 @@ def main(): # Management commands mgmt = parser.add_argument_group("Management commands") mgmt.add_argument("--list-signal", action="store_true", help="List linked Signal accounts") + mgmt.add_argument("--list-groups", action="store_true", + help="List Signal groups (and their IDs) for the account") mgmt.add_argument("--link-signal", action="store_true", help="Link a new device to your Signal account") mgmt.add_argument("--show-config", action="store_true", help="Print resolved configuration and where each value came from") @@ -973,7 +1148,8 @@ def main(): # CSV-specific csv_grp = parser.add_argument_group("CSV options") - csv_grp.add_argument("--delay", metavar="SECONDS", help="Wait N seconds between CSV row sends (default: 0)") + csv_grp.add_argument("--delay", metavar="SECONDS", type=float, + help="Wait N seconds between CSV row sends (default: 0)") csv_grp.add_argument("--dry-run", action="store_true", help="Preview a CSV batch without sending") csv_grp.add_argument("--yes", "-y", action="store_true", help="Skip the large-batch confirmation prompt") @@ -991,6 +1167,7 @@ def main(): "--sms": args.sms, "--csv": bool(args.csv), "--list-signal": args.list_signal, + "--list-groups": args.list_groups, "--link-signal": args.link_signal, "--show-config": args.show_config, } @@ -998,7 +1175,7 @@ def main(): if not chosen: parser.print_help() - print("\n❌ Error: Specify --signal, --sms, --csv, --list-signal, --link-signal, or --show-config", file=sys.stderr) + print("\n❌ Error: Specify --signal, --sms, --csv, --list-signal, --list-groups, --link-signal, or --show-config", file=sys.stderr) sys.exit(1) if len(chosen) > 1: @@ -1023,6 +1200,8 @@ def main(): cmd_csv(args) elif args.list_signal: cmd_list_signal(args) + elif args.list_groups: + cmd_list_groups(args) elif args.link_signal: cmd_link_signal(args) elif args.show_config: diff --git a/tests/test_sendmsg.py b/tests/test_sendmsg.py new file mode 100755 index 0000000..01e9269 --- /dev/null +++ b/tests/test_sendmsg.py @@ -0,0 +1,444 @@ +"""Unit tests for the sendmsg CLI. + +The script has no .py extension, so it's loaded via SourceFileLoader. +Environment variables are pinned before import so a developer's real +~/.sendmsg.conf or env can't leak into test results. +""" + +import importlib.util +import io +import json +import os +import sys +from importlib.machinery import SourceFileLoader +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT_PATH = REPO_ROOT / "sendmsg" + +# Pin config to known values BEFORE the module-level load_config() runs. +os.environ["SIGNAL_REST_URL"] = "http://localhost:8080" +os.environ["SIGNAL_ACCOUNT"] = "+15550001111" + +loader = SourceFileLoader("sendmsg_module", str(SCRIPT_PATH)) +spec = importlib.util.spec_from_loader("sendmsg_module", loader) +sendmsg = importlib.util.module_from_spec(spec) +loader.exec_module(sendmsg) + + +# --------------------------------------------------------------------------- +# Phone-number handling +# --------------------------------------------------------------------------- + +class TestPhoneNumbers: + @pytest.mark.parametrize("raw,expected", [ + ("+18005551212", "+18005551212"), + ("18005551212", "+18005551212"), + ("+1 (800) 555-1212", "+18005551212"), # formatted (bug fix) + ("555-867-5309", "+5558675309"), + ("+1.800.555.1212", "+18005551212"), + (" +18005551212 ", "+18005551212"), + ("group.ZzBHd3NZ", "group.ZzBHd3NZ"), # groups pass through + ("", ""), + (None, None), + ]) + def test_normalize_account(self, raw, expected): + assert sendmsg.normalize_account(raw) == expected + + @pytest.mark.parametrize("value,expected", [ + ("+18005551212", True), + ("18005551212", True), + ("+1 (800) 555-1212", True), # bug fix: formatted numbers + ("555-867-5309", True), + ("+1.800.555.1212", True), + ("group.ZzBHd3NZ", False), + ("ZzBHd3NZOWlrY2xrpB==", False), # raw base64 group key + ("+", False), + ("", False), + (None, False), + ("()- .", False), # separators only is not a number + ]) + def test_looks_like_phone_number(self, value, expected): + assert sendmsg.looks_like_phone_number(value) is expected + + @pytest.mark.parametrize("value,expected", [ + ("group.ZzBHd3NZ", True), + ("ZzBHd3NZOWlrY2xrpB==", True), # raw base64 key is a group + ("+18005551212", False), + ("18005551212", False), + ("+1 (800) 555-1212", False), # bug fix: NOT a group + ("555-867-5309", False), # bug fix: NOT a group + ("", False), + (None, False), + ]) + def test_is_group_id(self, value, expected): + assert sendmsg.is_group_id(value) is expected + + def test_detectors_agree(self): + """A value must never be both a phone number and a group ID.""" + for v in ["+18005551212", "555-867-5309", "group.abc", "Zz09==", "+1 (800) 555-1212"]: + assert not (sendmsg.looks_like_phone_number(v) and sendmsg.is_group_id(v)) + + def test_normalize_recipient(self): + assert sendmsg.normalize_recipient("800-555-1212") == "+8005551212" + assert sendmsg.normalize_recipient("group.abc") == "group.abc" + + +# --------------------------------------------------------------------------- +# Path expansion / attachment validation +# --------------------------------------------------------------------------- + +class TestPaths: + def test_expand_path_tilde(self, monkeypatch): + monkeypatch.setenv("HOME", "/home/tester") + assert sendmsg.expand_path("~/pic.jpg") == "/home/tester/pic.jpg" + + def test_expand_path_env_var(self, monkeypatch): + monkeypatch.setenv("PICDIR", "/data/pics") + assert sendmsg.expand_path("$PICDIR/a.png") == "/data/pics/a.png" + + def test_expand_path_empty(self): + assert sendmsg.expand_path("") == "" + assert sendmsg.expand_path(None) is None + + def test_validate_attachment_missing(self, capsys): + assert sendmsg.validate_attachment("/nope/missing.bin") is None + assert "not found" in capsys.readouterr().err + + def test_validate_attachment_ok(self, tmp_path): + f = tmp_path / "a.txt" + f.write_text("hi") + assert sendmsg.validate_attachment(str(f)) == str(f) + + def test_validate_attachment_too_large(self, tmp_path, monkeypatch, capsys): + f = tmp_path / "big.bin" + f.write_bytes(b"x" * 10) + monkeypatch.setattr(sendmsg, "MAX_ATTACHMENT_BYTES", 5) + assert sendmsg.validate_attachment(str(f)) is None + assert "too large" in capsys.readouterr().err + + def test_validate_voice_format_warning(self, tmp_path, capsys): + f = tmp_path / "clip.txt" + f.write_text("not audio") + # Unrecognized format warns but still returns the path. + assert sendmsg.validate_attachment(str(f), as_voice=True) == str(f) + assert "not a recognized voice/audio format" in capsys.readouterr().err + + +# --------------------------------------------------------------------------- +# Accounts response shape tolerance +# --------------------------------------------------------------------------- + +class TestExtractAccounts: + def test_none(self): + assert sendmsg._extract_accounts(None) == [] + + def test_bare_list(self): + assert sendmsg._extract_accounts(["+1", "+2"]) == ["+1", "+2"] + + def test_accounts_dict(self): + assert sendmsg._extract_accounts({"accounts": ["+1"]}) == ["+1"] + + def test_number_dict(self): + assert sendmsg._extract_accounts({"number": "+1"}) == ["+1"] + + def test_unknown_shape(self): + assert sendmsg._extract_accounts({"weird": 1}) == [] + assert sendmsg._extract_accounts("string") == [] + + +# --------------------------------------------------------------------------- +# Signal payload construction (voice fix) +# --------------------------------------------------------------------------- + +class TestSignalPayloads: + @pytest.fixture + def captured_posts(self, monkeypatch): + calls = [] + + def fake_post(endpoint, payload=None, files=None): + calls.append({"endpoint": endpoint, "payload": dict(payload or {}), "files": files}) + return {} + + monkeypatch.setattr(sendmsg, "signal_rest_post", fake_post) + return calls + + def test_voice_flag_not_sent(self, captured_posts, tmp_path): + """/v2/send has no 'voice' field upstream; we must not send one.""" + clip = tmp_path / "note.m4a" + clip.write_bytes(b"\x00\x01") + ok = sendmsg.send_one_signal("+15550001111", "+15550002222", + message=None, voice=str(clip)) + assert ok is True + assert len(captured_posts) == 1 + call = captured_posts[0] + assert call["endpoint"] == "/v2/send" + assert "voice" not in call["payload"] + assert call["files"] == str(clip) or call["files"] == [str(clip)] + + def test_voice_carries_text_when_no_attachments(self, captured_posts, tmp_path): + clip = tmp_path / "note.m4a" + clip.write_bytes(b"\x00") + sendmsg.send_one_signal("+15550001111", "+15550002222", + message="hi", voice=str(clip)) + assert len(captured_posts) == 1 + assert captured_posts[0]["payload"].get("message") == "hi" + + def test_voice_plus_attachment_two_sends_text_once(self, captured_posts, tmp_path): + clip = tmp_path / "note.m4a" + clip.write_bytes(b"\x00") + doc = tmp_path / "doc.pdf" + doc.write_bytes(b"%PDF") + sendmsg.send_one_signal("+15550001111", "+15550002222", + message="hi", voice=str(clip), attach=[str(doc)]) + assert len(captured_posts) == 2 + voice_call, attach_call = captured_posts + assert "message" not in voice_call["payload"] + assert attach_call["payload"].get("message") == "hi" + + def test_formatted_number_routed_direct_not_group(self, captured_posts): + sendmsg.send_one_signal("+15550001111", "+1 (555) 000-2222", message="hi") + assert captured_posts[0]["payload"]["recipients"] == ["+15550002222"] + + def test_empty_send_skipped(self, captured_posts, capsys): + ok = sendmsg.send_one_signal("+15550001111", "+15550002222", message=None) + assert ok is False + assert captured_posts == [] + assert "skipped" in capsys.readouterr().err + + +# --------------------------------------------------------------------------- +# CSV behavior (run through cmd_csv with sends stubbed out) +# --------------------------------------------------------------------------- + +def run_csv(monkeypatch, tmp_path, csv_bytes, argv_extra=(), signal_result=True, sms_result=True): + """Write csv_bytes to a file, run cmd_csv with stubbed senders. + + Returns (exit_code, stdout, stderr, signal_calls, sms_calls). + """ + csv_file = tmp_path / "batch.csv" + csv_file.write_bytes(csv_bytes) + + signal_calls, sms_calls = [], [] + monkeypatch.setattr(sendmsg, "send_one_signal", + lambda **kw: signal_calls.append(kw) or signal_result) + monkeypatch.setattr(sendmsg, "send_one_sms", + lambda **kw: sms_calls.append(kw) or sms_result) + monkeypatch.setattr(sendmsg.time, "sleep", lambda s: None) + + argv = ["sendmsg", "--csv", str(csv_file), *argv_extra] + monkeypatch.setattr(sys, "argv", argv) + + out, err = io.StringIO(), io.StringIO() + monkeypatch.setattr(sys, "stdout", out) + monkeypatch.setattr(sys, "stderr", err) + + code = 0 + try: + sendmsg.main() + except SystemExit as e: + code = e.code or 0 + return code, out.getvalue(), err.getvalue(), signal_calls, sms_calls + + +HEADER = b"method,recipient,name,message,account,service,file,voice,delay\n" + + +class TestCsv: + def test_bom_does_not_misroute_sms_rows(self, monkeypatch, tmp_path): + """A UTF-8 BOM previously blanked the method column, silently + sending explicit 'sms' rows via Signal.""" + data = b"\xef\xbb\xbf" + HEADER + b"sms,+15550003333,Bob,Hi,,imessage,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert code == 0 + assert sig == [] + assert len(sms) == 1 + assert sms[0]["service"] == "imessage" + + def test_missing_recipient_skipped(self, monkeypatch, tmp_path): + data = HEADER + b"signal,,NoOne,Hello,,,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert sig == [] and sms == [] + assert "missing recipient" in err + assert code == 0 # a skip is not a failure + + def test_json_stdout_is_pure_json(self, monkeypatch, tmp_path): + data = HEADER + (b"signal,+15550003333,A,Hello,,,,,\n" + b"signal,+15550004444,B,World,,,,,\n") + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data, + argv_extra=["--json"]) + summary = json.loads(out) # would raise if progress lines leaked in + assert summary["success"] == 2 + assert summary["failed"] == 0 + assert "[1/2]" in err # progress went to stderr + + def test_dry_run_counts_and_flags_missing_files(self, monkeypatch, tmp_path): + data = HEADER + b"signal,+15550003333,A,Hello,,,/definitely/missing.jpg,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data, + argv_extra=["--dry-run", "--json"]) + assert sig == [] and sms == [] + summary = json.loads(out) + assert summary["would_send"] == 1 + assert any("attachment not found" in n for n in summary["notes"]) + + def test_blank_rows_accounted(self, monkeypatch, tmp_path): + data = HEADER + (b"signal,+15550003333,A,Hello,,,,,\n" + b",,,,,,,,\n") + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data, + argv_extra=["--json"]) + summary = json.loads(out) + assert summary["total"] == 2 + assert summary["blank"] == 1 + assert summary["blank"] + summary["success"] + summary["skipped"] + summary["failed"] == summary["total"] + + def test_failed_send_sets_exit_code(self, monkeypatch, tmp_path): + data = HEADER + b"signal,+15550003333,A,Hello,,,,,\n" + code, *_ = run_csv(monkeypatch, tmp_path, data, signal_result=False) + assert code == 1 + + def test_sms_row_requires_service(self, monkeypatch, tmp_path): + data = HEADER + b"sms,+15550003333,A,Hello,,,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert sms == [] + assert "requires service" in err + + def test_voice_on_sms_row_skipped(self, monkeypatch, tmp_path): + data = HEADER + b"sms,+15550003333,A,Hello,,imessage,,~/note.m4a,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert sms == [] + assert "voice" in err + + def test_group_recipient_routed_as_group(self, monkeypatch, tmp_path): + data = HEADER + b"signal,group.ZzBHd3NZ,Team,Hi all,,,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert sig[0]["group_id"] == "group.ZzBHd3NZ" + + def test_formatted_phone_row_not_group(self, monkeypatch, tmp_path): + data = HEADER + b'signal,555-867-5309,Jenny,Hi,,,,,\n' + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert sig[0]["group_id"] is None + + def test_bad_delay_rejected_cleanly(self, monkeypatch, tmp_path): + """--delay abc previously crashed with a ValueError traceback.""" + data = HEADER + b"signal,+15550003333,A,Hello,,,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data, + argv_extra=["--delay", "abc"]) + assert code == 2 # argparse usage error + assert "invalid float value" in err + assert sig == [] + + # -- commas in the message field ------------------------------------ + + def test_quoted_commas_in_message(self, monkeypatch, tmp_path): + """Standard CSV quoting must pass commas through untouched.""" + data = HEADER + b'signal,+15550003333,Al,"Hi, there, friend",,,,,2\n' + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert code == 0 + assert sig[0]["message"] == "Hi, there, friend" + assert "unquoted commas" not in err # no repair needed + + def test_unquoted_commas_merged_into_message(self, monkeypatch, tmp_path): + """Unquoted commas previously shifted every column after 'message' + (text leaked into 'account', etc.); they are now merged back.""" + data = HEADER + b"signal,+15550003333,Al,Hey, how are you, friend,,,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert code == 0 + assert sig[0]["message"] == "Hey, how are you, friend" + assert sig[0]["account"] == "+15550001111" # default, not leaked text + assert "unquoted commas" in err + + def test_unquoted_commas_preserve_trailing_columns(self, monkeypatch, tmp_path): + """Columns after the message (service, voice, delay...) must still + land in the right place after the merge.""" + data = HEADER + b"sms,+15550003333,Al,Hi, there,,imessage,,,2\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert code == 0 + assert sms[0]["message"] == "Hi, there" + assert sms[0]["service"] == "imessage" + + def test_unquoted_comma_repair_noted_in_json(self, monkeypatch, tmp_path): + data = HEADER + b"signal,+15550003333,Al,One, two,,,,,\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data, + argv_extra=["--json"]) + summary = json.loads(out) + assert summary["success"] == 1 + assert any("unquoted commas" in n for n in summary["notes"]) + + def test_short_rows_tolerated(self, monkeypatch, tmp_path): + """Rows with fewer fields than the header must not crash.""" + data = HEADER + b"signal,+15550003333,Al,Hello\n" + code, out, err, sig, sms = run_csv(monkeypatch, tmp_path, data) + assert code == 0 + assert sig[0]["message"] == "Hello" + + +# --------------------------------------------------------------------------- +# CLI argument validation +# --------------------------------------------------------------------------- + +class TestCli: + def run_main(self, monkeypatch, argv): + monkeypatch.setattr(sys, "argv", ["sendmsg", *argv]) + out, err = io.StringIO(), io.StringIO() + monkeypatch.setattr(sys, "stdout", out) + monkeypatch.setattr(sys, "stderr", err) + code = 0 + try: + sendmsg.main() + except SystemExit as e: + code = e.code or 0 + return code, out.getvalue(), err.getvalue() + + def test_no_action_errors(self, monkeypatch): + code, out, err = self.run_main(monkeypatch, []) + assert code == 1 + assert "--list-groups" in err # new command advertised + + def test_multiple_actions_rejected(self, monkeypatch): + code, out, err = self.run_main(monkeypatch, ["--list-signal", "--link-signal"]) + assert code == 1 + assert "only one action" in err + + def test_sms_fans_out_to_all_recipients(self, monkeypatch): + calls = [] + monkeypatch.setattr(sendmsg, "send_one_sms", + lambda **kw: calls.append(kw) or True) + code, out, err = self.run_main( + monkeypatch, + ["--sms", "--to", "+15550001111", "+15550002222", + "--text", "hi", "--service", "sms"], + ) + assert code == 0 + assert [c["to"] for c in calls] == ["+15550001111", "+15550002222"] + + def test_list_groups_refuses_placeholder_account(self, monkeypatch): + monkeypatch.setattr(sendmsg, "SIGNAL_DEFAULT_ACCOUNT", "+1234567890") + code, out, err = self.run_main(monkeypatch, ["--list-groups"]) + assert code == 1 + assert "placeholder" in err + + def test_list_groups_output(self, monkeypatch): + monkeypatch.setattr(sendmsg, "signal_rest_get", lambda ep: [ + {"name": "Ops", "id": "group.QWJj", "members": ["+1", "+2"], "blocked": False}, + {"name": None, "internal_id": "Zz09"}, + ]) + code, out, err = self.run_main( + monkeypatch, ["--list-groups", "--account", "+15550001111"]) + assert code == 0 + assert "Ops" in out + assert "group.QWJj" in out + assert "2 member(s)" in out + assert "(unnamed)" in out + + def test_list_groups_url_encodes_account(self, monkeypatch): + seen = {} + + def fake_get(endpoint): + seen["endpoint"] = endpoint + return [] + + monkeypatch.setattr(sendmsg, "signal_rest_get", fake_get) + self.run_main(monkeypatch, ["--list-groups", "--account", "+15550001111"]) + assert seen["endpoint"] == "/v1/groups/%2B15550001111"