feat: channels path carries the negotiation in the open op (L1, L3)
The channels path no longer carries a second negotiation frame on the channel's data stream (ADR-009). The open op's registry-validated input IS the negotiation: - producer: make_tty_open_handler parses the open op's input into a NegotiateRequest and drives the new drive_session_pre_negotiated (same three-pump driver as drive_session, minus the wire-frame negotiation phase; validate/allocate factored into validate_and_allocate, shared by both paths). Post-open failures (unknown backend, allocate_failed, ownership denial) still go to the client as a 0x00-prefixed negotiation error frame, so the consumer's M1 disambiguation read applies unchanged. The tty:open scope gate is enforced by the registry's AccessControl (not re-checked in the handler). - consumer: open_via_channels parses params locally (fail-fast before a channel is allocated), opens the channel, and starts raw-chunk mode directly (from_halves_raw — no negotiation write; the 0x00-error-frame peek retained). - tty_open_spec's input schema is now the partial NegotiateRequest shape (carriage/backend/cmd required; backend params stay free-form — raw JSON Schema is permissive on unknown keys). Prerequisites landed upstream: alkcall 0.4.0 enforces OperationSpec.input_schema at dispatch (the registry check this design leans on never existed before); alkcall 0.4.1 parks early-arrival chunks for un-adopted channels instead of dropping them — the open response / producer's-first-write race was silently losing the first chunks (found by L3's test; the session never resolved). L3: open_via_channels + from_bidi_stream_via now covered end-to-end (5 session tests + pre-negotiated adapter test + shared-harness tests in the new crate::testing module; the channels harness moved there so session tests share it). Verification: cargo test 93 lib (default), 116 lib + 19 integration (--all-features); clippy -D warnings native + wasm clean; fmt clean; doc 0 warnings; wasm check clean.
This commit is contained in:
1 parent
9327a73496
commit
96692d3b6a
8 files changed
+781
-243
No files matched your search
+65
-142
@@ -22,7 +22,7 @@
|
||||
//! `OperationSpec`'s `AccessControl` and enforced by the registry's
|
||||
//! `invoke`/`invoke_streaming` before the `TtyOpenHandler` runs — the
|
||||
//! handler itself only validates that the negotiated backend exists
|
||||
//! and spawns the protocol. This is the difference from the
|
||||
//! and spawns the session driver. This is the difference from the
|
||||
//! direct-ALPN `TtyAdapter`, which does its own ad-hoc scope check;
|
||||
//! the channels path gets ACL for free from alkcall's registry.
|
||||
//!
|
||||
@@ -41,8 +41,9 @@ use alkcall::registry::spec::{
|
||||
use serde_json::{json, Value};
|
||||
use tracing::debug;
|
||||
|
||||
use crate::adapter::{drive_session, TTY_OPEN_SCOPE};
|
||||
use crate::adapter::{drive_session_pre_negotiated, TTY_OPEN_SCOPE};
|
||||
use crate::backend::TtyBackend;
|
||||
use crate::negotiation::NegotiateRequest;
|
||||
|
||||
/// The per-ALPN open operation name (`channels/<alpn>/sub` convention,
|
||||
/// ADR-047). TTY is consumer-opens (the client requests a shell), so
|
||||
@@ -78,12 +79,13 @@ pub const TTY_ALPN: &str = "alk/tty";
|
||||
///
|
||||
/// `ownership` is the optional `OwnershipProvider` for the ADR-050
|
||||
/// resource-ownership check. `None` = scope-gate only. The provider
|
||||
/// is consulted inside `drive_session` (after the backend is selected
|
||||
/// and `resource_id` is extracted from `backend_params`), not by the
|
||||
/// channels wrapper — the wrapper's `AccessControl` carries only the
|
||||
/// scope gate. This mirrors the direct-ALPN `TtyAdapter` shape: the
|
||||
/// scope gate is enforced once at the entry point, the ownership
|
||||
/// check is enforced once at the backend-selection point.
|
||||
/// is consulted inside `drive_session_pre_negotiated` (after the
|
||||
/// backend is selected and `resource_id` is extracted from
|
||||
/// `backend_params`), not by the channels wrapper — the wrapper's
|
||||
/// `AccessControl` carries only the scope gate. This mirrors the
|
||||
/// direct-ALPN `TtyAdapter` shape: the scope gate is enforced once at
|
||||
/// the entry point (here by the registry's `AccessControl`), the
|
||||
/// ownership check is enforced once at the backend-selection point.
|
||||
///
|
||||
/// `auth` is the peer's `AuthContext`, captured at
|
||||
/// `install_channel_zero` time and closed over by the wrapper so the
|
||||
@@ -117,14 +119,18 @@ pub fn register_openable(
|
||||
/// a data channel. The `AccessControl` carries the `tty:open` scope
|
||||
/// gate (the same scope the direct-ALPN `TtyAdapter` checks).
|
||||
///
|
||||
/// The input schema is permissive (`type: object`) — the
|
||||
/// `NegotiateRequest` JSON shape is validated by `drive_session`'s
|
||||
/// negotiation reader, not by the registry's schema validator. This
|
||||
/// keeps the wire-format definition in one place (`wire.rs` +
|
||||
/// `negotiation.rs`) and avoids duplicating the schema in the
|
||||
/// `OperationSpec`. A future tightening could add the full
|
||||
/// `NegotiateRequest` JSON schema here; the permissive shape is the
|
||||
/// starting point.
|
||||
/// The input schema is the shared part of the `NegotiateRequest` shape
|
||||
/// (`carriage`/`backend`/`cmd` required — alkcall 0.4 enforces it at
|
||||
/// dispatch). The schema is deliberately partial: `tty`, `cwd`, `env`,
|
||||
/// and backend-specific selector fields are free-form (`additionalProperties`
|
||||
/// defaults to `true` in raw JSON Schema), because the
|
||||
/// `NegotiateRequest` wire shape is owned by `negotiation.rs` and the
|
||||
/// backend params are opaque (ADR-053) — duplicating their full schemas
|
||||
/// here would create a second definition to drift. The registry check
|
||||
/// catches structurally-broken opens (missing command, wrong types);
|
||||
/// the full parse and semantic validation (`carriage == "raw"`,
|
||||
/// backend lookup) runs in the handler via
|
||||
/// [`crate::adapter::drive_session_pre_negotiated`].
|
||||
pub fn tty_open_spec() -> OperationSpec {
|
||||
OperationSpec::new(
|
||||
OP_TTY_OPEN,
|
||||
@@ -162,21 +168,35 @@ pub fn tty_open_spec() -> OperationSpec {
|
||||
|
||||
/// Build the [`OpenHandler`] for `channels/tty/sub`.
|
||||
///
|
||||
/// The handler receives the open op's `input` (the `NegotiateRequest`
|
||||
/// params — ignored here; `drive_session` reads it from the channel's
|
||||
/// `BiStream` as the first frame, same as the direct-ALPN path), the
|
||||
/// channel's [`Connection`] (data-plane ALPN `alk/tty`), and the
|
||||
/// peer's [`AuthContext`]. It calls `accept_bi()` to get the channel's
|
||||
/// [`BiStream`], splits it into read/write halves (the stdlib
|
||||
/// `tokio::io::split` idiom — the same split the direct-ALPN
|
||||
/// `TtyAdapter::handle` does), and runs [`drive_session`] on them.
|
||||
/// The handler receives the open op's `input` — the `NegotiateRequest`
|
||||
/// params, validated against the spec's input schema by the registry
|
||||
/// (alkcall 0.4: `input_schema` is enforced at dispatch) and passed
|
||||
/// through as the authoritative negotiation (ADR-009: the channels
|
||||
/// path does not carry a second negotiation frame on the channel; the
|
||||
/// open op's `input` *is* the negotiation). The channel's
|
||||
/// [`Connection`] (data-plane ALPN `alk/tty`) and the peer's
|
||||
/// [`AuthContext`] come with it. The handler parses `input`, calls
|
||||
/// `accept_bi()` to get the channel's `BiStream`, splits it into
|
||||
/// read/write halves (the stdlib `tokio::io::split` idiom — the same
|
||||
/// split the direct-ALPN `TtyAdapter::handle` does), and runs
|
||||
/// [`crate::adapter::drive_session_pre_negotiated`] on them — the same
|
||||
/// three-pump session driver as the direct-ALPN path, minus the
|
||||
/// wire-frame negotiation phase.
|
||||
///
|
||||
/// Parse or validation failures (unknown backend, `allocate_failed`,
|
||||
/// ADR-050 ownership denial) are reported to the client as a
|
||||
/// negotiation error frame on the channel stream (the same
|
||||
/// `0x00`-prefixed framing the direct path uses), so a consumer's
|
||||
/// negotiation-error disambiguation read applies unchanged. The
|
||||
/// `tty:open` scope gate is enforced by the registry's `AccessControl`
|
||||
/// before this handler runs — the handler does not re-check it.
|
||||
///
|
||||
/// The handler's `JoinHandle` is recorded by the channels wrapper for
|
||||
/// teardown (abort on `channel/close` / connection drop). When the
|
||||
/// session ends (exit chunk sent, stream closed, or stream reset),
|
||||
/// `drive_session` returns and the spawned task completes; the
|
||||
/// wrapper's teardown task then calls `manager.teardown_channel` and
|
||||
/// `policy.on_close` (ADR-047 §7).
|
||||
/// `drive_session_pre_negotiated` returns and the spawned task
|
||||
/// completes; the wrapper's teardown task then calls
|
||||
/// `manager.teardown_channel` and `policy.on_close` (ADR-047 §7).
|
||||
fn make_tty_open_handler(
|
||||
backends: Arc<HashMap<String, Arc<dyn TtyBackend>>>,
|
||||
ownership: Option<Arc<dyn OwnershipProvider>>,
|
||||
@@ -187,9 +207,15 @@ fn make_tty_open_handler(
|
||||
let backends = Arc::clone(&backends);
|
||||
let ownership = ownership.clone();
|
||||
let identity = identity.clone().or_else(|| auth.identity.clone());
|
||||
let _ = input;
|
||||
|
||||
tokio::spawn(async move {
|
||||
let req: NegotiateRequest = match serde_json::from_value(input) {
|
||||
Ok(r) => r,
|
||||
Err(e) => {
|
||||
debug!("tty: channels open: invalid negotiate params: {e}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let stream = match channel_conn.accept_bi().await {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
@@ -198,7 +224,15 @@ fn make_tty_open_handler(
|
||||
}
|
||||
};
|
||||
let (client_read, client_write) = tokio::io::split(stream);
|
||||
drive_session(client_write, client_read, backends, ownership, identity).await;
|
||||
drive_session_pre_negotiated(
|
||||
client_write,
|
||||
client_read,
|
||||
req,
|
||||
backends,
|
||||
ownership,
|
||||
identity,
|
||||
)
|
||||
.await;
|
||||
})
|
||||
},
|
||||
)
|
||||
@@ -208,125 +242,13 @@ fn make_tty_open_handler(
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::backend::{MockBackend, TtyError};
|
||||
use alkcall::channels::client::ChannelClient;
|
||||
use crate::testing::wire_client_and_server;
|
||||
use alkcall::channels::operations::ChannelCore;
|
||||
use alkcall::channels::policy::default_policy;
|
||||
use alkcall::core::auth::Identity;
|
||||
use alkcall::core::types::Connection as CoreConnection;
|
||||
use alkcall::registry::registration::OperationRegistry;
|
||||
use std::collections::HashMap as StdHashMap;
|
||||
use tokio::io::duplex;
|
||||
|
||||
/// Build a `ChannelClient` and a server-side `ChannelCore` + registry
|
||||
/// with `channels/tty/sub` registered, wired over a `tokio::io::duplex`
|
||||
/// carrying the channels 8-byte chunk header wire format. Returns the
|
||||
/// client and the per-connection `ChannelCore` (for any teardown
|
||||
/// assertions). The server-side dispatch loop is spawned and runs
|
||||
/// until the client drops or the test ends.
|
||||
///
|
||||
/// This mirrors the alkcall `channel_0_end_to_end_register_openable`
|
||||
/// test's wiring pattern but uses alktty's `register_openable` helper
|
||||
/// so the registration is the code under test.
|
||||
async fn wire_client_and_server(
|
||||
backends: Arc<HashMap<String, Arc<dyn TtyBackend>>>,
|
||||
ownership: Option<Arc<dyn OwnershipProvider>>,
|
||||
identity: Option<Identity>,
|
||||
) -> ChannelClient {
|
||||
use alkcall::channels::adapter::{ChannelsAdapter, InstallChannelZero};
|
||||
use alkcall::core::auth::IdentityProvider;
|
||||
use alkcall::protocol::connection::split_single_stream;
|
||||
use alkcall::protocol::dispatch::Dispatcher;
|
||||
|
||||
struct NoopIdProvider;
|
||||
impl IdentityProvider for NoopIdProvider {
|
||||
fn resolve_from_fingerprint(&self, _: &str) -> Option<alkcall::core::auth::Identity> {
|
||||
None
|
||||
}
|
||||
fn resolve_from_token(
|
||||
&self,
|
||||
_: &alkcall::core::auth::AuthToken,
|
||||
) -> Option<alkcall::core::auth::Identity> {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
let policy = default_policy();
|
||||
let policy_for_hook = Arc::clone(&policy);
|
||||
let identity_for_conn = identity.clone();
|
||||
|
||||
let install_hook: InstallChannelZero = Arc::new(move |manager, channel0_conn, auth| {
|
||||
let backends = Arc::clone(&backends);
|
||||
let ownership = ownership.clone();
|
||||
let _identity = identity.clone();
|
||||
let policy = Arc::clone(&policy_for_hook);
|
||||
tokio::spawn(async move {
|
||||
let channel0_bidi = match channel0_conn.accept_bi().await {
|
||||
Ok(s) => s,
|
||||
Err(_) => return,
|
||||
};
|
||||
let (writer, reader) = split_single_stream(channel0_bidi);
|
||||
|
||||
// Propagate the identity to the channel-0
|
||||
// connection so the call dispatch's
|
||||
// `resolve_identity` sees it. The
|
||||
// `ChannelsAdapter` builds `channel0_conn` fresh
|
||||
// from `channel_source` and does NOT inherit the
|
||||
// outer connection's identity — we set it here
|
||||
// so the `AccessControl` scope-gate can check it.
|
||||
if let Some(id) = _identity {
|
||||
let _ = channel0_conn.set_identity(id);
|
||||
}
|
||||
|
||||
let core = ChannelCore::new(manager, policy);
|
||||
let mut registry = OperationRegistry::new();
|
||||
register_openable(
|
||||
&core,
|
||||
Arc::clone(&backends),
|
||||
ownership.clone(),
|
||||
&mut registry,
|
||||
auth.clone(),
|
||||
)
|
||||
.expect("register_openable");
|
||||
let registry = Arc::new(registry);
|
||||
let provider: Arc<dyn IdentityProvider> = Arc::new(NoopIdProvider);
|
||||
let call_connection = Arc::new(
|
||||
alkcall::protocol::connection::CallConnection::new_single_stream(
|
||||
channel0_conn,
|
||||
Arc::clone(&writer),
|
||||
),
|
||||
);
|
||||
let dp = Dispatcher::new(registry, provider);
|
||||
dp.run_loop_single_stream(call_connection, reader, writer)
|
||||
.await;
|
||||
})
|
||||
});
|
||||
|
||||
let (client_end, server_end) = duplex(64 * 1024);
|
||||
let client_conn = CoreConnection::from_bidi(client_end, b"alk/channels".to_vec(), None);
|
||||
let server_conn = CoreConnection::from_bidi(server_end, b"alk/channels".to_vec(), None);
|
||||
// Set the identity on the server connection so the call
|
||||
// dispatch sees it (the ACL check runs against the
|
||||
// connection's identity, not the `install_channel_zero`
|
||||
// hook's `auth`).
|
||||
if let Some(id) = &identity_for_conn {
|
||||
let _ = server_conn.set_identity(id.clone());
|
||||
}
|
||||
|
||||
let adapter = ChannelsAdapter::new(install_hook, Arc::new(NoCap));
|
||||
let auth = AuthContext::anonymous(b"alk/channels");
|
||||
let _server_handle = tokio::spawn(async move {
|
||||
let _ =
|
||||
alkcall::core::types::ProtocolHandler::handle(&adapter, server_conn, &auth).await;
|
||||
});
|
||||
|
||||
ChannelClient::from_connection(client_conn)
|
||||
.await
|
||||
.expect("channel client init")
|
||||
}
|
||||
|
||||
/// A `ChannelLifecyclePolicy` with no cap (for tests).
|
||||
use alkcall::channels::policy::NoCap;
|
||||
|
||||
#[test]
|
||||
fn op_tty_open_is_channels_tty_sub() {
|
||||
assert_eq!(OP_TTY_OPEN, "channels/tty/sub");
|
||||
@@ -372,6 +294,7 @@ mod tests {
|
||||
/// must be discoverable by name afterwards.
|
||||
#[tokio::test]
|
||||
async fn register_openable_registers_op_on_registry() {
|
||||
use alkcall::channels::policy::default_policy;
|
||||
let (_client, server) = duplex(1024);
|
||||
let (_reader, writer) = tokio::io::split(server);
|
||||
let (handle, _runner) = alkcall::channels::mux::MuxRunner::new(Box::new(writer));
|
||||
|
||||
Reference in new issue
Block a user