Improve Output Formatting and Error Messages (#467)

Summary:
Pull Request resolved: #467

* Quality-of-life updates to output format
* Support description field in cleanup actions
* Fix some typos

Reviewed By: d0n601

Differential Revision: D51772090

fbshipit-source-id: 8a3605deb68d371156b6732f5b269505f7f30541

cmd/root.go

@@ -20,6 +20,8 @@ THE SOFTWARE.
 package cmd
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 )
 
@@ -55,7 +57,10 @@ TTPForge is a Purple Team engagement tool to execute Tactics, Techniques, and Pr
 		TraverseChildren: true,
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 			if cmd.CalledAs() != "init" {
-				return cfg.init()
+				err := cfg.init()
+				if err != nil {
+					return fmt.Errorf("failed to load TTPForge configuration file: %w", err)
+				}
 			}
 			return nil
 		},

docs/foundations/repositories.md

@@ -25,7 +25,7 @@ ttpforge list ttps
 ```
 
 This will print a list of **TTP References**, which have the format
-`[repository name]/path/to/ttp`
+`[repository name]//path/to/ttp`
 
 You can look at the configuration of any given TTP by running
 `ttpforge show ttp [ttp-reference]` - for example:

example-ttps/args/choices.yaml

@@ -7,14 +7,14 @@ description: |
   you with this capability.
 args:
   - name: arg_with_choices
-    descriptions: you must pass one of these values in order to avoid an error
+    description: you must pass one of these values in order to avoid an error
     choices:
       - A
       - B
       - C
   - name: with_default
     type: int
-    descriptions: |
+    description: |
       arguments with `choices` can have default values to,
       but the default value must be one of the choices.
     choices:

example-ttps/args/regexp.yaml

@@ -7,7 +7,7 @@ description: |
   NOTE: `regexp` is only supported for arguments of type `string` (the default)
 args:
   - name: must_contain_ab
-    descriptions: requirement satisfied if `ab` occurs anywhere in the string
+    description: requirement satisfied if `ab` occurs anywhere in the string
     regexp: ab
   - name: must_start_with_1_end_with_7
     type: string

example-ttps/cleanup/basic.yaml

@@ -21,4 +21,5 @@ steps:
       as its cleanup action - you may use any of TTPForge's supported action types
       as a cleanup action.
     cleanup:
+      description: You should fill in the descriptions of custom cleanup actions
       inline: echo "Cleaning up third_step"

pkg/blocks/actions.go

@@ -28,9 +28,21 @@ type Action interface {
 	GetDefaultCleanupAction() Action
 	CanBeUsedInCompositeAction() bool
 	IsNil() bool
+	GetDescription() string
 }
 
-type actionDefaults struct{}
+// Shared action fields struct that also provides
+// default implementations for some Action
+// interface methods.
+// Every new action type should embed this struct
+type actionDefaults struct {
+	Description string `yaml:"description,omitempty"`
+}
+
+// GetDescription returns the description field from the action
+func (ad *actionDefaults) GetDescription() string {
+	return ad.Description
+}
 
 // GetDefaultCleanupAction provides a default implementation
 // of the GetDefaultCleanupAction method from the Action interface.

pkg/blocks/basicstep.go

