diff --git a/.github/wiki/Compat-V15.md b/.github/wiki/Compat-V15.md index a5c96fdcd..840f90936 100644 --- a/.github/wiki/Compat-V15.md +++ b/.github/wiki/Compat-V15.md @@ -73,6 +73,7 @@ The table is generated. Never hand-edit it; run `compat-markers.sh --write-wiki` | D-0676 | bare stream path streams as path:// with a notice | refused | `internal/backup/stream_dest_compat.go` | | D-0685 | credential guard before E138 for a missing minio plugin | guard only for plugin-backed MinIO | `internal/config/validator_ports_services.go` | | D-0686 | S3_SECRET_KEY accepted at any strength | must be at least 16 characters when object storage is enabled | `internal/config/validator.go` | +| D-0737 | legacy MCP nself_config_set refuses secret keys | the generated gated v1.5 tool handles them | `internal/config/secretkeys.go` | | D-0752 | plugin disable never confirmed | prod-class and hand-edit confirmation before the marker is written. | `cmd/commands/plugin_disable_gate.go` | | D-0757 | nginx/sites may be a symlink out of the project and the build writes through it | v1.5 refuses a project's own nginx/sites that resolves outside the project | `internal/build/nginx_sites_dir.go` | | D-0843 | a downgrade only warns | refused unless --allow-downgrade is given | `internal/plugin/installer_downgrade.go` | diff --git a/cmd/commands/license_migrate_api.go b/cmd/commands/license_migrate_api.go index f999f381b..4265d732b 100644 --- a/cmd/commands/license_migrate_api.go +++ b/cmd/commands/license_migrate_api.go @@ -80,7 +80,7 @@ func fetchMigrationStatus(ctx context.Context, key string, pingURL string) (*mig resp, err := httptimeout.License.Do(req) if err != nil { - return nil, err + return nil, httptimeout.StripURL(err) } defer func() { _ = resp.Body.Close() }() diff --git a/cmd/commands/license_migrate_api_test.go b/cmd/commands/license_migrate_api_test.go new file mode 100644 index 000000000..42b35a765 --- /dev/null +++ b/cmd/commands/license_migrate_api_test.go @@ -0,0 +1,47 @@ +package commands + +// Purpose: fetchMigrationStatus keeps the key out of the request URL (it goes +// in X-License-Key) and never lets the key or the endpoint escape through a +// transport error, which net/http otherwise builds from the full request URL. +// Inputs: a synthetic key and httptest servers. Outputs: pass/fail. +// Constraints: loopback only. + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +const migrateTestKey = "synthetic key&x=1#frag" + +func TestFetchMigrationStatusEncodesKey(t *testing.T) { + var gotHeader, gotPath, gotQuery string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotHeader, gotPath, gotQuery = r.Header.Get("X-License-Key"), r.URL.Path, r.URL.RawQuery + _, _ = w.Write([]byte(`{}`)) + })) + defer srv.Close() + if _, err := fetchMigrationStatus(context.Background(), migrateTestKey, srv.URL); err != nil { + t.Fatal(err) + } + if gotPath != "/license/migration-status" || gotQuery != "" || gotHeader != migrateTestKey { + t.Errorf("server saw path %q query %q header %q", gotPath, gotQuery, gotHeader) + } +} + +func TestFetchMigrationStatusTransportErrorOmitsKey(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + url := srv.URL + srv.Close() // connection refused + _, err := fetchMigrationStatus(context.Background(), migrateTestKey, url) + if err == nil { + t.Fatal("want a transport error") + } + for _, leak := range []string{migrateTestKey, "synthetic", "key=", "migration-status"} { + if strings.Contains(err.Error(), leak) { + t.Errorf("transport error leaks %q: %v", leak, err) + } + } +} diff --git a/cmd/commands/license_refresh_down_test.go b/cmd/commands/license_refresh_down_test.go new file mode 100644 index 000000000..677023c0a --- /dev/null +++ b/cmd/commands/license_refresh_down_test.go @@ -0,0 +1,28 @@ +package commands + +// Purpose: `license refresh` against an unreachable ping_api prints only the +// fixed failure code, never the key, its mask, its hash or the request URL. +// Inputs: the grace fixture with its server shut down. Outputs: pass/fail. +// Constraints: loopback only. + +import ( + "strings" + "testing" + + "github.com/nself-org/cli/internal/errs" + "github.com/nself-org/cli/internal/license" +) + +func TestLicenseRefreshTransportFailureIsFixedCode(t *testing.T) { + f := newGraceFixture(t) + t.Setenv("NSELF_PING_API_URL", "http://127.0.0.1:1") // nothing listens + stdout, stderr, err := runRefresh(t, f.project, map[string]string{"project": f.project, "quiet": "true"}) + if stdout != "" || stderr != "entitlement refresh failed: ping-unavailable\n" || errs.ExitCodeFor(err) != 1 { + t.Fatalf("stdout %q stderr %q exit %d", stdout, stderr, errs.ExitCodeFor(err)) + } + for _, s := range []string{graceKey, license.MaskKey(graceKey), license.HashKey(graceKey), "127.0.0.1"} { + if strings.Contains(stdout+stderr, s) { + t.Errorf("output contains %q", s) + } + } +} diff --git a/cmd/commands/mcp_tools_mutate.go b/cmd/commands/mcp_tools_mutate.go index 505864ea3..d7b8d8c66 100644 --- a/cmd/commands/mcp_tools_mutate.go +++ b/cmd/commands/mcp_tools_mutate.go @@ -24,11 +24,13 @@ package commands import ( "context" + "errors" "fmt" "strings" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" + "github.com/nself-org/cli/internal/config" "github.com/nself-org/cli/internal/plugin" ) @@ -78,7 +80,7 @@ func mcpConfigSetHandler() server.ToolHandlerFunc { value, _ := args["value"].(string) envFlag, _ := args["env"].(string) - if err := validateConfigKey(key); err != nil { + if err := errors.Join(validateConfigKey(key), config.RefuseLegacyMCPSecretSet(key)); err != nil { // D-0737 return mcpErrorResult("Error: %v", err) } if err := validateConfigValue(value); err != nil { @@ -97,9 +99,7 @@ func mcpConfigSetHandler() server.ToolHandlerFunc { if err != nil { return mcpErrorResult("Error: %v", err) } - return mcp.NewToolResultStructuredOnly(ConfigSetResult{ - Key: key, File: envFile, Created: !updated, - }), nil + return mcp.NewToolResultStructuredOnly(ConfigSetResult{Key: key, File: envFile, Created: !updated}), nil } } diff --git a/cmd/commands/mcp_tools_mutate_secret_test.go b/cmd/commands/mcp_tools_mutate_secret_test.go new file mode 100644 index 000000000..ce8d262e9 --- /dev/null +++ b/cmd/commands/mcp_tools_mutate_secret_test.go @@ -0,0 +1,78 @@ +package commands + +// Purpose: proves the v1.4 legacy nself_config_set tool refuses secret keys +// (D-0737) and still writes ordinary keys; in v1.5 the generated gated tool +// replaces it, so the legacy guard does not apply there. +// Inputs: synthetic keys and a temp project. Outputs: pass/fail. +// Constraints: no network, temp dirs only. + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mark3labs/mcp-go/mcp" + + "github.com/nself-org/cli/internal/compat/compattest" +) + +func TestMCPConfigSetRefusesSecretKeyInLegacyMode(t *testing.T) { + compattest.Set(t, false) + dir := t.TempDir() + envPath := filepath.Join(dir, ".env") + if err := os.WriteFile(envPath, []byte("PROJECT_NAME=demo\n"), 0o600); err != nil { + t.Fatal(err) + } + defer chdir(t, dir)() + + for _, key := range []string{"POSTGRES_PASSWORD", "HASURA_GRAPHQL_ADMIN_SECRET", "S3_SECRET_KEY", "BASIC_AUTH"} { + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{"key": key, "value": "synthetic-value"} + res, err := mcpConfigSetHandler()(context.Background(), req) + if err != nil { + t.Fatal(err) + } + text := res.Content[0].(mcp.TextContent).Text + if !strings.Contains(text, "requires the v1.5 gated tool") { + t.Errorf("%s: want a secret-key refusal, got %q", key, text) + } + } + got, _ := os.ReadFile(envPath) + if string(got) != "PROJECT_NAME=demo\n" { + t.Errorf(".env changed by a refused call: %q", got) + } + + // A non-secret key still passes through. + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{"key": "ABC", "value": "v"} + if res, err := mcpConfigSetHandler()(context.Background(), req); err != nil || !isConfigSetOK(res) { + t.Fatalf("non-secret key refused: %v %#v", err, res) + } +} + +func TestMCPConfigSetSecretGuardIsLegacyOnly(t *testing.T) { + compattest.Set(t, true) + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, ".env"), []byte("PROJECT_NAME=demo\n"), 0o600); err != nil { + t.Fatal(err) + } + defer chdir(t, dir)() + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{"key": "POSTGRES_PASSWORD", "value": "synthetic-value"} + res, err := mcpConfigSetHandler()(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if !isConfigSetOK(res) { + t.Errorf("v1.5 hit the legacy guard: %#v", res) + } +} + +// isConfigSetOK reports whether the handler took the success path. +func isConfigSetOK(res *mcp.CallToolResult) bool { + _, ok := res.StructuredContent.(ConfigSetResult) + return ok +} diff --git a/cmd/commands/migrate_supabase_v15_test.go b/cmd/commands/migrate_supabase_v15_test.go index b89fe52e8..1913fefa2 100644 --- a/cmd/commands/migrate_supabase_v15_test.go +++ b/cmd/commands/migrate_supabase_v15_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "regexp" "strings" "testing" @@ -15,6 +16,10 @@ import ( "github.com/spf13/pflag" ) +// generatedStamp matches the one-second-resolution timestamp the generator embeds in +// supabase-migration.sql; two Generate calls straddling a second boundary differ only there. +var generatedStamp = regexp.MustCompile(`(?m)^-- Generated: \d+$`) + func resetSupabaseFlags(t *testing.T) { t.Helper() migrateSupabaseCmd.Flags().VisitAll(func(f *pflag.Flag) { @@ -91,7 +96,7 @@ func TestMigrateSupabaseModes(t *testing.T) { want := spmigrate.Generate("abcdefghijklmnopqrst", &spmigrate.PullResult{Tables: []spmigrate.Table{{Schema: "public", Name: "notes"}}}) for name, content := range map[string]string{"supabase-migration.sql": want.MigrationSQL, "hasura-metadata.yaml": want.HasuraMetadata, "import-auth-users.sh": want.AuthImportScript} { got, rerr := os.ReadFile(filepath.Join(out, name)) - if rerr != nil || string(got) != content { + if rerr != nil || generatedStamp.ReplaceAllString(string(got), "-- Generated: ") != generatedStamp.ReplaceAllString(content, "-- Generated: ") { t.Fatalf("v1.4 artifact %s differs from the generator output (err=%v)", name, rerr) } } diff --git a/internal/config/secretkeys.go b/internal/config/secretkeys.go index e1e00f26f..370905c24 100644 --- a/internal/config/secretkeys.go +++ b/internal/config/secretkeys.go @@ -11,8 +11,10 @@ package config // side: a match only redacts. internal/env must not keep a second list. import ( + "fmt" "strings" + "github.com/nself-org/cli/internal/compat" "github.com/nself-org/cli/internal/credmask" ) @@ -33,6 +35,19 @@ func IsSecretKey(key string) bool { return upper == "AUTH" || strings.HasSuffix(upper, "_AUTH") } +// RefuseLegacyMCPSecretSet is the D-0737 guard of the v1.4 nself_config_set +// MCP tool, which skips the invoke.Gate round trip of the generated v1.5 tool: +// in v1.4 mode it refuses secret keys so a token-less loopback client cannot +// write POSTGRES_PASSWORD or any other secret unconfirmed. Nil in v1.5 mode, +// where the generated gated tool replaces the legacy one (P7-SURF-03). +func RefuseLegacyMCPSecretSet(key string) error { + // compat.V15(D-0737): legacy MCP nself_config_set refuses secret keys -> the generated gated v1.5 tool handles them + if compat.V15() || !IsSecretKey(key) { + return nil + } + return fmt.Errorf("setting secret key %q requires the v1.5 gated tool; upgrade or use `nself config set` interactively", key) +} + // MaskURLCredentials masks the credentials of a value that holds a URL, a Go // DSN, a bare user:pass@host or password=/pwd= pairs. The rules and their // bounds are documented on credmask.URL. diff --git a/internal/docker/compose.go b/internal/docker/compose.go index e05c65afb..59548d3fa 100644 --- a/internal/docker/compose.go +++ b/internal/docker/compose.go @@ -93,7 +93,10 @@ func (c *Compose) ComposeDown(ctx context.Context, workdir string, opts DownOpti if opts.Timeout > 0 { args = append(args, "-t", fmt.Sprintf("%d", opts.Timeout)) } - return c.Run(ctx, workdir, args...) + if err := c.Run(ctx, workdir, args...); err != nil || !opts.RemoveVolumes || VolumesRemovedHook == nil { + return err + } + return VolumesRemovedHook(ctx, workdir) } // ComposeUpNoDeps runs `docker compose up -d --no-deps [services...]`. diff --git a/internal/docker/compose_hooks.go b/internal/docker/compose_hooks.go index 921f74ebf..e18139d2d 100644 --- a/internal/docker/compose_hooks.go +++ b/internal/docker/compose_hooks.go @@ -76,3 +76,11 @@ func RunPreDownHooks(ctx context.Context, workdir string) (release func()) { } } } + +// VolumesRemovedHook, when set at init, runs after a successful +// `docker compose down -v` in workdir and its error fails ComposeDown. The +// postgres image switch sets it (internal/postgres/imageswitch cannot be +// imported from here) to remove the old data volume a switch kept, which the +// current compose file no longer names, so reset, stop --volumes and +// uninstall --purge really wipe the project's data (D-0802). +var VolumesRemovedHook func(ctx context.Context, workdir string) error diff --git a/internal/docker/compose_hooks_test.go b/internal/docker/compose_hooks_test.go index f8cf17111..83161ddd2 100644 --- a/internal/docker/compose_hooks_test.go +++ b/internal/docker/compose_hooks_test.go @@ -2,6 +2,7 @@ package docker import ( "context" + "errors" "os" "path/filepath" "runtime" @@ -81,3 +82,40 @@ func TestComposeDownRemoveImages(t *testing.T) { t.Errorf("args = %q, want %q", got, want) } } + +func TestVolumesRemovedHook(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake docker is a POSIX shell script") + } + dir := t.TempDir() + fail := filepath.Join(dir, "fail") + script := "#!/bin/sh\n[ -e '" + fail + "' ] && exit 1\nexit 0\n" + bin := filepath.Join(dir, "docker") + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + c := &Compose{DockerPath: bin} + var calls []string + hookErr := error(nil) + old := VolumesRemovedHook + VolumesRemovedHook = func(_ context.Context, workdir string) error { calls = append(calls, workdir); return hookErr } + t.Cleanup(func() { VolumesRemovedHook = old }) + + if err := c.ComposeDown(context.Background(), dir, DownOptions{}); err != nil || len(calls) != 0 { + t.Fatalf("down without -v: err %v calls %v", err, calls) + } + if err := c.ComposeDown(context.Background(), dir, DownOptions{RemoveVolumes: true}); err != nil || !slices.Equal(calls, []string{dir}) { + t.Fatalf("down -v: err %v calls %v", err, calls) + } + hookErr = errors.New("volume is in use") + if err := c.ComposeDown(context.Background(), dir, DownOptions{RemoveVolumes: true}); err == nil || !strings.Contains(err.Error(), "in use") { + t.Fatalf("a hook failure must fail the down, got %v", err) + } + calls = nil + if err := os.WriteFile(fail, nil, 0o600); err != nil { + t.Fatal(err) + } + if err := c.ComposeDown(context.Background(), dir, DownOptions{RemoveVolumes: true}); err == nil || len(calls) != 0 { + t.Fatalf("hook ran after a failed down: err %v calls %v", err, calls) + } +} diff --git a/internal/docker/volume.go b/internal/docker/volume.go index edfaa0ab2..defbc630d 100644 --- a/internal/docker/volume.go +++ b/internal/docker/volume.go @@ -111,3 +111,30 @@ func VolumeLabels(ctx context.Context, name string) (labels map[string]string, e } return labels, true, nil } + +// VolumeListByLabels returns the names of the volumes that carry every given +// label filter ("key" or "key=value"; docker ANDs repeated filters). An empty +// result is not an error. Docker daemon unavailability is returned as an error +// so callers can distinguish "no matches" from "cannot list". With no filters +// it returns nothing, never every volume on the host. +func VolumeListByLabels(ctx context.Context, labels ...string) ([]string, error) { + if len(labels) == 0 { + return nil, nil + } + args := []string{"volume", "ls"} + for _, l := range labels { + args = append(args, "--filter", "label="+l) + } + out, err := runCapture(ctx, append(args, "--format", "{{.Name}}")...) + if err != nil { + return nil, err + } + var names []string + for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + line = strings.TrimSpace(line) + if line != "" { + names = append(names, line) + } + } + return names, nil +} diff --git a/internal/docker/volume_test.go b/internal/docker/volume_test.go index 0d999e194..9603a7463 100644 --- a/internal/docker/volume_test.go +++ b/internal/docker/volume_test.go @@ -141,3 +141,33 @@ func TestVolumeLabels(t *testing.T) { t.Errorf("a volume with no labels: %v %v %v", got, ok, err) } } + +func TestVolumeListByLabelsAndsFilters(t *testing.T) { + log := fakeVolumeDocker(t, "a\n\n b \n", "") + got, err := VolumeListByLabels(context.Background(), "org.nself.image-switch", "com.docker.compose.project=demo") + if err != nil || !reflect.DeepEqual(got, []string{"a", "b"}) { + t.Fatalf("got %v, %v", got, err) + } + want := "volume ls --filter label=org.nself.image-switch --filter label=com.docker.compose.project=demo --format {{.Name}}" + if c := lastCall(t, log); c != want { + t.Errorf("call = %q, want %q", c, want) + } +} + +func TestVolumeListByLabelsNoFiltersListsNothing(t *testing.T) { + log := fakeVolumeDocker(t, "every\nvolume\n", "") + got, err := VolumeListByLabels(context.Background()) + if err != nil || got != nil { + t.Fatalf("got %v, %v", got, err) + } + if _, statErr := os.Stat(log); statErr == nil { + t.Error("docker was called with no filters") + } +} + +func TestVolumeListByLabelsReturnsDaemonError(t *testing.T) { + fakeVolumeDocker(t, "", "Cannot connect to the Docker daemon") + if _, err := VolumeListByLabels(context.Background(), "x"); err == nil { + t.Fatal("a daemon failure must be an error, not an empty list") + } +} diff --git a/internal/httptimeout/strip.go b/internal/httptimeout/strip.go new file mode 100644 index 000000000..c325c1395 --- /dev/null +++ b/internal/httptimeout/strip.go @@ -0,0 +1,19 @@ +package httptimeout + +import ( + "errors" + "net/url" +) + +// StripURL returns the cause of a *url.Error and err itself otherwise. +// +// Purpose: net/http quotes the whole request URL in a transport error, so a +// credential sent in a query string would reach logs and terminals. +// Inputs: an error from http.Client.Do. Outputs: the same error without the URL. +func StripURL(err error) error { + var ue *url.Error + if errors.As(err, &ue) { + return ue.Err + } + return err +} diff --git a/internal/httptimeout/strip_test.go b/internal/httptimeout/strip_test.go new file mode 100644 index 000000000..b5b5468f7 --- /dev/null +++ b/internal/httptimeout/strip_test.go @@ -0,0 +1,26 @@ +package httptimeout + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestStripURLDropsTheRequestURL(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + target := srv.URL + "/p?key=synthetic-secret" + srv.Close() // connection refused + _, err := http.Get(target) + if err == nil || !strings.Contains(err.Error(), "synthetic-secret") { + t.Fatalf("precondition: net/http error should quote the URL, got %v", err) + } + if got := StripURL(err); strings.Contains(got.Error(), "synthetic-secret") || strings.Contains(got.Error(), "/p") { + t.Errorf("StripURL kept the URL: %v", got) + } + plain := errors.New("plain") + if StripURL(plain) != plain || StripURL(nil) != nil { + t.Error("non-URL errors and nil must pass through") + } +} diff --git a/internal/postgres/imageswitch/leftover.go b/internal/postgres/imageswitch/leftover.go new file mode 100644 index 000000000..d97e9a00f --- /dev/null +++ b/internal/postgres/imageswitch/leftover.go @@ -0,0 +1,75 @@ +package imageswitch + +// leftover.go — the data volumes a finished switch leaves behind. +// +// Purpose: after a switch the compose file names only the new volume, so +// `compose down --volumes` (reset, stop --volumes, uninstall --purge) cannot +// remove the kept old volume or the volumes of earlier switches. This finds +// them, scoped to this project, so those commands can wipe all project data. +// Inputs: the project directory (its switch state names the compose project). +// Outputs: volume names; none when the project never switched. +// Constraints: never lists volumes of another compose project; a volume whose +// compose project label names a different project is skipped; list errors are +// returned, never swallowed. + +import ( + "context" + "errors" + "fmt" + "slices" + + "github.com/nself-org/cli/internal/docker" +) + +func init() { + docker.VolumesRemovedHook = func(ctx context.Context, workdir string) error { + _, err := RemoveLeftovers(ctx, workdir) + return err + } +} + +const labelComposeProject = "com.docker.compose.project" + +// LeftoverVolumes returns the project's data volumes that a switch kept: the +// old volume recorded in the state, and every volume of the same compose +// project carrying the switch label (earlier switches' new volumes). +func LeftoverVolumes(ctx context.Context, projectDir string) ([]string, error) { + st, err := LoadState(projectDir) + if err != nil || st == nil || st.ComposeProject == "" { + return nil, err + } + names, err := docker.VolumeListByLabels(ctx, labelRun, labelComposeProject+"="+st.ComposeProject) + if err != nil { + return nil, err + } + if old := st.OldVolume; old != "" && !slices.Contains(names, old) { + labels, exists, err := docker.VolumeLabels(ctx, old) + if err != nil { + return nil, err + } + if p := labels[labelComposeProject]; exists && (p == "" || p == st.ComposeProject) { + names = append(names, old) + } + } + return names, nil +} + +// RemoveLeftovers removes LeftoverVolumes and returns the names removed. A list +// or removal failure is returned (all removals are still attempted), so the +// caller never reports the data as gone when it is not. +func RemoveLeftovers(ctx context.Context, projectDir string) ([]string, error) { + names, err := LeftoverVolumes(ctx, projectDir) + if err != nil { + return nil, fmt.Errorf("listing image-switch volumes: %w", err) + } + var removed []string + var errs []error + for _, v := range names { + if err := docker.VolumeRemove(ctx, v); err != nil { + errs = append(errs, fmt.Errorf("removing image-switch volume %s: %w", v, err)) + continue + } + removed = append(removed, v) + } + return removed, errors.Join(errs...) +} diff --git a/internal/postgres/imageswitch/leftover_test.go b/internal/postgres/imageswitch/leftover_test.go new file mode 100644 index 000000000..a42f34945 --- /dev/null +++ b/internal/postgres/imageswitch/leftover_test.go @@ -0,0 +1,164 @@ +package imageswitch + +// Purpose: D-0802. RemoveLeftovers removes the old volume a switch kept +// and the project's switch-labelled volumes, leaves every other project's +// volumes alone, and returns list/remove failures instead of swallowing them. +// Inputs: a fake docker that filters a directory of volume label files the +// way `docker volume ls --filter label=...` does, and a state.json. +// Outputs: pass/fail. Constraints: POSIX shell, temp dirs only. + +import ( + "context" + "os" + "path/filepath" + "runtime" + "sort" + "strings" + "testing" + + "github.com/nself-org/cli/internal/docker" +) + +const fakeVolumeDockerScript = `#!/bin/sh +D="$FAKE_VOLS" +echo "$*" >> "$D/calls.log" +[ -n "$FAKE_LS_FAIL" ] && [ "$1 $2" = "volume ls" ] && { echo "Cannot connect to the Docker daemon" >&2; exit 1; } +[ -n "$FAKE_RM_FAIL" ] && [ "$1 $2" = "volume rm" ] && { echo "volume is in use" >&2; exit 1; } +case "$1 $2" in +"volume ls") + shift 2; filters="" + while [ $# -gt 0 ]; do case "$1" in --filter) filters="$filters ${2#label=}"; shift 2;; *) shift;; esac; done + for f in "$D"/*.labels; do + [ -e "$f" ] || continue + name=$(basename "$f" .labels); labels=$(cat "$f"); ok=1 + for want in $filters; do + case " $labels " in *" $want "*) ;; *) case "$want" in *=*) ok=0;; *) case " $labels " in *" $want="*) ;; *) ok=0;; esac;; esac;; esac + done + [ $ok = 1 ] && echo "$name" + done ;; +"volume inspect") + for a; do name=$a; done + [ -e "$D/$name.labels" ] || { echo "Error response from daemon: get $name: no such volume" >&2; exit 1; } + printf '{'; sep=; for t in $(cat "$D/$name.labels"); do printf '%s"%s":"%s"' "$sep" "${t%%=*}" "${t#*=}"; sep=,; done; printf '}' ;; +"volume rm") rm -f "$D/$3.labels" ;; +esac +exit 0 +` + +func setupLeftoverFixture(t *testing.T) (project, vols string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("fake docker is a POSIX shell script") + } + bin, vols, project := t.TempDir(), t.TempDir(), t.TempDir() + if err := os.WriteFile(filepath.Join(bin, "docker"), []byte(fakeVolumeDockerScript), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("FAKE_VOLS", vols) + for name, labels := range map[string]string{ + // the kept old volume: compose labels only, no switch label + "alpha_postgres_data": "com.docker.compose.project=alpha com.docker.compose.volume=postgres_data", + // the new volume and an earlier switch's volume + "alpha_postgres_data_pgvector_2": "com.docker.compose.project=alpha org.nself.image-switch=sw-2", + "alpha_postgres_data_pgvector_1": "com.docker.compose.project=alpha org.nself.image-switch=sw-1", + // another project's switched volume and unrelated volumes + "beta_postgres_data_pgvector_1": "com.docker.compose.project=beta org.nself.image-switch=sw-9", + "beta_postgres_data": "com.docker.compose.project=beta com.docker.compose.volume=postgres_data", + "other_data": "", + } { + if err := os.WriteFile(filepath.Join(vols, name+".labels"), []byte(labels), 0o600); err != nil { + t.Fatal(err) + } + } + state := `{"version":1,"run_id":"sw-2","phase":"done","compose_project":"alpha",` + + `"old_volume":"alpha_postgres_data","new_volume":"alpha_postgres_data_pgvector_2"}` + sd := filepath.Join(project, ".nself", "image-switch") + if err := os.MkdirAll(sd, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sd, "state.json"), []byte(state), 0o600); err != nil { + t.Fatal(err) + } + return project, vols +} + +func remainingVolumes(t *testing.T, vols string) []string { + t.Helper() + files, _ := filepath.Glob(filepath.Join(vols, "*.labels")) + var out []string + for _, f := range files { + out = append(out, strings.TrimSuffix(filepath.Base(f), ".labels")) + } + sort.Strings(out) + return out +} + +func TestRemoveSwitchLeftoversIsScopedToTheProject(t *testing.T) { + project, vols := setupLeftoverFixture(t) + removed, err := RemoveLeftovers(context.Background(), project) + if err != nil { + t.Fatal(err) + } + sort.Strings(removed) + want := []string{"alpha_postgres_data", "alpha_postgres_data_pgvector_1", "alpha_postgres_data_pgvector_2"} + if strings.Join(removed, ",") != strings.Join(want, ",") { + t.Errorf("removed %v, want %v", removed, want) + } + left := remainingVolumes(t, vols) + if strings.Join(left, ",") != "beta_postgres_data,beta_postgres_data_pgvector_1,other_data" { + t.Errorf("other projects' volumes were touched: %v", left) + } +} + +func TestRemoveSwitchLeftoversSkipsOldVolumeOfAnotherProject(t *testing.T) { + project, vols := setupLeftoverFixture(t) + // A copied project directory: the state names a volume that now belongs to beta. + if err := os.WriteFile(filepath.Join(vols, "alpha_postgres_data.labels"), []byte("com.docker.compose.project=beta"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := RemoveLeftovers(context.Background(), project); err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(remainingVolumes(t, vols), ","), "alpha_postgres_data,") { + t.Errorf("a volume labelled for another project was removed: %v", remainingVolumes(t, vols)) + } +} + +func TestRemoveSwitchLeftoversNoStateTouchesNothing(t *testing.T) { + _, vols := setupLeftoverFixture(t) + removed, err := RemoveLeftovers(context.Background(), t.TempDir()) + if err != nil || len(removed) != 0 { + t.Fatalf("removed %v, err %v", removed, err) + } + if _, statErr := os.Stat(filepath.Join(vols, "calls.log")); statErr == nil { + t.Error("docker was called for a project that never switched") + } +} + +func TestRemoveSwitchLeftoversReturnsListAndRemoveErrors(t *testing.T) { + project, _ := setupLeftoverFixture(t) + t.Setenv("FAKE_LS_FAIL", "1") + if _, err := RemoveLeftovers(context.Background(), project); err == nil || !strings.Contains(err.Error(), "listing image-switch volumes") { + t.Fatalf("list failure not returned: %v", err) + } + t.Setenv("FAKE_LS_FAIL", "") + t.Setenv("FAKE_RM_FAIL", "1") + removed, err := RemoveLeftovers(context.Background(), project) + if err == nil || len(removed) != 0 || !strings.Contains(err.Error(), "alpha_postgres_data") { + t.Fatalf("remove failure not returned: removed %v err %v", removed, err) + } +} + +// ComposeDown with volumes is what reset, stop --volumes and uninstall --purge +// run; the hook this package registers must make it remove the kept volume. +func TestComposeDownVolumesRemovesSwitchLeftovers(t *testing.T) { + project, vols := setupLeftoverFixture(t) + c := &docker.Compose{DockerPath: "docker"} + if err := c.ComposeDown(context.Background(), project, docker.DownOptions{RemoveVolumes: true, RemoveOrphans: true}); err != nil { + t.Fatal(err) + } + if left := strings.Join(remainingVolumes(t, vols), ","); left != "beta_postgres_data,beta_postgres_data_pgvector_1,other_data" { + t.Errorf("remaining volumes = %s", left) + } +}