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
6 changes: 3 additions & 3 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,9 @@ architecture the filter gates on.
the question, ends it at the first question rather than answering it
([#225](https://github.com/danczw/sandbx/issues/225)). A pane too short for
the whole question shows its first and last rows with a `…` row between, one
too small even for that counts as unable to show it, and a resize while a
question waits is not redrawn until it is answered
([#508](https://github.com/danczw/sandbx/issues/508)).
too small even for that counts as unable to show it. A resize while one waits
redraws it within about a tenth of a second. A key pressed in that gap still
answers the old frame, and one pressed once the redraw begins does not.
- **A terminal that goes away *during* a run is fail-closed and noticed.** The
read fails rather than returning an answer, so that call and the ones behind
it are refused, no further request is sent, and the process exits 3 rather
Expand Down
15 changes: 10 additions & 5 deletions context/decision-approval-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ approve → Modal::ask
1 draw the question on the screen
2 Answers::arm ───────────────────────► open the question (see below)
3 Answers::wait (Condvar, parked) ◄── y / n / a · esc / ctrl-c · hangup
a resize wakes the wait: back to 1
4 take the question down, map the reply
```

Expand All @@ -335,17 +336,21 @@ descriptor no task feeds, and nothing else is in flight during `approve`, since
- **Drawn, then armed.** The other order lets a key pressed before the question
was visible answer it. A genuine press in the gap between the two is lost, and
the operator presses again: the wrong way round costs a keystroke, not a
consent.
consent. A resize is the same gap again: the thread that would repaint is the
one parked, so `wait` returns `Reply::Resized` and `ask` draws and arms once
more, and a key pressed against the old frame is forgotten with it. The
reader looks for a resize every 100 ms because crossterm reports `SIGWINCH`
only from inside its own poll; a key pressed in that gap still answers the
old frame.
- **Drawn means drawn.** A question the screen could not take, a draw that has
failed or a lock a panic poisoned, is not asked: a key pressed meanwhile would
answer something nobody saw, so the gate aborts instead of waiting.
- **A short pane cuts the middle.** The rows naming the call and the row saying
what the keys do are the two a question cannot lose, so a pane too short for
all of it keeps those and marks the cut with a `…` row. The transcript above
it goes first. A pane that cannot keep both is not asked on: `question_fits`
says so, and the gate aborts as for a draw that failed. A resize while a
question waits is not redrawn (#508): the thread that paints is the one
parked.
it goes first. A pane that cannot keep both, at the first draw or after a
resize, is not asked on: `question_fits` says so, and the gate aborts as for
a draw that failed.
- **The question opens when the reader is quiet.** The reader may already hold
events it took off the terminal, and those predate the question. `arm` does
not reach into crossterm for them: it marks the question pending, and the
Expand Down
29 changes: 18 additions & 11 deletions crates/sandbx-cli/src/tui_modal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,33 @@ impl<S: Shows, R: Replies> Operator for Modal<S, R> {
match self.replies.stopped() {
Some(Reply::Interrupted) => return deny(INTERRUPTED),
Some(Reply::Closed) => return abort(CLOSED),
Some(Reply::Key(_)) | None => {}
Some(Reply::Key(_) | Reply::Resized) | None => {}
}
if self.consent.covers(call.tool) {
return ApprovalDecision::Allow;
}

let [head, tail] = question(call);
// Not asked over a question nobody saw: a key pressed meanwhile would answer it.
if !self.screen.show(&[&head, &tail]) {
self.screen.clear();
return abort(UNDRAWN);
}
// Again after a resize, which leaves the first draw clipped or garbled at the new
// size: the thread that would repaint it is the one parked in `wait`.
let reply = loop {
// Not asked over a question nobody saw: a key pressed meanwhile would answer it.
if !self.screen.show(&[&head, &tail]) {
self.screen.clear();
return abort(UNDRAWN);
}

// After the draw and never before: a key pressed while nothing was on screen to
// answer must not be taken for an answer to it, and `arm` is what forgets those.
let reply = match self.replies.arm(ANSWERS) {
Ok(()) => self.replies.wait(),
Err(_) => {
// After the draw and never before: a key pressed while nothing was on screen to
// answer must not be taken for an answer to it, and `arm` is what forgets those.
if self.replies.arm(ANSWERS).is_err() {
self.screen.clear();
// Refused rather than asked anyway: the queue may already hold its answer.
return abort(UNCLEARED);
}
match self.replies.wait() {
Reply::Resized => {}
reply => break reply,
}
};
self.screen.clear();

Expand All @@ -113,6 +118,8 @@ impl<S: Shows, R: Replies> Operator for Modal<S, R> {
.unwrap_or_else(|| deny(REFUSED))
}
Reply::Interrupted => deny(INTERRUPTED),
// Unreachable, the loop above taking it for a redraw; and not consent if reached.
Reply::Resized => deny(REFUSED),
Reply::Closed => abort(CLOSED),
}
}
Expand Down
65 changes: 59 additions & 6 deletions crates/sandbx-cli/src/tui_modal_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@ use super::{ANSWERS, Modal, Replies, Shows};
/// What happened, in order, across both fakes: the order is half of what is pinned.
type Log = Rc<RefCell<Vec<String>>>;

/// Whether a question reaches the screen is the second field.
struct Screen(Log, bool);
/// How many more draws reach the screen is the second field.
struct Screen(Log, usize);

impl Shows for Screen {
fn show(&mut self, lines: &[&str]) -> bool {
self.0
.borrow_mut()
.push(format!("show {}", lines.join(" / ")));
self.1
let reaches = self.1 > 0;
self.1 = self.1.saturating_sub(1);
reaches
}

fn clear(&mut self) {
Expand Down Expand Up @@ -61,7 +63,7 @@ impl Replies for Keys {
.borrow_mut()
.pop_front()
.expect("asked more often than the test scripted");
if !matches!(reply, Reply::Key(_)) {
if matches!(reply, Reply::Interrupted | Reply::Closed) {
self.stopped.set(Some(reply));
}
reply
Expand All @@ -81,7 +83,7 @@ fn modal(replies: &[Reply]) -> (Modal<Screen, Keys>, Log) {
stopped: std::cell::Cell::new(None),
};

(Modal::new(Screen(Rc::clone(&log), true), keys), log)
(Modal::new(Screen(Rc::clone(&log), usize::MAX), keys), log)
}

fn ask(
Expand Down Expand Up @@ -129,6 +131,57 @@ fn the_question_is_drawn_then_armed_then_waited_on_then_taken_down() {
);
}

/// The frame the operator is looking at is the one they answer: a resize puts the question
/// up again at the new size, and arms it only after, so a key pressed against the old
/// frame is not an answer to the new one.
#[test]
fn a_resize_redraws_the_question_and_asks_again() {
let (mut modal, log) = modal(&[Reply::Resized, Reply::Key('y')]);

let verdict = ask(&mut modal, BuiltinTool::Write, &write());

assert_eq!(verdict, ApprovalDecision::Allow);
let log = log.borrow();
let shown = "show sandbx: write /work/a.rs / sandbx: this call writes a file. allow it? \
[y]es / [n]o / [a]ll `write` calls";
assert_eq!(
*log,
[
shown,
"arm ['y', 'n', 'a']",
"wait",
shown,
"arm ['y', 'n', 'a']",
"wait",
"clear",
]
);
}

/// A pane resized too small for the question is one nobody can read it on, and is refused
/// as a question that could not be drawn at all.
#[test]
fn a_resize_that_leaves_no_room_for_the_question_aborts() {
let (mut modal, log) = modal(&[Reply::Resized]);
modal.screen.1 = 1;

let verdict = ask(&mut modal, BuiltinTool::Write, &write());

match verdict {
ApprovalDecision::Abort { reason } => {
assert!(reason.contains("could not be drawn"), "{reason}");
}
other => panic!("asked a question nobody could see: {other:?}"),
}
let log = log.borrow();
assert_eq!(
log.iter().filter(|event| *event == "wait").count(),
1,
"waited on a question that was no longer on screen: {log:?}"
);
assert_eq!(log.last().map(String::as_str), Some("clear"));
}

#[test]
fn an_n_refuses_and_tells_the_model_no_flag_lifts_it() {
let (mut modal, _) = modal(&[Reply::Key('n')]);
Expand Down Expand Up @@ -296,7 +349,7 @@ fn the_account_of_a_call_goes_where_the_question_was_asked() {
#[test]
fn a_question_that_could_not_be_drawn_is_never_asked() {
let (mut modal, log) = modal(&[]);
modal.screen.1 = false;
modal.screen.1 = 0;

let verdict = ask(&mut modal, BuiltinTool::Write, &write());

Expand Down
1 change: 1 addition & 0 deletions crates/sandbx-tui-input/public-api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub enum sandbx_tui_input::Reply
pub sandbx_tui_input::Reply::Closed
pub sandbx_tui_input::Reply::Interrupted
pub sandbx_tui_input::Reply::Key(char)
pub sandbx_tui_input::Reply::Resized
impl core::clone::Clone for sandbx_tui_input::Reply
pub fn sandbx_tui_input::Reply::clone(&self) -> sandbx_tui_input::Reply
impl core::cmp::Eq for sandbx_tui_input::Reply
Expand Down
34 changes: 31 additions & 3 deletions crates/sandbx-tui-input/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ const GONE: PollFlags = PollFlags::POLLHUP
.union(PollFlags::POLLERR)
.union(PollFlags::POLLNVAL);

/// How long the reader blocks on the terminal before looking at crossterm's own signal source,
/// in milliseconds. crossterm reports a `SIGWINCH` as `Event::Resize` only from inside
/// `event::poll`, and a `poll` on the terminal alone is not woken by it: with no key typed
/// the resize would wait for one. This is the longest an operator waits for the redraw.
const RESIZE_LOOK: u16 = 100;

/// What the reader has seen so far; `stop` and `gone` are sticky, the channel's one slot
/// losing either unread otherwise.
#[derive(Debug, Clone, Copy, Default)]
Expand Down Expand Up @@ -75,6 +81,10 @@ struct Slate {
idle: bool,
accepts: &'static [char],
keys: VecDeque<char>,
/// The terminal changed size since a wait last returned. Kept whether or not a question
/// is armed: one resized between its draw and its arm would otherwise leave the frame
/// it made stale on screen, and a spare redraw costs nothing.
resized: bool,
/// The reader has exited, for whatever reason: nobody is left to answer.
done: bool,
}
Expand Down Expand Up @@ -106,6 +116,11 @@ impl Wire {
self.wake();
}

fn resized(&self) {
self.slate().resized = true;
self.wake.notify_all();
}

fn pressed(&self, key: KeyEvent) {
self.seen.send_modify(|seen| pressed(seen, key));

Expand Down Expand Up @@ -151,6 +166,10 @@ impl Wire {
pub enum Reply {
/// The operator pressed one of the keys asked for.
Key(char),
/// The terminal changed size, so the question is no longer on screen as it was drawn.
/// Nothing typed so far is kept: it was an answer to the old frame. The caller is to
/// draw it again and [`Answers::arm`] again, in that order.
Resized,
/// ctrl-c or escape, already latched for [`Keys::stop`].
Interrupted,
/// Nobody can answer: the terminal hung up, or the reader stopped.
Expand Down Expand Up @@ -227,7 +246,8 @@ impl Answers {
///
/// No timeout, as a prompt on a terminal has none. An interrupt outranks an answer
/// already pressed, and a hangup outranks both: both leave the call refused, which is
/// the direction a doubtful reading must lean.
/// the direction a doubtful reading must lean. A resize outranks an answer too, the
/// key having been pressed against a frame that is gone, and yields to both.
#[must_use]
pub fn wait(&self) -> Reply {
let mut slate = self.wire.slate();
Expand All @@ -239,6 +259,9 @@ impl Answers {
if seen.gone || slate.done {
break Reply::Closed;
}
if slate.resized {
break Reply::Resized;
}
if let Some(answer) = slate.keys.pop_front() {
break Reply::Key(answer);
}
Expand All @@ -252,6 +275,7 @@ impl Answers {

slate.armed = false;
slate.pending = false;
slate.resized = false;
slate.keys.clear();

reply
Expand Down Expand Up @@ -394,7 +418,7 @@ fn read(wire: &Wire, tty: BorrowedFd<'_>, out: BorrowedFd<'_>, source: &mut impl
let wait = if buffered {
PollTimeout::ZERO
} else {
PollTimeout::NONE
PollTimeout::from(RESIZE_LOOK)
};

// Quiet only across a blocking poll, and only once nothing is buffered: a question
Expand Down Expand Up @@ -432,7 +456,11 @@ fn read(wire: &Wire, tty: BorrowedFd<'_>, out: BorrowedFd<'_>, source: &mut impl
return;
}
}
// Resize, mouse, paste, focus: ignored; a redraw is driven by the event stream.
Ok(Some(Event::Resize(..))) => {
buffered = true;
wire.resized();
}
// Mouse, paste, focus: ignored.
Ok(Some(_)) => buffered = true,
Ok(None) => buffered = false,
Err(_) => return,
Expand Down
Loading
Loading