diff --git a/AGENTS.md b/AGENTS.md index 104d5b890..6414c9122 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ Things that look correct and are not: - **Locks.** `session_locks` serializes Signal encrypt/decrypt per protocol address; `chat_lanes` (`ChatLane::enqueue_lock` in `src/client.rs`) serializes *incoming* processing per chat. Outgoing sends are deliberately not per-chat locked — WA Web doesn't lock them either. - **Wire-tagged enums.** Every protocol enum derives `WireEnum`, and its `#[wire = ...]` attribute is the single source of truth for the wire value. Do not also derive `serde::Serialize`/`Deserialize` or add `#[serde(rename_all)]` — the derive owns both. In tagged mode it generates a sibling `Tag`; parsers must dispatch on `Tag::try_from(node.tag.as_ref())` rather than string literals, so renaming a tag stays a one-attribute change. Modes and attributes: `agent_docs/protocol_architecture.md`. - **Event payloads are a frozen API.** Sealed with `#[non_exhaustive]` + `#[derive(bon::Builder)]` and constructed via `Type::builder()…build()`; a maybe-absent field is `Option`, never an empty-string or zero sentinel. The full stability policy is the `Event` doc comment in `wacore/src/types/events.rs`. -- **Generated files are generated, not edited.** `wacore/src/iq/abprops.rs`, `wacore/src/iq/mex_operations.rs`, `wacore/appstate/src/schemas.rs`, `wacore/src/types/wire_enums.rs`, `wacore/binary/src/tokens.json`, `waproto/src/whatsapp.proto` and `wacore/src/version/generated.rs` all come out of `cargo run -p whatspec-codegen`, together, from one pinned whatspec commit. An action or flag the protocol carries but the bundle no longer builds goes in a hand-written sibling (`wacore/appstate/src/schemas_unlisted.rs`, `props::stale`), never in the generated file. `wire_enums.rs` binds only the catalog entries listed in the emitter's `WANTED`, because 88 of the 403 have a synthetic name and names repeat across modules; the variants themselves always come from the bundle. +- **Generated files are generated, not edited.** `wacore/src/iq/abprops.rs`, `wacore/src/iq/mex_operations.rs`, `wacore/appstate/src/schemas.rs`, `wacore/src/types/wire_enums.rs`, `wacore/src/iq/targets.rs`, `wacore/binary/src/tokens.json`, `waproto/src/whatsapp.proto` and `wacore/src/version/generated.rs` all come out of `cargo run -p whatspec-codegen`, together, from one pinned whatspec commit. An action or flag the protocol carries but the bundle no longer builds goes in a hand-written sibling (`wacore/appstate/src/schemas_unlisted.rs`, `props::stale`), never in the generated file. `wire_enums.rs` binds only the catalog entries listed in the emitter's `WANTED`, because 88 of the 403 have a synthetic name and names repeat across modules; the variants themselves always come from the bundle. A candidate is found by its variant set but decided by its module: two enums agreeing on every value are not the same enum unless the module owns the wire format we parse. `targets.rs` binds the same way and covers `w:g2` only, the one namespace where a request's target is not implied by its namespace. - **`whatsapp.proto` is not the whole persisted schema.** It comes from whatspec and is regenerated wholesale, so fields we persist but upstream does not declare live in `LOCAL_FIELDS` in `waproto/build.rs`, spliced into the descriptor at build time, and whole retained messages in `LOCAL_BLOCKS` in the codegen's proto emitter. Never hand-edit the `.proto` or `.desc` to add one — the next sync would drop it. - **Blocking work** — `ureq`, heavy CPU — belongs in `tokio::task::spawn_blocking`; it shares a runtime with the read loop. - **let-chains**, never nested `if let`. Clippy's `collapsible_if` is denied in CI. diff --git a/tests/ab_prop_watch_coverage.rs b/tests/ab_prop_watch_coverage.rs new file mode 100644 index 000000000..ee5479d14 --- /dev/null +++ b/tests/ab_prop_watch_coverage.rs @@ -0,0 +1,175 @@ +//! Every A/B gate this crate reads must be watched. +//! +//! `AbPropsCache::apply_props` keeps only codes in its interest set, seeded from +//! `iq::props::WATCHED`. A prop read without being watched is therefore not a +//! prop that reads stale -- it is one whose server value was thrown away during +//! parsing, so the read returns the registry default now and on every future +//! connect. Nothing errors and nothing logs. +//! +//! That is worth a scan rather than a runtime check alone. The cache does +//! `debug_assert` on read, but only a test that actually exercises the gated +//! path can trip it, and a gate is usually added precisely because the path is +//! hard to reach. Three shipped gates were dead this way before anyone noticed: +//! `receipt_mode_bitmask_enabled`, `enable_spam_report_iq_with_privacy_token` +//! and `profile_scraping_privacy_token_in_about_usync`. +//! +//! The scan is textual on purpose. Resolving these paths properly would mean +//! running the compiler, and the failure being guarded is a name appearing in +//! one file and not another -- exactly what text sees. +//! +//! What it looks for is the *read*, not the name: the argument to a cache +//! accessor. Keying on the name alone is too loose -- the registry has +//! thousands of flags, and `GROUP_CALL_MAX_PARTICIPANTS` (a `usize` derived +//! from a flag) and the `PLACEHOLDER_MESSAGE_RESEND` proto enum variant both +//! spell one without reading anything. Keying on a `web::` prefix is too +//! tight, since a grouped `use ...::web::{FOO}` leaves the call site saying +//! only `FOO`. The accessor argument is the thing that actually consults the +//! cache, so it is neither. +//! +//! The one form that escapes is a prop bound to a differently-named local +//! constant first. Nothing does that today, and the runtime `debug_assert` +//! still covers it if anything ever does. + +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; + +use wacore::iq::abprops::{self, AbProp}; +use wacore::iq::props::WATCHED; + +fn manifest_path(relative: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join(relative) +} + +/// Every `.rs` file under `src/`, as (display path, contents). +fn sources() -> Vec<(String, String)> { + fn walk(dir: &Path, out: &mut Vec<(String, String)>) { + let entries = std::fs::read_dir(dir).unwrap_or_else(|e| panic!("read {dir:?}: {e}")); + for entry in entries { + let path = entry.expect("dir entry").path(); + if path.is_dir() { + walk(&path, out); + } else if path.extension().is_some_and(|e| e == "rs") { + let text = + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {path:?}: {e}")); + out.push((path.display().to_string(), text)); + } + } + } + let mut out = Vec::new(); + walk(&manifest_path("src"), &mut out); + assert!(!out.is_empty(), "no sources found under src/"); + out +} + +/// Every flag the registries declare, keyed by the constant that carries it. +/// The emitter names each constant after its flag, so `foo_enabled` is +/// `FOO_ENABLED`; `props::stale` is hand-written to the same convention. +fn registry() -> HashMap { + let mut out = HashMap::new(); + for module in abprops::ALL { + for prop in *module { + out.insert(prop.name.to_uppercase(), *prop); + } + } + // `stale` holds flags the bundle no longer builds, so they are in no + // generated registry but are still read and still have to be watched. + for prop in WATCHED { + out.insert(prop.name.to_uppercase(), *prop); + } + out +} + +/// The accessors that consult the cache for one flag. `watch`/`watch_many` +/// are not reads, and a prop passed to them is registered by that very call. +const ACCESSORS: &[&str] = &["is_enabled(", "get_int(", ".get("]; + +/// Flags passed to a cache accessor, as (identifier, file). +/// +/// Takes the argument's last `::` segment, so a qualified path and a bare name +/// reduce to the same constant. Anything that is not a screaming-snake registry +/// name -- a local, an expression -- is not a flag and is skipped. +fn referenced_props( + sources: &[(String, String)], + registry: &HashMap, +) -> Vec<(String, String)> { + let mut found = Vec::new(); + for (file, text) in sources { + for accessor in ACCESSORS { + let mut rest = text.as_str(); + while let Some(at) = rest.find(accessor) { + rest = &rest[at + accessor.len()..]; + let Some(end) = rest.find(')') else { continue }; + let arg = rest[..end].trim().trim_end_matches(',').trim(); + let ident = arg.rsplit("::").next().unwrap_or(arg).trim(); + if registry.contains_key(ident) { + found.push((ident.to_string(), file.clone())); + } + } + } + } + found +} + +#[test] +fn every_ab_prop_this_crate_reads_is_watched() { + let registry = registry(); + let watched: HashSet = WATCHED.iter().map(|p| p.name.to_uppercase()).collect(); + + let sources = sources(); + let referenced = referenced_props(&sources, ®istry); + assert!( + !referenced.is_empty(), + "the scan found no prop constants at all, so it is no longer guarding anything" + ); + + let mut unwatched: Vec = referenced + .iter() + .filter(|(ident, _)| !watched.contains(ident)) + .map(|(ident, file)| { + let code = registry[ident].code; + format!("{ident} (code {code}, read in {file})") + }) + .collect(); + unwatched.sort(); + unwatched.dedup(); + + assert!( + unwatched.is_empty(), + "these A/B props are read but absent from `WATCHED` in \ + wacore/src/iq/props.rs, so the server's value is discarded and each \ + read yields the registry default forever:\n {}", + unwatched.join("\n "), + ); +} + +/// The scan is only worth having if it sees a name that no `web::` prefix +/// introduces, which is the form the previous version missed. +#[test] +fn a_grouped_import_is_still_seen() { + let registry = registry(); + let watched_name = WATCHED[0].name.to_uppercase(); + let source = vec![( + "fake.rs".to_string(), + format!( + "use wacore::iq::abprops::web::{{{watched_name}}};\n\ + let on = cache.is_enabled({watched_name}).await;\n" + ), + )]; + // Named without reading, which the scan must not treat as a read. + let decoy = vec![( + "decoy.rs".to_string(), + "let n = GROUP_CALL_MAX_PARTICIPANTS;\n\ + let t = wa::message::PeerDataOperationRequestType::PLACEHOLDER_MESSAGE_RESEND;\n" + .to_string(), + )]; + assert!( + referenced_props(&decoy, ®istry).is_empty(), + "the scan counted a flag name that no accessor reads" + ); + + let found = referenced_props(&source, ®istry); + assert!( + found.iter().any(|(ident, _)| *ident == watched_name), + "the scan missed {watched_name} brought in by a grouped import" + ); +} diff --git a/tools/whatspec-codegen/src/emit/iq_targets.rs b/tools/whatspec-codegen/src/emit/iq_targets.rs new file mode 100644 index 000000000..83f0c3338 --- /dev/null +++ b/tools/whatspec-codegen/src/emit/iq_targets.rs @@ -0,0 +1,295 @@ +//! `wacore/src/iq/targets.rs`: who each request this repository sends is +//! addressed to, taken from the whatspec IQ index rather than from a reading of +//! the bundle. +//! +//! Addressing is worth deriving because getting it wrong is invisible. A `w:g2` +//! request sent to `g.us` when the server expects the group JID is a +//! well-formed stanza that is simply never answered, so the only symptom is a +//! caller that waits out its timeout -- there is no error to see and no nack to +//! log. Nothing in the shape of the stanza says which of the two it should be. +//! +//! What this emits is the *expectation*, not the address itself. The specs keep +//! building their own `to`; the generated constants give the tests something to +//! check them against that upstream owns. When WhatsApp moves a request from one +//! target to the other, the next sync flips the constant and the test fails, +//! instead of the change going unnoticed until someone reports a hang. + +use std::collections::BTreeSet; + +use anyhow::{Result, bail, ensure}; + +use crate::ir::{IqIr, IqStanza, IqTarget}; + +const HEADER: &str = "\ +//! How the official client addresses the requests this repository sends. +//! +//! Regenerate with `cargo run -p whatspec-codegen`; never edit by hand. To pin +//! another request, add it to `WANTED` in the codegen's IQ target emitter. + +/// Who a request is addressed to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IqTarget { + /// The main server, `s.whatsapp.net`. + MainServer, + /// The group server, `g.us`: it answers about groups in general rather than + /// about one group. + GroupServer, + /// The one group the request acts on. + GroupJid, +} + +"; + +/// One IQ builder this repository pins, keyed the way the index is: module plus +/// the exported function, because neither alone is unique. +pub struct Wanted { + pub module: &'static str, + pub function: &'static str, + /// The generated constant's name. Named after our spec rather than after + /// the upstream function, since the constant exists to be read next to the + /// spec it checks. + pub rust: &'static str, + /// What our spec is called, for the generated doc line. A reader landing on + /// the constant should not have to guess which spec it governs. + pub spec: &'static str, +} + +/// The requests whose addressing is pinned. +/// +/// `w:g2` only, and deliberately: it is the only namespace where the +/// distinction exists. Outside it every entry in the index resolves to +/// `s.whatsapp.net`, except four `newsletter` builders whatspec cannot resolve +/// at all -- so a constant elsewhere would either restate what the namespace +/// already guarantees or pin a target nobody read. +pub const WANTED: &[Wanted] = &[ + Wanted { + module: "WAWebGroupExitJob", + function: "leaveGroup", + rust: "LEAVE_GROUP", + spec: "LeaveGroupIq", + }, + Wanted { + module: "WASmaxOutGroupsGetParticipatingGroupsRequest", + function: "makeGetParticipatingGroupsRequestParticipatingParticipants", + rust: "GROUP_PARTICIPATING", + spec: "GroupParticipatingIq", + }, + Wanted { + module: "WASmaxOutGroupsBatchGetGroupInfoRequest", + function: "makeBatchGetGroupInfoRequestQueryGroup", + rust: "BATCH_GET_GROUP_INFO", + spec: "BatchGetGroupInfoIq", + }, + Wanted { + module: "WASmaxOutGroupsGetInviteGroupInfoRequest", + function: "makeGetInviteGroupInfoRequest", + rust: "GET_GROUP_INVITE_INFO", + spec: "GetGroupInviteInfoIq", + }, + Wanted { + module: "WASmaxOutGroupsGetGroupInfoRequest", + function: "makeGetGroupInfoRequestQueryAddRequest", + rust: "GROUP_QUERY", + spec: "GroupQueryIq", + }, + Wanted { + module: "WASmaxOutGroupsSetSubjectRequest", + function: "makeSetSubjectRequest", + rust: "SET_GROUP_SUBJECT", + spec: "SetGroupSubjectIq", + }, + Wanted { + module: "WASmaxOutGroupsSetDescriptionRequest", + function: "makeSetDescriptionRequestDescriptionBody", + rust: "SET_GROUP_DESCRIPTION", + spec: "SetGroupDescriptionIq", + }, + Wanted { + module: "WASmaxOutGroupsSetPropertyRequest", + function: "makeSetPropertyRequestLocked", + rust: "SET_GROUP_LOCKED", + spec: "SetGroupLockedIq", + }, + Wanted { + module: "WASmaxOutGroupsReportMessagesRequest", + function: "makeReportMessagesRequest", + rust: "REPORT_GROUP_MESSAGES", + spec: "ReportGroupMessagesIq", + }, + Wanted { + module: "WASmaxOutGroupsGetReportedMessagesRequest", + function: "makeGetReportedMessagesRequest", + rust: "GET_REPORTED_GROUP_MESSAGES", + spec: "GetReportedGroupMessagesIq", + }, + Wanted { + module: "WASmaxOutGroupsGetMembershipApprovalRequestsRequest", + function: "makeGetMembershipApprovalRequestsRequest", + rust: "GET_MEMBERSHIP_REQUESTS", + spec: "GetMembershipRequestsIq", + }, + Wanted { + module: "WASmaxOutGroupsMembershipRequestsActionRequest", + function: "makeMembershipRequestsActionRequestMembershipRequestsActionApproveParticipant", + rust: "MEMBERSHIP_REQUEST_ACTION", + spec: "MembershipRequestActionIq", + }, +]; + +pub fn generate(ir: &IqIr) -> Result { + let mut out = super::header("IQ addressing", &ir.wa_version); + out.push_str(HEADER); + + let mut used = BTreeSet::new(); + for wanted in WANTED { + ensure!( + used.insert(wanted.rust), + "two WANTED entries both emit {}", + wanted.rust + ); + let stanza = lookup(ir, wanted)?; + let target = match stanza.target { + IqTarget::MainServer => "MainServer", + IqTarget::GroupServer => "GroupServer", + IqTarget::GroupJid => "GroupJid", + // Pinning an unresolved target would assert that we address the + // request the way whatspec failed to read, which is not a claim + // anything here can make. + IqTarget::Unknown => bail!( + "{}::{} resolves to no target, so its addressing cannot be pinned", + wanted.module, + wanted.function + ), + }; + out.push_str(&format!( + "/// Addressing of `{}`, from `{}` in `{}` (`{}` in `{}`).\npub const {}: IqTarget = IqTarget::{target};\n\n", + wanted.spec, + wanted.function, + stanza.module_name, + stanza.iq_type, + stanza.namespace, + wanted.rust, + )); + } + Ok(out) +} + +/// The one entry matching `(module, function)`. +/// +/// Ambiguity is fatal rather than first-wins, and here that is not a +/// hypothetical: `resetGroupInviteCode` appears twice in `WAWebGroupInviteJob`, +/// once resolving to `group_jid` and once to `g.us`. They are different calls +/// that share a name, so picking either by position would pin a coin flip. +fn lookup<'a>(ir: &'a IqIr, wanted: &Wanted) -> Result<&'a IqStanza> { + let matches: Vec<&IqStanza> = ir + .stanzas + .iter() + .filter(|s| s.module_name == wanted.module && s.exported_function == wanted.function) + .collect(); + match matches.as_slice() { + [one] => Ok(one), + [] => bail!( + "the IQ index has no {}::{}; it was renamed or dropped upstream", + wanted.module, + wanted.function + ), + many => bail!( + "the IQ index has {} entries for {}::{}, which resolve to {:?}; \ + a name shared by several calls cannot pin one of them", + many.len(), + wanted.module, + wanted.function, + many.iter().map(|s| s.target).collect::>(), + ), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn stanza(module: &str, function: &str, target: IqTarget) -> IqStanza { + IqStanza { + module_name: module.to_string(), + namespace: "w:g2".to_string(), + iq_type: "get".to_string(), + target, + exported_function: function.to_string(), + } + } + + /// An index covering every `WANTED` entry, since `generate` resolves the + /// whole table. Each case below then perturbs the one entry it is about, so + /// the failure it asserts is the one it caused. + fn fixture() -> IqIr { + IqIr { + wa_version: "2.0.0".to_string(), + stanzas: WANTED + .iter() + .map(|w| stanza(w.module, w.function, IqTarget::GroupJid)) + .collect(), + } + } + + /// `leaveGroup` leads `WANTED`, so a bail on it stops the run before any + /// later entry is reached. + const FIRST: &str = "leaveGroup"; + + fn first_entry() -> &'static Wanted { + WANTED + .iter() + .find(|w| w.function == FIRST) + .expect("WANTED leads with the entry these tests perturb") + } + + #[test] + fn the_target_comes_from_the_index_not_the_table() { + let entry = first_entry(); + for (target, rendered) in [ + (IqTarget::GroupServer, "IqTarget::GroupServer"), + (IqTarget::GroupJid, "IqTarget::GroupJid"), + ] { + let mut ir = fixture(); + ir.stanzas[0] = stanza(entry.module, entry.function, target); + let out = generate(&ir).expect("emit"); + assert!( + out.contains(&format!("pub const LEAVE_GROUP: IqTarget = {rendered};")), + "{out}" + ); + } + } + + /// The failure this exists for: two calls sharing a name, resolving + /// differently. Binding either would pin whichever the index happened to + /// list first. + #[test] + fn a_name_shared_by_two_targets_stops_the_generator() { + let entry = first_entry(); + let mut ir = fixture(); + ir.stanzas + .push(stanza(entry.module, entry.function, IqTarget::GroupServer)); + let err = generate(&ir).expect_err("ambiguous"); + assert!(err.to_string().contains("cannot pin one of them"), "{err}"); + } + + #[test] + fn an_unresolved_target_is_refused() { + let entry = first_entry(); + let mut ir = fixture(); + ir.stanzas[0] = stanza(entry.module, entry.function, IqTarget::Unknown); + let err = generate(&ir).expect_err("unknown target"); + assert!(err.to_string().contains("resolves to no target"), "{err}"); + } + + #[test] + fn a_dropped_entry_is_refused() { + let mut ir = fixture(); + ir.stanzas.remove(0); + let err = generate(&ir).expect_err("the entry is gone"); + assert!( + err.to_string() + .contains("has no WAWebGroupExitJob::leaveGroup"), + "{err}" + ); + } +} diff --git a/tools/whatspec-codegen/src/emit/mod.rs b/tools/whatspec-codegen/src/emit/mod.rs index b375d31e1..60ec7346c 100644 --- a/tools/whatspec-codegen/src/emit/mod.rs +++ b/tools/whatspec-codegen/src/emit/mod.rs @@ -6,6 +6,7 @@ pub mod abprops; pub mod appstate; pub mod enums; +pub mod iq_targets; pub mod mex; pub mod proto; pub mod tokens; diff --git a/tools/whatspec-codegen/src/ir.rs b/tools/whatspec-codegen/src/ir.rs index 0b7d47715..97ea2bccd 100644 --- a/tools/whatspec-codegen/src/ir.rs +++ b/tools/whatspec-codegen/src/ir.rs @@ -115,6 +115,48 @@ pub struct EnumsIr { pub enums: Vec, } +/// Who a request is addressed to, as whatspec resolves it from the builder. +/// +/// The distinction only became visible in schema 4: before it, every `w:g2` +/// stanza reported its namespace's base target, so a request sent to the wrong +/// one of the two was indistinguishable from a correct one. The symptom is a +/// server that never answers and a caller that waits out its timeout. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +pub enum IqTarget { + /// The main server, `s.whatsapp.net`. + #[serde(rename = "s.whatsapp.net")] + MainServer, + /// The group server, `g.us`, which answers about groups in general. + #[serde(rename = "g.us")] + GroupServer, + /// The one group the request acts on. + #[serde(rename = "group_jid")] + GroupJid, + /// whatspec could not resolve the target from the builder. + #[serde(rename = "unknown")] + Unknown, +} + +/// One outgoing IQ builder found in the bundle. A name is unique only together +/// with its module, and not always then: `resetGroupInviteCode` appears twice +/// in `WAWebGroupInviteJob` with different targets. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IqStanza { + pub module_name: String, + pub namespace: String, + pub iq_type: String, + pub target: IqTarget, + pub exported_function: String, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IqIr { + pub wa_version: String, + pub stanzas: Vec, +} + /// One element of an action's mutation index; `type` discriminates the shape. #[derive(Debug, Clone, Deserialize)] #[serde(tag = "type")] diff --git a/tools/whatspec-codegen/src/main.rs b/tools/whatspec-codegen/src/main.rs index 5a4facca8..e69d87d46 100644 --- a/tools/whatspec-codegen/src/main.rs +++ b/tools/whatspec-codegen/src/main.rs @@ -252,6 +252,8 @@ fn build(ir: &Ir, wa_version: &str) -> Result> { .context("parsing the appstate IR")?; let enums: ir::EnumsIr = serde_json::from_str(&ir.text("enums/index.json")?).context("parsing the enums IR")?; + let iq: ir::IqIr = + serde_json::from_str(&ir.text("iq/index.json")?).context("parsing the IQ IR")?; let mex: ir::MexIr = serde_json::from_str(&ir.text("mex/index.json")?).context("parsing the mex IR")?; let tokens: ir::TokensIr = @@ -273,6 +275,11 @@ fn build(ir: &Ir, wa_version: &str) -> Result> { content: emit::enums::generate(&enums)?, rust: true, }, + Artifact { + path: "wacore/src/iq/targets.rs", + content: emit::iq_targets::generate(&iq)?, + rust: true, + }, Artifact { path: "wacore/src/iq/mex_operations.rs", content: emit::mex::generate(&mex), diff --git a/tools/whatspec-codegen/src/source.rs b/tools/whatspec-codegen/src/source.rs index 999babe7a..38f6ff70f 100644 --- a/tools/whatspec-codegen/src/source.rs +++ b/tools/whatspec-codegen/src/source.rs @@ -21,6 +21,7 @@ pub const IR_FILES: &[&str] = &[ "abprops/index.json", "appstate/index.json", "enums/index.json", + "iq/index.json", "mex/index.json", "tokens/index.json", "proto/WAProto.proto", diff --git a/tools/whatspec-codegen/tests/committed_artifacts.rs b/tools/whatspec-codegen/tests/committed_artifacts.rs index d070aa7b8..e2c8b34a4 100644 --- a/tools/whatspec-codegen/tests/committed_artifacts.rs +++ b/tools/whatspec-codegen/tests/committed_artifacts.rs @@ -43,12 +43,19 @@ fn locked_version() -> String { #[test] fn every_generated_rust_file_stamps_the_locked_version() { let want = locked_version(); + // Every Rust artifact `build()` in the codegen emits. The list is repeated + // rather than shared because that lives in a binary crate an integration + // test cannot import, so a new artifact has to be added in both places -- + // this check is the offline one, and the only thing that notices an + // artifact left over from another build when `--check` cannot reach the + // network to fetch the IR. for rel in [ "wacore/src/version/generated.rs", "wacore/src/iq/abprops.rs", "wacore/src/iq/mex_operations.rs", "wacore/appstate/src/schemas.rs", "wacore/src/types/wire_enums.rs", + "wacore/src/iq/targets.rs", ] { let text = read(rel); let got = stamped_in_header(&text) diff --git a/tools/whatspec-codegen/whatspec.lock.json b/tools/whatspec-codegen/whatspec.lock.json index b5bd0f0aa..eb1003d72 100644 --- a/tools/whatspec-codegen/whatspec.lock.json +++ b/tools/whatspec-codegen/whatspec.lock.json @@ -6,6 +6,7 @@ "abprops/index.json": "sha256:21f6f4a28fef514bf2f4e9006839c946d9a6b9e2df376c7170bd2e8520ae9abd", "appstate/index.json": "sha256:655e50ecd5f3e592a9c5e5a32a98cfd40d9aa2e6a4b142f3f9203750116d6092", "enums/index.json": "sha256:e0f9d1d71ab254f14a7ac2725db50e6f820a408900feb422bca481292e46c104", + "iq/index.json": "sha256:e8e68c739a74e7551393e786eb9e63ae413209aa46330549135b7912bfa2431c", "manifest.json": "sha256:a944729a9d7741c7a6b27ee952853f787d9ba6380c324eb9cdb8d9c76eef8601", "mex/index.json": "sha256:7d14b1daf2169e9647adb3f18d31482bb7e0271344774ed7ae0e937392d8dbee", "proto/WAProto.proto": "sha256:021d53059e7b35d8553c97c09567da8d6aba7278c7f2510242eab686bff3647a", diff --git a/wacore/src/iq/groups.rs b/wacore/src/iq/groups.rs index 25913241e..4575c9b58 100644 --- a/wacore/src/iq/groups.rs +++ b/wacore/src/iq/groups.rs @@ -3690,44 +3690,102 @@ mod tests { /// read off the IR by hand, since the IR is not available at test time. #[test] fn group_requests_are_addressed_the_way_the_ir_resolves_them() { + use crate::iq::targets::{self, IqTarget}; + let group: Jid = "120363000000000001@g.us".parse().unwrap(); let server = Jid::new("", Server::Group); - // `g.us`: the group server answers these, not any one group. - assert_eq!(LeaveGroupIq::new(&group).build_iq().to, server); - assert_eq!(GroupParticipatingIq::new().build_iq().to, server); - assert_eq!( + // The expectation comes from the generated constants, so a request + // upstream moves between the two targets flips the constant on the next + // sync and fails here, rather than turning into a silent timeout. + let expect = |actual: Jid, want: IqTarget, spec: &str| { + let addressed = match want { + IqTarget::GroupServer => server.clone(), + IqTarget::GroupJid => group.clone(), + IqTarget::MainServer => Jid::new("", Server::Pn), + }; + assert_eq!(actual, addressed, "{spec} is addressed to the wrong target"); + }; + + expect( + LeaveGroupIq::new(&group).build_iq().to, + targets::LEAVE_GROUP, + "LeaveGroupIq", + ); + expect( + GroupParticipatingIq::new().build_iq().to, + targets::GROUP_PARTICIPATING, + "GroupParticipatingIq", + ); + expect( BatchGetGroupInfoIq::new(std::slice::from_ref(&group)) .build_iq() .to, - server + targets::BATCH_GET_GROUP_INFO, + "BatchGetGroupInfoIq", ); - assert_eq!(GetGroupInviteInfoIq::new("ABC123").build_iq().to, server); - - // `group_jid`: addressed to the one group they act on. - assert_eq!( + expect( + GetGroupInviteInfoIq::new("ABC123").build_iq().to, + targets::GET_GROUP_INVITE_INFO, + "GetGroupInviteInfoIq", + ); + expect( + GroupQueryIq::new(&group).build_iq().to, + targets::GROUP_QUERY, + "GroupQueryIq", + ); + expect( SetGroupSubjectIq::new(&group, GroupSubject::new("x").unwrap()) .build_iq() .to, - group + targets::SET_GROUP_SUBJECT, + "SetGroupSubjectIq", ); - assert_eq!( - GetGroupInviteLinkIq::new(&group, false).build_iq().to, - group + expect( + SetGroupDescriptionIq::new(&group, None, None).build_iq().to, + targets::SET_GROUP_DESCRIPTION, + "SetGroupDescriptionIq", ); - assert_eq!( - GetGroupInviteLinkIq::new(&group, true).build_iq().to, - group, - "the reset overload the IR resolves to group_jid is the one with an \ - empty ; the g.us overload carries a code and is not this" + expect( + SetGroupLockedIq::lock(&group).build_iq().to, + targets::SET_GROUP_LOCKED, + "SetGroupLockedIq", ); - assert_eq!( + expect( ReportGroupMessagesIq::new(&group, &["M1".to_string()]) .build_iq() .to, + targets::REPORT_GROUP_MESSAGES, + "ReportGroupMessagesIq", + ); + expect( + GetReportedGroupMessagesIq::new(&group).build_iq().to, + targets::GET_REPORTED_GROUP_MESSAGES, + "GetReportedGroupMessagesIq", + ); + expect( + GetMembershipRequestsIq::new(&group).build_iq().to, + targets::GET_MEMBERSHIP_REQUESTS, + "GetMembershipRequestsIq", + ); + expect( + MembershipRequestActionIq::approve(&group, &[]) + .build_iq() + .to, + targets::MEMBERSHIP_REQUEST_ACTION, + "MembershipRequestActionIq", + ); + + // Still hand-asserted: `resetGroupInviteCode` has two entries in + // `WAWebGroupInviteJob` resolving to different targets, so the emitter + // refuses to pin either. Ours is the `group_jid` overload, the one with + // an empty ``; the `g.us` overload carries a code and is a call + // this repository does not make. + assert_eq!( + GetGroupInviteLinkIq::new(&group, false).build_iq().to, group ); - assert_eq!(GetReportedGroupMessagesIq::new(&group).build_iq().to, group); + assert_eq!(GetGroupInviteLinkIq::new(&group, true).build_iq().to, group); } #[test] diff --git a/wacore/src/iq/mod.rs b/wacore/src/iq/mod.rs index e2596b0c8..cb8a51ed1 100644 --- a/wacore/src/iq/mod.rs +++ b/wacore/src/iq/mod.rs @@ -20,5 +20,6 @@ pub mod profile; pub mod props; pub mod spam_report; pub mod spec; +pub mod targets; pub mod tctoken; pub mod usync; diff --git a/wacore/src/iq/props.rs b/wacore/src/iq/props.rs index 4efd5ec28..718febe61 100644 --- a/wacore/src/iq/props.rs +++ b/wacore/src/iq/props.rs @@ -78,6 +78,8 @@ pub const WATCHED: &[abprops::AbProp] = &[ abprops::web::WA_NCT_TOKEN_SEND_ENABLED, abprops::web::RECEIPT_MODE_BITMASK_ENABLED, abprops::web::WEB_SEND_HID_FAILED_DECRYPT_IN_RECEIPTS_ENABLED, + abprops::web::ENABLE_SPAM_REPORT_IQ_WITH_PRIVACY_TOKEN, + abprops::web::PROFILE_SCRAPING_PRIVACY_TOKEN_IN_ABOUT_USYNC, stale::PRIVACY_TOKEN_ONLY_CHECK_LID, stale::PROFILE_PIC_PRIVACY_TOKEN, ]; diff --git a/wacore/src/iq/targets.rs b/wacore/src/iq/targets.rs new file mode 100644 index 000000000..8d0c81ec5 --- /dev/null +++ b/wacore/src/iq/targets.rs @@ -0,0 +1,54 @@ +//! Auto-generated IQ addressing (WhatsApp 2.3000.1044659339). DO NOT EDIT. +//! +//! How the official client addresses the requests this repository sends. +//! +//! Regenerate with `cargo run -p whatspec-codegen`; never edit by hand. To pin +//! another request, add it to `WANTED` in the codegen's IQ target emitter. + +/// Who a request is addressed to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IqTarget { + /// The main server, `s.whatsapp.net`. + MainServer, + /// The group server, `g.us`: it answers about groups in general rather than + /// about one group. + GroupServer, + /// The one group the request acts on. + GroupJid, +} + +/// Addressing of `LeaveGroupIq`, from `leaveGroup` in `WAWebGroupExitJob` (`set` in `w:g2`). +pub const LEAVE_GROUP: IqTarget = IqTarget::GroupServer; + +/// Addressing of `GroupParticipatingIq`, from `makeGetParticipatingGroupsRequestParticipatingParticipants` in `WASmaxOutGroupsGetParticipatingGroupsRequest` (`get` in `w:g2`). +pub const GROUP_PARTICIPATING: IqTarget = IqTarget::GroupServer; + +/// Addressing of `BatchGetGroupInfoIq`, from `makeBatchGetGroupInfoRequestQueryGroup` in `WASmaxOutGroupsBatchGetGroupInfoRequest` (`get` in `w:g2`). +pub const BATCH_GET_GROUP_INFO: IqTarget = IqTarget::GroupServer; + +/// Addressing of `GetGroupInviteInfoIq`, from `makeGetInviteGroupInfoRequest` in `WASmaxOutGroupsGetInviteGroupInfoRequest` (`get` in `w:g2`). +pub const GET_GROUP_INVITE_INFO: IqTarget = IqTarget::GroupServer; + +/// Addressing of `GroupQueryIq`, from `makeGetGroupInfoRequestQueryAddRequest` in `WASmaxOutGroupsGetGroupInfoRequest` (`get` in `w:g2`). +pub const GROUP_QUERY: IqTarget = IqTarget::GroupJid; + +/// Addressing of `SetGroupSubjectIq`, from `makeSetSubjectRequest` in `WASmaxOutGroupsSetSubjectRequest` (`set` in `w:g2`). +pub const SET_GROUP_SUBJECT: IqTarget = IqTarget::GroupJid; + +/// Addressing of `SetGroupDescriptionIq`, from `makeSetDescriptionRequestDescriptionBody` in `WASmaxOutGroupsSetDescriptionRequest` (`set` in `w:g2`). +pub const SET_GROUP_DESCRIPTION: IqTarget = IqTarget::GroupJid; + +/// Addressing of `SetGroupLockedIq`, from `makeSetPropertyRequestLocked` in `WASmaxOutGroupsSetPropertyRequest` (`set` in `w:g2`). +pub const SET_GROUP_LOCKED: IqTarget = IqTarget::GroupJid; + +/// Addressing of `ReportGroupMessagesIq`, from `makeReportMessagesRequest` in `WASmaxOutGroupsReportMessagesRequest` (`set` in `w:g2`). +pub const REPORT_GROUP_MESSAGES: IqTarget = IqTarget::GroupJid; + +/// Addressing of `GetReportedGroupMessagesIq`, from `makeGetReportedMessagesRequest` in `WASmaxOutGroupsGetReportedMessagesRequest` (`get` in `w:g2`). +pub const GET_REPORTED_GROUP_MESSAGES: IqTarget = IqTarget::GroupJid; + +/// Addressing of `GetMembershipRequestsIq`, from `makeGetMembershipApprovalRequestsRequest` in `WASmaxOutGroupsGetMembershipApprovalRequestsRequest` (`get` in `w:g2`). +pub const GET_MEMBERSHIP_REQUESTS: IqTarget = IqTarget::GroupJid; + +/// Addressing of `MembershipRequestActionIq`, from `makeMembershipRequestsActionRequestMembershipRequestsActionApproveParticipant` in `WASmaxOutGroupsMembershipRequestsActionRequest` (`set` in `w:g2`). +pub const MEMBERSHIP_REQUEST_ACTION: IqTarget = IqTarget::GroupJid; diff --git a/wacore/src/store/ab_props.rs b/wacore/src/store/ab_props.rs index c54ac5a6b..1ca51460c 100644 --- a/wacore/src/store/ab_props.rs +++ b/wacore/src/store/ab_props.rs @@ -77,6 +77,34 @@ impl AbPropsCache { } } + /// Panics in debug builds when `prop` is read without being watched. + /// + /// `apply_props` discards anything outside the interest set, so such a read + /// can never see the server's value: it returns the registry default now and + /// forever, with no error and no log line. That has silently disabled a + /// shipped feature gate more than once, and neither the type system nor a + /// test of the reading code can catch it, because the reading code is + /// correct -- what is missing sits in another file. + /// + /// Guards only the accessors that substitute a default. [`get`](Self::get) + /// returns `Option`, so a caller there is told the value is absent rather + /// than handed a plausible one. + #[cfg(debug_assertions)] + async fn debug_assert_watched(&self, prop: AbProp) { + assert!( + self.interest.read().await.contains(&prop.code), + "AB prop {:?} (code {}) was read but is not watched, so the server's \ + value is discarded and this read always yields the registry default. \ + Add it to `WATCHED` in wacore/src/iq/props.rs, or call `watch()` \ + before the first fetch.", + prop.name, + prop.code, + ); + } + + #[cfg(not(debug_assertions))] + async fn debug_assert_watched(&self, _prop: AbProp) {} + pub async fn get(&self, prop: AbProp) -> Option { self.props.read().await.get(&prop.code).cloned() } @@ -85,6 +113,7 @@ impl AbPropsCache { /// falling back to the flag's registry default when the server didn't send /// it. The registry is the single source of truth for the default. pub async fn is_enabled(&self, prop: AbProp) -> bool { + self.debug_assert_watched(prop).await; match self.props.read().await.get(&prop.code) { Some(value) => { value == "1" @@ -98,6 +127,7 @@ impl AbPropsCache { /// The cached int value, falling back to the flag's registry default when /// the server didn't send it (or it's not an int flag). pub async fn get_int(&self, prop: AbProp) -> i64 { + self.debug_assert_watched(prop).await; let fallback = match prop.default { AbDefault::Int(n) => n, _ => 0, @@ -152,8 +182,11 @@ mod tests { #[tokio::test] async fn is_enabled_checks_truthy_values() { let cache = AbPropsCache::new(); + // 999 is watched but never sent, which is the "absent" case asserted + // below. Watching it is what distinguishes absent-from-the-response + // from never-retained-at-all. cache - .watch_many(&[flag(1), flag(2), flag(3), flag(4), flag(5)]) + .watch_many(&[flag(1), flag(2), flag(3), flag(4), flag(5), flag(999)]) .await; let props = vec![ @@ -249,6 +282,15 @@ mod tests { assert_eq!(cache.get(flag(99999)).await, None); } + /// The guard has to fire on the read, not on the fetch: at fetch time a + /// missing prop is indistinguishable from one the server chose not to send. + #[tokio::test] + #[should_panic(expected = "is not watched")] + #[cfg(debug_assertions)] + async fn reading_an_unwatched_prop_trips_the_guard() { + AbPropsCache::new().is_enabled(flag(4242)).await; + } + /// Verify seeded flag is only set AFTER all props are inserted (not before). #[tokio::test] async fn seeded_set_after_inserts() {