Skip to content

Add a separate route for submitting file upload answers - #2352

Merged
stephencdaly merged 1 commit into
mainfrom
add-separate-route-for-submitting-file-upload-answers
Sep 22, 2026
Merged

stephencdaly merged 1 commit into
mainfrom
add-separate-route-for-submitting-file-upload-answers

Conversation

@stephencdaly

@stephencdaly stephencdaly commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

What problem does this pull request solve?

We need to be able to exempy file upload requests from particular WAF rules:

  • the file size rule so requests can include files >8KB in size
  • the XSS rule, so that files that contain XML, such as PDFs are not blocked.

So that we don't need to allow these rules for submitting all answer types, make it so that file upload requests are submitted to a dedicated route.

For now, this still allows file uploads to be submitted to the standard save question route so we don't cause issues for users currently filling out forms, but this will be changed in a separate deployment.

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Do the end to end tests need updating before these changes will pass?
  • Has all relevant documentation been updated?

@stephencdaly
stephencdaly force-pushed the add-separate-route-for-submitting-file-upload-answers branch from 7ba4349 to 37aa920 Compare September 22, 2026 08:45
@stephencdaly stephencdaly changed the title Add a separate route for submitting file upload answers [DO NOT MERGE]Add a separate route for submitting file upload answers Sep 22, 2026
@stephencdaly
stephencdaly force-pushed the add-separate-route-for-submitting-file-upload-answers branch 2 times, most recently from 832fa7d to 2657d61 Compare September 22, 2026 10:59
@stephencdaly stephencdaly changed the title [DO NOT MERGE]Add a separate route for submitting file upload answers Add a separate route for submitting file upload answers Sep 22, 2026
@stephencdaly
stephencdaly marked this pull request as ready for review September 22, 2026 12:40
@theseanything
theseanything requested a balanced review from Copilot September 22, 2026 13:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The key generated form-action behavior lacks end-to-end request coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds a dedicated submission route for file-upload answers so WAF exemptions can be scoped appropriately.

Changes:

  • Adds the upload route and controller action.
  • Routes file-question forms to the new endpoint.
  • Adds request coverage and corrects a spec description typo.
File Description
config/​routes.rb Defines the file-upload POST route.
app/​controllers/​forms/​step_controller.rb Selects the upload route and shares save logic.
app/​models/​step.rb Adds file-question detection.
spec/​requests/​forms/​step_controller_spec.rb Tests the new endpoint’s behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/controllers/forms/step_controller.rb
We need to be able to exempy file upload requests from particular
WAF rules:
- the file size rule so requests can include files >8KB in size
- the XSS rule, so that files that contain XML, such as PDFs are not
blocked.

So that we don't need to allow these rules for submitting all answer
types, make it so that file upload requests are submitted to a
dedicated route.

For now, this still allows file uploads to be submitted to the
standard save question route so we don't cause issues for users
currently filling out forms, but this will be changed in a separate
deployment.

Co-Authored-By: Claude
@stephencdaly
stephencdaly force-pushed the add-separate-route-for-submitting-file-upload-answers branch from 2657d61 to 8c42da6 Compare September 22, 2026 13:19
@github-actions

Copy link
Copy Markdown
Contributor

🎉 A review copy of this PR has been deployed! It is made of up two components

  1. A review copy of forms-runner
  2. A production copy of forms-admin

It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready
after 5 minutes, there may be something wrong with the ECS task. You will need to go to the integration AWS account
to debug, or otherwise ask an infrastructure person.

For the sign in details and more information, see the review apps wiki page.

@stephencdaly
stephencdaly added this pull request to the merge queue Sep 22, 2026
Merged via the queue into main with commit 3f1f8f0 Sep 22, 2026
3 checks passed
@stephencdaly
stephencdaly deleted the add-separate-route-for-submitting-file-upload-answers branch September 22, 2026 14:31
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.

3 participants