diff --git a/.github/workflows/coverage_comment.yml b/.github/workflows/coverage_comment.yml new file mode 100644 index 000000000000..d63e6f4a75ad --- /dev/null +++ b/.github/workflows/coverage_comment.yml @@ -0,0 +1,72 @@ +name: Coverage Comment + +on: + workflow_run: + workflows: ["Coverage Tests"] + types: + - completed + +permissions: + contents: read + pull-requests: write + +jobs: + comment: + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' && + github.repository == 'django/django' && + github.event.workflow_run.pull_requests[0] != null + name: Post Coverage Comment + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - name: Download diff coverage report + uses: actions/download-artifact@v4 + with: + name: diff-coverage-report-${{ github.event.workflow_run.pull_requests[0].number }} + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + + - name: Post/update PR comment + env: + PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }} + uses: actions/github-script@v8 + with: + script: | + const fs = require('fs'); + const reportPath = 'diff-cover-report.md'; + let body = 'No coverage data available.'; + if (fs.existsSync(reportPath)) { + body = fs.readFileSync(reportPath, 'utf8'); + } + const commentBody = '### 📊 Coverage Report for Changed Files\n\n```\n' + body + '\n```\n\n**Note:** Missing lines are warnings only. Some lines may not be covered by SQLite tests as they are database-specific.\n\nFor more information about code coverage on pull requests, see the [contributing documentation](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/#code-coverage-on-pull-requests).'; + + const prNumber = parseInt(process.env.PR_NUMBER); + if (isNaN(prNumber)) { + core.setFailed('PR number is not available or invalid.'); + return; + } + + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + }); + + for (const c of comments) { + if ((c.body || '').includes('📊 Coverage Report for Changed Files')) { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: c.id, + }); + } + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: commentBody, + }); diff --git a/.github/workflows/coverage_tests.yml b/.github/workflows/coverage_tests.yml new file mode 100644 index 000000000000..af463bb29502 --- /dev/null +++ b/.github/workflows/coverage_tests.yml @@ -0,0 +1,75 @@ +name: Coverage Tests + +on: + pull_request: + paths-ignore: + - 'docs/**' + - '**/*.md' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + coverage: + if: github.repository == 'django/django' + name: Coverage Tests (Windows) + runs-on: windows-latest + timeout-minutes: 60 + steps: + - name: Checkout + uses: actions/checkout@v5 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.14' + cache: 'pip' + cache-dependency-path: 'tests/requirements/py3.txt' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip wheel + python -m pip install -r tests/requirements/py3.txt -e . + python -m pip install 'coverage[toml]' diff-cover + + - name: Run tests with coverage + env: + PYTHONPATH: ${{ github.workspace }}/tests + COVERAGE_PROCESS_START: ${{ github.workspace }}/tests/.coveragerc + RUNTESTS_DIR: ${{ github.workspace }}/tests + run: | + python -Wall tests/runtests.py -v2 + + - name: Generate coverage report + if: success() + env: + COVERAGE_RCFILE: ${{ github.workspace }}/tests/.coveragerc + RUNTESTS_DIR: ${{ github.workspace }}/tests + run: | + python -m coverage combine + python -m coverage report --show-missing + python -m coverage xml -o tests/coverage.xml + + - name: Generate diff coverage report + if: success() + run: | + if (Test-Path 'tests/coverage.xml') { + diff-cover tests/coverage.xml --compare-branch=origin/main --fail-under=0 > diff-cover-report.md + } else { + Set-Content -Path diff-cover-report.md -Value 'No coverage data available.' + } + + - name: Upload diff coverage report + if: success() + uses: actions/upload-artifact@v4 + with: + name: diff-coverage-report-${{ github.event.pull_request.number }} + path: diff-cover-report.md + retention-days: 1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b709ed514540..079fae63283c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,6 +19,7 @@ permissions: jobs: windows: + if: github.event_name == 'push' runs-on: windows-latest strategy: matrix: diff --git a/django/db/backends/base/features.py b/django/db/backends/base/features.py index 1e957d105ed3..38af239cbe75 100644 --- a/django/db/backends/base/features.py +++ b/django/db/backends/base/features.py @@ -415,6 +415,10 @@ class BaseDatabaseFeatures: # Does the Round() database function round to even? rounds_to_even = False + # Should dollar signs be prohibited in column aliases to prevent SQL + # injection? + prohibits_dollar_signs_in_column_aliases = False + # A set of dotted paths to tests in Django's test suite that are expected # to fail on this database. django_test_expected_failures = set() diff --git a/django/db/backends/postgresql/compiler.py b/django/db/backends/postgresql/compiler.py index 08d78e333a5d..48d0ccfd9d06 100644 --- a/django/db/backends/postgresql/compiler.py +++ b/django/db/backends/postgresql/compiler.py @@ -1,6 +1,6 @@ from django.db.models.sql.compiler import ( # isort:skip SQLAggregateCompiler, - SQLCompiler as BaseSQLCompiler, + SQLCompiler, SQLDeleteCompiler, SQLInsertCompiler as BaseSQLInsertCompiler, SQLUpdateCompiler, @@ -25,15 +25,6 @@ def __str__(self): return "UNNEST(%s)" % ", ".join(self) -class SQLCompiler(BaseSQLCompiler): - def quote_name_unless_alias(self, name): - if "$" in name: - raise ValueError( - "Dollar signs are not permitted in column aliases on PostgreSQL." - ) - return super().quote_name_unless_alias(name) - - class SQLInsertCompiler(BaseSQLInsertCompiler): def assemble_as_sql(self, fields, value_rows): # Specialize bulk-insertion of literal values through UNNEST to diff --git a/django/db/backends/postgresql/features.py b/django/db/backends/postgresql/features.py index 5e4ed320be8e..30d06c95c500 100644 --- a/django/db/backends/postgresql/features.py +++ b/django/db/backends/postgresql/features.py @@ -70,6 +70,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_nulls_distinct_unique_constraints = True supports_no_precision_decimalfield = True can_rename_index = True + prohibits_dollar_signs_in_column_aliases = True test_collations = { "deterministic": "C", "non_default": "sv-x-icu", diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 20f06ad168a8..9068d87a8974 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -553,6 +553,14 @@ def quote_name_unless_alias(self, name): for table names. This avoids problems with some SQL dialects that treat quoted strings specially (e.g. PostgreSQL). """ + if ( + self.connection.features.prohibits_dollar_signs_in_column_aliases + and "$" in name + ): + raise ValueError( + "Dollar signs are not permitted in column aliases on " + f"{self.connection.display_name}." + ) if name in self.quote_cache: return self.quote_cache[name] if ( diff --git a/tests/annotations/tests.py b/tests/annotations/tests.py index 10cd05db63f4..6336cabafae9 100644 --- a/tests/annotations/tests.py +++ b/tests/annotations/tests.py @@ -1545,8 +1545,11 @@ def test_alias_filtered_relation_sql_injection_dollar_sign(self): qs = Book.objects.alias( **{"crafted_alia$": FilteredRelation("authors")} ).values("name", "crafted_alia$") - if connection.vendor == "postgresql": - msg = "Dollar signs are not permitted in column aliases on PostgreSQL." + if connection.features.prohibits_dollar_signs_in_column_aliases: + msg = ( + "Dollar signs are not permitted in column aliases on " + f"{connection.display_name}." + ) with self.assertRaisesMessage(ValueError, msg): list(qs) else: diff --git a/zizmor.yml b/zizmor.yml index 76e53f73cc29..23630337b48c 100644 --- a/zizmor.yml +++ b/zizmor.yml @@ -1,6 +1,7 @@ rules: dangerous-triggers: ignore: + - coverage_comment.yml - labels.yml - new_contributor_pr.yml unpinned-uses: