feat(ateapi): add actor egress policy API by EItanya · Pull Request #856 · agent-substrate/substrate

Eitan Yarmush (EItanya) · 2026-08-11T14:45:32Z

Part of #823.

Design: https://docs.google.com/document/d/1s2klDhbF2mB5sslwUyZBdyXD7JqRt8FWONwJ7wgsomE/edit?tab=t.0

Summary

add Actor-targeted EgressPolicy CRUD APIs with rules and extensions directly on the resource
add reusable Credential CRUD APIs backed by Kubernetes Secret key selectors
persist policies and credentials in Redis with Actor UID-pinned policy resolution
add the internal unary resolver used once per authenticated CONNECT
default missing or empty policies to deny-all and restrict resolver access to the egress gateway identity

Policy distribution is intentionally on demand for v1; this does not add policy xDS.

Kubernetes Secret read RBAC is a deployment prerequisite and is intentionally not included in this API-only PR.

Testing

go test ./...
bash hack/verify-all.sh

gemini-code-assist

Code Review

This pull request implements the Actor Egress Policy proposal, introducing egress policy bindings and egress credentials along with their CRUD APIs, Redis persistence, validation, and an internal Resolver service. The review feedback highlights critical safety issues in egress_policy.go, specifically potential runtime panics from nil pointer dereferences when cloning policies or accessing Kubernetes secret selectors, as well as a recommendation to improve error handling by not masking internal database errors as permission denied errors.

gemini-code-assist · 2026-08-11T14:49:22Z

+	if err != nil {
+		return nil, fmt.Errorf("while resolving egress policy binding: %w", err)
+	}
+	response := &egresspolicypb.EffectiveEgressPolicy{Policy: proto.Clone(binding.GetPolicy()).(*ateapipb.EgressPolicySpec)}


If binding.GetPolicy() is nil, proto.Clone(nil) will return a nil interface. Attempting to type-assert a nil interface to *ateapipb.EgressPolicySpec will cause a runtime panic. We should check if binding.GetPolicy() is nil before cloning, or handle it safely.

var policy *ateapipb.EgressPolicySpec if binding.GetPolicy() != nil { policy = proto.Clone(binding.GetPolicy()).(*ateapipb.EgressPolicySpec) } response := &egresspolicypb.EffectiveEgressPolicy{Policy: policy}

Obsolete after the API flattening: EgressPolicySpec was removed, so there is no nullable nested policy message to dereference.

gemini-code-assist · 2026-08-11T14:49:22Z

+		selector := credential.GetKubernetesSecret()
+		secret, err := s.kubeClient.CoreV1().Secrets(selector.GetNamespace()).Get(ctx, selector.GetName(), metav1.GetOptions{})


If selector is nil (for example, if the credential source is not a Kubernetes secret or is unset), calling selector.GetNamespace() will cause a nil pointer dereference panic. A defensive check should be added to ensure selector is non-nil before accessing its fields.

Suggested change

selector := credential.GetKubernetesSecret()

secret, err := s.kubeClient.CoreV1().Secrets(selector.GetNamespace()).Get(ctx, selector.GetName(), metav1.GetOptions{})

selector := credential.GetKubernetesSecret()

if selector == nil {

return nil, status.Errorf(codes.FailedPrecondition, "egress credential \"%q\" has no Kubernetes secret selector", name)

}

secret, err := s.kubeClient.CoreV1().Secrets(selector.GetNamespace()).Get(ctx, selector.GetName(), metav1.GetOptions{})

The generated getters are nil-safe, but I added an explicit selector check so malformed (and future unsupported) credential sources clearly fail closed with FailedPrecondition.

gemini-code-assist · 2026-08-11T14:49:22Z

+	actor, err := s.persistence.GetActor(ctx, actorRef)
+	if err != nil || actor.GetMetadata().GetUid() != req.GetActorUid() || actor.GetStatus() != ateapipb.Actor_STATUS_RUNNING {
+		return nil, status.Error(codes.PermissionDenied, "actor is not authorized for egress")
+	}


If s.persistence.GetActor fails due to an internal database error (rather than the actor simply not existing), returning codes.PermissionDenied masks the system failure as a client authorization issue. This can prevent the egress gateway from retrying transient errors. It is better to distinguish between store.ErrNotFound (which should return PermissionDenied or NotFound) and other database errors (which should return codes.Internal or codes.Unavailable).

actor, err := s.persistence.GetActor(ctx, actorRef) if err != nil { if errors.Is(err, store.ErrNotFound) { return nil, status.Error(codes.PermissionDenied, "actor is not authorized for egress") } return nil, status.Errorf(codes.Internal, "failed to resolve actor: %v", err) } if actor.GetMetadata().GetUid() != req.GetActorUid() || actor.GetStatus() != ateapipb.Actor_STATUS_RUNNING { return nil, status.Error(codes.PermissionDenied, "actor is not authorized for egress") }

