From c3bc0d3b3f63fb210d8ca19761fb24977836544e Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Fri, 23 Oct 2026 09:57:09 +0100 Subject: [PATCH] fix(cli): reject incomplete and trailing JSON in config files --- templates/cli/internal/jsonx/object.go | 38 +++++++++-- tests/e2e/CLIGo126Test.php | 1 + tests/e2e/languages/cli/main.go | 95 ++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 4 deletions(-) diff --git a/templates/cli/internal/jsonx/object.go b/templates/cli/internal/jsonx/object.go index 5cbf71f957..afc295766a 100644 --- a/templates/cli/internal/jsonx/object.go +++ b/templates/cli/internal/jsonx/object.go @@ -223,7 +223,11 @@ func (o *Object) UnmarshalJSON(data []byte) error { o.keys = nil o.values = map[string]any{} - return o.decodeInto(decoder) + if err := o.decodeInto(decoder); err != nil { + return err + } + + return requireEnd(decoder) } func (o *Object) decodeInto(decoder *json.Decoder) error { @@ -244,14 +248,32 @@ func (o *Object) decodeInto(decoder *json.Decoder) error { o.Set(key, value) } - // Consume the closing brace. - if _, err := decoder.Token(); err != nil && err != io.EOF { + // A truncated object is not an empty or complete one. + token, err := decoder.Token() + if err != nil { return err } + if token != json.Delim('}') { + return fmt.Errorf("jsonx: expected closing brace, got %v", token) + } return nil } +// A config or response body contains one complete JSON value. Ignoring a suffix +// can discard resource definitions when the decoded document is written back. +func requireEnd(decoder *json.Decoder) error { + token, err := decoder.Token() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + + return fmt.Errorf("jsonx: unexpected token after JSON value: %v", token) +} + func decodeValue(decoder *json.Decoder) (any, error) { token, err := decoder.Token() if err != nil { @@ -367,7 +389,15 @@ func DecodeValue(payload []byte) (any, error) { decoder := json.NewDecoder(bytes.NewReader(trimmed)) decoder.UseNumber() - return decodeValue(decoder) + value, err := decodeValue(decoder) + if err != nil { + return nil, err + } + if err := requireEnd(decoder); err != nil { + return nil, err + } + + return value, nil } // Marshal renders the object the way config files are written: four-space diff --git a/tests/e2e/CLIGo126Test.php b/tests/e2e/CLIGo126Test.php index 3dec0b451f..3c52d65e6e 100644 --- a/tests/e2e/CLIGo126Test.php +++ b/tests/e2e/CLIGo126Test.php @@ -50,6 +50,7 @@ final class CLIGo126Test extends Base ...Base::UPLOAD_RESPONSE, ...Base::UPLOAD_RESPONSE, ...Base::CLI_HEADERS_RESPONSES, + 'CLI_CONFIG_VALIDATION:passed', 'CLI_TYPES_DEPENDENCIES:passed', 'CLI_INIT_APNS:passed', 'CLI_INIT_APNS_APPLE:passed', diff --git a/tests/e2e/languages/cli/main.go b/tests/e2e/languages/cli/main.go index d6d2687158..6ea4c8fc09 100644 --- a/tests/e2e/languages/cli/main.go +++ b/tests/e2e/languages/cli/main.go @@ -97,6 +97,9 @@ func main() { } } + harness.configValidation() + fmt.Println("CLI_CONFIG_VALIDATION:passed") + harness.typesDependencies() fmt.Println("CLI_TYPES_DEPENDENCIES:passed") harness.initApns() @@ -121,6 +124,98 @@ func (h *harness) command(args ...string) *exec.Cmd { return command } +// Config rewrites must reject malformed documents before changing either the +// project file or any included resource file. +func (h *harness) configValidation() { + cli, err := filepath.Abs(binary) + if err != nil { + fail(err) + } + + cases := []struct { + name string + document string + included bool + }{ + {"stray-brace", `{"projectId":"before","functions":[]},"tables":[{"$id":"orders"}]}`, false}, + {"trailing-text", `{"projectId":"before"} trailing`, false}, + {"second-object", `{"projectId":"before"} {"tables":[]}`, false}, + {"open-object", `{`, false}, + {"missing-brace", `{"projectId":"before"`, false}, + {"missing-nested-brace", `{"projectId":"before","custom":{"keep":1}`, false}, + {"missing-array-close", `{"tables":[{"$id":"orders"}`, false}, + {"mismatched-close", `{"tables":[{"$id":"orders"}] ]`, false}, + {"include-trailing-text", `[{"$id":"orders"}] trailing`, true}, + {"include-second-array", `[{"$id":"orders"}] [{"$id":"lost"}]`, true}, + {"include-missing-close", `[{"$id":"orders"}`, true}, + {"include-missing-nested-close", `[{"$id":"orders","custom":{"keep":1}]`, true}, + } + for _, test := range cases { + directory := filepath.Join(h.home, "config-validation", test.name) + if err := os.MkdirAll(directory, 0o700); err != nil { + fail(err) + } + files := map[string]string{"appwrite.config.json": test.document} + want := "failed to read config file" + if test.included { + files["appwrite.config.json"] = `{"projectId":"before","includes":{"tables":"tables.json"}}` + files["tables.json"] = test.document + want = "failed to read included config" + } + for name, contents := range files { + if err := os.WriteFile(filepath.Join(directory, name), []byte(contents), 0o600); err != nil { + fail(err) + } + } + command := h.command("client", "--project-id", "after") + command.Path, command.Dir = cli, directory + output, err := command.CombinedOutput() + if err == nil || !strings.Contains(string(output), want) { + fail(fmt.Errorf("config %s: expected %q and failure, got %v: %s", test.name, want, err, output)) + } + for name, before := range files { + after, err := os.ReadFile(filepath.Join(directory, name)) + if err != nil || string(after) != before { + fail(fmt.Errorf("config %s: %s changed after rejection: %v\n%s", test.name, name, err, after)) + } + } + } + + // Whitespace after a complete value is valid, including for included arrays. + directory := filepath.Join(h.home, "config-validation", "valid") + if err := os.MkdirAll(directory, 0o700); err != nil { + fail(err) + } + files := map[string]string{ + "appwrite.config.json": "{\"projectId\":\"before\",\"custom\":{\"keep\":9007199254740993},\"includes\":{\"tables\":\"tables.json\"}} \n\t", + "tables.json": "[{\"$id\":\"orders\",\"name\":\"Orders\",\"databaseId\":\"main\",\"columns\":[]}] \n\t", + } + for name, contents := range files { + if err := os.WriteFile(filepath.Join(directory, name), []byte(contents), 0o600); err != nil { + fail(err) + } + } + command := h.command("client", "--project-id", "after") + command.Path, command.Dir = cli, directory + if output, err := command.CombinedOutput(); err != nil { + fail(fmt.Errorf("valid config rewrite failed: %w\n%s", err, output)) + } + for name, want := range map[string][]string{ + "appwrite.config.json": {`"projectId": "after"`, `"keep": 9007199254740993`, `"tables": "tables.json"`}, + "tables.json": {`"$id": "orders"`, `"name": "Orders"`, `"databaseId": "main"`}, + } { + contents, err := os.ReadFile(filepath.Join(directory, name)) + if err != nil { + fail(err) + } + for _, expected := range want { + if !strings.Contains(string(contents), expected) { + fail(fmt.Errorf("valid config rewrite lost %s in %s: %s", expected, name, contents)) + } + } + } +} + // typesDependencies runs from a monorepo root, not the nested output directory. // Only local configuration is used; no Appwrite project is read or written. func (h *harness) typesDependencies() {