Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,27 @@ EXPERIMENTAL, so on-disk formats and the CLI may change without notice.

## [Unreleased]

### Fixed documentation

- **The detached replay buffer is capped, and the retention docs said it was
not.** They claimed the buffer had no cap of its own, that a runaway remote
shell would retain roughly 17 MB across a 15 minute window, and that the
detached TTL was therefore the only backstop against such a process. Measured
directly, all three were wrong.

A detached session is never acknowledged, so its reader waits for buffer space
that never frees and stops at `MAX_BUFFERED_BYTES`, currently 4 MiB. A runaway
producer pins at exactly 4 MiB and stays there while the remote process blocks
writing to its own PTY. Retention is bounded per session no matter how long
the window is, and in aggregate by the server session cap — 256 MiB at the
default of 64 sessions.

Backpressure is the backstop against a runaway remote process; the TTL bounds
how long a session lives, not how much it holds. This matters beyond accuracy:
the "no cap" claim was the stated reason not to raise the detached TTL
further, and that reason does not hold. A test now pins the bound so the claim
cannot drift back.

### Added

- **Durable connection telemetry.** `fabric status` now reports, per peer, how
Expand Down Expand Up @@ -84,14 +105,11 @@ EXPERIMENTAL, so on-disk formats and the CLI may change without notice.
shell now survives a closed lid over lunch; previously it did not survive a
coffee break. The number comes from measured cost rather than taste: an idle
detached session buffers 0 bytes across a full window, so holding one costs a
session struct and a PTY process and nothing that grows with time, while a
session still producing output grows at whatever the remote writes — about
19 KB/s for a pathological loop, roughly 17 MB over this window. Past the
window the behaviour is unchanged and already proven: the client reports
`remote shell could not resume`, names the expired session, and exits
non-zero. This TTL remains the only backstop against a runaway remote process,
because the replay buffer has no cap of its own, so raising it much further
wants that cap first.
session struct and a PTY process and nothing that grows with time. A session
still producing output is bounded too: the replay buffer stops at 4 MiB, so
retention does not grow with the length of the window. Past the window the
behaviour is unchanged and already proven: the client reports `remote shell
could not resume`, names the expired session, and exits non-zero.

- **Release archives have an enforced one-file contract.** Each platform tarball
must contain exactly one member named literal `fabric` (not `./fabric`), and
Expand Down
23 changes: 17 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1348,12 +1348,23 @@ the PTY.
The window is 15 minutes because that is what the cost measures out to, not as a
round guess. An idle detached shell buffers nothing — 0 bytes across a full
detached window — so holding one costs a session struct and a PTY process and
nothing that grows with time. A session still producing output is the expensive
case, growing at whatever the remote writes; a pathological output loop measured
about 19 KB/s, so roughly 17 MB over this window for one runaway shell. Note
that this TTL is currently the only backstop against such a process, because the
replay buffer has no cap of its own, which is why raising it much further wants
that cap first.
nothing that grows with time.

A session still producing output is the expensive case, and it is bounded too.
The replay buffer stops at 4 MiB. Nothing acknowledges a detached session, so
the reader waits for buffer space that never frees and the remote process then
blocks writing to its own PTY. Measured directly, a runaway producer pins at
exactly 4 MiB and stays there. Retention is therefore bounded per session
regardless of how long this window is, and in aggregate by the server session
cap — 256 MiB at the default of 64 sessions.

Backpressure, not this TTL, is what bounds a runaway remote process. The TTL
bounds how long a session lives, not how much it holds.

> An earlier version of this section said the replay buffer had no cap, that a
> runaway shell would reach roughly 17 MB across this window, and that the TTL
> was the only backstop. All three were wrong. The "no cap" claim was also the
> stated reason not to raise the TTL further, so that reason no longer applies.