Fixed: operational actor lookup failures now return Unavailable; a missing actor and actor authorization mismatches remain PermissionDenied.

Julian Gutierrez Oschmann (juli4n) · 2026-08-11T19:06:10Z

  ResourceMetadata metadata = 1;
 }

+// EgressPolicy grants one Actor access to destinations. Rules are ORed;


Is Egress atespaced?

Yes. EgressPolicy is Atespace-scoped: its target Actor must be in the same Atespace, and Credential references resolve within that Atespace. I clarified the proto comment and added a regression assertion for cross-Atespace targets.

Julian Gutierrez Oschmann (juli4n) · 2026-08-11T22:25:10Z

+  repeated EgressRule rules = 4;
+  // Every extension is required. An enforcement point that does not
+  // understand one must fail closed.
+  repeated google.protobuf.Any extensions = 5;


I don't think we should use google.protobuf.Any in our public API. An any field is just a bytes blob and cannot be interpreted by any system that doesn't link against the proto. This means that, for example, a substrate client won't be able to even deserialize / print it unless it links against every single extension proto. I think this will also complicate updates.

The reasoning can be foudn here: https://docs.google.com/document/d/1s2klDhbF2mB5sslwUyZBdyXD7JqRt8FWONwJ7wgsomE/edit?tab=t.0#heading=h.7hkev5flhg13

TLDR is that policy computation and delivery is left to Substrate because of scale, so in order to have custom policy it has to be colocated on the object. This is the best way to do that in protobuf. Also, this will be an implementation detail in those instances.

I'm of course open to suggestions on better ways.

Julian Gutierrez Oschmann (juli4n) · 2026-08-11T22:32:11Z

+  oneof target {
+    ObjectRef actor = 2;
+  }
+  google.protobuf.Empty allow_all = 3;


Why is this an Empty message?

It's a signifier, we can also make it a bool if we want.

Julian Gutierrez Oschmann (juli4n) · 2026-08-11T22:37:59Z

+const egressGatewayPrincipal = "spiffe://cluster.local/ns/ate-system/sa/atenet-egress"
+
+var (
+	egressPolicyMutableFields = mutableFields[*ateapipb.EgressPolicy]{


FYI: You can remove this after you rebase on main. See #862

haiyanmeng · 2026-08-12T13:18:52Z

+}
+
+message IPBlockMatch {
+  repeated string cidrs = 1;


Do we need to do credential injection for IPBlackMatch?

haiyanmeng · 2026-08-12T13:20:40Z

This change seems unrelated to this PR.

haiyanmeng · 2026-08-12T13:21:17Z

@@ -1322,7 +1320,7 @@ func TestUploadLocalCheckpointDir(t *testing.T) {
 	fullRec := func(class string) sandboxAssetsRecord {


The change to this file seems unrelated to this PR.

Eitan Yarmush (EItanya) marked this pull request as draft August 11, 2026 14:47

gemini-code-assist Bot reviewed Aug 11, 2026

View reviewed changes

Eitan Yarmush (EItanya) force-pushed the egress-policy-api branch 4 times, most recently from 42fe4a0 to 4c4d5fc Compare August 11, 2026 15:24

Eitan Yarmush (EItanya) marked this pull request as ready for review August 11, 2026 16:25

Eitan Yarmush (EItanya) force-pushed the egress-policy-api branch from 4c4d5fc to 851f291 Compare August 11, 2026 16:28

Julian Gutierrez Oschmann (juli4n) reviewed Aug 11, 2026

View reviewed changes

Eitan Yarmush (EItanya) force-pushed the egress-policy-api branch 2 times, most recently from 1225585 to 279aa81 Compare August 11, 2026 20:30

Julian Gutierrez Oschmann (juli4n) reviewed Aug 11, 2026

View reviewed changes

Eitan Yarmush (EItanya) force-pushed the egress-policy-api branch from 279aa81 to 99bc40e Compare August 12, 2026 00:03

feat(ateapi): add actor egress policy API

817f5f4

Eitan Yarmush (EItanya) force-pushed the egress-policy-api branch from 99bc40e to 817f5f4 Compare August 12, 2026 11:57

haiyanmeng reviewed Aug 12, 2026

View reviewed changes

		selector := credential.GetKubernetesSecret()
		secret, err := s.kubeClient.CoreV1().Secrets(selector.GetNamespace()).Get(ctx, selector.GetName(), metav1.GetOptions{})

		@@ -1322,7 +1320,7 @@ func TestUploadLocalCheckpointDir(t *testing.T) {
		fullRec := func(class string) sandboxAssetsRecord {

Uh oh!

Conversation

Eitan Yarmush (EItanya) commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 11, 2026

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

Eitan Yarmush (EItanya) commented Aug 11, 2026 •

edited

Loading