/attach use file upload instead of embedding in the context

[jeanlaurent]

PR: Anthropic Files API Integration

What it does

Attachments are now uploaded once to Anthropic's Files API and referenced by ID, instead of being base64-
encoded inline in every request. This dramatically reduces context window usage for image and document
attachments.

Benefits

• Massive token savings: A 1MB image that previously consumed ~200k tokens now uses just a file reference
• Deduplication: Same file attached multiple times shares a single upload (content-hash based)
• Supports more file types: Images (jpeg, png, gif, webp) and documents (pdf, txt)
• Backward compatible: Existing sessions with base64 attachments continue to work

Supported file types

Type
Images: image/jpeg , image/png , image/gif , image/webp
Documents: application/pdf , text/plain , text/markdown

[github-actions]

Code Review Summary

I've identified several concurrency and security issues in the new file upload implementation.

🔴 HIGH SEVERITY Issues

1. Race Condition in FileManager.GetOrUpload (files.go:59-98)

The GetOrUpload method has a critical race condition that can cause duplicate uploads to Anthropic's API.

Problem: The read lock is released before the actual upload happens. If two goroutines call GetOrUpload with the same file simultaneously:

1. Both pass the cache checks (lines 64-73 and 79-87)
2. Both release locks and proceed to upload
3. Both call fm.upload() for the same file, creating duplicate uploads
4. Both cache their results, potentially overwriting each other

Impact: Wasted bandwidth, duplicate files in Anthropic storage, potential cost implications.

Fix: Add an "in-flight uploads" tracking mechanism:

type FileManager struct {
    mu       sync.RWMutex
    uploads  map[string]*UploadedFile
    paths    map[string]string
    inFlight sync.Map // hash -> chan *UploadedFile
}

Then coordinate concurrent uploads for the same hash using channels.

---

🟡 MEDIUM SEVERITY Issues

2. TOCTOU Race Condition (runner.go:335-337)

Time-of-Check-Time-of-Use vulnerability: File existence is checked with os.Stat() at line 335, but the actual file read happens much later in FileManager.GetOrUpload(). Between these points, the file could be deleted, moved, or have permissions changed.

Impact: Upload fails with confusing error "failed to open file" instead of the intended "file does not exist" message.

3. Security: Path Traversal & Information Disclosure (runner.go:334-341)

Two concerns:

1. Information Disclosure: Absolute paths are logged at line 341, revealing filesystem structure
2. Path Traversal: No validation prevents ../../etc/passwd style attacks

Fix: Add path validation and sanitize logs:

if !strings.HasPrefix(filepath.Clean(absPath), allowedBaseDir) {
    return session.UserMessage(userContent)
}

4. Silent Failure When FileManager is nil (beta_converter.go:110, client.go:524)

When c.fileManager is nil, files are silently skipped with only a warning log. The API call proceeds without the file, causing confusing model behavior.

Fix: Return an error instead of continuing:

if c.fileManager == nil {
    return nil, fmt.Errorf("file upload failed: FileManager not initialized")
}

---

Recommendation

Please address the HIGH severity race condition before merging. The MEDIUM issues should also be fixed to improve security and reliability.

---

This review was performed by an automated code analysis agent.

[jeanlaurent]

Issues fixed but the topcou one as the file path is submited by the users and never send remotely

[jeanlaurent]

/review

[github-actions]

Review Summary

Found 3 critical bugs related to modifying maps during iteration in the new FileManager code. These will cause undefined behavior and potential panics at runtime.

All issues are in the newly added files.go file and must be fixed before merging.

[jeanlaurent]

/review again

[github-actions]

Code Review Summary

This PR adds file upload support to Anthropic's Files API, replacing base64 encoding. I found 1 confirmed bug and 2 areas for improvement:

✅ Confirmed Issues

MEDIUM severity: File uploaded before MIME type validation (beta_converter.go)

⚠️ Areas for Improvement

MEDIUM severity: TOCTOU race condition acknowledged but not mitigated (app.go)
LOW severity: DetectMimeType allows unsupported file types without early validation (chat.go)

📊 Analysis Summary

• Reviewed 11 changed files
• Generated 7 bug hypotheses
• Verified all hypotheses
• Found 1 confirmed bug requiring fix
• Found 2 areas worth noting

The core file upload implementation is well-designed with proper concurrency handling, resource cleanup, and deduplication. The main issue is validation order.

---

Note: See inline comments below for details on each issue.

[krissetto]

will give this branch a try later! ❤️

[jeanlaurent]

/review again ...

[github-actions]

Review Summary

Found 3 issues in the file upload implementation that could affect reliability:

• 1 HIGH severity: Context cancellation after successful upload deletes the file, breaking deduplication
• 2 MEDIUM severity: Resource leaks and TOCTOU race conditions

High Priority Issue

The context cancellation handling has a critical flaw where successfully uploaded files are deleted if the context is cancelled, preventing reuse and contaminating the cache.

Recommendations

1. Fix the context cancellation logic to preserve successfully uploaded files
2. Add content verification before upload to prevent TOCTOU issues
3. Implement cleanup on partial failures to prevent resource leaks

[github-actions]

MEDIUM SEVERITY: File upload not cleaned up if content block creation fails

A file is successfully uploaded at line 158 (GetOrUpload succeeds), but if createBetaFileContentBlock fails at line 162, the uploaded file is never cleaned up. The file remains in Anthropic's storage and in the FileManager's cache, leaking resources.

While validation earlier in the pipeline ensures only supported MIME types should reach this point via IsSupportedMimeType(), there's a potential inconsistency: if MIME type checking at lines 165 (IsImageMime) and 170 (IsDocumentMime) diverges from IsSupportedMimeType(), this path could be triggered.

Suggested fix: Delete the uploaded file on error:

block, err := createBetaFileContentBlock(uploaded.FileID, uploaded.MimeType)
if err != nil {
    if delErr := c.fileManager.Delete(ctx, uploaded.FileID); delErr != nil {
        slog.Warn("Failed to cleanup uploaded file after block creation failure",
            "file_id", uploaded.FileID, "error", delErr)
    }
    return nil, err
}

[github-actions]

MEDIUM SEVERITY: TOCTOU vulnerability - file content may change between hash and upload

The file is hashed at line 115, but then re-opened here at line 217 for upload. There's no verification that the file content still matches the hash computed earlier. Between the hash computation and re-open, the file could be modified, deleted, or replaced on disk.

The computed hash is stored in the UploadedFile struct but never used to validate the actual bytes being uploaded. This creates a TOCTOU race condition where the uploaded file may not match the validated content.

Suggested fix: Either:

1. Keep the file open and seek back to start for upload (avoids re-open)
2. Verify the file hash after upload matches the expected hash
3. Use the original file handle instead of re-opening

// Option 1: Reuse the already-open file handle
if _, err := file.Seek(0, 0); err != nil {
    return nil, fmt.Errorf("failed to seek file: %w", err)
}
// Then pass 'file' to upload() instead of path

fixed