From cec005d47a26923f874ca20d709b5466c35f9fb6 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:54:51 +0700 Subject: [PATCH] Redact environment variable values in vminitd debug logs vminitd logged the full OCI spec/process at debug level, exposing secrets passed through environment variables (#518). Add redactingEnvironmentValues() to ContainerizationOCI's Spec, Process, Hook, and Hooks - masking values while keeping variable names - and use it at both log sites in ManagedContainer. Co-Authored-By: Claude Fable 5 --- .../ContainerizationOCI/Spec+Redaction.swift | 78 ++++++++++++ .../SpecRedactionTests.swift | 113 ++++++++++++++++++ .../VminitdCore/ManagedContainer.swift | 4 +- 3 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 Sources/ContainerizationOCI/Spec+Redaction.swift create mode 100644 Tests/ContainerizationOCITests/SpecRedactionTests.swift diff --git a/Sources/ContainerizationOCI/Spec+Redaction.swift b/Sources/ContainerizationOCI/Spec+Redaction.swift new file mode 100644 index 000000000..bf9479168 --- /dev/null +++ b/Sources/ContainerizationOCI/Spec+Redaction.swift @@ -0,0 +1,78 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the Containerization 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. +//===----------------------------------------------------------------------===// + +extension Spec { + /// Returns a copy of the spec that is safe to include in log output: + /// the environment variable values of the container process and of all + /// lifecycle hooks are replaced with ``, keeping only the + /// variable names. Environment variables routinely carry secrets, and + /// logs must not expose them. + public func redactingEnvironmentValues() -> Spec { + var copy = self + copy.process = copy.process?.redactingEnvironmentValues() + copy.hooks = copy.hooks?.redactingEnvironmentValues() + return copy + } +} + +extension Process { + /// Returns a copy of the process whose environment variable values are + /// replaced with ``, keeping only the variable names. Use this + /// when rendering a process into log output so secrets injected through + /// the environment are not exposed. + public func redactingEnvironmentValues() -> Process { + var copy = self + copy.env = redactedEnvironment(copy.env) + return copy + } +} + +extension Hook { + /// Returns a copy of the hook whose environment variable values are + /// replaced with ``, keeping only the variable names. + public func redactingEnvironmentValues() -> Hook { + var copy = self + copy.env = redactedEnvironment(copy.env) + return copy + } +} + +extension Hooks { + /// Returns a copy of the hooks whose environment variable values are + /// replaced with ``, keeping only the variable names. + public func redactingEnvironmentValues() -> Hooks { + var copy = self + copy.prestart = copy.prestart.map { $0.redactingEnvironmentValues() } + copy.createRuntime = copy.createRuntime.map { $0.redactingEnvironmentValues() } + copy.createContainer = copy.createContainer.map { $0.redactingEnvironmentValues() } + copy.startContainer = copy.startContainer.map { $0.redactingEnvironmentValues() } + copy.poststart = copy.poststart.map { $0.redactingEnvironmentValues() } + copy.poststop = copy.poststop.map { $0.redactingEnvironmentValues() } + return copy + } +} + +/// Replaces the value of every `NAME=value` entry with ``. Entries +/// without an `=` are kept as-is: they name variables to inherit and carry +/// no value of their own. +private func redactedEnvironment(_ env: [String]) -> [String] { + env.map { entry in + guard let separator = entry.firstIndex(of: "=") else { + return entry + } + return entry[.." + } +} diff --git a/Tests/ContainerizationOCITests/SpecRedactionTests.swift b/Tests/ContainerizationOCITests/SpecRedactionTests.swift new file mode 100644 index 000000000..a83c3f6fb --- /dev/null +++ b/Tests/ContainerizationOCITests/SpecRedactionTests.swift @@ -0,0 +1,113 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the Containerization 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 Foundation +import Testing + +@testable import ContainerizationOCI + +struct SpecRedactionTests { + @Test func processEnvironmentValuesAreRedacted() { + let process = ContainerizationOCI.Process( + args: ["python3", "-m", "http.server"], + cwd: "/app", + env: [ + "PATH=/usr/local/bin:/usr/bin", + "MY_SUPER_SECRET_PASSWORD=guest", + "EMPTY=", + "TOKEN=abc=def", + "INHERITED_NO_VALUE", + ] + ) + + let redacted = process.redactingEnvironmentValues() + + #expect( + redacted.env == [ + "PATH=", + "MY_SUPER_SECRET_PASSWORD=", + "EMPTY=", + "TOKEN=", + "INHERITED_NO_VALUE", + ]) + // Everything except env is untouched. + #expect(redacted.args == process.args) + #expect(redacted.cwd == process.cwd) + // The original is not mutated. + #expect(process.env.contains("MY_SUPER_SECRET_PASSWORD=guest")) + } + + @Test func redactedProcessRendersWithoutSecretValues() { + // Mirrors the report in issue #518: interpolating the process into a + // log message must not expose environment variable values. + let process = ContainerizationOCI.Process( + args: ["echo", "hello"], + env: ["MY_OTHER_SECRET_PASSWORD=abc123"] + ) + + let logged = "\(process.redactingEnvironmentValues())" + + #expect(!logged.contains("abc123")) + #expect(logged.contains("MY_OTHER_SECRET_PASSWORD")) + } + + @Test func specRedactsProcessAndHookEnvironments() { + let hook = Hook( + path: "/usr/local/bin/hook", + args: ["hook"], + env: ["HOOK_SECRET=hunter2"], + timeout: nil + ) + let spec = Spec( + hooks: Hooks( + prestart: [hook], + createRuntime: [hook], + createContainer: [hook], + startContainer: [hook], + poststart: [hook], + poststop: [hook] + ), + process: ContainerizationOCI.Process(env: ["MY_SUPER_SECRET_PASSWORD=guest"]), + hostname: "web", + mounts: [Mount(type: "proc", source: "proc", destination: "/proc")] + ) + + let redacted = spec.redactingEnvironmentValues() + + let logged = "\(redacted)" + #expect(!logged.contains("guest")) + #expect(!logged.contains("hunter2")) + #expect(logged.contains("MY_SUPER_SECRET_PASSWORD")) + #expect(logged.contains("HOOK_SECRET")) + + // Everything except environments is untouched. + #expect(redacted.hostname == spec.hostname) + #expect(redacted.mounts.count == spec.mounts.count) + #expect(redacted.process?.env == ["MY_SUPER_SECRET_PASSWORD="]) + #expect(redacted.hooks?.prestart.first?.env == ["HOOK_SECRET="]) + #expect(redacted.hooks?.poststop.first?.env == ["HOOK_SECRET="]) + // The original is not mutated. + #expect(spec.process?.env == ["MY_SUPER_SECRET_PASSWORD=guest"]) + } + + @Test func specWithoutProcessOrHooksIsUnchanged() { + let spec = Spec(version: "1.2.0") + let redacted = spec.redactingEnvironmentValues() + #expect(redacted.process == nil) + #expect(redacted.hooks == nil) + #expect(redacted.version == spec.version) + } +} diff --git a/vminitd/Sources/VminitdCore/ManagedContainer.swift b/vminitd/Sources/VminitdCore/ManagedContainer.swift index 545046a8f..77e96b693 100644 --- a/vminitd/Sources/VminitdCore/ManagedContainer.swift +++ b/vminitd/Sources/VminitdCore/ManagedContainer.swift @@ -55,7 +55,7 @@ public actor ManagedContainer { path: Self.craftBundlePath(id: id), spec: spec ) - log.debug("created bundle with spec \(spec)") + log.debug("created bundle with spec \(spec.redactingEnvironmentValues())") let cgManager = Cgroup2Manager( group: URL(filePath: cgroupsPath), @@ -164,7 +164,7 @@ extension ManagedContainer { stdio: HostStdio, process: ContainerizationOCI.Process ) throws { - log.debug("creating exec process with \(process)") + log.debug("creating exec process with \(process.redactingEnvironmentValues())") // Write the process config to the bundle, and pass this on // over to ManagedProcess to deal with.