(feat) early snap support by zyga · Pull Request #1238 · NVIDIA/OpenShell

zyga · 2026-05-07T16:28:10Z

Summary

This pull request adds early support for snap packaging so that openshell can be built in CI and eventually distributed from the snap store.

Related Issue

Changes

add mise tasks and scaffolding for snap packaging without invoking all of snapcraft yet (that is coming as a follow-up).

Testing

mise run pre-commit passes
Unit tests added/updated
E2E tests added/updated (if applicable)

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (if applicable)

copy-pr-bot · 2026-05-07T16:28:14Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-actions · 2026-05-07T16:28:21Z

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

alexclewontin · 2026-05-07T16:30:00Z

I will put another set of eyes on this from a snapping perspective

zyga · 2026-05-07T16:30:10Z

With this I can get a working VM sandbox on Ubuntu 26.04. My next steps are to propose a full snapcraft.yaml and then a full publishing pipeline.

zyga · 2026-05-07T16:30:43Z

I have read the DCO document and I hereby sign the DCO.

zyga · 2026-05-07T16:31:10Z

recheck

blkerby · 2026-05-07T17:19:07Z

 path = "src/main.rs"

+[features]
+SNAP_SANDBOX = []


Cargo feature names would normally be lowercase, so snap_sandbox or snap-sandbox.

ack, I'll update this in the next round

This is now removed along with fs changes.

blkerby · 2026-05-07T22:13:41Z

+            fs::symlink_metadata(&link)
+                .expect("stat symlink")
+                .file_type()
+                .is_symlink()


Looks like this is checking that the symlink is still a symlink. I imagine we want to validate the owner/group of the file that the symlink references, to make sure the chown did not affect it?

This is now removed along with fs changes.

blkerby · 2026-05-07T23:19:01Z

