From af4bad515fce81df77f7bcddebe89b163b801895 Mon Sep 17 00:00:00 2001 From: kinjo12 Date: Sat, 12 Sep 2026 11:02:01 +0900 Subject: [PATCH 1/4] fix: scope crm.toml resolution to project directory + hooks trust-on-first-use gate (#5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: scope crm.toml resolution to project directory findConfigFile previously walked from cwd all the way up to the filesystem root looking for crm.toml, and silently fell back to ~/.crm/config.toml if none was found. Since config.hooks executes shell commands without confirmation, an unrelated crm.toml placed in any ancestor directory (or a stale ~/.crm/config.toml) could lead to unintended command execution. Bound the search to the project root: walk up from cwd but stop as soon as a directory containing .git has been checked, and go no further. If no .git is found up to the filesystem root, only cwd itself is checked. Drop the implicit fallback to ~/.crm/config.toml entirely -- when no crm.toml is found within the project, the built-in default config is used instead. The auto-generated default config file is now created at the project root (or cwd, if no .git is found) rather than in the home directory. --config / CRM_CONFIG continue to be trusted unconditionally and bypass this resolution entirely. * fix: verify project root via real git repo, not a bare .git path findProjectRoot previously trusted existsSync(join(dir, '.git')) to mark the project-root boundary for crm.toml resolution. That check only verifies something named .git exists, not that it's a real repository, so an attacker-planted .git file (or directory) in an ancestor directory could smuggle a malicious crm.toml with an unconfirmed [hooks] entry into scope for any subdirectory below it, defeating the project-root boundary crm.toml resolution is meant to enforce. Shell out to `git rev-parse --show-toplevel` instead, mirroring how detectCountry() already shells out safely elsewhere in this file (no user input is interpolated into the command). If the CWD isn't inside a real git repository at all, there is no known project boundary, so config resolution now only checks the exact CWD and never walks upward toward the filesystem root. Test-only: replace the bare `.git` marker directories used to fake a project root in test/config.test.ts with real git repos via a new initGitRepo() helper, since the old fake marker no longer establishes a boundary. Add a regression test reproducing the spoofing PoC (a bare .git file plus a malicious crm.toml with a [hooks] entry above a project with no real git repo of its own). * fix: don't hard-fail the CLI when the auto-created config can't be written Auto-creating crm.toml on first run now targets the project root (previously always ~/.crm/config.toml, almost always writable). A project root can easily be read-only — CI, read-only checkouts, Docker mounts, a repo owned by another user, or a CWD with no git repo at all. createDefaultConfig() had no error handling, so a failed write propagated a raw fs error (e.g. EACCES) out of loadConfig() and hard-failed the entire command, even though defaultConfig() had already computed a perfectly usable in-memory config. Wrap the auto-create attempt in try/catch: on failure, print a warning consistent with the existing "could not parse config file" message and fall back to the in-memory default config, skipping the subsequent read/parse step since there's no file to read. * docs: correct config resolution and hooks docs for project-scoped crm.toml README.md and skills/SKILL.md still documented the old resolution order (unlimited upward directory walk, ~/.crm/config.toml global fallback) and told users to configure [hooks] in the global config file. That file is never read anymore — config resolution now walks up from CWD only as far as the real git repository root, with no global fallback, and [hooks] must live in the project's own crm.toml. * feat: add trust-on-first-use gate for implicitly-discovered crm.toml hooks Root-detection heuristics for crm.toml discovery (git rev-parse, gitlink resolution) can't fully close the ancestor-directory hook-execution risk on their own -- a crafted .git gitlink can still make an unrelated ancestor directory appear to be a real project root. Instead of continuing to patch the heuristic, gate hook execution itself: hooks defined in a crm.toml that was discovered implicitly (cwd-or-upward search) now require the user to trust that exact file (path + content hash) before they run, same as direnv/mise trust .envrc/.mise.toml. - src/trust-store.ts: local trust ledger at ~/.crm/trusted_configs.json (path -> sha256 of file bytes; content changes invalidate trust) - src/config.ts: resolveConfigPath() reports whether a resolved config came from --config/CRM_CONFIG (explicit, exempt) or discovery (implicit, gated); loadConfig() attaches this as CRMConfig._meta - src/hooks.ts: runHook() checks trust before running a hook from an implicit config -- prompts interactively (TTY) and remembers the answer, or fails closed with a warning when non-interactive, without hard-failing the CLI invocation - src/commands/config.ts: `crm config trust [path]` / `crm config untrust [path]`, defaulting to whatever config the normal resolution would load * test: add regression tests for hooks trust-on-first-use gate Covers: implicit-config hooks skipped when untrusted with no TTY (fail closed, command still completes); hooks run after `crm config trust`; re-trust required after the trusted config's content changes; explicit --config/CRM_CONFIG hooks run without any trust step; trust keyed per-path; and the gitlink-redirection PoC (an ancestor .git file pointing at a throwaway real repo, reopening the round-2 root-detection bypass) no longer executes hooks now that the gate doesn't depend on root detection. * docs: document the hooks trust-on-first-use requirement Extends the Hooks section (README) and SKILL.md with the trust gate added for implicitly-discovered crm.toml: interactive prompt-and-remember vs. fail-closed skip with a warning when non-interactive, re-trust on content change, --config/CRM_CONFIG exemption, and the `crm config trust`/`untrust` commands. --- README.md | 31 +++- skills/SKILL.md | 8 +- src/cli.ts | 2 + src/commands/config.ts | 51 ++++++ src/config.ts | 134 ++++++++++++--- src/hooks.ts | 62 +++++++ src/trust-store.ts | 104 +++++++++++ test/config.test.ts | 373 +++++++++++++++++++++++++++++++++++++++- test/helpers.ts | 11 ++ test/hook-trust.test.ts | 343 ++++++++++++++++++++++++++++++++++++ 10 files changed, 1084 insertions(+), 35 deletions(-) create mode 100644 src/commands/config.ts create mode 100644 src/trust-store.ts create mode 100644 test/hook-trust.test.ts 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/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/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/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 "