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
cc3b46f600
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.
1.2.x
Mitchell Hashimoto 2025-09-19 16:12:30 -07:00
parent e3cdf0faae
commit d231e94535
No known key found for this signature in database
GPG Key ID: 523D5DC389D273BC
6 changed files with 134 additions and 24 deletions

View File

@ -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

View File

@ -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 "$@"

View File

@ -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

View File

@ -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,

View File

@ -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,

View File

@ -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;