Skip to content

Commit 5d8cb06

Browse files
coadlercoadler
committed
fix: add requester IP to workspace build audit logs
Uses otel baggage to smuggle the IP into the provisioner.
1 parent 5fc9ff2 commit 5d8cb06

17 files changed

+381
-177
lines changed

‎cli/server.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ import (
4141
"github.com/prometheus/client_golang/prometheus"
4242
"github.com/prometheus/client_golang/prometheus/collectors"
4343
"github.com/prometheus/client_golang/prometheus/promhttp"
44+
"go.opentelemetry.io/otel"
45+
"go.opentelemetry.io/otel/propagation"
4446
"go.opentelemetry.io/otel/trace"
4547
"golang.org/x/mod/semver"
4648
"golang.org/x/oauth2"
@@ -2020,6 +2022,13 @@ func ConfigureTraceProvider(
20202022
sqlDriver = "postgres"
20212023
)
20222024

2025+
otel.SetTextMapPropagator(
2026+
propagation.NewCompositeTextMapPropagator(
2027+
propagation.TraceContext{},
2028+
propagation.Baggage{},
2029+
),
2030+
)
2031+
20232032
if cfg.Trace.Enable.Value() || cfg.Trace.DataDog.Value() || cfg.Trace.HoneycombAPIKey != "" {
20242033
sdkTracerProvider, _closeTracing, err := tracing.TracerProvider(ctx, "coderd", tracing.TracerOpts{
20252034
Default: cfg.Trace.Enable.Value(),

‎cli/templates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ package cli
33
import (
44
"time"
55

6-
"github.com/coder/pretty"
76
"github.com/google/uuid"
87
"golang.org/x/xerrors"
98

109
"github.com/coder/coder/v2/cli/clibase"
1110
"github.com/coder/coder/v2/cli/cliui"
1211
"github.com/coder/coder/v2/codersdk"
12+
"github.com/coder/pretty"
1313
)
1414

1515
func (r *RootCmd) templates() *clibase.Cmd {

‎cli/templateversionarchive.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ import (
66
"strings"
77
"time"
88

9-
"github.com/coder/pretty"
109
"golang.org/x/xerrors"
1110

1211
"github.com/coder/coder/v2/cli/clibase"
1312
"github.com/coder/coder/v2/cli/cliui"
1413
"github.com/coder/coder/v2/codersdk"
14+
"github.com/coder/pretty"
1515
)
1616

1717
func (r *RootCmd) unarchiveTemplateVersion() *clibase.Cmd {

‎cli/templateversions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ import (
55
"strings"
66
"time"
77

8-
"github.com/coder/pretty"
98
"github.com/google/uuid"
109
"golang.org/x/xerrors"
1110

1211
"github.com/coder/coder/v2/cli/clibase"
1312
"github.com/coder/coder/v2/cli/cliui"
1413
"github.com/coder/coder/v2/codersdk"
14+
"github.com/coder/pretty"
1515
)
1616

1717
func (r *RootCmd) templateVersions() *clibase.Cmd {

‎cli/user_delete_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ func TestUserDelete(t *testing.T) {
121121
// pw, err := cryptorand.String(16)
122122
// require.NoError(t, err)
123123

124-
// fmt.Println(aUser.OrganizationID)
125124
// toDelete, err := client.CreateUser(ctx, codersdk.CreateUserRequest{
126125
// Email: "colin5@coder.com",
127126
// Username: "coolin",

‎coderd/audit/request.go

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111

1212
"github.com/google/uuid"
1313
"github.com/sqlc-dev/pqtype"
14+
"go.opentelemetry.io/otel/baggage"
15+
"golang.org/x/xerrors"
1416

1517
"cdr.dev/slog"
1618
"github.com/coder/coder/v2/coderd/database"
@@ -54,6 +56,7 @@ type BuildAuditParams[T Auditable] struct {
5456
Status int
5557
Action database.AuditAction
5658
OrganizationID uuid.UUID
59+
IP string
5760
AdditionalFields json.RawMessage
5861

5962
New T
@@ -248,9 +251,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
248251
// WorkspaceBuildAudit creates an audit log for a workspace build.
249252
// The audit log is committed upon invocation.
250253
func WorkspaceBuildAudit[T Auditable](ctx context.Context, p *BuildAuditParams[T]) {
251-
// As the audit request has not been initiated directly by a user, we omit
252-
// certain user details.
253-
ip := parseIP("")
254+
ip := parseIP(p.IP)
254255

255256
diff := Diff(p.Audit, p.Old, p.New)
256257
var err error
@@ -280,16 +281,70 @@ func WorkspaceBuildAudit[T Auditable](ctx context.Context, p *BuildAuditParams[T
280281
RequestID: p.JobID,
281282
AdditionalFields: p.AdditionalFields,
282283
}
283-
exportErr := p.Audit.Export(ctx, auditLog)
284-
if exportErr != nil {
284+
err = p.Audit.Export(ctx, auditLog)
285+
if err != nil {
285286
p.Log.Error(ctx, "export audit log",
286287
slog.F("audit_log", auditLog),
287288
slog.Error(err),
288289
)
289-
return
290290
}
291291
}
292292

293+
type WorkspaceBuildBaggage struct {
294+
IP string
295+
}
296+
297+
func (b WorkspaceBuildBaggage) Props() ([]baggage.Property, error) {
298+
ipProp, err := baggage.NewKeyValueProperty("ip", b.IP)
299+
if err != nil {
300+
return nil, xerrors.Errorf("create ip kv property: %w", err)
301+
}
302+
303+
return []baggage.Property{ipProp}, nil
304+
}
305+
306+
func WorkspaceBuildBaggageFromRequest(r *http.Request) WorkspaceBuildBaggage {
307+
return WorkspaceBuildBaggage{IP: r.RemoteAddr}
308+
}
309+
310+
type Baggage interface {
311+
Props() ([]baggage.Property, error)
312+
}
313+
314+
func BaggageToContext(ctx context.Context, d Baggage) (context.Context, error) {
315+
props, err := d.Props()
316+
if err != nil {
317+
return ctx, xerrors.Errorf("create baggage properties: %w", err)
318+
}
319+
320+
m, err := baggage.NewMember("audit", "baggage", props...)
321+
if err != nil {
322+
return ctx, xerrors.Errorf("create new baggage member: %w", err)
323+
}
324+
325+
b, err := baggage.New(m)
326+
if err != nil {
327+
return ctx, xerrors.Errorf("create new baggage carrier: %w", err)
328+
}
329+
330+
return baggage.ContextWithBaggage(ctx, b), nil
331+
}
332+
333+
func BaggageFromContext(ctx context.Context) WorkspaceBuildBaggage {
334+
d := WorkspaceBuildBaggage{}
335+
b := baggage.FromContext(ctx)
336+
props := b.Member("audit").Properties()
337+
for _, prop := range props {
338+
switch prop.Key() {
339+
case "ip":
340+
d.IP, _ = prop.Value()
341+
default:
342+
}
343+
}
344+
345+
return d
346+
}
347+
293348
func either[T Auditable, R any](old, new T, fn func(T) R, auditAction database.AuditAction) R {
294349
if ResourceID(new) != uuid.Nil {
295350
return fn(new)

‎coderd/audit/request_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package audit_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
"go.opentelemetry.io/otel/propagation"
9+
10+
"github.com/coder/coder/v2/coderd/audit"
11+
)
12+
13+
func TestBaggage(t *testing.T) {
14+
t.Parallel()
15+
prop := propagation.NewCompositeTextMapPropagator(
16+
propagation.TraceContext{},
17+
propagation.Baggage{},
18+
)
19+
20+
expected := audit.WorkspaceBuildBaggage{
21+
IP: "127.0.0.1",
22+
}
23+
24+
ctx, err := audit.BaggageToContext(context.Background(), expected)
25+
require.NoError(t, err)
26+
27+
carrier := propagation.MapCarrier{}
28+
prop.Inject(ctx, carrier)
29+
bCtx := prop.Extract(ctx, carrier)
30+
got := audit.BaggageFromContext(bCtx)
31+
32+
require.Equal(t, expected, got)
33+
}

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ func (e *Executor) runOnce(t time.Time) Stats {
184184
builder = builder.ActiveVersion()
185185
}
186186

187-
build, job, err = builder.Build(e.ctx, tx, nil)
187+
build, job, err = builder.Build(e.ctx, tx, nil, audit.WorkspaceBuildBaggage{IP: "127.0.0.1"})
188+
188189
if err != nil {
189190
log.Error(e.ctx, "unable to transition workspace",
190191
slog.F("transition", nextTransition),

‎coderd/database/dbfake/dbfake.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4564,6 +4564,7 @@ func (q *FakeQuerier) InsertProvisionerJob(_ context.Context, arg database.Inser
45644564
Type: arg.Type,
45654565
Input: arg.Input,
45664566
Tags: arg.Tags,
4567+
TraceMetadata: arg.TraceMetadata,
45674568
}
45684569
job.JobStatus = provisonerJobStatus(job)
45694570
q.provisionerJobs = append(q.provisionerJobs, job)

‎coderd/externalauth.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ import (
66
"fmt"
77
"net/http"
88

9-
"golang.org/x/sync/errgroup"
10-
119
"github.com/sqlc-dev/pqtype"
10+
"golang.org/x/sync/errgroup"
1211

1312
"github.com/coder/coder/v2/coderd/database"
1413
"github.com/coder/coder/v2/coderd/database/dbtime"

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -903,12 +903,15 @@ func (s *server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto.
903903
s.Logger.Error(ctx, "marshal workspace resource info for failed job", slog.Error(err))
904904
}
905905

906+
bag := audit.BaggageFromContext(ctx)
907+
906908
audit.WorkspaceBuildAudit(ctx, &audit.BuildAuditParams[database.WorkspaceBuild]{
907909
Audit: *auditor,
908910
Log: s.Logger,
909911
UserID: job.InitiatorID,
910912
OrganizationID: workspace.OrganizationID,
911913
JobID: job.ID,
914+
IP: bag.IP,
912915
Action: auditAction,
913916
Old: previousBuild,
914917
New: build,
@@ -1245,12 +1248,15 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
12451248
s.Logger.Error(ctx, "marshal resource info for successful job", slog.Error(err))
12461249
}
12471250

1251+
bag := audit.BaggageFromContext(ctx)
1252+
12481253
audit.WorkspaceBuildAudit(ctx, &audit.BuildAuditParams[database.WorkspaceBuild]{
12491254
Audit: *auditor,
12501255
Log: s.Logger,
12511256
UserID: job.InitiatorID,
12521257
OrganizationID: workspace.OrganizationID,
12531258
JobID: job.ID,
1259+
IP: bag.IP,
12541260
Action: auditAction,
12551261
Old: previousBuild,
12561262
New: workspaceBuild,

‎coderd/workspacebuilds.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"cdr.dev/slog"
1919

20+
"github.com/coder/coder/v2/coderd/audit"
2021
"github.com/coder/coder/v2/coderd/database"
2122
"github.com/coder/coder/v2/coderd/database/db2sdk"
2223
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -372,6 +373,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
372373
func(action rbac.Action, object rbac.Objecter) bool {
373374
return api.Authorize(r, action, object)
374375
},
376+
audit.WorkspaceBuildBaggageFromRequest(r),
375377
)
376378
var buildErr wsbuilder.BuildError
377379
if xerrors.As(err, &buildErr) {

0 commit comments

Comments
 (0)