Fix -env silently not forwarding for -- <cmd> mode (only worked for -script)
Two-layer bug, found while diagnosing why chromium-fork's sccache/MinIO
credentials never reached the remote autoninja invocations in ci runs
#69/#70 (which use exactly this pattern: -env AWS_ACCESS_KEY_ID -env
AWS_SECRET_ACCESS_KEY -- 'export ... && autoninja ...'):
1. buildPayload() only ever built the env map inside its -script branch.
The plain -- <cmd> branch returned remotePayload{command: ...} with
env left nil -- so -env flags on a -- <cmd> invocation were silently
accepted by the CLI parser and then just... dropped. No forwarding,
and no validation that the named vars even existed locally either
(LookupEnv was never called for this path at all).
2. Even had (1) not existed, sshExec (the function -- <cmd> actually
calls) took no env parameter whatsoever -- there was nowhere for a
populated env map to go. sshExecScript (the -script path) already
prefixed `export KEY=value` lines onto its script correctly; sshExec
never did anything equivalent.
Fixed both: buildPayload now builds env unconditionally before branching
on script-vs-command, and sshExec now accepts env and prefixes `export
KEY='value'; ` onto the command string, mirroring sshExecScript.
Verified against a real box: `-env TESTVAR -- 'echo "TESTVAR=$TESTVAR"'`
printed empty before this fix, prints the real value after.
This commit is contained in:
parent
64914555c7
commit
19d5c6eecb
1 changed files with 43 additions and 16 deletions
59
main.go
59
main.go
|
|
@ -215,26 +215,35 @@ func bindPayloadFlags(fs *flag.FlagSet) (scriptPath *string, envNames *stringSli
|
|||
return scriptPath, envNames
|
||||
}
|
||||
|
||||
// buildPayload builds env (from -env flags) unconditionally, regardless of
|
||||
// whether -script or a plain -- command is used -- both need it forwarded
|
||||
// to the remote side. Previously env was only ever built inside the
|
||||
// -script branch; a plain -- command silently ignored -env entirely (no
|
||||
// forwarding *and* no validation that the named vars even existed
|
||||
// locally), found while diagnosing why chromium-fork's sccache/MinIO
|
||||
// credentials (forwarded via -env against -- 'autoninja ...' steps) never
|
||||
// reached the remote build at all.
|
||||
func buildPayload(subcmd string, scriptPath *string, envNames *stringSlice, remainder []string) remotePayload {
|
||||
env := map[string]string{}
|
||||
for _, name := range *envNames {
|
||||
v, ok := os.LookupEnv(name)
|
||||
if !ok {
|
||||
fatal("%s: -env %s: not set in this process's environment", subcmd, name)
|
||||
}
|
||||
env[name] = v
|
||||
}
|
||||
|
||||
if *scriptPath != "" {
|
||||
script, err := os.ReadFile(*scriptPath)
|
||||
if err != nil {
|
||||
fatal("%s: reading -script %q: %v", subcmd, *scriptPath, err)
|
||||
}
|
||||
env := map[string]string{}
|
||||
for _, name := range *envNames {
|
||||
v, ok := os.LookupEnv(name)
|
||||
if !ok {
|
||||
fatal("%s: -env %s: not set in this process's environment", subcmd, name)
|
||||
}
|
||||
env[name] = v
|
||||
}
|
||||
return remotePayload{script: string(script), env: env}
|
||||
}
|
||||
if len(remainder) == 0 {
|
||||
fatal("%s: usage: hetzner-runner %s [flags] -- <remote command...> (or -script <path>)", subcmd, subcmd)
|
||||
}
|
||||
return remotePayload{command: strings.Join(remainder, " ")}
|
||||
return remotePayload{command: strings.Join(remainder, " "), env: env}
|
||||
}
|
||||
|
||||
func bindConfig(fs *flag.FlagSet, cfg *config) {
|
||||
|
|
@ -352,7 +361,7 @@ func runExecCommand(client *hcloud.Client, st *state, payload remotePayload, upl
|
|||
if payload.script != "" {
|
||||
return sshExecScript(st, payload.script, payload.env)
|
||||
}
|
||||
return sshExec(st, payload.command)
|
||||
return sshExec(st, payload.command, payload.env)
|
||||
})
|
||||
if runErr != nil {
|
||||
dlog("exec: Go-level FAILURE (cleaning up): %v", runErr)
|
||||
|
|
@ -441,7 +450,7 @@ func runLifecycle(client *hcloud.Client, token string, cfg config, payload remot
|
|||
if payload.script != "" {
|
||||
return sshExecScript(st, payload.script, payload.env)
|
||||
}
|
||||
return sshExec(st, payload.command)
|
||||
return sshExec(st, payload.command, payload.env)
|
||||
})
|
||||
if err != nil {
|
||||
dlog("runLifecycle: remote command FAILED: %v", err)
|
||||
|
|
@ -573,7 +582,7 @@ func waitForSSH(st *state, timeout time.Duration) error {
|
|||
// TCP is up; give sshd + cloud-init user creation a moment,
|
||||
// then confirm with a real SSH handshake + command.
|
||||
time.Sleep(3 * time.Second)
|
||||
if code, err := sshExec(st, "true"); err == nil && code == 0 {
|
||||
if code, err := sshExec(st, "true", nil); err == nil && code == 0 {
|
||||
return nil
|
||||
} else {
|
||||
lastErr = err
|
||||
|
|
@ -586,7 +595,19 @@ func waitForSSH(st *state, timeout time.Duration) error {
|
|||
return fmt.Errorf("timed out after %s, last error: %v", timeout, lastErr)
|
||||
}
|
||||
|
||||
func sshExec(st *state, command string) (int, error) {
|
||||
// sshExec runs command via a plain SSH exec request (no stdin -- see
|
||||
// sshSession/sshExecScript for why that's what makes a PTY here safe).
|
||||
//
|
||||
// env is prefixed onto the command as `export KEY='value'; ...`. Found
|
||||
// missing entirely (env silently accepted by the CLI flags, validated as
|
||||
// present locally, then never actually forwarded) while diagnosing why
|
||||
// chromium-fork's sccache/MinIO credentials never reached the remote
|
||||
// `autoninja` invocations in ci runs #69/#70 -- every `-- <cmd>` step
|
||||
// using `-env AWS_ACCESS_KEY_ID -env AWS_SECRET_ACCESS_KEY` had those
|
||||
// values validated against *this* process's environment and then
|
||||
// dropped, never exported into the remote command's. sshExecScript
|
||||
// (the -script path) already did this correctly; -- <cmd> never did.
|
||||
func sshExec(st *state, command string, env map[string]string) (int, error) {
|
||||
session, client, err := sshSession(st, true)
|
||||
if err != nil {
|
||||
return -1, err
|
||||
|
|
@ -594,9 +615,15 @@ func sshExec(st *state, command string) (int, error) {
|
|||
defer client.Close()
|
||||
defer session.Close()
|
||||
|
||||
var buf strings.Builder
|
||||
for k, v := range env {
|
||||
fmt.Fprintf(&buf, "export %s=%s; ", k, shellQuote(v))
|
||||
}
|
||||
buf.WriteString(command)
|
||||
|
||||
session.Stdout = os.Stdout
|
||||
session.Stderr = os.Stderr
|
||||
return runSession(session, command)
|
||||
return runSession(session, buf.String())
|
||||
}
|
||||
|
||||
// uploadFile copies a local file to the remote box. See uploadBytes for
|
||||
|
|
@ -739,8 +766,8 @@ func sshExecScript(st *state, script string, env map[string]string) (int, error)
|
|||
if err := uploadBytes(st, []byte(buf.String()), remotePath); err != nil {
|
||||
return -1, fmt.Errorf("uploading script: %w", err)
|
||||
}
|
||||
code, err := sshExec(st, fmt.Sprintf("bash %s", shellQuote(remotePath)))
|
||||
if rmCode, rmErr := sshExec(st, fmt.Sprintf("rm -f %s", shellQuote(remotePath))); rmErr == nil && rmCode != 0 {
|
||||
code, err := sshExec(st, fmt.Sprintf("bash %s", shellQuote(remotePath)), nil)
|
||||
if rmCode, rmErr := sshExec(st, fmt.Sprintf("rm -f %s", shellQuote(remotePath)), nil); rmErr == nil && rmCode != 0 {
|
||||
fmt.Fprintf(os.Stderr, "warning: failed to remove remote script %s\n", remotePath)
|
||||
}
|
||||
return code, err
|
||||
|
|
|
|||
Loading…
Reference in a new issue