diff --git a/internal/tools/vault_read.go b/internal/tools/vault_read.go index e00c3c05..16df2b65 100644 --- a/internal/tools/vault_read.go +++ b/internal/tools/vault_read.go @@ -169,8 +169,14 @@ func (t *VaultReadTool) Execute(ctx context.Context, args map[string]any) *Resul } // Text-only gate, layer 3: UTF-8 sniff on first N bytes of content. - sniffLen := min(len(content), vaultReadUTF8SniffBytes) - if !utf8.Valid(content[:sniffLen]) { + sniff := content[:min(len(content), vaultReadUTF8SniffBytes)] + if len(sniff) < len(content) { + // The sniff window ends mid-content, so its last bytes may be the + // dangling prefix of a rune the cut split in half. Repair that before + // judging, otherwise valid non-ASCII text is reported as binary. + sniff = trimPartialTrailingRune(sniff) + } + if !utf8.Valid(sniff) { return ErrorResult("file content is not valid UTF-8 text — use read_image/read_audio/read_video/read_document for binary files") } @@ -407,7 +413,25 @@ func readCapped(path string, maxBytes int) ([]byte, bool, error) { return nil, false, err } if len(buf) > maxBytes { - return buf[:maxBytes], true, nil + // The cap is a byte offset, so it can land inside a multi-byte rune; + // drop the dangling prefix instead of emitting a broken character. + return trimPartialTrailingRune(buf[:maxBytes]), true, nil } return buf, false, nil } + +// trimPartialTrailingRune removes up to utf8.UTFMax-1 trailing bytes from b so +// that a cut made at a byte offset does not leave the prefix of a multi-byte +// rune dangling. Accented Vietnamese characters take 2-3 bytes, so a plain byte +// cut breaks the encoding of otherwise valid text; ASCII never does. +// +// Only call this on data known to be truncated. On complete content the +// trailing bytes belong to the file itself and must stay as they are, so that +// genuinely broken bytes are still detected. Stopping after 3 trims keeps the +// repair confined to the cut: a longer run of invalid bytes is real corruption. +func trimPartialTrailingRune(b []byte) []byte { + for trimmed := 0; trimmed < utf8.UTFMax-1 && len(b) > 0 && !utf8.Valid(b); trimmed++ { + b = b[:len(b)-1] + } + return b +} diff --git a/internal/tools/vault_read_test.go b/internal/tools/vault_read_test.go index f80f3757..60e97165 100644 --- a/internal/tools/vault_read_test.go +++ b/internal/tools/vault_read_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "unicode/utf8" "github.com/google/uuid" @@ -459,6 +460,122 @@ func TestVaultRead_Oversize_Truncated(t *testing.T) { } } +// --- 12b. non-ASCII text whose rune straddles the sniff window → readable. --- +func TestVaultRead_MultibyteRuneAtSniffBoundary_Allowed(t *testing.T) { + // The sniff window ends at vaultReadUTF8SniffBytes. Pad with ASCII so the + // window ends inside a multi-byte rune, leaving 1 or 2 bytes of it dangling. + cases := []struct { + name string + padding int + char string // 2-byte and 3-byte Vietnamese characters + }{ + {"two-byte rune split", vaultReadUTF8SniffBytes - 1, "â"}, + {"three-byte rune split after one byte", vaultReadUTF8SniffBytes - 1, "ấ"}, + {"three-byte rune split after two bytes", vaultReadUTF8SniffBytes - 2, "ấ"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tenantID := uuid.New() + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: tenantID.String(), + Scope: "shared", Path: "npk.md", Title: "NPK", DocType: "note", + } + tool, ws := newVaultReadTestTool(t, doc) + // ~25KB total so the file is well past the sniff window. + body := strings.Repeat("a", tc.padding) + strings.Repeat(tc.char+" phân bón ", 2000) + if len(body) <= vaultReadUTF8SniffBytes { + t.Fatalf("fixture must exceed the sniff window, got %d bytes", len(body)) + } + if utf8.ValidString(body[:vaultReadUTF8SniffBytes]) { + t.Fatalf("fixture must split a rune at the sniff boundary") + } + writeFile(t, ws, "npk.md", body) + + res := tool.Execute(makeCtx(tenantID, agentID), + map[string]any{"doc_id": docID.String()}) + if res.IsError { + t.Fatalf("valid UTF-8 document rejected: %s", res.ForLLM) + } + if !strings.Contains(res.ForLLM, "phân bón") { + t.Fatalf("expected document content, got: %.120s", res.ForLLM) + } + }) + } +} + +// --- 12c. binary content past the sniff window → still rejected. --- +func TestVaultRead_LargeBinaryContent_UTF8Rejected(t *testing.T) { + tenantID := uuid.New() + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: tenantID.String(), + Scope: "shared", Path: "renamed-png.md", Title: "Blob", DocType: "note", + } + tool, ws := newVaultReadTestTool(t, doc) + // PNG magic bytes plus 20KB of invalid sequences: trimming the tail of the + // sniff window must not rescue this. + blob := append([]byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, + bytes.Repeat([]byte{0xFF, 0xFE, 0xC3, 0x28}, 5000)...) + writeBytes(t, ws, "renamed-png.md", blob) + + res := tool.Execute(makeCtx(tenantID, agentID), + map[string]any{"doc_id": docID.String()}) + if !res.IsError || !strings.Contains(res.ForLLM, "UTF-8") { + t.Fatalf("expected UTF-8 rejection, got: %.120s", res.ForLLM) + } +} + +// --- 12d. broken byte at the end of a file shorter than the sniff window. --- +func TestVaultRead_ShortFileTrailingBrokenByte_Rejected(t *testing.T) { + tenantID := uuid.New() + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: tenantID.String(), + Scope: "shared", Path: "short.md", Title: "Short", DocType: "note", + } + tool, ws := newVaultReadTestTool(t, doc) + // Nothing is truncated here, so the trailing 0xC3 is the file's own byte + // and must still be reported as broken. + writeBytes(t, ws, "short.md", append([]byte("bảng giá phân bón"), 0xC3)) + + res := tool.Execute(makeCtx(tenantID, agentID), + map[string]any{"doc_id": docID.String()}) + if !res.IsError || !strings.Contains(res.ForLLM, "UTF-8") { + t.Fatalf("expected UTF-8 rejection, got: %.120s", res.ForLLM) + } +} + +// --- 12e. max_bytes landing inside a rune → no broken character in output. --- +func TestVaultRead_MaxBytes_CutInsideRune_NoReplacementChar(t *testing.T) { + tenantID := uuid.New() + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: tenantID.String(), + Scope: "shared", Path: "gia.md", Title: "Gia", DocType: "note", + } + tool, ws := newVaultReadTestTool(t, doc) + body := strings.Repeat("ấm ", 500) // 5 bytes per repetition + writeFile(t, ws, "gia.md", body) + + // 101 is not a multiple of 5, so the cap falls inside the 3-byte "ấ". + res := tool.Execute(makeCtx(tenantID, agentID), + map[string]any{"doc_id": docID.String(), "max_bytes": float64(101)}) + if res.IsError { + t.Fatalf("unexpected error: %s", res.ForLLM) + } + if !strings.Contains(res.ForLLM, "truncated") { + t.Fatalf("expected truncation marker, got: %.120s", res.ForLLM) + } + if strings.ContainsRune(res.ForLLM, utf8.RuneError) { + t.Fatalf("truncated output contains a broken character: %q", res.ForLLM) + } +} + // --- 13. symlink escaping workspace → denied. --- func TestVaultRead_SymlinkEscape_Denied(t *testing.T) { tenantID := uuid.New()