Skip to content

Commit 99ecf8a

Browse files
authored
fix: native Windows crash (os.O_DIRECTORY) — working dry-runs, fail-closed writes, Windows CI (#150)
fix: native Windows crash (os.O_DIRECTORY) — working dry-runs, fail-closed writes, Windows CI
2 parents 7040a2a + 32f21a8 commit 99ecf8a

24 files changed

Lines changed: 1230 additions & 132 deletions

.gitattributes

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Never translate line endings at checkout: frontmatter parsing requires exact
2+
# "---\n" and content hashes (templates, release manifests, approval plans)
3+
# must be byte-identical on every platform, including Windows checkouts.
4+
* -text

.github/workflows/test.yml

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,62 @@ jobs:
4949
git diff --exit-code
5050
test -z "$(git status --porcelain=v1 --untracked-files=all)"
5151
52+
windows-smoke:
53+
# The full suite exercises dirfd confinement, symlinks, fcntl, and bash —
54+
# deliberately unsupported on native Windows. This job runs the portable
55+
# surface (schema/package/contract validation plus the Windows
56+
# compatibility suite) as an explicit allowlist so a skip can never be
57+
# mistaken for a pass.
58+
name: windows-latest / Python 3.12 (portable surface)
59+
runs-on: windows-latest
60+
timeout-minutes: 10
61+
env:
62+
PYTHONDONTWRITEBYTECODE: '1'
63+
defaults:
64+
run:
65+
shell: bash
66+
67+
steps:
68+
- name: Disable line-ending translation before checkout
69+
# The runner image ships autocrlf=true globally; frontmatter parsing
70+
# and content hashing require byte-exact LF checkouts. Must run
71+
# before actions/checkout writes any file.
72+
run: git config --global core.autocrlf false
73+
74+
- name: Checkout
75+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
76+
77+
- name: Set up Python
78+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
79+
with:
80+
python-version: '3.12'
81+
82+
- name: Show runtime versions
83+
run: python --version
84+
85+
- name: Run portable-surface tests
86+
run: |
87+
set -eu
88+
for test_file in \
89+
tests/test_package_validation.py \
90+
tests/test_knowledge_contracts.py \
91+
tests/test_contracts.py \
92+
tests/test_benchmark_tools.py \
93+
tests/test_windows_compat.py; do
94+
echo "=== $test_file ==="
95+
python "$test_file"
96+
done
97+
98+
- name: Run portable validators
99+
run: |
100+
python scripts/claude-obsidian.py package validate
101+
python scripts/claude-obsidian.py contracts --check-only
102+
103+
- name: Prove tests did not mutate or create product files
104+
run: |
105+
git diff --exit-code
106+
test -z "$(git status --porcelain=v1 --untracked-files=all)"
107+
52108
release-safety:
53109
name: Reproducible release artifact
54110
runs-on: ubuntu-latest

CHANGELOG.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,50 @@ implementation record for older releases.
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Native Windows no longer crashes with `AttributeError: os.O_DIRECTORY` on
13+
every vault command. Read-only inspection and dry-runs (`transaction
14+
inspect`, `migrate`/`init`/`adopt`/`capture` previews) now work natively
15+
with real equivalent safety checks: path-based casefold-alias auditing,
16+
lstat-based vault identity, and symlink/junction rejection.
17+
- File reads in the transaction core now force binary mode (`O_BINARY`), so
18+
CRLF content is hashed byte-exactly on Windows instead of producing false
19+
`CONTENT_HASH_MISMATCH`/`EXPECTED_HASH_MISMATCH` failures.
20+
- CLI output is always UTF-8, fixing `UnicodeEncodeError` crashes when
21+
redirecting output containing non-ASCII titles on Windows (cp1252 consoles).
22+
- Retrieval no longer returns zero results for vaults with CRLF line endings:
23+
the chunker, index, and retriever now hash page bodies byte-identically
24+
(previously `read_text` newline normalization made every chunk look stale).
25+
- Under a held mutation lock, a vault whose on-disk name differs only by case
26+
from the spelling used on the command line is no longer misreported as its
27+
own portable alias on case-insensitive filesystems (APFS; the lock layer
28+
never runs on native Windows); the descriptor-pinned vault object is
29+
recognized as itself. Inspect-time auditing keeps the stricter shipped
30+
behavior, since an absent init root on a case-insensitive volume cannot
31+
distinguish itself from an alien sibling.
32+
1033
### Changed
1134

35+
- Vault mutation on hosts without directory-descriptor confinement (native
36+
Windows) is refused before any side effect with a new
37+
`UNSUPPORTED_PLATFORM` validation error (exit 2, previously a generic
38+
`LOCK_FAILED` exit 1 or a traceback), and `init --apply` no longer creates
39+
an abandoned empty vault directory on refused platforms.
40+
- New transaction write destinations are validated against
41+
portable-filesystem rules on every platform: Windows-reserved device names
42+
(`CON`, `NUL`, ...), `:<>|?*"` characters, and trailing dots/spaces are
43+
rejected with `UNPORTABLE_WRITE_PATH` so an approved plan means the same
44+
thing everywhere. Reads, retrieval indexing, and journal recovery of
45+
pre-existing files with such names are intentionally unaffected.
46+
- Degraded-mode path walks now reject Windows directory junctions and mount
47+
points in addition to symlinks.
48+
- File permission bits feeding plan hashes are normalized to `0o644` on
49+
Windows, keeping inspect output deterministic.
50+
- Added a `.gitattributes` that disables line-ending translation so checkouts
51+
hash identically on every platform.
52+
- CI gained a `windows-smoke` job exercising the portable Python surface and
53+
the new Windows compatibility suite on `windows-latest`.
1254
- Replaced the animated README hero with the selected static PNG cover while
1355
preserving the warm orbital style.
1456

README.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,15 @@ remain yours.
345345
- Bash for setup, optional extensions, and shell test suites
346346
- Git only for development, releases, or an explicit knowledge checkpoint
347347

348-
CI exercises Linux and macOS. On Windows, use WSL; native Windows and Git Bash
349-
are not current compatibility guarantees. Optional tools such as Obsidian CLI,
350-
Ollama, and defuddle are capability-detected and affect only their dependent
351-
workflow.
348+
CI exercises Linux and macOS, plus a native-Windows smoke job for the portable
349+
surface. On native Windows (including Git Bash), read-only inspection and
350+
dry-run commands work; vault writes require WSL and fail closed with an
351+
`UNSUPPORTED_PLATFORM` error otherwise. Approval hashes bind to the reviewing
352+
environment's filesystem identity, so run the dry-run review inside WSL when
353+
the apply will happen there — a natively produced `approved_plan_sha256`
354+
cannot be replayed from WSL. The bash setup scripts and shell test suites
355+
remain POSIX-only. Optional tools such as Obsidian CLI, Ollama, and defuddle
356+
are capability-detected and affect only their dependent workflow.
352357

353358
## Development and release
354359

claude_obsidian/capture.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
BUNDLE_SCHEMA,
3737
TransactionValidationError,
3838
_LockIdentityChanged,
39+
_PlatformConfinementUnavailable,
40+
_UNSUPPORTED_PLATFORM_MESSAGE,
3941
_lock_entry_matches,
4042
_open_lock_directory_at,
4143
_open_lock_parent_from_root_fd,
@@ -205,6 +207,10 @@ def _safe_runtime_dir(vault_root: Path) -> Path:
205207
create=True,
206208
)
207209
except OSError as exc:
210+
if isinstance(exc, _PlatformConfinementUnavailable):
211+
raise CaptureValidationError(
212+
"UNSUPPORTED_PLATFORM", _UNSUPPORTED_PLATFORM_MESSAGE
213+
) from exc
208214
if exc.errno in {errno.ELOOP, errno.ENOTDIR}:
209215
raise CaptureValidationError(
210216
"RUNTIME_SYMLINK",
@@ -1494,6 +1500,10 @@ def acquire(self) -> None:
14941500
try:
14951501
root_fd = _open_lock_root_fd(self.vault_root)
14961502
except OSError as exc:
1503+
if isinstance(exc, _PlatformConfinementUnavailable):
1504+
raise CaptureValidationError(
1505+
"UNSUPPORTED_PLATFORM", _UNSUPPORTED_PLATFORM_MESSAGE
1506+
) from exc
14971507
raise CaptureError(
14981508
"QUEUE_LOCK_FAILED", f"cannot pin capture vault root: {exc}"
14991509
) from exc

claude_obsidian/checkpoint.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
from .json_utils import parse_finite_json_float
2020
from .lint_engine import lint_vault
21-
from .paths import canonical
21+
from .paths import canonical, directory_open_flags, read_open_flags
2222
from .transaction import (
2323
RESULT_SCHEMA,
2424
MutationLock,
@@ -425,11 +425,7 @@ def read(self, name: str, *, label: str, code: str) -> dict[str, Any]:
425425
or before.st_size > MAX_CHECKPOINT_STATE_BYTES
426426
):
427427
raise CheckpointError(code, f"{label} is not a bounded regular file")
428-
descriptor = os.open(
429-
name,
430-
os.O_RDONLY | os.O_NOFOLLOW | getattr(os, "O_CLOEXEC", 0),
431-
dir_fd=self.operation_fd,
432-
)
428+
descriptor = os.open(name, read_open_flags(), dir_fd=self.operation_fd)
433429
opened = os.fstat(descriptor)
434430
if (opened.st_dev, opened.st_ino) != (before.st_dev, before.st_ino):
435431
raise CheckpointError(code, f"{label} changed before it could be read")
@@ -530,9 +526,7 @@ def other_pending(self) -> str | None:
530526
raise CheckpointError(
531527
"CORRUPT_CHECKPOINT", f"cannot list transaction state: {exc}"
532528
) from exc
533-
flags = (
534-
os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | getattr(os, "O_CLOEXEC", 0)
535-
)
529+
flags = directory_open_flags()
536530
for name in names:
537531
if name == self.operation_id:
538532
continue

claude_obsidian/cli.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
MutationLock,
4646
TransactionError,
4747
TransactionValidationError,
48+
_require_write_platform,
4849
apply_bundle,
4950
inspect_bundle,
5051
read_vault_regular,
@@ -810,6 +811,10 @@ def command_init(args: argparse.Namespace) -> int:
810811
}
811812
)
812813
return 0
814+
# Refuse unsupported write platforms before creating the destination
815+
# directory: a failed Init root is never path-deleted (see below), so the
816+
# check must precede the mkdir rather than surface later in apply_bundle.
817+
_require_write_platform()
813818
reviewed_plan = _require_approved_plan(args, destination, operation)
814819
approval = str(reviewed_plan["approval_sha256"])
815820
created = False
@@ -1218,7 +1223,27 @@ def build_parser() -> argparse.ArgumentParser:
12181223
return parser
12191224

