cve-dashboard/docs/security/security-audit-tracker.md

# Security Audit Tracker — STEAM Security Dashboard

**Last scan:** 2026-06-04
**Scope:** Full repository — backend routes, middleware, helpers, scripts, frontend components
**Baseline:** `docs/security-audit-2026-04-01.md` (31 findings), `docs/security-remediation-plan.md` (17 prioritised items)

---

## Table of Contents

- [Remediation Status — April 1 Audit](#remediation-status--april-1-audit)
- [New Findings — April 20 Scan](#new-findings--april-20-scan)
- [New Findings — June 4 Scan](#new-findings--june-4-scan)
- [Open Finding Summary](#open-finding-summary)
- [Positive Security Observations](#positive-security-observations)
- [Scan Metadata](#scan-metadata)

---

## Remediation Status — April 1 Audit

Cross-reference of the 31 original findings against the current codebase. Status: **Fixed**, **Partial**, or **Open**.

### Critical Findings

| ID | Title | Status | Evidence |
|---|---|---|---|
| C-1 | Missing auth on Ivanti findings endpoints | **Fixed** | `ivantiFindings.js` — router uses `requireAuth(db)` at router level, `requireGroup` on sync |
| C-2 | `requireRole(db)` bypasses role check in KB routes | **Fixed** | `knowledgeBase.js` — uses `requireGroup('Admin', 'Standard_User')` correctly |
| C-3 | Unauthenticated finding note writes | **Fixed** | `ivantiFindings.js` — note routes behind `requireAuth(db)` |
| C-4 | No brute force protection on login | **Fixed** | `auth.js` — `loginLimiter` (20 attempts / 15 min) applied to POST /login |
| C-5 | Default credentials displayed in login UI | **Fixed** | `LoginForm.js` — no hardcoded credentials in the component |
| C-6 | Missing sandbox on KB document iframe | **Fixed** | `KnowledgeBaseViewer.js:282` — `sandbox="allow-same-origin"` applied |

### High Findings

| ID | Title | Status | Evidence |
|---|---|---|---|
| H-1 | `/cleanup-sessions` missing role check | **Fixed** | `auth.js` — `requireAuth(db), requireGroup('Admin')` applied |
| H-2 | Hardcoded fallback SESSION_SECRET | **Fixed** | `server.js:34-37` — hard-fails with `process.exit(1)` if unset |
| H-3 | Audit log parameter mismatch — silent trail gaps | **Partial** | `knowledgeBase.js` — fixed. `archerTickets.js` — `logAudit` calls missing `username` field (see N-1 below) |
| H-4 | Viewers can write compliance notes | **Fixed** | `compliance.js` — `requireGroup('Admin', 'Standard_User')` on POST /notes |
| H-5 | Sync endpoints accessible to all authenticated users | **Fixed** | Both `ivantiFindings.js` and `ivantiWorkflows.js` — `requireGroup('Admin', 'Standard_User')` on POST /sync |
| H-6 | HTTP header injection via Content-Disposition filename | **Fixed** | `knowledgeBase.js` — filename sanitized with `.replace(/["\r\n\\]/g, '')` |
| H-7 | Race condition in KB file upload | **Fixed** | `knowledgeBase.js` — file moved after DB insert succeeds |
| H-8 | Hardcoded default admin password in setup.js | **Fixed** | `setup.js` — generates random password via `crypto.randomBytes(12)` |
| H-9 | ReactMarkdown renders HTML without sanitization | **Fixed** | `KnowledgeBaseViewer.js` — `rehypeSanitize` plugin applied |

### Medium Findings

| ID | Title | Status | Evidence |
|---|---|---|---|
| M-1 | No CSRF token protection | **Open** | Cookies use `SameSite: lax` — no CSRF token implemented |
| M-2 | CORS credentials with explicit origin list | **Open** | Acceptable for this deployment model — monitor |
| M-3 | No rate limiting on NVD API proxy | **Open** | No server-side cache or per-user rate limit on `/api/nvd/lookup` |
| M-4 | Admin self-demotion check uses loose equality | **Fixed** | `users.js` — uses `String(userId) === String(req.user.id)` |
| M-5 | Missing hostname format validation | **Fixed** | `compliance.js` POST /notes — regex validation `^[a-zA-Z0-9._-]+$` |
| M-6 | Vendor field validated before trim | **Fixed** | `ivantiTodoQueue.js:11-14` — `isValidVendor()` now trims before length check |
| M-7 | Unsanitized original filename in temp JSON | **Open** | `compliance.js:347` — `req.file.originalname.replace(/[^\w.\-() ]/g, '_')` sanitizes in temp JSON, but line 355 returns raw `req.file.originalname` to client |
| M-8 | Hardcoded frontend IP in CSP header | **Fixed** | `knowledgeBase.js:302` — reads from `CORS_ORIGINS` env var |
| M-9 | API error messages forwarded to UI | **Open** | Frontend still uses `alert(err.message)` in App.js, UserManagement.js, KnowledgeBasePage.js, JiraPage.js, and AdminPage.js |
| M-10 | User data in window.confirm dialogs | **Partial** | App.js replaced with `ConfirmModal`. `JiraPage.js:430` still uses `window.confirm('Delete this Jira ticket record?')` — static string, reduced risk |

### Low / Info Findings

| ID | Title | Status | Evidence |
|---|---|---|---|
| L-1 | Silent ROLLBACK on transaction failure | **Fixed** | `compliance.js` — no `.catch(() => {})` patterns remain; ROLLBACK is followed by `throw err` |
| L-2 | Fire-and-forget audit logging | **Partial** | `auditLog.js` — now logs to `console.error` on failure, but no alerting |
| L-3 | Async temp file cleanup with no error handling | **Open** | `compliance.js` — `fs.unlink(path, () => {})` still used in 5 locations |
| L-4 | IVANTI_SKIP_TLS with no startup warning | **Open** | No startup warning when `IVANTI_SKIP_TLS=true` |
| L-5 | console.error in production frontend | **Open** | No environment guard on console.error calls — 30+ instances across frontend |
| L-6 | localStorage column config lacks structural validation | **Open** | `ReportingPage.js:65-80` — parses JSON with try/catch but validates only `Array.isArray(saved)`, not element structure |

### Remediation Plan Items (not in original 31)

| ID | Title | Status | Evidence |
|---|---|---|---|
| RP-1 | Authenticate /uploads static file access | **Fixed** | `server.js:127` — `requireAuth()` middleware applied before `express.static('uploads')` |
| RP-2 | Sanitize Mermaid SVG output with DOMPurify | **Open** | `KnowledgeBaseViewer.js:38` — `innerHTML = svg` without DOMPurify |
| RP-3 | Strip server file paths from compliance preview response | **Fixed** | `compliance.js` and `vclMultiVertical.js` — preview returns only the temp filename; commit handler reconstructs full path via `path.join(TEMP_DIR, path.basename(tempFile))` |
| RP-4 | Add SESSION_SECRET to .env.example | **Fixed** | `.env.example` — `SESSION_SECRET=` with generation comment present |

---

## New Findings — April 20 Scan

Findings discovered in the April 20 scan that were not present in the April 1 audit.

---

### N-1 — Archer Ticket Audit Logs Missing `username` Field (Medium)

**File:** `backend/routes/archerTickets.js:89, 172, 195`

All three `logAudit` calls in the Archer tickets router omit the `username` field:

```javascript
logAudit({
  userId: req.user.id,
  action: 'CREATE_ARCHER_TICKET',
  // username: req.user.username  ← missing
  ...
});
```

The `auditLog.js` helper defaults missing username to `'unknown'`, so all Archer ticket audit entries show `username = 'unknown'` instead of the actual user.

**Impact:** Audit trail for Archer ticket operations cannot identify which user performed the action. Compliance reviews and incident investigations are degraded.

**Fix:** Add `username: req.user.username` to all three `logAudit` calls.

---

### N-2 — `migrate-to-1.1.js` Contains Hardcoded Admin Password (Medium)

**Status:** **Fixed** — file removed from codebase (confirmed 2026-06-04)

---

### N-3 — Compliance Preview Returns Full Server Filesystem Path (Medium) — FIXED

**Status:** Fixed — preview now returns only the temp filename (`tempFilename`). The commit handler reconstructs the full path server-side via `path.join(TEMP_DIR, path.basename(tempFile))`. Applied to both `compliance.js` and `vclMultiVertical.js`.

---

### N-4 — `/uploads` Static Directory Served Without Authentication (High) — FIXED

**Status:** Fixed — `requireAuth()` middleware is now applied before `express.static('uploads')` in `server.js`. All file access now requires a valid session cookie.

---

### N-5 — Mermaid SVG Rendered via `innerHTML` Without Sanitization (Medium)

**File:** `frontend/src/components/KnowledgeBaseViewer.js:38`

```javascript
ref.current.innerHTML = svg;
```

Mermaid-generated SVG is injected directly into the DOM via `innerHTML`. While Mermaid itself sanitizes most input, a crafted diagram definition in a knowledge base article could potentially produce SVG with embedded event handlers or script elements.

**Impact:** Stored XSS vector if Mermaid's internal sanitization is bypassed. Any user viewing the article would execute the payload.

**Fix:** Sanitize the SVG string before injection:
```javascript
import DOMPurify from 'dompurify';
ref.current.innerHTML = DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } });
```

---

### N-6 — `SESSION_SECRET` Not Documented in `.env.example` (Low)

**Status:** **Fixed** — `.env.example` now includes `SESSION_SECRET=` with generation instructions (confirmed 2026-06-04)

---

### N-7 — `requireGroup` Error Response Leaks Current User Group (Low)

**File:** `backend/middleware/auth.js:55-60`

```javascript
return res.status(403).json({
    error: 'Insufficient permissions',
    required: allowedGroups,
    current: req.user.group
});
```

The 403 response includes both the required groups and the user's current group. This is minor information disclosure — an attacker probing endpoints learns the exact group membership of the compromised account and which groups are needed.

**Fix:** Remove `required` and `current` from the response:
```javascript
return res.status(403).json({ error: 'Insufficient permissions' });
```

---

### N-8 — No Content-Security-Policy Header on Main Application (Medium)

**File:** `backend/server.js:107-113`

Security headers include `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Protection`, `Referrer-Policy`, and `Permissions-Policy`, but no `Content-Security-Policy` header. CSP is the primary browser-side defense against XSS.

**Impact:** No browser-enforced restriction on script sources. If an XSS vulnerability exists (e.g. N-5), there is no CSP to mitigate it.

**Fix:** Add a baseline CSP header:
```javascript
res.setHeader('Content-Security-Policy',
    "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; " +
    "img-src 'self' data:; font-src 'self'; connect-src 'self'");
```
Start with `Content-Security-Policy-Report-Only` to avoid breaking existing functionality.

---

### N-9 — Expired Sessions Not Cleaned Up Automatically (Low)

**File:** `backend/server.js`, `backend/routes/auth.js`

The `sessions` table has no automatic cleanup. Expired sessions accumulate indefinitely. The `/cleanup-sessions` endpoint exists but must be triggered manually by an admin.

**Impact:** Performance degradation over time as the sessions table grows. Not directly exploitable, but expired session rows increase the surface for timing attacks on session lookups.

**Fix:** Add a cleanup interval on server startup:
```javascript
setInterval(() => {
    pool.query("DELETE FROM sessions WHERE expires_at < NOW()");
}, 6 * 60 * 60 * 1000); // every 6 hours
```

---

## New Findings — June 4 Scan

Findings discovered in the June 4, 2026 scan.

---

### N-10 — Feedback Route Uses Incorrect TLS Option Name (Low)

**File:** `backend/routes/feedback.js:92, 221`

```javascript
const reqOpts = {
    // ...
    rejectAuthorized: false,
};
```

The option is spelled `rejectAuthorized` instead of the correct `rejectUnauthorized`. This means the option is silently ignored by Node's `https` module — TLS verification remains **enabled** (the secure default), so this is not a vulnerability in itself. However, it indicates the developer intended to disable TLS verification (likely for Charter's SSL inspection proxy), and the code does not achieve its intent. If the GitLab instance uses an internal CA certificate, feedback submissions will fail with a TLS error when the proxy is active.

**Impact:** Feedback submissions may fail behind the SSL inspection proxy due to unintended TLS verification. Not a security vulnerability — TLS remains enforced.

**Fix:** If TLS skip is intentional, use the correct option name and gate it behind an env var:
```javascript
rejectUnauthorized: process.env.GITLAB_SKIP_TLS === 'true' ? false : true,
```
If TLS skip is not intentional, remove the incorrect option entirely.

---

### N-11 — `window.confirm` Persists in JiraPage.js (Low)

**File:** `frontend/src/components/pages/JiraPage.js:430`

```javascript
if (!window.confirm('Delete this Jira ticket record?')) return;
```

The main `App.js` was migrated to use `ConfirmModal` (a themed replacement for `window.confirm`), but `JiraPage.js` still uses the raw browser `confirm()` dialog. While the confirmation string is static (not user-supplied data), this is inconsistent with the security pattern established elsewhere in the codebase.

**Impact:** Low — the string is static so there is no XSS vector. This is a consistency issue rather than a vulnerability.

**Fix:** Replace with the `ConfirmModal` pattern used in `App.js`:
```javascript
setPendingConfirm({ message: 'Delete this Jira ticket record?', onConfirm: () => doDelete(id) });
```

---

## Open Finding Summary

Prioritised list of all open findings requiring action.

### High Priority

| ID | Severity | Title | Source |
|---|---|---|---|
| — | — | (none — all High findings resolved) | — |

### Medium Priority

| ID | Severity | Title | Source |
|---|---|---|---|
| M-1 | Medium | No CSRF token protection | April 1 |
| M-3 | Medium | No rate limiting on NVD API proxy | April 1 |
| N-1 | Medium | Archer ticket audit logs missing `username` field | April 20 |
| N-5 / RP-2 | Medium | Mermaid SVG rendered via `innerHTML` without sanitization | April 20 |
| N-8 | Medium | No Content-Security-Policy header on main application | April 20 |
| M-7 | Medium | Unsanitized original filename returned to client in preview response | April 1 |
| M-9 | Medium | API error messages forwarded to UI via `alert()` | April 1 |

### Low Priority

| ID | Severity | Title | Source |
|---|---|---|---|
| M-10 | Low | User data in `window.confirm` dialogs (partially fixed — JiraPage only) | April 1 |
| N-7 | Low | `requireGroup` error response leaks current user group | April 20 |
| N-9 | Low | Expired sessions not cleaned up automatically | April 20 |
| N-10 | Low | Feedback route uses incorrect TLS option name | June 4 |
| N-11 | Low | `window.confirm` persists in JiraPage.js | June 4 |
| L-3 | Low | Async temp file cleanup with no error handling | April 1 |
| L-4 | Low | IVANTI_SKIP_TLS with no startup warning | April 1 |
| L-5 | Low | console.error in production frontend | April 1 |
| L-6 | Low | localStorage column config lacks structural validation | April 1 |

---

## Positive Security Observations

Verified secure patterns that should be preserved:

- **SQL injection prevention** — all queries use parameterized statements throughout the entire codebase (PostgreSQL `$1` placeholders)
- **Path traversal prevention** — `sanitizePathSegment()` and `isPathWithinUploads()` consistently applied in `server.js`, `compliance.js`, and `knowledgeBase.js`
- **Python script execution** — `spawn('python3', [SCRIPT, filePath])` with argument arrays — no shell injection
- **File upload security** — extension allowlist + MIME prefix validation + 10 MB size limit via multer
- **Password hashing** — bcrypt with cost factor 10 used for all password storage
- **Session management** — 32-byte random session IDs via `crypto.randomBytes`, httpOnly cookies, 24h expiry
- **Rate limiting** — login endpoint protected with 20 attempts per 15-minute window; password change limited to 5 per 15 minutes
- **Audit trail** — comprehensive audit logging on all state-changing operations (with noted exceptions above)
- **Self-modification prevention** — admin cannot demote or deactivate their own account
- **Ownership-scoped deletion** — Standard_User can only delete resources they created
- **Compliance linkage protection** — deletion blocked when tickets are linked to active compliance reports
- **Temp file path validation** — `isSafeTempPath()` enforces `.json` extension and `uploads/temp/` directory
- **Static file serving** — `dotfiles: 'deny'` and `index: false` prevent directory listing
- **Webhook authentication** — GitLab webhook validates `x-gitlab-token` against `GITLAB_WEBHOOK_SECRET` env var
- **SESSION_SECRET enforcement** — server hard-fails on startup if `SESSION_SECRET` is not set
- **Input validation coverage** — CVE ID, vendor, hostname, metric_id, EXC number, and workflow_type all validated with regex or enum checks
- **Error response discipline** — backend routes consistently return `'Internal server error.'` for 500s, avoiding stack trace leaks

---

## Scan Metadata

| Field | Value |
|---|---|
| Scan date | 2026-06-04 |
| Scan type | Full repository static analysis |
| Scope | `backend/server.js`, `backend/routes/`, `backend/middleware/`, `backend/helpers/`, `backend/scripts/`, `backend/migrations/`, `frontend/src/`, `.env.example` |
| Baseline | `docs/security-audit-2026-04-01.md` |
| Previous findings | 31 (6 Critical, 9 High, 10 Medium, 6 Low/Info) + 9 new (April 20) + 4 remediation plan items |
| Fixed since last scan | 5 (N-2, N-6, M-6, RP-4, L-1) |
| Downgraded | 1 (M-10: Medium → Low, partially fixed) |
| Still open (from baseline) | 10 |
| Still open (from April 20) | 7 |
| New findings (June 4) | 2 |
| Total open | 18 (1 High, 8 Medium, 9 Low) |
| Regressions | 0 |
| Methodology | Static analysis — code review of all route handlers, middleware, helpers, and frontend components |