fix(flatpak): run CLI on host so podman/docker resolve#636
Conversation
Inside the Flatpak sandbox the container runtime (docker/podman), git and ssh are not on $PATH and the host filesystem is only partially visible, so workspace creation failed with "podman: executable file not found in $PATH" (#634). Re-execute the CLI on the host via `flatpak-spawn --host` at command entry, mirroring pkg/agent.rerunAsRoot. The `internal` subtree is excluded because it owns the daemon whose Unix socket lives in the per-sandbox /tmp and must stay co-located with the desktop's daemon client. The re-exec is a no-op outside a sandbox, so the CLI is unchanged on macOS, Windows and native Linux. Also refactor Execute() into a thin os.Exit(run()) wrapper so deferred telemetry flushing runs on every exit path, and switch the exit-code error handling to errors.AsType so a wrapped ExitError still propagates its status. Fixes #634
✅ Deploy Preview for devsydev canceled.
|
✅ Deploy Preview for images-devsy-sh canceled.
|
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Replace repeated "workspace"/"provider"/"list" string literals in the cmd tests with shared constants, and drop the redundant doc comment on TestTopLevelCommand.
5a4a2e3 to
81cee24
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/flatpak/flatpak_test.go (1)
46-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for the successful re-exec path.
The tests cover the no-op (outside sandbox) and error (missing binary) paths well, but the happy path — inside sandbox, binary present,
flatpak-spawninvoked and succeeding — is untested. This is the most critical path for the PR's objective. Extracting theexec.Commandcall behind an interface would make this testable without a realflatpak-spawnbinary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/flatpak/flatpak_test.go` around lines 46 - 74, Test the successful path of ReexecOnHost by introducing an injectable command-execution seam around its exec.Command call, then add a test covering a detected Flatpak sandbox with flatpak-spawn available and succeeding. Assert that the command is invoked with the expected arguments, ReexecOnHost returns shouldExit=true, and no error is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/flatpak/flatpak.go`:
- Around line 45-53: Redact sensitive values before logging the re-execution
arguments in the debug statement near `exec.Command`; replace raw `os.Args[1:]`
in `log.Debugf` with an appropriate sanitized or redacted representation while
preserving useful argument context.
---
Nitpick comments:
In `@pkg/flatpak/flatpak_test.go`:
- Around line 46-74: Test the successful path of ReexecOnHost by introducing an
injectable command-execution seam around its exec.Command call, then add a test
covering a detected Flatpak sandbox with flatpak-spawn available and succeeding.
Assert that the command is invoked with the expected arguments, ReexecOnHost
returns shouldExit=true, and no error is returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c95d66fd-aa7a-493e-b7f8-bf318f2255ac
📒 Files selected for processing (6)
cmd/env_flags_test.gocmd/root.gocmd/root_test.godesktop/flatpak/devsy-wrapperpkg/flatpak/flatpak.gopkg/flatpak/flatpak_test.go
| spawnArgs := append([]string{"--host", hostBinary}, os.Args[1:]...) | ||
| log.Debugf("re-executing on host via flatpak-spawn: args=%v", os.Args[1:]) | ||
|
|
||
| //nolint:gosec | ||
| cmd := exec.Command(flatpakSpawn, spawnArgs...) | ||
| cmd.Stdin = os.Stdin | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| cmd.Env = os.Environ() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how flatpak-spawn --host handles environment by searching for docs or usage patterns
rg -n 'flatpak-spawn' --type go --type shell
# Also check if there are any env filtering patterns elsewhere in the codebase
rg -n 'cmd\.Env' --type go | head -20Repository: devsy-org/devsy
Length of output: 183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context
wc -l pkg/flatpak/flatpak.go
cat -n pkg/flatpak/flatpak.go | sed -n '1,180p'
# Find any surrounding flatpak-spawn usage and env handling in the repo
rg -n 'flatpak-spawn|cmd\.Env|os\.Environ\(\)|log\.Debugf\("re-executing on host' pkg -S
# Look for docs/comments that describe why Env is set this way
rg -n 'DEVSY_|XDG_DATA_HOME|PATH|flatpak-spawn|re-executing on host' -S .Repository: devsy-org/devsy
Length of output: 30182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Flatpak-specific docs, wrapper behavior, and any logging/redaction helpers
rg -n 'flatpak-spawn|debugf|redact|sensitive|token|secret|password|authorization|args=%v' -S \
desktop pkg cmd sites docs .github
# Show the Flatpak launcher/wrapper context
cat -n desktop/flatpak/sh.devsy.app.yml | sed -n '1,120p'
cat -n desktop/flatpak/devsy-wrapper | sed -n '1,120p'
# Look for any tests around flatpak re-exec behavior
cat -n pkg/flatpak/flatpak_test.go | sed -n '1,220p'Repository: devsy-org/devsy
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the Flatpak launcher and wrapper context with line numbers
cat -n desktop/flatpak/sh.devsy.app.yml | sed -n '1,120p'
echo '---'
cat -n desktop/flatpak/devsy-wrapper | sed -n '1,120p'
echo '---'
cat -n pkg/flatpak/flatpak_test.go | sed -n '1,220p'
echo '---'
cat -n pkg/log/logger.go | sed -n '1,240p'
# Search for any explicit redaction helpers or env sanitization around logging
rg -n 'redact|sanitize|mask|sensitive|args=%v|Debugf\(".*args' pkg desktop cmd sites -SRepository: devsy-org/devsy
Length of output: 32995
🌐 Web query:
flatpak-spawn --host environment inherit caller-provided env documentation
💡 Result:
By default, flatpak-spawn does not automatically inherit or mirror the environment variables of the calling process inside the sandbox [1][2]. When you use flatpak-spawn --host, the command is executed on the host system via the flatpak-session-helper [3][4]. Because this process runs as a separate service on the host, it does not implicitly receive the specific environment of the application inside the sandbox [4]. To pass environment variables to a command executed with flatpak-spawn, you must explicitly define them using the --env=VAR=VALUE option [5][6][3]. Key technical points regarding environment handling: 1. Manual Specification: You must provide each required environment variable individually using the --env flag (e.g., flatpak-spawn --host --env=FOO=bar COMMAND) [5][3]. 2. No Automatic Inheritance: There is no built-in flag (such as --inherit-env) to automatically copy all variables from the caller's environment [2]. 3. Execution Context: When using --host, the command runs in an environment managed by the D-Bus service. If the service was started before your graphical session (e.g., in a systemd user service), it may lack variables like DISPLAY regardless of what is set inside your sandbox [4]. 4. Subsandboxes: Note that flatpak-spawn without --host behaves differently, as it creates a new sandbox (a "subsandbox"). While improvements have been made to propagate certain base flatpak-run environment variables to subsandboxes [7], this does not equate to inheriting the arbitrary environment variables of the calling process [1]. For complex use cases requiring many variables, users often have to construct a shell command that sets the environment explicitly, such as flatpak-spawn --host sh -c 'FOO=bar BAZ=qux mycommand' [3].
Citations:
- 1: [Bug]: Subsandboxes doesn't inherit custom --env from
flatpak runflatpak/flatpak#6539 - 2: flatpak-spawn: Option to copy current env vars flatpak/flatpak-xdg-utils#28
- 3: [Bug]: flatpak-spawn command envs not available flatpak/flatpak#4562
- 4: [Bug]: Running flatpak apps as systemd services causes flatpak-spawn --host env | sort to be shorter with environments flatpak/flatpak#6486
- 5: https://manpages.debian.org/unstable/flatpak/flatpak-spawn.1.en.html
- 6: https://man.archlinux.org/man/flatpak-spawn.1.en.raw
- 7: Inherit flatpak-run environment from parent instance when spawning flatpak/flatpak#5510
Avoid logging raw CLI arguments here The debug line can leak tokens or other sensitive values passed on the command line. Use a redacted form instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/flatpak/flatpak.go` around lines 45 - 53, Redact sensitive values before
logging the re-execution arguments in the debug statement near `exec.Command`;
replace raw `os.Args[1:]` in `log.Debugf` with an appropriate sanitized or
redacted representation while preserving useful argument context.
Malformed invocations (unknown command, bad flag) exited 1 with no output: the failure happens before PersistentPreRunE, so the logger was never initialized (a no-op logger swallowed the error) and cobra's own error printing was disabled by SilenceErrors/SilenceUsage. Initialize the logger up front in run(), and silence cobra only in machine mode (--log-output json/logfmt, used by the desktop IPC). In interactive text mode cobra now prints its native "Error: ... / Did you mean / Run 'devsy --help' for usage." while machine mode still emits the structured cliError. exitCodeForError mirrors the split to avoid double-printing.
The desktop CI built the CLI with a plain `go build`, which defaults to CGO enabled on the runner and produces a glibc-dynamically-linked binary. Because that same binary is injected into workspace containers as the agent, it failed to exec in musl-based images (e.g. alpine) — the dynamic loader /lib64/ld-linux-x86-64.so.2 is absent — surfacing as "inject agent: EOF" (container exits 127) during workspace up. Match the goreleaser release build: CGO_ENABLED=0 with the netgo/osusergo tags, yielding a fully static binary that runs regardless of the container's libc.
ResolveDockerCommand expanded agent.docker.path (typically the template
"${DOCKER_PATH}") with os.ExpandEnv, but DOCKER_PATH is a provider
option rather than an OS environment variable. It therefore expanded to
empty and fell back to "docker", so `workspace exec` failed with
`docker: executable file not found` on podman-only hosts even though the
provider was configured with DOCKER_PATH=podman.
Expand against the workspace's resolved provider options first, falling
back to the OS environment.
The conditional cobra silencing regressed the agent protocol: `internal agent ...` commands are invoked without --log-output=json (they force JSON internally) and drive a binary/JSON protocol on stdout. With silencing keyed only on --log-output, a parse/usage error dumped cobra's human-readable Usage block into that stream, breaking the e2e log-stream contract (up-handle-errors). Treat the internal subtree as always machine-facing: silence cobra regardless of --log-output. Extracted configureOutput to keep run() within complexity limits, and added a regression test for the contract.
Rootless Podman has no persistent daemon, so its containers live under the user's systemd session and are reaped when that session ends (SSH disconnect, or the desktop closing the spawning process). A workspace that came up successfully then stops on its own — unlike Docker, whose system daemon keeps containers alive independently of login sessions. Detect this configuration (rootless podman + linger disabled) during container creation and surface a HostWarning telling the user to run `loginctl enable-linger`. The warning flows through the result envelope, so both the CLI and the desktop app can display it. Detection never blocks workspace creation and is skipped for Docker/nerdctl.
Output bifurcation previously keyed only on --log-output, so the desktop app had to pass it on every call and any invocation that forgot leaked human-formatted text into a parsed stream. Consolidate the decision into isMachineConsumer, which treats output as machine-consumed when: the command is in the internal subtree (agent protocol), DEVSY_UI marks a desktop-spawned process (provenance, already set by the desktop for every CLI call), --log-output requests a structured format, or — as a fallback — stderr is not a terminal (piped), matching conventional CLI behavior. A human at a terminal still gets cobra's usage/error text. exitCodeForError and configureOutput now share this single decision so they cannot drift.
1 participant
Summary
Fixes #634. Inside the Flatpak sandbox the container runtime (docker/podman), git and ssh are not on
$PATHand the host filesystem is only partially visible, so workspace creation failed withpodman: executable file not found in $PATH.Fixing that surfaced several related issues in how the CLI is dual-vended (human terminal, desktop IPC, in-container agent). This PR addresses the flatpak root cause and the adjacent bugs found while verifying it end-to-end on a real Fedora host.
Fixes
1. Flatpak host passthrough (#634 core)
The CLI re-executes itself on the host via
flatpak-spawn --hostat command entry (pkg/flatpak.ReexecOnHost), mirroring the existingpkg/agent.rerunAsRootpattern./.flatpak-infoexists — inert on macOS/Windows/native Linux.devsy-wrapperto$XDG_DATA_HOME/devsy/devsy), forwarding stdio and propagating the exit code.flatpak-spawn --hostinherits the caller env, so the host$PATHresolves podman/docker.internalsubtree is excluded: it owns the local daemon whose Unix socket lives in the per-sandbox/tmpand must stay co-located with the desktop's daemon client.local_dockerwriting to avolume inspectmountpoint) do raw host-filesystem work that only succeeds when the entire process is on the host.2. Static CLI build for agent injection
The desktop CI built the CLI with a plain
go build, which defaults to CGO-enabled and produced a glibc-dynamically-linked binary. Because that same binary is injected into workspace containers as the agent, it failed to exec in musl-based images (e.g. alpine) — surfacing asinject agent: EOF(container exits 127) duringworkspace up. Now matches the goreleaser release build:CGO_ENABLED=0withnetgo/osusergo.3.
workspace execdocker-path resolutionResolveDockerCommandexpandedagent.docker.path(the${DOCKER_PATH}/${PODMAN_PATH}template) withos.ExpandEnv, but those are provider options, not OS env vars — so it fell back todockerand failed on podman-only hosts. Now expands against the workspace's resolved provider options.4. Rootless-podman linger warning
Rootless podman has no persistent daemon, so its containers are reaped when the login session ends (SSH disconnect, or the desktop closing the spawning process) — unlike Docker. A workspace that came up successfully would then stop on its own.
upnow detects rootless-podman-without-linger and surfaces aHostWarning(via the result envelope, so both the CLI and desktop can display it) telling the user to runloginctl enable-linger. Detection never blocks workspace creation.5. CLI output contract for machine vs. human consumers
Malformed invocations (unknown command, bad flag) previously exited 1 with no output, and output bifurcation was inferred only from
--log-output. Consolidated intoisMachineConsumer, which treats output as machine-consumed when: the command is in theinternalsubtree (agent protocol),DEVSY_UImarks a desktop-spawned process (provenance),--log-outputrequests a structured format, or — as a fallback — stderr is not a terminal (piped). A human at a terminal now gets cobra's native error/usage text (including "Did you mean" suggestions); machine consumers get only the structuredcliError.Verification
Verified end-to-end on a real Fedora host with rootless podman, using the CI-built flatpak, through the sandboxed
flatpak runpath:DOCKER_PATH=podman) and the dedicated podman provider (${PODMAN_PATH}):workspace upsucceeds andworkspace execruns commands in the container (agent as PID 1).vscode-remote-try-node) deploys and runs (Node 18, Debian 11).cliErrorin machine mode.Not included
fpm, so it cannot build the arm64.debnatively. Tracked as a separate concern.Summary by CodeRabbit
New Features
XDG_DATA_HOMEwhen locating its application data and host-accessible executable.Bug Fixes