Skip to content

feat(node): expose authenticated identity evidence - #298

Open
rhein1 wants to merge 1 commit into
google:mainfrom
rhein1:codex/sam-transport-identity-20260823
Open

feat(node): expose authenticated identity evidence#298
rhein1 wants to merge 1 commit into
google:mainfrom
rhein1:codex/sam-transport-identity-20260823

Conversation

@rhein1

@rhein1 rhein1 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Problem

A local owner/control-plane process needs to authenticate its SAM node and independently verify fresh remote-provider identity evidence before any invocation. The original nonce-signed MCP tool duplicated SAM's trust machinery and did not expose the remote evidence that an independent verifier needs.

Change

Following maintainer direction, this revision exposes two authenticated, read-only HTTP endpoints using public messages defined in api/sam.proto and serialized with protojson:

  • GET /sam/identity returns api.IdentityEvidenceResponse: the local PeerID, raw Biscuit, Biscuit expiration, control-plane URL, trusted Ed25519 SPKI DER key set, and check time.
  • GET /sam/peer/{peer_id}/evidence performs one fresh existing AuthProtocolID handshake and returns api.PeerEvidenceResponse: one bound PeerID, the raw Biscuit, selected Ed25519 SPKI DER verifying key, roles, labels, expiration, revocation IDs, and check time.

A 200 is the node's fail-closed verdict. Independent callers recompute from the raw Biscuit and keys; the response no longer contains self-reported verification/enrollment booleans, duplicate PeerIDs, derivable fingerprints, fixed cache/revocation fields, or ad-hoc schema-version strings. Binary values are protobuf bytes, timestamps are Unix seconds, and errors use the sidecar's plain-text convention.

Both endpoints require the filesystem-protected Unix socket or verified mTLS. A bearer-authenticated plain TCP connection is rejected. The remote endpoint performs the intended pre-fetch and post-verification revocation checks and fails closed if requested, authenticated-connection, and Biscuit-bound PeerIDs differ, if the peer is revoked/banned, or if the selected control-plane key leaves the trusted set during verification.

This reuses SAM's existing libp2p TLS identity, mutual Biscuit handshake, VerifyBiscuitAndGetKey, key rotation, and revocation machinery. It adds no dependency and grants no discovery, invocation, routing, payment, or settlement authority.

Operator-flow integration coverage

tests/integration/identity_evidence_test.go starts two real sam-node processes against the existing mock control plane, enrolls both, connects the owner to the provider over libp2p, and reads both evidence endpoints through the owner's Unix socket before any provider request. It is intentionally focused on socket access, enrollment, and the peer handshake; generated response-field semantics and fail-closed cases remain in unit tests. It does not invoke a tool or provider.

Validation

Passed on signed head bdafa65413057886a22258347542775f076c71a5 in a clean Go 1.25.7 Linux container:

  • generated protobuf output reproduced byte-for-byte with protoc 3.21.12 and protoc-gen-go 1.36.12
  • go test ./api -count=1
  • go test ./internal/node -count=1
  • go test ./tests/integration -run TestIdentityEvidenceOperatorFlow -count=1 -timeout=10s (1.174s)
  • go vet ./api ./internal/node ./tests/integration
  • git diff --check

Hosted CI is authoritative for the complete repository matrix.

@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 a new local transport identity attestation tool get_transport_identity to the MCP server, allowing nodes to sign a nonce-bound statement proving their local node key and expected trusted control-plane key. Feedback on the implementation suggests optimizing lock contention by performing cryptographic operations outside of the read lock in trustsControlPlaneFingerprint, and improving error handling when reading the node public key by wrapping the original error and separating the key size validation.

Comment thread internal/node/transport_identity.go Outdated
Comment thread internal/node/transport_identity.go Outdated
@rhein1
rhein1 force-pushed the codex/sam-transport-identity-20260823 branch from 93507fc to 0a3ca54 Compare August 23, 2026 20:45
@rhein1

rhein1 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Interop follow-up, explicitly not a merge condition: Agoragentic will not treat public SAM discovery as permission to invoke an unknown provider. After this transport-identity primitive is accepted, we are looking for one independently operated, deterministic, free/read-only MCP fixture for a bounded conformance canary: exact PeerID/service/tool/schema, explicit operator consent and revocation contact, immutable UTC window, one zero-value/no-settlement call at most, no credentials or private data, and no retry. If a maintainer or community operator runs such a fixture on a SAM mesh, please point us to the appropriate operator contact. If no suitable fixture exists, we will keep the production activation gate closed.

