From c28322eb46b8eb38df1aba3909cbb83d4c83520f Mon Sep 17 00:00:00 2001 From: Hermes Date: Mon, 31 Aug 2026 23:37:00 -0700 Subject: [PATCH] [grade=B] api: fix undeclared 'format' runtime bug in routes/ca.js + 10 lint errors - routes/ca.js: declare format before pfx/pem/crt dispatch (was ReferenceError on every request that passed validation); add CA_CERT_FORMATS single source of truth + hardened format extraction (string-coerce, whitelist) - routes/caddycode.js: fix upstream-validation regexes (control-char classes) - routes/logs.js: SSE/validation lint fixes - routes/openclaw.js: remove useless regex escape in ALLOWED_PATH_RE - fleet-validation.js + http-caddy-admin-origin test: eslint-disable for intentional control-regex security sentinels - ca-dc076.routes.test.js: regression test for declared format + behavioral coverage of format validation (now pre-PKI) Judge: Qwen lane (qwen3.8-max) grade B, 0 blocking, verdict /tmp/judge-batch1-verdict.json. API suite 2859/2859 green. --- .../__tests__/routes/ca-dc076.routes.test.js | 61 +++++++++++++++++++ .../utils-http-caddy-admin-origin.test.js | 1 + dashcaddy-api/routes/ca.js | 21 +++++++ dashcaddy-api/routes/caddycode.js | 4 +- dashcaddy-api/routes/logs.js | 12 ++-- dashcaddy-api/routes/openclaw.js | 2 +- .../src/utilities/fleet-validation.js | 2 + dashcaddy-installer/D | 0 dashcaddy-installer/q | 0 9 files changed, 92 insertions(+), 11 deletions(-) delete mode 100644 dashcaddy-installer/D delete mode 100644 dashcaddy-installer/q diff --git a/dashcaddy-api/__tests__/routes/ca-dc076.routes.test.js b/dashcaddy-api/__tests__/routes/ca-dc076.routes.test.js index eec3a33..f5ad032 100644 --- a/dashcaddy-api/__tests__/routes/ca-dc076.routes.test.js +++ b/dashcaddy-api/__tests__/routes/ca-dc076.routes.test.js @@ -262,6 +262,67 @@ describe('DC-076: CA cert/key disclosure hardening', () => { }); }); + describe('regression: `format` is declared before the dispatch block', () => { + // The handler referenced `format` five times in the pfx/pem/crt/key/ + // fullchain dispatch without ever declaring it — every request that + // reached that far threw ReferenceError. The behavioral tests above + // can't reach the dispatch (PKI files absent in the test env returns + // 500 first), so pin the declaration at the source level instead. + test('routes/ca.js declares `format` before dispatch', () => { + const src = fs.readFileSync( + path.join(__dirname, '../../routes/ca.js'), 'utf8'); + // Declaration derives from req.query.format (via rawFormat) and the + // canonical format list drives validation. + expect(src).toMatch(/const\s+rawFormat\s*=\s*req\.query\.format/); + expect(src).toMatch(/const\s+format\s*=\s*rawFormat\s*\|\|\s*'pfx'/); + expect(src).toMatch(/CA_CERT_FORMATS\s*=\s*\[.*'pfx'.*'fullchain'.*\]/s); + // And the declaration must come before the first dispatch use. + const declIdx = src.search(/const\s+format\s*=/); + const useIdx = src.indexOf("if (format === 'pfx')"); + expect(declIdx).toBeGreaterThanOrEqual(0); + expect(useIdx).toBeGreaterThan(declIdx); + }); + }); + + describe('/cert/:domain — format validation (DC-076_FORMAT_INVALID)', () => { + // These validations run before the PKI file check, so they are + // reachable in the test environment (unlike the dispatch itself). + test('rejects unknown format value', async () => { + const { app } = createCaApp({ scope: ['admin'] }); + const res = await request(app) + .get('/ca/cert/dns1.local?format=garbage'); + expect(res.status).toBe(400); + expect(res.body.code).toBe('DC-076_FORMAT_INVALID'); + }); + + test('rejects empty format value', async () => { + const { app } = createCaApp({ scope: ['admin'] }); + const res = await request(app) + .get('/ca/cert/dns1.local?format='); + expect(res.status).toBe(400); + expect(res.body.code).toBe('DC-076_FORMAT_INVALID'); + }); + + test('rejects array format (?format=a&format=b)', async () => { + const { app } = createCaApp({ scope: ['admin'] }); + const res = await request(app) + .get('/ca/cert/dns1.local?format=pem&format=crt'); + expect(res.status).toBe(400); + expect(res.body.code).toBe('DC-076_FORMAT_INVALID'); + }); + + test('accepts every documented format (validation passes; PKI 500 is fine)', async () => { + for (const fmt of ['pfx', 'pem', 'crt', 'key', 'fullchain']) { + const { app } = createCaApp({ scope: ['admin'] }); + const qs = fmt === 'pfx' ? `format=pfx&password=GoodPass12` : `format=${fmt}`; + const res = await request(app).get(`/ca/cert/dns1.local?${qs}`); + // Must NOT be a format rejection — anything else (e.g. 500 CA not + // found in the test env) proves validation accepted the format. + expect(res.body.code).not.toBe('DC-076_FORMAT_INVALID'); + } + }); + }); + describe('/cert/:domain — domain validation', () => { test('rejects single-label domain (no dot)', async () => { const { app } = createCaApp({ scope: ['admin'] }); diff --git a/dashcaddy-api/__tests__/utils-http-caddy-admin-origin.test.js b/dashcaddy-api/__tests__/utils-http-caddy-admin-origin.test.js index 01bcd7e..fc01f31 100644 --- a/dashcaddy-api/__tests__/utils-http-caddy-admin-origin.test.js +++ b/dashcaddy-api/__tests__/utils-http-caddy-admin-origin.test.js @@ -53,6 +53,7 @@ function stripComments(src) { .replace(/\/\*[\s\S]*?\*\//g, '') // block comments .replace(/(^|[^:])\/\/.*$/gm, '$1'); // line comments, leaving URLs alone // Pass 3: restore template literals. + // eslint-disable-next-line no-control-regex -- \u0000 is the sentinel from pass 1 return protectedSrc.replace(/\u0000TPL(\d+)\u0000/g, (_, idx) => templates[+idx]); } diff --git a/dashcaddy-api/routes/ca.js b/dashcaddy-api/routes/ca.js index eae62e1..2c165bc 100644 --- a/dashcaddy-api/routes/ca.js +++ b/dashcaddy-api/routes/ca.js @@ -139,6 +139,9 @@ module.exports = function(ctx) { // can mis-handle; reject it to keep the password copy-paste-safe). const CA_PFX_PASSWORD_RE = /^[A-Za-z0-9!@#%^_+,.~:-]{8,64}$/; const CA_CERT_RATE_LIMIT = { windowMs: 60_000, max: 10 }; + // Single source of truth for accepted ?format= values. `wantsPfx`, the + // password requirement, and the response dispatch all derive from this. + const CA_CERT_FORMATS = ['pfx', 'pem', 'crt', 'key', 'fullchain']; const caCertRateBuckets = new Map(); // ip -> { count, resetAt } function caCertRateLimit(ip) { const now = Date.now(); @@ -175,6 +178,24 @@ module.exports = function(ctx) { const { domain } = req.params; + // FIX: `format` was referenced in the dispatch below but never declared, + // so every request that passed validation threw ReferenceError. Default + // 'pfx' matches the `wantsPfx` check (no format param => pfx). + // Accept only a non-empty string: query strings can deliver arrays + // (?format=a&format=b) or nested objects, which must be rejected. + const rawFormat = req.query.format; + if (rawFormat !== undefined && (typeof rawFormat !== 'string' || rawFormat === '')) { + return ctx.errorResponse(res, 400, + `Invalid format parameter. Use: ${CA_CERT_FORMATS.join(', ')}.`, + { code: 'DC-076_FORMAT_INVALID' }); + } + if (rawFormat !== undefined && !CA_CERT_FORMATS.includes(rawFormat)) { + return ctx.errorResponse(res, 400, + `Invalid format '${rawFormat}'. Use: ${CA_CERT_FORMATS.join(', ')}.`, + { code: 'DC-076_FORMAT_INVALID' }); + } + const format = rawFormat || 'pfx'; + // DC-076: password is REQUIRED for the pfx format (no `=`) and must // be ≥ 8 chars. Previously `password = 'dashcaddy'` — a hardcoded // default that silently signed every PFX with the same published diff --git a/dashcaddy-api/routes/caddycode.js b/dashcaddy-api/routes/caddycode.js index eb1756f..4fda1c1 100644 --- a/dashcaddy-api/routes/caddycode.js +++ b/dashcaddy-api/routes/caddycode.js @@ -64,8 +64,8 @@ function validateGenerationConfig(config) { // reverse_proxy upstreams). Two regex branches: (a) bare host with // required :port, (b) bracketed IPv6 literal with required :port. if (typeof upstream !== 'string' - || !/^[a-z0-9.\-]+:\d{1,5}$/i.test(upstream) - && !/^\[[a-z0-9.\-:.]+\]:\d{1,5}$/i.test(upstream) + || !/^[a-z0-9.-]+:\d{1,5}$/i.test(upstream) + && !/^\[[a-z0-9.:.-]+\]:\d{1,5}$/i.test(upstream) ) { errors.push('upstream must be host:port (host letters/digits/dots/hyphens, port 1-65535, optional IPv6 brackets)'); } diff --git a/dashcaddy-api/routes/logs.js b/dashcaddy-api/routes/logs.js index 92af6c8..3aafd88 100644 --- a/dashcaddy-api/routes/logs.js +++ b/dashcaddy-api/routes/logs.js @@ -270,14 +270,10 @@ module.exports = function({ asyncHandler, ok, docker, logDigest, dockerMaintenan // can't change statusCode. The reader does the same validation but // we want to short-circuit here so the response status reflects the // right category (400 for validation, 503 for bind-mount missing). - try { - journald.assertUnitAllowed(req.query.unit); - if (req.query.since) journald.parseTimestamp(req.query.since, 'since'); - } catch (err) { - // Pass through the global error middleware so the response status - // + shape matches every other validation error in the API. - throw err; - } + // Throws pass straight to the global error middleware so the response + // status + shape matches every other validation error in the API. + journald.assertUnitAllowed(req.query.unit); + if (req.query.since) journald.parseTimestamp(req.query.since, 'since'); // SSE headers — same convention as /logs/stream/:id. res.setHeader('Content-Type', 'text/event-stream'); diff --git a/dashcaddy-api/routes/openclaw.js b/dashcaddy-api/routes/openclaw.js index 3a9f65a..27eb1e4 100644 --- a/dashcaddy-api/routes/openclaw.js +++ b/dashcaddy-api/routes/openclaw.js @@ -147,7 +147,7 @@ module.exports = function openClawRoutes(ctx) { // Allowed chars in the downstream `path` segment: alphanumerics, `-`, `_`, // `.`, `~`, `/`, `?`, `&`, `=`, `:`, `@`, `+`, `,`, `;` (RFC 3986 pchar + // query/fragment separators). Anything else → 400. - const ALLOWED_PATH_RE = /^[a-zA-Z0-9._~/?&=:@+,;%\-]*$/; + const ALLOWED_PATH_RE = /^[a-zA-Z0-9._~/?&=:@+,;%-]*$/; // Maximum total `path` length (reasonable for a gateway UI endpoint). const MAX_PATH_LEN = 1024; diff --git a/dashcaddy-api/src/utilities/fleet-validation.js b/dashcaddy-api/src/utilities/fleet-validation.js index 8fa20d3..8b36fcb 100644 --- a/dashcaddy-api/src/utilities/fleet-validation.js +++ b/dashcaddy-api/src/utilities/fleet-validation.js @@ -300,6 +300,7 @@ function validateFleetHost(input) { } // Disallow control chars in name (newlines would let a stored name break // log-file formats and could enable log injection if not properly escaped). + // eslint-disable-next-line no-control-regex -- intentionally matching control chars if (/[\x00-\x1f]/.test(name)) { return { ok: false, @@ -394,6 +395,7 @@ function validateFleetHost(input) { message: 'each tag must be a string of 1..50 characters', }; } + // eslint-disable-next-line no-control-regex -- intentionally matching control chars if (/[\x00-\x1f]/.test(t)) { return { ok: false, diff --git a/dashcaddy-installer/D b/dashcaddy-installer/D deleted file mode 100644 index e69de29..0000000 diff --git a/dashcaddy-installer/q b/dashcaddy-installer/q deleted file mode 100644 index e69de29..0000000