4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
cbe47a5f6c |
perf(sms): close the outstanding Phase B and D items
Some checks failed
doc-engine / engine (push) Waiting to run
doc-engine / consumer (push) Waiting to run
nigig-map / test (push) Waiting to run
sms / gates (push) Waiting to run
sms / robius-sms (push) Waiting to run
sms / android (push) Waiting to run
sms / nigig-sms (push) Waiting to run
sms / supply-chain (push) Waiting to run
repo hygiene / hygiene (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
An audit of Phase 0 through D against the tree found three tasks marked
done in prose but absent from the code. This closes them.
D5 -- bulk send still blocked the UI thread.
The send loop ran synchronously in handle_send_bulk.
SmsManager.sendTextMessage queues to the radio and rate-limits, so a
200-recipient batch was a multi-minute ANR with no progress and no
way to tell whether anything was happening. Phase E8's throttle
bounded the worst case at 30 sends, but bounded blocking is still
blocking -- and I said as much when deferring it.
Now uses the worker pattern D1 established: spawn, publish progress
through a mutex-guarded slot, SignalToUI, drain on the UI thread. The
status line counts up ("Sending 12/200…") instead of freezing.
BULK_SEND_IN_FLIGHT prevents two overlapping batches.
D2 -- the ContentObserver, the half I left open.
Phase D removed the 5-second poll that re-armed itself via redraw()
and stopped the app ever idling. That fixed the busy loop but left a
gap I documented rather than closed: a message arriving while the app
was open did not surface until the next Resume or manual pull.
Adds SmsInboxObserver.java -- a ContentObserver on content://sms,
registered with a main-Looper Handler, idempotent so onResume can call
it freely -- compiled and dexed by the existing build.rs pipeline and
loaded through the same in-memory dex loader as the receivers.
onChange calls into Rust, which does two cheap things: set an atomic,
and invoke a registered waker. The waker matters. robius-sms has no UI
dependency and cannot call SignalToUI itself, so without it the flag
would only be observed on the next event-loop turn that happened for
some other reason -- which, with the poll gone, might be never while
the app sits idle. The app registers SignalToUI::set_ui_signal, so
this is a genuine push.
Native binding is dynamic, not #[no_mangle], for the same reason as
E6: the class comes from an in-memory dex and is not on the JVM's
search path.
B0 (wider) -- 23 crates declared robius-sms and never called it.
Phase B removed the three declarations that put RUSTSEC advisories on
nigig-build and explicitly flagged the rest as "the same latent
problem, sweep separately". This is that sweep: every crate with zero
references to robius_sms in its sources loses the dependency.
nigig-mpesa, nigig-pay and nigig-sms keep it -- they are the only real
users. nigig-system-prefs only mentions robius-sms in its package
description, so its manifest is untouched.
Cargo.lock loses another 21 lines.
Verified: 13/13 CI checks. clippy -D warnings clean on host and
aarch64-linux-android; the Android build compiles, javac-builds and
dexes the new observer class. cargo deny still "advisories ok, bans ok,
licenses ok, sources ok". clippy ratchet holds at 49. Sampled four of
the 23 stripped crates plus all five I edited; all build.
Pre-existing and unrelated: nigig-map fails to compile on pristine
origin/main (12 errors in view.rs, a Script/Widget derive problem), so
pageflipnav and anything else reaching it cannot be checked here. I
touched no files under crates/apps/map.
NOT verified on a device. The observer's registration, the onChange
callback and the waker all need an emulator or handset with a live SMS
provider; this sandbox has neither, and there is still no CI runner.
|
||
|
|
a38c41c00a |
security(sms): stop persisting message bodies, throttle sends (Phase E)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-map / test (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
E1 -- the inbox was written to disk in plaintext. offline_store wrote every SMS body to app_data_dir/offline_store/sms_messages.json as pretty-printed JSON. SMS is the transport for OTPs, banking codes and M-Pesa confirmations, so that file was the user's complete authentication history sitting in app-private storage -- readable by anything running as the same UID, and included in backups. This repository already knew the answer. THREAT_MODEL.md T-I2 records "Raw SMS persisted to PSV file" as fixed in Phase 0, with raw_message omitted from save_to_disk() so it "lives in memory only". The SMS app then re-introduced the same defect at larger scale: the entire inbox rather than just M-Pesa messages, and with no retention limit until D6. OfflineSmsMessage.body is now #[serde(skip)]. Dropping the field rather than encrypting it is the deliberate choice: every consumer already reads the device provider FIRST and writes the cache second (nigig-sms fetch_from_device, and the mpesa and pay transaction pages), so the provider is the system of record and no body needs to survive a restart. Encryption would keep the plaintext reachable to anything holding the key. Not writing it removes the asset. Two consequences handled: sms_key() no longer hashes the body, since a reloaded row has an empty one and dedupe would otherwise never match its own cached entry and grow a duplicate per refresh; and the cached first paint shows a neutral placeholder rather than a blank preview for the instant before the provider read lands. E9 -- the Linux backend's dependencies were pure cost. robius-sms declared polkit =0.17.0 and gio =0.17.0 for target_os = "linux". sys/linux.rs references neither: all twelve functions return Err(PermanentlyUnavailable). Those two crates dragged in glib and proc-macro-error and were the origin of RUSTSEC-2024-0370 and RUSTSEC-2024-0429 for every consumer of this crate. Deleting the block removes 340 lines from Cargo.lock. polkit, gio, glib and proc-macro-error no longer appear in the workspace at all, which also closes the LGPL-2.1 linkage question outright rather than routing around it as Phase B did for nigig-build alone. E4 -- ROBIUS_SMS_BOOT_LIB was a code-injection vector. build.rs interpolated that environment variable straight into a Java string literal, which is then compiled, dexed and loaded at runtime with the app's full permissions. A value containing a quote closes the literal and injects arbitrary Java that runs on the device at boot. Build-time environment is not trusted input. Now validated against [A-Za-z0-9_]+ and the build fails loudly otherwise. Tested both ways: an exec payload is rejected, a legitimate name builds. E5 -- undefined behaviour in the dex loader. new_direct_byte_buffer was handed RECEIVER_BYTECODE.as_ptr() as *mut u8 -- a &'static [u8] in .rodata cast to a mutable pointer, when the API is documented as taking writable memory and InMemoryDexClassLoader may write through it. Now copies into an owned allocation and leaks it, which is correct rather than lazy: the buffer backs a ClassLoader cached in a OnceLock for the process lifetime. E6 -- two bindings for one native method. rustRestoreSchedules was both exported #[no_mangle] and registered dynamically via register_native_methods. Which one won was unspecified. Kept the dynamic one, because the class is loaded from an in-memory dex and is not on the JVM's search path, so symbol binding is not guaranteed to find it. E8 -- no send rate limiting. Nothing capped send rate, and the bulk UI exists to blast a scraped directory. Android's practical throttle is ~30 messages per 30 minutes per app, past which sends are silently dropped -- so an unthrottled batch both overspends and fails opaquely. Adds SendRateLimiter, a pure token bucket taking an explicit clock so it is unit-testable without sleeping, wired into the bulk sender. A 200-recipient blast now stops at 30 and says why. E10 -- robius-sms carried no license field, so cargo-deny needed a [[licenses.clarify]] override asserting one. Stated in the manifest; override removed. E2 was already satisfied by D6 (retention capped at 5,000). E7/E11 are documented rather than fixed, which is the honest status: delivery confirmation needs real PendingIntents plumbed through (A8 documents that Ok != delivered), and sender validation cannot be solved client-side. Both are now rows in THREAT_MODEL.md instead of findings in a markdown report -- along with T-I2b for E1 and T-I4 for E3, which is NOT done: scheduled message bodies are still plaintext in SharedPreferences. CI: adds a gate asserting OfflineSmsMessage.body keeps #[serde(skip)]. Removing that attribute silently resumes writing plaintext and nothing else would fail. Negative-tested. deny-nigig-build.toml drops to 2 exemptions (from 5 before Phase B). Tests: robius-sms 21 -> 25. Verified: 12/12 CI jobs, clippy -D warnings clean on host and aarch64-linux-android, cargo deny "advisories ok, bans ok, licenses ok, sources ok", clippy ratchet holds at 49. |
||
|
|
817ba02625 |
build: drop the unused robius-sms dependency and its two advisories (Phase B)
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-map / test (push) Has been cancelled
nigig-build declared robius-sms and robius-location and called neither:
zero references to robius_sms or robius_location anywhere under
nigig-build/src. That dead dependency pulled in
robius-sms -> polkit -> gio -> glib -> glib-macros -> proc-macro-error
which is the ONLY reason deny-nigig-build.toml carried
RUSTSEC-2024-0370 (proc-macro-error, unmaintained)
RUSTSEC-2024-0429 (glib VariantStrIter unsoundness)
plus an unanswered LGPL-2.1 distribution question about linking polkit
into a shipped mobile binary -- all of it for code that never ran.
Removing the direct dependency alone was NOT enough, which is the part
worth recording. `cargo tree -p nigig-build -i polkit` showed three
paths, not one: the direct declaration, and two more through nigig-core
and nigig-uikit. Both of those also declare robius-sms and also never
use it. All three declarations had to go before polkit left the graph.
nigig-core keeps robius-location: unlike the others it genuinely uses it,
in src/location.rs.
Verified, not assumed:
- before: cargo tree -p nigig-build -i polkit resolved, three paths
- after: polkit, gio and proc-macro-error no longer resolve at all
- cargo deny check -> "advisories ok, bans ok, licenses ok, sources ok"
with two fewer ignores (5 -> 3)
- nigig-build, nigig-core, nigig-uikit and nigig-sms all still compile
- Cargo.lock loses 5 lines
Also in this commit:
- A CI gate so the declarations cannot come back. Deliberately scoped
to robius-sms/robius-location on these three manifests rather than a
blanket `cargo machete`: five other unused dependencies exist here
(chrono, futures, postcard, rand, serde_json) and a gate that is red
on its first run gets switched off. Negative-tested by re-adding the
dependency and confirming the gate fails.
- Three pages carried the same placeholder string telling the user
they were looking at a "RobrixStackNavigationView destination ...
just like SMS conversation screens". That is user-visible UI copy,
not a comment. Replaced with text describing the page. The identical
"This follows the SMS/Home pattern" comment in the same three files
now says what the code does instead of naming another module.
Note the scope limit: 26 of the 29 crates in this workspace declare
robius-sms and never use it. This commit fixes the three that put
advisories on nigig-build. The rest are the same latent problem and
should be swept separately.
|
||
|
|
c0143a3af8 |
build: track the workspace root Cargo.lock (Phase 0.1)
CI already runs with --locked: nigig-build.yml asserts
`cargo metadata --locked` and `git diff --exit-code -- Cargo.lock`,
and doc-engine.yml builds every target with --locked. None of that
can hold while the root lockfile is gitignored -- `--locked` fails
outright with no lockfile to check against, so in practice every
build re-resolved and a transitive dependency could change under CI
without any commit recording it.
The nigig-build workflow comment already claimed this was done
("Blocked until Cargo.lock was committed in Phase 0.2"); it was not.
This commits it.
The ignore rule becomes `**/Cargo.lock` + `!/Cargo.lock` so the root
lock is tracked while incidental nested locks stay ignored. The three
payment-crate locks keep their existing negations and stay tracked.
Generated with the pinned toolchain from rust-toolchain.toml (1.97.1)
against makepad rev a79f0dce; 566 packages. Verified `cargo metadata
--locked` exits 0.
|