Fix: Windows: downloaded mkcert binary saved without .exe extension - #119
Open
pullapprove5[bot] wants to merge 1 commit into
Open
Fix: Windows: downloaded mkcert binary saved without .exe extension#119pullapprove5[bot] wants to merge 1 commit into
pullapprove5[bot] wants to merge 1 commit into
Conversation
Reproduced the finding with a new unit test (plain-dev/tests/public/test_mkcert.py) that mocks platform.system() to \"Windows\" and drives MkcertManager.setup_mkcert() through its download path (stubbing the actual network download and CA-install subprocess calls to keep the test hermetic): before the fix, the resulting mkcert_bin path was the extensionless \"mkcert\", confirming the finding. Fixed plain-dev/plain/dev/mkcert.py by choosing \"mkcert.exe\" as the binary filename when platform.system() == \"Windows\" (else \"mkcert\"), mirroring the exact pattern already used in plain-tailwind/plain/tailwind/core.py and plain-code/plain/code/oxc.py for their own cached binaries. Re-ran the test after the fix and it passed. Ran the full plain-dev package test suite (146 tests, via `cd plain-dev/tests && uv run --isolated --package plain-dev --with 'psycopg[binary]' python -m pytest`, since scripts/test requires Docker/Postgres which isn't available in this sandbox) — all 146 passed, no regressions. Also ran `./scripts/fix plain-dev` (ruff + oxlint/oxfmt + prettier) with no issues. Committed the fix and test as a single commit (9b2f03c2f6). Note: uv wasn't preinstalled in this sandbox, so I installed it via `pip install --user uv` to run the checks — this is a sandbox-environment detail with no effect on the change itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reproduced the finding with a new unit test (plain-dev/tests/public/test_mkcert.py) that mocks platform.system() to "Windows" and drives MkcertManager.setup_mkcert() through its download path (stubbing the actual network download and CA-install subprocess calls to keep the test hermetic): before the fix, the resulting mkcert_bin path was the extensionless "mkcert", confirming the finding. Fixed plain-dev/plain/dev/mkcert.py by choosing "mkcert.exe" as the binary filename when platform.system() == "Windows" (else "mkcert"), mirroring the exact pattern already used in plain-tailwind/plain/tailwind/core.py and plain-code/plain/code/oxc.py for their own cached binaries. Re-ran the test after the fix and it passed. Ran the full plain-dev package test suite (146 tests, via
cd plain-dev/tests && uv run --isolated --package plain-dev --with 'psycopg[binary]' python -m pytest, since scripts/test requires Docker/Postgres which isn't available in this sandbox) — all 146 passed, no regressions. Also ran./scripts/fix plain-dev(ruff + oxlint/oxfmt + prettier) with no issues. Committed the fix and test as a single commit (9b2f03c2f6). Note: uv wasn't preinstalled in this sandbox, so I installed it viapip install --user uvto run the checks — this is a sandbox-environment detail with no effect on the change itself.Opened by a PullApprove implementation run (implement-finding v4) for:
Merging this is what closes them as fixed.