fix: robustness and injection hardening in action_kit_commons

[joshiste]

Summary

The final action-kit kit-audit item: robustness fixes across action_kit_commons (crash paths + privileged-command injection hardening). Grouped into cohesive commits; all in one PR per request.

Fixes

Crash / panic guards

• ociruntime: cmd.ProcessState.ExitCode() is read right after cmd.Run() — nil-deref if the runtime binary never started; guard with a -1 sentinel. unmarshalGuarded indexed output[0] with no length check — index panic on empty runtime output.
• netfault (bandwidth/delay/packageLoss/packageCorruption): len(cmds)/len(o.Interfaces) divide-by-zero when an attack has no interfaces; guarded with len(o.Interfaces) > 0 (matching tcp_reset).
• process wrappers (stress/memfill/diskfill): Exited()/Stop() dereferenced the background state / exec Process that are only set on a successful Start, so calling them after a failed/absent Start panicked. Guarded — Exited() reports "exited" when never started, Stop() is a no-op.

Injection hardening (defense-in-depth)

• Commands go one-per-line to tc/iptables -batch stdin and hostnames one-per-line to dig -f-. A newline in an unsanitized interface name / rate / hostname forwarded from user input would inject an extra privileged directive/query. Reject any batch command containing a newline (single validateBatchCommands at the 3 runner choke points) and any hostname containing whitespace (at the dig-stdin sink). All exec already uses argv, so there's no shell-metachar injection; this closes the batch-stdin vector.
• Also: capture the pid before the SIGTERM timer goroutine (removes a data race with the exec waiter), and skip un-parseable IPs in DNS resolution instead of appending nil.

Deliberately not changed / follow-ups

• ociruntime.ConfigFromEnvironment keeps log.Fatal — it's startup env-config parsing (unrecoverable misconfig, fail-fast is the codebase convention); changing it would ripple an error return through every caller. This PR is scoped to runtime crash paths.
• Optional refactors flagged by /simplify, not done here: extract the repeated stress/memfill/diskfill Stop/Exited block into a shared utils.StopBackgroundProcess helper; and express the netfault "≥1 interface" invariant once instead of four inline guards.
• The injection guard is containment (blocks directive injection); full input validation (interface charset, rate format) belongs upstream in the extensions, not this SDK.

Verification

go build ./..., go vet ./..., go test ./... pass (module go/action_kit_commons). New tests: validateBatchCommands (newline/CR rejection) and dnsresolve whitespace-hostname rejection.

[claude]

Claude finished @joshiste's task in 2m 38s —— View job

---

Review

• Check for previously unresolved review threads from Claude and see if fixed (none found)
• Gather context / read diff (git diff origin/main...HEAD)
• Review crash/panic guard fixes (ociruntime, netfault, process wrappers)
• Review injection hardening (tc/iptables/dig batch validation)
• Post review feedback

No unresolved prior review threads exist on this PR, so there was nothing to re-check.

Summary: I read through all 16 changed files against origin/main...HEAD. The changes are small, well-scoped, and each fix matches its stated purpose:

• ociruntime.go: cmd.ProcessState nil-guard before ExitCode(), and the output[0] bounds check in unmarshalGuarded — both correctly guard the only paths where these could be nil/empty (process never started, empty runtime output).
• netfault divide-by-zero guards (bandwidth.go, delay.go, packageCorruption.go, packageLoss.go): consistent len(o.Interfaces) > 0 && guard added in all four spots, matching the existing tcp_reset pattern.
• validateBatchCommands (utils.go) is wired into all three stdin choke points that write batch commands (runner_process.go, and both branches of runner_runc.go) — confirmed there are no other toReader(cmds) call sites that were missed. Rejecting \n/\r before the commands hit tc/iptables -batch stdin closes the injection vector described in the PR body; the corresponding test (utils_test.go) covers the clean, \n, and \r cases.
• dnsresolve/resolve.go: rejecting hostnames containing whitespace (via unicode.IsSpace) before they're joined into dig -f- stdin is correct and covered by the added test. Also good: net.ParseIP is now checked for nil before appending, avoiding a nil entry in the resolved-IP slice on a malformed dig answer line.
• Process wrapper guards (stress_process.go, memfill_process.go, diskfill_process.go, diskfill_runc.go): Exited()/Stop() now check state/cmd.Process for nil before dereferencing, correctly handling the "never started" / "Start failed" path. Traced through utils.RunCommandInBackground — state is only ever non-nil after cmd.Start() succeeds, so cmd.Process is guaranteed set whenever state is, meaning these guards are safe (if slightly redundant with each other) and don't change behavior on the happy path.
• Pid-capture-before-timer-closure change is a reasonable defensive tightening; didn't find a call path where it fixes an actively exploitable data race today (cmd.Process isn't reassigned after Start), but it's harmless and does remove a theoretical footgun if that ever changes.

I didn't find any bugs, security issues, or race conditions in the new code. The batch/dig validation is placed correctly (before stdin is ever written to) at every sink, and the nil-guards are consistent across all four process-wrapper implementations.

One note on my process: I was not able to run go build/go vet/go test in this sandbox (the go command itself requires approval I don't have in this context), so this review is based on manual reading of the diff rather than a green build/test confirmation. The PR description states these were run and passed locally.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud