Skip to content

router, node, standalone: external HTTP ingress for a2a, mcp, and inference services - #838

Merged
aojea merged 4 commits into
google:mainfrom
aojea:external-http-ingress
Nov 1, 2026
Merged

aojea merged 4 commits into
google:mainfrom
aojea:external-http-ingress

Conversation

@aojea

@aojea aojea commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds zero-trust External HTTP Ingress so callers outside the mesh can reach explicitly opted-in a2a://, mcp://, and inference:// services on enrolled agentmesh-node members through agentmesh-router (or agentmesh-one's single port) without routing data-plane traffic through agentmesh-control-plane.

Two-Layer Opt-In (Service Capability + Admin Role Ceiling)

  • Mesh Policy (PolicyRole.allowed_ingress): Admins set the ingress ceiling via repeated IngressGrant, compiled into granted_ingress_* Datalog rules by the control plane and enforced at directory announcement (RequireIngress). Omitting allowed_auth_modes grants INGRESS_AUTH_MODE_EDGE only; PASSTHROUGH requires an explicit role grant.
  • Node Configuration (ServiceIngress): Service owners opt in per service (ingress: in agentmesh-node.yaml), with optional caller narrowing via allow in PolicyBinding.members syntax ("email:alice@example.com", "email:*@example.com", "group:eng").

Default Edge Authentication (INGRESS_AUTH_MODE_EDGE) & Discovery

  • Path-Based URLs: Routers serve <public-url>/ingress/<peer_id>/<service_type>/<service_name>[/<subpath>] (zero wildcard DNS or extra TLS certificates required) plus RFC 9728 /.well-known/oauth-protected-resource, returning 401 Unauthorized with WWW-Authenticate: Bearer resource_metadata="..." when unauthenticated.
  • Credential & Policy Verification: Verifies Delegated Session Biscuits, RFC 8693 task-attenuated Task Biscuits, and trusted OIDC JWTs against mesh Datalog policy, TAR blocks, revocations, banned peers, and ServiceIngress.allow, while rejecting unattenuated standing member/router transport Biscuits.
  • A2A AgentCard Rewriting: GET /.well-known/agent-card.json (and bare service root GET) rewrites supportedInterfaces to the public ingress URL, strips stale backend signatures, and populates OAuth 2.1 / OIDC / Bearer securitySchemes in EDGE mode (preserving backend securitySchemes in PASSTHROUGH mode).
  • End-to-End Router-to-Node Protocol (/mesh/ingress/1.0.0): Streams across local or federated peer routers to the serving node, which admits only authenticated mesh:role:router peers, re-verifies local ingress opt-in and caller authorization end-to-end, strips spoofed X-Mesh-* headers, and injects verified attribution headers (X-Mesh-Ingress, X-Mesh-Principal, X-Mesh-Role, X-Mesh-Task).

Protocol Gate (/mesh/ingress/1.0.0 is router-only)

  • The node serves /mesh/ingress/1.0.0 through a routerHost gate: a stream from any peer that did not authenticate as mesh:role:router (a member included) is reset before the HTTP server reads a byte and counted in agentmesh_node_streams_refused_total{scope="not_router"}. The handler's own X-Mesh-Biscuit + authority-binding + role check stays as the second, independent layer.
  • HandleAuthHandshake now records the role under the key that verified the credential (authRouters alongside authPeers), so a federated router that handshakes inbound is recognised; a ban drops both.
  • TestNodeProtocolInventoryIsClassified lists every protocol a started node registers and fails on any that is not classified as handshake-open, in-stream-credential, or gated; for each gated protocol it opens a stream from an unauthenticated peer and checks it is refused and counted. TestExternalIngressProtocolServesOnlyRouters checks member-reset / router-served / ban. Both fail against the previous code.
  • ingress.allow node: entries are canonicalised at the config boundary (ServiceIngressFromConfig), and --allow-insecure-ingress-http names the plaintext risk in its help text.

