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:

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

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

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:

// 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:

// 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.