fix(agent,git,terminal): strip AppImage paths from spawned tool env

[theBGuy]

Inside the Linux AppImage, AppRun and its GTK hook point the loader and toolkit variables at the bundle, so every host binary we spawn (git, gh, glab, the terminal's shell) loads the bundle's Ubuntu 22.04 libraries instead of the system's — most visibly as git-remote-https: symbol lookup error on fetch/push against newer distributions. This adds a single subtractive sanitizer that removes the bundle's paths from each child's environment at spawn time, and applies it at every process-spawn site in the backend.

Environment sanitizer (src-tauri/src/agent.rs)

• Adds compute_child_env_overrides plus the APPDIR_PATHLIST_VARS / APPDIR_SCALAR_VARS tables: PATH-style lists (LD_LIBRARY_PATH, PATH, XDG_DATA_DIRS, GTK_PATH, the two GST_PLUGIN_SYSTEM_PATH spellings) have their $APPDIR entries dropped in place and are unset when nothing survives; single-path vars (GSETTINGS_SCHEMA_DIR, GTK_EXE_PREFIX, GTK_DATA_PREFIX, GTK_IM_MODULE_FILE, GDK_PIXBUF_MODULE_FILE, GIO_EXTRA_MODULES) are unset only when they point into the bundle.
• strip_appdir_pathlist / is_under_appdir match on a path boundary (and normalize a trailing slash) so a sibling mount sharing our prefix isn't stripped, and an override is emitted only when a bundle entry was actually dropped — a host-only list is inherited verbatim. PATH is emptied rather than unset so a child always has one.
• The plan is computed once by child_env_overrides() behind a OnceLock, gated on Linux and on a non-empty APPDIR (which also covers --appimage-extract runs, where APPIMAGE is absent); off-AppImage it is empty and every applier is a no-op. Comments record the deliberate exclusions — GDK_BACKEND, GTK_THEME and LD_PRELOAD are left alone — and that the environment is never cleared wholesale, so things like SSH_AUTH_SOCK still inherit.
• Introduces the ChildEnv trait with sanitize_child_env, implemented for tokio::process::Command, std::process::Command and portable_pty::CommandBuilder, so one helper covers every builder the app spawns through. It is called before each site's own .env() calls so explicit settings win.

Spawn sites

• Git and forge CLIs: git/runner.rs (run_git_raw_input, which also covers the credential helpers git spawns in turn), github/runner.rs (run_gh_raw, run_gh_input), forge/glab.rs (run_glab_raw, run_glab_ex) and forge/session.rs (run_reconnect_child).
• Terminal and external launchers: pty.rs (pty_open, with its comment updated to describe the subtract-then-advertise-TERM approach) and fsops.rs (launch_custom_command, both fallback loops in launch_terminal_unix, and open_with_program).
• Agent and tooling paths: agent.rs (resolve_via_login_shell, run_capture, stream_agent), agent_sandbox.rs (run_build, plus launch_container_shell, whose chained builder is unfolded into a local term_cmd), hooks.rs (git_run_hook_manager) and mcp.rs (run_client_cli).

Tests and changelog

• Adds a child_env_tests module in src-tauri/src/agent.rs covering the rules off-Linux via a fixture lookup: untouched host lists, all-bundle lists collapsing to unset, in-place removal preserving host order, trailing-slash equivalence, the prefix-vs-boundary case, degenerate/empty segments, the never-unset PATH behavior, scalars equal to $APPDIR, the excluded variables, and the empty-APPDIR no-op.
• Adds changelog.d/fixed-appimage-spawned-tools.md describing the fix in user terms.

Relates to #174

[cloudflare-workers-and-pages]

Deploying gitdesktop with    Cloudflare Pages

Latest commit:	ae716a5
Status:	✅  Deploy successful!
Preview URL:	https://ff1e6791.gitdesktop.pages.dev
Branch Preview URL:	https://fix-appimage-child-env-sanit.gitdesktop.pages.dev

View logs

[theBGuy]

Context for reviewers — deliberate calls with their evidence, numbered for reference. (Fixes #174; sibling of #173, which fixed the app's own EGL crash — this one fixes what the AppImage does to processes it spawns.)

1. What this is. The AppImage's AppRun/GTK hook export LD_LIBRARY_PATH and several GTK/GIO path vars pointing into the bundle; children inherit them, so the HOST git's git-remote-https resolves the bundled 2022-era libnghttp2 and dies (symbol lookup error: … nghttp2_option_set_no_rfc9113_leading_and_trailing_ws_validation — reproduced character-for-character in an ubuntu:24.04 container, and proven fixed by stripping $APPDIR-prefixed entries). The fix: a subtractive per-child sanitizer applied at every production spawn site (16, including the built-in terminal's PTY).

2. Subtractive, never env_clear/whitelist — deliberate. SSH agent auth, GIT_SSH_COMMAND, and credential managers work purely by env inheritance (the tree references none of them explicitly). A whitelist would break push/fetch over SSH for everyone.

3. The app's own process env is untouched — deliberate. WebKit's helper processes and the dlopen'd tray library need the bundled paths; stripping at startup would break the webview and tray. Only spawned children are sanitized.

4. Gating is APPDIR-only plus a compile-time Linux check; there is intentionally no runtime APPIMAGE check. Extracted runs (./App.AppImage --appimage-extract, the standard no-FUSE path) set only APPDIR — requiring APPIMAGE would disable the fix exactly where the bug still occurs.

5. The Linux gate is cfg!(...), not #[cfg(...)], deliberately. An attribute-gated body would make the pure functions and tables dead code on the Windows/macOS CI legs (clippy runs without --all-targets) and go red there. cfg! const-folds to an empty plan off-Linux with everything still compiled; Windows clippy is green in both profiles.

6. Ordering contract: sanitize first, site env wins. Every site calls the sanitizer before its own .env() chain, so GITLAB_TOKEN, GIT_TERMINAL_PROMPT, GH_*, COPILOT_GITHUB_TOKEN, and the PTY's TERM are never clobbered — walked site-by-site in review.

7. The var tables are measured, not guessed — read from the shipped artifact's apprun-hooks/linuxdeploy-plugin-gtk.sh and AppRun.wrapped. GDK_BACKEND, GTK_THEME, LD_PRELOAD are deliberately untouched (not $APPDIR-derived / user-owned). The list/scalar split tracks Tauri's plugin fork (appends to lists, overwrites scalars); the code header records that upstream's plugin would additionally need GI_TYPELIB_PATH.

8. PATH edge cases are pinned by tests: a var where nothing bundle-derived was actually stripped is left byte-identical (empty segments and all); PATH is never removed outright; the degenerate all-bundle PATH (unreachable — AppRun prepends to the host PATH) yields an empty-but-present PATH.

9. Known residual, disclosed rather than silently skipped: tauri-plugin-opener's xdg-open spawns inside the plugin where no Command of ours exists — "open in browser / reveal in file manager" from the AppImage still leaks the bundle env to host helpers. Follow-up candidate; out of scope here.

10. Verification so far: exact-error container repro + strip-proof (negative and positive controls); 12 hermetic unit tests (suite: 1082 green); clippy dev+release green on Windows AND a full Linux-container pass (check/clippy/test — the three cfg-gated Unix arms are compiled there, since a Windows box never does); adversarial spec review with all findings resolved. In progress: an end-to-end proof from inside a real AppImage — appimage-check was dispatched on this branch (run 31299185593; it doesn't auto-fire here, its paths filter doesn't cover src-tauri), and its artifact will be driven through the app's MCP mode in ubuntu:24.04 to run an actual git fetch, A/B against v0.7.0 as the failing control. Results will be posted before the ready-flip.

11. Deferred with homes: pre-existing --all-targets clippy findings in mcp_server/ (untouched files; CI doesn't run that form) → backlog. Excluded spawn sites with reasons: kill/taskkill (libc-only), the #[cfg(debug_assertions)] dev terminal helpers, #[cfg(test)] code, vendored portable-pty internals (sanitized at our call site instead).

---

Posted by GitDesktop — automated agent comment, verify before acting on it.

[theBGuy]

End-to-end proof from inside a real AppImage (context item 10, completed).

Setup: ubuntu:24.04 container (the reporter's distro; host libnghttp2 1.59.0-1ubuntu0.4), a fresh shallow clone of this repo, and each AppImage extracted and driven through the app's own MCP mode (AppRun mcp --repo <clone> --allow-git-write) with a tools/call fetch — i.e., the real production path: app → git runner → spawned host git → git-remote-https.

Arm	Binary	Result
Control	released v0.7.0 (no sanitizer)	fetch fails — symbol lookup error: /lib/x86_64-linux-gnu/libcurl-gnutls.so.4: undefined symbol: nghttp2_option_set_no_rfc9113_leading_and_trailing_ws_validation — the exact #174 report
Fixed	this PR's appimage-unsigned artifact (dispatch run 31299185593)	fetch succeeds

Same container, same clone, same invocation — the only variable is the sanitizer. This is the live confirmation that the fix cures the reported failure where it occurs, complementing the unit tests and the earlier mechanism-level repro. (The dispatched appimage-check run on this branch also passed its bundling guard and Mesa smoke test, so the #173 protections hold on this head too.)

---

Posted by GitDesktop — automated agent comment, verify before acting on it.

[Copilot]

Pull request overview

This PR fixes Linux AppImage breakage where spawned host tools (git/gh/glab, terminal shells, hook managers, etc.) incorrectly inherit AppImage-bundled loader/toolkit environment variables, causing host binaries to load the AppImage’s Ubuntu 22.04 libraries and fail (e.g., git-remote-https: symbol lookup error).

Changes:

• Added a centralized child-environment sanitizer in src-tauri/src/agent.rs that strips $APPDIR-scoped paths from selected path-list env vars and unsets bundle-owned scalar env vars when they point into $APPDIR.
• Applied the sanitizer at multiple backend process-spawn sites (git, forge CLIs, terminal launchers, hook manager, MCP client CLI, container shell).
• Added unit tests for the sanitizer rules and a changelog fragment describing the user-visible fix.

Reviewed changes

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

Show a summary per file

File	Description
src-tauri/src/agent.rs	Implements the AppImage child env override plan, a shared sanitize_child_env helper, and unit tests for stripping/unsetting rules.
src-tauri/src/git/runner.rs	Sanitizes env before spawning git (covers git’s own spawned helpers).
src-tauri/src/github/runner.rs	Sanitizes env before spawning gh.
src-tauri/src/forge/glab.rs	Sanitizes env before spawning glab.
src-tauri/src/forge/session.rs	Sanitizes env before spawning reconnect child processes.
src-tauri/src/pty.rs	Sanitizes env for PTY-launched commands so terminal-run host tools aren’t poisoned by AppImage vars.
src-tauri/src/fsops.rs	Sanitizes env for launching custom commands, terminals, and “open with program” flows.
src-tauri/src/hooks.rs	Sanitizes env before spawning the git hook manager.
src-tauri/src/mcp.rs	Sanitizes env before spawning the MCP client CLI.
src-tauri/src/agent_sandbox.rs	Sanitizes env before spawning sandbox build and container terminal shell processes.
changelog.d/fixed-appimage-spawned-tools.md	Documents the AppImage spawned-tools fix for end users.

Suppressed comments (1)

src-tauri/src/agent.rs:2694

• These comment lines are misleading: strip_appdir_pathlist always drops empty segments (!entry.is_empty()), even when no $APPDIR entries are removed. The reason the child keeps the original value is that compute_child_env_overrides emits no override when dropped == false.

        // Empty segments are dropped only as a side effect of a real strip; with
        // nothing bundle-derived to remove, the child inherits the value verbatim.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[theBGuy]

🤖 GitDesktop AI review · opus · automated

---

The change adds a subtractive, per-child env sanitizer (APPDIR-derived loader/toolkit vars) in agent.rs and applies it at every production spawn site before each site's own .env() calls; I walked all 18 call sites and the ordering contract holds everywhere, and the vendored portable-pty genuinely honors env_remove (its get_base_env() snapshots vars_os() and as_command() does env_clear() + re-apply, so removal reaches the child). The core logic and its tests look sound — nothing blocking.

Correctness / coverage

• should-fix — src-tauri/src/agent.rs:1815, kill_process_tree's Unix arm: std::process::Command::new("kill") is the one non-dev, non-test, Linux-reachable spawn left unsanitized. This remains a recorded decision ("libc-only"), but that justification doesn't hold for the reporter's own distro: Ubuntu 24.04's /usr/bin/kill is procps-ng and links libproc2 → libsystemd → liblzma/libzstd, and this bundle demonstrably ships transitive libs of exactly that kind (the fix(linux): AppImage leaks bundled libraries into spawned system git via LD_LIBRARY_PATH (git-remote-https symbol lookup error) #174 report is a bundled libnghttp2). If the group-kill dies with a symbol-lookup error, the fallback child.start_kill() at line 1822 only reaps the direct child — the agent CLI's node workers and MCP servers orphan, which is precisely what the function's doc comment says it exists to prevent. Fix: let mut cmd = std::process::Command::new("kill"); crate::agent::sanitize_child_env(&mut cmd); let _ = cmd.args(["-KILL", &format!("-{pid}")]).spawn(); — same shape as the sibling sites, one line, and it keeps the #[cfg(unix)] block's fire-and-forget semantics unchanged.

• should-fix — changelog.d/fixed-appimage-spawned-tools.md: "Git and other external tools launched from the Linux AppImage … no longer inherit the bundle's library paths" is broader than what ships. fsops.rs:231 (reveal_in_explorer → tauri_plugin_opener::reveal_item_in_dir) and fsops.rs:237 (open_with_default → tauri_plugin_opener::open_path) still spawn the system handler inside the plugin with the bundle env — the residual the author disclosed and deliberately scoped out. A user who reads the bullet and then hits a broken "Reveal in file manager" has been told wrong. Narrow the body to the surfaces actually covered, e.g. "Git, the GitHub/GitLab CLIs, agent CLIs and commands run in the built-in terminal no longer inherit the bundle's library paths when launched from the Linux AppImage — fixing fetches and pushes failing with a git-remote-https: symbol lookup error on newer distributions." (Keep the fragment as the only doc surface: README, site/, and src/features/help/content.ts carry no AppImage-limitation copy this fix would make stale — I checked README.md and site/src/pages/download.astro.)

Tests

• should-fix — src-tauri/src/agent.rs:335-342, impl ChildEnv for portable_pty::CommandBuilder: the whole terminal half of the fix rests on an external contract nothing pins — that CommandBuilder::env_remove removes an inherited var rather than just clearing a caller override. That contract lives in a vendored crate whose Cargo.toml patch comment says "Drop this when fixed upstream (wezterm)", so an upgrade or de-vendoring could flip it silently and the PTY would go back to leaking LD_LIBRARY_PATH with every unit test still green. Add a test in child_env_tests that needs no env mutation (so it can't race the std::env::set_var in the existing mod tests): let mut cmd = portable_pty::CommandBuilder::new("x"); assert!(cmd.get_env("PATH").is_some()); cmd.env_remove("PATH"); assert!(cmd.get_env("PATH").is_none(), "portable-pty must drop inherited vars, not just caller overrides");.

• should-fix — no guard keeps APPDIR_PATHLIST_VARS / APPDIR_SCALAR_VARS (agent.rs:205-225) in sync with the artifact they were measured from. Concrete drift case: appimage-check.yml already warns that "A Tauri CLI or bundle-config change can silently no-op the pin", and the code header itself records that upstream's linuxdeploy GTK plugin additionally exports GI_TYPELIB_PATH — a linuxdeploy/tauri-cli bump that swaps in that hook reintroduces a leaked $APPDIR path with every test still passing. .github/scripts/appimage-guard.sh already extracts the AppImage in the job whose PR paths filter covers exactly those drift vectors (release.yml, pin-linuxdeploy/**, src-tauri/tauri.conf.json), so extend it there: grep squashfs-root/AppRun* and squashfs-root/apprun-hooks/*.sh for export NAME= lines whose value references APPDIR, and fail on any name not in a shell list that mirrors the two Rust tables — with a comment on both the shell list and the Rust consts naming the other as its twin so the pair is updated together.

Edge cases

• nit — agent.rs:276-283: the all-bundle-PATH fallback sets PATH="", and POSIX/glibc treat an empty PATH element as the current directory, so a bare-name spawn under it (xterm, gnome-terminal, kill) would search the child's cwd — which for the terminal launchers is the user's repo. Reachability is a recorded decision (AppRun prepends to the host PATH), so this is defense-in-depth only: setting "/usr/local/bin:/usr/bin:/bin" instead of String::new() keeps portable-pty's resolve_path() happy exactly as the empty value does, and drops the cwd element; update the adjacent comment and the path_is_never_unset_even_when_every_entry_is_bundle_derived test's expected value to match.

Readability

• nit — agent.rs:203-204: the APPDIR_PATHLIST_VARS doc says the variable "is unset when nothing survives", which is untrue for PATH (special-cased to empty at line 279); append "— except PATH, which is emptied rather than unset".
• nit — agent.rs:2716 (a_host_only_list_keeps_its_empty_segments_and_emits_no_override, whose asserted return value "/usr/local/bin:/usr/bin" has the empty segment removed) and agent.rs:2731 (degenerate_segments_are_dropped_and_an_empty_value_unsets, where ("", APPDIR) returns dropped: false and so never unsets): both names describe plan-level behavior while asserting function-level results — rename the untouched helper to no_strip and reword the two names to say "emits no override" rather than "keeps"/"unsets".
• nit — agent.rs:296: child_env_overrides has no caller outside agent.rs; drop it to private fn (ChildEnv and sanitize_child_env must stay pub(crate)).
• nit — the Command::new(x) + sanitize_child_env(&mut cmd) pair is now repeated at 18 sites and nothing stops the 19th from forgetting; consider small constructors in agent.rs (one per builder type) that new-and-sanitize, so the sanitizer is the default rather than a convention.

---

Posted by GitDesktop — AI output, verify before acting on it.

[theBGuy]

🤖 GitDesktop AI security audit · opus · automated

---

No genuinely exploitable vulnerability is introduced by these changes — the sanitizer only subtracts bundle-derived values from a fixed table of loader/toolkit vars, is applied before every site's own .env() calls (so no token or GIT_TERMINAL_PROMPT/TERM setting is clobbered), and the binaries at the touched spawn sites are resolved through the existing absolute-path resolver rather than the modified child PATH.

---

Posted by GitDesktop — AI output, verify before acting on it.

[theBGuy]

Round-1 dispositions for the AI review (security audit was clean; Copilot's three threads answered inline). All fixes are in the tree, riding the next push.

SF1 (kill spawn unsanitized) — accepted with a correction on the record. The cited linkage is refuted: measured on ubuntu:24.04, ldd /usr/bin/kill yields linux-vdso, libc.so.6, ld-linux — no libproc2, no libsystemd, no liblzma/libzstd. The "libc-only" exclusion was factually sound on the reporter's distro. We applied the one-line sanitize anyway: it costs nothing, keeps the exclusion list from needing per-distro re-justification, and makes the spawn sites uniform.

SF2 (fragment over-claims) — accepted. The bullet now names the covered surfaces (git, GitHub/GitLab CLIs, agent tools, built-in terminal) instead of "other external tools", so it no longer promises the opener-plugin surfaces the fix deliberately excludes.

SF3 (pin the vendored env_remove contract) — accepted, test added. portable_pty_env_remove_drops_an_inherited_var, hermetic, using the builder's get_env. One detail worth recording: it passes on Windows because the vendored builder lowercases keys there (EnvEntry::map_key), so "PATH" matches the inherited "Path" — the test is meaningful on all three CI OSes.

SF4 (table-drift guard) — accepted with one scope adaptation. appimage-guard.sh now parses apprun-hooks/*.sh for export lines whose value references APPDIR and fails on any name outside an allowlist twinned with the Rust tables (pointer comments both directions). Scoped to the hook scripts only: AppRun.wrapped is a binary that sets LD_LIBRARY_PATH programmatically, so export-grepping it is meaningless — noted in the script. Functionally verified with a fixture: a realistic hook passes; appending export GI_TYPELIB_PATH="$APPDIR/…" fails naming the variable (your exact drift scenario); an absent hooks directory passes with a "0 hooks" note — a deliberate soft spot, since Tauri could legitimately restructure, and the libwayland check still gates.

Edge-case nit (empty PATH = cwd) — accepted. The degenerate fallback is now /usr/local/bin:/usr/bin:/bin with the comment and test updated.

Readability nits — all applied: the APPDIR_PATHLIST_VARS doc carries the PATH exception; both test names and the no_strip helper now describe function-level results with the plan-level consequence phrased as "emits no override"; child_env_overrides is private.

Constructor-pattern suggestion — deferred with a home. Reworking 18 sites into new-and-sanitize constructors late in the round trades churn for a class the drift guard and the helper's ordering contract already police; it's recorded in the backlog as a follow-up refactor.

Full re-verification after the batch: 1083 tests green (13 in child_env_tests), clippy dev+release clean on Windows, and a fresh Linux-container pass (check/clippy/test all exit 0 — the kill arm is cfg(unix), so that run is its compiler).

---

Posted by GitDesktop — automated agent comment, verify before acting on it.

[theBGuy]

🤖 GitDesktop AI review · opus · automated

---

The re-review focuses on the four accepted fixes and the new guard script; the core sanitizer is unchanged and still sound. Nothing blocking — the only substantive item is a coverage gap in the newly added drift guard.

Resolved since last review

• src-tauri/src/agent.rs:1818-1823 — the Unix kill spawn is now sanitized with the same shape as the sibling sites; #[cfg(unix)] fire-and-forget semantics unchanged. (The fallback PATH change can't break this resolution: FALLBACK_PATH contains /usr/bin.)
• changelog.d/fixed-appimage-spawned-tools.md — the bullet now enumerates the covered surfaces and no longer promises the tauri-plugin-opener paths (fsops.rs reveal/open-default) the fix deliberately excludes.
• src-tauri/src/agent.rs:2812-2827 — portable_pty_env_remove_drops_an_inherited_var pins the vendored contract, and it holds up: CommandBuilder::new seeds envs from get_base_env() (vendor/portable-pty/src/cmdbuilder.rs:215-224, 74-86) and env_remove deletes from that same map (:316-322), so the assertions are meaningful rather than vacuous. No other test in the crate mutates PATH (only GD_TEST_ROOT at agent.rs:3431/3443), so it can't race the sibling mod tests.
• .github/scripts/appimage-guard.sh:26-55 — the table-drift guard exists, its allowlist matches APPDIR_PATHLIST_VARS + APPDIR_SCALAR_VARS exactly (plus APPDIR), the space-delimited case match is well-formed on both ends, and || true correctly absorbs grep's no-match exit under set -euo pipefail.
• src-tauri/src/agent.rs:229/279-287/2801-2809 — FALLBACK_PATH replaces the empty-PATH fallback, with comment and expected test value updated.
• src-tauri/src/agent.rs:203-205, :2635/2693-2721 — doc exception for PATH, no_strip rename, and both reworded test names landed; child_env_overrides is now private and still has no caller outside agent.rs.

Recorded decisions I'm not re-flagging: the "0 hooks passes silently" soft spot in the guard, and deferring the new-and-sanitize constructor refactor to the backlog.

Correctness

• should-fix — .github/scripts/appimage-guard.sh:29-30, 38: the scan covers squashfs-root/apprun-hooks/*.sh only, but the header comment justifies excluding AppRun.wrapped alone. The other file linuxdeploy generates in that arrangement, squashfs-root/AppRun, is a shell wrapper (it exists precisely to source the hooks and exec AppRun.wrapped), and agent.rs:196 states AppRun itself points loader vars into the bundle — so a linuxdeploy/tauri-cli bump that moves or adds an export FOO="$APPDIR/…" into the wrapper instead of the GTK hook reintroduces a leaked bundle path with the guard still green, which is the exact drift class this guard was added for. Fix: iterate both, and make the grep binary-safe so the loop is correct whichever form AppRun takes on a future bundler —
  for hook in "$workdir"/squashfs-root/AppRun "$workdir"/squashfs-root/apprun-hooks/*.sh; do, [ -f "$hook" ] || continue, and grep -IE '^[[:space:]]*export…' (with -I a binary AppRun simply yields no matches instead of Binary file … matches flowing into sed). Knock-ons: reword the # Hook SCRIPTS only: … comment to say the wrapper and the hooks are scanned while AppRun.wrapped is skipped as a binary that sets LD_LIBRARY_PATH programmatically, and change the closing echo from startup hook(s) to startup script(s) so the count stays truthful.

Readability

• nit — .github/workflows/release.yml:137 and .github/workflows/appimage-check.yml:77: both step names still read "Guard against a bundled libwayland-client" though the script now also fails on unstripped $APPDIR exports; rename both to something like "Guard the AppImage bundle and its startup env" so a red step isn't misread.
• nit — .github/scripts/appimage-guard.sh:41: the pattern only matches single-line export NAME=…$APPDIR; the two-step NAME="$APPDIR/…" + export NAME form (and declare -x) slips through — worth a one-line comment recording the known shape limit, or a second export[[:space:]]+NAME$ pass.

Copilot's remaining open thread (gate on target_os = "linux" rather than all(unix, not(target_os = "macos"))) is answered by the repo's own idiom — the sibling spawn arms this PR touches (fsops.rs:550, pty.rs:712, agent_sandbox.rs:1049) all use the same gate, so changing this one alone would diverge it from its call sites; no action needed.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready

---

Posted by GitDesktop — AI output, verify before acting on it.

[theBGuy]

Round-2 dispositions (re-review verdict was "no blocking issues"; one should-fix + two nits, all applied — in the tree, riding the final push):

1. Drift guard now scans the AppRun wrapper too — the loop covers squashfs-root/AppRun + apprun-hooks/*.sh with [ -f ] guards and grep -IE. Fixture-verified in six arms, including the one this round adds: an $APPDIR export placed in the wrapper (clean hook) now FAILS by name — it passed silently before. The -I flag was proven load-bearing with a constructed binary that contains a matching line: without -I the "Binary file matches" sentence word-splits into bogus variable names downstream; with it, the binary is skipped. Comments and the OK/FAIL wording updated to "startup script(s)".
2. Both guard steps renamed to "Guard the AppImage bundle and its startup env" — kept byte-identical across the two workflows; nothing keys on step names.
3. Shape limit recorded in the script: only single-line export NAME=… is matched; NAME=…; export NAME and declare -x are out of scope (linuxdeploy emits single-line exports).

Two small facts on the record for future readers: the APPDIR allowlist entry is currently inert (a real AppRun sets it without $APPDIR on the right-hand side, so the grep never sees it — it guards only a hypothetical re-export) and the zero-startup-scripts case still passes with a "0 startup script(s)" note, both deliberate.

The auto-fired appimage run on the current head already passed with the round-1 guard — including the drift check's first execution against a real CI-built AppImage, which proves the shipped hooks' exports all sit within the sanitizer's tables.

---

Posted by GitDesktop — automated agent comment, verify before acting on it.

[theBGuy]

Correction to my previous comment. I wrote that the APPDIR allowlist entry in the drift guard is "currently inert" — that's wrong. The GTK hook re-exports it with a fallback on the right-hand side (export APPDIR="${APPDIR:-"$(dirname …)"}"), and the guard's \$\{?APPDIR pattern matches that form: running the exact pipeline against the real shipped hook extracts APPDIR as the first name. The entry is therefore load-bearing — without it the guard would fail every legitimate AppImage — and it is exercised on every CI run of the check (the green run on this head included). The earlier reasoning came from the AppRun wrapper's export shape and missed the hook's fallback form. No code change: the script is correct as written; only the recorded rationale flips.

---

Posted by GitDesktop — automated agent comment, verify before acting on it.

[theBGuy]

🤖 GitDesktop AI review · opus · automated

---

Only the CI guard script and two workflow step names changed since the last round; the Rust sanitizer and all 16 spawn sites are byte-identical to what I reviewed and re-verified before. The three items from last round are all addressed, and the new guard hunks introduce no problems I can find.

Resolved since last review

• .github/scripts/appimage-guard.sh:40-45 — the scan now covers squashfs-root/AppRun alongside apprun-hooks/*.sh, with [ -f ] per entry and grep -IE so a binary AppRun on a future bundler yields no matches instead of feeding Binary file … matches into sed. The loop is still correct under set -euo pipefail (|| true wraps the whole pipeline inside the substitution, and scripts=$((scripts + 1)) is assignment form, so a zero value can't trip -e), and every line the grep matches is also matched by the sed extraction, so no malformed names reach the case. The header comment (:29-30), the FAIL text (:54), and the closing count (:58) all read "startup script(s)" consistently.
• .github/workflows/appimage-check.yml:77 and .github/workflows/release.yml:137 — both steps renamed to "Guard the AppImage bundle and its startup env", identically; nothing in either workflow keys on the step name.
• .github/scripts/appimage-guard.sh:31-32 — the single-line-export-only shape limit is now recorded in the script, next to the allowlist it constrains.

Spot-checked afresh this round: no production Linux spawn site remains unsanitized — pty.rs:299/370/387 all build through build_command, whose only caller is pty_open, which sanitizes the returned builder before .env("TERM", …); the remaining bare Command::new sites are Windows-only (fsops.rs:458/467, pty.rs:423, agent_sandbox.rs:1014, git/ops.rs:5472, agent.rs:1811), macOS-only (fsops.rs:503/529/537/544/1047, pty.rs:703, agent_sandbox.rs:1040), #[cfg(debug_assertions)] (pty.rs:724), or test-only (git/ai_ignore.rs:1366). The allowlist's APPDIR entry does match the GTK hook's export APPDIR="${APPDIR:-…}" form under the script's \$\{?APPDIR pattern, so it is load-bearing as the latest note says.

Nothing further to raise — the deliberate calls on the record (zero-startup-scripts passing with a note, the deferred new-and-sanitize constructor refactor, the all(unix, not(target_os = "macos")) gate) still stand as recorded.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready

---

Posted by GitDesktop — AI output, verify before acting on it.