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

bin/controller/CProcessSpawnerRouter.cc

@@ -105,9 +105,13 @@ std::string jsonEscape(const std::string& s) {
 //! directory and report an empty deployment_id on exactly the degraded and
 //! fail_closed modes the signal exists to make debuggable.
 //! Returns "" when no path-bearing option was present at all.
-std::string deriveDeploymentId(const ml::controller::CProcessSpawnerRouter::TStrVec& args) {
+std::string trustedTmpDirFromEnvironment() {
     const char* tmpDirEnv{::getenv("TMPDIR")};
-    const std::string trustedTmpDir{tmpDirEnv != nullptr ? tmpDirEnv : "/tmp"};
+    return tmpDirEnv != nullptr ? std::string{tmpDirEnv} : std::string{"/tmp"};
+}
+
+std::string deriveDeploymentId(const ml::controller::CProcessSpawnerRouter::TStrVec& args) {
+    const std::string trustedTmpDir{trustedTmpDirFromEnvironment()};
     // validateChildIpcLaunchSpec() does live ::realpath() calls, which
     // require $TMPDIR/ml-child-ipc/<child-id> to already exist. This is the
     // *other* production call site that reaches that function (the one
@@ -135,7 +139,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 {
@@ -242,6 +254,12 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
     // processes (autodetect etc.) pay no ::realpath() cost.
     const std::string deploymentId{sandboxEligible ? deriveDeploymentId(args)
                                                    : std::string()};
+    const std::string childIpcRoot{
+        sandboxEligible
+            ? sandbox::perChildIpcRootFromArgs(trustedTmpDirFromEnvironment(), args)
+            : std::string()};
+    const std::string* childIpcRootPtr{
+        sandboxEligible && childIpcRoot.empty() == false ? &childIpcRoot : nullptr};
 
     m_LastSpawnFailureReason.clear();
     bool spawned{false};
@@ -260,7 +278,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.
@@ -287,6 +305,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
@@ -299,7 +318,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);
             landlockFallback = true;
             break;
         }
@@ -334,6 +353,9 @@ bool CProcessSpawnerRouter::spawn(ERoute route,
 
     if (sandboxEligible) {
         this->emitLaunchSignal(route, legacyReason, deploymentId, args, spawned, landlockFallback);
+        if (spawned == false && childIpcRoot.empty() == false) {
+            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 <sandbox/CSandboxedProcessSpawner.h>
 
@@ -211,6 +212,8 @@ class CProcessSpawnerRouter {
 
     //! See lastSpawnFailureReason().
     std::string m_LastSpawnFailureReason;
+
+    std::shared_ptr<sandbox::CChildIpcDirectoryReaper> m_ChildIpcReaper;
 };
 
 } // namespace controller

bin/pytorch_inference/Main.cc

@@ -356,30 +356,6 @@ 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 -
@@ -394,7 +370,7 @@ int main(int argc, char** argv) {
     // 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

@@ -134,14 +134,14 @@ present.
         - 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.
+Hard termination on a failed in-process seccomp installation
+(`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,58 @@
+/*
+ * 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 <core/ImportExport.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.
+//!
+//! Controller crash can still leave empty directories; there is no startup sweep because
+//! that would race another controller on the same $TMPDIR.
+class CORE_EXPORT 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) {