VPR-59 [1/4] CMS migration: files, photos, import, rate limiting (backend)#251
Open
rlorenzo wants to merge 2 commits into
Open
VPR-59 [1/4] CMS migration: files, photos, import, rate limiting (backend)#251rlorenzo wants to merge 2 commits into
rlorenzo wants to merge 2 commits into
Conversation
This was referenced Jul 3, 2026
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #251 +/- ##
==========================================
+ Coverage 45.29% 46.60% +1.30%
==========================================
Files 897 920 +23
Lines 51929 53704 +1775
Branches 4879 5089 +210
==========================================
+ Hits 23522 25028 +1506
- Misses 27803 27992 +189
- Partials 604 684 +80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Implements the first slice of the CMS migration backend for the files domain: new CMS file-management APIs (list/upload/edit/delete/restore/audit/import/bulk-encrypt), hardened legacy-parity download handling (nosniff + attachment for inline-unsafe types), CMS download rate limiting, a conditional-cached user-photo API, and a gated scheduled trash-purge job—plus supporting validators/constants and comprehensive unit tests.
Changes:
- Add CMS file management services/controllers (storage, encryption/crypto, audit, import, trash purge job) and DTO/mapping infrastructure.
- Harden and optimize the existing
/CMS/Filesdownload surface (rate limiting, access auditing parity, nosniff + attachment rules, permission lookup efficiency). - Add CMS user photo endpoints + service with Last-Modified/304 support and caching headers, plus shared validation/date-range helpers and tests.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/Viper.csproj | Adds htmldiff dependency for diff rendering. |
| web/Services/IHtmlSanitizerService.cs | Adds SanitizeDiff API contract. |
| web/Services/HtmlSanitizerService.cs | Implements diff-aware sanitizer policy. |
| web/Program.cs | Registers CMS download rate limiting + middleware. |
| web/Classes/Utilities/DateRangeHelper.cs | Shared inclusive “To date” handling. |
| web/Areas/CMS/Validation/SafeUrlAttribute.cs | URL validation with optional relative support. |
| web/Areas/CMS/Validation/MaxLengthEachAttribute.cs | Per-element max-length validation for collections. |
| web/Areas/CMS/Services/CmsUserPhotoService.cs | User photo resolution + alt photo support. |
| web/Areas/CMS/Services/CmsFileStorageService.cs | Safe file storage + path containment + collisions. |
| web/Areas/CMS/Services/CmsFileService.cs | Core file CRUD, encryption toggle, purge. |
| web/Areas/CMS/Services/CmsFileResponse.cs | Download response hardening helpers. |
| web/Areas/CMS/Services/CmsFileImportService.cs | Legacy-webroot import + bulk encrypt tools. |
| web/Areas/CMS/Services/CmsFileEncryptionService.cs | Master-key wrapper + in-place transforms. |
| web/Areas/CMS/Services/CmsFileCrypto.cs | CF-compatible AES-ECB + UU encoding primitives. |
| web/Areas/CMS/Services/CmsFileAuditService.cs | File audit row creation + paging/filtering. |
| web/Areas/CMS/Services/CmsDownloadRateLimiting.cs | Rate limiter policy + partition keying. |
| web/Areas/CMS/Services/CmsConcurrencyException.cs | Shared 409 stale-edit exception type. |
| web/Areas/CMS/Models/DTOs/CreateLinkDto.cs | Fixes SafeUrlAttribute namespace reference. |
| web/Areas/CMS/Models/DTOs/CmsFileRequests.cs | Request DTOs for create/update endpoints. |
| web/Areas/CMS/Models/DTOs/CmsFileImportDtos.cs | Import + preview + bulk-encrypt DTOs. |
| web/Areas/CMS/Models/DTOs/CmsFileDto.cs | File management DTOs and name-check result. |
| web/Areas/CMS/Models/CmsFileMapper.cs | Mapperly-based entity→DTO mapping. |
| web/Areas/CMS/Models/CMSFile.cs | Aligns URL generation with new handler shape. |
| web/Areas/CMS/Jobs/CmsTrashPurgeScheduledJob.cs | Gated daily purge job for trashed files. |
| web/Areas/CMS/Data/CMS.cs | Download hardening, auditing parity, perf tweaks. |
| web/Areas/CMS/Controllers/CMSUserPhotoController.cs | /api/cms/photos endpoints + 304 behavior. |
| web/Areas/CMS/Controllers/CMSOptionsController.cs | CMS option lists for permissions/people. |
| web/Areas/CMS/Controllers/CMSFilesController.cs | /api/cms/files management API surface. |
| web/Areas/CMS/Controllers/CMSController.cs | Enables rate limiting on /CMS/Files. |
| web/Areas/CMS/Constants/CmsTrash.cs | Trash retention + config key constants. |
| web/Areas/CMS/Constants/CmsPermissions.cs | Central CMS permission strings. |
| web/Areas/CMS/Constants/CmsFileTypes.cs | File allowlist + inline-unsafe extension set. |
| web/Areas/CMS/Constants/CmsFileNaming.cs | Friendly-name + canonical folder key rules. |
| web/appsettings.json | Adds default-off trash purge config. |
| test/Services/HtmlSanitizerServiceTests.cs | Tests for SanitizeDiff XSS guarantees. |
| test/CMS/SafeUrlAttributeTests.cs | SafeUrlAttribute absolute/relative-mode tests. |
| test/CMS/MaxLengthEachAttributeTests.cs | MaxLengthEachAttribute behavior tests. |
| test/CMS/CmsUserPhotoServiceTests.cs | Photo service resolution/traversal/Last-Modified tests. |
| test/CMS/CMSUserPhotoControllerTests.cs | Controller wiring + cache/304 behavior tests. |
| test/CMS/CmsTrashPurgeScheduledJobTests.cs | Ensures purge gate stays off by default. |
| test/CMS/CmsFileStorageServiceTests.cs | Storage containment, collisions, temp writes tests. |
| test/CMS/CMSFilesControllerTests.cs | Controller status mapping + admin-only hard delete tests. |
| test/CMS/CmsFileResponseTests.cs | nosniff + attachment/inline disposition tests. |
| test/CMS/CmsFileImportServiceTests.cs | Import preview/execute + bulk-encrypt tests. |
| test/CMS/CmsFileCryptoTests.cs | Crypto format round-trip compatibility tests. |
| test/CMS/CmsDownloadRateLimitingTests.cs | ZIP vs file partitioning key tests. |
| test/CMS/CMSContentPermissionTests.cs | Content permission gate tests (perf change coverage). |
| test/Classes/Utilities/DateRangeHelperTests.cs | Exclusive upper-bound helper tests. |
This was referenced Jul 3, 2026
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CMS file-management APIs, storage, encryption, import, purge, rate limiting, and photo serving, plus a diff-aware HTML sanitizer path and tests. ChangesCMS File Management System
HTML Diff Sanitization
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/Areas/CMS/Data/CMS.cs (1)
398-424: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-file synchronous
SaveChanges()in theDownloadZiploop.
AuditFileAccessclears the change tracker and callsSaveChanges()for every guid inguids(both allowed and now denied files). A zip request with many files causes that many individual round-trips to the audit table instead of one batched write.Consider collecting
FileAuditrows in the loop and callingAddRange+ a singleSaveChanges()after the loop completes.🤖 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 `@web/Areas/CMS/Data/CMS.cs` around lines 398 - 424, The DownloadZip audit path is doing a synchronous SaveChanges for every file through AuditFileAccess, causing one database round-trip per guid. Update the CMS.DownloadZip flow and the AuditFileAccess usage so the loop only collects FileAudit entries for both allowed and denied files, then persist them once after the loop with AddRange and a single SaveChanges call. Keep the existing audit action names and use the current _viperContext, CheckFilePermission, and AuditFileAccess flow to locate the batching change.
🤖 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 `@test/Services/HtmlSanitizerServiceTests.cs`:
- Around line 249-269: Add an idempotency regression test for SanitizeDiff in
HtmlSanitizerServiceTests, mirroring the existing Sanitization_is_idempotent
coverage for Sanitize. Use SanitizeDiff as the target and verify that running it
twice on already-sanitized diff HTML (for example content with <ins> and <del>
markers) produces the same output both times. This will protect the SanitizeDiff
method and its diff-specific allowlist from double-sanitization issues when
re-rendered.
In `@web/Areas/CMS/Constants/CmsFileTypes.cs`:
- Around line 31-37: The CmsFileTypes allow-list currently includes potentially
dangerous binary/archive types, which creates a malware-distribution risk.
Review the CmsFileTypes mapping and remove unsupported executable/archive
entries such as exe, dmg, 7z, and zip unless there is a confirmed legacy
requirement; if they must remain, ensure the upload flow that uses this
allow-list also routes those types through malware scanning before they are
accepted into the CMS file store. Refer to the CmsFileTypes constant so the
allow-list and any related upload validation stay in sync.
In `@web/Areas/CMS/Controllers/CMSFilesController.cs`:
- Around line 259-317: Add an upper bound check for batch sizes in
CMSFilesController so ImportFiles, PreviewImport, and BulkEncrypt reject
oversized requests instead of only checking for empty lists. Use the existing
request collections (CmsFileImportRequest.FilePaths and the BulkEncrypt
fileGuids parameter) to enforce a reasonable max, and return BadRequest when
exceeded. Keep the validation close to the guards in ImportFiles, PreviewImport,
and BulkEncrypt so the limit is enforced before calling _importService.
In `@web/Areas/CMS/Controllers/CMSOptionsController.cs`:
- Around line 47-84: Add coverage for CMSOptionsController.SearchPeople because
it has non-trivial search filtering and result-shaping logic. Create a
controller test class for CMSOptionsController that verifies the
min-length/whitespace guard returns an empty list, the multi-field OR search
matches DisplayLastName/DisplayFirstName/LoginId/MailId, and the query respects
ordering plus the Take(25) limit. Use the SearchPeople action and
CmsPersonOption type to locate the behavior.
- Around line 47-74: The SearchPeople action in CMSOptionsController is using
Contains() on concatenated name fields and LoginId/MailId, which translates to
leading-wildcard scans and won’t benefit from indexes. Update the filtering
logic to use an index-friendly search strategy for AaudUsers, such as a
persisted/computed display-name field or full-text search, and adjust the query
in SearchPeople so it targets those indexed/searchable fields instead of
concatenated expressions and broad Contains() predicates.
In `@web/Areas/CMS/Data/CMS.cs`:
- Around line 642-647: `DecryptFile` is re-reading the master key from disk on
every invocation, which is expensive when `DownloadZip` calls it repeatedly
inside the archive loop. Update the `DecryptFile` flow to reuse a cached master
key instead of calling
`CmsFileCrypto.ReadMasterKey(CmsFileCrypto.GetDefaultKeyFilePath())` each time,
either by confirming `ReadMasterKey` caches internally or by caching the key
once per `DownloadZip`/`ProvideFile` execution and passing it through to
`DecryptFile` or delegating to `CmsFileEncryptionService`.
- Around line 60-86: The CMS permission check is using a hardcoded security
string instead of the shared constant. Update the access-resolution logic in
CMS.cs where isCmsAdmin is assigned to use CmsPermissions.ManageContentBlocks
rather than "SVMSecure.CMS.ManageContentBlocks", so the check stays aligned with
the rest of the CMS permission code and avoids drift.
- Around line 611-619: The kiosk IP filter in AuditFileAccess only checks the
raw RemoteIpAddress string, so IPv4-mapped IPv6 values like ::ffff:192.168.x.x
slip through. Update the RemoteIpAddress handling in CMS.AuditFileAccess to
normalize mapped IPv4 addresses before evaluating the kiosk range, then perform
the 192.168. prefix check using StartsWith with StringComparison.Ordinal.
In `@web/Areas/CMS/Services/CmsFileAuditService.cs`:
- Around line 87-88: The audit history pagination in CmsFileAuditService has the
same unbounded perPage issue as CmsFileService.GetFilesAsync: only the lower
bound is enforced. Update the paging logic around the page/perPage normalization
to also cap perPage at a safe maximum, using the existing CmsFileAuditService
method that fetches audit history so callers cannot request arbitrarily large
page sizes.
In `@web/Areas/CMS/Services/CmsFileEncryptionService.cs`:
- Around line 25-31: `CmsFileEncryptionService` is still scoped, so the
`_masterKey` Lazy<string> is recreated per request and repeatedly hits the key
file on first use. Update the DI registration or introduce a singleton key-cache
so the master key is loaded once and reused across scopes, and keep the caching
behind `CmsFileEncryptionService`/`CmsFileCrypto.ReadMasterKey` rather than per
scoped instance.
In `@web/Areas/CMS/Services/CmsFileImportService.cs`:
- Around line 331-389: `EncryptSingleFileAsync` is missing some expected
file/key failure handling, so malformed key data or invalid paths can still
escape and abort `BulkEncryptAsync`. Update this method to catch the same
anticipated exceptions as `ImportSingleFileAsync`, especially `FormatException`
alongside the existing file access cases, and convert them into `result.Message`
so the bulk loop can continue collecting results. Keep the fix localized to
`EncryptSingleFileAsync` and preserve the existing rollback behavior for save
failures.
In `@web/Areas/CMS/Services/CmsFileService.cs`:
- Line 390: The current encryption toggle flow in CmsFileService unnecessarily
rewrites the existing file on disk when a replacement upload is present. Update
the logic around ApplyEncryptionToggle, WriteReplacementUploadAsync, and the
replacement path so that when file != null, only metadata changes (entity.Key
and entity.Encrypted) are applied before the new bytes are written. Keep the
in-place encrypt/decrypt work only for the no-replacement case, and ensure the
replacement upload already handles writing the file in the requested encryption
state.
- Around line 76-77: Clamp both bounds in CmsFileService pagination: the current
page/perPage normalization only enforces a minimum in the paging logic, so
perPage can still grow without limit. Update the relevant method in
CmsFileService to cap perPage at a safe maximum before it is used for Skip/Take,
while keeping the existing lower-bound guard and preserving the page
normalization behavior.
- Around line 235-297: The overwrite path in CreateFileAsync does not clean up
when ReplaceInPlace throws after BackupManagedFile succeeds, leaving a stale
backup and inconsistent disk state. Update the overwrite branch in
CmsFileService.CreateFileAsync to wrap ReplaceInPlace in error handling, and on
failure call ReconcileStoreAfterFailedCreate(targetPath, overwriteBackup) before
rethrowing so the backup/original is restored or removed consistently.
In `@web/Areas/CMS/Services/CmsFileStorageService.cs`:
- Around line 136-154: FileNameInUse is being called repeatedly from
GetUniqueFileName, causing up to 1000 synchronous, non-sargable database checks
per upload. Refactor the uniqueness check so GetUniqueFileName loads the
existing file names for the target folder once, combines that with the disk
listing, and then checks candidates in memory across the suffix loop. Keep
FileNameInUse/CmsFileStorageService as the place to locate the current logic,
but avoid per-candidate _context.Files.Any calls and the EndsWith-based DB scan.
---
Outside diff comments:
In `@web/Areas/CMS/Data/CMS.cs`:
- Around line 398-424: The DownloadZip audit path is doing a synchronous
SaveChanges for every file through AuditFileAccess, causing one database
round-trip per guid. Update the CMS.DownloadZip flow and the AuditFileAccess
usage so the loop only collects FileAudit entries for both allowed and denied
files, then persist them once after the loop with AddRange and a single
SaveChanges call. Keep the existing audit action names and use the current
_viperContext, CheckFilePermission, and AuditFileAccess flow to locate the
batching change.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fbfb8bc-5434-4e95-94af-4ed1a1343881
📒 Files selected for processing (49)
test/CMS/CMSContentPermissionTests.cstest/CMS/CMSFilesControllerTests.cstest/CMS/CMSUserPhotoControllerTests.cstest/CMS/CmsDownloadRateLimitingTests.cstest/CMS/CmsFileCryptoTests.cstest/CMS/CmsFileImportServiceTests.cstest/CMS/CmsFileResponseTests.cstest/CMS/CmsFileServiceTests.cstest/CMS/CmsFileStorageServiceTests.cstest/CMS/CmsTrashPurgeScheduledJobTests.cstest/CMS/CmsUserPhotoServiceTests.cstest/CMS/MaxLengthEachAttributeTests.cstest/CMS/SafeUrlAttributeTests.cstest/Classes/Utilities/DateRangeHelperTests.cstest/Services/HtmlSanitizerServiceTests.csweb/Areas/CMS/Constants/CmsFileNaming.csweb/Areas/CMS/Constants/CmsFileTypes.csweb/Areas/CMS/Constants/CmsPermissions.csweb/Areas/CMS/Constants/CmsTrash.csweb/Areas/CMS/Controllers/CMSController.csweb/Areas/CMS/Controllers/CMSFilesController.csweb/Areas/CMS/Controllers/CMSOptionsController.csweb/Areas/CMS/Controllers/CMSUserPhotoController.csweb/Areas/CMS/Data/CMS.csweb/Areas/CMS/Jobs/CmsTrashPurgeScheduledJob.csweb/Areas/CMS/Models/CMSFile.csweb/Areas/CMS/Models/CmsFileMapper.csweb/Areas/CMS/Models/DTOs/CmsFileDto.csweb/Areas/CMS/Models/DTOs/CmsFileImportDtos.csweb/Areas/CMS/Models/DTOs/CmsFileRequests.csweb/Areas/CMS/Models/DTOs/CreateLinkDto.csweb/Areas/CMS/Services/CmsConcurrencyException.csweb/Areas/CMS/Services/CmsDownloadRateLimiting.csweb/Areas/CMS/Services/CmsFileAuditService.csweb/Areas/CMS/Services/CmsFileCrypto.csweb/Areas/CMS/Services/CmsFileEncryptionService.csweb/Areas/CMS/Services/CmsFileImportService.csweb/Areas/CMS/Services/CmsFileResponse.csweb/Areas/CMS/Services/CmsFileService.csweb/Areas/CMS/Services/CmsFileStorageService.csweb/Areas/CMS/Services/CmsUserPhotoService.csweb/Areas/CMS/Validation/MaxLengthEachAttribute.csweb/Areas/CMS/Validation/SafeUrlAttribute.csweb/Classes/Utilities/DateRangeHelper.csweb/Program.csweb/Services/HtmlSanitizerService.csweb/Services/IHtmlSanitizerService.csweb/Viper.csprojweb/appsettings.json
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
✅ Action performedReview finished.
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/Areas/CMS/Services/CmsFileImportService.cs (2)
99-130: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
.AnyAsync()per line issues up toMaxBatchItems(500) sequential queries. Thef.FriendlyName == friendlyNameexistence check runs inside theforeach. Since preview is read-only, preload the folder's existing friendly names once and probe in memory alongsideplannedNames.As per coding guidelines: "Avoid nesting
.FirstOrDefault(),.Single(), or.Any()inside aforeachor.Select()over another list; materialize once with.ToDictionary()or.ToLookup()and useTryGetValueinstead."🤖 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 `@web/Areas/CMS/Services/CmsFileImportService.cs` around lines 99 - 130, The preview loop in CmsFileImportService currently runs an _context.Files.AnyAsync() query for every FilePaths entry, causing up to MaxBatchItems sequential database hits. Move the existing friendly-name lookup out of the foreach by preloading the folder’s current friendly names once, then check each generated friendlyName in memory together with plannedNames. Keep the existing ResolveSource, plannedNames, and friendlyName generation flow, but replace the per-item AnyAsync probe with a cached lookup (for example a hash set or lookup built before the loop) and preserve the same duplicate/existing-file messaging.Source: Coding guidelines
303-322: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCatch
FormatException/CryptographicExceptionhere too.GenerateKeyForDb()can throw on a bad master key, and this block currently lets that escape and abort the whole batch instead of failing just the current file. Mirror the catch set used byEncryptSingleFileAsync.🤖 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 `@web/Areas/CMS/Services/CmsFileImportService.cs` around lines 303 - 322, The exception handling in CmsFileImportService should match EncryptSingleFileAsync so a bad master key does not abort the whole batch. In the current catch block for the file import flow, add handling for FormatException and CryptographicException alongside the existing IOException, UnauthorizedAccessException, InvalidOperationException, and ArgumentException cases. Keep the behavior consistent by setting result.Message from the caught exception and returning result so only the current file fails.
🤖 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.
Outside diff comments:
In `@web/Areas/CMS/Services/CmsFileImportService.cs`:
- Around line 99-130: The preview loop in CmsFileImportService currently runs an
_context.Files.AnyAsync() query for every FilePaths entry, causing up to
MaxBatchItems sequential database hits. Move the existing friendly-name lookup
out of the foreach by preloading the folder’s current friendly names once, then
check each generated friendlyName in memory together with plannedNames. Keep the
existing ResolveSource, plannedNames, and friendlyName generation flow, but
replace the per-item AnyAsync probe with a cached lookup (for example a hash set
or lookup built before the loop) and preserve the same duplicate/existing-file
messaging.
- Around line 303-322: The exception handling in CmsFileImportService should
match EncryptSingleFileAsync so a bad master key does not abort the whole batch.
In the current catch block for the file import flow, add handling for
FormatException and CryptographicException alongside the existing IOException,
UnauthorizedAccessException, InvalidOperationException, and ArgumentException
cases. Keep the behavior consistent by setting result.Message from the caught
exception and returning result so only the current file fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 607c4e1a-0d23-4b26-923b-c773ad7f3c35
📒 Files selected for processing (6)
test/CMS/MaxLengthEachAttributeTests.csweb/Areas/CMS/Controllers/CMSFilesController.csweb/Areas/CMS/Services/CmsFileEncryptionService.csweb/Areas/CMS/Services/CmsFileImportService.csweb/Areas/CMS/Services/CmsFileStorageService.csweb/Areas/CMS/Validation/MaxLengthEachAttribute.cs
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
rlorenzo
changed the title
Slice 1/3 of the restacked CMS migration (built from the verified branch tip, so every slice carries the OS-independent path handling and the review fixes). Adds the files-domain backend: storage, CF-compatible crypto, audit, import, trash purge job, user photos, options lookups, the rebuilt /CMS/Files download path, and download rate limiting, with their unit tests. The content/nav backend and the management SPA follow in slices 2-3.
Its strict JSON parser cannot read appsettings*.json (ASP.NET parses those as JSON-with-comments by design) and warned on every review touching them; CI already covers its lint surface.
rlorenzo
force-pushed
the
VPR-59-cms-r1-files-backend
branch
from
5b15989 to
8b2cee3
Compare
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
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.
Slice 1 of 4 stacked PRs for the CMS migration (VPR-59), restacked from the CI-verified branch tip. Replaces the earlier 6-PR stack (#245-#250): those slices were historical cut points, so code predating the branch's later OS-independent path handling and ReSharper/Fallow cleanups failed CI on Linux runners. Each new slice is built from the final file states, so every slice carries all fixes and passes the full gate set (verified locally per slice: lint, 2,400+ backend tests, 1,026 frontend tests, build).
Scope — files-domain backend
/api/cms/files: paged list with filters and folder counts, upload with name-conflict handling, edit with permission/person deltas and a 409 stale-edit guard, soft delete/restore with owner scoping, admin-gated permanent delete.CmsFileStorageService(containment-checked paths, collision handling),CmsFileCrypto+CmsFileEncryptionService(CF-compatible AES so the legacy ColdFusion app can still read encrypted files),CmsFileAuditService, import-from-legacy-webroot, 30-day trash purge job (Cms:TrashPurgeEnabled, default off)./api/cms/photos, by mail/login/iam/mothra) with Last-Modified/304 conditional caching./CMS/Fileshandler (token bucket for ZIPs, sliding window for files) and the rebuiltData/CMS.csdownload path with legacy-parity access auditing.CMSOptionsControllerfor permission/person typeahead; unit tests for all of the above.All 27 review comments from the previous stack are addressed in this restack (fixes) or answered on the old PRs (declines with rationale).
Stack: this PR → content blocks/left nav backend → management SPA. Merging this slice alone ships inert-but-tested APIs.