refactor: address audit Bundle D — code-org quick wins (F016, F028, F049)

internal/actionparams/schedule.go

@@ -0,0 +1,72 @@
+package actionparams
+
+import (
+	"encoding/json"
+	"log/slog"
+
+	pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
+)
+
+// ScheduleToMap converts an ActionSchedule proto into the map shape
+// used for JSONB event payloads. Empty-valued fields are omitted so
+// the projector's ON CONFLICT / COALESCE behaviour treats them as
+// "preserve" rather than "set to zero".
+//
+// Used by the action / action_set / definition handlers when emitting
+// XxxCreated / XxxUpdated events. Centralised here so the serialised
+// shape stays identical across handlers — drift would surface as
+// silently-different schedule semantics depending on which entity
+// the schedule was attached to.
+func ScheduleToMap(s *pm.ActionSchedule) map[string]any {
+	m := map[string]any{}
+	if s == nil {
+		return m
+	}
+	if s.Cron != "" {
+		m["cron"] = s.Cron
+	}
+	if s.IntervalHours > 0 {
+		m["interval_hours"] = s.IntervalHours
+	}
+	if s.RunOnAssign {
+		m["run_on_assign"] = true
+	}
+	if s.SkipIfUnchanged {
+		m["skip_if_unchanged"] = true
+	}
+	return m
+}
+
+// ScheduleFromJSON deserialises a schedule JSONB column back into an
+// ActionSchedule proto. Returns nil for malformed JSON or for the
+// empty-object case (so callers can leave the proto field unset
+// instead of carrying an all-zeros placeholder).
+func ScheduleFromJSON(data []byte) *pm.ActionSchedule {
+	var raw struct {
+		Cron            string `json:"cron"`
+		IntervalHours   int32  `json:"interval_hours"`
+		RunOnAssign     bool   `json:"run_on_assign"`
+		SkipIfUnchanged bool   `json:"skip_if_unchanged"`
+	}
+	if err := json.Unmarshal(data, &raw); err != nil {
+		// Truly-empty input is a normal "no schedule configured"
+		// signal — stay silent. Non-empty bytes that fail to parse
+		// indicate event-store corruption or a schema drift the
+		// projector should surface, so emit a structured log so
+		// operators can grep for it.
+		if len(data) > 0 {
+			slog.Warn("actionparams: schedule JSON malformed; treating as no schedule",
+				"bytes", len(data), "error", err)
+		}
+		return nil
+	}
+	if raw.Cron == "" && raw.IntervalHours == 0 && !raw.RunOnAssign && !raw.SkipIfUnchanged {
+		return nil
+	}
+	return &pm.ActionSchedule{
+		Cron:            raw.Cron,
+		IntervalHours:   raw.IntervalHours,
+		RunOnAssign:     raw.RunOnAssign,
+		SkipIfUnchanged: raw.SkipIfUnchanged,
+	}
+}

internal/api/action_set_handler.go

@@ -9,6 +9,7 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
+	"github.com/manchtools/power-manage/server/internal/actionparams"
 	"github.com/manchtools/power-manage/server/internal/store"
 	db "github.com/manchtools/power-manage/server/internal/store/generated"
 )
@@ -50,7 +51,7 @@ func (h *ActionSetHandler) CreateActionSet(ctx context.Context, req *connect.Req
 	// non-empty so a zero-value request can't bypass the projection's
 	// default-schedule fallback by emitting an empty `{}` blob.
 	if req.Msg.Schedule != nil {
-		if schedule := scheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
+		if schedule := actionparams.ScheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
 			data["schedule"] = schedule
 		}
 	}
@@ -191,7 +192,7 @@ func (h *ActionSetHandler) UpdateActionSetSchedule(ctx context.Context, req *con
 
 	data := map[string]any{}
 	if req.Msg.Schedule != nil {
-		if schedule := scheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
+		if schedule := actionparams.ScheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
 			data["schedule"] = schedule
 		}
 	}
@@ -415,7 +416,7 @@ func (h *ActionSetHandler) actionSetToProto(s db.ActionSetsProjection) *pm.Actio
 		Description: s.Description,
 		MemberCount: s.MemberCount,
 		CreatedBy:   s.CreatedBy,
-		Schedule:    scheduleFromJSON(s.Schedule),
+		Schedule:    actionparams.ScheduleFromJSON(s.Schedule),
 	}
 
 	if s.CreatedAt != nil {

internal/api/definition_handler.go

@@ -9,6 +9,7 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
+	"github.com/manchtools/power-manage/server/internal/actionparams"
 	"github.com/manchtools/power-manage/server/internal/store"
 	db "github.com/manchtools/power-manage/server/internal/store/generated"
 )
