fix(ansible): validate playbook inventory, connection, and file paths (#25605)

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

pkg/ansibleserver/models/ansibleplaybooks.go

@@ -23,6 +23,7 @@ import (
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
 	"yunion.io/x/pkg/errors"
+	"yunion.io/x/pkg/tristate"
 	"yunion.io/x/sqlchemy"
 
 	"yunion.io/x/onecloud/pkg/ansibleserver/options"
@@ -43,6 +44,7 @@ import (
 
 type SAnsiblePlaybook struct {
 	db.SVirtualResourceBase
+	db.SEnabledResourceBase `enabled->default:"" enabled->nullable:"true"`
 
 	Playbook  *ansible.Playbook `length:"text" nullable:"false" create:"required" get:"user" update:"user"`
 	Output    string            `length:"medium" get:"user"`
@@ -52,6 +54,7 @@ type SAnsiblePlaybook struct {
 
 type SAnsiblePlaybookManager struct {
 	db.SVirtualResourceBaseManager
+	db.SEnabledResourceBaseManager
 
 	sessions    ansible.SessionManager
 	sessionsMux *sync.Mutex
@@ -73,6 +76,41 @@ func init() {
 	AnsiblePlaybookManager.SetVirtualObject(AnsiblePlaybookManager)
 }
 
+// requireSystemAdmin is required to enable a playbook.
+func requireSystemAdmin(userCred mcclient.TokenCredential) error {
+	if userCred == nil || !userCred.HasSystemAdminPrivilege() {
+		return httperrors.NewForbiddenError("enabling ansible playbook requires system admin privilege")
+	}
+	return nil
+}
+
+// applyPlaybookEnabledByCred sets enabled on create/update: system admin
+// defaults to enabled, others are always disabled.
+func applyPlaybookEnabledByCred(userCred mcclient.TokenCredential, data *jsonutils.JSONDict) {
+	if userCred != nil && userCred.HasSystemAdminPrivilege() {
+		if !data.Contains("enabled") {
+			data.Set("enabled", jsonutils.JSONTrue)
+		}
+		return
+	}
+	data.Set("enabled", jsonutils.JSONFalse)
+}
+
+func ensurePlaybookEnabled(enabled bool) error {
+	if !enabled {
+		return httperrors.NewForbiddenError("playbook is not enabled")
+	}
+	return nil
+}
+
+func (man *SAnsiblePlaybookManager) ListItemFilter(ctx context.Context, q *sqlchemy.SQuery, userCred mcclient.TokenCredential, query api.AnsiblePlaybookListInput) (*sqlchemy.SQuery, error) {
+	q, err := man.SVirtualResourceBaseManager.ListItemFilter(ctx, q, userCred, query.VirtualResourceListInput)
+	if err != nil {
+		return nil, err
+	}
+	return man.SEnabledResourceBaseManager.ListItemFilter(ctx, q, userCred, query.EnabledResourceBaseListInput)
+}
+
 func (man *SAnsiblePlaybookManager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
 	pbV := NewAnsiblePlaybookValidator("playbook", userCred)
 	if err := pbV.Validate(ctx, data); err != nil {
@@ -90,11 +128,15 @@ func (man *SAnsiblePlaybookManager) ValidateCreateData(ctx context.Context, user
 		return nil, err
 	}
 	data.Update(jsonutils.Marshal(input))
+	applyPlaybookEnabledByCred(userCred, data)
 	return data, nil
 }
 
 func (apb *SAnsiblePlaybook) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) {
 	apb.SVirtualResourceBase.PostCreate(ctx, userCred, ownerId, query, data)
+	if !apb.GetEnabled() {
+		return
+	}
 	err := apb.runPlaybook(ctx, userCred)
 	if err != nil {
 		log.Errorf("postCreate: runPlaybook: %v", err)
@@ -118,6 +160,28 @@ func (man *SAnsiblePlaybookManager) InitializeData() error {
 			log.Errorf("set playbook %s(%s) to unknown state: %v", pb.Name, pb.Id, err)
 		}
 	}
+	if err := man.eanbleExistingPlaybooks(); err != nil {
+		return errors.Wrap(err, "enable existing playbooks")
+	}
+	return nil
+}
+
+func (man *SAnsiblePlaybookManager) eanbleExistingPlaybooks() error {
+	pbs := []SAnsiblePlaybookV2{}
+	q := AnsiblePlaybookV2Manager.Query().IsNull("enabled")
+	if err := db.FetchModelObjects(AnsiblePlaybookV2Manager, q, &pbs); err != nil {
+		return errors.Wrap(err, "fetch running playbooks")
+	}
+	for i := 0; i < len(pbs); i++ {
+		pb := &pbs[i]
+		_, err := db.Update(pb, func() error {
+			pb.Enabled = tristate.True
+			return nil
+		})
+		if err != nil {
+			log.Errorf("enable playbook %s(%s): %v", pb.Name, pb.Id, err)
+		}
+	}
 	return nil
 }
 
@@ -142,17 +206,44 @@ func (apb *SAnsiblePlaybook) ValidateUpdateData(ctx context.Context, userCred mc
 	}
 	apb.Playbook = pbV.Playbook // Update as a whole
 	data.Set("status", jsonutils.NewString(api.AnsiblePlaybookStatusInit))
+	applyPlaybookEnabledByCred(userCred, data)
+	if enabled, err := data.Bool("enabled"); err == nil {
+		apb.SetEnabled(enabled)
+	}
 	return data, nil
 }
 
 func (apb *SAnsiblePlaybook) PostUpdate(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) {
 	apb.SVirtualResourceBase.PostUpdate(ctx, userCred, query, data)
+	if !apb.GetEnabled() {
+		return
+	}
 	err := apb.runPlaybook(ctx, userCred)
 	if err != nil {
 		log.Errorf("postUpdate: runPlaybook: %v", err)
 	}
 }
 
+func (apb *SAnsiblePlaybook) PerformEnable(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, input apis.PerformEnableInput) (jsonutils.JSONObject, error) {
+	if err := requireSystemAdmin(userCred); err != nil {
+		return nil, err
+	}
+	if err := ansible.ValidatePlaybook(apb.Playbook); err != nil {
+		return nil, httperrors.NewInputParameterError("%s", err.Error())
+	}
+	if err := db.EnabledPerformEnable(apb, ctx, userCred, true); err != nil {
+		return nil, err
+	}
+	return nil, nil
+}
+
+func (apb *SAnsiblePlaybook) PerformDisable(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, input apis.PerformDisableInput) (jsonutils.JSONObject, error) {
+	if err := db.EnabledPerformEnable(apb, ctx, userCred, false); err != nil {
+		return nil, err
+	}
+	return nil, nil
+}
+
 func (apb *SAnsiblePlaybook) PerformRun(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) {
 	err := apb.runPlaybook(ctx, userCred)
 	if err != nil {
@@ -176,9 +267,15 @@ func (apb *SAnsiblePlaybook) runPlaybook(ctx context.Context, userCred mcclient.
 	if man.sessions.Has(apb.Id) {
 		return fmt.Errorf("playbook is already running")
 	}
+	if err := ensurePlaybookEnabled(apb.GetEnabled()); err != nil {
+		return err
+	}
 
 	// init private key
 	pb := apb.Playbook.Copy()
+	if err := ansible.ValidatePlaybook(pb); err != nil {
+		return err
+	}
 	if len(pb.PrivateKey) == 0 {
 		if k, err := compute.Sshkeypairs.FetchPrivateKey(ctx, userCred); err != nil {
 			return err

pkg/ansibleserver/models/ansibleplaybooks_test.go

@@ -0,0 +1,80 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or authorized to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package models
+
+import (
+	"testing"
+
+	"yunion.io/x/jsonutils"
+	"yunion.io/x/onecloud/pkg/mcclient"
+)
+
+func TestRequireSystemAdmin(t *testing.T) {
+	systemAdmin := &mcclient.SSimpleToken{User: "admin", Project: "system", Roles: "admin"}
+	if err := requireSystemAdmin(systemAdmin); err != nil {
+		t.Fatalf("system admin should pass: %v", err)
+	}
+
+	tenant := &mcclient.SSimpleToken{User: "user1", Project: "proj1", Roles: "user"}
+	if err := requireSystemAdmin(tenant); err == nil {
+		t.Fatal("tenant should be forbidden")
+	}
+
+	projectAdmin := &mcclient.SSimpleToken{User: "owner", Project: "proj1", Roles: "admin"}
+	if err := requireSystemAdmin(projectAdmin); err == nil {
+		t.Fatal("project admin (non-system project) should be forbidden")
+	}
+
+	if err := requireSystemAdmin(nil); err == nil {
+		t.Fatal("nil credential should be forbidden")
+	}
+}
+
+func TestApplyPlaybookEnabledByCred(t *testing.T) {
+	admin := &mcclient.SSimpleToken{User: "admin", Project: "system", Roles: "admin"}
+	tenant := &mcclient.SSimpleToken{User: "user1", Project: "proj1", Roles: "user"}
+
+	data := jsonutils.NewDict()
+	applyPlaybookEnabledByCred(admin, data)
+	enabled, _ := data.Bool("enabled")
+	if !enabled {
+		t.Fatal("admin create should default enabled")
+	}
+
+	data = jsonutils.NewDict()
+	data.Set("enabled", jsonutils.JSONFalse)
+	applyPlaybookEnabledByCred(admin, data)
+	enabled, _ = data.Bool("enabled")
+	if enabled {
+		t.Fatal("admin should be able to create disabled")
+	}
+
+	data = jsonutils.NewDict()
+	data.Set("enabled", jsonutils.JSONTrue)
+	applyPlaybookEnabledByCred(tenant, data)
+	enabled, _ = data.Bool("enabled")
+	if enabled {
+		t.Fatal("non-admin create must be disabled")
+	}
+}
+
+func TestEnsurePlaybookEnabled(t *testing.T) {
+	if err := ensurePlaybookEnabled(true); err != nil {
+		t.Fatalf("enabled: %v", err)
+	}
+	if err := ensurePlaybookEnabled(false); err == nil {
+		t.Fatal("disabled playbook should not run")
+	}
+}

pkg/ansibleserver/models/ansibleplaybooks_v2.go

@@ -24,6 +24,7 @@ import (
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
 	"yunion.io/x/pkg/errors"
+	"yunion.io/x/pkg/tristate"
 	"yunion.io/x/sqlchemy"
 
 	"yunion.io/x/onecloud/pkg/ansibleserver/options"
@@ -40,6 +41,7 @@ import (
 // This is at the moment for internal use only.  Update is not allowed
 type SAnsiblePlaybookV2 struct {
 	db.SVirtualResourceBase
+	db.SEnabledResourceBase `enabled->default:"" enabled->nullable:"true"`
 
 	Playbook     string    `length:"text" nullable:"false" create:"required" get:"user"`
 	Inventory    string    `length:"text" nullable:"false" create:"required" get:"user"`
@@ -54,6 +56,7 @@ type SAnsiblePlaybookV2 struct {
 
 type SAnsiblePlaybookV2Manager struct {
 	db.SVirtualResourceBaseManager
+	db.SEnabledResourceBaseManager
 
 	sessions    ansible.SessionManager
 	sessionsMux *sync.Mutex
@@ -75,6 +78,14 @@ func init() {
 	AnsiblePlaybookV2Manager.SetVirtualObject(AnsiblePlaybookV2Manager)
 }
 
+func (man *SAnsiblePlaybookV2Manager) ListItemFilter(ctx context.Context, q *sqlchemy.SQuery, userCred mcclient.TokenCredential, query api.AnsiblePlaybookListInput) (*sqlchemy.SQuery, error) {
+	q, err := man.SVirtualResourceBaseManager.ListItemFilter(ctx, q, userCred, query.VirtualResourceListInput)
+	if err != nil {
+		return nil, err
+	}
+	return man.SEnabledResourceBaseManager.ListItemFilter(ctx, q, userCred, query.EnabledResourceBaseListInput)
+}
+
 func (man *SAnsiblePlaybookV2Manager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
 	data.Set("status", jsonutils.NewString(api.AnsiblePlaybookStatusInit))
 	input := apis.VirtualResourceCreateInput{}
@@ -87,11 +98,42 @@ func (man *SAnsiblePlaybookV2Manager) ValidateCreateData(ctx context.Context, us
 		return nil, err
 	}
 	data.Update(jsonutils.Marshal(input))
+	if err := validateAnsiblePlaybookV2Input(userCred, data); err != nil {
+		return nil, err
+	}
+	applyPlaybookEnabledByCred(userCred, data)
 	return data, nil
 }
 
+func validateAnsiblePlaybookV2Input(userCred mcclient.TokenCredential, data *jsonutils.JSONDict) error {
+	playbook, _ := data.GetString("playbook")
+	inventory, _ := data.GetString("inventory")
+	files, _ := data.GetString("files")
+	requirements, _ := data.GetString("requirements")
+	return validateAnsiblePlaybookV2Fields(userCred, playbook, inventory, files, requirements)
+}
+
+func validateAnsiblePlaybookV2Fields(userCred mcclient.TokenCredential, playbook, inventory, files, requirements string) error {
+	if err := ansible.ValidatePlaybookYAML(playbook); err != nil {
+		return httperrors.NewInputParameterError("%s", err.Error())
+	}
+	if err := ansible.ValidateInventoryYAML(inventory); err != nil {
+		return httperrors.NewInputParameterError("%s", err.Error())
+	}
+	if err := ansible.ValidateFilesJSON(files); err != nil {
+		return httperrors.NewInputParameterError("%s", err.Error())
+	}
+	if strings.TrimSpace(requirements) != "" && (userCred == nil || !userCred.HasSystemAdminPrivilege()) {
+		return httperrors.NewForbiddenError("requirements is not allowed")
+	}
+	return nil
+}
+
 func (apb *SAnsiblePlaybookV2) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) {
 	apb.SVirtualResourceBase.PostCreate(ctx, userCred, ownerId, query, data)
+	if !apb.GetEnabled() {
+		return
+	}
 	err := apb.runPlaybook(ctx, userCred)
 	if err != nil {
 		log.Errorf("postCreate: runPlaybook: %v", err)
@@ -115,6 +157,28 @@ func (man *SAnsiblePlaybookV2Manager) InitializeData() error {
 			log.Errorf("set playbook %s(%s) to unknown state: %v", pb.Name, pb.Id, err)
 		}
 	}
+	if err := man.eanbleExistingPlaybooks(); err != nil {
+		return errors.Wrap(err, "enable existing playbooks")
+	}
+	return nil
+}
+
+func (man *SAnsiblePlaybookV2Manager) eanbleExistingPlaybooks() error {
+	pbs := []SAnsiblePlaybookV2{}
+	q := AnsiblePlaybookV2Manager.Query().IsNull("enabled")
+	if err := db.FetchModelObjects(AnsiblePlaybookV2Manager, q, &pbs); err != nil {
+		return errors.Wrap(err, "fetch running playbooks")
+	}
+	for i := 0; i < len(pbs); i++ {
+		pb := &pbs[i]
+		_, err := db.Update(pb, func() error {
+			pb.Enabled = tristate.True
+			return nil
+		})
+		if err != nil {
+			log.Errorf("enable playbook %s(%s): %v", pb.Name, pb.Id, err)
+		}
+	}
 	return nil
 }
 
@@ -125,6 +189,26 @@ func (apb *SAnsiblePlaybookV2) ValidateDeleteCondition(ctx context.Context, info
 	return nil
 }
 
+func (apb *SAnsiblePlaybookV2) PerformEnable(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, input apis.PerformEnableInput) (jsonutils.JSONObject, error) {
+	if err := requireSystemAdmin(userCred); err != nil {
+		return nil, err
+	}
+	if err := validateAnsiblePlaybookV2Fields(userCred, apb.Playbook, apb.Inventory, apb.Files, apb.Requirements); err != nil {
+		return nil, err
+	}
+	if err := db.EnabledPerformEnable(apb, ctx, userCred, true); err != nil {
+		return nil, err
+	}
+	return nil, nil
+}
+
+func (apb *SAnsiblePlaybookV2) PerformDisable(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, input apis.PerformDisableInput) (jsonutils.JSONObject, error) {
+	if err := db.EnabledPerformEnable(apb, ctx, userCred, false); err != nil {
+		return nil, err
+	}
+	return nil, nil
+}
+
 func (apb *SAnsiblePlaybookV2) PerformRun(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) {
 	err := apb.runPlaybook(ctx, userCred)
 	if err != nil {
@@ -148,6 +232,21 @@ func (apb *SAnsiblePlaybookV2) runPlaybook(ctx context.Context, userCred mcclien
 	if man.sessions.Has(apb.Id) {
 		return fmt.Errorf("playbook is already running")
 	}
+	if err := ensurePlaybookEnabled(apb.GetEnabled()); err != nil {
+		return err
+	}
+	if err := ansible.ValidatePlaybookYAML(apb.Playbook); err != nil {
+		return err
+	}
+	if err := ansible.ValidateInventoryYAML(apb.Inventory); err != nil {
+		return err
+	}
+	if err := ansible.ValidateFilesJSON(apb.Files); err != nil {
+		return err
+	}
+	if strings.TrimSpace(apb.Requirements) != "" && !userCred.HasSystemAdminPrivilege() {
+		return httperrors.NewForbiddenError("requirements is not allowed")
+	}
 
 	// hack: force Sleep 50s to wait some host ssh service started
 	// time.Sleep(50 * time.Second)

pkg/ansibleserver/models/ansibleplaybooks_validator.go

@@ -87,6 +87,9 @@ func (v *ValidatorAnsiblePlaybook) Validate(ctx context.Context, data *jsonutils
 			}
 		}
 	}
+	if err := ansible.ValidatePlaybook(pb); err != nil {
+		return httperrors.NewInputParameterError("%s", err.Error())
+	}
 	// add LF for privateKey
 	if len(pb.PrivateKey) > 0 && pb.PrivateKey[len(pb.PrivateKey)-1] != 10 {
 		pb.PrivateKey = append(pb.PrivateKey, 10)

pkg/apis/ansibleserver/ansible.go

@@ -28,6 +28,11 @@ type AnsiblePlaybookCreateInput struct {
 	Playbook ansible.Playbook `json:"playbook"`
 }
 
+type AnsiblePlaybookListInput struct {
+	apis.VirtualResourceListInput
+	apis.EnabledResourceBaseListInput
+}
+
 type AnsiblePlaybookUpdateInput AnsiblePlaybookCreateInput
 
 type AnsibleHost struct {

pkg/cloudproxy/models/proxy_endpoints_remote_check_make.go

@@ -155,7 +155,9 @@ func (proxyendpoint *SProxyEndpoint) remoteConfigure(ctx context.Context, userCr
 		},
 	}
 
-	cliSess := auth.GetSession(ctx, userCred, "")
+	// the playbook content here is server-generated; use the admin session
+	// since ansible playbook creation requires system admin privilege
+	cliSess := auth.GetAdminSession(ctx, "")
 	pbId := ""
 	pbName := "pe-remote-configure-" + proxyendpoint.Name
 	_, err := ansible_modules.AnsiblePlaybooks.UpdateOrCreatePbModel(

pkg/compute/models/guest_sshable.go

@@ -405,9 +405,15 @@ func (guest *SGuest) PerformMakeSshable(
 	if input.User == "" {
 		return output, httperrors.NewBadRequestError("missing username")
 	}
+	if !ansible.IsValidAnsibleUser(input.User) {
+		return output, httperrors.NewInputParameterError("invalid username")
+	}
 	if input.PrivateKey == "" && input.Password == "" {
 		return output, httperrors.NewBadRequestError("private_key and password cannot both be empty")
 	}
+	if strings.ContainsAny(input.Password, "\x00\r\n") {
+		return output, httperrors.NewInputParameterError("invalid password")
+	}
 
 	_, projectPublicKey, err := sshkeys.GetSshProjectKeypair(ctx, guest.ProjectId)
 	if err != nil {
@@ -496,7 +502,9 @@ func (guest *SGuest) PerformMakeSshable(
 		host.SetVar("ansible_password", input.Password)
 	}
 
-	cliSess := auth.GetSession(ctx, userCred, "")
+	// the playbook content here is server-generated; use the admin session
+	// since ansible playbook creation requires system admin privilege
+	cliSess := auth.GetAdminSession(ctx, "")
 	pbId := ""
 	pbName := "make-sshable-" + guest.Id
 	pbModel, err := ansible_modules.AnsiblePlaybooks.UpdateOrCreatePbModel(

pkg/util/ansible/module.go

@@ -16,6 +16,7 @@ package ansible
 
 import (
 	"bytes"
+	"strings"
 )
 
 // Module represents name and args of ansible module to execute
@@ -74,9 +75,24 @@ func (i *Inventory) Data() []byte {
 			b.WriteRune(' ')
 			b.WriteString(k)
 			b.WriteRune('=')
-			b.WriteString(v)
+			b.WriteString(quoteInventoryValue(v))
 		}
 		b.WriteRune('\n')
 	}
 	return b.Bytes()
 }
+
+func quoteInventoryValue(v string) string {
+	var b strings.Builder
+	b.Grow(len(v) + 2)
+	b.WriteByte('"')
+	for i := 0; i < len(v); i++ {
+		c := v[i]
+		if c == '\\' || c == '"' {
+			b.WriteByte('\\')
+		}
+		b.WriteByte(c)
+	}
+	b.WriteByte('"')
+	return b.String()
+}

pkg/util/ansible/playbook.go

@@ -26,6 +26,8 @@ import (
 
 	"yunion.io/x/pkg/errors"
 	"yunion.io/x/pkg/gotypes"
+
+	"yunion.io/x/onecloud/pkg/util/fileutils2"
 )
 
 type pbState int
@@ -160,7 +162,11 @@ func (pb *Playbook) Run(ctx context.Context) (err error) {
 
 	// write out files
 	for name, content := range pb.Files {
-		path := filepath.Join(tmpdir, name)
+		path, err2 := fileutils2.JoinInside(tmpdir, name)
+		if err2 != nil {
+			err = errors.Wrapf(err2, "playbook file %s", name)
+			return
+		}
 		dir := filepath.Dir(path)
 		err = os.MkdirAll(dir, os.FileMode(0700))
 		if err != nil {

pkg/util/ansible/validate.go

@@ -0,0 +1,175 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ansible
+
+import (
+	"net"
+	"path"
+	"regexp"
+	"strings"
+
+	"yunion.io/x/pkg/errors"
+
+	"yunion.io/x/onecloud/pkg/util/fileutils2"
+)
+
+var (
+	ansibleUserRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`)
+	ansibleVarRe  = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
+)
+
+var allowedConnections = map[string]struct{}{
+	"":         {},
+	"ssh":      {},
+	"smart":    {},
+	"paramiko": {},
+	"winrm":    {},
+	"psrp":     {},
+}
+
+var deniedAnsibleVars = map[string]struct{}{
+	"ansible_shell_executable":   {},
+	"ansible_python_interpreter": {},
+	"ansible_ssh_executable":     {},
+	"ansible_remote_tmp":         {},
+	"ansible_executable":         {},
+	"ansible_shell_type":         {},
+	"ansible_become_exe":         {},
+	"ansible_become_flags":       {},
+	"ansible_ssh_args":           {},
+	"ansible_async_dir":          {},
+}
+
+func IsValidAnsibleUser(user string) bool {
+	return ansibleUserRe.MatchString(user)
+}
+
+func ValidateInventoryHostName(name string) error {
+	name = strings.TrimSpace(name)
+	if name == "" {
+		return errors.Errorf("empty host name")
+	}
+	if strings.ContainsAny(name, " \t\r\n=") {
+		return errors.Errorf("invalid host name")
+	}
+	if isLocalTarget(name) {
+		return errors.Errorf("host %q is not allowed", name)
+	}
+	return nil
+}
+
+func ValidateInventoryVar(key, value string) error {
+	if !ansibleVarRe.MatchString(key) {
+		return errors.Errorf("invalid inventory variable %q", key)
+	}
+	if strings.ContainsAny(value, "\x00\r\n") {
+		return errors.Errorf("invalid inventory variable %s", key)
+	}
+	lk := strings.ToLower(key)
+	if isDeniedAnsibleVar(lk) {
+		return errors.Errorf("inventory variable %s is not allowed", key)
+	}
+	switch lk {
+	case "ansible_connection":
+		if _, ok := allowedConnections[strings.ToLower(strings.TrimSpace(value))]; !ok {
+			return errors.Errorf("ansible_connection %q is not allowed", value)
+		}
+	case "ansible_host":
+		if isLocalTarget(value) {
+			return errors.Errorf("ansible_host %q is not allowed", value)
+		}
+	case "ansible_user":
+		if value != "" && !IsValidAnsibleUser(value) {
+			return errors.Errorf("invalid ansible_user")
+		}
+	case "ansible_ssh_private_key_file":
+		if _, err := fileutils2.CleanRelSubpath(value); err != nil {
+			return errors.Wrap(err, "ansible_ssh_private_key_file")
+		}
+	}
+	return nil
+}
+
+func ValidateInventoryHost(h Host) error {
+	if err := ValidateInventoryHostName(h.Name); err != nil {
+		return err
+	}
+	for k, v := range h.Vars {
+		if err := ValidateInventoryVar(k, v); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func ValidatePlaybookFileName(name string) error {
+	rel, err := fileutils2.CleanRelSubpath(name)
+	if err != nil {
+		return errors.Wrapf(err, "playbook file %q", name)
+	}
+	base := path.Base(rel)
+	if base == "ansible.cfg" || base == ".ansible.cfg" {
+		return errors.Errorf("playbook file %q is not allowed", name)
+	}
+	return nil
+}
+
+func ValidatePlaybook(pb *Playbook) error {
+	if pb == nil {
+		return errors.Errorf("empty playbook")
+	}
+	for i := range pb.Inventory.Hosts {
+		if err := ValidateInventoryHost(pb.Inventory.Hosts[i]); err != nil {
+			return err
+		}
+	}
+	for name := range pb.Files {
+		if err := ValidatePlaybookFileName(name); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func isDeniedAnsibleVar(key string) bool {
+	if _, ok := deniedAnsibleVars[key]; ok {
+		return true
+	}
+	if !strings.HasPrefix(key, "ansible_") {
+		return false
+	}
+	for _, suffix := range []string{"_executable", "_interpreter", "_extra_args", "_common_args"} {
+		if strings.HasSuffix(key, suffix) {
+			return true
+		}
+	}
+	return false
+}
+
+func isLocalTarget(s string) bool {
+	s = strings.ToLower(strings.TrimSpace(s))
+	if s == "" {
+		return false
+	}
+	switch s {
+	case "localhost", "localhost.localdomain", "ip6-localhost", "ip6-loopback":
+		return true
+	}
+	ip := net.ParseIP(s)
+	if ip == nil {
+		return false
+	}
+	return ip.IsLoopback() || ip.IsUnspecified()
+}

pkg/util/ansible/validate_test.go

@@ -0,0 +1,205 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ansible
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestValidateInventoryHost(t *testing.T) {
+	ok := Host{Name: "10.1.2.3", Vars: map[string]string{
+		"ansible_user":     "cloudroot",
+		"ansible_password": "p@ss word",
+		"ansible_become":   "yes",
+		"ansible_host":     "10.1.2.3",
+		"ansible_port":     "22",
+		"repo_base_url":    "https://example.com",
+	}}
+	if err := ValidateInventoryHost(ok); err != nil {
+		t.Fatalf("valid host: %v", err)
+	}
+	if err := ValidateInventoryHost(Host{
+		Name: "10.1.2.3",
+		Vars: map[string]string{
+			"ansible_connection":                   "winrm",
+			"ansible_user":                         "Administrator",
+			"ansible_password":                     "secret",
+			"ansible_winrm_transport":              "ntlm",
+			"ansible_winrm_server_cert_validation": "ignore",
+		},
+	}); err != nil {
+		t.Fatalf("winrm host: %v", err)
+	}
+
+	if err := ValidateInventoryHost(Host{Name: "127.0.0.1"}); err == nil {
+		t.Fatal("loopback host should fail")
+	}
+	if err := ValidateInventoryHost(Host{Name: "localhost"}); err == nil {
+		t.Fatal("localhost should fail")
+	}
+	if err := ValidateInventoryHost(Host{
+		Name: "10.1.2.3",
+		Vars: map[string]string{"ansible_connection": "local"},
+	}); err == nil {
+		t.Fatal("local connection should fail")
+	}
+	if err := ValidateInventoryHost(Host{
+		Name: "10.1.2.3",
+		Vars: map[string]string{"ansible_host": "127.0.0.1"},
+	}); err == nil {
+		t.Fatal("loopback ansible_host should fail")
+	}
+	if err := ValidateInventoryHost(Host{
+		Name: "10.1.2.3",
+		Vars: map[string]string{"ansible_user": "cloudroot ansible_connection=local"},
+	}); err == nil {
+		t.Fatal("injected ansible_user should fail")
+	}
+	if err := ValidateInventoryHost(Host{
+		Name: "10.1.2.3 cloudroot ansible_connection=local",
+	}); err == nil {
+		t.Fatal("injected host name should fail")
+	}
+	if err := ValidateInventoryHost(Host{
+		Name: "10.1.2.3",
+		Vars: map[string]string{"ansible_ssh_common_args": "-o ProxyCommand=id"},
+	}); err == nil {
+		t.Fatal("ssh extra args should fail")
+	}
+}
+
+func TestValidatePlaybookFiles(t *testing.T) {
+	pb := &Playbook{
+		Inventory: Inventory{Hosts: []Host{{Name: "10.1.2.3"}}},
+		Files: map[string][]byte{
+			"../etc/passwd": []byte("x"),
+		},
+	}
+	if err := ValidatePlaybook(pb); err == nil {
+		t.Fatal("path escape should fail")
+	}
+	pb.Files = map[string][]byte{"ansible.cfg": []byte("x")}
+	if err := ValidatePlaybook(pb); err == nil {
+		t.Fatal("ansible.cfg should fail")
+	}
+	pb.Files = map[string][]byte{"a/b.txt": []byte("x")}
+	if err := ValidatePlaybook(pb); err != nil {
+		t.Fatalf("relative file: %v", err)
+	}
+	pb.Files = map[string][]byte{".id_rsa": []byte("x")}
+	if err := ValidatePlaybook(pb); err != nil {
+		t.Fatalf("dotfile: %v", err)
+	}
+}
+
+func TestInventoryDataQuotesValues(t *testing.T) {
+	inv := Inventory{Hosts: []Host{{
+		Name: "10.1.2.3",
+		Vars: map[string]string{
+			"ansible_user":     "cloudroot",
+			"ansible_password": "p@ss word ansible_connection=local",
+		},
+	}}}
+	got := string(inv.Data())
+	quoted := quoteInventoryValue("p@ss word ansible_connection=local")
+	if !strings.Contains(got, quoted) {
+		t.Fatalf("expected quoted password %s, got %q", quoted, got)
+	}
+	if strings.Contains(got, " ansible_connection=local") && !strings.Contains(got, quoted) {
+		t.Fatalf("unquoted injection: %q", got)
+	}
+}
+
+func TestValidatePlaybookYAML(t *testing.T) {
+	if err := ValidatePlaybookYAML(`- hosts: all
+  become: true
+  tasks:
+    - service:
+        name: network
+        state: restarted
+`); err != nil {
+		t.Fatalf("valid playbook: %v", err)
+	}
+	if err := ValidatePlaybookYAML(`- hosts: localhost
+  connection: local
+  tasks:
+    - shell: id
+`); err == nil {
+		t.Fatal("local playbook should fail")
+	}
+	if err := ValidatePlaybookYAML(`- hosts: all
+  tasks:
+    - command: id
+      delegate_to: localhost
+`); err == nil {
+		t.Fatal("delegate_to localhost should fail")
+	}
+	if err := ValidatePlaybookYAML(`- hosts: all,localhost
+  tasks:
+    - ping:
+`); err == nil {
+		t.Fatal("hosts list with localhost should fail")
+	}
+	if err := ValidatePlaybookYAML(`- hosts: all
+  tasks:
+    - debug:
+        msg: "{{ lookup('pipe', 'id') }}"
+`); err == nil {
+		t.Fatal("lookup should fail")
+	}
+	if err := ValidatePlaybookYAML(`- hosts: all
+  tasks:
+    - local_action: shell id
+`); err == nil {
+		t.Fatal("local_action should fail")
+	}
+}
+
+func TestValidateInventoryYAML(t *testing.T) {
+	if err := ValidateInventoryYAML(`all:
+  hosts:
+    gw1:
+      ansible_user: root
+      ansible_host: 10.1.2.3
+      ansible_ssh_private_key_file: .id_rsa
+      ansible_become: yes
+`); err != nil {
+		t.Fatalf("valid inventory: %v", err)
+	}
+	if err := ValidateInventoryYAML(`all:
+  hosts:
+    x:
+      ansible_connection: local
+`); err == nil {
+		t.Fatal("local inventory should fail")
+	}
+	if err := ValidateInventoryYAML(`all:
+  hosts:
+    localhost:
+      ansible_user: root
+`); err == nil {
+		t.Fatal("localhost inventory host should fail")
+	}
+}
+
+func TestValidateFilesJSON(t *testing.T) {
+	if err := ValidateFilesJSON(`{".id_rsa":"k","wgX.conf.j2":"t"}`); err != nil {
+		t.Fatalf("valid files: %v", err)
+	}
+	if err := ValidateFilesJSON(`{"../etc/passwd":"x"}`); err == nil {
+		t.Fatal("path escape should fail")
+	}
+}

pkg/util/ansible/validate_yaml.go

@@ -0,0 +1,217 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ansible
+
+import (
+	"fmt"
+	"net"
+	"path/filepath"
+	"regexp"
+	"strings"
+
+	"github.com/go-yaml/yaml"
+
+	"yunion.io/x/jsonutils"
+	"yunion.io/x/pkg/errors"
+
+	"yunion.io/x/onecloud/pkg/util/fileutils2"
+)
+
+var controllerCallRe = regexp.MustCompile(`(?i)(^|[^A-Za-z0-9_])(lookup|query)\s*\(`)
+
+func ValidatePlaybookYAML(doc string) error {
+	doc = strings.TrimSpace(doc)
+	if doc == "" {
+		return errors.Errorf("empty playbook")
+	}
+	if err := rejectControllerCalls(doc); err != nil {
+		return err
+	}
+	var parsed interface{}
+	if err := yaml.Unmarshal([]byte(doc), &parsed); err != nil {
+		return errors.Wrap(err, "parse playbook")
+	}
+	return walkYAML(parsed, "")
+}
+
+func ValidateInventoryYAML(doc string) error {
+	doc = strings.TrimSpace(doc)
+	if doc == "" {
+		return errors.Errorf("empty inventory")
+	}
+	if err := rejectControllerCalls(doc); err != nil {
+		return err
+	}
+	var parsed interface{}
+	if err := yaml.Unmarshal([]byte(doc), &parsed); err != nil {
+		return errors.Wrap(err, "parse inventory")
+	}
+	return walkYAML(parsed, "")
+}
+
+func ValidateFilesJSON(files string) error {
+	files = strings.TrimSpace(files)
+	if files == "" {
+		return nil
+	}
+	obj, err := jsonutils.ParseString(files)
+	if err != nil {
+		return errors.Wrap(err, "parse files")
+	}
+	m, err := obj.GetMap()
+	if err != nil {
+		return errors.Wrap(err, "files must be a json object")
+	}
+	for name := range m {
+		if err := ValidatePlaybookFileName(name); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func rejectControllerCalls(doc string) error {
+	if controllerCallRe.MatchString(doc) {
+		return errors.Errorf("lookup/query is not allowed")
+	}
+	return nil
+}
+
+func walkYAML(v interface{}, parent string) error {
+	switch t := v.(type) {
+	case map[interface{}]interface{}:
+		for k, val := range t {
+			key := fmt.Sprint(k)
+			if err := checkYAMLPair(parent, key, val); err != nil {
+				return err
+			}
+			if err := walkYAML(val, key); err != nil {
+				return err
+			}
+		}
+	case map[string]interface{}:
+		for key, val := range t {
+			if err := checkYAMLPair(parent, key, val); err != nil {
+				return err
+			}
+			if err := walkYAML(val, key); err != nil {
+				return err
+			}
+		}
+	case []interface{}:
+		for _, item := range t {
+			if s, ok := yamlString(item); ok && (parent == "hosts" || parent == "delegate_to") {
+				if hostsContainLocal(s) {
+					return errors.Errorf("%s %q is not allowed", parent, s)
+				}
+			}
+			if err := walkYAML(item, parent); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func checkYAMLPair(parent, key string, val interface{}) error {
+	lk := strings.ToLower(strings.TrimSpace(key))
+	s, isStr := yamlString(val)
+	switch lk {
+	case "connection", "ansible_connection":
+		if !isStr {
+			return errors.Errorf("%s must be a string", key)
+		}
+		if _, ok := allowedConnections[strings.ToLower(strings.TrimSpace(s))]; !ok {
+			return errors.Errorf("%s %q is not allowed", key, s)
+		}
+	case "delegate_to", "ansible_host":
+		if isStr && hostsContainLocal(s) {
+			return errors.Errorf("%s %q is not allowed", key, s)
+		}
+	case "hosts":
+		if isStr && hostsContainLocal(s) {
+			return errors.Errorf("hosts %q is not allowed", s)
+		}
+	case "local_action":
+		return errors.Errorf("local_action is not allowed")
+	case "ansible_user":
+		if isStr && s != "" && !IsValidAnsibleUser(s) {
+			return errors.Errorf("invalid ansible_user")
+		}
+	case "ansible_ssh_private_key_file":
+		if isStr {
+			if _, err := fileutils2.CleanRelSubpath(s); err != nil {
+				return errors.Wrap(err, "ansible_ssh_private_key_file")
+			}
+		}
+	case "import_playbook", "include", "include_playbook", "import_tasks", "include_tasks", "include_vars":
+		if isStr {
+			if strings.Contains(s, "://") || filepath.IsAbs(s) || strings.Contains(s, "{{") {
+				return errors.Errorf("%s %q is not allowed", key, s)
+			}
+			if _, err := fileutils2.CleanRelSubpath(s); err != nil {
+				return errors.Wrapf(err, "%s", key)
+			}
+		}
+	}
+	if isDeniedAnsibleVar(lk) {
+		return errors.Errorf("%s is not allowed", key)
+	}
+	if parent == "hosts" && isLocalTarget(key) {
+		return errors.Errorf("host %q is not allowed", key)
+	}
+	return nil
+}
+
+func hostsContainLocal(s string) bool {
+	for _, part := range strings.Split(s, ",") {
+		part = strings.TrimSpace(part)
+		if part == "" {
+			continue
+		}
+		host := stripHostPort(part)
+		if isLocalTarget(host) {
+			return true
+		}
+	}
+	return false
+}
+
+func stripHostPort(part string) string {
+	if strings.HasPrefix(part, "[") {
+		end := strings.Index(part, "]")
+		if end > 1 {
+			return part[1:end]
+		}
+		return part
+	}
+	if i := strings.LastIndex(part, ":"); i > 0 {
+		if net.ParseIP(part) == nil {
+			return part[:i]
+		}
+	}
+	return part
+}
+
+func yamlString(v interface{}) (string, bool) {
+	switch t := v.(type) {
+	case string:
+		return t, true
+	case []byte:
+		return string(t), true
+	default:
+		return "", false
+	}
+}

pkg/util/ansiblev2/playbook_session.go

@@ -29,6 +29,8 @@ import (
 
 	"yunion.io/x/pkg/errors"
 	yerrors "yunion.io/x/pkg/util/errors"
+
+	"yunion.io/x/onecloud/pkg/util/fileutils2"
 )
 
 type IPlaybookSession interface {
@@ -145,7 +147,11 @@ func (r runnable) Run(ctx context.Context) (err error) {
 
 	// write out files
 	for name, content := range r.GetFiles() {
-		path := filepath.Join(tmpdir, name)
+		path, err2 := fileutils2.JoinInside(tmpdir, name)
+		if err2 != nil {
+			err = errors.Wrapf(err2, "playbook file %s", name)
+			return
+		}
 		dir := filepath.Dir(path)
 		err = os.MkdirAll(dir, os.FileMode(0700))
 		if err != nil {

pkg/util/ansiblev2/playbook_test.go

@@ -16,6 +16,8 @@ package ansiblev2
 
 import (
 	"testing"
+
+	"yunion.io/x/onecloud/pkg/util/ansible"
 )
 
 func TestPlaybookString(t *testing.T) {
@@ -102,5 +104,21 @@ func TestPlaybookString(t *testing.T) {
 	configureBlock.Name = "Configure wireguard networks"
 	play.Tasks = append(play.Tasks, configureBlock)
 	pb := NewPlaybook(play)
-	t.Logf("\n%s", pb.String())
+	yml := pb.String()
+	t.Logf("\n%s", yml)
+	if err := ansible.ValidatePlaybookYAML(yml); err != nil {
+		t.Fatalf("generated playbook rejected: %v\n%s", err, yml)
+	}
+	inv := NewInventory()
+	h := NewHost()
+	h.Vars = map[string]interface{}{
+		"ansible_user":                 "cloudroot",
+		"ansible_host":                 "10.1.2.3",
+		"ansible_ssh_private_key_file": ".id_rsa",
+		"ansible_become":               "yes",
+	}
+	inv.SetHost("gw1", h)
+	if err := ansible.ValidateInventoryYAML(inv.String()); err != nil {
+		t.Fatalf("generated inventory rejected: %v\n%s", err, inv.String())
+	}
 }

pkg/util/fileutils2/hostpath.go

@@ -102,11 +102,14 @@ func CleanRelSubpath(p string) (string, error) {
 	if p == "" {
 		return "", errors.Errorf("path is empty")
 	}
+	if strings.ContainsRune(p, 0) {
+		return "", errors.Errorf("path contains NUL")
+	}
 	if filepath.IsAbs(p) {
 		return "", errors.Errorf("path %q must be relative", p)
 	}
 	clean := filepath.Clean(p)
-	if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) {
+	if clean == "." || clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) {
 		return "", errors.Errorf("path %q is not allowed", p)
 	}
 	if filepath.IsAbs(clean) {

pkg/util/fileutils2/hostpath_test.go

@@ -90,7 +90,14 @@ func TestCleanRelSubpath(t *testing.T) {
 	if got != "foo/bar" {
 		t.Fatalf("got %q", got)
 	}
-	for _, p := range []string{"", "/abs", "../etc", "foo/../../etc", ".."} {
+	got, err = CleanRelSubpath(".id_rsa")
+	if err != nil {
+		t.Fatalf("dotfile: %v", err)
+	}
+	if got != ".id_rsa" {
+		t.Fatalf("got %q", got)
+	}
+	for _, p := range []string{"", "/abs", "../etc", "foo/../../etc", "..", ".", "a\x00b"} {
 		if _, err := CleanRelSubpath(p); err == nil {
 			t.Fatalf("expected error for %q", p)
 		}
@@ -111,6 +118,14 @@ func TestJoinInside(t *testing.T) {
 	if _, err := JoinInside("/mnt/disk", "/etc"); err == nil {
 		t.Fatal("expected absolute to fail")
 	}
+	base := t.TempDir()
+	got, err = JoinInside(base, "a/b.txt")
+	if err != nil {
+		t.Fatalf("temp base: %v", err)
+	}
+	if !IsPathInside(base, got) {
+		t.Fatalf("escaped: %q", got)
+	}
 	got, err = JoinInsideAll("/mnt/disk", "sub", "dir")
 	if err != nil {
 		t.Fatalf("join all: %v", err)