fix: resolve 5 remaining medium/low issues from third-pass go-review

- NEW-1: Add rows.Err() check in ListMembers to catch cursor errors
- NEW-2: Add minVal parameter to queryInt so offset=0 is not rejected
- NEW-3: Fix copyFile double-close by removing defer, using explicit
  close on both success and error paths
- NEW-4: Add GetAllChannelPermissionsForRole batch query, eliminating
  N+1 GetChannelPermissions calls in channel list and search handlers
- NEW-5: Cap fetchBody with io.LimitReader(1 MiB) to prevent memory
  exhaustion from malformed release assets
This commit is contained in:
jevb
2026-03-19 04:19:03 +01:00
parent 13797e7075
commit a7df9c2b3c
8 changed files with 84 additions and 11 deletions
+4 -2
View File
@@ -253,13 +253,15 @@ func pathInt64(r *http.Request, param string) (int64, error) {
return strconv.ParseInt(raw, 10, 64)
}
func queryInt(r *http.Request, key string, defaultVal int) int {
// queryInt parses an integer query parameter with a minimum and maximum bound.
// Use minVal=1 for limit parameters, minVal=0 for offset parameters.
func queryInt(r *http.Request, key string, defaultVal, minVal int) int {
raw := r.URL.Query().Get(key)
if raw == "" {
return defaultVal
}
n, err := strconv.Atoi(raw)
if err != nil || n < 1 {
if err != nil || n < minVal {
return defaultVal
}
// Cap to prevent unbounded result sets exhausting memory.
+1 -1
View File
@@ -172,9 +172,9 @@ func copyFile(src, dst string) error {
if err != nil {
return fmt.Errorf("create destination: %w", err)
}
defer out.Close() //nolint:errcheck
if _, err := io.Copy(out, in); err != nil {
_ = out.Close()
return fmt.Errorf("copy: %w", err)
}
return out.Close()
+2 -2
View File
@@ -209,8 +209,8 @@ func handleDeleteChannel(database *db.DB, hub HubBroadcaster) http.HandlerFunc {
func handleGetAuditLog(database *db.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
limit := queryInt(r, "limit", 50)
offset := queryInt(r, "offset", 0)
limit := queryInt(r, "limit", 50, 1)
offset := queryInt(r, "offset", 0, 0)
entries, err := database.GetAuditLog(limit, offset)
if err != nil {
+2 -2
View File
@@ -27,8 +27,8 @@ func handleGetStats(database *db.DB, hub HubBroadcaster) http.HandlerFunc {
func handleListUsers(database *db.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
limit := queryInt(r, "limit", 50)
offset := queryInt(r, "offset", 0)
limit := queryInt(r, "limit", 50, 1)
offset := queryInt(r, "offset", 0, 0)
users, err := database.ListAllUsers(limit, offset)
if err != nil {
+35 -3
View File
@@ -43,6 +43,20 @@ func hasChannelPermREST(database *db.DB, role *db.Role, channelID, perm int64) b
return effective&perm == perm
}
// hasChannelPermBatch checks permission using a pre-fetched overrides map,
// eliminating N+1 queries when filtering multiple channels.
func hasChannelPermBatch(role *db.Role, overrides map[int64]db.ChannelOverride, channelID, perm int64) bool {
if role == nil {
return false
}
if permissions.HasAdmin(role.Permissions) {
return true
}
o := overrides[channelID] // zero-value (0,0) when no override exists
effective := permissions.EffectivePerms(role.Permissions, o.Allow, o.Deny)
return effective&perm == perm
}
// handleListChannels returns all channels the authenticated user can see.
func handleListChannels(database *db.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
@@ -58,10 +72,20 @@ func handleListChannels(database *db.DB) http.HandlerFunc {
return
}
// Batch-fetch all channel permission overrides for this role in one query.
overrides := map[int64]db.ChannelOverride{}
if role != nil && !permissions.HasAdmin(role.Permissions) {
var oErr error
overrides, oErr = database.GetAllChannelPermissionsForRole(role.ID)
if oErr != nil {
slog.Error("handleListChannels GetAllChannelPermissionsForRole", "err", oErr)
}
}
// Filter channels by READ_MESSAGES permission.
var visible []db.Channel
for _, ch := range channels {
if hasChannelPermREST(database, role, ch.ID, permissions.ReadMessages) {
if hasChannelPermBatch(role, overrides, ch.ID, permissions.ReadMessages) {
visible = append(visible, ch)
}
}
@@ -221,11 +245,19 @@ func handleSearch(database *db.DB) http.HandlerFunc {
return
}
// Post-filter results by READ_MESSAGES permission on each channel.
// Batch-fetch overrides and post-filter results by READ_MESSAGES.
role, _ := r.Context().Value(RoleKey).(*db.Role)
overrides := map[int64]db.ChannelOverride{}
if role != nil && !permissions.HasAdmin(role.Permissions) {
var oErr error
overrides, oErr = database.GetAllChannelPermissionsForRole(role.ID)
if oErr != nil {
slog.Error("handleSearch GetAllChannelPermissionsForRole", "err", oErr)
}
}
var filtered []db.MessageSearchResult
for _, res := range results {
if hasChannelPermREST(database, role, res.ChannelID, permissions.ReadMessages) {
if hasChannelPermBatch(role, overrides, res.ChannelID, permissions.ReadMessages) {
filtered = append(filtered, res)
}
}
+3
View File
@@ -356,6 +356,9 @@ func (d *DB) ListMembers() ([]MemberSummary, error) {
}
members = append(members, m)
}
if rows.Err() != nil {
return nil, fmt.Errorf("ListMembers rows: %w", rows.Err())
}
if members == nil {
members = []MemberSummary{}
}
+34
View File
@@ -135,6 +135,40 @@ func (d *DB) GetChannelPermissions(channelID, roleID int64) (allow, deny int64,
return allow, deny, nil
}
// ChannelOverride holds the allow/deny permission bits for a single channel.
type ChannelOverride struct {
Allow int64
Deny int64
}
// GetAllChannelPermissionsForRole returns all channel permission overrides for
// a role in a single query, keyed by channel ID. Eliminates N+1 queries when
// filtering channels by permission.
func (d *DB) GetAllChannelPermissionsForRole(roleID int64) (map[int64]ChannelOverride, error) {
rows, err := d.sqlDB.Query(
`SELECT channel_id, allow, deny FROM channel_overrides WHERE role_id = ?`,
roleID,
)
if err != nil {
return nil, fmt.Errorf("GetAllChannelPermissionsForRole: %w", err)
}
defer rows.Close() //nolint:errcheck
result := make(map[int64]ChannelOverride)
for rows.Next() {
var chID int64
var o ChannelOverride
if scanErr := rows.Scan(&chID, &o.Allow, &o.Deny); scanErr != nil {
return nil, fmt.Errorf("GetAllChannelPermissionsForRole scan: %w", scanErr)
}
result[chID] = o
}
if rows.Err() != nil {
return nil, fmt.Errorf("GetAllChannelPermissionsForRole rows: %w", rows.Err())
}
return result, nil
}
// ─── helpers ──────────────────────────────────────────────────────────────────
// scanChannel scans a single channel row from *sql.Rows.
+3 -1
View File
@@ -296,7 +296,9 @@ func (u *Updater) fetchBody(ctx context.Context, url string) ([]byte, error) {
return nil, fmt.Errorf("HTTP %d fetching %s", resp.StatusCode, url)
}
return io.ReadAll(resp.Body)
// Cap reads at 1 MiB — checksum and signature files are tiny text;
// this prevents a malicious or corrupted release asset from exhausting memory.
return io.ReadAll(io.LimitReader(resp.Body, 1<<20))
}
// FindClientAssets scans the cached release assets for the Tauri NSIS