🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

function normalizeEchoTextKey(text: string | undefined): string | null {
  if (!text) {
    return null;
  }
  const normalized = stripLeadingIMessageEchoCorruption(text.replace(/\r\n?/g, "\n")).trim();
  return normalized ? normalized : null;
}

function normalizeEchoTextKey(text: string | undefined, opts?: { stripLeadingCorruption?: boolean }): string | null {
  if (!text) return null;
  const lf = text.replace(/\r\n?/g, "\n");
  const maybeStripped = opts?.stripLeadingCorruption ? stripLeadingIMessageEchoCorruption(lf) : lf;
  const normalized = maybeStripped.trim();
  return normalized ? normalized : null;
}

Analyzed PR: #73985 at commit febe43f

_{Last updated on: 2026-04-29T02:45:08Z}

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

Greptile Summary

This PR fixes iMessage echo-cache deduplication failures caused by leading attributedBody corruption bytes (control characters, C1 characters, and Unicode noncharacters) that iMessage prepends to reflected inbound text but not to the originally-sent text. The fix adds a leading-only stripping pass in normalizeEchoTextKey so both the stored and lookup keys normalize to the same value, restoring echo suppression without discarding interior replacement characters or line-structure content.

Confidence Score: 5/5

Safe to merge — the fix is well-scoped, symmetric across store and lookup paths, and backed by focused regression tests.

The logic is correct: normalization is applied identically to both remember and has paths so no false positives can be introduced. The corruption-code predicate correctly excludes mid-message content (emoji, interior U+FFFD, tabs, newlines after the leading position). Tests cover the primary regression scenario and the interior-preservation invariant. No security surface is affected.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(imessage): normalize leading echo-ca..." | Re-trigger Greptile}

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Keep this PR open. Current main and the latest release still lack leading iMessage attributedBody corruption-marker normalization in the sent-message echo cache, so the PR is not obsolete or implemented. It should be reviewed or consolidated with the overlapping open Clownfish PR, and the security review should address the non-injective text normalization risk for group echo scopes.

Best possible solution:

Keep the PR open for maintainer review, preferably as the current candidate or after consolidating with #73891. The best fix should keep the normalization narrowly scoped to proven leading iMessage attributedBody artifacts, add regression tests for corrupted-prefix echoes and preserved interior content, and either address or explicitly bound the group-scope false-positive risk. Any TTL change for #59973 should be decided separately or covered by a narrower follow-up.

What I checked:

• Current echo-cache behavior: Current main still normalizes echo text keys with only CRLF-to-LF conversion and trim; there is no stripping of leading U+FFFD, C0/C1 controls, or other attributedBody corruption markers. (extensions/imessage/src/monitor/echo-cache.ts:26, 180033eeaedd)
• Current tests preserve short text-only fallback behavior: The existing regression test still asserts that text-only echo fallback misses after 6 seconds, which means the broader delayed-echo part of iMessage DM echo: corrupted prefix breaks text-based dedup (v2026.3.31+) #59973 remains a separate review decision from this PR's leading-corruption normalization. (extensions/imessage/src/monitor/monitor-provider.echo-cache.test.ts:32, 180033eeaedd)
• Latest release check: The latest release tag v2026.4.26 contains the same echo-cache implementation: normalizeEchoTextKey only replaces CRLF and trims whitespace. The proposed PR is therefore not already shipped. (extensions/imessage/src/monitor/echo-cache.ts:26, be8c24633aaa)
• Security review: group echo scope: Current group echo scope is account plus chat target, not sender. The PR's proposed leading-control/U+FFFD normalization makes text keys less injective, so Aisle's low-severity concern about possible group-message suppression is grounded in the current scope shape. (extensions/imessage/src/monitor/inbound-processing.ts:656, 180033eeaedd)
• PR diff and discussion context: The provided PR diff only touches CHANGELOG.md, extensions/imessage/src/monitor/echo-cache.ts, and extensions/imessage/src/monitor/monitor-provider.echo-cache.test.ts, with no CI, dependency, lockfile, publishing, or lifecycle-script changes. The security-sensitive concern is functional canonicalization, not supply-chain surface. (febe43f51ebb)
• Related implementation context: The PR is tied to open trackers iMessage channel duplicate message loop - messages sent by agent are re-ingested as inbound #41330 and iMessage DM echo: corrupted prefix breaks text-based dedup (v2026.3.31+) #59973, carries forward source PR fix(imessage): strip U+FFFD garbage chars from echo text key normalization #62191, and overlaps another open Clownfish PR fix(imessage): normalize leading echo-cache corruption bytes #73891. That makes it an active implementation/review candidate rather than a cleanup-close candidate.

