feat: add boundary log forwarding from agent to coderd #21345

zedkipp · 2025-12-20T00:31:06Z

Add the agent forwarding of boundary audit logs from workspaces to coderd via the agent API, and re-emission of boundary logs to coderd stderr.

Log format example:

[API] 2025-12-23 18:31:46.755 [info] coderd.agentrpc: boundary_request owner=.. workspace_name=.. agent_name=.. decision=.. workspace_id=.. http_method=.. http_url=.. event_time=.. request_id=..

Corresponding boundary PR: coder/boundary#124
RFC: https://www.notion.so/coderhq/Agent-Boundary-Logs-2afd579be59280f29629fc9823ac41ba
#21280

coderd/agentapi/boundary_logs.go

Add receiving boundary logs via stream unix socket in the agent, forwarding of boundary audit logs from agent to coderd via agent API, and re-emission of boundary logs to coderd stderr. Log format example: [API] 2025-12-23 18:31:46.755 [info] coderd.agentrpc: boundary_request owner=.. workspace_name=.. agent_name=.. decision=.. workspace_id=.. http_method=.. http_url=.. event_time=.. request_id=..

dannykopping

Before we proceed, we need to extract the message framing into its own package so it can be shared. There's a lot of duplication across this PR and https://github.com/coder/boundary/pull/124, and it will lead to inconsistencies.

We also need a more resilient approach towards the socket addressing; /tmp is not guaranteed to exist.

dannykopping · 2025-12-24T07:20:00Z

agent/boundarylogproxy/proxy.go

