77565723d63a73aca3dfb1833be9211ee688bbff
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 bermudalamb
|
f32913ef51 |
refactor: turn on noUncheckedIndexedAccess in both workspaces (#101)
Indexing an array now yields `T | undefined`, which is what it always did — the compiler simply says so. Enabled in backend/tsconfig.json and frontend/tsconfig.json, and in tsconfig.sonar.json alongside it so the drift guard stays satisfied. The sequencing this issue insisted on was right, and the numbers show why. Before #159 typed the query results, `rows[0]` was `any` and `any` indexes to `any`: the flag would have found close to nothing and the count would have changed completely afterwards. With the rows typed it finds 57 — 50 in the backend, 4 in the frontend, 3 in the Playwright suite — and they fall into three kinds. Thirty are `rows[0]` after a `rows.length` guard. TypeScript cannot connect the two, and rewriting them as `const [row] = rows; if (!row) …` makes the guard and the use the same check, which is better code independently of the flag. Ten are rows a statement guarantees — `INSERT … RETURNING`, or a lookup for an id the session middleware has already matched. These get `requireRow(rows, what)`, a new helper in db.ts that throws naming the query. A thrown error rather than a non-null assertion: if the assumption is ever wrong, an assertion hands `undefined` to the next line and fails somewhere unrelated, whereas this fails at the query and says which. asyncRoute turns it into a 500, which is the right answer for "the database did not do what the statement says it does". It also states the assumption once instead of ten times. The rest is ordinary indexing the compiler cannot prove: a regex capture group that the pattern guarantees, `split('+')[0]`, a modulo kept in range, `hasOwnProperty` failing to narrow an index signature, and Express typing route params as an index signature so `req.params.itemId` is `string | undefined` on a route that cannot match without it. One correction to this issue's premise, which matters for what it was expected to find. The body says "in a handful it does not guard at all", and the unguarded-500 risk it describes was not found. Every `rows[0]` either sits behind a length check or behind a statement that guarantees a row. What the flag actually bought was the ten places where that guarantee was real but unstated, and those now say so. Two changes worth calling out because they are not mechanical. TAG_COLORS is typed `[string, ...string[]]` in both copies rather than `as const` — the first attempt used `as const`, which narrowed the elements to literals and broke adminTags, so the annotation keeps `string` while telling the compiler index 0 exists. And the filter drawer's slider falls back to the bounds it was given rather than to null, because null there reads as "no filter" and would widen the results rather than leave them unchanged. Test files needed changes too, since ts-jest compiles them against the same config: a regex destructure in the compose guard, and ten `mock.calls[0][0]` reads where the surrounding assertions already establish the call happened. Verified: tsc clean over backend, frontend src and the Playwright suite; unit 254/254; integration 238/238; frontend build clean; lint unchanged in both workspaces. Closes #101 |
||
|
bermudalamb
|
d43e2d5871 |
refactor(backend): type the admin item queries, and fix the stale status union (#159)
admin.ts has no untyped reads left. Typed sites go from 43 to 49. New ItemRecord in itemSelect.ts for the bare `items` row that `RETURNING *` gives back. Deliberately not AdminItemRow: that describes a select which joins the category and adds images and tags as subqueries, so typing a RETURNING * as it would promise three fields the result does not contain. Three shapes for one table, because three different queries return three different things. The typing found a real defect on its first run, which is the case for doing this at all. `ItemStatus` in types.ts was `'available' | 'reserved' | 'sold'`. The database has four values and defaults to 'pending' — items have arrived pending since #90. itemFilters.ts declared its own copy that had all four and was correct. Two declarations of one union with nothing connecting them: one went stale and nothing said so. It was invisible while query rows were `any`. Typing them turned `if (status === 'pending')` in admin.ts into TS2367, "this comparison appears to be unintentional because the types 'ItemStatus' and '\"pending\"' have no overlap" — a compiler telling us the unpublish route's guard could never be true, against a type that was simply wrong. Confirmed against the database rather than by picking the more plausible of the two declarations: `SELECT DISTINCT status FROM items` returns pending, available, reserved and sold. Fixed by removing the duplication rather than by patching both copies. types.ts now holds the only declaration and itemFilters.ts imports it, re-exporting so its existing importers are unaffected. Patching both would have left the next drift free to happen the same way. Verified: tsc clean, unit 254/254, integration 238/238, and backend lint unchanged — the four warnings it reports are identical to those on main with these changes stashed, so none of them are new. Refs #159 |
||
|
bermudalamb
|
a75d9fe155 |
refactor(backend): type the item query results (#159)
First stage of typing the query results, and the one that sets the pattern. `pg` types `rows` as `any[]`, so every row this application reads entered a strict codebase as `any` — 1 of roughly 184 query sites carried a type before this. The row types live in itemSelect.ts, beside the selects that produce them, rather than in types.ts. They describe a projection rather than a table, and the two projections differ on purpose: ADMIN_ITEM_SELECT takes `i.*` while PUBLIC_ITEM_SELECT names its columns so the storefront never sees paypal_order_id or reserved_until. Typing both as "an items row" would quietly re-admit exactly the columns that select was written to exclude, so PublicItemRow and AdminItemRow share a base and the admin one adds the three fields it is allowed. types.ts gains ItemTag, which the tags subquery has always built and nothing had named. What this buys, demonstrated rather than claimed: introducing `rows[0].price_cent` at a read site now fails the build with "Property 'price_cent' does not exist on type 'ItemRowBase'. Did you mean 'price_cents'?". Before this it compiled, returned undefined, and reached the customer as an empty price. What it does not buy is written into itemSelect.ts rather than left for the next reader to assume. `pool.query<T>` asserts a shape; it does not check the SQL, which TypeScript never reads. Dropping a column from a select without dropping it from its type compiles cleanly and every read goes on type-checking while being undefined at runtime. The selects and their types are kept in step by hand, and the integration suite is the only thing that catches them disagreeing, because it runs the real queries against a real schema. The acceptance criteria on #159 originally claimed the compiler would catch that; it will not, and the issue has been corrected. Verified: tsc clean, and the four integration suites that exercise these selects pass 87/87. Refs #159 |
||
 bermudalambandClaude Opus 5
|
4ed9513ad2 |
feat(backend): make the five customer emails editable copy (#92)
Every customer email was a template literal in the route that sent it, so changing a word meant a code change, a review and a deploy. All five now render from markdown that an admin can edit: verification, password reset, favorite sold, favorite withdrawn, and the cart reminder. Five, not the four the issue counted — the favorite alerts have separate copy for sold and withdrawn.
markdown-it runs with html disabled, which is its default and the reason for choosing it over marked. Raw HTML in a stored body is escaped rather than passed through, so editing copy cannot put script into a customer's inbox. That is a stronger guarantee than sanitising output, because there is no output to sanitise.
Values are substituted into the markdown before it renders, which means a value that should become a list has to arrive as markdown. The cart reminder previously built li elements by hand; those would now be escaped and shown to the customer as literal angle brackets, so it emits a markdown list instead. The greeting is one placeholder rather than a bare name, so a template author writes {{greeting}} instead of "Hi {{firstName}}," — which reads as "Hi ," for anyone who registered before first names were required.
Saving is refused when a body has dropped a placeholder it needs, naming all of them rather than the first. This is the rule that separates a convenience from a way to break password resets from a settings screen: a reset email with no link still sends, still looks correct in the log, and is useless to everyone who receives it.
The favorite alerts' consent sentence is appended by the server and is not editable. It explains why the customer is receiving the mail, which is a compliance artifact rather than copy, and editing wording should not be able to delete it.
Unset templates fall back to the built-in defaults, so an install that never touches the settings screen behaves exactly as it did. The API reports an uncustomised template as null rather than as its default text, so "never edited" stays distinguishable from "edited to something identical", and DELETE restores the default by forgetting the row rather than writing the default into it.
Two problems surfaced during verification, both worth recording.
Five favorite-alert tests failed with no error and no mail. The cause was not this code: resetDb does not truncate admin_settings, so a subject of "Gone" stored by the new template tests survived into a later suite and changed the mail it was asserting on. Cleaning up inside the template tests would have fixed only that pairing, so resetDb now clears stored templates for every suite — template rows are test data like any other, and one outliving the suite that wrote it makes a failure appear somewhere unrelated.
The withdrawal notification then failed on timing. Loading copy from the database made the sender async, and the removal path was fire-and-forget, so the response could beat the mail out of the door. Dispatch was previously synchronous even though the sends themselves were not awaited; that is now restored by awaiting it.
Verified: 197 unit and 195 integration passing, lint unchanged at 4 warnings. The admin screen for editing these follows in the next commit.
Refs #92
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
bermudalambandClaude Opus 5
|
35b242a66f |
feat(backend): accept only real images in the inventory upload (#95)
The upload bounded size and count and nothing else: POST /api/admin/items would take a PDF, a zip or an executable and store it as an item image, under an extension copied from whatever the caller named their file. Those files are served by express.static from the application's own origin, so a stored .html came back as text/html and a .svg as image/svg+xml — both able to run script as the site. Three types are accepted: JPEG, PNG and WebP. SVG is excluded deliberately even though it is an image, because it executes script when navigated to directly, which is the exposure #103 describes; a photograph of a one-of-a-kind item is never a vector drawing, so nothing real is lost. GIF is excluded as simply not wanted for product stills. Validation happens twice, because once is not enough. The declared content type is checked in multer's fileFilter, before a byte is written — that catches picking a PDF by accident, which is most of what goes wrong. But file.mimetype is whatever the caller wrote in the multipart headers, so the bytes are checked too: each stored file's leading bytes must match the format it claimed. That is what stops evil.html renamed to photo.jpg and declared image/jpeg, which an allowlist on the declared type alone waves straight through. The byte check cannot live in fileFilter — that runs before multer has read the stream, so there is nothing to look at yet. It runs after the write instead, and a failure removes every file from the request rather than only the offending one: accepting the good half of a refused upload would leave files on the volume that nothing references. Handles are closed before anything is unlinked, because an open handle makes the unlink fail on Windows. The stored name now takes its extension from the validated type rather than from path.extname(file.originalname), so the name on disk cannot disagree with what the file is. The random UUID is unchanged — that was already right, and its comment explains why. The picker offers exactly those three types rather than image/*, so a choice the API will refuse is not on the menu in the first place. That is a convenience, not a control: the operating system's All files option remains, drag-and-drop ignores accept, and anything calling the API directly never sees it. The server is the control. Nine integration tests, and they are the first in this project to upload real file content — which is why none of this was noticed. They cover a genuine PNG accepted, a PDF refused, SVG refused, HTML wearing image/jpeg refused, nothing left on the volume after a refusal, a mixed request discarding its valid file too, and no item created when the upload fails. Plus 21 unit tests on the pure signature checks, including a RIFF container that is not WebP. Verified: 162 unit, 178 integration, 94 end-to-end on a fresh container. Backend lint holds at 4 warnings — it caught the now-unused path import, which is exactly what it is for. Refs #95 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
ecc2219fa5 |
feat: stage new items as pending until an admin publishes them (#90)
An item used to be live on the storefront the instant it was created. Now it starts pending, and a customer sees it only once it is published. The migration changes the column default and nothing else. Backfilling would un-publish the entire live catalogue, which is the one thing it must not do. Hiding a pending item took four separate changes, not one, and that is the part worth knowing. The storefront's item routes had no status filter at all — sold items are listed and rendered with a Sold badge deliberately — so pending could not be expressed as one more optional filter. GET /api/items now carries an exclusion the caller cannot opt out of; GET /api/items/:id carries the same, because hiding an item from the list while still serving it by id would leave it reachable to anyone who kept a link; and GET /api/filters excludes pending from both aggregates it computes. That last one is the least obvious: a pending item would have inflated its tags' counts, so a customer would read "Rare (1)", filter by it, and be told nothing matches — and its price would have stretched the slider to a range no visible item occupies. The tag count is computed over the joined items rather than filtered with a WHERE. A WHERE would have dropped the row for a tag whose only item is pending, and the tag would have vanished from the drawer instead of showing zero. There is a test for exactly that, because the first version of this query had that bug. parseItemFilters is shared by the storefront and admin routes, so 'pending' parses on both. The public route refuses it explicitly rather than answering with an empty list, which would read as "no items match" instead of "you may not ask that". The storefront's URL reader is deliberately left not accepting it either, with a comment saying so, since a request guaranteed to fail is not worth constructing. Publishing is the existing mark-available: same transition, same UPDATE, so the admin UI labels that button "Publish" when the item is pending rather than adding a second endpoint that does the same thing. Unpublish is new and is not symmetrical — it is refused for a reserved item, which someone is holding in their cart right now, and for a sold one, which is a record of something that happened rather than a draft. Both refusals name their reason, and the buttons are hidden in those states so the refusal is not how you find out. Changing a column default has reach, and it surfaced eight test fixtures that silently depended on it. Each is now explicit about the status it wants rather than inheriting one — better practice regardless, and immune to the next default change. Two tests also used 'pending' as their example of an *unknown* status; both would have quietly become tautologies, so they now use one that is genuinely unknown. Verified: 98 unit, 160 integration and 94 end-to-end passing, the last on a freshly created container. One earlier run showed a single failure in favorites.spec.ts; it passes in isolation and on a clean container, and is the cross-spec interference already recorded against the suite rather than anything from this change. Refs #90 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
1d7aba2d60 |
fix(backend): route every async handler through the error middleware (#59)
Express 4 does not forward a rejected promise from an async handler, so an unwrapped async route never responds at all — the request hangs until the client gives up, nothing reaches the error middleware, and monitoring sees an open connection rather than a 500. That silence is the shape of the 2026-08-17 incident, where an unhandled rejection left every item query hanging and the storefront rendered it as an empty shop. `asyncRoute` was written in response, but it was only applied to some routes: 30 handlers added afterwards were still bare, including register, login, the whole cart, and PayPal checkout. Wraps all 30, plus two the issue's inventory missed. `attachCustomer` is a bare async middleware mounted globally in app.ts, so a rejection in its session lookup would hang every request in the application — including the 25 handlers that were already wrapped correctly, which meant the guarantee did not actually hold anywhere. The PayPal webhook registers on a second router named `webhookRouter`, so an audit grepping for `router.` walked straight past it. Adds a unit test that scans the route sources and fails on any registration whose handler is not wrapped. A convention already half-forgotten once will be forgotten again, and enforcement is what the issue asked for; ESLint would be the better home for it but there is no ESLint in this repo yet (#60). The test walks parens rather than lines, so it also catches a handler whose `async` sits on its own line, and it matches any `*Router` name rather than just `router` — the two ways the existing bare handlers escaped notice. It is deleted along with `asyncRoute` if the project moves to Express 5, which forwards rejections natively. No behaviour changes on the success path; the failure path turns a hung request into a logged 500. Closes #59 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalamb
|
d4e2abe743 |
feat: filter the storefront by favorited items (#35)
Adds favorites as another dimension of the existing storefront filter rather than a separate view, so it lives in the URL, shows up as a removable chip, and combines with category, tags, and price by AND like everything else. A customer can ask for "my favorites under $500 in Furniture" instead of only "my favorites". Sold favorites are included. The storefront shows sold items everywhere else, and a favorite that has just sold is often exactly what the customer came back to look at after being emailed about it in #34. Hiding them would make items disappear from a list the customer curated themselves. Anyone wanting only what they can still buy can combine the toggle with the status filter. Which customer "my favorites" means comes from the session, never from the query string, so a hand-edited URL cannot name someone else's favorites. A signed-out visitor sees the toggle and gets the same inline register/login prompt the heart button and Add to Cart already use; signing in resolves the gate and the filter applies on its own. A bookmarked favorites link whose session has expired says so rather than rendering an empty grid, which would tell the visitor they have no favorites instead of that we do not know who they are. The API answers 401 for the same reason, and the admin inventory refuses the filter outright rather than ignoring it. The shared SQL builder now requires callers to say whose favorites they mean, even when that is nobody, and throws instead of dropping the clause — a future caller that forgets the guard fails loudly rather than quietly returning the whole catalogue. Verified with 59 unit tests, 134 backend integration tests, and 70 end-to-end tests, all passing, with type checking clean on both sides. |
||
|
bermudalambandClaude Opus 5
|
c4fb47853f |
feat: tell favoriters when an item is withdrawn (#34)
Deleting an item cascades its favorites away, so anyone watching it lost the record silently and heard nothing. Recipients are now collected before the delete, since after it there is nobody left to look up, and the mail is sent only once the delete has succeeded so nobody hears about a withdrawal that did not happen. Items that had already sold are excluded. Their favoriters were told at the point of sale, and a second "no longer available" for the same item reads as a duplicate rather than news. The wording differs from the sale notification — withdrawn rather than sold — because the customer did not lose out to another buyer and saying so would be untrue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
f626f27e75 |
feat: favorite items and notify when a favorite is sold (#34)
Customers can favorite and unfavorite items from the storefront, opt in to being told when a favorite is sold to someone else, and manage that preference from their account page. The opt-in is a consent of its own rather than the existing marketing flag. Being told that a specific item you asked about has gone is a narrower thing than agreeing to marketing, and folding one into the other would leave marketing_consent_text no longer describing what was actually agreed to. It is recorded the same way as the marketing consent — flag, timestamp, and the exact wording shown — and accepting it does not set marketing_consent. The prompt appears only after a customer has actually favorited something, so the reason for asking is concrete rather than an abstract marketing ask, and it says plainly that it is separate from marketing email. Declining keeps the favorite. Notifications fire when an item reaches sold, either through checkout or an admin marking it sold, and never to the buyer — telling someone the item they just bought is unavailable reads as a bug. Reserved is deliberately not a trigger: reservations expire and get released, so a "gone" email would often be about an item still for sale. Disabled accounts are excluded, per #33. completeCheckout now returns the sold item ids and the buyer so its three call sites can notify after COMMIT. Sending inside the transaction would email people about a sale that then rolled back, and would hold the transaction open for SMTP. Each message is sent independently so one bad address cannot stop the rest, and the sale has already succeeded regardless. Favoriting while signed out opens the existing inline register/login modal, exactly as Add to Cart does, and completes the favorite on success. Also fixes a latent bug in the same component: while the session was still resolving, `customer` is null for a signed-in visitor too, so clicking Add to Cart or the new heart in that window prompted them to sign in again. Both now ignore clicks until the session has resolved, and the control shows as loading meanwhile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
f537314259 |
fix(admin): theme, American English, and inventory/reservation tooling (#27)
Seven reported items, of which the first four had two root causes. The active tab was invisible in dark mode because colorPrimary was hardcoded to #1a1a1a in both themes. The accent now inverts with the theme, and colorTextLightSolid inverts with it, or a near-white accent would get antd's default white label and disappear. The Category tab, Tag tab, and item-form category selector ignored the theme entirely. antd declares main: lib/index.js and module: es/index.js, so importing from 'antd' resolves to the ES build while 'antd/lib/...' loads the CommonJS one — two copies, two React contexts, and no ConfigProvider for anything deep-imported. Switching those files to antd/es/* keeps the deep-import convention and shares the instance. This was introduced by my own use of the lib path; es is correct under Vite. Two storefront components had the same latent bug. "Colour" is now "Color". The Customers tab shows how many items each customer is holding, as a link opening the item list with a Release button. Release mirrors the customer's own cart removal — drop the cart row, return the item to available, guarded on 'reserved' so it can never resurrect a sold item — and deliberately sends no email about an action the customer did not take. The count is a subquery rather than another join, which would have multiplied rows and inflated order_count and total_spent_cents. The Inventory tab filters by category, tags, price, and status, reusing the storefront's parser and query builder so the two cannot drift. Reserved is one option in a Status filter rather than a standalone toggle. Also fixes two defects the screenshots exposed: the reserved-count link bubbled to the row handler and opened the customer drawer behind the dialog, and .admin-category-node had no CSS at all, so the tree node name, item count, and actions ran together as one string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
9222e97deb |
feat(api): categories, tags, and storefront item filters (#23)
Adds a self-referencing categories tree, a tag registry with deterministic colours, and item_tags, plus admin CRUD for both. GET /api/items now accepts category, tags, min_price and max_price. Category matching walks the subtree with a recursive CTE so selecting a parent includes everything filed beneath it; tags match with AND via a count check, since ANY() alone would return items carrying only one of them. Malformed filter params return 400 rather than being ignored, so a broken link doesn't quietly list the whole catalogue. GET /api/filters serves the drawer its tree, tags, and price bounds in one request. Item image/tag aggregation moves from LEFT JOIN + GROUP BY to scalar subqueries. Joining two one-to-many relations multiplies their rows, so an item with 2 images and 3 tags would have repeated every image three times once tags were added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
ab1024a1b6 |
fix: resolve SonarQube hotspots for upload size and X-Powered-By
- S5693: MAX_IMAGE_BYTES was 8 * 1024 * 1024 (8,388,608), just over the 8,000,000-byte ceiling the rule treats as safe, so the hotspot on the multer storage config never cleared. Use 8_000_000. - S5689: Express advertises its stack in X-Powered-By by default, which tells an attacker what to aim exploits at. Disable the header. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
aba47c81e3 |
fix: bound admin image uploads and use a CSPRNG for stored filenames
SonarQube flagged three hotspots in the admin router: multer was configured with no content length limits, and stored filenames were derived from Date.now() plus Math.random(). - Cap the multipart body on every dimension: 6 files, 8 MiB per image, 8 fields, 64 KiB per field. Without limits a single request could fill the uploads volume. - Generate stored filenames with crypto.randomUUID() so paths are not predictable. Image ordering is unaffected; sort_order already drives it. - Wrap the upload middleware to translate MulterError into 413/400 JSON. The app mounts no error handler, so a limit rejection would otherwise surface as an HTML 500. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalamb
|
921022c658 |
Add backend unit/integration tests, Playwright e2e tests, README, cookie Secure fix
SonarQube Analysis / sonarqube (push) Successful in 4m20s
|
||
|
bermudalamb
|
9be4986dd3 | Initial commit: redefined-designs storefront |