mirror of
https://github.com/pgadmin-org/pgadmin4.git
synced 2026-08-17 16:34:44 -05:00
style: fix pre-existing pycodestyle violations across CVE-9.15 test files
24 violations had accumulated on the cve-9.15 branch from the #9901 and #9902 CVE-fix work and a couple of older spots. Surfaced when the full suite was run after the #9904 work; no functional change. - 22 x E501 (line too long > 79): - 15 in pgadmin/misc/file_manager/tests/test_filemanager_security.py (13 class declarations using two mixin parents, 2 docstrings) - 6 in pgadmin/utils/tests/test_session_file_format.py (5 class declarations, 1 docstring) - 1 in pgadmin/browser/tests/test_kerberos_with_mocking.py (extracted self.app.url_map._rules_by_endpoint into a local before the membership test) - 2 x E305 (expected 2 blank lines after class/function): - pgadmin/misc/file_manager/__init__.py (after _open_upload_target) - pgadmin/browser/__init__.py (after _first_form_error) Class declarations are wrapped via parenthesised continuation, the standard pgAdmin convention; docstrings are either shortened or wrapped across two lines preserving the same meaning. Verified: - pycodestyle clean project-wide (24 -> 0). - Affected tests still pass: test_filemanager_security 17/0/0, test_session_file_format 18/0/0, test_kerberos_with_mocking 2/0/3 (skips are pre-existing and unrelated -- Kerberos blueprint not loaded in default config).
This commit is contained in:
@@ -91,6 +91,8 @@ def _first_form_error_message(form, default=None):
|
||||
if error:
|
||||
return str(error)
|
||||
return default
|
||||
|
||||
|
||||
PGADMIN_BROWSER = 'pgAdmin.Browser'
|
||||
PASS_ERROR_MSG = gettext('Your password has not been changed.')
|
||||
SMTP_SOCKET_ERROR = gettext(
|
||||
|
||||
@@ -66,7 +66,8 @@ class KerberosLoginMockTestCase(BaseTestGenerator):
|
||||
# in setUp is too late to register the blueprint, so the failure
|
||||
# path that redirects to /kerberos/login would 500 with
|
||||
# BuildError. Skip when that endpoint isn't reachable.
|
||||
if 'authenticate.kerberos_login' not in self.app.url_map._rules_by_endpoint:
|
||||
endpoint_map = self.app.url_map._rules_by_endpoint
|
||||
if 'authenticate.kerberos_login' not in endpoint_map:
|
||||
self.skipTest(
|
||||
"Kerberos blueprint not loaded — set "
|
||||
"AUTHENTICATION_SOURCES=['kerberos'] in config_local.py at "
|
||||
|
||||
@@ -67,6 +67,7 @@ def _open_upload_target(path):
|
||||
fd = os.open(path, flags, 0o600)
|
||||
return os.fdopen(fd, 'wb')
|
||||
|
||||
|
||||
MODULE_NAME = 'file_manager'
|
||||
global transid
|
||||
|
||||
|
||||
@@ -75,7 +75,9 @@ class _CheckAccessPermissionMixin:
|
||||
# Positive — legitimate paths must continue to be allowed.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestLegitPathInsideStorage(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestLegitPathInsideStorage(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""A path entirely inside the storage dir must pass."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -85,7 +87,9 @@ class TestLegitPathInsideStorage(_CheckAccessPermissionMixin, BaseTestGenerator)
|
||||
Filemanager.check_access_permission(self.in_dir, "/myfile.txt")
|
||||
|
||||
|
||||
class TestRelativeNotationResolvingInside(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestRelativeNotationResolvingInside(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""A path with .. that resolves inside the sandbox must pass."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -97,7 +101,9 @@ class TestRelativeNotationResolvingInside(_CheckAccessPermissionMixin, BaseTestG
|
||||
self.in_dir, "/subdir/../myfile.txt")
|
||||
|
||||
|
||||
class TestSymlinkedStorageRootPassesForRealChild(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestSymlinkedStorageRootPassesForRealChild(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""If the storage root itself is a symlink, legitimate access still works.
|
||||
|
||||
Without realpath()ing in_dir, a symlinked storage root would compare
|
||||
@@ -118,7 +124,9 @@ class TestSymlinkedStorageRootPassesForRealChild(_CheckAccessPermissionMixin, Ba
|
||||
Filemanager.check_access_permission(symlinked_root, "/legit.txt")
|
||||
|
||||
|
||||
class TestServerModeFalseSkipsCheck(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestServerModeFalseSkipsCheck(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""SERVER_MODE=False: check is a no-op, allowing any path."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -135,7 +143,9 @@ class TestServerModeFalseSkipsCheck(_CheckAccessPermissionMixin, BaseTestGenerat
|
||||
# Negative — escapes (symlinks and ..) must be rejected.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestSymlinkPointingOutsideRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestSymlinkPointingOutsideRejected(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""The core Vuln 2 fix: a symlink whose target is outside in_dir blocks."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -149,7 +159,9 @@ class TestSymlinkPointingOutsideRejected(_CheckAccessPermissionMixin, BaseTestGe
|
||||
self.in_dir, evil_path + "/victim.txt")
|
||||
|
||||
|
||||
class TestSymlinkAtLeafRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestSymlinkAtLeafRejected(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""A symlink AS the leaf (not via intermediate) is also rejected."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -188,7 +200,9 @@ class TestDotDotEscapeRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
# static method so the assertions are about the function behavior, not the
|
||||
# wiring — which is verified by reading the code in §4.2.3 / §4.2.4.
|
||||
|
||||
class TestRenameTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestRenameTargetSymlinkRejected(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""rename's target path through a symlink → access denied."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -204,7 +218,9 @@ class TestRenameTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGener
|
||||
self.in_dir, evil_path + "/renamed.txt")
|
||||
|
||||
|
||||
class TestDeleteTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestDeleteTargetSymlinkRejected(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""delete's target path through a symlink → access denied."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -219,7 +235,9 @@ class TestDeleteTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGener
|
||||
Filemanager.check_access_permission(self.in_dir, evil_path)
|
||||
|
||||
|
||||
class TestDownloadTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestDownloadTargetSymlinkRejected(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""download's target path through a symlink → access denied."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -234,7 +252,9 @@ class TestDownloadTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGen
|
||||
Filemanager.check_access_permission(self.in_dir, evil_path)
|
||||
|
||||
|
||||
class TestAddfolderTargetSymlinkRejected(_CheckAccessPermissionMixin, BaseTestGenerator):
|
||||
class TestAddfolderTargetSymlinkRejected(
|
||||
_CheckAccessPermissionMixin, BaseTestGenerator
|
||||
):
|
||||
"""addfolder's target path through a symlink → access denied."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -267,7 +287,9 @@ class _OpenUploadTargetMixin:
|
||||
shutil.rmtree(self.tmpdir, ignore_errors=True)
|
||||
|
||||
|
||||
class TestOpenUploadTargetCreatesNewFile(_OpenUploadTargetMixin, BaseTestGenerator):
|
||||
class TestOpenUploadTargetCreatesNewFile(
|
||||
_OpenUploadTargetMixin, BaseTestGenerator
|
||||
):
|
||||
"""Positive: opening a non-existent path creates a regular file."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -282,8 +304,12 @@ class TestOpenUploadTargetCreatesNewFile(_OpenUploadTargetMixin, BaseTestGenerat
|
||||
self.assertEqual(f.read(), b"hello")
|
||||
|
||||
|
||||
class TestOpenUploadTargetOverwritesRegularFile(_OpenUploadTargetMixin, BaseTestGenerator):
|
||||
"""Positive: opening over an existing regular file truncates and rewrites."""
|
||||
class TestOpenUploadTargetOverwritesRegularFile(
|
||||
_OpenUploadTargetMixin, BaseTestGenerator
|
||||
):
|
||||
"""Positive: opening over an existing regular file truncates and
|
||||
rewrites.
|
||||
"""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
|
||||
@@ -299,7 +325,7 @@ class TestOpenUploadTargetOverwritesRegularFile(_OpenUploadTargetMixin, BaseTest
|
||||
|
||||
|
||||
class TestOpenUploadTargetMode0600(_OpenUploadTargetMixin, BaseTestGenerator):
|
||||
"""Positive: newly-created upload files are mode 0o600 (intentional hardening).
|
||||
"""Positive: newly-created upload files are mode 0o600.
|
||||
|
||||
Documented behavior change vs. the umask-default 0o644 of the prior
|
||||
open(name, 'wb'). Release notes call this out.
|
||||
@@ -320,7 +346,9 @@ class TestOpenUploadTargetMode0600(_OpenUploadTargetMixin, BaseTestGenerator):
|
||||
"Expected 0o600 (owner-only), got 0o%o" % mode)
|
||||
|
||||
|
||||
class TestOpenUploadTargetRejectsLeafSymlink(_OpenUploadTargetMixin, BaseTestGenerator):
|
||||
class TestOpenUploadTargetRejectsLeafSymlink(
|
||||
_OpenUploadTargetMixin, BaseTestGenerator
|
||||
):
|
||||
"""Negative: a pre-planted symlink at the leaf path raises ELOOP/EMLINK.
|
||||
|
||||
This is the TOCTOU-closing property: even if check_access_permission
|
||||
|
||||
@@ -193,7 +193,9 @@ class TestSessionRoundTrip(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
self.assertEqual(loaded.hmac_digest, sess.hmac_digest)
|
||||
|
||||
|
||||
class TestMultipleSessionsAreIndependent(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
class TestMultipleSessionsAreIndependent(
|
||||
_SessionTestSetupMixin, BaseTestGenerator
|
||||
):
|
||||
"""Two sessions must not bleed data into each other."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -351,7 +353,9 @@ class TestValidHeaderEmptyBody(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
self.assert_no_warning_logged("session file rejected")
|
||||
|
||||
|
||||
class TestCookieHmacMismatchWithValidFile(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
class TestCookieHmacMismatchWithValidFile(
|
||||
_SessionTestSetupMixin, BaseTestGenerator
|
||||
):
|
||||
"""Spec T8: a legitimate file but a cookie that doesn't bind to it.
|
||||
|
||||
The file-HMAC verifies the file is server-written; the cookie-HMAC
|
||||
@@ -375,7 +379,9 @@ class TestCookieHmacMismatchWithValidFile(_SessionTestSetupMixin, BaseTestGenera
|
||||
self.assert_no_warning_logged("session file rejected")
|
||||
|
||||
|
||||
class TestUnsafeSidReturnsNewSession(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
class TestUnsafeSidReturnsNewSession(
|
||||
_SessionTestSetupMixin, BaseTestGenerator
|
||||
):
|
||||
"""A sid containing path-traversal characters yields safe_join None."""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
@@ -411,8 +417,10 @@ class TestEmptySecretRaises(BaseTestGenerator):
|
||||
)
|
||||
|
||||
|
||||
class TestRealisticSessionShapeRoundTrip(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
"""Round-trip a session with realistic pgAdmin contents (MFA, OAuth2, etc.).
|
||||
class TestRealisticSessionShapeRoundTrip(
|
||||
_SessionTestSetupMixin, BaseTestGenerator
|
||||
):
|
||||
"""Round-trip a session with realistic pgAdmin contents.
|
||||
|
||||
Spec test 7: MFA fields are JSON-safe but exercise them through put/get
|
||||
to confirm the format change preserves the data structures pgAdmin
|
||||
@@ -509,7 +517,9 @@ class TestSessionFileMode0o600(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
"got 0o%o" % new_mode)
|
||||
|
||||
|
||||
class TestServerModeFalseDirectUpload(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
class TestServerModeFalseDirectUpload(
|
||||
_SessionTestSetupMixin, BaseTestGenerator
|
||||
):
|
||||
"""Spec T10: Scenario A chain closed at the session-read layer.
|
||||
|
||||
Even if SERVER_MODE=False permits an upload directly into the sessions
|
||||
|
||||
Reference in New Issue
Block a user