fix(rbac): cover stack assignment cleanup on blueprint withdraw (#1664)

Main already clears stack-scoped role_assignments through
DeployedStackDeletionService (used by local blueprint withdraw and
stack DELETE). Add call-path regressions for successful cleanup,
ENOENT-as-absent, non-ENOENT filesystem failure, and db_failed when
RBAC cleanup throws, plus remote hub isolation.
This commit is contained in:
Anso
2026-07-22 20:29:08 -04:00
committed by GitHub
parent 698b7d0713
commit 6484c79015
4 changed files with 194 additions and 0 deletions
@@ -214,4 +214,42 @@ describe('BlueprintService remote deploy', () => {
expect(delSpy.mock.calls[0][0]).toMatch(/\/api\/stacks\//);
expect(DatabaseService.getInstance().getDeployment(bp.id, node.id)).toBeUndefined();
});
it('does not clear hub role assignments when withdrawing a remote deployment', async () => {
const bcrypt = await import('bcrypt');
const db = DatabaseService.getInstance();
const node = seedRemoteNode();
const bp = seedBlueprint([node.id]);
const nodeObj = db.getNode(node.id)!;
const bpObj = db.getBlueprint(bp.id)!;
db.upsertDeployment({
blueprint_id: bp.id,
node_id: node.id,
status: 'active',
applied_revision: bpObj.revision,
});
const hash = await bcrypt.hash('password123', 1);
const userId = db.addUser({
username: `remote-wd-rbac-${counter}`, password_hash: hash, role: 'viewer',
});
db.addRoleAssignment({
user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: bpObj.name,
});
vi.spyOn(axios, 'get').mockResolvedValue({ status: 404, data: {} });
vi.spyOn(axios, 'post').mockResolvedValue({ status: 200, data: {} });
const delSpy = vi.spyOn(axios, 'delete').mockResolvedValue({ status: 200, data: {} });
const rbacSpy = vi.spyOn(db, 'deleteRoleAssignmentsByResource');
const result = await BlueprintService.getInstance().withdrawFromNode(bpObj, nodeObj);
expect(result.status).toBe('withdrawn');
expect(delSpy.mock.calls[0][0]).toMatch(/\/api\/stacks\//);
expect(rbacSpy).not.toHaveBeenCalled();
expect(db.getAllRoleAssignments(userId)
.some((a) => a.resource_type === 'stack' && a.resource_id === bpObj.name)).toBe(true);
db.deleteUser(userId);
});
});
+104
View File
@@ -13,6 +13,7 @@
* lifecycle in the plan.
*/
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
import bcrypt from 'bcrypt';
import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb';
import type { Blueprint, Node } from '../services/DatabaseService';
import type { ReconcileDecision } from '../services/BlueprintReconciler';
@@ -40,6 +41,7 @@ afterAll(() => cleanupTestDb(tmpDir));
beforeEach(() => {
const db = DatabaseService.getInstance().getDb();
db.prepare('DELETE FROM role_assignments').run();
db.prepare('DELETE FROM blueprint_deployments').run();
db.prepare('DELETE FROM blueprints').run();
db.prepare('DELETE FROM node_labels').run();
@@ -466,6 +468,108 @@ describe('BlueprintService per-stack lock', () => {
});
});
describe('BlueprintService local withdraw clears stack-scoped role assignments', () => {
function hasAssignment(userId: number, resourceType: string, resourceId: string): boolean {
return DatabaseService.getInstance().getAllRoleAssignments(userId)
.some((a) => a.resource_type === resourceType && a.resource_id === resourceId);
}
async function arrangeLocalWithdraw() {
const db = DatabaseService.getInstance();
const nodeId = seedNode();
const bp = seedBlueprint({ name: `rbac-wd-${counter}`, classification: 'stateless', nodeIds: [nodeId] });
const node = db.getNode(nodeId)!;
db.upsertDeployment({
blueprint_id: bp.id,
node_id: nodeId,
status: 'active',
applied_revision: bp.revision,
});
const hash = await bcrypt.hash('password123', 1);
const userId = db.addUser({ username: `bp-rbac-${counter}`, password_hash: hash, role: 'viewer' });
const otherNodeId = seedNode();
db.addRoleAssignment({
user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: bp.name,
});
db.addRoleAssignment({
user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: 'other-stack',
});
db.addRoleAssignment({
user_id: userId, role: 'deployer', resource_type: 'node', resource_id: String(otherNodeId),
});
vi.spyOn(BlueprintService.getInstance(), 'readMarker').mockResolvedValue(null);
const { ComposeService } = await import('../services/ComposeService');
const { FileSystemService } = await import('../services/FileSystemService');
vi.spyOn(ComposeService.prototype, 'downStack').mockResolvedValue(undefined);
const deleteStackSpy = vi.spyOn(FileSystemService.prototype, 'deleteStack').mockResolvedValue(undefined);
return { bp, node, nodeId, userId, deleteStackSpy, db };
}
it('clears the target assignment after successful filesystem deletion and preserves unrelated grants', async () => {
const { bp, node, userId, deleteStackSpy, db } = await arrangeLocalWithdraw();
const outcome = await BlueprintService.getInstance().withdrawFromNode(bp, node);
expect(outcome.status).toBe('withdrawn');
expect(deleteStackSpy).toHaveBeenCalledWith(bp.name);
expect(db.getDeployment(bp.id, node.id)).toBeUndefined();
expect(hasAssignment(userId, 'stack', bp.name)).toBe(false);
expect(hasAssignment(userId, 'stack', 'other-stack')).toBe(true);
expect(db.getAllRoleAssignments(userId).some((a) => a.resource_type === 'node')).toBe(true);
db.deleteUser(userId);
});
it('clears the target assignment when deleteStack treats the directory as already absent (ENOENT)', async () => {
// Shared deletion always calls deleteStack; FileSystemService maps ENOENT to success.
const { bp, node, userId, deleteStackSpy, db } = await arrangeLocalWithdraw();
deleteStackSpy.mockResolvedValue(undefined);
const outcome = await BlueprintService.getInstance().withdrawFromNode(bp, node);
expect(outcome.status).toBe('withdrawn');
expect(deleteStackSpy).toHaveBeenCalledWith(bp.name);
expect(hasAssignment(userId, 'stack', bp.name)).toBe(false);
expect(db.getAllRoleAssignments(userId)).toHaveLength(2);
db.deleteUser(userId);
});
it('preserves the grant when filesystem deletion fails with a non-ENOENT error', async () => {
const { bp, node, nodeId, userId, deleteStackSpy, db } = await arrangeLocalWithdraw();
const fsErr = Object.assign(new Error('permission denied'), { code: 'EACCES' });
deleteStackSpy.mockRejectedValue(fsErr);
const rbacSpy = vi.spyOn(db, 'deleteRoleAssignmentsByResource');
const outcome = await BlueprintService.getInstance().withdrawFromNode(bp, node);
expect(outcome.status).toBe('failed');
expect(rbacSpy).not.toHaveBeenCalled();
expect(db.getDeployment(bp.id, nodeId)?.status).toBe('failed');
expect(hasAssignment(userId, 'stack', bp.name)).toBe(true);
db.deleteUser(userId);
});
it('fails withdraw and keeps the deployment when role-assignment cleanup throws', async () => {
const { bp, node, nodeId, userId, db } = await arrangeLocalWithdraw();
vi.spyOn(db, 'deleteRoleAssignmentsByResource')
.mockImplementation(() => { throw new Error('simulated rbac cleanup failure'); });
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const outcome = await BlueprintService.getInstance().withdrawFromNode(bp, node);
expect(outcome.status).toBe('failed');
expect(db.getDeployment(bp.id, nodeId)).toBeDefined();
expect(db.getDeployment(bp.id, nodeId)?.status).toBe('failed');
expect(errorSpy.mock.calls.some((args) =>
typeof args[0] === 'string' && args[0].includes('Secondary DB cleanup failed'),
)).toBe(true);
expect(hasAssignment(userId, 'stack', bp.name)).toBe(true);
db.deleteUser(userId);
});
});
describe('BlueprintService marker parsing + name-conflict guard', () => {
it('parseMarker accepts a well-formed marker', () => {
const marker = BlueprintService.parseMarker(JSON.stringify({ blueprintId: 7, revision: 3, lastApplied: 12345 }));
@@ -131,3 +131,32 @@ describe('DELETE /api/stacks/:stackName mesh opt-out cascade (F-1 / F-14)', () =
expect(sawCascadeWarning).toBe(true);
});
});
describe('DELETE /api/stacks/:stackName clears stack-scoped role assignments', () => {
it('removes only the deleted stack assignment and preserves unrelated grants', async () => {
const db = DatabaseService.getInstance();
const bcrypt = await import('bcrypt');
const hash = await bcrypt.hash('password123', 1);
const userId = db.addUser({ username: 'stack-del-rbac', password_hash: hash, role: 'viewer' });
const otherNodeId = db.addNode({
name: 'stack-del-rbac-node', type: 'remote', api_url: 'http://test:1852',
api_token: '', compose_dir: '/tmp', is_default: false,
});
db.addRoleAssignment({ user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: 'api' });
db.addRoleAssignment({ user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: 'other-stack' });
db.addRoleAssignment({ user_id: userId, role: 'deployer', resource_type: 'node', resource_id: String(otherNodeId) });
const res = await request(app)
.delete('/api/stacks/api')
.set('Cookie', adminCookie);
expect(res.status).toBe(200);
const remaining = db.getAllRoleAssignments(userId);
expect(remaining.some((a) => a.resource_type === 'stack' && a.resource_id === 'api')).toBe(false);
expect(remaining.some((a) => a.resource_type === 'stack' && a.resource_id === 'other-stack')).toBe(true);
expect(remaining.some((a) => a.resource_type === 'node' && a.resource_id === String(otherNodeId))).toBe(true);
db.deleteUser(userId);
db.deleteNode(otherNodeId);
});
});
+23
View File
@@ -714,6 +714,29 @@ describe('Orphaned role assignment cleanup', () => {
// Cleanup
db.deleteUser(userId);
});
it('deleteRoleAssignmentsByResource removes only the matching resource tuple', async () => {
const db = DatabaseService.getInstance();
const hash = await bcrypt.hash('password123', 1);
const userId = db.addUser({ username: 'tupleorphan', password_hash: hash, role: 'viewer' });
const nodeId = db.addNode({
name: 'tuple-cleanup-node', type: 'remote', api_url: 'http://test:1852',
api_token: '', compose_dir: '/tmp', is_default: false,
});
db.addRoleAssignment({ user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: 'target-stack' });
db.addRoleAssignment({ user_id: userId, role: 'deployer', resource_type: 'stack', resource_id: 'keep-stack' });
db.addRoleAssignment({ user_id: userId, role: 'deployer', resource_type: 'node', resource_id: String(nodeId) });
db.deleteRoleAssignmentsByResource('stack', 'target-stack');
const after = db.getAllRoleAssignments(userId);
expect(after.some((a) => a.resource_type === 'stack' && a.resource_id === 'target-stack')).toBe(false);
expect(after.some((a) => a.resource_type === 'stack' && a.resource_id === 'keep-stack')).toBe(true);
expect(after.some((a) => a.resource_type === 'node' && a.resource_id === String(nodeId))).toBe(true);
db.deleteUser(userId);
db.deleteNode(nodeId);
});
});
// ---- Role-Based Permission Checks (via API) ----