Redact password column from CSV import logging and error output

CHANGELOG.md

@@ -1,6 +1,16 @@
 
 ## Unreleased
 
+* Security: `UserItem.CSVImport` no longer logs the password column when
+  validating a user-import CSV file. Previously, `validate_file_for_import`
+  emitted the first four characters of each raw row at INFO (which could
+  include the beginning of the password when the username was short), and the
+  full raw row was pushed into the returned `invalid_lines` list unmasked; the
+  per-column log inside `_validate_import_line_or_throw` also wrote the
+  password value at DEBUG. Row-level logging is now DEBUG-only and logs the
+  username instead of a raw slice, and the password column is replaced with
+  `***` before any line reaches a log handler or the invalid-lines list.
+  Fixes #1829.
 * Added `Projects.get_by_path(path)` to look up a project by its slash-separated
   hierarchy path (e.g. `"Marketing/Q1 Reports"`). The walk is performed level by
   level using the REST API name filter, so a path with *n* components issues *n*

tableauserverclient/models/user_item.py

@@ -478,17 +478,36 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l
             csv_file.seek(0)  # set to start of file in case it has been read earlier
             line: str = csv_file.readline()
             while line and line != "":
+                # Log only the username (column 0); the rest of the line contains the password (column 1) and other PII.
+                username = line.partition(",")[0].strip()
                 try:
-                    # do not print passwords
-                    logger.info(f"Reading user {line[:4]}")
+                    logger.debug(f"Reading user {username}")
                     UserItem.CSVImport._validate_import_line_or_throw(line, logger)
                     num_valid_lines += 1
                 except Exception as exc:
-                    logger.info(f"Error parsing {line[:4]}: {exc}")
-                    invalid_lines.append(line)
+                    logger.debug(f"Error parsing user {username}: {exc}")
+                    invalid_lines.append(UserItem.CSVImport._redact_password_column(line))
                 line = csv_file.readline()
             return num_valid_lines, invalid_lines
 
+        # Return a copy of a raw CSV line with the password column replaced by "***".
+        # Callers that log or expose invalid rows will not disclose the credential.
+        # Preserves the original line ending (\r\n or \n) so log output and
+        # returned rows stay byte-identical apart from the redaction.
+        @staticmethod
+        def _redact_password_column(line: str) -> str:
+            if line.endswith("\r\n"):
+                body, ending = line[:-2], "\r\n"
+            elif line.endswith("\n"):
+                body, ending = line[:-1], "\n"
+            else:
+                body, ending = line, ""
+            fields = body.split(",")
+            pass_index = UserItem.CSVImport.ColumnType.PASS.value
+            if len(fields) > pass_index:
+                fields[pass_index] = "***"
+            return ",".join(fields) + ending
+
         # Some fields in the import file are restricted to specific values
         # Iterate through each field and validate the given value against hardcoded constraints
         @staticmethod
@@ -511,10 +530,11 @@ def _validate_import_line_or_throw(incoming, logger) -> None:
             logger.debug(f"> details - {username}")
             UserItem.validate_username_or_throw(username)
             for i in range(1, len(line)):
-                logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {line[i]}")
-                UserItem.CSVImport._validate_attribute_value(
-                    line[i], _valid_attributes[i], UserItem.CSVImport.ColumnType(i)
-                )
+                column = UserItem.CSVImport.ColumnType(i)
+                # Mask the password column so it never reaches log handlers.
+                safe_value = "***" if column == UserItem.CSVImport.ColumnType.PASS else line[i]
+                logger.debug(f"column {column.name}: {safe_value}")
+                UserItem.CSVImport._validate_attribute_value(line[i], _valid_attributes[i], column)
 
         # Given a restricted set of possible values, confirm the item is in that set
         @staticmethod

test/test_user_model.py

@@ -136,3 +136,75 @@ def test_validate_usernames_file() -> None:
     test_data = _mock_file_content(usernames)
     valid, invalid = TSC.UserItem.CSVImport.validate_file_for_import(test_data, logger)
     assert valid == 5, f"Exactly 5 of the lines were valid, counted {valid + len(invalid)}"
