Skip to content

Commit 4615531

Browse files
authored
Enhance variable handling for Shell and Script tasks (#57)
* Enhance variable handling in Shell and Script tasks to ensure raid variables are accessible in subprocesses and maintain precedence over OS environment variables * bump * Enhance environment variable handling in subprocesses to drop invalid entries and ensure proper inheritance in Script tasks
1 parent b8024d5 commit 4615531

4 files changed

Lines changed: 210 additions & 1 deletion

File tree

site/docs/features/variables.mdx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,27 @@ environments:
121121
value: "http://localhost:$API_PORT"
122122
```
123123

124+
### Inside Script and Shell subprocesses
125+
126+
Raid variables are also exported as environment variables to the subprocesses that Shell and Script tasks spawn. A `Set` task earlier in the sequence is visible inside the script's source as `$VAR`:
127+
128+
```yaml
129+
tasks:
130+
- type: Set
131+
var: "DEPLOY_ENV"
132+
value: "staging"
133+
- type: Script
134+
path: "./scripts/deploy.sh" # can reference $DEPLOY_ENV directly
135+
```
136+
137+
```sh
138+
# scripts/deploy.sh
139+
echo "deploying to $DEPLOY_ENV"
140+
kubectl apply -f "manifests/$DEPLOY_ENV/"
141+
```
142+
143+
The same applies to a child process spawned from a Shell task — the child inherits the shell's env, which includes raid variables. When a raid variable name collides with one already in the OS environment, the raid value wins (matching the precedence used by raid's own field-level expansion).
144+
124145
## Shell pass-through
125146

126147
When raid expands variables for a Shell task, unknown variables are passed through to the shell intact. This means shell-local variables and parameter expansions work as expected:

src/internal/lib/task_runner.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ func execShell(task Task) error {
220220
if task.Path != "" {
221221
cmd.Dir = sys.ExpandPath(task.Path)
222222
}
223+
cmd.Env = buildSubprocessEnv()
223224
setCmdOutput(cmd)
224225

225226
runErr := cmd.Run()
@@ -319,6 +320,7 @@ func execScript(task Task) error {
319320
cmd = exec.Command(task.Path)
320321
}
321322

323+
cmd.Env = buildSubprocessEnv()
322324
setCmdOutput(cmd)
323325

324326
err := cmd.Run()
@@ -335,6 +337,60 @@ func setCmdOutput(cmd *exec.Cmd) {
335337
cmd.Stdin = os.Stdin
336338
}
337339

340+
// buildSubprocessEnv returns the OS environment merged with the current
341+
// commandSession exports and raidVars (Set tasks). Order matters: exec.Cmd
342+
// uses the LAST occurrence of a duplicate key, so we append in increasing
343+
// priority — OS env first, session next, raidVars last so they win on
344+
// collision. Mirrors the lookup order used by expandRaid.
345+
//
346+
// Applied to Shell and Script tasks so a `Set FOO bar` task earlier in the
347+
// sequence is visible to the spawned process (and any children it spawns)
348+
// as $FOO. Without this, raidVars only resolved via raid's pre-expansion of
349+
// the cmd string — which couldn't reach into a Script's source or into a
350+
// child process spawned from inside a Shell command.
351+
//
352+
// Entries with empty keys or with NUL / "=" bytes in the key (or NUL bytes
353+
// in the value) are silently dropped — those would either be re-parsed as
354+
// a different key at exec time or get rejected outright by the OS.
355+
// Defensive guard since Set / Prompt task input isn't yet schema-constrained.
356+
func buildSubprocessEnv() []string {
357+
env := os.Environ()
358+
if commandSession != nil {
359+
commandSession.mu.RLock()
360+
for k, v := range commandSession.vars {
361+
if validEnvPair(k, v) {
362+
env = append(env, k+"="+v)
363+
}
364+
}
365+
commandSession.mu.RUnlock()
366+
}
367+
raidVarsMu.RLock()
368+
for k, v := range raidVars {
369+
if validEnvPair(k, v) {
370+
env = append(env, k+"="+v)
371+
}
372+
}
373+
raidVarsMu.RUnlock()
374+
return env
375+
}
376+
377+
// validEnvPair reports whether (key, value) is safe to inject into a
378+
// subprocess environment. NUL bytes terminate C strings and would corrupt
379+
// the entry; "=" in the key would be re-split into a different key by
380+
// exec at process-start time.
381+
func validEnvPair(key, value string) bool {
382+
if key == "" {
383+
return false
384+
}
385+
if strings.ContainsAny(key, "=\x00") {
386+
return false
387+
}
388+
if strings.ContainsRune(value, '\x00') {
389+
return false
390+
}
391+
return true
392+
}
393+
338394
func execHTTP(task Task) error {
339395
task = task.Expand()
340396

src/internal/lib/task_runner_test.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package lib
22

33
import (
4+
"bytes"
45
"net"
56
"net/http"
67
"net/http/httptest"
@@ -1277,3 +1278,134 @@ func writeTempScript(t *testing.T, content string) string {
12771278
}
12781279
return path
12791280
}
1281+
1282+
// withRaidVar sets a raid variable for the duration of the test, restoring
1283+
// the previous raidVars map on cleanup.
1284+
func withRaidVar(t *testing.T, key, value string) {
1285+
t.Helper()
1286+
raidVarsMu.Lock()
1287+
prev := raidVars
1288+
raidVars = make(map[string]string, len(prev)+1)
1289+
for k, v := range prev {
1290+
raidVars[k] = v
1291+
}
1292+
raidVars[key] = value
1293+
raidVarsMu.Unlock()
1294+
t.Cleanup(func() {
1295+
raidVarsMu.Lock()
1296+
raidVars = prev
1297+
raidVarsMu.Unlock()
1298+
})
1299+
}
1300+
1301+
// captureCommandStdout swaps commandStdout for a buffer, returning a getter
1302+
// that returns the captured text and restores the previous writer on
1303+
// cleanup.
1304+
func captureCommandStdout(t *testing.T) func() string {
1305+
t.Helper()
1306+
var buf bytes.Buffer
1307+
prev := commandStdout
1308+
commandStdout = &buf
1309+
t.Cleanup(func() { commandStdout = prev })
1310+
return func() string { return buf.String() }
1311+
}
1312+
1313+
// --- Issue #20: Set variables reach Script + Shell subprocess env ---------
1314+
1315+
// TestExecScript_inheritsRaidVar guards the issue-#20 fix: a Set task's
1316+
// variable must be visible in the env of a subsequent Script task's
1317+
// subprocess.
1318+
func TestExecScript_inheritsRaidVar(t *testing.T) {
1319+
if runtime.GOOS == "windows" {
1320+
t.Skip("direct .sh execution not supported on Windows")
1321+
}
1322+
withRaidVar(t, "ISSUE20_FOO", "from-raid-var")
1323+
getOut := captureCommandStdout(t)
1324+
1325+
scriptPath := writeTempScript(t, "#!/bin/sh\nprintf 'got=%s' \"$ISSUE20_FOO\"\n")
1326+
1327+
if err := execScript(Task{Type: Script, Path: scriptPath}); err != nil {
1328+
t.Fatalf("execScript: %v", err)
1329+
}
1330+
if got := strings.TrimSpace(getOut()); got != "got=from-raid-var" {
1331+
t.Errorf("script saw $ISSUE20_FOO = %q, want %q", got, "got=from-raid-var")
1332+
}
1333+
}
1334+
1335+
// TestExecScript_raidVarOverridesOSEnv confirms the precedence promised by
1336+
// expandRaid: raidVars beat OS env when names collide. The fix appends
1337+
// raidVars last in cmd.Env so exec's last-occurrence-wins rule honors that.
1338+
func TestExecScript_raidVarOverridesOSEnv(t *testing.T) {
1339+
if runtime.GOOS == "windows" {
1340+
t.Skip("direct .sh execution not supported on Windows")
1341+
}
1342+
const key = "ISSUE20_OVERRIDE"
1343+
t.Setenv(key, "from-os")
1344+
withRaidVar(t, key, "from-raid")
1345+
getOut := captureCommandStdout(t)
1346+
1347+
scriptPath := writeTempScript(t, "#!/bin/sh\nprintf 'got=%s' \"$ISSUE20_OVERRIDE\"\n")
1348+
1349+
if err := execScript(Task{Type: Script, Path: scriptPath}); err != nil {
1350+
t.Fatalf("execScript: %v", err)
1351+
}
1352+
if got := strings.TrimSpace(getOut()); got != "got=from-raid" {
1353+
t.Errorf("script saw collision = %q, want raidVar to win (got=from-raid)", got)
1354+
}
1355+
}
1356+
1357+
// TestExecShell_passesRaidVarToChildScript ensures the same fix applies to
1358+
// Shell tasks: a child process spawned by the shell (e.g. another script)
1359+
// sees the raidVar even though the shell itself didn't pre-expand it.
1360+
// `literal: true` skips raid's pre-expansion so resolution happens at the
1361+
// child level instead.
1362+
func TestExecShell_passesRaidVarToChildScript(t *testing.T) {
1363+
if runtime.GOOS == "windows" {
1364+
t.Skip("direct .sh execution not supported on Windows")
1365+
}
1366+
withRaidVar(t, "ISSUE20_BAR", "from-raid-bar")
1367+
getOut := captureCommandStdout(t)
1368+
1369+
dir := t.TempDir()
1370+
childPath := filepath.Join(dir, "child.sh")
1371+
if err := os.WriteFile(childPath, []byte("#!/bin/sh\nprintf 'child=%s' \"$ISSUE20_BAR\"\n"), 0755); err != nil {
1372+
t.Fatalf("write child: %v", err)
1373+
}
1374+
1375+
task := Task{Type: Shell, Cmd: childPath, Literal: true}
1376+
if err := execShell(task); err != nil {
1377+
t.Fatalf("execShell: %v", err)
1378+
}
1379+
if got := strings.TrimSpace(getOut()); got != "child=from-raid-bar" {
1380+
t.Errorf("child saw $ISSUE20_BAR = %q, want %q", got, "child=from-raid-bar")
1381+
}
1382+
}
1383+
1384+
// TestBuildSubprocessEnv_orderRaidVarsLast directly exercises the helper to
1385+
// guard the precedence wiring (raidVars must appear AFTER OS env so exec's
1386+
// duplicate-key resolution gives them priority).
1387+
func TestBuildSubprocessEnv_orderRaidVarsLast(t *testing.T) {
1388+
const key = "ISSUE20_BUILD_ENV"
1389+
t.Setenv(key, "os-value")
1390+
withRaidVar(t, key, "raid-value")
1391+
1392+
env := buildSubprocessEnv()
1393+
osIdx, raidIdx := -1, -1
1394+
for i, kv := range env {
1395+
switch kv {
1396+
case key + "=os-value":
1397+
osIdx = i
1398+
case key + "=raid-value":
1399+
raidIdx = i
1400+
}
1401+
}
1402+
if osIdx < 0 {
1403+
t.Fatalf("buildSubprocessEnv missing OS-set %q", key)
1404+
}
1405+
if raidIdx < 0 {
1406+
t.Fatalf("buildSubprocessEnv missing raid-set %q", key)
1407+
}
1408+
if raidIdx < osIdx {
1409+
t.Errorf("raid-set entry at %d came before OS entry at %d; exec uses last-occurrence so raidVar must come last", raidIdx, osIdx)
1410+
}
1411+
}

src/resources/app.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
version=0.7.0-beta
1+
version=0.7.1-beta
22
environment=development

0 commit comments

Comments
 (0)