From 7d7f9a508fa8f490d8ffd3d6252ac6cd59af9bd5 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 23 Aug 2026 09:09:10 +0800 Subject: [PATCH] test(host): hold every host to keeping a script's executable bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A save is an overwrite of a file that already exists, and the mode belongs to the file rather than to the write. A host that wrote by creating a fresh file, or by sending the far side an explicit set of attributes, would turn a script into something the shell no longer runs — silently, on a save the user made for an unrelated reason, with nothing in the UI to report it. Both hosts already get this right; nothing here is a fix. It is the case that keeps them right, and it runs against the remote link too, where the write is a control request rather than a syscall. Checked by making the local write a remove-then-create: the case fails, as it should. The two sandbox hooks follow the existing symlink one — a host with no notion of an executable bit returns None and the case skips. --- crates/tty7-core/src/host/conformance.rs | 43 +++++++++++++++++++ crates/tty7-core/src/host/local.rs | 29 +++++++++++++ crates/tty7-server/tests/stdio_conformance.rs | 29 +++++++++++++ 3 files changed, 101 insertions(+) diff --git a/crates/tty7-core/src/host/conformance.rs b/crates/tty7-core/src/host/conformance.rs index f4c20fc1..56552fde 100644 --- a/crates/tty7-core/src/host/conformance.rs +++ b/crates/tty7-core/src/host/conformance.rs @@ -14,6 +14,18 @@ pub trait Sandbox { let _ = (target, link); None } + + /// Turns `p`'s executable bit on, or `None` where the idea does not apply. + fn set_executable(&self, p: &Path) -> Option> { + let _ = p; + None + } + + /// Whether `p` is executable, or `None` where the idea does not apply. + fn is_executable(&self, p: &Path) -> Option> { + let _ = p; + None + } } #[macro_export] @@ -39,6 +51,7 @@ macro_rules! for_each_host_case { read_file_on_a_dir_errors, write_file_creates_and_overwrites, write_file_reports_its_own_metadata, + write_file_keeps_the_executable_bit, write_file_to_missing_parent_errors, create_file_new_rejects_existing, create_dir_non_recursive_needs_parent, @@ -434,6 +447,36 @@ pub fn write_file_reports_its_own_metadata(h: &dyn Host, sb: &dyn Sandbox) { ); } +/// Saving a file must not cost it its executable bit. +/// +/// A save is an overwrite of a file that already exists, and the mode belongs +/// to the file rather than to the write — so a host that writes by creating a +/// fresh file, or by handing the far side an explicit set of attributes, turns +/// a script into something the shell will no longer run. Nothing about the +/// edit said to do that, and nothing in the UI would report it. +pub fn write_file_keeps_the_executable_bit(h: &dyn Host, sb: &dyn Sandbox) { + let f = h.join(sb.path(), "run.sh"); + h.write_file(&f, b"#!/bin/sh\necho before\n").unwrap(); + + let Some(marked) = sb.set_executable(&f) else { + return; + }; + marked.unwrap(); + assert!( + sb.is_executable(&f) + .expect("a sandbox that can set the bit can read it") + .unwrap(), + "the sandbox could not make the file executable to begin with" + ); + + h.write_file(&f, b"#!/bin/sh\necho after\n").unwrap(); + + assert!( + sb.is_executable(&f).expect("still readable").unwrap(), + "saving the script took its executable bit away" + ); +} + pub fn write_file_to_missing_parent_errors(h: &dyn Host, sb: &dyn Sandbox) { let sandbox = sb.path(); let parent = h.join(sandbox, "no-such-dir"); diff --git a/crates/tty7-core/src/host/local.rs b/crates/tty7-core/src/host/local.rs index 3e449c98..6678b740 100644 --- a/crates/tty7-core/src/host/local.rs +++ b/crates/tty7-core/src/host/local.rs @@ -501,6 +501,35 @@ mod tests { None } } + + fn set_executable(&self, p: &Path) -> Option> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + Some(std::fs::set_permissions( + p, + std::fs::Permissions::from_mode(0o755), + )) + } + #[cfg(not(unix))] + { + let _ = p; + None + } + } + + fn is_executable(&self, p: &Path) -> Option> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + Some(std::fs::metadata(p).map(|m| m.permissions().mode() & 0o111 != 0)) + } + #[cfg(not(unix))] + { + let _ = p; + None + } + } } fn sandbox() -> (SharedHost, TempSandbox) { diff --git a/crates/tty7-server/tests/stdio_conformance.rs b/crates/tty7-server/tests/stdio_conformance.rs index 4cdf5a8f..6df1f6de 100644 --- a/crates/tty7-server/tests/stdio_conformance.rs +++ b/crates/tty7-server/tests/stdio_conformance.rs @@ -43,6 +43,35 @@ impl Sandbox for TempSandbox { None } } + + fn set_executable(&self, p: &Path) -> Option> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + Some(std::fs::set_permissions( + p, + std::fs::Permissions::from_mode(0o755), + )) + } + #[cfg(not(unix))] + { + let _ = p; + None + } + } + + fn is_executable(&self, p: &Path) -> Option> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + Some(std::fs::metadata(p).map(|m| m.permissions().mode() & 0o111 != 0)) + } + #[cfg(not(unix))] + { + let _ = p; + None + } + } } fn stdio_host() -> (SharedHost, TempSandbox) {