From 2ecf3145aa7897445dcd4dc2b4936d814e42591a Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Tue, 31 Mar 2026 06:37:33 -0400 Subject: [PATCH 01/11] Improve testkit robustness and release readiness. Harden snapshot restore path handling, simplify git output parsing with stdlib helpers, add CI checks, and expand README usage/requirements to improve maintainability and consumer confidence. Made-with: Cursor --- .github/workflows/ci.yml | 30 +++++++++++++++++ README.md | 37 +++++++++++++++++++++ fixtures.go | 70 ++++++---------------------------------- go.mod | 2 +- snapshots.go | 32 +++++++++++++++++- 5 files changed, 109 insertions(+), 62 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..1b08eec --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,30 @@ +name: CI + +on: + push: + branches: + - main + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + go-version: ["1.22.x", "stable"] + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go-version }} + + - name: Run vet + run: go vet ./... + + - name: Run tests + run: go test ./... diff --git a/README.md b/README.md index 15bcfdd..863fa52 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,11 @@ go get github.com/git-fire/git-testkit ``` +## Requirements + +- `git` must be installed and available on `PATH` +- Go 1.22+ + ## What it includes - Repository fixtures (`CreateTestRepo`, `CreateBareRemote`, `RunGitCmd`) @@ -34,3 +39,35 @@ func TestWithRepo(t *testing.T) { } } ``` + +## Common patterns + +### Build a conflict scenario + +```go +func TestConflictFlow(t *testing.T) { + _, local, _ := testutil.CreateConflictScenario(t) + + // Exercise your logic against a real diverged local clone. + testutil.RunGitCmd(t, local.Path(), "status") +} +``` + +### Snapshot expensive setup + +```go +func TestUsingSnapshot(t *testing.T) { + _, repo := testutil.CreateLargeRepoScenario(t, 20, 10) + + snap := testutil.SnapshotRepo(t, repo.Path()) + clonePath := testutil.RestoreSnapshot(t, snap) + + // Use clonePath in assertions without rebuilding the fixture each time. + testutil.RunGitCmd(t, clonePath, "status") +} +``` + +## Notes + +- Snapshots are intended for deterministic test fixtures and only restore regular files/directories. +- Helpers fail tests immediately (`t.Fatalf`) when git commands fail, so errors surface close to setup code. diff --git a/fixtures.go b/fixtures.go index 93289ad..acf4d83 100644 --- a/fixtures.go +++ b/fixtures.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" ) @@ -193,71 +194,26 @@ func GetRemotes(t *testing.T, repoPath string) map[string]string { // "origin /path/to/remote (push)" remotes := make(map[string]string) - lines := string(output) + lines := strings.TrimSpace(string(output)) if lines == "" { return remotes } - // Simple parsing - just extract remote names - // Full parsing not needed for tests - for _, line := range splitLines(lines) { + for _, line := range strings.Split(lines, "\n") { if line == "" { continue } - // Just check if "origin" appears in the line - // Good enough for test validation - if len(line) > 0 { - // Extract first word (remote name) - parts := splitWhitespace(line) - if len(parts) >= 2 { - name := parts[0] - url := parts[1] - remotes[name] = url - } + parts := strings.Fields(line) + if len(parts) >= 2 { + name := parts[0] + url := parts[1] + remotes[name] = url } } return remotes } -// Helper: split by newlines -func splitLines(s string) []string { - var lines []string - current := "" - for _, ch := range s { - if ch == '\n' { - lines = append(lines, current) - current = "" - } else { - current += string(ch) - } - } - if current != "" { - lines = append(lines, current) - } - return lines -} - -// Helper: split by whitespace/tabs -func splitWhitespace(s string) []string { - var parts []string - current := "" - for _, ch := range s { - if ch == ' ' || ch == '\t' { - if current != "" { - parts = append(parts, current) - current = "" - } - } else { - current += string(ch) - } - } - if current != "" { - parts = append(parts, current) - } - return parts -} - // RunGitCmd runs a git command and fails the test if it errors // Exported version of runGit for use in other test packages func RunGitCmd(t *testing.T, dir string, args ...string) { @@ -277,13 +233,7 @@ func GetCurrentSHA(t *testing.T, repoPath string) string { t.Fatalf("Failed to get current SHA: %v", err) } - sha := string(output) - // Trim newline - if len(sha) > 0 && sha[len(sha)-1] == '\n' { - sha = sha[:len(sha)-1] - } - - return sha + return strings.TrimSpace(string(output)) } // GetBranches returns all branches in the repo @@ -298,7 +248,7 @@ func GetBranches(t *testing.T, repoPath string) []string { t.Fatalf("Failed to get branches: %v", err) } - branches := splitLines(string(output)) + branches := strings.Split(strings.TrimSpace(string(output)), "\n") // Filter out empty lines var result []string diff --git a/go.mod b/go.mod index a03c288..c806c50 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/git-fire/git-testkit -go 1.24.2 +go 1.22 diff --git a/snapshots.go b/snapshots.go index e5f536e..a5c7570 100644 --- a/snapshots.go +++ b/snapshots.go @@ -43,6 +43,9 @@ func SnapshotRepo(t *testing.T, repoPath string) *Snapshot { if err != nil { return fmt.Errorf("failed to get relative path: %w", err) } + if relPath == "." { + return nil + } header.Name = relPath // Write header @@ -117,7 +120,10 @@ func RestoreSnapshot(t *testing.T, snapshot *Snapshot) string { } // Construct full path - targetPath := filepath.Join(restorePath, header.Name) + targetPath, err := safeJoin(restorePath, header.Name) + if err != nil { + t.Fatalf("Invalid snapshot path %q: %v", header.Name, err) + } // Handle different file types switch header.Typeflag { @@ -154,6 +160,30 @@ func RestoreSnapshot(t *testing.T, snapshot *Snapshot) string { return restorePath } +func safeJoin(base, name string) (string, error) { + cleanName := filepath.Clean(name) + if cleanName == "." || cleanName == string(filepath.Separator) { + return "", fmt.Errorf("empty or root path is not allowed") + } + if filepath.IsAbs(cleanName) { + return "", fmt.Errorf("absolute paths are not allowed") + } + if cleanName == ".." || len(cleanName) >= 3 && cleanName[:3] == ".."+string(filepath.Separator) { + return "", fmt.Errorf("path traversal is not allowed") + } + + target := filepath.Join(base, cleanName) + rel, err := filepath.Rel(base, target) + if err != nil { + return "", err + } + if rel == ".." || len(rel) >= 3 && rel[:3] == ".."+string(filepath.Separator) { + return "", fmt.Errorf("resolved path escapes restore directory") + } + + return target, nil +} + // SnapshotSize returns the size of the snapshot in bytes func (s *Snapshot) Size() int { return len(s.tarball) From 0273a4102faf2b908233fe9b03f26427f8ceaaa6 Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Tue, 31 Mar 2026 06:41:07 -0400 Subject: [PATCH 02/11] Expand user and contributor documentation. Improve README onboarding for test usage and add a developer guide covering testing, contribution expectations, and maintainer release workflow. Made-with: Cursor --- DEVELOPER_GUIDE.md | 127 +++++++++++++++++++++++++++++++++++++++++++++ README.md | 49 +++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 DEVELOPER_GUIDE.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md new file mode 100644 index 0000000..b84ccb6 --- /dev/null +++ b/DEVELOPER_GUIDE.md @@ -0,0 +1,127 @@ +# Developer Guide + +This guide covers how to use `git-testkit`, contribute safely, run checks locally, and maintain releases. + +## Audience + +- **Users** writing tests that need real git repositories. +- **Contributors** making code/docs changes. +- **Maintainers/admins** preparing releases and keeping CI healthy. + +## Local setup + +Requirements: + +- Go 1.22+ +- `git` on `PATH` + +Clone and verify: + +```bash +git clone git@github.com:git-fire/git-testkit.git +cd git-testkit +go test ./... +``` + +## Project structure + +- `fixtures.go`: base repo creation and command helpers. +- `scenarios.go`: fluent scenario builder and predefined multi-repo scenarios. +- `snapshots.go`: snapshot/restore utilities for expensive test setup reuse. +- `fixtures_test.go`: external package tests (`testutil_test`) that validate public API usage. +- `scenarios_test.go`: package-internal tests for scenario/snapshot behavior. + +## Design principles + +- Prefer real git commands over mocks for behavior confidence. +- Keep helper APIs composable and minimal. +- Fail fast in setup helpers (`t.Fatalf`) so fixture errors are obvious. +- Keep tests isolated using `t.TempDir()`. + +## Testing and quality checks + +Run before opening a PR: + +```bash +gofmt -w *.go +go vet ./... +go test ./... +``` + +Optional: + +```bash +go test -short ./... +``` + +CI mirrors the required checks (`go vet` and `go test`) on pull requests. + +## Usage guidance for library consumers + +- Prefer scenario builders for integration-style tests with remotes/worktrees. +- Prefer base fixtures for small unit/integration tests that only need one repo. +- Keep assertions close to setup; helper failures already include command output. +- Use snapshots for expensive setups that are reused across subtests. + +Common usage split: + +- `CreateTestRepo`: fast setup for single-repo state. +- `NewScenario`: multi-repo topologies and fluent setup. +- `SnapshotRepo`/`RestoreSnapshot`: performance optimization for repeated expensive setups. + +## Adding new helpers + +When adding new exported helpers: + +- Add or update tests in `fixtures_test.go` and/or `scenarios_test.go`. +- Add usage notes/examples to `README.md` when the helper is user-facing. +- Keep function names explicit and test-focused (avoid generic utility names). + +## Snapshot safety expectations + +- Snapshot restore must never write outside the restore root. +- Avoid introducing behavior that can restore unsupported file types silently. +- Keep snapshot behavior deterministic for repeatable tests. + +## Pull request guidance + +- Keep PRs focused and small when possible. +- Include a clear "why" in the commit/PR description. +- Include a short test plan with commands you ran locally. +- Prefer additive, backward-compatible changes for existing exported helpers. + +Suggested PR checklist: + +- [ ] public API changes documented in `README.md` +- [ ] tests added/updated for behavior changes +- [ ] `gofmt`, `go vet`, and `go test` all pass locally +- [ ] no unrelated refactors mixed into functional changes + +## Maintainer/admin workflow + +### Branch and PR flow + +1. Create a feature branch from `main`. +2. Commit focused changes with clear commit messages. +3. Open a PR with summary and test plan. +4. Merge only after CI is green. + +### Release flow + +1. Verify `main` has passed CI. +2. Confirm `README.md` and this guide reflect current API/behavior. +3. Pull release notes from merged PRs (user-visible changes first). +4. Create and push a version tag. +5. Publish a GitHub release with: + - notable changes + - compatibility notes (for example, minimum Go version) + - migration notes when behavior changed + +## Release notes checklist + +Before cutting a release: + +- Ensure CI is green on `main`. +- Summarize user-visible changes (new helpers, behavior changes, compatibility notes). +- Call out any minimum Go version changes. +- Verify examples in `README.md` still compile conceptually with current API. diff --git a/README.md b/README.md index 863fa52..8fc79ba 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,12 @@ `git-testkit` provides helpers for writing Go tests that exercise real Git repositories. +## Why use this + +- Exercise real git behavior instead of mocking command output. +- Build common repo states quickly (dirty trees, detached HEAD, diverged remotes, worktrees). +- Reuse expensive setups across tests with in-memory snapshots. + ## Install ```bash @@ -19,6 +25,34 @@ go get github.com/git-fire/git-testkit - Scenario builders for common multi-repo states (`NewScenario`, conflict/worktree helpers) - Snapshot helpers for capturing and restoring repository state in tests +## API overview + +- `CreateTestRepo(t, RepoOptions)` creates a real repository with optional files/remotes/branches. +- `CreateBareRemote(t, name)` creates a bare repository for remote testing. +- `NewScenario(t)` returns a fluent builder for multi-repo test topologies. +- `SnapshotRepo(t, path)` and `RestoreSnapshot(t, snap)` speed up repeated fixture setup. +- `IsDirty`, `GetCurrentSHA`, `GetBranches`, `GetRemotes` provide common assertions/helpers. + +## Quickstart for using in tests + +1. Create fixture repos with `CreateTestRepo` or `NewScenario`. +2. Apply setup operations with fluent helpers (`AddFile`, `Commit`, `WithRemote`, `Push`). +3. Run your code under test against the real repository paths. +4. Assert with helper methods (`IsDirty`, `GetCurrentSHA`, `GetBranches`). + +Minimal test flow: + +```go +func TestMyGitBehavior(t *testing.T) { + repoPath := testutil.CreateTestRepo(t, testutil.RepoOptions{Name: "subject"}) + testutil.RunGitCmd(t, repoPath, "checkout", "-b", "feature") + // call your package functions here + if testutil.IsDirty(t, repoPath) { + t.Fatal("repo should be clean") + } +} +``` + ## Example ```go @@ -40,6 +74,12 @@ func TestWithRepo(t *testing.T) { } ``` +## Cleanup behavior + +- All helper-created repositories use `t.TempDir()`. +- Repos/worktrees are automatically removed by Go's test framework at test completion. +- As with any temp directories, force-killed test processes may leave files behind. + ## Common patterns ### Build a conflict scenario @@ -71,3 +111,12 @@ func TestUsingSnapshot(t *testing.T) { - Snapshots are intended for deterministic test fixtures and only restore regular files/directories. - Helpers fail tests immediately (`t.Fatalf`) when git commands fail, so errors surface close to setup code. +- When tests build large repo graphs repeatedly, prefer snapshot/restore to reduce total runtime. + +## Developer docs + +- See `DEVELOPER_GUIDE.md` for: + - testing and quality gates + - usage guidance for library consumers + - administration/maintenance and release workflow + - contribution process and PR expectations From 3d216ef09b7c853e9e825f3dfd72fb2eb1a3d804 Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Tue, 31 Mar 2026 07:36:22 -0400 Subject: [PATCH 03/11] Fix remote URL parsing for paths with spaces. Parse `git remote -v` output without truncating whitespace-containing URLs and add a regression test to lock in behavior for local remotes with spaced paths. Made-with: Cursor --- fixtures.go | 23 ++++++++++++++++++----- fixtures_test.go | 20 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/fixtures.go b/fixtures.go index acf4d83..57e4d00 100644 --- a/fixtures.go +++ b/fixtures.go @@ -200,14 +200,27 @@ func GetRemotes(t *testing.T, repoPath string) map[string]string { } for _, line := range strings.Split(lines, "\n") { + line = strings.TrimSpace(line) if line == "" { continue } - parts := strings.Fields(line) - if len(parts) >= 2 { - name := parts[0] - url := parts[1] - remotes[name] = url + + name, remainder, ok := strings.Cut(line, "\t") + if !ok { + // Fallback for unusual formatting that does not use tabs. + idx := strings.IndexAny(line, " \t") + if idx == -1 { + continue + } + name = line[:idx] + remainder = strings.TrimSpace(line[idx+1:]) + } + + remainder = strings.TrimSuffix(remainder, " (fetch)") + remainder = strings.TrimSuffix(remainder, " (push)") + + if name != "" && remainder != "" { + remotes[name] = remainder } } diff --git a/fixtures_test.go b/fixtures_test.go index 641605d..77f5af1 100644 --- a/fixtures_test.go +++ b/fixtures_test.go @@ -79,6 +79,26 @@ func TestCreateTestRepo_WithRemotes(t *testing.T) { } } +func TestCreateTestRepo_WithRemotePathContainingSpaces(t *testing.T) { + remotePath := testutil.CreateBareRemote(t, "origin with space") + + repoPath := testutil.CreateTestRepo(t, testutil.RepoOptions{ + Name: "remote-space-repo", + Remotes: map[string]string{ + "origin": remotePath, + }, + }) + + remotes := testutil.GetRemotes(t, repoPath) + originURL, exists := remotes["origin"] + if !exists { + t.Fatal("Expected 'origin' remote to be configured") + } + if originURL != remotePath { + t.Fatalf("Expected origin URL %q, got %q", remotePath, originURL) + } +} + func TestCreateBareRemote(t *testing.T) { remotePath := testutil.CreateBareRemote(t, "test-remote") From 58fe7d0f4031fbfaa130df5e81c6606f5b620fe9 Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Sun, 5 Apr 2026 01:36:45 -0400 Subject: [PATCH 04/11] feat: add USB fixture helpers for target-volume testing Provide reusable helpers for creating .git-fire volume roots, reading/writing marker config, asserting bare/non-bare git destinations, and file URL conversion. --- usb_fixtures.go | 134 +++++++++++++++++++++++++++++++++++++++++++ usb_fixtures_test.go | 48 ++++++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 usb_fixtures.go create mode 100644 usb_fixtures_test.go diff --git a/usb_fixtures.go b/usb_fixtures.go new file mode 100644 index 0000000..b40f85f --- /dev/null +++ b/usb_fixtures.go @@ -0,0 +1,134 @@ +package testutil + +import ( + "fmt" + "net/url" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "time" +) + +type USBVolumeOptions struct { + LayoutDir string + Strategy string + CreateReposDir bool +} + +type USBVolumeConfig struct { + SchemaVersion int + LayoutDir string + Strategy string + CreatedAt time.Time +} + +func MustUSBVolumeRoot(t *testing.T, opts USBVolumeOptions) string { + t.Helper() + root := t.TempDir() + cfg := USBVolumeConfig{ + SchemaVersion: 1, + LayoutDir: opts.LayoutDir, + Strategy: opts.Strategy, + CreatedAt: time.Now().UTC(), + } + if cfg.LayoutDir == "" { + cfg.LayoutDir = "repos" + } + if cfg.Strategy == "" { + cfg.Strategy = "git-mirror" + } + WriteUSBVolumeConfig(t, root, cfg) + if opts.CreateReposDir { + if err := os.MkdirAll(filepath.Join(root, cfg.LayoutDir), 0o755); err != nil { + t.Fatalf("failed creating repos dir: %v", err) + } + } + return root +} + +func WriteUSBVolumeConfig(t *testing.T, root string, cfg USBVolumeConfig) { + t.Helper() + if cfg.SchemaVersion <= 0 { + cfg.SchemaVersion = 1 + } + if cfg.LayoutDir == "" { + cfg.LayoutDir = "repos" + } + if cfg.Strategy == "" { + cfg.Strategy = "git-mirror" + } + if cfg.CreatedAt.IsZero() { + cfg.CreatedAt = time.Now().UTC() + } + content := fmt.Sprintf( + "schema_version = %d\nlayout_dir = %q\nstrategy = %q\ncreated_at = %q\n", + cfg.SchemaVersion, + cfg.LayoutDir, + cfg.Strategy, + cfg.CreatedAt.Format(time.RFC3339), + ) + if err := os.WriteFile(filepath.Join(root, ".git-fire"), []byte(content), 0o644); err != nil { + t.Fatalf("failed writing .git-fire: %v", err) + } +} + +func ReadUSBVolumeConfig(t *testing.T, root string) USBVolumeConfig { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, ".git-fire")) + if err != nil { + t.Fatalf("failed reading .git-fire: %v", err) + } + cfg := USBVolumeConfig{} + lines := strings.Split(string(data), "\n") + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + key, val, ok := strings.Cut(line, "=") + if !ok { + continue + } + key = strings.TrimSpace(key) + val = strings.Trim(strings.TrimSpace(val), "\"") + switch key { + case "schema_version": + n, _ := strconv.Atoi(val) + cfg.SchemaVersion = n + case "layout_dir": + cfg.LayoutDir = val + case "strategy": + cfg.Strategy = val + case "created_at": + if ts, err := time.Parse(time.RFC3339, val); err == nil { + cfg.CreatedAt = ts + } + } + } + return cfg +} + +func AssertGitDirAt(t *testing.T, path string, wantBare bool) { + t.Helper() + if wantBare { + if _, err := os.Stat(filepath.Join(path, "HEAD")); err != nil { + t.Fatalf("expected bare repo at %s: %v", path, err) + } + return + } + if _, err := os.Stat(filepath.Join(path, ".git")); err != nil { + t.Fatalf("expected non-bare repo at %s: %v", path, err) + } +} + +func FileURLForPath(t *testing.T, path string) string { + t.Helper() + abs, err := filepath.Abs(path) + if err != nil { + t.Fatalf("failed to make abs path: %v", err) + } + u := &url.URL{Scheme: "file", Path: filepath.ToSlash(abs)} + return u.String() +} diff --git a/usb_fixtures_test.go b/usb_fixtures_test.go new file mode 100644 index 0000000..7d8ae96 --- /dev/null +++ b/usb_fixtures_test.go @@ -0,0 +1,48 @@ +package testutil + +import ( + "os" + "path/filepath" + "testing" +) + +func TestMustUSBVolumeRoot(t *testing.T) { + root := MustUSBVolumeRoot(t, USBVolumeOptions{ + LayoutDir: "repos", + Strategy: "git-mirror", + CreateReposDir: true, + }) + if _, err := os.Stat(filepath.Join(root, ".git-fire")); err != nil { + t.Fatalf("expected .git-fire marker: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "repos")); err != nil { + t.Fatalf("expected repos dir: %v", err) + } +} + +func TestReadWriteUSBVolumeConfig(t *testing.T) { + root := t.TempDir() + WriteUSBVolumeConfig(t, root, USBVolumeConfig{ + SchemaVersion: 2, + LayoutDir: "custom", + Strategy: "git-clone", + }) + cfg := ReadUSBVolumeConfig(t, root) + if cfg.SchemaVersion != 2 { + t.Fatalf("schema mismatch: %d", cfg.SchemaVersion) + } + if cfg.LayoutDir != "custom" { + t.Fatalf("layout mismatch: %s", cfg.LayoutDir) + } + if cfg.Strategy != "git-clone" { + t.Fatalf("strategy mismatch: %s", cfg.Strategy) + } +} + +func TestFileURLForPath(t *testing.T) { + root := t.TempDir() + got := FileURLForPath(t, root) + if len(got) < 7 || got[:7] != "file://" { + t.Fatalf("expected file:// URL, got %s", got) + } +} From 53832d7ff144a5eaa7a3eadcf5c5c0d36466a118 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 5 Apr 2026 06:07:16 +0000 Subject: [PATCH 05/11] Harden snapshot restore root and add security regressions Co-authored-by: Ben Schellenberger --- snapshots.go | 16 ++++++--- snapshots_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 snapshots_test.go diff --git a/snapshots.go b/snapshots.go index a5c7570..12834ef 100644 --- a/snapshots.go +++ b/snapshots.go @@ -94,7 +94,10 @@ func RestoreSnapshot(t *testing.T, snapshot *Snapshot) string { // Create temp directory for restoration tmpDir := t.TempDir() - restorePath := filepath.Join(tmpDir, snapshot.name) + restorePath, err := safeJoin(tmpDir, snapshot.name) + if err != nil { + t.Fatalf("Invalid snapshot name %q: %v", snapshot.name, err) + } if err := os.MkdirAll(restorePath, 0755); err != nil { t.Fatalf("Failed to create restore directory: %v", err) @@ -204,16 +207,21 @@ func SaveSnapshotToDisk(t *testing.T, snapshot *Snapshot, filepath string) { } // LoadSnapshotFromDisk loads a snapshot from a file -func LoadSnapshotFromDisk(t *testing.T, filepath string) *Snapshot { +func LoadSnapshotFromDisk(t *testing.T, filePath string) *Snapshot { t.Helper() - data, err := os.ReadFile(filepath) + data, err := os.ReadFile(filePath) if err != nil { t.Fatalf("Failed to load snapshot from disk: %v", err) } + snapshotName := filepath.Base(filePath) + if snapshotName == "." || snapshotName == string(filepath.Separator) { + snapshotName = "snapshot" + } + return &Snapshot{ - name: filepath, + name: snapshotName, tarball: data, } } diff --git a/snapshots_test.go b/snapshots_test.go new file mode 100644 index 0000000..879bb77 --- /dev/null +++ b/snapshots_test.go @@ -0,0 +1,85 @@ +package testutil + +import ( + "os" + "path/filepath" + "testing" +) + +func TestRestoreSnapshotRejectsUnsafeSnapshotNames(t *testing.T) { + tests := []struct { + name string + base string + joinArg string + }{ + { + name: "path traversal name", + base: filepath.Join("tmp", "root"), + joinArg: "../escape", + }, + { + name: "absolute name", + base: filepath.Join("tmp", "root"), + joinArg: string(filepath.Separator) + "tmp/escape", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if _, err := safeJoin(tt.base, tt.joinArg); err == nil { + t.Fatalf("expected safeJoin to reject %q", tt.joinArg) + } + }) + } +} + +func TestRestoreSnapshotRejectsUnsafeArchivePaths(t *testing.T) { + tests := []struct { + name string + base string + joinArg string + }{ + { + name: "relative traversal path", + base: filepath.Join("tmp", "root"), + joinArg: "../escape.txt", + }, + { + name: "absolute path", + base: filepath.Join("tmp", "root"), + joinArg: string(filepath.Separator) + "tmp/escape.txt", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if _, err := safeJoin(tt.base, tt.joinArg); err == nil { + t.Fatalf("expected safeJoin to reject %q", tt.joinArg) + } + }) + } +} + +func TestLoadSnapshotFromDiskUsesBaseName(t *testing.T) { + _, repo := CreateCleanRepoScenario(t) + snapshot := SnapshotRepo(t, repo.path) + + snapshotPath := filepath.Join(t.TempDir(), "nested", "snapshot.tar.gz") + + if err := os.MkdirAll(filepath.Dir(snapshotPath), 0755); err != nil { + t.Fatalf("failed to create snapshot dir: %v", err) + } + SaveSnapshotToDisk(t, snapshot, snapshotPath) + + loaded := LoadSnapshotFromDisk(t, snapshotPath) + if got, want := loaded.Name(), filepath.Base(snapshotPath); got != want { + t.Fatalf("expected loaded snapshot name %q, got %q", want, got) + } + + restoredPath := RestoreSnapshot(t, loaded) + if got, want := filepath.Base(restoredPath), loaded.Name(); got != want { + t.Fatalf("expected restore dir base %q, got %q", want, got) + } +} From e8663de6eb5bd9ada6d8c6492d84c5d0bab9d60c Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Sun, 5 Apr 2026 02:23:26 -0400 Subject: [PATCH 06/11] Address CodeRabbit review on PR #1 - DEVELOPER_GUIDE: document HTTPS clone first, SSH as optional - GetRemotes: parse git remote -v by tab; strip (fetch)/(push) so URLs with spaces work - snapshots: shared normalizeSnapshotName for SnapshotRepo and LoadSnapshotFromDisk - Add regression test for SnapshotRepo(".") after chdir into repo --- DEVELOPER_GUIDE.md | 7 ++++++- fixtures.go | 16 +++++++++++----- snapshots.go | 17 ++++++++++------- snapshots_test.go | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index b84ccb6..1b885b0 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -18,7 +18,12 @@ Requirements: Clone and verify: ```bash -git clone git@github.com:git-fire/git-testkit.git +# HTTPS (no SSH keys required) +git clone https://github.com/git-fire/git-testkit.git + +# or SSH +# git clone git@github.com:git-fire/git-testkit.git + cd git-testkit go test ./... ``` diff --git a/fixtures.go b/fixtures.go index acf4d83..e18ba38 100644 --- a/fixtures.go +++ b/fixtures.go @@ -200,14 +200,20 @@ func GetRemotes(t *testing.T, repoPath string) map[string]string { } for _, line := range strings.Split(lines, "\n") { + line = strings.TrimSpace(line) if line == "" { continue } - parts := strings.Fields(line) - if len(parts) >= 2 { - name := parts[0] - url := parts[1] - remotes[name] = url + name, rest, ok := strings.Cut(line, "\t") + if !ok { + continue + } + name = strings.TrimSpace(name) + rest = strings.TrimSpace(rest) + rest = strings.TrimSuffix(rest, " (fetch)") + rest = strings.TrimSuffix(rest, " (push)") + if name != "" && rest != "" { + remotes[name] = rest } } diff --git a/snapshots.go b/snapshots.go index 12834ef..baf5a84 100644 --- a/snapshots.go +++ b/snapshots.go @@ -11,6 +11,14 @@ import ( "testing" ) +func normalizeSnapshotName(path string) string { + name := filepath.Base(path) + if name == "." || name == string(filepath.Separator) { + return "snapshot" + } + return name +} + // Snapshot represents a saved state of a git repository type Snapshot struct { name string @@ -82,7 +90,7 @@ func SnapshotRepo(t *testing.T, repoPath string) *Snapshot { } return &Snapshot{ - name: filepath.Base(repoPath), + name: normalizeSnapshotName(repoPath), tarball: buf.Bytes(), } } @@ -215,13 +223,8 @@ func LoadSnapshotFromDisk(t *testing.T, filePath string) *Snapshot { t.Fatalf("Failed to load snapshot from disk: %v", err) } - snapshotName := filepath.Base(filePath) - if snapshotName == "." || snapshotName == string(filepath.Separator) { - snapshotName = "snapshot" - } - return &Snapshot{ - name: snapshotName, + name: normalizeSnapshotName(filePath), tarball: data, } } diff --git a/snapshots_test.go b/snapshots_test.go index 879bb77..40c2825 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -62,6 +62,23 @@ func TestRestoreSnapshotRejectsUnsafeArchivePaths(t *testing.T) { } } +func TestSnapshotRepoNormalizesTrailingDotPath(t *testing.T) { + _, repo := CreateCleanRepoScenario(t) + oldWd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(oldWd) }) + if err := os.Chdir(repo.path); err != nil { + t.Fatal(err) + } + snap := SnapshotRepo(t, ".") + if got, want := snap.Name(), "snapshot"; got != want { + t.Fatalf("expected snapshot name %q, got %q", want, got) + } + _ = RestoreSnapshot(t, snap) +} + func TestLoadSnapshotFromDiskUsesBaseName(t *testing.T) { _, repo := CreateCleanRepoScenario(t) snapshot := SnapshotRepo(t, repo.path) From 929834e498508dd111f7debd7dbb3f6e3199c0c7 Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Sun, 5 Apr 2026 16:07:13 -0400 Subject: [PATCH 07/11] fix(testutil): harden USB fixtures per review feedback Validate layout_dir stays relative under the fixture root, surface invalid schema_version when parsing .git-fire, and build RFC 8089-style file URLs on Windows drive-letter paths. Tighten FileURLForPath test. Made-with: Cursor --- usb_fixtures.go | 35 ++++++++++++++++++++++++++--------- usb_fixtures_test.go | 16 ++++++++++++++-- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/usb_fixtures.go b/usb_fixtures.go index b40f85f..454f701 100644 --- a/usb_fixtures.go +++ b/usb_fixtures.go @@ -24,18 +24,30 @@ type USBVolumeConfig struct { CreatedAt time.Time } +func mustRelativeLayoutDir(t *testing.T, layoutDir string) string { + t.Helper() + if layoutDir == "" { + return "repos" + } + clean := filepath.Clean(layoutDir) + if filepath.IsAbs(clean) { + t.Fatalf("layout_dir must be relative to fixture root: %q", layoutDir) + } + if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + t.Fatalf("layout_dir must be relative to fixture root: %q", layoutDir) + } + return clean +} + func MustUSBVolumeRoot(t *testing.T, opts USBVolumeOptions) string { t.Helper() root := t.TempDir() cfg := USBVolumeConfig{ SchemaVersion: 1, - LayoutDir: opts.LayoutDir, + LayoutDir: mustRelativeLayoutDir(t, opts.LayoutDir), Strategy: opts.Strategy, CreatedAt: time.Now().UTC(), } - if cfg.LayoutDir == "" { - cfg.LayoutDir = "repos" - } if cfg.Strategy == "" { cfg.Strategy = "git-mirror" } @@ -53,9 +65,7 @@ func WriteUSBVolumeConfig(t *testing.T, root string, cfg USBVolumeConfig) { if cfg.SchemaVersion <= 0 { cfg.SchemaVersion = 1 } - if cfg.LayoutDir == "" { - cfg.LayoutDir = "repos" - } + cfg.LayoutDir = mustRelativeLayoutDir(t, cfg.LayoutDir) if cfg.Strategy == "" { cfg.Strategy = "git-mirror" } @@ -95,7 +105,10 @@ func ReadUSBVolumeConfig(t *testing.T, root string) USBVolumeConfig { val = strings.Trim(strings.TrimSpace(val), "\"") switch key { case "schema_version": - n, _ := strconv.Atoi(val) + n, err := strconv.Atoi(val) + if err != nil { + t.Fatalf("invalid schema_version %q: %v", val, err) + } cfg.SchemaVersion = n case "layout_dir": cfg.LayoutDir = val @@ -129,6 +142,10 @@ func FileURLForPath(t *testing.T, path string) string { if err != nil { t.Fatalf("failed to make abs path: %v", err) } - u := &url.URL{Scheme: "file", Path: filepath.ToSlash(abs)} + uPath := filepath.ToSlash(abs) + if filepath.VolumeName(abs) != "" && !strings.HasPrefix(uPath, "/") { + uPath = "/" + uPath + } + u := &url.URL{Scheme: "file", Path: uPath} return u.String() } diff --git a/usb_fixtures_test.go b/usb_fixtures_test.go index 7d8ae96..1a944b5 100644 --- a/usb_fixtures_test.go +++ b/usb_fixtures_test.go @@ -1,8 +1,10 @@ package testutil import ( + "net/url" "os" "path/filepath" + "strings" "testing" ) @@ -42,7 +44,17 @@ func TestReadWriteUSBVolumeConfig(t *testing.T) { func TestFileURLForPath(t *testing.T) { root := t.TempDir() got := FileURLForPath(t, root) - if len(got) < 7 || got[:7] != "file://" { - t.Fatalf("expected file:// URL, got %s", got) + parsed, err := url.Parse(got) + if err != nil { + t.Fatalf("parse URL: %v", err) + } + if parsed.Scheme != "file" { + t.Fatalf("scheme %q, want file", parsed.Scheme) + } + if parsed.Path == "" || parsed.Path[0] != '/' { + t.Fatalf("expected absolute path in URL, got path=%q for %q", parsed.Path, got) + } + if !strings.HasPrefix(got, "file:///") { + t.Fatalf("expected canonical file URL with empty authority (file:///...), got %q", got) } } From c2b6dc48a820742fb5d1f48c395eb2367ccd83b7 Mon Sep 17 00:00:00 2001 From: Ben Schellenberger Date: Sun, 5 Apr 2026 16:11:21 -0400 Subject: [PATCH 08/11] fix(testutil): stricter .git-fire parsing and layout validation tests Extract validateFixtureLayoutDir and readUSBVolumeConfigBytes so reads reject bad layout_dir, invalid schema_version, and bad or empty created_at. Normalize non-empty layout_dir on read. Add table tests for validation and parse errors. Made-with: Cursor --- usb_fixtures.go | 66 ++++++++++++++++++++++++--------- usb_fixtures_test.go | 87 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 17 deletions(-) diff --git a/usb_fixtures.go b/usb_fixtures.go index 454f701..c43324a 100644 --- a/usb_fixtures.go +++ b/usb_fixtures.go @@ -11,6 +11,22 @@ import ( "time" ) +// validateFixtureLayoutDir reports whether layoutDir may be joined under a fixture root. +// Empty layoutDir is allowed (caller may default it). +func validateFixtureLayoutDir(layoutDir string) error { + if layoutDir == "" { + return nil + } + clean := filepath.Clean(layoutDir) + if filepath.IsAbs(clean) { + return fmt.Errorf("must be relative to fixture root: %q", layoutDir) + } + if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return fmt.Errorf("must be relative to fixture root: %q", layoutDir) + } + return nil +} + type USBVolumeOptions struct { LayoutDir string Strategy string @@ -29,14 +45,10 @@ func mustRelativeLayoutDir(t *testing.T, layoutDir string) string { if layoutDir == "" { return "repos" } - clean := filepath.Clean(layoutDir) - if filepath.IsAbs(clean) { - t.Fatalf("layout_dir must be relative to fixture root: %q", layoutDir) - } - if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { - t.Fatalf("layout_dir must be relative to fixture root: %q", layoutDir) + if err := validateFixtureLayoutDir(layoutDir); err != nil { + t.Fatalf("layout_dir %v", err) } - return clean + return filepath.Clean(layoutDir) } func MustUSBVolumeRoot(t *testing.T, opts USBVolumeOptions) string { @@ -84,12 +96,7 @@ func WriteUSBVolumeConfig(t *testing.T, root string, cfg USBVolumeConfig) { } } -func ReadUSBVolumeConfig(t *testing.T, root string) USBVolumeConfig { - t.Helper() - data, err := os.ReadFile(filepath.Join(root, ".git-fire")) - if err != nil { - t.Fatalf("failed reading .git-fire: %v", err) - } +func readUSBVolumeConfigBytes(data []byte) (USBVolumeConfig, error) { cfg := USBVolumeConfig{} lines := strings.Split(string(data), "\n") for _, line := range lines { @@ -107,19 +114,44 @@ func ReadUSBVolumeConfig(t *testing.T, root string) USBVolumeConfig { case "schema_version": n, err := strconv.Atoi(val) if err != nil { - t.Fatalf("invalid schema_version %q: %v", val, err) + return cfg, fmt.Errorf("invalid schema_version %q: %w", val, err) } cfg.SchemaVersion = n case "layout_dir": - cfg.LayoutDir = val + if err := validateFixtureLayoutDir(val); err != nil { + return cfg, fmt.Errorf("layout_dir: %w", err) + } + if val == "" { + cfg.LayoutDir = "" + } else { + cfg.LayoutDir = filepath.Clean(val) + } case "strategy": cfg.Strategy = val case "created_at": - if ts, err := time.Parse(time.RFC3339, val); err == nil { - cfg.CreatedAt = ts + if val == "" { + return cfg, fmt.Errorf("created_at: empty value") } + ts, err := time.Parse(time.RFC3339, val) + if err != nil { + return cfg, fmt.Errorf("invalid created_at %q: %w", val, err) + } + cfg.CreatedAt = ts } } + return cfg, nil +} + +func ReadUSBVolumeConfig(t *testing.T, root string) USBVolumeConfig { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, ".git-fire")) + if err != nil { + t.Fatalf("failed reading .git-fire: %v", err) + } + cfg, err := readUSBVolumeConfigBytes(data) + if err != nil { + t.Fatalf("parse .git-fire: %v", err) + } return cfg } diff --git a/usb_fixtures_test.go b/usb_fixtures_test.go index 1a944b5..458c934 100644 --- a/usb_fixtures_test.go +++ b/usb_fixtures_test.go @@ -41,6 +41,93 @@ func TestReadWriteUSBVolumeConfig(t *testing.T) { } } +func TestValidateFixtureLayoutDir(t *testing.T) { + t.Parallel() + root := t.TempDir() + absUnderRoot := filepath.Join(root, "abs-layout") + if err := os.MkdirAll(absUnderRoot, 0o755); err != nil { + t.Fatal(err) + } + bad := []string{ + "..", + "../escape", + "nested/../../../escape", + absUnderRoot, + } + for _, dir := range bad { + if err := validateFixtureLayoutDir(dir); err == nil { + t.Errorf("validateFixtureLayoutDir(%q): want error, got nil", dir) + } + } + good := []string{"", "repos", "nested", "nested/../repos"} + for _, dir := range good { + if err := validateFixtureLayoutDir(dir); err != nil { + t.Errorf("validateFixtureLayoutDir(%q): %v", dir, err) + } + } +} + +func TestReadUSBVolumeConfigBytes_roundTrip(t *testing.T) { + t.Parallel() + input := "schema_version = 2\nlayout_dir = \"custom\"\nstrategy = \"git-clone\"\ncreated_at = \"2020-01-02T15:04:05Z\"\n" + cfg, err := readUSBVolumeConfigBytes([]byte(input)) + if err != nil { + t.Fatal(err) + } + if cfg.SchemaVersion != 2 { + t.Fatalf("schema: got %d", cfg.SchemaVersion) + } + if cfg.LayoutDir != "custom" { + t.Fatalf("layout: got %q", cfg.LayoutDir) + } + if cfg.Strategy != "git-clone" { + t.Fatalf("strategy: got %q", cfg.Strategy) + } + if cfg.CreatedAt.IsZero() { + t.Fatal("created_at: zero") + } +} + +func TestReadUSBVolumeConfigBytes_errors(t *testing.T) { + t.Parallel() + cases := []struct { + name, content, wantSubstring string + }{ + { + name: "invalid_schema_version", + content: "schema_version = notint\n", + wantSubstring: "schema_version", + }, + { + name: "layout_dir_escape", + content: "layout_dir = ../x\n", + wantSubstring: "layout_dir", + }, + { + name: "invalid_created_at", + content: "created_at = not-a-date\n", + wantSubstring: "created_at", + }, + { + name: "empty_created_at", + content: "created_at = \n", + wantSubstring: "created_at", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := readUSBVolumeConfigBytes([]byte(tc.content)) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), tc.wantSubstring) { + t.Fatalf("error %q does not contain %q", err.Error(), tc.wantSubstring) + } + }) + } +} + func TestFileURLForPath(t *testing.T) { root := t.TempDir() got := FileURLForPath(t, root) From d088c0835fb3dd543a307e7165c84fc6ca63d5f2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 5 Apr 2026 20:15:52 +0000 Subject: [PATCH 09/11] Fix snapshot name normalization for dot-dot paths --- snapshots.go | 2 +- snapshots_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/snapshots.go b/snapshots.go index baf5a84..98c25aa 100644 --- a/snapshots.go +++ b/snapshots.go @@ -13,7 +13,7 @@ import ( func normalizeSnapshotName(path string) string { name := filepath.Base(path) - if name == "." || name == string(filepath.Separator) { + if name == "." || name == ".." || name == string(filepath.Separator) { return "snapshot" } return name diff --git a/snapshots_test.go b/snapshots_test.go index 40c2825..49574d1 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -79,6 +79,22 @@ func TestSnapshotRepoNormalizesTrailingDotPath(t *testing.T) { _ = RestoreSnapshot(t, snap) } +func TestNormalizeSnapshotNameHandlesDotDot(t *testing.T) { + tests := []struct { + input string + want string + }{ + {input: "..", want: "snapshot"}, + {input: filepath.Join("foo", ".."), want: "snapshot"}, + } + + for _, tt := range tests { + if got := normalizeSnapshotName(tt.input); got != tt.want { + t.Fatalf("normalizeSnapshotName(%q) = %q, want %q", tt.input, got, tt.want) + } + } +} + func TestLoadSnapshotFromDiskUsesBaseName(t *testing.T) { _, repo := CreateCleanRepoScenario(t) snapshot := SnapshotRepo(t, repo.path) From 628a737553e65efb611ac242e795968eadc548f0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 5 Apr 2026 20:57:59 +0000 Subject: [PATCH 10/11] test: add coverage for AssertGitDirAt helper --- usb_fixtures_test.go | 48 ++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/usb_fixtures_test.go b/usb_fixtures_test.go index 458c934..1b919f2 100644 --- a/usb_fixtures_test.go +++ b/usb_fixtures_test.go @@ -10,9 +10,9 @@ import ( func TestMustUSBVolumeRoot(t *testing.T) { root := MustUSBVolumeRoot(t, USBVolumeOptions{ - LayoutDir: "repos", - Strategy: "git-mirror", - CreateReposDir: true, + LayoutDir: "repos", + Strategy: "git-mirror", + CreateReposDir: true, }) if _, err := os.Stat(filepath.Join(root, ".git-fire")); err != nil { t.Fatalf("expected .git-fire marker: %v", err) @@ -94,24 +94,24 @@ func TestReadUSBVolumeConfigBytes_errors(t *testing.T) { name, content, wantSubstring string }{ { - name: "invalid_schema_version", - content: "schema_version = notint\n", - wantSubstring: "schema_version", + name: "invalid_schema_version", + content: "schema_version = notint\n", + wantSubstring: "schema_version", }, { - name: "layout_dir_escape", - content: "layout_dir = ../x\n", - wantSubstring: "layout_dir", + name: "layout_dir_escape", + content: "layout_dir = ../x\n", + wantSubstring: "layout_dir", }, { - name: "invalid_created_at", - content: "created_at = not-a-date\n", - wantSubstring: "created_at", + name: "invalid_created_at", + content: "created_at = not-a-date\n", + wantSubstring: "created_at", }, { - name: "empty_created_at", - content: "created_at = \n", - wantSubstring: "created_at", + name: "empty_created_at", + content: "created_at = \n", + wantSubstring: "created_at", }, } for _, tc := range cases { @@ -145,3 +145,21 @@ func TestFileURLForPath(t *testing.T) { t.Fatalf("expected canonical file URL with empty authority (file:///...), got %q", got) } } + +func TestAssertGitDirAt(t *testing.T) { + t.Run("bare_repo", func(t *testing.T) { + repo := t.TempDir() + if err := os.WriteFile(filepath.Join(repo, "HEAD"), []byte("ref: refs/heads/main\n"), 0o644); err != nil { + t.Fatalf("write HEAD: %v", err) + } + AssertGitDirAt(t, repo, true) + }) + + t.Run("non_bare_repo", func(t *testing.T) { + repo := t.TempDir() + if err := os.Mkdir(filepath.Join(repo, ".git"), 0o755); err != nil { + t.Fatalf("create .git directory: %v", err) + } + AssertGitDirAt(t, repo, false) + }) +} From d0c6cb29985c2dc0ef2df9e3490c8c7f4fefaa21 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 5 Apr 2026 21:11:04 +0000 Subject: [PATCH 11/11] Fix traversal checks and USB config unquoting --- snapshots.go | 5 +++-- usb_fixtures.go | 15 ++++++++++++--- usb_fixtures_test.go | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/snapshots.go b/snapshots.go index 98c25aa..56a4667 100644 --- a/snapshots.go +++ b/snapshots.go @@ -8,6 +8,7 @@ import ( "io" "os" "path/filepath" + "strings" "testing" ) @@ -179,7 +180,7 @@ func safeJoin(base, name string) (string, error) { if filepath.IsAbs(cleanName) { return "", fmt.Errorf("absolute paths are not allowed") } - if cleanName == ".." || len(cleanName) >= 3 && cleanName[:3] == ".."+string(filepath.Separator) { + if cleanName == ".." || strings.HasPrefix(cleanName, ".."+string(filepath.Separator)) { return "", fmt.Errorf("path traversal is not allowed") } @@ -188,7 +189,7 @@ func safeJoin(base, name string) (string, error) { if err != nil { return "", err } - if rel == ".." || len(rel) >= 3 && rel[:3] == ".."+string(filepath.Separator) { + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return "", fmt.Errorf("resolved path escapes restore directory") } diff --git a/usb_fixtures.go b/usb_fixtures.go index c43324a..f453a83 100644 --- a/usb_fixtures.go +++ b/usb_fixtures.go @@ -28,8 +28,8 @@ func validateFixtureLayoutDir(layoutDir string) error { } type USBVolumeOptions struct { - LayoutDir string - Strategy string + LayoutDir string + Strategy string CreateReposDir bool } @@ -109,7 +109,7 @@ func readUSBVolumeConfigBytes(data []byte) (USBVolumeConfig, error) { continue } key = strings.TrimSpace(key) - val = strings.Trim(strings.TrimSpace(val), "\"") + val = strings.TrimSpace(val) switch key { case "schema_version": n, err := strconv.Atoi(val) @@ -118,6 +118,9 @@ func readUSBVolumeConfigBytes(data []byte) (USBVolumeConfig, error) { } cfg.SchemaVersion = n case "layout_dir": + if unquoted, err := strconv.Unquote(val); err == nil { + val = unquoted + } if err := validateFixtureLayoutDir(val); err != nil { return cfg, fmt.Errorf("layout_dir: %w", err) } @@ -127,8 +130,14 @@ func readUSBVolumeConfigBytes(data []byte) (USBVolumeConfig, error) { cfg.LayoutDir = filepath.Clean(val) } case "strategy": + if unquoted, err := strconv.Unquote(val); err == nil { + val = unquoted + } cfg.Strategy = val case "created_at": + if unquoted, err := strconv.Unquote(val); err == nil { + val = unquoted + } if val == "" { return cfg, fmt.Errorf("created_at: empty value") } diff --git a/usb_fixtures_test.go b/usb_fixtures_test.go index 1b919f2..2298af4 100644 --- a/usb_fixtures_test.go +++ b/usb_fixtures_test.go @@ -41,6 +41,23 @@ func TestReadWriteUSBVolumeConfig(t *testing.T) { } } +func TestReadWriteUSBVolumeConfig_preservesEscapedChars(t *testing.T) { + root := t.TempDir() + WriteUSBVolumeConfig(t, root, USBVolumeConfig{ + SchemaVersion: 1, + LayoutDir: `repo\"dir\path`, + Strategy: `mirror\"strategy\mode`, + }) + + cfg := ReadUSBVolumeConfig(t, root) + if cfg.LayoutDir != `repo\"dir\path` { + t.Fatalf("layout mismatch: got %q", cfg.LayoutDir) + } + if cfg.Strategy != `mirror\"strategy\mode` { + t.Fatalf("strategy mismatch: got %q", cfg.Strategy) + } +} + func TestValidateFixtureLayoutDir(t *testing.T) { t.Parallel() root := t.TempDir()