@rhein1

rhein1 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

@aojea Maintainer review requested on exact signed head 0a3ca548b4826884a5da28af9753e094dc2c5921. All hosted checks are green. The two Gemini findings (crypto under the keys lock and discarded Raw() error/size conflation) are fixed and their threads are resolved. Scope remains one read-only get_transport_identity tool: nonce-bound proof of the local node key and expected trusted control-plane key, with no discovery, provider invocation, routing, payment, or settlement authority. Please review when available; Agoragentic will keep its production SAM gate default-off until upstream acceptance.

@aojea

aojea commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

hi @rhein1 thank you very much, but I need to get a bit more context to be completely sure I fully understand the problem you are trying to solve.

Machinery already in tree that may overlap with this

Node identity is the libp2p PeerID. The host is created with
libp2p.Identity(privKey) and libp2p.Security(libp2ptls.ID, ...), so every
mesh connection already proves possession of that exact Ed25519 key as part of
the TLS handshake — and that proof is bound to the channel.

Every node carries a control-plane-signed biscuit. Minted at enrolment
(/register -> identity.MintBiscuitToken), held by the node, retrievable via
n.GetIdentity() and stored locally. It attests role, labels, granted services, expiry and
revocation ids.

A mutual identity handshake already exists. In internal/node/middleware.go,
a node that successfully authenticates a peer replies with its own biscuit.

The biscuit is holder-of-key, not bearer. Authorize requires the token to
contain node(<connecting peer id>), where the peer ID comes from the
authenticated libp2p connection, and BaselineReplayCheck pins
client_peer_id == connection_peer_id. A stolen biscuit is useless without the
node's libp2p private key. This is why we can expose it safely.

"Challenge a provider before invoking it" is implemented.
internal/node/labels_gate.go: fetchPeerBiscuit opens api.AuthProtocolID,
VerifyBiscuitAndGetKey verifies against the trusted control-plane key set bound
to the peer ID (and tells you which key verified, which is the rotation
selection you hand-rolled), then evaluates the requirement. Positive verdicts are
cached with a TTL.

And it's already exposed to clients. call_remote_tool takes
required_labels: "Comma-separated key=value pairs (e.g.
'region=us-east-1,team=platform'). Fails closed: the call is rejected unless the
peer attests any one of them." find_remote_tools also returns each provider's
declared labels. That is a working "discovery is not permission" gate today.

Trust-anchor distribution exists. GET <control-plane>/keys is
unauthenticated and returns every currently valid control-plane public key as an
api.KeysResponse. An off-node verifier can already fetch the anchor set and
verify a biscuit itself.

Key rotation is handled. Nodes learn new control-plane keys from signed
GossipSub KEY_ROTATION events; routers sync from /keys; verification tries
all trusted keys with a grace period.

Revocation exists. A revokedPeers cache checked on every authorization,
fed by /admin/revoke, plus biscuit revocation ids.

The local API already has three authentication modes. Unix socket
(filesystem permissions), shared-secret token, and TLS/mTLS with
RequireAndVerifyClientCert — see StartSidecarServer.

Questions

  1. Where does your verifier run relative to the node — same host, same trust
    domain, or across a network you don't control?
  2. Are you challenging your own node, or a provider's node? The code in this
    PR does the former; the description seems to indicate is for the latter.
  3. Given required_labels on call_remote_tool plus the biscuit's attested
    facts — what attribute does your policy need that the mesh doesn't already
    attest? T If the answer is "nothing",
    the gap is discoverability, not cryptography.
  4. Is the node's verdict enough, or do you need the raw evidence to re-verify
    independently? "Return verified facts" and "return the biscuit" are different
    APIs and we can expose the one you need.

@rhein1