+
+
+def _mask_present(records: list) -> bool:
+    combined = "\n".join(record.getMessage() for record in records)
+    return "PASS" in combined and "***" in combined
+
+
+def test_password_not_logged_at_debug(caplog: pytest.LogCaptureFixture) -> None:
+    """Regression test for #1829: passwords must not appear in DEBUG logs."""
+    secret = "hunter2SUPERSECRET"
+    line = f"jsmith,{secret},John Smith,creator,site,yes,jsmith@example.com"
+    with caplog.at_level(logging.DEBUG, logger=logger.name):
+        TSC.UserItem.CSVImport._validate_import_line_or_throw(line, logger)
+    combined = "\n".join(record.getMessage() for record in caplog.records)
+    assert secret not in combined, f"Password leaked into logs: {combined!r}"
+    # Positive assertion: something references the PASS column and something is
+    # masked as ***, so a "fix" that only removed the log line would not pass.
+    assert _mask_present(caplog.records), f"Expected masked PASS log line; got: {combined!r}"
+
+
+def test_password_not_logged_when_line_invalid(caplog: pytest.LogCaptureFixture) -> None:
+    """Regression test for #1829: passwords must not appear when a row fails to validate."""
+    secret = "hunter2SUPERSECRET"
+    line = f"jsmith,{secret},John Smith,not-a-real-license,site,yes,jsmith@example.com"
+    test_data = _mock_file_content([line])
+    with caplog.at_level(logging.DEBUG, logger=logger.name):
+        valid, invalid = TSC.UserItem.CSVImport.validate_file_for_import(test_data, logger)
+    assert valid == 0
+    assert len(invalid) == 1
+    assert secret not in invalid[0], f"Password leaked into returned invalid_lines: {invalid[0]!r}"
+    combined = "\n".join(record.getMessage() for record in caplog.records)
+    assert secret not in combined, f"Password leaked into logs on invalid row: {combined!r}"
+
+
+def test_password_with_comma_partially_masks(caplog: pytest.LogCaptureFixture) -> None:
+    """A password containing commas is misaligned by the naive split parser: only the
+    portion that lands in column 1 gets masked. The remaining fragments still leak.
+    This documents the limitation — fully protecting passwords with embedded commas
+    requires a proper CSV parser — but confirms that the column-1 mask holds even
+    when the password value contains a comma."""
+    line = "jsmith,hunter2,SECRETTAIL,creator,site,yes,jsmith@example.com"
+    with caplog.at_level(logging.DEBUG, logger=logger.name):
+        try:
+            TSC.UserItem.CSVImport._validate_import_line_or_throw(line, logger)
+        except Exception:
+            pass  # misaligned columns are expected to fail validation
+    combined = "\n".join(record.getMessage() for record in caplog.records)
+    # Column 1 ("hunter2") is masked; the fragment that spilled into column 2
+    # ("SECRETTAIL") is not — this is the documented limitation.
+    assert "hunter2" not in combined
+    assert _mask_present(caplog.records)
+
+
+def test_redact_password_column_helper() -> None:
+    """Unit-level coverage for _redact_password_column across newline and edge cases."""
+    redact = TSC.UserItem.CSVImport._redact_password_column
+    # LF-terminated
+    assert redact("jsmith,hunter2,fname\n") == "jsmith,***,fname\n"
+    # CRLF-terminated (the \r rides with the last field, ending is preserved)
+    assert redact("jsmith,hunter2,fname\r\n") == "jsmith,***,fname\r\n"
+    # CRLF where password IS the last field: the \r must not be silently
+    # dropped when the password value is replaced.
+    assert redact("jsmith,hunter2\r\n") == "jsmith,***\r\n"
+    # No trailing newline
+    assert redact("jsmith,hunter2,fname") == "jsmith,***,fname"
+    # Empty password field: still replaced (unconditional mask)
+    assert redact("jsmith,,fname") == "jsmith,***,fname"
+    # Trailing comma with nothing after: column 1 exists as empty string, gets masked
+    assert redact("jsmith,") == "jsmith,***"
+    # Single column: no password to redact; return line unchanged
+    assert redact("jsmith") == "jsmith"
+    assert redact("jsmith\n") == "jsmith\n"