Skip to content

Commit 151dfc3

Browse files
authored
Redact environment variable values in vminitd debug logs
vminitd logs the OCI spec and the exec process at debug level, so every NAME=value environment entry ended up in the boot log (#518). Environment variables routinely carry secrets. Conform Process and Hook to CustomStringConvertible with a rendering that masks environment values and keeps the names. Spec and Hooks inherit it, because reflection renders a nested value through its own description, so interpolating a spec is redacted whether or not the author knows to ask for it. The two existing log sites need no change. Codable is untouched: an encoded spec still carries the real values, and process.env still returns them.
1 parent 7800b46 commit 151dfc3

2 files changed

Lines changed: 189 additions & 0 deletions

File tree

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//===----------------------------------------------------------------------===//
2+
// Copyright © 2026 Apple Inc. and the Containerization project authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// https://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
//===----------------------------------------------------------------------===//
16+
17+
// Environment variables routinely carry secrets, so rendering a process or a
18+
// hook as text must not expose their values. These conformances make the
19+
// redacted form the *default* rendering rather than something a caller has to
20+
// opt into: any `\(spec)` or `\(process)`, in this repo or downstream, is safe
21+
// without the author knowing this file exists.
22+
//
23+
// Only the two types that own an `env` need conforming. Swift's reflection
24+
// based description uses a nested value's own `description`, so `Spec` and
25+
// `Hooks` inherit the redaction through the values they hold.
26+
//
27+
// This affects text rendering only. `Codable` is untouched, so an encoded spec
28+
// still carries the real values, and the unredacted environment remains
29+
// available to callers through `process.env`.
30+
31+
extension Process: CustomStringConvertible {
32+
public var description: String {
33+
var copy = self
34+
copy.env = redactingEnvironmentValues(copy.env)
35+
return describeFields(of: copy)
36+
}
37+
}
38+
39+
extension Hook: CustomStringConvertible {
40+
public var description: String {
41+
var copy = self
42+
copy.env = redactingEnvironmentValues(copy.env)
43+
return describeFields(of: copy)
44+
}
45+
}
46+
47+
/// Replaces the value of every `NAME=value` entry with `<redacted>`, keeping
48+
/// the name, which is still useful for seeing *which* variables were set.
49+
/// Entries without an `=` are kept as-is: they name a variable to inherit and
50+
/// carry no value of their own.
51+
private func redactingEnvironmentValues(_ env: [String]) -> [String] {
52+
env.map { entry in
53+
guard let separator = entry.firstIndex(of: "=") else {
54+
return entry
55+
}
56+
return entry[..<separator] + "=<redacted>"
57+
}
58+
}
59+
60+
/// Renders `TypeName(label: value, ...)`, the shape Swift's own description
61+
/// produces. Going through a mirror rather than listing the fields by hand
62+
/// keeps every field in the log line, and means a field added later shows up
63+
/// without anyone remembering to edit this file.
64+
private func describeFields<T>(of value: T) -> String {
65+
let fields = Mirror(reflecting: value).children.map { child in
66+
"\(child.label ?? "_"): \(String(describing: child.value))"
67+
}
68+
return "\(T.self)(\(fields.joined(separator: ", ")))"
69+
}
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
//===----------------------------------------------------------------------===//
2+
// Copyright © 2026 Apple Inc. and the Containerization project authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// https://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
//===----------------------------------------------------------------------===//
16+
17+
import Foundation
18+
import Testing
19+
20+
@testable import ContainerizationOCI
21+
22+
@Suite("Spec redaction")
23+
struct SpecRedactionTests {
24+
private static let secret = "hunter2"
25+
private static let hookSecret = "abc123"
26+
27+
private func spec() -> Spec {
28+
Spec(
29+
hooks: Hooks(
30+
prestart: [],
31+
createRuntime: [],
32+
createContainer: [],
33+
startContainer: [],
34+
poststart: [Hook(path: "/hook", args: [], env: ["HOOK_TOKEN=\(Self.hookSecret)"], timeout: nil)],
35+
poststop: []
36+
),
37+
process: Process(env: ["PATH=/usr/bin", "PASSWORD=\(Self.secret)", "INHERIT_ME"])
38+
)
39+
}
40+
41+
// The property that matters: a whole spec interpolated into a log line
42+
// must not carry the values, whether or not the author redacted it.
43+
@Test func interpolatingASpecNeverRendersEnvironmentValues() {
44+
let rendered = "\(spec())"
45+
#expect(!rendered.contains(Self.secret))
46+
#expect(!rendered.contains(Self.hookSecret))
47+
#expect(rendered.contains("PASSWORD=<redacted>"))
48+
#expect(rendered.contains("HOOK_TOKEN=<redacted>"))
49+
}
50+
51+
// print(), String(describing:) and String(reflecting:) all resolve through
52+
// description, so none of them is a way around the redaction.
53+
@Test func everyTextRenderingIsRedacted() {
54+
let s = spec()
55+
for rendered in ["\(s)", String(describing: s), String(reflecting: s)] {
56+
#expect(!rendered.contains(Self.secret))
57+
#expect(!rendered.contains(Self.hookSecret))
58+
}
59+
}
60+
61+
@Test func variableNamesSurviveSoLogsStayUseful() {
62+
let rendered = "\(spec())"
63+
#expect(rendered.contains("PATH="))
64+
#expect(rendered.contains("PASSWORD="))
65+
}
66+
67+
// A bare NAME names a variable to inherit from the parent and carries no
68+
// value, so there is nothing to hide and it passes through untouched.
69+
@Test func inheritedEntriesArePreserved() {
70+
#expect("\(spec())".contains("INHERIT_ME"))
71+
}
72+
73+
@Test(arguments: [
74+
"EMPTY=",
75+
"CONNECTION=postgres://user:pw@host/db?sslmode=require",
76+
])
77+
func valuesAreMaskedWholeIncludingAnyFurtherEquals(_ entry: String) {
78+
let name = String(entry[entry.startIndex..<entry.firstIndex(of: "=")!])
79+
let rendered = "\(Process(env: [entry]))"
80+
#expect(rendered.contains("\(name)=<redacted>"))
81+
if let value = entry.split(separator: "=", maxSplits: 1).last, entry.hasSuffix(String(value)), value != name {
82+
#expect(!rendered.contains(String(value)))
83+
}
84+
}
85+
86+
// Redaction is a rendering concern. Encoding must still produce the real
87+
// spec, or we would be corrupting what gets written to disk and sent to
88+
// the guest rather than just cleaning up a log line.
89+
@Test func encodingIsUnaffected() throws {
90+
let data = try JSONEncoder().encode(spec())
91+
let json = String(decoding: data, as: UTF8.self)
92+
#expect(json.contains(Self.secret))
93+
#expect(json.contains(Self.hookSecret))
94+
95+
let decoded = try JSONDecoder().decode(Spec.self, from: data)
96+
#expect(decoded.process?.env == ["PATH=/usr/bin", "PASSWORD=\(Self.secret)", "INHERIT_ME"])
97+
#expect(decoded.hooks?.poststart.first?.env == ["HOOK_TOKEN=\(Self.hookSecret)"])
98+
}
99+
100+
@Test func theValuesRemainAvailableToCallers() {
101+
#expect(spec().process?.env.contains("PASSWORD=\(Self.secret)") == true)
102+
}
103+
104+
@Test func renderingIsNonMutating() {
105+
let s = spec()
106+
_ = "\(s)"
107+
#expect(s.process?.env == ["PATH=/usr/bin", "PASSWORD=\(Self.secret)", "INHERIT_ME"])
108+
}
109+
110+
// Redacting must not cost the log line its other fields, which is the
111+
// reason description mirrors every field rather than listing a chosen few.
112+
@Test func theOtherFieldsAreStillRendered() {
113+
let rendered = "\(Process(args: ["/bin/sh"], cwd: "/work", env: ["A=b"], terminal: true))"
114+
#expect(rendered.contains("cwd:"))
115+
#expect(rendered.contains("/work"))
116+
#expect(rendered.contains("args:"))
117+
#expect(rendered.contains("/bin/sh"))
118+
#expect(rendered.contains("terminal:"))
119+
}
120+
}

0 commit comments

Comments
 (0)