Skip to content

Commit dd9d050

Browse files
add redact option for env block when pushing an app with a manifest.yml (#2763)
* 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. * 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. * add integration tests for --redact-env flag [#186767925]
1 parent 807dfad commit dd9d050

File tree

6 files changed

+480
-16
lines changed

6 files changed

+480
-16
lines changed

command/v7/apply_manifest_command.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type ApplyManifestCommand struct {
2020
PathToManifest flag.ManifestPathWithExistenceCheck `short:"f" description:"Path to app manifest"`
2121
Vars []template.VarKV `long:"var" description:"Variable key value pair for variable substitution, (e.g., name=app1); can specify multiple times"`
2222
PathsToVarsFiles []flag.PathWithExistenceCheck `long:"vars-file" description:"Path to a variable substitution file for manifest; can specify multiple times"`
23+
RedactEnv bool `long:"redact-env" description:"Do not print values for environment vars set in the application manifest"`
2324
usage interface{} `usage:"CF_NAME apply-manifest -f APP_MANIFEST_PATH"`
2425
relatedCommands interface{} `related_commands:"create-app, create-app-manifest, push"`
2526

@@ -33,7 +34,10 @@ type ApplyManifestCommand struct {
3334
func (cmd *ApplyManifestCommand) Setup(config command.Config, ui command.UI) error {
3435
cmd.ManifestLocator = manifestparser.NewLocator()
3536
cmd.ManifestParser = manifestparser.ManifestParser{}
36-
cmd.DiffDisplayer = shared.NewManifestDiffDisplayer(ui)
37+
cmd.DiffDisplayer = &shared.ManifestDiffDisplayer{
38+
UI: ui,
39+
RedactEnv: cmd.RedactEnv,
40+
}
3741

3842
currentDir, err := os.Getwd()
3943
if err != nil {

command/v7/push_command.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ type PushCommand struct {
9797
NoWait bool `long:"no-wait" description:"Exit when the first instance of the web process is healthy"`
9898
AppPath flag.PathWithExistenceCheck `long:"path" short:"p" description:"Path to app directory or to a zip file of the contents of the app directory"`
9999
RandomRoute bool `long:"random-route" description:"Create a random route for this app (except when no-route is specified in the manifest)"`
100+
RedactEnv bool `long:"redact-env" description:"Do not print values for environment vars set in the application manifest"`
100101
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)"`
101102
StartCommand flag.Command `long:"start-command" short:"c" description:"Startup command, set to null to reset to default start command"`
102103
Strategy flag.DeploymentStrategy `long:"strategy" description:"Deployment strategy, either rolling or null."`
@@ -140,7 +141,7 @@ func (cmd *PushCommand) Setup(config command.Config, ui command.UI) error {
140141

141142
cmd.ManifestLocator = manifestparser.NewLocator()
142143
cmd.ManifestParser = manifestparser.ManifestParser{}
143-
cmd.DiffDisplayer = shared.NewManifestDiffDisplayer(ui)
144+
cmd.DiffDisplayer = &shared.ManifestDiffDisplayer{UI: ui, RedactEnv: cmd.RedactEnv}
144145

145146
return err
146147
}

command/v7/shared/manifest_diff_displayer.go

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,64 @@
11
package shared
22

33
import (
4+
"errors"
45
"path"
56
"reflect"
67
"strconv"
78
"strings"
89

910
"code.cloudfoundry.org/cli/command"
1011
"code.cloudfoundry.org/cli/resources"
11-
"github.com/pkg/errors"
1212
"gopkg.in/yaml.v2"
1313
)
1414

15+
var redacted = "<redacted>"
16+
1517
type ManifestDiffDisplayer struct {
16-
UI command.UI
18+
UI command.UI
19+
RedactEnv bool
1720
}
1821

19-
func NewManifestDiffDisplayer(ui command.UI) *ManifestDiffDisplayer {
20-
return &ManifestDiffDisplayer{
21-
UI: ui,
22+
func redactEnvVarsInRawManifest(rawManifest []byte) ([]byte, error) {
23+
var manifestDataStructure struct {
24+
Version int `yaml:"version,omitempty"`
25+
Applications []yaml.MapSlice `yaml:"applications"`
2226
}
23-
}
2427

28+
if err := yaml.Unmarshal(rawManifest, &manifestDataStructure); err != nil {
29+
return []byte{}, err
30+
}
31+
for appListIndex, application := range manifestDataStructure.Applications {
32+
for appDataIndex, appDataKV := range application {
33+
if appDataKV.Key == "env" {
34+
envMapSlice := appDataKV.Value.(yaml.MapSlice)
35+
for envKeyIndex := range envMapSlice {
36+
envMapSlice[envKeyIndex].Value = redacted
37+
}
38+
appDataKV.Value = envMapSlice
39+
application[appDataIndex] = appDataKV
40+
manifestDataStructure.Applications[appListIndex] = application
41+
}
42+
}
43+
}
44+
return yaml.Marshal(manifestDataStructure)
45+
}
2546
func (display *ManifestDiffDisplayer) DisplayDiff(rawManifest []byte, diff resources.ManifestDiff) error {
26-
// If there are no diffs, just print the manifest
47+
48+
// Redact RAW manifest envVarValues in all apps
49+
// The below could probably be achieved easier if we stop trying to avoid changing map element ordering..
50+
if display.RedactEnv {
51+
var err error
52+
rawManifest, err = redactEnvVarsInRawManifest(rawManifest)
53+
if err != nil {
54+
return errors.New("unable to process manifest diff because its format is invalid")
55+
}
56+
}
57+
58+
// Always display the yaml header line
59+
display.UI.DisplayDiffUnchanged("---", 0, false)
60+
61+
// If there are no diffs, just print the (redacted) manifest
2762
if len(diff.Diffs) == 0 {
2863
display.UI.DisplayDiffUnchanged(string(rawManifest), 0, false)
2964
return nil
@@ -33,14 +68,15 @@ func (display *ManifestDiffDisplayer) DisplayDiff(rawManifest []byte, diff resou
3368
var yamlManifest yaml.MapSlice
3469
err := yaml.Unmarshal(rawManifest, &yamlManifest)
3570
if err != nil {
36-
return errors.New("Unable to process manifest diff because its format is invalid.")
71+
return errors.New("unable to process manifest diff because its format is invalid")
3772
}
3873

3974
// Distinguish between add/remove and replace diffs
4075
// Replace diffs have the key in the given manifest so we can navigate to that path and display the changed value
4176
// Add/Remove diffs will not, so we need to know their parent's path to insert the diff as a child
4277
pathAddReplaceMap := make(map[string]resources.Diff)
4378
pathRemoveMap := make(map[string]resources.Diff)
79+
4480
for _, diff := range diff.Diffs {
4581
switch diff.Op {
4682
case resources.AddOperation, resources.ReplaceOperation:
@@ -50,9 +86,6 @@ func (display *ManifestDiffDisplayer) DisplayDiff(rawManifest []byte, diff resou
5086
}
5187
}
5288

53-
// Always display the yaml header line
54-
display.UI.DisplayDiffUnchanged("---", 0, false)
55-
5689
// For each entry in the provided manifest, process any diffs at or below that entry
5790
for _, entry := range yamlManifest {
5891
display.processDiffsRecursively("/"+interfaceToString(entry.Key), entry.Value, 0, &pathAddReplaceMap, &pathRemoveMap, false)
@@ -119,14 +152,48 @@ func (display *ManifestDiffDisplayer) processDiffsRecursively(
119152
// Otherwise, print the unchanged field and value
120153
display.UI.DisplayDiffUnchanged(formatKeyValue(field, value), depth, addHyphen)
121154
}
155+
func redactDiff(diff resources.Diff) resources.Diff {
156+
// there are 3 possible ways that a diff can contain env data
157+
// - the diff applies to a path /applications/0/env/key that identifies a changed KV pair in the env object
158+
// in that case we want to change the diff.Value && diff.Was to ensure that no env item is printed
159+
// - the diff applies to a path /applications/0/env that identifies `env` itself, all subvalues must be replaced
160+
// in that case we want to change the diff.Value && diff.Was to ensure that no env item is printed
161+
// - the diff applies to a path /applications/0 that identifies the parent element that contains `env` itself, all subvalues in `env` must be replaced
162+
163+
isEnvKV := strings.HasSuffix(path.Dir(diff.Path), "env")
164+
isEnvMap := strings.HasSuffix(diff.Path, "env")
165+
fullMap, containsMap := diff.Value.(map[string]interface{})
166+
if isEnvKV {
167+
diff.Value = redacted
168+
diff.Was = redacted
169+
}
170+
if isEnvMap {
171+
cleanMap := diff.Value.(map[string]interface{})
172+
for index := range cleanMap {
173+
cleanMap[index] = redacted
174+
}
175+
diff.Value = cleanMap
176+
} else if envMap, containsEnvInMap := fullMap["env"].(map[string]interface{}); containsMap && containsEnvInMap {
177+
for index := range envMap {
178+
envMap[index] = redacted
179+
}
180+
fullMap["env"] = envMap
181+
diff.Value = fullMap
182+
}
183+
return diff
184+
}
122185

123186
func (display *ManifestDiffDisplayer) formatDiff(field string, diff resources.Diff, depth int, addHyphen bool) {
187+
if display.RedactEnv {
188+
diff = redactDiff(diff)
189+
}
124190
addHyphen = isInt(field) || addHyphen
125191
switch diff.Op {
126192
case resources.AddOperation:
127193
display.UI.DisplayDiffAddition(formatKeyValue(field, diff.Value), depth, addHyphen)
128194

129195
case resources.ReplaceOperation:
196+
130197
display.UI.DisplayDiffRemoval(formatKeyValue(field, diff.Was), depth, addHyphen)
131198
display.UI.DisplayDiffAddition(formatKeyValue(field, diff.Value), depth, addHyphen)
132199

0 commit comments

Comments
 (0)