Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion src/Netclaw.Actors.Tests/Channels/SlackAttachmentIngressTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ public sealed class SlackAttachmentIngressVisionTests : TestKit
private static readonly byte[] FakeDocxBytes =
"PK\u0003\u0004fake docx content"u8.ToArray();

private static readonly byte[] FakePlainTextBytes =
"meeting notes\n- discuss Q2 roadmap\n- assign OKRs\n"u8.ToArray();

private readonly RecordingChatClient _chatClient = new();
private readonly RecordingReplyClient _replyClient = new();
private readonly ConfigurableFakeSlackFileHandler _httpHandler = new();
Expand Down Expand Up @@ -125,7 +128,7 @@ private IActorRef BuildGateway(
BotUserId: new SlackUserId("UBOT"),
DefaultChannelId: null,
ReplyClient: _replyClient,
ContentScanner: scanner ?? new NullContentScanner(),
ContentScanner: scanner ?? new MagicByteContentScanner(new ContentPolicy()),
ThreadHistoryFetcher: EmptyThreadHistoryFetcher.Instance,
AudienceProfiles: profiles,
ModelCapabilities: Host.Services.GetRequiredService<ModelCapabilities>(),
Expand Down Expand Up @@ -437,6 +440,57 @@ await AwaitAssertAsync(() =>
}, duration: TimeSpan.FromSeconds(10), cancellationToken: TestContext.Current.CancellationToken);
}

[Fact]
public async Task PlainText_in_dm_flows_through_real_magic_byte_scanner()
{
// Regression: before the MagicByteValidator rewrite, only image MIMEs
// passed the scanner — text/plain was rejected with
// "File extension '.txt' is not allowed" even though the policy layer
// allowed it. This test uses the default (real) MagicByteContentScanner
// to prove the broader category support end-to-end.
_httpHandler.RespondWith("text/plain", FakePlainTextBytes);
var gateway = BuildGateway("slack-gw-plaintext-flow");

var files = new List<SlackFileReference>
{
new("F_TXT", "notes.txt", "text/plain", FakePlainTextBytes.Length,
"https://files.slack.com/files-pri/T1234-F_TXT/notes.txt")
};

gateway.Tell(new SlackInboundMessage(
Kind: SlackInboundKind.Message,
EventId: new SlackEventId("D_TXT:3800"),
ChannelId: new SlackChannelId("D_TXT"),
ThreadTs: null,
EventTs: new SlackEventTs("3800.1"),
UserId: new SlackUserId("U_HUMAN"),
BotId: null,
Text: "here are my notes",
Subtype: null,
Hidden: false,
IsDirectMessage: true,
Files: files));

await AwaitAssertAsync(() =>
{
Assert.Contains(_chatClient.ReceivedMessages,
contents => contents.Any(c => c is TextContent t
&& t.Text.Contains("[attachment]", StringComparison.Ordinal)
&& t.Text.Contains("notes.txt", StringComparison.Ordinal)
&& t.Text.Contains("path=\"inbox/notes.txt\"", StringComparison.Ordinal)));
}, duration: TimeSpan.FromSeconds(10), cancellationToken: TestContext.Current.CancellationToken);

var sessionId = new SessionId("D_TXT/3800.1");
var inboxPath = Path.Combine(
SessionDirectoryHelper.GetOrCreateInboxDirectory(sessionId, _paths.SessionsDirectory),
"notes.txt");
Assert.True(File.Exists(inboxPath), $"Expected inbox file at {inboxPath}");

// Scanner should not have posted any rejection reply.
Assert.DoesNotContain(_replyClient.PostedMessages,
m => m.Text.Contains("Content scanner rejected", StringComparison.Ordinal));
}

[Fact]
public async Task Scanner_rejection_surfaces_user_visible_reply_with_no_inbox_write()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1174,10 +1174,9 @@ await AwaitAssertAsync(() =>
}

