Skip to content

fix: surface upload validation reason in metadata API errors - #3154

Open
FrankApiyo wants to merge 2 commits into
mainfrom
fix/surface-upload-validation-reason
Open

FrankApiyo wants to merge 2 commits into
mainfrom
fix/surface-upload-validation-reason

Conversation

@FrankApiyo

@FrankApiyo FrankApiyo commented Jun 30, 2026

Copy link
Copy Markdown
Member

Changes / Features implemented

Metadata media-file uploads that fail strict validation returned only a generic "could not be validated" message — the actual reason was logged server-side and discarded. The API error now includes the validation reason, sanitized before it is surfaced.

Steps taken to verify this change does what is intended

  • Unit tests across the upload validation helper, the metadata serializer and the metadata viewset

Side effects of implementing this change

  • None expected: only the error message content changes

Before submitting this PR for review, please make sure you have:

  • Included tests
  • Updated documentation

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Comment thread onadata/libs/serializers/metadata_serializer.py Dismissed
Metadata media-file uploads that failed strict validation returned only
the generic "The uploaded file 'X' could not be validated." message. The
actual reason (e.g. "CSV files must be UTF-8 encoded.") was logged but
discarded, so API clients and the UI had no actionable detail.

generic_upload_validation_error_message now accepts an optional reason
and appends it. The serializer passes the caught UploadValidationError
through. The default (no reason) is unchanged, so other callers and the
generic-message contract are preserved. Validator reasons are static,
non-sensitive rule descriptions, so surfacing them does not leak data.

Adds a serializer test that uploads a CSV with a non-UTF-8 byte (0x92)
and asserts the reason is surfaced. Updates the metadata viewset and SVG
serializer assertions to match the file-identifying prefix.
Defence-in-depth for the CodeQL stack-trace-exposure finding: the reason
appended to the generic upload error is now collapsed to a single line and
length-capped (200 chars) before it can reach an API response. The caught
UploadValidationError already carries only curated, non-sensitive messages,
but this guarantees that even an unexpected value can never surface a
multi-line traceback or oversized internal detail.

Adds tests covering the appended-reason and sanitization behaviour.
@FrankApiyo
FrankApiyo force-pushed the fix/surface-upload-validation-reason branch from 9606c5e to 0814c77 Compare June 30, 2026 13:11
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.

2 participants