ROAD-354: expose contact labels, phone, and stable message IDs - #1
Conversation
The C binary find_all_keys_macos writes JSON in the schema
{"db.path": {"enc_key": "..."}} (without a salt field), but the
Python wrapper at scanner_macos.py was filtering on both enc_key and salt,
so every entry was rejected and key_map ended up empty. init.py then
echoed 'Extracted 0 keys' even though all_keys.json was correctly written
and downstream queries worked.
Build key_map keyed by rel_path instead, matching the C binary's actual
output and how core/key_utils.get_key_info already does lookups.
Fixes #2
- Decode contact.extra_buffer protobuf blob: field 30 (label IDs, joined to contact_label for names) and field 14->2->1 (mobile number) via one shared varint/tag parser. Surfaced as labels/phone in contacts list JSON and contacts --detail (labels, label_ids, phone). - Add server_id to the message query and return structured entries from collect_chat_history; history JSON now emits local_id/server_id/timestamp/time/sender/text per message, enabling wechat_message_id-based dedup. Text output and export unchanged. - Cherry-pick huohuoer#4: fix key_map indexing so init no longer misreports "提取到 0 个密钥" after a successful scan. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| messages = [ | ||
| {k: e[k] for k in ('local_id', 'server_id', 'timestamp', 'time', 'sender', 'text')} | ||
| for e in entries | ||
| ] |
There was a problem hiding this comment.
🔴 Large message IDs lose precision
When server_id exceeds JavaScript’s safe integer range, JSON consumers round the emitted number. Distinct WeChat messages can then share a deduplication key.
Learn more
WeChat server IDs are 64-bit integers. JSON permits numbers of this size, but common consumers parse them as IEEE-754 doubles, which preserve integers exactly only through 9,007,199,254,740,991. The bridge can therefore mutate the identifier before storing or comparing it.
Example: The illustrative IDs 7000000000000000000 and 7000000000000000001 both become 7000000000000000000 when parsed as JavaScript numbers. Two different messages can then appear to have one identifier.
Recommended fix: Serialize server_id as a decimal string in the public JSON response. Preserve None as JSON null if the database lacks a server-assigned ID.
| messages = [ | |
| {k: e[k] for k in ('local_id', 'server_id', 'timestamp', 'time', 'sender', 'text')} | |
| for e in entries | |
| ] | |
| messages = [] | |
| for e in entries: | |
| message = {k: e[k] for k in ('local_id', 'timestamp', 'time', 'sender', 'text')} | |
| message['server_id'] = str(e['server_id']) if e['server_id'] is not None else None | |
| messages.append(message) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| for r in conn.execute( | ||
| "SELECT username, nick_name, remark, extra_buffer FROM contact" | ||
| ).fetchall(): |
There was a problem hiding this comment.
🔍 Contact schema compatibility needs verification
_load_contacts_from now requires extra_buffer to load any contacts. Databases lacking that column yield an empty contact list instead of reduced metadata.
Was this helpful? React with 👍 or 👎 to provide feedback.
| SELECT local_id, server_id, local_type, create_time, real_sender_id, message_content, | ||
| WCDB_CT_message_content |
There was a problem hiding this comment.
🔍 Message schema compatibility needs verification
_query_messages now requires server_id for history, export, and search. A supported schema lacking it makes every affected table unreadable.
Was this helpful? React with 👍 or 👎 to provide feedback.
- db_cache: validate decrypted output with PRAGMA integrity_check and retry before caching — a torn read (WeChat checkpointing mid-decrypt) was previously cached by mtime and served forever, breaking all contact/message queries until manual cache removal. - init: reuse existing config db_dir before auto-detect, so init --force cannot silently switch accounts when multiple xwechat_files db_storage dirs exist. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Captures the read-only security posture, init/re-sign setup, layout, data-model gotchas (extra_buffer fields, Msg_ table naming, live-DB decrypt validation), and verification commands learned implementing ROAD-354. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| if rel_key in self._cache: | ||
| c_db_mt, c_wal_mt, c_path = self._cache[rel_key] | ||
| if c_db_mt == db_mtime and c_wal_mt == wal_mtime and os.path.exists(c_path): | ||
| if c_db_mt == db_mtime and c_wal_mt == wal_mtime and _has_sqlite_header(c_path): |
There was a problem hiding this comment.
🟡 Corrupt cached databases pass validation
When corruption leaves the header intact, _has_sqlite_header accepts the cached database without checking its remaining pages. DBCache.get then returns unreadable or inconsistent data instead of rebuilding the cache.
Learn more
The decrypt path runs PRAGMA integrity_check, but cache hits only compare the first 16 bytes. Every decrypted first page receives the SQLite header from decrypt_page, so that header does not prove that later pages are intact. A partial file, a torn prior decrypt, or corruption after page one therefore remains reusable indefinitely while source mtimes stay unchanged.
Example: A cached 20-page database is truncated after page 5. Its first 16 bytes still equal SQLITE_HDR, so the next query returns it and SQLite fails when reading a table stored on page 12. The cache was expected to be rejected and decrypted again.
Recommended fix: Use _is_valid_sqlite(c_path) for cache hits so the same full integrity check gates both newly decrypted and reused files.
| if c_db_mt == db_mtime and c_wal_mt == wal_mtime and _has_sqlite_header(c_path): | |
| if c_db_mt == db_mtime and c_wal_mt == wal_mtime and _is_valid_sqlite(c_path): |
Was this helpful? React with 👍 or 👎 to provide feedback.
| existing = json.load(f).get("db_dir") | ||
| if existing and os.path.isdir(existing): | ||
| db_dir = existing |
There was a problem hiding this comment.
🟡 Relative configured paths trigger account switching
With a relative configured db_dir, os.path.isdir resolves it against the process directory and can reject a valid account. load_config resolves that value from the configuration directory. Initialization then auto-detects another account and overwrites the configured keys.
Learn more
Configuration paths can be relative to the directory containing config.json; load_config implements that contract. Forced initialization reads the raw value instead. Its existence check therefore depends on the shell's current directory, and a failed check enters account auto-detection despite an existing configured account.
Example: ~/.wechat-cli/config.json contains {"db_dir": "../wechat/account-a/db_storage"}. Running wechat-cli init --force from /tmp tests /tmp/../wechat/account-a/db_storage, rejects it, and can select account B. The command then replaces all_keys.json and config.json with account B's data.
Recommended fix: Resolve a relative saved db_dir against os.path.dirname(CONFIG_FILE) before checking it, matching load_config. Preserve the resolved configured path whenever it names an existing directory.
Was this helpful? React with 👍 or 👎 to provide feedback.
| with open(CONFIG_FILE, encoding="utf-8") as f: | ||
| existing = json.load(f).get("db_dir") |
There was a problem hiding this comment.
🟡 Non-object config aborts initialization
When config.json contains valid non-object JSON, .get raises before init --force can recover or auto-detect an account.
Learn more
The exception handler covers invalid JSON and file errors, but JSON arrays, strings, numbers, and null decode successfully. None provides the mapping interface used by .get, so the command exits with an uncaught AttributeError before key extraction.
Example: If an interrupted manual repair leaves config.json containing [], wechat-cli init --force crashes at .get("db_dir"). It was expected to ignore the unusable saved value and run account detection.
Recommended fix: Decode into a temporary value and read db_dir only when that value is a dictionary. Treat every other JSON type as an empty configuration.
Was this helpful? React with 👍 or 👎 to provide feedback.
26 tests covering the extra_buffer protobuf decoder, contact loading/detail, history message IDs, db_cache torn-read poisoning, and init db_dir preservation. Regression-verified: the init and cache-poisoning tests fail against pre-fix code. Runs without WeChat or real data — protects future changes from reintroducing the two bugs found during ROAD-354 live testing. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Designated place for personal-data exports on disk — dir contents ignored except .gitkeep (JSON was already covered by the *.json rule; this also covers md/txt exports and makes the convention explicit). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Parser previously stopped at wt=1/5 fields — a fixed-width field appearing before field 30/14 would silently drop labels/phone. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
jacktator
left a comment
jacktator
left a comment
There was a problem hiding this comment.
Review — approve with comments
Reviewed all commits on this branch against the ROAD-354 acceptance criteria. Implementation is correct, validated against live data, and the hermetic test suite regression-covers the two bugs found during testing. One defect was found and fixed during this review (77c07e4).
Fixed during review
_parse_protobuf_fieldsbailed on fixed-width wire types — awt=1(fixed64) orwt=5(fixed32) field appearing before field 30/14 inextra_bufferwould have silently dropped labels/phone. Now skipped per spec; test added.
Non-blocking comments
historyJSON is a schema change —messageswentlist[str]→list[dict]withlocal_id/server_id/timestamp/sender/text. Intended per the ticket, but flag for any downstream consumer reading the old string format (text format unchanged).server_idcan be 0 for local-only messages — consumers doingwechat_message_iddedup should fall back to(local_id, table)whenserver_id == 0. Worth a note in the bridge ticket.PRAGMA integrity_checkcost — every decrypt now pays a full-DB scan. WAL mtimes churn constantly while WeChat runs, so largemessage_*.dbdecrypts (60–80MB) add ~seconds per CLI invocation. Correct trade-off (a poisoned cache was worse), butquick_checkor schema-page sampling is a fallback if it ever feels slow.- Residual edge in cache-hit path — hits only header-check the file, so a valid-header/torn-interior file written before this fix could still be served until its source mtime changes. New files can only enter cache post-
integrity_check, so this is a shrinking edge; per-hit integrity checks would be too expensive. _decode_extra_labelstakes the first field-30 occurrence only — fine for observed data (single occurrence per blob), but a repeated field-30 would be ignored._build_search_entryunpacksserver_idbut discards it — consistent with the shared row shape; search results could expose IDs later if the bridge wants them.- PR scope — the feature + two infra fixes (db_cache poisoning, init
db_dirclobber) + tests + docs/chores ride together. The infra fixes are small and were discovered by this ticket's testing, so keeping them is reasonable, but a reviewer should know they're not strictly ROAD-354 scope.
Verification confirmed
- Oliver test contact →
labels: ["买房客户"](246),phone: "+61451122734"; 41 real contacts validated for the 14→2→1 path init --forcerun live → "提取到 30 个密钥" (was 0)- 27 hermetic tests pass; init + cache-poisoning tests confirmed failing against pre-fix code
LGTM to merge.
| if rel_key in self._cache: | ||
| c_db_mt, c_wal_mt, c_path = self._cache[rel_key] | ||
| if c_db_mt == db_mtime and c_wal_mt == wal_mtime and os.path.exists(c_path): | ||
| if c_db_mt == db_mtime and c_wal_mt == wal_mtime and _has_sqlite_header(c_path): | ||
| return c_path |
There was a problem hiding this comment.
🔍 Cache validation cost needs follow-up
Correct cache-hit validation can scan the entire database on every query. Cache successful validation against the cached file’s own metadata to avoid repeated full scans.
Was this helpful? React with 👍 or 👎 to provide feedback.
| messages = [ | ||
| {k: e[k] for k in ('local_id', 'server_id', 'timestamp', 'time', 'sender', 'text')} | ||
| for e in entries |
There was a problem hiding this comment.
🔍 Message identity needs a fallback contract
server_id can be absent before synchronization, while local_id is table-scoped. Document a composite fallback for consumers deduplicating messages across databases.
Was this helpful? React with 👍 or 👎 to provide feedback.
| [project.optional-dependencies] | ||
| dev = [ | ||
| "pytest>=8,<9", | ||
| ] |
There was a problem hiding this comment.
🔍 Regression suite requires explicit setup
The checkout lacks pytest and no project virtual environment exists. Ensure CI installs .[dev] so the new regression tests actually run.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
Implements ROAD-354 — capabilities needed by the WeChat→HubSpot bridge (ROAD-336), plus the upstream key-count fix and image
.datdecryption.contact.extra_bufferprotobuf field 30 (comma-delimitedcontact_label.label_id_list) and join againstcontact_labelfor names. Exposed aslabels/label_idsincontacts --detailJSON andlabelsincontactslist JSON + text.phoneincontacts --detailand list JSON.server_idadded to the message query;collect_chat_historynow returns structured entries.historyJSON emits{local_id, server_id, timestamp, time, sender, text}per message — enableswechat_message_id-based dedup. Text output andexportunchanged.key_mapnow indexes byrel_path(matching the C binary's actual{rel_path: {enc_key}}schema) instead of a nonexistentsaltkey — fixes the "提取到 0 个密钥" misreport..datdecryption (cherry-picked fromlijinma/wechat-cli):history --medianow decrypts encrypted image.datfiles to$TMPDIR/wechat_cli_media— legacy single-byte-XOR.datplus macOS WeChat 4.x V2 (AES-128-ECB head + XOR tail; keys derived locally from kvcommkey_<n>_*.statisticfilenames + account dir name). Includes full-size_h.datpreference and device-suffix wxid handling. Sources never modified; no network calls;ffmpeg(wxgf/HEVC only) viashutil.which+ fixed argv + 15s timeout. Message→file binding uses XML size hints (hdlength/length/cdnthumblength) — probabilistic, documented in AGENTS.md.Both new contact fields share one generic varint/tag parser in
core/contacts.py, per the ticket's possible-solution note.Third-party fork review (evaluate, not merge)
lijinma/wechat-cliimage .dat decryption — ✅ cherry-picked (07786b8,2ea35d5,7297cfe).$TMPDIR/wechat_cli_media; no network calls.32c935e(only removes CDN-download call sites added by76b3cec, which is itself out of scope) and all GUI-automation commits (view_unread, AppleScript chat-list scrolling, keyboard scripts,trigger_wechat_download/try_cdn_download_imagemachinery) — those drive the WeChat GUI / make CDN network calls and stay out per the read-only posture.You-Agent/wechat-clikey-read fix (0ab50ee): reviewed, safe to cherry-pick if wanted later; inert until derived-pair keys exist.hmac_keyis present.db_cachedispatches onkey_type == "sqlcipher_derived_pair"— dormant with today's all-rawall_keys.json; backward-compatible.$TMPDIRto~/.wechat-cli/cache— consistent with the existing~/.wechat-clistate dir, but decrypted DBs then persist across reboots; worth a conscious decision before adopting.image-decryptcommand (7db8cfa) overlaps lijinma's media work — not needed now that lijinma's is in.Test plan
contacts --detail wxid_s5gfoubooxqc12→labels: ["买房客户"](label_id 246),phonedecoded — matches the ROAD-336 Oliver test contact exactlycontact_labelnameshistoryJSON →local_id+server_idpresent per message;--format textandexportoutput unchangedsearchstill works after the row-shape changeinit --forcerun live: correctly reports 30 keys (old logic reported 0); regenerated keys file identical to priorhistory "Angel" --type image --media→ decrypted.jpgpaths in$TMPDIR/wechat_cli_media; outputs verified as valid JPEGs (800×1740, 800×1160)db_dirpreservation, media XOR/V2 decrypt + fallbacksGenerated with Devin