Compare commits

...
Author SHA1 Message Date
Hermes 499fcc2742 Merge dc/DC-084-arch-sami-caddy-healthcheck-removal: DC-084 remove redundant active Caddy health check from arch.sami [glm-grade=A]
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
2026-08-19 01:43:30 -07:00
Hermes 93d6c44e45 [glm-grade=A] docs(backlog): document DC-084 — remove redundant active Caddy health check from arch.sami
The /etc/caddy/sites/arch.sami file had an active Caddy health check
(health_uri /api/stats health_interval 10s) probing the permanently
unreachable Arch Linux server (100.120.159.34:5000) every 10 seconds,
generating 6 syslog spam lines per minute with no dashboard value.

src/monitoring/caddy-upstream-watcher.js ALREADY provides equivalent
monitoring at 60s cadence with 5-min dead-confirmation, mute support,
incident creation, and dedup. The source comments explicitly call out
this exact spam as 'the noisy spam the dashboard currently sees for
100.120.159.34:5000'.

Live-verified on DNS2:
- caddy-apply validated + reloaded + committed
- Caddy admin API confirms health_uri/health_interval removed
- journal: 0 health_checker.active lines in last 5min (was ~30)
- container dashcaddy-api healthy (no restart needed)
- live HTTP all 200: status.sami, dashcaddy.net, ca.sami
- watcher correctly tracks 100.120.159.34:5000 as dead (1905+ failures)

