From d231e94535b35f4bb7b4777489e6c3c589aee7f5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 19 Sep 2025 16:12:30 -0700 Subject: [PATCH] Snap: Do not leak snap variables or snap paths into children (#8771) Avoid leaking snap environment values and in particular the `$SNAP*` values to the children that we run from the terminal. Do this programmatically instead of the launcher, since ghostty needs know the environment it runs in, while it must not leak the info to the children. We've also another leak on snap, that creates a more visible problem (wrong matching of children applications) that is the apparmor security profile. I've handled it at https://github.com/3v1n0/ghostty/commit/cc3b46f6001aa9539508a1e82ebb85a588c6c4f2 but that requires some love in order to fully decouple the snap option to the build, to avoid including it in non-snap builds, so an help would be appreciated there. > This PR was contains code (in `filterSnapPaths`) that was improved by DeepSeek. --- .github/workflows/test.yml | 39 +++++++++++++- snap/local/launcher | 6 --- snap/snapcraft.yaml | 11 ++-- src/apprt/gtk/class/surface.zig | 92 ++++++++++++++++++++++++++++----- src/build/Config.zig | 9 ++++ src/build_config.zig | 1 + 6 files changed, 134 insertions(+), 24 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3c3be31c6..af159c9c3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,6 +19,7 @@ jobs: - build-nix - build-macos - build-macos-matrix + - build-snap - build-windows - test - test-gtk @@ -118,7 +119,41 @@ jobs: run: | nix develop -c \ zig build \ - -Dflatpak=true + -Dflatpak + + build-snap: + strategy: + fail-fast: false + runs-on: namespace-profile-ghostty-sm + needs: test + env: + ZIG_LOCAL_CACHE_DIR: /zig/local-cache + ZIG_GLOBAL_CACHE_DIR: /zig/global-cache + steps: + - name: Checkout code + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + + - name: Setup Cache + uses: namespacelabs/nscloud-cache-action@a289cf5d2fcd6874376aa92f0ef7f99dc923592a # v1.2.17 + with: + path: | + /nix + /zig + + # Install Nix and use that to run our tests so our environment matches exactly. + - uses: cachix/install-nix-action@7be5dee1421f63d07e71ce6e0a9f8a4b07c2a487 # v31.6.1 + with: + nix_path: nixpkgs=channel:nixos-unstable + - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 + with: + name: ghostty + authToken: "${{ secrets.CACHIX_AUTH_TOKEN }}" + + - name: Build with Snap + run: | + nix develop -c \ + zig build \ + -Dsnap build-linux: strategy: @@ -275,7 +310,7 @@ jobs: trigger-snap: if: github.event_name != 'pull_request' runs-on: namespace-profile-ghostty-xsm - needs: build-dist + needs: [build-dist, build-snap] steps: - name: Checkout code uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 diff --git a/snap/local/launcher b/snap/local/launcher index b4141c7de..89e0d1709 100755 --- a/snap/local/launcher +++ b/snap/local/launcher @@ -61,10 +61,4 @@ fi [ "$needs_update" = true ] && echo "LAST_REVISION=$SNAP_REVISION" > "$SNAP_USER_DATA/.last_revision" -# Unset all SNAP specific environment variables to keep them from leaking -# into other snaps that might get executed from within the shell -for var in $(printenv | grep SNAP_ | cut -d= -f1); do - unset $var -done - exec "$@" diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index df8d6ae53..e48fa93c8 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -20,7 +20,7 @@ platforms: apps: ghostty: command: bin/ghostty - command-chain: [bin/launcher] + command-chain: [app/launcher] completer: share/bash-completion/completions/ghostty.bash desktop: share/applications/com.mitchellh.ghostty.desktop #refresh-mode: ignore-running # Store rejects this, needs fix in review-tools @@ -35,7 +35,7 @@ parts: source: snap/local source-type: local organize: - launcher: bin/ + launcher: app/ zig: plugin: nil @@ -79,7 +79,12 @@ parts: # TODO: Remove -fno-sys=gtk4-layer-shell when we upgrade to a version that packages it Ubuntu 24.10+ override-build: | craftctl set version=$(cat VERSION) - $CRAFT_PART_SRC/../../zig/src/zig build -Dpatch-rpath=\$ORIGIN/../usr/lib/$CRAFT_ARCH_TRIPLET_BUILD_FOR:/snap/core24/current/lib/$CRAFT_ARCH_TRIPLET_BUILD_FOR -Doptimize=ReleaseFast -Dcpu=baseline -fno-sys=gtk4-layer-shell + $CRAFT_PART_SRC/../../zig/src/zig build \ + -Dsnap \ + -Dpatch-rpath=\$ORIGIN/../usr/lib/$CRAFT_ARCH_TRIPLET_BUILD_FOR:/snap/core24/current/lib/$CRAFT_ARCH_TRIPLET_BUILD_FOR \ + -Doptimize=ReleaseFast \ + -Dcpu=baseline \ + -fno-sys=gtk4-layer-shell cp -rp zig-out/* $CRAFT_PART_INSTALL/ sed -i 's|Icon=com.mitchellh.ghostty|Icon=${SNAP}/share/icons/hicolor/512x512/apps/com.mitchellh.ghostty.png|g' $CRAFT_PART_INSTALL/share/applications/com.mitchellh.ghostty.desktop diff --git a/src/apprt/gtk/class/surface.zig b/src/apprt/gtk/class/surface.zig index c26d0c1ef..616ec54f5 100644 --- a/src/apprt/gtk/class/surface.zig +++ b/src/apprt/gtk/class/surface.zig @@ -9,6 +9,7 @@ const gobject = @import("gobject"); const gtk = @import("gtk"); const apprt = @import("../../../apprt.zig"); +const build_config = @import("../../../build_config.zig"); const datastruct = @import("../../../datastruct/main.zig"); const font = @import("../../../font/main.zig"); const input = @import("../../../input.zig"); @@ -1227,19 +1228,11 @@ pub const Surface = extern struct { // Unset environment varies set by snaps if we're running in a snap. // This allows Ghostty to further launch additional snaps. - if (env.get("SNAP")) |_| { - env.remove("SNAP"); - env.remove("DRIRC_CONFIGDIR"); - env.remove("__EGL_EXTERNAL_PLATFORM_CONFIG_DIRS"); - env.remove("__EGL_VENDOR_LIBRARY_DIRS"); - env.remove("LD_LIBRARY_PATH"); - env.remove("LIBGL_DRIVERS_PATH"); - env.remove("LIBVA_DRIVERS_PATH"); - env.remove("VK_LAYER_PATH"); - env.remove("XLOCALEDIR"); - env.remove("GDK_PIXBUF_MODULEDIR"); - env.remove("GDK_PIXBUF_MODULE_FILE"); - env.remove("GTK_PATH"); + if (comptime build_config.snap) { + if (env.get("SNAP") != null) try filterSnapPaths( + alloc, + &env, + ); } // This is a hack because it ties ourselves (optionally) to the @@ -1253,6 +1246,79 @@ pub const Surface = extern struct { return env; } + /// Filter out environment variables that start with forbidden prefixes. + fn filterSnapPaths(gpa: std.mem.Allocator, env_map: *std.process.EnvMap) !void { + comptime assert(build_config.snap); + + const snap_vars = [_][]const u8{ + "SNAP", + "SNAP_USER_COMMON", + "SNAP_USER_DATA", + "SNAP_DATA", + "SNAP_COMMON", + }; + + // Use an arena because everything in this function is temporary. + var arena = std.heap.ArenaAllocator.init(gpa); + defer arena.deinit(); + const alloc = arena.allocator(); + + var env_to_remove = std.ArrayList([]const u8).init(alloc); + var env_to_update = std.ArrayList(struct { + key: []const u8, + value: []const u8, + }).init(alloc); + + var it = env_map.iterator(); + while (it.next()) |entry| { + const key = entry.key_ptr.*; + const value = entry.value_ptr.*; + + // Ignore fields we set ourself + if (std.mem.eql(u8, key, "TERMINFO")) continue; + if (std.mem.startsWith(u8, key, "GHOSTTY")) continue; + + // Any env var starting with SNAP must be removed + if (std.mem.startsWith(u8, key, "SNAP_")) { + try env_to_remove.append(key); + continue; + } + + var filtered_paths = std.ArrayList([]const u8).init(alloc); + defer filtered_paths.deinit(); + + var modified = false; + var paths = std.mem.splitAny(u8, value, ":"); + while (paths.next()) |path| { + var include = true; + for (snap_vars) |k| if (env_map.get(k)) |snap_path| { + if (snap_path.len == 0) continue; + if (std.mem.startsWith(u8, path, snap_path)) { + include = false; + modified = true; + break; + } + }; + if (include) try filtered_paths.append(path); + } + + if (modified) { + if (filtered_paths.items.len > 0) { + const new_value = try std.mem.join(alloc, ":", filtered_paths.items); + try env_to_update.append(.{ .key = key, .value = new_value }); + } else { + try env_to_remove.append(key); + } + } + } + + for (env_to_update.items) |item| try env_map.put( + item.key, + item.value, + ); + for (env_to_remove.items) |key| _ = env_map.remove(key); + } + pub fn clipboardRequest( self: *Self, clipboard_type: apprt.Clipboard, diff --git a/src/build/Config.zig b/src/build/Config.zig index b4759b45c..a4c9335be 100644 --- a/src/build/Config.zig +++ b/src/build/Config.zig @@ -51,6 +51,7 @@ patch_rpath: ?[]const u8 = null, /// Artifacts flatpak: bool = false, +snap: bool = false, emit_bench: bool = false, emit_docs: bool = false, emit_exe: bool = false, @@ -152,6 +153,12 @@ pub fn init(b: *std.Build) !Config { "Build for Flatpak (integrates with Flatpak APIs). Only has an effect targeting Linux.", ) orelse false; + config.snap = b.option( + bool, + "snap", + "Build for Snap (do specific Snap operations). Only has an effect targeting Linux.", + ) orelse false; + config.sentry = b.option( bool, "sentry", @@ -442,6 +449,7 @@ pub fn addOptions(self: *const Config, step: *std.Build.Step.Options) !void { // We need to break these down individual because addOption doesn't // support all types. step.addOption(bool, "flatpak", self.flatpak); + step.addOption(bool, "snap", self.snap); step.addOption(bool, "x11", self.x11); step.addOption(bool, "wayland", self.wayland); step.addOption(bool, "sentry", self.sentry); @@ -506,6 +514,7 @@ pub fn fromOptions() Config { .app_runtime = std.meta.stringToEnum(apprt.Runtime, @tagName(options.app_runtime)).?, .font_backend = std.meta.stringToEnum(font.Backend, @tagName(options.font_backend)).?, .renderer = std.meta.stringToEnum(rendererpkg.Impl, @tagName(options.renderer)).?, + .snap = options.snap, .exe_entrypoint = std.meta.stringToEnum(ExeEntrypoint, @tagName(options.exe_entrypoint)).?, .wasm_target = std.meta.stringToEnum(WasmTarget, @tagName(options.wasm_target)).?, .wasm_shared = options.wasm_shared, diff --git a/src/build_config.zig b/src/build_config.zig index 903197717..7f44ca2a1 100644 --- a/src/build_config.zig +++ b/src/build_config.zig @@ -38,6 +38,7 @@ pub const artifact = Artifact.detect(); const config = BuildConfig.fromOptions(); pub const exe_entrypoint = config.exe_entrypoint; pub const flatpak = options.flatpak; +pub const snap = options.snap; pub const app_runtime: apprt.Runtime = config.app_runtime; pub const font_backend: font.Backend = config.font_backend; pub const renderer: rendererpkg.Impl = config.renderer;