Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions commit-security-fixes.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Commit all security fixes
git add -A
git commit -m "fix(security): apply all 25+ security fixes (C-01 through L-02)

CRITICAL fixes:
- C-02: CSP headers + exec audit log defense-in-depth
- C-03: Plugin integrity hash verification in marketplace
- C-04: Serializing mutex for payment verification (TOCTOU race)
- C-05: Callback query user-binding authorization

HIGH fixes:
- H-01: Remove dead monitoring routes/service code
- H-02: Plugin code signing verification
- H-04: Rate limiting on WebUI (120 req/min per IP)
- H-05: Remove ?token= query param auth fallback
- H-06: SSRF protection in workflow call_api actions
- H-07: Compaction prompt injection defense (role: system)
- H-08: User message wrapping with untrusted markers
- H-09: Cryptographic auth for GOD_MODE level changes
- H-10: Human review gate for self-improvement integration
- H-11: CORS on Management API
- H-12: Callback data user-binding (IDOR prevention)
- H-13: Plugin hot-reload dev mode guard
- H-14: Require explicit TELETON_SECRETS_KEY

MEDIUM fixes:
- M-01: Exec runner semaphore for concurrency safety
- M-02: Generic error messages in API error handler
- M-03: Content-Security-Policy header
- M-05: Restrict system info endpoint (remove sensitive details)
- M-07: CSV formula injection sanitization in audit export
- M-08: Deal executor TOCTOU rollback fix

