diff --git a/cmd/clear.go b/cmd/clear.go index 9fe07012..ea792815 100644 --- a/cmd/clear.go +++ b/cmd/clear.go @@ -5,12 +5,7 @@ import ( "errors" "fmt" "os" - "os/exec" - "path/filepath" - "strconv" "strings" - "syscall" - "time" "github.com/codefly-dev/cli/cmd/common" "github.com/codefly-dev/cli/pkg/processgroup" @@ -21,85 +16,6 @@ import ( "github.com/spf13/cobra" ) -// codeflyOwnedPIDs returns the PIDs of running codefly-owned processes: the -// `codefly` CLI binary itself and any agent binary under ~/.codefly/agents/. -// It matches on the EXECUTABLE path, never on a substring of the full command -// line, so unrelated processes that merely mention the repo path are spared. -// The caller's own PID (self) is always excluded. -func codeflyOwnedPIDs(ctx context.Context, self int, scope processgroup.Scope) ([]int, error) { - // `command=` prints the full argv; the first token is the executable. - out, err := exec.CommandContext(ctx, "ps", "-axo", "pid=,command=").Output() - if err != nil { - return nil, fmt.Errorf("list processes: %w", err) - } - pids := parseCodeflyOwnedPIDs(out, self) - if scope.All() { - return pids, nil - } - // A scoped kill only signals codefly processes belonging to this workspace's - // run. An agent or CLI whose workspace cannot be resolved belongs to nobody - // we can name, so a scoped kill leaves it alone. Attribution follows the run - // that launched the process, not its working directory: an agent serving a - // composed module runs from that module's checkout, outside the workspace - // that composed it. - kept := make([]int, 0, len(pids)) - for _, pid := range pids { - workspace, ok := processgroup.WorkspaceOfProcess(pid) - if ok && scope.Includes(workspace) { - kept = append(kept, pid) - } - } - return kept, nil -} - -func parseCodeflyOwnedPIDs(out []byte, self int) []int { - var pids []int - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { - fields := strings.Fields(line) - if len(fields) < 2 { - continue - } - pid, convErr := strconv.Atoi(fields[0]) - if convErr != nil || pid == self { - continue - } - exePath := fields[1] - owned := filepath.Base(exePath) == "codefly" || strings.Contains(exePath, "/.codefly/agents/") - if owned { - pids = append(pids, pid) - } - } - return pids -} - -// killedProcessSettle bounds how long clear waits for the codefly processes it -// SIGKILLed to actually exit before it sweeps for orphaned native services. Agents -// die promptly under SIGKILL; the cap only guards against a wedged process holding -// up the command indefinitely. -const killedProcessSettle = 3 * time.Second - -// waitProcessesExited blocks until every pid has been fully reaped (kill(0) fails -// with ESRCH) or the deadline passes, whichever comes first. A pid we cannot signal -// (still present) counts as alive; only a genuinely gone process lets the loop -// advance. The killed agents share this process's user, so a signal never fails -// with EPERM here. -func waitProcessesExited(ctx context.Context, pids []int, timeout time.Duration) { - deadline := time.Now().Add(timeout) - for { - anyAlive := false - for _, pid := range pids { - if syscall.Kill(pid, 0) == nil { - anyAlive = true - break - } - } - if !anyAlive || ctx.Err() != nil || !time.Now().Before(deadline) { - return - } - time.Sleep(20 * time.Millisecond) - } -} - var ( clearKeepProcesses bool clearKeepContainers bool @@ -215,13 +131,16 @@ func clearCommand(ctx context.Context, args []string, options clearOptions) (ret if options.keepProcesses { w.Info("keeping processes (--keep-processes)") - } else { - failures = append(failures, killCodeflyProcesses(ctx, w, options)...) } - failures = append(failures, sweepStaleState(ctx, w, options)...) - failures = append(failures, reapOrphanedDevServers(ctx, w, options)...) - failures = append(failures, reapOrphanedNativeServices(ctx, w, options)...) + managedFailures := sweepStaleState(ctx, w, options) + failures = append(failures, managedFailures...) + // The fallback scans must not turn a failed registry authentication into + // permission to signal the same group by its executable signature. + if len(managedFailures) == 0 { + failures = append(failures, reapOrphanedDevServers(ctx, w, options)...) + failures = append(failures, reapOrphanedNativeServices(ctx, w, options)...) + } if options.keepContainers { return errors.Join(failures...) @@ -264,60 +183,18 @@ func clearCommand(ctx context.Context, args []string, options clearOptions) (ret return errors.Join(failures...) } -// killCodeflyProcesses kills every codefly-owned process in scope, or lists -// them on a dry run, and waits for the killed ones to exit. -func killCodeflyProcesses(ctx context.Context, w *wool.Wool, options clearOptions) []error { - var failures []error - // Match codefly-owned processes by their EXECUTABLE, not by a substring - // over the whole command line. The old `ps aux | grep codefly.dev` - // matched any process whose argv merely mentioned the repo path — an - // editor, a `tail -f`, a build — and kill -9'd it. Agent binaries always - // live under ~/.codefly/agents/, and the CLI itself is the `codefly` - // executable; nothing else qualifies. - self := os.Getpid() - pids, err := codeflyOwnedPIDs(ctx, self, options.scope) - if err != nil { - w.Warn("cannot enumerate codefly processes", wool.ErrField(err)) - failures = append(failures, err) - } - switch { - case options.dryRun: - w.Info("would kill codefly processes", wool.Field("count", len(pids)), wool.Field("pids", pids)) - case len(pids) == 0: - w.Info("no codefly processes running") - default: - killed := 0 - for _, pid := range pids { - p, findErr := os.FindProcess(pid) - if findErr != nil { - failures = append(failures, fmt.Errorf("find process %d: %w", pid, findErr)) - continue - } - if killErr := p.Kill(); killErr != nil { - failures = append(failures, fmt.Errorf("kill process %d: %w", pid, killErr)) - continue - } - killed++ - } - w.Info("killed codefly processes", wool.Field("killed", killed), wool.Field("total", len(pids))) - // SIGKILL is asynchronous: the agents we just signalled are not yet - // reaped, so the native binaries they supervised have not yet - // reparented to init. The orphan sweeps below decide a group is - // reapable only once its supervisor is gone (groupIsStale), so racing - // them against a still-dying agent would leave the just-orphaned - // binary looking supervised and skip it. Wait for the agents to exit - // before sweeping so a `clear` that killed a live run still reaps the - // natives it just orphaned. - waitProcessesExited(ctx, pids, killedProcessSettle) - } - return failures -} - // sweepStaleState removes the state crashed or exited CLIs left behind — // per-spawn UDS sockets under /tmp/codefly-uds and process-group tracking // files under ~/.codefly/runs — or counts it on a dry run. func sweepStaleState(ctx context.Context, w *wool.Wool, options clearOptions) []error { if options.dryRun { + if !options.keepProcesses { + groups, err := processgroup.PreviewManagedProcessGroups(ctx, options.scope) + if err != nil { + return []error{err} + } + w.Info("would stop registered process groups", wool.Field("count", len(groups)), wool.Field("pgids", groups)) + } if n := manager.CountStaleAgentSockets(); n > 0 { w.Info("would remove stale agent sockets", wool.Field("count", n)) } else { diff --git a/cmd/clear_test.go b/cmd/clear_test.go index 8e7afb96..1266c12f 100644 --- a/cmd/clear_test.go +++ b/cmd/clear_test.go @@ -1,14 +1,6 @@ package cmd -import ( - "context" - "os/exec" - "reflect" - "testing" - "time" - - "github.com/codefly-dev/cli/pkg/processgroup" -) +import "testing" func TestClearAndStopCommandsReturnErrors(t *testing.T) { for name, command := range map[string]struct { @@ -39,89 +31,3 @@ func TestStopOptionsDoNotInheritClearFlags(t *testing.T) { t.Fatalf("stop options inherited clear state: %+v", got) } } - -func TestParseCodeflyOwnedPIDs(t *testing.T) { - out := []byte(` 10 /usr/local/bin/codefly run service api - 11 /Users/me/.codefly/agents/go/bin/agent serve - 12 /usr/bin/editor /repo/codefly.dev/file.go - 13 /usr/local/bin/codefly stop -bad /usr/local/bin/codefly -`) - if got, want := parseCodeflyOwnedPIDs(out, 13), []int{10, 11}; !reflect.DeepEqual(got, want) { - t.Fatalf("owned PIDs = %v, want %v", got, want) - } -} - -func TestCodeflyOwnedPIDsReturnsProcessErrors(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() - if _, err := codeflyOwnedPIDs(ctx, -1, processgroup.AllWorkspaces()); err == nil { - t.Fatal("expected cancelled process enumeration to return an error") - } -} - -// TestWaitProcessesExitedBlocksUntilGone guards the reparent race: clear must not -// sweep for orphaned natives until the agents it SIGKILLed have actually exited. -// A no-op wait (the original bug) would return immediately while the process is -// still alive, so the first half asserts the wait genuinely blocks; the second -// asserts it returns promptly once the process is gone. -func TestWaitProcessesExitedBlocksUntilGone(t *testing.T) { - cmd := exec.Command("sleep", "30") - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - pid := cmd.Process.Pid - reaped := make(chan struct{}) - go func() { - _, _ = cmd.Process.Wait() - close(reaped) - }() - t.Cleanup(func() { - _ = cmd.Process.Kill() - <-reaped - }) - - start := time.Now() - waitProcessesExited(context.Background(), []int{pid}, 300*time.Millisecond) - if elapsed := time.Since(start); elapsed < 250*time.Millisecond { - t.Fatalf("waitProcessesExited returned after %s while the process was still alive; expected to wait to the timeout", elapsed) - } - - if err := cmd.Process.Kill(); err != nil { - t.Fatal(err) - } - <-reaped - - start = time.Now() - waitProcessesExited(context.Background(), []int{pid}, 5*time.Second) - if elapsed := time.Since(start); elapsed > time.Second { - t.Fatalf("waitProcessesExited blocked %s after the process exited; expected a prompt return", elapsed) - } -} - -// TestWaitProcessesExitedHonorsCancelledContext ensures a wedged process cannot -// hang clear once the command's context is cancelled (e.g. a second Ctrl-C). -func TestWaitProcessesExitedHonorsCancelledContext(t *testing.T) { - cmd := exec.Command("sleep", "30") - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - pid := cmd.Process.Pid - reaped := make(chan struct{}) - go func() { - _, _ = cmd.Process.Wait() - close(reaped) - }() - t.Cleanup(func() { - _ = cmd.Process.Kill() - <-reaped - }) - - ctx, cancel := context.WithCancel(context.Background()) - cancel() - start := time.Now() - waitProcessesExited(ctx, []int{pid}, time.Minute) - if elapsed := time.Since(start); elapsed > time.Second { - t.Fatalf("waitProcessesExited ignored the cancelled context and blocked %s", elapsed) - } -} diff --git a/cmd/doctor_workspace.go b/cmd/doctor_workspace.go index 11f54387..f88ace6a 100644 --- a/cmd/doctor_workspace.go +++ b/cmd/doctor_workspace.go @@ -175,7 +175,7 @@ func workspaceReadiness(ctx context.Context, opts workspaceReadinessOptions) *wo // behind the materialization gate like every other check that loads them. checkRunnableBindingDeclines(ctx, ws, env, report) - scope, requiredBy := checkScope(ctx, ws, opts.module, opts.service, report) + scope, requiredBy := checkScope(ctx, ws, env, opts.module, opts.service, report) if scope == nil { return report } @@ -790,7 +790,11 @@ func checkSecretProviders(env *environments.Environment, report *workspaceReadin // acts on one unit must not be judged by a sibling unit's missing // configuration. --service wins when both are given, being the narrower of the // two. -func checkScope(ctx context.Context, ws *resources.Workspace, moduleName, serviceName string, report *workspaceReadinessReport) ([]*resources.Service, map[string][]string) { +func checkScope(ctx context.Context, ws *resources.Workspace, env *environments.Environment, moduleName, serviceName string, report *workspaceReadinessReport) ([]*resources.Service, map[string][]string) { + if err := environments.ValidateEnvironmentExclusions(ctx, ws, env); err != nil { + report.add(codeWorkspaceInvalid, "services", "fail", err.Error(), "correct the selected environment's exclude-services declarations") + return nil, nil + } services, err := ws.LoadServices(ctx) if err != nil { report.add(codeWorkspaceInvalid, "services", "fail", @@ -841,6 +845,21 @@ func checkScope(ctx context.Context, ws *resources.Workspace, moduleName, servic scope = []*resources.Service{svc} report.Service = serviceUnique(svc) } + excluded := env.ExcludedServiceUniques() + selected := make([]*resources.Service, 0, len(scope)) + for _, svc := range scope { + if slices.Contains(excluded, serviceUnique(svc)) { + if serviceName != "" { + report.add(codeServiceNotFound, "service", "fail", + fmt.Sprintf("environment %q excludes %s", env.Name, serviceUnique(svc)), + "select a service included in this environment") + return nil, nil + } + continue + } + selected = append(selected, svc) + } + scope = selected requiredBy := make(map[string][]string) for _, svc := range scope { for _, dep := range svc.WorkspaceConfigurationDependencies { @@ -1106,6 +1125,12 @@ func checkConfigurationSources(ctx context.Context, ws *resources.Workspace, env // so a second generated group needs no second exemption. delete(provided.Ambiguous, info.Name) if _, authored := byName[info.Name]; authored { + // Generated binding decisions must judge the values the + // invocation delivers, including Core-applied overrides. + // Keep the authored inventory for provenance and conflicts. + if info.Name == generate.RunnableBindingsGroup { + byName[info.Name] = info + } continue } byName[info.Name] = info @@ -1268,7 +1293,7 @@ func checkRequiredWorkspaceConfiguration(ctx context.Context, ws *resources.Work // no runnable binding, so the group absent or holding no value is its answer. // A selected binding the group does not hold fails, naming the generation // that writes it. It reports whether it judged the group; a group that holds -// every selected binding is left to the checks every group gets. +// every selected binding must also match the generator's prepared values. func checkRunnableBindingsGroup(ctx context.Context, ws *resources.Workspace, env *environments.Environment, name string, info *basev0.ConfigurationInformation, relCfgDir, requiredBy string, report *workspaceReadinessReport) bool { selection, err := generate.SelectRunnableBindings(ctx, ws, env) if err != nil { @@ -1301,6 +1326,11 @@ func checkRunnableBindingsGroup(ctx context.Context, ws *resources.Workspace, en report.add("", "workspace configuration "+name, "ok", fmt.Sprintf("environment %s prepares no runnable binding, so %q holds none (required by %s)", env.Name, name, requiredBy), "") default: + if err := generate.CheckRunnableBindings(ctx, ws, env, info); err != nil { + report.add(codeConfigurationInvalid, "workspace configuration "+name, checkStatusFail, + fmt.Sprintf("cannot use generated runnable bindings: %v (required by %s)", err, requiredBy), remedy) + return true + } return false } return true diff --git a/cmd/doctor_workspace_binding_values_test.go b/cmd/doctor_workspace_binding_values_test.go new file mode 100644 index 00000000..5a399d91 --- /dev/null +++ b/cmd/doctor_workspace_binding_values_test.go @@ -0,0 +1,73 @@ +package cmd + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/cmd/deploy" + "github.com/codefly-dev/cli/cmd/generate" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +func TestDoctorUsesEffectiveRunnableValueAndPreservesAuthoredFile(t *testing.T) { + artifact, err := os.ReadFile("generate/testdata/solution-runnables/api.swagger.json") + require.NoError(t, err) + dir := singleServiceWorkspace(t, testWorkspaceYAMLLocalDeclared, []string{generate.RunnableBindingsGroup}, map[string]string{ + "modules/backend/services/api/service.codefly.yaml": strings.NewReplacer("runtime::service", "codefly:service", "codefly.ai", "codefly.dev").Replace(testServiceYAML("api", generate.RunnableBindingsGroup)) + `endpoints: + - name: http + api: http + visibility: public + exposure: none +`, + "modules/backend/module.codefly.yaml": testModuleYAML("api") + `interface: + endpoints: + - service: api + endpoint: http + visibility: public +`, + "modules/backend/services/api/openapi/api.swagger.json": string(artifact), + }) + t.Chdir(dir) + require.NoError(t, generate.ContractsCmd.RunE(generate.ContractsCmd, []string{"backend"})) + require.NoError(t, generate.RunnablesCmd.RunE(generate.RunnablesCmd, []string{"backend"})) + require.NoError(t, generate.RunnableBindingsCmd.RunE(generate.RunnableBindingsCmd, nil)) + path := filepath.Join(dir, "configurations/local/runnable-bindings.env") + fresh, err := os.ReadFile(path) + require.NoError(t, err) + var key, value string + for _, line := range strings.Split(string(fresh), "\n") { + if !strings.HasPrefix(line, "#") && strings.Contains(line, "=") { + key, value, _ = strings.Cut(line, "=") + break + } + } + require.NotEmpty(t, key) + for _, validOverride := range []bool{false, true} { + t.Run(map[bool]string{false: "stale override", true: "valid replacement"}[validOverride], func(t *testing.T) { + authored, override := fresh, "{}" + if validOverride { + authored = []byte(strings.Replace(string(fresh), key+"="+value, key+"={}", 1)) + override = value + } + require.NoError(t, os.WriteFile(path, authored, 0o600)) + encoded, err := resources.EncodeWorkspaceConfigurationOverrides([]resources.WorkspaceConfigurationOverride{{Name: generate.RunnableBindingsGroup, Key: key, Value: override}}) + require.NoError(t, err) + t.Setenv(resources.WorkspaceConfigurationOverridesEnvironment, encoded) + report := runReadiness(t, workspaceReadinessOptions{dir: dir, module: "backend"}) + if validOverride { + require.Equal(t, readinessStatusReady, report.Status, reportJSON(t, report)) + require.NoError(t, deploy.WorkspaceReadiness(context.Background(), "local", "backend")) + } else { + requireCode(t, report, codeConfigurationInvalid, checkStatusFail) + require.ErrorContains(t, deploy.WorkspaceReadiness(context.Background(), "local", "backend"), "out of date") + } + after, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, authored, after) + }) + } +} diff --git a/cmd/doctor_workspace_exclusions_test.go b/cmd/doctor_workspace_exclusions_test.go new file mode 100644 index 00000000..c4b2d23c --- /dev/null +++ b/cmd/doctor_workspace_exclusions_test.go @@ -0,0 +1,36 @@ +package cmd + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDoctorJudgesOnlyServicesSelectedByTheEnvironment(t *testing.T) { + dir := writeTestWorkspace(t, map[string]string{ + "workspace.codefly.yaml": testWorkspaceYAML + `environments: + - name: local + - name: blue + exclude-services: + - service: backend/local +`, + "modules/backend/module.codefly.yaml": testModuleYAML("api", "local"), + "modules/backend/services/api/service.codefly.yaml": testServiceYAML("api", "active"), + "modules/backend/services/local/service.codefly.yaml": testServiceYAML("local", "fixture-composition", "local-only"), + "configurations/blue/active.env": "VALUE=present\n", + "configurations/local/active.env": "VALUE=present\n", + }) + for _, module := range []string{"", "backend"} { + report := runReadiness(t, workspaceReadinessOptions{dir: dir, env: "blue", module: module}) + require.Equal(t, readinessStatusReady, report.Status, reportJSON(t, report)) + require.Contains(t, findCheck(report, "services").Message, "1 service(s)") + } + // The same service still imposes its ordinary requirements where included. + // Fixture composition itself is supplied by the dedicated empty merger. + report := runReadiness(t, workspaceReadinessOptions{dir: dir, env: "local"}) + requireCode(t, report, codeConfigurationMissing, checkStatusFail) + // Selecting an excluded root explicitly must not claim it can run there. + report = runReadiness(t, workspaceReadinessOptions{dir: dir, env: "blue", service: "backend/local"}) + require.Equal(t, readinessStatusNotReady, report.Status) + require.Contains(t, reportJSON(t, report), "excludes backend/local") +} diff --git a/cmd/doctor_workspace_runnable_test.go b/cmd/doctor_workspace_runnable_test.go index 41f8d2c3..433bba0b 100644 --- a/cmd/doctor_workspace_runnable_test.go +++ b/cmd/doctor_workspace_runnable_test.go @@ -2,6 +2,10 @@ package cmd import ( "context" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "os" + "path/filepath" "strings" "testing" @@ -198,6 +202,17 @@ func TestDoctorWorkspaceJudgesTheRunnableBindingsGroupByWhatTheEnvironmentSelect t.Fatalf("want one failure, for the empty auth0 group: %+v", diagnostics) } }) + t.Run("obsolete binding after last operation disappears", func(t *testing.T) { + dir := singleServiceWorkspace(t, testWorkspaceYAML, deps, map[string]string{ + bindings: header + "OBSOLETE__OPERATION={}\n", + "configurations/local/auth0.env": "CLIENT_ID=public-id\n", + }) + report := runReadiness(t, workspaceReadinessOptions{dir: dir}) + diagnostic := requireCode(t, report, codeConfigurationInvalid, checkStatusFail) + if !strings.Contains(diagnostic.Message, "out of date") { + t.Fatalf("want stale output refusal: %+v", diagnostic) + } + }) } // The render's readiness gate is the doctor scoped to the module: a module @@ -214,3 +229,21 @@ func TestRenderReadinessAcceptsAnAbsentRunnableBindingsGroupWhenNoneIsSelected(t t.Fatalf("render was refused by readiness: %v\noutput: %s", err, out) } } + +func TestRunnableReadinessChecksInvocationValuesOverAuthoredGroup(t *testing.T) { + dir := singleServiceWorkspace(t, testWorkspaceYAML, []string{generate.RunnableBindingsGroup}, map[string]string{ + "configurations/local/runnable-bindings.env": "# no derived operations\n", + }) + encoded, err := resources.EncodeWorkspaceConfigurationOverrides([]resources.WorkspaceConfigurationOverride{ + {Name: generate.RunnableBindingsGroup, Key: "OBSOLETE__OPERATION", Value: "{}"}, + }) + require.NoError(t, err) + t.Setenv(resources.WorkspaceConfigurationOverridesEnvironment, encoded) + report := runReadiness(t, workspaceReadinessOptions{dir: dir, module: "backend"}) + requireCode(t, report, codeConfigurationInvalid, checkStatusFail) + t.Chdir(dir) + require.ErrorContains(t, deploy.WorkspaceReadiness(context.Background(), "local", "backend"), "out of date") + raw, err := os.ReadFile(filepath.Join(dir, "configurations/local/runnable-bindings.env")) + require.NoError(t, err) + require.Equal(t, "# no derived operations\n", string(raw)) +} diff --git a/cmd/generate/runnable_bindings.go b/cmd/generate/runnable_bindings.go index f6a376a0..5daaf89f 100644 --- a/cmd/generate/runnable_bindings.go +++ b/cmd/generate/runnable_bindings.go @@ -141,7 +141,12 @@ Examples: ctx, stop := common.SignalContext(ctx) defer stop() - workspace, err := common.LoadWorkspace(ctx) + // Every composed module is walked for its derived operations, so a + // pinned module is materialized first, as a run, a render and + // `generate fixture-composition` do: on a fresh checkout — a CI job — + // the plain loader refuses a pinned module as "not loadable as a local + // checkout", and --check never reaches the file it exists to judge. + workspace, err := common.LoadWorkspaceWithPinnedModules(ctx) if err != nil { return err } @@ -200,7 +205,7 @@ Examples: func init() { RunnableBindingsCmd.Flags().StringVar(&runnableBindingsEnv, "env", "local", "environment supplying resolved addresses and runnable-scope-selections") - RunnableBindingsCmd.Flags().BoolVar(&runnableBindingsCheck, "check", false, "do not write; exit 1 if the on-disk file differs from what would be generated") + RunnableBindingsCmd.Flags().BoolVar(&runnableBindingsCheck, "check", false, "leave binding output unchanged; materialize selected pins, then exit 1 if the output is stale") RunnableBindingsCmd.Flags().StringVar(&runnableBindingsNamingScope, "naming-scope", "", "`scope` addresses resolve in, as codefly run --naming-scope reads it: omitted keeps the environment's, empty clears it") RunnableBindingsCmd.Flags().StringVar(&runnableBindingsCallEndpoint, "call-endpoint", "", "declared HTTP-based endpoint, as `module/service/endpoint`, every prepared call is sent to instead of the owner's listener, overriding the environment's runnable-call-endpoint (empty: each owner's listener; the owner's operation, route, contract and policy are kept)") } @@ -256,6 +261,30 @@ type preparedRunnableBindings struct { content []byte count int declined []string + values map[string]string +} + +// CheckRunnableBindings checks the values a consumer would receive against +// the same preparation used by generate. It reads only; the caller supplies +// the effective configuration, including profile and composed-module selection. +// Comments and ordering are immaterial here; --check also checks file layout. +func CheckRunnableBindings(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, info *basev0.ConfigurationInformation) error { + prepared, err := prepareRunnableBindings(ctx, workspace, env, env.RunnableCallEndpoint) + if err != nil { + return err + } + current := len(info.GetConfigurationValues()) == len(prepared.values) + for _, value := range info.GetConfigurationValues() { + expected, exists := prepared.values[value.GetKey()] + if !exists || value.GetValue() != expected { + current = false + } + delete(prepared.values, value.GetKey()) + } + if !current { + return fmt.Errorf("runnable-bindings is out of date: run `%s`", runnableBindingsRegenerate(env, env.RunnableCallEndpoint)) + } + return nil } // prepareRunnableBindings walks every module of the workspace and prepares a @@ -341,7 +370,7 @@ func prepareRunnableBindings(ctx context.Context, workspace *resources.Workspace for _, key := range keys { out.WriteString(key + "=" + lines[key] + "\n") } - return &preparedRunnableBindings{content: []byte(out.String()), count: len(lines), declined: declined}, nil + return &preparedRunnableBindings{content: []byte(out.String()), count: len(lines), declined: declined, values: lines}, nil } // declinedBindingComment is the line the file carries for a declined key. It diff --git a/cmd/generate/runnable_bindings_freshness_test.go b/cmd/generate/runnable_bindings_freshness_test.go new file mode 100644 index 00000000..17d6aeb9 --- /dev/null +++ b/cmd/generate/runnable_bindings_freshness_test.go @@ -0,0 +1,104 @@ +package generate + +import ( + "context" + "github.com/codefly-dev/cli/pkg/composition" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + runnablev0 "github.com/codefly-dev/core/generated/go/codefly/runnable/v0" + "github.com/codefly-dev/core/resources" + corerunnable "github.com/codefly-dev/core/runnable" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +func TestCheckRunnableBindingsRefusesStaleValuesWithUnchangedKeys(t *testing.T) { + ctx := context.Background() + root, moduleDir := saveRunnableFixture(t, ctx, descriptorSet(t, ingestionFile(conformingOperation())), "0.1.0", connectEndpoint()) + t.Chdir(root) + resetRunnablesFlags(t) + resetRunnableBindingsFlags(t) + require.NoError(t, RunnablesCmd.RunE(RunnablesCmd, []string{"documents"})) + publishFixtureConnectEndpoint(t, moduleDir, "connect", nil) + require.NoError(t, RunnableBindingsCmd.RunE(RunnableBindingsCmd, nil)) + ws, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + env, err := environments.Select(ws, "local") + require.NoError(t, err) + target := filepath.Join(root, "configurations", "local", RunnableBindingsGroup+".env") + raw, err := os.ReadFile(target) + require.NoError(t, err) + info := &basev0.ConfigurationInformation{Name: RunnableBindingsGroup} + for _, line := range strings.Split(string(raw), "\n") { + if key, value, found := strings.Cut(line, "="); found && !strings.HasPrefix(line, "#") { + info.ConfigurationValues = append(info.ConfigurationValues, &basev0.ConfigurationValue{Key: key, Value: value}) + } + } + require.Len(t, info.ConfigurationValues, 1) + require.NoError(t, CheckRunnableBindings(ctx, ws, env, info)) + for name, change := range map[string]func(*runnablev0.PreparedBinding){ + "address": func(binding *runnablev0.PreparedBinding) { binding.Call.Address += "/stale" }, + "policy": func(binding *runnablev0.PreparedBinding) { binding.Policy.MaxAttempts++ }, + "contract": func(binding *runnablev0.PreparedBinding) { binding.Contract.Input.Fields[0].Name += "Old" }, + } { + t.Run(name, func(t *testing.T) { + stale := proto.CloneOf(info) + binding, err := corerunnable.DecodePrepared([]byte(stale.ConfigurationValues[0].Value)) + require.NoError(t, err) + change(binding) + binding.ContractDigest = "" + value, err := corerunnable.EncodePrepared(binding) + require.NoError(t, err) + stale.ConfigurationValues[0].Value = string(value) + require.ErrorContains(t, CheckRunnableBindings(ctx, ws, env, stale), "out of date") + }) + } + extra := proto.CloneOf(info) + extra.ConfigurationValues = append(extra.ConfigurationValues, &basev0.ConfigurationValue{Key: "OBSOLETE", Value: "old"}) + require.ErrorContains(t, CheckRunnableBindings(ctx, ws, env, extra), "out of date") + env.NamingScope = "another-run" + require.ErrorContains(t, CheckRunnableBindings(ctx, ws, env, info), "out of date") + // Checking never rewrites the consumer's file. + after, err := os.ReadFile(target) + require.NoError(t, err) + require.Equal(t, raw, after) +} + +func TestFreshCheckoutBindingCheckMaterializes(t *testing.T) { + t.Setenv(resources.CodeflyHomeEnv, t.TempDir()) + source := t.TempDir() + git := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = source + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + git("init", "--quiet") + git("config", "user.email", "review@example.invalid") + git("config", "user.name", "Review Fixture") + require.NoError(t, os.WriteFile(filepath.Join(source, resources.ModuleConfigurationName), []byte("name: sample\n"), 0600)) + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "fixture") + git("-c", "tag.gpgSign=false", "tag", "v0.0.1") + root := t.TempDir() + manifest := "name: review\nlayout: modules\n" + composition.ModuleResolutionKey + ":\n sample: git\nmodules:\n - name: sample\n source: file://" + source + "\n version: v0.0.1\n" + require.NoError(t, os.WriteFile(filepath.Join(root, resources.WorkspaceConfigurationName), []byte(manifest), 0600)) + t.Chdir(root) + resetRunnableBindingsFlags(t) + runnableBindingsCheck = true + require.NoFileExists(t, filepath.Join(root, resources.LocalOverlayConfigurationName)) + require.NoError(t, RunnableBindingsCmd.RunE(RunnableBindingsCmd, nil)) + require.FileExists(t, filepath.Join(root, resources.LocalOverlayConfigurationName)) + require.NoFileExists(t, filepath.Join(root, "configurations", "local", "runnable-bindings.env")) + ws, err := resources.LoadWorkspaceFromDir(context.Background(), root) + require.NoError(t, err) + _, err = ws.LoadModules(context.Background()) + require.NoError(t, err) +} diff --git a/cmd/ps.go b/cmd/ps.go index 9b3d2b5c..4478bf35 100644 --- a/cmd/ps.go +++ b/cmd/ps.go @@ -39,37 +39,24 @@ type psProcess struct { Started time.Time `json:"started"` } -// PsCmd lists what a codefly run is still holding: its frontend dev servers and -// its native-mode service processes — the compiled service binaries and the -// stateful stores it started. -// -// It lists a run COMPLETELY, which it did not. Two gaps, one cause: it scanned -// only dev servers, so a run's compiled services and stores never appeared at -// all; and a dev server was only found when its working directory sat inside a -// workspace, which is never true of a composed module's frontend — that runs -// from the module's own checkout, outside the workspace that composed it. A -// composed run therefore showed a fraction of itself, and `stop` then left -// exactly the processes `ps` could not name still holding their ports. -// -// It is scoped to the current workspace, so it answers "what is my run still -// holding" — the same set `codefly stop` acts on. `--all` widens it to every -// workspace on the machine, which is how to find a leak belonging to a checkout -// you are not standing in. +// PsCmd lists signature-matched frontend and native processes. It is a +// diagnostic view, not the complete registered population used by stop. var PsCmd = &cobra.Command{ Use: "ps", Short: "List the processes this workspace's run is still holding", Long: `List the processes a codefly run is keeping alive: frontend dev servers (next dev / npm run dev / vite) and native-mode services (compiled service -binaries and the stateful stores a run started), including those belonging to -composed modules checked out outside the workspace. +binaries and PostgreSQL), including composed modules when their launch +environment or working directory identifies the workspace. This signature scan +can omit registered processes whose command or environment has changed; stop +uses authenticated registry discovery independently. -Scoped to the current workspace — the same set 'codefly stop' acts on. Use ---all for every workspace on this machine. Outside a workspace the listing is -machine-wide, since there is nothing to scope to. +Scoped to the current workspace. Use --all for every workspace on this machine. +Outside a workspace the listing is machine-wide. -STATUS is one of: orphaned (codefly's, escaped its supervisor — reaped by -'codefly clear'), tracked (codefly's, still supervised), or external (not -codefly's — shown for visibility, never reaped).`, +STATUS is diagnostic: orphaned (marked process without its supervisor), tracked +(marked process still supervised), or external (no marker). A marker or status +never authorizes cleanup: clear must authenticate a registered identity.`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx, done := common.NewContext() diff --git a/cmd/publish/agent_release.go b/cmd/publish/agent_release.go index 20ac58f1..ebff2971 100644 --- a/cmd/publish/agent_release.go +++ b/cmd/publish/agent_release.go @@ -4,7 +4,6 @@ import ( "archive/tar" "compress/gzip" "context" - "crypto/sha256" "encoding/json" "fmt" "io" @@ -249,7 +248,7 @@ func collectLoaderAssets(reg *resources.AgentKindRegistration, ciOutput, name, v // expectedLoaderAssets names the archive every required loader platform must // carry on a release the agent's own workflow publishes. Nothing is staged: -// the workflow is the sole producer of those bytes, and verifyWorkflowRelease +// the workflow is the sole producer of those bytes, and verifyPublishedArchives // establishes them against the checksums that workflow published. func expectedLoaderAssets(reg *resources.AgentKindRegistration, name, version string) []loaderAsset { assets := make([]loaderAsset, 0, len(loaderPlatforms)) @@ -322,6 +321,10 @@ func createAndUploadRelease(ctx context.Context, client *github.Client, owner, r // publishReleaseAssets is createAndUploadRelease for an explicit release // request — the one upload path a semantic release and a dev build share. func publishReleaseAssets(ctx context.Context, client *github.Client, owner, repo string, request *github.CreateReleaseRequest, assets []loaderAsset) error { + files, err := stageReleaseChecksums(assets) + if err != nil { + return err + } release, err := getOrCreateRelease(ctx, client, owner, repo, request) if err != nil { return err @@ -330,15 +333,16 @@ func publishReleaseAssets(ctx context.Context, client *github.Client, owner, rep for _, asset := range release.Assets { existing[asset.GetName()] = asset } - for _, asset := range assets { - files := []string{asset.archivePath} - if asset.sbomPath != "" { - files = append(files, asset.sbomPath) + // Refuse a conflicting retry before uploading any new file, especially + // checksums describing bytes that this release can never accept. + for _, file := range files { + if err := validateExistingReleaseAsset(file, existing); err != nil { + return err } - for _, file := range files { - if err := uploadReleaseAsset(ctx, client, owner, repo, release.GetID(), file, existing); err != nil { - return err - } + } + for _, file := range files { + if err := uploadReleaseAsset(ctx, client, owner, repo, release.GetID(), file, existing); err != nil { + return err } } return nil @@ -397,15 +401,8 @@ func uploadReleaseAsset(ctx context.Context, client *github.Client, owner, repo return fmt.Errorf("open release asset %s: %w", path, err) } defer file.Close() - if asset, ok := existing[name]; ok { - digest := sha256.New() - if _, err := io.Copy(digest, file); err != nil { - return err - } - if asset.GetDigest() != fmt.Sprintf("sha256:%x", digest.Sum(nil)) { - return fmt.Errorf("release asset %s already exists with different or unverifiable bytes; published assets cannot be replaced", name) - } - return nil + if _, ok := existing[name]; ok { + return validateExistingReleaseAsset(path, existing) } if _, _, err := client.Repositories.UploadReleaseAsset(ctx, owner, repo, releaseID, &github.UploadOptions{Name: name}, file); err != nil { return fmt.Errorf("upload release asset %s: %w", name, err) @@ -650,7 +647,6 @@ func (r *agentReleaser) beforeCommit(ctx context.Context, newTag string) error { } func (r *agentReleaser) afterPush(ctx context.Context, newTag string) error { - version := strings.TrimPrefix(newTag, "v") owner := gh.Owner(r.publisher) repo := r.reg.GitHubRepository(r.name) client, err := gh.NewClient() @@ -673,7 +669,25 @@ func (r *agentReleaser) afterPush(ctx context.Context, newTag string) error { if err := publishReleaseAssets(ctx, client, owner, repo, releaseRequest(newTag, r.dev), r.assets); err != nil { return err } - return verifyReleaseAssets(ctx, client, r.reg, r.publisher, r.name, version, r.assets) + return r.verifyPublication(ctx, client, owner, repo, newTag) +} + +// verifyPublication separates the publisher's integrity proof from installer +// reachability. The latter uses Core's credential rule, never a gh CLI login +// that publishing may use. Public releases remain installable anonymously. +func (r *agentReleaser) verifyPublication(ctx context.Context, publisherClient *github.Client, owner, repo, tag string) error { + if err := verifyPublishedArchives(ctx, publisherClient, owner, repo, tag, r.assets); err != nil { + return err + } + options := []github.ClientOptionsFunc{} + if token := manager.GitHubReleaseToken(); token != "" { + options = append(options, github.WithAuthToken(token)) + } + installerClient, err := github.NewClient(options...) + if err != nil { + return err + } + return verifyReleaseAssets(ctx, installerClient, r.reg, r.publisher, r.name, strings.TrimPrefix(tag, "v"), r.assets) } // sourceTagReleaser publishes an immutable source release: it runs diff --git a/cmd/publish/client_versions_test.go b/cmd/publish/client_versions_test.go new file mode 100644 index 00000000..143923a3 --- /dev/null +++ b/cmd/publish/client_versions_test.go @@ -0,0 +1,152 @@ +package publish_test + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/core/composition" + "github.com/stretchr/testify/require" +) + +// clientReleaseRepo includes a generated client outside libraries/, a foreign +// client, and an ignored cache. Only a tracked source owned by the releasing +// package moves, and its copied contract bytes remain identical. +func clientReleaseRepo(t *testing.T) (string, map[string]string) { + t.Helper() + dir, _, originals := moduleAgentRepo(t, "0.2.0") + contract := `{"openapi":"3.0.0","paths":{"/ready":{"get":{}}}}` + digest := composition.APIContractDigest([]byte(contract)) + endpoint := composition.APIContractEndpoint{ + Service: "api", Endpoint: "rest", API: "rest", Kind: composition.APIContractKindOpenAPI, + Package: "payments", Path: "contracts/api/api/rest/openapi.json", Digest: digest, + Routes: []composition.APIContractRoute{{Method: "GET", Path: "/ready"}}, + } + catalog := composition.APIContractCatalog{Schema: composition.APIContractCatalogSchema, Package: "acme/payments", Version: "0.0.0", Endpoints: []composition.APIContractEndpoint{endpoint}} + data, err := catalog.CanonicalBytes() + require.NoError(t, err) + packagePath := "modules/payments/module.package.codefly.yaml" + originals[packagePath] = strings.Replace(originals[packagePath], " - name: api\n", " - name: api\n endpoints: [rest]\n api-contracts:\n - endpoint: rest\n kind: openapi\n package: payments\n path: "+endpoint.Path+"\n digest: "+digest+"\n", 1) + originals["modules/payments/"+composition.APIContractCatalogFileName] = string(data) + originals["modules/payments/"+endpoint.Path] = contract + catalog.Endpoints[0].Path = "openapi.json" + data, err = catalog.CanonicalBytes() + require.NoError(t, err) + originals["clients/payments/contract/catalog.codefly.json"] = string(data) + originals["clients/payments/contract/openapi.json"] = contract + originals["clients/payments/library.codefly.yaml"] = `kind: library +name: payments +version: 0.0.0 +languages: + - name: typescript + path: typescript/ + - name: python + path: python/ +sources: + - package: acme/payments + version: 0.0.0 + module: payments + service: api + endpoint: rest + contract-digest: ` + digest + ` +generated-by: + codefly: 0.1.219 + companion: preserved + facade: false +` + originals["clients/payments/typescript/package.json"] = `{"name":"@acme/payments","version":"0.0.0","description":"preserved"}` + originals["clients/payments/python/pyproject.toml"] = "[project]\nname = \"payments\"\nversion = \"0.0.0\"\n" + originals["libraries/foreign/library.codefly.yaml"] = "kind: library\nname: foreign\nversion: 1.0.0\nsources:\n - package: acme/foreign\n version: 1.0.0\n" + originals[".gitignore"] = ".cache/\n" + for relative, content := range originals { + path := filepath.Join(dir, relative) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + } + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".cache"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".cache", "library.codefly.yaml"), []byte("invalid YAML: ["), 0o600)) + require.NoError(t, runGit(dir, "add", ".")) + require.NoError(t, runGit(dir, "commit", "-m", "generated clients")) + require.NoError(t, runGit(dir, "push", "origin", "main")) + return dir, originals +} + +func TestModuleReleaseStampsTrackedClientsAtOwnerVersion(t *testing.T) { + dir, originals := clientReleaseRepo(t) + tag, err := moduleEngine(t, dir, "patch").Release(context.Background()) + require.NoError(t, err) + require.Equal(t, "v0.2.1", tag) + require.Contains(t, readRel(t, dir, "clients/payments/library.codefly.yaml"), "version: 0.2.1") + require.NotContains(t, readRel(t, dir, "clients/payments/library.codefly.yaml"), "version: 0.0.0") + require.Contains(t, readRel(t, dir, "clients/payments/library.codefly.yaml"), "companion: preserved") + require.Contains(t, readRel(t, dir, "clients/payments/contract/catalog.codefly.json"), `"version": "0.2.1"`) + require.Equal(t, strings.Replace(originals["clients/payments/typescript/package.json"], `"0.0.0"`, `"0.2.1"`, 1), readRel(t, dir, "clients/payments/typescript/package.json")) + require.Contains(t, readRel(t, dir, "clients/payments/python/pyproject.toml"), `version = "0.2.1"`) + for _, path := range []string{"clients/payments/contract/openapi.json", "libraries/foreign/library.codefly.yaml"} { + require.Equal(t, originals[path], readRel(t, dir, path)) + } + require.Equal(t, "invalid YAML: [", readRel(t, dir, ".cache/library.codefly.yaml")) + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD"), "every stamped path belongs to the release commit") +} + +func TestModuleClientReleaseFailureRestoresEveryStamp(t *testing.T) { + dir, originals := clientReleaseRepo(t) + engine := moduleEngine(t, dir, "patch") + failure := errors.New("release gate rejected candidate") + engine.BeforeCommit = func(context.Context, string) error { + require.Contains(t, readRel(t, dir, "clients/payments/contract/catalog.codefly.json"), `"version": "0.2.1"`) + require.Contains(t, readRel(t, dir, "clients/payments/typescript/package.json"), `"version":"0.2.1"`) + return failure + } + _, err := engine.Release(context.Background()) + require.ErrorIs(t, err, failure) + for path, before := range originals { + require.Equal(t, before, readRel(t, dir, path), "%s was not restored", path) + } + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD")) +} + +func TestModuleClientReleaseRefusesEscapingLanguagePath(t *testing.T) { + for _, symlink := range []bool{false, true} { + t.Run(map[bool]string{false: "traversal", true: "symlink"}[symlink], func(t *testing.T) { + dir, originals := clientReleaseRepo(t) + outside := t.TempDir() + outsideManifest := filepath.Join(outside, "package.json") + require.NoError(t, os.WriteFile(outsideManifest, []byte(`{"version":"9.0.0"}`), 0o600)) + if symlink { + require.NoError(t, os.Remove(filepath.Join(dir, "clients/payments/typescript/package.json"))) + require.NoError(t, os.Symlink(outsideManifest, filepath.Join(dir, "clients/payments/typescript/package.json"))) + } else { + manifest := strings.Replace(originals["clients/payments/library.codefly.yaml"], "path: typescript/", "path: ../../../../escape", 1) + require.NoError(t, os.WriteFile(filepath.Join(dir, "clients/payments/library.codefly.yaml"), []byte(manifest), 0o600)) + } + require.NoError(t, runGit(dir, "add", ".")) + require.NoError(t, runGit(dir, "commit", "-m", "escaping path")) + require.NoError(t, runGit(dir, "push", "origin", "main")) + _, err := moduleEngine(t, dir, "patch").Release(context.Background()) + require.ErrorContains(t, err, "escapes") + bytes, err := os.ReadFile(outsideManifest) + require.NoError(t, err) + require.Equal(t, `{"version":"9.0.0"}`, string(bytes)) + require.Equal(t, originals["agent.codefly.yaml"], readRel(t, dir, "agent.codefly.yaml")) + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD")) + }) + } +} + +func TestModuleClientReleasePythonVersionTargetsProjectTable(t *testing.T) { + dir, _ := clientReleaseRepo(t) + path := "clients/payments/python/pyproject.toml" + before := "[tool.example]\nversion = \"7.0.0\"\n\n[project] # package metadata\nname = 'payments'\nversion = \"0.0.0\" # release\n" + require.NoError(t, os.WriteFile(filepath.Join(dir, path), []byte(before), 0o600)) + require.NoError(t, runGit(dir, "commit", "-am", "customized Python manifest")) + require.NoError(t, runGit(dir, "push", "origin", "main")) + tag, err := moduleEngine(t, dir, "patch").Release(context.Background()) + require.NoError(t, err) + require.Equal(t, "v0.2.1", tag) + require.Equal(t, strings.Replace(before, `"0.0.0"`, `"0.2.1"`, 1), readRel(t, dir, path)) + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD")) +} diff --git a/cmd/publish/library.go b/cmd/publish/library.go index cca84ebc..4bb4c699 100644 --- a/cmd/publish/library.go +++ b/cmd/publish/library.go @@ -11,6 +11,7 @@ import ( "github.com/Masterminds/semver" "github.com/codefly-dev/cli/cmd/common" + "github.com/codefly-dev/cli/pkg/generators" "github.com/codefly-dev/cli/pkg/librarystore" "github.com/codefly-dev/core/resources" "github.com/spf13/cobra" @@ -273,53 +274,12 @@ func readJSONVersionField(path string) (string, error) { return doc.Version, nil } -// readPyprojectVersion extracts `version = "X.Y.Z"` from the [project] table -// of a pyproject.toml. A hand-written line scan, not a TOML parser: the -// version field is the only value this command needs, and codefly does not -// otherwise depend on a TOML library. +// readPyprojectVersion uses the same project identity as generated-client +// stamping, including quoted/dotted TOML keys and comments. func readPyprojectVersion(path string) (string, error) { data, err := os.ReadFile(path) if err != nil { return "", fmt.Errorf("read %s: %w", path, err) } - inProject := false - for _, rawLine := range strings.Split(string(data), "\n") { - trimmed := strings.TrimSpace(stripTOMLComment(rawLine)) - if strings.HasPrefix(trimmed, "[") { - inProject = trimmed == "[project]" - continue - } - if !inProject || trimmed == "" { - continue - } - key, value, ok := strings.Cut(trimmed, "=") - if !ok || strings.TrimSpace(key) != "version" { - continue - } - return strings.Trim(strings.TrimSpace(value), `"'`), nil - } - return "", fmt.Errorf("%s: no version field in [project]", path) -} - -// stripTOMLComment removes a trailing, unquoted "#" comment from a TOML -// line — mirroring how parseGoModulePath strips "//" from a go.mod line — so -// a table header or value followed by a comment (`[project] # PEP 621` or -// `version = "1.0.0" # keep in sync`) is matched correctly instead of -// leaving the comment text fused onto the extracted value. A "#" inside a -// quoted string is left alone. -func stripTOMLComment(line string) string { - var quote byte - for i := 0; i < len(line); i++ { - switch c := line[i]; { - case quote != 0: - if c == quote { - quote = 0 - } - case c == '"' || c == '\'': - quote = c - case c == '#': - return line[:i] - } - } - return line + return generators.PythonProjectVersion(path, data) } diff --git a/cmd/publish/library_test.go b/cmd/publish/library_test.go index 332b3db8..5a67563d 100644 --- a/cmd/publish/library_test.go +++ b/cmd/publish/library_test.go @@ -202,3 +202,18 @@ func TestPublishLibraryUnknownNameReturnsError(t *testing.T) { cmd, _ := newTestCmd() require.Error(t, publishLibrary(cmd, "does-not-exist")) } + +func TestReadPyprojectVersionUsesTOMLProjectIdentity(t *testing.T) { + for _, before := range []string{ + "[tool.example]\nversion='7.0.0'\n[ 'project' ]\n'version'='0.1.0' # metadata\n", + "[tool.example]\ndescription='''\n[project]\nversion='7.0.0'\n'''\n[project]\nversion='0.1.0'\n", + "project.version='0.1.0'\n[tool.example]\nversion='7.0.0'\n", + "project={version='0.1.0'}\n", + } { + path := filepath.Join(t.TempDir(), "pyproject.toml") + require.NoError(t, os.WriteFile(path, []byte(before), 0o600)) + version, err := readPyprojectVersion(path) + require.NoError(t, err) + require.Equal(t, "0.1.0", version) + } +} diff --git a/cmd/publish/module_services.go b/cmd/publish/module_services.go index 896ed24d..bddd9b5b 100644 --- a/cmd/publish/module_services.go +++ b/cmd/publish/module_services.go @@ -7,8 +7,10 @@ import ( "os" "path/filepath" "sort" + "strings" "github.com/codefly-dev/cli/cmd/generate" + "github.com/codefly-dev/cli/pkg/generators" "github.com/codefly-dev/core/composition" "github.com/codefly-dev/core/resources" "gopkg.in/yaml.v3" @@ -99,6 +101,8 @@ func resolveModuleRelease(ctx context.Context, agentManifest, repoRoot string) ( return contained, nil } var rekey []*resources.Module + packages := map[string]generators.ClientReleasePackage{} + packageIDs := map[*resources.Module]string{} for _, ref := range workspace.Modules { if ref.Source != "" || ref.Module != "" || ref.Version != "" { fmt.Printf("==> module %s is a pinned reference, released from its own source; its services keep their versions\n", ref.Name) @@ -152,23 +156,133 @@ func resolveModuleRelease(ctx context.Context, agentManifest, repoRoot string) ( } release.generated = append(release.generated, contained) } + id, producer, loadErr := loadClientReleasePackage(module.Dir()) + if loadErr != nil { + return nil, loadErr + } + packages[id] = producer + packageIDs[module] = id rekey = append(rekey, module) } + clients, err := planReleasedClients(ctx, repoRoot, packages) + if err != nil { + return nil, err + } + for _, client := range clients { + release.generated = append(release.generated, client.Paths...) + } sort.Strings(release.manifests) - if len(rekey) > 0 { - name := workspace.Name - release.regenerate = func(ctx context.Context) error { - for _, module := range rekey { - if err := generate.RekeyReleasedContracts(ctx, name, dir, module); err != nil { - return err - } - } - return nil - } + release.regenerate = func(ctx context.Context) error { + return regenerateModuleClients(ctx, workspace.Name, dir, rekey, packageIDs, clients) } return release, nil } +func regenerateModuleClients(ctx context.Context, name, dir string, modules []*resources.Module, packageIDs map[*resources.Module]string, clients []*generators.ClientRelease) error { + versions := map[string]string{} + for _, module := range modules { + if err := generate.RekeyReleasedContracts(ctx, name, dir, module); err != nil { + return err + } + version, err := readVersion(filepath.Join(module.Dir(), composition.PackageManifestFileName)) + if err != nil { + return err + } + versions[packageIDs[module]] = version.String() + } + for _, client := range clients { + if err := client.Write(ctx, versions); err != nil { + return err + } + } + return nil +} + +// A module may not export contracts yet. Read its schema identity without +// requiring an exported artifact tree, and fully verify catalogs that exist. +func loadClientReleasePackage(dir string) (string, generators.ClientReleasePackage, error) { + producer := generators.ClientReleasePackage{} + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + return "", producer, err + } + producer.Directory = resolved + data, err := os.ReadFile(filepath.Join(dir, composition.PackageManifestFileName)) //nolint:gosec // contained module package + if err != nil { + return "", producer, err + } + var manifest composition.PackageManifest + if err = yaml.Unmarshal(data, &manifest); err != nil { + return "", producer, err + } + if err = manifest.Validate(); err != nil { + return "", producer, err + } + producer.Catalog, err = composition.LoadAPIContractCatalog(dir) + if errors.Is(err, os.ErrNotExist) { + return manifest.ID, producer, nil + } + if err != nil { + return "", producer, err + } + if producer.Catalog.Package != manifest.ID || producer.Catalog.Version != manifest.Version { + return "", producer, fmt.Errorf("module %s catalog disagrees with its package; regenerate contracts before release", dir) + } + return manifest.ID, producer, composition.ValidatePackageAPIContracts(dir, &manifest, producer.Catalog) +} + +// planReleasedClients discovers only committed library manifests, wherever their +// owners placed them. Untracked caches, foreign sources and hand-written +// libraries have no release-owned source to stamp. +func planReleasedClients(ctx context.Context, repoRoot string, packages map[string]generators.ClientReleasePackage) ([]*generators.ClientRelease, error) { + if len(packages) == 0 { + return nil, nil + } + root, err := filepath.EvalSymlinks(repoRoot) + if err != nil { + return nil, err + } + files, err := (&Engine{WorkDir: root}).git(ctx, "ls-files", "-z") + if err != nil { + return nil, err + } + tracked := map[string]bool{} + for _, path := range strings.Split(files, "\x00") { + tracked[filepath.Join(root, path)] = true + } + check := func(path string) error { + relative, err := filepath.Rel(root, path) + if err != nil { + return err + } + contained, err := releaseManifestPath(root, relative) + if err != nil { + return err + } + if contained != path { + return fmt.Errorf("client release path %s follows a symlink", path) + } + if !tracked[path] { + return fmt.Errorf("client release path %s is not git-tracked", path) + } + return nil + } + var plans []*generators.ClientRelease + for _, relative := range strings.Split(files, "\x00") { + if filepath.Base(relative) != resources.LibraryConfigurationName { + continue + } + plan, err := generators.PlanClientRelease(filepath.Dir(filepath.Join(root, relative)), packages, check) + if err != nil { + return nil, fmt.Errorf("release client %s: %w", relative, err) + } + if plan != nil { + plans = append(plans, plan) + } + } + return plans, nil +} + // agentKind reads only the kind an agent manifest declares. It is lenient on // purpose: the release gate validates the rest of the manifest, and this only // decides whether there is a module release to carry. diff --git a/cmd/publish/publication_credentials_test.go b/cmd/publish/publication_credentials_test.go new file mode 100644 index 00000000..4f96462f --- /dev/null +++ b/cmd/publish/publication_credentials_test.go @@ -0,0 +1,131 @@ +package publish + +import ( + "context" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/codefly-dev/cli/pkg/gh" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +type publicationTransport func(*http.Request) (*http.Response, error) + +func (f publicationTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// Exercise both real afterPush branches, including the workflow tag/commit +// match. A gh login can publish a private release but must not certify that an +// installer without that credential can fetch it; public downloads need none. +func TestPublicationOwnersUseInstallerCredentials(t *testing.T) { + for _, owner := range []string{publicationCLI, publicationWorkflow} { + for _, scenario := range []string{"anonymous public", "private with only gh login", "GH_TOKEN", "GITHUB_TOKEN wins", "bad archive"} { + t.Run(owner+"/"+scenario, func(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + t.Setenv("GITHUB_API_URL", "") + bin := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(bin, "gh"), []byte("#!/bin/sh\nprintf publisher-login"), 0700)) + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + expectedInstaller := "" + if scenario == "GH_TOKEN" { + t.Setenv("GH_TOKEN", "installer-gh") + expectedInstaller = "Bearer installer-gh" + } + if scenario == "GITHUB_TOKEN wins" { + t.Setenv("GH_TOKEN", "ignored-gh") + t.Setenv("GITHUB_TOKEN", "installer-github") + expectedInstaller = "Bearer installer-github" + } + reg, err := resources.AgentKindRegistrationFor(resources.AgentKind("codefly:service")) + require.NoError(t, err) + target := platform{os: "linux", arch: "amd64"} + name := loaderArchiveName(®, "go", "0.0.16", target) + assets := stageAssets(t, name) + assets[0].platform = target + files, err := stageReleaseChecksums(assets) + require.NoError(t, err) + payloads := make([]string, len(files)) + var entries []string + for i, file := range files { + raw, err := os.ReadFile(file) + require.NoError(t, err) + payloads[i] = string(raw) + url := "https://github.com/codefly-dev/service-go/releases/download/v0.0.16/" + filepath.Base(file) + entries = append(entries, fmt.Sprintf(`{"id":%d,"name":%q,"size":%d,"digest":%q,"browser_download_url":%q}`, i+1, filepath.Base(file), len(raw), digestOf(string(raw)), url)) + } + dir, _, _ := releaseRepo(t, "0.0.16") + gitIn(t, dir, "tag", "v0.0.16") + head := gitIn(t, dir, "rev-parse", "HEAD") + var installerSeen, publisherSeen bool + original := http.DefaultTransport + http.DefaultTransport = publicationTransport(func(req *http.Request) (*http.Response, error) { + require.Equal(t, "api.github.com", req.URL.Host, "unexpected network call") + require.Equal(t, http.MethodGet, req.Method, "complete release retry must not mutate") + auth := req.Header.Get("Authorization") + if auth == "Bearer publisher-login" { + publisherSeen = true + } + if auth == expectedInstaller { + installerSeen = true + } + status, contentType, body := http.StatusOK, "application/json", "" + if scenario == "private with only gh login" && auth == "" { + status, body = http.StatusNotFound, `{"message":"not found"}` + } else { + switch req.URL.Path { + case "/repos/codefly-dev/service-go/releases/tags/v0.0.16": + body = fmt.Sprintf(`{"id":42,"tag_name":"v0.0.16","assets":[%s]}`, strings.Join(entries, ",")) + case "/repos/codefly-dev/service-go/releases/assets/1": + contentType, body = "application/octet-stream", payloads[0] + if scenario == "bad archive" { + body = "corrupted" + } + case "/repos/codefly-dev/service-go/releases/assets/2": + contentType, body = "application/octet-stream", payloads[1] + case "/repos/codefly-dev/service-go/actions/workflows/release.yml/runs": + body = fmt.Sprintf(`{"workflow_runs":[{"id":1,"head_sha":%q,"head_branch":"v0.0.16","event":"push","status":"completed","conclusion":"success"}]}`, head) + default: + t.Fatalf("unexpected request: %s", req.URL) + } + } + return &http.Response{StatusCode: status, Header: http.Header{"Content-Type": {contentType}}, Body: io.NopCloser(strings.NewReader(body)), Request: req}, nil + }) + t.Cleanup(func() { http.DefaultTransport = original }) + r := &agentReleaser{agentDir: dir, reg: ®, publisher: "codefly.dev", name: "go", assets: assets, publication: agentPublication{Owner: owner, Workflow: "release.yml"}} + // afterPush detaches a caller timeout after a tag ships. For the + // reachability refusal, call its shared publication verifier with + // a short context so this regression never sleeps through retries. + if scenario == "private with only gh login" { + ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) + defer cancel() + client, err := gh.NewClient() + require.NoError(t, err) + err = r.verifyPublication(ctx, client, "codefly-dev", "service-go", "v0.0.16") + require.Error(t, err) + } else { + err = r.afterPush(t.Context(), "v0.0.16") + if scenario == "bad archive" { + require.ErrorIs(t, err, errAssetVerdict) + } else { + require.NoError(t, err) + } + } + if expectedInstaller == "" { + require.True(t, publisherSeen, "fixture must exercise gh login publication") + } + if scenario == "bad archive" { + require.False(t, installerSeen, "invalid archive must fail before installer reachability") + } else { + require.True(t, installerSeen, "must check actual installer credential") + } + }) + } + } +} diff --git a/cmd/publish/release_checksums.go b/cmd/publish/release_checksums.go new file mode 100644 index 00000000..c3f5af52 --- /dev/null +++ b/cmd/publish/release_checksums.go @@ -0,0 +1,75 @@ +package publish + +import ( + "crypto/sha256" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/google/go-github/v89/github" +) + +// stageReleaseChecksums supplies the same producer evidence as workflow-owned +// publication. Upload archives first and their complete checksum list last; +// retries use the same sorted bytes and never replace an existing asset. +func stageReleaseChecksums(assets []loaderAsset) ([]string, error) { + var files []string + for _, asset := range assets { + files = append(files, asset.archivePath) + if asset.sbomPath != "" { + files = append(files, asset.sbomPath) + } + } + if len(files) == 0 { + return nil, fmt.Errorf("a release publishes no assets to checksum") + } + sort.Slice(files, func(i, j int) bool { return filepath.Base(files[i]) < filepath.Base(files[j]) }) + var checksums strings.Builder + for i, file := range files { + name := filepath.Base(file) + if name == "checksums.txt" || (i > 0 && name == filepath.Base(files[i-1])) { + return nil, fmt.Errorf("duplicate release asset name %q", name) + } + digest, err := releaseFileDigest(file) + if err != nil { + return nil, err + } + fmt.Fprintf(&checksums, "%s %s\n", digest, name) + } + target := filepath.Join(filepath.Dir(files[0]), "checksums.txt") + if err := os.WriteFile(target, []byte(checksums.String()), 0o600); err != nil { + return nil, err + } + return append(files, target), nil +} + +func releaseFileDigest(path string) (string, error) { + file, err := os.Open(path) + if err != nil { + return "", err + } + defer file.Close() + digest := sha256.New() + if _, err := io.Copy(digest, file); err != nil { + return "", err + } + return fmt.Sprintf("%x", digest.Sum(nil)), nil +} + +func validateExistingReleaseAsset(path string, existing map[string]*github.ReleaseAsset) error { + asset, exists := existing[filepath.Base(path)] + if !exists { + return nil + } + digest, err := releaseFileDigest(path) + if err != nil { + return err + } + if asset.GetDigest() != "sha256:"+digest { + return fmt.Errorf("release asset %s already exists with different or unverifiable bytes; published assets cannot be replaced", filepath.Base(path)) + } + return nil +} diff --git a/cmd/publish/release_checksums_test.go b/cmd/publish/release_checksums_test.go new file mode 100644 index 00000000..f9da0674 --- /dev/null +++ b/cmd/publish/release_checksums_test.go @@ -0,0 +1,102 @@ +package publish + +import ( + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestStageReleaseChecksumsIsDeterministicAndComplete(t *testing.T) { + assets := stageAssets(t, "z.tar.gz", "a.tar.gz") + assets[0].sbomPath = stageFile(t, filepath.Dir(assets[0].archivePath), "z.cdx.json") + files, err := stageReleaseChecksums(assets) + require.NoError(t, err) + var names []string + for _, file := range files { + names = append(names, filepath.Base(file)) + } + require.Equal(t, []string{"a.tar.gz", "z.cdx.json", "z.tar.gz", "checksums.txt"}, names) + var expected string + for _, name := range names[:3] { + expected += fmt.Sprintf("%x %s\n", sha256.Sum256([]byte("payload-"+name)), name) + } + raw, err := os.ReadFile(files[3]) + require.NoError(t, err) + require.Equal(t, expected, string(raw)) + _, err = stageReleaseChecksums([]loaderAsset{assets[1], assets[0]}) + require.NoError(t, err) + retried, err := os.ReadFile(files[3]) + require.NoError(t, err) + require.Equal(t, raw, retried) +} + +func TestStageReleaseChecksumsRefusesAmbiguousNames(t *testing.T) { + for _, names := range [][]string{{"same.tar.gz", "same.tar.gz"}, {"checksums.txt"}} { + _, err := stageReleaseChecksums(stageAssets(t, names...)) + require.ErrorContains(t, err, "duplicate release asset name") + } +} + +func TestReleaseRetryPreflightsEveryImmutableAssetBeforeMutation(t *testing.T) { + const tag = "v0.0.16" + for _, scenario := range []string{"complete identical retry", "late archive conflict", "checksum conflict"} { + t.Run(scenario, func(t *testing.T) { + assets := stageAssets(t, "a.tar.gz", "z.tar.gz") + files, err := stageReleaseChecksums(assets) + require.NoError(t, err) + existing := map[string]int64{} + f := &fakeGitHub{digests: map[string]string{}} + for i, file := range files { + raw, err := os.ReadFile(file) + require.NoError(t, err) + existing[filepath.Base(file)] = int64(i + 1) + f.digests[filepath.Base(file)] = fmt.Sprintf("sha256:%x", sha256.Sum256(raw)) + } + switch scenario { + case "late archive conflict": + delete(existing, "a.tar.gz") // would upload before discovering z's conflict + require.NoError(t, os.WriteFile(assets[1].archivePath, []byte("different"), 0600)) + case "checksum conflict": + delete(existing, "a.tar.gz") + f.digests["checksums.txt"] = digestOf("old evidence") + } + client := f.server(t, tag, existing) + err = createAndUploadRelease(t.Context(), client, "codefly-dev", "service-go", tag, assets) + if scenario == "complete identical retry" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, "cannot be replaced") + } + require.False(t, f.created) + require.Empty(t, f.uploaded) + require.Empty(t, f.deleted) + }) + } +} + +func TestVerifyPublishedArchivesChecksExplicitSBOM(t *testing.T) { + const sbom = "service-go.cdx.json" + for _, scenario := range []string{"present", "missing", "wrong bytes", "wrong producer digest"} { + t.Run(scenario, func(t *testing.T) { + p := publishedBy(t, map[string]string{testArchive: "archive bytes", sbom: "sbom bytes"}) + switch scenario { + case "missing": + delete(p.payloads, sbom) + case "wrong bytes": + p.payloads[sbom] = "different sbom" + case "wrong producer digest": + p.checksumDigests = map[string]string{sbom: fmt.Sprintf("%x", sha256.Sum256([]byte("old sbom")))} + } + err := p.withChecksums().verify([]loaderAsset{{archivePath: testArchive, sbomPath: sbom}}) + if scenario == "present" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, sbom) + } + }) + } +} diff --git a/cmd/publish/release_upload_test.go b/cmd/publish/release_upload_test.go index af2169d5..dc4fd3bb 100644 --- a/cmd/publish/release_upload_test.go +++ b/cmd/publish/release_upload_test.go @@ -18,9 +18,10 @@ import ( // fakeGitHub records the release-API calls createAndUploadRelease makes so the // create vs. clobber behavior can be asserted without reaching GitHub. type fakeGitHub struct { - uploaded []string // asset names uploaded, in order - deleted []int64 // asset ids deleted (the clobber step) - created bool // whether a release was created + uploaded []string // asset names uploaded, in order + deleted []int64 // asset ids deleted (the clobber step) + created bool // whether a release was created + digests map[string]string // explicit existing producer-evidence digests } // server stands up an httptest server speaking enough of the releases API and @@ -36,12 +37,12 @@ func (f *fakeGitHub) server(t *testing.T, tag string, existingAssets map[string] http.Error(w, "not found", http.StatusNotFound) return } - writeRelease(w, releaseID, existingAssets) + writeRelease(w, releaseID, existingAssets, f.digests) }) mux.HandleFunc("/repos/codefly-dev/service-go/releases", func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method) f.created = true - writeRelease(w, releaseID, nil) + writeRelease(w, releaseID, nil, nil) }) mux.HandleFunc(fmt.Sprintf("/repos/codefly-dev/service-go/releases/%d/assets", releaseID), func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method) @@ -65,10 +66,14 @@ func (f *fakeGitHub) server(t *testing.T, tag string, existingAssets map[string] return client } -func writeRelease(w http.ResponseWriter, id int64, assets map[string]int64) { +func writeRelease(w http.ResponseWriter, id int64, assets map[string]int64, digests map[string]string) { var parts []string for name, aid := range assets { - parts = append(parts, fmt.Sprintf(`{"id":%d,"name":%q,"digest":"sha256:%x"}`, aid, name, sha256.Sum256([]byte("payload-"+name)))) + digest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte("payload-"+name))) + if value, ok := digests[name]; ok { + digest = value + } + parts = append(parts, fmt.Sprintf(`{"id":%d,"name":%q,"digest":%q}`, aid, name, digest)) } fmt.Fprintf(w, `{"id":%d,"assets":[%s]}`, id, strings.Join(parts, ",")) } @@ -112,6 +117,7 @@ func TestCreateAndUploadRelease_CreatesWhenAbsent(t *testing.T) { require.ElementsMatch(t, []string{ "service-go_0.0.16_darwin_arm64.tar.gz", "service-go_0.0.16_linux_amd64.tar.gz", + "checksums.txt", }, f.uploaded) require.Empty(t, f.deleted, "nothing to clobber on a fresh release") } @@ -126,7 +132,7 @@ func TestCreateAndUploadRelease_ReusesIdenticalAssetWithoutMutation(t *testing.T require.False(t, f.created, "an existing release must be reused, not recreated") require.Empty(t, f.deleted) - require.Empty(t, f.uploaded) + require.Equal(t, []string{"checksums.txt"}, f.uploaded, "fill missing evidence without replacing the archive") } func TestCreateAndUploadRelease_UploadsSBOMAlongsideArchive(t *testing.T) { @@ -138,7 +144,7 @@ func TestCreateAndUploadRelease_UploadsSBOMAlongsideArchive(t *testing.T) { require.NoError(t, createAndUploadRelease(context.Background(), client, "codefly-dev", "service-go", "v0.0.16", assets)) - require.ElementsMatch(t, []string{archive, sbom}, f.uploaded, + require.ElementsMatch(t, []string{archive, sbom, "checksums.txt"}, f.uploaded, "both the archive and its SBOM must be uploaded") } @@ -153,7 +159,7 @@ func TestCreateAndUploadRelease_ReusesIdenticalSBOMWithoutMutation(t *testing.T) require.False(t, f.created, "an existing release must be reused, not recreated") require.Empty(t, f.deleted) - require.Empty(t, f.uploaded) + require.Equal(t, []string{"checksums.txt"}, f.uploaded, "fill missing evidence without replacing existing bytes") } func TestCreateAndUploadRelease_RejectsChangedBytesWithoutDeletingAsset(t *testing.T) { diff --git a/cmd/publish/workflow_release.go b/cmd/publish/workflow_release.go index 29bea2f7..e7892974 100644 --- a/cmd/publish/workflow_release.go +++ b/cmd/publish/workflow_release.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "crypto/sha256" + "encoding/hex" "errors" "fmt" "io" @@ -122,10 +123,7 @@ func (r *agentReleaser) waitForWorkflowRelease(ctx context.Context, client *gith // failed, so what follows runs on a budget of its own. ctx, cancel := postPublicationContext(ctx) defer cancel() - if err := verifyWorkflowRelease(ctx, client, owner, repo, tag, r.assets); err != nil { - return err - } - return verifyReleaseAssets(ctx, client, r.reg, r.publisher, r.name, strings.TrimPrefix(tag, "v"), r.assets) + return r.verifyPublication(ctx, client, owner, repo, tag) } // releaseVerifyBudget sizes the work that follows a live tag: downloading every @@ -136,22 +134,21 @@ func postPublicationContext(ctx context.Context) (context.Context, context.Cance return context.WithTimeout(context.WithoutCancel(ctx), releaseVerifyBudget) } -// verifyWorkflowRelease confirms the release the owner workflow published -// carries every loader archive, and that each one is the build the workflow -// made. The workflow is the sole publisher of those assets, so every call here -// reads: nothing is created, replaced or deleted. +// verifyPublishedArchives checks both CLI-owned and workflow-owned publication +// against producer checksums, GitHub digests and the downloaded bytes. Every +// call here reads: nothing is created, replaced or deleted. // // The archive is the contract — core's downloader fetches exactly that name. // Whether a workflow also publishes an SBOM beside it, and under what name, is // its own GoReleaser configuration and nothing the manifest declares, so one is -// verified when the release carries it and never demanded. -func verifyWorkflowRelease(ctx context.Context, client *github.Client, owner, repo, tag string, staged []loaderAsset) error { +// verified when present. A CLI-staged SBOM has a known filename and is required. +func verifyPublishedArchives(ctx context.Context, client *github.Client, owner, repo, tag string, staged []loaderAsset) error { release, _, err := client.Repositories.GetReleaseByTag(ctx, owner, repo, tag) if err != nil { - return fmt.Errorf("read workflow-published release: %w", err) + return fmt.Errorf("read published release: %w", err) } if release.GetTagName() != tag || release.GetDraft() { - return fmt.Errorf("workflow did not publish release %s", tag) + return fmt.Errorf("release %s is absent or still draft", tag) } assets := make(map[string]*github.ReleaseAsset) for _, asset := range release.Assets { @@ -165,12 +162,19 @@ func verifyWorkflowRelease(ctx context.Context, client *github.Client, owner, re archiveName := filepath.Base(candidate.archivePath) archive, ok := assets[archiveName] if !ok { - return fmt.Errorf("workflow release %s is missing loader archive %s", tag, archiveName) + return fmt.Errorf("release %s is missing loader archive %s", tag, archiveName) } if err := verifyPublishedRelease(ctx, client, owner, repo, archive, checksums); err != nil { return err } - if sbom, ok := assets[archiveName+".sbom.json"]; ok { + sbomName := archiveName + ".sbom.json" + if candidate.sbomPath != "" { + sbomName = filepath.Base(candidate.sbomPath) + if assets[sbomName] == nil { + return fmt.Errorf("release %s is missing staged SBOM %s", tag, sbomName) + } + } + if sbom, ok := assets[sbomName]; ok { if err := verifyPublishedRelease(ctx, client, owner, repo, sbom, checksums); err != nil { return err } @@ -180,16 +184,16 @@ func verifyWorkflowRelease(ctx context.Context, client *github.Client, owner, re } // verifyPublishedRelease establishes one asset three ways: the digest the -// workflow computed from its own build output, the digest GitHub recorded when +// publisher computed from its own build output, the digest GitHub recorded when // it accepted the upload, and the bytes GitHub actually serves. Comparing the // served bytes to GitHub's own record alone would only show GitHub agreeing // with itself; the checksums file is the independent producer that makes the -// comparison mean something. Core's downloader verifies nothing at install -// time, so this is the only place in the chain the bytes are ever checked. +// comparison mean something. Install-time verification is owned separately +// by Core's downloader; publication does not replace that check. func verifyPublishedRelease(ctx context.Context, client *github.Client, owner, repo string, asset *github.ReleaseAsset, checksums map[string]string) error { built, ok := checksums[asset.GetName()] if !ok { - return fmt.Errorf("%w: %s is absent from the checksums the workflow published", errAssetVerdict, asset.GetName()) + return fmt.Errorf("%w: %s is absent from the published checksums", errAssetVerdict, asset.GetName()) } if recorded := asset.GetDigest(); recorded != "sha256:"+built { return fmt.Errorf("%w: %s was built as sha256:%s but the release records %s", @@ -200,10 +204,8 @@ func verifyPublishedRelease(ctx context.Context, client *github.Client, owner, r }) } -// publishedChecksums reads the digest list the release workflow computed from -// its own build output before uploading anything. It is required: without it -// the release carries no digest source independent of GitHub's own record, and -// nothing downstream ever checks these archives again. +// publishedChecksums reads the producer digest list. It is required: without +// it publication has no digest source independent of GitHub's upload record. func publishedChecksums(ctx context.Context, client *github.Client, owner, repo, tag string, assets map[string]*github.ReleaseAsset) (map[string]string, error) { var found *github.ReleaseAsset for name, asset := range assets { @@ -211,13 +213,16 @@ func publishedChecksums(ctx context.Context, client *github.Client, owner, repo, continue } if found != nil { - return nil, fmt.Errorf("workflow release %s publishes more than one checksums file (%s, %s); which one records the archives is ambiguous", + return nil, fmt.Errorf("release %s publishes more than one checksums file (%s, %s); which one records the archives is ambiguous", tag, found.GetName(), name) } found = asset } if found == nil { - return nil, fmt.Errorf("workflow release %s publishes no checksums file, so its loader archives have no digest source independent of GitHub's own record (GoReleaser emits one unless `checksum: disable: true`)", tag) + return nil, fmt.Errorf("release %s publishes no checksums file, so its loader archives have no digest source independent of GitHub's own record (GoReleaser emits one unless `checksum: disable: true`)", tag) + } + if found.GetSize() > 1<<20 { + return nil, fmt.Errorf("%w: release checksums exceed 1 MiB", errAssetVerdict) } var raw bytes.Buffer if err := retryTransientRead(ctx, func() error { @@ -226,20 +231,36 @@ func publishedChecksums(ctx context.Context, client *github.Client, owner, repo, }); err != nil { return nil, err } - return parseChecksums(raw.Bytes()), nil + return parseChecksums(raw.Bytes()) } -// parseChecksums reads ` ` lines. The leading `*` of sha256sum's -// binary mode is stripped so a workflow that emits it still resolves by name. -func parseChecksums(raw []byte) map[string]string { +// parseChecksums reads SHA-256 records and refuses ambiguous producer evidence. +// sha256sum's binary-mode marker is accepted alongside its text-mode form. +func parseChecksums(raw []byte) (map[string]string, error) { checksums := map[string]string{} scanner := bufio.NewScanner(bytes.NewReader(raw)) for scanner.Scan() { - if fields := strings.Fields(scanner.Text()); len(fields) == 2 { - checksums[strings.TrimPrefix(fields[1], "*")] = fields[0] + fields := strings.Fields(scanner.Text()) + if len(fields) == 0 { + continue + } + if len(fields) != 2 { + return nil, fmt.Errorf("%w: malformed checksum entry", errAssetVerdict) + } + digest, err := hex.DecodeString(fields[0]) + if err != nil || len(digest) != sha256.Size { + return nil, fmt.Errorf("%w: invalid SHA-256 checksum", errAssetVerdict) } + name := strings.TrimPrefix(fields[1], "*") + if _, exists := checksums[name]; exists { + return nil, fmt.Errorf("%w: duplicate checksum for %s", errAssetVerdict, name) + } + checksums[name] = strings.ToLower(fields[0]) + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("%w: read checksums: %v", errAssetVerdict, err) } - return checksums + return checksums, nil } // retryTransientRead re-runs read while it fails for a reason a re-read could diff --git a/cmd/publish/workflow_release_test.go b/cmd/publish/workflow_release_test.go index b81fc4a7..f4c8088e 100644 --- a/cmd/publish/workflow_release_test.go +++ b/cmd/publish/workflow_release_test.go @@ -231,7 +231,7 @@ func (p *publishedRelease) client() *github.Client { } func (p *publishedRelease) verify(staged []loaderAsset) error { - return verifyWorkflowRelease(p.t.Context(), p.client(), "codefly-dev", "service-go", p.tag, staged) + return verifyPublishedArchives(p.t.Context(), p.client(), "codefly-dev", "service-go", p.tag, staged) } func TestVerifyWorkflowReleaseReadsPublishedAssetsWithoutWriting(t *testing.T) { @@ -269,7 +269,7 @@ func TestVerifyWorkflowReleaseReadsPublishedAssetsWithoutWriting(t *testing.T) { r.draft = true return r }, - wantErr: "workflow did not publish release", + wantErr: "absent or still draft", }, { name: "no checksums file", @@ -296,7 +296,7 @@ func TestVerifyWorkflowReleaseReadsPublishedAssetsWithoutWriting(t *testing.T) { r.omitFromChecksums = map[string]bool{testArchive: true} return r.withChecksums() }, - wantErr: testArchive + " is absent from the checksums", + wantErr: testArchive + " is absent from the published checksums", }, { name: "release records a digest the workflow did not build", @@ -400,7 +400,7 @@ func TestPostPublicationVerificationOutlivesTheSpentPublishBudget(t *testing.T) ctx, cancel := postPublicationContext(spent) defer cancel() - err := verifyWorkflowRelease(ctx, client, "codefly-dev", "service-go", "v0.0.16", stageAssets(t, testArchive)) + err := verifyPublishedArchives(ctx, client, "codefly-dev", "service-go", "v0.0.16", stageAssets(t, testArchive)) require.NoError(t, err) require.ErrorIs(t, spent.Err(), context.Canceled, "the publish budget must be spent by the time verification finishes") @@ -410,10 +410,21 @@ func TestPostPublicationVerificationOutlivesTheSpentPublishBudget(t *testing.T) } func TestParseChecksumsReadsBothSha256sumForms(t *testing.T) { - raw := "aaaa text-mode.tar.gz\nbbbb *binary-mode.tar.gz\n\ngarbage\ncccc extra fields\n" + digest := strings.TrimPrefix(digestOf("archive"), "sha256:") + raw := digest + " text-mode.tar.gz\n" + strings.ToUpper(digest) + " *binary-mode.tar.gz\n\n" + parsed, err := parseChecksums([]byte(raw)) + require.NoError(t, err) + require.Equal(t, map[string]string{"text-mode.tar.gz": digest, "binary-mode.tar.gz": digest}, parsed) +} - require.Equal(t, map[string]string{ - "text-mode.tar.gz": "aaaa", - "binary-mode.tar.gz": "bbbb", - }, parseChecksums([]byte(raw))) +func TestParseChecksumsRefusesAmbiguousOrMalformedEvidence(t *testing.T) { + digest := strings.TrimPrefix(digestOf("archive"), "sha256:") + for _, raw := range []string{ + "garbage", "abc archive", strings.Repeat("x", 64) + " archive", + digest + " archive\n" + digest + " archive\n", + digest + " archive extra", strings.Repeat("x", 70<<10), + } { + _, err := parseChecksums([]byte(raw)) + require.ErrorIs(t, err, errAssetVerdict) + } } diff --git a/cmd/stop_identity_fallback_test.go b/cmd/stop_identity_fallback_test.go new file mode 100644 index 00000000..23855942 --- /dev/null +++ b/cmd/stop_identity_fallback_test.go @@ -0,0 +1,127 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "github.com/codefly-dev/cli/pkg/processgroup" + runners "github.com/codefly-dev/core/runners/base" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +func TestStopIdentityFallbackOwner(t *testing.T) { + target := os.Getenv("CODEFLY_TEST_STOP_IDENTITY_CHILD_PID") + if target == "" { + return + } + child := exec.Command(os.Getenv("CODEFLY_TEST_STOP_IDENTITY_CHILD_BINARY"), "-test.run=^TestPsHelper$") + child.Env = append(os.Environ(), psHelperEnv+"=1") + child.Stdout = io.Discard + child.Stderr = io.Discard + group, err := runners.StartTrackedProcessGroup(child) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(target, []byte(strconv.Itoa(group.PGID())), 0600); err != nil { + t.Fatal(err) + } +} +func TestStopDoesNotFallbackAfterBootIdentityRefusal(t *testing.T) { testStopIdentityRefusal(t, false) } +func TestStopDoesNotKillAgentBeforeBootIdentityRefusal(t *testing.T) { + testStopIdentityRefusal(t, true) +} +func testStopIdentityRefusal(t *testing.T, agentShape bool) { + t.Setenv("HOME", t.TempDir()) + workspace := psWorkspace(t) + t.Chdir(workspace) + t.Setenv(processgroup.LaunchWorkspaceEnv, workspace) + binary := filepath.Join(t.TempDir(), ".cache", "native", "stop-child") + if agentShape { + binary = filepath.Join(t.TempDir(), ".codefly", "agents", "stop-child") + } + if err := os.MkdirAll(filepath.Dir(binary), 0700); err != nil { + t.Fatal(err) + } + executable, err := os.ReadFile(os.Args[0]) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(binary, executable, 0700); err != nil { + t.Fatal(err) + } + t.Setenv("CODEFLY_TEST_STOP_IDENTITY_CHILD_BINARY", binary) + pidFile := filepath.Join(t.TempDir(), "pid") + owner := exec.Command(os.Args[0], "-test.run=^TestStopIdentityFallbackOwner$") + owner.Env = append(os.Environ(), "CODEFLY_TEST_STOP_IDENTITY_CHILD_PID="+pidFile) + if out, err := owner.CombinedOutput(); err != nil { + t.Fatalf("owner: %v %s", err, out) + } + raw, err := os.ReadFile(pidFile) + if err != nil { + t.Fatal(err) + } + pid, err := strconv.Atoi(string(raw)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) + path := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v2", fmt.Sprintf("%d.pgid", pid)) + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var record map[string]any + if err = json.Unmarshal(data, &record); err != nil { + t.Fatal(err) + } + for _, k := range []string{"leader", "owner"} { + record[k].(map[string]any)["boot_id"] = "incompatible-recorded-boot" + } + data, err = json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + if !agentShape { + deadline := time.Now().Add(5 * time.Second) + for { + rows, scanErr := scanRunProcesses(context.Background(), processgroup.InWorkspace(workspace)) + if scanErr != nil { + t.Fatal(scanErr) + } + found := false + for _, row := range rows { + found = found || (row.PID == pid && row.Kind == "native" && row.Status == "orphaned") + } + if found { + break + } + if time.Now().After(deadline) { + t.Fatal("child never became eligible for orphan fallback") + } + time.Sleep(10 * time.Millisecond) + } + } + + err = clearCommand(context.Background(), nil, stopClearOptions()) + if err == nil || !strings.Contains(err.Error(), "incompatible boot identity") { + t.Fatalf("expected boot refusal: %v", err) + } + if syscall.Kill(pid, 0) != nil { + t.Fatalf("stop killed incompatible registered group %d through fallback after refusal", pid) + } + if retained, readErr := os.ReadFile(path); readErr != nil || !bytes.Equal(retained, data) { + t.Fatalf("stop changed incompatible record: %v", readErr) + } +} diff --git a/cmd/stop_identity_test.go b/cmd/stop_identity_test.go new file mode 100644 index 00000000..bedbfa67 --- /dev/null +++ b/cmd/stop_identity_test.go @@ -0,0 +1,73 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" + "time" + + "github.com/codefly-dev/cli/pkg/processgroup" + runners "github.com/codefly-dev/core/runners/base" +) + +func TestStopReportsIncompatibleBootIdentityWithoutDiscardingEvidence(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + workspace := psWorkspace(t) + t.Chdir(workspace) + t.Setenv(processgroup.LaunchWorkspaceEnv, workspace) + child := exec.Command(os.Args[0], "-test.run=^TestPsHelper$") + child.Env = append(os.Environ(), psHelperEnv+"=1", processgroup.LaunchWorkspaceEnv+"="+workspace) + child.Dir = workspace + group, err := runners.StartTrackedProcessGroup(child) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Process.Kill(); _ = child.Wait() }) + deadline := time.Now().Add(5 * time.Second) + for { + if found, ok := processgroup.LaunchWorkspaceOfProcess(child.Process.Pid); ok && found == workspace { + break + } + if time.Now().After(deadline) { + t.Fatal("child never inherited workspace") + } + time.Sleep(time.Millisecond) + } + path := filepath.Join(home, ".codefly", "runs", "authenticated-v2", fmt.Sprintf("%d.pgid", group.PGID())) + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var record map[string]any + if err := json.Unmarshal(data, &record); err != nil { + t.Fatal(err) + } + for _, key := range []string{"leader", "owner"} { + record[key].(map[string]any)["boot_id"] = "incompatible-recorded-boot" + } + data, err = json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + err = clearCommand(context.Background(), nil, stopClearOptions()) + if err == nil || !strings.Contains(err.Error(), "incompatible boot identity") { + t.Fatalf("stop reported %v", err) + } + if retained, err := os.ReadFile(path); err != nil || !bytes.Equal(retained, data) { + t.Fatalf("stop changed recovery evidence: %v", err) + } + if err := syscall.Kill(child.Process.Pid, 0); err != nil { + t.Fatalf("stop signaled unauthenticated process: %v", err) + } +} diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 45bdc9f4..8ab32321 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -3036,7 +3036,7 @@ Flags: ``` --call-endpoint module/service/endpoint declared HTTP-based endpoint, as module/service/endpoint, every prepared call is sent to instead of the owner's listener, overriding the environment's runnable-call-endpoint (empty: each owner's listener; the owner's operation, route, contract and policy are kept) - --check do not write; exit 1 if the on-disk file differs from what would be generated + --check leave binding output unchanged; materialize selected pins, then exit 1 if the output is stale --env string environment supplying resolved addresses and runnable-scope-selections (default "local") --naming-scope scope scope addresses resolve in, as codefly run --naming-scope reads it: omitted keeps the environment's, empty clears it ``` @@ -3832,16 +3832,17 @@ List the processes this workspace's run is still holding ``` List the processes a codefly run is keeping alive: frontend dev servers (next dev / npm run dev / vite) and native-mode services (compiled service -binaries and the stateful stores a run started), including those belonging to -composed modules checked out outside the workspace. +binaries and PostgreSQL), including composed modules when their launch +environment or working directory identifies the workspace. This signature scan +can omit registered processes whose command or environment has changed; stop +uses authenticated registry discovery independently. -Scoped to the current workspace — the same set 'codefly stop' acts on. Use ---all for every workspace on this machine. Outside a workspace the listing is -machine-wide, since there is nothing to scope to. +Scoped to the current workspace. Use --all for every workspace on this machine. +Outside a workspace the listing is machine-wide. -STATUS is one of: orphaned (codefly's, escaped its supervisor — reaped by -'codefly clear'), tracked (codefly's, still supervised), or external (not -codefly's — shown for visibility, never reaped). +STATUS is diagnostic: orphaned (marked process without its supervisor), tracked +(marked process still supervised), or external (no marker). A marker or status +never authorizes cleanup: clear must authenticate a registered identity. ``` ``` diff --git a/docs/commands.md b/docs/commands.md index 42dd9b0c..e196cd51 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2931,6 +2931,17 @@ publishing host, so on a linux/amd64 runner that qualification is the linux/amd64 build only. An agent with `release.owner: cli` uploads the archives itself and still requires a host that builds every platform. +Both publication owners verify the published archive bytes before reporting +success. CLI-owned uploads also stage deterministic checksums and refuse changed +existing assets before uploading anything. The final reachability check uses +installer credentials, independently of the publishing login. See +[publication verification](publish-verification.md). + +Module releases also carry their tracked generated client versions through the +existing release transaction; stale clients must be regenerated first. See +[generated client versions](publish-client-versions.md) for single-source and +aggregate behavior. + ### `codefly publish all [patch|minor|major]` Discover every git repository under the workspace that carries a codefly @@ -3902,7 +3913,7 @@ absent or empty exactly when the environment selects no binding. | Flag | Description | |------|-------------| | `--env` | Environment supplying resolved addresses and `runnable-scope-selections` (default: `local`) | -| `--check` | Regenerate in memory; exit non-zero if the output is stale, including after a selection or a decline changes; write nothing | +| `--check` | Regenerate in memory; exit non-zero if the output is stale, including after a selection or a decline changes. Leaves binding output unchanged; first materializes selected pins through the common loader, which may update the cache, local overlay and resolution receipts. | ## Infrastructure @@ -4210,7 +4221,11 @@ What it checks, in order: operations that cannot be read at all fail as `runnable_operations_unreadable`. Like the checks below, it needs every composed module materialized. -6. Every workspace configuration group services declare under +6. Services withdrawn by the selected environment's `exclude-services` impose + no configuration requirements, including in a module-scoped check. The same + exclusion validation used by run still refuses undeclared consumer withdrawals; + explicitly selecting an excluded service fails. + Every workspace configuration group remaining services declare under `workspace-configuration-dependencies` is provided and defines values. The doctor reads exactly what a run provisions: the workspace's own `configurations//*` composed with the groups each composed module @@ -4225,7 +4240,13 @@ What it checks, in order: (nothing derived, or every binding declined) the group absent or holding no value is ok; a selected binding the group does not hold fails as `configuration_missing` with `codefly generate runnable-bindings --env - ` as its remedy. + ` as its remedy. Existing binding values are also compared with the + generator's current preparation: changed addresses, contracts or policies, + obsolete extra bindings and output left after the last operation disappears + fail as `configuration_invalid`. This comparison checks effective values; + the generator's `--check` additionally checks exact file bytes. The delivery + readiness gate uses this check before build or push for modules consuming + the group; the explicit `--skip-workspace-readiness` override still applies. 7. Per-service `configurations/` files parse; duplicates are flagged. 8. Every `${endpoint://}` reference in a workspace configuration a service in scope declares names a service of the workspace @@ -4332,33 +4353,27 @@ Replay recorded operations. ### `codefly clear` -Clear cached state and temporary files. Also reaps leaked frontend dev servers -(`next dev` / `npm run dev` / `vite`) that codefly spawned — proven by their -process-group authentication — and that lost their supervisor: the leaks -`codefly ps` reports as `orphaned`. Dev servers the user launched by hand (shown -as `external`) are never touched. +Clear cached state and temporary files. Frontend/native signature scans identify +additional cleanup candidates. Recovery requires Core's authenticated process-group +registration and owner-death evidence; a name or environment marker alone never +permits signaling. Missing or ambiguous proof leaves the process running and +returns an error. Registry failures prevent fallback cleanup from bypassing refusal. ### `codefly ps` -List the processes this workspace's run is still holding: frontend dev servers -(`next dev` / `npm run dev` / `vite`) and native-mode services — the compiled -service binaries and the stateful stores a run started. STATUS is `orphaned` -(codefly's, escaped its supervisor — reaped by `codefly clear`), `tracked` -(codefly's, still supervised), or `external` (not codefly's — shown for -visibility, never reaped). Add `--json` for machine-readable output. - -A process belongs to the run that launched it, not to the directory it runs in. -That is the difference that makes a composed run listable at all: a composed -module is checked out outside the workspace that composes it, so its services -run from that checkout, and a store runs from a data directory under -`~/.codefly/data` that is in no workspace at all. The run is recorded in the -environment of everything a `codefly` invocation starts, so `ps` lists a -composed run whole — which matters because it is the set `codefly stop` acts on. - -Scoped to the current workspace. `--all` lists every workspace's processes on -this machine, which is how to find a leak belonging to a checkout you are not -standing in; outside a workspace the listing is machine-wide, since there is -nothing to scope to. +List signature-matched frontend dev servers (`next dev` / `npm run dev` / `vite`) +and native service binaries or PostgreSQL. Add `--json` for machine-readable +output. STATUS is diagnostic: `orphaned` means a marked process lost its +supervisor, `tracked` means it remains supervised, and `external` means no marker +was observed. These labels do not establish permission to signal a process. + +The view uses launch environment and working directory for workspace attribution. +It can omit registered children that rewrite their command/environment, including +Redis. `codefly stop` independently enumerates Core registrations and selects +persisted authenticated launch provenance; its coverage is broader than this view. + +Scoped to the current workspace; `--all` widens the diagnostic listing. Outside a +workspace the listing is machine-wide. --- diff --git a/docs/orchestration.md b/docs/orchestration.md index d5d9df99..bf843d9f 100644 --- a/docs/orchestration.md +++ b/docs/orchestration.md @@ -1129,6 +1129,32 @@ HTTP readiness deliberately stops at TCP reachability. Probing `/` would invent health contract and can itself keep a development server unready. ## Teardown +`codefly stop` discovers supported process-group registrations through Core, +which authenticates their recorded identities and owns signaling and removal. +The command does not kill CLI/agent processes by executable name or path. +The CLI selects the current workspace using the authenticated launch workspace +persisted at spawn. This survives an exited controller and children that rewrite +their environment. Independently built agents must adopt this Core producer; +older registrations without that metadata retain environment/cwd attribution. +Historical root records remain a separate compatibility path. A root record cannot override +a surviving supported registration's refusal or workspace selection. + +Signature scans can find additional frontend/native candidates, but a process +name or environment marker grants no signal authority. Missing or unproven +registered ownership leaves the process running and reports an error. Registry +inspection failure also prevents the legacy and signature fallback paths from +overriding the refusal. An older CLI that cannot read the new namespace cannot +recover its groups; upgrade the cleanup consumer along with the producer. + +`codefly stop` retains an authenticated process-group record and returns an +error when a live leader has the recorded start identity but a different boot +identity. Older Darwin agents wrote a boot timestamp where current agents write +a boot-session UUID. This mismatch does not prove PID reuse: cleanup neither +signals the group nor discards its recovery evidence. Recover the owning run +and update its agent before restarting. A successful stop must not hide a +process it could not authenticate. If registry cleanup fails, stop also skips +the signature-based orphan fallback so it cannot bypass that refusal. + `Stop()` and `Shutdown()` do not fan every manager out at once. Launch order is not completion order, so stopping a database concurrently with the API draining into it can cut off that API's last write. Both walk **reverse topological diff --git a/docs/publish-client-versions.md b/docs/publish-client-versions.md new file mode 100644 index 00000000..32e2fa0a --- /dev/null +++ b/docs/publish-client-versions.md @@ -0,0 +1,27 @@ +# Generated client versions in a module release + +`codefly publish` includes committed generated clients whose `sources` name a +package released by the module workspace. It finds tracked +`library.codefly.yaml` files throughout the repository, including `clients/`; +ignored caches and dependencies are not release inputs. + +Before changing versions, publication checks that each selected source names the +owner's current package version and contract digest. It verifies copied bytes +and catalog metadata against the committed producer, including protobuf service +subsets through the same narrowing code used by generation. Stale clients must +be regenerated and committed +before releasing. Every selected artifact must be tracked and remain inside the +repository; symlinks are refused. + +The release commit updates the selected source versions and copied catalogs. A +single-source client also receives the release version in its library manifest +and TypeScript/Python package manifests. Python stamping and publication both +read the TOML `project.version` field; tool versions, comments and unrelated +formatting remain unchanged. An aggregate keeps its own library and +language package versions; unrelated source entries stay unchanged. Contract +bytes and generation provenance are preserved. Go clients obtain their version +from the release tag. + +These changes use the existing release transaction. If regeneration or a +release gate fails before commit, all client version changes are restored with +the module manifests and generated contracts. diff --git a/docs/publish-verification.md b/docs/publish-verification.md new file mode 100644 index 00000000..6df7b804 --- /dev/null +++ b/docs/publish-verification.md @@ -0,0 +1,30 @@ +# Publication integrity and installer access + +For executable agent releases, `codefly publish` verifies the archives after +publication whether the manifest assigns publication to `cli` or `workflow`. +Each required loader archive must appear in the release's producer checksum +list. Its SHA-256 must agree with GitHub's upload digest, and the downloaded +bytes must match the recorded size and digest. A missing archive, draft release, +missing checksum evidence, or mismatched bytes makes publication verification +fail. A published tag is not rolled back when verification fails. + +When the CLI owns the uploads, it stages a deterministic `checksums.txt` +covering every archive and SBOM. Before uploading any file, it compares every +existing asset with the staged bytes. A retry can fill missing assets or reuse +identical assets, but cannot replace an existing archive, SBOM or checksum list. +It uploads the checksum list last, after the files it describes. A CLI-staged +SBOM is required and verified by its declared filename. Workflow-owned SBOMs +named `.sbom.json` are verified when present; a workflow need not publish +one. + +After integrity verification, publication checks that each exact loader URL is +reachable using Core's installer credential rule: `GITHUB_TOKEN`, otherwise +`GH_TOKEN`, otherwise anonymous access. Publishing may use a `gh auth login` +credential; installer verification deliberately does not. A public release can +pass without an environment token. A private release needs a credential that +the installer can actually use. + +These checks establish published archive integrity and installer access. They +do not start the agent or establish its runtime capabilities. Core owns download +verification and installed-cache admission; the release's agent CI owns runtime +qualification. diff --git a/go.mod b/go.mod index 1ed43ef7..7b049174 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 - github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 + github.com/codefly-dev/core v0.19.8 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 @@ -33,6 +33,7 @@ require ( github.com/moby/patternmatcher v0.6.1 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 + github.com/pelletier/go-toml/v2 v2.4.3 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 github.com/rs/cors v1.11.1 github.com/shirou/gopsutil/v3 v3.24.5 diff --git a/go.sum b/go.sum index b2dd79eb..e883dbd1 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,8 @@ github.com/ProtonMail/go-crypto v1.4.1 h1:9RfcZHqEQUvP8RzecWEUafnZVtEvrBVL9BiF67 github.com/ProtonMail/go-crypto v1.4.1/go.mod h1:e1OaTyu5SYVrO9gKOEhTc+5UcXtTUa+P3uLudwcgPqo= github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow= github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4= +github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= +github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= github.com/alecthomas/chroma/v2 v2.27.0 h1:FodwmyOBgJULFYmDqibcp9pvfDLWdtPRh9v/r5BXYZs= @@ -115,10 +117,16 @@ github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad722 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855/go.mod h1:rfn41uiORQ36RrOeFEjXHPLC9NUi0j14AoYJtcovsn4= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93 h1:4EygH7/J0E6aM7hUPdbQGfit1rsLUNCtWJ8kMh2b9PE= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93/go.mod h1:1rmjrVyr0h7zOuwOHS9i+BoeS9dr3XNvtupLZFwY6As= -github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a h1:ar8ERpN9n9ZL29LZ63mqBZNLPZ7OpcnQ1+D1g89DbPY= -github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 h1:OLQ/LG4+KGRz3crTc8zgQlxTS/Ta3Yh7OUFu+12N5u4= github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548 h1:5wqbjiooPNttorByfEApw4Sxjp1VSVyDU6ciHFfiMkg= +github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5 h1:08ZV27FM2N4V6NtGrp8T5k+Fc73Ten7BCX6FRkFIba4= +github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8-0.20261027211051-bc4e15cab5b0 h1:r5xxRusbZ54Wpk2T6aqqqNWvHmVVJ8HMJNdofE7mqrc= +github.com/codefly-dev/core v0.19.8-0.20261027211051-bc4e15cab5b0/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8 h1:7ZFXXo51P4H0r22QUbTPzs1j9d6iyftvoeqPGKDFAg4= +github.com/codefly-dev/core v0.19.8/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= @@ -133,9 +141,8 @@ github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M= github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151Xdx3ZPPE= github.com/containerd/errdefs/pkg v0.3.0/go.mod h1:NJw6s9HwNuRhnjJhM7pylWwMyAkmCQvQ4GpJHEqRLVk= -github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= -github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/log v0.2.0 h1:BewD/umNgVnoczglOpX8eRMyEy5t5iPlu5AIpnWDONc= +github.com/containerd/log v0.2.0/go.mod h1:/M7L7CXKcPTfNC74XzaK+5H5KbO5+4lJVpuVI6vRLoM= github.com/containerd/typeurl/v2 v2.3.0 h1:HZHPhRWo5XMy3QGQoPrUzbW/2ckwjfweHmOwlkIrPAQ= github.com/containerd/typeurl/v2 v2.3.0/go.mod h1:Qk+PAdUYArVj41TnGi6rJ+48RF0PkcTc4i/taoBcK0w= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -333,8 +340,6 @@ github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyua github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= -github.com/moby/buildkit v0.32.2 h1:Sfy7+u6dUv/2yuBc9KCoK70Re8atuV8aPZ5UOC068Vc= -github.com/moby/buildkit v0.32.2/go.mod h1:0GB/EJ1d+4VIVqIAgy3asaoGkVXy7IrDfVy7mPhOvg8= github.com/moby/buildkit v0.34.1 h1:jVdxsXX0UpU0q1Et+9lFezPmh9sGE6vF0TrKZaGlfy4= github.com/moby/buildkit v0.34.1/go.mod h1:e+Za70y2a3nL1U2Y0P8TvhPyKp2Ok2NS+Q4siSArDiM= github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= @@ -372,6 +377,8 @@ github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgr github.com/pb33f/ordered-map/v2 v2.3.1 h1:5319HDO0aw4DA4gzi+zv4FXU9UlSs3xGZ40wcP1nBjY= github.com/pb33f/ordered-map/v2 v2.3.1/go.mod h1:qxFQgd0PkVUtOMCkTapqotNgzRhMPL7VvaHKbd1HnmQ= github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= +github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= +github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pjbgf/sha1cd v0.7.0 h1:ZRNPKHj+gfkLBf0KJv/p1Hmz+7bqCT5o311YX0nq+DA= github.com/pjbgf/sha1cd v0.7.0/go.mod h1:pKR5Li+qTCo+ebqITZfwPlJGoCFCvSO00qxjbNtTUFQ= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= @@ -409,8 +416,6 @@ github.com/shoenig/test v1.7.0/go.mod h1:UxJ6u/x2v/TNs/LoLxBNJRV9DiwBBKYxXSyczsB github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k= github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME= github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= -github.com/sirupsen/logrus v1.10.0 h1:T8MxJJXVZkfcC5zSRMRAg2F8+lxjmUCGGWPzFxO+Msc= -github.com/sirupsen/logrus v1.10.0/go.mod h1:FXZFonkDAnFozmO+5hGAFvB0Yg9/j2SIhA/QuIkP180= github.com/sirupsen/logrus v1.10.2 h1:G2SED73/qrAu6YwbdxOD6peLkCBI3z7L+ykJFTXJBBo= github.com/sirupsen/logrus v1.10.2/go.mod h1:SLEg8TqYulVKKfIGHldVp2K2aYz2DKSVBq4g/H5bR7Q= github.com/skeema/knownhosts v1.3.2 h1:EDL9mgf4NzwMXCTfaxSD/o/a5fxDw/xL9nkU28JjdBg= @@ -448,6 +453,8 @@ github.com/tklauser/go-sysconf v0.4.0 h1:7H0uAN+7RkwWRaxhYXDLqa5V3LPrJeV8wmD9dRU github.com/tklauser/go-sysconf v0.4.0/go.mod h1:8mTNWyog7H+MpKijp4VmKJAd2bbYQ2zuUwkYRbUArPI= github.com/tklauser/numcpus v0.12.0 h1:NR85qdvHA9pFse3x3weVZ0r0ST8R6l5RHbZrlRaqob4= github.com/tklauser/numcpus v0.12.0/go.mod h1:ABHeXzJnr/qqwguhClkZKT1/8VABcYrsyUiUGobwWJg= +github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0 h1:2f304B10LaZdB8kkVEaoXvAMVan2tl9AiK4G0odjQtE= +github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0/go.mod h1:278M4p8WsNh3n4a1eqiFcV2FGk7wE5fwUpUom9mK9lE= github.com/tree-sitter-grammars/tree-sitter-kotlin v1.1.0 h1:SWIUDASa+WPhDDem1U5IJpYwQEezkqXrUI61OcnORzM= github.com/tree-sitter-grammars/tree-sitter-kotlin v1.1.0/go.mod h1:eH+flFf3QOa9c9BY9g3Bz02F7zTq30kGIG3cgB0lSlI= github.com/tree-sitter/go-tree-sitter v0.25.0 h1:sx6kcg8raRFCvc9BnXglke6axya12krCJF5xJ2sftRU= @@ -519,9 +526,8 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.47.0 h1:julhjPeUH/q/7hinbSd go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.47.0/go.mod h1:Ao2mz688LH/tFf0yMAenidq6k2YNSx6SIY2q6jDACck= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0 h1:UFpxOpYPmMNUtOWhdb+nC1WELIgVf8z9higURrbzYU4= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0/go.mod h1:YBGjxe3lt0jtXoEXxGrGhv/jM7oUBUDVK7zhAvNyEjc= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0 h1:QBajQ2SrwQijzHyZbQlPsuIzpl/ll8DY6wPWsajeGcI= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0/go.mod h1:08ZQLjrPLQ6R4kAXvuOvODEer5Yh4CoFvll5qB2BCI8= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0 h1:KrC1YrQeSt46ITMWAbgQx1M1eV1/1TKzttrBzymPmss= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0/go.mod h1:zDSEzoEqsOrgBeGvH66KRgxh90VonFyJqBHA0Pk3+rM= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.47.0 h1:N3YQCxjxQ/bMjyc3heladfRm9t9RTksGQH8z4w6yU/0= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.47.0/go.mod h1:Mp8HOFqcaUyypCuGv9IhDdTHnJ56lSudSHMd+pVSCEA= go.opentelemetry.io/otel/log v1.47.0 h1:cOTS1CcLbSQeZKanGJ+0JpF/+t4PELi3O3bbl2lqCcI= @@ -587,8 +593,6 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= -golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= -golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/time v0.16.0 h1:vMb6ptszcQMkcwiRTAuNNU50gom6++Q/6gY2hDM6VDE= golang.org/x/time v0.16.0/go.mod h1:rVKOqvZeKvrDKTQiAHJ7wmwP0RzleSphoEA9RcdLA0s= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index a24947bf..ae6d0e58 100644 --- a/pkg/generators/client_library.go +++ b/pkg/generators/client_library.go @@ -825,10 +825,6 @@ func writeTypeScriptScaffold(langDir, npmPackage string) error { return writeFileIfMissing(filepath.Join(langDir, "tsconfig.json"), []byte(tsconfig)) } -// pyprojectVersionRe matches pyproject.toml's project version line, the shape -// writePythonScaffold writes. -var pyprojectVersionRe = regexp.MustCompile(`(?m)^version = ("[^"]*")$`) - // stampPackageVersion sets the version a language's package manifest declares // to the library's own. The scaffold is written once and kept, so without this // every generated TypeScript and Python client stays at the scaffold's 0.0.1 @@ -904,11 +900,7 @@ func locatePackageVersion(lang languages.Language, path string, data []byte) (st if lang == languages.TYPESCRIPT { return locatePackageJSONVersion(path, data) } - match := pyprojectVersionRe.FindSubmatchIndex(data) - if match == nil { - return 0, 0, fmt.Errorf("cannot find a version in %s", path) - } - return match[2], match[3], nil + return locatePythonPackageVersion(path, data) } // locatePackageJSONVersion finds the value of package.json's top-level diff --git a/pkg/generators/client_release.go b/pkg/generators/client_release.go new file mode 100644 index 00000000..4c4c0e61 --- /dev/null +++ b/pkg/generators/client_release.go @@ -0,0 +1,292 @@ +package generators + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" + "github.com/codefly-dev/core/resources" + "github.com/codefly-dev/core/shared" + "gopkg.in/yaml.v3" +) + +// ClientRelease is the version-only part of an existing generated library. +// Paths lets the release owner restore every write if its release gate fails. +// Contract bytes, generator provenance and unrelated sources stay put. +type ClientRelease struct { + Paths []string + dir string + document yaml.Node + sources []librarySource + catalogs map[int]*composition.APIContractCatalog + catalogPaths map[int]string + langs []generatedLanguageExport +} + +// ClientReleasePackage is the committed producer whose contract a client binds. +type ClientReleasePackage struct { + Directory string + Catalog *composition.APIContractCatalog +} + +// PlanClientRelease selects sources owned by the releasing packages. The owner +// supplies its committed catalogs and checks every path before it is read, using +// the same containment/tracking rules as the rest of its release. No file is +// written during planning; a stale source must be regenerated before release. +func PlanClientRelease(dir string, packages map[string]ClientReleasePackage, checkPath func(string) error) (*ClientRelease, error) { + manifest := filepath.Join(dir, resources.LibraryConfigurationName) + if err := checkPath(manifest); err != nil { + return nil, err + } + sources, err := readExistingLibrarySources(dir) + if err != nil { + return nil, err + } + matched := false + for i := range sources { + _, ok := packages[sources[i].Package] + matched = matched || ok + } + if !matched { + return nil, nil + } + plan := &ClientRelease{dir: dir, sources: sources, catalogs: map[int]*composition.APIContractCatalog{}, catalogPaths: map[int]string{}} + raw, err := os.ReadFile(manifest) //nolint:gosec // checked release path + if err != nil { + return nil, err + } + if err = yaml.Unmarshal(raw, &plan.document); err != nil { + return nil, err + } + plan.Paths = append(plan.Paths, manifest) + for i := range sources { + if producer, releasing := packages[sources[i].Package]; releasing { + if err = plan.addSource(i, producer, checkPath); err != nil { + return nil, err + } + } + } + if len(sources) == 1 { + plan.langs, err = readExistingLanguageExports(dir) + if err != nil { + return nil, err + } + for _, lang := range plan.langs { + if !filepath.IsLocal(lang.Path) { + return nil, fmt.Errorf("client language path %q escapes its library", lang.Path) + } + if path, ok := packageManifest(languages.Language(lang.Name), filepath.Join(dir, lang.Path)); ok { + if err := checkPath(path); err != nil { + return nil, err + } + if err := checkPackageVersionStampable(languages.Language(lang.Name), filepath.Dir(path)); err != nil { + return nil, err + } + plan.Paths = append(plan.Paths, path) + } + } + } + return plan, nil +} + +func (plan *ClientRelease) addSource(i int, producer ClientReleasePackage, checkPath func(string) error) error { + source := &plan.sources[i] + owner := producer.Catalog + manifest := filepath.Join(plan.dir, resources.LibraryConfigurationName) + if owner == nil || owner.Package != source.Package || owner.Version != source.Version || !sourceMatchesCatalog(source, owner) { + return fmt.Errorf("%s source %s/%s is stale; regenerate the client before release", manifest, source.Service, source.Endpoint) + } + contractDir := filepath.Join(plan.dir, "contract") + if len(plan.sources) > 1 { + entry := ContractEntry{ModuleName: source.Module, Endpoint: composition.APIContractEndpoint{Service: source.Service, Endpoint: source.Endpoint}} + name := contractEntryDirName(&entry) + if !filepath.IsLocal(name) || strings.ContainsAny(name, `/\`) { + return fmt.Errorf("invalid generated contract directory %q", name) + } + contractDir = filepath.Join(contractDir, name) + } + path := filepath.Join(contractDir, "catalog.codefly.json") + if err := checkPath(path); err != nil { + return err + } + catalog, err := readClientReleaseCatalog(path) + if err != nil { + return err + } + if catalog.Package != source.Package || catalog.Version != source.Version || len(catalog.Endpoints) != 1 || catalog.Endpoints[0].Service != source.Service || catalog.Endpoints[0].Endpoint != source.Endpoint { + return fmt.Errorf("%s disagrees with its library source; regenerate the client before release", path) + } + endpoint := &catalog.Endpoints[0] + if err := checkPath(filepath.Join(contractDir, filepath.FromSlash(endpoint.Path))); err != nil { + return err + } + if err := verifyReleaseClientContract(contractDir, source, producer, endpoint, checkPath); err != nil { + return err + } + plan.catalogs[i], plan.catalogPaths[i] = catalog, path + plan.Paths = append(plan.Paths, path) + return nil +} + +func sourceMatchesCatalog(source *librarySource, catalog *composition.APIContractCatalog) bool { + for i := range catalog.Endpoints { + endpoint := &catalog.Endpoints[i] + if endpoint.Service == source.Service && endpoint.Endpoint == source.Endpoint { + return endpoint.Digest == source.ContractDigest + } + } + return false +} + +// A copied protobuf contract can be a generator-narrowed subset: its digest +// then differs from the source digest. Reproduce that existing transformation +// from the committed producer rather than trusting a self-consistent copy. +func verifyReleaseClientContract(dir string, source *librarySource, producer ClientReleasePackage, copied *composition.APIContractEndpoint, checkPath func(string) error) error { + if !slices.Equal(source.Services, protobufServiceNames(copied.Services)) { + return fmt.Errorf("copied client service selection disagrees with its source; regenerate the client before release") + } + var endpoint composition.APIContractEndpoint + for i := range producer.Catalog.Endpoints { + candidate := &producer.Catalog.Endpoints[i] + if candidate.Service == source.Service && candidate.Endpoint == source.Endpoint { + endpoint = *candidate + break + } + } + if err := checkPath(filepath.Join(producer.Directory, filepath.FromSlash(endpoint.Path))); err != nil { + return err + } + original, err := ReadCatalogContractBytes(producer.Directory, &endpoint) + if err != nil { + return err + } + actual, err := ReadCatalogContractBytes(dir, copied) + if err != nil { + return err + } + if !bytes.Equal(actual, original) { + if len(source.Services) == 0 || endpoint.Kind != composition.APIContractKindProtobuf { + return fmt.Errorf("copied client contract differs from its producer; regenerate the client before release") + } + entries, narrowErr := narrowEntries([]ContractEntry{{Endpoint: endpoint, ContractBytes: original, Services: source.Services}}) + if narrowErr != nil { + return narrowErr + } + if !bytes.Equal(actual, entries[0].ContractBytes) { + return fmt.Errorf("copied client contract differs from its selected producer services; regenerate the client before release") + } + endpoint = entries[0].Endpoint + } + endpoint.Path = "contract.binpb" + if endpoint.Kind == composition.APIContractKindOpenAPI { + endpoint.Path = "openapi.json" + } + endpoint.Digest = composition.APIContractDigest(actual) + expected := &composition.APIContractCatalog{Schema: composition.APIContractCatalogSchema, Package: source.Package, Version: source.Version, Endpoints: []composition.APIContractEndpoint{endpoint}} + got := *expected + got.Endpoints = []composition.APIContractEndpoint{*copied} + expectedBytes, err := expected.CanonicalBytes() + if err != nil { + return err + } + actualBytes, err := got.CanonicalBytes() + if err != nil { + return err + } + if !bytes.Equal(expectedBytes, actualBytes) { + return fmt.Errorf("copied client catalog differs from its producer; regenerate the client before release") + } + return nil +} + +func readClientReleaseCatalog(path string) (*composition.APIContractCatalog, error) { + data, err := os.ReadFile(path) //nolint:gosec // checked release path + if err != nil { + return nil, err + } + catalog := &composition.APIContractCatalog{} + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(catalog); err != nil { + return nil, err + } + var extra any + if err := decoder.Decode(&extra); !errors.Is(err, io.EOF) { + return nil, fmt.Errorf("%s has trailing JSON", path) + } + return catalog, catalog.Validate() +} + +// Write applies versions supplied by the updated owner package manifests. +// An aggregate keeps its own top-level and language versions, exactly as generate. +func (plan *ClientRelease) Write(ctx context.Context, versions map[string]string) error { + root := plan.document.Content[0] + sources := libraryField(root, "sources") + if sources == nil || len(sources.Content) != len(plan.sources) { + return fmt.Errorf("invalid library sources") + } + for i := range plan.sources { + catalog, releasing := plan.catalogs[i] + if !releasing { + continue + } + version := versions[plan.sources[i].Package] + if version == "" { + return fmt.Errorf("no released version for %s", plan.sources[i].Package) + } + catalog.Version = version + if err := catalog.Validate(); err != nil { + return err + } + data, err := catalog.CanonicalBytes() + if err != nil { + return err + } + if err := shared.WriteFileAtomic(ctx, plan.catalogPaths[i], data, 0o644); err != nil { + return err + } + field := libraryField(sources.Content[i], "version") + if field == nil { + return fmt.Errorf("library source has no version") + } + field.Value = version + if len(plan.sources) == 1 { + field := libraryField(root, "version") + if field == nil { + return fmt.Errorf("library has no version") + } + field.Value = version + for _, lang := range plan.langs { + if err := stampPackageVersion(languages.Language(lang.Name), filepath.Join(plan.dir, lang.Path), version); err != nil { + return err + } + } + } + } + data, err := yaml.Marshal(&plan.document) + if err != nil { + return err + } + return shared.WriteFileAtomic(ctx, filepath.Join(plan.dir, resources.LibraryConfigurationName), data, 0o644) +} + +func libraryField(mapping *yaml.Node, key string) *yaml.Node { + if mapping.Kind != yaml.MappingNode { + return nil + } + for i := 0; i+1 < len(mapping.Content); i += 2 { + if mapping.Content[i].Value == key { + return mapping.Content[i+1] + } + } + return nil +} diff --git a/pkg/generators/client_release_test.go b/pkg/generators/client_release_test.go new file mode 100644 index 00000000..ffdfd07d --- /dev/null +++ b/pkg/generators/client_release_test.go @@ -0,0 +1,209 @@ +package generators + +import ( + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +func releaseClientEntry(module, version string) ContractEntry { + data := []byte(`{"openapi":"3.0.0","paths":{"/ready":{"get":{}}}}`) + return ContractEntry{ + PackageID: "acme/" + module, PackageVersion: version, ModuleName: module, ContractBytes: data, + Endpoint: composition.APIContractEndpoint{ + Service: "gateway", Endpoint: "rest", API: "rest", Kind: composition.APIContractKindOpenAPI, + Package: module, Path: "contracts/api/gateway/rest/openapi.json", Digest: composition.APIContractDigest(data), + Routes: []composition.APIContractRoute{{Method: "GET", Path: "/ready"}}, + }, + } +} + +func releaseClientFixture(t *testing.T, entries []ContractEntry) (string, []generatedLanguageExport) { + t.Helper() + dir := t.TempDir() + langs := []generatedLanguageExport{{Name: "typescript", Path: "typescript/"}, {Name: "python", Path: "python/"}, {Name: "go", Path: "go/"}} + require.NoError(t, writeContractFiles(t.Context(), dir, entries)) + for _, entry := range entries { + path := filepath.Join(dir, "producer", entry.Endpoint.Path) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, entry.ContractBytes, 0o600)) + } + for _, lang := range langs { + require.NoError(t, os.MkdirAll(filepath.Join(dir, lang.Path), 0o755)) + } + require.NoError(t, writeTypeScriptScaffold(filepath.Join(dir, "typescript"), "@acme/client")) + require.NoError(t, writePythonScaffold(filepath.Join(dir, "python"), "acme_client")) + require.NoError(t, writeLibraryManifest(t.Context(), dir, "client", "0.1.0", entries, langs, false)) + return dir, langs +} + +func TestClientReleaseMatchesGeneratorVersionOutputs(t *testing.T) { + entry := releaseClientEntry("payments", "0.1.0") + dir, langs := releaseClientFixture(t, []ContractEntry{entry}) + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.NoError(t, err) + require.Len(t, plan.Paths, 4) + require.NoError(t, plan.Write(t.Context(), map[string]string{entry.PackageID: "0.2.0"})) + after := snapshotTree(t, dir) + + // Re-run the same version-writing paths as GenerateClientLibrary: nothing + // differs from the release output, including its copied contract catalog. + entry.PackageVersion = "0.2.0" + require.NoError(t, writeContractFiles(t.Context(), dir, []ContractEntry{entry})) + require.NoError(t, writeLibraryManifest(t.Context(), dir, "client", libraryVersion(dir, []ContractEntry{entry}), []ContractEntry{entry}, langs, false)) + for _, lang := range langs { + require.NoError(t, stampPackageVersion(languages.Language(lang.Name), filepath.Join(dir, lang.Path), "0.2.0")) + } + require.Equal(t, after, snapshotTree(t, dir)) +} + +func TestClientReleaseAggregateKeepsForeignSourceAndOwnVersion(t *testing.T) { + entries := []ContractEntry{releaseClientEntry("payments", "0.1.0"), releaseClientEntry("ledger", "3.4.0")} + dir, _ := releaseClientFixture(t, entries) + foreignPath := filepath.Join(dir, "contract", "ledger-gateway-rest", "catalog.codefly.json") + foreign, err := os.ReadFile(foreignPath) + require.NoError(t, err) + ts, err := os.ReadFile(filepath.Join(dir, "typescript", "package.json")) + require.NoError(t, err) + manifestPath := filepath.Join(dir, resources.LibraryConfigurationName) + manifest, err := os.ReadFile(manifestPath) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, append(manifest, []byte("custom-owner: keep\n")...), 0o600)) + + owner := &composition.APIContractCatalog{Package: "acme/payments", Version: "0.1.0", Endpoints: []composition.APIContractEndpoint{entries[0].Endpoint}} + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{"acme/payments": {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.NoError(t, err) + require.Len(t, plan.Paths, 2) + require.NoError(t, plan.Write(t.Context(), map[string]string{"acme/payments": "0.2.0"})) + got, err := os.ReadFile(manifestPath) + require.NoError(t, err) + var doc map[string]any + require.NoError(t, yaml.Unmarshal(got, &doc)) + require.Equal(t, "0.1.0", doc["version"]) + require.Equal(t, "keep", doc["custom-owner"]) + var original map[string]any + require.NoError(t, yaml.Unmarshal(manifest, &original)) + require.Equal(t, original["generated-by"], doc["generated-by"]) + sources, err := readExistingLibrarySources(dir) + require.NoError(t, err) + require.Equal(t, "0.2.0", sources[0].Version) + require.Equal(t, "3.4.0", sources[1].Version) + got, err = os.ReadFile(foreignPath) + require.NoError(t, err) + require.Equal(t, foreign, got) + got, err = os.ReadFile(filepath.Join(dir, "typescript", "package.json")) + require.NoError(t, err) + require.Equal(t, ts, got) +} + +func TestClientReleaseRefusesStaleArtifactsBeforeWriting(t *testing.T) { + for _, scenario := range []struct { + name string + mutate func(*testing.T, string, *composition.APIContractCatalog) + want string + }{ + {"producer changed", func(_ *testing.T, _ string, owner *composition.APIContractCatalog) { + owner.Endpoints[0].Digest = composition.APIContractDigest([]byte("changed")) + }, "is stale"}, + {"producer version changed", func(_ *testing.T, _ string, owner *composition.APIContractCatalog) { owner.Version = "0.2.0" }, "is stale"}, + {"copied contract changed", func(t *testing.T, dir string, _ *composition.APIContractCatalog) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "contract", "openapi.json"), []byte("changed"), 0o600)) + }, "does not match catalog digest"}, + {"unknown catalog field", func(t *testing.T, dir string, _ *composition.APIContractCatalog) { + path := filepath.Join(dir, "contract", "catalog.codefly.json") + data, err := os.ReadFile(path) + require.NoError(t, err) + require.NoError(t, os.WriteFile(path, append([]byte(`{"unknown":true,`), data[1:]...), 0o600)) + }, "unknown field"}, + {"language version absent", func(t *testing.T, dir string, _ *composition.APIContractCatalog) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "typescript", "package.json"), []byte(`{"name":"client"}`), 0o600)) + }, "cannot find a version"}, + } { + t.Run(scenario.name, func(t *testing.T) { + entry := releaseClientEntry("payments", "0.1.0") + dir, _ := releaseClientFixture(t, []ContractEntry{entry}) + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + scenario.mutate(t, dir, owner) + before := snapshotTree(t, dir) + _, err := PlanClientRelease(dir, map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.ErrorContains(t, err, scenario.want) + require.Equal(t, before, snapshotTree(t, dir)) + }) + } +} + +func TestClientReleaseDoesNotReadForeignCatalogs(t *testing.T) { + dir, _ := releaseClientFixture(t, []ContractEntry{releaseClientEntry("ledger", "1.0.0")}) + require.NoError(t, os.RemoveAll(filepath.Join(dir, "contract"))) + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{"acme/payments": {}}, func(string) error { return nil }) + require.NoError(t, err) + require.Nil(t, plan) +} + +func TestClientReleaseRefusesSelfConsistentForeignContract(t *testing.T) { + entry := releaseClientEntry("payments", "0.1.0") + dir, _ := releaseClientFixture(t, []ContractEntry{entry}) + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + copiedPath := filepath.Join(dir, "contract", "catalog.codefly.json") + copied, err := readClientReleaseCatalog(copiedPath) + require.NoError(t, err) + changed := []byte(`{"openapi":"3.0.0","paths":{"/different":{"get":{}}}}`) + copied.Endpoints[0].Digest = composition.APIContractDigest(changed) + copied.Endpoints[0].Routes[0].Path = "/different" + data, err := copied.CanonicalBytes() + require.NoError(t, err) + require.NoError(t, os.WriteFile(copiedPath, data, 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "contract", "openapi.json"), changed, 0o600)) + before := snapshotTree(t, dir) + _, err = PlanClientRelease(dir, map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.ErrorContains(t, err, "differs from its producer") + require.Equal(t, before, snapshotTree(t, dir)) +} + +func TestClientReleaseVerifiesNarrowedProducerContract(t *testing.T) { + entry := narrowEntry(narrowFixture(t), "acme.v1.Wanted") + entry.PackageID, entry.PackageVersion = "acme/payments", "0.1.0" + entry.Endpoint.API, entry.Endpoint.Path = "connect", "contracts/api/api/connect/contract.binpb" + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + narrowed, err := narrowEntries([]ContractEntry{entry}) + require.NoError(t, err) + dir, _ := releaseClientFixture(t, narrowed) + require.NoError(t, os.WriteFile(filepath.Join(dir, "producer", entry.Endpoint.Path), entry.ContractBytes, 0o600)) + packages := map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}} + plan, err := PlanClientRelease(dir, packages, func(string) error { return nil }) + require.NoError(t, err) + require.NotNil(t, plan) + // A different valid subset cannot be certified under the saved selection. + other := entry + other.Services = []string{"acme.v1.Other"} + otherEntries, err := narrowEntries([]ContractEntry{other}) + require.NoError(t, err) + require.NoError(t, writeContractFiles(t.Context(), dir, otherEntries)) + _, err = PlanClientRelease(dir, packages, func(string) error { return nil }) + require.ErrorContains(t, err, "service selection disagrees") +} + +func TestClientReleaseSamePackageAggregateKeepsAggregateVersion(t *testing.T) { + entries := []ContractEntry{releaseClientEntry("payments", "0.1.0"), releaseClientEntry("payments", "0.1.0")} + entries[1].Endpoint.Service = "worker" + entries[1].Endpoint.Path = "contracts/api/worker/rest/openapi.json" + dir, _ := releaseClientFixture(t, entries) + owner := &composition.APIContractCatalog{Package: "acme/payments", Version: "0.1.0", Endpoints: []composition.APIContractEndpoint{entries[0].Endpoint, entries[1].Endpoint}} + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{"acme/payments": {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.NoError(t, err) + require.Len(t, plan.Paths, 3) + require.NoError(t, plan.Write(t.Context(), map[string]string{"acme/payments": "0.2.0"})) + require.Equal(t, "0.1.0", existingTopVersion(dir)) + sources, err := readExistingLibrarySources(dir) + require.NoError(t, err) + for _, source := range sources { + require.Equal(t, "0.2.0", source.Version) + } +} diff --git a/pkg/generators/client_version_test.go b/pkg/generators/client_version_test.go index 9713d98c..0a651b91 100644 --- a/pkg/generators/client_version_test.go +++ b/pkg/generators/client_version_test.go @@ -182,3 +182,63 @@ func TestCheckPackageVersionStampable(t *testing.T) { t.Fatalf("go: %v", err) } } + +// Source offsets belong to project.version, independent of where another +// table's version or a quoted/multiline TOML value appears in the file. +func TestPythonVersionStampOnlyChangesProjectVersion(t *testing.T) { + for _, test := range []struct { + name, before, oldValue string + }{ + {"tool first", "[tool.example]\nversion = \"7.0.0\"\n\n[project]\nversion = \"0.1.0\"\n", `"0.1.0"`}, + {"quoted table and literal key", "[ 'project' ] # metadata\n\t'version'\t=\t'0.1.0' # release\n[tool.example]\nversion='7.0.0'\n", `'0.1.0'`}, + {"escaped key and table", "[\"pro\\u006Aect\"]\r\n\"ver\\u0073ion\" = \"0.1.0\" # release\r\n", `"0.1.0"`}, + {"multiline decoy", "[tool.example]\ndescription = '''\n[project]\nversion = \"7.0.0\"\n'''\n[project]\nversion = \"0.1.0\"\n", `"0.1.0"`}, + {"nested project table", "[project.urls]\nversion = \"7.0.0\"\n[project]\nversion = \"0.1.0\"\n", `"0.1.0"`}, + {"dotted root key", "project.version = '0.1.0' # release\n[tool.example]\nversion='7.0.0'\n", `'0.1.0'`}, + {"inline root table", "project = { name='client', version = '0.1.0' } # release\n", `'0.1.0'`}, + } { + t.Run(test.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "pyproject.toml") + if err := os.WriteFile(path, []byte(test.before), 0o600); err != nil { + t.Fatal(err) + } + if err := stampPackageVersion(languages.PYTHON, dir, "0.2.0"); err != nil { + t.Fatal(err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + want := strings.Replace(test.before, test.oldValue, `"0.2.0"`, 1) + if string(after) != want { + t.Fatalf("got:\n%s\nwant:\n%s", after, want) + } + }) + } +} + +func TestPythonVersionStampRefusesMissingOrInvalidProjectVersion(t *testing.T) { + for _, before := range []string{ + "[tool.example]\nversion = \"7.0.0\"\n[project]\ndynamic=['version']\n", + "[project]\nversion = 17\n", + "[project]\nversion = \"0.1.0\"\nversion = \"0.2.0\"\n", + "[project]\nversion = \"0.1.0\"\n[broken\n", + } { + dir := t.TempDir() + path := filepath.Join(dir, "pyproject.toml") + if err := os.WriteFile(path, []byte(before), 0o600); err != nil { + t.Fatal(err) + } + if err := stampPackageVersion(languages.PYTHON, dir, "0.2.0"); err == nil { + t.Fatalf("accepted invalid/missing project version:\n%s", before) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != before { + t.Fatalf("refused stamp changed file:\n%s", after) + } + } +} diff --git a/pkg/generators/python_version.go b/pkg/generators/python_version.go new file mode 100644 index 00000000..fdc05f83 --- /dev/null +++ b/pkg/generators/python_version.go @@ -0,0 +1,80 @@ +package generators + +import ( + "fmt" + "slices" + + "github.com/pelletier/go-toml/v2" + "github.com/pelletier/go-toml/v2/unstable" +) + +// locatePythonPackageVersion uses TOML's semantic keys and source ranges to +// replace only project.version. Decoding first rejects invalid/duplicate tables; +// the AST retains comments, quoting and unrelated bytes instead of reformatting +// a manifest a developer owns. Strings containing apparent TOML stay strings. +func locatePythonPackageVersion(path string, data []byte) (int, int, error) { + if _, err := PythonProjectVersion(path, data); err != nil { + return 0, 0, err + } + var parser unstable.Parser + parser.Reset(data) + var table []string + for parser.NextExpression() { + node := parser.Expression() + switch node.Kind { + case unstable.Table, unstable.ArrayTable: + table = tomlKeyPath(nil, node) + case unstable.KeyValue: + if span, ok := pythonVersionRange(node, table); ok { + return int(span.Offset), int(span.Offset + span.Length), nil + } + } + } + if err := parser.Error(); err != nil { + return 0, 0, fmt.Errorf("cannot parse %s: %w", path, err) + } + return 0, 0, fmt.Errorf("cannot find a version in [project] in %s", path) +} + +// PythonProjectVersion reads the static PEP 621 metadata version used by both +// generated-client stamping and publication. A tool's version is unrelated. +func PythonProjectVersion(path string, data []byte) (string, error) { + var document map[string]any + if err := toml.Unmarshal(data, &document); err != nil { + return "", fmt.Errorf("cannot parse %s: %w", path, err) + } + project, _ := document["project"].(map[string]any) + version, ok := project["version"].(string) + if !ok { + return "", fmt.Errorf("cannot find a version string in [project] in %s", path) + } + return version, nil +} + +func tomlKeyPath(prefix []string, node *unstable.Node) []string { + path := slices.Clone(prefix) + keys := node.Key() + for keys.Next() { + path = append(path, string(keys.Node().Data)) + } + return path +} + +func pythonVersionRange(node *unstable.Node, table []string) (unstable.Range, bool) { + path := tomlKeyPath(table, node) + value := node.Value() + if slices.Equal(path, []string{"project", "version"}) && value.Kind == unstable.String { + return value.Raw, true + } + // TOML also allows project = { version = "..." }. Only descend into + // that one table, never unrelated tables or arrays containing versions. + if slices.Equal(path, []string{"project"}) && value.Kind == unstable.InlineTable { + children := value.Children() + for children.Next() { + if span, ok := pythonVersionRange(children.Node(), path); ok { + return span, true + } + } + } + return unstable.Range{}, false +} diff --git a/pkg/processgroup/boot_identity_test.go b/pkg/processgroup/boot_identity_test.go new file mode 100644 index 00000000..9b2c901a --- /dev/null +++ b/pkg/processgroup/boot_identity_test.go @@ -0,0 +1,90 @@ +//go:build darwin + +package processgroup + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +// Isolate the writer's pre-v0.19.7 BootID convention; leave the PID, start +// identity, executable, owner and authentication intact. +func TestOldDarwinBootIdentityRetainsLiveRecord(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + dir := t.TempDir() + port := availablePort(t) + pidPath, readyPath := filepath.Join(dir, "pid"), filepath.Join(dir, "ready") + owner := exec.Command(os.Args[0], "-test.run=^TestProcessGroupHelper$") + owner.Env = append(os.Environ(), helperRoleEnv+"=authenticated-owner", helperPortEnv+"="+strconv.Itoa(port), helperPIDEnv+"="+pidPath, helperReadyEnv+"="+readyPath) + owner.Stdout, owner.Stderr = io.Discard, io.Discard + if err := owner.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = owner.Process.Kill(); _ = owner.Wait() }) + waitForFile(t, pidPath) + waitForFile(t, readyPath) + pid := readPID(t, pidPath) + t.Cleanup(func() { _ = syscall.Kill(-pid, syscall.SIGKILL) }) + path := filepath.Join(home, ".codefly", stateDirName, "authenticated-v2", fmt.Sprintf("%d.pgid", pid)) + original, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if err := ReapStaleProcessGroups(context.Background()); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("control removed current-format live record: %v", err) + } + assertPortHeld(t, port) + var record authenticatedRecord + if err := json.Unmarshal(original, &record); err != nil { + t.Fatal(err) + } + boot, err := unix.SysctlTimeval("kern.boottime") + if err != nil { + t.Fatal(err) + } + old := fmt.Sprintf("%d:%d", boot.Sec, boot.Usec) + record.Leader.BootID, record.Owner.BootID = old, old + data, err := json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + for _, stop := range []bool{false, true} { + var evidence CleanupEvidence + var cleanupErr error + if stop { + evidence, cleanupErr = StopManagedProcessGroups(context.Background(), AllWorkspaces()) + } else { + evidence, cleanupErr = ReapStaleProcessGroupsWithEvidence(context.Background(), AllWorkspaces()) + } + if cleanupErr == nil || !strings.Contains(cleanupErr.Error(), "incompatible boot identity") { + t.Fatalf("stop=%v error=%v", stop, cleanupErr) + } + if evidence.Outcome != CleanupFailed { + t.Fatalf("stop=%v outcome=%v", stop, evidence.Outcome) + } + retained, err := os.ReadFile(path) + if err != nil || !bytes.Equal(retained, data) { + t.Fatalf("stop=%v changed incompatible record: %v", stop, err) + } + assertPortHeld(t, port) + } +} diff --git a/pkg/processgroup/composed_test.go b/pkg/processgroup/composed_test.go index dd517ff4..8e325ae8 100644 --- a/pkg/processgroup/composed_test.go +++ b/pkg/processgroup/composed_test.go @@ -43,6 +43,9 @@ func newDirectory(t *testing.T, name string) string { // looks like and must still be attributable from its working directory. func startRunFrom(t *testing.T, workspace, dir string) int { t.Helper() + // Core transports the current parent's marker even when cmd.Env was + // materialized elsewhere. Model the launching run, not cached child input. + t.Setenv(LaunchWorkspaceEnv, workspace) port := availablePort(t) readyPath := filepath.Join(dir, "ready-"+strconv.Itoa(port)) command := exec.Command(os.Args[0], "-test.run=^TestProcessGroupHelper$") @@ -50,9 +53,6 @@ func startRunFrom(t *testing.T, workspace, dir string) int { helperRoleEnv+"=listener", helperPortEnv+"="+strconv.Itoa(port), helperReadyEnv+"="+readyPath) - if workspace != "" { - command.Env = append(command.Env, LaunchWorkspaceEnv+"="+workspace) - } command.Dir = dir command.Stdout = io.Discard command.Stderr = io.Discard diff --git a/pkg/processgroup/launch.go b/pkg/processgroup/launch.go index 57ceb87a..74ce344d 100644 --- a/pkg/processgroup/launch.go +++ b/pkg/processgroup/launch.go @@ -4,14 +4,16 @@ import ( "bytes" "os" "path/filepath" + + runners "github.com/codefly-dev/core/runners/base" ) // LaunchWorkspaceEnv carries the workspace whose run started a process. The CLI -// records it in its OWN environment before it starts anything, which is what -// makes every descendant carry it: the agents it spawns, the runners inside -// them, the service binaries those exec, and the stateful stores they bring up. -// Nothing downstream has to cooperate, which matters because agents are -// released independently of this repository and cannot be asked to. +// records it in its own environment before starting agents. Core's tracked and +// owned process launchers carry the current marker into service processes even +// when a runner materializes a separate environment, as Nix does. Independently +// released agents must adopt that Core transport; ordinary environment +// inheritance alone does not preserve attribution through those runners. // // It exists because a working directory cannot answer "whose run is this?". A // composed module is checked out OUTSIDE the workspace that composes it — a @@ -21,7 +23,7 @@ import ( // by working directory therefore misses precisely the processes a composed run // adds. The stores are the same gap from the other end: a postgres cluster runs // from a data directory under ~/.codefly/data, which is in no workspace at all. -const LaunchWorkspaceEnv = "CODEFLY_RUN_WORKSPACE" +const LaunchWorkspaceEnv = runners.LaunchWorkspaceEnv // MarkLaunchWorkspace records dir as the workspace this process's run belongs // to, so everything this process goes on to start is attributable to it. diff --git a/pkg/processgroup/orphans.go b/pkg/processgroup/orphans.go index c0367e66..8ca4165a 100644 --- a/pkg/processgroup/orphans.go +++ b/pkg/processgroup/orphans.go @@ -11,6 +11,7 @@ import ( "time" "github.com/codefly-dev/core/resources" + runners "github.com/codefly-dev/core/runners/base" "github.com/shirou/gopsutil/v3/process" ) @@ -49,9 +50,8 @@ type DevServerOrphan struct { // the registry-based reaper can no longer identify. Orphaned bool // Owned is true when the process carries codefly's process-group - // authentication in its environment, proving codefly spawned it. Only owned - // groups are ever signalled; a dev server the user launched by hand inside a - // workspace is surfaced by a scan but never reaped. + // marker in its environment. This is a discovery hint; the reaper separately + // requires Core to authenticate its registered identity before any signal. Owned bool } @@ -148,13 +148,10 @@ func snapshotProcess(ctx context.Context, pid int, proc *process.Process) (proce return snap, true } -// ReapDevServerOrphans terminates the process groups of leaked dev servers. A -// group is reaped only when it is (1) codefly-owned — a member carries codefly's -// process-group authentication, so codefly spawned it — and (2) stale — its -// group leader is gone or has itself been reparented to init. Dev servers the -// user launched by hand (no authentication) and servers still supervised by a -// live codefly (leader with a live parent) are left untouched. With dryRun set, -// nothing is signalled and the return value is what would be reaped. +// ReapDevServerOrphans inspects signature-matched candidates in scope. Core +// must prove registered ownership and owner death before an identity-bound +// signal. A missing registration is a refusal, even with a marker present. +// Dry runs perform the same admission checks without signaling. func ReapDevServerOrphans(ctx context.Context, dryRun bool, scope Scope) ([]DevServerOrphan, error) { orphans, err := ScanDevServerOrphans(ctx) if err != nil { @@ -172,19 +169,20 @@ func ReapDevServerOrphans(ctx context.Context, dryRun bool, scope Scope) ([]DevS } stale, err := groupIsStale(orphan.PGID) if err != nil { - failures = append(failures, fmt.Errorf("inspect dev-server process group %d (%s): %w", orphan.PGID, orphan.Cwd, err)) + failures = append(failures, err) continue } if !stale { continue } reapedGroups[orphan.PGID] = struct{}{} - reaped = append(reaped, orphan) - if dryRun { + selected, err := reapOrphanGroup(ctx, orphan.PGID, scope, dryRun) + if err != nil { + failures = append(failures, fmt.Errorf("reap dev-server process group %d (%s): %w", orphan.PGID, orphan.Cwd, err)) continue } - if err := killAuthenticatedProcessGroup(ctx, orphan.PGID); err != nil { - failures = append(failures, fmt.Errorf("reap dev-server process group %d (%s): %w", orphan.PGID, orphan.Cwd, err)) + if selected { + reaped = append(reaped, orphan) } } return reaped, errors.Join(failures...) @@ -202,7 +200,7 @@ func ReapDevServerOrphans(ctx context.Context, dryRun bool, scope Scope) ([]DevS // // Unlike a dev server, a native service is not required to sit inside a workspace // directory — a postgres data dir lives under ~/.codefly/data, outside the repo — -// so ownership rests entirely on the process-group authentication. +// so recovery requires authenticated registered ownership and workspace scope. type NativeServiceOrphan struct { PID int PGID int @@ -226,8 +224,7 @@ type NativeServiceOrphan struct { // ScanNativeServiceOrphans finds native-mode service processes (a compiled user // binary under a codefly build cache, or a postgres cluster) running anywhere on // the machine, without relying on the process-group registry. Ownership is decided -// by the process-group authentication alone, so a user's own or system postgres is -// surfaced with Owned=false and never reaped. +// by its marker for discovery only; signals require Core registration proof. func ScanNativeServiceOrphans(ctx context.Context) ([]NativeServiceOrphan, error) { pids, err := process.PidsWithContext(ctx) if err != nil { @@ -279,11 +276,9 @@ func ScanNativeServiceOrphans(ctx context.Context) ([]NativeServiceOrphan, error return orphans, nil } -// ReapNativeServiceOrphans terminates the process groups of leaked native-mode -// services. As with dev servers, a group is reaped only when it is (1) codefly-owned -// — a member carries the process-group authentication — and (2) stale — its leader -// is gone or reparented to init. With dryRun set, nothing is signalled and the -// return value is what would be reaped. +// ReapNativeServiceOrphans applies the same registered-identity and scope +// checks as dev-server recovery to native signature candidates. Missing or +// ambiguous registrations return an error and leave the process running. func ReapNativeServiceOrphans(ctx context.Context, dryRun bool, scope Scope) ([]NativeServiceOrphan, error) { orphans, err := ScanNativeServiceOrphans(ctx) if err != nil { @@ -301,19 +296,20 @@ func ReapNativeServiceOrphans(ctx context.Context, dryRun bool, scope Scope) ([] } stale, err := groupIsStale(orphan.PGID) if err != nil { - failures = append(failures, fmt.Errorf("inspect native-service process group %d (%s): %w", orphan.PGID, orphan.Command, err)) + failures = append(failures, err) continue } if !stale { continue } reapedGroups[orphan.PGID] = struct{}{} - reaped = append(reaped, orphan) - if dryRun { + selected, err := reapOrphanGroup(ctx, orphan.PGID, scope, dryRun) + if err != nil { + failures = append(failures, fmt.Errorf("reap native-service process group %d (%s): %w", orphan.PGID, orphan.Command, err)) continue } - if err := killAuthenticatedProcessGroup(ctx, orphan.PGID); err != nil { - failures = append(failures, fmt.Errorf("reap native-service process group %d (%s): %w", orphan.PGID, orphan.Command, err)) + if selected { + reaped = append(reaped, orphan) } } return reaped, errors.Join(failures...) @@ -384,9 +380,8 @@ func isRegularFile(path string) bool { } // processIsCodeflyOwned reports whether pid carries codefly's process-group -// authentication in its environment — proof that codefly spawned it (and, via -// inheritance, its whole tree). This survives the loss of the registry record, -// so it identifies leaked codefly servers that `clear` may safely reap. +// marker in its environment. This identifies candidates for inspection, not +// authority to signal: recovery requires Core to authenticate a registration. func processIsCodeflyOwned(pid int) (bool, error) { authentication, err := readProcessGroupAuthentication(pid) if errors.Is(err, errProcessNotFound) { @@ -416,77 +411,31 @@ func groupIsStale(pgid int) (bool, error) { return true, nil } -// groupHasCodeflyMember reports whether the live process group still contains a -// codefly-owned member. It is re-checked immediately before each destructive -// signal so that a process group whose number was recycled between the scan and -// the kill is never signalled unless it is, right now, codefly's. -func groupHasCodeflyMember(ctx context.Context, pgid int) (bool, error) { - members, err := inspectProcessGroup(ctx, pgid) +// reapOrphanGroup treats a signature and environment marker as discovery only. +// Core must prove the registered birth, owner death and scope before any signal. +func reapOrphanGroup(ctx context.Context, pgid int, scope Scope, dryRun bool) (bool, error) { + group, err := runners.LookupProcessGroup(pgid) if err != nil { - return false, err - } - for _, member := range members { - owned, err := processIsCodeflyOwned(member.pid) - if err != nil { - return false, err - } - if owned { - return true, nil - } - } - return false, nil -} - -// killAuthenticatedProcessGroup escalates SIGTERM then SIGKILL to the group, -// but re-confirms the group still holds a codefly-owned member immediately -// before each signal. That closes the window between scan and kill in which the -// process-group number could have been recycled by an unrelated group: if the -// group is no longer codefly's (or already gone), it aborts without signalling. -func killAuthenticatedProcessGroup(ctx context.Context, pgid int) error { - if pgid <= 1 { - return fmt.Errorf("refusing to signal process group %d", pgid) - } - if pgid == syscall.Getpgrp() { - return errors.New("refusing to signal own process group") + return false, fmt.Errorf("cannot authenticate orphan; process left running: %w", err) } - owned, err := groupHasCodeflyMember(ctx, pgid) + ownership, err := group.InspectOwnership(ctx) if err != nil { - return err - } - if !owned { - return nil - } - if err = signalGroup(pgid, syscall.SIGTERM); err != nil { - return err - } - if waitForGroupDeath(ctx, pgid, sigtermGrace) { - return nil - } - if err = ctx.Err(); err != nil { - return err - } - owned, err = groupHasCodeflyMember(ctx, pgid) - if err != nil { - return err + return false, err } - if !owned { - return nil + if !ownership.Authenticated { + return false, errors.New("orphan ownership is unproven; process left running") } - if err = signalGroup(pgid, syscall.SIGKILL); err != nil { - return err + if ownership.OwnerAlive || !registeredGroupInScope(group, ownership, scope) { + return false, nil } - if waitForGroupDeath(ctx, pgid, sigkillGrace) { - return nil + if dryRun { + return true, nil } - if err = ctx.Err(); err != nil { - return err + if err := group.Terminate(ctx); err != nil { + return false, err } - return fmt.Errorf("process group %d remained alive after SIGKILL", pgid) -} - -func signalGroup(pgid int, signal syscall.Signal) error { - if err := syscall.Kill(-pgid, signal); err != nil && !errors.Is(err, syscall.ESRCH) { - return fmt.Errorf("signal process group %d: %w", pgid, err) + if err := group.RemoveIfDeadContext(ctx); err != nil { + return false, err } - return nil + return true, nil } diff --git a/pkg/processgroup/orphans_test.go b/pkg/processgroup/orphans_test.go index 0f5c1a81..4203466f 100644 --- a/pkg/processgroup/orphans_test.go +++ b/pkg/processgroup/orphans_test.go @@ -254,6 +254,7 @@ func TestScanDevServerOrphansMarksUnauthenticatedExternal(t *testing.T) { } func TestReapDevServerOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) external := startDevServerHelper(t, workspace, "", 0) // no auth => not codefly's supervised := startDevServerHelper(t, workspace, testGroupAuth, 0) // owned, but leader has a live parent @@ -262,7 +263,7 @@ func TestReapDevServerOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapDevServerOrphans(ctx, true, AllWorkspaces()) + reaped, err := ReapDevServerOrphans(ctx, true, InWorkspace(workspace)) if err != nil { t.Fatal(err) } @@ -276,7 +277,8 @@ func TestReapDevServerOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { } } -func TestReapDevServerOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { +func TestReapDevServerOrphansDryRunRefusesUnregisteredStaleGroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) leader := startDevServerHelper(t, workspace, testGroupAuth, 0) leaderPID := leader.Process.Pid @@ -298,9 +300,9 @@ func TestReapDevServerOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapDevServerOrphans(ctx, true, AllWorkspaces()) - if err != nil { - t.Fatal(err) + reaped, err := ReapDevServerOrphans(ctx, true, InWorkspace(workspace)) + if err == nil { + t.Fatal("unregistered group was accepted") } selected := false for _, orphan := range reaped { @@ -308,12 +310,13 @@ func TestReapDevServerOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { selected = true } } - if !selected { - t.Fatalf("owned stale group %d was not selected for reaping", leaderPID) + if selected { + t.Fatalf("owned stale group %d was incorrectly selected for reaping", leaderPID) } } -func TestKillAuthenticatedProcessGroupReapsOwnedGroup(t *testing.T) { +func TestOrphanReaperRefusesMarkerWithoutRegistration(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) command := startDevServerHelper(t, workspace, testGroupAuth, 0) pid := command.Process.Pid @@ -322,15 +325,16 @@ func TestKillAuthenticatedProcessGroupReapsOwnedGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() - if err := killAuthenticatedProcessGroup(ctx, pid); err != nil { - t.Fatalf("killAuthenticatedProcessGroup(%d) = %v", pid, err) + if selected, err := reapOrphanGroup(ctx, pid, AllWorkspaces(), false); err == nil || selected { + t.Fatalf("unregistered marker accepted: selected=%v error=%v", selected, err) } - if isAlive(pid) { - t.Fatalf("owned dev-server group %d survived reaping", pid) + if !isAlive(pid) { + t.Fatalf("unregistered process %d was killed", pid) } } func TestKillAuthenticatedProcessGroupSparesUnownedGroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) command := startDevServerHelper(t, workspace, "", 0) // no auth => not codefly's pid := command.Process.Pid @@ -338,8 +342,8 @@ func TestKillAuthenticatedProcessGroupSparesUnownedGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := killAuthenticatedProcessGroup(ctx, pid); err != nil { - t.Fatalf("killAuthenticatedProcessGroup(%d) = %v, want nil (unowned group left untouched)", pid, err) + if selected, err := reapOrphanGroup(ctx, pid, AllWorkspaces(), false); err == nil || selected { + t.Fatalf("unregistered process accepted: selected=%v error=%v", selected, err) } if !isAlive(pid) { t.Fatalf("unowned dev server %d was killed despite lacking codefly authentication", pid) @@ -426,6 +430,9 @@ func TestScanNativeServiceOrphansMarksUnauthenticatedExternal(t *testing.T) { } func TestReapNativeServiceOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + workspace := tempWorkspace(t) + t.Setenv(LaunchWorkspaceEnv, workspace) external := startNativeServiceHelper(t, "postgres", "", 0) // no auth => not codefly's supervised := startNativeServiceHelper(t, nativeCacheBinary(t), testGroupAuth, 0) // owned, but leader has a live parent waitForScannedNativeOrphan(t, external.Process.Pid) @@ -433,7 +440,7 @@ func TestReapNativeServiceOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapNativeServiceOrphans(ctx, true, AllWorkspaces()) + reaped, err := ReapNativeServiceOrphans(ctx, true, InWorkspace(workspace)) if err != nil { t.Fatal(err) } @@ -447,7 +454,10 @@ func TestReapNativeServiceOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { } } -func TestReapNativeServiceOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { +func TestReapNativeServiceOrphansDryRunRefusesUnregisteredStaleGroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + workspace := tempWorkspace(t) + t.Setenv(LaunchWorkspaceEnv, workspace) leader := startNativeServiceHelper(t, nativeCacheBinary(t), testGroupAuth, 0) leaderPID := leader.Process.Pid waitForGroupLeader(t, leaderPID) @@ -468,9 +478,9 @@ func TestReapNativeServiceOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapNativeServiceOrphans(ctx, true, AllWorkspaces()) - if err != nil { - t.Fatal(err) + reaped, err := ReapNativeServiceOrphans(ctx, true, InWorkspace(workspace)) + if err == nil { + t.Fatal("unregistered group was accepted") } selected := false for _, orphan := range reaped { @@ -478,7 +488,7 @@ func TestReapNativeServiceOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { selected = true } } - if !selected { - t.Fatalf("owned stale native-service group %d was not selected for reaping", leaderPID) + if selected { + t.Fatalf("owned stale native-service group %d was incorrectly selected for reaping", leaderPID) } } diff --git a/pkg/processgroup/reaper.go b/pkg/processgroup/reaper.go index 39f3e69f..79c9d07f 100644 --- a/pkg/processgroup/reaper.go +++ b/pkg/processgroup/reaper.go @@ -10,11 +10,13 @@ import ( "io" "os" "path/filepath" + "slices" "strconv" "strings" "syscall" "time" + runners "github.com/codefly-dev/core/runners/base" "github.com/codefly-dev/core/wool" "github.com/gofrs/flock" "github.com/shirou/gopsutil/v3/process" @@ -22,7 +24,6 @@ import ( const ( stateDirName = "runs" - authenticatedDirName = "authenticated-v1" registryLockName = ".reaper.lock" maxSweepPasses = 4 maxRecordSize = 16 << 10 @@ -127,6 +128,8 @@ const ( ) // GroupCleanup identifies the outcome retained for one registry record. +// Record is a logical registered-group: key for Core-owned registrations +// and a filesystem path for historical root records. type GroupCleanup struct { PGID int Record string @@ -158,7 +161,11 @@ func (e CleanupEvidence) RecoveredPGIDs() []int { } func (e *CleanupEvidence) merge(other CleanupEvidence) { - e.OutOfScope = append(e.OutOfScope, other.OutOfScope...) + for _, pgid := range other.OutOfScope { + if !slices.Contains(e.OutOfScope, pgid) { + e.OutOfScope = append(e.OutOfScope, pgid) + } + } for _, group := range other.Groups { e.record(group.Record, group.PGID, group.Outcome) } @@ -245,13 +252,22 @@ func reconcileProcessGroupRegistries(ctx context.Context, policy reapPolicy) (Cl evidence.Outcome = CleanupFailed return evidence, err } - currentEvidence, currentErr := reapProcessGroups(ctx, filepath.Join(dir, authenticatedDirName), policy) - evidence.merge(currentEvidence) - legacyEvidence, legacyErr := reapProcessGroups(ctx, dir, policy) - evidence.merge(legacyEvidence) - if err := errors.Join(currentErr, legacyErr); err != nil { - evidence.Outcome = CleanupFailed - return evidence, err + // Reaping a parent may orphan a group already inspected earlier in the + // pass, including a child written by a different producer generation. + for range maxSweepPasses { + currentEvidence, currentErr := reapRegisteredProcessGroups(ctx, policy) + evidence.merge(currentEvidence) + if currentErr != nil { + return evidence, currentErr + } + legacyEvidence, legacyErr := reapProcessGroups(ctx, dir, policy) + evidence.merge(legacyEvidence) + if legacyErr != nil { + return evidence, legacyErr + } + if len(currentEvidence.RecoveredPGIDs()) == 0 && len(legacyEvidence.RecoveredPGIDs()) == 0 { + break + } } return evidence, nil } @@ -385,20 +401,7 @@ func reconcile(ctx context.Context, path string, policy reapPolicy) (string, err if errors.Is(err, os.ErrNotExist) { return dispositionRemoved, nil } - if filepath.Base(filepath.Dir(path)) == authenticatedDirName && snapshot.info != nil { - quarantinePath, quarantineErr := quarantineInvalidRecord(path, snapshot) - if quarantineErr != nil { - return dispositionFailed, errors.Join( - fmt.Errorf("read process-group record %s: %w", path, err), - fmt.Errorf("quarantine invalid process-group record %s: %w", path, quarantineErr), - ) - } - w.Warn("quarantined invalid process-group record without signaling", - wool.Field("record", path), - wool.Field("quarantine", quarantinePath), - wool.ErrField(err)) - return dispositionRemoved, nil - } + w.Warn("could not reconcile process-group record", wool.Field("record", path), wool.Field("disposition", "retained-invalid-record"), @@ -410,6 +413,13 @@ func reconcile(ctx context.Context, path string, policy reapPolicy) (string, err wool.Field("pgid", rec.pgid), wool.Field("parent", rec.parent), } + // A historical root record must not bypass a supported registration's + // authentication or persisted workspace selection. + if _, lookupErr := runners.LookupProcessGroup(rec.pgid); lookupErr == nil { + return dispositionFailed, fmt.Errorf("group %d has both a legacy root record and a supported registration; records retained", rec.pgid) + } else if !errors.Is(lookupErr, runners.ErrProcessGroupNotRegistered) { + return dispositionFailed, lookupErr + } if !groupAlive(rec.pgid) { if removeErr := removeRecord(path, snapshot); removeErr != nil { @@ -419,9 +429,9 @@ func reconcile(ctx context.Context, path string, policy reapPolicy) (string, err wool.Field("disposition", "removed-dead-group"))...) return dispositionRemoved, nil } - // A record carries no workspace of its own — independently released agents - // write these and this package cannot change their format — so the group is - // attributed the same way an orphan is: from its leader's working directory. + // Historical root records carry no workspace metadata. Supported owner + // registrations were handled above through Core; this compatibility path + // retains environment/cwd attribution for historical producers. // Out of scope means left alone entirely: not signalled, and its record kept, // because the workspace that owns it is still using it. if !policy.scope.includesProcessGroup(rec.pgid) { @@ -724,24 +734,6 @@ func removeRecord(path string, snapshot recordSnapshot) error { return nil } -func quarantineInvalidRecord(path string, snapshot recordSnapshot) (string, error) { - if err := recordIsUnchanged(path, snapshot); err != nil { - return "", err - } - quarantinePath := path + ".invalid" - if err := os.Rename(path, quarantinePath); err != nil { - return "", err - } - directory, err := os.Open(filepath.Dir(path)) - if err != nil { - return quarantinePath, err - } - if err := errors.Join(directory.Sync(), directory.Close()); err != nil { - return quarantinePath, err - } - return quarantinePath, nil -} - func recordedCommand(summary string) string { separator := strings.LastIndex(summary, " <") if separator <= 0 || !strings.HasSuffix(summary, " args>") { @@ -854,10 +846,8 @@ func inspectProcessGroup(ctx context.Context, pgid int) ([]processIdentity, erro return identities, nil } -var inspectAuthenticatedProcessGroup = inspectProcessGroup - func authenticateAuthenticatedGroup(ctx context.Context, rec *authenticatedRecord) ([]processIdentity, bool, error) { - members, err := inspectAuthenticatedProcessGroup(ctx, rec.PGID) + members, err := inspectProcessGroup(ctx, rec.PGID) if err != nil { return nil, false, err } @@ -866,6 +856,13 @@ func authenticateAuthenticatedGroup(ctx context.Context, rec *authenticatedRecor } for _, member := range members { if member.pid == rec.PGID { + // A different boot representation with the same start identity is + // not proof of PID reuse. Older Darwin agents recorded a mutable + // boot timestamp instead of the boot-session UUID. Keep their + // evidence and refuse cleanup; never reinterpret it or signal it. + if member.startID == rec.Leader.StartID && member.bootID != rec.Leader.BootID { + return members, false, fmt.Errorf("process group %d has an incompatible boot identity; record retained without signaling; recover the owning run and update its agent before restarting", rec.PGID) + } return members, member.matches(&rec.Leader), nil } } diff --git a/pkg/processgroup/reaper_test.go b/pkg/processgroup/reaper_test.go index 4be87f31..955f7e9e 100644 --- a/pkg/processgroup/reaper_test.go +++ b/pkg/processgroup/reaper_test.go @@ -22,15 +22,16 @@ import ( ) const ( - helperRoleEnv = "CODEFLY_TEST_PROCESS_GROUP_ROLE" - helperPortEnv = "CODEFLY_TEST_PROCESS_GROUP_PORT" - helperPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_PID_FILE" - helperChildPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_CHILD_PID_FILE" - helperReadyEnv = "CODEFLY_TEST_PROCESS_GROUP_READY_FILE" - helperReleaseEnv = "CODEFLY_TEST_PROCESS_GROUP_RELEASE_FILE" - helperTermEnv = "CODEFLY_TEST_PROCESS_GROUP_TERM_FILE" - helperPortsEnv = "CODEFLY_TEST_PROCESS_GROUP_PORTS" - helperStateDirEnv = "CODEFLY_TEST_PROCESS_GROUP_STATE_DIR" + authenticatedDirName = "authenticated-v1" // Explicit old-producer compatibility fixture. + helperRoleEnv = "CODEFLY_TEST_PROCESS_GROUP_ROLE" + helperPortEnv = "CODEFLY_TEST_PROCESS_GROUP_PORT" + helperPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_PID_FILE" + helperChildPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_CHILD_PID_FILE" + helperReadyEnv = "CODEFLY_TEST_PROCESS_GROUP_READY_FILE" + helperReleaseEnv = "CODEFLY_TEST_PROCESS_GROUP_RELEASE_FILE" + helperTermEnv = "CODEFLY_TEST_PROCESS_GROUP_TERM_FILE" + helperPortsEnv = "CODEFLY_TEST_PROCESS_GROUP_PORTS" + helperStateDirEnv = "CODEFLY_TEST_PROCESS_GROUP_STATE_DIR" ) func TestProcessGroupHelper(t *testing.T) { @@ -47,6 +48,32 @@ func TestProcessGroupHelper(t *testing.T) { if _, err := runnersbase.StartTrackedProcessGroup(command); err != nil { t.Fatal(err) } + if os.Getenv("CODEFLY_TEST_MIXED_REGISTRIES") == "1" { + current := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v2", fmt.Sprintf("%d.pgid", command.Process.Pid)) + data, err := os.ReadFile(current) + if err != nil { + t.Fatal(err) + } + var historical authenticatedRecord + if err := json.Unmarshal(data, &historical); err != nil { + t.Fatal(err) + } + data, err = json.Marshal(historical) + if err != nil { + t.Fatal(err) + } + previous := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v1") + if err := os.MkdirAll(previous, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(previous, fmt.Sprintf("%d.pgid", command.Process.Pid)), data, 0600); err != nil { + t.Fatal(err) + } + if err := os.Remove(current); err != nil { + t.Fatal(err) + } + + } if err := os.WriteFile(os.Getenv(helperPIDEnv), []byte(strconv.Itoa(command.Process.Pid)), 0o600); err != nil { t.Fatal(err) } @@ -77,14 +104,19 @@ func TestProcessGroupHelper(t *testing.T) { command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} command.Stdout = io.Discard command.Stderr = io.Discard - if err := command.Start(); err != nil { - t.Fatal(err) + if os.Getenv("CODEFLY_TEST_MIXED_REGISTRIES") == "1" { + if _, err := runnersbase.StartTrackedProcessGroup(command); err != nil { + t.Fatal(err) + } + } else { + if err := command.Start(); err != nil { + t.Fatal(err) + } + if err := writeLegacyPgidFile(command.Process.Pid, os.TempDir(), []string{os.Args[0]}); err != nil { + t.Fatal(err) + } } pid := command.Process.Pid - if err := writeLegacyPgidFile(pid, os.TempDir(), []string{os.Args[0]}); err != nil { - _ = syscall.Kill(-pid, syscall.SIGKILL) - t.Fatal(err) - } if err := os.WriteFile(os.Getenv(helperChildPIDEnv), []byte(strconv.Itoa(pid)), 0o600); err != nil { _ = syscall.Kill(-pid, syscall.SIGKILL) t.Fatal(err) @@ -279,6 +311,13 @@ func TestProcessGroupHelper(t *testing.T) { } func TestReaperCleansLegacyAgentProcessesAfterCurrentOwnerIsKilled(t *testing.T) { + testReaperOwnerChain(t) +} +func TestReaperConvergesAcrossProducerNamespaces(t *testing.T) { + t.Setenv("CODEFLY_TEST_MIXED_REGISTRIES", "1") + testReaperOwnerChain(t) +} +func testReaperOwnerChain(t *testing.T) { t.Setenv("HOME", t.TempDir()) dir := t.TempDir() port := availablePort(t) @@ -350,9 +389,26 @@ func TestReaperCleansAuthenticatedRootRecordAfterOwnerIsKilled(t *testing.T) { waitForFile(t, pidPath) waitForFile(t, readyPath) pid := readPID(t, pidPath) - currentRecord := filepath.Join(home, ".codefly", stateDirName, "authenticated-v1", fmt.Sprintf("%d.pgid", pid)) + currentRecord := filepath.Join(home, ".codefly", stateDirName, "authenticated-v2", fmt.Sprintf("%d.pgid", pid)) rootRecord := registryPath(t, pid) - if err := os.Rename(currentRecord, rootRecord); err != nil { + // Historical root JSON predates persisted workspace attribution. Preserve + // that old wire fixture rather than move a new namespace envelope to root. + data, err := os.ReadFile(currentRecord) + if err != nil { + t.Fatal(err) + } + var historical authenticatedRecord + if err := json.Unmarshal(data, &historical); err != nil { + t.Fatal(err) + } + data, err = json.Marshal(historical) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(rootRecord, data, 0600); err != nil { + t.Fatal(err) + } + if err := os.Remove(currentRecord); err != nil { t.Fatal(err) } t.Cleanup(func() { @@ -465,7 +521,7 @@ func TestStopManagedProcessGroupsReapsGraphAndPreservesStateForRestart(t *testin } } -func TestReaperTreatsEmptyAuthenticatedRecordAsClean(t *testing.T) { +func TestReaperUsesOwnerInspectionForSupportedRecords(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) port := availablePort(t) @@ -508,20 +564,15 @@ func TestReaperTreatsEmptyAuthenticatedRecordAsClean(t *testing.T) { if err := os.WriteFile(recordPath, recordData, 0o600); err != nil { t.Fatal(err) } - originalInspector := inspectAuthenticatedProcessGroup - inspectAuthenticatedProcessGroup = func(context.Context, int) ([]processIdentity, error) { return nil, nil } - t.Cleanup(func() { inspectAuthenticatedProcessGroup = originalInspector }) - evidence, err := ReapStaleProcessGroupsWithEvidence(context.Background(), AllWorkspaces()) - inspectAuthenticatedProcessGroup = originalInspector if err != nil { t.Fatal(err) } if evidence.Outcome != CleanupClean { t.Fatalf("cleanup outcome = %q, want %q", evidence.Outcome, CleanupClean) } - if _, err := os.Stat(recordPath); !errors.Is(err, os.ErrNotExist) { - t.Fatalf("empty authenticated record still exists: %v", err) + if _, err := os.Stat(recordPath); err != nil { + t.Fatalf("live v1 registration was not preserved: %v", err) } assertPortHeld(t, port) if err := syscall.Kill(-pid, 0); err != nil { @@ -895,6 +946,9 @@ func TestReaperContinuesConvergenceAfterRecordFailure(t *testing.T) { if err := os.WriteFile(lockPath, nil, 0o600); err != nil { t.Fatal(err) } + if _, err := runnersbase.ListProcessGroups(context.Background()); err != nil { + t.Fatal(err) + } if err := os.Chmod(registryDir, 0o555); err != nil { t.Fatal(err) } diff --git a/pkg/processgroup/registered.go b/pkg/processgroup/registered.go new file mode 100644 index 00000000..714be68e --- /dev/null +++ b/pkg/processgroup/registered.go @@ -0,0 +1,106 @@ +package processgroup + +import ( + "context" + "errors" + "fmt" + + runners "github.com/codefly-dev/core/runners/base" +) + +// PreviewManagedProcessGroups lists live, authenticated, in-scope registrations +// without signaling or removing records. Refusals match managed stop admission. +func PreviewManagedProcessGroups(ctx context.Context, scope Scope) ([]int, error) { + groups, err := runners.ListProcessGroups(ctx) + if err != nil { + return nil, err + } + var selected []int + for _, group := range groups { + if !group.Alive() { + continue + } + ownership, err := group.InspectOwnership(ctx) + if err != nil { + return nil, err + } + if !ownership.Authenticated { + return nil, fmt.Errorf("group %d ownership is unproven", group.PGID()) + } + if registeredGroupInScope(group, ownership, scope) { + selected = append(selected, group.PGID()) + } + } + return selected, nil +} + +// Core owns supported registry discovery, authentication and identity-bound +// signals. CLI supplies cleanup policy and evidence; the separate root scanner +// remains solely for historical line/JSON records outside Core's namespaces. +func reapRegisteredProcessGroups(ctx context.Context, policy reapPolicy) (CleanupEvidence, error) { + evidence := CleanupEvidence{Outcome: CleanupClean} + groups, err := runners.ListProcessGroups(ctx) + if err != nil { + evidence.Outcome = CleanupFailed + return evidence, err + } + var failures []error + for _, group := range groups { + outcome, outside, err := reconcileRegisteredGroup(ctx, group, policy) + if outside { + evidence.OutOfScope = append(evidence.OutOfScope, group.PGID()) + continue + } + if err != nil { + outcome = CleanupFailed + failures = append(failures, fmt.Errorf("reconcile registered group %d: %w", group.PGID(), err)) + } + // An owner handle has an identity, not a consumer-owned record path. + evidence.record(fmt.Sprintf("registered-group:%d", group.PGID()), group.PGID(), outcome) + } + return evidence, errors.Join(failures...) +} + +func reconcileRegisteredGroup(ctx context.Context, group *runners.TrackedProcessGroup, policy reapPolicy) (CleanupOutcome, bool, error) { + if err := ctx.Err(); err != nil { + return CleanupFailed, false, err + } + if !group.Alive() { + return CleanupClean, false, group.RemoveIfDeadContext(ctx) + } + ownership, err := group.InspectOwnership(ctx) + if err != nil { + return CleanupFailed, false, err + } + if !ownership.Authenticated { + return CleanupFailed, false, fmt.Errorf("live group ownership is unproven; registration retained without signaling") + } + if !registeredGroupInScope(group, ownership, policy.scope) { + return CleanupClean, true, nil + } + if ownership.OwnerAlive && !policy.stopManaged { + return CleanupClean, false, nil + } + if err := group.Terminate(ctx); err != nil { + return CleanupFailed, false, err + } + if err := group.RemoveIfDeadContext(ctx); err != nil { + return CleanupFailed, false, err + } + if !ownership.OwnerAlive { + return CleanupRecoveredOrphan, false, nil + } + return CleanupClean, false, nil +} + +func registeredGroupInScope(group *runners.TrackedProcessGroup, ownership *runners.ProcessGroupOwnership, scope Scope) bool { + if scope.All() { + return true + } + if ownership.LaunchWorkspace != "" { + return scope.Includes(ownership.LaunchWorkspace) + } + // Older registrations have no persisted workspace. Preserve their existing + // environment/cwd attribution only after Core has authenticated the group. + return scope.includesProcessGroup(group.PGID()) +} diff --git a/pkg/processgroup/registered_test.go b/pkg/processgroup/registered_test.go new file mode 100644 index 00000000..e5606ff4 --- /dev/null +++ b/pkg/processgroup/registered_test.go @@ -0,0 +1,94 @@ +package processgroup + +import ( + "context" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "syscall" + "testing" + "time" + + runners "github.com/codefly-dev/core/runners/base" +) + +func TestRegisteredWorkspaceOwner(t *testing.T) { + pidFile := os.Getenv("CODEFLY_TEST_REGISTERED_PID_FILE") + if pidFile == "" { + return + } + child := exec.Command("env", "-i", "sleep", "60") + child.Stdout, child.Stderr = io.Discard, io.Discard + group, err := runners.StartTrackedProcessGroup(child) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(pidFile, []byte(strconv.Itoa(group.PGID())), 0600); err != nil { + t.Fatal(err) + } +} + +func startUnobservableRun(t *testing.T, workspace, cwd string) int { + t.Helper() + pidFile := filepath.Join(t.TempDir(), "pid") + owner := exec.Command(os.Args[0], "-test.run=^TestRegisteredWorkspaceOwner$") + owner.Dir = cwd + owner.Env = append(withoutLaunchWorkspace(os.Environ()), LaunchWorkspaceEnv+"="+workspace, "CODEFLY_TEST_REGISTERED_PID_FILE="+pidFile) + if out, err := owner.CombinedOutput(); err != nil { + t.Fatalf("owner: %v: %s", err, out) + } + pid := readPID(t, pidFile) + t.Cleanup(func() { _ = syscall.Kill(-pid, syscall.SIGKILL) }) + deadline := time.Now().Add(5 * time.Second) + for { + identity, err := runners.InspectProcess(pid) + if err == nil && identity.Executable == "sleep" { + if _, visible := LaunchWorkspaceOfProcess(pid); !visible { + break + } + } + if time.Now().After(deadline) { + t.Fatal("child did not erase its environment") + } + time.Sleep(10 * time.Millisecond) + } + return pid +} + +func TestRegisteredStopUsesPersistedWorkspaceAfterOwnerAndEnvironmentDisappear(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + first, second := newWorkspace(t, "first"), newWorkspace(t, "second") + // Both children run from the sibling checkout. Its cwd is deliberately + // wrong evidence for the first child's launch ownership. + firstPID := startUnobservableRun(t, first, second) + secondPID := startUnobservableRun(t, second, second) + preview, err := PreviewManagedProcessGroups(context.Background(), InWorkspace(first)) + if err != nil || len(preview) != 1 || preview[0] != firstPID { + t.Fatalf("preview=%v err=%v", preview, err) + } + if !groupAlive(firstPID) || !groupAlive(secondPID) { + t.Fatal("preview signaled a group") + } + evidence, err := StopManagedProcessGroups(context.Background(), InWorkspace(first)) + if err != nil { + t.Fatal(err) + } + if groupAlive(firstPID) { + t.Fatal("target survived scoped stop") + } + if !groupAlive(secondPID) { + t.Fatal("scoped stop killed sibling") + } + if _, err := runners.LookupProcessGroup(secondPID); err != nil { + t.Fatalf("sibling registration lost: %v", err) + } + found := false + for _, pgid := range evidence.RecoveredPGIDs() { + found = found || pgid == firstPID + } + if !found { + t.Fatalf("target missing from recovery evidence: %+v", evidence) + } +} diff --git a/pkg/processgroup/scope.go b/pkg/processgroup/scope.go index 900a89dd..33ee74e5 100644 --- a/pkg/processgroup/scope.go +++ b/pkg/processgroup/scope.go @@ -65,16 +65,9 @@ func (s Scope) Includes(workspace string) bool { return sameDirectory(filepath.Clean(workspace), s.Workspace) } -// includesProcessGroup resolves a group's workspace from its leader and reports -// whether it is in scope. -// -// A registry record carries no workspace of its own — independently released -// agents write these and this package cannot change their format — so the -// leader is asked instead: the run recorded in its environment when it was -// launched, and only failing that the workspace enclosing its working -// directory. Asking the working directory first was the whole defect: a -// composed module's service runs from a checkout outside the workspace that -// composed it, so a scoped stop left it running and reported success. +// includesProcessGroup provides environment/cwd compatibility attribution for +// historical records without a persisted launch workspace. Current registered +// groups select Core's authenticated LaunchWorkspace before using this fallback. func (s Scope) includesProcessGroup(pgid int) bool { if s.All() { return true diff --git a/pkg/processgroup/scope_test.go b/pkg/processgroup/scope_test.go index 5ff56d44..1dc6d2a1 100644 --- a/pkg/processgroup/scope_test.go +++ b/pkg/processgroup/scope_test.go @@ -2,6 +2,7 @@ package processgroup import ( "context" + "errors" "io" "os" "os/exec" @@ -58,9 +59,11 @@ func startRunIn(t *testing.T, workspace string) int { t.Fatal(err) } pgid := tracked.PGID() + reaped := make(chan struct{}) + go func() { _, _ = command.Process.Wait(); close(reaped) }() t.Cleanup(func() { _ = syscall.Kill(-pgid, syscall.SIGKILL) - _, _ = command.Process.Wait() + <-reaped }) waitForFile(t, readyPath) return pgid @@ -73,15 +76,17 @@ func startRunIn(t *testing.T, workspace string) int { // find its run. func recordExists(t *testing.T, pgid int) bool { t.Helper() + if _, err := runnersbase.LookupProcessGroup(pgid); err == nil { + return true + } else if !errors.Is(err, runnersbase.ErrProcessGroupNotRegistered) { + t.Fatal(err) + } dir, err := stateDir() if err != nil { t.Fatal(err) } - name := strconv.Itoa(pgid) + ".pgid" - for _, candidate := range []string{filepath.Join(dir, authenticatedDirName, name), filepath.Join(dir, name)} { - if _, err := os.Stat(candidate); err == nil { - return true - } + if _, err := os.Stat(filepath.Join(dir, strconv.Itoa(pgid)+".pgid")); err == nil { + return true } return false }