V0.1.1 release, close to actual release. Bug & security fixes/improvements.

This commit is contained in:
unknown
2026-05-24 19:29:41 +02:00
parent a7b44af91a
commit b004e15948
38 changed files with 3145 additions and 137 deletions

175
agent2_batch4.md Normal file
View File

@@ -0,0 +1,175 @@
# Batch 4 — Telegram Bot + Permissions Verification
**File inspected:** `crates/cgcx-bot/src/main.rs` (2318 lines)
**Supporting files:** `crates/cgcx-crypto/src/lib.rs`, `crates/cgcx-db/src/repos.rs`, `migrations/003_forward_system.sql`
---
## 1. `finalize_upload` — Submission → Review Group
**Location:** `main.rs` lines ~15041595
| Requirement | Status | Evidence |
|-------------|--------|----------|
| Up to 10 items per batch | ✅ PASS | `let chunks: Vec<_> = decrypted.chunks(10).collect();` (line 1541) |
| Review text on LAST batch only | ✅ PASS | `if is_last { if let Some(last) = batch.last_mut() { ... set caption ... } }` (lines 15461559) |
| Action button message sent separately BEFORE media | ✅ PASS | `bot.send_message(ChatId(review_group_id), review_text.clone()).reply_markup(keyboard)` is called at line 15291533, **before** any media batches. `set_review_message_id` stores the returned message id at line 1535. |
### Notes
- The review group text message carries the action buttons (Approve / Ignore / Blacklist / Ban / Ban+BL).
- Media batches are sent **after** the action message, so moderators can act immediately even while media is still uploading.
- Caption is attached to the last media batch item, not the action message.
---
## 2. `handle_forward_callback` — "approve" Action
**Location:** `main.rs` lines ~19402040
| Requirement | Status | Evidence |
|-------------|--------|----------|
| Up to 10 items per batch | ✅ PASS | `let chunks: Vec<_> = decrypted.chunks(10).collect();` (line 1978) |
| Caption on last batch with custom msg + author + direct link + forward link | ✅ PASS | `caption` built at lines 19661970 containing `forward_def.forward_message`, `author_line`, `link`, `forward_link`. Applied to last batch at lines 19892002. |
| Text-only fallback if no files | ✅ PASS | `if files.is_empty()` sends text-only message at line 1972. Additional fallback `if decrypted.is_empty()` at line 1982. |
### Notes
- `author_line` respects `content.show_author` (lines 19481959).
- The `posted_link` is constructed from the first message of the media group or the text message, and is DMd to the submitter (line 2020).
---
## 3. Telegram API Issues
### 3.1 Videos sent as `InputMedia::Document` instead of `InputMedia::Video`
**Status:** ⚠️ ISSUE FOUND
**Evidence:**
- `main.rs` line 15551560 (review batching):
```rust
let media = if mime_type.starts_with("image/") {
InputMedia::Photo(InputMediaPhoto::new(input_file))
} else {
InputMedia::Document(InputMediaDocument::new(input_file))
};
```
- Same pattern in approve batching at lines 19851990.
**Impact:** Videos (`video/mp4`, etc.) and audio files are sent as generic documents. In Telegram clients this means:
- No inline video player in the chat.
- No audio waveform or music player UI.
- File appears as an attachment rather than native media.
**Fix needed:** Add `mime_type.starts_with("video/")` → `InputMedia::Video(...)` and `mime_type.starts_with("audio/")` → `InputMedia::Audio(...)`. Requires importing `InputMediaVideo`, `InputMediaAudio` from teloxide.
---
### 3.2 Audio files sent as documents
**Status:** ⚠️ ISSUE FOUND (same root cause as 3.1)
Audio (`audio/mpeg`, etc.) falls through to `InputMedia::Document`.
---
### 3.3 Caption length exceeding Telegram's 1024-character limit
**Status:** ⚠️ POTENTIAL ISSUE
**Evidence:**
- Caption built in approve path (lines 19661970):
```rust
let caption = format!(
"{}\n\nSubmitted by: {}\nDirect link: <code>{}</code>\nForward link: <code>{}</code>",
escape_html(&forward_def.forward_message),
author_line,
link,
forward_link
);
```
- `forward_def.forward_message` is admin-controlled and has no length validation.
- Direct link + forward link are each ~60100 chars.
- Author line is moderate.
- Total could exceed 1024 chars if the admin sets a long forward message.
**Impact:** `send_media_group` will fail with a Telegram API error if caption > 1024 characters.
**Fix needed:** Truncate `forward_def.forward_message` or the final caption to ensure it stays under 1024 characters (e.g., `caption.chars().take(1024).collect()` or pre-truncate the message component).
---
### 3.4 `decrypt_bytes` loads entire files into memory
**Status:** ⚠️ ISSUE FOUND
**Evidence:**
- `cgcx-crypto/src/lib.rs` lines 102126: `decrypt_bytes` takes `&[u8]` and returns `Vec<u8>`, accumulating all plaintext in a single `Vec`.
- In `main.rs` (lines 15381545 and 19751981):
```rust
match tokio::fs::read(&file.stored_path).await { // entire ciphertext in memory
Ok(ciphertext) => {
match cgcx_crypto::decrypt_bytes(&ciphertext, ...) { // entire plaintext in memory
Ok(bytes) => decrypted.push((file.mime_type.clone(), bytes)),
}
}
}
```
- Then `InputFile::memory(bytes.clone())` clones the bytes **again** for teloxide.
**Impact:**
- For a single large file (e.g., 500 MB video), the bot will hold:
- Ciphertext (~500 MB)
- Plaintext (~500 MB)
- Teloxide clone (~500 MB)
- Total ~1.5 GB for one file.
- In a 10-item batch, this scales linearly.
- High risk of OOM on constrained deployments.
**Fix needed:** Use streaming decryption and `InputFile::file(path)` or a temporary file approach so the bytes are not fully materialized in RAM. This is a larger architectural change.
---
## 4. `review_message_id` Storage & Editing
**Status:** ✅ PASS
**Evidence:**
### Storage
- `main.rs` line 1535:
```rust
forward_repo.set_review_message_id(submission_id, sent.id.0).await?;
```
- `ForwardRepo::set_review_message_id` in `crates/cgcx-db/src/repos.rs` line 679683:
```rust
pub async fn set_review_message_id(&self, id: i64, message_id: i32) -> Result<()> {
conn.execute(
"UPDATE forward_submissions SET review_message_id = ?1 WHERE id = ?2",
params![message_id, id],
)...
}
```
- Schema in `migrations/003_forward_system.sql`:
```sql
review_message_id INTEGER,
```
### Editing on resolution
| Action | Line | Edit behavior |
|--------|------|---------------|
| **approve** | ~2026 | `edit_message_text` → `<b>[ APPROVED ]</b> #{id}\nApproved by <code>{user_id}</code>`, keyboard cleared |
| **ignore** | ~2054 | `edit_message_text` → `<b>[ IGNORED ]</b> ...`, keyboard cleared |
| **blacklist (blk)** | ~2064 | `edit_message_text` → `<b>[ BLACKLISTED ]</b> ...`, keyboard cleared |
| **ban** | ~2073 | `edit_message_text` → `<b>[ BANNED ]</b> ...`, keyboard cleared |
| **banblk** | ~2094 | `edit_message_text` → `<b>[ BAN/BL ]</b> ...`, keyboard cleared |
All paths use `submission.review_message_id` (retrieved from DB) and call `edit_message_text` with an empty keyboard, preventing further interaction.
---
## Summary of Fixes Needed
| # | Issue | Severity | Suggested Fix |
|---|-------|----------|---------------|
| 1 | Videos sent as `InputMedia::Document` | Medium | Add `mime_type.starts_with("video/")` branch using `InputMediaVideo` |
| 2 | Audio sent as `InputMedia::Document` | Medium | Add `mime_type.starts_with("audio/")` branch using `InputMediaAudio` |
| 3 | Caption may exceed 1024 chars | Medium | Truncate caption to 1024 chars before sending |
| 4 | `decrypt_bytes` + `InputFile::memory` loads entire files into RAM | High (OOM risk) | Implement streaming file decryption or write decrypted data to temp files and use `InputFile::file` |
**No fixes needed for:** batch size logic, caption placement, action button ordering, text-only fallback, or `review_message_id` lifecycle.