Skip to content

Construct guest runc options rather than forwarding host options#225

Open
dmcgowan wants to merge 1 commit into
containerd:mainfrom
dmcgowan:shim-strip-options
Open

Construct guest runc options rather than forwarding host options#225
dmcgowan wants to merge 1 commit into
containerd:mainfrom
dmcgowan:shim-strip-options

Conversation

@dmcgowan

Copy link
Copy Markdown
Member

The runc Options proto mixes host-side shim config with guest-side
container config. Rather than filtering the incoming blob, always
construct a fresh Options for the guest with only the fields that are
meaningful inside the VM:

NoPivotRoot, NoNewKeyring forwarded
SystemdCgroup, Criu* dropped with a warning log (no systemd in
guest; checkpoint coordinated at VM level)
Root, BinaryName, dropped silently (host paths / identity,
ShimCgroup, IoUid/IoGid, meaningless inside the VM)
TaskApiAddress

An unrecognised options type is rejected with InvalidArgument rather
than forwarded opaquely; the shim knows the guest runtime.

Copilot AI review requested due to automatic review settings June 13, 2026 08:15
@dmcgowan dmcgowan changed the title construct guest runc options rather than forwarding host options Construct guest runc options rather than forwarding host options Jun 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents host-side runc task options from being forwarded into the VM by constructing a new, guest-safe runcOptions.Options Any for vminitd. It aims to forward only guest-meaningful fields, drop host-only fields, warn when certain dropped fields were set, and reject unsupported option types.

Changes:

  • Added guestRuncOptions to construct a fresh allow-listed runc options message for the guest and emit warnings for explicitly dropped fields.
  • Updated Create to forward the constructed guest options instead of the incoming host options blob.
  • Added unit tests covering nil/empty handling, allow-listed forwarding, stripping behavior, and unknown-type rejection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/shim/task/service.go Constructs guest-safe runc options and forwards those to the in-VM task service instead of forwarding host options directly.
internal/shim/task/options_test.go Adds tests validating allow-listing, stripping, nil/empty behavior, and unknown options handling for guestRuncOptions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/shim/task/service.go Outdated
Comment thread internal/shim/task/service.go
The runc Options proto mixes host-side shim config with guest-side
container config. Rather than filtering the incoming blob, always
construct a fresh Options for the guest with only the fields that are
meaningful inside the VM:

  NoPivotRoot, NoNewKeyring  forwarded
  SystemdCgroup, Criu*       dropped with a warning log (no systemd in
                             guest; checkpoint coordinated at VM level)
  Root, BinaryName,          dropped silently (host paths / identity,
  ShimCgroup, IoUid/IoGid,   meaningless inside the VM)
  TaskApiAddress

An unrecognised options type is rejected with InvalidArgument rather
than forwarded opaquely; the shim knows the guest runtime.

Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan dmcgowan force-pushed the shim-strip-options branch from eaaa301 to 14da49f Compare June 13, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants