From fb3b564e05b40edbb1a263bd58d17c96d4c46e2d Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 11 Aug 2026 13:39:52 -0700 Subject: [PATCH 1/7] Redact password column from CSV import logging and error output `UserItem.CSVImport.validate_file_for_import` and `_validate_import_line_or_throw` wrote the raw CSV line -- including the password column -- to any caller-supplied logger at INFO/DEBUG level, and the whole raw line was pushed into the `invalid_lines` list returned to callers when a row failed validation. Anyone using the sample logger config or forwarding logs to a centralized system would see clear-text passwords in the log stream. Changes: - `validate_file_for_import` logs only the username (column 0) at DEBUG, and calls a new `_redact_password_column` helper before appending an invalid row to the returned list. - `_validate_import_line_or_throw` masks the PASS column value as `***` before logging it. Other column values still logged as-is for debugging. - Both callers changed from INFO to DEBUG for these per-row messages; large imports were spamming operator-visible logs. - Two regression tests capture logs and returned invalid_lines to assert the secret never appears in either place, plus a positive assertion that a `***` masked value IS logged so a future refactor that just removes the log line entirely doesn't pass. Fixes #1829. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 4 ++ tableauserverclient/models/user_item.py | 29 ++++++++--- test/test_user_model.py | 69 +++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc1430d38..b19bbc0ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ ## Unreleased +* Security: `UserItem.CSVImport` no longer logs the password column when + validating a user-import CSV file. The password field was previously written + to any caller-supplied logger at DEBUG level, and the raw row was returned in + `validate_file_for_import`'s `invalid_lines` list unmasked. 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* diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 0ba1e8eb2..81c88928d 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -478,17 +478,29 @@ 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. + @staticmethod + def _redact_password_column(line: str) -> str: + trailing_newline = "\n" if line.endswith("\n") else "" + fields = line.rstrip("\n").split(",") + pass_index = UserItem.CSVImport.ColumnType.PASS.value + if len(fields) > pass_index: + fields[pass_index] = "***" + return ",".join(fields) + trailing_newline + # 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 +523,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 diff --git a/test/test_user_model.py b/test/test_user_model.py index 49e8dc25c..5e03bedc0 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -136,3 +136,72 @@ 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" + # 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" From 22934d4e2761f66f966235e0ae659dec7693ef09 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 17 Aug 2026 21:11:55 -0700 Subject: [PATCH 2/7] Preserve CRLF line endings when redacting the password column The original _redact_password_column stripped only \n before splitting on commas, so on a CRLF-terminated 2-column line ("user,pass\r\n") the \r rode with the password field, was thrown away when that field became "***", and the returned line silently downgraded to LF. Now the exact trailing sequence (\r\n, \n, or none) is preserved and reattached after the redaction. Adds a regression test for the CRLF-with-password-as-last-field case. Also corrects the CHANGELOG: the pre-fix code logged the first four characters of each raw row at INFO (not DEBUG), which is why the downgrade to DEBUG matters in addition to the redaction. --- CHANGELOG.md | 12 +++++++++--- tableauserverclient/models/user_item.py | 13 ++++++++++--- test/test_user_model.py | 3 +++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b19bbc0ca..ddd7a9810 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,15 @@ ## Unreleased * Security: `UserItem.CSVImport` no longer logs the password column when - validating a user-import CSV file. The password field was previously written - to any caller-supplied logger at DEBUG level, and the raw row was returned in - `validate_file_for_import`'s `invalid_lines` list unmasked. Fixes #1829. + 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* diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 81c88928d..d7aa4f96d 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -492,14 +492,21 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l # 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: - trailing_newline = "\n" if line.endswith("\n") else "" - fields = line.rstrip("\n").split(",") + 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) + trailing_newline + 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 diff --git a/test/test_user_model.py b/test/test_user_model.py index 5e03bedc0..ce81d5f7a 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -196,6 +196,9 @@ def test_redact_password_column_helper() -> None: 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) From 96b96d01d04c3c1f5145039120855abdd2b5ee10 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Wed, 9 Sep 2026 17:12:13 -0700 Subject: [PATCH 3/7] Never call _validate_attribute_value for PASS column Belt-and-braces: today PASS has an empty allowlist so the validator returns early, but if a future PR adds password-format checks (length, complexity, banned-chars) the raw password would leak via ValueError("Invalid value {item} for {column_type}"). Skip the call entirely for PASS so the invariant doesn't rely on the allowlist staying empty. Co-Authored-By: Claude Opus 4.7 (1M context) --- tableauserverclient/models/user_item.py | 7 ++++- test/test_user_model.py | 39 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 8de05f9b2..48b01d50d 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -605,7 +605,12 @@ def _validate_import_line_or_throw(incoming, logger) -> None: # Mask the password column so it never reaches log handlers. safe_value = "***" if column == UserItem.CSVImport.ColumnType.PASS else value logger.debug(f"column {column.name}: {safe_value}") - if not skip_validation: + # Never call _validate_attribute_value for PASS: its ValueError + # embeds the raw value, and a future PR that adds password-format + # checks to PASS's allowlist would leak the password through the + # exception message. Belt-and-braces even though PASS's allowlist + # is currently empty. + if not skip_validation and column != UserItem.CSVImport.ColumnType.PASS: UserItem.CSVImport._validate_attribute_value(value, valid, column) # Given a restricted set of possible values, confirm the item is in that set diff --git a/test/test_user_model.py b/test/test_user_model.py index 12464405e..e7992984d 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -222,6 +222,45 @@ def test_password_with_comma_partially_masks(caplog: pytest.LogCaptureFixture) - assert _mask_present(caplog.records) +def test_password_not_in_validate_attribute_value_error( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """Regression test: _validate_import_line_or_throw must never call + _validate_attribute_value for the PASS column. Today PASS has an empty + allowlist so the validator returns early, but if a future PR adds + password-format checks (length, complexity, banned chars) the raw password + would leak through ValueError("Invalid value {item} for {column_type}"). + Guarantee the invariant at the caller so it doesn't depend on the allowlist + staying empty.""" + secret = "hunter2SUPERSECRET" + line = f"jsmith,{secret},John Smith,creator,site,yes,jsmith@example.com" + + # Spy that records every call and simulates a future PR that added a + # non-empty allowlist to PASS (any value not in the allowlist raises). + calls: list[tuple] = [] + original = TSC.UserItem.CSVImport._validate_attribute_value + + def spy(item: str, possible_values: list, column_type) -> None: + calls.append((item, column_type)) + if column_type == TSC.UserItem.CSVImport.ColumnType.PASS: + # Simulate a hypothetical password-format check. + raise ValueError(f"Invalid value {item} for {column_type}") + return original(item, possible_values, column_type) + + monkeypatch.setattr(TSC.UserItem.CSVImport, "_validate_attribute_value", spy) + + with caplog.at_level(logging.DEBUG, logger=logger.name): + # If PASS were passed to the validator, the spy would raise with the + # raw secret embedded in the ValueError. It must not raise. + TSC.UserItem.CSVImport._validate_import_line_or_throw(line, logger) + + pass_calls = [c for c in calls if c[1] == TSC.UserItem.CSVImport.ColumnType.PASS] + assert not pass_calls, f"_validate_attribute_value was called for PASS: {pass_calls!r}" + + combined = "\n".join(record.getMessage() for record in caplog.records) + assert secret not in combined, f"Password leaked into logs: {combined!r}" + + 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 From 9d3e9f99a151c400a0e050fa169275b23bcdfeac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Oct 2026 05:36:11 +0000 Subject: [PATCH 4/7] Fix CSV-aware user password redaction Co-authored-by: bcantoni <58372+bcantoni@users.noreply.github.com> --- tableauserverclient/models/user_item.py | 22 +++++++++++--- test/test_user_model.py | 40 +++++++++++++++++-------- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 200c4dfe4..203fd17ca 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -1,3 +1,4 @@ +import csv import io import warnings import xml.etree.ElementTree as ET @@ -471,7 +472,7 @@ class ColumnType(IntEnum): def create_user_from_line(line: str): if line is None or line is False or line == "\n" or line == "": return None - values: list[str] = list(map(str.strip, line.strip().split(","))) + values = UserItem.CSVImport._parse_csv_line(line) if len(values) > UserItem.CSVImport.COLUMN_COUNT: raise ValueError("Too many attributes for user import") username = values[UserItem.CSVImport.ColumnType.USERNAME] @@ -521,6 +522,13 @@ def create_user_from_line(line: str): ) return user + @staticmethod + def _parse_csv_line(line: str) -> list[str]: + values = next(csv.reader([line], strict=True), []) + if not values: + values = [""] + return list(map(str.strip, values)) + # Read through an entire CSV file meant for user import # Return the number of valid lines and a list of all the invalid lines @staticmethod @@ -554,11 +562,17 @@ def _redact_password_column(line: str) -> str: body, ending = line[:-1], "\n" else: body, ending = line, "" - fields = body.split(",") + try: + fields = UserItem.CSVImport._parse_csv_line(body) + except csv.Error: + return "" + ending pass_index = UserItem.CSVImport.ColumnType.PASS.value if len(fields) > pass_index: fields[pass_index] = "***" - return ",".join(fields) + ending + output = io.StringIO() + csv.writer(output, lineterminator="").writerow(fields) + return output.getvalue() + ending + return line # Some fields in the import file are restricted to specific values # Iterate through each field and validate the given value against hardcoded constraints @@ -577,7 +591,7 @@ def _validate_import_line_or_throw(incoming, logger) -> None: list(UserItem.CSVImport._AUTH_CANONICAL.values()), # auth - normalized before comparison ] - line = list(map(str.strip, incoming.split(","))) + line = UserItem.CSVImport._parse_csv_line(incoming) if len(line) > UserItem.CSVImport.COLUMN_COUNT: raise ValueError("Too many attributes for user import") username = line[UserItem.CSVImport.ColumnType.USERNAME.value] diff --git a/test/test_user_model.py b/test/test_user_model.py index e7992984d..44095985e 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -203,25 +203,33 @@ def test_password_not_logged_when_line_invalid(caplog: pytest.LogCaptureFixture) 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" +def test_password_with_comma_is_fully_redacted(caplog: pytest.LogCaptureFixture) -> None: + """A quoted comma in a password must not leak from invalid row output or logs.""" + line = 'jsmith,"hunter2,SECRETTAIL",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): - try: - TSC.UserItem.CSVImport._validate_import_line_or_throw(line, logger) - except Exception: - pass # misaligned columns are expected to fail validation + valid, invalid = TSC.UserItem.CSVImport.validate_file_for_import(test_data, logger) + assert valid == 0 + assert len(invalid) == 1 + assert "hunter2" not in invalid[0] + assert "SECRETTAIL" not in invalid[0] 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 "SECRETTAIL" not in combined assert _mask_present(caplog.records) +def test_malformed_csv_row_is_not_returned_unredacted(caplog: pytest.LogCaptureFixture) -> None: + secret = "SECRETTAIL" + test_data = _mock_file_content([f'jsmith,"{secret}']) + 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 invalid == [""] + combined = "\n".join(record.getMessage() for record in caplog.records) + assert secret not in combined + + def test_password_not_in_validate_attribute_value_error( caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -310,6 +318,12 @@ def test_create_user_with_auth_column() -> None: assert user.auth_setting == "SAML", f"Expected SAML, got {user.auth_setting}" +def test_create_user_with_comma_in_password() -> None: + user = TSC.UserItem.CSVImport.create_user_from_line('username,"pass,word",fname,creator,none,yes,email') + assert user is not None + assert user.fullname == "fname" + + def test_too_many_columns_raises() -> None: with pytest.raises(ValueError): TSC.UserItem.CSVImport.create_user_from_line("u, p, n, creator, none, yes, email, SAML, extra") From 3615baf54fa8e58c64d40729efcfc93c3f56ad5b Mon Sep 17 00:00:00 2001 From: Brian Cantoni Date: Tue, 13 Oct 2026 22:46:05 -0700 Subject: [PATCH 5/7] Refactor password logging for UserItem Update password logging to mask sensitive data correctly. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tableauserverclient/models/user_item.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 203fd17ca..42639333f 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -625,8 +625,9 @@ def _validate_import_line_or_throw(incoming, logger) -> None: skip_validation = True else: value = value.lower() - # Mask the password column so it never reaches log handlers. - safe_value = "***" if column == UserItem.CSVImport.ColumnType.PASS else value + # A malformed row can shift password fragments into later columns; + # keep every field after the username out of the logs. + safe_value = value if column.value < UserItem.CSVImport.ColumnType.PASS.value else "***" logger.debug(f"column {column.name}: {safe_value}") # Never call _validate_attribute_value for PASS: its ValueError # embeds the raw value, and a future PR that adds password-format From 33361af2801f0c76defed4421b557a18307ec70a Mon Sep 17 00:00:00 2001 From: Jac Date: Thu, 15 Oct 2026 15:23:56 -0700 Subject: [PATCH 6/7] Fix CSV parsing by skipping initial spaces Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tableauserverclient/models/user_item.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 42639333f..be546fb98 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -524,7 +524,7 @@ def create_user_from_line(line: str): @staticmethod def _parse_csv_line(line: str) -> list[str]: - values = next(csv.reader([line], strict=True), []) + values = next(csv.reader([line], strict=True, skipinitialspace=True), []) if not values: values = [""] return list(map(str.strip, values)) From 3a771b67c8e8ccf05284ddb546311a60cbbce13f Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 19 Oct 2026 15:02:15 -0700 Subject: [PATCH 7/7] Remove duplicate ColumnType assignment, add _is_sensitive_column helper _validate_import_line_or_throw assigned `column` twice in the loop body (leftover from the redaction diff). Also had its own inline PASS-boundary check (`column.value < PASS.value`) duplicating the same boundary logic _redact_password_column derives independently via `ColumnType.PASS.value`. Factor both into one _is_sensitive_column helper so there's a single source of truth for "is this column sensitive enough to mask." --- tableauserverclient/models/user_item.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index be546fb98..2d0f0bff4 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -550,6 +550,16 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l line = csv_file.readline() return num_valid_lines, invalid_lines + # Single source of truth for "is this column (or, for a malformed row that + # shifted fields, anything at or after it) sensitive enough to mask before + # it reaches a log handler or an exposed invalid-lines entry." Both + # _redact_password_column (whole-line redaction) and + # _validate_import_line_or_throw (per-column debug logging) key off this + # instead of each re-deriving the PASS boundary independently. + @staticmethod + def _is_sensitive_column(column_index: int) -> bool: + return column_index >= UserItem.CSVImport.ColumnType.PASS.value + # 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 @@ -601,7 +611,6 @@ def _validate_import_line_or_throw(incoming, logger) -> None: column = UserItem.CSVImport.ColumnType(i) value = line[i] valid = _valid_attributes[i] - column = UserItem.CSVImport.ColumnType(i) # normalize case for fields with a restricted value set skip_validation = False if valid: @@ -626,8 +635,8 @@ def _validate_import_line_or_throw(incoming, logger) -> None: else: value = value.lower() # A malformed row can shift password fragments into later columns; - # keep every field after the username out of the logs. - safe_value = value if column.value < UserItem.CSVImport.ColumnType.PASS.value else "***" + # keep every field at or after PASS out of the logs. + safe_value = "***" if UserItem.CSVImport._is_sensitive_column(column.value) else value logger.debug(f"column {column.name}: {safe_value}") # Never call _validate_attribute_value for PASS: its ValueError # embeds the raw value, and a future PR that adds password-format