diff --git a/README.md b/README.md index a560557..9bbb2da 100644 --- a/README.md +++ b/README.md @@ -80,19 +80,18 @@ Config is loaded from `crm.toml`. Resolution order (first match wins): 1. `--config ` flag (explicit) 2. `CRM_CONFIG` env var -3. Walk up from CWD: `./crm.toml` → `../crm.toml` → `../../crm.toml` → ... → `/crm.toml` -4. `~/.crm/config.toml` (global fallback) +3. Walk up from CWD, but never past the current git repository's root: `./crm.toml` → `../crm.toml` → ... → `/crm.toml` +4. If CWD isn't inside a git repository at all, only `./crm.toml` is checked (no upward walk) -This means you can drop a `crm.toml` in your project root and it applies to everyone working in that directory — just like `.gitignore` or `biome.jsonc`. +There is no global `~/.crm/config.toml` fallback — config is always scoped to the current project. This means you can drop a `crm.toml` in your project root and it applies to everyone working in that directory — just like `.gitignore` or `biome.jsonc`. ```bash # Project-scoped config echo '[pipeline] stages = ["discovery", "demo", "trial", "closed-won", "closed-lost"]' > ./crm.toml -# Global config (applies everywhere unless overridden) -mkdir -p ~/.crm -cat > ~/.crm/config.toml << 'EOF' +# Full example, placed at the project root +cat > ./crm.toml << 'EOF' [database] path = "~/.crm/crm.db" @@ -122,7 +121,7 @@ search_limit = 20 # max results from search/find EOF ``` -Settings in a closer `crm.toml` override the global config. The `--config` flag overrides everything. +Settings in a closer `crm.toml` override one further up the repo. The `--config` flag overrides everything. --- @@ -782,7 +781,7 @@ crm contact list --format json | jq '.[] | select(.tags | contains(["hot-lead"]) ### Hooks -Shell commands triggered on mutations. Configured in `~/.crm/config.toml`: +Shell commands triggered on mutations. Configured in the project's own `crm.toml` (see [Configuration](#configuration) — there is no global config file): ```toml [hooks] @@ -797,6 +796,22 @@ Available hooks: - `pre-*` / `post-*` for: `contact-add`, `contact-edit`, `contact-rm`, `company-add`, `company-edit`, `company-rm`, `deal-add`, `deal-edit`, `deal-rm`, `deal-stage-change`, `activity-add` +#### Trust on first use + +Hooks are shell commands, so a `crm.toml` picked up via the implicit discovery walk (i.e. not passed via `--config` or `CRM_CONFIG`) could belong to an ancestor directory you don't fully control. Its `[hooks]` therefore require explicit trust before they run — the same model direnv/mise use for `.envrc`/`.mise.toml`: + +- On first use, if you're at an interactive terminal, crm prompts: run this hook once and remember the file (path + content hash), or skip it. +- Non-interactively (CI, scripts, no TTY) an untrusted hook is always skipped — the command still completes normally, and a warning is printed to stderr telling you which config to trust. +- If a trusted `crm.toml`'s content later changes, trust is invalidated (the hash no longer matches) and it must be re-trusted. +- A config supplied explicitly via `--config ` or `CRM_CONFIG` is a deliberate action and its hooks always run — no trust step. + +```bash +crm config trust ./crm.toml # trust a config's hooks; omit the path to trust whatever config would be auto-resolved +crm config untrust ./crm.toml # revoke trust +``` + +Trust decisions are stored locally in `~/.crm/trusted_configs.json` (path → content hash only — never config content, and never read as a config source itself). + --- ## Custom Fields diff --git a/skills/SKILL.md b/skills/SKILL.md index 6c98cfa..84b677c 100644 --- a/skills/SKILL.md +++ b/skills/SKILL.md @@ -30,7 +30,7 @@ crm --version ## Configuration -Optional. Create `crm.toml` in your project root or `~/.crm/config.toml`: +Optional. Create `crm.toml` in your project root: ```toml [database] @@ -52,7 +52,7 @@ display = "international" default_path = "~/crm" ``` -Config is auto-discovered by walking up from the current directory. Override with `--config ` or `CRM_CONFIG` env var. +Config is auto-discovered by walking up from the current directory, but never past the current git repository's root (if CWD isn't inside a git repository, only the current directory is checked). There is no global `~/.crm/config.toml` fallback. Override with `--config ` or `CRM_CONFIG` env var. ## Global Flags @@ -450,7 +450,7 @@ Prefix with `json:` for typed values (numbers, booleans, arrays). ## Hooks -Configure shell hooks in `crm.toml` that fire on mutations: +Configure shell hooks in the project's own `crm.toml` that fire on mutations (there is no global config file): ```toml [hooks] @@ -463,6 +463,8 @@ Entity data is passed as JSON on stdin. Pre-hooks abort on non-zero exit. Available hooks: `{pre,post}-{contact,company,deal}-{add,edit,rm}`, `{pre,post}-deal-stage-change`, `{pre,post}-activity-add`. +Hooks in an implicitly-discovered `crm.toml` (not passed via `--config`/`CRM_CONFIG`) require trust-on-first-use: interactively you'll be prompted once (approval is remembered by path + content hash); non-interactively an untrusted hook is skipped with a warning rather than run silently. Run `crm config trust ./crm.toml` to trust one ahead of time. Configs passed explicitly via `--config`/`CRM_CONFIG` are exempt. + ## Tips for AI Agents - **Mount first:** `crm mount ~/crm` gives you filesystem access — read JSON files directly instead of running CLI commands diff --git a/src/cli.ts b/src/cli.ts index 1e57530..e5d99d8 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -7,6 +7,7 @@ import { registerLogCommand, } from './commands/activity' import { registerCompanyCommands } from './commands/company' +import { registerConfigCommands } from './commands/config' import { registerContactCommands } from './commands/contact' import { registerDealCommands, registerPipelineCommand } from './commands/deal' import { registerDupesCommand } from './commands/dupes' @@ -42,6 +43,7 @@ registerReportCommands(program) registerImportExportCommands(program) registerDupesCommand(program) registerFuseCommands(program) +registerConfigCommands(program) // Hidden subcommand: runs the FUSE daemon in-process (used by `crm mount`) if (cleanArgv[0] === '__daemon') { diff --git a/src/commands/config.ts b/src/commands/config.ts new file mode 100644 index 0000000..15d53f5 --- /dev/null +++ b/src/commands/config.ts @@ -0,0 +1,51 @@ +import { existsSync } from 'node:fs' + +import type { Command } from 'commander' + +import { resolveConfigPath } from '../config' +import { die, gConfig } from '../lib/helpers' +import { trustConfig, untrustConfig } from '../trust-store' + +/** Resolve the target config path for `config trust`/`config untrust` when + * no explicit path argument is given: respect the same precedence + * (--config > CRM_CONFIG > implicit discovery) the rest of the CLI uses. */ +function resolveTarget(path: string | undefined): string { + const target = path || resolveConfigPath(gConfig).path + if (!target) { + die( + 'Error: no crm.toml found to trust — pass a path explicitly, e.g. `crm config trust ./crm.toml`', + ) + } + if (!existsSync(target)) { + die(`Error: config file not found: ${target}`) + } + return target +} + +export function registerConfigCommands(program: Command) { + const cmd = program.command('config').description('Manage crm.toml trust') + + cmd + .command('trust [path]') + .description( + 'Trust a crm.toml so its [hooks] run without a confirmation prompt', + ) + .action((path?: string) => { + const target = resolveTarget(path) + trustConfig(target) + console.log( + `Trusted ${target} — hooks defined in it will now run without prompting.`, + ) + }) + + cmd + .command('untrust [path]') + .description('Revoke trust for a crm.toml') + .action((path?: string) => { + const target = resolveTarget(path) + const removed = untrustConfig(target) + console.log( + removed ? `Untrusted ${target}.` : `${target} was not trusted.`, + ) + }) +} diff --git a/src/config.ts b/src/config.ts index 137ff45..1434429 100644 --- a/src/config.ts +++ b/src/config.ts @@ -6,6 +6,14 @@ import { dirname, join, resolve } from 'node:path' import { parse as parseTOML } from 'toml' export interface CRMConfig { + /** + * Resolution metadata — not part of the TOML schema. Populated by + * `loadConfig` so callers (notably the hooks trust gate in `hooks.ts`) + * can tell whether this config came from an explicit source (`--config` + * / `CRM_CONFIG`) or was discovered implicitly, since only implicitly + * discovered configs are subject to the hooks trust-on-first-use gate. + */ + _meta?: ConfigResolution database: { path: string } defaults: { format: string } hooks: Record @@ -26,6 +34,13 @@ export interface CRMConfig { pipeline: { stages: string[]; won_stage: string; lost_stage: string } } +export type ConfigSource = 'explicit' | 'implicit' | 'none' + +export interface ConfigResolution { + path: string | null + source: ConfigSource +} + export const SEARCH_MODEL = 'mxbai-embed-xsmall-v1' const DEFAULT_STAGES = [ @@ -58,24 +73,89 @@ function defaultConfig(): CRMConfig { } } +/** + * Find the real git repository root containing `startDir`, by shelling out + * to `git rev-parse --show-toplevel`. This is the only trustworthy way to + * establish a project-root boundary: unlike checking for a `.git` path with + * `existsSync`, it can't be spoofed by planting an arbitrary file or + * directory named `.git` in an ancestor directory, and it correctly handles + * worktrees, submodules, and `.git` files (vs. directories). + * + * Returns `null` if `startDir` is not inside a git repository at all (or + * `git` isn't installed) — in that case there is no project-root boundary + * to find, and callers must not search upward toward the filesystem root. + */ +function findProjectRoot(startDir: string): string | null { + try { + const out = execSync('git rev-parse --show-toplevel', { + cwd: startDir, + stdio: ['pipe', 'pipe', 'pipe'], + }) + .toString() + .trim() + return out ? resolve(out) : null + } catch { + return null + } +} + +/** + * Search for `crm.toml` starting at `startDir` and walking up parent + * directories, but never past the project root (see `findProjectRoot`). + * This prevents an unrelated ancestor directory's `crm.toml` — whose + * `hooks` are executed without confirmation — from being loaded. + * + * If `startDir` isn't inside a real git repository, there is no known + * project boundary, so only `startDir` itself is checked — never walking + * upward toward the filesystem root. + * + * There is no implicit fallback to a global `~/.crm/config.toml`: if no + * `crm.toml` is found within the project, callers fall back to the + * built-in default config. + */ function findConfigFile(startDir: string): string | null { - let dir = resolve(startDir) + const root = findProjectRoot(startDir) + const start = resolve(startDir) + + if (root === null) { + const candidate = join(start, 'crm.toml') + return existsSync(candidate) ? candidate : null + } + + let dir = start while (true) { const candidate = join(dir, 'crm.toml') if (existsSync(candidate)) { return candidate } + if (dir === root) { + return null + } const parent = dirname(dir) if (parent === dir) { - break + return null } dir = parent } - const global = join(homedir(), '.crm', 'config.toml') - if (existsSync(global)) { - return global +} + +/** + * Resolve which `crm.toml` (if any) `loadConfig` would load, without + * reading or parsing it, and report whether that resolution was explicit + * (deliberate user action: `--config` flag or `CRM_CONFIG` env var) or + * implicit (discovered by searching cwd-or-upward). Only implicit + * resolution is subject to the hooks trust gate — see `hooks.ts`. + */ +export function resolveConfigPath(explicitPath?: string): ConfigResolution { + const explicit = explicitPath || process.env.CRM_CONFIG || null + if (explicit) { + return { path: resolve(explicit), source: 'explicit' } } - return null + const found = findConfigFile(process.cwd()) + if (found) { + return { path: found, source: 'implicit' } + } + return { path: null, source: 'none' } } function mergeConfig( @@ -178,24 +258,38 @@ export function loadConfig(opts: { let config = defaultConfig() // Resolve config file — auto-create with sensible defaults on first run - const configPath = - opts.configPath || - process.env.CRM_CONFIG || - findConfigFile(process.cwd()) || - (() => { - const p = join(homedir(), '.crm', 'config.toml') + const resolved = resolveConfigPath(opts.configPath) + let configPath: string | null = resolved.path + let source: ConfigSource = resolved.source + + if (!configPath) { + const root = findProjectRoot(process.cwd()) + const p = join(root ?? process.cwd(), 'crm.toml') + try { createDefaultConfig(p) - return p - })() + configPath = p + // Auto-created configs are found the same way an implicit crm.toml + // would be on the next run — treat them as implicit for the hooks + // trust gate rather than exempting them. + source = 'implicit' + } catch (_e) { + console.error(`Warning: could not create default config at ${p}`) + configPath = null + } + } - try { - const raw = readFileSync(configPath, 'utf-8') - const parsed = parseTOML(raw) - config = mergeConfig(config, parsed) - } catch (_e) { - console.error(`Warning: could not parse config file ${configPath}`) + if (configPath) { + try { + const raw = readFileSync(configPath, 'utf-8') + const parsed = parseTOML(raw) + config = mergeConfig(config, parsed) + } catch (_e) { + console.error(`Warning: could not parse config file ${configPath}`) + } } + config._meta = { path: configPath, source } + // Env var overrides (take priority over config file) if (process.env.CRM_PHONE_DEFAULT_COUNTRY) { config.phone.default_country = process.env.CRM_PHONE_DEFAULT_COUNTRY diff --git a/src/export-fs.ts b/src/export-fs.ts index 556518a..c77d0b3 100644 --- a/src/export-fs.ts +++ b/src/export-fs.ts @@ -19,6 +19,7 @@ import { LLM_TXT, slugify, } from './fuse-json' +import { safeJoin, sanitizeFilenameSegment } from './path-safety' import { computeConversion, computeForecast, @@ -92,40 +93,58 @@ export async function generateFS( const emails: string[] = safeJSON(c.emails) for (const email of emails) { - copyFileSync( - filePath, - join(outDir, 'contacts', '_by-email', `${email}.json`), - ) + const target = safeJoin(outDir, 'contacts', '_by-email', `${email}.json`) + if (target) { + copyFileSync(filePath, target) + } } const phones: string[] = safeJSON(c.phones) for (const phone of phones) { - copyFileSync( - filePath, - join(outDir, 'contacts', '_by-phone', `${phone}.json`), - ) + const target = safeJoin(outDir, 'contacts', '_by-phone', `${phone}.json`) + if (target) { + copyFileSync(filePath, target) + } } if (c.linkedin) { - copyFileSync( - filePath, - join(outDir, 'contacts', '_by-linkedin', `${c.linkedin}.json`), + const target = safeJoin( + outDir, + 'contacts', + '_by-linkedin', + `${c.linkedin}.json`, ) + if (target) { + copyFileSync(filePath, target) + } } if (c.x) { - copyFileSync(filePath, join(outDir, 'contacts', '_by-x', `${c.x}.json`)) + const target = safeJoin(outDir, 'contacts', '_by-x', `${c.x}.json`) + if (target) { + copyFileSync(filePath, target) + } } if (c.bluesky) { - copyFileSync( - filePath, - join(outDir, 'contacts', '_by-bluesky', `${c.bluesky}.json`), + const target = safeJoin( + outDir, + 'contacts', + '_by-bluesky', + `${c.bluesky}.json`, ) + if (target) { + copyFileSync(filePath, target) + } } if (c.telegram) { - copyFileSync( - filePath, - join(outDir, 'contacts', '_by-telegram', `${c.telegram}.json`), + const target = safeJoin( + outDir, + 'contacts', + '_by-telegram', + `${c.telegram}.json`, ) + if (target) { + copyFileSync(filePath, target) + } } const companyIds: string[] = safeJSON(c.companies) @@ -141,8 +160,11 @@ export async function generateFS( const tags: string[] = safeJSON(c.tags) for (const tag of tags) { - ensureDir(join(outDir, 'contacts', '_by-tag', tag)) - copyFileSync(filePath, join(outDir, 'contacts', '_by-tag', tag, filename)) + const tagDir = safeJoin(outDir, 'contacts', '_by-tag', tag) + if (tagDir) { + ensureDir(tagDir) + copyFileSync(filePath, join(tagDir, filename)) + } } } @@ -155,27 +177,32 @@ export async function generateFS( const websites: string[] = safeJSON(co.websites) for (const website of websites) { - copyFileSync( - filePath, - join(outDir, 'companies', '_by-website', `${website}.json`), + const target = safeJoin( + outDir, + 'companies', + '_by-website', + `${website}.json`, ) + if (target) { + copyFileSync(filePath, target) + } } const phones: string[] = safeJSON(co.phones) for (const phone of phones) { - copyFileSync( - filePath, - join(outDir, 'companies', '_by-phone', `${phone}.json`), - ) + const target = safeJoin(outDir, 'companies', '_by-phone', `${phone}.json`) + if (target) { + copyFileSync(filePath, target) + } } const tags: string[] = safeJSON(co.tags) for (const tag of tags) { - ensureDir(join(outDir, 'companies', '_by-tag', tag)) - copyFileSync( - filePath, - join(outDir, 'companies', '_by-tag', tag, filename), - ) + const tagDir = safeJoin(outDir, 'companies', '_by-tag', tag) + if (tagDir) { + ensureDir(tagDir) + copyFileSync(filePath, join(tagDir, filename)) + } } } @@ -188,9 +215,15 @@ export async function generateFS( writeJSON(filePath, data) if (d.stage) { - const stageDir = join(outDir, 'deals', '_by-stage', d.stage) - ensureDir(stageDir) - copyFileSync(filePath, join(stageDir, filename)) + // `d.stage` is validated against config.pipeline.stages by `deal + // add`/`update`, but `crm import deals` accepts any trimmed string + // (src/commands/importexport.ts) without that check, so it must be + // treated as untrusted here too. + const stageDir = safeJoin(outDir, 'deals', '_by-stage', d.stage) + if (stageDir) { + ensureDir(stageDir) + copyFileSync(filePath, join(stageDir, filename)) + } } if (d.company) { @@ -210,8 +243,11 @@ export async function generateFS( const tags: string[] = safeJSON(d.tags) for (const tag of tags) { - ensureDir(join(outDir, 'deals', '_by-tag', tag)) - copyFileSync(filePath, join(outDir, 'deals', '_by-tag', tag, filename)) + const tagDir = safeJoin(outDir, 'deals', '_by-tag', tag) + if (tagDir) { + ensureDir(tagDir) + copyFileSync(filePath, join(tagDir, filename)) + } } } @@ -230,12 +266,20 @@ export async function generateFS( .from(schema.contacts) .where(eq(schema.contacts.id, contactId)) if (contactResults[0]) { - const contactSlug = `${contactId}...${slugify(contactResults[0].name || '')}` - ensureDir(join(outDir, 'activities', '_by-contact', contactSlug)) - copyFileSync( - filePath, - join(outDir, 'activities', '_by-contact', contactSlug, filename), + // contactId is the linked contact's raw primary key (from + // a.contacts), not a value this loop generates — sanitize it for the + // same reason fuse-json.ts's filename builders do. + const contactSlug = `${sanitizeFilenameSegment(contactId)}...${slugify(contactResults[0].name || '')}` + const contactDir = safeJoin( + outDir, + 'activities', + '_by-contact', + contactSlug, ) + if (contactDir) { + ensureDir(contactDir) + copyFileSync(filePath, join(contactDir, filename)) + } } } @@ -255,19 +299,19 @@ export async function generateFS( } if (a.deal) { - ensureDir(join(outDir, 'activities', '_by-deal', a.deal)) - copyFileSync( - filePath, - join(outDir, 'activities', '_by-deal', a.deal, filename), - ) + const dealDir = safeJoin(outDir, 'activities', '_by-deal', a.deal) + if (dealDir) { + ensureDir(dealDir) + copyFileSync(filePath, join(dealDir, filename)) + } } if (a.type) { - ensureDir(join(outDir, 'activities', '_by-type', a.type)) - copyFileSync( - filePath, - join(outDir, 'activities', '_by-type', a.type, filename), - ) + const typeDir = safeJoin(outDir, 'activities', '_by-type', a.type) + if (typeDir) { + ensureDir(typeDir) + copyFileSync(filePath, join(typeDir, filename)) + } } } diff --git a/src/fuse-helper.c b/src/fuse-helper.c index f7e624b..c04a019 100644 --- a/src/fuse-helper.c +++ b/src/fuse-helper.c @@ -183,6 +183,36 @@ static int json_escape(char *buf, size_t maxlen, const char *s) { return p; } +/* + * Build a request of the form {"op":"","path":""}. + * Used by getattr/readdir/read/unlink, whose requests only ever interpolate + * `path` (never raw — always through json_escape()). `path` comes straight + * from the kernel and, per FUSE semantics, may contain any byte except NUL + * and '/' (including '"', '\', and control characters), so it must never be + * interpolated unescaped into the JSON we hand-build here. + * + * The buffer is sized dynamically off strlen(path) rather than using a + * fixed-size stack buffer: json_escape() can expand its input by up to + * ~2x+2 bytes (every character escaped), so a fixed buffer could overflow + * or silently truncate a long or heavily-quoted path into a different, + * valid-but-wrong request. + * + * Returns a malloc'd string (caller must free), or NULL on allocation + * failure. + */ +static char *build_path_request(const char *op, const char *path) { + size_t path_escaped_max = strlen(path) * 2 + 3; /* worst case: every byte escaped, plus quotes */ + size_t reqsize = strlen(op) + path_escaped_max + 128; + char *req = malloc(reqsize); + if (!req) return NULL; + + int rp = 0; + rp += snprintf(req + rp, reqsize - rp, "{\"op\":\"%s\",\"path\":", op); + rp += json_escape(req + rp, reqsize - rp, path); + snprintf(req + rp, reqsize - rp, "}"); + return req; +} + static char **json_get_entries(const char *json, int *count) { *count = 0; const char *v = json_find_key(json, "entries"); @@ -278,10 +308,11 @@ static int crm_getattr(const char *path, struct stat *stbuf, (void)fi; memset(stbuf, 0, sizeof(struct stat)); - char req[8192]; - snprintf(req, sizeof(req), "{\"op\":\"getattr\",\"path\":\"%s\"}", path); + char *req = build_path_request("getattr", path); + if (!req) return -ENOMEM; char *resp = sock_request(req); + free(req); if (!resp) return -EIO; if (json_has_error(resp)) { @@ -311,10 +342,11 @@ static int crm_readdir(const char *path, void *buf, fuse_fill_dir_t filler, filler(buf, ".", NULL, 0, 0); filler(buf, "..", NULL, 0, 0); - char req[8192]; - snprintf(req, sizeof(req), "{\"op\":\"readdir\",\"path\":\"%s\"}", path); + char *req = build_path_request("readdir", path); + if (!req) return -ENOMEM; char *resp = sock_request(req); + free(req); if (!resp) return -EIO; if (json_has_error(resp)) { @@ -351,10 +383,11 @@ static int crm_read(const char *path, char *buf, size_t size, off_t offset, struct fuse_file_info *fi) { (void)fi; - char req[8192]; - snprintf(req, sizeof(req), "{\"op\":\"read\",\"path\":\"%s\"}", path); + char *req = build_path_request("read", path); + if (!req) return -ENOMEM; char *resp = sock_request(req); + free(req); if (!resp) return -EIO; if (json_has_error(resp)) { @@ -437,8 +470,9 @@ static int crm_write(const char *path, const char *data, size_t size, /* Send data to daemon immediately for validation + persistence. * Bun's writeFileSync doesn't check close() errors, so we must * validate here in the write() syscall where errors propagate. */ + size_t path_escaped_max = strlen(path) * 2 + 3; size_t data_escaped_max = wb->len * 2 + 3; - size_t reqsize = strlen(path) + data_escaped_max + 128; + size_t reqsize = path_escaped_max + data_escaped_max + 128; char *req = malloc(reqsize); if (!req) { pthread_mutex_unlock(&g_write_mutex); @@ -446,7 +480,9 @@ static int crm_write(const char *path, const char *data, size_t size, } int rp = 0; - rp += snprintf(req + rp, reqsize - rp, "{\"op\":\"write\",\"path\":\"%s\",\"data\":", path); + rp += snprintf(req + rp, reqsize - rp, "{\"op\":\"write\",\"path\":"); + rp += json_escape(req + rp, reqsize - rp, path); + rp += snprintf(req + rp, reqsize - rp, ",\"data\":"); rp += json_escape(req + rp, reqsize - rp, wb->data); rp += snprintf(req + rp, reqsize - rp, "}"); @@ -488,8 +524,9 @@ static int crm_flush(const char *path, struct fuse_file_info *fi) { } /* Send uncommitted data to daemon (fallback for multi-chunk writes) */ + size_t path_escaped_max = strlen(path) * 2 + 3; size_t data_escaped_max = wb->len * 2 + 3; - size_t reqsize = strlen(path) + data_escaped_max + 128; + size_t reqsize = path_escaped_max + data_escaped_max + 128; char *req = malloc(reqsize); if (!req) { pthread_mutex_unlock(&g_write_mutex); @@ -497,7 +534,9 @@ static int crm_flush(const char *path, struct fuse_file_info *fi) { } int rp = 0; - rp += snprintf(req + rp, reqsize - rp, "{\"op\":\"write\",\"path\":\"%s\",\"data\":", path); + rp += snprintf(req + rp, reqsize - rp, "{\"op\":\"write\",\"path\":"); + rp += json_escape(req + rp, reqsize - rp, path); + rp += snprintf(req + rp, reqsize - rp, ",\"data\":"); rp += json_escape(req + rp, reqsize - rp, wb->data); rp += snprintf(req + rp, reqsize - rp, "}"); @@ -530,10 +569,11 @@ static int crm_release(const char *path, struct fuse_file_info *fi) { } static int crm_unlink(const char *path) { - char req[8192]; - snprintf(req, sizeof(req), "{\"op\":\"unlink\",\"path\":\"%s\"}", path); + char *req = build_path_request("unlink", path); + if (!req) return -ENOMEM; char *resp = sock_request(req); + free(req); if (!resp) return -EIO; if (json_has_error(resp)) { diff --git a/src/fuse-json.ts b/src/fuse-json.ts index 6daa7ba..7a2f964 100644 --- a/src/fuse-json.ts +++ b/src/fuse-json.ts @@ -5,6 +5,7 @@ import type { DB } from './db' import type { Activity, Company, Contact, Deal } from './drizzle-schema' import * as schema from './drizzle-schema' import { safeJSON } from './format' +import { sanitizeFilenameSegment } from './path-safety' export const LLM_TXT = `# CRM Filesystem @@ -88,21 +89,28 @@ export function slugify(name: string): string { .replace(/^-|-$/g, '') } +// Primary keys are always generated internally via `makeId()` (alphanumeric +// ULID-based), so this is a no-op for every record written by this CLI. It's +// defense-in-depth against the same untrusted-input surface as the +// `d.stage`/import-bypass fix in export-fs.ts: a future API, a compromised +// import format, or direct DB manipulation could otherwise smuggle a +// traversal payload through an id that flows straight into a filename. export function contactFilename(c: Contact): string { - return `${c.id}...${slugify(c.name || '')}.json` + return `${sanitizeFilenameSegment(c.id)}...${slugify(c.name || '')}.json` } export function companyFilename(co: Company): string { - return `${co.id}...${slugify(co.name || '')}.json` + return `${sanitizeFilenameSegment(co.id)}...${slugify(co.name || '')}.json` } export function dealFilename(d: Deal): string { - return `${d.id}...${slugify(d.title || '')}.json` + return `${sanitizeFilenameSegment(d.id)}...${slugify(d.title || '')}.json` } export function activityFilename(a: Activity): string { - const dateStr = (a.created_at || '').slice(0, 10) - return `${a.id}...${a.type || 'unknown'}-${dateStr}.json` + const dateStr = sanitizeFilenameSegment((a.created_at || '').slice(0, 10)) + const type = sanitizeFilenameSegment(a.type || 'unknown') + return `${sanitizeFilenameSegment(a.id)}...${type}-${dateStr}.json` } export async function buildContactJSON( diff --git a/src/hooks.ts b/src/hooks.ts index 26266fa..2023d25 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,6 +1,62 @@ import { spawnSync } from 'node:child_process' +import { closeSync, openSync, readSync } from 'node:fs' import type { CRMConfig } from './config.ts' +import { isTrusted, trustConfig } from './trust-store.ts' + +/** + * Prompt (via /dev/tty, mirroring `confirmOrForce` in lib/helpers.ts) + * whether to run — and remember — hooks defined in an untrusted, + * implicitly-discovered crm.toml. + */ +function promptTrustHooks(configPath: string): boolean { + process.stdout.write( + `crm.toml at ${configPath} defines hooks that have not been trusted. Run this hook now and remember this file? [y/N] `, + ) + const buf = Buffer.alloc(64) + const fd = openSync('/dev/tty', 'r') + try { + const n = readSync(fd, buf, 0, 64, null) + const answer = buf.slice(0, n).toString().trim().toLowerCase() + return answer === 'y' || answer === 'yes' + } finally { + closeSync(fd) + } +} + +/** + * Trust-on-first-use gate: hooks defined in a `crm.toml` that was + * discovered *implicitly* (cwd-or-ancestor search) must be trusted before + * they run, regardless of how — or whether — project-root detection can be + * spoofed. Explicitly-supplied configs (`--config` / `CRM_CONFIG`) are a + * deliberate user action and are exempt (see `resolveConfigPath` in + * config.ts). Returns `true` if the hook is cleared to run. + */ +function checkHookTrust(config: CRMConfig, hookName: string): boolean { + const meta = config._meta + if (!meta || meta.source !== 'implicit' || !meta.path) { + return true + } + if (isTrusted(meta.path)) { + return true + } + if (process.stdin.isTTY) { + try { + if (promptTrustHooks(meta.path)) { + trustConfig(meta.path) + return true + } + } catch { + // /dev/tty unavailable despite isTTY — fail closed below + } + console.error(`Skipping hook '${hookName}': ${meta.path} was not trusted.`) + return false + } + console.error( + `Warning: hooks in ${meta.path} are not trusted and no TTY is available to confirm; skipping hook '${hookName}'. Run \`crm config trust ${meta.path}\` to allow it.`, + ) + return false +} export function runHook( config: CRMConfig, @@ -12,6 +68,12 @@ export function runHook( return true // no hook = success } + if (!checkHookTrust(config, hookName)) { + // Untrusted hook is skipped, not treated as a pre-hook rejection — the + // overall command still completes normally (see hooks trust gate docs). + return true + } + const jsonData = JSON.stringify(data) const result = spawnSync(hookCmd, { shell: true, diff --git a/src/path-safety.ts b/src/path-safety.ts new file mode 100644 index 0000000..363e5be --- /dev/null +++ b/src/path-safety.ts @@ -0,0 +1,84 @@ +import { join, resolve, sep } from 'node:path' + +// Control characters (including NUL) — never valid in a filename on any +// supported platform, and NUL specifically truncates paths in native APIs. +// biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally matching control characters to strip them from untrusted path segments +const CONTROL_AND_NUL = /[\x00-\x1f]/g +// Characters that are invalid in Windows filenames. Since this fork runs on +// Windows, leaving these in would make `export-fs` fail with a confusing +// filesystem error instead of a security-relevant one. +const WINDOWS_INVALID_CHARS = /[<>:"|?*]/g +// Path separators, both POSIX and Windows — the actual mechanism that would +// let a segment escape the directory it's being placed into. +const PATH_SEPARATORS = /[\\/]/g +// The literal ".." sequence. Neutralized independently of PATH_SEPARATORS +// as defense-in-depth: even if some future call site reintroduces separators +// after sanitizing, a lone ".." segment can't be (re)combined into a +// parent-directory reference. +const PARENT_DIR_SEQUENCE = /\.\./g +// Windows silently strips trailing dots/spaces from path components, which +// can otherwise cause a filename to mismatch the value that was intended. +const TRAILING_DOTS_OR_SPACES = /[. ]+$/ + +/** + * Sanitize a single path segment derived from user-controlled database + * fields (email, phone, tag, social handle, website, activity type, etc.) + * so it's always safe to use as a file or directory name. + * + * Normal filename-safe values (emails, E.164 phone numbers, @handles, + * domains, tags) are returned byte-for-byte unchanged — this is what keeps + * the documented `_by-email` / `_by-phone` / etc. lookup UX working. Only + * characters that are structurally dangerous (path separators, the literal + * `..` sequence) or invalid on a supported OS (NUL/control characters, + * Windows-reserved characters) are altered. + * + * Never returns an empty string: if sanitizing would otherwise produce one + * (e.g. the input was only `/`, `..`, or Windows-invalid characters), falls + * back to `'unknown'`, matching the convention `slugify()` in fuse-json.ts + * already uses for empty/missing values. + */ +export function sanitizeFilenameSegment( + value: string | null | undefined, +): string { + const sanitized = (value ?? '') + .replace(CONTROL_AND_NUL, '') + .replace(WINDOWS_INVALID_CHARS, '') + .replace(PATH_SEPARATORS, '_') + .replace(PARENT_DIR_SEQUENCE, '_') + .trim() + .replace(TRAILING_DOTS_OR_SPACES, '') + + return sanitized || 'unknown' +} + +/** + * Join `base` with one or more untrusted path segments, sanitizing each + * segment first and then verifying — via `path.resolve()` — that the + * resulting path still lives inside `base` before returning it. + * + * This is defense-in-depth on top of `sanitizeFilenameSegment`: sanitization + * alone should already make escaping `base` impossible, but this guards + * against any future regression or an edge case the sanitizer misses. + * + * Returns `null` (and logs a warning) instead of throwing when containment + * fails, so callers can skip that particular write and continue the export + * rather than aborting the whole run. + */ +export function safeJoin(base: string, ...segments: string[]): string | null { + const sanitizedSegments = segments.map(sanitizeFilenameSegment) + const candidate = join(base, ...sanitizedSegments) + const resolvedBase = resolve(base) + const resolvedCandidate = resolve(candidate) + + if ( + resolvedCandidate !== resolvedBase && + !resolvedCandidate.startsWith(resolvedBase + sep) + ) { + console.error( + `Warning: skipping write outside export directory: ${join(...segments)}`, + ) + return null + } + + return candidate +} diff --git a/src/trust-store.ts b/src/trust-store.ts new file mode 100644 index 0000000..12f7ba9 --- /dev/null +++ b/src/trust-store.ts @@ -0,0 +1,104 @@ +import { createHash } from 'node:crypto' +import { + existsSync, + mkdirSync, + readFileSync, + realpathSync, + writeFileSync, +} from 'node:fs' +import { homedir } from 'node:os' +import { dirname, join, resolve } from 'node:path' + +/** + * Local trust-on-first-use (TOFU) ledger for implicitly-discovered + * `crm.toml` files, keyed on the config's canonical absolute path plus a + * content hash. This is what direnv/mise call "trusting" a config: hooks + * defined in an implicitly-discovered config only run after the user has + * explicitly approved that exact file content. + * + * This file stores only paths and sha256 hashes — never config content — + * and is never read as a source of CRM configuration. It does not + * reintroduce the removed `~/.crm/config.toml` global-config fallback. + */ + +export const TRUST_STORE_PATH = join(homedir(), '.crm', 'trusted_configs.json') + +type TrustStoreData = Record + +/** + * Resolve to an absolute, symlink-resolved path so trust-store keys are + * stable regardless of how the config path was spelled on the command line. + * Falls back to a plain absolute path if the file doesn't exist (e.g. it + * was deleted after being trusted). + */ +function canonicalPath(configPath: string): string { + try { + return realpathSync(configPath) + } catch { + return resolve(configPath) + } +} + +function hashFile(configPath: string): string { + const contents = readFileSync(configPath) + return createHash('sha256').update(contents).digest('hex') +} + +function loadTrustStore(): TrustStoreData { + try { + const raw = readFileSync(TRUST_STORE_PATH, 'utf-8') + const parsed = JSON.parse(raw) + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + return parsed as TrustStoreData + } + } catch { + // missing, unreadable, or invalid JSON — treat as an empty store + } + return {} +} + +function saveTrustStore(store: TrustStoreData): void { + mkdirSync(dirname(TRUST_STORE_PATH), { recursive: true }) + writeFileSync(TRUST_STORE_PATH, `${JSON.stringify(store, null, 2)}\n`) +} + +/** + * Whether `configPath`'s current on-disk content matches a previously + * recorded trust entry. Returns `false` if the file was never trusted, no + * longer exists, or its content has changed since it was trusted (hash + * mismatch = untrusted, forcing re-trust). + */ +export function isTrusted(configPath: string): boolean { + if (!existsSync(configPath)) { + return false + } + const store = loadTrustStore() + const trustedHash = store[canonicalPath(configPath)] + if (!trustedHash) { + return false + } + try { + return hashFile(configPath) === trustedHash + } catch { + return false + } +} + +/** Record `configPath`'s current content hash as trusted. */ +export function trustConfig(configPath: string): void { + const store = loadTrustStore() + store[canonicalPath(configPath)] = hashFile(configPath) + saveTrustStore(store) +} + +/** Remove `configPath` from the trust store. Returns whether it was present. */ +export function untrustConfig(configPath: string): boolean { + const key = canonicalPath(configPath) + const store = loadTrustStore() + if (!(key in store)) { + return false + } + delete store[key] + saveTrustStore(store) + return true +} diff --git a/test/config.test.ts b/test/config.test.ts index 90755c9..50ce38d 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -1,8 +1,47 @@ import { describe, expect, test } from 'bun:test' -import { mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs' +import { execSync } from 'node:child_process' +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' import { join } from 'node:path' - -import { createTestContext } from './helpers.ts' +import { platform } from 'node:process' + +import { createTestContext, initGitRepo } from './helpers.ts' + +const CRM_BIN = join(import.meta.dir, '..', 'src', 'cli.ts') + +/** + * Deny write access to `dir` for the current user so a subsequent file + * creation attempt inside it fails deterministically. On Windows, a plain + * `chmodSync` read-only attribute on a *directory* does not block creating + * new files inside it, so an ACL deny rule (`icacls`) is required instead. + */ +function lockDirectory(dir: string): void { + if (platform === 'win32') { + execSync(`icacls "${dir}" /deny "${process.env.USERNAME}:(OI)(CI)W"`, { + stdio: 'ignore', + }) + } else { + chmodSync(dir, 0o555) + } +} + +function unlockDirectory(dir: string): void { + if (platform === 'win32') { + execSync(`icacls "${dir}" /remove:d "${process.env.USERNAME}"`, { + stdio: 'ignore', + }) + } else { + chmodSync(dir, 0o755) + } +} describe('config: phone settings', () => { test('phone.default_country allows short numbers', () => { @@ -471,7 +510,8 @@ describe('config resolution', () => { test('crm.toml in parent directory is found', () => { const ctx = createTestContext() - // Put config in ctx.dir (the parent). + // ctx.dir is the project root (a real git repo) and holds crm.toml. + initGitRepo(ctx.dir) writeFileSync( join(ctx.dir, 'crm.toml'), `[pipeline]\nstages = ["parent-1", "parent-2"]\n`, @@ -504,6 +544,8 @@ describe('config resolution', () => { test('crm.toml in grandparent directory is found', () => { const ctx = createTestContext() + // ctx.dir is the project root (a real git repo) and holds crm.toml. + initGitRepo(ctx.dir) writeFileSync( join(ctx.dir, 'crm.toml'), `[pipeline]\nstages = ["grandparent-1", "grandparent-2"]\n`, @@ -690,6 +732,329 @@ describe('config resolution', () => { }) }) +describe('config resolution: scoped to project directory (security)', () => { + test('crm.toml above the project root (.git boundary) is not loaded', () => { + const ctx = createTestContext({ noConfig: true }) + + // An unrelated ancestor directory has its own crm.toml. + writeFileSync( + join(ctx.dir, 'crm.toml'), + `[pipeline]\nstages = ["outer-stage"]\n`, + ) + + // The project root sits below it and is a real git repo. + const projectDir = join(ctx.dir, 'project') + mkdirSync(projectDir, { recursive: true }) + initGitRepo(projectDir) + + // The outer/ancestor stage must NOT be visible from inside the project. + const outerAttempt = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'deal', + 'add', + '--title', + 'Test', + '--stage', + 'outer-stage', + ], + { cwd: projectDir, env: { ...process.env, NO_COLOR: '1' } }, + ) + expect(outerAttempt.exitCode).not.toBe(0) + + // Built-in default config is used instead (its default stages apply). + const defaultAttempt = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'deal', + 'add', + '--title', + 'Test2', + '--stage', + 'lead', + ], + { cwd: projectDir, env: { ...process.env, NO_COLOR: '1' } }, + ) + expect(defaultAttempt.exitCode).toBe(0) + }) + + test('crm.toml at the project root (.git directory) is still found from a subdirectory', () => { + const ctx = createTestContext({ noConfig: true }) + initGitRepo(ctx.dir) + writeFileSync( + join(ctx.dir, 'crm.toml'), + `[pipeline]\nstages = ["root-stage"]\n`, + ) + + const nested = join(ctx.dir, 'a', 'b') + mkdirSync(nested, { recursive: true }) + + const proc = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'deal', + 'add', + '--title', + 'Test', + '--stage', + 'root-stage', + ], + { cwd: nested, env: { ...process.env, NO_COLOR: '1' } }, + ) + expect(proc.exitCode).toBe(0) + }) + + test('when no .git is found anywhere, only the current directory is checked (not ancestors)', () => { + const ctx = createTestContext({ noConfig: true }) + + // No .git anywhere in this chain — crm.toml sits in ctx.dir only. + writeFileSync( + join(ctx.dir, 'crm.toml'), + `[pipeline]\nstages = ["parent-only-stage"]\n`, + ) + + const subdir = join(ctx.dir, 'subproject') + mkdirSync(subdir) + + const proc = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'deal', + 'add', + '--title', + 'Test', + '--stage', + 'parent-only-stage', + ], + { cwd: subdir, env: { ...process.env, NO_COLOR: '1' } }, + ) + expect(proc.exitCode).not.toBe(0) + }) + + test('does not fall back to ~/.crm/config.toml when no project config exists', () => { + const ctx = createTestContext({ noConfig: true }) + const fakeHome = mkdtempSync(join(tmpdir(), 'crm-fakehome-')) + mkdirSync(join(fakeHome, '.crm'), { recursive: true }) + writeFileSync( + join(fakeHome, '.crm', 'config.toml'), + `[pipeline]\nstages = ["global-stage"]\n`, + ) + + const projectDir = join(ctx.dir, 'project') + mkdirSync(projectDir, { recursive: true }) + const fakeHomeEnv = { + ...process.env, + NO_COLOR: '1', + HOME: fakeHome, + USERPROFILE: fakeHome, + } + + // The global config's stage must be ignored. + const globalAttempt = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'deal', + 'add', + '--title', + 'Test', + '--stage', + 'global-stage', + ], + { cwd: projectDir, env: fakeHomeEnv }, + ) + expect(globalAttempt.exitCode).not.toBe(0) + + // Falls back to the built-in default config instead. + const defaultAttempt = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'deal', + 'add', + '--title', + 'Test2', + '--stage', + 'lead', + ], + { cwd: projectDir, env: fakeHomeEnv }, + ) + expect(defaultAttempt.exitCode).toBe(0) + }) + + test('auto-generated default config is created at the project root, not the home directory', () => { + const ctx = createTestContext({ noConfig: true }) + const fakeHome = mkdtempSync(join(tmpdir(), 'crm-fakehome-')) + const projectDir = join(ctx.dir, 'project') + mkdirSync(projectDir, { recursive: true }) + initGitRepo(projectDir) + + const proc = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'contact', + 'add', + '--name', + 'Jane', + ], + { + cwd: projectDir, + env: { + ...process.env, + NO_COLOR: '1', + HOME: fakeHome, + USERPROFILE: fakeHome, + }, + }, + ) + expect(proc.exitCode).toBe(0) + expect(existsSync(join(projectDir, 'crm.toml'))).toBe(true) + expect(existsSync(join(fakeHome, '.crm', 'config.toml'))).toBe(false) + }) + + test('auto-generated default config is created in the current directory when no .git is found', () => { + const ctx = createTestContext({ noConfig: true }) + const fakeHome = mkdtempSync(join(tmpdir(), 'crm-fakehome-')) + const projectDir = join(ctx.dir, 'project') + mkdirSync(projectDir, { recursive: true }) + + const proc = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'contact', + 'add', + '--name', + 'Jane', + ], + { + cwd: projectDir, + env: { + ...process.env, + NO_COLOR: '1', + HOME: fakeHome, + USERPROFILE: fakeHome, + }, + }, + ) + expect(proc.exitCode).toBe(0) + expect(existsSync(join(projectDir, 'crm.toml'))).toBe(true) + expect(existsSync(join(fakeHome, '.crm', 'config.toml'))).toBe(false) + }) + + test('a bare ".git" file (not a real repository) in an ancestor does not establish a trust boundary — hooks are not executed', () => { + const ctx = createTestContext({ noConfig: true }) + + // Attacker plants a fake ".git" — an arbitrary file, not a real git + // repository — plus a malicious crm.toml with a [hooks] entry, in an + // ancestor directory (e.g. an extracted archive, a shared drive, $HOME). + writeFileSync(join(ctx.dir, '.git'), 'not a real git repository') + + const markerFile = join(ctx.dir, 'pwned.txt').replace(/\\/g, '/') + const hookScript = join(ctx.dir, 'hook.js') + writeFileSync( + hookScript, + `require('node:fs').writeFileSync('${markerFile}', 'pwned')\n`, + ) + const hookScriptFwd = hookScript.replace(/\\/g, '/') + const hookCmd = `node "${hookScriptFwd}"` + writeFileSync( + join(ctx.dir, 'crm.toml'), + `[hooks]\npost-contact-add = "${hookCmd.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"\n`, + ) + + // The victim runs the CLI from a subdirectory that has NO real git repo + // of its own — a very common situation (ad hoc folder, extracted + // archive, shared drive, home directory). + const victimDir = join(ctx.dir, 'subproject') + mkdirSync(victimDir, { recursive: true }) + + Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'contact', + 'add', + '--name', + 'Jane', + ], + { cwd: victimDir, env: { ...process.env, NO_COLOR: '1' } }, + ) + + // The malicious hook must NOT have run — the fake ".git" file must not + // be trusted as a project-root boundary. + expect(existsSync(join(ctx.dir, 'pwned.txt'))).toBe(false) + }) +}) + +describe('config resolution: auto-create-default-config failure handling', () => { + test('loadConfig falls back to in-memory defaults when the auto-created config cannot be written', () => { + const ctx = createTestContext({ noConfig: true }) + const projectDir = join(ctx.dir, 'project') + mkdirSync(projectDir, { recursive: true }) + // No .git anywhere — findProjectRoot has no boundary, so the + // auto-created config would be written directly into projectDir. + + lockDirectory(projectDir) + try { + const proc = Bun.spawnSync( + [ + 'bun', + 'run', + CRM_BIN, + '--db', + ctx.dbPath, + 'contact', + 'add', + '--name', + 'Jane', + ], + { cwd: projectDir, env: { ...process.env, NO_COLOR: '1' } }, + ) + // Must NOT hard-fail with a raw fs error — should continue using the + // in-memory default config instead. + expect(proc.exitCode).toBe(0) + expect(proc.stdout.toString().trim().length).toBeGreaterThan(0) + // No config file should have been left behind in the locked directory. + expect(existsSync(join(projectDir, 'crm.toml'))).toBe(false) + } finally { + unlockDirectory(projectDir) + } + }) +}) + describe('config: env var overrides', () => { test('CRM_PHONE_DISPLAY env var overrides config file', () => { const ctx = createTestContext() diff --git a/test/export-fs-security.test.ts b/test/export-fs-security.test.ts new file mode 100644 index 0000000..be4bdf9 --- /dev/null +++ b/test/export-fs-security.test.ts @@ -0,0 +1,331 @@ +import { describe, expect, test } from 'bun:test' +import { existsSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' + +import type { CRMConfig } from '../src/config.ts' +import { openDB } from '../src/db.ts' +import * as schema from '../src/drizzle-schema.ts' +import { generateFS } from '../src/export-fs.ts' +import { safeJoin, sanitizeFilenameSegment } from '../src/path-safety.ts' +import { createTestContext } from './helpers.ts' + +function testConfig(): CRMConfig { + return { + database: { path: '' }, + defaults: { format: 'table' }, + hooks: {}, + mount: { + default_path: '', + readonly: false, + allow_other: false, + max_recent_activity: 10, + search_limit: 20, + }, + phone: { display: 'international' }, + pipeline: { + stages: [ + 'lead', + 'qualified', + 'proposal', + 'negotiation', + 'closed-won', + 'closed-lost', + ], + won_stage: 'closed-won', + lost_stage: 'closed-lost', + }, + } +} + +/** Recursively collect every file path under `dir`. */ +function walk(dir: string): string[] { + const out: string[] = [] + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const p = join(dir, entry.name) + if (entry.isDirectory()) { + out.push(...walk(p)) + } else { + out.push(p) + } + } + return out +} + +describe('path-safety: sanitizeFilenameSegment', () => { + test('passes through normal filename-safe values unchanged', () => { + expect(sanitizeFilenameSegment('jane@acme.com')).toBe('jane@acme.com') + expect(sanitizeFilenameSegment('+12125551234')).toBe('+12125551234') + expect(sanitizeFilenameSegment('acme.com')).toBe('acme.com') + expect(sanitizeFilenameSegment('vip')).toBe('vip') + expect(sanitizeFilenameSegment('jane-doe_99')).toBe('jane-doe_99') + }) + + test('neutralizes path separators', () => { + expect(sanitizeFilenameSegment('a/b')).not.toContain('/') + expect(sanitizeFilenameSegment('a\\b')).not.toContain('\\') + }) + + test('neutralizes the literal ".." sequence', () => { + const result = sanitizeFilenameSegment('../../../etc/passwd') + expect(result).not.toContain('..') + expect(result).not.toContain('/') + }) + + test('strips NUL bytes and control characters', () => { + const withNul = `evil${String.fromCharCode(0)}name` + expect(sanitizeFilenameSegment(withNul)).not.toContain( + String.fromCharCode(0), + ) + }) + + test('strips characters invalid in Windows filenames', () => { + const result = sanitizeFilenameSegment('ac:d"e|f?g*h') + for (const ch of ['<', '>', ':', '"', '|', '?', '*']) { + expect(result).not.toContain(ch) + } + }) + + test('falls back to a safe placeholder when sanitization empties the value', () => { + expect(sanitizeFilenameSegment('???')).toBe('unknown') + expect(sanitizeFilenameSegment('')).toBe('unknown') + }) +}) + +describe('path-safety: safeJoin', () => { + test('returns a path inside base for a well-formed segment', () => { + const base = join('tmp-base', 'out') + const result = safeJoin(base, 'contacts', '_by-email', 'jane@acme.com.json') + expect(result).not.toBeNull() + expect(result).toContain('jane@acme.com.json') + }) + + test('never escapes base even for a raw traversal segment', () => { + const base = join('tmp-base', 'out') + const result = safeJoin(base, '../../../etc/passwd') + expect(result).not.toBeNull() + expect((result as string).startsWith(join(base))).toBe(true) + }) +}) + +describe('export-fs: path traversal hardening', () => { + test('contact with traversal payloads in email/tag/linkedin does not escape outDir', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + ctx.runOK( + 'contact', + 'add', + '--name', + 'Evil', + '--email', + '../../../pwned-email@evil.com', + '--tag', + '../../../pwned-tag', + '--linkedin', + '../../../pwned-linkedin', + ) + ctx.runOK('export-fs', outDir) + + // Nothing escaped one level above outDir (into the test's own tmp dir). + expect(existsSync(join(ctx.dir, 'pwned-email@evil.com.json'))).toBe(false) + expect(existsSync(join(ctx.dir, 'pwned-tag'))).toBe(false) + expect(existsSync(join(ctx.dir, 'pwned-linkedin.json'))).toBe(false) + + // Every file actually written stays confined under outDir. + for (const f of walk(outDir)) { + expect(f.startsWith(outDir)).toBe(true) + } + }) + + test('normal email/tag/linkedin values still produce the documented lookup filenames', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + ctx.runOK( + 'contact', + 'add', + '--name', + 'Jane', + '--email', + 'jane@acme.com', + '--tag', + 'vip', + '--linkedin', + 'janedoe', + ) + ctx.runOK('export-fs', outDir) + + expect( + existsSync(join(outDir, 'contacts', '_by-email', 'jane@acme.com.json')), + ).toBe(true) + expect(existsSync(join(outDir, 'contacts', '_by-tag', 'vip'))).toBe(true) + expect( + existsSync(join(outDir, 'contacts', '_by-linkedin', 'janedoe.json')), + ).toBe(true) + }) + + test('company phone traversal payload (via import, bypassing strict CLI phone parsing) does not escape outDir', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + const jsonPath = join(ctx.dir, 'companies.json') + writeFileSync( + jsonPath, + JSON.stringify([{ name: 'Evil Corp', phone: '../../../pwned-phone' }]), + ) + ctx.runOK('import', 'companies', jsonPath) + ctx.runOK('export-fs', outDir) + + expect(existsSync(join(ctx.dir, 'pwned-phone.json'))).toBe(false) + for (const f of walk(outDir)) { + expect(f.startsWith(outDir)).toBe(true) + } + }) + + test('normal company phone value still produces the documented lookup filename', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + const jsonPath = join(ctx.dir, 'companies.json') + writeFileSync( + jsonPath, + JSON.stringify([{ name: 'Acme Corp', phone: '+12125551234' }]), + ) + ctx.runOK('import', 'companies', jsonPath) + ctx.runOK('export-fs', outDir) + + expect( + existsSync(join(outDir, 'companies', '_by-phone', '+12125551234.json')), + ).toBe(true) + }) + + test('company tag traversal payload does not escape outDir', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + ctx.runOK( + 'company', + 'add', + '--name', + 'Evil Co', + '--tag', + '../../../pwned-co-tag', + ) + ctx.runOK('export-fs', outDir) + expect(existsSync(join(ctx.dir, 'pwned-co-tag'))).toBe(false) + }) + + test('deal tag traversal payload does not escape outDir', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + ctx.runOK( + 'deal', + 'add', + '--title', + 'Evil Deal', + '--tag', + '../../../pwned-deal-tag', + ) + ctx.runOK('export-fs', outDir) + expect(existsSync(join(ctx.dir, 'pwned-deal-tag'))).toBe(false) + }) + + test("deal stage traversal payload (via import, bypassing `deal add`'s stage validation) does not escape outDir", () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + const csv = 'title,value,stage\nEvil Deal,1000,../../../pwned-stage\n' + const csvPath = join(ctx.dir, 'deals.csv') + writeFileSync(csvPath, csv) + ctx.runOK('import', 'deals', csvPath) + + ctx.runOK('export-fs', outDir) + + expect(existsSync(join(ctx.dir, 'pwned-stage'))).toBe(false) + for (const f of walk(outDir)) { + expect(f.startsWith(outDir)).toBe(true) + } + }) + + test('activity with traversal payloads in type/deal (bypassing CLI validation) does not escape outDir', async () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + const dbPath = join(ctx.dir, 'direct.db') + const db = await openDB(dbPath) + const now = new Date().toISOString() + await db.insert(schema.activities).values({ + id: 'act_evil', + type: '../../../pwned-type', + body: 'evil', + contacts: '[]', + company: null, + deal: '../../../pwned-deal', + custom_fields: '{}', + created_at: now, + }) + + await generateFS(db, testConfig(), outDir) + + expect(existsSync(join(ctx.dir, 'pwned-type'))).toBe(false) + expect(existsSync(join(ctx.dir, 'pwned-deal'))).toBe(false) + for (const f of walk(outDir)) { + expect(f.startsWith(outDir)).toBe(true) + } + }) + + test('a record with a traversal payload as its primary-key id (bypassing normal ID generation) does not escape outDir', async () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + const dbPath = join(ctx.dir, 'direct-id.db') + const db = await openDB(dbPath) + const now = new Date().toISOString() + const evilId = '../../../pwned-contact-id' + + await db.insert(schema.contacts).values({ + id: evilId, + name: 'Evil', + emails: '[]', + phones: '[]', + companies: '[]', + tags: '[]', + custom_fields: '{}', + created_at: now, + updated_at: now, + }) + // An activity referencing the malicious contact id exercises the + // `_by-contact` fan-out, which also embeds the raw contact id. + await db.insert(schema.activities).values({ + id: 'act_ref_evil', + type: 'note', + body: 'evil', + contacts: JSON.stringify([evilId]), + company: null, + deal: null, + custom_fields: '{}', + created_at: now, + }) + + // Compute where the unsanitized filename (`${id}...${slug}.json`) would + // land when joined as `contacts/` — three leading `../` + // segments in the id climb out of outDir/contacts, past outDir itself, + // to outDir's grandparent (ctx.dir's parent, i.e. the OS temp root). + const wouldBeEscapePath = join(outDir, 'contacts', `${evilId}...evil.json`) + expect(wouldBeEscapePath.startsWith(outDir)).toBe(false) // sanity: the payload is a real traversal + + try { + await generateFS(db, testConfig(), outDir) + + expect(existsSync(wouldBeEscapePath)).toBe(false) + for (const f of walk(outDir)) { + expect(f.startsWith(outDir)).toBe(true) + } + } finally { + rmSync(wouldBeEscapePath, { force: true }) + } + }) + + test('tag that sanitizes to empty falls back to a safe name instead of crashing', () => { + const ctx = createTestContext() + const outDir = join(ctx.dir, 'export') + ctx.runOK('contact', 'add', '--name', 'Weird', '--tag', '???') + const result = ctx.run('export-fs', outDir) + expect(result.exitCode).toBe(0) + expect(existsSync(join(outDir, 'contacts', '_by-tag', 'unknown'))).toBe( + true, + ) + }) +}) diff --git a/test/fuse-json-injection.test.ts b/test/fuse-json-injection.test.ts new file mode 100644 index 0000000..c5f0229 --- /dev/null +++ b/test/fuse-json-injection.test.ts @@ -0,0 +1,165 @@ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test' +import { existsSync, mkdirSync, readdirSync, statSync } from 'node:fs' +import { join } from 'node:path' + +import { canMount, createTestContext, type TestContext } from './helpers.ts' + +/** + * Regression coverage for the fuse-helper.c JSON injection vulnerability: + * `crm_getattr`/`crm_readdir`/`crm_read`/`crm_write`/`crm_flush`/`crm_unlink` + * interpolated the raw, unescaped FUSE `path` argument into a hand-rolled + * JSON request string sent to the daemon. Since FUSE passes through any byte + * except NUL and `/` as a path component, a path segment containing a + * literal `"` lets an attacker break out of the `"path":"..."` string and + * inject sibling JSON keys — e.g. turning a harmless `getattr` (triggered by + * a mere `stat()`/`existsSync()`) into an `unlink` of a real, pre-existing + * file, because `JSON.parse` keeps the *last* occurrence of a duplicate key. + * + * These tests drive the real compiled `crm-fuse` C binary + `fuse-daemon.ts` + * through a live FUSE mount (gated by `canMount`, same as test/fuse.test.ts) + * rather than relying on static analysis of fuse-helper.c. + */ + +interface FuseTestContext extends TestContext { + mounted: boolean + mountPoint: string +} + +let ctx: FuseTestContext | null = null + +function unmount() { + if (ctx?.mounted) { + ctx.run('unmount', ctx.mountPoint) + ctx.mounted = false + } +} + +beforeAll(() => { + if (!canMount) { + return + } + const c = createTestContext() as FuseTestContext + c.mountPoint = join(c.dir, 'mnt') + mkdirSync(c.mountPoint) + c.runOK('contact', 'list') + const result = c.run('mount', c.mountPoint) + if (result.exitCode !== 0) { + c.mounted = false + ctx = c + return + } + const deadline = Date.now() + 10_000 + let ready = false + while (Date.now() < deadline) { + try { + if (readdirSync(c.mountPoint).includes('contacts')) { + ready = true + break + } + } catch { + /* not mounted yet */ + } + Bun.sleepSync(50) + } + c.mounted = ready + ctx = c +}) + +afterAll(() => { + unmount() +}) + +function skipIfNoFuse(): boolean { + if (!ctx?.mounted) { + console.warn('mount not available — skipping test') + return true + } + return false +} + +/** Helper: list only entity .json files (excludes _by-* dirs) */ +function entityFiles(dir: string): string[] { + return readdirSync(dir).filter( + (f) => f.endsWith('.json') && !f.startsWith('_'), + ) +} + +describe('fuse: JSON/protocol injection hardening', () => { + test('getattr on a quote-injected path must not smuggle an unlink of a real file', () => { + if (skipIfNoFuse()) { + return + } + const mp = ctx!.mountPoint + + // 1. Create a real victim contact via the CLI. + ctx!.runOK( + 'contact', + 'add', + '--name', + 'Injection Victim', + '--email', + 'injection-victim@acme.com', + ) + + const victimFile = entityFiles(join(mp, 'contacts')).find((f) => + f.includes('injection-victim'), + ) + expect(victimFile).toBeDefined() + + const victimPath = join(mp, 'contacts', victimFile!) + expect(existsSync(victimPath)).toBe(true) + + // 2. Sanity-check the victim exists from the DB's point of view too. + // The FUSE kernel attribute/dentry cache means re-stat'ing the same + // clean victim path through the mount right after the exploit can + // return a stale cached "it still exists" result even if the backing + // DB row was actually deleted. Query through the CLI instead — it + // talks to the SQLite DB directly and is unaffected by FUSE caching. + function victimExistsInDB(): boolean { + return ( + ctx!.run('contact', 'show', 'injection-victim@acme.com').exitCode === 0 + ) + } + expect(victimExistsInDB()).toBe(true) + + // 3. Craft a path segment that, once naively interpolated into + // fuse-helper.c's `{"op":"getattr","path":"%s"}` template, closes the + // "path" string early and injects a sibling `"op":"unlink"` key — + // while leaving the (real, correct) "path" value untouched so the + // smuggled unlink targets the real victim file: + // + // {"op":"getattr","path":"/contacts/","op":"unlink"} + // + // JSON.parse keeps the last "op" (unlink) and the only "path" (the + // real victim), so this must not be reachable via a bare stat(). + const maliciousComponent = `${victimFile}","op":"unlink` + const maliciousPath = join(mp, 'contacts', maliciousComponent) + + // 4. A mere stat() on the crafted path is the trigger. Its own outcome + // (success vs. ENOENT) differs between vulnerable and fixed code, so we + // don't assert on it directly here — the real assertion is below: the + // untouched victim file must survive regardless. + let statThrew = false + let statErrorCode: string | undefined + try { + statSync(maliciousPath) + } catch (err) { + statThrew = true + statErrorCode = (err as NodeJS.ErrnoException).code + } + + // 5. Core regression assertion: the real victim contact must still + // exist in the DB. On unpatched fuse-helper.c, the crafted stat() above + // smuggles an unlink of the victim and this assertion fails (proving + // the vulnerability). After the fix, `path` is fully escaped, the + // crafted component can never terminate the JSON "path" string early, + // and the victim survives untouched. + expect(victimExistsInDB()).toBe(true) + + // 6. On fixed code, the crafted (garbage, escaped) filename doesn't + // correspond to any real entity, so stat() should fail cleanly with + // ENOENT rather than succeeding or crashing. + expect(statThrew).toBe(true) + expect(statErrorCode).toBe('ENOENT') + }) +}) diff --git a/test/helpers.ts b/test/helpers.ts index c7fed39..c515a58 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -1,3 +1,4 @@ +import { execSync } from 'node:child_process' import { existsSync, mkdtempSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -5,6 +6,16 @@ import { join } from 'node:path' /** Whether the current platform supports FUSE/NFS mount tests */ export const canMount = existsSync('/dev/fuse') // Linux FUSE only — macOS NFS mount causes kernel panics, skip for now +/** + * Initialize a real git repository at `dir`. Used by tests that need a + * genuine project-root boundary — config resolution trusts `git + * rev-parse --show-toplevel`, not the mere presence of a `.git` path, so + * tests must create real repos rather than a bare `.git` file/directory. + */ +export function initGitRepo(dir: string): void { + execSync('git init', { cwd: dir, stdio: 'ignore' }) +} + const CRM_BIN = join(import.meta.dir, '..', 'src', 'cli.ts') const TEST_CONFIG = `[phone] diff --git a/test/hook-trust.test.ts b/test/hook-trust.test.ts new file mode 100644 index 0000000..415d17c --- /dev/null +++ b/test/hook-trust.test.ts @@ -0,0 +1,343 @@ +import { describe, expect, test } from 'bun:test' +import { execSync } from 'node:child_process' +import { existsSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +const CRM_BIN = join(import.meta.dir, '..', 'src', 'cli.ts') + +/** + * Trust-on-first-use gate regression tests (see hooks.ts / trust-store.ts / + * commands/config.ts). Each test gets its own fake $HOME/%USERPROFILE% so + * the local trust ledger (~/.crm/trusted_configs.json) never touches the + * real developer machine's trust store and tests can't see each other's + * trust decisions. + */ + +function fakeHome(): string { + const home = mkdtempSync(join(tmpdir(), 'crm-trust-fakehome-')) + // Our hook commands shell out to `node`. On this machine `node` is a + // Volta shim that derives "LocalAppData" from %USERPROFILE% (not the + // LOCALAPPDATA env var) and fails hard if that subdirectory doesn't + // exist — so the fake $HOME/%USERPROFILE% used to isolate the trust + // store (~/.crm/trusted_configs.json) needs a real AppData/Local dir + // or every hook invocation errors out with "Volta error: Could not + // determine LocalAppData directory" before the trust gate is even + // reached. + mkdirSync(join(home, 'AppData', 'Local'), { recursive: true }) + return home +} + +function envFor( + home: string, + extra?: Record, +): NodeJS.ProcessEnv { + return { + ...process.env, + NO_COLOR: '1', + HOME: home, + USERPROFILE: home, + ...extra, + } +} + +function runCLI( + cwd: string, + env: NodeJS.ProcessEnv, + ...args: string[] +): { exitCode: number; stderr: string; stdout: string } { + const proc = Bun.spawnSync(['bun', 'run', CRM_BIN, ...args], { cwd, env }) + return { + exitCode: proc.exitCode, + stderr: proc.stderr.toString(), + stdout: proc.stdout.toString(), + } +} + +/** Escape a shell command for embedding as a TOML basic string value. */ +function toTOMLString(s: string): string { + return s.replace(/\\/g, '\\\\').replace(/"/g, '\\"') +} + +/** + * Write a `crm.toml` in `dir` with a `[hooks]` entry for `hookName` that + * writes a marker file (`marker`) when it runs, using `node "