fix: reject cross-org copy/move destinations instead of misdelivering#301
Merged
Conversation
The copy and move helpers parsed the destination form field with
split('/').slice(1), silently discarding the destination's org segment
and re-anchoring the path under the request org. A cross-org copy
returned 204 and created a phantom copy inside the source org; nothing
reached the intended destination org (confirmed against production
admin.da.live on 2026-07-10). Under a restrictive source-org ACL the
same request surfaced as a misleading 403.
Compare the destination's org segment to the request org and return an
explicit 400 on mismatch. Both endpoints are documented as operating
within an organization, and 400 is already a documented response code
for both. The underlying feature ask is adobe/da-live#623.
Relates to adobe/da-live#34.
kptdobe
reviewed
Jul 14, 2026
kptdobe
approved these changes
Jul 14, 2026
Contributor
|
Thanks @benpeter. I'll merge as soon as you addresses my very little comment. |
adobe-bot
pushed a commit
that referenced
this pull request
Jul 14, 2026
## [1.12.3](v1.12.2...v1.12.3) (2026-07-14) ### Bug Fixes * reject cross-org copy/move destinations instead of misdelivering ([#301](#301)) ([fd7a351](fd7a351))
Collaborator
|
🎉 This PR is included in version 1.12.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
benpeter
added a commit
that referenced
this pull request
Jul 17, 2026
Version bodies and audit logs live at the reserved key
{org}/{repo}/.da-versions/{fileId}/.... The generic /copy and /move routes read
their destination from the request body. They did not check it against that
namespace. A writer could copy or move an object into {repo}/.da-versions/....
That plants attacker-controlled bodies where version and audit storage lives.
It forges a document's version history and audit log.
#301 rejects a different-org destination. It does not reject a same-org
.da-versions destination, so the vector was open. The .da-versions router guard
added in #302 inspects daCtx.key, which is the copy/move source, not the
destination. So it did not cover this either.
Add hasReservedSegment to the version paths module. It is true when any segment
of a path is .da-versions. copyHelper and moveHelper reject a destination in
that folder, next to the existing cross-org check. copyFile applies the same
check to each derived Key, so a folder copy cannot slip a reserved key through.
The check does not rely on hasPermission, which grants .da-versions writes in
no-config and broad-grant orgs. The ACL-aware version routes build the physical
key internally. Their daCtx.key does not carry a .da-versions segment, so they
are unaffected. A segment that only contains 'da-versions' stays allowed.
benpeter
added a commit
that referenced
this pull request
Jul 20, 2026
#303) * test(copy-move): reproduce .da-versions destination forge The generic /copy and /move routes read their destination from the request body. The finding-16 router guard runs on daCtx.key. That key is the copy/move source, which has no .da-versions segment, so the guard passes. The destination is not checked against the reserved namespace. A writer can plant objects under {repo}/.da-versions/{fileId}/... and forge a document's version history and audit log. These tests drive a .da-versions destination through copyHandler, moveRoute, and copyFile. Each expects a 400 with no write. They fail against current code: the copy proceeds and returns 200. Companion tests assert a segment that only contains 'da-versions' (my-da-versions-notes) still dispatches, pinning segment-exact matching. * fix: reject copy/move destinations in the reserved .da-versions folder Version bodies and audit logs live at the reserved key {org}/{repo}/.da-versions/{fileId}/.... The generic /copy and /move routes read their destination from the request body. They did not check it against that namespace. A writer could copy or move an object into {repo}/.da-versions/.... That plants attacker-controlled bodies where version and audit storage lives. It forges a document's version history and audit log. #301 rejects a different-org destination. It does not reject a same-org .da-versions destination, so the vector was open. The .da-versions router guard added in #302 inspects daCtx.key, which is the copy/move source, not the destination. So it did not cover this either. Add hasReservedSegment to the version paths module. It is true when any segment of a path is .da-versions. copyHelper and moveHelper reject a destination in that folder, next to the existing cross-org check. copyFile applies the same check to each derived Key, so a folder copy cannot slip a reserved key through. The check does not rely on hasPermission, which grants .da-versions writes in no-config and broad-grant orgs. The ACL-aware version routes build the physical key internally. Their daCtx.key does not carry a .da-versions segment, so they are unaffected. A segment that only contains 'da-versions' stays allowed. * test(it): assert copy and move are blocked from the .da-versions folder Adds an integration assertion after the cross-org tests. A copy and a move whose destination is inside {repo}/.da-versions/... both return 400 from the destination guard. The blocked move leaves its source in place. Against pre-fix code the copy returns 204 and plants the object in version storage. * test(copy): reproduce .da-versions carry-along over-block on repo rename A repo-level move lists the repo's own version storage under {repo}/.da-versions/.... copyFile must carry those keys to the new repo. The broad reserved-Key guard rejects them with 400, so moveObject skips the delete and strands the history at the old repo name. This test drives copyFile with a source already under .da-versions and expects the copy to proceed. It fails against the broad guard: it returns 400 and sends nothing. * fix: allow repo-level copy/move to carry .da-versions along The copyFile reserved-key guard rejected every derived key under .da-versions. A repo-level move lists the repo's own version storage, so the guard 400-skipped those keys and moveObject left them at the old repo name, stranding the version history. Reject only when the destination introduces the segment: a source that is already under .da-versions carries along. The forge is still blocked, because its source is ordinary content and the destination is what adds .da-versions. * test(paths): lock hasReservedSegment segment-exact matching hasReservedSegment is the shared reserved-name check for the copy/move guard. It had no direct unit test, so a substring form would pass the suite. Lock the contract: it matches .da-versions as a whole path segment, ignores a segment that only contains the name (my-da-versions-notes, .da-versions-backup), and is safe for non-string input. * test(copy): require .da-versions destination block with no source exemption The refined guard skipped the check when the source was already under .da-versions, so a repo-level copy or move could relocate a .da-versions object into another .da-versions location. Chained with a separate write hole that plants crafted content in some .da-versions, that reopens the forge. This test drives copyFile with a source under .da-versions and expects 400 with no send. It fails against the exemption: it returns 200 and sends the copy. * fix: block all .da-versions copy/move destinations without exemption An earlier revision exempted a source already under .da-versions. That let a repo-level rename carry its version storage along. The exemption is a hole. An attacker who plants crafted content in any .da-versions object could relocate it into a victim's version history through a repo-level copy or move. Drop the exemption: reject every derived key under .da-versions. Documented copy and move act on a path below the repo. That path does not list the repo-root .da-versions folder, so this changes nothing for them. Only an undocumented repo-root move or copy reaches version storage, and it no longer carries or duplicates it. * Update src/helpers/move.js Co-authored-by: Alexandre Capt <acapt@adobe.com> * Update src/helpers/copy.js Co-authored-by: Alexandre Capt <acapt@adobe.com> * test: match the generic reserved-destination error message --------- Co-authored-by: Alexandre Capt <acapt@adobe.com>
adobe-bot
pushed a commit
that referenced
this pull request
Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The copy and move helpers dropped the
destinationfield's first path segment unconditionally. A destination in a different org (e.g. cross-org paste, adobe/da-live#623) had its org silently discarded and the path re-anchored under the request org: the request returned 204 and created a phantom copy inside the source org — nothing reached the intended org, and the caller got no error.Since storage pins all keys to the request org, a cross-org destination can never be honored. Both helpers now compare the destination's org segment to the request org and return
400 {"error":"Destination must be in the same org as the source."}on mismatch.Tests: route-level rejection and mixed-case same-org acceptance; integration coverage for same-org copy, cross-org copy (400, no phantom), and cross-org move (400, source intact).
Relates to adobe/da-live#623, adobe/da-live#34.