+        let mut lines = Vec::new();
+        for existing in contents.lines() {
+            if exists(existing) {
+                if !replaced {


Nit: this is going out of our way to only replace the first matching line. If there's not a specific reason for doing this, it could be simpler to just replace all matching lines. Alternatively, if we're only expecting a single match, we could give an error if a second match is found. I think this function is only used to search for a specific user or group in passwd, group, shadow, and gshadow, in which case it would be a misconfiguration for the same user or group to appear twice in the same file?

Ditto, removed.

drew

modify the VM driver to account for the snap sandbox limiting access to arbitrary chown to user/group 10001 - instead use the special snap_daemon user that is allowed by the sandbox

change the unpacked rootfs so that /sandbox in the rootfs is owned by that new snapd_daemon user UID/GID while still being called sandbox in the vm

it's a bit unfortunate that these permission changes are needed in the first place. this is a reflection of the underlying rootfs implementation, not this pr though.

ideally we'd fix it upstream using a disk image owned by the guest so you don't need to worry about syncing permissions. i don't think we should have special features for snap compiled into openshell.

to get this pr merged without blocking on a vm fix, i propose switching this pr to use the docker driver by default for now. i'll followup with a proper fix to the microvm, and then we can re-enabled the vm on the snap install.

we actually introduced an auto detection mechanism into the gateway so it will automatically use docker if it's available on the system. that means you can simply remove OPENSHELL_DRIVERS from the app yaml and it should auto select docker.

drew · 2026-05-08T05:15:59Z

Prefer to keep the root of the repo as clean as possible. I suggest adding a short blurb about snap to architecture/build.md and then adding more detailed information about builds to deploy/snap/README.md.

Note that snapcraft packaging requires either a top-level snapcraft.yaml or snap/snapcraft.yaml file.

I'll add the pass to architecture/build.md when I add a full snapcraft.yaml file

drew · 2026-05-08T05:17:19Z

+	cat >"$output" <<'EOF'
+  openshell:
+    command: bin/openshell
+    plugs:
+      - home
+      - network
+      - ssh-keys
+      - system-observe
+  gateway-vm:
+    command: bin/openshell-gateway
+    daemon: simple
+    refresh-mode: endure
+    environment:
+      OPENSHELL_BIND_ADDRESS: 127.0.0.1
+      OPENSHELL_SERVER_PORT: 17670
+      OPENSHELL_DB_URL: "sqlite:$SNAP_COMMON/gateway.db?mode=rwc"
+      OPENSHELL_GRPC_ENDPOINT: http://host.containers.internal:17670
+      OPENSHELL_DISABLE_TLS: true
+      OPENSHELL_DRIVERS: vm
+      OPENSHELL_DRIVER_DIR: "$SNAP/bin"
+      OPENSHELL_SANDBOX_IMAGE: ghcr.io/nvidia/openshell-community/sandboxes/base:latest
+      OPENSHELL_SANDBOX_IMAGE_PULL_POLICY: IfNotPresent
+      OPENSHELL_SANDBOX_SSH_PORT: 2222
+      OPENSHELL_SSH_GATEWAY_HOST: 127.0.0.1
+      OPENSHELL_SSH_GATEWAY_PORT: 8080
+      # Candidates for snap configuration, perhaps.
+      OPENSHELL_VM_DRIVER_VCPUS: 2
+      OPENSHELL_VM_DRIVER_MEM_MIB: 2084
+      OPENSHELL_VM_DRIVER_STATE_DIR: "$SNAP_COMMON/vm-driver"
+      # Used for creating and locating certain sockets.
+      XDG_RUNTIME_DIR: "$SNAP_COMMON/"
+
+    plugs:
+      - kvm
+      - log-observe
+      - mount-observe
+      - network
+      - network-bind
+      - ssh-keys
+      - system-observe
+EOF
+}


this might be easier to manage as a tpl file in deploy/snap that gets read in and interpolated from this script.

Ack.

This is still temporary on the way to a standard snapcraft file but I will quickly make the changes.

drew · 2026-05-08T05:18:59Z

+      - network
+      - ssh-keys
+      - system-observe
+  gateway-vm:


Suggested change

gateway-vm:

openshell-gateway:

I think openshell-gateway would be the more canonical name for the component

Right, I called it that because it was the vm gateway but yeah I see your point.

Perhaps the app can be called just gateway. Then the systemd service is snap.openshell.gateway.service which arguably looks nice.

I made it just gateway for the reason I explained in a comment elsewhere here (the service name is snap.openshell.gateway.service) with this approach.

drew · 2026-05-08T05:32:45Z

+      OPENSHELL_SANDBOX_IMAGE_PULL_POLICY: IfNotPresent
+      OPENSHELL_SANDBOX_SSH_PORT: 2222
+      OPENSHELL_SSH_GATEWAY_HOST: 127.0.0.1
+      OPENSHELL_SSH_GATEWAY_PORT: 8080


Suggested change

OPENSHELL_SSH_GATEWAY_PORT: 8080

we can remove this one

drew · 2026-05-08T05:39:26Z

+prepare_stage_dir() {
+	local dir="$1"
+
+	case "$dir" in
+	"" | "/" | "." | "..")
+		echo "error: refusing unsafe OPENSHELL_SNAP_STAGE_DIR: '${dir}'" >&2
+		exit 2
+		;;
+	esac
+
+	mkdir -p "$dir"
+	find "$dir" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +
+}


Suggested change

prepare_stage_dir() {

local dir="$1"

case "$dir" in

"" | "/" | "." | "..")

echo "error: refusing unsafe OPENSHELL_SNAP_STAGE_DIR: '${dir}'" >&2

exit 2

;;

esac

mkdir -p "$dir"

find "$dir" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +

}

prepare_stage_dir() {

local dir="$1"

if [[ -z "${dir}" ]]; then

echo "Refusing empty snap stage directory" >&2

exit 1

fi

mkdir -p "${dir}"

local canonical_dir

local canonical_repo_root

canonical_dir="$(cd "${dir}" && pwd -P)"

canonical_repo_root="$(cd "${repo_root}" && pwd -P)"

if [[ "${canonical_dir}" == "/" \

|| "${canonical_dir}" == "${canonical_repo_root}" \

|| "${canonical_repo_root}" == "${canonical_dir}/"* ]]; then

echo "Refusing unsafe snap stage directory: ${dir} resolved to ${canonical_dir}" >&2

exit 1

fi

find "${canonical_dir}" -mindepth 1 -maxdepth 1 -exec rm -rf {} +

}

