72 lines
6.3 KiB
Plaintext
72 lines
6.3 KiB
Plaintext
upgrades not implemented
|
|
Honest self-assessment, ranked by actual impact:
|
|
|
|
Things that are genuinely broken:
|
|
|
|
The audit log has a silent XSS vector. renderAuditLog() inserts sanitized details into innerHTML. If someone crafts input that bypasses the regex patterns (e.g., nested tags, unicode escapes, or pre-encoded entities like <script), it executes inside the audit table. The sanitizer is defense-in-depth, not a guarantee. The fix is to stop using innerHTML for user-sourced data — use textContent for the details column, or better, construct DOM nodes. This is the single most important fix.
|
|
|
|
The CSRF implementation is a placeholder labeled as production-ready. The token is generated client-side and validated client-side. The Security Dashboard shows a green dot. A real CSRF token must be server-issued and server-validated — that's the entire point. The client generating its own token and checking its own token provides zero protection. It should be labeled "demonstrational" in the dashboard, or the dashboard should distinguish between "running locally" and "requires server."
|
|
|
|
The localStorage audit chain is tamper-evident, not tamper-proof. Any script on the same origin (browser extension, devtools console, another tab on the same domain) can call localStorage.setItem('fortkart_audit', ...) with forged entries that have valid hash chains. The "Verify Chain" button would report success on tampered data. The fix is either an HMAC with a key that only exists in memory (not persisted), or being explicit that this is session-only and not a trust boundary.
|
|
|
|
Things that are missing and matter for real use:
|
|
|
|
No prefers-reduced-motion respect. The card hover transforms, view fade-ins, toast slide-ins, and spinner all run regardless of the user's motion sensitivity setting. Three lines of CSS fix this:
|
|
|
|
css
|
|
|
|
|
|
|
|
|
|
@media(prefers-reduced-motion:reduce){*,*::before,*::after{animation-duration:.01ms!important;animation-iteration-count:1!important;transition-duration:.01ms!important}}
|
|
|
|
|
|
|
|
No keyboard accessibility on interactive elements. Product cards, category pills, parametric chips, and tier rows are all divs with onclick. They can receive focus but don't respond to Enter/Space, don't have role="button", and don't show focus rings. This fails WCAG 2.1 Level A. Fix: add tabindex="0", role="button", onkeydown="if(event.key==='Enter')this.click()" to each, and a visible focus style.
|
|
|
|
No focus management when modals open or close. Opening the product modal doesn't move focus inside it. Closing it doesn't return focus to the card that triggered it. Screen readers announce nothing. The cart sidebar has the same problem. This is WCAG 2.4.3 and 2.1.1.
|
|
|
|
No search debounce. Every keystroke re-renders the entire product grid. With 30 products it's invisible. With 300 it wouldn't be. A 200ms debounce on the search input costs nothing and prevents unnecessary work.
|
|
|
|
Parametric filters reset on category switch. If I filter ball screws to "C3 grade, 20mm diameter", switch to spindles to compare something, then switch back — my filters are gone. The filters should persist per-category in a separate object.
|
|
|
|
Things that are technically correct but architecturally wrong:
|
|
|
|
The pure-JS SHA-256 fallback is unverified. I wrote it by hand from the spec. It handles the standard test vectors (I believe — I didn't actually run the NIST CAVS vectors against it). If it produces a single wrong bit, every audit entry on file:// origins has a corrupted chain. The honest fix: if crypto.subtle is unavailable, disable the audit log entirely rather than silently using an unverified implementation. Show a warning banner instead.
|
|
|
|
JSON.stringify with sorted keys is not a robust canonicalization. undefined values are silently dropped, NaN/Infinity become null, and some Unicode edge cases behave inconsistently across engines. If audit details ever contain these values, the same entry hashes differently in different browsers, breaking chain verification across environments. The fix is to normalize all values before stringifying — or restrict audit details to strings, numbers, and plain objects (which they currently are, but this isn't enforced).
|
|
|
|
Things I'd add for the business domain:
|
|
|
|
No order history view. After placing an order, the only exit is "Continue Shopping." The order is in state.orderHistory but there's no UI to view past orders. For a B2B parts supplier, order history is essential — engineers reference past orders constantly.
|
|
|
|
No tier indication in the cart. The cart shows "$89.99 ea" but doesn't tell the user "you're at the 5+ tier ($82.99)" or "add 2 more to hit the next break." This is where the savings actually happen and the user can't see it.
|
|
|
|
No way to clear a single item from the cart (only reduce to zero, which calls removeFromCart). Minor UX issue, but the delete button is too small on touch devices.
|
|
|
|
Things I would NOT change:
|
|
|
|
Rewriting HTML concatenation to DOM construction. It's more robust but 3-5x more verbose for zero practical benefit here, since all user data passes through the sanitizer. The right fix is making the sanitizer correct, not changing how rendering works.
|
|
Adding a product comparison view. Nice feature, but complex, and the modal already shows full specs. This is a catalog, not a configurator.
|
|
Skeleton loading. The app initializes in under 200ms. Skeletons would flash for fewer frames than the actual content takes to appear.
|
|
Replacing the IIFE SecurityEngine with class syntax. It's an unusual pattern, but the app already requires async/await, TextEncoder, and crypto.subtle — all modern APIs. The IIFE is consistent with the rest of the code's conservative style. Changing it adds zero functionality.
|
|
|
|
The order I'd actually implement them, if I were shipping this:
|
|
|
|
The prefers-reduced-motion rule is 3 lines. The search debounce is 5 lines. The CSRF label change is 1 word. Those three go first because they're trivial and correct real deficiencies.
|
|
|
|
Then the innerHTML fix in the audit log, because it's a real XSS vector. Then keyboard accessibility on product cards. Then focus management on modals. Then the rest as time allows.
|
|
|
|
Want me to implement any of these?
|
|
|
|
|
|
|
|
|
|
|
|
All 13 have sound reasoning.
|
|
Is it possible to add shipping estimation and taxes to the shoping cart?
|
|
|
|
|
|
|
|
|
|
|