System Design Audit
Critical analysis of the Item Image Upload system design, examining inconsistencies, non-met requirements, ambiguities, and opportunities for improvement across all 12 design documents and their 4 key upstream references.
Category Legend
Section titled “Category Legend”- INC — Inconsistency (contradictions between documents)
- NMR — Non-Met Requirement (source requirement not covered)
- AMB — Ambiguity (underspecified or unclear)
- IMP — Opportunity for Improvement
Findings
Section titled “Findings”| # | Cat | Area | Finding | Rationale | Recommendation | Action |
|---|---|---|---|---|---|---|
| 1 | INC | Presigned POST expiry | NFR-006 and OAM-C-002 say 5 minutes; BE-FR-001, S1 diagram, and AWS spec say 15 minutes. NFR-006: “expire within 5 minutes.” OAM-C-002: “default 5 minutes.” BE-FR-001: “Signature duration: 15 minutes.” AWS policy table: “Expiry: 15 minutes.” | Direct contradiction. An implementer reading NFR-006 would use 5 min; reading the Backend spec would use 15 min. The 15-minute value is more detailed (aligned with existing uploadSignatureDuration) so is likely correct. | Pick one value (15 min) and update NFR-006 and OAM-C-002 to match. | 15 min, make the updates |
| 2 | INC | Copyright logging | SD-07 and GEN::MEDIA::0001::0006.FS say the copyright acknowledgment is “logged with timestamp and user identity.” TD-04 and FR-019 say “No backend logging required.” The feature spec was listed as an updated source document, but the logging language in 0006.FS was not removed. | The design intentionally made this SPA-only (TD-04), but the upstream feature spec still mandates server-side logging. Either the feature spec needs to be updated to reflect TD-04, or the design needs to add a logging mechanism. As-is, an implementer reading the feature spec would implement logging the design explicitly excludes. | Update GEN::MEDIA::0001::0006.FS to remove “logged with a timestamp and the authenticated user’s identity” or change it to “SPA-only acknowledgment; no server-side logging (TD-04).” Also update SD-07 which says “logged with timestamp and user identity.” | Mark the logging functionality as out of scope for this release (scoping document) and update other documents as needed |
| 3 | INC | CDN URL construction | S1 narrative (scenarios.md:149) says “The SPA constructs the CDN URL from the object key.” BE-FR-007 says the Backend returns the cdnUrl alongside presigned form fields. The Backend response schema includes cdnUrl as a top-level field. | If the Backend returns the CDN URL, the SPA doesn’t construct it — it uses the value from the response. The S1 narrative contradicts the Backend spec. | Update S1 narrative to say “The SPA uses the cdnUrl from the Backend response” rather than “constructs.” | Agreed |
| 4 | INC | Presigned PUT reference | aws-specification.md:104 cross-reference table says “Presigned PUT URL — Key is embedded in the URL path; signed headers enforce metadata.” TD-08 explicitly chose presigned POST over PUT. All other references correctly say POST. | Stale reference from pre-TD-08 draft. Could confuse an implementer reading the AWS spec in isolation. | Replace “Presigned PUT URL” with “Presigned POST form” in the cross-references table. | Agreed, It must make the signature presecription correct also for POST method |
| 5 | INC | BFF endpoint path | Upload-url uses singular /api/item/<itemEId>/image-upload-url. Entity update uses plural /api/items/<itemEId>. Both appear in the SPA interfaces table and S1 diagram. The Backend also shows singular /v1/item/<itemEId>/image-upload-url vs plural for the existing entity endpoint. | Inconsistent pluralization between new and existing endpoints. The new endpoint should follow whatever convention the existing entity endpoints use. | Verify the existing convention (singular vs. plural) and make the new endpoint consistent. | Convention is singular. Check with current existing API’s in documentation |
| 6 | NMR | Minimum dimensions | GEN::MEDIA::FR-0007: “The system shall support configurable minimum image dimensions per entity type.” Not present in any system-level requirement (FR-001 through FR-039). Not in ImageFieldConfig fields. Not mentioned in any actor spec. Not explicitly excluded in scoping.md. | This feature requirement is completely missing from the system design. The use case spec 0003.FS mentions “Minimum dimensions: Defined per entity type where applicable.” The SPA validation logic should enforce it. | Either add a system-level requirement (e.g., FR-040) and a minDimension field to ImageFieldConfig, or explicitly scope it out in scoping.md with rationale (e.g., “Not needed for Items; deferred to first entity type that requires it”). | Configurability is postponed, The system will implement reasonable defaults for V1. |
| 7 | NMR | Feature FR-0016 | GEN::MEDIA::FR-0016: “Replacing an existing image shall use the same interaction flow as setting an image for the first time.” No system-level requirement tracks this. FR-016 in the system design covers comparison layout (a different concern). | The “same flow” guarantee is implicit in the scenario design (S7 reuses S1), but no requirement ID verifies it. An implementation could diverge without detection. | Add explicit traceability: either map a system requirement to FR-0016 or add it to the Scenario Cross-Reference table as a covered use case property. | Agreed |
| 8 | NMR | Feature FR-0019 | GEN::MEDIA::FR-0019: “Images provided via file, clipboard, or data URI shall be uploaded to platform-managed storage and served through a CDN.” No system-level requirement explicitly traces to this. FR-0020 (URL fetch-and-store) has traceability, but FR-0019 (the basic managed upload path) does not. | The system design implicitly covers this via FR-021 (direct upload) and TD-06, but the traceability chain from GEN::MEDIA::FR-0019 is broken. | Add FR-0019 to the Source column of FR-021 or FR-022. | Agreed |
| 9 | NMR | Feature FR-0015 optionality | GEN::MEDIA::FR-0015: “The system shall support optional crop functionality per entity type.” System design FR-015 makes crop mandatory with locked aspect ratio. ImageFieldConfig has no flag to disable cropping. | For entity types where crop doesn’t apply (e.g., a simple logo upload), the design forces the full editor. The feature spec’s “optional per entity type” is lost. | Add a boolean cropEnabled (or similar) to ImageFieldConfig, or document in scoping.md that for V1 crop is always enabled and optionality is deferred. | Exclude from V1. Make annotation in scoping.md |
| 10 | AMB | Orphan cleanup | NFR-012 says orphans “shall be cleaned up by S3 lifecycle rules on a staging prefix (7-day expiration).” But the AWS spec explicitly states “The current design uses a direct upload to the final key (no staging prefix).” The lifecycle rule targets staging/ which is never used. | NFR-012 is unmet by design. The requirement promises cleanup that the architecture doesn’t deliver. The risk doc (R-002) correctly notes the gap but NFR-012 is stated as a “shall” requirement. | Rewrite NFR-012 to say orphaned objects remain until a future cleanup mechanism is implemented, or remove the “staging prefix” reference and describe the actual approach (periodic scan job, deferred). | Agreed |
| 11 | AMB | Data URI scenario | FR-007 covers data: and blob: URI text entry, but no scenario (S1 through S7) explicitly covers this input method. S1 covers file/drag-drop/clipboard. S2 covers HTTPS URL. Data URI text is a distinct input detection path. | An implementer might overlook data URI handling since it has no scenario diagram or narrative. It’s implicitly part of S1’s “detect input type” step, but the routing logic is different (decode rather than upload directly). | Either add a brief narrative to S1 explaining the data URI sub-path, or add FR-007 to the S1 requirements list in the cross-reference table. | Both. It should also be added to the SPA Specification. |
| 12 | AMB | Item creation upload ordering | S6 shows draft-save (Step 2) before image upload (Step 3). The upload-url endpoint requires <itemEId>, which comes from the draft. But what if the user activates the image area before filling any other fields? | The design assumes draft-save always precedes image upload, but doesn’t state this as a constraint or explain how the SPA handles the case where no draft exists yet. | Add a note to S6 or SPA-FR-010 that presigned credential requests require a persisted entity ID, so the SPA must ensure a draft exists before requesting upload credentials. | Agreed |
| 13 | AMB | 403 retry limit | SPA-FR-019 says “On 403 from CDN, refresh cookies and retry.” No retry limit is specified. | Could cause an infinite retry loop if cookie signing is misconfigured. Each 403 triggers a BFF call, which triggers a new 403, etc. | Specify a max retry count (e.g., 1 or 2 retries per image request) and a fallback behavior (show error badge after exhausting retries). | Max 1 retry |
| 14 | AMB | BFF rate limiter state | BFF spec says rate limiter “tracks per-tenant request counts in memory (or via a shared cache if the BFF scales horizontally).” The parenthetical “or via shared cache” is left unresolved. | If the BFF runs multiple instances (likely in k8s), in-memory rate limiting is ineffective — each instance has its own counter. This should be a design decision, not a deferred detail. | Decide and document whether rate limiting is per-instance (simpler, approximate) or shared (e.g., Redis-based, precise). At minimum, state the assumption about BFF scaling. | BFF is an AWS Amplify Next Js application. Recommend the appropriate mechanism for that implementation platform |
| 15 | AMB | Cookie domain scope | Cookies are set on .arda.cards (leading dot = all subdomains). This means api.arda.cards, admin.arda.cards, or any other *.arda.cards subdomain receives the CloudFront cookies on every request. | Unnecessary cookie overhead on non-CDN requests. Potential information leakage to unintended services. If a subdomain is compromised, it receives the cookies. | Consider scoping the cookie domain more tightly to assets.<env>.arda.cards if browser behavior allows it, or document the risk and accept it as a trade-off for simpler configuration. | Document and Accept. |
| 16 | AMB | BE-FR-004 vs BE-FR-008 | BE-FR-004: “verify uploaded object exists via HEAD.” BE-FR-008: “validate object’s S3 metadata via HeadObject.” These are specified as separate requirements but use the same S3 operation. | An implementer might think these are two distinct checks requiring two HEAD requests. They should be a single HEAD request that validates both existence and metadata. | Merge into one requirement or add a note that both checks are performed in a single HeadObject call. | Agreed |
| 17 | INC | FR numbering order | FR-034 and FR-035 (Item-Specific) appear after FR-036 through FR-039 (CDN Access Control) in the document. Similarly, NFR-017 through NFR-020 appear after NFR-016. | The later-added CDN requirements broke the sequential ordering. While the IDs are unique, the non-sequential presentation is confusing. | Physically reorder the document sections so Item-Specific (FR-034 through FR-035) appears before CDN Access Control (FR-036 through FR-039), or renumber to be sequential. | Agreed |
| 18 | IMP | Upload progress | No requirement or scenario addresses upload progress indication. R-006 mentions “The SPA can detect upload progress and show a progress indicator” but it’s only in the risk mitigation, not a requirement. For 10 MB files this matters. | Users uploading large images over slower connections have no visual feedback during the S3 POST. The SPA state machine has Uploading state but no progress tracking is specified. | Add a non-functional or SPA-level requirement for upload progress indication (percentage or indeterminate) during the Uploading state. | Agreed |
| 19 | IMP | Accessibility | No accessibility requirements anywhere in the design. The image editor (crop/zoom/rotate/pan), drag-and-drop, hover popover, and modal overlay all have significant accessibility implications. | WCAG compliance is typically expected for web applications. The drop zone, editor controls, modal focus management, and hover-to-inspect all need keyboard and screen reader support. | Add NFR requirements for keyboard navigation of the editor, focus trap in modal overlay, alt-text for thumbnails, and screen reader announcements for upload states. | Add accessibility requirements and note in scoping that they are out of scope. |
| 20 | IMP | Concurrent upload guard | No requirement prevents the user from initiating a second upload while one is in progress. The ImageUploadDialog state machine has Uploading but doesn’t specify behavior if the user triggers another upload (e.g., from a different grid row). | Could result in race conditions, orphaned uploads, or confusing UX if two uploads complete out of order. | Add a requirement or SPA state constraint that only one upload can be active per browser session (or per entity), with appropriate UI disabling. | Agreed, per browser session. |
| 21 | IMP | Image format after edit | FR-008 accepts HEIC/HEIF, but the design doesn’t specify the output format after crop/zoom/rotate. The editor produces a Blob — is it always JPEG? PNG? The original format? | If the user uploads a HEIC and crops it, the output Blob format matters for quality, file size, and Content-Type in the presigned POST. The SPA external dependency browser-image-compression compresses to JPEG, suggesting JPEG is the output, but this isn’t explicit. | Specify the output format policy: e.g., “The editor always produces JPEG output (for photos) or PNG (if transparency is needed). HEIC/HEIF inputs are converted to JPEG during editing.” Add to SPA spec or ImageFieldConfig. | Agreed. JPEG output |
| 22 | IMP | Content-Length for URL-sourced images | S2 shows the SPA fetching an external image, but the upload-url request requires contentLength. When the SPA requests presigned credentials (Step 4), it needs the final file size. But the final size isn’t known until after crop/compress (Step 3). | The sequence implies credentials are requested after crop (which is correct per the diagram), but the narrative doesn’t emphasize that credential request must wait for the final Blob to be produced. For URL-sourced images, the SPA must fetch, edit, produce Blob, measure size, then request credentials. | Add a clarifying note to S2 that the presigned credential request happens after the final Blob is produced (post-crop), not after the initial fetch. | Agreed |
| 23 | IMP | Phasing: CDN cookies not in phases | The phasing document doesn’t mention the cdn-cookies BFF endpoint or CloudFront signing key infrastructure. Phase 3 (BFF) lists upload-url proxy, check-url, and fetch-url but not cdn-cookies. Phase 1 (AWS) doesn’t mention CloudFrontSigningKeyGroup. | The CDN access control (TD-11, TD-12) is a significant work stream that cuts across AWS (key group construct), BFF (signing endpoint), and SPA (cookie lifecycle). Omitting it from phasing means no one plans for it. | Add the CDN cookie infrastructure to Phase 1 (CloudFrontSigningKeyGroup construct), Phase 3 (cdn-cookies route, cloudfront-signer module), and Phase 4 (SPA cookie lifecycle manager). | Agreed |
| 24 | IMP | Testing gap: CDN cookies | Backend testing strategy (5 test classes) doesn’t cover CDN cookie signing. The BFF spec identifies cloudfront-signer.ts but no test strategy is defined for cookie generation, tenant scoping, TTL, or key rotation. | Cookie signing is security-critical (tenant isolation). Untested cookie generation could produce cookies with wrong tenant scope or expired TTL, breaking all image display. | Add a BFF testing strategy section (or a note in the phasing document) covering: cookie policy generation, tenant scope correctness, TTL enforcement, and key rotation simulation. | Agreed |
Summary
Section titled “Summary”The design is thorough overall — the actor decomposition, traceability web, and security analysis (CDN access control document) are strong. The most critical issues are:
- #1 — Presigned POST expiry contradiction (5 min vs 15 min) that would cause implementation bugs.
- #2 — Copyright logging contradiction with the upstream feature spec.
- #6 —
GEN::MEDIA::FR-0007(minimum dimensions) completely missing from the design. - #10 — NFR-012 promises orphan cleanup the architecture doesn’t deliver.
- #23 — CDN cookie infrastructure absent from phasing, risking unplanned work.
The remaining items range from traceability gaps to missing edge case coverage that should be addressed before implementation begins.
Copyright: (c) Arda Systems 2025-2026, All rights reserved
Copyright: © Arda Systems 2025-2026, All rights reserved