Merge commit from fork

security: GHSA-f637-w7p2-m7fx (LOW) Validation endpoints allow argument enumeration

service/internal/api/api.go

@@ -738,12 +738,46 @@ func (api *oliveTinAPI) argumentNotFoundForValidation(msg *apiv1.ValidateArgumen
 	return arg == nil
 }
 
+func (api *oliveTinAPI) validateArgumentTypeBindingAccess(user *authpublic.AuthenticatedUser, msg *apiv1.ValidateArgumentTypeRequest) error {
+	if msg == nil || msg.BindingId == "" {
+		return nil
+	}
+
+	return api.errUnlessUserMayValidateArgumentTypeForBinding(user, msg.BindingId)
+}
+
+func (api *oliveTinAPI) errUnlessUserMayValidateArgumentTypeForBinding(user *authpublic.AuthenticatedUser, bindingID string) error {
+	binding := api.executor.FindBindingByID(bindingID)
+	if binding == nil || binding.Action == nil {
+		return connect.NewError(connect.CodeNotFound, fmt.Errorf("action or argument not found for binding ID %s", bindingID))
+	}
+
+	if !api.userCanViewAction(user, binding.Action) {
+		return connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied"))
+	}
+
+	return nil
+}
+
 func (api *oliveTinAPI) ValidateArgumentType(ctx ctx.Context, req *connect.Request[apiv1.ValidateArgumentTypeRequest]) (*connect.Response[apiv1.ValidateArgumentTypeResponse], error) {
+	user := auth.UserFromApiCall(ctx, req, api.cfg)
+	if err := api.checkDashboardAccess(user); err != nil {
+		return nil, err
+	}
+
+	if err := api.validateArgumentTypeBindingAccess(user, req.Msg); err != nil {
+		return nil, err
+	}
+
 	if api.argumentNotFoundForValidation(req.Msg) {
 		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action or argument not found for binding ID %s", req.Msg.BindingId))
 	}
 
-	err := api.validateArgumentTypeInternal(req.Msg)
+	return api.validateArgumentTypeConnectResponse(req.Msg)
+}
+
+func (api *oliveTinAPI) validateArgumentTypeConnectResponse(msg *apiv1.ValidateArgumentTypeRequest) (*connect.Response[apiv1.ValidateArgumentTypeResponse], error) {
+	err := api.validateArgumentTypeInternal(msg)
 	desc := ""
 	if err != nil {
 		desc = err.Error()

service/internal/api/api_test.go

@@ -408,6 +408,96 @@ func TestGetActionBindingDeniedWhenNoViewPermission(t *testing.T) {
 		"user with view:false must get permission denied from GetActionBinding")
 }
 
+// TestValidateArgumentTypeDeniesGuestsWhenLoginRequired (GHSA-f637-w7p2-m7fx) asserts that when
+// guests must log in, ValidateArgumentType does not bypass dashboard access controls.
+func TestValidateArgumentTypeDeniesGuestsWhenLoginRequired(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.AuthRequireGuestsToLogin = true
+	cfg.Actions = append(cfg.Actions, &config.Action{
+		ID:    "a1",
+		Title: "Probe",
+		Shell: "echo",
+		Arguments: []config.ActionArgument{
+			{Name: "x", Type: "ascii"},
+		},
+	})
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClient(cfg)
+	defer ts.Close()
+
+	_, err := client.ValidateArgumentType(context.Background(), connect.NewRequest(&apiv1.ValidateArgumentTypeRequest{
+		BindingId:    "a1",
+		ArgumentName: "x",
+		Value:        "v",
+		Type:         "ascii",
+	}))
+	require.Error(t, err)
+	assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err),
+		"guest must not call ValidateArgumentType when AuthRequireGuestsToLogin is true")
+}
+
+// TestValidateArgumentTypeDeniedWithoutViewPermission (GHSA-f637-w7p2-m7fx) asserts ValidateArgumentType
+// respects the same view ACL as GetActionBinding so the RPC cannot enumerate restricted actions.
+func TestValidateArgumentTypeDeniedWithoutViewPermission(t *testing.T) {
+	cfg, _, _ := buildViewPermissionTestConfig(t)
+	cfg.AuthHttpHeaderUsername = "X-Ot-User"
+	for i := range cfg.Actions {
+		if cfg.Actions[i].ID == "secret_action" {
+			cfg.Actions[i].Arguments = []config.ActionArgument{{Name: "target", Type: "ascii"}}
+			break
+		}
+	}
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClient(cfg)
+	defer ts.Close()
+
+	req := connect.NewRequest(&apiv1.ValidateArgumentTypeRequest{
+		BindingId:    "secret_action",
+		ArgumentName: "target",
+		Value:        "ok",
+		Type:         "ascii",
+	})
+	req.Header().Set("X-Ot-User", "low")
+
+	_, err := client.ValidateArgumentType(context.Background(), req)
+	require.Error(t, err)
+	assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err),
+		"user with view:false must get permission denied from ValidateArgumentType")
+}
+
+// TestValidateArgumentTypeAllowedWithViewPermission (GHSA-f637-w7p2-m7fx) asserts authenticated users
+// with view access can still use ValidateArgumentType for argument validation.
+func TestValidateArgumentTypeAllowedWithViewPermission(t *testing.T) {
+	cfg, _, _ := buildViewPermissionTestConfig(t)
+	cfg.AuthHttpHeaderUsername = "X-Ot-User"
+	for i := range cfg.Actions {
+		if cfg.Actions[i].ID == "secret_action" {
+			cfg.Actions[i].Arguments = []config.ActionArgument{{Name: "target", Type: "ascii"}}
+			break
+		}
+	}
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClient(cfg)
+	defer ts.Close()
+
+	req := connect.NewRequest(&apiv1.ValidateArgumentTypeRequest{
+		BindingId:    "secret_action",
+		ArgumentName: "target",
+		Value:        "ok",
+		Type:         "ascii",
+	})
+	req.Header().Set("X-Ot-User", "admin")
+
+	resp, err := client.ValidateArgumentType(context.Background(), req)
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+	require.NotNil(t, resp.Msg)
+	assert.True(t, resp.Msg.Valid, "admin with view:true should get successful validation for a valid ascii value")
+}
+
 // TestViewPermissionAllowedSeesAction (GHSA: view permission) asserts that a user with view: true
 // still sees the action in the dashboard and can fetch it via GetActionBinding.
 func TestViewPermissionAllowedSeesAction(t *testing.T) {