Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions cli/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/Aureliolo/synthorg/cli/internal/health"
"github.com/Aureliolo/synthorg/cli/internal/ui"
"github.com/Aureliolo/synthorg/cli/internal/verify"
"github.com/Aureliolo/synthorg/cli/internal/version"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -121,7 +120,7 @@ func verifyAndPinImages(ctx context.Context, cmd *cobra.Command, state config.St
return fmt.Errorf("digest pin map: %w", err)
}

if err := writeDigestPinnedCompose(state, pins, safeDir, version.Version); err != nil {
if err := writeDigestPinnedCompose(state, pins, safeDir); err != nil {
return fmt.Errorf("pinning verified digests: %w", err)
}

Expand All @@ -137,9 +136,8 @@ func verifyAndPinImages(ctx context.Context, cmd *cobra.Command, state config.St
//
// Uses atomic write (temp file + rename) to prevent a partial write from
// corrupting the compose file if the process is interrupted.
func writeDigestPinnedCompose(state config.State, digestPins map[string]string, safeDir, cliVersion string) error {
func writeDigestPinnedCompose(state config.State, digestPins map[string]string, safeDir string) error {
params := compose.ParamsFromState(state)
params.CLIVersion = cliVersion
params.DigestPins = digestPins

composeYAML, err := compose.Generate(params)
Expand Down
230 changes: 207 additions & 23 deletions cli/cmd/update.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package cmd

import (
"bytes"
"context"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"time"

"github.com/Aureliolo/synthorg/cli/internal/compose"
"github.com/Aureliolo/synthorg/cli/internal/config"
"github.com/Aureliolo/synthorg/cli/internal/docker"
"github.com/Aureliolo/synthorg/cli/internal/health"
Expand All @@ -30,60 +35,239 @@
}

func runUpdate(cmd *cobra.Command, _ []string) error {
effectiveVersion, err := updateCLI(cmd)
if err != nil {
if err := updateCLI(cmd); errors.Is(err, errReexec) {
// Binary was replaced. Re-exec the new binary so compose refresh
// and image pull use the new embedded template and logic.
return reexecUpdate(cmd)
} else if err != nil {
return err
}
return updateContainerImages(cmd, effectiveVersion)

// Regenerate compose.yml from the current template to pick up any
// template changes (new env vars, hardening tweaks, service config).
if err := refreshCompose(cmd); err != nil {
return err
}

return updateContainerImages(cmd)
}

// errReexec is a sentinel error returned by updateCLI when the binary was
// replaced and the new binary should be re-executed to continue the update.
// The caller (runUpdate) handles this by spawning the new binary.
var errReexec = errors.New("cli updated, re-exec required")

// updateCLI checks for a new CLI release and optionally applies it.
// Returns the effective CLI version (the new version if updated, or the
// current version if not).
func updateCLI(cmd *cobra.Command) (string, error) {
// Returns errReexec if the binary was replaced (caller must re-exec).
func updateCLI(cmd *cobra.Command) error {
ctx := cmd.Context()
out := cmd.OutOrStdout()

// Warn on dev builds.
if version.Version == "dev" {
_, _ = fmt.Fprintln(out, "Warning: running a dev build update check will always report an update available.")
_, _ = fmt.Fprintln(out, "Warning: running a dev build -- update check will always report an update available.")
}

_, _ = fmt.Fprintln(out, "Checking for updates...")
result, err := selfupdate.Check(ctx)
if err != nil {
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not check for updates: %v\n", err)
return version.Version, nil
return nil
}

if !result.UpdateAvail {
_, _ = fmt.Fprintf(out, "CLI is up to date (%s)\n", result.CurrentVersion)
return version.Version, nil
return nil
}

_, _ = fmt.Fprintf(out, "New version available: %s (current: %s)\n", result.LatestVersion, result.CurrentVersion)

ok, err := confirmUpdate(fmt.Sprintf("Update CLI from %s to %s?", result.CurrentVersion, result.LatestVersion))
if err != nil {
return "", err
return err
}
if !ok {
return version.Version, nil
return nil
}

_, _ = fmt.Fprintln(out, "Downloading...")
binary, err := selfupdate.Download(ctx, result.AssetURL, result.ChecksumURL, result.SigstoreBundURL)
if err != nil {
return "", fmt.Errorf("downloading update: %w", err)
return fmt.Errorf("downloading update: %w", err)
}

if err := selfupdate.Replace(binary); err != nil {
return "", fmt.Errorf("replacing binary: %w", err)
return fmt.Errorf("replacing binary: %w", err)
}
_, _ = fmt.Fprintf(out, "CLI updated to %s\n", result.LatestVersion)
_, _ = fmt.Fprintf(out, "Release notes: %s/releases/tag/v%s\n",
version.RepoURL, strings.TrimPrefix(result.LatestVersion, "v"))
return result.LatestVersion, nil

// Signal the caller to re-exec the new binary so the rest of the
// update (compose refresh, image pull) uses the new embedded template.
return errReexec
}

// reexecUpdate spawns the new binary with the same arguments so the rest
// of the update (compose refresh, image pull) uses the new embedded template.
// The CLI update step already ran, so the new binary will see "up to date"
// and proceed directly to compose + images.
func reexecUpdate(cmd *cobra.Command) error {
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "Re-launching updated CLI to continue...")

execPath, err := os.Executable()
if err != nil {
return fmt.Errorf("finding executable path: %w", err)
}
// Resolve symlinks to match the pattern in uninstall.go --
// selfupdate.Replace writes to the resolved path.
if resolved, resolveErr := filepath.EvalSymlinks(execPath); resolveErr == nil {
execPath = resolved
}
Comment on lines +180 to +195

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error from filepath.EvalSymlinks is currently ignored. While it's unlikely to fail if the self-update process just succeeded, it's better practice to handle or at least log the error for robustness and easier debugging in case of unexpected filesystem issues or race conditions. This would provide more insight if the re-exec process fails to launch the correct binary.

Suggested change
if resolved, resolveErr := filepath.EvalSymlinks(execPath); resolveErr == nil {
execPath = resolved
}
if resolved, err := filepath.EvalSymlinks(execPath); err == nil {
execPath = resolved
} else {
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not resolve executable symlink: %v\n", err)
}


c := exec.CommandContext(cmd.Context(), execPath, os.Args[1:]...)
c.Stdin = os.Stdin
c.Stdout = cmd.OutOrStdout()
c.Stderr = cmd.ErrOrStderr()
return c.Run()
}
Comment on lines +171 to +211

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if other commands in this codebase propagate exit codes from child processes
rg -n 'exec\.CommandContext|exec\.Command' cli/cmd/*.go -A5 | head -60

Repository: Aureliolo/synthorg

Length of output: 647


🏁 Script executed:

# Find where reexecUpdate is called and understand the Cobra command setup
rg -n 'reexecUpdate' cli/cmd/*.go -B2 -A2

Repository: Aureliolo/synthorg

Length of output: 1087


🏁 Script executed:

# Check the main/root command setup and error handling
fd -t f -e go cli | xargs grep -l 'main\|cobra.Root' | head -5

Repository: Aureliolo/synthorg

Length of output: 44


🏁 Script executed:

# Look at how errors from Run/Execute are handled
rg -n 'cmd.Execute\|cmd.Run' cli/*.go -B2 -A2

Repository: Aureliolo/synthorg

Length of output: 44


🏁 Script executed:

# Find main entry point
fd -t f -e go cli | xargs grep -l 'func main'

Repository: Aureliolo/synthorg

Length of output: 46


🏁 Script executed:

# Look for root command setup in CLI
fd -t f -e go cli/cmd | xargs grep -l 'NewCmdRoot\|rootCmd\|RootCmd' | head -3

Repository: Aureliolo/synthorg

Length of output: 395


🏁 Script executed:

# Check how the update command is added to root command
rg -n 'updateCmd\|AddCommand.*update' cli/cmd/*.go

Repository: Aureliolo/synthorg

Length of output: 44


🏁 Script executed:

# List Go files in cli directory
find cli -maxdepth 2 -name '*.go' -type f | head -20

Repository: Aureliolo/synthorg

Length of output: 457


🏁 Script executed:

# Look for main function and root command setup
rg -n 'func main|RootCmd|rootCmd' cli/ -A 3

Repository: Aureliolo/synthorg

Length of output: 9619


Verify child process exit code propagation.

When c.Run() returns an error from the re-executed binary, the error is propagated to the caller. However, the main entry point (cli/main.go) converts any error to os.Exit(1), which masks the child's actual exit code. This loses exit code fidelity for CI systems expecting specific codes.

Consider extracting and returning the child's exit code:

♻️ Proposed enhancement for exit code fidelity
+	if err := c.Run(); err != nil {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
+			os.Exit(exitErr.ExitCode())
+		}
+		return err
+	}
+	return nil
-	return c.Run()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/cmd/update.go` around lines 126 - 157, reexecUpdate should preserve the
re-launched process exit code instead of always returning a generic error; after
c.Run(), detect if err is an *exec.ExitError (use errors.As) and extract the
exit code via ExitError.ExitCode(), then return a small typed error that carries
that code (e.g., define childExitError{code int} with Error() string) so the
program entrypoint can inspect that type and call os.Exit(code); otherwise
return the original err. Ensure references: reexecUpdate, exec.CommandContext /
c.Run, exec.ExitError, ExitCode(), and the new childExitError type.


// refreshCompose regenerates compose.yml from the current embedded template
// using the existing persisted state. If the regenerated compose differs from
// what is on disk, it shows the diff and asks the user to approve.
// This ensures template changes (new env vars, hardening, service config)
// are picked up on upgrade even when the image tag hasn't changed.
func refreshCompose(cmd *cobra.Command) error {
out := cmd.OutOrStdout()

dir := resolveDataDir()
state, err := config.Load(dir)
if err != nil {
return fmt.Errorf("loading config: %w", err)
}

safeDir, err := safeStateDir(state)
if err != nil {
return err
}

composePath := filepath.Join(safeDir, "compose.yml")
existing, fresh, err := loadAndGenerate(composePath, state)
if err != nil {
return err
}
if existing == nil {
return nil // no compose.yml on disk
}

if bytes.Equal(existing, fresh) {
_, _ = fmt.Fprintln(out, "Compose configuration is up to date.")
return nil
}

return applyComposeDiff(cmd, composePath, existing, fresh, safeDir)
}

// loadAndGenerate reads the existing compose and generates a fresh one from
// the template. Returns (nil, nil, nil) if no compose.yml exists on disk.
func loadAndGenerate(composePath string, state config.State) ([]byte, []byte, error) {
existing, err := os.ReadFile(composePath)
Comment thread Dismissed
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return nil, nil, nil
}
return nil, nil, fmt.Errorf("reading existing compose: %w", err)
}

params := compose.ParamsFromState(state)
params.DigestPins = state.VerifiedDigests
fresh, err := compose.Generate(params)
if err != nil {
return nil, nil, fmt.Errorf("generating compose from template: %w", err)
}
return existing, fresh, nil
}

// applyComposeDiff shows the diff between existing and fresh compose,
// asks the user to approve, and writes the fresh compose if approved.
func applyComposeDiff(cmd *cobra.Command, composePath string, existing, fresh []byte, safeDir string) error {
out := cmd.OutOrStdout()

diff := lineDiff(string(existing), string(fresh))
_, _ = fmt.Fprintln(out, "Compose template has changed:")
_, _ = fmt.Fprintln(out, diff)

ok, err := confirmUpdate("Apply compose configuration changes?")
if err != nil {
return err
}
if !ok {
_, _ = fmt.Fprintln(out, "Compose changes skipped. You can apply later with 'synthorg init'.")
return nil
}

if err := atomicWriteFile(composePath, fresh, safeDir); err != nil {
return fmt.Errorf("writing updated compose: %w", err)
}
_, _ = fmt.Fprintln(out, "Compose configuration updated.")
return nil
}

// secretKeyPattern matches YAML lines containing known secret keys.
// Used by lineDiff to redact sensitive values before displaying.
var secretKeyPattern = regexp.MustCompile(`(?i)^\s*(SYNTHORG_JWT_SECRET|JWT_SECRET|SECRET_KEY)\s*:`)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// lineDiff produces a bag-based diff showing added (+) and removed (-) lines
// between two strings. Lines containing secret keys are redacted.
//
// Note: this uses multiset membership, not positional diffing. Reordered
// lines are not reported as changes. This is acceptable for compose files
// where the user approves structural additions/removals, not reorderings.
func lineDiff(oldText, updated string) string {
oldLines := strings.Split(oldText, "\n")
newLines := strings.Split(updated, "\n")

newSet := make(map[string]int, len(newLines))
for _, l := range newLines {
newSet[l]++
}

oldSet := make(map[string]int, len(oldLines))
for _, l := range oldLines {
oldSet[l]++
}

var b strings.Builder
for _, l := range oldLines {
if newSet[l] > 0 {
newSet[l]--
continue
}
b.WriteString(" - ")
b.WriteString(redactSecret(l))
b.WriteByte('\n')
}
for _, l := range newLines {
if oldSet[l] > 0 {
oldSet[l]--
continue
}
b.WriteString(" + ")
b.WriteString(redactSecret(l))
b.WriteByte('\n')
}
return b.String()
}

// redactSecret replaces secret values with [REDACTED] in diff output.
func redactSecret(line string) string {
if secretKeyPattern.MatchString(line) {
idx := strings.Index(line, ":")
if idx >= 0 {
return line[:idx+1] + " [REDACTED]"
}
}
return line
}

// targetImageTag converts a CLI version string to a Docker image tag.
Expand Down Expand Up @@ -124,12 +308,12 @@
}

// updateContainerImages offers to update container images to match the
// given CLI version. Skips if images already match.
func updateContainerImages(cmd *cobra.Command, effectiveVersion string) error {
// current CLI version. Skips if images already match.
func updateContainerImages(cmd *cobra.Command) error {
ctx := cmd.Context()
out := cmd.OutOrStdout()

tag := targetImageTag(effectiveVersion)
tag := targetImageTag(version.Version)

dir := resolveDataDir()
state, err := config.Load(dir)
Expand Down Expand Up @@ -162,7 +346,7 @@
return nil
}

if err := pullAndPersist(ctx, cmd, info, state, tag, safeDir, effectiveVersion); err != nil {
if err := pullAndPersist(ctx, cmd, info, state, tag, safeDir); err != nil {
return err
}

Expand Down Expand Up @@ -190,7 +374,7 @@
// pullAndPersist regenerates compose.yml, pulls images, and persists config.
// If any step fails, the previous compose.yml is restored (or removed if it
// did not exist before) so that the on-disk state remains consistent.
func pullAndPersist(ctx context.Context, cmd *cobra.Command, info docker.Info, state config.State, tag, safeDir, effectiveVersion string) error {
func pullAndPersist(ctx context.Context, cmd *cobra.Command, info docker.Info, state config.State, tag, safeDir string) error {
out := ui.NewUI(cmd.OutOrStdout())

// Back up existing compose.yml for rollback on failure.
Expand All @@ -211,7 +395,7 @@
// Verify + write compose atomically: compose.yml is only updated after
// verification succeeds (or when --skip-verify explicitly skips it).
// This prevents a crash from leaving an unverified tag on disk.
digestPins, err := verifyAndPinForUpdate(ctx, state, tag, safeDir, effectiveVersion, out, errOut)
digestPins, err := verifyAndPinForUpdate(ctx, state, tag, safeDir, out, errOut)
if err != nil {
rollback()
return err
Expand All @@ -237,14 +421,14 @@

// verifyAndPinForUpdate runs image verification and regenerates the compose
// file with digest pins. Returns the digest pin map (nil if --skip-verify).
func verifyAndPinForUpdate(ctx context.Context, state config.State, tag, safeDir, effectiveVersion string, out *ui.UI, errOut *ui.UI) (map[string]string, error) {
func verifyAndPinForUpdate(ctx context.Context, state config.State, tag, safeDir string, out *ui.UI, errOut *ui.UI) (map[string]string, error) {
updatedState := state
updatedState.ImageTag = tag

if skipVerify {
errOut.Warn("Image verification skipped (--skip-verify). Containers are NOT verified.")
// Write compose with the new tag but no digest pins.
if err := writeDigestPinnedCompose(updatedState, nil, safeDir, effectiveVersion); err != nil {
if err := writeDigestPinnedCompose(updatedState, nil, safeDir); err != nil {
return nil, err
}
return nil, nil
Expand All @@ -270,7 +454,7 @@
}

// Write compose with verified digest pins.
if err := writeDigestPinnedCompose(updatedState, pins, safeDir, effectiveVersion); err != nil {
if err := writeDigestPinnedCompose(updatedState, pins, safeDir); err != nil {
return nil, err
}
return pins, nil
Expand Down
Loading
Loading