Skip to content

AO3-7461 Change send_data to a background job powered workflow - #5950

Open
y7nieSEl5 wants to merge 12 commits into
otwcode:masterfrom
y7nieSEl5:AO3-7461
Open

AO3-7461 Change send_data to a background job powered workflow#5950
y7nieSEl5 wants to merge 12 commits into
otwcode:masterfrom
y7nieSEl5:AO3-7461

Conversation

@y7nieSEl5

@y7nieSEl5 y7nieSEl5 commented Jul 27, 2026

Copy link
Copy Markdown

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-7461

Purpose

This PR moves large work and CSV download generation out of web requests and into background jobs.

It updates the following download paths:

  • Work downloads
  • Challenge signup CSVs
  • Tag wrangler reports
  • Admin bulk-user-search CSVs

When a download is requested, the controller creates a tokenized download record, queues a background job, and redirects to a status page. The job generates the file and uploads it through Active Storage. Once ready, the status page redirects the browser to the storage service URL, allowing S3 to serve the file instead of Puma.

CSV files are written incrementally to a tempfile in the worker while preserving the existing UTF-16LE, tab-separated Excel-compatible format.

Generated files expire after 24 hours. An hourly scheduled cleanup removes expired files and database records.

Testing Instructions

Run the relevant automated tests:

docker compose run --rm test bundle exec rspec \
  spec/models/generated_download_spec.rb \
  spec/controllers/generated_downloads_controller_spec.rb \
  spec/controllers/tag_wranglers_controller_spec.rb \
  spec/controllers/challenge_signups_controller_spec.rb

All tests should pass.
Manually test each download type:
Request a work download in the supported formats.
Request a challenge signup CSV.
Request a tag wrangler report.
Request an admin bulk-user-search CSV.
Confirm each request redirects promptly to a processing page.
Confirm the page automatically initiates the download when the background job completes.
Confirm CSV files open correctly and contain the expected columns and data.
Confirm the completed file is served by the configured Active Storage service rather than through the Rails web response.
Confirm failed generation displays an error page.
Confirm expired downloads return HTTP 410 and are removed by GeneratedDownload.cleanup.

Credit

What name and pronouns should we use to credit you in the Archive of Our Own's Release Notes?

y7nieSEl5, she/her

- Work downloads
- Challenge signup CSVs
- Tag wrangler reports
- Admin bulk-user-search CSVs
The new flow creates a tokenized download record, queues `GeneratedDownloadJob`, redirects to a status page, uploads the completed file through Active Storage, and redirects the browser directly to the storage URL when ready. Downloads expire after 24 hours and an hourly cleanup removes expired files and records.
CSV output is written incrementally to a tempfile in the worker, preserving the existing UTF-16LE Excel-compatible format.
…ation; added regression test for non-empty arguments
Comment thread app/jobs/generated_download_job.rb Fixed
Comment thread app/controllers/generated_downloads_controller.rb Fixed
@github-actions github-actions Bot added Has Migrations Contains migrations and therefore needs special attention when deploying Awaiting Review labels Jul 27, 2026
Comment thread app/jobs/generated_download_job.rb Fixed
@y7nieSEl5 y7nieSEl5 closed this Jul 28, 2026
@y7nieSEl5 y7nieSEl5 reopened this Jul 28, 2026
head :gone
elsif @generated_download.status == "ready" && @generated_download.file.attached?
blob = @generated_download.file.blob
redirect_to blob.service.url(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redirect target is not from request input but generated by the configured Active Storage service from a server-side blob key. Cross-host redirection is intentional so S3 serves the attachment. The generated URL is expiring and uses attachment disposition.

raise "Download generation failed" unless download.exists?

file_path = File.join(download.dir, File.basename(download.file_path))
File.open(file_path, "rb") do |file|

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the path is built inside a per-download temp directory, the filename is sanitized by Download#clean, and File.basename prevents traversal.

@y7nieSEl5

Copy link
Copy Markdown
Author

The redirect target is not taken from user input. The URL is generated by Active Storage from a server-side blob key blob.service.url(...). The redirect is intentionally cross-host because the file is supposed to be served by S3/storage, not by Puma. The URL is expiring, and the blob metadata comes from a blob we created ourselves in the background job.

The bot probably flagged it because static analyzers treat redirect_to ... allow_other_host: true as suspicious.

The built-in storage redirect route changes HTML downloads to application/octet-stream. That is intentional Rails behavior to avoid serving HTML through a redirect path that could be interpreted inline. In this codebase, that breaks existing file-type expectations for downloaded work files.

Therefore, keeping the Active Storage redirect route would satisfy the security scanner more easily but regress MIME behavior, while keeping the current direct service URL preserves the MIME behavior and is safe in the intended sense, though it will continue to trip the scanner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Awaiting Review Has Migrations Contains migrations and therefore needs special attention when deploying

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants