From 55b0a3e892254dc40d8124dab4efd250617a0c16 Mon Sep 17 00:00:00 2001 From: Daniel JB Clark Date: Thu, 15 Oct 2026 09:22:36 -0400 Subject: [PATCH 1/2] fix(gitignore): don't append /graft/ when git already ignores the dir (#582) ensureGitignored only read the root .gitignore, so an ignore rule that lives in .git/info/exclude, a parent .gitignore or core.excludesFile was invisible to it and the entry was re-appended on every build, dirtying a checkout whose owner keeps tooling noise out of the tracked .gitignore on purpose. Before appending, ask git: `git check-ignore -q -- /`. Exit 0 means git already ignores the dir, so return without writing. Any other outcome (not a work tree, git missing, timeout) falls through to the old behaviour. The guard sits after the existing presence check, so a legacy unanchored entry is still handled by whatever precedes it. Tests cover info/exclude, core.excludesFile, a parent .gitignore, and a git repo with no matching rule (still writes). GRAFT_NO_GITIGNORE is unchanged. Co-Authored-By: Claude Sonnet 5.5 --- src/context/node-file.ts | 30 +++++++++++++++--- test/context.test.ts | 67 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/src/context/node-file.ts b/src/context/node-file.ts index dcfbee54c..9b5d8af73 100644 --- a/src/context/node-file.ts +++ b/src/context/node-file.ts @@ -23,6 +23,7 @@ */ import { readFileSync, writeFileSync, mkdirSync, existsSync, readdirSync, rmSync } from "node:fs"; import { join } from "node:path"; +import { spawnSync } from "node:child_process"; import { matter } from "../util/front-matter.js"; import { contentHash, normalizeName } from "../util/id.js"; import { relPosix, stripTrailingSlashes } from "../util/paths.js"; @@ -115,10 +116,12 @@ export function contextDirFor(root: string, override?: string): string { * Make sure the repo's root `.gitignore` ignores the graft output dir. The * graph is a local, regenerable cache (like `node_modules`), not a committed * artifact, so every `graft build` adds the entry itself the first time — the - * user never has to think about it. No-ops when the entry is already present - * or the dir lives outside `root` (a custom `--dir` elsewhere, which can't be - * expressed as a repo-relative ignore). Best-effort: an unwritable `.gitignore` - * must never abort a build, so write failures are swallowed. + * user never has to think about it. No-ops when the entry is already present, + * when git already ignores the dir by other means (`.git/info/exclude`, a parent + * `.gitignore`, `core.excludesFile`), or when the dir lives outside `root` (a + * custom `--dir` elsewhere, which can't be expressed as a repo-relative ignore). + * Best-effort: an unwritable `.gitignore` must never abort a build, so write + * failures are swallowed. */ const GRAPH_CACHE_NOTE = "graft's local graph cache — regenerable, not committed (run `graft build`)."; @@ -146,11 +149,30 @@ export function ensureGitignored(root: string, contextDir: string, note = GRAPH_ return t === entry || t === `${bare}/` || t === bare; }); if (present) return; + // Not in `.gitignore` itself, but git may already ignore the dir through `.git/info/exclude`, + // a parent `.gitignore` or `core.excludesFile`. Then there is nothing to add, and appending + // would dirty the checkout again on every build for a user who keeps tooling noise out of + // the tracked `.gitignore` on purpose. + if (gitAlreadyIgnores(root, bare)) return; const gap = current === "" ? "" : current.endsWith("\n") ? "\n" : "\n\n"; const block = `${gap}# ${note}\n${entry}\n`; try { writeFileSync(path, current + block); } catch { /* best-effort — build already succeeded */ } } +/** True when `root` is a git work tree whose ignore rules (every source git reads: `.gitignore` + * files, `.git/info/exclude`, `core.excludesFile`) already cover the directory `rel/`. Any + * failure (git missing, not a repo, timeout) answers false, so the caller falls back to + * writing the entry. The trailing slash makes git treat a not-yet-created path as a directory, + * so dir-only patterns like `graft/` match. */ +function gitAlreadyIgnores(root: string, rel: string): boolean { + try { + const r = spawnSync("git", ["check-ignore", "-q", "--", `${rel}/`], { + cwd: root, stdio: "ignore", timeout: 5000, windowsHide: true, + }); + return r.status === 0; + } catch { return false; } +} + /** * Keep the card tree greppable even though it's gitignored, by writing a root * `.ignore` that re-admits it. diff --git a/test/context.test.ts b/test/context.test.ts index cf875040f..31ae7ad10 100644 --- a/test/context.test.ts +++ b/test/context.test.ts @@ -527,6 +527,73 @@ test("ensureGitignored: GRAFT_NO_GITIGNORE=1 skips writing .gitignore", () => { } }); +// A git-ignored context dir needs no `.gitignore` entry, whichever file git reads the rule from. +function gitInit(dir: string): void { + execFileSync("git", ["init", "-q"], { cwd: dir, stdio: "ignore" }); +} + +test("ensureGitignored: leaves .gitignore alone when .git/info/exclude already ignores the graph dir", () => { + const dir = mkdtempSync(join(tmpdir(), "ctxgi-")); + try { + gitInit(dir); + writeFileSync(join(dir, ".git", "info", "exclude"), "/graft/\n"); + writeFileSync(join(dir, ".gitignore"), "node_modules/\n"); + ensureGitignored(dir, contextDirFor(dir)); + assert.equal(readFileSync(join(dir, ".gitignore"), "utf8"), "node_modules/\n"); + const fresh = mkdtempSync(join(tmpdir(), "ctxgi-")); + try { + gitInit(fresh); + writeFileSync(join(fresh, ".git", "info", "exclude"), "/graft/\n"); + ensureGitignored(fresh, contextDirFor(fresh)); + assert.equal(existsSync(join(fresh, ".gitignore")), false, "must not create .gitignore either"); + } finally { + rmSync(fresh, { recursive: true, force: true }); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("ensureGitignored: leaves .gitignore alone when core.excludesFile already ignores the graph dir", () => { + const dir = mkdtempSync(join(tmpdir(), "ctxgi-")); + try { + gitInit(dir); + const globalIgnore = join(dir, ".git", "global-ignore"); + writeFileSync(globalIgnore, "graft/\n"); + execFileSync("git", ["config", "core.excludesFile", globalIgnore], { cwd: dir, stdio: "ignore" }); + ensureGitignored(dir, contextDirFor(dir)); + assert.equal(existsSync(join(dir, ".gitignore")), false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("ensureGitignored: leaves .gitignore alone when a parent .gitignore already ignores the graph dir", () => { + const dir = mkdtempSync(join(tmpdir(), "ctxgi-")); + try { + gitInit(dir); + writeFileSync(join(dir, ".gitignore"), "graft/\n"); + const sub = join(dir, "pkg"); + mkdirSync(sub); + ensureGitignored(sub, contextDirFor(sub)); + assert.equal(existsSync(join(sub, ".gitignore")), false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("ensureGitignored: still writes the entry in a git repo that does not ignore the graph dir", () => { + const dir = mkdtempSync(join(tmpdir(), "ctxgi-")); + try { + gitInit(dir); + writeFileSync(join(dir, ".git", "info", "exclude"), "/something-else/\n"); + ensureGitignored(dir, contextDirFor(dir)); + assert.match(readFileSync(join(dir, ".gitignore"), "utf8"), /^\/graft\/$/m); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("ensureSearchable: GRAFT_NO_IGNORE=1 skips writing .ignore", () => { const dir = mkdtempSync(join(tmpdir(), "ctxsearch-")); process.env.GRAFT_NO_IGNORE = "1"; From 52fb22d919e13c2ed0f7c2949580aa5b141b7f48 Mon Sep 17 00:00:00 2001 From: Daniel JB Clark Date: Thu, 15 Oct 2026 09:32:42 -0400 Subject: [PATCH 2/2] fix(gitignore): harden the check-ignore guard (review follow-ups) Follow-up to the git check-ignore guard in ensureGitignored, from a security review of it. - Run git with GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, GIT_COMMON_DIR, GIT_OBJECT_DIRECTORY and GIT_NAMESPACE removed from its environment. A git hook exports GIT_DIR and GIT_INDEX_FILE, and with them check-ignore answered for another repo, so the entry was skipped in the one being built. - Pass `-c core.fsmonitor=false`, so the call never runs a command named by the repo's own config. - Make the ensureGitignored tests independent of the developer's environment: empty global git config, no default excludes file, GIT_CONFIG_NOSYSTEM, GIT_CEILING_DIRECTORIES above the temp root, and GRAFT_NO_GITIGNORE and GRAFT_NO_IGNORE unset. A global excludes file, a TMPDIR inside a repo or an exported GRAFT_NO_GITIGNORE used to fail them. New test for an inherited GIT_DIR. - Correct the helper's comment about the trailing slash: it makes dir-only patterns match, but negation rules can judge a not-yet-created path differently, so callers create the dir first. Co-Authored-By: Claude Opus 5.5 --- src/context/node-file.ts | 20 +++++++++++++---- test/context.test.ts | 47 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/context/node-file.ts b/src/context/node-file.ts index 9b5d8af73..42b1d4b09 100644 --- a/src/context/node-file.ts +++ b/src/context/node-file.ts @@ -159,15 +159,27 @@ export function ensureGitignored(root: string, contextDir: string, note = GRAPH_ try { writeFileSync(path, current + block); } catch { /* best-effort — build already succeeded */ } } +/** Variables that point git at a different repo than the one in `cwd` (a git hook exports + * `GIT_DIR` and `GIT_INDEX_FILE`, a wrapper may export the rest). Left in the child's + * environment they make `check-ignore` answer for that other repo. */ +const GIT_REPO_ENV = [ + "GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR", "GIT_OBJECT_DIRECTORY", "GIT_NAMESPACE", +]; + /** True when `root` is a git work tree whose ignore rules (every source git reads: `.gitignore` * files, `.git/info/exclude`, `core.excludesFile`) already cover the directory `rel/`. Any * failure (git missing, not a repo, timeout) answers false, so the caller falls back to - * writing the entry. The trailing slash makes git treat a not-yet-created path as a directory, - * so dir-only patterns like `graft/` match. */ + * writing the entry. The trailing slash makes git treat the path as a directory, so dir-only + * patterns like `graft/` match. A negation rule (an allowlist `.gitignore` that re-includes + * directories) can judge a not-yet-created path differently from an existing one, so callers + * create the dir first. `core.fsmonitor` is off because it would run a command named by the + * repo's own config. */ function gitAlreadyIgnores(root: string, rel: string): boolean { try { - const r = spawnSync("git", ["check-ignore", "-q", "--", `${rel}/`], { - cwd: root, stdio: "ignore", timeout: 5000, windowsHide: true, + const env = { ...process.env }; + for (const k of GIT_REPO_ENV) delete env[k]; + const r = spawnSync("git", ["-c", "core.fsmonitor=false", "check-ignore", "-q", "--", `${rel}/`], { + cwd: root, env, stdio: "ignore", timeout: 5000, windowsHide: true, }); return r.status === 0; } catch { return false; } diff --git a/test/context.test.ts b/test/context.test.ts index 31ae7ad10..204d522ae 100644 --- a/test/context.test.ts +++ b/test/context.test.ts @@ -2,10 +2,10 @@ * End-to-end tests for the markdown-graph pipeline (`init` → `check`), driven by * offline test doubles so no LLM/network is needed. */ -import { test } from "node:test"; +import { test, beforeEach, afterEach } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync } from "node:fs"; -import { join } from "node:path"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, realpathSync, rmSync } from "node:fs"; +import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; import { execFileSync } from "node:child_process"; import { buildContext } from "../src/context/build.js"; @@ -423,6 +423,31 @@ test("graft check: keyless build then code changes (wiring stale) exits 1", () = } }); +// The ensureGitignored tests ask real git whether a path is ignored, so they must not see the +// developer's environment: a global excludes file, a TMPDIR inside a repo, GIT_DIR exported by +// a hook, or GRAFT_NO_GITIGNORE would each change the answer. Pin all of it for every test +// in this file and restore it afterwards. +const ISOLATED_ENV = [ + "GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_CONFIG_GLOBAL", "GIT_CONFIG_NOSYSTEM", + "GIT_CEILING_DIRECTORIES", "XDG_CONFIG_HOME", "GRAFT_NO_GITIGNORE", "GRAFT_NO_IGNORE", +]; +let savedEnv: Record = {}; +beforeEach(() => { + savedEnv = Object.fromEntries(ISOLATED_ENV.map((k) => [k, process.env[k]])); + for (const k of ["GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GRAFT_NO_GITIGNORE", "GRAFT_NO_IGNORE"]) delete process.env[k]; + // Missing files read as empty config / no default excludes file. + process.env.GIT_CONFIG_GLOBAL = join(tmpdir(), "ctxgi-no-global-config"); + process.env.XDG_CONFIG_HOME = join(tmpdir(), "ctxgi-no-xdg-config"); + process.env.GIT_CONFIG_NOSYSTEM = "1"; + // Stop git's repo discovery above the temp root, so a scratch dir never finds an outer repo. + process.env.GIT_CEILING_DIRECTORIES = dirname(realpathSync(tmpdir())); +}); +afterEach(() => { + for (const [k, v] of Object.entries(savedEnv)) { + if (v === undefined) delete process.env[k]; else process.env[k] = v; + } +}); + // ensureGitignored — every `graft build` self-ignores its regenerable graph dir. test("ensureGitignored: creates .gitignore with the graft/ entry when none exists", () => { const dir = mkdtempSync(join(tmpdir(), "ctxgi-")); @@ -594,6 +619,22 @@ test("ensureGitignored: still writes the entry in a git repo that does not ignor } }); +test("ensureGitignored: an inherited GIT_DIR does not make git answer for another repo", () => { + const dir = mkdtempSync(join(tmpdir(), "ctxgi-")); + const other = mkdtempSync(join(tmpdir(), "ctxgi-")); + try { + gitInit(dir); + gitInit(other); + writeFileSync(join(other, ".git", "info", "exclude"), "/graft/\n"); + process.env.GIT_DIR = join(other, ".git"); // what a git hook exports; restored by afterEach + ensureGitignored(dir, contextDirFor(dir)); + assert.match(readFileSync(join(dir, ".gitignore"), "utf8"), /^\/graft\/$/m); + } finally { + rmSync(dir, { recursive: true, force: true }); + rmSync(other, { recursive: true, force: true }); + } +}); + test("ensureSearchable: GRAFT_NO_IGNORE=1 skips writing .ignore", () => { const dir = mkdtempSync(join(tmpdir(), "ctxsearch-")); process.env.GRAFT_NO_IGNORE = "1";