From 18cf1cc608d45d8685e46bc1b0f586414663650c Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Wed, 3 Dec 2025 16:12:16 -0500 Subject: [PATCH 1/2] Log final LCOW OCI spec; always call go in Makefile Use an `io.MultiWriter` to capture the OCI spec as written to the `config.json` file that is provided to runc so it can be logged, and avoid needing to marshal the spec twice. Also, add a `FORCE` target to the Makefile to always run the `go build` recipe for Go binaries. This fixes a bug where make will not rebuild the binaries and instead `make clean` must be called to remove them before building. See: www.gnu.org/software/make/manual/html_node/Force-Targets.html Signed-off-by: Hamza El-Saawy --- Makefile | 9 +++++- internal/guest/runtime/hcsv2/uvm.go | 50 ++++++++++++++++++++++------- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index cc44ba8ae5..0c46422266 100644 --- a/Makefile +++ b/Makefile @@ -106,10 +106,17 @@ out/delta.tar.gz: bin/init bin/vsockexec bin/cmd/gcs bin/cmd/gcstools bin/cmd/ho tar -zcf $@ -C rootfs . rm -rf rootfs -bin/cmd/gcs bin/cmd/gcstools bin/cmd/hooks/wait-paths bin/cmd/tar2ext4 bin/internal/tools/snp-report: +# Use force target to always call `go build` per make invocation and rely on Go's build cache +# to decide if binaries should be (re)built. +# Note: don't use `.PHONY` since the targets are actual files. +# +# www.gnu.org/software/make/manual/html_node/Force-Targets.html +bin/cmd/gcs bin/cmd/gcstools bin/cmd/hooks/wait-paths bin/cmd/tar2ext4 bin/internal/tools/snp-report: FORCE @mkdir -p $(dir $@) GOOS=linux $(GO_BUILD) -o $@ $(SRCROOT)/$(@:bin/%=%) +FORCE: + bin/vsockexec: vsockexec/vsockexec.o vsockexec/vsock.o @mkdir -p bin $(CC) $(LDFLAGS) -o $@ $^ diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index 69cfedb68f..4af3719385 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -5,6 +5,7 @@ package hcsv2 import ( "bufio" + "bytes" "context" "encoding/json" "fmt" @@ -643,18 +644,9 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM if err := os.MkdirAll(settings.OCIBundlePath, 0700); err != nil { return nil, errors.Wrapf(err, "failed to create OCIBundlePath: '%s'", settings.OCIBundlePath) } - configFile := path.Join(settings.OCIBundlePath, "config.json") - f, err := os.Create(configFile) - if err != nil { - return nil, errors.Wrapf(err, "failed to create config.json at: '%s'", configFile) - } - defer f.Close() - writer := bufio.NewWriter(f) - if err := json.NewEncoder(writer).Encode(settings.OCISpecification); err != nil { - return nil, errors.Wrapf(err, "failed to write OCISpecification to config.json at: '%s'", configFile) - } - if err := writer.Flush(); err != nil { - return nil, errors.Wrapf(err, "failed to flush writer for config.json at: '%s'", configFile) + + if err := writeSpecToFile(ctx, path.Join(settings.OCIBundlePath, "config.json"), settings.OCISpecification); err != nil { + return nil, err } con, err := h.rtime.CreateContainer(id, settings.OCIBundlePath, nil) @@ -691,6 +683,40 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM return c, nil } +func writeSpecToFile(ctx context.Context, configFile string, spec *specs.Spec) error { + f, err := os.Create(configFile) + if err != nil { + return errors.Wrapf(err, "failed to create config.json at: '%s'", configFile) + } + defer f.Close() + + writer := bufio.NewWriter(f) + // capture what we write to the config file in a byte buffer so we can log it later + var w io.Writer = writer + buf := &bytes.Buffer{} + if logrus.IsLevelEnabled(logrus.TraceLevel) { + w = io.MultiWriter(writer, buf) + } + + enc := json.NewEncoder(w) + enc.SetEscapeHTML(false) // not embedding JSON into HTML, so no need to escape + if err := enc.Encode(spec); err != nil { + return errors.Wrapf(err, "failed to write OCISpecification to config.json at: '%s'", configFile) + } + if err := writer.Flush(); err != nil { + return errors.Wrapf(err, "failed to flush writer for config.json at: '%s'", configFile) + } + + if logrus.IsLevelEnabled(logrus.TraceLevel) { + log.G(ctx).WithFields(logrus.Fields{ + logfields.Path: configFile, + "config": strings.TrimSpace(buf.String()), + }).Trace("wrote OCI spec to config.json file") + } + + return nil +} + func (h *Host) modifyHostSettings(ctx context.Context, containerID string, req *guestrequest.ModificationRequest) (retErr error) { switch req.ResourceType { case guestresource.ResourceTypeSCSIDevice: From b6818d4aa089c969fc156bbbb4afa97e6b97350f Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Thu, 4 Dec 2025 11:50:13 -0500 Subject: [PATCH 2/2] PR: scrub logged OCI spec Signed-off-by: Hamza El-Saawy --- internal/guest/runtime/hcsv2/uvm.go | 13 ++++++--- internal/log/scrub.go | 43 +++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index 4af3719385..5737f861d6 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -708,10 +708,15 @@ func writeSpecToFile(ctx context.Context, configFile string, spec *specs.Spec) e } if logrus.IsLevelEnabled(logrus.TraceLevel) { - log.G(ctx).WithFields(logrus.Fields{ - logfields.Path: configFile, - "config": strings.TrimSpace(buf.String()), - }).Trace("wrote OCI spec to config.json file") + entry := log.G(ctx).WithField(logfields.Path, configFile) + + if b, err := log.ScrubOCISpec(buf.Bytes()); err != nil { + entry.WithError(err).Warning("could not scrub OCI spec written to config.json") + } else { + log.G(ctx).WithField( + "config", string(bytes.TrimSpace(b)), + ).Trace("wrote OCI spec to config.json") + } } return nil diff --git a/internal/log/scrub.go b/internal/log/scrub.go index 5346f9b7cf..83ee97f853 100644 --- a/internal/log/scrub.go +++ b/internal/log/scrub.go @@ -11,7 +11,7 @@ import ( // This package scrubs objects of potentially sensitive information to pass to logging -type genMap = map[string]interface{} +type genMap = map[string]any type scrubberFunc func(genMap) error const _scrubbedReplacement = "" @@ -20,7 +20,11 @@ var ( ErrUnknownType = errors.New("encoded object is of unknown type") // case sensitive keywords, so "env" is not a substring on "Environment" - _scrubKeywords = [][]byte{[]byte("env"), []byte("Environment")} + _scrubKeywords = [][]byte{ + []byte("env"), + []byte("Environment"), + []byte("annotations"), + } _scrub atomic.Bool ) @@ -32,7 +36,7 @@ func SetScrubbing(enable bool) { _scrub.Store(enable) } func IsScrubbingEnabled() bool { return _scrub.Load() } // ScrubProcessParameters scrubs HCS Create Process requests with config parameters of -// type internal/hcs/schema2.ScrubProcessParameters (aka hcsshema.ScrubProcessParameters) +// type [hcsschema.ProcessParameters]. func ScrubProcessParameters(s string) (string, error) { // todo: deal with v1 ProcessConfig b := []byte(s) @@ -81,19 +85,34 @@ func scrubBridgeCreate(m genMap) error { func scrubLinuxHostedSystem(m genMap) error { if m, ok := index(m, "OciSpecification"); ok { //nolint:govet // shadow - if _, ok := m["annotations"]; ok { - m["annotations"] = map[string]string{_scrubbedReplacement: _scrubbedReplacement} - } - if m, ok := index(m, "process"); ok { //nolint:govet // shadow - if _, ok := m["env"]; ok { - m["env"] = []string{_scrubbedReplacement} - return nil - } - } + return scrubOCISpec(m) } return ErrUnknownType } +// ScrubOCISpec scrubs a JSON encoded [github.com/opencontainers/runtime-spec/specs-go.Spec]. +// +// Ideally the spec struct would be scrubbed directly, but that would need a deep clone to +// prevent modifying the original, and, absent one implemented on the Spec +// (e.g., [google.golang.org/protobuf/proto.CloneOf]), unmarshalling a marshalled struct +// functions as a deep clone. +func ScrubOCISpec(b []byte) ([]byte, error) { + return scrubBytes(b, scrubOCISpec) +} + +func scrubOCISpec(m genMap) error { + if _, ok := m["annotations"]; ok { + m["annotations"] = map[string]string{_scrubbedReplacement: _scrubbedReplacement} + } + if m, ok := index(m, "process"); ok { //nolint:govet // shadow + if _, ok := m["env"]; ok { + m["env"] = []string{_scrubbedReplacement} + } + } + + return nil +} + // ScrubBridgeExecProcess scrubs requests sent over the bridge of type // internal/gcs/protocol.containerExecuteProcess func ScrubBridgeExecProcess(b []byte) ([]byte, error) {