Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/google_meet/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def _confirm(prompt: str) -> bool:
elif system == "Darwin":
have_bh = False
try:
out = _sp.check_output(["system_profiler", "SPAudioDataType"], text=True)
out = _sp.check_output(["system_profiler", "SPAudioDataType"], text=True) # windows-footgun: ok — macOS only (system_profiler), guarded by platform check above
have_bh = "BlackHole" in out
except Exception:
pass
Expand Down
113 changes: 113 additions & 0 deletions scripts/check-windows-footguns.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,55 @@ class Footgun:
" pass # Windows asyncio doesn't support signal handlers"
),
),
Footgun(
name="subprocess text=True without explicit encoding=",
# Match ``text=True`` (or ``text = True``) anywhere on a line. We
# rely on the post_filter to (a) skip lines that already pass
# ``encoding=`` on the same line, and (b) skip false positives like
# ``def text(self, ...)`` or string literals. ``text=True`` is
# overwhelmingly a subprocess kwarg, so a bare match + filter has a
# high signal-to-noise ratio and avoids the complexity of parsing
# multi-line subprocess calls (which the line-based scanner can't
# reliably attribute to a single line anyway).
pattern=re.compile(r"\btext\s*=\s*True\b"),
message=(
"subprocess text=True without explicit encoding= decodes "
"child output with locale.getpreferredencoding() — cp936 "
"(GBK) on Chinese Windows, cp1252 on Western Windows — "
"which crashes _readerthread with UnicodeDecodeError on "
"non-default-codepage bytes. Always pass encoding='utf-8' "
"(and errors='replace' for Windows-native CLIs that emit "
"non-UTF-8). See issues #47939, #53428, #57238."
),
fix=(
"subprocess.run(..., text=True, encoding='utf-8', "
"errors='replace')\n"
"Both params are required: encoding alone still crashes on "
"non-UTF-8 bytes from Windows-native CLIs (tasklist, "
"schtasks)."
),
post_filter=lambda m, line: (
# Skip if the same line already specifies encoding=.
"encoding=" not in line
and "encoding =" not in line
# Skip method definitions named ``text`` (def text(self, ...)).
and not line.lstrip().startswith("def ")
and not line.lstrip().startswith("async def ")
# Skip ``text=True`` inside string literals (heuristic: the
# substring appears between matching quotes that aren't part
# of an f-string expression). This is imperfect but catches
# the common case of docstrings mentioning text=True.
and not _looks_like_string_literal(line, m)
# Skip lines that are obviously not subprocess calls — e.g.
# DataFrame.rename(text=True) or similar. We can't know for
# sure without parsing, so we accept some false negatives by
# only flagging when ``subprocess`` or a known subprocess-
# shaped call (run/Popen/call/check_output/check_call/
# check_output) appears on the same line. This keeps the
# rule focused on the actual footgun.
and _is_likely_subprocess_call(line)
),
),
]


Expand Down Expand Up @@ -408,6 +457,66 @@ def _find_unquoted_hash(line: str) -> int | None:
return None


# Subprocess method names that accept ``text=`` and are affected by the
# encoding-default footgun. Used by ``_is_likely_subprocess_call`` below to
# keep the ``text=True`` rule focused on subprocess calls (and avoid flagging
# unrelated APIs that happen to accept a ``text`` kwarg).
_SUBPROCESS_METHODS = (
"subprocess.run",
"subprocess.Popen",
"subprocess.call",
"subprocess.check_output",
"subprocess.check_call",
"_sp.run", # common alias
"_sp.Popen",
"_sp.check_output",
"_sp.check_call",
"_sp.call",
".run(", # bare .run( — usually subprocess.run
".Popen(",
".check_output(",
".check_call(",
".call(",
)


def _is_likely_subprocess_call(line: str) -> bool:
"""Heuristic: does this line look like a subprocess invocation?

The ``text=True`` footgun rule only fires when the matched line also
contains a subprocess-shaped call site. This avoids false positives on
unrelated APIs that accept a ``text`` kwarg (e.g. DataFrame.rename,
custom library calls). Multi-line calls where the ``subprocess.X(``
prefix is on a previous line won't be flagged — that's an acceptable
false negative for a line-based scanner.
"""
return any(token in line for token in _SUBPROCESS_METHODS)


def _looks_like_string_literal(line: str, match: "re.Match") -> bool:
"""Heuristic: is the ``text=True`` match inside a string literal?

Catches the common case of docstrings/comments that mention ``text=True``
as prose. Walks the line tracking single/double quote state and returns
True if the match start index falls inside a quoted region.
"""
start = match.start()
in_s = False
in_d = False
i = 0
while i < start and i < len(line):
c = line[i]
if c == "\\" and (in_s or in_d) and i + 1 < len(line):
i += 2
continue
if not in_d and c == "'":
in_s = not in_s
elif not in_s and c == '"':
in_d = not in_d
i += 1
return in_s or in_d


def scan_file(path: Path, footguns: list[Footgun]) -> list[tuple[int, str, Footgun]]:
"""Return a list of (line_number, line, footgun) for unsuppressed matches."""
try:
Expand Down Expand Up @@ -493,6 +602,8 @@ def get_staged_files() -> list[Path]:
cwd=REPO_ROOT,
stderr=subprocess.DEVNULL,
text=True,
encoding="utf-8",
errors="replace",
)
except (subprocess.CalledProcessError, FileNotFoundError):
return []
Expand All @@ -507,6 +618,8 @@ def get_diff_files(ref: str) -> list[Path]:
cwd=REPO_ROOT,
stderr=subprocess.DEVNULL,
text=True,
encoding="utf-8",
errors="replace",
)
except (subprocess.CalledProcessError, FileNotFoundError):
return []
Expand Down
Loading