Merge branch 'fix/files' into integration/round-1
This commit is contained in:
File diff suppressed because it is too large
Load Diff
@@ -196,6 +196,12 @@ pub async fn upload_host_file_to_terminal(
|
|||||||
host_path: String,
|
host_path: String,
|
||||||
state: State<'_, AppState>,
|
state: State<'_, AppState>,
|
||||||
) -> Result<String, String> {
|
) -> Result<String, String> {
|
||||||
|
// The drop target is a host path chosen by the webview, not by the OS drag
|
||||||
|
// itself, so it gets the same host-read policy as the Files pane's upload:
|
||||||
|
// absolute, no traversal, and nothing out of a hidden directory
|
||||||
|
// (`~/.ssh`, `~/.aws`) or a system location.
|
||||||
|
let host_path = crate::commands::file_commands::validate_host_read_path(&host_path)?;
|
||||||
|
|
||||||
let container_id = state.exec_manager.get_container_id(&session_id).await?;
|
let container_id = state.exec_manager.get_container_id(&session_id).await?;
|
||||||
|
|
||||||
let meta = tokio::fs::metadata(&host_path)
|
let meta = tokio::fs::metadata(&host_path)
|
||||||
|
|||||||
@@ -611,18 +611,42 @@ async fn exec_oneshot_inner(
|
|||||||
|
|
||||||
// The output stream draining doesn't strictly guarantee inspect_exec has the
|
// The output stream draining doesn't strictly guarantee inspect_exec has the
|
||||||
// final exit_code populated yet, so poll until the exec reports finished.
|
// final exit_code populated yet, so poll until the exec reports finished.
|
||||||
let exit_code = wait_for_exec_exit(&exec.id).await.unwrap_or(0);
|
let exit_code = require_exit_code(wait_for_exec_exit(&exec.id).await)?;
|
||||||
|
|
||||||
Ok((combined, exit_code))
|
Ok((combined, exit_code))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Turn "the exit code could not be determined" into an error rather than a 0.
|
||||||
|
///
|
||||||
|
/// `unwrap_or(0)` is how a rename that never happened reported success: callers
|
||||||
|
/// branch on `code != 0`, so an unreadable status silently became "it worked",
|
||||||
|
/// the UI closed its rename box and the file had not moved. An exec whose
|
||||||
|
/// outcome cannot be established has not been established to have succeeded —
|
||||||
|
/// fail closed and let the caller surface it.
|
||||||
|
///
|
||||||
|
/// The `test -e` probe in `rename_container_path` also fails closed under this:
|
||||||
|
/// it propagates the error instead of reading an undeterminable status as
|
||||||
|
/// "the destination does not exist".
|
||||||
|
fn require_exit_code(code: Option<i64>) -> Result<i64, String> {
|
||||||
|
code.ok_or_else(|| {
|
||||||
|
"Could not determine whether the command finished (Docker did not report an exit status)"
|
||||||
|
.to_string()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
/// Poll `inspect_exec` until the exec reports finished and return its exit code.
|
/// Poll `inspect_exec` until the exec reports finished and return its exit code.
|
||||||
/// Returns `None` if the code can't be determined (inspect error, or the exec
|
/// Returns `None` if the code can't be determined (inspect error, or the exec
|
||||||
/// doesn't report finished within ~1s — which shouldn't happen once its output
|
/// doesn't report finished within ~5s — which shouldn't happen once its output
|
||||||
/// stream has drained).
|
/// stream has drained).
|
||||||
|
///
|
||||||
|
/// The window is generous because `None` is no longer a shrug: since
|
||||||
|
/// [`require_exit_code`], it fails the whole call. Waiting a few seconds longer
|
||||||
|
/// for a busy daemon to settle costs nothing in the normal case — the loop exits
|
||||||
|
/// on the first poll that reports finished — and it is the difference between a
|
||||||
|
/// spurious "the rename failed" and a real one.
|
||||||
pub async fn wait_for_exec_exit(exec_id: &str) -> Option<i64> {
|
pub async fn wait_for_exec_exit(exec_id: &str) -> Option<i64> {
|
||||||
let docker = get_docker().ok()?;
|
let docker = get_docker().ok()?;
|
||||||
for _ in 0..40 {
|
for _ in 0..200 {
|
||||||
match docker.inspect_exec(exec_id).await {
|
match docker.inspect_exec(exec_id).await {
|
||||||
Ok(info) => {
|
Ok(info) => {
|
||||||
if info.running != Some(true) {
|
if info.running != Some(true) {
|
||||||
@@ -666,6 +690,16 @@ mod tests {
|
|||||||
assert!(buf.is_empty());
|
assert!(buf.is_empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_undeterminable_exit_status_is_an_error_not_a_zero() {
|
||||||
|
// The bug this guards: `unwrap_or(0)` made every caller that branches on
|
||||||
|
// `code != 0` — rename, mkdir — report success for an exec whose outcome
|
||||||
|
// nobody could read.
|
||||||
|
assert_eq!(require_exit_code(Some(0)).unwrap(), 0);
|
||||||
|
assert_eq!(require_exit_code(Some(1)).unwrap(), 1);
|
||||||
|
assert!(require_exit_code(None).is_err());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_bridge_budget_is_far_smaller_than_the_general_one() {
|
fn the_bridge_budget_is_far_smaller_than_the_general_one() {
|
||||||
// The auth bridge re-reads container-controlled procfs every 2s, so it
|
// The auth bridge re-reads container-controlled procfs every 2s, so it
|
||||||
|
|||||||
Reference in New Issue
Block a user