Skip to content

fix(security): expand MagicByteValidator beyond image-only allowlist - #626

Merged
Aaronontheweb merged 3 commits into
devfrom
claude-wt-slack-pdf-issues
Apr 13, 2026
Merged

fix(security): expand MagicByteValidator beyond image-only allowlist#626
Aaronontheweb merged 3 commits into
devfrom
claude-wt-slack-pdf-issues

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Summary

  • Production Slack session D0AC6CKBK5K/1776042979.764809 rejected a PDF with Content scanner rejected … File extension '.pdf' is not allowed. Root cause: PR feat(channels): audience-gated attachment ingress contract + Slack PDF support #601 added ChannelAttachmentPolicy audience gates that allow PDFs, OOXML, archives, and media — but MagicByteValidator was still hardcoded to PNG/JPG/GIF/WebP, so everything else was rejected at the scanner layer after passing the policy layer.
  • Rewrite MagicByteValidator around a signature-rule table keyed by declared MIME. Support every category the Team audience advertises: PDF, OOXML (docx/xlsx/pptx), ODF, legacy OLE Office, plain/structured text, RTF, zip/7z/rar/gzip/bzip2/xz, and mp3/mp4/m4a/wav/ogg/avi/webm/mkv. Seed ContentPolicy.DefaultAllowedMimeTypes from MagicByteValidator.GetSupportedMimeTypes() so the two layers can't drift again, and raise DefaultMaxFileSizeBytes from 20 MB to 25 MiB to match ChannelAttachmentPolicy.DefaultMaxFileBytes.
  • Harden each matcher beyond minimum magic: JPEG rejects stuffed-byte markers, GIF validates full GIF8[79]a, PDF requires a version digit + dot, RTF requires a version digit, ZIP enforces exact header-pair combinations, gzip requires the DEFLATE compression method, bzip2 validates the full 10-byte BCD-Pi block header, RAR requires the v4 (00) or v5 (01 00) variant tail, ISO BMFF validates box size + printable-ASCII major brand, Ogg requires version byte 0x00, and MP3 uses strict 12-bit sync plus reserved-layer rejection and ID3v2 major-version validation.
  • Flip SlackAttachmentIngressTests.BuildGateway to default to the real MagicByteContentScanner instead of NullContentScanner. This is the test-harness gap that allowed PR feat(channels): audience-gated attachment ingress contract + Slack PDF support #601 to ship — the existing Pdf_in_dm_* and Docx_in_dm_* regression tests now exercise production behavior. Adds a PlainText_in_dm_* ingress test and 15 adversarial polyglot-rejection tests for the hardened matchers.

Test plan

  • dotnet test src/Netclaw.Security.Tests — 259/259 passing (+37 new cases)
  • dotnet test src/Netclaw.Actors.Tests --filter "FullyQualifiedName~Slack" — 147/147 passing
  • dotnet test full suite — all green
  • dotnet slopwatch analyze — 0 issues
  • Manual verification in dev Slack: DM a PDF, docx, zip, and mp4 to the bot. Expect [attachment] lines with path="inbox/<name>.<ext>" for all four; PDF inlined as DataContent on vision-capable models; docx/zip/mp4 path-only with FormatNotInlineable. Files present under sessions/<sessionId>/inbox/.
  • Regression guard: send a .exe renamed to .pdf → expect ExecutableContent rejection. Send a PNG renamed to report.pdf → expect MimeTypeMismatch rejection.

PR #601 added the audience-gated ChannelAttachmentPolicy that allows
PDFs, Office documents, archives, and media for Team/Personal
audiences — but MagicByteValidator's hardcoded AllowedExtensions
dictionary still only accepted PNG/JPG/GIF/WebP, rejecting everything
else at ingress with "File extension '.pdf' is not allowed". The
SlackAttachmentIngressTests suite used NullContentScanner by default,
so the Pdf_in_dm_* and Docx_in_dm_* tests only exercised the policy
layer and never saw the real scanner.

Rewrite MagicByteValidator around a signature-rule table keyed by
declared MIME. Support every category the Team audience advertises:
PDF, OOXML/ODF, legacy OLE Office, plain/structured text, RTF,
zip/7z/rar/gzip/bzip2/xz, and mp3/mp4/wav/ogg/avi/webm/mkv. Harden
each matcher beyond minimum magic — validate PDF version digit, ZIP
exact header pair, gzip DEFLATE method, bzip2 BCD-Pi block header,
RAR v4/v5 variant tail, ISO BMFF box size + printable-ASCII major
brand, Ogg version byte, ID3v2 major version, MP3 strict 12-bit sync
plus reserved-layer check. Seed ContentPolicy.DefaultAllowedMimeTypes
from the validator's supported set so the two layers can't drift, and
raise DefaultMaxFileSizeBytes from 20 MB to 25 MiB to match
ChannelAttachmentPolicy.

Flip SlackAttachmentIngressTests.BuildGateway to default to the real
MagicByteContentScanner so the existing Pdf_in_dm_* and Docx_in_dm_*
regression tests now actually exercise production behavior. Add a
PlainText_in_dm_* ingress test, 13 MagicByteValidator category happy
paths, and 15 adversarial polyglot-rejection tests for the hardened
matchers.
@Aaronontheweb Aaronontheweb added .NET Pull requests that update .NET code security Security-related changes labels Apr 13, 2026
@Aaronontheweb
Aaronontheweb marked this pull request as ready for review April 13, 2026 10:40
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) April 13, 2026 10:40
@Aaronontheweb
Aaronontheweb merged commit a55df09 into dev Apr 13, 2026
4 checks passed
@Aaronontheweb
Aaronontheweb deleted the claude-wt-slack-pdf-issues branch April 13, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Pull requests that update .NET code security Security-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant