mirror of
https://github.com/docker/docker-agent.git
synced 2026-11-04 10:02:15 +00:00
David Gageot
c4dc536ead
- Fix TOCTOU race condition in Cache.maybeReload by re-stating the file
under the write lock. The previous implementation stat'd the file before
acquiring the lock, then used the stale FileInfo after the lock, which
could lead to storing an outdated mtime that doesn't match the loaded
content. This could cause subsequent Lookups to miss updates in
high-concurrency scenarios.
- Clarify Windows lock semantics: the mandatory LockFileEx lock is on the
separate .lock file (not the data file), so both Unix and Windows achieve
the same advisory-lock effect for the cache operations.
- Document mtimeOf zero-value semantics: the zero time.Time{} is a sentinel
for 'file not found' and is safe to compare with time.Time.Equal.
- Add comment explaining Store's update-after-persist ordering: the in-memory
map is updated after the disk write so that persistence failures still keep
the entry in memory for the current process.
- Document resolveCachePath's lexical-only path traversal check: symlinks are
not resolved, which is acceptable for the threat model (cache paths come
from agent configs, not untrusted user input).
All tests pass with -race. No functional changes to the cache behavior.
Assisted-By: docker-agent
62 lines
2.1 KiB
Go
62 lines
2.1 KiB
Go
package teamloader
|
|
|
|
import (
|
|
"fmt"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"github.com/docker/docker-agent/pkg/cache"
|
|
"github.com/docker/docker-agent/pkg/config/latest"
|
|
)
|
|
|
|
// buildAgentCache turns a per-agent CacheConfig into a [cache.Cache]
|
|
// instance, resolving any relative path against parentDir and rejecting
|
|
// paths that try to escape it.
|
|
//
|
|
// The caller is expected to gate this on cfg.Enabled; passing a disabled
|
|
// config still returns (nil, nil) so the caller can stay symmetric.
|
|
func buildAgentCache(agentName string, cfg *latest.CacheConfig, parentDir string) (*cache.Cache, error) {
|
|
if cfg == nil || !cfg.Enabled {
|
|
return nil, nil
|
|
}
|
|
|
|
path, err := resolveCachePath(cfg.Path, parentDir)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("agent %q: %w", agentName, err)
|
|
}
|
|
|
|
c, err := cache.New(cache.Config{
|
|
Enabled: true,
|
|
CaseSensitive: cfg.CaseSensitive,
|
|
TrimSpaces: cfg.TrimSpaces,
|
|
Path: path,
|
|
})
|
|
if err != nil {
|
|
return nil, fmt.Errorf("agent %q: initializing response cache: %w", agentName, err)
|
|
}
|
|
return c, nil
|
|
}
|
|
|
|
// resolveCachePath returns path unchanged when it is empty (in-memory
|
|
// cache) or absolute; otherwise it joins it with parentDir, cleans the
|
|
// result, and refuses any path that escapes parentDir via "..".
|
|
//
|
|
// Note: This is a lexical check only and does not resolve symlinks. An
|
|
// attacker with write access to parentDir could create a symlink that
|
|
// points outside the directory tree. This is acceptable because:
|
|
// 1. The cache path comes from the agent config, not user input
|
|
// 2. An attacker with write access to the config directory already has
|
|
// significant privileges
|
|
// 3. The cache only stores agent responses, not sensitive data
|
|
func resolveCachePath(path, parentDir string) (string, error) {
|
|
if path == "" || filepath.IsAbs(path) {
|
|
return path, nil
|
|
}
|
|
resolved := filepath.Clean(filepath.Join(parentDir, path))
|
|
cleanParent := filepath.Clean(parentDir) + string(filepath.Separator)
|
|
if !strings.HasPrefix(resolved+string(filepath.Separator), cleanParent) {
|
|
return "", fmt.Errorf("cache path %q escapes parent directory", path)
|
|
}
|
|
return resolved, nil
|
|
}
|