chore: reorganize docs, document migrations, gitignore operational files for v1.0.0 release
This commit is contained in:
154
docs/security/security-remediation-plan.md
Normal file
154
docs/security/security-remediation-plan.md
Normal file
@@ -0,0 +1,154 @@
|
||||
# Security Remediation Plan
|
||||
|
||||
Based on the External Data Handling security audit (April 2026). 17 findings total — 0 Critical, 2 High, 6 Medium, 6 Low, 3 Informational. Ordered by priority based on real-world exploitability and effort.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Data Exposure & XSS (High Priority)
|
||||
|
||||
### 1. L-4: Authenticate /uploads static file access
|
||||
**Location:** `server.js:127`
|
||||
**Risk:** Uploaded documents (vulnerability data, compliance files) served without authentication. Anyone with the URL can access them.
|
||||
**Fix:** Replace `express.static('/uploads')` with a route handler that runs `requireAuth(db)` before streaming the file. Use `res.sendFile()` with the validated path.
|
||||
**Effort:** Small — single route change.
|
||||
|
||||
### 2. M-6: Sanitize Mermaid SVG output with DOMPurify
|
||||
**Location:** `frontend/src/components/KnowledgeBaseViewer.js:38`
|
||||
**Risk:** Mermaid renders SVG which is injected via `innerHTML`. If KB content contains malicious markup, this is a stored XSS vector.
|
||||
**Fix:** Install `dompurify`, sanitize the SVG string before assigning to `innerHTML`. Use `DOMPurify.sanitize(svgString, { USE_PROFILES: { svg: true } })`.
|
||||
**Effort:** Small — add dependency, wrap one line.
|
||||
|
||||
### 3. M-4: Strip server file paths from compliance preview response
|
||||
**Location:** `backend/routes/compliance.js:278`
|
||||
**Risk:** Full server-side file path returned to client. Helps attackers map the filesystem.
|
||||
**Fix:** Return only the filename (use `path.basename()`) instead of the full path. Or return a reference ID that maps to the file server-side.
|
||||
**Effort:** Small — one-line change.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — Deployment & Setup Hygiene
|
||||
|
||||
### 4. H-2: Add SESSION_SECRET to .env.example and setup-env.sh
|
||||
**Location:** `backend/.env.example`, `backend/setup-env.sh`
|
||||
**Risk:** Fresh deployments fail with no guidance on required env vars.
|
||||
**Fix:** Add `SESSION_SECRET=` to `.env.example` with a comment explaining it should be a random 64+ character string. Add generation logic to `setup-env.sh` (e.g., `openssl rand -hex 32`).
|
||||
**Effort:** Small.
|
||||
|
||||
### 5. I-3: Set user_group on default admin in setup.js
|
||||
**Location:** `backend/setup.js:180`
|
||||
**Risk:** Default admin created without `user_group`, potentially locked out of `requireGroup`-protected routes on fresh install.
|
||||
**Fix:** Set `user_group = 'Admin'` in the INSERT statement for the default admin user.
|
||||
**Effort:** Trivial — one column added to the INSERT.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3 — Error Message Sanitization (Batch)
|
||||
|
||||
### 6. L-2: Sanitize Python parser error messages
|
||||
**Location:** `backend/routes/compliance.js:284`
|
||||
**Risk:** Stack traces and server paths leaked to client when Python parser fails.
|
||||
**Fix:** Catch the error, log the full details server-side, return a generic "Compliance file parsing failed" message to the client.
|
||||
**Effort:** Small.
|
||||
|
||||
### 7. L-3: Sanitize Ivanti API error responses
|
||||
**Location:** `backend/routes/ivantiFpWorkflow.js:393`
|
||||
**Risk:** Raw Ivanti API error body forwarded to client, potentially exposing internal API details.
|
||||
**Fix:** Log the raw error server-side, return a generic "Ivanti API request failed" message to the client.
|
||||
**Effort:** Small.
|
||||
|
||||
### 8. L-6: Remove group name from requireGroup error response
|
||||
**Location:** `backend/middleware/auth.js:60`
|
||||
**Risk:** Error response leaks the user's current group name, which is minor info disclosure.
|
||||
**Fix:** Change the error message from something like "User group 'Viewer' not authorized" to "Insufficient permissions."
|
||||
**Effort:** Trivial.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Security Headers
|
||||
|
||||
### 9. M-1: Add Content-Security-Policy header
|
||||
**Location:** `server.js:107-113`
|
||||
**Risk:** No CSP means no browser-side XSS mitigation layer.
|
||||
**Fix:** Add a CSP header via middleware. Start with a report-only policy to avoid breaking things, then tighten. Suggested baseline:
|
||||
```
|
||||
default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self'
|
||||
```
|
||||
Note: `'unsafe-inline'` for styles is needed because the app uses inline style objects extensively. Evaluate whether `script-src 'self'` breaks anything (it shouldn't with CRA).
|
||||
**Effort:** Medium — needs testing to ensure nothing breaks.
|
||||
|
||||
### 10. M-2: Add Strict-Transport-Security (HSTS) header
|
||||
**Location:** `server.js:107-113`
|
||||
**Risk:** No HSTS means browsers don't enforce HTTPS on subsequent visits.
|
||||
**Fix:** Add `Strict-Transport-Security: max-age=31536000; includeSubDomains` header. Only apply when running behind HTTPS (check `req.secure` or a trusted proxy header). Do NOT enable if the app is accessed over plain HTTP.
|
||||
**Effort:** Small, but verify deployment is HTTPS-only first.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5 — Operational Maintenance
|
||||
|
||||
### 11. L-5: Add expired session cleanup
|
||||
**Location:** `backend/middleware/auth.js:271`
|
||||
**Risk:** Sessions table grows indefinitely. Not a security exploit, but degrades performance over time.
|
||||
**Fix:** Add a cleanup function that runs on server startup (and optionally on a setInterval) to DELETE sessions where `expires_at < CURRENT_TIMESTAMP`. Run once at boot, then every 6 hours.
|
||||
**Effort:** Small.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6 — Session Signing (Larger Effort)
|
||||
|
||||
### 12. H-1: Use SESSION_SECRET for HMAC-signed session tokens
|
||||
**Location:** `server.js:33`
|
||||
**Risk:** Session tokens are random bytes stored in DB with no signing. An attacker with DB read access can replay any session. For self-hosted SQLite, DB access already implies full compromise, so this is a defense-in-depth measure.
|
||||
**Fix:** When creating a session, generate a random token and store its HMAC (using SESSION_SECRET) in the DB. On validation, recompute the HMAC and compare. This means a DB dump alone isn't enough to forge sessions — the attacker also needs the secret.
|
||||
**Effort:** Medium — touches session creation, validation, and requires SESSION_SECRET to actually be wired in.
|
||||
|
||||
---
|
||||
|
||||
## Phase 7 — Investigate Before Changing
|
||||
|
||||
### 13. M-3: Review application/octet-stream in MIME allowlist
|
||||
**Location:** `server.js:62`
|
||||
**Risk:** Allows uploads that bypass MIME type checking. May be intentional for specific file types.
|
||||
**Action:** Check what file types are uploaded that resolve to `application/octet-stream`. If none are legitimate, remove it from the allowlist. If some are (e.g., `.db` files, binary exports), consider adding those specific MIME types instead.
|
||||
**Effort:** Investigation first, then trivial change.
|
||||
|
||||
### 14. M-5: Evaluate CORS HTTP origin policy
|
||||
**Location:** `server.js:38-40`
|
||||
**Risk:** CORS allows HTTP origins, no HTTPS enforcement.
|
||||
**Action:** Check if production runs behind a reverse proxy with HTTPS termination. If yes, the backend legitimately sees HTTP origins from the proxy. If production traffic is ever plain HTTP end-to-end, restrict CORS to HTTPS origins only.
|
||||
**Effort:** Investigation first, then small config change.
|
||||
|
||||
---
|
||||
|
||||
## Phase 8 — Low Priority / Monitor
|
||||
|
||||
### 15. L-1: Add startup warning for IVANTI_SKIP_TLS=true
|
||||
**Location:** `backend/helpers/ivantiApi.js:28`
|
||||
**Risk:** TLS validation disabled silently. Acceptable in dev, risky if accidentally left on in production.
|
||||
**Fix:** Add a `console.warn('⚠ IVANTI_SKIP_TLS is enabled — TLS certificate validation is disabled')` at startup when the flag is set.
|
||||
**Effort:** Trivial.
|
||||
|
||||
### 16. I-1: Monitor react-scripts version
|
||||
**Location:** `frontend/package.json`
|
||||
**Risk:** Build-time only, not runtime. No immediate action needed.
|
||||
**Action:** Upgrade to latest react-scripts when convenient. Consider migrating to Vite if a major frontend overhaul is planned.
|
||||
|
||||
### 17. I-2: Monitor xlsx dependency
|
||||
**Location:** `frontend/package.json`
|
||||
**Risk:** Community fork, unmaintained since 2022. Used for spreadsheet parsing.
|
||||
**Action:** Monitor for security advisories. If a vulnerability is found, evaluate alternatives (e.g., `exceljs`, `sheetjs` pro). No immediate action needed unless a CVE is published against it.
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| Phase | Items | Effort | Impact |
|
||||
|-------|-------|--------|--------|
|
||||
| 1 — Data Exposure & XSS | L-4, M-6, M-4 | Small | High |
|
||||
| 2 — Deployment Hygiene | H-2, I-3 | Small | Medium |
|
||||
| 3 — Error Sanitization | L-2, L-3, L-6 | Small | Low-Medium |
|
||||
| 4 — Security Headers | M-1, M-2 | Medium | Medium |
|
||||
| 5 — Session Cleanup | L-5 | Small | Low |
|
||||
| 6 — Session Signing | H-1 | Medium | Medium |
|
||||
| 7 — Investigate | M-3, M-5 | Investigation | TBD |
|
||||
| 8 — Monitor | L-1, I-1, I-2 | Trivial | Low |
|
||||
Reference in New Issue
Block a user