Skip to content

Commit 28730ca

Browse files
johnstcnjohnstcn
authored
fix(support): sanitize manifest (#12711)
1 parent f2a9e51 commit 28730ca

File tree

4 files changed

+31
-11
lines changed

4 files changed

+31
-11
lines changed

‎cli/support_test.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cli_test
22

33
import (
44
"archive/zip"
5+
"bytes"
56
"encoding/json"
67
"io"
78
"os"
@@ -12,6 +13,7 @@ import (
1213

1314
"tailscale.com/ipn/ipnstate"
1415

16+
"github.com/google/uuid"
1517
"github.com/stretchr/testify/require"
1618

1719
"github.com/coder/coder/v2/agent"
@@ -23,6 +25,7 @@ import (
2325
"github.com/coder/coder/v2/coderd/database/dbtime"
2426
"github.com/coder/coder/v2/codersdk"
2527
"github.com/coder/coder/v2/codersdk/agentsdk"
28+
"github.com/coder/coder/v2/provisionersdk/proto"
2629
"github.com/coder/coder/v2/tailnet"
2730
"github.com/coder/coder/v2/testutil"
2831
)
@@ -38,10 +41,15 @@ func TestSupportBundle(t *testing.T) {
3841
ctx := testutil.Context(t, testutil.WaitShort)
3942
client, db := coderdtest.NewWithDatabase(t, nil)
4043
owner := coderdtest.CreateFirstUser(t, client)
44+
randSecretValue := uuid.NewString()
4145
r := dbfake.WorkspaceBuild(t, db, database.Workspace{
4246
OrganizationID: owner.OrganizationID,
4347
OwnerID: owner.UserID,
44-
}).WithAgent().Do()
48+
}).WithAgent(func(agents []*proto.Agent) []*proto.Agent {
49+
// This should not show up in the bundle output
50+
agents[0].Env["SECRET_VALUE"] = randSecretValue
51+
return agents
52+
}).Do()
4553
ws, err := client.Workspace(ctx, r.Workspace.ID)
4654
require.NoError(t, err)
4755
tempDir := t.TempDir()
@@ -81,7 +89,7 @@ func TestSupportBundle(t *testing.T) {
8189
clitest.SetupConfig(t, client, root)
8290
err = inv.Run()
8391
require.NoError(t, err)
84-
assertBundleContents(t, path)
92+
assertBundleContents(t, path, randSecretValue)
8593
})
8694

8795
t.Run("NoWorkspace", func(t *testing.T) {
@@ -126,12 +134,13 @@ func TestSupportBundle(t *testing.T) {
126134
})
127135
}
128136

129-
func assertBundleContents(t *testing.T, path string) {
137+
func assertBundleContents(t *testing.T, path string, badValues ...string) {
130138
t.Helper()
131139
r, err := zip.OpenReader(path)
132140
require.NoError(t, err, "open zip file")
133141
defer r.Close()
134142
for _, f := range r.File {
143+
assertDoesNotContain(t, f, badValues...)
135144
switch f.Name {
136145
case "deployment/buildinfo.json":
137146
var v codersdk.BuildInfoResponse
@@ -244,3 +253,13 @@ func readBytesFromZip(t *testing.T, f *zip.File) []byte {
244253
require.NoError(t, err, "read bytes from zip")
245254
return bs
246255
}
256+
257+
func assertDoesNotContain(t *testing.T, f *zip.File, vals ...string) {
258+
t.Helper()
259+
bs := readBytesFromZip(t, f)
260+
for _, val := range vals {
261+
if bytes.Contains(bs, []byte(val)) {
262+
t.Fatalf("file %q should not contain value %q", f.Name, val)
263+
}
264+
}
265+
}

‎coderd/database/dbfake/dbfake.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,7 @@ func (b WorkspaceBuildBuilder) WithAgent(mutations ...func([]*sdkproto.Agent) []
9595
Auth: &sdkproto.Agent_Token{
9696
Token: b.agentToken,
9797
},
98-
Env: map[string]string{
99-
"SECRET_TOKEN": "supersecret",
100-
},
98+
Env: map[string]string{},
10199
}}
102100
for _, m := range mutations {
103101
agents = m(agents)

‎support/support.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ func connectedAgentInfo(ctx context.Context, client *codersdk.Client, log slog.L
407407
if err := json.NewDecoder(bytes.NewReader(manifestRes)).Decode(&a.Manifest); err != nil {
408408
return xerrors.Errorf("decode agent manifest: %w", err)
409409
}
410+
sanitizeEnv(a.Manifest.EnvironmentVariables)
410411

411412
return nil
412413
})

‎support/support_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,11 @@ func TestRun(t *testing.T) {
7373
assertNotNilNotEmpty(t, bun.Workspace.TemplateFileBase64, "workspace template file should be present")
7474
require.NotNil(t, bun.Workspace.Parameters, "workspace parameters should be present")
7575
assertNotNilNotEmpty(t, bun.Agent.Agent, "agent should be present")
76-
assertSanitizedAgent(t, *bun.Agent.Agent)
76+
assertSanitizedEnv(t, bun.Agent.Agent.EnvironmentVariables)
7777
assertNotNilNotEmpty(t, bun.Agent.ListeningPorts, "agent listening ports should be present")
7878
assertNotNilNotEmpty(t, bun.Agent.Logs, "agent logs should be present")
79+
assertNotNilNotEmpty(t, bun.Agent.Manifest, "agent manifest should be present")
80+
assertSanitizedEnv(t, bun.Agent.Manifest.EnvironmentVariables)
7981
assertNotNilNotEmpty(t, bun.Agent.AgentMagicsockHTML, "agent magicsock should be present")
8082
assertNotNilNotEmpty(t, bun.Agent.ClientMagicsockHTML, "client magicsock should be present")
8183
assertNotNilNotEmpty(t, bun.Agent.PeerDiagnostics, "agent peer diagnostics should be present")
@@ -164,15 +166,15 @@ func assertSanitizedWorkspace(t *testing.T, ws codersdk.Workspace) {
164166
t.Helper()
165167
for _, res := range ws.LatestBuild.Resources {
166168
for _, agt := range res.Agents {
167-
assertSanitizedAgent(t, agt)
169+
assertSanitizedEnv(t, agt.EnvironmentVariables)
168170
}
169171
}
170172
}
171173

172-
func assertSanitizedAgent(t *testing.T, agt codersdk.WorkspaceAgent) {
174+
func assertSanitizedEnv(t *testing.T, env map[string]string) {
173175
t.Helper()
174-
for k, v := range agt.EnvironmentVariables {
175-
assert.Equal(t, "***REDACTED***", v, "agent %q environment variable %q not sanitized", agt.Name, k)
176+
for k, v := range env {
177+
assert.Equal(t, "***REDACTED***", v, "environment variable %q not sanitized", k)
176178
}
177179
}
178180

0 commit comments

Comments
 (0)