@@ -49,7 +50,7 @@ func (h *DefinitionHandler) CreateDefinition(ctx context.Context, req *connect.R
 	// payload defensively — see action_set_handler.go.CreateActionSet
 	// for the rationale (default-schedule fallback in the projector).
 	if req.Msg.Schedule != nil {
-		if schedule := scheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
+		if schedule := actionparams.ScheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
 			data["schedule"] = schedule
 		}
 	}
@@ -189,7 +190,7 @@ func (h *DefinitionHandler) UpdateDefinitionSchedule(ctx context.Context, req *c
 
 	data := map[string]any{}
 	if req.Msg.Schedule != nil {
-		if schedule := scheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
+		if schedule := actionparams.ScheduleToMap(req.Msg.Schedule); len(schedule) > 0 {
 			data["schedule"] = schedule
 		}
 	}
@@ -413,7 +414,7 @@ func (h *DefinitionHandler) definitionToProto(d db.DefinitionsProjection) *pm.De
 		Description: d.Description,
 		MemberCount: d.MemberCount,
 		CreatedBy:   d.CreatedBy,
-		Schedule:    scheduleFromJSON(d.Schedule),
+		Schedule:    actionparams.ScheduleFromJSON(d.Schedule),
 	}
 
 	if d.CreatedAt != nil {

internal/api/internal_handler.go

@@ -177,7 +177,7 @@ func (h *InternalHandler) ProxySyncActions(ctx context.Context, req *connect.Req
 		}
 		groups = append(groups, &pm.ActionGroup{
 			SourceLabel: g.SourceLabel,
-			Schedule:    scheduleFromJSON(g.Schedule),
+			Schedule:    actionparams.ScheduleFromJSON(g.Schedule),
 			Actions:     groupActions,
 		})
 	}
@@ -232,7 +232,7 @@ func dbActionToWireAction(a db.ActionsProjection) *pm.Action {
 		actionparams.PopulateAction(action, a.ActionType, a.Params)
 	}
 	if len(a.Schedule) > 0 {
-		action.Schedule = scheduleFromJSON(a.Schedule)
+		action.Schedule = actionparams.ScheduleFromJSON(a.Schedule)
 	}
 	return action
 }
@@ -518,7 +518,7 @@ func dbResolvedActionToWireAction(a db.ListResolvedActionsForDeviceRow) *pm.Acti
 	}
 
 	if len(a.Schedule) > 0 {
-		action.Schedule = scheduleFromJSON(a.Schedule)
+		action.Schedule = actionparams.ScheduleFromJSON(a.Schedule)
 	}
 
 	return action

internal/api/service.go

@@ -55,7 +55,7 @@ type ControlServiceConfig struct {
 }
 
 // NewControlService creates a new control service.
