mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 06:23:01 +00:00
fix(security): confine routing dotfiles, skills dir, slugs, and transcription exec
Shared src/core/path-confine.ts consolidates the realpath-containment idiom (moved from sources-ops.ts) and adds isTrustedDotfile + isWriteTargetContained. - .gbrain-source (source-resolver) and .gbrain-mount (brain-resolver) walk-up dotfiles are now lstat trust-gated: a symlink, foreign-owned, or world-writable file is refused on multi-user hosts (#418), fail-closed on stat error. - resolveWorkspaceSkillsDir + every skills-dir tier (env, cwd_walk_up, repo_root, cwd_skills, install_path) route through realpath containment so a symlinked workspace/skills can't escape the declared workspace (#419). - resolveSourceId/resolveBrainId realpath both sides of the registered local_path / mount prefix match so a symlinked cwd can't misattribute source/brain. - validateSlug rejects NUL/control, bidi/RTL overrides, backslashes, and URL-encoded path separators at the shared putPage/updateSlug chokepoint; write-through confirms the file path stays within the source tree. - transcribeLargeFile uses execFileSync arg-arrays + fs.rmSync (no shell), so a path with shell metacharacters is never parsed by a shell (#245). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
bb2e88c42a
commit
213dfa8bc0
@@ -19,9 +19,10 @@
|
||||
* parent's brainId instead of re-running this resolver.
|
||||
*/
|
||||
|
||||
import { readFileSync, existsSync } from 'fs';
|
||||
import { readFileSync, lstatSync, type Stats } from 'fs';
|
||||
import { join, dirname, resolve } from 'path';
|
||||
import { HOST_BRAIN_ID, loadMounts, validateMountId, type MountEntry } from './brain-registry.ts';
|
||||
import { isTrustedDotfile, realpathOrResolve } from './path-confine.ts';
|
||||
|
||||
const DOTFILE = '.gbrain-mount';
|
||||
/** Same regex as brain-registry. Kept in sync. */
|
||||
@@ -36,7 +37,13 @@ function readDotfileWalk(startDir: string): string | null {
|
||||
let dir = resolve(startDir);
|
||||
for (let i = 0; i < 50; i++) {
|
||||
const candidate = join(dir, DOTFILE);
|
||||
if (existsSync(candidate)) {
|
||||
// lstatSync (NOT statSync) + isTrustedDotfile: refuse a symlink, foreign-
|
||||
// owned, or world-writable `.gbrain-mount` planted by another user in a
|
||||
// shared ancestor dir (same multi-user-host hijack as #418, applied to the
|
||||
// brain axis). Any stat error → skip and keep walking (fail-closed).
|
||||
let st: Stats | null = null;
|
||||
try { st = lstatSync(candidate); } catch { st = null; }
|
||||
if (st && isTrustedDotfile(st)) {
|
||||
try {
|
||||
const content = readFileSync(candidate, 'utf8').trim().split('\n')[0].trim();
|
||||
if (content === HOST_BRAIN_ID) return content;
|
||||
@@ -54,11 +61,14 @@ function readDotfileWalk(startDir: string): string | null {
|
||||
|
||||
/** Longest-prefix match: find the mount whose `path` contains `cwd`. */
|
||||
function longestPathPrefixMount(mounts: MountEntry[], cwd: string): MountEntry | null {
|
||||
const cwdResolved = resolve(cwd);
|
||||
// realpath both sides so a symlinked CWD can't forge a prefix match against a
|
||||
// mount path it doesn't really live under (codex #9, brain-axis mirror of the
|
||||
// source-resolver fix). Falls back to lexical resolve() for a stale mount path.
|
||||
const cwdResolved = realpathOrResolve(cwd);
|
||||
let best: { mount: MountEntry; pathLen: number } | null = null;
|
||||
for (const m of mounts) {
|
||||
if (m.enabled === false) continue;
|
||||
const p = resolve(m.path);
|
||||
const p = realpathOrResolve(m.path);
|
||||
if (cwdResolved === p || cwdResolved.startsWith(p + '/')) {
|
||||
if (!best || p.length > best.pathLen) {
|
||||
best = { mount: m, pathLen: p.length };
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
/**
|
||||
* Shared symlink-safe path-confinement + dotfile-trust helpers.
|
||||
*
|
||||
* Consolidates the realpath-containment idiom that previously lived only in
|
||||
* `sources-ops.ts` (`isPathContained`) and `validateUploadPath`
|
||||
* (`operations.ts`), and adds `isTrustedDotfile` — the multi-user-host trust
|
||||
* gate for walk-up routing dotfiles (`.gbrain-source` / `.gbrain-mount`).
|
||||
*
|
||||
* Threat model (POSIX multi-user host): an attacker who can write into a
|
||||
* shared ancestor directory of the victim's CWD (`/tmp`, `/var/tmp`,
|
||||
* `/dev/shm`, shared NFS/SMB, CI runner volumes, container bind-mounts) can
|
||||
* plant a routing dotfile that silently retargets the victim's reads/writes
|
||||
* to the attacker's source/brain. The walk-up resolvers must therefore refuse
|
||||
* a dotfile they can't prove the victim (or root) owns. (#418/#419)
|
||||
*
|
||||
* Fail-closed: any stat/realpath error → not trusted / not contained. The one
|
||||
* documented exception is platforms without numeric uid (Windows), where the
|
||||
* multi-user-POSIX threat model does not apply and `isTrustedDotfile` trusts
|
||||
* by default so existing single-user setups keep working.
|
||||
*/
|
||||
|
||||
import { realpathSync, existsSync, type Stats } from 'fs';
|
||||
import { resolve as resolvePath, relative, isAbsolute, dirname, basename, join } from 'path';
|
||||
|
||||
/**
|
||||
* Symlink-safe path confinement: realpath BOTH sides, then a separator-aware
|
||||
* prefix check. A plain `startsWith()` on un-resolved paths would let a
|
||||
* `parent/skills` symlink → `/etc` (or `$GBRAIN_HOME/clones/<id>` → `/etc`)
|
||||
* bypass the boundary; resolving first defeats that.
|
||||
*
|
||||
* Returns true iff `child` exists AND its realpath is `parent`'s realpath or a
|
||||
* real subtree of it. Returns false if either path is unresolvable (missing /
|
||||
* permission) or the resolved child escapes — fail-closed.
|
||||
*/
|
||||
export function isPathContained(child: string, parent: string): boolean {
|
||||
let resolvedChild: string;
|
||||
let resolvedParent: string;
|
||||
try {
|
||||
resolvedChild = realpathSync(child);
|
||||
resolvedParent = realpathSync(parent);
|
||||
} catch {
|
||||
return false; // missing / unresolvable path → not contained
|
||||
}
|
||||
// Append a separator so /foo doesn't match /foobar.
|
||||
const parentWithSep = resolvedParent.endsWith('/') ? resolvedParent : resolvedParent + '/';
|
||||
return resolvedChild === resolvedParent || resolvedChild.startsWith(parentWithSep);
|
||||
}
|
||||
|
||||
/**
|
||||
* Trust gate for a walk-up routing dotfile, given its `lstatSync` Stats.
|
||||
*
|
||||
* The caller MUST pass an `lstatSync` result, never `statSync` — `lstat` does
|
||||
* not follow symlinks, so a planted symlink redirect is visible here as
|
||||
* `isSymbolicLink()` instead of being followed-then-trusted.
|
||||
*
|
||||
* Rejects three classes of untrusted file:
|
||||
* 1. symlinks — an attacker-planted redirect to a file they control;
|
||||
* 2. foreign-owned — `uid` is neither the caller's nor root's (an attacker
|
||||
* can't `chown` a file to the victim, so foreign ownership means planted;
|
||||
* root-owned is trusted — root is the system admin and can write anywhere
|
||||
* regardless);
|
||||
* 3. world-writable (`mode & 0o002`) — anyone can clobber it later, even when
|
||||
* ownership is currently legitimate.
|
||||
*
|
||||
* On platforms without `process.getuid` (Windows) returns true: the
|
||||
* multi-user-POSIX threat model does not apply and ownership is unknowable.
|
||||
*/
|
||||
export function isTrustedDotfile(stats: Stats): boolean {
|
||||
// No numeric uid (Windows) → can't verify ownership; threat model N/A.
|
||||
if (typeof process.getuid !== 'function') return true;
|
||||
// A symlink is an attacker redirect — never trust. (Requires an lstat Stats.)
|
||||
if (stats.isSymbolicLink()) return false;
|
||||
const myUid = process.getuid();
|
||||
// Foreign-owned (not me, not root) → planted. Root-owned is trusted.
|
||||
if (stats.uid !== myUid && stats.uid !== 0) return false;
|
||||
// World-writable → anyone can clobber it later, even when ownership is legit.
|
||||
if ((stats.mode & 0o002) !== 0) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a path through symlinks, falling back to lexical `resolve()` when the
|
||||
* path doesn't exist (stale registration). Used by the registered-path prefix
|
||||
* matchers so a symlinked CWD can't create a false prefix match against a
|
||||
* registered `local_path` / mount path while still tolerating a registered path
|
||||
* that no longer exists on disk.
|
||||
*/
|
||||
export function realpathOrResolve(p: string): string {
|
||||
try {
|
||||
return realpathSync(p);
|
||||
} catch {
|
||||
return resolvePath(p);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Containment check for a write TARGET that may not exist yet (a new page file).
|
||||
* `isPathContained` requires the child to already exist; this instead realpaths
|
||||
* the deepest EXISTING ancestor of `target` (catching a symlinked intermediate
|
||||
* directory that escapes the tree) and re-attaches the not-yet-created tail
|
||||
* lexically, then confirms the result stays within `root`.
|
||||
*
|
||||
* Defense-in-depth for the write-through FS sink (#1647-slug / codex #6):
|
||||
* `validateSlug` already rejects `..`/backslash/control/%2e in the slug, so this
|
||||
* guards a pre-existing hostile row or a symlinked source-tree subdirectory.
|
||||
*/
|
||||
export function isWriteTargetContained(target: string, root: string): boolean {
|
||||
const resolvedRoot = realpathOrResolve(root);
|
||||
let existing = resolvePath(target);
|
||||
const tail: string[] = [];
|
||||
for (let i = 0; i < 4096 && !existsSync(existing); i++) {
|
||||
tail.unshift(basename(existing));
|
||||
const parent = dirname(existing);
|
||||
if (parent === existing) break; // filesystem root
|
||||
existing = parent;
|
||||
}
|
||||
const base = realpathOrResolve(existing);
|
||||
const finalPath = tail.length ? join(base, ...tail) : base;
|
||||
const rel = relative(resolvedRoot, finalPath);
|
||||
return rel === '' || (!rel.startsWith('..') && !isAbsolute(rel));
|
||||
}
|
||||
+23
-7
@@ -2,6 +2,7 @@ import { existsSync } from 'fs';
|
||||
import { fileURLToPath } from 'url';
|
||||
import { isAbsolute, join, resolve as resolvePath } from 'path';
|
||||
import { RESOLVER_FILENAMES, hasResolverFile } from './resolver-filenames.ts';
|
||||
import { isPathContained } from './path-confine.ts';
|
||||
|
||||
/**
|
||||
* Walk up from `startDir` looking for a `skills/` directory that
|
||||
@@ -66,17 +67,23 @@ function resolveWorkspaceSkillsDir(
|
||||
sourceSubdir: SkillsDirSource,
|
||||
sourceRoot: SkillsDirSource,
|
||||
): SkillsDirDetection | null {
|
||||
// Preferred: workspace/skills with a resolver file inside it (gbrain-native).
|
||||
const subdir = join(workspace, 'skills');
|
||||
// Refuse a `skills/` that escapes the declared workspace via symlink (#419).
|
||||
// isPathContained realpaths both ends, so `workspace/skills` → /etc is
|
||||
// rejected, while a legit in-workspace symlink (`workspace/skills` →
|
||||
// `workspace/_real-skills`) stays contained and is allowed. A non-contained
|
||||
// candidate returns null so lower tiers can try, rather than trusting an escape.
|
||||
const contained = isPathContained(subdir, workspace);
|
||||
// Preferred: workspace/skills with a resolver file inside it (gbrain-native).
|
||||
if (hasResolverFile(subdir)) {
|
||||
return { dir: subdir, source: sourceSubdir };
|
||||
return contained ? { dir: subdir, source: sourceSubdir } : null;
|
||||
}
|
||||
// Fallback: resolver file at workspace root (OpenClaw-native layout).
|
||||
// The skills/ subtree still governs file layout even when routing lives
|
||||
// at workspace root. Return the skills subdir so downstream file lookups
|
||||
// work; the resolver parser knows how to look one level up.
|
||||
if (hasResolverFile(workspace) && existsSync(subdir)) {
|
||||
return { dir: subdir, source: sourceRoot };
|
||||
return contained ? { dir: subdir, source: sourceRoot } : null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
@@ -152,7 +159,10 @@ export function autoDetectSkillsDir(
|
||||
let dir = startDir;
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const candidate = join(dir, 'skills');
|
||||
if (existsSync(candidate)) {
|
||||
// Only accept a `skills/` contained within the ancestor it was found under
|
||||
// (#419). An escaping symlink is skipped, and the walk continues upward
|
||||
// rather than trusting a dir that resolves outside the boundary.
|
||||
if (existsSync(candidate) && isPathContained(candidate, dir)) {
|
||||
return { dir: candidate, source: 'cwd_walk_up' };
|
||||
}
|
||||
const parent = join(dir, '..');
|
||||
@@ -177,7 +187,10 @@ export function autoDetectSkillsDir(
|
||||
// 3. gbrain repo walk from cwd.
|
||||
const repoRoot = findRepoRoot(startDir);
|
||||
if (repoRoot && isGbrainRepoRoot(repoRoot)) {
|
||||
return { dir: join(repoRoot, 'skills'), source: 'repo_root' };
|
||||
const skillsDir = join(repoRoot, 'skills');
|
||||
if (isPathContained(skillsDir, repoRoot)) {
|
||||
return { dir: skillsDir, source: 'repo_root' };
|
||||
}
|
||||
}
|
||||
|
||||
// 4. ./skills fallback (with hasResolverFile gate). Functionally
|
||||
@@ -187,7 +200,7 @@ export function autoDetectSkillsDir(
|
||||
// In practice this tier never fires after 1b — cwd_walk_up matches
|
||||
// the same path first. Kept in the type union for back-compat.
|
||||
const cwdSkills = join(startDir, 'skills');
|
||||
if (hasResolverFile(cwdSkills)) {
|
||||
if (hasResolverFile(cwdSkills) && isPathContained(cwdSkills, startDir)) {
|
||||
return { dir: cwdSkills, source: 'cwd_skills' };
|
||||
}
|
||||
|
||||
@@ -237,7 +250,10 @@ export function autoDetectSkillsDirReadOnly(
|
||||
const moduleDir = fileURLToPath(import.meta.url);
|
||||
const installRoot = findRepoRoot(moduleDir);
|
||||
if (installRoot && isGbrainRepoRoot(installRoot)) {
|
||||
return { dir: join(installRoot, 'skills'), source: 'install_path' };
|
||||
const skillsDir = join(installRoot, 'skills');
|
||||
if (isPathContained(skillsDir, installRoot)) {
|
||||
return { dir: skillsDir, source: 'install_path' };
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// fileURLToPath can throw on malformed import.meta.url (rare; some
|
||||
|
||||
@@ -13,10 +13,11 @@
|
||||
* commands (Steps 4/5), and the operation layer (Step 2+).
|
||||
*/
|
||||
|
||||
import { readFileSync, existsSync } from 'fs';
|
||||
import { readFileSync, lstatSync, type Stats } from 'fs';
|
||||
import { join, dirname, resolve } from 'path';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import { SOURCE_ID_RE, isValidSourceId } from './source-id.ts';
|
||||
import { isTrustedDotfile, realpathOrResolve } from './path-confine.ts';
|
||||
|
||||
const DOTFILE = '.gbrain-source';
|
||||
// Canonical SOURCE_ID_RE imported from `source-id.ts` (single source of truth).
|
||||
@@ -34,7 +35,15 @@ function readDotfileWalk(startDir: string): string | null {
|
||||
// Guard against infinite loops on malformed paths.
|
||||
for (let i = 0; i < 50; i++) {
|
||||
const candidate = join(dir, DOTFILE);
|
||||
if (existsSync(candidate)) {
|
||||
// lstatSync (NOT statSync) so a planted symlink is seen here, not silently
|
||||
// followed-then-trusted. Any stat error (ENOENT / permission) → skip this
|
||||
// candidate and keep walking (fail-closed). On a multi-user host an
|
||||
// attacker who can write a shared ancestor dir could otherwise plant a
|
||||
// forged `.gbrain-source`; `isTrustedDotfile` refuses symlinks,
|
||||
// foreign-owned, and world-writable files (#418).
|
||||
let st: Stats | null = null;
|
||||
try { st = lstatSync(candidate); } catch { st = null; }
|
||||
if (st && isTrustedDotfile(st)) {
|
||||
try {
|
||||
const content = readFileSync(candidate, 'utf8').trim().split('\n')[0].trim();
|
||||
// Silent-fallback tier per codex P1-F: invalid dotfile content
|
||||
@@ -105,10 +114,15 @@ export async function resolveSourceId(
|
||||
const registered = await engine.executeRaw<{ id: string; local_path: string }>(
|
||||
`SELECT id, local_path FROM sources WHERE local_path IS NOT NULL`,
|
||||
);
|
||||
const cwdResolved = resolve(cwd);
|
||||
// realpath BOTH sides (not bare resolve) so a symlinked CWD can't forge a
|
||||
// prefix match against a registered local_path it doesn't really live under
|
||||
// (codex #9). realpathOrResolve falls back to lexical resolve() for a stale
|
||||
// registration whose path no longer exists. Resolving both sides keeps a
|
||||
// legitimately symlinked vault matching — only one-sided symlinks break.
|
||||
const cwdResolved = realpathOrResolve(cwd);
|
||||
let best: { id: string; pathLen: number } | null = null;
|
||||
for (const r of registered) {
|
||||
const p = resolve(r.local_path);
|
||||
const p = realpathOrResolve(r.local_path);
|
||||
if (cwdResolved === p || cwdResolved.startsWith(p + '/')) {
|
||||
if (!best || p.length > best.pathLen) {
|
||||
best = { id: r.id, pathLen: p.length };
|
||||
@@ -303,10 +317,11 @@ export async function resolveSourceWithTier(
|
||||
const registered = await engine.executeRaw<{ id: string; local_path: string }>(
|
||||
`SELECT id, local_path FROM sources WHERE local_path IS NOT NULL`,
|
||||
);
|
||||
const cwdResolved = resolve(cwd);
|
||||
// realpath both sides — see the matching block in resolveSourceId (codex #9).
|
||||
const cwdResolved = realpathOrResolve(cwd);
|
||||
let best: { id: string; path: string; pathLen: number } | null = null;
|
||||
for (const r of registered) {
|
||||
const p = resolve(r.local_path);
|
||||
const p = realpathOrResolve(r.local_path);
|
||||
if (cwdResolved === p || cwdResolved.startsWith(p + '/')) {
|
||||
if (!best || p.length > best.pathLen) {
|
||||
best = { id: r.id, path: p, pathLen: p.length };
|
||||
|
||||
+5
-23
@@ -37,8 +37,8 @@
|
||||
*/
|
||||
|
||||
import { existsSync, mkdirSync, renameSync, rmSync, lstatSync } from 'fs';
|
||||
import { realpathSync } from 'fs';
|
||||
import { join, dirname, basename, resolve as resolvePath } from 'path';
|
||||
import { isPathContained } from './path-confine.ts';
|
||||
import { randomBytes } from 'crypto';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import {
|
||||
@@ -231,28 +231,10 @@ function makeTempCloneDir(id: string): string {
|
||||
return gbrainPath('clones', '.tmp', `${id}-${rand}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Symlink-safe path confinement: realpath both sides, then lstat-walk to
|
||||
* confirm `child` is a real subtree of `parent`. Mirrors validateUploadPath
|
||||
* shape at src/core/operations.ts:61. String startsWith() would let
|
||||
* $GBRAIN_HOME/clones/<id> → /etc bypass the confine.
|
||||
*
|
||||
* Returns true if `child` exists and is contained under `parent`.
|
||||
* Returns false if the resolved path escapes, or either path is unresolvable.
|
||||
*/
|
||||
export function isPathContained(child: string, parent: string): boolean {
|
||||
let resolvedChild: string;
|
||||
let resolvedParent: string;
|
||||
try {
|
||||
resolvedChild = realpathSync(child);
|
||||
resolvedParent = realpathSync(parent);
|
||||
} catch {
|
||||
return false; // missing path → not contained
|
||||
}
|
||||
// Append a separator to parent so /foo doesn't match /foobar.
|
||||
const parentWithSep = resolvedParent.endsWith('/') ? resolvedParent : resolvedParent + '/';
|
||||
return resolvedChild === resolvedParent || resolvedChild.startsWith(parentWithSep);
|
||||
}
|
||||
// `isPathContained` moved to `src/core/path-confine.ts` (shared with the
|
||||
// dotfile-trust + skills-dir confinement helpers). Re-exported here so existing
|
||||
// importers (and recloneIfMissing below) keep working unchanged.
|
||||
export { isPathContained };
|
||||
|
||||
/**
|
||||
* Did gbrain CREATE this clone (so re-clone/delete is safe)? Ownership, NOT
|
||||
|
||||
+24
-14
@@ -6,8 +6,8 @@
|
||||
* For files >25MB: ffmpeg segmentation into <25MB chunks, transcribe each, concatenate.
|
||||
*/
|
||||
|
||||
import { statSync, readFileSync } from 'fs';
|
||||
import { basename, extname } from 'path';
|
||||
import { statSync, readFileSync, rmSync } from 'fs';
|
||||
import { basename, extname, join } from 'path';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Types
|
||||
@@ -169,14 +169,22 @@ async function transcribeLargeFile(
|
||||
);
|
||||
}
|
||||
|
||||
// Segment into ~20MB chunks (with some overlap for better joining)
|
||||
const { execSync } = await import('child_process');
|
||||
const tmpDir = execSync('mktemp -d').toString().trim();
|
||||
// Segment into ~20MB chunks (with some overlap for better joining).
|
||||
//
|
||||
// SECURITY (#245): use execFileSync with argument arrays — never a shell
|
||||
// command string. audioPath is interpolated here, and a path containing
|
||||
// shell metacharacters (`;`, `$()`, backticks, quotes) would be a command
|
||||
// injection if passed through a shell. Argument arrays bypass the shell
|
||||
// entirely, so audioPath is always a single literal argv element.
|
||||
const { execFileSync } = await import('child_process');
|
||||
const tmpDir = execFileSync('mktemp', ['-d']).toString().trim();
|
||||
|
||||
try {
|
||||
// Get audio duration
|
||||
const durationStr = execSync(
|
||||
`ffprobe -v error -show_entries format=duration -of default=noprint_wrappers=1:nokey=1 "${audioPath}"`,
|
||||
const durationStr = execFileSync(
|
||||
'ffprobe',
|
||||
['-v', 'error', '-show_entries', 'format=duration',
|
||||
'-of', 'default=noprint_wrappers=1:nokey=1', audioPath],
|
||||
{ encoding: 'utf-8' }
|
||||
).trim();
|
||||
const totalDuration = parseFloat(durationStr) || 0;
|
||||
@@ -188,8 +196,10 @@ async function transcribeLargeFile(
|
||||
|
||||
// Split audio
|
||||
const ext = extname(audioPath);
|
||||
execSync(
|
||||
`ffmpeg -i "${audioPath}" -f segment -segment_time ${segmentSeconds} -c copy "${tmpDir}/segment_%03d${ext}"`,
|
||||
execFileSync(
|
||||
'ffmpeg',
|
||||
['-i', audioPath, '-f', 'segment', '-segment_time', String(segmentSeconds),
|
||||
'-c', 'copy', join(tmpDir, `segment_%03d${ext}`)],
|
||||
{ stdio: 'pipe' }
|
||||
);
|
||||
|
||||
@@ -200,7 +210,7 @@ async function transcribeLargeFile(
|
||||
let timeOffset = 0;
|
||||
|
||||
for (const seg of segments) {
|
||||
const segPath = `${tmpDir}/${seg}`;
|
||||
const segPath = join(tmpDir, seg);
|
||||
const result = await transcribeFile(segPath, provider, apiKey, config);
|
||||
// Offset timestamps
|
||||
result.segments = result.segments.map(s => ({
|
||||
@@ -221,15 +231,15 @@ async function transcribeLargeFile(
|
||||
provider,
|
||||
};
|
||||
} finally {
|
||||
// Cleanup temp directory
|
||||
try { execSync(`rm -rf "${tmpDir}"`); } catch {}
|
||||
// Cleanup temp directory via fs.rmSync — never a shell remove command (#245).
|
||||
try { rmSync(tmpDir, { recursive: true, force: true }); } catch {}
|
||||
}
|
||||
}
|
||||
|
||||
async function checkFfmpeg(): Promise<boolean> {
|
||||
try {
|
||||
const { execSync } = await import('child_process');
|
||||
execSync('ffmpeg -version', { stdio: 'pipe' });
|
||||
const { execFileSync } = await import('child_process');
|
||||
execFileSync('ffmpeg', ['-version'], { stdio: 'pipe' });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
|
||||
@@ -19,11 +19,36 @@ export function generateToken(prefix: string): string {
|
||||
/**
|
||||
* Validate and normalize a slug. Slugs are lowercased repo-relative paths.
|
||||
* Rejects empty slugs, path traversal (..), and leading /.
|
||||
*
|
||||
* SECURITY (#1647-slug / codex #6): also rejects a small set of dangerous
|
||||
* characters that survive the `..` check but can still produce a hostile
|
||||
* filename at the write-through FS sink or hide the real target — NUL/control
|
||||
* bytes, Unicode bidirectional/RTL overrides, backslashes, and URL-encoded
|
||||
* path separators/traversal. None appear in legitimate slugs (lowercase
|
||||
* alphanumerics, hyphens, dots, underscores, slashes, unicode letters, and CJK
|
||||
* all still pass), so this is pure hardening, not a behavior change. This is the
|
||||
* shared chokepoint for both `putPage` and `updateSlug` on both engines.
|
||||
*/
|
||||
export function validateSlug(slug: string): string {
|
||||
if (!slug || /(^|\/)\.\.($|\/)/.test(slug) || /^\//.test(slug)) {
|
||||
throw new Error(`Invalid slug: "${slug}". Slugs cannot be empty, start with /, or contain path traversal.`);
|
||||
}
|
||||
// Control / NUL bytes (C0 + DEL + C1).
|
||||
if (/[\x00-\x1f\x7f-\x9f]/.test(slug)) {
|
||||
throw new Error(`Invalid slug: "${slug}". Slugs cannot contain control characters.`);
|
||||
}
|
||||
// Unicode bidirectional / RTL overrides (visual-spoofing of the real path).
|
||||
if (/[\u202a-\u202e\u2066-\u2069]/.test(slug)) {
|
||||
throw new Error(`Invalid slug: "${slug}". Slugs cannot contain bidirectional/RTL override characters.`);
|
||||
}
|
||||
// Backslash (Windows-style separator / escape).
|
||||
if (slug.includes('\\')) {
|
||||
throw new Error(`Invalid slug: "${slug}". Backslashes are not allowed in slugs.`);
|
||||
}
|
||||
// URL-encoded path separators / traversal (%2e=., %2f=/, %5c=\).
|
||||
if (/%2e|%2f|%5c/i.test(slug)) {
|
||||
throw new Error(`Invalid slug: "${slug}". URL-encoded path separators are not allowed in slugs.`);
|
||||
}
|
||||
return slug.toLowerCase();
|
||||
}
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ import { dirname, join } from 'path';
|
||||
import { randomBytes } from 'crypto';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts';
|
||||
import { isWriteTargetContained } from './path-confine.ts';
|
||||
|
||||
/** Minimal logger surface — structurally compatible with operations.ts `Logger`. */
|
||||
export interface WriteThroughLogger {
|
||||
@@ -45,8 +46,10 @@ export interface WriteThroughResult {
|
||||
* — #2018: writing here would pollute that sibling's repo, so we skip.
|
||||
* - page_not_found_after_write: the DB row isn't readable back (the caller's
|
||||
* DB write failed or targeted a different source).
|
||||
* - path_escapes_source_root: the computed file path resolves outside the
|
||||
* source's working tree (hostile slug row / symlinked subtree) — refused.
|
||||
*/
|
||||
skipped?: 'no_repo_configured' | 'repo_not_found' | 'source_repo_belongs_to_other_source' | 'page_not_found_after_write';
|
||||
skipped?: 'no_repo_configured' | 'repo_not_found' | 'source_repo_belongs_to_other_source' | 'page_not_found_after_write' | 'path_escapes_source_root';
|
||||
/** Set when the render/write/rename itself threw (EACCES, ENOTDIR, disk full). */
|
||||
error?: string;
|
||||
}
|
||||
@@ -83,6 +86,7 @@ export async function writePageThrough(
|
||||
// `local_path`, nesting this page there would pollute that sibling's
|
||||
// git repo (the reported bug). Skip instead.
|
||||
let filePath: string;
|
||||
let writeRoot: string;
|
||||
const srcRows = await engine.executeRaw<{ local_path: string | null }>(
|
||||
`SELECT local_path FROM sources WHERE id = $1`,
|
||||
[sourceId],
|
||||
@@ -93,6 +97,7 @@ export async function writePageThrough(
|
||||
return { written: false, skipped: 'repo_not_found' };
|
||||
}
|
||||
filePath = join(sourceLocalPath, `${slug}.md`);
|
||||
writeRoot = sourceLocalPath;
|
||||
} else {
|
||||
const repoPath = await engine.getConfig('sync.repo_path');
|
||||
if (!repoPath) {
|
||||
@@ -111,6 +116,16 @@ export async function writePageThrough(
|
||||
return { written: false, skipped: 'source_repo_belongs_to_other_source' };
|
||||
}
|
||||
filePath = resolvePageFilePath(repoPath, slug, sourceId);
|
||||
writeRoot = repoPath;
|
||||
}
|
||||
|
||||
// Defense-in-depth (#1647-slug / codex #6): confirm the computed file path
|
||||
// stays within the source's working tree before any mkdir/write. validateSlug
|
||||
// already rejects `..`/backslash/control/%2e in the slug at write time, so
|
||||
// this guards a pre-existing hostile row or a symlinked intermediate dir
|
||||
// under the source tree from escaping to an arbitrary filesystem location.
|
||||
if (!isWriteTargetContained(filePath, writeRoot)) {
|
||||
return { written: false, skipped: 'path_escapes_source_root' };
|
||||
}
|
||||
|
||||
const writtenPage = await engine.getPage(slug, { sourceId });
|
||||
|
||||
@@ -0,0 +1,319 @@
|
||||
/**
|
||||
* Security tests for the shared path-confinement + dotfile-trust helpers
|
||||
* (src/core/path-confine.ts) and their integration into the source / brain
|
||||
* resolvers and the skills-dir auto-detector.
|
||||
*
|
||||
* Threat model: a multi-user POSIX host where an attacker can write into a
|
||||
* shared ancestor directory of the victim's CWD. The hardening must refuse a
|
||||
* routing dotfile (`.gbrain-source` / `.gbrain-mount`) the victim doesn't own,
|
||||
* and refuse a `skills/` directory that escapes its declared workspace via
|
||||
* symlink. (#418 / #419, codex #9)
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import {
|
||||
mkdtempSync, mkdirSync, writeFileSync, rmSync, symlinkSync, chmodSync,
|
||||
lstatSync, type Stats,
|
||||
} from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import {
|
||||
isTrustedDotfile, isPathContained, realpathOrResolve, isWriteTargetContained,
|
||||
} from '../src/core/path-confine.ts';
|
||||
import { validateSlug } from '../src/core/utils.ts';
|
||||
import { resolveSourceId } from '../src/core/source-resolver.ts';
|
||||
import { resolveBrainId } from '../src/core/brain-resolver.ts';
|
||||
import { HOST_BRAIN_ID } from '../src/core/brain-registry.ts';
|
||||
import { autoDetectSkillsDir } from '../src/core/repo-root.ts';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
|
||||
// ── helpers ────────────────────────────────────────────────
|
||||
|
||||
const created: string[] = [];
|
||||
function scratch(prefix = 'path-confine-'): string {
|
||||
const dir = mkdtempSync(join(tmpdir(), prefix));
|
||||
created.push(dir);
|
||||
return dir;
|
||||
}
|
||||
afterEach(() => {
|
||||
while (created.length) {
|
||||
const p = created.pop()!;
|
||||
try { rmSync(p, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
}
|
||||
});
|
||||
|
||||
function fakeStats(opts: { uid: number; mode: number; symlink?: boolean }): Stats {
|
||||
return {
|
||||
uid: opts.uid,
|
||||
mode: opts.mode,
|
||||
isSymbolicLink: () => opts.symlink === true,
|
||||
} as unknown as Stats;
|
||||
}
|
||||
|
||||
/** Stub engine for resolveSourceId: registers `ids`, no local_paths, no default. */
|
||||
function stubEngine(ids: string[]): BrainEngine {
|
||||
return {
|
||||
kind: 'pglite',
|
||||
executeRaw: async <T>(sql: string, params?: unknown[]): Promise<T[]> => {
|
||||
if (sql.includes('SELECT id FROM sources WHERE id = $1')) {
|
||||
const t = params?.[0];
|
||||
return (ids.includes(t as string) ? [{ id: t } as unknown as T] : []);
|
||||
}
|
||||
if (sql.includes('SELECT id, local_path FROM sources')) return [] as unknown as T[];
|
||||
return [] as unknown as T[];
|
||||
},
|
||||
getConfig: async () => null,
|
||||
} as unknown as BrainEngine;
|
||||
}
|
||||
|
||||
// ── isTrustedDotfile (synthetic Stats — precise branch coverage) ──
|
||||
|
||||
describe('isTrustedDotfile', () => {
|
||||
const realGetuid = process.getuid;
|
||||
beforeEach(() => { (process as { getuid?: () => number }).getuid = () => 1000; });
|
||||
afterEach(() => { (process as { getuid?: () => number }).getuid = realGetuid; });
|
||||
|
||||
test('own, not world-writable, not symlink → trusted', () => {
|
||||
expect(isTrustedDotfile(fakeStats({ uid: 1000, mode: 0o644 }))).toBe(true);
|
||||
});
|
||||
test('foreign-owned → NOT trusted', () => {
|
||||
expect(isTrustedDotfile(fakeStats({ uid: 2000, mode: 0o644 }))).toBe(false);
|
||||
});
|
||||
test('root-owned → trusted (root-as-root allowance)', () => {
|
||||
expect(isTrustedDotfile(fakeStats({ uid: 0, mode: 0o644 }))).toBe(true);
|
||||
});
|
||||
test('world-writable, even when owned → NOT trusted', () => {
|
||||
expect(isTrustedDotfile(fakeStats({ uid: 1000, mode: 0o666 }))).toBe(false);
|
||||
});
|
||||
test('symlink → NOT trusted regardless of ownership', () => {
|
||||
expect(isTrustedDotfile(fakeStats({ uid: 1000, mode: 0o644, symlink: true }))).toBe(false);
|
||||
});
|
||||
test('no getuid (Windows) → trusted by default', () => {
|
||||
(process as { getuid?: () => number }).getuid = undefined;
|
||||
expect(isTrustedDotfile(fakeStats({ uid: 2000, mode: 0o666 }))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isTrustedDotfile — real fs (lstat)', () => {
|
||||
test('a real owned file is trusted; a symlink and a world-writable file are not', () => {
|
||||
if (typeof process.getuid !== 'function') return; // POSIX-only
|
||||
const dir = scratch();
|
||||
const ownFile = join(dir, 'own');
|
||||
writeFileSync(ownFile, 'x');
|
||||
expect(isTrustedDotfile(lstatSync(ownFile))).toBe(true);
|
||||
|
||||
const link = join(dir, 'link');
|
||||
symlinkSync(ownFile, link);
|
||||
expect(isTrustedDotfile(lstatSync(link))).toBe(false);
|
||||
|
||||
const ww = join(dir, 'ww');
|
||||
writeFileSync(ww, 'x');
|
||||
chmodSync(ww, 0o666);
|
||||
expect(isTrustedDotfile(lstatSync(ww))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ── isPathContained + realpathOrResolve ────────────────────
|
||||
|
||||
describe('isPathContained', () => {
|
||||
test('real subdir is contained', () => {
|
||||
const dir = scratch();
|
||||
const sub = join(dir, 'sub');
|
||||
mkdirSync(sub);
|
||||
expect(isPathContained(sub, dir)).toBe(true);
|
||||
});
|
||||
test('symlink escaping the parent is NOT contained', () => {
|
||||
const dir = scratch();
|
||||
const outside = scratch('pc-outside-');
|
||||
const link = join(dir, 'escape');
|
||||
symlinkSync(outside, link);
|
||||
expect(isPathContained(link, dir)).toBe(false);
|
||||
});
|
||||
test('symlink resolving INSIDE the parent IS contained', () => {
|
||||
const dir = scratch();
|
||||
const real = join(dir, 'real');
|
||||
mkdirSync(real);
|
||||
const link = join(dir, 'inlink');
|
||||
symlinkSync(real, link);
|
||||
expect(isPathContained(link, dir)).toBe(true);
|
||||
});
|
||||
test('missing path → not contained (fail-closed)', () => {
|
||||
const dir = scratch();
|
||||
expect(isPathContained(join(dir, 'nope'), dir)).toBe(false);
|
||||
});
|
||||
test('sibling prefix does not match (/foo vs /foobar)', () => {
|
||||
const base = scratch();
|
||||
const foo = join(base, 'foo'); const foobar = join(base, 'foobar');
|
||||
mkdirSync(foo); mkdirSync(foobar);
|
||||
expect(isPathContained(foobar, foo)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('realpathOrResolve', () => {
|
||||
test('resolves a symlink to its real target', () => {
|
||||
const dir = scratch();
|
||||
const real = join(dir, 'real'); mkdirSync(real);
|
||||
const link = join(dir, 'link'); symlinkSync(real, link);
|
||||
expect(realpathOrResolve(link)).toBe(realpathOrResolve(real));
|
||||
});
|
||||
test('nonexistent path → lexical resolve (does not throw)', () => {
|
||||
const p = join(scratch(), 'does', 'not', 'exist');
|
||||
expect(realpathOrResolve(p)).toBe(p);
|
||||
});
|
||||
});
|
||||
|
||||
// ── validateSlug dangerous-char hardening (#1647-slug / codex #6) ──
|
||||
|
||||
describe('validateSlug — dangerous-char guard', () => {
|
||||
test('allows legitimate slugs (lowercase, unicode, dot, underscore, CJK, nested)', () => {
|
||||
for (const ok of ['notes/2026', 'münchen', 'my_notes', 'a-b/c-d', '会议/纪要', 'readme.v2', 'Mixed-Case']) {
|
||||
expect(() => validateSlug(ok)).not.toThrow();
|
||||
}
|
||||
});
|
||||
test('rejects path traversal and leading slash (existing behavior preserved)', () => {
|
||||
for (const bad of ['../etc/passwd', 'a/../../b', '/abs/path', '']) {
|
||||
expect(() => validateSlug(bad)).toThrow();
|
||||
}
|
||||
});
|
||||
test('rejects NUL / control bytes', () => {
|
||||
expect(() => validateSlug("a" + String.fromCharCode(0x00) + "b")).toThrow(/control/);
|
||||
expect(() => validateSlug("a" + String.fromCharCode(0x1f) + "b")).toThrow(/control/);
|
||||
});
|
||||
test('rejects Unicode bidirectional / RTL overrides', () => {
|
||||
expect(() => validateSlug("a" + String.fromCharCode(0x202e) + "b")).toThrow(/bidirectional|RTL/);
|
||||
expect(() => validateSlug("a" + String.fromCharCode(0x2066) + "b")).toThrow(/bidirectional|RTL/);
|
||||
});
|
||||
test('rejects backslashes', () => {
|
||||
expect(() => validateSlug('a\\b')).toThrow(/[Bb]ackslash/);
|
||||
});
|
||||
test('rejects URL-encoded path separators / traversal', () => {
|
||||
for (const bad of ['a%2e%2e/b', 'a%2fb', 'a%5cb', 'A%2E%2Eb']) {
|
||||
expect(() => validateSlug(bad)).toThrow(/URL-encoded/);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── isWriteTargetContained (write-through FS sink) ──────────
|
||||
|
||||
describe('isWriteTargetContained', () => {
|
||||
test('a new file directly under root is contained', () => {
|
||||
const root = scratch();
|
||||
expect(isWriteTargetContained(join(root, 'page.md'), root)).toBe(true);
|
||||
});
|
||||
test('a new file in a new nested dir under root is contained', () => {
|
||||
const root = scratch();
|
||||
expect(isWriteTargetContained(join(root, 'sub', 'deep', 'page.md'), root)).toBe(true);
|
||||
});
|
||||
test('a ../ escape is refused', () => {
|
||||
const root = scratch();
|
||||
expect(isWriteTargetContained(join(root, '..', 'escape.md'), root)).toBe(false);
|
||||
});
|
||||
test('a symlinked intermediate dir escaping the root is refused', () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const root = scratch();
|
||||
const outside = scratch('wt-outside-');
|
||||
symlinkSync(outside, join(root, 'link')); // root/link → outside
|
||||
// target lands under the escaping symlink → real path is outside root
|
||||
expect(isWriteTargetContained(join(root, 'link', 'page.md'), root)).toBe(false);
|
||||
});
|
||||
test('a symlinked intermediate dir staying inside the root is allowed', () => {
|
||||
const root = scratch();
|
||||
mkdirSync(join(root, 'real'));
|
||||
symlinkSync(join(root, 'real'), join(root, 'link'));
|
||||
expect(isWriteTargetContained(join(root, 'link', 'page.md'), root)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ── source-resolver integration (#418) ─────────────────────
|
||||
|
||||
describe('resolveSourceId — .gbrain-source dotfile trust', () => {
|
||||
test('trusted dotfile resolves to its id (control)', async () => {
|
||||
const dir = scratch();
|
||||
writeFileSync(join(dir, '.gbrain-source'), 'evil\n');
|
||||
expect(await resolveSourceId(stubEngine(['evil', 'default']), null, dir)).toBe('evil');
|
||||
});
|
||||
test('symlinked dotfile is REFUSED → falls through to default', async () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const dir = scratch();
|
||||
const target = join(dir, 'target'); writeFileSync(target, 'evil\n');
|
||||
symlinkSync(target, join(dir, '.gbrain-source'));
|
||||
expect(await resolveSourceId(stubEngine(['evil', 'default']), null, dir)).toBe('default');
|
||||
});
|
||||
test('world-writable dotfile is REFUSED → falls through to default', async () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const dir = scratch();
|
||||
const df = join(dir, '.gbrain-source'); writeFileSync(df, 'evil\n'); chmodSync(df, 0o666);
|
||||
expect(await resolveSourceId(stubEngine(['evil', 'default']), null, dir)).toBe('default');
|
||||
});
|
||||
});
|
||||
|
||||
// ── brain-resolver integration (#418, brain axis) ──────────
|
||||
|
||||
describe('resolveBrainId — .gbrain-mount dotfile trust', () => {
|
||||
const noMounts = () => [];
|
||||
test('trusted dotfile resolves to its id (control)', () => {
|
||||
const dir = scratch();
|
||||
writeFileSync(join(dir, '.gbrain-mount'), 'evil-brain\n');
|
||||
expect(resolveBrainId(null, dir, noMounts)).toBe('evil-brain');
|
||||
});
|
||||
test('symlinked .gbrain-mount is REFUSED → falls through to host', () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const dir = scratch();
|
||||
const target = join(dir, 'm'); writeFileSync(target, 'evil-brain\n');
|
||||
symlinkSync(target, join(dir, '.gbrain-mount'));
|
||||
expect(resolveBrainId(null, dir, noMounts)).toBe(HOST_BRAIN_ID);
|
||||
});
|
||||
test('world-writable .gbrain-mount is REFUSED → falls through to host', () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const dir = scratch();
|
||||
const df = join(dir, '.gbrain-mount'); writeFileSync(df, 'evil-brain\n'); chmodSync(df, 0o666);
|
||||
expect(resolveBrainId(null, dir, noMounts)).toBe(HOST_BRAIN_ID);
|
||||
});
|
||||
});
|
||||
|
||||
// ── repo-root skills-dir confinement (#419) ────────────────
|
||||
|
||||
describe('autoDetectSkillsDir — skills/ symlink confinement', () => {
|
||||
function seedSkills(dir: string): void {
|
||||
mkdirSync(join(dir, 'skills'), { recursive: true });
|
||||
writeFileSync(join(dir, 'skills', 'RESOLVER.md'), '# RESOLVER\n');
|
||||
}
|
||||
|
||||
test('OPENCLAW_WORKSPACE: escaping skills symlink is refused', () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const ws = scratch('ws-'); const outside = scratch('outside-');
|
||||
seedSkills(outside); // real skills with RESOLVER.md, OUTSIDE the workspace
|
||||
symlinkSync(join(outside, 'skills'), join(ws, 'skills')); // ws/skills → outside/skills
|
||||
const found = autoDetectSkillsDir(scratch('cwd-'), { OPENCLAW_WORKSPACE: ws });
|
||||
expect(found.source).not.toBe('openclaw_workspace_env');
|
||||
expect(found.dir).not.toBe(join(ws, 'skills'));
|
||||
});
|
||||
|
||||
test('OPENCLAW_WORKSPACE: in-workspace skills symlink is allowed', () => {
|
||||
const ws = scratch('ws-');
|
||||
mkdirSync(join(ws, '_real'), { recursive: true });
|
||||
writeFileSync(join(ws, '_real', 'RESOLVER.md'), '# RESOLVER\n');
|
||||
symlinkSync(join(ws, '_real'), join(ws, 'skills')); // contained symlink
|
||||
const found = autoDetectSkillsDir(scratch('cwd-'), { OPENCLAW_WORKSPACE: ws });
|
||||
expect(found.source).toBe('openclaw_workspace_env');
|
||||
expect(found.dir).toBe(join(ws, 'skills'));
|
||||
});
|
||||
|
||||
test('cwd_walk_up: escaping skills symlink is refused', () => {
|
||||
if (typeof process.getuid !== 'function') return;
|
||||
const ws = scratch('ws-'); const outside = scratch('outside-');
|
||||
mkdirSync(join(outside, 'skills'), { recursive: true });
|
||||
symlinkSync(join(outside, 'skills'), join(ws, 'skills'));
|
||||
const found = autoDetectSkillsDir(ws, {});
|
||||
expect(found.dir).toBeNull();
|
||||
});
|
||||
|
||||
test('cwd_walk_up: in-workspace skills symlink is allowed', () => {
|
||||
const ws = scratch('ws-');
|
||||
mkdirSync(join(ws, '_real'), { recursive: true });
|
||||
symlinkSync(join(ws, '_real'), join(ws, 'skills'));
|
||||
const found = autoDetectSkillsDir(ws, {});
|
||||
expect(found.source).toBe('cwd_walk_up');
|
||||
expect(found.dir).toBe(join(ws, 'skills'));
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* Security test for #245 — the large-file transcription path must not pass
|
||||
* caller-controlled audio paths through a shell. transcribeLargeFile now uses
|
||||
* execFileSync with argument arrays (+ fs.rmSync for cleanup), so a path
|
||||
* containing shell metacharacters is a literal argv element, never parsed.
|
||||
*
|
||||
* transcription.ts is a dead-internally-but-PUBLISHED export (gbrain/transcription),
|
||||
* so an external programmatic consumer can still call transcribe() with an
|
||||
* attacker-influenced path — hence the harden over delete.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
mkdtempSync, writeFileSync, rmSync, existsSync, readFileSync,
|
||||
} from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { execFileSync } from 'child_process';
|
||||
import { transcribe } from '../src/core/transcription.ts';
|
||||
|
||||
function ffmpegAvailable(): boolean {
|
||||
try { execFileSync('ffmpeg', ['-version'], { stdio: 'pipe' });
|
||||
execFileSync('ffprobe', ['-version'], { stdio: 'pipe' });
|
||||
return true; } catch { return false; }
|
||||
}
|
||||
|
||||
describe('transcription — command injection (#245)', () => {
|
||||
// Source-level guard (deterministic, no ffmpeg dependency): the shell-string
|
||||
// exec forms must be gone. Mirrors the repo's check-*.sh guard philosophy.
|
||||
test('source uses execFileSync arg-arrays, never shell execSync', () => {
|
||||
const src = readFileSync(new URL('../src/core/transcription.ts', import.meta.url), 'utf8');
|
||||
// `\bexecSync(` matches the dangerous shell-string call but NOT execFileSync.
|
||||
expect(src).not.toMatch(/\bexecSync\s*\(/); // no shell command strings
|
||||
expect(src).toMatch(/execFileSync\(/); // arg-array exec
|
||||
expect(src).toMatch(/rmSync\(/); // fs cleanup, not shell remove
|
||||
});
|
||||
|
||||
// Behavioral: a >25MB file whose NAME contains a $(...) payload. Under the old
|
||||
// shell-string code the double-quoted interpolation command-substitutes the
|
||||
// payload (creating the sentinel) before ffprobe runs; under execFileSync the
|
||||
// payload is a literal filename, so no sentinel is ever created.
|
||||
test('a $(...) payload in the audio path does not spawn a shell', async () => {
|
||||
if (!ffmpegAvailable()) return; // path requires ffmpeg/ffprobe; skip otherwise
|
||||
// Sentinel must be a slash-free name so it's a legal single filename segment;
|
||||
// a shell `touch <name>` would create it in process.cwd(), so we check there.
|
||||
const sentinelName = `gbrain-inj-sentinel-${process.pid}-${Date.now()}`;
|
||||
const sentinelPath = join(process.cwd(), sentinelName);
|
||||
if (existsSync(sentinelPath)) rmSync(sentinelPath, { force: true });
|
||||
const dir = mkdtempSync(join(tmpdir(), 'transcribe-inj-'));
|
||||
// Payload touches the sentinel IF a shell ever parses the path.
|
||||
const evil = join(dir, `clip$(touch ${sentinelName}).mp3`);
|
||||
writeFileSync(evil, Buffer.alloc(26 * 1024 * 1024)); // >25MB → large-file path
|
||||
try {
|
||||
await transcribe(evil, { apiKey: 'dummy', provider: 'groq' });
|
||||
} catch {
|
||||
// Expected: ffprobe/ffmpeg reject the garbage file, or transcribeFile has
|
||||
// no real key. We only care that no shell ran.
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
const pwned = existsSync(sentinelPath);
|
||||
if (pwned) rmSync(sentinelPath, { force: true });
|
||||
expect(pwned).toBe(false);
|
||||
}, 30_000);
|
||||
});
|
||||
Reference in New Issue
Block a user