mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-10 10:49:35 +00:00
fix(resources): harden Resource Explorer with auth, validation, design, and UX fixes (#527)
- Sanitize error messages in all delete/prune/create/inspect endpoints to prevent Docker internals from leaking to the frontend - Add CIDR, IPv4, and Docker resource ID input validation - Add requirePaid gate to network topology endpoint - Add invalidateNodeCaches after image/volume/network mutations - Fix design system violations: card borders, destructive button variant, visible DialogDescription, overflow-auto replaced with ScrollArea, hardcoded Tailwind colors replaced with tokens - Gate purge button behind isAdmin to prevent silent 403s - Fix shared inspect loading state to be per-network-row - Parse error response bodies for meaningful toast messages - Add clipboard API fallback for non-HTTPS contexts - Render Options section in network inspect sheet - Add operational and diagnostic logging for resource operations - Extend validation and DockerController test suites - Update docs with Options field in network inspect
This commit is contained in:
@@ -555,3 +555,52 @@ describe('DockerController - inspectNetwork edge cases', () => {
|
||||
await expect(dc.inspectNetwork('any-id')).rejects.toThrow('Cannot connect to Docker daemon');
|
||||
});
|
||||
});
|
||||
|
||||
// --- createNetwork validation --------------------------------------------------
|
||||
|
||||
describe('createNetwork', () => {
|
||||
it('rejects empty network name', async () => {
|
||||
const dc = DockerController.getInstance(1);
|
||||
await expect(dc.createNetwork({ Name: '' })).rejects.toThrow('Invalid network name');
|
||||
});
|
||||
|
||||
it('rejects names with invalid characters', async () => {
|
||||
const dc = DockerController.getInstance(1);
|
||||
await expect(dc.createNetwork({ Name: 'net work' })).rejects.toThrow('Invalid network name');
|
||||
await expect(dc.createNetwork({ Name: '../escape' })).rejects.toThrow('Invalid network name');
|
||||
await expect(dc.createNetwork({ Name: 'net;rm' })).rejects.toThrow('Invalid network name');
|
||||
});
|
||||
|
||||
it('accepts valid network names and passes through to Docker', async () => {
|
||||
mockDocker.createNetwork.mockResolvedValue({ id: 'new-net-id' });
|
||||
const dc = DockerController.getInstance(1);
|
||||
const result = await dc.createNetwork({ Name: 'my-network_v2' });
|
||||
expect(result.id).toBe('new-net-id');
|
||||
expect(mockDocker.createNetwork).toHaveBeenCalledWith({ Name: 'my-network_v2' });
|
||||
});
|
||||
});
|
||||
|
||||
// --- removeContainers mixed results -------------------------------------------
|
||||
|
||||
describe('removeContainers', () => {
|
||||
it('returns mixed results when some removals fail', async () => {
|
||||
mockDocker.getContainer.mockImplementation((id: string) => {
|
||||
if (id === 'fail-id') {
|
||||
return { remove: vi.fn().mockRejectedValue(new Error('no such container')) };
|
||||
}
|
||||
return { remove: vi.fn().mockResolvedValue(undefined) };
|
||||
});
|
||||
|
||||
const dc = DockerController.getInstance(1);
|
||||
const results = await dc.removeContainers(['ok-id-000000', 'fail-id']);
|
||||
expect(results).toHaveLength(2);
|
||||
expect(results[0]).toEqual({ id: 'ok-id-000000', success: true });
|
||||
expect(results[1]).toMatchObject({ id: 'fail-id', success: false });
|
||||
});
|
||||
|
||||
it('returns empty array for empty input', async () => {
|
||||
const dc = DockerController.getInstance(1);
|
||||
const results = await dc.removeContainers([]);
|
||||
expect(results).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { isValidStackName, isValidRemoteUrl, isPathWithinBase } from '../utils/validation';
|
||||
import {
|
||||
isValidStackName, isValidRemoteUrl, isPathWithinBase,
|
||||
isValidCidr, isValidIPv4, isValidDockerResourceId,
|
||||
} from '../utils/validation';
|
||||
|
||||
// ─── isValidStackName ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -122,3 +125,103 @@ describe('isPathWithinBase', () => {
|
||||
expect(isPathWithinBase('/app/compose/other-stack/.env', '/app/compose/mystack')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// --- isValidCidr ---------------------------------------------------------------
|
||||
|
||||
describe('isValidCidr', () => {
|
||||
it('accepts valid CIDR notation', () => {
|
||||
expect(isValidCidr('10.0.0.0/24')).toBe(true);
|
||||
expect(isValidCidr('172.16.0.0/16')).toBe(true);
|
||||
expect(isValidCidr('192.168.1.0/28')).toBe(true);
|
||||
expect(isValidCidr('0.0.0.0/0')).toBe(true);
|
||||
expect(isValidCidr('255.255.255.255/32')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects missing prefix', () => {
|
||||
expect(isValidCidr('10.0.0.0')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects prefix out of range', () => {
|
||||
expect(isValidCidr('10.0.0.0/33')).toBe(false);
|
||||
expect(isValidCidr('10.0.0.0/99')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects octet out of range', () => {
|
||||
expect(isValidCidr('256.0.0.0/24')).toBe(false);
|
||||
expect(isValidCidr('10.999.0.0/16')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects empty and garbage input', () => {
|
||||
expect(isValidCidr('')).toBe(false);
|
||||
expect(isValidCidr('not-a-cidr')).toBe(false);
|
||||
expect(isValidCidr('10.0.0/24')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// --- isValidIPv4 ---------------------------------------------------------------
|
||||
|
||||
describe('isValidIPv4', () => {
|
||||
it('accepts valid IPv4 addresses', () => {
|
||||
expect(isValidIPv4('10.0.0.1')).toBe(true);
|
||||
expect(isValidIPv4('192.168.1.1')).toBe(true);
|
||||
expect(isValidIPv4('0.0.0.0')).toBe(true);
|
||||
expect(isValidIPv4('255.255.255.255')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects incomplete addresses', () => {
|
||||
expect(isValidIPv4('10.0.0')).toBe(false);
|
||||
expect(isValidIPv4('10')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects octet out of range', () => {
|
||||
expect(isValidIPv4('256.1.2.3')).toBe(false);
|
||||
expect(isValidIPv4('10.0.0.999')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects CIDR notation (use isValidCidr instead)', () => {
|
||||
expect(isValidIPv4('10.0.0.1/24')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects empty and garbage input', () => {
|
||||
expect(isValidIPv4('')).toBe(false);
|
||||
expect(isValidIPv4('not-an-ip')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// --- isValidDockerResourceId ---------------------------------------------------
|
||||
|
||||
describe('isValidDockerResourceId', () => {
|
||||
it('accepts 12-character hex IDs (short form)', () => {
|
||||
expect(isValidDockerResourceId('a1b2c3d4e5f6')).toBe(true);
|
||||
expect(isValidDockerResourceId('AABB00112233')).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts 64-character hex IDs (full SHA256)', () => {
|
||||
expect(isValidDockerResourceId('a'.repeat(64))).toBe(true);
|
||||
expect(isValidDockerResourceId('abcdef0123456789'.repeat(4))).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts mixed-case hex of valid lengths', () => {
|
||||
expect(isValidDockerResourceId('aAbBcCdDeEfF')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects IDs shorter than 12 characters', () => {
|
||||
expect(isValidDockerResourceId('a1b2c3d4e5f')).toBe(false);
|
||||
expect(isValidDockerResourceId('')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects IDs longer than 64 characters', () => {
|
||||
expect(isValidDockerResourceId('a'.repeat(65))).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects non-hex characters', () => {
|
||||
expect(isValidDockerResourceId('g1b2c3d4e5f6')).toBe(false);
|
||||
expect(isValidDockerResourceId('hello-world!')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects IDs with slashes, dots, or spaces', () => {
|
||||
expect(isValidDockerResourceId('a1b2c3/d4e5f6')).toBe(false);
|
||||
expect(isValidDockerResourceId('a1b2c3.d4e5f6')).toBe(false);
|
||||
expect(isValidDockerResourceId('a1b2c3 d4e5f6')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
+47
-19
@@ -62,7 +62,7 @@ import { getErrorMessage } from './utils/errors';
|
||||
import SelfUpdateService from './services/SelfUpdateService';
|
||||
import semver from 'semver';
|
||||
import { CronExpressionParser } from 'cron-parser';
|
||||
import { isValidStackName, isValidRemoteUrl, isPathWithinBase } from './utils/validation';
|
||||
import { isValidStackName, isValidRemoteUrl, isPathWithinBase, isValidCidr, isValidIPv4, isValidDockerResourceId } from './utils/validation';
|
||||
import YAML from 'yaml';
|
||||
import { promises as fsPromises } from 'fs';
|
||||
|
||||
@@ -5414,8 +5414,15 @@ app.post('/api/system/prune/orphans', async (req: Request, res: Response) => {
|
||||
if (!Array.isArray(containerIds)) {
|
||||
return res.status(400).json({ error: 'containerIds must be an array' });
|
||||
}
|
||||
const invalidIds = containerIds.filter((id: unknown) => typeof id !== 'string' || !isValidDockerResourceId(id));
|
||||
if (invalidIds.length > 0) {
|
||||
return res.status(400).json({ error: 'One or more container IDs have an invalid format' });
|
||||
}
|
||||
console.log(`[Resources] Prune orphans: ${containerIds.length} container(s) requested`);
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
const results = await dockerController.removeContainers(containerIds);
|
||||
const succeeded = results.filter((r: { success: boolean }) => r.success).length;
|
||||
console.log(`[Resources] Prune orphans completed: ${succeeded}/${containerIds.length} removed`);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
res.json({ results });
|
||||
} catch (error) {
|
||||
@@ -5432,8 +5439,9 @@ app.post('/api/system/prune/system', async (req: Request, res: Response) => {
|
||||
return res.status(400).json({ error: 'Invalid prune target' });
|
||||
}
|
||||
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
const pruneScope = scope === 'managed' ? 'managed' : 'all';
|
||||
console.log(`[Resources] System prune: ${target} (scope: ${pruneScope})`);
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
|
||||
let result: { success: boolean; reclaimedBytes: number };
|
||||
if (pruneScope === 'managed' && target !== 'containers') {
|
||||
@@ -5446,13 +5454,14 @@ app.post('/api/system/prune/system', async (req: Request, res: Response) => {
|
||||
result = await dockerController.pruneSystem(target as 'containers' | 'images' | 'networks' | 'volumes');
|
||||
}
|
||||
|
||||
console.log(`[Resources] System prune completed: ${target}, reclaimed ${result.reclaimedBytes} bytes`);
|
||||
if (target === 'containers') {
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
}
|
||||
res.json({ message: 'Prune completed', ...result });
|
||||
} catch (error: any) {
|
||||
} catch (error: unknown) {
|
||||
console.error('System prune error:', error);
|
||||
res.status(500).json({ error: 'System prune failed', details: error.message });
|
||||
res.status(500).json({ error: 'System prune failed' });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -5518,12 +5527,17 @@ app.post('/api/system/images/delete', async (req: Request, res: Response) => {
|
||||
try {
|
||||
const { id } = req.body;
|
||||
if (!id) return res.status(400).json({ error: 'ID is required' });
|
||||
if (typeof id !== 'string' || !isValidDockerResourceId(id)) {
|
||||
return res.status(400).json({ error: 'Invalid image ID format' });
|
||||
}
|
||||
console.log(`[Resources] Delete image: ${id.substring(0, 12)}`);
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
await dockerController.removeImage(id);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
res.json({ success: true, message: 'Image deleted' });
|
||||
} catch (error: any) {
|
||||
} catch (error: unknown) {
|
||||
console.error('Failed to delete image:', error);
|
||||
res.status(500).json({ error: error.message || 'Failed to delete image' });
|
||||
res.status(500).json({ error: 'Failed to delete image' });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -5531,13 +5545,15 @@ app.post('/api/system/volumes/delete', async (req: Request, res: Response) => {
|
||||
if (!requireAdmin(req, res)) return;
|
||||
try {
|
||||
const { id } = req.body;
|
||||
if (!id) return res.status(400).json({ error: 'ID is required' });
|
||||
if (!id || typeof id !== 'string') return res.status(400).json({ error: 'Volume name is required' });
|
||||
console.log(`[Resources] Delete volume: ${id}`);
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
await dockerController.removeVolume(id);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
res.json({ success: true, message: 'Volume deleted' });
|
||||
} catch (error: any) {
|
||||
} catch (error: unknown) {
|
||||
console.error('Failed to delete volume:', error);
|
||||
res.status(500).json({ error: error.message || 'Failed to delete volume' });
|
||||
res.status(500).json({ error: 'Failed to delete volume' });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -5546,26 +5562,32 @@ app.post('/api/system/networks/delete', async (req: Request, res: Response) => {
|
||||
try {
|
||||
const { id } = req.body;
|
||||
if (!id) return res.status(400).json({ error: 'ID is required' });
|
||||
if (typeof id !== 'string' || !isValidDockerResourceId(id)) {
|
||||
return res.status(400).json({ error: 'Invalid network ID format' });
|
||||
}
|
||||
console.log(`[Resources] Delete network: ${id.substring(0, 12)}`);
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
await dockerController.removeNetwork(id);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
res.json({ success: true, message: 'Network deleted' });
|
||||
} catch (error: any) {
|
||||
} catch (error: unknown) {
|
||||
console.error('Failed to delete network:', error);
|
||||
res.status(500).json({ error: error.message || 'Failed to delete network' });
|
||||
res.status(500).json({ error: 'Failed to delete network' });
|
||||
}
|
||||
});
|
||||
|
||||
app.get('/api/system/networks/topology', async (req: Request, res: Response) => {
|
||||
if (!requirePaid(req, res)) return;
|
||||
try {
|
||||
const includeSystem = req.query.includeSystem === 'true';
|
||||
const knownStacks = await FileSystemService.getInstance(req.nodeId).getStacks();
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
const topology = await dockerController.getTopologyData(knownStacks, includeSystem);
|
||||
if (isDebugEnabled()) console.debug('[Resources:debug] Topology fetched', { networkCount: topology.length, includeSystem });
|
||||
res.json(topology);
|
||||
} catch (error: unknown) {
|
||||
const msg = error instanceof Error ? error.message : 'Failed to fetch network topology';
|
||||
console.error('Failed to fetch network topology:', error);
|
||||
res.status(500).json({ error: msg });
|
||||
res.status(500).json({ error: 'Failed to fetch network topology' });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -5579,10 +5601,9 @@ app.get('/api/system/networks/:id', async (req: Request, res: Response) => {
|
||||
} catch (error: unknown) {
|
||||
console.error('Failed to inspect network:', error);
|
||||
const err = error as Record<string, unknown>;
|
||||
const status = (typeof err.statusCode === 'number' ? err.statusCode : null)
|
||||
|| (error instanceof Error && error.message.includes('404') ? 404 : 500);
|
||||
const msg = error instanceof Error ? error.message : 'Failed to inspect network';
|
||||
res.status(status).json({ error: msg });
|
||||
const is404 = (typeof err.statusCode === 'number' && err.statusCode === 404)
|
||||
|| (error instanceof Error && error.message.includes('404'));
|
||||
res.status(is404 ? 404 : 500).json({ error: is404 ? 'Network not found' : 'Failed to inspect network' });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -5600,6 +5621,8 @@ app.post('/api/system/networks', async (req: Request, res: Response) => {
|
||||
options.Driver = driver;
|
||||
}
|
||||
if (subnet || gateway) {
|
||||
if (subnet && !isValidCidr(subnet)) return res.status(400).json({ error: 'Invalid subnet CIDR notation (e.g. 172.20.0.0/16)' });
|
||||
if (gateway && !isValidIPv4(gateway)) return res.status(400).json({ error: 'Invalid gateway IP address (e.g. 172.20.0.1)' });
|
||||
options.IPAM = { Config: [{}] };
|
||||
if (subnet) options.IPAM.Config[0].Subnet = subnet;
|
||||
if (gateway) options.IPAM.Config[0].Gateway = gateway;
|
||||
@@ -5610,11 +5633,16 @@ app.post('/api/system/networks', async (req: Request, res: Response) => {
|
||||
|
||||
const dockerController = DockerController.getInstance(req.nodeId);
|
||||
const network = await dockerController.createNetwork(options);
|
||||
console.log(`[Resources] Network created: ${name}`);
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
res.status(201).json({ success: true, message: 'Network created', id: network.id });
|
||||
} catch (error: unknown) {
|
||||
const msg = error instanceof Error ? error.message : 'Failed to create network';
|
||||
console.error('Failed to create network:', error);
|
||||
res.status(500).json({ error: msg });
|
||||
const msg = getErrorMessage(error, '');
|
||||
const safePatterns = ['already exists', 'name is invalid', 'invalid network name'];
|
||||
const lowerMsg = msg.toLowerCase();
|
||||
const isSafe = safePatterns.some(p => lowerMsg.includes(p));
|
||||
res.status(isSafe ? 409 : 500).json({ error: isSafe ? msg : 'Failed to create network' });
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ import * as yaml from 'yaml';
|
||||
import { NodeRegistry } from './NodeRegistry';
|
||||
import { CacheService } from './CacheService';
|
||||
import { isPathWithinBase } from '../utils/validation';
|
||||
import { isDebugEnabled } from '../utils/debug';
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
const COMPOSE_DIR = process.env.COMPOSE_DIR || '/app/compose';
|
||||
@@ -205,6 +206,8 @@ class DockerController {
|
||||
volumes: ClassifiedVolume[];
|
||||
networks: ClassifiedNetwork[];
|
||||
}> {
|
||||
const debug = isDebugEnabled();
|
||||
const t0 = debug ? Date.now() : 0;
|
||||
const knownSet = new Set(knownStackNames);
|
||||
|
||||
const [rawImages, rawVolumeData, rawNetworks, allContainers, projectToStack] = await Promise.all([
|
||||
@@ -274,6 +277,10 @@ class DockerController {
|
||||
};
|
||||
});
|
||||
|
||||
if (debug) console.debug('[Resources:debug] Classification completed', {
|
||||
ms: Date.now() - t0, images: images.length, volumes: volumes.length, networks: networks.length,
|
||||
});
|
||||
|
||||
return { images, volumes, networks };
|
||||
}
|
||||
|
||||
@@ -926,6 +933,7 @@ class DockerController {
|
||||
}
|
||||
|
||||
public async removeContainers(containerIds: string[]) {
|
||||
if (isDebugEnabled()) console.debug('[Resources:debug] removeContainers', { count: containerIds.length });
|
||||
const results = [];
|
||||
for (const id of containerIds) {
|
||||
try {
|
||||
|
||||
@@ -39,6 +39,39 @@ export function isValidRemoteUrl(
|
||||
return { valid: true, url };
|
||||
}
|
||||
|
||||
/** Returns true when all four captured octet strings are in 0-255 range. */
|
||||
function octetsInRange(a: string, b: string, c: string, d: string): boolean {
|
||||
return [a, b, c, d].map(Number).every(o => o >= 0 && o <= 255);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates an IPv4 CIDR notation string (e.g. `10.0.0.0/24`).
|
||||
* Checks octet ranges (0-255) and prefix length (0-32).
|
||||
*/
|
||||
export function isValidCidr(value: string): boolean {
|
||||
const match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})\/(\d{1,2})$/.exec(value);
|
||||
if (!match) return false;
|
||||
return octetsInRange(match[1], match[2], match[3], match[4]) && Number(match[5]) <= 32;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a plain IPv4 address (e.g. `192.168.1.1`).
|
||||
* Rejects CIDR notation; use `isValidCidr` for that.
|
||||
*/
|
||||
export function isValidIPv4(value: string): boolean {
|
||||
const match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(value);
|
||||
if (!match) return false;
|
||||
return octetsInRange(match[1], match[2], match[3], match[4]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a Docker resource ID (hex string, 12-64 characters).
|
||||
* Covers both short IDs (12 chars) and full SHA256 IDs (64 chars).
|
||||
*/
|
||||
export function isValidDockerResourceId(id: string): boolean {
|
||||
return /^[a-f0-9]{12,64}$/i.test(id);
|
||||
}
|
||||
|
||||
/**
|
||||
* Asserts that a resolved file path stays within a given base directory.
|
||||
* Returns true if the path is safe, false if it escapes the base.
|
||||
|
||||
Reference in New Issue
Block a user