Skip to content

Enable log scrubbing by default#2725

Open
ryankeithster wants to merge 2 commits intomicrosoft:mainfrom
ryankeithster:msrc107671-scrublogs-default
Open

Enable log scrubbing by default#2725
ryankeithster wants to merge 2 commits intomicrosoft:mainfrom
ryankeithster:msrc107671-scrublogs-default

Conversation

@ryankeithster
Copy link
Copy Markdown

Summary

Addresses security concerns related to the logging of environment variables, which could potentially contain sensitive information, in cleartext to ETW traces and log files by enabling scrubbing of this information from the logs by default.

Changes

Default scrubbing ON (internal/log/scrub.go)

  • Added init() that sets scrubbing enabled by default, eliminating the need for explicit opt-in via config
  • Added ScrubCreateOptions() to scrub the OCI spec (env vars + annotations) within CreateOptions logged during container creation

Proto: three-state ScrubLogs field (cmd/containerd-shim-runhcs-v1/options/runhcs.proto)

  • Changed bool scrub_logs = 20optional bool scrub_logs = 20
  • Enables distinguishing "not set" (nil → default scrub ON) from "explicitly false" (opt-out)
  • Backward compatible: old containerd omits zero-value bools on the wire → new shim sees nil → scrubbing stays ON

Shim consumers updated for *bool semantics

  • cmd/containerd-shim-runhcs-v1/serve.go: Only disables scrubbing if ScrubLogs is explicitly *false
  • cmd/containerd-shim-lcow-v2/main.go: Same pattern
  • internal/builder/vm/lcow/kernel_args.go: Passes -scrub-logs=false to GCS only if explicitly disabled; otherwise always passes -scrub-logs
  • cmd/gcs/main.go: Changed --scrub-logs flag default from false to true

New scrub coverage (internal/hcsoci/create.go, internal/log/format.go)

  • initializeCreateOptions now scrubs env vars and annotations before logging via new FormatScrub helper
  • Previously logged the full OCI spec in cleartext at debug level

@ryankeithster ryankeithster requested a review from a team as a code owner May 6, 2026 22:50
@ryankeithster
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Microsoft"

Ryan Keith (from Dev Box) added 2 commits May 6, 2026 17:17
Signed-off-by: Ryan Keith (from Dev Box) <ryankeith@microsoft.com>
Signed-off-by: Ryan Keith (from Dev Box) <ryankeith@microsoft.com>
@ryankeithster ryankeithster force-pushed the msrc107671-scrublogs-default branch from c60931d to 7cd244c Compare May 7, 2026 00:18
Copy link
Copy Markdown
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits with log package change, but lgtm overall

Comment thread internal/hcsoci/create.go
Comment on lines 131 to 134
log.G(ctx).WithFields(logrus.Fields{
"options": log.Format(ctx, createOptions),
"options": log.FormatScrub(ctx, createOptions, log.ScrubCreateOptions),
"schema": log.Format(ctx, coi.actualSchemaVersion),
}).Debug("hcsshim::initializeCreateOptions")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could probably get away with:

Suggested change
log.G(ctx).WithFields(logrus.Fields{
"options": log.Format(ctx, createOptions),
"options": log.FormatScrub(ctx, createOptions, log.ScrubCreateOptions),
"schema": log.Format(ctx, coi.actualSchemaVersion),
}).Debug("hcsshim::initializeCreateOptions")
if logrus.IsLevelEnabled(logrus.DebugLevel) {
if b, err := log.ScrubCreateOptions([]byte(log.Format(ctx, createOptions))); err != nil {
log.G(ctx).WithError(err).Warning("could not scrub CreateOptions")
} else {
log.G(ctx).WithFields(logrus.Fields{
"options": string(b)
"schema": log.Format(ctx, coi.actualSchemaVersion),
}).Debug("hcsshim::initializeCreateOptions")
}
}

this way, we don't need FormatScrub

Comment thread internal/log/format.go
Comment on lines +81 to +101
func FormatScrub(ctx context.Context, v interface{}, scrub func([]byte) ([]byte, error)) string {
b, err := encode(v)
if err != nil {
G(ctx).WithFields(logrus.Fields{
logrus.ErrorKey: err,
"type": fmt.Sprintf("%T", v),
}).Debug("could not format value")
return ""
}

b, err = scrub(b)
if err != nil {
G(ctx).WithFields(logrus.Fields{
logrus.ErrorKey: err,
"type": fmt.Sprintf("%T", v),
}).Debug("could not scrub value")
return ""
}

return string(b)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just be (though, as mentioned above comment, we can probably get away without this):

Suggested change
func FormatScrub(ctx context.Context, v interface{}, scrub func([]byte) ([]byte, error)) string {
b, err := encode(v)
if err != nil {
G(ctx).WithFields(logrus.Fields{
logrus.ErrorKey: err,
"type": fmt.Sprintf("%T", v),
}).Debug("could not format value")
return ""
}
b, err = scrub(b)
if err != nil {
G(ctx).WithFields(logrus.Fields{
logrus.ErrorKey: err,
"type": fmt.Sprintf("%T", v),
}).Debug("could not scrub value")
return ""
}
return string(b)
}
func FormatScrub(ctx context.Context, v any, scrub func([]byte) ([]byte, error)) string {
s := Format(ctx, v)
if s == "" {
return ""
}
b, err := scrub([]byte(s))
if err != nil {
G(ctx).WithFields(logrus.Fields{
logrus.ErrorKey: err,
"type": fmt.Sprintf("%T", v),
}).Debug("could not scrub value")
}
return string(b)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants