fix: several merge messes

This commit is contained in:
jamesread 2026-06-28 22:52:16 +01:00
parent e63094e8e8
commit 57f3ef46ad
8 changed files with 207 additions and 4 deletions

View File

@ -1,5 +1,14 @@
import { needsArgumentForm } from './needsArgumentForm.js' import { needsArgumentForm } from './needsArgumentForm.js'
const nonStorableArgumentTypes = new Set([
'password',
'very_dangerous_raw_string'
])
function isNonStorableArgumentType (type) {
return nonStorableArgumentTypes.has(type)
}
export function logEntryArgumentsToStartActionArgs (logEntry) { export function logEntryArgumentsToStartActionArgs (logEntry) {
return (logEntry?.arguments ?? []).map((arg) => ({ return (logEntry?.arguments ?? []).map((arg) => ({
name: arg.name, name: arg.name,
@ -23,7 +32,7 @@ export function hasMissingRerunArguments (action, storedArgs) {
const stored = new Map(storedArgs.map((arg) => [arg.name, arg.value])) const stored = new Map(storedArgs.map((arg) => [arg.name, arg.value]))
for (const arg of action?.arguments ?? []) { for (const arg of action?.arguments ?? []) {
if (arg.type === 'password') { if (isNonStorableArgumentType(arg.type)) {
return true return true
} }

View File

@ -37,6 +37,20 @@ test('hasMissingRerunArguments requires password fields to be re-entered', () =>
) )
}) })
test('hasMissingRerunArguments requires very_dangerous_raw_string fields to be re-entered', () => {
const action = {
arguments: [
{ name: 'host', type: 'ascii_identifier', required: true },
{ name: 'payload', type: 'very_dangerous_raw_string', required: false }
]
}
assert.equal(
hasMissingRerunArguments(action, [{ name: 'host', value: 'db-1' }]),
true
)
})
test('hasMissingRerunArguments detects missing required stored arguments', () => { test('hasMissingRerunArguments detects missing required stored arguments', () => {
const action = { const action = {
arguments: [{ name: 'host', type: 'ascii_identifier', required: true }] arguments: [{ name: 'host', type: 'ascii_identifier', required: true }]

View File

@ -7,7 +7,6 @@ import (
"os" "os"
"path" "path"
"sort" "sort"
"strings"
"connectrpc.com/connect" "connectrpc.com/connect"
"google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/encoding/protojson"
@ -1550,8 +1549,8 @@ func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv
return nil, err return nil, err
} }
if execReqLogEntry.Binding.Action.Justification && strings.TrimSpace(execReqLogEntry.Justification) == "" { if err := validateRestartLogEntry(execReqLogEntry); err != nil {
return nil, restartRequiresJustificationError() return nil, err
} }
authenticatedUser := auth.UserFromApiCall(ctx, req, api.cfg) authenticatedUser := auth.UserFromApiCall(ctx, req, api.cfg)

View File

@ -1,9 +1,14 @@
package api package api
import ( import (
"fmt"
"sort" "sort"
"strings"
"connectrpc.com/connect"
apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1" apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
"github.com/OliveTin/OliveTin/internal/executor"
) )
func logEntryArgumentsToProto(args map[string]string) []*apiv1.StartActionArgument { func logEntryArgumentsToProto(args map[string]string) []*apiv1.StartActionArgument {
@ -36,3 +41,19 @@ func copyStringMap(source map[string]string) map[string]string {
return copied return copied
} }
func restartArgumentsIncompleteError() error {
return connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("stored arguments are incomplete for restart; use StartAction with the required arguments instead"))
}
func validateRestartLogEntry(entry *executor.InternalLogEntry) error {
if entry.Binding.Action.Justification && strings.TrimSpace(entry.Justification) == "" {
return restartRequiresJustificationError()
}
if executor.RestartArgumentsIncomplete(entry.Binding.Action, entry.Binding.Entity, entry.Arguments) {
return restartArgumentsIncompleteError()
}
return nil
}

View File

@ -158,6 +158,75 @@ func TestRestartActionReusesStoredArguments(t *testing.T) {
assert.Equal(t, "server-a", restartedArgs["host"]) assert.Equal(t, "server-a", restartedArgs["host"])
} }
func TestRestartActionRejectsIncompleteStoredArguments(t *testing.T) {
cfg := config.DefaultConfig()
cfg.Actions = []*config.Action{
{
Title: "Connect",
Exec: []string{"echo", "{{ user }}"},
MaxConcurrent: 1,
Arguments: []config.ActionArgument{
{Name: "user", Type: "ascii_identifier"},
{Name: "pass", Type: "password"},
},
},
}
ex := executor.DefaultExecutor(cfg)
ex.RebuildActionMap()
binding := ex.FindBindingWithNoEntity(cfg.Actions[0])
require.NotNil(t, binding)
ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
defer ts.Close()
startResp, err := client.StartAction(context.Background(), connect.NewRequest(&apiv1.StartActionRequest{
BindingId: binding.ID,
Arguments: []*apiv1.StartActionArgument{
{Name: "user", Value: "alice"},
{Name: "pass", Value: "secret"},
},
}))
require.NoError(t, err)
_, err = client.RestartAction(context.Background(), connect.NewRequest(&apiv1.RestartActionRequest{
ExecutionTrackingId: startResp.Msg.ExecutionTrackingId,
}))
require.Error(t, err)
assert.Contains(t, err.Error(), "stored arguments are incomplete for restart")
}
func TestRestartActionRejectsMissingRequiredStoredArguments(t *testing.T) {
cfg := config.DefaultConfig()
cfg.Actions = []*config.Action{
argumentAction("Ping host", "echo {{ host }}", []config.ActionArgument{
{Name: "host", Type: "ascii_identifier"},
}),
}
ex := executor.DefaultExecutor(cfg)
ex.RebuildActionMap()
binding := ex.FindBindingWithNoEntity(cfg.Actions[0])
require.NotNil(t, binding)
trackingID := "a1b2c3d4-e5f6-7890-abcd-ef1234567890"
ex.SetLog(trackingID, &executor.InternalLogEntry{
Binding: binding,
ExecutionFinished: true,
ExecutionTrackingID: trackingID,
Arguments: map[string]string{},
})
ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
defer ts.Close()
_, err := client.RestartAction(context.Background(), connect.NewRequest(&apiv1.RestartActionRequest{
ExecutionTrackingId: trackingID,
}))
require.Error(t, err)
assert.Contains(t, err.Error(), "stored arguments are incomplete for restart")
}
func TestLogEntryArgumentsToProto(t *testing.T) { func TestLogEntryArgumentsToProto(t *testing.T) {
assert.Nil(t, logEntryArgumentsToProto(nil)) assert.Nil(t, logEntryArgumentsToProto(nil))
assert.Nil(t, logEntryArgumentsToProto(map[string]string{})) assert.Nil(t, logEntryArgumentsToProto(map[string]string{}))

View File

@ -113,6 +113,15 @@ func loadConfigFromYAML(t *testing.T, yamlStr string, cfg *Config) bool {
} }
func TestEnvInConfig(t *testing.T) { func TestEnvInConfig(t *testing.T) {
originalInput, hadInput := os.LookupEnv("INPUT")
t.Cleanup(func() {
if hadInput {
setTestEnvInput(t, originalInput)
return
}
setTestEnvInput(t, "")
})
for _, tt := range envConfigTests { for _, tt := range envConfigTests {
cfg := DefaultConfig() cfg := DefaultConfig()
setTestEnvInput(t, tt.input) setTestEnvInput(t, tt.input)

View File

@ -4,6 +4,8 @@ import (
"strings" "strings"
config "github.com/OliveTin/OliveTin/internal/config" config "github.com/OliveTin/OliveTin/internal/config"
"github.com/OliveTin/OliveTin/internal/entities"
"github.com/OliveTin/OliveTin/internal/tpl"
) )
func argumentTypeStorableInLog(argType string) bool { func argumentTypeStorableInLog(argType string) bool {
@ -93,3 +95,47 @@ func copyStorableArgumentsToLogEntry(req *ExecutionRequest) {
entry.Arguments = args entry.Arguments = args
}) })
} }
func restartArgumentMissingFromStored(arg *config.ActionArgument, entity *entities.Entity, storedArgs map[string]string) bool {
if !argumentTypeStorableInLog(arg.Type) {
return true
}
if !restartArgumentRequired(arg, entity) {
return false
}
if storedArgs == nil {
return true
}
_, ok := storedArgs[arg.Name]
return !ok
}
func RestartArgumentsIncomplete(action *config.Action, entity *entities.Entity, storedArgs map[string]string) bool {
if action == nil {
return false
}
for i := range action.Arguments {
if restartArgumentMissingFromStored(&action.Arguments[i], entity, storedArgs) {
return true
}
}
return false
}
func restartArgumentRequired(arg *config.ActionArgument, entity *entities.Entity) bool {
if argumentSkipsValidation(arg) {
return false
}
defaultValue := arg.Default
if defaultValue != "" {
defaultValue = tpl.ParseTemplateOfActionBeforeExec(defaultValue, entity)
}
return defaultValue == ""
}

View File

@ -119,3 +119,39 @@ func TestExecRequestStoresArgumentsOnLogEntry(t *testing.T) {
require.NotNil(t, logEntry.Arguments) require.NotNil(t, logEntry.Arguments)
assert.Equal(t, "yourself", logEntry.Arguments["person"]) assert.Equal(t, "yourself", logEntry.Arguments["person"])
} }
func TestRestartArgumentsIncompleteDetectsNonStorableArguments(t *testing.T) {
action := &config.Action{
Arguments: []config.ActionArgument{
{Name: "host", Type: "ascii_identifier"},
{Name: "pass", Type: "password"},
},
}
assert.True(t, RestartArgumentsIncomplete(action, nil, map[string]string{
"host": "db-1",
}))
}
func TestRestartArgumentsIncompleteDetectsMissingRequiredStoredArguments(t *testing.T) {
action := &config.Action{
Arguments: []config.ActionArgument{
{Name: "host", Type: "ascii_identifier"},
},
}
assert.True(t, RestartArgumentsIncomplete(action, nil, map[string]string{}))
assert.False(t, RestartArgumentsIncomplete(action, nil, map[string]string{
"host": "db-1",
}))
}
func TestRestartArgumentsIncompleteAllowsOptionalArgumentsWithDefaults(t *testing.T) {
action := &config.Action{
Arguments: []config.ActionArgument{
{Name: "host", Type: "ascii_identifier", Default: "example.com"},
},
}
assert.False(t, RestartArgumentsIncomplete(action, nil, map[string]string{}))
}