[Fact]
public async Task Scanner_failure_does_not_silently_drop_image()
public async Task Scanner_failure_rejects_attachment_and_does_not_inline()
{
// When the content scanner itself is broken (e.g. TypeInitializationException),
// images should still flow through to the LLM rather than being silently dropped.
// Scanner failures must fail closed for inbound attachments.
var pipeline = Host.Services.GetRequiredService<SessionPipeline>();
var httpClient = new HttpClient(_httpHandler);

Expand Down Expand Up @@ -1231,8 +1230,11 @@ await AwaitAssertAsync(() =>
"Expected at least one Slack reply to be posted");
}, duration: TimeSpan.FromSeconds(10), cancellationToken: TestContext.Current.CancellationToken);

Assert.True(_chatClient.ReceivedImageContent,
"Expected LLM to receive image even when scanner is broken");
Assert.False(_chatClient.ReceivedImageContent,
"Expected LLM not to receive image when scanner fails");

Assert.Contains(_replyClient.PostedMessages,
m => m.Text.Contains("Couldn't scan `drawing.png`", StringComparison.Ordinal));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,107 @@ await AwaitAssertAsync(() =>
}, duration: TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken);
}

[Fact]
public async Task Backfill_document_in_public_channel_is_not_forwarded_as_data_content()
{
var pipeline = Host.Services.GetRequiredService<SessionPipeline>();
var httpClient = new HttpClient(_httpHandler);

var fetcher = new SlackThreadHistoryFetcher(
(channelId, threadTs, limit, cursor, ct) =>
Task.FromResult(new SlackNet.WebApi.ConversationMessagesResponse
{
Messages =
[
new SlackNet.Events.MessageEvent
{
Ts = threadTs,
User = "U_ROOT",
Text = "thread root"
},
new SlackNet.Events.MessageEvent
{
Ts = $"{threadTs[..^1]}1",
User = "U_ALICE",
Text = "historical doc",
Files =
[
new SlackNet.File
{
Id = "F_DOCX",
Name = "notes.docx",
Mimetype = "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
Size = FakePngBytes.Length,
UrlPrivateDownload = "https://files.slack.com/fake/notes.docx"
}
]
}
]
}),
new SlackChannelOptions { BotToken = new SensitiveString("xoxb-fake") },
httpClient,
new NullContentScanner(),
NullLogger<SlackThreadHistoryFetcher>.Instance);

var profiles = ToolAudienceProfileDefaults.CreateProfiles();
profiles.Public.ChannelAttachments = ToolAudienceProfileDefaults.CreatePublicChannelAttachments();

var deps = new SlackGatewayDependencies(
Pipeline: pipeline,
IngressGate: null,
ActorSystem: Sys,
TimeProvider: TimeProvider.System,
Options: new SlackChannelOptions
{
Enabled = true,
MentionOnly = true,
AllowedChannelIds = ["C_PUBLIC"],
ChannelAudiences = new Dictionary<string, string>(StringComparer.Ordinal)
{
["C_PUBLIC"] = "public"
},
BotToken = new SensitiveString("xoxb-fake-token")
},
BotUserId: new SlackUserId("UBOT"),
DefaultChannelId: null,
ReplyClient: _replyClient,
ContentScanner: new NullContentScanner(),
HttpClient: httpClient,
ThreadHistoryFetcher: fetcher,
AudienceProfiles: profiles,
ModelCapabilities: TestSlackGatewayDeps.DefaultVisionCapableModel,
Paths: _paths);

var gateway = Sys.ActorOf(SlackGatewayActor.CreateProps(deps), "slack-gw-backfill-public-doc");

gateway.Tell(new SlackInboundMessage(
Kind: SlackInboundKind.AppMention,
EventId: new SlackEventId("C_PUBLIC:7000.2"),
ChannelId: new SlackChannelId("C_PUBLIC"),
ThreadTs: new SlackThreadTs("7000.0"),
EventTs: new SlackEventTs("7000.2"),
UserId: new SlackUserId("U_MENTIONER"),
BotId: null,
Text: "<@UBOT> please summarize",
Subtype: null,
Hidden: false,
IsDirectMessage: false));

await AwaitAssertAsync(() =>
{
Assert.True(_chatClient.CallCount > 0, "Expected at least one LLM call");
}, duration: TimeSpan.FromSeconds(10), cancellationToken: TestContext.Current.CancellationToken);

var messages = _chatClient.LastMessages!;
var user = Assert.Single(messages, m => m.Role == AiChatRole.User);

Assert.Empty(user.Contents.OfType<DataContent>());

var mergedText = string.Join("", user.Contents.OfType<TextContent>().Select(t => t.Text));
Assert.Contains("attachment rejected", mergedText, StringComparison.OrdinalIgnoreCase);
Assert.Contains("category not allowed", mergedText, StringComparison.OrdinalIgnoreCase);
}

