Skip to content

Don't leak overwrite kwarg into plugin validated_data#7717

Merged
ggainey merged 1 commit into
pulp:mainfrom
daviddavis:fix/overwrite-default-leak
May 14, 2026
Merged

Don't leak overwrite kwarg into plugin validated_data#7717
ggainey merged 1 commit into
pulp:mainfrom
daviddavis:fix/overwrite-default-leak

Conversation

@daviddavis
Copy link
Copy Markdown
Contributor

Problem

The overwrite field added in #7550 (commit f6dc7b9) on NoArtifactContentSerializer was declared with default=True. DRF auto-injects any field with a default=... into validated_data even when the caller does not supply it, so plugins that splat **serializer.validated_data into a content model constructor were broken by the unexpected overwrite=True kwarg.

For example, in pulp_deb's sync stage (pulp_deb/app/tasks/synchronizing.py:1057):

serializer = serializer_class.from822(data=package_paragraph)
serializer.is_valid(raise_exception=True)
package_content_unit = package_class(
    relative_path=package_relpath,
    sha256=package_sha256,
    **serializer.validated_data,   # <-- now contains overwrite=True
)

The Django Package model has no overwrite field, so:

TypeError: Package() got unexpected keyword arguments: 'overwrite'

This caused 10 functional test_sync tests in pulp_deb to fail.

The other inherited control fields on NoArtifactContentSerializer (repository, pulp_labels) don't trigger the same issue because none of them declare a default=, so DRF only puts them into validated_data when explicitly supplied. overwrite was the only one that leaked unconditionally.

Fix

Remove the default=True from the field declaration. The existing validated_data.pop("overwrite", True) in NoArtifactContentSerializer.create() already encodes the "absent ⇒ True" semantics, so there is no behavior change for REST API callers:

Caller behavior Before After
Omits overwrite validated_data["overwrite"] = True, pop returns True → silent overwrite Field absent from validated_data, pop("overwrite", True) returns True → silent overwrite
Sends overwrite: false Field validates → False in validated_data → check runs Same
Sends overwrite: true Field validates → True in validated_data → silent overwrite Same

Verification

  • All 7 existing tests in pulp_file/tests/functional/api/test_overwrite_content.py still pass.
  • The 10 pulp_deb test_sync failures are resolved by this change alone.

AI attribution

AI-Generated: github-copilot

@daviddavis daviddavis force-pushed the fix/overwrite-default-leak branch from f95c106 to 87063e9 Compare May 14, 2026 14:32
The overwrite field on NoArtifactContentSerializer was declared with
default=True, which causes DRF to inject overwrite=True into
validated_data even when the caller omits it. This broke plugins that
splat **validated_data into a content model constructor (e.g. pulp_deb
sync), producing:

    TypeError: Package() got unexpected keyword arguments: 'overwrite'

Drop the field-level default and rely on the existing
validated_data.pop('overwrite', True) in create() to preserve the
'absent => True' semantics. No behavior change for API callers.

AI-Generated: github-copilot
@ggainey ggainey merged commit e2c3f4d into pulp:main May 14, 2026
24 of 27 checks passed
@patchback
Copy link
Copy Markdown

patchback Bot commented May 14, 2026

Backport to 3.111: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.111/e2c3f4dbf4620feb16b27c5c11b3a9ae920d067e/pr-7717

Backported as #7718

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants