Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
99ec6ebc53 |
@@ -131,6 +131,20 @@ describe('routes/tailscale-admin: PUT /settings', () => {
|
|||||||
expect(res.status).toBe(400);
|
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 () => {
|
test('200 + saves token + writes metadata on valid token', async () => {
|
||||||
const fakeClient = makeFakeClient({
|
const fakeClient = makeFakeClient({
|
||||||
ping: jest.fn(async () => ({ domain: 'real.ts.net' })),
|
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(res.body.valid).toBe(true);
|
||||||
expect(fakeClient.setApiToken).toHaveBeenCalledWith('tskey-api-test-only');
|
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', () => {
|
describe('routes/tailscale-admin: GET /admin/devices', () => {
|
||||||
@@ -511,6 +595,99 @@ describe('routes/tailscale-admin: pre-auth keys', () => {
|
|||||||
expect(res.status).toBe(400);
|
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 () => {
|
test('POST /admin/keys rejects negative expirySeconds', async () => {
|
||||||
const fakeClient = makeFakeClient();
|
const fakeClient = makeFakeClient();
|
||||||
const app = express();
|
const app = express();
|
||||||
@@ -572,4 +749,110 @@ describe('routes/tailscale-admin: security boundary', () => {
|
|||||||
await request(app).delete('/api/v1/tailscale/settings');
|
await request(app).delete('/api/v1/tailscale/settings');
|
||||||
expect(stored.token).toBeNull();
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -41,12 +41,124 @@
|
|||||||
*
|
*
|
||||||
* DELETE /api/v1/tailscale/admin/devices/:id
|
* DELETE /api/v1/tailscale/admin/devices/:id
|
||||||
* Revokes a device from the tailnet.
|
* 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 express = require('express');
|
||||||
const { ok, errorResponse } = require('../src/utils/responses');
|
const { ok, errorResponse } = require('../src/utils/responses');
|
||||||
const { TailscaleCoordError } = require('../src/managers/tailscale-coord');
|
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({
|
module.exports = function({
|
||||||
tailscaleCoord,
|
tailscaleCoord,
|
||||||
asyncHandler,
|
asyncHandler,
|
||||||
@@ -75,9 +187,12 @@ module.exports = function({
|
|||||||
|
|
||||||
router.put('/settings', asyncHandler(async (req, res) => {
|
router.put('/settings', asyncHandler(async (req, res) => {
|
||||||
const token = req.body && req.body.apiToken;
|
const token = req.body && req.body.apiToken;
|
||||||
if (!token || typeof token !== 'string' || !token.startsWith('tskey-api-')) {
|
// DC-080: validate prefix + length cap. The pre-fix code only checked
|
||||||
return errorResponse(res, 400, 'Invalid API token (must start with tskey-api-)');
|
// 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
|
// Validate before storing
|
||||||
const client = new (require('../src/managers/tailscale-coord').TailscaleCoordClient)({ apiToken: token });
|
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) => {
|
router.post('/settings/test', asyncHandler(async (req, res) => {
|
||||||
const token = (req.body && req.body.apiToken) || null;
|
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();
|
const client = await tailscaleCoord.getClient();
|
||||||
if (token) {
|
if (token) {
|
||||||
// Caller provided a fresh token to test — don't save it
|
// 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');
|
return errorResponse(res, 503, 'Tailscale API token not configured');
|
||||||
}
|
}
|
||||||
const opts = req.body || {};
|
const opts = req.body || {};
|
||||||
// Reject obviously-bad input early
|
// Reject obviously-bad input early.
|
||||||
if (opts.tags && !Array.isArray(opts.tags)) {
|
// DC-080: pre-fix the route only checked `Array.isArray(opts.tags)`.
|
||||||
return errorResponse(res, 400, 'tags must be an array of strings');
|
// 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)) {
|
if (opts.expirySeconds !== undefined && (!Number.isInteger(opts.expirySeconds) || opts.expirySeconds <= 0)) {
|
||||||
return errorResponse(res, 400, 'expirySeconds must be a positive integer');
|
return errorResponse(res, 400, 'expirySeconds must be a positive integer');
|
||||||
}
|
}
|
||||||
@@ -254,4 +386,10 @@ module.exports = function({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
return router;
|
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;
|
||||||
Reference in New Issue
Block a user