Просмотр исходного кода

refactor: collapse action justification into a string template field

Use a single justification string for both requirement and templating so
empty values skip prompts while templates can prefill audit reasons at start.

Co-authored-by: Cursor <cursoragent@cursor.com>
jamesread 16 часов назад
Родитель
Сommit
a530dca579

+ 1 - 1
config.yaml

@@ -133,7 +133,7 @@ actions:
   # Docs: https://docs.olivetin.app/args/input_confirmation.html
   - title: Delete old backups
     icon: ashtonished
-    justification: true
+    justification: " "
     shell: rm -rf /opt/oliveTinOldBackups/ && sleep 5
     arguments:
       - type: html

+ 2 - 2
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 = 16;
    */
-  justification: boolean;
+  justification: string;
 
   /**
    * @generated from field: bool has_running_instance = 17;

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
frontend/resources/scripts/gen/olivetin/api/v1/olivetin_pb.js


+ 15 - 0
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 ?? ''
+}

+ 3 - 1
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)
 }

+ 3 - 2
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
   }
 

+ 2 - 2
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' }]
     }),
     {

+ 6 - 2
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)
   )
 })
 

+ 1 - 1
proto/olivetin/api/v1/olivetin.proto

@@ -20,7 +20,7 @@ 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;
+	string justification = 16;
 	bool has_running_instance = 17;
 	bool has_queued_instance = 18;
 	repeated ActionGroupMembership groups = 19;

+ 4 - 4
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,16,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 {
@@ -4595,7 +4595,7 @@ 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\x10 \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" +

+ 9 - 5
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),
-		Justification:     req.Msg.Justification,
+		Arguments:         args,
+		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"))
 	}

+ 33 - 1
service/internal/api/api_justification.go

@@ -9,7 +9,9 @@ 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"
 )
 
 func validateJustificationRequired(action *config.Action, justification string, user *authpublic.AuthenticatedUser) error {
@@ -21,7 +23,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 +42,36 @@ 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 {
+		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"))
 }

+ 91 - 5
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: " ",
 		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: " ",
+		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, " ", 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: " "}
 
 	err := validateJustificationRequired(action, "", auth.UserFromSystem(cfg, "cron"))
 	require.NoError(t, err)

+ 1 - 1
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()
 	}
 

+ 2 - 2
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: " ",
 		},
 	}
 

+ 13 - 1
service/internal/config/config.go

@@ -35,7 +35,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 == nil || !action.RequiresJustification() {
+		return ""
+	}
+
+	return action.Justification
 }
 
 // ActionGroup defines shared limits and metadata for a set of actions.

+ 1 - 1
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 {

+ 5 - 5
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"},

+ 1 - 1
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

Некоторые файлы не были показаны из-за большого количества измененных файлов