Two latent bugs in pgadmin/setup/tests/test_export_import_servers.py
caused this test to fail on macOS / modern Linux distros:
1. The test invoked the subprocesses with a hardcoded "python" via
os.system. macOS and many modern Linux distros do not provide a
"python" alias — only "python3" or a venv-specific binary. The
subprocess fails with "sh: python: command not found", exits non-
zero, and writes nothing to the dump-servers output file.
Same root cause that a50a553b0 ("chore: feature tests use
sys.executable") fixed for AppStarter; this is the same fix in the
regression test.
2. Both os.system calls redirected stderr to /dev/null. When the
subprocess failed the empty output file then tripped json.loads
with the misleading "Expecting value: line 1 column 1 (char 0)"
instead of surfacing the underlying "command not found" error.
Switch to subprocess.run with a list (so there is no shell quoting and
no command-injection surface), use sys.executable, capture stderr, and
self.fail() with the captured stderr if the subprocess exits non-zero.
Verified: setup.tests goes from 4 pass / 1 fail / 1 skip to 5 pass / 0
fail / 1 skip on PG18.
The "Avoiding a bruteforce attack" section in login.rst implied that
MAX_LOGIN_ATTEMPTS protected all logins; in fact it only applies to the
INTERNAL authentication source (the /authenticate/login view filters by
auth_source=INTERNAL). Operators using LDAP / OAUTH2 / KERBEROS /
WEBSERVER got no signal that brute-force protection lives at a different
layer.
Adds:
- login.rst: the bruteforce section now states that MAX_LOGIN_ATTEMPTS
is INTERNAL-only and points operators of external sources to the
upstream identity provider's lockout policy and to the reverse proxy
for IP-based throttling.
- ldap.rst: a new "Brute-force protection" section calling out that
LDAP credential lockout is the directory's responsibility (ppolicy /
AD account-lockout GPO) and that request rate-limiting belongs on
the proxy. Cross-references the login-page section.
No code change. Closes the documentation gap that surfaced during the
analysis of the #9904 lockout-bypass fix without inheriting the
maintenance burden of duplicating directory lockout in pgAdmin.
Mirrors the CVE-pending entries for #9898-#9902 with reporter credit.
The detailed CVE record (CVSS, description, affected files) is held in
the local docs/CVEs/ working draft and submitted to MITRE/Vulnogram
externally; only the release-notes summary lands in git pre-disclosure.
pgAdmin enforces MAX_LOGIN_ATTEMPTS only inside its own /authenticate/login
view. Flask-Security's default /login view (registered automatically by
security.init_app) was reachable but never consulted the locked field:
- User inherited Flask-Security's UserMixin.is_locked(), which always
returns True (= "not locked, proceed").
- User inherited Flask-Login's is_active, which only checks the active
column, not locked.
So an attacker with valid credentials could bypass a lockout by posting
to /login after triggering the counter at /authenticate/login.
Override both contracts on the User model so every auth path inherits
them:
- is_active: False when active=False OR locked=True, so
flask_security.login_user() refuses to mint a session.
- is_locked(form_error): returns False (= "locked") and appends the
same "Your account is locked" message /authenticate/login uses, so
LoginForm.validate() rejects the submission cleanly.
Only INTERNAL accounts are reachable by the bypass (LDAP/OAuth2/Kerberos/
Webserver users have no local password, which LoginForm.validate rejects
before the locked check) and lockout itself is internal-only (auth_source
filter at authenticate/__init__.py:124), so the overrides only take
effect for INTERNAL accounts.
Adds a unit test covering the four (active, locked) combinations of the
contract LoginForm.validate() depends on.
Also includes a SQLite-only data-cleanup migration. Migration 6650c52670c2
added the locked column with server_default='false'. SQLite has no native
BOOLEAN affinity, so the literal 'false' was stored as TEXT both in the
column DEFAULT and on existing rows backfilled by ALTER TABLE.
SQLAlchemy's Boolean processor then reads that TEXT back as Python True
(because bool('false') is True for any non-empty string), which means
the new is_active override would otherwise see every legacy row as
locked and refuse login. The new migration normalize_locked_text_default
rewrites every SQLite row's locked to integer 0/1 (NULL -> 0); on a
PostgreSQL config DB the column was always stored as a proper BOOLEAN
so the migration is a no-op there (and integer literals would fail
on a BOOLEAN column anyway). SCHEMA_VERSION is bumped from 51 to 52.
Reported-by: Fernando Bortotti <fernando.bortotti@bsd.com.br>
Drafts the user-facing release notes for the 9.15 release. Covers all
47 non-merge commits since REL-9_14:
- 18 issue-linked entries under New features / Housekeeping / Bug fixes,
with reporter credits (names only) for the six external CVE reports
- #9901 absorbs 14 follow-up commits (session encryption + 0o600,
SHA-256 digest, drop of live AuthSourceManager / cloud provider
instances, DATA_DIR perms, log file mode, log handler hardening,
user_info_server prompt-loop bound) via "Also..."
- #9830 absorbs the @with_object_filters extension to ServerNode.list
- 14 commits without an associated GitHub issue listed under
"Additional changes" (bug fixes / test stability / refactoring /
housekeeping) for transparency
CVE IDs are placeholders ("CVE pending") and will be filled in once
MITRE assigns them. Release date and bundled-utility version are
also placeholders pending the actual release.
click_modal: wait up to 5s for the MuiDialog-backdrop to become
invisible after clicking the modal button. MUI v7 leaves the backdrop
in the DOM during the ~300ms close animation, which intercepts the
next click in tests that chain modal->modal interactions.
open_query_tool: the execute-query toolbar button can re-render between
the visibility wait and the ActionChains.move_to_element call
(Firefox/geckodriver hits this regularly). Retry the move up to 3
times on StaleElementReferenceException, refetching the element each
attempt rather than reusing a stale handle.
Both changes are pure test-side stability fixes; no production code
or behavior is affected.
1. AppStarter.start_app() spawned the pgAdmin subprocess with bare
"python" via subprocess.Popen. macOS (and many modern Linux
distros) do not provide a "python" symlink — only "python3" or a
venv-specific binary. The result was an Errno 2 "No such file or
directory" the moment a feature test tried to launch pgAdmin.
Switch to sys.executable so the spawned pgAdmin uses the same
interpreter and venv as the test runner. Works regardless of how
the venv is named or whether "python" is on PATH.
2. yarn.lock churn from `yarn install` resolving the @tanstack /
moment-timezone / postcss peer-dep ranges. Snapshot the resolved
versions for reproducibility (post the dependency bumps in
d55ffe405).
- Database terminology corrections (bord → tabell, Visa → Vy, etc.)
- Standardize Tabellrymd, Sammansatt utlösare
- Fix punctuation in placeholder lists (Oxford-comma alignment)
- Drop incorrect fuzzy flag from .po header
Note: msgid "on" (from ai_tools.js) is missing here because the PR's
.po was regenerated against a slightly older source tree; it will be
re-added on the next `make messages` run.
Two related issues, both surfaced when running the regression suite
non-interactively:
1. user_info_server()'s while-not-validate retry loops had no upper
bound. With a mocked or closed stdin (test mocks, EOF in CI/cron,
typo'd PGADMIN_SETUP_EMAIL=''), the loop would call input()/pprompt()
forever, printing 'Invalid email address. Please try again.' on
every iteration. We saw this manifest as a 13.5 million-line
25 GB log when test_no_email_deliverability hit the case.
Cap each loop at MAX_PROMPT_ATTEMPTS=5 and raise RuntimeError with a
pointer to PGADMIN_SETUP_EMAIL/PASSWORD env vars on exhaustion.
2. test_no_email_deliverability included pg@postgres.local in its
"should be accepted with deliverability=False" data set. The .local
suffix is in email_validator.SPECIAL_USE_DOMAIN_NAMES which fails
syntactic validation regardless of the deliverability flag, so
validate_email always rejected it - looping forever pre-fix #1.
Set config.ALLOW_SPECIAL_EMAIL_DOMAINS = ['local'] in the test's
try/finally block so .local is allowed for this scenario, restored
afterward to avoid leaking state into other tests.
With both fixes in place, the test can be removed from the regression
runtests --exclude list.
Two test-infra issues exposed by the full regression suite:
1. PSQL socket tests (test_backend_task, test_psql_input,
test_resize_terminal, test_socket_disconnect, test_start_process)
created a fresh, unauthenticated app.test_client() and built a
socketio test client around it. The /pty connect handler is
wrapped with @socket_login_required, so the unauthenticated client
was rejected and is_connected('/pty') returned False.
Switch to the authenticated self.tester from BaseTestGenerator so
the connect handler accepts the connection. Matches the pattern
already used by BaseSocketTestGenerator (test_socket_connect,
test_psql_disabled).
2. test_role_dependencies_sql creates a temporary LOGIN role and
calls create_table as that role. On clusters where pg_hba.conf
does not allow arbitrary roles to connect from 127.0.0.1, the
create_table swallows the connection error via try/except and
the subsequent pg_class lookup returns no row, surfacing as an
opaque "NoneType is not subscriptable" error.
Detect the empty fetchone() and skipTest with a clear message
pointing at pg_hba.conf so the test fails gracefully on
environmentally-restricted clusters instead of erroring.
Tests that worked on legacy CI envs but failed under newer
email_validator / non-postgres-named superuser / no-app-context
on tearDown / etc.:
1. test_import_export_create_job_unit_test.py - E-string scenario
was labelled "Rejected: ... (false positive, safe)" expecting
parser over-rejection. The parser actually correctly identifies
the query as balanced (close paren is inside the literal). Update
expected to True and rename scenario to reflect the real
handling.
2. test_sg_data_isolation.py - tearDown does ORM query/delete after
the @create_user_wise_test_client wrapper has popped the test
client/context. Wrap tearDown's ORM ops in an explicit
app_context().
3. test_validate_user_email.py - the "with deliverability" scenario
used postgres@local.dev expecting rejection. Newer email_validator
no longer rejects this. Switch to postgres@nonexistent.invalid,
which is RFC-reserved and rejected regardless of DNS state. Also
align expected message with the actual format string used by
validate_user.
4-5. test_grant_wizard_save_permissions.py and
test_grant_wizard_get_sql.py - test data hardcoded 'postgres' /
'enterprisedb' as grantee/grantor. On clusters where the
superuser is named differently (e.g. dev Homebrew installs use
the local OS account), the GRANT statement fails with
"role X does not exist". Use self.server['username'] instead.
6. test_role_dependencies_sql.py - the test creates a fresh LOGIN
role and tries to CREATE TABLE as that role; without explicit
CREATE-on-database grant, the create silently fails and the
subsequent pg_class lookup returns no row. Mark the test role
SUPERUSER so it can create the table.
All are test-infra fixes; none of the production code paths under
test were modified.
Two related issues that surfaced in BatchProcessTest:
1. ConnectionLocker.__enter__ accessed Flask session (via 'in' check)
AFTER acquiring the lock. When called outside a request context,
session access raises RuntimeError. Python's 'with' semantics skip
__exit__ if __enter__ raises, so the lock leaked - any subsequent
call to ConnectionLocker hung forever waiting on a lock that
would never be released.
Wrap session access in try/except RuntimeError so missing-request-
context falls through cleanly and the lock is released normally
on with-block exit. Also use .get() chains so partial session
shapes do not raise KeyError.
2. The four batch_process unit tests (backup, import_export,
maintenance, restore) used app_context() instead of
test_request_context(). flask-babel's gettext() in
BackupMessage.details() and similar code paths require a request
context; ConnectionLocker.__enter__ also touches session as above.
Switching to test_request_context('/') gives both the request
binding they need.
Verified: tools.backup.tests.test_batch_process now runs 4/4 passing
(was 3 ERROR + 1 hang before fix#1; 3 FAIL before fix#2).
Both are pre-existing issues exposed by Python 3.14 / flask 3.1 /
flask-babel stricter context enforcement; not introduced by 9.15
CVE work.
processes.py:322 unconditionally indexed session['auth_source_manager']
to gate the KRB5CCNAME copy. When the Flask session does not have the
auth_source_manager key (test request contexts that bypass the login
flow, or sessions whose login hasn't populated this key yet), the
indexing raises KeyError, aborting BatchProcess.start() before it ever
spawns the subprocess. The error has no obvious traceback in the
runtests harness because it bubbles up through BackupCreateJob's mock
infrastructure - the test just records ERROR with no message.
Replace [...] with .get(...) chains so a missing auth_source_manager
yields None and the Kerberos branch is skipped, matching the original
intent.
Surfaced by BatchProcessTest "When backup server" / "When backup
globals" scenarios which mock current_user but leave session in its
default unauthenticated state. Same shape on master; the fix is a
one-line robustness improvement.
Two pre-existing master bugs surfaced by the data-isolation regression
tests:
1. 'e.message' on caught exceptions (13 sites). Exception.message was
removed in Python 3 (it lived on BaseException in Py2). When the
except handler runs, accessing e.message raises AttributeError,
masking the original exception. Werkzeug HTTPException doesn't
expose .message either - it uses .description / .__str__.
Replace 'e.message' with str(e) across:
- browser/server_groups/__init__.py (delete/update/create handlers)
- browser/server_groups/servers/__init__.py (5 handlers)
- browser/server_groups/servers/databases/__init__.py
- misc/cloud/__init__.py
- tools/debugger/__init__.py
- tools/grant_wizard/__init__.py (2 sites)
Sites guarded by hasattr(e, 'message') first (psycopg3 driver) are
left as-is; module.messages dict access (utils/__init__) is also
unrelated.
2. ServerNode.list() missing @with_object_filters decorator. PR #8917
(99b822e47) added object_filters as a required positional arg to
list() but only added the @with_object_filters decorator to
get_nodes(), leaving list() with a signature the Flask routing
couldn't satisfy. Surfaced by SharedServersGetTestCase 'Get a all
shared server' test, which calls the list endpoint directly.
Five small defensive fixes that were exposed by running the full
regression suite end-to-end:
1. utils/validation_utils.py: validate_email() now returns False
instead of raising TypeError when passed a non-str/bytes value
(e.g. None from a missing form field). Matches the wrapper's
contract that it only ever returns bool.
2. tools/user_management/__init__.py: list endpoint guarded against
users with no roles. u.roles[0].id -> u.roles[0].id if u.roles
else None. Triggered by ChangePasswordTestCase fixtures.
3. utils/preferences.py: control_props['tags'] / ['creatable']
replaced with .get(...) so preferences whose control_props omit
these keys do not raise KeyError on update.
4. browser/server_groups/servers/__init__.py (create endpoint):
convert_connection_parameter() is bidirectional (list<->dict).
The save path always wants the storage shape (dict). When input
is already a dict (internal callers / tests mimicking storage
form), skip conversion to avoid the dict->list round-trip that
breaks the MutableDict column. Same fix applied to the workspaces
save path.
5. misc/workspaces/__init__.py: same defensive handling for
convert_connection_parameter() on the save path.
These are all pre-existing master bugs surfaced by edge-case test
data; none are introduced by the 9.15 CVE work.
Same dot-username bug as 504775de8 / 9b29bc203 but in a different shape:
view_test_data.json query strings are Python expressions interpolating
server['username'] into ALTER TABLE ... OWNER TO and GRANT ... TO
clauses. views/tests/utils.py:create_view evaluates these expressions,
producing SQL with an unquoted identifier. A local PG role with a dot
(e.g. 'ashesh.vashi') was rejected with 'syntax error at or near "."'.
Substitute server['username'] with Driver.qtIdent(None, server['username'])
in the query template before evaluating, so the resulting SQL contains
the properly-quoted identifier. The substitution covers every
occurrence in the template (some queries have OWNER TO and GRANT TO in
the same string).
Accounts for the remaining 72 'syntax error at or near "."' cases that
504775de8 + 9b29bc203 did not reach.
Two more test setUp helpers had the same dot-username bug as
user_mappings (504775de8): server['username'] was substituted into
the OWNER TO clause unquoted, so a local PG role with a dot (e.g.
'ashesh.vashi') was rejected with 'syntax error at or near "."'.
- types/tests/utils.py - 'ALTER TYPE ... OWNER TO %s' now uses
Driver.qtIdent() for the username.
- compound_triggers/tests/utils.py - sql_query template substitution
now passes the qtIdent-quoted username.
Same Driver.qtIdent(None, server['username']) approach as in 504775de8.
The setUp helper in user_mappings/tests/utils.py interpolated
server['username'] directly into the CREATE USER MAPPING DDL, so a
local PostgreSQL role containing a dot (e.g. 'ashesh.vashi') was
parsed by PG as a schema-qualified identifier and rejected with
'syntax error at or near "."'. This blocked every test whose setUp
created a user mapping (UserMappingGetSQLTestCase, etc.).
Wrap the FOR target with Driver.qtIdent() so the identifier is
double-quoted when it contains special characters. Mirrors the
approach used by the resql framework's _normalize_owner() / <OWNER>
substitution introduced in d112dc3b9 - that fix covered SQL
expectation comparison; this fix covers test setUp DDL generation,
which the resql logic does not reach.
The OPTIONS user/password are left as raw '%s' since they are SQL
string literals (single-quoted), where dots are syntactically safe.
Verified: 33/33 user_mappings tests pass; zero 'syntax error at or
near "."' occurrences in the targeted run.
Found by aggressive review of 83abb1c4f:
1. fd leak on os.fdopen failure — os.open returns a raw fd; if
os.fdopen() raises after, the fd is orphaned. Wrap in try/except
and close on failure. Built-in open() doesn't have this issue
because it manages the fd internally.
2. Silent regression of close-on-exec — Python's built-in open() sets
fds non-inheritable by default since PEP 446 (3.4+). os.open()
does not. Replacing the default open path therefore made the log
fd inheritable across fork/exec — mostly harmless in pgAdmin's
subprocess paths (they default close_fds=True) but still a
silent behavioral diff. OR in O_CLOEXEC to match.
Also adds web/pgadmin/utils/tests/test_enhanced_log_rotation.py
covering: new file mode, rotated archive + new active file mode,
pre-existing file mode unchanged, and fd non-inheritable. Skipped on
Windows where POSIX mode bits and PEP 446 semantics differ. Verified
all 4 cases pass via web/regression/runtests.py --pkg utils.
Override EnhancedRotatingFileHandler._open() so the active log file
and rotated backups are created owner-only on POSIX. The parent
DATA_DIR is already 0o700, so this is defense-in-depth — but the
log file can contain sensitive context (auth events, query
fragments, paths) and should not be world/group readable on its own.
Pre-existing log files keep their current permissions; new installs
and rotated files pick up the tighter mode. Windows uses the default
open path since os.open mode is ignored there.
Two endpoints (change_password, forgot_password) had previously been
patched ad-hoc to handle wtforms validators that emit Babel
LazyString (LazyProxy) error messages — those instances report as
iterable, so json.dumps fails with "Circular reference detected" if
they reach the encoder. Both fixes lived inline with slightly
different shapes and a shared trap waiting to bite the next caller.
Extract `_first_form_error_message(form, default=None)`: walks
form.errors, force-resolves the first LazyString to plain str, returns
the default when the form has no errors. Use it from both endpoints.
Also drop the now-dead `from flask_security.views import
default_render_json` import — the only callsite in browser/__init__.py
was replaced when that function's signature drifted; the import was
left behind.
No behavior change beyond consolidating the two ad-hoc patterns; both
endpoint test suites still pass.
Two related hardening changes around the pgAdmin data directory:
1. Atomic 0o600 for pgadmin4.db
----------------------------
pgadmin/__init__.py:run_migration_for_sqlite() and
setup.py:setup_db()'s run_migration_for_sqlite() previously chmod'd
pgadmin4.db to 0o600 *after* SQLAlchemy/SQLite created it via
db_upgrade(). With a typical 0o022 umask, the file existed at 0o644
between SQLite's create and pgAdmin's chmod — a TOCTOU window.
Practically narrow because the parent dir is 0o700, but easy to
close: wrap the migration call in `os.umask(0o077)` so the file is
born 0o600. Both code paths use try/finally so the prior umask is
restored even on migration failure (a raise during db_upgrade no
longer leaves the rest of the worker running with 0o077 in effect).
The post-hoc chmod is kept as belt-and-suspenders for the case where
the file already existed at a wider mode from an older install.
2. 0o700 for sensitive DATA_DIR subdirectories
--------------------------------------------
setup/data_directory.py previously chmod'd only SESSION_DB_PATH and
the parent dir of SQLITE_PATH to 0o700. STORAGE_DIR (user uploads
including saved cloud-deployment certs/keys), AZURE_CREDENTIAL_CACHE
_DIR (MSAL token cache files — real Azure credentials), KERBEROS_
CCACHE_DIR (Kerberos credential caches), and the directory holding
pgadmin4.log (stack traces frequently capture sensitive context)
were left at umask-default (typically 0o755 — directory listing
readable by any local user).
Refactor the create-loop to track which directories were newly
created on this invocation, then apply chmod 0o700 uniformly to all
sensitive dirs at once. Errors (e.g., chmod on a mounted volume in
OpenShift) emit a WARNING but don't abort — same lenient pattern the
existing SQLITE_PATH-dir chmod already used.
pgadmin4.log itself is still 0o644 (umask-default) because Python's
logging.FileHandler doesn't take a mode argument; tracked as a
follow-up task to subclass the handler.
The session file contains OAuth access/refresh tokens, AWS / Google /
Azure / BigAnimal cloud credentials, the Kerberos cache path, MFA OTP
material, and pass_enc_key — the symmetric KEK that decrypts the user's
saved Postgres server passwords. The HMAC header introduced earlier in
this branch protects integrity but not confidentiality: a leak of
sessions/<sid> alone exposes every secret in plaintext.
Wrap the pickle body in Fernet (AES-128-CBC + HMAC-SHA256, AEAD) before
the on-disk HMAC computation — encrypt-then-MAC, so pickle.loads is
unreachable on the read path until both the file HMAC verifies and
Fernet authenticates the ciphertext.
File format becomes:
+----------------------------------------------------------+
| _HMAC_HEX_LEN bytes : hex HMAC over the ciphertext |
+----------------------------------------------------------+
| N bytes : Fernet(pickle((randval, digest, data))) |
+----------------------------------------------------------+
Fernet key is derived from SECRET_KEY via HKDF-SHA256 with a fixed,
versioned salt and info string (`pga-session-body-v1`,
`pgadmin session body encryption v1`) so multiple workers produce the
same key deterministically and a future format swap can derive a fresh
key without reusing bytes already in flight on disk.
Caveat (not theoretical): SECRET_KEY currently lives in pgadmin4.db in
the same DATA_DIR. A leak that includes BOTH sessions/ AND pgadmin4.db
recovers the derived Fernet key and decrypts session bodies. Closing
that gap requires moving SECRET_KEY out of DATA_DIR (e.g., into the OS
keychain via USE_OS_SECRET_STORAGE) — tracked as a Layer-2 follow-up.
Backwards compat: pre-Layer-1 session files (HMAC over plain pickle, no
Fernet) pass the HMAC check but raise InvalidToken on Fernet.decrypt.
That's caught and logged as "legacy unencrypted body"; users see a
one-time re-login on upgrade. (The same upgrade also flips
SESSION_DIGEST_METHOD's default from sha1 to sha256 and changes the
file format, so the re-login is unavoidable regardless.)
Three new tests:
* TestSessionBodyIsEncryptedOnDisk: place a sentinel in the session,
read raw bytes, assert the sentinel does NOT appear on disk.
* TestSessionBodyRejectedWithDifferentSecret: write under SECRET_KEY=A,
read under SECRET_KEY=B, confirm rejection (rules out hard-coded-key
bugs).
* TestLegacyHmacOnlyFileRejected: pre-Layer-1 file is rejected with the
expected "legacy unencrypted body" log message.
Pre-existing tests that built bodies directly (TestCorruptedHmacHeader,
TestCookieHmacMismatchWithValidFile) updated to wrap the body via the
new make_encrypted_body() helper so they exercise the realistic file
shape.
$DATA_DIR/sessions/<sid> contains OAuth access_tokens / refresh_tokens
(via session['oauth2_token']), AWS / Google / Azure / BigAnimal cloud
credentials (via the cloud refactors earlier in this branch), the
Kerberos credential cache path, MFA OTP material, and pass_enc_key —
the symmetric KEK that decrypts the user's saved Postgres server
passwords.
The HMAC header added earlier in this branch protects integrity but
not confidentiality: anyone with read access to the file gets the
secrets. Default `open(path, 'wb')` uses the process umask, which on
typical systems leaves files 0o644 (world-readable). Switch
new_session() and put() to a new _open_session_file() helper that
opens with `os.open(... O_WRONLY | O_CREAT | O_TRUNC, 0o600)`, mirroring
the upload helper introduced in PR 1.
The directory itself is already 0o700, so this is defense-in-depth for
container scenarios where the data volume might be mounted under shared
uids, or for misconfigurations of the directory mode.
NB: Existing session files retain their old mode until next write, then
adopt 0o600. Operators who want to forcibly tighten existing files can
chmod the sessions directory recursively post-upgrade.
Adds a positive test asserting both put() and new_session() produce
0o600 files (skipped on Windows where POSIX mode bits are not
meaningful).
Default SESSION_DIGEST_METHOD from hashlib.sha1 to hashlib.sha256.
HMAC-SHA1 is still cryptographically acceptable for the cookie's
(sid, randval) signature, but SHA-256 is the modern default and aligns
with the file-HMAC header introduced earlier in this branch. The session
file format already invalidates all existing sessions on upgrade (the
new HMAC header is required), so flipping this default at the same time
is a free hardening rather than an additional break.
Test polish from the post-merge hostile review:
* Tighten the "no unsafe deserializer imported" assertion in the four
cloud-module test files (RDS, Google, BigAnimal, Azure) to a regex
anchored at line start with a word boundary, so it catches
`from pickle import dumps, loads`, `import pickle as p`, and indented
imports — not just bare `import pickle`.
* test_login.py: drop a sid-rotation assertion that would have given
false confidence. Flask-Paranoid does NOT rotate the session id on
login (it binds a `_paranoid_token` to UA+IP and validates per
request), so an `assertNotEqual(pre_sid, post_sid)` would always fail
for the wrong reason. Comment the limitation in the test for the next
reviewer; stronger fixation testing is owed as a follow-up.
* docs/proposals: spec line numbers in §4.2 had drifted ~10 lines from
the implemented branch (helper extraction, etc.); refresh them and
the audit-summary table to point at HEAD-of-branch lines. Append a
§1.6 entry enumerating the residual `pickle.loads` callsites in
sqleditor / schema_diff / bgprocess that PR 7 / Phase 2 will close,
so future reviewers see the surface.
CI runs the python test suite with SERVER_MODE=False (DESKTOP) which
explicitly skips OAuth2 / LDAP / Kerberos / change-password / forgot-
password tests via runtime guards. Running the suite with
SERVER_MODE=True surfaced two real pgAdmin endpoint regressions and a
batch of stale test fixtures that this commit addresses.
Test-infra fixes (regression/python_test_utils/csrf_test_client.py):
* fetch_csrf was looking for <input id="csrf_token" ...> which the
React SPA login no longer renders. The token is exposed as
"csrfToken": "..." in a JSON config block embedded in /login HTML
(camelCase) and as "csrf_token": "..." in JSON API responses
(snake_case). Match both, fall back to the legacy hidden-input form.
* login() captured the CSRF from GET /login, but Flask-Paranoid
regenerates the session on login (anti-fixation), which drops the
csrf_token from the new session. Subsequent state-changing API calls
failed with "The CSRF session token is missing." Refresh the token
from a post-login GET /browser/ when SERVER_MODE is True.
Endpoint regressions (web/pgadmin/browser/__init__.py):
* change_password JSON path: bad_request(list(form.errors.values())[0][0])
passed a Babel LazyString (LazyProxy) to the JSON encoder, producing
500 with "Circular reference detected." Force resolution to plain
str.
* forgot_password JSON path: default_render_json(form, include_user=
False) used a Flask-Security API that no longer accepts include_user,
yielding 500 on every JSON POST. Replace with pgAdmin's standard
make_json_response/bad_request envelope.
Test-fixture maintenance:
* test_change_password.py / browser/tests/utils.py: send JSON to the
JSON-only change_password endpoint (was sending form data, which the
endpoint silently ignored); use DELETE /user_management/save/<id>
for cleanup; update respdata strings to match current pgAdmin output.
* test_login.py, test_gravatar_image_display.py,
test_ldap_with_mocking.py, test_kerberos_with_mocking.py,
test_webserver_with_mocking.py: drop the
'Gravatar image for X' assertion (server-rendered HTML no longer
exists; React renders the gravatar client-side) and verify successful
authentication via session._user_id instead.
* test_ldap_login.py: skip when LDAP config is template placeholders
(no live LDAP server reachable in dev/CI).
* test_kerberos_with_mocking.py: skip when authenticate.kerberos_login
is not registered (the blueprint loads only when KERBEROS is in
AUTHENTICATION_SOURCES at app-init time).
* test_reset_password.py: switch to JSON POST against the JSON-only
forgot_password endpoint; parametrize expected status; drop HTML
pre-check.
* test_validate_email.py: drop pg@postgres.local (modern email-
validator rejects RFC 6761 special-use TLDs); switch deliverability
scenario to .invalid/.test domains for stable DNS.
Net result: 296 tests pass, 0 failures, 12 skipped (LDAP/Kerberos
require infra; 1 pgcrypto scenario; cloud-wizard real-credential
tests).
The azure cloud-deployment module wrote a live Azure class instance
directly into session['azure']['azure_obj'] (no pickle.dumps -- it
relied on the session backend to serialize anything). That is an
implicit pickle dependency: any session-format change would crash, and
the live class instance carries Azure SDK credential objects with
mutable state.
Add Azure.to_state()/from_state() that round-trip the persistable
fields (tenant_id, session_token, use_interactive_credential,
authentication_record_json, region, subscription_id, availability_zone,
available_capabilities_list, azure_cache_name, azure_cache_location)
through a plain dict. Live SDK objects (_clients, _credentials,
_cli_credentials) are intentionally NOT in to_state -- they're rebuilt
lazily from authentication_record_json on first credential use.
from_state bypasses __init__ (which references current_user.username)
via cls.__new__(cls) so unit tests work without a Flask login context.
Module-level _get_azure_from_session()/_save_azure_to_session() helpers
replace the 18 session['azure']['azure_obj'] sites across 12 endpoints.
Worker-restart UX trade-off: today's behavior pickles the populated
Azure SDK client cache, surviving a worker recycle. After this change
the in-memory cache is gone on restart, but the persisted
authentication_record_json is sufficient for the SDK to silently rebuild
the credential without re-prompting for device code -- the class was
designed for this replay. Verification owed in PR review.
Seven new tests cover: round-trip of persistable fields, helper from
session state, lazy SDK objects after from_state, missing-session
graceful return, defaults for partial state, regression assertion that
no live instance leaks into 'azure_obj', and that the unsafe
deserializer is not used.
The biganimal cloud-deployment module pickled the live BigAnimalProvider
across 15 sites (verify, polling-for-token, projects, providers,
regions, db_types, db_versions, instance_types, volume_types,
volume_properties, deploy). Same pattern, same risk as the other
cloud providers.
Add BigAnimalProvider.to_state()/from_state() and
_get_biganimal_from_session()/_save_biganimal_to_session() helpers.
from_state uses cls.__new__(cls) to bypass __init__'s
get_auth_provider() HTTP call -- the persisted `provider` dict already
holds that data.
Five new tests cover: round-trip preservation, helper construction,
defaults for partial state, missing-session graceful return, and a
regression assertion that pickle is gone.
The google cloud-deployment module persisted the live Google class via
pickle.dumps/pickle.loads across 14 sites: the verify_credentials
handler, the OAuth2 callback, and every region/zone/version/instance-
type lookup. Each call paid a pickle round-trip and re-introduced the
deserialization risk inside session storage.
Add Google.to_state()/Google.from_state() that round-trip the
persistable fields (client_config, credentials_json, redirect_url,
project_id, regions, availability_zones, verification flags) through
a plain dict, and rebuild the google.oauth2.credentials.Credentials
SDK object lazily from credentials_json. Module-level
_get_google_from_session() / _save_google_to_session() helpers replace
the per-site pickle dance.
Six new tests cover: round-trip of all persistable fields, credential
rebuild from token dict, helper construction from session state,
helper returning None for missing/empty session, defaults for partial
state, and a regression assertion that pickle is gone from the module.
verify_credentials previously stored pickle.dumps(rds, -1) under
session['aws']['aws_rds_obj']; subsequent endpoints unpickled it via
pickle.loads. That pickle round-trip was an in-session deserialization
vector and forced the session storage to carry binary blobs.
The RDS class is a thin wrapper over boto3 clients: all of its mutable
state is the credential dict already saved at session['aws']['secret'].
Drop the pickled blob and reconstruct RDS per request via a new
_get_rds_from_session() helper. boto3 client construction is cheap
(microseconds) and clients are cached on the per-request instance.
Also fixes a pre-existing latent bug in verify_credentials where status
was undefined when the cached creds matched the new ones (only the
inside of the cache-miss branch assigned it), now explicitly set to
True for the cache-hit path.
Five new tests cover: full creds, missing session_token, no aws key,
no secret key, and a regression assertion that the cloud.rds module
no longer imports the unsafe deserializer.
The OAuth2 login redirect flow stashed the live AuthSourceManager
instance in flask.session via session['auth_obj'] = auth_obj, requiring
the session storage to handle arbitrary Python objects. That coupled the
auth flow to the pickle-backed session format and presented a second-
order deserialization vector if any path could influence session
contents.
The only piece of state that genuinely needs to cross the OAuth2
provider redirect is the provider name. Persist that as
session['oauth2_current_client'] (a plain str) and reconstruct a fresh
AuthSourceManager in oauth_authorize from current_app's auth-source
registry. The OAuth2 source's per-instance oauth2_current_client is
restored before login() so client selection still works on the callback.
Defensive: oauth_authorize now redirects with a flash error when the
session-state key is missing (e.g., session expired between login and
callback), instead of KeyError'ing.
Two new scenarios in test_oauth2_with_mocking.py verify (a) post-redirect
session contains the new minimal state and not a live class instance,
and (b) the callback handles missing provider state without 500.
CWE-61 / CWE-22 in file_manager: check_access_permission used
os.path.abspath, which resolves '..' but not symbolic links, while
the subsequent kernel write follows symlinks. An authenticated user
could plant a symlink inside their storage area pointing outside it
and write to any path the pgAdmin process could reach.
Fix: switch to os.path.realpath for both orig_path and in_dir, and
add a new _open_upload_target helper that opens with O_NOFOLLOW
(and mode 0o600) to close the leaf-component TOCTOU between the
access check and the open. Drops the redundant post-write
check_access_permission call in add().
Mode change for uploaded files (0o644 -> 0o600) is intentional
hardening; release notes will call this out.
Tests: 16 file-manager security tests covering realpath enforcement
on all five access-check consumers and O_NOFOLLOW leaf-symlink
rejection.
The shared design proposal lives in docs/proposals/ and was added
in the preceding pickle-RCE commit.
Reported-by: Fernando Bortotti <fernando.bortotti@bsd.com.br>
CWE-502 in FileBackedSessionManager.get(): pickle.load was called on
the session file before the HMAC integrity check. Any file dropped
in the sessions directory was deserialized unconditionally, allowing
an authenticated user with sessions-dir write access to achieve
OS-level RCE.
Fix: prepend a 64-byte hex SHA-256 HMAC header over the pickle body,
computed with the SECRET_KEY, and verify it via hmac.compare_digest
before any deserialization. Also raises (not asserts) on empty
SECRET_KEY so -O does not strip the check, and narrows the
post-deserialize except clause to surface programming errors instead
of masking them.
Tests: 13 new session-format tests covering round-trip, malicious-
pickle rejection, header tampering, empty/truncated files,
cookie-HMAC mismatch, unsafe sids, empty SECRET_KEY, MFA-shaped data,
and SERVER_MODE=False direct upload.
Includes docs/proposals/2026-04-30-eliminate-rce-and-symlink-escape.md,
the shared design doc covering this fix and the symlink-traversal fix
in the following commit.
Reported-by: Fernando Bortotti <fernando.bortotti@bsd.com.br>
User-supplied api_key_file and api_url preferences fed pgAdmin's LLM
provider clients without validation. An authenticated user could read
arbitrary server-side files (LFI) or coerce pgAdmin into requesting
internal targets such as 169.254.169.254 (SSRF) via the chat path and
model-list endpoints.
- validate_api_key_path() restricts user-supplied paths to the user's
private storage directory in server mode (covering both old- and
new-style names) or the home directory in desktop mode; resolves
symlinks and rejects null bytes. Shared storage is intentionally
excluded since API keys are per-user secrets.
- _read_api_key_from_file() caps reads at 1024 bytes and enforces a
printable-ASCII no-whitespace key shape so it cannot be repurposed
as an arbitrary file reader.
- validate_api_url() enforces config.ALLOWED_LLM_API_URLS by exact
scheme://host:port match, applied at refresh endpoints, accessor
fallbacks, and provider client constructors so the chat path is
also covered. Logs a startup warning if the allowlist is empty.
- Adds test coverage for path validation, URL validation, refresh-
endpoint rejection paths, and refresh-endpoint happy paths.
Reported-by: j3seer <jasserchebbi@outlook.com>
User-supplied input was interpolated directly into a psql \copy
metacommand template without sanitization. An authenticated pgAdmin
user could inject ") TO PROGRAM 'cmd'" to achieve RCE on the pgAdmin
server, or ") TO '/path'" for arbitrary file write, by breaking out
of the \copy (...) context.
Add _is_query_parens_balanced() which tracks parenthesis depth in the
query, modeled on psql's strtokx tokenizer used by parse_slash_copy.
Only single-quoted strings ('...' with '' and \' escaping) and
double-quoted identifiers ("..." with "" escaping) are recognized;
line/block comments and dollar-quoting are deliberately NOT skipped
because psql's \copy parser does not recognize them either, so any
unbalanced ')' inside those constructs must remain visible to the
validator.
Whitelist-validate format, on_error, and log_verbosity which were
also raw-interpolated, and write the normalized lowercase value back
to data so downstream template equality checks (e.g. data.format ==
'csv') match regardless of input case.
Reject queries containing null bytes (which could cause a C-string
truncation mismatch between Python validation and psql execution)
and normalize \r\n / \r / \n to spaces (required for Windows; also
prevents psql metacommand termination).
Type-check 'query' before string operations so malformed payloads
return a clean 400 instead of a 500. Tighten is_query_export gating
to 'is True' in both update_data_for_import_export and the validation
block so a client cannot exploit a truthy-but-not-True value to
bypass one check while satisfying the other.
Add unit tests covering the parens parser (balanced cases, RCE
attempts, escape edge cases) and route-level tests asserting
BatchProcess is not invoked for malicious payloads.
Reported-by: chungkn (Chung Kim), OneMount Group <kimngocchung.k2a@gmail.com>
The Maintenance tool concatenated four user-supplied JSON fields
(buffer_usage_limit, vacuum_parallel, vacuum_index_cleanup,
reindex_tablespace) directly into the rendered VACUUM/ANALYZE/REINDEX
command, which was passed to psql --command. An authenticated user
with the tools_maintenance permission could break out of the option
syntax and execute arbitrary SQL on the connected server, escalating
to RCE on the database host via COPY ... TO PROGRAM.
Fix:
- Allow-list values server-side before rendering: vacuum_index_cleanup
in {AUTO, ON, OFF}; vacuum_parallel a non-negative integer up to
1024; buffer_usage_limit matching ^\d+\s*(kB|MB|GB|TB)$
(case-insensitive). Reject with HTTP 400 on failure.
- Switch reindex_tablespace from manual double-quote wrapping to the
qtIdent filter used elsewhere in the same template, which both
escapes embedded quotes and only quotes when required.
- Type-guard against unhashable values so non-string payloads return
400 instead of triggering a 500.
Reported-by: j3seer <jasserchebbi@outlook.com>
The reverse-engineered SQL tests hardcoded 'postgres' as the role name
in test JSON data and expected SQL files. This fails on systems where
the superuser is not 'postgres' (e.g. 'ashesh.vashi').
Changes:
- Replace hardcoded 'postgres' with '<OWNER>' placeholder in ~400 test
JSON, SQL, and MSQL files across all browser module tests
- Extend test_resql.py to resolve '<OWNER>' in scenario data sent to
API endpoints (not just in expected SQL)
- Handle identifier quoting for usernames containing special characters
(dots, etc.) by normalizing both expected and actual SQL
- Fix test_domain_sql.py to use qtIdent for username in expected SQL
- Split Authlib version pin: 1.6.x for Python <=3.9, 1.7.x for >3.9
Add passexec_cmd, passexec_expiration, kerberos_conn, tags, and
post_connection_sql to SharedServer so non-owners get their own
per-user values instead of inheriting the owner's. Drop the unused
db_res column which was never overlaid or writable by non-owners.
Key changes:
- New Alembic migration (sharedserver_feature_parity) adds 5 columns,
drops db_res, cleans up orphaned records. All operations idempotent.
- Overlay copies new fields from SharedServer instead of suppressing
- _owner_only_fields guard blocks non-owners from setting passexec_cmd,
passexec_expiration, db_res, db_res_type via API
- Non-owners can set post_connection_sql (runs under their own creds)
- update_tags and flag_modified use sharedserver for non-owners
- update() response returns sharedserver tags for non-owners
- ServerManager passexec suppression with config.SERVER_MODE guard
- UI: post_connection_sql editable for non-owners (readonly only when
connected, not when shared)
- SCHEMA_VERSION bumped to 51
- Comprehensive unit tests for overlay, write guards, and tag deltas
Add support for custom container user permissions via PUID and PGID
environment variables. When the container is started as root
(--user root), the pgadmin user is reassigned to the requested UID/GID
and all initialization runs under that user via su-exec, ensuring
files are created with correct ownership from the start.
Key changes:
- Dockerfile: add su-exec package, add chmod g=u for /run/pgadmin
(fixes OpenShift random UID access)
- entrypoint.sh: add PUID/PGID validation and privilege dropping
before initialization (not after), preserving OpenShift compatibility
Three modes supported:
- Default (USER 5050): unchanged behavior
- Custom UID (--user root -e PUID=N -e PGID=N): drops to target user
before any init
- OpenShift (random UID, GID 0): passwd fixup + group permissions
Ashesh Vashi
rx500
Daniel Nylander
MichalBartos
Degit22
Aditya Toshniwal