mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-21 15:46:43 +00:00
fix(nodes): never send stored node tokens to clients (#1281)
Node read endpoints now return a client-safe projection that omits the stored api_token and exposes a has_token boolean instead, so a node's long-lived proxy credential is never serialized to a browser or API token client. The token stays encrypted at rest and is read server-side only by the components that need it (the remote proxy, the connection test, and the mesh dialer). The edit form opens the API Token field blank, and a blank value keeps the existing credential, so saving an edit without retyping the token no longer clears it; a non-empty value rotates it. The backend enforces the same rule defensively. Node management actions (add, edit, delete, test connection, generate node token) are gated in the UI to match their server-side permission checks, so operators no longer see an action the API would reject. The test-connection route also gains the missing server-side permission and token-scope guards. Also validate the x-node-id header and fall back to the default node for malformed values instead of an obscure 404, and return 400 (not 500) when deleting the default node.
This commit is contained in:
@@ -8,6 +8,34 @@ import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './he
|
||||
import { disableCapability, enableCapability } from '../services/CapabilityRegistry';
|
||||
import { NodeRegistry } from '../services/NodeRegistry';
|
||||
import { CacheService } from '../services/CacheService';
|
||||
import { DatabaseService } from '../services/DatabaseService';
|
||||
import { nodeContextMiddleware } from '../middleware/nodeContext';
|
||||
|
||||
/** Mint a Bearer for a non-admin user, creating the row if needed so
|
||||
* authMiddleware (which resolves the role from the DB) sees the real role. */
|
||||
function tokenForRole(username: string, role: 'viewer' | 'deployer'): string {
|
||||
const db = DatabaseService.getInstance();
|
||||
if (!db.getUserByUsername(username)) {
|
||||
db.addUser({ username, password_hash: 'x', role });
|
||||
}
|
||||
return `Bearer ${jwt.sign({ username }, TEST_JWT_SECRET, { expiresIn: '1m' })}`;
|
||||
}
|
||||
|
||||
async function createRemoteNode(token: string): Promise<number> {
|
||||
const res = await request(app)
|
||||
.post('/api/nodes')
|
||||
.set('Authorization', token)
|
||||
.send({
|
||||
name: `remote-${Date.now()}-${Math.random().toString(36).slice(2)}`,
|
||||
type: 'remote',
|
||||
mode: 'proxy',
|
||||
api_url: 'http://192.168.1.77:1852',
|
||||
api_token: 'tok-original',
|
||||
compose_dir: '/app/compose',
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
return res.body.id as number;
|
||||
}
|
||||
|
||||
let tmpDir: string;
|
||||
let app: import('express').Express;
|
||||
@@ -135,3 +163,125 @@ describe('Stack name validation on GET routes (H3 fix)', () => {
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Node read endpoints never leak the api_token (C-1)', () => {
|
||||
it('GET /api/nodes omits api_token and exposes has_token instead', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const res = await request(app).get('/api/nodes').set('Authorization', authHeader);
|
||||
expect(res.status).toBe(200);
|
||||
const nodes = res.body as Array<Record<string, unknown>>;
|
||||
for (const n of nodes) {
|
||||
expect(Object.prototype.hasOwnProperty.call(n, 'api_token')).toBe(false);
|
||||
expect(typeof n.has_token).toBe('boolean');
|
||||
}
|
||||
const created = nodes.find((n) => n.id === id)!;
|
||||
expect(created.has_token).toBe(true);
|
||||
const local = nodes.find((n) => n.type === 'local')!;
|
||||
expect(local.has_token).toBe(false);
|
||||
});
|
||||
|
||||
it('GET /api/nodes/:id omits api_token and exposes has_token', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const res = await request(app).get(`/api/nodes/${id}`).set('Authorization', authHeader);
|
||||
expect(res.status).toBe(200);
|
||||
expect(Object.prototype.hasOwnProperty.call(res.body, 'api_token')).toBe(false);
|
||||
expect(res.body.has_token).toBe(true);
|
||||
// The decrypted secret still lives server-side for the proxy / test paths.
|
||||
expect(DatabaseService.getInstance().getNode(id)?.api_token).toBe('tok-original');
|
||||
});
|
||||
});
|
||||
|
||||
describe('PUT /api/nodes/:id preserves the token unless a new one is supplied (H-1)', () => {
|
||||
it('keeps the stored token when api_token is omitted', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const res = await request(app)
|
||||
.put(`/api/nodes/${id}`)
|
||||
.set('Authorization', authHeader)
|
||||
.send({ name: 'renamed-keep', api_url: 'http://192.168.1.77:1852', compose_dir: '/app/compose' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(DatabaseService.getInstance().getNode(id)?.api_token).toBe('tok-original');
|
||||
});
|
||||
|
||||
it('keeps the stored token when api_token is an empty string', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const res = await request(app)
|
||||
.put(`/api/nodes/${id}`)
|
||||
.set('Authorization', authHeader)
|
||||
.send({ name: 'renamed-blank', api_token: '', api_url: 'http://192.168.1.77:1852', compose_dir: '/app/compose' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(DatabaseService.getInstance().getNode(id)?.api_token).toBe('tok-original');
|
||||
});
|
||||
|
||||
it('rotates the token when a non-empty api_token is supplied', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const res = await request(app)
|
||||
.put(`/api/nodes/${id}`)
|
||||
.set('Authorization', authHeader)
|
||||
.send({ name: 'renamed-rotate', api_token: 'tok-new', api_url: 'http://192.168.1.77:1852', compose_dir: '/app/compose' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(DatabaseService.getInstance().getNode(id)?.api_token).toBe('tok-new');
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/nodes/:id/test authorization (H-3)', () => {
|
||||
it('403s a non-admin (viewer) with PERMISSION_DENIED', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const res = await request(app)
|
||||
.post(`/api/nodes/${id}/test`)
|
||||
.set('Authorization', tokenForRole('node-test-viewer', 'viewer'));
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.code).toBe('PERMISSION_DENIED');
|
||||
});
|
||||
|
||||
it('403s a full-admin API token with SCOPE_DENIED', async () => {
|
||||
const id = await createRemoteNode(authHeader);
|
||||
const mint = await request(app)
|
||||
.post('/api/api-tokens')
|
||||
.set('Authorization', authHeader)
|
||||
.send({ name: `node-test-reject-${Date.now()}`, scope: 'full-admin' });
|
||||
const apiToken = mint.body.token as string;
|
||||
expect(apiToken).toBeTruthy();
|
||||
const res = await request(app)
|
||||
.post(`/api/nodes/${id}/test`)
|
||||
.set('Authorization', `Bearer ${apiToken}`);
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.code).toBe('SCOPE_DENIED');
|
||||
});
|
||||
});
|
||||
|
||||
describe('nodeContextMiddleware nodeId validation (M-2)', () => {
|
||||
type MiddlewareReq = { headers: Record<string, string>; query: Record<string, string>; path: string; nodeId?: number };
|
||||
|
||||
function run(req: MiddlewareReq): { status: number; nextCalled: boolean } {
|
||||
let status = 0;
|
||||
let nextCalled = false;
|
||||
const res = { status: (c: number) => { status = c; return { json: () => undefined }; } };
|
||||
nodeContextMiddleware(req as never, res as never, (() => { nextCalled = true; }) as never);
|
||||
return { status, nextCalled };
|
||||
}
|
||||
|
||||
it('falls back to the default node for a malformed x-node-id instead of 404', () => {
|
||||
const req: MiddlewareReq = { headers: { 'x-node-id': 'abc' }, query: {}, path: '/api/stats' };
|
||||
const { status, nextCalled } = run(req);
|
||||
expect(nextCalled).toBe(true);
|
||||
expect(status).toBe(0);
|
||||
expect(req.nodeId).toBe(NodeRegistry.getInstance().getDefaultNodeId());
|
||||
});
|
||||
|
||||
it('still 404s a well-formed but non-existent node id', () => {
|
||||
const req: MiddlewareReq = { headers: { 'x-node-id': '999999' }, query: {}, path: '/api/stats' };
|
||||
const { status, nextCalled } = run(req);
|
||||
expect(status).toBe(404);
|
||||
expect(nextCalled).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DELETE /api/nodes/:id default-node guard (M-4)', () => {
|
||||
it('400s when deleting the default node', async () => {
|
||||
const list = await request(app).get('/api/nodes').set('Authorization', authHeader);
|
||||
const def = (list.body as Array<{ id: number; is_default: boolean }>).find((n) => n.is_default)!;
|
||||
const res = await request(app).delete(`/api/nodes/${def.id}`).set('Authorization', authHeader);
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/default node/i);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user