fix(support): sanitize manifest (#12711)

This commit is contained in:
Cian Johnston 2024-03-21 19:55:34 +00:00 committed by GitHub
parent f2a9e515df
commit 28730ca3d8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 31 additions and 11 deletions

View File

@ -2,6 +2,7 @@ package cli_test
import ( import (
"archive/zip" "archive/zip"
"bytes"
"encoding/json" "encoding/json"
"io" "io"
"os" "os"
@ -12,6 +13,7 @@ import (
"tailscale.com/ipn/ipnstate" "tailscale.com/ipn/ipnstate"
"github.com/google/uuid"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/agent"
@ -23,6 +25,7 @@ import (
"github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/coder/v2/provisionersdk/proto"
"github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/tailnet"
"github.com/coder/coder/v2/testutil" "github.com/coder/coder/v2/testutil"
) )
@ -38,10 +41,15 @@ func TestSupportBundle(t *testing.T) {
ctx := testutil.Context(t, testutil.WaitShort) ctx := testutil.Context(t, testutil.WaitShort)
client, db := coderdtest.NewWithDatabase(t, nil) client, db := coderdtest.NewWithDatabase(t, nil)
owner := coderdtest.CreateFirstUser(t, client) owner := coderdtest.CreateFirstUser(t, client)
randSecretValue := uuid.NewString()
r := dbfake.WorkspaceBuild(t, db, database.Workspace{ r := dbfake.WorkspaceBuild(t, db, database.Workspace{
OrganizationID: owner.OrganizationID, OrganizationID: owner.OrganizationID,
OwnerID: owner.UserID, OwnerID: owner.UserID,
}).WithAgent().Do() }).WithAgent(func(agents []*proto.Agent) []*proto.Agent {
// This should not show up in the bundle output
agents[0].Env["SECRET_VALUE"] = randSecretValue
return agents
}).Do()
ws, err := client.Workspace(ctx, r.Workspace.ID) ws, err := client.Workspace(ctx, r.Workspace.ID)
require.NoError(t, err) require.NoError(t, err)
tempDir := t.TempDir() tempDir := t.TempDir()
@ -81,7 +89,7 @@ func TestSupportBundle(t *testing.T) {
clitest.SetupConfig(t, client, root) clitest.SetupConfig(t, client, root)
err = inv.Run() err = inv.Run()
require.NoError(t, err) require.NoError(t, err)
assertBundleContents(t, path) assertBundleContents(t, path, randSecretValue)
}) })
t.Run("NoWorkspace", func(t *testing.T) { t.Run("NoWorkspace", func(t *testing.T) {
@ -126,12 +134,13 @@ func TestSupportBundle(t *testing.T) {
}) })
} }
func assertBundleContents(t *testing.T, path string) { func assertBundleContents(t *testing.T, path string, badValues ...string) {
t.Helper() t.Helper()
r, err := zip.OpenReader(path) r, err := zip.OpenReader(path)
require.NoError(t, err, "open zip file") require.NoError(t, err, "open zip file")
defer r.Close() defer r.Close()
for _, f := range r.File { for _, f := range r.File {
assertDoesNotContain(t, f, badValues...)
switch f.Name { switch f.Name {
case "deployment/buildinfo.json": case "deployment/buildinfo.json":
var v codersdk.BuildInfoResponse var v codersdk.BuildInfoResponse
@ -244,3 +253,13 @@ func readBytesFromZip(t *testing.T, f *zip.File) []byte {
require.NoError(t, err, "read bytes from zip") require.NoError(t, err, "read bytes from zip")
return bs return bs
} }
func assertDoesNotContain(t *testing.T, f *zip.File, vals ...string) {
t.Helper()
bs := readBytesFromZip(t, f)
for _, val := range vals {
if bytes.Contains(bs, []byte(val)) {
t.Fatalf("file %q should not contain value %q", f.Name, val)
}
}
}

View File

@ -95,9 +95,7 @@ func (b WorkspaceBuildBuilder) WithAgent(mutations ...func([]*sdkproto.Agent) []
Auth: &sdkproto.Agent_Token{ Auth: &sdkproto.Agent_Token{
Token: b.agentToken, Token: b.agentToken,
}, },
Env: map[string]string{ Env: map[string]string{},
"SECRET_TOKEN": "supersecret",
},
}} }}
for _, m := range mutations { for _, m := range mutations {
agents = m(agents) agents = m(agents)

View File

@ -407,6 +407,7 @@ func connectedAgentInfo(ctx context.Context, client *codersdk.Client, log slog.L
if err := json.NewDecoder(bytes.NewReader(manifestRes)).Decode(&a.Manifest); err != nil { if err := json.NewDecoder(bytes.NewReader(manifestRes)).Decode(&a.Manifest); err != nil {
return xerrors.Errorf("decode agent manifest: %w", err) return xerrors.Errorf("decode agent manifest: %w", err)
} }
sanitizeEnv(a.Manifest.EnvironmentVariables)
return nil return nil
}) })

View File

@ -73,9 +73,11 @@ func TestRun(t *testing.T) {
assertNotNilNotEmpty(t, bun.Workspace.TemplateFileBase64, "workspace template file should be present") assertNotNilNotEmpty(t, bun.Workspace.TemplateFileBase64, "workspace template file should be present")
require.NotNil(t, bun.Workspace.Parameters, "workspace parameters should be present") require.NotNil(t, bun.Workspace.Parameters, "workspace parameters should be present")
assertNotNilNotEmpty(t, bun.Agent.Agent, "agent should be present") assertNotNilNotEmpty(t, bun.Agent.Agent, "agent should be present")
assertSanitizedAgent(t, *bun.Agent.Agent) assertSanitizedEnv(t, bun.Agent.Agent.EnvironmentVariables)
assertNotNilNotEmpty(t, bun.Agent.ListeningPorts, "agent listening ports should be present") assertNotNilNotEmpty(t, bun.Agent.ListeningPorts, "agent listening ports should be present")
assertNotNilNotEmpty(t, bun.Agent.Logs, "agent logs should be present") assertNotNilNotEmpty(t, bun.Agent.Logs, "agent logs should be present")
assertNotNilNotEmpty(t, bun.Agent.Manifest, "agent manifest should be present")
assertSanitizedEnv(t, bun.Agent.Manifest.EnvironmentVariables)
assertNotNilNotEmpty(t, bun.Agent.AgentMagicsockHTML, "agent magicsock should be present") assertNotNilNotEmpty(t, bun.Agent.AgentMagicsockHTML, "agent magicsock should be present")
assertNotNilNotEmpty(t, bun.Agent.ClientMagicsockHTML, "client magicsock should be present") assertNotNilNotEmpty(t, bun.Agent.ClientMagicsockHTML, "client magicsock should be present")
assertNotNilNotEmpty(t, bun.Agent.PeerDiagnostics, "agent peer diagnostics should be present") assertNotNilNotEmpty(t, bun.Agent.PeerDiagnostics, "agent peer diagnostics should be present")
@ -164,15 +166,15 @@ func assertSanitizedWorkspace(t *testing.T, ws codersdk.Workspace) {
t.Helper() t.Helper()
for _, res := range ws.LatestBuild.Resources { for _, res := range ws.LatestBuild.Resources {
for _, agt := range res.Agents { for _, agt := range res.Agents {
assertSanitizedAgent(t, agt) assertSanitizedEnv(t, agt.EnvironmentVariables)
} }
} }
} }
func assertSanitizedAgent(t *testing.T, agt codersdk.WorkspaceAgent) { func assertSanitizedEnv(t *testing.T, env map[string]string) {
t.Helper() t.Helper()
for k, v := range agt.EnvironmentVariables { for k, v := range env {
assert.Equal(t, "***REDACTED***", v, "agent %q environment variable %q not sanitized", agt.Name, k) assert.Equal(t, "***REDACTED***", v, "environment variable %q not sanitized", k)
} }
} }