fix: git-ignore-block.sh writes an exclude target only inside a real git dir (#431)

.claude/.idd/attachments/issue-431/_manifest.json

@@ -0,0 +1,6 @@
+{
+  "issue": 431,
+  "fetched_at": "2026-10-29T21:16:48Z",
+  "fetched_by": "idd-diagnose",
+  "files": []
+}

plugins/issue-driven-dev/CHANGELOG.md

@@ -5,6 +5,22 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+Version number left to the merge, as in PR #442.
+
+### Fixed
+
+- **`git-ignore-block.sh` writes an `exclude` target only inside a real git dir (#431).** The #194 allowlist
+  checked the basename alone, so any `<dir>/exclude` passed and `mkdir -p` created its parent directories —
+  including `/info/exclude`, which the callers' `"$(git rev-parse --git-dir)/info/exclude"` becomes outside any
+  repository. An `exclude` target must now be `<D>/info/exclude` where `<D>` is a directory and
+  `git --git-dir=<D> rev-parse --git-dir` succeeds; a `--target` containing a control character is refused first
+  (`$(dirname …)` strips trailing newlines, so the check could otherwise test one path and write another).
+  Anything else exits 2 before any write. Relative `.git/info/exclude`, linked-worktree git dirs and bare repos
+  are still accepted (a linked worktree's own `info/exclude` is not read by git — #446). Symlinked targets, the
+  `.gitignore` branch's any-directory rule and unchecked write failures are #447.
+
 ## [3.5.0] - 2026-10-20
 
 ### Added

plugins/issue-driven-dev/scripts/git-ignore-block.sh

@@ -51,15 +51,37 @@ case "$DIRECTION" in
   *) echo "✗ git-ignore-block: --direction must be exclude|re-include (got '${DIRECTION}')" >&2; exit 2 ;;
 esac
 
+# A control character in --target is refused before anything else: `$(dirname …)`
+# strips trailing newlines, so the allowlist below could check one path while the
+# write lands on another (#431 verify).
+case "$TARGET" in
+  *[[:cntrl:]]*) echo "✗ git-ignore-block: --target contains a control character (refusing to write)." >&2; exit 2 ;;
+esac
+
 # Target allowlist (#194) — the helper only ever writes git ignore files. Reject
 # any other --target so a future caller passing untrusted input can't write to an
 # executable path like .git/hooks/pre-commit (which git RUNS). The legitimate
-# targets are `.gitignore` (any dir) and `.git/info/exclude` (basename `exclude`).
+# targets are `.gitignore` (any dir) and `<git-dir>/info/exclude`.
 TARGET_BASE="$(basename "$TARGET")"
 case "$TARGET_BASE" in
   .gitignore|exclude) ;;
   *) echo "✗ git-ignore-block: --target basename '$TARGET_BASE' not allowed — only '.gitignore' or 'exclude' (refusing to write outside git ignore files)." >&2; exit 2 ;;
 esac
+# An `exclude` target must sit in info/ under a real git dir (#431). The basename
+# alone let any <dir>/exclude through — including /info/exclude, which callers
+# produce when `git rev-parse --git-dir` prints nothing outside any repository —
+# and the mkdir -p below then created its parent directories. The git dir must be
+# a directory: `git --git-dir` also accepts a .git FILE (linked worktree,
+# submodule) by following its gitdir: line, and nothing can be written under it.
+if [ "$TARGET_BASE" = exclude ]; then
+  TARGET_INFO="$(dirname "$TARGET")"
+  TARGET_GIT_DIR="$(dirname "$TARGET_INFO")"
+  if [ "$(basename "$TARGET_INFO")" != info ] || [ ! -d "$TARGET_GIT_DIR" ] \
+     || ! git --git-dir="$TARGET_GIT_DIR" rev-parse --git-dir >/dev/null 2>&1; then
+    echo "✗ git-ignore-block: --target '$TARGET' is not <git-dir>/info/exclude of a git repository (refusing to write)." >&2
+    exit 2
+  fi
+fi
 
 # --- Build block body ---------------------------------------------------------
 BODY=()

plugins/issue-driven-dev/scripts/tests/git-ignore-block/test.sh

@@ -14,39 +14,45 @@ SCRIPT="$HERE/../../git-ignore-block.sh"
 
 . "$HERE/../../lib/assert-helpers.sh"
 
+# Every throwaway repo lives under one root, so the EXIT trap removes all of them
+# (a list appended inside "$(mk)" was lost with the subshell). A repo that cannot
+# be created stops the run: with R="" every "$R/..." below is a path at the
+# filesystem root (#431 verify).
+TROOT="$(mktemp -d)" || { echo "FATAL: mktemp -d failed" >&2; exit 1; }
+trap 'rm -rf "$TROOT"' EXIT
 new_repo() {
   local d
-  d="$(mktemp -d)"
-  git -C "$d" init -b main -q
+  d="$(mktemp -d "$TROOT/repo.XXXXXX")" || return 1
+  git -C "$d" init -b main -q || return 1
   git -C "$d" config user.email t@t.t
   git -C "$d" config user.name t
   git -C "$d" rev-parse --show-toplevel
 }