Past the window, the client does not retry a session the server has already
refused: it reports `remote shell could not resume`, names the expired session,
Expand Down
21 changes: 15 additions & 6 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,25 @@ pub const DEFAULT_SERVER_SESSION_MAX_PER_PEER: usize = 16;
/// longer window is bounded by what a detached session actually retains: an idle
/// shell buffers nothing at all, measured at 0 bytes across a full detached
/// window, so holding it costs a session struct and a PTY process and nothing
/// that grows with time. A session still producing output is the expensive case
/// and it grows at whatever the remote writes, measured at about 19 KB/s for a
/// pathological loop, so roughly 17 MB over this window for one runaway shell.
/// that grows with time.
///
/// The benefit is the case that actually happens: a closed laptop lid over lunch
/// keeps its shell. Sixty seconds did not survive a coffee break.
///
/// This is the only backstop against a runaway remote process, because the
/// server's replay buffer has no cap of its own, so it should not be raised much
/// further without capping that buffer first.
/// A session still producing output is the expensive case, and it is bounded
/// too. The tunnel replay buffer stops at its own cap, currently 4 MiB:
/// nothing ACKs a detached session, the reader waits for buffer space
/// that never frees, and the remote process then blocks on its own PTY write.
/// Measured directly, a runaway producer pins at exactly 4 MiB and stays there.
/// So retention is bounded per session no matter how long this window is, and in
/// aggregate by `DEFAULT_SERVER_SESSION_MAX_TOTAL`, which is 256 MiB at the
/// defaults.
///
/// This doc previously said the buffer had no cap, that a runaway shell would
/// reach roughly 17 MB across this window, and that this TTL was the only
/// backstop. All three were wrong, and the first was the stated reason not to
/// raise this value further. Backpressure is the backstop against a runaway
/// process; this TTL bounds how long a session lives, not how much it holds.
pub const DEFAULT_SERVER_SESSION_DETACHED_TTL_SECS: u64 = 15 * 60;

#[derive(Debug, Clone)]
Expand Down
66 changes: 66 additions & 0 deletions src/tunnel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2084,6 +2084,72 @@ mod tests {
assert!(kill.is_cancelled());
}

/// A producer that never stops, standing in for a runaway remote process.
struct EndlessProducer;

impl AsyncRead for EndlessProducer {
fn poll_read(
self: std::pin::Pin<&mut Self>,
_cx: &mut std::task::Context<'_>,
buf: &mut tokio::io::ReadBuf<'_>,
) -> std::task::Poll<std::io::Result<()>> {
let n = buf.remaining().min(8192);
buf.put_slice(&vec![b'x'; n]);
std::task::Poll::Ready(Ok(()))
}
}

/// A detached session's replay buffer is bounded, and the bound is the real
/// backstop against a runaway remote process.
///
/// The retention docs used to say this buffer had no cap of its own, that a
/// runaway shell would reach roughly 17 MB across a 15 minute window, and
/// that the detached TTL was therefore the only backstop. Measured, all
/// three are wrong. The reader waits for buffer space, nothing ACKs a
/// detached session, so it stops at exactly MAX_BUFFERED_BYTES and the
/// remote process blocks on its own PTY write instead. Backpressure is the
/// backstop; the TTL bounds how long a session lives, not how much it holds.
///
/// This matters beyond tidiness: "no cap" was the stated reason not to raise
/// the detached TTL further.
#[tokio::test]
async fn a_detached_replay_buffer_stops_at_the_cap_instead_of_growing() {
let (_write_peer, write) = duplex(64);
let (session, local_read) = TunnelSession::new_parts(
session_id(77),
peer_id(),
Box::new(EndlessProducer),
Box::new(write),
);
// Never attached, so nothing ever ACKs. That is the worst case for
// retention and exactly the detached-session case being bounded here.
tokio::spawn(session.clone().run_local_reader(local_read));

let mut samples = Vec::new();
for _ in 0..4 {
tokio::time::sleep(Duration::from_millis(150)).await;
samples.push(session.state.lock().await.buffered_bytes);
}

// POSITIVE CONTROL. A producer that silently produced nothing would
// satisfy every bound below while proving nothing at all.
assert!(
samples[0] > 0,
"the producer never produced, so this test measured nothing"
);
for (index, bytes) in samples.iter().enumerate() {
assert!(
*bytes <= MAX_BUFFERED_BYTES,
"sample {index} held {bytes} bytes, past the {MAX_BUFFERED_BYTES} byte cap"
);
}
assert_eq!(
samples.last(),
samples.first(),
"the buffer must settle at the cap rather than keep growing: {samples:?}"
);
}

/// A local reader that hands over `bytes`, then ends the way the test asks.
///
/// This is the whole platform difference in issue 32, made explicit. Dropping
Expand Down
Loading