From a2d75c462c51b01aedce7b44f7b5bdbea40aaa68 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 11:36:40 +0000 Subject: [PATCH] Preserve child working directory in environment overlays --- docs/roadmap/remediation.md | 9 +++++- internal/process/process.go | 10 +++---- internal/process/process_test.go | 49 ++++++++++++++++++++++++++++++++ publish_test.go | 44 ++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 69fa4b8..a3377df 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -61,9 +61,16 @@ result. This repair requires no new credential restrictions or public API. ## R02. Original-object Git overlays retain the caller's PWD -**Priority:** P2. **Status:** Pending. This concerns the interaction of A02/A12 +**Priority:** P2. **Status:** Complete. This addressed the interaction of A02/A12 with the existing process environment helper and native Git hooks. +**Completion:** Child overlays now start from `Cmd.Environ()` after setting +`Dir`. Directory-derived `PWD`, explicit overrides/removals, and parent-state +preservation have process regressions; a native Go pre-push hook verifies public +publication from another directory. Both new regressions failed before the fix +and pass afterward. Process, Go-tool, and Git suites passed under `-race`; +public hook and original-object source/build eligibility regressions passed. + **Relevant code:** `run` in [internal/git/remote.go](../../internal/git/remote.go), lines 233–240; `newCommand` and `environmentEntries` in diff --git a/internal/process/process.go b/internal/process/process.go index 0a89ead..1f1b600 100644 --- a/internal/process/process.go +++ b/internal/process/process.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "os" "os/exec" "sort" "strings" @@ -151,14 +150,15 @@ func newCommand(ctx context.Context, invocation Invocation) (*exec.Cmd, error) { command.Dir = invocation.Dir command.WaitDelay = time.Second if len(invocation.Env) != 0 || len(invocation.UnsetEnv) != 0 { - command.Env = environmentEntries(invocation.Env, invocation.UnsetEnv) + // Dir contributes PWD on POSIX before explicit overrides are applied. + command.Env = environmentEntries(command.Environ(), invocation.Env, invocation.UnsetEnv) } return command, nil } -func environmentEntries(overlay map[string]string, unset []string) []string { - environment := make(map[string]string, len(os.Environ())+len(overlay)) - for _, entry := range os.Environ() { +func environmentEntries(base []string, overlay map[string]string, unset []string) []string { + environment := make(map[string]string, len(base)+len(overlay)) + for _, entry := range base { key, value, ok := strings.Cut(entry, "=") if ok { environment[key] = value diff --git a/internal/process/process_test.go b/internal/process/process_test.go index 7a35167..db1d6b5 100644 --- a/internal/process/process_test.go +++ b/internal/process/process_test.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -79,6 +80,50 @@ func TestRunCanRemoveInheritedEnvironment(t *testing.T) { } } +func TestRunDirectoryEnvironmentPreservesExplicitPWDSelection(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("directory-derived PWD is a POSIX behavior") + } + parent := mustGetwd(t) + t.Setenv("PWD", parent) + workdir := t.TempDir() + for _, test := range []struct { + name, override, want string + unset bool + present bool + }{ + {name: "directory", want: workdir, present: true}, + {name: "override", override: "selected PWD", want: "selected PWD", present: true}, + {name: "removed", unset: true}, + {name: "override after removal", unset: true, override: "selected PWD", want: "selected PWD", present: true}, + } { + t.Run(test.name, func(t *testing.T) { + invocation := Invocation{ + Args: []string{helperExecutable(t), "-test.run=TestProcessHelper", "--", "pwd"}, + Dir: workdir, + Env: map[string]string{"RELEASEKIT_PROCESS_HELPER": "1"}, + } + if test.override != "" { + invocation.Env["PWD"] = test.override + } + if test.unset { + invocation.UnsetEnv = []string{"PWD"} + } + var stdout bytes.Buffer + invocation.Stdout = &stdout + if err := Run(t.Context(), invocation); err != nil { + t.Fatal(err) + } + if got, want := stdout.String(), fmt.Sprintf("%s|%t", test.want, test.present); got != want { + t.Fatalf("child PWD = %q, want %q", got, want) + } + if os.Getenv("PWD") != parent || mustGetwd(t) != parent { + t.Fatal("child invocation changed the parent's directory or PWD") + } + }) + } +} + func TestRunReportsMissingExecutable(t *testing.T) { err := Run(context.Background(), Invocation{Args: []string{"releasekit-command-that-does-not-exist"}}) if err == nil || !strings.Contains(err.Error(), "releasekit-command-that-does-not-exist") { @@ -378,6 +423,10 @@ func TestProcessHelper(t *testing.T) { } mode := args[len(args)-1] switch mode { + case "pwd": + value, present := os.LookupEnv("PWD") + fmt.Fprintf(os.Stdout, "%s|%t", value, present) + os.Exit(0) case "success": workdir, err := os.Getwd() if err != nil { diff --git a/publish_test.go b/publish_test.go index fb28a34..e6e539c 100644 --- a/publish_test.go +++ b/publish_test.go @@ -14,10 +14,54 @@ import ( releasekit "go.maximumdirect.net/releasekit" "go.maximumdirect.net/releasekit/internal/git" + "go.maximumdirect.net/releasekit/internal/gotool" ) const releaseNote = "# Sample App v1.2.3\n\nA source release.\n\nUpgrade notes stay intact.\n" +func TestPublishPreservesHookDirectoryEnvironment(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("directory-derived PWD is a POSIX behavior") + } + repo := newPublishFixture(t) + marker := filepath.Join(t.TempDir(), "hook-ran") + t.Setenv("RELEASEKIT_HOOK_MARKER", marker) + helperDir := t.TempDir() + writeFile(t, helperDir, "main.go", `package main +import "os" +func main() { + cwd, err := os.Getwd() + if err != nil || os.Getenv("PWD") != cwd { + os.Stderr.WriteString("pre-push PWD does not identify the selected repository\n") + os.Exit(1) + } + if err := os.WriteFile(os.Getenv("RELEASEKIT_HOOK_MARKER"), []byte("checked"), 0600); err != nil { + os.Exit(2) + } +} +`) + hooks := filepath.Join(repo.work, ".git", "hooks") + if err := os.MkdirAll(hooks, 0o700); err != nil { + t.Fatal(err) + } + gitRun(t, repo.work, "config", "core.hooksPath", hooks) + tool, err := gotool.New(t.Context(), repo.work) + if err != nil { + t.Fatal(err) + } + if err := tool.Run(t.Context(), repo.work, gotool.Static, tool.HostTarget(), nil, + "build", "-o", filepath.Join(hooks, "pre-push"), filepath.Join(helperDir, "main.go")); err != nil { + t.Fatal(err) + } + result, err := releasekit.Publish(t.Context(), releasekit.ReleaseRequest{RepositoryDir: repo.work, Product: "Sample App", Version: "v1.2.3"}) + if err != nil || result.State != releasekit.PublicationPublished { + t.Fatalf("publication with directory-aware hook = %#v, %v", result, err) + } + if data, err := os.ReadFile(marker); err != nil || string(data) != "checked" { + t.Fatalf("pre-push hook observation = %q, %v", data, err) + } +} + func TestCheckRejectsInvalidDerivedTagBeforeValidation(t *testing.T) { for _, module := range []string{"module space", "module.lock"} { t.Run(module, func(t *testing.T) {