Skip to content

Commit 7ad0cd1

Browse files
committed
Implement "at least as secure" permission semantics and fix export directory permissions
* Separate export and runtime config save methods with appropriate permissions * Add SaveExportedConfig() for portable configs (0755/0644 permissions) * Rename permission functions to EnsureAtLeastSecureDir/EnsureAtLeastRegularDir * Implement isPermissionAcceptable() using bitwise logic for "more restrictive" checks * Add comprehensive permission acceptance tests with umask handling * Update export functionality to use regular permissions instead of secure * Reorder context.go to follow Go conventions (exported before unexported, alphabetical) * Add detailed comments explaining umask override in tests
1 parent d9139a6 commit 7ad0cd1

File tree

4 files changed

+329
-161
lines changed

4 files changed

+329
-161
lines changed

internal/cache/cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func NewCache(logger hclog.Logger, opts ...Option) (*Cache, error) {
4242

4343
// Only create cache directory if caching is enabled.
4444
if options.enabled {
45-
if err := context.EnsureRegularDir(options.dir); err != nil {
45+
if err := context.EnsureAtLeastRegularDir(options.dir); err != nil {
4646
return nil, fmt.Errorf("failed to create cache directory: %w", err)
4747
}
4848
}

internal/context/context.go

Lines changed: 152 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -23,49 +23,23 @@ const (
2323
EnvVarXDGCacheHome = "XDG_CACHE_HOME"
2424
)
2525

26+
// DefaultLoader loads execution context configurations.
27+
type DefaultLoader struct{}
28+
29+
// ExecutionContextConfig stores execution context data for all configured MCP servers.
30+
type ExecutionContextConfig struct {
31+
Servers map[string]ServerExecutionContext `toml:"servers"`
32+
filePath string `toml:"-"`
33+
}
34+
2635
// ServerExecutionContext stores execution context data for an MCP server.
2736
type ServerExecutionContext struct {
2837
Name string `toml:"-"`
2938
Args []string `toml:"args,omitempty"`
3039
Env map[string]string `toml:"env,omitempty"`
3140
}
3241

33-
func (s *ServerExecutionContext) Equals(b ServerExecutionContext) bool {
34-
if s.Name != b.Name {
35-
return false
36-
}
37-
38-
if !equalSlices(s.Args, b.Args) {
39-
return false
40-
}
41-
42-
if len(s.Env) != len(b.Env) || !maps.Equal(s.Env, b.Env) {
43-
return false
44-
}
45-
46-
return true
47-
}
48-
49-
func equalSlices(a []string, b []string) bool {
50-
if len(a) != len(b) {
51-
return false
52-
}
53-
54-
sortedA := slices.Clone(a)
55-
slices.Sort(sortedA)
56-
57-
sortedB := slices.Clone(b)
58-
slices.Sort(sortedB)
59-
60-
return slices.Equal(sortedA, sortedB)
61-
}
62-
63-
func (s *ServerExecutionContext) IsEmpty() bool {
64-
return len(s.Args) == 0 && len(s.Env) == 0
65-
}
66-
67-
type DefaultLoader struct{}
68-
42+
// Load loads an execution context configuration from the specified path.
6943
func (d *DefaultLoader) Load(path string) (Modifier, error) {
7044
path = strings.TrimSpace(path)
7145
if path == "" {
@@ -85,30 +59,7 @@ func (d *DefaultLoader) Load(path string) (Modifier, error) {
8559
return cfg, nil
8660
}
8761

88-
// ExecutionContextConfig stores execution context data for all configured MCP servers.
89-
type ExecutionContextConfig struct {
90-
Servers map[string]ServerExecutionContext `toml:"servers"`
91-
filePath string `toml:"-"`
92-
}
93-
94-
// NewExecutionContextConfig returns a newly initialized ExecutionContextConfig.
95-
func NewExecutionContextConfig(path string) *ExecutionContextConfig {
96-
return &ExecutionContextConfig{
97-
Servers: map[string]ServerExecutionContext{},
98-
filePath: strings.TrimSpace(path),
99-
}
100-
}
101-
102-
func (c *ExecutionContextConfig) List() []ServerExecutionContext {
103-
servers := slices.Collect(maps.Values(c.Servers))
104-
105-
slices.SortFunc(servers, func(a, b ServerExecutionContext) int {
106-
return cmp.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
107-
})
108-
109-
return servers
110-
}
111-
62+
// Get retrieves the execution context for the specified server name.
11263
func (c *ExecutionContextConfig) Get(name string) (ServerExecutionContext, bool) {
11364
name = strings.TrimSpace(name)
11465
if name == "" {
@@ -126,6 +77,29 @@ func (c *ExecutionContextConfig) Get(name string) (ServerExecutionContext, bool)
12677
return ServerExecutionContext{}, false
12778
}
12879

80+
// List returns all server execution contexts sorted by name.
81+
func (c *ExecutionContextConfig) List() []ServerExecutionContext {
82+
servers := slices.Collect(maps.Values(c.Servers))
83+
84+
slices.SortFunc(servers, func(a, b ServerExecutionContext) int {
85+
return cmp.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
86+
})
87+
88+
return servers
89+
}
90+
91+
// SaveConfig saves the execution context configuration to a file with secure permissions.
92+
// Used for runtime execution contexts that may contain sensitive data.
93+
func (c *ExecutionContextConfig) SaveConfig() error {
94+
return c.saveConfig(EnsureAtLeastSecureDir, perms.SecureFile)
95+
}
96+
97+
// SaveExportedConfig saves the execution context configuration to a file with regular permissions.
98+
// Used for exported configurations that are sanitized and suitable for sharing.
99+
func (c *ExecutionContextConfig) SaveExportedConfig() error {
100+
return c.saveConfig(EnsureAtLeastRegularDir, perms.RegularFile)
101+
}
102+
129103
// Upsert updates the execution context for the given server name.
130104
// If the context is empty and does not exist in config, it does nothing.
131105
// If the context is empty and previously existed in config, it deletes the entry.
@@ -169,6 +143,120 @@ func (c *ExecutionContextConfig) Upsert(ec ServerExecutionContext) (UpsertResult
169143
return op, nil
170144
}
171145

146+
// Equals checks if this ServerExecutionContext is equal to another.
147+
func (s *ServerExecutionContext) Equals(b ServerExecutionContext) bool {
148+
if s.Name != b.Name {
149+
return false
150+
}
151+
152+
if !equalSlices(s.Args, b.Args) {
153+
return false
154+
}
155+
156+
if len(s.Env) != len(b.Env) || !maps.Equal(s.Env, b.Env) {
157+
return false
158+
}
159+
160+
return true
161+
}
162+
163+
// IsEmpty returns true if the ServerExecutionContext has no args or env vars.
164+
func (s *ServerExecutionContext) IsEmpty() bool {
165+
return len(s.Args) == 0 && len(s.Env) == 0
166+
}
167+
168+
// AppDirName returns the name of the application directory for use in user-specific operations where data is being written.
169+
func AppDirName() string {
170+
return "mcpd"
171+
}
172+
173+
// EnsureAtLeastRegularDir creates a directory with standard permissions if it doesn't exist,
174+
// and verifies that it has at least the required regular permissions if it already exists.
175+
// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error.
176+
// Used for cache directories, data directories, and documentation.
177+
func EnsureAtLeastRegularDir(path string) error {
178+
return ensureAtLeastDir(path, perms.RegularDir)
179+
}
180+
181+
// EnsureAtLeastSecureDir creates a directory with secure permissions if it doesn't exist,
182+
// and verifies that it has at least the required secure permissions if it already exists.
183+
// It does not attempt to repair ownership or permissions: if they are wrong,
184+
// it returns an error.
185+
func EnsureAtLeastSecureDir(path string) error {
186+
return ensureAtLeastDir(path, perms.SecureDir)
187+
}
188+
189+
// NewExecutionContextConfig returns a newly initialized ExecutionContextConfig.
190+
func NewExecutionContextConfig(path string) *ExecutionContextConfig {
191+
return &ExecutionContextConfig{
192+
Servers: map[string]ServerExecutionContext{},
193+
filePath: strings.TrimSpace(path),
194+
}
195+
}
196+
197+
// UserSpecificCacheDir returns the directory that should be used to store any user-specific cache files.
198+
// It adheres to the XDG Base Directory Specification, respecting the XDG_CACHE_HOME environment variable.
199+
// When XDG_CACHE_HOME is not set, it defaults to ~/.cache/mcpd/
200+
// See: https://specifications.freedesktop.org/basedir-spec/latest/
201+
func UserSpecificCacheDir() (string, error) {
202+
return userSpecificDir(EnvVarXDGCacheHome, ".cache")
203+
}
204+
205+
// UserSpecificConfigDir returns the directory that should be used to store any user-specific configuration.
206+
// It adheres to the XDG Base Directory Specification, respecting the XDG_CONFIG_HOME environment variable.
207+
// When XDG_CONFIG_HOME is not set, it defaults to ~/.config/mcpd/
208+
// See: https://specifications.freedesktop.org/basedir-spec/latest/
209+
func UserSpecificConfigDir() (string, error) {
210+
return userSpecificDir(EnvVarXDGConfigHome, ".config")
211+
}
212+
213+
// ensureAtLeastDir creates a directory with the specified permissions if it doesn't exist,
214+
// and verifies that it has at least the required permissions if it already exists.
215+
// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error.
216+
func ensureAtLeastDir(path string, perm os.FileMode) error {
217+
if err := os.MkdirAll(path, perm); err != nil {
218+
return fmt.Errorf("could not ensure directory exists for '%s': %w", path, err)
219+
}
220+
221+
info, err := os.Stat(path)
222+
if err != nil {
223+
return fmt.Errorf("could not stat directory '%s': %w", path, err)
224+
}
225+
226+
if !isPermissionAcceptable(info.Mode().Perm(), perm) {
227+
return fmt.Errorf(
228+
"incorrect permissions for directory '%s' (%#o, want %#o or more restrictive)",
229+
path, info.Mode().Perm(),
230+
perm,
231+
)
232+
}
233+
234+
return nil
235+
}
236+
237+
// equalSlices compares two string slices for equality, ignoring order.
238+
func equalSlices(a []string, b []string) bool {
239+
if len(a) != len(b) {
240+
return false
241+
}
242+
243+
sortedA := slices.Clone(a)
244+
slices.Sort(sortedA)
245+
246+
sortedB := slices.Clone(b)
247+
slices.Sort(sortedB)
248+
249+
return slices.Equal(sortedA, sortedB)
250+
}
251+
252+
// isPermissionAcceptable checks if the actual permissions are acceptable for the required permissions.
253+
// It returns true if the actual permissions are equal to or more restrictive than required.
254+
// "More restrictive" means: no permission bit set in actual that isn't also set in required.
255+
func isPermissionAcceptable(actual, required os.FileMode) bool {
256+
// Check that actual doesn't grant any permissions that required doesn't grant
257+
return (actual & ^required) == 0
258+
}
259+
172260
// loadExecutionContextConfig loads a runtime execution context file from disk and expands environment variables.
173261
//
174262
// The function parses the TOML file at the specified path and automatically expands all ${VAR} references
@@ -210,21 +298,19 @@ func loadExecutionContextConfig(path string) (*ExecutionContextConfig, error) {
210298
return cfg, nil
211299
}
212300

213-
// SaveConfig writes the ExecutionContextConfig to disk as a TOML file,
214-
// creating parent directories and setting secure file permissions.
215-
func (c *ExecutionContextConfig) SaveConfig() error {
301+
// saveConfig saves the execution context configuration to a file with the specified directory and file permissions.
302+
func (c *ExecutionContextConfig) saveConfig(ensureDirFunc func(string) error, fileMode os.FileMode) error {
216303
path := c.filePath
217304
if path == "" {
218305
return fmt.Errorf("config file path not present")
219306
}
220307

221308
// Ensure the directory exists before creating the file.
222-
if err := EnsureSecureDir(filepath.Dir(path)); err != nil {
309+
if err := ensureDirFunc(filepath.Dir(path)); err != nil {
223310
return fmt.Errorf("could not ensure execution context directory exists: %w", err)
224311
}
225312

226-
// owner: rw-, group: ---, others: ---
227-
f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perms.SecureFile)
313+
f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
228314
if err != nil {
229315
return fmt.Errorf("could not create file '%s': %w", path, err)
230316
}
@@ -273,64 +359,3 @@ func userSpecificDir(envVar string, dir string) (string, error) {
273359

274360
return filepath.Join(homeDir, dir, AppDirName()), nil
275361
}
276-
277-
// UserSpecificConfigDir returns the directory that should be used to store any user-specific configuration.
278-
// It adheres to the XDG Base Directory Specification, respecting the XDG_CONFIG_HOME environment variable.
279-
// When XDG_CONFIG_HOME is not set, it defaults to ~/.config/mcpd/
280-
// See: https://specifications.freedesktop.org/basedir-spec/latest/
281-
func UserSpecificConfigDir() (string, error) {
282-
return userSpecificDir(EnvVarXDGConfigHome, ".config")
283-
}
284-
285-
// UserSpecificCacheDir returns the directory that should be used to store any user-specific cache files.
286-
// It adheres to the XDG Base Directory Specification, respecting the XDG_CACHE_HOME environment variable.
287-
// When XDG_CACHE_HOME is not set, it defaults to ~/.cache/mcpd/
288-
// See: https://specifications.freedesktop.org/basedir-spec/latest/
289-
func UserSpecificCacheDir() (string, error) {
290-
return userSpecificDir(EnvVarXDGCacheHome, ".cache")
291-
}
292-
293-
// AppDirName returns the name of the application directory for use in user-specific operations where data is being written.
294-
func AppDirName() string {
295-
return "mcpd"
296-
}
297-
298-
// ensureDir creates a directory with the specified permissions if it doesn't exist,
299-
// and verifies that it has the expected permissions if it already exists.
300-
// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error.
301-
func ensureDir(path string, perm os.FileMode) error {
302-
if err := os.MkdirAll(path, perm); err != nil {
303-
return fmt.Errorf("could not ensure directory exists for '%s': %w", path, err)
304-
}
305-
306-
info, err := os.Stat(path)
307-
if err != nil {
308-
return fmt.Errorf("could not stat directory '%s': %w", path, err)
309-
}
310-
311-
if info.Mode().Perm() != perm {
312-
return fmt.Errorf(
313-
"incorrect permissions for directory '%s' (%#o, want %#o)",
314-
path, info.Mode().Perm(),
315-
perm,
316-
)
317-
}
318-
319-
return nil
320-
}
321-
322-
// EnsureSecureDir creates a directory with secure permissions if it doesn't exist,
323-
// and verifies that it has the expected permissions if it already exists.
324-
// It does not attempt to repair ownership or permissions: if they are wrong,
325-
// it returns an error.
326-
func EnsureSecureDir(path string) error {
327-
return ensureDir(path, perms.SecureDir)
328-
}
329-
330-
// EnsureRegularDir creates a directory with standard permissions if it doesn't exist,
331-
// and verifies that it has the expected permissions if it already exists.
332-
// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error.
333-
// Used for cache directories, data directories, and documentation.
334-
func EnsureRegularDir(path string) error {
335-
return ensureDir(path, perms.RegularDir)
336-
}

0 commit comments

Comments
 (0)