Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 28 additions & 20 deletions commands/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,15 @@ func (c *ApplyCommand) Run(args []string) int {
// parsed and rendered, so a template or parse error that interpolated one
// of them is masked. Task-declared sensitive values are added once the
// recipe parses (below).
subprocess.SetGlobalSensitive(sensitiveValues)
defer subprocess.SetGlobalSensitive(nil)
// The masker belongs to this run and goes out of scope with it, so there
// is no teardown: the deferred clear this replaces is exactly what made a
// second run in the same process lose its secrets.
masker := subprocess.NewMasker(sensitiveValues...)
ctx = subprocess.ContextWithMasker(ctx, masker)

plays, err := tasks.GetPlaysWithFormat(data, c.tasksFormat, inputCtx, userSet)
if err != nil {
c.Ui.Error(subprocess.MaskString(fmt.Sprintf("task error: %v", err)))
c.Ui.Error(masker.String(fmt.Sprintf("task error: %v", err)))
return 1
}

Expand All @@ -249,14 +252,14 @@ func (c *ApplyCommand) Run(args []string) int {
// declared inputs the surviving play's when: depends on.
fileLevelKeys := tasks.FileLevelInputNames(plays)

selected, err := filterPlaysByName(plays, c.play)
selected, err := filterPlaysByName(masker, plays, c.play)
if err != nil {
// The hint names every play in the file, so a value any of their tasks
// declares sensitive is in scope for this message - unlike the filtered
// collection below, which deliberately leaves out a play --play
// excluded. Registering the whole file costs nothing here: this branch
// prints one line and returns.
subprocess.AddGlobalSensitive(tasks.CollectPlaySensitiveValues(plays)...)
masker.Add(tasks.CollectPlaySensitiveValues(plays)...)
c.Ui.Error(err.Error())
return 1
}
Expand All @@ -271,13 +274,13 @@ func (c *ApplyCommand) Run(args []string) int {
// play --play excluded from masking output it never appears in. The
// unmatched --play branch above is the one place that collects from the
// whole file, and says why.
subprocess.AddGlobalSensitive(tasks.CollectPlaySensitiveValues(plays)...)
masker.Add(tasks.CollectPlaySensitiveValues(plays)...)

if c.startAtTask != "" {
if !startAtTaskMatches(plays, c.startAtTask) {
c.Ui.Error(subprocess.MaskString(fmt.Sprintf(
c.Ui.Error(masker.String(fmt.Sprintf(
"--start-at-task %q: no task matched name; available names: %s",
c.startAtTask, formatStartAtTaskNames(plays),
c.startAtTask, formatStartAtTaskNames(masker, plays),
)))
return 1
}
Expand All @@ -292,6 +295,7 @@ func (c *ApplyCommand) Run(args []string) int {
userSet: userSet,
context: inputCtx,
jsonOut: c.json,
masker: masker,
target: target,
})
}
Expand All @@ -302,7 +306,7 @@ func (c *ApplyCommand) Run(args []string) int {
// not collide; nothing was closing the extra ones.
defer closeControlMasters(target, plays)

emitter := c.newEmitter()
emitter := c.newEmitter(masker)
start := time.Now()
counts := ApplyCounts{}
playWhenExprCtx := buildEnvelopeExprContext(buildPlayWhenContext(inputCtx, fileLevelKeys, userSet))
Expand Down Expand Up @@ -942,7 +946,7 @@ func startAtTaskMatches(plays []*tasks.Play, target string) bool {
// text that carries the secret escaped twice over - and would miss it (#475).
// Deduplication still keys on the real name: two tasks that mask alike are two
// tasks.
func formatStartAtTaskNames(plays []*tasks.Play) string {
func formatStartAtTaskNames(masker *subprocess.Masker, plays []*tasks.Play) string {
seen := map[string]bool{}
var quoted []string
for _, play := range plays {
Expand All @@ -952,7 +956,7 @@ func formatStartAtTaskNames(plays []*tasks.Play) string {
for _, name := range play.Tasks.Keys() {
if !seen[name] {
seen[name] = true
quoted = append(quoted, fmt.Sprintf("%q", subprocess.MaskString(name)))
quoted = append(quoted, fmt.Sprintf("%q", masker.String(name)))
}
env := play.Tasks.GetEnvelope(name)
for _, descendant := range tasks.CollectEnvelopeNames([]*tasks.TaskEnvelope{env}) {
Expand All @@ -961,7 +965,7 @@ func formatStartAtTaskNames(plays []*tasks.Play) string {
}
if !seen[descendant] {
seen[descendant] = true
quoted = append(quoted, fmt.Sprintf("%q", subprocess.MaskString(descendant)))
quoted = append(quoted, fmt.Sprintf("%q", masker.String(descendant)))
}
}
}
Expand All @@ -980,13 +984,13 @@ func formatStartAtTaskNames(plays []*tasks.Play) string {
// formatStartAtTaskNames gives: `%q` escapes what it wraps, so masking the
// finished message would be matching a registered literal against text that
// carries the secret escaped (#477).
func formatAvailablePlayNames(plays []*tasks.Play) string {
func formatAvailablePlayNames(masker *subprocess.Masker, plays []*tasks.Play) string {
quoted := make([]string, 0, len(plays))
for _, play := range plays {
if play == nil {
continue
}
quoted = append(quoted, fmt.Sprintf("%q", subprocess.MaskString(play.Name)))
quoted = append(quoted, fmt.Sprintf("%q", masker.String(play.Name)))
}
if len(quoted) == 0 {
return "(none)"
Expand All @@ -1002,18 +1006,22 @@ func formatAvailablePlayNames(plays []*tasks.Play) string {
type unknownPlayError struct {
target string
plays []*tasks.Play
// masker is carried on the error because Error() is called from places
// that have neither a context nor an emitter, and the message names both
// the requested play and every available one.
masker *subprocess.Masker
}

func (e *unknownPlayError) Error() string {
return fmt.Sprintf("--play %q: no play with that name; available plays: %s",
subprocess.MaskString(e.target), formatAvailablePlayNames(e.plays))
e.masker.String(e.target), formatAvailablePlayNames(e.masker, e.plays))
}

// filterPlaysByName narrows plays to the single play whose Name matches
// target. An empty target returns plays unchanged. An unmatched target
// returns an error so the user sees a clear "no such play" diagnostic
// rather than silently doing nothing.
func filterPlaysByName(plays []*tasks.Play, target string) ([]*tasks.Play, error) {
func filterPlaysByName(masker *subprocess.Masker, plays []*tasks.Play, target string) ([]*tasks.Play, error) {
if target == "" {
return plays, nil
}
Expand All @@ -1022,16 +1030,16 @@ func filterPlaysByName(plays []*tasks.Play, target string) ([]*tasks.Play, error
return []*tasks.Play{play}, nil
}
}
return nil, &unknownPlayError{target: target, plays: plays}
return nil, &unknownPlayError{target: target, plays: plays, masker: masker}
}

// newEmitter constructs the EventEmitter for this run. --json builds a
// JSONEmitter; otherwise the human Formatter is used. The verbose flag is
// only meaningful for the human path - JSON output already includes the
// resolved commands in each task event's `commands` array.
func (c *ApplyCommand) newEmitter() EventEmitter {
func (c *ApplyCommand) newEmitter(masker *subprocess.Masker) EventEmitter {
if c.json {
return NewJSONEmitter(c.Ui)
return NewJSONEmitter(c.Ui, masker)
}
return NewFormatter(c.Ui, c.verbose)
return NewFormatter(c.Ui, c.verbose, masker)
}
8 changes: 4 additions & 4 deletions commands/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ func TestFilterPlaysByName(t *testing.T) {
{Name: "worker"},
}

out, err := filterPlaysByName(plays, "")
out, err := filterPlaysByName(nil, plays, "")
if err != nil || len(out) != 2 {
t.Errorf("empty target should pass through; got len=%d err=%v", len(out), err)
}

out, err = filterPlaysByName(plays, "api")
out, err = filterPlaysByName(nil, plays, "api")
if err != nil || len(out) != 1 || out[0].Name != "api" {
t.Errorf(`--play "api" got len=%d names=%v err=%v`, len(out), playNames(out), err)
}

_, err = filterPlaysByName(plays, "missing")
_, err = filterPlaysByName(nil, plays, "missing")
if err == nil {
t.Fatal("expected error for unknown play")
}
Expand All @@ -139,7 +139,7 @@ func TestFilterPlaysByName(t *testing.T) {

// An empty recipe has no name to suggest, so the hint says so rather
// than trailing off after the colon.
_, err = filterPlaysByName(nil, "missing")
_, err = filterPlaysByName(nil, nil, "missing")
if err == nil {
t.Fatal("expected error for unknown play in an empty list")
}
Expand Down
11 changes: 5 additions & 6 deletions commands/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,14 @@ func (c *ExportCommand) Run(args []string) int {
// arguments - the --app names and --resource addresses reported missing,
// and the output paths - because a name masked down to *** would hide the
// typo the message exists to report.
subprocess.SetGlobalSensitive(res.SensitiveValues())
defer subprocess.SetGlobalSensitive(nil)
masker := subprocess.NewMasker(res.SensitiveValues()...)

if err != nil {
c.Ui.Error(fmt.Sprintf("export failed: %v", subprocess.MaskString(err.Error())))
c.Ui.Error(fmt.Sprintf("export failed: %v", masker.String(err.Error())))
return 1
}
for _, w := range res.Report.Warnings {
c.Ui.Warn(fmt.Sprintf("warning: %s", subprocess.MaskString(w)))
c.Ui.Warn(fmt.Sprintf("warning: %s", masker.String(w)))
}

// A nonexistent --app must not silently produce an empty recipe (which the
Expand All @@ -249,7 +248,7 @@ func (c *ExportCommand) Run(args []string) int {

recipeBytes, err := res.MarshalRecipe(recipeFormat)
if err != nil {
c.Ui.Error(fmt.Sprintf("marshal recipe: %v", subprocess.MaskString(err.Error())))
c.Ui.Error(fmt.Sprintf("marshal recipe: %v", masker.String(err.Error())))
return 1
}

Expand Down Expand Up @@ -310,7 +309,7 @@ func (c *ExportCommand) Run(args []string) int {
if writeVars {
varsBytes, err := res.MarshalVars(varsFormat)
if err != nil {
c.Ui.Error(fmt.Sprintf("marshal vars: %v", subprocess.MaskString(err.Error())))
c.Ui.Error(fmt.Sprintf("marshal vars: %v", masker.String(err.Error())))
return 1
}
if err := c.writeVarsFile(varsOutput, varsBytes); err != nil {
Expand Down
5 changes: 0 additions & 5 deletions commands/export_masking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func failingExecRunner(responses map[string]string, failing string, err error) f
// deliberate: it pins that masking happens when the warning is printed, after
// the whole read, rather than when it is appended.
func TestExportCommandWarningMasksAConfigValue(t *testing.T) {
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
defer subprocess.SetExecRunner(failingExecRunner(
exportCommandFixture(),
"--quiet apps:locked web",
Expand Down Expand Up @@ -70,7 +69,6 @@ func TestExportCommandWarningMasksAConfigValue(t *testing.T) {
// placeholder there, so the vars map holds nothing to mask with - while the
// real value was still read off the server and is still in the warning.
func TestExportCommandRedactWarningMasksAConfigValue(t *testing.T) {
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
defer subprocess.SetExecRunner(failingExecRunner(
exportCommandFixture(),
"--quiet apps:locked web",
Expand Down Expand Up @@ -101,7 +99,6 @@ func TestExportCommandRedactWarningMasksAConfigValue(t *testing.T) {
// exported before apps:list runs, so by the time the failure is printed the
// export is already holding the cluster token it read.
func TestExportCommandFailureMasksASecretReadBeforeTheAppList(t *testing.T) {
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
defer subprocess.SetExecRunner(failingExecRunner(
map[string]string{
"--quiet scheduler-k3s:report --global --format json": `{"global-token":"s3cr3ttoken"}`,
Expand Down Expand Up @@ -134,7 +131,6 @@ func TestExportCommandFailureMasksASecretReadBeforeTheAppList(t *testing.T) {
// masked Ui, so an export whose every config value is registered still writes
// a vars-file the operator can apply.
func TestExportCommandMaskingLeavesTheVarsFileInTheClear(t *testing.T) {
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
defer subprocess.SetExecRunner(fakeExecRunner(exportCommandFixture()))()
dir := t.TempDir()
t.Chdir(dir)
Expand All @@ -161,7 +157,6 @@ func TestExportCommandMaskingLeavesTheVarsFileInTheClear(t *testing.T) {
// point at the typo - which "*** not found on server" would not do. It stays
// unmasked even when it collides with a value the export registered.
func TestExportCommandMissingAppNameStaysReadable(t *testing.T) {
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
responses := exportCommandFixture()
responses["--quiet config:export --format json web"] = `{"API_KEY":"nope-app"}`
defer subprocess.SetExecRunner(fakeExecRunner(responses))()
Expand Down
23 changes: 14 additions & 9 deletions commands/list_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ type listTasksOptions struct {
userSet map[string]bool
context map[string]interface{}
jsonOut bool
// masker holds the run's sensitive values; the rendered plan echoes task
// and play names, which are exactly where a secret reaches output.
masker *subprocess.Masker
// target is the run-wide target the plays resolve against, so the
// rendered plan can say which server each play would talk to. The JSON
// stream is deliberately left alone: it has no play_start event to hang a
Expand Down Expand Up @@ -75,17 +78,17 @@ func renderListTasks(ui cli.Ui, opts listTasksOptions) int {
// per value rather than per output site is what keeps the two paths
// from drifting apart, and it leaves docket's own decorations - the
// `==> Play: ` prefix, the markers, `(group)` - untouched.
playName := subprocess.MaskString(play.Name)
playName := opts.masker.String(play.Name)
if play.HasWhen() {
whenSrc := subprocess.MaskString(play.When)
whenSrc := opts.masker.String(play.When)
playCtx := buildEnvelopeExprContext(buildPlayWhenContext(opts.context, opts.fileLevelKeys, opts.userSet))
ok, err := tasks.EvalBool(play.WhenProgram(), playCtx)
if err != nil {
whenError = true
// An expr runtime error quotes the predicate's own source
// back in its snippet, so the formatted error - not just the
// play name - carries whatever the predicate interpolated.
reason := subprocess.MaskString(fmt.Sprintf("when error: %v", err))
reason := opts.masker.String(fmt.Sprintf("when error: %v", err))
if opts.jsonOut {
emitListJSON(ui, map[string]interface{}{
"type": "play_skipped",
Expand Down Expand Up @@ -122,6 +125,7 @@ func renderListTasks(ui cli.Ui, opts listTasksOptions) int {
}

rc := listRenderContext{
masker: opts.masker,
ui: ui,
playName: playName,
playExprCtx: buildEnvelopeExprContext(tasks.BuildPerPlayContext(opts.context, play.Inputs, opts.userSet)),
Expand Down Expand Up @@ -151,6 +155,7 @@ type listRenderContext struct {
playName string
playExprCtx map[string]interface{}
jsonOut bool
masker *subprocess.Masker
}

// renderListEnvelope renders one envelope's line and, for a group,
Expand Down Expand Up @@ -189,16 +194,16 @@ func renderListEnvelope(
// `phase` are deliberately absent - they are docket's own vocabulary,
// pinned as enums in docs/schemas/list-tasks-v1.schema.json, and masking
// one would emit a stream that fails its own schema.
display := subprocess.MaskString(env.Name)
whenSrc := subprocess.MaskString(env.When)
tags := maskedStrings(env.Tags)
display := rc.masker.String(env.Name)
whenSrc := rc.masker.String(env.When)
tags := maskedStrings(rc.masker, env.Tags)
deprecation := ""
caveat := ""
var probe tasks.ProbeSupport
if env != nil && env.Task != nil {
deprecation = subprocess.MaskString(tasks.TaskDeprecation(env.Task))
deprecation = rc.masker.String(tasks.TaskDeprecation(env.Task))
probe, _ = tasks.TaskProbeSupport(env.Task)
caveat = subprocess.MaskString(probe.Caveat)
caveat = rc.masker.String(probe.Caveat)
}

if rc.jsonOut {
Expand Down Expand Up @@ -239,7 +244,7 @@ func renderListEnvelope(
if env.IsLoopExpansion {
ev["loop_index"] = env.LoopIndex
if env.LoopItem != nil {
ev["loop_item"] = subprocess.MaskValue(env.LoopItem)
ev["loop_item"] = rc.masker.Value(env.LoopItem)
}
}
emitListJSON(rc.ui, ev)
Expand Down
10 changes: 4 additions & 6 deletions commands/masking_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,10 @@ func TestPlanMasksSensitiveInputInParseError(t *testing.T) {
}

func TestValidateMasksSensitiveInJSONProblem(t *testing.T) {
subprocess.SetGlobalSensitive([]string{"tok_secret"})
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
masker := subprocess.NewMasker("tok_secret")

ui := cli.NewMockUi()
c := &ValidateCommand{Meta: command.Meta{Ui: ui}}
c := &ValidateCommand{Meta: command.Meta{Ui: ui}, masker: masker}
c.emitJSONProblem(tasks.Problem{
Code: "template_error",
Message: `cannot render "tok_secret"`,
Expand All @@ -74,11 +73,10 @@ func TestValidateMasksSensitiveInJSONProblem(t *testing.T) {
}

func TestValidateMasksSensitiveInHumanProblem(t *testing.T) {
subprocess.SetGlobalSensitive([]string{"tok_secret"})
t.Cleanup(func() { subprocess.SetGlobalSensitive(nil) })
masker := subprocess.NewMasker("tok_secret")

ui := cli.NewMockUi()
c := &ValidateCommand{Meta: command.Meta{Ui: ui}}
c := &ValidateCommand{Meta: command.Meta{Ui: ui}, masker: masker}
c.renderHumanProblems([]tasks.Problem{{
Play: "play tok_secret",
Task: "task tok_secret",
Expand Down
Loading