-func NewControlService(st *store.Store, jwtManager *auth.JWTManager, signer ActionSigner, certAuth *ca.CA, gatewayURL string, logger *slog.Logger, enc *crypto.Encryptor, cfg ControlServiceConfig) *ControlService {
+func NewControlService(st *store.Store, jwtManager *auth.JWTManager, signer ca.ActionSigner, certAuth *ca.CA, gatewayURL string, logger *slog.Logger, enc *crypto.Encryptor, cfg ControlServiceConfig) *ControlService {
 	actionHandler := NewActionHandler(st, logger.With("component", "action_handler"), signer)
 	systemActions := NewSystemActionManager(st, signer, logger)
 	settingsHandler := NewSettingsHandler(st, logger, systemActions)

internal/api/system_actions.go

@@ -10,6 +10,7 @@ import (
 
 	pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
 	"github.com/manchtools/power-manage/server/internal/actionparams"
+	"github.com/manchtools/power-manage/server/internal/ca"
 	"github.com/manchtools/power-manage/server/internal/store"
 	db "github.com/manchtools/power-manage/server/internal/store/generated"
 )
@@ -35,12 +36,12 @@ import (
 // land on every device for permission holders" property.
 type SystemActionManager struct {
 	store  *store.Store
-	signer ActionSigner
+	signer ca.ActionSigner
 	logger *slog.Logger
 }
 
 // NewSystemActionManager creates a new system action manager.
-func NewSystemActionManager(st *store.Store, signer ActionSigner, logger *slog.Logger) *SystemActionManager {
+func NewSystemActionManager(st *store.Store, signer ca.ActionSigner, logger *slog.Logger) *SystemActionManager {
 	return &SystemActionManager{
 		store:  st,
 		signer: signer,

internal/ca/signer.go

@@ -2,6 +2,24 @@ package ca
 
 import "github.com/manchtools/power-manage/sdk/go/verify"
 
+// ActionSigner is the minimal contract dispatch / inbox handlers need
+// to sign action payloads so agents can verify authenticity. Defined
+// as an interface (vs the SDK's concrete *verify.ActionSigner) so
+// tests can substitute a NoOp implementation without dragging in the
+// real CA.
+//
+// A nil ActionSigner disables signing globally — this is intentional
+// for development bootstraps that haven't yet provisioned a CA, and
+// is checked at every dispatch site so production handlers fail
+// loudly rather than silently produce unsigned actions.
+//
+// Previously declared independently in internal/api/action_handler.go
+// and internal/control/inbox_worker.go; consolidated here so the two
+// can't drift.
+type ActionSigner interface {
+	Sign(actionID string, actionType int32, paramsJSON []byte) ([]byte, error)
+}
+
 // NewActionSigner creates a new action signer using the CA's private key.
 func NewActionSigner(ca *CA) *verify.ActionSigner {
 	return verify.NewActionSigner(ca.Signer())

internal/control/inbox_worker.go

@@ -15,34 +15,28 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 
 	pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
+	"github.com/manchtools/power-manage/server/internal/ca"
 	"github.com/manchtools/power-manage/server/internal/store"
 	db "github.com/manchtools/power-manage/server/internal/store/generated"
 	"github.com/manchtools/power-manage/server/internal/taskqueue"
 )
 
-// ActionSigner signs action payloads so agents can verify authenticity.
-// The Sign method returns the signature bytes for the given action ID,
-// type, and canonical params JSON.
-//
-// A nil ActionSigner disables signing globally (development only).
-// Note that dispatchPendingActions requires a non-nil signer and will
-// skip dispatch for any execution that references an action when the
-// signer is nil, since the agent rejects unsigned payloads.
-type ActionSigner interface {
-	Sign(actionID string, actionType int32, paramsJSON []byte) ([]byte, error)
-}
-
 // InboxWorker processes tasks from the control:inbox queue.
 // It replaces the PostgreSQL LISTEN-based Handler for gateway → control messages.
+//
+// signer follows the ca.ActionSigner contract: a nil signer disables
+// signing (dev mode); dispatchPendingActions skips any execution
+// referencing an action when signer is nil, since agents reject
+// unsigned payloads.
 type InboxWorker struct {
 	store    *store.Store
 	aqClient *taskqueue.Client
-	signer   ActionSigner
+	signer   ca.ActionSigner
 	logger   *slog.Logger
 }
 
 // NewInboxWorker creates a new inbox worker.
-func NewInboxWorker(st *store.Store, aqClient *taskqueue.Client, signer ActionSigner, logger *slog.Logger) *InboxWorker {
+func NewInboxWorker(st *store.Store, aqClient *taskqueue.Client, signer ca.ActionSigner, logger *slog.Logger) *InboxWorker {
 	return &InboxWorker{
 		store:    st,
 		aqClient: aqClient,

internal/handler/agent.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"net"
 	"net/http"
 	"time"
 
@@ -225,9 +226,16 @@ func BootstrapRedirectMiddleware(next http.Handler, bootstrapHost, assignedHost
 		// :authority pseudo-header). Strip any port so we compare
 		// just the hostname — the agent may include the port,
 		// the bootstrap config typically doesn't.
+		//
+		// net.SplitHostPort handles bracketed IPv6 authorities
+		// ([2001:db8::1]:443) correctly; falling back to the raw
+		// r.Host when there is no port keeps unbracketed IPv4 /
+		// hostname inputs working unchanged. Audit / CR catch:
+		// strings.IndexByte(':') broke IPv6 by truncating at the
+		// first internal colon and producing reqHost == "[".
 		reqHost := r.Host
-		if i := indexByte(reqHost, ':'); i >= 0 {
-			reqHost = reqHost[:i]
+		if h, _, err := net.SplitHostPort(reqHost); err == nil {
+			reqHost = h
 		}
 		if reqHost != bootstrapHost {
 			next.ServeHTTP(w, r)
@@ -243,17 +251,6 @@ func BootstrapRedirectMiddleware(next http.Handler, bootstrapHost, assignedHost
 	})
 }
 
-// indexByte is a tiny stdlib-free helper so this file doesn't need
-// the strings import for one call.
-func indexByte(s string, c byte) int {
-	for i := 0; i < len(s); i++ {
-		if s[i] == c {
-			return i
-		}
-	}
-	return -1
-}
-
 // Stream handles the bidirectional stream between agent and server.
 func (h *AgentHandler) Stream(ctx context.Context, stream *connect.BidiStream[pm.AgentMessage, pm.ServerMessage]) (err error) {
 	// Recover from panics to prevent server crashes