+ conn, err := s.listener.Accept()
+ if err != nil {
+ if ctx.Err() != nil {
+ return


I'd advise against silently dropping context errors.

In this case you could log that the accept loop has terminated.

dannykopping · 2025-12-24T07:25:23Z

agent/boundarylogproxy/proxy.go

+ s.wg.Add(1)
+ go func() {
+ defer s.wg.Done()
+ <-ctx.Done()
+ _ = conn.Close()
+	}()


Could you not move the conn.Close to the same defer as cancel() but run it afterwards?
I'm not seeing why we need the extra goroutine.

dannykopping · 2025-12-24T07:27:33Z

agent/boundarylogproxy/proxy.go

+ // Even though the length of data received can be larger than maxMsgSize,
+ // practically they are not expected to be. This is a sanity check and
+ // allows re-using a small fixed size read buffer.
+ const maxMsgSize = 1 << 15


The wire format allows for 2^28=256MiB but here you're imposing a 2^15=32KiB limit.
It'd be helpful to move this higher up, or document it along with the wire protocol description.

dannykopping · 2025-12-24T07:29:33Z

agent/agent.go


+// startBoundaryLogProxyServer starts the boundary log proxy socket server.
+func (a *agent) startBoundaryLogProxyServer() {
+ const boundaryAuditSocketPath = "/tmp/boundary-audit.sock"


/tmp is not guaranteed to exist.

I'd suggest passing an env var down with the path to the socket which both the agent and boundary will use.

dannykopping · 2025-12-24T08:27:58Z

agent/boundarylogproxy/proxy_test.go

+func sendMessage(t *testing.T, conn net.Conn, req *agentproto.ReportBoundaryLogsRequest) {
+ t.Helper()
+
+ data, err := proto.Marshal(req)
+ if err != nil {
+ //nolint:gocritic // In tests we're not worried about conn being nil.
+ t.Errorf("%s marshal req: %s", conn.LocalAddr().String(), err)
+	}
+
+ var header uint32
+ //nolint:gosec // In tests we're not worried about possible integer overflow.
+ header |= uint32(len(data))
+ header |= 1 << 28
+
+ err = binary.Write(conn, binary.BigEndian, header)
+ if err != nil {
+ //nolint:gocritic // In tests we're not worried about conn being nil.
+ t.Errorf("%s write conn length: %s", conn.LocalAddr().String(), err)
+	}
+
+ _, err = conn.Write(data)
+ if err != nil {
+ //nolint:gocritic // In tests we're not worried about conn being nil.
+ t.Errorf("%s write conn data: %s", conn.LocalAddr().String(), err)
+	}


Duplicated code from sendBoundaryLogsRequest; see my review of the boundary code which suggests extract the framing into its own package for reuse.

dannykopping · 2025-12-24T08:30:01Z

agent/boundarylogproxy/proxy.go

+ // practically they are not expected to be. This is a sanity check and
+ // allows re-using a small fixed size read buffer.
+ const maxMsgSize = 1 << 15
+ buf := make([]byte, maxMsgSize)


Each conn is prospectively allocating 32KiB; we may never use that full buffer.

I'd suggest preallocating something smaller & more reasonable, and grow the slice if/when needed.

dannykopping · 2025-12-24T08:37:30Z

coderd/agentapi/boundary_logs.go

+ slog.F("decision", allowBoolToString(l.Allowed)),
+ slog.F("workspace_id", a.WorkspaceID.String()),
+ slog.F("http_method", r.HttpRequest.Method),
+ slog.F("http_url", r.HttpRequest.Url),
+ slog.F("event_time", logTime.Format(time.RFC3339Nano)),


How are the other fields injected?

owner

workspace_name

agent_name

request_id

Does the logger already have these?

dannykopping · 2025-12-24T08:39:24Z

agent/boundarylogproxy/proxy_test.go

@@ -0,0 +1,588 @@
+//go:build linux || darwin


What about Windows? :thinking_face:

dannykopping · 2025-12-24T08:40:08Z

coderd/agentapi/boundary_logs.go

+ switch r := l.Resource.(type) {
+ case *agentproto.BoundaryLog_HttpRequest_:
+ if r.HttpRequest == nil {
+ continue


This could hide protocol bugs; you should log this.

dannykopping · 2025-12-24T08:40:36Z

coderd/agentapi/boundary_logs.go

+ slog.F("event_time", logTime.Format(time.RFC3339Nano)),
+			}
+ if l.Allowed {
+ fields = append(fields, slog.F("matched_rule", r.HttpRequest.MatchedRule))


Is it possible to log which rule caused the denial? That would also be very useful.

dannykopping · 2025-12-24T08:47:44Z

coderd/agentapi/boundary_logs.go

+			}
+
+ a.Log.With(fields...).Info(ctx, "boundary_request")
+ default:


We should probably handle the default case a bit better. Maybe log something like "unexpected type"?

github-actions bot assigned zedkipp Dec 20, 2025

zedkipp force-pushed the zedkipp/boundary-logs branch 3 times, most recently from 334f1ca to ab19300 Compare December 22, 2025 17:55

zedkipp commented Dec 23, 2025

View reviewed changes

coderd/agentapi/boundary_logs.go Outdated Show resolved Hide resolved

zedkipp force-pushed the zedkipp/boundary-logs branch 4 times, most recently from ced7c18 to adf8dda Compare December 23, 2025 19:01

zedkipp marked this pull request as ready for review December 23, 2025 21:39

zedkipp requested review from cstyan, dannykopping and evgeniy-scherbina December 23, 2025 21:40

zedkipp mentioned this pull request Dec 24, 2025

feat: add socket auditor for forwarding logs to coder agent coder/boundary#124

Open

zedkipp force-pushed the zedkipp/boundary-logs branch from adf8dda to 752536f Compare December 24, 2025 00:23

dannykopping requested changes Dec 24, 2025

View reviewed changes

dannykopping reviewed Dec 24, 2025

View reviewed changes

feat: add boundary log forwarding from agent to coderd #21345

Are you sure you want to change the base?

feat: add boundary log forwarding from agent to coderd #21345

Uh oh!

Conversation

zedkipp commented Dec 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zedkipp commented Dec 20, 2025 •

edited

Loading