diff --git a/cmd/commands/functions_deploy.go b/cmd/commands/functions_deploy.go index 5b4507d16..b2b59e944 100644 --- a/cmd/commands/functions_deploy.go +++ b/cmd/commands/functions_deploy.go @@ -114,7 +114,7 @@ func runFunctionsDeploy(cmd *cobra.Command, args []string) error { // Signal container reload. cfg, _, loadErr := loadHealthConfig() if loadErr == nil { - containerID := fmt.Sprintf("%s_functions", cfg.ProjectName) + containerID := cfg.ServiceContainerName("functions") reloadCtx, cancel := context.WithTimeout(cmd.Context(), 10*time.Second) defer cancel() // nhost/functions watches for a .reload file touch. diff --git a/cmd/commands/functions_logs.go b/cmd/commands/functions_logs.go index 6565c5bba..c5bdb30b9 100644 --- a/cmd/commands/functions_logs.go +++ b/cmd/commands/functions_logs.go @@ -55,7 +55,7 @@ func runFunctionsLogs(cmd *cobra.Command, args []string) error { cfg, _, cfgErr := loadHealthConfig() containerID := "functions" if cfgErr == nil { - containerID = fmt.Sprintf("%s_functions", cfg.ProjectName) + containerID = cfg.ServiceContainerName("functions") } dockerArgs := []string{"logs"} diff --git a/cmd/commands/start_containers.go b/cmd/commands/start_containers.go index 039faedd9..f2b30619b 100644 --- a/cmd/commands/start_containers.go +++ b/cmd/commands/start_containers.go @@ -182,7 +182,7 @@ func startPostgresPhase(ctx context.Context, opts startOpts, cfg *config.Config, // Skip ComposeUp for an already healthy postgres unless a clean/fresh // start cleared it. Compose may otherwise recreate it after config drift. if !opts.cleanStart && !opts.fresh { - containerName := fmt.Sprintf("%s_postgres", cfg.ProjectName) + containerName := cfg.ServiceContainerName("postgres") health, healthErr := docker.GetHealthStatus(ctx, containerName) if healthErr == nil && postgresAlreadyRunning(health) { sp.Success("PostgreSQL container already running and healthy") diff --git a/cmd/commands/testdata/json/argv/README.md b/cmd/commands/testdata/json/argv/README.md index 174dcfa43..3530b2670 100644 --- a/cmd/commands/testdata/json/argv/README.md +++ b/cmd/commands/testdata/json/argv/README.md @@ -5,6 +5,7 @@ Each `.txt` file contains one command argv per line, without the leadi - `legacy-json` compares v1.4 `--json` stdout and exit status byte for byte against merge-base. - `volatile-json=timestamp` replaces only the top-level `timestamp` value in the raw legacy JSON bytes before comparison. The `health` row applies this automatically. Every other byte and the exit status must match. - `no-json=` skips the v1.5 envelope check for a command assigned to a later conversion ticket. +- `error-json=` marks a row that fails by design on the fixture (for example `start` without a compose file). Besides the human comparison, the head binary runs with `--json` in v1.5 mode and must exit non-zero, print exactly one error envelope that validates against `envelope.v1.schema.json`, and carry the named code (`E004`). It replaces `no-json` for such rows. - `human-skip=` skips the base versus head human stdout, stderr and exit comparison when a documented difference is expected. The harness checks human stdout, stderr and exit status in v1.4 and v1.5 modes, then requires a successful exit and exactly one valid v1 data envelope with the registered data shape for every row without `no-json`. diff --git a/cmd/commands/testdata/json/argv/lifecycle.txt b/cmd/commands/testdata/json/argv/lifecycle.txt index 467ddef31..ed7fd59b5 100644 --- a/cmd/commands/testdata/json/argv/lifecycle.txt +++ b/cmd/commands/testdata/json/argv/lifecycle.txt @@ -3,10 +3,11 @@ # row goes first and writes AGENTS.md without a word, so the second row prints the # same stderr on every one of its runs. start, stop and restart cannot finish on # the fixture (no docker-compose.yml; --skip-build keeps start from generating -# one), so each exits 1 the same way in base and head and is tagged no-json; their -# success documents and their failures are covered by TestLifecycleJSON. +# one), so each fails the same way in base and head and is tagged error-json with its +# code, which the script checks as one valid error envelope; their success documents +# and other failures are covered by TestLifecycleJSON. init --quiet --force --non-interactive --name smoke --domain example.test init --force --non-interactive --name smoke --domain example.test -start --skip-build #| no-json=fixture-has-no-docker-compose-yml-see-json_lifecycle_test.go -stop #| no-json=fixture-has-no-docker-compose-yml-see-json_lifecycle_test.go -restart #| no-json=fixture-has-no-docker-compose-yml-see-json_lifecycle_test.go +start --skip-build #| error-json=E004 +stop #| error-json=E400 +restart #| error-json=E400 diff --git a/internal/build/orchestrator_build_compose.go b/internal/build/orchestrator_build_compose.go index 9076bd44d..dd0c52de7 100644 --- a/internal/build/orchestrator_build_compose.go +++ b/internal/build/orchestrator_build_compose.go @@ -54,7 +54,7 @@ func (st *buildState) generateCompose() error { // ── Step 8.1: Warn if this regen would swap a running postgres image ── // (cli#384 — advisory only, never blocks the build; see // PostgresImageChangeWarning for the data-loss scenario this guards.) - postgresContainer := fmt.Sprintf("%s_postgres", st.cfg.ProjectName) + postgresContainer := st.cfg.ServiceContainerName("postgres") generatedPostgresImage := compose.ResolveImage("postgres", compose.ResolvePostgresImage(st.cfg.Postgres)) // Plan mode skips it: the check runs `docker inspect`, a host effect. if !st.planning() { diff --git a/internal/build/secrets_template_test.go b/internal/build/secrets_template_test.go index f80e320ca..d548d11b1 100644 --- a/internal/build/secrets_template_test.go +++ b/internal/build/secrets_template_test.go @@ -154,7 +154,7 @@ func TestTemplateSecrets_CommandAndHealthcheckLiteralsRewritten(t *testing.T) { } for _, ref := range []string{ "REDISCLI_AUTH: ${REDIS_PASSWORD}", - `--requirepass "$$REDISCLI_AUTH"`, + `requirepass "$$esc"`, "TYPESENSE_API_KEY: ${TYPESENSE_API_KEY}", } { if !strings.Contains(doc, ref) { diff --git a/internal/builtin/admin/cmd_root.go b/internal/builtin/admin/cmd_root.go index 7188b1ddb..21a1d1c78 100644 --- a/internal/builtin/admin/cmd_root.go +++ b/internal/builtin/admin/cmd_root.go @@ -34,7 +34,7 @@ func (r runner) adminContainerID() string { if err != nil || cfg.ProjectName == "" { return "nself_admin" } - return fmt.Sprintf("%s_admin", cfg.ProjectName) + return cfg.ServiceContainerName("admin") } func rootCommand(r runner) *cobra.Command { diff --git a/internal/compose/cache_service.go b/internal/compose/cache_service.go index a14e63a8a..e9f97482f 100644 --- a/internal/compose/cache_service.go +++ b/internal/compose/cache_service.go @@ -141,6 +141,17 @@ func (g *Generator) cacheImage(engine string) string { return ResolveImage("redis", ImageRef("redis", version)) } +// redisPasswordScript is the `sh -c` body that starts the cache server with the +// password from $REDISCLI_AUTH on stdin as a `requirepass` config line. The +// value is escaped for the config parser (backslash and double quote), so any +// password round-trips. exec keeps the server as PID 1 for signal handling; +// $$ survives compose interpolation as $. +func redisPasswordScript(server string, poolSize int) string { + return fmt.Sprintf(`esc=$$(printf '%%s' "$$REDISCLI_AUTH" | sed 's/[\\"]/\\&/g'); `+ + "exec %s --appendonly yes --protected-mode yes --maxclients %d - <_` itself makes a naming change a dozen edits again. + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// handRolledName matches a ContainerName field set from Sprintf("%s_", ... +// ProjectName) or ProjectName + "_". Network and volume names share the +// prefix but are not container names, so only ContainerName lines count. +var handRolledName = regexp.MustCompile(`ContainerName\b.*("%s_[^"]*"\s*,[^)]*ProjectName|ProjectName\s*\+\s*"_)`) + +func TestContainerNamesComeFromOneHelper(t *testing.T) { + files, err := filepath.Glob("*.go") + if err != nil || len(files) == 0 { + t.Fatalf("no compose sources found: %v", err) + } + for _, f := range files { + if strings.HasSuffix(f, "_test.go") { + continue + } + src, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(string(src), "\n") { + if handRolledName.MatchString(line) { + t.Errorf("%s:%d builds a container name by hand, use cfg.ServiceContainerName: %s", f, i+1, strings.TrimSpace(line)) + } + } + } +} + +// TestHandRolledNamePatternControl proves the guard matches both shapes it +// exists to stop. +func TestHandRolledNamePatternControl(t *testing.T) { + for _, bad := range []string{ + `ContainerName: fmt.Sprintf("%s_redis", g.cfg.ProjectName),`, + `ContainerName: cfg.ProjectName + "_postgres",`, + } { + if !handRolledName.MatchString(bad) { + t.Errorf("pattern misses %q", bad) + } + } + if handRolledName.MatchString(`ContainerName: g.cfg.ServiceContainerName("redis"),`) { + t.Error("pattern flags the helper call") + } +} diff --git a/internal/compose/core_services.go b/internal/compose/core_services.go index 4d1b8f624..68a307d62 100644 --- a/internal/compose/core_services.go +++ b/internal/compose/core_services.go @@ -59,7 +59,7 @@ func (g *Generator) buildPostgresService() ServiceConfig { } return ServiceConfig{ Image: image, - ContainerName: fmt.Sprintf("%s_postgres", cfg.ProjectName), + ContainerName: cfg.ServiceContainerName("postgres"), Restart: "unless-stopped", Networks: []string{cfg.DockerNetwork}, User: user, @@ -171,7 +171,7 @@ func (g *Generator) buildHasuraService() (ServiceConfig, error) { return ServiceConfig{ Image: hasuraImage(cfg.Hasura.Version), - ContainerName: fmt.Sprintf("%s_hasura", cfg.ProjectName), + ContainerName: cfg.ServiceContainerName("hasura"), Restart: "unless-stopped", User: "1001:1001", Networks: []string{cfg.DockerNetwork}, diff --git a/internal/compose/core_services_auth_nginx.go b/internal/compose/core_services_auth_nginx.go index 500f55a23..30a3a0500 100644 --- a/internal/compose/core_services_auth_nginx.go +++ b/internal/compose/core_services_auth_nginx.go @@ -98,7 +98,7 @@ func (g *Generator) buildAuthService() (ServiceConfig, error) { } return ServiceConfig{ Image: image, - ContainerName: fmt.Sprintf("%s_auth", cfg.ProjectName), + ContainerName: cfg.ServiceContainerName("auth"), Restart: "unless-stopped", Networks: []string{cfg.DockerNetwork}, DependsOn: map[string]DepOn{ @@ -146,7 +146,7 @@ func (g *Generator) buildNginxService(dc *DockerCompose) ServiceConfig { return ServiceConfig{ Image: ResolveImage("nginx", ImageRef("nginx", "")), - ContainerName: fmt.Sprintf("%s_nginx", cfg.ProjectName), + ContainerName: cfg.ServiceContainerName("nginx"), Restart: "unless-stopped", // No User override on purpose. The nginx.conf this same tool generates // declares `user nginx;` on line 4, which requires the MASTER process to diff --git a/internal/compose/custom_services.go b/internal/compose/custom_services.go index 98517888c..c8ff0218c 100644 --- a/internal/compose/custom_services.go +++ b/internal/compose/custom_services.go @@ -207,7 +207,7 @@ func (g *Generator) buildCustomService(cs config.CustomService) (ServiceConfig, } svc := ServiceConfig{ - ContainerName: fmt.Sprintf("%s_%s", cfg.ProjectName, cs.Name), + ContainerName: cfg.ServiceContainerName(cs.Name), Restart: "unless-stopped", Networks: []string{cfg.DockerNetwork}, DependsOn: map[string]DepOn{ diff --git a/internal/compose/object_storage.go b/internal/compose/object_storage.go index 8309ac353..67ad4d6e5 100644 --- a/internal/compose/object_storage.go +++ b/internal/compose/object_storage.go @@ -184,7 +184,7 @@ func (g *Generator) buildSeaweedService() (ServiceConfig, error) { } return ServiceConfig{ Image: ResolveImage("seaweedfs", ImageRef("seaweedfs", os.Getenv("SEAWEEDFS_VERSION"))), - ContainerName: g.cfg.ProjectName + "_seaweedfs", Restart: "unless-stopped", + ContainerName: g.cfg.ServiceContainerName("seaweedfs"), Restart: "unless-stopped", Networks: []string{g.cfg.DockerNetwork}, // The image's seaweed user (uid 1000) owns /data. CapDrop ALL removes root's // DAC_OVERRIDE, so root could not write a fresh volume; run as that user. @@ -224,7 +224,7 @@ func objectStorageKeyProblem(value string) string { func (g *Generator) buildSeaweedInit() ServiceConfig { return ServiceConfig{ Image: ResolveImage("seaweedfs", ImageRef("seaweedfs", os.Getenv("SEAWEEDFS_VERSION"))), - ContainerName: g.cfg.ProjectName + "_seaweedfs_init", Restart: "no", + ContainerName: g.cfg.ServiceContainerName("seaweedfs_init"), Restart: "no", // Shares the server's network namespace: it reaches the master on 127.0.0.1 and // needs no network or port of its own (the master and filer stay closed to the network). NetworkMode: "service:seaweedfs", diff --git a/internal/compose/optional_services.go b/internal/compose/optional_services.go index bcaf3390c..cf6d2b0c2 100644 --- a/internal/compose/optional_services.go +++ b/internal/compose/optional_services.go @@ -25,7 +25,7 @@ func (g *Generator) buildMinioService() ServiceConfig { // The minio entry in images.yaml is registry-qualified (docker.io/pgsty/minio); // the Docker Hub minio/minio repository no longer exists (ADR 0028). Image: ResolveImage("minio", ImageRef("minio", version)), - ContainerName: fmt.Sprintf("%s_minio", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("minio"), Restart: "unless-stopped", Networks: []string{g.cfg.DockerNetwork}, Environment: map[string]string{ @@ -90,7 +90,7 @@ func (g *Generator) buildMailpitService() ServiceConfig { return ServiceConfig{ Image: ResolveImage("mailpit", ImageRef("mailpit", version)), - ContainerName: fmt.Sprintf("%s_mailpit", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("mailpit"), Restart: "unless-stopped", User: "1000:1000", Networks: []string{g.cfg.DockerNetwork}, diff --git a/internal/compose/optional_services_apps.go b/internal/compose/optional_services_apps.go index 40c14ad64..c09967b43 100644 --- a/internal/compose/optional_services_apps.go +++ b/internal/compose/optional_services_apps.go @@ -57,7 +57,7 @@ func (g *Generator) buildAdminService() ServiceConfig { return ServiceConfig{ Image: ResolveImage("admin", ImageRef("admin", version)), - ContainerName: fmt.Sprintf("%s_admin", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("admin"), Restart: "unless-stopped", User: adminUser, GroupAdd: groupAdd, @@ -145,7 +145,7 @@ func (g *Generator) buildFunctionsService() ServiceConfig { svc := ServiceConfig{ Image: image, - ContainerName: fmt.Sprintf("%s_functions", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("functions"), Restart: "unless-stopped", Networks: []string{g.cfg.DockerNetwork}, DependsOn: map[string]DepOn{ diff --git a/internal/compose/optional_services_search.go b/internal/compose/optional_services_search.go index e7ea6191a..bd28e9b9b 100644 --- a/internal/compose/optional_services_search.go +++ b/internal/compose/optional_services_search.go @@ -16,7 +16,7 @@ import ( func (g *Generator) buildMeiliInitService() ServiceConfig { return ServiceConfig{ Image: ResolveImage("meilisearch-init", ImageRef("meilisearch-init", "")), - ContainerName: fmt.Sprintf("%s_meilisearch_init", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("meilisearch_init"), Restart: "no", User: "root", Networks: []string{g.cfg.DockerNetwork}, @@ -51,7 +51,7 @@ func (g *Generator) buildMeiliService() ServiceConfig { return ServiceConfig{ Image: ResolveImage("meilisearch", ImageRef("meilisearch", version)), - ContainerName: fmt.Sprintf("%s_meilisearch", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("meilisearch"), Restart: "unless-stopped", User: "1000:1000", Networks: []string{g.cfg.DockerNetwork}, @@ -101,7 +101,7 @@ func (g *Generator) buildTypesenseService() ServiceConfig { return ServiceConfig{ Image: ResolveImage("typesense", ImageRef("typesense", version)), - ContainerName: fmt.Sprintf("%s_typesense", g.cfg.ProjectName), + ContainerName: g.cfg.ServiceContainerName("typesense"), Restart: "unless-stopped", Networks: []string{g.cfg.DockerNetwork}, Environment: map[string]string{ diff --git a/internal/compose/secret_argv_expanded_test.go b/internal/compose/secret_argv_expanded_test.go new file mode 100644 index 000000000..cdfe56b9f --- /dev/null +++ b/internal/compose/secret_argv_expanded_test.go @@ -0,0 +1,97 @@ +package compose_test + +// secret_argv_expanded_test.go — D-0587 guard: the command a shell EXPANDS +// inside the cache container carries no secret on its argv. secret_argv_test +// checks the compose text; a `$$REDISCLI_AUTH` there is fine, yet `sh -c` turns +// it into `--requirepass ` and host `ps` or /proc//cmdline shows +// it. This test runs the generated command with a stub server and reads the +// argv the server really receives. + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/nself-org/cli/internal/build" + "github.com/nself-org/cli/internal/compat/compattest" + "github.com/nself-org/cli/internal/compose" +) + +// awkwardPassword holds every character the config line has to escape or the +// shell could mangle. +const awkwardPassword = "p\"a\\ss w$o`rd'x;#9" + +// runCacheCommand runs the sh -c script with a stub engine binary on PATH and +// returns what the stub saw: its argv (one argument per line) and its stdin. +func runCacheCommand(t *testing.T, server, script string) (argv, stdin string) { + t.Helper() + dir := t.TempDir() + stub := "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"" + dir + "/argv\"\ncat > \"" + dir + "/stdin\"\n" + if err := os.WriteFile(filepath.Join(dir, server), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + // compose turns $$ into $ before the container shell sees the script. + cmd := exec.Command("sh", "-c", strings.ReplaceAll(script, "$$", "$")) + cmd.Env = append(os.Environ(), "PATH="+dir+string(os.PathListSeparator)+os.Getenv("PATH"), "REDISCLI_AUTH="+awkwardPassword) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("run %q: %v\n%s", script, err, out) + } + a, err := os.ReadFile(filepath.Join(dir, "argv")) + if err != nil { + t.Fatal(err) + } + s, err := os.ReadFile(filepath.Join(dir, "stdin")) + if err != nil { + t.Fatal(err) + } + return string(a), string(s) +} + +// TestExpandedCacheCommandNoSecretOnArgv: the expanded server argv carries no +// part of the password and the server still receives it as a config line. +func TestExpandedCacheCommandNoSecretOnArgv(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("needs a POSIX sh") + } + compattest.Set(t, false) + for _, cache := range []string{"redis", "valkey"} { + cfg := optionalSecretsConfig("meilisearch", setSecrets) + cfg.Redis.Engine = cache + cfg.Redis.Password = awkwardPassword + out, err := compose.NewGenerator(cfg).Generate() + if err != nil { + t.Fatalf("%s: Generate() error: %v", cache, err) + } + cmd := argvStrings(t, build.TemplateSecrets(out, build.SecretEnvMap(cfg)))["redis command"] + if len(cmd) != 3 || cmd[0] != "sh" || cmd[1] != "-c" { + t.Fatalf("%s: redis command = %q, want sh -c