Replace dill with validated JSON for pytest error reports - #427
Merged
Merged
Conversation
The report plugin loaded every value in the build:<id>:error-reports Redis hash with dill.loads. Any client with write access to the build namespace could store a pickle whose __reduce__ ran arbitrary code in the reporting job (CWE-502). zlib.decompress was the only step between Redis and the deserializer. Pickling was load-bearing: workers stored CallInfo.__dict__ with a live ExceptionInfo and traceback so the reporter could re-render the failure via pytest's default makereport. That is also why tblib and the whole _pytest/outcomes.py swap dance existed. Ship the rendered report instead. Workers now serialize the final TestReport for each non-passing phase through pytest's public pytest_report_to_serializable hook (the same path pytest-xdist uses) into compressed JSON. The reporter validates shape, identity and outcome before reconstructing via pytest_report_from_serializable and replays it from the outermost makereport wrapper. - Reject anything that is not a well-formed TestReport for the expected nodeid/phase with pytest.UsageError (exit 4). Malformed or legacy dill records fail the report job; they never become a pass. - Reject records that would fail-open: wasxfail on a failed outcome, or a skipped setup paired with a call report. - Serialize before acknowledge so an encoding error cannot lose a failure; late failures after another worker's ack are still discarded as TIMED OUT. - Clear wasxfail when synthesizing WILL_RETRY/TIMED OUT skips; carry user_properties and captured sections into the reporter's teardown so JUnit output is unchanged. user_properties are stringified on the wire. - Remove dill and tblib; delete ciqueue/_pytest/outcomes.py; raise the pytest floor to >=6.2.5 (hooks exist since 5.1; verified 6.2.5, 7.4.4, 8.4.2, 9.1.1 on Python 3.10-3.14). Wire format change: workers and the reporter must run the same ciqueue version within a build, and upgraded workers need a fresh build ID. Traceback style (--tb, --showlocals) is now decided by the worker's options. Strict-xfail XPASS, previously dropped on the wire, is now reported as failed. xfail-failed tests no longer consume a requeue. Assisted-By: devx/6b388d1b-b275-4664-9297-6db9839a7769
markdorison
marked this pull request as ready for review
September 18, 2026 20:17
mdwn
approved these changes
Sep 18, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ciqueue.pytestworkers now store each failing or skipped phase's finalTestReportas compressed JSON, via pytest's publicpytest_report_to_serializablehook (the same path pytest-xdist uses).ciqueue.pytest_reportvalidates every record (identity, phase, outcome, shape) and reconstructs it withpytest_report_from_serializable. Anything else, including legacy dill blobs, fails the report job with exit 4 rather than being skipped or treated as a pass.dillandtblibare removed,ciqueue/_pytest/outcomes.pyis deleted, and the pytest floor is now>=6.2.5.Why
The reporter called
dill.loadson every value in thebuild:<id>:error-reportsRedis hash. A client with write access to the build namespace could store a pickle whose__reduce__ran code in the reporting job (CWE-502). The pickle was load-bearing: workers shipped liveExceptionInfoand traceback objects so the reporter could re-render failures. So the fix is a wire-format change, not a check in front of the sink.Behaviour changes
ciqueueversion within a build; use a fresh build ID after upgrading. Mixed versions fail loudly at collection.--tb,--showlocals) is now decided by the worker's flags. The reporter no longer needs the source checkout to render frames.XPASSwas previously lost on the wire and is now reported as failed. xfail-failed tests no longer consume a requeue.record_propertyvalues are stringified on the wire, matching what JUnit already emits.Testing
Tests cover: rejection of an executable pickle record with no side effect and a non-zero exit; malformed and invalid envelopes; records that would fail open (mismatched nodeid,
wasxfailon a failed outcome, contradictory setup plus call); xfail, strict XPASS and dynamic xfail across worker to reporter including JUnit; retried tests whose teardown callspytest.xfail;record_propertywith a non-JSON value through both plugins. Existing integration counts, skip locations, WILL_RETRY and JUnit assertions are unchanged.The suite (35 tests) passes on Python 3.10 through 3.14 with current pytest, and on pytest 6.2.5, 7.4.4 and 8.4.2.
Out of scope
The Ruby runner has the same class of sink (
Marshal.loadinci/queue/redis/build_record.rb,ErrorReport.coder = Marshalin minitest and rspec). Tracked separately.