diff --git a/pkg/history/history.go b/pkg/history/history.go index 16437eac8..774f3d722 100644 --- a/pkg/history/history.go +++ b/pkg/history/history.go @@ -9,6 +9,8 @@ import ( "path/filepath" "slices" "strings" + + "github.com/docker/portcullis" ) // History is the in-memory view of a persistent message history. The cursor @@ -43,8 +45,12 @@ func New(baseDir string) (*History, error) { } // Add records a new message. Any prior occurrence of the same message is -// removed and the new one becomes the most recent entry. +// removed and the new one becomes the most recent entry. The message is +// scrubbed of secret material via [portcullis.Redact] before being stored +// in memory or written to disk so secrets pasted into the prompt never +// linger in the persistent history. func (h *History) Add(message string) error { + message = portcullis.Redact(message) h.addInMemory(message) h.current = len(h.Messages) return h.append(message) @@ -162,7 +168,10 @@ func (h *History) load() error { if err := json.Unmarshal(line, &msg); err != nil { continue } - h.addInMemory(msg) + // Redact on read so a history file that predates redaction + // (or was tampered with externally) never exposes secrets to + // the in-memory navigation cursor or downstream callers. + h.addInMemory(portcullis.Redact(msg)) } return nil } @@ -188,7 +197,7 @@ func (h *History) migrateOldHistory(baseDir string) error { } for _, msg := range old.Messages { - if err := h.append(msg); err != nil { + if err := h.append(portcullis.Redact(msg)); err != nil { return err } } diff --git a/pkg/history/history_test.go b/pkg/history/history_test.go index b5a8f8a4c..be250601a 100644 --- a/pkg/history/history_test.go +++ b/pkg/history/history_test.go @@ -5,8 +5,11 @@ import ( "path/filepath" "testing" + "github.com/docker/portcullis" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/internal/portcullistest" ) func TestNew(t *testing.T) { @@ -472,3 +475,62 @@ func TestHistory_VeryLongMessage(t *testing.T) { assert.Equal(t, longStr, h2.Messages[0]) assert.Equal(t, "short message after", h2.Messages[1]) } + +func TestHistory_RedactsOnAdd(t *testing.T) { + tmpDir := t.TempDir() + + h, err := New(tmpDir) + require.NoError(t, err) + + pat := portcullistest.FakeGitHubPAT("cxLeRrvbJfmYdUtr70xnNE3Q7Gvli4") + msg := "deploy with token " + pat + require.NoError(t, h.Add(msg)) + + require.Len(t, h.Messages, 1) + stored := h.Messages[0] + assert.NotContains(t, stored, pat, "in-memory history must not contain the secret") + assert.Contains(t, stored, portcullis.Marker) + + // On-disk file must also be redacted. + data, err := os.ReadFile(filepath.Join(tmpDir, ".cagent", "history")) + require.NoError(t, err) + assert.NotContains(t, string(data), pat, "persisted history must not contain the secret") + assert.Contains(t, string(data), portcullis.Marker) +} + +func TestHistory_RedactsOnLoad(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".cagent"), 0o700)) + histFile := filepath.Join(tmpDir, ".cagent", "history") + + pat := portcullistest.FakeGitHubPAT("cxLeRrvbJfmYdUtr70xnNE3Q7Gvli4") + // Simulate a pre-existing history file written before redaction was wired in. + require.NoError(t, os.WriteFile(histFile, []byte(`"deploy with token `+pat+"\"\n"), 0o600)) + + h, err := New(tmpDir) + require.NoError(t, err) + + require.Len(t, h.Messages, 1) + assert.NotContains(t, h.Messages[0], pat, "loaded history must not expose the secret in memory") + assert.Contains(t, h.Messages[0], portcullis.Marker) +} + +func TestHistory_RedactsOnMigrate(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".cagent"), 0o700)) + oldHistFile := filepath.Join(tmpDir, ".cagent", "history.json") + + pat := portcullistest.FakeGitHubPAT("cxLeRrvbJfmYdUtr70xnNE3Q7Gvli4") + require.NoError(t, os.WriteFile(oldHistFile, []byte(`{"messages":["leak `+pat+`"]}`), 0o600)) + + h, err := New(tmpDir) + require.NoError(t, err) + + require.Len(t, h.Messages, 1) + assert.NotContains(t, h.Messages[0], pat, "migrated history must not expose the secret") + assert.Contains(t, h.Messages[0], portcullis.Marker) + + data, err := os.ReadFile(filepath.Join(tmpDir, ".cagent", "history")) + require.NoError(t, err) + assert.NotContains(t, string(data), pat, "migrated on-disk history must not contain the secret") +}