Likely related people:

• neeravmakwana: Current code comments and tests reference the destination_caller_id/self-chat line of work, and the changelog credits @neeravmakwana for recent iMessage self-chat/reflected-echo fixes. The related closed PR fix(imessage): catch delayed reflected echoes #59980 also attempted this same delayed/corrupted echo-cache problem. (role: recent iMessage self-chat and reflected-echo contributor; confidence: medium; commits: [CHANGELOG.md](https://github.com/openclaw/openclaw/commit/CHANGELOG.md entry for fix(imessage): avoid DM self-chat false positives #61619/[Bug]: iMessage (imsg) channel - binary length prefix leaking into message content #63868/iMessage echo detection: DM misidentified as self-chat when sender == chat_identifier #63980/fix(imessage): require destination_caller_id for self-chat classification (#63980) #63989/fix(imessage): strip length-prefixed UTF-8 from imsg rpc text #64000), [related PR #](related PR fix(imessage): catch delayed reflected echoes #59980); files: extensions/imessage/src/monitor/inbound-processing.ts, extensions/imessage/src/monitor/self-chat-dedupe.test.ts, extensions/imessage/src/monitor/echo-cache.ts)
• joelnishanth: The changelog credits @joelnishanth for iMessage echo-loop hardening that stripped reflected assistant content, extended echo-cache text retention for delayed reflections, and suppressed loop amplification, which is directly adjacent to this PR's echo-cache behavior. (role: earlier iMessage echo-loop hardening contributor; confidence: medium; commits: [CHANGELOG.md](https://github.com/openclaw/openclaw/commit/CHANGELOG.md entry for fix(imessage): prevent echo loop from leaking internal metadata and amplifying NO_REPLY into queue overflow #33295); files: extensions/imessage/src/monitor/echo-cache.ts, extensions/imessage/src/monitor/inbound-processing.ts)
• vincentkoc: Recent changelog entries credit @vincentkoc for iMessage channel maintenance, and the timeline shows vincentkoc reopened this PR for continued review. The local git history is shallow/grafted, so this is a routing signal rather than exact authorship. (role: recent adjacent maintainer; confidence: low; commits: [CHANGELOG.md](https://github.com/openclaw/openclaw/commit/CHANGELOG.md entries for Bonjour advertiser failure cascades into iMessage channel teardown #65393), [timeline reo](timeline reopened event for this PR); files: extensions/imessage/src/monitor/echo-cache.ts, extensions/imessage/src/monitor/inbound-processing.ts)

Remaining risk / open question:

• The PR's broad leading-control/U+FFFD normalization is non-injective; in group chats the current echo scope is per account/chat, not sender, so review should address Aisle's potential false-drop scenario before merge.
• This PR overlaps fix(imessage): normalize leading echo-cache corruption bytes #73891 and earlier source PR fix(imessage): strip U+FFFD garbage chars from echo text key normalization #62191. Maintainers should consolidate the active branch choice to avoid two Clownfish PRs carrying similar changelog and test changes.
• The related iMessage DM echo: corrupted prefix breaks text-based dedup (v2026.3.31+) #59973 report also mentions 5-30 second delayed echoes, while current main still has a 4 second text fallback and this PR only targets leading corruption markers.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 180033eeaedd.

Thanks for pushing this. We merged the narrower canonical fix in #73942: #73942, merge be445dd. This broader control-byte variant had a real group-scope false-positive risk called out in review; #73942 avoids that by keeping normalization leading-marker-only and preserving tabs, line breaks, and interior markers. Testbox proof: tbx_01kqbp2m7s1mn1nd4fhgnp74wk focused iMessage tests plus check:changed passed. Closing this overlap.