fix(git): decode workbench text output as UTF-8 - #226
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87e9aa2d9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| full_command, | ||
| check=False, | ||
| capture_output=True, | ||
| encoding="utf-8" if text else None, |
There was a problem hiding this comment.
On POSIX filesystems, Git emits pathnames as their raw filesystem bytes rather than guaranteeing UTF-8. If a repository path is valid in the host locale but not UTF-8 (for example, caf\xe9 under a Latin-1 locale), rev-parse --show-toplevel now raises UnicodeDecodeError before git_worktree_context() can inspect the return code, whereas the previous locale-based decoding produced the correct path. Limit the UTF-8 override to Windows or decode pathname output in a way that preserves non-UTF-8 filesystem bytes.
Useful? React with 👍 / 👎.
| "from pathlib import Path", | ||
| "sys.path.insert(0, sys.argv[1])", | ||
| "import workbench_target as target", | ||
| "locale.getencoding = lambda: 'cp1252'", |
There was a problem hiding this comment.
This assignment does not reliably force subprocess to use cp1252: Python 3.10 (which the package explicitly supports) passes the special "locale" encoding directly to its text wrappers and does not have locale.getencoding, while Python 3.11+ ignores this function when UTF-8 mode is enabled. In either environment, removing the production encoding="utf-8" change can still leave this regression test green, so the test does not consistently exercise the failure it is intended to prevent. Launch the probe with a deterministically non-UTF-8 decoder or intercept the subprocess decoding path in a version-independent way.
Useful? React with 👍 / 👎.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
2 participants
Fixes #192
Summary
cp1252.Root cause
subprocess.run(..., text=True)used Python's locale encoding. On Windows systems with a non-UTF-8 code page, Git's UTF-8 worktree path was decoded into a different string, soPath.relative_to()incorrectly concluded that the scan target was outside its repository.Impact
Windows scans can inspect valid Git worktrees whose paths contain Cyrillic or other non-ASCII characters without requiring
PYTHONUTF8=1. POSIX behavior and byte-oriented Git commands are unchanged.Validation
pnpm dlx bun test --timeout 30000 ./tests-ts/workbench-canonical-paths.test.ts— 4 passed, 2 platform skipspnpm dlx bun test --timeout 30000 ./tests-ts/scan-recovery.test.ts— 16 passedpnpm exec tsc --noEmitpython -m py_compile sdk/typescript/_bundled_plugin/scripts/workbench_target.pypnpm exec prettier --check tests-ts/workbench-canonical-paths.test.tsgit diff --check