From 249efaee4617c898b7eff090ec303e29ba18980f Mon Sep 17 00:00:00 2001 From: Tony <1094086026@qq.com> Date: Fri, 22 May 2026 05:41:48 +0800 Subject: [PATCH] feat: add --split-exp-no-overwrite flag to refuse overwriting existing files When using --split-exp to split documents into per-file outputs, yq silently overwrites any pre-existing files at the target paths because os.Create truncates. For workflows that generate filenames from input data (e.g. '.metadata.name + ".yml"'), this can clobber unrelated files when two documents map to the same name, or when a target path collides with something already on disk. This change adds an opt-in --split-exp-no-overwrite flag (and a matching yqlib constructor NewMultiPrinterWriterWithOptions) that uses O_WRONLY|O_CREATE|O_EXCL so existing files are left untouched and yq exits with a clear error message instead. The default behaviour (overwrite) is unchanged; the original NewMultiPrinterWriter constructor still exists and now delegates to the new options-aware constructor with noOverwrite=false. Fixes #2028 --- README.md | 1 + cmd/constant.go | 1 + cmd/root.go | 1 + cmd/utils.go | 2 +- pkg/yqlib/printer_writer.go | 27 +++++++-- pkg/yqlib/printer_writer_test.go | 96 ++++++++++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 pkg/yqlib/printer_writer_test.go diff --git a/README.md b/README.md index 25e4895f..790988ee 100644 --- a/README.md +++ b/README.md @@ -433,6 +433,7 @@ Flags: --shell-key-separator string separator for shell variable key paths (default "_") -s, --split-exp string print each result (or doc) into a file named (exp). [exp] argument must return a string. You can use $index in the expression as the result counter. The necessary directories will be created. --split-exp-file string Use a file to specify the split-exp expression. + --split-exp-no-overwrite When using --split-exp, fail if a target file already exists instead of overwriting it. --string-interpolation Toggles strings interpolation of \(exp) (default true) --tsv-auto-parse parse TSV YAML/JSON values (default true) -r, --unwrapScalar unwrap scalar, print the value with no quotes, colors or comments. Defaults to true for yaml (default true) diff --git a/cmd/constant.go b/cmd/constant.go index bf15c4fb..68096982 100644 --- a/cmd/constant.go +++ b/cmd/constant.go @@ -31,6 +31,7 @@ var frontMatter = "" var splitFileExp = "" var splitFileExpFile = "" +var splitFileNoOverwrite = false var completedSuccessfully = false diff --git a/cmd/root.go b/cmd/root.go index d2f72bfa..c91788d9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -204,6 +204,7 @@ yq -P -oy sample.json if err = rootCmd.MarkPersistentFlagFilename("split-exp-file"); err != nil { panic(err) } + rootCmd.PersistentFlags().BoolVarP(&splitFileNoOverwrite, "split-exp-no-overwrite", "", false, "When using --split-exp, fail if a target file already exists instead of overwriting it.") rootCmd.PersistentFlags().StringVarP(&expressionFile, "from-file", "", "", "Load expression from specified file.") if err = rootCmd.MarkPersistentFlagFilename("from-file"); err != nil { diff --git a/cmd/utils.go b/cmd/utils.go index 553fc994..e427701b 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -186,7 +186,7 @@ func configurePrinterWriter(format *yqlib.Format, out io.Writer) (yqlib.PrinterW if err != nil { return nil, fmt.Errorf("bad split document expression: %w", err) } - printerWriter = yqlib.NewMultiPrinterWriter(splitExp, format) + printerWriter = yqlib.NewMultiPrinterWriterWithOptions(splitExp, format, splitFileNoOverwrite) } else { printerWriter = yqlib.NewSinglePrinterWriter(out) } diff --git a/pkg/yqlib/printer_writer.go b/pkg/yqlib/printer_writer.go index 88d60a0f..3456c3ff 100644 --- a/pkg/yqlib/printer_writer.go +++ b/pkg/yqlib/printer_writer.go @@ -32,9 +32,17 @@ type multiPrintWriter struct { nameExpression *ExpressionNode extension string index int + noOverwrite bool } func NewMultiPrinterWriter(expression *ExpressionNode, format *Format) PrinterWriter { + return NewMultiPrinterWriterWithOptions(expression, format, false) +} + +// NewMultiPrinterWriterWithOptions creates a multi-file printer writer. +// When noOverwrite is true, attempting to write to a file that already +// exists will fail with an error instead of silently overwriting it. +func NewMultiPrinterWriterWithOptions(expression *ExpressionNode, format *Format, noOverwrite bool) PrinterWriter { extension := "yml" switch format { @@ -49,6 +57,7 @@ func NewMultiPrinterWriter(expression *ExpressionNode, format *Format) PrinterWr extension: extension, treeNavigator: NewDataTreeNavigator(), index: 0, + noOverwrite: noOverwrite, } } @@ -75,10 +84,20 @@ func (sp *multiPrintWriter) GetWriter(node *CandidateNode) (*bufio.Writer, error if err != nil { return nil, err } - f, err := os.Create(name) - - if err != nil { - return nil, err + var f *os.File + if sp.noOverwrite { + f, err = os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) + if err != nil { + if os.IsExist(err) { + return nil, fmt.Errorf("refusing to overwrite existing file %q (--no-overwrite is set)", name) + } + return nil, err + } + } else { + f, err = os.Create(name) + if err != nil { + return nil, err + } } sp.index = sp.index + 1 diff --git a/pkg/yqlib/printer_writer_test.go b/pkg/yqlib/printer_writer_test.go new file mode 100644 index 00000000..54d79c60 --- /dev/null +++ b/pkg/yqlib/printer_writer_test.go @@ -0,0 +1,96 @@ +package yqlib + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// helper to build an ExpressionNode that just yields a fixed string for the file name +func parseFilenameExp(t *testing.T, exp string) *ExpressionNode { + t.Helper() + InitExpressionParser() + node, err := ExpressionParser.ParseExpression(exp) + if err != nil { + t.Fatalf("failed to parse split-exp test expression %q: %v", exp, err) + } + return node +} + +func TestMultiPrinterWriterOverwriteDefault(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "out.yml") + if err := os.WriteFile(target, []byte("pre-existing\n"), 0600); err != nil { + t.Fatalf("setup: %v", err) + } + + exp := parseFilenameExp(t, `"`+target+`"`) + pw := NewMultiPrinterWriter(exp, YamlFormat) + + node := &CandidateNode{Kind: ScalarNode, Tag: "!!str", Value: "hello"} + w, err := pw.GetWriter(node) + if err != nil { + t.Fatalf("default behaviour should silently overwrite, got error: %v", err) + } + if w == nil { + t.Fatalf("expected a writer, got nil") + } + // confirm the file was truncated/recreated by os.Create + info, err := os.Stat(target) + if err != nil { + t.Fatalf("stat target: %v", err) + } + if info.Size() != 0 { + t.Fatalf("expected file to be truncated (size 0) before writes, got %d bytes", info.Size()) + } +} + +func TestMultiPrinterWriterNoOverwriteRefusesExisting(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "out.yml") + if err := os.WriteFile(target, []byte("pre-existing\n"), 0600); err != nil { + t.Fatalf("setup: %v", err) + } + + exp := parseFilenameExp(t, `"`+target+`"`) + pw := NewMultiPrinterWriterWithOptions(exp, YamlFormat, true) + + node := &CandidateNode{Kind: ScalarNode, Tag: "!!str", Value: "hello"} + _, err := pw.GetWriter(node) + if err == nil { + t.Fatalf("expected error when --no-overwrite is set and target exists, got nil") + } + if !strings.Contains(err.Error(), "refusing to overwrite") { + t.Fatalf("expected refusing-to-overwrite error message, got: %v", err) + } + + // file must be untouched + data, err := os.ReadFile(target) + if err != nil { + t.Fatalf("read target: %v", err) + } + if string(data) != "pre-existing\n" { + t.Fatalf("file should be untouched, contents = %q", string(data)) + } +} + +func TestMultiPrinterWriterNoOverwriteCreatesNew(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "new.yml") + + exp := parseFilenameExp(t, `"`+target+`"`) + pw := NewMultiPrinterWriterWithOptions(exp, YamlFormat, true) + + node := &CandidateNode{Kind: ScalarNode, Tag: "!!str", Value: "hello"} + w, err := pw.GetWriter(node) + if err != nil { + t.Fatalf("no-overwrite should still create new files, got: %v", err) + } + if w == nil { + t.Fatalf("expected a writer, got nil") + } + if _, err := os.Stat(target); err != nil { + t.Fatalf("expected new file to exist, stat err: %v", err) + } +}