fix(#337): Enable Docker sandbox by default and warn when disabled
- Sandbox now enabled by default for security - Logs prominent warning when explicitly disabled - Agents run in containers unless SANDBOX_ENABLED=false Refs #337 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
86
apps/orchestrator/src/config/orchestrator.config.spec.ts
Normal file
86
apps/orchestrator/src/config/orchestrator.config.spec.ts
Normal file
@@ -0,0 +1,86 @@
|
|||||||
|
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||||
|
import { orchestratorConfig } from "./orchestrator.config";
|
||||||
|
|
||||||
|
describe("orchestratorConfig", () => {
|
||||||
|
const originalEnv = process.env;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
process.env = { ...originalEnv };
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
process.env = originalEnv;
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("sandbox.enabled", () => {
|
||||||
|
it("should be enabled by default when SANDBOX_ENABLED is not set", () => {
|
||||||
|
delete process.env.SANDBOX_ENABLED;
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.sandbox.enabled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should be enabled when SANDBOX_ENABLED is set to 'true'", () => {
|
||||||
|
process.env.SANDBOX_ENABLED = "true";
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.sandbox.enabled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should be disabled only when SANDBOX_ENABLED is explicitly set to 'false'", () => {
|
||||||
|
process.env.SANDBOX_ENABLED = "false";
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.sandbox.enabled).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should be enabled for any other value of SANDBOX_ENABLED", () => {
|
||||||
|
process.env.SANDBOX_ENABLED = "yes";
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.sandbox.enabled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should be enabled when SANDBOX_ENABLED is empty string", () => {
|
||||||
|
process.env.SANDBOX_ENABLED = "";
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.sandbox.enabled).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("other config values", () => {
|
||||||
|
it("should use default port when ORCHESTRATOR_PORT is not set", () => {
|
||||||
|
delete process.env.ORCHESTRATOR_PORT;
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.port).toBe(3001);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should use provided port when ORCHESTRATOR_PORT is set", () => {
|
||||||
|
process.env.ORCHESTRATOR_PORT = "4000";
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.port).toBe(4000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should use default valkey config when not set", () => {
|
||||||
|
delete process.env.VALKEY_HOST;
|
||||||
|
delete process.env.VALKEY_PORT;
|
||||||
|
delete process.env.VALKEY_URL;
|
||||||
|
|
||||||
|
const config = orchestratorConfig();
|
||||||
|
|
||||||
|
expect(config.valkey.host).toBe("localhost");
|
||||||
|
expect(config.valkey.port).toBe(6379);
|
||||||
|
expect(config.valkey.url).toBe("redis://localhost:6379");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -22,7 +22,7 @@ export const orchestratorConfig = registerAs("orchestrator", () => ({
|
|||||||
enabled: process.env.KILLSWITCH_ENABLED === "true",
|
enabled: process.env.KILLSWITCH_ENABLED === "true",
|
||||||
},
|
},
|
||||||
sandbox: {
|
sandbox: {
|
||||||
enabled: process.env.SANDBOX_ENABLED === "true",
|
enabled: process.env.SANDBOX_ENABLED !== "false",
|
||||||
defaultImage: process.env.SANDBOX_DEFAULT_IMAGE ?? "node:20-alpine",
|
defaultImage: process.env.SANDBOX_DEFAULT_IMAGE ?? "node:20-alpine",
|
||||||
defaultMemoryMB: parseInt(process.env.SANDBOX_DEFAULT_MEMORY_MB ?? "512", 10),
|
defaultMemoryMB: parseInt(process.env.SANDBOX_DEFAULT_MEMORY_MB ?? "512", 10),
|
||||||
defaultCpuLimit: parseFloat(process.env.SANDBOX_DEFAULT_CPU_LIMIT ?? "1.0"),
|
defaultCpuLimit: parseFloat(process.env.SANDBOX_DEFAULT_CPU_LIMIT ?? "1.0"),
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { ConfigService } from "@nestjs/config";
|
import { ConfigService } from "@nestjs/config";
|
||||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
import { Logger } from "@nestjs/common";
|
||||||
|
import { describe, it, expect, beforeEach, vi, afterEach } from "vitest";
|
||||||
import { DockerSandboxService } from "./docker-sandbox.service";
|
import { DockerSandboxService } from "./docker-sandbox.service";
|
||||||
import Docker from "dockerode";
|
import Docker from "dockerode";
|
||||||
|
|
||||||
@@ -331,4 +332,45 @@ describe("DockerSandboxService", () => {
|
|||||||
expect(disabledService.isEnabled()).toBe(false);
|
expect(disabledService.isEnabled()).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("security warning", () => {
|
||||||
|
let warnSpy: ReturnType<typeof vi.spyOn>;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
warnSpy = vi.spyOn(Logger.prototype, "warn").mockImplementation(() => undefined);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
warnSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should log security warning when sandbox is disabled", () => {
|
||||||
|
const disabledConfigService = {
|
||||||
|
get: vi.fn((key: string, defaultValue?: unknown) => {
|
||||||
|
const config: Record<string, unknown> = {
|
||||||
|
"orchestrator.docker.socketPath": "/var/run/docker.sock",
|
||||||
|
"orchestrator.sandbox.enabled": false,
|
||||||
|
"orchestrator.sandbox.defaultImage": "node:20-alpine",
|
||||||
|
"orchestrator.sandbox.defaultMemoryMB": 512,
|
||||||
|
"orchestrator.sandbox.defaultCpuLimit": 1.0,
|
||||||
|
"orchestrator.sandbox.networkMode": "bridge",
|
||||||
|
};
|
||||||
|
return config[key] !== undefined ? config[key] : defaultValue;
|
||||||
|
}),
|
||||||
|
} as unknown as ConfigService;
|
||||||
|
|
||||||
|
new DockerSandboxService(disabledConfigService, mockDocker);
|
||||||
|
|
||||||
|
expect(warnSpy).toHaveBeenCalledWith(
|
||||||
|
"SECURITY WARNING: Docker sandbox is DISABLED. Agents will run directly on the host without container isolation."
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not log security warning when sandbox is enabled", () => {
|
||||||
|
// Use the default mockConfigService which has sandbox enabled
|
||||||
|
new DockerSandboxService(mockConfigService, mockDocker);
|
||||||
|
|
||||||
|
expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("SECURITY WARNING"));
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -53,6 +53,12 @@ export class DockerSandboxService {
|
|||||||
this.logger.log(
|
this.logger.log(
|
||||||
`DockerSandboxService initialized (enabled: ${this.sandboxEnabled.toString()}, socket: ${socketPath})`
|
`DockerSandboxService initialized (enabled: ${this.sandboxEnabled.toString()}, socket: ${socketPath})`
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if (!this.sandboxEnabled) {
|
||||||
|
this.logger.warn(
|
||||||
|
"SECURITY WARNING: Docker sandbox is DISABLED. Agents will run directly on the host without container isolation."
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user