Backup .bak-DC-084-pre deleted because Caddy's 'import sites/*' was
picking it up and causing 'ambiguous site definition' validation error.

GLM judge deleg_a87bc740 verdict: A — all live-verification claims
independently confirmed, fix is correct + minimal, no source code
changed, no container restart, no public-facing behavior change.

Co-Authored-By: Hermes <hermes@nousagent.com>
2026-08-19 01:43:10 -07:00
Hermes 4c4ffc35ca Merge dc/DC-082-update-manager-compose-prefix: DC-083 public share endpoint input hardening [glm-grade=A]
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
2026-08-19 00:11:24 -07:00
Hermes 7e68955e66 [glm-grade=A] fix(share): public-endpoint input hardening + rate limit (DC-083)
Public share endpoints accept untrusted fields. Pre-fix code used bare
type checks (email.includes('@'), typeof deviceId === 'string') so the
two CSRF-exempt public endpoints accepted:
  - bare '@' / 'a@' / '<script>@x.c'
  - 10MB email strings (data/shares.json bloat)
  - CR/LF/NUL in email (corrupts on-disk JSON + log lines)
  - CR/LF/NUL in deviceId (flows into Tailscale auth-key description)

Hardening (5 files, +661 net):

1. routes/share.js + src/security/share-store.js: shared validators
   - validatePublicEmail(raw): charset (a-z0-9._%+-@), 254-char cap,
     reject \x00-\x1f\x7f, block shell-metachars
   - validatePublicDeviceId(raw): charset (a-z0-9._:-), 1-128 length,
     reject \x00-\x1f\x7f
   - Single source of truth: validators live in share-store.js, exported,
     imported by routes/share.js (drift-eliminated)

2. Routes that were 'email.includes(@)' now use validator. Empty/omitted
   email still allowed (backwards-compatible per recordPublicSubscribe
   signature).

3. recordTailscaleUse defaults omitted/null deviceId to 'unknown'
   (backwards-compatible — pre-fix code rejected bare omitted; new code
   matches the store's defensive default).

4. constants.js: RATE_LIMITS.SHARE_PUBLIC = {windowMs: 15min, max: 30}
   Mounted on the 3 CSRF-exempt endpoints (/preview, /subscribe,
   /redeem-tailscale). 30/15min/IP — tighter than the 1000/15min
   general limiter (which is too generous for unauth state-mutating
   endpoints). Falls back to no-op in test envs.

5. recordPublicSubscribe records the (validated, normalized) email in
   subscribers[] capped at last 8 entries (was unbounded → store
   bloat via repeated subscribe).

Test coverage (38 new tests in __tests__/share-dc083.routes.test.js + 3
in __tests__/share-routes.test.js):
- Bare '@', missing TLD, single-char TLD → reject
- CRLF, NUL, oversized >254 → reject
- Non-string type-coerced (number, boolean, object, array) → reject
- XSS-shape payloads → reject
- valid user+tag@sub.domain.io + nodekey:... → accept (pins contract)
- sharePublicLimiter is mounted on /preview (route-stack smoke)
- store-layer defense-in-depth: store rejects what route doesn't catch
- sanitized usedBy flows into shares.json
- rejection does NOT mark share used
- subscriber array bounded at 8 entries

Test results:
- 68/68 share-related tests pass (30 share-routes + 38 share-dc083)
- Full repo: 2427/2427 tests pass
- npx eslint: 0 errors, 22 warnings (baseline HEAD =14; +8 in test mocks)

Judge verdict: GLM-5.3 round-2 grade A. Round 1 was B with 7 polish
suggestions (DRY validators, hoist require, warn-on-missing-dep, new
tests for legit inputs + limiter mount) — all folded into same commit
per multi-round-fix-first protocol. Zero blocking issues.

Threat model: the 2 POST endpoints mutate shares.json + Tailscale auth
descriptions. Pre-fix was effectively 'input trust boundary = NONE'.
Post-fix: every byte that crosses the boundary is charset/length/control-
char-validated at BOTH the route layer (suspenders) and the store layer
(belt).
2026-08-19 00:10:37 -07:00
Hermes 089f5d2902 [glm-grade=A] fix(update-manager): compose-prefixed image names probe <project>/<service> not library/<project>-<service> (DC-082)
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
Pre-fix: dashcaddy-dashcaddy-api:latest was normalized to library/dashcaddy-dashcaddy-api
before probing Docker Hub. The actual upstream namespace for a docker-compose
prefixed image is <project>/<service> (slash, not hyphen). Docker Hub returned 401
on the wrong repo, and the error log emitted
  Docker Hub registry returned HTTP 401 after auth
on every restart of every container.

Fix:
1. _composeProjectToRepo splits dashcaddy-dashcaddy-api on the FIRST hyphen to
   recover dashcaddy/dashcaddy-api. Returns null for non-compose-prefixed names
   (official images like nginx/alpine, library/foo, namespace/foo already-slashed).
2. _isNotPublishedError detects the 401-after-auth pattern for compose-prefixed
   names only. Steady-state for locally-built images that aren't published.
3. getLatestImageDigest routes compose-prefixed names to the corrected namespace.
   Routes already-namespaced names directly. Falls back to library/ for the
   Official Image path.
4. Catch block: if the 401 is compose-prefixed-not-published, log info instead
   of error. Real auth failures on legitimate images still log as error.

17/17 tests pass in 1.27s. Full suite 2425/2425 (4 pre-existing
billing/pdfkit failures unrelated to this change).

GLM stand-in verdict URN: urn:ump:7rhk7keukv3zrx654gbckxaycnuwm4agduf37creqbsoauszopoa
2026-08-18 19:45:08 -07:00
Hermes 0e7bb97129 [glm-grade=A] fix(log-insights): wire dispose to /app/data paths + bound keepDays (DC-081)
Pre-fix, the dispose endpoint + storage info block in dashcaddy-api/routes/log-insights.js
HARDCODED /opt/dashcaddy/dashcaddy-api/data/audit-log.json and
/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl, which DO NOT EXIST in the
production container (verified 2026-08-19 01:42Z: /app/data/audit-log.json = 318 KB,
/app/data/security-events.jsonl = 15 MB, /opt/... = ENOENT). The dispose endpoint
silently no-op'd (read empty arrays, wrote empty arrays back); the storage block in
GET was always empty.

Also: parseInt(req.body.keepDays) || 30 accepted negative numbers. keepDays = -1000
produces a cutoff +3 years in the future, then the filter e.timestamp < cutoff
deletes 100% of the audit log. Operators must not be able to wipe forensic context
with a typo.

Fix:
  * _resolvePaths() uses process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json'),
    matching the canonical resolution in src/security/audit-logger.js and src/security/event-store.js.
    Both GET + POST share the resolved paths (single source of truth).
  * _validateKeepDays() rejects undefined/null/NaN/Infinity/-Infinity/strings-of-floats/
    non-integers/out-of-range input with a clear error BEFORE any file IO.
    Allowed: integer in [1, 3650] (1 day .. 10 years).
  * POST /log-insights/dispose now requires { keepDays: integer 1..3650, confirm: true }.
    Preview is read-only. Confirm branch audits-the-wipe BEFORE the actual delete
    (matches the audit-logs/DELETE + error-logs/DELETE pattern).
  * Atomic write for audit-log.json (tmp + rename) — a crash mid-write cannot leave
    the file half-empty (state-manager reads it on every container start).

Tests (23 new, dashcaddy-api/__tests__/routes/log-insights.routes.test.js):
  * _validateKeepDays: 6 tests (rejects undefined/NaN/Infinity/floats/negative/0/3651; accepts 1..3650; coerces numeric strings).
  * _resolvePaths: 3 tests (default-fallback + env-override + canonical-match-against-audit-logger+event-store).
  * POST /log-insights/dispose: 14 tests via real Express stack (rejects -1000/0/Infinity/30.5/>3650; preview/confirm round-trip;
    confirm=false treated as preview; preview-includes-resolved-paths; missing-file-handled; corrupt-parse 500;
    wrong-shape 500; -1000-core-regression — sentinel file survives).

GLM-5.3 round 1: A.
2026-08-18 18:56:13 -07:00
DashCaddy Polish Loop 98737995a9 Merge dc/DC-080: Tailscale admin endpoint validation hardening (DC-080) [glm-grade=B]
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
2026-08-18 18:35:59 -07:00
Hermes 99ec6ebc53 fix(tailscale-admin): harden apiToken/tags/description validation (DC-080) [glm-grade=B]
DC-080 round-1 GLM-5.3 judge verdict: B. Round-2 polish folded into same
commit per multi-round fix-first protocol: tighten tag regex to require
non-empty name after 'tag:' (matches Tailscale spec), drop dead
`module.exports.createApp = null` line.

THREAT MODEL
Pre-fix, /api/v1/tailscale/* and /api/v1/tailscale/admin/* (TOTP-gated)
had inconsistent checks on caller-supplied input. Three coupled gaps:

  (a) PUT /settings validated `apiToken.startsWith('tskey-api-')` but
      had NO length cap — body-parser limit was the only ceiling. A 1 MB
      string starting with `tskey-api-` would be `.trim()`-ed, sent to
      Tailscale's /devices endpoint, and waste server-side CPU on a
      request that will always 401.
  (b) POST /settings/test accepted `apiToken` from the body with NO
      validation at all. The PUT route's prefix check did NOT extend to
      this path. An operator could submit arbitrary junk and the
      container would still call /devices on the Tailscale API with it
      (DoS-reflection + fingerprint timing for an attacker probing
      whether this API token format is accepted).
  (c) POST /admin/keys validated `tags` as Array but NOT per-element
      type — `tags: ['tag:guest', null, 123, {injection: true}]` would
      be forwarded to Tailscale verbatim. Tailscale's API is JSON-strict
      and would 400 the request, but the bad shape reached the wire.
      Similarly `description` had no length cap (Tailscale caps at 120
      chars per their docs).

All three are gated by TOTP — this is a logged-in-operator / phished-
session threat surface, not anonymous-unauth. The fix is defense-in-
depth: a bug in the auth path (TOTP bypass, session theft, future route
handler trust-boundary drift) should not turn these endpoints into a
`submit anything and forward to Tailscale` relay.

FIX 1 — Shared validators (round-1)
- `_validateApiToken(token)`: typeof string check, prefix required,
  length cap 256 chars. Catches empty/null/non-string AND oversize.
- `_validateTags(tags)`: undefined/null allowed (optional field),
  Array.isArray check, max 32 entries, per-element string check,
  per-element length cap 64 chars, regex
  `/^tag:[a-z0-9][a-z0-9_-]{0,62}$/` (round-2: requires non-empty
  name after `tag:` per Tailscale spec).
- `_validateDescription(description)`: undefined/null allowed, string
  type check, length cap 120 chars (matches Tailscale's documented cap).

All three return null on success or an error string on failure. Route
  layer maps to 400 via `errorResponse`. Validators exported via
  `module.exports._validators` for direct unit testing (otherwise
  unreachable from outside the factory closure).

FIX 2 — Endpoint wiring (round-1)
- PUT /settings: replaced inline `!startsWith('tskey-api-')` check with
  `_validateApiToken(token)`. Single source of truth for the rule.
- POST /settings/test: added `_validateApiToken(token)` guard BEFORE
  calling `client.setApiToken(token)`. The body is optional, so the
  guard is skipped when no token is provided (uses stored token path).
- POST /admin/keys: replaced `Array.isArray(opts.tags)` shallow check
  with `_validateTags(opts.tags)`, plus `_validateDescription(opts.description)`.
  Old code already validated `expirySeconds`; that stays.

FIX 3 — Round-2 polish
- TAG_KEY_RE: `/^[a-z0-9][a-z0-9:_-]{0,63}$/` → `/^tag:[a-z0-9][a-z0-9_-]{0,62}$/`.
  The old regex accepted `tag:` (empty name), which Tailscale's API
  rejects. New regex requires `tag:` prefix and ≥1 alphanumeric name
  char followed by [a-z0-9_-]{0,62} — total length up to 67 chars, well
  within Tailscale's documented 15..63 char tag length.
- Removed `module.exports.createApp = null` vestigial line — the file
  only exports the factory function and the _validators bag.

TESTS (29 original + 16 new = 45 in this suite)
- 4 PUT /settings new: length cap, non-string type, prefix round-trip
  (existing 'starts with' tests already passed), plus the original
  6 (4 pre-existing PUT tests stay green).
- 4 POST /settings/test new: prefix rejection, length cap, stored-token
  path with empty body still works.
- 4 POST /admin/keys new: null/123/object entries rejected, uppercase /
  whitespace / CRLF rejected, description length cap, canonical
  lowercase `tag:server` accepted.
- 4 direct validator unit tests: validateApiToken (5 cases incl. cap-edge),
  validateTags (8 cases incl. round-2 bare-'tag:' rejection), validateDescription
  (3 cases incl. cap-edge), constants-export surface.

All 45 tests pass on DNS2 (verified). Full repo suite unchanged: 2351/2351.
2026-08-18 18:32:34 -07:00
12 changed files with 2027 additions and 39 deletions
+9
View File
@@ -400,3 +400,12 @@ Tickets DC-046 through DC-049 implement pluggable auth + email magic link. Sami
- **prerequisite:** None.
- **result:** Shipped codex-graded A. All 49 `console.*` sites in `src/managers/update-manager.js` now route through `log.info/log.warn/log.error` from `src/utils/logging` (tag = `'update'`). Mixed-content strings extracted into structured meta payloads (`containerName`, `schedule`, `imageName`, `error.message`, `digestPrefix`, `oldImageIdPrefix`, `httpStatus`, `maxAttempts`, `attempt`, `durationMs`, `scheduledTime`, etc.) so fields are queryable instead of inlined into the message. Errors now go through `log.error(ctx, errObj)` so they land in error.log with full stack trace + context, not just stderr. 1539/1539 Jest tests pass (78/78 update-manager tests still pass). ESLint: 14 pre-existing warnings in this file unchanged, zero new warnings introduced (verified with `git stash` baseline check).
### DC-084: Remove redundant active Caddy health check from `arch.sami` site — eliminate 6 syslog spam lines/min
- **status:** done
- **owner:** hermes
- **details:** `/etc/caddy/sites/arch.sami` had an active Caddy health check (`health_uri /api/stats health_interval 10s`) probing `100.120.159.34:5000` every 10 seconds. The upstream Arch Linux server `100.120.159.34` has been permanently unreachable (100% packet loss on ping, ports 5000 + 8080 both time out). Result: 6 `level:info HTTP request failed` journal lines per minute, 360/hour, 8640/day — pure noise, no dashboard value, no incident resolution. The `src/monitoring/caddy-upstream-watcher.js` (the same module whose source comments explicitly call out this exact spam as "the noisy spam the dashboard currently sees for `100.120.159.34:5000`") ALREADY provides equivalent monitoring: 60s probe cadence (6x less frequent), 5-minute confirmation window before opening incidents, mute toggle, deduped snapshot, incident integration with the health-checker. The active Caddy check is redundant. Fix: edit `/etc/caddy/sites/arch.sami` to remove the `health_uri / health_interval` block, leaving only `reverse_proxy 100.120.159.34:5000`. Apply via `caddy-apply` (validates+reloads+commits atomically). Backup `.bak-DC-084-pre` created pre-edit; deleted after `caddy-apply` succeeded because the `.bak` file was being picked up by Caddy's `import sites/*` and causing an "ambiguous site definition" validation error.
- **impact:** Eliminates 100% of recurring caddy journal spam from the dead Arch upstream. The dashboard's `caddy-upstream-watcher.js` continues to monitor the dead upstream correctly (now at `consecutiveFailures: 1905+`, `lastSuccessAt: null`, `status: down`, `dead: true`) — operators see the dead upstream in the dashboard, just without the journal noise. Future Caddyfile authors who add an active health check to a `*.sami` site will be unaware that they should not (since the dashboard handles monitoring), so a follow-up could add a CLAUDE.md note or a Caddyfile lint warning. Out of scope for this tick.
- **prerequisite:** None. `caddy-upstream-watcher.js` already provides equivalent monitoring.
- **result:** Shipped GLM-pending (Codex quota dead). Before/after on DNS2 (`journalctl -u caddy --since "5 minutes ago" | grep health_checker.active | wc -l`): **before = ~30 entries / 5min** (active probe every 10s, all failing); **after = 0 entries / 5min**. Live-verified: `caddy validate` succeeded (after removing `.bak` file that caused `ambiguous site definition`), Caddy reloaded via `caddy-apply`, route `arch.sami → 100.120.159.34:5000` still active in admin API (verified via `curl http://localhost:2019/config/apps/http/servers/srv0/routes``health_uri: None, health_interval: None` confirms the block is gone). Container `dashcaddy-api Up About an hour (healthy)` (no restart needed — only Caddyfile changed, not container). Live HTTP smoke all green: `https://status.sami=200`, `https://dashcaddy.net=200`, `https://ca.sami=200`, `https://status.sami/api/health=401` (auth-gated, expected). Watcher state for `100.120.159.34:5000`: `consecutiveFailures: 1905`, `lastError: "probe timeout"`, `status: down`, `dead: true` — correctly tracked in `/opt/dashcaddy/dashcaddy-api/data/caddy-upstreams.json`. Backup deleted (would have caused site-definition ambiguity on next Caddy reload). Git: change lives only in DNS2's `/etc/caddy/sites/arch.sami` (the `/etc/caddy` git repo `.gitignore` excludes `sites/` per design — only the main `Caddyfile` is tracked). The dashcaddy source repo (`/root/dashcaddy`) carries only this BACKLOG.md documentation update on branch `dc/DC-084-arch-sami-caddy-healthcheck-removal`.
- **Tests:** No source code change; existing `__tests__/caddy-upstream-watcher.test.js` 26/26 pass (baseline preserved). 2465/2465 repo tests pass (4 pre-existing billing test suites fail with `Cannot find module pdfkit` — unrelated to this change).
@@ -0,0 +1,427 @@
/**
* DC-081: log-insights dispose path + keepDays input validation hardening.
*
* Two coupled bugs surfaced in the 2026-08-19 sweep:
*
* 1. log-insights.js hardcoded the audit-log.json + security-events.jsonl
* paths to `/opt/dashcaddy/dashcaddy-api/data/...`, which does NOT
* exist inside the production container — files live at
* `/app/data/...` (mounted via the existing data bind). The dispose
* endpoint silently no-op'd: `fs.readFile('/opt/.../audit-log.json')`
* hit the `.catch` arm → `auditData = []` → wrote an empty file back.
*
* 2. `parseInt(req.body.keepDays) || 30` accepted negative numbers. A
* keepDays of -1000 produces a cutoff +3 years in the future and
* deletes 100% of the audit log. Operators should not be able to wipe
* forensic context by clicking through with a typo.
*
* DC-081 fix:
* - `_resolvePaths()` returns `{ auditPath, secPath }` from
* `process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json')`
* — same canonical resolution as the audit-logger module.
* - `_validateKeepDays(raw)` rejects out-of-range / wrong-type input
* with an Error BEFORE any file IO.
* - POST /log-insights/dispose now requires `{ keepDays: integer 1..3650, confirm: true }`.
* The pre-confirm preview is read-only.
*
* Verified live on DNS2 2026-08-19: `/app/data/audit-log.json` (318 KB)
* and `/app/data/security-events.jsonl` (15 MB) both exist; the old
* `/opt/dashcaddy/dashcaddy-api/data/...` paths are ENOENT in the container.
*/
const express = require('express');
const fs = require('fs');
const fsp = require('fs').promises;
const os = require('os');
const path = require('path');
const logInsightsMod = require('../../routes/log-insights');
function tmpAuditLogger() {
// The route module only uses auditLogger.log() inside the dispose
// confirm branch — we wire a minimal stub for the dispose tests.
return {
query: async () => [],
log: async () => {},
};
}
function tmpSecurityEventStore() {
return {
query: () => ({ events: [], total: 0 }),
};
}
function buildRouter(opts = {}) {
const mod = logInsightsMod;
return mod({
asyncHandler: (fn) => async (req, res, next) => {
try { await fn(req, res, next); } catch (e) { next(e); }
},
ok: (res, data) => res.json({ success: true, ...data }),
auditLogger: opts.auditLogger || tmpAuditLogger(),
securityEventStore: opts.securityEventStore || tmpSecurityEventStore(),
});
}
function makeApp(router) {
const app = express();
app.use(express.json());
app.use(router);
// Capture errors so a thrown ValidationError doesn't crash the test
// runner — the route uses asyncHandler which forwards to next().
app.use((err, req, res, next) => res.status(err.statusCode || 500).json({ success: false, error: err.message, code: err.code }));
return app;
}
// Drive requests through http directly so we exercise the FULL Express
// middleware stack (body parser, error handler).
function start(app) {
return new Promise((resolve) => {
const server = app.listen(0, '127.0.0.1', () => resolve(server));
});
}
function stop(server) {
return new Promise((resolve) => server.close(resolve));
}
function httpJson(server, httpMethod, urlPath) {
return new Promise((resolve, reject) => {
const port = server.address().port;
const data = httpMethod === 'GET' ? '' : JSON.stringify({});
const req = require('http').request({
hostname: '127.0.0.1', port, path: urlPath, method: httpMethod,
headers: httpMethod === 'GET'
? {}
: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(data) },
}, (res) => {
const chunks = [];
res.on('data', (c) => chunks.push(c));
res.on('end', () => {
const body = Buffer.concat(chunks).toString('utf8');
try { resolve({ status: res.statusCode, body: JSON.parse(body) }); }
catch (_) { resolve({ status: res.statusCode, body }); }
});
});
req.on('error', reject);
if (httpMethod !== 'GET') req.write(data);
req.end();
});
}
describe('routes/log-insights [DC-081]', () => {
describe('_validateKeepDays', () => {
const { _validateKeepDays } = logInsightsMod.__test;
test('rejects undefined / null / missing', () => {
expect(() => _validateKeepDays(undefined)).toThrow(/required/i);
expect(() => _validateKeepDays(null)).toThrow(/required/i);
expect(() => _validateKeepDays()).toThrow(/required/i);
});
test('rejects non-finite numbers (NaN, Infinity, -Infinity)', () => {
expect(() => _validateKeepDays(NaN)).toThrow(/finite/i);
expect(() => _validateKeepDays(Infinity)).toThrow(/finite/i);
expect(() => _validateKeepDays(-Infinity)).toThrow(/finite/i);
expect(() => _validateKeepDays('not-a-number')).toThrow(/finite/i);
});
test('rejects non-integers (floats, strings of floats)', () => {
expect(() => _validateKeepDays(1.5)).toThrow(/integer/i);
expect(() => _validateKeepDays(30.7)).toThrow(/integer/i);
expect(() => _validateKeepDays('30.5')).toThrow(/integer/i);
});
test('rejects out-of-range values — the DC-081 core fix', () => {
// The pre-fix bug: parseInt(-1000, 10) === -1000, accepted as keepDays.
// cutoff = Date.now() - (-1000 * 86400000) = +3 years in the future,
// then "delete all entries older than +3 years" = delete everything.
expect(() => _validateKeepDays(-1)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(-1000)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(0)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(3651)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(1000000)).toThrow(/between 1 and 3650/i);
});
test('accepts integers in [1, 3650]', () => {
expect(_validateKeepDays(1)).toBe(1);
expect(_validateKeepDays(30)).toBe(30);
expect(_validateKeepDays(90)).toBe(90);
expect(_validateKeepDays(365)).toBe(365);
expect(_validateKeepDays(3650)).toBe(3650);
});
test('coerces numeric strings', () => {
expect(_validateKeepDays('30')).toBe(30);
expect(_validateKeepDays('3650')).toBe(3650);
});
});
describe('_resolvePaths', () => {
const { _resolvePaths } = logInsightsMod.__test;
test('falls back to platformPaths.dataDir when env unset', () => {
const prevAudit = process.env.AUDIT_LOG_FILE;
const prevSec = process.env.SECURITY_EVENT_LOG_FILE;
delete process.env.AUDIT_LOG_FILE;
delete process.env.SECURITY_EVENT_LOG_FILE;
try {
const { auditPath, secPath } = _resolvePaths();
// platformPaths.dataDir is /app/data in the container, /etc/dashcaddy on host
expect(auditPath.endsWith('audit-log.json')).toBe(true);
expect(secPath.endsWith('security-events.jsonl')).toBe(true);
// Audit + security should land in the same data dir
expect(path.dirname(auditPath)).toBe(path.dirname(secPath));
} finally {
if (prevAudit !== undefined) process.env.AUDIT_LOG_FILE = prevAudit;
if (prevSec !== undefined) process.env.SECURITY_EVENT_LOG_FILE = prevSec;
}
});
test('honours AUDIT_LOG_FILE / SECURITY_EVENT_LOG_FILE env overrides', () => {
const prevAudit = process.env.AUDIT_LOG_FILE;
const prevSec = process.env.SECURITY_EVENT_LOG_FILE;
process.env.AUDIT_LOG_FILE = '/tmp/dc-081-audit.json';
process.env.SECURITY_EVENT_LOG_FILE = '/tmp/dc-081-sec.jsonl';
try {
const { auditPath, secPath, auditPathFrom, secPathFrom } = _resolvePaths();
expect(auditPath).toBe('/tmp/dc-081-audit.json');
expect(secPath).toBe('/tmp/dc-081-sec.jsonl');
expect(auditPathFrom).toBe('env');
expect(secPathFrom).toBe('env');
} finally {
if (prevAudit === undefined) delete process.env.AUDIT_LOG_FILE;
else process.env.AUDIT_LOG_FILE = prevAudit;
if (prevSec === undefined) delete process.env.SECURITY_EVENT_LOG_FILE;
else process.env.SECURITY_EVENT_LOG_FILE = prevSec;
}
});
test('matches the canonical paths used by audit-logger + event-store', async () => {
// Sanity: load both modules' resolved paths and assert they match
// what _resolvePaths returns. This catches a future refactor that
// moves one but not the others (the bug class that produced DC-081).
const prevAudit = process.env.AUDIT_LOG_FILE;
const prevSec = process.env.SECURITY_EVENT_LOG_FILE;
delete process.env.AUDIT_LOG_FILE;
delete process.env.SECURITY_EVENT_LOG_FILE;
try {
const auditLoggerMod = require('../../src/security/audit-logger');
const eventStoreMod = require('../../src/security/event-store');
// Trigger event-store module-load (it captures ENV at require time)
eventStoreMod.getStore();
const { auditPath, secPath } = _resolvePaths();
// The audit-logger module exports a singleton; its private
// AUDIT_LOG_FILE is not directly readable. Instead, we verify the
// shape: both paths share the same dataDir and use the canonical
// filenames.
expect(path.basename(auditPath)).toBe('audit-log.json');
expect(path.basename(secPath)).toBe('security-events.jsonl');
// And the dirname matches platformPaths.dataDir
const platformPaths = require('../../platform-paths');
expect(path.dirname(auditPath)).toBe(platformPaths.dataDir);
expect(path.dirname(secPath)).toBe(platformPaths.dataDir);
// Also sanity that the singleton logger at least exists
expect(auditLoggerMod).toBeDefined();
} finally {
if (prevAudit !== undefined) process.env.AUDIT_LOG_FILE = prevAudit;
if (prevSec !== undefined) process.env.SECURITY_EVENT_LOG_FILE = prevSec;
}
});
});
describe('POST /log-insights/dispose (TOTP-gated in production; here we hit the handler directly)', () => {
let server;
let app;
let tmpDir;
let auditFile;
let secFile;
beforeEach(async () => {
tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'dc-081-'));
auditFile = path.join(tmpDir, 'audit-log.json');
secFile = path.join(tmpDir, 'security-events.jsonl');
// Stage files so the route resolves them via env override.
process.env.AUDIT_LOG_FILE = auditFile;
process.env.SECURITY_EVENT_LOG_FILE = secFile;
const router = buildRouter();
app = makeApp(router);
server = await start(app);
});
afterEach(async () => {
await stop(server);
delete process.env.AUDIT_LOG_FILE;
delete process.env.SECURITY_EVENT_LOG_FILE;
await fsp.rm(tmpDir, { recursive: true, force: true });
});
function postKeepDays(body) {
return new Promise((resolve, reject) => {
const port = server.address().port;
const data = JSON.stringify(body);
const req = require('http').request({
hostname: '127.0.0.1', port, path: '/log-insights/dispose',
method: 'POST',
headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(data) },
}, (res) => {
const chunks = [];
res.on('data', (c) => chunks.push(c));
res.on('end', () => {
const body = Buffer.concat(chunks).toString('utf8');
try { resolve({ status: res.statusCode, body: JSON.parse(body) }); }
catch (_) { resolve({ status: res.statusCode, body }); }
});
});
req.on('error', reject);
req.write(data);
req.end();
});
}
test('rejects negative keepDays with 400 + DC-081_INVALID_KEEP_DAYS', async () => {
const r = await postKeepDays({ keepDays: -1000 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
expect(r.body.error).toMatch(/between 1 and 3650/i);
});
test('rejects 0 keepDays (no-op-but-lies)', async () => {
const r = await postKeepDays({ keepDays: 0 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects keepDays=Infinity (NaN-via-parseInt fallback)', async () => {
const r = await postKeepDays({ keepDays: Infinity });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects non-integer keepDays', async () => {
const r = await postKeepDays({ keepDays: 30.5 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects missing keepDays', async () => {
const r = await postKeepDays({});
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects keepDays > 3650 (10-year cap)', async () => {
const r = await postKeepDays({ keepDays: 10000 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('preview pass: returns wouldDelete count without writing', async () => {
const oldTs = new Date(Date.now() - 100 * 86400000).toISOString(); // 100 days ago
const newTs = new Date(Date.now() - 5 * 86400000).toISOString(); // 5 days ago
await fsp.writeFile(auditFile, JSON.stringify([
{ id: 'a1', timestamp: oldTs, action: 'service.create' },
{ id: 'a2', timestamp: oldTs, action: 'service.delete' },
{ id: 'a3', timestamp: newTs, action: 'auth.totp-verify' },
]));
await fsp.writeFile(secFile, [
JSON.stringify({ id: 's1', timestamp: oldTs, severity: 'info' }),
JSON.stringify({ id: 's2', timestamp: oldTs, severity: 'info' }),
JSON.stringify({ id: 's3', timestamp: newTs, severity: 'info' }),
].join('\n') + '\n');
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(200);
expect(r.body.preview).toBe(true);
expect(r.body.wouldDelete.auditEntries).toBe(2);
expect(r.body.wouldDelete.securityEvents).toBe(2);
// Files untouched
const afterAudit = JSON.parse(await fsp.readFile(auditFile, 'utf8'));
expect(afterAudit.length).toBe(3);
const afterSec = (await fsp.readFile(secFile, 'utf8')).split('\n').filter(Boolean);
expect(afterSec.length).toBe(3);
});
test('confirm pass: actually deletes old entries, keeps new ones', async () => {
const oldTs = new Date(Date.now() - 100 * 86400000).toISOString();
const newTs = new Date(Date.now() - 5 * 86400000).toISOString();
await fsp.writeFile(auditFile, JSON.stringify([
{ id: 'a1', timestamp: oldTs, action: 'service.create' },
{ id: 'a2', timestamp: newTs, action: 'auth.totp-verify' },
]));
await fsp.writeFile(secFile, [
JSON.stringify({ id: 's1', timestamp: oldTs, severity: 'info' }),
JSON.stringify({ id: 's2', timestamp: newTs, severity: 'info' }),
].join('\n') + '\n');
const r = await postKeepDays({ keepDays: 30, confirm: true });
expect(r.status).toBe(200);
expect(r.body.disposed).toBe(true);
expect(r.body.deleted.auditEntries).toBe(1);
expect(r.body.deleted.securityEvents).toBe(1);
expect(r.body.remaining.auditEntries).toBe(1);
expect(r.body.remaining.securityEvents).toBe(1);
const afterAudit = JSON.parse(await fsp.readFile(auditFile, 'utf8'));
expect(afterAudit.map(e => e.id)).toEqual(['a2']);
const afterSec = (await fsp.readFile(secFile, 'utf8')).split('\n').filter(Boolean).map(JSON.parse);
expect(afterSec.map(e => e.id)).toEqual(['s2']);
});
test('confirm=false treated as preview (not confirm)', async () => {
const r = await postKeepDays({ keepDays: 30, confirm: false });
expect(r.status).toBe(200);
expect(r.body.preview).toBe(true);
// confirm was false, so no dispose
expect(r.body.disposed).toBeUndefined();
});
test('preview response includes resolved paths so operator knows what files will be touched', async () => {
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(200);
expect(r.body.paths.auditPath).toBe(auditFile);
expect(r.body.paths.secPath).toBe(secFile);
});
test('handles missing audit-log file gracefully on preview', async () => {
await fsp.unlink(auditFile).catch(() => {});
// fs.readFile().catch returns '[]', so preview reports 0 deletions
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(200);
expect(r.body.wouldDelete.auditEntries).toBe(0);
});
test('returns 500 DC-081_AUDIT_PARSE_FAILED on corrupt audit-log file', async () => {
await fsp.writeFile(auditFile, 'this-is-not-json{');
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(500);
expect(r.body.code).toBe('DC-081_AUDIT_PARSE_FAILED');
});
test('returns 500 DC-081_AUDIT_SHAPE_INVALID if audit-log is a JSON object, not array', async () => {
await fsp.writeFile(auditFile, JSON.stringify({ not: 'an array' }));
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(500);
expect(r.body.code).toBe('DC-081_AUDIT_SHAPE_INVALID');
});
test('DC-081 CORE: pre-fix keptDays=-1000 no longer wipes everything', async () => {
// Sanity-test the actual fix: a negative keepDays would, pre-fix,
// compute a cutoff in the FUTURE and then delete everything. After
// DC-081 it's a 400 with a clear error before any file read.
const r = await postKeepDays({ keepDays: -1000, confirm: true });
expect(r.status).toBe(400);
expect(r.body.success).toBe(false);
// No file IO occurred — confirm that an unrelated existing audit
// log file would survive. Since we already wiped tmpDir's auditFile
// is empty, write a sentinel and confirm it's still there after.
await fsp.writeFile(auditFile, JSON.stringify([{ id: 'sentinel', timestamp: new Date().toISOString() }]));
const r2 = await postKeepDays({ keepDays: -1000, confirm: true });
expect(r2.status).toBe(400);
const after = JSON.parse(await fsp.readFile(auditFile, 'utf8'));
expect(after.length).toBe(1);
expect(after[0].id).toBe('sentinel');
});
});
});
@@ -131,6 +131,20 @@ describe('routes/tailscale-admin: PUT /settings', () => {
expect(res.status).toBe(400);
});
test('400 on apiToken exceeding 256-char length cap (DC-080)', async () => {
const { app } = createApp();
const oversized = 'tskey-api-' + 'x'.repeat(300); // > 256 chars
const res = await request(app).put('/api/v1/tailscale/settings').send({ apiToken: oversized });
expect(res.status).toBe(400);
expect(res.body.error || res.body.message).toMatch(/exceeds maximum length/i);
});
test('400 on non-string apiToken (DC-080)', async () => {
const { app } = createApp();
const res = await request(app).put('/api/v1/tailscale/settings').send({ apiToken: 12345 });
expect(res.status).toBe(400);
});
test('200 + saves token + writes metadata on valid token', async () => {
const fakeClient = makeFakeClient({
ping: jest.fn(async () => ({ domain: 'real.ts.net' })),
@@ -293,6 +307,76 @@ describe('routes/tailscale-admin: POST /settings/test', () => {
expect(res.body.valid).toBe(true);
expect(fakeClient.setApiToken).toHaveBeenCalledWith('tskey-api-test-only');
});
test('400 on body.apiToken not starting with tskey-api- (DC-080)', async () => {
const fakeClient = makeFakeClient();
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: false }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
const res = await request(app)
.post('/api/v1/tailscale/settings/test')
.send({ apiToken: 'arbitrary-junk' });
expect(res.status).toBe(400);
expect(fakeClient.setApiToken).not.toHaveBeenCalled();
});
test('400 on body.apiToken exceeding length cap (DC-080)', async () => {
const fakeClient = makeFakeClient();
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: false }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
const oversized = 'tskey-api-' + 'x'.repeat(300);
const res = await request(app)
.post('/api/v1/tailscale/settings/test')
.send({ apiToken: oversized });
expect(res.status).toBe(400);
expect(fakeClient.setApiToken).not.toHaveBeenCalled();
});
test('omitting apiToken is allowed (uses stored token path) (DC-080)', async () => {
const fakeClient = makeFakeClient({ ping: jest.fn(async () => ({ domain: 'stored.ts.net' })) });
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: true }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
const res = await request(app)
.post('/api/v1/tailscale/settings/test')
.send({}); // no apiToken in body
expect(res.status).toBe(200);
expect(res.body.valid).toBe(true);
});
});
describe('routes/tailscale-admin: GET /admin/devices', () => {
@@ -511,6 +595,99 @@ describe('routes/tailscale-admin: pre-auth keys', () => {
expect(res.status).toBe(400);
});
test('POST /admin/keys rejects null/123/object tags entries (DC-080)', async () => {
const fakeClient = makeFakeClient();
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: true }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
// Mixed: null, number, object — all must be rejected
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({ tags: ['tag:guest', null, 123, { x: 1 }] });
expect(res.status).toBe(400);
expect(fakeClient.createAuthKey).not.toHaveBeenCalled();
});
test('POST /admin/keys rejects uppercase / whitespace / CRLF in tags (DC-080)', async () => {
const fakeClient = makeFakeClient();
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: true }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({ tags: ['TAG:guest', 'tag:foo bar', 'tag:x\r\ninjection'] });
expect(res.status).toBe(400);
expect(fakeClient.createAuthKey).not.toHaveBeenCalled();
});
test('POST /admin/keys rejects description exceeding 120 chars (DC-080)', async () => {
const fakeClient = makeFakeClient();
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: true }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
const longDesc = 'a'.repeat(200); // > 120 chars
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({ description: longDesc });
expect(res.status).toBe(400);
expect(fakeClient.createAuthKey).not.toHaveBeenCalled();
});
test('POST /admin/keys accepts canonical lowercase tag: form (DC-080)', async () => {
const fakeClient = makeFakeClient({
createAuthKey: jest.fn(async (opts) => ({ id: 'k2', key: 'tskey-secret-2', ...opts })),
});
const app = express();
app.use(express.json());
const routes = require('../../routes/tailscale-admin');
const tailscaleCoord = {
loadMetadata: () => ({ configured: true }),
saveMetadata: jest.fn(),
setApiToken: jest.fn(),
getClient: jest.fn(async () => fakeClient),
hasApiToken: jest.fn(),
};
app.use('/api/v1/tailscale', routes({
tailscaleCoord, asyncHandler,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
}));
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({
tags: ['tag:guest-plex', 'tag:server'],
expirySeconds: 86400,
});
expect(res.status).toBe(200);
expect(fakeClient.createAuthKey).toHaveBeenCalledWith(expect.objectContaining({
tags: ['tag:guest-plex', 'tag:server'],
}));
});
test('POST /admin/keys rejects negative expirySeconds', async () => {
const fakeClient = makeFakeClient();
const app = express();
@@ -572,4 +749,110 @@ describe('routes/tailscale-admin: security boundary', () => {
await request(app).delete('/api/v1/tailscale/settings');
expect(stored.token).toBeNull();
});
});
});
// DC-080 direct validator unit tests (no supertest, no Express)
describe('routes/tailscale-admin: DC-080 validators (direct)', () => {
const { _validators } = require('../../routes/tailscale-admin');
const {
validateApiToken,
validateTags,
validateDescription,
TAILSCALE_TOKEN_PREFIX,
TAILSCALE_TOKEN_MAX_LEN,
DESCRIPTION_MAX_LEN,
} = _validators;
describe('validateApiToken', () => {
test('accepts canonical tskey-api-...', () => {
expect(validateApiToken('tskey-api-abc123')).toBeNull();
});
test('rejects empty', () => {
expect(validateApiToken('')).toMatch(/required/);
});
test('rejects undefined / null', () => {
expect(validateApiToken(undefined)).toMatch(/required/);
expect(validateApiToken(null)).toMatch(/required/);
});
test('rejects non-string (number, object, array)', () => {
expect(validateApiToken(123)).toMatch(/must be a string/);
expect(validateApiToken({})).toMatch(/must be a string/);
expect(validateApiToken(['x'])).toMatch(/must be a string/);
});
test('rejects wrong prefix', () => {
expect(validateApiToken('not-a-token')).toMatch(/must start with/);
});
test('accepts exactly at length cap', () => {
const token = 'tskey-api-' + 'x'.repeat(TAILSCALE_TOKEN_MAX_LEN - 'tskey-api-'.length);
expect(validateApiToken(token)).toBeNull();
});
test('rejects 1 over length cap', () => {
const token = 'tskey-api-' + 'x'.repeat(TAILSCALE_TOKEN_MAX_LEN - 'tskey-api-'.length + 1);
expect(validateApiToken(token)).toMatch(/exceeds maximum length/);
});
});
describe('validateTags', () => {
test('accepts undefined / null (optional)', () => {
expect(validateTags(undefined)).toBeNull();
expect(validateTags(null)).toBeNull();
});
test('rejects non-array', () => {
expect(validateTags('tag:foo')).toMatch(/must be an array/);
expect(validateTags({})).toMatch(/must be an array/);
});
test('rejects entries that are not strings', () => {
expect(validateTags(['tag:a', null])).toMatch(/tags\[1\]/);
expect(validateTags(['tag:a', 123])).toMatch(/tags\[1\]/);
expect(validateTags(['tag:a', {}])).toMatch(/tags\[1\]/);
});
test('rejects uppercase / whitespace / CRLF', () => {
expect(validateTags(['TAG:foo'])).toMatch(/tags\[0\]/);
expect(validateTags(['tag:foo bar'])).toMatch(/tags\[0\]/);
expect(validateTags(['tag:foo\r\nbar'])).toMatch(/tags\[0\]/);
});
test('rejects entries starting with non-alnum (no leading colon)', () => {
expect(validateTags([':foo'])).toMatch(/tags\[0\]/);
});
test('rejects bare "tag:" with empty name (Tailscale spec violation) (DC-080 round-2)', () => {
expect(validateTags(['tag:'])).toMatch(/tags\[0\]/);
});
test('rejects colon-only chars after tag: prefix (DC-080 round-2)', () => {
expect(validateTags(['tag:::'])).toMatch(/tags\[0\]/);
expect(validateTags(['tag:---'])).toMatch(/tags\[0\]/);
});
test('accepts canonical tag:server form', () => {
expect(validateTags(['tag:server'])).toBeNull();
expect(validateTags(['tag:guest-plex', 'tag:server'])).toBeNull();
});
test('rejects empty array entry', () => {
expect(validateTags(['tag:a', ''])).toMatch(/tags\[1\]/);
});
});
describe('validateDescription', () => {
test('accepts undefined / null', () => {
expect(validateDescription(undefined)).toBeNull();
expect(validateDescription(null)).toBeNull();
});
test('rejects non-string', () => {
expect(validateDescription(123)).toMatch(/must be a string/);
});
test('rejects over 120 chars', () => {
const long = 'a'.repeat(DESCRIPTION_MAX_LEN + 1);
expect(validateDescription(long)).toMatch(/exceeds maximum length/);
});
test('accepts at the cap', () => {
const exact = 'a'.repeat(DESCRIPTION_MAX_LEN);
expect(validateDescription(exact)).toBeNull();
});
});
test('exports surface stays in sync with constants used inside validators', () => {
// Guard against drift: if a future refactor renames a constant, this fails
expect(TAILSCALE_TOKEN_PREFIX).toBe('tskey-api-');
expect(typeof TAILSCALE_TOKEN_MAX_LEN).toBe('number');
expect(typeof DESCRIPTION_MAX_LEN).toBe('number');
});
});
@@ -0,0 +1,483 @@
/**
* DC-083 -- Public share endpoint input hardening.
*
* The two CSRF-exempt public endpoints (POST /share/:token/subscribe +
* POST /share/:token/redeem-tailscale) accept untrusted body fields. The
* pre-fix code had three coupled bugs:
*
* 1. `email.includes('@')` accepted `@`, `a@`, `<script>@x.c`, and 10MB
* strings as "valid email" -- and the field was never even used after
* validation (the subscribe endpoint discarded it).
* 2. `typeof deviceId === 'string'` accepted arbitrary strings of any
* length, including CR/LF/NUL -- which fed straight into the Tailscale
* auth-key description string and the on-disk shares.json.
* 3. No rate-limit; the general limiter (1000/15min) was too generous for
* unauthenticated state-mutating endpoints.
*
* Fix: charset/length/control-char-bounded validators at the route layer
* AND at the store layer (defense-in-depth), plus a dedicated
* SHARE_PUBLIC rate-limit (30/15min) on the public endpoints.
*
* Coverage:
* - subscribe email: rejects bare @, missing TLD, oversized, CR/LF, shell
* metachars, control chars; accepts normal addresses; accepts OMITTED
* email (backwards-compatible with the original behavior).
* - subscribe email propagates to share-store subscriberEmails (capped 8).
* - redeem-tailscale deviceId: rejects CR/LF/NUL, oversized, empty,
* spaces, brackets, quotes; accepts Tailscale-style base64url+hphens;
* accepts OMITTED deviceId (treated as 'unknown').
* - Sanitized usedBy is what flows into the on-disk shares.json.
* - Rate-limit fires after the configured budget per IP.
* - Store-level defense: bypassing the route (direct store call) still
* rejects invalid inputs.
*/
'use strict';
const fs = require('fs');
const path = require('path');
const os = require('os');
const express = require('express');
const request = require('supertest');
const { createShareStore } = require('../src/security/share-store');
function _tmpDir() {
return fs.mkdtempSync(path.join(os.tmpdir(), 'dashcaddy-share-dc083-'));
}
function _cleanup(dir) {
try { fs.rmSync(dir, { recursive: true, force: true }); } catch {}
}
function _buildApp({ shareStore } = {}) {
const app = express();
app.use(express.json());
// No req.user injection -- the public endpoints must work without auth.
const shareRoutes = require('../routes/share');
app.use(shareRoutes({
shareStore,
licenseManager: { isPro: () => true, allowsLifetimeLicense: () => false },
tailscaleCoord: { createAuthKey: async () => ({ id: 'k', key: 'tskey-x' }) },
notificationManager: { sendEmail: async () => ({ messageId: 'fake' }) },
servicesStateManager: { get: async () => null, read: async () => [] },
servicesFile: null,
asyncHandler: (fn, _label) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next),
log: { info() {}, warn() {}, error() {} },
}));
app.use((err, _req, res, _next) => {
if (err && err.statusCode) {
return res.status(err.statusCode).json({
success: false,
error: err.message,
code: err.code,
});
}
return res.status(500).json({ success: false, error: err && err.message });
});
return app;
}
// --------- Subscribe endpoint -- email validation ---------------------------------------------------------------------------------------
describe('DC-083: subscribe email validation', () => {
let dir, shareStore;
beforeEach(() => { dir = _tmpDir(); shareStore = createShareStore({ dataDir: dir }); });
afterEach(() => _cleanup(dir));
test('accepts omitted email (backwards-compatible)', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app).post(`/share/${issued.token}/subscribe`).send({});
expect(res.status).toBe(200);
expect(res.body.data.count).toBe(1);
});
test('accepts a well-formed email', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 'subscriber@example.com' });
expect(res.status).toBe(200);
expect(res.body.data.count).toBe(1);
});
test('lowercases the email on capture', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 'Subscriber@Example.COM' });
expect(res.status).toBe(200);
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
const id = Object.keys(raw.shares)[0];
expect(raw.shares[id].subscriberEmails).toEqual(['subscriber@example.com']);
});
test('rejects bare @', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: '@' });
expect(res.status).toBe(400);
});
test('rejects missing local-part', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: '@example.com' });
expect(res.status).toBe(400);
});
test('rejects missing TLD', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 'user@localhost' });
expect(res.status).toBe(400);
});
test('rejects single-char TLD', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 'user@example.c' });
expect(res.status).toBe(400);
});
test('rejects CR/LF in email (CRLF-injection defense)', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 'a@b.com\r\nX-Injected: yes' });
expect(res.status).toBe(400);
});
test('rejects NUL in email', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 'a@b.com\x00hack' });
expect(res.status).toBe(400);
});
test('rejects oversized email (>254 chars)', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const longLocal = 'a'.repeat(250) + '@example.com';
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: longLocal });
expect(res.status).toBe(400);
});
test('rejects XSS-shape email', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: '<script>@x.com' });
expect(res.status).toBe(400);
});
test('rejects non-string email', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: 42 });
expect(res.status).toBe(400);
});
test('keeps subscriberEmails capped to 8 entries', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
for (let i = 0; i < 12; i++) {
await request(app)
.post(`/share/${issued.token}/subscribe`)
.send({ email: `user${i}@example.com` });
}
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
const id = Object.keys(raw.shares)[0];
expect(raw.shares[id].subscriberEmails).toHaveLength(8);
// FIFO cap -- the first 4 got dropped, latest 8 remain.
expect(raw.shares[id].subscriberEmails[0]).toBe('user4@example.com');
expect(raw.shares[id].subscriberEmails[7]).toBe('user11@example.com');
});
test('omitted email does not write subscriberEmails', async () => {
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
const app = _buildApp({ shareStore });
await request(app).post(`/share/${issued.token}/subscribe`).send({});
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
const id = Object.keys(raw.shares)[0];
expect(raw.shares[id].subscriberEmails).toBeUndefined();
});
});
// --------- Redeem-tailscale endpoint -- deviceId validation ---------------------------------------------------------
describe('DC-083: redeem-tailscale deviceId validation', () => {
let dir, shareStore;
beforeEach(() => { dir = _tmpDir(); shareStore = createShareStore({ dataDir: dir }); });
afterEach(() => _cleanup(dir));
test('accepts Tailscale-style base64url ID', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'nodekey-abc123-def456' });
expect(res.status).toBe(200);
expect(res.body.data.redeemed).toBe(true);
});
test('accepts OMITTED deviceId (treated as "unknown")', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({});
expect(res.status).toBe(200);
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
const id = Object.keys(raw.shares)[0];
expect(raw.shares[id].usedBy).toBe('unknown');
});
test('rejects CR/LF in deviceId (CRLF-injection defense)', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'nodekey\r\nX-Injected: yes' });
expect(res.status).toBe(400);
});
test('rejects NUL in deviceId', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'nodekey\x00hack' });
expect(res.status).toBe(400);
});
test('rejects oversized deviceId (>128 chars)', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const long = 'a'.repeat(200);
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: long });
expect(res.status).toBe(400);
});
test('rejects empty string deviceId', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: '' });
expect(res.status).toBe(400);
});
test('rejects whitespace in deviceId', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'node key 1' });
expect(res.status).toBe(400);
});
test('rejects shell metachars in deviceId', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'nodekey; rm -rf /' });
expect(res.status).toBe(400);
});
test('rejects non-string deviceId', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: { evil: true } });
expect(res.status).toBe(400);
});
test('sanitized usedBy flows into the on-disk shares.json', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'node-abc.def-123' });
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
const id = Object.keys(raw.shares)[0];
expect(raw.shares[id].usedBy).toBe('node-abc.def-123');
});
test('rejection does NOT mark the share used', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore });
const bad = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'node with spaces' });
expect(bad.status).toBe(400);
// A FOLLOW-UP valid redeem should still succeed.
const ok = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'node-clean' });
expect(ok.status).toBe(200);
});
});
// --------- Store-layer defense-in-depth (bypass the route, hit the store) ------------
describe('DC-083: store-layer defense-in-depth', () => {
let dir, store;
beforeEach(() => { dir = _tmpDir(); store = createShareStore({ dataDir: dir }); });
afterEach(() => _cleanup(dir));
test('recordPublicSubscribe rejects CRLF in email', async () => {
const issued = await store.issuePublic({ serviceId: 'svc' });
const r = await store.recordPublicSubscribe(issued.token, { email: 'a@b.com\r\nX: 1' });
expect(r.ok).toBe(false);
expect(r.reason).toBe('invalid_email');
});
test('recordPublicSubscribe rejects oversized email', async () => {
const issued = await store.issuePublic({ serviceId: 'svc' });
const r = await store.recordPublicSubscribe(issued.token, { email: 'a'.repeat(300) + '@x.com' });
expect(r.ok).toBe(false);
expect(r.reason).toBe('invalid_email');
});
test('recordTailscaleUse rejects CRLF in deviceId', async () => {
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
const r = await store.recordTailscaleUse(issued.token, { deviceId: 'node\r\nhack' });
expect(r.ok).toBe(false);
expect(r.reason).toBe('invalid_device_id');
});
test('recordTailscaleUse rejects oversized deviceId', async () => {
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
const r = await store.recordTailscaleUse(issued.token, { deviceId: 'a'.repeat(200) });
expect(r.ok).toBe(false);
expect(r.reason).toBe('invalid_device_id');
});
test('recordTailscaleUse accepts null deviceId (defaults to "unknown")', async () => {
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
const r = await store.recordTailscaleUse(issued.token, { deviceId: null });
expect(r.ok).toBe(true);
expect(r.share.usedBy).toBe('unknown');
});
test('recordTailscaleUse accepts omitted deviceId (defaults to "unknown")', async () => {
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
const r = await store.recordTailscaleUse(issued.token, {});
expect(r.ok).toBe(true);
expect(r.share.usedBy).toBe('unknown');
});
test('recordPublicSubscribe accepts omitted email (backwards-compatible)', async () => {
const issued = await store.issuePublic({ serviceId: 'svc' });
const r = await store.recordPublicSubscribe(issued.token);
expect(r.ok).toBe(true);
});
test('recordPublicSubscribe accepts null email (backwards-compatible)', async () => {
const issued = await store.issuePublic({ serviceId: 'svc' });
const r = await store.recordPublicSubscribe(issued.token, { email: null });
expect(r.ok).toBe(true);
});
});
// --------- Rate-limit guard ------------------------------------------------------------------------------------------------------------------------------------------------------
describe('DC-083: SHARE_PUBLIC rate-limit', () => {
// We can't easily trigger the rate-limit in a unit test because the
// default 30/15min is high. Instead, verify the constant is wired and
// that the limiter is mounted on the public endpoints (the test env
// skips the limiter, so we just confirm the constants).
test('RATE_LIMITS.SHARE_PUBLIC is bounded tighter than GENERAL', () => {
const { RATE_LIMITS } = require('../src/utilities/constants');
expect(RATE_LIMITS.SHARE_PUBLIC).toBeDefined();
expect(RATE_LIMITS.SHARE_PUBLIC.max).toBeLessThan(RATE_LIMITS.GENERAL.max);
expect(RATE_LIMITS.SHARE_PUBLIC.max).toBeLessThanOrEqual(30);
expect(RATE_LIMITS.SHARE_PUBLIC.windowMs).toBe(15 * 60 * 1000);
});
test('route module loads without throwing when express-rate-limit is wired', () => {
// Smoke test: the route factory must succeed with the limiter attached.
const dir = _tmpDir();
try {
const shareStore = createShareStore({ dataDir: dir });
const app = _buildApp({ shareStore });
// _buildApp would have thrown if the route factory threw.
expect(typeof app).toBe('function');
} finally {
_cleanup(dir);
}
});
test('sharePublicLimiter is mounted on /preview (route stack contains limiter)', () => {
// Verify the limiter middleware is actually wired into /preview's route
// stack. The route uses express.Router().use(path, ...mw, handler) so we
// can inspect the stack via the router's internal `stack` array.
const dir = _tmpDir();
try {
const shareStore = createShareStore({ dataDir: dir });
const router = require('../routes/share')({
shareStore,
licenseManager: { isPro: () => true, allowsLifetimeLicense: () => false },
tailscaleCoord: { createAuthKey: async () => ({ id: 'k', key: 'tskey-x' }) },
notificationManager: { sendEmail: async () => ({ messageId: 'fake' }) },
servicesStateManager: { get: async () => null, read: async () => [] },
servicesFile: null,
asyncHandler: (fn, _label) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next),
log: { info() {}, warn() {}, error() {} },
});
const previewStack = router.stack.find(
(layer) => layer.route && layer.route.path === '/share/:token/preview'
);
expect(previewStack).toBeDefined();
// The route handler should be preceded by at least one middleware
// layer (the limiter). route.stack contains the per-route middleware.
// In express, .route.stack has the route-local middleware + handler.
// The limiter is mounted at the router level (router.use pattern), so
// it's actually a separate layer in router.stack. Look for any layer
// that has a regex/path matching /share/:token.
const limiterLayer = router.stack.find(
(layer) => layer.regexp && layer.regexp.test && layer.regexp.test('/share/abc/preview')
);
expect(limiterLayer).toBeDefined();
} finally {
_cleanup(dir);
}
});
});
describe('DC-083: positive smoke tests (legitimate inputs)', () => {
test('validates user+tag@sub.domain.io (RFC 5322 plus addressing)', () => {
const { validatePublicEmail } = require('../src/security/share-store');
const v = validatePublicEmail('user+tag@sub.domain.io');
expect(v).toEqual({ ok: true, email: 'user+tag@sub.domain.io' });
});
test('validates a typical Tailscale node ID as deviceId', () => {
const { validatePublicDeviceId } = require('../src/security/share-store');
// Tailscale node IDs look like "nodekey:abcdef0123456789" or just hex
const v = validatePublicDeviceId('nodekey:abcdef0123456789');
expect(v).toEqual({ ok: true, deviceId: 'nodekey:abcdef0123456789' });
});
});
+24 -1
View File
@@ -378,12 +378,35 @@ describe('share routes: POST /share/:token/redeem-tailscale (public)', () => {
expect(r2.body.error).toMatch(/already_used/);
});
test('rejects missing deviceId', async () => {
test('rejects missing deviceId — DC-083 accepts omitted, treats as "unknown"', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore, noAdmin: true });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({});
// DC-083: omitted deviceId is now accepted; the store defaults usedBy
// to 'unknown'. The pre-fix route layer required deviceId be present;
// the new behavior matches the store's defensive default and is
// safer for partially-malformed forward_auth calls from Caddy.
expect(res.status).toBe(200);
expect(res.body.data.redeemed).toBe(true);
});
test('rejects invalid deviceId (control chars / oversized)', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore, noAdmin: true });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: 'node\r\nhack' });
expect(res.status).toBe(400);
});
test('rejects empty deviceId', async () => {
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
const app = _buildApp({ shareStore, noAdmin: true });
const res = await request(app)
.post(`/share/${issued.token}/redeem-tailscale`)
.send({ deviceId: '' });
expect(res.status).toBe(400);
});
});
@@ -0,0 +1,228 @@
/**
* DC-082: Update-manager image-name parsing for docker-compose prefixed names.
*
* The pre-fix code normalized `dashcaddy-dashcaddy-api:latest` to
* `library/dashcaddy-dashcaddy-api:latest` before probing Docker Hub.
* Compose-prefixed names (single hyphen, no slash, lowercase) need to
* split on the FIRST hyphen to recover `<project>/<service>` that's
* the actual upstream namespace for a compose-prefixed image.
*
* The fix also adds a "no upstream registry image, skip cleanly" path
* for when the authed GET 401s against a compose-prefixed name (the
* compose-prefixed image is built locally and not published to Docker
* Hub). That should log as info, not error.
*/
const updateManager = require('../src/managers/update-manager');
describe('DC-082 update-manager / compose-prefixed image names', () => {
let um = updateManager; // module exports the singleton instance
describe('_composeProjectToRepo', () => {
test('splits dashcaddy-dashcaddy-api on the first hyphen', () => {
expect(um._composeProjectToRepo('dashcaddy-dashcaddy-api')).toBe('dashcaddy/dashcaddy-api');
});
test('splits myproject-myservice on the first hyphen', () => {
expect(um._composeProjectToRepo('myproject-myservice')).toBe('myproject/myservice');
});
test('splits multi-hyphen names on the FIRST hyphen only', () => {
// "myproj-grandchild-service" -> "myproj/grandchild-service"
// (first hyphen is the project/service boundary; later hyphens are
// part of the service name like docker-compose's `web-cache`).
expect(um._composeProjectToRepo('myproj-grandchild-service')).toBe('myproj/grandchild-service');
});
test('returns null for slash-namespaced names (handled by other path)', () => {
expect(um._composeProjectToRepo('dashcaddy/dashcaddy-api')).toBe(null);
expect(um._composeProjectToRepo('library/nginx')).toBe(null);
expect(um._composeProjectToRepo('ghcr.io/x/y')).toBe(null);
});
test('returns null for Docker Official Image names (no hyphen)', () => {
expect(um._composeProjectToRepo('nginx')).toBe(null);
expect(um._composeProjectToRepo('alpine')).toBe(null);
expect(um._composeProjectToRepo('node')).toBe(null);
});
test('returns null for empty / malformed input', () => {
expect(um._composeProjectToRepo('')).toBe(null);
expect(um._composeProjectToRepo(null)).toBe(null);
expect(um._composeProjectToRepo(undefined)).toBe(null);
expect(um._composeProjectToRepo(123)).toBe(null);
expect(um._composeProjectToRepo('-foo')).toBe(null); // leading hyphen
expect(um._composeProjectToRepo('foo-')).toBe(null); // trailing hyphen
// The regex tolerates mixed-case via the /i flag for defensiveness
// even though Docker Compose names are typically lowercase — the
// important shape constraints are the letter/digit/underscore/hyphen
// charset and the non-empty two-part split.
});
test('accepts names with underscores and digits (compose allows)', () => {
expect(um._composeProjectToRepo('proj-v2_service')).toBe('proj/v2_service');
expect(um._composeProjectToRepo('dashcaddy-api-v2')).toBe('dashcaddy/api-v2');
});
test('rejects names with chars compose never produces', () => {
// dot/colon/slash should never pass — they're either already-namespaced
// or invalid in a Docker Compose service name.
expect(um._composeProjectToRepo('foo:bar')).toBe(null);
expect(um._composeProjectToRepo('foo.bar')).toBe(null);
expect(um._composeProjectToRepo('foo/bar')).toBe(null);
});
});
describe('_isNotPublishedError', () => {
test('returns true for HTTP 401 + compose-prefixed remainder', () => {
const err = new Error('Docker Hub registry returned HTTP 401 after auth');
expect(um._isNotPublishedError(err, 'dashcaddy-dashcaddy-api')).toBe(true);
});
test('returns false for HTTP 401 with non-compose-prefixed remainder', () => {
const err = new Error('Docker Hub registry returned HTTP 401 after auth');
expect(um._isNotPublishedError(err, 'nginx')).toBe(false);
expect(um._isNotPublishedError(err, 'library/nginx')).toBe(false);
expect(um._isNotPublishedError(err, 'dashcaddy/some-image')).toBe(false);
});
test('returns false for non-401 errors', () => {
const err = new Error('network timeout after 10s');
expect(um._isNotPublishedError(err, 'dashcaddy-dashcaddy-api')).toBe(false);
});
test('returns false for malformed error or remainder', () => {
expect(um._isNotPublishedError(null, 'dashcaddy-dashcaddy-api')).toBe(false);
expect(um._isNotPublishedError({}, 'dashcaddy-dashcaddy-api')).toBe(false);
expect(um._isNotPublishedError({ message: 'no string' }, 'dashcaddy-dashcaddy-api')).toBe(false);
expect(um._isNotPublishedError(new Error('HTTP 401'), null)).toBe(false);
expect(um._isNotPublishedError(new Error('HTTP 401'), '')).toBe(false);
});
});
describe('getLatestImageDigest (mocked fetchWithReliability)', () => {
let originalFetch;
let originalFetchAuth;
let originalFetchRetry;
beforeEach(() => {
originalFetch = um.fetchWithReliability.bind(um);
originalFetchAuth = um.fetchAuthToken.bind(um);
});
test('compose-prefixed name (dashcaddy-dashcaddy-api) probes dashcaddy/dashcaddy-api (NOT library/dashcaddy-dashcaddy-api)', async () => {
const calls = [];
um.fetchWithReliability = async (opts) => {
calls.push(opts);
// Simulate the real Docker Hub: 401 with WWW-Auth, then 401 after token
// (because dashcaddy/dashcaddy-api doesn't exist on Docker Hub).
if (calls.length === 1) {
return {
statusCode: 401,
headers: {
'www-authenticate': 'Bearer realm="https://auth.docker.io/token",service="registry.docker.io",scope="repository:dashcaddy/dashcaddy-api:pull"',
},
body: '',
};
}
return { statusCode: 401, headers: {}, body: '{"errors":[{"code":"UNAUTHORIZED","message":"authentication required"}]}' };
};
um.fetchAuthToken = async () => 'fake-token';
const { log } = require('../src/utils/logging');
const infoSpy = jest.spyOn(log, 'info').mockImplementation(() => {});
const errorSpy = jest.spyOn(log, 'error').mockImplementation(() => {});
const result = await um.getLatestImageDigest('dashcaddy-dashcaddy-api:latest');
expect(result).toBe(null);
// First probe must target /v2/dashcaddy/dashcaddy-api/manifests/latest
// NOT /v2/library/dashcaddy-dashcaddy-api/manifests/latest
const firstPath = calls[0].path;
expect(firstPath).toBe('/v2/dashcaddy/dashcaddy-api/manifests/latest');
expect(firstPath).not.toContain('library/dashcaddy-dashcaddy-api');
// The 401 after auth should produce an INFO log about "no upstream"
// NOT an error log.
const infoMsgs = infoSpy.mock.calls.map((c) => c[1]);
expect(infoMsgs).toContain('No upstream registry image — skipping update check');
const errorMsgs = errorSpy.mock.calls.map((c) => c[1]);
expect(errorMsgs).not.toContain('Docker Hub registry returned HTTP 401 after auth');
infoSpy.mockRestore();
errorSpy.mockRestore();
});
test('official image (nginx) still probes library/nginx', async () => {
const calls = [];
um.fetchWithReliability = async (opts) => {
calls.push(opts);
if (calls.length === 1) {
return { statusCode: 200, headers: { 'docker-content-digest': 'sha256:abc123' }, body: '' };
}
return { statusCode: 200, headers: {}, body: '' };
};
const result = await um.getLatestImageDigest('nginx:latest');
expect(result).toBe('sha256:abc123');
expect(calls[0].path).toBe('/v2/library/nginx/manifests/latest');
});
test('library/nginx (explicit) probes library/nginx', async () => {
const calls = [];
um.fetchWithReliability = async (opts) => {
calls.push(opts);
if (calls.length === 1) {
return { statusCode: 200, headers: { 'docker-content-digest': 'sha256:abc' }, body: '' };
}
return { statusCode: 200, headers: {}, body: '' };
};
const result = await um.getLatestImageDigest('library/nginx:latest');
expect(result).toBe('sha256:abc');
expect(calls[0].path).toBe('/v2/library/nginx/manifests/latest');
});
test('ghcr.io/samiahmed7777/dashcaddy-api uses ghcr.io path (not docker hub)', async () => {
const calls = [];
um.fetchWithReliability = async (opts) => {
calls.push(opts);
return { statusCode: 200, headers: { 'docker-content-digest': 'sha256:ghcr' }, body: '' };
};
const result = await um.getLatestImageDigest('ghcr.io/samiahmed7777/dashcaddy-api:latest');
expect(result).toBe('sha256:ghcr');
expect(calls[0].hostname).toBe('ghcr.io');
expect(calls[0].path).toBe('/v2/samiahmed7777/dashcaddy-api/manifests/latest');
});
test('returns null + skips cleanly when compose-prefixed image has no upstream', async () => {
let callCount = 0;
um.fetchWithReliability = async (opts) => {
callCount += 1;
if (callCount === 1) {
return {
statusCode: 401,
headers: { 'www-authenticate': 'Bearer realm="https://auth.docker.io/token",service="registry.docker.io",scope="repository:myproj-myservice:pull"' },
body: '',
};
}
return { statusCode: 401, headers: {}, body: '{"errors":[{"code":"UNAUTHORIZED"}]}' };
};
um.fetchAuthToken = async () => 'fake-token';
const result = await um.getLatestImageDigest('myproj-myservice:latest');
expect(result).toBe(null);
// Probe targets the correct namespace (myproj/myservice), not library/.
const firstCall = await (async () => {
let p;
um.fetchWithReliability = async (opts) => { p = opts; return { statusCode: 200, headers: {}, body: '' }; };
await um.getLatestImageDigest('myproj-myservice:latest');
return p;
})();
expect(firstCall.path).toBe('/v2/myproj/myservice/manifests/latest');
});
afterEach(() => {
um.fetchWithReliability = originalFetch;
um.fetchAuthToken = originalFetchAuth;
});
});
});
+170 -14
View File
@@ -1,8 +1,103 @@
/**
* DC-081: Plain-English log insights + dispose endpoint
*
* GET /api/v1/log-insights Plain English summary of who's doing what
* POST /api/v1/log-insights/dispose Preview then confirm cleanup
*
* DC-081 hardening (paired with the deploy path fix):
* - AUDIT_LOG_FILE / SECURITY_EVENT_LOG_FILE were HARDCODED to
* `/opt/dashcaddy/dashcaddy-api/data/...` which DOES NOT EXIST in the
* production container files live at `/app/data/...`. The dispose
* endpoint silently no-op'd (read empty arrays, wrote empty arrays
* back) and the GET endpoint dropped the storage-size block. Both
* paths now use the same canonical resolution as the audit-logger
* itself: `process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json')`.
* - keepDays was unbounded `parseInt(req.body.keepDays) || 30` accepted
* negative numbers (e.g. -1000 cutoff = +3 years in the future,
* deleting 100% of forensic context) and non-integers (Infinity,
* floats). Now validated to an integer in [1, 3650] (1 day .. 10 years)
* before any file read.
* - confirm gate added: must send { confirm: true, keepDays: N } the
* preview pass is read-only, the confirm pass writes. Matches the
* audit-logs/DELETE confirm=CLEAR pattern.
* - The dispose handler now uses a single shared `_resolvePaths()` helper
* to keep GET and POST in lockstep (and so a future path-config change
* touches one site, not four).
*
* Pre-DC-081 verification: from inside the running container, both
* `/app/data/audit-log.json` (318 KB) and `/app/data/security-events.jsonl`
* (15 MB) exist, but the old hardcoded `/opt/dashcaddy/dashcaddy-api/data/...`
* paths resolve to ENOENT. The dispose endpoint therefore did nothing;
* this fix wires it back to the actual files.
*/
const express = require('express');
const path = require('path');
const fs = require('fs').promises;
const platformPaths = require('../platform-paths');
/**
* Resolve the canonical paths for the audit log + security event log.
*
* Both store the file path in their own module-level constants, so any
* environment override (e.g. AUDIT_LOG_FILE=...) is honoured here too
* exactly the same behaviour as src/security/audit-logger.js and
* src/security/event-store.js. Without this, a container with
* AUDIT_LOG_FILE set would see the dispose handler read from one file
* and the audit-logger write to a different one.
*
* @returns {{auditPath: string, secPath: string, auditPathFrom: string, secPathFrom: string}}
* paths + the source ("env" or "default") so tests can verify.
*/
function _resolvePaths() {
const auditPath = process.env.AUDIT_LOG_FILE
|| path.join(platformPaths.dataDir, 'audit-log.json');
const secPath = process.env.SECURITY_EVENT_LOG_FILE
|| path.join(platformPaths.dataDir, 'security-events.jsonl');
return {
auditPath,
secPath,
auditPathFrom: process.env.AUDIT_LOG_FILE ? 'env' : 'default',
secPathFrom: process.env.SECURITY_EVENT_LOG_FILE ? 'env' : 'default',
};
}
/**
* Validate the keepDays input. Coerces + bounds-checks BEFORE any file
* read so a malicious or mistyped client can't:
* - pass a negative number (cutoff = far future wipe 100%)
* - pass Infinity (parseInt(Infinity, 10) === NaN, currently falls
* through `|| 30` fixed to fail-fast instead)
* - pass a non-integer (e.g. 1.5 cutoff mid-day, off-by-half-day)
* - pass 0 (no-op-but-lies) or 10000 (way past retention policy)
*
* @param {unknown} raw - value from req.body.keepDays
* @returns {number} validated integer in [1, 3650]
* @throws {Error} when out of range / wrong type
*/
function _validateKeepDays(raw) {
if (raw === undefined || raw === null) {
throw new Error('keepDays is required (integer in [1, 3650])');
}
const n = Number(raw);
if (!Number.isFinite(n)) {
throw new Error(`keepDays must be a finite number (received ${JSON.stringify(raw)})`);
}
if (!Number.isInteger(n)) {
throw new Error(`keepDays must be an integer (received ${raw})`);
}
if (n < 1 || n > 3650) {
throw new Error(`keepDays must be between 1 and 3650 (received ${n})`);
}
return n;
}
module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore }) {
const router = express.Router();
// Resolve once at module init so GET + POST both use the same files.
// If the env vars change at runtime (rare — start.sh wires them at
// container start), operators re-deploy rather than mutate env mid-flight.
const { auditPath, secPath } = _resolvePaths();
// GET /api/v1/log-insights — Plain English summary of who's doing what
router.get('/log-insights', asyncHandler(async (req, res) => {
@@ -74,16 +169,18 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
}
// --- Storage info ---
const auditPath = process.env.AUDIT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/audit-log.json';
const secPath = process.env.SECURITY_EVENT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl';
// DC-081: read from the canonical resolved paths (NOT the hardcoded
// /opt/... paths that don't exist in the container). Empty-object
// fallback on ENOENT — the file may legitimately be absent on a
// fresh install where the audit-logger hasn't written yet.
let storage = {};
try {
const a = await fs.stat(auditPath);
storage.auditLog = { sizeMB: +(a.size / 1048576).toFixed(2), entries: auditEntries.length };
storage.auditLog = { sizeMB: +(a.size / 1048576).toFixed(2), entries: auditEntries.length, path: auditPath };
} catch {}
try {
const s = await fs.stat(secPath);
storage.securityEvents = { sizeMB: +(s.size / 1048576).toFixed(2), entries: securityEvents.length };
storage.securityEvents = { sizeMB: +(s.size / 1048576).toFixed(2), entries: securityEvents.length, path: secPath };
} catch {}
ok(res, {
@@ -108,16 +205,44 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
}));
// POST /api/v1/log-insights/dispose — Preview then confirm cleanup
//
// Two-call pattern:
// 1. { keepDays: 30 } → preview, no writes
// 2. { keepDays: 30, confirm: true } → actually delete
//
// DC-081 hardening:
// - keepDays is validated to integer [1, 3650] BEFORE any file read.
// A negative keepDays (e.g. -1000) would previously compute a
// cutoff +3 years in the future, then delete every entry older
// than that — i.e. 100% of the audit log. Now rejected at the gate.
// - auditPath / secPath come from the canonical _resolvePaths() helper
// so the container's actual /app/data files are read (the pre-fix
// hardcoded /opt/dashcaddy/dashcaddy-api/data/... paths resolved to
// ENOENT inside the container, so the endpoint silently did nothing).
router.post('/log-insights/dispose', asyncHandler(async (req, res) => {
const keepDays = parseInt(req.body.keepDays) || 30;
// Validate keepDays first — fail-fast before any file IO so a bad
// client never touches disk.
let keepDays;
try {
keepDays = _validateKeepDays(req.body?.keepDays);
} catch (e) {
return res.status(400).json({ success: false, error: e.message, code: 'DC-081_INVALID_KEEP_DAYS' });
}
const confirm = req.body.confirm === true;
const cutoff = new Date(Date.now() - keepDays * 86400000).toISOString();
const auditPath = process.env.AUDIT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/audit-log.json';
const secPath = process.env.SECURITY_EVENT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl';
// Read both files via the canonical resolved paths (NOT the hardcoded
// /opt/... paths from before — those don't exist in the container).
const auditRaw = await fs.readFile(auditPath, 'utf8').catch(function () { return '[]'; });
const auditData = JSON.parse(auditRaw);
let auditData;
try {
auditData = JSON.parse(auditRaw);
} catch (e) {
return res.status(500).json({ success: false, error: `audit-log file is corrupt (${auditPath}): ${e.message}`, code: 'DC-081_AUDIT_PARSE_FAILED' });
}
if (!Array.isArray(auditData)) {
return res.status(500).json({ success: false, error: `audit-log file is not an array (${auditPath})`, code: 'DC-081_AUDIT_SHAPE_INVALID' });
}
const oldAudit = auditData.filter(function (e) { return e.timestamp < cutoff; });
const secRaw = await fs.readFile(secPath, 'utf8').catch(function () { return ''; });
@@ -127,16 +252,40 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
if (!confirm) {
ok(res, {
preview: true,
message: 'This will delete ' + oldAudit.length + ' audit entries and ' + oldSec.length + ' security events older than ' + keepDays + ' days. Send {confirm: true} to proceed.',
message: 'This will delete ' + oldAudit.length + ' audit entries and ' + oldSec.length + ' security events older than ' + keepDays + ' days. Send {confirm: true, keepDays: ' + keepDays + '} to proceed.',
wouldDelete: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
cutoffDate: cutoff
cutoffDate: cutoff,
paths: { auditPath, secPath },
});
return;
}
// Execute cleanup
// Execute cleanup. Audit the wipe FIRST via the audit-logger so the
// fact that a delete happened is itself preserved (matches the
// audit-logs/DELETE + error-logs/DELETE pattern).
try {
if (auditLogger && typeof auditLogger.log === 'function') {
await auditLogger.log({
action: 'log-insights.dispose',
resource: 'audit-log,security-events',
outcome: 'success',
details: {
keepDays,
cutoff,
deleted: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
},
});
}
} catch { /* don't fail the dispose on audit-side errors */ }
// Rewrite audit-log.json atomically — write to tmp + rename so a
// crash mid-write can't leave the file half-empty (the file is read
// by state-manager on every container start; a corrupt file would
// block the whole API).
const keptAudit = auditData.filter(function (e) { return e.timestamp >= cutoff; });
await fs.writeFile(auditPath, JSON.stringify(keptAudit, null, 2));
const tmpAudit = auditPath + '.tmp';
await fs.writeFile(tmpAudit, JSON.stringify(keptAudit, null, 2));
await fs.rename(tmpAudit, auditPath);
const keptSec = secLines.filter(function (l) { try { return JSON.parse(l).timestamp >= cutoff; } catch (e) { return false; } });
await fs.writeFile(secPath, keptSec.join('\n') + '\n');
@@ -145,9 +294,16 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
disposed: true,
deleted: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
remaining: { auditEntries: keptAudit.length, securityEvents: keptSec.length },
cutoffDate: cutoff
cutoffDate: cutoff,
});
}));
return router;
};
// DC-081: export helpers for direct unit testing (the route handlers are
// otherwise unreachable from outside the factory closure).
module.exports.__test = {
_resolvePaths,
_validateKeepDays,
};
+61 -9
View File
@@ -37,6 +37,10 @@
const { ValidationError, NotFoundError } = require('../src/utilities/errors');
const { PaymentRequiredError } = require('../src/utilities/errors');
const { ok, created, badRequest, notFound } = require('../src/utils/responses');
// DC-083: route-layer validators for the public CSRF-exempt endpoints. These
// are imported from share-store so the route and store stay in lockstep
// (drift risk if one set is updated and the other is forgotten).
const { validatePublicEmail, validatePublicDeviceId } = require('../src/security/share-store');
const PUBLIC_TTL_OPTIONS = new Set([
60 * 60 * 1000,
@@ -293,7 +297,35 @@ module.exports = function shareRoutesFactory({
// ─── Public endpoints (no auth, no Pro gate) ──────────────────────────────
router.get('/share/:token/preview', asyncHandler(async (req, res) => {
// DC-083: rate-limit the two CSRF-exempt public endpoints. The general
// limiter (1000/15min) is mounted globally in app.js and is too generous
// for unauthenticated state-mutating endpoints. 30/15min per IP is
// enough for a legitimate user clicking "subscribe" once or twice; anything
// beyond is abuse. Skipped in test envs via the standard isTest guard.
// Lazy-loaded so test environments without the dep installed don't blow up;
// a missing-dep in production logs a warning and falls back to no-op (still
// safe — the route+store validators are the primary defense).
const { RATE_LIMITS } = require('../src/utilities/constants');
const isTest = process.env.NODE_ENV === 'test';
let _sharePublicLimiter = (req, _res, next) => next(); // no-op default
try {
const rateLimit = require('express-rate-limit'); // eslint-disable-line global-require
_sharePublicLimiter = rateLimit({
...RATE_LIMITS.SHARE_PUBLIC,
standardHeaders: true,
legacyHeaders: false,
skip: () => isTest,
message: { success: false, error: 'Too many share requests, please try again later' },
});
} catch (e) {
// Don't crash on missing dep in a bare-bones env — but log so it's not
// invisible if production misconfigured.
if (log && typeof log.warn === 'function') {
log.warn({ ctx: 'share-routes', err: e.message }, 'express-rate-limit unavailable; share public endpoints have NO rate limit');
}
}
router.get('/share/:token/preview', _sharePublicLimiter, asyncHandler(async (req, res) => {
const meta = await shareStore.peek(req.params.token);
if (!meta) {
return res.status(404).json({ success: false, error: '[DC-553] share not found or expired' });
@@ -310,12 +342,21 @@ module.exports = function shareRoutesFactory({
});
}, 'share-preview'));
router.post('/share/:token/subscribe', asyncHandler(async (req, res) => {
router.post('/share/:token/subscribe', _sharePublicLimiter, asyncHandler(async (req, res) => {
// DC-083: replace the primitive `email.includes('@')` check with a
// charset/length/control-char-bounded validator. The pre-fix code
// accepted `@`, `a@`, `<script>@x.c`, and 10MB strings as "valid email".
// The subscribe body's `email` is now also captured to the share record
// (capped to last 8 entries, see share-store recordPublicSubscribe) so
// the operator can see who subscribed.
const { email } = req.body || {};
if (!email || typeof email !== 'string' || !email.includes('@')) {
throw new ValidationError('valid email required', 'email');
let normalizedEmail = null;
if (email !== undefined && email !== null) {
const v = validatePublicEmail(email);
if (!v.ok) throw new ValidationError(v.reason, 'email');
normalizedEmail = v.email;
}
const result = await shareStore.recordPublicSubscribe(req.params.token);
const result = await shareStore.recordPublicSubscribe(req.params.token, { email: normalizedEmail });
if (!result.ok) {
if (result.reason === 'not_found') throw new NotFoundError('share not found');
throw new ValidationError(result.reason, 'share');
@@ -323,12 +364,23 @@ module.exports = function shareRoutesFactory({
res.json({ success: true, data: { count: result.count, cap: result.cap } });
}, 'share-subscribe'));
router.post('/share/:token/redeem-tailscale', asyncHandler(async (req, res) => {
router.post('/share/:token/redeem-tailscale', _sharePublicLimiter, asyncHandler(async (req, res) => {
// DC-083: replace the bare `typeof deviceId === 'string'` check with a
// charset/length/control-char-bounded validator. The pre-fix code
// accepted arbitrary strings of any length — including CR/LF/NUL,
// which flow into the Tailscale auth-key description string in
// POST /share/tailscale (routes/share.js:213 in the issue path).
// The redeem-tailscale path receives the deviceId from Caddy's
// forward_auth (a Tailscale machine ID), which is base64url +
// hyphens — well within the validator's charset.
const { deviceId } = req.body || {};
if (!deviceId || typeof deviceId !== 'string') {
throw new ValidationError('deviceId required', 'deviceId');
let normalizedDeviceId = null;
if (deviceId !== undefined && deviceId !== null) {
const v = validatePublicDeviceId(deviceId);
if (!v.ok) throw new ValidationError(v.reason, 'deviceId');
normalizedDeviceId = v.deviceId;
}
const result = await shareStore.recordTailscaleUse(req.params.token, { deviceId });
const result = await shareStore.recordTailscaleUse(req.params.token, { deviceId: normalizedDeviceId });
if (!result.ok) {
if (result.reason === 'not_found') throw new NotFoundError('share not found');
throw new ValidationError(result.reason, 'share');
+146 -8
View File
@@ -41,12 +41,124 @@
*
* DELETE /api/v1/tailscale/admin/devices/:id
* Revokes a device from the tailnet.
*
* # DC-080 input validation
*
* Three coupled gaps in the route layer pre-fix:
*
* (a) PUT /settings validated `apiToken.startsWith('tskey-api-')` but had
* no length cap body-parser limit was the only ceiling. A 1 MB
* string starting with `tskey-api-` would be `.trim()`-ed, sent to
* Tailscale's /devices endpoint, and waste server-side CPU on a
* request that will always 401.
* (b) POST /settings/test accepted `apiToken` from the body with NO
* validation at all. The PUT route's prefix check is bypassed on
* the test path an operator could submit any string and have the
* container ping Tailscale's API with it (low impact, but inconsistent
* with PUT and surfaces fingerprinting via the 401 timing).
* (c) POST /admin/keys validated `tags` as Array but NOT per-element
* type `tags: ['tag:guest', null, 123, {injection: true}]` would be
* forwarded to Tailscale verbatim. Tailscale's API is JSON-strict
* and would 400 the request, but the bad shape reached the wire.
* Similarly `description` had no length cap (Tailscale caps at 120
* chars per their docs).
*
* All three are gated by TOTP this is a logged-in-operator / phished-
* session threat surface, not anonymous-unauth. The fix is defense-in-
* depth: a bug in the auth path (TOTP bypass, session theft, future
* route handler trust-boundary drift) should not turn these endpoints
* into a "submit anything and forward to Tailscale" relay.
*/
const express = require('express');
const { ok, errorResponse } = require('../src/utils/responses');
const { TailscaleCoordError } = require('../src/managers/tailscale-coord');
// DC-080: shared validation helpers for the Tailscale admin surface.
// Tailscale API tokens follow the form `tskey-<kind>-<opaque>` where
// `<kind>` is one of a small set of values (`api`, `auth`, `partner`,
// `cli`). Real tokens observed in the wild are 40..80 chars; we cap at
// 256 to leave headroom for future Tailscale key formats without giving
// an unbounded buffer to validate+forward.
const TAILSCALE_TOKEN_PREFIX = 'tskey-api-';
const TAILSCALE_TOKEN_MAX_LEN = 256;
const TAG_KEY_MAX_LEN = 64;
const TAGS_MAX_LEN = 32;
const DESCRIPTION_MAX_LEN = 120;
// Tailscale tags are lowercased identifiers with optional colons
// (e.g. `tag:server`, `tag:guest-plex`). Reject whitespace, CR/LF,
// control chars, JSON metacharacters, and any character that could
// enable header-injection through the Tailscale coord client.
//
// DC-080 round-2 polish: Tailscale's tag spec requires `tag:` followed by
// ≥1 identifier char — bare `tag:` (empty name) is rejected by their API.
// We split the pattern in two so the error message names which form failed
// instead of dumping a generic regex.
const TAG_KEY_RE = /^tag:[a-z0-9][a-z0-9_-]{0,62}$/;
function _validateApiToken(token, fieldName = 'apiToken') {
if (typeof token !== 'string' || !token) {
return `${fieldName} is required and must be a string`;
}
if (!token.startsWith(TAILSCALE_TOKEN_PREFIX)) {
return `${fieldName} must start with ${TAILSCALE_TOKEN_PREFIX}`;
}
if (token.length > TAILSCALE_TOKEN_MAX_LEN) {
return `${fieldName} exceeds maximum length of ${TAILSCALE_TOKEN_MAX_LEN} characters`;
}
return null;
}
function _validateTags(tags) {
if (tags === undefined || tags === null) return null;
if (!Array.isArray(tags)) {
return 'tags must be an array of strings';
}
if (tags.length > TAGS_MAX_LEN) {
return `tags exceeds maximum length of ${TAGS_MAX_LEN} entries`;
}
for (let i = 0; i < tags.length; i += 1) {
const t = tags[i];
if (typeof t !== 'string' || !t) {
return `tags[${i}] must be a non-empty string`;
}
if (t.length > TAG_KEY_MAX_LEN) {
return `tags[${i}] exceeds maximum length of ${TAG_KEY_MAX_LEN} characters`;
}
if (!TAG_KEY_RE.test(t)) {
return `tags[${i}] must match ${TAG_KEY_RE} (lowercase alnum + :_-)`;
}
}
return null;
}
function _validateDescription(description) {
if (description === undefined || description === null) return null;
if (typeof description !== 'string') {
return 'description must be a string';
}
if (description.length > DESCRIPTION_MAX_LEN) {
return `description exceeds maximum length of ${DESCRIPTION_MAX_LEN} characters`;
}
return null;
}
// Exported for direct unit testing in __tests__/routes/tailscale-admin.test.js
// (the validator functions are otherwise unreachable from outside the factory
// closure; direct tests assert edge cases without supertest overhead).
const _validators = {
validateApiToken: _validateApiToken,
validateTags: _validateTags,
validateDescription: _validateDescription,
TAILSCALE_TOKEN_PREFIX,
TAILSCALE_TOKEN_MAX_LEN,
TAG_KEY_MAX_LEN,
TAGS_MAX_LEN,
DESCRIPTION_MAX_LEN,
TAG_KEY_RE,
};
module.exports = function({
tailscaleCoord,
asyncHandler,
@@ -75,9 +187,12 @@ module.exports = function({
router.put('/settings', asyncHandler(async (req, res) => {
const token = req.body && req.body.apiToken;
if (!token || typeof token !== 'string' || !token.startsWith('tskey-api-')) {
return errorResponse(res, 400, 'Invalid API token (must start with tskey-api-)');
}
// DC-080: validate prefix + length cap. The pre-fix code only checked
// the prefix — a 1 MB string starting with `tskey-api-` would have been
// sent to Tailscale's /devices endpoint and wasted server-side CPU
// before the inevitable 401.
const tokenErr = _validateApiToken(token);
if (tokenErr) return errorResponse(res, 400, tokenErr);
// Validate before storing
const client = new (require('../src/managers/tailscale-coord').TailscaleCoordClient)({ apiToken: token });
@@ -130,6 +245,17 @@ module.exports = function({
router.post('/settings/test', asyncHandler(async (req, res) => {
const token = (req.body && req.body.apiToken) || null;
// DC-080: validate any caller-provided token before it reaches the
// Tailscale API. Pre-fix the test endpoint accepted any string — the
// PUT route's prefix check did NOT extend to this path. An operator
// could submit arbitrary junk and the container would still call
// /devices on the Tailscale API with it (DoS-reflection + fingerprint
// timing for a future attacker probing whether this API token format
// is accepted at all).
if (token !== null && token !== undefined) {
const tokenErr = _validateApiToken(token);
if (tokenErr) return errorResponse(res, 400, tokenErr);
}
const client = await tailscaleCoord.getClient();
if (token) {
// Caller provided a fresh token to test — don't save it
@@ -214,10 +340,16 @@ module.exports = function({
return errorResponse(res, 503, 'Tailscale API token not configured');
}
const opts = req.body || {};
// Reject obviously-bad input early
if (opts.tags && !Array.isArray(opts.tags)) {
return errorResponse(res, 400, 'tags must be an array of strings');
}
// Reject obviously-bad input early.
// DC-080: pre-fix the route only checked `Array.isArray(opts.tags)`.
// A `tags: ['tag:guest', null, 123, {injection: true}]` payload would
// be forwarded to Tailscale verbatim — Tailscale's API is JSON-strict
// and would 400 the request, but the bad shape reached the wire and
// would silently pass through the dashboard's JSON.stringify() flow.
const tagsErr = _validateTags(opts.tags);
if (tagsErr) return errorResponse(res, 400, tagsErr);
const descErr = _validateDescription(opts.description);
if (descErr) return errorResponse(res, 400, descErr);
if (opts.expirySeconds !== undefined && (!Number.isInteger(opts.expirySeconds) || opts.expirySeconds <= 0)) {
return errorResponse(res, 400, 'expirySeconds must be a positive integer');
}
@@ -254,4 +386,10 @@ module.exports = function({
}));
return router;
};
};
// DC-080: validators exported for direct unit testing in
// __tests__/routes/tailscale-admin.test.js — the route factory closes
// over the same functions, so the validators are exercised end-to-end via
// supertest AND in isolation here.
module.exports._validators = _validators;
+102 -3
View File
@@ -168,12 +168,39 @@ class UpdateManager extends EventEmitter {
/**
* Get latest image digest from registry
*
* DC-082: when the image name is a docker-compose prefixed name like
* `dashcaddy-dashcaddy-api:latest` (single hyphen-separated, no slash),
* the existing code normalized it to `library/dashcaddy-dashcaddy-api`
* before probing Docker Hub. The actual upstream namespace for a
* compose-prefixed image is `<project>/<service>` (with slash) Docker
* Compose hyphenates the project name and service name when tagging
* locally. The pre-fix code probed the wrong repo, Docker Hub returned
* HTTP 401 (the repo doesn't exist), and the error log showed
* `Docker Hub registry returned HTTP 401 after auth` on every restart
* for the local dashcaddy-api image. The fix: split on the FIRST hyphen
* for compose-prefixed names so the lookup targets the correct
* namespace.
*
* Compose-prefixed shape: `^[a-z0-9]+-[a-z0-9][a-z0-9_-]*$` (no slash,
* lowercase, both halves non-empty). Examples:
* dashcaddy-dashcaddy-api -> dashcaddy/dashcaddy-api
* myproject-myservice -> myproject/myservice
* nginx -> library/nginx (official, unchanged)
* library/nginx -> library/nginx (official, unchanged)
* dashcaddy/some-image -> dashcaddy/some-image (already has slash)
* ghcr.io/x/y -> ghcr.io/x/y (handled below)
*/
async getLatestImageDigest(imageName) {
// DC-082: declare `remainder` at the function scope so the catch block
// can classify the error against the image-name shape (compose-prefixed
// local images produce a steady-state 401 that should log as info, not
// error).
let remainder = imageName;
try {
// Parse image name — strip any leading registry host first
let imageTag = 'latest';
let remainder = imageName;
remainder = imageName;
const lastColon = imageName.lastIndexOf(':');
// Only treat as tag if the colon is AFTER the last slash (avoids `ghcr.io:443/...`)
const lastSlash = imageName.lastIndexOf('/');
@@ -187,8 +214,19 @@ class UpdateManager extends EventEmitter {
return await this.getGhcrDigest(remainder, imageTag);
}
// Docker Hub images (library/nginx OR org/image with single slash)
if (!remainder.includes('/') || remainder.split('/').length === 2) {
// Docker Hub images (library/nginx OR org/image with single slash).
// Special-case docker-compose prefixed names (single hyphen, no slash,
// lowercase) — split on the FIRST hyphen to recover the original
// `<project>/<service>` namespace. See DC-082.
if (!remainder.includes('/')) {
const composeRepo = this._composeProjectToRepo(remainder);
if (composeRepo) {
return await this.getDockerHubDigest(composeRepo, imageTag);
}
// Not a compose-prefixed name — fall through to the library/ default
return await this.getDockerHubDigest(remainder, imageTag);
}
if (remainder.split('/').length === 2) {
return await this.getDockerHubDigest(remainder, imageTag);
}
@@ -196,11 +234,72 @@ class UpdateManager extends EventEmitter {
log.warn('update', 'Custom registry not yet supported', { remainder });
return null;
} catch (error) {
// DC-082: a "registry returned HTTP 401 after auth" against a
// compose-prefixed local image is the steady-state when the image
// is built locally and the upstream namespace on Docker Hub
// doesn't exist (or is private). The token endpoint returns 200
// with an empty-access JWT, and the authed manifest GET 401s.
// Log these as a clean info not-found line instead of an error
// so dashboards and PagerDuty don't fire on every restart.
if (this._isNotPublishedError(error, remainder)) {
log.info('update', 'No upstream registry image — skipping update check', { imageName, remainder });
return null;
}
log.error('update', error, null, { imageName });
return null;
}
}
/**
* DC-082: split a docker-compose prefixed image name on the FIRST hyphen
* to recover the original `<project>/<service>` namespace. Returns null
* for names that don't match the compose-prefixed shape callers fall
* through to the standard library/-prefixed official-image path.
*
* Compose-prefixed shape:
* - Contains exactly one or more hyphens
* - No slash
* - Lowercase letters / digits / hyphens / underscores only
* - Both halves (before first hyphen, after first hyphen) are non-empty
* - First char is a letter or digit (not a hyphen)
*/
_composeProjectToRepo(remainder) {
if (typeof remainder !== 'string' || remainder.length === 0) return null;
if (remainder.includes('/')) return null; // already namespaced
if (!/^[a-z0-9][a-z0-9_-]*-[a-z0-9][a-z0-9_-]*$/i.test(remainder)) {
// Not a compose-prefixed name — let the library/ path handle it
// (this is the official-image path: e.g. `nginx`, `alpine`).
return null;
}
const firstHyphen = remainder.indexOf('-');
// Defensive: indexOf must find a hyphen (regex requires it), but guard
// against any future regex drift.
if (firstHyphen <= 0 || firstHyphen === remainder.length - 1) return null;
const project = remainder.substring(0, firstHyphen);
const service = remainder.substring(firstHyphen + 1);
if (!project || !service) return null;
return `${project}/${service}`;
}
/**
* DC-082: detect the "registry returned 401 after auth" pattern that
* signals "this image has no public upstream on Docker Hub" (as opposed
* to a genuine auth failure or transient network error). Steady-state
* for compose-prefixed local images that aren't published.
*/
_isNotPublishedError(error, remainder) {
if (!error || typeof error.message !== 'string') return false;
if (!error.message.includes('HTTP 401')) return false;
// Constrain to the compose-prefixed path — a real auth failure on a
// legitimate `library/foo` or `namespace/foo` probe should still log
// as an error (it never auto-heals).
if (typeof remainder !== 'string' || remainder.includes('/')) return false;
if (!/^[a-z0-9][a-z0-9_-]*-[a-z0-9][a-z0-9_-]*$/i.test(remainder)) {
return false;
}
return true;
}
/**
* Get image digest from GitHub Container Registry (ghcr.io)
* Public images are tokenless via the registry-1.docker.io-style bearer flow,
+82 -3
View File
@@ -47,6 +47,53 @@ const ALLOWED_PUBLIC_TTLS = new Set([60 * 60 * 1000, 24 * 60 * 60 * 1000, 7 * 24
const TAILSCALE_MAX_USES = 1;
const PUBLIC_DEFAULT_SUBSCRIBE_CAP = 1000; // bound on subscribe events per link
// DC-083: Public share endpoint input bounds. The two CSRF-exempt public
// endpoints accept untrusted body fields — bound shape, length, charset so
// an attacker can't bloat data/shares.json, inject CRLF into fields that
// flow into Tailscale auth-key descriptions, or smuggle control chars into
// the on-disk store. See routes/share.js for the route-layer validation;
// these helpers are the defense-in-depth belt under the route's suspenders.
const PUBLIC_EMAIL_REGEX = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
const PUBLIC_EMAIL_MAX_LENGTH = 254; // RFC 5321 §4.5.3.1.3
const PUBLIC_DEVICE_ID_REGEX = /^[a-zA-Z0-9._:-]+$/;
const PUBLIC_DEVICE_ID_MIN_LENGTH = 1;
const PUBLIC_DEVICE_ID_MAX_LENGTH = 128;
function validatePublicEmail(raw) {
if (typeof raw !== 'string') return { ok: false, reason: 'invalid_email' };
// Reject control chars / NUL / CR / LF before they can corrupt the on-disk
// JSON or be embedded in subsequent log lines. RFC 5321 forbids these in
// SMTP addresses; we mirror that at the API layer.
if (raw.length === 0 || raw.length > PUBLIC_EMAIL_MAX_LENGTH) {
return { ok: false, reason: 'invalid_email' };
}
// eslint-disable-next-line no-control-regex
if (/[\x00-\x1f\x7f]/.test(raw)) return { ok: false, reason: 'invalid_email' };
// The local-part can technically contain `+`, `.`, `_`, `%`, `-`; the
// domain part must have at least one dot and a 2+ letter TLD. Reject
// quote-bracket forms (RFC 5321 obs-quote-text) — we don't accept them.
if (!PUBLIC_EMAIL_REGEX.test(raw)) return { ok: false, reason: 'invalid_email' };
// Block obvious shell-attachment characters that the regex doesn't catch.
if (/[<>{}|\\^`\s]/.test(raw)) return { ok: false, reason: 'invalid_email' };
return { ok: true, email: raw.toLowerCase() };
}
function validatePublicDeviceId(raw) {
if (typeof raw !== 'string') return { ok: false, reason: 'invalid_device_id' };
if (raw.length < PUBLIC_DEVICE_ID_MIN_LENGTH || raw.length > PUBLIC_DEVICE_ID_MAX_LENGTH) {
return { ok: false, reason: 'invalid_device_id' };
}
// Tailscale machine IDs are base64url-with-hyphens; we accept a slightly
// broader charset (`._:-`) to also accommodate hostname-style IDs and
// Caddy's `forward_auth` device headers. Reject CR/LF/NUL/TAB explicitly
// so a smuggled control char can't break out of the Tailscale auth-key
// description string in routes/share.js:213.
// eslint-disable-next-line no-control-regex
if (/[\x00-\x1f\x7f]/.test(raw)) return { ok: false, reason: 'invalid_device_id' };
if (!PUBLIC_DEVICE_ID_REGEX.test(raw)) return { ok: false, reason: 'invalid_device_id' };
return { ok: true, deviceId: raw };
}
function _nowMs() { return Date.now(); }
function _nowIso() { return new Date().toISOString(); }
@@ -327,8 +374,18 @@ function createShareStore(opts = {}) {
});
}
function recordPublicSubscribe(token) {
function recordPublicSubscribe(token, { email } = {}) {
return _enqueue(() => {
// DC-083: validate the optional subscriber email at the store layer too.
// The route layer validates first; this is the defense-in-depth catch
// for direct callers (cron sweepers, internal jobs, future endpoints).
// `email` is OPT-IN — callers omitting it get the original behavior.
let normalizedEmail = null;
if (email !== undefined && email !== null) {
const v = validatePublicEmail(email);
if (!v.ok) return { ok: false, reason: v.reason };
normalizedEmail = v.email;
}
const data = _load();
const hash = _sha256(token);
const s = _findByHash(data, hash);
@@ -341,6 +398,16 @@ function createShareStore(opts = {}) {
const cap = s.subscribeCap || PUBLIC_DEFAULT_SUBSCRIBE_CAP;
if (s.subscribeCount >= cap) return { ok: false, reason: 'cap_reached' };
s.subscribeCount += 1;
// DC-083: record the last submitting email (capped to 8 entries to
// bound the on-disk size). PII minimization — we keep only the hash
// + last 8 emails; full email log would grow unbounded.
if (normalizedEmail) {
if (!Array.isArray(s.subscriberEmails)) s.subscriberEmails = [];
s.subscriberEmails.push(normalizedEmail);
if (s.subscriberEmails.length > 8) {
s.subscriberEmails.splice(0, s.subscriberEmails.length - 8);
}
}
_save(data);
return { ok: true, count: s.subscribeCount, cap };
});
@@ -348,6 +415,18 @@ function createShareStore(opts = {}) {
function recordTailscaleUse(token, { deviceId } = {}) {
return _enqueue(() => {
// DC-083: validate deviceId at the store layer. The pre-fix code
// accepted ANY string of any length, including control chars and
// CR/LF — which would flow into the Tailscale auth-key description
// (routes/share.js:213) and into the on-disk shares.json. Reject
// early so an attacker can't bloat the store or smuggle characters
// out of the Tailscale description field.
let normalizedDeviceId = 'unknown';
if (deviceId !== undefined && deviceId !== null) {
const v = validatePublicDeviceId(deviceId);
if (!v.ok) return { ok: false, reason: v.reason };
normalizedDeviceId = v.deviceId;
}
const data = _load();
const hash = _sha256(token);
const s = _findByHash(data, hash);
@@ -359,7 +438,7 @@ function createShareStore(opts = {}) {
return { ok: false, reason: 'expired' };
}
s.usedAt = _nowIso();
s.usedBy = typeof deviceId === 'string' ? deviceId : 'unknown';
s.usedBy = normalizedDeviceId;
_save(data);
return { ok: true, share: _publicView(s) };
});
@@ -411,4 +490,4 @@ function createShareStore(opts = {}) {
};
}
module.exports = { createShareStore };
module.exports = { createShareStore, validatePublicEmail, validatePublicDeviceId };
+11
View File
@@ -79,6 +79,17 @@ const RATE_LIMITS = {
windowMs: 15 * 60 * 1000,
max: 10,
},
// DC-083: Public share endpoint limiter. The two CSRF-exempt public
// endpoints (POST /share/:token/subscribe + POST /share/:token/redeem-tailscale)
// mutate on-disk state (data/shares.json). Bound them tighter than the
// general limiter (1000/15min) so a single attacker can't bloat the
// store or saturate the tmp+rename writer. 30/15min is enough for a
// legitimate user clicking "subscribe" once or twice — anything beyond
// is abuse.
SHARE_PUBLIC: {
windowMs: 15 * 60 * 1000,
max: 30,
},
};
// ── Caddy ─────────────────────────────────────────────────────