fix(sheets): require explicit csv input

This commit is contained in:
zhengzhijie
2026-07-13 20:54:45 +08:00
parent 6f41dbdb11
commit dc5075b0a8
3 changed files with 12 additions and 156 deletions

View File

@@ -1,115 +0,0 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT
package sheets
import (
"testing"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/spf13/cobra"
)
// +csv-put lets a piped CSV satisfy an omitted --csv: agents routinely redirect
// a file into stdin but forget the `--csv -`. PostMount relaxes the required
// gate and installs a PreRunE that, when stdin is a non-interactive pipe,
// defaults an absent --csv to "-" so the standard stdin path reads it. On an
// interactive terminal the fallback stays off so the command never blocks.
func mountCsvPut(t *testing.T) *cobra.Command {
t.Helper()
f, _, _, _ := cmdutil.TestFactory(t, nil)
parent := &cobra.Command{Use: "sheets"}
CsvPut.Mount(parent, f)
cmd, _, err := parent.Find([]string{"+csv-put"})
if err != nil {
t.Fatalf("Find(+csv-put) error = %v", err)
}
return cmd
}
func csvRequiredAnnotationPresent(cmd *cobra.Command) bool {
fl := cmd.Flags().Lookup("csv")
if fl == nil {
return false
}
_, ok := fl.Annotations[cobra.BashCompOneRequiredFlag]
return ok
}
// withStdinIsPipe swaps the package-level pipe detector for the duration of a
// test so behavior does not depend on the real process stdin.
func withStdinIsPipe(t *testing.T, piped bool) {
t.Helper()
prev := csvPutStdinIsPipe
csvPutStdinIsPipe = func() bool { return piped }
t.Cleanup(func() { csvPutStdinIsPipe = prev })
}
func TestCsvPutPostMount_RelaxesCsvRequired(t *testing.T) {
cmd := mountCsvPut(t)
if csvRequiredAnnotationPresent(cmd) {
t.Error("--csv required annotation should be relaxed so csvPutInput reports the typed error")
}
if cmd.PreRunE == nil {
t.Fatal("PostMount should install a PreRunE for the stdin fallback")
}
}
func TestCsvPutPreRunE_PipedAbsentDefaultsToDash(t *testing.T) {
withStdinIsPipe(t, true)
cmd := mountCsvPut(t)
if err := cmd.PreRunE(cmd, nil); err != nil {
t.Fatalf("PreRunE error = %v", err)
}
if got, _ := cmd.Flags().GetString("csv"); got != "-" {
t.Errorf("csv = %q, want %q (piped + absent should default to '-')", got, "-")
}
}
func TestCsvPutPreRunE_InteractiveAbsentStaysEmpty(t *testing.T) {
withStdinIsPipe(t, false)
cmd := mountCsvPut(t)
if err := cmd.PreRunE(cmd, nil); err != nil {
t.Fatalf("PreRunE error = %v", err)
}
if got, _ := cmd.Flags().GetString("csv"); got != "" {
t.Errorf("csv = %q, want empty (interactive stdin must not be consumed / must not hang)", got)
}
}
func TestCsvPutPreRunE_ExplicitValueUnchanged(t *testing.T) {
withStdinIsPipe(t, true)
cmd := mountCsvPut(t)
if err := cmd.Flags().Set("csv", "x,y"); err != nil {
t.Fatalf("Set(csv) error = %v", err)
}
if err := cmd.PreRunE(cmd, nil); err != nil {
t.Fatalf("PreRunE error = %v", err)
}
if got, _ := cmd.Flags().GetString("csv"); got != "x,y" {
t.Errorf("csv = %q, want %q (explicit value must not be overridden)", got, "x,y")
}
}
func TestCsvPutPostMount_ComposesExistingPreRunE(t *testing.T) {
withStdinIsPipe(t, true)
cmd := &cobra.Command{Use: "+csv-put"}
cmd.Flags().String("start-cell", "", "")
cmd.Flags().String("range", "", "")
cmd.Flags().String("csv", "", "")
called := false
cmd.PreRunE = func(*cobra.Command, []string) error {
called = true
return nil
}
CsvPut.PostMount(cmd)
if err := cmd.PreRunE(cmd, nil); err != nil {
t.Fatalf("PreRunE error = %v", err)
}
if !called {
t.Fatal("existing PreRunE was not called")
}
if got, _ := cmd.Flags().GetString("csv"); got != "-" {
t.Fatalf("csv = %q, want fallback after existing PreRunE", got)
}
}

