fix: Raise an error if a Mango VDU contains invalid top-level fields

Author: jcoglan

src/mango/src/mango_native_proc.erl

@@ -112,20 +112,27 @@ handle_call({prompt, [<<"ddoc">>, DDocId, [<<"validate_doc_update">>], Args]}, _
             Msg = [<<"validate_doc_update">>, DDocId],
             {stop, {invalid_call, Msg}, {invalid_call, Msg}, St};
         Selector ->
-            [NewDoc, OldDoc, _Ctx, _SecObj] = Args,
-            Struct =
-                case OldDoc of
-                    null -> {[{<<"newDoc">>, NewDoc}]};
-                    Doc -> {[{<<"newDoc">>, NewDoc}, {<<"oldDoc">>, Doc}]}
-                end,
-            Reply =
-                case mango_selector:match_failures(Selector, Struct) of
-                    [] ->
-                        true;
-                    Failures ->
-                        {[{<<"forbidden">>, {[{<<"failures">>, Failures}]}}]}
-                end,
-            {reply, Reply, St}
+            case mango_selector:has_allowed_fields(Selector, [<<"newDoc">>, <<"oldDoc">>]) of
+                false ->
+                    Msg =
+                        <<"'validate_doc_update' may only contain 'newDoc' and 'oldDoc' as top-level fields">>,
+                    {stop, {invalid_call, Msg}, {invalid_call, Msg}, St};
+                true ->
+                    [NewDoc, OldDoc, _Ctx, _SecObj] = Args,
+                    Struct =
+                        case OldDoc of
+                            null -> {[{<<"newDoc">>, NewDoc}]};
+                            Doc -> {[{<<"newDoc">>, NewDoc}, {<<"oldDoc">>, Doc}]}
+                        end,
+                    Reply =
+                        case mango_selector:match_failures(Selector, Struct) of
+                            [] ->
+                                true;
+                            Failures ->
+                                {[{<<"forbidden">>, {[{<<"failures">>, Failures}]}}]}
+                        end,
+                    {reply, Reply, St}
+            end
     end;
 handle_call(Msg, _From, St) ->
     {stop, {invalid_call, Msg}, {invalid_call, Msg}, St}.

src/mango/src/mango_selector.erl

@@ -17,6 +17,7 @@
     match/2,
     match_failures/2,
     has_required_fields/2,
+    has_allowed_fields/2,
     is_constant_field/2,
     fields/1
 ]).
@@ -883,6 +884,39 @@ has_required_fields_int([{[{Field, Cond}]} | Rest], RequiredFields) ->
             has_required_fields_int(Rest, lists:delete(Field, RequiredFields))
     end.
 
+has_allowed_fields(Selector, AllowedFields) ->
+    Paths = lists:map(
+        fun(Field) ->
+            {ok, Path} = mango_util:parse_field(Field),
+            Path
+        end,
+        AllowedFields
+    ),
+    has_allowed_fields_int(Selector, Paths).
+
+has_allowed_fields_int({[{Field, Cond}]}, Paths) when is_list(Field) ->
+    Stemmed = [match_prefix(Field, Path) || Path <- Paths],
+    Matched = [Path || Path <- Stemmed, Path /= nil],
+    case Matched of
+        [] -> false;
+        M -> has_allowed_fields_int(Cond, M)
+    end;
+has_allowed_fields_int({[{_Op, Conds}]}, Paths) when is_list(Conds) ->
+    lists:all(fun(Cond) -> has_allowed_fields_int(Cond, Paths) end, Conds);
+has_allowed_fields_int({[{_Op, Cond}]}, Paths) ->
+    has_allowed_fields_int(Cond, Paths);
+has_allowed_fields_int(_, _) ->
+    true.
+
+match_prefix([A | Rest1], [A | Rest2]) ->
+    match_prefix(Rest1, Rest2);
+match_prefix([], Rest) ->
+    Rest;
+match_prefix(_, []) ->
+    [];
+match_prefix(_, _) ->
+    nil.
+
 % Returns true if a field in the selector is a constant value e.g. {a: {$eq: 1}}
 is_constant_field(Selector, Field) when not is_list(Field) ->
     {ok, Path} = mango_util:parse_field(Field),
@@ -1262,6 +1296,23 @@ has_required_fields_or_nested_or_false_test() ->
     Normalized = normalize(Selector),
     ?assertEqual(false, has_required_fields(Normalized, RequiredFields)).
 
+has_allowed_fields_test() ->
+    Sel1 = normalize({[{<<"a">>, 1}]}),
+    ?assertEqual(has_allowed_fields(Sel1, [<<"a">>]), true),
+    ?assertEqual(has_allowed_fields(Sel1, [<<"a">>, <<"b">>]), true),
+    ?assertEqual(has_allowed_fields(Sel1, [<<"b">>]), false),
+
+    Sel2 = normalize({[{<<"$or">>, [{[{<<"a.b">>, 1}]}, {[{<<"c.d">>, 2}]}]}]}),
+    ?assertEqual(has_allowed_fields(Sel2, [<<"a">>, <<"c">>]), true),
+    ?assertEqual(has_allowed_fields(Sel2, [<<"a.b">>, <<"c.d">>]), true),
+    ?assertEqual(has_allowed_fields(Sel2, [<<"a">>]), false),
+
+    Sel3 = normalize({[{<<"a">>, {[{<<"$or">>, [{[{<<"b">>, 1}]}, {[{<<"c">>, 2}]}]}]}}]}),
+    ?assertEqual(has_allowed_fields(Sel3, [<<"a">>]), true),
+    ?assertEqual(has_allowed_fields(Sel3, [<<"a.b">>, <<"a.c">>]), true),
+    ?assertEqual(has_allowed_fields(Sel3, [<<"a.c">>]), false),
+    ?assertEqual(has_allowed_fields(Sel3, [<<"b">>]), false).
+
 check_match(Selector) ->
     % Call match_int/2 to avoid ERROR for missing metric; this is confusing
     % in the middle of test output.

test/elixir/test/config/suite.elixir

@@ -532,6 +532,7 @@
     "converting a Mango VDU to JavaScript updates its effects",
     "deleting a Mango VDU removes its effects",
     "Mango VDU rejects a doc if any existing ddoc fails to match",
+    "Mango VDU rejects a design doc if it contains unknown fields",
   ],
   "SecurityValidationTest": [
     "Author presence and user security",

test/elixir/test/validate_doc_update_test.exs

@@ -214,4 +214,22 @@ defmodule ValidateDocUpdateTest do
     assert resp.status_code == 403
     assert resp.body["error"] == "forbidden"
   end
+
+  @tag :with_db
+  test "Mango VDU rejects a design doc if it contains unknown fields", context do
+    db = context[:db_name]
+
+    ddoc = %{
+      language: "query",
+
+      validate_doc_update: %{
+        "wrongField" => %{"year" => %{"$lt" => 2026}}
+      }
+    }
+    resp = Couch.put("/#{db}/_design/mango-test-2", body: ddoc)
+    assert resp.status_code == 201
+
+    resp = Couch.put("/#{db}/doc", body: %{"year" => 1994})
+    assert resp.status_code == 500
+  end
 end