bug: add a data validation process to avoid merging incorrect operations

21 files changed, 452 insertions, 63 deletions

diff --git a/bug/bug.go b/bug/bug.go
index e548757e..3342ecfa 100644
--- a/bug/bug.go
+++ b/bug/bug.go
@@ -2,13 +2,13 @@ package bug
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"strings"
 
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
+	"github.com/pkg/errors"
 )
 
 const bugsRefPattern = "refs/bugs/"
@@ -279,31 +279,31 @@ func refsToIds(refs []string) []string {
 	return ids
 }
 
-// IsValid check if the Bug data is valid
-func (bug *Bug) IsValid() bool {
+// Validate check if the Bug data is valid
+func (bug *Bug) Validate() error {
 	// non-empty
 	if len(bug.packs) == 0 && bug.staging.IsEmpty() {
-		return false
+		return fmt.Errorf("bug has no operations")
 	}
 
-	// check if each pack is valid
+	// check if each pack and operations are valid
 	for _, pack := range bug.packs {
-		if !pack.IsValid() {
-			return false
+		if err := pack.Validate(); err != nil {
+			return err
 		}
 	}
 
 	// check if staging is valid if needed
 	if !bug.staging.IsEmpty() {
-		if !bug.staging.IsValid() {
-			return false
+		if err := bug.staging.Validate(); err != nil {
+			return errors.Wrap(err, "staging")
 		}
 	}
 
 	// The very first Op should be a CreateOp
 	firstOp := bug.FirstOp()
 	if firstOp == nil || firstOp.OpType() != CreateOp {
-		return false
+		return fmt.Errorf("first operation should be a Create op")
 	}
 
 	// Check that there is no more CreateOp op
@@ -316,10 +316,10 @@ func (bug *Bug) IsValid() bool {
 	}
 
 	if createCount != 1 {
-		return false
+		return fmt.Errorf("only one Create op allowed")
 	}
 
-	return true
+	return nil
 }
 
 // Append an operation into the staging area, to be committed later
@@ -338,6 +338,10 @@ func (bug *Bug) Commit(repo repository.Repo) error {
 		return fmt.Errorf("can't commit a bug with no pending operation")
 	}
 
+	if err := bug.Validate(); err != nil {
+		return errors.Wrap(err, "can't commit a bug with invalid data")
+	}
+
 	// Write the Ops as a Git blob containing the serialized array
 	hash, err := bug.staging.Write(repo)
 	if err != nil {
diff --git a/bug/bug_actions.go b/bug/bug_actions.go
index 8bda15b7..7f2194ab 100644
--- a/bug/bug_actions.go
+++ b/bug/bug_actions.go
@@ -66,8 +66,8 @@ func MergeAll(repo repository.Repo, remote string) <-chan MergeResult {
 			}
 
 			// Check for error in remote data
-			if !remoteBug.IsValid() {
-				out <- newMergeStatus(MergeStatusInvalid, id, nil)
+			if err := remoteBug.Validate(); err != nil {
+				out <- newMergeInvalidStatus(id, err.Error())
 				continue
 			}
 
@@ -128,12 +128,26 @@ const (
 	MergeStatusNothing
 )
 
-func (ms MergeStatus) String() string {
-	switch ms {
+type MergeResult struct {
+	// Err is set when a terminal error occur in the process
+	Err error
+
+	Id     string
+	Status MergeStatus
+
+	// Only set for invalid status
+	Reason string
+
+	// Not set for invalid status
+	Bug *Bug
+}
+
+func (mr MergeResult) String() string {
+	switch mr.Status {
 	case MergeStatusNew:
 		return "new"
 	case MergeStatusInvalid:
-		return "invalid data"
+		return fmt.Sprintf("invalid data: %s", mr.Reason)
 	case MergeStatusUpdated:
 		return "updated"
 	case MergeStatusNothing:
@@ -143,15 +157,6 @@ func (ms MergeStatus) String() string {
 	}
 }
 
-type MergeResult struct {
-	// Err is set when a terminal error occur in the process
-	Err error
-
-	Id     string
-	Status MergeStatus
-	Bug    *Bug
-}
-
 func newMergeError(err error, id string) MergeResult {
 	return MergeResult{
 		Err: err,
@@ -168,3 +173,11 @@ func newMergeStatus(status MergeStatus, id string, bug *Bug) MergeResult {
 		Bug: bug,
 	}
 }
+
+func newMergeInvalidStatus(id string, reason string) MergeResult {
+	return MergeResult{
+		Id:     id,
+		Status: MergeStatusInvalid,
+		Reason: reason,
+	}
+}
diff --git a/bug/interface.go b/bug/interface.go
index c397bfbc..72dee61c 100644
--- a/bug/interface.go
+++ b/bug/interface.go
@@ -12,8 +12,8 @@ type Interface interface {
 	// HumanId return the Bug identifier truncated for human consumption
 	HumanId() string
 
-	// IsValid check if the Bug data is valid
-	IsValid() bool
+	// Validate check if the Bug data is valid
+	Validate() error
 
 	// Append an operation into the staging area, to be committed later
 	Append(op Operation)
diff --git a/bug/label.go b/bug/label.go
index b19a980f..b73222dd 100644
--- a/bug/label.go
+++ b/bug/label.go
@@ -3,6 +3,9 @@ package bug
 import (
 	"fmt"
 	"io"
+	"strings"
+
+	"github.com/MichaelMure/git-bug/util/text"
 )
 
 type Label string
@@ -27,3 +30,21 @@ func (l *Label) UnmarshalGQL(v interface{}) error {
 func (l Label) MarshalGQL(w io.Writer) {
 	w.Write([]byte(`"` + l.String() + `"`))
 }
+
+func (l Label) Validate() error {
+	str := string(l)
+
+	if text.Empty(str) {
+		return fmt.Errorf("empty")
+	}
+
+	if strings.Contains(str, "\n") {
+		return fmt.Errorf("should be a single line")
+	}
+
+	if !text.Safe(str) {
+		return fmt.Errorf("not fully printable")
+	}
+
+	return nil
+}
diff --git a/bug/operation.go b/bug/operation.go
index 6a8aa0cd..b148141c 100644
--- a/bug/operation.go
+++ b/bug/operation.go
@@ -2,6 +2,9 @@ package bug
 
 import (
 	"github.com/MichaelMure/git-bug/util/git"
+	"github.com/pkg/errors"
+
+	"fmt"
 	"time"
 )
 
@@ -25,13 +28,14 @@ type Operation interface {
 	Time() time.Time
 	// GetUnixTime return the unix timestamp when the operation was added
 	GetUnixTime() int64
-	// Apply the operation to a Snapshot to create the final state
-	Apply(snapshot Snapshot) Snapshot
+	// GetAuthor return the author of the operation
+	GetAuthor() Person
 	// GetFiles return the files needed by this operation
 	GetFiles() []git.Hash
-
-	// TODO: data validation (ex: a title is a single line)
-	// Validate() bool
+	// Apply the operation to a Snapshot to create the final state
+	Apply(snapshot Snapshot) Snapshot
+	// Validate check if the operation is valid (ex: a title is a single line)
+	Validate() error
 }
 
 // OpBase implement the common code for all operations
@@ -65,7 +69,35 @@ func (op OpBase) GetUnixTime() int64 {
 	return op.UnixTime
 }
 
+// GetAuthor return the author of the operation
+func (op OpBase) GetAuthor() Person {
+	return op.Author
+}
+
 // GetFiles return the files needed by this operation
 func (op OpBase) GetFiles() []git.Hash {
 	return nil
 }
+
+// Validate check the OpBase for errors
+func OpBaseValidate(op Operation, opType OperationType) error {
+	if op.OpType() != opType {
+		return fmt.Errorf("incorrect operation type (expected: %v, actual: %v)", opType, op.OpType())
+	}
+
+	if op.GetUnixTime() == 0 {
+		return fmt.Errorf("time not set")
+	}
+
+	if err := op.GetAuthor().Validate(); err != nil {
+		return errors.Wrap(err, "author")
+	}
+
+	for _, hash := range op.GetFiles() {
+		if !hash.IsValid() {
+			return fmt.Errorf("file with invalid hash %v", hash)
+		}
+	}
+
+	return nil
+}
diff --git a/bug/operation_pack.go b/bug/operation_pack.go
index fe26952f..03d538d5 100644
--- a/bug/operation_pack.go
+++ b/bug/operation_pack.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/MichaelMure/git-bug/util/git"
+	"github.com/pkg/errors"
 )
 
 const formatVersion = 1
@@ -24,8 +25,10 @@ type OperationPack struct {
 	commitHash git.Hash
 }
 
+// hold the different operation type to instantiate to parse JSON
 var operations map[OperationType]reflect.Type
 
+// Register will register a new type of Operation to be able to parse the corresponding JSON
 func Register(t OperationType, op interface{}) {
 	if operations == nil {
 		operations = make(map[OperationType]reflect.Type)
@@ -96,8 +99,18 @@ func (opp *OperationPack) IsEmpty() bool {
 }
 
 // IsValid tell if the OperationPack is considered valid
-func (opp *OperationPack) IsValid() bool {
-	return !opp.IsEmpty()
+func (opp *OperationPack) Validate() error {
+	if opp.IsEmpty() {
+		return fmt.Errorf("empty")
+	}
+
+	for _, op := range opp.Operations {
+		if err := op.Validate(); err != nil {
+			return errors.Wrap(err, "op")
+		}
+	}
+
+	return nil
 }
 
 // Write will serialize and store the OperationPack as a git blob and return
diff --git a/bug/person.go b/bug/person.go
index d9f1108b..9fa86804 100644
--- a/bug/person.go
+++ b/bug/person.go
@@ -2,9 +2,11 @@ package bug
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/MichaelMure/git-bug/repository"
+	"github.com/MichaelMure/git-bug/util/text"
 )
 
 type Person struct {
@@ -37,3 +39,27 @@ func GetUser(repo repository.Repo) (Person, error) {
 func (p Person) Match(query string) bool {
 	return strings.Contains(strings.ToLower(p.Name), strings.ToLower(query))
 }
+
+func (p Person) Validate() error {
+	if text.Empty(p.Name) {
+		return fmt.Errorf("name is not set")
+	}
+
+	if strings.Contains(p.Name, "\n") {
+		return fmt.Errorf("name should be a single line")
+	}
+
+	if !text.Safe(p.Name) {
+		return fmt.Errorf("name is not fully printable")
+	}
+
+	if strings.Contains(p.Email, "\n") {
+		return fmt.Errorf("email should be a single line")
+	}
+
+	if !text.Safe(p.Email) {
+		return fmt.Errorf("email is not fully printable")
+	}
+
+	return nil
+}
diff --git a/bug/status.go b/bug/status.go
index f15924e2..737c8d31 100644
--- a/bug/status.go
+++ b/bug/status.go
@@ -47,3 +47,11 @@ func StatusFromString(str string) (Status, error) {
 		return 0, fmt.Errorf("unknow status")
 	}
 }
+
+func (s Status) Validate() error {
+	if s != OpenStatus && s != ClosedStatus {
+		return fmt.Errorf("invalid")
+	}
+
+	return nil
+}
diff --git a/cache/bug_cache.go b/cache/bug_cache.go
index 67c16e96..b0dbb6cc 100644
--- a/cache/bug_cache.go
+++ b/cache/bug_cache.go
@@ -44,7 +44,10 @@ func (c *BugCache) AddCommentWithFiles(message string, files []git.Hash) error {
 		return err
 	}
 
-	operations.CommentWithFiles(c.bug, author, message, files)
+	err = operations.CommentWithFiles(c.bug, author, message, files)
+	if err != nil {
+		return err
+	}
 
 	return c.notifyUpdated()
 }
@@ -74,7 +77,10 @@ func (c *BugCache) Open() error {
 		return err
 	}
 
-	operations.Open(c.bug, author)
+	err = operations.Open(c.bug, author)
+	if err != nil {
+		return err
+	}
 
 	return c.notifyUpdated()
 }
@@ -85,7 +91,10 @@ func (c *BugCache) Close() error {
 		return err
 	}
 
-	operations.Close(c.bug, author)
+	err = operations.Close(c.bug, author)
+	if err != nil {
+		return err
+	}
 
 	return c.notifyUpdated()
 }
@@ -96,7 +105,10 @@ func (c *BugCache) SetTitle(title string) error {
 		return err
 	}
 
-	operations.SetTitle(c.bug, author, title)
+	err = operations.SetTitle(c.bug, author, title)
+	if err != nil {
+		return err
+	}
 
 	return c.notifyUpdated()
 }
diff --git a/commands/pull.go b/commands/pull.go
index 64de8bf5..5ad4acc3 100644
--- a/commands/pull.go
+++ b/commands/pull.go
@@ -42,7 +42,7 @@ func runPull(cmd *cobra.Command, args []string) error {
 		}
 
 		if merge.Status != bug.MergeStatusNothing {
-			fmt.Printf("%s: %s\n", merge.Bug.HumanId(), merge.Status)
+			fmt.Printf("%s: %s\n", merge.Bug.HumanId(), merge)
 		}
 	}
 
diff --git a/misc/random_bugs/create_random_bugs.go b/misc/random_bugs/create_random_bugs.go
index ad81e2ab..75ce5f82 100644
--- a/misc/random_bugs/create_random_bugs.go
+++ b/misc/random_bugs/create_random_bugs.go
@@ -57,8 +57,8 @@ func GenerateRandomBugsWithSeed(opts Options, seed int64) []*bug.Bug {
 		comment,
 		title,
 		labels,
-		operations.Open,
-		operations.Close,
+		open,
+		close,
 	}
 
 	result := make([]*bug.Bug, opts.BugNumber)
@@ -148,11 +148,19 @@ func paragraphs() string {
 }
 
 func comment(b bug.Interface, p bug.Person) {
-	operations.Comment(b, p, paragraphs())
+	_ = operations.Comment(b, p, paragraphs())
 }
 
 func title(b bug.Interface, p bug.Person) {
-	operations.SetTitle(b, p, fake.Sentence())
+	_ = operations.SetTitle(b, p, fake.Sentence())
+}
+
+func open(b bug.Interface, p bug.Person) {
+	_ = operations.Open(b, p)
+}
+
+func close(b bug.Interface, p bug.Person) {
+	_ = operations.Close(b, p)
 }
 
 var addedLabels []string
@@ -179,5 +187,5 @@ func labels(b bug.Interface, p bug.Person) {
 	// ignore error
 	// if the randomisation produce no changes, no op
 	// is added to the bug
-	operations.ChangeLabels(b, p, added, removed)
+	_, _ = operations.ChangeLabels(b, p, added, removed)
 }
diff --git a/operations/add_comment.go b/operations/add_comment.go
index 8c4b8b9c..01572eb7 100644
--- a/operations/add_comment.go
+++ b/operations/add_comment.go
@@ -1,8 +1,11 @@
 package operations
 
 import (
+	"fmt"
+
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/util/git"
+	"github.com/MichaelMure/git-bug/util/text"
 )
 
 // AddCommentOperation will add a new comment in the bug
@@ -33,6 +36,22 @@ func (op AddCommentOperation) GetFiles() []git.Hash {
 	return op.Files
 }
 
+func (op AddCommentOperation) Validate() error {
+	if err := bug.OpBaseValidate(op, bug.AddCommentOp); err != nil {
+		return err
+	}
+
+	if text.Empty(op.Message) {
+		return fmt.Errorf("message is empty")
+	}
+
+	if !text.Safe(op.Message) {
+		return fmt.Errorf("message is not fully printable")
+	}
+
+	return nil
+}
+
 func NewAddCommentOp(author bug.Person, message string, files []git.Hash) AddCommentOperation {
 	return AddCommentOperation{
 		OpBase:  bug.NewOpBase(bug.AddCommentOp, author),
@@ -42,11 +61,15 @@ func NewAddCommentOp(author bug.Person, message string, files []git.Hash) AddCom
 }
 
 // Convenience function to apply the operation
-func Comment(b bug.Interface, author bug.Person, message string) {
-	CommentWithFiles(b, author, message, nil)
+func Comment(b bug.Interface, author bug.Person, message string) error {
+	return CommentWithFiles(b, author, message, nil)
 }
 
-func CommentWithFiles(b bug.Interface, author bug.Person, message string, files []git.Hash) {
+func CommentWithFiles(b bug.Interface, author bug.Person, message string, files []git.Hash) error {
 	addCommentOp := NewAddCommentOp(author, message, files)
+	if err := addCommentOp.Validate(); err != nil {
+		return err
+	}
 	b.Append(addCommentOp)
+	return nil
 }
diff --git a/operations/create.go b/operations/create.go
index a671171e..efd4492f 100644
--- a/operations/create.go
+++ b/operations/create.go
@@ -1,8 +1,12 @@
 package operations
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/util/git"
+	"github.com/MichaelMure/git-bug/util/text"
 )
 
 // CreateOperation define the initial creation of a bug
@@ -34,6 +38,30 @@ func (op CreateOperation) GetFiles() []git.Hash {
 	return op.Files
 }
 
+func (op CreateOperation) Validate() error {
+	if err := bug.OpBaseValidate(op, bug.CreateOp); err != nil {
+		return err
+	}
+
+	if text.Empty(op.Title) {
+		return fmt.Errorf("title is empty")
+	}
+
+	if strings.Contains(op.Title, "\n") {
+		return fmt.Errorf("title should be a single line")
+	}
+
+	if !text.Safe(op.Title) {
+		return fmt.Errorf("title is not fully printable")
+	}
+
+	if !text.Safe(op.Message) {
+		return fmt.Errorf("message is not fully printable")
+	}
+
+	return nil
+}
+
 func NewCreateOp(author bug.Person, title, message string, files []git.Hash) CreateOperation {
 	return CreateOperation{
 		OpBase:  bug.NewOpBase(bug.CreateOp, author),
@@ -51,6 +79,11 @@ func Create(author bug.Person, title, message string) (*bug.Bug, error) {
 func CreateWithFiles(author bug.Person, title, message string, files []git.Hash) (*bug.Bug, error) {
 	newBug := bug.NewBug()
 	createOp := NewCreateOp(author, title, message, files)
+
+	if err := createOp.Validate(); err != nil {
+		return nil, err
+	}
+
 	newBug.Append(createOp)
 
 	return newBug, nil
diff --git a/operations/label_change.go b/operations/label_change.go
index a3cc9898..507651df 100644
--- a/operations/label_change.go
+++ b/operations/label_change.go
@@ -5,6 +5,7 @@ import (
 	"sort"
 
 	"github.com/MichaelMure/git-bug/bug"
+	"github.com/pkg/errors"
 )
 
 var _ bug.Operation = LabelChangeOperation{}
@@ -49,6 +50,30 @@ AddLoop:
 	return snapshot
 }
 
+func (op LabelChangeOperation) Validate() error {
+	if err := bug.OpBaseValidate(op, bug.LabelChangeOp); err != nil {
+		return err
+	}
+
+	for _, l := range op.Added {
+		if err := l.Validate(); err != nil {
+			return errors.Wrap(err, "added label")
+		}
+	}
+
+	for _, l := range op.Removed {
+		if err := l.Validate(); err != nil {
+			return errors.Wrap(err, "removed label")
+		}
+	}
+
+	if len(op.Added)+len(op.Removed) <= 0 {
+		return fmt.Errorf("no label change")
+	}
+
+	return nil
+}
+
 func NewLabelChangeOperation(author bug.Person, added, removed []bug.Label) LabelChangeOperation {
 	return LabelChangeOperation{
 		OpBase:  bug.NewOpBase(bug.LabelChangeOp, author),
@@ -108,6 +133,10 @@ func ChangeLabels(b bug.Interface, author bug.Person, add, remove []string) ([]L
 
 	labelOp := NewLabelChangeOperation(author, added, removed)
 
+	if err := labelOp.Validate(); err != nil {
+		return nil, err
+	}
+
 	b.Append(labelOp)
 
 	return results, nil
diff --git a/operations/operations_test.go b/operations/operations_test.go
new file mode 100644
index 00000000..6fba7917
--- /dev/null
+++ b/operations/operations_test.go
@@ -0,0 +1,69 @@
+package operations
+
+import (
+	"testing"
+
+	"github.com/MichaelMure/git-bug/bug"
+	"github.com/MichaelMure/git-bug/util/git"
+)
+
+func TestValidate(t *testing.T) {
+	rene := bug.Person{
+		Name:  "René Descartes",
+		Email: "rene@descartes.fr",
+	}
+
+	good := []bug.Operation{
+		NewCreateOp(rene, "title", "message", nil),
+		NewSetTitleOp(rene, "title2", "title1"),
+		NewAddCommentOp(rene, "message2", nil),
+		NewSetStatusOp(rene, bug.ClosedStatus),
+		NewLabelChangeOperation(rene, []bug.Label{"added"}, []bug.Label{"removed"}),
+	}
+
+	for _, op := range good {
+		if err := op.Validate(); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	bad := []bug.Operation{
+		// opbase
+		NewSetStatusOp(bug.Person{Name: "", Email: "rene@descartes.fr"}, bug.ClosedStatus),
+		NewSetStatusOp(bug.Person{Name: "René Descartes\u001b", Email: "rene@descartes.fr"}, bug.ClosedStatus),
+		NewSetStatusOp(bug.Person{Name: "René Descartes", Email: "rene@descartes.fr\u001b"}, bug.ClosedStatus),
+		NewSetStatusOp(bug.Person{Name: "René \nDescartes", Email: "rene@descartes.fr"}, bug.ClosedStatus),
+		NewSetStatusOp(bug.Person{Name: "René Descartes", Email: "rene@\ndescartes.fr"}, bug.ClosedStatus),
+		CreateOperation{OpBase: bug.OpBase{
+			Author:        rene,
+			UnixTime:      0,
+			OperationType: bug.CreateOp,
+		},
+			Title:   "title",
+			Message: "message",
+		},
+
+		NewCreateOp(rene, "multi\nline", "message", nil),
+		NewCreateOp(rene, "title", "message", []git.Hash{git.Hash("invalid")}),
+		NewCreateOp(rene, "title\u001b", "message", nil),
+		NewCreateOp(rene, "title", "message\u001b", nil),
+		NewSetTitleOp(rene, "multi\nline", "title1"),
+		NewSetTitleOp(rene, "title", "multi\nline"),
+		NewSetTitleOp(rene, "title\u001b", "title2"),
+		NewSetTitleOp(rene, "title", "title2\u001b"),
+		NewAddCommentOp(rene, "", nil),
+		NewAddCommentOp(rene, "message\u001b", nil),
+		NewAddCommentOp(rene, "message", []git.Hash{git.Hash("invalid")}),
+		NewSetStatusOp(rene, 1000),
+		NewSetStatusOp(rene, 0),
+		NewLabelChangeOperation(rene, []bug.Label{}, []bug.Label{}),
+		NewLabelChangeOperation(rene, []bug.Label{"multi\nline"}, []bug.Label{}),
+	}
+
+	for i, op := range bad {
+		if err := op.Validate(); err == nil {
+			t.Fatal("validation should have failed", i, op)
+		}
+	}
+
+}
diff --git a/operations/set_status.go b/operations/set_status.go
index 37ebac10..07dcf208 100644
--- a/operations/set_status.go
+++ b/operations/set_status.go
@@ -2,6 +2,7 @@ package operations
 
 import (
 	"github.com/MichaelMure/git-bug/bug"
+	"github.com/pkg/errors"
 )
 
 // SetStatusOperation will change the status of a bug
@@ -19,6 +20,18 @@ func (op SetStatusOperation) Apply(snapshot bug.Snapshot) bug.Snapshot {
 	return snapshot
 }
 
+func (op SetStatusOperation) Validate() error {
+	if err := bug.OpBaseValidate(op, bug.SetStatusOp); err != nil {
+		return err
+	}
+
+	if err := op.Status.Validate(); err != nil {
+		return errors.Wrap(err, "status")
+	}
+
+	return nil
+}
+
 func NewSetStatusOp(author bug.Person, status bug.Status) SetStatusOperation {
 	return SetStatusOperation{
 		OpBase: bug.NewOpBase(bug.SetStatusOp, author),
@@ -27,13 +40,21 @@ func NewSetStatusOp(author bug.Person, status bug.Status) SetStatusOperation {
 }
 
 // Convenience function to apply the operation
-func Open(b bug.Interface, author bug.Person) {
+func Open(b bug.Interface, author bug.Person) error {
 	op := NewSetStatusOp(author, bug.OpenStatus)
+	if err := op.Validate(); err != nil {
+		return err
+	}
 	b.Append(op)
+	return nil
 }
 
 // Convenience function to apply the operation
-func Close(b bug.Interface, author bug.Person) {
+func Close(b bug.Interface, author bug.Person) error {
 	op := NewSetStatusOp(author, bug.ClosedStatus)
+	if err := op.Validate(); err != nil {
+		return err
+	}
 	b.Append(op)
+	return nil
 }
diff --git a/operations/set_title.go b/operations/set_title.go
index 7aa76268..46addce6 100644
--- a/operations/set_title.go
+++ b/operations/set_title.go
@@ -1,7 +1,11 @@
 package operations
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/MichaelMure/git-bug/bug"
+	"github.com/MichaelMure/git-bug/util/text"
 )
 
 // SetTitleOperation will change the title of a bug
@@ -20,6 +24,34 @@ func (op SetTitleOperation) Apply(snapshot bug.Snapshot) bug.Snapshot {
 	return snapshot
 }
 
+func (op SetTitleOperation) Validate() error {
+	if err := bug.OpBaseValidate(op, bug.SetTitleOp); err != nil {
+		return err
+	}
+
+	if text.Empty(op.Title) {
+		return fmt.Errorf("title is empty")
+	}
+
+	if strings.Contains(op.Title, "\n") {
+		return fmt.Errorf("title should be a single line")
+	}
+
+	if !text.Safe(op.Title) {
+		return fmt.Errorf("title should be fully printable")
+	}
+
+	if strings.Contains(op.Was, "\n") {
+		return fmt.Errorf("previous title should be a single line")
+	}
+
+	if !text.Safe(op.Was) {
+		return fmt.Errorf("previous title should be fully printable")
+	}
+
+	return nil
+}
+
 func NewSetTitleOp(author bug.Person, title string, was string) SetTitleOperation {
 	return SetTitleOperation{
 		OpBase: bug.NewOpBase(bug.SetTitleOp, author),
@@ -29,7 +61,7 @@ func NewSetTitleOp(author bug.Person, title string, was string) SetTitleOperatio
 }
 
 // Convenience function to apply the operation
-func SetTitle(b bug.Interface, author bug.Person, title string) {
+func SetTitle(b bug.Interface, author bug.Person, title string) error {
 	it := bug.NewOperationIterator(b)
 
 	var lastTitleOp bug.Operation
@@ -48,5 +80,11 @@ func SetTitle(b bug.Interface, author bug.Person, title string) {
 	}
 
 	setTitleOp := NewSetTitleOp(author, title, was)
+
+	if err := setTitleOp.Validate(); err != nil {
+		return err
+	}
+
 	b.Append(setTitleOp)
+	return nil
 }
diff --git a/termui/bug_table.go b/termui/bug_table.go
index 7cd3ba9a..8c32a631 100644
--- a/termui/bug_table.go
+++ b/termui/bug_table.go
@@ -434,7 +434,7 @@ func (bt *bugTable) pull(g *gocui.Gui, v *gocui.View) error {
 				})
 			} else {
 				fmt.Fprintf(&buffer, "%s%s: %s",
-					beginLine, colors.Cyan(merge.Bug.HumanId()), merge.Status,
+					beginLine, colors.Cyan(merge.Bug.HumanId()), merge,
 				)
 
 				beginLine = "\n"
diff --git a/tests/bug_test.go b/tests/bug_test.go
index fdda31b1..02336974 100644
--- a/tests/bug_test.go
+++ b/tests/bug_test.go
@@ -2,10 +2,14 @@ package tests
 
 import (
 	"github.com/MichaelMure/git-bug/bug"
+	"github.com/MichaelMure/git-bug/repository"
+
 	"testing"
 )
 
 func TestBugId(t *testing.T) {
+	mockRepo := repository.NewMockRepoForTest()
+
 	bug1 := bug.NewBug()
 
 	bug1.Append(createOp)
@@ -20,33 +24,35 @@ func TestBugId(t *testing.T) {
 }
 
 func TestBugValidity(t *testing.T) {
+	mockRepo := repository.NewMockRepoForTest()
+
 	bug1 := bug.NewBug()
 
-	if bug1.IsValid() {
+	if bug1.Validate() == nil {
 		t.Fatal("Empty bug should be invalid")
 	}
 
 	bug1.Append(createOp)
 
-	if !bug1.IsValid() {
+	if bug1.Validate() != nil {
 		t.Fatal("Bug with just a CreateOp should be valid")
 	}
 
-	bug1.Append(createOp)
-
-	if bug1.IsValid() {
-		t.Fatal("Bug with multiple CreateOp should be invalid")
-	}
-
 	err := bug1.Commit(mockRepo)
-
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if bug1.IsValid() {
+	bug1.Append(createOp)
+
+	if bug1.Validate() == nil {
 		t.Fatal("Bug with multiple CreateOp should be invalid")
 	}
+
+	err = bug1.Commit(mockRepo)
+	if err == nil {
+		t.Fatal("Invalid bug should not commit")
+	}
 }
 
 //func TestBugSerialisation(t *testing.T) {
diff --git a/tests/operation_iterator_test.go b/tests/operation_iterator_test.go
index f1840091..790c8a62 100644
--- a/tests/operation_iterator_test.go
+++ b/tests/operation_iterator_test.go
@@ -18,10 +18,10 @@ var (
 	addCommentOp  = operations.NewAddCommentOp(rene, "message2", nil)
 	setStatusOp   = operations.NewSetStatusOp(rene, bug.ClosedStatus)
 	labelChangeOp = operations.NewLabelChangeOperation(rene, []bug.Label{"added"}, []bug.Label{"removed"})
-	mockRepo      = repository.NewMockRepoForTest()
 )
 
 func TestOpIterator(t *testing.T) {
+	mockRepo := repository.NewMockRepoForTest()
 
 	bug1 := bug.NewBug()
 
diff --git a/util/text/validate.go b/util/text/validate.go
new file mode 100644
index 00000000..68bdf48b
--- /dev/null
+++ b/util/text/validate.go
@@ -0,0 +1,33 @@
+package text
+
+import (
+	"strings"
+	"unicode"
+)
+
+// Empty tell if the string is considered empty once space
+// and not graphics characters are removed
+func Empty(s string) bool {
+	trim := strings.TrimFunc(s, func(r rune) bool {
+		return unicode.IsSpace(r) || !unicode.IsGraphic(r)
+	})
+
+	return trim == ""
+}
+
+// Safe will tell if a character in the string is considered unsafe
+// Currently trigger on unicode control character except \n, \t and \r
+func Safe(s string) bool {
+	for _, r := range s {
+		switch r {
+		case '\t', '\r', '\n':
+			continue
+		}
+
+		if unicode.IsControl(r) {
+			return false
+		}
+	}
+
+	return true
+}