fix: block .da-versions access through the generic source/list/delete routes - #302
Merged
Conversation
…utes
The router guard only matches a leading '.da-versions', but version and
audit objects are stored at the repo-scoped key '{repo}/.da-versions/...'.
daCtx.key is org-stripped, so it starts with '{repo}/' and the guard never
fires - the generic source/list/delete routes reach raw version and audit
storage. These tests drive GET/DELETE/list at a .da-versions path and expect
404; they fail (200) against the current guard.
Version bodies and audit logs are stored at the repo-scoped R2 key
'{org}/{repo}/.da-versions/{fileId}/...'. The router guard that is meant to
keep the generic source/list/delete routes out of that storage only matched
a leading '.da-versions', but daCtx.key is org-stripped and so begins with
'{repo}/'. The guard stopped firing when versions moved from the org-root
layout to the repo-scoped one, letting any authorized org member read version
history and audit logs of documents their ACL forbids, and delete or forge
the audit log through the generic routes.
Match '.da-versions' as any path segment of the org-stripped key. The
ACL-aware versionsource/versionlist routes build the physical key internally
and never carry a '.da-versions' segment in daCtx.key, so they are unaffected;
ordinary content paths are unaffected. The legacy org-root layout stays covered
as the first segment.
…ions Adds two integration assertions after page creation: the generic list route on the reserved .da-versions folder and a write+read through the generic source route both return 404. Against the stale guard the list returns 200 and leaks the audit/version entries; the fix makes both 404.
…substring)
Adds three regression locks: the legacy org-root '.da-versions' first-segment
layout still 404s, a POST write to a .da-versions audit key 404s (the forge
vector), and a path segment that merely contains 'da-versions'
('my-da-versions-notes') still dispatches - pinning segment-exact matching so a
future change to a substring check would fail loudly.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
kptdobe
approved these changes
Jul 16, 2026
adobe-bot
pushed a commit
that referenced
this pull request
Jul 16, 2026
## [1.12.4](v1.12.3...v1.12.4) (2026-07-16) ### Bug Fixes * block .da-versions access through the generic source/list/delete routes ([#302](#302)) ([3d74efa](3d74efa))
Collaborator
|
🎉 This PR is included in version 1.12.4 🎉 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 generic
source/list/deleteroutes have a guard that should keep them out of version storage. It matched only a leading.da-versions. But version bodies and audit logs live at{org}/{repo}/.da-versions/{fileId}/..., anddaCtx.keyis org-stripped. So the key starts with{repo}/. The guard did not fire. Any authorized org member could reach version storage through/source. They could read, delete, or forge the version history and audit log of a document their ACL forbids. The guard broke when versions moved to the repo-scoped layout (#253).src/index.jswas not updated.The fix matches
.da-versionsas any path segment of the key, not only the first./versionsourceand/versionlistare not affected. They build the physical key later, in the storage layer. TheirdaCtx.keyhas no.da-versionssegment. Ordinary content and the legacy org-root layout stay covered.Tests: the generic routes now return 404 on
.da-versionspaths (was 200). Unit tests cover the read, write, and list vectors. A live integration check shows the leak before the fix and 404 after.