Skip to content

INTEGRATION [PR#2599 > development/8.3] ARSN-560: return AccessDenied error if x-amz-timestamp values before epoch#2601

Merged
bert-e merged 3 commits intodevelopment/8.3from
w/8.3/bugfix/ARSN-560-error-with-date-before-epoch
Mar 25, 2026
Merged

INTEGRATION [PR#2599 > development/8.3] ARSN-560: return AccessDenied error if x-amz-timestamp values before epoch#2601
bert-e merged 3 commits intodevelopment/8.3from
w/8.3/bugfix/ARSN-560-error-with-date-before-epoch

Conversation

@bert-e
Copy link
Copy Markdown
Contributor

@bert-e bert-e commented Mar 25, 2026

This pull request has been created automatically.
It is linked to its parent pull request #2599.

Do not edit this pull request directly.
If you need to amend/cancel the changeset on branch
w/8.3/bugfix/ARSN-560-error-with-date-before-epoch, please follow this
procedure:

 git fetch
 git checkout w/8.3/bugfix/ARSN-560-error-with-date-before-epoch
 # <amend or cancel the changeset by _adding_ new commits>
 git push origin w/8.3/bugfix/ARSN-560-error-with-date-before-epoch

Please always comment pull request #2599 instead of this one.

@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

LGTM — the refactoring from isValidISO8601CompactparseISO8601Compact is clean, avoids double-parsing the timestamp, and the pre-epoch rejection with AccessDenied is correct.

One consideration for a follow-up:

  • queryAuthCheck.ts (line 61/116) does not have the same pre-epoch guard. A pre-epoch X-Amz-Date in a presigned URL would likely be caught by checkTimeSkew as RequestTimeTooSkewed rather than AccessDenied, so the behavior is inconsistent between header auth and query auth. Consider applying the same fix there for consistency, or document that it's intentionally out of scope.

Review by Claude Code

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.37%. Comparing base (22e8dbc) to head (ded10b1).
⚠️ Report is 3 commits behind head on development/8.3.

Files with missing lines Patch % Lines
lib/auth/v4/headerAuthCheck.ts 90.47% 2 Missing ⚠️
lib/auth/v4/timeUtils.ts 88.88% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           development/8.3    #2601      +/-   ##
===================================================
+ Coverage            73.35%   73.37%   +0.01%     
===================================================
  Files                  222      222              
  Lines                18149    18163      +14     
  Branches              3781     3785       +4     
===================================================
+ Hits                 13314    13327      +13     
- Misses                4830     4831       +1     
  Partials                 5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bert-e bert-e merged commit ded10b1 into development/8.3 Mar 25, 2026
8 checks passed
@bert-e bert-e deleted the w/8.3/bugfix/ARSN-560-error-with-date-before-epoch branch March 25, 2026 11:06
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.

2 participants