fix: make fduty resolvable by bare name in BYOC sandboxes (single writable tools dir + hard-fail self-check + bundling)#54
Open
ysyneu wants to merge 4 commits into
Open
Conversation
`fduty` was unreachable by bare name inside BYOC self-hosted runner
sandboxes (`bash: fduty: command not found`, exit 127). The runner held
two divergent notions of "where fduty lives":
- PATH dir = BundledToolsDir() = dir(os.Executable()) = /usr/local/bin,
which is read-only under the systemd unit's ProtectSystem=strict (no
sudo, NoNewPrivileges).
- Install dir = the same value as FLASHDUTY_INSTALL_DIR, but flashduty-cli's
install.sh relocates to ~/.local/bin when the target is unwritable — a
dir never on the bash tool's PATH. -> 127.
Collapse install dir and PATH dir into ONE runner-owned, guaranteed-writable,
deterministic directory so they cannot diverge by construction:
- BundledToolsDir() now defaults to <runner home>/bin (home from
FLASHDUTY_RUNNER_HOME, deprecated FLASHDUTY_RUNNER_WORKSPACE alias, else
~/.flashduty), under systemd ReadWritePaths=${STATE_DIR} and writable.
FLASHDUTY_RUNNER_BIN_DIR still overrides outright, so the AGS cloud image
(baked /usr/local/bin/fduty) is unaffected. No PATH enumeration.
- cmd pins FLASHDUTY_RUNNER_HOME from the resolved config before provisioning
so the --workspace flag path can't desync the tools dir.
- install.sh creates + chowns ${WORKSPACE_DIR}/bin (== the runtime tools dir).
- ensureFdutyCLI now returns an error and HARD-FAILS startup: after staging,
it runs `fduty version` through the EXACT bash-tool env
(environment.BashToolEnv: secrets scrubbed, bundled dir first on PATH) via
`bash -c` and asserts exit 0. Running it directly with exec.Command would
miss the bundled dir — Go's LookPath ignores cmd.Env — so it goes through
bash exactly like the tool does. A runner that boots healthy but 127s every
fduty call is worse than one that refuses to start.
- New BashToolEnv() is the single source of truth for the bash subprocess env
(executeBashCommand now uses it too), so the self-check validates the real
call-time PATH.
Tests: BundledToolsDir default/alias/override resolution; copyExecutable;
provisionFduty no-op/no-source-error; verifyFdutyOnPath pass/fail via a stub
fduty on PATH.
Remove the boot-time network + installer fragility: ship the matched-os/arch
fduty CLI inside each runner release archive so the runner stages it from disk
at first run instead of curl|sh'ing the CDN installer.
- scripts/bundle-fduty.sh (goreleaser `before` hook) downloads the pinned
flashduty-cli release (v1.3.4) for every os/arch the runner builds, extracts
the binary, and lays it out as .fduty-bundle/<goos>_<goarch>/fduty(.exe).
Best-effort per os/arch: a missing asset just contributes nothing.
- .goreleaser.yaml: run the hook, and add a templated archives.files glob
(.fduty-bundle/{{ .Os }}_{{ .Arch }}/fduty*) so each archive ships fduty in
its root next to the runner binary.
- install.sh: install_bundled_fduty stages the extracted fduty into the
writable tools dir (${WORKSPACE_DIR}/bin) at install time.
The runner keeps the CDN install path as a fallback when the bundled binary is
absent (old archives, skipped os/arch), so this packaging change is purely
additive.
Verified with `goreleaser release --snapshot`: hook fetched real v1.3.4
binaries, every tar.gz/zip contains fduty(.exe), the bundled darwin/arm64
binary runs (`flashduty version 1.3.4`), and an extract+stage+`bash -c 'fduty
version'` through a prepended-PATH exits 0 — the production self-check path.
The Windows build job runs the unit tests; the new tests assumed POSIX: - BundledToolsDir expectations hardcoded forward-slash paths — compare against filepath.Join output instead so they match the backslash result on Windows. - copyExecutable's owner-executable assertion is Unix-only (Windows has no exec bit) — gate it behind runtime.GOOS != "windows". Production code already uses filepath.Join throughout; only the test assertions were non-portable.
On Windows os.UserHomeDir reads %USERPROFILE%, not $HOME, so the previous
t.Setenv("HOME", ...) override was ignored and the assertion compared the temp
dir against the real C:\Users home. Compute the expected value from
os.UserHomeDir() — the same call BundledToolsDir makes — so the test is correct
on every OS without depending on which env var maps to home.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (root cause)
fdutyis unreachable by bare name inside BYOC self-hosted runner sandboxes —bash: fduty: command not found(exit 127). The runner held two divergent notions of "where fduty lives":BundledToolsDir()=filepath.Dir(os.Executable())=/usr/local/bin, which is read-only under the systemd unit'sProtectSystem=strict(no sudo,NoNewPrivileges).FLASHDUTY_INSTALL_DIR, but flashduty-cli'sinstall.shrelocates to$HOME/.local/binwhen the target is unwritable — a dir never added to the bash tool's PATH. → 127.End-state: one runner-owned, writable, deterministic tools dir
Collapse "install dir" and "PATH dir" into a single directory so they cannot diverge by construction. No PATH enumeration (explicitly out of scope — brittle across environments).
BundledToolsDir()now defaults to<runner home>/bin— home resolved the same waycmdresolves it (FLASHDUTY_RUNNER_HOME> deprecatedFLASHDUTY_RUNNER_WORKSPACEalias >~/.flashduty). Under the systemd unit that root is insideReadWritePaths=${STATE_DIR}, so it's writable even withProtectSystem=strict— install.sh never takes its~/.local/binfallback branch. The same value feeds both the PATH prepend andFLASHDUTY_INSTALL_DIR.FLASHDUTY_RUNNER_BIN_DIRstill overrides outright, so the AGS cloud image (baked/usr/local/bin/fduty,FLASHDUTY_RUNNER_BIN_DIR=/usr/local/bin) is unaffected.cmdpinsFLASHDUTY_RUNNER_HOMEfrom the resolved config before provisioning so the--workspaceflag path can't desync the tools dir from the workspace.install.shcreates + chowns${WORKSPACE_DIR}/bin(== the runtime tools dir; under the existing recursive chown /ReadWritePaths). No unit change.ensureFdutyCLInow returns an error and aborts startup. After staging, it runsfduty versionthrough the exact bash-tool env (newenvironment.BashToolEnv()— secrets scrubbed, bundled dir first on PATH) viabash -cand asserts exit 0. A runner that boots "healthy" but 127s every fduty call is worse than one that refuses to start.bash -c, notexec.Command("fduty", ...). Go'sLookPathresolves the program against the parent process PATH and ignorescmd.Env, so a direct exec would miss the bundled dir and not mirror real call-time resolution.Bundling (2nd commit) — remove boot-time network fragility
Ship the matched-os/arch
fdutyinside each runner release archive:scripts/bundle-fduty.sh(goreleaserbeforehook) downloads the pinned flashduty-cli release (v1.3.4) for every os/arch, extracts the binary →.fduty-bundle/<goos>_<goarch>/fduty(.exe)..goreleaser.yamlincludes it per archive via a templatedarchives.filesglob.install.shstages the extractedfdutyinto the writable tools dir at install time.Verification
go build ./...,go test ./...— all green (new tests:BundledToolsDirdefault/alias/override resolution;copyExecutable;provisionFdutyno-op + no-source-error;verifyFdutyOnPathpass/fail via a stub fduty on PATH).golangci-lint run ./...— 0 issues.shellcheck install.sh scripts/bundle-fduty.sh— clean.goreleaser check— config valid.goreleaser release --snapshot— hook fetched real v1.3.4 binaries; every tar.gz/zip containsfduty(.exe); bundled darwin/arm64 binary runs (flashduty version 1.3.4).install.shstage into<workspace>/bin→bash -c 'fduty version'with prepended PATH → exit 0 (the production self-check path).