feat: Prevent implicit document creation via If-Match header

src/edge.js

@@ -138,17 +138,17 @@ export async function handleApiRequest(request, env) {
     const timingBeforeDaAdminHead = Date.now();
     const initialReq = await env.daadmin.fetch(docName, opts);
 
-    // this seems to be required by CloudFlare to consider the request as completed
-    await initialReq.text();
-
     timingDaAdminHeadDuration = Date.now() - timingBeforeDaAdminHead;
 
-    if (!initialReq.ok && initialReq.status !== 404) {
+    if (!initialReq.ok) {
       // eslint-disable-next-line no-console
       console.log(`[worker] Unable to get resource ${docName}: ${initialReq.status} - ${initialReq.statusText}`);
       return new Response('unable to get resource', { status: initialReq.status });
     }
 
+    // this seems to be required by CloudFlare to consider the request as completed
+    await initialReq.text();
+
     const daActions = initialReq.headers.get('X-da-actions') ?? '';
     [, authActions] = daActions.split('=');
   } catch (err) {

src/shareddoc.js

@@ -16,7 +16,7 @@ import * as awarenessProtocol from 'y-protocols/awareness.js';
 import * as encoding from 'lib0/encoding.js';
 import * as decoding from 'lib0/decoding.js';
 import debounce from 'lodash/debounce.js';
-import { aem2doc, doc2aem, EMPTY_DOC } from './collab.js';
+import { aem2doc, doc2aem } from './collab.js';
 
 const wsReadyStateConnecting = 0;
 const wsReadyStateOpen = 1;
@@ -191,12 +191,12 @@ export const persistence = {
   closeConn: closeConn.bind(this),
 
   /**
-   * Get the document from da-admin. If da-admin doesn't have the doc, a new empty doc is
-   * returned.
+   * Get the document from da-admin.
    * @param {string} docName - The document name
    * @param {string} auth - The authorization header
    * @param {object} daadmin - The da-admin worker service binding
    * @returns {Promise<string>} - The content of the document
+   * @throws {Error} - If the document cannot be retrieved (including 404)
    */
   get: async (docName, auth, daadmin) => {
     const initalOpts = {};
@@ -206,8 +206,6 @@ export const persistence = {
     const initialReq = await daadmin.fetch(docName, initalOpts);
     if (initialReq.ok) {
       return initialReq.text();
-    } else if (initialReq.status === 404) {
-      return null;
     } else {
       // eslint-disable-next-line no-console
       console.error(`[docroom] Unable to get resource from da-admin: ${initialReq.status} - ${initialReq.statusText}`);
@@ -236,17 +234,22 @@ export const persistence = {
       console.log('[docroom] All connections are read only, not storing');
       return { ok: true };
     }
+
+    const headers = {
+      'If-Match': '*',
+      'X-DA-Initiator': 'collab',
+    };
+
     const auth = keys
       .filter((con) => con.readOnly !== true)
       .map((con) => con.auth);
 
     if (auth.length > 0) {
-      opts.headers = new Headers({
-        Authorization: [...new Set(auth)].join(','),
-        'X-DA-Initiator': 'collab',
-      });
+      headers.Authorization = [...new Set(auth)].join(',');
     }
 
+    opts.headers = new Headers(headers);
+
     if (blob.size < 84) {
       // eslint-disable-next-line no-console
       console.warn('[docroom] Writting back an empty document', ydoc.name, blob.size);
@@ -284,7 +287,20 @@ export const persistence = {
         const { ok, status, statusText } = await persistence.put(ydoc, content);
 
         if (!ok) {
-          closeAll = (status === 401 || status === 403);
+          if (status === 412) {
+            // Document doesn't exist - clean up cached state
+            if (ydoc.storage) {
+              try {
+                await ydoc.storage.deleteAll();
+                // eslint-disable-next-line no-console
+                console.log('[docroom] Cleaned worker storage after 412 (document deleted)');
+              } catch (storageErr) {
+                // eslint-disable-next-line no-console
+                console.error('[docroom] Failed to clean storage', storageErr);
+              }
+            }
+          }
+          closeAll = (status === 401 || status === 403 || status === 412);
           throw new Error(`${status} - ${statusText}`);
         }
 
@@ -312,31 +328,26 @@ export const persistence = {
    */
   bindState: async (docName, ydoc, conn, storage) => {
     let timingReadStateDuration;
-    let timingDaAdminGetDuration;
+
+    // Store storage reference for later use in persistence.update
+    // eslint-disable-next-line no-param-reassign
+    ydoc.storage = storage;
 
     let current;
     let restored = false; // True if restored from worker storage
-    try {
-      let newDoc = false;
-      const timingBeforeDaAdminGet = Date.now();
-      current = await persistence.get(docName, conn.auth, ydoc.daadmin);
-      timingDaAdminGetDuration = Date.now() - timingBeforeDaAdminGet;
 
+    // Get document from da-admin (throws on error including 404)
+    const timingBeforeDaAdminGet = Date.now();
+    current = await persistence.get(docName, conn.auth, ydoc.daadmin);
+    const timingDaAdminGetDuration = Date.now() - timingBeforeDaAdminGet;
+
+    // Read the stored state from internal worker storage (errors are non-fatal)
+    try {
       const timingBeforeReadState = Date.now();
-      // Read the stored state from internal worker storage
       const stored = await readState(docName, storage);
       timingReadStateDuration = Date.now() - timingBeforeReadState;
 
-      if (current === null) {
-        if (!stored) {
-          // This is a new document, it wasn't present in local storage
-          newDoc = true;
-        }
-        // if stored has a value, the document previously existed but was deleted
-
-        current = EMPTY_DOC;
-        await storage.deleteAll();
-      } else if (stored && stored.length > 0) {
+      if (stored && stored.length > 0) {
         Y.applyUpdate(ydoc, stored);
 
         // Check if the state from the worker storage is the same as the current state in da-admin.
@@ -350,12 +361,6 @@ export const persistence = {
           console.log('[docroom] Restored from worker persistence', docName);
         }
       }
-
-      if (newDoc === true) {
-        // There is no stored state and the document is empty, which means
-        // we have a new doc here, which doesn't need to be restored from da-admin
-        restored = true;
-      }
     } catch (error) {
       // eslint-disable-next-line no-console
       console.error('[docroom] Problem restoring state from worker storage', error);

test/edge.test.js

@@ -294,6 +294,47 @@ describe('Worker test suite', () => {
     assert.equal(400, resp.status, 'Expected a document name');
   });
 
+  it('Test DocRoom fetch fails when document deleted after auth', async () => {
+    const savedNWSP = DocRoom.newWebSocketPair;
+    const savedBS = persistence.bindState;
+
+    try {
+      // Mock bindState to throw 404 error (simulating document deleted between auth and bindState)
+      persistence.bindState = async () => {
+        throw new Error('unable to get resource - status: 404');
+      };
+
+      const wsp0 = {};
+      const wsp1 = {
+        accept() {},
+        addEventListener() {},
+        close() {}
+      };
+      DocRoom.newWebSocketPair = () => [wsp0, wsp1];
+
+      const daadmin = { fetch: async () => ({ ok: true }) };
+      const dr = new DocRoom({ storage: null }, { daadmin });
+      const headers = new Map();
+      headers.set('Upgrade', 'websocket');
+      headers.set('X-collab-room', 'http://foo.bar/test.html');
+      headers.set('X-auth-actions', 'read=allow,write=allow');
+
+      const req = {
+        headers,
+        url: 'http://localhost:4711/'
+      };
+
+      const resp = await dr.fetch(req, {}, 306);
+
+      // Should return 500 error when bindState fails
+      assert.equal(500, resp.status);
+      assert.equal('internal server error', await resp.text());
+    } finally {
+      DocRoom.newWebSocketPair = savedNWSP;
+      persistence.bindState = savedBS;
+    }
+  });
+
   it('Test DocRoom fetch WebSocket setup exception', async () => {
     const savedNWSP = DocRoom.newWebSocketPair;
     const savedBS = persistence.bindState;
@@ -479,6 +520,20 @@ describe('Worker test suite', () => {
     assert.equal(404, res.status);
   });
 
+  it('Test handleApiRequest document not found (404)', async () => {
+    const req = {
+      url: 'http://do.re.mi/https://admin.da.live/nonexistent.html',
+    }
+
+    const mockFetch = async (url, opts) => new Response(null, {status: 404});
+    const daadmin = { fetch: mockFetch };
+    const env = { daadmin };
+
+    const res = await handleApiRequest(req, env);
+    assert.equal(404, res.status);
+    assert.equal('unable to get resource', await res.text());
+  });
+
   it('Test handleApiRequest not authorized', async () => {
     const req = {
       url: 'http://do.re.mi/https://admin.da.live/hihi.html',