diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 49d63529..9787b1d3 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2647,7 +2647,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: @@ -2669,12 +2669,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 diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 6df79cb7..881a5a34 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -13,6 +13,7 @@ from datetime import datetime, date, time, timedelta, timezone import time as time_module import decimal +import traceback from contextlib import closing import threading import mssql_python @@ -10451,7 +10452,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") @@ -10460,15 +10466,77 @@ 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_high_precision(db_connection): """Test setinputsizes with SQL_DECIMAL preserves full DECIMAL(38,18) precision (GH-503).""" cursor = db_connection.cursor()