feat(security): CI secret-scan + dep-audit, SECURITY.md, auth-coverage test (Pillar C)
- CI now runs gitleaks (secret scanning, .gitleaks.toml allowlists fixtures/data/generated) and `npm audit --audit-level=critical`. - SECURITY.md: disclosure policy, the controls in place, operational hardening, and the two assessed HIGH audit exceptions (nodemailer `raw` unused → not exploitable; xlsx confined to import w/ raw:false + limits). - tests/securityCoverage.test.js: static invariant that every non-public router mount carries requireAuth (+ CSRF for authed, requireAdmin for admin) — a new unprotected route now fails CI. 4 tests; suite 236/236. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
e7196abbdf
commit
aee23d6025
|
|
@ -23,6 +23,16 @@ jobs:
|
|||
- name: Install dependencies
|
||||
run: npm ci
|
||||
|
||||
- name: Secret scan (gitleaks)
|
||||
run: |
|
||||
GITLEAKS_VERSION=8.21.2
|
||||
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \
|
||||
| tar -xz -C /usr/local/bin gitleaks
|
||||
gitleaks detect --source=. --redact --no-banner
|
||||
|
||||
- name: Dependency audit (fail on critical)
|
||||
run: npm audit --omit=dev --audit-level=critical
|
||||
|
||||
- name: Format check (Prettier)
|
||||
run: npm run format:check
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,17 @@
|
|||
title = "Bill Tracker gitleaks config"
|
||||
|
||||
[extend]
|
||||
useDefault = true
|
||||
|
||||
[allowlist]
|
||||
description = "Paths with no real secrets (placeholders, fixtures, generated, runtime data)"
|
||||
paths = [
|
||||
'''\.env\.example$''',
|
||||
'''data/''',
|
||||
'''.*\.db$''',
|
||||
'''.*\.db-(wal|shm)$''',
|
||||
'''package-lock\.json$''',
|
||||
'''tests/.*\.test\.js$''',
|
||||
'''e2e/.*''',
|
||||
'''docs/.*\.json$''',
|
||||
]
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
# Security
|
||||
|
||||
Bill Tracker is a self-hosted app that stores financial records, bank connections, and
|
||||
encryption keys. Security is a first-class concern.
|
||||
|
||||
## Reporting a vulnerability
|
||||
|
||||
Please report suspected vulnerabilities privately to the maintainer (do not open a public
|
||||
issue). Include steps to reproduce and impact. We aim to acknowledge within a few days.
|
||||
|
||||
## Controls in place
|
||||
|
||||
- **Authentication:** bcrypt passwords, hashed + rotating session tokens, optional TOTP 2FA
|
||||
and WebAuthn/FIDO2, OIDC. Failed-login tracking + login history.
|
||||
- **Authorization:** `requireAuth` / `requireUser` / `requireAdmin` on every non-public
|
||||
route; all user data is scoped by `user_id` (isolation is covered by tests).
|
||||
- **CSRF:** double-submit token (`csrfMiddleware`) on all authenticated mutating routes.
|
||||
- **Rate limiting:** on auth, admin, import/export, backup, and password-change surfaces.
|
||||
- **Encryption at rest:** AES-256-GCM with HKDF key derivation for stored secrets
|
||||
(SimpleFIN token, SMTP/OIDC secrets, login metadata); a non-reversible key fingerprint
|
||||
lets operators verify the active key. Set `TOKEN_ENCRYPTION_KEY` in production so the key
|
||||
lives outside the database.
|
||||
- **Transport/headers:** security headers + CSP (`securityHeaders`), Secure/HttpOnly/SameSite
|
||||
cookies, opt-in CORS allowlist.
|
||||
- **Audit:** security-sensitive actions recorded to `audit_log`.
|
||||
- **Automated in CI:** secret scanning (gitleaks), dependency audit (`npm audit`), lint +
|
||||
typecheck + tests on every push.
|
||||
|
||||
## Known dependency-audit exceptions (assessed)
|
||||
|
||||
`npm audit` reports two HIGH advisories that are **not currently actionable via npm** and are
|
||||
assessed as follows:
|
||||
|
||||
- **nodemailer — `raw` message option file-read/SSRF.** Not exploitable here: the app builds
|
||||
messages via the normal API and never passes a user-controlled `raw` option. The npm fix
|
||||
requires nodemailer 9 (breaking); deferred until a maintenance window with SMTP re-test.
|
||||
- **xlsx (SheetJS) — prototype pollution.** Confined to `services/spreadsheetImportService.cts`
|
||||
(import only), which parses with `raw: false` behind a 10 MB size cap and content-type
|
||||
allowlist. SheetJS publishes fixes only via its own CDN (not npm); migrating to the CDN
|
||||
build (or a maintained alternative) is tracked as follow-up.
|
||||
|
||||
The CI dependency gate fails on **critical**; highs are reviewed here.
|
||||
|
||||
## Operational hardening
|
||||
|
||||
- Set `TOKEN_ENCRYPTION_KEY` (outside the DB) in production.
|
||||
- Run behind HTTPS with `TRUST_PROXY` set appropriately.
|
||||
- Turn off verbose/debug logging (e.g. `simplefin_debug_logging`) in production.
|
||||
- Back up `data/db` encrypted; never commit databases or keys (gitignored + gitleaks).
|
||||
|
|
@ -0,0 +1,73 @@
|
|||
'use strict';
|
||||
|
||||
// Security invariant: every API router mount is either explicitly public or carries
|
||||
// requireAuth (+ CSRF for authenticated routers). This is a static guard so a new
|
||||
// route added without auth/CSRF fails CI instead of silently exposing data.
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const server = fs.readFileSync(path.join(__dirname, '..', 'server.cts'), 'utf8');
|
||||
|
||||
// Routers that are intentionally public (or handle their own auth internally).
|
||||
const PUBLIC_ROUTERS = new Set([
|
||||
'auth', // login/register/csrf-token are public; authed sub-routes guard themselves
|
||||
'authOidc', // OIDC login/callback are public entry points
|
||||
'calendarFeed', // token-authenticated ICS feed (no session)
|
||||
'about', // public marketing/info
|
||||
'privacy', // public policy
|
||||
'version', // public version / release-notes / update-status
|
||||
]);
|
||||
|
||||
// Match: app.use('<path>', <middleware...>, require('./routes/<file>.cts'))
|
||||
const mountRe =
|
||||
/app\.use\(\s*['"](\/api\/[^'"]+)['"]([\s\S]*?)require\(\s*['"]\.\/routes\/([A-Za-z0-9_-]+)\.cts['"]\s*\)/g;
|
||||
|
||||
const mounts = [];
|
||||
for (let m; (m = mountRe.exec(server));) {
|
||||
mounts.push({ path: m[1], middleware: m[2], router: m[3] });
|
||||
}
|
||||
|
||||
test('server.cts mounts are discoverable', () => {
|
||||
assert.ok(mounts.length >= 20, `expected many router mounts, found ${mounts.length}`);
|
||||
});
|
||||
|
||||
test('every non-public router mount requires authentication', () => {
|
||||
const offenders = mounts
|
||||
.filter((m) => !PUBLIC_ROUTERS.has(m.router))
|
||||
.filter((m) => !/requireAuth/.test(m.middleware))
|
||||
.map((m) => `${m.path} → routes/${m.router}.cts`);
|
||||
assert.deepEqual(
|
||||
offenders,
|
||||
[],
|
||||
`router mounts missing requireAuth:\n ${offenders.join('\n ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('every authenticated router mount enforces CSRF', () => {
|
||||
const offenders = mounts
|
||||
.filter((m) => !PUBLIC_ROUTERS.has(m.router))
|
||||
.filter((m) => !/csrfMiddleware/.test(m.middleware))
|
||||
.map((m) => `${m.path} → routes/${m.router}.cts`);
|
||||
assert.deepEqual(
|
||||
offenders,
|
||||
[],
|
||||
`authed router mounts missing csrfMiddleware:\n ${offenders.join('\n ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('admin router mounts require admin', () => {
|
||||
const adminMounts = mounts.filter(
|
||||
(m) => m.path.startsWith('/api/admin') || m.router === 'aboutAdmin' || m.router === 'status',
|
||||
);
|
||||
assert.ok(adminMounts.length >= 1, 'expected at least one admin mount');
|
||||
const offenders = adminMounts
|
||||
.filter((m) => !/requireAdmin/.test(m.middleware))
|
||||
.map((m) => `${m.path} → routes/${m.router}.cts`);
|
||||
assert.deepEqual(
|
||||
offenders,
|
||||
[],
|
||||
`admin mounts missing requireAdmin:\n ${offenders.join('\n ')}`,
|
||||
);
|
||||
});
|
||||
Loading…
Reference in New Issue