Skip to content

Commit eadfc11

Browse files
committed
fix: mask secrets in CI publications
1 parent 7ac2ec4 commit eadfc11

11 files changed

Lines changed: 176 additions & 14 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package provider
2+
3+
import (
4+
atmosio "github.com/cloudposse/atmos/pkg/io"
5+
"github.com/cloudposse/atmos/pkg/perf"
6+
)
7+
8+
// MaskPublishedContent applies the global secret masker before CI content
9+
// leaves the process through a human-facing provider surface.
10+
func MaskPublishedContent(content string) string {
11+
defer perf.Track(nil, "provider.MaskPublishedContent")()
12+
13+
return atmosio.MaskString(content)
14+
}

pkg/ci/internal/provider/output.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (w *FileOutputWriter) WriteSummary(content string) error {
9696
}
9797
defer f.Close()
9898

99-
_, err = f.WriteString(content)
99+
_, err = f.WriteString(MaskPublishedContent(content))
100100
if err != nil {
101101
return fmt.Errorf("%w: failed to write summary: %w", errUtils.ErrCISummaryWriteFailed, err)
102102
}

pkg/ci/internal/provider/output_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"path/filepath"
66
"testing"
77

8+
atmosio "github.com/cloudposse/atmos/pkg/io"
89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
@@ -119,6 +120,23 @@ func TestFileOutputWriter_WriteSummary(t *testing.T) {
119120
assert.Equal(t, "# Summary\n\nThis is a test.", string(content))
120121
}
121122

