diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 6b60c71..1b995d3 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -10,6 +10,7 @@ import ( "bytes" "encoding/json" "fmt" + "io" "reflect" "sort" "strings" @@ -569,7 +570,24 @@ func strictDecode(raw []byte, into any) error { // field the host does not know is a thing the control plane believes it asked for. dec := json.NewDecoder(bytes.NewReader(raw)) dec.DisallowUnknownFields() - return dec.Decode(into) + if err := dec.Decode(into); err != nil { + return err + } + // And **nothing after it**. A decoder reads one value and stops, so a file holding a + // declaration followed by anything at all — a truncated rewrite, two declarations + // concatenated, a stray line from whatever wrote the file — parses as the first value and the + // rest is never looked at. + // + // That is the same fault this host refuses everywhere else, in its quietest form: the machine + // applies something, reports success, and what it applied is not what the file says. Found + // when a test harness appended a line to a bundle by accident and every apply kept working. + if _, err := dec.Token(); err != io.EOF { + return fmt.Errorf( + "there is more in this file after the declaration ends. Refused whole: a file with " + + "something after it may be a truncated rewrite or two declarations run together, " + + "and applying the first would be applying something nobody wrote") + } + return nil } // unknownFields names every JSON key the kind's struct has no field for. diff --git a/internal/declaration/declaration_test.go b/internal/declaration/declaration_test.go index 3ed10f7..3196ecf 100644 --- a/internal/declaration/declaration_test.go +++ b/internal/declaration/declaration_test.go @@ -313,3 +313,34 @@ func TestSomethingInsideAValueIsNotAComment(t *testing.T) { t.Fatalf("a value was mangled: %q", file.Content) } } + +// A declaration followed by anything at all is refused whole. +// +// A JSON decoder reads one value and stops, so a file holding a declaration and then a stray line +// parses as the declaration and the rest is never looked at. The machine applies something, +// reports success, and what it applied is not what the file says. +// +// Not hypothetical: a test harness appended a line to the substrate bundle by accident, every +// apply kept working, and nothing said so for the entire time it was wrong. +func TestSomethingAfterTheDeclarationIsRefused(t *testing.T) { + good := `{"declaration":1,"resources":[{"id":"a","type":"file","path":"/tmp/a",` + + `"content":"x","mode":"0644"}]}` + if _, err := ParseFileTrusted([]byte(good)); err != nil { + t.Fatalf("an ordinary declaration was refused: %v", err) + } + for _, after := range []string{ + "MESHBUNDLE 2>&1; echo \"__exit=$?\"", + good, + "garbage", + } { + _, err := ParseFileTrusted([]byte(good + "\n" + after)) + if err == nil { + t.Fatalf("a file with %q after the declaration was accepted", after) + } + } + // Trailing whitespace is not "something after it", and refusing it would make every file + // written by an editor unusable. + if _, err := ParseFileTrusted([]byte(good + "\n\n \n")); err != nil { + t.Fatalf("a declaration with a trailing newline was refused: %v", err) + } +}