# Code Debt & Improvements

Technical debt and improvement opportunities discovered during development. Items here are not blocking but worth addressing in future passes.

## Security Hardening

### `containsDotDot` duplicated across packages
`handler.go` and `store.go` both define identical `containsDotDot` functions. Both only split on `/`, which is correct for the Mark Protocol but would miss `\` on Windows. Consider extracting to a shared `internal/pathutil` package and splitting on both separators.

### `parseVersionPath` uses `filepath.Split` on protocol paths
`handler.go:parseVersionPath` uses `filepath.Split`, which uses the OS separator. On Windows, this would silently break version path parsing (e.g., `FETCH /doc.md/v3`). Should use `path.Split` instead since protocol paths always use forward slashes.

### `findVersions` and `VerifyChain` bypass `resolve()`
Both `findVersions` and `VerifyChain` construct filesystem paths from request paths using `filepath.Join` without routing through `resolve()`. They rely on callers having already validated the path, but `CurrentVersion` and `VerifyChain` are exported methods that accept arbitrary strings. A future caller could bypass path validation. Consider adding `resolve()` calls or making these methods unexported.

## Resilience

### Stale `.tmp` files visible in directory listings
If the server crashes between `os.WriteFile` and `os.Rename` in `Store.Archive`, a `.tmp` file persists and would appear in directory listings via `ListDir`. Consider filtering names ending in `.tmp` in `ListDir`, alongside the existing `versions` and dot-file filters.

### `handleVersions` accesses `versions[0]` without length guard
`Store.Versions` returns `os.ErrNotExist` for empty version lists, so `handleVersions` never receives an empty slice today. But the contract doesn't guarantee this — a defensive `len(versions) == 0` check before accessing `versions[0]` would be safer.

## Code Quality

### Handler `Logger` nil fallback
`Handler.logger()` silently falls back to `slog.Default()` when `Logger` is nil. This masks misconfiguration and can cause noisy test output. Consider requiring `Logger` at construction time or initializing it in a constructor.