Task 4: Platform-illegal filename detection #4

New issue

Closed

opened 2026-09-10 17:14:59 +00:00 by Cordy · 1 comment

Cordy commented

2026-09-10 17:14:59 +00:00

Owner

Depends on Task 3.

Goal

Detect paths that cannot exist on the target platform, so the engine can skip and report them instead of failing the entire sync.

Why

A file legally created on Linux may be impossible to create on Windows. If one bad filename aborts the run, the whole folder stops syncing — an outage caused by one file.

Files

Create: internal/sync/names.go, internal/sync/names_test.go

Produces

type Platform int
const (
	PlatformPOSIX Platform = iota
	PlatformWindows
)
func IllegalOn(p string, plat Platform) error

Rules to implement

Illegal characters on Windows: < > : " | ? *
Trailing dot or space on any segment is illegal on Windows.
Reserved device names: CON PRN AUX NUL COM1–COM9 LPT1–LPT9 — reserved as the stem, with or without an extension, case-insensitively. So CON, con.txt and CON.tar.gz are reserved, but CONSOLE.txt is fine.
NUL byte is illegal on every platform.

Steps

Write the failing test:

package sync

import "testing"

func TestIllegalOnWindows(t *testing.T) {
	bad := []string{
		"a/CON", "a/con.txt", "a/PRN", "a/AUX", "a/NUL", "a/COM1", "a/LPT9",
		"a/what?.txt", "a/b<c>.txt", `a/b:c.txt`, "a/b|c.txt", "a/b*.txt",
		`a/b"c.txt`, "a/trailing.", "a/trailing ",
	}
	for _, p := range bad {
		if err := IllegalOn(p, PlatformWindows); err == nil {
			t.Errorf("IllegalOn(%q, Windows) = nil, want error", p)
		}
	}

	good := []string{"a/b.txt", "a/CONSOLE.txt", "a/comm1.txt", "a/b c.txt", "a/héllo.txt"}
	for _, p := range good {
		if err := IllegalOn(p, PlatformWindows); err != nil {
			t.Errorf("IllegalOn(%q, Windows) = %v, want nil", p, err)
		}
	}
}

func TestPOSIXAllowsWindowsIllegalNames(t *testing.T) {
	if err := IllegalOn("a/what?.txt", PlatformPOSIX); err != nil {
		t.Errorf("POSIX should allow %q, got %v", "a/what?.txt", err)
	}
}

func TestNulByteIllegalEverywhere(t *testing.T) {
	if err := IllegalOn("a/b\x00c", PlatformPOSIX); err == nil {
		t.Error("NUL byte must be illegal on POSIX too")
	}
}

Run it, confirm it fails.
Implement. Split on / and check each segment. The returned error text is shown to users, so make it say what is wrong and why — it becomes a skip reason in the UI.
Run tests, confirm they pass.
Commit: git commit -s -m "feat(sync): detect platform-illegal filenames"

Acceptance criteria

Every name in bad errors; every name in good does not.
CONSOLE.txt and comm1.txt are explicitly allowed — only exact reserved stems match.

