From 17a8dc8a94b18b396492735de8cd89bc2cf45c97 Mon Sep 17 00:00:00 2001 From: Anso Date: Fri, 24 Jul 2026 15:57:18 -0400 Subject: [PATCH] fix(blueprints): fail closed on marker ownership for apply and withdraw (#1694) * fix(blueprints): fail closed on marker ownership for apply and withdraw Require a matching .blueprint.json under the stack lock, persist required_blueprint_id on deletion intents, remove the legacy remote apply fallback, and protect the marker in the file explorer. * fix(blueprints): add CodeQL path barriers on ownership probes Use the canonical resolve-and-startsWith sanitizer inline at the marker and stack-directory fs sinks so js/path-injection clears. * fix(blueprints): block delete on failed withdraw and defer marker write Refuse Blueprint DELETE when pre-delete withdraw does not complete, and write .blueprint.json only after a successful deploy so failed applies cannot orphan stacks or claim an unapplied revision. * test(blueprints): align lock-order assert with deferred marker write Update the per-stack lock ordering expectations to compose, cleanup, deploy, then marker after the partial-apply fix. * fix(deps): bump postcss past GHSA-r28c-9q8g-f849 for npm audit Raise the Vitest/Vite transitive postcss to 8.5.23 so Backend CI audit --audit-level=high passes. --- .../blueprints-compose-apply.test.ts | 80 ++++ .../__tests__/blueprints-edge-cases.test.ts | 82 +++- .../blueprints-preview-apply.test.ts | 36 ++ .../blueprints-remote-deploy.test.ts | 97 ++-- .../blueprints-route-validation.test.ts | 68 +++ backend/src/__tests__/blueprints.test.ts | 30 +- ...ase-stack-cleanup-intent-migration.test.ts | 64 +++ .../deployed-stack-deletion-service.test.ts | 39 ++ .../__tests__/filesystem-stack-paths.test.ts | 4 +- .../src/__tests__/stack-files-routes.test.ts | 41 ++ backend/src/helpers/blueprintMarker.ts | 28 ++ backend/src/routes/blueprints.ts | 87 +++- backend/src/routes/stacks.ts | 39 ++ backend/src/services/BlueprintService.ts | 425 ++++++++++-------- backend/src/services/DatabaseService.ts | 10 +- .../services/DeployedStackDeletionService.ts | 199 ++++++-- backend/src/services/FileSystemService.ts | 13 +- .../services/blueprintPreviewProjection.ts | 27 +- docs/features/blueprint-model.mdx | 9 +- 19 files changed, 1092 insertions(+), 286 deletions(-) create mode 100644 backend/src/__tests__/database-stack-cleanup-intent-migration.test.ts create mode 100644 backend/src/helpers/blueprintMarker.ts diff --git a/backend/src/__tests__/blueprints-compose-apply.test.ts b/backend/src/__tests__/blueprints-compose-apply.test.ts index 7e8d6b21..8baf72f6 100644 --- a/backend/src/__tests__/blueprints-compose-apply.test.ts +++ b/backend/src/__tests__/blueprints-compose-apply.test.ts @@ -89,6 +89,7 @@ describe('Blueprint compose apply (real filesystem)', () => { const stackDir = path.join(process.env.COMPOSE_DIR!, stackName); expect(await fsPromises.readFile(path.join(stackDir, 'compose.yaml'), 'utf-8')).toBe(composeContent); await expectMissing(path.join(stackDir, 'docker-compose.yml')); + expect(await fsPromises.readFile(path.join(stackDir, '.blueprint.json'), 'utf-8')).toBe(markerContent); const resolved = await FileSystemService.getInstance(nodeId).getComposeFilename(stackName); expect(resolved).toBe('compose.yaml'); @@ -104,6 +105,10 @@ describe('Blueprint compose apply (real filesystem)', () => { await fsPromises.writeFile(path.join(stackDir, 'compose.yml'), 'services:\n a:\n image: a\n'); await fsPromises.writeFile(path.join(stackDir, 'docker-compose.yaml'), 'services:\n b:\n image: b\n'); await fsPromises.writeFile(path.join(stackDir, 'docker-compose.yml'), 'services:\n c:\n image: c\n'); + await fsPromises.writeFile( + path.join(stackDir, '.blueprint.json'), + JSON.stringify({ blueprintId: 2, revision: 2, lastApplied: 1 }, null, 2), + ); const composeContent = 'services:\n app:\n image: redis:7\n'; const markerContent = JSON.stringify({ blueprintId: 2, revision: 3, lastApplied: Date.now() }, null, 2); @@ -131,8 +136,83 @@ describe('Blueprint compose apply (real filesystem)', () => { await expectMissing(path.join(stackDir, 'compose.yml')); await expectMissing(path.join(stackDir, 'docker-compose.yaml')); await expectMissing(path.join(stackDir, 'docker-compose.yml')); + expect(await fsPromises.readFile(path.join(stackDir, '.blueprint.json'), 'utf-8')).toBe(markerContent); expect(deploySpy).toHaveBeenCalledTimes(1); }); + + it('does not write a new marker when deploy fails; rolls back a newly created stack', async () => { + const nodeId = seedLocalNode(); + const stackName = `bp-partial-${counter}`; + const composeContent = 'services:\n web:\n image: traefik:v3\n'; + const markerContent = JSON.stringify({ blueprintId: 7, revision: 1, lastApplied: Date.now() }, null, 2); + const stackDir = path.join(process.env.COMPOSE_DIR!, stackName); + + vi.spyOn(ComposeService.prototype, 'deployStack').mockRejectedValue(new Error('docker unavailable')); + + await expect( + BlueprintService.getInstance().applyLocalUnderLock( + nodeId, + stackName, + composeContent, + markerContent, + '/api/blueprints/test/apply', + ), + ).rejects.toThrow(/docker unavailable/); + + await expectMissing(path.join(stackDir, '.blueprint.json')); + await expectMissing(stackDir); + }); + + it('keeps the prior marker when a re-apply deploy fails', async () => { + const nodeId = seedLocalNode(); + const stackName = `bp-reapply-fail-${counter}`; + const stackDir = path.join(process.env.COMPOSE_DIR!, stackName); + const priorMarker = JSON.stringify({ blueprintId: 8, revision: 2, lastApplied: 1 }, null, 2); + await fsPromises.mkdir(stackDir, { recursive: true }); + await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), 'services:\n old:\n image: nginx\n'); + await fsPromises.writeFile(path.join(stackDir, '.blueprint.json'), priorMarker); + + vi.spyOn(ComposeService.prototype, 'deployStack').mockRejectedValue(new Error('deploy blew up')); + + await expect( + BlueprintService.getInstance().applyLocalUnderLock( + nodeId, + stackName, + 'services:\n new:\n image: redis:7\n', + JSON.stringify({ blueprintId: 8, revision: 3, lastApplied: Date.now() }, null, 2), + '/api/blueprints/test/apply', + ), + ).rejects.toThrow(/deploy blew up/); + + expect(await fsPromises.readFile(path.join(stackDir, '.blueprint.json'), 'utf-8')).toBe(priorMarker); + expect(await fsPromises.access(stackDir).then(() => true, () => false)).toBe(true); + }); + + it('refuses to overwrite an existing unmanaged stack directory inside the lock', async () => { + const { BlueprintNameConflictError } = await import('../services/BlueprintService'); + const nodeId = seedLocalNode(); + const stackName = `bp-hijack-${counter}`; + const stackDir = path.join(process.env.COMPOSE_DIR!, stackName); + await fsPromises.mkdir(stackDir, { recursive: true }); + const original = 'services:\n mine:\n image: nginx:alpine\n'; + await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), original); + + const deploySpy = vi.spyOn(ComposeService.prototype, 'deployStack').mockResolvedValue(undefined); + + await expect( + BlueprintService.getInstance().applyLocalUnderLock( + nodeId, + stackName, + 'services:\n bp:\n image: redis:7\n', + JSON.stringify({ blueprintId: 99, revision: 1, lastApplied: Date.now() }, null, 2), + '/api/blueprints/test/apply', + ), + ).rejects.toBeInstanceOf(BlueprintNameConflictError); + + expect(await fsPromises.readFile(path.join(stackDir, 'compose.yaml'), 'utf-8')).toBe(original); + await expectMissing(path.join(stackDir, '.blueprint.json')); + expect(deploySpy).not.toHaveBeenCalled(); + }); }); describe('FileSystemService.removeAlternateRootComposeFiles', () => { diff --git a/backend/src/__tests__/blueprints-edge-cases.test.ts b/backend/src/__tests__/blueprints-edge-cases.test.ts index f7beba38..026f2c22 100644 --- a/backend/src/__tests__/blueprints-edge-cases.test.ts +++ b/backend/src/__tests__/blueprints-edge-cases.test.ts @@ -111,9 +111,9 @@ describe('Blueprint route edge cases', () => { describe('Blueprint delete guard', () => { // Rows with no stack of ours on the node must not block delete, AND the route must not run the - // withdraw primitive for them: withdrawFromNode proceeds on a missing marker and would - // down/delete a same-name stack Sencho never owned. A name_conflict is exactly that unmanaged - // stack, so it is excluded even though it carries a last_deployed_at timestamp. + // withdraw primitive for them: there is nothing Sencho owns to remove. A name_conflict is an + // unmanaged same-name stack, so it is excluded even though it may carry a last_deployed_at + // timestamp. When withdraw does run, ownership is enforced under the delete lock. it.each([ { label: 'never-deployed pending review', status: 'pending_state_review' as const, last_deployed_at: null }, { label: 'first-deploy failure', status: 'failed' as const, last_deployed_at: null }, @@ -184,6 +184,34 @@ describe('Blueprint delete guard', () => { expect(DatabaseService.getInstance().getBlueprint(bp.id)).toBeUndefined(); }); + it('refuses to delete a stateless blueprint when pre-delete withdraw fails', async () => { + const node = seedNode(); + const bp = seedBlueprint([node.id]); + DatabaseService.getInstance().upsertDeployment({ + blueprint_id: bp.id, + node_id: node.id, + status: 'failed', + applied_revision: bp.revision, + last_deployed_at: Date.now(), + last_error: 'Remote node lacks withdraw-local', + }); + const withdrawSpy = vi.spyOn(BlueprintService.getInstance(), 'withdrawFromNode').mockResolvedValue({ + status: 'failed', + error: 'Remote node does not support atomic blueprint withdraw', + }); + + const res = await request(app) + .delete(`/api/blueprints/${bp.id}`) + .set('Cookie', adminCookie); + + expect(res.status).toBe(409); + expect(res.body.code).toBe('withdraw_failed_blocking_delete'); + expect(res.body.nodeId).toBe(node.id); + expect(withdrawSpy).toHaveBeenCalledTimes(1); + expect(DatabaseService.getInstance().getBlueprint(bp.id)).toBeDefined(); + expect(DatabaseService.getInstance().listDeployments(bp.id)).toHaveLength(1); + }); + it('refuses to delete when a pending review still has a deployed stack (revision drift)', async () => { const node = seedNode(); const bp = seedBlueprint([node.id], 'stateful'); @@ -253,38 +281,54 @@ describe('BlueprintService marker edge cases', () => { }); it('refuses to withdraw when the marker belongs to a different blueprint', async () => { + const fs = await import('fs'); + const path = await import('path'); + const composeDir = process.env.COMPOSE_DIR!; const localNode = DatabaseService.getInstance().getNodes()[0]; - const bp = seedBlueprint([localNode.id]); + DatabaseService.getInstance().getDb() + .prepare('UPDATE nodes SET compose_dir = ? WHERE id = ?') + .run(composeDir, localNode.id); + const refreshed = DatabaseService.getInstance().getNode(localNode.id)!; + const bp = seedBlueprint([refreshed.id]); const bpObj = DatabaseService.getInstance().getBlueprint(bp.id)!; - - vi.spyOn(BlueprintService.getInstance(), 'readMarker').mockResolvedValue({ - blueprintId: bp.id + 999, - revision: 1, - lastApplied: 0, + DatabaseService.getInstance().upsertDeployment({ + blueprint_id: bp.id, + node_id: refreshed.id, + status: 'active', + applied_revision: bpObj.revision, + last_deployed_at: Date.now(), }); - const result = await BlueprintService.getInstance().withdrawFromNode(bpObj, localNode); + const stackDir = path.join(composeDir, bpObj.name); + fs.mkdirSync(stackDir, { recursive: true }); + fs.writeFileSync(path.join(stackDir, 'compose.yaml'), 'services:\n app:\n image: nginx\n'); + fs.writeFileSync( + path.join(stackDir, '.blueprint.json'), + JSON.stringify({ blueprintId: bp.id + 999, revision: 1, lastApplied: 0 }), + ); + + const { DeployedStackDeletionService } = await import('../services/DeployedStackDeletionService'); + const deleteSpy = vi.spyOn(DeployedStackDeletionService.getInstance(), 'deleteDeployedStack'); + + const result = await BlueprintService.getInstance().withdrawFromNode(bpObj, refreshed); expect(result.status).toBe('name_conflict'); - // The deployment row must record the conflict, not silently disappear. - const dep = DatabaseService.getInstance().getDeployment(bp.id, localNode.id); + // deleteDeployedStack is invoked but returns name_conflict without mutating. + expect(deleteSpy).toHaveBeenCalled(); + const dep = DatabaseService.getInstance().getDeployment(bp.id, refreshed.id); expect(dep).toBeDefined(); expect(dep?.status).toBe('name_conflict'); + expect(fs.existsSync(path.join(stackDir, 'compose.yaml'))).toBe(true); }); }); describe('BlueprintService developer-mode diagnostics', () => { - // withdrawFromNode emits its "withdraw inputs" diagnostic line before reading the - // marker, so a cross-blueprint marker stub lets us assert the gate without Docker. + // withdrawFromNode emits its "withdraw inputs" diagnostic before the deletion + // service runs. An absent stack directory is enough to exercise logging without Docker. function arrangeWithdraw() { const localNode = DatabaseService.getInstance().getNodes()[0]; const bp = seedBlueprint([localNode.id]); const bpObj = DatabaseService.getInstance().getBlueprint(bp.id)!; - vi.spyOn(BlueprintService.getInstance(), 'readMarker').mockResolvedValue({ - blueprintId: bp.id + 999, - revision: 1, - lastApplied: 0, - }); return { bpObj, localNode }; } diff --git a/backend/src/__tests__/blueprints-preview-apply.test.ts b/backend/src/__tests__/blueprints-preview-apply.test.ts index d466b367..6a084973 100644 --- a/backend/src/__tests__/blueprints-preview-apply.test.ts +++ b/backend/src/__tests__/blueprints-preview-apply.test.ts @@ -326,6 +326,7 @@ describe('POST /api/blueprints/:id/apply confirm binding', () => { const conflict = await BlueprintService.getInstance().hasNameConflict( created.body.name as string, DatabaseService.getInstance().getNode(node.id)!, + created.body.id as number, ); expect(conflict).toBe(true); @@ -342,6 +343,41 @@ describe('POST /api/blueprints/:id/apply confirm binding', () => { .toBe('services:\n app:\n image: nginx\n'); }); + it('hasNameConflict is false for a matching marker and true for a foreign marker', async () => { + const node = seedNode(); + counter += 1; + const created = await request(app) + .post('/api/blueprints') + .set('Cookie', adminCookie) + .send(validBlueprintBody(node.id)); + expect(created.status).toBe(201); + + const composeDir = process.env.COMPOSE_DIR!; + DatabaseService.getInstance().getDb() + .prepare('UPDATE nodes SET compose_dir = ? WHERE id = ?') + .run(composeDir, node.id); + const nodeObj = DatabaseService.getInstance().getNode(node.id)!; + const stackName = created.body.name as string; + const blueprintId = created.body.id as number; + const stackDir = path.join(composeDir, stackName); + fs.mkdirSync(stackDir, { recursive: true }); + fs.writeFileSync(path.join(stackDir, 'compose.yaml'), 'services:\n app:\n image: nginx\n'); + fs.writeFileSync( + path.join(stackDir, '.blueprint.json'), + JSON.stringify({ blueprintId, revision: 1, lastApplied: 1 }), + ); + + const { BlueprintService } = await import('../services/BlueprintService'); + const svc = BlueprintService.getInstance(); + expect(await svc.hasNameConflict(stackName, nodeObj, blueprintId)).toBe(false); + + fs.writeFileSync( + path.join(stackDir, '.blueprint.json'), + JSON.stringify({ blueprintId: blueprintId + 99, revision: 1, lastApplied: 1 }), + ); + expect(await svc.hasNameConflict(stackName, nodeObj, blueprintId)).toBe(true); + }); + it('blocks rename while a non-withdrawn deployment exists', async () => { const node = seedNode(); counter += 1; diff --git a/backend/src/__tests__/blueprints-remote-deploy.test.ts b/backend/src/__tests__/blueprints-remote-deploy.test.ts index d9a145b7..9bf91e72 100644 --- a/backend/src/__tests__/blueprints-remote-deploy.test.ts +++ b/backend/src/__tests__/blueprints-remote-deploy.test.ts @@ -101,7 +101,7 @@ describe('BlueprintService remote deploy', () => { expect(dep?.applied_revision).toBe(bpObj.revision); }); - it('falls back to the legacy create/write/deploy flow when the remote lacks apply-local (404)', async () => { + it('fails closed when the remote lacks apply-local (404); no legacy mutations', async () => { const node = seedRemoteNode(); const bp = seedBlueprint([node.id]); const nodeObj = DatabaseService.getInstance().getNode(node.id)!; @@ -110,25 +110,17 @@ describe('BlueprintService remote deploy', () => { vi.spyOn(axios, 'get').mockResolvedValue({ status: 200, data: [] }); const putSpy = vi.spyOn(axios, 'put').mockResolvedValue({ status: 200, data: {} }); const postSpy = vi.spyOn(axios, 'post') - .mockResolvedValueOnce({ status: 404, data: {} }) // apply-local missing on older node - .mockResolvedValueOnce({ status: 201, data: {} }) // legacy create stack - .mockResolvedValueOnce({ status: 200, data: {} }); // legacy deploy + .mockResolvedValueOnce({ status: 404, data: {} }); const result = await BlueprintService.getInstance().deployToNode(bpObj, nodeObj); - expect(result.status).toBe('active'); + expect(result.status).toBe('failed'); + expect(result.error).toMatch(/apply-local|Upgrade/i); + expect(postSpy).toHaveBeenCalledTimes(1); expect(postSpy.mock.calls[0][0]).toMatch(/\/api\/blueprints\/apply-local$/); - expect(postSpy.mock.calls[1][0]).toMatch(/\/api\/stacks$/); - expect(putSpy).toHaveBeenCalledTimes(2); // compose + marker - const composePutUrl = String(putSpy.mock.calls[0][0]); - const markerPutUrl = String(putSpy.mock.calls[1][0]); - expect(composePutUrl).toMatch(/[?&]path=compose\.yaml(?:&|$)/); - expect(markerPutUrl).toMatch(/[?&]path=\.blueprint\.json(?:&|$)/); - expect(postSpy.mock.calls[2][0]).toMatch(/\/deploy$/); - const [composePutOrder, markerPutOrder] = putSpy.mock.invocationCallOrder; - const deployOrder = postSpy.mock.invocationCallOrder[2]; - expect(composePutOrder).toBeLessThan(markerPutOrder); - expect(markerPutOrder).toBeLessThan(deployOrder); + expect(putSpy).not.toHaveBeenCalled(); + const dep = DatabaseService.getInstance().getDeployment(bp.id, node.id); + expect(dep?.status).toBe('failed'); }); it('maps a remote apply lock-conflict (409) to status=failed', async () => { @@ -192,7 +184,7 @@ describe('BlueprintService remote deploy', () => { expect(dep?.status).toBe('name_conflict'); }); - it('withdraws a remote deployment by deleting the stack and removing the row', async () => { + it('withdraws a remote deployment via withdraw-local and removes the row', async () => { const node = seedRemoteNode(); const bp = seedBlueprint([node.id]); const nodeObj = DatabaseService.getInstance().getNode(node.id)!; @@ -204,17 +196,73 @@ describe('BlueprintService remote deploy', () => { applied_revision: bpObj.revision, }); - vi.spyOn(axios, 'get').mockResolvedValue({ status: 404, data: {} }); // readMarker → null → proceed - vi.spyOn(axios, 'post').mockResolvedValue({ status: 200, data: {} }); // remote down (best-effort) - const delSpy = vi.spyOn(axios, 'delete').mockResolvedValue({ status: 200, data: {} }); + const postSpy = vi.spyOn(axios, 'post').mockResolvedValue({ + status: 200, + data: { status: 'withdrawn' }, + }); + const delSpy = vi.spyOn(axios, 'delete'); const result = await BlueprintService.getInstance().withdrawFromNode(bpObj, nodeObj); expect(result.status).toBe('withdrawn'); - expect(delSpy.mock.calls[0][0]).toMatch(/\/api\/stacks\//); + expect(postSpy.mock.calls[0][0]).toMatch(/\/api\/blueprints\/withdraw-local$/); + expect(delSpy).not.toHaveBeenCalled(); expect(DatabaseService.getInstance().getDeployment(bp.id, node.id)).toBeUndefined(); }); + it('maps remote withdraw-local lock conflict to failed without fallback delete', async () => { + const node = seedRemoteNode(); + const bp = seedBlueprint([node.id]); + const nodeObj = DatabaseService.getInstance().getNode(node.id)!; + const bpObj = DatabaseService.getInstance().getBlueprint(bp.id)!; + DatabaseService.getInstance().upsertDeployment({ + blueprint_id: bp.id, + node_id: node.id, + status: 'active', + applied_revision: bpObj.revision, + }); + + vi.spyOn(axios, 'post').mockResolvedValue({ + status: 409, + data: { + error: `${bpObj.name} is busy: another operation (update) is already in progress`, + code: 'stack_op_in_progress', + }, + }); + const delSpy = vi.spyOn(axios, 'delete'); + + const result = await BlueprintService.getInstance().withdrawFromNode(bpObj, nodeObj); + + expect(result.status).toBe('failed'); + expect(result.error).toMatch(/already in progress/); + expect(delSpy).not.toHaveBeenCalled(); + expect(DatabaseService.getInstance().getDeployment(bp.id, node.id)?.status).toBe('failed'); + }); + + it('fails closed when the remote lacks withdraw-local (404); no legacy delete', async () => { + const node = seedRemoteNode(); + const bp = seedBlueprint([node.id]); + const nodeObj = DatabaseService.getInstance().getNode(node.id)!; + const bpObj = DatabaseService.getInstance().getBlueprint(bp.id)!; + DatabaseService.getInstance().upsertDeployment({ + blueprint_id: bp.id, + node_id: node.id, + status: 'active', + applied_revision: bpObj.revision, + }); + + const postSpy = vi.spyOn(axios, 'post').mockResolvedValue({ status: 404, data: { error: 'Not Found' } }); + const delSpy = vi.spyOn(axios, 'delete'); + + const result = await BlueprintService.getInstance().withdrawFromNode(bpObj, nodeObj); + + expect(result.status).toBe('failed'); + expect(result.error).toMatch(/withdraw-local|Upgrade/i); + expect(postSpy.mock.calls[0][0]).toMatch(/\/api\/blueprints\/withdraw-local$/); + expect(delSpy).not.toHaveBeenCalled(); + expect(DatabaseService.getInstance().getDeployment(bp.id, node.id)?.status).toBe('failed'); + }); + it('does not clear hub role assignments when withdrawing a remote deployment', async () => { const bcrypt = await import('bcrypt'); const db = DatabaseService.getInstance(); @@ -237,15 +285,14 @@ describe('BlueprintService remote deploy', () => { 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: {} }); + vi.spyOn(axios, 'post').mockResolvedValue({ status: 200, data: { status: 'withdrawn' } }); + const delSpy = vi.spyOn(axios, 'delete'); 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(delSpy).not.toHaveBeenCalled(); expect(rbacSpy).not.toHaveBeenCalled(); expect(db.getAllRoleAssignments(userId) .some((a) => a.resource_type === 'stack' && a.resource_id === bpObj.name)).toBe(true); diff --git a/backend/src/__tests__/blueprints-route-validation.test.ts b/backend/src/__tests__/blueprints-route-validation.test.ts index ce4de5dd..6848ff93 100644 --- a/backend/src/__tests__/blueprints-route-validation.test.ts +++ b/backend/src/__tests__/blueprints-route-validation.test.ts @@ -129,3 +129,71 @@ describe('POST /api/blueprints/apply-local (node-to-node atomic apply)', () => { expect(StackOpLockService.getInstance().get(1, 'apply-local-busy')?.action).toBe('update'); }); }); + +describe('POST /api/blueprints/withdraw-local', () => { + let viewerCookie: string; + + beforeAll(async () => { + const bcrypt = (await import('bcrypt')).default; + const passwordHash = await bcrypt.hash('bp-wd-viewer-pass', 1); + DatabaseService.getInstance().addUser({ + username: 'bp-wd-viewer', + password_hash: passwordHash, + role: 'viewer', + }); + const res = await request(app) + .post('/api/auth/login') + .send({ username: 'bp-wd-viewer', password: 'bp-wd-viewer-pass' }); + const cookies = res.headers['set-cookie'] as string | string[]; + viewerCookie = Array.isArray(cookies) ? cookies[0] : cookies; + }); + + it('rejects an invalid stack name', async () => { + const res = await request(app) + .post('/api/blueprints/withdraw-local') + .set('Cookie', adminCookie) + .send({ stackName: '../escape', blueprintId: 1 }); + expect(res.status).toBe(400); + expect(res.body.error).toContain('Invalid stack name'); + }); + + it('rejects a non-positive blueprintId', async () => { + const res = await request(app) + .post('/api/blueprints/withdraw-local') + .set('Cookie', adminCookie) + .send({ stackName: 'wd-local-stack', blueprintId: 0 }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/blueprintId/i); + }); + + it('returns 403 for a viewer without stack:delete', async () => { + const res = await request(app) + .post('/api/blueprints/withdraw-local') + .set('Cookie', viewerCookie) + .send({ stackName: 'wd-local-stack', blueprintId: 1 }); + expect(res.status).toBe(403); + }); + + it('returns 409 self_stack_protected for Sencho own stack', async () => { + const selfStackGuard = await import('../helpers/selfStackGuard'); + vi.spyOn(selfStackGuard, 'refuseIfSelfStack').mockImplementation(async (_req, res) => { + res.status(409).json({ error: 'self', code: 'self_stack_protected' }); + return true; + }); + const res = await request(app) + .post('/api/blueprints/withdraw-local') + .set('Cookie', adminCookie) + .send({ stackName: 'sencho-self', blueprintId: 1 }); + expect(res.status).toBe(409); + expect(res.body.code).toBe('self_stack_protected'); + }); + + it('returns already_absent when the stack directory is missing', async () => { + const res = await request(app) + .post('/api/blueprints/withdraw-local') + .set('Cookie', adminCookie) + .send({ stackName: `wd-absent-${Date.now()}`, blueprintId: 42 }); + expect(res.status).toBe(200); + expect(res.body.status).toBe('already_absent'); + }); +}); diff --git a/backend/src/__tests__/blueprints.test.ts b/backend/src/__tests__/blueprints.test.ts index 337298e4..261a0e22 100644 --- a/backend/src/__tests__/blueprints.test.ts +++ b/backend/src/__tests__/blueprints.test.ts @@ -399,7 +399,7 @@ describe('BlueprintReconciler developer-mode diagnostics', () => { }); describe('BlueprintService per-stack lock', () => { - it('deploy under a free lock writes compose then marker, then deploys', async () => { + it('deploy under a free lock writes compose, cleans siblings, deploys, then writes the marker', async () => { const nodeId = seedNode(); const bp = seedBlueprint({ classification: 'stateless', nodeIds: [nodeId] }); const node = DatabaseService.getInstance().getNode(nodeId)!; @@ -419,7 +419,7 @@ describe('BlueprintService per-stack lock', () => { source: 'blueprint', actor: 'system:blueprint', }); - // Compose is written first, then the marker, both before sibling cleanup and deploy. + // Compose is written first; the marker is deferred until after sibling cleanup and deploy. expect(writeSpy).toHaveBeenCalledTimes(2); expect(writeSpy.mock.calls[0][1]).toBe('compose.yaml'); expect(writeSpy.mock.calls[0][2]).toBe(bp.compose_content); @@ -429,9 +429,9 @@ describe('BlueprintService per-stack lock', () => { const [composeOrder, markerOrder] = writeSpy.mock.invocationCallOrder; const [cleanupOrder] = cleanupSpy.mock.invocationCallOrder; const [deployOrder] = deploySpy.mock.invocationCallOrder; - expect(composeOrder).toBeLessThan(markerOrder); - expect(markerOrder).toBeLessThan(cleanupOrder); + expect(composeOrder).toBeLessThan(cleanupOrder); expect(cleanupOrder).toBeLessThan(deployOrder); + expect(deployOrder).toBeLessThan(markerOrder); }); it('deploy skips, writes no stack files, and records failed when the stack lock is held', async () => { @@ -476,7 +476,10 @@ describe('BlueprintService local withdraw clears stack-scoped role assignments', async function arrangeLocalWithdraw() { const db = DatabaseService.getInstance(); + const composeDir = process.env.COMPOSE_DIR!; const nodeId = seedNode(); + db.getDb().prepare('UPDATE nodes SET compose_dir = ? WHERE id = ?').run(composeDir, nodeId); + const bp = seedBlueprint({ name: `rbac-wd-${counter}`, classification: 'stateless', nodeIds: [nodeId] }); const node = db.getNode(nodeId)!; db.upsertDeployment({ @@ -499,7 +502,15 @@ describe('BlueprintService local withdraw clears stack-scoped role assignments', user_id: userId, role: 'deployer', resource_type: 'node', resource_id: String(otherNodeId), }); - vi.spyOn(BlueprintService.getInstance(), 'readMarker').mockResolvedValue(null); + // Matching on-disk marker so ownership passes; FS/down are stubbed. + const fs = await import('fs'); + const path = await import('path'); + const stackDir = path.join(composeDir, bp.name); + fs.mkdirSync(stackDir, { recursive: true }); + fs.writeFileSync( + path.join(stackDir, '.blueprint.json'), + JSON.stringify({ blueprintId: bp.id, revision: bp.revision, lastApplied: Date.now() }), + ); const { ComposeService } = await import('../services/ComposeService'); const { FileSystemService } = await import('../services/FileSystemService'); vi.spyOn(ComposeService.prototype, 'downStack').mockResolvedValue(undefined); @@ -522,15 +533,16 @@ describe('BlueprintService local withdraw clears stack-scoped role assignments', 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. + it('clears the target assignment when the stack directory is already absent', async () => { const { bp, node, userId, deleteStackSpy, db } = await arrangeLocalWithdraw(); - deleteStackSpy.mockResolvedValue(undefined); + const fs = await import('fs'); + const path = await import('path'); + fs.rmSync(path.join(process.env.COMPOSE_DIR!, bp.name), { recursive: true, force: true }); const outcome = await BlueprintService.getInstance().withdrawFromNode(bp, node); expect(outcome.status).toBe('withdrawn'); - expect(deleteStackSpy).toHaveBeenCalledWith(bp.name); + expect(deleteStackSpy).not.toHaveBeenCalled(); expect(hasAssignment(userId, 'stack', bp.name)).toBe(false); expect(db.getAllRoleAssignments(userId)).toHaveLength(2); db.deleteUser(userId); diff --git a/backend/src/__tests__/database-stack-cleanup-intent-migration.test.ts b/backend/src/__tests__/database-stack-cleanup-intent-migration.test.ts new file mode 100644 index 00000000..9723050b --- /dev/null +++ b/backend/src/__tests__/database-stack-cleanup-intent-migration.test.ts @@ -0,0 +1,64 @@ +/** + * Legacy installations upgrade stack_update_cleanup_pending with nullable + * required_blueprint_id via maybeAddCol. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb'; + +let tmpDir: string; +let DatabaseService: typeof import('../services/DatabaseService').DatabaseService; + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ DatabaseService } = await import('../services/DatabaseService')); +}); + +afterAll(() => cleanupTestDb(tmpDir)); + +describe('stack_update_cleanup_pending required_blueprint_id migration', () => { + it('upgrades a legacy table without the column and preserves null for old rows', () => { + const db = DatabaseService.getInstance(); + const raw = db.getDb(); + + // Simulate a pre-migration installation: drop the column if present, then + // re-add it the same way initSchema's maybeAddCol does. + try { + raw.exec('ALTER TABLE stack_update_cleanup_pending DROP COLUMN required_blueprint_id'); + } catch { + // Column may already be absent in a hand-built fixture. + } + + const now = Date.now(); + raw.prepare(` + INSERT INTO stack_update_cleanup_pending ( + id, node_id, stack_name, status, target_kind, rollback_tags_json, + override_paths_json, prune_volumes_requested, created_at, updated_at + ) VALUES (?, ?, ?, 'prepared', 'local_socket', '[]', '[]', 0, ?, ?) + `).run('legacy-1', 1, 'legacy-stack', now, now); + + try { + raw.exec('ALTER TABLE stack_update_cleanup_pending ADD COLUMN required_blueprint_id INTEGER'); + } catch { + // Idempotent if a parallel path re-added it. + } + + const legacy = db.getCleanupPending('legacy-1'); + expect(legacy).toBeDefined(); + expect(legacy?.required_blueprint_id ?? null).toBeNull(); + + db.insertCleanupPending({ + id: 'owned-1', + node_id: 2, + stack_name: 'bp-stack', + status: 'prepared', + target_kind: 'local_socket', + rollback_tags_json: '[]', + override_paths_json: '[]', + prune_volumes_requested: 0, + required_blueprint_id: 42, + created_at: now, + updated_at: now, + }); + expect(db.getCleanupPending('owned-1')?.required_blueprint_id).toBe(42); + }); +}); diff --git a/backend/src/__tests__/deployed-stack-deletion-service.test.ts b/backend/src/__tests__/deployed-stack-deletion-service.test.ts index 27052fd8..d8dfc0b4 100644 --- a/backend/src/__tests__/deployed-stack-deletion-service.test.ts +++ b/backend/src/__tests__/deployed-stack-deletion-service.test.ts @@ -89,6 +89,7 @@ describe('DeployedStackDeletionService ready transaction', () => { rollback_tags_json: '[]', override_paths_json: '[]', prune_volumes_requested: 0, + required_blueprint_id: null, created_at: now, updated_at: now, }; @@ -111,6 +112,7 @@ describe('DeployedStackDeletionService ready transaction', () => { rollback_tags_json: '[]', override_paths_json: '[]', prune_volumes_requested: 0, + required_blueprint_id: null, created_at: now, updated_at: now, }); @@ -129,6 +131,7 @@ describe('DeployedStackDeletionService ready transaction', () => { rollback_tags_json: '[]', override_paths_json: '[]', prune_volumes_requested: 0, + required_blueprint_id: null, created_at: now, updated_at: now, }); @@ -154,3 +157,39 @@ describe('overrideDeletionContainmentBase', () => { }); }); +describe('DeployedStackDeletionService blueprint ownership probe', () => { + it('returns failed (not name_conflict) when marker read fails with non-ENOENT I/O', async () => { + const { promises: fsPromises } = await import('fs'); + const { vi } = await import('vitest'); + const composeDir = process.env.COMPOSE_DIR!; + const stackName = `del-probe-${Date.now()}`; + const stackDir = path.join(composeDir, stackName); + await fsPromises.mkdir(stackDir, { recursive: true }); + await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), 'services: {}\n'); + await fsPromises.writeFile( + path.join(stackDir, '.blueprint.json'), + JSON.stringify({ blueprintId: 7, revision: 1, lastApplied: 0 }), + ); + + const accessErr = Object.assign(new Error('EACCES'), { code: 'EACCES' }); + const readSpy = vi.spyOn(fsPromises, 'readFile').mockRejectedValueOnce(accessErr); + + const result = await DeployedStackDeletionService.getInstance().deleteDeployedStack({ + nodeId: NODE, + stackName, + pruneVolumes: false, + actor: 'test', + requireBlueprintId: 7, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.code).toBe('failed'); + expect(result.error).toMatch(/EACCES|Failed to read|permission/i); + } + expect(readSpy).toHaveBeenCalled(); + readSpy.mockRestore(); + await fsPromises.rm(stackDir, { recursive: true, force: true }); + }); +}); + diff --git a/backend/src/__tests__/filesystem-stack-paths.test.ts b/backend/src/__tests__/filesystem-stack-paths.test.ts index abf7d2e7..00b29665 100644 --- a/backend/src/__tests__/filesystem-stack-paths.test.ts +++ b/backend/src/__tests__/filesystem-stack-paths.test.ts @@ -95,9 +95,10 @@ describe('FileSystemService stack methods', () => { expect(fileNames).toEqual(['.env', 'compose.yaml']); }); - it('marks compose.yaml and .env as protected', async () => { + it('marks compose.yaml, .env, and .blueprint.json as protected', async () => { await fs.writeFile(path.join(stackDir, 'compose.yaml'), ''); await fs.writeFile(path.join(stackDir, '.env'), ''); + await fs.writeFile(path.join(stackDir, '.blueprint.json'), '{}'); await fs.writeFile(path.join(stackDir, 'custom.conf'), ''); const service = FileSystemService.getInstance(); @@ -106,6 +107,7 @@ describe('FileSystemService stack methods', () => { const byName = Object.fromEntries(entries.map(e => [e.name, e])); expect(byName['compose.yaml'].isProtected).toBe(true); expect(byName['.env'].isProtected).toBe(true); + expect(byName['.blueprint.json'].isProtected).toBe(true); expect(byName['custom.conf'].isProtected).toBe(false); }); diff --git a/backend/src/__tests__/stack-files-routes.test.ts b/backend/src/__tests__/stack-files-routes.test.ts index 37373309..2195bb10 100644 --- a/backend/src/__tests__/stack-files-routes.test.ts +++ b/backend/src/__tests__/stack-files-routes.test.ts @@ -1116,6 +1116,37 @@ describe('PUT /api/stacks/:stackName/files/content', () => { expect(content).toBe('community-write'); }); + it('blocks root trust file writes while a stack op lock is held', async () => { + const { StackOpLockService } = await import('../services/StackOpLockService'); + StackOpLockService.getInstance().tryAcquire(1, STACK, 'deploy', 'admin'); + try { + const composeRes = await request(app) + .put(`/api/stacks/${STACK}/files/content`) + .query({ path: 'compose.yaml' }) + .set('Cookie', adminCookie) + .send({ content: 'services:\n app:\n image: nginx\n' }); + expect(composeRes.status).toBe(409); + expect(composeRes.body.code).toBe('stack_op_in_progress'); + + const markerRes = await request(app) + .put(`/api/stacks/${STACK}/files/content`) + .query({ path: '.blueprint.json' }) + .set('Cookie', adminCookie) + .send({ content: '{"blueprintId":1,"revision":1,"lastApplied":0}' }); + expect(markerRes.status).toBe(409); + expect(markerRes.body.code).toBe('stack_op_in_progress'); + + const nestedRes = await request(app) + .put(`/api/stacks/${STACK}/files/content`) + .query({ path: 'config/app.conf' }) + .set('Cookie', adminCookie) + .send({ content: 'ok' }); + expect(nestedRes.status).toBe(204); + } finally { + StackOpLockService.getInstance().release(1, STACK); + } + }); + it('returns 400 when content is not a string', async () => { const res = await request(app) .put(`/api/stacks/${STACK}/files/content`) @@ -1931,6 +1962,16 @@ describe('protected stack files', () => { expect(res.body.code).toBe('PROTECTED_FILE'); }); + it('DELETE /files refuses .blueprint.json with 409 PROTECTED_FILE', async () => { + await fs.writeFile(path.join(stacksDir, STACK, '.blueprint.json'), '{"blueprintId":1,"revision":1}\n'); + const res = await request(app) + .delete(`/api/stacks/${STACK}/files`) + .query({ path: '.blueprint.json' }) + .set('Cookie', adminCookie); + expect(res.status).toBe(409); + expect(res.body.code).toBe('PROTECTED_FILE'); + }); + it('PATCH /files/rename refuses compose.yaml as source with 409 PROTECTED_FILE', async () => { const res = await request(app) .patch(`/api/stacks/${STACK}/files/rename`) diff --git a/backend/src/helpers/blueprintMarker.ts b/backend/src/helpers/blueprintMarker.ts new file mode 100644 index 00000000..d341f594 --- /dev/null +++ b/backend/src/helpers/blueprintMarker.ts @@ -0,0 +1,28 @@ +/** + * Dependency-neutral blueprint marker parse helpers. + * Used by BlueprintService and DeployedStackDeletionService without a service import cycle. + */ + +export const BLUEPRINT_MARKER_FILENAME = '.blueprint.json'; + +export interface BlueprintMarker { + blueprintId: number; + revision: number; + lastApplied: number; +} + +export function parseBlueprintMarker(content: string): BlueprintMarker | null { + try { + const parsed: unknown = JSON.parse(content); + if (!parsed || typeof parsed !== 'object') return null; + const obj = parsed as Record; + if (typeof obj.blueprintId !== 'number' || typeof obj.revision !== 'number') return null; + return { + blueprintId: obj.blueprintId, + revision: obj.revision, + lastApplied: typeof obj.lastApplied === 'number' ? obj.lastApplied : 0, + }; + } catch { + return null; + } +} diff --git a/backend/src/routes/blueprints.ts b/backend/src/routes/blueprints.ts index ecdef6db..84c113d8 100644 --- a/backend/src/routes/blueprints.ts +++ b/backend/src/routes/blueprints.ts @@ -7,7 +7,9 @@ import { type BlueprintSelector, type DriftMode, } from '../services/DatabaseService'; -import { BlueprintService } from '../services/BlueprintService'; +import { BlueprintService, BlueprintNameConflictError, BlueprintOwnershipProbeError } from '../services/BlueprintService'; +import { DeployedStackDeletionService } from '../services/DeployedStackDeletionService'; +import { refuseIfSelfStack } from '../helpers/selfStackGuard'; import { BlueprintReconciler, messageForConfirmedOutcomes, @@ -317,12 +319,9 @@ blueprintsRouter.delete('/:id', async (req: Request, res: Response): Promise n.id === dep.node_id); if (!node) continue; try { - await BlueprintService.getInstance().withdrawFromNode(blueprint, node); + const outcome = await BlueprintService.getInstance().withdrawFromNode(blueprint, node); + if (outcome.status !== 'withdrawn') { + res.status(409).json({ + error: `Cannot delete blueprint: withdraw on node "${node.name}" ended as ${outcome.status}. Resolve that deployment, then retry.`, + code: 'withdraw_failed_blocking_delete', + nodeId: node.id, + withdrawStatus: outcome.status, + }); + return; + } } catch (err) { console.warn(`[Blueprints] Pre-delete withdraw failed for blueprint ${id} on node ${node.id}:`, err); + res.status(409).json({ + error: `Cannot delete blueprint: withdraw on node "${node.name}" failed. Resolve that deployment, then retry.`, + code: 'withdraw_failed_blocking_delete', + nodeId: node.id, + }); + return; } } DatabaseService.getInstance().deleteBlueprint(id); @@ -384,11 +398,68 @@ blueprintsRouter.post('/apply-local', async (req: Request, res: Response): Promi } res.json({ deployed: true }); } catch (error) { + if (error instanceof BlueprintNameConflictError) { + res.status(409).json({ error: error.message, code: 'name_conflict' }); + return; + } + if (error instanceof BlueprintOwnershipProbeError) { + console.error('[Blueprints] apply-local ownership probe failed:', sanitizeForLog(error.message)); + res.status(500).json({ error: error.message }); + return; + } console.error('[Blueprints] apply-local error:', sanitizeForLog(getErrorMessage(error, 'apply failed'))); res.status(500).json({ error: getErrorMessage(error, 'Blueprint apply failed') }); } }); +// Node-to-node atomic blueprint withdraw. Ownership is validated under the +// delete lock on this node. Requires stack:delete and refuses Sencho's own stack. +blueprintsRouter.post('/withdraw-local', async (req: Request, res: Response): Promise => { + const body = (req.body ?? {}) as { stackName?: unknown; blueprintId?: unknown }; + if (typeof body.stackName !== 'string' || !isValidStackName(body.stackName)) { + res.status(400).json({ error: 'Invalid stack name' }); + return; + } + if (typeof body.blueprintId !== 'number' || !Number.isInteger(body.blueprintId) || body.blueprintId <= 0) { + res.status(400).json({ error: 'blueprintId must be a positive integer' }); + return; + } + if (!requirePermission(req, res, 'stack:delete', 'stack', body.stackName)) return; + if (await refuseIfSelfStack(req, res, body.stackName)) return; + + try { + const result = await DeployedStackDeletionService.getInstance().deleteDeployedStack({ + nodeId: req.nodeId, + stackName: body.stackName, + pruneVolumes: false, + actor: req.user?.username ?? 'system:blueprint', + requireBlueprintId: body.blueprintId, + }); + if (result.ok) { + res.json({ + status: result.status === 'already_absent' ? 'already_absent' : 'withdrawn', + }); + return; + } + if (result.code === 'name_conflict') { + res.status(409).json({ error: result.error, code: 'name_conflict' }); + return; + } + if (result.code === 'lock_conflict') { + res.status(409).json({ + error: result.error, + code: 'stack_op_in_progress', + inProgress: { action: result.existingAction }, + }); + return; + } + res.status(500).json({ error: result.error }); + } catch (error) { + console.error('[Blueprints] withdraw-local error:', sanitizeForLog(getErrorMessage(error, 'withdraw failed'))); + res.status(500).json({ error: getErrorMessage(error, 'Blueprint withdraw failed') }); + } +}); + blueprintsRouter.post('/:id/apply', async (req: Request, res: Response): Promise => { if (!requireAdmin(req, res)) return; const id = parseIntParam(req, res, 'id'); diff --git a/backend/src/routes/stacks.ts b/backend/src/routes/stacks.ts index 4fe318de..6df26f2a 100644 --- a/backend/src/routes/stacks.ts +++ b/backend/src/routes/stacks.ts @@ -135,6 +135,40 @@ function releaseStackOpLock(req: Request, stackName: string): void { StackOpLockService.getInstance().release(req.nodeId, stackName); } +/** Root compose + blueprint marker on the stack-source root must not change while a lifecycle op holds the stack lock. */ +const STACK_OP_LOCKED_ROOT_TRUST_FILES = new Set([ + 'compose.yaml', + 'compose.yml', + 'docker-compose.yaml', + 'docker-compose.yml', + '.blueprint.json', +]); + +function rejectIfStackOpBlocksRootTrustFileWrite( + req: Request, + res: Response, + stackName: string, + relPath: string, + root: StackFileRoot, +): boolean { + if (root.kind !== 'stack-source') return false; + if (relPath.includes('/')) return false; + const base = relPath.toLowerCase(); + if (!STACK_OP_LOCKED_ROOT_TRUST_FILES.has(base)) return false; + const existing = StackOpLockService.getInstance().get(req.nodeId, stackName); + if (!existing) return false; + res.status(409).json({ + error: `${stackName} is busy: another operation (${existing.action}) is already in progress`, + code: 'stack_op_in_progress', + inProgress: { + action: existing.action, + startedAt: existing.startedAt, + user: existing.user, + }, + }); + return true; +} + function stackFileEtag(mtimeMs: number): string { return `W/"${Math.floor(mtimeMs)}"`; } @@ -2884,6 +2918,10 @@ stacksRouter.post( return res.status(400).json({ error: 'Invalid filename' }); } const targetRelPath = relPath ? `${relPath}/${originalName}` : originalName; + if (rejectIfStackOpBlocksRootTrustFileWrite(req, res, stackName, targetRelPath, root)) { + await cleanupUploadTemp(req); + return; + } const overwrite = String(req.query.overwrite) === '1'; // The multer wrapper stashed the route-entry timestamp on the request so // the success path and the rejection paths share one window. Fall back to @@ -2987,6 +3025,7 @@ stacksRouter.put('/:stackName/files/content', async (req: Request, res: Response const expectedVersion = req.header('if-match') || undefined; const root = await resolveRootForOp(req, res, stackName, 'write'); if (!root) return; + if (rejectIfStackOpBlocksRootTrustFileWrite(req, res, stackName, relPath, root)) return; const startedAt = Date.now(); logFileDiag('write start', { stackName, relPath, nodeId: req.nodeId, bytes: Buffer.byteLength(content, 'utf-8'), hasIfMatch: expectedVersion !== undefined, rootKind: root.kind }); try { diff --git a/backend/src/services/BlueprintService.ts b/backend/src/services/BlueprintService.ts index e9a8aa69..adddbee8 100644 --- a/backend/src/services/BlueprintService.ts +++ b/backend/src/services/BlueprintService.ts @@ -19,8 +19,12 @@ import { assertPolicyGateAllows, buildSystemPolicyGateOptions, describePolicyBlo import { enforcePolicyForImageRefs } from './PolicyEnforcement'; import { BlueprintAnalyzer } from './BlueprintAnalyzer'; import { sanitizeForLog } from '../utils/safeLog'; - -const MARKER_FILENAME = '.blueprint.json'; +import { isPathWithinBase } from '../utils/validation'; +import { + BLUEPRINT_MARKER_FILENAME, + parseBlueprintMarker, + type BlueprintMarker, +} from '../helpers/blueprintMarker'; /** On-disk compose name for Blueprint applies. Must match createStack scaffold and Sencho discovery priority. */ const COMPOSE_FILENAME = 'compose.yaml'; const REMOTE_HTTP_TIMEOUT_MS = 30_000; @@ -41,10 +45,32 @@ function diagnosticLog(message: string, fields: Record { try { if (node.type === 'local') { - const baseDir = NodeRegistry.getInstance().getComposeDir(node.id); - const markerPath = path.resolve(baseDir, blueprintName, MARKER_FILENAME); - if (!markerPath.startsWith(path.resolve(baseDir))) return null; - const content = await fsPromises.readFile(markerPath, 'utf-8'); - return BlueprintService.parseMarker(content); + const markerRead = await this.readLocalMarkerFromDisk(node.id, blueprintName); + return markerRead.kind === 'present' ? markerRead.marker : null; } const target = NodeRegistry.getInstance().getProxyTarget(node.id); if (!target) return null; - const url = `${target.apiUrl.replace(/\/$/, '')}/api/stacks/${encodeURIComponent(blueprintName)}/files/content?path=${encodeURIComponent(MARKER_FILENAME)}`; + const url = `${target.apiUrl.replace(/\/$/, '')}/api/stacks/${encodeURIComponent(blueprintName)}/files/content?path=${encodeURIComponent(BLUEPRINT_MARKER_FILENAME)}`; const res = await axios.get(url, { headers: this.remoteHeaders(target.apiToken), timeout: REMOTE_HTTP_TIMEOUT_MS, @@ -146,7 +175,7 @@ export class BlueprintService { const body = res.data; const content = typeof body === 'string' ? body : (typeof body?.content === 'string' ? body.content : null); if (content == null) return null; - return BlueprintService.parseMarker(content); + return parseBlueprintMarker(content); } catch { return null; } @@ -154,47 +183,77 @@ export class BlueprintService { /** * Returns true when a stack directory by this name exists on the target - * node but does not carry our marker file. The reconciler must not - * deploy in that case: there is a real user-authored stack with the - * same name and we must not overwrite it. + * node and the on-disk marker is missing, malformed, or references a + * different blueprint ID. Throws BlueprintOwnershipProbeError when the + * directory or marker cannot be probed (non-ENOENT I/O or remote list failure). */ - async hasNameConflict(blueprintName: string, node: Node): Promise { - try { - if (node.type === 'local') { - const baseDir = NodeRegistry.getInstance().getComposeDir(node.id); - const stackDir = path.resolve(baseDir, blueprintName); - if (!stackDir.startsWith(path.resolve(baseDir))) return true; - try { - const stat = await fsPromises.stat(stackDir); - if (!stat.isDirectory()) return false; - } catch { - return false; // directory doesn't exist → no conflict - } - const markerPath = path.join(stackDir, MARKER_FILENAME); - try { - await fsPromises.stat(markerPath); - return false; // marker present → ours - } catch { - return true; // directory exists but no marker → conflict - } + async hasNameConflict(blueprintName: string, node: Node, blueprintId: number): Promise { + if (node.type === 'local') { + const baseDir = NodeRegistry.getInstance().getComposeDir(node.id); + const stackDir = path.resolve(baseDir, blueprintName); + if (!isPathWithinBase(stackDir, baseDir)) return true; + try { + const stat = await fsPromises.stat(stackDir); + if (!stat.isDirectory()) return false; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return false; + throw BlueprintService.ownershipProbeError(blueprintName, BlueprintService.formatError(err)); } - const target = NodeRegistry.getInstance().getProxyTarget(node.id); - if (!target) return false; - const baseUrl = target.apiUrl.replace(/\/$/, ''); - const listUrl = `${baseUrl}/api/stacks`; - const listRes = await axios.get(listUrl, { + const markerRead = await this.readLocalMarkerFromDisk(node.id, blueprintName); + if (markerRead.kind === 'failed') { + throw BlueprintService.ownershipProbeError(blueprintName, markerRead.error); + } + return markerRead.kind === 'missing' || markerRead.marker.blueprintId !== blueprintId; + } + const target = NodeRegistry.getInstance().getProxyTarget(node.id); + if (!target) { + throw new BlueprintOwnershipProbeError( + `Cannot verify stack ownership on remote node "${node.name}": no proxy target configured`, + ); + } + const baseUrl = target.apiUrl.replace(/\/$/, ''); + let listRes; + try { + listRes = await axios.get(`${baseUrl}/api/stacks`, { headers: this.remoteHeaders(target.apiToken), timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true, }); - if (listRes.status !== 200) return false; - const stacks = Array.isArray(listRes.data) ? listRes.data as Array<{ name?: string }> : []; - const exists = stacks.some(s => s?.name === blueprintName); - if (!exists) return false; - const marker = await this.readMarker(blueprintName, node); - return marker == null; - } catch { - return false; + } catch (err) { + throw new BlueprintOwnershipProbeError( + `Cannot verify stack ownership on remote node "${node.name}": ${BlueprintService.formatError(err)}`, + ); + } + if (listRes.status !== 200) { + throw new BlueprintOwnershipProbeError( + `Cannot verify stack ownership on remote node "${node.name}" (HTTP ${listRes.status})`, + ); + } + const stacks = Array.isArray(listRes.data) ? listRes.data as Array<{ name?: string }> : []; + const exists = stacks.some(s => s?.name === blueprintName); + if (!exists) return false; + const marker = await this.readMarker(blueprintName, node); + return marker == null || marker.blueprintId !== blueprintId; + } + + /** Read and parse a local on-disk marker without going through the remote HTTP path. */ + private async readLocalMarkerFromDisk(nodeId: number, stackName: string): Promise { + try { + // Canonical js/path-injection barrier inline with the read sink. + const baseResolved = path.resolve(NodeRegistry.getInstance().getComposeDir(nodeId)); + const safePath = path.resolve(baseResolved, stackName, BLUEPRINT_MARKER_FILENAME); + if (!safePath.startsWith(baseResolved + path.sep)) { + return { kind: 'failed', error: 'Invalid stack path for blueprint marker' }; + } + const content = await fsPromises.readFile(safePath, 'utf-8'); + const marker = parseBlueprintMarker(content); + if (!marker) return { kind: 'missing' }; + return { kind: 'present', marker }; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return { kind: 'missing' }; + return { kind: 'failed', error: BlueprintService.formatError(err) }; } } @@ -222,7 +281,7 @@ export class BlueprintService { }); try { this.setStatus(blueprint.id, node.id, 'deploying'); - if (await this.hasNameConflict(blueprint.name, node)) { + if (await this.hasNameConflict(blueprint.name, node, blueprint.id)) { this.setStatus(blueprint.id, node.id, 'name_conflict', { last_error: `A stack named "${blueprint.name}" already exists on this node and is not managed by Sencho.`, }); @@ -249,6 +308,12 @@ export class BlueprintService { sanitizeForLog(blueprint.name), node.id, Date.now() - started); return { status: 'active' }; } catch (err) { + if (err instanceof BlueprintNameConflictError) { + this.setStatus(blueprint.id, node.id, 'name_conflict', { last_error: err.message }); + console.warn('[BlueprintService] deploy name conflict blueprint=%s node=%s durationMs=%s', + sanitizeForLog(blueprint.name), node.id, Date.now() - started); + return { status: 'name_conflict', error: 'name_conflict' }; + } const message = BlueprintService.formatError(err); this.setStatus(blueprint.id, node.id, 'failed', { last_error: message }); console.error('[BlueprintService] deploy failed blueprint=%s node=%s durationMs=%s error=%s', @@ -280,20 +345,15 @@ export class BlueprintService { }); try { this.setStatus(blueprint.id, node.id, 'withdrawing'); - // Refuse to withdraw a directory we do not own - const marker = await this.readMarker(blueprint.name, node); - if (marker && marker.blueprintId !== blueprint.id) { - this.setStatus(blueprint.id, node.id, 'name_conflict', { - last_error: `Marker on this node points to a different blueprint (id=${marker.blueprintId}); refusing to withdraw.`, - }); - return { status: 'name_conflict' }; - } + // Ownership is validated on the node that owns the stack, inside the delete lock. if (node.type === 'local') { diagnosticLog('withdraw branch', { blueprintId: blueprint.id, nodeId: node.id, target: 'local' }); - await this.withdrawLocal(blueprint, node); + const localOutcome = await this.withdrawLocal(blueprint, node); + if (localOutcome.status !== 'withdrawn') return localOutcome; } else { diagnosticLog('withdraw branch', { blueprintId: blueprint.id, nodeId: node.id, target: 'remote' }); - await this.withdrawRemote(blueprint, node); + const remoteOutcome = await this.withdrawRemote(blueprint, node); + if (remoteOutcome.status !== 'withdrawn') return remoteOutcome; } DatabaseService.getInstance().deleteDeployment(blueprint.id, node.id); console.info('[BlueprintService] withdraw complete blueprint=%s node=%s durationMs=%s', @@ -382,15 +442,22 @@ export class BlueprintService { // ---- local primitives ---- + /** Returns whether the stack directory exists. Throws on non-ENOENT I/O. */ private async stackDirExists(nodeId: number, blueprintName: string): Promise { const baseDir = NodeRegistry.getInstance().getComposeDir(nodeId); const stackDir = path.resolve(baseDir, blueprintName); - if (!stackDir.startsWith(path.resolve(baseDir))) return false; + if (!isPathWithinBase(stackDir, baseDir)) { + throw new BlueprintOwnershipProbeError(`Invalid stack path for "${blueprintName}"`); + } try { const stat = await fsPromises.stat(stackDir); return stat.isDirectory(); - } catch { - return false; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return false; + throw new BlueprintOwnershipProbeError( + `Cannot access stack directory "${blueprintName}": ${BlueprintService.formatError(err)}`, + ); } } @@ -423,14 +490,14 @@ export class BlueprintService { } /** - * Create the stack if needed, write the compose and marker files, run the - * deploy policy gate, and deploy, all under the per-stack operation lock so - * none of it can race a manual deploy/update/rollback/backup on the same - * stack and node. Runs on the node that owns the stack: deployLocal calls it - * for the hub's own node, and the /api/blueprints/apply-local route calls it - * on a remote node receiving a blueprint apply from its hub (so the file - * writes hold the remote's lock, not just the deploy). On lock conflict - * nothing is written and { ran: false } is returned. + * Create the stack if needed, write the compose file, run the deploy policy + * gate and deploy, then write the marker, all under the per-stack operation + * lock. The marker is written only after a successful deploy so a failed + * apply cannot claim an applied revision that never ran. Runs on the node + * that owns the stack: deployLocal calls it for the hub's own node, and the + * /api/blueprints/apply-local route calls it on a remote node receiving a + * blueprint apply from its hub. On lock conflict nothing is written and + * { ran: false } is returned. */ async applyLocalUnderLock( nodeId: number, @@ -439,47 +506,83 @@ export class BlueprintService { markerContent: string, auditPath: string, ): Promise<{ ran: true } | { ran: false; existingAction: StackOpAction }> { + const expected = parseBlueprintMarker(markerContent); + if (!expected) { + throw new Error('Invalid blueprint marker'); + } const fs = FileSystemService.getInstance(nodeId); const lock = await StackOpLockService.getInstance().runExclusive( nodeId, stackName, 'deploy', 'system', async () => { - if (!(await this.stackDirExists(nodeId, stackName))) { + let createdStack = false; + if (await this.stackDirExists(nodeId, stackName)) { + const existing = await this.readLocalMarkerFromDisk(nodeId, stackName); + if (existing.kind === 'failed') { + throw new BlueprintOwnershipProbeError( + `Cannot verify ownership of stack "${stackName}": ${existing.error}`, + ); + } + if (existing.kind === 'missing' || existing.marker.blueprintId !== expected.blueprintId) { + throw new BlueprintNameConflictError( + `A stack named "${stackName}" already exists on this node and is not managed by this blueprint.`, + ); + } + } else { await fs.createStack(stackName); + createdStack = true; } await fs.writeStackFile(stackName, COMPOSE_FILENAME, composeContent); - await fs.writeStackFile(stackName, MARKER_FILENAME, markerContent); // Clear lower-priority compose siblings so discovery cannot shadow compose.yaml. - // Local + modern apply-local only; legacy remote has no sibling DELETE. await fs.removeAlternateRootComposeFiles(stackName); - await assertPolicyGateAllows( - stackName, - nodeId, - buildSystemPolicyGateOptions('blueprint', { auditPath }), - ); - await ComposeService.getInstance(nodeId).deployStack( - stackName, - undefined, - false, - { source: 'blueprint', actor: 'system:blueprint' }, - ); + try { + await assertPolicyGateAllows( + stackName, + nodeId, + buildSystemPolicyGateOptions('blueprint', { auditPath }), + ); + await ComposeService.getInstance(nodeId).deployStack( + stackName, + undefined, + false, + { source: 'blueprint', actor: 'system:blueprint' }, + ); + await fs.writeStackFile(stackName, BLUEPRINT_MARKER_FILENAME, markerContent); + } catch (err) { + if (createdStack) { + try { + await fs.deleteStack(stackName); + } catch (cleanupErr) { + console.warn( + '[BlueprintService] Failed to roll back newly created stack "%s" after apply error: %s', + sanitizeForLog(stackName), + sanitizeForLog(BlueprintService.formatError(cleanupErr)), + ); + } + } + throw err; + } }, ); return lock.ran ? { ran: true } : { ran: false, existingAction: lock.existing.action }; } - private async withdrawLocal(blueprint: Blueprint, node: Node): Promise { + private async withdrawLocal(blueprint: Blueprint, node: Node): Promise { const result = await DeployedStackDeletionService.getInstance().deleteDeployedStack({ nodeId: node.id, stackName: blueprint.name, pruneVolumes: false, actor: 'system:blueprint', + requireBlueprintId: blueprint.id, }); - if (!result.ok) { - if (result.code === 'lock_conflict') { - throw new Error(result.error); - } - throw new Error(result.error); + if (result.ok) { + return { status: 'withdrawn' }; } + if (result.code === 'name_conflict') { + this.setStatus(blueprint.id, node.id, 'name_conflict', { last_error: result.error }); + return { status: 'name_conflict' }; + } + this.setStatus(blueprint.id, node.id, 'failed', { last_error: result.error }); + return { status: 'failed', error: result.error }; } // ---- remote primitives ---- @@ -500,10 +603,7 @@ export class BlueprintService { const baseUrl = target.apiUrl.replace(/\/$/, ''); const headers = this.remoteHeaders(target.apiToken); - // Atomic apply: the remote runs create + write compose/marker + deploy - // under its own per-stack lock, so the file writes cannot race a manual - // operation on that node. Older nodes without this route answer 404; we - // fall back to the legacy multi-call flow there (not lock-atomic). + // Atomic apply: the remote validates ownership and writes under its stack lock. const res = await axios.post( `${baseUrl}/api/blueprints/apply-local`, { @@ -514,11 +614,17 @@ export class BlueprintService { { headers, timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true }, ); if (res.status === 404) { - console.warn(`[BlueprintService] remote node ${node.id} lacks /api/blueprints/apply-local; using legacy non-atomic apply`); - await this.deployRemoteLegacy(blueprint, node, marker); - return; + throw new BlueprintRemoteUpgradeRequiredError( + `Remote node "${node.name}" does not support atomic blueprint apply (/api/blueprints/apply-local). Upgrade that Sencho instance, then retry.`, + ); } if (res.status === 409) { + if (BlueprintService.extractApiCode(res.data) === 'name_conflict') { + throw new BlueprintNameConflictError( + BlueprintService.extractApiError(res.data) + || `A stack named "${blueprint.name}" already exists on this node and is not managed by this blueprint.`, + ); + } throw new Error(`blueprint apply skipped: ${BlueprintService.extractApiError(res.data) || 'another operation is already in progress'}`); } if (res.status >= 400) { @@ -526,105 +632,62 @@ export class BlueprintService { } } - /** - * Legacy remote apply for nodes that predate /api/blueprints/apply-local: - * create, write compose, write marker, deploy as separate calls. The remote - * deploy locks, but the preceding file writes do not, so this is not atomic - * against a concurrent manual operation on that node. Kept only as a - * compatibility fallback. - */ - private async deployRemoteLegacy(blueprint: Blueprint, node: Node, marker: BlueprintMarker): Promise { + private async withdrawRemote(blueprint: Blueprint, node: Node): Promise { const target = NodeRegistry.getInstance().getProxyTarget(node.id); if (!target) throw new Error(`Remote node "${node.name}" has no proxy target configured`); const baseUrl = target.apiUrl.replace(/\/$/, ''); const headers = this.remoteHeaders(target.apiToken); - // 1. Ensure stack exists. POST returns 409 when already exists; we treat that as success. - const createRes = await axios.post(`${baseUrl}/api/stacks`, - { stackName: blueprint.name }, - { headers, timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true }, - ); - if (createRes.status >= 400 && createRes.status !== 409) { - throw new Error(`create stack: HTTP ${createRes.status} ${BlueprintService.extractApiError(createRes.data)}`); - } - - // 2. Write the compose file - await this.remotePutFile(baseUrl, headers, blueprint.name, COMPOSE_FILENAME, blueprint.compose_content); - - // 3. Write the marker (last so a partial failure leaves us in name_conflict-recoverable state) - await this.remotePutFile(baseUrl, headers, blueprint.name, MARKER_FILENAME, JSON.stringify(marker, null, 2)); - - // 4. Deploy - const deployRes = await axios.post( - `${baseUrl}/api/stacks/${encodeURIComponent(blueprint.name)}/deploy`, - {}, - { headers, timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true }, - ); - if (deployRes.status >= 400) { - throw new Error(`deploy: HTTP ${deployRes.status} ${BlueprintService.extractApiError(deployRes.data)}`); - } - } - - private async withdrawRemote(blueprint: Blueprint, node: Node): Promise { - const target = NodeRegistry.getInstance().getProxyTarget(node.id); - if (!target) throw new Error(`Remote node "${node.name}" has no proxy target configured`); - const baseUrl = target.apiUrl.replace(/\/$/, ''); - const headers = this.remoteHeaders(target.apiToken); - - // down (best-effort) + let res; try { - await axios.post( - `${baseUrl}/api/stacks/${encodeURIComponent(blueprint.name)}/down`, - {}, + res = await axios.post( + `${baseUrl}/api/blueprints/withdraw-local`, + { stackName: blueprint.name, blueprintId: blueprint.id }, { headers, timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true }, ); } catch (err) { - console.warn(`[BlueprintService] remote down failed for "${blueprint.name}" on node ${node.id}: ${BlueprintService.formatError(err)}`); + const message = BlueprintService.formatError(err); + this.setStatus(blueprint.id, node.id, 'failed', { last_error: message }); + return { status: 'failed', error: message }; } - // delete the stack directory entirely - const delRes = await axios.delete( - `${baseUrl}/api/stacks/${encodeURIComponent(blueprint.name)}`, - { headers, timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true }, - ); - if (delRes.status >= 400 && delRes.status !== 404) { - throw new Error(`remote delete: HTTP ${delRes.status} ${BlueprintService.extractApiError(delRes.data)}`); + if (res.status === 404) { + throw new BlueprintRemoteUpgradeRequiredError( + `Remote node "${node.name}" does not support atomic blueprint withdraw (/api/blueprints/withdraw-local). Upgrade that Sencho instance, then retry.`, + ); } - } - - private async remotePutFile( - baseUrl: string, - headers: Record, - stackName: string, - relPath: string, - content: string, - ): Promise { - const url = `${baseUrl}/api/stacks/${encodeURIComponent(stackName)}/files/content?path=${encodeURIComponent(relPath)}`; - const res = await axios.put(url, - { content }, - { headers, timeout: REMOTE_HTTP_TIMEOUT_MS, validateStatus: () => true }, - ); - if (res.status >= 400) { - throw new Error(`PUT ${relPath}: HTTP ${res.status} ${BlueprintService.extractApiError(res.data)}`); + if (res.status === 200) { + return { status: 'withdrawn' }; } + if (res.status === 409) { + const error = BlueprintService.extractApiError(res.data) || 'withdraw refused'; + if (BlueprintService.extractApiCode(res.data) === 'name_conflict') { + this.setStatus(blueprint.id, node.id, 'name_conflict', { last_error: error }); + return { status: 'name_conflict' }; + } + // stack_op_in_progress and any other 409: match local withdraw lock-conflict → failed + this.setStatus(blueprint.id, node.id, 'failed', { last_error: error }); + return { status: 'failed', error }; + } + const message = `blueprint withdraw: HTTP ${res.status} ${BlueprintService.extractApiError(res.data)}`; + this.setStatus(blueprint.id, node.id, 'failed', { last_error: message }); + return { status: 'failed', error: message }; } static parseMarker(content: string): BlueprintMarker | null { - try { - const parsed = JSON.parse(content); - if (parsed && typeof parsed === 'object' - && typeof parsed.blueprintId === 'number' - && typeof parsed.revision === 'number') { - return { - blueprintId: parsed.blueprintId, - revision: parsed.revision, - lastApplied: typeof parsed.lastApplied === 'number' ? parsed.lastApplied : 0, - }; - } - } catch { - // fall through - } - return null; + return parseBlueprintMarker(content); + } + + private static ownershipProbeError(blueprintName: string, detail: string): BlueprintOwnershipProbeError { + return new BlueprintOwnershipProbeError( + `Cannot verify stack ownership for "${blueprintName}": ${detail}`, + ); + } + + static extractApiCode(body: unknown): string { + if (!body || typeof body !== 'object') return ''; + const code = (body as Record).code; + return typeof code === 'string' ? code : ''; } static formatError(err: unknown): string { diff --git a/backend/src/services/DatabaseService.ts b/backend/src/services/DatabaseService.ts index 9f66c424..ace540a4 100644 --- a/backend/src/services/DatabaseService.ts +++ b/backend/src/services/DatabaseService.ts @@ -251,6 +251,8 @@ export interface StackUpdateCleanupPendingRow { rollback_tags_json: string; override_paths_json: string; prune_volumes_requested: number; + /** Blueprint ID that authorized this deletion; null for manual deletes. */ + required_blueprint_id: number | null; created_at: number; updated_at: number; } @@ -1828,6 +1830,7 @@ export class DatabaseService { `); maybeAddCol('stack_update_recovery_generations', 'artifacts_retired', 'INTEGER NOT NULL DEFAULT 0'); + maybeAddCol('stack_update_cleanup_pending', 'required_blueprint_id', 'INTEGER'); // Distributed API model columns maybeAddCol('nodes', 'api_url', "TEXT DEFAULT ''"); @@ -3933,12 +3936,12 @@ export class DatabaseService { this.db.prepare( `INSERT INTO stack_update_cleanup_pending ( id, node_id, stack_name, status, target_kind, rollback_tags_json, - override_paths_json, prune_volumes_requested, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + override_paths_json, prune_volumes_requested, required_blueprint_id, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` ).run( row.id, row.node_id, row.stack_name, row.status, row.target_kind, row.rollback_tags_json, row.override_paths_json, row.prune_volumes_requested, - row.created_at, row.updated_at, + row.required_blueprint_id, row.created_at, row.updated_at, ); } @@ -4409,6 +4412,7 @@ export class DatabaseService { rollback_tags_json: JSON.stringify(localCleanup.tags), override_paths_json: JSON.stringify(localCleanup.overridePaths), prune_volumes_requested: 0, + required_blueprint_id: null, created_at: now, updated_at: now, }); diff --git a/backend/src/services/DeployedStackDeletionService.ts b/backend/src/services/DeployedStackDeletionService.ts index dce2d133..b9ca7b92 100644 --- a/backend/src/services/DeployedStackDeletionService.ts +++ b/backend/src/services/DeployedStackDeletionService.ts @@ -23,6 +23,10 @@ import { StackOpLockService, stackOpSkipMessage } from './StackOpLockService'; import { getErrorMessage } from '../utils/errors'; import { sanitizeForLog } from '../utils/safeLog'; import { isPathWithinBase, isValidStackName } from '../utils/validation'; +import { + BLUEPRINT_MARKER_FILENAME, + parseBlueprintMarker, +} from '../helpers/blueprintMarker'; /** * Directory that may contain recovery override files for a tombstone sweep. @@ -46,13 +50,30 @@ export interface DeleteDeployedStackInput { stackName: string; pruneVolumes: boolean; actor: string; + /** When set, deletion requires an on-disk .blueprint.json matching this blueprint ID. */ + requireBlueprintId?: number; /** When true, skip acquiring a new lock (caller already holds delete via continuation). */ continuationIntentId?: string; } export type DeleteDeployedStackResult = - | { ok: true } - | { ok: false; code: 'lock_conflict' | 'fs_failed' | 'tombstone_failed' | 'db_failed'; error: string; existingAction?: string }; + | { ok: true; status: 'deleted' | 'already_absent' } + | { + ok: false; + code: 'lock_conflict' | 'fs_failed' | 'tombstone_failed' | 'db_failed' | 'name_conflict' | 'failed'; + error: string; + existingAction?: string; + }; + +type DirProbe = { kind: 'absent' } | { kind: 'present' } | { kind: 'error'; error: string }; +type MarkerProbe = + | { kind: 'match' } + | { kind: 'name_conflict'; error: string } + | { kind: 'failed'; error: string }; + +function blueprintMarkerMismatchError(stackName: string): string { + return `Stack "${stackName}" exists without a matching blueprint marker; refusing to withdraw.`; +} function collectArtifactsFromGenerations( generations: Array<{ override_path: string | null; services_json: string }>, @@ -100,6 +121,51 @@ function parseJsonStringArray(raw: string): string[] { } } + +async function probeStackDirectory(nodeId: number, stackName: string): Promise { + // Canonical js/path-injection barrier inline with the stat sink. + const baseResolved = path.resolve(FileSystemService.getInstance(nodeId).getBaseDir()); + const safePath = path.resolve(baseResolved, stackName); + if (!safePath.startsWith(baseResolved + path.sep)) { + return { kind: 'error', error: 'Invalid stack path' }; + } + try { + const stat = await fs.stat(safePath); + return stat.isDirectory() ? { kind: 'present' } : { kind: 'absent' }; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return { kind: 'absent' }; + return { kind: 'error', error: getErrorMessage(error, 'Failed to access stack directory') }; + } +} + +async function probeBlueprintMarkerOwnership( + nodeId: number, + stackName: string, + requireBlueprintId: number, +): Promise { + // Canonical js/path-injection barrier inline with the read sink. + const baseResolved = path.resolve(NodeRegistry.getInstance().getComposeDir(nodeId)); + const safePath = path.resolve(baseResolved, stackName, BLUEPRINT_MARKER_FILENAME); + if (!safePath.startsWith(baseResolved + path.sep)) { + return { kind: 'failed', error: 'Invalid stack path for blueprint marker' }; + } + try { + const content = await fs.readFile(safePath, 'utf-8'); + const marker = parseBlueprintMarker(content); + if (!marker || marker.blueprintId !== requireBlueprintId) { + return { kind: 'name_conflict', error: blueprintMarkerMismatchError(stackName) }; + } + return { kind: 'match' }; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT') { + return { kind: 'name_conflict', error: blueprintMarkerMismatchError(stackName) }; + } + return { kind: 'failed', error: getErrorMessage(error, 'Failed to read blueprint marker') }; + } +} + export class DeployedStackDeletionService { private static instance: DeployedStackDeletionService; @@ -164,6 +230,42 @@ export class DeployedStackDeletionService { const { nodeId, stackName, pruneVolumes } = input; const db = DatabaseService.getInstance(); + // Continuation loads ownership from the persisted intent; first call uses input. + let requiredBlueprintId: number | null = + typeof input.requireBlueprintId === 'number' ? input.requireBlueprintId : null; + + if (existingIntentId) { + const existing = db.getDeletionIntentById(existingIntentId); + if (!existing || existing.status !== 'prepared') { + return { ok: false, code: 'tombstone_failed', error: 'Deletion intent is not prepared' }; + } + if (existing.required_blueprint_id != null) { + requiredBlueprintId = existing.required_blueprint_id; + } + } + + let skipPhysical = false; + if (requiredBlueprintId != null) { + const dirProbe = await probeStackDirectory(nodeId, stackName); + if (dirProbe.kind === 'error') { + return { ok: false, code: 'failed', error: dirProbe.error }; + } + if (dirProbe.kind === 'absent') { + skipPhysical = true; + } else { + const ownership = await probeBlueprintMarkerOwnership(nodeId, stackName, requiredBlueprintId); + if (ownership.kind === 'failed') { + return { ok: false, code: 'failed', error: ownership.error }; + } + if (ownership.kind === 'name_conflict') { + if (existingIntentId) { + db.updateCleanupPendingStatus(existingIntentId, 'cancelled'); + } + return { ok: false, code: 'name_conflict', error: ownership.error }; + } + } + } + let intentId = existingIntentId; if (!intentId) { const { tags, overridePaths } = collectArtifacts(nodeId, stackName); @@ -177,6 +279,7 @@ export class DeployedStackDeletionService { rollback_tags_json: JSON.stringify(tags), override_paths_json: JSON.stringify(overridePaths), prune_volumes_requested: pruneVolumes ? 1 : 0, + required_blueprint_id: requiredBlueprintId, created_at: now, updated_at: now, }; @@ -197,38 +300,53 @@ export class DeployedStackDeletionService { return { ok: false, code: 'tombstone_failed', error: 'Deletion intent is not prepared' }; } - try { - await ComposeService.getInstance(nodeId).downStack(stackName); - } catch (downErr) { - console.warn( - '[DeployedStackDeletion] Compose down failed or no-op for %s:', - sanitizeForLog(stackName), - downErr, - ); - } - - if (intent.prune_volumes_requested === 1) { + if (!skipPhysical) { try { - await DockerController.getInstance(nodeId).pruneManagedOnly('volumes', [stackName]); - } catch (pruneErr) { + await ComposeService.getInstance(nodeId).downStack(stackName); + } catch (downErr) { console.warn( - '[DeployedStackDeletion] Volume prune failed for %s, continuing delete:', + '[DeployedStackDeletion] Compose down failed or no-op for %s:', sanitizeForLog(stackName), - pruneErr, + downErr, ); } + + if (intent.prune_volumes_requested === 1) { + try { + await DockerController.getInstance(nodeId).pruneManagedOnly('volumes', [stackName]); + } catch (pruneErr) { + console.warn( + '[DeployedStackDeletion] Volume prune failed for %s, continuing delete:', + sanitizeForLog(stackName), + pruneErr, + ); + } + } + + try { + await FileSystemService.getInstance(nodeId).deleteStack(stackName); + } catch (fsErr) { + db.updateCleanupPendingStatus(intentId, 'cancelled'); + return { + ok: false, + code: 'fs_failed', + error: getErrorMessage(fsErr, 'Failed to remove stack files'), + }; + } } - try { - await FileSystemService.getInstance(nodeId).deleteStack(stackName); - } catch (fsErr) { - db.updateCleanupPendingStatus(intentId, 'cancelled'); - return { - ok: false, - code: 'fs_failed', - error: getErrorMessage(fsErr, 'Failed to remove stack files'), - }; - } + const finalized = await this.finalizeLogicalDeletion(input, intentId); + if (!finalized.ok) return finalized; + return { ok: true, status: skipPhysical ? 'already_absent' : 'deleted' }; + } + + /** Ready transaction, secondary DB/RBAC cleanup, mesh opt-out, sweep, invalidate. */ + private async finalizeLogicalDeletion( + input: DeleteDeployedStackInput, + intentId: string, + ): Promise { + const { nodeId, stackName } = input; + const db = DatabaseService.getInstance(); if (!db.commitStackDeletionReadyTransaction(intentId, nodeId, stackName)) { return { @@ -282,7 +400,7 @@ export class DeployedStackDeletionService { stackName, ts: Date.now(), }); - return { ok: true }; + return { ok: true, status: 'deleted' }; } /** @@ -497,6 +615,31 @@ export class DeployedStackDeletionService { continue; } + if (intent.required_blueprint_id != null) { + const ownership = await probeBlueprintMarkerOwnership( + nodeId, + stackName, + intent.required_blueprint_id, + ); + if (ownership.kind === 'name_conflict') { + db.updateCleanupPendingStatus(intent.id, 'cancelled'); + console.warn( + '[DeployedStackDeletion] Startup cancelled blueprint deletion for %s: %s', + sanitizeForLog(stackName), + sanitizeForLog(ownership.error), + ); + continue; + } + if (ownership.kind === 'failed') { + console.warn( + '[DeployedStackDeletion] Startup ownership probe failed for %s (leaving prepared): %s', + sanitizeForLog(stackName), + sanitizeForLog(ownership.error), + ); + continue; + } + } + const result = await this.deleteDeployedStack({ nodeId, stackName, diff --git a/backend/src/services/FileSystemService.ts b/backend/src/services/FileSystemService.ts index f0e038b1..fecb9dd0 100644 --- a/backend/src/services/FileSystemService.ts +++ b/backend/src/services/FileSystemService.ts @@ -58,6 +58,13 @@ const PROTECTED_STACK_FILES = new Set([ '.env', ]); +// Explorer-only protection: includes the blueprint ownership marker without +// putting it in PROTECTED_STACK_FILES (backup/rollback orphan removal). +const EXPLORER_PROTECTED_STACK_FILES = new Set([ + ...PROTECTED_STACK_FILES, + '.blueprint.json', +]); + // Bookkeeping markers Sencho writes into the backup slot. They are never copied // back into the stack directory on restore: `.timestamp` records when the backup // was taken; `.checksums` is the integrity manifest verified before a restore. @@ -169,7 +176,7 @@ function isProtectedRelPath(relPath: string): boolean { if (normalized.includes('/')) return false; // Fold case so e.g. a request for COMPOSE.YAML cannot dodge the gate on a // case-insensitive filesystem where it resolves to the real compose.yaml. - return PROTECTED_STACK_FILES.has(fsCaseKey(normalized)); + return EXPLORER_PROTECTED_STACK_FILES.has(fsCaseKey(normalized)); } function protectedFileError(relPath: string): Error & { code: string } { @@ -1643,7 +1650,7 @@ export class FileSystemService { type, size, mtime, - isProtected: protectedEnabled && PROTECTED_STACK_FILES.has(dirent.name), + isProtected: protectedEnabled && EXPLORER_PROTECTED_STACK_FILES.has(dirent.name), }; }) ); @@ -2096,7 +2103,7 @@ export class FileSystemService { type, size: stat.isDirectory() ? 0 : stat.size, mtime: stat.mtimeMs, - isProtected: (scope?.protectedEnabled ?? true) && PROTECTED_STACK_FILES.has(name), + isProtected: (scope?.protectedEnabled ?? true) && EXPLORER_PROTECTED_STACK_FILES.has(name), }; } } diff --git a/backend/src/services/blueprintPreviewProjection.ts b/backend/src/services/blueprintPreviewProjection.ts index f32e2c71..d4991406 100644 --- a/backend/src/services/blueprintPreviewProjection.ts +++ b/backend/src/services/blueprintPreviewProjection.ts @@ -457,15 +457,30 @@ function asApprovedBlueprint(blueprint: Blueprint): Blueprint & BlueprintApprova } /** Upgrade create rows to blockers when an unmanaged same-name stack already exists. */ -async function applyCreateNameConflictBlockers(blueprintName: string, raw: RawAction[]): Promise { +function blockCreateForOwnership(row: RawAction, detail: string): void { + row.action = 'blocked_name_conflict'; + row.severity = 'blocker'; + row.detail = detail; +} + +async function applyCreateNameConflictBlockers( + blueprintName: string, + blueprintId: number, + raw: RawAction[], +): Promise { const { BlueprintService } = await import('./BlueprintService'); const svc = BlueprintService.getInstance(); for (const row of raw) { if (row.action !== 'create') continue; - if (!(await svc.hasNameConflict(blueprintName, row.node))) continue; - row.action = 'blocked_name_conflict'; - row.severity = 'blocker'; - row.detail = 'Unmanaged stack with this name already exists on this node'; + try { + if (!(await svc.hasNameConflict(blueprintName, row.node, blueprintId))) continue; + blockCreateForOwnership(row, 'Unmanaged stack with this name already exists on this node'); + } catch (err) { + blockCreateForOwnership( + row, + err instanceof Error ? err.message : 'Cannot verify stack ownership on this node', + ); + } } } @@ -477,7 +492,7 @@ export async function buildBlueprintPreview(blueprintId: number): Promise Blueprint detail sheet with a deployment row in Awaiting confirmation state @@ -309,7 +309,7 @@ A future Volume Migration feature will automate this with app-aware backup tooli - A directory by the blueprint's name already exists on that node and does not carry the `.blueprint.json` marker. The most likely cause is a manually created stack with the same name. Resolution: rename either the existing stack or the blueprint, then click **Apply now** on the detail sheet to retry. + A directory by the blueprint's name already exists on that node without a matching `.blueprint.json` marker (missing, malformed, or owned by another blueprint). The most likely cause is a manually created stack with the same name. Resolution: rename either the existing stack or the blueprint, then click **Apply now** on the detail sheet to retry. The reconciler will not auto-deploy a stateful blueprint to a node it has never run on. Click **Confirm deploy** on the row, then choose **Deploy fresh** in the dialog. Sencho will create empty named volumes and start the stack. @@ -323,6 +323,9 @@ A future Volume Migration feature will automate this with app-aware backup tooli The row moves to **Failed** with the remote error. Reconnect the node (see [Pilot Agent](/features/pilot-agent) and [Multi-node management](/features/multi-node)), verify whether the stack directory contains `compose.yaml` and `.blueprint.json`, then click **Apply now**. If the directory exists without a matching marker, Sencho treats it as a name conflict until you rename or remove the remote stack manually. + + Blueprint apply and withdraw on a remote node require that node's Sencho build to expose the atomic node-local endpoints (`/api/blueprints/apply-local` and `/api/blueprints/withdraw-local`). Older remotes refuse the mutation with a failed deployment row rather than writing files without ownership checks. Upgrade the remote instance to a matching Sencho release, then retry **Apply now** or withdraw. + The deployment row moves to **Failed** and records the Docker or registry error. Resolve the daemon, socket, registry credentials, rate limit, disk, or volume-permission issue on the affected node, then click **Apply now**. Watch that node's stack activity and security scan status after retry.