Skip to content

fix(export): serialize WSGI-safe attachment filenames - #212

Merged
ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/export-attachment-filenames
Nov 7, 2026
Merged

ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/export-attachment-filenames

Conversation

@TimeToBuildBob

@TimeToBuildBob TimeToBuildBob commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Reuse the existing attachment helper across both JSON routes and CSV; sanitize controls and unsafe delimiters consistently.
  • Use Werkzeug header serialization, an ASCII fallback and RFC 5987 filename* for Unicode. Preserve ordinary filenames, extensions and JSON/CSV payloads.
  • Add 25 complete-response cases covering plain IDs, delimiters, quotes, backslashes, CR/LF/tab/controls, spaces, Latin-1, non-Latin-1 and percent signs. Assert parsed filename semantics, fallback/Unicode behavior and WSGI Latin-1 header encoding.

Closes #210

Reproduction and validation

  • Original master 994ea988: work;secondary JSON parses with filename aw-bucket-export_work (lost extension); a newline-containing ID returns HTTP 500.
  • New tests before repair: 12 failed, 13 passed. After repair: 25 passed.
  • Full Makefile test selection after CI follow-up: 169 passed locally (benchmarks disabled, no live listener).
  • Typecheck passed for 31 source files; Ruff and Black checks passed. This repository has no make test-fast target; repository test selection used instead.
  • Filing-time desired-contract probe passes against the repaired worktree and freshly archived core master.

Final diff and CI

Rebased onto master f2aec97 before merge. Master already contained the Ubuntu 24.04 runner pin; the final PR contains no workflow changes. It changes only aw_server/csv_export.py, aw_server/rest.py and tests/test_server.py. Test setup/cleanup requests are separate from assertions to address CodeQL's side-effect finding.

All nine reported checks passed on final head a4e770e01bea410582ca16b24f86ed7bcfb05cb2, including Linux/Windows/macOS tests, typecheck, lint, format, CodeQL and Greptile (5/5). Merged 2026-11-07.

No browser-download, released-build or header-injection exploit claim. No changes to accepted bucket IDs, event data or streaming. Retains the existing helper rather than introducing a second serializer or custom HTTP quoting library. Independent of #173/#200/#208.

Comment thread tests/test_server.py Fixed
@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues were found.

Summary

The PR makes attachment filenames safe for WSGI across JSON and CSV exports.

  • JSON and CSV exports now share WSGI-safe attachment filenames.

No actionable issues were found. Tests were inspected, not run.

Reviews (3) · Last reviewed commit: "fix(ci): pin Python 3.9 runners and avoi..." · Reviewed by Greptile

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@TimeToBuildBob

TimeToBuildBob commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

Updates content_disposition() in aw_server/csv_export.py to serialize attachment headers with Werkzeug, an ASCII fallback filename, and an RFC 5987 filename* parameter for Unicode names. Both JSON export routes now use this helper alongside CSV export. Updates the existing CSV header assertion and adds attachment filename checks for JSON and CSV exports.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings. The diff looks correct to me on this pass.

Files changed (3) — the diff as I read it
  • aw_server/csv_export.py — Adds quote and dump_options_header, and changes content_disposition() to emit an ASCII filename fallback plus a percent-encoded Unicode parameter.
  • aw_server/rest.py — Replaces inline Content-Disposition construction in ExportAllResource.get() and BucketExportResource.get() with calls to content_disposition().
  • tests/test_server.py — Parses the existing CSV attachment assertion and adds 24 parametrized filename cases plus an all-buckets export header check.
Previous review passes
commit score findings engine when
7a5499458a60 5/5 0 subscription 2026-10-28 23:42 UTC

Reviewed a4e770e01bea · panel: codex:gpt-6.1-sol + claude-code:sonnet · subscription engine · 22s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

Git-Session-Id: c3071fc5-5325-40ad-8ab0-152928f57e79
Git-Session-Id: 45a70f01-f4f0-5132-ac6f-3e0179dc5b53
@TimeToBuildBob
TimeToBuildBob force-pushed the fix/export-attachment-filenames branch from 7a54994 to a4e770e Compare November 6, 2026 15:37
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (f2aec97) and pushed a4e770e with an explicit force-with-lease. The conflict was only in .github/workflows/build.yml: master already has the Ubuntu 24.04 pin, so I retained its workflow verbatim, including checkout/setup-python v7. The filename implementation and all regression tests are preserved; the PR now contains no workflow diff.

Verification: 169 tests passed from the Makefile selection (benchmarks disabled, isolated test data), mypy passed for 31 source files, Black and the standard Ruff E/F checks passed. Tests used Peewee 3.17.0, matching poetry.lock; an initial unlocked install selected Peewee 4 and caused three unrelated database-fixture errors before that correction. GitHub now reports the PR mergeable. New-head CI is running; the earlier green checks and reviews apply to the old head, not this rebase.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@ErikBjare
ErikBjare merged commit b298f74 into ActivityWatch:master Nov 7, 2026
9 checks passed
TimeToBuildBob added a commit to TimeToBuildBob/aw-server that referenced this pull request Nov 11, 2026
)

* fix(export): serialize WSGI-safe attachment filenames

Git-Session-Id: c3071fc5-5325-40ad-8ab0-152928f57e79

* fix(ci): pin Python 3.9 runners and avoid assert side effects

Git-Session-Id: 45a70f01-f4f0-5132-ac6f-3e0179dc5b53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON export attachment filenames break on accepted bucket IDs

3 participants