diff --git a/coderd/agentapi/boundary_logs.go b/coderd/agentapi/boundary_logs.go index 16703c8384..a4b3956186 100644 --- a/coderd/agentapi/boundary_logs.go +++ b/coderd/agentapi/boundary_logs.go @@ -2,9 +2,8 @@ package agentapi import ( "context" - "database/sql" - "errors" "fmt" + "sync" "time" "github.com/google/uuid" @@ -47,6 +46,13 @@ type BoundaryLogsAPI struct { TemplateID uuid.UUID TemplateVersionID uuid.UUID BoundaryUsageTracker *boundaryusage.Tracker + + // mu guards ensuredSessions, which records session IDs already persisted + // by this connection so repeated batches skip the existence check and + // insert. The API is one instance per agent connection, so this lives for + // the session's lifetime. + mu sync.Mutex + ensuredSessions map[uuid.UUID]struct{} } func (a *BoundaryLogsAPI) ReportBoundaryLogs(ctx context.Context, req *agentproto.ReportBoundaryLogsRequest) (*agentproto.ReportBoundaryLogsResponse, error) { @@ -80,16 +86,19 @@ func (a *BoundaryLogsAPI) ReportBoundaryLogs(ctx context.Context, req *agentprot } } - if persistEnabled { + if persistEnabled && !a.sessionEnsured(sessionID) { // Lazy-create the boundary session on first log arrival. // If this fails (transient DB error), we continue so that // logs are still persisted. The session will be created on // a subsequent batch since every request carries the session - // details. + // details. On success we record the session so later batches + // skip the existence check and insert entirely. if sessionErr := a.ensureSession(ctx, sessionID, req.GetConfinedProcessName(), now); sessionErr != nil { a.Log.Error(ctx, "failed to ensure boundary session", slog.F("session_id", sessionID.String()), slog.Error(sessionErr)) + } else { + a.markSessionEnsured(sessionID) } } @@ -179,6 +188,25 @@ func (a *BoundaryLogsAPI) ReportBoundaryLogs(ctx context.Context, req *agentprot return &agentproto.ReportBoundaryLogsResponse{}, nil } +// sessionEnsured reports whether this connection has already persisted the +// session, letting repeated batches skip the database round-trip. +func (a *BoundaryLogsAPI) sessionEnsured(sessionID uuid.UUID) bool { + a.mu.Lock() + defer a.mu.Unlock() + _, ok := a.ensuredSessions[sessionID] + return ok +} + +// markSessionEnsured records that the session has been persisted. +func (a *BoundaryLogsAPI) markSessionEnsured(sessionID uuid.UUID) { + a.mu.Lock() + defer a.mu.Unlock() + if a.ensuredSessions == nil { + a.ensuredSessions = make(map[uuid.UUID]struct{}) + } + a.ensuredSessions[sessionID] = struct{}{} +} + // ensureSession creates the boundary_sessions row if it does not // already exist. func (a *BoundaryLogsAPI) ensureSession(ctx context.Context, sessionID uuid.UUID, confinedProcess string, now time.Time) error { @@ -186,19 +214,7 @@ func (a *BoundaryLogsAPI) ensureSession(ctx context.Context, sessionID uuid.UUID return nil } - // Check the database in case another replica or reconnection - // already created this session. - _, err := a.Database.GetBoundarySessionByID(ctx, sessionID) - if err == nil { - return nil - } - if !errors.Is(err, sql.ErrNoRows) { - return xerrors.Errorf("check boundary session existence: %w", err) - } - - // Session does not exist; create it. started_at is the time - // the first log is received by coderd, per the RFC. - _, err = a.Database.InsertBoundarySession(ctx, database.InsertBoundarySessionParams{ + _, err := a.Database.InsertBoundarySession(ctx, database.InsertBoundarySessionParams{ ID: sessionID, WorkspaceAgentID: a.AgentID, OwnerID: uuid.NullUUID{UUID: a.OwnerID, Valid: true}, @@ -207,13 +223,8 @@ func (a *BoundaryLogsAPI) ensureSession(ctx context.Context, sessionID uuid.UUID UpdatedAt: now, }) if err != nil { - // A second coderd replica may receive a batch for this session - // before the first replica has finished inserting it. Both - // attempt the INSERT; the second fails with a primary-key - // unique violation. Treat it as success because the session - // now exists. if database.IsUniqueViolation(err, database.UniqueBoundarySessionsPkey) { - a.Log.Debug(ctx, "boundary session already created by another replica", + a.Log.Debug(ctx, "boundary session already created", slog.F("session_id", sessionID.String())) return nil } diff --git a/coderd/agentapi/boundary_logs_test.go b/coderd/agentapi/boundary_logs_test.go index ad8baaec8e..dd9a88e266 100644 --- a/coderd/agentapi/boundary_logs_test.go +++ b/coderd/agentapi/boundary_logs_test.go @@ -2,16 +2,20 @@ package agentapi_test import ( "context" + "database/sql" "testing" "github.com/google/uuid" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "google.golang.org/protobuf/types/known/timestamppb" + "cdr.dev/slog/v3/sloggers/slogtest" agentproto "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd/agentapi" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/testutil" @@ -454,3 +458,98 @@ func TestReportBoundaryLogs(t *testing.T) { require.Len(t, logs, 2, "logs from both agents must be persisted") }) } + +// httpLogRequest builds a ReportBoundaryLogsRequest carrying a single allowed +// HTTP log for the given session. +func httpLogRequest(sessionID uuid.UUID, seq int32) *agentproto.ReportBoundaryLogsRequest { + return &agentproto.ReportBoundaryLogsRequest{ + SessionId: sessionID.String(), + ConfinedProcessName: "claude-code", + Logs: []*agentproto.BoundaryLog{ + { + Allowed: true, + Time: timestamppb.New(dbtime.Now()), + SequenceNumber: seq, + Resource: &agentproto.BoundaryLog_HttpRequest_{ + HttpRequest: &agentproto.BoundaryLog_HttpRequest{ + Method: "GET", + Url: "https://example.com", + MatchedRule: "domain=example.com", + }, + }, + }, + }, + } +} + +// TestReportBoundaryLogsSessionGuard verifies that once a session has been +// ensured, later batches from the same connection skip the existence check and +// insert entirely, touching the database only for the logs themselves. +func TestReportBoundaryLogsSessionGuard(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + db := dbmock.NewMockStore(ctrl) + + sessionID := uuid.New() + api := &agentapi.BoundaryLogsAPI{ + Log: testutil.Logger(t), + Database: db, + AgentID: uuid.New(), + WorkspaceID: uuid.New(), + OwnerID: uuid.New(), + TemplateID: uuid.New(), + TemplateVersionID: uuid.New(), + } + + // The session is inserted once, even though two batches arrive. Times(1) + // fails the test if the guard does not suppress the second ensure attempt. + // Logs insert on every batch. + db.EXPECT().InsertBoundarySession(gomock.Any(), gomock.Any()). + Return(database.BoundarySession{}, nil).Times(1) + db.EXPECT().InsertBoundaryLogs(gomock.Any(), gomock.Any()). + Return([]database.BoundaryLog{}, nil).Times(2) + + _, err := api.ReportBoundaryLogs(context.Background(), httpLogRequest(sessionID, 0)) + require.NoError(t, err) + _, err = api.ReportBoundaryLogs(context.Background(), httpLogRequest(sessionID, 1)) + require.NoError(t, err) +} + +// TestReportBoundaryLogsSessionRetriedOnError verifies that when ensureSession +// fails, the guard is not set, so the next batch retries the existence check. +func TestReportBoundaryLogsSessionRetriedOnError(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + db := dbmock.NewMockStore(ctrl) + + sessionID := uuid.New() + api := &agentapi.BoundaryLogsAPI{ + // The first batch deliberately triggers a transient error, which + // logs at ERROR level. Ignore errors so slogtest does not fail. + Log: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}), + Database: db, + AgentID: uuid.New(), + WorkspaceID: uuid.New(), + OwnerID: uuid.New(), + TemplateID: uuid.New(), + TemplateVersionID: uuid.New(), + } + + // First batch: insert fails transiently, so the session is not marked + // ensured. Second batch: insert is retried and succeeds. Logs insert on both. + gomock.InOrder( + db.EXPECT().InsertBoundarySession(gomock.Any(), gomock.Any()). + Return(database.BoundarySession{}, sql.ErrConnDone), + db.EXPECT().InsertBoundarySession(gomock.Any(), gomock.Any()). + Return(database.BoundarySession{}, nil), + ) + db.EXPECT().InsertBoundaryLogs(gomock.Any(), gomock.Any()). + Return([]database.BoundaryLog{}, nil).Times(2) + + _, err := api.ReportBoundaryLogs(context.Background(), httpLogRequest(sessionID, 0)) + require.NoError(t, err) + _, err = api.ReportBoundaryLogs(context.Background(), httpLogRequest(sessionID, 1)) + require.NoError(t, err) +} diff --git a/coderd/boundary_logs_test.go b/coderd/boundary_logs_test.go new file mode 100644 index 0000000000..bc13659ab7 --- /dev/null +++ b/coderd/boundary_logs_test.go @@ -0,0 +1,107 @@ +package coderd_test + +import ( + "context" + "sync/atomic" + "testing" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/timestamppb" + + agentproto "github.com/coder/coder/v2/agent/proto" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/codersdk/agentsdk" + "github.com/coder/coder/v2/testutil" +) + +// TestReportBoundaryLogsAgentRBAC guards against regressions where +// a pre-insert read (e.g. GetBoundarySessionByID) would be silently denied for +// agents and prevent session creation. +func TestReportBoundaryLogsAgentRBAC(t *testing.T) { + t.Parallel() + + store, ps := dbtestutil.NewDB(t) + client := coderdtest.New(t, &coderdtest.Options{Database: store, Pubsub: ps}) + user := coderdtest.CreateFirstUser(t, client) + r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ + OrganizationID: user.OrganizationID, + OwnerID: user.UserID, + }).WithAgent().Do() + + ctx := testutil.Context(t, testutil.WaitLong) + + // Connect as a real workspace agent. + ac := agentsdk.New(client.URL, agentsdk.WithFixedToken(r.AgentToken)) + conn, err := ac.ConnectRPC(ctx) + require.NoError(t, err) + defer conn.Close() + + agentClient := agentproto.NewDRPCAgentClient(conn) + sessionID := uuid.New() + + _, err = agentClient.ReportBoundaryLogs(ctx, &agentproto.ReportBoundaryLogsRequest{ + SessionId: sessionID.String(), + ConfinedProcessName: "claude-code", + Logs: []*agentproto.BoundaryLog{ + { + Allowed: true, + Time: timestamppb.New(dbtime.Now()), + SequenceNumber: 0, + Resource: &agentproto.BoundaryLog_HttpRequest_{ + HttpRequest: &agentproto.BoundaryLog_HttpRequest{ + Method: "GET", + Url: "https://example.com", + MatchedRule: "domain=example.com", + }, + }, + }, + }, + }) + require.NoError(t, err) + + // Verify persistence via the raw store: because ReportBoundaryLogs swallows + // DB errors and returns success regardless, only a direct read proves the + // session and log were actually persisted under agent RBAC. + sess, err := store.GetBoundarySessionByID(ctx, sessionID) + require.NoError(t, err, "session must be persisted") + require.Equal(t, r.Agents[0].ID, sess.WorkspaceAgentID) + + logs, err := store.ListBoundaryLogsBySessionID(ctx, database.ListBoundaryLogsBySessionIDParams{ + SessionID: sessionID, + }) + require.NoError(t, err) + require.Len(t, logs, 1, "log must be persisted") + + // Assert that the agent subject cannot read boundary sessions. + memberRole, err := rbac.RoleByName(rbac.RoleMember()) + require.NoError(t, err) + agentSubject := rbac.Subject{ + ID: r.Workspace.OwnerID.String(), + Roles: rbac.Roles{memberRole}, + Scope: rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{ + WorkspaceID: r.Workspace.ID, + OwnerID: r.Workspace.OwnerID, + TemplateID: r.Workspace.TemplateID, + VersionID: r.Build.TemplateVersionID, + }), + }.WithCachedASTValue() + + auth := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry()) + acsPtr := &atomic.Pointer[dbauthz.AccessControlStore]{} + var acs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{} + acsPtr.Store(&acs) + authzStore := dbauthz.New(store, auth, testutil.Logger(t), acsPtr) + + agentCtx := dbauthz.As(context.Background(), agentSubject) + _, err = authzStore.GetBoundarySessionByID(agentCtx, sessionID) + require.True(t, dbauthz.IsNotAuthorizedError(err), + "agents must not be able to read boundary sessions, got: %v", err) +} diff --git a/docs/reference/cli/agent-firewall.md b/docs/reference/cli/agent-firewall.md index add4098c6b..f68b381607 100644 --- a/docs/reference/cli/agent-firewall.md +++ b/docs/reference/cli/agent-firewall.md @@ -35,15 +35,6 @@ Path to YAML config file. Allow rule (repeatable). These are merged with allowlist from config file. Format: "pattern" or "METHOD[,METHOD] pattern". -### -- - -| | | -|------|---------------------------| -| Type | string-array | -| YAML | allowlist | - -Allowlist rules from config file (YAML only). - ### --log-level | | | @@ -155,3 +146,22 @@ Path to the socket where the boundary log proxy server listens for audit logs. | Type | bool | Print version information and exit. + +### --enable-session-correlation + +| | | +|-------------|----------------------------------------------------| +| Type | bool | +| Environment | $BOUNDARY_SESSION_CORRELATION_ENABLED | +| YAML | session_correlation_enabled | + +Enable session correlation header injection. When no inject targets are configured, the target is auto-derived from CODER_AGENT_URL (set automatically inside Coder workspaces). Disable for deployments without Coder AI Gateway in front. + +### --session-id-inject-target + +| | | +|-------------|-------------------------------------------------| +| Type | string | +| Environment | $BOUNDARY_SESSION_ID_INJECT_TARGET | + +Inject target for session correlation headers. Repeat the flag once per target; each value describes exactly one target. Format: "domain= [path=]". Example: --session-id-inject-target "domain=prod.coder.com path=/api/v2/aibridge/*". diff --git a/enterprise/cli/testdata/coder_agent-firewall_--help.golden b/enterprise/cli/testdata/coder_agent-firewall_--help.golden index 5c6dcf7adb..9ebb32ba94 100644 --- a/enterprise/cli/testdata/coder_agent-firewall_--help.golden +++ b/enterprise/cli/testdata/coder_agent-firewall_--help.golden @@ -23,6 +23,12 @@ OPTIONS: --disable-audit-logs bool, $DISABLE_AUDIT_LOGS Disable sending of audit logs to the workspace agent when set to true. + --enable-session-correlation bool, $BOUNDARY_SESSION_CORRELATION_ENABLED + Enable session correlation header injection. When no inject targets + are configured, the target is auto-derived from CODER_AGENT_URL (set + automatically inside Coder workspaces). Disable for deployments + without Coder AI Gateway in front. + --jail-type string, $BOUNDARY_JAIL_TYPE (default: nsjail) Jail type to use for network isolation. Options: nsjail (default), landjail. @@ -50,6 +56,15 @@ OPTIONS: --proxy-port int, $PROXY_PORT (default: 8080) Set a port for HTTP proxy. + --session-id-inject-target string, $BOUNDARY_SESSION_ID_INJECT_TARGET + Inject target for session correlation headers. Repeat the flag once + per target; each value describes exactly one target. Format: + "domain= [path=]". Example: --session-id-inject-target + "domain=prod.coder.com path=/api/v2/aibridge/*". + + string-array + Inject targets from config file (YAML only). + --use-real-dns bool, $BOUNDARY_USE_REAL_DNS Use real DNS in the jail instead of the dummy DNS (allows DNS exfiltration). Default: false. diff --git a/go.mod b/go.mod index 0dd80b3224..55b0dc30b5 100644 --- a/go.mod +++ b/go.mod @@ -520,7 +520,7 @@ require ( github.com/brianvoe/gofakeit/v7 v7.15.0 github.com/coder/agentapi-sdk-go v0.0.0-20250505131810-560d1d88d225 github.com/coder/aisdk-go v0.0.9 - github.com/coder/boundary v0.9.0 + github.com/coder/boundary v0.10.0 github.com/coder/preview v1.0.10-0.20260521153517-34deb0946c4f github.com/danieljoos/wincred v1.2.3 github.com/dgraph-io/ristretto/v2 v2.4.0 diff --git a/go.sum b/go.sum index a4d84ad7f6..9c95875e9c 100644 --- a/go.sum +++ b/go.sum @@ -316,8 +316,8 @@ github.com/coder/aisdk-go v0.0.9 h1:Vzo/k2qwVGLTR10ESDeP2Ecek1SdPfZlEjtTfMveiVo= github.com/coder/aisdk-go v0.0.9/go.mod h1:KF6/Vkono0FJJOtWtveh5j7yfNrSctVTpwgweYWSp5M= github.com/coder/anthropic-sdk-go v0.0.0-20260428122333-47cab198e449 h1:X4XOtomDcJlr5/bmgcnrZiJeZIS+qixzVn1EWqgCZ4E= github.com/coder/anthropic-sdk-go v0.0.0-20260428122333-47cab198e449/go.mod h1:hqlYqR7uPKOKfnNeicUbZp0Ps0GeYFlKYtwh5HGDCx8= -github.com/coder/boundary v0.9.0 h1:JthV9N9R/4QFoPd6L7i04O3xAOtXvSVu4v7CiAaEzu0= -github.com/coder/boundary v0.9.0/go.mod h1:BhJhyKW/+zZQzaGZ3vn27if2k0Vx5xLXzq7ZCQx5gPk= +github.com/coder/boundary v0.10.0 h1:qX8iGKpAx5jm4wdbhYfLQLvNMiq16pkZiOjLLBklI+Q= +github.com/coder/boundary v0.10.0/go.mod h1:dDILpof96k+ixVbVDRA2ez+zmj+n0ZhJPSiidB6rtrw= github.com/coder/bubbletea v1.2.2-0.20241212190825-007a1cdb2c41 h1:SBN/DA63+ZHwuWwPHPYoCZ/KLAjHv5g4h2MS4f2/MTI= github.com/coder/bubbletea v1.2.2-0.20241212190825-007a1cdb2c41/go.mod h1:I9ULxr64UaOSUv7hcb3nX4kowodJCVS7vt7VVJk/kW4= github.com/coder/clistat v1.2.1 h1:P9/10njXMyj5cWzIU5wkRsSy5LVQH49+tcGMsAgWX0w= diff --git a/scripts/clidocgen/command.tpl b/scripts/clidocgen/command.tpl index 39065392f7..1f2a4a0b97 100644 --- a/scripts/clidocgen/command.tpl +++ b/scripts/clidocgen/command.tpl @@ -40,7 +40,7 @@ Aliases: {{- if eq $index 0 }} ## Options {{- end }} -### {{ with $opt.FlagShorthand}}-{{ . }}, {{end}}--{{ $opt.Flag }} +### {{ with $opt.FlagShorthand}}-{{ . }}, {{end}}{{ if $opt.Flag }}--{{ $opt.Flag }}{{ else }}{{ $opt.YAMLPath }}{{ end }} {{" "}} {{ tableHeader }} | Type | {{ typeHelper $opt | wrapCode }} | diff --git a/scripts/clidocgen/gen.go b/scripts/clidocgen/gen.go index 6679fb6853..dde21ef78c 100644 --- a/scripts/clidocgen/gen.go +++ b/scripts/clidocgen/gen.go @@ -41,6 +41,11 @@ func init() { if opt.Hidden { continue } + // Skip YAML-only options that have no CLI flag; documenting them + // as if they were flags is misleading in the CLI reference. + if opt.Flag == "" && opt.FlagShorthand == "" { + continue + } visible = append(visible, opt) } return visible