// --- Fake replies fetcher ---

private Task<SlackNet.WebApi.ConversationMessagesResponse> FakeRepliesFetcher(
Expand Down
90 changes: 60 additions & 30 deletions src/Netclaw.Channels.Slack/SlackThreadBindingActor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -473,22 +473,18 @@ private async Task<AttachmentIngestResult> TryIngestSingleAttachmentAsync(

if (!scanResult.IsAllowed)
{
_log.Warning(
"slack_attachment_rejected name={Name} mime={Mime} reason=scan-blocked error={ScanError} message={ScanMessage}",
file.Name, file.MimeType, scanResult.Error?.ToString(), scanResult.Message ?? scanResult.Error?.ToString());

if (scanResult.Error == ContentScanError.ScanFailure)
{
// Scanner itself is broken — allow the file through rather than
// silently dropping it. The LLM provider will still validate.
_log.Error(
"Content scanner failed for file {Name}: {Message} — allowing file through",
file.Name, scanResult.Message);
}
else
{
_log.Warning(
"slack_attachment_rejected name={Name} mime={Mime} reason=scan-blocked message={ScanMessage}",
file.Name, file.MimeType, scanResult.Message ?? scanResult.Error?.ToString());
return new AttachmentIngestResult.Rejected(
$"Content scanner rejected `{file.Name}`: {scanResult.Message ?? scanResult.Error?.ToString()}.");
$"Couldn't scan `{file.Name}` — please try again later.");
}

return new AttachmentIngestResult.Rejected(
$"Content scanner rejected `{file.Name}`: {scanResult.Message ?? scanResult.Error?.ToString()}.");
}

// Write to inbox with filesystem-level collision suffixing and atomic move.
Expand Down Expand Up @@ -794,7 +790,12 @@ private async Task<InboundBuildResult> BuildInputForInboundAsync(
if (gap.Count == 0)
return new InboundBuildResult(baseInput, detectorUnavailable);

var mergedContents = MergeGapWithLiveContents(gap, liveContents);
var mergedContents = MergeGapWithLiveContents(
gap,
liveContents,
triggeringMessage.Audience,
_dependencies.AudienceProfiles,
_dependencies.ModelCapabilities);
return new InboundBuildResult(baseInput with { Contents = mergedContents }, detectorUnavailable);
}

Expand Down Expand Up @@ -875,12 +876,25 @@ private enum ClassificationOutcome

private readonly record struct CursorAdvanced(string CursorTs);

private static List<AIContent> MergeGapWithLiveContents(IReadOnlyList<ChannelInput> gap, IReadOnlyList<AIContent> liveContents)
private static List<AIContent> MergeGapWithLiveContents(
IReadOnlyList<ChannelInput> gap,
IReadOnlyList<AIContent> liveContents,
TrustAudience audience,
ToolAudienceProfiles? audienceProfiles,
ModelCapabilities modelCapabilities)
{
var profile = ToolAudienceProfileDefaults.GetResolvedProfile(audienceProfiles, audience);
var attachmentPolicy = profile.ChannelAttachments ?? ChannelAttachmentPolicy.Empty;
var inlineImages = modelCapabilities.InputModalities.HasFlag(ModelModality.Image);
var inlinePdfs = modelCapabilities.InputModalities.HasFlag(ModelModality.Image);

var sb = new StringBuilder();
sb.AppendLine("[thread history — messages exchanged before this inbound event]");
sb.AppendLine();

var merged = new List<AIContent>();
var acceptedBackfillData = new List<AIContent>();

foreach (var item in gap)
{
var ts = item.ReceivedAt == default ? string.Empty : $", {item.ReceivedAt:yyyy-MM-dd HH:mm} UTC";
Expand All @@ -894,8 +908,36 @@ private static List<AIContent> MergeGapWithLiveContents(IReadOnlyList<ChannelInp
case TextContent text when !string.IsNullOrWhiteSpace(text.Text):
sb.AppendLine(text.Text);
break;
case DataContent:
imageCount++;

case DataContent data:
var mimeType = data.MediaType ?? "application/octet-stream";
var category = AttachmentCategories.FromMime(mimeType);

if (!attachmentPolicy.Allows(category))
{
sb.AppendLine(
$"[attachment rejected: historical attachment ({EscapeQuoted(mimeType)}) category not allowed in {audience}]");
break;
}

var (inlined, note) = ResolveInlineDecision(category, inlineImages, inlinePdfs);
if (!inlined)
{
var effectiveNote = note ?? AttachmentNotes.FormatNotInlineable;
sb.AppendLine(
$"[attachment] mime=\"{EscapeQuoted(mimeType)}\" inlined=\"false\" note=\"{EscapeQuoted(effectiveNote)}\"");
break;
}

acceptedBackfillData.Add(data);
if (category == AttachmentCategory.Image)
{
imageCount++;
}
else
{
sb.AppendLine($"[attachment] mime=\"{EscapeQuoted(mimeType)}\" inlined=\"true\"");
}
break;
}
}
Expand All @@ -917,20 +959,8 @@ private static List<AIContent> MergeGapWithLiveContents(IReadOnlyList<ChannelInp
? sb.ToString()
: $"{sb}\n\n{liveText}";

var merged = new List<AIContent> { new TextContent(mergedText) };

// Gap image DataContent is carried through to the session so vision-
// capable models see the prior-thread images. The session actor's
// modality gate strips them when the resolved model doesn't support
// image input, so non-vision models are unaffected.
foreach (var item in gap)
{
foreach (var content in item.Contents)
{
if (content is not TextContent)
merged.Add(content);
}
}
merged.Add(new TextContent(mergedText));
merged.AddRange(acceptedBackfillData);

foreach (var content in liveContents)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public sealed class AttachmentCategoriesTests
[InlineData("text/markdown", AttachmentCategory.Document)]
[InlineData("application/json", AttachmentCategory.Document)]
[InlineData("application/zip", AttachmentCategory.Archive)]
[InlineData("application/x-tar", AttachmentCategory.Archive)]
[InlineData("application/x-tar", AttachmentCategory.Other)]
[InlineData("application/gzip", AttachmentCategory.Archive)]
[InlineData("application/x-7z-compressed", AttachmentCategory.Archive)]
[InlineData("video/mp4", AttachmentCategory.Media)]
Expand Down
3 changes: 0 additions & 3 deletions src/Netclaw.Configuration/ChannelAttachmentPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,9 @@ public static AttachmentCategory FromMime(string? mime)
{
"application/zip" => true,
"application/x-zip-compressed" => true,
"application/x-tar" => true,
"application/gzip" => true,
"application/x-gzip" => true,
"application/x-7z-compressed" => true,
"application/x-rar-compressed" => true,
"application/vnd.rar" => true,
"application/x-bzip2" => true,
"application/x-xz" => true,
_ => false
Expand Down
Loading
Loading