refactor: collapse action justification into a string template field (#1067)

Makefile

@@ -57,6 +57,7 @@ devrun: compile
 devcontainer: compile podman-image podman-container
 
 webui-dist:
+	$(call delete-files,webui)
 	$(MAKE) -wC frontend dist
 	mv frontend/dist webui
 

config.yaml

@@ -133,7 +133,8 @@ actions:
   # Docs: https://docs.olivetin.app/args/input_confirmation.html
   - title: Delete old backups
     icon: ashtonished
-    justification: true
+    # A single space requires justification with no prefilled template (empty disables it).
+    justification: " "
     shell: rm -rf /opt/oliveTinOldBackups/ && sleep 5
     arguments:
       - type: html

frontend/resources/scripts/gen/olivetin/api/v1/olivetin_pb.d.ts

@@ -92,9 +92,9 @@ export declare type Action = Message<"olivetin.api.v1.Action"> & {
   execOnWebhooks: ActionWebhookExecHint[];
 
   /**
-   * @generated from field: bool justification = 16;
+   * @generated from field: string justification = 20;
    */
-  justification: boolean;
+  justification: string;
 
   /**
    * @generated from field: bool has_running_instance = 17;

frontend/resources/vue/utils/justificationTemplate.js

@@ -0,0 +1,15 @@
+export function applyArgumentTemplate(template, args) {
+  if (!template) {
+    return ''
+  }
+
+  return template.replace(/\{\{\s*([a-zA-Z0-9_]+)\s*\}\}/g, (_, name) => args[name] ?? '')
+}
+
+export function actionRequiresJustification(justification) {
+  return (justification ?? '').length > 0
+}
+
+export function actionJustificationTemplate(justification) {
+  return justification ?? ''
+}

frontend/resources/vue/utils/needsArgumentForm.js

@@ -1,3 +1,5 @@
+import { actionRequiresJustification } from './justificationTemplate.js'
+
 export function needsArgumentForm (action) {
-  return (action?.arguments?.length > 0) || action?.justification
+  return (action?.arguments?.length > 0) || actionRequiresJustification(action?.justification)
 }

frontend/resources/vue/utils/rerunArguments.js

@@ -1,4 +1,5 @@
 import { needsArgumentForm } from './needsArgumentForm.js'
+import { actionRequiresJustification } from './justificationTemplate.js'
 
 const nonStorableArgumentTypes = new Set([
   'password',
@@ -17,7 +18,7 @@ export function logEntryArgumentsToStartActionArgs (logEntry) {
 }
 
 export function rerunNeedsArgumentForm (action, logEntry) {
-  if (action?.justification && !logEntry?.justification) {
+  if (actionRequiresJustification(action?.justification) && !logEntry?.justification) {
     return true
   }
 
@@ -60,7 +61,7 @@ export function buildRerunStartActionArgs (bindingId, logEntry, action) {
     arguments: logEntryArgumentsToStartActionArgs(logEntry)
   }
 
-  if (action?.justification && logEntry?.justification) {
+  if (actionRequiresJustification(action?.justification) && logEntry?.justification) {
     startActionArgs.justification = logEntry.justification
   }
 

frontend/resources/vue/utils/rerunArguments.test.mjs

@@ -75,7 +75,7 @@ test('rerunNeedsArgumentForm can start directly when stored args are complete',
 })
 
 test('rerunNeedsArgumentForm opens the form when justification is missing', () => {
-  const action = { justification: true, arguments: [] }
+  const action = { justification: ' ', arguments: [] }
 
   assert.equal(rerunNeedsArgumentForm(action, {}), true)
   assert.equal(
@@ -90,7 +90,7 @@ test('buildRerunStartActionArgs includes stored justification', () => {
       arguments: [{ name: 'host', value: 'db-1' }],
       justification: 'maintenance window'
     }, {
-      justification: true,
+      justification: ' ',
       arguments: [{ name: 'host', type: 'ascii_identifier' }]
     }),
     {

frontend/resources/vue/views/ActionDetailsView.vue

@@ -96,7 +96,9 @@
               <td class="duration">{{ formatExecutionDuration(log) }}</td>
               <td>
                 <router-link :to="`/logs/${log.executionTrackingId}`">
-                  {{ log.executionTrackingId }}
+                  <LogActionTitle :justification="log.justification">
+                    {{ log.executionTrackingId }}
+                  </LogActionTitle>
                 </router-link>
               </td>
               <td class="tags">
@@ -134,6 +136,7 @@ import Section from 'picocrank/vue/components/Section.vue'
 import ActionIconGlyph from '../components/ActionIconGlyph.vue'
 import ActionStatusDisplay from '../components/ActionStatusDisplay.vue'
 import ActionGroupLimitsLabel from '../components/ActionGroupLimitsLabel.vue'
+import LogActionTitle from '../components/LogActionTitle.vue'
 import { HugeiconsIcon } from '@hugeicons/vue'
 import { DashboardSquare01Icon, WorkoutRunIcon } from '@hugeicons/core-free-icons'
 import { requestReconnectNow } from '../../../js/websocket.js'
@@ -162,7 +165,8 @@ const filteredLogs = computed(() => {
   const searchLower = searchText.value.toLowerCase()
   return logs.value.filter(log =>
     log.executionTrackingId.toLowerCase().includes(searchLower) ||
-    log.actionTitle.toLowerCase().includes(searchLower)
+    log.actionTitle.toLowerCase().includes(searchLower) ||
+    (log.justification || '').toLowerCase().includes(searchLower)
   )
 })
 

frontend/resources/vue/views/ArgumentForm.vue

@@ -70,6 +70,11 @@ import { useRouter } from 'vue-router'
 import { requestReconnectNow } from '../../../js/websocket.js'
 import ChoiceCombobox from '../components/ChoiceCombobox.vue'
 import ChoiceChecklist from '../components/ChoiceChecklist.vue'
+import {
+  actionJustificationTemplate,
+  actionRequiresJustification,
+  applyArgumentTemplate
+} from '../utils/justificationTemplate.js'
 
 const router = useRouter()
 
@@ -114,8 +119,7 @@ async function setup() {
     icon.value = action.icon
     popupOnStart.value = action.popupOnStart || ''
     actionArguments.value = action.arguments || []
-    justificationRequired.value = action.justification || false
+    justificationRequired.value = actionRequiresJustification(action.justification)
-    justificationValue.value = ''
     argValues.value = {}
     formErrors.value = {}
     confirmationChecked.value = false
@@ -151,6 +155,12 @@ async function setup() {
     }
     })
 
+    const prefilledJustification = applyArgumentTemplate(
+      actionJustificationTemplate(action.justification),
+      argValues.value
+    )
+    justificationValue.value = prefilledJustification.trim() === '' ? '' : prefilledJustification
+
     // Run initial validation on all fields after DOM is updated
     await nextTick()
     for (const arg of actionArguments.value) {

integration-tests/Makefile

@@ -1,8 +1,12 @@
-default: test-install test-run
+default: test-install prep test-run
 
 test-install:
 	npm install --no-fund
 
+prep:
+	$(MAKE) -wC .. webui-dist
+	$(MAKE) -wC ../service compile-currentenv
+
 test-run:
     # GitHub Actions fails badly on the default timeout of 2000ms
 	npx mocha tests --recursive -t 10000
@@ -24,4 +28,4 @@ getsnapshot:
 	rm -rf /opt/OliveTin-snapshot/*
 	gh run download -D /opt/OliveTin-snapshot/
 
-.PHONY: default find-flakey-tests find-flakey-tests-inf
+.PHONY: default find-flakey-tests find-flakey-tests-inf prep

proto/olivetin/api/v1/olivetin.proto

@@ -20,7 +20,8 @@ message Action {
 	repeated string exec_on_file_changed_in_dir = 13;
 	string exec_on_calendar_file = 14;
 	repeated ActionWebhookExecHint exec_on_webhooks = 15;
-	bool justification = 16;
+	reserved 16;
+	string justification = 20;
 	bool has_running_instance = 17;
 	bool has_queued_instance = 18;
 	repeated ActionGroupMembership groups = 19;

service/gen/olivetin/api/v1/olivetin.pb.go

@@ -38,7 +38,7 @@ type Action struct {
 	ExecOnFileChangedInDir   []string                 `protobuf:"bytes,13,rep,name=exec_on_file_changed_in_dir,json=execOnFileChangedInDir,proto3" json:"exec_on_file_changed_in_dir,omitempty"`
 	ExecOnCalendarFile       string                   `protobuf:"bytes,14,opt,name=exec_on_calendar_file,json=execOnCalendarFile,proto3" json:"exec_on_calendar_file,omitempty"`
 	ExecOnWebhooks           []*ActionWebhookExecHint `protobuf:"bytes,15,rep,name=exec_on_webhooks,json=execOnWebhooks,proto3" json:"exec_on_webhooks,omitempty"`
-	Justification            bool                     `protobuf:"varint,16,opt,name=justification,proto3" json:"justification,omitempty"`
+	Justification            string                   `protobuf:"bytes,20,opt,name=justification,proto3" json:"justification,omitempty"`
 	HasRunningInstance       bool                     `protobuf:"varint,17,opt,name=has_running_instance,json=hasRunningInstance,proto3" json:"has_running_instance,omitempty"`
 	HasQueuedInstance        bool                     `protobuf:"varint,18,opt,name=has_queued_instance,json=hasQueuedInstance,proto3" json:"has_queued_instance,omitempty"`
 	Groups                   []*ActionGroupMembership `protobuf:"bytes,19,rep,name=groups,proto3" json:"groups,omitempty"`
@@ -181,11 +181,11 @@ func (x *Action) GetExecOnWebhooks() []*ActionWebhookExecHint {
 	return nil
 }
 
-func (x *Action) GetJustification() bool {
+func (x *Action) GetJustification() string {
 	if x != nil {
 		return x.Justification
 	}
-	return false
+	return ""
 }
 
 func (x *Action) GetHasRunningInstance() bool {
@@ -4575,7 +4575,7 @@ var File_olivetin_api_v1_olivetin_proto protoreflect.FileDescriptor
 
 const file_olivetin_api_v1_olivetin_proto_rawDesc = "" +
 	"\n" +
-	"\x1eolivetin/api/v1/olivetin.proto\x12\x0folivetin.api.v1\"\xd1\x06\n" +
+	"\x1eolivetin/api/v1/olivetin.proto\x12\x0folivetin.api.v1\"\xd7\x06\n" +
 	"\x06Action\x12\x1d\n" +
 	"\n" +
 	"binding_id\x18\x01 \x01(\tR\tbindingId\x12\x14\n" +
@@ -4595,10 +4595,10 @@ const file_olivetin_api_v1_olivetin_proto_rawDesc = "" +
 	"\x1bexec_on_file_changed_in_dir\x18\r \x03(\tR\x16execOnFileChangedInDir\x121\n" +
 	"\x15exec_on_calendar_file\x18\x0e \x01(\tR\x12execOnCalendarFile\x12P\n" +
 	"\x10exec_on_webhooks\x18\x0f \x03(\v2&.olivetin.api.v1.ActionWebhookExecHintR\x0eexecOnWebhooks\x12$\n" +
-	"\rjustification\x18\x10 \x01(\bR\rjustification\x120\n" +
+	"\rjustification\x18\x14 \x01(\tR\rjustification\x120\n" +
 	"\x14has_running_instance\x18\x11 \x01(\bR\x12hasRunningInstance\x12.\n" +
 	"\x13has_queued_instance\x18\x12 \x01(\bR\x11hasQueuedInstance\x12>\n" +
-	"\x06groups\x18\x13 \x03(\v2&.olivetin.api.v1.ActionGroupMembershipR\x06groups\"q\n" +
+	"\x06groups\x18\x13 \x03(\v2&.olivetin.api.v1.ActionGroupMembershipR\x06groupsJ\x04\b\x10\x10\x11\"q\n" +
 	"\x15ActionGroupMembership\x12\x12\n" +
 	"\x04name\x18\x01 \x01(\tR\x04name\x12%\n" +
 	"\x0emax_concurrent\x18\x02 \x01(\x05R\rmaxConcurrent\x12\x1d\n" +

service/internal/api/api.go

@@ -156,15 +156,17 @@ func (api *oliveTinAPI) StartAction(ctx ctx.Context, req *connect.Request[apiv1.
 	}
 
 	authenticatedUser := auth.UserFromApiCall(ctx, req, api.cfg)
-	if err := validateJustificationRequired(pair.Action, req.Msg.Justification, authenticatedUser); err != nil {
+	args := startActionArgumentsFromProto(req.Msg.Arguments)
+	justification := resolveStartJustification(pair.Action, pair, req.Msg.Justification, args)
+	if err := validateJustificationRequired(pair.Action, justification, authenticatedUser); err != nil {
 		return nil, connectInvalidJustification(err)
 	}
 
 	execReq := executor.ExecutionRequest{
 		Binding:           pair,
 		TrackingID:        req.Msg.UniqueTrackingId,
-		Arguments:         startActionArgumentsFromProto(req.Msg.Arguments),
+		Arguments:         args,
-		Justification:     req.Msg.Justification,
+		Justification:     justification,
 		AuthenticatedUser: authenticatedUser,
 		Cfg:               api.cfg,
 	}
@@ -287,11 +289,13 @@ func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request
 	}
 
 	user := auth.UserFromApiCall(ctx, req, api.cfg)
-	if err := validateJustificationRequired(binding.Action, req.Msg.Justification, user); err != nil {
+	args := startActionArgumentsFromProto(req.Msg.Arguments)
+	justification := resolveStartJustification(binding.Action, binding, req.Msg.Justification, args)
+	if err := validateJustificationRequired(binding.Action, justification, user); err != nil {
 		return nil, connectInvalidJustification(err)
 	}
 
-	internalLogEntry, ok := api.startActionAndWaitRun(binding, startActionArgumentsFromProto(req.Msg.Arguments), req.Msg.Justification, user)
+	internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user)
 	if !ok {
 		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
 	}

service/internal/api/api_justification.go

@@ -9,7 +9,10 @@ import (
 	apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
 	authpublic "github.com/OliveTin/OliveTin/internal/auth/authpublic"
 	"github.com/OliveTin/OliveTin/internal/config"
+	"github.com/OliveTin/OliveTin/internal/entities"
 	"github.com/OliveTin/OliveTin/internal/executor"
+	"github.com/OliveTin/OliveTin/internal/tpl"
+	log "github.com/sirupsen/logrus"
 )
 
 func validateJustificationRequired(action *config.Action, justification string, user *authpublic.AuthenticatedUser) error {
@@ -21,7 +24,7 @@ func validateJustificationRequired(action *config.Action, justification string,
 }
 
 func actionRequiresJustificationConfig(action *config.Action) bool {
-	return action != nil && action.Justification
+	return action != nil && action.RequiresJustification()
 }
 
 func justificationProvided(justification string, user *authpublic.AuthenticatedUser) bool {
@@ -40,6 +43,41 @@ func startActionArgumentsFromProto(args []*apiv1.StartActionArgument) map[string
 	return result
 }
 
+func resolveStartJustification(action *config.Action, binding *executor.ActionBinding, clientJustification string, args map[string]string) string {
+	if strings.TrimSpace(clientJustification) != "" {
+		return clientJustification
+	}
+
+	return resolveJustificationFromTemplate(action, binding, clientJustification, args)
+}
+
+func resolveJustificationFromTemplate(action *config.Action, binding *executor.ActionBinding, fallback string, args map[string]string) string {
+	templateText := action.JustificationTemplateText()
+	if templateText == "" {
+		return fallback
+	}
+
+	resolved, err := tpl.ParseTemplateWithActionContext(templateText, bindingEntity(binding), args)
+	if err != nil {
+		log.WithFields(log.Fields{
+			"template": templateText,
+			"entity":   bindingEntity(binding),
+			"error":    err,
+		}).Warn("Failed to resolve justification template")
+		return fallback
+	}
+
+	return resolved
+}
+
+func bindingEntity(binding *executor.ActionBinding) *entities.Entity {
+	if binding == nil {
+		return nil
+	}
+
+	return binding.Entity
+}
+
 func restartRequiresJustificationError() error {
 	return connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("justification is required for this action; use StartAction with a justification instead"))
 }

service/internal/api/api_justification_test.go

@@ -21,7 +21,7 @@ func TestStartActionRequiresJustificationForGuest(t *testing.T) {
 	action := &config.Action{
 		Title:         "Send email",
 		ID:            "send_email",
-		Justification: true,
+		Justification: config.JustificationRequiredNoTemplate,
 		Shell:         "echo done",
 	}
 	cfg.Actions = append(cfg.Actions, action)
@@ -56,12 +56,12 @@ func TestStartActionRequiresJustificationForGuest(t *testing.T) {
 	assert.Equal(t, "New user registration foo@example.com", entry.Justification)
 }
 
-func TestBuildActionExposesJustificationFlag(t *testing.T) {
+func TestBuildActionExposesJustificationTemplate(t *testing.T) {
 	cfg := config.DefaultConfig()
 	action := &config.Action{
 		Title:         "Audited action",
 		ID:            "audited",
-		Justification: true,
+		Justification: "{{ target }}",
 		Shell:         "echo hi",
 	}
 	cfg.Actions = append(cfg.Actions, action)
@@ -77,12 +77,98 @@ func TestBuildActionExposesJustificationFlag(t *testing.T) {
 	})
 
 	require.NotNil(t, pb)
-	assert.True(t, pb.Justification)
+	assert.Equal(t, "{{ target }}", pb.Justification)
+}
+
+func TestBuildActionExposesBlankRequiredJustification(t *testing.T) {
+	cfg := config.DefaultConfig()
+	action := &config.Action{
+		Title:         "Audited action",
+		ID:            "audited",
+		Justification: config.JustificationRequiredNoTemplate,
+		Shell:         "echo hi",
+	}
+	cfg.Actions = append(cfg.Actions, action)
+
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	binding := ex.FindBindingWithNoEntity(action)
+	require.NotNil(t, binding)
+
+	pb := buildAction(binding, &DashboardRenderRequest{
+		cfg: cfg,
+		ex:  ex,
+	})
+
+	require.NotNil(t, pb)
+	assert.Equal(t, config.JustificationRequiredNoTemplate, pb.Justification)
+}
+
+func TestResolveStartJustificationUsesTemplateWhenClientValueEmpty(t *testing.T) {
+	action := &config.Action{
+		Justification: "{{ ansible_host }}",
+	}
+	binding := &executor.ActionBinding{}
+
+	got := resolveStartJustification(action, binding, "", map[string]string{
+		"ansible_host": "192.168.66.8",
+	})
+	assert.Equal(t, "192.168.66.8", got)
+}
+
+func TestResolveStartJustificationPrefersClientValue(t *testing.T) {
+	action := &config.Action{
+		Justification: "{{ ansible_host }}",
+	}
+	binding := &executor.ActionBinding{}
+
+	got := resolveStartJustification(action, binding, "manual reason", map[string]string{
+		"ansible_host": "192.168.66.8",
+	})
+	assert.Equal(t, "manual reason", got)
+}
+
+func TestStartActionResolvesJustificationTemplateForGuest(t *testing.T) {
+	cfg := config.DefaultConfig()
+	action := &config.Action{
+		Title:         "Run playbook",
+		ID:            "run_playbook",
+		Justification: "{{ ansible_host }}",
+		Shell:         "echo done",
+		Arguments: []config.ActionArgument{
+			{Name: "ansible_host", Title: "Host"},
+		},
+	}
+	cfg.Actions = append(cfg.Actions, action)
+
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	binding := ex.FindBindingWithNoEntity(action)
+	require.NotNil(t, binding)
+
+	ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
+	defer ts.Close()
+
+	resp, err := client.StartAction(context.Background(), connect.NewRequest(&apiv1.StartActionRequest{
+		BindingId:        binding.ID,
+		UniqueTrackingId: uuid.NewString(),
+		Arguments: []*apiv1.StartActionArgument{
+			{Name: "ansible_host", Value: "stuffbox"},
+		},
+	}))
+	require.NoError(t, err)
+	require.NotEmpty(t, resp.Msg.ExecutionTrackingId)
+
+	time.Sleep(200 * time.Millisecond)
+
+	entry, ok := ex.GetLog(resp.Msg.ExecutionTrackingId)
+	require.True(t, ok)
+	assert.Equal(t, "stuffbox", entry.Justification)
 }
 
 func TestValidateJustificationRequiredAllowsSystemUser(t *testing.T) {
 	cfg := config.DefaultConfig()
-	action := &config.Action{Title: "Cron job", Justification: true}
+	action := &config.Action{Title: "Cron job", Justification: config.JustificationRequiredNoTemplate}
 
 	err := validateJustificationRequired(action, "", auth.UserFromSystem(cfg, "cron"))
 	require.NoError(t, err)

service/internal/api/api_log_arguments.go

@@ -47,7 +47,7 @@ func restartArgumentsIncompleteError() error {
 }
 
 func validateRestartLogEntry(entry *executor.InternalLogEntry) error {
-	if entry.Binding.Action.Justification && strings.TrimSpace(entry.Justification) == "" {
+	if entry.Binding.Action.RequiresJustification() && strings.TrimSpace(entry.Justification) == "" {
 		return restartRequiresJustificationError()
 	}
 

service/internal/api/api_log_arguments_test.go

@@ -286,7 +286,7 @@ func TestRestartActionRequiresJustificationWhenMissingFromStoredLog(t *testing.T
 			Title:         "Dangerous action",
 			Shell:         "echo ok",
 			MaxConcurrent: 1,
-			Justification: true,
+			Justification: " ",
 		},
 	}
 
@@ -319,7 +319,7 @@ func TestRestartActionReusesStoredJustificationViaStartActionPath(t *testing.T)
 			Title:         "Dangerous action",
 			Shell:         "echo ok",
 			MaxConcurrent: 1,
-			Justification: true,
+			Justification: " ",
 		},
 	}
 

service/internal/config/config.go

@@ -7,6 +7,9 @@ import (
 // ReservedArgumentNamePrefix is reserved for OliveTin-injected system arguments.
 const ReservedArgumentNamePrefix = "ot_"
 
+// JustificationRequiredNoTemplate requires a manual justification with no prefilled template.
+const JustificationRequiredNoTemplate = " "
+
 // Action represents the core functionality of OliveTin - commands that show up
 // as buttons in the UI.
 type Action struct {
@@ -35,7 +38,19 @@ type Action struct {
 	SaveLogs               SaveLogsConfig   `koanf:"saveLogs"`
 	EnabledExpression      string           `koanf:"enabledExpression"`
 	Groups                 []string         `koanf:"groups"`
-	Justification          bool             `koanf:"justification"`
+	Justification          string           `koanf:"justification"`
+}
+
+func (action *Action) RequiresJustification() bool {
+	return action != nil && action.Justification != ""
+}
+
+func (action *Action) JustificationTemplateText() string {
+	if !action.RequiresJustification() {
+		return ""
+	}
+
+	return action.Justification
 }
 
 // ActionGroup defines shared limits and metadata for a set of actions.

service/internal/config/config_reloader.go

@@ -55,6 +55,7 @@ func unmarshalRoot(k *koanf.Koanf, cfg *Config) bool {
 		DecoderConfig: &mapstructure.DecoderConfig{
 			DecodeHook: mapstructure.ComposeDecodeHookFunc(
 				envDecodeHookFunc,
+				justificationDecodeHookFunc,
 				mapstructure.StringToTimeDurationHookFunc(),
 				mapstructure.TextUnmarshallerHookFunc(),
 			),
@@ -259,6 +260,18 @@ func mergeFunc(src map[string]interface{}, dest map[string]interface{}) error {
 
 var envRegex = regexp.MustCompile(`\${{ *?(\S+) *?}}`)
 
+func justificationDecodeHookFunc(from reflect.Type, to reflect.Type, data any) (any, error) {
+	if to.Kind() != reflect.String || from.Kind() != reflect.Bool {
+		return data, nil
+	}
+
+	if data.(bool) {
+		return JustificationRequiredNoTemplate, nil
+	}
+
+	return "", nil
+}
+
 func envDecodeHookFunc(from reflect.Type, to reflect.Type, data any) (any, error) {
 	log.Debugf("envDecodeHookFunc called: from=%v, to=%v, data=%v", from, to, data)
 	if from.Kind() != reflect.String {

service/internal/config/justification_compat_test.go

@@ -0,0 +1,58 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/knadh/koanf/parsers/yaml"
+	"github.com/knadh/koanf/providers/rawbytes"
+	"github.com/knadh/koanf/v2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestJustificationDecodeHookMigratesLegacyBooleanFalse(t *testing.T) {
+	cfg := loadJustificationCompatConfig(t, `
+actions:
+  - title: Legacy disabled
+    shell: echo hi
+    justification: false
+`)
+
+	require.Len(t, cfg.Actions, 1)
+	assert.Empty(t, cfg.Actions[0].Justification)
+}
+
+func TestJustificationDecodeHookMigratesLegacyBooleanTrue(t *testing.T) {
+	cfg := loadJustificationCompatConfig(t, `
+actions:
+  - title: Legacy required
+    shell: echo hi
+    justification: true
+`)
+
+	require.Len(t, cfg.Actions, 1)
+	assert.Equal(t, JustificationRequiredNoTemplate, cfg.Actions[0].Justification)
+}
+
+func TestSanitizeJustificationMigratesWeaklyTypedLegacyStrings(t *testing.T) {
+	action := &Action{Justification: "false"}
+	action.sanitizeJustification()
+	assert.Empty(t, action.Justification)
+
+	action.Justification = "true"
+	action.sanitizeJustification()
+	assert.Equal(t, JustificationRequiredNoTemplate, action.Justification)
+}
+
+func loadJustificationCompatConfig(t *testing.T, yamlBody string) *Config {
+	t.Helper()
+
+	k := koanf.New(".")
+	require.NoError(t, k.Load(rawbytes.Provider([]byte(yamlBody)), yaml.Parser()))
+
+	cfg := DefaultConfig()
+	require.True(t, unmarshalRoot(k, cfg))
+	cfg.Sanitize()
+
+	return cfg
+}

service/internal/config/sanitize.go

@@ -227,6 +227,7 @@ func (action *Action) sanitize(cfg *Config) {
 	action.ID = getActionID(action)
 	action.Icon = lookupHTMLIcon(action.Icon, cfg.DefaultIconForActions)
 	migrateActionOnClick(action)
+	action.sanitizeJustification()
 	action.OnClick = sanitizeOnClick(action.OnClick, cfg)
 	action.PopupOnStart = action.OnClick
 
@@ -449,6 +450,15 @@ func migrateActionOnClick(action *Action) {
 	}
 }
 
+func (action *Action) sanitizeJustification() {
+	switch action.Justification {
+	case "false":
+		action.Justification = ""
+	case "true":
+		action.Justification = JustificationRequiredNoTemplate
+	}
+}
+
 func shouldMigrateDefaultOnClickFromPopup(onClick, popupOnStart string) bool {
 	if popupOnStart == "" {
 		return false

service/internal/executor/justification.go

@@ -45,7 +45,7 @@ func ResolveJustification(req *ExecutionRequest) string {
 }
 
 func actionRequiresJustification(req *ExecutionRequest) bool {
-	return req != nil && req.Binding != nil && req.Binding.Action != nil && req.Binding.Action.Justification
+	return req != nil && req.Binding != nil && req.Binding.Action != nil && req.Binding.Action.RequiresJustification()
 }
 
 func defaultJustificationForRequest(req *ExecutionRequest) string {

service/internal/executor/justification_test.go

@@ -11,7 +11,7 @@ import (
 
 func TestResolveJustificationUsesProvidedValue(t *testing.T) {
 	cfg := config.DefaultConfig()
-	action := &config.Action{Title: "Send email", Justification: true, Shell: "echo hi"}
+	action := &config.Action{Title: "Send email", Justification: " ", Shell: "echo hi"}
 	cfg.Actions = append(cfg.Actions, action)
 	ex := DefaultExecutor(cfg)
 	ex.RebuildActionMap()
@@ -29,7 +29,7 @@ func TestResolveJustificationUsesProvidedValue(t *testing.T) {
 
 func TestResolveJustificationCronDefault(t *testing.T) {
 	cfg := config.DefaultConfig()
-	action := &config.Action{Title: "Nightly backup", Justification: true, Shell: "echo hi"}
+	action := &config.Action{Title: "Nightly backup", Justification: " ", Shell: "echo hi"}
 	cfg.Actions = append(cfg.Actions, action)
 	ex := DefaultExecutor(cfg)
 	ex.RebuildActionMap()
@@ -45,7 +45,7 @@ func TestResolveJustificationCronDefault(t *testing.T) {
 
 func TestResolveJustificationStartupDefault(t *testing.T) {
 	cfg := config.DefaultConfig()
-	action := &config.Action{Title: "Init", Justification: true, Shell: "echo hi"}
+	action := &config.Action{Title: "Init", Justification: " ", Shell: "echo hi"}
 	cfg.Actions = append(cfg.Actions, action)
 	ex := DefaultExecutor(cfg)
 	ex.RebuildActionMap()
@@ -61,7 +61,7 @@ func TestResolveJustificationStartupDefault(t *testing.T) {
 
 func TestResolveJustificationWebhookDefault(t *testing.T) {
 	cfg := config.DefaultConfig()
-	action := &config.Action{Title: "Deploy", Justification: true, Exec: []string{"echo", "deploy"}}
+	action := &config.Action{Title: "Deploy", Justification: " ", Exec: []string{"echo", "deploy"}}
 	cfg.Actions = append(cfg.Actions, action)
 	ex := DefaultExecutor(cfg)
 	ex.RebuildActionMap()
@@ -95,7 +95,7 @@ func TestJustificationNotPassedToShellArgs(t *testing.T) {
 	cfg := config.DefaultConfig()
 	action := &config.Action{
 		Title:         "Echo",
-		Justification: true,
+		Justification: " ",
 		Shell:         "echo {{ message }}",
 		Arguments: []config.ActionArgument{
 			{Name: "message", Type: "ascii_sentence"},

var/macos/config.yaml

@@ -147,7 +147,7 @@ actions:
   # Using a path under the user's home is more natural on macOS.
   - title: Delete old backups
     icon: ashtonished
-    justification: true
+    justification: " "
     shell: rm -rf "$HOME/Backups/old/"
     arguments:
       - name: confirm