From 47f23e4b9570b7808116e654839068a66cf05c46 Mon Sep 17 00:00:00 2001 From: Chris Cheng Date: Thu, 23 Jul 2026 21:07:05 +0800 Subject: [PATCH] Fix ClientProcess.kill delivering the signal as an int, not a string The API server reads the `.signal` XPC key as a string (via `xpc_dictionary_get_string`) and resolves it against the Linux signal table, but `ClientProcess.kill(_:)` wrote it with `set(key: .signal, value: Int64(signal))`. `xpc_dictionary_get_string` returns nil for an integer value, so every signal sent through this path was rejected with `invalidArgument: "missing signal in xpc message"`. This path is used both by the public `ContainerAPIClient` (`ClientProcess.kill`) and by the foreground CLI's signal forwarding, so `^C` on a `-t` container and `SIGWINCH` terminal-resize forwarding were both broken. Send the canonical signal name instead, matching the already-working `ContainerClient.kill` path. Because the server resolves names against the Linux table, sending the name (rather than the raw macOS number) also keeps signals whose numbers differ between macOS and Linux correct (e.g. host SIGUSR1=30 -> "USR1" -> Linux 10, instead of Linux SIGPWR=30). Falls back to the numeric string for real-time signals that have no platform name. Fixes #1941 Fixes #1876 Fixes #1747 Co-Authored-By: Claude Opus 4.8 Signed-off-by: Chris Cheng --- .../Client/ClientProcess.swift | 9 ++- .../SignalWireFormatTests.swift | 64 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 Tests/ContainerAPIServiceTests/SignalWireFormatTests.swift diff --git a/Sources/Services/ContainerAPIService/Client/ClientProcess.swift b/Sources/Services/ContainerAPIService/Client/ClientProcess.swift index 5a5f8543e..a6b8410cf 100644 --- a/Sources/Services/ContainerAPIService/Client/ClientProcess.swift +++ b/Sources/Services/ContainerAPIService/Client/ClientProcess.swift @@ -80,7 +80,14 @@ struct ClientProcessImpl: ClientProcess, Sendable { let request = XPCMessage(route: .containerKill) request.set(key: .id, value: containerId) request.set(key: .processIdentifier, value: id) - request.set(key: .signal, value: Int64(signal)) + // The server reads `.signal` as a string and resolves it against the Linux + // signal table, so send the canonical signal name rather than a raw integer. + // A bare Int64 is read back as an empty string and rejected with + // "missing signal in xpc message". Translating the host signal number to its + // name (instead of sending the number) also keeps signals whose numbers differ + // between macOS and Linux (e.g. SIGUSR1) correct. Fall back to the numeric + // string for signals with no platform name (e.g. real-time signals). + request.set(key: .signal, value: Signal.platformName(signal) ?? "\(signal)") try await xpcClient.send(request) } diff --git a/Tests/ContainerAPIServiceTests/SignalWireFormatTests.swift b/Tests/ContainerAPIServiceTests/SignalWireFormatTests.swift new file mode 100644 index 000000000..0779bee69 --- /dev/null +++ b/Tests/ContainerAPIServiceTests/SignalWireFormatTests.swift @@ -0,0 +1,64 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Containerization +import Darwin +import Testing + +/// Guards the client -> server signal contract that `ClientProcess.kill(_:)` +/// depends on. The client holds a raw host (macOS) signal number and must put a +/// value on the wire that the server resolves back to the intended signal via +/// `Signal(_:)` (default Linux table), matching `RuntimeService.kill`. +/// +/// Regression coverage for the "missing signal in xpc message" bug where the +/// client sent a bare integer that the server read as a string (issues #1941, +/// #1876, #1747). +struct SignalWireFormatTests { + /// Exactly the expression `ClientProcess.kill(_:)` puts on the wire. + private func wireValue(_ signal: Int32) -> String { + Signal.platformName(signal) ?? "\(signal)" + } + + /// Exactly how the server resolves it (see `RuntimeService.kill`). + private func resolved(_ signal: Int32) throws -> Int32 { + try Signal(wireValue(signal)).rawValue + } + + @Test func commonSignalsRoundTrip() throws { + // Signals that share a number across macOS and Linux. + #expect(try resolved(SIGINT) == 2) // ^C (#1876) + #expect(try resolved(SIGWINCH) == 28) // terminal resize (#1747) + #expect(try resolved(SIGKILL) == 9) + #expect(try resolved(SIGTERM) == 15) + #expect(try resolved(SIGHUP) == 1) + } + + @Test func platformDivergentSignalTranslatesByName() throws { + // macOS SIGUSR1 == 30, Linux SIGUSR1 == 10. Sending the name (not the + // raw number) is what keeps this correct: a raw "30" would land on the + // container as Linux signal 30 (SIGPWR). + #expect(SIGUSR1 == 30) + #expect(wireValue(SIGUSR1) == "USR1") + #expect(try resolved(SIGUSR1) == 10) + } + + @Test func nonEmptyWireValue() { + // The original bug delivered an empty/absent string. Whatever we send + // must be a non-empty string the server can parse. + #expect(!wireValue(SIGINT).isEmpty) + #expect((try? Signal(wireValue(SIGINT))) != nil) + } +}