Skip to content

[AUTOMATION] fix(clawpatch): address daily finding #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
michiosw wants to merge 1 commit into
base: main
Choose a base branch
from
+75 −3
Open

[AUTOMATION] fix(clawpatch): address daily finding #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions internal/guard/cli/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,8 @@ func readClaudeSettings() (string, map[string]any, error) {
}

func backupFile(path, label string) error {
if _, err := os.Stat(path); os.IsNotExist(err) {
mode, err := fileModeOrDefault(path, 0o600)
if os.IsNotExist(err) {
return nil
} else if err != nil {
return err
Expand All @@ -450,16 +451,28 @@ func backupFile(path, label string) error {
if err != nil {
return err
}
return os.WriteFile(backupPath, input, 0o644)
return os.WriteFile(backupPath, input, mode)

@greptile-apps greptile-apps Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Backup collisions keep permissions

The backup filename has only second-level precision, and this write overwrites an existing backup if two hook operations run in the same second. Since os.WriteFile does not chmod an existing file, a colliding backup that was first created with broader permissions can stay broad even when this call computes 0600; it can also replace the earlier backup contents.

}

func writeJSONFile(path string, value any) error {
mode, err := fileModeOrDefault(path, 0o600)
if err != nil && !os.IsNotExist(err) {
return err
}
bytes, err := json.MarshalIndent(value, "", " ")
if err != nil {
return err
}
bytes = append(bytes, '\n')
return os.WriteFile(path, bytes, 0o644)
return os.WriteFile(path, bytes, mode)

@greptile-apps greptile-apps Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Existing modes stay broad

os.WriteFile only applies the mode argument when it creates a new file. When settings.json already exists, this call truncates and rewrites it without changing its permissions. A user whose Claude settings were already widened to 0644 by an earlier install/uninstall will still have a world-readable settings file after running this fixed path, so the affected local privacy issue remains in place.

}

func fileModeOrDefault(path string, fallback os.FileMode) (os.FileMode, error) {
info, err := os.Stat(path)
if err != nil {
return fallback, err
}
return info.Mode().Perm(), nil
}

func mergeHooks(raw any, hookCommand string) map[string]any {
Expand Down
59 changes: 59 additions & 0 deletions internal/guard/cli/cli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,40 @@ func TestUninstallClaudeHooksPreservesNonGuardHookInMixedGroup(t *testing.T) {
if command != "/usr/local/bin/custom-hook" {
t.Fatalf("preserved command = %v, want custom hook", command)
}
assertFileMode(t, settingsPath, 0o600)
assertBackupMode(t, settingsPath, 0o600)
}

func TestInstallClaudeHooksPreservesClaudeSettingsPermissions(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
settingsPath := filepath.Join(home, ".claude", "settings.json")
if err := os.MkdirAll(filepath.Dir(settingsPath), 0o755); err != nil {
t.Fatal(err)
}
settings := `{
"hooks": {
"PreToolUse": [
{
"matcher": "Bash",
"hooks": [
{"type": "command", "command": "/usr/local/bin/custom-hook"}
]
}
]
}
}`
if err := os.WriteFile(settingsPath, []byte(settings), 0o600); err != nil {
t.Fatal(err)
}

var stdout bytes.Buffer
if err := installClaudeHooks(&stdout, "/tmp/kontext.sock"); err != nil {
t.Fatal(err)
}

assertFileMode(t, settingsPath, 0o600)
assertBackupMode(t, settingsPath, 0o600)
}

func TestPrintHookStatusReportsGuardAndHostedConflict(t *testing.T) {
Expand Down Expand Up @@ -240,6 +274,31 @@ func TestPrintHookStatusReportsGuardAndHostedConflict(t *testing.T) {
}
}

func assertFileMode(t *testing.T, path string, want os.FileMode) {
t.Helper()

info, err := os.Stat(path)
if err != nil {
t.Fatal(err)
}
if got := info.Mode().Perm(); got != want {
t.Fatalf("%s mode = %o, want %o", path, got, want)
}
}

func assertBackupMode(t *testing.T, path string, want os.FileMode) {
t.Helper()

matches, err := filepath.Glob(path + ".kontext-guard-backup-*")
if err != nil {
t.Fatal(err)
}
if len(matches) != 1 {
t.Fatalf("backup count = %d, want 1", len(matches))
}
assertFileMode(t, matches[0], want)
}

func TestValidateLocalJudgeURLRejectsHostedURL(t *testing.T) {
if err := validateLocalJudgeURL("https://api.example.com/v1"); err == nil {
t.Fatal("validateLocalJudgeURL() error = nil, want hosted URL rejection")
Expand Down
Loading