From df355c53a945d799d7c8e03536084b9c1242a708 Mon Sep 17 00:00:00 2001 From: "glm-5.2" Date: Sun, 28 Jun 2026 21:08:41 +0000 Subject: [PATCH] tasks: decompose ADR-029/030/031/032/034/035 source sync into 17 tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decompose the source-to-spec sync for the core and call crates into atomic, dependency-ordered tasks for implementation agents: Core (7 tasks + review): - peer-entry-model: PeerEntry struct, AuthPolicy.peers (ADR-030 keystone) - credential-store-trait: CredentialStore/InMemoryCredentialStore/StoreError (ADR-031/035) - identity-store-trait: IdentityStore async write trait (ADR-035) - config-identity-provider-peerentry: ConfigIdentityProvider PeerEntry resolution (ADR-030) - fingerprint-normalization: ed25519:hex for raw keys across quinn/iroh (ADR-030 §6) - three-remote-roles-docs: document ADR-034 roles and verifier selection - review-core-sync: phase gate before call consumes new identity semantics Call (9 tasks + review): - retire-remote-safe: remove ADR-028 machinery, AccessControl is the gate (ADR-029 §3) - operation-context-forwarded-for: forwarded_for field, wire-ingress only (ADR-032) - peer-composite-env: PeerCompositeEnv, PeerId=Identity.id, remove UUID (ADR-029/030) - operation-env-invoke-peer: invoke_peer/peer_contains/PeerRef (ADR-029 §2) - services-list-accesscontrol-filtered: AccessControl filter, list-peers opt-in (ADR-029 §6) - call-client-verifier-selection: TLS client-auth, verifier by PeerEntry (OQ-29, ADR-034) - from-call-forwarded-for: populate forwarded_for, peer-keyed registration (ADR-029 §5, ADR-032) - dispatch-peer-identity: AccessControl::check(peer_identity), PeerId from resolution (ADR-029 §3, ADR-030 §5) - review-call-sync: phase gate for the call sync Validated: 58 tasks, no cycles, logical topo order, two review checkpoints. --- tasks/call/call-client-verifier-selection.md | 177 +++++++++++++++++ tasks/call/dispatch-peer-identity.md | 129 ++++++++++++ tasks/call/from-call-forwarded-for.md | 114 +++++++++++ tasks/call/operation-context-forwarded-for.md | 148 ++++++++++++++ tasks/call/operation-env-invoke-peer.md | 156 +++++++++++++++ tasks/call/peer-composite-env.md | 187 ++++++++++++++++++ tasks/call/retire-remote-safe.md | 125 ++++++++++++ tasks/call/review-call-sync.md | 155 +++++++++++++++ .../services-list-accesscontrol-filtered.md | 122 ++++++++++++ .../config-identity-provider-peerentry.md | 114 +++++++++++ tasks/core/credential-store-trait.md | 144 ++++++++++++++ tasks/core/fingerprint-normalization.md | 127 ++++++++++++ tasks/core/identity-store-trait.md | 98 +++++++++ tasks/core/peer-entry-model.md | 175 ++++++++++++++++ tasks/core/review-core-sync.md | 123 ++++++++++++ tasks/core/three-remote-roles-docs.md | 85 ++++++++ 16 files changed, 2179 insertions(+) create mode 100644 tasks/call/call-client-verifier-selection.md create mode 100644 tasks/call/dispatch-peer-identity.md create mode 100644 tasks/call/from-call-forwarded-for.md create mode 100644 tasks/call/operation-context-forwarded-for.md create mode 100644 tasks/call/operation-env-invoke-peer.md create mode 100644 tasks/call/peer-composite-env.md create mode 100644 tasks/call/retire-remote-safe.md create mode 100644 tasks/call/review-call-sync.md create mode 100644 tasks/call/services-list-accesscontrol-filtered.md create mode 100644 tasks/core/config-identity-provider-peerentry.md create mode 100644 tasks/core/credential-store-trait.md create mode 100644 tasks/core/fingerprint-normalization.md create mode 100644 tasks/core/identity-store-trait.md create mode 100644 tasks/core/peer-entry-model.md create mode 100644 tasks/core/review-core-sync.md create mode 100644 tasks/core/three-remote-roles-docs.md diff --git a/tasks/call/call-client-verifier-selection.md b/tasks/call/call-client-verifier-selection.md new file mode 100644 index 0000000..51522b6 --- /dev/null +++ b/tasks/call/call-client-verifier-selection.md @@ -0,0 +1,177 @@ +--- +id: call/call-client-verifier-selection +name: Wire CallClient TLS client-auth and server cert verifier selection by PeerEntry presence (OQ-29, ADR-034) +status: pending +depends_on: [call/peer-composite-env] +scope: moderate +risk: high +impact: component +level: implementation +--- + +## Description + +Wire the `CallClient` TLS client-auth (present Ed25519 key as RFC 7250 raw +public key client cert) and the server cert verifier selection by `PeerEntry` +presence. Per OQ-29 (resolved) and ADR-034 §2-3. This is the most +security-critical call-side change — TLS wiring and verifier selection. + +### Current state + +```rust +// crates/alknet-call/src/client/call_client.rs +fn build_quinn_client_config(_credentials: &CallCredentials, alpn: &[u8]) + -> Result +{ + let config = rustls::ClientConfig::builder() + .dangerous() + .with_custom_certificate_verifier(Arc::new(AcceptAnyServerCertVerifier)) // ← accepts ANY + .with_no_client_auth(); // ← doesn't present client cert + // ... +} +``` + +`AcceptAnyServerCertVerifier` is a security hole for X.509 (accepts any cert +without CA verification). `with_no_client_auth()` doesn't present the client's +Ed25519 key, so the server has no client cert to extract a fingerprint from — +the ADR-030 `PeerEntry` fingerprint → `peer_id` resolution path is not +activated for quinn connections. + +### Target state (OQ-29 + ADR-034) + +#### 1. TLS client-auth: present Ed25519 key as raw public key client cert + +Replace `with_no_client_auth()` with presenting the client's Ed25519 key as an +RFC 7250 raw public key client cert. This is the client-side equivalent of the +server's `RawKeyCertResolver`. The `CallCredentials.tls_identity` carries the +`TlsIdentity::RawKey(Ed25519SecretKey)` (or X.509 cert pair). + +```rust +fn build_quinn_client_config(credentials: &CallCredentials, alpn: &[u8]) + -> Result +{ + // 1. Client cert: present Ed25519 raw key (if configured) + let client_auth = build_client_auth(&credentials.tls_identity)?; + + // 2. Server cert verifier: by PeerEntry presence (ADR-034 §3) + let verifier = select_server_verifier(&credentials.remote_identity)?; + + let config = rustls::ClientConfig::builder() + .dangerous() + .with_custom_certificate_verifier(verifier) + .with_client_auth(client_auth); // ← present the key, not no_client_auth + // ... +} +``` + +#### 2. Server cert verifier selection by PeerEntry presence (ADR-034 §3) + +| Local has `PeerEntry` for remote? | Remote cert type | Client verifier | +|----------------------------------|------------------|-----------------| +| No (public X.509 endpoint) | X.509 | `WebPkiServerVerifier` (CA verification) | +| No | Ed25519 raw key | fails closed (no CA to fall back to) | +| Yes (hub, Ed25519 path) | Ed25519 raw key | fingerprint match (`ed25519:`) | +| Yes (hub, X.509 path) | X.509 | fingerprint match (`SHA256:`) | + +`CallCredentials.remote_identity: Option` is load-bearing: +- `Some(fingerprint)` → known peer → fingerprint pin (the fingerprint IS the + trust anchor). +- `None` → no `PeerEntry` for the remote → CA verification for X.509, fail + closed for Ed25519 raw key. `None` is the public-X.509-endpoint state, not a + missing field. An implementer must not default `remote_identity` to a + placeholder, and must not treat `None` as "skip verification." + +```rust +fn select_server_verifier(remote_identity: &Option) + -> Result, String> +{ + match remote_identity { + Some(ri) => { + // Known peer → fingerprint pin + Ok(Arc::new(FingerprintPinVerifier::new(ri.fingerprint.clone()))) + } + None => { + // Unknown remote → CA verification (WebPkiServerVerifier) + // For Ed25519 raw-key remotes, this fails closed (no CA). + // This is the public-X.509-endpoint path (ADR-034 §2). + let roots = rustls::crypto::aws_lc_rs::default_provider().root_certificates; + Ok(Arc::new(rustls::client::WebPkiServerVerifier::builder(roots.into()).build()?)) + } + } +} +``` + +#### 3. FingerprintPinVerifier + +A new `ServerCertVerifier` that pins a specific fingerprint: +- For `ed25519:` remotes: extract the raw Ed25519 pub key from the + presented cert and match against the pinned fingerprint. +- For `SHA256:` remotes: hash the cert DER and match against the pinned + fingerprint. +- No match → verification failure (connection rejected). + +#### 4. CallCredentials + +`CallCredentials` (already defined) carries the three credential dimensions: + +```rust +pub struct CallCredentials { + pub tls_identity: Option, // RFC 7250 raw key or X.509 + pub auth_token: Option, // call-protocol-level token + pub remote_identity: Option, // expected fingerprint (None = CA path) +} + +pub struct RemoteIdentity { pub fingerprint: String } +``` + +`remote_identity: None` is load-bearing — the public-X.509-endpoint state +(ADR-034 §2). The implementation must not default it to a placeholder. + +### What this task does NOT do + +- Does NOT change the server-side endpoint (`AcceptAnyCertVerifier` in + alknet-core is unchanged — it's "request-but-don't-require" for fingerprint + extraction). +- Does NOT build `PeerCompositeEnv` (that's `call/peer-composite-env`, a + dependency) — but a connection with no resolved identity (no `PeerEntry`) gets + no `PeerId` and is not added to `PeerCompositeEnv` (that's handled in + `call/peer-composite-env` / `call/dispatch-peer-identity`). + +## Acceptance Criteria + +- [ ] `build_quinn_client_config` presents Ed25519 key as RFC 7250 raw public key client cert (replaces `with_no_client_auth()`) +- [ ] `select_server_verifier` selects verifier by `remote_identity` presence +- [ ] `Some(fingerprint)` → `FingerprintPinVerifier` (fingerprint match) +- [ ] `None` + X.509 → `WebPkiServerVerifier` (CA verification) +- [ ] `None` + Ed25519 raw key → fails closed (no CA to fall back to) +- [ ] `FingerprintPinVerifier` matches `ed25519:` (raw key extraction) and `SHA256:` (DER hash) +- [ ] `AcceptAnyServerCertVerifier` removed (security hole for X.509) +- [ ] `CallCredentials.remote_identity: None` is load-bearing (not defaulted to placeholder) +- [ ] No-env-vars invariant preserved (credentials from Capabilities, not env vars) +- [ ] Unit test: `FingerprintPinVerifier` matches correct fingerprint +- [ ] Unit test: `FingerprintPinVerifier` rejects wrong fingerprint +- [ ] Unit test: `select_server_verifier` returns CA verifier for `None` +- [ ] Unit test: client auth presents Ed25519 key (config built without error) +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/client-and-adapters.md — CallCredentials, verifier selection, TLS client-auth +- docs/architecture/crates/core/auth.md — Client-side verifier selection table +- docs/architecture/decisions/034-outgoing-only-x509-and-three-peer-roles.md — ADR-034 §2-3 +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md — ADR-030 §6 (fingerprint normalization) + +## Notes + +> Most security-critical call-side change. `AcceptAnyServerCertVerifier` is a +> security hole for X.509 — replaced by verifier selection by `PeerEntry` +> presence. `None` + X.509 = CA verification (public X.509 endpoint); `None` + +> Ed25519 = fail closed (raw-key remotes are always known peers). `Some` = +> fingerprint pin (known peer). The client presents its Ed25519 key as a raw +> public key client cert so the server can extract the fingerprint — this +> activates the PeerEntry fingerprint → peer_id resolution path on quinn. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/dispatch-peer-identity.md b/tasks/call/dispatch-peer-identity.md new file mode 100644 index 0000000..b06664c --- /dev/null +++ b/tasks/call/dispatch-peer-identity.md @@ -0,0 +1,129 @@ +--- +id: call/dispatch-peer-identity +name: Wire dispatch_requested to resolve peer Identity and run AccessControl::check (ADR-029 §3, ADR-030 §5) +status: pending +depends_on: [call/peer-composite-env, call/retire-remote-safe] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Wire `dispatch_requested` to resolve the peer's `Identity` and run +`AccessControl::check(peer_identity)` as the authorization mechanism, and +ensure the `PeerId` for a connection comes from `connection.identity().id` +(IdentityProvider resolution). Per ADR-029 §3 (AccessControl-based peer +authorization) and ADR-030 §5 (PeerId from IdentityProvider resolution). + +### Current state + +After `call/retire-remote-safe`, the `RemoteFilter` gate is removed. +`dispatch_requested` resolves identity (connection-level + auth_token override) +and `OperationRegistry::invoke` runs `AccessControl::check`. The remaining gap +is ensuring the `PeerId` for the connection comes from `IdentityProvider` +resolution (not a UUID), and that connections with no resolved identity get no +`PeerId` (not added to `PeerCompositeEnv`). + +### Target state + +#### 1. PeerId from IdentityProvider resolution (ADR-030 §5) + +The `PeerId` for a connection is `connection.identity().id` — the resolved +`Identity.id` from `IdentityProvider` (= `PeerEntry.peer_id`, stable). The UUID +workaround is removed (done in `call/peer-composite-env`). This task verifies +the dispatch path reads `connection.identity().id` for the peer-keyed overlay +and that a connection with no resolved identity is handled correctly. + +#### 2. AccessControl::check(peer_identity) is the authorization gate + +`dispatch_requested` resolves the peer's `Identity` (from the connection's TLS +fingerprint or the `auth_token` payload, via the existing `IdentityProvider`) +and `OperationRegistry::invoke` runs `AccessControl::check(peer_identity)` +against the op's `AccessControl`: + +- If the op's `AccessControl` is satisfied → dispatch (capabilities populated + from the bundle). +- If not → `FORBIDDEN` before the handler runs (capabilities never populated — + the security property). +- If the op is `Visibility::Internal` → `NOT_FOUND` before ACL (existing + behavior). + +This is the existing `OperationRegistry::invoke` path — the `RemoteFilter` gate +(removed in `call/retire-remote-safe`) was a *parallel* gate. This task +verifies the `AccessControl::check` path is the sole authorization mechanism +and that no `remote_safe`/`trusted_peer` remnants remain. + +#### 3. Connections with no resolved identity + +A connection with no resolved identity (no client cert, unrecognized +fingerprint) has no `PeerId` and is not added to `PeerCompositeEnv` +(ADR-030 §5). The handler either rejects the connection or falls back to a +connection-without-peer-identity path. The dispatch path must handle this case: +- `connection.identity()` returns `None` → no `PeerId` +- The connection's ops (if any discovered via `from_call`) are invoked through + the `CallConnection` handle directly, not via `PeerRef::Specific` + +### dispatch_requested flow (post-sync) + +```rust +async fn dispatch_requested(&self, connection: &Arc, + request_id: String, payload: Value) -> ResponseEnvelope { + let operation_id = payload.get("operationId").and_then(|v| v.as_str()).unwrap_or(""); + let operation_name = Self::strip_leading_slash(operation_id).to_string(); + + // No RemoteFilter gate (removed). AccessControl::check in invoke is the gate. + + let connection_identity = connection.connection().identity().cloned(); + let identity = self.resolve_identity(connection_identity, &payload); + let forwarded_for = payload.get("forwarded_for") + .and_then(|v| serde_json::from_value::(v.clone()).ok()); + + let input = payload.get("input").cloned().unwrap_or(Value::Null); + let context = self.build_root_context( + request_id.clone(), &operation_name, identity, forwarded_for, connection); + + // OperationRegistry::invoke runs AccessControl::check(identity) — + // the sole authorization mechanism (ADR-029 §3). + self.registry.invoke(&operation_name, input, context).await +} +``` + +## Acceptance Criteria + +- [ ] `dispatch_requested` resolves peer `Identity` (connection-level + auth_token override) +- [ ] `OperationRegistry::invoke` runs `AccessControl::check(peer_identity)` as the sole authorization gate +- [ ] No `RemoteFilter`/`remote_safe`/`trusted_peer` remnants in dispatch +- [ ] `PeerId` for connection comes from `connection.identity().id` (not UUID) +- [ ] Connection with no resolved identity → no `PeerId`, not added to `PeerCompositeEnv` +- [ ] Op with `AccessControl::default()` dispatches to any peer +- [ ] Op with `required_scopes` → `FORBIDDEN` for unauthorized peers (capabilities never populated) +- [ ] Op with `Visibility::Internal` → `NOT_FOUND` before ACL +- [ ] `forwarded_for` extracted from payload and passed to `build_root_context` +- [ ] Unit test: authorized peer → dispatch (capabilities populated) +- [ ] Unit test: unauthorized peer → FORBIDDEN (capabilities never populated) +- [ ] Unit test: Internal op → NOT_FOUND from wire +- [ ] Unit test: connection with no identity → no PeerId +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/call-protocol.md — dispatch_requested, AuthContext and Identity Resolution +- docs/architecture/crates/call/client-and-adapters.md — peer authorization via AccessControl +- docs/architecture/decisions/029-peer-graph-routing-model.md — ADR-029 §3 (AccessControl-based peer auth) +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md — ADR-030 §5 (PeerId from IdentityProvider) + +## Notes + +> The `RemoteFilter` gate (removed in `call/retire-remote-safe`) was a parallel +> authorization system. This task verifies `AccessControl::check` in +> `OperationRegistry::invoke` is the sole gate and that the `PeerId` comes from +> `IdentityProvider` resolution (not UUID). Connections with no resolved +> identity get no `PeerId` and are not in the peer-keyed overlay — their ops +> are invoked through the `CallConnection` handle directly. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/from-call-forwarded-for.md b/tasks/call/from-call-forwarded-for.md new file mode 100644 index 0000000..832b14b --- /dev/null +++ b/tasks/call/from-call-forwarded-for.md @@ -0,0 +1,114 @@ +--- +id: call/from-call-forwarded-for +name: Wire from_call forwarding handler to populate forwarded_for and use peer-keyed registration (ADR-029 §5, ADR-032) +status: pending +depends_on: [call/operation-context-forwarded-for, call/peer-composite-env] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Update the `from_call` forwarding handler to populate `forwarded_for` on the +`call.requested` payload it constructs, and update `from_call` registration to +use the peer-keyed overlay model. Per ADR-029 §5 (peer-keyed registration, +collision rule change) and ADR-032 §3 (from_call populates forwarded_for). + +### forwarded_for population (ADR-032 §3) + +The hub's `from_call` forwarding handler constructs the `call.requested` payload +to send to the spoke. It populates `forwarded_for` with the end user's identity +— read from the hub's `OperationContext.identity` (the caller the hub +authenticated) when the hub forwards the call. + +```rust +// In the from_call forwarding handler: +let mut payload = serde_json::json!({ + "operationId": operation_id, + "input": input, +}); +// Populate forwarded_for from the hub's context.identity (ADR-032) +if let Some(originator) = &context.identity { + payload["forwarded_for"] = serde_json::to_value(originator).ok().unwrap_or(Value::Null); +} +// The hub authenticates as itself (its own auth_token) +if let Some(token) = &credentials.auth_token { + payload["auth_token"] = serde_json::Value::String(token); +} +``` + +The hub may set `forwarded_for: None` (omit the field) if it doesn't want to +disclose the originator. The spoke receives it as metadata on its +`OperationContext` — available for logging, auditing, per-user rate limiting, +but never used by `AccessControl::check` (the spoke authorizes the hub, its +direct caller). + +### Peer-keyed registration (ADR-029 §5) + +`from_call` registers into the specific peer's sub-overlay (via +`CallConnection::register_imported`), not a flat overlay. Cross-peer collision +dissolves: same name on different peers is fine (separate sub-overlays, no +collision, no prefix needed). Same-peer collision stays an error (a peer +shouldn't expose two ops with the same name). + +`FromCallConfig::namespace_prefix` becomes optional local-naming sugar for +when the importing node wants to expose a peer's ops under a different name +*locally* — a local-naming concern, not a disambiguation concern. It defaults +to `None`. + +### Collision rule change + +- **Same-peer collision** = error (a peer shouldn't expose two ops with the + same name). `AdapterError::SamePeerCollision`. +- **Cross-peer collision** dissolves (same name on different peers is fine — + separate sub-overlays, ADR-029 §5). + +The existing `from_call` namespace-collision check (which was flat-namespace) +changes to same-peer-only. The `AdapterError::Conflict` variant (if it exists) +renames to `SamePeerCollision` (OQ-26). + +### What this task does NOT do + +- Does NOT build `PeerCompositeEnv` (that's `call/peer-composite-env`, a + dependency) — `from_call` registers into the connection's overlay, which + `PeerCompositeEnv` aggregates by peer. +- Does NOT add `forwarded_for` to `OperationContext` (that's + `call/operation-context-forwarded-for`, a dependency) — this task populates + the wire field. + +## Acceptance Criteria + +- [ ] `from_call` forwarding handler populates `forwarded_for` on the `call.requested` payload from `context.identity` +- [ ] `forwarded_for` omitted (None) when the hub chooses not to disclose the originator +- [ ] `from_call` registers into the connection's overlay (peer-keyed via `PeerCompositeEnv`) +- [ ] Same-peer collision = error (`AdapterError::SamePeerCollision`) +- [ ] Cross-peer collision dissolves (same name on different peers is fine) +- [ ] `FromCallConfig::namespace_prefix` defaults to `None` (optional local-naming sugar) +- [ ] `AdapterError::Conflict` renamed to `SamePeerCollision` (OQ-26) +- [ ] Unit test: `forwarded_for` populated from `context.identity` on forwarding +- [ ] Unit test: `forwarded_for` omitted when context.identity is None +- [ ] Unit test: same-peer collision returns `SamePeerCollision` error +- [ ] Unit test: cross-peer same name does not collide (separate sub-overlays) +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/client-and-adapters.md — from_call, forwarded_for, namespace collision +- docs/architecture/decisions/029-peer-graph-routing-model.md — ADR-029 §5 (peer-keyed registration, collision rule) +- docs/architecture/decisions/032-forwarded-for-identity.md — ADR-032 §3 (from_call populates forwarded_for) + +## Notes + +> The `from_call` handler is the hub's forwarding path. It populates +> `forwarded_for` from the hub's `context.identity` (the end user) so the spoke +> has the originator as metadata. The spoke authorizes the hub (its direct +> caller), not the end user — `AccessControl::check` never reads +> `forwarded_for`. Cross-peer collision dissolves under the peer-keyed model +> (separate sub-overlays); same-peer collision stays an error. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/operation-context-forwarded-for.md b/tasks/call/operation-context-forwarded-for.md new file mode 100644 index 0000000..6f5af72 --- /dev/null +++ b/tasks/call/operation-context-forwarded-for.md @@ -0,0 +1,148 @@ +--- +id: call/operation-context-forwarded-for +name: Add forwarded_for field to OperationContext and wire from call.requested (ADR-032) +status: pending +depends_on: [call/retire-remote-safe] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Add the `forwarded_for: Option` field to `OperationContext` and wire +it from the `call.requested` payload. Per ADR-032, `forwarded_for` is metadata +only — `AccessControl::check` never reads it; the ACL always authorizes the +direct caller's `identity`. This is the wire-format one-way door folded into +the `OperationContext` edit window (ADR-032 says this is the cheapest point +since `OperationContext` is under edit for the ADR-029 migration). + +### OperationContext change + +```rust +pub struct OperationContext { + // ... existing fields ... + pub forwarded_for: Option, // NEW (ADR-032) +} +``` + +- `forwarded_for`: The original caller when this call was forwarded by a + `from_call` handler (ADR-032). **Metadata only** — `AccessControl::check` + never reads it; the ACL always authorizes `identity` (the direct caller). + Handlers may read it for logging, auditing, per-user rate limiting, or + application context. The forwarder's claim, not a verified identity — a + malicious hub can lie (same property as HTTP `X-Forwarded-For`). + +### Wire format (call.requested payload) + +The `call.requested` payload already carries `operationId`, `input`, and +optional `auth_token`. Add optional `forwarded_for`: + +```json +{ + "operationId": "/docker/start", + "input": { ... }, + "auth_token": "alk_...", + "forwarded_for": { // optional (ADR-032) + "id": "alice", + "scopes": ["fs:read", "docker:start"], + "resources": {} + } +} +``` + +The payload is a `serde_json::Value` (not a typed struct), so `forwarded_for` +is read from `payload.get("forwarded_for")` and deserialized into `Identity`. + +### build_root_context wiring + +`Dispatcher::build_root_context` reads `forwarded_for` from the payload and +populates the field: + +```rust +fn build_root_context( + &self, + request_id: String, + operation_name: &str, + identity: Option, + forwarded_for: Option, // NEW parameter + connection: &CallConnection, +) -> OperationContext { + // ... + OperationContext { + // ... + forwarded_for, // from call.requested.forwarded_for + // ... + } +} +``` + +`dispatch_requested` extracts `forwarded_for` from the payload: + +```rust +let forwarded_for = payload.get("forwarded_for") + .and_then(|v| serde_json::from_value::(v.clone()).ok()); +``` + +### OperationEnv::invoke sets None for composed children + +`forwarded_for` is **wire-ingress only** (ADR-032 Assumption 3). Composed +children (calls via `OperationEnv::invoke`) do NOT inherit `forwarded_for` — +they get `None`. The field is populated from `call.requested.forwarded_for` by +the dispatch path, and the `from_call` forwarding handler sets it when +constructing the forwarded payload (that's `call/from-call-forwarded-for`). + +In `LocalOperationEnv::invoke_with_policy` (and `PeerCompositeEnv` when built): + +```rust +let context = OperationContext { + // ... + forwarded_for: None, // composed children do not inherit (ADR-032) + // ... +}; +``` + +### AccessControl::check never reads forwarded_for + +The security property is structural: `AccessControl::check` takes +`Option<&Identity>` (the direct caller's `identity`). The `forwarded_for` field +is `Option` on `OperationContext`, but the check signature doesn't +accept it. Verify this invariant holds — no code path passes `forwarded_for` +to `AccessControl::check`. + +## Acceptance Criteria + +- [ ] `OperationContext.forwarded_for: Option` field added +- [ ] `build_root_context` accepts `forwarded_for` parameter and populates the field +- [ ] `dispatch_requested` extracts `forwarded_for` from `payload.get("forwarded_for")` +- [ ] `forwarded_for` deserialized from JSON `{ id, scopes, resources }` into `Identity` +- [ ] `OperationEnv::invoke_with_policy` sets `forwarded_for: None` for composed children +- [ ] `AccessControl::check` never reads `forwarded_for` (verify no code path passes it) +- [ ] Missing `forwarded_for` in payload → `None` (no error) +- [ ] Unit test: `forwarded_for` populated from payload +- [ ] Unit test: missing `forwarded_for` → None +- [ ] Unit test: composed children get `forwarded_for: None` +- [ ] Unit test: `AccessControl::check` still uses `identity` (not `forwarded_for`) +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/operation-registry.md — OperationContext.forwarded_for +- docs/architecture/crates/call/call-protocol.md — call.requested payload, build_root_context +- docs/architecture/decisions/032-forwarded-for-identity.md — ADR-032 + +## Notes + +> `forwarded_for` is a wire-format one-way door (ADR-032) — folded into the +> OperationContext edit window because ADR-029 is already rewriting the +> composition env. The field is metadata only: `AccessControl::check` never +> reads it; the ACL always authorizes the direct caller's `identity`. The +> `from_call` handler populates it when forwarding (that's +> `call/from-call-forwarded-for`). Composed children get `None` (wire-ingress +> only, not composition-ingress). + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/operation-env-invoke-peer.md b/tasks/call/operation-env-invoke-peer.md new file mode 100644 index 0000000..a68c4a4 --- /dev/null +++ b/tasks/call/operation-env-invoke-peer.md @@ -0,0 +1,156 @@ +--- +id: call/operation-env-invoke-peer +name: Add invoke_peer/peer_contains/PeerRef to OperationEnv trait for peer-keyed routing (ADR-029 §2) +status: pending +depends_on: [call/peer-composite-env] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Add the `invoke_peer` and `peer_contains` methods to the `OperationEnv` trait, +with the `PeerRef` selector enum. Per ADR-029 §2. `PeerCompositeEnv` (built in +`call/peer-composite-env`) overrides these with real peer-keyed routing; the +default-impl preserves back-compat for single-layer envs +(`LocalOperationEnv`, connection overlays) that don't route by peer. + +### PeerRef enum + +```rust +pub enum PeerRef { + Specific(PeerId), // route to this peer; NOT_FOUND if it doesn't serve the op + Any, // first peer (insertion order) that serves it +} +``` + +`PeerRef::Specific(PeerId)` routes to the named peer's overlay only — no +fallthrough (explicit routing must be honored or fail loudly, ADR-029 §2). +`PeerRef::Any` reuses `invoke_with_policy` (the insertion-order fan-out built +in `call/peer-composite-env`). + +### OperationEnv trait additions + +```rust +#[async_trait] +pub trait OperationEnv: Send + Sync { + // ... existing invoke, invoke_with_policy, contains ... + + /// Peer-routing composition (ADR-029 §2). Routes to a specific peer + /// (`PeerRef::Specific`) or to the first peer that serves the op + /// (`PeerRef::Any`). The default impl ignores the peer selector and + /// delegates to `invoke_with_policy`, preserving back-compat for + /// single-layer envs that don't route by peer. `PeerCompositeEnv` + /// overrides with real peer-keyed routing. + async fn invoke_peer( + &self, + peer: &PeerRef, + namespace: &str, + operation: &str, + input: Value, + parent: &OperationContext, + policy: AbortPolicy, + ) -> ResponseEnvelope { + let _ = peer; // unused — single-layer envs don't route by peer + self.invoke_with_policy(namespace, operation, input, parent, policy).await + } + + /// Does this env contain the named op *on the named peer*? Used by + /// `PeerCompositeEnv` to probe a specific peer's sub-overlay before + /// dispatching via `invoke_peer` with `PeerRef::Specific`. Default impl + /// delegates to `contains` (single-layer envs ignore the peer dimension). + fn peer_contains(&self, _peer: &PeerId, name: &str) -> bool { + self.contains(name) + } +} +``` + +### PeerCompositeEnv overrides + +```rust +#[async_trait] +impl OperationEnv for PeerCompositeEnv { + // ... invoke_with_policy, contains from call/peer-composite-env ... + + async fn invoke_peer( + &self, + peer: &PeerRef, + namespace: &str, + operation: &str, + input: Value, + parent: &OperationContext, + policy: AbortPolicy, + ) -> ResponseEnvelope { + let name = format!("{namespace}/{operation}"); + if !parent.scoped_env.allows(&name) { + return ResponseEnvelope::not_found(parent.request_id.clone(), &name); + } + match peer { + PeerRef::Specific(peer_id) => { + // Route to this peer's sub-overlay only. No fallthrough — + // explicit routing must be honored or fail loudly (ADR-029 §2). + match self.connections.get(peer_id) { + Some(conn_env) if conn_env.contains(&name) => { + conn_env.invoke_with_policy(namespace, operation, input, parent, policy).await + } + _ => ResponseEnvelope::not_found(parent.request_id.clone(), &name), + } + } + PeerRef::Any => { + // Same as invoke_with_policy: session → peers in order → base. + self.invoke_with_policy(namespace, operation, input, parent, policy).await + } + } + } + + fn peer_contains(&self, peer: &PeerId, name: &str) -> bool { + self.connections.get(peer).map_or(false, |c| c.contains(name)) + } +} +``` + +### Back-compat + +Existing impls (`LocalOperationEnv`, connection overlay envs) use the default +`invoke_peer` (delegates to `invoke_with_policy`, ignores peer selector) and +default `peer_contains` (delegates to `contains`). No changes needed to those +impls — the trait surface grows, the behavior is preserved. + +## Acceptance Criteria + +- [ ] `PeerRef` enum with `Specific(PeerId)` and `Any` variants +- [ ] `OperationEnv::invoke_peer` method with default-impl (delegates to `invoke_with_policy`) +- [ ] `OperationEnv::peer_contains` method with default-impl (delegates to `contains`) +- [ ] `PeerCompositeEnv` overrides `invoke_peer` with real peer-keyed routing +- [ ] `PeerRef::Specific` routes to named peer only (no fallthrough → NOT_FOUND if peer doesn't serve op) +- [ ] `PeerRef::Any` reuses `invoke_with_policy` (insertion-order fan-out) +- [ ] `PeerCompositeEnv` overrides `peer_contains` (checks specific peer's sub-overlay) +- [ ] Reachability check (`scoped_env.allows`) gates before peer routing +- [ ] `LocalOperationEnv` and overlay envs use default-impls (no changes) +- [ ] Unit test: `PeerRef::Specific` routes to the named peer +- [ ] Unit test: `PeerRef::Specific` → NOT_FOUND when peer doesn't serve the op (no fallthrough) +- [ ] Unit test: `PeerRef::Any` routes to first peer (insertion order) that serves it +- [ ] Unit test: `peer_contains` checks specific peer's overlay +- [ ] Unit test: default-impl `invoke_peer` delegates to `invoke_with_policy` (back-compat) +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/operation-registry.md — OperationEnv, invoke_peer, PeerRef +- docs/architecture/decisions/029-peer-graph-routing-model.md — ADR-029 §2 (PeerRef routing) + +## Notes + +> The default-impl preserves back-compat — existing single-layer envs +> (`LocalOperationEnv`, connection overlays) work unchanged. `PeerCompositeEnv` +> overrides with real peer-keyed routing. `PeerRef::Specific` has no +> fallthrough (explicit routing must be honored or fail loudly). `PeerRef::Any` +> reuses the `invoke_with_policy` fan-out. The reachability check +> (`scoped_env.allows`) gates before peer routing, same as before. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/peer-composite-env.md b/tasks/call/peer-composite-env.md new file mode 100644 index 0000000..a1bdc1d --- /dev/null +++ b/tasks/call/peer-composite-env.md @@ -0,0 +1,187 @@ +--- +id: call/peer-composite-env +name: Replace CompositeOperationEnv with PeerCompositeEnv (peer-keyed overlays) and PeerId from Identity.id (ADR-029/030) +status: pending +depends_on: [call/retire-remote-safe] +scope: broad +risk: high +impact: phase +level: implementation +--- + +## Description + +Replace `CompositeOperationEnv` (singular `connection: Option>`) with `PeerCompositeEnv` (peer-keyed `HashMap`), and change `PeerId` from a connection-assigned UUID to +`Identity.id` from `IdentityProvider` resolution (= `PeerEntry.peer_id`, +stable across key rotation). Per ADR-029 §1 and ADR-030 §4-5. + +This is the highest-risk call task — a structural rewrite of the composition +env that aggregates multiple connections. The singular-connection case (one +peer) is the degenerate case with a single-entry map. + +### PeerCompositeEnv struct + +```rust +pub struct PeerCompositeEnv { + pub base: Arc, // Layer 0 curated + pub session: Option>, // Layer 1 + pub connections: HashMap>, // Layer 2, peer-keyed + connection_order: Vec, // insertion order for PeerRef::Any first-match +} + +pub type PeerId = String; // = Identity.id from IdentityProvider resolution + // = PeerEntry.peer_id (stable, not crypto material — ADR-030) +``` + +### PeerCompositeEnv methods + +```rust +impl PeerCompositeEnv { + pub fn new(base: Arc) -> Self; + + pub fn with_session(mut self, session: Arc) -> Self; + + /// Attach a peer's connection overlay. The `peer_id` comes from + /// `connection.identity().id` (IdentityProvider resolution). A connection + /// with no resolved identity has no PeerId and is NOT attached (ADR-030 §5). + pub fn attach_peer(&mut self, peer_id: PeerId, overlay: Arc); + + /// Detach a peer's overlay (on disconnect). The peer's sub-overlay drops; + /// in-flight PeerRef::Specific(that_peer) gets NOT_FOUND. + pub fn detach_peer(&mut self, peer_id: &PeerId); +} +``` + +### PeerCompositeEnv::invoke_with_policy + +```rust +async fn invoke_with_policy(&self, namespace: &str, operation: &str, input: Value, + parent: &OperationContext, policy: AbortPolicy) -> ResponseEnvelope { + let name = format!("{namespace}/{operation}"); + if !parent.scoped_env.allows(&name) { + return ResponseEnvelope::not_found(parent.request_id.clone(), &name); + } + // PeerRef::Any routing (ADR-029 §2): session → peers in insertion + // order → curated base. First overlay that contains the op wins. + if let Some(session) = &self.session { + if session.contains(&name) { + return session.invoke_with_policy(namespace, operation, input, parent, policy).await; + } + } + for peer_id in &self.connection_order { + if let Some(conn_env) = self.connections.get(peer_id) { + if conn_env.contains(&name) { + return conn_env.invoke_with_policy(namespace, operation, input, parent, policy).await; + } + } + } + self.base.invoke_with_policy(namespace, operation, input, parent, policy).await +} + +fn contains(&self, name: &str) -> bool { + self.session.as_ref().map_or(false, |s| s.contains(name)) + || self.connections.values().any(|c| c.contains(name)) + || self.base.contains(name) +} +``` + +The `invoke_peer` / `peer_contains` methods (PeerRef routing) are +`call/operation-env-invoke-peer` — this task builds the struct and the +`invoke_with_policy` / `contains` methods; the peer-routing methods are the +next task. + +### compose_root_env rewrite + +`Dispatcher::compose_root_env` builds a `PeerCompositeEnv` per incoming call: + +```rust +fn compose_root_env(&self, connection: &CallConnection, context: &OperationContext) + -> Arc +{ + let base = Arc::new(LocalOperationEnv::new(Arc::clone(&self.registry))); + let session = self.session_source.as_ref() + .and_then(|s| s.overlay_for(context)); + + let mut env = PeerCompositeEnv::new(base); + if let Some(session) = session { + env = env.with_session(session); + } + // Attach this connection's overlay, keyed by the peer's PeerId. + // PeerId = connection.identity().id (IdentityProvider resolution). + // A connection with no resolved identity is NOT attached to the + // peer-keyed overlay (ADR-030 §5) — its ops are invoked through the + // CallConnection handle directly, not via PeerRef::Specific. + if let Some(peer_id) = connection.connection().identity().map(|id| id.id.clone()) { + env.attach_peer(peer_id, connection.overlay_env()); + } + Arc::new(env) +} +``` + +### PeerId source: Identity.id (remove UUID workaround) + +`PeerId` is `Identity.id` from `IdentityProvider` resolution — the stable +`PeerEntry.peer_id` (ADR-030 §4). The UUID workaround (ADR-029 Assumption 1's +connection-assigned UUID) is removed. A connection with no resolved identity +has no `PeerId` and is not added to `PeerCompositeEnv` (ADR-030 §5). + +### Migration: CompositeOperationEnv → PeerCompositeEnv + +All call sites that construct `CompositeOperationEnv::new(base, Some(conn), +session)` migrate to `PeerCompositeEnv::new(base).with_session(session). +attach_peer(peer_id, conn)`. The singular-connection case (one peer) is the +degenerate case (`connections` with one entry). + +### What this task does NOT do + +- Does NOT add `invoke_peer` / `peer_contains` / `PeerRef` — that's + `call/operation-env-invoke-peer`. This task builds the struct and the + `invoke_with_policy` (PeerRef::Any equivalent) + `contains` methods. +- Does NOT change `from_call` registration — that's + `call/from-call-forwarded-for` (peer-keyed registration, forwarded_for). +- Does NOT change `services/list` — that's + `call/services-list-accesscontrol-filtered`. + +## Acceptance Criteria + +- [ ] `PeerCompositeEnv` struct with `base`, `session`, `connections: HashMap`, `connection_order: Vec` +- [ ] `PeerId = String` (= `Identity.id`, not UUID) +- [ ] `PeerCompositeEnv::new(base)`, `with_session(session)`, `attach_peer(peer_id, overlay)`, `detach_peer(peer_id)` +- [ ] `invoke_with_policy` routes: session → peers in insertion order → base (first `contains` wins) +- [ ] `contains` checks session + all connections + base +- [ ] `compose_root_env` builds `PeerCompositeEnv` per call, attaches this connection's overlay keyed by `connection.identity().id` +- [ ] Connection with no resolved identity → not attached to peer-keyed overlay (no PeerId) +- [ ] `CompositeOperationEnv` removed (all call sites migrated) +- [ ] UUID workaround removed (no connection-assigned UUID for PeerId) +- [ ] Singular-connection case works (degenerate single-entry map) +- [ ] Unit test: PeerCompositeEnv routes to session when it contains the op +- [ ] Unit test: PeerCompositeEnv routes to first peer (insertion order) that contains the op +- [ ] Unit test: PeerCompositeEnv falls through to base when no overlay contains the op +- [ ] Unit test: attach_peer + detach_peer (detach → NOT_FOUND for that peer) +- [ ] Unit test: connection with no identity → not attached +- [ ] Unit test: reachability check (scoped_env.allows) still gates before routing +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/operation-registry.md — PeerCompositeEnv, OperationEnv +- docs/architecture/crates/call/call-protocol.md — compose_root_env, build_root_context +- docs/architecture/decisions/029-peer-graph-routing-model.md — ADR-029 §1 (peer-keyed overlays) +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md — ADR-030 §4-5 (PeerId source) + +## Notes + +> Highest-risk call task — structural rewrite of the composition env. The +> singular-connection case is the degenerate case (one-entry map). PeerId is +> `Identity.id` (stable `peer_id`), not a UUID — the UUID workaround is removed. +> A connection with no resolved identity gets no PeerId and is not attached +> (ADR-030 §5). The `invoke_peer` / `PeerRef` routing methods are the next +> task (`call/operation-env-invoke-peer`); this task builds the struct and the +> PeerRef::Any-equivalent routing (`invoke_with_policy`). + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/retire-remote-safe.md b/tasks/call/retire-remote-safe.md new file mode 100644 index 0000000..cf95c53 --- /dev/null +++ b/tasks/call/retire-remote-safe.md @@ -0,0 +1,125 @@ +--- +id: call/retire-remote-safe +name: Retire remote_safe/trusted_peer/RemoteFilter — peer authorization via AccessControl (ADR-029 §3) +status: pending +depends_on: [core/review-core-sync] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Remove the ADR-028 peer-authorization machinery from alknet-call. Per ADR-029 +§3, peer authorization now flows through the existing `AccessControl::check +(peer_identity)` — the same mechanism that gates every other call. No +`remote_safe` flag, no `trusted_peer` bypass, no `RemoteFilter` gate. + +This task is the "remove the old" step before the "build the new" (PeerCompositeEnv, +invoke_peer). Removing the ADR-028 machinery first means the new +`AccessControl`-based authorization replaces the old model rather than +coexisting. + +### What to remove + +1. **`HandlerRegistration.remote_safe: bool`** (`registry/registration.rs`): + - Remove the field + - Remove `HandlerRegistration::remote_safe()` setter + - Remove `OperationRegistryBuilder::remote_safe()` method + - Remove all tests asserting `remote_safe` defaults/setter + +2. **`OperationRegistry::list_operations_peer_scoped`** (`registry/registration.rs`): + - Remove the method (replaced by AccessControl-filtered `services/list` in + `call/services-list-accesscontrol-filtered`) + +3. **`services_list_handler_peer_scoped`** (`registry/discovery.rs`): + - Remove the function (replaced by single AccessControl-filtered handler in + `call/services-list-accesscontrol-filtered`) + +4. **`RemoteFilter`** (`protocol/dispatch.rs`): + - Remove the `RemoteFilter` struct and its `default_deny()`/`trusted()`/ + `allows()` methods + - Remove the `remote_filter` field from `Dispatcher` + - Remove the `RemoteFilter` parameter from `Dispatcher::new()` + - Remove the `remote_filter.allows(registration.remote_safe)` gate in + `dispatch_requested` (the AccessControl gate in `OperationRegistry::invoke` + already handles authorization — this task removes the *parallel* gate) + +5. **`CallClient::trusted_peer`** (`client/call_client.rs`): + - Remove the `trusted_peer: bool` field + - Remove `CallClient::trusted_peer()` constructor + - Remove `CallClient::is_trusted_peer()` method + - Remove the `RemoteFilter::trusted()`/`default_deny()` selection in + `spawn_dispatch` + - `CallClient::new()` stays; `spawn_dispatch` constructs `Dispatcher::new` + without `RemoteFilter` + +6. **All ADR-028 tests**: + - Remove tests asserting `remote_safe` behavior, `trusted_peer` mode, + `RemoteFilter` filtering, `list_operations_peer_scoped`, + `services_list_handler_peer_scoped` + - These tests verify the old model; the new model's tests land in the + consuming tasks (`call/services-list-accesscontrol-filtered`, + `call/dispatch-peer-identity`) + +### Transient state + +After this task, the dispatch path authorizes via `AccessControl::check` (which +`OperationRegistry::invoke` already runs) — no parallel gate. The +`PeerCompositeEnv` and `invoke_peer` are not yet built (those are +`call/peer-composite-env` and `call/operation-env-invoke-peer`), so the +composition env is still `CompositeOperationEnv` (singular connection). The +`services/list` handler is the unfiltered `services_list_handler` until +`call/services-list-accesscontrol-filtered` adds the AccessControl filter. + +This transient state compiles and is correct — it's just the ADR-028 model +removed without the ADR-029 peer-keyed routing yet added. The +`AccessControl::check` gate in `OperationRegistry::invoke` is the authorization +mechanism throughout. + +### ADR-029 §3 mapping (the three `remote_safe` cases) + +| `remote_safe` case | Replacement (already in place via AccessControl) | +|---|---| +| Op callable by any peer (was `remote_safe: true`) | `AccessControl::default()` — no restrictions | +| Op callable only by some peers | `AccessControl { required_scopes: [...] }` — peer's `Identity.scopes` must satisfy | +| Op never callable from wire | `Visibility::Internal` — `NOT_FOUND` before ACL | + +## Acceptance Criteria + +- [ ] `HandlerRegistration.remote_safe` field removed +- [ ] `HandlerRegistration::remote_safe()` setter removed +- [ ] `OperationRegistryBuilder::remote_safe()` removed +- [ ] `OperationRegistry::list_operations_peer_scoped` removed +- [ ] `services_list_handler_peer_scoped` removed +- [ ] `RemoteFilter` struct removed from `protocol/dispatch.rs` +- [ ] `Dispatcher.remote_filter` field removed +- [ ] `Dispatcher::new()` no longer takes `RemoteFilter` +- [ ] `CallClient.trusted_peer` field removed +- [ ] `CallClient::trusted_peer()` constructor removed +- [ ] `CallClient::is_trusted_peer()` removed +- [ ] `dispatch_requested` no longer has the `remote_filter.allows` gate +- [ ] All ADR-028 tests removed +- [ ] No `remote_safe`/`trusted_peer`/`RemoteFilter` references remain in the crate +- [ ] `cargo test -p alknet-call` succeeds (remaining tests pass — the AccessControl gate in invoke still works) +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/decisions/029-peer-graph-routing-model.md — ADR-029 §3 (retire remote_safe/trusted_peer) +- docs/architecture/crates/call/operation-registry.md — HandlerRegistration (remote_safe removed) +- docs/architecture/crates/call/client-and-adapters.md — CallClient (trusted_peer removed) + +## Notes + +> This is the "remove the old" step. The new model (PeerCompositeEnv, +> invoke_peer, AccessControl-filtered services/list) lands in subsequent tasks. +> The transient state after this task compiles and is correct — +> `AccessControl::check` in `OperationRegistry::invoke` is the authorization +> mechanism throughout. The ADR-028 tests are removed because they verify the +> old model; the new model's tests land in the consuming tasks. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/review-call-sync.md b/tasks/call/review-call-sync.md new file mode 100644 index 0000000..a52f904 --- /dev/null +++ b/tasks/call/review-call-sync.md @@ -0,0 +1,155 @@ +--- +id: call/review-call-sync +name: Review alknet-call ADR-029/030/032/034 sync for spec conformance +status: pending +depends_on: [call/operation-env-invoke-peer, call/services-list-accesscontrol-filtered, call/call-client-verifier-selection, call/from-call-forwarded-for, call/dispatch-peer-identity] +scope: broad +risk: low +impact: phase +level: review +--- + +## Description + +Review the alknet-call implementation after the ADR-029/030/032/034 sync for +spec conformance, pattern consistency, and correctness. This is the quality +checkpoint at the end of the call phase — the most complex sync in this batch. + +### Review Checklist + +1. **remote_safe/trusted_peer retirement** (ADR-029 §3): + - No `remote_safe` field on `HandlerRegistration` + - No `trusted_peer` on `CallClient` + - No `RemoteFilter` in dispatch + - No `list_operations_peer_scoped` / `services_list_handler_peer_scoped` + - No ADR-028 test remnants + - Peer authorization via `AccessControl::check(peer_identity)` only + +2. **PeerCompositeEnv conformance** (ADR-029 §1, ADR-030 §4-5): + - `PeerCompositeEnv` with peer-keyed `connections: HashMap` + - `connection_order: Vec` (insertion order for `PeerRef::Any`) + - `PeerId = Identity.id` (stable `peer_id`, not UUID) + - `compose_root_env` builds `PeerCompositeEnv` per call + - Connection with no resolved identity → not attached (no PeerId) + - `CompositeOperationEnv` removed (all call sites migrated) + - Singular-connection case works (degenerate single-entry map) + +3. **invoke_peer / PeerRef conformance** (ADR-029 §2): + - `PeerRef::Specific(PeerId)` / `PeerRef::Any` enum + - `OperationEnv::invoke_peer` with default-impl (back-compat) + - `OperationEnv::peer_contains` with default-impl + - `PeerCompositeEnv` overrides with real peer-keyed routing + - `PeerRef::Specific` → no fallthrough (NOT_FOUND if peer doesn't serve op) + - `PeerRef::Any` → insertion-order first-match + - Reachability check (`scoped_env.allows`) gates before routing + +4. **forwarded_for conformance** (ADR-032): + - `OperationContext.forwarded_for: Option` field + - `build_root_context` populates from `call.requested.forwarded_for` + - `OperationEnv::invoke` sets `None` for composed children (wire-ingress only) + - `AccessControl::check` never reads `forwarded_for` (structural invariant) + - `from_call` handler populates `forwarded_for` from `context.identity` + +5. **services/list AccessControl-filtered** (ADR-029 §6): + - `services/list` filters by `AccessControl::check(calling_peer_identity)` + - Op with `AccessControl::default()` listed to any peer + - Op with `required_scopes` hidden from unauthorized peers + - `services/list-peers` opt-in (peer-attributed, AccessControl-filtered) + - `services/schema` unchanged + +6. **CallClient verifier selection** (OQ-29, ADR-034): + - Client presents Ed25519 key as RFC 7250 raw public key client cert + - `AcceptAnyServerCertVerifier` removed (security hole) + - Verifier by `PeerEntry` presence: `Some` → fingerprint pin, `None` + X.509 → CA, `None` + Ed25519 → fail closed + - `CallCredentials.remote_identity: None` is load-bearing (not placeholder) + - No-env-vars invariant preserved (credentials from Capabilities) + +7. **from_call peer-keyed registration** (ADR-029 §5): + - `from_call` registers into peer's sub-overlay + - Same-peer collision = error (`SamePeerCollision`) + - Cross-peer collision dissolves (separate sub-overlays) + - `namespace_prefix` defaults to `None` (optional local-naming sugar) + - `forwarded_for` populated from hub's `context.identity` + +8. **dispatch_peer_identity** (ADR-029 §3, ADR-030 §5): + - `dispatch_requested` resolves peer `Identity` + - `AccessControl::check(peer_identity)` is the sole authorization gate + - `PeerId` from `connection.identity().id` (not UUID) + - Connection with no identity → no PeerId, not in PeerCompositeEnv + +9. **ADR conformance**: + - ADR-029: peer-keyed overlays, PeerRef routing, AccessControl-based peer auth, retire remote_safe + - ADR-030: PeerId = Identity.id = PeerEntry.peer_id (stable) + - ADR-032: forwarded_for (metadata only, wire-ingress only, never in ACL) + - ADR-034: three remote roles, verifier selection by PeerEntry presence + +10. **Security constraints**: + - Capabilities non-serializable, zeroized, immutable (unchanged) + - No secret material on wire (unchanged) + - `forwarded_for` is metadata, not authority (AccessControl::check never reads it) + - Internal ops → NOT_FOUND from wire (unchanged) + - Reachability check bounds composition (unchanged) + - No-env-vars invariant (credentials from Capabilities, not env vars) + - `AcceptAnyServerCertVerifier` removed (no security hole for X.509) + +11. **Pattern consistency**: + - `OperationEnv` is a trait (not concrete) — preserved + - `PeerCompositeEnv` uses `contains()` probe before dispatch — preserved + - Authority switch on `internal: true` — preserved + - Deadline inheritance — preserved + - Metadata fresh on composition (`HashMap::new()`) — preserved + +12. **Test coverage**: + - PeerCompositeEnv routing (session, peers in order, base fallthrough) + - PeerRef::Specific (routes, NOT_FOUND on missing peer) + - PeerRef::Any (insertion-order first-match) + - forwarded_for (populated, None for composed children, never in ACL) + - services/list AccessControl-filtered + - AccessControl::check peer authorization (allowed, FORBIDDEN, Internal NOT_FOUND) + - CallClient verifier selection (fingerprint pin, CA, fail closed) + - from_call forwarded_for + peer-keyed registration + collision rules + +## Acceptance Criteria + +- [ ] No `remote_safe`/`trusted_peer`/`RemoteFilter` references remain +- [ ] `PeerCompositeEnv` matches ADR-029 §1 (peer-keyed, insertion order) +- [ ] `PeerId = Identity.id` (stable, not UUID) +- [ ] `invoke_peer`/`peer_contains`/`PeerRef` match ADR-029 §2 +- [ ] `forwarded_for` matches ADR-032 (metadata only, wire-ingress only, never in ACL) +- [ ] `services/list` AccessControl-filtered; `services/list-peers` opt-in +- [ ] CallClient verifier selection matches ADR-034 §3 +- [ ] `from_call` peer-keyed registration + forwarded_for + collision rules +- [ ] `AccessControl::check(peer_identity)` is the sole authorization gate +- [ ] `AcceptAnyServerCertVerifier` removed +- [ ] No-env-vars invariant preserved +- [ ] `OperationEnv` is a trait (not concrete) +- [ ] Test coverage adequate for all new functionality +- [ ] `cargo fmt --check -p alknet-call` passes +- [ ] `cargo clippy -p alknet-call` passes with no warnings +- [ ] All tests pass + +## References + +- docs/architecture/crates/call/README.md +- docs/architecture/crates/call/call-protocol.md +- docs/architecture/crates/call/operation-registry.md +- docs/architecture/crates/call/client-and-adapters.md +- docs/architecture/decisions/029-peer-graph-routing-model.md +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md +- docs/architecture/decisions/032-forwarded-for-identity.md +- docs/architecture/decisions/034-outgoing-only-x509-and-three-peer-roles.md + +## Notes + +> This is the most complex sync in this batch. The review should verify that +> the peer-keyed routing model (ADR-029), the stable PeerId (ADR-030), the +> forwarded_for metadata field (ADR-032), and the verifier selection (ADR-034) +> all work correctly together. The `OperationEnv` trait-object design is +> load-bearing — verify it's still a trait, not concrete. The +> `AccessControl::check`-based peer authorization is the security property — +> verify no parallel gate remains. If deviations are found, document and fix +> before considering the call sync complete. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/call/services-list-accesscontrol-filtered.md b/tasks/call/services-list-accesscontrol-filtered.md new file mode 100644 index 0000000..15ccd82 --- /dev/null +++ b/tasks/call/services-list-accesscontrol-filtered.md @@ -0,0 +1,122 @@ +--- +id: call/services-list-accesscontrol-filtered +name: Filter services/list by AccessControl::check(peer_identity) and add services/list-peers opt-in (ADR-029 §6) +status: pending +depends_on: [call/retire-remote-safe] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Change `services/list` to filter by `AccessControl::check(calling_peer_identity)` +— the calling peer sees only ops it is authorized to call. Collapse the +`services_list_handler` / `services_list_handler_peer_scoped` split (the latter +was removed in `call/retire-remote-safe`) into a single AccessControl-filtered +handler. Add the opt-in `services/list-peers` for peer-attributed re-export +listing. Per ADR-029 §6. + +### services/list (AccessControl-filtered) + +The single `services_list_handler` filters by `AccessControl::check` against +the calling peer's resolved `Identity`: + +```rust +pub fn services_list_handler(registry: Arc) -> Handler { + Arc::new(move |_input, context| { + let registry = Arc::clone(®istry); + Box::pin(async move { + let calling_identity = &context.identity; + let ops: Vec = registry.list_operations() + .into_iter() + .filter(|spec| { + // Only list ops the calling peer is authorized to call. + // AccessControl::check returns Allowed/Forbidden. + spec.access_control.check(calling_identity.as_ref()).is_allowed() + }) + .map(|spec| serde_json::json!({ + "name": spec.name, + "namespace": spec.namespace, + "op_type": spec.op_type, + })) + .collect(); + ResponseEnvelope::ok(context.request_id, serde_json::json!({ "operations": ops })) + }) + }) +} +``` + +- An op with `AccessControl::default()` (no restrictions) is listed to any + peer — implicitly callable by any authenticated peer. +- An op with `required_scopes` is listed only to peers whose `Identity.scopes` + satisfy them. +- An op with `Visibility::Internal` is never listed (excluded from + `list_operations()` which already filters to `External`). + +### services/list-peers (opt-in) + +The opt-in for peer-attributed re-export listing — each peer's sub-overlay +listed with attribution, filtered by the calling peer's authorization: + +```rust +pub fn services_list_peers_handler(/* ... */) -> Handler { + // Lists ops from each peer's sub-overlay in PeerCompositeEnv, + // attributed by peer_id, filtered by AccessControl::check(calling_identity). + // Opt-in: the calling peer invokes this operation name explicitly. +} +``` + +This operation is registered alongside `services/list` and `services/schema`. +It iterates the peer-keyed overlays (via `context.env`), lists each peer's ops +with `peer_id` attribution, and filters by the calling peer's authorization. + +### What this task does NOT do + +- Does NOT change `services/schema` (unchanged — returns full spec for a + named op). +- Does NOT build the `PeerCompositeEnv` (that's `call/peer-composite-env`) — + but `services/list-peers` consumes it via `context.env`. If + `PeerCompositeEnv` is not yet built, `services/list-peers` can be registered + with a stub that returns empty until the env is ready, or this task can + depend on `call/peer-composite-env`. **This task depends only on + `call/retire-remote-safe`** so `services/list` (the AccessControl filter) + can land independently; `services/list-peers` is implemented to consume + `PeerCompositeEnv` via `context.env.peer_contains` (which has a default-impl + that works even before `PeerCompositeEnv` is wired). + +## Acceptance Criteria + +- [ ] `services_list_handler` filters by `AccessControl::check(context.identity)` +- [ ] Op with `AccessControl::default()` listed to any peer +- [ ] Op with `required_scopes` listed only to authorized peers +- [ ] Op with `Visibility::Internal` never listed (unchanged — `list_operations` filters to External) +- [ ] `services_list_handler_peer_scoped` removed (was removed in `call/retire-remote-safe`; verify gone) +- [ ] `services/list-peers` opt-in operation added (peer-attributed, AccessControl-filtered) +- [ ] `services/schema` unchanged +- [ ] Unit test: `services/list` lists only ops the calling peer is authorized for +- [ ] Unit test: op with no restrictions listed to any peer +- [ ] Unit test: op with required_scopes hidden from unauthorized peer +- [ ] Unit test: `services/list-peers` attributes ops by peer_id +- [ ] `cargo test -p alknet-call` succeeds +- [ ] `cargo clippy -p alknet-call` succeeds with no warnings + +## References + +- docs/architecture/crates/call/operation-registry.md — Service Discovery +- docs/architecture/crates/call/client-and-adapters.md — services/list AccessControl-filtered +- docs/architecture/decisions/029-peer-graph-routing-model.md — ADR-029 §6 + +## Notes + +> `services/list` semantics change: the filter is `AccessControl`-based, not +> `remote_safe`-based. An op with `AccessControl::default()` is now listed to +> any peer — this is correct (it's implicitly callable by any authenticated +> peer). Operators who relied on `remote_safe: false` to hide ops from peers +> must instead set `required_scopes` or `Visibility::Internal`. The +> `services/list-peers` opt-in is for peer-attributed re-export listing. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/config-identity-provider-peerentry.md b/tasks/core/config-identity-provider-peerentry.md new file mode 100644 index 0000000..3cdc341 --- /dev/null +++ b/tasks/core/config-identity-provider-peerentry.md @@ -0,0 +1,114 @@ +--- +id: core/config-identity-provider-peerentry +name: Rewrite ConfigIdentityProvider resolution to use PeerEntry multi-credential path (ADR-030) +status: pending +depends_on: [core/peer-entry-model] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Rewrite `ConfigIdentityProvider::resolve_from_fingerprint` and +`resolve_from_token` to use the new `PeerEntry`-based resolution from +`core/peer-entry-model`. This is the resolution-logic half of the ADR-030 +change — the data model (PeerEntry struct, AuthPolicy.peers) lands in +`core/peer-entry-model`; this task updates the `ConfigIdentityProvider` methods +that delegate to `AuthPolicy`. + +### Current state (pre-ADR-030) + +```rust +impl IdentityProvider for ConfigIdentityProvider { + fn resolve_from_fingerprint(&self, fingerprint: &str) -> Option { + let config = self.dynamic.load(); + config.auth.resolve_identity_from_fingerprint(fingerprint) + } + + fn resolve_from_token(&self, token: &AuthToken) -> Option { + let config = self.dynamic.load(); + let token_str = String::from_utf8_lossy(&token.raw); + config.auth.resolve_api_key(&token_str) // ← only ApiKeyEntry path + } +} +``` + +### Target state (ADR-030) + +```rust +impl IdentityProvider for ConfigIdentityProvider { + fn resolve_from_fingerprint(&self, fingerprint: &str) -> Option { + let config = self.dynamic.load(); + config.auth.resolve_identity_from_fingerprint(fingerprint) + // Now resolves: fingerprint → PeerEntry → Identity { id: peer_id, ... } + } + + fn resolve_from_token(&self, token: &AuthToken) -> Option { + let config = self.dynamic.load(); + let token_str = String::from_utf8_lossy(&token.raw); + config.auth.resolve_identity_from_token(&token_str) + // Now tries PeerEntry.auth_token_hash first, falls through to ApiKeyEntry + } +} +``` + +The key change in `resolve_from_token`: it now calls +`AuthPolicy::resolve_identity_from_token` (added in `core/peer-entry-model`), +which tries the `PeerEntry.auth_token_hash` path first (token is one credential +path among several for a stable logical peer → `Identity.id = peer_id`), then +falls through to `resolve_api_key` (token IS the identity → `Identity.id = +prefix`). + +### ConfigIdentityProvider stays read-only + +`ConfigIdentityProvider` still reads from `ArcSwap` on every call +(hot-reloadable). It does NOT implement `IdentityStore` (that's +`core/identity-store-trait` — config reload is its write path, not a method +call). + +### Test migration + +The existing auth.rs tests use `authorized_fingerprints: HashSet` and +expect `Identity.id == fingerprint`. These must migrate to the `PeerEntry` model: +- `config_with_fingerprint` helper → `config_with_peer_entry` helper +- Fingerprint resolution tests expect `Identity.id == peer_id` (not the fingerprint) +- Add token-resolution-via-PeerEntry tests (auth_token_hash path) +- Config reload tests use `PeerEntry` in the new config + +## Acceptance Criteria + +- [ ] `ConfigIdentityProvider::resolve_from_fingerprint` delegates to `AuthPolicy::resolve_identity_from_fingerprint` (PeerEntry path) +- [ ] `ConfigIdentityProvider::resolve_from_token` delegates to `AuthPolicy::resolve_identity_from_token` (PeerEntry.auth_token_hash → fall through to ApiKeyEntry) +- [ ] `ConfigIdentityProvider` reads from ArcSwap on every call (hot-reloadable — unchanged) +- [ ] `ConfigIdentityProvider` does NOT implement `IdentityStore` +- [ ] Fingerprint resolution returns `Identity { id: peer_id, ... }` (stable, not the fingerprint) +- [ ] Token resolution: PeerEntry.auth_token_hash match → `Identity { id: peer_id }`; no match → ApiKeyEntry fall-through → `Identity { id: prefix }` +- [ ] All existing auth.rs tests migrated to PeerEntry model (no `authorized_fingerprints` references) +- [ ] Unit test: fingerprint resolution via PeerEntry (known → Some with peer_id, unknown → None) +- [ ] Unit test: token resolution via PeerEntry.auth_token_hash (matching → Some with peer_id) +- [ ] Unit test: token resolution falls through to ApiKeyEntry when no PeerEntry matches +- [ ] Unit test: config reload changes resolution results immediately (PeerEntry model) +- [ ] Unit test: disabled PeerEntry returns None +- [ ] `cargo test -p alknet-core` succeeds +- [ ] `cargo clippy -p alknet-core` succeeds with no warnings + +## References + +- docs/architecture/crates/core/auth.md — ConfigIdentityProvider, multi-credential resolution +- docs/architecture/crates/core/config.md — AuthPolicy.peers, PeerEntry +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md — ADR-030 §2 (resolution semantics) + +## Notes + +> This is the resolution-logic half of the ADR-030 change. The data model lands +> in `core/peer-entry-model`; this task wires `ConfigIdentityProvider` to the +> new `AuthPolicy` methods. The semantic shift: `Identity.id` changes from the +> fingerprint to the `peer_id` on the fingerprint path. The token path gains a +> new first-try (PeerEntry.auth_token_hash) before the existing ApiKeyEntry +> fall-through. ConfigIdentityProvider stays read-only and ArcSwap-backed. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/credential-store-trait.md b/tasks/core/credential-store-trait.md new file mode 100644 index 0000000..9fe48aa --- /dev/null +++ b/tasks/core/credential-store-trait.md @@ -0,0 +1,144 @@ +--- +id: core/credential-store-trait +name: Add CredentialStore trait, InMemoryCredentialStore, EncryptedData mirror, and StoreError (ADR-031/035) +status: pending +depends_on: [] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Add the second repo trait to alknet-core: `CredentialStore` for encrypted- +credential persistence, alongside its in-memory default adapter and the shared +`StoreError` type. Per ADR-031 (the trait) and ADR-035 (refines `put`/`delete` +to async, renames the error to `StoreError`). + +This task is standalone — it has no dependency on `core/peer-entry-model`. The +`CredentialStore` trait persists `EncryptedData` blobs (the vault's encrypted +output); it never decrypts (ADR-025 — the vault is the sole decryption +boundary). + +### CredentialStore trait + +```rust +pub trait CredentialStore: Send + Sync { + fn get(&self, provider: &str) -> Option; + async fn put(&self, provider: &str, data: &EncryptedData) -> Result<(), StoreError>; + async fn delete(&self, provider: &str) -> Result<(), StoreError>; +} +``` + +- `get` is **sync** (cached read — the hot path; ADR-035 §1). +- `put`/`delete` are **async** (they hit the backend; ADR-035 §3 refines + ADR-031's sync sketch to async within the one-way door). +- `get` returns `Option` (missing credential is the common case, + not an error). +- No `list` method (ADR-031 §4 — additive if needed later). + +### InMemoryCredentialStore + +```rust +pub struct InMemoryCredentialStore { + entries: RwLock>, +} + +impl InMemoryCredentialStore { + pub fn new() -> Self; + pub fn with_entries(entries: HashMap) -> Self; +} + +impl CredentialStore for InMemoryCredentialStore { ... } +``` + +The default adapter covers tests and config-loaded deployments. `put`/`delete` +are async with no `.await` points (trivially satisfy an async trait — ADR-035 +§3). Same posture as `ConfigIdentityProvider` — no persistence, no backend +dependency, no env vars. + +### EncryptedData core mirror + +A thin serializable struct mirroring the vault's `EncryptedData` (ADR-020), +so the trait can reference it without a vault dependency (ADR-018 — vault is +standalone with zero alknet-crate dependencies): + +```rust +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct EncryptedData { + pub key_version: u32, + pub salt: Vec, // wire-format compat (OQ-20); unused in v2 but kept + pub iv: Vec, // AES-GCM IV (OsRng-generated) + pub data: Vec, // ciphertext +} +``` + +The `salt` field is kept for wire-format compatibility with the TS predecessor +(OQ-20) — a core mirror that omitted it could not round-trip the vault's +`EncryptedData`. v2 may write a zero-length salt but must not drop the field +(ADR-035 §6). + +### StoreError + +```rust +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum StoreError { + #[error("backend error: {message}")] + Backend { message: String }, + #[error("not found: {entity}")] + NotFound { entity: String }, + #[error("serialization error: {message}")] + Serialization { message: String }, +} +``` + +Shared by both `CredentialStore` and `IdentityStore` (ADR-035 §7 renames +ADR-031's `CredentialStoreError` to `StoreError`). `#[non_exhaustive]` so +adapter crates can extend without breaking match arms. Lives in alknet-core +(where the traits live). + +### Module placement + +Add a new `store` module (or `credential_store` module) in `alknet-core/src/`. +Re-export `CredentialStore`, `InMemoryCredentialStore`, `EncryptedData`, and +`StoreError` from `lib.rs`. + +## Acceptance Criteria + +- [ ] `CredentialStore` trait with sync `get`, async `put`/`delete` +- [ ] `InMemoryCredentialStore` with `new()` and `with_entries()` +- [ ] `InMemoryCredentialStore` implements `CredentialStore` (async put/delete with no .await points) +- [ ] `EncryptedData` core mirror with 4 fields (`key_version`, `salt`, `iv`, `data`), derives Serialize/Deserialize/Clone/Debug +- [ ] `StoreError` enum with 3 variants, `#[non_exhaustive]`, `thiserror::Error` +- [ ] No vault dependency added to alknet-core (EncryptedData is a core-owned mirror) +- [ ] No `list` method on the trait +- [ ] Unit test: InMemoryCredentialStore get/put/delete round-trip +- [ ] Unit test: get returns None for missing provider +- [ ] Unit test: EncryptedData serializes and deserializes (round-trip) +- [ ] Unit test: StoreError Display formatting +- [ ] `cargo test -p alknet-core` succeeds +- [ ] `cargo clippy -p alknet-core` succeeds with no warnings + +## References + +- docs/architecture/crates/core/auth.md — CredentialStore, StoreError, EncryptedData mirror +- docs/architecture/decisions/031-credentialstore-repo-trait.md — ADR-031 (the trait) +- docs/architecture/decisions/035-concrete-persistence-adapter-shapes.md — ADR-035 (async put/delete, StoreError rename, schema) +- docs/architecture/decisions/033-storage-boundary-and-repo-adapter-pattern.md — ADR-033 (the pattern) + +## Notes + +> Standalone task — no dependency on PeerEntry. The `CredentialStore` trait is +> the second repo trait in core (alongside `IdentityProvider`), establishing +> the repo/adapter pattern concretely (ADR-033). The trait is the one-way door; +> the in-memory default is the reference implementation; persistence adapters +> (alknet-store-sqlite, ADR-035) are separate crates, not built in this sync. +> `get` stays sync because the credential load happens at startup into +> `Capabilities` (ADR-031); `put`/`delete` are async because a SQLite-backed +> adapter cannot do a sync write without blocking (ADR-035 §3). + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/fingerprint-normalization.md b/tasks/core/fingerprint-normalization.md new file mode 100644 index 0000000..539c21f --- /dev/null +++ b/tasks/core/fingerprint-normalization.md @@ -0,0 +1,127 @@ +--- +id: core/fingerprint-normalization +name: Normalize quinn Ed25519 raw-key fingerprint to ed25519:hex format (ADR-030 §6) +status: pending +depends_on: [core/peer-entry-model] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Normalize the quinn Ed25519 raw-key fingerprint extraction to produce +`ed25519:`, matching the iroh path. Currently +`fingerprint_from_cert_der` produces `SHA256:` for ALL certs, +including RFC 7250 raw public keys. ADR-030 §6 requires that Ed25519 raw keys +produce `ed25519:` regardless of transport (quinn or iroh), so the same +key has the same fingerprint in `PeerEntry.fingerprints` — one entry, both +transports. + +### Current state + +```rust +// crates/alknet-core/src/endpoint.rs +fn extract_quinn_client_fingerprint(connection: &quinn::Connection) -> Option { + let identity = connection.peer_identity()?; + let cert = identity.iter().next()?; + fingerprint_from_cert_der(cert.as_ref()) +} + +fn fingerprint_from_cert_der(cert_der: &[u8]) -> Option { + // Always SHA256: — wrong for Ed25519 raw keys + let mut hasher = Sha256::new(); + hasher.update(cert_der); + Some(format!("SHA256:{}", hex::encode(hasher.finalize()))) +} + +fn extract_iroh_client_fingerprint(connection: &iroh::endpoint::Connection) -> Option { + let node_id = connection.remote_node_id().ok()?; + Some(format!("ed25519:{}", node_id)) // ← already correct +} +``` + +### Target state (ADR-030 §6) + +`fingerprint_from_cert_der` (or a new `fingerprint_from_client_cert` function) +must distinguish: + +1. **RFC 7250 raw public key cert** (SPKI with Ed25519 algorithm identifier): + extract the raw 32-byte Ed25519 public key from the SPKI DER and format as + `ed25519:`. This matches the iroh path — the same + key has the same fingerprint regardless of transport. + +2. **X.509 cert**: keep `SHA256:` (the DER hash — X.509 certs + don't have a "raw public key" form). + +The distinction is whether the presented cert is an RFC 7250 raw public key +(SPKI with Ed25519 algorithm identifier, no X.509 wrapper) or a full X.509 +cert. The `RawKeyCertResolver` on the server side already has the raw key bytes +via `Ed25519SecretKey::public()`; the client-side extraction must parse the +SPKI DER to extract the raw key. + +### Fingerprint format table (ADR-030 §6) + +| Transport | Source | Format | +|-----------|--------|--------| +| iroh (direct or relay) | peer `NodeId` (Ed25519 public key) | `ed25519:` | +| quinn (RFC 7250 raw key) | SPKI cert → extract raw Ed25519 pub key | `ed25519:` (normalized) | +| quinn (X.509) | leaf client cert DER | `SHA256:` | + +### Implementation approach + +Parse the cert DER to detect whether it's a raw public key (SPKI) or an X.509 +cert. If SPKI with Ed25519 algorithm identifier, extract the 32-byte public key +and format as `ed25519:`. Otherwise, hash the full DER as `SHA256:`. + +The `rustls-pki-types` crate (already a dependency) provides +`CertificateDer`. The `rustls` crate's webpki or a manual DER parse of the +SPKI's `SubjectPublicKeyInfo` → `subjectPublicKey` field can extract the raw +key. A minimal DER parser for the SPKI structure (AlgorithmIdentifier + +subjectPublicKey) is sufficient — the structure is small and well-defined. + +### Test migration + +The existing endpoint.rs tests expect `SHA256:` for all fingerprints. Tests +with Ed25519 raw keys must migrate to expect `ed25519:`. Tests with X.509 certs +stay `SHA256:`. Add a test that the same Ed25519 key produces the same +fingerprint via both the quinn SPKI-extraction path and the iroh NodeId path +(if testable without a live iroh connection, test the format function directly). + +## Acceptance Criteria + +- [ ] `fingerprint_from_cert_der` (or replacement) distinguishes RFC 7250 raw key SPKI from X.509 cert +- [ ] Ed25519 raw key (SPKI) → `ed25519:` +- [ ] X.509 cert → `SHA256:` (unchanged) +- [ ] iroh path already produces `ed25519:` (unchanged — verify) +- [ ] Same Ed25519 key produces same fingerprint via quinn and iroh paths +- [ ] No-client-cert case still produces `tls_client_fingerprint: None` (no regression) +- [ ] Unit test: Ed25519 raw key SPKI → `ed25519:` format +- [ ] Unit test: X.509 cert → `SHA256:` format (unchanged) +- [ ] Unit test: fingerprint is lowercase hex +- [ ] Unit test: 32-byte pub key extracted correctly (not the DER wrapper) +- [ ] Existing endpoint.rs fingerprint tests migrated (Ed25519 → `ed25519:`, X.509 → `SHA256:`) +- [ ] `cargo test -p alknet-core` succeeds +- [ ] `cargo clippy -p alknet-core` succeeds with no warnings + +## References + +- docs/architecture/crates/core/auth.md — Fingerprint string format table +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md — ADR-030 §6 (normalization rationale) +- docs/architecture/decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md — ADR-027 (RawKey model) + +## Notes + +> The normalization is load-bearing for the peer graph: a peer that connects +> via quinn direct and via iroh must have the same fingerprint in +> `PeerEntry.fingerprints` — one entry, both transports. Without this, the same +> key produces `ed25519:abc...` on iroh and `SHA256:def...` on quinn, breaking +> the ADR-030 resolution path. The X.509 path stays `SHA256:` +> because X.509 certs don't have a "raw public key" form. This also simplifies +> the coming WebTransport relay work (proxied Ed25519 identity is the same +> `ed25519:` whether direct or proxied). + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/identity-store-trait.md b/tasks/core/identity-store-trait.md new file mode 100644 index 0000000..3decb0d --- /dev/null +++ b/tasks/core/identity-store-trait.md @@ -0,0 +1,98 @@ +--- +id: core/identity-store-trait +name: Add IdentityStore async write trait extending IdentityProvider (ADR-035) +status: pending +depends_on: [core/peer-entry-model] +scope: single +risk: low +impact: component +level: implementation +--- + +## Description + +Add the `IdentityStore` async write trait for peer management, extending the +read-only `IdentityProvider` trait. Per ADR-035 §2. + +`IdentityProvider` is read-only today and stays read-only — it is the hot-path +trait called on every incoming connection (sync, no `.await`). Peer mutations +(add/update/remove a `PeerEntry`) go through this separate async trait. + +### IdentityStore trait + +```rust +/// Write trait — management path, async (ADR-035). ConfigIdentityProvider +/// does NOT implement this (config reload is its write path — see below). +/// SqliteIdentityProvider does: writes hit SQLite, emit honker NOTIFY, +/// and the local LISTEN refreshes the in-memory read index. +#[async_trait] +pub trait IdentityStore: IdentityProvider { + async fn put_peer(&self, peer: &PeerEntry) -> Result<(), StoreError>; + async fn update_peer(&self, peer_id: &str, peer: &PeerEntry) -> Result<(), StoreError>; + async fn remove_peer(&self, peer_id: &str) -> Result<(), StoreError>; +} +``` + +- `put_peer` — insert or replace a `PeerEntry` (upsert by `peer_id`). +- `update_peer` — update an existing `PeerEntry` (error if `peer_id` not found; + for upsert semantics use `put_peer`). +- `remove_peer` — delete a `PeerEntry` by `peer_id`. + +### Why a separate trait, not async methods on IdentityProvider + +- The hot-path read trait is consumed by the accept loop and every handler — + those call sites are sync and must not gain `.await`. If `put_peer` were on + `IdentityProvider`, every consumer would see the async method even though + only the management path calls it. A separate `IdentityStore: IdentityProvider` + supertrait keeps the read surface lean and makes the write surface opt-in. +- `ConfigIdentityProvider` does **not** implement `IdentityStore`. Its write + path is config reload (`ConfigReloadHandle::reload`), not a method call. This + preserves the config-is-source-of-truth model. Implementing `IdentityStore` +> for `ConfigIdentityProvider` "for symmetry" would violate that model — the +> constraint is the absence of a backend, not a type-system constraint. + +### ConfigIdentityProvider posture + +`ConfigIdentityProvider` deliberately does NOT implement `IdentityStore`. This +task does not change `ConfigIdentityProvider` — it only adds the trait. The +trait is defined for future adapters (`SqliteIdentityProvider` in +`alknet-store-sqlite`) to implement. `StoreError` is already defined by +`core/credential-store-trait`. + +### Module placement + +Add `IdentityStore` alongside `IdentityProvider` in `alknet-core/src/auth.rs` +(or a new `store` module if `CredentialStore` landed there). Re-export from +`lib.rs`. + +## Acceptance Criteria + +- [ ] `IdentityStore` trait with `put_peer`, `update_peer`, `remove_peer` (all async) +- [ ] `IdentityStore: IdentityProvider` (supertrait) +- [ ] `StoreError` used as the error type (from `core/credential-store-trait`) +- [ ] `ConfigIdentityProvider` does NOT implement `IdentityStore` +- [ ] `#[async_trait]` on the trait +- [ ] No changes to `IdentityProvider` trait (stays read-only, sync) +- [ ] Unit test: a mock/test impl of `IdentityStore` compiles and works (verify the trait is implementable) +- [ ] Unit test: `ConfigIdentityProvider` does not implement `IdentityStore` (compile-time or trait-bound assertion) +- [ ] `cargo test -p alknet-core` succeeds +- [ ] `cargo clippy -p alknet-core` succeeds with no warnings + +## References + +- docs/architecture/crates/core/auth.md — IdentityStore write trait, ConfigIdentityProvider posture +- docs/architecture/decisions/035-concrete-persistence-adapter-shapes.md — ADR-035 §2 (the trait, read/write split rationale) +- docs/architecture/decisions/033-storage-boundary-and-repo-adapter-pattern.md — ADR-033 (the pattern) + +## Notes + +> Small task but locks the trait shape — a one-way door. The read/write split +> keeps the hot path sync (no `.await` in the accept loop). `ConfigIdentityProvider` +> not implementing `IdentityStore` is a design posture, not a type-system +> constraint: it holds no backend, and its write path is config reload. A +> deployment that wants method-call peer management wires the SQLite adapter +> (a separate crate, not built in this sync). + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/peer-entry-model.md b/tasks/core/peer-entry-model.md new file mode 100644 index 0000000..8797135 --- /dev/null +++ b/tasks/core/peer-entry-model.md @@ -0,0 +1,175 @@ +--- +id: core/peer-entry-model +name: Add PeerEntry struct and replace AuthPolicy.authorized_fingerprints with peers (ADR-030) +status: pending +depends_on: [] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Replace `AuthPolicy.authorized_fingerprints: HashSet` with +`AuthPolicy.peers: Vec`, per ADR-030. This is the foundational data +change for the entire ADR-029/030 sync — every downstream task (core resolution +logic, IdentityStore, call peer-keyed routing, fingerprint normalization) depends +on this struct and the `AuthPolicy.peers` field. + +This task adds the `PeerEntry` struct and the `AuthPolicy.peers` field, and +migrates the `AuthPolicy` resolution methods to the new model. The +`ConfigIdentityProvider` rewrite (the resolution-logic half) is a separate task +(`core/config-identity-provider-peerentry`) so this task stays focused on the +data model + `AuthPolicy` resolution methods. + +### PeerEntry struct + +```rust +pub struct PeerEntry { + /// Stable logical peer id ("worker-a", "alice"). Does NOT change on + /// key rotation. This becomes Identity.id on resolution, regardless of + /// which credential path resolved the identity. + pub peer_id: String, + + /// TLS fingerprints for this peer — one or more. A peer may have + /// multiple keys (e.g., an Ed25519 raw key for P2P and an X.509 cert + /// for domain-facing). Resolution matches against any entry. + /// Format: "ed25519:" for RFC 7250 raw keys + /// (normalized across quinn and iroh — ADR-030 §6), "SHA256:" for + /// X.509 certs (DER hash). Changes on key rotation. + pub fingerprints: Vec, + + /// Optional: bearer-token authentication for this peer. A peer that + /// also authenticates via auth_token (e.g., HTTP clients that can't + /// do TLS client-auth) stores the SHA-256 hash of the token here. + /// Resolution via resolve_from_token matches this field and returns + /// the same Identity { id: peer_id, ... } as the fingerprint path. + pub auth_token_hash: Option, + + /// Authorization scopes granted to this peer. Resolved into + /// Identity.scopes. + pub scopes: Vec, + + /// Named resource lists granted to this peer. Resolved into + /// Identity.resources. + pub resources: HashMap>, + + /// Human-readable display name for logs / UIs. Optional. + pub display_name: Option, + + /// Whether this peer is authorized at all. false = recognized but + /// disabled (revoked). Resolution returns None. + pub enabled: bool, +} +``` + +### AuthPolicy change + +```rust +pub struct AuthPolicy { + /// Replaces authorized_fingerprints: HashSet. Each entry maps + /// a stable logical peer_id to its credential paths (fingerprints, + /// optional auth_token_hash) + scopes + resources. The list is keyed + /// by peer_id; resolution looks up by fingerprint OR auth_token. + pub peers: Vec, + + /// API keys for bearer-token auth where the token IS the identity + /// (rotation = new identity). Unchanged by ADR-030. + pub api_keys: Vec, +} +``` + +### AuthPolicy resolution methods (new model) + +`AuthPolicy::resolve_identity_from_fingerprint` and a new +`resolve_identity_from_token` method resolve via `PeerEntry`: + +```rust +impl AuthPolicy { + pub fn resolve_identity_from_fingerprint(&self, fingerprint: &str) -> Option { + self.peers.iter() + .find(|p| p.enabled && p.fingerprints.iter().any(|f| f == fingerprint)) + .map(|p| Identity { + id: p.peer_id.clone(), + scopes: p.scopes.clone(), + resources: p.resources.clone(), + }) + } + + pub fn resolve_identity_from_token(&self, token: &str) -> Option { + let token_hash = sha256(token); + self.peers.iter() + .find(|p| p.enabled && p.auth_token_hash.as_deref() == Some(&token_hash)) + .map(|p| Identity { + id: p.peer_id.clone(), + scopes: p.scopes.clone(), + resources: p.resources.clone(), + }) + .or_else(|| self.resolve_api_key(token)) // fall through to ApiKeyEntry + } +} +``` + +The key change: `Identity.id` is now the stable `peer_id`, **not** the +fingerprint. Key rotation changes the fingerprint but not the `peer_id`, so ACL +entries and routing references stay stable (ADR-030 §2-3). + +`resolve_api_key` stays unchanged (the `ApiKeyEntry` path where the token IS the +identity — `Identity.id = prefix`). + +### Config validation + +`PeerEntry.peer_id` is operator-chosen and unique within a config. Add a +validation method or assertion that duplicate `peer_id` values in +`AuthPolicy.peers` are a config error (ADR-030 Assumption 2). + +### What this task does NOT do + +- Does NOT rewrite `ConfigIdentityProvider` — that's + `core/config-identity-provider-peerentry` (the `ConfigIdentityProvider` methods + delegate to `AuthPolicy` resolution, so they keep working once `AuthPolicy` + is updated, but the token-resolution path in `ConfigIdentityProvider` needs to + call the new `resolve_identity_from_token` instead of only `resolve_api_key`). +- Does NOT normalize quinn fingerprints to `ed25519:` — that's + `core/fingerprint-normalization`. +- Does NOT add `IdentityStore` or `CredentialStore` — those are separate tasks. + +## Acceptance Criteria + +- [ ] `PeerEntry` struct with all 7 fields (`peer_id`, `fingerprints`, `auth_token_hash`, `scopes`, `resources`, `display_name`, `enabled`) +- [ ] `AuthPolicy.authorized_fingerprints` removed; replaced with `peers: Vec` +- [ ] `AuthPolicy.api_keys` unchanged +- [ ] `AuthPolicy::resolve_identity_from_fingerprint` resolves fingerprint → PeerEntry → `Identity { id: peer_id, ... }` +- [ ] `AuthPolicy::resolve_identity_from_token` resolves token hash → PeerEntry → `Identity { id: peer_id, ... }`, falls through to `resolve_api_key` +- [ ] `Identity.id` is the `peer_id` (stable), not the fingerprint +- [ ] Disabled peers (`enabled: false`) return `None` from resolution +- [ ] Duplicate `peer_id` validation (config error) +- [ ] Unit test: fingerprint resolution via PeerEntry (known → Some with peer_id, unknown → None, disabled → None) +- [ ] Unit test: token resolution via PeerEntry.auth_token_hash (matching → Some with peer_id, non-matching → fall through to ApiKeyEntry) +- [ ] Unit test: multi-fingerprint PeerEntry (any fingerprint in the list resolves to the same peer_id) +- [ ] Unit test: resources populated from PeerEntry.resources on both paths +- [ ] Unit test: duplicate peer_id detected/rejected +- [ ] `cargo test -p alknet-core` succeeds +- [ ] `cargo clippy -p alknet-core` succeeds with no warnings + +## References + +- docs/architecture/crates/core/config.md — PeerEntry, AuthPolicy.peers +- docs/architecture/crates/core/auth.md — Identity.id = peer_id, multi-credential resolution +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md — ADR-030 + +## Notes + +> This is the foundational data change for the ADR-029/030 sync. The key +> semantic shift: `Identity.id` changes from the fingerprint (crypto material) +> to the `peer_id` (stable logical id). Key rotation changes the fingerprint +> but not the `peer_id`, so ACL entries and `PeerRef::Specific(peer_id)` +> references stay stable. `ConfigIdentityProvider` keeps working (it delegates +> to `AuthPolicy`), but the token path needs the new +> `resolve_identity_from_token` — that's the separate +> `core/config-identity-provider-peerentry` task. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/review-core-sync.md b/tasks/core/review-core-sync.md new file mode 100644 index 0000000..d5dd994 --- /dev/null +++ b/tasks/core/review-core-sync.md @@ -0,0 +1,123 @@ +--- +id: core/review-core-sync +name: Review alknet-core ADR-029/030/031/034/035 sync for spec conformance +status: pending +depends_on: [core/credential-store-trait, core/identity-store-trait, core/config-identity-provider-peerentry, core/fingerprint-normalization, core/three-remote-roles-docs] +scope: moderate +risk: low +impact: phase +level: review +--- + +## Description + +Review the alknet-core implementation after the ADR-029/030/031/034/035 sync +for spec conformance, pattern consistency, and correctness. This is the quality +checkpoint at the end of the core phase — before alknet-call (which depends on +the new `Identity.id = peer_id` semantics) begins its sync. + +### Review Checklist + +1. **PeerEntry / AuthPolicy conformance** (config.md, auth.md, ADR-030): + - `PeerEntry` has all 7 fields (peer_id, fingerprints, auth_token_hash, scopes, resources, display_name, enabled) + - `AuthPolicy.authorized_fingerprints` removed; `peers: Vec` in place + - `AuthPolicy.api_keys` unchanged + - `resolve_identity_from_fingerprint` resolves fingerprint → PeerEntry → `Identity { id: peer_id }` + - `resolve_identity_from_token` resolves auth_token_hash → PeerEntry → falls through to ApiKeyEntry + - `Identity.id` is the stable `peer_id`, not the fingerprint + - Disabled peers (`enabled: false`) return None + - Duplicate `peer_id` validation + +2. **ConfigIdentityProvider conformance** (auth.md, ADR-030): + - `resolve_from_fingerprint` delegates to `AuthPolicy::resolve_identity_from_fingerprint` + - `resolve_from_token` delegates to `AuthPolicy::resolve_identity_from_token` (PeerEntry first, ApiKeyEntry fall-through) + - Reads from ArcSwap on every call (hot-reloadable — unchanged) + - Does NOT implement `IdentityStore` + +3. **CredentialStore conformance** (auth.md, ADR-031/035): + - `CredentialStore` trait with sync `get`, async `put`/`delete` + - `InMemoryCredentialStore` default adapter (async put/delete with no .await points) + - `EncryptedData` core mirror (4 fields, serializable, no vault dep) + - `StoreError` enum (`#[non_exhaustive]`, thiserror, 3 variants) + - No `list` method + - No vault dependency added to core + +4. **IdentityStore conformance** (auth.md, ADR-035): + - `IdentityStore: IdentityProvider` supertrait + - `put_peer`/`update_peer`/`remove_peer` all async + - `ConfigIdentityProvider` does NOT implement it + - `IdentityProvider` trait unchanged (read-only, sync) + +5. **Fingerprint normalization conformance** (auth.md, ADR-030 §6): + - Ed25519 raw key (SPKI) → `ed25519:` + - X.509 cert → `SHA256:` (unchanged) + - iroh path → `ed25519:` (unchanged) + - Same key, same fingerprint across quinn and iroh + - No-client-cert → None (no regression) + +6. **Three remote roles documentation** (ADR-034): + - `auth.rs` comments document the three roles and verifier selection rule + - `endpoint.rs` comments clarify server-side vs client-side verifier concerns + +7. **Pattern consistency**: + - ArcSwap used consistently for DynamicConfig (unchanged) + - Repo/adapter pattern consistent (trait + in-memory default, no backend dep in core) + - No russh dependency in core (unchanged) + - Feature flags (quinn, iroh) gate transport code correctly + +8. **Security constraints**: + - `PeerEntry.enabled: false` → resolution returns None (revoked peers) + - `StoreError` is `#[non_exhaustive]` + - `EncryptedData` carries no plaintext (encrypted blob only) + - No env vars in the credential path (ADR-014 invariant preserved) + +9. **Test coverage**: + - PeerEntry resolution (fingerprint, auth_token_hash, ApiKeyEntry fall-through) + - Multi-fingerprint PeerEntry + - Disabled peer → None + - Duplicate peer_id validation + - CredentialStore get/put/delete round-trip + - EncryptedData serialization round-trip + - Fingerprint normalization (Ed25519 → ed25519:, X.509 → SHA256:) + - Config reload with PeerEntry model + +## Acceptance Criteria + +- [ ] All PeerEntry / AuthPolicy types match config.md and auth.md +- [ ] ConfigIdentityProvider resolution matches auth.md (PeerEntry multi-credential path) +- [ ] CredentialStore trait + InMemoryCredentialStore + EncryptedData + StoreError match ADR-031/035 +- [ ] IdentityStore trait matches ADR-035 (read/write split, ConfigIdentityProvider posture) +- [ ] Fingerprint normalization matches ADR-030 §6 (ed25519: for raw keys, SHA256: for X.509) +- [ ] Three remote roles documented in source comments (ADR-034) +- [ ] No `authorized_fingerprints` references remain +- [ ] No `remote_safe`/`trusted_peer` references in core (those are call-side) +- [ ] ArcSwap pattern consistent +- [ ] No russh dependency, no vault dependency in core +- [ ] Test coverage adequate for all new functionality +- [ ] `cargo fmt --check -p alknet-core` passes +- [ ] `cargo clippy -p alknet-core` passes with no warnings +- [ ] All tests pass + +## References + +- docs/architecture/crates/core/README.md +- docs/architecture/crates/core/auth.md +- docs/architecture/crates/core/config.md +- docs/architecture/decisions/030-peerentry-and-identity-id-decoupling.md +- docs/architecture/decisions/031-credentialstore-repo-trait.md +- docs/architecture/decisions/033-storage-boundary-and-repo-adapter-pattern.md +- docs/architecture/decisions/034-outgoing-only-x509-and-three-peer-roles.md +- docs/architecture/decisions/035-concrete-persistence-adapter-shapes.md + +## Notes + +> This review verifies core is spec-conformant after the ADR-029/030/031/034/035 +> sync before alknet-call begins its sync. alknet-call depends heavily on the new +> `Identity.id = peer_id` semantics (PeerCompositeEnv keys, PeerRef::Specific +> routing, AccessControl-based peer authorization) — any issues here propagate +> to call. If deviations are found, document and fix before proceeding to the +> call phase. + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/core/three-remote-roles-docs.md b/tasks/core/three-remote-roles-docs.md new file mode 100644 index 0000000..9e7c028 --- /dev/null +++ b/tasks/core/three-remote-roles-docs.md @@ -0,0 +1,85 @@ +--- +id: core/three-remote-roles-docs +name: Document the three remote roles and client-side verifier selection rule (ADR-034) +status: pending +depends_on: [core/peer-entry-model] +scope: single +risk: trivial +impact: isolated +level: implementation +--- + +## Description + +Update the in-code comments and doc comments in `alknet-core/src/auth.rs` and +`alknet-core/src/endpoint.rs` to document the three remote roles (ADR-034) and +the client-side verifier selection rule. This is a documentation/comment task — +the server-side endpoint code is unchanged; the client-side verifier selection +is a call-side task (`call/call-client-verifier-selection`). + +### Three remote roles (ADR-034 §1) + +| Role | Identity | alknet peer? | `PeerEntry` on local side? | +|------|----------|--------------|----------------------------| +| **Public X.509 endpoint** | Domain + CA-issued X.509 | No (local node is a client) | No | +| **Transport relay** (iroh's DERP-equivalent) | iroh `NodeId` (Ed25519) | No (infrastructure) | No | +| **Hub / hosting node** | Ed25519 raw key **and/or** X.509 | Yes (full peer) | Yes | + +`PeerEntry` (and the `PeerId` it resolves to) is the model for peers in the +call-protocol peer graph (ADR-029). A pure-client connection to a public X.509 +endpoint is **not** in that graph on the client side: no `PeerEntry`, no +`PeerId`, no `PeerRef::Specific` routing. + +### Client-side verifier selection rule (ADR-034 §3) + +| Local has `PeerEntry` for remote? | Remote cert type | Client verifier | +|----------------------------------|------------------|-----------------| +| No (public X.509 endpoint) | X.509 | `WebPkiServerVerifier` (CA verification) | +| No | Ed25519 raw key | fails closed (no CA to fall back to) | +| Yes (hub, Ed25519 path) | Ed25519 raw key | fingerprint match (`ed25519:`) | +| Yes (hub, X.509 path) | X.509 | fingerprint match (`SHA256:`) | + +### What to update + +1. **`auth.rs` doc comments**: add the three-roles table and the verifier + selection rule to the `Identity` / `PeerEntry` section doc comments, + referencing ADR-034. The `auth.md` spec already has this; mirror it in the + source comments. + +2. **`endpoint.rs` doc comments**: clarify that the server-side + `AcceptAnyCertVerifier` is "request-but-don't-require" mode for fingerprint + extraction (unchanged), and that the **client-side** verifier selection is + by `PeerEntry` presence (ADR-034 §3) — note that this is a `CallClient` + concern, not an endpoint concern. + +3. **No code changes** — this is comments/docs only. The server-side endpoint + is unchanged by ADR-034. The client-side verifier is + `call/call-client-verifier-selection`. + +## Acceptance Criteria + +- [ ] `auth.rs` doc comments document the three remote roles (ADR-034 §1) +- [ ] `auth.rs` doc comments document the client-side verifier selection rule (ADR-034 §3) +- [ ] `endpoint.rs` doc comments clarify server-side vs client-side verifier concerns +- [ ] Comments reference ADR-034 and `auth.md` +- [ ] No code changes (comments only) +- [ ] `cargo test -p alknet-core` succeeds (no regressions from comment changes) +- [ ] `cargo clippy -p alknet-core` succeeds with no warnings + +## References + +- docs/architecture/crates/core/auth.md — Three Remote Roles, Client-side verifier selection +- docs/architecture/decisions/034-outgoing-only-x509-and-three-peer-roles.md — ADR-034 + +## Notes + +> Documentation-only task to ensure the three-roles model and verifier selection +> rule are visible in the source, not just the specs. The server-side endpoint +> is unchanged by ADR-034; the client-side verifier selection is implemented in +> `call/call-client-verifier-selection`. Folding this into a standalone task +> keeps the fingerprint-normalization and resolution-logic tasks focused on +> code, not prose. + +## Summary + +> To be filled on completion \ No newline at end of file