diff --git a/src/helpers/copy.js b/src/helpers/copy.js index d984278f..13d8ab8c 100644 --- a/src/helpers/copy.js +++ b/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 }; } diff --git a/src/helpers/move.js b/src/helpers/move.js index 8f3c6f01..ff7ae004 100644 --- a/src/helpers/move.js +++ b/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 diff --git a/src/storage/object/copy.js b/src/storage/object/copy.js index fb4fc83f..322bfa24 100644 --- a/src/storage/object/copy.js +++ b/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: { diff --git a/src/storage/version/paths.js b/src/storage/version/paths.js index fd30d191..10ef161d 100644 --- a/src/storage/version/paths.js +++ b/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'); +} diff --git a/test/it/it-tests.js b/test/it/it-tests.js index 5dd71401..f5eda1a8 100644 --- a/test/it/it-tests.js +++ b/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, diff --git a/test/routes/copy.test.js b/test/routes/copy.test.js index 97d54d90..6b2c8684 100644 --- a/test/routes/copy.test.js +++ b/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'); + }); }); diff --git a/test/routes/move.test.js b/test/routes/move.test.js index 5cf8d096..f85fc6e1 100644 --- a/test/routes/move.test.js +++ b/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'); + }); }); diff --git a/test/storage/object/copy.test.js b/test/storage/object/copy.test.js index 5a73a0a7..d755b408 100644 --- a/test/storage/object/copy.test.js +++ b/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', [ diff --git a/test/storage/version/paths.test.js b/test/storage/version/paths.test.js index d7dd6075..00ce068d 100644 --- a/test/storage/version/paths.test.js +++ b/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); + }); + }); });