feat: TUI remote support (#130)

[EconoBen]

The Problem

msgvault already supports remote CLI commands (stats, search, show-message) against a NAS server. But the TUI—the primary way to explore your archive—only works locally. If your archive lives on a NAS, you can't browse it from your laptop.

What This PR Does

Enables msgvault tui to work transparently against a remote server. When [remote].url is configured, the TUI connects to your NAS and gives you the full browsing experience: aggregate views, drill-down navigation, search, filtering—everything works identically to local mode.

# In config.toml
[remote]
url = "https://nas:8080"
api_key = "your-key"

# Then just run TUI as normal
msgvault tui              # Connects to NAS automatically
msgvault tui --local      # Force local if needed

Implementation

Server side: 6 new API endpoints that expose the query.Engine capabilities needed by the TUI:

• /api/v1/aggregates – sender/domain/label/time aggregations
• /api/v1/aggregates/sub – drill-down into sub-aggregations
• /api/v1/messages/filter – filtered message lists with pagination
• /api/v1/stats/total – stats with filter context
• /api/v1/search/fast – metadata search (subject, sender, recipient)
• /api/v1/search/deep – full-text body search via FTS5

Client side: remote.Engine implements the full query.Engine interface over HTTP, so the TUI code doesn't change at all—it just gets a different engine.

Safety: Deletion staging and attachment export are disabled in remote mode. These require local file access and are potentially destructive, so they're local-only operations.

Testing

• All handler tests pass (10 new tests for aggregate endpoints)
• Remote engine unit tests pass
• TUI builds with remote support
• Manual end-to-end testing against NAS

---

Closes #130

[roborev-ci]

roborev: Combined Review

Verdict: Not ready to merge — there is one high-severity auth exposure risk plus multiple medium-severity correctness/availability issues.

High

1. Potential unauthenticated access to sensitive API endpoints
   • Location: internal/api/server.go (routes), validated via internal/api/handlers_test.go
   • Details: New sensitive routes POST /auth/token/{email} and POST /accounts appear reachable without API-key auth middleware; one review notes tests exercising access without an API key.
   • Risk: Unauthorized token overwrite/account creation.
   • Fix: Ensure both routes are mounted under the API-key-protected router group in setupRouter.

Medium

1. Request cancellation/timeouts bypassed in query-heavy handlers

   • Location: internal/api/handlers.go:~900, ~940, ~970, ~1010/~1020, ~1060/~1070, ~1100
   • Details: Handlers (handleAggregates, handleSubAggregates, handleFilteredMessages, handleTotalStats, handleFastSearch, handleDeepSearch) use context.Background() instead of r.Context().
   • Risk: Expensive work continues after client disconnect/timeout, increasing DoS exposure.
   • Fix: Pass r.Context() through all engine/DB calls and ensure downstream honors cancellation.

2. Config save is unsafe (non-atomic + permissions may remain too broad)

   • Location: internal/config/config.go:243, internal/config/config.go:257
   • Details: Config.Save() writes with os.O_TRUNC directly to config.toml; crash/ENOSPC can corrupt file. Also, existing files may retain permissive mode because open flags alone do not tighten permissions.
   • Risk: Config loss/corruption; potential secret exposure if mode is too open.
   • Fix: Write to temp file in same dir, fsync, rename atomically, then enforce 0600 (e.g., explicit chmod).

3. Remote deep-search query reconstruction is lossy

   • Location: internal/remote/engine.go:487, internal/remote/engine.go:611
   • Details: Rebuilding raw query strings drops semantics (quoted phrases unquoted; fields like AccountID/HideDeleted omitted).
   • Risk: Remote search results diverge from local behavior (broader or incorrectly filtered results).
   • Fix: Send structured filters to /search/deep (preferred) or fully preserve quoting/escaping and all filter fields.

4. search --json output schema differs between local and remote modes

   • Location: cmd/msgvault/cmd/search.go:158, cmd/msgvault/cmd/search.go:183
   • Details: Local returns array; remote returns object envelope { total, results }.
   • Risk: Script/API consumer breakage when switching to remote mode.
   • Fix: Normalize to one stable schema across modes (or gate new schema behind an explicit flag/version).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Changes are not ready to merge due to 1 High and 4 Medium-severity issues.

High

1. Credential leakage risk on HTTP redirects
   • Locations: internal/remote/store.go:66, internal/remote/store.go:91, cmd/msgvault/cmd/export_token.go:149, cmd/msgvault/cmd/export_token.go:181, cmd/msgvault/cmd/export_token.go:223
   • Default http.Client redirect behavior can forward X-API-Key (and for 307/308, potentially replay sensitive request bodies) to unintended targets.
   • Recommended fix: Set CheckRedirect to fail closed (or only allow strict same-origin redirects with scheme/host validation).

Medium

1. Unbounded limit enables resource exhaustion

   • Locations: internal/api/handlers.go:414, internal/api/handlers.go:~520, internal/api/handlers.go:533, internal/api/handlers.go:~600, internal/api/handlers.go:642
   • limit is accepted without a hard upper bound in filter/aggregate paths, allowing very large requests (DB/memory/response-size DoS risk).
   • Recommended fix: Enforce a max limit (for example 500), reject oversized values with 400, and add tests.

2. Config save path can preserve insecure file permissions (and is non-atomic)

   • Locations: internal/config/config.go:243, internal/config/config.go:257
   • Save() uses direct O_TRUNC writes; existing permissive modes can persist and expose secrets (api_key), and truncation is not atomic.
   • Recommended fix: Write temp file with 0600, fsync, rename, then enforce secure chmod on final path.

3. Remote search query serialization changes query semantics

   • Locations: internal/remote/engine.go:469, internal/remote/engine.go:505, internal/remote/engine.go:620
   • Rebuilding parsed queries into raw strings without proper quoting/escaping can alter behavior (especially quoted phrases / values with spaces).
   • Recommended fix: Preserve original raw query string end-to-end or implement correct escaping/quoting with round-trip tests.

4. Setup defaults container to root user

   • Locations: cmd/msgvault/cmd/setup.go:281
   • Generated docker-compose.yml runs as root by default, increasing impact of container compromise.
   • Recommended fix: Default to non-root; make root explicit/opt-in (or clearly prompt/warn when required for Synology).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: PR has one High and multiple Medium issues that should be addressed before merge.

High

• Insecure config file permissions can expose secrets
  Refs: internal/config/config.go:243, internal/config/config.go:257
Config.Save() uses os.OpenFile(..., O_TRUNC, 0600), which does not fix existing permissive modes on pre-existing files (e.g., 0644). This can leave values like [server].api_key / [remote].api_key world-readable.
  Suggested fix: write to a new temp file with 0600 and atomic rename, or enforce mode with chmod/secure helper before or after write.

Medium

• Auth bypass for write endpoints when API key is unset
  Refs: internal/api/server.go:132, internal/api/server.go:188
  Auth middleware skips checks if api_key is empty; new mutating endpoints (e.g., token upload/account updates) can then be called unauthenticated by local actors.
  Suggested fix: require auth for mutating endpoints regardless, or fail startup when write endpoints are enabled without an API key.

• Unbounded limit enables resource-exhaustion (DoS) patterns
  Refs: internal/api/handlers.go:627, internal/api/handlers.go:717, internal/api/handlers.go:783, internal/api/handlers.go:~560, internal/api/handlers.go:~640
  Multiple handlers/parsers accept arbitrarily large positive limits, allowing expensive queries and large response generation.
  Suggested fix: enforce a hard max (commonly <= 500) across all relevant paths and test oversized inputs.

• /messages/filter pagination metadata appears incorrect
  Refs: internal/api/handlers.go:526, internal/api/handlers.go:958
  Response total is reported as page length (len(summaries)) rather than total matching rows, which breaks pagination semantics for clients.
  Suggested fix: return true total count, or rename/document field semantics (e.g., returned_count).

• Remote query reconstruction can change search semantics
  Refs: internal/remote/engine.go:650, internal/remote/engine.go:665, internal/remote/engine.go:678, internal/remote/engine.go:687
  Rebuilt query strings do not reliably re-quote multi-word terms (e.g., subject:"foo bar" can become subject:foo bar), altering parser behavior.
  Suggested fix: quote/escape terms during reconstruction and add round-trip tests for quoted/mixed queries.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Not ready to merge; reviewers identified 1 High and 5 Medium-severity issues.

High

1. API can start without enforced auth key (fail-open risk)

• Files: cmd/msgvault/cmd/serve.go (function runServe), internal/api/server.go
• Issue: Server startup does not explicitly fail when cfg.Server.APIKey is empty, which may expose sensitive endpoints without authentication.
• Suggested fix: Fail secure on empty API key (or strictly bind to localhost with explicit warning), and ensure auth middleware rejects requests when server API key is unset.

Medium

1. Unbounded limit enables DoS via large result sets

• Files/lines: internal/api/handlers.go:717, internal/api/handlers.go:783, internal/api/handlers.go:785, internal/api/handlers.go:936
• Issue: parseMessageFilter / aggregate parsing accepts any positive limit without a hard cap, allowing expensive CPU/memory/JSON work.
• Suggested fix: Enforce server-side maximum (for example <=500) and reject oversized requests.

2. Config save may leave sensitive file readable by other users

• File/line: internal/config/config.go:257
• Issue: Config.Save() uses OpenFile(..., 0600) with truncate; existing file modes are not corrected, so previously permissive config.toml can remain readable.
• Suggested fix: Enforce chmod 0600 on save and prefer temp-file write + atomic rename.

3. TUI --local flag behavior diverges from global mode handling

• Files/lines: cmd/msgvault/cmd/tui.go:64, cmd/msgvault/cmd/tui.go:236, cmd/msgvault/cmd/root.go:150
• Issue: TUI uses a separate --local path (forceLocalTUI) instead of shared global mode logic, creating inconsistent CLI behavior.
• Suggested fix: Use shared useLocal / IsRemoteMode() flow and remove duplicate flag semantics.

4. Fast-search default limit logic is inconsistent

• Files/lines: internal/api/handlers.go:788, internal/api/handlers.go:1034, internal/api/handlers.go:1037
• Issue: Parser default (500) prevents /search/fast from using its intended default (100) when limit is omitted.
• Suggested fix: Let endpoint-specific handlers apply defaults, or distinguish “not provided” from “provided”.

5. /messages/filter returns page-size count as total

• File/line: internal/api/handlers.go:958
• Issue: Response uses total: len(summaries) (current page length) instead of full match count, breaking expected pagination semantics.
• Suggested fix: Return full-match total (or rename field if intentionally page-local).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[wesm]

If there are security concerns it keeps raising that don't make sense, feel free to add stuff to review_guidelines in .roborev.toml

[EconoBen]

@wesm @hughbien

This PR implements TUI remote support (Issue #130) - enabling msgvault tui to work against a remote NAS when [remote].url is configured.

Changes:

• 6 new API endpoints for TUI aggregate queries (/api/v1/aggregates, /api/v1/search/fast, etc.)
• remote.Engine implementing query.Engine interface over HTTP
• --local flag to force local mode; deletion/export disabled remotely for safety

Status: All tests pass locally. Just pushed a lint fix - waiting on CI.

Ready for review when CI is green.

[roborev-ci]

roborev: Combined Review

Verdict: 4 Medium-severity issues should be addressed before merge (1 availability risk, 3 regressions).

Medium

1. Unbounded limit enables expensive queries (DoS risk)
   Refs: internal/api/handlers.go:717, internal/api/handlers.go:736, internal/api/handlers.go:783, internal/api/handlers.go:792, internal/api/handlers.go:875, internal/api/handlers.go:918, internal/api/handlers.go:946
parseAggregateOptions / parseMessageFilter accept very large positive limit values that flow into query execution for aggregates and filtered messages, allowing oversized scans/responses.

2. Fast-search default limit regression (doc/behavior mismatch)
   Refs: internal/api/handlers.go:788, internal/api/handlers.go:1035
parseMessageFilter defaults limit=500; handleFastSearch only overrides when limit==0 || limit>500, so fast search defaults to 500 instead of documented 100.

3. Remote search query reconstruction loses phrase semantics
   Refs: internal/remote/engine.go:650, internal/remote/engine.go:664, internal/remote/engine.go:667, internal/remote/engine.go:680
buildSearchQueryString rebuilds terms without robust quoting/escaping, so multi-word terms/phrases can be re-parsed with different meaning.

4. No SQLite fallback in serve when DuckDB/parquet path fails
   Refs: cmd/msgvault/cmd/serve.go:88, internal/query/duckdb.go:1018, internal/query/duckdb.go:1449
   Server wiring can leave remote query endpoints failing at runtime instead of degrading to SQLite when parquet/extension setup is unavailable.

---

Synthesized from 4 reviews (agents: gemini, codex | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Not ready to merge; there are multiple Medium-severity issues that should be fixed first.

Critical

• None.

High

• None.

Medium

1. Unbounded limit allows authenticated resource exhaustion (DoS)

   • Locations: internal/api/handlers.go:135, internal/api/handlers.go:201, internal/api/handlers.go:132 (approx), internal/api/handlers.go:200 (approx), internal/api/handlers.go:~706, internal/api/handlers.go:~717, internal/api/handlers.go:~753, internal/api/handlers.go:~783, internal/api/handlers.go:~938
   • Details: parseMessageFilter / parseAggregateOptions accept very large limit values without a hard cap, and downstream handlers (handleFilteredMessages, handleAggregates, handleSubAggregates) can execute expensive large-result queries.
   • Suggested fix: Enforce a server-side max (e.g., 500/1000) in the parsing helpers and reject oversized values (or clamp consistently).

2. Deep-search runtime regression when DuckDB SQLite extension is unavailable

   • Locations: cmd/msgvault/cmd/serve.go:88, internal/query/duckdb.go:1454
   • Details: serve initializes DuckDB with sqliteDB=nil, which can cause deep search to fail at runtime (Search requires SQLite) instead of gracefully falling back when sqlite_scanner is not available.
   • Suggested fix: Pass the live SQLite handle (s.DB()) into NewDuckDBEngine so fallback paths remain functional.

3. Pagination semantics bug in filtered messages response

   • Location: internal/api/handlers.go:958
   • Details: /api/v1/messages/filter returns "total": len(summaries) (page size), not total matching rows, which breaks expected pagination behavior.
   • Suggested fix: Return actual total-match count (separate count query) or rename the field to reflect returned-page count.

4. Missing tests for new remote engine logic

   • Location: internal/remote/engine.go
   • Details: New remote engine request/response logic was flagged as untested, increasing regression risk in critical client/server integration paths.
   • Suggested fix: Add focused unit tests (e.g., internal/remote/engine_test.go) for request construction, error handling, and response parsing.

---

Synthesized from 4 reviews (agents: gemini, codex | types: security, default)