Skip to content

vminitd: Scrub envvars printed in debug log #521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dcantah wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

vminitd: Scrub envvars printed in debug log #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions vminitd/Sources/vminitd/LogRedaction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//===----------------------------------------------------------------------===//
// 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 ContainerizationOCI

/// Redacts environment variable values from a list of env vars in "KEY=VALUE" format.
/// Returns the env vars with values replaced by "<redacted>".
func redactEnvValues(_ env: [String]) -> [String] {
Copy link
Contributor

@manojmahapatra manojmahapatra Feb 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I was reading the Docker Compose secrets docs and wondered, should we adopt a policy of never logging any env/argv values (even at debug), that is always redact env + args/commandLine? Or is the intent to only redact env and leave argv visible for debugging?

Copy link
Contributor

@jglogan jglogan Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. We should clearly understand where app secret values could be stored at rest:

  • .env files or anything similar - these are owned by the user so container has no responsibility for them.
  • Container configuration files in the application data store. In Docker, it appears that these are stored as plaintext under /var/lib/docker.
  • boot logs - perhaps redact entirely, or log them at trace level (which means we shouldn't see them until we add log level control, at which time we could emit a warning if trace is selected)
  • application logs - the user's responsibility

Copy link
Contributor

@manojmahapatra manojmahapatra Feb 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I agree w/ the boot logs part which are written today by default. As you said gate boot logs and if I'm thinking it right;
(1) dont set config.bootLog = BootLog.file(...) by default
(2) only write boot logs if the user explicitly asks?
(3) move sensitive guest logs to trace by making the spec/argv logs trace-level.

Copy link
Member Author

@dcantah dcantah Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should emit boot logs always, it's too useful for diagnostics. I think John meant redact the container configs in them entirely possibly

Copy link
Contributor

@manojmahapatra manojmahapatra Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see what you mean. yeah, config only redaction will be a balanced approach.

env.map { entry in
if let equalsIndex = entry.firstIndex(of: "=") {
let key = entry[..<equalsIndex]
return "\(key)=<redacted>"
}
return entry
}
}

/// A log-safe representation of a Process that redacts environment variable values.
struct RedactedProcess: CustomStringConvertible {
let process: ContainerizationOCI.Process

var description: String {
var copy = process
copy.env = redactEnvValues(copy.env)
return "\(copy)"
}
}

/// A log-safe representation of a Spec that redacts environment variable values.
struct RedactedSpec: CustomStringConvertible {
let spec: ContainerizationOCI.Spec

var description: String {
var copy = spec
if var process = copy.process {
process.env = redactEnvValues(process.env)
copy.process = process
}
return "\(copy)"
}
}

extension ContainerizationOCI.Spec {
var redacted: RedactedSpec {
RedactedSpec(spec: self)
}
}

extension ContainerizationOCI.Process {
var redacted: RedactedProcess {
RedactedProcess(process: self)
}
}
4 changes: 2 additions & 2 deletions vminitd/Sources/vminitd/ManagedContainer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ actor ManagedContainer {
path: Self.craftBundlePath(id: id),
spec: spec
)
log.debug("created bundle with spec \(spec)")
log.debug("created bundle with spec \(spec.redacted)")

let cgManager = Cgroup2Manager(
group: URL(filePath: cgroupsPath),
Expand Down Expand Up @@ -163,7 +163,7 @@ extension ManagedContainer {
stdio: HostStdio,
process: ContainerizationOCI.Process
) throws {
log.debug("creating exec process with \(process)")
log.debug("creating exec process with \(process.redacted)")

// Write the process config to the bundle, and pass this on
// over to ManagedProcess to deal with.
Expand Down