Skip to content

fix(preflight): normalize deployment paths on Windows#9743

Open
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/windows-deployment-secret-paths
Open

fix(preflight): normalize deployment paths on Windows#9743
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/windows-deployment-secret-paths

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What changed and why

Normalize current-tree deployment paths to POSIX repository-relative form before comparing them with Git's base-tree paths. This prevents the deployment secret-boundary ratchet from treating every existing binding as new on native Windows. Add a host-independent PureWindowsPath regression test so Linux CI also enforces the separator contract. Closes #9738.

Product invariants affected

none

How it was verified

  • Reproduced the bug on Windows 10.0.26200 with Python 3.11.15: the existing 7-test fixture suite failed test_rejects_new_unclassified_binding_but_allows_legacy_baseline, and the production checker failed even with --base HEAD while reporting hundreds of unchanged bindings.
  • python3 .github/scripts/test_check_deployment_secret_boundary.py (8 tests passed after the fix)
  • python3 .github/scripts/check_deployment_secret_boundary.py --base HEAD
  • black --check --line-length 120 --skip-string-normalization .github/scripts/check_deployment_secret_boundary.py .github/scripts/test_check_deployment_secret_boundary.py
  • ruff check --target-version py39 .github/scripts/check_deployment_secret_boundary.py .github/scripts/test_check_deployment_secret_boundary.py
  • python3 .github/scripts/pr_preflight.py --lane local --base origin/main --head HEAD (10 selected checks passed)
  • bash scripts/pre-push fork (all fast pre-push checks passed on native Windows)

Tests

The new PureWindowsPath fixture verifies that production path normalization emits .github/workflows/deploy.yml, matching Git tree output. The existing temporary-repository fixture then exercises the complete current-vs-base binding comparison and now passes on Windows.

Root cause and durable guard (bug fixes)

_deployment_paths() converted Path.relative_to() with str(), producing backslashes on Windows, while _base_paths() consumes Git output with forward slashes. Since each Binding includes its path, identical bindings never compared equal and the diff ratchet collapsed into a full-repository scan. The shared path conversion now uses PurePath.as_posix() for every discovered current-tree file, and the Windows-path unit fixture keeps that cross-platform contract enforced in all CI environments.

Review in cubic

Current bindings used pathlib's native separators while Git base paths always used forward slashes, so the ratchet treated every binding as new on Windows. Emit POSIX repository-relative paths and cover the invariant with PureWindowsPath.

Closes BasedHardware#9738.
@tianmind-studio

Copy link
Copy Markdown
Contributor Author

Fresh Windows reproduction while preparing #9753 from current main: the shared PR preflight fed backslash paths into the deployment binding guard, so checked-in bindings were misclassified against the slash-normalized policy and the check failed across unrelated workflow entries. Re-running the focused validators directly passed. This confirms the normalization here fixes a current contributor workflow, not only a synthetic unit case.

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking this down — the actual path-normalization fix looks directionally right to me. I verified the focused regression on Linux with the PR head checked out:

  • python3 .github/scripts/test_check_deployment_secret_boundary.py passes (8 tests)
  • python3 .github/scripts/check_deployment_secret_boundary.py --base HEAD passes
  • uvx ruff check --target-version py39 ... passes

Requesting changes only because the touched Python files do not currently satisfy the repo's Black formatting command from the PR description:

uvx black --check --line-length 120 --skip-string-normalization \
  .github/scripts/check_deployment_secret_boundary.py \
  .github/scripts/test_check_deployment_secret_boundary.py

would reformat .github/scripts/test_check_deployment_secret_boundary.py
would reformat .github/scripts/check_deployment_secret_boundary.py

Black's diff is just removing the extra blank line after the from pathlib import ... import in both files, so this should be a tiny cleanup. After that, this is a good targeted fix for the Windows separator mismatch in the deployment secret-boundary preflight. Because this touches workflow/security preflight code, I’m leaving it for maintainer review rather than formal auto-approval.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added workflow-review Needs maintainer review for workflow, automation, hooks, or CI behavior needs-maintainer-review Needs a human maintainer to sign off before merge labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-maintainer-review Needs a human maintainer to sign off before merge workflow-review Needs maintainer review for workflow, automation, hooks, or CI behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deployment secret-boundary ratchet treats all bindings as new on Windows

2 participants