sdk: an egress floor, stated at join and held on every call - #538
Conversation
join({ egressRequireLabels }) / join(egress_require_labels=) is
sam-node's egress.require_labels for an SDK member: every provider the
session calls must attest all of its pairs, on top of a call's required
labels, MCP and HTTP alike, however the peer was named. A call cannot
waive or widen it; an unmet floor is a LabelsNotSatisfiedError.
The HTTP path (request, fetch, MeshTransport) now verifies the provider
through the mutual /sam/auth handshake before sending anything, floor or
not, as the MCP path and sam-node's egress proxy already did; a positive
verdict is kept per peer for five minutes, sam-node's labelGateTTL.
The integration runners hold a floor the mesh satisfies, and one extra
runner per SDK, whose floor nobody attests, is refused by the node and by
the other members before anything is sent.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for egress floors in both the JavaScript and Python SDKs, aligning their behavior with sam-node's egress.require_labels. When joining a mesh session, an egress floor can be specified (via egressRequireLabels or egress_require_labels), requiring all called providers to attest to all specified label pairs for both MCP and HTTP outbound calls. The implementations verify providers, enforce the conjunction of these labels, and cache positive verdicts for five minutes. Conformance runners, documentation, unit tests, and integration tests have been updated accordingly. As there are no review comments to evaluate, I have no additional feedback to provide.
aojea
left a comment
aojea
left a comment
There was a problem hiding this comment.
Checked the semantics against sam-node (api.LabelCheck, api.LabelFloorCheck, VerifyPeerLabels/checkPeerLabels). They agree on the points that matter:
- Caller requirement is a disjunction:
or/some/any. Empty or absent is no requirement in all three (len(required) > 0,!required || pairs.length === 0,not required). - Floor is a conjunction:
,/every/all. Empty or absent is no floor in all three (len(floor) > 0,!requiredwithevery([]) === true,not required). - Both apply independently when both are set; the tests in both SDKs cover the cross cases where one's pairs would otherwise stand in for the other's.
- The gate runs with no requirement and no floor and still verifies an enrolled, bound credential; the impostor case covers it.
- Positive verdicts are kept five minutes, misses never. Keying the cache on the peer alone is enough here because the floor is fixed for the session and the HTTP path carries no per-call requirement, which is why
sam-nodekeys on(peer, required, floor). - The runners read a blank
SAM_SDK_EGRESS_REQUIRE_LABELSas no floor, asparseRequiredLabels("")does.
Two divergences inline: the provider role that checkPeerLabels requires and the SDK gates do not, and the runners' env parser turning a malformed floor into no floor. Two style nits on the docs. discover() does not apply the floor to gossip labels the way rankProviders does; that is ranking only, enforcement is unchanged, so no action needed.
| return conn; | ||
| } | ||
| const provider = await authenticateWithPeer(conn, this.mesh.authFrame(), this.mesh.credential.controlPlaneKeys); | ||
| requireEgressLabels(provider, this.#egressRequireLabels); |
There was a problem hiding this comment.
checkPeerLabels also requires role("sam:role:node") on the provider ("only nodes host services"), so on sam-node a router's or an admin's credential never passes the gate even when it attests the floor. This gate (and openMCPSession) stops at verifyPeerBiscuit. Since the PR states parity with VerifyPeerLabels on the HTTP path, a requireRole(provider, ROLE_NODE) here (and its Python twin in _egress_peer) would close the gap; the MCP path has the same gap, pre-existing.
There was a problem hiding this comment.
Done: requireRole(provider, ROLE_NODE) runs between authenticateWithPeer and the label check, so the order is checkPeerLabels's (verify, role, labels). Both SDKs' HTTP-path tests now include a router-role credential that attests the floor and is refused. The MCP path is left as it was; it is the same one line in openMCPSession / open_mcp_session if you want it in this PR.
There was a problem hiding this comment.
Done: the same requireRole(provider, ROLE_NODE) / require_role(provider, ROLE_NODE) now runs in openMCPSession and open_mcp_session right after the provider's credential verifies, before the requirement and the floor, so both paths match checkPeerLabels. Each SDK has a test with a router-role provider that attests the floor and is refused on the MCP path.
| if self._egress_verdicts.get(str(peer_id), 0.0) > time.monotonic(): | ||
| return peer_id | ||
| provider = await authenticate_with_peer(self.host, peer_id, self.mesh.auth_frame(), self.mesh.credential.control_plane_keys) | ||
| require_egress_labels(provider, self.egress_require_labels) |
There was a problem hiding this comment.
Same as the JS side: sam-node's checkPeerLabels requires the node role on the provider before evaluating the requirement and the floor. require_role(provider, ROLE_NODE) here would match it.
There was a problem hiding this comment.
Done: require_role(provider, ROLE_NODE) before require_egress_labels in _egress_peer, same order as checkPeerLabels.
| return Object.fromEntries( | ||
| (process.env[name] ?? "") | ||
| .split(",") | ||
| .filter((pair) => pair.includes("=")) |
There was a problem hiding this comment.
Fail-open for a floor: SAM_SDK_EGRESS_REQUIRE_LABELS=teamnobody (no =) parses to {}, which main then reads as "no floor". sam-node's parseRequiredLabels rejects a non-blank specification that yields no pair for exactly this reason (a typo must not switch the gate off), and its config validates require_labels at load. This is the test runner, and the same parser already served SAM_SDK_LABELS, but for the floor the safe direction is to fail: throw on a pair without = (or on non-blank input that produces no pair). Same for _labels_from_env in the Python runner.
There was a problem hiding this comment.
Done in both runners: a pair without = now fails the runner (blank input is still no floor), as parseRequiredLabels does. The helper is shared with SAM_SDK_LABELS, so a malformed enrollment label fails too instead of being dropped.
| `egress.require_labels` does for a `sam-node`. It is stated once at `join` | ||
| and held for the session, on every call and however the peer was named; | ||
| the agent's calls cannot waive or widen it. It is the program author's | ||
| floor, not the operator's: nothing outside the process sets it. |
There was a problem hiding this comment.
Style: "It is the program author's floor, not the operator's: nothing outside the process sets it." is an antithesis with a colon fragment. Suggest: "The floor belongs to the program that calls join; no configuration outside the process sets it."
There was a problem hiding this comment.
Taken as suggested.
| `MeshTransport`) verifies the provider with or without a floor, through | ||
| the mutual `/sam/auth/1.0.0` handshake, as `sam-node`'s `VerifyPeerLabels` | ||
| does before its egress proxy sends anything; a positive verdict is kept | ||
| per peer for five minutes (`labelGateTTL`), a miss never. Refusals are |
There was a problem hiding this comment.
Style: "a positive verdict is kept per peer for five minutes (labelGateTTL), a miss never" is a fragment. Suggest: "a positive verdict is kept per peer for five minutes (labelGateTTL); a refusal is not kept."
There was a problem hiding this comment.
Taken, with the next sentence folded in: "; a refusal is not kept. An unmet floor is a LabelsNotSatisfiedError naming the floor."
…the runner sam-node's checkPeerLabels requires the node role on the provider before the requirement and the floor are evaluated; the HTTP-path gate in both SDKs now does the same, so a router's or an admin's credential attesting the floor is still not a provider. The runners' label parser refused nothing: a pair without "=" was dropped and a floor spelled wrong became no floor. It now fails on such a pair, as sam-node's parseRequiredLabels does. Two doc sentences reworded.
Better to get all the things fixed in one PR Can you fix MCP path too? |
The same node-role check as on the HTTP path, in openMCPSession and open_mcp_session right after the provider's credential verifies, as sam-node's checkPeerLabels runs it for every service call.
2 participants
join({ egressRequireLabels })(join(egress_require_labels=)) issam-node'segress.require_labelsfor an SDK member: labels every provider the session calls must attest, all of them, on top of a call's required labels, on every outbound call, MCP and HTTP alike, however the peer was named. It is stated once at join and held for the session; a call cannot waive or widen it. An unmet floor is aLabelsNotSatisfiedError, in both SDKs.The HTTP path (
request,fetch, PythonMeshTransport) now verifies the provider through the mutual/sam/auth/1.0.0handshake before sending anything, floor or not, as the MCP path andsam-node's egress proxy (VerifyPeerLabels) already did. A positive verdict is kept per peer for five minutes,sam-node'slabelGateTTL; a miss is never kept. A banned peer is still refused atconnect().Tests: unit tests in both SDKs (floor met and missed on the MCP and the HTTP path, an unenrolled provider refused on the HTTP path, the cached verdict); the integration runners take
SAM_SDK_EGRESS_REQUIRE_LABELS, every member holds a floor the mesh satisfies, and a newegress-floorsubtest runs one extra member per SDK whose floor nobody attests, refused by the node and by the other members.Docs:
sdk/README.md, the native SDKs guide and both SDK READMEs.