mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-13 12:17:34 +00:00
feat: validate scheduled task target existence at creation time (#1757)
* feat: validate scheduled task target existence at creation time
Reject POST/PUT /api/scheduled-tasks with 400 when the target stack,
container, or node does not exist. Previously only structural format was
validated; a task targeting a deleted stack would return 201 and fail
forever at execution time with noisy error logs.
Node existence is now validated for every action that requires a node
(previously only scan/prune got this check). Stack and container
existence is validated on local nodes; remote nodes are skipped since
the check would require a proxy call (execution-time validation still
serves as the safety net there).
Permission checks run before existence checks, so unauthorized callers
receive 403 regardless of whether the target exists.
* fix: close node-existence oracle in scheduled task creation
Move validateActionNode (which contains a getNode DB lookup) after the
permission gate in both POST and PUT handlers, so unauthorized callers
always receive 403 regardless of whether a fleet or system target node
exists. Previously a viewer probing a nonexistent fleet node would get
400 ("node not found"), leaking node ID enumeration through the error
code difference.
The stack/container existence check (validateTargetExists) was already
correctly positioned after permission; this fix extends the same
discipline to the node-existence path.
This commit is contained in:
@@ -7,6 +7,8 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
|
||||
import request from 'supertest';
|
||||
import bcrypt from 'bcrypt';
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
|
||||
|
||||
let tmpDir: string;
|
||||
@@ -67,6 +69,13 @@ beforeAll(async () => {
|
||||
else auditorCookie = c;
|
||||
}
|
||||
|
||||
// Seed real stack directories so existence validators pass.
|
||||
const composeDir = path.join(tmpDir, 'compose');
|
||||
for (const name of ['web', 'api']) {
|
||||
fs.mkdirSync(path.join(composeDir, name), { recursive: true });
|
||||
fs.writeFileSync(path.join(composeDir, name, 'compose.yaml'), 'version: "3"\n');
|
||||
}
|
||||
|
||||
// Insert a local node for stack-target fixtures
|
||||
for (const [nodeName, nodeType] of [['local-test', 'local'], ['remote-test', 'remote']] as const) {
|
||||
const existing = db.getDb().prepare('SELECT id FROM nodes WHERE name = ?').get(nodeName) as { id: number } | undefined;
|
||||
|
||||
@@ -6,6 +6,8 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
|
||||
import request from 'supertest';
|
||||
import bcrypt from 'bcrypt';
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
|
||||
|
||||
let tmpDir: string;
|
||||
@@ -30,6 +32,24 @@ beforeAll(async () => {
|
||||
const viewerRes = await request(app).post('/api/auth/login').send({ username: 'sched-viewer', password: 'viewerpass' });
|
||||
const cookies = viewerRes.headers['set-cookie'] as string | string[];
|
||||
viewerCookie = Array.isArray(cookies) ? cookies[0] : cookies;
|
||||
|
||||
// Seed real stack directories so existence validators pass.
|
||||
const composeDir = path.join(tmpDir, 'compose');
|
||||
for (const name of ['my-stack', 's']) {
|
||||
fs.mkdirSync(path.join(composeDir, name), { recursive: true });
|
||||
fs.writeFileSync(path.join(composeDir, name, 'compose.yaml'), 'version: "3"\n');
|
||||
}
|
||||
|
||||
// Mock DockerController.findContainerByName so container existence checks pass
|
||||
// in tests (no real Docker daemon available). Only resolve for names used by
|
||||
// the test fixtures; everything else returns null to exercise the 400 path.
|
||||
const { default: DockerController } = await import('../services/DockerController');
|
||||
const containerFixture = { id: 'abc123test', name: 'test-container', state: 'running', image: 'test:latest', stackProject: null };
|
||||
vi.spyOn(DockerController.prototype, 'findContainerByName')
|
||||
.mockImplementation(async (name: string) => {
|
||||
if (name === 'watchtower' || name === 'sidecar') return { ...containerFixture, name };
|
||||
return null;
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(() => cleanupTestDb(tmpDir));
|
||||
@@ -178,6 +198,15 @@ describe('POST /api/scheduled-tasks', () => {
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('returns 403 (not 400) for unauthorized caller on nonexistent target', async () => {
|
||||
// Permissions check runs first; an unauthorized caller must not learn
|
||||
// whether a stack exists through the error code difference.
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', viewerCookie).send({
|
||||
...basePayload, target_id: 'nonexistent-stack',
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('creates a task and returns the new record', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send(basePayload);
|
||||
expect(res.status).toBe(201);
|
||||
@@ -202,6 +231,52 @@ describe('POST /api/scheduled-tasks', () => {
|
||||
expect(res.body.error).toMatch(/5 fields/);
|
||||
});
|
||||
|
||||
it('rejects a nonexistent node_id with 400', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, node_id: 9999,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/require an existing node/);
|
||||
});
|
||||
|
||||
it('returns 403 (not 400) for unauthorized caller probing nonexistent node via fleet update', async () => {
|
||||
// A viewer must never learn whether a node ID exists through the error
|
||||
// code difference (400 "node doesn't exist" vs 403 "permission denied").
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', viewerCookie).send({
|
||||
name: 'probe-node',
|
||||
action: 'update',
|
||||
target_type: 'fleet',
|
||||
node_id: 999999,
|
||||
cron_expression: '0 0 * * *',
|
||||
enabled: true,
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('returns same 403 for unauthorized caller regardless of node existence', async () => {
|
||||
const resNonexistent = await request(app).post('/api/scheduled-tasks')
|
||||
.set('Cookie', viewerCookie).send({
|
||||
name: 'probe-nonexistent', action: 'update', target_type: 'fleet',
|
||||
node_id: 999999, cron_expression: '0 0 * * *', enabled: true,
|
||||
});
|
||||
const resExisting = await request(app).post('/api/scheduled-tasks')
|
||||
.set('Cookie', viewerCookie).send({
|
||||
name: 'probe-existing', action: 'update', target_type: 'fleet',
|
||||
node_id: 1, cron_expression: '0 0 * * *', enabled: true,
|
||||
});
|
||||
expect(resNonexistent.status).toBe(403);
|
||||
expect(resExisting.status).toBe(403);
|
||||
expect(resNonexistent.body.error).toBe(resExisting.body.error);
|
||||
});
|
||||
|
||||
it('rejects a nonexistent stack target with 400', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, target_id: 'nonexistent-stack',
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/not found on the target node/);
|
||||
});
|
||||
|
||||
it('rejects a missing cron expression with a clear message', async () => {
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, cron_expression: undefined,
|
||||
@@ -616,6 +691,23 @@ describe('POST /api/scheduled-tasks - container lifecycle', () => {
|
||||
expect(res.body.action).toBe('restart');
|
||||
});
|
||||
|
||||
it('rejects a nonexistent container target with 400', async () => {
|
||||
const res = await request(app)
|
||||
.post('/api/scheduled-tasks')
|
||||
.set('Cookie', adminCookie)
|
||||
.send({
|
||||
name: 'missing-ctr',
|
||||
target_type: 'container',
|
||||
target_id: 'nonexistent-container',
|
||||
node_id: 1,
|
||||
action: 'restart',
|
||||
cron_expression: '0 3 * * *',
|
||||
enabled: true,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/not found on the target node/);
|
||||
});
|
||||
|
||||
it('rejects invalid container names', async () => {
|
||||
const res = await request(app)
|
||||
.post('/api/scheduled-tasks')
|
||||
|
||||
Reference in New Issue
Block a user