feat(server): add streaming CSV export endpoint for bucket events - #722
TimeToBuildBob wants to merge 4 commits into
Conversation
GET /api/0/buckets/{id}/export/csv streams events as CSV, sending HTTP
headers before serialization so large buckets don't look like a hung
connection on Android WebView or slow networks.
Mirrors the existing streaming JSON export (BucketsExportRocket / ActivityWatch#721):
OS pipe + background thread serializes into a tempfile, then copies to
the client. The route is at /export/csv (parallel to /export for JSON)
to avoid any Rocket route collision with the single-event endpoint.
RFC-4180 CSV: id, timestamp, duration (fractional seconds), then all
data-map keys from the first event. Fields containing commas, quotes, or
newlines are double-quoted and internal quotes are doubled.
Test: csv_export_returns_csv_with_correct_headers_and_missing_bucket_errors
Git-Session-Id: cdc6
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #722 +/- ##
==========================================
+ Coverage 70.81% 79.74% +8.92%
==========================================
Files 51 75 +24
Lines 2916 8628 +5712
==========================================
+ Hits 2065 6880 +4815
- Misses 851 1748 +897 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address Greptile P1s on ActivityWatch#722: - Write CSV from SQL row-by-row on the datastore worker (no full Vec + String) - Format duration from nanoseconds so 0.0015s is not truncated to 0.001 - Neutralize spreadsheet formula prefixes (=, +, -, @) - Preflight LIMIT 1 before 200 so worker/SQL failures still return JSON Git-Session-Id: 79e64905-13b9-5822-b880-3863140ba2d7
|
@greptileai review |
The ExportEventsCsv worker command blocked the single shared datastore worker for the entire duration of the export — heartbeats and all other requests queued until serialization finished. Fix: remove ExportEventsCsv from the worker. The background thread now calls get_events() (worker holds the DB lock only for the SQL read), then serializes CSV via write_csv_from_events() off-worker. The worker is free for other requests during the write. write_csv_from_events() is the Vec<Event>-based counterpart to the connection-based write_events_csv(); both produce identical RFC-4180 output with ns-precision durations and formula neutralization. Git-Session-Id: bb28
|
@greptileai review |
BufWriter::drop swallows a final flush failure, so a full staging filesystem still rewound and copied a truncated CSV under 200 OK. Keep the writer in a local, flush explicitly, and flush inside write_csv_from_events / write_events_csv so the error propagates. Git-Session-Id: 01a0ce79
|
@greptileai review |
|
Greptile re-review of |
🤖 AI code reviewAdds a streaming CSV export endpoint for a single bucket's events. The new route GET /api/0/buckets/<bucket_id>/export/csv parses start/end/limit query params, validates the bucket and a LIMIT 1 probe before committing headers, then streams CSV from a background thread via a pipe. The datastore gains write_events_csv and write_csv_from_events helpers, CSV escaping with spreadsheet formula neutralization, and tests for duration precision, escaping, flush errors, and row counts. Not safe to merge — 3 P1 openConfidence 2/5 3 findings · ❌ 3 P1 · 🔒 1 security❌ P1 high — The CSV export fetches all events into memory via datastore.get_events before writing any CSV, so the claimed streaming benefit is lost for large buckets. In spawn_csv_export_stream, How this was verified: Checked aw-server/src/endpoints/util.rs lines 177-193: get_events returns Vec<Event>, then write_csv_from_events(&events, ...) serializes the whole slice. The datastore-level write_events_csv (aw-datastore/src/export.rs:237) streams from SQL but is not called from the HTTP path. ❌ P1 high — The CSV column set is derived from the first event's data keys, but later events may have different keys. write_csv_from_events and write_events_csv both collect data_keys from the first event (or first valid event) and then emit empty cells for missing keys in subsequent rows. This is a schema mismatch: if the first event has keys {a,b} and a later event has {a,c}, the CSV has columns id,timestamp,duration,a,b and the later event's c value is silently dropped while its b cell is empty. The webui's client-side CSV used PapaParse on the full JSON array, which would have included all keys across all events. The observable consequence is data loss in the exported CSV for buckets with heterogeneous event data, which is common in ActivityWatch (e.g. app/title keys vary). How this was verified: Checked aw-datastore/src/export.rs lines 221-224 and 306-311: data_keys is set once from the first event and reused for all rows. event_field_value (line 158) returns empty string for missing keys, so extra keys are dropped. ❌ P1 high · 🔒 security — The formula neutralization in neutralize_formula only checks the first character, but spreadsheet formula injection can also occur with leading whitespace before the formula character. For example, a value like " =1+1" (space then equals) is not neutralized because the first char is a space, not one of the listed starters. Excel and LibreOffice trim leading whitespace before evaluating formulas, so a cell containing " =1+1" will still execute as a formula when opened. The current code only handles tab and carriage return as whitespace starters, not space. The observable consequence is that a crafted event data value can bypass the CSV injection protection and execute a formula in a user's spreadsheet, which is a security issue for exported data. How this was verified: Checked neutralize_formula (lines 132-137) and csv_escape (lines 140-147). A string starting with a space is not matched by the first-char match, and csv_escape only quotes if it contains comma, quote, newline, or CR. So " =1+1" passes through unquoted and unneutralized. 1 advisory finding (summary-only, not scored)These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.
The CSV export endpoint's pre-flight check in BucketEventsCsvRocket::new calls datastore.get_events(bucket_id, start, end, Some(1)) to force the same SQL path and catch errors before headers commit. However, this check runs a full query with LIMIT 1, which is not the same as the actual export query when limit is None. The actual export in spawn_csv_export_stream calls get_events with the original limit (which may be None), so a bucket with a corrupt row that appears after the first row will not be caught by the pre-flight check, but will be skipped during the actual export (with a warning) — that is fine. More importantly, the pre-flight check does not validate that the bucket has any events; it only checks that the query succeeds. If the bucket exists but the datastore worker is down, get_bucket would fail first, so that is covered. The real issue is that the pre-flight get_events with Some(1) can return Ok even when the full query would fail due to a SQL error that only manifests with a larger scan (e.g., a corrupt index or a row that fails to parse after the first). But parse errors are skipped, not fatal, so the full export would also succeed with skipped rows. The pre-flight check is redundant and adds an extra round-trip to the worker, but it does not cause a correctness bug. This is a minor performance concern, not a defect. How this was verified: Traced the pre-flight and actual export paths. The pre-flight uses Some(1) while the actual uses the original limit. No distinct failure mode found beyond the extra query. Files changed (8) — the diff as I read it
Reviewed Maintainer commands
|
| // Fetch events via the worker. The worker holds the DB lock only for | ||
| // this SQL read, then is immediately free for heartbeats and other | ||
| // requests while CSV serialization runs here, off-worker. | ||
| let events = match datastore.get_events(&bucket_id, start, end, limit) { |
There was a problem hiding this comment.
❌ P1 — The CSV export fetches all events into memory via datastore.get_events before writing any CSV, so the claimed streaming benefit is lost for large buckets. In spawn_csv_export_stream, let events = match datastore.get_events(...) materializes the entire Vec in RAM, then write_csv_from_events serializes that slice. For the 500k+ event buckets that motivated this PR, this can OOM the server process just like the client-side OOM it was meant to fix. The datastore-level write_events_csv streams row-by-row from SQL, but the HTTP path never uses it; it uses the worker's get_events which returns a full Vec. The observable consequence is a memory spike proportional to bucket size, potentially crashing the server on large exports.
| events: &[aw_models::Event], | ||
| mut writer: impl Write, | ||
| ) -> Result<(), DatastoreError> { | ||
| let data_keys: Vec<String> = events |
There was a problem hiding this comment.
❌ P1 — The CSV column set is derived from the first event's data keys, but later events may have different keys. write_csv_from_events and write_events_csv both collect data_keys from the first event (or first valid event) and then emit empty cells for missing keys in subsequent rows. This is a schema mismatch: if the first event has keys {a,b} and a later event has {a,c}, the CSV has columns id,timestamp,duration,a,b and the later event's c value is silently dropped while its b cell is empty. The webui's client-side CSV used PapaParse on the full JSON array, which would have included all keys across all events. The observable consequence is data loss in the exported CSV for buckets with heterogeneous event data, which is common in ActivityWatch (e.g. app/title keys vary).
| /// Prefix spreadsheet-formula starters so Excel/Sheets will not execute them. | ||
| fn neutralize_formula(s: &str) -> String { | ||
| match s.chars().next() { | ||
| Some('=' | '+' | '-' | '@' | '\t' | '\r') => format!("'{s}"), |
There was a problem hiding this comment.
❌ P1 — The formula neutralization in neutralize_formula only checks the first character, but spreadsheet formula injection can also occur with leading whitespace before the formula character. For example, a value like " =1+1" (space then equals) is not neutralized because the first char is a space, not one of the listed starters. Excel and LibreOffice trim leading whitespace before evaluating formulas, so a cell containing " =1+1" will still execute as a formula when opened. The current code only handles tab and carriage return as whitespace starters, not space. The observable consequence is that a crafted event data value can bypass the CSV injection protection and execute a formula in a user's spreadsheet, which is a security issue for exported data.
1 participant
Summary
Fixes the large bucket CSV export silently failing on Android WebView (and slow networks) by moving CSV generation server-side with immediate header delivery.
Root cause: The existing CSV export in aw-webui fetches all events as JSON into JS memory, converts with PapaParse, then creates a Blob. On Android WebView with 500k+ events this causes OOM or a connection that appears hung until the full response is ready.
Fix: New endpoint
GET /api/0/buckets/{id}/export/csvthat:200 OKwithContent-Type: text/csvandContent-Dispositionheaders before serialization begins (same streaming pattern as fix(export): send HTTP headers before serializing large exports #721 for JSON export)id,timestamp,duration+ all data-map keys from the first eventstart,end,limitquery params as the JSON events endpoint404for missing buckets (before any streaming begins, so errors are well-formed)The route path is
/export/csv(parallel to/exportfor JSON) to avoid any Rocket route collision with/<bucket_id>/events/<event_id>.Changes
aw-server/src/endpoints/util.rs—BucketEventsCsvRocketstruct +Responderimpl;events_to_csv()+csv_escape()helpersaw-server/src/endpoints/bucket.rs—bucket_events_get_csvroute at/<bucket_id>/export/csvaw-server/src/endpoints/mod.rs— register new routeaw-server/tests/api.rs—csv_export_returns_csv_with_correct_headers_and_missing_bucket_errorstestRelated