Skip to content

fix: reject cross-org copy/move destinations instead of misdelivering #301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kptdobe merged 2 commits into from
Jul 14, 2026
+212 −10
Merged

fix: reject cross-org copy/move destinations instead of misdelivering #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed0480f
fix: reject cross-org copy/move destinations instead of misdelivering
benpeter Jul 10, 2026
3eadb26
style(copy): align cross-org guard formatting with move helper
benpeter Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/helpers/copy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ const BAD_CONTENT_TYPE_ERROR = {
status: 400,
};

const CROSS_ORG_ERROR = {
body: JSON.stringify({ error: 'Destination must be in the same org as the source.' }),
status: 400,
};

export default async function copyHelper(req, daCtx) {
let formData;
try {
Expand All @@ -32,7 +37,12 @@ export default async function copyHelper(req, daCtx) {
const continuationToken = formData.get('continuation-token');
const lower = fullDest.slice(1).toLowerCase();
const sanitized = lower.endsWith('/') ? lower.slice(0, -1) : lower;
const destination = sanitized.split('/').slice(1).join('/');

// Reject cross-org destinations
const [destOrg, ...destParts] = sanitized.split('/');
if (destOrg !== daCtx.org) return { error: CROSS_ORG_ERROR };

const destination = destParts.join('/');
Comment thread
benpeter marked this conversation as resolved.
const source = daCtx.key;
return { source, destination, continuationToken };
}
12 changes: 11 additions & 1 deletion src/helpers/move.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ const NO_PARENT_ERROR = {
status: 400,
};

const CROSS_ORG_ERROR = {
body: JSON.stringify({ error: 'Destination must be in the same org as the source.' }),
status: 400,
};

export default async function moveHelper(req, daCtx) {
try {
const formData = await req.formData();
Expand All @@ -28,7 +33,12 @@ export default async function moveHelper(req, daCtx) {
if (!fullDest) return { error: NO_DEST_ERROR };
const lower = fullDest.slice(1).toLowerCase();
const sanitized = lower.endsWith('/') ? lower.slice(0, -1) : lower;
let destination = sanitized.split('/').slice(1).join('/');

// Reject cross-org destinations
const [destOrg, ...destParts] = sanitized.split('/');
if (destOrg !== daCtx.org) return { error: CROSS_ORG_ERROR };

let destination = destParts.join('/');
const source = daCtx.key;

// Ensure destination is not child of source
Expand Down
69 changes: 69 additions & 0 deletions test/it/it-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,75 @@ export default (ctx) => describe('Integration Tests: it tests', function () {
assert.ok(fileNames.includes('page2'), 'Should list page2');
});

it('[super user] should copy a page within the org', async () => {
const {
serverUrl, org, repo, superUser,
} = ctx;
const formData = new FormData();
formData.append('destination', `/${org}/${repo}/test-folder/page1-copy.html`);

let resp = await fetch(`${serverUrl}/copy/${org}/${repo}/test-folder/page1.html`, {
method: 'POST',
body: formData,
headers: { Authorization: `Bearer ${superUser.accessToken}` },
});
assert.strictEqual(resp.status, 204, `Expected 204 No Content, got ${resp.status} - user: ${superUser.email}`);

// validate the copy exists
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}`);
const body = await resp.text();
assert.strictEqual(body, '<html><body><h1>Page 1</h1></body></html>');
});

it('[super user] cannot copy a page to another org', async () => {
const {
serverUrl, org, repo, superUser,
} = ctx;
const formData = new FormData();
formData.append('destination', `/other-${org}/${repo}/test-folder/page1-xorg.html`);

let resp = await fetch(`${serverUrl}/copy/${org}/${repo}/test-folder/page1.html`, {
method: 'POST',
body: formData,
headers: { Authorization: `Bearer ${superUser.accessToken}` },
});
assert.strictEqual(resp.status, 400, `Expected 400 Bad Request, got ${resp.status} - user: ${superUser.email}`);
const body = await resp.json();
assert.match(body.error, /same org/i, `Expected cross-org error, got ${body.error}`);

// validate no phantom copy was re-anchored into the source org
resp = await fetch(`${serverUrl}/source/${org}/${repo}/test-folder/page1-xorg.html`, {
headers: { Authorization: `Bearer ${superUser.accessToken}` },
});
assert.strictEqual(resp.status, 404, `Expected 404 Not Found, got ${resp.status} - user: ${superUser.email}`);
});

it('[super user] cannot move a page to another org', async () => {
const {
serverUrl, org, repo, superUser,
} = ctx;
const formData = new FormData();
formData.append('destination', `/other-${org}/${repo}/test-folder/page1-moved.html`);

let resp = await fetch(`${serverUrl}/move/${org}/${repo}/test-folder/page1-copy.html`, {
method: 'POST',
body: formData,
headers: { Authorization: `Bearer ${superUser.accessToken}` },
});
assert.strictEqual(resp.status, 400, `Expected 400 Bad Request, got ${resp.status} - user: ${superUser.email}`);
const body = await resp.json();
assert.match(body.error, /same org/i, `Expected cross-org error, got ${body.error}`);

// validate the source was not touched
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,
Expand Down
65 changes: 61 additions & 4 deletions test/routes/copy.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ describe('Copy Route', () => {
formData: () => formdata,
};

const resp = await copyHandler({ req, env: {}, daCtx: { key: 'my/src.html' } });
const resp = await copyHandler({ req, env: {}, daCtx: { org: 'myorg', key: 'my/src.html' } });
assert.strictEqual(403, resp.status);
assert.strictEqual(copyCalled.length, 0);

const resp2 = await copyHandler({ req, env: {}, daCtx: { key: 'my/src2.html' } });
const resp2 = await copyHandler({ req, env: {}, daCtx: { org: 'myorg', key: 'my/src2.html' } });
assert.strictEqual(403, resp2.status);
assert.strictEqual(copyCalled.length, 0);

Expand All @@ -58,11 +58,11 @@ describe('Copy Route', () => {
formData: () => formdata2,
};

const resp3 = await copyHandler({ req: req2, env: {}, daCtx: { key: 'my/src.html' } });
const resp3 = await copyHandler({ req: req2, env: {}, daCtx: { org: 'myorg', key: 'my/src.html' } });
assert.strictEqual(403, resp3.status);
assert.strictEqual(copyCalled.length, 0);

const resp4 = await copyHandler({ req: req2, env: {}, daCtx: { key: 'my/src2.html' } });
const resp4 = await copyHandler({ req: req2, env: {}, daCtx: { org: 'myorg', key: 'my/src2.html' } });
assert.strictEqual(200, resp4.status);
assert.strictEqual(copyCalled.length, 1);
assert.strictEqual('my/src2.html', copyCalled[0].d.source);
Expand Down Expand Up @@ -99,6 +99,63 @@ describe('Copy Route', () => {
assert.match(body.error, /Content-Type/i);
});

it('Test copyHandler returns 400 when destination org differs from request org', 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', '/otherorg/my/dest.html');
const req = {
formData: () => formdata,
};

const resp = await copyHandler({ req, env: {}, daCtx: { org: 'myorg', key: 'my/src.html' } });
assert.strictEqual(resp.status, 400);
assert.strictEqual(copyCalled.length, 0);
const body = JSON.parse(resp.body);
assert.match(body.error, /same org/i);
});

it('Test copyHandler accepts a same-org destination regardless of case', 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/dest.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/dest.html');
});

it('Test copyHandler - no destination provided', async () => {
const copyCalled = [];
const copyObject = (e, c, d, m) => {
Expand Down
64 changes: 60 additions & 4 deletions test/routes/move.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ describe('Move Route', () => {
formData: () => formdata,
};

const resp = await moveRoute({ req, env: {}, daCtx: { key: 'abc.html' } });
const resp = await moveRoute({ req, env: {}, daCtx: { org: 'someorg', key: 'abc.html' } });
assert.strictEqual(403, resp.status);
assert.strictEqual(0, moCalled.length);

const resp2 = await moveRoute({ req, env: {}, daCtx: { key: 'zzz.html' } });
const resp2 = await moveRoute({ req, env: {}, daCtx: { org: 'someorg', key: 'zzz.html' } });
assert.strictEqual(403, resp2.status);
assert.strictEqual(0, moCalled.length);

Expand All @@ -58,13 +58,69 @@ describe('Move Route', () => {
formData: () => formdata2,
};

const resp3 = await moveRoute({ req: req2, env: {}, daCtx: { key: 'abc.html' } });
const resp3 = await moveRoute({ req: req2, env: {}, daCtx: { org: 'someorg', key: 'abc.html' } });
assert.strictEqual(403, resp3.status);
assert.strictEqual(0, moCalled.length);

await moveRoute({ req: req2, env: {}, daCtx: { key: 'zzz.html' } });
await moveRoute({ req: req2, env: {}, daCtx: { org: 'someorg', key: 'zzz.html' } });
assert.strictEqual(1, moCalled.length);
assert.strictEqual('zzz.html', moCalled[0].d.source);
assert.strictEqual('someotherdest', moCalled[0].d.destination);
});

it('Test moveRoute returns 400 when destination org differs from request org', async () => {
const moCalled = [];
const moveObject = (e, c, d) => {
moCalled.push({ e, c, d });
};

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', '/otherorg/somedest/');
const req = {
formData: () => formdata,
};

const resp = await moveRoute({ req, env: {}, daCtx: { org: 'someorg', key: 'abc.html' } });
assert.strictEqual(resp.status, 400);
assert.strictEqual(0, moCalled.length);
const body = JSON.parse(resp.body);
assert.match(body.error, /same org/i);
});

it('Test moveRoute accepts a same-org destination regardless of case', 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/somedest/');
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('somedest', moCalled[0].d.destination);
});
});
Loading