ask: guard key and system file reads, name stdin/stdout/connection causes, widen tests

Key file: nonblocking open, regular file only, 1024-byte bound, refuse
group/world-readable. --system-file capped at 64 KiB. Tests drive a forced
tools request through the real serve loop and the real claude backend.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Eureka 2026-11-11 08:10:53 +00:00
commit 42a157b049
3 changed files with 434 additions and 12 deletions

View file

@ -24,6 +24,7 @@ pub const DEFAULT_ENDPOINT: &str = "127.0.0.1:4110";
/// The payload bound the connector's own `config init` defaults to.
pub const PAYLOAD_BYTES: usize = 1 << 20;
const MAX_KEY_BYTES: usize = 1024;
pub const MAX_SYSTEM_BYTES: usize = 64 * 1024;
pub struct AskArgs {
pub endpoint: SocketAddr,
@ -49,6 +50,26 @@ pub enum AskError {
KeyUnreadable,
#[error("the connection key file is not text")]
KeyEncoding,
#[error("the connection key file is not a regular file")]
KeyNotRegular,
#[error("the connection key file is readable by group or world (chmod 600)")]
KeyExposed,
#[error("the instructions file cannot be read as text from a regular file")]
SystemUnreadable,
#[error("the instructions file is larger than {MAX_SYSTEM_BYTES} bytes")]
SystemTooLarge,
#[error("standard input cannot be read")]
StdinRead,
#[error("standard input is not UTF-8 text")]
StdinNotText,
#[error("the answer cannot be written to standard output")]
StdoutWrite,
#[error("nothing is listening at the connector endpoint")]
NotListening,
#[error(
"the connector closed the connection without answering (key, caller or frame not accepted)"
)]
ClosedWithoutAnswer,
#[error("the connection key file is empty or longer than {MAX_KEY_BYTES} bytes")]
KeyLength,
#[error("the connection key is not usable as a key (surrounding whitespace?)")]
@ -75,10 +96,57 @@ pub enum AskError {
EmptyAnswer,
}
enum ReadFailure {
Unreadable,
NotRegular,
TooLarge,
Exposed,
}
/// Opens `path` without blocking (a FIFO must not hang the open), requires the
/// opened file to be a regular file (a symlink is followed and its target
/// judged), optionally refuses one readable by group or world, and reads at
/// most `cap` bytes, failing if there are more.
fn read_regular(path: &Path, cap: usize, private: bool) -> Result<Zeroizing<Vec<u8>>, ReadFailure> {
use std::io::Read;
let mut options = std::fs::OpenOptions::new();
options.read(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
options.custom_flags(libc::O_NONBLOCK);
}
let file = options.open(path).map_err(|_| ReadFailure::Unreadable)?;
let metadata = file.metadata().map_err(|_| ReadFailure::Unreadable)?;
if !metadata.is_file() {
return Err(ReadFailure::NotRegular);
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if private && metadata.permissions().mode() & 0o077 != 0 {
return Err(ReadFailure::Exposed);
}
}
let mut bytes = Zeroizing::new(Vec::new());
file.take(cap as u64 + 1)
.read_to_end(&mut bytes)
.map_err(|_| ReadFailure::Unreadable)?;
if bytes.len() > cap {
return Err(ReadFailure::TooLarge);
}
Ok(bytes)
}
/// Reads the key as the connector reads its own key file (the text without the
/// trailing line ending). The bytes are zeroized on drop and never shown.
pub fn read_key(path: &Path) -> Result<Zeroizing<String>, AskError> {
let raw = Zeroizing::new(std::fs::read(path).map_err(|_| AskError::KeyUnreadable)?);
let raw = read_regular(path, MAX_KEY_BYTES, true).map_err(|failure| match failure {
ReadFailure::Unreadable => AskError::KeyUnreadable,
ReadFailure::NotRegular => AskError::KeyNotRegular,
ReadFailure::TooLarge => AskError::KeyLength,
ReadFailure::Exposed => AskError::KeyExposed,
})?;
let text = std::str::from_utf8(raw.as_slice()).map_err(|_| AskError::KeyEncoding)?;
let key = Zeroizing::new(text.trim_end_matches(['\r', '\n']).to_string());
if key.is_empty() || key.len() > MAX_KEY_BYTES {
@ -90,6 +158,15 @@ pub fn read_key(path: &Path) -> Result<Zeroizing<String>, AskError> {
Ok(key)
}
/// The instructions file: a regular file of at most [`MAX_SYSTEM_BYTES`] of text.
pub fn read_system_file(path: &Path) -> Result<String, AskError> {
let raw = read_regular(path, MAX_SYSTEM_BYTES, false).map_err(|failure| match failure {
ReadFailure::TooLarge => AskError::SystemTooLarge,
_ => AskError::SystemUnreadable,
})?;
String::from_utf8(raw.to_vec()).map_err(|_| AskError::SystemUnreadable)
}
fn fresh_id(prefix: &str) -> String {
let mut nonce = [0_u8; 16];
OsRng.fill_bytes(&mut nonce);
@ -182,7 +259,13 @@ pub fn ask(args: &AskArgs) -> Result<String, AskError> {
{
AskError::Timeout
}
ConnectorClientError::Connection(io) => AskError::Connection(io.kind()),
ConnectorClientError::Connection(io) => match io.kind() {
std::io::ErrorKind::ConnectionRefused => AskError::NotListening,
std::io::ErrorKind::UnexpectedEof
| std::io::ErrorKind::ConnectionReset
| std::io::ErrorKind::BrokenPipe => AskError::ClosedWithoutAnswer,
kind => AskError::Connection(kind),
},
ConnectorClientError::Transport(error) => AskError::Transport(error),
ConnectorClientError::FrameSize => AskError::FrameSize,
ConnectorClientError::InvalidConfig | ConnectorClientError::RequestIdTooLong => {

View file

@ -125,8 +125,8 @@ fn run_ask(args: &[&str]) -> Result<(), Failure> {
std::io::stdin()
.take(ask::PAYLOAD_BYTES as u64 + 1)
.read_to_end(&mut raw)
.map_err(|_| Failure::Malformed("stdin"))?;
let prompt = String::from_utf8(raw).map_err(|_| Failure::Malformed("stdin"))?;
.map_err(|_| AskError::StdinRead)?;
let prompt = String::from_utf8(raw).map_err(|_| AskError::StdinNotText)?;
let answer = ask::ask(&AskArgs {
endpoint,
caller_id: flags.text("--caller-id")?,
@ -141,7 +141,7 @@ fn run_ask(args: &[&str]) -> Result<(), Failure> {
let mut out = std::io::stdout();
out.write_all(answer.as_bytes())
.and_then(|()| out.flush())
.map_err(|_| Failure::Malformed("stdout"))
.map_err(|_| AskError::StdoutWrite.into())
}
/// The flags of one config verb: each single flag at most once, each list

View file

@ -24,6 +24,8 @@ enum Script {
Empty,
Failed(&'static str),
Sleep(Duration),
ToolCall,
Stop(StopReason),
}
struct Backend {
@ -53,9 +55,13 @@ impl ProviderBackend for Backend {
payload: TransportEventPayload::TextDelta { text: t.into() },
}]
};
let stop = match &self.script {
Script::Stop(reason) => *reason,
_ => StopReason::EndTurn,
};
let completed = TransportOutcome::Completed {
provider_response_id: None,
stop_reason: StopReason::EndTurn,
stop_reason: stop,
usage: usage.clone(),
};
match &self.script {
@ -69,6 +75,18 @@ impl ProviderBackend for Backend {
usage: None,
},
),
Script::Stop(_) => call.transported(text("cut off"), completed),
Script::ToolCall => call.transported(
vec![TransportEvent {
sequence: 0,
payload: TransportEventPayload::ToolCall {
call_id: "c".into(),
name: "n".into(),
arguments: "{}".into(),
},
}],
completed,
),
Script::Sleep(d) => {
thread::sleep(*d);
call.transported(text("late"), completed)
@ -77,6 +95,12 @@ impl ProviderBackend for Backend {
}
}
fn write_key(path: &std::path::Path, bytes: &[u8], mode: u32) {
use std::os::unix::fs::PermissionsExt;
std::fs::write(path, bytes).unwrap();
std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).unwrap();
}
struct Rig {
endpoint: SocketAddr,
dir: PathBuf,
@ -93,7 +117,7 @@ fn rig(script: Script) -> Rig {
));
std::fs::create_dir_all(&dir).unwrap();
let key_file = dir.join("caller.key");
std::fs::write(&key_file, format!("{KEY}\n")).unwrap();
write_key(&key_file, format!("{KEY}\n").as_bytes(), 0o600);
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let endpoint = listener.local_addr().unwrap();
let admission =
@ -216,7 +240,7 @@ fn a_wrong_key_or_caller_is_an_error_and_never_reaches_the_provider() {
let rig = rig(Script::Text("x"));
let mut wrong_key = rig.args();
let other = rig.dir.join("other.key");
std::fs::write(&other, "some-other-connection-key-0123456789").unwrap();
write_key(&other, b"some-other-connection-key-0123456789", 0o600);
wrong_key.key_file = other;
let error = ask(&wrong_key).unwrap_err();
assert!(!message(&error).contains("some-other"), "{error}");
@ -237,7 +261,7 @@ fn an_unusable_key_file_errors_without_echoing_its_contents() {
];
for (name, bytes) in cases {
let path = rig.dir.join(format!("{name}.key"));
std::fs::write(&path, &bytes).unwrap();
write_key(&path, &bytes, 0o600);
let mut args = rig.args();
args.key_file = path;
let text = message(&ask(&args).unwrap_err());
@ -254,7 +278,7 @@ fn an_unusable_key_file_errors_without_echoing_its_contents() {
directory.key_file = rig.dir.clone();
assert!(matches!(
ask(&directory).unwrap_err(),
AskError::KeyUnreadable
AskError::KeyNotRegular
));
assert!(rig.seen().is_empty());
}
@ -302,10 +326,19 @@ fn a_dead_endpoint_is_a_connection_error() {
let mut args = rig.args();
args.endpoint = listener.local_addr().unwrap();
drop(listener);
assert!(matches!(ask(&args).unwrap_err(), AskError::Connection(_)));
assert!(matches!(ask(&args).unwrap_err(), AskError::NotListening));
}
fn run_binary(rig: &Rig, stdin: &str, extra: &[&str]) -> std::process::Output {
run_binary_with_key(rig, &rig.key_file, stdin, extra)
}
fn run_binary_with_key(
rig: &Rig,
key_file: &std::path::Path,
stdin: &str,
extra: &[&str],
) -> std::process::Output {
let mut child = Command::new(env!("CARGO_BIN_EXE_claude-connector"))
.args(["ask", "--endpoint", &rig.endpoint.to_string()])
.args([
@ -316,7 +349,7 @@ fn run_binary(rig: &Rig, stdin: &str, extra: &[&str]) -> std::process::Output {
"--system",
"Be brief.",
])
.args(["--connection-key-file", rig.key_file.to_str().unwrap()])
.args(["--connection-key-file", key_file.to_str().unwrap()])
.args(extra)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
@ -330,6 +363,312 @@ fn run_binary(rig: &Rig, stdin: &str, extra: &[&str]) -> std::process::Output {
child.wait_with_output().unwrap()
}
#[test]
fn a_toolcall_in_the_answer_is_an_error() {
let rig = rig(Script::ToolCall);
assert!(matches!(ask(&rig.args()).unwrap_err(), AskError::ToolCall));
}
/// The rule: only a turn that ended (end of turn or a stop sequence) is an
/// answer; a truncated, refused or context-limited turn is an error even when
/// it carries text.
#[test]
fn a_turn_that_did_not_end_is_an_error() {
for reason in [
StopReason::MaxOutputTokens,
StopReason::Refusal,
StopReason::ContextWindow,
StopReason::ToolUse,
] {
let rig = rig(Script::Stop(reason));
assert!(
matches!(ask(&rig.args()).unwrap_err(), AskError::NotCompleted(r) if r == reason),
"{reason:?}"
);
}
let rig = rig(Script::Stop(StopReason::StopSequence));
assert_eq!(ask(&rig.args()).unwrap(), "cut off");
}
#[test]
fn a_fifo_key_file_errors_promptly_instead_of_hanging() {
let rig = rig(Script::Text("x"));
let fifo = rig.dir.join("fifo.key");
let path = std::ffi::CString::new(fifo.to_str().unwrap()).unwrap();
assert_eq!(unsafe { libc::mkfifo(path.as_ptr(), 0o600) }, 0);
let mut args = rig.args();
args.key_file = fifo;
let (tx, rx) = std::sync::mpsc::channel();
thread::spawn(move || tx.send(ask(&args)).unwrap());
let error = rx
.recv_timeout(Duration::from_secs(3))
.expect("reading a FIFO key file must not hang")
.unwrap_err();
assert!(matches!(error, AskError::KeyNotRegular), "{error}");
}
#[test]
fn a_key_file_readable_by_group_or_world_is_refused_and_a_private_one_accepted() {
let rig = rig(Script::Text("fine"));
for mode in [0o644, 0o640, 0o604] {
let path = rig.dir.join(format!("mode-{mode:o}.key"));
write_key(&path, KEY.as_bytes(), mode);
let mut args = rig.args();
args.key_file = path;
let error = ask(&args).unwrap_err();
assert!(matches!(error, AskError::KeyExposed), "{mode:o}: {error}");
assert!(!message(&error).contains(KEY));
}
assert!(rig.seen().is_empty());
assert_eq!(ask(&rig.args()).unwrap(), "fine");
}
#[test]
fn a_symlinked_key_is_judged_by_its_target() {
let rig = rig(Script::Text("linked"));
let link = rig.dir.join("link.key");
std::os::unix::fs::symlink(&rig.key_file, &link).unwrap();
let mut args = rig.args();
args.key_file = link;
assert_eq!(ask(&args).unwrap(), "linked");
let dir_link = rig.dir.join("dir.key");
std::os::unix::fs::symlink(&rig.dir, &dir_link).unwrap();
args.key_file = dir_link;
assert!(matches!(ask(&args).unwrap_err(), AskError::KeyNotRegular));
}
#[test]
fn the_key_read_is_bounded_at_1024_bytes() {
let rig = rig(Script::Text("x"));
let path = rig.dir.join("big.key");
write_key(&path, &vec![b'k'; 1_025], 0o600);
assert!(matches!(
claude_connector::ask::read_key(&path).unwrap_err(),
AskError::KeyLength
));
write_key(&path, &vec![b'k'; 1_024], 0o600);
assert!(claude_connector::ask::read_key(&path).is_ok());
}
#[test]
fn the_instructions_file_is_capped_and_must_be_a_regular_file() {
use claude_connector::ask::{MAX_SYSTEM_BYTES, read_system_file};
let rig = rig(Script::Text("x"));
let path = rig.dir.join("system.txt");
std::fs::write(&path, vec![b's'; MAX_SYSTEM_BYTES]).unwrap();
assert_eq!(read_system_file(&path).unwrap().len(), MAX_SYSTEM_BYTES);
std::fs::write(&path, vec![b's'; MAX_SYSTEM_BYTES + 1]).unwrap();
assert!(matches!(
read_system_file(&path).unwrap_err(),
AskError::SystemTooLarge
));
assert!(matches!(
read_system_file(&rig.dir).unwrap_err(),
AskError::SystemUnreadable
));
}
#[test]
fn the_binary_never_prints_key_bytes_for_a_bad_key_file() {
let rig = rig(Script::Text("x"));
let cases: [(&str, &[u8], u32); 3] = [
("exposed", b"exposed-key-bytes-0123456789", 0o644),
("padded", b" padded-key-bytes-0123456789 ", 0o600),
("wrong", b"wrong-but-wellformed-key-0123456789", 0o600),
];
for (name, bytes, mode) in cases {
let path = rig.dir.join(format!("bin-{name}.key"));
write_key(&path, bytes, mode);
let out = run_binary_with_key(&rig, &path, "q", &[]);
assert!(!out.status.success(), "{name}");
let shown = [out.stdout.clone(), out.stderr.clone()].concat();
let shown = String::from_utf8_lossy(&shown);
assert!(
!shown.contains("key-bytes") && !shown.contains("wellformed"),
"{name}: {shown}"
);
assert!(out.stdout.is_empty());
}
let out = run_binary(&rig, "q", &["--system-file", "/nonexistent"]);
assert!(!out.status.success());
}
#[test]
fn stdin_and_stdout_failures_have_their_own_wording() {
let rig = rig(Script::Text("x"));
let mut child = Command::new(env!("CARGO_BIN_EXE_claude-connector"))
.args(["ask", "--endpoint", &rig.endpoint.to_string()])
.args(["--caller-id", CALLER, "--model", MODEL, "--system", "s"])
.args(["--connection-key-file", rig.key_file.to_str().unwrap()])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.unwrap();
{
use std::io::Write;
child
.stdin
.take()
.unwrap()
.write_all(&[0xff, 0xfe])
.unwrap();
}
let out = child.wait_with_output().unwrap();
let err = String::from_utf8(out.stderr).unwrap();
assert!(err.contains("not UTF-8"), "{err}");
assert!(!err.contains("flag"), "{err}");
}
/// What the wire returns for a tools-bearing request that gets past the CLI's
/// own construction (here forced by hand) when the real claude backend sits
/// behind the real serve loop: the service admits it, the backend's planner
/// refuses it, the caller receives a transported `Failed(InvalidRequest)`
/// result, and the claude child is never started.
#[test]
fn a_forced_tools_request_reaches_the_planner_is_refused_and_no_child_starts() {
use claude_connector::{
ChildEnv, ClaudeCodeBackend, ClaudeCodeConfig, ClaudeCodeLimits, ProcessPort, RunningChild,
SessionStore, StderrSink, TokenSource,
};
use sha2::{Digest, Sha256};
use zeroize::Zeroizing;
struct CountingPort(Arc<AtomicUsize>);
impl ProcessPort for CountingPort {
fn spawn(
&self,
_: &[String],
_: &ChildEnv,
_: &std::path::Path,
_: Vec<u8>,
_: &str,
) -> std::io::Result<Box<dyn RunningChild>> {
self.0.fetch_add(1, Ordering::SeqCst);
Err(std::io::Error::other("no child in this test"))
}
}
struct Fixed(&'static str);
impl TokenSource for Fixed {
fn read(&self) -> Option<Zeroizing<String>> {
Some(Zeroizing::new(self.0.to_string()))
}
}
struct NoSessions;
impl SessionStore for NoSessions {
fn exists(&self, _: &str) -> bool {
false
}
}
struct Quiet;
impl StderrSink for Quiet {
fn stderr_tail(&self, _: &[u8]) {}
}
let dir =
std::env::temp_dir().join(format!("claude-connector-ask-tools-{}", std::process::id()));
for sub in ["work", "config", "prompt"] {
std::fs::create_dir_all(dir.join(sub)).unwrap();
}
let binary = dir.join("claude");
std::fs::write(&binary, b"pretend claude").unwrap();
let spawns = Arc::new(AtomicUsize::new(0));
let backend = ClaudeCodeBackend::with_ports(
ClaudeCodeConfig {
claude_binary: binary,
claude_binary_sha256: Sha256::digest(b"pretend claude").into(),
work_dir: dir.join("work"),
config_dir: dir.join("config"),
prompt_dir: dir.join("prompt"),
token_credential: "t".into(),
handle_secret_credential: "h".into(),
instance: "i".into(),
limits: ClaudeCodeLimits {
deadline: Duration::from_secs(5),
max_line_bytes: 1 << 16,
max_total_bytes: 1 << 20,
stderr_tail_bytes: 64,
},
},
Box::new(CountingPort(spawns.clone())),
Box::new(NoSessions),
Box::new(Quiet),
Box::new(Fixed("token")),
Box::new(Fixed(
"handle-secret-0000000000000000000000000000000000000000",
)),
);
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let endpoint = listener.local_addr().unwrap();
let admission =
CallerAdmission::new(CALLER, KEY, 1, [MODEL.to_string()], 4, 1 << 20, 4_096).unwrap();
let limits = ServeLimits {
max_frame_bytes: FRAME,
max_connections: 8,
socket_timeout_ms: 5_000,
max_expiry_skew_ms: 300_000,
};
let store = dir.join("replay.cc");
thread::spawn(move || serve(listener, limits, vec![admission], &store, Arc::new(backend)));
let mut args = AskArgs {
endpoint,
caller_id: CALLER.into(),
key_file: dir.join("unused"),
model: MODEL.into(),
instructions: "i".into(),
effort: None,
max_output_tokens: 64,
timeout: Duration::from_secs(5),
prompt: "p".into(),
};
let mut forced = build_invocation(&args).unwrap().request;
forced.tools.push(ToolDefinition {
name: "lookup".into(),
description: "d".into(),
parameters_json: "{}".into(),
});
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_millis() as u64;
let invocation = TransportInvocation::new(
CALLER,
now + 60_000,
provider_request_sha256(&forced).unwrap(),
forced,
)
.unwrap();
let client = ConnectorClient::new(endpoint, KEY, FRAME, Some(Duration::from_secs(5))).unwrap();
let result = client.execute(&invocation).unwrap();
let TransportDisposition::Transported { receipt, .. } = &result.disposition else {
panic!(
"expected the planner's failed result, got {:?}",
result.disposition
);
};
assert!(matches!(
receipt.outcome,
TransportOutcome::Failed {
kind: FailureKind::InvalidRequest,
..
}
));
assert_eq!(
spawns.load(Ordering::SeqCst),
0,
"the claude child must never start"
);
// The same CLI path with no tools would have reached the spawn.
args.key_file = dir.join("unused");
let clean = client.execute(&build_invocation(&args).unwrap()).unwrap();
assert!(matches!(
clean.disposition,
TransportDisposition::Transported { .. }
));
assert_eq!(spawns.load(Ordering::SeqCst), 1);
}
#[test]
fn the_binary_prints_the_answer_on_success() {
let rig = rig(Script::Text("pong"));