Skip to content

Commit a3ff017

Browse files
committed
Tighten config-dir perms and validate plugin scope argv
- Create the global config dir at 0700 (it can hold credentials.json) in the skill-refresh and update-notice bootstrap paths, and chmod it once early in root.go so an existing 0755 dir from an older version is tightened. The chmod Lstats first and only touches a real directory (never a symlink), since GlobalConfigDir can fall back to TempDir where a local user could plant one. - Whitelist the plugin --scope value {user,project,local,global} read from installed_plugins.json before passing it to 'claude plugin uninstall', preventing a '-'-leading value from injecting a flag into the argv.
1 parent 3f44637 commit a3ff017

5 files changed

Lines changed: 58 additions & 2 deletions

File tree

internal/cli/root.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,20 @@ func NewRootCmd() *cobra.Command {
4444
return nil
4545
}
4646

47+
// Tighten global config dir perms: an older CLI version (or the
48+
// skill/update bootstrap) may have created it world-listable at
49+
// 0755, yet it can hold credentials.json. Best-effort, runs before
50+
// anything writes into the dir.
51+
//
52+
// Lstat first and only chmod a real directory: os.Chmod follows
53+
// symlinks, and GlobalConfigDir() can fall back to TempDir(), so a
54+
// local attacker could plant a symlink there and redirect the chmod.
55+
if cfgDir := config.GlobalConfigDir(); cfgDir != "" {
56+
if fi, statErr := os.Lstat(cfgDir); statErr == nil && fi.IsDir() && fi.Mode()&os.ModeSymlink == 0 {
57+
_ = os.Chmod(cfgDir, 0o700) //nolint:gosec // G302: 0700 is correct for a directory (needs the execute bit) that can hold credentials.json
58+
}
59+
}
60+
4761
// Start background update check early so it runs during command execution
4862
updateCheck = commands.StartUpdateCheck()
4963

internal/commands/skill.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,8 @@ func RefreshSkillsIfVersionChanged() bool {
344344
// On transient failure, leave the sentinel stale so the next run retries.
345345
needsRefresh := baselineSkillInstalled()
346346
if !needsRefresh || refreshed {
347-
_ = os.MkdirAll(filepath.Dir(sentinelPath), 0o755) //nolint:gosec // G301: config dir
347+
// 0o700: GlobalConfigDir can hold credentials.json; keep it owner-only.
348+
_ = os.MkdirAll(filepath.Dir(sentinelPath), 0o700)
348349
_ = os.WriteFile(sentinelPath, []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret
349350
}
350351

internal/commands/update_notice.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ func writeUpdateCache(latestVersion string) {
130130
return
131131
}
132132
dir := filepath.Dir(updateCachePath())
133-
_ = os.MkdirAll(dir, 0o755) //nolint:gosec // G301: config dir
133+
// 0o700: GlobalConfigDir can hold credentials.json; keep it owner-only.
134+
_ = os.MkdirAll(dir, 0o700)
134135
_ = os.WriteFile(updateCachePath(), data, 0o644) //nolint:gosec // G306: not a secret
135136
}

internal/commands/wizard_agents.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,12 @@ func removeStaleClaudePlugins(ctx context.Context, claudePath string, plugins []
322322
if len(p.Scopes) > 0 {
323323
anyRemoved := false
324324
for _, scope := range p.Scopes {
325+
// scope comes from installed_plugins.json (not first-party).
326+
// Whitelist it so a "-"-leading value can't inject a flag into
327+
// the uninstall argv.
328+
if !validPluginScope(scope) {
329+
continue
330+
}
325331
c := exec.CommandContext(ctx, claudePath, "plugin", "uninstall", p.Key, "--scope", scope) //nolint:gosec // G204: claudePath from FindClaudeBinary
326332
if err := c.Run(); err == nil {
327333
anyRemoved = true
@@ -351,6 +357,18 @@ func removeStaleClaudePlugins(ctx context.Context, claudePath string, plugins []
351357
return removed, scopes
352358
}
353359

360+
// validPluginScope reports whether scope is one of Claude's accepted plugin
361+
// scopes. Used to gate untrusted scope values from installed_plugins.json
362+
// before they reach `claude plugin uninstall --scope <scope>`.
363+
func validPluginScope(scope string) bool {
364+
switch scope {
365+
case "user", "project", "local", "global":
366+
return true
367+
default:
368+
return false
369+
}
370+
}
371+
354372
// claudeManualInstallHint returns the two-line manual install instructions.
355373
func claudeManualInstallHint(styles *tui.Styles) (string, string) {
356374
return styles.Bold.Render(fmt.Sprintf(" claude plugin marketplace add %s", harness.ClaudeMarketplaceSource)),
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package commands
2+
3+
import "testing"
4+
5+
// TestValidPluginScope guards the argv-injection whitelist: scope values come
6+
// from installed_plugins.json (not first-party), so a "-"-leading value must
7+
// not reach `claude plugin uninstall --scope <scope>`.
8+
func TestValidPluginScope(t *testing.T) {
9+
valid := []string{"user", "project", "local", "global"}
10+
for _, s := range valid {
11+
if !validPluginScope(s) {
12+
t.Errorf("validPluginScope(%q) = false, want true", s)
13+
}
14+
}
15+
16+
invalid := []string{"", "-rf", "--force", "User", "system", "/etc", "user ", " user"}
17+
for _, s := range invalid {
18+
if validPluginScope(s) {
19+
t.Errorf("validPluginScope(%q) = true, want false", s)
20+
}
21+
}
22+
}

0 commit comments

Comments
 (0)