12201225

1226+
def _force_utf8_stdio() -> None:
1227+
"""Emit UTF-8 regardless of the console code page.
1228+
1229+
On Windows, redirected stdout/stderr default to the locale encoding
1230+
(cp1252), so any non-ASCII page title crashes ``_emit``'s
1231+
``ensure_ascii=False`` output. ``newline=""`` keeps JSON output
1232+
byte-identical across platforms.
1233+
"""
1234+
1235+
for stream in (sys.stdout, sys.stderr):
1236+
reconfigure = getattr(stream, "reconfigure", None)
1237+
if reconfigure is None:
1238+
continue
1239+
try:
1240+
reconfigure(encoding="utf-8", newline="")
1241+
except (OSError, ValueError):
1242+
pass
1243+
1244+
12211245
def main(argv: Sequence[str] | None = None) -> int:
1246+
_force_utf8_stdio()
12221247
parser = build_parser()
12231248
args = parser.parse_args(argv)
12241249
try:

claude_obsidian/hook_adapter.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
VaultSelectionError,
1717
assert_within,
1818
canonical,
19+
is_name_surrogate,
1920
is_relative_to,
2021
resolve_vault_root,
2122
)
@@ -47,8 +48,13 @@ def _bounded_regular_bytes(root: Path, path: Path, limit: int) -> bytes | None:
4748
| getattr(os, "O_CLOEXEC", 0)
4849
| getattr(os, "O_NOFOLLOW", 0)
4950
)
51+
# O_BINARY keeps the Windows CRT from CRLF-translating reads at the
52+
# descriptor layer, which would corrupt content passed to hashing callers.
5053
file_flags = (
51-
os.O_RDONLY | getattr(os, "O_CLOEXEC", 0) | getattr(os, "O_NOFOLLOW", 0)
54+
os.O_RDONLY
55+
| getattr(os, "O_CLOEXEC", 0)
56+
| getattr(os, "O_NOFOLLOW", 0)
57+
| getattr(os, "O_BINARY", 0)
5258
)
5359
if os.name != "nt" and os.open in os.supports_dir_fd and hasattr(os, "O_DIRECTORY"):
5460
try:
@@ -83,7 +89,7 @@ def _bounded_regular_bytes(root: Path, path: Path, limit: int) -> bytes | None:
8389
metadata = cursor.lstat()
8490
except OSError:
8591
return None
86-
if stat.S_ISLNK(metadata.st_mode):
92+
if is_name_surrogate(metadata):
8793
return None
8894
try:
8995
descriptor = os.open(path, file_flags)

0 commit comments

Comments
 (0)