fix(backup,invoke): schedule .env read error, E420 number echo (D-0590, D-1113)

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)
+	}
+}

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
 	}

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 <projectDir>/.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

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 {

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)
+	}
+}