Skip to content

LINE: fix buffer guards in detectContentType + add tests#11419

Closed
rahmat-ullah wants to merge 1 commit intoopenclaw:mainfrom
rahmat-ullah:fix/line-download-buffer-guards
Closed

LINE: fix buffer guards in detectContentType + add tests#11419
rahmat-ullah wants to merge 1 commit intoopenclaw:mainfrom
rahmat-ullah:fix/line-download-buffer-guards

Conversation

@rahmat-ullah
Copy link

@rahmat-ullah rahmat-ullah commented Feb 7, 2026

Summary

Fixes a bug in src/line/download.ts where buffer length guards were insufficient, causing potential out-of-bounds array access when detecting content types from LINE media downloads.

What was fixed

  1. Added per-format buffer length checks: Each magic-byte pattern now has its own length guard matching the exact number of bytes it accesses (JPEG: >=2, PNG: >=4, GIF: >=3, WebP: >=12, M4A: >=8, MP4: >=8)
  2. Reordered M4A before MP4: The M4A check is more specific (requires 0x00 0x00 0x00 prefix + ftyp), so it must come before the generic MP4 check to avoid being unreachable (fixes pre-existing dead code)
  3. Added __testing export: Exposed detectContentType and getExtensionForContentType for unit testing

Testing

  • Added 24 new tests in src/line/download.test.ts:
    • 14 tests for detectContentType (all 6 formats + 8 edge cases including empty buffers, truncated buffers, and unrecognized bytes)
    • 10 tests for getExtensionForContentType (all 7 known MIME types + 3 fallback cases)
  • All new tests pass (24/24)
  • Full test suite: 843 test files pass (8 pre-existing failures unrelated to these changes)
  • pnpm lint and pnpm format both pass

Impact

This is a bug fix with no user-facing behavior changes. It only prevents potential crashes when processing malformed or truncated media files from LINE.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +94 to +97
// M4A/AAC (indices 0–2 and 4–7, needs ≥8 bytes)
// Must come before the generic MP4 check since M4A is a more specific ftyp match.
if (
buffer.length >= 8 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over-broad M4A detection

detectContentType will classify any file with buffer[0..2] === 0x00 and buffer[4..7] === "ftyp" as audio/mp4 (M4A). That pattern also matches many regular MP4s where the initial box size begins with 0x00 0x00 0x00 (e.g., 0x00000018/0x00000020), so valid video MP4s can be misidentified and saved with a .m4a extension. The tests currently avoid this by forcing the MP4 fixture’s third byte to be non-zero, so they won’t catch this misclassification.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/line/download.ts
Line: 94:97

Comment:
**Over-broad M4A detection**

`detectContentType` will classify any file with `buffer[0..2] === 0x00` and `buffer[4..7] === "ftyp"` as `audio/mp4` (M4A). That pattern also matches many regular MP4s where the initial box size begins with `0x00 0x00 0x00` (e.g., `0x00000018/0x00000020`), so valid video MP4s can be misidentified and saved with a `.m4a` extension. The tests currently avoid this by forcing the MP4 fixture’s third byte to be non-zero, so they won’t catch this misclassification.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +143 to +146
export const __testing = {
detectContentType,
getExtensionForContentType,
} as const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production __testing export

Exporting __testing from a non-test module makes it part of the runtime/public export surface, which consumers can start depending on. Since this is only needed for tests, it’s safer to avoid a special production-only export (e.g., move these helpers into a separate module that both production code and tests import, or export the helpers normally if they’re intended to be public).

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/line/download.ts
Line: 143:146

Comment:
**Production `__testing` export**

Exporting `__testing` from a non-test module makes it part of the runtime/public export surface, which consumers can start depending on. Since this is only needed for tests, it’s safer to avoid a special production-only export (e.g., move these helpers into a separate module that both production code and tests import, or export the helpers normally if they’re intended to be public).

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

SutanuNandigrami pushed a commit to SutanuNandigrami/openclaw that referenced this pull request Feb 8, 2026
robertchang-ga added a commit to robertchang-ga/openclaw that referenced this pull request Feb 24, 2026
@discordjs/voice 0.19.x has a known broken DAVE receive implementation
(GitHub openclaw#11419) causing DecryptionFailed(UnencryptedWhenPassthroughDisabled)
on all incoming audio. Disabling DAVE falls back to standard XSalsa20
transport encryption (still encrypted, just not E2E).

TODO: Re-enable when @discordjs/voice ships a fix (DAVE enforced March 2 2026).

Also:
- Added info-level logging to voice capture pipeline stages
- Connected speaches to openclaw-secure-net for container networking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants