fix(cli): regenerate compose and re-exec binary on update#576
fix(cli): regenerate compose and re-exec binary on update#576
Conversation
The `synthorg update` command updated the CLI binary and pulled new container images, but never regenerated compose.yml from the updated template. This meant new env vars (like SYNTHORG_LOG_DIR), hardening changes, and service config additions were silently missed -- users had to manually run `synthorg init` to pick them up. Two fixes: - After CLI binary replacement, re-exec the new binary so compose refresh and image pull use the new embedded template - Add refreshCompose step that diffs the on-disk compose against a fresh render from the template, shows the diff, and asks the user to approve before writing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…, docs Pre-reviewed by 4 agents, 15 findings addressed: - Redact JWT secrets in compose diff output (security) - Only ignore ErrNotExist when reading compose, propagate other errors - Resolve symlinks in reexecUpdate (consistent with uninstall.go) - Change updateCLI signature to return error only (string was unused) - Extract loadAndGenerate + applyComposeDiff to keep functions under 50 lines - Remove dead oldSet allocation in lineDiff - Rename 'new' parameter to 'updated' (avoids shadowing built-in) - Add secret redaction with secretKeyPattern regex - Convert lineDiff tests to table-driven pattern - Add trailing-newline and secret-redaction test cases - Improve errReexec test to verify errors.Is identity - Update operations.md and user_guide.md for new update behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🧰 Additional context used📓 Path-based instructions (1)cli/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2026-03-19T11:29:37.522ZApplied to files:
📚 Learning: 2026-03-15T11:48:14.867ZApplied to files:
📚 Learning: 2026-03-15T11:48:14.867ZApplied to files:
🧬 Code graph analysis (1)cli/cmd/update_test.go (1)
🔇 Additional comments (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds CLI self-update with automatic re-exec, interactive compose-template refresh showing a redacted diff for user approval, optional compose-preserving image updates that patch only image: lines, and removes CLI-version propagation when writing digest-pinned compose files. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as "CLI Process"
participant OS as "OS / Process Launcher"
participant State as "Persisted State"
participant Compose as "Compose Generator"
participant Images as "Image Updater"
User->>CLI: run "synthorg update"
CLI->>CLI: updateCLI()
alt binary replaced (errReexec)
CLI->>OS: reexecUpdate() (spawn updated exe with flags)
OS-->>CLI: new CLI process starts
end
CLI->>State: load persisted state
CLI->>Compose: loadAndGenerate(state, verifiedDigests)
Compose-->>CLI: generated compose bytes
CLI->>CLI: lineDiff + redactSecret -> prompt user
alt user approves
CLI->>Compose: atomic write new compose.yml
else user declines
CLI->>CLI: set preserveCompose = true
end
CLI->>Images: updateContainerImages(state, preserveCompose)
Images-->>CLI: image updates complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a robust self-update mechanism for the CLI, including re-executing the new binary and refreshing the Docker Compose configuration. The changes are well-structured, with clear separation of concerns into new functions. The addition of secret redaction in the compose diff is a great security improvement. The new logic is also well-covered by unit tests and documentation updates. I have one minor suggestion to improve error handling for even greater robustness.
| if resolved, resolveErr := filepath.EvalSymlinks(execPath); resolveErr == nil { | ||
| execPath = resolved | ||
| } |
There was a problem hiding this comment.
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.
| 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) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/update_test.go`:
- Around line 33-99: Add a fuzz test for lineDiff named FuzzLineDiff that uses
testing.F: register representative seed pairs with f.Add (e.g.,
"line1\nline2","line1\nline3" and "" , "new content") and call f.Fuzz with a
func(t *testing.T, old, updated string) that simply invokes lineDiff(old,
updated) to ensure it never panics on arbitrary inputs; place the new test
alongside TestLineDiff and import testing if needed.
In `@cli/cmd/update.go`:
- Around line 216-218: The secretKeyPattern used by lineDiff currently only
matches SYNTHORG_JWT_SECRET, JWT_SECRET, and SECRET_KEY; expand it to
future-proof redaction by adding common sensitive keys such as API_KEY,
PASSWORD, TOKEN (and any other env names you expect) and ensure the regex keeps
case-insensitive and leading whitespace handling; update the var
secretKeyPattern = regexp.MustCompile(...) accordingly and keep lineDiff usage
unchanged so new keys are redacted without further code changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ecf8cc8f-4bd3-4498-913b-afb568ff1317
📒 Files selected for processing (5)
cli/cmd/start.gocli/cmd/update.gocli/cmd/update_test.godocs/design/operations.mddocs/user_guide.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CLI Test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue. The design spec is the starting point for architecture, data models, and behavior.
Files:
docs/user_guide.mddocs/design/operations.md
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use native Go testing withtesting.Ffuzz functions (Fuzz*) for fuzz testing.
Lint Go code withgolangci-lintandgo vet. Run tests with-raceflag to detect race conditions.
Exception to the 'never use cd' rule: Go commands requirecd clibecause the Go module is incli/— Go tooling requires working directory to be the module root.
Files:
cli/cmd/start.gocli/cmd/update_test.gocli/cmd/update.go
🧠 Learnings (8)
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to docker/compose.yml : Configure Docker Compose with backend, web, and sandbox services. All environment variables defined in `docker/.env` (generate from `docker/.env.example`).
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
docs/design/operations.mdcli/cmd/start.gocli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
docs/design/operations.mdcli/cmd/start.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: CLI workflow (`.github/workflows/cli.yml`) runs Go lint (golangci-lint + go vet) + test (race, coverage) + build (cross-compile matrix) + vulnerability check (govulncheck) + fuzz testing. Cross-compiles for linux/darwin/windows × amd64/arm64. GoReleaser release on v* tags with cosign keyless signing and SLSA L3 attestations.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Shell commands: for Go CLI work, cd cli is an exception because Go tooling requires working directory to be the module root. Go commands require `cd cli` for other work, never use `cd`.
Applied to files:
docs/design/operations.mdcli/cmd/update.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Lint Go code with `golangci-lint` and `go vet`. Run tests with `-race` flag to detect race conditions.
Applied to files:
cli/cmd/update_test.go
🧬 Code graph analysis (2)
cli/cmd/start.go (1)
cli/internal/config/state.go (1)
State(16-28)
cli/cmd/update.go (6)
cli/internal/version/version.go (2)
Version(9-9)RepoURL(5-5)cli/internal/selfupdate/updater.go (3)
Check(67-69)Download(195-230)Replace(233-239)cli/internal/config/state.go (2)
Load(53-91)State(16-28)cli/internal/compose/generate.go (2)
ParamsFromState(49-62)Generate(66-85)cli/internal/docker/client.go (1)
Info(22-29)cli/internal/ui/ui.go (1)
UI(44-53)
🪛 GitHub Check: CodeQL
cli/cmd/update.go
[failure] 174-174: Uncontrolled data used in path expression
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
🔇 Additional comments (16)
docs/design/operations.md (1)
974-977: LGTM!Documentation accurately reflects the new
updatecommand behavior: automatic re-exec after binary replacement and compose template refresh with diff approval.docs/user_guide.md (1)
27-27: LGTM!Clear user-facing documentation explaining the re-launch behavior after binary replacement and the compose diff approval flow.
cli/cmd/start.go (2)
123-125: LGTM!The removal of the
cliVersionparameter is correct sincecompose.ParamsFromStatealready populatesCLIVersionfromversion.Versioninternally.
139-149: LGTM!Function signature simplification is well-documented and the implementation correctly delegates version handling to
compose.ParamsFromState.cli/cmd/update_test.go (3)
33-99: LGTM!Comprehensive table-driven tests for
lineDiffcovering identical inputs, additions, removals, changes, and trailing newline edge cases.
101-131: LGTM!Good coverage for
redactSecretincluding case-insensitive matching and ensuring non-secret lines remain unchanged.
133-141: LGTM!Sentinel error identity test correctly verifies
errors.Issemantics for the re-exec flow.cli/cmd/update.go (9)
37-58: LGTM!Clean implementation using a sentinel error for re-exec control flow. The
errors.Ischeck is idiomatic and the flow is well-structured.
60-109: LGTM!Clean refactor removing version propagation. Graceful handling of check failures with warnings, and clear signaling via
errReexecafter successful binary replacement.
111-133: LGTM!Proper re-exec implementation with symlink resolution matching
selfupdate.Replacebehavior. Context and stdio are correctly propagated.
191-214: LGTM!Clear user experience with diff display, confirmation prompt, and helpful message when changes are skipped.
216-260: LGTM!The bag-based diff approach is appropriate for YAML files where line order often doesn't affect semantics. Secret redaction is correctly applied to all output lines.
Minor UX note: The diff shows isolated added/removed lines without surrounding context, which may be harder to interpret for large files. Consider this a potential future enhancement if user feedback indicates confusion.
310-356: LGTM!Clean implementation with proper version comparison to skip unnecessary updates, graceful Docker unavailability handling, and clear update flow.
377-420: LGTM!Robust rollback mechanism for compose.yml changes. The backup/restore pattern ensures consistency on failure.
424-461: LGTM!Clean separation of verification logic with proper handling of
--skip-verifyflag. The 120-second timeout on OCI registry calls prevents indefinite hangs.
171-189: No action required — path traversal protection is properly implemented.
safeStateDircallsconfig.SecurePath, which validates the path by callingfilepath.Clean()and checkingfilepath.IsAbs(). TheSecurePathfunction is explicitly documented as satisfying CodeQL'sgo/path-injectioncheck. The composed pathfilepath.Join(safeDir, "compose.yml")is safe since the directory is validated before the hardcoded filename is appended.
Pre-reviewed by 4 local agents + CodeRabbit + Gemini + CodeQL, 12 findings: - Eliminate double config.Load TOCTOU: load state once in runUpdate, thread through refreshCompose and updateContainerImages - Reconstruct re-exec args explicitly instead of forwarding raw os.Args (prevents silent --skip-verify propagation) - Broaden secretKeyPattern to cover TOKEN, PASSWORD, API_KEY, CREDENTIALS - Use regex submatch position in redactSecret instead of strings.Index - Fix os.Stat error handling in start.go (was ignoring non-ErrNotExist) - Replace em-dash with ASCII dashes in start.go error message - Log EvalSymlinks failure to stderr in reexecUpdate - Add FuzzLineDiff fuzz test - Add TestLoadAndGenerate_NoCompose and _PermissionError tests - Improve TestErrReexec_Identity to test wrapping survival - Expand TestRedactSecret with TOKEN/PASSWORD/API_KEY/CREDENTIALS cases - Update Cobra Short help string to mention compose refresh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/update_test.go`:
- Around line 198-215: The test TestLoadAndGenerate_PermissionError should
restore file permissions before making assertions to avoid masking failures:
after invoking loadAndGenerate(composePath, config.State{}) and before checking
err or using the file, call os.Chmod(composePath, 0o600) (failing the test with
t.Fatalf if that chmod returns an error) so the file is accessible for
subsequent assertions; keep the existing t.Cleanup(func() { _ =
os.Chmod(composePath, 0o600) }) for best-effort cleanup but perform the
guaranteed chmod immediately before the error checks that reference composePath
or the error content.
In `@cli/cmd/update.go`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b5c364d-c15e-4c62-b8f4-e7b4496a516f
📒 Files selected for processing (3)
cli/cmd/start.gocli/cmd/update.gocli/cmd/update_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CLI Test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use native Go testing withtesting.Ffuzz functions (Fuzz*) for fuzz testing.
Lint Go code withgolangci-lintandgo vet. Run tests with-raceflag to detect race conditions.
Exception to the 'never use cd' rule: Go commands requirecd clibecause the Go module is incli/— Go tooling requires working directory to be the module root.
Files:
cli/cmd/start.gocli/cmd/update_test.gocli/cmd/update.go
🧠 Learnings (5)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/start.gocli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
cli/cmd/start.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Lint Go code with `golangci-lint` and `go vet`. Run tests with `-race` flag to detect race conditions.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Shell commands: for Go CLI work, cd cli is an exception because Go tooling requires working directory to be the module root. Go commands require `cd cli` for other work, never use `cd`.
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (2)
cli/cmd/start.go (1)
cli/internal/config/state.go (1)
State(16-28)
cli/cmd/update.go (5)
cli/internal/config/state.go (2)
Load(53-91)State(16-28)cli/internal/version/version.go (1)
Version(9-9)cli/internal/selfupdate/updater.go (3)
Check(67-69)Download(195-230)Replace(233-239)cli/internal/compose/generate.go (2)
ParamsFromState(49-62)Generate(66-85)cli/internal/docker/client.go (1)
Info(22-29)
🔇 Additional comments (15)
cli/cmd/start.go (2)
45-50: LGTM! Proper error differentiation foros.Stat.The change correctly distinguishes between a missing
compose.yml(user-actionable: run init) and otherStaterrors (permission issues, broken symlinks, etc.) that should surface the underlying cause.
126-127: LGTM! Simplified signature removes redundant version threading.Removing the
cliVersionparameter is correct sincecompose.ParamsFromState(state)already populatesCLIVersionfromversion.Version(as shown incli/internal/compose/generate.golines 48-61). This eliminates redundant parameter passing and ensures consistency.Also applies to: 142-144
cli/cmd/update_test.go (4)
39-105: LGTM! Comprehensive table-driven tests forlineDiff.Good coverage of edge cases: identical input, added/removed/changed lines, and trailing newline handling. The use of
wantContains/wantAbsentassertions appropriately validates the bag-based diff behavior without being overly strict about formatting.
107-116: LGTM! Fuzz test follows coding guidelines.The
FuzzLineDifffunction correctly usestesting.Fwith representative seed values and validates thatlineDiffdoesn't panic on arbitrary input, per the coding guidelines forcli/**/*.go.
118-168: LGTM! Thorough secret redaction test coverage.Tests cover all documented secret patterns (SECRET, PASSWORD, TOKEN, API_KEY, CREDENTIALS) including case-insensitivity and non-secret pass-through behavior.
170-184: LGTM! Sentinel error identity tests are well-structured.The tests correctly verify
errReexecbehavior witherrors.Isfor both direct comparison and wrapped errors, ensuring the sentinel pattern works as expected for the re-exec control flow.cli/cmd/update.go (9)
37-61: LGTM! Well-structured update flow with single config load.The refactored
runUpdatecorrectly:
- Handles
errReexecto re-launch after binary replacement- Loads config once and threads state through subsequent steps (avoiding TOCTOU)
- Separates compose refresh from container image updates
The re-exec won't infinite loop because the new binary's
updateCLIwill see "CLI is up to date" and returnnilinstead oferrReexec.
63-117: LGTM! Clean sentinel error pattern for re-exec signaling.Using
errReexecas a sentinel is idiomatic Go. The flow correctly returnsnilfor all non-update paths (no update available, user declined, dev build warning) and only returnserrReexecafter successfulReplace.
141-150: Good defensive arg reconstruction.Explicitly reconstructing args from known flags (
--data-dir,--skip-verify) instead of forwardingos.Argsprevents unexpected flag propagation. The warning for--skip-verifyis appropriate given its security implications.
188-206: Path input is validated upstream; GHAS finding is a false positive.The
composePathat line 191 is constructed fromsafeDir(validated viasafeStateDir→config.SecurePath) and a constant filename"compose.yml". The GHAS alert about "uncontrolled data used in path expression" is mitigated by the upstream validation inconfig.SecurePath.
247-281: Bag-based diff is appropriate for compose approval UX.The multiset approach correctly identifies added/removed lines without being sensitive to reordering. This is a reasonable trade-off for user-facing diffs where structural changes matter more than line positions.
One edge case: if the same line appears multiple times, only the net difference is shown. This is acceptable for compose files where duplicate lines are rare.
286-293: LGTM! Redaction uses submatch position correctly.Using
loc[1](end of match including colon) ensures the key name is preserved while the value is replaced. This provides useful context in the diff while protecting secrets.
332-372: LGTM! Simplified signature aligns with compose generation changes.Removing
effectiveVersionparameter is consistent with the changes instart.goandwriteDigestPinnedCompose. The version is now derived fromversion.Versionviacompose.ParamsFromState.
393-436: LGTM! Consistent signature updates throughout the update flow.
pullAndPersistandverifyAndPinForUpdatesignatures are updated consistently with the removal of explicit version threading. The rollback logic inpullAndPersistcorrectly handles both existing and non-existing compose files.Also applies to: 440-477
237-239: The regex intentionally uses broad patterns to avoid missing secrets—false positives are acceptable here.The pattern
\w*(SECRET|...)\w*will match variations likeSECRETKEEPERorNOTSECRETLYalongside legitimate secrets likeSYNTHORG_JWT_SECRET. This is by design: the goal is to redact sensitive values in diff output before displaying in logs, so over-redaction is safer than missing an actual secret. Tests confirm all common secret patterns (JWT_SECRET, PASSWORD, TOKEN, API_KEY, CREDENTIALS) are correctly identified, and the broad matching prevents false negatives at the acceptable cost of potential false positives.
| 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 | ||
| } else { | ||
| _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not resolve executable symlink: %v\n", resolveErr) | ||
| } | ||
|
|
||
| // Reconstruct args from known flags instead of forwarding os.Args | ||
| // to avoid silently propagating unexpected flags. | ||
| reArgs := []string{"update"} | ||
| if dataDir != "" { | ||
| reArgs = append(reArgs, "--data-dir", dataDir) | ||
| } | ||
| if skipVerify { | ||
| reArgs = append(reArgs, "--skip-verify") | ||
| _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Warning: --skip-verify is being carried forward to the re-launched CLI.") | ||
| } | ||
|
|
||
| c := exec.CommandContext(cmd.Context(), execPath, reArgs...) | ||
| c.Stdin = os.Stdin | ||
| c.Stdout = cmd.OutOrStdout() | ||
| c.Stderr = cmd.ErrOrStderr() | ||
| return c.Run() | ||
| } |
There was a problem hiding this comment.
🧹 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 -60Repository: 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 -A2Repository: 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 -5Repository: 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 -A2Repository: 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 -3Repository: 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/*.goRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
# List Go files in cli directory
find cli -maxdepth 2 -name '*.go' -type f | head -20Repository: Aureliolo/synthorg
Length of output: 457
🏁 Script executed:
# Look for main function and root command setup
rg -n 'func main|RootCmd|rootCmd' cli/ -A 3Repository: 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.
- reexecUpdate now returns ChildExitError with the child's exit code instead of a generic exec.ExitError, so main.go can os.Exit(code) rather than always exiting 1 - TestLoadAndGenerate_PermissionError restores file permissions before assertions to avoid masking failures on cleanup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/update.go`:
- Around line 56-60: The current flow lets updateContainerImages run even when
applyComposeDiff was declined, which bypasses the compose-approval prompt and
causes writeDigestPinnedCompose to rewrite compose.yml; fix by short-circuiting
when the user declines (or by providing an explicit result flag) so
updateContainerImages is not invoked after a declined applyComposeDiff.
Concretely: have refreshCompose/applyComposeDiff return a non-nil sentinel (or
error) or a boolean (e.g., "applied bool" or "approved bool") and check that
value before calling updateContainerImages in the caller (the block around
refreshCompose -> updateContainerImages), or alternatively change
verifyAndPinForUpdate/writeDigestPinnedCompose to support an
"update-only-digests-in-place" path that modifies the existing compose file
instead of regenerating the template when applyComposeDiff indicates decline;
reference functions applyComposeDiff, refreshCompose, updateContainerImages,
verifyAndPinForUpdate, and writeDigestPinnedCompose when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7184ed0c-8044-4188-b5da-3d384b953d20
📒 Files selected for processing (3)
cli/cmd/update.gocli/cmd/update_test.gocli/main.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use native Go testing withtesting.Ffuzz functions (Fuzz*) for fuzz testing.
Lint Go code withgolangci-lintandgo vet. Run tests with-raceflag to detect race conditions.
Exception to the 'never use cd' rule: Go commands requirecd clibecause the Go module is incli/— Go tooling requires working directory to be the module root.
Files:
cli/main.gocli/cmd/update_test.gocli/cmd/update.go
🧠 Learnings (6)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/main.gocli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: CLI workflow (`.github/workflows/cli.yml`) runs Go lint (golangci-lint + go vet) + test (race, coverage) + build (cross-compile matrix) + vulnerability check (govulncheck) + fuzz testing. Cross-compiles for linux/darwin/windows × amd64/arm64. GoReleaser release on v* tags with cosign keyless signing and SLSA L3 attestations.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Lint Go code with `golangci-lint` and `go vet`. Run tests with `-race` flag to detect race conditions.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Shell commands: for Go CLI work, cd cli is an exception because Go tooling requires working directory to be the module root. Go commands require `cd cli` for other work, never use `cd`.
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (3)
cli/main.go (1)
cli/cmd/update.go (1)
ChildExitCode(132-138)
cli/cmd/update_test.go (1)
cli/internal/config/state.go (1)
State(16-28)
cli/cmd/update.go (2)
cli/internal/version/version.go (1)
Version(9-9)cli/internal/compose/generate.go (2)
ParamsFromState(49-62)Generate(66-85)
🔇 Additional comments (1)
cli/main.go (1)
12-16: Nice exit-code propagation.This preserves the re-exec'd child's status for callers and CI instead of collapsing every failure to exit code
1.
When the user declines compose template changes during update, writeDigestPinnedCompose would silently regenerate compose from the new template anyway (applying the declined changes). Now: - User declines compose refresh -> warned that new images may not work with old compose -> asked to confirm (default: no) - If they insist: images update but compose.yml is preserved -- only image references (tag/digest) are patched via patchComposeImageRefs - If they decline: image update skipped entirely Also adds confirmUpdateWithDefault for prompts with configurable default, and tests for patchComposeImageRefs (digest-pinned + tag fallback scenarios). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/update_test.go`:
- Around line 204-222: The test sets composePath to 0o000 to simulate permission
denial but can be bypassed in root/container environments; after creating the
file (os.WriteFile) and before calling loadAndGenerate(composePath,
config.State{}), attempt to open/read the file (e.g., os.Open or os.ReadFile)
and if that read succeeds, call t.Skipf explaining the environment can bypass
mode bits so the test is not valid; keep the existing cleanup that restores
permissions via os.Chmod(composePath, 0o600) and then proceed with the original
assertions only when the read attempt fails.
In `@cli/cmd/update.go`:
- Around line 542-546: patchComposeImageRefs can return nil even when it made no
replacements (or only a subset), causing writeOrPatchCompose to treat
preserve-mode updates as successful while config.ImageTag/VerifiedDigests are
advanced; change patchComposeImageRefs to detect and return an error when zero
replacements occur or when not all expected service image refs were patched
(consider validating against state.Services or the digestPins keys), then update
writeOrPatchCompose to propagate that error (so preserve-mode fails fast) and
ensure the callers that call config.Save only do so when patchComposeImageRefs
succeeded; also apply the same validation/propagation to the other preserve-mode
call sites that currently assume a nil result (e.g., the second invocation
similar to writeOrPatchCompose) so compose.yml is never left unpatched while
metadata advances.
- Around line 66-78: The prompt text passed to confirmUpdateWithDefault is
misleading because even when preserving the compose file it may still rewrite
matched image: lines via patchComposeImageRefs when updateContainerImages is
called; either change the prompt string to clearly state that images in
compose.yml may be updated (e.g., "Compose file preserved except image:
references may be updated") or, if a true no-touch option is desired, skip
calling updateContainerImages when the preserve flag is set; update the
call-site using confirmUpdateWithDefault and the subsequent branch that calls
updateContainerImages(cmd, state, true) (and consider the forceImages flag) so
the behavior and message align.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bc99e6fc-5620-472a-b13e-e06665053024
📒 Files selected for processing (2)
cli/cmd/update.gocli/cmd/update_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use native Go testing withtesting.Ffuzz functions (Fuzz*) for fuzz testing.
Lint Go code withgolangci-lintandgo vet. Run tests with-raceflag to detect race conditions.
Exception to the 'never use cd' rule: Go commands requirecd clibecause the Go module is incli/— Go tooling requires working directory to be the module root.
Files:
cli/cmd/update_test.gocli/cmd/update.go
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: CLI workflow (`.github/workflows/cli.yml`) runs Go lint (golangci-lint + go vet) + test (race, coverage) + build (cross-compile matrix) + vulnerability check (govulncheck) + fuzz testing. Cross-compiles for linux/darwin/windows × amd64/arm64. GoReleaser release on v* tags with cosign keyless signing and SLSA L3 attestations.
Applied to files:
cli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to cli/**/*.go : Lint Go code with `golangci-lint` and `go vet`. Run tests with `-race` flag to detect race conditions.
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T09:01:47.243Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T09:01:47.243Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
cli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Pre-push hooks: mypy type-check + pytest unit tests + golangci-lint + go vet + go test (CLI, conditional on cli/**/*.go) (fast gate before push, skipped in pre-commit.ci — dedicated CI jobs already run these).
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Shell commands: for Go CLI work, cd cli is an exception because Go tooling requires working directory to be the module root. Go commands require `cd cli` for other work, never use `cd`.
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (1)
cli/cmd/update.go (4)
cli/internal/config/state.go (2)
Load(53-91)State(16-28)cli/internal/version/version.go (2)
Version(9-9)RepoURL(5-5)cli/internal/selfupdate/updater.go (3)
Check(67-69)Download(195-230)Replace(233-239)cli/internal/compose/generate.go (2)
ParamsFromState(49-62)Generate(66-85)
🪛 GitHub Check: CodeQL
cli/cmd/update.go
[failure] 560-560: Uncontrolled data used in path expression
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
- patchComposeImageRefs now errors when zero image refs found or required services (backend, web) are missing -- prevents config.Save from advancing state when compose is unpatched - Fix misleading prompt: "will NOT be modified" -> clarifies that image references will be updated while template changes are preserved - TestLoadAndGenerate_PermissionError: skip when environment bypasses file mode bits (root/container) - Add TestPatchComposeImageRefs_NoMatchesError and _MissingRequiredService - Fix TestPatchComposeImageRefs_TagFallback fixture (was missing web) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cli/cmd/update.go (1)
540-598:⚠️ Potential issue | 🟡 MinorAdd sandbox validation to
patchComposeImageRefswhen sandbox service is enabled.When
preserveCompose=trueandstate.Sandbox=true,digestPinsincludes a sandbox digest from verification. However,patchComposeImageRefsonly validates thatbackendandwebimage references were patched—sandbox validation is skipped. If the compose file has a malformed or mirrored sandbox image reference that doesn't match the regex, the function succeeds whileconfig.SaveadvancesVerifiedDigestswith the sandbox digest, leaving compose.yml pointing to an outdated sandbox image.Pass
state.SandboxtopatchComposeImageRefsand validate sandbox when enabled:Proposed fix
-func patchComposeImageRefs(tag string, digestPins map[string]string, safeDir string) error { +func patchComposeImageRefs(tag string, digestPins map[string]string, safeDir string, sandbox bool) error { composePath := filepath.Join(safeDir, "compose.yml") existing, err := os.ReadFile(composePath) if err != nil { return fmt.Errorf("reading compose for image patching: %w", err) } replaced := make(map[string]bool) patched := imageLinePattern.ReplaceAllStringFunc(string(existing), func(match string) string { sub := imageLinePattern.FindStringSubmatch(match) if len(sub) < 3 { return match } prefix := sub[1] name := sub[2] repo := "ghcr.io/aureliolo/synthorg-" + name replaced[name] = true if d, ok := digestPins[name]; ok && d != "" { return prefix + repo + "@" + d } return prefix + repo + ":" + tag }) if len(replaced) == 0 { return fmt.Errorf("no synthorg image references found in %s -- compose may be manually edited; run 'synthorg init' to regenerate", composePath) } - // At minimum, backend and web must be patched. - for _, required := range []string{"backend", "web"} { + // Required services must be patched. + required := []string{"backend", "web"} + if sandbox { + required = append(required, "sandbox") + } + for _, svc := range required { - if !replaced[required] { - return fmt.Errorf("image reference for %q not found in %s -- compose may be manually edited; run 'synthorg init' to regenerate", required, composePath) + if !replaced[svc] { + return fmt.Errorf("image reference for %q not found in %s -- compose may be manually edited; run 'synthorg init' to regenerate", svc, composePath) } } return atomicWriteFile(composePath, []byte(patched), safeDir) }Update the caller in
writeOrPatchCompose:func writeOrPatchCompose(state config.State, digestPins map[string]string, safeDir string, preserveCompose bool) error { if !preserveCompose { return writeDigestPinnedCompose(state, digestPins, safeDir) } - return patchComposeImageRefs(state.ImageTag, digestPins, safeDir) + return patchComposeImageRefs(state.ImageTag, digestPins, safeDir, state.Sandbox) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/cmd/update.go` around lines 540 - 598, The patch misses validating the sandbox image when preserveCompose is true: update writeOrPatchCompose to pass state.Sandbox into patchComposeImageRefs, change patchComposeImageRefs signature to accept a sandboxEnabled bool, and inside patchComposeImageRefs add "sandbox" to the list of required services (or conditionally check it) when sandboxEnabled is true so that the function fails if a sandbox image reference isn't patched; keep using imageLinePattern, digestPins and atomicWriteFile as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cli/cmd/update.go`:
- Around line 540-598: The patch misses validating the sandbox image when
preserveCompose is true: update writeOrPatchCompose to pass state.Sandbox into
patchComposeImageRefs, change patchComposeImageRefs signature to accept a
sandboxEnabled bool, and inside patchComposeImageRefs add "sandbox" to the list
of required services (or conditionally check it) when sandboxEnabled is true so
that the function fails if a sandbox image reference isn't patched; keep using
imageLinePattern, digestPins and atomicWriteFile as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 61bfccef-9fc7-4ad3-bc8f-1784553ff48d
📒 Files selected for processing (2)
cli/cmd/update.gocli/cmd/update_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use nativetesting.Ffuzz functions (Fuzz*) for fuzz testing Go code
Run Go lint viagolangci-lint run, vet viago vet, tests viago test ./..., and fuzz viago test -fuzz=FuzzTarget -fuzztime=30s
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in CLI code — use generic names:test-provider,test-small-001, etc.; vendor names only in third-party import paths
Files:
cli/cmd/update_test.gocli/cmd/update.go
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to cli/**/*.go : Run Go lint via `golangci-lint run`, vet via `go vet`, tests via `go test ./...`, and fuzz via `go test -fuzz=FuzzTarget -fuzztime=30s`
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to cli/**/*.go : Use native `testing.F` fuzz functions (`Fuzz*`) for fuzz testing Go code
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Pre-push hooks: mypy type-check + pytest unit tests + golangci-lint + go vet + go test (CLI, conditional on cli/**/*.go) (fast gate before push, skipped in pre-commit.ci — dedicated CI jobs already run these).
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Shell commands: for Go CLI work, cd cli is an exception because Go tooling requires working directory to be the module root. Go commands require `cd cli` for other work, never use `cd`.
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (1)
cli/cmd/update.go (6)
cli/internal/config/state.go (2)
Load(53-91)State(16-28)cli/internal/version/version.go (2)
Version(9-9)RepoURL(5-5)cli/internal/selfupdate/updater.go (3)
Check(67-69)Download(195-230)Replace(233-239)cli/internal/compose/generate.go (2)
ParamsFromState(49-62)Generate(66-85)cli/internal/docker/client.go (1)
Info(22-29)cli/internal/ui/ui.go (1)
UI(44-53)
🔇 Additional comments (13)
cli/cmd/update_test.go (7)
39-105: LGTM! Comprehensive table-driven tests forlineDiff.The tests cover key scenarios: identical inputs, added/removed/changed lines, and trailing newline handling. The structure with
wantContains,wantAbsent, andwantEmptyfields allows precise assertions without brittle exact-match comparisons.
107-116: LGTM! Fuzz test compliant with coding guidelines.The
FuzzLineDifffunction usestesting.Fwith representative seed inputs and verifies the function doesn't panic on arbitrary string pairs. As per coding guidelines: "Use nativetesting.Ffuzz functions (Fuzz*) for fuzz testing Go code".
118-168: LGTM! Good coverage of secret key patterns.Tests cover JWT_SECRET, TOKEN, PASSWORD, API_KEY, and CREDENTIALS variants with case-insensitive matching, matching the expanded
secretKeyPatternregex.
170-184: LGTM! Proper sentinel error identity testing.Tests verify
errReexecworks correctly witherrors.Isboth directly and when wrapped via%w.
198-228: LGTM! Robust permission test with environment bypass detection.The test now properly:
- Skips on Windows where mode bits aren't enforced
- Probes to skip when running as root/in containers that bypass mode bits
- Restores permissions before assertions to ensure cleanup succeeds
230-286: LGTM! ThoroughpatchComposeImageRefstest.Tests verify that:
- All three services get digest-pinned updates
- Non-image configuration lines are preserved
- The CLI version comment is preserved (important for preserveCompose mode)
288-358: LGTM! Good error case coverage forpatchComposeImageRefs.Tests cover:
- Tag fallback when digest pins are nil
- Error when no synthorg image refs are found
- Error when required services are missing
cli/cmd/update.go (6)
37-82: LGTM! Clear control flow with proper error handling.The refactored
runUpdatehas a well-structured flow:
- CLI update with re-exec on binary replacement
- Single config load (avoiding TOCTOU)
- Compose refresh with user approval
- Graceful handling when user declines compose changes (warning + secondary confirmation for image-only updates)
The
preserveComposeflag correctly propagates the user's decision through the image update path.
84-138: LGTM! Clean sentinel error pattern for re-exec signaling.Returning
errReexecto signal binary replacement is cleaner than returning a tuple or out-parameter. The caller can handle it specifically viaerrors.Is.
140-211: LGTM! Proper exit code propagation for re-exec.
ChildExitErrorandChildExitCodeprovide a clean way for the entrypoint to propagate the child's exit code. The symlink resolution inreexecUpdatematches the pattern inselfupdate.Replace, ensuring the correct binary is executed.
213-286: LGTM! Compose refresh with atomic write and user approval.The flow is correct:
loadAndGenerategracefully handles missing compose (returns nil, nil, nil)applyComposeDiffshows the diff and requires explicit approvalatomicWriteFileensures the write is atomic
288-348: LGTM! Secure diff output with comprehensive secret redaction.The
secretKeyPatterncovers common sensitive key naming conventions (SECRET, PASSWORD, TOKEN, API_KEY, CREDENTIALS) with case-insensitive matching. The bag-based diff approach is documented as intentional—reordered lines won't show as changes, which is acceptable for compose files where YAML key order typically doesn't matter semantically.
430-451: Clarify the auto-accept behavior for compose configuration changes in non-interactive mode.The default-yes behavior differs inconsistently across call sites. Line 66 conservatively uses a false default when container images might be incompatible with the current compose (a risky fallback scenario), yet line 272 auto-accepts compose configuration changes in non-interactive mode despite showing a diff first. Similarly, line 114 (CLI update) and line 414 (normal image update) both use true defaults. Confirm whether auto-accepting compose template changes in CI/automation environments (line 272) is intentional, given the risk of unexpected configuration modifications.
patchComposeImageRefs now accepts sandboxEnabled and validates that the sandbox image reference was patched when sandbox is active. Prevents config.Save from advancing VerifiedDigests for sandbox while the compose still references the old sandbox image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/update_test.go`:
- Around line 39-105: Add a symmetric "trailing newline removed" test case to
TestLineDiff to mirror the existing "trailing newline added" case: in the tests
slice inside TestLineDiff add an entry where old has a trailing newline (e.g.,
"line1\nline2\n"), updated lacks it ("line1\nline2"), and assert that the diff
from lineDiff(old, updated) contains the expected removal marker (e.g., "- " or
"- <line>") similar to the other cases; place this new case alongside the other
test entries so it runs with the table-driven tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6f06241-32ff-4d54-841a-204f70a31215
📒 Files selected for processing (2)
cli/cmd/update.gocli/cmd/update_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use Cobra framework for CLI commands in the Go CLI binary
Use charmbracelet/lipgloss for styled CLI output and charmbracelet/huh for interactive forms in the Go CLI
Run golangci-lint, go vet, and go test for linting and testing all Go CLI code
Files:
cli/cmd/update_test.gocli/cmd/update.go
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to cli/**/*.go : Run golangci-lint, go vet, and go test for linting and testing all Go CLI code
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/update_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Pre-push hooks: mypy type-check + pytest unit tests + golangci-lint + go vet + go test (CLI, conditional on cli/**/*.go) (fast gate before push, skipped in pre-commit.ci — dedicated CI jobs already run these).
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to cli/**/*.go : Use Cobra framework for CLI commands in the Go CLI binary
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Shell commands: for Go CLI work, cd cli is an exception because Go tooling requires working directory to be the module root. Go commands require `cd cli` for other work, never use `cd`.
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (1)
cli/cmd/update_test.go (1)
cli/internal/config/state.go (1)
State(16-28)
🔇 Additional comments (23)
cli/cmd/update_test.go (10)
3-13: LGTM!Imports are appropriate for the test coverage being added. The standard library packages and internal config import are correctly scoped for the tests in this file.
107-116: LGTM!Fuzz test properly validates that
lineDiffdoesn't panic on arbitrary inputs. The seed corpus covers relevant edge cases.
118-168: LGTM!Comprehensive test coverage for
redactSecretcovering all secret key patterns including case insensitivity. The test cases align with thesecretKeyPatternregex inupdate.go.
170-184: LGTM!Proper verification of sentinel error identity and wrapping behavior. This ensures the re-exec flow correctly detects
errReexeceven when wrapped.
186-196: LGTM!Correctly tests the "no existing compose" edge case. Uses
t.TempDir()for automatic cleanup.
198-228: LGTM!Well-structured permission error test with appropriate guards:
- Windows skip for reliability
- Root/container environment detection to avoid false failures
- Permission restoration before assertions per past feedback
230-286: LGTM!Comprehensive test of
patchComposeImageRefswith digest pins. Validates image reference updates while preserving non-image configuration and the CLI version comment.
288-317: LGTM!Correctly tests the tag-based fallback path when digest pins are unavailable. The
sandboxEnabled=falsematches the compose content having only backend and web services.
319-337: LGTM!Validates the error case when no SynthOrg image references are found, ensuring users get clear guidance to run
synthorg init.
339-358: LGTM!Validates that missing required services (web in this case) are detected and reported with actionable error messaging.
cli/cmd/update.go (13)
3-25: LGTM!Imports align with coding guidelines: Cobra for CLI commands and charmbracelet/huh for interactive forms.
37-82: LGTM!Well-structured update flow:
- Handles
errReexecsentinel correctly for CLI self-update- Loads config once to avoid TOCTOU gaps
- Provides clear user guidance when compose changes are declined
- Past feedback about compose-approval bypass has been addressed
84-138: LGTM!Clean implementation of the CLI self-update with
errReexecsentinel pattern. The flow handles all cases (up-to-date, declined, error, success) appropriately.
140-159: LGTM!
ChildExitErrorandChildExitCodeproperly enable exit code propagation from the re-exec'd child process, addressing CI exit code fidelity concerns from past feedback.
161-211: LGTM!Robust re-exec implementation:
- Resolves symlinks to match
selfupdate.Replacebehavior- Reconstructs args from known flags for safety
- Warns about
--skip-verifypropagation- Properly extracts and wraps child exit codes
262-286: LGTM!Clean implementation of the compose diff approval flow with atomic writes.
288-348: LGTM!Secret redaction covers common sensitive key patterns. The bag-based diff is appropriate for compose files where line order is semantically irrelevant, and the limitation is clearly documented.
387-429: LGTM!Image update flow correctly threads the
preserveComposeflag through topullAndPersist.
431-451: LGTM!Confirmation helpers follow coding guidelines by using
charmbracelet/huhfor interactive forms, with proper non-interactive mode handling.
453-538: LGTM!Robust implementation with proper rollback on failure and clear error handling. The air-gapped environment hint is helpful UX.
540-602: LGTM!
patchComposeImageRefscorrectly validates that all required services were patched before succeeding, preventing config state advancement when compose is unpatched. Clear error messages guide users to regenerate viasynthorg init.
604-665: LGTM!Restart logic handles both interactive and non-interactive modes appropriately, with reasonable health check timeout and error handling.
242-260: No action needed — path is properly sanitized.The
composePathderivation is safe.safeStateDir()callsconfig.SecurePath(state.DataDir), which usesfilepath.Clean()to normalize the path andfilepath.IsAbs()to enforce absolute paths, rejecting relative paths and traversal attempts like../etc/passwd. The subsequentfilepath.Join(safeDir, "compose.yml")is secure.
Symmetric with the existing "trailing newline added" case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These Docker container log files were picked up by git add -A. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/update_test.go`:
- Around line 124-174: Extend TestRedactSecret to add targeted edge-case
table-driven tests for the redactSecret function: include cases for empty values
and missing quotes, values with single quotes, keys appearing as substrings
(e.g., NOT_A_SECRET_KEY), lines with extra whitespace or tabs, inline comments
after the value, multiple colons or YAML-style mappings, and mixed casing and
punctuation; keep tests in the same TestRedactSecret harness and add distinct
names for each case so redactSecret behavior is validated across these edge
conditions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9af53ce9-bad4-49b7-ac2a-4a614a61b4d0
📒 Files selected for processing (1)
cli/cmd/update_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use Cobra framework for CLI commands in the Go CLI binary
Use charmbracelet/lipgloss for styled CLI output and charmbracelet/huh for interactive forms in the Go CLI
Run golangci-lint, go vet, and go test for linting and testing all Go CLI code
Files:
cli/cmd/update_test.go
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to cli/**/*.{sh,ps1} : Install scripts (install.sh, install.ps1) should provide cross-platform CLI installation with automated self-update capability
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to cli/**/*.go : Run golangci-lint, go vet, and go test for linting and testing all Go CLI code
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/update_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Pre-push hooks: mypy type-check + pytest unit tests + golangci-lint + go vet + go test (CLI, conditional on cli/**/*.go) (fast gate before push, skipped in pre-commit.ci — dedicated CI jobs already run these).
Applied to files:
cli/cmd/update_test.go
🔇 Additional comments (10)
cli/cmd/update_test.go (10)
3-13: LGTM!Clean import organization with only standard library and internal project packages.
39-111: LGTM!Well-structured table-driven tests with good coverage including edge cases for trailing newlines. The addition of the "trailing newline removed" case provides symmetrical coverage.
113-122: LGTM!Good use of fuzzing to ensure
lineDiffdoesn't panic on arbitrary input combinations.
176-190: LGTM!Thorough verification of sentinel error identity semantics including the wrapped case with
%w.
192-202: LGTM!Clean test verifying graceful handling when
compose.ymldoesn't exist.
204-234: LGTM!The permission test properly handles all edge cases: Windows skip, root/container environment detection with pre-test probe, and defensive permission restoration before assertions. Past review feedback has been well incorporated.
236-292: LGTM!Comprehensive test validating that
patchComposeImageRefsupdates image references while preserving non-image configuration and the original CLI version comment. This correctly tests the compose-preservation behavior for users who decline regeneration.
294-323: LGTM!Good coverage of the tag-based fallback path when digest pins are not provided.
325-343: LGTM!Good error case coverage ensuring the function fails gracefully for compose files without SynthOrg image references.
345-364: LGTM!Good validation test ensuring that required services are present before patching proceeds.
| func TestRedactSecret(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| line string | ||
| want string | ||
| }{ | ||
| { | ||
| name: "jwt secret redacted", | ||
| line: ` SYNTHORG_JWT_SECRET: "supersecret123"`, | ||
| want: ` SYNTHORG_JWT_SECRET: [REDACTED]`, | ||
| }, | ||
| { | ||
| name: "non-secret line unchanged", | ||
| line: ` SYNTHORG_LOG_DIR: "/data/logs"`, | ||
| want: ` SYNTHORG_LOG_DIR: "/data/logs"`, | ||
| }, | ||
| { | ||
| name: "case insensitive match", | ||
| line: ` synthorg_jwt_secret: "abc"`, | ||
| want: ` synthorg_jwt_secret: [REDACTED]`, | ||
| }, | ||
| { | ||
| name: "token key redacted", | ||
| line: ` AUTH_TOKEN: "mytoken"`, | ||
| want: ` AUTH_TOKEN: [REDACTED]`, | ||
| }, | ||
| { | ||
| name: "password key redacted", | ||
| line: ` DB_PASSWORD: "hunter2"`, | ||
| want: ` DB_PASSWORD: [REDACTED]`, | ||
| }, | ||
| { | ||
| name: "api key redacted", | ||
| line: ` EXTERNAL_API_KEY: "key123"`, | ||
| want: ` EXTERNAL_API_KEY: [REDACTED]`, | ||
| }, | ||
| { | ||
| name: "credentials key redacted", | ||
| line: ` SERVICE_CREDENTIALS: "creds"`, | ||
| want: ` SERVICE_CREDENTIALS: [REDACTED]`, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got := redactSecret(tt.line) | ||
| if got != tt.want { | ||
| t.Errorf("redactSecret(%q) = %q, want %q", tt.line, got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good coverage of secret redaction patterns.
The tests cover the main secret key patterns (JWT_SECRET, TOKEN, PASSWORD, API_KEY, CREDENTIALS) and verify case-insensitivity.
Consider adding edge case tests for robustness:
♻️ Optional: Additional edge cases
{
name: "credentials key redacted",
line: ` SERVICE_CREDENTIALS: "creds"`,
want: ` SERVICE_CREDENTIALS: [REDACTED]`,
},
+ {
+ name: "empty line unchanged",
+ line: "",
+ want: "",
+ },
+ {
+ name: "line without colon unchanged",
+ line: "# comment mentioning SECRET",
+ want: "# comment mentioning SECRET",
+ },
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/cmd/update_test.go` around lines 124 - 174, Extend TestRedactSecret to
add targeted edge-case table-driven tests for the redactSecret function: include
cases for empty values and missing quotes, values with single quotes, keys
appearing as substrings (e.g., NOT_A_SECRET_KEY), lines with extra whitespace or
tabs, inline comments after the value, multiple colons or YAML-style mappings,
and mixed casing and punctuation; keep tests in the same TestRedactSecret
harness and add distinct names for each case so redactSecret behavior is
validated across these edge conditions.
Add 9 edge-case tests: empty value, single quotes, keyword substring, tab indent, inline comments, multiple colons, mixed case, no indent, and non-secret with colon in value. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **cli:** auto-delete binary on Windows, prune images, fix GoReleaser ([#590](#590)) ([eb7c691](eb7c691)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) ### Maintenance * **main:** release 0.3.7 ([#583](#583)) ([bf58779](bf58779)) * reset failed v0.3.7 release ([#591](#591)) ([b69000d](b69000d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added engine Hybrid Plan + ReAct execution loop * Added first-run setup wizard * **Bug Fixes** * Addressed ZAP DAST scan issues * Fixed CLI Windows/image/update issues * **Maintenance** * Added SBOM generation for Docker/CLI releases * General maintenance updates <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **ci:** reset failed v0.3.7 release and fix syft SBOM scan ([#593](#593)) ([d1508c2](d1508c2)) * **cli:** auto-delete binary on Windows, prune images, fix GoReleaser ([#590](#590)) ([eb7c691](eb7c691)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **ci:** remove CLI SBOM generation, reset failed v0.3.7 ([#595](#595)) ([d0f4992](d0f4992)) * **ci:** reset failed v0.3.7 release and fix syft SBOM scan ([#593](#593)) ([d1508c2](d1508c2)) * **cli:** auto-delete binary on Windows, prune images, fix GoReleaser ([#590](#590)) ([eb7c691](eb7c691)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
synthorg updatenow re-execs the new binary after CLI replacement so compose refresh and image pull use the new embedded templaterefreshComposestep diffs on-disk compose.yml against the current template, shows changes, and asks the user to approve before writingSYNTHORG_LOG_DIRin compose because the template changed but compose was never regenerated)Changes
cli/cmd/update.go: AddreexecUpdate(re-exec after binary replacement with symlink resolution),refreshCompose(template diff + approval),lineDiff(bag-based diff with secret redaction),redactSecrethelper. RefactorupdateCLIto returnerroronly (was(string, error)with unused string). ExtractloadAndGenerate+applyComposeDiffto keep functions under 50 lines. RemovecliVersionparameter threading (now read fromversion.VersionviaParamsFromState).cli/cmd/start.go: RemovecliVersionparameter fromwriteDigestPinnedCompose(usesParamsFromStatewhich already readsversion.Version). Drop unusedversionimport.cli/cmd/update_test.go: Table-drivenTestLineDiff(6 cases including trailing newline),TestRedactSecret(3 cases),TestErrReexec_Identity(errors.Is contract).docs/design/operations.md: Update CLI tool description to mention re-exec and compose refresh.docs/user_guide.md: Document compose template refresh behavior during update.Test plan
go build ./...cleango vet ./...cleangolangci-lint runcleango test ./...all passReview coverage
Pre-reviewed by 4 agents (go-reviewer, go-security-reviewer, go-conventions-enforcer, docs-consistency), 15 findings addressed:
ErrNotExist-only error handling instead of swallowing all ReadFile errorsoldSetallocation, unused string return)new->updatedto avoid shadowing built-in)🤖 Generated with Claude Code