fix: resolve CodeQL path-injection alerts in Go CLI#412
Conversation
Add config.SecurePath() helper that validates paths are absolute and applies filepath.Clean() — satisfies CodeQL's go/path-injection taint tracking. Apply at all filesystem operation call sites: - config: EnsureDir, Load, Save - cmd: start, stop, logs, doctor, status, uninstall, init Also update post-merge-cleanup skill to avoid piped xargs commands that trigger permission prompts.
Fixes from go-reviewer, security-reviewer, CodeRabbit, Copilot, Gemini: - Fix 4 missed files: update.go, uninstall.go (stopAndRemoveVolumes), status.go (printContainerStates/printResourceUsage), diagnostics/collect.go - Remove dead redundant validation in state.go Load (use safeDir directly) - Use SecurePath in fileExists instead of bare filepath.Clean - Remove double-SecurePath in Save/writeInitFiles (use os.MkdirAll directly) - Normalize state.DataDir before marshaling in Save and writeInitFiles - Add trust boundary doc comment to SecurePath - Fix DataDir() fallback from "." to os.Getwd() (absolute path) - Add 5 unit tests for SecurePath (absolute, relative, empty, clean, traversal) - Fix SKILL.md markdown lint (blank lines around fenced code blocks)
- state.go: replace manual filepath.Clean+IsAbs with SecurePath for loaded DataDir (consistency with file-not-found branch) - diagnostics/collect.go: don't early-return on path error — continue collecting health, config, and error info (most useful when broken) - uninstall.go: use safeStateDir helper consistently (not direct config.SecurePath)
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds path sanitization via config.SecurePath and a safeStateDir helper; threads validated data-dir values through CLI commands, state persistence, and diagnostics; adjusts init/uninstall flows and tests; updates pre-commit hooks and a docs cleanup file. Changes
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 unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
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 security posture of the CLI by addressing multiple path-injection vulnerabilities identified by CodeQL. It introduces a new 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.
Pull request overview
This PR resolves 11 CodeQL go/path-injection alerts in the Go CLI by introducing a SecurePath() helper that validates paths are absolute and cleaned before any filesystem operations. It applies this validation consistently across all command files and configuration loading/saving, superseding the closed PR #411 with additional fixes from two review rounds.
Changes:
- Added
config.SecurePath()for absolute-path validation withfilepath.Clean, and asafeStateDir()cmd helper for consistent usage across all command handlers - Applied
SecurePathat all filesystem call sites (Load,Save,EnsureDir, and all cmd files) and fixedDataDir()fallback from"."toos.Getwd()for absolute paths - Improved
diagnostics/collect.goto gracefully handle path errors without losing Docker version and health information, and added 5 unit tests forSecurePath
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
cli/internal/config/paths.go |
Added SecurePath() function; updated DataDir() fallback to absolute CWD; EnsureDir now uses SecurePath |
cli/internal/config/state.go |
Load and Save now validate paths via SecurePath; removed redundant inline validation |
cli/internal/config/paths_test.go |
Added 5 unit tests for SecurePath (absolute, relative, empty, clean, traversal) |
cli/cmd/root.go |
Added safeStateDir() helper wrapping config.SecurePath(state.DataDir) |
cli/cmd/start.go |
Uses safeStateDir for compose path and operations |
cli/cmd/stop.go |
Uses safeStateDir for compose path and operations |
cli/cmd/logs.go |
Uses safeStateDir for compose path and operations |
cli/cmd/doctor.go |
Uses safeStateDir for diagnostic file save path |
cli/cmd/status.go |
Uses safeStateDir; changed helper functions to accept string instead of config.State |
cli/cmd/init.go |
Uses SecurePath directly; fileExists now validates path |
cli/cmd/uninstall.go |
Uses safeStateDir in both volume removal and data removal paths |
cli/cmd/update.go |
Uses safeStateDir for compose operations |
cli/internal/diagnostics/collect.go |
Gracefully handles path errors; still collects Docker version, health, and config |
.claude/skills/post-merge-cleanup/SKILL.md |
Markdown lint fix; replaced piped xargs with individual git branch -D |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This is an excellent and thorough pull request that systematically addresses the CodeQL go/path-injection alerts. The introduction of config.SecurePath() and the safeStateDir() helper provides a robust and consistent way to sanitize paths across the entire CLI. The changes are well-structured, the new unit tests provide good coverage, and the refactoring to make diagnostic collection more resilient is a great improvement. I have one minor suggestion to improve code clarity and reduce duplication in the diagnostics collection logic.
cli/internal/diagnostics/collect.go
Outdated
| } else if pathErr == nil { | ||
| r.DockerVersion = info.DockerVersion | ||
| r.ComposeVersion = info.ComposeVersion | ||
|
|
||
| // Container states. | ||
| if ps, err := docker.ComposeExecOutput(ctx, info, state.DataDir, "ps", "--format", "json"); err == nil { | ||
| if ps, err := docker.ComposeExecOutput(ctx, info, safeDir, "ps", "--format", "json"); err == nil { | ||
| r.ContainerPS = strings.TrimSpace(ps) | ||
| } | ||
|
|
||
| // Recent logs (last 50 lines). | ||
| if logs, err := docker.ComposeExecOutput(ctx, info, state.DataDir, "logs", "--tail", "50", "--no-color"); err == nil { | ||
| if logs, err := docker.ComposeExecOutput(ctx, info, safeDir, "logs", "--tail", "50", "--no-color"); err == nil { | ||
| r.RecentLogs = truncate(logs, 4000) | ||
| } | ||
| } else { | ||
| r.DockerVersion = info.DockerVersion | ||
| r.ComposeVersion = info.ComposeVersion | ||
| } |
There was a problem hiding this comment.
This else if/else block can be simplified to reduce code duplication. The r.DockerVersion and r.ComposeVersion assignments are repeated. You can use a single else block and move the pathErr == nil check inside it to wrap the logic that depends on a valid path. This makes the code more concise and easier to maintain.
} else {
r.DockerVersion = info.DockerVersion
r.ComposeVersion = info.ComposeVersion
if pathErr == nil {
// Container states.
if ps, err := docker.ComposeExecOutput(ctx, info, safeDir, "ps", "--format", "json"); err == nil {
r.ContainerPS = strings.TrimSpace(ps)
}
// Recent logs (last 50 lines).
if logs, err := docker.ComposeExecOutput(ctx, info, safeDir, "logs", "--tail", "50", "--no-color"); err == nil {
r.RecentLogs = truncate(logs, 4000)
}
}
}
Greptile SummaryThis PR resolves all 11 CodeQL
Confidence Score: 4/5
Important Files Changed
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/cmd/init.go (1)
183-208:⚠️ Potential issue | 🟡 MinorOutput messages may show unsanitized path while files are written to cleaned path.
writeInitFilesreceivesstateby value and updatesstate.DataDir = safeDiron line 188, but this doesn't affect the caller. The output messages inrunInit(lines 69-74) still reference the originalstate.DataDir:// In runInit, after writeInitFiles returns: composePath := filepath.Join(state.DataDir, "compose.yml") // original, not cleaned _, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nSynthOrg initialized in %s\n", state.DataDir)If the user provides
/home/user//synthorg/(double slash or trailing slash), files are written to/home/user/synthorgbut the output shows the original path.Proposed fix: return sanitized state or use safeDir in caller
Option 1 - Return sanitized state:
-func writeInitFiles(state config.State) error { +func writeInitFiles(state config.State) (config.State, error) { safeDir, err := config.SecurePath(state.DataDir) if err != nil { - return err + return state, err } state.DataDir = safeDir // normalize before persisting // ... rest of function ... - return nil + return state, nil }Then in
runInit:- if err := writeInitFiles(state); err != nil { + state, err = writeInitFiles(state) + if err != nil { return err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/cmd/init.go` around lines 183 - 208, writeInitFiles currently normalizes DataDir into a local safeDir but doesn't propagate it back to the caller, so runInit still prints and uses the un-cleaned state.DataDir; modify writeInitFiles (or its signature) to return the sanitized path or the updated config.State (e.g., return (config.State, error) or (string, error)), ensure it persists the normalized value (safeDir) before saving, and then update runInit to use the returned sanitized value (replace uses of state.DataDir when constructing composePath and printing the "initialized in" message) so output and file locations match the cleaned path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/post-merge-cleanup/SKILL.md:
- Line 31: Replace the absolute claim "Do NOT use a piped `xargs` command — it
triggers unnecessary permission prompts." with a softened recommendation that
avoids attributing universal behavior to shells/configs; for example, rephrase
to something like "Avoid piped bulk deletion (e.g., via `xargs`) to reduce the
risk of accidental destructive operations; instead use explicit `git branch -D
branch1 branch2` calls." Update the sentence in SKILL.md where the original line
appears and keep the example `git branch -D branch1 branch2` as the recommended
alternative.
In `@cli/cmd/init.go`:
- Around line 235-242: fileExists currently swallows SecurePath errors and
returns false, masking invalid-path issues (e.g., when calling
fileExists(config.StatePath(...))). Change fileExists to surface path validation
errors instead of silently returning false: either (preferred) change its
signature to return (bool, error) and return the SecurePath error to the caller,
or (alternatively) log the SecurePath failure before returning false; update all
callers (e.g., the call that checks config.StatePath(state.DataDir)) to handle
the returned error and act accordingly. Ensure references to SecurePath,
fileExists, and the caller that checks StatePath are updated to handle the new
error-returning behavior.
In `@cli/cmd/uninstall.go`:
- Around line 62-67: Compute safeDir once in runUninstall and pass it into
stopAndRemoveVolumes and confirmAndRemoveData instead of calling
safeStateDir(state) inside each helper: call safeStateDir(state) at the start of
runUninstall, handle the error there, then change the signatures of
stopAndRemoveVolumes(cmd *cobra.Command, info docker.Info, state config.State)
and confirmAndRemoveData(...) to accept the precomputed safeDir (string) and
remove their internal safeStateDir calls so they use the passed safeDir for
validation and operations.
In `@cli/internal/config/paths.go`:
- Around line 21-26: The DataDir fallback currently assigns home = "/" which is
unsafe on Windows; update the fallback in paths.go where home is set (after
os.Getwd()) to be platform-aware: if runtime.GOOS == "windows" choose a sensible
Windows fallback such as os.TempDir() (or compute the system root via
filepath.VolumeName(os.Getenv("SystemRoot")) + "\\" if you prefer a drive root),
otherwise keep "/" for Unix; ensure you import runtime and filepath as needed
and preserve existing SecurePath/absolute-path expectations.
---
Outside diff comments:
In `@cli/cmd/init.go`:
- Around line 183-208: writeInitFiles currently normalizes DataDir into a local
safeDir but doesn't propagate it back to the caller, so runInit still prints and
uses the un-cleaned state.DataDir; modify writeInitFiles (or its signature) to
return the sanitized path or the updated config.State (e.g., return
(config.State, error) or (string, error)), ensure it persists the normalized
value (safeDir) before saving, and then update runInit to use the returned
sanitized value (replace uses of state.DataDir when constructing composePath and
printing the "initialized in" message) so output and file locations match the
cleaned path.
🪄 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: 9ab44a38-3949-42a8-93e9-6abb3812dc47
📒 Files selected for processing (14)
.claude/skills/post-merge-cleanup/SKILL.mdcli/cmd/doctor.gocli/cmd/init.gocli/cmd/logs.gocli/cmd/root.gocli/cmd/start.gocli/cmd/status.gocli/cmd/stop.gocli/cmd/uninstall.gocli/cmd/update.gocli/internal/config/paths.gocli/internal/config/paths_test.gocli/internal/config/state.gocli/internal/diagnostics/collect.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). (4)
- GitHub Check: Agent
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: CLI Go code: Go 1.26+, dependencies incli/go.mod. Commands include: init, start, stop, status, logs, doctor, update, uninstall, version. Build with GoReleaser for cross-compile (linux/darwin/windows × amd64/arm64).
Go CLI: Go 1.26+, dependencies incli/go.mod. Use Cobra for commands, charmbracelet/huh for CLI UI. Build with GoReleaser.
Files:
cli/cmd/uninstall.gocli/internal/config/paths_test.gocli/cmd/logs.gocli/cmd/init.gocli/cmd/stop.gocli/internal/config/state.gocli/cmd/doctor.gocli/cmd/start.gocli/cmd/root.gocli/internal/diagnostics/collect.gocli/internal/config/paths.gocli/cmd/status.gocli/cmd/update.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow (`.github/workflows/cli.yml`): Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile matrix: linux/darwin/windows × amd64/arm64) + vulnerability check (govulncheck) on `cli/**` changes. GoReleaser release on `v*` tags (attaches assets to existing Release Please release). Post-release pins checksums in install scripts and appends to GitHub Release notes.
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to cli/**/*.go : CLI Go code: Go 1.26+, dependencies in `cli/go.mod`. Commands include: init, start, stop, status, logs, doctor, update, uninstall, version. Build with GoReleaser for cross-compile (linux/darwin/windows × amd64/arm64).
Applied to files:
cli/cmd/start.gocli/cmd/update.go
🧬 Code graph analysis (7)
cli/internal/config/paths_test.go (1)
cli/internal/config/paths.go (1)
SecurePath(56-62)
cli/cmd/init.go (1)
cli/internal/config/paths.go (2)
SecurePath(56-62)DataDir(18-29)
cli/internal/config/state.go (1)
cli/internal/config/paths.go (2)
SecurePath(56-62)DataDir(18-29)
cli/cmd/root.go (2)
cli/internal/config/state.go (1)
State(14-23)cli/internal/config/paths.go (2)
SecurePath(56-62)DataDir(18-29)
cli/internal/diagnostics/collect.go (2)
cli/internal/config/paths.go (2)
SecurePath(56-62)DataDir(18-29)cli/internal/docker/client.go (2)
Detect(34-66)ComposeExecOutput(100-107)
cli/cmd/status.go (1)
cli/internal/docker/client.go (2)
Info(22-29)ComposeExecOutput(100-107)
cli/cmd/update.go (1)
cli/internal/docker/client.go (2)
Detect(34-66)ComposeExecOutput(100-107)
🪛 GitHub Check: CLI Lint
cli/internal/diagnostics/collect.go
[failure] 56-56:
ifElseChain: rewrite if-else to switch statement (gocritic)
🪛 GitHub Check: CodeQL
cli/cmd/uninstall.go
[failure] 115-115: 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/cmd/logs.go
[failure] 49-49: 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/cmd/init.go
[failure] 189-189: 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] 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.
cli/cmd/stop.go
[failure] 34-34: 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/config/state.go
[failure] 49-49: 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] 82-82: 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] 89-89: 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/cmd/doctor.go
[failure] 44-44: 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/cmd/start.go
[failure] 38-38: 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/cmd/status.go
[failure] 44-44: 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)
.claude/skills/post-merge-cleanup/SKILL.md (1)
19-29: Good safety upgrade to cleanup flow.Previewing gone branches before deletion and adding a final
git statuscheck makes this procedure safer and more auditable.Also applies to: 35-35
cli/internal/config/paths.go (2)
49-62: SecurePath implementation and trust boundary documentation look good.The function correctly:
- Cleans the path with
filepath.Clean- Validates it's absolute with
filepath.IsAbs- Documents the explicit trust boundary in the security note
This addresses the CodeQL path-injection alerts by establishing a validation checkpoint, while correctly noting that the CLI intentionally trusts user-provided paths (this is a local CLI tool where the user controls their own installation directory).
64-71: LGTM — EnsureDir now validates paths before creating directories.Applying SecurePath before MkdirAll ensures the path is absolute and cleaned, providing defense-in-depth at the filesystem operation site.
cli/internal/config/paths_test.go (1)
69-118: Comprehensive test coverage for SecurePath.The tests appropriately cover:
- Valid absolute paths (platform-aware)
- Rejection of relative paths
- Rejection of empty strings
- Path cleaning/normalization
- Traversal attempts with
..The
absTestPathhelper correctly handles cross-platform differences between Windows and Unix path formats.cli/internal/config/state.go (3)
44-49: Path validation correctly applied before filesystem reads.The flow validates
dataDirviaSecurePathbefore constructing the config file path. The//nolint:gosecannotation with the "path validated by SecurePath" comment appropriately documents the trust boundary for static analysis tools.The CodeQL alert at line 49 is a known false positive — the path is sanitized, but CodeQL cannot trace through the validation. The existing nolint comment is the correct approach.
64-70: Post-load validation of persisted DataDir is correct.Re-validating
s.DataDirafter unmarshaling ensures that even if a config file was manually edited with an invalid path, it will be caught before use. The wrapped error with "data_dir:" prefix provides clear context.
76-89: Save correctly canonicalizes DataDir before persisting.The pattern of:
- Validate with SecurePath
- Assign canonical form back to struct
- Use validated path for MkdirAll and WriteFile
ensures the persisted config always contains a clean, absolute path. The CodeQL warnings at lines 82 and 89 are false positives for the same reason as above — the paths flow through SecurePath validation.
cli/cmd/root.go (1)
44-48: LGTM — centralized helper for consistent path validation.
safeStateDirprovides a single call site for commands to validatestate.DataDir, ensuring consistent CodeQL compliance across the codebase. WhileLoad()already validates paths, this helper provides defense-in-depth for cases where state is constructed from defaults or modified after loading.cli/cmd/start.go (1)
37-44: Path validation correctly integrated into start command.The command validates
state.DataDirviasafeStateDirbefore any filesystem or docker-compose operations. The CodeQL warning at line 38 is a false positive —safeStateDirwrapsSecurePathwhich validates the path is absolute and cleaned.All subsequent operations (compose.yml lookup, composeRun calls) correctly use
safeDir.cli/cmd/uninstall.go (1)
113-125: Good defense-in-depth with multiple safety checks before RemoveAll.The code correctly:
- Validates the path with
safeStateDir(line 113)- Applies additional safety checks (line 119) to refuse removing root, home, or empty paths
The Windows root detection (
len(dir) == 3 && dir[1] == ':' && dir[2] == '\\') correctly identifies drive roots likeC:\.The CodeQL warning at line 115 is a false positive since the path is validated by
safeStateDir.cli/cmd/logs.go (1)
48-55: Path validation correctly applied to logs command.The command follows the established pattern: validate with
safeStateDir, usesafeDirfor all path operations. Combined with the existingserviceNamePatternvalidation for service names, this command has good security coverage.The CodeQL warning at line 49 is a false positive.
cli/cmd/doctor.go (2)
42-48: Path validation correctly applied for diagnostic file output.The save path for diagnostic files is properly validated via
safeStateDirbefore theos.WriteFilecall. The CodeQL warning at line 44 is a false positive.
38-38:diagnostics.Collectproperly validates paths internally and uses sanitized values for filesystem operations.The function calls
config.SecurePath(state.DataDir)at line 49 ofcollect.go, which cleans the path withfilepath.Clean()and validates it is absolute. All filesystem operations—Docker Compose commands and disk info retrieval—use only the validatedsafeDir. Non-filesystem operations (health endpoint, config redaction) do not use paths. Validation errors are captured and included in the report, and the function only proceeds with filesystem operations when the path is valid. The design is correct and secure.cli/cmd/stop.go (1)
33-48: LGTM! Consistent use ofsafeStateDirfor path sanitization.The changes correctly apply the safe directory pattern:
safeStateDir(state)validates and cleans the path before any filesystem or Docker Compose operations. ThecomposePath, existence check, andcomposeRunall properly use the sanitizedsafeDir.Regarding the CodeQL alert:
SecurePathvalidates the path is absolute and appliesfilepath.Clean(), which is a reasonable mitigation for a CLI tool where the user controls their own system. CodeQL may require explicit configuration (e.g., a.codeqlmodel or// codeql[go/path-injection]=sanitizedcomment) to recognize this custom sanitizer.cli/internal/diagnostics/collect.go (2)
49-74: LGTM! Graceful degradation when path validation fails.Good approach: the diagnostic collection continues gathering information even when
SecurePathfails. Docker/Compose version info is captured in both success and failure cases (lines 59-60 and 72-73), while compose-dependent operations are correctly skipped whenpathErr != nil.Regarding the gocritic
ifElseChainsuggestion: the current structure is reasonably clear given the three distinct conditions (err != nil,pathErr == nil, fallback). A switch wouldn't improve readability here since the conditions check different variables. This can safely be ignored or suppressed with a//nolint:gocriticcomment if needed.
105-108: LGTM! Disk info collection correctly guarded.Properly skips disk info collection when the path is invalid, avoiding potential errors from
diskInfo()operating on an unsanitized path.cli/cmd/update.go (1)
96-140: LGTM! ConsistentsafeDirusage throughout container update flow.All Docker Compose operations (
pull,ps,down,up) correctly use the sanitizedsafeDir. The non-interactive path (lines 123-126) is a good UX addition with clear guidance.cli/cmd/status.go (2)
43-62: LGTM! Clean refactoring to threadsafeDirthrough helpers.Good approach: updating
printContainerStatesandprintResourceUsageto acceptdataDir stringinstead of the full state keeps these functions focused and ensures they receive the sanitized path.printHealthStatuscorrectly still receivesstatesince it needsBackendPort.
74-86: LGTM! Helper function signatures updated appropriately.Both
printContainerStatesandprintResourceUsagenow takedataDir stringand use it correctly fordocker.ComposeExecOutputcalls. This is cleaner than passing the entire state or config object.
| func fileExists(path string) bool { | ||
| _, err := os.Stat(path) | ||
| safe, err := config.SecurePath(path) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| _, err = os.Stat(safe) | ||
| return err == nil | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
fileExists silently returns false for invalid paths.
When SecurePath fails (e.g., relative path), the function returns false rather than propagating the error. This masks path validation failures as "file doesn't exist."
On line 49, this is used to check for existing config:
if existing := config.StatePath(state.DataDir); fileExists(existing) {If state.DataDir were invalid, fileExists returns false, skipping the overwrite warning. However, since buildState already validates IsAbs(dir) (line 135), this is unlikely to be an issue in practice.
Consider documenting this behavior or logging when path validation fails:
Optional: Log validation failures for debugging
func fileExists(path string) bool {
safe, err := config.SecurePath(path)
if err != nil {
+ // Path validation failed; treat as non-existent.
return false
}
_, err = os.Stat(safe)
return err == nil
}🧰 Tools
🪛 GitHub Check: CodeQL
[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.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/cmd/init.go` around lines 235 - 242, fileExists currently swallows
SecurePath errors and returns false, masking invalid-path issues (e.g., when
calling fileExists(config.StatePath(...))). Change fileExists to surface path
validation errors instead of silently returning false: either (preferred) change
its signature to return (bool, error) and return the SecurePath error to the
caller, or (alternatively) log the SecurePath failure before returning false;
update all callers (e.g., the call that checks config.StatePath(state.DataDir))
to handle the returned error and act accordingly. Ensure references to
SecurePath, fileExists, and the caller that checks StatePath are updated to
handle the new error-returning behavior.
Review fixes: - diagnostics/collect.go: hoist docker version capture, guard compose/disk with pathErr check (no duplication, no early return) - uninstall.go: compute safeDir once in runUninstall, pass string to helpers (removes dead dir=="" guard) - init.go: writeInitFiles returns (string, error), callers use sanitized path for display; fileExists doc comment documents absolute-path requirement - paths.go: platform-aware DataDir fallback (os.TempDir on Windows) - state.go: use SecurePath for loaded DataDir validation (replaces manual check) - pre-commit: add golangci-lint + go vet to pre-commit stage (not just pre-push) - gofmt: format all Go files - SKILL.md: soften xargs wording
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 @.pre-commit-config.yaml:
- Around line 76-82: The go-vet hook (id "go-vet") was added as a language:
system hook but not included in the pre-commit.ci skip list; update the
.pre-commit-config.yaml ci.skip configuration to include "go-vet" alongside the
other heavy local hooks so pre-commit.ci will ignore it (locate the hook by id
"go-vet" and add that id to the existing ci.skip array).
In `@cli/cmd/uninstall.go`:
- Around line 116-118: The deletion safety check in uninstall.go currently only
rejects "/", the user's home and drive-root like "C:\" but misses UNC share
roots; update the check to normalize dir (use filepath.Clean) and detect UNC
volumes by calling filepath.VolumeName(dir) and refusing removal when the volume
name starts with "\\" or "//" (i.e., a UNC root), in addition to the existing
checks on dir, home and drive roots so variable dir and the existing
early-return error path are extended to block UNC roots as well.
🪄 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: 25271723-c803-4d78-ae32-40fd8057c1c7
📒 Files selected for processing (6)
.claude/skills/post-merge-cleanup/SKILL.md.pre-commit-config.yamlcli/cmd/init.gocli/cmd/uninstall.gocli/internal/config/paths.gocli/internal/diagnostics/collect.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 (windows-latest)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: CLI Go code: Go 1.26+, dependencies incli/go.mod. Commands include: init, start, stop, status, logs, doctor, update, uninstall, version. Build with GoReleaser for cross-compile (linux/darwin/windows × amd64/arm64).
Go CLI: Go 1.26+, dependencies incli/go.mod. Use Cobra for commands, charmbracelet/huh for CLI UI. Build with GoReleaser.
Files:
cli/internal/config/paths.gocli/internal/diagnostics/collect.gocli/cmd/init.gocli/cmd/uninstall.go
.pre-commit-config.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Pre-commit hooks: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint (Dockerfile linting)
Files:
.pre-commit-config.yaml
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow (`.github/workflows/cli.yml`): Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile matrix: linux/darwin/windows × amd64/arm64) + vulnerability check (govulncheck) on `cli/**` changes. GoReleaser release on `v*` tags (attaches assets to existing Release Please release). Post-release pins checksums in install scripts and appends to GitHub Release notes.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
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.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to cli/**/*.go : CLI Go code: Go 1.26+, dependencies in `cli/go.mod`. Commands include: init, start, stop, status, logs, doctor, update, uninstall, version. Build with GoReleaser for cross-compile (linux/darwin/windows × amd64/arm64).
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
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.
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .pre-commit-config.yaml : Pre-commit hooks: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint (Dockerfile linting)
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow (`.github/workflows/cli.yml`): Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile matrix: linux/darwin/windows × amd64/arm64) + vulnerability check (govulncheck) on `cli/**` changes. GoReleaser release on `v*` tags (attaches assets to existing Release Please release). Post-release pins checksums in install scripts and appends to GitHub Release notes.
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to cli/**/*.go : Go CLI: Go 1.26+, dependencies in `cli/go.mod`. Use Cobra for commands, charmbracelet/huh for CLI UI. Build with GoReleaser.
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to cli/**/*.go : CLI Go code: Go 1.26+, dependencies in `cli/go.mod`. Commands include: init, start, stop, status, logs, doctor, update, uninstall, version. Build with GoReleaser for cross-compile (linux/darwin/windows × amd64/arm64).
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Go CLI build commands: `cd cli && go build -o synthorg ./main.go`, `cd cli && go test ./...`, `cd cli && go vet ./...`, `cd cli && golangci-lint run`
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .github/workflows/docker.yml : Docker CI workflow (`.github/workflows/docker.yml`): builds backend + web images, pushes to GHCR, signs with cosign. Scans: Trivy (CRITICAL = hard fail, HIGH = warn-only) + Grype (critical cutoff). 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:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .github/workflows/ci.yml : CI jobs: lint (ruff) + type-check (mypy) + test (pytest + coverage) + python-audit (pip-audit) + dockerfile-lint (hadolint) + dashboard-lint/type-check/test/build/audit (npm) run in parallel → ci-pass (gate)
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2026-03-14T18:26:05.851Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T18:26:05.851Z
Learning: Applies to .github/workflows/gitleaks.yml : Gitleaks workflow on push/PR + weekly schedule for secret scanning. Enforced by pre-commit hook locally.
Applied to files:
.pre-commit-config.yaml
🧬 Code graph analysis (3)
cli/internal/diagnostics/collect.go (2)
cli/internal/config/paths.go (2)
SecurePath(58-64)DataDir(18-31)cli/internal/docker/client.go (1)
ComposeExecOutput(100-107)
cli/cmd/init.go (2)
cli/internal/config/paths.go (2)
SecurePath(58-64)DataDir(18-31)cli/internal/compose/generate.go (2)
ParamsFromState(43-54)Generate(58-76)
cli/cmd/uninstall.go (1)
cli/internal/docker/client.go (2)
Detect(34-66)Info(22-29)
🪛 GitHub Check: CodeQL
cli/cmd/init.go
[failure] 192-192: 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] 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.
cli/cmd/uninstall.go
[failure] 115-115: 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 (8)
.pre-commit-config.yaml (1)
74-74: Good shift-left on CLI linting.Running
golangci-lintat both pre-commit and pre-push gives earlier feedback and reduces push-time surprises.cli/internal/config/paths.go (2)
21-28: Platform-aware absolute fallback is implemented correctly.Good update: this keeps
homeabsolute in all fallback branches and avoids the earlier cross-platform edge case.
51-74: Path validation is now consistently enforced before directory creation.
SecurePath+EnsureDiris a solid trust-boundary pattern and closes the path-injection surface for these call sites.cli/internal/diagnostics/collect.go (1)
49-53: Good degraded-mode behavior after path validation failure.This preserves health/config diagnostics while safely skipping path-dependent docker/disk calls when
state.DataDiris invalid.Also applies to: 62-71, 104-107
cli/cmd/uninstall.go (1)
38-55: Nice cleanup:safeDiris now resolved once and threaded through uninstall.This removes redundant checks and keeps all path-dependent operations on the validated directory.
Also applies to: 67-105
cli/cmd/init.go (2)
65-74: Good end-to-end propagation of sanitizedsafeDir.The flow now validates once, persists normalized
DataDir, and consistently uses the canonical path for compose/config output and writes.Also applies to: 184-211
238-246:fileExistsnow correctly guards path usage withSecurePath.This is a good hardening step for the pre-init overwrite check path.
.claude/skills/post-merge-cleanup/SKILL.md (1)
19-31: Docs update improves safety for destructive cleanup operations.The move to explicit branch deletion and the added final
git statuscheck are both solid improvements.Also applies to: 35-35
…tives - state.go: fall back to safeDir when config file has empty data_dir (prevents confusing "path must be absolute" error downstream) - Dismissed 11 CodeQL go/path-injection alerts as false positives — paths are validated by SecurePath() but CodeQL cannot trace taint removal through user-defined wrapper functions
- Remove -race from go-test pre-push hook (requires CGO, unavailable on Windows; CI runs -race on Linux) - Add go-vet to pre-commit.ci skip list - uninstall.go: reject UNC share roots in deletion safety check - state.go: fall back to safeDir when config has empty data_dir
There was a problem hiding this comment.
Pull request overview
This PR resolves CodeQL go/path-injection alerts in the Go CLI by introducing a SecurePath() validation function that ensures paths are absolute and cleaned before filesystem use. It applies this validation consistently across all command files and the config/state layer, superseding the previously closed PR #411.
Changes:
- Added
config.SecurePath()for path validation andsafeStateDir()cmd helper; applied at all filesystem call sites acrossstart,stop,logs,doctor,status,uninstall,init,update, anddiagnostics/collect.go - Refactored
state.go(Load/Save) anduninstall.goto use validated paths, improvedDataDir()fallback from"."to absolute paths, and enhanced the safety guard inconfirmAndRemoveDatafor UNC/drive roots - Updated
.pre-commit-config.yamlto addgo-vethook, adjust entry points formypy/pytest, and fix SKILL.md markdown lint
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cli/internal/config/paths.go |
Added SecurePath() function, updated DataDir() fallback, EnsureDir uses SecurePath |
cli/internal/config/paths_test.go |
Added 5 unit tests for SecurePath |
cli/internal/config/state.go |
Load/Save use SecurePath for path validation |
cli/cmd/root.go |
Added safeStateDir() helper |
cli/cmd/start.go |
Uses safeStateDir for validated path |
cli/cmd/stop.go |
Uses safeStateDir for validated path |
cli/cmd/logs.go |
Uses safeStateDir for validated path |
cli/cmd/doctor.go |
Uses safeStateDir for validated path |
cli/cmd/status.go |
Threads safeDir through helper functions |
cli/cmd/init.go |
writeInitFiles returns sanitized path; fileExists uses SecurePath |
cli/cmd/update.go |
Uses safeStateDir for validated path |
cli/cmd/uninstall.go |
Refactored to pass dataDir string; enhanced safety guard for UNC/drive roots |
cli/internal/diagnostics/collect.go |
Validates path; gracefully degrades if path invalid |
.pre-commit-config.yaml |
Added go-vet hook, removed -race from go test, adjusted mypy/pytest entries |
.claude/skills/post-merge-cleanup/SKILL.md |
Markdown lint fixes for branch cleanup instructions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| isUNC := strings.HasPrefix(vol, `\\`) || strings.HasPrefix(vol, "//") | ||
| isDriveRoot := len(dir) == 3 && dir[1] == ':' && (dir[2] == '\\' || dir[2] == '/') | ||
| if dir == "/" || dir == home || isDriveRoot || isUNC { |
| - id: go-test | ||
| name: go test (CLI) | ||
| entry: bash -c 'cd cli && go test -race ./...' | ||
| entry: bash -c 'cd cli && go test ./...' |
| if removeData { | ||
| dir := state.DataDir | ||
| // Safety: refuse to remove root, home, or empty paths. | ||
| dir := filepath.Clean(dataDir) |
There was a problem hiding this comment.
Redundant filepath.Clean on already-sanitized path
dataDir reaches this point having already been cleaned by config.SecurePath inside safeStateDir. Calling filepath.Clean again is a no-op and may mislead future readers into thinking dataDir could still be uncleaned at this point.
| dir := filepath.Clean(dataDir) | |
| dir := dataDir |
Prompt To Fix With AI
This is a comment left during a code review.
Path: cli/cmd/uninstall.go
Line: 115
Comment:
**Redundant `filepath.Clean` on already-sanitized path**
`dataDir` reaches this point having already been cleaned by `config.SecurePath` inside `safeStateDir`. Calling `filepath.Clean` again is a no-op and may mislead future readers into thinking `dataDir` could still be uncleaned at this point.
```suggestion
dir := dataDir
```
How can I resolve this? If you propose a fix, please make it concise.| home, _ := os.UserHomeDir() | ||
| if dir == "" || dir == "/" || dir == home || (len(dir) == 3 && dir[1] == ':' && dir[2] == '\\') { | ||
| vol := filepath.VolumeName(dir) | ||
| isUNC := strings.HasPrefix(vol, `\\`) || strings.HasPrefix(vol, "//") | ||
| isDriveRoot := len(dir) == 3 && dir[1] == ':' && (dir[2] == '\\' || dir[2] == '/') | ||
| if dir == "/" || dir == home || isDriveRoot || isUNC { |
There was a problem hiding this comment.
Home directory guard silently disabled when UserHomeDir fails
os.UserHomeDir() errors are swallowed with home, _ := os.UserHomeDir(). When it fails, home is "". Since dir is guaranteed to be absolute (and therefore non-empty) by SecurePath, the check dir == home can never fire in the failure case — the home directory guard is silently bypassed. A user whose $HOME is misconfigured would have no protection against accidentally uninstalling their home directory if the app's data dir were ever set to equal home.
Consider logging or handling the error:
home, homeErr := os.UserHomeDir()
if homeErr != nil {
return fmt.Errorf("cannot determine home directory for safety check: %w", homeErr)
}Prompt To Fix With AI
This is a comment left during a code review.
Path: cli/cmd/uninstall.go
Line: 117-121
Comment:
**Home directory guard silently disabled when `UserHomeDir` fails**
`os.UserHomeDir()` errors are swallowed with `home, _ := os.UserHomeDir()`. When it fails, `home` is `""`. Since `dir` is guaranteed to be absolute (and therefore non-empty) by `SecurePath`, the check `dir == home` can never fire in the failure case — the home directory guard is silently bypassed. A user whose `$HOME` is misconfigured would have no protection against accidentally uninstalling their home directory if the app's data dir were ever set to equal home.
Consider logging or handling the error:
```go
home, homeErr := os.UserHomeDir()
if homeErr != nil {
return fmt.Errorf("cannot determine home directory for safety check: %w", homeErr)
}
```
How can I resolve this? If you propose a fix, please make it concise.🤖 I have created a release *beep* *boop* --- ## [0.2.0](v0.1.4...v0.2.0) (2026-03-15) ##First probably usable release? Most likely not no and everything will break ### Features * add /get/ installation page for CLI installer ([#413](#413)) ([6a47e4a](6a47e4a)) * add cross-platform Go CLI for container lifecycle management ([#401](#401)) ([0353d9e](0353d9e)), closes [#392](#392) * add explicit ScanOutcome signal to OutputScanResult ([#394](#394)) ([be33414](be33414)), closes [#284](#284) * add meeting scheduler, event-triggered meetings, and Go CLI lint fixes ([#407](#407)) ([5550fa1](5550fa1)) * wire MultiAgentCoordinator into runtime ([#396](#396)) ([7a9e516](7a9e516)) ### Bug Fixes * CLA signatures branch + declutter repo root ([#409](#409)) ([cabe953](cabe953)) * correct Release Please branch name in release workflow ([#410](#410)) ([515d816](515d816)) * replace slsa-github-generator with attest-build-provenance, fix DAST ([#424](#424)) ([eeaadff](eeaadff)) * resolve CodeQL path-injection alerts in Go CLI ([#412](#412)) ([f41bf16](f41bf16)) ### Refactoring * rename package from ai_company to synthorg ([#422](#422)) ([df27c6e](df27c6e)), closes [#398](#398) ### Tests * add fuzz and property-based testing across all layers ([#421](#421)) ([115a742](115a742)) ### CI/CD * add SLSA L3 provenance for CLI binaries and container images ([#423](#423)) ([d3dc75d](d3dc75d)) * bump the major group with 4 updates ([#405](#405)) ([20c7a04](20c7a04)) ### Maintenance * bump github.com/spf13/cobra from 1.9.1 to 1.10.2 in /cli in the minor-and-patch group ([#402](#402)) ([e31edbb](e31edbb)) * narrow BSL Additional Use Grant and add CLA ([#408](#408)) ([5ab15bd](5ab15bd)), closes [#406](#406) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Resolves all 11 CodeQL
go/path-injectionalerts plus reviewer findings from two review rounds (6 reviewers: go-reviewer, security-reviewer, CodeRabbit, Copilot, Gemini, Greptile).config.SecurePath()— validates absolute +filepath.Clean(with trust boundary doc comment)safeStateDir()cmd helper for consistent usage across all commandsEnsureDir,Load,Save, and all cmd files (start,stop,logs,doctor,status,uninstall,init,update)diagnostics/collect.go— don't early-return on path error (health/config collection still useful)status.go— threadsafeDirthrough helper functions instead of passing rawstatestate.goLoadSecurePathconsistently for loaded DataDir validationDataDir()fallback from "." toos.Getwd()(absolute path)SecurePathSupersedes #411 (closed).
Test plan
go vet ./...passesgo build ./...passesgo test ./...— all tests pass (including new SecurePath tests)