Skip to content

fix: only send upload notification emails for valid addresses and real logs - #346

Merged
mrpollo merged 1 commit into
mainfrom
fix-upload-email-validation
Aug 17, 2026
Merged

mrpollo merged 1 commit into
mainfrom
fix-upload-email-validation

Conversation

@mrpollo

@mrpollo mrpollo commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

The upload handler sends the notification email for any file that passes the 7-byte ULog magic check, and CI uploads skip parsing entirely. That means a 16-byte header plus an email address is enough to have the server relay arbitrary text (the description and feedback fields) from our SMTP sender to any recipient, and bots have been using the public form for exactly that.

This adds two helpers in plot_app/helper.py as the single source of truth for what the app considers valid, so the logic isn't repeated in handlers:

  • is_valid_email(): non-empty, one @, no whitespace, dotted domain, at most 254 chars, full match (so a trailing newline or \r\nBcc: is rejected)
  • is_valid_ulog(): a parsed object with at least one logged data message (len(ulog.data_list) > 0)

send_notification_email() now uses is_valid_email() instead of the bare == '' check, and the upload handler only calls it when source != 'CI' and both helpers pass. CI uploads therefore never send mail (they still skip parsing as before), and header-only, garbage-after-header, or definitions-only files never trigger one. Legit uploads with a real log and a proper address behave exactly as today.

Tested end to end on a scratch copy with SMTP pointed at a dead host and counting send attempts: on main, header-only + email, header-only + email + source=CI, real log + source=CI, and a header-injection address each produced one send; on this branch all of those produce zero, while a real log with a valid address still produces one. Corrupt files keep returning the existing 400. Pylint is clean on the touched files.

…l logs

The upload handler sent the notification email for any file that passed
the 7-byte ULog magic check, and CI uploads skipped parsing entirely, so
a 16-byte header plus an email address was enough to have the server
relay arbitrary text (description/feedback) to arbitrary recipients.
Bots have been using this against the public form.

Add two helpers in plot_app/helper.py as the single source of truth for
what the app considers valid: is_valid_email() (non-empty, one '@', no
whitespace, dotted domain, <=254 chars, full match) and is_valid_ulog()
(parsed object with at least one logged data message). Use them in
send_notification_email() and gate the send in the upload handler on
source != 'CI' and both helpers, so CI uploads never send mail and empty
or header-only files never trigger one.
@mrpollo
mrpollo merged commit ea5c16a into main Aug 17, 2026
5 checks passed
@mrpollo
mrpollo deleted the fix-upload-email-validation branch August 17, 2026 18:34
@mrpollo

mrpollo commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Merging because we have an issue with both bots and users abusing the form, either by entering bad emails on purpose to hide their email, or by bots using it to send SPAM

dakejahl added a commit to ARK-Electronics/flight_review that referenced this pull request Oct 1, 2026
Brings in PX4/flight_review through PX4#346: log load timeout, connected devices table, estimator plot fix for logs without innovation_check_flags, median FFT sampling rate, and the notification email validation.
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