From a649b09305e8d9ab9815d6e0c51873305bf19295 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:53:29 +0800 Subject: [PATCH] feat(events): attribute explicit sandbox kills with requester IP and user agent A sandbox.lifecycle.killed event records why a sandbox died (kill_reason) but, for a user-issued DELETE sandboxes/{id} (kill_reason=request), nothing about who issued it. Operators cannot tell from the event stream which client IP or which SDK/version initiated an explicit kill - useful for debugging and for spotting a client deleting sandboxes it should not. Add two optional fields to the killed event_data, populated only on a request kill: requester_ip (c.ClientIP) and requester_user_agent (c.Request.UserAgent). Threaded API -> orchestrator over SandboxDeleteRequest (two new optional proto fields), then written into event_data in the orchestrator Delete path. Timeout, eviction and orchestrator-internal kills leave both absent. Design notes: - Only KillReasonRequest carries them; every internal killSandboxOnNode caller passes empty, so no internal kill is ever attributed to a client. - The fields ride the killed event's existing per-team EventsTTLDays retention, so this introduces no new unbounded store of client data - a deliberate, bounded handling of the IP (PII) rather than an open-ended log. - Server side uses a small requesterAttribution(in) decorator passed to emitSandboxKilled (now variadic), so the other six emit callers are untouched and the attribution logic is unit-tested in isolation. Rebased onto current main (emitSandboxKilled/killSandboxOnNode signatures and the NodeSandbox kill path have changed since the original #3465). Regenerated orchestrator.pb.go for the two proto fields only; generator version headers kept as-is to avoid toolchain churn. Adds TestRequesterAttribution. Closes #3461. Supersedes #3465. --- .../api/internal/handlers/sandbox_kill.go | 6 ++- .../internal/orchestrator/create_instance.go | 2 + .../internal/orchestrator/delete_instance.go | 20 ++++++--- .../internal/sandbox/sandboxtypes/states.go | 14 +++--- packages/orchestrator/orchestrator.proto | 2 + .../pkg/server/requester_attribution_test.go | 44 +++++++++++++++++++ packages/orchestrator/pkg/server/sandboxes.go | 24 +++++++++- .../pkg/grpc/orchestrator/orchestrator.pb.go | 36 +++++++++++---- .../grpc/orchestrator/orchestrator_grpc.pb.go | 2 +- 9 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 packages/orchestrator/pkg/server/requester_attribution_test.go diff --git a/packages/api/internal/handlers/sandbox_kill.go b/packages/api/internal/handlers/sandbox_kill.go index b30ac6b5a9..9c10d22807 100644 --- a/packages/api/internal/handlers/sandbox_kill.go +++ b/packages/api/internal/handlers/sandbox_kill.go @@ -62,8 +62,10 @@ func (a *APIStore) DeleteSandboxesSandboxID( killedOrRemoved := false err = a.orchestrator.RemoveSandbox(ctx, teamID, sandboxID, sandbox.RemoveOpts{ - Action: sandbox.StateActionKill, - Reason: sandbox.KillReasonRequest, + Action: sandbox.StateActionKill, + Reason: sandbox.KillReasonRequest, + RequesterIP: c.ClientIP(), + UserAgent: c.Request.UserAgent(), }) switch { case err == nil: diff --git a/packages/api/internal/orchestrator/create_instance.go b/packages/api/internal/orchestrator/create_instance.go index 67b294afcf..1a139be851 100644 --- a/packages/api/internal/orchestrator/create_instance.go +++ b/packages/api/internal/orchestrator/create_instance.go @@ -607,6 +607,7 @@ func (o *Orchestrator) CreateSandbox( sandbox.StateActionKill, sandbox.KillReasonUnknown, false, // kill: no snapshot + "", "", ) if killErr != nil { logger.L().Error(ctx, "Error removing memory-restored sandbox after unhonored filesystem-boot demand", @@ -637,6 +638,7 @@ func (o *Orchestrator) CreateSandbox( sandbox.StateActionKill, sandbox.KillReasonUnknown, false, // kill: no snapshot + "", "", ) if killErr != nil { logger.L().Error(ctx, "Error removing sandbox", diff --git a/packages/api/internal/orchestrator/delete_instance.go b/packages/api/internal/orchestrator/delete_instance.go index ad9ff60df6..86d9d2a5c2 100644 --- a/packages/api/internal/orchestrator/delete_instance.go +++ b/packages/api/internal/orchestrator/delete_instance.go @@ -149,7 +149,7 @@ func (o *Orchestrator) RemoveSandbox(ctx context.Context, teamID uuid.UUID, sand o.sandboxStore.Remove(context.WithoutCancel(ctx), teamID, sandboxID) go o.analyticsRemove(context.WithoutCancel(ctx), sbx, opts.Action) }() - err = o.removeSandboxFromNode(ctx, sbx, opts.Action, opts.Reason, opts.FilesystemOnly) + err = o.removeSandboxFromNode(ctx, sbx, opts.Action, opts.Reason, opts.FilesystemOnly, opts.RequesterIP, opts.UserAgent) if err != nil { if errors.Is(err, PauseQueueExhaustedError{}) { if restoreOnRefusal { @@ -231,7 +231,7 @@ func (o *Orchestrator) killRefusedSandbox(ctx context.Context, sbx sandbox.Sandb return } - if err := o.killSandboxOnNode(ctx, node, sbx.ToNodeSandbox(), sandbox.KillReasonOrphaned); err != nil { + if err := o.killSandboxOnNode(ctx, node, sbx.ToNodeSandbox(), sandbox.KillReasonOrphaned, "", ""); err != nil { logger.L().Error(ctx, "failed to kill a refused sandbox after a failed restore", zap.Error(err), logger.WithSandboxID(sbx.SandboxID)) } } @@ -276,6 +276,8 @@ func (o *Orchestrator) removeSandboxFromNode( stateAction sandbox.StateAction, reason sandbox.KillReason, filesystemOnly bool, + requesterIP string, + userAgent string, ) error { ctx, span := tracer.Start(ctx, "remove-sandbox-from-node") defer span.End() @@ -304,7 +306,7 @@ func (o *Orchestrator) removeSandboxFromNode( err := o.pauseSandbox(ctx, node, sbx, filesystemOnly) if err != nil { if dberrors.IsForeignKeyViolation(err) { - killErr := o.killSandboxOnNode(ctx, node, sbx.ToNodeSandbox(), sandbox.KillReasonBaseTemplateMissing) + killErr := o.killSandboxOnNode(ctx, node, sbx.ToNodeSandbox(), sandbox.KillReasonBaseTemplateMissing, "", "") logger.L().Error(ctx, "Pause failed due to missing base template, killed sandbox as fallback", logger.WithSandboxID(sbx.SandboxID), zap.String("base_template_id", sbx.BaseTemplateID), @@ -321,7 +323,7 @@ func (o *Orchestrator) removeSandboxFromNode( return nil case sandbox.StateActionKill: - return o.killSandboxOnNode(ctx, node, sbx.ToNodeSandbox(), reason) + return o.killSandboxOnNode(ctx, node, sbx.ToNodeSandbox(), reason, requesterIP, userAgent) } return nil @@ -339,7 +341,7 @@ func (o *Orchestrator) killOrphanSandbox(ctx context.Context, sbx sandbox.NodeSa return } - err := o.killSandboxOnNode(ctx, node, sbx, sandbox.KillReasonOrphaned) + err := o.killSandboxOnNode(ctx, node, sbx, sandbox.KillReasonOrphaned, "", "") if err != nil { logger.L().Error(ctx, "Failed to kill orphan sandbox on node", zap.Error(err), @@ -355,12 +357,20 @@ func (o *Orchestrator) killSandboxOnNode( node *nodemanager.Node, sbx sandbox.NodeSandbox, reason sandbox.KillReason, + requesterIP string, + userAgent string, ) error { killReason := reason.String() req := &orchestrator.SandboxDeleteRequest{ SandboxId: sbx.SandboxID, KillReason: &killReason, } + if requesterIP != "" { + req.RequesterIp = &requesterIP + } + if userAgent != "" { + req.RequesterUserAgent = &userAgent + } client, ctx := node.GetSandboxClient(ctx, sbx.TeamID.String()) _, err := client.Sandbox.Delete(ctx, req) diff --git a/packages/api/internal/sandbox/sandboxtypes/states.go b/packages/api/internal/sandbox/sandboxtypes/states.go index f80ff073cd..c6f3c09cfc 100644 --- a/packages/api/internal/sandbox/sandboxtypes/states.go +++ b/packages/api/internal/sandbox/sandboxtypes/states.go @@ -84,13 +84,15 @@ type RemoveOpts struct { // When set, StartRemoving fails with ErrExecutionMismatch if the stored // record carries a different execution ID. // - // For callers that decide from a snapshot taken earlier — a background - // scan, a queued batch — the sandbox ID alone is not a stable identity: - // the record can be removed and the ID reused by a resume before the - // removal runs, and the transition would then land on a live sandbox that - // was never in scope. Empty means "remove whatever is stored", which is - // correct for callers acting on a fresh read or on user intent. ExpectExecutionID string + + // RequesterIP and UserAgent attribute an explicit API kill (KillReasonRequest) + // to the client that issued DELETE /sandboxes/{id}, so the sandbox.lifecycle.killed + // event can record who initiated it. Both are empty for timeout, eviction and + // orchestrator-internal kills. They ride the event's existing per-team TTL, so + // no new unbounded store of client data is introduced. + RequesterIP string + UserAgent string } var AllowedTransitions = map[State]map[State]bool{ diff --git a/packages/orchestrator/orchestrator.proto b/packages/orchestrator/orchestrator.proto index b9ce038665..1c9aa206b7 100644 --- a/packages/orchestrator/orchestrator.proto +++ b/packages/orchestrator/orchestrator.proto @@ -210,6 +210,8 @@ message SandboxUpdateRequest { message SandboxDeleteRequest { string sandbox_id = 1; optional string kill_reason = 2; + optional string requester_ip = 3; + optional string requester_user_agent = 4; } message SandboxPauseRequest { diff --git a/packages/orchestrator/pkg/server/requester_attribution_test.go b/packages/orchestrator/pkg/server/requester_attribution_test.go new file mode 100644 index 0000000000..28c769a38b --- /dev/null +++ b/packages/orchestrator/pkg/server/requester_attribution_test.go @@ -0,0 +1,44 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/e2b-dev/infra/packages/shared/pkg/grpc/orchestrator" +) + +// TestRequesterAttribution pins that an explicit API kill records the client IP +// and User-Agent on the killed event, while a kill without them (timeout, +// eviction, internal) adds nothing. +func TestRequesterAttribution(t *testing.T) { + t.Parallel() + + ip := "203.0.113.7" + ua := "e2b-python-sdk/1.2.3" + + t.Run("request kill records requester fields", func(t *testing.T) { + t.Parallel() + in := &orchestrator.SandboxDeleteRequest{ + SandboxId: "isbx", + RequesterIp: &ip, + RequesterUserAgent: &ua, + } + data := map[string]any{} + requesterAttribution(in)(data) + assert.Equal(t, ip, data["requester_ip"]) + assert.Equal(t, ua, data["requester_user_agent"]) + }) + + t.Run("kill without requester adds nothing", func(t *testing.T) { + t.Parallel() + in := &orchestrator.SandboxDeleteRequest{SandboxId: "isbx"} + data := map[string]any{} + requesterAttribution(in)(data) + _, hasIP := data["requester_ip"] + _, hasUA := data["requester_user_agent"] + assert.False(t, hasIP, "requester_ip must be absent") + assert.False(t, hasUA, "requester_user_agent must be absent") + assert.Empty(t, data) + }) +} diff --git a/packages/orchestrator/pkg/server/sandboxes.go b/packages/orchestrator/pkg/server/sandboxes.go index 6abe238390..4ca8c1311f 100644 --- a/packages/orchestrator/pkg/server/sandboxes.go +++ b/packages/orchestrator/pkg/server/sandboxes.go @@ -863,7 +863,7 @@ func (s *Server) Delete(ctxConn context.Context, in *orchestrator.SandboxDeleteR } }() - s.emitSandboxKilled(ctx, sbx, killReason) + s.emitSandboxKilled(ctx, sbx, killReason, requesterAttribution(in)) return &emptypb.Empty{}, nil } @@ -940,10 +940,13 @@ func (s *Server) reportCheckpointStepError(ctx context.Context, msg string, err // are the Delete RPC and the paths that destroy a live-registered sandbox // in-request without passing through it: the pause-path seal-failure kill and // the in-place resume-failure teardown. -func (s *Server) emitSandboxKilled(ctx context.Context, sbx *sandbox.Sandbox, killReason string) { +func (s *Server) emitSandboxKilled(ctx context.Context, sbx *sandbox.Sandbox, killReason string, decorate ...func(map[string]any)) { teamID, buildId, eventsTTLDays, eventData := s.prepareSandboxEventData(ctx, sbx) eventData[executionEventDataKey] = s.getSandboxExecutionData(sbx) addKillReason(eventData, killReason) + for _, d := range decorate { + d(eventData) + } recordSandboxKill(ctx, s.sandboxKilledCounter, sbx.Runtime.TeamID, killReason) s.publishEventAsync( ctx, @@ -967,6 +970,23 @@ func (s *Server) emitSandboxKilled(ctx context.Context, sbx *sandbox.Sandbox, ki // addKillReason records the kill reason on killed events. Empty input is // normalized to "unknown" so killed events always carry a kill_reason key. +// requesterAttribution returns an event-data decorator that records who issued +// an explicit API kill. The API populates requester_ip / requester_user_agent +// only on a user-issued DELETE (KillReasonRequest); for timeout, eviction and +// orchestrator-internal kills both are empty and the decorator adds nothing. +// The fields ride the killed event's existing per-team TTL, so no new unbounded +// store of client data is created. +func requesterAttribution(in *orchestrator.SandboxDeleteRequest) func(map[string]any) { + return func(eventData map[string]any) { + if ip := in.GetRequesterIp(); ip != "" { + eventData["requester_ip"] = ip + } + if ua := in.GetRequesterUserAgent(); ua != "" { + eventData["requester_user_agent"] = ua + } + } +} + func addKillReason(eventData map[string]any, killReason string) { if killReason == "" { killReason = killReasonUnknown diff --git a/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go b/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go index b4a0d01a72..264e47d097 100644 --- a/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go +++ b/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go @@ -1241,11 +1241,13 @@ func (x *SandboxUpdateRequest) GetEgress() *SandboxNetworkEgressConfig { } type SandboxDeleteRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - SandboxId string `protobuf:"bytes,1,opt,name=sandbox_id,json=sandboxId,proto3" json:"sandbox_id,omitempty"` - KillReason *string `protobuf:"bytes,2,opt,name=kill_reason,json=killReason,proto3,oneof" json:"kill_reason,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + state protoimpl.MessageState `protogen:"open.v1"` + SandboxId string `protobuf:"bytes,1,opt,name=sandbox_id,json=sandboxId,proto3" json:"sandbox_id,omitempty"` + KillReason *string `protobuf:"bytes,2,opt,name=kill_reason,json=killReason,proto3,oneof" json:"kill_reason,omitempty"` + RequesterIp *string `protobuf:"bytes,3,opt,name=requester_ip,json=requesterIp,proto3,oneof" json:"requester_ip,omitempty"` + RequesterUserAgent *string `protobuf:"bytes,4,opt,name=requester_user_agent,json=requesterUserAgent,proto3,oneof" json:"requester_user_agent,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *SandboxDeleteRequest) Reset() { @@ -1292,6 +1294,20 @@ func (x *SandboxDeleteRequest) GetKillReason() string { return "" } +func (x *SandboxDeleteRequest) GetRequesterIp() string { + if x != nil && x.RequesterIp != nil { + return *x.RequesterIp + } + return "" +} + +func (x *SandboxDeleteRequest) GetRequesterUserAgent() string { + if x != nil && x.RequesterUserAgent != nil { + return *x.RequesterUserAgent + } + return "" +} + type SandboxPauseRequest struct { state protoimpl.MessageState `protogen:"open.v1"` SandboxId string `protobuf:"bytes,1,opt,name=sandbox_id,json=sandboxId,proto3" json:"sandbox_id,omitempty"` @@ -2006,13 +2022,17 @@ const file_orchestrator_proto_rawDesc = "" + "\bend_time\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampH\x00R\aendTime\x88\x01\x01\x128\n" + "\x06egress\x18\x03 \x01(\v2\x1b.SandboxNetworkEgressConfigH\x01R\x06egress\x88\x01\x01B\v\n" + "\t_end_timeB\t\n" + - "\a_egress\"k\n" + + "\a_egress\"\xf4\x01\n" + "\x14SandboxDeleteRequest\x12\x1d\n" + "\n" + "sandbox_id\x18\x01 \x01(\tR\tsandboxId\x12$\n" + "\vkill_reason\x18\x02 \x01(\tH\x00R\n" + - "killReason\x88\x01\x01B\x0e\n" + - "\f_kill_reason\"\x99\x01\n" + + "killReason\x88\x01\x01\x12&\n" + + "\frequester_ip\x18\x03 \x01(\tH\x01R\vrequesterIp\x88\x01\x01\x125\n" + + "\x14requester_user_agent\x18\x04 \x01(\tH\x02R\x12requesterUserAgent\x88\x01\x01B\x0e\n" + + "\f_kill_reasonB\x0f\n" + + "\r_requester_ipB\x17\n" + + "\x15_requester_user_agent\"\x99\x01\n" + "\x13SandboxPauseRequest\x12\x1d\n" + "\n" + "sandbox_id\x18\x01 \x01(\tR\tsandboxId\x12\x1f\n" + diff --git a/packages/shared/pkg/grpc/orchestrator/orchestrator_grpc.pb.go b/packages/shared/pkg/grpc/orchestrator/orchestrator_grpc.pb.go index 8ff0d06dee..f14dc9aa2a 100644 --- a/packages/shared/pkg/grpc/orchestrator/orchestrator_grpc.pb.go +++ b/packages/shared/pkg/grpc/orchestrator/orchestrator_grpc.pb.go @@ -1,6 +1,6 @@ // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: -// - protoc-gen-go-grpc v1.6.1 +// - protoc-gen-go-grpc v1.6.2 // - protoc v7.34.1 // source: orchestrator.proto