From 63607f600bdfe45dad9f7fd68434588d34898e36 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 6 May 2026 09:39:31 +0200 Subject: [PATCH 1/2] feat(httpclient): forward cagent install UUID on gateway-bound requests --- pkg/httpclient/client.go | 9 +++ pkg/httpclient/client_test.go | 91 ++++++++++++++++++++++++++++++ pkg/telemetry/utils.go | 55 +++--------------- pkg/userid/userid.go | 92 ++++++++++++++++++++++++++++++ pkg/userid/userid_test.go | 102 ++++++++++++++++++++++++++++++++++ 5 files changed, 303 insertions(+), 46 deletions(-) create mode 100644 pkg/userid/userid.go create mode 100644 pkg/userid/userid_test.go diff --git a/pkg/httpclient/client.go b/pkg/httpclient/client.go index f968ec984..bb256c7b7 100644 --- a/pkg/httpclient/client.go +++ b/pkg/httpclient/client.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "github.com/docker/docker-agent/pkg/remote" + "github.com/docker/docker-agent/pkg/userid" "github.com/docker/docker-agent/pkg/version" ) @@ -71,6 +72,14 @@ func WithProxiedBaseURL(value string) Opt { o.Header.Set("X-Cagent-Arch", runtime.GOARCH) o.Header.Set("X-Cagent-Runtime", "cagent") o.Header.Set("X-Cagent-Runtime-Version", version.Version) + + // Stamp the persistent UUID identifying this cagent install so + // the gateway can correlate calls coming from the same client + // across sessions and processes. Same value as the `user_uuid` + // telemetry property; the gateway is free to ignore it. + if id := userid.Get(); id != "" { + o.Header.Set("X-Cagent-Id", id) + } } } diff --git a/pkg/httpclient/client_test.go b/pkg/httpclient/client_test.go index a9667f714..58e5f45ea 100644 --- a/pkg/httpclient/client_test.go +++ b/pkg/httpclient/client_test.go @@ -4,12 +4,41 @@ import ( "context" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/paths" + "github.com/docker/docker-agent/pkg/userid" ) +// TestMain redirects the config directory used by [userid.Get] to a +// throw-away temp dir so the package's tests, which exercise +// gateway-bound HTTP requests, never read or write the real user-uuid +// file in the developer's config dir. Individual tests can still +// override the directory and call [userid.ResetForTests] for finer +// control. +func TestMain(m *testing.M) { + //nolint:forbidigo // TestMain has no *testing.T, so t.TempDir is unavailable. + dir, err := os.MkdirTemp("", "httpclient-test-config-*") + if err != nil { + panic(err) + } + + paths.SetConfigDir(dir) + userid.ResetForTests() + + code := m.Run() + + paths.SetConfigDir("") + _ = os.RemoveAll(dir) + os.Exit(code) +} + func TestHeaders(t *testing.T) { t.Parallel() @@ -150,3 +179,65 @@ func TestContextWithSessionID_RoundTrip(t *testing.T) { ctx := ContextWithSessionID(t.Context(), "sess-xyz") assert.Equal(t, "sess-xyz", SessionIDFromContext(ctx)) } + +func TestCagentIDHeader_GatewayBoundOnly(t *testing.T) { + // Pin the persistent UUID file to a temp dir so the test does + // not touch the real config dir and the value is deterministic. + // We do not call t.Parallel because we mutate the package-level + // paths override and the userid cache. + const stored = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + withStoredUserUUID(t, stored) + + tests := []struct { + name string + opts []Opt + wantHeaderSent bool + }{ + { + name: "gateway-bound (X-Cagent-Forward set) → X-Cagent-Id sent", + opts: []Opt{WithProxiedBaseURL("https://gateway.example/v1")}, + wantHeaderSent: true, + }, + { + name: "no X-Cagent-Forward → X-Cagent-Id skipped", + opts: nil, + wantHeaderSent: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + headers := doRequest(t, tt.opts...) + + if tt.wantHeaderSent { + assert.Equal(t, stored, headers.Get("X-Cagent-Id")) + } else { + assert.Empty(t, headers.Get("X-Cagent-Id")) + } + }) + } +} + +// withStoredUserUUID seeds a fixed UUID into a temporary config dir for +// the duration of the test, so the persistent identifier surfaced by +// userid.Get is deterministic and isolated from other tests. The +// previous override is restored on cleanup so we keep the package-wide +// isolation set up by [TestMain]. +func withStoredUserUUID(t *testing.T, id string) { + t.Helper() + + _, err := uuid.Parse(id) + require.NoError(t, err, "seeded value must be a valid UUID") + + previous := paths.GetConfigDir() + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "user-uuid"), []byte(id), 0o600)) + + paths.SetConfigDir(dir) + userid.ResetForTests() + t.Cleanup(func() { + paths.SetConfigDir(previous) + userid.ResetForTests() + }) +} diff --git a/pkg/telemetry/utils.go b/pkg/telemetry/utils.go index 10ec92fa3..ac7ebd04b 100644 --- a/pkg/telemetry/utils.go +++ b/pkg/telemetry/utils.go @@ -6,13 +6,9 @@ import ( "flag" "fmt" "os" - "path/filepath" "runtime" - "strings" - "github.com/google/uuid" - - "github.com/docker/docker-agent/pkg/paths" + "github.com/docker/docker-agent/pkg/userid" ) // getSystemInfo collects system information for events @@ -41,48 +37,15 @@ func getTelemetryEnabledFromEnv() bool { return true } -// getUserUUIDFilePath returns the path to the user UUID file -func getUserUUIDFilePath() string { - configDir := paths.GetConfigDir() - return filepath.Join(configDir, "user-uuid") -} - -// getUserUUID gets or creates a persistent user UUID +// getUserUUID returns the persistent UUID identifying this cagent +// installation, generating and persisting one on first use. +// +// It delegates to [userid.Get], which is also used by the HTTP +// transport so the same identifier appears as the `user_uuid` +// telemetry property and as the `X-Cagent-Id` header on gateway-bound +// requests. func getUserUUID() string { - uuidFile := getUserUUIDFilePath() - - // Try to read existing UUID - if data, err := os.ReadFile(uuidFile); err == nil { - existingUUID := strings.TrimSpace(string(data)) - if existingUUID != "" { - return existingUUID - } - // UUID file exists but is empty/invalid - will generate new one - } - - // Generate new UUID and save it - newUUID := uuid.New().String() - if err := saveUserUUID(newUUID); err != nil { - // If we can't save, still return a UUID for this session - // but it won't persist across runs - return newUUID - } - - return newUUID -} - -// saveUserUUID saves the UUID to disk -func saveUserUUID(newUUID string) error { - uuidFile := getUserUUIDFilePath() - - // Ensure directory exists - dir := filepath.Dir(uuidFile) - if err := os.MkdirAll(dir, 0o755); err != nil { - return err - } - - // Write UUID to file (readable only by user) - return os.WriteFile(uuidFile, []byte(newUUID), 0o600) + return userid.Get() } // structToMap converts a struct to map[string]any using JSON marshaling diff --git a/pkg/userid/userid.go b/pkg/userid/userid.go new file mode 100644 index 000000000..bc72ff85e --- /dev/null +++ b/pkg/userid/userid.go @@ -0,0 +1,92 @@ +// Package userid exposes the persistent UUID identifying this cagent +// installation. The value is stored in `$configDir/user-uuid`, generated +// lazily on first use, and shared across cagent runs on the same machine. +// +// It is consumed both by telemetry (as the `user_uuid` event property) +// and by the HTTP transport (as the `X-Cagent-Id` header on +// gateway-bound requests) so that the gateway can correlate calls made +// by the same cagent install without having to invent a new identifier. +package userid + +import ( + "os" + "path/filepath" + "strings" + "sync" + + "github.com/google/uuid" + + "github.com/docker/docker-agent/pkg/paths" +) + +// fileName is the basename of the file holding the persistent UUID, +// stored under [paths.GetConfigDir]. +const fileName = "user-uuid" + +var ( + mu sync.Mutex + cached string +) + +// Get returns the persistent UUID identifying this cagent installation. +// +// On the first call it tries to read the value from +// `$configDir/user-uuid`; if the file does not exist, is empty, or +// cannot be read, a fresh UUID is generated and persisted (best +// effort). The result is cached in memory for the lifetime of the +// process so subsequent calls do not touch the filesystem. +func Get() string { + mu.Lock() + defer mu.Unlock() + + if cached != "" { + return cached + } + + file := filePath() + + if data, err := os.ReadFile(file); err == nil { + if existing := strings.TrimSpace(string(data)); existing != "" { + // Validate that the stored value is actually a valid UUID. + // If the file was manually edited or corrupted, regenerate + // rather than propagating invalid data to telemetry and + // the gateway. + if _, err := uuid.Parse(existing); err == nil { + cached = existing + return cached + } + // File contains invalid UUID — fall through and regenerate. + } + // File exists but is empty/whitespace — fall through and + // regenerate so we always return a valid UUID. + } + + id := uuid.New().String() + // Best-effort persistence: even if we cannot save the value to + // disk we still cache it in memory so the same identifier is used + // for the rest of this process. + _ = save(file, id) + cached = id + return cached +} + +// ResetForTests clears the in-memory cache. Tests in any package +// that rely on a deterministic config dir override should call this +// after [paths.SetConfigDir] to force the next [Get] call to re-read +// from disk. +func ResetForTests() { + mu.Lock() + defer mu.Unlock() + cached = "" +} + +func filePath() string { + return filepath.Join(paths.GetConfigDir(), fileName) +} + +func save(file, id string) error { + if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil { + return err + } + return os.WriteFile(file, []byte(id), 0o600) +} diff --git a/pkg/userid/userid_test.go b/pkg/userid/userid_test.go new file mode 100644 index 000000000..ef179a32a --- /dev/null +++ b/pkg/userid/userid_test.go @@ -0,0 +1,102 @@ +package userid + +import ( + "os" + "path/filepath" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/paths" +) + +func TestGet_GeneratesAndPersistsUUID(t *testing.T) { + dir := t.TempDir() + useConfigDir(t, dir) + + id := Get() + + require.NotEmpty(t, id) + _, err := uuid.Parse(id) + require.NoError(t, err, "Get must return a valid UUID") + + data, err := os.ReadFile(filepath.Join(dir, fileName)) + require.NoError(t, err) + assert.Equal(t, id, string(data), "Get must persist the UUID to disk") +} + +func TestGet_ReturnsExistingUUID(t *testing.T) { + dir := t.TempDir() + useConfigDir(t, dir) + + const stored = "11111111-2222-3333-4444-555555555555" + require.NoError(t, os.WriteFile(filepath.Join(dir, fileName), []byte(stored+"\n"), 0o600)) + + assert.Equal(t, stored, Get(), "Get must return the persisted UUID, trimmed") +} + +func TestGet_RegeneratesOnEmptyFile(t *testing.T) { + dir := t.TempDir() + useConfigDir(t, dir) + + require.NoError(t, os.WriteFile(filepath.Join(dir, fileName), []byte(" \n"), 0o600)) + + id := Get() + require.NotEmpty(t, id) + _, err := uuid.Parse(id) + require.NoError(t, err, "Get must regenerate when the existing file is blank") +} + +func TestGet_CachesAcrossCalls(t *testing.T) { + dir := t.TempDir() + useConfigDir(t, dir) + + first := Get() + + // Mutating the file on disk after the first call must not change + // the value returned by subsequent calls (it is served from the + // in-memory cache). + require.NoError(t, os.WriteFile(filepath.Join(dir, fileName), []byte("changed-on-disk"), 0o600)) + + assert.Equal(t, first, Get(), "Get must return the cached value on subsequent calls") +} + +// useConfigDir points paths.GetConfigDir at dir for the duration of the +// test and resets the in-memory cache so [Get] is forced to re-read +// from disk. The override is removed and the cache is cleared on +// cleanup so subsequent tests start fresh. +// +// These tests intentionally do not call [t.Parallel] because they +// share package-level mutable state (the cached UUID and the global +// config-dir override). +func useConfigDir(t *testing.T, dir string) { + t.Helper() + + paths.SetConfigDir(dir) + ResetForTests() + + t.Cleanup(func() { + paths.SetConfigDir("") + ResetForTests() + }) +} + +func TestGet_RegeneratesOnInvalidUUID(t *testing.T) { + dir := t.TempDir() + useConfigDir(t, dir) + + // Write an invalid UUID to the file (e.g., manually corrupted) + require.NoError(t, os.WriteFile(filepath.Join(dir, fileName), []byte("not-a-valid-uuid"), 0o600)) + + id := Get() + require.NotEmpty(t, id) + _, err := uuid.Parse(id) + require.NoError(t, err, "Get must regenerate when the existing file contains an invalid UUID") + + // Verify the new valid UUID was persisted + data, err := os.ReadFile(filepath.Join(dir, fileName)) + require.NoError(t, err) + assert.Equal(t, id, string(data), "Get must persist the regenerated UUID") +} From deec8cb680ab2ac44e29269fca717ffae63a35d1 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 6 May 2026 11:34:42 +0200 Subject: [PATCH 2/2] fix(userid): tighten config dir perms to 0o700 to match file 0o600 The save helper created the config directory with 0o755, letting any local user enumerate it and observe that the user-uuid file exists (and its mtime via ls -la), even though they cannot read its 0o600-protected contents. Since the UUID is a persistent per-install identifier forwarded as X-Cagent-Id on every gateway request, directory-level enumeration on a shared/multi-user host is a mild privacy leak. Use 0o700 on the directory to match the 0o600 protection on the file itself, consistent with other sensitive paths in the codebase (sandbox tokens, sqlite stores, log files). Assisted-By: docker-agent --- pkg/userid/userid.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/userid/userid.go b/pkg/userid/userid.go index bc72ff85e..39990d4b6 100644 --- a/pkg/userid/userid.go +++ b/pkg/userid/userid.go @@ -85,7 +85,11 @@ func filePath() string { } func save(file, id string) error { - if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil { + // Use 0o700 on the directory to match the 0o600 protection on the + // file itself: the per-install UUID is forwarded as `X-Cagent-Id` + // on every gateway request, so even directory-level enumeration on + // a shared host is a mild privacy leak we'd like to avoid. + if err := os.MkdirAll(filepath.Dir(file), 0o700); err != nil { return err } return os.WriteFile(file, []byte(id), 0o600)