Bound ShellApp.Run wait on inherited stdout/stderr pipes to prevent task hangs

[Copilot]

This PR addresses the review concern that ShellApp.Run can block indefinitely when a script exits but leaves background descendants holding inherited stdout/stderr descriptors open. The change keeps output draining behavior while decoupling task completion from orphaned pipe lifetimes.

• Problem scope

  • ShellApp.Run waited on log-drain completion with no upper bound; background children inheriting pipes could prevent EOF and stall completion.

• Runtime behavior change

  • Set a bounded exec.Cmd.WaitDelay in ShellApp.Run to cap pipe-drain wait time after process exit.
  • Preserve WaitLog() call so normal stdout/stderr draining still occurs.
  • Handle exec.ErrWaitDelay explicitly: log that output draining was cut short and treat it as non-fatal for successful command exit.

• Regression coverage

  • Added focused ShellApp test exercising a command that backgrounds a child (sleep ... &) and asserting Run returns promptly instead of hanging.

cmd := t.makeShellCmd(cliArgs, args.EnvironmentVars)
cmd.WaitDelay = shellAppWaitDelay

err := cmd.Wait()
t.Logger.WaitLog()

if errors.Is(err, exec.ErrWaitDelay) {
    t.Logger.Logf("shell command output draining exceeded %s and was stopped", shellAppWaitDelay)
    return nil
}
return err

[cursor]

Stale comment

Security review — no findings

Reviewed PR #4161 at d44cfb3b (ready_for_review). The branch contains a single Initial plan commit with no added or modified source files (files: [] on GitHub; git diff develop...HEAD is empty).

Because there is no changed code to analyze, I did not identify any medium, high, or critical vulnerabilities introduced or exposed by this PR.

Prior automation threads: none found on this PR.

Note: This PR is still WIP relative to its stated goal (applying the review comment from PR #4159). Re-run this security review once implementation commits land.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review — no findings

Reviewed PR #4161 at ed4c877d (ShellApp WaitDelay handling).

Scope: db_lib/ShellApp.go, db_lib/ShellApp_test.go

Prior threads: No unresolved inline security-review threads from earlier automation runs.

Assessment: No medium, high, or critical vulnerabilities identified in the added/modified code.

Reviewed paths:

• cmd.WaitDelay = shellAppWaitDelay (5s cap on post-exit pipe draining)
• errors.Is(err, exec.ErrWaitDelay) branch returning nil

Why no finding:

• Attacker-controlled input (CliArgs) already reaches exec.Command via existing ShellApp paths; this PR does not expand execution surface or bypass auth.
• Per Go os/exec semantics, exec.ErrWaitDelay is returned only when the child already exited successfully while stdout/stderr pipes remain open. Non-zero exits still propagate via cmd.Wait(); the new branch does not mask failed commands.
• The change improves runner availability by preventing indefinite hangs on inherited background pipe handles (DoS/resilience), without introducing injection, authz bypass, secret leakage, or traversal/deserialization risks.

No inline findings to report.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Security review — no findings

Reviewed PR #4161 at df7f522c (ShellApp WaitDelay handling + pro/go.mod tidy).

Scope: db_lib/ShellApp.go, db_lib/ShellApp_test.go, pro/go.mod, pro/go.sum

Prior threads: No unresolved inline security-review threads from earlier automation runs. Previous top-level assessments (empty WIP commit, ed4c877d review) validated against current HEAD.

Assessment: No medium, high, or critical vulnerabilities identified in the added/modified code.

Reviewed paths:

• cmd.WaitDelay = shellAppWaitDelay (5s cap on post-exit pipe draining)
• errors.Is(err, exec.ErrWaitDelay) branch returning nil
• Indirect dependency bumps in pro/go.mod / pro/go.sum

Why no finding:

• Attacker-controlled input (CliArgs) already reaches exec.Command via existing ShellApp paths; this PR does not expand execution surface or bypass auth.
• Per Go os/exec semantics (Cmd.Wait at Go 1.26), exec.ErrWaitDelay is returned only when the child already exited successfully while stdout/stderr pipes remain open. Non-zero exits still propagate via cmd.Wait() (ExitError is preferred over goroutine/pipe errors); the new branch does not mask failed commands.
• The change improves runner availability by preventing indefinite hangs on inherited background pipe handles (resilience), without introducing injection, authz bypass, secret leakage, or traversal/deserialization risks.
• pro/go.mod changes are indirect dependency version bumps (e.g. go-git 5.19.1→5.19.2); no new direct runtime dependencies or known high-severity CVEs identified in this diff.

No inline findings to report.

  

_{Sent by Cursor Automation: Find vulnerabilities}