diff --git a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py index ce1d7bc03c6..da88264954f 100644 --- a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py +++ b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py @@ -1,28 +1,104 @@ # Generated by Django 5.2.15 on 2026-08-10 10:26 +import re + import django.contrib.postgres.indexes import django.contrib.postgres.operations -import django.db.models.expressions -import pulpcore.app.models.fields from django.db import migrations +import pulpcore.app.models.fields -class Migration(migrations.Migration): +# PostgreSQL regex matching paths that violate the relative_path domain constraint. +# Mirrors: '/' || VALUE || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)' +_INVALID_PATH_RE = r"[\n\r\s\t\?#]|(/\.{0,2}/)" + + +def _normalize_path(value): + """Return a normalized version of value that satisfies the relative_path constraint.""" + # Strip query string and URL fragment + value = re.sub(r"[?#].*$", "", value) + # Remove all whitespace and control characters + value = re.sub(r"[\n\r\t\s]+", "", value) + # Strip leading and trailing slashes + value = value.strip("/") + # Collapse runs of slashes + value = re.sub(r"/+", "/", value) + # Drop bare "." and ".." path components produced by the above steps + while True: + cleaned = re.sub(r"(?:^|/)\.\.?(?:/|$)", "/", value).strip("/") + cleaned = re.sub(r"/+", "/", cleaned) + if cleaned == value: + break + value = cleaned + return value + + +def fix_base_path_violations(apps, schema_editor): + """ + Normalize core_distribution.base_path values that would violate the + relative_path domain check constraint introduced in migration 0155. + The constraint rejects paths where '/' || VALUE || '/' matches + '[\n\r\s\t\?#]|(/\.{0,2}/)' — i.e. paths containing whitespace, '?', '#', + double-slashes, or dot/dotdot segments. This function detects such rows, + normalizes them where possible, and raises RuntimeError listing any that + cannot be automatically fixed. + """ + from django.db import connection + + unfixable = [] + + with connection.cursor() as cursor: + cursor.execute( + "SELECT pulp_id, base_path FROM core_distribution WHERE '/' || base_path || '/' ~ %s", + [_INVALID_PATH_RE], + ) + rows = cursor.fetchall() + + for pk, value in rows: + normalized = _normalize_path(value) + test = f"/{normalized}/" + if not normalized or re.search(r"[\n\r\s\t?#]|(/\.{0,2}/)", test): + unfixable.append(f" core_distribution.base_path (pk={pk!r}): {value!r}") + else: + with connection.cursor() as cursor: + cursor.execute( + "UPDATE core_distribution SET base_path = %s WHERE pulp_id = %s", + [normalized, pk], + ) + + if unfixable: + raise RuntimeError( + "The following core_distribution rows have base_path values that violate " + "the 'relative_path' domain constraint and could not be automatically " + "normalized. Fix or delete these records before running migrations:\n" + + "\n".join(unfixable) + ) + + +class Migration(migrations.Migration): atomic = False dependencies = [ - ('core', '0155_create_rel_path_domains'), + ("core", "0155_create_rel_path_domains"), ] operations = [ + migrations.RunPython( + fix_base_path_violations, + migrations.RunPython.noop, + ), migrations.AlterField( - model_name='distribution', - name='base_path', + model_name="distribution", + name="base_path", field=pulpcore.app.models.fields.RelativePathField(), ), django.contrib.postgres.operations.AddIndexConcurrently( - model_name='distribution', - index=django.contrib.postgres.indexes.SpGistIndex(django.contrib.postgres.indexes.OpClass("base_path", name='text_ops'), include=('pulp_domain',), name='core_distribution_base_path_text'), + model_name="distribution", + index=django.contrib.postgres.indexes.SpGistIndex( + django.contrib.postgres.indexes.OpClass("base_path", name="text_ops"), + include=("pulp_domain",), + name="core_distribution_base_path_text", + ), ), ] diff --git a/pulpcore/tests/unit/models/test_0156_migration.py b/pulpcore/tests/unit/models/test_0156_migration.py new file mode 100644 index 00000000000..69037df5a86 --- /dev/null +++ b/pulpcore/tests/unit/models/test_0156_migration.py @@ -0,0 +1,120 @@ +"""Unit tests for migration 0156's normalize helper and migration function.""" + +import importlib +from unittest.mock import MagicMock, patch + +import pytest + +_migration = importlib.import_module( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more" +) +_normalize_path = _migration._normalize_path +fix_base_path_violations = _migration.fix_base_path_violations + + +# --------------------------------------------------------------------------- +# _normalize_path — pure Python, no database required +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "value, expected", + [ + # trailing slash is stripped + ("fedora/", "fedora"), + # leading slash is stripped + ("/fedora", "fedora"), + # double slash is collapsed + ("fedora//el9", "fedora/el9"), + # newline is removed + ("fedora\nel9", "fedorael9"), + # space is removed + ("fedora el9", "fedorael9"), + # query string is stripped + ("fedora?foo=1", "fedora"), + # fragment is stripped + ("fedora#anchor", "fedora"), + # dot component is removed + ("fedora/./el9", "fedora/el9"), + # dotdot component is collapsed + ("fedora/../el9", "el9"), + # clean path is returned unchanged + ("fedora/el9/x86_64", "fedora/el9/x86_64"), + ], +) +def test_normalize_path(value, expected): + assert _normalize_path(value) == expected + + +# --------------------------------------------------------------------------- +# fix_base_path_violations — mock the DB cursor to avoid constraint conflicts +# --------------------------------------------------------------------------- + + +def _make_cursor(rows): + """Return a context-manager mock cursor that yields *rows* on fetchall().""" + cursor = MagicMock() + cursor.__enter__ = lambda s: s + cursor.__exit__ = MagicMock(return_value=False) + cursor.fetchall.return_value = rows + return cursor + + +def test_fix_base_path_violations_normalizes_row(monkeypatch): + """A row with a trailing-slash base_path is UPDATE-d to the normalized value.""" + pk = "some-uuid" + bad_path = "trailing/" + good_path = "trailing" + + select_cursor = _make_cursor([(pk, bad_path)]) + update_cursor = _make_cursor([]) + + cursors = iter([select_cursor, update_cursor]) + connection_mock = MagicMock() + connection_mock.cursor.side_effect = lambda: next(cursors) + + with patch( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", + connection_mock, + ): + fix_base_path_violations(None, None) + + update_cursor.execute.assert_called_once_with( + "UPDATE core_distribution SET base_path = %s WHERE pulp_id = %s", + [good_path, pk], + ) + + +def test_fix_base_path_violations_skips_clean_rows(monkeypatch): + """A row with a valid base_path is NOT UPDATE-d.""" + # SELECT returns no rows (valid paths don't match the invalid regex) + select_cursor = _make_cursor([]) + connection_mock = MagicMock() + connection_mock.cursor.return_value = select_cursor + + with patch( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", + connection_mock, + ): + fix_base_path_violations(None, None) + + # Only one cursor call (the SELECT), no UPDATE + assert connection_mock.cursor.call_count == 1 + + +def test_fix_base_path_violations_raises_for_unfixable_path(): + """A row whose path normalizes to empty string raises RuntimeError.""" + pk = "some-uuid" + # A bare "." normalizes to "" which is unfixable + unfixable_path = "." + + select_cursor = _make_cursor([(pk, unfixable_path)]) + connection_mock = MagicMock() + connection_mock.cursor.return_value = select_cursor + + with patch( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", + connection_mock, + ): + with pytest.raises(RuntimeError, match="could not be automatically normalized"): + fix_base_path_violations(None, None)