Skip to content

Commit

Permalink
V8 add redact option for env block when pushing an app with a manifes…
Browse files Browse the repository at this point in the history
…t.yml (#2787)

* implement redact to avoid leaking secrets from `env`

[#186767925]

IF the app is pushed with a manifest
AND the app needs secrets set via environment variables
THEN currently there is no way to avoid leaking secrets in the output of
cf push commands.

technically this could be avoided by not using a manifest and running:
```cf push --no-start
cf set-env ...
cf set-env ...
cf set-env ...
...
`

but that escalates quickly if the APP contains many secret things in it's
env requirements.

We face this in an errand where:
- the bosh release renders the application.yml
- the application.yml contains secrets in the env block
- the errand VM streams it's logs to a logging server

This can be remediated by the suggested redacttion.
`--redact-env` to indiscriminately change all values
to `<redacted>` if they're contained within an apps `env` block.

* duplicate test cases to enable testing redact feature

[#186767925]

IF the app is pushed with a manifest
AND the app needs secrets set via environment variables
THEN currently there is no way to avoid leaking secrets in the output of
cf push commands.

technically this could be avoided by not using a manifest and running:
```cf push --no-start
cf set-env ...
cf set-env ...
cf set-env ...
...
`

but that escalates quickly if the APP contains many secret things in it's
env requirements.

We face this in an errand where:
- the bosh release renders the application.yml
- the application.yml contains secrets in the env block
- the errand VM streams it's logs to a logging server

This can be remediated by the suggested redacttion.
`--redact-env` to indiscriminately change all values
to `<redacted>` if they're contained within an apps `env` block.

* add integration tests for --redact-env flag

[#186767925]
  • Loading branch information
nouseforaname authored Feb 22, 2024
1 parent 5bfdbe7 commit b933a3c
Show file tree
Hide file tree
Showing 6 changed files with 480 additions and 16 deletions.
6 changes: 5 additions & 1 deletion command/v7/apply_manifest_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type ApplyManifestCommand struct {
PathToManifest flag.ManifestPathWithExistenceCheck `short:"f" description:"Path to app manifest"`
Vars []template.VarKV `long:"var" description:"Variable key value pair for variable substitution, (e.g., name=app1); can specify multiple times"`
PathsToVarsFiles []flag.PathWithExistenceCheck `long:"vars-file" description:"Path to a variable substitution file for manifest; can specify multiple times"`
RedactEnv bool `long:"redact-env" description:"Do not print values for environment vars set in the application manifest"`
usage interface{} `usage:"CF_NAME apply-manifest -f APP_MANIFEST_PATH"`
relatedCommands interface{} `related_commands:"create-app, create-app-manifest, push"`

Expand All @@ -33,7 +34,10 @@ type ApplyManifestCommand struct {
func (cmd *ApplyManifestCommand) Setup(config command.Config, ui command.UI) error {
cmd.ManifestLocator = manifestparser.NewLocator()
cmd.ManifestParser = manifestparser.ManifestParser{}
cmd.DiffDisplayer = shared.NewManifestDiffDisplayer(ui)
cmd.DiffDisplayer = &shared.ManifestDiffDisplayer{
UI: ui,
RedactEnv: cmd.RedactEnv,
}

currentDir, err := os.Getwd()
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion command/v7/push_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ type PushCommand struct {
NoWait bool `long:"no-wait" description:"Exit when the first instance of the web process is healthy"`
AppPath flag.PathWithExistenceCheck `long:"path" short:"p" description:"Path to app directory or to a zip file of the contents of the app directory"`
RandomRoute bool `long:"random-route" description:"Create a random route for this app (except when no-route is specified in the manifest)"`
RedactEnv bool `long:"redact-env" description:"Do not print values for environment vars set in the application manifest"`
Stack string `long:"stack" short:"s" description:"Stack to use (a stack is a pre-built file system, including an operating system, that can run apps)"`
StartCommand flag.Command `long:"start-command" short:"c" description:"Startup command, set to null to reset to default start command"`
Strategy flag.DeploymentStrategy `long:"strategy" description:"Deployment strategy, either rolling or null."`
Expand Down Expand Up @@ -140,7 +141,7 @@ func (cmd *PushCommand) Setup(config command.Config, ui command.UI) error {

cmd.ManifestLocator = manifestparser.NewLocator()
cmd.ManifestParser = manifestparser.ManifestParser{}
cmd.DiffDisplayer = shared.NewManifestDiffDisplayer(ui)
cmd.DiffDisplayer = &shared.ManifestDiffDisplayer{UI: ui, RedactEnv: cmd.RedactEnv}

return err
}
Expand Down
89 changes: 78 additions & 11 deletions command/v7/shared/manifest_diff_displayer.go
Original file line number Diff line number Diff line change
@@ -1,29 +1,64 @@
package shared

import (
"errors"
"path"
"reflect"
"strconv"
"strings"

"code.cloudfoundry.org/cli/command"
"code.cloudfoundry.org/cli/resources"
"github.com/pkg/errors"
"gopkg.in/yaml.v2"
)

var redacted = "<redacted>"

type ManifestDiffDisplayer struct {
UI command.UI
UI command.UI
RedactEnv bool
}

func NewManifestDiffDisplayer(ui command.UI) *ManifestDiffDisplayer {
return &ManifestDiffDisplayer{
UI: ui,
func redactEnvVarsInRawManifest(rawManifest []byte) ([]byte, error) {
var manifestDataStructure struct {
Version int `yaml:"version,omitempty"`
Applications []yaml.MapSlice `yaml:"applications"`
}
}

if err := yaml.Unmarshal(rawManifest, &manifestDataStructure); err != nil {
return []byte{}, err
}
for appListIndex, application := range manifestDataStructure.Applications {
for appDataIndex, appDataKV := range application {
if appDataKV.Key == "env" {
envMapSlice := appDataKV.Value.(yaml.MapSlice)
for envKeyIndex := range envMapSlice {
envMapSlice[envKeyIndex].Value = redacted
}
appDataKV.Value = envMapSlice
application[appDataIndex] = appDataKV
manifestDataStructure.Applications[appListIndex] = application
}
}
}
return yaml.Marshal(manifestDataStructure)
}
func (display *ManifestDiffDisplayer) DisplayDiff(rawManifest []byte, diff resources.ManifestDiff) error {
// If there are no diffs, just print the manifest

// Redact RAW manifest envVarValues in all apps
// The below could probably be achieved easier if we stop trying to avoid changing map element ordering..
if display.RedactEnv {
var err error
rawManifest, err = redactEnvVarsInRawManifest(rawManifest)
if err != nil {
return errors.New("unable to process manifest diff because its format is invalid")
}
}

// Always display the yaml header line
display.UI.DisplayDiffUnchanged("---", 0, false)

// If there are no diffs, just print the (redacted) manifest
if len(diff.Diffs) == 0 {
display.UI.DisplayDiffUnchanged(string(rawManifest), 0, false)
return nil
Expand All @@ -33,14 +68,15 @@ func (display *ManifestDiffDisplayer) DisplayDiff(rawManifest []byte, diff resou
var yamlManifest yaml.MapSlice
err := yaml.Unmarshal(rawManifest, &yamlManifest)
if err != nil {
return errors.New("Unable to process manifest diff because its format is invalid.")
return errors.New("unable to process manifest diff because its format is invalid")
}

// Distinguish between add/remove and replace diffs
// Replace diffs have the key in the given manifest so we can navigate to that path and display the changed value
// Add/Remove diffs will not, so we need to know their parent's path to insert the diff as a child
pathAddReplaceMap := make(map[string]resources.Diff)
pathRemoveMap := make(map[string]resources.Diff)

for _, diff := range diff.Diffs {
switch diff.Op {
case resources.AddOperation, resources.ReplaceOperation:
Expand All @@ -50,9 +86,6 @@ func (display *ManifestDiffDisplayer) DisplayDiff(rawManifest []byte, diff resou
}
}

// Always display the yaml header line
display.UI.DisplayDiffUnchanged("---", 0, false)

// For each entry in the provided manifest, process any diffs at or below that entry
for _, entry := range yamlManifest {
display.processDiffsRecursively("/"+interfaceToString(entry.Key), entry.Value, 0, &pathAddReplaceMap, &pathRemoveMap, false)
Expand Down Expand Up @@ -119,14 +152,48 @@ func (display *ManifestDiffDisplayer) processDiffsRecursively(
// Otherwise, print the unchanged field and value
display.UI.DisplayDiffUnchanged(formatKeyValue(field, value), depth, addHyphen)
}
func redactDiff(diff resources.Diff) resources.Diff {
// there are 3 possible ways that a diff can contain env data
// - the diff applies to a path /applications/0/env/key that identifies a changed KV pair in the env object
// in that case we want to change the diff.Value && diff.Was to ensure that no env item is printed
// - the diff applies to a path /applications/0/env that identifies `env` itself, all subvalues must be replaced
// in that case we want to change the diff.Value && diff.Was to ensure that no env item is printed
// - the diff applies to a path /applications/0 that identifies the parent element that contains `env` itself, all subvalues in `env` must be replaced

isEnvKV := strings.HasSuffix(path.Dir(diff.Path), "env")
isEnvMap := strings.HasSuffix(diff.Path, "env")
fullMap, containsMap := diff.Value.(map[string]interface{})
if isEnvKV {
diff.Value = redacted
diff.Was = redacted
}
if isEnvMap {
cleanMap := diff.Value.(map[string]interface{})
for index := range cleanMap {
cleanMap[index] = redacted
}
diff.Value = cleanMap
} else if envMap, containsEnvInMap := fullMap["env"].(map[string]interface{}); containsMap && containsEnvInMap {
for index := range envMap {
envMap[index] = redacted
}
fullMap["env"] = envMap
diff.Value = fullMap
}
return diff
}

func (display *ManifestDiffDisplayer) formatDiff(field string, diff resources.Diff, depth int, addHyphen bool) {
if display.RedactEnv {
diff = redactDiff(diff)
}
addHyphen = isInt(field) || addHyphen
switch diff.Op {
case resources.AddOperation:
display.UI.DisplayDiffAddition(formatKeyValue(field, diff.Value), depth, addHyphen)

case resources.ReplaceOperation:

display.UI.DisplayDiffRemoval(formatKeyValue(field, diff.Was), depth, addHyphen)
display.UI.DisplayDiffAddition(formatKeyValue(field, diff.Value), depth, addHyphen)

Expand Down
Loading

0 comments on commit b933a3c

Please sign in to comment.