Deployment & Operations

  • agentmesh-one: Enabled out of the box on the single port with zero extra flags.
  • Routers, Helm & GKE Testnets: Added --ingress-addr, --ingress-url, --public-control-plane, --ingress-tls-cert-path, --ingress-tls-key-path, and --allow-insecure-ingress-http on agentmesh-router, surfaced RouterInfo.ingress_url on /info and the console (SQLite/Postgres schema v21), and wired charts/agentmesh and .github/k8s Gateway HTTPRoute (/ingress/* -> agentmesh-router).

Validation

  • make, make lint, unit tests (-race), integration tests (TestStandaloneExternalIngressCUJ, TestFederatedMultiRouterIngressRoleCeilingAndBan), and make e2e-test (32/32 Bats tests).

…erence services

Add zero-trust external HTTP ingress so callers outside the mesh can reach
explicitly opted-in a2a://, mcp://, and inference:// services on enrolled
agentmesh-node members through agentmesh-router (or agentmesh-one's single
port) without routing data-plane traffic through agentmesh-control-plane.

Two-layer opt-in:
- Mesh policy admins set the ceiling via PolicyRole.allowed_ingress
  (repeated IngressGrant), compiled into Datalog rules by the control plane
  and enforced at directory announcement (RequireIngress). Omitting
  allowed_auth_modes grants INGRESS_AUTH_MODE_EDGE only; PASSTHROUGH requires
  an explicit role grant.
- Service owners opt in per service via ServiceIngress (ingress: in
  agentmesh-node.yaml), with optional allow narrowing in PolicyBinding.members
  syntax (e.g. "email:alice@example.com", "group:eng").

Default edge authentication (INGRESS_AUTH_MODE_EDGE):
- Routers serve path-based endpoints at
  /ingress/<peer_id>/<service_type>/<service_name>[/<subpath>] plus RFC 9728
  /.well-known/oauth-protected-resource, returning 401 WWW-Authenticate with
  resource_metadata when unauthenticated.
- Verifies Delegated Session Biscuits, RFC 8693 task-attenuated Task Biscuits,
  and trusted OIDC JWTs against mesh Datalog policy, TAR blocks, revocations,
  banned peers, and ServiceIngress.allow, while rejecting unattenuated
  standing member/router transport Biscuits.
- Rewrites public A2A AgentCard URLs and populates OAuth 2.1 / OIDC / Bearer
  securitySchemes on GET /.well-known/agent-card.json (preserving backend
  securitySchemes in PASSTHROUGH mode).
- Streams across local or federated peer routers over /mesh/ingress/1.0.0 to
  the serving node, which admits only authenticated mesh:role:router peers,
  re-verifies local ingress opt-in and caller authorization end-to-end, strips
  spoofed X-Mesh-* headers, and injects verified attribution headers.

Deployment & operations:
- Enabled out of the box with zero extra flags on agentmesh-one.
- Added --ingress-addr, --ingress-url, --public-control-plane,
  --ingress-tls-cert-path, --ingress-tls-key-path, and
  --allow-insecure-ingress-http on agentmesh-router, surfaced RouterInfo.ingress_url
  on /info and the console (SQLite/Postgres schema v21), and wired the Helm
  chart and GKE Gateway HTTPRoute (/ingress/* -> agentmesh-router).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces external HTTP ingress capabilities to Agent Mesh, allowing services to expose endpoints to external callers via the router or standalone single-port gateway. It implements two-layer opt-in (admin role ceiling and node service configuration), supporting both zero-trust edge authentication (OIDC JWT or Biscuit) and passthrough modes. The review feedback highlights style guide violations regarding the direct comparison of raw protobuf peer IDs instead of decoding them first, as well as several early returns that fail to leave a trace (logs or metrics) of the decision.

Comment on lines +394 to +399
for _, rec := range providers {
if rec.GetPeerId() == targetPeer.String() {
providerRec = rec
break
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

According to Rule 1.5 of the Agent Mesh Code Review Style Guide, raw strings from protobuf messages (such as rec.GetPeerId()) must not be compared directly for identity. Both sides should be decoded to peer.ID values and compared, or their .String() values compared after decoding both sides.

	for _, rec := range providers {
		recPID, err := peer.Decode(rec.GetPeerId())
		if err != nil {
			continue
		}
		if recPID == targetPeer {
			providerRec = rec
			break
		}
	}
References
  1. Rule 1.5: Raw string compared for identity. Decode both sides and compare peer.ID values, or compare .String() of two decoded IDs. (link)

Comment thread internal/node/ingress.go
Comment on lines +262 to +265
if n.services == nil {
http.Error(w, "Service registry not initialized", http.StatusServiceUnavailable)
return
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to Rule 6.1 of the Agent Mesh Code Review Style Guide, every decision about a peer or request must leave a trace (either a log line or a counter). The early return when n.services == nil does not log or increment any counter.

		if n.services == nil {
			externalIngressRequestsTotal.WithLabelValues(modeMetric, externalIngressOutcomeInvalid).Inc()
			logger.Error("[ExternalIngress] Service registry not initialized")
			http.Error(w, "Service registry not initialized", http.StatusServiceUnavailable)
			return
		}
References
  1. Rule 6.1: Every decision about a peer leaves a trace. Flag a continue, return or break on an error or a condition that has neither. (link)

Comment on lines +314 to +317
if !r.isReady.Load() || r.shutdown.Load() {
http.Error(w, "Router is not ready", http.StatusServiceUnavailable)
return
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to Rule 6.1 of the Agent Mesh Code Review Style Guide, every decision must leave a trace (either a log line or a counter). The early return when the router is not ready or is shutting down does not log or increment any counter.

Suggested change
if !r.isReady.Load() || r.shutdown.Load() {
http.Error(w, "Router is not ready", http.StatusServiceUnavailable)
return
}
if !r.isReady.Load() || r.shutdown.Load() {
routerIngressRequestsTotal.WithLabelValues(ingressModeUnknown, ingressOutcomeRateLimited).Inc()
logger.Warn("[Ingress] Router is not ready or is shutting down")
http.Error(w, "Router is not ready", http.StatusServiceUnavailable)
return
}
References
  1. Rule 6.1: Every decision about a peer leaves a trace. Flag a continue, return or break on an error or a condition that has neither. (link)

Comment thread internal/router/ingress.go Outdated
Comment on lines +621 to +625
sanitizeRouterOutboundHeaders(pr.Out.Header, stripAuth)
pr.Out.Header.Set(api.HeaderMeshBiscuit, routerBiscuitHeader)
pr.Out.Header.Set(api.HeaderMeshIngress, modeStr)
if callerBiscuitHdr != "" {
pr.Out.Header.Set(api.HeaderMeshIngressCallerBiscuit, callerBiscuitHdr)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to Rule 6.1 of the Agent Mesh Code Review Style Guide, every decision must leave a trace (either a log line or a counter). The early return when json.Marshal fails does not log or increment any counter.

	out, err := json.Marshal(&card)
	if err != nil {
		routerIngressRequestsTotal.WithLabelValues(modeStr, ingressOutcomeBadGateway).Inc()
		logger.Errorf("[Ingress] Failed to marshal AgentCard: %v", err)
		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
		return
	}
References
  1. Rule 6.1: Every decision about a peer leaves a trace. Flag a continue, return or break on an error or a condition that has neither. (link)

@aojea aojea left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

Build, make lint, race unit tests, and both integration tests pass on b196ded3. The two-layer opt-in (role ceiling + node config), the per-request re-check of the provider ceiling at the router, and the end-to-end re-verification at the node are the right shape. The tests are fast and cover the real CUJs.

One architectural point (#1) should change before merge. The rest are concrete gaps, ordered by severity.

High

1. The router becomes a second identity authority (OIDC JWT -> roles at the edge). The control plane explicitly forbids routers from minting identities (HandleTokenExchange: "router credentials cannot exchange tokens"), and nodes never verify JWTs themselves (they call ExchangeSubjectJWT). The new edge path skips the control plane's checks: email_verified is not enforced, IsIdentityBanned is never consulted (a banned user keeps access with fresh IdP tokens), isWorkloadClaims is not applied, and allowedAudiences() always appends api.DefaultAudience. It also makes the node trust router-asserted identity (X-Mesh-Ingress-Claims): any enrolled router can impersonate any user to any edge service. Recommendation: accept only Biscuits at the ingress. JWT holders get the 401 + RFC 9728 metadata that already points at /oauth/token (RFC 8693 jwt -> Biscuit). That removes AuthorizeIngressOIDCFacts, X-Mesh-Ingress-Claims, resolveOIDCProviders, --oidc-issuer/--audience, and the synced issuer/audience state.

2. passthrough + target_auth_path is an open proxy with the operator's credential. backendTarget.apply always overwrites Authorization with the operator secret. In passthrough mode nobody authenticates the caller, so anyone with the peer ID can use the injected API key. Refuse auth_mode: passthrough with target_auth_path at config/registration time and name the risk in the guide.

Medium

  1. Router revocation cache is an LRU sized handshakeLimiterSize (4096) and stores each ID twice -> ~2048 revoked tokens before a deny-list entry is silently dropped. A deny list must not evict.
  2. handleIngressRequest dials and handshakes the provider and extracts target facts before the caller's bearer token is checked. An anonymous client can make the router open connections to any advertised provider. Verify the Biscuit first.
  3. Ingress rate limit is keyed on RemoteAddr. Behind the GKE Gateway every external client shares one IP and one 200 r/s bucket.
  4. leasedIngressURL advertises the bind address: --ingress-addr=0.0.0.0:8090 without --ingress-url (Helm default) leases http://[::]:8090/ingress.
  5. A refused ingress DirectoryProvide also drops the in-mesh name record; the node only logs a Warn and the service is no longer resolvable by name inside the mesh.
  6. Three copies of MCP body inspection (inspectRouterMCPBody, node inspectMCPHTTPRequestBody, envoy.InspectJSONRPCMCPBody) and two copies of header stripping. The auth-free MCP method allowlist is security-relevant and will drift.

Low

  • security-architecture.md 4.3 says client_peer_id prevents replay from another peer. The ingress accepts any /token/exchange-minted token as a pure bearer from the internet. Document this in 4.3 / 5.2.
  • effectiveIngressBaseURL falls back to req.Host, so the Host header controls URLs in WWW-Authenticate and the agent card when nothing else is configured.
  • Default agentmesh-one dev policy grants PASSTHROUGH to all nodes.
  • Integration tests use 20-25s context timeouts (runtime is <0.5s, so fine, but AGENTS.md says <=10s).
  • The internal/node/oidc.go interactive-login changes are unrelated to this PR.

Gemini's four comments are valid but minor; the peer.Decode one is functionally safe today (records are rendered via info.ID.String()), but it is a one-line fix.

Comment thread internal/router/ingress.go Outdated
var callerRes *identity.IngressCallerResult
var callerBiscuitHdr, callerClaimsHdr string

if isCompactJWT(bearerToken) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High. This makes the router an identity authority. The control plane forbids this on purpose (HandleTokenExchange: "router credentials cannot exchange tokens"), and nodes never verify external JWTs themselves. This branch skips: email_verified (control plane drops an unverified email in mintDelegatedBiscuitFromJWT; here it still satisfies email: bindings and allow lists), IsIdentityBanned (a banned user keeps access with fresh IdP tokens), isWorkloadClaims, and the audience list differs (allowedAudiences() always adds api.DefaultAudience).

Suggest: accept only Biscuits here. JWT holders already get a 401 + RFC 9728 metadata pointing at /oauth/token, which is the RFC 8693 jwt -> Biscuit exchange. That also removes X-Mesh-Ingress-Claims, resolveOIDCProviders, --oidc-issuer, --audience, and the synced issuer/audience state.

Comment thread internal/node/ingress.go Outdated
// AgentCard without caller credentials so external A2A clients can
// discover SecuritySchemes before authenticating.
callerBiscuitHeader := strings.TrimSpace(r.Header.Get(api.HeaderMeshIngressCallerBiscuit))
callerClaimsHeader := strings.TrimSpace(r.Header.Get(api.HeaderMeshIngressClaims))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High (follows from the router OIDC path). The node trusts identity asserted by any peer with mesh:role:router. A compromised or misbehaving router can impersonate any user to any edge service on this node. With a caller Biscuit the node re-verifies the token itself, which is the right model. If the router only forwards Biscuits, this header and branch go away.

Comment thread internal/identity/ingress.go Outdated
// AuthorizeIngressOIDCFacts authorizes verified OIDC JWT fact claims against
// mesh Datalog policy, local node attenuation, and ServiceIngress.allow using
// the exact same Biscuit Datalog authorizer as Delegated Session Biscuits.
func AuthorizeIngressOIDCFacts(oidcFacts map[string][]string, p IngressEvalParams) (*IngressCallerResult, error) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the router stops verifying OIDC JWTs (see comment on router/ingress.go), this function, SanitizeOIDCFacts, MarshalIngressClaimsHeader, UnmarshalIngressClaimsHeader, and the ephemeral eval key can all be removed.

Comment thread internal/node/config.go
if typeErr == nil && sType == api.ServiceType_SERVICE_TYPE_EGRESS {
return nil, fmt.Errorf("service %q: egress destinations are assigned by the control plane (PolicyConfig.egress), not declared in the node config", svc.Name)
}
if svc.Ingress != nil {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High. auth_mode: passthrough together with target_auth_path is an unauthenticated open proxy with the operator's credential: backendTarget.apply always overwrites Authorization with the operator secret, and in passthrough mode nobody checks the caller. Please refuse this combination here (and in buildRegisterRequest / NewServiceFromRequest), and name the risk in external-ingress.md.

Comment thread internal/router/router.go Outdated
cancel()
return nil, fmt.Errorf("failed to create ingress rate limiter: %w", err)
}
revokedTokens, err := lru.New[string, time.Time](handshakeLimiterSize)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium. This is a deny list in an LRU. recordRevokedBiscuitID stores each ID under two keys, so ~2048 revoked tokens fit before an entry is silently evicted and a revoked token is accepted again. The node uses RevocationCacheSize (10000). A deny list should not evict; a map pruned by expiry is enough, or at least size it like the node and count evictions.

Comment thread internal/router/ingress.go Outdated
if r.config.IngressTLSCertPath != "" {
scheme = "https"
}
return scheme + "://" + r.ingressAddr.String() + "/ingress"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium. With --ingress-addr=0.0.0.0:8090 and no --ingress-url (the Helm default: ingressUrl: ""), r.ingressAddr.String() is [::]:8090, so the router leases http://[::]:8090/ingress. The control plane accepts it and nodes log it as the public URL. Return "" when the bound host is unspecified.

Comment thread internal/router/ingress.go Outdated
scheme = "https"
}
if req != nil && req.Host != "" {
return scheme + "://" + req.Host + "/ingress"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: this lets the Host header choose the URL that goes into WWW-Authenticate and the rewritten agent card. Only reachable when neither IngressURL nor a bound listener is set, but worth a comment or dropping the fallback.

Comment thread internal/node/service_registry.go Outdated
defer nameCancel()
if err := r.dht.Provide(nameCtx, nameKey, serviceTarget(info)); err != nil {
if err := r.provide(nameCtx, nameKey, serviceTarget(info), info.GetIngress()); err != nil {
logger.Warnf("[ServiceRegistry] Announcing %s by name: %v", info.Name, err)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium. When the router refuses the ingress part of this DirectoryProvide (role ceiling), the whole name record is dropped, so the service is no longer resolvable by name inside the mesh either. This is only a Warn. Per the auditability rule it should be an Error plus a counter, and it would be safer to re-announce without ingress so in-mesh callers keep working while the operator fixes the policy.

Comment thread internal/router/ingress.go Outdated
return false
}

func inspectRouterMCPBody(r *http.Request) (mcpTool string, allowStreamInit bool, err error) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium. This is the third copy of the MCP body inspection (node inspectMCPHTTPRequestBody, envoy.InspectJSONRPCMCPBody), and this PR already had to edit the envoy copy to keep the method allowlist in sync. The auth-free method list is security-relevant. Please move it to one shared function (api/ or internal/identity). Same for sanitizeRouterOutboundHeaders vs node stripExternalIngressHeaders.

// defaultDevPolicyRoles mirrors the Helm bootstrap job's role set with the
// node role opened up for zero-config development use.
func defaultDevPolicyRoles() []*api.PolicyRole {
devIngress := []*api.IngressGrant{{

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: the dev default grants PASSTHROUGH to every node. The PR description frames PASSTHROUGH as needing an explicit grant; even in dev, EDGE-only by default would match the "safe default" rule.

…ernal ingress

- Remove direct OIDC JWT verification from agentmesh-router (--oidc-issuer,
  --audience, HeaderMeshIngressClaims, and node trust in router-asserted OIDC
  claims); EDGE mode on /ingress/ accepts only Mesh Biscuits while returning
  401 + RFC 9728 resource_metadata so JWT callers exchange at POST /oauth/token.
- Verify EDGE caller Biscuit signature, revocation, banned status, TAR rules,
  and ServiceIngress.allow locally before dialing or handshaking any provider.
- Replace the router's bounded LRU revocation cache with a non-evicting,
  expiry-pruned revocationSet, export agentmesh_router_revoked_tokens, and
  enforce revocations across mutual auth, live sessions, and /mesh/ingress.
- Key verified EDGE ingress rate limits per Biscuit revocation ID so callers
  behind an L7 Gateway do not share one SNAT IP bucket, and document Gateway
  source-IP rate limiting for unauthenticated routes.
- Return "" from leasedIngressURL() on unspecified bind hosts (0.0.0.0 / ::)
  and remove req.Host fallbacks from metadata and AgentCard URL rewriting.
- Refuse auth_mode: passthrough combined with target_auth_path or inline
  target_url credentials at config validation and service registration.
- Log errors, increment agentmesh_node_service_provide_failures_total, and fall
  back to internal-only directory announcement when an ingress role ceiling
  rejects ProvideWithIngress.
- Deduplicate MCP JSON-RPC body inspection and external ingress header
  sanitization in api/, decode peer.ID before identity comparisons, add missing
  log/counter traces on early returns, revert unrelated internal/node/oidc.go
  diffs, and restrict agentmesh-one's default dev policy to EDGE-only ingress.
@aojea aojea added this to the v0.2.0 milestone Nov 1, 2026
…l's gate

The external ingress protocol was registered through meshHost, so any peer
that had completed the handshake with the node, a member included, could open
a /mesh/ingress/1.0.0 stream and reach the HTTP server behind it before the
handler's X-Mesh-Biscuit check refused it with 403. The gate now decides
first: routerHost wraps the handler in requireRouter, which waits for the
peer's handshake like requireAuthenticated and then resets the stream of any
peer that did not authenticate as mesh:role:router, logging it and counting
it under agentmesh_node_streams_refused_total{scope="not_router"}. The
handler's credential check stays as the second, independent layer.

To know a peer's role at the gate, HandleAuthHandshake verifies the credential
with VerifyBiscuitAdmission and records the role under the key that verified
it: every admitted peer in authPeers as before, and routers additionally in
authRouters with the same expiry. isAdmittedRouter requires both, so a ban
or an eviction that drops the admission withdraws the router standing with
it. A router the node attached to is already role-verified on that session
and passes through isAuthenticatedAndConnected.

A protocol added without a gate used to go unnoticed until someone read the
code. TestNodeProtocolInventoryIsClassified lists every protocol a started
node registers and fails on any that is not named in one of three sets:
handshakeProtocols (open before the handshake), credentialProtocols (the
handler verifies a Biscuit in the stream) or gatedProtocols. For each gated
protocol it opens a stream from an unauthenticated peer, checks that nothing
is answered, and checks the refusal counter once the peer disconnects.
TestExternalIngressProtocolServesOnlyRouters checks that an admitted member
is reset at the gate and counted, that an admitted router reaches the
handler, and that a ban withdraws router standing. Both fail against the
previous code: the first on an ungated protocol, the second on the member
receiving an HTTP 401 from the handler.
@aojea

aojea commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces external HTTP ingress support for Agent Mesh, allowing inbound services (such as A2A, MCP, and inference) to be securely exposed through the router or standalone single-port configurations. It implements two authentication modes: a zero-trust edge mode that validates Delegated Session Biscuits and enforces fine-grained Datalog and TAR policies, and an opt-in passthrough mode. Feedback on the changes suggests canonicalizing peer IDs at the boundary in ValidatePeerRecordIngress to prevent security bypasses from uncanonicalized strings, and updating the help text for the --allow-insecure-ingress-http flag to explicitly warn operators of the security risks associated with plaintext transmission.

Comment thread api/ingress.go
Comment on lines +134 to +143
for _, member := range ing.Allow {
if err := ValidateBindingMember(member, "ingress.allow"); err != nil {
return err
}
if prefix, val, ok := strings.Cut(member, ":"); ok && prefix == FactNode {
if _, err := peer.Decode(val); err != nil {
return fmt.Errorf("ingress.allow entry %q has invalid peer ID: %w", member, err)
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Group 1: Peer IDs must be canonicalized at the boundary.

In ValidatePeerRecordIngress, the peer ID in member (e.g., node:<peer_id>) is decoded using peer.Decode(val) to validate it, but the decoded canonical peer.ID is discarded, and the raw uncanonicalized member string continues to be used and stored in the ServiceIngress record. This violates the peer ID canonicalization rules.

To fix this, we should update the ing.Allow slice in-place with the canonicalized peer ID string (i.e., "node:" + pID.String()) so that the stored record always holds the canonical form.

	for i, member := range ing.Allow {
		if err := ValidateBindingMember(member, "ingress.allow"); err != nil {
			return err
		}
		if prefix, val, ok := strings.Cut(member, ":"); ok && prefix == FactNode {
			pID, err := peer.Decode(val)
			if err != nil {
				return fmt.Errorf("ingress.allow entry %q has invalid peer ID: %w", member, err)
			}
			ing.Allow[i] = "node:" + pID.String()
		}
	}
References
  1. Rule 1: Peer IDs must be canonicalized at the boundary. Decoded peer IDs must be kept and used, never the raw input string, and raw strings must not be stored in records. (link)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 339ec9c, with one change to the suggestion: the rewrite lives in ServiceIngressFromConfig (the config boundary where operator text enters the mesh), not in the validator, so Validate* keeps no side effect on its input. Matching already decoded both sides (MatchIngressAllow), so this is about what the record stores. Covered by TestServiceIngressFromConfig with a CIDv1 spelling.

Comment thread cmd/agentmesh-router/main.go Outdated
rootCmd.Flags().StringVar(&ingressURL, "ingress-url", "", "Public base URL advertised for external HTTP ingress on this router (e.g. https://bananas.sam-mesh.dev/ingress)")
rootCmd.Flags().StringVar(&ingressTLSCertPath, "ingress-tls-cert-path", "", "Path to TLS certificate file for --ingress-addr")
rootCmd.Flags().StringVar(&ingressTLSKeyPath, "ingress-tls-key-path", "", "Path to TLS private key file for --ingress-addr")
rootCmd.Flags().BoolVar(&allowInsecureIngressHTTP, "allow-insecure-ingress-http", false, "Allow --ingress-addr on a non-loopback interface without TLS (only when behind a TLS-terminating L7 load balancer) and allow http:// in --ingress-url")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Group 6: Behavior must be auditable from logs and metrics.

According to Rule 6.6, any flag that opens up security (such as --allow-insecure-ingress-http) must explicitly name the risk of loosening it in its help text. We should update the help text to warn the operator about the risk of transmitting credentials and data in plaintext over the network if not properly protected by a TLS-terminating proxy.

Suggested change
rootCmd.Flags().BoolVar(&allowInsecureIngressHTTP, "allow-insecure-ingress-http", false, "Allow --ingress-addr on a non-loopback interface without TLS (only when behind a TLS-terminating L7 load balancer) and allow http:// in --ingress-url")
rootCmd.Flags().BoolVar(&allowInsecureIngressHTTP, "allow-insecure-ingress-http", false, "Allow --ingress-addr on a non-loopback interface without TLS (only when behind a TLS-terminating L7 load balancer) and allow http:// in --ingress-url. WARNING: This transmits credentials and data in plaintext over the network if not properly protected by a TLS-terminating proxy.")
References
  1. Rule 6.6: Safe defaults, zero configuration. Flags that loosen security checks must explicitly name the risk of doing so in their help text. (link)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 339ec9c: the help text now names the cost (bearer Biscuits and request bodies cross the network in plaintext between client and router) and the one deployment where that is acceptable.

ServiceIngressFromConfig is where an operator's ingress.allow text enters
the mesh. A "node:<peer_id>" entry was validated by decoding it and then
stored as typed, so a record could carry the CIDv1 spelling of a peer ID
while every comparison decoded both sides again. The config boundary now
rewrites the entry to the canonical base58 form once, and the validator
stays a validator with no side effect on its input.

The help text of --allow-insecure-ingress-http said when to set it but not
what it costs: every caller's bearer Biscuit and request body then cross the
network in plaintext between the client and the router. It now says so and
names the one deployment where that is acceptable.
@aojea
aojea merged commit c556228 into google:main Nov 1, 2026
23 checks passed
aojea added a commit that referenced this pull request Nov 1, 2026
…ingress

- Remove direct OIDC JWT verification from agentmesh-router (--oidc-issuer,
  --audience, HeaderMeshIngressClaims, and node trust in router-asserted OIDC
  claims); EDGE mode on /ingress/ accepts only Mesh Biscuits while returning
  401 + RFC 9728 resource_metadata so JWT callers exchange at POST /oauth/token.
- Verify EDGE caller Biscuit signature, revocation, banned status, TAR rules,
  and ServiceIngress.allow locally before dialing or handshaking any provider.
- Replace the router's bounded LRU revocation cache with a non-evicting,
  expiry-pruned revocationSet, export agentmesh_router_revoked_tokens, and
  enforce revocations across mutual auth, live sessions, and /mesh/ingress.
- Key verified EDGE ingress rate limits per Biscuit revocation ID so callers
  behind an L7 Gateway do not share one SNAT IP bucket, and document Gateway
  source-IP rate limiting for unauthenticated routes.
- Return "" from leasedIngressURL() on unspecified bind hosts (0.0.0.0 / ::)
  and remove req.Host fallbacks from metadata and AgentCard URL rewriting.
- Refuse auth_mode: passthrough combined with target_auth_path or inline
  target_url credentials at config validation and service registration.
- Log errors, increment agentmesh_node_service_provide_failures_total, and fall
  back to internal-only directory announcement when an ingress role ceiling
  rejects ProvideWithIngress.
- Deduplicate MCP JSON-RPC body inspection and external ingress header
  sanitization in api/, decode peer.ID before identity comparisons, add missing
  log/counter traces on early returns, revert unrelated internal/node/oidc.go
  diffs, and restrict agentmesh-one's default dev policy to EDGE-only ingress.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant