Skip to content

fix: reject an existing bare repository as an init destination - #29

Open
ashproto wants to merge 1 commit into
nextfrom
fix/detect-bare-repo-destination
Open

fix: reject an existing bare repository as an init destination#29
ashproto wants to merge 1 commit into
nextfrom
fix/detect-bare-repo-destination

Conversation

@ashproto

@ashproto ashproto commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Reported by Codex review on #26 as P2. The code is not part of that PR — crates/git-core/src/git_ops.rs is untouched by #26, and the guard came from 15e51c3 (2026-07-17) — so it is fixed here on its own branch, same as #27.

The gap

initialize_repository guards against an existing repository two ways, and a bare one slips between them:

guard bare repo
destination.join(".git").exists() false — a bare repo has no .git child
is_inside_worktree(destination) falserev-parse --is-inside-work-tree says false inside a bare repo

The destination is non-empty, so the user gets the "folder is not empty" prompt rather than "already a Git repository". On confirming, git init runs with the bare repository as its working directory.

What actually happens

Verified against real git rather than assumed — this is the exact command git_ops.rs issues, cwd set to a bare repo holding a real branch:

$ (cd bare.git && git init --initial-branch=newbranch)
Initialized empty Git repository in …/bare.git/.git/

nested .git created?  YES
bare HEAD after:      refs/heads/main     (unchanged)
refs still present:   refs/heads/trunk    (unchanged)
core.bare after:      true                (unchanged)

$ diff -rq bare-backup.git bare.git
Only in bare.git: .git

So it is a wrong state, not data loss — the bare repo survives intact and the only change is the nested .git. Worth stating plainly since "mutates a directory that should have been rejected" could read as destructive. It is still exactly what the sibling guard exists to prevent, and the app then reports success and opens the nested repo.

The fix

Probe the destination itself with rev-parse --resolve-git-dir:

if destination.join(".git").exists() || is_git_dir(&destination) {
    return Err("That folder is already a Git repository. Open it instead.".to_string());
}

I characterised the probe against every shape before picking it, because the risk is over-reach — refusing a legitimate destination:

path --resolve-git-dir
bare repo dir OK ← the case being fixed
normal repo .git OK
normal repo root fail
plain empty dir fail
subdir inside a normal repo fail ← does not walk up
nonexistent path fail

That last row is the important one: --resolve-git-dir answers for the path given, so a plain folder that merely sits inside a repository is still a valid destination. Nesting remains is_inside_worktree's job, and a test now pins that boundary.

The .git check is kept alongside rather than replaced, so nothing refused before is accepted now.

The operand is always absolute (parent is fs::canonicalized before the join), so it cannot be read as a flag — noted in the doc comment since the project's shell-out rule would otherwise want a --, which --resolve-git-dir cannot take because it consumes the next argument.

Tests (TDD)

  • initialize_repository_rejects_an_existing_bare_repository — watched fail first, returning Ok(initialized: true, existing_entries: 7). Asserts the error and that no nested .git was created.
  • initialize_repository_still_accepts_a_plain_empty_folder — the over-reach guard. Passed before the change and after.

Both run with cwd inside a repository (cargo's working dir), so the "already inside a repo" case is exercised naturally.

Gates: cargo test full workspace green (git-core 185) · npm run check 524 files / 0 errors · npm test 330 passed.

🤖 Generated with Claude Code

`initialize_repository` guarded against an existing repository two ways, and a
bare one slipped between them: it has no `.git` child, and `rev-parse
--is-inside-work-tree` answers `false` inside it. The destination is non-empty,
so the user got the "folder is not empty" prompt rather than "already a Git
repository" — and on confirming, `git init` ran with the bare repo as its
working directory.

Verified against real git: it prints "Initialized empty Git repository in
.../bare.git/.git/" and creates a nested repository inside the bare one. The
bare repo's own HEAD, refs and core.bare survive untouched — `diff -rq` against
a backup shows only the added `.git` — so this is a wrong state rather than
data loss, but it is exactly what the sibling guard exists to prevent.

Probe the destination itself with `rev-parse --resolve-git-dir`. It answers for
the path GIVEN and does not walk up to a parent, so a plain folder that merely
sits inside a repository is still a valid destination — that case belongs to
is_inside_worktree, and a test now pins it. The `.git` check is kept alongside
rather than replaced, so nothing that was refused before is accepted now.

Reported by Codex review on #26; the code is not part of that PR — it came from
15e51c3 (2026-07-17) and git_ops.rs is untouched there — so it is fixed here.

cargo test full workspace green (git-core 185); npm run check 524 files
0 errors; npm test 330 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

&parent
};
if is_inside_worktree(nesting_probe) {

P2 Badge Reject Git directories used as repository parents

When the selected parent is itself a bare repository or a normal repository's .git directory and the requested child does not exist, nesting_probe becomes parent, but --is-inside-work-tree returns false in Git directories. The flow consequently creates the new repository inside the existing repository's metadata directory; check is_git_dir(&parent) as well so these parent selections are rejected before filesystem mutation.


"vite": "^8.1.4",

P2 Badge Declare the Node runtime required by Vite 8

The upgrade to Vite 8 raises the runtime requirement to Node ^20.19.0 || >=22.12.0 (recorded in package-lock.json), while package.json has no engines field and the build-from-source instructions specify no Node version. Contributors following those instructions with Node 18 or early Node 20 can install with only engine warnings and then fail when running the documented build commands; declare the requirement or retain a compatible toolchain.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant