triton6564685/gbrain
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

security: fix wave 2 — 5 vulns + typed health check DSL (v0.9.3) (#95)

Browse Source 
* security: path traversal, query bounds, marker injection fixes

LocalStorage: contained() method validates all paths stay within storage root.
file-resolver: resolveFile validates filePath within brainRoot, marker prefix
rejects ../, absolute paths, bare '..'. file_list: LIMIT 100 on slug-filtered
branch + FILE_LIST_LIMIT constant for both branches.

Co-Authored-By: Gus <garagon@users.noreply.github.com>

* security: symlink hardening in all file walkers

All 4 walkers in files.ts (collectFiles, findRedirects, findAndClean, scan)
plus init.ts counter now use lstatSync + isSymbolicLink skip. Tests import
production collectFiles instead of reimplementing it. node_modules skipped.
CLI file list and verify queries bounded with LIMIT.

Co-Authored-By: Gus <garagon@users.noreply.github.com>

* feat: typed health check DSL + recipe migration

4 DSL types: http, env_exists, command, any_of. Replaces raw execSync
on recipe YAML. All 7 first-party recipes migrated from shell strings
to typed objects. String health_checks still accepted with deprecation
warning + metachar validation for non-embedded recipes. isUnsafeHealthCheck
blocks shell injection for user-created recipes.

Co-Authored-By: Gus <garagon@users.noreply.github.com>

* chore: bump version and changelog (v0.9.3)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: E2E test for file_list LIMIT enforcement against real Postgres

Inserts 150 file rows for one slug, verifies file_list returns at most
100 (both slug-filtered and unfiltered branches). Proves the LIMIT
works at the database level, not just in unit tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Gus <garagon@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-04-13 07:49:13 -10:00
committed by GitHub
parent adb02b7826
commit f82978d38d
22 changed files with 704 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
src/commands/files.ts
View File

@@ -1,4 +1,4 @@
import { readFileSync, readdirSync, statSync, existsSync, writeFileSync, unlinkSync, mkdirSync } from 'fs';
import { readFileSync, readdirSync, statSync, lstatSync, existsSync, writeFileSync, unlinkSync, mkdirSync } from 'fs';
import { join, relative, extname, basename, dirname } from 'path';
import { createHash } from 'crypto';
import type { BrainEngine } from '../core/engine.ts';
@@ -102,7 +102,7 @@ async function listFiles(slug?: string) {
const sql = db.getConnection();
let rows;
if (slug) {
rows = await sql`SELECT * FROM files WHERE page_slug = ${slug} ORDER BY filename`;
rows = await sql`SELECT * FROM files WHERE page_slug = ${slug} ORDER BY filename LIMIT 100`;
} else {
rows = await sql`SELECT * FROM files ORDER BY page_slug, filename LIMIT 100`;
}
@@ -348,7 +348,7 @@ async function syncFiles(dir?: string) {
async function verifyFiles() {
const sql = db.getConnection();
const rows = await sql`SELECT * FROM files ORDER BY storage_path`;
const rows = await sql`SELECT * FROM files ORDER BY storage_path LIMIT 1000`;
if (rows.length === 0) {
console.log('No files to verify.');
@@ -526,7 +526,14 @@ async function restoreFiles(args: string[]) {
for (const entry of readdirSync(d)) {
if (entry.startsWith('.')) continue;
const full = join(d, entry);
if (statSync(full).isDirectory()) findRedirects(full);
let stat;
try {
stat = lstatSync(full);
} catch {
continue; // Broken symlink or permission error
}
if (stat.isSymbolicLink()) continue;
if (stat.isDirectory()) findRedirects(full);
else if (entry.endsWith('.redirect.yaml') || entry.endsWith('.redirect')) redirectFiles.push(full);
}
}
@@ -571,7 +578,14 @@ async function cleanFiles(args: string[]) {
for (const entry of readdirSync(d)) {
if (entry.startsWith('.')) continue;
const full = join(d, entry);
if (statSync(full).isDirectory()) findAndClean(full);
let stat;
try {
stat = lstatSync(full);
} catch {
continue; // Broken symlink or permission error
}
if (stat.isSymbolicLink()) continue;
if (stat.isDirectory()) findAndClean(full);
else if (entry.endsWith('.redirect.yaml') || entry.endsWith('.redirect')) { unlinkSync(full); cleaned++; }
}
}
@@ -590,7 +604,14 @@ async function filesStatus(args: string[]) {
if (entry.startsWith('.') && entry !== '.supabase') continue;
const full = join(d, entry);
if (entry === '.supabase') { mirrored++; continue; }
if (statSync(full).isDirectory()) scan(full);
let stat;
try {
stat = lstatSync(full);
} catch {
continue; // Broken symlink or permission error
}
if (stat.isSymbolicLink()) continue;
if (stat.isDirectory()) scan(full);
else if (entry.endsWith('.redirect.yaml') || entry.endsWith('.redirect')) redirected++;
else if (!entry.endsWith('.md')) local++;
}
@@ -609,15 +630,22 @@ async function filesStatus(args: string[]) {
}
}
function collectFiles(dir: string): string[] {
export function collectFiles(dir: string): string[] {
const files: string[] = [];
function walk(d: string) {
for (const entry of readdirSync(d)) {
if (entry.startsWith('.')) continue;
if (entry === 'node_modules') continue;
const full = join(d, entry);
const stat = statSync(full);
let stat;
try {
stat = lstatSync(full);
} catch {
continue; // Broken symlink or permission error
}
if (stat.isSymbolicLink()) continue;
if (stat.isDirectory()) {
walk(full);

8
src/commands/init.ts
View File

@@ -1,5 +1,5 @@
import { execSync } from 'child_process';
import { readdirSync, statSync } from 'fs';
import { readdirSync, lstatSync } from 'fs';
import { join } from 'path';
import { homedir } from 'os';
import { saveConfig, type GBrainConfig } from '../core/config.ts';
@@ -166,7 +166,11 @@ function countMarkdownFiles(dir: string, maxScan = 1500): number {
if (entry.startsWith('.') || entry === 'node_modules') continue;
const full = join(d, entry);
try {
const stat = statSync(full);
let stat;
try {
stat = lstatSync(full);
} catch { continue; }
if (stat.isSymbolicLink()) continue;
if (stat.isDirectory()) scan(full);
else if (entry.endsWith('.md')) count++;
} catch { /* skip unreadable */ }

205
src/commands/integrations.ts
View File

@@ -41,7 +41,7 @@ interface RecipeFrontmatter {
category: 'infra' | 'sense' | 'reflex';
requires: string[];
secrets: RecipeSecret[];
health_checks: string[];
health_checks: HealthCheck[];
setup_time: string;
cost_estimate?: string;
}
@@ -50,6 +50,7 @@ interface ParsedRecipe {
frontmatter: RecipeFrontmatter;
body: string;
filename: string;
embedded: boolean;
}
interface HeartbeatEntry {
@@ -61,6 +62,169 @@ interface HeartbeatEntry {
error?: string;
}
// --- Health Check DSL Types ---
interface HttpCheck {
type: 'http';
url: string;
method?: string;
headers?: Record<string, string>;
body?: string;
auth?: 'basic' | 'bearer';
auth_user?: string;
auth_pass?: string;
auth_token?: string;
label?: string;
}
interface EnvExistsCheck {
type: 'env_exists';
name: string;
label?: string;
}
interface CommandCheck {
type: 'command';
argv: string[];
label?: string;
}
interface AnyOfCheck {
type: 'any_of';
label?: string;
checks: HealthCheck[];
}
type HealthCheck = string | HttpCheck | EnvExistsCheck | CommandCheck | AnyOfCheck;
interface CheckResult {
integration: string;
check: string;
status: 'ok' | 'fail' | 'timeout' | 'blocked';
output: string;
}
/**
* Returns true if a string health_check contains shell metacharacters.
* Only applied to user-created (non-embedded) recipes.
*/
export function isUnsafeHealthCheck(check: string): boolean {
return /[;&|`$(){}\\<>\n]/.test(check);
}
/** Expand $VAR references with process.env values */
export function expandVars(s: string): string {
return s.replace(/\$([A-Z_][A-Z0-9_]*)/g, (_, name) => process.env[name] || '');
}
export async function executeHealthCheck(
check: HealthCheck,
integrationId: string,
isEmbedded: boolean,
): Promise<CheckResult> {
const label = typeof check === 'string' ? check : (check as any).label || JSON.stringify(check);
const base = { integration: integrationId, check: label };
// String health checks (deprecated path)
if (typeof check === 'string') {
if (!isEmbedded && isUnsafeHealthCheck(check)) {
return { ...base, status: 'blocked', output: 'Blocked: contains unsafe shell characters. Migrate to typed health_check DSL.' };
}
try {
const output = execSync(check, { timeout: 10000, encoding: 'utf-8', env: process.env }).trim();
if (!isEmbedded) {
console.error(` Warning: string health_check is deprecated. Migrate to typed DSL format.`);
}
return { ...base, status: output.includes('FAIL') ? 'fail' : 'ok', output };
} catch (e: unknown) {
const msg = e instanceof Error ? e.message : String(e);
return { ...base, status: msg.includes('TIMEDOUT') ? 'timeout' : 'fail', output: msg };
}
}
// Typed DSL checks
switch (check.type) {
case 'http': {
try {
const url = expandVars(check.url);
if (!url || url.includes('undefined')) {
return { ...base, status: 'fail', output: `Missing env var in URL: ${check.url}` };
}
const headers: Record<string, string> = {};
if (check.headers) {
for (const [k, v] of Object.entries(check.headers)) {
headers[k] = expandVars(v);
}
}
if (check.auth === 'basic' && check.auth_user && check.auth_pass) {
const user = expandVars(check.auth_user);
const pass = expandVars(check.auth_pass);
headers['Authorization'] = 'Basic ' + Buffer.from(`${user}:${pass}`).toString('base64');
} else if (check.auth === 'bearer' && check.auth_token) {
headers['Authorization'] = 'Bearer ' + expandVars(check.auth_token);
}
const fetchOpts: RequestInit = {
method: check.method || 'GET',
headers,
signal: AbortSignal.timeout(10000),
};
if (check.body) {
fetchOpts.body = expandVars(check.body);
if (!headers['Content-Type']) headers['Content-Type'] = 'application/json';
}
const resp = await fetch(url, fetchOpts);
const ok = resp.status >= 200 && resp.status < 400;
return { ...base, status: ok ? 'ok' : 'fail', output: `${check.label || 'HTTP'}: ${ok ? 'OK' : `HTTP ${resp.status}`}` };
} catch (e: unknown) {
const msg = e instanceof Error ? e.message : String(e);
if (msg.includes('TimeoutError') || msg.includes('abort')) {
return { ...base, status: 'timeout', output: `${check.label || 'HTTP'}: timeout` };
}
return { ...base, status: 'fail', output: `${check.label || 'HTTP'}: ${msg}` };
}
}
case 'env_exists': {
const val = process.env[check.name];
return {
...base,
status: val ? 'ok' : 'fail',
output: `${check.label || check.name}: ${val ? 'set' : 'NOT SET'}`,
};
}
case 'command': {
try {
const { spawnSync } = await import('child_process');
const result = spawnSync(check.argv[0], check.argv.slice(1), {
timeout: 10000,
encoding: 'utf-8',
env: process.env,
});
const ok = result.status === 0;
const output = (result.stdout || '').trim() || (ok ? 'OK' : 'FAIL');
return { ...base, status: ok ? 'ok' : 'fail', output: `${check.label || check.argv[0]}: ${output}` };
} catch (e: unknown) {
const msg = e instanceof Error ? e.message : String(e);
return { ...base, status: 'fail', output: `${check.label || check.argv[0]}: ${msg}` };
}
}
case 'any_of': {
for (const sub of check.checks) {
const result = await executeHealthCheck(sub, integrationId, isEmbedded);
if (result.status === 'ok') {
return { ...base, status: 'ok', output: `${check.label || 'any_of'}: ${result.output}` };
}
}
return { ...base, status: 'fail', output: `${check.label || 'any_of'}: all checks failed` };
}
default:
return { ...base, status: 'fail', output: `Unknown check type: ${(check as any).type}` };
}
}
// --- Recipe Parsing ---
/**
@@ -81,12 +245,13 @@ export function parseRecipe(content: string, filename: string): ParsedRecipe | n
category: data.category || 'sense',
requires: data.requires || [],
secrets: data.secrets || [],
health_checks: data.health_checks || [],
health_checks: (data.health_checks || []) as HealthCheck[],
setup_time: data.setup_time || 'unknown',
cost_estimate: data.cost_estimate,
},
body: body.trim(),
filename,
embedded: false,
};
} catch {
return null;
@@ -127,6 +292,7 @@ function loadAllRecipes(): ParsedRecipe[] {
const content = readFileSync(join(dir, file), 'utf-8');
const recipe = parseRecipe(content, file);
if (recipe) {
recipe.embedded = true;
recipes.push(recipe);
} else {
console.error(`Warning: skipping ${file} (invalid or missing 'id' in frontmatter)`);
@@ -440,7 +606,7 @@ function cmdStatus(args: string[]): void {
console.log('');
}
function cmdDoctor(args: string[]): void {
async function cmdDoctor(args: string[]): Promise<void> {
const jsonMode = args.includes('--json');
const recipes = loadAllRecipes();
const configured = recipes.filter(r => getStatus(r) !== 'available');
@@ -454,37 +620,12 @@ function cmdDoctor(args: string[]): void {
return;
}
interface CheckResult {
integration: string;
check: string;
status: 'ok' | 'fail' | 'timeout';
output: string;
}
const results: CheckResult[] = [];
for (const recipe of configured) {
for (const check of recipe.frontmatter.health_checks) {
try {
const output = execSync(check, {
timeout: 10000,
encoding: 'utf-8',
env: process.env,
}).trim();
results.push({
integration: recipe.frontmatter.id,
check,
status: output.includes('FAIL') ? 'fail' : 'ok',
output,
});
} catch (e: unknown) {
const msg = e instanceof Error ? e.message : String(e);
results.push({
integration: recipe.frontmatter.id,
check,
status: msg.includes('TIMEDOUT') ? 'timeout' : 'fail',
output: msg,
});
}
const result = await executeHealthCheck(check, recipe.frontmatter.id, recipe.embedded);
results.push(result);
}
}
@@ -502,7 +643,7 @@ function cmdDoctor(args: string[]): void {
const allOk = checks.every(c => c.status === 'ok');
console.log(` ${recipe.frontmatter.id}: ${allOk ? 'OK' : 'ISSUES'}`);
for (const c of checks) {
const icon = c.status === 'ok' ? ' ✓' : c.status === 'timeout' ? ' ⏱' : ' ✗';
const icon = c.status === 'ok' ? ' \u2713' : c.status === 'timeout' ? ' \u23F1' : ' \u2717';
console.log(`${icon} ${c.output}`);
}
}
@@ -670,7 +811,7 @@ export async function runIntegrations(args: string[]): Promise<void> {
cmdStatus(subArgs);
break;
case 'doctor':
cmdDoctor(subArgs);
await cmdDoctor(subArgs);
break;
case 'stats':
cmdStats(subArgs);

20
src/core/file-resolver.ts
View File

@@ -52,6 +52,14 @@ export async function resolveFile(
brainRoot: string,
storage?: StorageBackend,
): Promise<ResolvedFile> {
// Validate filePath stays within brainRoot (prevents MCP callers from reading arbitrary files)
const { resolve: resolvePath } = await import('path');
const resolvedRoot = resolvePath(brainRoot);
const resolvedFull = resolvePath(brainRoot, filePath);
if (!resolvedFull.startsWith(resolvedRoot + '/') && resolvedFull !== resolvedRoot) {
throw new Error(`Path traversal blocked: ${filePath} resolves outside brain root`);
}
const fullPath = join(brainRoot, filePath);
// 1. Local file exists
@@ -82,7 +90,17 @@ export async function resolveFile(
if (existsSync(markerPath)) {
if (!storage) throw new Error(`Directory mirrored to storage but no storage backend configured: ${filePath}`);
const marker = parseMarker(markerPath);
const storagePath = marker.prefix + filePath.split('/').pop();
// Validate marker.prefix: reject path traversal, absolute paths, bare '..'
if (marker.prefix) {
if (/\.\.[\\/]/.test(marker.prefix) || marker.prefix === '..' || marker.prefix.startsWith('/')) {
throw new Error(`Blocked: .supabase marker prefix contains path traversal: ${marker.prefix}`);
}
}
const filename = filePath.split('/').pop() || '';
if (/\.\.[\\/]/.test(filename) || filename === '..' || filename.startsWith('/')) {
throw new Error(`Blocked: filename contains path traversal: ${filename}`);
}
const storagePath = (marker.prefix || '') + filename;
try {
const data = await storage.download(storagePath);
return { data, source: 'storage' };

9
src/core/operations.ts
View File

@@ -535,6 +535,11 @@ const get_ingest_log: Operation = {
// --- File Operations ---
// Both branches need a LIMIT. Without one, the slug-filtered branch materializes
// every file for that slug — an MCP caller can force unbounded memory consumption
// by targeting a page with many attachments.
const FILE_LIST_LIMIT = 100;
const file_list: Operation = {
name: 'file_list',
description: 'List stored files',
@@ -545,9 +550,9 @@ const file_list: Operation = {
const sql = db.getConnection();
const slug = p.slug as string | undefined;
if (slug) {
return sql`SELECT id, page_slug, filename, storage_path, mime_type, size_bytes, content_hash, created_at FROM files WHERE page_slug = ${slug} ORDER BY filename`;
return sql`SELECT id, page_slug, filename, storage_path, mime_type, size_bytes, content_hash, created_at FROM files WHERE page_slug = ${slug} ORDER BY filename LIMIT ${FILE_LIST_LIMIT}`;
}
return sql`SELECT id, page_slug, filename, storage_path, mime_type, size_bytes, content_hash, created_at FROM files ORDER BY page_slug, filename LIMIT 100`;
return sql`SELECT id, page_slug, filename, storage_path, mime_type, size_bytes, content_hash, created_at FROM files ORDER BY page_slug, filename LIMIT ${FILE_LIST_LIMIT}`;
},
};

27
src/core/storage/local.ts
View File

@@ -1,5 +1,5 @@
import { readFileSync, writeFileSync, unlinkSync, existsSync, mkdirSync, readdirSync } from 'fs';
import { join, dirname } from 'path';
import { readFileSync, writeFileSync, unlinkSync, existsSync, mkdirSync, readdirSync, realpathSync } from 'fs';
import { join, dirname, resolve } from 'path';
import type { StorageBackend } from '../storage.ts';
/**
@@ -7,33 +7,44 @@ import type { StorageBackend } from '../storage.ts';
* Stores files in a local directory, mimicking S3/Supabase behavior.
*/
export class LocalStorage implements StorageBackend {
private readonly canonicalBase: string;
constructor(private basePath: string) {
mkdirSync(basePath, { recursive: true });
this.canonicalBase = realpathSync(basePath);
}
private contained(path: string): string {
const full = resolve(this.canonicalBase, path);
if (!full.startsWith(this.canonicalBase + '/') && full !== this.canonicalBase) {
throw new Error('Path traversal blocked: ' + path + ' resolves outside storage root');
}
return full;
}
async upload(path: string, data: Buffer, _mime?: string): Promise<void> {
const full = join(this.basePath, path);
const full = this.contained(path);
mkdirSync(dirname(full), { recursive: true });
writeFileSync(full, data);
}
async download(path: string): Promise<Buffer> {
const full = join(this.basePath, path);
const full = this.contained(path);
if (!existsSync(full)) throw new Error(`File not found in storage: ${path}`);
return readFileSync(full);
}
async delete(path: string): Promise<void> {
const full = join(this.basePath, path);
const full = this.contained(path);
if (existsSync(full)) unlinkSync(full);
}
async exists(path: string): Promise<boolean> {
return existsSync(join(this.basePath, path));
return existsSync(this.contained(path));
}
async list(prefix: string): Promise<string[]> {
const dir = join(this.basePath, prefix);
const dir = this.contained(prefix);
if (!existsSync(dir)) return [];
const results: string[] = [];
function walk(d: string, rel: string) {
@@ -51,6 +62,6 @@ export class LocalStorage implements StorageBackend {
}
async getUrl(path: string): Promise<string> {
return `file://${join(this.basePath, path)}`;
return `file://${this.contained(path)}`;
}
}