From 8663beeb96caff39c51192f1d26338ad7c232833 Mon Sep 17 00:00:00 2001 From: Tyler Koenig Date: Wed, 20 May 2026 20:41:53 -0400 Subject: [PATCH] fix: harden API, DB, and web layer from audit findings - Cap list API limit at 200 to prevent unbounded queries - Sanitize markdown output with DOMPurify to prevent XSS - Add v4 migration with indexes on deleted_at and modified_at - Fix v2 migration swallowed ALTER TABLE errors - Tighten ~/.nib directory permissions to 0o700 --- TODO.md | 27 +++++++++++++++++++++++++++ internal/api/entities.go | 3 +++ internal/db/db.go | 25 +++++++++++++++++++++---- web/app.js | 3 ++- web/index.html | 1 + 5 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 TODO.md diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..62473e5 --- /dev/null +++ b/TODO.md @@ -0,0 +1,27 @@ +# Code Hardening — Senior Dev Audit Fixes + +## Phase 1: Quick Wins (safety + correctness) +- [ ] Cap API list limit at 200 +- [ ] Fix markdown XSS — add DOMPurify to sanitize marked output +- [ ] Add missing DB indexes (deleted_at, modified_at) via v4 migration +- [ ] Fix v2 migration error handling (swallowed ALTER TABLE errors) +- [ ] Fix ~/.nib directory permissions (0o755 → 0o700) + +## Phase 2: CI Pipeline +- [ ] Gitea Actions workflow: test + lint on PR + +## Phase 3: context.Context in Store +- [ ] Thread context.Context through all Store methods +- [ ] Use context in API handlers (from r.Context()) +- [ ] Use context in CLI commands (cobra context) + +## Phase 4: cmd/ Tests +- [ ] Test add command +- [ ] Test ls command +- [ ] Test promote/demote commands +- [ ] Test delete command +- [ ] Test absorb command + +## Phase 5: Backup/Export +- [ ] nib export — dump entities to JSON +- [ ] nib backup — safe SQLite backup (handles WAL) diff --git a/internal/api/entities.go b/internal/api/entities.go index b4058d4..cc0283b 100644 --- a/internal/api/entities.go +++ b/internal/api/entities.go @@ -92,6 +92,9 @@ func listEntities(store *db.Store) http.HandlerFunc { writeError(w, http.StatusBadRequest, "invalid_input", "limit must be a positive integer") return } + if limit > 200 { + limit = 200 + } p.Limit = limit } if offsetStr := r.URL.Query().Get("offset"); offsetStr != "" { diff --git a/internal/db/db.go b/internal/db/db.go index 982e072..e1f0c42 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -51,7 +51,7 @@ func (s *Store) Close() error { return s.db.Close() } -const currentSchema = 3 +const currentSchema = 4 var migrations = []func(db *sql.DB) error{ // v1: initial schema @@ -92,8 +92,12 @@ var migrations = []func(db *sql.DB) error{ // v2: add title and description columns func(db *sql.DB) error { - db.Exec(`ALTER TABLE entities ADD COLUMN title TEXT`) - db.Exec(`ALTER TABLE entities ADD COLUMN description TEXT`) + if _, err := db.Exec(`ALTER TABLE entities ADD COLUMN title TEXT`); err != nil { + return fmt.Errorf("add title column: %w", err) + } + if _, err := db.Exec(`ALTER TABLE entities ADD COLUMN description TEXT`); err != nil { + return fmt.Errorf("add description column: %w", err) + } return nil }, @@ -166,6 +170,19 @@ var migrations = []func(db *sql.DB) error{ return tx.Commit() }, + + // v4: add indexes for common query filters + func(db *sql.DB) error { + for _, idx := range []string{ + `CREATE INDEX IF NOT EXISTS idx_entities_deleted ON entities(deleted_at)`, + `CREATE INDEX IF NOT EXISTS idx_entities_modified ON entities(modified_at DESC) WHERE deleted_at IS NULL`, + } { + if _, err := db.Exec(idx); err != nil { + return fmt.Errorf("create index: %w", err) + } + } + return nil + }, } func (s *Store) migrate() error { @@ -200,7 +217,7 @@ func DefaultPath() (string, error) { return "", err } dir := filepath.Join(home, ".nib") - if err := os.MkdirAll(dir, 0o755); err != nil { + if err := os.MkdirAll(dir, 0o700); err != nil { return "", err } return filepath.Join(dir, "nib.db"), nil diff --git a/web/app.js b/web/app.js index ade4b6b..71d738b 100644 --- a/web/app.js +++ b/web/app.js @@ -1946,7 +1946,8 @@ function renderMd(s) { if (!s) return ''; if (typeof marked === 'undefined') return escHtml(s); - return marked.parse(s, { breaks: true }); + const html = marked.parse(s, { breaks: true }); + return typeof DOMPurify !== 'undefined' ? DOMPurify.sanitize(html) : escHtml(s); } function isSafeUrl(url) { diff --git a/web/index.html b/web/index.html index cfa9da4..872e1b7 100644 --- a/web/index.html +++ b/web/index.html @@ -97,6 +97,7 @@ +