name: email # Phase 0.3 of the nigig-email remediation plan. # # Before this file existed, nigig-email had NO CI of any kind. That is # how a binary with unbalanced braces reached main and stayed there: # `cargo check -p nigig-email` failed on main while `--lib` passed, so # the library was fine and the BINARY had never compiled once. Nobody # had ever run the crate standalone. # # It is also how four unused dependencies survived, one of them # robius-location -- the exact dependency SMS Phase B removed from three # other crates because it pulls polkit/gio/glib and two RUSTSEC # advisories in for code that is never called. # # Scope note: this crate is a UI shell over nigig-core::email_account, # email_store and email_worker. The domain logic lives in nigig-core and # is host-testable; the SMTP transport is not testable here at all # (no live server in CI), so the tests that matter are the pure ones and # this workflow gates those plus compilation of both targets. on: push: paths: - 'crates/apps/nigig-email/**' - 'crates/nigig-core/src/email_account.rs' - 'crates/nigig-core/src/email_send.rs' - 'crates/nigig-core/src/email_store.rs' - 'crates/nigig-core/src/email_worker.rs' - 'crates/nigig-core/src/mail_backend.rs' - 'crates/nigig-core/src/mail_proxy.rs' - 'crates/nigig-core/src/secret.rs' - 'tools/test-email-coverage.sh' - 'Cargo.lock' - 'Cargo.toml' - 'rust-toolchain.toml' - '.forgejo/workflows/email.yml' pull_request: paths: - 'crates/apps/nigig-email/**' - 'crates/nigig-core/src/email_account.rs' - 'crates/nigig-core/src/email_send.rs' - 'crates/nigig-core/src/email_store.rs' - 'crates/nigig-core/src/email_worker.rs' - 'crates/nigig-core/src/mail_backend.rs' - 'crates/nigig-core/src/mail_proxy.rs' - 'crates/nigig-core/src/secret.rs' - 'tools/test-email-coverage.sh' - 'Cargo.lock' - 'Cargo.toml' - 'rust-toolchain.toml' - '.forgejo/workflows/email.yml' jobs: # --------------------------------------------------------------------- # Source-scanning gates. No toolchain needed, so they run first and # fail fast. # --------------------------------------------------------------------- gates: runs-on: ubuntu-latest timeout-minutes: 10 steps: - uses: actions/checkout@v4 # S2 of the assessment. SmtpConfig carries a plaintext password and # derives Serialize, so anything that reuses it for storage leaks # the credential. EmailAccount is the persistable half and must # never grow a password field. # # There is a unit test asserting the serialised account contains # neither the secret nor a field named "password"; this is the # cheaper structural check that catches the field being added even # if someone deletes the test. - name: The persistable account type must not carry a password run: | set -euo pipefail # Look only at the EmailAccount struct body. body=$(sed -n '/^pub struct EmailAccount {/,/^}/p' \ crates/nigig-core/src/email_account.rs) if echo "$body" | grep -nE '^[[:space:]]*pub[[:space:]]+password'; then echo echo "ERROR: EmailAccount declares a password field." echo "That type is written to disk. The secret is returned" echo "separately by AccountDraft::validate and held in memory" echo "for the session only. Persisting it needs the platform" echo "keystore first (plan item A1 / C1f)." exit 1 fi echo "OK" # A1 / S2. SmtpConfig used to carry `pub password: String` and derive # Serialize, so on wasm the entire struct -- password included -- was # serde_json-encoded and POSTed to the email API. Any proxy logging # request bodies captured the credential. # # Two properties must hold, and both are structural rather than # advisory: # 1. the password field is a `Secret`, whose Debug renders "***"; # 2. SmtpConfig does NOT derive Serialize, so it cannot be # serialised wholesale by accident. The wasm path builds its # JSON field by field instead, naming the secret at exactly one # line. - name: The SMTP password must be a Secret, and the config unserialisable run: | set -euo pipefail bad=0 decl=$(sed -n '/^pub struct SmtpConfig {/,/^}/p' \ crates/nigig-core/src/email_worker.rs \ | grep -E '^[[:space:]]*pub[[:space:]]+password' || true) if ! echo "$decl" | grep -q 'Secret'; then echo "ERROR: SmtpConfig.password is not a Secret:" echo " $decl" echo "A plain String Debug-prints and serialises the credential." bad=1 fi # The derive list immediately above the struct. derives=$(grep -B2 '^pub struct SmtpConfig {' \ crates/nigig-core/src/email_worker.rs | grep '#\[derive' || true) if echo "$derives" | grep -qE 'Serialize|Deserialize'; then echo "ERROR: SmtpConfig derives Serialize/Deserialize:" echo " $derives" echo "That is how the plaintext password reached the wire. Build" echo "the request body field by field instead." bad=1 fi [ "$bad" -eq 0 ] || exit 1 echo "OK" # A5. The wasm proxy POST carries the credential, so the endpoint must # be same-origin or https. The setter used to accept any String, # including http://, which sends the password in clear text. - name: The email API endpoint must be validated before use run: | set -euo pipefail if ! grep -q 'fn email_api_url_is_safe' \ crates/nigig-core/src/email_worker.rs; then echo "ERROR: email_api_url_is_safe() is gone." echo "set_email_api_url must reject non-https absolute URLs." exit 1 fi if ! grep -q 'email_api_url_is_safe(&url)' \ crates/nigig-core/src/email_worker.rs; then echo "ERROR: set_email_api_url no longer calls the validator." exit 1 fi echo "OK" # A3. The TLS mode was implicit -- inherited from lettre's defaults via # a bare port match. Those defaults are safe, but nothing asserted it, # so a refactor could have removed encryption with no test failing. # Every port must map to a TLS mode; there is no cleartext arm. - name: SMTP transport must never be cleartext run: | set -euo pipefail if ! grep -q 'fn tls_mode_for_port' \ crates/nigig-core/src/email_worker.rs; then echo "ERROR: tls_mode_for_port() is gone; the TLS policy is" echo "implicit again." exit 1 fi if grep -nE 'Tls::None|Tls::Opportunistic' \ crates/nigig-core/src/email_worker.rs; then echo echo "ERROR: a non-mandatory TLS mode above. This transport always" echo "carries credentials; Opportunistic silently accepts a" echo "downgrade." exit 1 fi echo "OK" # B1. The recipient field is labelled "To (comma-separated)" and the # worker did `to.parse()` into a single lettre Mailbox, so ANY list # failed with "Invalid to: ...". The tab named "Bulk" could reach # exactly one person -- the crate's headline feature did not work. # # The send path must go through email_send::parse_recipients, and must # add every accepted recipient rather than one. - name: Multi-recipient send must not regress to a single mailbox run: | set -euo pipefail bad=0 if ! grep -q 'email_send::parse_recipients' \ crates/nigig-core/src/email_worker.rs; then echo "ERROR: send_email_impl no longer parses the recipient list." bad=1 fi # The old shape, in non-comment code. if grep -nE 'let[[:space:]]+to_mbox[[:space:]]*:[[:space:]]*Mailbox' \ crates/nigig-core/src/email_worker.rs \ | grep -vE '^[^:]*:[0-9]*:[[:space:]]*(//|///|/\*|\*)'; then echo echo "ERROR: a single-Mailbox parse of the whole To field is back." echo "Mailbox::parse accepts ONE address; a comma-separated list" echo "fails outright. Use email_send::parse_recipients." bad=1 fi if ! grep -q 'for r in &parsed.accepted' \ crates/nigig-core/src/email_worker.rs; then echo "ERROR: the builder no longer adds every recipient." bad=1 fi [ "$bad" -eq 0 ] || exit 1 echo "OK" # B5. Both spawn_* functions used to fire unconditionally, so a double # tap sent the message twice -- irreversible, to a real person. The # guard is an AtomicBool swap, the same control robius-sms uses. - name: A second concurrent send must be refused run: | set -euo pipefail # Look only at PRODUCTION code. The unit tests for this guard # also contain `SEND_IN_FLIGHT.swap(true`, so grepping the whole # file passes even when spawn_send_email has lost the guard -- # which is exactly the regression this gate exists to catch. I # found that by negative-testing: deleting the guard left the # gate green. prod=$(sed -n '1,/^#\[cfg(test)\]/p' \ crates/nigig-core/src/email_worker.rs) if ! echo "$prod" | grep -q 'SEND_IN_FLIGHT.swap(true'; then echo "ERROR: the in-flight guard is gone from spawn_send_email." echo "Without the swap, a double tap sends the message twice --" echo "irreversible, to a real recipient." exit 1 fi echo "OK" # B6. abandon_send() shipped as DEAD CODE: it existed in nigig-core # and nothing in the UI called it, so the user had no way to stop # waiting on a hung send. Found by auditing the tree rather than my # own notes. # # A control the user cannot reach is not a control. This gate makes # the wiring, not just the function, the thing being checked. - name: The abandon-send control must be reachable from the UI run: | set -euo pipefail if ! grep -q 'pub fn abandon_send' \ crates/nigig-core/src/email_worker.rs; then echo "ERROR: abandon_send() is gone from nigig-core." exit 1 fi # Exclude comments. The block explaining WHY this control exists # mentions abandon_send() by name, so a naive grep passes even # when the call is unwired -- the same flaw the B5 gate had. I # found both the same way: by deleting the fix and watching the # gate stay green. if ! grep -rn 'abandon_send()' crates/apps/nigig-email/src \ | grep -vE ':[[:space:]]*(//|///|/\*|\*)' \ | grep -q .; then echo "ERROR: abandon_send() exists but no UI calls it." echo "It shipped that way once. A stop control the user cannot" echo "reach is dead code wearing a safety label." exit 1 fi echo "OK" # C1a/C1b. Two backends are supported (IMAP-on-device and a # server-side proxy), so the security difference between them must # stay visible and the persisted settings must stay secret-free. # # IMAP keeps a REUSABLE mailbox password on the device -- for most # people that is the password-reset channel for every other account # they own. A revocable proxy token is strictly safer. The chooser # must not present them as equivalent. - name: The backend chooser must not hide the security trade-off run: | set -euo pipefail bad=0 if ! grep -q 'fn stores_reusable_password' \ crates/nigig-core/src/mail_backend.rs; then echo "ERROR: stores_reusable_password() is gone. The UI can no" echo "longer tell the user which backend keeps their password." bad=1 fi # IMAP is raw TCP; a browser cannot open one. Offering it on wasm # would be a dead option the user can select and never use. if ! grep -q 'fn is_available_on_wasm' \ crates/nigig-core/src/mail_backend.rs; then echo "ERROR: is_available_on_wasm() is gone; the chooser can" echo "offer IMAP in a browser, where it cannot work." bad=1 fi # BackendSettings is written to disk with the account. for field in password token secret; do if sed -n '/^pub struct ProxySettings {/,/^}/p;/^pub struct ImapSmtpSettings {/,/^}/p' \ crates/nigig-core/src/mail_backend.rs \ | grep -qE "^[[:space:]]*pub[[:space:]]+$field"; then echo "ERROR: backend settings declare a '$field' field." echo "Those structs are persisted. Secrets go in a Secret," echo "returned separately by BackendDraft::validate." bad=1 fi done [ "$bad" -eq 0 ] || exit 1 echo "OK" # B3. `port.parse().unwrap_or(587)` silently rewrote a typo'd port, # and because the port selects the transport (465 implicit TLS vs # 587 STARTTLS) that silently changed the security posture too. # AccountDraft::validate now rejects a malformed port instead. - name: A malformed port must not silently become a default run: | set -euo pipefail # Exclude comment lines: email_account.rs documents the old # behaviour by quoting it, and a gate that trips on its own # rationale is a gate nobody keeps. if grep -rnE 'parse\(\)[[:space:]]*\.unwrap_or\(587\)' \ crates/apps/nigig-email/src \ crates/nigig-core/src/email_account.rs \ crates/nigig-core/src/email_store.rs \ | grep -vE '^[^:]*:[0-9]*:[[:space:]]*(//|/\*|\*|///)'; then echo echo "ERROR: a port is being parsed with unwrap_or(587)." echo "That turns '465x' into 587 and switches the transport" echo "from implicit TLS to STARTTLS without telling the user." echo "Return AccountError::PortInvalid instead." exit 1 fi echo "OK" # A3, the byte-offset slicing class of bug. `&s[..n]` panics when n # is not a UTF-8 boundary; one inbound message containing emoji or # non-Latin text took down the whole SMS conversation list on every # frame until it was deleted. Email bodies are equally untrusted -- # more so, they arrive with arbitrary MIME -- and email_store's # preview_line runs per visible row inside draw_walk. # # The SMS crates carry the same gate. This one is a hard zero # because the email code has never had such a slice. - name: No byte-offset string slicing in the email text helpers run: | set -euo pipefail PATTERN='[A-Za-z_][A-Za-z0-9_]*\[(\.\.=?[A-Za-z0-9_]+|[A-Za-z0-9_]+\.\.)[A-Za-z0-9_]*\]' # flat[..idx] in preview_line is proven safe -- idx comes from # char_indices() -- and carries a comment saying so. Exclude it # by line content rather than by muting the whole file. hits=$(grep -rnE --include='*.rs' "$PATTERN" \ crates/apps/nigig-email/src \ crates/nigig-core/src/email_store.rs \ crates/nigig-core/src/email_account.rs \ | grep -vE '^[^:]*:[0-9]*:[[:space:]]*(//|/\*|\*)' \ | grep -v 'flat\[\.\.idx\]' || true) if [ -n "$hits" ]; then echo "$hits" echo echo "ERROR: byte-offset slice(s) above. These panic when the" echo "offset is not a char boundary, which any email body" echo "containing emoji or non-Latin text will produce. Use" echo "char_indices() to find a real boundary first." exit 1 fi echo "OK" # C4b wired the inbox to a real backend fetch, so sample_thread() is # now TEST-ONLY: it must never be called from the UI crate at all. # A hard zero keeps the placeholder from sneaking back into the # shipping path under a new name or call site. - name: Development sample data must not reach the UI run: | set -euo pipefail count=$(grep -rn 'sample_thread' --include='*.rs' \ crates/apps/nigig-email/src | wc -l) || true if [ "$count" -gt 0 ]; then grep -rn 'sample_thread' --include='*.rs' crates/apps/nigig-email/src echo echo "ERROR: sample_thread() is referenced $count times in the UI." echo "The inbox fetches real mail now (C4b). Sample data is for" echo "tests only; any UI reference is a regression to the" echo "placeholder list." exit 1 fi echo "OK" # --------------------------------------------------------------------- # The email domain logic. Pure, host-testable, no display and no # network -- this is the job that can actually prove something. # --------------------------------------------------------------------- email-domain: runs-on: ubuntu-latest # Two full compiles now (plain tests + the instrumented coverage run), # so allow more than the pre-coverage 30 minutes. timeout-minutes: 45 steps: - uses: actions/checkout@v4 # nigig-core pulls sqlite and the matrix client. - name: Install native dependencies run: | sudo apt-get update -qq sudo apt-get install -y -qq \ pkg-config libssl-dev libsqlite3-dev # NOT actions/setup-rust@v1 -- that action does not exist on this # instance's registry and fails the job in "Set up job" before any # step runs. See .forgejo/RUNNER.md. - name: Install the declared toolchain run: | set -e version="$(sed -n 's/^[[:space:]]*channel[[:space:]]*=[[:space:]]*"\([^"]*\)".*/\1/p' \ rust-toolchain.toml | head -n 1)" curl --fail --location --proto '=https' --tlsv1.2 https://sh.rustup.rs -o /tmp/rustup-init chmod 700 /tmp/rustup-init /tmp/rustup-init -y --profile minimal --default-toolchain "$version" \ --component rustfmt --component clippy --no-modify-path echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - name: Email domain tests run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store:: finance_report::" # A floor, not a ratchet: these tests are cheap, pure, and the # number should only go up. 38 at Phase 0; 154 after C1c/C1d; # 195 after the Phase C/D completion; 214 after Phase E (proptest + # the SMTP sink integration tests); 216 after the §8 TLS handshake # tests; 234 after the trip-receipt extraction and finance report; # 237 after the report-email attachment path. - name: The email domain test suite must not shrink run: | set -euo pipefail FLOOR=230 out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store:: finance_report:: 2>&1)" # C1e/C1f: the IMAP transport and the platform keystore are both # feature-gated (native only). They must still COMPILE when the # features are on, or the direct backend's read path and credential # persistence silently rot. A check, not a test: the socket and the # secret service are not exercised, only type-checked. - name: The IMAP and keystore features must compile run: cargo check --locked -p nigig-core --features imap,keystore # §8 of the assessment: "the wasm path has not been built." The # credential-bearing proxy POST (call_email_api, WasmFetchTransport, # set_email_api_url) is #[cfg(target_arch = "wasm32")], so a host-only # build never type-checks it — which is how a browser-only breakage # would reach main. This installs the wasm target and checks the # email domain's wasm half. (no-default-features: the `native` feature # would pull tokio/reqwest/lettre, which do not exist for wasm.) - name: The wasm path must compile run: | set -euo pipefail rustup target add wasm32-unknown-unknown cargo check --locked --target wasm32-unknown-unknown -p nigig-core \ --no-default-features --features async-rt # A coverage number that is only printed drifts down. This enforces a # whole-domain floor plus per-file floors on the files that have # actually harboured bugs (the SMTP password serialisation, the bulk # tab that could not bulk-send, the proxy parser). The script is # self-contained: its own rustup, cargo home and target dir, removed # on exit. Same pattern as tools/test-pdf-coverage.sh. - name: Coverage floors run: ./tools/test-email-coverage.sh # --------------------------------------------------------------------- # The UI crate. Pulls the whole Makepad stack, so it is the slow job # and needs the GUI system libraries. # --------------------------------------------------------------------- nigig-email: runs-on: ubuntu-latest timeout-minutes: 60 steps: - uses: actions/checkout@v4 - name: Install native dependencies run: | sudo apt-get update -qq sudo apt-get install -y -qq \ pkg-config libwayland-dev libxcursor-dev libxrandr-dev \ libxi-dev libx11-dev libgl1-mesa-dev libasound2-dev \ libglib2.0-dev libssl-dev libsqlite3-dev libudev-dev \ libpulse-dev libxkbcommon-dev - name: Install the declared toolchain run: | set -e version="$(sed -n 's/^[[:space:]]*channel[[:space:]]*=[[:space:]]*"\([^"]*\)".*/\1/p' \ rust-toolchain.toml | head -n 1)" curl --fail --location --proto '=https' --tlsv1.2 https://sh.rustup.rs -o /tmp/rustup-init chmod 700 /tmp/rustup-init /tmp/rustup-init -y --profile minimal --default-toolchain "$version" \ --component rustfmt --component clippy --no-modify-path echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" # --all-targets on purpose. `--lib` alone passed for the entire # time main.rs was syntactically invalid, which is precisely the # failure this job exists to prevent. - name: Check (lib AND bin) run: cargo check --locked -p nigig-email --all-targets - name: Test run: cargo test --locked -p nigig-email # E6: the shared conversation kit (nigig-uikit/src/shared/conversation) # is consumed by BOTH SMS and email, so a regression there moves two # features at once. It now has its own tests pinning the click guard # and payload; run them so an email-driven regression cannot silently # surface in SMS. - name: Conversation kit tests run: cargo test --locked -p nigig-uikit --lib -- conversation # Phase 0.6. A hard gate, not report-only: this crate is 1,100 # lines and already formats clean, so there is no pre-existing # drift to grandfather in. Contrast sms.yml and nigig-map.yml, # which are report-only because they inherited hundreds of diffs. - name: Formatting run: | cargo fmt -p nigig-email -- --check \ || { echo "run: cargo fmt -p nigig-email"; exit 1; } # Ratchet at the measured baseline, which is ZERO. # # I first set this to 2, having seen two `unexpected_cfgs` warnings # for `native_activity` emitted by the app_main! macro. Measuring # properly with the same dedupe this script uses gives 0 -- those # two are attributed to the bin target and filtered out by the # package_id check. A baseline above the real count is not a # harmless margin: this script fails when n < BASELINE precisely so # that slack cannot hide a regression. - name: Clippy ratchet (nigig-email-owned diagnostics only) run: | set -euo pipefail BASELINE=0 cargo clippy --locked -p nigig-email --all-targets \ --message-format=json > /tmp/clippy-email.json 2>/tmp/clippy-email.err || true cat /tmp/clippy-email.err || true BASELINE="$BASELINE" python3 - <<'PY' import json, os, sys baseline = int(os.environ['BASELINE']) owned, seen = [], set() with open('/tmp/clippy-email.json') as fh: for line in fh: try: m = json.loads(line) except ValueError: continue if m.get('reason') != 'compiler-message': continue if 'nigig-email' not in m.get('package_id', ''): continue msg = m['message'] if msg.get('level') not in ('warning', 'error'): continue # --all-targets compiles lib and lib-test, duplicating # every diagnostic; dedupe on rendered text. key = msg.get('rendered', '') if key in seen: continue seen.add(key) owned.append(msg) n = len(owned) print("found %d nigig-email diagnostics, baseline %d" % (n, baseline)) if n > baseline: for msg in owned: sys.stdout.write(msg.get('rendered', '')) print() print("ERROR: %d diagnostics, up from %d." % (n, baseline)) sys.exit(1) if n < baseline: print() print("Good: down to %d. Lower BASELINE in this file to %d " "so the progress cannot be undone." % (n, n)) sys.exit(1) print("OK") PY # --------------------------------------------------------------------- # Supply chain. # --------------------------------------------------------------------- supply-chain: runs-on: ubuntu-latest timeout-minutes: 15 steps: - uses: actions/checkout@v4 # Phase 0.5. Four dependencies were declared and never referenced, # one of them robius-location, which drags polkit/gio/glib and two # RUSTSEC advisories in for code that is never called. - name: nigig-email must not declare dependencies it never uses run: | set -euo pipefail bad=0 # chrono is used by inbox.rs::format_thread_time. Everything # else in the manifest must appear in src/ under its # underscored crate name. for dep in serde serde_json robius-location robius-sms lettre; do declared=$(grep -cE "^[[:space:]]*${dep}[[:space:]]*=" \ crates/apps/nigig-email/Cargo.toml || true) [ "$declared" -eq 0 ] && continue underscored="${dep//-/_}" used=$(grep -rl "$underscored" crates/apps/nigig-email/src 2>/dev/null | wc -l) if [ "$used" -eq 0 ]; then echo "ERROR: nigig-email declares $dep but never uses it." bad=1 fi done if [ "$bad" -ne 0 ]; then echo echo "Unused dependencies enlarge the attack surface and the" echo "licence obligations for no benefit. Remove them." exit 1 fi echo "OK" - name: Lockfile must be committed and current run: | set -euo pipefail test -f Cargo.lock || { echo "ERROR: Cargo.lock is not committed."; exit 1; } git diff --exit-code -- Cargo.lock - name: Reject whitespace errors run: git diff --check "$(git rev-list --max-parents=0 HEAD | tail -1)"..HEAD || git diff --check