Compare commits

...

4 Commits

Author SHA1 Message Date
Hamzah Yousuf
8ab5deb54a FIX: Refactor formatted raw SQL in unified_jobs result_stdout_raw_handle (#16522)
* Refactor result_stdout_raw_handle to use parameterized COPY SQL.
Replace f-string SQL construction with psycopg.sql composables and bound
parameters so security scans no longer flag formatted raw SQL in the
unified jobs stdout path.
Fix sqlite_copy mock rendering for psycopg3 SQL composables.

* Fix sqlite_copy mock without psycopg SQL internals.
Load stdout from the first populated event table instead of rendering
psycopg composables, which use version-specific private attributes.

* Use sql.Literal in COPY query for Django cursor.copy compatibility.
Django's cursor.copy() does not forward bind parameters to psycopg,
which caused stdout API 500s against real PostgreSQL.
2026-06-25 13:49:22 +00:00
Rodrigo Toshiaki Horie
843f23f4cb Fix SonarCloud security rating: remove user-controlled data from sqlite filepath (#16516)
* Fix SonarCloud security rating by removing user-controlled data from sqlite filepath

Replace os.path.basename(sys.argv[0]) with a hardcoded 'unknown' fallback
in RecordedQueryLog.write() to eliminate path injection via CLI arguments.
This resolves SonarCloud rule pythonsecurity:S8706 and helps restore the
AWX security rating from C to A.

Closes: AAP-80006

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Remove unused sys import from test_db.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-06-25 10:08:54 -03:00
Lila Yasin
6d665dda33 Fix awxkit tox test failure caused by root pytest.ini config bleed (#16520)
Forward-port of ansible/tower#7537 for the devel branch.

When running awxkit's tox tests, pytest picks up the root pytest.ini
which pulls in pytest-django options (--reuse-db, --nomigrations,
DJANGO_SETTINGS_MODULE) and Django-specific filterwarnings.  Since the
awxkit tox environment does not install Django or pytest-django, these
cause test collection to fail.

The root cause is that pytest.ini has absolute priority in pytest's
config discovery — it searches all ancestor directories for pytest.ini
before falling back to tox.ini's [pytest] section.  A [pytest] section
in awxkit/tox.ini alone cannot prevent the root config from being used.

Fix by:
- Adding awxkit/pytest.ini to act as the primary config boundary
  (pytest.ini has the highest priority in config discovery, so its
  presence in awxkit/ stops the upward search before reaching root)
- Adding explicit `test` path argument to the pytest command in
  awxkit/tox.ini so pytest discovers tests correctly
- Adding `testpaths` and `python_files` to the [pytest] section in
  awxkit/tox.ini as a secondary config boundary
- Adding awxkit/conftest.py that registers the Django-specific CLI
  options and INI keys as harmless no-ops, as a further safety net

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-06-24 10:42:23 -04:00
Stevenson Michel
17dc7f898a Fix: handle cursor-paginated responses in get_one() (#16521)
* Fix: handle cursor-paginated API responses in get_one()

DAB PR #1025 switched role_team_assignments and role_user_assignments
endpoints from PageNumberPagination to CursorPagination. Cursor
pagination returns {results, next, previous} without a count field,
causing get_one() to fail with "The endpoint did not provide count
and results".

When the response includes results but no count, infer count from
len(results). Also guard get_all_endpoint() against missing count.
2026-06-24 09:56:26 -04:00
8 changed files with 57 additions and 21 deletions

View File

@@ -68,7 +68,7 @@ class RecordedQueryLog(object):
progname = match progname = match
break break
else: else:
progname = os.path.basename(sys.argv[0]) progname = 'unknown'
filepath = os.path.join(self.dest, '{}.sqlite'.format(progname)) filepath = os.path.join(self.dest, '{}.sqlite'.format(progname))
version = _get_version('awx') version = _get_version('awx')
log = sqlite3.connect(filepath, timeout=3) log = sqlite3.connect(filepath, timeout=3)

View File

@@ -20,6 +20,9 @@ from dispatcherd.factories import get_control_from_settings
# Django # Django
from django.conf import settings from django.conf import settings
from django.db import models, connection, transaction from django.db import models, connection, transaction
# psycopg
from psycopg import sql
from django.db.models.constraints import UniqueConstraint from django.db.models.constraints import UniqueConstraint
from django.core.exceptions import NON_FIELD_ERRORS from django.core.exceptions import NON_FIELD_ERRORS
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
@@ -1179,17 +1182,23 @@ class UnifiedJob(
raise StdoutMaxBytesExceeded(total, max_supported) raise StdoutMaxBytesExceeded(total, max_supported)
tbl = self._meta.db_table + 'event' tbl = self._meta.db_table + 'event'
created_by_cond = '' where_parts = [
sql.SQL('{} = {}').format(sql.Identifier(self.event_parent_key), sql.Literal(self.id)),
sql.SQL("stdout != ''"),
]
if self.has_unpartitioned_events: if self.has_unpartitioned_events:
tbl = f'_unpartitioned_{tbl}' tbl = '_unpartitioned_' + tbl
else: else:
created_by_cond = f"job_created='{self.created.isoformat()}' AND " where_parts.insert(0, sql.SQL('job_created = {}').format(sql.Literal(self.created)))
sql = f"copy (select stdout from {tbl} where {created_by_cond}{self.event_parent_key}={self.id} and stdout != '' order by start_line) to stdout" # nosql copy_sql = sql.SQL('COPY (SELECT stdout FROM {} WHERE {} ORDER BY start_line) TO STDOUT').format(
sql.Identifier(tbl),
sql.SQL(' AND ').join(where_parts),
)
# psycopg3's copy writes bytes, but callers of this # psycopg3's copy writes bytes, but callers of this
# function assume a str-based fd will be returned; decode # function assume a str-based fd will be returned; decode
# .write() calls on the fly to maintain this interface # .write() calls on the fly to maintain this interface
with cursor.copy(sql) as copy: with cursor.copy(copy_sql) as copy:
while data := copy.read(): while data := copy.read():
fd.write(smart_str(bytes(data))) fd.write(smart_str(bytes(data)))

View File

@@ -830,14 +830,13 @@ class MockCopy:
events = [] events = []
index = -1 index = -1
def __init__(self, sql): def __init__(self):
self.events = [] self.events = []
parts = sql.split(' ')
tablename = parts[parts.index('from') + 1]
for cls in (JobEvent, AdHocCommandEvent, ProjectUpdateEvent, InventoryUpdateEvent, SystemJobEvent): for cls in (JobEvent, AdHocCommandEvent, ProjectUpdateEvent, InventoryUpdateEvent, SystemJobEvent):
if cls._meta.db_table == tablename: events = list(cls.objects.order_by('start_line').values_list('stdout', flat=True))
for event in cls.objects.order_by('start_line').all(): if events:
self.events.append(event.stdout) self.events = events
break
def read(self): def read(self):
self.index = self.index + 1 self.index = self.index + 1
@@ -858,9 +857,8 @@ def sqlite_copy(request, mocker):
# copy is postgres-specific, and SQLite doesn't support it; mock its # copy is postgres-specific, and SQLite doesn't support it; mock its
# behavior to test that it writes a file that contains stdout from events # behavior to test that it writes a file that contains stdout from events
def write_stdout(self, sql): def write_stdout(self, sql, params=None):
mock_copy = MockCopy(sql) return MockCopy()
return mock_copy
mocker.patch.object(SQLiteCursorWrapper, 'copy', write_stdout, create=True) mocker.patch.object(SQLiteCursorWrapper, 'copy', write_stdout, create=True)

View File

@@ -1,7 +1,6 @@
import collections import collections
import os import os
import sqlite3 import sqlite3
import sys
import unittest import unittest
import pytest import pytest
@@ -125,7 +124,7 @@ def test_sql_above_threshold(tmpdir):
args, kw = _call args, kw = _call
assert args == ('EXPLAIN VERBOSE {}'.format(QUERY['sql']),) assert args == ('EXPLAIN VERBOSE {}'.format(QUERY['sql']),)
path = os.path.join(tmpdir, '{}.sqlite'.format(os.path.basename(sys.argv[0]))) path = os.path.join(tmpdir, 'unknown.sqlite')
assert os.path.exists(path) assert os.path.exists(path)
# verify the results # verify the results

View File

@@ -438,7 +438,7 @@ class ControllerAPIModule(ControllerModule):
raise RuntimeError('Expected list from API at {0}, got: {1}'.format(endpoint, response)) raise RuntimeError('Expected list from API at {0}, got: {1}'.format(endpoint, response))
next_page = response['json']['next'] next_page = response['json']['next']
if response['json']['count'] > 10000: if response['json'].get('count', 0) > 10000:
self.fail_json(msg='The number of items being queried for is higher than 10,000.') self.fail_json(msg='The number of items being queried for is higher than 10,000.')
while next_page is not None: while next_page is not None:
@@ -493,8 +493,11 @@ class ControllerAPIModule(ControllerModule):
fail_msg += ', detail: {0}'.format(response['json']['detail']) fail_msg += ', detail: {0}'.format(response['json']['detail'])
self.fail_json(msg=fail_msg) self.fail_json(msg=fail_msg)
if 'count' not in response['json'] or 'results' not in response['json']: if 'results' not in response['json']:
self.fail_json(msg="The endpoint did not provide count and results") self.fail_json(msg="The endpoint did not provide a results list")
if 'count' not in response['json']:
response['json']['count'] = len(response['json']['results'])
if response['json']['count'] == 0: if response['json']['count'] == 0:
if allow_none: if allow_none:

16
awxkit/conftest.py Normal file
View File

@@ -0,0 +1,16 @@
# This conftest registers pytest-django CLI options and INI keys as harmless
# no-ops so that the awxkit tox environment (which does not install Django or
# pytest-django) does not crash when the root pytest.ini config leaks through.
#
# Root pytest.ini contains settings like --reuse-db, --nomigrations, and
# DJANGO_SETTINGS_MODULE that are only meaningful for Django tests. Placing
# this conftest at the awxkit/ level ensures it is loaded before argument
# parsing regardless of which config file pytest ultimately selects.
def pytest_addoption(parser):
group = parser.getgroup("awxkit-compat", "awxkit tox compatibility shims")
group.addoption("--reuse-db", action="store_true", default=False, help="(no-op in awxkit) pytest-django compat")
group.addoption("--nomigrations", action="store_true", default=False, help="(no-op in awxkit) pytest-django compat")
group.addoption("--create-db", action="store_true", default=False, help="(no-op in awxkit) pytest-django compat")
parser.addini("DJANGO_SETTINGS_MODULE", help="(no-op in awxkit) pytest-django compat", default="")

9
awxkit/pytest.ini Normal file
View File

@@ -0,0 +1,9 @@
[pytest]
addopts = -v --tb=native
testpaths = test
python_files = test_*.py
filterwarnings =
error
junit_family=xunit2

View File

@@ -19,7 +19,7 @@ deps =
pytest-mock pytest-mock
commands = commands =
coverage run --parallel --source awxkit -m pytest --doctest-glob='*.md' --junit-xml=report.xml {posargs} coverage run --parallel --source awxkit -m pytest -c pytest.ini test --doctest-glob='*.md' --junit-xml=report.xml {posargs}
coverage combine coverage combine
coverage xml coverage xml
@@ -44,6 +44,8 @@ max-line-length = 120
[pytest] [pytest]
addopts = -v --tb=native addopts = -v --tb=native
testpaths = test
python_files = test_*.py
filterwarnings = filterwarnings =
error error