[ML] Reap child IPC dirs and fail closed on degraded seccomp

bin/controller/CProcessSpawnerRouter.cc

@@ -149,7 +149,15 @@ CProcessSpawnerRouter::CProcessSpawnerRouter(const TStrVec& permittedProcessPath
     : m_LegacySpawner{permittedProcessPaths}, m_SandboxedProcessPaths{sandboxedProcessPaths},
       m_ConfinementFn{confinementFn ? std::move(confinementFn) : TConfinementFn{[] {
           return sandbox::hostConfinement();
-      }}} {
+      }}},
+      m_ChildIpcReaper{std::make_shared<sandbox::CChildIpcDirectoryReaper>()} {
+    m_LegacySpawner.setChildIpcDirectoryCallbacks(
+        [reaper = m_ChildIpcReaper](core::CProcess::TPid pid) {
+            reaper->onChildExited(pid);
+        },
+        [reaper = m_ChildIpcReaper](core::CProcess::TPid pid, const std::string& root) {
+            reaper->noteSpawn(pid, root);
+        });
 }
 
 const std::string& CProcessSpawnerRouter::lastSpawnFailureReason() const {
@@ -252,14 +260,25 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
 
     // Derived exactly once per spawn() call, before either backend runs, so
     // the sandbox2_launch signal below reports the same childId the
-    // dispatch decision was taken against - see deriveDeploymentId()'s comment for why a
-    // post-spawn second derivation is not equivalent. Skipped entirely for
-    // processes that can never emit the signal, so unrelated permitted
-    // processes (autodetect etc.) pay no ::realpath() cost.
+    // dispatch decision was taken against - see prepareChildIpcLaunch()'s
+    // comment for why a post-spawn second derivation is not equivalent.
+    // Skipped entirely for processes that can never emit the signal, so
+    // unrelated permitted processes (autodetect etc.) pay no ::realpath() cost.
     const SPreparedChildIpcLaunch prepared{
         sandboxEligible ? prepareChildIpcLaunch(args) : SPreparedChildIpcLaunch{}};
     const std::string deploymentId{
         sandboxEligible ? prepared.s_Validation.s_Spec.s_ChildId : std::string()};
+    std::string childIpcRoot;
+    if (sandboxEligible) {
+        if (prepared.s_Validation.s_Ok) {
+            childIpcRoot = prepared.s_Validation.s_Spec.s_ChildIpcRoot;
+        } else {
+            childIpcRoot = sandbox::perChildIpcRootFromArgs(
+                trustedTmpDirFromEnvironment(), args);
+        }
+    }
+    const std::string* childIpcRootPtr{
+        sandboxEligible && childIpcRoot.empty() == false ? &childIpcRoot : nullptr};
 
     m_LastSpawnFailureReason.clear();
     bool spawned{false};
@@ -278,7 +297,7 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
         // sandbox2_launch signal below can report it.
         LOG_INFO(<< "Launching '" << processPath << "' without Sandbox2 (legacy route selected by the controller); "
                  << "the in-process seccomp filter applies");
-        spawned = m_LegacySpawner.spawn(processPath, args, childPid);
+        spawned = m_LegacySpawner.spawn(processPath, args, childPid, childIpcRootPtr);
     } else if (sandboxEligible) {
     // route == ERoute::E_Sandbox2, and processPath is configured as
     // sandboxed.
@@ -305,6 +324,7 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
             // contract as the legacy spawner - see the member's declaration.
             if (m_SandboxSpawner == nullptr) {
                 m_SandboxSpawner = std::make_unique<sandbox::CSandboxedProcessSpawner>();
+                m_SandboxSpawner->setChildIpcDirectoryReaper(m_ChildIpcReaper);
             }
             // No automatic fallback on a Sandbox2 *failure*: a host that can
             // run Sandbox2 but fails this launch has a problem worth
@@ -335,7 +355,7 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
             LOG_INFO(<< sandbox::landlockFallbackMessage(host, processPath));
             TStrVec landlockArgs{args};
             landlockArgs.emplace_back(RESTRICT_FILESYSTEM_TOKEN);
-            spawned = m_LegacySpawner.spawn(processPath, landlockArgs, childPid);
+            spawned = m_LegacySpawner.spawn(processPath, landlockArgs, childPid, childIpcRootPtr);
             break;
         }
         case sandbox::EConfinementLevel::E_Unavailable:
