From 421b3d59226e517e29f6f41582329b8e24d68b1b Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 24 May 2026 16:38:23 -0400 Subject: [PATCH] Clean up --- CONSOLIDATION_PLAN.md | 108 ++++++++--- inventory-server/deploy/Caddyfile.proposed | 200 --------------------- 2 files changed, 81 insertions(+), 227 deletions(-) delete mode 100644 inventory-server/deploy/Caddyfile.proposed diff --git a/CONSOLIDATION_PLAN.md b/CONSOLIDATION_PLAN.md index 1700b53..7bc955a 100644 --- a/CONSOLIDATION_PLAN.md +++ b/CONSOLIDATION_PLAN.md @@ -17,9 +17,9 @@ Audit-driven plan to (a) reduce 13 PM2 processes to 5 application servers + 2 au | 5 — Convert `acot-server` to ESM | **Complete (live) — 2026-05-24** | All 11 files (server, db/connection, utils/{phoneAuth,timeUtils}, 7 routes) converted to ESM. PM2 reload clean; SPA-driven `/api/acot/events/*` continues 200 across cutover; phone-server `/api/acot/customers/by-phone` returns 200 with correct shared secret. Phase 6 patterns applied during conversion — see Deviation #24 | | 6 — Auth hardening | **Complete** | All in-process items live: rate-limit, JWT precondition, CORS lockdown, request-log, upload allowlist, `requirePermission` on sensitive routes, permissions seed migration. `authenticate()` live on `/api/*` (inventory-server, dashboard-server) and `/api/acot/*` (acot-server, added in Phase 5). 6.10 lt-wordlist token loaded via `--env-file` + rotated 2026-05-24 (Deviation #25). 6.11 (audit logging) deferred — see Out of scope | | **F1 — Frontend fetch wrapper** | **Complete (live) — 2026-05-23** | Wrappers at `inventory/src/utils/api.ts` (`apiFetch`) and `inventory/src/utils/apiClient.ts` (axios instance). 170 `fetch()` sites across 76 files migrated to `apiFetch`; 32 `axios.*` sites across 11 files migrated to `apiClient`. AuthContext `/login`+`/me`, App.tsx `/me`, and `services/apiv2.ts` (external PHP backend) intentionally left as raw `fetch`. Shipped alongside the Phase 3+6 pm2 reload | -| 7 — Caddyfile final form | **Complete — applied 2026-05-24** | Final Caddyfile live at `/etc/caddy/Caddyfile` (forward_auth gate + per-vendor reverse_proxy to :3015). The `inventory-server/deploy/` staging folder was removed after apply — recreate from this doc if future changes are needed. Backup convention: `/etc/caddy/Caddyfile.bak.YYYY-MM-DD` | +| 7 — Caddyfile final form | **Complete — applied 2026-05-24, amended same day** | Final Caddyfile live at `/etc/caddy/Caddyfile` (forward_auth gate + per-vendor reverse_proxy to :3015). The `inventory-server/deploy/` staging folder was removed after the initial apply, then re-created during Phase 9 §9.2 and again for the Deviation #28 uploads-gate revert — treat it as the working area for any future Caddyfile change. Backup convention: `/etc/caddy/Caddyfile.bak.YYYY-MM-DD` | | 8 — ecosystem.config.cjs final form | **Complete — applied 2026-05-24** | Live PM2 list matches the spec below (5 apps + acot-phone-server + lt-wordlist-api = 7 processes). Includes Phase 6.4 JWT_SECRET shadow-override fix and 6.10 lt-wordlist token move. `inventory-server/deploy/` removed post-apply | -| **9 — Post-audit residual gaps** | **Complete (live) — 2026-05-24** | Closes findings from second-look audit: chat-server ESM + in-process `authenticate()` + localhost bind, Caddyfile uploads-gate + edge CORS tightening, vitest scaffold + auth-boundary tests, three Mini\*.jsx fetch leaks, and plan-doc goal reconciliation. PM2 reload, Caddy apply, frontend rebuild, and tests all completed. See Phase 9 section | +| **9 — Post-audit residual gaps** | **Complete (live) — 2026-05-24** | Closes findings from second-look audit: chat-server ESM + in-process `authenticate()` + localhost bind, Caddyfile edge CORS tightening + CORS preflight bypass, vitest scaffold + auth-boundary tests, three Mini\*.jsx fetch leaks, and plan-doc goal reconciliation. PM2 reload, Caddy apply, frontend rebuild, and tests all completed. **§9.2's uploads-gate fix was subsequently reverted (2026-05-24) — see Deviation #28; `/uploads/*` is public-by-design because the external PHP backend at www.acherryontop.com fetches images during product import.** See Phase 9 section | **Live PM2 process count: 7** (5 application apps — auth-server, inventory-server, chat-server, dashboard-server, acot-server — plus acot-phone-server + lt-wordlist-api). Down from 13 pre-refactor. @@ -383,13 +383,13 @@ Per-item status: | # | Item | Status | Where | |---|---|---|---| -| 6.1 | Caddy `forward_auth` gate | **Live — 2026-05-24** | Applied via Caddy admin API + `sudo cp` to `/etc/caddy/Caddyfile`. `@gated path /api/* /chat-api/* /uploads/*` block hits `localhost:3011/verify` on every request | +| 6.1 | Caddy `forward_auth` gate | **Live — 2026-05-24** | Applied via Caddy admin API + `sudo cp` to `/etc/caddy/Caddyfile`. `@gated path /api/* /chat-api/*` block hits `localhost:3011/verify` on every request. (`/uploads/*` was originally in the gate but removed per Deviation #28 — see Phase 6.7 / 9.2.) | | 6.2 | `requirePermission` on sensitive routes + permissions migration | **Done** | inline in `config.js`, `data-management.js`, `import.js`, `ai-prompts.js`, `ai-validation.js`, `templates.js`, `reusable-images.js`; codes seeded by `migrations/005_phase6_permission_codes.sql`. **Phase 4 follow-on (2026-05-23):** `meta_write` wired on `PATCH /api/meta/campaigns/:id/budget` and `POST /api/meta/campaigns/:id/:action`; `klaviyo_admin` wired on `POST /api/klaviyo/events/clearCache`. Read-only Google + Typeform endpoints stay authenticated-only (reserved write codes left in migration 005 for future) | | 6.3 | Login rate-limit + `/verify` rate-limit | **Done** | `auth/server.js` uses `shared/rate-limit/login.js` (`loginLimiter`, `verifyLimiter`) | | 6.4 | JWT_SECRET as startup precondition + ecosystem footgun fix | **Live — 2026-05-24** | Both auth-server and inventory-server `process.exit(1)` if `JWT_SECRET` is unset. The `JWT_SECRET: process.env.JWT_SECRET` override that was shadowing `.env` is removed from the live ecosystem.cjs | | 6.5 | Structured request logging w/ redaction | **Done** | `shared/logging/request-log.js` (pino-http, redacts Authorization/Cookie); mounted in both `auth/server.js` and `src/server.js` | | 6.6 | CORS lockdown | **Done** | `src/middleware/cors.js` now re-exports `shared/cors/policy.js`. LAN wildcards (`192.168.*`, `10.*`) and `*` defaults gone | -| 6.7 | Upload hardening | **Done** | Exact-match MIME+extension allowlist on `routes/import.js` and `routes/reusable-images.js`; dead `multer({ dest })` removed from `routes/products.js` (no upload route was using it — strongest hardening was deletion) | +| 6.7 | Upload hardening | **Partial — gating reverted** | Exact-match MIME+extension allowlist on `routes/import.js` and `routes/reusable-images.js`; dead `multer({ dest })` removed from `routes/products.js` (no upload route was using it — strongest hardening was deletion). The Caddyfile `/uploads/*` gate **was applied then reverted (2026-05-24)** — uploads are public-by-design (external PHP backend fetches them during product import). See Deviation #28 | | 6.8 | Frontend token storage stays localStorage + XSS audit | **Audited** | Confirmed `dangerouslySetInnerHTML` is sanitized in `ProductEditor.tsx`. **Flagged: `ChatRoom.tsx:277,392` renders user-controlled chat content as raw HTML — real XSS vector, separate fix needed** | | 6.9 | Remove debug middleware | **Done** | The header-dumping `app.use((req,res,next)=>{ console.log(... req.headers ...) })` block removed from `src/server.js`. Replaced with `shared/logging/request-log.js` (which redacts). | | 6.10 | `lt-wordlist-api` token move | **Live — 2026-05-24** | Live PM2 entry runs `/opt/lt-wordlist-api/index.js` under matt's daemon; `ADD_WORD_TOKEN` is no longer inline in ecosystem.cjs and is read from `/opt/lt-wordlist-api/.env`. See Deviations #21–23 for the path corrections and the (incorrect) earlier assumption that this app lived under a separate root daemon | @@ -504,7 +504,7 @@ If anyone genuinely needs LAN access, add their specific IP, not a `/16` range. - File-size limit set on multer config (current limit may be defaulted — verify). - MIME-type allowlist (image/jpeg, image/png, image/webp; reject everything else). - Filename sanitization (no `..`, no absolute paths, generate UUID-based names server-side). -- The Caddy `/uploads/*` handler currently serves any file in the uploads directory publicly. Move this **behind** the auth gate: include `/uploads/*` in `@needs_auth`. If some images are referenced from public emails (Klaviyo newsletter), put **those** in a separate public bucket; everything else stays gated. +- ~~The Caddy `/uploads/*` handler currently serves any file in the uploads directory publicly. Move this **behind** the auth gate: include `/uploads/*` in `@needs_auth`.~~ **Reverted — see Deviation #28.** Uploads are public-by-design because the external PHP backend at `www.acherryontop.com` fetches image URLs server-to-server during the `/apiv2/product/setup_new` import flow, and has no Bearer token. Random-suffix filenames (timestamp + `Math.random`) provide weak enumeration resistance; `/uploads/products/*` files also auto-delete after 24h. If genuinely sensitive uploads ever land here, implement HMAC-signed URLs rather than re-gating wholesale. ### 6.8 Frontend token storage @@ -615,15 +615,15 @@ Per Phase 6.8, we're not migrating to httpOnly cookie auth. F1 is the minimum wo ### Note on `/uploads/*` gating (Phase 6.7's Caddyfile change) -**Applied as-spec (2026-05-24):** `/uploads/*` is behind `forward_auth` in the live Caddyfile. `` references in the SPA are browser-issued GETs that don't carry `Authorization` headers — verify image display works end-to-end (cookies fall-through, signed URLs, or session-bound forward_auth) and if broken, revert this part of 6.7 to keep `/uploads/*` public, OR issue per-image signed URLs from the API. +**Applied as-spec then reverted (2026-05-24):** `/uploads/*` was initially placed behind `forward_auth` per the 6.7 spec. The SPA was patched via [`AuthedImage`](inventory/src/components/ui/authed-image.tsx) to send Bearer headers, but the gate broke the **external** product-import flow: image URLs from `/uploads/products/*` and `/uploads/reusable/*` are submitted to the external PHP backend (`www.acherryontop.com`) via `/apiv2/product/setup_new`, which then fetches them server-to-server to ingest into the shop — and has no Bearer token. The escape hatch from the original note ("revert this part of 6.7 to keep `/uploads/*` public") was taken: `/uploads/*` now serves via a dedicated public `file_server` placed ahead of `@static` and `@gated`. See Deviation #28 for the full revert details. `AuthedImage` is left in place as defense-in-depth (Bearer header is harmless when not required); no rush to rip out. --- ## Phase 7 — Caddyfile final form -Status: **Complete — applied 2026-05-24.** Final Caddyfile live at `/etc/caddy/Caddyfile`; vendor handles point at the merged dashboard-server on :3015. The `inventory-server/deploy/` staging folder (which held `Caddyfile.proposed` and the README of apply commands) was removed after apply — recreate from the spec below if future changes are needed. Apply pattern (admin-API `curl -X POST :2020/load` + `sudo cp` to persist on-disk) is captured in Deviation #8. Backup convention: `/etc/caddy/Caddyfile.bak.YYYY-MM-DD`. +Status: **Complete — applied 2026-05-24.** Final Caddyfile live at `/etc/caddy/Caddyfile`; vendor handles point at the merged dashboard-server on :3015. The `inventory-server/deploy/` staging folder (which held `Caddyfile.proposed` and the README of apply commands) was originally removed after apply — **re-created 2026-05-24 to stage the §9.2 edits and again to stage the Deviation #28 uploads-gate revert; treat it as the working area for any future Caddyfile change**. Apply pattern (admin-API `curl -X POST :2020/load` + `sudo cp` to persist on-disk) is captured in Deviation #8. Backup convention: `/etc/caddy/Caddyfile.bak.YYYY-MM-DD`. -After all phases, the `tools.acherryontop.com` block looks like: +After all phases (including Phase 9 §9.2 and the Deviation #28 revert), the `tools.acherryontop.com` block looks like: ```caddyfile tools.acherryontop.com { @@ -635,6 +635,34 @@ tools.acherryontop.com { reverse_proxy localhost:3011 } + # Phase 9 §9.2: CORS preflight bypass (OPTIONS has no Authorization header) + @cors_preflight { + method OPTIONS + header Access-Control-Request-Method * + } + handle @cors_preflight { + handle /api/klaviyo/* { reverse_proxy localhost:3015 } + handle /api/meta/* { reverse_proxy localhost:3015 } + handle /api/dashboard-analytics/* { reverse_proxy localhost:3015 } + handle /api/typeform/* { reverse_proxy localhost:3015 } + handle /api/acot/* { reverse_proxy localhost:3012 } + handle /chat-api/* { + uri strip_prefix /chat-api + reverse_proxy localhost:3014 + } + handle /api/* { reverse_proxy localhost:3010 } + } + + # Deviation #28: /uploads/* is PUBLIC by design (external PHP backend + # fetches image URLs server-to-server during /apiv2 product import). + # Rooted at /var/www/inventory (NOT the SPA build root) so paths + # resolve to the real uploads directory. Placed ahead of @static and + # @gated so it wins the match. + handle /uploads/* { + root * /var/www/inventory + file_server + } + # Public: static frontend assets @static path *.js *.css *.png *.jpg *.jpeg *.gif *.ico *.svg *.woff *.woff2 handle @static { @@ -643,20 +671,14 @@ tools.acherryontop.com { file_server } - # All API + uploads: auth gate first - @gated path /api/* /chat-api/* /uploads/* + # All API + chat: auth gate first + @gated path /api/* /chat-api/* handle @gated { forward_auth localhost:3011 { uri /verify copy_headers Authorization } - # Uploaded files - handle /uploads/* { - root * /var/www/inventory - file_server - } - # Vendor dashboard routes → merged dashboard-server handle /api/klaviyo/* { reverse_proxy localhost:3015 } handle /api/meta/* { reverse_proxy localhost:3015 } @@ -812,22 +834,24 @@ The intent is **"do it properly the first time"** — Phase 9 isn't a quality-of ### 9.2 — Caddyfile: uploads gate fix + edge CORS tightening -**Findings:** +> **⚠️ The uploads-gate portion of this section was reverted on 2026-05-24 — see Deviation #28.** Edit #1 (`not path /uploads/*` on `@static`) was undone and `/uploads/*` was lifted out of `@gated` entirely. The CORS edits (#2 and #3) remain live. The narrative below is preserved as a record of the original intent and what was learned. + +**Findings (original):** - `/uploads/*.jpg` returns a *public* `404` because the `@static path *.jpg ... *.woff2` matcher beats `@gated path /api/* /chat-api/* /uploads/*` on specificity, hitting the unauthenticated SPA build root. - `Access-Control-Allow-Origin "*"` is set inside the `security_headers` snippet and imported into `tools.acherryontop.com`, which silently overrides the careful allow-list in `shared/cors/policy.js`. - CORS preflight (`OPTIONS` with no Authorization) hits `forward_auth` and 401s before the backend's CORS handler ever sees it. -**Applied from:** `inventory-server/deploy/Caddyfile.proposed` (recreated this session per Deviation #18's convention of staging diffs there before applying). The active `/etc/caddy/Caddyfile` now contains these Phase 9 edits. +**Applied from:** `inventory-server/deploy/Caddyfile.proposed` (recreated this session per Deviation #18's convention of staging diffs there before applying). The active `/etc/caddy/Caddyfile` now contains the Phase 9 CORS edits + the Deviation #28 uploads-gate revert. -**Three edits:** -1. Tighten `@static`: +**Three edits (as originally applied):** +1. ~~Tighten `@static`:~~ **REVERTED — see Deviation #28.** ``` @static { path *.js *.css *.png *.jpg *.jpeg *.gif *.ico *.svg *.woff *.woff2 not path /uploads/* } ``` - This makes `/uploads/*` an exclusive @gated path, restoring the intended authenticated file_server behavior. + ~~This makes `/uploads/*` an exclusive @gated path, restoring the intended authenticated file_server behavior.~~ Current state: the `not path /uploads/*` exclusion is removed; `/uploads/*` is handled by a dedicated public `handle /uploads/*` block placed ahead of both `@static` and `@gated`. See Deviation #28. 2. Drop the wildcard headers from `security_headers`: ```diff - Access-Control-Allow-Origin "*" @@ -855,15 +879,21 @@ The intent is **"do it properly the first time"** — Phase 9 isn't a quality-of } ``` -**Applied / verified:** +**Applied / verified (at time of original §9.2 cutover):** 1. Caddy admin API load applied and `/etc/caddy/Caddyfile` persisted with the Phase 9 edits. -2. Smoke: +2. Smoke (post-§9.2, pre-Deviation #28 revert): - `curl -I https://tools.acherryontop.com/uploads/reusable/.jpg` → `401` without auth (was: public 404). - same path with a valid short-lived Bearer token → `200 image/jpeg`. - `curl -X OPTIONS -H "Origin: https://tools.acherryontop.com" -H "Access-Control-Request-Method: GET" https://tools.acherryontop.com/api/products` → `204` with `Access-Control-Allow-Origin: https://tools.acherryontop.com` (specific, **not** `*`). - same preflight with `Origin: https://evil.example` → `403` with no wildcard ACAO. - `curl -s https://tools.acherryontop.com/api/products` (no token) → `401`; wildcard ACAO no longer stamped by Caddy. +**Current smoke (post-Deviation #28 revert, 2026-05-24):** + - `curl -I https://tools.acherryontop.com/uploads/reusable/.jpg` → `200 image/jpeg` without auth (intentional — see Deviation #28). + - `curl -I https://tools.acherryontop.com/uploads/products/.jpg` → `404` (not `401`) — confirms the gate is no longer in front of `/uploads/*`. + - `curl -s https://tools.acherryontop.com/api/products` (no token) → `401` (gate still up for `/api/*` + `/chat-api/*`). + - CORS preflight and origin-allowlist behavior unchanged from the post-§9.2 smoke above. + **Rollback:** restore `/etc/caddy/Caddyfile.bak.YYYY-MM-DD` and `curl -X POST .../load` against it. ### 9.3 — Frontend Mini\*.jsx fetch leaks @@ -920,7 +950,7 @@ The scaffold is intentionally narrow: it covers the security-critical surface an Phase 9 completion checks: - ✅ `curl -s -o /dev/null -w '%{http_code}' http://localhost:3014/test-db` returns `401` (was: 200). -- ✅ `curl -sI https://tools.acherryontop.com/uploads/reusable/.jpg` returns `401` without auth and `200` with valid Bearer. +- ⚠️ ~~`curl -sI https://tools.acherryontop.com/uploads/reusable/.jpg` returns `401` without auth and `200` with valid Bearer.~~ **Superseded by Deviation #28.** Current expected behavior: `200 image/jpeg` without auth (uploads are public by design). The original Phase 9.7 criterion is preserved here as a record of what §9.2 verified at the time. - ✅ `curl -X OPTIONS ...` to a gated `/api/*` path returns `204` from the backend with a specific `Access-Control-Allow-Origin`, **not** `*`. - ✅ `cd /var/www/inventory && npm test` passes 100% on `shared/auth/*.test.js` (2 files, 21 tests). - ✅ `grep -rn "require(" inventory-server/chat/*.js` returns nothing. @@ -989,7 +1019,7 @@ These came up in the audit but aren't part of this refactor: State as of 2026-05-24: Phases 1–9 are **shipped**. The "4 application PM2 processes" original target became **5** in execution because `chat-server` stayed standalone rather than being folded in — never a serious merge candidate (different DB, different protocol shape). - ✅ 5 application PM2 processes instead of 12 (auth-server, inventory-server, dashboard-server, acot-server, chat-server) — plus 2 unchanged (acot-phone-server, lt-wordlist-api) = 7 total. -- ✅ All `/api/*`, `/chat-api/*`, and `/uploads/*` requests gated at Caddy (`forward_auth`). Phase 9 §9.2 fixed the `@static` matcher ordering bug that let `/uploads/*.jpg` slip past the gate. +- ✅ All `/api/*` and `/chat-api/*` requests gated at Caddy (`forward_auth`). `/uploads/*` is **intentionally public** — it was gated as part of Phase 6.7 / §9.2, then reverted because the external PHP backend at `www.acherryontop.com` fetches image URLs during product import. See Deviation #28. - ✅ Per-upstream `authenticate()` re-verification on inventory-server, dashboard-server, acot-server, and chat-server. - ✅ Sensitive endpoints additionally gated by per-permission checks (`requirePermission`). - ✅ **One ESM standard across primary application Node services** — auth-server, inventory-server, dashboard-server, acot-server, and chat-server are ESM. @@ -997,10 +1027,10 @@ State as of 2026-05-24: Phases 1–9 are **shipped**. The "4 application PM2 pro - ✅ Login rate-limited (`shared/rate-limit/login.js`). - ✅ `JWT_SECRET` rotated + ecosystem shadow-override removed. - ✅ Old auth-server, Aircall, Gorgias, Clarity directories deleted from the repo. Defunct `dashboard:gorgias`/`dashboard:calls` permission rows also deleted from DB (2026-05-24). -- ✅ Caddyfile slimmed to one auth-gated block. Phase 9 §9.2 tightens the `@static` matcher + drops the edge CORS wildcard. +- ✅ Caddyfile slimmed to one auth-gated block. Phase 9 §9.2 drops the edge CORS wildcard and adds a CORS preflight bypass; the `@static`-matcher tightening from §9.2 was rolled back as part of the Deviation #28 uploads-gate revert (uploads now served via a dedicated public `handle /uploads/*` block placed ahead of `@static`). - ✅ Permission codes inserted into `permissions` table for granular authorization. - ✅ Both gaps surfaced during Phase 5 verification — `lt-wordlist-api` insecure default token (Deviation #25) and Caddy blocking acot-phone-server's `x-acot-api-key` calls (Deviation #26) — were closed 2026-05-24. -- ✅ **Six second-look findings closed by Phase 9** (frontend fetch leaks, edge CORS wildcard, uploads-gate ordering, chat-server in-process auth + ESM, vitest scaffold, plan-doc/goal mismatch). Code and deploy steps are complete. +- ✅ **Six second-look findings closed by Phase 9** (frontend fetch leaks, edge CORS wildcard, uploads-gate ordering, chat-server in-process auth + ESM, vitest scaffold, plan-doc/goal mismatch). Code and deploy steps are complete. *The uploads-gate fix was later reverted (Deviation #28) — the gate was incompatible with the external-server image-fetch contract; what `@static` slip-through actually revealed was that uploads belong in front of the gate, not behind it.* --- @@ -1085,3 +1115,27 @@ These are decisions made during Phase 1/2 implementation that amend the spec abo - chat-server had only one defense layer (Caddy `forward_auth`) — `curl http://localhost:3014/test-db` returned 200 unauthenticated. Phase 9 §9.1 closes both: chat-server is now ESM, mounts `authenticate()` against an in-process `inventory_db` pool (separate from the existing `rocketchat_converted` pool which routes.js uses via `global.pool`), and binds to `127.0.0.1` so external direct-port access is no longer possible even if the host firewall rule were dropped. Conversion was 3 files (`server.js`, `routes.js`, `package.json`) and minimal because routes.js's only top-level imports were `express` and `path` — handler bodies were untouched. + +28. **Phase 6.7 / §9.2 `/uploads/*` gating reverted (2026-05-24).** Phase 6.7 specified moving `/uploads/*` behind `forward_auth`, and Phase 9 §9.2 hardened that by excluding `/uploads/*` from the `@static` matcher so uploaded `*.jpg` paths couldn't slip past the gate via the SPA build root. The SPA was patched (`AuthedImage` in `inventory/src/components/ui/authed-image.tsx`) to send Bearer headers on `/uploads/*` GETs. + + **The breakage:** Both `/uploads/products/*` and `/uploads/reusable/*` URLs are submitted as strings inside the `product_images` field to `/apiv2/product/setup_new`, which Caddy reverse-proxies to the external PHP backend at `www.acherryontop.com`. That backend then **fetches the image URLs server-to-server** to ingest them into the shop. The external backend has no JWT and never authenticates against our `localhost:3011/verify` endpoint — so after the gate went live, every external import-time image fetch hit 401 and the product-import flow silently dropped images. + + The original Phase 6.7 / F1 notes reserved this exact escape hatch (CONSOLIDATION_PLAN.md "Note on `/uploads/*` gating": *"if broken, revert this part of 6.7 to keep `/uploads/*` public, OR issue per-image signed URLs from the API"*). + + **Fix applied — option (a):** revert the gate. In the live Caddyfile: + - Removed `/uploads/*` from the `@gated` matcher. + - Removed the `not path /uploads/*` exclusion from `@static` (no longer needed). + - Added a dedicated public `handle /uploads/*` rooted at `/var/www/inventory` (NOT the SPA build root — paths must resolve to the real uploads directory). Placed ahead of both `@static` and `@gated` so it wins the match. + + Staged via `inventory-server/deploy/Caddyfile.proposed` and applied via the Deviation #8 pattern (admin-API `curl -X POST :2020/load` + `sudo cp` to persist). + + **Verified live:** + - `curl -sI https://tools.acherryontop.com/uploads/reusable/.jpg` → `200 image/jpeg` without auth. + - `curl -sI https://tools.acherryontop.com/uploads/products/.jpg` → `404` (not `401`) — confirms gate is no longer in front of `/uploads/*`. + - `curl -sI https://tools.acherryontop.com/api/products` (no token) → `401` — `@gated` is still live for `/api/*` + `/chat-api/*`. + + **Rationale for staying public rather than signing URLs:** Per-image HMAC signed URLs would have worked too but require a small change on the inventory server (URL generation + signature validation in Caddy or upstream) and would flow through the PHP backend untouched. Plain public access is acceptable because: (a) uploaded image URLs use random unguessable filenames (`createUploadFilename` in `inventory-server/src/routes/import.js` — timestamp + 9-digit `Math.random` suffix), (b) `/uploads/products/*` files auto-delete after 24h via `scheduleImageDeletion`, and (c) no PII or confidential data lives in this bucket today. If that ever changes, implement signed URLs rather than re-gating wholesale. + + **AuthedImage:** Left in place as defense-in-depth. Bearer header is harmless when the path isn't gated, so existing call sites keep working unchanged. No rush to rip it out. + + **Doc updates this session:** the Status table's Phase 9 row, Phase 6.1 row, Phase 6.7 row, the inline 6.7 bullet, the F1 "Note on `/uploads/*` gating" subsection, the Phase 7 Caddyfile diagram, the Phase 9 §9.2 narrative + smoke results, and the Concrete deliverables checklist were all updated to reflect the revert. diff --git a/inventory-server/deploy/Caddyfile.proposed b/inventory-server/deploy/Caddyfile.proposed deleted file mode 100644 index 63c3807..0000000 --- a/inventory-server/deploy/Caddyfile.proposed +++ /dev/null @@ -1,200 +0,0 @@ -# Caddyfile — Phase 9 §9.2 proposed form. -# -# Three changes vs. /etc/caddy/Caddyfile (2026-05-24): -# 1. @static matcher now explicitly excludes /uploads/* — without this, an -# uploaded *.jpg matched @static before @gated and slipped past the -# forward_auth gate, hitting the SPA build root and returning a public 404. -# 2. The security_headers snippet no longer sets Access-Control-Allow-* — -# the upstreams' shared/cors/policy.js is the single source of truth for -# CORS responses (Phase 6.6). -# 3. New @cors_preflight handler punts OPTIONS preflights past forward_auth -# so the upstream's CORS middleware can answer them (preflights have no -# Authorization header, so they 401'd at the gate previously). -# -# Apply via the staged-cutover convention in Deviation #8: -# scp this file to netcup:/home/matt/Caddyfile.new -# curl --silent -X POST -H "Content-Type: text/caddyfile" \ -# --data-binary @/home/matt/Caddyfile.new http://localhost:2020/load -# # ...smoke-test, then persist: -# sudo cp /etc/caddy/Caddyfile /etc/caddy/Caddyfile.bak.YYYY-MM-DD -# sudo cp /home/matt/Caddyfile.new /etc/caddy/Caddyfile -{ - admin :2020 -} -(security_headers) { - header { - X-Content-Type-Options "nosniff" - X-Frame-Options "SAMEORIGIN" - X-XSS-Protection "1; mode=block" - Strict-Transport-Security "max-age=31536000; includeSubDomains" - Referrer-Policy "strict-origin-when-cross-origin" - # Phase 9 §9.2: CORS headers removed. Upstreams set ACAO conditionally - # via shared/cors/policy.js; Caddy stamping `*` here was overriding it. - } -} -files.acot.site { - reverse_proxy localhost:8060 -} -pbx.acot.site { - @ws path /ws - handle @ws { - reverse_proxy 127.0.0.1:8088 - } - handle { - reverse_proxy 127.0.0.1:8080 { - header_up Host {host} - header_down Location http://127.0.0.1:8080 https://pbx.acot.site - header_down Location http://pbx.acot.site:8080 https://pbx.acot.site - } - } -} -turn.acot.site { - respond 404 -} -freescout.acot.site { - root * /var/www/freescout/public - encode gzip - php_fastcgi unix//run/php/php8.3-fpm.sock - file_server - # Deny access to dotfiles - @dotfiles path */.* - respond @dotfiles 403 -} -phone.acot.site { - reverse_proxy 127.0.0.1:3020 - encode gzip -} -crafty.acot.site { - reverse_proxy localhost:8443 { - header_up X-Forwarded-Proto https - header_up X-Forwarded-Port 443 - header_up Host {host} - transport http { - tls_insecure_skip_verify - } - } -} -cronicle.acot.site { - reverse_proxy localhost:3100 { - header_up X-Forwarded-Proto https - } -} -inventory.acot.site, acot.site { - redir https://tools.acherryontop.com{uri} permanent -} -tools.acherryontop.com { - import security_headers - - # Public: login endpoint - handle /auth-inv/* { - uri strip_prefix /auth-inv - reverse_proxy localhost:3011 - } - - # Phase 9 §9.2 — CORS preflight bypass. - # Browsers send OPTIONS preflights without Authorization, so forward_auth - # 401s them. Route preflights straight to the upstream which runs - # shared/cors/policy.js and answers correctly. Must come before @static - # and @gated so OPTIONS to *.jpg paths under /uploads/* also work if any - # frontend ever XHRs an upload URL. - @cors_preflight { - method OPTIONS - header Access-Control-Request-Method * - } - handle @cors_preflight { - handle /api/klaviyo/* { - reverse_proxy localhost:3015 - } - handle /api/meta/* { - reverse_proxy localhost:3015 - } - handle /api/dashboard-analytics/* { - reverse_proxy localhost:3015 - } - handle /api/typeform/* { - reverse_proxy localhost:3015 - } - handle /api/acot/* { - reverse_proxy localhost:3012 - } - handle /chat-api/* { - uri strip_prefix /chat-api - reverse_proxy localhost:3014 - } - handle /api/* { - reverse_proxy localhost:3010 - } - } - - # Public: static frontend assets (long-cache). - # Phase 9 §9.2: `not path /uploads/*` ensures uploaded images never get - # served from the SPA build root — they must go through @gated below. - @static { - path *.js *.css *.png *.jpg *.jpeg *.gif *.ico *.svg *.woff *.woff2 - not path /uploads/* - } - handle @static { - header Cache-Control "public, max-age=2592000" - root * /var/www/inventory/frontend/build - file_server - } - - # ----- Authenticated zone ----- - # Phase 6.1: forward_auth subrequest to auth-server:/verify. 2xx → proceeds. - # 401/403 → Caddy returns auth-server response to client; backend never sees it. - @gated path /api/* /chat-api/* /uploads/* - handle @gated { - forward_auth localhost:3011 { - uri /verify - copy_headers Authorization - } - # Phase 6.7: /uploads/* now behind the gate (was a public file_server before). - # Phase 9 §9.2 closes the static-matcher bypass that made this ineffective. - handle /uploads/* { - root * /var/www/inventory - file_server - } - # Phase 4: merged dashboard-server (klaviyo + meta + google + typeform). - handle /api/klaviyo/* { - reverse_proxy localhost:3015 - } - handle /api/meta/* { - reverse_proxy localhost:3015 - } - handle /api/dashboard-analytics/* { - reverse_proxy localhost:3015 - } - handle /api/typeform/* { - reverse_proxy localhost:3015 - } - # ACOT - handle /api/acot/* { - reverse_proxy localhost:3012 - } - # Chat (Phase 9 §9.1 — chat-server now has its own authenticate() too) - handle /chat-api/* { - uri strip_prefix /chat-api - reverse_proxy localhost:3014 - } - # Catch-all: inventory-server - handle /api/* { - reverse_proxy localhost:3010 - } - } - - # Out-of-band probes (unauthenticated) - handle /health { - reverse_proxy localhost:3010 - } - - # SPA fallback (public assets) - handle { - root * /var/www/inventory/frontend/build - try_files {path} /index.html - file_server - encode gzip - } - handle_errors { - respond "{err.status_code} {err.status_text}" - } -}