Add HTTP API server and daemon mode with scheduled sync#77
Conversation
Add configuration types to support NAS/Docker deployment: - ServerConfig: API port, API key, MCP enabled flag - AccountSchedule: per-account cron schedules for automated sync - ScheduledAccounts(): helper to get enabled scheduled accounts - GetAccountSchedule(): lookup schedule by email These types enable Phase 2 daemon mode implementation with scheduled syncing for headless NAS deployments. Part of: T-8 (Extend config for server and accounts) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add scheduler package for automated email sync in daemon mode: - Cron-based scheduling using robfig/cron/v3 - Per-account schedules with enable/disable support - Callback-based sync execution for clean separation - Prevents concurrent syncs for the same account - TriggerSync() for manual/API-initiated syncs - Status() returns current state of all scheduled accounts - ValidateCronExpr() helper for config validation - Comprehensive test coverage Part of: T-7 (Create scheduler package) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add serve command that runs msgvault as a long-running daemon: - Loads scheduled accounts from config.toml - Runs incremental sync on cron schedule - Automatically rebuilds cache after syncs with new messages - Graceful shutdown on SIGINT/SIGTERM - Prints schedule status on startup Usage: msgvault serve Part of: T-6 (Create serve command) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tests for serve command and scheduler integration: - TestServeConfigParsing: verify config.toml parsing with accounts - TestSchedulerWithConfig: verify scheduler handles config correctly - TestServeCmdNoAccounts: verify empty accounts handling - TestCronExpressionValidation: verify cron expression validation Part of: T-9 (Daemon integration tests) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- wrapOAuthError now checks os.ErrPermission in addition to os.ErrNotExist - Changed error message to "not accessible" (covers both cases) - Added comprehensive tests for OAuth error helpers - Fixed trailing newline in .roborev.toml Addresses: roborev findings from commits 1d6ba2f, 1c43ecf, f1d5714 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add HTTP API server integrated with serve command:
- Chi router with standard middleware (RequestID, RealIP, Logger, Recoverer)
- API key authentication via Authorization or X-API-Key headers
- Health check endpoint: GET /health
- Stats endpoint: GET /api/v1/stats
- Accounts endpoint: GET /api/v1/accounts
- Scheduler status: GET /api/v1/scheduler/status
- Trigger sync: POST /api/v1/sync/{account}
- Placeholder handlers for messages/search (TODO: T-11)
- Server tests for health, auth middleware, and endpoints
Part of: T-10 (Create API Server Setup)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add middleware for API security and performance: - CORS middleware with configurable origins, methods, headers - Preflight (OPTIONS) request handling - Per-IP rate limiting using golang.org/x/time/rate - Default: 10 req/sec with burst of 20 - Rate limited responses include Retry-After header - Comprehensive middleware tests Part of: T-12 (Create API Middleware) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add full API handler implementations:
- GET /api/v1/messages: paginated message listing
- GET /api/v1/messages/{id}: full message details with body and attachments
- GET /api/v1/search?q=...: FTS5 search with LIKE fallback
Add store methods for API support:
- ListMessages: paginated listing with sender, recipients, labels
- GetMessage: full message with body and attachment metadata
- SearchMessages: FTS5 search or LIKE fallback
- ListSources: list all configured sources
Part of: T-11 (Create API Handlers)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add handler tests with real database: - TestHandleStats: verify stats endpoint returns data - TestHandleListMessages: test message listing - TestHandleListMessagesPagination: test pagination params - TestHandleGetMessage: test single message retrieval - TestHandleGetMessageNotFound: test 404 response - TestHandleGetMessageInvalidID: test bad request handling - TestHandleSearchMissingQuery: test search validation - TestHandleSearch: test search functionality - TestHandleTriggerSync: test manual sync trigger Part of: T-13 (API Tests) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive API documentation: - Authentication methods (Bearer, X-API-Key) - Rate limiting (10 req/sec per IP) - All endpoints with request/response examples - Error response format and common codes - CORS configuration - Code examples (cURL, Python, JavaScript) Part of: T-14 (API Documentation) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
hi @EconoBen — can you resolve the conflicts in your branch? I can take a closer look at this in the next few days |
yes! sorry. I'm doing these PR's as a phased approach. I can post the plan somewhere if you'd like. |
|
@EconoBen Yes, if you could open a GitHub issue explaining what you are doing, and keep your work rebased on main (or merged with main — whatever works for you since I squash / enforce linear histories in all my projects at PR merge time) that would be great. |
|
Please try to get synced up with main so I can evaluate this branch and give you some feedback on what you are doing before you go too much further down the rabbit hole. Let me know if you need help |
|
Also, I would greatly prefer that you work in a single branch vs. continuing to open more pull requests, just update the PR title and description and let's focus the discussion in one place |
Resolve merge conflicts: - .roborev.toml: Keep upstream review guidelines - cmd/msgvault/cmd/root.go: Combine ErrPermission check with upstream function syntax - cmd/msgvault/cmd/root_test.go: Merge both OAuth error tests and context tests - internal/config/config.go: Merge server/scheduler config with upstream path handling - internal/config/config_test.go: Merge scheduler tests with upstream path tests - internal/store/api.go: Remove duplicate ListSources (upstream version has filter param) - cmd/msgvault/cmd/serve.go: Use DatabaseDSN() instead of DatabasePath() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security Review: 6 High/Medium Issues FoundClaude's automated security review identified potential security concerns. Please review the inline comments. Additionally:
Note: This is an automated review. False positives are possible. Please review each issue carefully and use your judgment. Powered by Claude 4.5 Sonnet |
|
You still have some conflicts to resolve |
Accept upstream changes and adapt: - Update go.mod/go.sum to upstream versions, add chi/cron/time deps - Update config.Load() calls to new two-argument signature (path, homeDir) - Preserve Phase 2+3 features (scheduler, API server) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security Review: 2 High/Medium Issues FoundClaude's automated security review identified potential security concerns. Please review the inline comments. Additionally:
Note: This is an automated review. False positives are possible. Please review each issue carefully and use your judgment. Powered by Claude 4.5 Sonnet |
…njection - Use crypto/subtle.ConstantTimeCompare for API key validation to prevent timing side-channel attacks - Return copy from GetAccountSchedule instead of pointer to slice element to prevent dangling pointer if Accounts slice is reallocated - Escape %, _, and \ in LIKE fallback search patterns using SQLite ESCAPE clause to prevent wildcard injection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security Review: 1 High/Medium Issue FoundClaude's automated security review identified potential security concerns. Please review the inline comments. Additionally:
Note: This is an automated review. False positives are possible. Please review each issue carefully and use your judgment. Powered by Claude 4.5 Sonnet |
…tics - TestEscapeLike: 8 cases covering plain text, %, _, backslash, combinations, empty string, and adjacent specials - TestGetAccountScheduleReturnsCopy: verifies mutations to the returned struct don't leak back to the config's Accounts slice Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security Review: 3 High/Medium Issues FoundClaude's automated security review identified potential security concerns. Please review the inline comments. Additionally:
Note: This is an automated review. False positives are possible. Please review each issue carefully and use your judgment. Powered by Claude 4.5 Sonnet |
TestSearchMessagesLikeLiteralWildcards seeds messages with literal % and _ in subjects, calls searchMessagesLike directly, and verifies both count and row results treat wildcards literally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security Review: 5 High/Medium Issues FoundClaude's automated security review identified potential security concerns. Please review the inline comments. Additionally:
Note: This is an automated review. False positives are possible. Please review each issue carefully and use your judgment. Powered by Claude 4.5 Sonnet |
Adds control rows ("100 days sale", "fileXname.txt") that would match
the search queries if % and _ were treated as SQL wildcards instead of
literal characters. This proves the escapeLike function is actually
necessary, not just coincidentally passing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| // without an explicit opt-in via allow_insecure. | ||
| func (s ServerConfig) ValidateSecure() error { | ||
| if !s.IsLoopback() && s.APIKey == "" && !s.AllowInsecure { | ||
| return fmt.Errorf("refusing to start: bind address %q is not loopback and no api_key is set\n\n"+ |
There was a problem hiding this comment.
🚨 Non-loopback binding without API key allows unauthenticated access (high severity)
The ValidateSecure() check can be bypassed by setting allow_insecure=true, exposing the entire email archive API (including OAuth tokens, messages, and deletion endpoints) on the network without authentication. This is documented as 'not recommended for production' but should require a stronger warning or be disallowed entirely. Consider removing the allow_insecure bypass or requiring explicit confirmation (e.g., environment variable override).
Automated security review by Claude 4.5 Sonnet - Human review still required
|
In The evictLoop goroutine accesses shared state without holding the mutex during the select statement, which could race with Close() or other operations. If evictLoop panics or encounters an error, the goroutine leaks. Consider adding a recover() block and ensuring all goroutine cleanup is robust. Automated security review by Claude 4.5 Sonnet - Human review still required |
|
|
||
| // authMiddleware validates the API key. | ||
| func (s *Server) authMiddleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
The authMiddleware uses subtle.ConstantTimeCompare to prevent timing attacks, but does not rate-limit failed authentication attempts per IP. An attacker can brute-force the API key without penalty. Add per-IP rate limiting specifically for 401 responses or integrate with the existing RateLimiter for auth failures.
Automated security review by Claude 4.5 Sonnet - Human review still required
| func runServe(cmd *cobra.Command, args []string) error { | ||
| // Validate security posture before doing any work | ||
| if err := cfg.Server.ValidateSecure(); err != nil { | ||
| return err |
There was a problem hiding this comment.
The runServe function checks if cfg.OAuth.ClientSecrets is empty but does not verify the file exists or is readable before starting the long-running daemon. If the file is missing, scheduled syncs will fail silently. Add an explicit file existence/readability check before starting the scheduler to fail fast with a clear error message.
Automated security review by Claude 4.5 Sonnet - Human review still required
Security Review: 4 High/Medium Issues FoundClaude's automated security review identified potential security concerns. Please review the inline comments. Additionally:
Note: This is an automated review. False positives are possible. Please review each issue carefully and use your judgment. Powered by Claude 4.5 Sonnet |
|
I think this is good enough. I'll merge this once the CI is green |
|
Thanks for this. We'll want to add documentation in https://github.com/wesm/msgvault-docs, I can work on a PR for this and I'll tag you for review later today or tomorrow if that sounds good |
## Summary
Adds a long-running daemon mode (`msgvault serve`) with an HTTP API
server and cron-based scheduled sync for email accounts. This is the
foundation for remote access to the email archive, including a future
web UI.
### What's included
- **Daemon mode** (`msgvault serve`): runs in the foreground with signal
handling and graceful shutdown
- **HTTP API server**: chi-based router with health check, message
list/detail/search, account management, sync triggering, and scheduler
status
- **Cron scheduler**: automated incremental sync on configurable
per-account schedules with cancellation propagation
- **Security defaults**: loopback-only bind by default, API key required
for non-loopback, config-driven CORS (disabled by default)
- **Rate limiting**: per-IP token bucket with TTL-based eviction and
clean shutdown
- **Interface boundaries**: API package depends on `MessageStore` and
`SyncScheduler` interfaces, not concrete types
- **Batch query performance**: N+1 queries eliminated with IN-clause
batch loading for recipients and labels
- **Comprehensive tests**: mock-based API tests, scheduler lifecycle
tests, security validation, concurrent safety tests
### API Endpoints
| Method | Path | Description |
|--------|------|-------------|
| GET | `/health` | Health check (no auth) |
| GET | `/api/v1/stats` | Archive statistics |
| GET | `/api/v1/messages` | Paginated message list |
| GET | `/api/v1/messages/{id}` | Full message detail |
| GET | `/api/v1/search?q=...` | Full-text search |
| GET | `/api/v1/accounts` | Configured accounts |
| POST | `/api/v1/sync/{account}` | Trigger manual sync |
| GET | `/api/v1/scheduler/status` | Scheduler status |
### Configuration
```toml
[server]
api_port = 8080
bind_addr = "127.0.0.1" # loopback only by default
api_key = "your-secret-key" # required for non-loopback
cors_origins = ["http://localhost:3000"] # disabled by default
[[accounts]]
email = "you@gmail.com"
schedule = "0 2 * * *" # cron format
enabled = true
```
### Deferred enhancements
- Unifying API on top of `internal/query.Engine`
- Advanced auth (tokens/scopes/users)
- Multi-node coordination
- Observability (metrics/tracing)
- IMAP import for Yahoo, Outlook, and other email systems
- Static file serving for web UI
## Test plan
- [x] All existing tests pass (`go test ./...`)
- [x] `go fmt`, `go vet`, `golangci-lint` clean
- [x] API handler tests with mock store/scheduler
- [x] Security validation tests (loopback detection, IPv6, insecure
override)
- [x] CORS tests (config-driven, disabled by default)
- [x] Scheduler lifecycle tests (start/stop/cancel, trigger-after-stop)
- [x] Concurrent safety tests (rate limiter Close)
- [x] Nil dependency tests (503 for missing store/scheduler)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
|
Sorry I'm just seeing these updates. Was busy this weekend. Yes! I'll create a new branch and just put it all there. |
|
I already created documentation for this https://www.msgvault.io/api-server/. So feel free to open a PR to improve / refine the docs, or make other improvements to the API server here! thanks again |
Summary
Adds a long-running daemon mode (
msgvault serve) with an HTTP API server and cron-based scheduled sync for email accounts. This is the foundation for remote access to the email archive, including a future web UI.What's included
msgvault serve): runs in the foreground with signal handling and graceful shutdownMessageStoreandSyncSchedulerinterfaces, not concrete typesAPI Endpoints
/health/api/v1/stats/api/v1/messages/api/v1/messages/{id}/api/v1/search?q=.../api/v1/accounts/api/v1/sync/{account}/api/v1/scheduler/statusConfiguration
Deferred enhancements
internal/query.EngineTest plan
go test ./...)go fmt,go vet,golangci-lintclean🤖 Generated with Claude Code