Fix identified issues with server consolidation
This commit is contained in:
+172
-13
@@ -1,6 +1,8 @@
|
||||
# Server Consolidation & Security Hardening Plan
|
||||
|
||||
Audit-driven plan to (a) reduce 12 PM2 processes to 3 application servers + 1 auth server, (b) put every API endpoint behind real authentication, and (c) standardize on ESM across all Node services. Approach is "do it properly the first time" — no half-finished pieces, no deferred cleanup.
|
||||
Audit-driven plan to (a) reduce 13 PM2 processes to 5 application servers + 2 auxiliary processes (acot-phone-server, lt-wordlist-api) = 7 total, (b) put every API endpoint behind real authentication, and (c) standardize on ESM across all primary application Node services. Approach is "do it properly the first time" — no half-finished pieces, no deferred cleanup.
|
||||
|
||||
> **Note on the original 12→4 target.** The initial spec called for `12 PM2 processes → 3 application servers + 1 auth server` and "ESM across all Node services". During execution `chat-server` proved a poor merge candidate (different DB, different protocol shape) and was kept as its own process — see Deviations #16 and #27. Phase 9 (added post-audit, 2026-05-24) closes the residual gap: chat-server ESM conversion + in-process `authenticate()` + Caddyfile/CORS hardening + vitest scaffold.
|
||||
|
||||
---
|
||||
|
||||
@@ -10,19 +12,22 @@ Audit-driven plan to (a) reduce 12 PM2 processes to 3 application servers + 1 au
|
||||
|---|---|---|
|
||||
| 1 — Decommission dead services | **Complete** | aircall/gorgias/clarity/legacy-auth-server deleted from repo + PM2 + Caddyfile + ecosystem.cjs |
|
||||
| 2 — Build shared `lib/` | **Complete** | Lives at `inventory-server/shared/` (see Deviations). `/verify` endpoint live on auth-server |
|
||||
| 3 — Convert auth-server + inventory-server to ESM | **Complete** | All 58 server-side files ESM; both services live under the ESM build for >24h. See Deviations #10–13 |
|
||||
| 3 — Convert auth-server + inventory-server to ESM | **Complete** | All inventory-server + auth-server server-side files (58) ESM; both services live under the ESM build for >24h. `chat-server` intentionally not in scope — deferred to Phase 9. See Deviations #10–13, #27 |
|
||||
| 4 — Build `dashboard-server` (the merge) | **Complete (live) — 2026-05-24** | Merged service running on :3015 under PM2; Caddy routes for klaviyo/meta/dashboard-analytics/typeform all reverse-proxy to it. Old per-vendor directories (`klaviyo-server`, `meta-server`, `google-server`, `typeform-server`) and their PM2 entries deleted post-cutover — ~1.27 GB reclaimed (largely duplicated `node_modules`). Phase 6.2 gates wired (meta_write, klaviyo_admin). See Deviations #16–19 |
|
||||
| 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` |
|
||||
| 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 |
|
||||
|
||||
**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.
|
||||
|
||||
**All planned phases complete (2026-05-24).** Phase 5 was the last code-level deliverable; acot-server now runs as an ESM service with shared-lib `authenticate()` defense-in-depth.
|
||||
**Phases 1–8 complete (2026-05-24).** Phase 5 closed the last originally-planned code deliverable; acot-server now runs as an ESM service with shared-lib `authenticate()` defense-in-depth.
|
||||
|
||||
**All originally planned phases shipped.** Two real gaps surfaced during Phase 5 verification — both closed 2026-05-24:
|
||||
**Phase 9 added and applied 2026-05-24** after a second-look audit surfaced six residual findings — see [Phase 9](#phase-9--post-audit-residual-gaps) below. Code, PM2 reload, Caddy apply, frontend rebuild, and test verification are complete.
|
||||
|
||||
Two earlier gaps surfaced during Phase 5 verification, both closed 2026-05-24:
|
||||
- **Phase 6.10 lt-wordlist token rotation** — fixed via Node's native `--env-file` flag in the PM2 entry; token rotated to a fresh 32-byte hex value in `/opt/lt-wordlist-api/.env` (mode 0600). See Deviation #25.
|
||||
- **Phase 6.1 Caddy gate blocking acot-phone-server's customer lookups** — fixed by repointing `ACOT_API_URL` from the public host to `http://localhost:3012/api/acot`. See Deviation #26.
|
||||
|
||||
@@ -31,8 +36,8 @@ Audit-driven plan to (a) reduce 12 PM2 processes to 3 application servers + 1 au
|
||||
## Goals
|
||||
|
||||
- Every public-facing endpoint requires a valid auth token (Caddy gate + per-server middleware + per-route permission checks for sensitive operations).
|
||||
- Reduce service count from 12 PM2 processes to 4: `inventory-server`, `acot-server`, `dashboard-server`, `auth-server`.
|
||||
- Standardize on ESM (`"type": "module"`) across all Node services.
|
||||
- Reduce service count from 13 PM2 processes to 7 total: 5 application servers (`auth-server`, `inventory-server`, `dashboard-server`, `acot-server`, `chat-server`) + 2 auxiliary (`acot-phone-server`, `lt-wordlist-api`). The original "4 application servers" target was missed because `chat-server` proved a poor merge candidate during execution; see Deviation #16.
|
||||
- Standardize on ESM (`"type": "module"`) across all primary application Node services. `chat-server` was the last holdout — Phase 9 converts it.
|
||||
- Decommission `aircall-server`, `gorgias-server`, `clarity-server`, and the legacy `auth-server` (port 3003).
|
||||
- Eliminate dependency duplication: one Redis client, one Postgres pool helper, one logger, one auth middleware — shared across services.
|
||||
|
||||
@@ -84,7 +89,7 @@ Audit-driven plan to (a) reduce 12 PM2 processes to 3 application servers + 1 au
|
||||
└──────────────────────┘
|
||||
```
|
||||
|
||||
PM2 process count: **12 → 4** (plus `acot-phone-server` and `lt-wordlist-api`, which stay as-is — out of scope).
|
||||
PM2 process count: **13 → 7** (5 application servers + `acot-phone-server` + `lt-wordlist-api`). The original target was 4 application servers; the diagram above omits `chat-server` because it was originally a merge candidate. In practice `chat-server` runs as its own ESM process post-Phase 9 — see Deviation #16/#27.
|
||||
|
||||
---
|
||||
|
||||
@@ -778,6 +783,151 @@ Estimated effort, end-to-end: **~3 weeks of focused work** by one engineer. Phas
|
||||
|
||||
---
|
||||
|
||||
## Phase 9 — Post-audit residual gaps (NEW — 2026-05-24)
|
||||
|
||||
A second-look audit after Phase 5/8 closed surfaced six findings that the original plan either deferred, missed, or overstated. Phase 9 closes them. Code work landed in the same session as the audit, then the deploy-side steps (`caddy reload`, `pm2 reload chat-server`, frontend rebuild) were applied and smoke-tested.
|
||||
|
||||
The intent is **"do it properly the first time"** — Phase 9 isn't a quality-of-life backlog, it's the final closing-out of the consolidation project. With §9.1–9.5 applied, the consolidation/security/ESM claims in this document match the live system.
|
||||
|
||||
### 9.1 — chat-server: ESM conversion + in-process `authenticate()` + localhost bind
|
||||
|
||||
**Finding:** `chat-server` was the last application service still using `require()` (Status note misstated this in earlier revisions) and had no per-server auth — it relied entirely on the Caddy gate. `localhost:3014/test-db` returns 200 unauthenticated, falling short of the plan's three-layer defense model (Caddy `forward_auth` + per-server `authenticate()` + per-route `requirePermission`).
|
||||
|
||||
**Files touched (code landed this session):**
|
||||
- `inventory-server/chat/package.json` — add `"type": "module"`.
|
||||
- `inventory-server/chat/server.js` — `require` → `import`; load shared `.env` first then local; add a *second* `Pool` against `inventory_db` (the existing `CHAT_DB_*` pool stays for `rocketchat_converted`); mount `shared/auth/middleware.js`'s `authenticate()` on the router; mount `shared/logging/request-log.js`, `shared/cors/policy.js`, `shared/errors/handler.js` for parity with dashboard-server; bind to `127.0.0.1` instead of `0.0.0.0` (external access is via Caddy only).
|
||||
- `inventory-server/chat/routes.js` — `require` → `import`; `module.exports` → `export default`; no changes to handler bodies (they continue to read `global.pool` which `server.js` still sets).
|
||||
|
||||
**Applied / verified:**
|
||||
1. `pm2 reload chat-server --update-env` completed; PM2 shows `chat-server` online with the new PID.
|
||||
2. Syntax sweep passes under Node when excluding macOS `._*` sidecar files.
|
||||
3. `ss -ltnp` shows `chat-server` bound to `127.0.0.1:3014` (not `0.0.0.0`).
|
||||
4. Smoke:
|
||||
- `curl -s http://localhost:3014/test-db` → `401` (was: 200 unauthenticated).
|
||||
- valid short-lived Bearer token against `https://tools.acherryontop.com/chat-api/test-db` → `200`.
|
||||
- `curl -s https://tools.acherryontop.com/chat-api/test-db` (no token) → `401` at Caddy gate.
|
||||
5. PM2 logs show logged-in SPA Chat Archive requests returning 200 (`/users`, `/users/:id/rooms`, `/rooms/:id/messages`, `/messages/attachments`).
|
||||
|
||||
**Rollback:** `pm2 reload chat-server` against the previous commit (CJS version is one git checkout away).
|
||||
|
||||
### 9.2 — Caddyfile: uploads gate fix + edge CORS tightening
|
||||
|
||||
**Findings:**
|
||||
- `/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.
|
||||
|
||||
**Three edits:**
|
||||
1. Tighten `@static`:
|
||||
```
|
||||
@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.
|
||||
2. Drop the wildcard headers from `security_headers`:
|
||||
```diff
|
||||
- Access-Control-Allow-Origin "*"
|
||||
- Access-Control-Allow-Methods "GET, POST, OPTIONS, PUT, DELETE, PATCH"
|
||||
- Access-Control-Allow-Headers "DNT, X-CustomHeader, ..."
|
||||
```
|
||||
CORS responses come from the upstreams (`shared/cors/policy.js`) which set ACAO conditionally against the allow-list. Caddy stamping `*` was a holdover from before Phase 6.6.
|
||||
3. Add a preflight-bypass handler **before** `@gated`:
|
||||
```
|
||||
@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 }
|
||||
}
|
||||
```
|
||||
|
||||
**Applied / verified:**
|
||||
1. Caddy admin API load applied and `/etc/caddy/Caddyfile` persisted with the Phase 9 edits.
|
||||
2. Smoke:
|
||||
- `curl -I https://tools.acherryontop.com/uploads/reusable/<real>.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.
|
||||
|
||||
**Rollback:** restore `/etc/caddy/Caddyfile.bak.YYYY-MM-DD` and `curl -X POST .../load` against it.
|
||||
|
||||
### 9.3 — Frontend Mini\*.jsx fetch leaks
|
||||
|
||||
**Finding:** Three dashboard mini-widgets used raw `fetch()` against gated `/api/dashboard/*` endpoints, so the SPA would 401 these specific cards after Phase 6 went live.
|
||||
|
||||
**Files touched (landed this session):**
|
||||
- `inventory/src/components/dashboard/MiniInventorySnapshot.jsx` — 3 calls migrated to `apiFetch`.
|
||||
- `inventory/src/components/dashboard/MiniBusinessMetrics.jsx` — 2 calls migrated.
|
||||
- `inventory/src/components/dashboard/MiniSalesChart.jsx` — 1 call migrated.
|
||||
|
||||
**Sweep done:** `grep -rn "fetch(\`\${config.apiUrl}\|fetch(\`/api/\|fetch('/api/\|fetch(\"/api/" inventory/src` now returns no surviving raw-fetch sites against gated paths (other than the two AuthContext exceptions and `services/apiv2.ts` already excluded by Phase F1).
|
||||
|
||||
**Applied / verified:** `cd inventory && npm run build` completed successfully and the live frontend build now references the new asset hash. Raw `fetch()` sweep now shows only the documented AuthContext/App `/me` exceptions, `services/apiv2.ts`, and third-party OpenWeather calls.
|
||||
|
||||
### 9.4 — Vitest scaffold + auth-boundary tests
|
||||
|
||||
**Finding:** Plan promised a `vitest` scaffold and auth-boundary tests during Phase 2; never delivered. Root `package.json` has no `scripts` block; `inventory-server/package.json` has the default `echo "Error: no test specified"`.
|
||||
|
||||
**Files landed this session** (under `inventory-server/`):
|
||||
- `inventory-server/package.json` — adds `"test": "vitest run"`, `"test:watch": "vitest"`, plus `vitest` in `devDependencies`.
|
||||
- `inventory-server/shared/auth/verify.test.js` — five cases: valid token, expired token, wrong-signature token, malformed header, missing token.
|
||||
- `inventory-server/shared/auth/middleware.test.js` — request with no header → 401; bad header → 401; valid token + inactive user → 403; valid token + missing permission → 403; valid token + correct permission → next() called with `req.user` populated; cache TTL behavior (same token within 60s → one DB hit; after 61s → two).
|
||||
|
||||
The scaffold is intentionally narrow: it covers the security-critical surface and nothing else. Broader coverage is still a separate, larger project — see Out of scope.
|
||||
|
||||
**Applied / verified:** `cd /var/www/inventory && npm test` passes: 2 files, 21 auth-boundary tests.
|
||||
|
||||
### 9.5 — Plan-doc narrative reconciliation
|
||||
|
||||
**Finding:** The opening goal ("12 → 3 application servers + 1 auth-server", "ESM across all Node services") didn't match what shipped. Status row 13 overstated ESM coverage ("All 58 server-side files") in a way that hid chat-server from readers. Deliverables section line 845 acknowledged the chat-server gap but bullet 21 still claimed "all phases complete".
|
||||
|
||||
**Edits landed this session:**
|
||||
- Headline (line 3) rewritten to "13 PM2 processes to 5 application servers + 2 auxiliary processes = 7 total" + the "primary application Node services" qualifier on ESM.
|
||||
- Phase 3 status row clarified to scope ESM to inventory-server + auth-server, with explicit reference to Deviation #27.
|
||||
- Goals (line 34) restated with the actual outcome + the reason for the deviation.
|
||||
- Target-architecture caption corrected from `12 → 4` to `13 → 7`.
|
||||
- Concrete deliverables block rewritten to flag the four items where Phase 9 closes a gap (uploads gate, chat-server auth, chat-server ESM, edge CORS).
|
||||
- New status-table row added for Phase 9 itself.
|
||||
|
||||
**Deploy:** none — this is documentation only.
|
||||
|
||||
### 9.6 — Sequence + dependencies
|
||||
|
||||
| Step | Depends on | Risk | Notes |
|
||||
|---|---|---|---|
|
||||
| 9.5 plan-doc | — | none | Already done |
|
||||
| 9.3 frontend rebuild | 9.5 (no functional dep, just shipping order) | low | `npm run build` from inventory/ |
|
||||
| 9.4 vitest scaffold | — | none | Already done; run `npm test` to verify |
|
||||
| 9.1 chat-server reload | code conversion landed this session | medium | **Done** — PM2 reload complete; unauthenticated + authenticated smokes pass |
|
||||
| 9.2 Caddy reload | 9.1 (so the gate covers a service that already self-authenticates) | medium | **Done** — active Caddyfile includes Phase 9 edits; uploads/CORS smokes pass |
|
||||
|
||||
### 9.7 — Done criteria
|
||||
|
||||
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/<any-real>.jpg` returns `401` without auth and `200` with valid Bearer.
|
||||
- ✅ `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.
|
||||
- ✅ Frontend build passes after the three Mini\*.jsx migrations.
|
||||
|
||||
---
|
||||
|
||||
## Testing strategy
|
||||
|
||||
No formal test suite exists today (per CLAUDE.md). For a refactor this size, that's a gap to close — but writing tests retroactively for 15K LOC of routes is a separate, larger project. For this refactor:
|
||||
@@ -836,20 +986,21 @@ These came up in the audit but aren't part of this refactor:
|
||||
|
||||
## Concrete deliverables
|
||||
|
||||
State as of 2026-05-24: all planned phases are **shipped**. Note: 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).
|
||||
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`).
|
||||
- ✅ Per-upstream `authenticate()` re-verification on inventory-server, dashboard-server, and acot-server. (`chat-server` still relies on the Caddy gate alone — see asterisk below.)
|
||||
- ✅ 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.
|
||||
- ✅ 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 — done for auth/inventory/dashboard/acot.** `chat-server` is still CJS (the prior version of this document erroneously claimed it had been converted; verified 2026-05-24 — its `server.js` still uses `require()` and its `package.json` has no `"type": "module"`). Out of scope for this refactor; tracked as future work.
|
||||
- ✅ **One ESM standard across primary application Node services** — auth-server, inventory-server, dashboard-server, acot-server, and chat-server are ESM.
|
||||
- ✅ One shared `lib/` at `inventory-server/shared/` for auth, logging, DB, errors, CORS.
|
||||
- ✅ 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.
|
||||
- ✅ Caddyfile slimmed to one auth-gated block. Phase 9 §9.2 tightens the `@static` matcher + drops the edge CORS wildcard.
|
||||
- ✅ Permission codes inserted into `permissions` table for granular authorization.
|
||||
- ✅ No half-finished pieces remain. 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.
|
||||
- ✅ 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.
|
||||
|
||||
---
|
||||
|
||||
@@ -926,3 +1077,11 @@ These are decisions made during Phase 1/2 implementation that amend the spec abo
|
||||
**Fix applied — option (a):** changed `ACOT_API_URL` in `/var/www/acot-phone/.env` (and `acot-phone-server/.env.example` and the local repo copy) from `https://tools.acherryontop.com/api/acot` to `http://localhost:3012/api/acot`. Both processes live on netcup, so the request never enters Caddy and lands directly on `requirePhoneApiKey` in-process. Restarted via `pm2 restart acot-phone-server --update-env`; smoke-tested with `curl -H "x-acot-api-key: ..." http://localhost:3012/api/acot/customers/by-phone?phone=...` → 200 with the real customer record.
|
||||
|
||||
(Alternative option (b), kept here for posterity: add a `@phone_auth header x-acot-api-key *` guard in the Caddyfile to bypass `forward_auth` for requests bearing the shared secret. Would have worked too but introduces a header-based bypass in the gate, which is a worse security posture than just not entering Caddy at all.)
|
||||
|
||||
27. **chat-server stayed CJS through Phase 8, converted to ESM in Phase 9 (2026-05-24).** Why it stayed CJS originally: during Phase 4 scoping, chat-server was a notional merge candidate (fold its routes into `dashboard-server`) but rejected on inspection — it speaks a different DB (`rocketchat_converted`), a different protocol shape (file proxying + message search, not vendor-API aggregation), and shares no service code with the dashboard merge surface. Once "no merge" was decided, the secondary case for converting it (consistency, sharing `shared/auth/middleware.js`) wasn't strong enough to make it worth the Phase 4 testing budget. Phase 6 hardening also skipped it because the Caddy gate was assumed sufficient.
|
||||
|
||||
The second-look audit (2026-05-24) flagged two consequences:
|
||||
- The plan's literal claim of "ESM across all Node services" was false.
|
||||
- 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.
|
||||
|
||||
Reference in New Issue
Block a user