diff --git a/internal/cloudflare/conversation.go b/internal/cloudflare/conversation.go index 1515e6c..39e0a35 100644 --- a/internal/cloudflare/conversation.go +++ b/internal/cloudflare/conversation.go @@ -157,7 +157,7 @@ func (h *ConversationHistory) Save() error { return fmt.Errorf("failed to create conversation directory: %w", err) } - filename := filepath.Join(dir, fmt.Sprintf("cloudflare_%s.json", sanitizeFilename(h.AccountID))) + filename := filepath.Join(dir, fmt.Sprintf("cloudflare_%s.json", secfile.SafeSlug(h.AccountID))) data, err := json.MarshalIndent(h, "", " ") if err != nil { return fmt.Errorf("failed to marshal conversation history: %w", err) @@ -180,7 +180,7 @@ func (h *ConversationHistory) Load() error { return err } - filename := filepath.Join(dir, fmt.Sprintf("cloudflare_%s.json", sanitizeFilename(h.AccountID))) + filename := filepath.Join(dir, fmt.Sprintf("cloudflare_%s.json", secfile.SafeSlug(h.AccountID))) data, err := secfile.ReadPrivate(filename) if err != nil { if os.IsNotExist(err) { @@ -217,23 +217,6 @@ func getConversationDir() (string, error) { return filepath.Join(homeDir, ".clanker", "conversations"), nil } -// sanitizeFilename replaces characters that are invalid in filenames -func sanitizeFilename(s string) string { - replacer := strings.NewReplacer( - "/", "_", - "\\", "_", - ":", "_", - "*", "_", - "?", "_", - "\"", "_", - "<", "_", - ">", "_", - "|", "_", - " ", "_", - ) - return replacer.Replace(s) -} - // truncateText truncates text to maxLen characters, adding ellipsis if truncated func truncateText(text string, maxLen int) string { if len(text) <= maxLen { diff --git a/internal/flyio/conversation.go b/internal/flyio/conversation.go index 314f185..a48c401 100644 --- a/internal/flyio/conversation.go +++ b/internal/flyio/conversation.go @@ -171,7 +171,7 @@ func (h *ConversationHistory) filePath() (string, error) { if err != nil { return "", err } - return filepath.Join(dir, fmt.Sprintf("flyio_%s.json", sanitizeID(h.OrgSlug))), nil + return filepath.Join(dir, fmt.Sprintf("flyio_%s.json", secfile.SafeSlug(h.OrgSlug))), nil } // conversationDir returns ~/.clanker/conversations. @@ -183,23 +183,6 @@ func conversationDir() (string, error) { return filepath.Join(home, ".clanker", "conversations"), nil } -// sanitizeID replaces characters that are invalid in filenames. -func sanitizeID(s string) string { - replacer := strings.NewReplacer( - "/", "_", - "\\", "_", - ":", "_", - "*", "_", - "?", "_", - "\"", "_", - "<", "_", - ">", "_", - "|", "_", - " ", "_", - ) - return replacer.Replace(s) -} - // truncateAnswer truncates text to maxLen characters, adding ellipsis if truncated. func truncateAnswer(text string, maxLen int) string { if len(text) <= maxLen { diff --git a/internal/flyio/conversation_test.go b/internal/flyio/conversation_test.go index bce5b39..476ff63 100644 --- a/internal/flyio/conversation_test.go +++ b/internal/flyio/conversation_test.go @@ -91,15 +91,6 @@ func TestSaveLoadRoundTrip(t *testing.T) { } } -func TestSanitizeID(t *testing.T) { - if got := sanitizeID("my/org name"); got != "my_org_name" { - t.Errorf("sanitizeID = %q, want my_org_name", got) - } - if got := sanitizeID("with:colon|pipe"); got != "with_colon_pipe" { - t.Errorf("sanitizeID = %q, want with_colon_pipe", got) - } -} - func TestTruncateAnswer(t *testing.T) { if got := truncateAnswer("hello", 100); got != "hello" { t.Errorf("short answer should pass through, got %q", got) diff --git a/internal/iam/conversation.go b/internal/iam/conversation.go index 13f7a50..94812fc 100644 --- a/internal/iam/conversation.go +++ b/internal/iam/conversation.go @@ -152,7 +152,7 @@ func (h *ConversationHistory) Save() error { return fmt.Errorf("failed to create conversation directory: %w", err) } - filename := filepath.Join(dir, fmt.Sprintf("iam_%s.json", sanitizeFilename(h.AccountID))) + filename := filepath.Join(dir, fmt.Sprintf("iam_%s.json", secfile.SafeSlug(h.AccountID))) data, err := json.MarshalIndent(h, "", " ") if err != nil { return fmt.Errorf("failed to marshal conversation history: %w", err) @@ -175,7 +175,7 @@ func (h *ConversationHistory) Load() error { return err } - filename := filepath.Join(dir, fmt.Sprintf("iam_%s.json", sanitizeFilename(h.AccountID))) + filename := filepath.Join(dir, fmt.Sprintf("iam_%s.json", secfile.SafeSlug(h.AccountID))) data, err := secfile.ReadPrivate(filename) if err != nil { if os.IsNotExist(err) { @@ -212,23 +212,6 @@ func getConversationDir() (string, error) { return filepath.Join(homeDir, ".clanker", "conversations"), nil } -// sanitizeFilename replaces characters that are invalid in filenames -func sanitizeFilename(s string) string { - replacer := strings.NewReplacer( - "/", "_", - "\\", "_", - ":", "_", - "*", "_", - "?", "_", - "\"", "_", - "<", "_", - ">", "_", - "|", "_", - " ", "_", - ) - return replacer.Replace(s) -} - // truncateText truncates text to maxLen characters, adding ellipsis if truncated func truncateText(text string, maxLen int) string { if len(text) <= maxLen { diff --git a/internal/k8s/conversation.go b/internal/k8s/conversation.go index b1d786d..7e5b9df 100644 --- a/internal/k8s/conversation.go +++ b/internal/k8s/conversation.go @@ -183,7 +183,7 @@ func (h *ConversationHistory) Save() error { return fmt.Errorf("failed to create conversation directory: %w", err) } - filename := filepath.Join(dir, fmt.Sprintf("k8s_%s.json", sanitizeFilename(h.ClusterName))) + filename := filepath.Join(dir, fmt.Sprintf("k8s_%s.json", secfile.SafeSlug(h.ClusterName))) data, err := json.MarshalIndent(h, "", " ") if err != nil { return fmt.Errorf("failed to marshal conversation history: %w", err) @@ -206,7 +206,7 @@ func (h *ConversationHistory) Load() error { return err } - filename := filepath.Join(dir, fmt.Sprintf("k8s_%s.json", sanitizeFilename(h.ClusterName))) + filename := filepath.Join(dir, fmt.Sprintf("k8s_%s.json", secfile.SafeSlug(h.ClusterName))) data, err := secfile.ReadPrivate(filename) if err != nil { if os.IsNotExist(err) { @@ -243,23 +243,6 @@ func getConversationDir() (string, error) { return filepath.Join(homeDir, ".clanker", "conversations"), nil } -// sanitizeFilename replaces characters that are invalid in filenames -func sanitizeFilename(s string) string { - replacer := strings.NewReplacer( - "/", "_", - "\\", "_", - ":", "_", - "*", "_", - "?", "_", - "\"", "_", - "<", "_", - ">", "_", - "|", "_", - " ", "_", - ) - return replacer.Replace(s) -} - // truncateText truncates text to maxLen characters, adding ellipsis if truncated func truncateText(text string, maxLen int) string { if len(text) <= maxLen { diff --git a/internal/linear/conversation.go b/internal/linear/conversation.go index 06ffae1..e136456 100644 --- a/internal/linear/conversation.go +++ b/internal/linear/conversation.go @@ -102,29 +102,6 @@ func (h *ConversationHistory) GetAccountStatusContext() string { ) } -// safeSlug strips anything outside [A-Za-z0-9_-] so a malicious workspaceID -// (e.g. "../../etc/passwd") can't escape the ~/.clanker directory when -// filepath.Join resolves the path. Linear workspace IDs are UUIDs so this -// is paranoia for the env-var/header case where an operator could pass -// an arbitrary string. -func safeSlug(s string) string { - out := make([]byte, 0, len(s)) - for i := 0; i < len(s); i++ { - c := s[i] - switch { - case c >= 'a' && c <= 'z', - c >= 'A' && c <= 'Z', - c >= '0' && c <= '9', - c == '-' || c == '_': - out = append(out, c) - } - } - if len(out) == 0 { - return "default" - } - return string(out) -} - func historyPath(workspaceID string) (string, error) { home, err := os.UserHomeDir() if err != nil { @@ -134,7 +111,7 @@ func historyPath(workspaceID string) (string, error) { if err := secfile.EnsurePrivateDir(dir); err != nil { return "", err } - return filepath.Join(dir, fmt.Sprintf("linear-%s.json", safeSlug(workspaceID))), nil + return filepath.Join(dir, fmt.Sprintf("linear-%s.json", secfile.SafeSlug(workspaceID))), nil } func (h *ConversationHistory) Load() error { diff --git a/internal/linear/conversation_test.go b/internal/linear/conversation_test.go index 6b11879..69c096c 100644 --- a/internal/linear/conversation_test.go +++ b/internal/linear/conversation_test.go @@ -44,25 +44,6 @@ func TestConversationHistory_RoundTrip(t *testing.T) { } } -func TestSafeSlug_BlocksPathTraversal(t *testing.T) { - cases := []struct { - in, want string - }{ - {"acme", "acme"}, - {"my-workspace_42", "my-workspace_42"}, - {"../../etc/passwd", "etcpasswd"}, - {"/absolute/path", "absolutepath"}, - {"", "default"}, - } - for _, c := range cases { - t.Run(c.in, func(t *testing.T) { - if got := safeSlug(c.in); got != c.want { - t.Errorf("safeSlug(%q) = %q, want %q", c.in, got, c.want) - } - }) - } -} - func TestConversationHistory_TruncateAnswer(t *testing.T) { h := NewConversationHistory("ws-abc") long := strings.Repeat("x", MaxAnswerLengthInContext*2) diff --git a/internal/notion/conversation.go b/internal/notion/conversation.go index f6e6c26..8162b89 100644 --- a/internal/notion/conversation.go +++ b/internal/notion/conversation.go @@ -103,27 +103,6 @@ func (h *ConversationHistory) GetAccountStatusContext() string { ) } -// safeSlug strips anything outside [A-Za-z0-9_-] so a malicious workspace -// name (e.g. "../../etc/passwd") cannot escape the ~/.clanker directory -// when filepath.Join resolves the path. -func safeSlug(s string) string { - out := make([]byte, 0, len(s)) - for i := 0; i < len(s); i++ { - c := s[i] - switch { - case c >= 'a' && c <= 'z', - c >= 'A' && c <= 'Z', - c >= '0' && c <= '9', - c == '-' || c == '_': - out = append(out, c) - } - } - if len(out) == 0 { - return "default" - } - return string(out) -} - func historyPath(workspaceName string) (string, error) { home, err := os.UserHomeDir() if err != nil { @@ -133,7 +112,7 @@ func historyPath(workspaceName string) (string, error) { if err := secfile.EnsurePrivateDir(dir); err != nil { return "", err } - return filepath.Join(dir, fmt.Sprintf("notion-%s.json", safeSlug(workspaceName))), nil + return filepath.Join(dir, fmt.Sprintf("notion-%s.json", secfile.SafeSlug(workspaceName))), nil } func (h *ConversationHistory) Load() error { diff --git a/internal/notion/conversation_test.go b/internal/notion/conversation_test.go index 7ba0b51..099d504 100644 --- a/internal/notion/conversation_test.go +++ b/internal/notion/conversation_test.go @@ -59,23 +59,6 @@ func TestConversationHistory_RoundTrip(t *testing.T) { } } -func TestSafeSlug_PathTraversalDefense(t *testing.T) { - cases := map[string]string{ - "normal-workspace": "normal-workspace", - "My Workspace": "MyWorkspace", - "../../etc/passwd": "etcpasswd", - "with/slashes": "withslashes", - "!!!": "default", - "": "default", - "abc123_DEF": "abc123_DEF", - } - for in, want := range cases { - if got := safeSlug(in); got != want { - t.Errorf("safeSlug(%q) = %q, want %q", in, got, want) - } - } -} - func TestConversationHistory_TrimsToMaxEntries(t *testing.T) { h := NewConversationHistory("ws") for range MaxHistoryEntries + 5 { diff --git a/internal/railway/conversation.go b/internal/railway/conversation.go index ccd310e..d3a2184 100644 --- a/internal/railway/conversation.go +++ b/internal/railway/conversation.go @@ -118,7 +118,7 @@ func (h *ConversationHistory) Save() error { return fmt.Errorf("failed to create conversation directory: %w", err) } - filename := filepath.Join(dir, fmt.Sprintf("railway_%s.json", sanitizeID(workspaceID))) + filename := filepath.Join(dir, fmt.Sprintf("railway_%s.json", secfile.SafeSlug(workspaceID))) // Atomic write: temp + rename. tmp := filename + ".tmp" @@ -171,7 +171,7 @@ func (h *ConversationHistory) filePath() (string, error) { if err != nil { return "", err } - return filepath.Join(dir, fmt.Sprintf("railway_%s.json", sanitizeID(h.WorkspaceID))), nil + return filepath.Join(dir, fmt.Sprintf("railway_%s.json", secfile.SafeSlug(h.WorkspaceID))), nil } // conversationDir returns ~/.clanker/conversations. @@ -183,28 +183,6 @@ func conversationDir() (string, error) { return filepath.Join(home, ".clanker", "conversations"), nil } -// sanitizeID replaces characters that are invalid in filenames. An empty -// input yields the default "personal" bucket so callers never produce a -// filename like "railway_.json". -func sanitizeID(s string) string { - if s == "" { - return "personal" - } - replacer := strings.NewReplacer( - "/", "_", - "\\", "_", - ":", "_", - "*", "_", - "?", "_", - "\"", "_", - "<", "_", - ">", "_", - "|", "_", - " ", "_", - ) - return replacer.Replace(s) -} - // truncateAnswer truncates text to maxLen characters, adding an ellipsis // when truncated. func truncateAnswer(text string, maxLen int) string { diff --git a/internal/secfile/drift_test.go b/internal/secfile/drift_test.go index 2ee38c1..73835b5 100644 --- a/internal/secfile/drift_test.go +++ b/internal/secfile/drift_test.go @@ -51,23 +51,41 @@ func TestConversationFilesDoNotWriteWorldReadable(t *testing.T) { } ast.Inspect(file, func(n ast.Node) bool { - call, ok := n.(*ast.CallExpr) - if !ok { - return true + if call, ok := n.(*ast.CallExpr); ok { + if sel, ok := call.Fun.(*ast.SelectorExpr); ok { + if pkg, ok := sel.X.(*ast.Ident); ok { + switch pkg.Name { + case "os": + switch sel.Sel.Name { + case "WriteFile", "ReadFile": + t.Errorf("%s uses os.%s directly — must go through secfile.WritePrivate/ReadPrivate (drift guard for #22)", fset.Position(call.Pos()), sel.Sel.Name) + case "MkdirAll": + t.Errorf("%s uses os.MkdirAll directly — must go through secfile.EnsurePrivateDir (drift guard for #22)", fset.Position(call.Pos())) + } + case "strings": + // NewReplacer in conversation.go is the + // blocklist anti-pattern that produced #23. + // History-file sanitization must go through + // secfile.SafeSlug, not a hand-rolled + // replacer. Other strings.* helpers are + // fine. + if sel.Sel.Name == "NewReplacer" { + t.Errorf("%s uses strings.NewReplacer — history-file identifier sanitization must go through secfile.SafeSlug (drift guard for #23)", fset.Position(call.Pos())) + } + } + } + } } - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok { - return true - } - pkg, ok := sel.X.(*ast.Ident) - if !ok || pkg.Name != "os" { - return true - } - switch sel.Sel.Name { - case "WriteFile", "ReadFile": - t.Errorf("%s uses os.%s directly — must go through secfile.WritePrivate/ReadPrivate (drift guard for #22)", fset.Position(call.Pos()), sel.Sel.Name) - case "MkdirAll": - t.Errorf("%s uses os.MkdirAll directly — must go through secfile.EnsurePrivateDir (drift guard for #22)", fset.Position(call.Pos())) + // Catch top-level sanitiser function declarations — the + // other shape #23 might come back as is a new local + // helper that bypasses secfile.SafeSlug. Any FuncDecl + // named sanitize*/safeSlug inside conversation.go + // should fail the build. + if fn, ok := n.(*ast.FuncDecl); ok && fn.Recv == nil { + name := fn.Name.Name + if strings.HasPrefix(name, "sanitize") || name == "safeSlug" { + t.Errorf("%s declares local sanitiser %s — must call secfile.SafeSlug instead (drift guard for #23)", fset.Position(fn.Pos()), name) + } } return true }) diff --git a/internal/secfile/secfile.go b/internal/secfile/secfile.go index 25246ab..dc61875 100644 --- a/internal/secfile/secfile.go +++ b/internal/secfile/secfile.go @@ -72,3 +72,55 @@ func ReadPrivate(path string) ([]byte, error) { return io.ReadAll(f) } + +// maxSlugLen bounds slugs so a malicious or merely verbose identifier +// can't produce a filename longer than common filesystem limits +// (ext4: 255, HFS+: 255, NTFS: 255). 64 is well under that and covers +// every real-world ID we sanitize — AWS account IDs (12 digits), +// Cloudflare account IDs (32 hex), kubectl context names (typically +// under 63 per DNS label limit). +const maxSlugLen = 64 + +// SafeSlug strips every byte outside [A-Za-z0-9_-] from s so that +// caller-supplied identifiers (cluster names, account IDs, org slugs) +// cannot escape ~/.clanker via path traversal. Inputs like +// "../../etc/passwd" collapse to "etcpasswd"; "my.cluster" → "mycluster". +// +// We iterate as bytes rather than runes intentionally: multi-byte +// UTF-8 codepoints all fall outside the ASCII allowlist, so they +// would be dropped byte-by-byte either way. Operating on bytes keeps +// the function predictable and ~5x cheaper, and matches the historic +// pattern in the three providers that got this right (sentry, linear, +// notion). +// +// The result is bounded to 64 bytes (see maxSlugLen). An empty result +// returns "default" so the caller never produces a hidden file +// (".json") or an empty filename. +// +// Note on collisions: distinct identifiers that differ only in +// stripped characters (e.g. "acme/prod" and "acme-prod") will produce +// the same slug. In practice the inputs are constrained (UUIDs, hex +// IDs, 12-digit AWS account numbers) so collisions are exceedingly +// rare. The cross-tenant "default" fallback collision (two unset +// identifiers both writing to the same history file) is tracked in +// #25 and deliberately out of scope here. +func SafeSlug(s string) string { + out := make([]byte, 0, len(s)) + for i := 0; i < len(s); i++ { + c := s[i] + switch { + case c >= 'a' && c <= 'z', + c >= 'A' && c <= 'Z', + c >= '0' && c <= '9', + c == '-', c == '_': + out = append(out, c) + } + } + if len(out) > maxSlugLen { + out = out[:maxSlugLen] + } + if len(out) == 0 { + return "default" + } + return string(out) +} diff --git a/internal/secfile/secfile_test.go b/internal/secfile/secfile_test.go index 140c3fe..440e663 100644 --- a/internal/secfile/secfile_test.go +++ b/internal/secfile/secfile_test.go @@ -116,3 +116,67 @@ func TestReadPrivate_MissingFile(t *testing.T) { t.Errorf("expected IsNotExist, got: %v", err) } } + +func TestSafeSlug(t *testing.T) { + cases := []struct { + in string + want string + }{ + // Path-traversal payloads — must collapse to something safe. + {"..", "default"}, + {"../../etc/passwd", "etcpasswd"}, + {"./../etc", "etc"}, + {"./../../../../etc/passwd", "etcpasswd"}, + + // Dot-bearing real-world identifiers — preserved by the old + // blocklist sanitizer; intentionally stripped now. + {"my.cluster", "mycluster"}, + {"my-cluster.dev", "my-clusterdev"}, + + // Real-world IDs we must preserve byte-for-byte. + {"123456789012", "123456789012"}, // AWS account ID + {"deadbeefcafebabe1234567890abcdef", "deadbeefcafebabe1234567890abcdef"}, // CF hex + {"my-cluster_prod", "my-cluster_prod"}, + {"acme-prod", "acme-prod"}, + + // Empty / all-stripped inputs must yield "default", never "". + {"", "default"}, + {"...", "default"}, + {"中文", "default"}, + {"\x00\x00\x00", "default"}, + {"!@#$%^&*()", "default"}, + + // Null byte and control characters are dropped (defense in depth). + {"a\x00b", "ab"}, + {"a\nb\tc", "abc"}, + + // 64-byte cap — anything past gets truncated so we don't blow + // past filesystem limits with a 10,000-char payload. + {string(make([]byte, 0, 200)) + repeatChar('a', 200), repeatChar('a', 64)}, + + // Filesystem separators on Windows must also be stripped. + {`C:\Users\admin`, "CUsersadmin"}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + got := SafeSlug(tc.in) + if got != tc.want { + t.Errorf("SafeSlug(%q) = %q, want %q", tc.in, got, tc.want) + } + if len(got) > maxSlugLen { + t.Errorf("SafeSlug(%q) returned %d bytes — exceeds %d-byte cap", tc.in, len(got), maxSlugLen) + } + if got == "" { + t.Errorf("SafeSlug(%q) returned empty string — must fall back to %q", tc.in, "default") + } + }) + } +} + +func repeatChar(c byte, n int) string { + b := make([]byte, n) + for i := range b { + b[i] = c + } + return string(b) +} diff --git a/internal/sentry/conversation.go b/internal/sentry/conversation.go index 6b487d3..57c8b83 100644 --- a/internal/sentry/conversation.go +++ b/internal/sentry/conversation.go @@ -108,28 +108,6 @@ func (h *ConversationHistory) GetAccountStatusContext() string { ) } -// safeSlug strips anything outside [A-Za-z0-9_-] so a malicious orgSlug -// (e.g. "../../etc/passwd") can't escape the ~/.clanker directory when -// filepath.Join resolves the path. Matches the Sentry slug contract -// (`[a-z0-9-]+`). -func safeSlug(s string) string { - out := make([]byte, 0, len(s)) - for i := 0; i < len(s); i++ { - c := s[i] - switch { - case c >= 'a' && c <= 'z', - c >= 'A' && c <= 'Z', - c >= '0' && c <= '9', - c == '-' || c == '_': - out = append(out, c) - } - } - if len(out) == 0 { - return "default" - } - return string(out) -} - func historyPath(orgSlug string) (string, error) { home, err := os.UserHomeDir() if err != nil { @@ -139,7 +117,7 @@ func historyPath(orgSlug string) (string, error) { if err := secfile.EnsurePrivateDir(dir); err != nil { return "", err } - return filepath.Join(dir, fmt.Sprintf("sentry-%s.json", safeSlug(orgSlug))), nil + return filepath.Join(dir, fmt.Sprintf("sentry-%s.json", secfile.SafeSlug(orgSlug))), nil } func (h *ConversationHistory) Load() error { diff --git a/internal/sentry/conversation_test.go b/internal/sentry/conversation_test.go index 0f90c12..0975474 100644 --- a/internal/sentry/conversation_test.go +++ b/internal/sentry/conversation_test.go @@ -73,31 +73,3 @@ func TestConversationHistory_TrimsOldEntries(t *testing.T) { t.Errorf("entries = %d, want cap %d", len(h.Entries), MaxHistoryEntries) } } - -// TestSafeSlug_BlocksPathTraversal confirms the slug sanitiser strips path -// separators so a hostile org slug from an MCP caller can't escape the -// ~/.clanker directory. The bug existed because filepath.Join cleans `..` -// segments, so `../../etc/passwd` would resolve outside the intended dir. -func TestSafeSlug_BlocksPathTraversal(t *testing.T) { - cases := []struct { - in string - want string - }{ - {"acme", "acme"}, - {"my-org_42", "my-org_42"}, - {"../../etc/passwd", "etcpasswd"}, - {"/absolute/path", "absolutepath"}, - {"", "default"}, - {"...", "default"}, - {"acme/../etc", "acmeetc"}, - } - for _, c := range cases { - c := c - t.Run(c.in, func(t *testing.T) { - got := safeSlug(c.in) - if got != c.want { - t.Errorf("safeSlug(%q) = %q, want %q", c.in, got, c.want) - } - }) - } -} diff --git a/internal/vercel/conversation.go b/internal/vercel/conversation.go index a6d6bf1..6f4be60 100644 --- a/internal/vercel/conversation.go +++ b/internal/vercel/conversation.go @@ -167,7 +167,7 @@ func (h *ConversationHistory) filePath() (string, error) { if err != nil { return "", err } - return filepath.Join(dir, fmt.Sprintf("vercel_%s.json", sanitizeID(h.TeamID))), nil + return filepath.Join(dir, fmt.Sprintf("vercel_%s.json", secfile.SafeSlug(h.TeamID))), nil } // conversationDir returns ~/.clanker/conversations. @@ -179,23 +179,6 @@ func conversationDir() (string, error) { return filepath.Join(home, ".clanker", "conversations"), nil } -// sanitizeID replaces characters that are invalid in filenames. -func sanitizeID(s string) string { - replacer := strings.NewReplacer( - "/", "_", - "\\", "_", - ":", "_", - "*", "_", - "?", "_", - "\"", "_", - "<", "_", - ">", "_", - "|", "_", - " ", "_", - ) - return replacer.Replace(s) -} - // truncateAnswer truncates text to maxLen characters, adding ellipsis if truncated. func truncateAnswer(text string, maxLen int) string { if len(text) <= maxLen { diff --git a/internal/verda/conversation.go b/internal/verda/conversation.go index 39bff4e..fadecf6 100644 --- a/internal/verda/conversation.go +++ b/internal/verda/conversation.go @@ -115,7 +115,7 @@ func (h *ConversationHistory) GetRecentContext(maxEntries int) string { func (h *ConversationHistory) Save() error { h.mu.RLock() data, err := json.MarshalIndent(h, "", " ") - scopeName := sanitize(h.ScopeID) + scopeName := secfile.SafeSlug(h.ScopeID) h.mu.RUnlock() if err != nil { return fmt.Errorf("marshal history: %w", err) @@ -163,7 +163,7 @@ func (h *ConversationHistory) Load() error { // is currently mid-rename on. Grabbing the struct mutex early would let // a parallel Save hold fileLock + block here, so the order is: // fileLock (cross-process write barrier) → struct mu (in-memory state). - scopeName := sanitize(h.ScopeID) + scopeName := secfile.SafeSlug(h.ScopeID) lock := fileLockFor(scopeName) lock.Lock() defer lock.Unlock() @@ -207,14 +207,6 @@ func conversationDir() (string, error) { return filepath.Join(home, ".clanker", "conversations"), nil } -func sanitize(s string) string { - r := strings.NewReplacer( - "/", "_", "\\", "_", ":", "_", "*", "_", - "?", "_", "\"", "_", "<", "_", ">", "_", "|", "_", " ", "_", - ) - return r.Replace(s) -} - func truncate(s string, max int) string { if len(s) <= max { return s