Depends on Task 3. ## Goal Detect paths that cannot exist on the target platform, so the engine can skip and report them instead of failing the entire sync. ## Why A file legally created on Linux may be impossible to create on Windows. If one bad filename aborts the run, the whole folder stops syncing — an outage caused by one file. ## Files - Create: `internal/sync/names.go`, `internal/sync/names_test.go` ## Produces ```go type Platform int const ( PlatformPOSIX Platform = iota PlatformWindows ) func IllegalOn(p string, plat Platform) error ``` ## Rules to implement - **Illegal characters on Windows:** `< > : " | ? *` - **Trailing dot or space** on any segment is illegal on Windows. - **Reserved device names:** `CON PRN AUX NUL COM1`–`COM9` `LPT1`–`LPT9` — reserved as the *stem*, with or without an extension, case-insensitively. So `CON`, `con.txt` and `CON.tar.gz` are reserved, but `CONSOLE.txt` is fine. - **NUL byte** is illegal on every platform. ## Steps - [ ] **Write the failing test:** ```go package sync import "testing" func TestIllegalOnWindows(t *testing.T) { bad := []string{ "a/CON", "a/con.txt", "a/PRN", "a/AUX", "a/NUL", "a/COM1", "a/LPT9", "a/what?.txt", "a/b<c>.txt", `a/b:c.txt`, "a/b|c.txt", "a/b*.txt", `a/b"c.txt`, "a/trailing.", "a/trailing ", } for _, p := range bad { if err := IllegalOn(p, PlatformWindows); err == nil { t.Errorf("IllegalOn(%q, Windows) = nil, want error", p) } } good := []string{"a/b.txt", "a/CONSOLE.txt", "a/comm1.txt", "a/b c.txt", "a/héllo.txt"} for _, p := range good { if err := IllegalOn(p, PlatformWindows); err != nil { t.Errorf("IllegalOn(%q, Windows) = %v, want nil", p, err) } } } func TestPOSIXAllowsWindowsIllegalNames(t *testing.T) { if err := IllegalOn("a/what?.txt", PlatformPOSIX); err != nil { t.Errorf("POSIX should allow %q, got %v", "a/what?.txt", err) } } func TestNulByteIllegalEverywhere(t *testing.T) { if err := IllegalOn("a/b\x00c", PlatformPOSIX); err == nil { t.Error("NUL byte must be illegal on POSIX too") } } ``` - [ ] **Run it, confirm it fails.** - [ ] **Implement.** Split on `/` and check each segment. The returned error text is shown to users, so make it say *what* is wrong and *why* — it becomes a skip reason in the UI. - [ ] **Run tests, confirm they pass.** - [ ] **Commit:** `git commit -s -m "feat(sync): detect platform-illegal filenames"` ## Acceptance criteria - Every name in `bad` errors; every name in `good` does not. - `CONSOLE.txt` and `comm1.txt` are explicitly allowed — only exact reserved stems match.

Cordy added this to the phase-1-engine milestone

2026-09-10 17:14:59 +00:00

Cordy added the

data-integrity

engine

labels

2026-09-10 17:14:59 +00:00

Cordy commented

2026-09-10 20:37:25 +00:00

Author

Owner

Done

296de58 feat(sync): detect platform-illegal filenames
3c43c2b fix(sync): reject control characters and superscript device names on Windows

What was built

internal/sync/names.go: Platform (PlatformPOSIX, PlatformWindows) and IllegalOn(p string, plat Platform) error.
Per-/-segment checks on Windows: illegal characters < > : " | ? *, trailing dot/space, reserved device stems CON PRN AUX NUL COM1-9 LPT1-9 (exact-stem match, case-insensitive).
NUL byte rejected on every platform, checked before the platform branch.
Fix round added: C0 control characters (< 0x20), superscript COM¹/COM²/COM³/LPT¹/LPT²/LPT³ stems, and trailing-space-before-extension stem trimming ("CON .txt" → CON), per Microsoft's Naming Files, Paths, and Namespaces docs.

Tests

Issue's test file copied verbatim (TestIllegalOnWindows, TestPOSIXAllowsWindowsIllegalNames, TestNulByteIllegalEverywhere), plus TestIllegalOnWindowsDocumentedExtras for the fix round. Red→green shown for both rounds (compile-fail red on missing IllegalOn, then 9/9 red→green on the extras).
Whole-repo gate green: go vet ./..., go test -count=1 ./..., gofmt -l . clean; go.mod/go.sum untouched (only fmt/strings imported — arch guard stays green).
CI run #4 (id 2866): green on linux/arm64, Go 1.25.5 — http://192.168.10.245/Cordy/cairn-desktop/actions/runs/4

Acceptance criteria

"Every name in bad errors; every name in good does not." — met, TestIllegalOnWindows covers all 14 bad / 5 good cases.
"CONSOLE.txt and comm1.txt are explicitly allowed — only exact reserved stems match." — met, exact-key map lookup on the uppercased stem, no prefix/substring match.

Rulings