-
-TMPDIRS=()
-mk() { local r; r="$(new_repo)"; TMPDIRS+=("$r"); printf '%s' "$r"; }
-cleanup() { for d in "${TMPDIRS[@]:-}"; do [ -n "${d:-}" ] && rm -rf "$d"; done; }
-trap cleanup EXIT
+mk() {
+  local r
+  r="$(new_repo)" && [ -n "$r" ] || { echo "FATAL: could not create a test repo under $TROOT" >&2; return 1; }
+  printf '%s' "$r"
+}
 
 MARKER="IDD test block (#192)"
 BEGINLINE="# >>> ${MARKER} >>>"
 
 # --- Test 1: exclude direction → .git/info/exclude ignores the path ---------
-R="$(mk)"
+R="$(mk)" || exit 1
 bash "$SCRIPT" --target "$R/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/"
 assert_exit "exclude exits 0" 0 $?
 require "exclude: .claude/.idd/local.json now ignored" git -C "$R" check-ignore -q .claude/.idd/local.json
 refute  "exclude: unrelated path not ignored"          git -C "$R" check-ignore -q src/main.py
 
 # --- Test 2: idempotent re-run (same marker+body) → one block ---------------
-R="$(mk)"
+R="$(mk)" || exit 1
 bash "$SCRIPT" --target "$R/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/"
 bash "$SCRIPT" --target "$R/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/"
 CNT=$(grep -cF "$BEGINLINE" "$R/.git/info/exclude")
 assert_eq "idempotent: block begin-sentinel appears once" "1" "$CNT"
 
 # --- Test 3: stale block upgrade-in-place (same marker, different body) ------
-R="$(mk)"
+R="$(mk)" || exit 1
 bash "$SCRIPT" --target "$R/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/old/"
 bash "$SCRIPT" --target "$R/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/"
 MCNT=$(grep -cF "$BEGINLINE" "$R/.git/info/exclude")
@@ -55,15 +61,15 @@ require "stale: new pattern ignored" git -C "$R" check-ignore -q .claude/.idd/x
 refute  "stale: old pattern gone"    git -C "$R" check-ignore -q .claude/old/x
 
 # --- Test 4: adjacent user content preserved --------------------------------
-R="$(mk)"
+R="$(mk)" || exit 1
 printf '# user header\n*.log\n' > "$R/.git/info/exclude"
 bash "$SCRIPT" --target "$R/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/"
 assert_grep "preserve: user header kept" "# user header" "$(cat "$R/.git/info/exclude")"
 require "preserve: user *.log still effective" git -C "$R" check-ignore -q debug.log
 require "preserve: new exclude effective"      git -C "$R" check-ignore -q .claude/.idd/y
 
 # --- Test 5: re-include direction → parent-dir-excluded carve-out trackable --
-R="$(mk)"
+R="$(mk)" || exit 1
 printf '.claude/\n' > "$R/.gitignore"
 require "re-include precheck: path ignored before carve-out" git -C "$R" check-ignore -q .claude/.idd/issue-runs/x.jsonl
 bash "$SCRIPT" --target "$R/.gitignore" --marker "$MARKER" --direction re-include ".claude/.idd/issue-runs"
@@ -81,7 +87,7 @@ bash "$SCRIPT" --target 2>/dev/null   # valueless option must not hang/loop
 assert_exit "valueless --target exits 2 (no infinite loop)" 2 $?
 
 # --- Test 7: missing-END corruption → abort, surrounding content preserved ----
-R="$(mk)"
+R="$(mk)" || exit 1
 EXC="$R/.git/info/exclude"
 mkdir -p "$(dirname "$EXC")"
 printf '# >>> %s >>>\n.claude/old/\n# user line AFTER a corrupted (END-less) block\nsecrets/\n' "$MARKER" > "$EXC"
@@ -91,7 +97,7 @@ assert_grep "missing-END: trailing user line preserved" "user line AFTER" "$(cat
 assert_grep "missing-END: secrets/ line preserved" "secrets/" "$(cat "$EXC")"
 
 # --- Test 8: re-include idempotency (run twice → one block) -------------------
-R="$(mk)"
+R="$(mk)" || exit 1
 printf '.claude/\n' > "$R/.gitignore"
 bash "$SCRIPT" --target "$R/.gitignore" --marker "$MARKER" --direction re-include ".claude/.idd/issue-runs"
 bash "$SCRIPT" --target "$R/.gitignore" --marker "$MARKER" --direction re-include ".claude/.idd/issue-runs"
@@ -105,10 +111,63 @@ assert_exit "#194: .git/hooks/pre-commit target rejected → exit 2" 2 $?
 bash "$SCRIPT" --target /tmp/idd-hax/random.txt --marker m --direction exclude ".x" 2>/dev/null
 assert_exit "#194: arbitrary basename rejected → exit 2" 2 $?
 # allowed basenames still work