rhein1 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Thank you. This exposed an ambiguity in our PR wording. Exact answers:

  1. The verifier is a separate Agoragentic owner job in the same ephemeral private container/trust domain as our own sam-node. It reaches the authenticated local MCP surface over loopback HTTP with the bearer-token mode. It is not running across an untrusted network. The token authenticates the caller to the node, but plain loopback HTTP does not authenticate the node back to the caller.

  2. The current code intentionally challenges our own local node. It is not a provider challenge. Remote provider identity and authorization remain SAM's libp2p TLS plus mutual biscuit path. Our description conflated those two checks, and we should correct it.

  3. No additional SAM-attested provider attribute is missing. Agoragentic's operator consent, principal grant, exact target binding, and zero-value/no-settlement policy are external policy and should not be encoded as SAM labels. required_labels is sufficient for attributes SAM attests. The only purpose of this PR is to prove that the local endpoint owns the pinned SAM node key and currently trusts the pinned control-plane key before our owner job relies on that node's discovery or authorization verdicts.

  4. For the remote provider, we need private raw evidence so the owner job can independently re-verify it and bind a hash into a durable receipt, rather than relying only on a boolean verdict. The useful evidence is the provider biscuit, the connection-authenticated PeerID, the selected verifying control-plane key/fingerprint, and the relevant expiry/revocation facts. The raw biscuit would remain private and ephemeral; only bounded hashes and verification status would be retained. /keys can provide the public anchor set.

Given the machinery you described, I agree this PR should not be treated as a new provider-challenge primitive. I see two narrow paths:

  • keep and reword it only if a token-authenticated local client needs explicit server-to-client proof of the node key and current trust set; or
  • withdraw/replace it with an authenticated, read-only preflight that exposes SAM's existing provider biscuit and verification-key selection before invocation.

If Unix socket or mTLS plus an evidence/preflight API is the canonical SAM design, we should use that and avoid parallel cryptography. Which direction would you prefer?

@aojea

aojea commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

I think we should go second path.

For the local check, use the Unix socket. In withAuth
(internal/node/sidecar.go) socket callers skip the token entirely, since being
able to open the socket already proves you're the user that owns it. If something forces you
onto TCP we can use mTLS --tls-cert/--tls-key.

Once the connection itself is trustworthy the rest is just a read. I think new local endpoint that allows to get the identities will solve your problem:

GET /sam/identity

{
  "peer_id": "12D3KooWLTp...",
  "control_plane_url": "https://cp.example",
  "trusted_control_plane_keys": [
    { "fingerprint": "sha256:9f86d0...", "received_at": "2026-08-24T14:02:11Z" }
  ],
  "enrolled": true,
  "biscuit": "<base64>",
  "biscuit_expires_at": "2026-08-25T14:02:11Z"
}

I'd keep your fingerprint format (sha256 over the SPKI DER) so any pinning config
you already have keeps working.

For the preflight:

GET /sam/peer/{peer_id}/evidence

{
  "peer_id": "12D3KooWProvider...",
  "verified": true,
  "biscuit": "<base64 raw bytes>",
  "verifying_key_fingerprint": "sha256:2c26b4...",
  "attested": { "role": ["sam:role:node"], "labels": { "region": "us-east-1" } },
  "expiration": "2026-08-25T09:00:00Z",
  "revocation_ids": ["a3f1...", "bb90..."],
  "checked_at": "2026-08-24T16:40:02Z"
}

Most of this exists already. fetchPeerBiscuit in labels_gate.go
opens the auth stream, and the peer id there comes from the authenticated libp2p
connection rather than anything the peer claims.
identity.VerifyBiscuitAndGetKey already returns which trusted key verified the
token. We just never exposed any of it to the local client.

IMHO both of these should be plain HTTP endpoints rather than MCP tools. The MCP
surface is what the model chooses to call. does the evidence list above cover what you need to bind into the
receipt, or is something missing?

NOTE: On doing so we we can add a field with the node id to get_mesh_info, it returns connected_peers and router_peer_id but not your own ID, so an agent asked "what's my node id?

@rhein1

rhein1 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

The second path is the right one, and plain authenticated HTTP endpoints are a better boundary than an MCP tool. The model should not choose whether identity evidence is collected.

The proposed split also matches our two checks:

  • GET /sam/identity: authenticate the local SAM node and its current trust set.
  • GET /sam/peer/{peer_id}/evidence: obtain fresh remote-peer evidence before any tool invocation.