R12: commit subjects match the issue verbatim, git commit -s, with the Co-Authored-By: Claude Opus 5 trailer — DCO is non-negotiable.
CV1: TDD red/green independently reproduced against a clean clone of 296de58.
CV3: added the <0x20 control-character check and the superscript COM¹/²/³/LPT¹/²/³ stems plus trailing-space stem trim, per Microsoft's current naming docs (a missed name could otherwise write silently to the NUL/CON device and later read as a delete).
CV4: downstream handling (SyncOnce must call IllegalOn before Decide/FS/Remote, record exactly one Skip, never treat a skipped path as missing/delete-eligible) carried into #12, #13, #14.
CV2: CI-green-required-to-close (R11) — satisfied by run #4 above.

Deferred

Minor findings F3–F5 from the review (unknown Platform values fail open; illegal-character message doesn't name the offending character; tests don't pin CON.tar.gz/CON/b.txt/segment naming) were not in this round's ruling scope — deferred to the phase-1 final review.
windowsReservedStems doc comment (still describes the stem cut without mentioning the trailing-space trim) — deferred.

Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI.

**Done** - [`296de58`](http://192.168.10.245/Cordy/cairn-desktop/commit/296de581f6b58f8c808609e8c99f82d4de96bd0f) feat(sync): detect platform-illegal filenames - [`3c43c2b`](http://192.168.10.245/Cordy/cairn-desktop/commit/3c43c2b6ed8945f5b8747a753b88a2ec8bb2a439) fix(sync): reject control characters and superscript device names on Windows **What was built** - `internal/sync/names.go`: `Platform` (`PlatformPOSIX`, `PlatformWindows`) and `IllegalOn(p string, plat Platform) error`. - Per-`/`-segment checks on Windows: illegal characters `< > : " | ? *`, trailing dot/space, reserved device stems `CON PRN AUX NUL COM1-9 LPT1-9` (exact-stem match, case-insensitive). - NUL byte rejected on every platform, checked before the platform branch. - Fix round added: C0 control characters (`< 0x20`), superscript `COM¹/COM²/COM³/LPT¹/LPT²/LPT³` stems, and trailing-space-before-extension stem trimming (`"CON .txt"` → `CON`), per Microsoft's Naming Files, Paths, and Namespaces docs. **Tests** - Issue's test file copied verbatim (`TestIllegalOnWindows`, `TestPOSIXAllowsWindowsIllegalNames`, `TestNulByteIllegalEverywhere`), plus `TestIllegalOnWindowsDocumentedExtras` for the fix round. Red→green shown for both rounds (compile-fail red on missing `IllegalOn`, then 9/9 red→green on the extras). - Whole-repo gate green: `go vet ./...`, `go test -count=1 ./...`, `gofmt -l .` clean; `go.mod`/`go.sum` untouched (only `fmt`/`strings` imported — arch guard stays green). - CI run **#4** (id 2866): green on **linux/arm64**, Go 1.25.5 — http://192.168.10.245/Cordy/cairn-desktop/actions/runs/4 **Acceptance criteria** - "Every name in `bad` errors; every name in `good` does not." — met, `TestIllegalOnWindows` covers all 14 `bad` / 5 `good` cases. - "`CONSOLE.txt` and `comm1.txt` are explicitly allowed — only exact reserved stems match." — met, exact-key map lookup on the uppercased stem, no prefix/substring match. **Rulings** - R12: commit subjects match the issue verbatim, `git commit -s`, with the `Co-Authored-By: Claude Opus 5` trailer — DCO is non-negotiable. - CV1: TDD red/green independently reproduced against a clean clone of 296de58. - CV3: added the `<0x20` control-character check and the superscript `COM¹/²/³`/`LPT¹/²/³` stems plus trailing-space stem trim, per Microsoft's current naming docs (a missed name could otherwise write silently to the `NUL`/`CON` device and later read as a delete). - CV4: downstream handling (SyncOnce must call `IllegalOn` before Decide/FS/Remote, record exactly one Skip, never treat a skipped path as missing/delete-eligible) carried into #12, #13, #14. - CV2: CI-green-required-to-close (R11) — satisfied by run #4 above. **Deferred** - Minor findings F3–F5 from the review (unknown `Platform` values fail open; illegal-character message doesn't name the offending character; tests don't pin `CON.tar.gz`/`CON/b.txt`/segment naming) were not in this round's ruling scope — deferred to the phase-1 final review. - `windowsReservedStems` doc comment (still describes the stem cut without mentioning the trailing-space trim) — deferred. _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-10 20:37:29 +00:00