Skip to content

Commit 6d60593

Browse files
committed
refactor(gitrepo): route all repo opens through gitrepo for reftable safety
checkpoint/remote's openRepoAt opened repositories with git.PlainOpenWithOptions directly, bypassing the reftable-aware opener. On a reftable repository that call fails with 'unknown extension: refstorage', so provider checkpoint-URL resolution silently lost the ability to reuse an existing remote's scheme. Route openRepoAt through gitrepo.OpenCurrent/OpenPath (threading ctx) so every repo open goes through the single reftable- and alternates-aware entry point. Detect reftable stacks with os.Lstat instead of os.Stat so a symlink in place of the reftable/ directory is not mistaken for a genuine stack. Document in CLAUDE.md that gitrepo is the sole sanctioned opener and no code may call git.Open/PlainOpen directly. Add TODOs to drop the reftableStorer shim once go-git ships native reftable support. Assisted-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io> Entire-Checkpoint: 01KYM8EC609H2J2N4S0KYA95TA
1 parent 8571f81 commit 6d60593

4 files changed

Lines changed: 59 additions & 10 deletions

File tree

CLAUDE.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,31 @@ Don't use `fmt.Print*` for operational messages (checkpoint saves, hook invocati
460460

461461
We use github.com/go-git/go-git for most git operations, but with important exceptions:
462462

463+
#### Opening Repositories - Always Use `gitrepo`
464+
465+
**Never call `git.Open`, `git.PlainOpen`, or `git.PlainOpenWithOptions` directly.
466+
`cmd/entire/cli/gitrepo` is the single source of truth for opening a
467+
repository.** Use `gitrepo.OpenCurrent(ctx)` for the current worktree or
468+
`gitrepo.OpenPath(root)` for a specific worktree root. Both funnel through
469+
`openPathWithAlternates`, which is the only place that opens a `*git.Repository`.
470+
471+
Routing every open through `gitrepo` guarantees two behaviours no ad-hoc
472+
`git.PlainOpen` call gets right:
473+
474+
- **Object alternates** are rewritten to absolute paths so shared clones resolve
475+
their objects (`PlainOpen` cannot follow relative/absolute alternates).
476+
- **Reftable repositories** are detected and opened through the git-CLI-backed
477+
reference storer (`reftableStorer`). A direct `git.PlainOpen` on a reftable
478+
repo fails outright with `unknown extension: refstorage`, because go-git's
479+
filesystem storer cannot read the reftable backend. The reftable storer also
480+
re-approves the `objectformat` (sha1/sha256) and `worktreeconfig` extensions
481+
that go-git verifies at open time.
482+
483+
If a code path opens a repo with a bare go-git call, it silently breaks on
484+
reftable and sha256 repositories. Reviewers should flag any new
485+
`git.PlainOpen*`/`git.Open` outside `gitrepo`. Key files: `gitrepo/repository.go`
486+
(open entry points) and `gitrepo/reftable.go` (`reftableStorer`).
487+
463488
#### go-git v5 Bugs - Use CLI Instead
464489

465490
**Do NOT use go-git v5 for `checkout` or `reset --hard` operations.**

cmd/entire/cli/checkpoint/remote/util.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010

1111
"github.com/entireio/cli/cmd/entire/cli/gitremote"
12+
"github.com/entireio/cli/cmd/entire/cli/gitrepo"
1213
"github.com/entireio/cli/cmd/entire/cli/logging"
1314
"github.com/entireio/cli/cmd/entire/cli/settings"
1415

