Skip to content

Commit be39d6a

Browse files
authored
Fix security review followups from #161 (#162)
* Fix security review followups from PR #161 - Resolve symlinks on CWD in repoConfigPath and localConfigPaths so trust-boundary comparisons are consistent (both sides now resolved) - Remove redundant duplicate RequireSecureURL check after profile merge - Fix isConfigCmd docstring placement (was fused with transformCobraError) * Fail closed when symlink resolution fails in config path discovery repoConfigPath and localConfigPaths now return empty/nil when EvalSymlinks(cwd) fails, rather than falling back to the unresolved path. Prevents trust-boundary bypass under unusual filesystem conditions.
1 parent 66cab7b commit be39d6a

2 files changed

Lines changed: 20 additions & 10 deletions

File tree

internal/cli/root.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,6 @@ func NewRootCmd() *cobra.Command {
8484
}
8585
return fmt.Errorf("base_url (%s): %w\nFix with: basecamp config unset base_url", source, err)
8686
}
87-
if profileName != "" {
88-
// Re-validate: profile may have changed base_url
89-
if err := hostutil.RequireSecureURL(cfg.BaseURL); err != nil {
90-
return fmt.Errorf("base_url (from profile %q): %w\nFix with: basecamp config unset base_url", profileName, err)
91-
}
92-
}
9387
}
9488

9589
// Resolve behavior preferences: explicit flag > config > version.IsDev()
@@ -377,8 +371,6 @@ func promptForProfile(cfg *config.Config) (string, error) {
377371
return selected.ID, nil
378372
}
379373

380-
// transformCobraError transforms Cobra's default error messages to match the
381-
// Bash CLI format for consistency with existing tests and user expectations.
382374
// isConfigCmd returns true if cmd is "config" or any of its subcommands.
383375
// Used to skip HTTPS enforcement so users can repair a bad base_url.
384376
func isConfigCmd(cmd *cobra.Command) bool {
@@ -390,6 +382,8 @@ func isConfigCmd(cmd *cobra.Command) bool {
390382
return false
391383
}
392384

385+
// transformCobraError transforms Cobra's default error messages to match the
386+
// Bash CLI format for consistency with existing tests and user expectations.
393387
func transformCobraError(err error) error {
394388
msg := err.Error()
395389

internal/config/config.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,15 @@ func repoConfigPath() string {
410410
// Walk up to find .git directory, then look for .basecamp/config.json.
411411
// Bounded by $HOME: only search within the home directory tree.
412412
// If CWD is outside $HOME (e.g., /tmp), no repo config is trusted.
413-
dir, _ := os.Getwd()
413+
dir, err := os.Getwd()
414+
if err != nil {
415+
return "" // fail closed: can't determine CWD
416+
}
417+
resolved, err := filepath.EvalSymlinks(dir)
418+
if err != nil {
419+
return "" // fail closed: can't resolve symlinks for trust boundary
420+
}
421+
dir = resolved
414422
home, _ := os.UserHomeDir()
415423
if resolved, err := filepath.EvalSymlinks(home); err == nil {
416424
home = resolved
@@ -466,7 +474,15 @@ func isInsideDir(child, parent string) bool {
466474
// - Inside a git repo: only paths at or below the repo root
467475
// - Outside a git repo: only the current working directory (no parent traversal)
468476
func localConfigPaths(repoConfigPath string) []string {
469-
dir, _ := os.Getwd()
477+
dir, err := os.Getwd()
478+
if err != nil {
479+
return nil // fail closed: can't determine CWD
480+
}
481+
resolved, err := filepath.EvalSymlinks(dir)
482+
if err != nil {
483+
return nil // fail closed: can't resolve symlinks for trust boundary
484+
}
485+
dir = resolved
470486
var paths []string
471487

472488
// Determine trust boundary (resolve symlinks for reliable comparison

0 commit comments

Comments
 (0)