-RA="$(mk)"
+RA="$(mk)" || exit 1
 bash "$SCRIPT" --target "$RA/.gitignore" --marker "$MARKER" --direction exclude ".claude/.idd/"
 assert_exit "#194: .gitignore basename allowed → exit 0" 0 $?
 bash "$SCRIPT" --target "$RA/.git/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/"
 assert_exit "#194: exclude basename allowed → exit 0" 0 $?
 
+# --- Test 10: an `exclude` target must be <git-dir>/info/exclude of a real repo (#431)
+# Outside a work tree `git rev-parse --git-dir` prints nothing, so the callers'
+# "$(git rev-parse --git-dir)/info/exclude" becomes /info/exclude. That case runs
+# only where /info does not exist; a helper that still writes it fails here and
+# leaves /info/exclude for the person running the tests to remove. If the file
+# there holds exactly this test's own block (an earlier run of a broken helper
+# wrote it), the case still runs — it cannot touch anything else — and the file
+# must come out unchanged. Any other /info is reported, not silently dropped.
+if [ ! -e /info ]; then
+  bash "$SCRIPT" --target "/info/exclude" --marker m --direction exclude ".x" 2>/dev/null
+  assert_exit "#431: empty git-dir (/info/exclude) rejected → exit 2" 2 $?
+  assert_file_absent "#431: /info/exclude not created" /info/exclude
+elif [ "$(cat /info/exclude 2>/dev/null)" = "$(printf '# >>> m >>>\n.x\n# <<< m <<<')" ]; then
+  BEFORE="$(cksum < /info/exclude)"
+  bash "$SCRIPT" --target "/info/exclude" --marker m --direction exclude ".x" 2>/dev/null
+  assert_exit "#431: empty git-dir (/info/exclude) rejected → exit 2" 2 $?
+  assert_eq "#431: leftover /info/exclude unchanged" "$BEFORE" "$(cksum < /info/exclude)"
+else
+  echo "  SKIP #431: empty git-dir (/info/exclude) — /info exists on this host"
+fi
+# A trailing newline in a path component is stripped by $(dirname …), so the
+# probe would check a real git dir while the write lands next to it.
+RN="$(mk)" || exit 1
+bash "$SCRIPT" --target "$RN/.git"$'\n'"/info/exclude" --marker m --direction exclude ".x" 2>/dev/null
+assert_exit "#431: newline in the git-dir component rejected → exit 2" 2 $?
+assert_file_absent "#431: no '.git<newline>' dir created" "$RN/.git"$'\n'
+bash "$SCRIPT" --target "$RN/.git/info/exclude"$'\n' --marker m --direction exclude ".x" 2>/dev/null
+assert_exit "#431: trailing newline on the target rejected → exit 2" 2 $?
+assert_file_absent "#431: no 'exclude<newline>' file created" "$RN/.git/info/exclude"$'\n'
+# A .git FILE (linked worktree, submodule) passes `git --git-dir`, which follows
+# its gitdir: line, but it is not a directory the helper can write under.
+RW="$(mk)" || exit 1
+git -C "$RW" commit -q --allow-empty -m init
+git -C "$RW" worktree add -q "$TROOT/wt-gitfile" 2>/dev/null
+bash "$SCRIPT" --target "$TROOT/wt-gitfile/.git/info/exclude" --marker m --direction exclude ".x" 2>/dev/null
+assert_exit "#431: a .git file (gitfile) as the git dir rejected → exit 2" 2 $?
+NG="$(mktemp -d "$TROOT/nongit.XXXXXX")" || exit 1
+bash "$SCRIPT" --target "$NG/info/exclude" --marker m --direction exclude ".x" 2>/dev/null
+assert_exit "#431: info/exclude under a non-git dir rejected → exit 2" 2 $?
+assert_file_absent "#431: non-git info/ not created" "$NG/info"
+RB="$(mk)" || exit 1
+bash "$SCRIPT" --target "$RB/.git/hooks/exclude" --marker m --direction exclude ".x" 2>/dev/null
+assert_exit "#431: <git-dir>/hooks/exclude (not info/) rejected → exit 2" 2 $?
+assert_file_absent "#431: hooks/exclude not created" "$RB/.git/hooks/exclude"
+bash "$SCRIPT" --target "$NG/some/where/exclude" --marker m --direction exclude ".x" 2>/dev/null
+assert_exit "#431: arbitrary <dir>/exclude rejected → exit 2" 2 $?
+assert_file_absent "#431: arbitrary parent dir not created" "$NG/some"
+# the form the callers pass from a repo's top level is relative: .git/info/exclude
+RR="$(mk)" || exit 1
+( cd "$RR" && bash "$SCRIPT" --target "$(git rev-parse --git-dir)/info/exclude" --marker "$MARKER" --direction exclude ".claude/.idd/" )
+assert_exit "#431: relative .git/info/exclude from the top level → exit 0" 0 $?
+require "#431: relative target is effective" git -C "$RR" check-ignore -q .claude/.idd/local.json
+
 print_summary "git-ignore-block"