Audit 2: Post-Remediation Document Review
Second review of all documents in the 3-frontend-implementation/ project
directory and referenced parent project context, performed after the
Audit 1 remediations were applied. Focuses on residual issues,
issues introduced by the remediation changes, and new findings not covered in
the first pass.
Audit 1 Remediation Verification
Section titled “Audit 1 Remediation Verification”All 28 findings from Audit 1 have been addressed. Spot-checks confirm:
- #1 (path): BFF spec, 3.5 T-5, SPA spec, and TD-13 all use module-scoped
POST /api/image-upload/POST /v1/item/image-upload. Consistent. - #7 (Max-Age): Present in 3.5 T-8, requirements REQ-FE-017, BFF spec.
- #8 (Content-Type): Validation steps added to 3.5 T-6 and T-7 with tests.
- #10 (cache TTL): 8-hour default in 3.5 T-1c with eviction logic.
- #11 (403 recovery):
useImageWithCdnRecoveryhook in 3.6 T-7b, wired in T-8 and T-9. - #14 (isEqual): Added to 3.2 T-4
UseDraftOptions<T>with tests. - #25 (X-oidc-subject): Added to 3.5 T-5 step 3 and REQ-FE-010.
- #28 (CORS): CloudFront policy in infrastructure#439,
crossOriginspec in 3.3 T-1 change #7.
New Findings
Section titled “New Findings”| # | File | Lines | Issue Summary | Severity | Recommendation | Action |
|---|---|---|---|---|---|---|
| 1 | index.md (parent) | Lines 72-78 | SD-01 still shown as “Pending” in Open Decisions table. The decision log records SD-01 as “Resolved” (via TD-01/TD-09 — SPA-side fetch-and-store). The index.md Open Decisions table is stale. SD-10 also says “pending effort confirmation” but is in scope with auto-compression implemented in the UX components. | Low | Update index.md Open Decisions: SD-01 → “Resolved (TD-01/TD-09)”, SD-10 → “In scope (implemented)”. Remove resolved items or mark their status. | Agreed |
| 2 | risks-and-assumptions.md | Line 19 (R-003) | R-003 (CORS restrictions) not updated with FD-17 mitigation. R-003 describes CORS as a risk for external URL fetching. The new FD-17 (CloudFront CORS for canvas editing) is a related but distinct CORS concern — CDN-to-SPA cross-origin for the edit-existing flow. R-003’s mitigation doesn’t mention the canvas tainting issue or the CloudFront Response Headers Policy. | Low | Add a note to R-003 referencing FD-17 and infrastructure#439 as the mitigation for the CDN-to-SPA CORS scenario. Keep the existing mitigation for external URL CORS (BFF fallback). | Agreed |
| 3 | goal.md constraint 12 vs FD-13 | goal:351-354 | Constraint conflicts with decision. Goal.md constraint 12 says “The SPA never imports @arda-cards/api-proxy” and “@arda-cards/api-proxy is used exclusively in Next.js server-side API routes.” FD-13 says SPA types are re-exported from api-proxy through a shared barrel, which creates a compile-time import from the SPA layer. The constraint text is now inconsistent with the decided approach. | Medium | Update constraint 12 to say: “@arda-cards/api-proxy is used at runtime exclusively in server-side API routes. SPA code may import type-only re-exports from the package (FD-13) but must not import runtime code or access the Backend API key.” This preserves the security intent (no API key in SPA) while allowing type sharing. | Option (a) |
| 4 | 3.6-spa-integration spec (T-7b) | useImageWithCdnRecovery | CDN URL detection mechanism not specified. The hook checks if a URL “matches *.assets.arda.cards” but doesn’t specify how the SPA determines the CDN host pattern. The server-side resolveAwsNaming().cdnHost (3.5 T-1b) is not available to client code. The pattern could be hardcoded, derived from NEXT_PUBLIC_* env var, or inferred from the URL structure. | Medium | Specify the detection mechanism. Recommended: add a NEXT_PUBLIC_CDN_HOST environment variable (e.g., dev.alpha002.assets.arda.cards) set alongside the existing infrastructure env vars. The hook checks url.includes(cdnHost). Alternatively, check for the fixed suffix .assets.arda.cards. | The structure of the CDN host is always: <partition>.<infrastructure>.assets.arda.cards so it can be derived from the values of provided Infrastructure and Partition environment variables. Implement this in the SPA code similar to how the server side does it, or move those naming convention capabilities to shared utilities. |
| 5 | 33-component-updates spec (T-4) vs 3.3 T-2 | T-4 lines 132-138 | ItemGridEditorHooks has optional hooks but ImageCellEditorConfig requires them. T-2 made useImageUpload and useCheckReachability required in ImageCellEditorConfig (FD-15). But T-4’s ItemGridEditorHooks still has them as optional (?). When the grid column factory calls createImageCellEditor(), it must handle the case where ItemGridEditorHooks doesn’t provide the hooks, even though ImageCellEditorConfig requires them. | Medium | Either: (a) make the hooks required in ItemGridEditorHooks too, which pushes the obligation to the app’s grid assembly point (Phase 3.6 T-8), or (b) document that the grid column factory only wires the image editor when hooks are present (skip image editing when hooks aren’t provided). Option (a) is more consistent with FD-15. | Option (a) |
| 6 | specification.md Phase 3.3 overview | Line 146 | Phase 3.3 task count and entry criteria stale. The overview says “Tasks: 6” but the 3.3 spec now has 7 tasks (T-1 gained change #7 for crossOrigin). The entry criteria says “Phase 3.2 complete” but the 3.3 spec now also requires infrastructure#439 deployed. | Low | Update specification.md Phase 3.3: Tasks → 7, Entry criteria → “Phase 3.2 complete + infrastructure#439 deployed (for crossOrigin in T-1)”. | Agreed |
| 7 | specification.md Phase 3.6 overview | Line 196 | Phase 3.6 task count stale. The overview says “Tasks: 10” but the 3.6 spec now has 12 tasks (T-7b added, T-11 is documentation). | Low | Update specification.md Phase 3.6: Tasks → 12. | Agreed |
| 8 | 3.6-spa-integration spec (T-4) | Lines 104-105 | useCdnCookies refetchInterval hardcoded, not coupled to cookie TTL. refetchInterval: 15min is hardcoded as ~50% of the default 30-min TTL (REQ-FE-062). But cookie TTL is configurable (REQ-FE-018). If the TTL changes, the refetch interval must be updated separately. No coupling mechanism exists. | Low | Add a note that refetchInterval should be derived from the cookie TTL configuration (e.g., COOKIE_TTL_MS / 2). Or introduce a shared constant that both the signer and the cookie query reference. | Introduce Shared Constant. |
| 9 | 3.5-bff-routes spec (T-5) | Lines 486-504 | T-5 handler constructs ItemProxy but doesn’t pass RequestContext. Step 2 constructs ItemProxy with { host, apiKey }. Step 3 says “Inject headers from UserContext.” But with Phase 3.1’s RequestContext support, the proxy should receive context via the RequestOptions parameter on createImageUploadUrl(request, options). The spec doesn’t show the RequestContext being passed to the proxy method — it describes header injection as if it’s manual, but the whole point of Phase 3.1 was to make the proxy handle this. | Medium | Rewrite T-5 steps 2-4 to use the Phase 3.1 RequestContext pattern: construct ItemProxy with ProxyConfig, then call proxy.createImageUploadUrl(request, { context: { author, tenantId, userId } }). Remove the manual header injection description — the HttpClient handles it via RequestOptions. | Agreed. |
| 10 | verification.md | Throughout | Verification matrix not updated for new requirements. The matrix covers REQ-FE-001 through REQ-FE-NF-005 (52 items). New specifications from the audit (crossOrigin canvas behavior, CDN recovery hook, Content-Type validation, cache TTL, Max-Age cookie attribute) don’t have explicit REQ-FE entries or verification rows. They are specified in sub-phase docs but lack formal traceability. | Low | Either: (a) add new REQ-FE entries for the audit-driven additions and extend the verification matrix, or (b) accept that sub-phase specification tasks serve as the verification anchor for audit-driven changes. Document the choice. | Option (a) |
| 11 | SPA spec (spa-specification.md) | Lines 44, 99, 173 | SPA-FR-009 source references still cite FR-018/FR-019. These source requirement IDs referred to the original checkbox-based copyright flow. With the change to inline text (no blocking), the source requirements may need updating in the parent requirements doc, or the FR IDs may be correct but their content has evolved. Verify FR-018/FR-019 in the parent requirements doc match the new behavior. | Low | Verify that FR-018 and FR-019 in the system-design requirements document match the inline-text behavior. If they still describe a checkbox, update them. | The decision is to not have a check box but instead have a note to the user that by providing images they are accepting that they have rights to it. No Checkbox. Update all documents to reflect this decision (from features down). Record the decision in the decision log. |
| 12 | 37-release spec (T-2) | Lines 55-64 | CHANGELOG listing incomplete for audit-driven additions. T-2 lists Added items for arda-frontend-app but doesn’t include: src/server/lib/aws-naming.ts (naming convention utility), src/server/lib/secrets.ts (generic secrets with TTL), useImageWithCdnRecovery hook, NEXT_PUBLIC_CDN_HOST env var (if adopted per finding #4). | Low | Update T-2 CHANGELOG listing to include all new utilities and hooks added during the audit remediation. | Agreed |
Severity Summary
Section titled “Severity Summary”| Severity | Count | Findings |
|---|---|---|
| Critical | 0 | — |
| High | 0 | — |
| Medium | 4 | #3, #4, #5, #9 |
| Low | 8 | #1, #2, #6, #7, #8, #10, #11, #12 |
No critical or high-severity issues remain. The 4 medium findings are consistency gaps introduced by audit-1 remediations (#3, #4, #5) and one pre-existing specification gap (#9) that should be resolved before implementation begins. The 8 low findings are housekeeping items that can be addressed during implementation.
Copyright: (c) Arda Systems 2025-2026, All rights reserved
Copyright: © Arda Systems 2025-2026, All rights reserved