123+
func TestFileOutputWriter_WriteSummary_MasksRegisteredSecrets(t *testing.T) {
124+
atmosio.Reset()
125+
t.Cleanup(atmosio.Reset)
126+
127+
const secret = "ci-summary-secret-ABCD1234"
128+
atmosio.RegisterSecret(secret)
129+
130+
summaryPath := filepath.Join(t.TempDir(), "summary.md")
131+
writer := &FileOutputWriter{SummaryPath: summaryPath}
132+
require.NoError(t, writer.WriteSummary("value: postgresql://user:"+secret+"@database.example:5432/app"))
133+
134+
content, err := os.ReadFile(summaryPath)
135+
require.NoError(t, err)
136+
assert.NotContains(t, string(content), secret)
137+
assert.Contains(t, string(content), atmosio.MaskReplacement)
138+
}
139+
122140
func TestFileOutputWriter_WriteSummary_Append(t *testing.T) {
123141
tmpDir := t.TempDir()
124142
summaryPath := filepath.Join(tmpDir, "summary.md")

pkg/ci/providers/generic/check.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ import (
1212
// CreateCheckRun writes check run status to stderr and returns a synthetic CheckRun.
1313
func (p *Provider) CreateCheckRun(_ context.Context, opts *provider.CreateCheckRunOptions) (*provider.CheckRun, error) {
1414
defer perf.Track(nil, "generic.Provider.CreateCheckRun")()
15+
title := provider.MaskPublishedContent(opts.Title)
16+
summary := provider.MaskPublishedContent(opts.Summary)
1517

1618
ui.Infof("Check run created: %s [%s]", opts.Name, opts.Status)
17-
if opts.Title != "" {
18-
ui.Infof(" Title: %s", opts.Title)
19+
if title != "" {
20+
ui.Infof(" Title: %s", title)
1921
}
20-
if opts.Summary != "" {
21-
ui.Infof(" Summary: %s", opts.Summary)
22+
if summary != "" {
23+
ui.Infof(" Summary: %s", summary)
2224
}
2325

2426
id := p.nextCheckRunID.Add(1)
@@ -27,15 +29,17 @@ func (p *Provider) CreateCheckRun(_ context.Context, opts *provider.CreateCheckR
2729
ID: id,
2830
Name: opts.Name,
2931
Status: opts.Status,
30-
Title: opts.Title,
31-
Summary: opts.Summary,
32+
Title: title,
33+
Summary: summary,
3234
StartedAt: time.Now(),
3335
}, nil
3436
}
3537

3638
// UpdateCheckRun writes check run status to stderr and returns an updated CheckRun.
3739
func (p *Provider) UpdateCheckRun(_ context.Context, opts *provider.UpdateCheckRunOptions) (*provider.CheckRun, error) {
3840
defer perf.Track(nil, "generic.Provider.UpdateCheckRun")()
41+
title := provider.MaskPublishedContent(opts.Title)
42+
summary := provider.MaskPublishedContent(opts.Summary)
3943
var uiMethod func(format string, a ...interface{})
4044
var verb string
4145
switch opts.Status {
@@ -55,19 +59,19 @@ func (p *Provider) UpdateCheckRun(_ context.Context, opts *provider.UpdateCheckR
5559

5660
uiMethod("Check run %s: %s [%s]", verb, opts.Name, opts.Status)
5761

58-
if opts.Title != "" {
59-
uiMethod(" Title: %s", opts.Title)
62+
if title != "" {
63+
uiMethod(" Title: %s", title)
6064
}
61-
if opts.Summary != "" {
62-
uiMethod(" Summary: %s", opts.Summary)
65+
if summary != "" {
66+
uiMethod(" Summary: %s", summary)
6367
}
6468

6569
return &provider.CheckRun{
6670
ID: p.nextCheckRunID.Add(1),
6771
Name: opts.Name,
6872
Status: opts.Status,
6973
Conclusion: opts.Conclusion,
70-
Title: opts.Title,
71-
Summary: opts.Summary,
74+
Title: title,
75+
Summary: summary,
7276
}, nil
7377
}

pkg/ci/providers/generic/check_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/cloudposse/atmos/pkg/ci/internal/provider"
8+
atmosio "github.com/cloudposse/atmos/pkg/io"
89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
@@ -48,6 +49,25 @@ func TestCreateCheckRun(t *testing.T) {
4849
assert.NotEqual(t, first.ID, second.ID)
4950
assert.Equal(t, first.ID+1, second.ID)
5051
})
52+
53+
t.Run("masks registered secrets in published fields", func(t *testing.T) {
54+
atmosio.Reset()
55+
t.Cleanup(atmosio.Reset)
56+
57+
const secret = "generic-check-secret-ABCD1234"
58+
atmosio.RegisterSecret(secret)
59+
checkRun, err := p.CreateCheckRun(ctx, &provider.CreateCheckRunOptions{
60+
Name: "atmos/plan/test/service",
61+
Status: provider.CheckRunStatePending,
62+
Title: "credential: " + secret,
63+
Summary: "diff contains " + secret,
64+
})
65+
require.NoError(t, err)
66+
assert.NotContains(t, checkRun.Title, secret)
67+
assert.NotContains(t, checkRun.Summary, secret)
68+
assert.Contains(t, checkRun.Title, atmosio.MaskReplacement)
69+
assert.Contains(t, checkRun.Summary, atmosio.MaskReplacement)
70+
})
5171
}
5272

5373
func TestUpdateCheckRun(t *testing.T) {

pkg/ci/providers/generic/provider.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ func (w *OutputWriter) WriteOutput(key, value string) error {
165165
// WriteSummary writes content to the job summary.
166166
func (w *OutputWriter) WriteSummary(content string) error {
167167
defer perf.Track(nil, "generic.OutputWriter.WriteSummary")()
168+
content = provider.MaskPublishedContent(content)
168169

169170
if w.summaryFile != "" {
170171
// Write to file.

pkg/ci/providers/generic/provider_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"path/filepath"
66
"testing"
77

8+
atmosio "github.com/cloudposse/atmos/pkg/io"
89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
@@ -109,6 +110,23 @@ func TestOutputWriter(t *testing.T) {
109110
assert.Contains(t, string(content), "Test content")
110111
})
111112

113+
t.Run("WriteSummary masks registered secrets", func(t *testing.T) {
114+
atmosio.Reset()
115+
t.Cleanup(atmosio.Reset)
116+
117+
const secret = "generic-summary-secret-ABCD1234"
118+
atmosio.RegisterSecret(secret)
119+
120+
summaryFile := filepath.Join(t.TempDir(), "summary")
121+
w := &OutputWriter{summaryFile: summaryFile}
122+
require.NoError(t, w.WriteSummary("credential: "+secret))
123+
124+
content, err := os.ReadFile(summaryFile)
125+
require.NoError(t, err)
126+
assert.NotContains(t, string(content), secret)
127+
assert.Contains(t, string(content), atmosio.MaskReplacement)
128+
})
129+
112130
t.Run("WriteOutput without file logs debug", func(t *testing.T) {
113131
w := &OutputWriter{}
114132

pkg/ci/providers/github/checks.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func (p *Provider) setCommitStatus(ctx context.Context, owner, repo, sha, status
2828
repoStatus := &github.RepoStatus{
2929
State: github.String(state),
3030
Context: github.String(statusContext),
31-
Description: github.String(truncateDescription(description)),
31+
Description: github.String(truncateDescription(provider.MaskPublishedContent(description))),
3232
}
3333

3434
if targetURL != "" {

pkg/ci/providers/github/checks_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/stretchr/testify/require"
1717

1818
"github.com/cloudposse/atmos/pkg/ci/internal/provider"
19+
atmosio "github.com/cloudposse/atmos/pkg/io"
1920
)
2021

2122
func TestMapCheckRunStateToStatusState(t *testing.T) {
@@ -183,6 +184,49 @@ func TestProvider_CreateCheckRun(t *testing.T) {
183184
})
184185
}
185186

187+
func TestProvider_CreateCheckRun_MasksRegisteredSecrets(t *testing.T) {
188+
atmosio.Reset()
189+
t.Cleanup(atmosio.Reset)
190+
191+
const secret = "status-secret-ABCD1234"
192+
atmosio.RegisterSecret(secret)
193+
194+
var capturedRequest map[string]any
195+
mux := http.NewServeMux()
196+
mux.HandleFunc("/repos/owner/repo/statuses/abc123", func(w http.ResponseWriter, r *http.Request) {
197+
require.NoError(t, json.NewDecoder(r.Body).Decode(&capturedRequest))
198+
w.Header().Set("Content-Type", "application/json")
199+
_ = json.NewEncoder(w).Encode(map[string]any{
200+
"id": 12345,
201+
"context": capturedRequest["context"],
202+
"state": capturedRequest["state"],
203+
"description": capturedRequest["description"],
204+
})
205+
})
206+
207+
server := httptest.NewServer(mux)
208+
t.Cleanup(server.Close)
209+
serverURL, err := url.Parse(server.URL + "/")
210+
require.NoError(t, err)
211+
ghClient := github.NewClient(nil)
212+
ghClient.BaseURL = serverURL
213+
p := NewProviderWithClient(&Client{client: ghClient})
214+
215+
_, err = p.CreateCheckRun(context.Background(), &provider.CreateCheckRunOptions{
216+
Owner: "owner",
217+
Repo: "repo",
218+
SHA: "abc123",
219+
Name: "atmos/plan/test/service",
220+
Status: provider.CheckRunStatePending,
221+
Title: "credential: " + secret,
222+
})
223+
require.NoError(t, err)
224+
description, ok := capturedRequest["description"].(string)
225+
require.True(t, ok)
226+
assert.NotContains(t, description, secret)
227+
assert.Contains(t, description, atmosio.MaskReplacement)
228+
}
229+
186230
func TestProvider_UpdateCheckRun(t *testing.T) {
187231
t.Run("update is idempotent CreateStatus call", func(t *testing.T) {
188232
// UpdateCheckRun should call the same CreateStatus endpoint.

pkg/ci/providers/github/comments.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ func (p *Provider) postComment(ctx context.Context, opts *provider.PostCommentOp
3939
if err := validatePostCommentOptions(opts); err != nil {
4040
return nil, err
4141
}
42+
maskedOpts := *opts
43+
maskedOpts.Body = provider.MaskPublishedContent(opts.Body)
44+
if err := validatePostCommentOptions(&maskedOpts); err != nil {
45+
return nil, err
46+
}
47+
opts = &maskedOpts
4248

4349
behavior, err := normalizeBehavior(opts.Behavior)
4450
if err != nil {

0 commit comments

Comments
 (0)