From 3f9970e7c633de78efeb31d5a6f4de35b84241ff Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 12 Mar 2024 14:10:58 +0000 Subject: [PATCH 1/3] fix(support): sanitize agent env --- coderd/database/dbfake/dbfake.go | 3 +++ support/support.go | 16 ++++++++++++++++ support/support_test.go | 17 +++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 1e9a3408e8571..b871089789096 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -95,6 +95,9 @@ func (b WorkspaceBuildBuilder) WithAgent(mutations ...func([]*sdkproto.Agent) [] Auth: &sdkproto.Agent_Token{ Token: b.agentToken, }, + Env: map[string]string{ + "SECRET_TOKEN": "supersecret", + }, }} for _, m := range mutations { agents = m(agents) diff --git a/support/support.go b/support/support.go index fcd260ecdc7c1..2951452c056fb 100644 --- a/support/support.go +++ b/support/support.go @@ -191,6 +191,11 @@ func WorkspaceInfo(ctx context.Context, client *codersdk.Client, log slog.Logger log.Error(ctx, "fetch workspace", slog.Error(err), slog.F("workspace_id", workspaceID)) return w } + for _, res := range ws.LatestBuild.Resources { + for _, agt := range res.Agents { + sanitizeEnv(agt.EnvironmentVariables) + } + } w.Workspace = ws eg.Go(func() error { @@ -346,3 +351,14 @@ func Run(ctx context.Context, d *Deps) (*Bundle, error) { return &b, nil } + +// sanitizeEnv modifies kvs in place and erases the values of keys containing +// the strings "secret", "token", or "pass" +func sanitizeEnv(kvs map[string]string) { + for k := range kvs { + kl := strings.ToLower(k) + if strings.Contains(kl, "secret") || strings.Contains(kl, "token") || strings.Contains(kl, "pass") { + kvs[k] = "" + } + } +} diff --git a/support/support_test.go b/support/support_test.go index c2427fadb88af..dd991da4e358a 100644 --- a/support/support_test.go +++ b/support/support_test.go @@ -5,6 +5,7 @@ import ( "context" "io" "net/http" + "strings" "testing" "time" @@ -57,6 +58,7 @@ func TestRun(t *testing.T) { require.NotEmpty(t, bun.Network.TailnetDebug) require.NotNil(t, bun.Network.NetcheckLocal) require.NotNil(t, bun.Workspace.Workspace) + assertSanitizedWorkspace(t, bun.Workspace.Workspace) require.NotEmpty(t, bun.Workspace.BuildLogs) require.NotNil(t, bun.Workspace.Agent) require.NotEmpty(t, bun.Workspace.AgentStartupLogs) @@ -92,6 +94,7 @@ func TestRun(t *testing.T) { require.NotEmpty(t, bun.Network.CoordinatorDebug) require.NotEmpty(t, bun.Network.TailnetDebug) require.NotNil(t, bun.Workspace) + assertSanitizedWorkspace(t, bun.Workspace.Workspace) require.NotEmpty(t, bun.Logs) }) @@ -140,6 +143,20 @@ func assertSanitizedDeploymentConfig(t *testing.T, dc *codersdk.DeploymentConfig } } +func assertSanitizedWorkspace(t *testing.T, ws codersdk.Workspace) { + t.Helper() + for _, res := range ws.LatestBuild.Resources { + for _, agt := range res.Agents { + for k, v := range agt.EnvironmentVariables { + kl := strings.ToLower(k) + if strings.Contains(kl, "secret") || strings.Contains(kl, "token") || strings.Contains(kl, "pass") { + assert.Empty(t, v, "environment variable %q not sanitized", k) + } + } + } + } +} + func setupWorkspaceAndAgent(ctx context.Context, t *testing.T, client *codersdk.Client, db database.Store, user codersdk.CreateFirstUserResponse) (codersdk.Workspace, codersdk.WorkspaceAgent) { // This is a valid zip file zipBytes := make([]byte, 22) From da0fb1e360472bbf88137cbbf45008b800cb575e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 12 Mar 2024 14:25:30 +0000 Subject: [PATCH 2/3] redact all the things --- support/support.go | 7 +++---- support/support_test.go | 6 +----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/support/support.go b/support/support.go index 2951452c056fb..4572134dc0b0b 100644 --- a/support/support.go +++ b/support/support.go @@ -355,10 +355,9 @@ func Run(ctx context.Context, d *Deps) (*Bundle, error) { // sanitizeEnv modifies kvs in place and erases the values of keys containing // the strings "secret", "token", or "pass" func sanitizeEnv(kvs map[string]string) { - for k := range kvs { - kl := strings.ToLower(k) - if strings.Contains(kl, "secret") || strings.Contains(kl, "token") || strings.Contains(kl, "pass") { - kvs[k] = "" + for k, v := range kvs { + if v != "" { + kvs[k] = "***REDACTED***" } } } diff --git a/support/support_test.go b/support/support_test.go index dd991da4e358a..ce21bfe999c06 100644 --- a/support/support_test.go +++ b/support/support_test.go @@ -5,7 +5,6 @@ import ( "context" "io" "net/http" - "strings" "testing" "time" @@ -148,10 +147,7 @@ func assertSanitizedWorkspace(t *testing.T, ws codersdk.Workspace) { for _, res := range ws.LatestBuild.Resources { for _, agt := range res.Agents { for k, v := range agt.EnvironmentVariables { - kl := strings.ToLower(k) - if strings.Contains(kl, "secret") || strings.Contains(kl, "token") || strings.Contains(kl, "pass") { - assert.Empty(t, v, "environment variable %q not sanitized", k) - } + assert.Equal(t, "***REDACTED***", v, "environment variable %q not sanitized", k) } } } From e13e5b33e54ea2c2592a93b28ea84c2ebac06f63 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 12 Mar 2024 14:27:47 +0000 Subject: [PATCH 3/3] Update support/support.go --- support/support.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/support/support.go b/support/support.go index 4572134dc0b0b..30aa77eef2ff7 100644 --- a/support/support.go +++ b/support/support.go @@ -352,8 +352,8 @@ func Run(ctx context.Context, d *Deps) (*Bundle, error) { return &b, nil } -// sanitizeEnv modifies kvs in place and erases the values of keys containing -// the strings "secret", "token", or "pass" +// sanitizeEnv modifies kvs in place and replaces the values all non-empty keys +// with the string ***REDACTED*** func sanitizeEnv(kvs map[string]string) { for k, v := range kvs { if v != "" {