@@ -369,6 +389,11 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
 
     if (sandboxEligible) {
         this->emitLaunchSignal(route, legacyReason, deploymentId, args, spawned, landlockFallback);
+        if (spawned == false && childIpcRoot.empty() == false) {
+            // Backstop for routes/backends that do not self-notify (legacy,
+            // Landlock, unavailable); Sandbox2 may already have called this.
+            m_ChildIpcReaper->onSpawnFailed(childIpcRoot);
+        }
     }
 
     return spawned;

bin/controller/CProcessSpawnerRouter.h

@@ -14,6 +14,7 @@
 #include <core/CDetachedProcessSpawner.h>
 #include <core/CProcess.h>
 
+#include <sandbox/CChildIpcDirectoryReaper.h>
 #include <sandbox/CSandbox2Diagnostics.h>
 
 #include <functional>
@@ -212,6 +213,8 @@ class CProcessSpawnerRouter {
 
     //! See lastSpawnFailureReason().
     std::string m_LastSpawnFailureReason;
+
+    std::shared_ptr<sandbox::CChildIpcDirectoryReaper> m_ChildIpcReaper;
 };
 
 } // namespace controller

bin/pytorch_inference/Main.cc

@@ -356,45 +356,21 @@ int main(int argc, char** argv) {
         return EXIT_FAILURE;
     }
 
-    // Internal switch, deliberately still OFF (log-and-continue on a failed
-    // in-process seccomp installation, exactly as before typed routing).
-    //
-    // Turning it on is only safe once a degraded/legacy-route launch is
-    // guaranteed to be a deliberate decision rather than an unrequested
-    // default. CProcessSpawnerRouter supplies half of that guarantee - it
-    // never falls back to the legacy spawner after a failed Sandbox2
-    // attempt - but the controller's no-token case still always takes the
-    // legacy route (see bin/controller/CCommandProcessor.cc), and a caller
-    // that omits both routing tokens is not necessarily choosing that
-    // deliberately. So an ordinary launch with no explicit token is a
-    // degraded-route launch, and terminating on seccomp-install failure
-    // would fail every launch on a host lacking usable seccomp BPF
-    // (restricted containers, some CI images) with no fallback to select
-    // instead.
-    //
-    // Activate this once every caller that matters (in practice,
-    // Elasticsearch) always sends an explicit --disableSandbox or
-    // --requireSandbox token per launch, so a degraded launch really is
-    // only ever reachable via an explicit, controller-validated
-    // --disableSandbox token, which is what makes hard termination safe
-    // (track: elastic/ml-cpp#3213).
-    constexpr bool TERMINATE_ON_DEGRADED_SECCOMP_FAILURE{false};
-
     // The in-process filter belongs to the legacy/non-sandboxed route only.
     // On the Sandbox2 route the executor's own policy is already the
     // security boundary and ML_SANDBOXED is exactly "1", so the whole step -
     // install, degraded-mode decision, attestation marker - is skipped.
     // Attempting it from inside an already-sandboxed environment would
-    // either fail (which would terminate every enforced-route launch once
-    // hard termination above is activated) or succeed and emit the
+    // either fail (which would terminate every enforced-route launch when
+    // TERMINATE_ON_DEGRADED_SECCOMP_FAILURE is true) or succeed and emit the
     // legacy-route attestation marker on a launch the controller's
     // sandbox2_launch signal reports as "route":"sandbox2".
     const bool sandbox2Launched{ml::seccomp::sandbox2LaunchedChild()};
     // The same filter is installed on the Landlock rung - Landlock and seccomp
     // are meant to stack - so its attestation names that route, matching the
     // controller's sandbox2_launch signal for this launch.
     const ml::seccomp::SInProcessFilterResult seccompResult{ml::seccomp::applyInProcessSeccompFilter(
-        sandbox2Launched, TERMINATE_ON_DEGRADED_SECCOMP_FAILURE,
+        sandbox2Launched, ml::seccomp::TERMINATE_ON_DEGRADED_SECCOMP_FAILURE,
         [] { return ml::seccomp::CSystemCallFilter::installSystemCallFilter(); },
         restrictFilesystem ? "landlock" : "legacy")};
 

docs/sandbox2_production_failure_modes.md

