Skip to content

fix(research): re-anchor the export sanitizer after aw-server-rust#677 streaming exports - #1449

Merged
ErikBjare merged 1 commit into
masterfrom
fix/research-export-patch-after-677
Sep 17, 2026
Merged

ErikBjare merged 1 commit into
masterfrom
fix/research-export-patch-after-677

Conversation

@ErikBjare

Copy link
Copy Markdown
Member

Fixes the two red Research Edition legs on master since the v0.14.0 submodule bump (aac77dc):

Patch research edition export hostname sanitizer
  export.rs: expected exactly one export-sanitizer insertion point, found 0

Cause

The bump pulled in ActivityWatch/aw-server-rust#677 (bounded-buffer streaming exports). Both export endpoints are now one-line calls to BucketsExportRocket::new, which spools the JSON via aw_datastore::export_to_file; no whole BucketsExport value exists at the endpoints any more, so both needles the patcher looked for (export.rs, bucket.rs) are gone. Reproduced locally by running the patcher against b0fab73.

This surfaced at the bump rather than when #677 merged because the research patch is only ever exercised against the bundle's pinned pointer — exactly the gap #1448 (nightly build against submodule masters) closes.

Fix

The sanitizer needs the whole export (fail-closed on unfiltered events; identity rewriting with cross-bucket collision detection), so it is spliced into BucketsExportRocket::new — the single point both endpoints share: re-read the spooled JSON, sanitize_buckets_export, spool again, rewind. Trade-off stated in the code: this relaxes #677's memory bound only for research builds, whose exports are category-only and small. The patcher no longer touches export.rs/bucket.rs, so endpoint churn can't break it again; a missing call site still fails closed.

Verified

  • pytest scripts/tests/test_patch_research_edition_export.py — 7 pass (fixture is now the post-#677 util.rs; adds a fail-closed test for a pre-#677 tree).
  • Patch applied to the real aw-server-rust submodule at b0fab73; cargo check -p aw-server compiles the inserted code. Submodule reverted afterwards.

…7 streaming exports

Both Research Edition legs fail on master since the v0.14.0 submodule bump
(aac77dc) at "Patch research edition export hostname sanitizer":

  export.rs: expected exactly one export-sanitizer insertion point, found 0

aw-server-rust#677 (perf(export): serialize HTTP exports with bounded event
buffering) rewrote the export path: `/api/0/export` and
`/api/0/buckets/<id>/export` are now one-line calls to
`BucketsExportRocket::new`, which spools the JSON through
`aw_datastore::export_to_file`. No whole `BucketsExport` value exists at the
endpoints any more, so the old needles in export.rs and bucket.rs are gone.

The sanitizer needs the whole export (fail closed on unfiltered events,
identity rewriting with collision detection across buckets), so it is now
spliced into `BucketsExportRocket::new` instead: re-read the spooled JSON,
sanitize, spool again. That is one insertion point for both endpoints, and
it relaxes #677's memory bound only for research builds, whose exports are
category-only and small. Fails closed as before if the call site drifts.

Verified: patcher tests pass; patch applied to aw-server-rust b0fab73 and
`cargo check -p aw-server` compiles the inserted code.
@ErikBjare

Copy link
Copy Markdown
Member Author

@greptileai review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-17T15:15:52.738180Z 889dbc5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR appears safe to merge, with a non-blocking scalability concern for large Research Edition exports.

Findings

  1. P2 Unbounded export memory usage ▶

Summary

This PR relocates the Research Edition export sanitizer from two obsolete endpoint-level insertion points to the shared post-streaming BucketsExportRocket::new path.

  • Re-reads the generated export, applies fail-closed sanitization, and serves a rewound sanitized tempfile.
  • Updates patch fixtures and failure tests for the post-aw-server-rust#677 source layout.
  • Retains fail-closed behavior when the expected shared insertion point is absent.
  • Introduces a non-blocking scalability concern by materializing the complete export in memory.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Request[Research export request] --> Stream[export_to_file streams raw JSON]
  Stream --> Raw[(Raw tempfile)]
  Raw --> Deserialize[Deserialize complete BucketsExport]
  Deserialize --> Sanitize[Validate and rewrite identities]
  Sanitize --> Serialize[Serialize sanitized JSON]
  Serialize --> Clean[(Sanitized tempfile)]
  Clean --> Rewind[Rewind]
  Rewind --> Response[HTTP export response]
Loading

Reviews (1) · Last reviewed commit: "fix(research): re-anchor the export sani..."

Comment thread scripts/patch_research_edition_export.py
@ErikBjare
ErikBjare merged commit d1f0b4a into master Sep 17, 2026
21 checks passed
ErikBjare added a commit that referenced this pull request Sep 26, 2026
…xport

