admin/raven
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Security review pass 9: client, headers, CORS, Swagger

Browse Source 
From code: some record text is rendered as HTML without escaping (F33); shipped
DOMPurify and Vuetify are behind on security fixes (F34, needs a decision on
front-end packages); headers, CORS, Swagger and keys in URLs (F35). npm audit
run read-only and triaged. Review only, no code or dependency changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John Cardinal
2026-10-23 20:33:26 -07:00
parent f1f01b47f1
commit 52763366c2
1 changed files with 54 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
devdocs/modernization/SECURITY-REVIEW.md
View File

@@ -19,8 +19,8 @@ keep it that way while findings are open.
| 6 | Raw SQL: data lists, saved filters, search, KPI, schedule, pick lists | Done 2026-10-23 |
| 7 | Staff roles: the authenticated routes without a role check in the action (list below) | Done 2026-10-23 |
| 8 | Email and notifications (`direct-smtp`, templates, delivery) | Done 2026-10-23 |
| 9 | Client (XSS, token storage, `npm audit` triage), CORS, headers, Swagger UI | Next |
| 10 | Deployment: installers, Linux setup script, nginx, systemd, config and data permissions | |
| 9 | Client (XSS, token storage, `npm audit` triage), CORS, headers, Swagger UI | Done 2026-10-23 |
| 10 | Deployment: installers, Linux setup script, nginx, systemd, config and data permissions | Next |
| 11 | Sockeye public routes (licence fetch, trial requests) | |
The order is by risk to internet-facing servers. Findings from any pass go in **Findings**.
@@ -200,12 +200,14 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca
| F20 | High | candidate | Report templates receive the renderer's token (SuperUser for customer notification reports) and the report browser can reach any network address |
| F27 | High | **confirmed** | Restricted roles (TechRestricted, SubContractor, SubContractorRestricted) get whole work-order items through report data, rendering and export |
| F28 | High | candidate | Users with change rights on Customer (Service, Sales, Accounting) can give customer contact logins any role, BizAdmin and OpsAdmin included |
| F33 | High | confirmed (code) | Client renders some record text as HTML without escaping (search results, work-order address dialog, unit warranty info, dialogs) |
| F3 | High | confirmed (code) | Any logged-in user, customer contacts included, can send arbitrary email through the company's SMTP server |
| F23 | Medium | confirmed (code) | Logo upload accepts any content type and the anonymous logo route serves it inline from the app's origin (stored XSS) |
| F24 | Medium | confirmed (code) | Translation import has no role check: any login can add translations |
| F30 | Medium | confirmed (code) | SMTP: the mail server's TLS certificate is never checked |
| F31 | Medium | confirmed (code) | The SMTP password is returned to read-only roles (OpsAdminRestricted, BizAdmin, BizAdminRestricted) |
| F32 | Medium | confirmed (code) | A real-looking SMTP account and password are in the DEBUG defaults in `raven` and `sockeye` source |
| F34 | Medium | confirmed | Shipped client libraries behind on security fixes: DOMPurify 2.3.8 (wiki sanitiser), Vuetify 2.6.6 |
| F6 | Medium | confirmed | No lockout or rate limit on login or the 2FA step; no password rules |
| F9 | Medium | confirmed | Default SuperUser password, and an anonymous route that says whether it is still set |
| F2 | Medium | candidate | Windows installers give all local users modify rights on the program and data folders |
@@ -217,6 +219,7 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca
| F12 | Low | confirmed | Password hashing: PBKDF2-SHA512 with 10,000 iterations |
| F25 | Low | confirmed (code) | Attachment and export gaps: upload stored before the rights check, "attached by" can be set to anyone, exports not tied to their user |
| F29 | Low | confirmed (code) | `server-info` gives every login, customer contacts included, the server DB ID and full licence details |
| F35 | Low | confirmed | No security headers from the server itself (self-hosted), no CSP anywhere, CORS any origin, public Swagger UI, download keys in URLs |
| F22 | Low | confirmed | Minor report and job route gaps: template export by any download key, job status, logs and cancel by GUID without an owner check, browser without sandbox |
**Release decisions.** John, 2026-10-23: **F13 to F17 are urgent and are fixed in 8.3.0**, as
@@ -390,7 +393,7 @@ CSR `Change` includes `Customer`, and `HasReadFullRole` counts `Change` as read.
`CustomerServiceRequestBiz` never checks `CustomerId` against the contact (create, get, put,
delete). **Proof**: the contact read CSR 1 of customer 16 (200, with its text). Change, delete
and create for another customer were not tested (they write data) but follow from the same
code. Search also returns CSRs of all customers (it filters by role only).
code. Search also returns CSRs of all customers (it filters by role only). Pass 9: `unit/customerunit` (`UnitBiz.CustomerCreateAsync`) also takes `CustomerId` from the request, so a contact can add units to any customer; same fix (force the contact's customer).
### F15 Pick lists expose the whole customer list (pass 3, confirmed)
@@ -659,6 +662,53 @@ OK in pass 8: SMTP settings change and the test-send route are OpsAdmin only; ba
records' own addresses; notification emails are plain text built from record names (no HTML
injection); the customer notification "who" query escapes tags.
### F33 Unescaped record text rendered as HTML in the client (pass 9, from code)
Of the 13 `v-html` uses in `raven-client`, these render text that users enter, without
escaping it first:
- `views/home-search.vue`: the search result excerpt (`item.info`, from `Search.GetInfoAsync`,
which does not encode it), with the search terms highlighted. Searchable text includes
fields customer contacts can write (service requests, units).
- `components/work-order-address.vue`: addresses built by `formatAddress` from customer, head
office and unit address fields.
- `components/work-order-item-units.vue`, `quote-item-units.vue`, `pm-item-units.vue`: the
warranty summary (vendor name, receipt number, work order list).
- `components/gzconfirm.vue`: dialog messages, which are translation strings; with F24 any login
can import a translation.
The session token is kept in `localStorage` (`vuex-persistedstate`), so script running in the
page could act as the signed-in user. Fix: escape these values (an `escapeHtml` helper, or
build the markup with Vue templates instead of strings); encode the excerpt on the server too;
render translation strings as text. Safe: the wiki (`marked` then DOMPurify, see F34), the
calendar summaries (Vuetify escapes event names), and the remaining sinks (fixed strings).
A Content-Security-Policy (F35) would be a second layer.
### F34 Client libraries behind on security fixes (pass 9)
`npm audit` (read-only, lockfile) reports 550 advisories, almost all in build tools (webpack,
Vue CLI, Babel, ESLint) that run only on the build machine and do not ship. Of the packages
the app ships: **DOMPurify 2.3.8**, the wiki sanitiser, has sanitiser-bypass fixes in 2.4.x and
2.5.x; **Vuetify 2.6.6** has XSS fixes in 2.6.10 (the last 2.x is 2.7.x); `marked` 1.2.9, `luxon`
1.28.0 and `vue` 2.6.14 have only slow-regex (browser-side) advisories. Fix: raise DOMPurify to
the latest 2.5.x and Vuetify to the latest 2.7.x, both on the same major version. This is a
front-end dependency change, which CLAUDE.md rules out without John's decision; check that
both still build with Node 12.22.9.
### F35 Headers, CORS, Swagger, keys in URLs (pass 9)
- The server sets no security headers. Hosted droplets get HSTS, `nosniff`, `X-Frame-Options`
and `Referrer-Policy` from the nginx template in the setup script; self-hosted IIS, Linux and
Windows installs get none. No deployment sets a Content-Security-Policy.
- CORS allows any origin, method and header. Impact is small because the token travels in the
`Authorization` header (not a cookie), so another site cannot use a visitor's session.
- Swagger UI and the full API description are public at `/api-docs`.
- Download keys (`?t=`) are in URLs, so they reach proxy access logs and browser history;
they last 5 days (F7 makes some last longer).
Fix: add the headers in the app (so every deployment gets them), with a CSP that allows the
client's own scripts; limit CORS to the server's own origin unless API users need more;
consider Swagger UI for logged-in users only.
### F2 Windows folder permissions (from the 8.3.0 installer test)
`lan.iss` and `standalone.iss` set `Permissions: users-modify` on `{app}` (the program
@@ -737,3 +787,4 @@ roles that should not see the schedule (pass 7).
| 2026-10-23 | 6 Raw SQL | Code reading only. Listed every hand-built query (`CommandText`, `*SqlRaw`, interpolated SQL) and traced each request value into it: data-list filter, sort and select builders, all 51 `ClientCriteria` users, saved filters, search tokenizer, every KPI's criteria, pick-list builder and variants, customer notification query, controllers and biz classes with raw SQL. New: **F26** (KPI `interval`). F18 completed by code. Everything else safe (see "Raw SQL: what was checked") |
| 2026-10-23 | 7 Staff roles | Went through the remaining logged-in routes without a role check, the restricted-role filtering in `WorkOrderBiz`, and user and contact administration in `UserBiz`. Tested on the local dev server, read-only: **F27 confirmed** (`SubContractorRestricted` through `report/data`). F28 from code (live test not done). New: F29. The rest of the list found OK (see "Pass 7: routes ... found OK") |
| 2026-10-23 | 8 Email and notifications | Code reading only, nothing sent. Read `NotifyController` (direct SMTP and message), `Mailer`, `GlobalOpsNotificationSettings` controller, biz and model, `NotifyEventHelper`, `CoreJobNotify`, `CoreJobCustomerNotify`, `NotifySubscriptionBiz`. F3 and F4 confirmed by code; F17 confirmed and re-rated Medium (names and links, no reports). New: F30, F31, F32. **Action for John: rotate the FastMail app password in F32** |
| 2026-10-23 | 9 Client, headers, CORS, Swagger | Code reading of every `v-html` use in `raven-client` and its data source; `npm audit` read-only against the lockfile (no install, no change), triaged to shipped packages; server and nginx headers, CORS, Swagger. New: F33, F34 (needs John's decision on two front-end packages), F35; F14 extended (`unit/customerunit`) |