@@ -126,12 +126,13 @@ in-process filter. Only `CSandboxedProcessSpawner` sets it, and only on real
 sandboxees.
 
 Hard termination on a failed in-process seccomp installation
-(`TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` in
-`bin/pytorch_inference/Main.cc`) is deliberately **off**: an ordinary launch
-with no explicit routing token is a degraded-route launch, so terminating
-would fail every launch on a host without usable seccomp BPF. It becomes
-safe to activate once every caller that matters always sends an explicit
-`--disableSandbox` or `--requireSandbox` token per launch.
+(`TERMINATE_ON_DEGRADED_SECCOMP_FAILURE` in `include/seccomp/CSystemCallFilter.h`)
+is **on** for legacy and Landlock launches: if the filter cannot be installed,
+`pytorch_inference` exits before reading untrusted model bytes. Elasticsearch
+sends an explicit `--disableSandbox` or `--requireSandbox` token on every Linux
+production launch; direct controller invocations with no token still take the
+legacy route and fail closed the same way. Sandbox2 children (`ML_SANDBOXED=1`)
+never install this filter.
 
 Example:
 

include/core/CDetachedProcessSpawner.h

@@ -14,6 +14,7 @@
 #include <core/CProcess.h>
 #include <core/ImportExport.h>
 
+#include <functional>
 #include <memory>
 #include <string>
 #include <vector>
@@ -154,6 +155,25 @@ class CORE_EXPORT CDetachedProcessSpawner {
     //! started.
     bool spawn(const std::string& processPath, const TStrVec& args, CProcess::TPid& childPid);
 
+    //! As above. When \p childIpcRoot is non-null and non-empty, invokes the
+    //! child-spawned IPC callback (see setChildIpcDirectoryCallbacks()) with
+    //! the new pid and root before the tracker mutex is released, so a fast
+    //! child exit cannot be observed before the binding exists.
+    bool spawn(const std::string& processPath,
+               const TStrVec& args,
+               CProcess::TPid& childPid,
+               const std::string* childIpcRoot);
+
+    //! Optional hooks for CChildIpcDirectoryReaper wiring from the controller
+    //! router. The exit callback is invoked from the tracker thread after a
+    //! reaped pid is removed from the live set; it must not call back into
+    //! this spawner.
+    using TChildExitedCallback = std::function<void(CProcess::TPid)>;
+    using TChildSpawnedIpcCallback =
+        std::function<void(CProcess::TPid, const std::string&)>;
+    void setChildIpcDirectoryCallbacks(TChildExitedCallback onChildExited,
+                                       TChildSpawnedIpcCallback onChildSpawnedWithIpc);
+
     //! Kill the child process with the specified PID.  If there is a
     //! process running with the specified PID that was not spawned by this
     //! object then it will NOT be killed.
@@ -171,6 +191,8 @@ class CORE_EXPORT CDetachedProcessSpawner {
     //! alive.
     TTrackerThreadP m_TrackerThread;
 
+    TChildSpawnedIpcCallback m_OnChildSpawnedWithIpc;
+
 #ifndef Windows
     //! On *nix testing which files need to be closed when spawning a process
     //! can be expensive, so this variable is used to learn the highest file

include/sandbox/CChildIpcDirectoryReaper.h

@@ -0,0 +1,60 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the following additional limitation. Functionality enabled by the
+ * files subject to the Elastic License 2.0 may only be used in production when
+ * invoked by an Elasticsearch process with a license key installed that permits
+ * use of machine learning features. You may not use this file except in
+ * compliance with the Elastic License 2.0 and the foregoing additional
+ * limitation.
+ */
+#ifndef INCLUDED_ml_sandbox_CChildIpcDirectoryReaper_h
+#define INCLUDED_ml_sandbox_CChildIpcDirectoryReaper_h
+
+#include <core/CProcess.h>
+
+#include <map>
+#include <mutex>
+#include <string>
+
+namespace ml {
+namespace sandbox {
+
+//! Removes a single per-deployment directory under $TMPDIR/ml-child-ipc/<child-id>
+//! once the controller no longer has a live child bound to it.
+//!
+//! Lifecycle (paired with ensureChildIpcDirectory()):
+//! - ensureChildIpcDirectory() creates $TMPDIR/ml-child-ipc/<child-id> (0700) before
+//!   realpath()/policy construction; the intermediate ml-child-ipc directory is kept.
+//! - noteSpawn() records which pid owns that canonical root for a successful launch.
+//! - onChildExited() removes the directory only when the root is still bound to that pid
+//!   (a fast restart reusing the same deployment id updates the binding first).
+//! - onSpawnFailed() removes a root that was created but never got a live pid.
+//! Ownership validation and directory removal run under one lock: a concurrent
+//! noteSpawn() for the same deployment root cannot interleave between unbinding
+//! and unlink/rmdir.
+//!
+//! Controller crash can still leave empty directories; there is no startup sweep because
+//! that would race another controller on the same $TMPDIR.
+class CChildIpcDirectoryReaper {
+public:
+    void noteSpawn(core::CProcess::TPid pid, const std::string& canonicalChildRoot);
+
+    void onChildExited(core::CProcess::TPid pid);
+
+    void onSpawnFailed(const std::string& canonicalChildRoot);
+
+private:
+    static bool canonicalPerChildIpcRootHasExpectedShape(const std::string& canonicalChildRoot);
+
+    static bool tryRemovePerChildIpcDirectory(const std::string& canonicalChildRoot);
+
+    std::mutex m_Mutex;
+    std::map<core::CProcess::TPid, std::string> m_PidToRoot;
+    std::map<std::string, core::CProcess::TPid> m_RootToPid;
+};
+
+} // namespace sandbox
+} // namespace ml
+
+#endif // INCLUDED_ml_sandbox_CChildIpcDirectoryReaper_h

include/sandbox/CPytorchInferenceSandboxPolicy.h

@@ -124,10 +124,11 @@ enum class EChildIpcDirectoryOutcome {
 //! the sandbox2_launch signal, which runs even on the legacy route) - must
 //! call this first.
 //!
-//! The per-child directory is not removed here: it is keyed by deployment
-//! id, reused across controller/process restarts for the same id, and is
-//! empty once pytorch_inference has unlinked its FIFOs. Removing it is the
-//! caller's responsibility once the deployment ends (elastic/ml-cpp#3214).
+//! The per-child directory is not removed here. Once pytorch_inference has
+//! unlinked its FIFOs the directory is usually empty; CChildIpcDirectoryReaper
+//! removes it when the controller observes that child exit (or after a failed
+//! spawn that never bound a live pid). The parent $TMPDIR/ml-child-ipc
+//! directory is never removed here.
 //!
 //! Idempotent: an already-existing directory is E_Ready, not an error, so a
 //! retry/restart that reuses the same child-id never fails here. Uses only
@@ -139,6 +140,13 @@ enum class EChildIpcDirectoryOutcome {
 EChildIpcDirectoryOutcome ensureChildIpcDirectory(const std::string& trustedTmpDir,
                                                   const std::vector<std::string>& args);
 
+//! Returns the canonical per-child IPC root when validation succeeds, or the
+//! literal $TMPDIR/ml-child-ipc/<child-id> path implied by args when a path
+//! option matches the expected shape (used to reap directories left behind by
+//! a failed spawn after ensureChildIpcDirectory()).
+std::string perChildIpcRootFromArgs(const std::string& trustedTmpDir,
+                                    const std::vector<std::string>& args);
+
 #ifdef SANDBOX2_AVAILABLE
 
 //! Builds the filesystem and network-shape portion of the pytorch_inference

include/sandbox/CSandboxedProcessSpawner.h

@@ -44,6 +44,8 @@ class Sandbox2;
 namespace ml {
 namespace sandbox {
 
+class CChildIpcDirectoryReaper;
+
 //! \brief
 //! Spawn and own the lifecycle of processes inside a Sandbox2 isolation
 //! boundary.
@@ -269,6 +271,8 @@ class CSandboxedProcessSpawner {
     //! PID that is still live (not yet Reaped or Failed).
     bool hasChild(core::CProcess::TPid pid) const;
 
+    void setChildIpcDirectoryReaper(const std::shared_ptr<CChildIpcDirectoryReaper>& reaper);
+
 private:
     const TPidRegistryPtr m_PidRegistry{std::make_shared<SPidRegistry>()};
 
@@ -282,6 +286,8 @@ class CSandboxedProcessSpawner {
 #ifdef SANDBOX2_AVAILABLE
     TAwaitResultFn m_AwaitResultFn;
 #endif
+
+    std::shared_ptr<CChildIpcDirectoryReaper> m_ChildIpcReaper;
 };
 
 } // namespace sandbox

include/seccomp/CSystemCallFilter.h

@@ -83,25 +83,21 @@ enum class EDegradedModeAction {
 //! Pure decision function: does this install outcome require terminating
 //! before untrusted IO/model processing?
 //!
-//! terminateOnFailure is an internal switch, not an operator setting. Every
-//! degraded-mode seccomp failure should eventually terminate before
-//! processing, but flipping that on for every call site before the
-//! ml-cpp/Elasticsearch controller protocol can guarantee a degraded-mode
-//! launch was a deliberate operator choice would fail every launch on a
-//! host lacking seccomp BPF, with no operator fallback setting to select
-//! instead. It is only safe to pass true where a degraded-mode launch is
-//! guaranteed to be a deliberate route decision rather than the production
-//! default. bin/controller's CProcessSpawnerRouter provides half of that
-//! guarantee (it never retries a failed Sandbox2 spawn through the legacy
-//! spawner), but while CCommandProcessor's no-token case still always
-//! routes to legacy, and no caller is yet guaranteed to always send an
-//! explicit --disableSandbox/--requireSandbox token, an ordinary launch
-//! *is* a degraded-route launch, so bin/pytorch_inference/Main.cc passes
-//! false. See the comment at TERMINATE_ON_DEGRADED_SECCOMP_FAILURE there
-//! for when it flips.
-//! This decision only ever
-//! applies to a launch that installs its own in-process filter at all - see
-//! sandbox2LaunchedChild() and applyInProcessSeccompFilter() below.
+//! Legacy and Landlock pytorch_inference launches pass this to
+//! applyInProcessSeccompFilter(). When true, a failed in-process filter
+//! installation terminates before untrusted model IO. Elasticsearch sends an
+//! explicit `--disableSandbox` or `--requireSandbox` token on every Linux
+//! production launch; direct controller invocations with no token still take
+//! the legacy route and fail closed here too. Sandbox2 children
+//! (`ML_SANDBOXED=1`) never install this filter regardless of this value.
+inline constexpr bool TERMINATE_ON_DEGRADED_SECCOMP_FAILURE{true};
+
+//! Maps a filter installation outcome to the action the pytorch_inference
+//! startup path must take before model IO. \p terminateOnFailure is normally
+//! TERMINATE_ON_DEGRADED_SECCOMP_FAILURE above.
+//! This decision only ever applies to a launch that installs its own
+//! in-process filter at all - see sandbox2LaunchedChild() and
+//! applyInProcessSeccompFilter() below.
 inline EDegradedModeAction decideDegradedModeAction(ESystemCallFilterInstallOutcome outcome,
                                                     bool terminateOnFailure) {
     if (outcome == ESystemCallFilterInstallOutcome::E_Installed || !terminateOnFailure) {
@@ -117,8 +113,8 @@ inline EDegradedModeAction decideDegradedModeAction(ESystemCallFilterInstallOutc
 //! before initIo() ran" — with an explicit signal a test or observer can
 //! assert on directly. Returns empty when installation did not succeed so
 //! this marker can never falsely attest a filter that isn't there.
-//! Terminate-before-initIo() applies only when decideDegradedModeAction()
-//! is called with terminateOnFailure true (not today's production default).
+//! Terminate-before-initIo() applies when decideDegradedModeAction() is
+//! called with terminateOnFailure true (TERMINATE_ON_DEGRADED_SECCOMP_FAILURE).
 //! Logged over the existing per-process log pipe; this is not a new startup
 //! channel.
 //!
@@ -188,7 +184,7 @@ struct SInProcessFilterResult {
 //!        marker is produced, regardless of what an installation attempt
 //!        would have returned. Installing an in-process filter from inside
 //!        an already-sandboxed environment can fail (which would kill every
-//!        enforced-route launch once TERMINATE_ON_DEGRADED_SECCOMP_FAILURE is activated) or
+//!        enforced-route launch when TERMINATE_ON_DEGRADED_SECCOMP_FAILURE is true) or
 //!        succeed and mislabel the launch as legacy.
 //! \param terminateOnFailure passed through to decideDegradedModeAction().
 //! \param installer invoked at most once; normally