The provider payload is almost sufficient for our private receipt verifier. We need these semantics made explicit:

  1. Independent signature verification material. Alongside verifying_key_fingerprint, return the selected public key bytes in a canonical form such as verifying_key_spki_der_base64, or return a stable locally trusted key-set snapshot containing that key. A fingerprint alone lets us compare a pin but cannot verify the raw Biscuit. We will recompute the SHA-256 SPKI fingerprint ourselves.
  2. Authenticated channel binding. Return the PeerID derived from the authenticated libp2p stream separately from the requested path value, plus a boolean or structured verdict showing that the Biscuit's node binding matched that connection PeerID. The endpoint must fail closed if requested, connected, and Biscuit-bound identities differ.
  3. Current revocation and freshness. Return the current peer-revocation verdict, Biscuit expiration, checked_at, and, if any positive result is cached, fetched_at/cache_expires_at. A peer present in the local revocation cache must never produce verified: true. The Biscuit revocation IDs are useful evidence, but they do not by themselves state whether revocation is currently effective.
  4. Versioned closed response. Add a schema/version field so the owner job can reject unknown or missing evidence fields rather than silently accepting a widened response.

The raw Biscuit, roles, labels, expiration, revocation IDs, selected key, and connection-bound PeerID then cover what Agoragentic needs. We do not need SAM to carry Agoragentic operator consent, Principal Authority, target binding, or zero-value/no-settlement policy; those remain separately resolved platform authority.

Adding the local node ID to get_mesh_info is useful for agents, but /sam/identity is the authoritative owner-job source for this flow.

With those additions, I will rework this PR away from get_transport_identity and toward the two authenticated HTTP reads you proposed, using Unix socket by default and mTLS only when TCP is unavoidable.

@rhein1 rhein1 changed the title feat(node): add local transport identity attestation feat(node): expose authenticated identity evidence Aug 24, 2026
@rhein1

rhein1 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@aojea Reworked as requested at exact signed head 19fb4c1.\n\nThe model-callable get_transport_identity MCP tool is gone. This head adds two read-only HTTP endpoints:\n\n- GET /sam/identity\n- GET /sam/peer/{peer_id}/evidence\n\nBoth require a strongly authenticated local transport: Unix socket by default, or verified mTLS on TCP. Bearer-only/plain TCP is rejected for these evidence reads. The peer endpoint reuses etchPeerBiscuit and VerifyBiscuitAndGetKey, returns the connection-derived, requested, and Biscuit-bound PeerIDs with a fail-closed binding verdict, selected trusted Ed25519 SPKI DER key material/fingerprint, raw Biscuit, labels/roles, expiry, revocation IDs/current revocation verdict, and freshness fields under closed versioned schemas. It performs fresh evidence retrieval and revalidates revocation and the selected trust key before returning.\n\nLocal validation at this exact head:\n- go test ./internal/node -run Evidence -count=1\n- go test ./internal/node -count=1\n- go vet ./internal/node\n- git diff --check\n\nThe code-bearing hosted checks observed so far are green. Bats e2e tests and �2e-android did not start: GitHub annotated both with the repository/account billing or spending-limit runner-allocation error, so they produced no test evidence.\n\nPlease review the endpoint contract and implementation when available. Separately, if you know an independently operated SAM mesh exposing a deterministic free/read-only fixture, please connect us with its operator. Agoragentic will keep its private SAM client and all live authority default-off until upstream contract review plus explicit operator consent, exact PeerID/tool/schema binding, and a bounded zero-value/no-settlement window exist.

@aojea

aojea commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

you need to rebase to avoid the problem with the runners, will review later.

See tests/integration or tests/e2e for adding a test that guarantees the end to end feature works as we designed, basically mimicking what your operator will do, so guarantee that flow is consistent and does not break you in the future

@aojea

aojea commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

CI fixed in #312
Please rebase.
Apologies for the inconvenience

@rhein1
rhein1 force-pushed the codex/sam-transport-identity-20260823 branch from 19fb4c1 to e1f5561 Compare August 25, 2026 15:22
@rhein1

