diff --git a/build_test.go b/build_test.go index 6117bf2..2e7f58c 100644 --- a/build_test.go +++ b/build_test.go @@ -520,3 +520,32 @@ func (w *sourceChangeWriter) Write(p []byte) (int, error) { } return len(p), nil } + +func TestCandidateBuildDoesNotRequireWritableRepositoryParent(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("requires ordinary Unix permission checks") + } + repository := newBuildConsumer(t, `package main +import("fmt";"gitea.maximumdirect.net/eric/ReleaseKit/buildinfo") +func main(){fmt.Println("fixture",buildinfo.Version())} +`) + writeBuildFixture(t, repository, "docs/releases/v1.2.3.md", "# Fixture v1.2.3\n") + buildGit(t, repository, "add", "--all") + buildGit(t, repository, "commit", "-m", "note") + parent := t.TempDir() + moved := filepath.Join(parent, "repo") + if err := os.Rename(repository, moved); err != nil { + t.Fatal(err) + } + if err := os.Chmod(parent, 0555); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(parent, 0700) }) + _, err := releasekit.Check(context.Background(), releasekit.ReleaseRequest{RepositoryDir: moved, Product: "Fixture", Version: "v1.2.3", Validate: func(context.Context, releasekit.Candidate) error { return nil }, Build: &releasekit.BuildSpec{MainPackage: "./cmd/app", BinaryName: "fixture", Targets: []releasekit.Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}}}) + if err != nil { + t.Fatal(err) + } + if status := buildGit(t, moved, "status", "--porcelain"); status != "" { + t.Fatalf("candidate dirtied: %s", status) + } +} diff --git a/cmd/releasekit/internal/cli/config.go b/cmd/releasekit/internal/cli/config.go index 8c6863b..71c1e35 100644 --- a/cmd/releasekit/internal/cli/config.go +++ b/cmd/releasekit/internal/cli/config.go @@ -14,6 +14,7 @@ import ( "strings" "gitea.maximumdirect.net/eric/ReleaseKit/internal/process" + "gitea.maximumdirect.net/eric/ReleaseKit/internal/tempdir" ) const Usage = "usage: releasekit version\n releasekit check [--repo DIR] [--config FILE] VERSION\n releasekit publish [--repo DIR] [--config FILE] VERSION\n releasekit build [--repo DIR] [--config FILE] [--output DIR] VERSION\n releasekit deliver [--repo DIR] [--config FILE] [--assets DIR] VERSION" @@ -202,3 +203,8 @@ func RunChecks(ctx context.Context, repoDir string, checks []CheckCommand, owned } return nil } + +// TemporaryBuildDir allocates command-owned storage outside its source checkout. +func TemporaryBuildDir(repository string) (string, error) { + return tempdir.Outside(repository, "releasekit-delivery-") +} diff --git a/cmd/releasekit/main.go b/cmd/releasekit/main.go index 0ca5977..f2f1d7c 100644 --- a/cmd/releasekit/main.go +++ b/cmd/releasekit/main.go @@ -197,7 +197,7 @@ func runDelivery(ctx context.Context, invocation cli.Invocation, stdout, stderr fmt.Fprintf(stderr, "resolve repository directory: %v\n", resolveErr) return 1 } - temporaryDir, err = os.MkdirTemp(filepath.Dir(repositoryDir), ".releasekit-delivery-") + temporaryDir, err = cli.TemporaryBuildDir(repositoryDir) if err != nil { fmt.Fprintf(stderr, "create temporary build directory: %v\n", err) return 1 diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index 9256f07..0fa9bfb 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -762,3 +762,33 @@ func main(){fmt.Println("fixture",buildinfo.Version())} t.Fatalf("stale receipt remains: %v", err) } } + +func TestTemporaryDeliveryBuildDoesNotRequireWritableRepositoryParent(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("requires ordinary Unix permission checks") + } + repository := newDeliveryBuildRepository(t) + state := &deliveryServerState{} + server := newDeliveryServer(t, state) + defer server.Close() + writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}}, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}}) + state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD") + cliGitRun(t, repository, "tag", "v1.2.3") + parent := t.TempDir() + moved := filepath.Join(parent, "repo") + if err := os.Rename(repository, moved); err != nil { + t.Fatal(err) + } + if err := os.Chmod(parent, 0555); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(parent, 0700) }) + t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token") + var stdout, stderr bytes.Buffer + if status := run(context.Background(), []string{"deliver", "--repo", moved, "v1.2.3"}, &stdout, &stderr); status != 0 { + t.Fatalf("status=%d %s", status, stderr.String()) + } + if status := cliGitOutput(t, moved, "status", "--porcelain"); status != "" { + t.Fatalf("source dirtied: %s", status) + } +} diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 93c7bf5..ff72890 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -39,6 +39,7 @@ these implemented packages: | `internal/delivery` | Tagged-checkout verification, artifact inventory/hash validation, manifest assembly, and coordination of hosted delivery results. | | `internal/git` | Git command boundary, push-destination resolution, exact ref inspection, and tag operations. | | `internal/gitea` | Gitea API transport, hosted release pages, asset inspection, manifests, and uploads. | +| `internal/tempdir` | Disposable system temporary storage outside source worktrees. | | `internal/process` | Cancellable child-process execution with per-command environment overlays. | | `internal/publish` | Publication guards, one validation run, annotated tag creation, push, and recovery state. | | `internal/runtimeinfo` | Private version lookup used by the runtime facade. | diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index a62c16c..89d1138 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -230,6 +230,8 @@ offline suite, and compiled-export/import checks pass. ### R6 — Medium: Builds require a writable parent of the repository +Status: Resolved. + Category: Inadvertently strengthened filesystem prerequisite. **Evidence:** [Candidate builds](../../internal/build/candidate.go), around line 21, @@ -250,6 +252,11 @@ adding broad output-directory restrictions. read-only repository parent and a usable system temporary directory. Generated candidate output still leaves the worktree clean. +**Resolution:** Candidate and CLI delivery builds use shared system temporary +storage allocated by `internal/tempdir`, with worktree containment checks and +caller-owned cleanup. Regressions execute both operations with read-only +repository parents and confirm clean worktrees. Affected suites pass. + ### R7 — Medium: Attachment origin restrictions exceed credential isolation Category: Inadvertently strengthened prohibition. diff --git a/internal/build/candidate.go b/internal/build/candidate.go index 828c09f..16c6461 100644 --- a/internal/build/candidate.go +++ b/internal/build/candidate.go @@ -5,10 +5,9 @@ import ( "fmt" "io" "os" - "path/filepath" - "strings" "gitea.maximumdirect.net/eric/ReleaseKit/internal/candidate" + "gitea.maximumdirect.net/eric/ReleaseKit/internal/tempdir" ) func CandidateCallback(spec *Spec, output io.Writer) func(context.Context, candidate.Candidate) error { @@ -18,18 +17,11 @@ func CandidateCallback(spec *Spec, output io.Writer) func(context.Context, candi selected := *spec selected.Targets = append([]Target(nil), spec.Targets...) return func(ctx context.Context, captured candidate.Candidate) error { - temporaryDir, err := os.MkdirTemp(filepath.Dir(captured.RepositoryDir), ".releasekit-candidate-build-") + temporaryDir, err := tempdir.Outside(captured.RepositoryDir, "releasekit-candidate-build-") if err != nil { return fmt.Errorf("create temporary candidate build output: %w", err) } defer os.RemoveAll(temporaryDir) - relative, err := filepath.Rel(captured.RepositoryDir, temporaryDir) - if err != nil { - return fmt.Errorf("verify temporary candidate build output: %w", err) - } - if relative == "." || relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) { - return fmt.Errorf("temporary candidate build output %q is inside the repository", temporaryDir) - } _, err = Build(ctx, Request{ RepositoryDir: captured.RepositoryDir, Version: captured.Version, diff --git a/internal/tempdir/tempdir.go b/internal/tempdir/tempdir.go new file mode 100644 index 0000000..0eed265 --- /dev/null +++ b/internal/tempdir/tempdir.go @@ -0,0 +1,33 @@ +// Package tempdir allocates disposable storage outside a source worktree. +package tempdir + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// Outside uses the system temporary directory and leaves cleanup to its caller. +func Outside(repositoryDir, pattern string) (string, error) { + directory, err := os.MkdirTemp("", pattern) + if err != nil { + return "", err + } + repository, err := filepath.Abs(repositoryDir) + if err == nil && !strings.EqualFold(filepath.VolumeName(repository), filepath.VolumeName(directory)) { + return directory, nil + } + if err == nil { + var relative string + relative, err = filepath.Rel(repository, directory) + if err == nil && (relative == "." || relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator))) { + err = fmt.Errorf("temporary storage %q is inside repository %q; select an external temporary directory", directory, repository) + } + } + if err != nil { + os.RemoveAll(directory) + return "", err + } + return directory, nil +}