# Journal — 2026-03-05

## 2026-03-05 — Bookmarks: Escape to Exit

Short session. Fritz noticed there was no way to exit the bookmark screen in the TUI. Added `Escape` key handling in `handleViewportKey`:

- When `m.status == "bookmarks"` and there's history, `Escape` calls `restoreHistory()` (same as `[` back).
- When no history exists, clears all bookmarks-derived state (`links`, `rawBody`, `linkIdx`, `metadata`, `status`) and resets the viewport.
- Updated help text: `Esc` now reads "Exit bookmarks / dismiss help / blur address bar" to accurately describe all three behaviors.

Copilot review caught two issues:
1. Help text was too vague ("Back / dismiss overlay") — didn't match actual behavior. Fixed to be precise.
2. No-history exit branch left stale `links`/`rawBody`/`linkIdx` — Tab/Enter would still target old bookmark links after "exiting". Fixed by clearing all derived state.

All checks passing — lint clean, tests green.


---

## 2026-03-05 — Lint Fix: De Morgan's Law

Quick polish session. The frontmatter metadata work on `feat-frontmatter-metadata` was complete and all tests passing, but `golangci-lint` flagged a De Morgan's law violation in `isValidMetaKey`:

```go
if !((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-') {
    return false
}
```

Extracted a helper `isValidMetaChar(c rune)` to clarify the check:

```go
func isValidMetaChar(c rune) bool {
    return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-'
}

func isValidMetaKey(k string) bool {
    if k == "" {
        return false
    }
    for _, c := range k {
        if !isValidMetaChar(c) {
            return false
        }
    }
    return true
}
```

This is cleaner and reads more clearly: "for each character, check if it's valid." `bash pre-commit.sh` all green.

Ready to keep shipping.

## 2026-03-05 — Control Key Ordering: Security Fix

Copilot review on the frontmatter metadata feature caught a potential override vulnerability in the client library. The `Publish` and `Append` methods were setting control keys (auth, expected-version), then copying user-provided metadata *after*, which could overwrite them:

```go
// VULNERABLE: user meta can override control keys
req.Metadata["auth"] = token
req.Metadata["expected-version"] = strconv.Itoa(expectedVersion)
maps.Copy(req.Metadata, meta)  // ← could override auth/expected-version
```

Fixed by reversing the order — copy user metadata first, then set control keys last:

```go
// SAFE: control keys always take precedence
maps.Copy(req.Metadata, meta)
req.Metadata["auth"] = token
req.Metadata["expected-version"] = strconv.Itoa(expectedVersion)
```

Both `Publish` and `Append` methods updated. Tests passing, lint clean.

This is a good example of defense in depth. The server-side `extractPublisherMeta` function also filters these keys, so even if they got through, they'd be rejected. But catching it client-side is better — fails faster and fails obviously.


---

## 2026-03-05 — Frontmatter Metadata: Copilot Review Hardening

Continued on `feat-frontmatter-metadata`. This session was entirely Copilot review — roughly 15 comments across handler, store, protocol, client, and Makefile. The feature was functionally complete; this was about closing every edge case.

### What Changed

**Security hardening:**
- `copyPublisherMeta` now validates keys/values with `protocol.IsValidMetaKey`/`IsValidMetaValue` before copying stored metadata into responses. Prevents tampered on-disk files from injecting invalid metadata into FETCH responses.
- Client `Publish`/`Append` reordered: user meta copied first, then control keys (auth, expected-version) set last so they can't be overridden. (Previous session.)
- `serveDocument` replaced `maps.Copy(meta, doc.Metadata)` with `copyPublisherMeta` that filters reserved and control keys. (Previous session.)

**Validation consolidation:**
- Moved `IsValidMetaKey`, `IsValidMetaValue`, `MaxMetaKeys`, `MaxMetaBytes` to protocol package — single source of truth across handler, store, and client.
- Store `validateMeta` uses protocol constants for key/value validation and size/count limits.
- Handler `extractPublisherMeta` uses protocol validation.

**Performance:**
- `extractMetadata` now uses `bytes.HasPrefix`/`bytes.Index` to find frontmatter boundaries before converting to string. Only the small frontmatter block (~200 bytes) gets allocated as a string, not the entire document body (up to 1 MiB). Fritz pushed back on deferring this — under heavy use with large documents and high concurrency, the extra per-request allocation matters.

**Documentation:**
- Updated `Store.Write` doc comment to reflect current version file format (archived field always present, meta.* keys).
- Fixed archived field comment: `true|false` (always present), not "only when archived".

**Skipped (intentional design or already addressed):**
- Defense-in-depth duplicate validation in `Write`/`Append` + `buildVersionFile` — intentional, noted multiple times.
- `extractMetadata` not validating parsed keys — already handled by `copyPublisherMeta` on the response path.
- Metadata not inherited across versions — intentional per-version design.

### What I Learned

The review pattern that emerged: Copilot flags ~60% genuine issues and ~40% over-engineering suggestions. The genuine issues cluster around trust boundaries (what can disk-tampered data inject? what can user metadata override?) and documentation accuracy (comments drifting from implementation). The over-engineering suggestions typically want to add validation at every layer regardless of whether the data has already been validated upstream.

The right response to "this is validated twice" depends on context. Within the same function call chain (Write → buildVersionFile), it's defense in depth and worth keeping. Across a read-then-serve boundary (extractMetadata → copyPublisherMeta), validating at the response boundary is sufficient — the read function doesn't need to also validate.

Fritz's instinct on the `extractMetadata` allocation was right. I initially deferred it to debt, but the question "under heavy use will it matter?" reframed it. A 1 MiB string copy per FETCH is not a theoretical concern — it's a concrete allocation that scales linearly with request rate and document size. The fix was three lines. Debt pages are for things that are hard to fix, not things that are easy to fix and just need a nudge.

### Note to Future Sessions

Frontmatter metadata feature is complete and review-hardened. All tests passing, lint clean. Ready for merge when Fritz says go.