Skip to content

CLDSRV-1021: Compare s3:VersionId with the copy source version ID as sent - #6332

Open
tcarmet wants to merge 2 commits into
bugfix/CLDSRV-1021-copy-source-bucket-policyfrom
bugfix/CLDSRV-1021-copy-source-version-id
Open

tcarmet wants to merge 2 commits into
bugfix/CLDSRV-1021-copy-source-bucket-policyfrom
bugfix/CLDSRV-1021-copy-source-version-id

Conversation

@tcarmet

@tcarmet tcarmet commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

A policy condition on s3:VersionId was compared with the copy source's internal version ID instead of the version ID the client sent. So a Deny on a specific version blocked a GetObject of that version but not a copy of it, in both bucket and identity policies. The copy source's version ID is now evaluated as sent, like for a direct GetObject. A repeated versionId in the copy source is rejected, so it never reaches policy evaluation as a list. Version IDs that decode to the same version under different spellings are still not normalized; that's tracked in CLDSRV-1038 and affects GetObject the same way.

Stacked on #6324.

…sent

- Use the copy source version ID as sent by the client, for both the bucket
  policy and the authorization request, instead of the internal version ID
- Return InvalidArgument when the copy source versionId is repeated, so an
  array never reaches the bucket policy conditions
@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.31%. Comparing base (ca2dde6) to head (57d9720).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/api/api.js 95.07% <100.00%> (+0.72%) ⬆️
lib/api/apiUtils/authorization/permissionChecks.js 97.32% <100.00%> (+0.01%) ⬆️
...i/apiUtils/authorization/prepareRequestContexts.js 95.58% <ø> (ø)
lib/api/apiUtils/object/parseCopySource.js 96.00% <100.00%> (+0.76%) ⬆️
lib/api/objectCopy.js 91.73% <ø> (ø)
lib/api/objectPutCopyPart.js 86.98% <ø> (-0.60%) ⬇️
lib/metadata/metadataUtils.js 94.73% <ø> (ø)

... and 3 files with indirect coverage changes

@@                               Coverage Diff                                @@
##           bugfix/CLDSRV-1021-copy-source-bucket-policy    #6332      +/-   ##
================================================================================
- Coverage                                         85.37%   85.31%   -0.06%     
================================================================================
  Files                                               206      206              
  Lines                                             13446    13453       +7     
================================================================================
- Hits                                              11479    11477       -2     
- Misses                                             1967     1976       +9     
Flag Coverage Δ
file-ft-tests 68.31% <83.33%> (+<0.01%) ⬆️
file-ft-tests-null-compat 68.83% <83.33%> (+<0.01%) ⬆️
kmip-ft-tests 28.36% <66.66%> (+0.01%) ⬆️
mongo-v0-ft-tests 69.51% <83.33%> (-0.08%) ⬇️
mongo-v1-ft-tests 69.51% <83.33%> (-0.02%) ⬇️
multiple-backend 36.77% <16.66%> (-0.02%) ⬇️
s3c-ft-tests-v0 61.28% <91.66%> (-2.75%) ⬇️
s3c-ft-tests-v0-null-compat 64.11% <91.66%> (+<0.01%) ⬆️
s3c-ft-tests-v1 64.03% <91.66%> (+0.02%) ⬆️
sur-tests 35.98% <66.66%> (+0.04%) ⬆️
sur-tests-inflights 37.79% <66.66%> (+<0.01%) ⬆️
unit 71.75% <100.00%> (+0.04%) ⬆️
utapi-v2-tests 34.60% <66.66%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant