fix(Issue #392): distinguish Windows daemon startup failures

.github/workflows/ci.yml

@@ -63,6 +63,9 @@ jobs:
       - name: Run Windows named-pipe tests
         run: cargo test -p bsk --lib --locked daemon::ipc::windows::tests
 
+      - name: Run Windows daemon startup diagnostics tests
+        run: cargo test -p bsk --lib --locked daemon::start::windows::tests
+
       - name: Run Windows audit storage tests
         run: cargo test -p bsk --lib --locked daemon::audit::tests
 

crates/bsk-cli/src/cli/doctor.rs

@@ -739,6 +739,10 @@ mod m2_tests {
         for failure in [
             DaemonStartFailure::AutoStartDisabled,
             DaemonStartFailure::IndependentStartFailed,
+            DaemonStartFailure::BreakawayUnverified,
+            DaemonStartFailure::ProcessCreationDenied,
+            DaemonStartFailure::PartialBreakaway,
+            DaemonStartFailure::ResumeFailed,
         ] {
             let error = anyhow::anyhow!("startup fixture cause")
                 .context(failure)

crates/bsk-cli/src/cli/error.rs

@@ -564,6 +564,19 @@ mod tests {
                 DaemonStartFailure::IndependentStartFailed,
                 "use `bsk daemon start --foreground` in a persistent host task",
             ),
+            (
+                DaemonStartFailure::BreakawayUnverified,
+                "check access to the bsk executable",
+            ),
+            (
+                DaemonStartFailure::ProcessCreationDenied,
+                "check access to the bsk executable",
+            ),
+            (
+                DaemonStartFailure::PartialBreakaway,
+                "use `bsk daemon start --foreground` in a persistent host task",
+            ),
+            (DaemonStartFailure::ResumeFailed, "retry `bsk daemon start`"),
         ] {
             let error = anyhow::Error::new(std::io::Error::new(
                 std::io::ErrorKind::PermissionDenied,

crates/bsk-cli/src/daemon/start/windows.rs

@@ -3,17 +3,19 @@
 
 use std::ffi::{OsStr, OsString};
 use std::fs::File;
+use std::io;
 use std::os::windows::ffi::{OsStrExt, OsStringExt};
 use std::path::Path;
 
 use anyhow::{Context, Result};
+use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
 use windows_sys::Win32::System::Threading::{
     CREATE_BREAKAWAY_FROM_JOB, CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, DETACHED_PROCESS,
 };
 
 use super::{DAEMON_REPLACEMENT_WAIT_ENV, DAEMONIZED_ENV, StartArgs, apply_start_args};
 use crate::daemon::start_error::DaemonStartFailure;
-use crate::windows_process::{self, Process};
+use crate::windows_process::{self, JobBreakaway, Process};
 
 pub(super) fn spawn(exe: &Path, args: &StartArgs, predecessor_pid: Option<u32>) -> Result<Process> {
     let mut command = std::process::Command::new(exe);
@@ -42,14 +44,48 @@ pub(super) fn spawn(exe: &Path, args: &StartArgs, predecessor_pid: Option<u32>)
         [&input, &output, &output],
         DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB | CREATE_SUSPENDED,
     )
-    .context(DaemonStartFailure::IndependentStartFailed)?;
-    if let Err(err) = child.resume_outside_job() {
-        // A suspended child must never be left behind if validation/resume fails.
-        let _ = child.kill();
-        let _ = child.wait();
-        return Err(err).context(DaemonStartFailure::IndependentStartFailed);
+    .map_err(|err| spawn_failure(err, exe))?;
+    // A nested Job may allow only partial breakaway, so a successful
+    // CreateProcessW is not sufficient.
+    let failure = match child.outside_job() {
+        Ok(true) => match child.resume() {
+            Ok(()) => return Ok(child),
+            Err(err) => anyhow::Error::new(err).context(DaemonStartFailure::ResumeFailed),
+        },
+        Ok(false) => {
+            anyhow::anyhow!("daemon process still belongs to a Job Object after breakaway")
+                .context(DaemonStartFailure::PartialBreakaway)
+        }
+        Err(err) => anyhow::Error::new(err).context("check the daemon's Job membership"),
+    };
+    // A suspended child must never be left behind if validation/resume fails.
+    let _ = child.kill();
+    let _ = child.wait();
+    Err(failure)
+}
+
+fn spawn_failure(err: io::Error, exe: &Path) -> anyhow::Error {
+    classify_spawn_failure(err, windows_process::current_job_breakaway().ok(), exe)
+}
+
+/// With CREATE_BREAKAWAY_FROM_JOB, CreateProcessW fails with access denied
+/// when the immediate Job forbids breakaway, but access to the executable or
+/// process can be denied for other reasons. Only a Job policy that was read
+/// as forbidding breakaway is reported as a Job refusal; `job` is `None` when
+/// the policy could not be read.
+fn classify_spawn_failure(err: io::Error, job: Option<JobBreakaway>, exe: &Path) -> anyhow::Error {
+    let denied = err.raw_os_error() == Some(ERROR_ACCESS_DENIED as i32);
+    let err = anyhow::Error::new(err).context(format!("create daemon process {}", exe.display()));
+    if !denied {
+        return err;
     }
-    Ok(child)
+    err.context(match job {
+        Some(JobBreakaway::Forbidden) => DaemonStartFailure::IndependentStartFailed,
+        Some(JobBreakaway::Allowed | JobBreakaway::NotInJob) => {
+            DaemonStartFailure::ProcessCreationDenied
+        }
+        None => DaemonStartFailure::BreakawayUnverified,
+    })
 }
 
 /// Whether [`spawn`] can start a daemon outside this process's Jobs. Creates
@@ -70,15 +106,15 @@ pub(super) fn check_breakaway(exe: &Path) -> Result<()> {
         [&input, &output, &output],
         DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB | CREATE_SUSPENDED,
     )
-    .context(DaemonStartFailure::IndependentStartFailed)?;
+    .map_err(|err| spawn_failure(err, exe))?;
     let outside = probe.outside_job();
     let _ = probe.kill();
     let _ = probe.wait();
     if !outside.context("check the probe's Job membership")? {
         return Err(anyhow::anyhow!(
-            "breakaway from an outer Job is not allowed"
+            "daemon probe still belongs to a Job Object after breakaway"
         ))
-        .context(DaemonStartFailure::IndependentStartFailed);
+        .context(DaemonStartFailure::PartialBreakaway);
     }
     Ok(())
 }
@@ -112,3 +148,71 @@ pub(super) fn command_line<'a>(args: impl Iterator<Item = &'a OsStr>) -> OsStrin
     }
     OsString::from_wide(&result)
 }
+
+#[cfg(test)]
+mod tests {
+    use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND;
+
+    use super::*;
+
+    const POLICIES: [Option<JobBreakaway>; 4] = [
+        Some(JobBreakaway::NotInJob),
+        Some(JobBreakaway::Allowed),
+        Some(JobBreakaway::Forbidden),
+        None,
+    ];
+
+    fn classified(code: u32, job: Option<JobBreakaway>) -> anyhow::Error {
+        classify_spawn_failure(
+            io::Error::from_raw_os_error(code as i32),
+            job,
+            Path::new(r"C:\bsk\bsk.exe"),
+        )
+    }
+
+    fn assert_keeps_cause(error: &anyhow::Error, code: u32) {
+        let message = format!("{error:#}");
+        assert!(
+            message.contains(r"create daemon process C:\bsk\bsk.exe"),
+            "{message}"
+        );
+        assert!(message.contains(&format!("(os error {code})")), "{message}");
+    }
+
+    #[test]
+    fn access_denied_is_a_job_refusal_only_when_the_job_forbids_breakaway() {
+        for (job, expected) in [
+            (
+                Some(JobBreakaway::Forbidden),
+                DaemonStartFailure::IndependentStartFailed,
+            ),
+            (
+                Some(JobBreakaway::Allowed),
+                DaemonStartFailure::ProcessCreationDenied,
+            ),
+            (
+                Some(JobBreakaway::NotInJob),
+                DaemonStartFailure::ProcessCreationDenied,
+            ),
+            (None, DaemonStartFailure::BreakawayUnverified),
+        ] {
+            let error = classified(ERROR_ACCESS_DENIED, job);
+            assert_eq!(
+                error.downcast_ref::<DaemonStartFailure>(),
+                Some(&expected),
+                "{job:?}"
+            );
+            assert_keeps_cause(&error, ERROR_ACCESS_DENIED);
+        }
+    }
+
+    #[test]
+    fn other_spawn_errors_keep_their_cause_without_job_guidance() {
+        for job in POLICIES {
+            let error = classified(ERROR_FILE_NOT_FOUND, job);
+            assert!(error.downcast_ref::<DaemonStartFailure>().is_none());
+            assert_keeps_cause(&error, ERROR_FILE_NOT_FOUND);
+            assert!(!format!("{error:#}").contains("Job Object"), "{job:?}");
+        }
+    }
+}

crates/bsk-cli/src/daemon/start_error.rs

@@ -1,16 +1,52 @@
 //! Recovery guidance for startup failures, preserved through anyhow contexts.
 
-#[derive(Debug, Clone, Copy, thiserror::Error)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)]
 pub(crate) enum DaemonStartFailure {
     #[error(
         "automatic daemon startup is disabled (BSK_AUTO_START=0); restore the daemon in its owning environment with its original configuration"
     )]
     AutoStartDisabled,