View File

@@ -11,7 +11,6 @@ import (
_ "image/gif"
_ "image/jpeg"
_ "image/png"
"os"
"path/filepath"
"strconv"
"strings"
@@ -204,19 +203,6 @@ func cellsSetStyleInput(runtime flagView, token, sheetID, sheetName string) (map
return input, nil
}
// csvPutStdinIsPipe reports whether process stdin is a non-interactive pipe or
// redirect (rather than an interactive terminal), so an omitted --csv can be
// satisfied from it without risking a hang on a real terminal. Overridable in
// tests. A char device is a terminal; anything else (pipe, redirect, /dev/null)
// counts as piped input.
var csvPutStdinIsPipe = func() bool {
fi, err := os.Stdin.Stat() //nolint:forbidigo // pipe detection needs the real process fd; IOStreams.In is a plain io.Reader without Stat
if err != nil {
return false
}
return fi.Mode()&os.ModeCharDevice == 0
}
// CsvPut wraps set_range_from_csv: dump a CSV blob into a sheet. A cell whose
// text starts with = is evaluated as a formula; use +cells-set for styles / notes / images.
var CsvPut = common.Shortcut{
@@ -239,33 +225,6 @@ var CsvPut = common.Shortcut{
}
cmd.MarkFlagsOneRequired("start-cell", "range")
cmd.MarkFlagsMutuallyExclusive("start-cell", "range")
// Let a piped CSV satisfy --csv when the flag is omitted: agents
// routinely redirect a file into stdin but forget the `--csv -`, so
// `+csv-put ... < data.csv` would otherwise fail its first try on a
// missing --csv. Relax the required-gate (flag-defs marks --csv
// required) so an absent value surfaces csvPutInput's own typed error
// instead of cobra's bare "required flag(s) ... not set"; then, in
// PreRunE (which cobra runs before it validates required flags), default
// an omitted --csv to "-" when stdin is a non-interactive pipe so the
// standard stdin-resolution path reads it. The pipe guard means an
// interactive terminal never blocks waiting on stdin — a real miss still
// errors.
if fl := cmd.Flags().Lookup("csv"); fl != nil {
delete(fl.Annotations, cobra.BashCompOneRequiredFlag)
}
prev := cmd.PreRunE
cmd.PreRunE = func(c *cobra.Command, args []string) error {
if prev != nil {
if err := prev(c, args); err != nil {
return err
}
}
if v, _ := c.Flags().GetString("csv"); strings.TrimSpace(v) == "" && csvPutStdinIsPipe() {
_ = c.Flags().Set("csv", "-")
}
return nil
}
},
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
if err := guardCSVValueIsNotFilePath(runtime); err != nil {

View File

@@ -116,6 +116,18 @@ func TestWriteCellsShortcuts_DryRun(t *testing.T) {
}
}
func TestCsvPut_MissingCSVFailsRequiredGate(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, CsvPut, []string{
"--url", testURL,
"--sheet-id", testSheetID,
"--start-cell", "A1",
})
if err == nil || !strings.Contains(err.Error(), "required flag(s) \"csv\" not set") {
t.Fatalf("missing --csv error = %v, want cobra required-flag error", err)
}
}
// TestDropdownSet_CellsShape inspects the 3×1 matrix produced from
// --range A2:A4 to confirm the data_validation prototype is replicated.
// Also covers --colors / --highlight emitting the canonical