might be a little safer

zyga · 2026-05-08T06:56:14Z

to get this pr merged without blocking on a vm fix, i propose switching this pr to use the docker driver by default for now. i'll followup with a proper fix to the microvm, and then we can re-enabled the vm on the snap install.

This will only work with the docker snap though, as snapd doesn't have a way for allowing communication with non-snap version of docker yet.

we actually introduced an auto detection mechanism into the gateway so it will automatically use docker if it's available on the system. that means you can simply remove OPENSHELL_DRIVERS from the app yaml and it should auto select docker.

Right but there are a number of storage elements specific to the VM driver. Is the architecture that a single gateway is multi-driver or is there always a 1:1 pairing?

The reason I set up several variables there is to get the microvm driver to use the right storage location and work well across refreshes.

If we now make the gateway generic, I'm not sure if we keep those variables intact or do something else?

this is a stepping stone towards a full snapcraft based build, by allowing us to reuse the existing mise-based build-system to construct the binaries and merely assemble the snap with "snap pack". signed-off-by: zygmunt krynicki <zygmunt.krynicki@canonical.com>

Document how to build and use a locally-built snap package of openshell, as well as how it works internally. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

zyga · 2026-05-08T08:20:56Z

@drew please re-review. I force pushed with the requested docker driver and confirmed this all works.

For gateway setup, note that in the snap the gateway runs as root. There's no mtls as gateway and the user run as different users so either the mlts keys are public on the host or a user would have to manually copy them using sudo.

Copilot

Pull request overview

Adds initial snap packaging scaffolding so OpenShell can be staged/packed as a snap from prebuilt binaries, including a snap-compatible version string and mise task hooks.

Changes:

Extend tasks/scripts/release.py to compute/print a snap-friendly version (--snap, VERSION_SNAP).
Add a hand-staged snap pack script (tasks/scripts/package-snap.sh) plus mise tasks to build/stage/pack.
Add snap metadata template + build/run documentation under deploy/snap/.

Reviewed changes

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

Show a summary per file

File	Description
tasks/scripts/release.py	Adds `snap` version computation and CLI/env output support.
tasks/scripts/package-snap.sh	New staging + optional packing workflow for a hand-authored snap payload.
tasks/package.toml	Adds mise tasks for snap packaging/staging.
tasks/ci.toml	Adds a dedicated Rust build task for snap-consumed release binaries.
deploy/snap/README.md	Documents prerequisites and steps to build, stage, pack, and run the snap.
deploy/snap/meta/snap.yaml.in	Introduces snap metadata template (apps, plugs, service env).

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

zyga · 2026-05-08T08:59:07Z

+snap pack "$snap_root" "$output_dir"
+echo "Wrote ${output_dir}/${APP_NAME}_${OPENSHELL_SNAP_VERSION}_${OPENSHELL_SNAP_ARCH}.snap"


Yes it does:

$ snap pack --help Usage: snap pack [pack-OPTIONS] [<snap-dir>] [<target-dir>] The pack command packs the given snap-dir as a snap and writes the result to target-dir. If target-dir is omitted, the result is written to current directory. If both source-dir and target-dir are omitted, the pack command packs the current directory. The default file name for a snap can be derived entirely from its snap.yaml, but in some situations it's simpler for a script to feed the filename in. In those cases, --filename can be given to override the default. If this filename is not absolute it will be taken as relative to target-dir. When used with --check-skeleton, pack only checks whether snap-dir contains valid snap metadata and raises an error otherwise. Application commands listed in snap metadata file, but appearing with incorrect permission bits result in an error. Commands that are missing from snap-dir are listed in diagnostic messages. [pack command options] --check-skeleton Validate snap-dir metadata only --filename= Output to this filename (default: <name>_<version>_<architecture>.snap) --compression= Compression to use (e.g. xz or lzo)

+    # Snap versions cannot contain '+' and are limited to 32 characters.
+    snap_version = re.sub(r"\.d\d{8}$", "", docker_version)


