Skip to content

tokio is built with "full", two direct dependencies are dead, and two unregistered SFTP commands sit one line from being reachable #60

Description

@Sadykhzadeh

Measured against main (4abcf33) on Windows 11 Pro 26200, rustc 1.95.0, cargo 1.95.0.

Five findings in the same area: three dependency-manifest items, one reachable-surface item, one input-validation item. Only the fourth has a security dimension. I have measured every number below and I want to be plain up front that the two "size" items are not size wins — I include them for hygiene and for correctness of the dependency graph, not because they shrink the shipped binary.

1. tokio is pulled with features = ["full"] but nothing needs a third of it

Cargo.toml:16:

tokio = { version = "1", features = ["full"] }

full is the union of fs, io-util, io-std, macros, net, parking_lot, process, rt, rt-multi-thread, signal, sync, time. Walking the feature graph with cargo tree --workspace -e features -i tokio shows six of those features have no enabler in the graph other than full itself:

tokio feature "io-std"                 <- only "full"
tokio feature "parking_lot"            <- only "full"
tokio feature "process"                <- only "full"
tokio feature "signal"                 <- only "full"
tokio feature "signal-hook-registry"   <- only "process" / "signal"
tokio feature "full"                   <- only clavyn-core, clavyn-desktop

The rest are demanded by dependents regardless: rt by pageant, russh-sftp, russh-util; macros by russh-sftp, russh-util; net by russh-keys, reqwest, tokio-stream, hyper-util; io-util, sync, time, fs by russh/russh-keys/russh-sftp/tauri.

Clavyn's own use is sync, task::spawn_blocking, io::copy, fs, time::sleep, spawn, select! and #[tokio::test] — nothing that touches child processes, signal handlers or stdin/stdout wrappers.

Replacing full with the explicit list:

tokio = { version = "1", features = [
    "rt-multi-thread", "macros", "sync", "time", "net", "io-util", "fs",
] }

Measured: enabled tokio features 21 → 15; the six above disappear. Workspace crate count unchanged — 401 (host) and 570 (--target all) before and after. signal-hook-registry survives on Unix through tauri-plugin-shell -> shared_child -> sigchld -> signal-hook, and parking_lot survives through tauri-utils -> dom_query -> html5ever -> markup5ever -> web_atoms -> string_cache. So this removes zero crates on every platform; what it removes is tokio's own process/signal/io-std driver code from the compiled tokio rlib.

2. chrono is a dead direct dependency, and removing it saves nothing

grep -rn "chrono" core/src desktop/src-tauri/src --include=*.rs returns exactly one hit — the word "asynchronous" inside a doc comment in vault_keychain_cleanup.rs. It is declared in Cargo.toml:26 and core/Cargo.toml:27 and never used.

It should still go, but the payoff is close to nil, and I would rather say so than let it be read as a size fix. cargo tree --workspace -i chrono -e features shows both russh-sftp 2.4.0 and russh-util 0.46.0 depend on chrono with default features — neither sets default-features = false:

chrono feature "default"
├── clavyn-core
├── russh-sftp v2.4.0
└── russh-util v0.46.0

default drags in clock, now, std, alloc, oldtime, wasmbind, js-sys, wasm-bindgen, iana-time-zone, winapi/windows-link whatever we do. Our declaration contributes exactly one feature on top: serde.

Measured: chrono features 13 → 12, the single removal being serde. chrono itself stays in the graph. Crates removed: 0. Bytes saved: whatever chrono's serde impls monomorphise to, which is below the noise floor of a release build.

The only way to actually drop chrono is upstream — russh-sftp and russh-util setting default-features = false, or moving off the 0.46 line.

3. tracing-subscriber in core/Cargo.toml is unused and belongs to the binary

core/Cargo.toml:22 declares tracing-subscriber. grep -rn "tracing_subscriber" core/ returns nothing. core uses tracing itself, correctly — one warn! at connection.rs:31.

A library should not install a global subscriber; that is the binary's decision, and clavyn-desktop already makes it at main.rs:18. Having the dependency in core means anything linking core — including the planned mobile FFI consumer, which has no main of its own — drags a log formatter it cannot use.

Measured, and this is the one item with a real graph delta: cargo tree -p clavyn-core -e normal goes 169 → 163 crates. Removed: tracing-subscriber, tracing-log, nu-ansi-term, sharded-slab, thread_local, parking_lot. Whole-workspace count is still unchanged, because clavyn-desktop legitimately keeps tracing-subscriber with env-filter.

4. Two #[tauri::command] functions exist that generate_handler! never registers

