fix: reject copy/move destinations in the reserved .da-versions folder

src/helpers/copy.js

@@ -9,6 +9,8 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
+import { hasReservedSegment } from '../storage/version/paths.js';
+
 const NO_DEST_ERROR = {
   body: JSON.stringify({ error: 'No destination provided.' }),
   status: 400,
@@ -24,6 +26,11 @@ const CROSS_ORG_ERROR = {
   status: 400,
 };
 
+const RESERVED_DEST_ERROR = {
+  body: JSON.stringify({ error: 'Invalid or reserved destination.' }),
+  status: 400,
+};
+
 export default async function copyHelper(req, daCtx) {
   let formData;
   try {
@@ -43,6 +50,10 @@ export default async function copyHelper(req, daCtx) {
   if (destOrg !== daCtx.org) return { error: CROSS_ORG_ERROR };
 
   const destination = destParts.join('/');
+
+  // Reject destinations inside the reserved .da-versions folder
+  if (hasReservedSegment(destination)) return { error: RESERVED_DEST_ERROR };
+
   const source = daCtx.key;
   return { source, destination, continuationToken };
 }

src/helpers/move.js

@@ -10,6 +10,8 @@
  * governing permissions and limitations under the License.
  */
 
+import { hasReservedSegment } from '../storage/version/paths.js';
+
 const NO_DEST_ERROR = {
   body: JSON.stringify({ error: 'No destination provided.' }),
   status: 400,
@@ -25,6 +27,11 @@ const CROSS_ORG_ERROR = {
   status: 400,
 };
 
+const RESERVED_DEST_ERROR = {
+  body: JSON.stringify({ error: 'Invalid or reserved destination.' }),
+  status: 400,
+};
+
 export default async function moveHelper(req, daCtx) {
   try {
     const formData = await req.formData();
@@ -39,6 +46,10 @@ export default async function moveHelper(req, daCtx) {
     if (destOrg !== daCtx.org) return { error: CROSS_ORG_ERROR };
 
     let destination = destParts.join('/');
+
+    // Reject destinations inside the reserved .da-versions folder
+    if (hasReservedSegment(destination)) return { error: RESERVED_DEST_ERROR };
+
     const source = daCtx.key;
 
     // Ensure destination is not child of source

src/storage/object/copy.js

@@ -21,12 +21,21 @@ import { putObjectWithVersion } from '../version/put.js';
 import { getUsersForMetadata } from '../utils/version.js';
 import { listCommand } from '../utils/list.js';
 import { hasPermission } from '../../utils/auth.js';
+import { hasReservedSegment } from '../version/paths.js';
 
 const MAX_KEYS = 900;
 
 export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) => {
   const Key = sourceKey.replace(details.source, details.destination);
 
+  // A folder copy derives many keys from one request. Reject any that lands in
+  // the reserved .da-versions folder, whatever the caller's grants and whatever
+  // the source looks like. Version storage is only reachable through the
+  // ACL-aware version routes, so no copy or move may write into it.
+  if (hasReservedSegment(Key)) {
+    return { $metadata: { httpStatusCode: 400 } };
+  }
+
   if (!hasPermission(daCtx, sourceKey, 'read') || !hasPermission(daCtx, Key, 'write')) {
     return {
       $metadata: {

src/storage/version/paths.js

@@ -91,3 +91,14 @@ export function auditArchiveKey(repo, fileId, timestamp) {
 export function auditDirPrefix(repo, fileId) {
   return `${repo}/.da-versions/${fileId}/audit`;
 }
+
+/**
+ * True when any path segment is the reserved .da-versions folder. Keeps
+ * user-controlled destinations and keys out of version storage, which is
+ * only reachable through the ACL-aware version routes.
+ * @param {string} path org-stripped key or copy/move destination
+ * @returns {boolean}
+ */
+export function hasReservedSegment(path) {
+  return typeof path === 'string' && path.split('/').includes('.da-versions');
+}

test/it/it-tests.js

@@ -613,6 +613,44 @@ export default (ctx) => describe('Integration Tests: it tests', function () {
     assert.strictEqual(resp.status, 200, `Expected 200 OK, got ${resp.status} - user: ${superUser.email}`);
   });
 
+  it('[super user] cannot copy or move into the reserved .da-versions folder', async () => {
+    // The generic copy/move routes take their destination from the request body.
+    // A destination inside {repo}/.da-versions/... lands in version and audit
+    // storage and forges a document's history. The destination guard must reject
+    // it with 400, whatever the caller's grants.
+    const {
+      serverUrl, org, repo, superUser,
+    } = ctx;
+
+    const copyForm = new FormData();
+    copyForm.append('destination', `/${org}/${repo}/.da-versions/forge-target/audit-9999999999.txt`);
+    let resp = await fetch(`${serverUrl}/copy/${org}/${repo}/test-folder/page1.html`, {
+      method: 'POST',
+      body: copyForm,
+      headers: { Authorization: `Bearer ${superUser.accessToken}` },
+    });
+    assert.strictEqual(resp.status, 400, `Expected 400 from the destination guard on copy, got ${resp.status} - user: ${superUser.email}`);
+    let body = await resp.json();
+    assert.match(body.error, /invalid or reserved/i, `Expected reserved-destination error, got ${body.error}`);
+
+    const moveForm = new FormData();
+    moveForm.append('destination', `/${org}/${repo}/.da-versions/forge-target/0000.html`);
+    resp = await fetch(`${serverUrl}/move/${org}/${repo}/test-folder/page1-copy.html`, {
+      method: 'POST',
+      body: moveForm,
+      headers: { Authorization: `Bearer ${superUser.accessToken}` },
+    });
+    assert.strictEqual(resp.status, 400, `Expected 400 from the destination guard on move, got ${resp.status} - user: ${superUser.email}`);
+    body = await resp.json();
+    assert.match(body.error, /invalid or reserved/i, `Expected reserved-destination error, got ${body.error}`);
+
+    // the blocked move must leave the source in place
+    resp = await fetch(`${serverUrl}/source/${org}/${repo}/test-folder/page1-copy.html`, {
+      headers: { Authorization: `Bearer ${superUser.accessToken}` },
+    });
+    assert.strictEqual(resp.status, 200, `Expected 200 OK, got ${resp.status} - user: ${superUser.email}`);
+  });
+
   it('[anonymous] cannot delete an object', async () => {
     const {
       serverUrl, org, repo, key,

test/routes/copy.test.js

@@ -181,4 +181,61 @@ describe('Copy Route', () => {
     assert.strictEqual(400, resp.status);
     assert.strictEqual(copyCalled.length, 0);
   });
+
+  it('Test copyHandler returns 400 when destination is in the reserved .da-versions folder', async () => {
+    const copyCalled = [];
+    const copyObject = (e, c, d, m) => {
+      copyCalled.push({
+        e, c, d, m,
+      });
+      return { status: 200 };
+    };
+
+    const copyHandler = await esmock('../../src/routes/copy.js', {
+      '../../src/storage/object/copy.js': {
+        default: copyObject,
+      },
+      '../../src/utils/auth.js': { hasPermission: () => true },
+    });
+
+    const formdata = new Map();
+    formdata.set('destination', '/myorg/my/.da-versions/1234/audit-9999999999.txt');
+    const req = {
+      formData: () => formdata,
+    };
+
+    const resp = await copyHandler({ req, env: {}, daCtx: { org: 'myorg', key: 'my/decoy.html' } });
+    assert.strictEqual(resp.status, 400);
+    assert.strictEqual(copyCalled.length, 0);
+    const body = JSON.parse(resp.body);
+    assert.match(body.error, /invalid or reserved/i);
+  });
+
+  it('Test copyHandler allows a destination segment that merely contains da-versions', async () => {
+    const copyCalled = [];
+    const copyObject = (e, c, d, m) => {
+      copyCalled.push({
+        e, c, d, m,
+      });
+      return { status: 200 };
+    };
+
+    const copyHandler = await esmock('../../src/routes/copy.js', {
+      '../../src/storage/object/copy.js': {
+        default: copyObject,
+      },
+      '../../src/utils/auth.js': { hasPermission: () => true },
+    });
+
+    const formdata = new Map();
+    formdata.set('destination', '/myorg/my/my-da-versions-notes.html');
+    const req = {
+      formData: () => formdata,
+    };
+
+    const resp = await copyHandler({ req, env: {}, daCtx: { org: 'myorg', key: 'my/src.html' } });
+    assert.strictEqual(resp.status, 200);
+    assert.strictEqual(copyCalled.length, 1);
+    assert.strictEqual(copyCalled[0].d.destination, 'my/my-da-versions-notes.html');
+  });
 });

test/routes/move.test.js

@@ -123,4 +123,61 @@ describe('Move Route', () => {
     assert.strictEqual(1, moCalled.length);
     assert.strictEqual('somedest', moCalled[0].d.destination);
   });
+
+  it('Test moveRoute returns 400 when destination is in the reserved .da-versions folder', async () => {
+    const moCalled = [];
+    const moveObject = (e, c, d) => {
+      moCalled.push({ e, c, d });
+      return { status: 200 };
+    };
+
+    const moveRoute = await esmock('../../src/routes/move.js', {
+      '../../src/storage/object/move.js': {
+        default: moveObject,
+      },
+      '../../src/utils/auth.js': {
+        hasPermission: () => true,
+      },
+    });
+
+    const formdata = new Map();
+    formdata.set('destination', '/someorg/my/.da-versions/1234/audit-9999999999.txt');
+    const req = {
+      formData: () => formdata,
+    };
+
+    const resp = await moveRoute({ req, env: {}, daCtx: { org: 'someorg', key: 'my/decoy.html' } });
+    assert.strictEqual(resp.status, 400);
+    assert.strictEqual(0, moCalled.length);
+    const body = JSON.parse(resp.body);
+    assert.match(body.error, /invalid or reserved/i);
+  });
+
+  it('Test moveRoute allows a destination segment that merely contains da-versions', async () => {
+    const moCalled = [];
+    const moveObject = (e, c, d) => {
+      moCalled.push({ e, c, d });
+      return { status: 204 };
+    };
+
+    const moveRoute = await esmock('../../src/routes/move.js', {
+      '../../src/storage/object/move.js': {
+        default: moveObject,
+      },
+      '../../src/utils/auth.js': {
+        hasPermission: () => true,
+      },
+    });
+
+    const formdata = new Map();
+    formdata.set('destination', '/someorg/my/my-da-versions-notes.html');
+    const req = {
+      formData: () => formdata,
+    };
+
+    const resp = await moveRoute({ req, env: {}, daCtx: { org: 'someorg', key: 'abc.html' } });
+    assert.strictEqual(resp.status, 204);
+    assert.strictEqual(1, moCalled.length);
+    assert.strictEqual(moCalled[0].d.destination, 'my/my-da-versions-notes.html');
+  });
 });

test/storage/object/copy.test.js

@@ -89,6 +89,89 @@ describe('Object copy', () => {
     assert.strictEqual(resp.$metadata.httpStatusCode, 403);
   });
 
+  it('returns 400 without sending a copy when the destination Key is in the reserved .da-versions folder', async () => {
+    const s3Sent = [];
+    const mockS3Client = class {
+      // eslint-disable-next-line class-methods-use-this
+      send(command) {
+        s3Sent.push(command);
+        return { $metadata: { httpStatusCode: 200 } };
+      }
+
+      middlewareStack = { add: () => {} };
+    };
+
+    const mockGetObject = async () => ({ contentType: 'text/html', status: 200 });
+
+    // eslint-disable-next-line no-shadow
+    const { copyFile } = await esmock('../../../src/storage/object/copy.js', {
+      '@aws-sdk/client-s3': {
+        S3Client: mockS3Client,
+      },
+      '../../../src/storage/object/get.js': {
+        default: mockGetObject,
+      },
+      '../../../src/utils/auth.js': { hasPermission: () => true },
+    });
+
+    const env = { dacollab: { fetch: () => ({ body: { cancel: () => {} } }) } };
+    const daCtx = {
+      bucket: 'root-bucket',
+      org: 'myorg',
+      origin: 'https://test.com',
+      users: [{ email: 'test@example.com' }],
+    };
+    const details = { source: 'mysrc', destination: 'my/.da-versions/1234' };
+
+    const resp = await copyFile({}, env, daCtx, 'mysrc/audit-9999999999.txt', details, false);
+    assert.strictEqual(resp.$metadata.httpStatusCode, 400);
+    assert.strictEqual(s3Sent.length, 0);
+  });
+
+  it('blocks a .da-versions destination even when the source is already under .da-versions', async () => {
+    // No source-side exemption. A repo-level copy or move must not relocate a
+    // .da-versions object into another .da-versions location. Otherwise an
+    // attacker who gets crafted content into any .da-versions (through a separate
+    // write hole) could plant it in a victim's version history. The guard rejects
+    // the reserved destination whatever the source looks like.
+    const s3Sent = [];
+    const mockS3Client = class {
+      // eslint-disable-next-line class-methods-use-this
+      send(command) {
+        s3Sent.push(command);
+        return { $metadata: { httpStatusCode: 200 } };
+      }
+
+      middlewareStack = { add: () => {} };
+    };
+
+    const mockGetObject = async () => ({ contentType: 'text/html', status: 200 });
+
+    // eslint-disable-next-line no-shadow
+    const { copyFile } = await esmock('../../../src/storage/object/copy.js', {
+      '@aws-sdk/client-s3': {
+        S3Client: mockS3Client,
+      },
+      '../../../src/storage/object/get.js': {
+        default: mockGetObject,
+      },
+      '../../../src/utils/auth.js': { hasPermission: () => true },
+    });
+
+    const env = { dacollab: { fetch: () => ({ body: { cancel: () => {} } }) } };
+    const daCtx = {
+      bucket: 'root-bucket',
+      org: 'myorg',
+      origin: 'https://test.com',
+      users: [{ email: 'test@example.com' }],
+    };
+    const details = { source: 'oldrepo', destination: 'newrepo' };
+
+    const resp = await copyFile({}, env, daCtx, 'oldrepo/.da-versions/fid1/v1.html', details, true);
+    assert.strictEqual(resp.$metadata.httpStatusCode, 400);
+    assert.strictEqual(s3Sent.length, 0);
+  });
+
   it('Copy to location with permission', async () => {
     const pathLookup = new Map();
     pathLookup.set('aaa@bbb.ccc', [

test/storage/version/paths.test.js

@@ -17,6 +17,7 @@ import {
   auditDirPrefix,
   isValidId,
   isSafeId,
+  hasReservedSegment,
 } from '../../../src/storage/version/paths.js';
 
 describe('Version Paths', () => {
@@ -136,4 +137,24 @@ describe('Version Paths', () => {
       assert.strictEqual(isSafeId(null), false);
     });
   });
+
+  describe('hasReservedSegment', () => {
+    it('matches the reserved folder as any path segment', () => {
+      assert.strictEqual(hasReservedSegment('repo/.da-versions/fid/audit.txt'), true);
+      assert.strictEqual(hasReservedSegment('.da-versions/fid/v1.html'), true);
+      assert.strictEqual(hasReservedSegment('a/b/.da-versions'), true);
+    });
+
+    it('does not match a segment that merely contains the name', () => {
+      assert.strictEqual(hasReservedSegment('repo/my-da-versions-notes.html'), false);
+      assert.strictEqual(hasReservedSegment('repo/.da-versions-backup/x'), false);
+      assert.strictEqual(hasReservedSegment('repo/foo.da-versions'), false);
+      assert.strictEqual(hasReservedSegment('repo/page1.html'), false);
+    });
+
+    it('is safe for non-string input', () => {
+      assert.strictEqual(hasReservedSegment(undefined), false);
+      assert.strictEqual(hasReservedSegment(null), false);
+    });
+  });
 });