diff --git a/docs/agent-guide/implemented/bugfix/2026-10-14-external-input-open-json.md b/docs/agent-guide/implemented/bugfix/2026-10-14-external-input-open-json.md new file mode 100644 index 0000000..efd18ab --- /dev/null +++ b/docs/agent-guide/implemented/bugfix/2026-10-14-external-input-open-json.md @@ -0,0 +1,49 @@ +# Open external OCaml JSON inputs and keep owned contracts closed + +## Problem + +A real Journal sync error rejects pull/ok.txs[0].tx-id before transactions are applied. Both the generated server DTO and the structural whitelist reject external response extensions. External services can add fields Journal does not consume without changing required fields. + +## Decision + +Limit this PR to OCaml JSON codecs, as explicitly requested by the user. Keep PPX. External WS and E2EE HTTP records use separate input-only open readers, including nested records; declared required fields, types, nullability, supported variants, semantics and size budgets remain validated. Unused pull tx-id is ignored. Owned client/output DTOs remain closed, and existing internal Config/Protocol/full persistence admission stays closed. Catalog and snapshot metadata retain open input behavior with decode-only derivations. Outbox metadata readers remain decode-only projections rather than full-record admission. + +All Swift codec behavior and its compiler/test/CI configuration are restored exactly to the PR's merge-base. The generic Swift decoder, native call-site replacements, native boundary tests and offline provider compile mode are outside the narrowed scope. + +The user additionally approved the smallest test-only correction for the existing CI failure in Application's retained root feedback checks. The fixture's global title lookup can select Detail's same-title node instead of the root node. Add an optional identity predicate to that test helper and restrict root-content observations to the root navigation owner; retain the original layout and node identity assertions. This does not change production UI behavior. + +## Alternatives considered + +### Declare only tx-id + +Would still reject the next external extension. The user requested open external input schemas. + +### Make shared input/output DTOs open + +Would spread external tolerance into owned contracts. Keep separate input-only readers and closed output DTOs. + +### Add strict Swift receiving infrastructure + +Not part of the narrowed OCaml scope. Generated Encodable/PPX writers already emit their declared fields; strict receiving is a separate defensive policy. Restore Swift and dependent wiring to their prior behavior. + +## Consequences + +Open object validation checks declared fields and duplicate safety without applying the owned-field whitelist. Output uses the original generated closed DTOs and readers with semantic/budget validation. Internal full-record codecs stay closed; projections do not expose encoders. All Dune/spec/Swift/CI files have no net changes relative to the merge-base. No UI changes, real account/graph writes, new diagnostics, merge or phone installation belong to this PR. + +## Validation and coverage + +Tests were written first. Against unchanged main953, public WS extension/declared-control tests failed, and public Runner/local TLS HTTP tests failed four extension-acceptance assertions; required/type/null/duplicate controls passed. Core metadata and internal Config controls passed at baseline. These are synthetic regression payloads, not live service compatibility claims. + +| Boundary | Actual evidence | Public regression boundary | +| --- | --- | --- | +| WS server/client | Materialized and viewed screenshot establishes pull/ok.txs[0].tx-id rejection; payload and deployed SHA unknown | Sync_protocol: all supported variants and rejection reasons, root/nested extras, ignored tx-id shapes, required/type/null/semantic/variant failures, closed client/output and limits | +| Catalog/E2EE HTTP | Prior authorized type-only App logs show catalog/user-key HTTP 200 and graph-key behavior before/after PR72; no fresh signed-in payload collection | Runner with independent localhost TLS peer and generated readers | +| Snapshot metadata/baseline | Upstream schema, no live signed-in response | Core raw completions preserve required/type/null/HTTPS URL checks | +| Internal/outbound | No real account record accessed | Config rejects root/nested extras; existing Protocol and persistence suites retain closed full admission; generated client/server output round trips | +| Transit/snapshot/assets | No new live graph/asset samples | Existing envelope/size/transaction/snapshot/asset regressions; formats unchanged | + +Frozen upstream dev 949a4f0e72cddb2dad34091393bfa09592171944 declares optional non-null UUID tx-id in server entries; old 015cc9f does not declare that server field. Both use open Malli maps. This source evidence does not establish the screenshot value/type or deployment revision: [frozen schema](https://github.com/logseq/logseq/blob/949a4f0e72cddb2dad34091393bfa09592171944/deps/db-sync/src/logseq/db_sync/malli_schema.cljs#L10). Journal does not consume tx-id. + +After scope removal, rerun the OCaml build/native embed and regression suite, review the exact new head in the existing independent Codex session using journal-code-review, and track its CI to success. Prior Swift-inclusive review/CI snapshots are superseded. spec-dev-tool check --all has three preexisting invalid historical decisions unrelated to this change. + +The unchanged root-feedback assertion was reproduced on both 953d4a7 and 9293bad by randomizing only the test observer table's hash order. The root body has one stack candidate; both snapshots preserve root content node 180, while the global title lookup may select Detail node 259. The candidate correction passes the two fixed public Application cases in five randomized runs per snapshot (20 case executions). All original containment and node identity comparisons remain. Diagnostic mirrors and logs stay outside the repository. diff --git a/logseq_db_storage/lib/sync_outbox_store.ml b/logseq_db_storage/lib/sync_outbox_store.ml index 23307a5..79c87a2 100644 --- a/logseq_db_storage/lib/sync_outbox_store.ml +++ b/logseq_db_storage/lib/sync_outbox_store.ml @@ -33,7 +33,9 @@ let schema db = | _ -> Error "unsupported sync outbox storage format") ;; -type mutation_identity = { mutation_id : string } [@@deriving yojson { strict = false }] +(* These decode-only projections extract indexing metadata. The full persisted + record is admitted by the closed domain codec at its owner. *) +type mutation_identity = { mutation_id : string } [@@deriving of_yojson { strict = false }] type mutation_identity_wire = | Save_block of mutation_identity @@ -44,7 +46,7 @@ type mutation_identity_wire = | Create_journal_page of mutation_identity | Set_task_status of mutation_identity | Clear_task_status of mutation_identity -[@@deriving yojson] +[@@deriving of_yojson] type payload_header = { format_version : int @@ -52,7 +54,7 @@ type payload_header = ; sequence : int ; sync_revision : int } -[@@deriving yojson { strict = false }] +[@@deriving of_yojson { strict = false }] let payload_identity record = match diff --git a/logseq_db_worker/test/test_protocol.ml b/logseq_db_worker/test/test_protocol.ml index 3d64258..083db60 100644 --- a/logseq_db_worker/test/test_protocol.ml +++ b/logseq_db_worker/test/test_protocol.ml @@ -1,6 +1,56 @@ module T = Logseq_db_worker_test_support.Test_support module P = Logseq_db_worker.Protocol +let closed_internal_config_fields () = + let base = + `Assoc + [ "application_support_directory", `String "/tmp/synthetic-support" + ; ( "target" + , `List + [ `String "Managed_sync" + ; `Assoc [ "base_url", `String "https://example.invalid" ] + ] ) + ; "compatibility_profile", `List [ `String "Logseq_65_33_or_newer" ] + ; "response_budget_bytes", `Int P.maximum_response_bytes + ; "default_page_size", `Int P.default_page_size + ] + in + T.require + (Result.is_ok (Logseq_db_worker.Config.of_yojson base)) + "valid owned Config control was rejected"; + let outer = + match base with + | `Assoc fields -> `Assoc (("future", `Bool true) :: fields) + | _ -> assert false + in + let nested = + match base with + | `Assoc fields -> + `Assoc + (List.map + (fun (key, value) -> + if key = "target" + then + ( key + , `List + [ `String "Managed_sync" + ; `Assoc + [ "base_url", `String "https://example.invalid" + ; "future", `Bool true + ] + ] ) + else key, value) + fields) + | _ -> assert false + in + List.iter + (fun json -> + T.require + (Result.is_error (Logseq_db_worker.Config.of_yojson json)) + "owned internal Config accepted an extra field") + [ outer; nested ] +;; + let page_tree_contract_rejects_retired_scope () = let raw = Yojson.Safe.from_string @@ -934,7 +984,8 @@ let request_tree_boundaries () = let () = T.run "protocol" - [ T.case "tree depth node and root boundaries" request_tree_boundaries + [ T.case "closed internal Config root and nested fields" closed_internal_config_fields + ; T.case "tree depth node and root boundaries" request_tree_boundaries ; T.case "config golden and validation" config_generated_golden_and_validation ; T.case "generated golden and legacy rejection" generated_golden_and_legacy_rejection ; T.case "request domain constraints" request_domain_constraints diff --git a/logseq_sync/lib/effect_runner/protocol/catalog.ml b/logseq_sync/lib/effect_runner/protocol/catalog.ml index bdb03ac..9b78b5e 100644 --- a/logseq_sync/lib/effect_runner/protocol/catalog.ml +++ b/logseq_sync/lib/effect_runner/protocol/catalog.ml @@ -44,7 +44,7 @@ let schema = function ;; type readiness = { ready : bool option [@key "graph-ready-for-use?"] [@default None] } -[@@deriving yojson { strict = false }] +[@@deriving of_yojson { strict = false }] type graph_dto = { graph_id : Graph_types.Uuid.t [@key "graph-id"] @@ -52,9 +52,9 @@ type graph_dto = ; schema_version : string option [@key "schema-version"] [@default None] ; encrypted : bool option [@key "graph-e2ee?"] [@default None] } -[@@deriving yojson { strict = false }] +[@@deriving of_yojson { strict = false }] -type response = { graphs : Yojson.Safe.t list } [@@deriving yojson { strict = false }] +type response = { graphs : Yojson.Safe.t list } [@@deriving of_yojson { strict = false }] let decode_graph json = bind (readiness_of_yojson json) (fun ready -> diff --git a/logseq_sync/lib/effect_runner/protocol/e2ee.ml b/logseq_sync/lib/effect_runner/protocol/e2ee.ml index b5bf3a3..aa03828 100644 --- a/logseq_sync/lib/effect_runner/protocol/e2ee.ml +++ b/logseq_sync/lib/effect_runner/protocol/e2ee.ml @@ -40,15 +40,15 @@ let bounded_string field value = type graph_key_response_dto = { encrypted_aes_key : string [@key "encrypted-aes-key"] ; encrypted_aes_key_base64 : string option - [@key "encrypted-aes-key-base64"] [@default None] + [@key "encrypted-aes-key-base64"] [@default None] } -[@@deriving yojson] +[@@deriving of_yojson { strict = false }] type user_keys_response_dto = { public_key : string [@key "public-key"] ; encrypted_private_key : string [@key "encrypted-private-key"] } -[@@deriving yojson] +[@@deriving of_yojson { strict = false }] let graph_key_response source = try @@ -56,7 +56,10 @@ let graph_key_response source = (Logseq_db_types.Json_codec.decode graph_key_response_dto_of_yojson (Yojson.Safe.from_string source)) - (fun dto -> bounded_string "encrypted-aes-key" dto.encrypted_aes_key) + (fun dto -> + (* The optional compatibility field is validated by PPX but not consumed. *) + let _ = dto.encrypted_aes_key_base64 in + bounded_string "encrypted-aes-key" dto.encrypted_aes_key) with | Yojson.Json_error _ -> Error "E2EE response must be valid JSON" ;; diff --git a/logseq_sync/lib/pure_reducer/core.ml b/logseq_sync/lib/pure_reducer/core.ml index 9be5743..886150e 100644 --- a/logseq_sync/lib/pure_reducer/core.ml +++ b/logseq_sync/lib/pure_reducer/core.ml @@ -1019,7 +1019,7 @@ type snapshot_uri_response = { ok : bool ; url : string } -[@@deriving yojson { strict = false }] +[@@deriving of_yojson { strict = false }] let decode_snapshot_uri source = try diff --git a/logseq_sync/lib/pure_reducer/sync_protocol.ml b/logseq_sync/lib/pure_reducer/sync_protocol.ml index fb6c444..7d9ed4a 100644 --- a/logseq_sync/lib/pure_reducer/sync_protocol.ml +++ b/logseq_sync/lib/pure_reducer/sync_protocol.ml @@ -163,8 +163,6 @@ let error_to_string (error : codec_error) = ;; module Wire = struct - type header = { kind : string [@key "type"] } [@@deriving yojson { strict = false }] - type client_transaction = { tx : string ; tx_id : Logseq_db_types.Graph_types.Uuid.t option [@key "tx-id"] [@default None] @@ -279,12 +277,74 @@ module Wire = struct ; message : string } [@@deriving yojson] +end - type baseline = - { kind : string [@key "type"] - ; t : int64 +(* External input has readers only; owned output keeps the closed Wire codecs. *) +module External_input = struct + type header = { kind : string [@key "type"] } [@@deriving of_yojson { strict = false }] + + type pull_transaction = + { t : int64 + ; tx : string + ; outliner_op : string option [@key "outliner-op"] [@default None] } - [@@deriving yojson { strict = false }] + [@@deriving of_yojson { strict = false }] + + type user_presence = + { user_id : string [@key "user-id"] + ; email : string option [@default None] + ; username : string option [@default None] + ; name : string option [@default None] + } + [@@deriving of_yojson { strict = false }] + + type server_hello = + { t : int64 + ; checksum : string option [@default None] + } + [@@deriving of_yojson { strict = false }] + + type server_online_users = { online_users : user_presence list [@key "online-users"] } + [@@deriving of_yojson { strict = false }] + + type server_presence = + { user_id : string [@key "user-id"] + ; editing_block_uuid : string option [@key "editing-block-uuid"] + } + [@@deriving of_yojson { strict = false }] + + type server_pull_ok = + { t : int64 + ; checksum : string option [@default None] + ; txs : pull_transaction list + } + [@@deriving of_yojson { strict = false }] + + type server_tx_batch_ok = + { t : int64 + ; checksum : string option [@default None] + } + [@@deriving of_yojson { strict = false }] + + type server_changed = { t : int64 } [@@deriving of_yojson { strict = false }] + + type server_tx_reject = + { reason : string + ; t : int64 option [@default None] + ; checksum : string option [@default None] + ; success_tx_ids : Logseq_db_types.Graph_types.Uuid.t list + [@key "success-tx-ids"] [@default []] + ; failed_tx_id : Logseq_db_types.Graph_types.Uuid.t option + [@key "failed-tx-id"] [@default None] + ; missing_block_uuids : Logseq_db_types.Graph_types.Uuid.t list + [@key "missing-block-uuids"] [@default []] + ; error_detail : string option [@key "error-detail"] [@default None] + ; data : string option [@default None] + } + [@@deriving of_yojson { strict = false }] + + type server_error = { message : string } [@@deriving of_yojson { strict = false }] + type baseline = { t : int64 } [@@deriving of_yojson { strict = false }] end (* Structural checks preserve the public safe diagnostics. They never construct @@ -330,7 +390,11 @@ let check_fields context path allowed fields = | _ :: _ -> error context path (Unexpected_fields unexpected)) ;; -let rec validate context path shape json = +type object_policy = + | Closed + | Open + +let rec validate ?(object_policy = Closed) context path shape json = let invalid expected = error context path (Invalid_field { expected }) in match shape, json with | Text maximum, `String value | Nullable_text maximum, `String value -> @@ -383,14 +447,23 @@ let rec validate context path shape json = let rec loop index = function | [] -> Ok () | value :: rest -> - let* () = validate context (path @ [ string_of_int index ]) item value in + let* () = + validate ~object_policy context (path @ [ string_of_int index ]) item value + in loop (index + 1) rest in loop 0 values | Array _, _ -> invalid "array" | Object schema, `Assoc fields -> let* () = - check_fields context path (List.map (fun (name, _, _) -> name) schema) fields + match object_policy with + | Closed -> + check_fields context path (List.map (fun (name, _, _) -> name) schema) fields + | Open -> + (match duplicate_names (List.map fst fields) with + | [] -> Ok () + | _ :: _ -> + error context path (Invalid_field { expected = "unique object fields" })) in let rec loop = function | [] -> Ok () @@ -399,7 +472,7 @@ let rec validate context path shape json = match List.assoc_opt name fields with | None when is_required -> error context path (Missing_field name) | None -> Ok () - | Some json -> validate context (path @ [ name ]) shape json + | Some json -> validate ~object_policy context (path @ [ name ]) shape json in loop rest in @@ -445,8 +518,10 @@ let rejection_shape = ] ;; -let decode_dto context schema decoder json = - let* () = validate context [] (Object (required "type" metadata :: schema)) json in +let decode_dto ?(object_policy = Closed) context schema decoder json = + let* () = + validate ~object_policy context [] (Object (required "type" metadata :: schema)) json + in match Logseq_db_types.Json_codec.decode decoder json with | Ok dto -> Ok dto | Error _ -> error context [] (Invalid_field { expected = "valid protocol fields" }) @@ -459,11 +534,11 @@ let client_transaction (value : Wire.client_transaction) : Client.transaction = { tx = value.tx; tx_id = value.tx_id; outliner_op = value.outliner_op } ;; -let pull_transaction (value : Wire.pull_transaction) : Server.pull_transaction = +let pull_transaction (value : External_input.pull_transaction) : Server.pull_transaction = { t = cursor value.t; tx = value.tx; outliner_op = value.outliner_op } ;; -let user_presence (value : Wire.user_presence) : user_presence = +let user_presence (value : External_input.user_presence) : user_presence = { user_id = value.user_id ; email = value.email ; username = value.username @@ -560,13 +635,16 @@ let decode_client_fields context message_type json = ;; let decode_server_fields context message_type json = + let decode_dto context schema decoder json = + decode_dto ~object_policy:Open context schema decoder json + in match message_type with | "hello" -> let* dto = decode_dto context [ required "t" Cursor; optional "checksum" Checksum ] - Wire.server_hello_of_yojson + External_input.server_hello_of_yojson json in Ok (Server.Hello { t = cursor dto.t; checksum = dto.checksum }) @@ -575,7 +653,7 @@ let decode_server_fields context message_type json = decode_dto context [ required "online-users" (Array user_presence_shape) ] - Wire.server_online_users_of_yojson + External_input.server_online_users_of_yojson json in Ok (Server.Online_users { online_users = List.map user_presence dto.online_users }) @@ -584,7 +662,7 @@ let decode_server_fields context message_type json = decode_dto context [ required "user-id" metadata; required "editing-block-uuid" nullable_metadata ] - Wire.server_presence_of_yojson + External_input.server_presence_of_yojson json in Ok @@ -598,7 +676,7 @@ let decode_server_fields context message_type json = ; optional "checksum" Checksum ; required "txs" (Array pull_transaction_shape) ] - Wire.server_pull_ok_of_yojson + External_input.server_pull_ok_of_yojson json in Ok @@ -612,17 +690,23 @@ let decode_server_fields context message_type json = decode_dto context [ required "t" Cursor; optional "checksum" Checksum ] - Wire.server_tx_batch_ok_of_yojson + External_input.server_tx_batch_ok_of_yojson json in Ok (Server.Tx_batch_ok { t = cursor dto.t; checksum = dto.checksum }) | "changed" -> let* dto = - decode_dto context [ required "t" Cursor ] Wire.server_changed_of_yojson json + decode_dto + context + [ required "t" Cursor ] + External_input.server_changed_of_yojson + json in Ok (Server.Changed { t = cursor dto.t }) | "tx/reject" -> - let* dto = decode_dto context rejection_shape Wire.server_tx_reject_of_yojson json in + let* dto = + decode_dto context rejection_shape External_input.server_tx_reject_of_yojson json + in let* rejection = validate_rejection context @@ -638,11 +722,15 @@ let decode_server_fields context message_type json = in Ok (Server.Tx_reject rejection) | "pong" -> - let* _ = decode_dto context [] Wire.server_pong_of_yojson json in + let* _ = decode_dto context [] External_input.header_of_yojson json in Ok Server.Pong | "error" -> let* dto = - decode_dto context [ required "message" metadata ] Wire.server_error_of_yojson json + decode_dto + context + [ required "message" metadata ] + External_input.server_error_of_yojson + json in Ok (Server.Error { message = dto.message }) | unsupported -> error context [] (Unsupported_message_type unsupported) @@ -662,7 +750,7 @@ let decode ?maximum direction decode_fields wire = | None -> error context [] (Missing_field "type") | Some json -> validate context [ "type" ] metadata json in - (match Wire.header_of_yojson json with + (match External_input.header_of_yojson json with | Ok header -> decode_fields { context with message_type = Some header.kind } header.kind json | Error _ -> @@ -687,7 +775,9 @@ let decode_snapshot_baseline = | None -> error context [] (Missing_field "t") | Some json -> validate context [ "t" ] Cursor json in - (match Logseq_db_types.Json_codec.decode Wire.baseline_of_yojson json with + (match + Logseq_db_types.Json_codec.decode External_input.baseline_of_yojson json + with | Ok dto -> Ok (cursor dto.t) | Error _ -> error context [] (Invalid_field { expected = "snapshot baseline" })) @@ -775,6 +865,31 @@ let encode_client_message message = encode decode_client_message (client_message_json message) ;; +(* The external reader is never used as the field-set validator for output. *) +let validate_server_output json = + let context = { direction = Server; message_type = None } in + let check decoder = + match Logseq_db_types.Json_codec.decode decoder json with + | Ok _ -> Ok () + | Error _ -> + error context [] (Invalid_field { expected = "closed server output fields" }) + in + match External_input.header_of_yojson json with + | Ok { kind = "hello" } -> check Wire.server_hello_of_yojson + | Ok { kind = "online-users" } -> check Wire.server_online_users_of_yojson + | Ok { kind = "presence" } -> check Wire.server_presence_of_yojson + | Ok { kind = "pull/ok" } -> check Wire.server_pull_ok_of_yojson + | Ok { kind = "tx/batch/ok" } -> check Wire.server_tx_batch_ok_of_yojson + | Ok { kind = "changed" } -> check Wire.server_changed_of_yojson + | Ok { kind = "tx/reject" } -> check Wire.server_tx_reject_of_yojson + | Ok { kind = "pong" } -> check Wire.server_pong_of_yojson + | Ok { kind = "error" } -> check Wire.server_error_of_yojson + | Ok { kind } -> error context [] (Unsupported_message_type kind) + | Error _ -> error context [] (Invalid_field { expected = "message discriminator" }) +;; + let encode_server_message message = - encode decode_server_message (server_message_json message) + let json = server_message_json message in + let* () = validate_server_output json in + encode decode_server_message json ;; diff --git a/logseq_sync/test/core_contract.ml b/logseq_sync/test/core_contract.ml index 50f1db9..b7ab164 100644 --- a/logseq_sync/test/core_contract.ml +++ b/logseq_sync/test/core_contract.ml @@ -5196,3 +5196,61 @@ let scenarios = scenarios @ ; Alcotest.test_case "E2EE invalidated challenge is not advertised" `Quick e2ee_invalidated_challenge_is_not_advertised ] ;; + + +let snapshot_metadata_open_fields_keep_declared_constraints () = + List.iter + (fun ((body : string), success) -> + let requested, _ = begin_snapshot_bootstrap () in + let metadata = + List.find_map + (function + | Core.Run (Core.Request (ticket, Core.Fetch_snapshot_baseline _)) -> + Some + (Core.step + requested.next + (Core.Runner_completed + (Core.Completion + ( ticket + , Ok {|{"type":"pull/ok","t":1,"future":{"nested":true}}|} )))) + | _ -> None) + requested.effects + |> Option.get + in + let downloaded = + List.find_map + (function + | Core.Run (Core.Request (ticket, Core.Fetch_snapshot_metadata _)) -> + Some + (Core.step + metadata.next + (Core.Runner_completed (Core.Completion (ticket, Ok body)))) + | _ -> None) + metadata.effects + |> Option.get + in + Alcotest.(check bool) + "public Core owns metadata decoding and download decision" + success + (List.exists + (function + | Core.Run (Core.Request (_, Core.Download_snapshot _)) -> true + | _ -> false) + downloaded.effects)) + [ ( {|{"ok":true,"url":"https://example.com/snapshot","key":"synthetic","future":{"nested":true}}|} + , true ) + ; {|{"ok":true,"future":true}|}, false + ; {|{"ok":true,"url":null,"future":true}|}, false + ; {|{"ok":"true","url":"https://example.com/snapshot","future":true}|}, false + ; {|{"ok":true,"url":"http://example.com/snapshot","future":true}|}, false + ] +;; + +let scenarios = + scenarios + @ [ Alcotest.test_case + "snapshot metadata open fields keep declared constraints" + `Quick + snapshot_metadata_open_fields_keep_declared_constraints + ] +;; diff --git a/logseq_sync/test/sync_protocol_contract.ml b/logseq_sync/test/sync_protocol_contract.ml index 8252732..4bccfa9 100644 --- a/logseq_sync/test/sync_protocol_contract.ml +++ b/logseq_sync/test/sync_protocol_contract.ml @@ -215,36 +215,7 @@ let test_unknown_fields_are_strict_and_safe () = Alcotest.bool "nested error omits transaction" false - (contains (Protocol.error_to_string nested) "secret tx"); - let pulled_transaction = - Protocol.decode_server_message - {|{"type":"pull/ok","t":1,"txs":[{"t":1,"tx":"secret tx","extra":1}]}|} - |> require_error "unknown pulled transaction field" - in - Alcotest.(check (list string)) - "pulled transaction path is precise" - [ "txs"; "0" ] - pulled_transaction.path; - Alcotest.check - Alcotest.bool - "pulled transaction field is named" - true - (pulled_transaction.kind = Protocol.Unexpected_fields [ "extra" ]); - Alcotest.check - Alcotest.bool - "pulled transaction error omits transaction" - false - (contains (Protocol.error_to_string pulled_transaction) "secret tx"); - let presence = - Protocol.decode_server_message - {|{"type":"online-users","online-users":[{"user-id":"u1","future":true}]}|} - |> require_error "unknown online user field" - in - Alcotest.check Alcotest.bool "direction is server" true (presence.direction = Server); - Alcotest.(check (list string)) - "online-user path is precise" - [ "online-users"; "0" ] - presence.path + (contains (Protocol.error_to_string nested) "secret tx") ;; let test_decode_errors_are_structured () = @@ -388,8 +359,165 @@ let test_cursor_wire_range () = (Client.Pull { since = Some (server_cursor_of_int 9007199254740992) }))) ;; +(* Core receives typed Server.message values, so raw extension acceptance is + owned by this public codec boundary rather than a fabricated reducer event. *) +let external_server_cases = + [ {|{"type":"hello","t":0}|} + ; {|{"type":"online-users","online-users":[{"user-id":"u","future-user":{"nested":true}}]}|} + ; {|{"type":"presence","user-id":"u","editing-block-uuid":null}|} + ; {|{"type":"pull/ok","t":1,"txs":[{"t":1,"tx":"synthetic transit","outliner-op":null,"future-entry":{"nested":true}}]}|} + ; {|{"type":"tx/batch/ok","t":1}|} + ; {|{"type":"changed","t":1}|} + ; {|{"type":"tx/reject","reason":"stale","t":1}|} + ; {|{"type":"tx/reject","reason":"snapshot upload in progress","t":1}|} + ; {|{"type":"tx/reject","reason":"empty tx data"}|} + ; {|{"type":"tx/reject","reason":"invalid tx"}|} + ; {|{"type":"tx/reject","reason":"invalid t-before"}|} + ; {|{"type":"tx/reject","reason":"db transact failed","t":1}|} + ; {|{"type":"pong"}|} + ; {|{"type":"error","message":"synthetic server error"}|} + ] +;; + +let test_external_server_extensions_are_open () = + List.iter + (fun source -> + let json = Yojson.Safe.from_string source in + let extended = + match json with + | `Assoc fields -> + `Assoc + (("future-top", `Assoc [ "nested", `List [ `Null; `Bool true ] ]) :: fields) + | _ -> assert false + in + let input = Yojson.Safe.to_string extended in + let message = + Protocol.decode_server_message input |> require_ok "open external server input" + in + let canonical = + Protocol.encode_server_message message |> require_ok "closed server output" + in + let without_extension = Yojson.Safe.from_string canonical in + let fields = + match without_extension with + | `Assoc f -> f + | _ -> assert false + in + Alcotest.(check bool) + "external extension does not enter owned output" + false + (List.mem_assoc "future-top" fields)) + external_server_cases; + List.iter + (fun ignored -> + let json = + `Assoc + [ "type", `String "pull/ok" + ; "t", `Int 1 + ; ( "txs" + , `List + [ `Assoc + [ "t", `Int 1; "tx", `String "synthetic transit"; "tx-id", ignored ] + ] ) + ] + in + let message = + Protocol.decode_server_message (Yojson.Safe.to_string json) + |> require_ok "unused real-world tx-id extension" + in + let wire = + Protocol.encode_server_message message |> require_ok "re-encode owned pull" + in + Alcotest.(check bool) + "unused tx-id is not reflected in output" + false + (contains wire "tx-id")) + [ `Null; `String "synthetic-id"; `Int 1; `Bool true; `List []; `Assoc [] ] +;; + +let test_external_declared_fields_remain_validated () = + List.iter + (fun source -> + let error = + Protocol.decode_server_message source + |> require_error "declared external contract violation" + in + Alcotest.(check bool) + "extensions never bypass declared-field validation" + false + (match error.kind with + | Protocol.Unexpected_fields _ -> true + | _ -> false)) + [ {|{"type":"changed","future":true}|} + ; {|{"type":"changed","t":"1","future":true}|} + ; {|{"type":"changed","t":null,"future":true}|} + ; {|{"type":"pull/ok","t":1,"txs":[{"t":1,"future":true}],"future":true}|} + ; {|{"type":"pull/ok","t":1,"txs":[{"t":1,"tx":null,"future":true}]}|} + ; {|{"type":"online-users","online-users":[{"user-id":null,"future":true}]}|} + ; {|{"type":"presence","user-id":"u","future":true}|} + ; {|{"type":"hello","t":1,"checksum":null,"future":true}|} + ; {|{"type":"tx/reject","reason":"unrecognized","future":true}|} + ; {|{"type":"tx/reject","reason":"stale","t":1,"failed-tx-id":"not-a-uuid","future":true}|} + ; {|{"type":"future-event","future":true}|} + ; {|{"type":"changed","t":1,"t":2,"future":true}|} + ]; + let missing = + Protocol.decode_server_message {|{"type":"changed","future":true}|} + |> require_error "missing required cursor remains classified" + in + Alcotest.(check bool) + "required field takes precedence over extras" + true + (missing.kind = Protocol.Missing_field "t") +;; + +let test_outgoing_wire_contracts_are_closed () = + List.iter + (fun (message, expected) -> + let wire = + Protocol.encode_client_message message |> require_ok "outgoing contract" + in + Alcotest.(check bool) + "generated output has exactly the contract fields" + true + (Yojson.Safe.equal + (Yojson.Safe.from_string wire) + (Yojson.Safe.from_string expected)); + let extended = + match Yojson.Safe.from_string wire with + | `Assoc fields -> `Assoc (("future", `Bool true) :: fields) + | _ -> assert false + in + ignore + (Protocol.decode_client_message (Yojson.Safe.to_string extended) + |> require_error "closed owned client wire")) + [ Client.Hello { client = "synthetic" }, {|{"type":"hello","client":"synthetic"}|} + ; Client.Presence { editing_block_uuid = None }, {|{"type":"presence"}|} + ; Client.Pull { since = None }, {|{"type":"pull"}|} + ; ( Client.Tx_batch + { client_revision = None + ; t_before = server_cursor_of_int 0 + ; txs = [ { tx = "synthetic"; tx_id = None; outliner_op = None } ] + } + , {|{"type":"tx/batch","t-before":0,"txs":[{"tx":"synthetic"}]}|} ) + ; Client.Ping, {|{"type":"ping"}|} + ] +;; + let scenarios = [ Alcotest.test_case + "external server extensions are open" + `Quick + test_external_server_extensions_are_open + ; Alcotest.test_case + "external declared fields remain validated" + `Quick + test_external_declared_fields_remain_validated + ; Alcotest.test_case + "outgoing JSON contracts are closed" + `Quick + test_outgoing_wire_contracts_are_closed + ; Alcotest.test_case "numeric cursor wire safe integer range" `Quick test_cursor_wire_range @@ -406,7 +534,7 @@ let scenarios = `Quick test_all_rejection_shapes_round_trip ; Alcotest.test_case - "unknown fields are strict and safe" + "owned client fields are strict and safe" `Quick test_unknown_fields_are_strict_and_safe ; Alcotest.test_case diff --git a/logseq_sync/test/transport_contract.ml b/logseq_sync/test/transport_contract.ml index 8eb753f..78406a2 100644 --- a/logseq_sync/test/transport_contract.ml +++ b/logseq_sync/test/transport_contract.ml @@ -4037,7 +4037,7 @@ let test_unlock_response_structure ~expected ~success body () = let scenarios=scenarios @ [ Alcotest.test_case "unlock shape object extra secret field" `Quick - (test_unlock_response_structure ~success:false + (test_unlock_response_structure ~success:true ~expected:"json_kind=object known_fields=1 unknown_fields=1 encrypted_key_present=true" {|{"encrypted-aes-key":"LOG_SECRET_WRAPPED","LOG_SECRET_UNKNOWN_FIELD_NAME":"LOG_SECRET_BODY"}|}) ; Alcotest.test_case "unlock shape array" `Quick @@ -4086,9 +4086,131 @@ let scenarios = scenarios @ (test_unlock_response_structure ~success:false ~expected:"base64_field_kind=number" {|{"encrypted-aes-key":"LOG_SECRET_WRAPPED","encrypted-aes-key-base64":7}|}) - ; Alcotest.test_case "E2EE DTO retains unrelated field strictness" `Quick - (test_unlock_response_structure ~success:false + ; Alcotest.test_case "E2EE DTO ignores unrelated external fields" `Quick + (test_unlock_response_structure ~success:true ~expected:"base64_field_kind=string" {|{"encrypted-aes-key":"LOG_SECRET_WRAPPED","encrypted-aes-key-base64":"LOG_SECRET_BODY","LOG_SECRET_UNKNOWN_FIELD_NAME":false}|}) ] ;; + + +let test_external_http_contract kind ~success body () = + ignore + (with_peer + [ case (unlock_json_wire body) ] + (fun ~environment ~sw ~support ~origin -> + let initial = + Core.config ~managed_sync_origin:origin ~limits:(Core_contract.limits ()) + |> Result.get_ok + |> Core.initial + |> Result.get_ok + in + let request = + match kind with + | `Catalog -> + let auth = + Core.step + initial + (Core.Account_authenticated { user_id = Some "synthetic-user" }) + in + List.find_map + (function + | Core.Run (Core.Request (_, Core.Fetch_catalog _) as request) -> + Some request + | _ -> None) + auth.effects + |> Option.get + | `Graph_key -> snd (unlock_classification_effect origin) + | `User_keys -> + let keys, _ = unlock_classification_effect origin in + let private_keys = + List.find_map + (function + | Core.Run (Core.Request (ticket, Core.Fetch_e2ee_graph_key _)) -> + Some + (Core.step + keys.next + (Core.Runner_completed + (Core.Completion (ticket, Ok "synthetic wrapped key")))) + | _ -> None) + keys.effects + |> Option.get + in + List.find_map + (function + | Core.Run (Core.Request (_, Core.Fetch_e2ee_user_keys _) as request) -> + Some request + | _ -> None) + private_keys.effects + |> Option.get + in + let posted = ref [] + and invalidations = ref 0 in + let t = runner ~environment ~sw ~support ~posted ~invalidations () in + Fun.protect + ~finally:(fun () -> Runner.shutdown t) + (fun () -> + Runner.submit t request; + wait (Eio.Stdenv.clock environment) (fun () -> !posted <> []); + Alcotest.(check bool) + "actual HTTP JSON decoder completion" + success + (List.exists + (function + | Core.Runner_completed (Core.Completion (_, result)) -> + Result.is_ok result + | _ -> false) + !posted))) + : Yojson.Basic.t) +;; + +let scenarios = + scenarios + @ [ Alcotest.test_case + "HTTP graph key ignores external extensions" + `Quick + (test_external_http_contract + `Graph_key + ~success:true + {|{"encrypted-aes-key":"synthetic","future":{"nested":null}}|}) + ; Alcotest.test_case + "HTTP user keys ignore external extensions" + `Quick + (test_external_http_contract + `User_keys + ~success:true + {|{"public-key":"synthetic","encrypted-private-key":"synthetic","future":{"nested":[1,null]}}|}) + ; Alcotest.test_case + "HTTP catalog ignores nested external extensions" + `Quick + (test_external_http_contract + `Catalog + ~success:true + {|{"graphs":[{"graph-id":"11111111-1111-4111-8111-111111111111","graph-name":"synthetic","graph-ready-for-use?":true,"schema-version":null,"future":{"nested":true}}],"future":[1,null]}|}) + ; Alcotest.test_case + "HTTP graph key validates declared fields with extras" + `Quick + (test_external_http_contract + `Graph_key + ~success:false + {|{"encrypted-aes-key":null,"future":true}|}) + ; Alcotest.test_case + "HTTP graph key still requires declared fields" + `Quick + (test_external_http_contract `Graph_key ~success:false {|{"future":true}|}) + ; Alcotest.test_case + "HTTP user keys validate declared fields with extras" + `Quick + (test_external_http_contract + `User_keys + ~success:false + {|{"public-key":"synthetic","encrypted-private-key":false,"future":true}|}) + ; Alcotest.test_case + "HTTP catalog validates nested declared fields" + `Quick + (test_external_http_contract + `Catalog + ~success:false + {|{"graphs":[{"graph-id":"11111111-1111-4111-8111-111111111111","graph-name":null,"graph-ready-for-use?":true,"future":true}]}|}) + ] +;; diff --git a/test/application_view_test.ml b/test/application_view_test.ml index 7d6ade2..f2eaacc 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -1176,10 +1176,10 @@ let run_favorites_native_visibility ((key, member "value" op) :: List.remove_assoc key previous) | _ -> ()) in - let find key value = + let find ?(accept = fun _ -> true) key value = let direct = Hashtbl.fold (fun id values found -> - if List.assoc_opt key values = Some (`String value) then Some id else found) + if accept id && List.assoc_opt key values = Some (`String value) then Some id else found) props None in @@ -1189,7 +1189,7 @@ let run_favorites_native_visibility match List.assoc_opt "payload" values with | Some (`String payload) -> let p = Yojson.Safe.from_string payload in - if (List.mem (Yojson.Safe.Util.member "mode" p) [`String "detail-text"; `String "bounded-text"]) + if accept id && (List.mem (Yojson.Safe.Util.member "mode" p) [`String "detail-text"; `String "bounded-text"]) && Yojson.Safe.Util.member "text" p = `String value then Some id else found | _ -> found) props None | None -> None @@ -1786,19 +1786,23 @@ let run_favorites_native_visibility ~some:inside_body (Hashtbl.find_opt parents id) in + let root_content = + find ~accept:(fun id -> navigation_page id = Some (`String "root")) + "text" "Timeline fixture 1" |> Option.get + in Alcotest.(check bool) "root feedback reserves space for the retained page content" true - (inside_body (Option.get (find "text" "Timeline fixture 1"))); + (inside_body root_content); Alcotest.(check int) "showing feedback preserves the original root content node" original_root_content - (Option.get (find "text" "Timeline fixture 1")); + root_content; Alcotest.(check bool) "failed import keeps its own staged file" true (Sys.file_exists (List.hd !staged_paths)); - let retained_root = Option.get (find "text" "Timeline fixture 1") in + let retained_root = root_content in back (); Alcotest.(check int) "Back retains the root feedback owner and its unresolved error" @@ -1807,7 +1811,9 @@ let run_favorites_native_visibility Alcotest.(check int) "feedback updates retain the root content" retained_root - (Option.get (find "text" "Timeline fixture 1")); + (find + ~accept:(fun id -> navigation_page id = Some (`String "root")) + "text" "Timeline fixture 1" |> Option.get); enter 0; dispatch (Lui_protocol.Press detail_actions); settle_worker (); diff --git a/test/source_boundary_test.ml b/test/source_boundary_test.ml index dfd2c2e..c9d823a 100644 --- a/test/source_boundary_test.ml +++ b/test/source_boundary_test.ml @@ -639,7 +639,8 @@ let test_standalone_sync_protocol_boundary root = [ "module Protocol = Logseq_sync_pure_reducer.Sync_protocol" ; "all client messages round trip" ; "all server messages round trip" - ; "unknown fields are strict and safe" + ; "owned client fields are strict and safe" + ; "external server extensions are open" ; "protocol validation is fail closed" ] ;;