@@ -47,7 +47,7 @@ const (
 
 // BasicStep is a type that represents a basic execution step.
 type BasicStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	Executor       string                  `yaml:"executor,omitempty"`
 	Inline         string                  `yaml:"inline,flow"`
 	Environment    map[string]string       `yaml:"env,omitempty"`

pkg/blocks/copypath.go

@@ -33,7 +33,7 @@ import (
 // via a C2, where there is no corresponding shell history
 // telemetry.
 type CopyPathStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	Source         string   `yaml:"copy_path,omitempty"`
 	Destination    string   `yaml:"to,omitempty"`
 	Recursive      bool     `yaml:"recursive,omitempty"`

pkg/blocks/createfile.go

@@ -34,7 +34,7 @@ import (
 // through an editor program or via a C2, where there is no
 // corresponding shell history telemetry
 type CreateFileStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	Path           string   `yaml:"create_file,omitempty"`
 	Contents       string   `yaml:"contents,omitempty"`
 	Overwrite      bool     `yaml:"overwrite,omitempty"`

pkg/blocks/editstep.go

@@ -39,7 +39,7 @@ type Edit struct {
 
 // EditStep represents one or more edits to a specific file
 type EditStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	FileToEdit     string   `yaml:"edit_file,omitempty"`
 	Edits          []*Edit  `yaml:"edits,omitempty"`
 	FileSystem     afero.Fs `yaml:"-,omitempty"`

pkg/blocks/fetchuri.go

@@ -36,7 +36,7 @@ import (
 // FetchURIStep represents a step in a process that consists of a main action,
 // a cleanup action, and additional metadata.
 type FetchURIStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	FetchURI       string   `yaml:"fetch_uri,omitempty"`
 	Retries        string   `yaml:"retries,omitempty"`
 	Location       string   `yaml:"location,omitempty"`

pkg/blocks/filestep.go

@@ -33,7 +33,7 @@ import (
 // FileStep represents a step in a process that consists of a main action,
 // a cleanup action, and additional metadata.
 type FileStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	FilePath       string                  `yaml:"file,omitempty"`
 	Executor       string                  `yaml:"executor,omitempty"`
 	Environment    map[string]string       `yaml:"env,omitempty"`

pkg/blocks/loader.go

@@ -110,7 +110,7 @@ func LoadTTP(ttpFilePath string, fsys afero.Fs, execCfg *TTPExecutionConfig, arg
 	var tmpContainer ArgSpecContainer
 	err = yaml.Unmarshal(result.PreambleBytes, &tmpContainer)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to unmarshal YAML preamble section: %w", err)
 	}
 
 	argValues, err := args.ParseAndValidate(tmpContainer.ArgSpecs, argsKvStrs)

pkg/blocks/printstr.go

@@ -28,7 +28,7 @@ import (
 
 // PrintStrAction is used to print a string to the console
 type PrintStrAction struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	Message        string `yaml:"print_str,omitempty"`
 }
 

pkg/blocks/removepath.go

@@ -32,7 +32,7 @@ import (
 // It will delete the file at the specified path
 // You must pass `recursive: true` to delete directories
 type RemovePathAction struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	Path           string   `yaml:"remove_path,omitempty"`
 	Recursive      bool     `yaml:"recursive,omitempty"`
 	FileSystem     afero.Fs `yaml:"-,omitempty"`

pkg/blocks/step.go

@@ -32,9 +32,8 @@ import (
 // common to every type of step (such as Name).
 // It centralizes validation to simplify the code
 type CommonStepFields struct {
-	Name        string         `yaml:"name,omitempty"`
-	Description string         `yaml:"description,omitempty"`
-	Checks      []checks.Check `yaml:"checks,omitempty"`
+	Name   string         `yaml:"name,omitempty"`
+	Checks []checks.Check `yaml:"checks,omitempty"`
 
 	// CleanupSpec is exported so that UnmarshalYAML
 	// can see it - however, it should be considered
@@ -121,7 +120,7 @@ func (s *Step) UnmarshalYAML(node *yaml.Node) error {
 	// associated with executing this step
 	s.action, err = s.ParseAction(node)
 	if err != nil {
-		return err
+		return fmt.Errorf("could not parse action for step %q: %w", s.Name, err)
 	}
 
 	// figure out what kind of action is
@@ -146,20 +145,28 @@ func (s *Step) UnmarshalYAML(node *yaml.Node) error {
 
 		s.cleanup, err = s.ParseAction(&csf.CleanupSpec)
 		if err != nil {
-			return err
+			return fmt.Errorf("could not parse cleanup action for step %q: %w", s.Name, err)
 		}
 	}
 	return nil
 }
 
 // Execute runs the action associated with this step
 func (s *Step) Execute(execCtx TTPExecutionContext) (*ActResult, error) {
+	desc := s.action.GetDescription()
+	if desc != "" {
+		logging.L().Infof("Description: %v", desc)
+	}
 	return s.action.Execute(execCtx)
 }
 
 // Cleanup runs the cleanup action associated with this step
 func (s *Step) Cleanup(execCtx TTPExecutionContext) (*ActResult, error) {
 	if s.cleanup != nil {
+		desc := s.cleanup.GetDescription()
+		if desc != "" {
+			logging.L().Infof("Description: %v", desc)
+		}
 		return s.cleanup.Execute(execCtx)
 	}
 	logging.L().Infof("No Cleanup Action Defined for Step %v", s.Name)
@@ -204,7 +211,7 @@ func (s *Step) ParseAction(node *yaml.Node) (Action, error) {
 		}
 	}
 	if action == nil {
-		return nil, fmt.Errorf("step %v did not match any valid step type", s.Name)
+		return nil, errors.New("action fields did not match any valid action type")
 	}
 	return action, nil
 }

pkg/blocks/subttp.go

@@ -28,7 +28,7 @@ import (
 
 // SubTTPStep represents a step within a parent TTP that references a separate TTP file.
 type SubTTPStep struct {
-	actionDefaults `yaml:"-"`
+	actionDefaults `yaml:",inline"`
 	TtpRef         string            `yaml:"ttp"`
 	Args           map[string]string `yaml:"args"`
 

pkg/blocks/ttps.go

@@ -135,7 +135,7 @@ func reduceIndentation(b []byte, n int) []byte {
 //
 // error: An error if any part of the validation fails, otherwise nil.
 func (t *TTP) Validate(execCtx TTPExecutionContext) error {
-	logging.L().Info("[*] Validating TTP")
+	logging.L().Debugf("Validating TTP %q...", t.Name)
 
 	// validate MITRE mapping
 	if t.MitreAttackMapping != nil && len(t.MitreAttackMapping.Tactics) == 0 {
@@ -153,7 +153,7 @@ func (t *TTP) Validate(execCtx TTPExecutionContext) error {
 			return err
 		}
 	}
-	logging.L().Info("[+] Finished validating steps")
+	logging.L().Debug("...finished validating TTP.")
 	return nil
 }
 
@@ -189,6 +189,7 @@ func (t *TTP) chdir() (func(), error) {
 // *StepResultsRecord: A StepResultsRecord containing the results of each step.
 // error: An error if any of the steps fail to execute.
 func (t *TTP) Execute(execCtx *TTPExecutionContext) (*StepResultsRecord, error) {
+	logging.DividerThick()
 	logging.L().Infof("RUNNING TTP: %v", t.Name)
 
 	// verify that we actually meet the necessary requirements to execute this TTP
@@ -203,11 +204,12 @@ func (t *TTP) Execute(execCtx *TTPExecutionContext) (*StepResultsRecord, error)
 	}
 
 	stepResults, firstStepToCleanupIdx, runErr := t.RunSteps(execCtx)
+	logging.DividerThin()
 	if runErr != nil {
 		// we need to run cleanup so we don't return here
 		logging.L().Errorf("[*] Error executing TTP: %v", runErr)
 	} else {
-		logging.L().Info("[*] Completed TTP - No Errors :)")
+		logging.L().Info("TTP Completed Successfully! ✅")
 	}
 	if !execCtx.Cfg.NoCleanup {
 		if execCtx.Cfg.CleanupDelaySeconds > 0 {
@@ -253,7 +255,8 @@ func (t *TTP) RunSteps(execCtx *TTPExecutionContext) (*StepResultsRecord, int, e
 	firstStepToCleanupIdx := -1
 	for stepIdx, step := range t.Steps {
 		stepCopy := step
-		logging.L().Infof("Executing Step #%d: %s", stepIdx+1, step.Name)
+		logging.DividerThin()
+		logging.L().Infof("Executing Step #%d: %q", stepIdx+1, step.Name)
 
 		// core execution - run the step action
 		stepResult, err := stepCopy.Execute(*execCtx)
@@ -305,10 +308,13 @@ func (t *TTP) startCleanupAtStepIdx(firstStepToCleanupIdx int, execCtx *TTPExecu
 	}
 	defer changeBack()
 
-	logging.L().Info("[*] Beginning Cleanup")
+	logging.DividerThick()
+	logging.L().Infof("CLEANING UP TTP: %q", t.Name)
 	var cleanupResults []*ActResult
 	for cleanupIdx := firstStepToCleanupIdx; cleanupIdx >= 0; cleanupIdx-- {
 		stepToCleanup := t.Steps[cleanupIdx]
+		logging.DividerThin()
+		logging.L().Infof("Cleaning Up Step #%d: %q", cleanupIdx+1, stepToCleanup.Name)
 		cleanupResult, err := stepToCleanup.Cleanup(*execCtx)
 		// must be careful to put these in step order, not in execution (reverse) order
 		cleanupResults = append([]*ActResult{cleanupResult}, cleanupResults...)
@@ -318,6 +324,7 @@ func (t *TTP) startCleanupAtStepIdx(firstStepToCleanupIdx int, execCtx *TTPExecu
 			continue
 		}
 	}
-	logging.L().Info("[*] Finished Cleanup")
+	logging.DividerThin()
+	logging.L().Info("Finished Cleanup Successfully ✅")
 	return cleanupResults, nil
 }