mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-07 09:24:09 +00:00
fix(stacks): return 404 for nonexistent stacks on deploy/down/update (F-7) (#1108)
POST /api/stacks/:name/{deploy,down,update} previously returned HTTP 500
with body {"error":"spawn docker ENOENT"} when invoked against a stack
whose compose directory was missing. The status code was wrong (the
named resource did not exist, so 404 is the right answer) and the
message misled operators into thinking the docker CLI was unavailable.
Add a small requireStackExists(nodeId, stackName, res) helper in
routes/stacks.ts that validates the stack name and confirms a compose
file is present via FileSystemService.hasComposeFile before any of the
three handlers spawn docker compose. The helper is called immediately
after requirePermission and before runPolicyGate so unauthorized
callers still get 403 first and the policy gate never runs against a
phantom stack.
In ComposeService.execute(), narrow the child.on('error') handler so
the genuine docker-binary-missing case (ENOENT on the spawn itself)
rejects with "Docker CLI unavailable on this node" instead of the raw
"spawn docker ENOENT". This is defense in depth for the rare case the
pre-check cannot cover, and it fixes the misleading-message half of
the bug as well.
Cover the new contract with stack-actions-missing-stack.test.ts (four
cases: deploy/down/update return 404, invalid name returns 400). Mock
ComposeService as a tripwire so a future code path that bypasses the
guard would fail loudly. Fix stacks-failure-notifications.test.ts by
adding hasComposeFile to its FileSystemService partial mock so the
existing happy-path-error-handling cases continue to flow into
ComposeService.
This commit is contained in:
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* Integration tests for the stack-action 404 contract (F-7).
|
||||
*
|
||||
* Verifies that POST /api/stacks/:stackName/{deploy,down,update} returns
|
||||
* HTTP 404 with `{ error: 'Stack not found' }` when the named stack has no
|
||||
* compose file under COMPOSE_DIR, instead of allowing the request to flow
|
||||
* into ComposeService and surface the raw `spawn docker ENOENT` from the
|
||||
* child_process spawn cwd failure.
|
||||
*
|
||||
* ComposeService is mocked to act as a tripwire: if any of these endpoints
|
||||
* ever reach the service layer for a nonexistent stack, the mock assertion
|
||||
* will fail loudly.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
|
||||
import request from 'supertest';
|
||||
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
|
||||
|
||||
const {
|
||||
mockDeployStack,
|
||||
mockRunCommand,
|
||||
mockUpdateStack,
|
||||
} = vi.hoisted(() => ({
|
||||
mockDeployStack: vi.fn(),
|
||||
mockRunCommand: vi.fn(),
|
||||
mockUpdateStack: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../services/ComposeService', async () => {
|
||||
const actual = await vi.importActual<typeof import('../services/ComposeService')>(
|
||||
'../services/ComposeService',
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
ComposeService: {
|
||||
...actual.ComposeService,
|
||||
getInstance: () => ({
|
||||
deployStack: mockDeployStack,
|
||||
runCommand: mockRunCommand,
|
||||
updateStack: mockUpdateStack,
|
||||
}),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
let tmpDir: string;
|
||||
let app: import('express').Express;
|
||||
let authCookie: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ app } = await import('../index'));
|
||||
authCookie = await loginAsTestAdmin(app);
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
vi.restoreAllMocks();
|
||||
cleanupTestDb(tmpDir);
|
||||
});
|
||||
|
||||
describe('POST /api/stacks/:stackName/deploy on a nonexistent stack', () => {
|
||||
it('returns 404 with "Stack not found" and never enters ComposeService.deployStack', async () => {
|
||||
mockDeployStack.mockClear();
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/stacks/does-not-exist-f7/deploy')
|
||||
.set('Cookie', authCookie);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Stack not found' });
|
||||
expect(mockDeployStack).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/stacks/:stackName/down on a nonexistent stack', () => {
|
||||
it('returns 404 with "Stack not found" and never enters ComposeService.runCommand', async () => {
|
||||
mockRunCommand.mockClear();
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/stacks/does-not-exist-f7/down')
|
||||
.set('Cookie', authCookie);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Stack not found' });
|
||||
expect(mockRunCommand).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/stacks/:stackName/update on a nonexistent stack', () => {
|
||||
it('returns 404 with "Stack not found" and never enters ComposeService.updateStack', async () => {
|
||||
mockUpdateStack.mockClear();
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/stacks/does-not-exist-f7/update')
|
||||
.set('Cookie', authCookie);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Stack not found' });
|
||||
expect(mockUpdateStack).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Invalid stack names are rejected with 400 before the existence check', () => {
|
||||
it('POST /api/stacks/..bad../deploy returns 400 Invalid stack name', async () => {
|
||||
mockDeployStack.mockClear();
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/stacks/..bad../deploy')
|
||||
.set('Cookie', authCookie);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toEqual({ error: 'Invalid stack name' });
|
||||
expect(mockDeployStack).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -99,6 +99,7 @@ vi.mock('../services/FileSystemService', () => ({
|
||||
getStacks: vi.fn().mockResolvedValue([]),
|
||||
getBaseDir: () => '/tmp/compose',
|
||||
readComposeFile: vi.fn().mockResolvedValue(''),
|
||||
hasComposeFile: vi.fn().mockResolvedValue(true),
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -37,6 +37,25 @@ function notifyActionSuccess(category: NotificationCategory, message: string, st
|
||||
.catch(err => console.error('[Stacks] Failed to dispatch activity for %s:', sanitizeForLog(stackName), err));
|
||||
}
|
||||
|
||||
async function requireStackExists(nodeId: number, stackName: string, res: Response): Promise<boolean> {
|
||||
if (!isValidStackName(stackName)) {
|
||||
res.status(400).json({ error: 'Invalid stack name' });
|
||||
return false;
|
||||
}
|
||||
const fsSvc = FileSystemService.getInstance(nodeId);
|
||||
const stackDir = path.join(fsSvc.getBaseDir(), stackName);
|
||||
try {
|
||||
if (!(await fsSvc.hasComposeFile(stackDir))) {
|
||||
res.status(404).json({ error: 'Stack not found' });
|
||||
return false;
|
||||
}
|
||||
} catch {
|
||||
res.status(404).json({ error: 'Stack not found' });
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export async function resolveAllEnvFilePaths(nodeId: number, stackName: string): Promise<string[]> {
|
||||
const fsService = FileSystemService.getInstance(nodeId);
|
||||
const stackDir = path.join(fsService.getBaseDir(), stackName);
|
||||
@@ -620,6 +639,7 @@ stacksRouter.get('/:stackName/services', async (req: Request, res: Response) =>
|
||||
stacksRouter.post('/:stackName/deploy', async (req: Request, res: Response) => {
|
||||
const stackName = req.params.stackName as string;
|
||||
if (!requirePermission(req, res, 'stack:deploy', 'stack', stackName)) return;
|
||||
if (!(await requireStackExists(req.nodeId, stackName, res))) return;
|
||||
try {
|
||||
if (!(await runPolicyGate(req, res, stackName, req.nodeId))) return;
|
||||
const skipScan = req.body?.skip_scan === true;
|
||||
@@ -656,6 +676,7 @@ stacksRouter.post('/:stackName/deploy', async (req: Request, res: Response) => {
|
||||
stacksRouter.post('/:stackName/down', async (req: Request, res: Response) => {
|
||||
const stackName = req.params.stackName as string;
|
||||
if (!requirePermission(req, res, 'stack:deploy', 'stack', stackName)) return;
|
||||
if (!(await requireStackExists(req.nodeId, stackName, res))) return;
|
||||
try {
|
||||
await ComposeService.getInstance(req.nodeId).runCommand(stackName, 'down', getTerminalWs());
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
@@ -802,6 +823,7 @@ stacksRouter.get('/:stackName/update-preview', async (req: Request, res: Respons
|
||||
stacksRouter.post('/:stackName/update', async (req: Request, res: Response) => {
|
||||
const stackName = req.params.stackName as string;
|
||||
if (!requirePermission(req, res, 'stack:deploy', 'stack', stackName)) return;
|
||||
if (!(await requireStackExists(req.nodeId, stackName, res))) return;
|
||||
try {
|
||||
if (!(await runPolicyGate(req, res, stackName, req.nodeId))) return;
|
||||
const skipScan = req.body?.skip_scan === true;
|
||||
|
||||
@@ -199,16 +199,20 @@ export class ComposeService {
|
||||
});
|
||||
});
|
||||
|
||||
child.on('error', (error: Error) => {
|
||||
child.on('error', (error: Error & { code?: string }) => {
|
||||
exited = true;
|
||||
finish(() => {
|
||||
sendOutput(`Error: ${redactSensitiveText(error.message)}\n`);
|
||||
let message = redactSensitiveText(error.message);
|
||||
if (error.code === 'ENOENT' && /^spawn docker(?:$| )/.test(error.message)) {
|
||||
message = 'Docker CLI unavailable on this node';
|
||||
}
|
||||
sendOutput(`Error: ${message}\n`);
|
||||
if (pendingTerminationError) {
|
||||
if (throwOnError) reject(pendingTerminationError);
|
||||
else resolve();
|
||||
return;
|
||||
}
|
||||
if (throwOnError) reject(new Error(redactSensitiveText(error.message)));
|
||||
if (throwOnError) reject(new Error(message));
|
||||
else resolve();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user