[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.
This commit is contained in:
@@ -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', () => {
|
describe('/cert/:domain — domain validation', () => {
|
||||||
test('rejects single-label domain (no dot)', async () => {
|
test('rejects single-label domain (no dot)', async () => {
|
||||||
const { app } = createCaApp({ scope: ['admin'] });
|
const { app } = createCaApp({ scope: ['admin'] });
|
||||||
|
|||||||
@@ -53,6 +53,7 @@ function stripComments(src) {
|
|||||||
.replace(/\/\*[\s\S]*?\*\//g, '') // block comments
|
.replace(/\/\*[\s\S]*?\*\//g, '') // block comments
|
||||||
.replace(/(^|[^:])\/\/.*$/gm, '$1'); // line comments, leaving URLs alone
|
.replace(/(^|[^:])\/\/.*$/gm, '$1'); // line comments, leaving URLs alone
|
||||||
// Pass 3: restore template literals.
|
// 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]);
|
return protectedSrc.replace(/\u0000TPL(\d+)\u0000/g, (_, idx) => templates[+idx]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -139,6 +139,9 @@ module.exports = function(ctx) {
|
|||||||
// can mis-handle; reject it to keep the password copy-paste-safe).
|
// 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_PFX_PASSWORD_RE = /^[A-Za-z0-9!@#%^_+,.~:-]{8,64}$/;
|
||||||
const CA_CERT_RATE_LIMIT = { windowMs: 60_000, max: 10 };
|
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 }
|
const caCertRateBuckets = new Map(); // ip -> { count, resetAt }
|
||||||
function caCertRateLimit(ip) {
|
function caCertRateLimit(ip) {
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
@@ -175,6 +178,24 @@ module.exports = function(ctx) {
|
|||||||
|
|
||||||
const { domain } = req.params;
|
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
|
// DC-076: password is REQUIRED for the pfx format (no `=`) and must
|
||||||
// be ≥ 8 chars. Previously `password = 'dashcaddy'` — a hardcoded
|
// be ≥ 8 chars. Previously `password = 'dashcaddy'` — a hardcoded
|
||||||
// default that silently signed every PFX with the same published
|
// default that silently signed every PFX with the same published
|
||||||
|
|||||||
@@ -64,8 +64,8 @@ function validateGenerationConfig(config) {
|
|||||||
// reverse_proxy upstreams). Two regex branches: (a) bare host with
|
// reverse_proxy upstreams). Two regex branches: (a) bare host with
|
||||||
// required :port, (b) bracketed IPv6 literal with required :port.
|
// required :port, (b) bracketed IPv6 literal with required :port.
|
||||||
if (typeof upstream !== 'string'
|
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)');
|
errors.push('upstream must be host:port (host letters/digits/dots/hyphens, port 1-65535, optional IPv6 brackets)');
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -270,14 +270,10 @@ module.exports = function({ asyncHandler, ok, docker, logDigest, dockerMaintenan
|
|||||||
// can't change statusCode. The reader does the same validation but
|
// can't change statusCode. The reader does the same validation but
|
||||||
// we want to short-circuit here so the response status reflects the
|
// we want to short-circuit here so the response status reflects the
|
||||||
// right category (400 for validation, 503 for bind-mount missing).
|
// right category (400 for validation, 503 for bind-mount missing).
|
||||||
try {
|
// 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);
|
journald.assertUnitAllowed(req.query.unit);
|
||||||
if (req.query.since) journald.parseTimestamp(req.query.since, 'since');
|
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;
|
|
||||||
}
|
|
||||||
|
|
||||||
// SSE headers — same convention as /logs/stream/:id.
|
// SSE headers — same convention as /logs/stream/:id.
|
||||||
res.setHeader('Content-Type', 'text/event-stream');
|
res.setHeader('Content-Type', 'text/event-stream');
|
||||||
|
|||||||
@@ -147,7 +147,7 @@ module.exports = function openClawRoutes(ctx) {
|
|||||||
// Allowed chars in the downstream `path` segment: alphanumerics, `-`, `_`,
|
// Allowed chars in the downstream `path` segment: alphanumerics, `-`, `_`,
|
||||||
// `.`, `~`, `/`, `?`, `&`, `=`, `:`, `@`, `+`, `,`, `;` (RFC 3986 pchar +
|
// `.`, `~`, `/`, `?`, `&`, `=`, `:`, `@`, `+`, `,`, `;` (RFC 3986 pchar +
|
||||||
// query/fragment separators). Anything else → 400.
|
// 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).
|
// Maximum total `path` length (reasonable for a gateway UI endpoint).
|
||||||
const MAX_PATH_LEN = 1024;
|
const MAX_PATH_LEN = 1024;
|
||||||
|
|
||||||
|
|||||||
@@ -300,6 +300,7 @@ function validateFleetHost(input) {
|
|||||||
}
|
}
|
||||||
// Disallow control chars in name (newlines would let a stored name break
|
// Disallow control chars in name (newlines would let a stored name break
|
||||||
// log-file formats and could enable log injection if not properly escaped).
|
// 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)) {
|
if (/[\x00-\x1f]/.test(name)) {
|
||||||
return {
|
return {
|
||||||
ok: false,
|
ok: false,
|
||||||
@@ -394,6 +395,7 @@ function validateFleetHost(input) {
|
|||||||
message: 'each tag must be a string of 1..50 characters',
|
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)) {
|
if (/[\x00-\x1f]/.test(t)) {
|
||||||
return {
|
return {
|
||||||
ok: false,
|
ok: false,
|
||||||
|
|||||||
Reference in New Issue
Block a user