+    /// Process creation was denied and the immediate Job's policy was read as
+    /// forbidding breakaway.
     #[cfg(any(windows, test))]
     #[error(
-        "cannot start an independent Windows daemon; the host may prohibit Job Object breakaway; use `bsk daemon start --foreground` in a persistent host task or start from an independent terminal"
+        "cannot start an independent Windows daemon; the Job Object of this process does not allow breakaway; \
+         use `bsk daemon start --foreground` in a persistent host task"
     )]
     IndependentStartFailed,
+
+    /// Process creation was denied inside a Job whose policy could not be read.
+    #[cfg(any(windows, test))]
+    #[error(
+        "cannot start an independent Windows daemon; process creation was denied, either by a Job Object \
+         that forbids breakaway or by access permissions on the executable or process; \
+         check access to the bsk executable, or use `bsk daemon start --foreground` in a persistent host task"
+    )]
+    BreakawayUnverified,
+
+    /// Process creation was denied although no Job policy forbids breakaway.
+    #[cfg(any(windows, test))]
+    #[error(
+        "cannot start an independent Windows daemon; process creation was denied although the Job Object \
+         of this process, if any, allows breakaway; check access to the bsk executable"
+    )]
+    ProcessCreationDenied,
+
+    /// The child was created, so the immediate Job allowed breakaway, but it
+    /// still belongs to a Job. Which Job holds it is not inspected.
+    #[cfg(any(windows, test))]
+    #[error(
+        "cannot start an independent Windows daemon; the daemon process was created but still belongs to \
+         a Job Object after breakaway; use `bsk daemon start --foreground` in a persistent host task"
+    )]
+    PartialBreakaway,
+
+    #[cfg(any(windows, test))]
+    #[error(
+        "cannot start an independent Windows daemon; the daemon process left every Job Object \
+         but could not be resumed; retry `bsk daemon start`"
+    )]
+    ResumeFailed,
 }
 
 impl DaemonStartFailure {
@@ -25,11 +61,48 @@ impl DaemonStartFailure {
             }
             #[cfg(any(windows, test))]
             Self::IndependentStartFailed => {
-                "Do not retry detached startup in this host. Run `bsk daemon start --foreground` \
-                 in a persistent host task that outlives client commands, or run `bsk daemon start` \
-                 from an independent terminal. Use the same BSK_HOME and OS user as the clients. \
-                 Then set BSK_AUTO_START=0 for client commands and verify `bsk status --json` \
-                 in a separate invocation."
+                "The Job Object of this process does not allow breakaway, so a detached daemon \
+                 cannot leave it. Do not retry detached startup in this host. Run \
+                 `bsk daemon start --foreground` in a persistent host task that outlives client \
+                 commands, or run `bsk daemon start` from an independent terminal that is not \
+                 inside this Job. Use the same BSK_HOME and OS user as the clients. Then set \
+                 BSK_AUTO_START=0 for client commands and verify `bsk status --json` in a \
+                 separate invocation."
+            }
+            #[cfg(any(windows, test))]
+            Self::BreakawayUnverified => {
+                "This process is in a Job Object whose breakaway policy could not be read, so \
+                 the cause is not confirmed: a Job restriction or denied access to the \
+                 executable or process. Check that this OS user can run the bsk executable named \
+                 above and that security software does not block it. If access is fine, run \
+                 `bsk daemon start --foreground` in a persistent host task that outlives client \
+                 commands, or run `bsk daemon start` from an independent terminal. Use the same \
+                 BSK_HOME and OS user as the clients. Then set BSK_AUTO_START=0 for client \
+                 commands and verify `bsk status --json` in a separate invocation."
+            }
+            #[cfg(any(windows, test))]
+            Self::ProcessCreationDenied => {
+                "Breakaway is permitted here, so a host Job restriction did not cause this. \
+                 Check that this OS user can run the bsk executable named above and that \
+                 security software does not block it, then retry `bsk daemon start`."
+            }
+            #[cfg(any(windows, test))]
+            Self::PartialBreakaway => {
+                "The daemon process was created, so the immediate Job Object allows breakaway, \
+                 but the process still belongs to a Job Object after breakaway, for example an \
+                 outer Job held by an IDE, a launcher or a system service. Run \
+                 `bsk daemon start --foreground` in a persistent host task that outlives client \
+                 commands. If another host or terminal does not run inside that Job, \
+                 `bsk daemon start` from there can succeed. Use the same BSK_HOME and OS user \
+                 as the clients. Then set BSK_AUTO_START=0 for client commands and verify \
+                 `bsk status --json` in a separate invocation."
+            }
+            #[cfg(any(windows, test))]
+            Self::ResumeFailed => {
+                "Job Object breakaway succeeded, so this is not a host Job restriction. \
+                 The suspended daemon process was terminated before it ran. \
+                 Retry `bsk daemon start`. If it keeps failing, check whether security \
+                 software is blocking bsk.exe."
             }
         }
     }