@@ -117,7 +118,7 @@ func FetchURL(ctx context.Context, opts ...FetchURLOptions) (string, error) {
117118
// or an entire:// mirror of a different forge than the configured
118119
// provider). Honor the configured checkpoint_remote by targeting the
119120
// provider's canonical host over HTTPS rather than falling back to origin.
120-
if providerURL, ok := resolveProviderCheckpointURL(config, opt.WorktreeRoot); ok {
121+
if providerURL, ok := resolveProviderCheckpointURL(ctx, config, opt.WorktreeRoot); ok {
121122
return providerURL, nil
122123
}
123124
logFallback(ctx, "fetch", originURL, "derive checkpoint remote URL", err)
@@ -223,7 +224,7 @@ func PushURL(ctx context.Context, pushRemoteName string) (string, bool, error) {
223224
// The checkpoint token is an HTTPS credential for the provider host;
224225
// it can't ride through the entire:// helper (which does its own
225226
// auth). Route to the provider over HTTPS instead of the mirror.
226-
if providerURL, ok := resolveProviderCheckpointURL(config, ""); ok {
227+
if providerURL, ok := resolveProviderCheckpointURL(ctx, config, ""); ok {
227228
return providerURL, true, nil
228229
}
229230
}
@@ -235,7 +236,7 @@ func PushURL(ctx context.Context, pushRemoteName string) (string, bool, error) {
235236
// configured provider). Honor the configured checkpoint_remote by
236237
// targeting the provider's canonical host over HTTPS rather than
237238
// misrouting checkpoints to the origin remote.
238-
if providerURL, ok := resolveProviderCheckpointURL(config, ""); ok {
239+
if providerURL, ok := resolveProviderCheckpointURL(ctx, config, ""); ok {
239240
return providerURL, true, nil
240241
}
241242
fallbackURL, fallbackErr := resolvePushFallbackURL(ctx, pushRemoteName, originURL)
@@ -340,8 +341,8 @@ func deriveCheckpointURLFromInfo(info *Info, config *settings.CheckpointRemoteCo
340341
//
341342
// Returns ok=false when no transport can be determined (unknown provider with no
342343
// usable signal), in which case the caller falls back to the origin remote.
343-
func resolveProviderCheckpointURL(config *settings.CheckpointRemoteConfig, dir string) (string, bool) {
344-
repo, err := openRepoAt(dir)
344+
func resolveProviderCheckpointURL(ctx context.Context, config *settings.CheckpointRemoteConfig, dir string) (string, bool) {
345+
repo, err := openRepoAt(ctx, dir)
345346
if err != nil {
346347
repo = nil // Fall back to env/provider-only signals.
347348
}
@@ -384,12 +385,20 @@ func pickProviderTransport(repo *git.Repository, config *settings.CheckpointRemo
384385
}
385386

386387
// openRepoAt opens the git repository at dir (current directory when dir is
387-
// empty), walking up to the enclosing .git directory.
388-
func openRepoAt(dir string) (*git.Repository, error) {
388+
// empty). It routes through gitrepo, the single reftable-aware opener, so a
389+
// reftable repository is opened via the git-CLI storer rather than rejected by
390+
// go-git's extension check. gitrepo.OpenCurrent resolves the worktree root from
391+
// the current directory (the walk-up equivalent of the previous DetectDotGit
392+
// open) when no explicit root is given.
393+
func openRepoAt(ctx context.Context, dir string) (*git.Repository, error) {
389394
if dir == "" {
390-
dir = "."
395+
repo, err := gitrepo.OpenCurrent(ctx)
396+
if err != nil {
397+
return nil, fmt.Errorf("open git repository: %w", err)
398+
}
399+
return repo, nil
391400
}
392-
repo, err := git.PlainOpenWithOptions(dir, &git.PlainOpenOptions{DetectDotGit: true})
401+
repo, err := gitrepo.OpenPath(dir)
393402
if err != nil {
394403
return nil, fmt.Errorf("open git repository: %w", err)
395404
}

cmd/entire/cli/gitrepo/reftable.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ func repoUsesReftable(dotGitPath, commonGitPath string) bool {
4444
candidates = append(candidates, filepath.Join(commonGitPath, "reftable"))
4545
}
4646
for _, dir := range candidates {
47-
if info, err := os.Stat(dir); err == nil && info.IsDir() {
47+
// Lstat, not Stat: git creates reftable/ as a real directory, so a
48+
// symlink in its place is not a genuine reftable stack. Lstat inspects
49+
// the entry itself rather than following the link, so a symlink reports
50+
// IsDir()==false and is correctly not treated as a reftable repository.
51+
if info, err := os.Lstat(dir); err == nil && info.IsDir() {
4852
return true
4953
}
5054
}
@@ -60,6 +64,12 @@ func repoUsesReftable(dotGitPath, commonGitPath string) bool {
6064
//
6165
// It also advertises reftable support via the ExtensionChecker interface so
6266
// go-git's extension verification does not reject the repository on open.
67+
//
68+
// TODO: remove this entire type (and its wiring in repository.go) once go-git
69+
// gains a built-in reftable reader/writer. It exists only because the vendored
70+
// go-git has no reftable backend, so ref operations must shell out to the git
71+
// CLI. When upstream supports reftable natively, the plain filesystem storer
72+
// handles these repositories and this shim can be deleted.
6373
type reftableStorer struct {
6474
*gitfilesystem.Storage
6575

cmd/entire/cli/gitrepo/repository.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ func openPathWithAlternates(repoRoot string) (*git.Repository, error) {
124124
// authoritative in a reftable repository, and its extension check rejects
125125
// extensions.refstorage=reftable outright. Route ref operations through the
126126
// git CLI for such repositories while keeping object storage on go-git.
127+
//
128+
// TODO: drop the reftable branch below and the whole reftableStorer
129+
// (reftable.go) once go-git ships a native reftable reader/writer. At that
130+
// point a plain git.Open(storage, worktreeFS) will handle reftable
131+
// repositories directly and this CLI-backed shim is dead weight.
127132
worktreeFS := osfs.New(repoRoot, osfs.WithBoundOS())
128133
if repoUsesReftable(dotGitPath, commonGitPath) {
129134
repo, err := git.Open(newReftableStorer(storage, dotGitPath), worktreeFS)

0 commit comments

Comments
 (0)