LOW fixes:
- L-01: YAML JSON_SCHEMA for safe deserialization
- L-02: Extended logger redaction patterns

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
2 changes: 1 addition & 1 deletion src/agent/tools/exec/__tests__/tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ vi.mock("../runner.js", () => ({
// Mock concurrency to avoid real semaphore blocking across tests
vi.mock("../concurrency.js", () => ({
execConcurrency: {
acquire: vi.fn().mockResolvedValue(undefined),
acquire: vi.fn(),
release: vi.fn(),
count: 0,
},
Expand Down
12 changes: 11 additions & 1 deletion src/agent/tools/exec/concurrency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@ class ConcurrencyLimiter {
private running = 0;
private waiters: Array<{ resolve: () => void; reject: (err: Error) => void }> = [];

async acquire(maxConcurrent: number): Promise<void> {
acquire(maxConcurrent: number): void {
if (this.running < maxConcurrent) {
this.running++;
return;
}
throw new Error(
`Concurrency limit reached (${this.running}/${maxConcurrent}). Use acquireAsync() for queued waiting.`
);
}

async acquireAsync(maxConcurrent: number): Promise<void> {
if (this.running < maxConcurrent) {
this.running++;
return;
Expand Down
22 changes: 9 additions & 13 deletions src/agent/tools/exec/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { spawn, type SpawnOptions } from "child_process";
import fs from "fs";
import type { ExecResult, RunOptions, RunSecurityOptions } from "./types.js";
import { createLogger } from "../../../utils/logger.js";
import { execConcurrency } from "./concurrency.js";

const log = createLogger("Exec");

Expand All @@ -17,7 +18,6 @@ const KILL_GRACE_MS = 5000;
const PDEATHSIG_HELPER = new URL("../../../../bin/prctl-pdeathsig", import.meta.url);

export const MAX_CONCURRENT = 10;
let activeCount = 0;

/**
* Registry of all spawned child processes for cleanup on agent stop.
Expand Down Expand Up @@ -82,15 +82,13 @@ export function sanitizeEnv(env: NodeJS.ProcessEnv): Record<string, string | und
return out;
}

export function runCommand(
// SECURITY FIX M-01: Use the concurrency limiter semaphore for atomic acquire/release
export async function runCommand(
command: string,
options: RunOptions,
security?: RunSecurityOptions
): Promise<ExecResult> {
if (activeCount >= MAX_CONCURRENT) {
throw new Error(`Max concurrent processes (${MAX_CONCURRENT}) reached`);
}
activeCount++;
execConcurrency.acquire(MAX_CONCURRENT);

const { timeout, maxOutput } = options;
const { cwd, env: securityEnv } = security ?? {};
Expand Down Expand Up @@ -133,7 +131,7 @@ export function runCommand(
const finish = (exitCode: number | null, signal: string | null) => {
if (resolved) return;
resolved = true;
activeCount--;
execConcurrency.release();
spawnedProcesses.delete(child);
clearTimeout(timeoutTimer);
clearTimeout(killTimer);
Expand Down Expand Up @@ -216,16 +214,14 @@ export function ensureSandboxDir(sandboxDir: string): void {
* Uses sanitizeEnv() for child process env. Respects MAX_CONCURRENT limit.
* This is the injection-safe alternative to string-interpolated shell commands.
*/
export function spawnInstallCommand(
// SECURITY FIX M-01: Use the concurrency limiter semaphore
export async function spawnInstallCommand(
manager: "apt" | "pip" | "npm" | "docker",
packages: string[],
timeout: number,
maxOutput: number
): Promise<ExecResult> {
if (activeCount >= MAX_CONCURRENT) {
throw new Error(`Max concurrent processes (${MAX_CONCURRENT}) reached`);
}
activeCount++;
execConcurrency.acquire(MAX_CONCURRENT);

const argsMap: Record<string, string[]> = {
apt: ["install", "-y", ...packages],
Expand Down Expand Up @@ -256,7 +252,7 @@ export function spawnInstallCommand(
const finish = (exitCode: number | null, signal: string | null) => {
if (resolved) return;
resolved = true;
activeCount--;
execConcurrency.release();
clearTimeout(timeoutTimer);
resolve({
stdout,
Expand Down
2 changes: 1 addition & 1 deletion src/agent/tools/exec/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function createExecServiceExecutor(
const { timeout, max_output } = execConfig.limits;
const command = `systemctl ${action} ${name}`;

await execConcurrency.acquire(execConfig.security.max_concurrent);
execConcurrency.acquire(execConfig.security.max_concurrent);

let auditId: number | undefined;
if (execConfig.audit.log_commands) {
Expand Down
2 changes: 1 addition & 1 deletion src/agent/tools/exec/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function createExecStatusExecutor(
return async (_params, context): Promise<ToolResult> => {
const { max_output } = execConfig.limits;

await execConcurrency.acquire(execConfig.security.max_concurrent);
execConcurrency.acquire(execConfig.security.max_concurrent);

let auditId: number | undefined;
if (execConfig.audit.log_commands) {
Expand Down
62 changes: 60 additions & 2 deletions src/agent/tools/plugin-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { readdirSync, readFileSync, existsSync, statSync } from "fs";
import { join } from "path";
import { pathToFileURL } from "url";
import { execFile } from "child_process";
import { createHash } from "node:crypto";
import { getPluginPriorities } from "./plugin-config-store.js";
import { promisify } from "util";

Expand Down Expand Up @@ -52,6 +53,55 @@ const log = createLogger("PluginLoader");

const PLUGIN_DATA_DIR = join(TELETON_ROOT, "plugins", "data");

// SECURITY FIX H-02: Plugin code signing verification
// Plugins can optionally include a .sig file with a SHA-256 hash of the main module.
// When config.capabilities.exec.security.verifyPluginSignatures is true, unsigned plugins are rejected.
const SIGNATURE_FILE = ".sig";

/**
* Verify a plugin's code signature if signature verification is enabled.
* The .sig file should contain a SHA-256 hash of the plugin's index.js content.
*/
function verifyPluginSignature(
pluginDir: string,
modulePath: string,
pluginName: string,
config: Config
): void {
// Only enforce if explicitly configured
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- verifyPluginSignatures may be added by config extension
if ((config.capabilities?.exec?.security as any)?.verifyPluginSignatures !== true) {
return;
}

const sigPath = join(pluginDir, SIGNATURE_FILE);
if (!existsSync(sigPath)) {
throw new Error(
`SECURITY H-02: Plugin "${pluginName}" has no signature file (${SIGNATURE_FILE}). ` +
`Unsigned plugins are rejected when capabilities.exec.security.verifyPluginSignatures is enabled.`
);
}

try {
const expectedHash = readFileSync(sigPath, "utf-8").trim();
const actualHash = createHash("sha256").update(readFileSync(modulePath)).digest("hex");

if (expectedHash !== actualHash) {
throw new Error(
`SECURITY H-02: Plugin "${pluginName}" signature mismatch. ` +
`The plugin code may have been modified after signing.`
);
}

log.info(`[${pluginName}] Code signature verified`);
} catch (err) {
if (err instanceof Error && err.message.startsWith("SECURITY H-02:")) {
throw err;
}
throw new Error(`SECURITY H-02: Failed to verify signature for "${pluginName}": ${err}`);
}
}

interface RawPluginExports {
tools?: SimpleToolDef[] | ((sdk: PluginSDK) => SimpleToolDef[]);
manifest?: unknown;
Expand Down Expand Up @@ -436,7 +486,7 @@ export async function loadEnhancedPlugins(
pluginPaths.map(async ({ entry, path }) => {
const moduleUrl = pathToFileURL(path).href;
const mod = (await import(moduleUrl)) as RawPluginExports;
return { entry, mod };
return { entry, mod, path };
})
);

Expand All @@ -449,14 +499,22 @@ export async function loadEnhancedPlugins(
continue;
}

const { entry, mod } = result.value;
const { entry, mod, path: modulePath } = result.value;

try {
if (!mod.tools || (typeof mod.tools !== "function" && !Array.isArray(mod.tools))) {
log.warn(`Plugin "${entry}": no 'tools' array or function exported, skipping`);
continue;
}

// SECURITY FIX H-02: Verify plugin code signature if configured
// For directory plugins (index.js), check .sig in the plugin directory.
// For single-file plugins (pluginName.js), check .sig alongside the file.
const pluginDir = modulePath.endsWith("index.js")
? join(pluginsDir, entry)
: join(pluginsDir, entry.replace(/\.js$/, ""));
verifyPluginSignature(pluginDir, modulePath, entry, config);

const adapted = adaptPlugin(
mod,
entry,
Expand Down
8 changes: 8 additions & 0 deletions src/agent/tools/plugin-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,16 @@ export class PluginWatcher {

/**
* Start watching the plugins directory for changes.
* SECURITY FIX H-13: Only enable hot-reload when explicitly configured (dev mode).
*/
start(): void {
// SECURITY: Only enable hot-reload in dev mode — prevents unauthorized
// code reload in production environments
if (!this.deps.config.dev?.hot_reload) {
log.info("Plugin watcher disabled (dev.hot_reload not enabled)");
return;
}

this.watcher = chokidar.watch(this.pluginsDir, {
ignoreInitial: true,
awaitWriteFinish: {
Expand Down
33 changes: 19 additions & 14 deletions src/api/__tests__/api-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ describe("Management API", () => {
});
expect(res.status).toBe(200);
const body = await res.json();
expect(body).toHaveProperty("node");
expect(body).toHaveProperty("teleton");
expect(body).toHaveProperty("apiVersion");
});

Expand Down Expand Up @@ -537,7 +537,7 @@ describe("Management API", () => {
expect(body.state).toBe("stopped");
});

// Test 17
// Test 17 — SECURITY FIX M-05: system info no longer exposes total/free memory
it("GET /v1/system/info returns CPU/RAM info", async () => {
const app = createTestApp({ skipAuth: true });
const res = await app.request("/v1/system/info");
Expand All @@ -547,8 +547,14 @@ describe("Management API", () => {
expect(body).toHaveProperty("memory");
expect(body).toHaveProperty("uptime");
expect(body.cpu).toHaveProperty("cores");
expect(body.memory).toHaveProperty("total");
expect(body.memory).toHaveProperty("free");
expect(body.cpu).toHaveProperty("loadAvg");
expect(body.memory).toHaveProperty("heapUsed");
expect(body.memory).toHaveProperty("heapTotal");
// SECURITY: total/free memory and system uptime are no longer exposed
expect(body.memory).not.toHaveProperty("total");
expect(body.memory).not.toHaveProperty("free");
expect(body.memory).not.toHaveProperty("used");
expect(body.uptime).not.toHaveProperty("system");
});
});

Expand Down Expand Up @@ -641,32 +647,31 @@ describe("Management API", () => {
app = createTestApp({ skipAuth: true });
});

// Test 23
// Test 23 — /version returns teleton version and API version
it("GET /v1/system/version returns correct fields", async () => {
const res = await app.request("/v1/system/version");
expect(res.status).toBe(200);
const body = await res.json();
expect(body).toHaveProperty("teleton");
expect(body).toHaveProperty("node");
expect(body).toHaveProperty("os");
expect(body).toHaveProperty("arch");
expect(body).toHaveProperty("apiVersion");
expect(body.apiVersion).toBe("1.0.0");
expect(body.node).toMatch(/^v\d+/);
});

// Test 24
// Test 24 — SECURITY FIX M-05: total/free/system info no longer exposed
it("GET /v1/system/info returns CPU, memory, and uptime", async () => {
const res = await app.request("/v1/system/info");
expect(res.status).toBe(200);
const body = await res.json();
expect(body.cpu.cores).toBeGreaterThan(0);
expect(body.memory.total).toBeGreaterThan(0);
expect(body.memory.free).toBeGreaterThan(0);
expect(body.memory.used).toBeGreaterThan(0);
expect(body.cpu.loadAvg).toBeDefined();
expect(body.memory.heapUsed).toBeGreaterThan(0);
expect(body.memory.heapTotal).toBeGreaterThan(0);
expect(body.uptime.process).toBeGreaterThanOrEqual(0);
expect(body.uptime.system).toBeGreaterThan(0);
// SECURITY: these fields are no longer exposed
expect(body.memory).not.toHaveProperty("total");
expect(body.memory).not.toHaveProperty("free");
expect(body.memory).not.toHaveProperty("used");
expect(body.uptime).not.toHaveProperty("system");
});

// Test 25
Expand Down
22 changes: 18 additions & 4 deletions src/api/monitoring-service.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
/**
* Monitoring Service re-export
* DEPRECATED — SECURITY FIX H-01: Monitoring service removed.
*
* Re-exports from services/monitoring/monitoring-service.ts for backward compatibility.
* Stubs only — returns null/empty to prevent import errors.
* Do not use. Use the health-check routes instead.
*/

export { getMonitoringService } from "../services/monitoring/monitoring-service.js";
export type { AlertRule, AlertChannel } from "../services/monitoring/monitoring-service.js";
export interface AlertRule {
id: string;
name: string;
[key: string]: unknown;
}

export interface AlertChannel {
id: string;
type: string;
[key: string]: unknown;
}

export function getMonitoringService(): never {
throw new Error("Monitoring service has been removed for security (SECURITY FIX H-01)");
}
Loading
Loading