commands.rs:642 and commands.rs:651:

#[tauri::command]
pub async fn sftp_read_file(..., session_id: String, path: String) -> ApiResult<Vec<u8>> {
    state.sftp.read_file(&session_id, &path).await.map_err(err)
}

#[tauri::command]
pub async fn sftp_write_file(..., session_id: String, path: String, data: Vec<u8>) -> ApiResult<()> {

Neither appears in the generate_handler! list at main.rs:50-104, and no frontend code references them — grep -rn "sftp_read_file\|sftp_write_file" desktop/src desktop/e2e is empty. They are unreachable today, which is why the compiler flags them.

They are not a live vulnerability. They are a one-line-away vulnerability, and that is the reason to remove rather than annotate them:

  • sftp_read_file is an unbounded remote read with an unconstrained path, returning the whole file as bytes across the IPC boundary — no size cap, no path scoping, no streaming. sftp_download_to_local (the registered path) goes through sftp_transfer.rs instead.
  • sftp_write_file is an unconstrained remote write.

Both take a caller-supplied path verbatim. Adding either name to generate_handler! — a plausible accident during an SFTP feature, since the names read like they are already wired — instantly exposes arbitrary remote read/write to anything running in the webview. Dead code that becomes an exploit primitive on a one-line change is worse than no code.

Git history says nothing is staged: git log -S sftp_read_file returns a single commit, the squashed initial import (24c4fb7). They have never been touched since, and their SftpManager counterparts in core/src/sftp.rs remain available to any future consumer that needs them.

The third dead_code warning is not the same thing

A release build emits exactly three warnings:

warning: variants `Stored` and `Invalidated` are never constructed
  --> desktop\src-tauri\src\biometric_commands.rs:11:5
warning: function `sftp_read_file` is never used
warning: function `sftp_write_file` is never used
warning: `clavyn-desktop` (bin "clavyn-desktop") generated 3 warnings

The first is a false positive of platform conditionality, not dead code. CredentialState::{Stored, Invalidated} are constructed — inside the #[cfg(all(target_os = "macos", feature = "macos-biometric"))] mod platform at biometric_commands.rs:127-129. Every other build compiles the stub at biometric_commands.rs:164, which only ever returns Missing. Deleting the variants would break the macOS Touch ID path. The fix is a cfg_attr-scoped allow(dead_code) that stays silent on macOS, so a genuine regression there is still reported.

5. session_resize truncates caller-controlled dimensions with as u16

commands.rs:527-528:

pub async fn session_resize(..., cols: u32, rows: u32) -> ApiResult<()> {
    ...
    term.master.resize(PtySize {
        rows: rows as u16,
        cols: cols as u16,

The command signature takes u32; PtySize is u16. as wraps silently, so cols: 65536 becomes 0, cols: 65537 becomes 1, and cols: 131072 becomes 0. The values arrive from the webview via IPC.

Impact is modest — the caller is our own TerminalPane, which passes xterm's measured geometry — but it is untrusted input from the renderer reaching a PTY, and a degenerate 0-column PTY is a confusing failure rather than a clean rejection. u16::try_from plus an explicit upper bound turns it into an error the caller can see. The SSH branch above it is unaffected: SessionManager::resize takes u32 end to end.


What a fix is and is not worth

Measured on this machine, before vs after all five changes, cargo build --release --workspace:

before after delta
clavyn-desktop.exe 8,438,272 B 8,434,688 B -3,584 B (-0.04%)
release build warnings 3 0 -3
workspace crates (host) 401 401 0
workspace crates (--target all) 570 570 0
clavyn-core crates 169 163 -6
enabled tokio features 21 15 -6
enabled chrono features 13 12 -1

3.5 KB. Binary size here is dominated by Tauri, wry, tao and the Windows crates; feature trimming at this layer cannot move it meaningfully, and I would not want the table above read as a size result.

Compile time is the more plausible benefit and I could not demonstrate it either. Cold cargo build -p clavyn-core into a fresh target dir: 173 crates in 57.5 s before, 167 crates in 61.0 s after — six fewer crates and a slower wall clock, i.e. entirely inside this machine's noise. I am not claiming a compile-time win.

The honest case for all five is: the manifest should describe what the code actually needs, a library should not install a log subscriber, and an unregistered arbitrary-remote-read command should not be sitting one line away from being reachable.

Not verified locally: macOS and Linux builds. The tokio feature set is platform-independent, so a missing feature would surface as a compile error on any target, but the cfg-gated macOS biometric path is only exercised on macOS.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    SecurityWhen there are security issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions