From df3f26d7b745a5927b3b6d1098d9013c56d1f0b4 Mon Sep 17 00:00:00 2001 From: Aucoto Date: Tue, 13 Oct 2026 14:01:41 +0800 Subject: [PATCH] fix(transform): don't panic on flow-style compose files in publish rewrites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit replace() locates the value's line by scanning for the newline that precedes it: line 1 has no preceding newline, so the scan ran to the end of the stream and the column offset then sliced out of range. A flow-style document ({services: {test: {extends: {file: x}}}}) puts every node on line 1, and compose publish feeds the user's original file bytes into this path (processFile), so the panic was reachable from the CLI. Refuse flow-style nodes explicitly instead: the rewrite runs from the value's start to the end of its line, which would swallow the closing braces and commas following the value in flow style and corrupt the stream. Also guard the env_file entry index against out-of-range values. The processFile errors in createLayers and processExtends are wrapped with the compose file name, so a rejection in any of the project's files — the extended ones included — tells the user which file to fix. Signed-off-by: Aucoto --- pkg/compose/publish.go | 4 +- pkg/compose/publish_test.go | 41 +++++++++++++++++ pkg/compose/transform/replace.go | 66 ++++++++++++++++++++++----- pkg/compose/transform/replace_test.go | 31 +++++++++++++ 4 files changed, 128 insertions(+), 14 deletions(-) diff --git a/pkg/compose/publish.go b/pkg/compose/publish.go index b3616030dfe..55ebe8cf485 100644 --- a/pkg/compose/publish.go +++ b/pkg/compose/publish.go @@ -199,7 +199,7 @@ func (s *composeService) createLayers(ctx context.Context, project *types.Projec for _, file := range project.ComposeFiles { data, err := processFile(ctx, file, project, extFiles, envFiles) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to process compose file %s: %w", file, err) } layerDescriptor := oci.DescriptorForComposeFile(file, data) @@ -235,7 +235,7 @@ func processExtends(ctx context.Context, project *types.Project, extFiles map[st for xf, hash := range extFiles { data, err := processFile(ctx, xf, project, moreExtFiles, envFiles) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to process compose file %s: %w", xf, err) } layerDescriptor := oci.DescriptorForComposeFile(hash, data) diff --git a/pkg/compose/publish_test.go b/pkg/compose/publish_test.go index 79b3884da57..5036c0a966f 100644 --- a/pkg/compose/publish_test.go +++ b/pkg/compose/publish_test.go @@ -112,6 +112,47 @@ services: }, cmp.Ignore())) } +// a processFile failure must say which file it came from: a project can +// carry many compose files and its extends references add more, so an +// unattributed error leaves the user guessing which one to fix. The +// flow-style file makes the assertions meaningful: the rejection itself +// does not name the file, so only the wrap satisfies them. +func Test_createLayers_error_includes_file_name(t *testing.T) { + dir := t.TempDir() + flowFile := filepath.Join(dir, "flow.yaml") + assert.NilError(t, os.WriteFile(flowFile, []byte(`{services: {test: {env_file: old.env}}}`), 0o600)) + // the env file must exist, or the loader rejects it before the + // flow-style rewrite gets a chance to fail + assert.NilError(t, os.WriteFile(filepath.Join(dir, "old.env"), []byte("FOO=bar\n"), 0o600)) + + project := &types.Project{ + Name: "test", + WorkingDir: dir, + Environment: types.Mapping{}, + ComposeFiles: []string{flowFile}, + } + + service := &composeService{} + _, err := service.createLayers(t.Context(), project, api.PublishOptions{}) + assert.ErrorContains(t, err, "failed to process compose file "+flowFile) + assert.ErrorContains(t, err, "flow style") +} + +func Test_processExtends_error_includes_file_name(t *testing.T) { + dir := t.TempDir() + flowFile := filepath.Join(dir, "flow.yaml") + assert.NilError(t, os.WriteFile(flowFile, []byte(`{services: {test: {env_file: old.env}}}`), 0o600)) + assert.NilError(t, os.WriteFile(filepath.Join(dir, "old.env"), []byte("FOO=bar\n"), 0o600)) + + _, err := processExtends(t.Context(), &types.Project{ + Name: "test", + WorkingDir: dir, + Environment: types.Mapping{}, + }, map[string]string{flowFile: "deadbeef"}) + assert.ErrorContains(t, err, "failed to process compose file "+flowFile) + assert.ErrorContains(t, err, "flow style") +} + func Test_preChecks_sensitive_data_detected_decline(t *testing.T) { dir := t.TempDir() envPath := dir + "/secrets.env" diff --git a/pkg/compose/transform/replace.go b/pkg/compose/transform/replace.go index d15dc1d0bc7..d41d27bcc03 100644 --- a/pkg/compose/transform/replace.go +++ b/pkg/compose/transform/replace.go @@ -58,7 +58,10 @@ func ReplaceExtendsFile(in []byte, serviceName string, value string) ([]byte, er } // we've found target `file` yaml node. Let's replace value in stream at node position - return replace(in, file.Line, file.Column, value), nil + if isFlowStyle(root, services, target, extends, file) { + return nil, fmt.Errorf("service %q: extends.file is in a flow-style mapping, which cannot be rewritten while preserving formatting", serviceName) + } + return replace(in, file.Line, file.Column, value) } // ReplaceEnvFile changes value for service.extends.env_file in input yaml stream, preserving formatting @@ -93,6 +96,12 @@ func ReplaceEnvFile(in []byte, serviceName string, i int, value string) ([]byte, // env_file can be either a string, sequence of strings, or sequence of mappings with path attribute if envFile.Kind == yaml.SequenceNode { + if i < 0 || i >= len(envFile.Content) { + return nil, fmt.Errorf("service %q: env_file has %d entries, cannot rewrite entry %d", serviceName, len(envFile.Content), i) + } + if isFlowStyle(root, services, target, envFile) || isFlowStyle(envFile.Content[i]) { + return nil, fmt.Errorf("service %q: env_file is in flow style, which cannot be rewritten while preserving formatting", serviceName) + } envFile = envFile.Content[i] if envFile.Kind == yaml.MappingNode { envFile, err = getMapping(envFile, "path") @@ -100,9 +109,12 @@ func ReplaceEnvFile(in []byte, serviceName string, i int, value string) ([]byte, return nil, err } } - return replace(in, envFile.Line, envFile.Column, value), nil + return replace(in, envFile.Line, envFile.Column, value) } else { - return replace(in, envFile.Line, envFile.Column, value), nil + if isFlowStyle(root, services, target, envFile) { + return nil, fmt.Errorf("service %q: env_file is in flow style, which cannot be rewritten while preserving formatting", serviceName) + } + return replace(in, envFile.Line, envFile.Column, value) } } @@ -123,20 +135,34 @@ func getMapping(root *yaml.Node, key string) (*yaml.Node, error) { } // replace changes yaml node value in stream at position, preserving content -func replace(in []byte, line int, column int, value string) []byte { +func replace(in []byte, line int, column int, value string) ([]byte, error) { var out []byte l := 1 pos := 0 - for _, b := range in { - if b == '\n' { - l++ - if l == line { - break + // the scan looks for the newline that precedes the target line; line 1 + // has none, so without the guard pos would run to the end of the stream + // and the column offset below would slice out of range + if line > 1 { + for _, b := range in { + if b == '\n' { + l++ + if l == line { + break + } } + pos++ } - pos++ + // pos stopped at the newline before the target line, one byte before + // the line start; columns are 1-based, so the value starts at + // line-start + column - 1, which is pos + column + pos += column + } else { + // line 1 starts at offset 0, so the 1-based column maps to column - 1 + pos = column - 1 + } + if pos > len(in) { + return nil, fmt.Errorf("cannot locate line %d column %d in stream", line, column) } - pos += column out = append(out, in[0:pos]...) out = append(out, []byte(value)...) for ; pos < len(in); pos++ { @@ -145,5 +171,21 @@ func replace(in []byte, line int, column int, value string) []byte { } } out = append(out, in[pos:]...) - return out + return out, nil +} + +// isFlowStyle reports whether any node on the path to a rewritten value is +// in flow style. replace rewrites from the value's start to the end of its +// line — anything after the value on that line, a trailing comment +// included, is dropped — so it is only safe when the value is the sole +// content of its line: flow style puts closing braces, commas and sibling +// keys after the value on the same line, and swallowing them would corrupt +// the stream. +func isFlowStyle(nodes ...*yaml.Node) bool { + for _, n := range nodes { + if n != nil && n.Style&yaml.FlowStyle != 0 { + return true + } + } + return false } diff --git a/pkg/compose/transform/replace_test.go b/pkg/compose/transform/replace_test.go index 6f7477e0350..68b3b1184e1 100644 --- a/pkg/compose/transform/replace_test.go +++ b/pkg/compose/transform/replace_test.go @@ -83,3 +83,34 @@ func TestReplace(t *testing.T) { }) } } + +// A flow-style document puts every node on line 1: the position scan in +// replace never sees a newline preceding the target, pos runs to the end of +// the stream, and the column offset then slices out of range. compose +// publish feeds the user's original file bytes into this path +// (processFile), so the flow style is reachable input, not a hypothetical. +func TestReplaceFlowStyle(t *testing.T) { + t.Run("extends file", func(t *testing.T) { + _, err := ReplaceExtendsFile([]byte(`{services: {test: {extends: {file: old.yaml}}}}`), "test", "REPLACED") + assert.ErrorContains(t, err, "flow-style") + }) + t.Run("env_file", func(t *testing.T) { + _, err := ReplaceEnvFile([]byte(`{services: {test: {env_file: old.env}}}`), "test", 0, "REPLACED") + assert.ErrorContains(t, err, "flow style") + }) + // on main this silently corrupted the stream: rewriting an entry of a + // flow sequence swallowed the rest of the line, closing bracket included + t.Run("env_file flow sequence", func(t *testing.T) { + _, err := ReplaceEnvFile([]byte(`{services: {test: {env_file: [a.env, b.env]}}}`), "test", 1, "REPLACED") + assert.ErrorContains(t, err, "flow style") + }) + t.Run("env_file out-of-range entry", func(t *testing.T) { + _, err := ReplaceEnvFile([]byte(`services: + test: + env_file: + - a.env + - b.env +`), "test", 5, "REPLACED") + assert.ErrorContains(t, err, "cannot rewrite entry 5") + }) +}