fix: CLI improvements — config show, completion install, enhanced doctor, Sigstore verification#465
fix: CLI improvements — config show, completion install, enhanced doctor, Sigstore verification#465
Conversation
…hosts GitHub started routing release asset downloads through this domain, causing `synthorg update` to reject the redirect as an unexpected host.
Displays current CLI configuration with masked JWT secret. Warns if not initialized and suggests running synthorg init.
Enhances diagnostics with five new checks: - Compose file existence and validity - Port conflict detection (skipped when containers are running) - Local Docker image availability - Container detail parsing from compose ps JSON - Docker/Compose minimum version warnings
…etup Detects the user's shell and installs tab-completion for synthorg. Supports bash, zsh, fish, and powershell. Idempotent — safe to run multiple times. Leaves Cobra's built-in completion command untouched.
Replaces fsutil/df subprocess calls with direct syscall usage: - Windows: GetDiskFreeSpaceExW (no admin required) - Unix: syscall.Statfs Fixes disk info showing "unavailable" on non-admin Windows.
Pre-reviewed by 5 agents, 15 findings addressed: - Fix non-idiomatic iota usage in ShellType constants - Fix appendToFile to check f.Close() error - Replace custom contains helper with strings.Contains - Validate PowerShell profile path (absolute, clean, length) - Fix StatePath called twice with potentially different inputs - Split Collect and FormatText into sub-50-line functions - Update CLAUDE.md: add completion/ package, config and completion-install commands, expand diagnostics description - Add Sigstore bundle verification to self-update using sigstore-go (verifies checksums.txt signature against GitHub Actions OIDC identity via Sigstore public good transparency log)
Dependency ReviewThe following issues were found:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds new CLI commands ( Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (self-update)"
participant Releases as "GitHub Releases"
participant Fetcher as "HTTP/TUF Fetcher"
participant Verifier as "Sigstore Verifier"
CLI->>Releases: request assetURL, checksumURL, optional bundleURL
Releases-->>CLI: assetURL + checksumURL (+ bundleURL)
CLI->>Fetcher: download checksum (checksumURL)
Fetcher-->>CLI: checksumData
alt bundleURL provided
CLI->>Fetcher: download bundle (bundleURL)
Fetcher-->>CLI: bundleData
CLI->>Verifier: verify(bundleData, checksumData)
Verifier-->>CLI: ok / error
end
alt verified or no bundle
CLI->>Fetcher: download asset (assetURL)
Fetcher-->>CLI: assetArchive
CLI->>CLI: extract & replace binary
end
CLI-->>User: report success or failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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 upgrades the CLI by introducing new commands for configuration management and streamlined shell completion installation. It also dramatically improves the diagnostic capabilities of the Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to the CLI, including new config show and completion-install commands, an enhanced doctor command with more diagnostics, and importantly, Sigstore verification for self-updates. The code is well-structured and includes tests for most of the new functionality. My review focuses on improving robustness in error handling, ensuring code comments are accurate, and highlighting the need for testing on the new security-critical Sigstore verification feature.
| // Sigstore bundle verification (optional but recommended). | ||
| if bundleURL != "" { | ||
| bundleData, err := httpGetWithClient(ctx, client, bundleURL, maxAPIResponseBytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("downloading sigstore bundle: %w", err) | ||
| } | ||
| if err := verifySigstoreBundle(checksumData, bundleData); err != nil { | ||
| return nil, fmt.Errorf("sigstore verification failed: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
This new Sigstore verification logic is a critical security feature, but it doesn't appear to be covered by any tests. The related sigstore.go file also lacks tests. It's important to add unit and/or integration tests to verify the signature validation process, including success and failure scenarios (e.g., invalid signature, wrong issuer, mismatched artifact digest) to ensure its correctness and robustness.
cli/internal/completion/install.go
Outdated
| if _, err := f.WriteString(content); err != nil { | ||
| _ = f.Close() | ||
| return fmt.Errorf("writing to %s: %w", path, err) | ||
| } | ||
| return f.Close() |
There was a problem hiding this comment.
The error from f.Close() on line 260 is ignored when a write error occurs. While the write error is the primary one, it's more robust to handle the close error as well, as it might indicate that the write was not fully flushed to disk. The current implementation also returns the f.Close() error on the happy path without wrapping it, which could lose context. Consider refactoring to ensure both write and close errors are checked and properly wrapped.
_, writeErr := f.WriteString(content)
closeErr := f.Close()
if writeErr != nil {
return fmt.Errorf("writing to %s: %w", path, writeErr)
}
if closeErr != nil {
return fmt.Errorf("closing %s: %w", path, closeErr)
}
return nil
cli/internal/diagnostics/collect.go
Outdated
| // diskInfoNative returns disk usage for the given path using native Go syscalls. | ||
| // Platform-specific implementations are in disk_unix.go and disk_windows.go. |
There was a problem hiding this comment.
The comment on line 320 refers to the function as diskInfoNative, but the function is named diskInfo. This discrepancy could be confusing for future maintainers. Please update the comment to match the function name.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/config.go`:
- Around line 35-43: runConfigShow currently calls resolveDataDir() and probes
config.StatePath(dir) before validating the directory, which can misreport
invalid --data-dir as "Not initialized"; instead validate the resolved data-dir
using the same validation used by config.Load (e.g., call config.SecurePath or
the canonical validation function with resolveDataDir() and return any error),
then build statePath from the validated safeDir via config.StatePath(safeDir)
and proceed to os.Stat; update runConfigShow to return the validation error when
present and only check for state file after successful validation.
In `@cli/internal/completion/install.go`:
- Around line 127-129: The current installZsh and installFish functions treat an
existing completion file (compFile) as "already installed" and return early;
change this so the presence check only sets res.AlreadyInstalled = true but does
not return — always regenerate and overwrite the completion file contents (use
os.WriteFile or equivalent with correct permissions) so users get updated
commands on rerun; keep the shell-init idempotent logic that updates ~/.zshrc or
fish config (the fpath/stanza checks) but do not skip writing the generated
completion file when it already exists. Ensure error handling/logging remains
for the write operations in installZsh and installFish (and the analogous
branches referenced around the other checks).
In `@cli/internal/diagnostics/collect.go`:
- Around line 236-247: checkComposeFile currently only checks for "compose.yml"
so projects using other default Compose filenames are treated as missing; update
checkComposeFile to look for the full default set
["compose.yml","compose.yaml","docker-compose.yml","docker-compose.yaml"], pick
the first existing path (set composePath to that), and only if none exist return
(false,false). Then run docker.ComposeExec with the discovered composePath (or
ensure the command is invoked in dataDir but referencing the discovered
filename) and return (true, err==nil) from checkComposeFile.
- Around line 291-307: The parseContainerDetails function currently treats
psJSON as NDJSON and splits on newlines; instead, parse the entire psJSON string
as a single JSON array into []ContainerDetail (i.e.,
json.Unmarshal([]byte(psJSON), &details)). Update parseContainerDetails to
unmarshal into details directly and return it; optionally keep a fallback path
to handle legacy NDJSON by falling back to line-by-line unmarshalling only if
the array unmarshal fails, ensuring ContainerDetail and callers like
hasRunningContainers receive the populated slice.
In `@cli/internal/diagnostics/disk_windows.go`:
- Around line 12-13: The code currently creates kernel32 :=
syscall.NewLazyDLL("kernel32.dll") and getDiskFreeSpaceExW :=
kernel32.NewProc("GetDiskFreeSpaceExW") on each call; move those to
package-level variables (e.g., var kernel32 = syscall.NewLazyDLL("kernel32.dll")
and var getDiskFreeSpaceExW = kernel32.NewProc("GetDiskFreeSpaceExW")) or
initialize them once in an init() function, then update the function that
currently declares the local kernel32/getDiskFreeSpaceExW to use the
package-level symbols instead; ensure any existing error handling still works if
the proc is nil or fails to find the symbol.
In `@cli/internal/selfupdate/sigstore.go`:
- Around line 32-36: root.FetchTrustedRoot() blocks without timeout; replace the
direct call with a Sigstore/TUF fetch using tuf.Options.WithFetcher(...) that
supplies a custom HTTP fetcher backed by an http.Client with a timeout;
implement a fetcher function (used by tuf.Options.WithFetcher) that performs the
same retrieval as FetchTrustedRoot but via HTTP requests with a configured
Timeout (or context-aware requests), and use that fetcher when constructing the
trusted root so network calls are bounded.
In `@cli/internal/selfupdate/updater_test.go`:
- Line 545: Add tests in updater_test.go that cover the bundleURL != "" path of
Download by exercising error scenarios: create httptest servers that when
Download is called with a non-empty bundleURL simulate (1) a bundle download
failure (return HTTP 500 or close connection) to assert Download returns an
error, and (2) a malformed/invalid JSON bundle response so that
verifySigstoreBundle (called from Download) fails and Download returns that
error; use the existing test harness that calls Download(ctx, assetURL,
checksumsURL, bundleURL) and assert errors are propagated from
Download/verifySigstoreBundle.
In `@cli/internal/selfupdate/updater.go`:
- Around line 176-179: The branch handling the "checksums.txt.sigstore.json"
asset silently ignores a.BrowserDownloadURL when it doesn't start with
expectedURLPrefix; change this to emit a debug-level log entry (using the
updater's existing logger) that includes the actual a.BrowserDownloadURL and the
expectedURLPrefix so callers can diagnose why the Sigstore bundle URL was
skipped, while leaving bundleURL unchanged. Target the switch case that matches
"checksums.txt.sigstore.json" and add the log on the else path where
strings.HasPrefix(a.BrowserDownloadURL, expectedURLPrefix) is false.
🪄 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: 2cd08054-40f5-4ac1-9d3a-e06dcf8a4e1d
⛔ Files ignored due to path filters (1)
cli/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
CLAUDE.mdcli/cmd/completion_install.gocli/cmd/config.gocli/cmd/config_test.gocli/cmd/update.gocli/go.modcli/internal/completion/install.gocli/internal/completion/install_test.gocli/internal/diagnostics/collect.gocli/internal/diagnostics/collect_test.gocli/internal/diagnostics/disk_unix.gocli/internal/diagnostics/disk_windows.gocli/internal/selfupdate/sigstore.gocli/internal/selfupdate/updater.gocli/internal/selfupdate/updater_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 (3)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use native Go fuzz testing: testing.F fuzz functions (Fuzz*). Fuzz targets are in testdata/.
Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Note: Go commands requirecd clibecause the Go module is incli/(exception to the "never use cd" rule—Go tooling requires working directory to be the module root).
Files:
cli/internal/diagnostics/disk_windows.gocli/cmd/config_test.gocli/internal/selfupdate/updater_test.gocli/cmd/completion_install.gocli/internal/diagnostics/collect_test.gocli/internal/diagnostics/disk_unix.gocli/internal/selfupdate/sigstore.gocli/internal/selfupdate/updater.gocli/cmd/config.gocli/internal/completion/install_test.gocli/cmd/update.gocli/internal/diagnostics/collect.gocli/internal/completion/install.go
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue. The design spec in docs/design/ is the starting point for architecture, data models, and behavior. When a spec topic is referenced, read the relevant docs/design/ page before coding.
Files:
CLAUDE.md
cli/go.mod
📄 CodeRabbit inference engine (CLAUDE.md)
Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Files:
cli/go.mod
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
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).
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-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/cmd/config_test.goCLAUDE.mdcli/internal/selfupdate/sigstore.gocli/internal/selfupdate/updater.gocli/cmd/update.gocli/go.mod
📚 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:
CLAUDE.mdcli/cmd/update.gocli/internal/completion/install.gocli/go.mod
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
CLAUDE.mdcli/go.mod
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
CLAUDE.mdcli/internal/completion/install.gocli/go.mod
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Note: Go commands require `cd cli` because the Go module is in `cli/` (exception to the "never use cd" rule—Go tooling requires working directory to be the module root).
Applied to files:
CLAUDE.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:
CLAUDE.md
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to docker/** : All Docker files (Dockerfiles, compose, .env.example) are in docker/. Backend: 3-stage build (builder → setup → distroless), Chainguard Python, non-root UID 65532. Web: nginxinc/nginx-unprivileged, Vue 3 SPA, API/WebSocket proxy. Sandbox: Python 3.14 + Node.js + git, non-root UID 10001.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (7)
cli/internal/selfupdate/updater_test.go (1)
cli/internal/selfupdate/updater.go (1)
Download(195-230)
cli/cmd/completion_install.go (1)
cli/internal/completion/install.go (3)
DetectShell(49-68)Unknown(22-22)Install(78-93)
cli/internal/diagnostics/collect_test.go (1)
cli/internal/diagnostics/collect.go (2)
ContainerDetail(23-28)Report(31-52)
cli/cmd/config.go (3)
cli/internal/ui/ui.go (1)
NewUI(57-69)cli/internal/config/state.go (2)
StatePath(40-42)Load(46-84)cli/internal/config/paths.go (1)
DataDir(18-31)
cli/internal/completion/install_test.go (1)
cli/internal/completion/install.go (8)
Unknown(22-22)PowerShell(26-26)ShellType(19-19)Bash(23-23)Zsh(24-24)Fish(25-25)DetectShell(49-68)Install(78-93)
cli/cmd/update.go (1)
cli/internal/selfupdate/updater.go (1)
Download(195-230)
cli/internal/diagnostics/collect.go (2)
cli/internal/config/state.go (1)
State(14-23)cli/internal/docker/client.go (6)
Info(22-29)Detect(34-66)CheckMinVersions(69-78)ComposeExecOutput(100-107)ComposeExec(91-97)RunCmd(110-119)
🪛 GitHub Check: CodeQL
cli/cmd/config.go
[failure] 40-40: 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.
cli/internal/diagnostics/collect.go
[failure] 238-238: 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.
cli/internal/completion/install.go
[failure] 127-127: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 132-132: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 140-140: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 171-171: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 176-176: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 184-184: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 239-239: 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.
[failure] 252-252: 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.
[failure] 255-255: 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.
🔇 Additional comments (12)
cli/internal/diagnostics/disk_unix.go (1)
1-16: LGTM!The Unix disk usage implementation is correct:
- Uses
Bavail(blocks available to unprivileged users) rather thanBfree, which is the appropriate choice for user-facing disk space reporting.- Proper build tag for non-Windows platforms.
- Error handling is straightforward and correct.
cli/internal/diagnostics/disk_windows.go (1)
1-31: LGTM overall — Windows implementation is correct.The implementation correctly:
- Uses
freeBytesAvailablewhich respects disk quotas (matchingBavailon Unix).- Properly checks the return value (non-zero = success).
- Handles UTF-16 conversion for Windows paths.
cli/go.mod (2)
5-10: Direct dependencies align with project requirements.The direct dependencies match the coding guidelines: Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output, and the new sigstore-go for Sigstore verification as described in the PR objectives.
8-8: sigstore-go v1.1.4 is current and secure.v1.1.4 is the latest release (Dec 10, 2025) with no known vulnerabilities. The sole published CVE for sigstore-go (CVE-2024-45395) affects only versions ≤ 0.6.0 and is already patched in v0.6.1 and all later versions, including v1.1.4.
CLAUDE.md (1)
147-157: LGTM!Documentation updates accurately reflect the new CLI features:
configandcompletion-installcommands added to the cmd list- Diagnostics expanded to include compose validation, port conflicts, image checks, and native disk usage
- New
completion/package for shell completion auto-installcli/internal/selfupdate/sigstore.go (2)
13-18: LGTM — Identity constraints are appropriately scoped.The expected issuer and SAN regex correctly constrain signing to:
- GitHub Actions OIDC issuer only
- Workflows from the
Aureliolo/synthorgrepositoryThe regex is anchored at the start (
^) which allows any workflow path within the repo, which is appropriate for release workflows.
71-78: LGTM — Bundle parsing helper is correct.The helper properly initializes the nested protobuf structure before unmarshaling.
cli/internal/selfupdate/updater.go (3)
217-226: LGTM — Sigstore verification is correctly positioned.The verification flow is secure:
- Download and verify checksum first (integrity check)
- Then verify Sigstore bundle (authenticity/provenance check)
- Only extract binary after both pass
This ordering is correct — checksum verification is a prerequisite for Sigstore verification since the bundle signs the checksums file.
324-329: LGTM — AllowedDownloadHosts expansion is correct.Adding
release-assets.githubusercontent.comaccommodates GitHub's CDN routing for release assets, as mentioned in the PR objectives.
56-63: LGTM — CheckResult struct update is well-documented.The new
SigstoreBundURLfield is clearly marked as optional via the comment, maintaining backward compatibility.cli/cmd/update.go (1)
74-74: LGTM!The update command correctly passes the Sigstore bundle URL to the Download function, enabling optional cryptographic verification of release artifacts.
cli/internal/selfupdate/updater_test.go (1)
571-571: LGTM — Test signature updates are consistent.All
Downloadcalls in tests have been updated to include the emptybundleURLparameter, maintaining test functionality for the checksum verification path.Also applies to: 713-713
| case "checksums.txt.sigstore.json": | ||
| if strings.HasPrefix(a.BrowserDownloadURL, expectedURLPrefix) { | ||
| bundleURL = a.BrowserDownloadURL | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider logging when Sigstore bundle URL is skipped due to prefix mismatch.
When checksums.txt.sigstore.json exists but doesn't match the expected URL prefix, it's silently ignored. This differs from asset/checksum URL handling which returns an error on prefix mismatch.
While acceptable for optional verification, a debug log could help diagnose unexpected behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/internal/selfupdate/updater.go` around lines 176 - 179, The branch
handling the "checksums.txt.sigstore.json" asset silently ignores
a.BrowserDownloadURL when it doesn't start with expectedURLPrefix; change this
to emit a debug-level log entry (using the updater's existing logger) that
includes the actual a.BrowserDownloadURL and the expectedURLPrefix so callers
can diagnose why the Sigstore bundle URL was skipped, while leaving bundleURL
unchanged. Target the switch case that matches "checksums.txt.sigstore.json" and
add the log on the else path where strings.HasPrefix(a.BrowserDownloadURL,
expectedURLPrefix) is false.
GO-2026-4354 and GO-2026-4355 in rekor@v1.4.3 (transitive dependency from sigstore-go) are fixed in rekor@v1.5.0.
- Validate data-dir via SecurePath before probing config file - Always regenerate completion files on rerun (pick up new commands) - Wrap f.Close() error with context in appendToFile - Fix diskInfoNative → diskInfo comment mismatch - Check all compose file variants (compose.yml/yaml, docker-compose.yml/yaml) - Parse container details as JSON array first, NDJSON fallback - Move Windows kernel32 DLL loading to package-level init - Add TUF fetch timeout (30s) to Sigstore trusted root retrieval - Add tests for Download with invalid/failing bundle URL - Extract testArchive helper to reduce test duplication
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/config.go`:
- Around line 45-49: The os.Stat check around statePath only handles
os.ErrNotExist and otherwise lets other errors fall through; change the logic in
the block that calls os.Stat so that if err != nil you distinguish
errors.Is(err, os.ErrNotExist) (keep the out.Warn/out.Hint and return nil) and
for any other err immediately return that error (wrap with context if desired)
instead of continuing to config.Load; refer to the os.Stat call, statePath
variable, and the out.Warn/out.Hint calls to locate where to add the early
return for non-ErrNotExist errors.
- Around line 14-28: The config and config show commands currently accept
arbitrary positional args because Cobra's default Arg validator is
cobra.ArbitraryArgs; update the command definitions for configCmd and
configShowCmd to explicitly set Args: cobra.NoArgs so unexpected positional
arguments are rejected (keep RunE: runConfigShow unchanged) and ensure both
configCmd and configShowCmd include the Args field.
In `@cli/internal/completion/install.go`:
- Line 95: The installBash function signature includes an unused rootCmd
parameter; remove that parameter from installBash and update its signature to
just func installBash(res Result) (Result, error), then update any call sites
that currently pass rootCmd (e.g. where installBash is invoked alongside
installPowerShell) to call installBash(res) instead; ensure the function body is
adjusted to reference only res and that installPowerShell remains unchanged for
consistency.
In `@cli/internal/diagnostics/collect.go`:
- Around line 299-323: parseContainerDetails currently filters out entries with
empty Name only in the NDJSON fallback but returns the unmarshaled JSON array
as-is, which can pass empty-named ContainerDetail entries downstream; update
parseContainerDetails (function name) to enforce the same Name != "" filtering
for the JSON array branch (ContainerDetail entries) before returning so both
array and NDJSON paths produce only entries with non-empty Name (used later to
build ContainerSummary).
In `@cli/internal/selfupdate/sigstore.go`:
- Around line 20-23: The SAN regex defined by expectedSANRegex is too broad (it
only pins the repo) and thus accepts any workflow; update expectedSANRegex in
sigstore.go to tightly match the full GitHub Actions workflow identity including
the workflow file and refs segment (e.g. include
".github/workflows/cli.yml@refs/" and, if applicable, the specific tag pattern
like "refs/tags/v[0-9]+"), so the matcher only accepts signatures issued by the
intended workflow that publishes releases.
🪄 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: 0c54938d-bca3-41e3-b6d9-c27543f36e3a
⛔ Files ignored due to path filters (1)
cli/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
cli/cmd/config.gocli/go.modcli/internal/completion/install.gocli/internal/diagnostics/collect.gocli/internal/diagnostics/disk_windows.gocli/internal/selfupdate/sigstore.gocli/internal/selfupdate/updater_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 (2)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use native Go fuzz testing: testing.F fuzz functions (Fuzz*). Fuzz targets are in testdata/.
Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Note: Go commands requirecd clibecause the Go module is incli/(exception to the "never use cd" rule—Go tooling requires working directory to be the module root).
Files:
cli/internal/selfupdate/updater_test.gocli/cmd/config.gocli/internal/completion/install.gocli/internal/selfupdate/sigstore.gocli/internal/diagnostics/disk_windows.gocli/internal/diagnostics/collect.go
cli/go.mod
📄 CodeRabbit inference engine (CLAUDE.md)
Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Files:
cli/go.mod
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
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-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/config.gocli/internal/completion/install.gocli/go.mod
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
cli/cmd/config.gocli/internal/selfupdate/sigstore.gocli/go.mod
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
cli/cmd/config.gocli/internal/completion/install.gocli/go.mod
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Note: Go commands require `cd cli` because the Go module is in `cli/` (exception to the "never use cd" rule—Go tooling requires working directory to be the module root).
Applied to files:
cli/cmd/config.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/internal/selfupdate/sigstore.gocli/go.mod
🧬 Code graph analysis (3)
cli/internal/selfupdate/updater_test.go (1)
cli/internal/selfupdate/updater.go (1)
Download(195-230)
cli/cmd/config.go (2)
cli/internal/config/paths.go (2)
SecurePath(58-64)DataDir(18-31)cli/internal/config/state.go (2)
StatePath(40-42)Load(46-84)
cli/internal/diagnostics/collect.go (2)
cli/internal/config/state.go (1)
State(14-23)cli/internal/docker/client.go (6)
Info(22-29)Detect(34-66)CheckMinVersions(69-78)ComposeExecOutput(100-107)ComposeExec(91-97)RunCmd(110-119)
🪛 GitHub Check: CodeQL
cli/cmd/config.go
[failure] 45-45: 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.
cli/internal/completion/install.go
[failure] 127-127: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 132-132: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 139-139: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 170-170: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 175-175: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 182-182: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 240-240: 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.
[failure] 253-253: 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.
[failure] 256-256: 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.
cli/internal/diagnostics/collect.go
[failure] 245-245: 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 (19)
cli/internal/diagnostics/disk_windows.go (2)
10-13: LGTM! DLL loading now cached at package level.The kernel32 DLL and
GetDiskFreeSpaceExWproc are now initialized once at package level, eliminating the per-call overhead. This properly addresses the previous review feedback.
16-32: Implementation looks correct.The syscall usage follows standard Windows API patterns—UTF-16 path conversion, pointer passing, and checking return value for success (non-zero).
cli/internal/diagnostics/collect.go (7)
22-51: LGTM! Well-structured diagnostic types.The
ContainerDetailtype cleanly maps to compose ps JSON output, and theReportextensions (ComposeFileExists,PortConflicts, etc.) provide rich diagnostic data with appropriateomitemptytags for optional fields.
54-103: Clean refactoring with proper path validation.The
SecurePathvalidation at line 64 ensuressafeDiris sanitized before being passed to downstream functions. The decomposition intocollectDocker,collectHealth,collectConfig, andcollectInfrahelpers improves readability.
105-127: LGTM with minor observation.The
LimitReaderat line 121 properly bounds memory usage. Discarding theresp.Body.Close()error is acceptable here since response body close failures are non-actionable in this context.
335-362: LGTM! Clean disk info formatting.The unused
ctxparameter is correctly named_to indicate it's intentionally ignored (common pattern for interface compatibility). ThehumanBytesfunction correctly implements binary prefix formatting (KiB, MiB, GiB...).
257-276: LGTM! Port conflict detection is well-implemented.Using
DialContextwith a 1-second timeout to detect bound ports is a reasonable approach. The structured iteration over port configurations keeps the code clean.
278-297: LGTM! Image availability check is straightforward.Using
docker image inspectto verify local image presence is the correct approach. The conditional sandbox image check aligns with the broader CLI design.
241-255: Compose file variants properly addressed; CodeQL warning is a false positive.The function now correctly searches all default compose filenames (
compose.yaml,compose.yml,docker-compose.yaml,docker-compose.yml), addressing the previous review feedback.The CodeQL warning at line 244 is a false positive.
dataDiris guaranteed to be an absolute, cleaned path (fromconfig.SecurePath), andnamecomes from a hardcoded slice of safe filenames. The combination of an absolute cleaned path with hardcoded filenames is safe from path traversal—no user input reaches the path construction.cli/internal/completion/install.go (7)
18-43: LGTM!Clean idiomatic iota enum with exhaustive
String()method covering all cases including a default fallback.
47-68: LGTM!Shell detection logic is sound—correctly extracts the base name, handles PowerShell Core (
pwsh) on Unix, and applies sensible defaults per platform.
116-158: Regeneration on rerun is now correctly implemented.The previous review flagged that completion files weren't being regenerated on rerun. This is now fixed: lines 127-129 only set
AlreadyInstalled = truebut no longer return early—the completion file is always rewritten (lines 131-141), ensuring users get updated commands after CLI upgrades.Regarding the CodeQL warnings on lines 127, 132, 139: These are false positives. The paths are constructed from
os.UserHomeDir(), which returns an OS-provided value, not attacker-controlled input. Combined withfilepath.Joinfor safe path construction, there is no path traversal vulnerability here.
160-186: LGTM!Correctly follows the same regeneration-on-rerun pattern as zsh. The CodeQL warnings (lines 170, 175, 182) are false positives for the same reason—paths are derived from
os.UserHomeDir(), not user-controlled input.
208-224: PowerShell profile path validation is reasonable but consider documenting threat model.The path comes from executing
$PROFILEin PowerShell (line 212), which is external process output. The current validation (lines 215-221) provides reasonable defense:
- Rejects empty or excessively long (>2048) paths
- Requires an absolute path via
filepath.IsAbs()- Normalizes with
filepath.Clean()The CodeQL warning here has some validity since this is external data, but the practical risk is low: if an attacker can manipulate PowerShell's
$PROFILEoutput, they already have code execution on the system. Thefilepath.IsAbs()check prevents relative path exploits.No changes required, but consider adding a brief comment noting that
$PROFILEis trusted because compromising it implies existing system access.
237-248: LGTM!Clean helper with well-documented behavior for non-existent files. The CodeQL warning (line 240) is a false positive—all callers pass paths derived from
os.UserHomeDir()or validated PowerShell profile paths.
250-269: LGTM!Solid helper with proper error handling:
- Defensively creates parent directories
- Correctly handles both write and close errors separately (avoiding the common mistake of ignoring
f.Close()errors)- Uses appropriate file permissions (0o644)
The CodeQL warnings (lines 253, 256) are false positives for the same reasons as above.
cli/cmd/config.go (2)
39-42: Good fix: data-dir is validated before path-based file checks.
config.SecurePathbeforeconfig.StatePath/os.Statcloses the earlier unsafe path flow and aligns behavior withconfig.Load.
71-76: Secret masking behavior is correct and safe.Returning
(not set)for empty and a constant mask for non-empty values avoids leaking secret length/content.cli/internal/selfupdate/updater_test.go (1)
758-824: Good bundle-path error coverage.These two cases exercise the new non-empty
bundleURLbranches without depending on live Sigstore infrastructure, which makes regressions in the self-update flow much easier to catch.
- Handle non-ErrNotExist os.Stat errors in config show - Add cobra.NoArgs to config and config show commands - Remove unused rootCmd param from installBash - Filter empty-name containers in JSON array parse path - Tighten Sigstore SAN regex to cli.yml workflow on tag refs only
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/internal/completion/install.go`:
- Around line 53-62: The shell-detection switch uses the variable base and only
checks for "pwsh" before returning PowerShell, so add an additional match for
"powershell" (e.g., change the pwsh case to check strings.Contains(base, "pwsh")
|| strings.Contains(base, "powershell")) so systems reporting "powershell" are
detected; modify the case that returns PowerShell (symbols: base, PowerShell)
accordingly and keep the other cases for Bash, Zsh, and Fish unchanged.
In `@cli/internal/diagnostics/collect.go`:
- Around line 242-255: The checkComposeFile function currently returns a boolean
validity that is incorrect when validation is skipped and hides non-ErrNotExist
os.Stat errors; change the validity return to a tri-state (e.g. change signature
to func checkComposeFile(ctx context.Context, info docker.Info, dataDir string)
(exists bool, valid *bool) so nil = unknown), and update callers accordingly;
inside checkComposeFile, treat os.Stat errors: if os.IsNotExist(err) continue,
otherwise return true, nil (existence uncertain/permission error) so the error
isn't swallowed; when a compose file is found and info.DockerPath == "" return
true, nil (unknown) instead of true,false; when running docker.ComposeExec, set
valid = pointerToBool(err == nil); apply the same tri-state and os.Stat error
handling to the analogous function referenced at lines 178-186.
In `@cli/internal/selfupdate/sigstore.go`:
- Line 22: Update the expectedSANRegex variable in sigstore.go to require a
full-string match and enforce a semver tag instead of a loose numeric token;
specifically replace the current pattern (expectedSANRegex) so it adds an end
anchor ($) and a semver constraint like
v[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)? to ensure the
SAN exactly matches the workflow ref string and prevents suffix injection.
🪄 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: 644f0c04-599e-4a17-b68c-69595136034f
📒 Files selected for processing (4)
cli/cmd/config.gocli/internal/completion/install.gocli/internal/diagnostics/collect.gocli/internal/selfupdate/sigstore.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 fuzz testing: testing.F fuzz functions (Fuzz*). Fuzz targets are in testdata/.
Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Note: Go commands requirecd clibecause the Go module is incli/(exception to the "never use cd" rule—Go tooling requires working directory to be the module root).
Files:
cli/cmd/config.gocli/internal/diagnostics/collect.gocli/internal/selfupdate/sigstore.gocli/internal/completion/install.go
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
📚 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/config.gocli/internal/completion/install.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/docker.yml : Docker workflow: builds backend + web + sandbox images, pushes to GHCR, signs with cosign. SLSA L3 provenance attestations via actions/attest-build-provenance. Scans: Trivy (CRITICAL = hard fail, HIGH = warn) + Grype (critical cutoff) + CIS Docker Benchmark v1.6.0 compliance (informational). CVE triage via .github/.trivyignore.yaml and .github/.grype.yaml. Images only pushed after scans pass. Triggers on push to main and version tags (v*).
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/cla-signatures.json : CLA signatures stored in .github/cla-signatures.json on the cla-signatures branch (unprotected, so the action can commit directly). CLA check via contributor-assistant/github-action on pull_request_target and issue_comment. Skips Dependabot.
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/finalize-release.yml : Finalize Release workflow: publishes draft releases created by Release Please. Triggers on workflow_run completion of Docker and CLI workflows. Verifies both workflows succeeded for the associated tag before publishing. Extracts CLI checksums, cosign verification, and container verification data from HTML comments, assembles into combined Verification section. Guards against PR-triggered runs. Handles TOCTOU races. Immutable releases enabled—once published, release assets and body cannot be modified.
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
cli/internal/completion/install.go
🧬 Code graph analysis (1)
cli/cmd/config.go (2)
cli/internal/config/paths.go (2)
SecurePath(58-64)DataDir(18-31)cli/internal/config/state.go (2)
StatePath(40-42)Load(46-84)
🪛 GitHub Check: CodeQL
cli/cmd/config.go
[failure] 47-47: 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.
🔇 Additional comments (16)
cli/cmd/config.go (6)
1-12: LGTM!Imports are clean and all are used within the file.
14-30: LGTM!Both commands correctly specify
Args: cobra.NoArgsto reject unexpected positional arguments, and sharing the sameRunEhandler for the parent command provides intuitive default behavior.
32-35: LGTM!Standard Cobra command registration pattern.
37-54: Path validation is appropriate for CLI context.The CodeQL warning about "uncontrolled data used in path expression" (line 47) is mitigated here:
SecurePathon line 41 cleans and validates the path is absolute before any filesystem operations- This is a CLI tool where users inherently control their own system—specifying arbitrary paths is expected behavior (similar to
cat <path>)The validation ensures consistent behavior (absolute paths only, no
..components) without artificially restricting legitimate use cases. The past review suggestion to validate withSecurePathbeforeos.Stathas been correctly implemented.
61-74: LGTM!Clean structured output with:
- Appropriate use of
strconvfor type conversions- JWT secret properly masked for security
- Docker socket conditionally displayed only when relevant (sandbox enabled)
76-81: LGTM!Good secret masking approach—uses a fixed mask length regardless of actual secret length, preventing information leakage while distinguishing between "not set" and "configured."
cli/internal/completion/install.go (8)
1-16: LGTM!Package declaration and imports are clean. Standard library imports are appropriate for file I/O, path manipulation, and subprocess execution. Cobra dependency aligns with the project's CLI framework choice.
18-43: LGTM!Idiomatic use of
iotafor the shell type enum withUnknownas the zero value. TheString()method provides complete coverage including the default case.
77-93: LGTM!Clean dispatcher pattern with appropriate error handling for unsupported shells. The selective propagation of
ctxandrootCmdto only the installers that need them is reasonable given the quick local I/O operations.
95-114: LGTM!The runtime
evalapproach ensures bash completions are always current without needing static file regeneration. Marker-based idempotency is correctly implemented. The previous review finding about unusedrootCmdparameter has been addressed.
116-158: LGTM!The completion file is now always regenerated (line 131 comment), addressing the prior review finding about stale completions. The
AlreadyInstalledflag correctly serves as an informational indicator without blocking regeneration. The.zshrcmodification remains idempotent via thefpathLinecheck.
160-186: LGTM!Clean implementation that leverages Fish's auto-discovery of completion files. The regeneration behavior mirrors the zsh installer, ensuring updated commands are picked up. No profile modification needed since Fish automatically loads completions from
~/.config/fish/completions/.
188-235: LGTM!Good security practices in
powershellProfilePath: the validation on lines 215-221 ensures the path from the external process is non-empty, within reasonable length bounds, cleaned, and absolute before use. The context is properly propagated for subprocess cancellation. Fallback paths provide reasonable defaults when the subprocess approach fails.
237-269: LGTM!Well-implemented helpers with robust error handling.
fileContainscorrectly treats non-existent files as "does not contain" rather than an error.appendToFileproperly captures both write and close errors separately, ensuring close failures aren't silently ignored.cli/internal/selfupdate/sigstore.go (1)
39-44: Good reliability hardening on trusted-root fetch timeout.Line 42’s bounded HTTP client timeout is a solid safeguard against hanging network calls during TUF root retrieval.
cli/internal/diagnostics/collect.go (1)
299-329: Robust container parsing fallback looks good.JSON-array-first with NDJSON fallback and
Name != ""filtering is solid and avoids the prior empty-summary failure mode.
…r SAN regex - Detect "powershell" in addition to "pwsh" in shell detection - Surface permission errors from compose file stat in Errors slice - Anchor SAN regex with $ and enforce full semver tag pattern
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
cli/internal/diagnostics/collect.go (1)
48-50:⚠️ Potential issue | 🟠 MajorCompose validity is modeled as binary, so “not checked” is reported as “invalid”.
Line 50 and Line 181 force a yes/no state even when validation is skipped (e.g., compose command unavailable), and Line 248-250 can record stat errors while still ending up with a misleading “Not found/Valid: no” report.
🔧 Proposed fix
type Report struct { ComposeFileExists bool `json:"compose_file_exists"` - ComposeFileValid bool `json:"compose_file_valid,omitempty"` + ComposeFileValid *bool `json:"compose_file_valid,omitempty"` PortConflicts []string `json:"port_conflicts,omitempty"` ImageStatus []string `json:"image_status,omitempty"` ContainerSummary []ContainerDetail `json:"container_summary,omitempty"` } @@ func (r Report) formatComposeSection(b *strings.Builder) { b.WriteString("--- Compose File ---\n") if r.ComposeFileExists { - valid := "yes" - if !r.ComposeFileValid { - valid = "no" + valid := "unknown" + if r.ComposeFileValid != nil { + if *r.ComposeFileValid { + valid = "yes" + } else { + valid = "no" + } } fmt.Fprintf(b, "Exists: yes Valid: %s\n\n", valid) } else { b.WriteString("Not found\n\n") } } @@ func checkComposeFile(ctx context.Context, r *Report, info docker.Info, dataDir string) { for _, name := range composeFileNames { composePath := filepath.Join(dataDir, name) if _, err := os.Stat(composePath); err != nil { if errors.Is(err, os.ErrNotExist) { continue } + r.ComposeFileExists = true r.Errors = append(r.Errors, fmt.Sprintf("compose: %s: %v", composePath, err)) - continue + return } r.ComposeFileExists = true - if info.DockerPath != "" { - r.ComposeFileValid = docker.ComposeExec(ctx, info, dataDir, "config", "--quiet") == nil + if len(info.ComposeCmd) > 0 { + ok := docker.ComposeExec(ctx, info, dataDir, "config", "--quiet") == nil + r.ComposeFileValid = &ok } return } }Also applies to: 177-184, 241-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/internal/diagnostics/collect.go` around lines 48 - 50, The ComposeFileValid field is currently a bool so skipped/unchecked states are reported as "invalid"; change ComposeFileValid from bool to *bool in the diagnostics struct (keep `json:"compose_file_valid,omitempty"`), update all code paths that set ComposeFileValid (places around the existing compose-check logic referenced by ComposeFileExists and the compose validation code paths) to assign a pointer to true/false only when validation actually ran, and leave it nil when validation was skipped or a stat/error prevented checking so the field is omitted from JSON; also ensure any code that reads ComposeFileValid dereferences safely (nil means "not checked").cli/internal/completion/install.go (1)
214-223:⚠️ Potential issue | 🟠 MajorConstrain PowerShell profile path to trusted locations before writing.
This is still an uncontrolled path sink pattern: an absolute path from external command output is later used in
appendToFile(Line 251+).Clean+IsAbsis not enough; enforce that the path is under the current user’s home and matches expected profile filename(s).Proposed hardening
func powershellProfilePath(ctx context.Context) (string, error) { + home, homeErr := os.UserHomeDir() + if homeErr != nil { + return "", fmt.Errorf("cannot determine home directory: %w", homeErr) + } + // Try pwsh (PowerShell Core) first, then powershell (Windows PowerShell). for _, shell := range []string{"pwsh", "powershell"} { out, err := exec.CommandContext(ctx, shell, "-NoProfile", "-Command", "echo $PROFILE").Output() if err == nil { p := strings.TrimSpace(string(out)) if p == "" || len(p) > 2048 { continue } p = filepath.Clean(p) if !filepath.IsAbs(p) { continue } + rel, relErr := filepath.Rel(home, p) + if relErr != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + continue + } + base := strings.ToLower(filepath.Base(p)) + if base != "microsoft.powershell_profile.ps1" && base != "profile.ps1" { + continue + } return p, nil } } // Fallback: construct the default path. - home, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("cannot determine home directory: %w", err) - } if runtime.GOOS == "windows" { return filepath.Join(home, "Documents", "PowerShell", "Microsoft.PowerShell_profile.ps1"), nil } return filepath.Join(home, ".config", "powershell", "Microsoft.PowerShell_profile.ps1"), nil }Also applies to: 251-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/internal/completion/install.go` around lines 214 - 223, The returned PowerShell profile path (variable p) is currently only cleaned and checked for absoluteness before being used by appendToFile; tighten this by validating p is inside the current user's home directory and has an expected profile filename. Update the code around the filepath.Clean/IsAbs checks in the function that returns p to: resolve the current user home (os.UserHomeDir or equivalent), ensure p has the home directory as a path prefix (use filepath.Rel or strings.HasPrefix after evaluating symlinks if relevant), and verify filepath.Base(p) matches an allowlist of acceptable names (e.g., expected PowerShell profile filenames). Only return p if all checks pass; otherwise continue the loop so appendToFile never writes to an untrusted location.
🤖 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/internal/completion/install.go`:
- Around line 81-87: Guard against a nil rootCmd in the exported Install
function before calling the zsh/fish installers: if shell is Zsh or Fish and
rootCmd is nil, return a typed error instead of dispatching to installZsh or
installFish; ensure installZsh and installFish still call
rootCmd.GenZshCompletion() / rootCmd.GenFishCompletion() only when rootCmd is
non-nil. Reference Install, installZsh, installFish, GenZshCompletion and
GenFishCompletion in your change.
In `@cli/internal/diagnostics/collect.go`:
- Around line 82-87: The docker.Detect call in collectDocker can return a
partial docker.Info on error, causing later logic that only checks
info.DockerPath (e.g., the checkImages call at/around lines 150-152) to run and
produce false negatives; change collectDocker so that when docker.Detect returns
an error you append the error to r.Errors and return a zeroed/empty docker.Info
(not the partial info) to signal detection failure (or explicitly clear
info.DockerPath / set a detection flag) so downstream checks like checkImages
will be skipped; update collectDocker (and its return behavior) to ensure
callers rely on a clear "not detected" state rather than partial data.
---
Duplicate comments:
In `@cli/internal/completion/install.go`:
- Around line 214-223: The returned PowerShell profile path (variable p) is
currently only cleaned and checked for absoluteness before being used by
appendToFile; tighten this by validating p is inside the current user's home
directory and has an expected profile filename. Update the code around the
filepath.Clean/IsAbs checks in the function that returns p to: resolve the
current user home (os.UserHomeDir or equivalent), ensure p has the home
directory as a path prefix (use filepath.Rel or strings.HasPrefix after
evaluating symlinks if relevant), and verify filepath.Base(p) matches an
allowlist of acceptable names (e.g., expected PowerShell profile filenames).
Only return p if all checks pass; otherwise continue the loop so appendToFile
never writes to an untrusted location.
In `@cli/internal/diagnostics/collect.go`:
- Around line 48-50: The ComposeFileValid field is currently a bool so
skipped/unchecked states are reported as "invalid"; change ComposeFileValid from
bool to *bool in the diagnostics struct (keep
`json:"compose_file_valid,omitempty"`), update all code paths that set
ComposeFileValid (places around the existing compose-check logic referenced by
ComposeFileExists and the compose validation code paths) to assign a pointer to
true/false only when validation actually ran, and leave it nil when validation
was skipped or a stat/error prevented checking so the field is omitted from
JSON; also ensure any code that reads ComposeFileValid dereferences safely (nil
means "not checked").
🪄 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: bc92a65a-16bd-489f-904a-08fb463f9a36
📒 Files selected for processing (3)
cli/internal/completion/install.gocli/internal/diagnostics/collect.gocli/internal/selfupdate/sigstore.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 fuzz testing: testing.F fuzz functions (Fuzz*). Fuzz targets are in testdata/.
Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Note: Go commands requirecd clibecause the Go module is incli/(exception to the "never use cd" rule—Go tooling requires working directory to be the module root).
Files:
cli/internal/diagnostics/collect.gocli/internal/selfupdate/sigstore.gocli/internal/completion/install.go
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/*.yml : Dependabot: daily updates for uv + github-actions + npm + pre-commit + docker + gomod, grouped minor/patch, no auto-merge. Use `/review-dep-pr` to review Dependabot PRs before merging.
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).
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/internal/diagnostics/collect.gocli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/docker.yml : Docker workflow: builds backend + web + sandbox images, pushes to GHCR, signs with cosign. SLSA L3 provenance attestations via actions/attest-build-provenance. Scans: Trivy (CRITICAL = hard fail, HIGH = warn) + Grype (critical cutoff) + CIS Docker Benchmark v1.6.0 compliance (informational). CVE triage via .github/.trivyignore.yaml and .github/.grype.yaml. Images only pushed after scans pass. Triggers on push to main and version tags (v*).
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/cla-signatures.json : CLA signatures stored in .github/cla-signatures.json on the cla-signatures branch (unprotected, so the action can commit directly). CLA check via contributor-assistant/github-action on pull_request_target and issue_comment. Skips Dependabot.
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/finalize-release.yml : Finalize Release workflow: publishes draft releases created by Release Please. Triggers on workflow_run completion of Docker and CLI workflows. Verifies both workflows succeeded for the associated tag before publishing. Extracts CLI checksums, cosign verification, and container verification data from HTML comments, assembles into combined Verification section. Guards against PR-triggered runs. Handles TOCTOU races. Immutable releases enabled—once published, release assets and body cannot be modified.
Applied to files:
cli/internal/selfupdate/sigstore.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 .github/workflows/docker.yml : CI Docker: build → scan → push to GHCR + cosign sign + SLSA L3 provenance via attest-build-provenance (images only pushed after Trivy/Grype scans pass).
Applied to files:
cli/internal/selfupdate/sigstore.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to .github/workflows/*.yml : Dependabot: daily updates for uv + github-actions + npm + pre-commit + docker + gomod, grouped minor/patch, no auto-merge. Use `/review-dep-pr` to review Dependabot PRs before merging.
Applied to files:
cli/internal/selfupdate/sigstore.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/internal/completion/install.go
📚 Learning: 2026-03-15T19:54:39.887Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:54:39.887Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
cli/internal/completion/install.go
🧬 Code graph analysis (1)
cli/internal/diagnostics/collect.go (2)
cli/internal/config/state.go (1)
State(14-23)cli/internal/docker/client.go (6)
Info(22-29)Detect(34-66)CheckMinVersions(69-78)ComposeExecOutput(100-107)ComposeExec(91-97)RunCmd(110-119)
🪛 GitHub Check: CodeQL
cli/internal/diagnostics/collect.go
[failure] 244-244: 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 (4)
cli/internal/selfupdate/sigstore.go (3)
17-25: Signer identity and tag scoping look correctly hardened.The SAN regex is fully anchored and scoped to
cli.ymlon semver tags, and the trusted-root fetch has an explicit timeout bound.
33-80: Verification path is robust and correctly layered.Good sequence: bundle parse → trusted root fetch → verifier with SCT/tlog requirements → certificate identity policy → artifact digest verification.
83-89:loadBundleFromJSONis clean and purpose-specific.The helper is concise and returns errors to the caller without swallowing context.
cli/internal/completion/install.go (1)
131-141: Good fix: completion files are always regenerated on rerun.This correctly prevents stale zsh/fish completions after upgrades while keeping install idempotent.
Also applies to: 174-184
…e tri-state - Guard against nil rootCmd in Install for zsh/fish shells - Return zero docker.Info on Detect failure to prevent false negatives - Validate PowerShell profile path is inside user's home directory - Change ComposeFileValid to *bool: nil = not checked, true/false = result
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
cli/internal/completion/install.go (1)
227-235:⚠️ Potential issue | 🟠 MajorHarden path containment before writing profile files.
At Lines 227-235, the current check is lexical (
Rel+ prefix). A symlinked path inside home can still resolve outside home and be written byappendToFile. Resolve symlinks (or reject symlink targets) before accepting the final write path.♻️ Proposed fix
func powershellProfilePath(ctx context.Context) (string, error) { home, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("cannot determine home directory: %w", err) } + resolvedHome, err := filepath.EvalSymlinks(home) + if err != nil { + return "", fmt.Errorf("resolving home directory: %w", err) + } @@ p = filepath.Clean(p) if !filepath.IsAbs(p) { continue } - // Ensure path is inside the user's home directory. - if rel, relErr := filepath.Rel(home, p); relErr != nil || strings.HasPrefix(rel, "..") { + resolvedParent, relErr := filepath.EvalSymlinks(filepath.Dir(p)) + if relErr != nil { + continue + } + candidate := filepath.Join(resolvedParent, filepath.Base(p)) + // Ensure resolved path is inside the user's home directory. + rel, relErr := filepath.Rel(resolvedHome, candidate) + if relErr != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { continue } - return p, nil + return candidate, nil } } @@ func appendToFile(path, content string) error { + if fi, err := os.Lstat(path); err == nil && (fi.Mode()&os.ModeSymlink) != 0 { + return fmt.Errorf("refusing to write via symlink: %s", path) + } dir := filepath.Dir(path)Also applies to: 259-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/internal/completion/install.go` around lines 227 - 235, The lexical containment check using filepath.Rel can be bypassed by symlinks; update the path validation in install.go (the block that calls filepath.Clean, filepath.IsAbs, filepath.Rel and returns p) to resolve and validate symlinks before accepting the path: call filepath.EvalSymlinks on both the candidate path and the user's home directory (or use os.Lstat to detect and reject symlinked targets), then verify the evaluated absolute path is inside the evaluated home directory and reject (continue) on EvalSymlinks errors or when the evaluated path escapes home; mirror the same fix for the similar block around lines 259-278 so appendToFile only ever writes to resolved paths inside the home.
🤖 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/internal/completion/install.go`:
- Around line 220-237: The PowerShell probing loop uses exec.CommandContext(ctx,
...) but ctx has no deadline so a hanging shell can block; wrap each
exec.CommandContext call in a short per-command context with a timeout (e.g.,
context.WithTimeout(ctx, 3-5s)), defer the cancel, and use that timed context
for exec.CommandContext in the loop that iterates over {"pwsh","powershell"}
(the block that calls exec.CommandContext, trims output, cleans path and
validates it) so each probe will be bounded and won't hang indefinitely.
In `@cli/internal/diagnostics/collect_test.go`:
- Around line 200-206: The test TestCheckPortsNoConflict makes a misleading
comment about port 0; update the comment near TestCheckPortsNoConflict to
clarify that we use port 0 because attempting to dial 127.0.0.1:0 will fail (you
can't connect to port 0) rather than implying port 0 is never bound; leave the
test invoking checkPorts(context.Background(), 0, 0) unchanged but replace the
comment with a precise explanation referencing TestCheckPortsNoConflict and
checkPorts.
- Around line 105-133: Add a test case to TestParseContainerDetails exercising
the JSON array format path in parseContainerDetails: create a new table entry
(e.g., name "json_array") whose input is a JSON array string like
`[{"Name":"app","State":"running","Status":"Up"},{"Name":"db","State":"exited","Status":"Exited
(1)"}]` and set want to 2, then run the same t.Run assertion so both NDJSON and
JSON-array parsing branches of parseContainerDetails are validated.
In `@cli/internal/diagnostics/collect.go`:
- Around line 364-375: The humanBytes function can panic if exp exceeds the
length of the suffix string "KMGTPE"; update humanBytes to defensively handle
extremely large values by bounding exp before indexing: compute exp as now, then
if exp >= len(suffixes) set exp = len(suffixes)-1 (or choose an extended suffix
string) so indexing into the suffix string (referenced in humanBytes) is always
safe; keep the rest of the formatting logic unchanged.
---
Duplicate comments:
In `@cli/internal/completion/install.go`:
- Around line 227-235: The lexical containment check using filepath.Rel can be
bypassed by symlinks; update the path validation in install.go (the block that
calls filepath.Clean, filepath.IsAbs, filepath.Rel and returns p) to resolve and
validate symlinks before accepting the path: call filepath.EvalSymlinks on both
the candidate path and the user's home directory (or use os.Lstat to detect and
reject symlinked targets), then verify the evaluated absolute path is inside the
evaluated home directory and reject (continue) on EvalSymlinks errors or when
the evaluated path escapes home; mirror the same fix for the similar block
around lines 259-278 so appendToFile only ever writes to resolved paths inside
the home.
🪄 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: 701cf309-5710-495a-952c-cee39bd41259
📒 Files selected for processing (3)
cli/internal/completion/install.gocli/internal/diagnostics/collect.gocli/internal/diagnostics/collect_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 (2)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use nativetesting.Ffuzz functions (Fuzz*) for fuzz testing in Go.
Use Cobra for CLI commands in the Go CLI binary.
Use charmbracelet/huh for interactive CLI prompts.
Use charmbracelet/lipgloss for styled CLI output.
Files:
cli/internal/diagnostics/collect_test.gocli/internal/completion/install.gocli/internal/diagnostics/collect.go
cli/**/*test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Run Go tests with race detector and coverage:
go test -race -coverprofile.
Files:
cli/internal/diagnostics/collect_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: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
📚 Learning: 2026-03-15T21:30:26.325Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:30:26.325Z
Learning: Applies to cli/**/*test.go : Run Go tests with race detector and coverage: `go test -race -coverprofile`.
Applied to files:
cli/internal/diagnostics/collect_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/internal/completion/install.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/internal/diagnostics/collect.go
🧬 Code graph analysis (2)
cli/internal/diagnostics/collect_test.go (1)
cli/internal/diagnostics/collect.go (2)
ContainerDetail(24-29)Report(32-53)
cli/internal/diagnostics/collect.go (1)
cli/internal/docker/client.go (6)
Info(22-29)Detect(34-66)CheckMinVersions(69-78)ComposeExecOutput(100-107)ComposeExec(91-97)RunCmd(110-119)
🔇 Additional comments (14)
cli/internal/diagnostics/collect.go (8)
23-29: LGTM!The
ContainerDetailstruct is well-designed with appropriate JSON tags matching Docker Compose's output format. TheomitemptyonHealthcorrectly handles containers without health checks.
47-52: LGTM!The Report struct extensions are appropriate:
ComposeFileValidas*boolcorrectly implements tri-state (nil = not checked, true/false = result) as indicated in the commit messages.- New fields use
omitemptyappropriately for optional data.
82-104: LGTM!The refactored
collectDockercorrectly returns a zero-valuedocker.Info{}on detection failure (line 86), which addresses the previous review feedback about false negatives from partial state. Downstream checks likecheckImageswill be properly skipped sinceinfo.DockerPathwill be empty.
106-128: LGTM!The function properly limits response body reads to 64KB using
io.LimitReader, propagates context correctly, and handles errors appropriately.
244-262: LGTM!The
checkComposeFilefunction now correctly:
- Uses tri-state
*boolfor validity (nil = not checked, true/false = result)- Properly distinguishes
os.ErrNotExistfrom other errors usingerrors.Is- Logs non-existence errors to
r.Errorsinstead of swallowing themThis addresses the previous review feedback about misreported validation status and swallowed stat errors.
264-283: LGTM!The port conflict detection uses a reasonable approach with proper timeout and connection cleanup. The logic correctly identifies ports already bound by attempting a TCP connection.
285-304: LGTM!The image availability check is straightforward and correctly uses
docker image inspectto verify local presence. The conditional sandbox image check respects the configuration.
306-336: LGTM!The function correctly handles both JSON array (newer Compose) and NDJSON (legacy) formats, addressing previous review feedback. The consistent
Name != ""filtering in both paths ensuresContainerSummarynever receives empty-named entries.cli/internal/diagnostics/collect_test.go (5)
105-133: LGTM!The
TestParseContainerDetailsuses idiomatic table-driven tests covering key scenarios: single object, multiple NDJSON lines, empty input, invalid JSON, and blank lines. Good coverage of the parsing logic.
135-151: LGTM!
TestParseContainerDetailsFieldsvalidates that individual fields are correctly extracted from JSON, complementing the count-based tests with field-level assertions.
153-171: LGTM!
TestHasRunningContainerscovers the key states: empty slice, running, exited, and mixed containers. The test correctly verifies the "running" state detection logic.
208-238: LGTM!
TestFormatTextNewSectionscomprehensively validates that all new report sections (Compose File, Container Summary, Port Conflicts, Docker Images) are correctly rendered in the text output. The test properly usesptrBoolhelper for the tri-stateComposeFileValidfield.
240-240: LGTM!The
ptrBoolhelper is a clean, minimal utility for creating*boolvalues in tests. Idiomatic Go approach.cli/internal/completion/install.go (1)
81-83: Good defensive nil guard for generator-backed shells.The check prevents panic paths before
GenZshCompletion/GenFishCompletionare reached.
…coverage - Add 5s timeout per PowerShell probe to prevent hanging - Resolve symlinks before home-directory containment check - Bound humanBytes exp index to prevent panic on exabyte+ values - Fix misleading port 0 test comment - Add JSON array test case for parseContainerDetails
🤖 I have created a release *beep* *boop* --- ## [0.2.6](v0.2.5...v0.2.6) (2026-03-15) ### Features * add intra-loop stagnation detector ([#415](#415)) ([#458](#458)) ([8e9f34f](8e9f34f)) * add RFC 9457 structured error responses (Phase 1) ([#457](#457)) ([6612a99](6612a99)), closes [#419](#419) * implement AgentStateRepository for runtime state persistence ([#459](#459)) ([5009da7](5009da7)) * **site:** add SEO essentials, contact form, early-access banner ([#467](#467)) ([11b645e](11b645e)), closes [#466](#466) ### Bug Fixes * CLI improvements — config show, completion install, enhanced doctor, Sigstore verification ([#465](#465)) ([9e08cec](9e08cec)) * **site:** add reCAPTCHA v3, main landmark, and docs sitemap ([#469](#469)) ([fa6d35c](fa6d35c)) * use force-tag-creation instead of manual tag creation hack ([#462](#462)) ([2338004](2338004)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
release-assets.githubusercontent.comto allowed download hosts — GitHub started routing release assets through this domain, breakingsynthorg updatesynthorg config show: Displays current CLI configuration with masked JWT secret. Warns if not initializedsynthorg completion-install: Auto-detects shell (bash/zsh/fish/powershell) and installs tab-completion. Idempotent. Leaves Cobra's built-incompletioncommand untouchedsynthorg doctor: Adds compose file validation, port conflict detection, Docker image availability checks, container detail parsing from compose ps JSON, Docker/Compose minimum version warningsfsutil/dfsubprocess with Go syscalls (GetDiskFreeSpaceExWon Windows,Statfson Unix) — fixes "unavailable" on non-admin Windowschecksums.txt.sigstore.jsonagainst Sigstore's public good transparency log, checking that the signing identity matches the GitHub Actions OIDC issuer for this repo. Usessigstore/sigstore-go(new dependency)Closes #234
Test plan
cd cli && go vet ./...— cleancd cli && golangci-lint run— 0 issuescd cli && go test ./...— all passcd cli && go build -o synthorg ./main.go— builds./synthorg config show— displays config or "not initialized"./synthorg completion-install— detects shell, installs completions./synthorg completion-install(again) — reports "already installed"./synthorg doctor— shows compose file, images, disk info sections./synthorg completion bash— Cobra built-in still worksReview coverage
Pre-reviewed by 5 agents (go-reviewer, go-security-reviewer, go-conventions-enforcer, docs-consistency, issue-resolution-verifier). 15 findings addressed, 1 deferred (log secret redaction — user decision).
🤖 Generated with Claude Code