fix: address security and quality issues from code review

Security fixes:
- Fix XSS in Atom feed: escape user-supplied URLs in HTML content
- Wrap signup request approval in a transaction to prevent
  partial state on crash (user created but request still pending)
- Stop leaking internal error messages to admin UI
- Add request body size limit on API import endpoint
- Log SetMustResetPassword errors instead of silently discarding

Correctness fixes:
- Handle errors from API fave update/delete instead of returning
  success on failure
- Use actual data timestamp for feed <updated> instead of
  time.Now() (improves HTTP caching)
- Replace hardcoded 10000 export limit with named constant
  (maxExportFaves = 100000)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ole-Morten Duesund 2026-03-29 16:19:44 +02:00
commit 395b1b7523
5 changed files with 63 additions and 21 deletions

View file

@ -102,18 +102,28 @@ func (s *SignupRequestStore) ListPending() ([]*model.SignupRequest, error) {
// Approve marks a request as approved and creates the user account.
// The new user will have must_reset_password=1.
func (s *SignupRequestStore) Approve(id int64, adminID int64) error {
sr, err := s.GetByID(id)
tx, err := s.db.Begin()
if err != nil {
return err
return fmt.Errorf("begin tx: %w", err)
}
if sr.Status != "pending" {
return fmt.Errorf("request is not pending (status: %s)", sr.Status)
defer tx.Rollback()
// Fetch and verify the request is still pending, within the transaction.
var username, passwordHash, status string
err = tx.QueryRow(
`SELECT username, password_hash, status FROM signup_requests WHERE id = ?`, id,
).Scan(&username, &passwordHash, &status)
if err != nil {
return ErrSignupRequestNotFound
}
if status != "pending" {
return fmt.Errorf("request is not pending (status: %s)", status)
}
// Create the user with the already-hashed password.
_, err = s.db.Exec(
_, err = tx.Exec(
`INSERT INTO users (username, password_hash, must_reset_password) VALUES (?, ?, 1)`,
sr.Username, sr.PasswordHash,
username, passwordHash,
)
if err != nil {
if strings.Contains(err.Error(), "UNIQUE constraint failed") {
@ -123,14 +133,18 @@ func (s *SignupRequestStore) Approve(id int64, adminID int64) error {
}
// Mark the request as approved.
_, err = s.db.Exec(
_, err = tx.Exec(
`UPDATE signup_requests SET status = 'approved',
reviewed_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now'),
reviewed_by = ?
WHERE id = ?`,
adminID, id,
)
return err
if err != nil {
return fmt.Errorf("update request status: %w", err)
}
return tx.Commit()
}
// Reject marks a request as rejected.