Re-run the security audit against everything shipped since 2026-08-28

npm audit stays clean (0 vulnerabilities). Checked every new db.prepare()/spawn()/
execFileSync() call added since the last audit for injection risk - none found, all
parameterized SQL and array-form process args. Confirmed the CORS/security-header
middleware still covers every route added since, including the new ones.

One new notable finding: POST /api/update/run unconditionally stops the server before
checking whether an update is even available, making it a more direct denial-of-service
primitive than the app's other already-documented unauthenticated endpoints. No code
change made - tracked alongside the deferred permissions/auth work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
larssand
2026-09-12 20:22:31 +02:00
parent 922bd704eb
commit ae445097b0
5 changed files with 48 additions and 3 deletions

View File

@@ -8,6 +8,12 @@ The application version is maintained in `package.json` and displayed in the adm
No changes recorded yet.
## 1.21.1 - 2026-09-12
### Changed
- Re-ran the security/dependency audit against everything shipped since 2026-08-28 (`docs/security-performance-audit.md`). `npm audit` stayed clean (0 vulnerabilities); no new SQL-injection or shell-injection risk found in any new code. One new notable finding: `POST /api/update/run` (the in-app update checker) is an unauthenticated trigger that unconditionally stops and restarts the server, making it a more direct denial-of-service primitive than the app's other already-documented unauthenticated endpoints - no code change made, tracked for the deferred permissions/auth work.
## 1.21.0 - 2026-09-12
### Changed

View File

