From 143dcac89ad4dc5064205ada7a673e0cf05f2f28 Mon Sep 17 00:00:00 2001 From: Sumit Sarabhai Date: Thu, 13 Aug 2026 23:15:56 +0530 Subject: [PATCH 1/3] FIX: Redact parameter values from executemany Decimal-conversion error (AB#47300) The executemany() Decimal/NUMERIC conversion path embedded the full parameter row into the raised ValueError, so every column value in a failing row (potentially PII such as SSNs, emails, or balances) leaked into caller error handlers, tracebacks, and log/APM stores even without DEBUG logging enabled. Report metadata only (row index, column index, value type). The original decimal error is preserved via exception chaining. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- mssql_python/cursor.py | 11 +++++++++-- tests/test_004_cursor.py | 22 +++++++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 49d63529..d6931639 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: @@ -2672,8 +2672,15 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s try: processed_row[i] = format(decimal.Decimal(str(val)), "f") except Exception as e: # pylint: disable=broad-exception-caught + # 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); the original + # decimal error is preserved via exception chaining. raise ValueError( - f"Failed to convert parameter at row {row}, column {i} to Decimal: {e}" + f"Failed to convert parameter to Decimal at row {row_index}, " + f"column {i} (value type: {type(val).__name__})" ) from e processed_parameters.append(processed_row) diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 6df79cb7..2fc7daff 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -10451,7 +10451,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,11 +10465,22 @@ 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 "(" not in message # no repr of the parameter tuple finally: cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_bad") From 43affe5560f35517158b3a91c40b359114f4e732 Mon Sep 17 00:00:00 2001 From: Sumit Sarabhai Date: Thu, 13 Aug 2026 23:22:25 +0530 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/test_004_cursor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 2fc7daff..21f7e926 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -10480,7 +10480,7 @@ def test_setinputsizes_sql_decimal_unconvertible_value(db_connection): 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 "(" not in message # no repr of the parameter tuple + assert repr((sensitive_value,)) not in message # no repr of the parameter tuple finally: cursor.execute("DROP TABLE IF EXISTS #test_sis_dec_bad") From 4d0c3dd8c9bfba322b2026df8470c4ef3ef58c44 Mon Sep 17 00:00:00 2001 From: Sumit Sarabhai Date: Thu, 13 Aug 2026 23:33:17 +0530 Subject: [PATCH 3/3] FIX: Suppress value-bearing exception cause in Decimal-conversion error (AB#47300) The prior redaction cleaned the outer ValueError message but chained the raw exception via 'from e'. For a parameter whose str() raises with content (or any exception echoing the input), that cause surfaces through __cause__ and traceback.format_exc(), which APM/log shippers capture -- defeating the metadata-only guarantee the threat model requires. Now only decimal.DecimalException (proven value-free, e.g. ConversionSyntax) is chained for debuggability; str(val) failures and other unexpected errors are re-raised with the chain suppressed (from None). Adds a test asserting the formatted traceback leaks neither the value nor a str()-raised secret. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- mssql_python/cursor.py | 37 ++++++++++++++++++---------- tests/test_004_cursor.py | 52 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index d6931639..9787b1d3 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2669,19 +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 - # 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); the original - # decimal error is preserved via exception chaining. - raise ValueError( - f"Failed to convert parameter to Decimal at row {row_index}, " - f"column {i} (value type: {type(val).__name__})" - ) 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 21f7e926..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 @@ -10481,10 +10482,61 @@ def test_setinputsizes_sql_decimal_unconvertible_value(db_connection): # ...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()