aw-server-rust#721 moved JSON export serialization onto a background
thread that runs after the 200 headers, so the sanitizer anchor from
#1449 no longer exists (the research legs failed closed on it), and a
sanitizer failure there could no longer become an error response.
aw-server-rust#722 added a raw-event CSV export that no sanitizer covers.

In Research Edition builds (approved by Erik, options A + B):
- A: BucketsExportRocket::new exports, sanitizes and re-spools to a
  tempfile before any headers, so a refused export is still a
  fail-closed 409; respond_to only streams the sanitized spool.
- B: BucketEventsCsvRocket::new returns 403 "CSV export is disabled in
  Research Edition".

The patcher checks every anchor (export struct, export impl, CSV, mod.rs)
for exactly one match before writing anything, so a missing or duplicated
anchor fails the build without leaving a half-patched tree. Tempfiles are
unnamed (tempfile::tempfile), so they are removed on every path.

Verified on a patched aw-server-rust e7c5439 release build: a clean
currentwindow bucket exports with 200 and the hostname rewritten to
research-participant (the real one appears nowhere), an event with a
url makes /api/0/export and /api/0/buckets/<id>/export return 409, and
/api/0/buckets/<id>/export/csv returns 403.
ErikBjare added a commit that referenced this pull request Sep 26, 2026
)

* build(deps): bump aw-core and aw-server-rust for the parity fixes

aw-core 33c8528 -> 99cb7ac (#161, #167, #168, #169, #159) and aw-server-rust
c3baa9c -> e7c5439 (#744, #748, #749, #750, #753). 433 known failures now
pass, none are new.

The remaining 532 are re-attributed with attribute.py from runs at the
new pins: base, aw-core#165 alone, aw-core#166 alone, and both (with two
open fixes, each leave-one-out run is the other fix alone). 81 are fixed
by those PRs (CORE_165: 22, CORE_166: 59); the other 451 are the #1466
output shapes (SHAPE) and 1 ms timestamp resolution (PRECISION).

* build(deps): bump aw-core to 7b24386 and aw-tauri to ec46d16

- aw-core 99cb7ac -> 7b24386: #171 fixes the aw-core test failure that broke
  the Qt legs (#159 + #169 disagreed on iter_events order), plus #166
  and #170.
- aw-tauri a2f72df -> ec46d16: ActivityWatch/aw-tauri#271 locks
  aw-server-rust at e7c5439, matching the release submodule, which
  scripts/check_tauri_server.py requires (the Tauri legs failed on it).

#166 landing fixes the 59 CORE_166 known failures. Re-attributed from
runs at the new pins (base and aw-core#165): 473 remain, 22 CORE_165,
the rest SHAPE (#1466) and PRECISION.

* build(deps): bump aw-core to d6ec34c (aw-core#165)

The 22 CORE_165 known failures pass now and none are new. 451 remain,
all SHAPE (#1466) and PRECISION: no open parity fixes left.

* fix(research): sanitize JSON exports before headers and disable CSV export

aw-server-rust#721 moved JSON export serialization onto a background
thread that runs after the 200 headers, so the sanitizer anchor from
#1449 no longer exists (the research legs failed closed on it), and a
sanitizer failure there could no longer become an error response.
aw-server-rust#722 added a raw-event CSV export that no sanitizer covers.

In Research Edition builds (approved by Erik, options A + B):
- A: BucketsExportRocket::new exports, sanitizes and re-spools to a
  tempfile before any headers, so a refused export is still a
  fail-closed 409; respond_to only streams the sanitized spool.
- B: BucketEventsCsvRocket::new returns 403 "CSV export is disabled in
  Research Edition".

The patcher checks every anchor (export struct, export impl, CSV, mod.rs)
for exactly one match before writing anything, so a missing or duplicated
anchor fails the build without leaving a half-patched tree. Tempfiles are
unnamed (tempfile::tempfile), so they are removed on every path.

Verified on a patched aw-server-rust e7c5439 release build: a clean
currentwindow bucket exports with 200 and the hostname rewritten to
research-participant (the real one appears nowhere), an event with a
url makes /api/0/export and /api/0/buckets/<id>/export return 409, and
/api/0/buckets/<id>/export/csv returns 403.

* fix(research): assert the research export behaviour in upstream's tests

The research legs run aw-server-rust's full test suite against the patched
server, and two tests new since the bump assert the standard behaviour:
aw-server's CSV export test (now 403 by design) and aw-client-rust's
export/import round trip (#727; research exports rewrite the bucket id and
hostname). The patcher now rewrites both in research builds to assert the
research behaviour instead: CSV export is 403 for existing and missing
buckets, and exports contain no real hostname or bucket id. Same
exactly-one-anchor, all-or-nothing rules as the server edits.

`cargo test --no-fail-fast` on a patched aw-server-rust e7c5439: all pass.
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.

1 participant