diff --git a/internal/backup/schedule_test.go b/internal/backup/schedule_test.go index f68a6adfd..f87276431 100644 --- a/internal/backup/schedule_test.go +++ b/internal/backup/schedule_test.go @@ -360,3 +360,26 @@ func TestScheduleStreamRefusesLineBreakInPaths(t *testing.T) { } } } + +// TestScheduleStreamMalformedDotEnv: an unparseable .env and no process ENV is +// an error, not a unit written for ENV=dev; a missing .env still means dev. +func TestScheduleStreamMalformedDotEnv(t *testing.T) { + skipUnitTestOnWindows(t) + setScheduleProcessEnv(t, "") + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, ".env"), []byte("ENV=\"unterminated\n"), 0o600); err != nil { + t.Fatal(err) + } + var out bytes.Buffer + err := ScheduleStream(minimalConfig(), ScheduleOptions{Cron: "0 3 * * *", To: "r2:b", ProjectDir: dir, BinaryPath: "/bin/sh", DryRun: true, Out: &out}) + if err == nil || !strings.Contains(err.Error(), ".env") { + t.Fatalf("err = %v, want a .env read error", err) + } + if out.Len() != 0 { + t.Errorf("nothing may be printed, got %q", out.String()) + } + got := scheduleDryRun(t, ScheduleOptions{Cron: "0 3 * * *", To: "r2:b", ProjectDir: t.TempDir(), BinaryPath: "/bin/sh"}) + if !strings.Contains(got, "Environment=ENV=dev") { + t.Errorf("a missing .env must still default to dev:\n%s", got) + } +} diff --git a/internal/backup/stream_schedule.go b/internal/backup/stream_schedule.go index be4a3f1d3..2d9cc77e7 100644 --- a/internal/backup/stream_schedule.go +++ b/internal/backup/stream_schedule.go @@ -77,7 +77,10 @@ func ScheduleStream(cfg *config.Config, opts ScheduleOptions) error { // The ENV selector the nightly run needs: the one that picked the cascade // when this schedule was made (see scheduleEnvSelector). It is written into // the unit as a bare word, so it is checked before anything is rendered. - envSelector := scheduleEnvSelector(projectDir) + envSelector, err := scheduleEnvSelector(projectDir) + if err != nil { + return err + } if err := checkScheduleEnv(envSelector); err != nil { return err } diff --git a/internal/backup/stream_schedule_project.go b/internal/backup/stream_schedule_project.go index 68acf96d9..7839d7d35 100644 --- a/internal/backup/stream_schedule_project.go +++ b/internal/backup/stream_schedule_project.go @@ -9,8 +9,10 @@ package backup import ( "bufio" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "regexp" @@ -70,15 +72,24 @@ func composeHasGeneratedHeader(path string) bool { // projectDir, in the order config.ResolveEnv uses: the ENV the process was // started with, else the ENV key of /.env, else "dev". The raw // value is returned; the loader normalises it again when the unit runs. -// godotenv.Read does not touch os.Environ. -func scheduleEnvSelector(projectDir string) string { +// godotenv.Read does not touch os.Environ. A missing .env falls through to +// "dev"; an unreadable or unparseable one is an error, because the unit would +// otherwise be written for "dev" while the nightly run fails on the same file. +func scheduleEnvSelector(projectDir string) (string, error) { if scheduleProcessEnv != "" { - return scheduleProcessEnv + return scheduleProcessEnv, nil } - if m, err := godotenv.Read(filepath.Join(projectDir, ".env")); err == nil && m["ENV"] != "" { - return m["ENV"] + m, err := godotenv.Read(filepath.Join(projectDir, ".env")) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return "dev", nil + } + return "", fmt.Errorf("read the ENV selector from %s: %w", filepath.Join(projectDir, ".env"), err) + } + if m["ENV"] != "" { + return m["ENV"], nil } - return "dev" + return "dev", nil } // checkScheduleEnv refuses a selector that could add a directive or expand in diff --git a/internal/invoke/request_nulls.go b/internal/invoke/request_nulls.go index 684ba5c24..74274057c 100644 --- a/internal/invoke/request_nulls.go +++ b/internal/invoke/request_nulls.go @@ -23,6 +23,7 @@ func checkNoNullItems(raw []byte, member string) error { return nil } dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() // keep a number a string: no float parse, so no digits echoed in an error _, _ = dec.Token() // the opening bracket depth := 0 for dec.More() || depth > 0 { diff --git a/internal/invoke/request_nulls_test.go b/internal/invoke/request_nulls_test.go new file mode 100644 index 000000000..49eb890a0 --- /dev/null +++ b/internal/invoke/request_nulls_test.go @@ -0,0 +1,28 @@ +package invoke + +import ( + "strings" + "testing" +) + +// An overlong number in args must not be echoed back in the E420 message: +// without UseNumber the decoder parses it as a float64 and puts all of its +// digits in the parse error. +func TestCheckNoNullItemsBoundsNumberError(t *testing.T) { + raw := []byte(`["a", ` + strings.Repeat("9", 5000) + `]`) + err := checkNoNullItems(raw, "args") + if err != nil && len(err.Error()) > 500 { + t.Fatalf("E420 message echoes the number: %d bytes", len(err.Error())) + } +} + +func TestCheckNoNullItemsStillRefusesNull(t *testing.T) { + for _, raw := range []string{`[null]`, `["a", null]`} { + if err := checkNoNullItems([]byte(raw), "args"); err == nil { + t.Fatalf("%s: want an error", raw) + } + } + if err := checkNoNullItems([]byte(`["a", 1, [null], {"k": null}]`), "args"); err != nil { + t.Fatalf("nested null must be left to the typed decode: %v", err) + } +}