[all-mcp] getContacts has unbounded getX-per-id fan-out (no limit) — sister to #25

[kiki830621]

Problem

Surfaced by PR #32 verify (security reviewer Q2). getContacts has the same
getX-per-id fan-out shape that #25 just bounded for limit — but with no
limit at all. The handler calls getContacts and then issues one getChat
(or equivalent) round-trip per returned contact id, with nothing capping the
number of ids processed.

This is strictly worse than the path #25 fixed: #25's getChats/searchChats
now reject limit > 10_000 via validateLimitCap, but getContacts iterates
the full contact list unconditionally. A large address book translates directly
into N sequential TDLib round-trips.

Type

bug (resource / fan-out amplification)

Expected

Apply the same bounding discipline as #25:

• Cap the number of per-id round-trips (reuse validateLimitCap / a parseLimit-style bound, or page the contact list)
• Decide the right default ceiling (parity with the 10_000 used elsewhere, or a contact-specific bound)
• Test: oversized contact set does not fan out unbounded

Code Reference

• Sources/CheTelegramAllMCPCore/Server.swift — getContacts handler + its per-id loop
• Sources/CheTelegramAllMCPCore/TDLibClient.swift:338-347 — the getChat-per-id fan-out pattern (same shape audited for Apply parseMaxMessages pattern to limit and other numeric option args #25)

Source

PR #32 6-AI verify, security reviewer Q2: #32 (comment)

Related: #25 (bounded the limit fan-out paths), #22/#23 (same parser-consistency cluster)

[kiki830621]

Diagnosis

2-lens workflow (design tradeoff + testability/regression), ground truth verified against the SDK + all callers.

Problem

get_contacts has an unbounded getUser-per-id fan-out. TDLibClient.getContacts() (TDLibClient.swift:544-553) calls client.getContacts() — which returns all contact userIds in one call — then loops one getUser round-trip per id with no cap. A large address book → N sequential TDLib round-trips. Sister to #25 (which bounded the limit-based paths).

Root Cause — the load-bearing difference vs #25

TDLib's getContacts takes no args (no server-side limit), unlike getChats(limit:)/searchChats(limit:) which bound at the source. So parseLimit on the arg only bounds the arg, not the fan-out — the fix MUST truncate result.userIds client-side (prefix(limit)) before the getUser loop. A parseLimit-only test would be a placebo.

Solution — Option (a): client-side truncation

• Add optional limit to the get_contacts schema (Server.swift:152-154, currently properties: [:]).
• Handler (Server.swift:426-427): let limit = try parseLimit(args, default: 200) → tdlib.getContacts(limit: limit).
• TDLibClient.getContacts(limit: Int = 200): loop over boundedContactIds(result.userIds, limit: limit).
• Falsifiable seam: pure boundedContactIds(_ ids: [Int64], limit: Int) -> [Int64] = Array(ids.prefix(limit)), unit-tested (10/5→5, 3/5→3, 5/5→5, empty). The fan-out itself needs a live TDLib connection → untestable in place (same constraint as [all-mcp] Apply int64ArgValueStrict to Server.swift direct callsites (sister to #22) #33).

Architecture correction to the diagnose lens: the helper must live in TelegramAllLib (next to getContacts), NOT HandlerArgs.swift — TelegramAllLib is the lower layer and cannot import CheTelegramAllMCPCore. It will be tested in the existing hermetic TelegramAllLibTests target.

Why not searchContacts (the design lens's alt): the wrapper has no searchContacts method (the SDK does — TDLibApi.swift:18207), and searchContacts(query:"") has uncertain empty-query semantics (typeahead-ranked, not "all contacts in order"). Option (a) is more faithful + minimal and fully bounds the expensive loop.

Backward-compat

getContacts() callers — Server.swift:427, CLI TelegramAllCLI.swift:207, E2E TelegramE2ETests.swift:130 — all consume a bare JSON array. getContacts(limit: Int = 200) (defaulted) keeps all three compiling + consuming an array. Rejected: wrapping output in {contacts, total} (breaks all three callers). Same dual-default pattern as getChatMembers(limit: Int = 200) + handler parseLimit(default: 200).

Behavior change (CHANGELOG)

get_contacts gains optional limit (default 200, 10_000 cap). No-arg callers with >200 contacts now get the first 200 silently truncated (wire-observable; forced by keeping the bare array — a signal would break callers). Default 200 matches getChatMembers, the most analogous bounded tool.

Proceeding with

Option (a), default 200, bare array preserved, silent truncation. Adjustable at the verify checkpoint before merge.