docs(architecture): resolve review #002 Tiers 1-3 — mechanical and consistency fixes
Governance (Tier 2):
- Advance ADR-022 and ADR-023 from Proposed to Accepted (specs already
depend on their types as source of truth)
- Amend ADR-015: mark Decision 3 and Assumption 6 as superseded by ADR-022;
update handler_identity type to CompositionAuthority
- Amend ADR-002: note handle() signature revised by ADR-007 (BiStream → Connection)
- Amend ADR-004: note 'enrich/replace' AuthContext language superseded by
ADR-011's immutability model; update to describe set_identity on Connection
- Update main README ADR table to show ADR-022/023 as Accepted
Spec-ADR consistency (Tier 3):
- Add abort_policy: AbortPolicy field to OperationContext struct (ADR-016
Decision 6 mandated this but the spec omitted it)
- Define AbortPolicy enum (AbortDependents | ContinueRunning) with Default impl
- Add abort_policy to build_root_context and LocalOperationEnv::invoke()
- Define the OperationEnv trait explicitly with invoke() and
invoke_with_policy() methods (was referenced as 'must remain a trait'
but never defined)
- Specify From<StreamError> for HandlerError impl with exact variant mapping
- Add Connection::from_quinn() / from_iroh() constructors (was referenced
as Connection::new() but never defined)
- Remove undefined CertAuthorityEntry placeholder from AuthPolicy v1 (will
be added additively when alknet-ssh lands)
- Fix config.md key-differences table: rate limits are in DynamicConfig,
not StaticConfig
Mechanical fixes (Tier 1):
- overview.md: 'closes the QUIC stream' → 'closes the connection' (stale
from pre-ADR-007 model)
- overview.md: OQ-04 entry updated from stale 'defer to implementation'
to 'resolved: static at startup'
- mnemonic-derivation.md: remove duplicate helper functions block (incomplete
first copy, complete second copy)
- ADR-003: add iroh (feature-gated) to alknet-core dependency list, added
by ADR-010
- ADR-021: fix ambiguous 'W1 drift issue from the vault review' cross-reference
- ADR-022: rephrase FromCall 'leaf locally' to 'leaf in the local registry'
- ADR-017: add error_schemas to from_call mirror list and services/schema
step (inconsistency with ADR-023)
- ADR-016: fix self-referential citation ('ADR-016 Assumption 5' → 'Assumption 5')
- Add ScopedOperationEnv::empty(), allows(), new() and
CompositionAuthority::none(), new() impl blocks (referenced but undefined)
- Add call.completed clarification for non-subscription calls
- Add services/schema leading-slash normalization note
- Crate README ADR tables: add missing ADR-013 (call), ADR-015 (core),
ADR-006 + ADR-010 (vault)
- Vault README: add consolidated 'Known Source Drift' table tracking all
four drift items (OsRng, unwrap, CURRENT_KEY_VERSION, spawn bug) in one
place, including the two previously missing from README
This commit is contained in:
@@ -16,19 +16,27 @@ iroh's `ProtocolHandler` trait demonstrates this: it takes a bidirectional QUIC
|
||||
|
||||
A single `ProtocolHandler` trait replaces both `StreamInterface` and `MessageInterface`:
|
||||
|
||||
> **Note**: The signature below was revised by ADR-007. The `handle()` method
|
||||
> now receives a `Connection` (not a `BiStream`) — see ADR-007 for the
|
||||
> current authoritative signature. The original signature is retained here
|
||||
> for historical context.
|
||||
|
||||
```rust
|
||||
#[async_trait]
|
||||
pub trait ProtocolHandler: Send + Sync + 'static {
|
||||
/// The ALPN string this handler claims (e.g. b"alknet/ssh")
|
||||
fn alpn(&self) -> &'static [u8];
|
||||
|
||||
/// Handle an incoming bidirectional QUIC stream
|
||||
async fn handle(&self, stream: BiStream, auth: &AuthContext) -> Result<(), HandlerError>;
|
||||
/// Handle an incoming connection (revised by ADR-007 to receive
|
||||
/// `Connection` instead of `BiStream`)
|
||||
async fn handle(&self, connection: Connection, auth: &AuthContext) -> Result<(), HandlerError>;
|
||||
}
|
||||
```
|
||||
|
||||
- `alpn()` returns a static byte string — the handler's ALPN identifier
|
||||
- `handle()` receives a `BiStream` (a joined `(SendStream, RecvStream)` implementing `AsyncRead + AsyncWrite`) and an `AuthContext` carrying the authenticated identity, and returns `HandlerError` on failure
|
||||
- `handle()` receives a `Connection` (revised by ADR-007 from the original
|
||||
`BiStream`) and an `AuthContext` carrying the authenticated identity, and
|
||||
returns `HandlerError` on failure
|
||||
- Every handler manages its own wire format — no shared framing, no StreamInterface/MessageInterface split
|
||||
- The `ListenerConfig` enum is eliminated — ALPN advertisement configuration replaces it
|
||||
|
||||
|
||||
@@ -24,7 +24,7 @@ The workspace decomposes into the following crates:
|
||||
|
||||
| Crate | Responsibility | Depends on |
|
||||
|-------|---------------|------------|
|
||||
| `alknet-core` | ProtocolHandler trait, ALPN router, endpoint, BiStream, AuthContext, IdentityProvider, config, ArcSwap dynamic config | tokio, quinn, rustls, irpc |
|
||||
| `alknet-core` | ProtocolHandler trait, ALPN router, endpoint, BiStream, AuthContext, IdentityProvider, config, ArcSwap dynamic config | tokio, quinn, rustls, irpc, iroh (feature-gated, added by ADR-010) |
|
||||
| `alknet-vault` | Local key vault: BIP39/SLIP-0010/AES-GCM key derivation, encryption, VaultProtocol dispatch | (standalone, no alknet-core) |
|
||||
| `alknet-ssh` | SshAdapter (russh, SOCKS5, port forwarding) | alknet-core, russh |
|
||||
| `alknet-call` | CallAdapter (JSON-RPC via irpc, operation registry, pub/sub, access control, call protocol client, adapter traits) | alknet-core, irpc |
|
||||
|
||||
@@ -12,6 +12,13 @@ The ALPN dispatch model simplifies this: every handler receives the same `AuthCo
|
||||
|
||||
## Decision
|
||||
|
||||
> **Note**: The original text of this decision described the handler
|
||||
> "enriching or replacing" the `AuthContext`. This was superseded by
|
||||
> ADR-011, which made `AuthContext` immutable in `handle()` (passed as
|
||||
> `&AuthContext`). Handlers resolve identity into a local variable and
|
||||
> store it on `Connection` via `set_identity()`. The text below has been
|
||||
> updated to reflect the ADR-011 model.
|
||||
|
||||
Authentication and identity resolution live in `alknet-core` as shared infrastructure. Each handler presents credentials differently, but all resolve through the same `IdentityProvider`:
|
||||
|
||||
```rust
|
||||
@@ -36,7 +43,7 @@ Auth resolution is **hybrid** — the endpoint resolves what it can, and handler
|
||||
|
||||
1. **Endpoint-level resolution** (before `handle()` is called): If the TLS handshake provides a client certificate, the endpoint resolves the fingerprint to an `Identity` and passes it in `AuthContext`. This is the case for SSH (where the key exchange happens at the protocol level, but the TLS layer may also provide information).
|
||||
|
||||
2. **Handler-level resolution** (inside `handle()`): For protocols that carry credentials in application frames (AuthToken in the first call frame, Bearer header in HTTP), the handler extracts the credential from the stream and calls `IdentityProvider` to resolve it. The handler then enriches or replaces the partial `AuthContext` with the fully resolved `Identity`.
|
||||
2. **Handler-level resolution** (inside `handle()`): For protocols that carry credentials in application frames (AuthToken in the first call frame, Bearer header in HTTP), the handler extracts the credential from the stream and calls `IdentityProvider` to resolve it. The handler then resolves the `Identity` into a local variable and stores it on the `Connection` via `set_identity()` for observability — it does **not** mutate the `AuthContext` (which is passed as `&AuthContext`, an immutable reference — see ADR-011). The per-request identity (for ACL) is resolved separately by the `CallAdapter` at `call.requested` time.
|
||||
|
||||
The `AuthContext` passed to `handle()` may be partial — containing only transport-level information if no TLS client certificate was provided. Handlers must not assume `AuthContext` contains a fully resolved `Identity`. Each handler knows its own credential extraction protocol and is responsible for completing authentication.
|
||||
|
||||
|
||||
@@ -114,7 +114,7 @@ fn dispatch(connection) {
|
||||
match handler {
|
||||
Some(h) => {
|
||||
auth = AuthContext::from_connection(&connection)
|
||||
conn = Connection::new(connection)
|
||||
conn = Connection::from_quinn(connection) // or from_iroh
|
||||
tokio::spawn(h.handle(conn, &auth))
|
||||
}
|
||||
None => connection.close()
|
||||
|
||||
@@ -113,15 +113,23 @@ enumerate the internal call tree.
|
||||
|
||||
### 3. Handler identity is carried on OperationContext
|
||||
|
||||
`OperationContext` carries both the caller's identity (who invoked me) and the
|
||||
handler's identity (who am I acting as):
|
||||
> **Note**: This decision's `handler_identity: Option<Identity>` type was
|
||||
> superseded by ADR-022, which replaced `Identity` with
|
||||
> `CompositionAuthority` — a declared authority bundle that is not a peer
|
||||
> identity and is not resolvable through `IdentityProvider`. The core
|
||||
> decision (authority switch, not ACL skip) holds unchanged. See ADR-022
|
||||
> Decision 2 for the current type.
|
||||
|
||||
`OperationContext` carries both the caller's identity (who invoked me) and
|
||||
the handler's identity (who am I acting as):
|
||||
|
||||
```rust
|
||||
pub struct OperationContext {
|
||||
pub request_id: String,
|
||||
pub parent_request_id: Option<String>,
|
||||
pub identity: Option<Identity>, // Caller's identity (inbound)
|
||||
pub handler_identity: Option<Identity>, // Handler's identity (composition authority)
|
||||
// Type changed to Option<CompositionAuthority> by ADR-022:
|
||||
pub handler_identity: Option<CompositionAuthority>, // Handler's composition authority
|
||||
pub capabilities: Capabilities,
|
||||
pub metadata: HashMap<String, Value>,
|
||||
pub env: OperationEnv,
|
||||
@@ -273,11 +281,14 @@ Principle of least privilege.
|
||||
is more important than debuggability from the wire.
|
||||
|
||||
6. **The handler identity is a full `Identity` (with scopes), not a special
|
||||
principal type.** This reuses the existing `Identity` type and `IdentityProvider`
|
||||
infrastructure (ADR-004). If handler identities need different resolution
|
||||
semantics (e.g., not resolvable through `IdentityProvider`), a separate type
|
||||
may be needed. The assumption is that the existing identity infrastructure
|
||||
suffices.
|
||||
principal type.** ~~This reuses the existing `Identity` type and
|
||||
`IdentityProvider` infrastructure (ADR-004).~~ **Superseded by ADR-022
|
||||
Decision 2**: composition authority is a declared authority bundle
|
||||
(`CompositionAuthority`), not a peer `Identity`. It is not resolvable
|
||||
through `IdentityProvider` and does not represent an inbound caller. The
|
||||
distinction is necessary because a handler is not a network peer — its
|
||||
authority is declared by the assembly layer at registration, not resolved
|
||||
from credentials.
|
||||
|
||||
## References
|
||||
|
||||
|
||||
@@ -131,7 +131,7 @@ abort — the handler that composes the child knows that, not the wire caller.
|
||||
Putting the policy on the wire payload would give the wire caller control
|
||||
over internal composition behavior it can't see.
|
||||
|
||||
**Why not per-operation declaration**: ADR-016 Assumption 5 says the policy
|
||||
**Why not per-operation declaration**: Assumption 5 says the policy
|
||||
is per-call, not per-operation. The same operation may need
|
||||
`abort-dependents` in one composition context and `continue-running` in
|
||||
another. A static property on `OperationSpec` can't express that.
|
||||
|
||||
@@ -113,10 +113,11 @@ pub async fn from_call(
|
||||
The adapter:
|
||||
1. Calls `services/list` on the remote node → gets the list of `External`
|
||||
operations
|
||||
2. Calls `services/schema` for each → gets the input/output JSON Schemas
|
||||
2. Calls `services/schema` for each → gets the input/output JSON Schemas and
|
||||
declared error_schemas (ADR-023)
|
||||
3. For each discovered operation, constructs an `(OperationSpec, Handler)` pair:
|
||||
- The spec mirrors the remote operation's name, namespace, type, schemas,
|
||||
and access control
|
||||
- The spec mirrors the remote operation's name, namespace, type, schemas
|
||||
(input, output, and error_schemas — ADR-023), and access control
|
||||
- The handler sends `call.requested` through the `CallConnection` and awaits
|
||||
`call.responded` (or streams for subscriptions)
|
||||
4. The caller registers these pairs in their local registry
|
||||
@@ -272,8 +273,8 @@ same as `from_openapi` receives HTTP credentials.
|
||||
remote call failures are handled the same as local handler failures.
|
||||
|
||||
4. **`from_call`-registered operations mirror the remote spec.** The imported
|
||||
`OperationSpec` has the same name, namespace, type, schemas, and access
|
||||
control as the remote operation. If the remote operation changes (new
|
||||
`OperationSpec` has the same name, namespace, type, schemas (input, output,
|
||||
and error_schemas per ADR-023), and access control as the remote operation. If the remote operation changes (new
|
||||
schema, renamed), the imported spec is stale until re-import. The
|
||||
assumption is that re-import happens on reconnection or is triggered
|
||||
explicitly. Hot-swapping imported specs is a two-way door.
|
||||
|
||||
@@ -123,9 +123,9 @@ impl VaultServiceHandle {
|
||||
```
|
||||
|
||||
`decrypt` now derives the key at the path **indicated by
|
||||
`encrypted.key_version`** — not always at `PATHS::ENCRYPTION`. This is
|
||||
the fix for the W1 drift issue from the vault review: the current source
|
||||
ignores `key_version` for key selection; the spec now makes it functional.
|
||||
`encrypted.key_version`** — not always at `PATHS::ENCRYPTION`. This corrects
|
||||
a source drift: the current source ignores `key_version` for key selection;
|
||||
the spec now makes it functional.
|
||||
|
||||
### 3. `rotate` method
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
## Status
|
||||
|
||||
Proposed
|
||||
Accepted
|
||||
|
||||
## Context
|
||||
|
||||
@@ -120,7 +120,8 @@ pub enum OperationProvenance {
|
||||
FromOpenAPI,
|
||||
/// MCP forwarding stub (from_mcp), leaf — cannot compose.
|
||||
FromMCP,
|
||||
/// QUIC forwarding stub (from_call), leaf locally — cannot compose.
|
||||
/// QUIC forwarding stub (from_call). Leaf in the local registry —
|
||||
/// forwards calls to a remote node; cannot compose locally.
|
||||
FromCall,
|
||||
/// JSON Schema definition (from_jsonschema), no handler — schema only.
|
||||
FromJsonSchema,
|
||||
@@ -134,7 +135,7 @@ pub enum OperationProvenance {
|
||||
| `Local` | Yes | Yes — scopes set by assembly layer | External or Internal (assembly declares) | Trusted code |
|
||||
| `FromOpenAPI` | No (leaf) | No | Internal | HTTP endpoint trusted; handler is a forwarding stub |
|
||||
| `FromMCP` | No (leaf) | No | Internal | MCP server trusted; handler is a forwarding stub |
|
||||
| `FromCall` | No (leaf locally) | No | Internal | Remote node trusted; handler is a forwarding stub |
|
||||
| `FromCall` | No (leaf in local registry) | No | Internal | Remote node trusted; handler is a forwarding stub |
|
||||
| `FromJsonSchema` | N/A (no handler) | No | N/A | N/A |
|
||||
| `Session` | Yes (within sandbox) | Yes — scopes set by assembly layer at sandbox creation | Internal always | Untrusted code in sandbox |
|
||||
|
||||
@@ -178,6 +179,24 @@ pub struct CompositionAuthority {
|
||||
/// handler can reach in composition.
|
||||
pub resources: HashMap<String, Vec<String>>,
|
||||
}
|
||||
|
||||
impl CompositionAuthority {
|
||||
/// `None` — for leaves that don't compose (convenience for
|
||||
/// `composition_authority: CompositionAuthority::none()`).
|
||||
pub fn none() -> Option<Self> { None }
|
||||
|
||||
/// Construct a composition authority with the given label and scopes.
|
||||
pub fn new(
|
||||
label: &str,
|
||||
scopes: impl IntoIterator<Item = String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
label: label.to_string(),
|
||||
scopes: scopes.into_iter().collect(),
|
||||
resources: HashMap::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This supersedes ADR-015's Assumption 6. ADR-015's core decision (authority
|
||||
@@ -212,6 +231,23 @@ pub struct ScopedOperationEnv {
|
||||
/// parameterized-dispatch attack surface.
|
||||
pub allowed_operations: HashSet<String>,
|
||||
}
|
||||
|
||||
impl ScopedOperationEnv {
|
||||
/// Empty set — for leaves that don't compose (no reachable operations).
|
||||
pub fn empty() -> Self {
|
||||
Self { allowed_operations: HashSet::new() }
|
||||
}
|
||||
|
||||
/// Construct from an iterable of operation names.
|
||||
pub fn new(ops: impl IntoIterator<Item = String>) -> Self {
|
||||
Self { allowed_operations: ops.into_iter().collect() }
|
||||
}
|
||||
|
||||
/// Returns true if the given operation name is reachable.
|
||||
pub fn allows(&self, name: &str) -> bool {
|
||||
self.allowed_operations.contains(name)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### 4. The registration bundle carries all three
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
## Status
|
||||
|
||||
Proposed
|
||||
Accepted
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
Reference in New Issue
Block a user