rhein1 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@aojea Rebased onto current google/sam main (7453c4ad, including the CI repair from #312) and added the requested integration coverage.

Exact signed head: e1f5561f8ee9af8896873d6eefe318f7d9030b5a

The new tests/integration/identity_evidence_test.go starts two real sam-node processes, enrolls them through the existing mock control plane, connects the owner to the provider over libp2p, then exercises /sam/identity and /sam/peer/{peer_id}/evidence through the owner Unix socket. It verifies the closed schemas, three-way PeerID binding, SPKI fingerprint, provider role, revocation verdict, and fresh/non-cached evidence before any provider or tool invocation.

Focused race tests, make build, and vet pass on Go 1.26.3. The PR body records the exact commands and the unrelated local Windows-mount CRLF artifact from the broader suite. Hosted exact-head CI is now running.

@rhein1

rhein1 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head hosted CI is complete for e1f5561f8ee9af8896873d6eefe318f7d9030b5a: every executed check passed, including the main Go test suite, lint, Bats e2e, kind-mesh e2e, Android e2e, security scan, and govulncheck. The PR is now CLEAN/MERGEABLE with no unresolved review threads. Ready for maintainer review; I have not merged it.

@aojea

aojea commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Thanks. One blocking issue, then a few smaller points.

public API schemas must be defined in api/sam.proto

These are two new public sidecar endpoints, but their wire schemas are unexported Go structs with JSON tags in internal/node/identity_evidence.go that will make it hard to be consumed by others, they are also stamped with ad-hoc version strings (sam.identity.v1, sam.peer-evidence.v1, sam.evidence-error.v1).
The repo policy is that public API surfaces go through api/sam.proto, and the sidecar already follows it: handleRegisterService consumes api.RegisterServiceRequest via protojson, discovery returns []*api.DiscoveredProvider.

Please:

  1. Define the messages in api/sam.proto and serialize with protojson.Marshal (already a dependency, no go.mod change).

  2. Declare biscuit / SPKI DER as bytes (protojson base64-encodes automatically) and timestamps as int64 unix seconds like the rest of the file — this deletes most of the manual base64/RFC3339 plumbing in buildIdentityEvidence/buildPeerEvidence.

  3. Drop the schema: "sam.*.v1" strings. Proto field numbers are the compatibility mechanism; a parallel versioning scheme for just two endpoints fragments the API.

  4. Regenerate with hack/gen-proto.sh so verify-generated passes.

  5. While moving the schema, shrink it. I prefer to start small,:

    • verified, peer_binding_verified, enrolled are always true in a 200 — the handler fails closed, so the status code is the verdict. A self-reported verification flag also carries no evidentiary weight for an independent verifier, which must recompute from the raw biscuit anyway.
    • requested_peer_id / connection_peer_id / biscuit_peer_id are guaranteed equal (mismatch is an error), so one peer_id suffices.
    • cache_hit (always false), cache_expires_at (always null), revocation.source (fixed string), peer_revoked (always false) are constants.
    • Fingerprints are sha256 of the SPKI DER already in the response — derivable by the caller.
    • fetched_at / checked_at / verifying_key_received_at are three timestamps within milliseconds; one checked_at tells the story.

Some concrete proposal

// ============================================================================
// Identity Evidence API
// ============================================================================
//
// Read-only owner/control-plane material served by the node sidecar on
// GET /sam/identity and GET /sam/peer/{peer_id}/evidence. Only reachable
// over the filesystem-protected Unix socket or verified mTLS. A 200 asserts
// the node verified the biscuit, its PeerID binding, and revocation state at
// checked_at; independent verifiers recompute from the raw biscuit and keys.

message IdentityEvidenceResponse {
  string peer_id = 1;
  bytes biscuit = 2;
  int64 biscuit_expires_at = 3;
  string control_plane_url = 4;
  repeated bytes trusted_control_plane_keys = 5; // Ed25519 SPKI DER
  int64 checked_at = 6;
}

message PeerEvidenceResponse {
  string peer_id = 1;
  bytes biscuit = 2;
  bytes verifying_key = 3; // Ed25519 SPKI DER, member of the trusted set
  repeated string roles = 4;
  map<string, string> labels = 5;
  int64 expiration = 6;
  repeated string revocation_ids = 7; // hex
  int64 checked_at = 8;
}

The principle: return the raw evidence (biscuit, keys) plus minimal convenience claims, never the server's assertions about its own trustworthiness. roles/labels/expiration are kept as conveniences, but the biscuit is authoritative.

Non-blocking

  • Error format: the sidecar uses plain-text http.Error everywhere; this PR introduces a JSON error envelope for only two endpoints. Prefer plain text for consistency — if we want structured errors, that should be a sidecar-wide proto change, not a bespoke third style.
  • Triple revocation check: peerIsRevoked runs in the handler, at the top of buildPeerEvidence, and again after claims extraction. The pre-fetch + post-verify pair is a legitimate fail-closed TOCTOU guard; three is noise.
  • Testing pyramid: the integration test starts two real sam-node processes and re-asserts schema fields already covered by the unit test. Keep it focused on the socket/enrollment/handshake flow, push field-level assertions down to the unit test, and make sure it stays within the 10s integration bound.

@rhein1

rhein1 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the blocking review and all three non-blocking points at signed head bdafa65413057886a22258347542775f076c71a5:

  • moved both public response schemas into api/sam.proto and regenerated api/sam.pb.go;
  • switched success responses to generated messages plus protojson, with Biscuit/SPKI as bytes and timestamps as Unix-second int64 fields;
  • removed ad-hoc schema strings, self-attested verdict booleans, duplicate PeerIDs, derivable fingerprints, fixed cache/revocation fields, and redundant timestamps;
  • restored plain-text sidecar errors;
  • retained only the pre-fetch and post-verification revocation checks;
  • reduced the integration test to the Unix-socket/enrollment/peer-handshake flow, with field semantics and fail-closed cases in unit tests.

Local exact-head validation: generated output byte-identical after regeneration; go test ./api, full go test ./internal/node, focused integration under -timeout=10s (1.174s), go vet ./api ./internal/node ./tests/integration, and git diff --check all pass. Hosted checks are now running; a fresh review on bdafa65 would be appreciated once they settle.

Comment thread internal/node/identity_evidence.go Outdated
Comment thread internal/node/identity_evidence.go Outdated
@kaisoz

kaisoz commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the PR! Overall this LGTM after @aojea's suggestions. A few minor, cheap things to consider:

  1. Metrics classificationclassifyRoute in internal/node/sidecar_metrics.go buckets the two new endpoints as "egress" because they match the /sam/ prefix. One extra case would keep the dashboards honest.

  2. Trailing slash falls through to the egress proxyGET /sam/identity/ misses the exact-match route and lands in the /sam/ egress handler, whose Director doesn't rewrite paths with fewer than 5 segments, so the caller gets an opaque proxy error (and a different auth mode, since egress uses allowAuthorizationFallback=false). A 404 for that case would be kinder.

  3. Error-body style (for consistency) — the errors are now plain text 👍, but the bodies are machine codes (sam_peer_evidence_fetch_failed) while the rest of the sidecar uses human-readable sentences ("Method not allowed"). Matching the existing style would make the two endpoints indistinguishable from the rest of the surface.

  4. Handler-level error tests — I'd like to see small httptest cases for the peer endpoint's error contract: bad peer ID → 400, pre-fetch revoked → 403, non-GET → 405, wrong path shape → 404. The transport gate and the fail-closed buildPeerEvidence logic are well covered; this would pin down the HTTP layer too.

This is a great contribution — nice work!

@aojea

aojea commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

@rhein1 seems we are almost there, just the last comments and please squash all commits and we get this in, I want to cut beta soon and have this as part of it

rhein1 added a commit to rhein1/sam that referenced this pull request Aug 27, 2026
Split trusted-key snapshot failures from the empty-key case so error formatting never wraps nil with %w. Preserve the existing queryExpiration error propagation cleanup.

Addresses the two unresolved review threads on google#298.
@rhein1
rhein1 force-pushed the codex/sam-transport-identity-20260823 branch from 9e11c27 to a9ef783 Compare August 27, 2026 14:34
rhein1 added a commit to rhein1/sam that referenced this pull request Aug 27, 2026
Classify identity evidence separately from egress, keep trailing-slash identity requests on the protected local surface, use human-readable sidecar errors, and pin the peer handler's HTTP error contract with focused tests.

Addresses all four follow-ups in google#298 issue comment 5431144547.
rhein1 added a commit to rhein1/sam that referenced this pull request Aug 27, 2026
Add a canonical transport-identity statement, issue and verify signed local evidence, expose authenticated identity and peer evidence endpoints on the protected sidecar surface, integrate operator verification and interoperability fixtures, and cover the flow with focused tests.

Review follow-ups:
- classify identity-evidence requests separately from egress
- keep /sam/identity/ on the protected local surface with a clear 404
- use human-readable plain-text sidecar errors
- pin peer endpoint 400, 403, 404, and 405 contracts, headers, and bodies

Squashes the six commits in google#298.
@rhein1
rhein1 force-pushed the codex/sam-transport-identity-20260823 branch from 5ceb056 to 50db599 Compare August 27, 2026 16:00
@rhein1

rhein1 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the review and for including me in the beta. I’m genuinely excited to exercise SAM in the real Agoragentic operator flow and provide focused interoperability and safety feedback. I'm currently writing a research paper on agentic discovery and safety based on the production issues and flaws I have found. I am very excited for SAM as the work we are doing complements each other in many regards. I really appreciate you guys being receptive to my input and I hope we can do more together in the near future. I love Agent 2 Agent and the infrastructure that we have for it.

rhein1 added a commit to rhein1/sam that referenced this pull request Aug 27, 2026
Expose authenticated, read-only local and peer identity-evidence endpoints using public protobuf schemas and SAM's existing libp2p TLS, Biscuit verification, key rotation, and revocation machinery.

Review follow-ups:
- classify identity-evidence requests separately from egress
- keep /sam/identity/ on the protected local surface with a clear 404
- use human-readable plain-text sidecar errors
- pin peer endpoint 400, 403, 404, and 405 contracts, headers, and bodies

Rebased onto google/sam main 1fc706f and preserves GitHub's clean virtual-merge tree for google#298.
rhein1 added a commit to rhein1/sam that referenced this pull request Aug 27, 2026
Expose authenticated, read-only local and peer identity-evidence endpoints using public protobuf schemas and SAM's existing libp2p TLS, Biscuit verification, key rotation, and revocation machinery.

Review follow-ups:
- classify identity-evidence requests separately from egress
- keep /sam/identity/ on the protected local surface with a clear 404
- use human-readable plain-text sidecar errors
- pin peer endpoint 400, 403, 404, and 405 contracts, headers, and bodies

Rebased onto current google/sam main and preserves the clean virtual-merge result for google#298.
@rhein1
rhein1 force-pushed the codex/sam-transport-identity-20260823 branch from 50db599 to bcc1bd7 Compare August 27, 2026 20:14
Expose authenticated, read-only local and peer identity-evidence endpoints using public protobuf schemas and SAM's existing libp2p TLS, Biscuit verification, key rotation, and revocation machinery.

Review follow-ups:
- classify identity-evidence requests separately from egress
- keep /sam/identity/ on the protected local surface with a clear 404
- use human-readable plain-text sidecar errors
- pin peer endpoint 400, 403, 404, and 405 contracts, headers, and bodies

Rebased onto current google/sam main and preserves the clean virtual-merge result for google#298.
@rhein1
rhein1 force-pushed the codex/sam-transport-identity-20260823 branch from bcc1bd7 to 170abad Compare August 27, 2026 21:04
@rhein1

rhein1 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Final exact-head audit update for signed, single-commit head 170abada32c8c3d396034bd073dba455074ab2bf:

  • closed a local-identity revocation gap: GET /sam/identity now rejects a revoked local PeerID before evidence construction and revalidates local PeerID binding, revocation, and trusted-key membership after Biscuit verification;
  • added unit and HTTP regressions for that fail-closed behavior;
  • all four review threads are resolved and the PR is mergeable.

Hosted evidence is green for verify, lint, govulncheck, build-ffi, Bats e2e, Console UI, Kind mesh e2e, Android e2e, Helm, workflow security, and CLA. The sole red job is the repository-wide test job: unchanged internal/controlplane test TestBiscuitExpiryIsCappedByItsVoucher/refresh_clamps_to_the_end_of_the_OIDC_session hit datalog: world runtime limit: timeout. This PR does not modify internal/controlplane. The fork owner cannot rerun that job because GitHub requires repository-admin rights, so a maintainer rerun is needed.

No production SAM activation or provider call is implied by this source/CI state.

@aojea

aojea commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

it failed twice in

biscuit_ttl_test.go:210: query: datalog: world runtime limit: timeout

=== RUN TestBiscuitExpiryIsCappedByItsVoucher/refresh_clamps_to_the_end_of_the_OIDC_session
=== RUN TestBiscuitExpiryIsCappedByItsVoucher/refresh_uses_the_configured_TTL_when_the_session_never_expires
--- FAIL: TestBiscuitExpiryIsCappedByItsVoucher (0.82s)
--- PASS: TestBiscuitExpiryIsCappedByItsVoucher/register_clamps_to_the_OIDC_token_when_it_expires_first (0.05s)
--- FAIL: TestBiscuitExpiryIsCappedByItsVoucher/register_uses_the_configured_TTL_when_it_expires_first (0.03s)
--- PASS: TestBiscuitExpiryIsCappedByItsVoucher/refresh_clamps_to_the_end_of_the_OIDC_session (0.08s)

does not seem related

@rhein1

rhein1 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@aojea @kaisoz Looks like this is ready to go! Please tag me for updates on the Beta.

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.

3 participants