crates/bsk-cli/src/windows_process.rs

@@ -14,7 +14,11 @@ use std::process::ExitStatus;
 use windows_sys::Win32::Foundation::{
     DUPLICATE_SAME_ACCESS, DuplicateHandle, HANDLE, WAIT_OBJECT_0, WAIT_TIMEOUT,
 };
-use windows_sys::Win32::System::JobObjects::IsProcessInJob;
+use windows_sys::Win32::System::JobObjects::{
+    IsProcessInJob, JOB_OBJECT_LIMIT_BREAKAWAY_OK, JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK,
+    JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JobObjectExtendedLimitInformation,
+    QueryInformationJobObject,
+};
 use windows_sys::Win32::System::Threading::{
     CREATE_UNICODE_ENVIRONMENT, CreateProcessW, DeleteProcThreadAttributeList,
     EXTENDED_STARTUPINFO_PRESENT, GetCurrentProcess, GetExitCodeProcess, GetProcessId,
@@ -73,15 +77,8 @@ impl Process {
         Ok(!process_in_job(self.handle.as_raw_handle())?)
     }
 
-    /// Called only for a child created suspended. A nested Job may allow only
-    /// partial breakaway, so successful CreateProcessW is not sufficient.
-    pub(crate) fn resume_outside_job(&self) -> io::Result<()> {
-        if process_in_job(self.handle.as_raw_handle())? {
-            return Err(io::Error::new(
-                io::ErrorKind::PermissionDenied,
-                "daemon is still associated with a host Job Object after breakaway",
-            ));
-        }
+    /// Called only for a child created suspended.
+    pub(crate) fn resume(&self) -> io::Result<()> {
         // SAFETY: this is the primary thread returned by CreateProcessW.
         if unsafe { ResumeThread(self.thread.as_raw_handle()) } == u32::MAX {
             return Err(io::Error::last_os_error());
@@ -232,6 +229,44 @@ pub(crate) fn current_process_in_job() -> io::Result<bool> {
     process_in_job(unsafe { GetCurrentProcess() })
 }
 
+/// Breakaway policy of the current process's immediate Job.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum JobBreakaway {
+    NotInJob,
+    Allowed,
+    Forbidden,
+}
+
+/// Only the immediate Job is visible here; outer Jobs of a nested chain are
+/// not, so `Allowed` does not mean a child can leave every Job.
+pub(crate) fn current_job_breakaway() -> io::Result<JobBreakaway> {
+    if !current_process_in_job()? {
+        return Ok(JobBreakaway::NotInJob);
+    }
+    // SAFETY: the struct permits zero initialization.
+    let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { std::mem::zeroed() };
+    // SAFETY: a null Job queries the immediate Job of the current process, and
+    // the buffer matches the requested information class.
+    if unsafe {
+        QueryInformationJobObject(
+            std::ptr::null_mut(),
+            JobObjectExtendedLimitInformation,
+            (&mut info as *mut JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(),
+            std::mem::size_of_val(&info) as u32,
+            std::ptr::null_mut(),
+        )
+    } == 0
+    {
+        return Err(io::Error::last_os_error());
+    }
+    let breakaway = JOB_OBJECT_LIMIT_BREAKAWAY_OK | JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK;
+    Ok(if info.BasicLimitInformation.LimitFlags & breakaway != 0 {
+        JobBreakaway::Allowed
+    } else {
+        JobBreakaway::Forbidden
+    })
+}
+
 fn process_in_job(process: HANDLE) -> io::Result<bool> {
     let mut in_job = 0;
     // SAFETY: a null job queries membership in any Job; the output is valid.