From bd2c228b57dddcf31edb01db432e908d10aa5bab Mon Sep 17 00:00:00 2001
From: mazdak <mazdak@mac.com>
Date: Fri, 5 Sep 2025 23:49:54 -0400
Subject: [PATCH] DefaultCommand signal behavior improvements for plugins
 (#570)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update

## Description
Correct signal semantics for plugins: Container binary currently execs
into plugin binaries. If the parent CLI keeps SIGINT/SIGTERM handlers
installed, it can intercept/alter signal behavior intended for the
plugin (e.g., preventing graceful shutdown in foreground workflows).

## Motivation and Context
During the development of a plugin (docker compose compatibility
plugin), I encountered a major issues where CTRL-C (SIGTERM) was not
being sent to my plugin. CLI plugins, especially those that have long
running tasks need a way to handle signals from the OS. Current, we exec
into plugin binaries. If the parent CLI keeps SIGINT/SIGTERM handlers
installed, it can intercept/alter signal behavior intended for the
plugin (e.g., preventing graceful shutdown in foreground workflows).

### What we changed:
- Signals handed back to plugins:
- DefaultCommand resets SIGINT/SIGTERM to defaults immediately before
exec’ing the plugin.
- Rationale: since exec replaces the process image, signals should be
delivered to (and handled by) the plugin without parent interference.
  - Non‑plugin commands remain unaffected by this change.
  - Compatibility: No change to plugin ABI or exec flow.

### Alternatives considered:
- Supervising child instead of exec: central forwarding of signals from
parent to plugin. Rejected for now to avoid changing process tree/stdio
semantics; resetting to defaults before exec preserves current model
while fixing signal interference.

## Testing
- [X] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
---
 Sources/CLI/DefaultCommand.swift | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Sources/CLI/DefaultCommand.swift b/Sources/CLI/DefaultCommand.swift
index a8c5f020..30b55d1d 100644
--- a/Sources/CLI/DefaultCommand.swift
+++ b/Sources/CLI/DefaultCommand.swift
@@ -17,6 +17,7 @@
 import ArgumentParser
 import ContainerClient
 import ContainerPlugin
+import Darwin
 
 struct DefaultCommand: AsyncParsableCommand {
     static let configuration = CommandConfiguration(
@@ -48,7 +49,17 @@ struct DefaultCommand: AsyncParsableCommand {
         guard let plugin = pluginLoader?.findPlugin(name: command), plugin.config.isCLI else {
             throw ValidationError("failed to find plugin named container-\(command)")
         }
+        // Before execing into the plugin, restore default SIGINT/SIGTERM so the plugin can manage signals.
+        Self.resetSignalsForPluginExec()
         // Exec performs execvp (with no fork).
         try plugin.exec(args: remaining)
     }
 }
+
+extension DefaultCommand {
+    // Exposed for tests to verify signal reset semantics.
+    static func resetSignalsForPluginExec() {
+        signal(SIGINT, SIG_DFL)
+        signal(SIGTERM, SIG_DFL)
+    }
+}