Fix REST endpoint /set allowing clients to set serverOnlyFields

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bug Fixes
 
+- Fix REST endpoint `/set` allowing clients to populate `serverOnlyFields` when inserting NEW documents. Updates to existing documents already stripped client-supplied values for these fields, but the insert path passed the client document straight to `RxCollection.insert()`, so a client could persist arbitrary values into fields that are documented as server-only. The handler now strips server-only fields from the client document before inserting, matching the documented contract that clients cannot do writes where one of the `serverOnlyFields` is set.
 - Fix replication pull URL not URL-encoding the checkpoint `id`. When a document's primary key contained URL-reserved characters (for example `&`, `#`, `=`), the URL was parsed incorrectly on the server, causing the checkpoint to be truncated. With `batchSize: 1` this could make the pull loop never advance past such a document. The client now encodes the `id` with `encodeURIComponent`.
 - Fix REST endpoint `/set` not protecting `serverOnlyFields` from client overwrites. Clients could include server-only fields in write requests to `/set`, and those values would be stored directly instead of being ignored. The handler now uses `mergeServerDocumentFields` (consistent with the replication endpoint) to ensure server-only field values are always preserved from the server-side document, not taken from client input.
 - Fix missing `await` in `RxRestClient.get()`, `RxRestClient.set()`, and `RxRestClient.delete()` methods. The `postRequest()` call was not awaited before calling `handleError()`, which caused server errors (e.g. 403 Forbidden from `changeValidator`) to be silently swallowed instead of thrown to the caller.

src/plugins/server/endpoint-rest.ts

@@ -23,7 +23,8 @@ import {
     getDocAllowedMatcher,
     mergeServerDocumentFieldsMonad,
     removeServerOnlyFieldsMonad,
-    setCors
+    setCors,
+    stripServerOnlyFieldsMonad
 } from './helper.ts';
 
 
@@ -84,6 +85,7 @@ export class RxServerRestEndpoint<ServerAppType, AuthType, RxDocType> implements
         }
         const removeServerOnlyFields = removeServerOnlyFieldsMonad(this.serverOnlyFields);
         const mergeServerDocumentFields = mergeServerDocumentFieldsMonad<RxDocType>(this.serverOnlyFields);
+        const stripServerOnlyFields = stripServerOnlyFieldsMonad<RxDocType>(this.serverOnlyFields);
 
         this.server.adapter.post(this.server.serverApp, '/' + this.urlPath + '/query', async (req, res) => {
             ensureNotFalsy(adapter.getRequestBody(req), 'req body is empty');
@@ -219,7 +221,15 @@ export class RxServerRestEndpoint<ServerAppType, AuthType, RxDocType> implements
                     const id = (docData as any)[primaryPath];
                     const doc = docs.get(id);
                     if (!doc) {
-                        const mergedDocData = mergeServerDocumentFields(docData, undefined);
+                        // Strip server-only fields from the client doc before
+                        // inserting. Without this, a client could populate
+                        // server-only ("readonly") fields when creating a new
+                        // document via /set, which contradicts the documented
+                        // behavior that clients cannot do writes where one of
+                        // the serverOnlyFields is set.
+                        const mergedDocData = stripServerOnlyFields(
+                            mergeServerDocumentFields(docData, undefined) as RxDocType
+                        );
                         promises.push(this.collection.insert(mergedDocData).catch(err => onWriteError(err, mergedDocData)));
                     } else {
                         const isAllowed = this.changeValidator(authData, {

src/plugins/server/helper.ts

@@ -86,6 +86,28 @@ export function docContainsServerOnlyFields(
     return has;
 }
 
+/**
+ * Returns a function that strips the given server-only fields from a client
+ * document. This is used when accepting a NEW document from a client write to
+ * make sure that server-only fields cannot be populated by the client.
+ *
+ * Unlike {@link removeServerOnlyFieldsMonad} this does not assign undefined to
+ * RxDB internal meta fields (`_meta`, `_rev`, `_attachments`) so the result can
+ * be safely passed to `RxCollection.insert()`.
+ */
+export function stripServerOnlyFieldsMonad<RxDocType>(serverOnlyFields: string[]) {
+    if (serverOnlyFields.length === 0) {
+        return (docData: RxDocType | RxDocumentData<RxDocType>) => docData;
+    }
+    return (docData: RxDocType | RxDocumentData<RxDocType>) => {
+        const ret: any = flatClone(docData);
+        for (let i = 0; i < serverOnlyFields.length; i++) {
+            delete ret[serverOnlyFields[i]];
+        }
+        return ret;
+    };
+}
+
 export function removeServerOnlyFieldsMonad<RxDocType>(serverOnlyFields: string[]) {
     const serverOnlyFieldsStencil: any = {
         _meta: undefined,

test/unit/endpoint-rest.test.ts

@@ -14,7 +14,8 @@ import {
 import {
     schemaObjects,
     humansCollection,
-    HumanDocumentType
+    HumanDocumentType,
+    humanDefault
 } from 'rxdb/plugins/test-utils';
 import { nextPort } from './test-helpers.ts';
 import { assertThrows, waitUntil } from 'async-test-util';
@@ -735,6 +736,44 @@ describe('endpoint-rest.test.ts', () => {
 
             await col.database.close();
         });
+        it('should not allow clients to set serverOnlyFields when inserting NEW documents via /set', async () => {
+            // Use humanDefault schema where lastName is optional so the insert
+            // with lastName stripped is still schema-valid.
+            const col = await humansCollection.createBySchema(humanDefault);
+            const port = await nextPort();
+            const server = await createRxServer({
+                adapter: TEST_SERVER_ADAPTER,
+                database: col.database,
+                authHandler,
+                port
+            });
+            const endpoint = await server.addRestEndpoint({
+                name: randomToken(10),
+                collection: col,
+                serverOnlyFields: ['lastName']
+            });
+            await server.start();
+
+            const client = createRestClient<HumanDocumentType>('http://localhost:' + port + '/' + endpoint.urlPath, headers);
+
+            // The client crafts a NEW document with the server-only field set.
+            const newDoc: HumanDocumentType = schemaObjects.humanData('new-doc-restset', 1);
+            newDoc.lastName = 'HackedValue';
+
+            await client.set([newDoc]);
+
+            // The server-only field must NOT have been written with the
+            // client-provided value when the document is created.
+            const docAfter = await col.findOne('new-doc-restset').exec(true);
+            assert.strictEqual(docAfter.firstName, newDoc.firstName);
+            assert.notStrictEqual(
+                docAfter.lastName,
+                'HackedValue',
+                'Client must not be able to set a server-only field on document creation via /set'
+            );
+
+            await col.database.close();
+        });
         it('should not allow clients to overwrite serverOnlyFields via /set', async () => {
             const col = await humansCollection.create(1);
             const doc = await col.findOne().exec(true);