diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 57d200a2..f9abaa50 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2693,7 +2693,7 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s # Process parameters into column-wise format with possible type conversions # First, convert any Decimal types as needed for NUMERIC/DECIMAL columns processed_parameters = [] - for row in seq_of_parameters: + for row_index, row in enumerate(seq_of_parameters): processed_row = list(row) for i, val in enumerate(processed_row): if val is None: @@ -2715,12 +2715,32 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s if isinstance(val, decimal.Decimal): processed_row[i] = format(val, "f") else: + # Do not embed the parameter value or the full row in the + # message: rows may contain PII (SSNs, emails, balances) + # that would leak into caller error handlers, tracebacks, + # and log/APM stores. Report metadata only (row index, + # column index, value type). + err_msg = ( + f"Failed to convert parameter to Decimal at row " + f"{row_index}, column {i} (value type: {type(val).__name__})" + ) + # Split str(val) from the decimal parse so we only chain a + # cause we know is value-free. decimal.DecimalException + # messages (e.g. ConversionSyntax) never echo the input, so + # they are safe to preserve for debugging. str(val) itself + # or any other error could carry the value in its message + # and surface through __cause__ / formatted tracebacks, so + # those are re-raised with the chain suppressed (from None). + try: + val_text = str(val) + except Exception: # pylint: disable=broad-exception-caught + raise ValueError(err_msg) from None try: - processed_row[i] = format(decimal.Decimal(str(val)), "f") - except Exception as e: # pylint: disable=broad-exception-caught - raise ValueError( - f"Failed to convert parameter at row {row}, column {i} to Decimal: {e}" - ) from e + processed_row[i] = format(decimal.Decimal(val_text), "f") + except decimal.DecimalException as e: + raise ValueError(err_msg) from e + except Exception: # pylint: disable=broad-exception-caught + raise ValueError(err_msg) from None processed_parameters.append(processed_row) # Now transpose the processed parameters @@ -2729,14 +2749,15 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s # Get encoding settings encoding_settings = self._get_encoding_settings() - # Add debug logging + # Debug logging: emit batch metadata only. Never log parameter values or + # row representations here -- rows may contain PII (SSNs, emails, + # balances) that would leak into log files and APM/log shippers even + # though this is a DEBUG-level statement. Metadata (batch size, column + # count) is sufficient for diagnostics without exposing user data. logger.debug( - "Executing batch query with %d parameter sets:\n%s", + "Executing batch query with %d parameter sets (%d columns per row)", len(seq_of_parameters), - "\n".join( - f" {i+1}: {tuple(p) if isinstance(p, (list, tuple)) else p}" - for i, p in enumerate(seq_of_parameters[:5]) - ), # Limit to first 5 rows for large batches + len(parameters_type), ) ret = ddbc_bindings.SQLExecuteMany( diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 126ba735..a3ddaaa1 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -15,6 +15,7 @@ from pathlib import Path import time as time_module import decimal +import traceback from contextlib import closing import threading import mssql_python @@ -10462,7 +10463,12 @@ def test_setinputsizes_sql_decimal_null(db_connection): def test_setinputsizes_sql_decimal_unconvertible_value(db_connection): - """Test setinputsizes with SQL_DECIMAL raises ValueError for unconvertible values (GH-503).""" + """Test setinputsizes with SQL_DECIMAL raises ValueError for unconvertible values (GH-503). + + The raised message must be metadata-only: it reports the row index, column + index, and value type, but must NOT embed the offending value or the full + parameter row (which may contain PII such as SSNs/emails/balances). + """ cursor = db_connection.cursor() cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_bad") @@ -10471,15 +10477,184 @@ def test_setinputsizes_sql_decimal_unconvertible_value(db_connection): cursor.setinputsizes([(mssql_python.SQL_DECIMAL, 18, 2)]) - with pytest.raises(ValueError, match="Failed to convert parameter"): + sensitive_value = "123-45-6789" # stand-in for PII in the failing row + with pytest.raises(ValueError) as exc_info: cursor.executemany( "INSERT INTO #test_sis_dec_bad (Price) VALUES (?)", - [("not_a_number",)], + [(sensitive_value,)], + ) + + message = str(exc_info.value) + # Contract: metadata is present... + assert "Failed to convert parameter" in message + assert "row 0" in message + assert "column 0" in message + assert "str" in message # value type name + # ...and the sensitive value / raw row is NOT leaked into the message. + assert sensitive_value not in message + assert repr((sensitive_value,)) not in message # no repr of the parameter tuple + # ...nor into the chained cause or the fully formatted traceback, which + # is what tracebacks and APM/log shippers actually capture. + formatted = "".join( + traceback.format_exception( + type(exc_info.value), exc_info.value, exc_info.value.__traceback__ ) + ) + assert sensitive_value not in formatted finally: cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_bad") +def test_setinputsizes_sql_decimal_str_raises_no_leak(db_connection): + """A parameter whose str() raises must not leak the exception text (GH-503). + + Exception chaining (raise ... from e) can surface a value-bearing cause + through __cause__ and formatted tracebacks. For a value whose str() raises, + the chain must be suppressed so the metadata-only guarantee holds across + tracebacks and APM/log shippers, not just str(exc). + """ + cursor = db_connection.cursor() + + secret = "secret-987-65-4321" + + class ExplodingStr: + def __str__(self): + raise ValueError(secret) + + cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_explode") + try: + cursor.execute("CREATE TABLE #test_sis_dec_explode (Price DECIMAL(18,2))") + + cursor.setinputsizes([(mssql_python.SQL_DECIMAL, 18, 2)]) + + with pytest.raises(ValueError) as exc_info: + cursor.executemany( + "INSERT INTO #test_sis_dec_explode (Price) VALUES (?)", + [(ExplodingStr(),)], + ) + + # The metadata-only message must not carry the secret, and the chain + # must be suppressed so neither __cause__ nor the formatted traceback + # exposes it. + assert secret not in str(exc_info.value) + assert exc_info.value.__cause__ is None + formatted = "".join( + traceback.format_exception( + type(exc_info.value), exc_info.value, exc_info.value.__traceback__ + ) + ) + assert secret not in formatted + finally: + cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_explode") + + +def test_setinputsizes_sql_decimal_non_decimal_exception_no_leak(db_connection): + """Cover the non-DecimalException conversion branch with no value leak (GH-503). + + ``format(decimal.Decimal("1e999999999999999999"), "f")`` raises MemoryError + (not a decimal.DecimalException) quickly and deterministically, exercising + the branch that re-raises with the chain suppressed. The resulting + ValueError must be metadata-only: no chained cause, and the offending input + must be absent from both the message and the fully formatted traceback. + """ + cursor = db_connection.cursor() + + # A syntactically valid Decimal whose fixed-point expansion is astronomically + # large; format(..., "f") raises MemoryError rather than a DecimalException. + sensitive_value = "1e999999999999999999" + + cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_mem") + try: + cursor.execute("CREATE TABLE #test_sis_dec_mem (Price DECIMAL(18,2))") + + cursor.setinputsizes([(mssql_python.SQL_DECIMAL, 18, 2)]) + + with pytest.raises(ValueError) as exc_info: + cursor.executemany( + "INSERT INTO #test_sis_dec_mem (Price) VALUES (?)", + [(sensitive_value,)], + ) + + message = str(exc_info.value) + # Metadata-only message... + assert "Failed to convert parameter" in message + assert "row 0" in message + assert "column 0" in message + # ...no chained cause (the non-DecimalException branch suppresses it)... + assert exc_info.value.__cause__ is None + # ...and the input is absent from the message and formatted traceback. + assert sensitive_value not in message + formatted = "".join( + traceback.format_exception( + type(exc_info.value), exc_info.value, exc_info.value.__traceback__ + ) + ) + assert sensitive_value not in formatted + finally: + cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_mem") + + +def test_executemany_debug_log_no_parameter_values(db_connection): + """executemany() DEBUG logging must not emit parameter values or rows (GH-503). + + The batch-execution debug log previously dumped the first 5 full parameter + rows, leaking the same PII the exception path now redacts. This test enables + DEBUG capture, runs a successful batch of sensitive-looking values, and + asserts the values are absent from the logs while batch metadata is present + (the metadata assertion is a positive control proving capture is working, so + the absence assertions are meaningful rather than vacuous). + """ + import logging as _logging + import io + from mssql_python.logging import logger, driver_logger + + cursor = db_connection.cursor() + + # Values that stand in for PII; both insert successfully into an NVARCHAR + # column so execution reaches the batch debug-log statement. + ssn = "123-45-6789" + email = "jane.doe@example.com" + + log_stream = io.StringIO() + test_handler = _logging.StreamHandler(log_stream) + test_handler.setLevel(_logging.DEBUG) + + # Save state we mutate so the global logger is restored afterwards. + original_cached_level = logger._cached_level + original_driver_level = driver_logger.level + + cursor.execute("DROP TABLE IF EXISTS #test_dbg_no_pii") + try: + cursor.execute("CREATE TABLE #test_dbg_no_pii (Data NVARCHAR(50))") + + # Enable DEBUG: bypass the wrapper's cached-level gate and lower the + # underlying stdlib logger, then attach our capturing handler. + logger._cached_level = _logging.DEBUG + driver_logger.setLevel(_logging.DEBUG) + driver_logger.addHandler(test_handler) + + cursor.executemany( + "INSERT INTO #test_dbg_no_pii (Data) VALUES (?)", + [(ssn,), (email,)], + ) + + test_handler.flush() + log_contents = log_stream.getvalue() + + # Positive control: batch metadata is logged (proves capture works). + assert "Executing batch query with 2 parameter sets" in log_contents + # Redaction: no parameter value or row representation is emitted. + assert ssn not in log_contents + assert email not in log_contents + assert repr((ssn,)) not in log_contents + assert repr((email,)) not in log_contents + finally: + driver_logger.removeHandler(test_handler) + driver_logger.setLevel(original_driver_level) + logger._cached_level = original_cached_level + cursor.execute("DROP TABLE IF EXISTS #test_dbg_no_pii") + + def test_setinputsizes_sql_decimal_high_precision(db_connection): """Test setinputsizes with SQL_DECIMAL preserves full DECIMAL(38,18) precision (GH-503).""" cursor = db_connection.cursor()