@@ -221,6 +221,8 @@ Verified 2026-09-10 (sidebar link): a full headless-browser run against a throwa
Verified 2026-09-09: a new integration test (`tests/server_persistence.integration.test.js`) builds a throwaway local bare git repo plus two clones (standing in for "this installation" and "somewhere a newer commit was pushed"), confirms `available: false` initially, pushes a version bump from the second clone, confirms `available: true` with the correct `remoteVersion` read from its `package.json`, and confirms the 10-minute cache is respected until `force=1`. A full headless-browser run went further than any test this session before it: cloned the real project into a throwaway git working copy (own bare "remote", own temporary SQLite data dir - the real installation and its git history were never touched), started the real server from that clone, pushed a version bump to the throwaway remote from a second clone, then in a real browser opened Settings, saw "Version 9.9.9 finns tillgänglig", clicked "Uppdatera nu", accepted the confirmation dialog, and confirmed end-to-end: the server actually stopped, `git pull --ff-only` fast-forwarded, `npm install` ran, the server came back up on the same port serving the new code, and the open browser tab auto-reloaded on its own and displayed the new version - with zero uncaught JS exceptions throughout.
**Security note added 2026-09-12** (see `docs/security-performance-audit.md`'s follow-up review): the "active race" confirmation dialog described above is a client-side UX safeguard against an accidental click, not a security control - `POST /api/update/run` itself has no server-side authentication or confirmation, same as every other admin endpoint (see "Permissions and mobile Race Control" below), but is worth flagging on its own since it's a direct stop-the-server primitive rather than a data-exposure one. No code change made; tracked as part of the deferred permissions work.
## Race flow correctness: qualifying, finals, bump-up, judging, pause timing
Reported 2026-09-10 by the user, asking specifically about practice/qualifying/heats/finals, bump-up, judging, pause/start, timers and audio. A thorough read-only audit of the actual code (not assumption) found several real, concrete bugs across this area - fixed together, at the user's request ("don't want those bugs").

View File

@@ -126,3 +126,40 @@ The AMMC archive and multi-platform binaries dominate repository and deployment
4. Add request limits, rate limiting, import schema validation, and CSP.
5. Add AMMC checksum verification and platform-specific release packaging.
6. Profile a simulated 12-car race and an endurance dataset before deeper leaderboard optimization.
## Follow-up review — 2026-09-12
Requested by the user: re-check dependencies and everything added to `server.js`/the module tree since the 2026-08-28 audit above (a long session's worth of features: track layouts, race day scheduling, staggered starts, driver transponder-conflict warnings, Settings/Event Manager/Guide tab restructurings, the video-guide and screenshot capture tooling, and several new backend routes). This is a source-level re-check, not a new penetration test.
### Verification baseline
- `npm audit` and `npm audit --omit=dev`: **0 known vulnerabilities**, both re-run 2026-09-12 (3 direct production dependencies: `better-sqlite3`, `express`, `qrcode`; ~200 packages in the full production tree).
- `npm test`: 205 tests passing; `npm run test:browser`: 37 scenarios passing.
- Node runtime observed: v22.23.2 (within the `>=22 <27` engines range).
- Checked every `db.prepare()`/`db.exec()` call in `server.js` (12 total) for injection risk: all parameterized (`?` placeholders); the one call that builds SQL with a template literal (`/api/passings/purge`'s dynamic `IN (...)` clause) only interpolates a computed number of `?` placeholders, never request data itself - the actual values are still bound via `.run(...sessionIds)`.
- Checked every `spawn()`/`execFileSync()` call reachable from the server (in `server.js`, `scripts/serverctl.js`, and the two new dev-only tooling scripts) for shell-injection risk: all use array-form arguments with no `shell: true` and no string-interpolated shell commands, so none are exploitable via shell metacharacters in a config value or filename.
- Re-confirmed the CORS/security-header middleware from the 2026-08-28 audit (`X-Content-Type-Options`, `Referrer-Policy`, origin allowlist via `RC_ALLOWED_ORIGINS`) is registered before every route added since, including the new ones below - no regression.
### New finding - High: `POST /api/update/run` is an unauthenticated remote restart/update trigger
Added since the last audit (the in-app update checker, ROADMAP.md "In-app update checker and one-click updater"). This inherits the existing "Critical: administration API has no authentication" finding above, but is worth calling out on its own: unlike most other unauthenticated endpoints, which expose or mutate *data*, this one is a direct **disruption primitive**. Any client that can reach the server -
- **unconditionally stops the running server first**, before checking whether an update is even available or possible (`scripts/serverctl.js`'s `update()` calls `stop()` immediately) - so a bare `POST /api/update/run` with no real update pending still causes a guaranteed brief outage, repeatable at will (no rate limiting exists anywhere in the app - see the pre-existing "Medium: large request bodies and no rate limiting" finding above);
- then runs `git pull --ff-only` and, if that succeeds, `npm install` against whatever the configured git remote/npm registry currently serve, before restarting.
The `--ff-only` pull and graceful fallback-to-restart-on-failure (confirmed by reading `scripts/serverctl.js`) mean this cannot rewrite git history or corrupt the working tree, and a failed pull still leaves the server running on its current code - so this is not an arbitrary-code-execution vector by itself. But on a shared trackside LAN, an unauthenticated repeated-restart trigger during a live event is a real, easy-to-trigger denial of service, more severe in practice than most of the "no auth" list above because it requires no knowledge of the app's data model to abuse - just knowing the URL.
Same LAN-only deployment mitigation as the existing "Critical" finding applies today (do not expose port 8081 to the public internet); no code change made as part of this review. Worth prioritizing ahead of the rest of the deferred auth work if a minimal interim gate (e.g. restricting this one route to loopback requests, or requiring a simple shared-secret header) is wanted before the full permissions system is designed - see ROADMAP.md, "Permissions and mobile Race Control."
### New routes since 2026-08-28, otherwise covered by existing findings
No other new route introduces a new class of risk; each falls under a finding already listed above:
- `GET /api/update-status` - read-only, same "no auth" umbrella as the rest of `/api/*`.
- `GET /api/network/public-address` - calls a fixed, hardcoded external URL (`api.ipify.org`) server-side with a 5s timeout; not user-controlled, so no SSRF risk. Same "no auth" umbrella (anyone can trigger an outbound request, but only ever to that one fixed address).
- `POST /api/passings/purge` - mutating, parameterized SQL (see above), same "no auth" umbrella.
- `POST /public-training/api/:shareToken/even-laps`, `.../step-training`, `.../exercise-sessions` - each still validated against the existing per-token authorization model documented in the 2026-08-28 audit's scope (shareToken lookup, 404 on invalid/disabled/unknown) and each read-modify-writes only its own slice of `state.training` server-side (`server/public_training_profile.js`) rather than accepting a client's full state blob, the same scoped-write pattern the original audit already treated as a mitigating factor for the mobile self-service surface.
### New dev-only tooling - no production exposure
`scripts/record-video-guides.mjs` and `scripts/capture-screenshots.mjs` (this session's video-guide/screenshot generators) are not imported by `server.js` or any `src/*.js` module, are never started by any HTTP route, and are meant to be run manually by a developer against an isolated temp server/SQLite instance - they have no network-reachable attack surface and were excluded from further review on that basis beyond the injection-safety check above.

4
package-lock.json generated
View File

@@ -1,12 +1,12 @@
{
"name": "rc-racecore",
"version": "1.21.0",
"version": "1.21.1",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "rc-racecore",
"version": "1.21.0",
"version": "1.21.1",
"hasInstallScript": true,
"dependencies": {
"better-sqlite3": "^12.11.1",

View File

@@ -1,6 +1,6 @@
{
"name": "rc-racecore",
"version": "1.21.0",
"version": "1.21.1",
"private": true,
"description": "RC RaceCore live timing and race control with AMMC ingest and SQLite persistence",
"engines": {