Skip to content

Logging: include config-DB path/URI in migration failure messages (refs #9976) #10012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
asheshv wants to merge 1 commit into
base: master
Choose a base branch
from
+24 −4
Open

Logging: include config-DB path/URI in migration failure messages (refs #9976) #10012

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions web/pgadmin/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,24 @@ def get_locale():
from config import SQLITE_PATH
from pgadmin.setup import db_upgrade

def _describe_config_db():
"""
Return a human-readable identifier of the configuration database
currently in use, for inclusion in error logs. Passwords in an
external CONFIG_DATABASE_URI are redacted.
"""
if config.CONFIG_DATABASE_URI is not None and \
len(config.CONFIG_DATABASE_URI) > 0:
# Redact any password in the URI before logging.
import re
safe_uri = re.sub(
r'(://[^:/@]+:)([^@]+)(@)',
r'\1***\3',
config.CONFIG_DATABASE_URI
)
return 'external config DB: {0}'.format(safe_uri)
Comment on lines +375 to +380

@coderabbitai coderabbitai Bot Jun 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Redaction misses secret-bearing query parameters before logging URI

Current masking only handles user:password@ and can still leak credentials/tokens passed via query params or fragments in CONFIG_DATABASE_URI. Since this string is emitted on migration failures, sanitize those parts too before logging.

Suggested hardening 
 def _describe_config_db():
@@
-            safe_uri = re.sub(
+            safe_uri = re.sub(
                 r'(://[^:/@]+:)([^@]+)(@)',
                 r'\1***\3',
                 config.CONFIG_DATABASE_URI
             )
+            # Redact common sensitive query params if present.
+            safe_uri = re.sub(
+                r'([?&](?:password|passwd|pwd|access_token|token)=)[^&#]*',
+                r'\1***',
+                safe_uri,
+                flags=re.IGNORECASE
+            )
             return 'external config DB: {0}'.format(safe_uri)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/pgadmin/__init__.py` around lines 375 - 380, The current redaction that
builds safe_uri from config.CONFIG_DATABASE_URI only masks the user:password@
portion (via safe_uri and the regex) but can still leak secrets in query
parameters or fragments; update the sanitization to parse CONFIG_DATABASE_URI
(e.g., with a URL parser equivalent used in the repo) and remove or mask the
query and fragment parts before formatting the log: ensure safe_uri is
constructed from the parsed components with query and fragment set to empty (or
replace each query value with '***') and continue to mask the userinfo portion
as before so that CONFIG_DATABASE_URI never emits credentials or tokens in query
params/fragments when returned.

return 'SQLite config DB at: {0}'.format(SQLITE_PATH)

def backup_db_file():
"""
Create a backup of the current database file
Expand All @@ -378,7 +396,8 @@ def backup_db_file():
'CORRUPTED_DB_BACKUP_FILE'] = backup_file_name
app.logger.info('Database migration completed.')
except Exception:
app.logger.error('Database migration failed')
app.logger.error(
'Database migration failed (%s)', _describe_config_db())
app.logger.error(traceback.format_exc())
raise RuntimeError('Migration failed')

Expand All @@ -390,16 +409,17 @@ def upgrade_db():
db_upgrade(app)
os.environ['CORRUPTED_DB_BACKUP_FILE'] = ''
except Exception:
app.logger.error('Database migration failed')
app.logger.error(
'Database migration failed (%s)', _describe_config_db())
app.logger.error(traceback.format_exc())
backup_db_file()

# check all tables are present in the db.
is_db_error, invalid_tb_names = check_db_tables()
if is_db_error:
app.logger.error(
'Table(s) {0} are missing in the'
' database'.format(invalid_tb_names))
'Table(s) {0} are missing in the database ({1})'.format(
invalid_tb_names, _describe_config_db()))
backup_db_file()

def run_migration_for_sqlite():
Expand Down
Loading