security: graceful shutdown on restart event

handleMessage returns bool to signal session end instead of calling
osExit directly. Defers (pool stop, writeCh close, writerWg wait)
now run before exit, preventing in-flight result loss and panics
from writes to closed channels.
This commit is contained in:
Graham McIntire 2026-02-12 10:56:56 -06:00
parent 87606bb84a
commit 8b65dffc37
No known key found for this signature in database
2 changed files with 24 additions and 18 deletions

View file

@ -4,6 +4,7 @@ import (
"context" "context"
"encoding/base64" "encoding/base64"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"math/rand/v2" "math/rand/v2"
@ -22,6 +23,8 @@ import (
var osExit = os.Exit var osExit = os.Exit
var doSelfUpdate = selfUpdate var doSelfUpdate = selfUpdate
var errRestartRequested = fmt.Errorf("restart requested")
const maxJobPayloadBytes = 4 << 20 // 4 MB — well above any legitimate job list const maxJobPayloadBytes = 4 << 20 // 4 MB — well above any legitimate job list
// channelMsg is the WebSocket channel message format (JSON wrapper around binary protobuf). // channelMsg is the WebSocket channel message format (JSON wrapper around binary protobuf).
@ -49,6 +52,10 @@ func runAgent(ctx context.Context, wsURL, token string) {
if ctx.Err() != nil { if ctx.Err() != nil {
return return
} }
if errors.Is(err, errRestartRequested) {
osExit(0)
return
}
if err != nil { if err != nil {
slog.Error("agent disconnected", "error", err) slog.Error("agent disconnected", "error", err)
} }
@ -239,7 +246,10 @@ func runSession(ctx context.Context, baseURL, token string) error {
slog.Warn("invalid message", "error", err) slog.Warn("invalid message", "error", err)
continue continue
} }
handleMessage(ctx, msg, pools, snmpResultCh, mikrotikResultCh, credTestResultCh, monitoringCheckCh) if handleMessage(ctx, msg, pools, snmpResultCh, mikrotikResultCh, credTestResultCh, monitoringCheckCh) {
flushSnmpBatch()
return errRestartRequested
}
case result := <-snmpResultCh: case result := <-snmpResultCh:
snmpBatch = append(snmpBatch, result) snmpBatch = append(snmpBatch, result)
@ -290,6 +300,7 @@ func runSession(ctx context.Context, baseURL, token string) error {
} }
// handleMessage dispatches incoming channel messages. // handleMessage dispatches incoming channel messages.
// Returns true if the session should end (e.g. restart requested).
func handleMessage( func handleMessage(
ctx context.Context, ctx context.Context,
msg channelMsg, msg channelMsg,
@ -298,7 +309,7 @@ func handleMessage(
mikrotikResultCh chan<- *pb.MikrotikResult, mikrotikResultCh chan<- *pb.MikrotikResult,
credTestResultCh chan<- *pb.CredentialTestResult, credTestResultCh chan<- *pb.CredentialTestResult,
monitoringCheckCh chan<- *pb.MonitoringCheck, monitoringCheckCh chan<- *pb.MonitoringCheck,
) { ) bool {
switch msg.Event { switch msg.Event {
case "phx_reply": case "phx_reply":
slog.Debug("channel reply", "topic", msg.Topic) slog.Debug("channel reply", "topic", msg.Topic)
@ -309,21 +320,21 @@ func handleMessage(
} }
if err := json.Unmarshal(msg.Payload, &payload); err != nil { if err := json.Unmarshal(msg.Payload, &payload); err != nil {
slog.Error("decode job payload", "error", err) slog.Error("decode job payload", "error", err)
return return false
} }
if len(payload.Binary) > maxJobPayloadBytes { if len(payload.Binary) > maxJobPayloadBytes {
slog.Error("job payload too large", "size", len(payload.Binary), "max", maxJobPayloadBytes) slog.Error("job payload too large", "size", len(payload.Binary), "max", maxJobPayloadBytes)
return return false
} }
bin, err := base64.StdEncoding.DecodeString(payload.Binary) bin, err := base64.StdEncoding.DecodeString(payload.Binary)
if err != nil { if err != nil {
slog.Error("decode base64", "error", err) slog.Error("decode base64", "error", err)
return return false
} }
var jobList pb.AgentJobList var jobList pb.AgentJobList
if err := proto.Unmarshal(bin, &jobList); err != nil { if err := proto.Unmarshal(bin, &jobList); err != nil {
slog.Error("unmarshal job list", "error", err) slog.Error("unmarshal job list", "error", err)
return return false
} }
slog.Info("received jobs", "count", len(jobList.Jobs)) slog.Info("received jobs", "count", len(jobList.Jobs))
for _, job := range jobList.Jobs { for _, job := range jobList.Jobs {
@ -331,8 +342,8 @@ func handleMessage(
} }
case "restart": case "restart":
slog.Info("restart requested by server, exiting") slog.Info("restart requested by server")
osExit(0) return true
case "update": case "update":
var payload struct { var payload struct {
@ -341,7 +352,7 @@ func handleMessage(
} }
if err := json.Unmarshal(msg.Payload, &payload); err != nil || payload.URL == "" || payload.Checksum == "" { if err := json.Unmarshal(msg.Payload, &payload); err != nil || payload.URL == "" || payload.Checksum == "" {
slog.Error("invalid update payload") slog.Error("invalid update payload")
return return false
} }
slog.Info("update requested", "url", payload.URL) slog.Info("update requested", "url", payload.URL)
if err := doSelfUpdate(payload.URL, payload.Checksum); err != nil { if err := doSelfUpdate(payload.URL, payload.Checksum); err != nil {
@ -351,6 +362,7 @@ func handleMessage(
default: default:
slog.Debug("ignoring event", "event", msg.Event) slog.Debug("ignoring event", "event", msg.Event)
} }
return false
} }
// jobPools holds the worker pools for each job type. // jobPools holds the worker pools for each job type.

View file

@ -165,20 +165,14 @@ func TestHandleMessage(t *testing.T) {
}) })
t.Run("restart", func(t *testing.T) { t.Run("restart", func(t *testing.T) {
origExit := osExit
defer func() { osExit = origExit }()
var exitCode int
osExit = func(code int) { exitCode = code }
snmpCh := make(chan *pb.SnmpResult, 1) snmpCh := make(chan *pb.SnmpResult, 1)
mtCh := make(chan *pb.MikrotikResult, 1) mtCh := make(chan *pb.MikrotikResult, 1)
credCh := make(chan *pb.CredentialTestResult, 1) credCh := make(chan *pb.CredentialTestResult, 1)
monCh := make(chan *pb.MonitoringCheck, 1) monCh := make(chan *pb.MonitoringCheck, 1)
handleMessage(context.Background(), channelMsg{Event: "restart", Payload: json.RawMessage(`{}`)}, testPools(t), snmpCh, mtCh, credCh, monCh) shouldEnd := handleMessage(context.Background(), channelMsg{Event: "restart", Payload: json.RawMessage(`{}`)}, testPools(t), snmpCh, mtCh, credCh, monCh)
if exitCode != 0 { if !shouldEnd {
t.Errorf("expected exit code 0, got %d", exitCode) t.Error("expected handleMessage to return true for restart")
} }
}) })