+    # Snap versions cannot contain '+' and are limited to 32 characters.
+    snap_version = re.sub(r"\.d\d{8}$", "", docker_version)
+    if len(snap_version) > 32:
+        raise ValueError(f"snap version must be at most 32 characters: {snap_version}")
+


zyga · 2026-05-08T09:00:16Z

+# Building a snap package
+
+OpenShell snap packages are hand-staged and packed with `snap pack`. They do
+not use Snapcraft yet.
+


I've updated the description.

+- Linux on `amd64` or `arm64`
+- `snap` from `snapd`
+- Docker from the Docker snap (`sudo snap install docker`)
+- `mise`


Add snapcraft.yaml that builds OpenShell with Snapcraft's Rust plugin and core24 platforms syntax, and update deploy/snap README to document Snapcraft-first packaging while preserving the staged helper flow. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

zyga changed the title ~~Feature/upstream snap~~ (feat) early snap support May 7, 2026

blkerby reviewed May 7, 2026

View reviewed changes

drew reviewed May 8, 2026

View reviewed changes

zyga added 2 commits May 8, 2026 10:17

docs(package): document snap packaging

0de8016

Document how to build and use a locally-built snap package of openshell, as well as how it works internally. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

zyga force-pushed the feature/upstream-snap branch from fef1a99 to 0de8016 Compare May 8, 2026 08:19

zyga requested a review from drew May 8, 2026 08:21

zyga marked this pull request as ready for review May 8, 2026 08:49

zyga requested a review from mrunalp as a code owner May 8, 2026 08:49

Copilot AI review requested due to automatic review settings May 8, 2026 08:49

zyga requested review from a team, derekwaynecarr and maxamillion as code owners May 8, 2026 08:49

Copilot started reviewing on behalf of zyga May 8, 2026 08:49 View session

zyga requested a review from blkerby May 8, 2026 08:51

Copilot AI reviewed May 8, 2026

View reviewed changes

-prepare_stage_dir() {
-	local dir="$1"
-	case "$dir" in
-	"" | "/" | "." | "..")
-		echo "error: refusing unsafe OPENSHELL_SNAP_STAGE_DIR: '${dir}'" >&2
-		exit 2
-		;;
-	esac
-	mkdir -p "$dir"
-	find "$dir" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +
-}
+prepare_stage_dir() {
+    local dir="$1"
+    if [[ -z "${dir}" ]]; then
+        echo "Refusing empty snap stage directory" >&2
+        exit 1
+    fi
+    mkdir -p "${dir}"
+    local canonical_dir
+    local canonical_repo_root
+    canonical_dir="$(cd "${dir}" && pwd -P)"
+    canonical_repo_root="$(cd "${repo_root}" && pwd -P)"
+    if [[ "${canonical_dir}" == "/" \
+        || "${canonical_dir}" == "${canonical_repo_root}" \
+        || "${canonical_repo_root}" == "${canonical_dir}/"* ]]; then
+        echo "Refusing unsafe snap stage directory: ${dir} resolved to ${canonical_dir}" >&2
+        exit 1
+    fi
+    find "${canonical_dir}" -mindepth 1 -maxdepth 1 -exec rm -rf {} +
+}

		snap pack "$snap_root" "$output_dir"
		echo "Wrote ${output_dir}/${APP_NAME}_${OPENSHELL_SNAP_VERSION}_${OPENSHELL_SNAP_ARCH}.snap"

		# Snap versions cannot contain '+' and are limited to 32 characters.
		snap_version = re.sub(r"\.d\d{8}$", "", docker_version)

Conversation

zyga commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexclewontin commented May 7, 2026

Uh oh!

zyga commented May 7, 2026

Uh oh!

zyga commented May 7, 2026

Uh oh!

zyga commented May 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drew left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drew May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zyga commented May 8, 2026

Uh oh!

zyga commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

zyga commented May 7, 2026 •

edited

Loading

github-actions Bot commented May 7, 2026 •

edited

Loading

drew left a comment •

edited

Loading

drew May 8, 2026 •

edited

Loading