1901997 - [traceback] InvalidDocumentError: invalid BuildID during reducing

Status: RESOLVED FIXED

(Socorro :: Webapp, defect, P2)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

When someone who doesn't have protected data access requests a crash report, the raw crash gets reduced to just the public parts. If the BuildID annotation value is not a string, that raises a InvalidDocumentError and returns an HTTP 500:

Sentry: https://mozilla.sentry.io/issues/5481234398/events/97b5251a96fc49568883c9fa90968ce0/

release: v2024.06.07:cf09fe93

InvalidDocumentError: invalid: .BuildID: type null not in ['string']
  File "django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "crashstats/crashstats/decorators.py", line 149, in inner
    response = view(request, *args, **kwargs)
  File "csp/decorators.py", line 20, in _wrapped
    r = f(*a, **kw)
  File "crashstats/crashstats/decorators.py", line 101, in inner
    return view(request, *args, **kwargs)
  File "crashstats/crashstats/views.py", line 111, in report_index
    context["raw"] = raw_api.get(crash_id=crash_id)
  File "crashstats/crashstats/models.py", line 786, in get
    return reducer.traverse(data)
  File "socorro/lib/libsocorrodataschema.py", line 413, in traverse
    return self._traverse(schema_part=self.schema, document_part=document)
  File "socorro/lib/libsocorrodataschema.py", line 483, in _traverse
    new_doc[name] = self._traverse(
  File "socorro/lib/libsocorrodataschema.py", line 445, in _traverse
    raise InvalidDocumentError(

I think that given that this is the raw crash and nothing depends on it in this form, we should probably adjust the schema to allow for null, too.

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

I talked with Gabriele about this a bit on #crashreporting. It's not ok for the crash reporter to send a BuildID like this.

If we generalize this a bit, we want to think about what to do with invalid documents and how we guarantee we're not inadvertently showing protected data. In that case, instead of adding null to the list of types the BuildID should have, we should adjust the report view code to return a template that says something like "This crash report is invalid in some complicated way, so we're not going to show it to you." and show that instead of an HTTP 500.

This only happens when someone is trying to view a crash report and they don't have protected data access. In that case, we don't know why the document is invalid and we shouldn't show the document at all to people who don't have protected data access.

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

Grabbing this one because it's a nuisance.

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: [mozilla-services/socorro] bug-1901997: handle invalid BuildID for redacted crash reports (#6777)

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

willkg merged PR [mozilla-services/socorro]: bug-1901997: handle invalid BuildID for redacted crash reports (#6777) in 165fd98.

Comment 6

[Will Kahn-Greene [:willkg] ET needinfo? me]

This went to production in: https://github.com/mozilla-services/socorro/releases/tag/v2024.11.01

I'm only aware of one crash report that had the problem. We had many instances of the error because people kept viewing the report. It has since expired out of the system, so we don't have it available anymore so there isn't anything to verify this with.

However, I generated a test crash report for ReferenceBrowser:

https://crash-stats.mozilla.org/report/index/b5db5b3f-5202-4a0f-9938-730f40241101

If you view that and you're either not logged in or you do not have protected data access, you get a Malformed Report page.

If you view that and you're logged in and you have protected data access, you see the crash report.