Expand production backlog: 19 → 39 tasks (DC-081–DC-100)
Deep audit additions:
P2.5 Security: route validation gap (151/160 unvalidated), cmd injection
surface in ca.js, 30 untested source files, no .dockerignore, Math.random IDs
P3.5 Ops: error codes, SDK/types, log rotation, license rate limit, Node
version pin, Dependabot, dependency health checks, workflow retry, audit trail
P4 Advanced: multi-user RBAC, API keys, Prometheus/Grafana, changelog,
migration system, service auto-discovery
This commit is contained in:
@@ -128,6 +128,120 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## P2.5 — Security Hardening (Deep Audit Findings)
|
||||||
|
|
||||||
|
### DC-081: 151 of 160 mutating routes have NO Joi input validation
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** P1-1 added Joi validation to 8 routes, but a scan shows **151 out of 160** POST/PUT/PATCH/DELETE routes still accept raw `req.body` without schema validation. That's 94% of the mutation surface unvalidated. Routes like `POST /api/v1/services/:id`, `PUT /api/v1/config`, `POST /api/v1/tailscale/*`, `POST /api/v1/health/config/:id` all accept arbitrary input. Fix: extend `src/utilities/validate.js` with schemas for every mutating route, wire them in. This is the single highest-impact security improvement. Effort: ~4 hr (batch by route file).
|
||||||
|
- **impact:** Input validation is the #1 defense against injection, abuse, and crashes. 94% gap is a P0 hiding as a P2.
|
||||||
|
|
||||||
|
### DC-082: Command injection surface in ca.js — 5 execSync calls with interpolation
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** `routes/ca.js` has 5 `execSync()` calls with template-string interpolation: lines 164, 175, 180, 203, 263. P0-2 fixed the password injection (`execFileSync`), but the remaining calls interpolate file paths and subjects (`${certFile}`, `${keyFile}`, `${subject}`, `${configFile}`). If any of these contain user input (e.g., a service name with `;` or backticks), it's command injection. Fix: convert ALL `execSync(\`...\`)` calls to `execFileSync('openssl', [...args])` with no shell interpolation. Also fix `src/docker/self-updater.js:717` (`execSync(\`tar xzf \"${tarballPath}\"...`)`) and `src/utilities/backup-manager.js:8` (imported execSync). Effort: ~2 hr.
|
||||||
|
- **impact:** Any execSync with interpolation is a potential RCE. This is the same class of bug P0-2 already fixed — finish the job.
|
||||||
|
|
||||||
|
### DC-083: 30 source files have zero test coverage
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** The test gap scan found 30 source files with NO corresponding test file, including critical paths: `license-manager.js` (534 lines, the entire revenue validation path), `config-schema.js`, `middleware.js` (the auth/rate-limit/CORS stack), `startup-validator.js`, all 7 DNS provider modules (`technitium.js`, `cloudflare.js`, `rfc2136.js`, `manual.js`, `base.js`, `registry.js`, `email.js`), `docker-maintenance.js`, `config/migrations.js`, `event-workers.js`, `keychain-manager.js`, `event-store.js`, `host-registry.js`. Fix: prioritize license-manager.js (revenue path) and middleware.js (security stack) first, then work through the rest. Effort: ~8 hr (can be done incrementally, 2-3 files per PR).
|
||||||
|
- **impact:** license-manager.js validates Pro licenses — an untested bug there could silently break activation for every paying customer.
|
||||||
|
|
||||||
|
### DC-084: No .dockerignore — test files and .git leak into Docker image
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** There is no `.dockerignore` file. The Docker build context includes `__tests__/` (hundreds of test files), any `.git/` directory, `coverage/`, `node_modules/` from the host, and markdown files. This bloats the image (currently 249MB) and can leak sensitive test fixtures. Fix: create `.dockerignore` with: `__tests__/`, `.git/`, `node_modules/`, `coverage/`, `*.md`, `.eslintrc.js`, `jest.config.js`, `npm-debug.log*`, `.env*`, `openapi.yaml` (only needed at build time if at all). Also add `.dockerignore` to the git repo. Effort: ~15 min.
|
||||||
|
- **impact:** Faster builds, smaller images, no test fixture leaks.
|
||||||
|
|
||||||
|
### DC-085: Math.random() used for security-sensitive IDs
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** `health-checker.js:352` generates incident IDs with `Math.random().toString(36)`. `resource-monitor.js:143` uses `Math.random()` for sampling. `rfc2136.js:120` generates temp filenames with `Math.random()`. While these aren't crypto-level secrets, `Math.random()` is not collision-resistant and is predictable. Fix: use `crypto.randomUUID()` for incident IDs, `crypto.randomBytes()` for temp filenames, and a simple counter for sampling. Effort: ~30 min.
|
||||||
|
- **impact:** Defense in depth. Predictable IDs can be exploited if they ever become user-facing.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## P3.5 — Operational Maturity
|
||||||
|
|
||||||
|
### DC-086: No structured error codes — errors are ad-hoc strings
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** The HTTP status code audit shows only 6 distinct status codes used across routes (200, 201, 400, 401, 404, 429). Error responses are plain strings like `"Invalid input"` or `"Unauthorized"`. There is no error code system (like `INVALID_CONFIG`, `SERVICE_NOT_FOUND`, `LICENSE_EXPIRED`). Fix: define a canonical error code enum in `src/utilities/errors.js`, return `{ error: { code: "SERVICE_NOT_FOUND", message: "..." } }` in all error responses. This makes API integration programmable (consumers switch on `code`, not parse `message`). Effort: ~3 hr.
|
||||||
|
- **impact:** API consumers can handle errors programmatically. Required for SDK generation and good DX.
|
||||||
|
|
||||||
|
### DC-087: No API client SDK / type definitions
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** There is no TypeScript definitions file (`.d.ts`) or client SDK. Anyone integrating against the API has to read the source code to understand request/response shapes. Fix: (1) Generate TypeScript types from the OpenAPI spec (once DC-062 updates it) using `openapi-typescript`. (2) Ship a `@dashcaddy/api-types` npm package or include a `types/index.d.ts` in the repo. (3) Optionally, a thin JS client wrapper. Effort: ~2 hr (after DC-062).
|
||||||
|
- **impact:** Developer adoption. A typed SDK lowers the barrier to integration.
|
||||||
|
|
||||||
|
### DC-088: No log rotation — error.log grows forever
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** The logger has basic rotation (rename to `.1` when it hits a size limit), but only keeps ONE rotated file. In production, error.log can grow rapidly during incident bursts. There's no retention policy, no compression, no date-based rotation. Fix: (1) Add a max-size threshold (e.g., 10MB) and keep N rotated files (e.g., 5). (2) Compress rotated files with gzip. (3) Add date-based naming so logs are greppable by date. (4) Add a `GET /api/v1/system/logs` endpoint so operators can view recent logs without SSH. Effort: ~1.5 hr.
|
||||||
|
- **impact:** Prevents disk fill during incident storms. Makes logs accessible without SSH access.
|
||||||
|
|
||||||
|
### DC-089: No rate limit on public license activation endpoint
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** The rate limiter `skip` list includes `req.path === '/api/v1/license/status'` and `req.path.startsWith('/api/v1/license/feature/')` — meaning license checks bypass rate limiting. While these are GET endpoints, the license *activation* endpoint (`POST /api/v1/license/activate`) should have its own dedicated rate limit to prevent brute-force license key guessing. Fix: add a dedicated `licenseLimiter` with tighter limits (e.g., 10 attempts per 15 min per IP) on POST /license/activate. Effort: ~30 min.
|
||||||
|
- **impact:** Prevents license key brute-forcing. Pro keys follow a predictable format (DC-XXX-XXXXX-XXXXXX) making them guessable without rate limiting.
|
||||||
|
|
||||||
|
### DC-090: Node.js version drift — Dockerfile says 20, host runs 22
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** Dockerfile uses `FROM node:20-alpine` (floating). The development machine runs Node v22.22.3. The container uses whatever `node:20-alpine` resolves to at build time. This version drift can cause "works on my machine" bugs (especially around `fetch()`, `crypto`, and `structuredClone` which changed between 20 and 22). Fix: (1) Pin the exact version: `FROM node:20.10.0-alpine3.19`. (2) Add `.nvmrc` or `engines` field to package.json specifying the minimum version. (3) Optionally upgrade to Node 22 across the board. Effort: ~30 min.
|
||||||
|
- **impact:** Reproducible builds. No surprise behavior from Node version drift.
|
||||||
|
|
||||||
|
### DC-091: No dependency update automation (Dependabot/Renovate)
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** Dependencies are updated manually. The 21 production dependencies and 4 dev dependencies can fall behind silently. There's no automated PR for security patches or major version bumps. Fix: add either GitHub Dependabot config (`.github/dependabot.yml`) or Renovate config (`renovate.json`). Schedule weekly checks. Group minor/patch updates into one PR. Keep major updates separate for review. Effort: ~30 min.
|
||||||
|
- **impact:** Security patches arrive automatically. No more manual `npm audit` sessions.
|
||||||
|
|
||||||
|
### DC-092: No health check for DashCaddy's own dependencies (disk space, memory)
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** The Dockerfile has a HEALTHCHECK that hits `/health`, but that endpoint only checks if the Express server responds. It doesn't check: disk space (if `/app/data` is on a full disk), memory pressure (Node heap near limit), Docker socket connectivity (if Docker daemon is down), Caddy admin API reachability. Fix: extend the health endpoint to include dependency checks: `{ diskSpace: { free: ..., total: ... }, memory: { heapUsed: ..., heapTotal: ..., rss: ... }, docker: { reachable: true/false }, caddy: { reachable: true/false } }`. Return 503 if any critical dependency is down. Effort: ~1.5 hr.
|
||||||
|
- **impact:** Catch systemic issues before they become outages. External monitoring can alert on `503`.
|
||||||
|
|
||||||
|
### DC-093: Workflow engine has no retry/backoff for failed actions
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** When the workflow engine's health-check action fails, it logs the failure and moves on — no retry. If a service is temporarily down and recovers in 30s, the workflow reports it as failed for the entire 15-min cycle. Fix: add configurable retry logic to workflow actions (e.g., retry 2 times with 30s backoff before reporting failure). Also add a `maxRetries` config to the health-check workflow. Effort: ~1.5 hr.
|
||||||
|
- **impact:** Fewer false-positive alerts. More resilient monitoring.
|
||||||
|
|
||||||
|
### DC-094: No audit trail for config changes (who changed what, when)
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** The audit logger (`src/security/audit-logger.js`) captures POST/PUT/DELETE events, but config changes (services.json, health-config.json, config.json) are made via file writes, not API calls. There's no record of who changed a service URL, disabled a health check, or modified a workflow. Fix: (1) Route all config mutations through API endpoints that log to the audit trail. (2) Add a `GET /api/v1/system/audit-log` endpoint for viewing the trail. (3) Include a diff of what changed in each audit entry. Effort: ~2 hr.
|
||||||
|
- **impact:** Accountability. When something breaks, you can trace who changed the config and when.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## P4 — Advanced Features
|
||||||
|
|
||||||
|
### DC-095: No multi-user support — single-admin only
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** DashCaddy has one admin user. For teams or homelab groups, there's no way to add a second admin or a read-only viewer. Fix: (1) Add a `users.json` with role-based access (admin, editor, viewer). (2) Add user management endpoints. (3) Add per-service permissions (editor can manage services but not billing). This is a significant feature, not a quick fix. Effort: ~6 hr.
|
||||||
|
- **impact:** Multi-admin is a requirement for team/enterprise adoption.
|
||||||
|
|
||||||
|
### DC-096: No API key management (create/revoke/scoped keys)
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** API authentication uses session cookies or TOTP. There's no way to create scoped API keys for automation (e.g., a read-only key for monitoring, a key that can only manage one service). Fix: add `POST /api/v1/api-keys` (create with scopes), `GET /api/v1/api-keys` (list), `DELETE /api/v1/api-keys/:id` (revoke). Store hashed in credentials.json. Effort: ~2 hr.
|
||||||
|
- **impact:** Enables automation and third-party integrations without sharing the admin password.
|
||||||
|
|
||||||
|
### DC-097: No Prometheus / Grafana metrics export
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** There's a basic `/metrics` endpoint, but it returns JSON, not Prometheus format. Fix: (1) Add `prom-client` dependency. (2) Instrument key metrics: HTTP request duration histogram, active WebSocket connections, health check pass/fail counter, container count gauge, API error rate. (3) Expose `GET /metrics` in Prometheus exposition format alongside the existing JSON endpoint. (4) Ship a Grafana dashboard JSON as a reference. Effort: ~2 hr.
|
||||||
|
- **impact:** Industry-standard observability. Drop-in Grafana dashboard for operators.
|
||||||
|
|
||||||
|
### DC-098: No changelog / release notes generation
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** Releases are tracked via git commits and VERSION file, but there's no user-facing changelog. For a public product, customers need to know what changed between versions. Fix: (1) Add a `CHANGELOG.md` following Keep a Changelog format. (2) Auto-generate from conventional commits (if adopted) or git log. (3) Display "What's new" on the dashboard after updates. Effort: ~1.5 hr.
|
||||||
|
- **impact:** Customer trust. Users won't update without knowing what changed.
|
||||||
|
|
||||||
|
### DC-099: No automated database migration system
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** Config migrations exist (`src/config/migrations.js`) but are ad-hoc. As the data schema evolves (new fields in services.json, config.json), there's no versioned migration system. Fix: (1) Add a `schemaVersion` field to config files. (2) Create a migration runner that applies migrations sequentially on startup. (3) Log each migration. (4) Support rollback on failure. Effort: ~2 hr.
|
||||||
|
- **impact:** Safe upgrades. No more manual config patching after updates.
|
||||||
|
|
||||||
|
### DC-100: No service discovery / auto-detect running containers
|
||||||
|
- **status:** pending
|
||||||
|
- **details:** Services are added manually by specifying URLs. DashCaddy doesn't auto-detect running Docker containers and suggest adding them as services. Fix: (1) Scan `docker ps` for containers with exposed ports. (2) Match against known app templates (Plex, Sonarr, etc.). (3) Show a "Detected services" panel with one-click add. (4) Periodically re-scan for new containers. Effort: ~3 hr.
|
||||||
|
- **impact:** Zero-config onboarding. New users see their services auto-discovered.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Summary by Priority
|
## Summary by Priority
|
||||||
|
|
||||||
| Priority | Count | Effort | Theme |
|
| Priority | Count | Effort | Theme |
|
||||||
@@ -135,5 +249,8 @@
|
|||||||
| P0 | 3 (DC-062–064) | ~7 hr | Public release blockers |
|
| P0 | 3 (DC-062–064) | ~7 hr | Public release blockers |
|
||||||
| P1 | 5 (DC-065–069) | ~7 hr | Reliability & code quality |
|
| P1 | 5 (DC-065–069) | ~7 hr | Reliability & code quality |
|
||||||
| P2 | 6 (DC-070–075) | ~5.5 hr | Polish & DX |
|
| P2 | 6 (DC-070–075) | ~5.5 hr | Polish & DX |
|
||||||
|
| P2.5 | 5 (DC-081–085) | ~15 hr | Security hardening (deep audit) |
|
||||||
| P3 | 5 (DC-076–080) | ~16 hr | Future growth |
|
| P3 | 5 (DC-076–080) | ~16 hr | Future growth |
|
||||||
| **Total** | **19** | **~35.5 hr** | |
|
| P3.5 | 9 (DC-086–094) | ~14.5 hr | Operational maturity |
|
||||||
|
| P4 | 6 (DC-095–100) | ~16.5 hr | Advanced features |
|
||||||
|
| **Total** | **39** | **~81.5 hr** | |
|
||||||
|
|||||||
Reference in New Issue
Block a user