Skip to content

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

Open
TimeToBuildBob wants to merge 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/export-attachment-filenames
Open

TimeToBuildBob wants to merge 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

  • Current 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.

CI follow-up

  • Pin Ubuntu build/typecheck jobs to ubuntu-24.04: Python 3.9 is unavailable on the Ubuntu 26.04 image now selected by ubuntu-latest.
  • Separate test setup/cleanup requests from assertions to address CodeQL's side-effect finding.
  • New-head CI must finish before this is considered CI-green.

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; those exporter changes may require a small rebase.

Git-Session-Id: c3071fc5-5325-40ad-8ab0-152928f57e79
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

[Low impact] The PR appears safe to merge.

Summary

The PR shares attachment filename formatting across JSON and CSV exports.

  • JSON and CSV exports use safe, readable download names.

No actionable issues were found.

Reviews (1) · Last reviewed commit: "fix(export): serialize WSGI-safe attachm..." · Reviewed by Greptile

Git-Session-Id: 45a70f01-f4f0-5132-ac6f-3e0179dc5b53
@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

The PR changes content_disposition() to serialize attachment headers with Werkzeug, an ASCII filename fallback, and a UTF-8 filename* parameter for Unicode names. Both JSON export routes now call the helper already used by CSV export. Tests add attachment filename cases across JSON and CSV, and the build workflow pins Ubuntu jobs to ubuntu-24.04.

Safe to merge — no P0/P1 findings

Confidence 5/5

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

Files changed (4) — the diff as I read it
  • .github/workflows/build.yml — Changes the build and typecheck Ubuntu matrix entries from ubuntu-latest to ubuntu-24.04.
  • aw_server/csv_export.py — Changes content_disposition() to generate an ASCII fallback and optional percent-encoded UTF-8 filename* parameter using dump_options_header().
  • aw_server/rest.py — Replaces inline Content-Disposition formatting with content_disposition() in ExportAllResource.get() and BucketExportResource.get().
  • tests/test_server.py — Adds JSON and CSV attachment filename cases, checks header encoding and exported payloads, and changes the existing CSV filename assertion to parse_options_header().

Reviewed 7a5499458a60 · panel: codex:gpt-6.1-sol · subscription engine · 26s · about this reviewer

Maintainer commands

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

Outage coverage: 1-pass, not merge evidence. Subscription review supplied findings while full consensus capacity was unavailable. The consensus merge gate remains unsatisfied.

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

2 participants