gosec govulncheck runs (#1)

* gosec govulncheck runs

* Fix flaky TestRateLimiter_Matrix test

The test was failing due to two issues:
1. Test name generation used invalid character conversion (string(rune('0'+limit)))
   which produced non-printable characters for limits >= 10
2. Using 10ms windows with 100 requests caused race conditions - early requests
   would expire before all 100 were made, allowing the 101st request

Changed to use struct-based test cases with proper fmt.Sprintf naming and
a consistent 1-second window that won't expire during rapid test execution.
This commit is contained in:
2025-12-09 01:07:16 +00:00
committed by GitHub
parent 27d5011ab1
commit 29263dc8a2
17 changed files with 80 additions and 215 deletions
+1 -1
View File
@@ -44,7 +44,7 @@ func New(configPath string) (*Daemon, error) {
cfg.EnsureDefaultGroup()
// Save if we added a default group
if len(cfg.Groups) == 1 && cfg.Groups[0].Name == "default" && len(cfg.Groups[0].Hosts) == 0 {
cfgManager.Save()
_ = cfgManager.Save()
}
}
+7 -2
View File
@@ -178,13 +178,14 @@ func (m *HostsManager) buildManagedSection(entries []HostEntry) string {
func (m *HostsManager) writeAtomic(content string) error {
// Write to temp file first
tmpFile := m.hostsPath + ".tmp"
// #nosec G306 -- hosts file must be world-readable
if err := os.WriteFile(tmpFile, []byte(content), 0644); err != nil {
return err
}
// Rename atomically
if err := os.Rename(tmpFile, m.hostsPath); err != nil {
os.Remove(tmpFile)
_ = os.Remove(tmpFile)
return err
}
@@ -193,6 +194,7 @@ func (m *HostsManager) writeAtomic(content string) error {
// CreateBackup creates a backup of the current hosts file.
func (m *HostsManager) CreateBackup() error {
// #nosec G301 -- backup directory should be world-readable for recovery
if err := os.MkdirAll(m.backupDir, 0755); err != nil {
return fmt.Errorf("failed to create backup directory: %w", err)
}
@@ -205,6 +207,7 @@ func (m *HostsManager) CreateBackup() error {
timestamp := time.Now().Format("20060102-150405")
backupPath := filepath.Join(m.backupDir, fmt.Sprintf("hosts.%s.bak", timestamp))
// #nosec G306 -- backup files should be world-readable for recovery
if err := os.WriteFile(backupPath, content, 0644); err != nil {
return fmt.Errorf("failed to write backup: %w", err)
}
@@ -243,7 +246,7 @@ func (m *HostsManager) cleanupBackups() error {
// Remove oldest backups
for i := MaxBackups; i < len(backups); i++ {
path := filepath.Join(m.backupDir, backups[i].Name())
os.Remove(path)
_ = os.Remove(path)
}
return nil
@@ -301,6 +304,7 @@ func (m *HostsManager) GetBackupContent(name string) (string, error) {
return "", fmt.Errorf("invalid backup name")
}
// #nosec G304 -- backupPath is validated above: filepath.Base(name) == name and prefix/suffix checks
content, err := os.ReadFile(backupPath)
if err != nil {
return "", fmt.Errorf("failed to read backup: %w", err)
@@ -318,6 +322,7 @@ func (m *HostsManager) RestoreBackup(name string) error {
return fmt.Errorf("invalid backup name")
}
// #nosec G304 -- backupPath is validated above: filepath.Base(name) == name and prefix/suffix checks
content, err := os.ReadFile(backupPath)
if err != nil {
return fmt.Errorf("failed to read backup: %w", err)
+2 -1
View File
@@ -24,7 +24,7 @@ func (s *Server) getPeerCredentials(conn net.Conn) *PeerCredentials {
}
var creds *PeerCredentials
rawConn.Control(func(fd uintptr) {
_ = rawConn.Control(func(fd uintptr) {
xucred, err := unix.GetsockoptXucred(int(fd), unix.SOL_LOCAL, unix.LOCAL_PEERCRED)
if err != nil {
return
@@ -33,6 +33,7 @@ func (s *Server) getPeerCredentials(conn net.Conn) *PeerCredentials {
// Get PID separately using LOCAL_PEERPID
var pid int32
pidLen := uint32(unsafe.Sizeof(pid))
// #nosec G103 -- unsafe required for low-level syscall to get peer PID
_, _, errno := syscall.Syscall6(
syscall.SYS_GETSOCKOPT,
fd,
+2
View File
@@ -114,10 +114,12 @@ type AuditEntry struct {
func NewAuditLogger(path string) (*AuditLogger, error) {
// Ensure directory exists
dir := path[:len(path)-len("/audit.log")]
// #nosec G301 -- log directory should be world-readable
if err := os.MkdirAll(dir, 0755); err != nil {
return nil, fmt.Errorf("failed to create log directory: %w", err)
}
// #nosec G304,G302 -- path is from constant AuditLogPath; audit log should be world-readable
file, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644)
if err != nil {
return nil, fmt.Errorf("failed to open audit log: %w", err)
+23 -17
View File
@@ -1,6 +1,7 @@
package daemon
import (
"fmt"
"os"
"path/filepath"
"testing"
@@ -145,26 +146,31 @@ func TestPeerCredentials(t *testing.T) {
// Matrix test for rate limiting
func TestRateLimiter_Matrix(t *testing.T) {
limits := []int{1, 5, 10, 100}
windows := []time.Duration{10 * time.Millisecond, 100 * time.Millisecond, time.Second}
testCases := []struct {
limit int
window time.Duration
}{
{1, time.Second},
{5, time.Second},
{10, time.Second},
{100, time.Second},
}
for _, limit := range limits {
for _, window := range windows {
t.Run(
"limit="+string(rune('0'+limit))+"_window="+window.String(),
func(t *testing.T) {
rl := NewRateLimiter(limit, window)
for _, tc := range testCases {
t.Run(
fmt.Sprintf("limit=%d_window=%s", tc.limit, tc.window),
func(t *testing.T) {
rl := NewRateLimiter(tc.limit, tc.window)
// Should allow exactly 'limit' requests
for i := 0; i < limit; i++ {
assert.True(t, rl.Allow(1))
}
// Should allow exactly 'limit' requests
for i := 0; i < tc.limit; i++ {
assert.True(t, rl.Allow(1), "request %d should be allowed", i)
}
// Next should be blocked
assert.False(t, rl.Allow(1))
},
)
}
// Next should be blocked
assert.False(t, rl.Allow(1), "request after limit should be blocked")
},
)
}
}
+9 -8
View File
@@ -48,7 +48,7 @@ func NewServer(socketPath string, cfgManager *config.Manager) *Server {
// Start starts the server.
func (s *Server) Start() error {
// Remove existing socket
os.Remove(s.socketPath)
_ = os.Remove(s.socketPath)
listener, err := net.Listen("unix", s.socketPath)
if err != nil {
@@ -56,14 +56,15 @@ func (s *Server) Start() error {
}
// Set socket permissions: 0660 root:lolcathost
// #nosec G302 -- socket must be group-accessible for lolcathost group members
if err := os.Chmod(s.socketPath, 0660); err != nil {
listener.Close()
_ = listener.Close()
return fmt.Errorf("failed to set socket permissions: %w", err)
}
// Set socket group to lolcathost (GID 850)
if err := os.Chown(s.socketPath, 0, 850); err != nil {
listener.Close()
_ = listener.Close()
return fmt.Errorf("failed to set socket ownership: %w", err)
}
@@ -94,13 +95,13 @@ func (s *Server) Stop() error {
close(s.stopCh)
if s.listener != nil {
s.listener.Close()
_ = s.listener.Close()
}
os.Remove(s.socketPath)
_ = os.Remove(s.socketPath)
if s.auditLogger != nil {
s.auditLogger.Close()
_ = s.auditLogger.Close()
}
return nil
@@ -200,7 +201,7 @@ func (s *Server) isAuthorized(creds *PeerCredentials) bool {
func (s *Server) writeResponse(conn net.Conn, resp *protocol.Response) {
data, _ := json.Marshal(resp)
data = append(data, '\n')
conn.Write(data)
_, _ = conn.Write(data)
}
func (s *Server) handleRequest(req *protocol.Request, creds *PeerCredentials) *protocol.Response {
@@ -492,7 +493,7 @@ func (s *Server) handleRollback(req *protocol.Request) *protocol.Response {
}
// Flush DNS after restore
s.flusher.Flush()
_ = s.flusher.Flush()
resp, _ := protocol.NewOKResponse(map[string]string{"restored": payload.BackupName})
return resp