fix: reject invalid signed URL expiries across storage providers - #1781
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough스토리지 코어에 signed URL 만료값 검증 계약이 추가되었습니다. 모든 storage provider는 서명 전에 값을 검증합니다. 잘못된 값은 공통 문제 코드로 거부합니다. 적합성 테스트와 API·문서·릴리스 메타데이터가 갱신되었습니다. ChangesSigned URL 만료 검증
Workspace dependency override
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant StorageProvider
participant validateSignedUrlExpiry
participant Signer
Caller->>StorageProvider: getSignedUrl(expiresIn)
StorageProvider->>validateSignedUrlExpiry: 만료값 검증
validateSignedUrlExpiry-->>StorageProvider: 검증된 초 단위 만료값
StorageProvider->>Signer: 검증된 만료값 전달
Signer-->>Caller: signed URL 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-08-08T09:30:15.484Z · Commit: 5d7355b |
|
CI follow-up: the publish profile began blocking the newly reported |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/docs/src/content/docs/api/storage-core/src/functions/validateSignedUrlExpiry.md (1)
8-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
@throws태그를 추가하여 Throws 섹션을 생성하십시오.이 문서에는
expiresIn값이 유효하지 않을 때 발생하는 예외에 대한 설명이 없습니다. 구현은 안전 정수가 아니거나, 0 이하이거나,MAX_SIGNED_URL_EXPIRY_SECONDS를 초과하면InvalidSignedUrlExpiryProblem을 던집니다.소스 함수(
packages/storage-core/src/libs/signedUrlExpiry.ts)에@throws {InvalidSignedUrlExpiryProblem}JSDoc 태그를 추가하십시오. TypeDoc이 이 태그를 사용해 "## Throws" 섹션을 자동으로 생성합니다. 이 생성된 파일은 직접 편집하지 마십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/docs/src/content/docs/api/storage-core/src/functions/validateSignedUrlExpiry.md` around lines 8 - 23, Update the source function validateSignedUrlExpiry in signedUrlExpiry.ts by adding a JSDoc `@throws` annotation for InvalidSignedUrlExpiryProblem, covering invalid, non-positive, or over-limit expiry values; do not edit the generated Markdown documentation directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/storage-core/src/tests/InMemoryStorageProvider.spec.ts`:
- Around line 205-208: Update the rejection assertion in the affected test to
use rejects.toThrow(InvalidSignedUrlExpiryProblem) instead of
rejects.toBeInstanceOf, matching the existing async error assertion pattern;
retain the separate code and message toMatchObject validation.
In `@packages/storage-core/src/tests/SignedUrlExpiry.spec.ts`:
- Around line 21-45: Update the test setup around the “signed URL expiry
contract” suite to add a beforeEach that resets the DI Container and explicitly
initializes the test instance/state for every test. Use the project’s existing
container reset and test initialization helpers, if available, and leave the
expiry assertions unchanged.
In `@packages/testing/src/libs/provider-conformance.ts`:
- Around line 45-56: Rename the constants invalidSignedUrlExpiryCases and
invalidSignedUrlExpiryMessage to INVALID_SIGNED_URL_EXPIRY_CASES and
INVALID_SIGNED_URL_EXPIRY_MESSAGE, respectively, and update every reference to
these symbols throughout the conformance tests.
---
Outside diff comments:
In
`@packages/docs/src/content/docs/api/storage-core/src/functions/validateSignedUrlExpiry.md`:
- Around line 8-23: Update the source function validateSignedUrlExpiry in
signedUrlExpiry.ts by adding a JSDoc `@throws` annotation for
InvalidSignedUrlExpiryProblem, covering invalid, non-positive, or over-limit
expiry values; do not edit the generated Markdown documentation directly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ee173d09-668a-482a-aa31-7d5c1ca7d684
⛔ Files ignored due to path filters (2)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.changeset/valid-signed-url-expiry.mddocs/problem-code-registry.jsonpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/api/storage-core/src/classes/InvalidSignedUrlExpiryProblem.mdpackages/docs/src/content/docs/api/storage-core/src/classes/StorageProblem.mdpackages/docs/src/content/docs/api/storage-core/src/functions/validateSignedUrlExpiry.mdpackages/docs/src/content/docs/api/storage-core/src/type-aliases/SignedUrlOptions.mdpackages/docs/src/content/docs/api/storage-core/src/variables/MAX_SIGNED_URL_EXPIRY_SECONDS.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/storage-cloudflare/src/libs/CloudflareImagesProvider.tspackages/storage-cloudflare/src/tests/CloudflareImagesProvider.spec.tspackages/storage-cloudinary/src/libs/CloudinaryProvider.tspackages/storage-cloudinary/src/tests/CloudinaryProvider.spec.tspackages/storage-core/README.mdpackages/storage-core/src/index.tspackages/storage-core/src/libs/InMemoryStorageProvider.tspackages/storage-core/src/libs/problems/InvalidSignedUrlExpiryProblem.tspackages/storage-core/src/libs/signedUrlExpiry.tspackages/storage-core/src/libs/types.tspackages/storage-core/src/tests/InMemoryStorageProvider.spec.tspackages/storage-core/src/tests/SignedUrlExpiry.spec.tspackages/storage-r2/src/libs/R2StorageProvider.tspackages/storage-r2/src/tests/R2StorageProvider.spec.tspackages/testing/src/libs/provider-conformance.tspackages/testing/src/tests/ConformanceContract.spec.tspnpm-workspace.yamlpublic-api-surface.snapshot.jsontsconfig/contract-strict.baseline.json
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
Outcome
Storage providers now share one signed URL expiry contract: a positive safe integer from 1 through 604800 seconds. Invalid values fail before signing with the stable
STORAGE_INVALID_SIGNED_URL_EXPIRYProblem instead of reaching provider-specific behavior.The contract is enforced by in-memory, R2, Cloudflare Images, and Cloudinary providers, published from
@croco/storage-core, covered by provider conformance and boundary tests, and reflected in generated API and Problem registry artifacts.Verification
Review gates
pnpm checkalso completed 22/25 repository checks. The three aggregate-runner failures were isolated to a verification-test timeout, temporary SARIF generation, and a moved strict-contract line; their exact isolated gates passed after regeneration or direct rerun.Fixes #1708
Summary by CodeRabbit
새 기능
문서
버그 수정